From c2821e670c05b600c083ea2ad84c8e95b9ee83f6 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 30 Jul 2026 04:46:54 -0300 Subject: [PATCH] fix(ci): require the reused next-build artifact to come from this repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL raised actions/artifact-poisoning/critical on the `next-build` fast path this PR builds on (#8941). The finding is real and it sits on the path that produces the published npm tarball. The step picks a CI run by querying the runs API for `head_sha` and filtering on `name == "CI" and conclusion == "success"`. That query also returns `pull_request` runs from FORKS: they execute in this repository's context and upload their own `next-build`, built from fork-controlled source. Measured today, 57 runs in this repo have a `head_repository` other than the repo itself. So the selection trusted bytes by coincidence of commit SHA — anything that made a fork's head commit coincide with the publish commit could put attacker-built bytes on npm. Adds `and .head_repository.full_name == env.REPO` to the selection. Provenance is now explicit; `head_sha` still carries tree-equality. Verified against the live API using the expression extracted from the workflow itself — the same single run (30518663668) is selected either way for the current tip, so the fast path keeps working while every fork run is excluded. Not a dismissal (hard rule #14) — the clause removes the flagged trust. node --import tsx/esm --test tests/unit/npm-publish-artifact-provenance.test.ts # 3 pass, 0 fail (base: 2 pass, 1 fail) --- .github/workflows/npm-publish.yml | 17 +++- .../npm-publish-artifact-provenance.test.ts | 90 +++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 tests/unit/npm-publish-artifact-provenance.test.ts diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 1b2fe6a6e8..ea9cad488d 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -159,6 +159,17 @@ jobs: # `head_sha` is the tree-equality guarantee: same commit, same tree. # Best-effort by design (retention is 1 day): every miss falls through to the build # step below, which is why the dynamic runner above matters as the backstop. + # + # The `head_repository.full_name == env.REPO` clause is a supply-chain guard, not a + # filter refinement. This artifact becomes the published npm tarball. `pull_request` + # runs from forks execute in THIS repository's context and upload their own + # `next-build` built from fork-controlled source, and the runs API returns them for a + # matching `head_sha` — 57 such runs exist in this repo today. Without the clause, + # anything that made a fork's head commit coincide with the publish commit could put + # attacker-built bytes on npm. Requiring the run to originate from this repository + # excludes every fork run while keeping the fast path intact (verified: the same + # single run is selected either way for the current tip). + # CodeQL: actions/artifact-poisoning/critical. - name: Reuse CI's next-build artifact (skips the heavy rebuild) if: steps.resolve.outputs.skip != 'true' continue-on-error: true @@ -169,7 +180,11 @@ jobs: run: | set -uo pipefail RUN=$(gh api "repos/$REPO/actions/runs?head_sha=$HEAD_SHA&per_page=100" \ - --jq '[.workflow_runs[] | select(.name == "CI" and .conclusion == "success")] | .[0].id // empty') || RUN="" + --jq '[.workflow_runs[] + | select(.name == "CI" + and .conclusion == "success" + and .head_repository.full_name == env.REPO)] + | .[0].id // empty') || RUN="" if [ -z "$RUN" ]; then echo "::notice::no successful CI run for $HEAD_SHA — falling back to a full build" exit 0 diff --git a/tests/unit/npm-publish-artifact-provenance.test.ts b/tests/unit/npm-publish-artifact-provenance.test.ts new file mode 100644 index 0000000000..392e469a5b --- /dev/null +++ b/tests/unit/npm-publish-artifact-provenance.test.ts @@ -0,0 +1,90 @@ +/** + * Supply-chain guard for the `next-build` fast path in npm-publish.yml. + * + * The publish job restores a CI-built standalone tree and ships it as the npm tarball. + * The run it restores from is picked by querying the runs API for `head_sha`. That query + * also returns `pull_request` runs from FORKS — they execute in this repository's context + * and upload their own `next-build`, built from fork-controlled source. Measured on + * 2026-07-30: 57 runs in this repo have a `head_repository` other than the repo itself. + * + * Selecting on name + conclusion alone therefore trusts bytes by coincidence of commit + * SHA. The `head_repository.full_name == env.REPO` clause is what makes the provenance + * explicit. Raised as CodeQL `actions/artifact-poisoning/critical`. + * + * This is a guard, not a reproduction: nothing here can exercise a real poisoning attempt. + * It asserts the clause cannot be dropped in a future edit without a test turning red. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); + +function readPublishWorkflow(): string { + return fs.readFileSync(path.join(repoRoot, ".github/workflows/npm-publish.yml"), "utf-8"); +} + +/** The `run:` body of the step whose `name:` matches, at any indentation. */ +function extractStep(yaml: string, stepName: string): string { + const lines = yaml.split("\n"); + const startIdx = lines.findIndex((l) => l.includes(`- name: ${stepName}`)); + assert.ok(startIdx !== -1, `npm-publish.yml must define the "${stepName}" step`); + const indent = lines[startIdx].indexOf("- name:"); + const block: string[] = []; + for (let i = startIdx + 1; i < lines.length; i++) { + // The next list item at the same indentation ends this step. + if (lines[i].slice(indent).startsWith("- ")) break; + block.push(lines[i]); + } + return block.join("\n"); +} + +const ARTIFACT_STEP = "Reuse CI's next-build artifact (skips the heavy rebuild)"; + +test("the artifact fast path only trusts runs from THIS repository", () => { + const step = extractStep(readPublishWorkflow(), ARTIFACT_STEP); + + assert.match( + step, + /head_repository\.full_name == env\.REPO/, + "the run selection must require the run to originate from this repository — without it, " + + "a fork's pull_request run supplies the bytes that get published to npm" + ); + // Provenance is necessary but not sufficient: tree-equality still comes from head_sha. + assert.match( + step, + /head_sha=\$HEAD_SHA/, + "the run must still be matched on head_sha — that is the tree-equality guarantee" + ); + assert.match(step, /\.conclusion == "success"/, "only a successful run may be restored"); +}); + +test("REPO is passed through env, never interpolated into the script body", () => { + const step = extractStep(readPublishWorkflow(), ARTIFACT_STEP); + + assert.match(step, /REPO:\s*\$\{\{\s*github\.repository\s*\}\}/, "REPO must come from env:"); + // Hard rule #13 / zizmor template-injection: no ${{ }} inside the run: body. + const runBody = step.slice(step.indexOf("run: |")); + assert.doesNotMatch( + runBody, + /\$\{\{/, + "the run: body must not interpolate any ${{ }} expression — pass values via env:" + ); +}); + +test("a miss falls through to a real build instead of publishing an empty tree", () => { + const step = extractStep(readPublishWorkflow(), ARTIFACT_STEP); + + assert.match( + step, + /continue-on-error:\s*true/, + "the fast path is best-effort — a miss must not fail the publish" + ); + assert.match( + step, + /falling back to a full build/, + "a miss must say so, so a silent no-op cannot look like a restore" + ); +});