install: reject bun.lockb packages whose resolution tag no writer produces

Closed
RO
1mo ago
+168-33

Problem

  • A bun.lockb package whose resolution tag byte is 0 (Uninitialized) or 100 (SingleFileModule) loads. bun install prints Saved lockfile, exits 0, installs nothing for it, and writes it back out. A debug build panics instead: unreachable!() in ResolutionType::eql (src/install/resolution.rs:525). Install fuzzer ledger entry 16544.
  • Cause: the tag check in load_fields (src/install/lockfile/Package.rs) accepted every tag constant. Dependencies on such a package keep a valid package id, so verify_resolutions counts them as resolved. No installer has a path for either tag.

Fix

  • Tag::belongs_in_lockfile lists the nine tags a writer produces. The bun.lockb loader rejects every other byte with invalid resolution tag, as it did for unknown bytes: bun install prints Ignoring lockfile and resolves again, bun bun.lockb exits 1. The bun.lock writer, which skipped the same two tags with its own list, uses the helper too.
  • Correct because a package is appended only once it is resolved, and SingleFileModule is a placeholder with no producer. The one way to get such a file today is the yarn.lock migration bug that the open #39001 fixes, with saveTextLockfile = false. That file misses a dependency on every install. Now the next install resolves it again from package.json.
  • Verified: test/cli/install/bun-lockb.test.ts, four new cases (both tags, format v3 through the registry and format v2 through the checked in fixture). All four fail without the src change.

Background

  • bun.lockb stores packages as columns. Each resolution entry is a tag byte plus a union that says where the package comes from (npm, folder, git, workspace, ...). Tag 0 is the state of a package that is not resolved yet.
  • load_fields copies each column from the file and checks the enum bytes first, because the rest of the loader matches on them.
  • On a load error bun install warns Ignoring lockfile and resolves from package.json. --frozen-lockfile fails with lockfile had changes.
Notes

Debug build panic path for tag 0: ResolutionType::eql <- Lockfile::get_package_id <- the debug_assert! in Lockfile::append_package_with_id <- Package::clone <- Lockfile::clean_with_logger. The release build skips the assertion and carries the package through. Tag 100 has an eql arm and reaches the _ arm of PackageInstaller::install_package_with_name_and_resolution instead, which panics in debug builds and counts the package as installed in release builds. isolated_install treats the two tags as one class as well.

The format v2 migration arms for the two tags return the same error as the loader. History of this PR: the first version rejected tag 0 only. The self-review found that tag 100 reproduces the report one byte away. The comment that explained the loader's list was then flagged as too long, so the list moved next to the tag constants, the writer shares it, and the loader reads the tag offset with offset_of! like the Meta, Bin and Scripts checks next to it.

Before this change bun install --frozen-lockfile already refused the mutated file (lockfile had changes). It accepted the file that the plain install saved afterwards, so the broken state was sticky. The new tests check that the saved file has no such tag left.

Suites run with the debug build: bun-lockb, bun-lock, bun-install, migrate-bun-lockb-v2, bun-workspaces, bun-link, bun-add, bun-update, bun-remove, catalogs, migration/migrate, and the port-era-markers, pre-port-identifiers and byte-search source lints. bun-install.test.ts: 224 pass, 14 fail. The failures are the bitbucket, gitlab and external tarball URL tests plus --registry CLI flag. All of them fail the same way on the released build in this environment (no network). bun-link.test.ts has one failure, should link dependency without crashing. It expects exact stdout and gets the debug only debug_trace of an install failure. Both are unrelated to this change.

The fuzz report had a second part: a bun.lock whose "packages" key names one package (xbin) while the tuple id names another (p13@1.1.0) installs p13 at node_modules/xbin with exit 0. This PR does not change that, on purpose:

  • The key is the install path and the tuple is the package that lives there. They differ in every lockfile with an npm: alias ("my-alias": ["no-deps@1.0.0", ...], checked with the released build) and with overrides. A check on the key name rejects those lockfiles.
  • A check of the dependency's real name against the package name is not sound either. enqueue_dependency_with_main_and_success_fn redirects a plain dependency to a same named npm: alias declared elsewhere in the tree (known_npm_aliases). When the alias is removed later, the redirected edge survives clean_with_logger. So bun itself writes a lockfile in which a plain foo edge points at a package named bar, with no alias or override left to explain it. --frozen-lockfile would then fail on a lockfile bun wrote.
  • The version is not checked today either: a bun.lock that pins no-deps@2.0.0 under a package.json that says 1.0.0 installs 2.0.0 under --frozen-lockfile (checked with the released build). Locked edges are trusted as a whole. Validating them against package.json is a feature in the area of #33632, not a loader bug.

The report's third point, never exit 0 with a root dependency bound to a package that cannot be installed, has two producers: this loader, fixed here, and the yarn.lock migration, fixed by the open #39001. A release mode check at the append_package* entry points would also cover a future producer. It is left out here because both known producers are covered.

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-lockb.test.ts