Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions github/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
98 changes: 92 additions & 6 deletions github/server/lib/repo-grant.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -224,6 +231,13 @@ describe("buildUpgradeLadder", () => {
checks: "read",
}),
).toEqual([
{
contents: "write",
metadata: "read",
checks: "read",
deployments: "read",
statuses: "read",
},
{
contents: "write",
metadata: "read",
Expand All @@ -240,6 +254,7 @@ describe("buildUpgradeLadder", () => {
metadata: "read",
checks: "read",
deployments: "read",
statuses: "read",
};
expect(buildUpgradeLadder(full)).toEqual([full]);
});
Expand All @@ -255,6 +270,13 @@ describe("buildUpgradeLadder", () => {
checks: "write",
}),
).toEqual([
{
contents: "write",
metadata: "read",
checks: "read",
deployments: "read",
statuses: "read",
},
{
contents: "write",
metadata: "read",
Expand Down Expand Up @@ -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(
{
Expand Down Expand Up @@ -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<Record<string, string>> = [];
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(
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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",
Expand All @@ -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<Record<string, string>> = [];
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
Expand Down
3 changes: 2 additions & 1 deletion github/server/lib/repo-grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 10 additions & 2 deletions github/server/lib/repo-token.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
31 changes: 21 additions & 10 deletions github/server/lib/repo-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, ...).
Expand All @@ -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",
Expand All @@ -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.
Expand All @@ -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"]);
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion github/server/tools/mint-repo-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
Loading