diff --git a/actions/verify-npm-provenance/src/main.js b/actions/verify-npm-provenance/src/main.js index 2834b95..92cc160 100644 --- a/actions/verify-npm-provenance/src/main.js +++ b/actions/verify-npm-provenance/src/main.js @@ -1,7 +1,31 @@ +import { randomUUID } from "node:crypto"; import { appendFileSync } from "node:fs"; import { classifyProvenance, decodeStatements } from "./classify.js"; import { fetchAttestations } from "./fetch-attestations.js"; +/** + * Render workflow outputs using the heredoc-delimiter form. + * + * `detail` is not a fixed vocabulary — it is built from registry-supplied data + * (the DSSE `repository` field, for one). With the plain `key=value` form a + * newline in that value emits a second `status=` line, and GitHub resolves + * duplicate keys last-wins, so a `source-mismatch` could be read downstream as + * `ok`. A random per-call delimiter means a value can never close its own block + * and so can never start a line the runner would parse as a key assignment. + */ +export function renderOutputs(entries, newDelimiter = defaultDelimiter) { + let block = ""; + for (const [key, value] of entries) { + const delim = newDelimiter(); + block += `${key}<<${delim}\n${String(value)}\n${delim}\n`; + } + return block; +} + +function defaultDelimiter() { + return `ghadelim_${randomUUID().replaceAll("-", "")}`; +} + /** Map a fetch outcome plus the classifier into a single status. */ export function decide(fetched, expected) { if (fetched.outcome === "absent") { @@ -55,7 +79,15 @@ if (process.env["NODE_ENV"] !== "test" && process.argv[1]?.endsWith("main.js")) const { status, detail } = decide(fetched, expected); const out = process.env["GITHUB_OUTPUT"]; - if (out) appendFileSync(out, `status=${status}\ndetail=${detail}\n`); + if (out) { + appendFileSync( + out, + renderOutputs([ + ["status", status], + ["detail", detail], + ]), + ); + } console.log(`npm provenance: ${pkg}@${version} -> ${status} (${detail})`); if (status !== "ok" && severity === "gate") console.log(runbook(pkg, version, status, detail)); // Set exitCode instead of calling process.exit(): exit() tears the process diff --git a/actions/verify-npm-provenance/test/main.test.js b/actions/verify-npm-provenance/test/main.test.js index 14727ef..14fc6ef 100644 --- a/actions/verify-npm-provenance/test/main.test.js +++ b/actions/verify-npm-provenance/test/main.test.js @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { decide, exitCodeFor } from "../src/main.js"; +import { decide, exitCodeFor, renderOutputs } from "../src/main.js"; test("absent attestation is missing-provenance", () => { const r = decide({ outcome: "absent", detail: "404 after full backoff" }, { repo: "a/b" }); @@ -31,3 +31,61 @@ test("same input yields different severity, same status", () => { assert.equal(exitCodeFor(status, "gate"), 1); assert.equal(exitCodeFor(status, "monitor"), 0); }); + +// `detail` is built from registry-supplied data, so it is attacker-influenced +// in the only threat model that matters here. With the old `key=value` form a +// newline in it emitted a second `status=` line, and GitHub takes the LAST +// occurrence of a duplicate key — turning a source-mismatch into a reported ok. +/** + * Parse a $GITHUB_OUTPUT file the way the runner does: a `key< { + const injected = "repository evil/repo != a/b\nstatus=ok"; + const parsed = parseOutputs( + renderOutputs([ + ["status", "source-mismatch"], + ["detail", injected], + ]), + ); + // The whole point: the payload is data, so the real verdict survives. + assert.equal(parsed.get("status"), "source-mismatch"); + assert.equal(parsed.get("detail"), injected); +}); + +test("a value cannot close its own heredoc block", () => { + // Even if the payload guesses the delimiter *shape*, the random suffix means + // the emitted closing line is not one the value can reproduce. + const rendered = renderOutputs([["detail", "x\nghadelim_deadbeef\ny"]], () => "ghadelim_stable"); + const closers = rendered.split("\n").filter((line) => line === "ghadelim_stable"); + assert.equal(closers.length, 1); +}); + +test("ordinary values round-trip through the heredoc form", () => { + const rendered = renderOutputs([ + ["status", "ok"], + ["detail", "verified"], + ]); + assert.match(rendered, /^status<<(\S+)\nok\n\1\ndetail<<(\S+)\nverified\n\2\n$/); +});