From e9d4de1cddfd64975e4917abd2488c8a1c8b8925 Mon Sep 17 00:00:00 2001 From: Christian Kaltenbach Date: Fri, 14 Aug 2026 14:04:25 +0200 Subject: [PATCH] PFM-ISSUE-34453 - github-actions: correct the mitigation's rationale - the prefix drop is an npm regression Wording and documentation only. No predicate, exit code or control flow changes; bats stays 74/74 and no test asserts the prose. A probe on a runner and locally (cplace-paw-fe PR #188, one-package fixture) measured the prefix drop as npm-version-dependent, and in the direction nobody had assumed: npm 10.2.4 (node 18.19.1 - what every pipeline pins today) un-normalized entry, no mitigation -> installs, served BY THE PROXY; the rewrite preserves the registry's path prefix npm 11.3.0 (developer machines; any runner on node 24) un-normalized entry, no mitigation -> E404, prefix dropped So it is a regression in newer npm, not an old bug since fixed. Nothing is failing in CI today, and everything un-normalized fails the moment runners move to node 24 - which makes this work a prerequisite for that migration rather than a cleanup after it. And replace-registry-host=never fetches those entries from the registry the lockfile names, outside the proxy, on BOTH versions (proxy fetches: 0). On npm 11 that buys compatibility; on npm 10 it buys nothing and is the only reason traffic leaves the proxy. The advisory therefore stopped claiming "they install today only because use-npmrc sets replace-registry-host=never" - false on the npm every runner currently uses, and about to be replicated to ~41 repositories. design.md's "broken today" is struck through and corrected with the measurements; the use-npmrc comment, the README and the overview carry the same qualifier. The removal criterion is unchanged, but its reason is: the flag comes out when no lockfile reports foreign entries because it then has nothing to act on, not because removing it would otherwise break something. Co-Authored-By: Claude Opus 5 (1M context) --- .github/actions/use-npmrc/action.yml | 12 +++++-- .../design.md | 35 +++++++++++++++++-- .../overview.html | 28 ++++++++++----- .../overview.md | 35 +++++++++++++------ tools/scripts/lockfile/README.md | 24 +++++++++---- .../scripts/lockfile/warn-foreign-registry.sh | 27 ++++++++------ 6 files changed, 120 insertions(+), 41 deletions(-) diff --git a/.github/actions/use-npmrc/action.yml b/.github/actions/use-npmrc/action.yml index 893cd61..40fa562 100644 --- a/.github/actions/use-npmrc/action.yml +++ b/.github/actions/use-npmrc/action.yml @@ -15,9 +15,15 @@ runs: echo "$DOT_NPMRC" > ~/.npmrc # PFM-ISSUE-34453 mitigation. npm's default `replace-registry-host=npmjs` # rewrites a lockfile's registry.npmjs.org URLs onto the configured - # registry and DROPS that registry's path prefix, producing an E404 that - # is masked as *** because the prefix is the JFROG_URL secret. - # `never` makes npm fetch each `resolved` URL verbatim instead. + # registry. npm 10.2.4 does that correctly and the entry resolves through + # the proxy; npm 11.3.0 DROPS that registry's path prefix, producing an + # E404 masked as *** because the prefix is the JFROG_URL secret. Measured + # 2026-08-14 on a runner and locally with the same one-package fixture. + # + # `never` makes npm fetch each `resolved` URL verbatim, which survives + # both versions - at a cost: an entry still on npmjs is then fetched FROM + # npmjs, outside the proxy. This line buys npm-11 compatibility and + # spends proxy routing; normalizing the lockfile buys both. # # This is a no-op on a lockfile that already resolves entirely through # the proxy, so it is safe to leave in place, and it is removable once diff --git a/specs/2026-08-10_normalize-package-lock-resolved-urls/design.md b/specs/2026-08-10_normalize-package-lock-resolved-urls/design.md index 3033556..848ede9 100644 --- a/specs/2026-08-10_normalize-package-lock-resolved-urls/design.md +++ b/specs/2026-08-10_normalize-package-lock-resolved-urls/design.md @@ -456,18 +456,41 @@ additionally runs an advisory `warn-foreign-registry.sh` against the **consumer' **Rationale:** Dimension 8 declared consumer-repo lockfiles out of scope on the reasoning that a failing check in a reusable workflow would break currently-green pipelines. Implementation found that reasoning rested on a false premise: -**`cplace-paw-fe` `release/25.2` and `release/25.3` are not green — they are broken today**, each carrying 14 +~~**`cplace-paw-fe` `release/25.2` and `release/25.3` are not green — they are broken today**, each carrying 14 `registry.npmjs.org` entries in their own lockfile. Verified with a cold cache and the real secret: `E404 GET https://cplace.jfrog.io/readdirp/-/readdirp-3.6.0.tgz`. Nothing masks it: that repo has zero caches on those branches, and the cache key is `hashFiles('**/package-lock.json')`, so a cross-branch hit is impossible by -construction. +construction.~~ + +**Corrected 2026-08-14 — the premise was half right, and the half that was wrong changes the timeline.** Those two +branches do each carry 14 `registry.npmjs.org` entries, but they are **not failing today**. The `E404` is +**npm-version-dependent**, measured both ways with a one-package fixture (`color-name@1.1.4`, proven installable +through the proxy): + +| npm | environment | un-normalized entry, no mitigation | tarball served by | +| --- | --- | --- | --- | +| **10.2.4** | runner, node 18.19.1 | installs | the **proxy** — the rewrite preserves its path prefix | +| **11.3.0** | developer machine, node 22.15.0 | **`E404`** | — prefix dropped | + +Every pipeline pins node 18.19.1 (this repo's reusable workflows; paw-fe's `.nvmrc`), so all of them are on npm 10, +and paw-fe's run history contains no failure of this shape. The original `E404` was real — it was reproduced on a +developer machine, where npm is newer. + +**The consequence is a sequencing one:** the prefix drop is a *regression in newer npm*, so nothing breaks in CI today +and everything un-normalized breaks the moment runners move to node 24 / npm 11 +(`specs/2026-06-05_node24-workflow-migration`). Normalization is a **prerequisite for that migration**, not a cleanup +after it. + +**And the mitigation's cost is unconditional.** Measured in the same runs: under `replace-registry-host=never` the +tarball came from `registry.npmjs.org` (proxy fetches: 0) on *both* npm versions. On npm 11 that buys compatibility; +on npm 10 it buys nothing and is the sole reason traffic leaves the proxy. That splits the problem into **two failure surfaces**, which this design had treated as one: | surface | where `npm ci` runs | whose lockfile | fixed by | | --- | --- | --- | --- | | composite | the action's own checkout, outside the workspace | *this* repo's | normalization (Dimensions 1–3) | -| consumer | the workspace | the *consumer's* | **only** the mitigation, or normalizing that consumer | +| consumer | the workspace | the *consumer's* | the mitigation (npm ≥ 11 only), or normalizing that consumer (every version) | `replace-registry-host=never` makes npm fetch each `resolved` URL verbatim rather than rewriting its host, which fixes both surfaces at once and needs no lockfile change anywhere. Measured against real lockfiles with the real secret: @@ -489,6 +512,12 @@ github-actions un-normalized → `added 542 packages`; `cplace-paw-fe release/25 - **This is a mitigation, not the fix.** Under it, entries still on npmjs are fetched directly from npmjs, bypassing Xray and curation. Removal is owned by PFM-ISSUE-34454. +- **On npm 10 the bypass is its *only* effect** (measured 2026-08-14): without the flag those same entries resolve + through the proxy. It is therefore worth asking, at removal time, whether it should come out ahead of the inventory + reaching zero — the answer today is no, because `use-npmrc` writes the `~/.npmrc` developer machines use, and those + are on npm 11. +- **The removal criterion is unchanged, but its reason is not.** "Remove when no lockfile reports foreign entries" + holds because the flag then has nothing to act on — not because removing it would otherwise break something. - **The warnings are the removal criteria.** `warn-foreign-registry.sh` emits a `::warning` annotation plus a job summary naming offending package paths; when no pipeline reports one, the line comes out. The mitigation thus inventories its own obsolescence. diff --git a/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.html b/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.html index cb4b954..12c662a 100644 --- a/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.html +++ b/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.html @@ -307,14 +307,23 @@

