fix(verify-npm-provenance): write outputs in heredoc form (#4)

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

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<<DELIM` line
+ * opens a block that ends at a line exactly equal to DELIM, and everything in
+ * between is literal data; otherwise `key=value`. Later keys win, which is the
+ * property the old format let an attacker exploit.
+ */
+function parseOutputs(rendered) {
+  const parsed = new Map();
+  const lines = rendered.split("\n");
+  for (let i = 0; i < lines.length; i++) {
+    const heredoc = /^([A-Za-z_][\w-]*)<<(\S+)$/.exec(lines[i]);
+    if (heredoc) {
+      const [, key, delim] = heredoc;
+      const body = [];
+      i++;
+      while (i < lines.length && lines[i] !== delim) body.push(lines[i++]);
+      parsed.set(key, body.join("\n"));
+      continue;
+    }
+    const kv = /^([A-Za-z_][\w-]*)=(.*)$/.exec(lines[i]);
+    if (kv) parsed.set(kv[1], kv[2]);
+  }
+  return parsed;
+}
+
+test("a newline in detail cannot forge a second status assignment", () => {
+  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$/);
+});