diff --git a/github/README.md b/github/README.md index 58e71c5a..53ee9736 100644 --- a/github/README.md +++ b/github/README.md @@ -76,6 +76,7 @@ mint whose permission set exceeds the installation's grant. So the App's | Pull requests | Read & write | open/update PRs | | Issues | Read & write | open/comment issues | | Checks | Read | `GET_CHECK_RUN`, the PR panel's Checks tab | +| Commit statuses | Read | commit statuses and GraphQL's combined CI check rollup | | Deployments | Read | `GET_PREVIEW_DEPLOYMENT` — the only place a VTEX FastStore WebOps preview URL is published | Adding a permission to an existing App is **not** self-applying: GitHub marks diff --git a/github/server/lib/repo-grant.test.ts b/github/server/lib/repo-grant.test.ts index 8cd14e66..54bf1823 100644 --- a/github/server/lib/repo-grant.test.ts +++ b/github/server/lib/repo-grant.test.ts @@ -202,6 +202,13 @@ describe("buildUpgradeLadder", () => { test("widens first, then sheds the newest optional one at a time", () => { expect(buildUpgradeLadder({ contents: "write", metadata: "read" })).toEqual( [ + { + contents: "write", + metadata: "read", + checks: "read", + deployments: "read", + statuses: "read", + }, { contents: "write", metadata: "read", @@ -224,6 +231,13 @@ describe("buildUpgradeLadder", () => { checks: "read", }), ).toEqual([ + { + contents: "write", + metadata: "read", + checks: "read", + deployments: "read", + statuses: "read", + }, { contents: "write", metadata: "read", @@ -240,6 +254,7 @@ describe("buildUpgradeLadder", () => { metadata: "read", checks: "read", deployments: "read", + statuses: "read", }; expect(buildUpgradeLadder(full)).toEqual([full]); }); @@ -255,6 +270,13 @@ describe("buildUpgradeLadder", () => { checks: "write", }), ).toEqual([ + { + contents: "write", + metadata: "read", + checks: "read", + deployments: "read", + statuses: "read", + }, { contents: "write", metadata: "read", @@ -430,13 +452,13 @@ describe("refreshRepoGrant — minting", () => { const body = JSON.parse((init as { body?: string }).body ?? "{}"); expect(body.repository_ids).toEqual([999]); // Refresh widens a legacy grant into every optional read the PR panel - // needs — checks:read (CI check runs) and deployments:read (the preview - // URL) — so it self-heals with no re-import and no re-install. + // needs, including commit statuses, without re-import or re-install. expect(body.permissions).toEqual({ contents: "write", metadata: "read", checks: "read", deployments: "read", + statuses: "read", }); return json( { @@ -477,14 +499,18 @@ describe("refreshRepoGrant — minting", () => { test("no optional upgrade available (422) falls back to the grant's permissions without revoking", async () => { const kv = fakeKV(); const store = getRepoGrantStore(kv); - const { creds, meta } = await seedGrant(store); // no checks, no deployments + const { creds, meta } = await seedGrant(store); // no optional reads const asked: Array> = []; setFetch(async (input, init) => { const url = urlOf(input); if (/\/app\/installations\/42\/access_tokens/.test(url)) { const body = JSON.parse((init as { body?: string }).body ?? "{}"); asked.push(body.permissions); - if (asked.length < 3) { + if ( + Object.keys(body.permissions).some( + (key) => key !== "contents" && key !== "metadata", + ) + ) { return json({ message: "permissions exceed grant" }, 422); } return json( @@ -515,6 +541,13 @@ describe("refreshRepoGrant — minting", () => { // Newest optional shed first, then the next — the last rung is exactly what // the grant was issued with, so the connection always keeps working. expect(asked).toEqual([ + { + contents: "write", + metadata: "read", + checks: "read", + deployments: "read", + statuses: "read", + }, { contents: "write", metadata: "read", @@ -528,7 +561,7 @@ describe("refreshRepoGrant — minting", () => { expect(kv.store.has(`grant:${meta.grantId}`)).toBe(true); }); - test("an installation granting checks but not deployments keeps checks", async () => { + test("an installation granting checks but not deployments or statuses keeps checks", async () => { // The regression this ladder exists for: adding deployments to the widened // set without shedding it one at a time would 422 the whole mint for every // grant that already had checks, and (before the ladder) go straight to @@ -544,7 +577,7 @@ describe("refreshRepoGrant — minting", () => { if (/\/app\/installations\/42\/access_tokens/.test(url)) { const body = JSON.parse((init as { body?: string }).body ?? "{}"); asked.push(body.permissions); - if (body.permissions.deployments) { + if (body.permissions.statuses || body.permissions.deployments) { return json({ message: "permissions exceed grant" }, 422); } return json( @@ -573,6 +606,13 @@ describe("refreshRepoGrant — minting", () => { if (!r.ok) throw new Error("expected ok"); expect(r.success.access_token).toBe("ghs_with_checks"); expect(asked).toEqual([ + { + contents: "write", + metadata: "read", + checks: "read", + deployments: "read", + statuses: "read", + }, { contents: "write", metadata: "read", @@ -584,6 +624,52 @@ describe("refreshRepoGrant — minting", () => { expect(kv.store.has(`grant:${meta.grantId}`)).toBe(true); }); + test("a refused statuses upgrade retains approved checks and deployments", async () => { + const kv = fakeKV(); + const store = getRepoGrantStore(kv); + const permissions = { + contents: "write", + metadata: "read", + checks: "read", + deployments: "read", + }; + const { creds, meta } = await seedGrant(store, { permissions }); + const asked: Array> = []; + setFetch(async (_input, init) => { + const body = JSON.parse((init as { body?: string }).body ?? "{}"); + expect(body.repository_ids).toEqual([999]); + asked.push(body.permissions); + if (body.permissions.statuses) { + return json({ message: "permissions exceed grant" }, 422); + } + return json( + { + token: "ghs_existing_reads", + expires_at: "2026-06-10T01:00:00.000Z", + permissions: body.permissions, + }, + 201, + ); + }); + + const result = await refreshRepoGrant({ + now: TEST_NOW, + store, + grantType: "refresh_token", + refreshToken: creds.refreshToken, + clientId: "Iv1.abc", + expectedClientId: "Iv1.abc", + jwt: "fake.jwt", + }); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected ok"); + expect(result.success.access_token).toBe("ghs_existing_reads"); + expect(result.success.refresh_token).toBe(creds.refreshToken); + expect(asked).toEqual([{ ...permissions, statuses: "read" }, permissions]); + expect(kv.store.has(`grant:${meta.grantId}`)).toBe(true); + }); + test("a non-422 mint failure stops the ladder after one attempt", async () => { // A 5xx says nothing about the permission set, so retrying narrower would // just multiply GitHub calls during an outage — and must stay transient diff --git a/github/server/lib/repo-grant.ts b/github/server/lib/repo-grant.ts index cd7c44bd..f420e170 100644 --- a/github/server/lib/repo-grant.ts +++ b/github/server/lib/repo-grant.ts @@ -200,7 +200,8 @@ const samePermissions = ( * The permission maps a refresh tries, in order, when re-minting a grant. * * Rung 0 widens the grant into every {@link OPTIONAL_READ_UPGRADES} permission - * — `checks:read` (CI check runs) and `deployments:read` (a PR's preview URL, + * — `statuses:read` (commit statuses), `checks:read` (CI check runs), and + * `deployments:read` (a PR's preview URL, * the ONLY place a VTEX FastStore WebOps deploy publishes it). That is what * lets a grant issued before a permission joined the allowlist pick it up on * its next refresh, riding the ~1h token cycle: no re-import, no re-install, diff --git a/github/server/lib/repo-token.test.ts b/github/server/lib/repo-token.test.ts index a91e673b..ed44a7fa 100644 --- a/github/server/lib/repo-token.test.ts +++ b/github/server/lib/repo-token.test.ts @@ -86,13 +86,21 @@ describe("capPermissions", () => { expect(capPermissions({ metadata: "write" })).toEqual({ metadata: "read" }); }); - test.each(["checks", "deployments"])( + test("allows statuses:read for commit statuses in the combined CI rollup", () => { + expect(capPermissions({ statuses: "read" })).toEqual({ + statuses: "read", + metadata: "read", + }); + }); + + test.each(["checks", "deployments", "statuses"])( "caps the read-only permission %s down to read", (perm) => { // checks:write would let a token post a green check run — and Studio // gates PR merges on check status. deployments:write would let it write // the environment_url the PR panel renders as a preview link. Neither is - // ever needed, so neither is ever minted. + // ever needed. Status writes can also forge a successful CI signal, so + // these permissions are always minted at read. expect(capPermissions({ [perm]: "write" })).toEqual({ [perm]: "read", metadata: "read", diff --git a/github/server/lib/repo-token.ts b/github/server/lib/repo-token.ts index 2390d19b..d4800377 100644 --- a/github/server/lib/repo-token.ts +++ b/github/server/lib/repo-token.ts @@ -48,7 +48,7 @@ export class RepoTokenError extends Error { /** * Positive allowlist of permissions we are willing to mint — strictly - * repo-content / PR / issue level plus two read-only CI/deploy signals. + * repo-content / PR / issue level plus read-only CI/deploy signals. * Anything outside this list is hard-rejected, which by construction also * rejects every escalation vector the spec bans (administration, members, * organization_*, secrets, actions, environments, ...). @@ -57,9 +57,11 @@ export class RepoTokenError extends Error { * (`GET /commits/{sha}/check-runs`); `deployments` so it can read a PR's preview * URL from the Deployments API (`GET /repos/{o}/{r}/deployments` + `/statuses`, * via GET_PREVIEW_DEPLOYMENT) — the ONLY place a VTEX FastStore WebOps preview - * is published (not a commit-status `target_url`, not a bot comment). Without - * each, the minted installation token gets `403 Resource not accessible by - * integration` on that endpoint. + * is published (not a commit-status `target_url`, not a bot comment). + * `statuses` is needed to read commit statuses, including status contexts in + * GitHub's combined GraphQL check rollup. Without each permission, the minted + * installation token gets `403 Resource not accessible by integration` on + * that endpoint. */ export const ALLOWED_PERMISSIONS = new Set([ "contents", @@ -68,16 +70,18 @@ export const ALLOWED_PERMISSIONS = new Set([ "issues", "checks", "deployments", + "statuses", ]); /** * Permissions we only ever mint at `read`, whatever the caller asks for. These - * are observability signals, and write on either is an escalation with teeth: + * are observability signals, and write access can forge a merge signal: * `checks:write` lets a token POST a green check run — and Studio gates PR * merges on check status, so that is a forged ship signal — while * `deployments:write` lets it create deployments and deployment statuses, * including the `environment_url` that the PR panel then renders as a preview - * link. Capped rather than rejected, matching this function's contract (and how + * link. `statuses:write` can post a successful commit status. Capped rather + * than rejected, matching this function's contract (and how * `metadata` has always been handled): a stored grant is re-capped on every * refresh, so a throw here would turn a legacy over-broad grant into a hard * refresh failure instead of quietly narrowing it. @@ -86,17 +90,22 @@ export const READ_ONLY_PERMISSIONS = new Set([ "metadata", "checks", "deployments", + "statuses", ]); /** * The optional read permissions a refresh tries to widen an existing grant - * into, MOST-DROPPABLE FIRST. `deployments` is the newest, so an installation - * that has approved `checks` but not yet `deployments` sheds only the latter. + * into, MOST-DROPPABLE FIRST. `statuses` is the newest, so installations that + * have approved checks and deployments retain them when statuses is refused. * Every entry must also be in {@link ALLOWED_PERMISSIONS} (asserted in the unit * test): this list marks which permissions are droppable, it does not add new * ones. See `buildUpgradeLadder` in repo-grant.ts for how it is applied. */ -export const OPTIONAL_READ_UPGRADES = ["deployments", "checks"] as const; +export const OPTIONAL_READ_UPGRADES = [ + "statuses", + "deployments", + "checks", +] as const; /** GitHub permission levels we allow. `admin` is never granted. */ const ALLOWED_VALUES = new Set(["read", "write"]); @@ -195,7 +204,9 @@ async function findCallerInstallation( while (true) { const res = await fetch( `${GITHUB_API}/user/installations?per_page=${PER_PAGE}&page=${page}`, - { headers: githubHeaders(callerToken) }, + { + headers: githubHeaders(callerToken), + }, ); if (!res.ok) { if (isTransientGitHubResponse(res)) { diff --git a/github/server/tools/mint-repo-token.ts b/github/server/tools/mint-repo-token.ts index 61da5cdd..27f26c67 100644 --- a/github/server/tools/mint-repo-token.ts +++ b/github/server/tools/mint-repo-token.ts @@ -35,7 +35,7 @@ export function createMintRepoTokenTool() { "caller must already be entitled to the installation and repository — the " + "tool verifies this against the caller's own GitHub context before minting. " + "The token grants only repo-content / pull-request / issue access plus " + - "read-only CI checks and deployments. Also " + + "read-only CI checks, commit statuses, and deployments. Also " + "returns a durable refresh token (refreshToken) plus tokenEndpoint and " + "clientId: POST grant_type=refresh_token to tokenEndpoint to mint a fresh " + "token later without the caller's GitHub login.",