Conversation
…ble version
When the installed version of a vulnerable package comes from an
overrides/resolutions rule in the root package.json, the plan used to
print the rule's range as if each dependent had declared it
("a@1.0.0 depends on b@1.0.0"), offered no remedy, and --latest could
not get past it either, since pin_for refused every overridden edge.
Edges held by a rule are now attributed to the rule: the blocked
section prints one "package.json overrides b@<range>" line per rule
(with the rule's key when it is scoped), and the rule's value is
rewritten in place the way dependency pins and catalog entries are:
an exact value whenever the fix stays within its caret range, any
range under --latest. The entry is located by the normalized rule
rather than by key text, so every accepted spelling is handled
(plain, "a>b", "a/b", "**/b", nested objects, "." and "name@range"
keys); a value written as a $reference is reported as the blocker and
left alone. --json carries the rule key as "override" on blockers and
package.json edits.
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
Comment |
|
Status: ready for review; needs a maintainer's eye on the Reproduced on main with the cases now in test/cli/install/bun-audit.test.ts (a root 8934192 addresses the two review notes (every entry spelling the same rule is rewritten, planning reads the last one; CI (build 97070, final): 178 of 179 jobs passed. The one non-passing job is a |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
OverrideMap keeps the last of several entries that normalize to one rule, so the planner now reads the last matching entry and the edit rewrites each matching entry that holds the version, instead of both stopping at the first one.
There was a problem hiding this comment.
Both earlier inline findings are addressed in 8934192 — for_each_override_value now visits every matching entry (planning reads the last, the edit rewrites each one holding the old value), and the override_literal doc comment is accurate. No further issues found this pass.
Given the scope — ~700 lines across the audit-fix planner, package.json rewriting, OverrideMap, and a new override key in the --json output — a human look is still worthwhile.
What was reviewed
pin_forrefactor: override edges now produce anEditSite::Overrideedit; bundled edges stay attributed to the dependent;$refvalues compared against the lockfile literal and left alone.for_each_override_valuematches entries by re-parsing keys with the sameparse_selector/parse_package_segmenttheOverrideMapparser uses, so the eight accepted spellings agree; nested-object form gated onoverridesonly.OverrideMap::lookup/getanddedupe::effective_versionare behavior-preserving refactors — same alias/workspace exclusions, same clone semantics.--jsonschema:overrideadded alongside existing keys; existing test assertions updated to includeoverride: null.
Extended reasoning...
Overview
Teaches bun audit fix to recognize when a vulnerable version is held by an overrides/resolutions rule in the root package.json, report it as such, and rewrite the rule under the same policy as dependency/catalog pins. Touches the audit-fix planner (audit_fix.rs), the package.json edit applier (package_json_edits.rs), the JSON output writer, dedupe.rs (extracts applied_override), and OverrideMap.rs (adds lookup returning a borrowed OverrideRule so callers can read the rule's shape, with get now a thin wrapper). ~300 lines of new test coverage in bun-audit.test.ts, plus docs.
Prior findings
Two inline comments from the previous run were both addressed in 8934192:
- The single-slot
override_rule_valuewas replaced with a visitor (for_each_override_value) so duplicate spellings of one rule are all rewritten; twotest.eachrows cover both-rewritten and shadowed-first cases. - The misleading
override_literaldoc comment was reworded; the$refhandling is now documented at the equality check inpin_forwhere it actually happens.
Security risks
None identified. The code reads and rewrites the user's own root package.json based on lockfile contents and registry advisories; no untrusted-input parsing beyond what bun install already does. The written == literal guard ensures an edit is only planned when the file text matches what bun.lock recorded, so $ref values and hand-edited entries aren't silently overwritten with something else.
Level of scrutiny
Medium-high. This is package-manager code that rewrites user files (package.json) and changes the shape of bun audit fix --json output (adds an override key to blockers and packageJson edits). The refactor of pin_for changes how every edge is classified, and effective_version in dedupe.rs now goes through applied_override — I traced that both preserve the prior alias/workspace: exclusion and clone semantics. The OverrideMap::get → lookup split is mechanical.
Other factors
- Test coverage is thorough: direct and transitive dependents, eight key spellings across
overridesandresolutions,$refleft alone, scoped rule blocked and rewritten,--jsonshape, duplicate-spelling rows. The PR body confirms 25 targeted failures on main and a clean 189-test pass on the branch, plus overrides/nested-overrides/dedupe/update-transitive suites (395 tests) covering the refactor. - Not approving automatically because of size and because it changes user-facing
--jsonoutput and file-rewriting behavior; a maintainer should confirm theoverrideJSON key naming and the(overrides ...)text-output format are what they want.
This branch has not been deployed
2 participants
Problem
bun audit fixon a project whoseoverridesentry pins the vulnerable version blames the third-party dependents: the plan printsa@1.0.0 depends on b@1.0.0andc@1.0.0 depends on b@1.0.0althoughaandcdeclareb@^1.0.0;1.0.0is the override's value.bun audit fix --latestalso ends inFixed 0 of 1, so the one file the user controls is never named or touched. docs/pm/cli/audit.mdx even suggests adding an override as the way out.src/install/audit_fix.rsbuilds eachBlockerfrom the edge's post-override range (effective_npm_range) and labels it with the dependent, andpin_forreturnedNonefor every edge matched bylockfile.overrides, so neither mode had an edit to offer.audit fixto meet.Fix
OverrideMap::lookupreturns the rule that applies to an edge (getis now a wrapper over it);dedupe::applied_overrideadds the existing "aliases andworkspace:edges are never overridden" condition, andeffective_versionis built on it, so the planner and the range computation agree on which rule governs an edge.package.json overrides b@<value>, followed by the rule's key when it is scoped ((a>b),(b@<1.1.0)); all edges held by one rule collapse into one line. Bundled edges stay attributed to the package that bundles them.pin_forplans an edit for the rule's value under the same policy as dependency pins and catalog entries: an exact value is rewritten whenever the fix stays within^current, any range is rewritten under--latest, and thebun audit fix --latesthint appears in the blocked section when that would help. A rule whose value iscatalog:produces the usual catalog edit.PackageJsonEdit.catalogbecomesEditSite { Dependencies, Catalog, Override(OverrideSelector) }. Applying an override edit walksoverrides(elseresolutions) and matches each entry by its normalized selector, so the entry is found in any accepted spelling:"b","a>b","a/b","**/b",{"a": {"b": ..}},{"b": {".": ..}},"a@1>b","b@<1.0.1". The edit is only planned when the entry's current text equals the value bun.lock recorded; an entry written as"$dep"is therefore reported as the blocker and left alone instead of being claimed as fixed.package.json (overrides): 1.0.0 -> 1.0.1, or(overrides a>b)for a scoped rule.--json: blockers andpackageJsonedits gain"override"(the rule key,nullelsewhere); the other keys are unchanged.$ref, scoped rule blocked and rewritten,--json) fail on the unfixed build (25 failures, 14 of them these cases, the rest theoverrideJSON key) and the whole file passes with it (189 tests).OverrideMap/effective_versionrefactor; the byte-search and pub-exports source lints pass.Background
overrides(npm) /resolutions(yarn) in the root package.json replace the range a dependent declares for a package. bun stores each rule in the lockfile'sOverrideMapin a normalized form: either a flat rule keyed by name, or aScopedOverridecarrying an optional parent (name plus optional range) and an optional target range; the key text the user wrote is not kept, which is why the edit matches entries by re-parsing their keys with the same selector parserOverrideMapuses.$namevalue copies the range declared fornamein the project's own dependencies; bun.lock records the resolved range, so it is the one case where the file and the lockfile literal differ.bun audit fixplans per installed vulnerable version ("instance") over the edges that resolve to it. An edge either accepts a safe candidate through its effective range (fixed in bun.lock only), or through aPackageJsonEdit("pin") that will be rewritten before the install runs; edges with neither becomeBlockers. After a package.json edit, the regular install differ notices the changed override and re-resolves every edge of that name, which is what moves the dependents onto the fixed version.[review] gate passed · iteration 0 · 7 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 0
evidence per changed file