diff --git a/src/github/app.ts b/src/github/app.ts index 3aea62ecc4..53420b44ee 100644 --- a/src/github/app.ts +++ b/src/github/app.ts @@ -483,33 +483,41 @@ export async function getRepositoryCollaboratorPermission( const parsed = parseRepoFullNameStrict(repoFullName); if (!parsed || !login) return null; const { owner, repo: name } = parsed; - const token = await createInstallationToken(env, installationId); - const response = await timeoutFetch( - `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/collaborators/${encodeURIComponent(login)}/permission`, - { - headers: githubHeaders({ token, json: true }), - githubRateLimitAdmission: true, - githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId), - }, - ); - if (response.status === 404) return null; - if (!response.ok) { - const body = await response.text(); - throw new Error( - `Failed to fetch GitHub collaborator permission (${response.status}): ${body.slice(0, 200)}`, + // Route through withInstallationTokenRetry so a stale cached installation token self-heals once + // (#9315) — same convention every sibling GitHub helper in this directory already uses (#6191 / #8892). + // Fail-closed callers (requireRepoWriteAccess / isPerTenantAdmin) must not deny a legitimate + // collaborator on a transient bad-credentials 401. + return await withInstallationTokenRetry(env, installationId, async (token) => { + const response = await timeoutFetch( + `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/collaborators/${encodeURIComponent(login)}/permission`, + { + headers: githubHeaders({ token, json: true }), + githubRateLimitAdmission: true, + githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId), + }, ); - } - const payload = (await response.json()) as { - permission?: GitHubRepositoryCollaboratorPermission; - role_name?: string; - }; - // #9152: `permission` is GitHub's coarse legacy field — only ever admin/write/read/none, so Maintain collapses - // into "write" and Triage into "read" — making `permission === "maintain"` dead at both call sites - // (isPerTenantAdmin, resolveRealRepoPermissionAssociation). `role_name` carries the actual granular tier - // (including "maintain"/"triage", or a custom org role) and is always present on this endpoint's real response; - // prefer it, falling back to `permission` only for a response that omits it (defensive — keeps the pre-#9152 - // behavior for any caller/fixture that only ever set `permission`). - return payload.role_name ?? payload.permission ?? null; + if (response.status === 404) return null; + if (!response.ok) { + const body = await response.text(); + // Attach `status` so withInstallationTokenRetry's isGitHubBadCredentialsError / permission-scope + // detectors can recognize a 401/403 even when the body text is empty or non-matching. + throw Object.assign( + new Error(`Failed to fetch GitHub collaborator permission (${response.status}): ${body.slice(0, 200)}`), + { status: response.status }, + ); + } + const payload = (await response.json()) as { + permission?: GitHubRepositoryCollaboratorPermission; + role_name?: string; + }; + // #9152: `permission` is GitHub's coarse legacy field — only ever admin/write/read/none, so Maintain collapses + // into "write" and Triage into "read" — making `permission === "maintain"` dead at both call sites + // (isPerTenantAdmin, resolveRealRepoPermissionAssociation). `role_name` carries the actual granular tier + // (including "maintain"/"triage", or a custom org role) and is always present on this endpoint's real response; + // prefer it, falling back to `permission` only for a response that omits it (defensive — keeps the pre-#9152 + // behavior for any caller/fixture that only ever set `permission`). + return payload.role_name ?? payload.permission ?? null; + }); } /** Account-age throttle (#2561, anti-abuse): `GET /users/{login}` for `created_at`, so a repo can flag a @@ -524,18 +532,28 @@ export async function getGithubUserCreatedAt( ): Promise { if (!login) return null; try { - const token = await createInstallationToken(env, installationId); - const response = await timeoutFetch( - `https://api.github.com/users/${encodeURIComponent(login)}`, - { - headers: githubHeaders({ token, json: true }), - githubRateLimitAdmission: true, - githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId), - }, - ); - if (!response.ok) return null; - const payload = (await response.json()) as { created_at?: unknown }; - return typeof payload.created_at === "string" ? payload.created_at : null; + // withInstallationTokenRetry (#9315): a stale-token 401 must re-mint once rather than fail-open to null + // and silently disable the account-age check. Non-token failures still fail open below. + return await withInstallationTokenRetry(env, installationId, async (token) => { + const response = await timeoutFetch( + `https://api.github.com/users/${encodeURIComponent(login)}`, + { + headers: githubHeaders({ token, json: true }), + githubRateLimitAdmission: true, + githubRateLimitAdmissionKey: githubRateLimitAdmissionKeyForInstallation(installationId), + }, + ); + if (response.status === 401 || response.status === 403) { + const body = await response.text(); + throw Object.assign( + new Error(`Failed to fetch GitHub user created_at (${response.status}): ${body.slice(0, 200)}`), + { status: response.status }, + ); + } + if (!response.ok) return null; + const payload = (await response.json()) as { created_at?: unknown }; + return typeof payload.created_at === "string" ? payload.created_at : null; + }); } catch { return null; } diff --git a/test/unit/github-app.test.ts b/test/unit/github-app.test.ts index ce0472e50e..7bd5df90b3 100644 --- a/test/unit/github-app.test.ts +++ b/test/unit/github-app.test.ts @@ -838,6 +838,32 @@ describe("GitHub check runs", () => { ).rejects.toThrow(/Failed to fetch GitHub collaborator permission/); }); + it("#9315: getRepositoryCollaboratorPermission self-heals a stale cached installation token (401 -> re-mint -> permission)", async () => { + // Same bug class as #6191/#8892: this read fed a stale cached token straight into the collaborator + // permission fetch with no retry, so a transient 401 failed closed for requireRepoWriteAccess / + // isPerTenantAdmin. It now routes through withInstallationTokenRetry. + let rejected = false; + setInstallationTokenStore({ + get: async () => ({ token: rejected ? "fresh-token" : "stale-token", expiresAtMs: Date.now() + 60 * 60_000 }), + set: async () => {}, + }); + const seenTokens: string[] = []; + vi.stubGlobal("fetch", async (_input: RequestInfo | URL, init?: RequestInit) => { + const token = (new Headers(init?.headers).get("authorization") ?? "").replace(/^Bearer\s+/i, ""); + seenTokens.push(token); + if (token === "stale-token") { + rejected = true; + return new Response(JSON.stringify({ message: "Bad credentials" }), { status: 401 }); + } + return Response.json({ permission: "write", role_name: "maintain" }); + }); + + await expect( + getRepositoryCollaboratorPermission(createTestEnv(), 123, "JSONbored/gittensory", "maintainer"), + ).resolves.toBe("maintain"); + expect(seenTokens).toEqual(["stale-token", "fresh-token"]); + }); + it("getGithubUserCreatedAt fetches the account creation date, and fails OPEN (null) on any error (#2561)", async () => { const privateKey = await generatePrivateKeyPem(); await expect(getGithubUserCreatedAt(createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey }), 123, "")).resolves.toBeNull(); @@ -871,6 +897,53 @@ describe("GitHub check runs", () => { ).resolves.toBeNull(); }); + it("#9315: getGithubUserCreatedAt self-heals a stale cached installation token (401 -> re-mint -> created_at)", async () => { + // Same bug class as #6191/#8892: a stale-token 401 previously fail-opened to null and silently disabled + // the account-age throttle. It now routes through withInstallationTokenRetry; non-token failures still + // fail open (covered by the existing #2561 test above). + let rejected = false; + setInstallationTokenStore({ + get: async () => ({ token: rejected ? "fresh-token" : "stale-token", expiresAtMs: Date.now() + 60 * 60_000 }), + set: async () => {}, + }); + const seenTokens: string[] = []; + vi.stubGlobal("fetch", async (_input: RequestInfo | URL, init?: RequestInit) => { + const token = (new Headers(init?.headers).get("authorization") ?? "").replace(/^Bearer\s+/i, ""); + seenTokens.push(token); + if (token === "stale-token") { + rejected = true; + return new Response(JSON.stringify({ message: "Bad credentials" }), { status: 401 }); + } + return Response.json({ login: "newbie", created_at: "2026-06-01T00:00:00Z" }); + }); + + await expect(getGithubUserCreatedAt(createTestEnv(), 123, "newbie")).resolves.toBe("2026-06-01T00:00:00Z"); + expect(seenTokens).toEqual(["stale-token", "fresh-token"]); + }); + + it("#9315: getGithubUserCreatedAt still fail-opens when a 401 persists after the retry", async () => { + setInstallationTokenStore({ + get: async () => ({ token: "always-stale", expiresAtMs: Date.now() + 60 * 60_000 }), + set: async () => {}, + }); + vi.stubGlobal("fetch", async () => new Response(JSON.stringify({ message: "Bad credentials" }), { status: 401 })); + await expect(getGithubUserCreatedAt(createTestEnv(), 123, "newbie")).resolves.toBeNull(); + }); + + it("#9315: getGithubUserCreatedAt still fail-opens on a non-retryable 403", async () => { + // 403 without the installation permission-scope message is not self-healable — throw propagates out of + // withInstallationTokenRetry and the outer catch returns null (same fail-open contract as before). + const privateKey = await generatePrivateKeyPem(); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return new Response(JSON.stringify({ message: "forbidden" }), { status: 403 }); + }); + await expect( + getGithubUserCreatedAt(createTestEnv({ GITHUB_APP_PRIVATE_KEY: privateKey }), 123, "newbie"), + ).resolves.toBeNull(); + }); + it("cancelInFlightWorkflowRunsForHeadSha lists in_progress + queued runs at a head SHA and cancels each (#2462)", async () => { const privateKey = await generatePrivateKeyPem(); const cancelledIds: number[] = []; diff --git a/test/unit/selfhost-pg-retention.test.ts b/test/unit/selfhost-pg-retention.test.ts index 9dca5db0ee..6d74653504 100644 --- a/test/unit/selfhost-pg-retention.test.ts +++ b/test/unit/selfhost-pg-retention.test.ts @@ -19,7 +19,9 @@ interface MockPgPool { /** A minimal fake Postgres that also acts as a regression guard: if untranslated SQLite SQL (the `rowid` * pseudo-column) ever reaches it again, it throws the exact error a real Postgres raised in the live - * self-host incident (dead-lettered job `_selfhost_jobs.id = 61132`, `prune-retention`, 5 attempts). */ + * self-host incident (dead-lettered job `_selfhost_jobs.id = 61132`, `prune-retention`, 5 attempts). + * #9083: production deletes by a real PK (`id` / `delivery_id`) when mapped; the mock must accept that + * shape (and the rowid→ctid fallback for unmapped tables), not only the pre-#9083 `ctid`-only form. */ function makeRetentionPgPool(remaining: Record = {}): MockPgPool { const calls: string[] = []; const fn = vi.fn().mockImplementation(async (sql: unknown) => {