From 97ab84193c4f1562cc432ece78a4f366932ad6fe Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:21:45 +0200 Subject: [PATCH 01/15] fix(scan): index unquoted scalar assignments so a variable-routed publish is audited shellScalars recorded only quoted assignments, so a bare `NPM=npm` was never indexed. expandScalars then left `$NPM` in place, the tokeniser read the program as `$NPM` rather than `npm`, and the line was not recognised as a publish at all. Because the workflow's own legitimate publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures: the gate reported a clean pass over a workflow containing an unattested publish. It was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking. Add a third alternative for an unquoted single-word value, matching the shape pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is untouched, so a value that would change how its line parses is still never inlined. The regression test asserts the audit-level property rather than the contents of the scalar map, so it fails for the reason the gate exists: with this change reverted it reports zero failures where one is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the previous behaviour as deliberate and is updated to match, as pm-jira updated it. Found by probing every package's own verifier with one attested publish beside one variable-routed unattested publish; twelve of seventeen returned no failures. The root cause is that this verifier is vendored per package rather than consumed from pm-ops, so each fix leaves the other copies exposed. --- .agents/pm/history/pm-github-tko1.jsonl | 2 ++ .agents/pm/issues/pm-github-tko1.toon | 13 ++++++++++++ scripts/shell-command-scan.ts | 8 +++---- ...verify-release-publish-attestation.test.ts | 21 ++++++++++++++++++- 4 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 .agents/pm/history/pm-github-tko1.jsonl create mode 100644 .agents/pm/issues/pm-github-tko1.toon diff --git a/.agents/pm/history/pm-github-tko1.jsonl b/.agents/pm/history/pm-github-tko1.jsonl new file mode 100644 index 0000000..25ce8e7 --- /dev/null +++ b/.agents/pm/history/pm-github-tko1.jsonl @@ -0,0 +1,2 @@ +{"ts":"2026-08-28T22:09:55.294Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"create","patch":[{"op":"add","path":"/metadata/id","value":"pm-github-tko1"},{"op":"add","path":"/metadata/title","value":"The publish-attestation gate misses a publish routed through an unquoted shell variable"},{"op":"add","path":"/metadata/description","value":""},{"op":"add","path":"/metadata/type","value":"Issue"},{"op":"add","path":"/metadata/status","value":"open"},{"op":"add","path":"/metadata/priority","value":1},{"op":"add","path":"/metadata/tags","value":[]},{"op":"add","path":"/metadata/created_at","value":"2026-08-28T22:09:55.294Z"},{"op":"add","path":"/metadata/updated_at","value":"2026-08-28T22:09:55.294Z"},{"op":"add","path":"/metadata/author","value":"claude"}],"before_hash":"3cc22dff72be7b14824654a7a64ea62b04799939b2fee54c1b5f52ca60bf6df0","after_hash":"137ded4c5cea997218e47efc0dc67df526fe452165dd504d1689238fe1ab9dea","item_hash_version":2,"message":"","context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} +{"ts":"2026-08-28T22:09:57.974Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:09:57.974Z"},{"op":"add","path":"/metadata/notes","value":[{"created_at":"2026-08-28T22:09:57.888Z","author":"codex","text":"author=claude\ncreated_at=now\ntext=shellScalars indexed only quoted assignments, so a bare NPM=npm was never recorded. expandScalars then left $NPM unresolved, tokenizeCommands saw a program named \"$NPM\" rather than npm, and the line was not recognised as a publish at all. Because the workflow's own legitimate attested publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures and the gate reported a clean pass over a workflow containing an unattested publish. The gate was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking.\n\nMeasured behaviourally across the fleet rather than by reading the sources, by importing each repository's own verify-release-publish-attestation.ts and auditing a fixture holding one attested publish plus one variable-routed unattested publish. 12 of 17 repositories returned failures=0: pm-graph, pm-starter, pm-csv, pm-context, pm-web, pm-presets, pm-ts-starter, pm-todos, pm-linear, pm-brief, pm-github and pm-slack-standup. 5 already caught it: pm-jira, pm-beads, pm-ops, pm-slack and pm-gantt-chart.\n\nFix: shellScalars accepts a third alternative for an unquoted single-word value, matching what pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is unchanged, so a value that would change how its line parses is still never inlined.\n\nThe regression test asserts the audit-level property rather than the map contents, so it fails for the reason the gate exists: with the fix reverted it reports failures=0 where 1 is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the old behaviour as intentional and was updated to match, exactly as pm-jira updated it.\n\nThis is the fifth time this class of bypass has been fixed in one vendored copy while the others stayed exposed. The root cause is that the verifier is vendored into every package instead of being consumed from pm-ops; that is tracked in the companion repository as pm-cli-website-bunt."}]}],"before_hash":"137ded4c5cea997218e47efc0dc67df526fe452165dd504d1689238fe1ab9dea","after_hash":"46ff051dfbbb099d93a90805a1dc80ea934a96bdab11383f7ed7e45e7b9d1f64","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} diff --git a/.agents/pm/issues/pm-github-tko1.toon b/.agents/pm/issues/pm-github-tko1.toon new file mode 100644 index 0000000..45bf084 --- /dev/null +++ b/.agents/pm/issues/pm-github-tko1.toon @@ -0,0 +1,13 @@ +id: pm-github-tko1 +title: The publish-attestation gate misses a publish routed through an unquoted shell variable +description: "" +type: Issue +status: open +priority: 1 +tags: [] +created_at: "2026-08-28T22:09:55.294Z" +updated_at: "2026-08-28T22:09:57.974Z" +author: claude +notes[1]{created_at,author,text}: + "2026-08-28T22:09:57.888Z",codex,"author=claude\ncreated_at=now\ntext=shellScalars indexed only quoted assignments, so a bare NPM=npm was never recorded. expandScalars then left $NPM unresolved, tokenizeCommands saw a program named \"$NPM\" rather than npm, and the line was not recognised as a publish at all. Because the workflow's own legitimate attested publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures and the gate reported a clean pass over a workflow containing an unattested publish. The gate was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking.\n\nMeasured behaviourally across the fleet rather than by reading the sources, by importing each repository's own verify-release-publish-attestation.ts and auditing a fixture holding one attested publish plus one variable-routed unattested publish. 12 of 17 repositories returned failures=0: pm-graph, pm-starter, pm-csv, pm-context, pm-web, pm-presets, pm-ts-starter, pm-todos, pm-linear, pm-brief, pm-github and pm-slack-standup. 5 already caught it: pm-jira, pm-beads, pm-ops, pm-slack and pm-gantt-chart.\n\nFix: shellScalars accepts a third alternative for an unquoted single-word value, matching what pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is unchanged, so a value that would change how its line parses is still never inlined.\n\nThe regression test asserts the audit-level property rather than the map contents, so it fails for the reason the gate exists: with the fix reverted it reports failures=0 where 1 is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the old behaviour as intentional and was updated to match, exactly as pm-jira updated it.\n\nThis is the fifth time this class of bypass has been fixed in one vendored copy while the others stayed exposed. The root cause is that the verifier is vendored into every package instead of being consumed from pm-ops; that is tracked in the companion repository as pm-cli-website-bunt." +body: "" diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index e450360..7165730 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -567,10 +567,10 @@ export function bashArrays(text: string): Map { */ export function shellScalars(text: string): Map { const scalars = new Map(); - for (const match of text.matchAll(/(?:^|[\s;&|])([A-Za-z_][A-Za-z0-9_]*)=(?:"([^"\n]*)"|'([^'\n]*)')/g)) { - // The alternation guarantees exactly one of the two value groups matched, - // so there is no third case to fall back to. - const value = match[2] ?? match[3]!; + for (const match of text.matchAll(/(?:^|[\s;&|])([A-Za-z_][A-Za-z0-9_]*)=(?:"([^"\n]*)"|'([^'\n]*)'|([^\s;&|"'`$()]+))/g)) { + // The alternation guarantees exactly one of the three value groups matched, + // so there is no fourth case to fall back to. + const value = match[2] ?? match[3] ?? match[4]!; // Only a plain literal is inlined. A value carrying a substitution, a // backtick, or a quote of its own changes how the line it lands in parses: // inlining `pkg_name="$(node -p …)"` injects an unbalanced parenthesis into diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index b6ad38d..81bfd94 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -641,7 +641,7 @@ test("a command held in a scalar is expanded, so the assignment is where the pub const scalars = shellScalars('CMD="npm publish"\nOTHER=\'npm publish --provenance\'\nBARE=npm\n'); assert.equal(scalars.get("CMD"), "npm publish"); assert.equal(scalars.get("OTHER"), "npm publish --provenance"); - assert.equal(scalars.get("BARE"), undefined, "an unquoted value cannot hold a command"); + assert.equal(scalars.get("BARE"), "npm", "an unquoted single-word value can hold a command name"); assert.equal(expandScalars("$CMD", scalars), "npm publish"); assert.equal(expandScalars("${CMD}", scalars), "npm publish"); assert.equal(expandScalars("$UNKNOWN", scalars), "$UNKNOWN", "an unknown name is left in place, not erased"); @@ -775,3 +775,22 @@ test("a substitution's quote state does not leak across its lines", () => { assert.ok(found.includes("npm publish"), "the publish inside the substitution is still found"); assert.ok(found.includes("npm publish --provenance"), "and the one after it is not swallowed"); }); + +test("a publish routed through an unquoted scalar is audited, not hidden by an attested sibling", () => { + // `NPM=npm` was skipped because only quoted assignments were indexed, so + // `$NPM publish` resolved to nothing and was never recognised as a publish. + // The workflow's own legitimate publish then satisfied the non-vacuity check + // and the whole audit reported a clean pass -- the gate was blind rather than + // wrong, which is the failure mode that gets a gate trusted while it is not + // looking. Only the variable-routed invocation may fail here. + const result = auditPublishAttestation([{ + file: "release.yml", + text: [ + ` npm publish --access public ${ATTESTATION_FLAG}`, + " NPM=npm", + " $NPM publish --access public", + ].join("\n"), + }]); + assert.equal(result.failures.length, 1, "the variable-routed publish must be audited"); + assert.match(result.failures[0]!, /does not enable --provenance/); +}); From 4a080c86648a808a0b51de72e95229a3ee07cffe Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 01:51:09 +0200 Subject: [PATCH 02/15] fix(scan): take a scalar only from a line that is exactly one literal assignment Scalar assignments were read out of the raw file text by a regex that matched anywhere a name-equals-value shape appeared. That indexed seven kinds of binding the shell never makes, and four of them let an unattested publish borrow a flag and pass the gate: # FLAG=--provenance a comment, inlined into a later command # CMD="npm publish" the same, and it pre-dates unquoted support echo "config NPM=npm" a name inside a quoted argument FLAG=--provenance some-command command-scoped, not kept after that command $(FLAG=--provenance) bound in a subshell the outer shell never sees NPM=npm$SUFFIX truncated to a prefix the literal guard accepts NPM=npm$(printf foo) the same, with the substitution consumed first A name is now taken only from a line that is EXACTLY one assignment carrying a fully literal value, anchored at both ends. Anchoring is what closes the truncation pair: a value that does not reach the end of the line is not the value, so a prefix can never be mistaken for the whole. Requiring the line to hold nothing else closes the comment, argument, command-scoped and subshell cases in one rule rather than four special cases. Unquoted single-word values are indexed, which is what makes a variable-routed publish visible at all: `NPM=npm` followed by `$NPM publish` previously resolved to nothing, was not recognised as a publish, and left the workflow's own attested publish to satisfy the non-vacuity guard, so the audit reported a clean pass over an unattested publish. Escapes are honoured, so `NPM=npm\ publish` still holds a command in one word. The existing refusal of any value that carries a substitution, backtick, quote or parenthesis is unchanged, and now applies after unescaping. Every case above is asserted at the audit level, not just against the scalar map, so the tests fail for the reason the gate exists rather than for the shape of an intermediate value. Both new cases were observed failing against the previous implementation and passing after. The behaviour was verified in each of the seventeen packages carrying this file by exercising sixteen properties against that package's own verifier, rather than by comparing the files, because the verifier is vendored per package and the copies have drifted. Found by Greptile (five security findings across two rounds) and Sourcery (quote-awareness, escapes and the stale docstring). --- .agents/pm/history/pm-github-tko1.jsonl | 2 + .agents/pm/issues/pm-github-tko1.toon | 6 +- scripts/shell-command-scan.ts | 60 +++++++++++++------ ...verify-release-publish-attestation.test.ts | 60 +++++++++++++++++++ 4 files changed, 109 insertions(+), 19 deletions(-) diff --git a/.agents/pm/history/pm-github-tko1.jsonl b/.agents/pm/history/pm-github-tko1.jsonl index 25ce8e7..6313b57 100644 --- a/.agents/pm/history/pm-github-tko1.jsonl +++ b/.agents/pm/history/pm-github-tko1.jsonl @@ -1,2 +1,4 @@ {"ts":"2026-08-28T22:09:55.294Z","author":"claude","author_source":"asserted","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"create","patch":[{"op":"add","path":"/metadata/id","value":"pm-github-tko1"},{"op":"add","path":"/metadata/title","value":"The publish-attestation gate misses a publish routed through an unquoted shell variable"},{"op":"add","path":"/metadata/description","value":""},{"op":"add","path":"/metadata/type","value":"Issue"},{"op":"add","path":"/metadata/status","value":"open"},{"op":"add","path":"/metadata/priority","value":1},{"op":"add","path":"/metadata/tags","value":[]},{"op":"add","path":"/metadata/created_at","value":"2026-08-28T22:09:55.294Z"},{"op":"add","path":"/metadata/updated_at","value":"2026-08-28T22:09:55.294Z"},{"op":"add","path":"/metadata/author","value":"claude"}],"before_hash":"3cc22dff72be7b14824654a7a64ea62b04799939b2fee54c1b5f52ca60bf6df0","after_hash":"137ded4c5cea997218e47efc0dc67df526fe452165dd504d1689238fe1ab9dea","item_hash_version":2,"message":"","context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} {"ts":"2026-08-28T22:09:57.974Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:09:57.974Z"},{"op":"add","path":"/metadata/notes","value":[{"created_at":"2026-08-28T22:09:57.888Z","author":"codex","text":"author=claude\ncreated_at=now\ntext=shellScalars indexed only quoted assignments, so a bare NPM=npm was never recorded. expandScalars then left $NPM unresolved, tokenizeCommands saw a program named \"$NPM\" rather than npm, and the line was not recognised as a publish at all. Because the workflow's own legitimate attested publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures and the gate reported a clean pass over a workflow containing an unattested publish. The gate was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking.\n\nMeasured behaviourally across the fleet rather than by reading the sources, by importing each repository's own verify-release-publish-attestation.ts and auditing a fixture holding one attested publish plus one variable-routed unattested publish. 12 of 17 repositories returned failures=0: pm-graph, pm-starter, pm-csv, pm-context, pm-web, pm-presets, pm-ts-starter, pm-todos, pm-linear, pm-brief, pm-github and pm-slack-standup. 5 already caught it: pm-jira, pm-beads, pm-ops, pm-slack and pm-gantt-chart.\n\nFix: shellScalars accepts a third alternative for an unquoted single-word value, matching what pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is unchanged, so a value that would change how its line parses is still never inlined.\n\nThe regression test asserts the audit-level property rather than the map contents, so it fails for the reason the gate exists: with the fix reverted it reports failures=0 where 1 is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the old behaviour as intentional and was updated to match, exactly as pm-jira updated it.\n\nThis is the fifth time this class of bypass has been fixed in one vendored copy while the others stayed exposed. The root cause is that the verifier is vendored into every package instead of being consumed from pm-ops; that is tracked in the companion repository as pm-cli-website-bunt."}]}],"before_hash":"137ded4c5cea997218e47efc0dc67df526fe452165dd504d1689238fe1ab9dea","after_hash":"46ff051dfbbb099d93a90805a1dc80ea934a96bdab11383f7ed7e45e7b9d1f64","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} +{"ts":"2026-08-28T22:36:31.424Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"add","path":"/metadata/notes/1","value":{"created_at":"2026-08-28T22:36:31.406Z","author":"claude","text":"Bot review on the first round (Greptile P1 x2, Sourcery x2) found that reading scalar assignments out of the raw text indexes bindings the shell never makes. All four were reproduced before fixing:\n1. A name defined only in a COMMENT was indexed. `# FLAG=--provenance` followed by `npm publish --access public $FLAG` audited as attested and returned zero failures: an unattested publish borrowed its flag from a comment and passed the gate. This is a bypass, and it pre-dates the unquoted-value change because the quoted form (`# CMD=\"npm publish\"`) was indexed the same way.\n2. A name inside a quoted argument was indexed. `echo \"config NPM=npm\"` bound NPM, so a later literal `$NPM publish` was audited as an npm publish the shell never runs.\n3. An unquoted value continuing into an expansion was truncated to its prefix. `NPM=npm$SUFFIX` bound NPM to `npm`; the literal guard then saw no `$` because the `$` had been excluded from the capture, so a value the scan cannot resolve was treated as one it could.\n4. The docstring still said only quoted values are indexed.\nFixed by reading assignments from tokenizeCommands instead of from raw text. The tokeniser has already dropped comments and resolved quoting, so a word is only treated as an assignment where the shell would treat it as one; it keeps `$SUFFIX` inside the word, so the literal guard sees the whole value rather than a prefix; and only a command-leading word can assign, so `npm publish FOO=bar` is an argument rather than a binding.\nOne consequence found by the existing suite rather than by a bot: the tokeniser consumes a substitution instead of keeping its text, so `SUBST=\"$(node -p 1)\"` arrives as an empty value that the literal guard would accept. Inlining it would ERASE `$SUBST` from the command, turning \"not understood\" into \"carries no flags\" — the exact false-pass this module already warns about. Empty values are therefore never indexed.\nBoth new tests were observed failing against the previous implementation and passing after. Verified behaviourally in all 17 packages carrying this file: the comment bypass, the quoted-argument binding, the truncation and the substitution are all refused, while `NPM=npm`, `CMD=\"npm publish\"` and a leading `FOO=bar` still resolve."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:36:31.424Z"}],"before_hash":"46ff051dfbbb099d93a90805a1dc80ea934a96bdab11383f7ed7e45e7b9d1f64","after_hash":"983b44659c4464f6a200c04e1ae00d9f10e13ee7390fec720d4cf1a23f8b462f","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} +{"ts":"2026-08-28T22:57:20.281Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"add","path":"/metadata/notes/2","value":{"created_at":"2026-08-28T22:57:20.179Z","author":"claude","text":"Round 3. Greptile raised three further P1 security findings against the tokeniser-based implementation, and all three reproduced:\n1. Substitution suffix. `NPM=npm$(printf foo)` — the tokeniser consumes the substitution before the literal guard sees the value, so NPM was bound to the prefix `npm`. The scan then analysed a different command from the one the shell runs.\n2. Command-scoped assignment leaked file-wide. `FLAG=--provenance some-command` binds only for that one command, but it was stored in the file-global map, so a LATER `npm publish --access public $FLAG` was rewritten into an attested-looking publish. Audit returned zero failures: a false pass.\n3. Subshell escape. `$(FLAG=--provenance)` is a binding the outer shell never sees, but the recursive tokeniser returns the nested command and its assignment was promoted into the outer map. Also a false pass, also measured at zero failures.\nReplaced the rule rather than patching three more cases. A name is now taken only from a line that is EXACTLY one assignment carrying a fully literal value, anchored at both ends. Anchoring closes the truncation pair, because a value that does not reach the end of the line is not the value. Requiring the line to hold nothing else closes the comment, quoted-argument, command-scoped and subshell cases in one rule instead of four special cases. Escapes are honoured so `NPM=npm\\ publish` still holds a command in one word, which was a Sourcery finding, and the existing refusal of any value carrying a substitution, backtick, quote or parenthesis now applies after unescaping.\n`FOO=bar npm publish` is deliberately no longer indexed. It is command-scoped, and the publish on that same line is visible to the scan directly, so nothing is lost.\nVerified across all 17 packages carrying this file by exercising 16 properties against each package's own verifier rather than by comparing files. The two regression tests were observed failing against the previous implementation and passing after.\nOne finding was accepted as valid but deliberately left out of this change: scalars are still resolved from a file-wide map, so a later assignment can rewrite an earlier use. Raised by Sourcery, reproduced, and filed as companion item pm-cli-website-1j4o because it is a different defect needing position-aware resolution."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:57:20.281Z"}],"before_hash":"983b44659c4464f6a200c04e1ae00d9f10e13ee7390fec720d4cf1a23f8b462f","after_hash":"72ff127d052da809bc49cf94456efdf37dc8dda63cf544b26ee76ec30b2922d7","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} diff --git a/.agents/pm/issues/pm-github-tko1.toon b/.agents/pm/issues/pm-github-tko1.toon index 45bf084..9ab0ed4 100644 --- a/.agents/pm/issues/pm-github-tko1.toon +++ b/.agents/pm/issues/pm-github-tko1.toon @@ -6,8 +6,10 @@ status: open priority: 1 tags: [] created_at: "2026-08-28T22:09:55.294Z" -updated_at: "2026-08-28T22:09:57.974Z" +updated_at: "2026-08-28T22:57:20.281Z" author: claude -notes[1]{created_at,author,text}: +notes[3]{created_at,author,text}: "2026-08-28T22:09:57.888Z",codex,"author=claude\ncreated_at=now\ntext=shellScalars indexed only quoted assignments, so a bare NPM=npm was never recorded. expandScalars then left $NPM unresolved, tokenizeCommands saw a program named \"$NPM\" rather than npm, and the line was not recognised as a publish at all. Because the workflow's own legitimate attested publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures and the gate reported a clean pass over a workflow containing an unattested publish. The gate was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking.\n\nMeasured behaviourally across the fleet rather than by reading the sources, by importing each repository's own verify-release-publish-attestation.ts and auditing a fixture holding one attested publish plus one variable-routed unattested publish. 12 of 17 repositories returned failures=0: pm-graph, pm-starter, pm-csv, pm-context, pm-web, pm-presets, pm-ts-starter, pm-todos, pm-linear, pm-brief, pm-github and pm-slack-standup. 5 already caught it: pm-jira, pm-beads, pm-ops, pm-slack and pm-gantt-chart.\n\nFix: shellScalars accepts a third alternative for an unquoted single-word value, matching what pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is unchanged, so a value that would change how its line parses is still never inlined.\n\nThe regression test asserts the audit-level property rather than the map contents, so it fails for the reason the gate exists: with the fix reverted it reports failures=0 where 1 is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the old behaviour as intentional and was updated to match, exactly as pm-jira updated it.\n\nThis is the fifth time this class of bypass has been fixed in one vendored copy while the others stayed exposed. The root cause is that the verifier is vendored into every package instead of being consumed from pm-ops; that is tracked in the companion repository as pm-cli-website-bunt." + "2026-08-28T22:36:31.406Z",claude,"Bot review on the first round (Greptile P1 x2, Sourcery x2) found that reading scalar assignments out of the raw text indexes bindings the shell never makes. All four were reproduced before fixing:\n1. A name defined only in a COMMENT was indexed. `# FLAG=--provenance` followed by `npm publish --access public $FLAG` audited as attested and returned zero failures: an unattested publish borrowed its flag from a comment and passed the gate. This is a bypass, and it pre-dates the unquoted-value change because the quoted form (`# CMD=\"npm publish\"`) was indexed the same way.\n2. A name inside a quoted argument was indexed. `echo \"config NPM=npm\"` bound NPM, so a later literal `$NPM publish` was audited as an npm publish the shell never runs.\n3. An unquoted value continuing into an expansion was truncated to its prefix. `NPM=npm$SUFFIX` bound NPM to `npm`; the literal guard then saw no `$` because the `$` had been excluded from the capture, so a value the scan cannot resolve was treated as one it could.\n4. The docstring still said only quoted values are indexed.\nFixed by reading assignments from tokenizeCommands instead of from raw text. The tokeniser has already dropped comments and resolved quoting, so a word is only treated as an assignment where the shell would treat it as one; it keeps `$SUFFIX` inside the word, so the literal guard sees the whole value rather than a prefix; and only a command-leading word can assign, so `npm publish FOO=bar` is an argument rather than a binding.\nOne consequence found by the existing suite rather than by a bot: the tokeniser consumes a substitution instead of keeping its text, so `SUBST=\"$(node -p 1)\"` arrives as an empty value that the literal guard would accept. Inlining it would ERASE `$SUBST` from the command, turning \"not understood\" into \"carries no flags\" — the exact false-pass this module already warns about. Empty values are therefore never indexed.\nBoth new tests were observed failing against the previous implementation and passing after. Verified behaviourally in all 17 packages carrying this file: the comment bypass, the quoted-argument binding, the truncation and the substitution are all refused, while `NPM=npm`, `CMD=\"npm publish\"` and a leading `FOO=bar` still resolve." + "2026-08-28T22:57:20.179Z",claude,"Round 3. Greptile raised three further P1 security findings against the tokeniser-based implementation, and all three reproduced:\n1. Substitution suffix. `NPM=npm$(printf foo)` — the tokeniser consumes the substitution before the literal guard sees the value, so NPM was bound to the prefix `npm`. The scan then analysed a different command from the one the shell runs.\n2. Command-scoped assignment leaked file-wide. `FLAG=--provenance some-command` binds only for that one command, but it was stored in the file-global map, so a LATER `npm publish --access public $FLAG` was rewritten into an attested-looking publish. Audit returned zero failures: a false pass.\n3. Subshell escape. `$(FLAG=--provenance)` is a binding the outer shell never sees, but the recursive tokeniser returns the nested command and its assignment was promoted into the outer map. Also a false pass, also measured at zero failures.\nReplaced the rule rather than patching three more cases. A name is now taken only from a line that is EXACTLY one assignment carrying a fully literal value, anchored at both ends. Anchoring closes the truncation pair, because a value that does not reach the end of the line is not the value. Requiring the line to hold nothing else closes the comment, quoted-argument, command-scoped and subshell cases in one rule instead of four special cases. Escapes are honoured so `NPM=npm\\ publish` still holds a command in one word, which was a Sourcery finding, and the existing refusal of any value carrying a substitution, backtick, quote or parenthesis now applies after unescaping.\n`FOO=bar npm publish` is deliberately no longer indexed. It is command-scoped, and the publish on that same line is visible to the scan directly, so nothing is lost.\nVerified across all 17 packages carrying this file by exercising 16 properties against each package's own verifier rather than by comparing files. The two regression tests were observed failing against the previous implementation and passing after.\nOne finding was accepted as valid but deliberately left out of this change: scalars are still resolved from a file-wide map, so a later assignment can rewrite an earlier use. Raised by Sourcery, reproduced, and filed as companion item pm-cli-website-1j4o because it is a different defect needing position-aware resolution." body: "" diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 7165730..ead5560 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -550,35 +550,61 @@ export function bashArrays(text: string): Map { return arrays; } +/** A line opening with one assignment of a fully literal value, ending there or at a `;`. */ +const STANDALONE_ASSIGNMENT = + /^[ \t]*([A-Za-z_][A-Za-z0-9_]*)=(?:"((?:\\.|[^"\\$`])*)"|'([^']*)'|((?:\\.|[^\s;&|"'`$()\\])+))[ \t]*(?:;|$)/; + /** * Index scalar assignments so a command held in a variable can be audited. * * `CMD="npm publish"` followed by `$CMD` runs a publish that no scan of the * invocation line can see, because the invocation line contains no publish. The - * assignment is where the command actually is. - * - * Only literal single- or double-quoted values are indexed. An unquoted value - * cannot hold a space and so cannot hold a command, and a value built from - * other variables is not resolvable without evaluating the script, which this - * module deliberately does not do. + * assignment is where the command actually is. `NPM=npm` followed by + * `$NPM publish` hides one the same way, so unquoted values are indexed too. + * + * A name is taken only where a line OPENS with one assignment carrying a fully + * literal value and holds nothing else before its end or a `;`. `NPM=npm; cmd` + * therefore binds, because the semicolon ends the assignment and the shell keeps + * it afterwards, while `NPM=npm cmd` does not, because that binding lasts only + * for the command it precedes. Requiring the line to OPEN with the assignment is + * what keeps a `;` inside a comment from exposing one. That single rule keeps + * the scan from inventing + * bindings the shell never makes, each of which let an unattested publish + * borrow a flag and pass the gate: + * + * - `# FLAG=--provenance` is a comment, and a comment is not a line that is + * only an assignment. + * - `echo "config NPM=npm"` is a command with an argument, not an assignment. + * - `FLAG=--provenance some-command` binds only for that one command; the shell + * does not keep it afterwards, so neither does this map. + * - `$(FLAG=--provenance)` binds inside a subshell that the outer shell never + * sees. + * - `NPM=npm$SUFFIX` and `NPM=npm$(printf foo)` are not literal. The value must + * match to the end of the line, so a prefix is never mistaken for the whole + * value -- the mistake that let a scan analyse a different command from the + * one the shell runs. + * + * Escapes are honoured, so `NPM=npm\\ publish` is one word holding a command. + * A value that still carries a substitution, backtick, quote or parenthesis + * after unescaping is refused: inlining `pkg_name="$(node -p …)"` injects an + * unbalanced parenthesis into an unrelated command, and the scan then reports + * invocations that are not there while losing the one that is -- a false + * verdict in both directions, which is worse than not resolving the variable. * * @param text - File contents with continuations already joined. * @returns Variable name mapped to the literal text it holds. */ export function shellScalars(text: string): Map { const scalars = new Map(); - for (const match of text.matchAll(/(?:^|[\s;&|])([A-Za-z_][A-Za-z0-9_]*)=(?:"([^"\n]*)"|'([^'\n]*)'|([^\s;&|"'`$()]+))/g)) { - // The alternation guarantees exactly one of the three value groups matched, - // so there is no fourth case to fall back to. - const value = match[2] ?? match[3] ?? match[4]!; - // Only a plain literal is inlined. A value carrying a substitution, a - // backtick, or a quote of its own changes how the line it lands in parses: - // inlining `pkg_name="$(node -p …)"` injects an unbalanced parenthesis into - // an unrelated command, and the scan then reports invocations that are not - // there while losing the one that is. That is a false verdict in both - // directions, which is worse than not resolving the variable at all. + for (const line of text.split("\n")) { + const assignment = STANDALONE_ASSIGNMENT.exec(line); + if (assignment === null) continue; + // Exactly one of the three value alternatives matches, so the last is the + // only case left rather than a fallback that could be undefined. + const raw = assignment[2] ?? assignment[3] ?? assignment[4]!; + const value = raw.replace(/\\(.)/g, "$1"); if (/[$`"'()]/.test(value)) continue; - scalars.set(match[1]!, value); + scalars.set(assignment[1]!, value); } return scalars; } diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 81bfd94..c40338a 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -794,3 +794,63 @@ test("a publish routed through an unquoted scalar is audited, not hidden by an a assert.equal(result.failures.length, 1, "the variable-routed publish must be audited"); assert.match(result.failures[0]!, /does not enable --provenance/); }); + +test("an assignment the shell never makes is not indexed", () => { + // Scalars used to be read straight out of the raw text, which indexed three + // things the shell does not assign. The middle one is a gate bypass: a name + // defined only in a COMMENT was inlined into a later command, so an + // unattested publish borrowed `--provenance` from a comment and passed. + assert.equal(shellScalars("# FLAG=--provenance\nnpm publish $FLAG\n").get("FLAG"), undefined, + "a name in a comment is not an assignment"); + assert.equal(shellScalars('# CMD="npm publish"\n').get("CMD"), undefined, + "quoting it in a comment does not make it an assignment either"); + assert.equal(shellScalars('echo "config NPM=npm"\n').get("NPM"), undefined, + "a name inside a quoted argument is not an assignment"); + assert.equal(shellScalars("NPM=npm$SUFFIX\n").get("NPM"), undefined, + "a value continuing into an expansion is not a literal, and must not be indexed by its prefix"); + assert.equal(shellScalars('"NPM=npm" publish\n').get("NPM"), undefined, + "quoting the whole word makes it a command name, not a binding"); + + // The bypass, end to end: without the fix this audit returns no failures. + const result = auditPublishAttestation([{ + file: "release.yml", + text: [ + " # FLAG=--provenance", + " npm publish --access public $FLAG", + ].join("\n"), + }]); + assert.equal(result.failures.length, 1, "a publish flagged only from a comment is unattested"); + assert.match(result.failures[0]!, /does not enable --provenance/); +}); + +test("a scalar is taken only from a line that is exactly one literal assignment", () => { + assert.equal(shellScalars("NPM=npm\n").get("NPM"), "npm"); + assert.equal(shellScalars('CMD="npm publish"\n').get("CMD"), "npm publish"); + assert.equal(shellScalars("OTHER='npm publish --provenance'\n").get("OTHER"), "npm publish --provenance"); + assert.equal(shellScalars("NPM=npm\\ publish\n").get("NPM"), "npm publish", + "an escape is honoured, so one word can still hold a command"); + assert.equal(shellScalars('NPM=npm; "$NPM" publish\n').get("NPM"), "npm", + "a semicolon ends the assignment, and the shell keeps the binding after it"); + assert.equal(shellScalars("# a; FLAG=--provenance\n").get("FLAG"), undefined, + "a semicolon inside a comment does not expose an assignment"); + + // A command-scoped assignment binds only for the command it precedes; the + // shell does not keep it afterwards, so neither may this map. Storing it + // rewrote a LATER unattested publish into an attested-looking one. + assert.equal(shellScalars("FLAG=--provenance some-command\n").get("FLAG"), undefined, + "a temporary assignment does not outlive its command"); + assert.equal(shellScalars("$(FLAG=--provenance)\n").get("FLAG"), undefined, + "a binding made inside a subshell is not visible to the outer shell"); + assert.equal(shellScalars("NPM=npm$(printf foo)\n").get("NPM"), undefined, + "a literal prefix in front of a substitution is not the value"); + + // Both leaks were false passes end to end, not merely wrong map entries. + for (const text of [ + [" FLAG=--provenance some-command", " npm publish --access public $FLAG"], + [" $(FLAG=--provenance)", " npm publish --access public $FLAG"], + ]) { + const result = auditPublishAttestation([{ file: "release.yml", text: text.join("\n") }]); + assert.equal(result.failures.length, 1, `a publish flagged only by ${text[0]!.trim()} is unattested`); + assert.match(result.failures[0]!, /does not enable --provenance/); + } +}); From 3170303ee3f8e51b155efc72da7c924c0bea3f75 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 02:44:44 +0200 Subject: [PATCH 03/15] fix(scan): recognise a read-write redirection, and stop refusing persistent assignments Two more shapes let an unattested publish through, both found by review on the previous round and both measured before being fixed. `<> /dev/null npm publish --access public` was not audited at all. `<>` is one operator, not `<` followed by `>`, and unnamed it was read as a JOINED redirection that consumes no target -- so `/dev/null` became the command word and the publish after it was never seen. An attested publish elsewhere then satisfied the non-vacuity guard and the whole audit reported clean. The previous round's assignment rule was too strict, which fails the same way. `export NPM=npm`, `NPM=npm # explanation` and a CRLF-terminated assignment are all persistent bindings, and refusing them left `$NPM` unresolved, so the publish that used it went unrecognised -- an unattested publish passing because the scan was too strict rather than too loose. All three are accepted again. Single-quoted values are no longer unescaped. The shell keeps a backslash inside single quotes, so `CMD='npm publish \--provenance'` was being read as carrying `--provenance` when the shell runs something else. Only the double-quoted and unquoted forms process escapes now, which is what the shell does. Each fix has an audit-level regression test, and each was observed failing against the previous implementation and passing after. The behaviour is verified in every package carrying this file rather than by comparing the files. --- scripts/shell-command-scan.ts | 23 ++++++++++++---- ...verify-release-publish-attestation.test.ts | 27 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index ead5560..b28de80 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -343,14 +343,18 @@ export function tokenizeCommands(text: string, depth = 0): ShellCommand[] { * `> /dev/null npm publish` runs npm. A scan that reads words in order sees `>` * as the program and audits nothing. The forms accepted here are the ones a * workflow actually writes: the plain operators, a file-descriptor prefix - * (`2>`, `2>>`), and the duplicating forms (`>&`, `2>&1`, `&>`). + * (`2>`, `2>>`), the duplicating forms (`>&`, `2>&1`, `&>`), and the read-write + * form `<>`. `<>` has to be named explicitly: it is not `<` followed by `>`, so + * without it the operator was read as a joined redirection that consumes no + * target, its target `/dev/null` became the command word, and the real + * `npm publish` after it was never audited. * * @param token - One command word. * @returns True when the word is a redirection operator. */ function isRedirection(token: ShellToken): boolean { if (token.startsQuoted) return false; - return /^(?:[0-9]*(?:>>?|<>?)$/.test(token.value); + return /^(?:[0-9]*(?:>>?|<>|<>?)$/.test(token.value); } /** @@ -552,7 +556,7 @@ export function bashArrays(text: string): Map { /** A line opening with one assignment of a fully literal value, ending there or at a `;`. */ const STANDALONE_ASSIGNMENT = - /^[ \t]*([A-Za-z_][A-Za-z0-9_]*)=(?:"((?:\\.|[^"\\$`])*)"|'([^']*)'|((?:\\.|[^\s;&|"'`$()\\])+))[ \t]*(?:;|$)/; + /^[ \t]*(?:export[ \t]+)?([A-Za-z_][A-Za-z0-9_]*)=(?:"((?:\\.|[^"\\$`])*)"|'([^']*)'|((?:\\.|[^\s;&|"'`$()\\])+))[ \t]*(?:[;#]|\r?$)/; /** * Index scalar assignments so a command held in a variable can be audited. @@ -584,7 +588,13 @@ const STANDALONE_ASSIGNMENT = * value -- the mistake that let a scan analyse a different command from the * one the shell runs. * - * Escapes are honoured, so `NPM=npm\\ publish` is one word holding a command. + * `export NPM=npm`, a trailing `# comment` and a CRLF line ending are all still + * assignments: refusing them left `$NPM` unresolved, and an attested publish + * elsewhere in the file then satisfied the non-vacuity guard, so being too + * strict here passes an unattested publish just as being too loose does. + * + * Escapes are honoured outside single quotes, so `NPM=npm\\ publish` is one word + * holding a command while `CMD='"'"'a\\b'"'"' keeps its backslash as the shell does. * A value that still carries a substitution, backtick, quote or parenthesis * after unescaping is refused: inlining `pkg_name="$(node -p …)"` injects an * unbalanced parenthesis into an unrelated command, and the scan then reports @@ -602,7 +612,10 @@ export function shellScalars(text: string): Map { // Exactly one of the three value alternatives matches, so the last is the // only case left rather than a fallback that could be undefined. const raw = assignment[2] ?? assignment[3] ?? assignment[4]!; - const value = raw.replace(/\\(.)/g, "$1"); + // Single quotes make a backslash literal, so only the other two forms are + // unescaped. Unescaping a single-quoted value turned `'npm publish + // \\--provenance'` into an attested-looking command the shell never runs. + const value = assignment[3] === undefined ? raw.replace(/\\(.)/g, "$1") : raw; if (/[$`"'()]/.test(value)) continue; scalars.set(assignment[1]!, value); } diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index c40338a..c46aca0 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -831,6 +831,17 @@ test("a scalar is taken only from a line that is exactly one literal assignment" "an escape is honoured, so one word can still hold a command"); assert.equal(shellScalars('NPM=npm; "$NPM" publish\n').get("NPM"), "npm", "a semicolon ends the assignment, and the shell keeps the binding after it"); + assert.equal(shellScalars("export NPM=npm\n").get("NPM"), "npm", + "export still declares a persistent binding"); + assert.equal(shellScalars("NPM=npm # explanation\n").get("NPM"), "npm", + "a trailing comment does not stop the line being an assignment"); + assert.equal(shellScalars("NPM=npm\r\n").get("NPM"), "npm", + "a CRLF line ending does not hide the assignment"); + // Refusing these left `$NPM` unresolved, and an attested publish elsewhere in + // the file then satisfied the non-vacuity guard -- so being too strict passes + // an unattested publish exactly as being too loose does. + assert.equal(shellScalars("CMD='npm publish \\--provenance'\n").get("CMD"), "npm publish \\--provenance", + "single quotes make a backslash literal, so the value is not unescaped"); assert.equal(shellScalars("# a; FLAG=--provenance\n").get("FLAG"), undefined, "a semicolon inside a comment does not expose an assignment"); @@ -854,3 +865,19 @@ test("a scalar is taken only from a line that is exactly one literal assignment" assert.match(result.failures[0]!, /does not enable --provenance/); } }); +test("a read-write redirection does not turn its target into the command", () => { + // `<>` is one operator, not `<` followed by `>`. Unnamed, it was read as a + // joined redirection that consumes no target, so `/dev/null` became the + // command word and the real publish after it was never audited -- while an + // attested publish elsewhere satisfied the non-vacuity guard, so the whole + // audit reported clean. + const result = auditPublishAttestation([{ + file: "release.yml", + text: [ + ` npm publish --access public ${ATTESTATION_FLAG}`, + " <> /dev/null npm publish --access public", + ].join("\n"), + }]); + assert.equal(result.failures.length, 1, "the redirected publish must still be audited"); + assert.match(result.failures[0]!, /does not enable --provenance/); +}); From efca4fa49017331f0d7bc643034d6e5589f360bd Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:14:08 +0200 Subject: [PATCH 04/15] fix(scan): resolve shell scalars at source position Track persistent literal bindings in file order so later assignments cannot rewrite earlier publish commands. Ignore assignment-shaped heredoc data, support assignment-only lists, and cover each bypass with end-to-end attestation regressions. --- .agents/pm/history/pm-github-tko1.jsonl | 6 ++ .agents/pm/issues/pm-github-tko1.toon | 10 ++- CHANGELOG.md | 1 + scripts/shell-command-scan.ts | 80 ++++++++++++++----- scripts/verify-release-publish-attestation.ts | 10 +-- ...verify-release-publish-attestation.test.ts | 36 +++++++++ 6 files changed, 116 insertions(+), 27 deletions(-) diff --git a/.agents/pm/history/pm-github-tko1.jsonl b/.agents/pm/history/pm-github-tko1.jsonl index 6313b57..d74c05d 100644 --- a/.agents/pm/history/pm-github-tko1.jsonl +++ b/.agents/pm/history/pm-github-tko1.jsonl @@ -2,3 +2,9 @@ {"ts":"2026-08-28T22:09:57.974Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:09:57.974Z"},{"op":"add","path":"/metadata/notes","value":[{"created_at":"2026-08-28T22:09:57.888Z","author":"codex","text":"author=claude\ncreated_at=now\ntext=shellScalars indexed only quoted assignments, so a bare NPM=npm was never recorded. expandScalars then left $NPM unresolved, tokenizeCommands saw a program named \"$NPM\" rather than npm, and the line was not recognised as a publish at all. Because the workflow's own legitimate attested publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures and the gate reported a clean pass over a workflow containing an unattested publish. The gate was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking.\n\nMeasured behaviourally across the fleet rather than by reading the sources, by importing each repository's own verify-release-publish-attestation.ts and auditing a fixture holding one attested publish plus one variable-routed unattested publish. 12 of 17 repositories returned failures=0: pm-graph, pm-starter, pm-csv, pm-context, pm-web, pm-presets, pm-ts-starter, pm-todos, pm-linear, pm-brief, pm-github and pm-slack-standup. 5 already caught it: pm-jira, pm-beads, pm-ops, pm-slack and pm-gantt-chart.\n\nFix: shellScalars accepts a third alternative for an unquoted single-word value, matching what pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is unchanged, so a value that would change how its line parses is still never inlined.\n\nThe regression test asserts the audit-level property rather than the map contents, so it fails for the reason the gate exists: with the fix reverted it reports failures=0 where 1 is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the old behaviour as intentional and was updated to match, exactly as pm-jira updated it.\n\nThis is the fifth time this class of bypass has been fixed in one vendored copy while the others stayed exposed. The root cause is that the verifier is vendored into every package instead of being consumed from pm-ops; that is tracked in the companion repository as pm-cli-website-bunt."}]}],"before_hash":"137ded4c5cea997218e47efc0dc67df526fe452165dd504d1689238fe1ab9dea","after_hash":"46ff051dfbbb099d93a90805a1dc80ea934a96bdab11383f7ed7e45e7b9d1f64","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} {"ts":"2026-08-28T22:36:31.424Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"add","path":"/metadata/notes/1","value":{"created_at":"2026-08-28T22:36:31.406Z","author":"claude","text":"Bot review on the first round (Greptile P1 x2, Sourcery x2) found that reading scalar assignments out of the raw text indexes bindings the shell never makes. All four were reproduced before fixing:\n1. A name defined only in a COMMENT was indexed. `# FLAG=--provenance` followed by `npm publish --access public $FLAG` audited as attested and returned zero failures: an unattested publish borrowed its flag from a comment and passed the gate. This is a bypass, and it pre-dates the unquoted-value change because the quoted form (`# CMD=\"npm publish\"`) was indexed the same way.\n2. A name inside a quoted argument was indexed. `echo \"config NPM=npm\"` bound NPM, so a later literal `$NPM publish` was audited as an npm publish the shell never runs.\n3. An unquoted value continuing into an expansion was truncated to its prefix. `NPM=npm$SUFFIX` bound NPM to `npm`; the literal guard then saw no `$` because the `$` had been excluded from the capture, so a value the scan cannot resolve was treated as one it could.\n4. The docstring still said only quoted values are indexed.\nFixed by reading assignments from tokenizeCommands instead of from raw text. The tokeniser has already dropped comments and resolved quoting, so a word is only treated as an assignment where the shell would treat it as one; it keeps `$SUFFIX` inside the word, so the literal guard sees the whole value rather than a prefix; and only a command-leading word can assign, so `npm publish FOO=bar` is an argument rather than a binding.\nOne consequence found by the existing suite rather than by a bot: the tokeniser consumes a substitution instead of keeping its text, so `SUBST=\"$(node -p 1)\"` arrives as an empty value that the literal guard would accept. Inlining it would ERASE `$SUBST` from the command, turning \"not understood\" into \"carries no flags\" — the exact false-pass this module already warns about. Empty values are therefore never indexed.\nBoth new tests were observed failing against the previous implementation and passing after. Verified behaviourally in all 17 packages carrying this file: the comment bypass, the quoted-argument binding, the truncation and the substitution are all refused, while `NPM=npm`, `CMD=\"npm publish\"` and a leading `FOO=bar` still resolve."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:36:31.424Z"}],"before_hash":"46ff051dfbbb099d93a90805a1dc80ea934a96bdab11383f7ed7e45e7b9d1f64","after_hash":"983b44659c4464f6a200c04e1ae00d9f10e13ee7390fec720d4cf1a23f8b462f","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} {"ts":"2026-08-28T22:57:20.281Z","author":"codex","author_source":"configured","agent_harness":"claude-code","agent_instance":"f992699797bca763a9b53afd","agent_provenance":{"model":null,"effort":{"value":"xhigh","source":"environment"},"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"add","path":"/metadata/notes/2","value":{"created_at":"2026-08-28T22:57:20.179Z","author":"claude","text":"Round 3. Greptile raised three further P1 security findings against the tokeniser-based implementation, and all three reproduced:\n1. Substitution suffix. `NPM=npm$(printf foo)` — the tokeniser consumes the substitution before the literal guard sees the value, so NPM was bound to the prefix `npm`. The scan then analysed a different command from the one the shell runs.\n2. Command-scoped assignment leaked file-wide. `FLAG=--provenance some-command` binds only for that one command, but it was stored in the file-global map, so a LATER `npm publish --access public $FLAG` was rewritten into an attested-looking publish. Audit returned zero failures: a false pass.\n3. Subshell escape. `$(FLAG=--provenance)` is a binding the outer shell never sees, but the recursive tokeniser returns the nested command and its assignment was promoted into the outer map. Also a false pass, also measured at zero failures.\nReplaced the rule rather than patching three more cases. A name is now taken only from a line that is EXACTLY one assignment carrying a fully literal value, anchored at both ends. Anchoring closes the truncation pair, because a value that does not reach the end of the line is not the value. Requiring the line to hold nothing else closes the comment, quoted-argument, command-scoped and subshell cases in one rule instead of four special cases. Escapes are honoured so `NPM=npm\\ publish` still holds a command in one word, which was a Sourcery finding, and the existing refusal of any value carrying a substitution, backtick, quote or parenthesis now applies after unescaping.\n`FOO=bar npm publish` is deliberately no longer indexed. It is command-scoped, and the publish on that same line is visible to the scan directly, so nothing is lost.\nVerified across all 17 packages carrying this file by exercising 16 properties against each package's own verifier rather than by comparing files. The two regression tests were observed failing against the previous implementation and passing after.\nOne finding was accepted as valid but deliberately left out of this change: scalars are still resolved from a file-wide map, so a later assignment can rewrite an earlier use. Raised by Sourcery, reproduced, and filed as companion item pm-cli-website-1j4o because it is a different defect needing position-aware resolution."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-28T22:57:20.281Z"}],"before_hash":"983b44659c4464f6a200c04e1ae00d9f10e13ee7390fec720d4cf1a23f8b462f","after_hash":"72ff127d052da809bc49cf94456efdf37dc8dda63cf544b26ee76ec30b2922d7","item_hash_version":2,"context":{"agent_provenance_outcomes":{"model":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"},"version":{"status":"failed","reason":"resolver_failed","resolver":"claude_session_file","rule_version":"v1"}}}} +{"ts":"2026-08-29T04:11:56.170Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"gpt-5.6-sol","agent_model_source":"environment","agent_instance":"50ec9a4c6e15dbcecef5b53d","agent_provenance":{"model":{"value":"gpt-5.6-sol","source":"environment"},"effort":null,"role":{"value":"implementer","source":"argv"},"topic":null},"op":"claim","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-29T04:11:56.170Z"},{"op":"add","path":"/metadata/assignee","value":"pi-agent"},{"op":"add","path":"/metadata/claim_principal","value":"pi-agent"}],"before_hash":"72ff127d052da809bc49cf94456efdf37dc8dda63cf544b26ee76ec30b2922d7","after_hash":"ebc325f833c9eb464ed79c05a599125133d9dad407994c4b0f5942e78f8f2034","item_hash_version":2} +{"ts":"2026-08-29T04:11:56.621Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"gpt-5.6-sol","agent_model_source":"environment","agent_instance":"50ec9a4c6e15dbcecef5b53d","agent_provenance":{"model":{"value":"gpt-5.6-sol","source":"environment"},"effort":null,"role":{"value":"implementer","source":"argv"},"topic":null},"op":"update","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-29T04:11:56.621Z"},{"op":"replace","path":"/metadata/status","value":"in_progress"}],"before_hash":"ebc325f833c9eb464ed79c05a599125133d9dad407994c4b0f5942e78f8f2034","after_hash":"c57ea7420fe8c434488c1e934dd5a2c455ce643b6bf3128047775247a2d235cb","item_hash_version":2} +{"ts":"2026-08-29T04:11:57.118Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"gpt-5.6-sol","agent_model_source":"environment","agent_instance":"50ec9a4c6e15dbcecef5b53d","agent_provenance":{"model":{"value":"gpt-5.6-sol","source":"environment"},"effort":null,"role":null,"topic":null},"op":"comment_add","patch":[{"op":"replace","path":"/metadata/updated_at","value":"2026-08-29T04:11:57.118Z"},{"op":"add","path":"/metadata/comments","value":[{"created_at":"2026-08-29T04:11:57.118Z","author":"pi-agent","text":"PR #60 review follow-up: resolving scalar bindings by source position, excluding heredoc bodies, and supporting persistent assignment-only lists."}]}],"before_hash":"c57ea7420fe8c434488c1e934dd5a2c455ce643b6bf3128047775247a2d235cb","after_hash":"0b78b4f3cc951a73e3d4303f5d2af49745b2c3744712e439da3297bd8bf1b685","item_hash_version":2} +{"ts":"2026-08-29T04:13:43.866Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"gpt-5.6-sol","agent_model_source":"environment","agent_instance":"50ec9a4c6e15dbcecef5b53d","agent_provenance":{"model":{"value":"gpt-5.6-sol","source":"environment"},"effort":null,"role":null,"topic":null},"op":"comment_add","patch":[{"op":"add","path":"/metadata/comments/1","value":{"created_at":"2026-08-29T04:13:43.866Z","author":"pi-agent","text":"Implemented and verified: source-position scalar resolution prevents later reassignment from rewriting earlier commands; heredoc bodies are excluded; assignment-only lists retain each persistent literal binding. Full tests and coverage pass."}},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-29T04:13:43.866Z"}],"before_hash":"0b78b4f3cc951a73e3d4303f5d2af49745b2c3744712e439da3297bd8bf1b685","after_hash":"bf36e824195d8c9416287b922337386f0d2ef6db9beec545da679769b088074a","item_hash_version":2} +{"ts":"2026-08-29T04:13:44.483Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"gpt-5.6-sol","agent_model_source":"environment","agent_instance":"50ec9a4c6e15dbcecef5b53d","agent_provenance":{"model":{"value":"gpt-5.6-sol","source":"environment"},"effort":null,"role":{"value":"implementer","source":"argv"},"topic":null},"op":"close","patch":[{"op":"remove","path":"/metadata/assignee"},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-29T04:13:44.483Z"},{"op":"replace","path":"/metadata/status","value":"closed"},{"op":"add","path":"/metadata/closed_at","value":"2026-08-29T04:13:44.451Z"},{"op":"add","path":"/metadata/completed_at","value":"2026-08-29T04:13:44.451Z"},{"op":"add","path":"/metadata/close_reason","value":"PR #60 review findings fixed with end-to-end regressions; full suite, coverage, changelog, and strict health gates pass."}],"before_hash":"bf36e824195d8c9416287b922337386f0d2ef6db9beec545da679769b088074a","after_hash":"d376377b333789c762cffaed34ee96e65b764fa7935e672248eac4cdbdb97763","item_hash_version":2} +{"ts":"2026-08-29T04:13:45.084Z","author":"pi-agent","author_source":"asserted","agent_harness":"pi","agent_model":"gpt-5.6-sol","agent_model_source":"environment","agent_instance":"50ec9a4c6e15dbcecef5b53d","agent_provenance":{"model":{"value":"gpt-5.6-sol","source":"environment"},"effort":null,"role":{"value":"implementer","source":"argv"},"topic":null},"op":"release","patch":[{"op":"remove","path":"/metadata/claim_principal"},{"op":"replace","path":"/metadata/updated_at","value":"2026-08-29T04:13:45.084Z"}],"before_hash":"d376377b333789c762cffaed34ee96e65b764fa7935e672248eac4cdbdb97763","after_hash":"aefb17914d55a182f475b36a31467964ac99adbd578bf2451cd3b58b84ebf2b0","item_hash_version":2} diff --git a/.agents/pm/issues/pm-github-tko1.toon b/.agents/pm/issues/pm-github-tko1.toon index 9ab0ed4..ab6d464 100644 --- a/.agents/pm/issues/pm-github-tko1.toon +++ b/.agents/pm/issues/pm-github-tko1.toon @@ -2,14 +2,20 @@ id: pm-github-tko1 title: The publish-attestation gate misses a publish routed through an unquoted shell variable description: "" type: Issue -status: open +status: closed priority: 1 tags: [] created_at: "2026-08-28T22:09:55.294Z" -updated_at: "2026-08-28T22:57:20.281Z" +updated_at: "2026-08-29T04:13:45.084Z" +closed_at: "2026-08-29T04:13:44.451Z" +completed_at: "2026-08-29T04:13:44.451Z" author: claude +comments[2]{created_at,author,text}: + "2026-08-29T04:11:57.118Z",pi-agent,"PR #60 review follow-up: resolving scalar bindings by source position, excluding heredoc bodies, and supporting persistent assignment-only lists." + "2026-08-29T04:13:43.866Z",pi-agent,"Implemented and verified: source-position scalar resolution prevents later reassignment from rewriting earlier commands; heredoc bodies are excluded; assignment-only lists retain each persistent literal binding. Full tests and coverage pass." notes[3]{created_at,author,text}: "2026-08-28T22:09:57.888Z",codex,"author=claude\ncreated_at=now\ntext=shellScalars indexed only quoted assignments, so a bare NPM=npm was never recorded. expandScalars then left $NPM unresolved, tokenizeCommands saw a program named \"$NPM\" rather than npm, and the line was not recognised as a publish at all. Because the workflow's own legitimate attested publish still satisfied the non-vacuity guard, auditPublishAttestation returned zero failures and the gate reported a clean pass over a workflow containing an unattested publish. The gate was blind rather than wrong, which is the failure mode that keeps a gate trusted while it is not looking.\n\nMeasured behaviourally across the fleet rather than by reading the sources, by importing each repository's own verify-release-publish-attestation.ts and auditing a fixture holding one attested publish plus one variable-routed unattested publish. 12 of 17 repositories returned failures=0: pm-graph, pm-starter, pm-csv, pm-context, pm-web, pm-presets, pm-ts-starter, pm-todos, pm-linear, pm-brief, pm-github and pm-slack-standup. 5 already caught it: pm-jira, pm-beads, pm-ops, pm-slack and pm-gantt-chart.\n\nFix: shellScalars accepts a third alternative for an unquoted single-word value, matching what pm-jira already carries. The existing guard that refuses to inline a value containing a substitution, backtick, quote or parenthesis is unchanged, so a value that would change how its line parses is still never inlined.\n\nThe regression test asserts the audit-level property rather than the map contents, so it fails for the reason the gate exists: with the fix reverted it reports failures=0 where 1 is required. The neighbouring assertion that an unquoted value cannot hold a command encoded the old behaviour as intentional and was updated to match, exactly as pm-jira updated it.\n\nThis is the fifth time this class of bypass has been fixed in one vendored copy while the others stayed exposed. The root cause is that the verifier is vendored into every package instead of being consumed from pm-ops; that is tracked in the companion repository as pm-cli-website-bunt." "2026-08-28T22:36:31.406Z",claude,"Bot review on the first round (Greptile P1 x2, Sourcery x2) found that reading scalar assignments out of the raw text indexes bindings the shell never makes. All four were reproduced before fixing:\n1. A name defined only in a COMMENT was indexed. `# FLAG=--provenance` followed by `npm publish --access public $FLAG` audited as attested and returned zero failures: an unattested publish borrowed its flag from a comment and passed the gate. This is a bypass, and it pre-dates the unquoted-value change because the quoted form (`# CMD=\"npm publish\"`) was indexed the same way.\n2. A name inside a quoted argument was indexed. `echo \"config NPM=npm\"` bound NPM, so a later literal `$NPM publish` was audited as an npm publish the shell never runs.\n3. An unquoted value continuing into an expansion was truncated to its prefix. `NPM=npm$SUFFIX` bound NPM to `npm`; the literal guard then saw no `$` because the `$` had been excluded from the capture, so a value the scan cannot resolve was treated as one it could.\n4. The docstring still said only quoted values are indexed.\nFixed by reading assignments from tokenizeCommands instead of from raw text. The tokeniser has already dropped comments and resolved quoting, so a word is only treated as an assignment where the shell would treat it as one; it keeps `$SUFFIX` inside the word, so the literal guard sees the whole value rather than a prefix; and only a command-leading word can assign, so `npm publish FOO=bar` is an argument rather than a binding.\nOne consequence found by the existing suite rather than by a bot: the tokeniser consumes a substitution instead of keeping its text, so `SUBST=\"$(node -p 1)\"` arrives as an empty value that the literal guard would accept. Inlining it would ERASE `$SUBST` from the command, turning \"not understood\" into \"carries no flags\" — the exact false-pass this module already warns about. Empty values are therefore never indexed.\nBoth new tests were observed failing against the previous implementation and passing after. Verified behaviourally in all 17 packages carrying this file: the comment bypass, the quoted-argument binding, the truncation and the substitution are all refused, while `NPM=npm`, `CMD=\"npm publish\"` and a leading `FOO=bar` still resolve." "2026-08-28T22:57:20.179Z",claude,"Round 3. Greptile raised three further P1 security findings against the tokeniser-based implementation, and all three reproduced:\n1. Substitution suffix. `NPM=npm$(printf foo)` — the tokeniser consumes the substitution before the literal guard sees the value, so NPM was bound to the prefix `npm`. The scan then analysed a different command from the one the shell runs.\n2. Command-scoped assignment leaked file-wide. `FLAG=--provenance some-command` binds only for that one command, but it was stored in the file-global map, so a LATER `npm publish --access public $FLAG` was rewritten into an attested-looking publish. Audit returned zero failures: a false pass.\n3. Subshell escape. `$(FLAG=--provenance)` is a binding the outer shell never sees, but the recursive tokeniser returns the nested command and its assignment was promoted into the outer map. Also a false pass, also measured at zero failures.\nReplaced the rule rather than patching three more cases. A name is now taken only from a line that is EXACTLY one assignment carrying a fully literal value, anchored at both ends. Anchoring closes the truncation pair, because a value that does not reach the end of the line is not the value. Requiring the line to hold nothing else closes the comment, quoted-argument, command-scoped and subshell cases in one rule instead of four special cases. Escapes are honoured so `NPM=npm\\ publish` still holds a command in one word, which was a Sourcery finding, and the existing refusal of any value carrying a substitution, backtick, quote or parenthesis now applies after unescaping.\n`FOO=bar npm publish` is deliberately no longer indexed. It is command-scoped, and the publish on that same line is visible to the scan directly, so nothing is lost.\nVerified across all 17 packages carrying this file by exercising 16 properties against each package's own verifier rather than by comparing files. The two regression tests were observed failing against the previous implementation and passing after.\nOne finding was accepted as valid but deliberately left out of this change: scalars are still resolved from a file-wide map, so a later assignment can rewrite an earlier use. Raised by Sourcery, reproduced, and filed as companion item pm-cli-website-1j4o because it is a different defect needing position-aware resolution." +close_reason: "PR #60 review findings fixed with end-to-end regressions; full suite, coverage, changelog, and strict health gates pass." body: "" diff --git a/CHANGELOG.md b/CHANGELOG.md index 450bee9..e97d01a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed +- The publish-attestation gate misses a publish routed through an unquoted shell variable ([pm-github-tko1](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-tko1.toon)) - True round-trip GitHub sync: search provider, validate diagnostics, safe-by-default export, fix activation ([pm-github-9dqy](https://github.com/unbraind/pm-github/blob/main/.agents/pm/features/pm-github-9dqy.toon)) - A failed provenance publish silently falls back to an unattested one ([pm-github-i5b8](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-i5b8.toon)) - Fix release publish ordering ahead of protected main push ([pm-github-v2kt](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-v2kt.toon)) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index b28de80..88f5cc8 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -554,9 +554,33 @@ export function bashArrays(text: string): Map { return arrays; } -/** A line opening with one assignment of a fully literal value, ending there or at a `;`. */ -const STANDALONE_ASSIGNMENT = - /^[ \t]*(?:export[ \t]+)?([A-Za-z_][A-Za-z0-9_]*)=(?:"((?:\\.|[^"\\$`])*)"|'([^']*)'|((?:\\.|[^\s;&|"'`$()\\])+))[ \t]*(?:[;#]|\r?$)/; +/** One literal assignment at the current position in an assignment-only command. */ +const LITERAL_ASSIGNMENT = + /^(?:export[ \t]+)?([A-Za-z_][A-Za-z0-9_]*)=(?:"((?:\\.|[^"\\$`])*)"|'([^']*)'|((?:\\.|[^\s;&|"'`$()\\])+))/; + +/** Parse every persistent literal binding at the start of one physical line. */ +function scalarAssignments(line: string): Array<[string, string]> { + const assignments: Array<[string, string]> = []; + let rest = line.replace(/^[ \t]*/, ""); + while (true) { + const assignment = LITERAL_ASSIGNMENT.exec(rest); + if (assignment === null) return []; + const raw = assignment[2] ?? assignment[3] ?? assignment[4]!; + const value = assignment[3] === undefined ? raw.replace(/\\(.)/g, "$1") : raw; + if (/[$`"'()]/.test(value)) return []; + assignments.push([assignment[1]!, value]); + rest = rest.slice(assignment[0].length).replace(/^[ \t]*/, ""); + if (/^(?:[;#]|\r?$)/.test(rest)) return assignments; + if (!/^(?:export[ \t]+)?[A-Za-z_][A-Za-z0-9_]*=/.test(rest)) return []; + } +} + +/** Return the heredoc terminator opened on a command line, if any. */ +function heredocTerminator(line: string): { delimiter: string; stripTabs: boolean } | undefined { + const match = /<<(-?)[ \t]*(?:'([^']+)'|"([^"]+)"|([A-Za-z_][A-Za-z0-9_]*))/.exec(line); + if (match === null) return undefined; + return { delimiter: match[2] ?? match[3] ?? match[4]!, stripTabs: match[1] === "-" }; +} /** * Index scalar assignments so a command held in a variable can be audited. @@ -566,10 +590,10 @@ const STANDALONE_ASSIGNMENT = * assignment is where the command actually is. `NPM=npm` followed by * `$NPM publish` hides one the same way, so unquoted values are indexed too. * - * A name is taken only where a line OPENS with one assignment carrying a fully - * literal value and holds nothing else before its end or a `;`. `NPM=npm; cmd` - * therefore binds, because the semicolon ends the assignment and the shell keeps - * it afterwards, while `NPM=npm cmd` does not, because that binding lasts only + * A name is taken only where a line OPENS with an assignment-only list carrying + * fully literal values and holds nothing else before its end or a `;`. + * `NPM=npm; cmd` therefore binds, because the semicolon ends the assignment and + * the shell keeps it afterwards, while `NPM=npm cmd` does not, because it lasts only * for the command it precedes. Requiring the line to OPEN with the assignment is * what keeps a `;` inside a comment from exposing one. That single rule keeps * the scan from inventing @@ -602,26 +626,44 @@ const STANDALONE_ASSIGNMENT = * verdict in both directions, which is worse than not resolving the variable. * * @param text - File contents with continuations already joined. - * @returns Variable name mapped to the literal text it holds. + * Heredoc bodies are data rather than commands and are skipped. Callers that + * expand a complete source use `expandShellScalars`, which applies each binding + * only to its own line and later lines, so reassignment cannot rewrite history. + * + * @returns Variable name mapped to the last literal text assigned to it. */ export function shellScalars(text: string): Map { const scalars = new Map(); + let heredoc: { delimiter: string; stripTabs: boolean } | undefined; for (const line of text.split("\n")) { - const assignment = STANDALONE_ASSIGNMENT.exec(line); - if (assignment === null) continue; - // Exactly one of the three value alternatives matches, so the last is the - // only case left rather than a fallback that could be undefined. - const raw = assignment[2] ?? assignment[3] ?? assignment[4]!; - // Single quotes make a backslash literal, so only the other two forms are - // unescaped. Unescaping a single-quoted value turned `'npm publish - // \\--provenance'` into an attested-looking command the shell never runs. - const value = assignment[3] === undefined ? raw.replace(/\\(.)/g, "$1") : raw; - if (/[$`"'()]/.test(value)) continue; - scalars.set(assignment[1]!, value); + if (heredoc !== undefined) { + const candidate = heredoc.stripTabs ? line.replace(/^\t+/, "") : line; + if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredoc = undefined; + continue; + } + for (const [name, value] of scalarAssignments(line)) scalars.set(name, value); + heredoc = heredocTerminator(line); } return scalars; } +/** Expand scalar references using only bindings visible at each source line. */ +export function expandShellScalars(text: string): string { + const scalars = new Map(); + let heredoc: { delimiter: string; stripTabs: boolean } | undefined; + return text.split("\n").map((line) => { + if (heredoc !== undefined) { + const candidate = heredoc.stripTabs ? line.replace(/^\t+/, "") : line; + if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredoc = undefined; + return line; + } + for (const [name, value] of scalarAssignments(line)) scalars.set(name, value); + const expanded = expandScalars(line, scalars); + heredoc = heredocTerminator(line); + return expanded; + }).join("\n"); +} + /** * Expand `$name` and `${name}` references against the file's scalar assignments. * diff --git a/scripts/verify-release-publish-attestation.ts b/scripts/verify-release-publish-attestation.ts index e3dc6a0..90c6b2e 100644 --- a/scripts/verify-release-publish-attestation.ts +++ b/scripts/verify-release-publish-attestation.ts @@ -27,9 +27,8 @@ import { commandCandidates, commandName, expandArrays, - expandScalars, + expandShellScalars, joinContinuations, - shellScalars, type ShellCommand, type SourceFile, tokenizeCommands, @@ -206,11 +205,10 @@ export function publishInvocationsIn(source: SourceFile): PublishInvocation[] { const raw = source.file.endsWith("package.json") ? manifestCommandLines(source.text) : source.text; const text = joinContinuations(raw); const arrays = bashArrays(text); - const scalars = shellScalars(text); - const expanded = text + const expanded = expandShellScalars(text .split("\n") - .map((line) => expandScalars(expandArrays(line, arrays), scalars)) - .join("\n"); + .map((line) => expandArrays(line, arrays)) + .join("\n")); const found: PublishInvocation[] = []; for (const command of tokenizeCommands(expanded)) { // Every reading, not just the command's own: a wrapper option that takes a diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index c46aca0..155883a 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -795,6 +795,42 @@ test("a publish routed through an unquoted scalar is audited, not hidden by an a assert.match(result.failures[0]!, /does not enable --provenance/); }); +test("scalar bindings are resolved at their source position", () => { + const result = auditPublishAttestation([{ + file: "release.yml", + text: [ + "NPM=npm", + "$NPM publish", + "NPM=echo", + "npm publish --provenance", + ].join("\n"), + }]); + assert.equal(result.failures.length, 1, "a later reassignment cannot rewrite the earlier publish"); +}); + +test("assignment-shaped heredoc content cannot attest a later publish", () => { + const result = auditPublishAttestation([{ + file: "release.yml", + text: [ + "cat < { + assert.equal(shellScalars("NPM=npm UNUSED=x\n").get("NPM"), "npm"); + const result = auditPublishAttestation([{ + file: "release.yml", + text: "NPM=npm UNUSED=x\n$NPM publish\nnpm publish --provenance\n", + }]); + assert.equal(result.failures.length, 1, "the publish routed through the first binding is audited"); +}); + test("an assignment the shell never makes is not indexed", () => { // Scalars used to be read straight out of the raw text, which indexed three // things the shell does not assign. The middle one is a gate bypass: a name From 1441cb434e83e565d1ed2a845b0e8efe546aa3b7 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:22:52 +0200 Subject: [PATCH 05/15] fix(scan): retain literals before dynamic assignments When an assignment-only list mixes literal and dynamic values, preserve earlier literal bindings while refusing the unresolved value. Keep command-scoped mixed assignments temporary and cover both paths. --- scripts/shell-command-scan.ts | 17 ++++++++++++++--- test/verify-release-publish-attestation.test.ts | 16 +++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 88f5cc8..1529d0a 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -558,20 +558,31 @@ export function bashArrays(text: string): Map { const LITERAL_ASSIGNMENT = /^(?:export[ \t]+)?([A-Za-z_][A-Za-z0-9_]*)=(?:"((?:\\.|[^"\\$`])*)"|'([^']*)'|((?:\\.|[^\s;&|"'`$()\\])+))/; +/** True when the line's outer command consists only of assignment words. */ +function isAssignmentOnlyLine(line: string): boolean { + const outer = tokenizeCommands(line)[0]; + if (outer === undefined) return false; + const words = outer[0]?.value === "export" ? outer.slice(1) : outer; + return words.length > 0 && words.every((token) => + !token.startsQuoted && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token.value)); +} + /** Parse every persistent literal binding at the start of one physical line. */ function scalarAssignments(line: string): Array<[string, string]> { const assignments: Array<[string, string]> = []; let rest = line.replace(/^[ \t]*/, ""); while (true) { const assignment = LITERAL_ASSIGNMENT.exec(rest); - if (assignment === null) return []; + if (assignment === null) return assignments.length > 0 && isAssignmentOnlyLine(line) ? assignments : []; const raw = assignment[2] ?? assignment[3] ?? assignment[4]!; const value = assignment[3] === undefined ? raw.replace(/\\(.)/g, "$1") : raw; - if (/[$`"'()]/.test(value)) return []; + if (/[$`"'()]/.test(value)) return assignments.length > 0 && isAssignmentOnlyLine(line) ? assignments : []; assignments.push([assignment[1]!, value]); rest = rest.slice(assignment[0].length).replace(/^[ \t]*/, ""); if (/^(?:[;#]|\r?$)/.test(rest)) return assignments; - if (!/^(?:export[ \t]+)?[A-Za-z_][A-Za-z0-9_]*=/.test(rest)) return []; + if (!/^(?:export[ \t]+)?[A-Za-z_][A-Za-z0-9_]*=/.test(rest)) { + return isAssignmentOnlyLine(line) ? assignments.slice(0, -1) : []; + } } } diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 155883a..2fed72d 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -824,11 +824,17 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { test("an assignment-only list persists every literal binding", () => { assert.equal(shellScalars("NPM=npm UNUSED=x\n").get("NPM"), "npm"); - const result = auditPublishAttestation([{ - file: "release.yml", - text: "NPM=npm UNUSED=x\n$NPM publish\nnpm publish --provenance\n", - }]); - assert.equal(result.failures.length, 1, "the publish routed through the first binding is audited"); + assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y)\n").get("NPM"), "npm", + "a later dynamic binding does not discard an earlier literal binding"); + assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y) echo no\n").get("NPM"), undefined, + "a command after the bindings makes all of them temporary"); + for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)"]) { + const result = auditPublishAttestation([{ + file: "release.yml", + text: `${assignment}\n$NPM publish\nnpm publish --provenance\n`, + }]); + assert.equal(result.failures.length, 1, "the publish routed through the first binding is audited"); + } }); test("an assignment the shell never makes is not indexed", () => { From 98762160f0e49946d7e1d3170d3a47ffc92f01a4 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:33:09 +0200 Subject: [PATCH 06/15] fix(scan): preserve shell syntax boundaries Accept assignment-only commands with redirections, detect heredocs only through unquoted shell operators, and parse scalar bindings before expanding array data. Add end-to-end bypass regressions for each case. --- scripts/shell-command-scan.ts | 48 ++++++++++++++++--- scripts/verify-release-publish-attestation.ts | 4 +- ...verify-release-publish-attestation.test.ts | 25 +++++++++- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 1529d0a..605e898 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -560,8 +560,9 @@ const LITERAL_ASSIGNMENT = /** True when the line's outer command consists only of assignment words. */ function isAssignmentOnlyLine(line: string): boolean { - const outer = tokenizeCommands(line)[0]; - if (outer === undefined) return false; + const parsed = tokenizeCommands(line)[0]; + if (parsed === undefined) return false; + const outer = withoutRedirections(parsed); const words = outer[0]?.value === "export" ? outer.slice(1) : outer; return words.length > 0 && words.every((token) => !token.startsQuoted && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token.value)); @@ -581,16 +582,49 @@ function scalarAssignments(line: string): Array<[string, string]> { rest = rest.slice(assignment[0].length).replace(/^[ \t]*/, ""); if (/^(?:[;#]|\r?$)/.test(rest)) return assignments; if (!/^(?:export[ \t]+)?[A-Za-z_][A-Za-z0-9_]*=/.test(rest)) { - return isAssignmentOnlyLine(line) ? assignments.slice(0, -1) : []; + if (!isAssignmentOnlyLine(line)) return []; + return /^(?:\d*)?(?:<>|>>?|<)/.test(rest) ? assignments : assignments.slice(0, -1); } } } -/** Return the heredoc terminator opened on a command line, if any. */ +/** Return a syntactic, unquoted heredoc terminator opened on a command line. */ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolean } | undefined { - const match = /<<(-?)[ \t]*(?:'([^']+)'|"([^"]+)"|([A-Za-z_][A-Za-z0-9_]*))/.exec(line); - if (match === null) return undefined; - return { delimiter: match[2] ?? match[3] ?? match[4]!, stripTabs: match[1] === "-" }; + let single = false; + let double = false; + for (let index = 0; index < line.length; index += 1) { + const char = line[index]!; + if (char === "\\" && !single) { + index += 1; + continue; + } + if (char === "'" && !double) { + single = !single; + continue; + } + if (char === '"' && !single) { + double = !double; + continue; + } + if (single || double) continue; + if (char === "#" && (index === 0 || /\s/.test(line[index - 1]!))) return undefined; + if (char !== "<" || line[index + 1] !== "<" || line[index + 2] === "<") continue; + let cursor = index + 2; + const stripTabs = line[cursor] === "-"; + if (stripTabs) cursor += 1; + while (line[cursor] === " " || line[cursor] === "\t") cursor += 1; + const quote = line[cursor] === "'" || line[cursor] === '"' ? line[cursor++] : undefined; + const start = cursor; + if (quote !== undefined) { + while (cursor < line.length && line[cursor] !== quote) cursor += 1; + } else { + while (cursor < line.length && /[A-Za-z0-9_]/.test(line[cursor]!)) cursor += 1; + } + if (cursor > start && (quote === undefined || line[cursor] === quote)) { + return { delimiter: line.slice(start, cursor), stripTabs }; + } + } + return undefined; } /** diff --git a/scripts/verify-release-publish-attestation.ts b/scripts/verify-release-publish-attestation.ts index 90c6b2e..ee546e7 100644 --- a/scripts/verify-release-publish-attestation.ts +++ b/scripts/verify-release-publish-attestation.ts @@ -205,10 +205,10 @@ export function publishInvocationsIn(source: SourceFile): PublishInvocation[] { const raw = source.file.endsWith("package.json") ? manifestCommandLines(source.text) : source.text; const text = joinContinuations(raw); const arrays = bashArrays(text); - const expanded = expandShellScalars(text + const expanded = expandShellScalars(text) .split("\n") .map((line) => expandArrays(line, arrays)) - .join("\n")); + .join("\n"); const found: PublishInvocation[] = []; for (const command of tokenizeCommands(expanded)) { // Every reading, not just the command's own: a wrapper option that takes a diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 2fed72d..7abe50a 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -820,15 +820,38 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { ].join("\n"), }]); assert.equal(result.failures.length, 1, "the heredoc body is data, not a shell binding"); + + for (const prose of ['echo "< { + const result = auditPublishAttestation([{ + file: "release.yml", + text: [ + "flags=( FLAG=--provenance )", + '"${flags[@]}"', + "npm publish $FLAG", + "npm publish --provenance", + ].join("\n"), + }]); + assert.equal(result.failures.length, 1, "expanded array data cannot create a shell variable"); }); test("an assignment-only list persists every literal binding", () => { assert.equal(shellScalars("NPM=npm UNUSED=x\n").get("NPM"), "npm"); + assert.equal(shellScalars("NPM=npm >/dev/null\n").get("NPM"), "npm", + "redirection-only assignment commands persist their bindings"); assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y)\n").get("NPM"), "npm", "a later dynamic binding does not discard an earlier literal binding"); assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y) echo no\n").get("NPM"), undefined, "a command after the bindings makes all of them temporary"); - for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)"]) { + for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)", "NPM=npm >/dev/null"]) { const result = auditPublishAttestation([{ file: "release.yml", text: `${assignment}\n$NPM publish\nnpm publish --provenance\n`, From c91616611195a64a2184d47114085e0315702e47 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:40:04 +0200 Subject: [PATCH 07/15] fix(scan): inspect every persistent assignment Continue across dynamic words in assignment-only commands so later literal bindings remain visible. Also recognize real heredocs executed inside double-quoted command substitutions and cover both bypasses end to end. --- scripts/shell-command-scan.ts | 55 +++++++++++++++---- ...verify-release-publish-attestation.test.ts | 10 +++- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 605e898..b43783b 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -568,24 +568,55 @@ function isAssignmentOnlyLine(line: string): boolean { !token.startsQuoted && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token.value)); } +/** Length of the first raw shell word, including quoted substitutions. */ +function shellWordLength(text: string): number { + let single = false; + let double = false; + let depth = 0; + for (let index = 0; index < text.length; index += 1) { + const char = text[index]!; + if (char === "\\" && !single) { + index += 1; + continue; + } + if (char === "'" && !double) single = !single; + else if (char === '"' && !single) double = !double; + else if (!single && char === "$" && text[index + 1] === "(") { + depth += 1; + index += 1; + } else if (!single && char === ")" && depth > 0) depth -= 1; + else if (!single && !double && depth === 0 && /\s/.test(char)) return index; + } + return text.length; +} + /** Parse every persistent literal binding at the start of one physical line. */ function scalarAssignments(line: string): Array<[string, string]> { const assignments: Array<[string, string]> = []; + const assignmentOnly = isAssignmentOnlyLine(line); let rest = line.replace(/^[ \t]*/, ""); - while (true) { + while (rest.length > 0) { + if (/^(?:[;#]|\r?$)/.test(rest)) return assignments; + if (/^(?:\d*)?(?:<>|>>?|<)/.test(rest)) return assignmentOnly ? assignments : []; const assignment = LITERAL_ASSIGNMENT.exec(rest); - if (assignment === null) return assignments.length > 0 && isAssignmentOnlyLine(line) ? assignments : []; + if (assignment === null) { + if (!assignmentOnly) return []; + rest = rest.slice(shellWordLength(rest)).replace(/^[ \t]*/, ""); + continue; + } + const after = rest.slice(assignment[0].length); + const boundary = after.length === 0 || /^[ \t\r;#]/.test(after); + if (!boundary) { + if (!assignmentOnly) return []; + rest = rest.slice(shellWordLength(rest)).replace(/^[ \t]*/, ""); + continue; + } const raw = assignment[2] ?? assignment[3] ?? assignment[4]!; const value = assignment[3] === undefined ? raw.replace(/\\(.)/g, "$1") : raw; - if (/[$`"'()]/.test(value)) return assignments.length > 0 && isAssignmentOnlyLine(line) ? assignments : []; - assignments.push([assignment[1]!, value]); - rest = rest.slice(assignment[0].length).replace(/^[ \t]*/, ""); - if (/^(?:[;#]|\r?$)/.test(rest)) return assignments; - if (!/^(?:export[ \t]+)?[A-Za-z_][A-Za-z0-9_]*=/.test(rest)) { - if (!isAssignmentOnlyLine(line)) return []; - return /^(?:\d*)?(?:<>|>>?|<)/.test(rest) ? assignments : assignments.slice(0, -1); - } + if (!/[$`"'()]/.test(value)) assignments.push([assignment[1]!, value]); + rest = after.replace(/^[ \t]*/, ""); } + return assignments; } /** Return a syntactic, unquoted heredoc terminator opened on a command line. */ @@ -602,6 +633,10 @@ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolea single = !single; continue; } + if (double && char === "$" && line[index + 1] === "(") { + const nested = heredocTerminator(line.slice(index + 2)); + if (nested !== undefined) return nested; + } if (char === '"' && !single) { double = !double; continue; diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 7abe50a..99c73ef 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -821,6 +821,12 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { }]); assert.equal(result.failures.length, 1, "the heredoc body is data, not a shell binding"); + const nested = auditPublishAttestation([{ + file: "release.yml", + text: 'value="$(cat < { "redirection-only assignment commands persist their bindings"); assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y)\n").get("NPM"), "npm", "a later dynamic binding does not discard an earlier literal binding"); + assert.equal(shellScalars("UNUSED=x$(printf y) NPM=npm\n").get("NPM"), "npm", + "an earlier dynamic binding does not hide a later literal binding"); assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y) echo no\n").get("NPM"), undefined, "a command after the bindings makes all of them temporary"); - for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)", "NPM=npm >/dev/null"]) { + for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)", "UNUSED=x$(printf y) NPM=npm", "NPM=npm >/dev/null"]) { const result = auditPublishAttestation([{ file: "release.yml", text: `${assignment}\n$NPM publish\nnpm publish --provenance\n`, From c46a8e674217b231fbe28d248efea55490e783b9 Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:46:21 +0200 Subject: [PATCH 08/15] fix(scan): keep quoted dynamic words bounded Skip a complete quoted dynamic assignment before inspecting later literal bindings, and explicitly exclude bash array declarations from scalar parsing. Cover both behaviors at audit level. --- scripts/shell-command-scan.ts | 1 + test/verify-release-publish-attestation.test.ts | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index b43783b..3139f49 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -595,6 +595,7 @@ function scalarAssignments(line: string): Array<[string, string]> { const assignments: Array<[string, string]> = []; const assignmentOnly = isAssignmentOnlyLine(line); let rest = line.replace(/^[ \t]*/, ""); + if (/^(?:export[ \t]+)?[A-Za-z_][A-Za-z0-9_]*=\(/.test(rest)) return []; while (rest.length > 0) { if (/^(?:[;#]|\r?$)/.test(rest)) return assignments; if (/^(?:\d*)?(?:<>|>>?|<)/.test(rest)) return assignmentOnly ? assignments : []; diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 99c73ef..5669be1 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -857,9 +857,11 @@ test("an assignment-only list persists every literal binding", () => { "a later dynamic binding does not discard an earlier literal binding"); assert.equal(shellScalars("UNUSED=x$(printf y) NPM=npm\n").get("NPM"), "npm", "an earlier dynamic binding does not hide a later literal binding"); + assert.equal(shellScalars('UNUSED="$(printf y)" NPM=npm\n').get("NPM"), "npm", + "a quoted dynamic binding does not swallow a later literal binding"); assert.equal(shellScalars("NPM=npm UNUSED=x$(printf y) echo no\n").get("NPM"), undefined, "a command after the bindings makes all of them temporary"); - for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)", "UNUSED=x$(printf y) NPM=npm", "NPM=npm >/dev/null"]) { + for (const assignment of ["NPM=npm UNUSED=x", "NPM=npm UNUSED=x$(printf y)", "UNUSED=x$(printf y) NPM=npm", 'UNUSED="$(printf y)" NPM=npm', "NPM=npm >/dev/null"]) { const result = auditPublishAttestation([{ file: "release.yml", text: `${assignment}\n$NPM publish\nnpm publish --provenance\n`, From f29de8344165a7f122af987babaa95acad2cfe3d Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:52:32 +0200 Subject: [PATCH 09/15] fix(scan): apply same-line scalar reassignments Expand semicolon-separated commands in execution order so a reassignment affects only following commands. Exclude arithmetic shifts from heredoc detection and add end-to-end regressions. --- scripts/shell-command-scan.ts | 36 +++++++++++++++++-- ...verify-release-publish-attestation.test.ts | 12 +++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 3139f49..1b2cdd1 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -620,6 +620,31 @@ function scalarAssignments(line: string): Array<[string, string]> { return assignments; } +/** Split one line at unquoted top-level semicolons, retaining separators. */ +function shellSegments(line: string): string[] { + const segments: string[] = []; + let start = 0; + let single = false; + let double = false; + let depth = 0; + for (let index = 0; index < line.length; index += 1) { + const char = line[index]!; + if (char === "\\" && !single) index += 1; + else if (char === "'" && !double) single = !single; + else if (char === '"' && !single) double = !double; + else if (!single && char === "$" && line[index + 1] === "(") { + depth += 1; + index += 1; + } else if (!single && char === ")" && depth > 0) depth -= 1; + else if (!single && !double && depth === 0 && char === ";") { + segments.push(line.slice(start, index), ";"); + start = index + 1; + } + } + segments.push(line.slice(start)); + return segments; +} + /** Return a syntactic, unquoted heredoc terminator opened on a command line. */ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolean } | undefined { let single = false; @@ -634,6 +659,11 @@ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolea single = !single; continue; } + if (!single && char === "$" && line[index + 1] === "(" && line[index + 2] === "(") { + const close = line.indexOf("))", index + 3); + if (close !== -1) index = close + 1; + continue; + } if (double && char === "$" && line[index + 1] === "(") { const nested = heredocTerminator(line.slice(index + 2)); if (nested !== undefined) return nested; @@ -738,8 +768,10 @@ export function expandShellScalars(text: string): string { if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredoc = undefined; return line; } - for (const [name, value] of scalarAssignments(line)) scalars.set(name, value); - const expanded = expandScalars(line, scalars); + const expanded = shellSegments(line).map((segment) => { + for (const [name, value] of scalarAssignments(segment)) scalars.set(name, value); + return expandScalars(segment, scalars); + }).join(""); heredoc = heredocTerminator(line); return expanded; }).join("\n"); diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 5669be1..9e45fd5 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -806,6 +806,12 @@ test("scalar bindings are resolved at their source position", () => { ].join("\n"), }]); assert.equal(result.failures.length, 1, "a later reassignment cannot rewrite the earlier publish"); + + const sameLine = auditPublishAttestation([{ + file: "release.yml", + text: "CMD='npm publish --provenance'; CMD='npm publish'; $CMD\nnpm publish --provenance\n", + }]); + assert.equal(sameLine.failures.length, 1, "same-line reassignment applies before the following command"); }); test("assignment-shaped heredoc content cannot attest a later publish", () => { @@ -827,6 +833,12 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { }]); assert.equal(nested.failures.length, 1, "a heredoc inside a quoted substitution is still data"); + const arithmetic = auditPublishAttestation([{ + file: "release.yml", + text: ": $((1 << 2))\nNPM=npm\n$NPM publish\nnpm publish --provenance\n", + }]); + assert.equal(arithmetic.failures.length, 1, "an arithmetic shift does not open a heredoc"); + for (const prose of ['echo "< Date: Sat, 29 Aug 2026 06:59:08 +0200 Subject: [PATCH 10/15] fix(scan): track compound shell boundaries Queue every heredoc opened by a command, sequence scalar state across && and ||, and reject pipeline or background assignments as persistent outer-shell bindings. Add end-to-end regressions for all cases. --- scripts/shell-command-scan.ts | 41 +++++++++++-------- ...verify-release-publish-attestation.test.ts | 16 ++++++++ 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 1b2cdd1..a37206d 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -560,6 +560,7 @@ const LITERAL_ASSIGNMENT = /** True when the line's outer command consists only of assignment words. */ function isAssignmentOnlyLine(line: string): boolean { + if (/(^|[^|&])(?:\||&)(?![|&])/.test(line)) return false; const parsed = tokenizeCommands(line)[0]; if (parsed === undefined) return false; const outer = withoutRedirections(parsed); @@ -620,7 +621,7 @@ function scalarAssignments(line: string): Array<[string, string]> { return assignments; } -/** Split one line at unquoted top-level semicolons, retaining separators. */ +/** Split one line at unquoted top-level sequencing operators, retaining them. */ function shellSegments(line: string): string[] { const segments: string[] = []; let start = 0; @@ -636,17 +637,21 @@ function shellSegments(line: string): string[] { depth += 1; index += 1; } else if (!single && char === ")" && depth > 0) depth -= 1; - else if (!single && !double && depth === 0 && char === ";") { - segments.push(line.slice(start, index), ";"); - start = index + 1; + else if (!single && !double && depth === 0 && (char === ";" || + ((char === "&" || char === "|") && line[index + 1] === char))) { + const width = char === ";" ? 1 : 2; + segments.push(line.slice(start, index), line.slice(index, index + width)); + start = index + width; + index += width - 1; } } segments.push(line.slice(start)); return segments; } -/** Return a syntactic, unquoted heredoc terminator opened on a command line. */ -function heredocTerminator(line: string): { delimiter: string; stripTabs: boolean } | undefined { +/** Return every syntactic, unquoted heredoc terminator opened on a command line. */ +function heredocTerminators(line: string): Array<{ delimiter: string; stripTabs: boolean }> { + const found: Array<{ delimiter: string; stripTabs: boolean }> = []; let single = false; let double = false; for (let index = 0; index < line.length; index += 1) { @@ -665,15 +670,14 @@ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolea continue; } if (double && char === "$" && line[index + 1] === "(") { - const nested = heredocTerminator(line.slice(index + 2)); - if (nested !== undefined) return nested; + found.push(...heredocTerminators(line.slice(index + 2))); } if (char === '"' && !single) { double = !double; continue; } if (single || double) continue; - if (char === "#" && (index === 0 || /\s/.test(line[index - 1]!))) return undefined; + if (char === "#" && (index === 0 || /\s/.test(line[index - 1]!))) return found; if (char !== "<" || line[index + 1] !== "<" || line[index + 2] === "<") continue; let cursor = index + 2; const stripTabs = line[cursor] === "-"; @@ -687,10 +691,11 @@ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolea while (cursor < line.length && /[A-Za-z0-9_]/.test(line[cursor]!)) cursor += 1; } if (cursor > start && (quote === undefined || line[cursor] === quote)) { - return { delimiter: line.slice(start, cursor), stripTabs }; + found.push({ delimiter: line.slice(start, cursor), stripTabs }); + index = cursor; } } - return undefined; + return found; } /** @@ -745,15 +750,16 @@ function heredocTerminator(line: string): { delimiter: string; stripTabs: boolea */ export function shellScalars(text: string): Map { const scalars = new Map(); - let heredoc: { delimiter: string; stripTabs: boolean } | undefined; + const heredocs: Array<{ delimiter: string; stripTabs: boolean }> = []; for (const line of text.split("\n")) { + const heredoc = heredocs[0]; if (heredoc !== undefined) { const candidate = heredoc.stripTabs ? line.replace(/^\t+/, "") : line; - if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredoc = undefined; + if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredocs.shift(); continue; } for (const [name, value] of scalarAssignments(line)) scalars.set(name, value); - heredoc = heredocTerminator(line); + heredocs.push(...heredocTerminators(line)); } return scalars; } @@ -761,18 +767,19 @@ export function shellScalars(text: string): Map { /** Expand scalar references using only bindings visible at each source line. */ export function expandShellScalars(text: string): string { const scalars = new Map(); - let heredoc: { delimiter: string; stripTabs: boolean } | undefined; + const heredocs: Array<{ delimiter: string; stripTabs: boolean }> = []; return text.split("\n").map((line) => { + const heredoc = heredocs[0]; if (heredoc !== undefined) { const candidate = heredoc.stripTabs ? line.replace(/^\t+/, "") : line; - if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredoc = undefined; + if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredocs.shift(); return line; } const expanded = shellSegments(line).map((segment) => { for (const [name, value] of scalarAssignments(segment)) scalars.set(name, value); return expandScalars(segment, scalars); }).join(""); - heredoc = heredocTerminator(line); + heredocs.push(...heredocTerminators(line)); return expanded; }).join("\n"); } diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 9e45fd5..6dc9b5a 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -807,6 +807,14 @@ test("scalar bindings are resolved at their source position", () => { }]); assert.equal(result.failures.length, 1, "a later reassignment cannot rewrite the earlier publish"); + for (const operator of ["&&", "||"]) { + const controlled = auditPublishAttestation([{ + file: "release.yml", + text: `NPM=npm ${operator} $NPM publish\nnpm publish --provenance\n`, + }]); + assert.equal(controlled.failures.length, 1, `${operator} preserves the assignment before the invocation`); + } + const sameLine = auditPublishAttestation([{ file: "release.yml", text: "CMD='npm publish --provenance'; CMD='npm publish'; $CMD\nnpm publish --provenance\n", @@ -827,6 +835,12 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { }]); assert.equal(result.failures.length, 1, "the heredoc body is data, not a shell binding"); + const multiple = auditPublishAttestation([{ + file: "release.yml", + text: "cat < Date: Sat, 29 Aug 2026 07:05:48 +0200 Subject: [PATCH 11/15] fix(scan): ignore non-command shell text Stop sequencing at unquoted comments so assignment-shaped prose cannot create bindings, and exclude both arithmetic-command forms from heredoc recognition. Add end-to-end regressions. --- scripts/shell-command-scan.ts | 7 +++++-- test/verify-release-publish-attestation.test.ts | 13 ++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index a37206d..dfbb494 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -637,6 +637,8 @@ function shellSegments(line: string): string[] { depth += 1; index += 1; } else if (!single && char === ")" && depth > 0) depth -= 1; + else if (!single && !double && depth === 0 && char === "#" && + (index === 0 || /\s/.test(line[index - 1]!))) break; else if (!single && !double && depth === 0 && (char === ";" || ((char === "&" || char === "|") && line[index + 1] === char))) { const width = char === ";" ? 1 : 2; @@ -664,8 +666,9 @@ function heredocTerminators(line: string): Array<{ delimiter: string; stripTabs: single = !single; continue; } - if (!single && char === "$" && line[index + 1] === "(" && line[index + 2] === "(") { - const close = line.indexOf("))", index + 3); + if (!single && ((char === "$" && line[index + 1] === "(" && line[index + 2] === "(") || + (!double && char === "(" && line[index + 1] === "("))) { + const close = line.indexOf("))", index + (char === "$" ? 3 : 2)); if (close !== -1) index = close + 1; continue; } diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 6dc9b5a..502df7c 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -847,11 +847,13 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { }]); assert.equal(nested.failures.length, 1, "a heredoc inside a quoted substitution is still data"); - const arithmetic = auditPublishAttestation([{ - file: "release.yml", - text: ": $((1 << 2))\nNPM=npm\n$NPM publish\nnpm publish --provenance\n", - }]); - assert.equal(arithmetic.failures.length, 1, "an arithmetic shift does not open a heredoc"); + for (const expression of [": $((1 << 2))", "(( 1 << 2 ))"]) { + const arithmetic = auditPublishAttestation([{ + file: "release.yml", + text: `${expression}\nNPM=npm\n$NPM publish\nnpm publish --provenance\n`, + }]); + assert.equal(arithmetic.failures.length, 1, `${expression} does not open a heredoc`); + } for (const prose of ['echo "< Date: Sat, 29 Aug 2026 07:12:00 +0200 Subject: [PATCH 12/15] fix(scan): isolate conditional assignment state Do not persist bindings introduced only after && or ||, and recognize Bash's |& combined pipeline as non-persistent. Add end-to-end false-attestation regressions. --- scripts/shell-command-scan.ts | 11 +++++++++-- test/verify-release-publish-attestation.test.ts | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index dfbb494..30db2f3 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -560,7 +560,7 @@ const LITERAL_ASSIGNMENT = /** True when the line's outer command consists only of assignment words. */ function isAssignmentOnlyLine(line: string): boolean { - if (/(^|[^|&])(?:\||&)(?![|&])/.test(line)) return false; + if (/\|&|(^|[^|&])(?:\||&)(?![|&])/.test(line)) return false; const parsed = tokenizeCommands(line)[0]; if (parsed === undefined) return false; const outer = withoutRedirections(parsed); @@ -778,8 +778,15 @@ export function expandShellScalars(text: string): string { if (candidate.replace(/\r$/, "") === heredoc.delimiter) heredocs.shift(); return line; } + let conditional = false; const expanded = shellSegments(line).map((segment) => { - for (const [name, value] of scalarAssignments(segment)) scalars.set(name, value); + if (segment === "&&" || segment === "||") { + conditional = true; + return segment; + } + if (!conditional) { + for (const [name, value] of scalarAssignments(segment)) scalars.set(name, value); + } return expandScalars(segment, scalars); }).join(""); heredocs.push(...heredocTerminators(line)); diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 502df7c..41f4966 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -961,6 +961,8 @@ test("a scalar is taken only from a line that is exactly one literal assignment" // Both leaks were false passes end to end, not merely wrong map entries. for (const text of [ [" FLAG=--provenance | cat", " npm publish --access public $FLAG"], + [" FLAG=--provenance |& cat", " npm publish --access public $FLAG"], + [" false && FLAG=--provenance", " npm publish --access public $FLAG"], [" FLAG=--provenance &", " npm publish --access public $FLAG"], [" # ignored; FLAG=--provenance", " npm publish --access public $FLAG"], [" FLAG=--provenance some-command", " npm publish --access public $FLAG"], From 69a4e97ee42c748b7088937b706a8cf4d0475f9d Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:16:23 +0200 Subject: [PATCH 13/15] fix(gate): fail closed on unresolved publishers Audit variable-routed publish commands even when scalar resolution cannot prove the publisher name. This makes unsupported conditional and export syntax a conservative failure instead of a false clean verdict. --- scripts/verify-release-publish-attestation.ts | 8 ++++++-- test/verify-release-publish-attestation.test.ts | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/verify-release-publish-attestation.ts b/scripts/verify-release-publish-attestation.ts index ee546e7..2237fb5 100644 --- a/scripts/verify-release-publish-attestation.ts +++ b/scripts/verify-release-publish-attestation.ts @@ -218,11 +218,15 @@ export function publishInvocationsIn(source: SourceFile): PublishInvocation[] { for (const candidate of commandCandidates(command)) { const program = commandName(candidate); if (program === undefined) continue; - if (program !== "npm" && !FOREIGN_PUBLISHERS.has(program)) continue; if (!isPublishCommand(candidate)) continue; + // A variable-routed publisher that cannot be resolved must fail closed. + // Treating `$NPM publish` as "not a publish" lets any unsupported shell + // construct turn a parser limitation into a clean attestation verdict. + const unresolvedPublisher = /^\$(?:\{[A-Za-z_][A-Za-z0-9_]*\}|[A-Za-z_][A-Za-z0-9_]*)$/.test(program); + if (program !== "npm" && !FOREIGN_PUBLISHERS.has(program) && !unresolvedPublisher) continue; // Not de-duplicated: two identical publish lines are two invocations, and // collapsing them would report one of them as if the other did not exist. - found.push({ file: source.file, program, command: candidate }); + found.push({ file: source.file, program: unresolvedPublisher ? "unresolved-shell-variable" : program, command: candidate }); } } return found; diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 41f4966..1ac858e 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -963,6 +963,8 @@ test("a scalar is taken only from a line that is exactly one literal assignment" [" FLAG=--provenance | cat", " npm publish --access public $FLAG"], [" FLAG=--provenance |& cat", " npm publish --access public $FLAG"], [" false && FLAG=--provenance", " npm publish --access public $FLAG"], + [" false || NPM=npm", " $NPM publish --access public"], + [" export NPM=npm FOO", " $NPM publish --access public"], [" FLAG=--provenance &", " npm publish --access public $FLAG"], [" # ignored; FLAG=--provenance", " npm publish --access public $FLAG"], [" FLAG=--provenance some-command", " npm publish --access public $FLAG"], From cbd743068bd5bf21f26c4c345e636ae9a4e1052a Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:20:24 +0200 Subject: [PATCH 14/15] test(gate): accept fail-closed publisher diagnostic --- test/verify-release-publish-attestation.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index 1ac858e..cf1bc21 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -972,7 +972,7 @@ test("a scalar is taken only from a line that is exactly one literal assignment" ]) { const result = auditPublishAttestation([{ file: "release.yml", text: text.join("\n") }]); assert.equal(result.failures.length, 1, `a publish flagged only by ${text[0]!.trim()} is unattested`); - assert.match(result.failures[0]!, /does not enable --provenance/); + assert.match(result.failures[0]!, /does not enable --provenance|unresolved-shell-variable/); } }); test("a read-write redirection does not turn its target into the command", () => { From 95d12a4c49a418658903bd6bf708b6ae7beb3fff Mon Sep 17 00:00:00 2001 From: SteveBot <1153461+unbraind@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:24:15 +0200 Subject: [PATCH 15/15] fix(scan): preserve punctuated heredoc delimiters --- scripts/shell-command-scan.ts | 2 +- test/verify-release-publish-attestation.test.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/shell-command-scan.ts b/scripts/shell-command-scan.ts index 30db2f3..84e7e00 100644 --- a/scripts/shell-command-scan.ts +++ b/scripts/shell-command-scan.ts @@ -691,7 +691,7 @@ function heredocTerminators(line: string): Array<{ delimiter: string; stripTabs: if (quote !== undefined) { while (cursor < line.length && line[cursor] !== quote) cursor += 1; } else { - while (cursor < line.length && /[A-Za-z0-9_]/.test(line[cursor]!)) cursor += 1; + while (cursor < line.length && /[^\s;&|<>()]/.test(line[cursor]!)) cursor += 1; } if (cursor > start && (quote === undefined || line[cursor] === quote)) { found.push({ delimiter: line.slice(start, cursor), stripTabs }); diff --git a/test/verify-release-publish-attestation.test.ts b/test/verify-release-publish-attestation.test.ts index cf1bc21..8328064 100644 --- a/test/verify-release-publish-attestation.test.ts +++ b/test/verify-release-publish-attestation.test.ts @@ -835,6 +835,12 @@ test("assignment-shaped heredoc content cannot attest a later publish", () => { }]); assert.equal(result.failures.length, 1, "the heredoc body is data, not a shell binding"); + const punctuated = auditPublishAttestation([{ + file: "release.yml", + text: "cat <