Repository navigation
fix(scripts): scope a hoisted npm copy by the ranges it satisfies - #5529
Conversation
The alert sweep treated the hoisted root copy of a package as serving every manifest that declares it. A workspace whose declared range that copy does not satisfy has a nested copy of its own, so for it the root copy is only transitive. #5481 got the wrong fix (raise src/filesystem's range) and the wrong scope label (server-filesystem) because of it. npmDeclarers now returns each declarer's range, and npmCopyDeclarers counts the root copy as a manifest's only when semver.satisfies holds. The EOVERRIDE warning is now given only when the root package.json declares the package, which is npm's actual rule (verified against npm 11.20: a workspace's declaration installs cleanly under a package-wide override). Closes #5506 Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: cliffhall <[email protected]>
|
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the hoisted-dependency attribution and override-guidance changes.
No open issues 🎉
What changed in this PR
Fixes the Dependabot alert sweep’s hoisted npm dependency attribution, addressing #5506 so generated fix instructions and server labels reflect the declared ranges that a vulnerable copy satisfies.
Changes:
- Records declared ranges and uses semver matching to identify which manifests a hoisted copy serves, preserving fallback behavior for unreadable ranges.
- Limits parent-scoped override advice and
EOVERRIDEwarnings to root-package declarations. - Adds regression coverage for a vulnerable hoisted copy alongside a fixed workspace copy.
| File | Description |
|---|---|
scripts/dependabot-alerts.test.mjs |
Updates declaration fixtures and tests hoisted-copy attribution, labels, and override advice. |
scripts/dependabot-alerts.mjs |
Filters hoisted-copy declarers by range and corrects override scoping guidance. |
Review effort: Balanced
|
Copilot round 1: clean, no findings and no suppressed comments. Review loop converged on the first round. |
Closes #5506
Description
The Dependabot alert sweep (
scripts/dependabot-alerts.mjs) treated a hoisted root copy of a package as the declared copy of every manifest that declares the package. A workspace whose declared range that copy does not satisfy has its own nested copy, so for that workspace the root copy is only transitive. For #5481 this produced the wrong fix ("raise the declared range insrc/filesystem/package.json", already^11.0.4), nooverridesstep, and a wrongserver-filesystemlabel.npmDeclarersnow returns{ manifest, range }for each declaring manifest.npmCopyDeclarerscounts the hoisted root copy as a manifest's declared copy only whensemver.satisfies(version, range). A range semver cannot read (an alias, a URL) keeps the old behavior. A workspace's own top-level copy is unchanged.EOVERRIDEwarning, and the parent-scopedoverridesform it requires, now apply only when the rootpackage.jsondeclares the package, which is npm's actual rule (verified below).Server Details
None (repository-wide): the alert sweep script and its tests.
Motivation and Context
#5506, found while fixing #5481.
How Has This Been Tested?
No client-observable surface (a sweep script), so targeted probes:
minimatchto11.0.1inpackage-lock.json(1 advisory) #5481 lockfile shape:src/filesystemdeclaresminimatch ^11.0.4with its own nested11.0.4, and a hoisted10.2.5pulled in only by dev tooling. Asserts the root copy has no declarer, the labels carry no server scope, and the body asks for anoverridesentry with no "Raise the declared range" step and noEOVERRIDEnote. Run against the script fromv2/main, it fails (actual: [ 'src/filesystem/package.json' ]); against this branch, all 45 tests pass.EOVERRIDErule, against npm 11.20.0 (npm install --package-lock-onlyin scratch projects):adeclaringminimatch ^11.0.4, rootoverrides: { "minimatch": "^11.0.1" }: installs cleanly;minimatch ^10.0.0itself with the same override:npm error code EOVERRIDE — Override for minimatch@^10.0.0 conflicts with direct dependency.npm run local:gateexits 0.Breaking Changes
None.
Types of changes
Checklist
🤖 Generated with Claude Code