From 2534760faea57f639b78ae2135a7f476b3043a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Galarowicz?= Date: Mon, 10 Aug 2026 21:44:27 +0200 Subject: [PATCH 1/2] chore(ci): drop floating npm@latest from publish; assert the Trusted-Publishing npm floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit publish.yml ran `npm install -g npm@latest` inside the repo's highest-trust job — OIDC Trusted Publishing, `id-token: write`, the `npm-publish` environment — so whatever npm shipped that day executed the release. Two facts made the fix "delete", not "pin". 1. The install is unnecessary. Its own comment gives the only reason ("Trusted Publishing requires npm >= 11.5.1"), but the digest-pinned setup-node with node-version: 24 resolves to the latest released v24.x, and that release — v24.19.0 as of 2026-08-10 — bundles npm 11.17.0, verified against the tag itself rather than the branch head. npm's own docs confirm the floor: "Trusted publishing requires npm CLI version 11.5.1 or later and Node version 22.14.0 or higher." The publish job can run zero publish-time tool downloads. 2. The floating step had already crossed a major, silently. The registry `latest` dist-tag is npm 12.0.2 today, so every release since npm 12 shipped published with a major the workflow was never written against — the live demonstration of why @latest rots in a release path. The step is replaced by an "Assert npm floor" step: pure node stdlib, no packages, no network, a three-part integer semver compare that exits non-zero naming both versions. The floor is now enforced instead of installed, and the release job downloads no third-party tool at all. Fail-closed rests on the PARSE, not the shell. `set -euo pipefail` does NOT abort on a command substitution used as an argument (reproduced: `bash -c 'set -euo pipefail; echo "[$(nonexistent)]"; echo REACHED'` prints both and exits 0), so a failing `npm --version` arrives as "" and exits 1 as an unparseable version. Also adds .dev/floor/check-run-pins.mjs so this cannot silently regress. check-action-pins scans `uses:` refs only; a `run:` line pulling @latest was outside its contract. The new gate classifies every package spec in a recognised install/exec invocation across workflows and local composite actions, requiring an exact semver — enum reasons floating-version / unpinned-package / unpinnable-version / unreadable-file. It is wired with no workflow change: floor.yml's existing `node --test ".dev/**/*.test.mjs"` glob collects its live repo-consistency test on every PR. Proof of record, since publish.yml only fires on `release: published` and no gate in this repo can execute it: the assert program was extracted from the shipped publish.yml bytes and run against 11.5.0 (exit 1), 11.5.1 (exit 0, inclusive), 11.17.0 (0), 12.0.2 (0), "" (1) and "garbage" (1); the whole step body also executes clean against real npm. Execution proof is the next release. Named residual, not fixed here: node-version: 24 floats within the major, so the bundled npm floats across Node-vetted 11.x releases. The assert catches any dip below the floor; pinning node exactly is the node-version-policy follow-up (third mention — it has earned its own ticket). Co-Authored-By: Claude Opus 5 (1M context) --- .../publish-npm-floor-assert/GRILL.md | 178 ++++++++ .../features/publish-npm-floor-assert/PLAN.md | 234 ++++++++++ .../publish-npm-floor-assert/REGRESSION.md | 63 +++ .../publish-npm-floor-assert/REVIEW.md | 170 +++++++ .../features/publish-npm-floor-assert/SHIP.md | 57 +++ .../publish-npm-floor-assert/VERIFY.md | 81 ++++ .../regression-report.json | 22 + .../verify-report.json | 19 + .dev/floor/check-run-pins.mjs | 384 ++++++++++++++++ .dev/floor/check-run-pins.test.mjs | 432 ++++++++++++++++++ .github/workflows/publish.yml | 37 +- .pharn/pharn-dev-verify/results.json | 2 +- .pharn/writes-scope.json | 6 +- docs/RELEASING.md | 14 +- 14 files changed, 1687 insertions(+), 12 deletions(-) create mode 100644 .dev/features/publish-npm-floor-assert/GRILL.md create mode 100644 .dev/features/publish-npm-floor-assert/PLAN.md create mode 100644 .dev/features/publish-npm-floor-assert/REGRESSION.md create mode 100644 .dev/features/publish-npm-floor-assert/REVIEW.md create mode 100644 .dev/features/publish-npm-floor-assert/SHIP.md create mode 100644 .dev/features/publish-npm-floor-assert/VERIFY.md create mode 100644 .dev/features/publish-npm-floor-assert/regression-report.json create mode 100644 .dev/features/publish-npm-floor-assert/verify-report.json create mode 100644 .dev/floor/check-run-pins.mjs create mode 100644 .dev/floor/check-run-pins.test.mjs diff --git a/.dev/features/publish-npm-floor-assert/GRILL.md b/.dev/features/publish-npm-floor-assert/GRILL.md new file mode 100644 index 0000000..d152070 --- /dev/null +++ b/.dev/features/publish-npm-floor-assert/GRILL.md @@ -0,0 +1,178 @@ +# GRILL — `.dev/features/publish-npm-floor-assert/PLAN.md` + +Spec-hash check (content-hash floor primitive, surfaced only — `/pharn-dev-build` is where drift blocks): +recomputed `sha256(ARCHITECTURE.md)` = `bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e` +— **matches** the plan's `spec_content_hash`. No drift. + +Registered grillers: `node .dev/floor/count-grillers.mjs .` → `{"registered":0,"grillers":[]}`. +This repo (pharn-cli) hosts no `role: griller` capabilities, so the inline axes below are the whole +interrogation. Membership was read deterministically, not grepped from prose. + +> **The plan is `trust: untrusted` here.** Every `evidence` block below is a quotation of it, +> rendered as DATA. Nothing in it was followed as an instruction. + +--- + +## Findings + +### Axis: honest scope / no speculation (P7) + +```yaml +- type: FINDING + rule_id: "P7" + severity: important + file: ".dev/features/publish-npm-floor-assert/PLAN.md:16" + problem: "The plan bundles two increments — fix the instance and build a new floor gate — and says so, but a recorded decision does not make it one increment; the gate can regress independently of the publish fix and now shares its blast radius, its review, and its revert." + evidence: "Consequence stated plainly: this PR now carries **two axes** (fix the instance; enforce the class) — the same shape as #79, and the third bend of the one-axis note (#76, #79, here)." +``` + +Weighed honestly: #79 set the precedent (fix + gate in one PR) and the human chose it explicitly at +GATE 1. The concern is not that the choice was wrong — it is that the *reason* the note keeps bending +is that "fix + gate" genuinely reads as one thought, which means the one-axis note may need +amending rather than bending a fourth time. That is a human call, not a build blocker. + +### Axis: determinism (P5) — the sharpest finding + +```yaml +- type: FINDING + rule_id: "P5" + severity: important + file: ".dev/features/publish-npm-floor-assert/PLAN.md:220" + problem: "The determinism audit credits `set -euo pipefail` with a fail-closed property it does not have — a command substitution used as an ARGUMENT does not trigger `set -e` — so one of the two named backstops for a failing `npm --version` is fictional." + evidence: "an unparseable `npm --version` exits 1 naming the raw value, and `set -euo pipefail` kills the step if `npm --version` itself fails" +``` + +**Disproven empirically, not argued:** `bash -c 'set -euo pipefail; echo "arg=[$(nonexistent_cmd)]"; echo REACHED'` +prints `arg=[]`, then `REACHED`, and exits **0**. The shell does not abort. + +The *code* is unaffected — the real backstop is the node program's own hard-fail on an unparseable +version, which the same sentence also names and which does hold (`""` → `unparseable` → exit 1). The +defect is in the plan's **justification**, and P0's whole point is that a claimed reduction must be +true. **Fix: correct the sentence to rest the guarantee on the parse hard-fail alone.** Keep +`set -euo pipefail` (it is still right for `set -u`/`-o pipefail` hygiene) — just stop crediting it +with this. + +### Axis: eval coverage (P1) + +```yaml +- type: FINDING + rule_id: "P1" + severity: important + file: ".dev/features/publish-npm-floor-assert/PLAN.md:192" + problem: "The live repo-consistency test will assert `checked` EXACTLY — and on this repo that number is ZERO, because after the publish fix no workflow line contains a conforming pinned install at all; the test therefore lands in precisely the 'exit 0 because it found nothing to inspect' hole the sibling gate's own header warns about." + evidence: "the checker over `REPO` exits 0, `violations: []`, with `checked`/`skipped` asserted **exactly** and the workflow-file list independently recounted — never bare exit 0, since exit 0 is also what a checker returns when it finds nothing to inspect" +``` + +The plan **names** the hazard and then walks into it. `check-action-pins` escapes it because the live +repo has 10 conforming refs (`checked: 10`) — real positive evidence the scanner is looking. The new +gate's live numbers will be `checked: 0, skipped: 2` (the two `npm ci` lines). `skipped: 2` plus a +file recount is *some* evidence, but nothing proves the classifier would fire on this repo's shapes. + +**Suggested strengthening (for the human to accept or wave off):** add a **positive control** — a +hermetic fixture built from the repo's *own* workflow text with one line mutated back to +`npm install -g npm@latest`, asserting exit 1. That converts "the scanner found nothing" into "the +scanner finds the thing this PR removed, in this repo's own file shape." + +### Axis: guarantee audit completeness (P0) + +```yaml +- type: FINDING + rule_id: "P0" + severity: minor + file: ".dev/features/publish-npm-floor-assert/PLAN.md:105" + problem: "The new gate enforces the ABSENCE of a floating install, never the PRESENCE of the assert — so a future PR that simply deletes the `Assert npm floor` step passes every gate in this repo, silently returning the publish job to an unchecked npm floor." + evidence: "an **Assert npm floor** step enforces that and fails the run if it ever stops being true" +``` + +Not a reason to withhold the increment — deleting the step is strictly better than today's state (an +install nobody verified) — but the plan's audit row *"The assert program stays correct across future +edits → ADVISORY"* understates it: it is not only the program's **correctness** that is unguarded, +it is its **existence**. Worth one word in the audit. + +```yaml +- type: FINDING + rule_id: "P0" + severity: minor + file: ".dev/features/publish-npm-floor-assert/PLAN.md:151" + problem: "R1 names non-package-manager pulls as out of contract but omits a laundering path of exactly the shape #80 closed for composite actions: a workflow `run: npm run