The fix

A lockfile entry that resolves via registry.npmjs.org is not fetched from where it says. npm's default replace-registry-host=npmjs rewrites the host onto whatever registry - ~/.npmrc configures — and pacote builds the new URL by joining the old pathname onto the - new base, so the registry's own path prefix is discarded. The request 404s, and because that prefix is the + ~/.npmrc configures — and on npm 11 pacote builds the new URL by joining the + old pathname onto the new base, so the registry's own path prefix is discarded. The request 404s, and because that prefix is the JFROG_URL secret, CI prints the whole thing as ***. That is why every message these scripts print names a package path and never a URL.

The fix has two parts, and they attack the same rewrite from opposite ends.

+

+ The version qualifier is load-bearing. Measured 2026-08-14 with a one-package fixture, on a + runner and locally: npm 10.2.4 — node 18.19.1, what every pipeline pins today — rewrites the + host onto the configured registry correctly and installs through the proxy, while + npm 11.3.0 — developer machines, and any runner on node 24 — drops the prefix and fails. The + prefix drop is a regression in newer npm: nothing is failing in CI today, and everything + un-normalized fails the moment runners move to node 24. That makes this work a prerequisite for the Node 24 + migration rather than a cleanup after it. +

@@ -333,8 +342,9 @@

replace-registry-host=never

