install: enforce minimumReleaseAge against lockfile-pinned versions#30526
install: enforce minimumReleaseAge against lockfile-pinned versions#30526robobun wants to merge 8 commits into
Conversation
The resolution-time cooldown filter only runs when Bun is choosing a new version. If a version was already pinned in bun.lock before the cooldown was configured (or pinned by a developer whose local bunfig was less strict), subsequent `bun install` and `bun install --frozen-lockfile` runs installed it unconditionally — defeating the supply-chain use case the setting is meant to cover. Add a post-resolution gate in installWithManager that walks every npm-tagged package in the lockfile, loads its manifest (honoring extended-manifest fetching when minimumReleaseAge is set), looks up the pinned version's publish timestamp, and errors out with every violation when any version falls inside the cooldown window. Excludes from `install.minimumReleaseAgeExcludes` are honored. Fixes #30525
|
Updated 10:27 PM PT - May 11th, 2026
❌ @robobun, your commit 0c9fe32 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 30526That installs a local version of the PR into your bun-30526 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR enforces configured minimum_release_age_ms for versions already pinned in bun.lock by adding enforceLockfileAgeFilter to validate locked npm packages before install; installWithManager invokes it and aborts with logged errors and remediation guidance if violations are found. ChangesLockfile Release Age Enforcement
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/PackageManager/PopulateManifestCache.zig`:
- Around line 189-200: The current use of "orelse continue" on
manager.manifests.byNameHash(...) and manifest.findByVersion(...) lets missing
manifest/version metadata bypass minimumReleaseAge checks; change this to "fail
closed" by treating missing metadata as a blocked (too-recent) result instead of
silently continuing: replace the first "orelse continue" after
manager.manifests.byNameHash(...) with logic that returns an error or marks the
resolution as disallowed when the manifest is missing, and replace the "orelse
continue" after manifest.findByVersion(...) with logic that treats a missing
find_result as failing the Npm.PackageManifest.isPackageVersionTooRecent check
(use min_age_ms and manager.options.minimum_release_age_excludes) so the code
enforces the cooldown rather than skipping it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b48aae88-dc10-44d1-af20-d538b76f3b45
📒 Files selected for processing (4)
src/install/PackageManager.zigsrc/install/PackageManager/PopulateManifestCache.zigsrc/install/PackageManager/install_with_manager.zigtest/cli/install/minimum-release-age.test.ts
Silently continuing when `manifests.byNameHash` or `findByVersion` returns null would reopen the lockfile bypass the surrounding gate exists to close — a supply-chain actor who unpublished (or fabricated) a too-young version could slip past the cooldown because we had nothing to check its publish timestamp against. Surface both cases as errors with their own remediation context, and honor the bare-name entries of `minimumReleaseAgeExcludes` for the manifest-unavailable branch (the `name@version` form — if/when that PR lands — still requires a manifest lookup to confirm the pin). Add a regression test that tampers with bun.lock to point at a version the registry does not list and asserts the install aborts.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/PackageManager/PopulateManifestCache.zig`:
- Around line 244-248: The isExcludedByName helper currently only compares the
full package name and therefore misses bare-name exclusions for scoped packages;
update isExcludedByName to also treat a scoped package "@scope/name" as matching
an exclusion entry "name" by checking, when name contains '/', the segment after
the last '/' (or the part after the scope) against exclusions in addition to the
full name. Modify the loop in isExcludedByName to compare both
bun.strings.eql(entry, name) and, if name contains '/', bun.strings.eql(entry,
segmentAfterSlash) so bare-name excludes like "scoped-package" will also match
"@scope/scoped-package".
In `@test/cli/install/minimum-release-age.test.ts`:
- Around line 1848-1853: Move the diagnostics assertions to check stderr content
before asserting the process exit code: keep the existing reads of
proc.stderr.text() and proc.exited but assert that stderr contains
"regular-package", "3.0.0", and a case-insensitive "minimum release age" first,
and only after those expectations assert expect(exitCode).toBe(1); apply the
same reorder for the other two similar blocks in this test file (the blocks
around the other mentioned ranges that use proc, stderr, and exitCode).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: da0ff362-c6c4-4336-877c-e9b3571f7c17
📒 Files selected for processing (2)
src/install/PackageManager/PopulateManifestCache.zigtest/cli/install/minimum-release-age.test.ts
The ban-words check forbids bare `catch bun.outOfMemory()` — it can accidentally swallow non-OOM errors. Use `bun.handleOom(expr)` instead, which crashes only on `error.OutOfMemory` and propagates any other error intact.
…nor --silent CodeRabbit + claude[bot] review feedback: 1. The gate was firing on *any* error in `manager.log`, so a transient manifest-fetch failure (populateManifestCache funnels registry 5xx / parse errors into the same log) would crash with the misleading `remove the offending version from bun.lock...` note even though nothing violated the cooldown. Snapshot `log.errors` before the call and gate the note on the delta. Unrelated errors still crash the install (we can't safely proceed with missing manifest data) but get the generic error output instead of the cooldown-specific remediation. 2. The note + log print ran regardless of `log_level`. Wrap them in `if (log_level != .silent)` to match the frozen-lockfile crash path and other output sites in this function. 3. Reorder the three frozen-lockfile tests to assert stderr content before exit code — when a test fails, the stderr expectations surface the actual install output instead of a bare `expected 1 got 0`.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/install/PackageManager/install_with_manager.zig`:
- Around line 794-817: The enforceLockfileAgeFilter() block (guarded by
manager.options.minimum_release_age_ms) must be moved earlier — place the entire
if-block that calls manager.enforceLockfileAgeFilter(), computes violations,
prints the note, resets manager.log, and calls Global.crash() immediately after
resolution verification completes and before
performSecurityScanAfterResolution() and any scanner/lifecycle side effects;
ensure you keep the same checks on manager.log/errors, log_level behavior, and
the remediation message referencing install.minimumReleaseAgeExcludes so the
failure path remains identical but runs before any
security-scanner/network/install work.
In `@test/cli/install/minimum-release-age.test.ts`:
- Around line 1885-1889: Add the same assertion that checks the rejection reason
for the "minimum release age" gate in this non-frozen test: after the existing
expects for stderr containing "regular-package" and "3.0.0" and before asserting
exitCode, add an expectation that stderr includes the "minimum release age"
message (the same string used in the frozen and missing-manifest variants) so
the test verifies the specific gate rather than any generic failure mode; update
the block around the variables stderr and exitCode used in the Promise.all call
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 213d9253-735a-490b-a5dc-5c8d5dabf5ec
📒 Files selected for processing (2)
src/install/PackageManager/install_with_manager.zigtest/cli/install/minimum-release-age.test.ts
…non-frozen test CodeRabbit review feedback: 1. The gate was running after `performSecurityScanAfterResolution`, which meant a poisoned lockfile could still trigger scanner installation and network work before we rejected the install. Move the gate to directly after `verifyResolutions`, before the security scanner, so lockfile-age enforcement happens first. 2. The non-frozen `bun install` test was asserting only `regular-package` + `3.0.0` in stderr — a different failure mode could satisfy that. Add a case-insensitive `minimum release age` assertion so the test pins this specific gate, matching the frozen and absent-manifest variants.
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/install/PackageManager/install_with_manager.zig:800-806— Theerrors_beforesnapshot is captured beforeenforceLockfileAgeFilter(), but that function's first step istry populateManifestCache(manager, .all)— so any manifest-fetch errorrunTaskslogs (registry 5xx, network failure, parse failure) lands after the snapshot and is counted inviolations, contradicting the comment at lines 802-805 and the resolved-thread reply. HaveenforceLockfileAgeFilterreturn an explicit violation count from its own loop (or snapshotlog.errorsinside it, afterpopulateManifestCachereturns) so the remediation note is gated on actual cooldown/fail-closed errors only.Extended reasoning...
What the bug is
The fix in fb8c2c5 for the earlier review comment snapshots
errors_before = manager.log.errorsimmediately before callingmanager.enforceLockfileAgeFilter(), then computesviolations = manager.log.errors -| errors_beforeand gates the remediation note onviolations > 0. The accompanying comment at lines 802-805 says this is to "distinguish cooldown violations (added by enforceLockfileAgeFilter) from unrelated errors that populateManifestCache may have funneled into manager.log (e.g. a transient registry 5xx)", and the resolved-thread reply (3222998605) says "Unrelated errors (manifest fetch 5xx, parse failures)… fall through to the generic error output instead of being misattributed to a cooldown violation."But
enforceLockfileAgeFilter()callspopulateManifestCache(manager, .all)internally as its very first step (PopulateManifestCache.zig:175). Any error thatpopulateManifestCache→runTasksdeposits intomanager.logis therefore logged aftererrors_beforewas captured and counts towardviolations. The snapshot only filters out errors that pre-existed theenforceLockfileAgeFiltercall (e.g. fromcleanWithLogger); it does not filter out manifest-fetch errors raised inside it.The specific code path
populateManifestCachedrains tasks viarunTasks(..., .{ .onPackageManifestError = {}, ... }). With a voidonPackageManifestError,runTasks.zigwrites directly tomanager.logviaaddErrorFmtat:- ~line 191 — non-retryable network failure on a required manifest,
- ~line 247 — HTTP status > 399 on a required manifest,
- ~line 612 — manifest parse failure.
All three increment
manager.log.errorsafter the snapshot at install_with_manager.zig:800 was taken, so all three are counted inviolations.Why existing code doesn't prevent it
The snapshot is taken at the call site in
installWithManager, one frame above wherepopulateManifestCacheruns. There is no second snapshot insideenforceLockfileAgeFilterbetween thepopulateManifestCachecall and the per-package loop, andenforceLockfileAgeFilterreturns!voidrather than a count, so the caller has no way to distinguish which errors inside the delta came from the loop vs. from manifest fetching.Step-by-step proof
bun.lockpinsfoo@1.0.0andfoois listed inminimumReleaseAgeExcludes.minimumReleaseAgeis configured.installWithManagerreaches line 800:errors_before = manager.log.errors(say, 0).enforceLockfileAgeFilter()is called →populateManifestCache(manager, .all)runs → registry returns 503 forfoo→runTaskscallsmanager.log.addErrorFmt("GET … - 503").manager.log.errorsis now 1.- Back in
enforceLockfileAgeFilter's loop:byNameHashreturnsnullforfoo(no manifest).isExcludedByName("foo", …)is true →continue. The loop adds zero errors of its own. - Control returns to
installWithManager.violations = 1 -| 0 = 1.manager.log.hasErrors()is true. - The gate prints
error: GET … - 503, then printsnote: remove the offending version from bun.lock, raise the bound, or add it to install.minimumReleaseAgeExcludes— for a package that is already in the excludes list and has no cooldown violation. ThenGlobal.crash().
This is exactly the misattribution the comment claims to prevent.
Impact
Diagnostic-quality only — the install correctly fails either way, and the underlying error text (e.g. the 503) is printed first. In the common non-excluded case, the loop's own fail-closed "manifest unavailable" error fires alongside the network error, and two of the note's three remediations (remove from
bun.lock/ add to excludes) are technically applicable to that fail-closed error, so the note isn't entirely nonsensical there. But the code demonstrably does not do what its comment and the resolved-thread reply say it does, and the excluded-package scenario above produces a purely spurious note.How to fix
Have
enforceLockfileAgeFilterreturn the count of errors its own loop added (incrementing a localvar violations: u32 = 0;at eachaddErrorFmtsite and returning it), and gate the note on that. Alternatively, snapshotmanager.log.errorsinsideenforceLockfileAgeFilterafterpopulateManifestCachereturns and return the delta. Either way the comment at lines 802-805 should be updated to match.
The previous fix snapshotted `manager.log.errors` at the call site, before calling `enforceLockfileAgeFilter`. But that function's first step is `populateManifestCache(manager, .all)`, which funnels registry errors (5xx, parse failures) through `runTasks` into the same log. Those errors landed *after* the snapshot and got counted as cooldown violations — exactly the misattribution the snapshot comment claimed to prevent. Return the violation count from `enforceLockfileAgeFilter` itself, so only errors its own loop adds are counted. The caller still crashes on any logged error (manifest-fetch failures can't be silently ignored, we'd be installing with missing data), but the cooldown-specific remediation note fires only when we actually found cooldown violations.
…gate Two claude[bot] review findings: 1. The remediation note said "raise the bound" — but the violation fires because a package is *too recent*, so raising `minimumReleaseAge` makes things worse. Change to "lower `install.minimumReleaseAge`". 2. `docs/pm/cli/install.mdx` still said "Only affects new package resolution — existing packages in bun.lock remain unchanged". That is the bypass this PR closes. Replace with a bullet describing the new lockfile-age gate and pointing at `minimumReleaseAgeExcludes`.
There was a problem hiding this comment.
All earlier review feedback has been addressed and I have nothing further to flag, but this is a user-visible behavior change to a supply-chain security gate in the bun install hot path (plus a fail-closed design decision that makes installs network-dependent when minimumReleaseAge is set), so it's worth a maintainer's sign-off.
Extended reasoning...
Overview
This PR closes a bypass in the minimumReleaseAge supply-chain protection: previously the cooldown only applied during version resolution, so any version already pinned in bun.lock would install regardless. The fix adds enforceLockfileAgeFilter() in PopulateManifestCache.zig (~90 new lines) which, after resolution, force-loads extended manifests for every locked npm package via populateManifestCache(.all), looks up each pinned version's publish timestamp, and fails the install if any violate the cooldown (or cannot be verified — fail-closed). The gate is wired into install_with_manager.zig immediately after verifyResolutions and before the security scanner. Docs and tests are updated to match.
Security risks
The change strengthens a security feature rather than weakening one. No new injection/auth/data-exposure surface. The fail-closed behavior on missing manifest/version metadata is the conservative choice for a supply-chain gate. Error messages echo package names/versions from the lockfile into stderr, which is normal for install diagnostics.
Level of scrutiny
High — human review warranted. This is not a mechanical change:
- It flips documented behavior (the old docs explicitly said lockfile-pinned versions were untouched; now they hard-fail), so users who enabled
minimumReleaseAgewith an existing lockfile may see their nextbun install/ CI run break. - When the feature is enabled, every
bun installnow triggerspopulateManifestCache(.all), which may fetch extended manifests for every npm package in the lockfile — a latency/network cost that didn't exist before on the locked-install path. - The fail-closed choice means a transient registry outage or an unpublished version will block installs entirely (with the
minimumReleaseAgeExcludesescape hatch as the only workaround). That's defensible for a security gate but is a UX/policy call.
Other factors
The PR has been through several review rounds (CodeRabbit + my own inline comments) and every raised issue — fail-open on missing metadata, --silent handling, bun.handleOom ban-words, violation-count attribution vs. unrelated manifest errors, gate ordering vs. the security scanner, stale docs, and the reversed "raise the bound" hint — has been addressed in subsequent commits. Test coverage is solid (frozen + non-frozen rejection, excludes pass-through, old-enough pass, fail-closed on missing-version). The two CI failures reported by robobun on 09ec15f (bun-install-registry.test.ts on Win-aarch64 and an unrelated http-abort timeout) look like known flakes rather than regressions from this change, but a maintainer should confirm. Given the behavior change and perf/UX trade-offs, this should not be auto-approved.
…lidation The previous revision relied on storeController.requestPackage to recover each pinned version's publish timestamp. That path can take the peekManifestFromStore fast path (which returns publishedAt: undefined) or fall through to abbreviated metadata + a 304 cache hit, in either case the publish time is missing and the check would silently no-op — re-opening the very bypass the gate is meant to close. Manual end-to-end testing surfaced this regression after the initial commit landed. Mirror the approach bun took in oven-sh/bun#30526: build a dedicated metadata fetcher that goes directly at the registry asking for the full (un-abbreviated) manifest for every npm-resolved lockfile entry, and fail closed when the manifest is unreachable or the pinned version is missing. Decoupling from the resolver pipeline also avoids the non-strict fallback that would otherwise let an unmatched exact version be re-picked from the unfiltered manifest. Also skip the optimisticRepeatInstall fast path in installDeps when minimumReleaseAge is configured. That fast path declared the install "up to date" based on lockfile/node_modules state alone and returned before reaching tryFrozenInstall, so the revalidation gate inside it never ran. The lookup is fail-closed: manifest-unavailable, version-not-in-manifest, or an invalid publish timestamp all surface as violations rather than silent passes. Violations are aggregated and reported with the three remediation options (remove from lockfile, lower the bound, add to minimumReleaseAgeExclude).
…lidation The previous revision relied on storeController.requestPackage to recover each pinned version's publish timestamp. That path can take the peekManifestFromStore fast path (which returns publishedAt: undefined) or fall through to abbreviated metadata + a 304 cache hit, in either case the publish time is missing and the check would silently no-op — re-opening the very bypass the gate is meant to close. Manual end-to-end testing surfaced this regression after the initial commit landed. Mirror the approach bun took in oven-sh/bun#30526: build a dedicated metadata fetcher that goes directly at the registry asking for the full (un-abbreviated) manifest for every npm-resolved lockfile entry, and fail closed when the manifest is unreachable or the pinned version is missing. Decoupling from the resolver pipeline also avoids the non-strict fallback that would otherwise let an unmatched exact version be re-picked from the unfiltered manifest. Also skip the optimisticRepeatInstall fast path in installDeps when minimumReleaseAge is configured. That fast path declared the install "up to date" based on lockfile/node_modules state alone and returned before reaching tryFrozenInstall, so the revalidation gate inside it never ran. The lookup is fail-closed: manifest-unavailable, version-not-in-manifest, or an invalid publish timestamp all surface as violations rather than silent passes. Violations are aggregated and reported with the three remediation options (remove from lockfile, lower the bound, add to minimumReleaseAgeExclude).
Closes #10438. ## What Re-verify every entry in `pnpm-lock.yaml` against the policies the resolver chain was configured with — today: `minimumReleaseAge` in strict mode — right after the lockfile is loaded from disk and before any tarball is fetched. A locked version that fails the policy aborts the install with `ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION`; `minimumReleaseAgeExclude` is honored. ## Why The policy only fires while pnpm is *choosing* a version. Once a version is pinned in the lockfile — e.g. a developer disabled the policy locally and committed a fresh dependency, or a CI cache restored a stale lockfile — every later `pnpm install` (including `--frozen-lockfile` and `pnpm fetch`) installs it without re-checking, which defeats the supply-chain protection the setting is supposed to provide. The threat model is **a lockfile someone else resolved**, not local resolution: local resolution is already covered by the resolver's own per-version filter. bun fixed the same shape of bug in [oven-sh/bun#30526](oven-sh/bun#30526); this PR is the pnpm side. ## How The fix introduces a generic `ResolutionVerifier` abstraction in the resolver chain — each resolver factory can ship a sibling verifier factory, exactly the way each resolver ships a `resolve` function. Today there's one verifier (npm); the shape leaves room for future ones (jsr, attestation-based, etc.) without changing the install-side interface. - **`@pnpm/resolving.resolver-base`** exports the `ResolutionVerifier` / `ResolutionVerification` types — the shared contract. - **`@pnpm/resolving.npm-resolver`** exports `createNpmResolutionVerifier`. Returns `undefined` when no policy is active, so callers can cheaply decide whether to iterate at all. When active, it inspects each lockfile entry, handles `minimumReleaseAgeExclude`, routes through named-registry prefixes (built-ins like `gh:` merged in), and uses `fetchFullMetadataCached` to fetch full registry metadata — decoupled from the resolver pipeline so neither `peekManifestFromStore` nor abbreviated metadata can hide the publish timestamp. - **`@pnpm/resolving.default-resolver`** exports `createResolutionVerifier`, a combinator that asks each underlying verifier (today: npm) if it has work and returns `undefined` when none does. Designed so that adding more verifiers later doesn't change the install side. - **`@pnpm/installing.client`** exposes `verifyResolution` on `Client`, built from the same `fetchFromRegistry` / `getAuthHeader` the resolver chain already uses — **no second fetcher is constructed**. - **`@pnpm/store.connection-manager`** and **`@pnpm/testing.temp-store`** surface `verifyResolution` alongside the store controller they hand back, so it reaches `mutateModules` through the existing plumbing. - **`@pnpm/installing.deps-installer`** gains one option on `StrictInstallOptions`: `verifyResolution?: ResolutionVerifier`. `mutateModules` invokes `verifyLockfileResolutions(ctx.wantedLockfile, opts.verifyResolution)` **once**, right after `getContext` returns the on-disk lockfile and before any path branches. When the verifier is `undefined`, the call is a no-op. The iteration is policy-neutral: dedupes by `(name, version)`, applies `pLimit(16)`, sorts violations stably, caps the printed list at 20 with an `…and N more` summary, throws a `PnpmError` carrying the verifier-supplied error code. The error includes a recovery hint that points at `pnpm clean --lockfile` followed by `pnpm install` — the safe way to throw away a poisoned lockfile and rebuild from fresh resolution. ## Tests - **9 unit tests** for `verifyLockfileResolutions` against a mock `ResolutionVerifier` — dedup, aggregation, stable ordering, the 20-entry cap, no-op behavior, the verifier-supplied error code surfacing in `PnpmError`. - **13 integration tests** in `installing/deps-installer/test/install/minimumReleaseAge.ts` via the real `install()` entry — `testDefaults()` wires `verifyResolution` from `createTempStore` → `createClient`, so the npm verifier runs end-to-end at the install boundary. Covers the rejection scenario, `minimumReleaseAgeExclude`, the strict-mode toggle, the existing `minimumReleaseAge` resolver-side suite, and a `pnpm add` scenario where a pre-existing entry would otherwise survive resolution. - **3 e2e tests** in `pnpm/test/install/minimumReleaseAge.ts` against the bundled CLI: rejection path with the right `ERR_PNPM_*` code and `pnpm clean --lockfile` hint in output, `minimumReleaseAgeExclude` honored, and the strict-off path (which now requires an explicit `minimumReleaseAgeStrict: false` since the config reader auto-enables strict mode when `minimumReleaseAge` is set). - Existing `frozenLockfile` suite (12 tests) and npm-resolver suite (179 tests) still pass. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
Closes pnpm#10438. ## What Re-verify every entry in `pnpm-lock.yaml` against the policies the resolver chain was configured with — today: `minimumReleaseAge` in strict mode — right after the lockfile is loaded from disk and before any tarball is fetched. A locked version that fails the policy aborts the install with `ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION`; `minimumReleaseAgeExclude` is honored. ## Why The policy only fires while pnpm is *choosing* a version. Once a version is pinned in the lockfile — e.g. a developer disabled the policy locally and committed a fresh dependency, or a CI cache restored a stale lockfile — every later `pnpm install` (including `--frozen-lockfile` and `pnpm fetch`) installs it without re-checking, which defeats the supply-chain protection the setting is supposed to provide. The threat model is **a lockfile someone else resolved**, not local resolution: local resolution is already covered by the resolver's own per-version filter. bun fixed the same shape of bug in [oven-sh/bun#30526](oven-sh/bun#30526); this PR is the pnpm side. ## How The fix introduces a generic `ResolutionVerifier` abstraction in the resolver chain — each resolver factory can ship a sibling verifier factory, exactly the way each resolver ships a `resolve` function. Today there's one verifier (npm); the shape leaves room for future ones (jsr, attestation-based, etc.) without changing the install-side interface. - **`@pnpm/resolving.resolver-base`** exports the `ResolutionVerifier` / `ResolutionVerification` types — the shared contract. - **`@pnpm/resolving.npm-resolver`** exports `createNpmResolutionVerifier`. Returns `undefined` when no policy is active, so callers can cheaply decide whether to iterate at all. When active, it inspects each lockfile entry, handles `minimumReleaseAgeExclude`, routes through named-registry prefixes (built-ins like `gh:` merged in), and uses `fetchFullMetadataCached` to fetch full registry metadata — decoupled from the resolver pipeline so neither `peekManifestFromStore` nor abbreviated metadata can hide the publish timestamp. - **`@pnpm/resolving.default-resolver`** exports `createResolutionVerifier`, a combinator that asks each underlying verifier (today: npm) if it has work and returns `undefined` when none does. Designed so that adding more verifiers later doesn't change the install side. - **`@pnpm/installing.client`** exposes `verifyResolution` on `Client`, built from the same `fetchFromRegistry` / `getAuthHeader` the resolver chain already uses — **no second fetcher is constructed**. - **`@pnpm/store.connection-manager`** and **`@pnpm/testing.temp-store`** surface `verifyResolution` alongside the store controller they hand back, so it reaches `mutateModules` through the existing plumbing. - **`@pnpm/installing.deps-installer`** gains one option on `StrictInstallOptions`: `verifyResolution?: ResolutionVerifier`. `mutateModules` invokes `verifyLockfileResolutions(ctx.wantedLockfile, opts.verifyResolution)` **once**, right after `getContext` returns the on-disk lockfile and before any path branches. When the verifier is `undefined`, the call is a no-op. The iteration is policy-neutral: dedupes by `(name, version)`, applies `pLimit(16)`, sorts violations stably, caps the printed list at 20 with an `…and N more` summary, throws a `PnpmError` carrying the verifier-supplied error code. The error includes a recovery hint that points at `pnpm clean --lockfile` followed by `pnpm install` — the safe way to throw away a poisoned lockfile and rebuild from fresh resolution. ## Tests - **9 unit tests** for `verifyLockfileResolutions` against a mock `ResolutionVerifier` — dedup, aggregation, stable ordering, the 20-entry cap, no-op behavior, the verifier-supplied error code surfacing in `PnpmError`. - **13 integration tests** in `installing/deps-installer/test/install/minimumReleaseAge.ts` via the real `install()` entry — `testDefaults()` wires `verifyResolution` from `createTempStore` → `createClient`, so the npm verifier runs end-to-end at the install boundary. Covers the rejection scenario, `minimumReleaseAgeExclude`, the strict-mode toggle, the existing `minimumReleaseAge` resolver-side suite, and a `pnpm add` scenario where a pre-existing entry would otherwise survive resolution. - **3 e2e tests** in `pnpm/test/install/minimumReleaseAge.ts` against the bundled CLI: rejection path with the right `ERR_PNPM_*` code and `pnpm clean --lockfile` hint in output, `minimumReleaseAgeExclude` honored, and the strict-off path (which now requires an explicit `minimumReleaseAgeStrict: false` since the config reader auto-enables strict mode when `minimumReleaseAge` is set). - Existing `frozenLockfile` suite (12 tests) and npm-resolver suite (179 tests) still pass. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
Fixes #30525.
Fixes #25305.
Reproduction
Cause
The
minimumReleaseAgefilter only runs during version resolution —findByDistTagWithFilter/findBestVersionWithFilterinsrc/install/npm.zig, whichPackageManagerEnqueuecalls when Bun picks a version. Whenbun.lockalready has a resolution pinned,installHoistedPackages→PackageInstaller.installPackageWithNameAndResolution(src/install/PackageInstaller.zig:772) reads the locked version, dispatches straight toenqueuePackageForDownload(PackageManagerEnqueue.zig:237), and extracts the tarball. No manifest lookup, no age check.--frozen-lockfiletightens the "lockfile must not change" gate but does not add any new filtering, so it behaved identically.This is the same shape as pnpm/pnpm#10438 — the gate only protects net-new resolutions, which is exactly the opposite of what a team adding the cooldown after an incident expects.
Fix
Add a post-resolution gate in
installWithManager(src/install/PackageManager/install_with_manager.zig) that runs whenminimumReleaseAgeis configured, right before the install phase begins:populateManifestCache(.all)to make sure every locked npm package has its (extended) manifest loaded..npmresolution inlockfile.packages, callmanifest.findByVersion(resolution.value.npm.version)to recover the exactPackageVersionand itspublish_timestamp_ms.isPackageVersionTooRecentand the package isn't ininstall.minimumReleaseAgeExcludes, accumulate an error onmanager.log.Global.crash()before installation starts.The walk only happens when
minimum_release_age_ms != null, so the hot path (no cooldown configured) is untouched. Exclusions are honored because the sameshouldExcludeFromAgeFilterpredicate used by the resolution-time filter is reused here.Under
--frozen-lockfilethe install fails fast without touching the filesystem. Without--frozen-lockfile, the user is told which versions violate the cooldown and pointed at the three remediation options: remove the offending version frombun.lock, raise the bound, or extendinstall.minimumReleaseAgeExcludes.Verification
With the src/ changes stashed, two of the four frozen-lockfile tests — the ones that assert the cooldown now blocks a locked violator under both
bun installandbun install --frozen-lockfile— fail onmain, which confirms they exercise the fix.Also manually reproduced: the repro above now exits 1 with a clear per-package error naming
is-odd@3.0.1(and its transitiveis-number@6.0.0) under bothbun installandbun install --frozen-lockfile, and succeeds cleanly when the packages are added tominimumReleaseAgeExcludes.