From 2fad81b5cb1cde91fa6df9e770e35ac358592d62 Mon Sep 17 00:00:00 2001 From: Manzoor Wani Date: Fri, 26 Jun 2026 19:00:42 +0530 Subject: [PATCH] fix(arborist): apply overrides across a file:/workspace link boundary (#9671) (backport release/v11) A root `overrides` entry targeting a transitive dependency was silently dropped when the path to that dependency crossed a `file:`/workspace link, so the dependency resolved to its un-overridden version and the lockfile pinned the wrong version. It reproduced under both the `hoisted` and `linked` install strategies, while the same override applied correctly when the dependency was reached without crossing a link. Override rules propagate through dependency edges, but a Link and its target are not edge-connected, so they are bridged by forwarding the Link's `OverrideSet` to its target. That forwarding ran while the target's subtree was still unbuilt, so its guard found no matching rule and never forwarded, leaving the target and its descendant edges without the rule. `buildIdealTree` now forwards a link's overrides to its target before the target's subtree is resolved, so descendant edges inherit the rule as they are added, matching how a registry node always inherits its ancestor's `OverrideSet`. `loadActual` now repropagates overrides through links once all edges are resolved, so a transitive override reached through a `file:` link is reported as `overridden` rather than `invalid` by `npm ls`. Fixes #9659 (cherry picked from commit 968e42fbd62eb3a6f446466359c9431f41d76b2b) --- .../arborist/lib/arborist/build-ideal-tree.js | 4 ++ .../arborist/lib/arborist/load-actual.js | 5 +- .../test/arborist/build-ideal-tree.js | 40 +++++++++++ .../arborist/test/arborist/load-actual.js | 66 +++++++++++++++++++ 4 files changed, 112 insertions(+), 3 deletions(-) diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 121b20246731b..63157b9a550de 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -1595,6 +1595,10 @@ This is a one-time fix-up, please be patient... !link.target.parent && !link.target.fsParent || unseenLink) { + // Forward the link's overrides before its subtree resolves, so a root override reaches a transitive dep across the link boundary (npm/cli#9659). + if (link.overrides) { + link.target.updateOverridesEdgeInAdded(link.overrides) + } this.addTracker('idealTree', link.target.name, link.target.location) this.#depsQueue.push(link.target) } diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index bf73856ec2ddc..2158f6d32ae1a 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -70,6 +70,7 @@ module.exports = cls => class ActualLoader extends cls { // only reset root flags if we're not re-rooting, // otherwise leave as-is calcDepFlags(tree, !options.root) + this.#repropagateOverrides() this.actualTree = treeCheck(tree) return this.actualTree }) @@ -198,8 +199,6 @@ module.exports = cls => class ActualLoader extends cls { this.#transplant(root) - this.#repropagateOverrides() - if (global) { // need to depend on the children, or else all of them // will end up being flagged as extraneous, since the @@ -402,7 +401,7 @@ module.exports = cls => class ActualLoader extends cls { } } - // Re-forward overrides through links after the tree is complete, since a store Link may forward before its subtree resolves and miss a transitive match (npm/cli#9619). + // Re-forward overrides through links once all edges are resolved, since a Link may forward before its subtree resolves and miss a transitive match (npm/cli#9619, #9659). #repropagateOverrides () { if (!this.#actualTree.overrides) { return diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 9bad026633823..6443f70fe58a8 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -3655,6 +3655,46 @@ t.test('overrides', async t => { t.equal(barEdge.to.version, '2.0.0') }) + t.test('overrides a nested dependency reached through a file: link — npm/cli#9659', async (t) => { + // A root override targeting a transitive dep must apply even when the path to that dep crosses a file:/workspace link boundary. + const registry = createRegistry(t, false) + const barPackuments = registry.packuments([ + { version: '1.0.0', dependencies: { baz: '^1.0.0' } }, + ], 'bar') + const barManifest = registry.manifest({ name: 'bar', packuments: barPackuments }) + const bazPackuments = registry.packuments(['1.0.0', '2.0.0'], 'baz') + const bazManifest = registry.manifest({ name: 'baz', packuments: bazPackuments }) + await registry.package({ manifest: barManifest }) + await registry.package({ manifest: bazManifest }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { + a: 'file:./pkgs/a', + }, + overrides: { + baz: '2.0.0', + }, + }), + pkgs: { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + }), + }, + }, + }) + + const tree = await buildIdeal(path) + + const barNode = tree.inventory.query('name', 'bar').values().next().value + const bazEdge = barNode.edgesOut.get('baz') + t.equal(bazEdge.to.version, '2.0.0', 'override applies across the file: link') + }) + t.test('does not override a nested dependency when parent spec does not match', async (t) => { const registry = createRegistry(t, false) const fooPackuments = registry.packuments([ diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index f699fa594f557..1c248bdba68ec 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -727,6 +727,72 @@ t.test('forwards a transitive override through a linked store link — npm/cli#9 t.equal(leafEdge.to.target.version, '2.0.0', 'edge resolves to the overridden package') }) +t.test('forwards a transitive override across a file: link boundary — npm/cli#9659', async t => { + // The override path crosses a file: link (root -> a) before entering the store chain (a -> b -> leaf). + // Loading from the hidden lockfile, the file link target's subtree resolves late, so override repropagation must run once all edges are resolved or `npm ls` reports the edge `invalid`. + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + dependencies: { a: 'file:./pkgs/a' }, + overrides: { leaf: '2.0.0' }, + }), + pkgs: { + a: { + 'package.json': JSON.stringify({ name: 'a', version: '1.0.0', dependencies: { b: '1.0.0' } }), + node_modules: { + b: t.fixture('symlink', '../../../node_modules/.store/b@1.0.0/node_modules/b'), + }, + }, + }, + node_modules: { + a: t.fixture('symlink', '../pkgs/a'), + '.store': { + 'b@1.0.0': { + node_modules: { + // leaf is declared ^1.0.0 but overridden to 2.0.0, outside that range + b: { 'package.json': JSON.stringify({ name: 'b', version: '1.0.0', dependencies: { leaf: '^1.0.0' } }) }, + leaf: t.fixture('symlink', '../../leaf@2.0.0/node_modules/leaf'), + }, + }, + 'leaf@2.0.0': { + node_modules: { + leaf: { 'package.json': JSON.stringify({ name: 'leaf', version: '2.0.0' }) }, + }, + }, + }, + '.package-lock.json': JSON.stringify({ + name: 'root', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'root', version: '1.0.0', dependencies: { a: 'file:./pkgs/a' }, overrides: { leaf: '2.0.0' } }, + 'node_modules/a': { resolved: 'pkgs/a', link: true }, + 'pkgs/a': { version: '1.0.0', dependencies: { b: '1.0.0' } }, + 'pkgs/a/node_modules/b': { resolved: 'node_modules/.store/b@1.0.0/node_modules/b', link: true }, + 'node_modules/.store/b@1.0.0': {}, + 'node_modules/.store/b@1.0.0/node_modules/b': { version: '1.0.0', dependencies: { leaf: '^1.0.0' } }, + 'node_modules/.store/b@1.0.0/node_modules/leaf': { resolved: 'node_modules/.store/leaf@2.0.0/node_modules/leaf', link: true }, + 'node_modules/.store/leaf@2.0.0': {}, + 'node_modules/.store/leaf@2.0.0/node_modules/leaf': { version: '2.0.0' }, + }, + }), + }, + }) + // make the hidden lockfile the newest entry so loadActual loads from it + const hidden = resolve(path, 'node_modules/.package-lock.json') + const then = Date.now() + 10000 + fs.utimesSync(hidden, new Date(then), new Date(then)) + + const tree = await loadActual(path) + const b = tree.children.get('a').target.edgesOut.get('b').to.target + const leafEdge = b.edgesOut.get('leaf') + t.ok(leafEdge && !leafEdge.error, 'transitive overridden edge resolves without error') + t.ok(leafEdge.overrides, 'edge carries the override rule') + t.equal(leafEdge.spec, '2.0.0', 'edge spec is the overridden version') + t.equal(leafEdge.to.target.version, '2.0.0', 'edge resolves to the overridden package') +}) + t.test('store nodes do not load devDependencies as required edges', async t => { // A package in the linked store is structurally a tree top, so without the isInStore guard its devDependencies would load as required edges and surface as missing (e.g. npm sbom ESBOMPROBLEMS). const path = t.testdir({