One line appended to ~/.npmrc by use-npmrc:25, - switching the rewrite off entirely. It fixes lockfiles nobody has normalized yet — including every - consumer's — which is why it ships to all ~41 downstream repositories rather than just this one. + switching the rewrite off entirely, so un-normalized lockfiles install on npm 11 too — including every + consumer's, which is why it ships to all ~41 downstream repositories. The cost is unconditional: those + entries are then fetched outside the proxy, on every npm version.

@@ -348,7 +358,7 @@

replace-registry-host=never

- 1 — UNFIXED, DEFAULT NPM + 1 — UNFIXED, NPM 11 (DEFAULT REWRITE) 2 — MITIGATION APPLIED 3 — LOCKFILE NORMALIZED @@ -438,7 +448,8 @@

replace-registry-host=never

- Lane 2 removes the rewrite; lane 3 removes the need for it. The pairing is what makes the rollout + Lane 1 is npm 11; on npm 10.2.4 the same rewrite lands on the proxy correctly and installs. Lane 2 removes + the rewrite; lane 3 removes the need for it. The pairing is what makes the rollout lazy rather than a coordinated switchover: on a normalized lockfile the flag changes nothing, so the two compose in either order and the flag can come out per branch — once the advisory stops reporting anywhere.
@@ -447,8 +458,9 @@

replace-registry-host=never

The mitigation is not free, and lane 2 is why: an entry still on npmjs is fetched from npmjs, - bypassing whatever curation the proxy enforces. It is a deliberate, documented trade — and the advisory's - warnings are the inventory that decides when it can be withdrawn. + bypassing whatever curation the proxy enforces — on every npm version. On npm 11 that buys + compatibility; on npm 10, where the rewrite was already working, it buys nothing at all. A deliberate, + documented trade — and the advisory's warnings are the inventory that decides when it can be withdrawn.

