Repository navigation
fix(sc4): keep npm link slots from borrowing ancestor pins - #1106
Merged
Merged
Conversation
A lockfileVersion 2/3 `link: true` install records `resolved` and `link` but no `name` or `version`, so its slot was left out of the nearest layer and ChainMap returned an ancestor's registry pin for it. A workspace whose `lodash: file:...` dependency is linked to local code then inherited the root's `lodash@4.17.20`, and SC4 attributed that release's advisories to the workspace. Record every occupied install slot in its owner's layer, as Node's module lookup does. An alias, a `link: true` install or a slot without a version stays unresolved (None), as alias slots already did; any other slot keeps its own version. The ancestor install is still audited through its lockfile. Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #940 for the open review thread on
link: trueslots (comment).npm lockfileVersion 2/3 records a local-link install as
{"resolved": ..., "link": true}, with nonameorversion._collect_npm_locked_versionsstored nothing for such a slot, so it was missing from its owner's layer andChainMapreturned an ancestor's registry pin instead. Example: the root installslodash@4.17.20andpackages/a/node_modules/lodashlinks to a local package. The workspace'slodash: file:...dependency then resolved to 4.17.20, and SC4 reported that release's advisories againstpackages/a/package.jsonas a version-matched finding.Changes
_collect_npm_locked_versions: every occupiednode_modulesinstall slot now writes its name into its owner's layer, because Node's module lookup finds the nearest slot first. A slot keeps its own version unless it is an alias, alink: trueinstall or has no version. Those stay unresolved (None), which is how alias slots were already handled. The alias branch is folded into the same rule without a behavior change.test_npm_workspace_link_shadows_ancestor_registry_pincovers three slot shapes (a link, a link with a stray version, a versionless slot) across lockfileVersion 2 and 3,package-lock.jsonandnpm-shrinkwrap.json, plain and scoped names, and component order.test_npm_workspace_link_does_not_send_the_ancestor_pin_to_osvchecks end to end, throughnode(), that OSV is never asked about the root's version for the workspace.Behavior
('lodash', None)instead of('lodash', '4.17.20'). OSV is queried by name only, which yields at most a LOW "Unverifiable Dependency" result, the same as for an alias slot. This also closes the opposite case: a patched root pin no longer hides a vulnerable vendored copy behind the link.file:installs that record a version keep it.file:spec in its own slot, so it never borrowed an ancestor pin.Validation
mainand pass with this change.ruff checkandruff format --checkpass on the changed files.maineverywhere except unpinned, non-alias slots, which now stay unresolved.-m "not integration and not provider", Python 3.14, run locally on a heavily loaded machine): 25,190 passed, 19 failed. None of the failures involves this change. 18 are wall-clock or timeout tests (subprocess cold imports, worker startup, elapsed-time limits) that fail the same way onmain(17c4d38) or pass and fail at random under load.test_deep_npm_own_lock_keeps_parse_error_without_borrowing_parent_pinfails identically onmain, because on Python 3.14jsonno longer raisesRecursionErrorfor deeply nested input (CI uses Python 3.12).test-unit;publishwas skipped.🤖 Generated with Claude Code