diff --git a/.dev/features/harden-action-pin-coverage/PLAN.md b/.dev/features/harden-action-pin-coverage/PLAN.md new file mode 100644 index 0000000..dd83f68 --- /dev/null +++ b/.dev/features/harden-action-pin-coverage/PLAN.md @@ -0,0 +1,79 @@ +# PLAN — harden check-action-pins: make the gate see what it claims to see + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: Close the coverage holes in `.dev/floor/check-action-pins.mjs` — refs it never sees, exemptions it grants silently, and output it truncates — so a clean exit means "every executed ref was inspected", not "every ref I happened to parse". +- layer(s): `.dev/floor/` (deterministic floor) +- constitution_refs: [P0, P1, P3, P5, P7] + +## Files + +- `.dev/floor/check-action-pins.mjs` — discovery + parse + classification hardening +- `.dev/floor/check-action-pins.test.mjs` — a regression test per hole, plus strengthened live assertions + +Stacked on `chore/pin-action-digests` (PR #79): the checker exists only on that branch. + +## Why this increment exists (P7 — real, reproduced, not hypothetical) + +PR #79 shipped a gate that reports clean while unpinned third-party code executes. Every row below was **reproduced by running the shipped checker** this run, not inferred: + +| probe | shipped behaviour | why it matters | +| --- | --- | --- | +| `- {uses: actions/checkout@main}` | `checked:0`, exit 0 | valid YAML flow mapping; GitHub executes it, the gate never classifies it | +| `- "uses": actions/checkout@v1` | `checked:0`, exit 0 | valid quoted key; same | +| `CI.YML` (uppercase ext) | `files:[]`, exit 0 | whole file skipped — **and the test's "independent" recount replicates the same case-sensitive filter, so it is not independent on this axis** | +| `docker://alpine:latest` | `skipped:1`, exit 0 | a fully mutable image tag is remote code, blanket-exempted by scheme | +| `./${{ … }}` | `skipped:1`, exit 0 | `isExempt()` runs **before** `classify()`, so the prefix defeats the `unpinnable-ref` rule | +| dangling symlink `*.yml` | exit 1, **no JSON** | uncaught ENOENT inside `Array.filter` — no report at all | +| 4000 violations through a pipe | truncated at 65536 B | `process.exit()` after `stdout.write` — JSON unparseable by any consumer | + +Two more, found the same way, deliberately **out of scope** (see follow-ups): local composite actions were also never walked, which this plan fixes, but **owner binding** and **wiring fragility** are separate axes. + +## The changes + +**A. Nothing executes invisibly.** + +1. Recognise all three `uses:` spellings — bare key, quoted key (`"uses":`), and flow-mapping entries (`{uses: …}`) — instead of only a line-initial bare key. +2. Also walk **local action definitions**: `.github/actions/**/action.{yml,yaml}` (bounded, symlink-safe recursion). GitHub executes a composite action's `runs.steps[].uses:`, so a `./` wrapper was previously a laundering path — exempt at the call site and never scanned at the definition. +3. Extension match becomes **case-insensitive**. +4. A file that cannot be stat'ed or read (dangling symlink) becomes a **violation** (`unreadable-file`), never a crash and never a silent skip. + +**B. No silent exemption.** + +1. `classify()` runs the `${{ }}` test **before** exemption, so `./${{ … }}` and `docker://${{ … }}` are `unpinnable-ref`. +2. `docker://` must be digest-pinned (`@sha256:<64 hex>`); a tag is `unpinned-container`. +3. A `./` ref containing a `..` segment is `escaping-local-ref` — the exemption covers genuinely-local paths only. +4. The live test asserts `skipped` **exactly**, so any future exemption use is surfaced rather than absorbed. + +**C. Output integrity.** + +1. `emit()` sets `process.exitCode` and returns instead of calling `process.exit()`, so stdout drains. Reproduced: 400 KB of JSON truncated to exactly 65536 B through a pipe and via `spawnSync`. + +## Evals to write (P1) + +One hermetic regression per row of the table above, plus: conforming flow-mapping and quoted-key refs pass; `docker://img@sha256:<64hex>` passes; `./local-action` still passes when its `action.yml` is clean; a composite `action.yml` with a floating ref is **caught**; large output survives a pipe intact. Live: `violations == []`, `skipped == 0` exactly, `checked >= 10`, and the workflow file set matches a **case-insensitive** independent recount. + +## Guarantee audit (P0) + +| Claim | Reduction | +| --- | --- | +| "Every `uses:` GitHub executes from this repo's workflows and local actions is inspected" | **FLOOR — regex/enum over an enumerated file set.** Strictly wider than what shipped; each previously-invisible form now has a regression test. | +| "No ref is exempted without leaving a trace" | **FLOOR** — `skipped` asserted exactly in the live test. | +| "The report survives a pipe" | **FLOOR — reproduced** both before (truncated) and after. | +| "The digest belongs to the *intended* owner" | **NOT GUARANTEED — named.** `attacker/checkout@<40hex> # v7.0.1` conforms today. Binding owners needs an allowlist policy; out of scope. | +| "The comment is TRUE" | **NOT GUARANTEED — unchanged.** Needs network; floor scripts are network-free. | +| "No YAML form can hide a ref" | **ADVISORY.** This is a line scanner, not a YAML parser (floor scripts are stdlib-only, and `js-yaml` is transitive, not a dependency). Unrecognised shapes now fail **toward** flagging, but exotic YAML (block scalars, anchors) is a known imprecision — documented, fail-closed in direction. | + +## Determinism audit (P5) + +Every branch is a regex match, a set membership, or a path test; `reason` stays an enum; unreadable input becomes a violation rather than a skip. No classification, no guessing fallback. + +## Out of axis — named, NOT in this increment (P3/P7) + +1. **Owner binding** — `attacker/checkout@` conforms. Needs an allowlist policy decision. +2. **Wiring fragility** — the gate rides one glob string in `floor.yml`; deleting the test file, a typo in the glob, or `continue-on-error: true` all silently disarm it. Needs a different mechanism (required checks / a meta-test), not a change to this checker. +3. **Supply chain beyond `uses:`** — `publish.yml` installs `npm@latest` inside the `id-token: write` job; `npm ci` runs dependency install scripts; the gitleaks step is `curl`-and-execute (checksum-verified today); pinned actions have their own transitive floating refs. All real, none addressable by a `uses:` form checker. +4. **`node-version` policy** — carried forward, still open. + +## Open questions (HALT) + +None blocking. Scope grew from the three items originally described to the full coverage class because the sweep reproduced seven distinct holes; the growth is recorded here rather than absorbed silently, and everything outside the coverage axis is listed above instead of being fixed opportunistically. diff --git a/.dev/features/harden-action-pin-coverage/REVIEW.md b/.dev/features/harden-action-pin-coverage/REVIEW.md new file mode 100644 index 0000000..fa2ef43 --- /dev/null +++ b/.dev/features/harden-action-pin-coverage/REVIEW.md @@ -0,0 +1,128 @@ +# REVIEW — harden-action-pin-coverage + +**Step 1 — floor first (P0):** `node .dev/floor/validate.mjs .` → **exit 0**, `FLOOR: GREEN`. + +Increment under review (`trust: untrusted`): `.dev/floor/check-action-pins.mjs` + its test. + +Standing floor verdicts: `validate` exit 0 · `regression-report.json` `"no-regressions"` · `verify-report.json` `"PASS"` · full `floor.yml` command **704 tests, 0 fail** (684 → +20 across both rounds; see "Second round" below). + +--- + +## Findings + +### L-floor → P0 + +```yaml +- type: FINDING + rule_id: "P0" + severity: important + file: ".dev/floor/check-action-pins.mjs:23" + problem: "The scanner is a line matcher presented alongside a floor-primitive guarantee, and while unrecognised shapes now fail toward flagging, no test establishes that a ref hidden in an unmodelled YAML construct is caught rather than merely unmatched." + evidence: "'Total YAML fidelity. This is a LINE SCANNER, not a YAML parser'" + gate: advisory-gate + +- type: FINDING + rule_id: "P0" + severity: minor + file: ".dev/floor/check-action-pins.mjs:21" + problem: "A conforming digest is still not bound to an owner, so a fork substitution with a truthful-looking comment passes, which the header now discloses but the gate does not detect." + evidence: "'The OWNER of the digest. `attacker/checkout@<40hex> # v7.0.1` is fully conforming here'" + gate: advisory-gate +``` + +**On the first finding — the honest limit of this increment.** Eight concrete holes are closed and each has a regression test, but "we fixed the eight we found" is not "no ninth exists." A line scanner cannot enumerate the YAML shapes it does not model. The mitigation is directional, not total: unrecognised input now fails **toward** flagging (`unreadable-file`, empty-ref → `floating-ref`), so the next gap should surface as noise rather than silence. That is a real improvement over the shipped version, where four distinct forms produced `checked:0, exit 0` — indistinguishable from a clean repo. Stated as a limit, not sold as coverage. + +### L-eval → P1 + +No finding. Every closed hole ships a regression test that **encodes the shipped behaviour it replaces** (`✱` markers record `was checked:0, exit 0` etc.), so a revert cannot pass silently. Both directions are covered — the fixes do not blanket-fail their forms (a conforming flow mapping, a digest-pinned image, and a clean local composite action each have a passing test). Floor agreement: `validate.mjs` reports `0 capabilities`; no capability eval binding is owed. + +### L-trust → P2 + +No finding. No untrusted artifact is ingested by the product; the checker reads repo files as data, never executes them, adds no network/`child_process`/`eval`. `ref` remains verbatim file content and is disclosed as such in the header — but the verdict is `violations.length > 0`, an integer test, so no decision reads a tainted value. The new recursive walk **refuses symlinks** rather than following them, so it cannot be induced to read outside the tree. + +### L-axis → P3 + +No finding. Two files, one axis: coverage integrity of one checker. `validate.mjs` untouched. No sibling imports. + +### Process + +```yaml +- type: FINDING + rule_id: "P6" + severity: important + file: ".dev/features/harden-action-pin-coverage/PLAN.md:1" + problem: "The adversarial sweep's verification phase ran concurrently with the fix being applied, so its refutations cannot distinguish a weak finding from one already repaired mid-flight, making its confirmed/refuted counts unusable as evidence." + evidence: "'REFUTED as stated. The script ... DOES flag the claimed payload'" + gate: advisory-gate +``` + +This is a genuine methodology error and it is recorded rather than hidden. 5 confirmed / 16 refuted **must not** be cited as a clean result: the checker was rewritten while verifiers were still running against it, so several refutations describe the *fixed* script. What is sound is the **before/after evidence produced directly**: each hole was reproduced against the shipped version (recorded in `PLAN.md`'s table with literal output), then re-probed after the fix (all seven now exit 1 with the correct enum reason). The correct process would have been to snapshot the script, or to hold the fix until verification drained. Proposed as a lesson candidate below. + +--- + +## Gate split (fix #3) + +- **floor-gate (blocking): none.** +- **advisory-gate: all three findings.** + +## What the increment got right (checked, not assumed) + +- **Every previously-confirmed bypass re-probed and closed**, each with the correct enum reason — flow mapping and quoted key now `checked:1` + `floating-ref`; `docker://alpine:latest` → `unpinned-container`; `./${{ }}` → `unpinnable-ref`; `CI.YML` opened; dangling symlink → `unreadable-file` **with JSON on stdout**; composite laundering → the inner `attacker/evil@main` caught with `action.yml` enumerated. +- **The truncation fix is proven by a test, not by reasoning** — 4000 violations through `spawnSync`'s pipe, asserting `stdout.length > 65536` and that the JSON parses. The shipped version cut it at exactly 65536. +- **The anti-vacuity assertions are genuinely stronger**: `skipped` is now exact (an exemption cannot be absorbed silently), and the independent recount is case-**insensitive** — the previous "independent" check replicated the very filter bug that made `CI.YML` vanish from both sides at once. + +## Verdict + +**GREEN — 0 floor-gate findings; 3 advisory (0 blocking, 2 important, 1 minor).** + +--- + +--- + +## Second round (appended after the sweep completed) + +The adversarial sweep finished **after** the first fix was pushed, and it confirmed **five more bypasses that survived it** — reproduced against the pushed code, not inferred: + +| input | pushed behaviour | +| --- | --- | +| CRLF line endings anywhere in a workflow | `checked:0`, exit 0 — **whole file invisible** | +| lone-CR (classic Mac) endings | `checked:0`, exit 0 — file collapses to one line | +| `steps: [{uses: evil@v1}]` | `checked:0`, exit 0 | +| `- {with: {x: 1}, uses: evil@v1}` | `checked:0`, exit 0 | +| `[{uses: evil@v1}]` on its own line | `checked:0`, exit 0 | + +**CRLF is the serious one.** It is reachable *by accident*: `core.autocrlf=true` on a Windows checkout produces it, this repo has no `.gitattributes`, and prettier's globs exclude `.github/**` — so nothing normalises or rejects it. A verifier reproduced it against a copy of this repo's real `.github/`: injecting `- uses: evil/exfiltrate@main\r\n` into `ci.yml` left the output **byte-identical to the clean baseline** (`checked:10, skipped:0, violations:[]`, exit 0). + +**Both had structural root causes, not missing cases** — which is exactly why round one missed them: + +1. the file was split on `"\n"` only, so a trailing `\r` defeated every line-anchored match; +2. the `uses:` key was anchored to line start, so any flow position hid it. + +Fixed at the roots: split on `/\r\n|\r|\n/`, and match the key **anywhere** in the line (bounded by `(?:^|[\s,{[])` so `causes:` does not match, and capturing to the next `,`/`}`/`]` so a multi-ref line yields every ref). All five closed, plus a sixth found while fixing: two refs on one line previously yielded only the first. + +```yaml +- type: FINDING + rule_id: "P0" + severity: important + file: ".dev/floor/check-action-pins.mjs:57" + problem: "Round one closed eight enumerated cases while leaving two structural assumptions untouched, so the gate was re-shipped with a whole-file fail-open reachable by an ordinary Windows checkout." + evidence: "'const USES_LINE_RE = /^\\s*(?:-\\s*)?[\"\']?uses[\"\']?\\s*:\\s*(.*)$/;'" + gate: advisory-gate +``` + +**The lesson this makes concrete, and it upgrades the epistemics of the whole increment.** Round one fixed the cases the sweep listed; the sweep's own list was not exhaustive, and re-running it against the hardened artifact found more. The honest reading is that *enumerating bad inputs does not bound a line scanner's failure set* — only changing the parsing model does. That is now reflected in the code (root-cause fixes) and in the header's disclosure, but it remains true that a **third** round could find a third class. The `## Verdict` above stands, with this qualification attached. + +**Process note, again:** the first push happened while verification was still running. That is the same error already logged as lesson candidate 1, and this is its concrete cost — a PR whose description overstated its coverage for roughly twenty minutes. + +## Proposed lesson candidates (NOT written to canon — P2/P7) + +1. **Do not run an adversarial verification phase against a moving target.** If a fix lands while verifiers are still probing, refutations become uninterpretable — you cannot tell a weak finding from one already fixed. Snapshot the artifact under test, or hold the fix. *Provenance:* this increment; sweep `wf_885c411f-670`; observable as refutations reporting that the script "DOES flag" a payload it did not flag when the finding was raised. +2. **A form-checker's exemptions are its attack surface.** Every `isExempt()` branch is a place unpinned code can be moved to. Exemptions must be counted and asserted exactly, and the thing an exemption points at (here, `action.yml`) must itself be enumerated — otherwise the exemption is a laundering path. *Provenance:* `./` composite-action laundering and the `docker://` scheme exemption, both reproduced against PR #79's shipped gate. +3. Carried forward, still unpromoted: the stale-pin-comment lesson and the "don't format outside `format:check`'s globs" lesson from the two prior increments. + +## Named follow-ups (not built — different axes, P3/P7) + +1. **Owner binding** — needs an allowlist policy decision. +2. **Wiring fragility** — the gate rides one glob string in `floor.yml`; deleting the test, a typo in the glob, or `continue-on-error: true` disarms it silently. Needs required-checks config or a meta-test, not a change to this checker. +3. **Supply chain beyond `uses:`** — `publish.yml` installs `npm@latest` inside the `id-token: write` job; `npm ci` runs dependency install scripts; gitleaks is `curl`-and-execute (checksum-verified); pinned actions have their own transitive floating refs. Real, and none addressable by a `uses:` form checker. +4. **`node-version` policy** — carried forward. diff --git a/.dev/features/harden-action-pin-coverage/SHIP.md b/.dev/features/harden-action-pin-coverage/SHIP.md new file mode 100644 index 0000000..ee78f49 --- /dev/null +++ b/.dev/features/harden-action-pin-coverage/SHIP.md @@ -0,0 +1,32 @@ +# SHIP — harden-action-pin-coverage + +Third increment of the session. Stacked on `chore/pin-action-digests` (PR #79) — the checker exists only on that branch. + +## Stages run + +| # | stage | outcome | +| --- | --- | --- | +| 0 | adversarial sweep (`wf_885c411f-670`) | 5 lenses, 29 candidates — see the caveat below | +| 1 | `/pharn-dev-plan` | `PLAN.md`; scope grew from 3 items to the full coverage class | +| 2 | `/pharn-dev-build` | 2 files; floor GREEN | +| 3 | `/pharn-dev-regress` | `"no-regressions"` | +| 4 | `/pharn-dev-verify` | `"PASS"` | +| 5 | `/pharn-dev-review` | `REVIEW.md` — GREEN, 3 advisory | + +## Structural verdicts, verbatim + +- **`validate.mjs` exit `0`** (`FLOOR: GREEN`); `npm run check` exit `0`; full `floor.yml` `node --test` exit `0`, **696 tests / 0 fail** (684 → +12). +- **`regression-report.json` `.verdict`: `"no-regressions"`** (`check-regress` exit 0; `escaped: []`). +- **`verify-report.json` `.verdict`: `"PASS"`** (`check-verify` exit 0; `failing_gates: []`; `verifiers.registered: 0`). + +## Evidence that the holes are actually closed + +Each hole was reproduced against the **shipped** gate (PR #79), then re-probed after the fix. All seven now exit 1 with the correct enum reason; each has a regression test encoding the old behaviour. `npm test` is vitest and does not collect `.mjs`, so the 30 tests here are proven by the floor.yml command directly, not by the `test` gate. + +## Recorded methodology error (P6) + +The sweep's verification phase ran **concurrently with the fix**, so its 5-confirmed/16-refuted split is uninterpretable and is NOT cited as evidence — several refutations describe the already-repaired script. The reliable evidence is the direct before/after probing. Proposed as a lesson candidate in `REVIEW.md`. + +## Standing decision + +The chain ran; the named floor verdicts are as shown — **this is NOT a judgment that the increment is good or wise; that is the human's call at the post-review gate.** Nothing merged or sealed. diff --git a/.dev/features/harden-action-pin-coverage/regression-report.json b/.dev/features/harden-action-pin-coverage/regression-report.json new file mode 100644 index 0000000..cd7edb7 --- /dev/null +++ b/.dev/features/harden-action-pin-coverage/regression-report.json @@ -0,0 +1,20 @@ +{ + "base": "25f8f4bd07f0d5d44e7c00280c1a7eb4d20bd690", + "inside": [ + ".dev/floor/check-action-pins.mjs", + ".dev/floor/check-action-pins.test.mjs" + ], + "outside_gates": { + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/harden-action-pin-coverage/verify-report.json b/.dev/features/harden-action-pin-coverage/verify-report.json new file mode 100644 index 0000000..a38667e --- /dev/null +++ b/.dev/features/harden-action-pin-coverage/verify-report.json @@ -0,0 +1,16 @@ +{ + "feature": "harden-action-pin-coverage", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "test": 0, + "validate": 0 + }, + "verdict": "PASS", + "failing_gates": [], + "verifiers": { + "registered": 0, + "findings": [] + } +} diff --git a/.dev/floor/check-action-pins.mjs b/.dev/floor/check-action-pins.mjs index 3bfcd58..89e0ad0 100644 --- a/.dev/floor/check-action-pins.mjs +++ b/.dev/floor/check-action-pins.mjs @@ -1,165 +1,286 @@ #!/usr/bin/env node -// .dev/floor/check-action-pins.mjs — deterministic floor check over .github/workflows/*.yml: -// every third-party GitHub Actions `uses:` ref must be pinned by a 40-hex COMMIT DIGEST and carry a -// full-semver `# vX.Y.Z` comment. +// .dev/floor/check-action-pins.mjs — deterministic floor check over the GitHub Actions definitions +// this repo executes: every third-party `uses:` ref must be pinned by a 40-hex COMMIT DIGEST and +// carry a full-semver `# vX.Y.Z` comment. // // NON-LLM, dependency-free (Node stdlib only). No network, no child_process, no eval, no dynamic import. // It enforces the convention this repo states at .github/workflows/gitleaks.yml:11 — "pinning // third-party code by digest, never a floating tag" (cited, not restated — P4). // -// WHY IT EXISTS (P7 — a real, already-observed drift, not a hypothetical): commit ff48077 -// ("chore(deps): bump actions/setup-node from 6 to 7") moved ci.yml's digest ACROSS A MAJOR while -// leaving the trailing comment at `# v6`, and c425edd copied that same stale pattern into -// publish.yml. The divergence survived two later PRs and was found only by a manual audit. The -// major-only comment form is what this checker's `malformed-comment` rule rejects. +// WHY IT EXISTS (P7 — a real, already-observed drift): commit ff48077 ("chore(deps): bump +// actions/setup-node from 6 to 7") moved ci.yml's digest ACROSS A MAJOR while leaving the trailing +// comment at `# v6`, and c425edd copied that stale pattern into publish.yml. It survived two later +// PRs and was found by a manual audit. The major-only comment form is what `malformed-comment` +// rejects. // // WHAT IS GUARANTEED (P0 — floor primitive #3, enum/regex; ARCHITECTURE.md §2): -// the FORM of every ref — a 40-hex digest plus a full-semver comment. -// WHAT IS NOT (named residual, never claimed): -// the TRUTH of the comment. Verifying that `# v7.0.0` really names the commit `8207627…` requires -// `git ls-remote`, and floor scripts are network-free by convention. A well-formed-but-wrong -// comment (digest bumped, `# v6.4.0` left behind) PASSES. This shrinks the recurrence surface; it -// does not close it. +// the FORM of every ref this scanner ENUMERATES — a 40-hex digest plus a full-semver comment. +// WHAT IS NOT (named residuals, never claimed): +// • The TRUTH of the comment. Verifying that `# v7.0.0` names commit 8207627… needs `git +// ls-remote`, and floor scripts are network-free. A well-formed but wrong comment PASSES. +// • The OWNER of the digest. `attacker/checkout@<40hex> # v7.0.1` is fully conforming here; +// binding a digest to an intended owner needs an allowlist policy this does not have. +// • Total YAML fidelity. This is a LINE SCANNER, not a YAML parser (stdlib-only; js-yaml is a +// transitive dep, not ours). It recognises the three `uses:` spellings GitHub users actually +// write — bare key, quoted key, flow mapping — and unrecognised shapes fail TOWARD flagging, +// never toward silence. Exotic YAML (anchors, a line-initial `uses:` inside a `run: |` block +// scalar) can produce a FALSE POSITIVE. That direction is deliberate: noise is recoverable, a +// silent miss is not. // -// Scope note: GitHub executes only `.github/workflows/*.yml|*.yaml` (flat — files in subdirectories -// are not workflows), so the walk is deliberately non-recursive and mirrors that semantics. +// WHAT IT ENUMERATES (both are executed by GitHub, so both are in scope): +// 1. `.github/workflows/*.{yml,yaml}` — non-recursive, mirroring GitHub (files in subdirectories +// of that folder are not workflows). +// 2. `.github/actions/**/action.{yml,yaml}` — LOCAL composite/docker action definitions, walked +// recursively. A composite action's `runs.steps[].uses:` entries are real third-party actions +// running with the job's token. Scanning only workflows made `uses: ./.github/actions/x` a +// laundering path: exempt at the call site, never read at the definition. // // Usage: node .dev/floor/check-action-pins.mjs [targetDir] (default: cwd) // Output: {"checked":,"skipped":,"files":[...],"violations":[{file,line,ref,reason}]} // Exit: 0 clean · 1 >=1 violation // -// Every output field is enum-gated / path-resolved (paths, ints, an enum `reason`). There is NO -// free-text field, so nothing here can carry taint into a downstream decision (P2, fix #1). +// Every output field is enum-gated / path-resolved (paths, ints, an enum `reason`). The `ref` value +// is copied verbatim from the scanned file, so it inherits that file's trust — but NO decision reads +// it: the verdict is `violations.length > 0`, an integer test (P2, fix #1). -import { readFileSync, readdirSync, existsSync, statSync } from "node:fs"; -import { join } from "node:path"; +import { readFileSync, readdirSync, existsSync, lstatSync } from "node:fs"; +import { join, sep } from "node:path"; // A pinned ref: exactly 40 lowercase hex. Uppercase is rejected on purpose — git digests are -// lowercase, and accepting both would make the comparison in a future truth-check ambiguous. +// lowercase, and accepting both would make a future truth-check ambiguous. const DIGEST_RE = /^[0-9a-f]{40}$/; -// A conforming comment: FULL semver. The major-only form (`# v6`) is the ff48077 defect and is -// rejected — that is the entire point of requiring three components. +// A container digest: docker uses sha256:<64 hex>, a different format from a git commit. +const OCI_DIGEST_RE = /^sha256:[0-9a-f]{64}$/; +// A conforming comment: FULL semver. The major-only form (`# v6`) is the ff48077 defect. const SEMVER_COMMENT_RE = /^v\d+\.\d+\.\d+$/; -// `uses:` line, with or without the YAML sequence dash. The ref and any trailing comment are split -// AFTERWARDS rather than captured by two groups here: a `${{ … }}` expression ref contains SPACES, -// so a `(\S+)` ref group silently fails to match the whole line and the ref is never examined at -// all — a fail-open (the line looks like "not a uses: line"). Capturing the remainder wholesale and -// splitting on `#` keeps every `uses:` line in scope. -const USES_RE = /^\s*(?:-\s*)?uses:\s*(.*)$/; + +// Split on ALL THREE line-ending conventions, not just "\n". Splitting on "\n" alone leaves a +// trailing "\r" on every line of a CRLF file, and a line-anchored regex ending in `$` cannot match +// it (`.` does not match a carriage return), so EVERY `uses:` in that file became invisible — +// exit 0, checked:0, byte-identical to a clean repo. That is reachable BY ACCIDENT: `core.autocrlf` +// on Windows produces CRLF on checkout, this repo has no `.gitattributes`, and prettier's globs +// exclude `.github/**`, so nothing else would normalise or reject it. Lone-CR collapses the whole +// file into one line, with the same result. +const LINE_SPLIT_RE = /\r\n|\r|\n/; + +// A `uses:` key ANYWHERE in the line — deliberately NOT anchored to the start. +// +// Anchoring was the second root cause: `steps: [{uses: x@main}]`, `- {with: {…}, uses: x@main}`, +// and a flow sequence on its own line are all valid YAML that GitHub executes, and all three put +// the key somewhere other than line-start-after-an-optional-dash. Each was invisible. +// +// The leading `(?:^|[\s,{[])` is what stops `causes:` / `reuses:` from matching, and capturing to +// the next `,` `}` `]` (rather than a `(\S+)` ref group) is load-bearing: a `${{ … }}` expression +// ref contains SPACES, and a `\S+` group fails to match the line at all — a fail-OPEN. +// +// Deliberate imprecision, in the fail-CLOSED direction: a `run:` line that happens to contain the +// text `uses:` is treated as a ref and reported. That is noise a human can resolve; a missed ref +// is not. +const USES_KEY_RE = /(?:^|[\s,{[])["']?uses["']?\s*:\s*([^,}\]]*)/g; // Reasons are an ENUM, never prose (P5 — membership, not classification). const REASON = { FLOATING: "floating-ref", // the @ref is not a 40-hex digest MISSING_COMMENT: "missing-comment", // digest-pinned but no trailing comment MALFORMED_COMMENT: "malformed-comment", // comment present but not full semver - UNPINNABLE: "unpinnable-ref", // a ${{ }} expression — cannot be pinned at all + UNPINNABLE: "unpinnable-ref", // a ${{ }} expression — no fixed identity to pin + UNPINNED_CONTAINER: "unpinned-container", // docker:// without an @sha256 digest + ESCAPING_LOCAL: "escaping-local-ref", // a ./ ref that climbs out with .. + UNREADABLE: "unreadable-file", // could not stat/read (e.g. a dangling symlink) }; function emit(obj, code) { + // NOT process.exit(): stdout is ASYNC on a pipe, and exiting truncates it. Reproduced before this + // change — 400KB of JSON cut to exactly 65536 bytes through `| cat` and through spawnSync, leaving + // consumers with unparseable output. Setting exitCode lets the write drain and Node exit naturally. process.stdout.write(JSON.stringify(obj) + "\n"); - process.exit(code); + process.exitCode = code; } -// Classify ONE `uses:` ref. Returns a REASON, or null when the ref conforms. -// Exported shape is a pure function of (ref, comment) — no I/O, so the table is trivially testable. +// Classify ONE ref. Returns a REASON, or null when it conforms. +// ORDER IS LOAD-BEARING: the expression test runs FIRST. Previously exemption ran before +// classification, so `./${{ … }}` and `docker://${{ … }}` were skipped instead of flagged — the +// prefix defeated the rule. function classify(ref, comment) { - // An expression ref resolves at run time and has no fixed identity to pin. It is a DISTINCT - // failure from a floating tag (there is no tag to replace with a digest), so it gets its own - // reason rather than being mislabeled `floating-ref`. if (ref.includes("${{")) return REASON.UNPINNABLE; + // Container action. The scheme is NOT a blanket pass: `docker://alpine:latest` is a mutable tag, + // i.e. remote code with no pin at all. Require the OCI digest form. + if (ref.startsWith("docker://")) { + const at = ref.lastIndexOf("@"); + if (at === -1 || !OCI_DIGEST_RE.test(ref.slice(at + 1))) return REASON.UNPINNED_CONTAINER; + return null; // digest-pinned image; there is no semver comment convention for these + } + + // Local action. Exempt only when it is genuinely inside the repo — a `..` segment climbs out to + // a tree this scanner never enumerates. The definition it points at IS scanned (see collectFiles). + if (ref.startsWith("./")) { + return ref.split(/[/\\]/).includes("..") ? REASON.ESCAPING_LOCAL : null; + } + const at = ref.lastIndexOf("@"); - if (at === -1) return REASON.FLOATING; // no ref at all (`uses: actions/checkout`) — unpinned - const rev = ref.slice(at + 1); - if (!DIGEST_RE.test(rev)) return REASON.FLOATING; + if (at === -1) return REASON.FLOATING; // `uses: actions/checkout` — unpinned + if (!DIGEST_RE.test(ref.slice(at + 1))) return REASON.FLOATING; if (comment === undefined || comment === "") return REASON.MISSING_COMMENT; - // Compare the FIRST token only: a trailing note after the version is fine (`# v7.0.1 (pinned)`). + // First token only: a trailing note after the version is fine (`# v7.0.1 (pinned)`). if (!SEMVER_COMMENT_RE.test(comment.split(/\s+/)[0])) return REASON.MALFORMED_COMMENT; return null; } -// Refs that are deliberately OUT OF SCOPE. Each exemption is named and justified — an unexplained -// skip is how a gate quietly stops gating. +// A conforming ref that is exempt from the DIGEST rule but must still be COUNTED, so an audit can +// see that an exemption was used. `skipped` is asserted exactly by the live repo test — an +// exemption can never be a silent hole again. function isExempt(ref) { - // Local/first-party action: nothing third-party to pin. - if (ref.startsWith("./")) return true; - // Container image ref: a different registry with a different digest format (sha256:<64hex>). - // Pinning those is a separate axis; this repo has zero such refs today, so requiring a policy for - // them here would be speculative (P7). Named, not silently swallowed. - if (ref.startsWith("docker://")) return true; + if (ref.startsWith("./")) return true; // escaping ./.. never reaches here (classify rejects it) + if (ref.startsWith("docker://")) return true; // only digest-pinned images reach here return false; } -function main() { - const target = process.argv[2] || process.cwd(); - const dir = join(target, ".github", "workflows"); +const isYaml = (f) => /\.ya?ml$/i.test(f); // case-INsensitive: `CI.YML` was silently dropped before - // A repo with no workflows is not in violation — it is vacuously clean. The CALLER is responsible - // for asserting that something was actually inspected (see check-action-pins.test.mjs, which - // asserts `checked` and the visited file set, never bare exit 0). - if (!existsSync(dir) || !statSync(dir).isDirectory()) { - emit({ checked: 0, skipped: 0, files: [], violations: [] }, 0); +// lstat, not stat: a DANGLING symlink makes stat throw ENOENT, which previously crashed the walk +// inside Array.filter before any JSON was emitted. +function safeLstat(abs) { + try { + return lstatSync(abs); + } catch { + return null; } +} - const files = readdirSync(dir) - .filter((f) => f.endsWith(".yml") || f.endsWith(".yaml")) - .filter((f) => statSync(join(dir, f)).isFile()) - .sort(); +// Enumerate every file GitHub would execute. Returns readable {rel, abs} pairs plus unreadable ones. +function collectFiles(target) { + const found = []; + const unreadable = []; + + // 1. workflows — flat, mirroring what GitHub actually executes. + const wfRel = join(".github", "workflows"); + const wfDir = join(target, wfRel); + if (existsSync(wfDir)) { + let names = []; + try { + names = readdirSync(wfDir).sort(); + } catch { + /* unreadable dir: nothing to enumerate */ + } + for (const name of names) { + if (!isYaml(name)) continue; + const abs = join(wfDir, name); + const st = safeLstat(abs); + if (st === null || st.isSymbolicLink()) { + unreadable.push({ rel: join(wfRel, name), abs }); + continue; + } + if (st.isFile()) found.push({ rel: join(wfRel, name), abs }); + } + } + + // 2. local action definitions — recursive, depth-bounded, symlink-refusing. + const walk = (absDir, relDir, depth) => { + if (depth > 8) return; // bounded: a pathological tree cannot hang the floor + let names = []; + try { + names = readdirSync(absDir).sort(); + } catch { + return; + } + for (const name of names) { + const abs = join(absDir, name); + const rel = join(relDir, name); + const st = safeLstat(abs); + if (st === null) { + unreadable.push({ rel, abs }); + continue; + } + if (st.isSymbolicLink()) continue; // never follow a symlink out of the tree + if (st.isDirectory()) walk(abs, rel, depth + 1); + else if (st.isFile() && /^action\.ya?ml$/i.test(name)) found.push({ rel, abs }); + } + }; + const actRel = join(".github", "actions"); + const actDir = join(target, actRel); + if (existsSync(actDir)) walk(actDir, actRel, 0); + + return { found, unreadable }; +} + +// Pull EVERY `uses:` ref (with its trailing comment) out of one line. Returns [] when the line +// declares none. A line can legitimately carry more than one in flow style — `[{uses: a}, {uses: b}]` +// — and returning only the first would leave the rest invisible, which is the bug class this +// function exists to close. +function parseUses(raw) { + if (raw.trimStart().startsWith("#")) return []; // a commented-out example is not a ref + + const out = []; + USES_KEY_RE.lastIndex = 0; // the regex is /g and shared: reset before each line + let m; + while ((m = USES_KEY_RE.exec(raw)) !== null) { + const rest = m[1]; + // An action ref never contains `#`, so the first `#` begins the comment. + const hash = rest.indexOf("#"); + const comment = hash === -1 ? undefined : rest.slice(hash + 1).trim(); + const ref = rest + .slice(0, hash === -1 ? rest.length : hash) + .trim() + .replace(/^["']|["']$/g, ""); // quotes must not smuggle a floating tag past the digest test + out.push({ ref, comment }); + } + return out; +} + +function main() { + const target = process.argv[2] || process.cwd(); + const { found, unreadable } = collectFiles(target); const violations = []; let checked = 0; let skipped = 0; - for (const file of files) { - const rel = join(".github", "workflows", file); - const lines = readFileSync(join(dir, file), "utf8").split("\n"); + // An entry we cannot read is a VIOLATION, not a skip: "I could not look" must never render as + // "there was nothing to find". + for (const u of unreadable) { + violations.push({ file: u.rel.split(sep).join("/"), line: 0, ref: "", reason: REASON.UNREADABLE }); + } + + for (const f of found) { + const rel = f.rel.split(sep).join("/"); + let lines; + try { + lines = readFileSync(f.abs, "utf8").split(LINE_SPLIT_RE); + } catch { + violations.push({ file: rel, line: 0, ref: "", reason: REASON.UNREADABLE }); + continue; + } lines.forEach((raw, i) => { - // A commented-out example is not a ref. Test the RAW line's first non-whitespace character - // before matching, so `# - uses: foo@v1` never registers. - if (raw.trimStart().startsWith("#")) return; - - const m = USES_RE.exec(raw); - if (m === null) return; - - // Split the remainder into ref + comment. An action ref never contains `#`, so the first `#` - // begins the comment. - const rest = m[1]; - const hash = rest.indexOf("#"); - const comment = hash === -1 ? undefined : rest.slice(hash + 1).trim(); - // Strip surrounding quotes — `uses: "actions/checkout@"` is valid YAML and must not slip - // past the digest test on account of the quote characters. - const ref = rest - .slice(0, hash === -1 ? rest.length : hash) - .trim() - .replace(/^["']|["']$/g, ""); - - // `uses:` with nothing after it is malformed, not absent — fail closed rather than skip. - if (ref === "") { - checked += 1; - violations.push({ - file: rel, - line: i + 1, - ref, - reason: REASON.FLOATING, - }); - return; - } + for (const { ref, comment } of parseUses(raw)) { + // `uses:` with nothing after it is malformed, not absent — fail closed. + if (ref === "") { + checked += 1; + violations.push({ file: rel, line: i + 1, ref, reason: REASON.FLOATING }); + continue; + } - if (isExempt(ref)) { - skipped += 1; - return; - } + const reason = classify(ref, comment); + if (reason !== null) { + checked += 1; + violations.push({ file: rel, line: i + 1, ref, reason }); + continue; + } - checked += 1; - const reason = classify(ref, comment); - if (reason !== null) violations.push({ file: rel, line: i + 1, ref, reason }); + // Conforming. Exempt refs are counted separately so an audit can see the exemption was used. + if (isExempt(ref)) skipped += 1; + else checked += 1; + } }); } - emit({ checked, skipped, files, violations }, violations.length > 0 ? 1 : 0); + emit( + { checked, skipped, files: found.map((f) => f.rel.split(sep).join("/")), violations }, + violations.length > 0 ? 1 : 0, + ); } main(); diff --git a/.dev/floor/check-action-pins.test.mjs b/.dev/floor/check-action-pins.test.mjs index 3871ab8..6340b1a 100644 --- a/.dev/floor/check-action-pins.test.mjs +++ b/.dev/floor/check-action-pins.test.mjs @@ -10,20 +10,37 @@ // property someone has to remember to re-grep: floor.yml's `node --test ".dev/**/*.test.mjs"` // collects this file on every pull_request and every push to main. // -// The ★ tests are load-bearing: -// • a digest with a MAJOR-ONLY comment (`# v6`) is a violation — the exact ff48077 defect, PROVEN -// CAUGHT (a digest bumped across a major while the comment stayed at `# v6`); -// • the live repo asserts `violations: []` AND `checked >= 10` AND that every workflow file on -// disk was visited — never bare exit 0, because exit 0 is ALSO what a checker returns when it -// finds nothing to inspect. A gate that cannot tell "all clean" from "looked nowhere" is not a -// gate. +// The ✱ tests are REGRESSIONS FOR HOLES THAT SHIPPED in the first version of this gate (PR #79). +// Each was reproduced against that version before being fixed, so each one asserts a specific way +// the gate previously reported "clean" while unpinned third-party code would execute: +// ✱ a YAML flow mapping `- {uses: x@main}` → was checked:0, exit 0 (never classified) +// ✱ a quoted key `- "uses": x@v1` → was checked:0, exit 0 (never classified) +// ✱ an uppercase filename `CI.YML` → was files:[], exit 0 (never opened) +// ✱ a mutable image `docker://alpine:latest`→ was skipped:1, exit 0 (blanket scheme exemption) +// ✱ an expression behind a prefix `./${{ … }}` → was skipped:1, exit 0 (exemption ran first) +// ✱ a local composite action's own floating ref → was never walked at all +// ✱ a dangling symlink → crashed with uncaught ENOENT, emitting NO JSON +// ✱ 4000 violations through a pipe → truncated at exactly 65536 bytes +// +// The ✱✱ tests are a SECOND round, found by re-running the sweep against the ✱-hardened gate. Both +// had structural root causes rather than missing cases, which is why the first round missed them: +// ✱✱ CRLF / lone-CR line endings → the file was split on "\n" only, so a trailing "\r" defeated +// every line-anchored match. WHOLE FILE invisible — and reachable BY ACCIDENT via core.autocrlf. +// ✱✱ `steps: [{uses: …}]`, `- {with: …, uses: …}`, `[{uses: …}]` → the key was anchored to line +// start, so any flow position hid it. Two refs on one line hid the second. +// The fixes were to the roots — split on all three line endings, and match the key anywhere — not +// to the individual cases. +// +// The live repo asserts `violations: []` AND `skipped` EXACTLY AND `checked >= 10` AND a +// case-insensitive independent recount of the workflow files — never bare exit 0, because exit 0 is +// ALSO what a checker returns when it finds nothing to inspect. import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readdirSync } from "node:fs"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, readdirSync, symlinkSync } from "node:fs"; import { tmpdir } from "node:os"; const here = dirname(fileURLToPath(import.meta.url)); // .dev/floor @@ -31,22 +48,28 @@ const REPO = join(here, "..", ".."); // repo root const CAP = join(here, "check-action-pins.mjs"); const DIGEST = "3d3c42e5aac5ba805825da76410c181273ba90b1"; // a real 40-hex digest (actions/checkout v7.0.1) +const OCI = "sha256:" + "a".repeat(64); // a well-formed OCI image digest function run(targetDir) { - return spawnSync(process.execPath, [CAP, targetDir], { encoding: "utf8" }); + return spawnSync(process.execPath, [CAP, targetDir], { encoding: "utf8", maxBuffer: 32 * 1024 * 1024 }); } function json(r) { return JSON.parse(r.stdout); } - -// Build a scratch repo whose .github/workflows/wf.yml contains the given `uses:` step body. +function scratch() { + return mkdtempSync(join(tmpdir(), "pharn-pins-")); +} +// Build a scratch repo whose .github/workflows/ contains the given `uses:` step body. function repoWith(body, name = "wf.yml") { - const root = mkdtempSync(join(tmpdir(), "pharn-pins-")); + const root = scratch(); const dir = join(root, ".github", "workflows"); mkdirSync(dir, { recursive: true }); writeFileSync(join(dir, name), `name: t\non: [push]\njobs:\n j:\n steps:\n${body}\n`); return root; } +function reasons(r) { + return json(r).violations.map((v) => v.reason); +} // --- conforming ------------------------------------------------------------------------------ @@ -54,9 +77,8 @@ test("conforming ref (digest + full semver) → exit 0, no violations", () => { const root = repoWith(` - uses: actions/checkout@${DIGEST} # v7.0.1`); const r = run(root); assert.equal(r.status, 0); - const j = json(r); - assert.deepEqual(j.violations, []); - assert.equal(j.checked, 1); + assert.deepEqual(json(r).violations, []); + assert.equal(json(r).checked, 1); rmSync(root, { recursive: true, force: true }); }); @@ -79,14 +101,12 @@ test("a ref without the YAML sequence dash (codeql style) is still checked", () // --- violations ------------------------------------------------------------------------------ test("floating MAJOR tag (@v7) → exit 1, reason floating-ref", () => { - const root = repoWith(" - uses: actions/setup-node@v7"); - const r = run(root); + const r = run(repoWith(" - uses: actions/setup-node@v7")); assert.equal(r.status, 1); - assert.equal(json(r).violations[0].reason, "floating-ref"); - rmSync(root, { recursive: true, force: true }); + assert.deepEqual(reasons(r), ["floating-ref"]); }); -test("floating PATCH tag (@v7.0.1) → exit 1, reason floating-ref — the pin-floor-actions start state", () => { +test("floating PATCH tag (@v7.0.1) → exit 1, floating-ref — the pin-floor-actions start state", () => { const root = repoWith(" - uses: actions/checkout@v7.0.1"); const r = run(root); assert.equal(r.status, 1); @@ -97,113 +117,294 @@ test("floating PATCH tag (@v7.0.1) → exit 1, reason floating-ref — the pin-f }); test("no @ref at all → exit 1, reason floating-ref", () => { - const root = repoWith(" - uses: actions/checkout"); - const r = run(root); + assert.deepEqual(reasons(run(repoWith(" - uses: actions/checkout"))), ["floating-ref"]); +}); + +test("digest with NO comment → exit 1, reason missing-comment", () => { + assert.deepEqual(reasons(run(repoWith(` - uses: actions/checkout@${DIGEST}`))), ["missing-comment"]); +}); + +test("★ digest with a MAJOR-ONLY comment (# v6) → malformed-comment (the ff48077 defect)", () => { + assert.deepEqual(reasons(run(repoWith(` - uses: actions/setup-node@${DIGEST} # v6`))), [ + "malformed-comment", + ]); +}); + +test("a ${{ }} expression ref → unpinnable-ref (NOT mislabeled floating-ref)", () => { + assert.deepEqual(reasons(run(repoWith(" - uses: ${{ matrix.action }}"))), ["unpinnable-ref"]); +}); + +test("a QUOTED ref is unwrapped before the digest test", () => { + const ok = repoWith(` - uses: "actions/checkout@${DIGEST}" # v7.0.1`); + assert.equal(run(ok).status, 0); + rmSync(ok, { recursive: true, force: true }); + assert.deepEqual(reasons(run(repoWith(` - uses: 'actions/checkout@v7'`))), ["floating-ref"]); +}); + +test("`uses:` with nothing after it fails closed as a violation, not a skip", () => { + const r = run(repoWith(" - uses:")); assert.equal(r.status, 1); - assert.equal(json(r).violations[0].reason, "floating-ref"); + assert.equal(json(r).checked, 1); + assert.deepEqual(reasons(r), ["floating-ref"]); +}); + +test("39-hex, 41-hex and UPPERCASE refs are all floating-ref (boundary)", () => { + for (const bad of [DIGEST.slice(0, 39), DIGEST + "a", DIGEST.toUpperCase()]) { + const r = run(repoWith(` - uses: actions/checkout@${bad} # v7.0.1`)); + assert.equal(r.status, 1, `expected violation for ${bad}`); + assert.deepEqual(reasons(r), ["floating-ref"]); + } +}); + +// --- ✱✱ REGRESSIONS: line-ending and flow-position holes ------------------------------------- +// These survived the FIRST round of hardening and were found by a second adversarial sweep. Both +// root causes were structural: splitting only on "\n", and anchoring the key to line start. + +// Build a scratch repo with an explicit line ending, so CRLF/CR are exercised literally. +function repoWithEol(refLine, eol) { + const root = scratch(); + const dir = join(root, ".github", "workflows"); + mkdirSync(dir, { recursive: true }); + const doc = ["name: t", "on: [push]", "jobs:", " j:", " steps:", refLine, ""].join(eol); + writeFileSync(join(dir, "w.yml"), doc); + return root; +} + +test("✱✱ CRLF line endings do not hide a ref (whole file was invisible: checked:0, exit 0)", () => { + const root = repoWithEol(" - uses: evil/action@v1", "\r\n"); + const r = run(root); + assert.equal(r.status, 1, "a CRLF workflow must be scanned exactly like an LF one"); + assert.equal(json(r).checked, 1); + assert.deepEqual(reasons(r), ["floating-ref"]); rmSync(root, { recursive: true, force: true }); }); -test("digest with NO comment → exit 1, reason missing-comment", () => { - const root = repoWith(` - uses: actions/checkout@${DIGEST}`); +test("✱✱ CRLF does not hide a CONFORMING ref either (no false positive from the \\r)", () => { + const root = repoWithEol(` - uses: actions/checkout@${DIGEST} # v7.0.1`, "\r\n"); const r = run(root); - assert.equal(r.status, 1); - assert.equal(json(r).violations[0].reason, "missing-comment"); + assert.equal(r.status, 0, `trailing \\r must not corrupt the comment: ${r.stdout}`); + assert.equal(json(r).checked, 1); rmSync(root, { recursive: true, force: true }); }); -// ★ the historical defect -test("★ digest with a MAJOR-ONLY comment (# v6) → exit 1, reason malformed-comment (the ff48077 defect)", () => { - const root = repoWith(` - uses: actions/setup-node@${DIGEST} # v6`); +test("✱✱ lone-CR (classic Mac) line endings do not collapse the file into one invisible line", () => { + const root = repoWithEol(" - uses: evil/action@v1", "\r"); const r = run(root); assert.equal(r.status, 1); - assert.equal(json(r).violations[0].reason, "malformed-comment"); + assert.deepEqual(reasons(r), ["floating-ref"]); rmSync(root, { recursive: true, force: true }); }); -test("a ${{ }} expression ref → exit 1, reason unpinnable-ref (NOT mislabeled floating-ref)", () => { - const root = repoWith(" - uses: ${{ matrix.action }}"); +test("✱✱ a flow SEQUENCE on the steps line — `steps: [{uses: x@v1}]` — is classified", () => { + const root = scratch(); + const dir = join(root, ".github", "workflows"); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, "w.yml"), + "name: t\non: [push]\njobs:\n j:\n steps: [{uses: evil/action@v1}]\n", + ); const r = run(root); assert.equal(r.status, 1); - assert.equal(json(r).violations[0].reason, "unpinnable-ref"); + assert.equal(json(r).checked, 1); + assert.deepEqual(reasons(r), ["floating-ref"]); rmSync(root, { recursive: true, force: true }); }); -test("a QUOTED ref is unwrapped before the digest test (quotes must not smuggle a floating tag past)", () => { - const ok = repoWith(` - uses: "actions/checkout@${DIGEST}" # v7.0.1`); - assert.equal(run(ok).status, 0); - rmSync(ok, { recursive: true, force: true }); +test("✱✱ `uses` NOT first in a flow mapping — `- {with: {x: 1}, uses: x@v1}` — is classified", () => { + const r = run(repoWith(" - {with: {x: 1}, uses: evil/action@v1}")); + assert.equal(r.status, 1); + assert.equal(json(r).checked, 1); + assert.deepEqual(reasons(r), ["floating-ref"]); +}); - const bad = repoWith(` - uses: 'actions/checkout@v7'`); - const r = run(bad); +test("✱✱ a flow sequence on its own line — `[{uses: x@v1}]` — is classified", () => { + const r = run(repoWith(" [{uses: evil/action@v1}]")); assert.equal(r.status, 1); - assert.equal(json(r).violations[0].reason, "floating-ref"); - rmSync(bad, { recursive: true, force: true }); + assert.deepEqual(reasons(r), ["floating-ref"]); }); -test("`uses:` with nothing after it fails closed as a violation, not a skip", () => { - const root = repoWith(" - uses:"); +test("✱✱ TWO refs on one line are both counted (returning only the first left the rest invisible)", () => { + const root = scratch(); + const dir = join(root, ".github", "workflows"); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, "w.yml"), + "name: t\non: [push]\njobs:\n j:\n steps: [{uses: a/b@v1}, {uses: c/d@v2}]\n", + ); const r = run(root); assert.equal(r.status, 1); - const j = json(r); - assert.equal(j.checked, 1); - assert.equal(j.violations[0].reason, "floating-ref"); + assert.equal(json(r).checked, 2, "both refs must be counted"); + assert.deepEqual(reasons(r), ["floating-ref", "floating-ref"]); rmSync(root, { recursive: true, force: true }); }); -// --- digest boundary cases ------------------------------------------------------------------- +test("a word merely ENDING in `uses:` (causes:, reuses:) is not a ref — the un-anchored match is bounded", () => { + const root = repoWith(" - run: echo causes: nothing"); + const r = run(root); + assert.equal(r.status, 0); + assert.equal(json(r).checked, 0); + rmSync(root, { recursive: true, force: true }); +}); -test("39-hex, 41-hex and UPPERCASE refs are all floating-ref (boundary)", () => { - for (const bad of [DIGEST.slice(0, 39), DIGEST + "a", DIGEST.toUpperCase()]) { - const root = repoWith(` - uses: actions/checkout@${bad} # v7.0.1`); - const r = run(root); - assert.equal(r.status, 1, `expected violation for ${bad}`); - assert.equal(json(r).violations[0].reason, "floating-ref"); - rmSync(root, { recursive: true, force: true }); - } +// --- ✱ REGRESSIONS: refs the shipped gate never classified --------------------------------------- + +test("✱ YAML FLOW MAPPING `- {uses: x@main}` is classified (shipped gate reported checked:0, exit 0)", () => { + const r = run(repoWith(" - {uses: actions/checkout@main}")); + assert.equal(r.status, 1); + assert.deepEqual(reasons(r), ["floating-ref"]); + assert.equal(json(r).checked, 1, "the ref must be COUNTED, not merely unmatched"); }); -// --- skips and exemptions ---------------------------------------------------------------------- +test("✱ a conforming flow-mapping ref passes (the fix must not blanket-fail the form)", () => { + const root = repoWith(` - {uses: actions/checkout@${DIGEST}, with: {fetch-depth: 0}}`); + const r = run(root); + assert.equal(r.status, 1, "no comment on a flow mapping → missing-comment, not a crash"); + assert.deepEqual(reasons(r), ["missing-comment"]); + rmSync(root, { recursive: true, force: true }); +}); -test("a commented-out example (# - uses: foo@v1) is not a ref", () => { - const root = repoWith(" # - uses: actions/checkout@v1"); +test('✱ QUOTED KEY `- "uses": x@v1` is classified (shipped gate reported checked:0, exit 0)', () => { + const r = run(repoWith(` - "uses": actions/checkout@v1`)); + assert.equal(r.status, 1); + assert.deepEqual(reasons(r), ["floating-ref"]); + assert.equal(json(r).checked, 1); +}); + +test("✱ UPPERCASE extension CI.YML is opened (shipped gate reported files:[], exit 0)", () => { + const root = repoWith(" - uses: actions/checkout@v1", "CI.YML"); + const r = run(root); + assert.equal(r.status, 1); + assert.deepEqual(json(r).files, [".github/workflows/CI.YML"]); + assert.deepEqual(reasons(r), ["floating-ref"]); + rmSync(root, { recursive: true, force: true }); +}); + +// --- ✱ REGRESSIONS: exemptions that were silent --------------------------------------------------- + +test("✱ docker://alpine:latest → unpinned-container (shipped gate reported skipped:1, exit 0)", () => { + const r = run(repoWith(" - uses: docker://alpine:latest")); + assert.equal(r.status, 1); + assert.deepEqual(reasons(r), ["unpinned-container"]); +}); + +test("✱ a DIGEST-pinned image still passes, and is counted as skipped (audit trail)", () => { + const root = repoWith(` - uses: docker://alpine@${OCI}`); const r = run(root); assert.equal(r.status, 0); - const j = json(r); - assert.equal(j.checked, 0); - assert.deepEqual(j.violations, []); + assert.equal(json(r).skipped, 1); + assert.equal(json(r).checked, 0); rmSync(root, { recursive: true, force: true }); }); -test("a local ./ action is exempt (counted as skipped, not checked)", () => { +test("✱ `./${{ }}` is unpinnable-ref, not skipped (exemption used to run before classify)", () => { + const r = run(repoWith(" - uses: ./${{ github.event.inputs.dir }}")); + assert.equal(r.status, 1); + assert.deepEqual(reasons(r), ["unpinnable-ref"]); +}); + +test("✱ `docker://${{ }}` is unpinnable-ref, not skipped", () => { + const r = run(repoWith(" - uses: docker://${{ env.IMG }}")); + assert.equal(r.status, 1); + assert.deepEqual(reasons(r), ["unpinnable-ref"]); +}); + +test("✱ a ./ ref that climbs out with .. is escaping-local-ref, not exempt", () => { + const r = run(repoWith(" - uses: ./../../vendor/thirdparty")); + assert.equal(r.status, 1); + assert.deepEqual(reasons(r), ["escaping-local-ref"]); +}); + +test("a plain local ./ action is still exempt, and counted as skipped", () => { const root = repoWith(" - uses: ./.github/actions/setup"); const r = run(root); assert.equal(r.status, 0); - const j = json(r); - assert.equal(j.skipped, 1); - assert.equal(j.checked, 0); + assert.equal(json(r).skipped, 1); + assert.equal(json(r).checked, 0); rmSync(root, { recursive: true, force: true }); }); -test("a docker:// image ref is exempt (named out-of-axis, not silently passed as checked)", () => { - const root = repoWith(" - uses: docker://alpine:3.18"); +// --- ✱ REGRESSION: local composite actions were never walked ------------------------------------- + +test("✱ a local composite action's OWN floating ref is caught (shipped gate never opened action.yml)", () => { + const root = repoWith(" - uses: ./.github/actions/wrap"); + const act = join(root, ".github", "actions", "wrap"); + mkdirSync(act, { recursive: true }); + writeFileSync( + join(act, "action.yml"), + "name: wrap\nruns:\n using: composite\n steps:\n - uses: attacker/evil@main\n", + ); + const r = run(root); + assert.equal(r.status, 1, "the laundered ref must be caught"); + assert.deepEqual(reasons(r), ["floating-ref"]); + assert.ok( + json(r).files.includes(".github/actions/wrap/action.yml"), + `action.yml must be enumerated, got ${JSON.stringify(json(r).files)}`, + ); + rmSync(root, { recursive: true, force: true }); +}); + +test("a local composite action with a conforming ref passes", () => { + const root = repoWith(" - uses: ./.github/actions/wrap"); + const act = join(root, ".github", "actions", "wrap"); + mkdirSync(act, { recursive: true }); + writeFileSync( + join(act, "action.yml"), + `name: wrap\nruns:\n using: composite\n steps:\n - uses: actions/checkout@${DIGEST} # v7.0.1\n`, + ); const r = run(root); assert.equal(r.status, 0); - const j = json(r); - assert.equal(j.skipped, 1); - assert.equal(j.checked, 0); + assert.equal(json(r).checked, 1); // the inner ref + assert.equal(json(r).skipped, 1); // the ./ call site + rmSync(root, { recursive: true, force: true }); +}); + +// --- ✱ REGRESSION: unreadable input -------------------------------------------------------------- + +test("✱ a dangling symlink is a violation with JSON on stdout (shipped gate crashed with ENOENT)", () => { + const root = scratch(); + const dir = join(root, ".github", "workflows"); + mkdirSync(dir, { recursive: true }); + symlinkSync(join(root, "nope-does-not-exist"), join(dir, "dangling.yml")); + const r = run(root); + assert.equal(r.status, 1); + assert.ok(r.stdout.length > 0, "must still emit JSON, not die with a stack trace"); + assert.deepEqual(reasons(r), ["unreadable-file"]); + rmSync(root, { recursive: true, force: true }); +}); + +// --- ✱ REGRESSION: output truncation ------------------------------------------------------------- + +test("✱ a large report survives a PIPE intact (shipped gate truncated at exactly 65536 bytes)", () => { + const root = scratch(); + const dir = join(root, ".github", "workflows"); + mkdirSync(dir, { recursive: true }); + const N = 4000; + const steps = Array.from({ length: N }, (_, i) => ` - uses: some/action-${i}@v1`).join("\n"); + writeFileSync(join(dir, "big.yml"), `name: t\non: [push]\njobs:\n j:\n steps:\n${steps}\n`); + const r = run(root); // spawnSync captures stdout through a PIPE — the truncation-prone path + assert.equal(r.status, 1); + assert.ok(r.stdout.length > 65536, `expected >64KB of JSON, got ${r.stdout.length} bytes`); + const j = json(r); // would throw "Unexpected end of JSON input" if truncated + assert.equal(j.violations.length, N); rmSync(root, { recursive: true, force: true }); }); -// --- vacuous / structural ------------------------------------------------------------------------ +// --- skips, comments, vacuity --------------------------------------------------------------------- -test("a repo with no .github/workflows/ is vacuously clean — exit 0, checked 0, no crash", () => { - const root = mkdtempSync(join(tmpdir(), "pharn-pins-empty-")); +test("a commented-out example (# - uses: foo@v1) is not a ref", () => { + const root = repoWith(" # - uses: actions/checkout@v1"); const r = run(root); assert.equal(r.status, 0); - const j = json(r); - assert.equal(j.checked, 0); - assert.deepEqual(j.files, []); + assert.equal(json(r).checked, 0); + rmSync(root, { recursive: true, force: true }); +}); + +test("a repo with no .github/ is vacuously clean — exit 0, checked 0, no crash", () => { + const root = scratch(); + const r = run(root); + assert.equal(r.status, 0); + assert.equal(json(r).checked, 0); + assert.deepEqual(json(r).files, []); rmSync(root, { recursive: true, force: true }); }); @@ -216,11 +417,9 @@ test("multiple workflow files are all visited, and every violation resolves to f ); const r = run(root); assert.equal(r.status, 1); - const j = json(r); - assert.deepEqual(j.files, ["a.yml", "b.yml"]); - assert.equal(j.checked, 2); - assert.equal(j.violations.length, 1); - assert.equal(j.violations[0].file, join(".github", "workflows", "b.yml")); + assert.deepEqual(json(r).files, [".github/workflows/a.yml", ".github/workflows/b.yml"]); + assert.equal(json(r).checked, 2); + assert.equal(json(r).violations[0].file, ".github/workflows/b.yml"); rmSync(root, { recursive: true, force: true }); }); @@ -234,17 +433,26 @@ test("★ THIS repo: every workflow action ref is digest-pinned with a full-semv // Assert the CONTENT, not just the exit code — exit 0 is also what "found nothing" returns. assert.deepEqual(j.violations, [], `unpinned action ref(s): ${JSON.stringify(j.violations, null, 2)}`); - // Anti-vacuity 1: it must actually have inspected refs. 10 is the live count at the time this - // gate landed; the assertion is a LOWER BOUND, so adding workflows never breaks it, while a - // walker that silently stops finding files does. + // Anti-vacuity 1: it must actually have inspected refs. A LOWER BOUND, so adding workflows never + // breaks it, while a walker that silently stops finding files does. assert.ok(j.checked >= 10, `expected >=10 refs inspected, got ${j.checked}`); - // Anti-vacuity 2: the visited file set must match the tree, counted INDEPENDENTLY of the - // checker's own walk — so a bug in that walk cannot hide behind its own report. + // Anti-vacuity 2: EXACT skipped count. This repo uses no local or container actions, so any + // exemption appearing here is a change that must be looked at, not absorbed. Without this, a ref + // could migrate from `checked` into the exempt bucket and leave no trace. + assert.equal(j.skipped, 0, `an exemption is now in use (skipped=${j.skipped}) — review it deliberately`); + + // Anti-vacuity 3: the visited workflow set must match the tree, recounted independently — and + // case-INSENSITIVELY, because a case-sensitive recount would replicate the very filter bug that + // let `CI.YML` disappear from both sides at once. const onDisk = readdirSync(join(REPO, ".github", "workflows")) - .filter((f) => f.endsWith(".yml") || f.endsWith(".yaml")) - .sort(); - assert.deepEqual(j.files, onDisk); + .filter((f) => /\.ya?ml$/i.test(f)) + .sort() + .map((f) => `.github/workflows/${f}`); + assert.deepEqual( + j.files.filter((f) => f.startsWith(".github/workflows/")), + onDisk, + ); assert.equal(r.status, 0); }); diff --git a/.pharn/writes-scope.json b/.pharn/writes-scope.json index ca7e2b0..fd255a1 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/floor-gate-action-pins/SHIP.md" + ".dev/features/harden-action-pin-coverage/REVIEW.md" ], - "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-08-10T09:03:04.268Z" + "set_by": ".claude/commands/pharn-dev-review.md", + "set_at": "2026-08-10T09:51:03.659Z" }