Repository navigation
fix(libnpmexec): honor min-release-age-exclude in npx - #9768
Sanjays2402 wants to merge 5 commits into
Conversation
npx calls pacote.manifest directly with flatOptions.before set by min-release-age, but pacote has no notion of min-release-age-exclude so npx would fail with ETARGET even for packages that should be exempt. arborist already does the equivalent per-spec drop inside npm install; do the same in libnpmexec's getManifest by clearing before when the resolved spec name matches an exclude pattern. Fixes: npm#9765
The manifest lookup cache was module-scoped and keyed only by spec.raw, while min-release-age-exclude is applied only on a cache miss. The first exec() for a spec matching the exclude list resolved and cached a manifest with the `before` cutoff dropped; a later exec() for the same spec without the exclusion reused that cached manifest and bypassed its release-age restriction. Move the cache into a per-exec() Map and thread it through getManifest and all three missingFromTree() call sites, so caching still avoids duplicate fetches within a single exec() while policy-specific results no longer leak across separate exec() invocations.
|
Good catch, thanks — the leak is real and your reproduction matches what I see. I've pushed the cache scoping in On the two-call regression test — I don't have one I'm willing to push yet, and I'd rather say that than push something that only looks like coverage. The shapes I tried:
The thing that makes this awkward under Do you have a preference for the shape here? Either:
I'd lean toward the first since it tests the actual user-visible symptom, but it's the more expensive one to get deterministic. Happy to write whichever you'd rather review. |
There was a problem hiding this comment.
🟡 Changes recommended
The new runtime dependency lacks its generated DEPENDENCIES.md edge, so dependency regeneration will leave a dirty tree and fail the libnpmexec CI check.
3 open issues
What changed in this PR
This PR adds release-age exclusions to libnpmexec manifest resolution so npx can honor min-release-age-exclude, aligning with npm install and addressing #9765.
Changes:
- Applies exclusions using hardened glob matching and alias-target names.
- Scopes manifest caching to each execution to prevent policy results leaking across calls.
- Adds helper and registry regression tests, with dependency and lockfile updates.
| File | Description |
|---|---|
workspaces/libnpmexec/test/release-age-exclude.js |
Tests matching, aliases, option passthrough, and non-mutation. |
workspaces/libnpmexec/test/registry.js |
Tests exclusion-policy isolation across execution calls. |
workspaces/libnpmexec/package.json |
Adds the direct minimatch dependency. |
workspaces/libnpmexec/lib/release-age-exclude.js |
Adds exclusion matching and per-spec cutoff adjustment. |
workspaces/libnpmexec/lib/index.js |
Applies exclusions and confines manifest caching to one call. |
package-lock.json |
Records the workspace dependency and an optional development peer. |
Review effort: Max
Keep release-age cutoffs for non-registry specs, cover hardened matching and alias identity, and update the generated dependency graph. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 067c8fbe-a9f3-4aab-b6d6-c5f730c65705
There was a problem hiding this comment.
🔵 Needs a closer look
The npx min-release-age bypass relaxes a supply-chain control and intentionally differs from arborist for named non-registry specs, requiring maintainer sign-off on the policy semantics.
No open issues 🎉
3 resolved since last review
Review effort: Max
Mock each execution's manifest lookup now that the cache is scoped per call. Remove the unrelated optional peer entry pruned by CI dependency setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 067c8fbe-a9f3-4aab-b6d6-c5f730c65705



Description
npxcallspacote.manifest(spec, { ...flatOptions })directly, which forwardsflatOptions.before(set bymin-release-age) to pacote. pacote has no concept ofmin-release-age-exclude, so even packages whose name matches an exclude pattern are rejected withETARGET.arboristalready handles this per-spec insidenpm installby droppingbeforewhen the resolved package name matches;libnpmexecneeds to do the same.Change in
workspaces/libnpmexec/lib/index.js: before callingpacote.manifestingetManifest, run the spec through a new helper (applyReleaseAgeExclude) that returns a shallow copy offlatOptionswithbeforeset tonullwhen the spec's trusted registry name matchesminReleaseAgeExclude.npm:aliases are keyed on the alias target, mirroring arborist'strustedSpecName. The pattern semantics match arborist exactly (nonegate,nocomment,noext) so an exemption list can never silently widen into match-all.Repro before the fix, with
~/.npmrc:After the fix, the exempt package resolves normally; unrelated packages still respect the cutoff.
Existing Issue
Fixes #9765
Test Coverage
New unit tests in
workspaces/libnpmexec/test/release-age-exclude.jscover the helper end to end: exact and glob patterns, hardened!/#/extglob semantics,npm:alias resolution, non-mutation of the input options, and the passthrough branches (nobefore, no match, empty/undefined options). File coverage is 100%. The existinglibnpmexecsuite continues to pass, since the wire-in is a passthrough wheneverbeforeis unset (the norm in existing tests).Verified both directions on the affected test file: it fails on
main(module missing) and passes with the patch applied.AI Disclosure
This change was written with the assistance of Claude. I have reviewed the diff and the test and take responsibility for the code.