fix: include transitive peer dependencies in npm lock v2/v3 graph [OSM-3794]#316
Open
gergo-papp wants to merge 1 commit into
Open
fix: include transitive peer dependencies in npm lock v2/v3 graph [OSM-3794]#316gergo-papp wants to merge 1 commit into
gergo-papp wants to merge 1 commit into
Conversation
…M-3794] npm v7+ installs peerDependencies by default and records them in the lockfile (flagged "peer": true). The V2/V3 dep-graph builder only walked peerDependencies for the root package — getChildNode collected dependencies/devDependencies/optionalDependencies but never peers — so a peer dependency of a (transitive) dependency was dropped from the graph and its vulnerabilities went unreported. getChildNode now merges a resolved package's peerDependencies into its subtree via a new getPeerDependencies helper, which: - excludes peers marked optional in peerDependenciesMeta (npm v7+ does not auto-install those), and - flags entries isPeer so an unmet/conflicting peer with no lockfile entry is skipped rather than raising OutOfSyncError. Earlier attempts that threw here (PRs #233/#239, epic OSM-2056) were reverted for this. Verified purely additive across existing fixtures (no nodes/edges removed). Adds a transitive-peer-deps fixture and peerDependencies tests. Co-Authored-By: Claude Opus 4.8 <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.
What this does
Fixes OSM-3794: Snyk misses vulnerabilities in the peer dependencies of a (transitive) dependency for npm v7+ projects.
From npm v7 onwards,
peerDependenciesare installed by default and recorded in the lockfile (flagged"peer": true). The npm lock v2/v3 dep-graph builder honoured this only for the root package (getTopLevelDeps(..., includePeerDeps: true)) —getChildNodecollecteddependencies/devDependencies/optionalDependenciesbut neverpeerDependencies. As a result, a peer dependency of any non-root package (e.g. a direct dependency's peer, or a peer-of-a-peer) was silently dropped from the dep graph and its vulnerabilities went unreported.This also covers the duplicate reports OSM-3645, OSM-2116 and OSM-1226.
How it's fixed
getChildNodenow merges a resolved package'speerDependenciesinto its subtree, via a newgetPeerDependencieshelper inutil.ts.peerDependenciesMeta[name].optional === true) are excluded — npm v7+ does not auto-install those (see OSM-1079).isPeer, so an unmet/conflicting peer with no lockfile entry is skipped rather than raisingOutOfSyncError. Previous attempts that threw here (PRs fix: [OSM-1079] support the parsing of transitive peer deps - if they are installed #233 / Fix/ignore optional deps if not installed #239, epic OSM-2056) were reverted for exactly this reason.dependencies/optional/deventry for the same name takes precedence over the peer range.Notes for the reviewer
"peer": trueentries. The legacy v1 (buildDepTree) path is untouched.expected.jsonfixtures were regenerated and verified purely additive (node/edge set comparison: 0 nodes removed, 0 edges removed in every fixture — only new, genuine peer edges such asreact-dom → react,@babel/* → @babel/core,ajv-errors → ajv,cosmiconfig-typescript-loader → typescript). The largedeeply-scopeddiff is only DFS traversal-order reshuffling of the same nodes.transitive-peer-depsfixture +peerDependenciestest block assert the peer subtree is included and that an optional, uninstalled peer is skipped without throwing understrictOutOfSync: true.consumer-app-using-remote-lib-2): now resolvesminimatch → brace-expansion → balanced-match.More information
🤖 Generated with Claude Code