Skip to content

fix(hosted): re-redirect stale bun.lock URLs, fail closed on drift - #186

Merged
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
fix/bun-hosted-reredirect-warnings
Aug 14, 2026
Merged

fix(hosted): re-redirect stale bun.lock URLs, fail closed on drift#186
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
fix/bun-hosted-reredirect-warnings

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Problem

Four audit findings (2026-08-13 hosted-mode audit, against 4e5288e) in the bun hosted-redirect path:

  1. HIGH — re-redirect after artifact URL change silently leaves a stale hosted URL in bun.lock. The first redirect replaces the registry name@version tuple spec with name@<artifactUrl>, destroying the version key. A later run only recognized its own entry by exact equality with the CURRENT URL, so when the artifact URL changes — a patch republish rotates the uuid path segment, or a grant-token rotation changes the token — the stale pin was classified "unowned" and left forever: the new patch never landed, the dep silently dropped out of redirected/VEX, and the lock kept installing the old artifact (or 403ing once the old token died), with no warning and no re-run able to fix it. Every other npm-family rewriter re-targets fine because its match key survives the first rewrite; bun alone could not.
  2. MEDIUM — no entry-not-found warning. pnpm/berry/uv rewriters warn when a granted dep matches no rewritable entry; the bun rewriter had no matched_any tracking, so version drift or the stale-URL case above produced redirected: 0, warnings: [] — nothing greppable in CI.
  3. LOW — non-canonical "packages" header failed OPEN. A bun.lock whose packages header was re-indented (tab, 4-space) or spelled "packages" : { parsed as an EMPTY lock instead of refusing, so the hosted rewriter silently skipped a lock bun itself parses fine — violating the module's fail-closed contract.
  4. LOW — bun.lockb irreversibly deleted by a zero-redirect run. The lockb→text auto-migration was gated only on an npm override EXISTING, not on the rewrite landing; an override whose version doesn't match the lock still converted the user's lockfile format permanently (removal recorded with original: None — git history the only restore).

Fix

Each at the boundary that broke:

  • rewrite_bun_lock gains a third ownership branch: a URL 3-tuple whose spec URL shares both the origin and the trailing <name>-<version>.tgz path leaf with the CURRENT override's artifact URL is recognized as our own earlier redirect and re-pinned to the new URL + sha512. Both halves are derived from the live override — nothing about the patch server's URL layout is hard-coded — and anything that fails to parse fails the match (closed): foreign-origin user URL deps and other-version leaves are never claimed, and a same-URL rerun stays a byte-identical no-op.
  • Per-dep matched_any tracking now emits redirect_bun_entry_not_found (naming the dep) when a granted dep matches no rewritable tuple, mirroring the pnpm/berry/uv rewriters. The sha512 refusal path does not double-warn.
  • parse_packages_section (shared with vendor mode) returns Err for any "packages" header not in bun's byte-exact emitted shape, surfacing the existing redirect_bun_lock_unsupported / lockfile-unsupported refusals in both modes. A truly absent packages object and a dependency literally named "packages" in another section still parse as empty.
  • The CLI reads the bun.lockb bytes before invoking bun's migration; when the subsequent rewrite lands nothing in the migrated bun.lock, the migration is undone — lockb restored byte-identical, generated text lock removed, ledger removal record dropped — with a redirect_bun_lockb_migration_reverted warning (and a loud redirect_bun_lockb_migrated_without_redirect fallback if the restore itself fails).

No golden fixture was added for the re-redirect case: the redirect goldens are the cross-language contract shared with the TS backend rewriter, so the new behavior is pinned by unit tests instead.

No findings were skipped.

Testing

New regression tests, each verified to fail against the pre-fix code:

  • patch::redirect::tests::bun_lock_re_redirects_stale_hosted_url — stale URL re-pinned to the new URL + sha512; same-URL rerun no-op; foreign-origin and other-version-leaf tuples untouched (with entry-not-found warning).
  • patch::redirect::tests::bun_lock_entry_not_found_warns — version-drifted lock warns and names the dep; a successful rewrite emits no warning.
  • patch::redirect::tests::bun_lock_noncanonical_packages_header_fails_closed and vendor::bun_lock_text::tests::noncanonical_packages_header_is_an_error_not_empty — tab/4-space/space-colon headers refuse; absent section and a dep named "packages" still parse empty; unterminated still errors.
  • in_process_redirect::zero_redirect_restores_bun_lockb_after_migration — migrated lock at a non-matching version → bun.lockb restored byte-identical, bun.lock removed, no ledger written.
  • Extended bun_lock_warning_branches — the sha512 refusal must not also warn entry-not-found.

Suites run green (implementation + independent review passes): cargo test -p socket-patch-core --lib (2079), --test redirect_golden (incl. rerun-noop determinism), cargo test -p socket-patch-cli --lib (367), --test in_process_redirect (24), --test in_process_vendor (27), --test e2e_redirect_bun_build (7, real bun), --test e2e_vendor_bun_build (6, covers the shared parser change in vendor mode).

🤖 Generated with Claude Code


Note

Medium Risk
Changes hosted lockfile rewrite and destructive bun.lockb migration behavior; incorrect matching could leave wrong URLs or convert lock formats, but behavior is heavily regression-tested.

Overview
Bun lock rewriter now treats existing URL 3-tuples as owned when the spec shares origin and <name>-<version>.tgz with the current artifact URL, so republished patches or rotated tokens get re-pinned instead of staying stale. Granted deps with no matching tuple emit redirect_bun_entry_not_found (aligned with other npm-family rewriters).

Shared bun.lock parsing rejects non-canonical "packages" headers (tabs, extra spaces, etc.) instead of treating them as empty locks, surfacing redirect_bun_lock_unsupported in redirect and vendor paths.

Hosted scan backs up bun.lockb before migration; if the rewrite does not change bun.lock, it restores the binary lock, removes the generated text lock, and clears migration ledger edits, with JSON warnings when restore fails.

Reviewed by Cursor Bugbot for commit 1d203fe. Configure here.

The first bun redirect replaces the registry `name@version` spec with
`name@<artifactUrl>`, so a later run could only recognize its own entry
by exact URL equality. When the artifact URL changes — a patch republish
rotates the uuid path segment, a token rotation changes the token — the
stale pin was classified "unowned" and left in place forever, silently:
the new patch never landed, the dep dropped out of the redirected count
and VEX, and the lock kept installing the old artifact (or 403ing once
the old token died) with no warning and no re-run able to fix it.

Four fixes, each at the boundary that broke:

- rewrite_bun_lock now re-pins a URL 3-tuple written by an earlier
  redirect whose artifact URL has since changed. Ownership is claimed
  narrowly from the live override — same origin and the same
  `<name>-<version>.tgz` path leaf as the CURRENT artifact URL — so
  user URL deps and other-version artifacts never match (fail-closed),
  and a same-URL rerun stays a no-op.

- A granted dep that matches no rewritable tuple now warns
  `redirect_bun_entry_not_found`, mirroring the pnpm/berry/uv
  rewriters, instead of vanishing from the redirected count with an
  empty warnings array.

- parse_packages_section fails CLOSED on a `"packages"` header spelled
  any way other than bun's byte-exact emitted shape (tab/4-space
  re-indent, `"packages" : {`). Treating those locks as empty made the
  hosted rewriter silently skip files bun itself parses fine; they now
  surface the unsupported-shape refusal in both hosted and vendor
  modes.

- The bun.lockb→bun.lock auto-migration is undone when the subsequent
  rewrite lands nothing in the migrated lock: the pre-migration lockb
  bytes are restored, the generated text lock is removed, and no ledger
  removal is recorded — a zero-redirect scan no longer permanently
  converts the user's lockfile format as a side effect.

Regression tests pin all four (unit tests in redirect/mod.rs and
bun_lock_text.rs; an in-process CLI test for the migration restore);
each was verified red against the pre-fix code.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 3e0afed into main Aug 14, 2026
231 of 234 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the fix/bun-hosted-reredirect-warnings branch August 14, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants