Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions package-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -6203,6 +6203,18 @@
}
}
},
"node_modules/git-raw-commits/node_modules/conventional-commits-filter": {
"version": "5.0.0",
"resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-5.0.0.tgz",
"integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==",
"dev": true,
"license": "MIT",
"optional": true,
"peer": true,
"engines": {
"node": ">=18"
}
},
"node_modules/github-slugger": {
"version": "2.0.0",
"dev": true,
Expand Down Expand Up @@ -15077,6 +15089,7 @@
"@npmcli/package-json": "^8.0.0",
"@npmcli/run-script": "^11.0.0",
"ci-info": "^4.0.0",
"minimatch": "^10.0.3",
"npm-package-arg": "^14.0.0",
"pacote": "^22.0.0",
"proc-log": "^7.0.0",
Expand Down
10 changes: 9 additions & 1 deletion workspaces/libnpmexec/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,23 @@ const noTTY = require('./no-tty.js')
const runScript = require('./run-script.js')
const isWindows = require('./is-windows.js')
const withLock = require('./with-lock.js')
const { applyReleaseAgeExclude } = require('./release-age-exclude.js')

// 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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


const getManifest = async (spec, flatOptions) => {
if (!manifests.has(spec.raw)) {
// Honor `min-release-age-exclude` for the top-level exec spec the
// same way arborist does inside `npm install`: if the requested
// package name matches the exclude list, drop the `before` cutoff
// that `min-release-age` set on flatOptions before handing the
// options to pacote. Without this, `npx @myscope/foo` fails with
// ETARGET even though `min-release-age-exclude=@myscope/*` is set.
const manifestOptions = applyReleaseAgeExclude(spec, flatOptions)
const manifest = await pacote.manifest(spec, {
...flatOptions,
...manifestOptions,
preferOnline: true,
Arborist,
_isRoot: true,
Expand Down
56 changes: 56 additions & 0 deletions workspaces/libnpmexec/lib/release-age-exclude.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Mirrors the semantics used by @npmcli/arborist so `npx` honors
// `min-release-age-exclude` the same way `npm install` does.
//
// When the config layer sets `flatOptions.before` from `min-release-age`,
// pacote will refuse to resolve versions published after that cutoff.
// pacote itself has no notion of `min-release-age-exclude`; arborist
// applies it per-spec by dropping `before` for matched names. libnpmexec
// calls `pacote.manifest` directly, so it has to do the same.
//
// Patterns are exact names or minimatch globs against the resolved
// package name. Only the named package is exempt; its own dependencies
// still follow the release-age policy unless they also match a pattern.
const { minimatch } = require('minimatch')

// Match arborist's hardened option set: `nonegate` keeps a leading `!`
// literal so a stray `!foo` exempts nothing instead of everything-but-foo,
// `nocomment` keeps `#` literal, and `noext` disables extglobs. This list
// only ever widens the exemption, so we never want a pattern feature to
// silently turn into match-all.
const minimatchOptions = { nonegate: true, nocomment: true, noext: true }

const isReleaseAgeExcluded = (name, patterns) => {
if (!name || !Array.isArray(patterns) || patterns.length === 0) {
return false
}
return patterns.some(pattern =>
name === pattern || minimatch(name, pattern, minimatchOptions))
}

// For `npm:` aliases the fetched package is the alias target, not the
// alias key, so the exemption must be keyed on the underlying name.
// Mirrors `trustedSpecName` in arborist's release-age-exclude.js.
const trustedSpecName = (spec) => {
if (!spec) {
return undefined
}
if (spec.type === 'alias' && spec.subSpec && spec.subSpec.registry) {
return spec.subSpec.name
}
return spec.name
}

// Return a shallow copy of `flatOptions` with `before` cleared when the
// spec's trusted name matches an exclude pattern. Non-mutating so shared
// option objects aren't clobbered for later, unrelated specs.
const applyReleaseAgeExclude = (spec, flatOptions) => {
if (!flatOptions || flatOptions.before == null) {
return flatOptions
}
if (!isReleaseAgeExcluded(trustedSpecName(spec), flatOptions.minReleaseAgeExclude)) {
return flatOptions
}
return { ...flatOptions, before: null }
}

module.exports = { isReleaseAgeExcluded, trustedSpecName, applyReleaseAgeExclude }
1 change: 1 addition & 0 deletions workspaces/libnpmexec/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"@npmcli/package-json": "^8.0.0",
"@npmcli/run-script": "^11.0.0",
"ci-info": "^4.0.0",
"minimatch": "^10.0.3",
"npm-package-arg": "^14.0.0",
"pacote": "^22.0.0",
"proc-log": "^7.0.0",
Expand Down
80 changes: 80 additions & 0 deletions workspaces/libnpmexec/test/release-age-exclude.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
const t = require('tap')
const npa = require('npm-package-arg')
const {
isReleaseAgeExcluded,
trustedSpecName,
applyReleaseAgeExclude,
} = require('../lib/release-age-exclude.js')

t.test('isReleaseAgeExcluded: empty / invalid patterns', async t => {
t.equal(isReleaseAgeExcluded('lodash', []), false)
t.equal(isReleaseAgeExcluded('lodash', undefined), false)
t.equal(isReleaseAgeExcluded('lodash', null), false)
t.equal(isReleaseAgeExcluded(undefined, ['*']), false)
t.equal(isReleaseAgeExcluded('', ['*']), false)
})

t.test('isReleaseAgeExcluded: exact and glob patterns', async t => {
t.equal(isReleaseAgeExcluded('lodash', ['lodash']), true, 'exact match')
t.equal(isReleaseAgeExcluded('lodash', ['other']), false, 'exact miss')
t.equal(isReleaseAgeExcluded('@myorg/foo', ['@myorg/*']), true, 'scope glob match')
t.equal(isReleaseAgeExcluded('@other/foo', ['@myorg/*']), false, 'scope glob miss')
})

t.test('isReleaseAgeExcluded: hardened glob semantics', async t => {
// `!foo` must NOT act as a negation exempting everything but foo.
t.equal(isReleaseAgeExcluded('bar', ['!foo']), false,
'leading ! stays literal and does not exempt unrelated names')
// `#foo` must NOT be treated as a comment (which would match nothing).
t.equal(isReleaseAgeExcluded('#foo', ['#foo']), true,
'leading # is literal')
})

t.test('trustedSpecName: registry, alias, and edge cases', async t => {
t.equal(trustedSpecName(undefined), undefined)
t.equal(trustedSpecName(npa('lodash@1.2.3')), 'lodash', 'registry spec')
// npm: alias — the fetched package is the alias target.
t.equal(trustedSpecName(npa('foo@npm:@myorg/real@1')), '@myorg/real',
'alias resolves to underlying package name')
})

t.test('applyReleaseAgeExclude: no before => passthrough', async t => {
const opts = { before: null, minReleaseAgeExclude: ['@myorg/*'] }
t.equal(applyReleaseAgeExclude(npa('@myorg/foo@1'), opts), opts,
'returns same object when there is no cutoff to clear')
})

t.test('applyReleaseAgeExclude: not excluded => passthrough', async t => {
const opts = { before: new Date('2026-01-01'), minReleaseAgeExclude: ['@myorg/*'] }
t.equal(applyReleaseAgeExclude(npa('lodash@1'), opts), opts,
'unrelated package keeps the before cutoff')
})

t.test('applyReleaseAgeExclude: excluded => before cleared, non-mutating', async t => {
const before = new Date('2026-01-01')
const opts = { before, minReleaseAgeExclude: ['@myorg/*'], other: 'x' }
const out = applyReleaseAgeExclude(npa('@myorg/foo@1.2.3'), opts)
t.not(out, opts, 'returns a copy')
t.equal(opts.before, before, 'input untouched')
t.equal(out.before, null, 'before cleared for excluded spec')
t.equal(out.other, 'x', 'other options preserved')
t.same(out.minReleaseAgeExclude, ['@myorg/*'], 'exclude list preserved')
})

t.test('applyReleaseAgeExclude: alias excluded by alias-target name', async t => {
const opts = {
before: new Date('2026-01-01'),
minReleaseAgeExclude: ['@myorg/real'],
}
const out = applyReleaseAgeExclude(npa('foo@npm:@myorg/real@1'), opts)
t.equal(out.before, null,
'alias exemption keyed on underlying package name, not alias key')
})

t.test('applyReleaseAgeExclude: null / empty options', async t => {
t.equal(applyReleaseAgeExclude(npa('lodash@1'), null), null)
t.equal(applyReleaseAgeExclude(npa('lodash@1'), undefined), undefined)
const opts = {}
t.equal(applyReleaseAgeExclude(npa('lodash@1'), opts), opts,
'no before => passthrough')
})