diff --git a/components/runner-policy/README.md b/components/runner-policy/README.md index 14417d2d..d7c37011 100644 --- a/components/runner-policy/README.md +++ b/components/runner-policy/README.md @@ -305,6 +305,15 @@ repository. Changes outside that bounded runner-contract surface may be auto-approved; any change to the surface itself requires a human to add a new contract entry. +A surface match is not enough on its own: GitHub resolves a commit SHA from +anywhere in a repository's fork network, so a fork-only or unreleased commit +could carry a matching surface around malicious steps. The candidate is +approved only when a tag of its source repository points at it (annotated tags +peeled) and that tag's release is immutable, published, and not a prerelease +(`GET /repos/{owner}/{repo}/git/matching-refs/tags/`, then +`GET /repos/{owner}/{repo}/releases/tags/{tag}`, sent with `GITHUB_TOKEN` when +set). Any other answer, or a failed request, declines. + Several shapes are excluded from this diff-based extension entirely, because no structural surface comparison can prove them unchanged. First, a job whose routing-relevant fields (`runs-on`, `strategy`, the reusable-call diff --git a/components/runner-policy/THREAT-MODEL.md b/components/runner-policy/THREAT-MODEL.md index 09b05e89..a6287e90 100644 --- a/components/runner-policy/THREAT-MODEL.md +++ b/components/runner-policy/THREAT-MODEL.md @@ -86,8 +86,11 @@ GitHub is independent evidence; it must agree with checked-in inventory. of that job in full. A structurally identical surface auto-approves the candidate only after every reviewed revision for the workflow path has been fetched, parsed, and validated, and - every surface-matching revision agrees on the same effective contract terms. - Incomplete basis evidence, no matching basis, any surface change, or + every surface-matching revision agrees on the same effective contract terms, + and only when the candidate SHA is the commit of an immutable, published, + non-prerelease release tag of the source repository (annotated tags peeled). + Incomplete basis evidence, a non-release candidate, a failed tag or release + lookup, no matching basis, any surface change, or disagreement among matching contracts leaves it unapproved and folds the reason into the existing fail-closed diagnostic. A job whose routing-relevant fields reference the `needs` context in any form, on @@ -136,6 +139,7 @@ GitHub is independent evidence; it must agree with checked-in inventory. | A consumer deletes a caller input that narrows a reusable-workflow gate while the shared contract still admits the input optionally, so CI stays green and the caller silently inherits a wider default. | `requiredReusableCallInputs` lets a repository pin presence-only mandatory inputs on the exact reusable-call job key. Every listed input must appear in the caller's `with:` mapping, must be admitted by the reviewed contract, and an unconsumed entry fails as inventory drift. Values are not pinned: widening still requires an explicit diff line, not an invisible deletion. | Required reusable-call input presence, presence-only value acceptance, contract-admission, and drift cases in [`runner-policy.test.mjs`](runner-policy.test.mjs). | | A policy or dependency change weakens validation unnoticed. | Central and repository schemas reject unknown shapes; component dependencies are exactly locked; CI runs behavioral tests and then audits this repository with external visibility evidence. | [`policy.schema.json`](policy.schema.json), [`repository-policy.schema.json`](repository-policy.schema.json), [`package-lock.json`](package-lock.json), and the `check-policies` CI job. | | A Dependabot SHA bump of an already-reviewed reusable workflow is auto-approved even though the bump silently removes callability, changes permissions/inputs/secrets/routing/boundaries, adds a called job's credential-minting action or unapproved credential expression, swaps an already-declared/allowed secret for a different secret in the identical position, repoints an already-declared credential-minting action at a different, unreviewed `@ref` while keeping the action name identical, rewrites a step body or a sibling env value that consumes a credential exposed by workflow- or job-scope `env` while every field containing a credential expression stays byte-identical, changes workflow- or job-level run `defaults` (shell, working-directory) so byte-identical step bodies execute differently, or inherits a broader contract from one of several matching reviewed revisions by insertion order or partial fetch evidence. | Auto-approval requires a structurally identical match of `on.workflow_call` presence/validity, workflow- and effective job-level permissions, declared inputs/secrets, workflow- and job-level run defaults, job routing/nested calls, container/service/environment declarations, the same privileged-control-plane credential detection enforced against every directly declared or repository-local job, and the exact credential-bearing values each job references, between reviewed revisions and the candidate, all fetched fresh from the source repository. The credential-value comparison includes a `localCredentialActions` step's full normalized `uses:` value, not just that detection's category or the bare action name, and the full inherited env (sibling values included), full non-step job mapping, and every step in full for any job whose workflow- or job-scope context exposes a credential. Every reviewed revision for the path must fetch, parse, and validate before every surface-matching revision is compared for agreement on the effective input, secret, runner-input, and hosted-only contract; incomplete evidence, any diff, or ambiguity fails closed with a deterministic diagnostic. `disableAutoApproval`/`CI_RUNNER_POLICY_DISABLE_AUTO_APPROVAL=true` restores the pre-auto-approval behavior for any repository that wants no automatic extension at all. | Identical-surface and identical-contract, contract-ambiguity/permutation, partial-evidence fetch/parse failures, removed/malformed-callability, omitted/empty and widened-permissions, changed-inputs, changed-routing/boundaries, changed workflow- and job-level run-defaults, added-credential-action, changed-credential-reference, changed-credential-action-ref, inherited-env (workflow-scope step-body, sibling-value, job-scope, and unchanged-surface control), and escape-hatch cases in [`runner-policy.test.mjs`](runner-policy.test.mjs). | +| A path@SHA reference names a fork-only or unreleased commit (GitHub resolves a SHA from anywhere in the repository's fork network) whose compared surface matches a reviewed basis while its steps are malicious, and auto-approval carries the reviewed contract, including forwarded hosted-only secrets, onto it. | Auto-approval approves a candidate only when a tag of the source repository, peeled through annotated tag objects, points at the SHA and that tag's release is immutable, published, and not a prerelease (`git/matching-refs/tags/`, `git/tags/{sha}`, `releases/tags/{tag}`). A non-release commit declines with a diagnostic; any failed tag or release request declines. | Release-check cases (lightweight and annotated release tags, annotated tag peeling to another commit, non-release SHA with an identical surface, tag without a release, draft, prerelease, and non-immutable releases, tag-list and release-lookup errors) in [`runner-policy.test.mjs`](runner-policy.test.mjs). | | A Dependabot SHA bump keeps a job's literal `runs-on` (or other routing field) unchanged while a value it indirectly resolves through the `needs` context, in any form and any letter case, changes the actual runner/container/environment boundary: a job output, a job `result`, an object-filter expression across every job's outputs (e.g. `needs.*.outputs.runner`, which has no named job-id segment for a job-id-shaped detector to enumerate), or any other property or index reachable off `needs`, and the bump is still auto-approved because the structural surface diff cannot observe the change. A detector that enumerates specific dangerous `needs` spellings will always have a next gap, because GitHub's expression grammar for context/property access is large and can grow (new index forms, new filters, new functions). | Any job on either the reviewed basis or the candidate whose routing-relevant fields reference `needs` in any form declines auto-approval unconditionally: the check is a coarse allowlist of provably `needs`-free routing fields, not an enumerated blocklist of indirection syntaxes, so it does not need to recognize the specific shape of the reference to decline it. It requires a human to add a new contract entry. | Needs-reference indirection (candidate and basis; property dereference, index-syntax, object-filter, case-variant, and non-outputs `needs` property cases) in [`runner-policy.test.mjs`](runner-policy.test.mjs). | | A Dependabot SHA bump of a secret-capable runner-input workflow (a reviewed contract whose nonempty `allowedSecrets` mapping lets a statically read-only caller forward exact named secrets to a caller-chosen, potentially self-hosted runner) changes what the workflow's steps do with a forwarded secret while every compared field stays identical, silently extending the secret-forwarding trust to unreviewed executable content. | Any reviewed runner-input contract with a nonempty `allowedSecrets` mapping declines auto-approval unconditionally, for the same reason `allowedCallerPermissions` does: the trusted property depends on the called workflow's own steps, which sit outside the compared surface. A human must review the new SHA's content and add a new contract entry. Hosted-only contracts keep their existing eligibility because their secrets stay bound to the fixed hosted runner recorded in the reviewed contract. | `resolveAutoApprovedContracts`'s secret-capable decline in [`runner-policy.mjs`](runner-policy.mjs); secret-capable contract decline case in [`runner-policy.test.mjs`](runner-policy.test.mjs). | | A Dependabot SHA bump of a workflow reviewed under a privileged `allowedCallerPermissions` grant (an exact caller permission and secret boundary that lets a job reach a self-hosted runner despite elevated permissions) changes what the workflow's steps do with that grant, for example exfiltrating an `id-token`-derived credential or misusing a `pull-requests: write` grant, while every field this diff compares (declared permissions, routing, and credential *references*) stays identical, because the diff never inspects step bodies for content. Auto-approval would then silently extend an already-reviewed privileged grant to unreviewed executable content. | Any reviewed contract carrying `allowedCallerPermissions` declines auto-approval unconditionally regardless of surface match, because the property being trusted depends on the called workflow's own steps, which sit outside the compared surface, and here that property is a privileged permission and self-hosted-reachability grant. A human must review the new SHA's content and add a new contract entry. | `resolveAutoApprovedContracts`'s unconditional `allowedCallerPermissions` decline in [`runner-policy.mjs`](runner-policy.mjs); allowedCallerPermissions-contract case in [`runner-policy.test.mjs`](runner-policy.test.mjs). | @@ -155,7 +159,8 @@ not duplicate those changing inventories. identical security surface to a later SHA of the same workflow path, and it fetches the diffed bytes fresh from the source repository at audit time rather than trusting a cached or asserted copy. -- The auto-approval fetch depends on `raw.githubusercontent.com` availability. +- The auto-approval fetch depends on `raw.githubusercontent.com` and + `api.github.com` availability. A network outage or block does not approve an unreviewed candidate; the audit fails closed on the fetch error, the same as an unreviewed reference. Repositories that want zero audit-time network access can set diff --git a/components/runner-policy/runner-policy.mjs b/components/runner-policy/runner-policy.mjs index c921e59e..d4f67f5a 100755 --- a/components/runner-policy/runner-policy.mjs +++ b/components/runner-policy/runner-policy.mjs @@ -1577,22 +1577,29 @@ function errorText(error) { return error instanceof Error ? error.message : String(error); } -async function fetchCommitTree(repository, revision, fetchImpl, githubToken) { - const label = `${repository}@${revision}`; +async function fetchGitHubJson(apiPath, fetchImpl, githubToken) { const headers = { accept: "application/vnd.github+json" }; if (githubToken) { headers.authorization = `Bearer ${githubToken}`; } + const response = await fetchImpl(`${GITHUB_API_BASE}${apiPath}`, { headers }); + if (!response.ok) { + throw Object.assign(new Error(`${response.status} ${response.statusText}`), { + status: response.status, + }); + } + return response.json(); +} + +async function fetchCommitTree(repository, revision, fetchImpl, githubToken) { + const label = `${repository}@${revision}`; let body; try { - const response = await fetchImpl( - `${GITHUB_API_BASE}/repos/${repository}/git/trees/${revision}?recursive=1`, - { headers }, + body = await fetchGitHubJson( + `/repos/${repository}/git/trees/${revision}?recursive=1`, + fetchImpl, + githubToken, ); - if (!response.ok) { - throw new Error(`${response.status} ${response.statusText}`); - } - body = await response.json(); } catch (error) { throw new ConfigurationError(`could not fetch the git tree of ${label}: ${errorText(error)}`); } @@ -1927,13 +1934,51 @@ async function fetchReusableWorkflowSource(workflowPath, revision, fetchImpl) { return response.text(); } +// GitHub resolves a commit SHA from anywhere in a repository's fork network, +// so a matching surface alone would carry a reviewed contract onto a +// fork-only or unreleased commit. A candidate is eligible only when a tag of +// its source repository, peeled through annotated tag objects, points at it +// and that tag's release is published, immutable, and not a prerelease. +async function isImmutableReleaseCommit(repository, revision, fetchImpl, githubToken) { + const fetchJson = (apiPath) => fetchGitHubJson(apiPath, fetchImpl, githubToken); + const refs = await fetchJson(`/repos/${repository}/git/matching-refs/tags/`); + if (!Array.isArray(refs)) { + throw new Error("the tag list is malformed"); + } + for (const ref of refs) { + let object = ref?.object; + while (object?.type === "tag") { + object = (await fetchJson(`/repos/${repository}/git/tags/${object.sha}`))?.object; + } + if (object?.type !== "commit" || object.sha.toLowerCase() !== revision.toLowerCase()) { + continue; + } + let release; + try { + release = await fetchJson( + `/repos/${repository}/releases/tags/${encodeURIComponent(ref.ref.replace(/^refs\/tags\//, ""))}`, + ); + } catch (error) { + if (error.status === 404) { + continue; + } + throw error; + } + if (release?.draft === false && release.prerelease === false && release.immutable === true) { + return true; + } + } + return false; +} + // Deterministic, non-LLM auto-approval: a new path@SHA reusable-workflow // reference that has no reviewed contract yet is eligible only when (a) the // exact same workflow path already has at least one reviewed contract at a // different SHA (i.e. the source is already trusted), and (b) fetching both // the previously approved SHA and the new SHA from the source repository and // structurally diffing workflow_call presence and validity, permissions, -// workflow_call inputs/secrets, and job routing shows no change. If multiple +// workflow_call inputs/secrets, and job routing shows no change, and (c) the +// new SHA is a release commit (isImmutableReleaseCommit). If multiple // reviewed revisions match that fetched surface, their effective reviewed // contracts must also agree; contract ambiguity fails closed rather than // letting policy insertion order select the inherited authority. Every @@ -2176,6 +2221,21 @@ async function resolveAutoApprovedContracts({ continue; } + let releaseDecline; + try { + if ( + !(await isImmutableReleaseCommit(sourceRepository, parsed.revision, fetchImpl, githubToken)) + ) { + releaseDecline = `is not the commit of an immutable, published, non-prerelease release tag of ${sourceRepository}`; + } + } catch (error) { + releaseDecline = `could not be checked against the release tags of ${sourceRepository}: ${errorText(error)}`; + } + if (releaseDecline) { + diagnostics.set(reference, `${reference} ${releaseDecline}, so auto-approval is declined`); + continue; + } + approved.set(reference, { ...matchedBasis.contract, autoApproved: { basisSha: matchedBasis.revision, approvedAt: now().toISOString() }, diff --git a/components/runner-policy/runner-policy.test.mjs b/components/runner-policy/runner-policy.test.mjs index 70f53412..c9153b38 100644 --- a/components/runner-policy/runner-policy.test.mjs +++ b/components/runner-policy/runner-policy.test.mjs @@ -219,15 +219,59 @@ function audit(root, options = {}) { }); } +// Serves the tag refs and release-by-tag reads of the release check: each +// `tags` entry is [tag name, ref object], and every tag's release has the +// GitHub release fields in `release` (immutable and published by default). +// Annotated tag objects resolve through `tagObjects` (tag object SHA -> object). +function releaseFetchImpl(inner, tags, { release = {}, tagObjects = {} } = {}) { + return async (url, init) => { + if (url.endsWith("/git/matching-refs/tags/")) { + return { + ok: true, + json: async () => tags.map(([name, object]) => ({ ref: `refs/tags/${name}`, object })), + }; + } + const tagObject = /\/git\/tags\/([0-9a-f]{40})$/.exec(url); + if (tagObject && tagObjects[tagObject[1]]) { + return { ok: true, json: async () => ({ object: tagObjects[tagObject[1]] }) }; + } + const releaseTag = /\/releases\/tags\/([^/]+)$/.exec(url); + if (releaseTag && release === null) { + return { ok: false, status: 404, statusText: "Not Found" }; + } + if (releaseTag) { + return { + ok: true, + json: async () => ({ + tag_name: releaseTag[1], + draft: false, + prerelease: false, + immutable: true, + ...release, + }), + }; + } + return inner(url, init); + }; +} + +// Every SHA in `shas` is the commit of its own lightweight release tag. +function releasedShas(inner, shas) { + return releaseFetchImpl( + inner, + shas.map((sha, index) => [`v0.0.${index}`, { type: "commit", sha }]), + ); +} + function fetchImplFor(sourcesBySha) { - return async (url) => { + return releasedShas(async (url) => { for (const [sha, body] of Object.entries(sourcesBySha)) { if (url.includes(`/${sha}/`)) { return { ok: true, text: async () => body }; } } return { ok: false, status: 404, statusText: "Not Found" }; - }; + }, Object.keys(sourcesBySha)); } const REUSABLE_WORKFLOW_BASIS_SOURCE = `name: osv-scanner @@ -4306,6 +4350,100 @@ test("Dependabot SHA bump with an identical security surface is auto-approved", assert.deepEqual(findings, []); }); +const RELEASE_TAG_OBJECT_SHA = "fedcba9876543210fedcba9876543210fedcba98"; +const NOT_A_RELEASE_COMMIT = `${DEPENDABOT_BUMP_REFERENCE} is not the commit of an immutable, published, non-prerelease release tag of melodic-software/ci-workflows, so auto-approval is declined`; + +async function releaseCheckFindings(wrap) { + const root = await repository({ + visibility: "public", + selfHostedCi: false, + workflows: { + "ci.yml": `jobs: + scan: + uses: ${DEPENDABOT_BUMP_REFERENCE} + with: + runner: ubuntu-24.04 +`, + }, + }); + return audit(root, { + fetchImpl: wrap( + fetchImplFor({ + [SHA]: REUSABLE_WORKFLOW_BASIS_SOURCE, + [DEPENDABOT_BUMP_SHA]: REUSABLE_WORKFLOW_IDENTICAL_SURFACE_SOURCE, + }), + ), + }); +} + +for (const [label, tags, options, expected] of [ + [ + "approves a lightweight release-tag commit", + [["v0.65.13", { type: "commit", sha: DEPENDABOT_BUMP_SHA }]], + {}, + null, + ], + [ + "peels an annotated release tag to its commit", + [["v0.65.13", { type: "tag", sha: RELEASE_TAG_OBJECT_SHA }]], + { tagObjects: { [RELEASE_TAG_OBJECT_SHA]: { type: "commit", sha: DEPENDABOT_BUMP_SHA } } }, + null, + ], + [ + "declines an annotated release tag that peels to another commit", + [["v0.65.12", { type: "tag", sha: RELEASE_TAG_OBJECT_SHA }]], + { tagObjects: { [RELEASE_TAG_OBJECT_SHA]: { type: "commit", sha: SHA } } }, + NOT_A_RELEASE_COMMIT, + ], + [ + "declines a non-release SHA with an identical surface", + [["v0.65.12", { type: "commit", sha: SHA }]], + {}, + NOT_A_RELEASE_COMMIT, + ], + [ + "declines a tag without a release", + [["v0.65.13", { type: "commit", sha: DEPENDABOT_BUMP_SHA }]], + { release: null }, + NOT_A_RELEASE_COMMIT, + ], + ...[{ draft: true }, { prerelease: true }, { immutable: false }].map((release) => [ + `declines a release with ${JSON.stringify(release)}`, + [["v0.65.13", { type: "commit", sha: DEPENDABOT_BUMP_SHA }]], + { release }, + NOT_A_RELEASE_COMMIT, + ]), +]) { + test(`Dependabot SHA bump release check ${label}`, async () => { + const findings = await releaseCheckFindings((inner) => releaseFetchImpl(inner, tags, options)); + if (expected === null) { + assert.deepEqual(findings, []); + } else { + assert.ok( + contractFindingMessage(findings).includes(expected), + contractFindingMessage(findings), + ); + } + }); +} + +for (const failing of ["/git/matching-refs/tags/", "/releases/tags/v0.0.1"]) { + test(`Dependabot SHA bump release check declines when ${failing} errors`, async () => { + const findings = await releaseCheckFindings( + (inner) => async (url, init) => + url.endsWith(failing) + ? { ok: false, status: 500, statusText: "Internal Server Error" } + : inner(url, init), + ); + assert.ok( + contractFindingMessage(findings).includes( + `${DEPENDABOT_BUMP_REFERENCE} could not be checked against the release tags of melodic-software/ci-workflows: 500 Internal Server Error, so auto-approval is declined`, + ), + contractFindingMessage(findings), + ); + }); +} + test("Dependabot SHA bump declines ambiguous surface-matching reviewed contracts independent of insertion order", async () => { const runnerContract = ({ runnerInput = "runner", @@ -5678,7 +5816,7 @@ function scanSetupTree(treeSha, { actionSource = SCAN_SETUP_ACTION_BLOB_SHA, ext // (raw.githubusercontent.com). `files` maps ":" or "" to // content; every other request is a 404. function commitRelativeFetchImpl({ workflows, trees, files = {}, truncated = false }) { - return async (url) => { + return releasedShas(async (url) => { const treeMatch = COMMIT_RELATIVE_TREE_URL.exec(url); if (treeMatch) { const tree = trees[treeMatch[1]]; @@ -5699,7 +5837,7 @@ function commitRelativeFetchImpl({ workflows, trees, files = {}, truncated = fal } } return { ok: false, status: 404, statusText: "Not Found" }; - }; + }, Object.keys(workflows)); } async function dependabotBumpRepository() {