diff --git a/COVERAGE.md b/COVERAGE.md index 81434b9e..262d269b 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -35,8 +35,8 @@ The tenet: this action can control everything about a repository and nothing abo | [GitHub Pages](https://docs.github.com/en/rest/pages/pages) (build_type, source, cname, https_enforced and any PUT field GitHub adds; pages: null disables the site) | `pages` | POST /repos/{owner}/{repo}/pages (create accepts only build_type/source) then PUT for the rest; existing sites get straight PUT passthrough; pages: null issues DELETE, mirroring branches' protection: null. In a mode: merge fold, pages: null over a lower layer's pages declaration removes the key from the merged document instead (the layering guide owns that rule); a multi-repo defaults-file never merges into a target's file. The Enterprise Cloud site-visibility boolean `public` (public vs repo-members-only) rides the PUT passthrough too - the dotcom docs omit it but the GHEC flavor documents it, and the GET echoes it in both, so declaring it applies and check-verifies today. Snapshot reads the live site back on the declared keys; a repository without Pages is omitted rather than declared null. | | [Code scanning default setup](https://docs.github.com/en/rest/code-scanning/code-scanning) | `code_scanning_default_setup` | GET/PATCH /repos/{owner}/{repo}/code-scanning/default-setup, PATCH body verbatim (state, query_suite, languages, runner_type, runner_label, threat_model). A 202 answer means GitHub rolls the change out in a configuration run, which the log names. Check compares declared keys only, languages as a set. Needs GitHub Advanced Security on private repositories; a 403 can mean that (or an archived repository) rather than a missing permission. Snapshot reads the GET body back on the declared keys; a null the slice does not take (a not-configured setup's runner_type) is omitted. | | [Code quality setup](https://docs.github.com/en/rest/code-quality/code-quality) | `code_quality_setup` | GET/PATCH /repos/{owner}/{repo}/code-quality/setup, PATCH body verbatim (state, languages, runner_type, runner_label, ai_findings_option), the near field-for-field mirror of code_scanning_default_setup. A 202 answer means GitHub rolls the change out in a configuration run, which the log names; a 409 means a configuration run is already in progress (re-run the workflow after it finishes); a 403 can mean code quality is unavailable on the repository or the repository is archived, rather than a missing permission, and a 422 (the change cannot be applied) carries GitHub's message verbatim. Check compares declared keys only, languages as a set. ASSUMPTION: the GET is graded as a plain read under the Administration permission, mirroring code scanning - the fine-grained grading of these endpoints is not verified against production. The sibling /code-quality/findings endpoints are read-only and stay out of scope. Snapshot reads the GET body back on the declared keys; a null the slice does not take (a not-configured setup's runner_type) is omitted. | -| [Collaborators](https://docs.github.com/en/rest/collaborators/collaborators) (direct) | `collaborators` | PUT/DELETE /repos/{owner}/{repo}/collaborators/{username} (affiliation=direct); pending invitations reconciled alongside via GET /repos/{owner}/{repo}/invitations and PATCH/DELETE /repos/{owner}/{repo}/invitations/{invitation_id}: a declared user's matching pending invitation converges, a stale permission is PATCHed in place (a declared custom role cannot be verified against an invitation - the standard-roles enum - so the invitation is kept with a note), an expired invitation is cancelled and re-sent, and undeclared pending invitations follow the section's one undeclared policy (email invitations, which no username can declare, are noted and left untouched); vocabulary mapping push<->write, pull<->read for check mode; custom org role names pass through; undeclared direct collaborators REMOVED by default (kept as notes under the wrapped `_undeclared: keep` form), owner never touched; new users get invitations. | -| [Team repository permissions](https://docs.github.com/en/rest/teams/teams) (org repos) | `teams` | PUT /orgs/{org}/teams/{slug}/repos/{owner}/{repo}; probes GET /orgs/{owner} and no-ops with a note on personal accounts (404 only; 403/5xx still fail). Both modes probe each team's access with the v3.repository media type to read role_name, and the grant is issued only when the team lacks access or its live role diverges. | +| [Collaborators](https://docs.github.com/en/rest/collaborators/collaborators) (direct) | `collaborators` | PUT/DELETE /repos/{owner}/{repo}/collaborators/{username} (affiliation=direct); pending invitations reconciled alongside via GET /repos/{owner}/{repo}/invitations and PATCH/DELETE /repos/{owner}/{repo}/invitations/{invitation_id}: a declared user's matching pending invitation converges, a stale permission is PATCHed in place (a declared custom role cannot be verified against an invitation - the standard-roles enum - so the invitation is kept with a note), an expired invitation is cancelled and re-sent, and undeclared pending invitations follow the section's one undeclared policy (email invitations, which no username can declare, are noted and left untouched); vocabulary mapping push<->write, pull<->read for check mode; custom org role names pass through; undeclared direct collaborators REMOVED by default (kept as notes under the wrapped `_undeclared: keep` form), owner never touched; new users get invitations. Snapshot reads the direct collaborators and the pending username invitations back as entries under the delete default, each role through the inverse mapping (write -> push, read -> pull, custom role names verbatim); the repository owner, email invitations, and expired invitations are noted, not declared, and a live role no declaration plans as (a custom role named "push" or "pull") fails the collaborators section; the snapshot is written as partial without it. | +| [Team repository permissions](https://docs.github.com/en/rest/teams/teams) (org repos) | `teams` | PUT /orgs/{org}/teams/{slug}/repos/{owner}/{repo}; probes GET /orgs/{owner} and no-ops with a note on personal accounts (404 only; 403/5xx still fail). Both modes probe each team's access with the v3.repository media type to read role_name, and the grant is issued only when the team lacks access or its live role diverges. Snapshot lists the teams with access (GET /repos/{owner}/{repo}/teams) and reads each team's role through the same probe, so a custom role reads back by name; noted, not declared: access granted at the organization or enterprise level, a probe 404 (no access, or a fine-grained token missing the grant; the note names both), a role the probe does not report, and a role no declaration plans as (a custom role named "push" or "pull"). A personal account snapshots nothing. | | [Milestones](https://docs.github.com/en/rest/issues/milestones) | `milestones` | POST/PATCH/DELETE /repos/{owner}/{repo}/milestones, matched by title, declared-keys-only (description/state untouched unless declared); undeclared milestones kept by default and surfaced as notes, because deleting a milestone DETACHES it from every issue carrying it - the wrapped `_undeclared: delete` form opts into exactly that. Snapshot reads every milestone, open and closed, back as entries under the keep default. | | [Interaction limits](https://docs.github.com/en/rest/interactions/repos) | `interaction_limits` | GET/PUT/DELETE /repos/{owner}/{repo}/interaction-limits, plus the two routed keys below. Limits self-expire (expiry tops out at six_months), so apply re-arms the declared limit on EVERY run and check mode reports drift once it lapses - schedule apply more often than the chosen expiry to keep it armed. The declared expiry is write-only (GitHub reads back only the computed expires_at), so check verifies the limit value, not the duration. `interaction_limits: null` clears a live repo-level limit - the base limit only, never the cap or bypass list below. In a mode: merge fold, interaction_limits: null over a lower layer's declaration removes the key from the merged document instead, mirroring pages: null. A 409 means an organization- or user-level limit overrides the repository's; writes surface that as a note, not a failure, while check mode still reports a mismatched declaration as drift - the org is the place to change it. `pull_request_creation_cap` routes to GET/PATCH /repos/{owner}/{repo}/interaction-limits/pulls/creation-cap: persistent desired state with no self-expiry (enabled plus max_open_pull_requests 1-1000, read back verbatim), so check diffs it exactly and apply PATCHes only on divergence - no re-arm. Where the cap is unavailable the endpoints answer 405, which apply surfaces as a note (like the 409) and check as honest drift. `pull_request_creation_bypass` routes to GET/PUT/DELETE /repos/{owner}/{repo}/interaction-limits/pulls/bypass-list and reconciles: apply DELETEs only the undeclared logins and then PUTs only the missing ones ({users: [logins]}, compared case-insensitively; removals go first because the list holds at most 100 users, which also caps the declaration), never a wholesale replace; a declared empty list removes everyone. Snapshot reads back the repository-level limit (an inherited one is noted, not declared, and the expiry duration is not readable), the cap when the feature exists and is enabled, and the bypass list when it has members. | | [Actions variables](https://docs.github.com/en/rest/actions/variables) | `actions_variables` | GET/POST /repos/{owner}/{repo}/actions/variables and PATCH/DELETE /repos/{owner}/{repo}/actions/variables/{name}: plain-text repository variables, upserted by name. Names are case-insensitive (GitHub stores them uppercased regardless of how they are entered), so matching and duplicate rejection compare uppercased names. Values read back in full, which is what makes exact check-mode diffing possible; undeclared variables are DELETED by default, kept as notes under the wrapped `_undeclared: keep` form. The list endpoint caps per_page at 30 (not the usual 100), which the page loop honors so a large inventory is never truncated. Secrets are write-only material and deliberately NOT this section: they live in the `actions_secrets` section above. Snapshot reads the variables back as name and value entries under the delete default. | diff --git a/docs/operate/snapshot.md b/docs/operate/snapshot.md index c586afd5..0a0ae914 100644 --- a/docs/operate/snapshot.md +++ b/docs/operate/snapshot.md @@ -121,6 +121,10 @@ A snapshot is written so that applying it changes nothing and checking it reads | `webhooks[].config.secret` | GitHub echoes `********` | A `$WEBHOOK_SECRET_N` reference and a note per hook | | `interaction_limits.expiry` | GitHub reports only the computed `expires_at` | No `expiry` key; apply re-arms the limit with GitHub's default unless you declare one | | `check_suite_preferences` | GitHub exposes no read endpoint | Nothing; the header says so | +| `collaborators`: the repository owner, email invitations | The owner's access is implicit, and an email invitation has no username to declare | No entry and a note each; apply leaves both alone | +| `collaborators`: expired invitations | A declared one would be cancelled and re-sent; an undeclared one is cancelled under the delete default the file carries | No entry and a note per invitation; add the entry to re-invite | +| A custom role named `push` or `pull` | In a settings file those words mean the `write` and `read` roles, so no declaration plans as the live role | `collaborators` fails and the file is written without it; `teams` omits the team with a note | +| `teams`: access granted at the organization or enterprise level | Declaring it would grant direct repository access on top | No entry and a note; apply leaves it alone | | Sections the snapshot does not read back yet | Not implemented for that section | Nothing; the header lists each, and the run notices them | A section that exists but holds nothing live (no milestones, no Pages site) is omitted with a header line, never written as an empty list: an empty list under `_undeclared: delete` would delete on apply. diff --git a/docs/reference/sections.md b/docs/reference/sections.md index 49bf086e..55026b88 100644 --- a/docs/reference/sections.md +++ b/docs/reference/sections.md @@ -26,7 +26,7 @@ Every top-level key a settings file can declare, one row per section: the endpoi | `code_scanning_default_setup` | code scanning default setup | Administration or Code scanning alerts: write | untouched | `state`, `query_suite`, `languages`; needs Advanced Security on private repositories | | `code_quality_setup` | code-quality setup | Administration: write | untouched | `state`, `languages`, runner and AI-findings options; a 202 means GitHub rolls the change out in a configuration run; needs code quality available on the repository | | `collaborators` | direct collaborators + pending invitations | Administration: write | deleted (settable) | invitations for new users, pending ones reconciled (stale permission updated, expired re-sent, undeclared cancelled); the repository owner is never touched | -| `teams` | org team repo permissions | Members: read (org permission) + Administration: write | untouched | org repos only, skipped with a notice on personal accounts | +| `teams` | org team repo permissions + repo team list | Members: read (org permission) + Administration: write | untouched | org repos only, skipped with a notice on personal accounts | | `milestones` | milestones | Issues: write | kept (settable) | upsert by title; deleting a milestone detaches it from every issue carrying it, which is why keep is the default | | `interaction_limits` | interaction-limits + pulls creation-cap/bypass-list | Administration: write | untouched | re-arms the self-expiring limit every apply run; `null` clears it (base limit only); a 409 (org/user-level limit overrides) becomes a note; the PR creation cap is persistent (PATCHed only on divergence, 405 where unavailable) and its bypass logins reconcile add/remove | | `actions_variables` | Actions variables CRUD | Variables: write | deleted (settable) | plain-text variables upserted by name (case-insensitive); values read back in full, so check mode diffs them exactly | diff --git a/src/sections/collaborators/collaborators.docs.yml b/src/sections/collaborators/collaborators.docs.yml index fb065e50..89e0914f 100644 --- a/src/sections/collaborators/collaborators.docs.yml +++ b/src/sections/collaborators/collaborators.docs.yml @@ -18,6 +18,11 @@ coverage: custom org role names pass through; undeclared direct collaborators REMOVED by default (kept as notes under the wrapped `_undeclared: keep` form), owner never touched; new users get invitations. + Snapshot reads the direct collaborators and the pending username invitations back as entries + under the delete default, each role through the inverse mapping (write -> push, read -> pull, + custom role names verbatim); the repository owner, email invitations, and expired invitations + are noted, not declared, and a live role no declaration plans as (a custom role named "push" + or "pull") fails the collaborators section; the snapshot is written as partial without it. schema: SettingsFile.collaborators: >- Direct collaborators, with pending invitations reconciled alongside; undeclared ones are REMOVED diff --git a/src/sections/collaborators/collaborators.test.ts b/src/sections/collaborators/collaborators.test.ts index 2ed5072d..d10c8245 100644 --- a/src/sections/collaborators/collaborators.test.ts +++ b/src/sections/collaborators/collaborators.test.ts @@ -13,6 +13,8 @@ const LIST = "GET /repos/o/r/collaborators?affiliation=direct&per_page=100&page= const INVITATIONS = "GET /repos/o/r/invitations?per_page=100&page=1"; const plan = (api: MockApi, desired: Parameters[1]) => collaboratorsSection.plan(planContext(collaboratorsSection, api, REPO), desired); +const snapshot = (api: MockApi) => + collaboratorsSection.snapshot(planContext(collaboratorsSection, api, REPO)); const NO_SECRETS = { resolveSecret: (): string => { throw new Error("no secrets"); @@ -326,4 +328,88 @@ describe("collaborators", () => { ctx.read.list.probeAbsent; expect(typeof ctx.read.listInvitations.listAll).toBe("function"); }); + + describe("snapshot", () => { + test("reads collaborators then pending invitations back under the delete default; the owner, an expired invitation, and an email invitation are noted, not declared", async () => { + const api = new MockApi({ + [LIST]: { + data: [ + { login: "O", role_name: "admin" }, + { login: "alice", role_name: "write" }, + { login: "bob", role_name: "read" }, + { login: "carol", role_name: "security-team" }, + ], + }, + [INVITATIONS]: { + data: [ + { id: 7, invitee: { login: "dave" }, permissions: "write", expired: false }, + { id: 8, invitee: { login: "erin" }, permissions: "read", expired: true }, + { id: 9, invitee: null, permissions: "read", expired: false }, + ], + }, + }); + expect(await snapshot(api)).toEqual({ + value: { + _undeclared: "delete", + entries: [ + { username: "alice", permission: "push" }, + { username: "bob", permission: "pull" }, + { username: "carol", permission: "security-team" }, + { username: "dave", permission: "push" }, + ], + }, + notes: [ + "collaborators[O]: the repository owner's access is implicit and never managed, so it is not declared", + "collaborators[erin]: the pending invitation has expired, so it is not declared; apply cancels it - add the entry to re-invite them", + "invitation 9 was sent by email, so no username can declare it; not declared, and apply leaves it untouched", + ], + }); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([LIST, INVITATIONS]); + }); + + test("only the owner, an expired invitation, and an email invitation snapshot as nothing to declare; the expired note says apply leaves it", async () => { + const api = new MockApi({ + [LIST]: { data: [{ login: "o", role_name: "admin" }] }, + [INVITATIONS]: { + data: [ + { id: 8, invitee: { login: "erin" }, permissions: "read", expired: true }, + { id: 9, invitee: null, permissions: "read", expired: false }, + ], + }, + }); + expect(await snapshot(api)).toEqual({ + value: undefined, + notes: [ + "collaborators[o]: the repository owner's access is implicit and never managed, so it is not declared", + "collaborators[erin]: the pending invitation has expired, so it is not declared; nothing else is declared, so the section is omitted and apply leaves it - declare the entry to re-invite them", + "invitation 9 was sent by email, so no username can declare it; not declared, and apply leaves it untouched", + ], + }); + }); + + test("a collaborator or invitation without a role, or with a role no declaration plans as, fails loudly instead of declaring a guess", async () => { + const roleless = new MockApi({ + [LIST]: { data: [{ login: "alice" }] }, + [INVITATIONS]: { data: [] }, + }); + await expect(snapshot(roleless)).rejects.toThrow( + "collaborators[alice]: GitHub reported no role_name for this collaborator, so their permission cannot be read back", + ); + const blankInvitation = new MockApi({ + [LIST]: { data: [] }, + [INVITATIONS]: { data: [{ id: 7, invitee: { login: "dave" }, expired: false }] }, + }); + await expect(snapshot(blankInvitation)).rejects.toThrow( + "collaborators[dave]: GitHub reported no permissions on the pending invitation, so it cannot be read back", + ); + // A custom role named "push": declaring "push" plans the write role, so the entry can never converge. + const collision = new MockApi({ + [LIST]: { data: [{ login: "frank", role_name: "push" }] }, + [INVITATIONS]: { data: [] }, + }); + await expect(snapshot(collision)).rejects.toThrow( + 'collaborators[frank]: the live role "push" has no declaration that plans as itself ("push" in a settings file means the "write" role), so it cannot be read back', + ); + }); + }); }); diff --git a/src/sections/collaborators/index.ts b/src/sections/collaborators/index.ts index 55c03703..fe8cce0f 100644 --- a/src/sections/collaborators/index.ts +++ b/src/sections/collaborators/index.ts @@ -6,16 +6,23 @@ import { parseLive } from "../contract/live.js"; import { defaultUndeclaredPolicy, loosen, + type SectionMeta, type SectionModule, undeclaredDrift, undeclaredNote, undeclaredPolicy, } from "../contract/module.js"; import type { SectionPermission } from "../contract/permissions.js"; -import type { PlannedOp, SectionPlan } from "../contract/plan.js"; +import type { PlanContext, PlannedOp, SectionPlan } from "../contract/plan.js"; import { rejectDuplicates } from "../contract/requests.js"; -import { DEFAULT_ROLE, INVITATION_ROLES, roleForPermission } from "../shared/roles.js"; +import { + DEFAULT_ROLE, + INVITATION_ROLES, + permissionForRole, + roleForPermission, +} from "../shared/roles.js"; import { knobbed } from "../shared/schema-helpers.js"; +import { knobbedSnapshot } from "../shared/snapshot-helpers.js"; import { CollaboratorConfig } from "./schema.js"; const LiveCollaborator = z.looseObject({ @@ -23,6 +30,7 @@ const LiveCollaborator = z.looseObject({ permissions: z.record(z.string(), z.boolean()).optional(), role_name: z.string().optional(), }); +type LiveCollaborator = z.infer; // `permissions` speaks the READ vocabulary (read/write/...) that roleForPermission maps declared // permissions into; `invitee` is null on email invitations. @@ -70,6 +78,55 @@ const ENDPOINTS = { }, } as const satisfies Record; +type CollaboratorsContext = PlanContext; + +/** The permission a live role declares as; a role no declaration plans as fails loudly. */ +function declaredPermission(label: string, role: string): string { + const permission = permissionForRole(role); + if (permission === undefined) { + throw new Error( + `${label}: the live role "${role}" has no declaration that plans as itself ("${role}" in a settings file means the "${roleForPermission(role)}" role), so it cannot be read back`, + ); + } + return permission; +} + +/** Both pools in one read, so plan() and snapshot() see the same live access. */ +async function readLiveAccess( + ctx: CollaboratorsContext, + section: SectionMeta, +): Promise<{ + collaborators: LiveCollaborator[]; + invitations: NamedInvitation[]; + emailInvitations: LiveInvitation[]; +}> { + const collaborators = parseLive( + section, + ENDPOINTS.list, + z.array(LiveCollaborator), + await ctx.read.list.listAll({ query: { affiliation: "direct" } }), + ); + const allInvitations = parseLive( + section, + ENDPOINTS.listInvitations, + z.array(LiveInvitation), + await ctx.read.listInvitations.listAll(), + ); + return { + collaborators, + invitations: allInvitations.filter(isNamedInvitation), + emailInvitations: allInvitations.filter((invitation) => !isNamedInvitation(invitation)), + }; +} + +function isOwner(ctx: CollaboratorsContext, login: string): boolean { + return login.toLowerCase() === ctx.repo.owner.toLowerCase(); +} + +function emailInvitationNote(invitation: LiveInvitation, outcome: string): string { + return `invitation ${invitation.id} was sent by email, so no username can declare it; ${outcome}`; +} + export const collaboratorsSection = { key: "collaborators", undeclaredDefault: "delete", @@ -90,24 +147,11 @@ export const collaboratorsSection = { (c) => c.username.toLowerCase(), (c) => c.username, ); - const live = parseLive( - this, - ENDPOINTS.list, - z.array(LiveCollaborator), - await ctx.read.list.listAll({ query: { affiliation: "direct" } }), - ); - const liveByLogin = new Map(live.map((c) => [c.login.toLowerCase(), c])); // Both pools are resolved BEFORE the declared walk, so a declared user is never mistaken for // undeclared in the other pool; email invitations (null invitee, which no username can declare) // split into their own pool. - const allInvitations = parseLive( - this, - ENDPOINTS.listInvitations, - z.array(LiveInvitation), - await ctx.read.listInvitations.listAll(), - ); - const invitations = allInvitations.filter(isNamedInvitation); - const emailInvitations = allInvitations.filter((invitation) => !isNamedInvitation(invitation)); + const { collaborators: live, invitations, emailInvitations } = await readLiveAccess(ctx, this); + const liveByLogin = new Map(live.map((c) => [c.login.toLowerCase(), c])); const inviteByLogin = new Map( invitations.map((invitation) => [invitation.invitee.login.toLowerCase(), invitation]), ); @@ -191,7 +235,7 @@ export const collaboratorsSection = { for (const collaborator of live) { const login = collaborator.login.toLowerCase(); - if (login === ctx.repo.owner.toLowerCase() || declaredKeys.has(login)) { + if (isOwner(ctx, login) || declaredKeys.has(login)) { continue; } if (policy === "keep") { @@ -256,9 +300,74 @@ export const collaboratorsSection = { for (const invitation of emailInvitations) { plan.notes.push( - `invitation ${invitation.id} was sent by email, so no username can declare it; left untouched - cancel it from the repository's Access settings if it is unwanted`, + emailInvitationNote( + invitation, + "left untouched - cancel it from the repository's Access settings if it is unwanted", + ), ); } return plan; }, + /** + * Omitted with a note: the owner and email invitations (no-ops for plan()), and expired + * invitations (plan() cancels an undeclared one; a declared one it would cancel and re-send). + * A role the snapshot cannot declare fails loudly: + * dropping the entry would plan a removal, guessing a role would plan a grant. + */ + async snapshot(ctx) { + const { collaborators, invitations, emailInvitations } = await readLiveAccess(ctx, this); + const notes: string[] = []; + const entries: CollaboratorConfig[] = []; + const expired: string[] = []; + for (const collaborator of collaborators) { + const label = `collaborators[${collaborator.login}]`; + if (isOwner(ctx, collaborator.login)) { + notes.push( + `${label}: the repository owner's access is implicit and never managed, so it is not declared`, + ); + continue; + } + if (collaborator.role_name === undefined) { + throw new Error( + `${label}: GitHub reported no role_name for this collaborator, so their permission cannot be read back`, + ); + } + entries.push({ + username: collaborator.login, + permission: declaredPermission(label, collaborator.role_name), + }); + } + for (const invitation of invitations) { + const login = invitation.invitee.login; + const label = `collaborators[${login}]`; + if (invitation.expired === true) { + expired.push(label); + continue; + } + if (invitation.permissions === undefined) { + throw new Error( + `${label}: GitHub reported no permissions on the pending invitation, so it cannot be read back`, + ); + } + entries.push({ + username: login, + permission: declaredPermission(label, invitation.permissions), + }); + } + // With nothing to declare the section is omitted, so apply never reaches the expired ones. + const outcome = + entries.length > 0 + ? "apply cancels it - add the entry to re-invite them" + : "nothing else is declared, so the section is omitted and apply leaves it - declare the entry to re-invite them"; + for (const label of expired) { + notes.push(`${label}: the pending invitation has expired, so it is not declared; ${outcome}`); + } + for (const invitation of emailInvitations) { + notes.push(emailInvitationNote(invitation, "not declared, and apply leaves it untouched")); + } + if (entries.length === 0) { + return { value: undefined, notes }; + } + return { value: knobbedSnapshot(this, entries), notes }; + }, } satisfies SectionModule<"collaborators", typeof ENDPOINTS>; diff --git a/src/sections/collaborators/scenarios/collaborators-snapshot-expired-invitation.yml b/src/sections/collaborators/scenarios/collaborators-snapshot-expired-invitation.yml new file mode 100644 index 00000000..da1d63c9 --- /dev/null +++ b/src/sections/collaborators/scenarios/collaborators-snapshot-expired-invitation.yml @@ -0,0 +1,35 @@ +name: collaborators-snapshot-expired-invitation +description: "mode: snapshot omits an expired invitation with a note instead of declaring it: a declared entry would be cancelled and re-sent on apply, and the undeclared one is cancelled under the delete default the snapshot carries, so the file cannot read clean against this state. The accepted collaborator still reads back." +settings: {} +live_state: + collaborators: + - login: alice + role_name: write + invitations: + - id: 901 + invitee: + login: dave + permissions: write + expired: true +inputs: + mode: snapshot + snapshot_file: snapshot.yml + sections: collaborators +expect: + exit_code: 0 + result: snapshot + outcomes: + collaborators: snapshot + snapshot: + collaborators: + _undeclared: delete + entries: + - username: alice + permission: push + summary_contains: + - "collaborators[dave]: the pending invitation has expired, so it is not declared; apply cancels it - add the entry to re-invite them" + never: + - POST / + - PATCH / + - PUT / + - DELETE / diff --git a/src/sections/collaborators/scenarios/collaborators-snapshot-roundtrip.yml b/src/sections/collaborators/scenarios/collaborators-snapshot-roundtrip.yml new file mode 100644 index 00000000..1a780237 --- /dev/null +++ b/src/sections/collaborators/scenarios/collaborators-snapshot-roundtrip.yml @@ -0,0 +1,47 @@ +name: collaborators-snapshot-roundtrip +description: "mode: snapshot reads the live direct collaborators and pending username invitations back into snapshot.yml and writes nothing to GitHub. Roles read back through the inverse of the declared vocabulary (write -> push, read -> pull, a custom role by name); the repository owner and the email invitation are noted, not declared. The written file, fed back to a check run against the same state, reads clean." +settings: {} +live_state: + collaborators: + - login: e2e-owner + role_name: admin + - login: alice + role_name: write + - login: carol + role_name: security-team + invitations: + - id: 501 + invitee: + login: bob + permissions: read + - id: 502 + invitee: null + permissions: read +inputs: + mode: snapshot + snapshot_file: snapshot.yml + sections: collaborators +expect: + exit_code: 0 + result: snapshot + outcomes: + collaborators: snapshot + snapshot: + collaborators: + _undeclared: delete + entries: + - username: alice + permission: push + - username: carol + permission: security-team + - username: bob + permission: pull + summary_contains: + - "collaborators[e2e-owner]: the repository owner's access is implicit and never managed, so it is not declared" + - "invitation 502 was sent by email, so no username can declare it; not declared, and apply leaves it untouched" + never: + - POST / + - PATCH / + - PUT / + - DELETE / + snapshot_converges: true diff --git a/src/sections/teams/index.ts b/src/sections/teams/index.ts index 3eada591..dd7f1365 100644 --- a/src/sections/teams/index.ts +++ b/src/sections/teams/index.ts @@ -3,12 +3,12 @@ import { z } from "zod"; import type { EndpointDecl } from "../contract/endpoints.js"; import { parseLive } from "../contract/live.js"; -import { loosen, type SectionModule } from "../contract/module.js"; +import { loosen, type SectionMeta, type SectionModule, sectionGrant } from "../contract/module.js"; import type { SectionPermission } from "../contract/permissions.js"; -import type { PlannedOp, SectionPlan } from "../contract/plan.js"; +import type { PlanContext, PlannedOp, SectionPlan } from "../contract/plan.js"; import { rejectDuplicates } from "../contract/requests.js"; -import { DEFAULT_ROLE, roleForPermission } from "../shared/roles.js"; -import { TeamsConfig } from "./schema.js"; +import { DEFAULT_ROLE, permissionForRole, roleForPermission } from "../shared/roles.js"; +import { type TeamConfig, TeamsConfig } from "./schema.js"; const permission: SectionPermission = { repo: ["administration"], org: "members" }; @@ -20,6 +20,10 @@ const ENDPOINTS = { permission: "none", primaryRead: { notFound: "absent" }, }, + list: { + route: "GET /repos/{owner}/{repo}/teams", + statuses: { 200: "the teams with access to the repository" }, + }, probe: { route: "GET /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo}", statuses: { 200: "the team's access to the repository", 404: "the team has no access" }, @@ -30,10 +34,39 @@ const ENDPOINTS = { }, } as const satisfies Record; +type TeamsContext = PlanContext; + // The repo object under the repository media type, of which only role_name is read. Nullish, // because a server ignoring the media type answers a bare 204, which still means "the team has access". const LiveTeamRepo = z.looseObject({ role_name: z.string().optional() }).nullish(); +/** + * A listed team: the slug the probe and the grant address, and how its access was granted + * (present only in a repository listing; absent reads as direct). + */ +const LiveTeam = z.looseObject({ slug: z.string(), access_source: z.string().optional() }); + +/** The probe plan() and snapshot() share, under the media type LiveTeamRepo describes. */ +async function probeTeamRole( + ctx: TeamsContext, + section: SectionMeta, + slug: string, +): Promise<{ access: false } | { access: true; role: string | undefined }> { + const probe = await ctx.read.probe.probeAbsent({ + params: { org: ctx.repo.owner, team_slug: slug }, + accept: "application/vnd.github.v3.repository+json", + }); + if ("missing" in probe) { + return { access: false }; + } + const live = parseLive(section, ENDPOINTS.probe, LiveTeamRepo, probe.data, `team "${slug}"`); + return { access: true, role: live?.role_name }; +} + +function personalAccountNote(owner: string): string { + return `teams: owner "${owner}" is a personal account, not an organization, so team access does not apply`; +} + export const teamsSection = { key: "teams", undeclaredDefault: "untouched", @@ -60,31 +93,20 @@ export const teamsSection = { const orgProbe = await ctx.read.org.probeAbsent({ params: { org: ctx.repo.owner } }); if ("missing" in orgProbe) { plan.notes.push( - `teams: owner "${ctx.repo.owner}" is a personal account, not an organization, so team access does not apply; section skipped - remove the teams section from the settings file to silence this note`, + `${personalAccountNote(ctx.repo.owner)}; section skipped - remove the teams section from the settings file to silence this note`, ); return plan; } for (const team of desired) { const role = team.permission ?? DEFAULT_ROLE; const params = { org: ctx.repo.owner, team_slug: team.name }; - // The repository media type makes this endpoint return the repo object (with role_name) instead of 204. - const probe = await ctx.read.probe.probeAbsent({ - params, - accept: "application/vnd.github.v3.repository+json", - }); + const probe = await probeTeamRole(ctx, this, team.name); const wantRole = roleForPermission(role); let drift: string; - if ("missing" in probe) { + if (!probe.access) { drift = `teams[${team.name}]: no access to ${ctx.repo.slug}; apply will grant "${role}"`; } else { - const live = parseLive( - this, - ENDPOINTS.probe, - LiveTeamRepo, - probe.data, - `team "${team.name}"`, - ); - const liveRole = live?.role_name ?? ""; + const liveRole = probe.role ?? ""; if (liveRole === wantRole) { continue; } @@ -101,4 +123,54 @@ export const teamsSection = { } return plan; }, + /** + * The role comes from the probe, not the listing's `permission`: + * the listing reports a custom role as its base role, role_name names it. + * Omitted with a note, each a no-op since the section never removes an undeclared team: + * non-direct access (declaring it would grant direct access), a probe 404 (no access, or a + * concealed denial), an unreadable role, a role no declaration plans as. + */ + async snapshot(ctx) { + const orgProbe = await ctx.read.org.probeAbsent({ params: { org: ctx.repo.owner } }); + if ("missing" in orgProbe) { + return { value: undefined, notes: [personalAccountNote(ctx.repo.owner)] }; + } + const teams = parseLive(this, ENDPOINTS.list, z.array(LiveTeam), await ctx.read.list.listAll()); + const notes: string[] = []; + const entries: TeamConfig[] = []; + for (const team of teams) { + const label = `teams[${team.slug}]`; + if (team.access_source !== undefined && team.access_source !== "direct") { + notes.push( + `${label}: access to ${ctx.repo.slug} is granted at the ${team.access_source} level, not on the repository; not declared, since declaring it would grant direct access`, + ); + continue; + } + const probe = await probeTeamRole(ctx, this, team.slug); + if (!probe.access) { + // No write follows to surface a denial, so the note names both readings of the 404. + notes.push( + `${label}: listed with access to ${ctx.repo.slug}, but the access probe answered 404, ` + + "read here as no access; not declared. A fine-grained token missing the grant gets the " + + `same answer; if the team does have access, ${sectionGrant(this)}, then snapshot again`, + ); + continue; + } + if (probe.role === undefined) { + notes.push( + `${label}: has access to ${ctx.repo.slug}, but GitHub reported no role for it; not declared - add the entry with the intended permission`, + ); + continue; + } + const permission = permissionForRole(probe.role); + if (permission === undefined) { + notes.push( + `${label}: the live role "${probe.role}" has no declaration that plans as itself ("${probe.role}" in a settings file means the "${roleForPermission(probe.role)}" role); not declared`, + ); + continue; + } + entries.push({ name: team.slug, permission }); + } + return { value: entries.length === 0 ? undefined : entries, notes }; + }, } satisfies SectionModule<"teams", typeof ENDPOINTS>; diff --git a/src/sections/teams/mock.ts b/src/sections/teams/mock.ts index 2ff98689..0c7c19b4 100644 --- a/src/sections/teams/mock.ts +++ b/src/sections/teams/mock.ts @@ -6,14 +6,68 @@ import { restRepoSurface, teamRepoFromPut } from "../../../test/e2e/mock/state.js"; import { asObject, + type Json, noContent, ok, orgProbeHandler, type SectionRestHandlers, + slicePage, } from "../../../test/e2e/mock/support.js"; +import { permissionForRole } from "../shared/roles.js"; + +/** The base roles the listing's `permission` can spell; a custom role collapses to "push" there. */ +const BASE_PERMISSIONS: ReadonlySet = new Set([ + "pull", + "triage", + "push", + "maintain", + "admin", +]); + +/** + * The teams with access, in the team GET shape, one per non-null access entry. The listing's + * `permission` speaks the PUT vocabulary and collapses a custom role to a base role, like GitHub; + * the probe's role_name is where the section reads the role. + */ +function repoTeams(state: { + teams: Record; + org: Json | null; +}): Json[] { + const org = String(state.org?.login ?? "org"); + const orgId = Number(state.org?.id ?? 1); + return Object.entries(state.teams).flatMap(([slug, access], index) => { + if (access === null) { + return []; + } + const id = 7000 + index; + const permission = permissionForRole(access.role_name); + return [ + { + id, + node_id: `T_kwDOAAAAAM${id}`, + name: slug, + slug, + description: null, + privacy: "closed", + notification_setting: "notifications_enabled", + permission: + permission !== undefined && BASE_PERMISSIONS.has(permission) ? permission : "push", + url: `https://api.github.com/organizations/${orgId}/team/${id}`, + html_url: `https://github.com/orgs/${org}/teams/${slug}`, + members_url: `https://api.github.com/organizations/${orgId}/team/${id}/members{/member}`, + repositories_url: `https://api.github.com/organizations/${orgId}/team/${id}/repos`, + type: "organization", + organization_id: orgId, + access_source: "direct", + parent: null, + }, + ]; + }); +} export const teamsMockHandlers: SectionRestHandlers<"teams"> = { "teams.org": orgProbeHandler, + "teams.list": ({ state, query }) => ok(slicePage(repoTeams(state), query)), "teams.probe": ({ state, param }) => { const slug = param("team_slug"); const access = state.teams[slug]; diff --git a/src/sections/teams/scenarios/teams-snapshot-roundtrip.yml b/src/sections/teams/scenarios/teams-snapshot-roundtrip.yml new file mode 100644 index 00000000..60550508 --- /dev/null +++ b/src/sections/teams/scenarios/teams-snapshot-roundtrip.yml @@ -0,0 +1,39 @@ +name: teams-snapshot-roundtrip +description: "mode: snapshot lists the teams with access to an organization repo, reads each team's role through the access probe, and writes nothing to GitHub. Roles read back through the inverse of the declared vocabulary (write -> push, read -> pull, a custom role by name); a custom role literally named push has no declaration that plans as itself, so that team is noted, not declared. The written file, fed back to a check run against the same state, reads clean." +settings: {} +owner_kind: org +live_state: + teams: + platform: + role_name: write + auditors: + role_name: security-auditor + readers: + role_name: read + pushy: + role_name: push +inputs: + mode: snapshot + snapshot_file: snapshot.yml + sections: teams +expect: + exit_code: 0 + result: snapshot + outcomes: + teams: snapshot + snapshot: + teams: + - name: platform + permission: push + - name: auditors + permission: security-auditor + - name: readers + permission: pull + summary_contains: + - "teams[pushy]: the live role \"push\" has no declaration that plans as itself (\"push\" in a settings file means the \"write\" role); not declared" + never: + - POST / + - PATCH / + - PUT / + - DELETE / + snapshot_converges: true diff --git a/src/sections/teams/schema.ts b/src/sections/teams/schema.ts index b23a4b8d..6c744bdd 100644 --- a/src/sections/teams/schema.ts +++ b/src/sections/teams/schema.ts @@ -8,5 +8,6 @@ const TeamConfig = z permission: z.string().optional(), }) .meta({ id: "TeamConfig" }); +export type TeamConfig = z.infer; export const TeamsConfig = z.array(TeamConfig); diff --git a/src/sections/teams/teams.docs.yml b/src/sections/teams/teams.docs.yml index ba6ec379..d2a70f65 100644 --- a/src/sections/teams/teams.docs.yml +++ b/src/sections/teams/teams.docs.yml @@ -1,5 +1,5 @@ sections_table: - endpoints: org team repo permissions + endpoints: org team repo permissions + repo team list notes: org repos only, skipped with a notice on personal accounts coverage: - area: >- @@ -9,6 +9,12 @@ coverage: note on personal accounts (404 only; 403/5xx still fail). Both modes probe each team's access with the v3.repository media type to read role_name, and the grant is issued only when the team lacks access or its live role diverges. + Snapshot lists the teams with access (GET /repos/{owner}/{repo}/teams) and reads each team's + role through the same probe, so a custom role reads back by name; noted, not declared: access + granted at the organization or enterprise level, a probe 404 (no access, or a fine-grained + token missing the grant; the note names both), a role the probe does not report, and a role + no declaration plans as (a custom role named "push" or "pull"). A personal account snapshots + nothing. schema: SettingsFile.teams: Org team access to the repo; skipped on personal accounts. TeamConfig: >- diff --git a/src/sections/teams/teams.test.ts b/src/sections/teams/teams.test.ts index be6e0db8..ba691c94 100644 --- a/src/sections/teams/teams.test.ts +++ b/src/sections/teams/teams.test.ts @@ -8,9 +8,11 @@ import { teamsSection } from "./index.js"; import { teamsMockHandlers } from "./mock.js"; const ORG = "GET /orgs/o"; +const LIST = "GET /repos/o/r/teams?per_page=100&page=1"; const probeOf = (slug: string) => `GET /orgs/o/teams/${slug}/repos/o/r`; const plan = (api: MockApi, desired: Parameters[1]) => teamsSection.plan(planContext(teamsSection, api, REPO), desired); +const snapshot = (api: MockApi) => teamsSection.snapshot(planContext(teamsSection, api, REPO)); describe("teams", () => { test("a personal account no-ops with a note after the org probe alone", async () => { @@ -118,9 +120,9 @@ describe("teams", () => { }); }); - test("the read port exposes the org probe in its absent posture and the team probe, never the grant", () => { + test("the read port exposes the org probe in its absent posture, the list, and the team probe, never the grant", () => { const ctx = planContext(teamsSection, new MockApi({}), REPO); - expect(Object.keys(ctx.read)).toEqual(["org", "probe"]); + expect(Object.keys(ctx.read)).toEqual(["org", "list", "probe"]); // @ts-expect-error a write role is not a read: the port has no `grant` ctx.read.grant; // @ts-expect-error an "absent" primary read offers no throwing helper @@ -128,5 +130,70 @@ describe("teams", () => { // @ts-expect-error nor a list ctx.read.org.listAll; expect(typeof ctx.read.probe.probeAbsent).toBe("function"); + expect(typeof ctx.read.list.listAll).toBe("function"); + }); + + describe("snapshot", () => { + test("reads each listed team's role through the probe, not the listing's permission, so a custom role reads back by name; a probe 404 is noted with both readings", async () => { + const api = new MockApi({ + [ORG]: { data: { login: "o" } }, + [LIST]: { + data: [ + { slug: "platform", permission: "push", access_source: "direct" }, + { slug: "auditors", permission: "pull", access_source: "direct" }, + { slug: "everyone", permission: "pull", access_source: "organization" }, + { slug: "legacy", permission: "push" }, + { slug: "gone", permission: "push", access_source: "direct" }, + { slug: "roleless", permission: "push", access_source: "direct" }, + { slug: "pushy", permission: "push", access_source: "direct" }, + ], + }, + [probeOf("platform")]: { data: { role_name: "write" } }, + [probeOf("auditors")]: { data: { role_name: "security-auditor" } }, + [probeOf("legacy")]: { data: { role_name: "read" } }, + [probeOf("gone")]: { error: { status: 404, message: "Not Found", body: "" } }, + [probeOf("roleless")]: { data: null }, + [probeOf("pushy")]: { data: { role_name: "push" } }, + }); + expect(await snapshot(api)).toEqual({ + value: [ + { name: "platform", permission: "push" }, + { name: "auditors", permission: "security-auditor" }, + { name: "legacy", permission: "pull" }, + ], + notes: [ + "teams[everyone]: access to o/r is granted at the organization level, not on the repository; not declared, since declaring it would grant direct access", + "teams[gone]: listed with access to o/r, but the access probe answered 404, read here as no access; not declared. " + + "A fine-grained token missing the grant gets the same answer; if the team does have access, " + + 'grant "Members" (read) under the PAT\'s Organization permissions and "Administration" (read and write) under its Repository permissions, then snapshot again', + "teams[roleless]: has access to o/r, but GitHub reported no role for it; not declared - add the entry with the intended permission", + 'teams[pushy]: the live role "push" has no declaration that plans as itself ("push" in a settings file means the "write" role); not declared', + ], + }); + expect(api.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ + ORG, + LIST, + probeOf("platform"), + probeOf("auditors"), + probeOf("legacy"), + probeOf("gone"), + probeOf("roleless"), + probeOf("pushy"), + ]); + }); + + test("a personal account snapshots nothing after the org probe alone; an org repo with no team access snapshots nothing after the list", async () => { + const personal = new MockApi({}); + expect(await snapshot(personal)).toEqual({ + value: undefined, + notes: [ + 'teams: owner "o" is a personal account, not an organization, so team access does not apply', + ], + }); + expect(personal.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ORG]); + const empty = new MockApi({ [ORG]: { data: { login: "o" } }, [LIST]: { data: [] } }); + expect(await snapshot(empty)).toEqual({ value: undefined, notes: [] }); + expect(empty.calls.map((c) => `${c.method} ${c.path}`)).toEqual([ORG, LIST]); + }); }); }); diff --git a/test/engine/snapshot.test.ts b/test/engine/snapshot.test.ts index 8c9ab02e..672ddad2 100644 --- a/test/engine/snapshot.test.ts +++ b/test/engine/snapshot.test.ts @@ -112,8 +112,6 @@ describe("snapshotRepository", () => { "branches", "actions", "check_suite_preferences", - "collaborators", - "teams", ]); expect( result.outcomes.filter((o) => o.status === "unsupported").map((o) => [o.key, o.detail]), diff --git a/test/schema-corpus.test.ts b/test/schema-corpus.test.ts index 8867dfe3..ad9859f0 100644 --- a/test/schema-corpus.test.ts +++ b/test/schema-corpus.test.ts @@ -66,7 +66,7 @@ function scenarioDocs(): CorpusDoc[] { } } // The corpus size is pinned exactly so a loader that silently drops a root, a file, or a document kind cannot pass. - expect(docs.length).toBe(305); + expect(docs.length).toBe(311); return docs; } diff --git a/test/scripts/changed-sections.test.ts b/test/scripts/changed-sections.test.ts index 0a66e1db..e2c28a85 100644 --- a/test/scripts/changed-sections.test.ts +++ b/test/scripts/changed-sections.test.ts @@ -114,7 +114,7 @@ describe("changed-sections derived fan-out", () => { "repo-variables.ts": inKeyOrder("actions_variables", "agents_variables"), "setup-section.ts": inKeyOrder("code_scanning_default_setup", "code_quality_setup"), // The snapshot projection and knobbed wrapper: every section with a snapshot() except - // workflows, whose two-field entry is spelled by hand. + // workflows and teams, whose two-field entries are spelled by hand. "snapshot-helpers.ts": inKeyOrder( "labels", "autolinks", @@ -125,6 +125,7 @@ describe("changed-sections derived fan-out", () => { "pages", "code_scanning_default_setup", "code_quality_setup", + "collaborators", "milestones", "interaction_limits", "actions_variables", diff --git a/test/sections/registry.test.ts b/test/sections/registry.test.ts index be34f1e4..9238bb82 100644 --- a/test/sections/registry.test.ts +++ b/test/sections/registry.test.ts @@ -625,6 +625,7 @@ describe("allEndpoints", () => { "secret_scanning_custom_patterns.remove", "secret_scanning_custom_patterns.update", "teams.grant", + "teams.list", "teams.org", "teams.probe", "webhooks.create", diff --git a/test/sections/snapshot-rows/collaborators.ts b/test/sections/snapshot-rows/collaborators.ts new file mode 100644 index 00000000..80b63923 --- /dev/null +++ b/test/sections/snapshot-rows/collaborators.ts @@ -0,0 +1,32 @@ +import { collaboratorsSection } from "../../../src/sections/collaborators/index.js"; +import type { Row } from "../snapshot-roundtrip.js"; + +// The owner is listed as a direct collaborator; the email invitation has no username to declare. +export const row: Row = { + section: collaboratorsSection, + live: { + collaborators: [ + { login: "alice", role_name: "write" }, + { login: "O", role_name: "admin" }, + { login: "carol", role_name: "security-team" }, + ], + invitations: [ + { id: 501, invitee: { login: "bob" }, permissions: "read" }, + { id: 502, invitee: null, permissions: "read" }, + ], + }, + expected: { + value: { + _undeclared: "delete", + entries: [ + { username: "alice", permission: "push" }, + { username: "carol", permission: "security-team" }, + { username: "bob", permission: "pull" }, + ], + }, + notes: [ + "collaborators[O]: the repository owner's access is implicit and never managed, so it is not declared", + "invitation 502 was sent by email, so no username can declare it; not declared, and apply leaves it untouched", + ], + }, +}; diff --git a/test/sections/snapshot-rows/teams.ts b/test/sections/snapshot-rows/teams.ts new file mode 100644 index 00000000..c8c7756e --- /dev/null +++ b/test/sections/snapshot-rows/teams.ts @@ -0,0 +1,27 @@ +import { teamsSection } from "../../../src/sections/teams/index.js"; +import type { Row } from "../snapshot-roundtrip.js"; + +// A null access entry is a team without access, which the listing omits; a custom role named +// "push" has no declaration that plans as itself, and the section never removes an undeclared team. +export const row: Row = { + section: teamsSection, + live: { + teams: { + platform: { role_name: "write" }, + auditors: { role_name: "security-auditor" }, + readers: { role_name: "read" }, + pushy: { role_name: "push" }, + none: null, + }, + }, + expected: { + value: [ + { name: "platform", permission: "push" }, + { name: "auditors", permission: "security-auditor" }, + { name: "readers", permission: "pull" }, + ], + notes: [ + 'teams[pushy]: the live role "push" has no declaration that plans as itself ("push" in a settings file means the "write" role); not declared', + ], + }, +};