diff --git a/package-lock.json b/package-lock.json index 19e5307f..4888eb43 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "bottleneck": "^2.15.3" }, "devDependencies": { + "@octokit/auth-app": "^7.2.0", "@octokit/core": "^6.1.3", "@octokit/request-error": "^6.1.6", "@octokit/tsconfig": "^4.0.0", @@ -679,6 +680,76 @@ "@jridgewell/sourcemap-codec": "^1.4.14" } }, + "node_modules/@octokit/auth-app": { + "version": "7.2.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-app/-/auth-app-7.2.0.tgz", + "integrity": "sha512-js6wDY3SNLNZo5XwybhC8WKEw8BonEa9vqxN4IKbhEbo22i2+DinHxapV/PpFCTsmlkT1HMhF75xyOG9RVvI5g==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-app": "^8.1.3", + "@octokit/auth-oauth-user": "^5.1.3", + "@octokit/request": "^9.2.1", + "@octokit/request-error": "^6.1.7", + "@octokit/types": "^13.8.0", + "toad-cache": "^3.7.0", + "universal-github-app-jwt": "^2.2.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/auth-oauth-app": { + "version": "8.1.3", + "resolved": "https://registry.npmjs.org/@octokit/auth-oauth-app/-/auth-oauth-app-8.1.3.tgz", + "integrity": "sha512-4e6OjVe5rZ8yBe8w7byBjpKtSXFuro7gqeGAAZc7QYltOF8wB93rJl2FE0a4U1Mt88xxPv/mS+25/0DuLk0Ewg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-device": "^7.1.3", + "@octokit/auth-oauth-user": "^5.1.3", + "@octokit/request": "^9.2.1", + "@octokit/types": "^13.6.2", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/auth-oauth-device": { + "version": "7.1.4", + "resolved": "https://registry.npmjs.org/@octokit/auth-oauth-device/-/auth-oauth-device-7.1.4.tgz", + "integrity": "sha512-yK35I9VGDGjYxu0NPZ9Rl+zXM/+DO/Hu1VR5FUNz+ZsU6i8B8oQ43TPwci9nuH8bAF6rQrKDNR9F0r0+kzYJhA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/oauth-methods": "^5.1.4", + "@octokit/request": "^9.2.1", + "@octokit/types": "^13.6.2", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/auth-oauth-user": { + "version": "5.1.3", + "resolved": "https://registry.npmjs.org/@octokit/auth-oauth-user/-/auth-oauth-user-5.1.3.tgz", + "integrity": "sha512-zNPByPn9K7TC+OOHKGxU+MxrE9SZAN11UHYEFLsK2NRn3akJN2LHRl85q+Eypr3tuB2GrKx3rfj2phJdkYCvzw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-device": "^7.1.3", + "@octokit/oauth-methods": "^5.1.3", + "@octokit/request": "^9.2.1", + "@octokit/types": "^13.6.2", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 18" + } + }, "node_modules/@octokit/auth-token": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-5.1.2.tgz", @@ -737,6 +808,32 @@ "node": ">= 18" } }, + "node_modules/@octokit/oauth-authorization-url": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/@octokit/oauth-authorization-url/-/oauth-authorization-url-7.1.1.tgz", + "integrity": "sha512-ooXV8GBSabSWyhLUowlMIVd9l1s2nsOGQdlP2SQ4LnkEsGXzeCvbSbCPdZThXhEFzleGPwbapT0Sb+YhXRyjCA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">= 18" + } + }, + "node_modules/@octokit/oauth-methods": { + "version": "5.1.4", + "resolved": "https://registry.npmjs.org/@octokit/oauth-methods/-/oauth-methods-5.1.4.tgz", + "integrity": "sha512-Jc/ycnePClOvO1WL7tlC+TRxOFtyJBGuTDsL4dzXNiVZvzZdrPuNw7zHI3qJSUX2n6RLXE5L0SkFmYyNaVUFoQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@octokit/oauth-authorization-url": "^7.0.0", + "@octokit/request": "^9.2.1", + "@octokit/request-error": "^6.1.7", + "@octokit/types": "^13.6.2" + }, + "engines": { + "node": ">= 18" + } + }, "node_modules/@octokit/openapi-types": { "version": "23.0.1", "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-23.0.1.tgz", @@ -2866,6 +2963,16 @@ "node": ">=14.0.0" } }, + "node_modules/toad-cache": { + "version": "3.7.0", + "resolved": "https://registry.npmjs.org/toad-cache/-/toad-cache-3.7.0.tgz", + "integrity": "sha512-/m8M+2BJUpoJdgAHoG+baCwBT+tf2VraSfkBgl0Y00qIWt41DJ8R5B8nsEw0I58YwF5IZH6z24/2TobDKnqSWw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=12" + } + }, "node_modules/tslib": { "version": "2.8.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", @@ -2894,6 +3001,13 @@ "dev": true, "license": "MIT" }, + "node_modules/universal-github-app-jwt": { + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/universal-github-app-jwt/-/universal-github-app-jwt-2.2.2.tgz", + "integrity": "sha512-dcmbeSrOdTnsjGjUfAlqNDJrhxXizjAz94ija9Qw8YkZ1uu0d+GoZzyH+Jb9tIIqvGsadUfwg+22k5aDqqwzbw==", + "dev": true, + "license": "MIT" + }, "node_modules/universal-user-agent": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/universal-user-agent/-/universal-user-agent-7.0.2.tgz", diff --git a/package.json b/package.json index ed0a9c83..d8b18e76 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "@octokit/core": "^6.1.3" }, "devDependencies": { + "@octokit/auth-app": "^7.2.0", "@octokit/core": "^6.1.3", "@octokit/request-error": "^6.1.6", "@octokit/tsconfig": "^4.0.0", diff --git a/src/index.ts b/src/index.ts index 1b540160..895151ab 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,6 +30,11 @@ const createGroups = function ( maxConcurrent: 10, ...common, }); + groups.auth = new Bottleneck.Group({ + id: "octokit-auth", + maxConcurrent: 1, + ...common, + }); groups.search = new Bottleneck.Group({ id: "octokit-search", maxConcurrent: 1, diff --git a/src/types.ts b/src/types.ts index d27f485f..6989c24c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -39,6 +39,7 @@ export type ThrottlingOptions = export type Groups = { global?: Bottleneck.Group; + auth?: Bottleneck.Group; write?: Bottleneck.Group; search?: Bottleneck.Group; notifications?: Bottleneck.Group; diff --git a/src/wrap-request.ts b/src/wrap-request.ts index 6665c122..a7af4d1c 100644 --- a/src/wrap-request.ts +++ b/src/wrap-request.ts @@ -20,8 +20,10 @@ async function doRequest( ) => Promise>) & { retryCount: number }, options: Required, ) { - const isWrite = options.method !== "GET" && options.method !== "HEAD"; const { pathname } = new URL(options.url, "http://github.test"); + const isAuth = isAuthRequest(options.method, pathname); + const isWrite = + !isAuth && options.method !== "GET" && options.method !== "HEAD"; const isSearch = options.method === "GET" && pathname.startsWith("/search/"); const isGraphQL = pathname.startsWith("/graphql"); @@ -50,7 +52,7 @@ async function doRequest( await state.search.key(state.id).schedule(jobOptions, noop); } - const req = state.global + const req = (isAuth ? state.auth : state.global) .key(state.id) .schedule< OctokitResponse, @@ -73,3 +75,18 @@ async function doRequest( } return req; } + +function isAuthRequest(method: string, pathname: string) { + return ( + (method === "PATCH" && + // https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#create-a-scoped-access-token + /^\/applications\/[^/]+\/token\/scoped$/.test(pathname)) || + (method === "POST" && + // https://docs.github.com/en/rest/apps/oauth-applications?apiVersion=2022-11-28#reset-a-token + (/^\/applications\/[^/]+\/token$/.test(pathname) || + // https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#create-an-installation-access-token-for-an-app + /^\/app\/installations\/[^/]+\/access_tokens$/.test(pathname) || + // https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps + pathname === "/login/oauth/access_token")) + ); +} diff --git a/test/plugin.test.ts b/test/plugin.test.ts index b0355845..7914c90b 100644 --- a/test/plugin.test.ts +++ b/test/plugin.test.ts @@ -1,7 +1,12 @@ import { describe, it, expect } from "vitest"; import Bottleneck from "bottleneck"; +import { createAppAuth } from "@octokit/auth-app"; import { TestOctokit } from "./octokit.ts"; import { throttling } from "../src/index.ts"; +import { Octokit } from "@octokit/core"; +import * as crypto from "node:crypto"; +import { promisify } from "node:util"; +const generateKeyPair = promisify(crypto.generateKeyPair); describe("General", function () { it("Should be possible to disable the plugin", async function () { @@ -377,4 +382,85 @@ describe("GitHub API best practices", function () { octokit.__requestTimings[12] - octokit.__requestTimings[10], ).toBeLessThan(30); }); + + it("should not deadlock concurrent auth requests", async function () { + // instrument a fake fetch rather than using TestOctokit; this way + // @octokit/auth-app's request hook will actually run and we can + // track all requests (auth ones too, not just the top-level ones + // we make) + const requestLog: string[] = []; + const fakeFetch = async (url: string, init: any) => { + requestLog.push(`${init.method.toUpperCase()} ${url}`); + let data = {}; + if (init.method === "POST" && url.includes("/app/installations/")) { + data = { + token: "token", + expires_at: new Date(Date.now() + 60_000).toISOString(), + permissions: {}, + single_file: "", + }; + } + + return Promise.resolve( + new Response(JSON.stringify(data), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + }; + // jsonwebtoken needs a valid private key to sign the JWT, though the + // actual value doesn't matter since nothing will validate it + const privateKey = ( + await generateKeyPair("rsa", { + modulusLength: 4096, + publicKeyEncoding: { type: "spki", format: "pem" }, + privateKeyEncoding: { type: "pkcs8", format: "pem" }, + }) + ).privateKey; + + const octokit = new (Octokit.plugin(throttling))({ + authStrategy: createAppAuth, + auth: { + appId: 123, + privateKey, + installationId: 456, + }, + throttle: { + onSecondaryRateLimit: () => 0, + onRateLimit: () => 0, + }, + request: { + fetch: fakeFetch, + }, + }); + + const routes = [ + "/route01", + "/route02", + "/route03", + "/route04", + "/route05", + "/route06", + "/route07", + "/route08", + "/route09", + "/route10", + ]; + + await Promise.all(routes.map((route) => octokit.request(`GET ${route}`))); + + expect(requestLog).toStrictEqual([ + "POST https://api.github.com/app/installations/456/access_tokens", + "GET https://api.github.com/route01", + "GET https://api.github.com/route02", + "GET https://api.github.com/route03", + "GET https://api.github.com/route04", + "GET https://api.github.com/route05", + "GET https://api.github.com/route06", + "GET https://api.github.com/route07", + "GET https://api.github.com/route08", + "GET https://api.github.com/route09", + "GET https://api.github.com/route10", + ]); + }); });