Skip to content

audit fix: never downgrade a package out of its installed release line - #39317

Open
robobun wants to merge 1 commit into
mainfrom
farm/2bb66791/audit-fix-no-cross-major-downgrade
Open

robobun wants to merge 1 commit into
mainfrom
farm/2bb66791/audit-fix-no-cross-major-downgrade

Conversation

@robobun

@robobun robobun commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • bun audit fix --latest on "aws-sdk": "^2.1130.0" prints v aws-sdk 2.1693.0 -> 1.18.0 (downgrade), rewrites package.json to ^1.18.0 and installs a 2013 release that the re-audit then flags as vulnerable (bun audit fix --latest downgrades a package across majors to a *more* vulnerable version (aws-sdk 2.1693.0 -> 1.18.0) #39309). The advisory covers every aws-sdk 2.x, so no upgrade is safe, and the planner falls back to the highest older release without any advisory.
  • The downgrade loop in plan_fixes (src/install/audit_fix.rs, the releases.iter().enumerate().rev() loop) accepts any release below the installed one. An older major is a different API, and the bulk advisory response only contains advisories that match the versions sent, so releases that far back only look safe.
  • Not limited to --latest: plain bun audit fix does the same through any dependent whose range admits the older major (peer-deps@1.0.0 declares no-deps@*; with every 2.x advised it installs no-deps@1.1.0). Without --latest on a root range it printed the downgrade as blocked and suggested --latest, which then performed it.
  • Found while testing: a downgrade that has to rewrite a declared range (--latest, ^1.1.0 with every release >=1.1.0 advised) wrote ^1.0.1 and then installed 1.1.0 again, because the rewritten row is re-resolved to the newest release its new range allows. The run ends with vulnerable after install and a changed package.json.

Fix

  • same_release_line filters downgrade candidates to the installed major (the installed minor below 1.0.0). The loop walks releases from newest to oldest, so it stops at the first release outside the line. With no candidates left the instance lands in unfixable ("no published version fixes"), which is also what --json reports.
  • Edited edges whose target is a downgrade are added to PlannedFix.edges, so the lockfile row is pinned to the planned version the same way peer rows already are. Upgrades through a rewritten range are unchanged.
  • Same-line downgrades still work (a compromised release rolled back to the previous one); the existing downgrade tests still pass unchanged.
  • Docs: the downgrade bullet in docs/pm/cli/audit.mdx describes the floor.
  • Tests, test/cli/install/bun-audit.test.ts. New cases that fail on the unfixed build (outputs below): "a safe release in an older major is not a downgrade candidate", "a dependent's wide range does not let a fix downgrade into an older major", "a 0.x package is not downgraded into an older minor line", --latest "never rewrites a range to downgrade into an older major" (the issue's scenario, text and --json), --latest "rewrites a range for a downgrade that stays in the installed major" and the catalog variant (these two cover the pin; on the unfixed build they end in vulnerable after install). "a 0.x package downgrades within its minor line" guards the allowed side of the 0.x rule.
  • The two existing tests that asserted a cross-major downgrade as the blocked target ("a range that rejects every safe release is blocked on the highest safe downgrade" and its --json twin) now use a same-major input (^1.1.0, advisory >=1.1.0, blocked on 1.0.1); their previous input is the new "older major" case above.
  • New registry fixture zero-major (0.4.0, 0.4.1, 0.5.0, 0.5.1) generated by create-zero-major-packages.ts, following create-catalog-packages.ts; no existing fixture has two 0.x minor lines.
  • Verified: bun bd test test/cli/install/bun-audit.test.ts passes (189 tests); the new cases fail on a build of main as shown below. cargo clippy -p bun_install is clean.

Not changed here: with --latest, an upgrade that crosses a major still plans the lowest safe release and lets the rewritten range resolve upward, which is the "picks 7.0.0 when 7.18.x exists" note at the end of the issue.

Background

  • bun audit fix asks the registry's bulk advisory endpoint about the versions currently in bun.lock. The response only lists advisories matching those versions, so the planner knows nothing about advisories that only affect other releases of the same package. After installing, it re-audits the new lockfile, which is where vulnerable after install comes from.
  • For each vulnerable installed version the planner builds a candidate list: safe newer releases in ascending order, then safe older releases in descending order. Each dependent edge takes the first candidate its range accepts; with --latest, a range declared in your own package.json or catalog accepts anything and gets rewritten (^1.0.0 -> ^2.0.0) instead of blocking.
  • A planned fix carries edges, the dependency rows to pin to the chosen version in the lockfile. Rows whose package.json range was rewritten were previously left to ordinary resolution, which picks the newest release the new range allows; that is fine for an upgrade and wrong for a downgrade.
Unfixed build (main) on the new tests
a safe release in an older major is not a downgrade candidate
  blocked by a dependent's range:
    v no-deps 2.0.0 -> 1.1.0 (downgrade)
      package.json depends on no-deps@^2.0.0
      bun audit fix --latest

a dependent's wide range does not let a fix downgrade into an older major
  fixing:
    v no-deps 2.0.0 -> 1.1.0 (downgrade)
  Fixed 1 vulnerability in 1 package (checked 2)

a 0.x package is not downgraded into an older minor line
  fixing:
    v zero-major 0.5.1 -> 0.4.1 (downgrade)

--latest: never rewrites a range to downgrade into an older major
  fixing:
    v no-deps 2.0.0 -> 1.1.0 (downgrade)
      package.json: ^2.0.0 -> ^1.1.0
  Fixed 1 vulnerability in 1 package (checked 1)

--latest: rewrites a range for a downgrade that stays in the installed major
  fixing:
    v no-deps 1.1.0 -> 1.0.1 (downgrade)
      package.json: ^1.1.0 -> ^1.0.1
  vulnerable after install:
    no-deps@1.1.0  1
  Fixed 0 vulnerabilities in 1 package (checked 1)
  1 vulnerability remaining

A downgrade candidate must share the installed version's major (minor
below 1.0.0). When an advisory covers every release of the installed
line, the package is reported under "no published version fixes" instead
of being moved to an older major, with or without --latest.

A downgrade that rewrites a declared range also pins the lockfile edge,
since the rewritten range would otherwise re-resolve to the newest
release it allows, which is the vulnerable one.

Fixes #39309
@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0b340954-7baa-4984-857d-32f71cc51c31

📥 Commits

Reviewing files that changed from the base of the PR and between 23d535a and ee5b57f.

📒 Files selected for processing (9)
  • docs/pm/cli/audit.mdx
  • src/install/audit_fix.rs
  • test/cli/install/bun-audit.test.ts
  • test/cli/install/registry/packages/create-zero-major-packages.ts
  • test/cli/install/registry/packages/zero-major/package.json
  • test/cli/install/registry/packages/zero-major/zero-major-0.4.0.tgz
  • test/cli/install/registry/packages/zero-major/zero-major-0.4.1.tgz
  • test/cli/install/registry/packages/zero-major/zero-major-0.5.0.tgz
  • test/cli/install/registry/packages/zero-major/zero-major-0.5.1.tgz

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

Status: reproduced and fixed, waiting on CI.

Reproduced on a build of main with the test registry in test/cli/install/bun-audit.test.ts: no-deps@^2.0.0 installed with an advisory covering >=2.0.0 gets v no-deps 2.0.0 -> 1.1.0 (downgrade) and package.json: ^2.0.0 -> ^1.1.0 under --latest, and plain bun audit fix performs the same downgrade through a dependent's * range. Both now end in "no published version fixes" (see the PR description for the unfixed outputs of every new test).

The CodeRabbit comment above is only its rate-limit notice; there is nothing in it to act on.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because it changes the user-facing policy for bun audit fix downgrades — including the 0.x-minor-as-release-line rule and the new lockfile pin for rewritten downgrade edges — a human look to confirm the product decision would still be worthwhile.

What was reviewed:

  • The break in the reversed downgrade loop is sound — pkg.releases is sorted at serialization time (npm.rs), so once a version falls outside the installed line every remaining entry is older.
  • same_release_line correctly treats 0.x minors as separate lines (matching caret semantics); same-line downgrades and the existing downgrade tests are preserved.
  • The edge.peer || candidate.downgrade pin only affects the t.edit (rewritten-range) branch, so upgrades through a rewrite still resolve upward as before; the new --latest and catalog tests plus frozenLockfile re-install cover it.
  • Two existing tests were re-inputted (not weakened) — their old cross-major scenario is now the new "older major" test.
Extended reasoning...

Overview

The PR fixes #39309: bun audit fix (and --latest) could downgrade a package into an older major (e.g. aws-sdk 2.x → 1.18.0 from 2013) when every release in the installed major is advised. It touches src/install/audit_fix.rs (adds same_release_line and gates the reversed downgrade-candidate loop on it; also pins rewritten-range edges when the target is a downgrade so ^1.0.1 doesn't re-resolve to 1.1.0), docs/pm/cli/audit.mdx (documents the floor), test/cli/install/bun-audit.test.ts (7 new tests, 2 existing tests re-inputted to a same-major scenario), and adds a zero-major registry fixture with two 0.x minor lines.

Security risks

None identified. This is a pure planner-side change to how candidate versions are filtered and how a lockfile row is pinned; no parsing of untrusted input, no new syscalls, no auth/crypto surface. If anything it's a safety improvement — it stops installing very old releases whose advisories weren't queried.

Level of scrutiny

Medium. The Rust change is small (~15 lines) and localized to plan_fixes, but it encodes a user-facing product policy: downgrades never cross the installed major (or minor, below 1.0.0), even under --latest. That policy is well-justified — an older major is a different API, and the bulk advisory endpoint only returns advisories for the versions sent, so older-major releases merely look safe — and matches the filed issue. The 0.x-minor rule is a reasonable extension following semver caret semantics, but it's a design choice beyond the literal report that a maintainer should confirm.

I verified the break (rather than continue) is correct: pkg.releases.keys is sorted at serialization time per src/install/npm.rs ("This list is sorted at serialization time"), and the loop iterates .rev(), so once a version below the current one falls outside the release line, all remaining entries are older still. Prereleases live in a separate list and aren't walked here.

Other factors

Test coverage is thorough: both sides of each boundary (0.x downgrades within its minor / not into an older minor; --latest rewrites for a same-major downgrade / never into an older major; wide dependent range doesn't admit a cross-major downgrade), text and --json output, catalog variant, and frozenLockfile re-install after the pin. The two edited existing tests ("blocked on the highest safe downgrade" text and --json) still assert the same property with a same-major input, and their previous cross-major input is preserved as the new "older major is not a candidate" test — no coverage was lost. The PR description shows the new tests failing on main. No CODEOWNERS entry covers src/install/. Given the behavior/policy nature of the change, deferring rather than auto-approving.

@robobun

robobun commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

The review above has no findings; the two product decisions it points at are the ones to confirm when reviewing:

  1. Floor for downgrades: same major, and below 1.0.0 the same minor (same_release_line). The alternative of refusing every downgrade, which is what the issue asks for literally, would also drop the rollback case the existing tests cover ("downgrades when no newer release is safe", "downgrades a transitive dependency within its dependent's range"), so this PR keeps same-line downgrades. Both sides of the 0.x rule are covered by "a 0.x package downgrades within its minor line" and "a 0.x package is not downgraded into an older minor line".
  2. Pinning the lockfile row when --latest rewrites a range for a downgrade. Without it the rewrite is a no-op (^1.0.1 resolves to 1.1.0 again), so after the floor every remaining --latest downgrade would be one. Covered by the two "downgrade that stays in the installed major" cases, including the frozen-lockfile reinstall afterwards.

Dropping either decision is a small change to the same function if preferred.

@robobun

robobun commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 11:47 PM PT - Aug 15th, 2026

✅ @robobun, your commit ee5b57f4aedab91c1442cf0726a7b50d10145f6f passed in Build #99220! 🎉


🧪   To try this PR locally:

bunx bun-pr 39317

That installs a local version of the PR into your bun-39317 executable, so you can run:

bun-39317 --bun

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants