Skip to content

Commit 822ce86

Browse files
fix(arborist): skip lockfile entries for optional deps with incomplete manifests (npm#9343)
### What When using a proxy/upstream registry (e.g. Azure Artifacts), npm 11 writes lockfile entries for platform-specific optional dependencies without `version`, `resolved`, or `integrity` fields. Subsequent `npm ci` fails with `Invalid Version:` errors. Example broken lockfile entry: ```json "node_modules/@esbuild/aix-ppc64": { "optional": true } ``` ### Why `#fetchManifest()` in `build-ideal-tree.js` fetches manifests for ALL platform variants of optional dependencies. Proxy registries that haven't cached packages for non-current platforms return incomplete metadata (missing `version` field). npm creates a Node with empty `pkg.version`, and `metaFromNode()` writes `{"optional": true}` to the lockfile without version. npm 10 never attempted to resolve non-current-platform variants, so this only affects npm 11+. ### How Two-layer defense: 1. **`build-ideal-tree.js` (`#nodeFromSpec`)**: When a registry manifest lacks a `version` field, treat it as an `EINCOMPLETEMANIFEST` load failure so that `#pruneFailedOptional()` marks it inert. Only applies to `spec.registry` specs (not `file:` dependencies which may legitimately omit version). 2. **`shrinkwrap.js` (`commit()`)**: Skip writing entries where the node is optional, has no version, and is not the root. This acts as a defense-in-depth layer. ### Testing - Unit test added to `workspaces/arborist/test/shrinkwrap.js` — simulates proxy registry scenario where an optional dep node has no version - All existing tests pass (`shrinkwrap.js` 45/45, `build-ideal-tree.js` 114/114) - Manually verified with Azure Artifacts upstream proxy feed: - Before fix: 46 broken lockfile entries (optional deps without version) - After fix: 0 broken entries - Output matches npm 10 behavior Closes npm#9342 --------- Co-authored-by: Michael Smith <owlstronaut@github.com>
1 parent 2c9587e commit 822ce86

4 files changed

Lines changed: 229 additions & 2 deletions

File tree

workspaces/arborist/lib/arborist/build-ideal-tree.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1351,7 +1351,22 @@ This is a one-time fix-up, please be patient...
13511351
// doesn't satisfy the edge. try to fetch a manifest and build a node from that.
13521352
return this.#fetchManifest(spec, parent, edge)
13531353
.then(
1354-
pkg => new Node({ name, pkg, parent, installLinks, legacyPeerDeps }),
1354+
pkg => {
1355+
// When a proxy/upstream registry returns an incomplete manifest
1356+
// (e.g. missing version field for platform-specific packages it
1357+
// hasn't cached), treat it as a load failure so that optional deps
1358+
// are properly pruned instead of written to the lockfile without
1359+
// version metadata. Only apply to registry specs — file: deps
1360+
// legitimately omit version.
1361+
if (!pkg.version && spec.registry) {
1362+
const error = Object.assign(
1363+
new Error(`incomplete manifest for ${name}, missing version`),
1364+
{ code: 'EINCOMPLETEMANIFEST' }
1365+
)
1366+
return this.#failureNode(name, parent, error, edge)
1367+
}
1368+
return new Node({ name, pkg, parent, installLinks, legacyPeerDeps })
1369+
},
13551370
error => this.#failureNode(name, parent, error, edge)
13561371
)
13571372
}

workspaces/arborist/lib/shrinkwrap.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,10 +909,20 @@ class Shrinkwrap {
909909
if (node.extraneous && !/(^|\/)node_modules\//.test(loc) && loc !== 'node_modules') {
910910
continue
911911
}
912-
this.data.packages[loc] = Shrinkwrap.metaFromNode(
912+
const meta = Shrinkwrap.metaFromNode(
913913
node,
914914
this.path,
915915
this.resolveOptions)
916+
// Skip inert nodes — these are optional deps that failed to load
917+
// (e.g. 404 from a proxy registry that hasn't cached the package,
918+
// or incomplete manifest missing version field).
919+
// #pruneFailedOptional marks them inert so they won't be reified;
920+
// writing them to the lockfile produces invalid entries like
921+
// {"optional": true} that cause "Invalid Version:" errors.
922+
if (node.inert && !node.package.version) {
923+
continue
924+
}
925+
this.data.packages[loc] = meta
916926
}
917927
} else if (this.#awaitingUpdate.size > 0) {
918928
for (const loc of this.#awaitingUpdate.keys()) {

workspaces/arborist/test/arborist/build-ideal-tree.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4816,3 +4816,70 @@ t.test('allow-directory=root soft-skips a transitive optional directory dependen
48164816
t.ok(optChild, 'blocked optional transitive is recorded in the tree')
48174817
t.equal(optChild.inert, true, 'blocked optional transitive is marked inert (will not be reified)')
48184818
})
4819+
4820+
t.test('incomplete manifest from proxy registry prunes optional dep (#9342)', async t => {
4821+
// When a proxy/upstream registry returns an
4822+
// incomplete manifest for a platform-specific optional dep it hasn't
4823+
// cached, the version field is missing. Our fix in #nodeFromSpec
4824+
// treats this as EINCOMPLETEMANIFEST load failure so that
4825+
// #pruneFailedOptional() marks it inert instead of writing a broken
4826+
// lockfile entry like {"optional": true}.
4827+
const registry = createRegistry(t, false)
4828+
4829+
// parent package with an optional dep
4830+
const esbuildPack = registry.packument({
4831+
name: 'esbuild',
4832+
version: '0.27.7',
4833+
optionalDependencies: {
4834+
'@esbuild/aix-ppc64': '0.27.7',
4835+
},
4836+
})
4837+
const esbuildManifest = registry.manifest({ name: 'esbuild', packuments: [esbuildPack] })
4838+
await registry.package({ manifest: esbuildManifest })
4839+
4840+
// simulate proxy registry returning incomplete manifest (no version field)
4841+
await registry.package({
4842+
manifest: {
4843+
_id: '@esbuild/aix-ppc64',
4844+
_rev: '00-incomplete',
4845+
name: '@esbuild/aix-ppc64',
4846+
description: 'incomplete proxy manifest',
4847+
'dist-tags': { latest: '0.27.7' },
4848+
versions: {
4849+
'0.27.7': {
4850+
_id: '@esbuild/aix-ppc64@0.27.7',
4851+
name: '@esbuild/aix-ppc64',
4852+
// NO version field — this is the proxy registry bug
4853+
dependencies: {},
4854+
dist: {
4855+
tarball: 'https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.27.7.tgz',
4856+
},
4857+
},
4858+
},
4859+
},
4860+
})
4861+
4862+
const path = t.testdir({
4863+
'package.json': JSON.stringify({
4864+
name: 'test-incomplete-manifest',
4865+
version: '1.0.0',
4866+
devDependencies: { esbuild: '^0.27.0' },
4867+
}),
4868+
})
4869+
4870+
const arb = newArb(path)
4871+
const tree = await arb.buildIdealTree()
4872+
4873+
// esbuild itself should be in the tree
4874+
t.ok(tree.children.get('esbuild'), 'esbuild is installed')
4875+
t.equal(tree.children.get('esbuild').version, '0.27.7', 'esbuild has correct version')
4876+
4877+
// @esbuild/aix-ppc64 should be marked inert (EINCOMPLETEMANIFEST → loadFailure)
4878+
// pruneFailedOptional marks it inert so it won't be written to lockfile
4879+
const aixNodes = [...tree.inventory.query('name', '@esbuild/aix-ppc64')]
4880+
const aixNode = aixNodes.find(n => n.root === tree)
4881+
t.ok(aixNode, 'incomplete optional dep node exists in tree')
4882+
t.equal(aixNode.inert, true, 'incomplete optional dep is marked inert')
4883+
t.equal(aixNode.errors[0].code, 'EINCOMPLETEMANIFEST',
4884+
'node has EINCOMPLETEMANIFEST error')
4885+
})

workspaces/arborist/test/shrinkwrap.js

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,141 @@ t.test('load a hidden lockfile', async t => {
863863
t.equal(data.dependencies, undefined, 'deleted legacy metadata')
864864
})
865865

866+
t.test('skip inert nodes in commit', async t => {
867+
// When a proxy registry returns 404 or incomplete manifests for
868+
// platform-specific optional deps, #pruneFailedOptional marks them
869+
// inert. commit() must skip inert nodes — otherwise the lockfile
870+
// gets entries like {"optional": true} without version/resolved/integrity
871+
// that cause "Invalid Version:" errors on subsequent npm ci.
872+
const path = t.testdir({
873+
'package.json': JSON.stringify({
874+
name: 'proxy-registry-repro',
875+
version: '1.0.0',
876+
devDependencies: { esbuild: '^0.27.0' },
877+
}),
878+
})
879+
const meta = new Shrinkwrap({ path })
880+
meta.data = {
881+
lockfileVersion: 3,
882+
packages: {},
883+
}
884+
885+
const root = new Node({
886+
pkg: {
887+
name: 'proxy-registry-repro',
888+
version: '1.0.0',
889+
devDependencies: { esbuild: '^0.27.0' },
890+
},
891+
path,
892+
realpath: path,
893+
})
894+
895+
// esbuild with full metadata (valid)
896+
const esbuild = new Node({
897+
pkg: {
898+
name: 'esbuild',
899+
version: '0.27.7',
900+
optionalDependencies: {
901+
'@esbuild/linux-x64': '0.27.7',
902+
'@esbuild/aix-ppc64': '0.27.7',
903+
},
904+
},
905+
name: 'esbuild',
906+
parent: root,
907+
})
908+
esbuild.dev = true
909+
910+
// platform dep with full metadata (current platform — valid, NOT inert)
911+
const validDep = new Node({
912+
pkg: {
913+
name: '@esbuild/linux-x64',
914+
version: '0.27.7',
915+
os: ['linux'],
916+
cpu: ['x64'],
917+
},
918+
name: '@esbuild/linux-x64',
919+
parent: root,
920+
})
921+
validDep.optional = true
922+
validDep.dev = true
923+
924+
// platform dep marked inert (proxy registry 404'd or returned incomplete manifest)
925+
// #pruneFailedOptional sets inert=true on these nodes
926+
const brokenDep = new Node({
927+
pkg: {
928+
name: '@esbuild/aix-ppc64',
929+
// no version — proxy registry returned 404 or incomplete manifest
930+
},
931+
name: '@esbuild/aix-ppc64',
932+
parent: esbuild,
933+
})
934+
brokenDep.optional = true
935+
brokenDep.dev = true
936+
brokenDep.extraneous = false
937+
brokenDep.inert = true
938+
939+
// file: optional dep WITHOUT version (legitimate — NOT inert, should be kept)
940+
const fileDep = new Node({
941+
pkg: {
942+
name: 'my-local-optional',
943+
// no version — but this is a file: dep, so it's legitimate
944+
},
945+
name: 'my-local-optional',
946+
parent: root,
947+
resolved: 'file:../my-local-optional',
948+
})
949+
fileDep.optional = true
950+
fileDep.extraneous = false
951+
952+
// local optional dep WITHOUT version or resolved (NOT inert, should be kept)
953+
const localDep = new Node({
954+
pkg: {
955+
name: 'my-disk-optional',
956+
// no version, no resolved — loaded from local node_modules
957+
},
958+
name: 'my-disk-optional',
959+
parent: root,
960+
})
961+
localDep.optional = true
962+
localDep.extraneous = false
963+
964+
meta.tree = root
965+
const committed = meta.commit()
966+
967+
// The valid platform dep should be in the lockfile
968+
const validLoc = 'node_modules/@esbuild/linux-x64'
969+
t.ok(
970+
committed.packages[validLoc],
971+
'valid optional dep is included'
972+
)
973+
t.equal(
974+
committed.packages[validLoc].version,
975+
'0.27.7',
976+
'valid optional dep has version'
977+
)
978+
979+
// The inert dep should NOT be in the lockfile
980+
const brokenLoc = 'node_modules/esbuild/node_modules/@esbuild/aix-ppc64'
981+
t.notOk(
982+
committed.packages[brokenLoc],
983+
'inert optional dep is excluded from lockfile'
984+
)
985+
986+
// The file: optional dep WITHOUT version SHOULD be kept (not inert)
987+
const fileLoc = 'node_modules/my-local-optional'
988+
t.ok(
989+
committed.packages[fileLoc],
990+
'file: optional dep without version is preserved in lockfile'
991+
)
992+
993+
// The local (resolved=null) optional dep WITHOUT version SHOULD be kept (not inert)
994+
const localLoc = 'node_modules/my-disk-optional'
995+
t.ok(
996+
committed.packages[localLoc],
997+
'local optional dep without version is preserved in lockfile'
998+
)
999+
})
1000+
8661001
t.test('load a fresh hidden lockfile', async t => {
8671002
const sw = await Shrinkwrap.reset({
8681003
path: hiddenLockfileFixture,

0 commit comments

Comments
 (0)