fix(libnpmexec): honor min-release-age-exclude in npx#9768
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
|
|
||
| // when checking the local tree we look up manifests, cache those results by | ||
| // spec.raw so we don't have to fetch again when we check npxCache | ||
| const manifests = new Map() |
There was a problem hiding this comment.
Thanks for adding min-release-age-exclude support to npx. I found one cache-lifetime issue that should be addressed before merging.
manifests is module-scoped cache and keyed only by spec.raw, while applyReleaseAgeExclude() runs only on a cache miss. If the first libnpmexec call excludes a package, it can cache a post-cutoff manifest resolved with before: null. A later call for the same package without the exclusion then reuses that manifest and bypasses its release-age restriction.
I reproduced this with two sequential calls: both executed the newer version, although the second call should have selected the older, cutoff-eligible version.
Could we scope the manifest cache to each exec() invocation and pass it through all three missingFromTree()? That preserves caching between the local and npx-tree checks while preventing policy-specific results from leaking across calls.
const getManifest = async (spec, flatOptions, manifestCache) => {
if (!manifestCache.has(spec.raw)) {
...
manifestCache.set(spec.raw, manifest)
}
return manifestCache.get(spec.raw)
}
...
const missingFromTree = async ({
spec,
tree,
flatOptions,
manifestCache,
isNpxTree,
shallow,
}) => {
// Existing tree-checking logic...
const manifest = await getManifest(spec, flatOptions, manifestCache)
return { manifest }
} else {
const manifest = await getManifest(spec, flatOptions, manifestCache)
// Existing logic...
}
}Please also add a two-call regression test covering this scenario.
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.
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.