Skip to content

Commit

Permalink
feat: support caching of verification kys (#81)
Browse files Browse the repository at this point in the history
## Summary

Closes #9 

Previously, clients who fetched verification keys from the server had no
way to cache those keys and reuse them for other requests.

This PR proposes a change using the GitHub API's [conditional
requests](https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api?apiVersion=2022-11-28#use-conditional-requests-if-appropriate)
feature: clients can optionally specify a cache for their keys and only
fetch new ones if the cache is outdated.

BREAKING CHANGE: `verifyRequestByKeyId() now returns an object with a
`isValid` property and a`cache` property.

Before

```js
const isValid = await verifyRequestByKeyId();
```

After

```js
const { isValid, cache } = await verifyRequestByKeyId();
```

BREAKING CHANGE: `fetchVerificationKeys()` now returns an object with a
`keys` property and a`cache` property.

Before

```js
const keys = await fetchVerificationKeys();
```

After

```js
const { keys, cache } = await fetchVerificationKeys();
```

---------

Co-authored-by: Gregor Martynus <[email protected]>
  • Loading branch information
francisfuzz and gr2m authored Sep 18, 2024
1 parent 38e210f commit 1f56d8e
Show file tree
Hide file tree
Showing 9 changed files with 4,448 additions and 4,242 deletions.
76 changes: 55 additions & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,16 @@ We consider this SDK alpha software in terms of API stability, but we adhere to
```js
import { verifyRequestByKeyId } from "@copilot-extensions/preview-sdk";

const payloadIsVerified = await verifyRequestByKeyId(
const { isValid, cache } = await verifyRequestByKeyId(
request.body,
signature,
keyId,
{
token: process.env.GITHUB_TOKEN,
},
);
// true or false
// isValid: true or false
// cache: { id, keys }
```

### Build a response
Expand Down Expand Up @@ -76,54 +77,82 @@ try {

Verify the request payload using the provided signature and key ID. The method will request the public key from GitHub's API for the given keyId and then verify the payload.

The `options` argument is optional. It can contain a `token` to authenticate the request to GitHub's API, or a custom `request` instance to use for the request.
The `requestOptions` argument is optional. It can contain:

- a `token` to authenticate the request to GitHub's API
- a custom [octokit `request`](https://github.com/octokit/request.js) instance to use for the request
- a `cache` to use cached keys

```js
import { verifyRequestByKeyId } from "@copilot-extensions/preview-sdk";

const payloadIsVerified = await verifyRequestByKeyId(
const { isValid, cache } = await verifyRequestByKeyId(
request.body,
signature,
key,
keyId,
);

// with token
await verifyRequestByKeyId(request.body, signature, key, { token: "ghp_1234" });
const { isValid, cache } = await verifyRequestByKeyId(
request.body,
signature,
keyId,
{ token: "ghp_1234" },
);

// with custom octokit request instance
await verifyRequestByKeyId(request.body, signature, key, { request });
const { isValid, cache } = await verifyRequestByKeyId(
request.body,
signature,
keyId,
{ request },
);

// with cache
const previousCache = {
id: "etag_value",
keys: [{ key_identifier: "key1", key: "public_key1", is_current: true }],
};
const { isValid, cache } = await verifyRequestByKeyId(
request.body,
signature,
keyId,
{ cache: previousCache },
);
```

#### `async fetchVerificationKeys(options)`

Fetches public keys for verifying copilot extension requests [from GitHub's API](https://api.github.com/meta/public_keys/copilot_api)
and returns them as an array. The request can be made without authentication, with a token, or with a custom [octokit request](https://github.com/octokit/request.js) instance.
Fetches public keys for verifying copilot extension requests [from GitHub's API](https://api.github.com/meta/public_keys/copilot_api) and returns them as an array. The request can be made without authentication, with a token, with a custom [octokit request](https://github.com/octokit/request.js) instance, or with a cache.

```js
import { fetchVerificationKeys } from "@copilot-extensions/preview-sdk";

// fetch without authentication
const [current] = await fetchVerificationKeys();
const { id, keys } = await fetchVerificationKeys();

// with token
const [current] = await fetchVerificationKeys({ token: "ghp_1234" });
const { id, keys } = await fetchVerificationKeys({ token: "ghp_1234" });

// with custom octokit request instance
const [current] = await fetchVerificationKeys({ request });)
const { id, keys } = await fetchVerificationKeys({ request });

// with cache
const cache = {
id: "etag_value",
keys: [{ key_identifier: "key1", key: "public_key1" }],
};
const { id, keys } = await fetchVerificationKeys({ cache });
```

#### `async verifyRequestPayload(rawBody, signature, keyId)`
#### `async verifyRequest(rawBody, signature, keyId)`

Verify the request payload using the provided signature and key. Note that the raw body as received by GitHub must be passed, before any parsing.

```js
import { verify } from "@copilot-extensions/preview-sdk";

const payloadIsVerified = await verifyRequestPayload(
request.body,
signature,
key,
);
const payloadIsVerified = await verifyRequest(request.body, signature, key);
// true or false
```

Expand Down Expand Up @@ -274,17 +303,22 @@ Convenience method to verify and parse a request in one go. It calls [`verifyReq
```js
import { verifyAndParseRequest } from "@copilot-extensions/preview-sdk";

const { isValidRequest, payload } = await verifyAndParseRequest(
request,
const { isValidRequest, payload, cache } = await verifyAndParseRequest(
request.body,
signature,
key,
keyId,
{
token: process.env.GITHUB_TOKEN,
},
);

if (!isValidRequest) {
throw new Error("Request could not be verified");
}

// `isValidRequest` is a boolean.
// `payload` has type support.
// `cache` contains the id and keys used for verification.
```
#### `getUserMessage()`
Expand Down
18 changes: 15 additions & 3 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@ import { request } from "@octokit/request";
// verification types

type RequestInterface = typeof request;
export type VerificationKeysCache = {
id: string;
keys: VerificationPublicKey[];
};
type RequestOptions = {
request?: RequestInterface;
token?: string;
cache?: VerificationKeysCache;
};
export type VerificationPublicKey = {
key_identifier: string;
Expand All @@ -18,7 +23,7 @@ interface VerifyRequestInterface {
}

interface FetchVerificationKeysInterface {
(requestOptions?: RequestOptions): Promise<VerificationPublicKey[]>;
(requestOptions?: RequestOptions): Promise<VerificationKeysCache>;
}

interface VerifyRequestByKeyIdInterface {
Expand All @@ -27,7 +32,10 @@ interface VerifyRequestByKeyIdInterface {
signature: string,
keyId: string,
requestOptions?: RequestOptions,
): Promise<boolean>;
): Promise<{
isValid: boolean;
cache: VerificationKeysCache;
}>;
}

// response types
Expand Down Expand Up @@ -188,7 +196,11 @@ export interface VerifyAndParseRequestInterface {
signature: string,
keyID: string,
requestOptions?: RequestOptions,
): Promise<{ isValidRequest: boolean; payload: CopilotRequestPayload }>;
): Promise<{
isValidRequest: boolean;
payload: CopilotRequestPayload;
cache: VerificationKeysCache;
}>;
}