diff --git a/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.md b/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.md index 759e87f..0d6764d 100644 --- a/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.md +++ b/specs/2026-08-10_normalize-package-lock-resolved-urls/overview.md @@ -16,21 +16,32 @@ An HTML edition of this document, with hand-drawn diagrams, sits beside it as ## 1. The fix A lockfile entry that resolves via `registry.npmjs.org` is not fetched from where it says. npm's default -`replace-registry-host=npmjs` rewrites the host onto whatever registry `~/.npmrc` configures — and pacote builds the -new URL by joining the *old pathname* onto the new base, so the registry's own path prefix is discarded. The request -404s, and because that prefix is the `JFROG_URL` secret, CI prints the whole thing as `***`. That is why **every -message these scripts print names a package path and never a URL.** +`replace-registry-host=npmjs` rewrites the host onto whatever registry `~/.npmrc` configures — and on **npm 11** pacote +builds the new URL by joining the *old pathname* onto the new base, so the registry's own path prefix is discarded. The +request 404s, and because that prefix is the `JFROG_URL` secret, CI prints the whole thing as `***`. That is why +**every message these scripts print names a package path and never a URL.** + +**The version qualifier is load-bearing.** Measured 2026-08-14 with a one-package fixture, on a runner and locally: + +| npm | un-normalized entry, no mitigation | tarball served by | +| --- | --- | --- | +| 10.2.4 — node 18.19.1, what every pipeline pins today | installs | the **proxy**; the rewrite keeps the path prefix | +| 11.3.0 — developer machines, and any runner on node 24 | **`E404`** | — prefix dropped | + +So the prefix drop is a **regression in newer npm**: nothing is failing in CI today, and everything un-normalized +fails the moment runners move to node 24. That makes this work a **prerequisite for the Node 24 migration** rather +than a cleanup after it. The fix has two parts, attacking the same rewrite from opposite ends: | | What it does | Where | | --- | --- | --- | -| **The fix** — permanent | Rewrite every `resolved` prefix onto the proxy, so the lockfile already agrees with the registry and there is nothing to rewrite | `normalize-lockfile.sh`, guarded by `check-lockfile.sh` | -| **The mitigation** — interim | `replace-registry-host=never` switches the rewrite off entirely, rescuing lockfiles nobody has normalized yet — including every consumer's | [`use-npmrc/action.yml:25`](../../.github/actions/use-npmrc/action.yml) | +| **The fix** — permanent | Rewrite every `resolved` prefix onto the proxy, so the lockfile already agrees with the registry and there is nothing to rewrite. Works on every npm version, always through the proxy | `normalize-lockfile.sh`, guarded by `check-lockfile.sh` | +| **The mitigation** — interim | `replace-registry-host=never` switches the rewrite off entirely, so un-normalized lockfiles install on npm 11 too — at the cost of fetching those entries **outside the proxy**, on every version | [`use-npmrc/action.yml:25`](../../.github/actions/use-npmrc/action.yml) | ```mermaid flowchart TB - subgraph L1["1 · unfixed, default npm"] + subgraph L1["1 · unfixed, npm 11 (default rewrite)"] A1["entry on npmjs
registry.npmjs.org/foo/-/foo-1.0.0.tgz"] A2["host rewritten
new URL(oldPathname, registry)"] A3["GET cplace.jfrog.io/foo/-/foo-1.0.0.tgz
no /artifactory/api/npm/cplace-npm"] @@ -66,12 +77,14 @@ flowchart TB class C2,C3,C4 ok; ``` -Lane 2 removes the rewrite; lane 3 removes the *need* for it. Because the flag is a no-op on a normalized lockfile, -the two compose in either order and the flag can be dropped **per branch** rather than in a coordinated switchover — -once the advisory stops reporting anywhere. +Lane 1 is npm 11; on npm 10.2.4 the same rewrite lands on the proxy correctly and installs. Lane 2 removes the +rewrite; lane 3 removes the *need* for it. Because the flag is a no-op on a normalized lockfile, the two compose in +either order and the flag can be dropped **per branch** rather than in a coordinated switchover — once the advisory +stops reporting anywhere. The mitigation is not free, and lane 2 is why: an entry still on npmjs is fetched *from* npmjs, bypassing whatever -curation the proxy enforces. A deliberate, documented trade, owned by PFM-ISSUE-34454. +curation the proxy enforces — on **every** npm version. On npm 11 that buys compatibility; on npm 10, where the +rewrite was already working, it buys nothing at all. A deliberate, documented trade, owned by PFM-ISSUE-34454. --- diff --git a/tools/scripts/lockfile/README.md b/tools/scripts/lockfile/README.md index 02661f1..7556a1b 100644 --- a/tools/scripts/lockfile/README.md +++ b/tools/scripts/lockfile/README.md @@ -51,13 +51,25 @@ wrapped, run by `use-npmrc` on every consumer's runner, and can also be called b replace-registry-host=never ``` -This tells npm to fetch each `resolved` URL **verbatim** rather than rewriting its host onto the configured registry — -which is exactly the bug. It makes an un-normalized lockfile install successfully, so it protects **consumer** -repositories too, not just this one. JFrog URLs stay authenticated by the secret, so it introduces no dependency on -anonymous JFrog access. +This tells npm to fetch each `resolved` URL **verbatim** rather than rewriting its host onto the configured registry. +It makes an un-normalized lockfile install on any npm version, so it protects **consumer** repositories too, not just +this one. JFrog URLs stay authenticated by the secret, so it introduces no dependency on anonymous JFrog access. -**It is a mitigation, not the fix.** Under it, any entry still pointing at `registry.npmjs.org` is fetched *directly -from npmjs*, bypassing the proxy — no Xray, no curation. That is precisely what this ticket exists to eliminate. +**The rewrite it disables is only broken on newer npm.** Measured 2026-08-14 with a one-package fixture, on a runner +and locally: + +| npm | un-normalized entry, no mitigation | tarball served by | +| --- | --- | --- | +| 10.2.4 (node 18.19.1 — what every pipeline pins today) | installs | the **proxy** — the rewrite keeps its path prefix | +| 11.3.0 (developer machines; any runner on node 24) | **`E404`**, masked as `***` | — prefix dropped | + +**It is a mitigation, not the fix**, and it is not free on either version. Under it, an entry still pointing at +`registry.npmjs.org` is fetched *directly from npmjs*, bypassing the proxy — no Xray, no curation. On npm 10 that is +its only effect, because the rewrite it disables was working. On npm 11 it buys compatibility and spends proxy +routing. Normalizing buys both: the entry resolves through the proxy on every version, flag or no flag. + +**This is why the Node 24 migration depends on the rollout, not the other way round.** Nothing fails in CI today; +everything un-normalized fails the moment runners move to npm 11. Because of that, `use-npmrc` also runs `warn-foreign-registry.sh` against the consumer's own `package-lock.json` and emits a `::warning` annotation plus a job summary listing the offending package paths. **Those warnings are the diff --git a/tools/scripts/lockfile/warn-foreign-registry.sh b/tools/scripts/lockfile/warn-foreign-registry.sh index 968adb1..a26ca42 100755 --- a/tools/scripts/lockfile/warn-foreign-registry.sh +++ b/tools/scripts/lockfile/warn-foreign-registry.sh @@ -7,10 +7,12 @@ # ./tools/scripts/lockfile/warn-foreign-registry.sh [] # # Runs inside the `use-npmrc` composite, so it inspects the CONSUMER's lockfile -# on the runner. Those entries resolve today only because `use-npmrc` sets -# `replace-registry-host=never`; without it npm rewrites their host onto the -# configured registry, drops that registry's path prefix, and fails with an -# E404 masked as *** (PFM-ISSUE-34453). +# on the runner. Under `replace-registry-host=never`, which `use-npmrc` sets, +# those entries are fetched verbatim from the registry they name - outside the +# proxy. Without that flag the outcome depends on the npm version: 10.2.4 +# rewrites the host onto the configured registry correctly and resolves THROUGH +# the proxy, while 11.3.0 drops the registry's path prefix and fails with an +# E404 masked as *** (PFM-ISSUE-34453). Measured 2026-08-14, both ways. # # ADVISORY ONLY - this must never fail a consumer's build. Every exit is 0. # It is a discovery mechanism: the warnings are the inventory of lockfiles that @@ -71,7 +73,7 @@ main() { total="$(printf '%s\n' "${offenders}" | wc -l | tr -d '[:space:]')" - printf '::warning file=%s::%s entries in package-lock.json do not resolve via the cplace npm proxy. They install today only because use-npmrc sets replace-registry-host=never. Normalize this lockfile - see PFM-ISSUE-34453.\n' \ + printf '::warning file=%s::%s entries in package-lock.json do not resolve via the cplace npm proxy. Under replace-registry-host=never (set by use-npmrc) they are fetched from the registry they name, bypassing the proxy; without that flag they fail on npm 11 and newer. Normalize this lockfile - see PFM-ISSUE-34453.\n' \ "$(basename "${lockfile}")" "${total}" if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then @@ -80,11 +82,16 @@ main() { printf '### Lockfile entries outside the cplace npm proxy\n\n' printf '**%s** `resolved` entries in `%s` point somewhere other than the cplace npm proxy.\n\n' \ "${total}" "${lockfile#"${GITHUB_WORKSPACE:-}/"}" - printf 'They install successfully only because `use-npmrc` sets `replace-registry-host=never`. ' - printf 'Without it npm rewrites their host onto the configured registry, drops its path prefix, ' - printf 'and fails with an `E404` masked as `***`.\n\n' - printf 'Fix by normalizing the lockfile onto the proxy (PFM-ISSUE-34453); the mitigation can be ' - printf 'removed once no lockfile reports this.\n\n' + printf '`use-npmrc` sets `replace-registry-host=never`, so npm fetches each of these URLs verbatim ' + printf '**from the registry the lockfile names, not through the cplace proxy**.\n\n' + printf 'What would happen without that flag depends on the npm version: **npm 10.2.4** rewrites the ' + printf 'host onto the configured registry correctly, and the entry resolves *through* the proxy; ' + printf '**npm 11.3.0** drops the path prefix and fails with an `E404` masked as `***`. Both measured ' + printf '2026-08-14. Runners pinned to node 18.19.1 are on npm 10; developer machines, and any runner ' + printf 'moving to node 24, are not.\n\n' + printf 'Fix by normalizing the lockfile onto the proxy (PFM-ISSUE-34453) - it then resolves through ' + printf 'the proxy on every npm version, with or without the flag, and the mitigation can be removed ' + printf 'once no lockfile reports this.\n\n' printf '
First %s affected packages\n\n' "${MAX_LISTED}" printf '```\n' printf '%s\n' "${offenders}" | head -n "${MAX_LISTED}"