mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-18 21:22:28 +03:00
fix(ci): require the reused next-build artifact to come from this repository
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)
This commit is contained in:
17
.github/workflows/npm-publish.yml
vendored
17
.github/workflows/npm-publish.yml
vendored
@@ -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
|
||||
|
||||
90
tests/unit/npm-publish-artifact-provenance.test.ts
Normal file
90
tests/unit/npm-publish-artifact-provenance.test.ts
Normal file
@@ -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"
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user