export interface GetUserMessageInterface {
Expand Down
24 changes: 19 additions & 5 deletions index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
prompt,
PromptResult,
getFunctionCalls,
VerificationKeysCache,
} from "./index.js";

const token = "";
Expand All @@ -32,7 +33,10 @@ export async function verifyRequestByKeyIdTest(
keyId: string,
) {
const result = await verifyRequestByKeyId(rawBody, signature, keyId);
expectType<boolean>(result);
expectType<{
isValid: boolean;
cache: { id: string; keys: VerificationPublicKey[] };
}>(result);

// @ts-expect-error - first 3 arguments are required
verifyRequestByKeyId(rawBody, signature);
Expand All @@ -51,6 +55,11 @@ export async function verifyRequestByKeyIdTest(

// accepts a request argument
await verifyRequestByKeyId(rawBody, signature, keyId, { request });

// accepts a cache argument
await verifyRequestByKeyId(rawBody, signature, keyId, {
cache: { id: "test", keys: [] },
});
}

export async function verifyRequestTest(
Expand All @@ -76,13 +85,16 @@ export async function verifyRequestTest(

export async function fetchVerificationKeysTest() {
const result = await fetchVerificationKeys();
expectType<VerificationPublicKey[]>(result);
expectType<{ id: string; keys: VerificationPublicKey[] }>(result);

// accepts a token argument
await fetchVerificationKeys({ token });

// accepts a request argument
await fetchVerificationKeys({ request });

// accepts a cache argument
await fetchVerificationKeys({ cache: { id: "test", keys: [] } });
}

export function createAckEventTest() {
Expand Down Expand Up @@ -181,9 +193,11 @@ export async function verifyAndParseRequestTest(
keyId: string,
) {
const result = await verifyAndParseRequest(rawBody, signature, keyId);
expectType<{ isValidRequest: boolean; payload: CopilotRequestPayload }>(
result,
);
expectType<{
isValidRequest: boolean;
payload: CopilotRequestPayload;
cache: { id: string; keys: VerificationPublicKey[] };
}>(result);
}

export function getUserMessageTest(payload: CopilotRequestPayload) {
Expand Down
5 changes: 3 additions & 2 deletions lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,17 @@ export function transformPayloadForOpenAICompatibility(payload) {

/** @type {import('..').VerifyAndParseRequestInterface} */
export async function verifyAndParseRequest(body, signature, keyID, options) {
const isValidRequest = await verifyRequestByKeyId(
const { isValid, cache } = await verifyRequestByKeyId(
body,
signature,
keyID,
options
);

return {
isValidRequest,
isValidRequest: isValid,
payload: parseRequestBody(body),
cache,
};
}

Expand Down
40 changes: 28 additions & 12 deletions lib/verification.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { createVerify } from "node:crypto";

import { request as defaultRequest } from "@octokit/request";

/** @type {import('..').VerifyRequestByKeyIdInterface} */
/** @type {import('..').VerifyRequestInterface} */
export async function verifyRequest(rawBody, signature, key) {
// verify arguments
assertValidString(rawBody, "Invalid payload");
Expand All @@ -23,51 +23,67 @@ export async function verifyRequest(rawBody, signature, key) {

/** @type {import('..').FetchVerificationKeysInterface} */
export async function fetchVerificationKeys(
{ token = "", request = defaultRequest } = { request: defaultRequest }
{ token = "", request = defaultRequest, cache = { id: "", keys: [] } } = {
request: defaultRequest,
},
) {
const { data } = await request("GET /meta/public_keys/copilot_api", {
headers: token
try {
const headers = token
? {
Authorization: `token ${token}`,
}
: {},
});
: {};

if (cache.id) headers["if-none-match"] = cache.id;

return data.public_keys;
const response = await request("GET /meta/public_keys/copilot_api", {
headers,
});

const cacheId = response.headers.etag || "";
return { id: cacheId, keys: response.data.public_keys };
} catch (error) {
if (error.status === 304) {
return cache;
}

throw error;
}
}

/** @type {import('..').VerifyRequestByKeyIdInterface} */
export async function verifyRequestByKeyId(
rawBody,
signature,
keyId,
requestOptions
requestOptions,
) {
// verify arguments
assertValidString(rawBody, "Invalid payload");
assertValidString(signature, "Invalid signature");
assertValidString(keyId, "Invalid keyId");

// receive valid public keys from GitHub
const keys = await fetchVerificationKeys(requestOptions);
const { id, keys } = await fetchVerificationKeys(requestOptions);

// verify provided key Id
const publicKey = keys.find((key) => key.key_identifier === keyId);

if (!publicKey) {
const keyNotFoundError = Object.assign(
new Error(
"[@copilot-extensions/preview-sdk] No public key found matching key identifier"
"[@copilot-extensions/preview-sdk] No public key found matching key identifier",
),
{
keyId,
keys,
}
},
);
throw keyNotFoundError;
}

return verifyRequest(rawBody, signature, publicKey.key);
const isValid = await verifyRequest(rawBody, signature, publicKey.key);
return { isValid, cache: { id: id, keys } };
}

function assertValidString(value, message) {
Expand Down
Loading

0 comments on commit 1f56d8e

Please sign in to comment.