Skip to content

install: bump default lockfileVersion to 2, gate stricter parse checks behind it #31539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 10 commits into from
May 29, 2026
Merged

install: bump default lockfileVersion to 2, gate stricter parse checks behind it #31539

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
946fb2c
install: bump default lockfileVersion to 2, gate stricter parse check…
robobun May 28, 2026
cffec9e
[autofix.ci] apply automated fixes
autofix-ci[bot] May 28, 2026
0b22aca
test: address review feedback on lockfile v2 tests
robobun May 28, 2026
58bc805
install: only stamp lockfileVersion 2 when the lockfile is v2-valid; …
robobun May 29, 2026
7299573
types/docs: account for lockfileVersion 2
robobun May 29, 2026
11a6d6c
[autofix.ci] apply automated fixes
autofix-ci[bot] May 29, 2026
bb65252
install: scope version_to_write git-tag gating to git only
robobun May 29, 2026
70dbf31
test: correct the save-trigger comment in the v1 round-trip test
robobun May 29, 2026
4509e0a
install: version_to_write must only consider serialized packages
robobun May 29, 2026
272af67
test: fix stale file-header comment in lockfile-version-2 test
robobun May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions src/install/lockfile/bun.lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,18 @@

/// fixed unnecessary listing of workspace dependencies
V1 = 1,

/// Stricter parsing that rejects, rather than accepts, lockfiles the
/// earlier versions tolerated. Gated here so an already-written v0/v1
/// lockfile keeps loading:
/// - an npm package resolved to a tarball URL outside the configured
/// registry must carry a supported integrity hash
/// - a git/github `.bun-tag` must be a safe path/checkout component
V2 = 2,
}

impl Version {
pub const CURRENT: Version = Version::V1;
pub const CURRENT: Version = Version::V2;

Check failure on line 122 in src/install/lockfile/bun.lock.rs

View check run for this annotation

Claude / Claude Code Review

v1→v2 auto-upgrade produces a v2 lockfile that fails on next install 

The v1 backward-compat only survives until the lockfile is re-saved: the writer always emits `Version::CURRENT` (now V2) at bun.lock.rs:263 and writes `pkg_meta.integrity` / the git `.bun-tag` verbatim with no backfill, so a v1 lockfile carrying an off-registry tarball with `""` integrity (or an unsafe `.bun-tag`) gets rewritten as `"lockfileVersion": 2` with the same empty integrity, and the *next* `bun install` rejects it with the very error this gate is meant to suppress. Any save trigger — `
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.

#[inline]
pub const fn current() -> Version {
Expand All @@ -122,9 +130,17 @@
match n {
0 => Some(Version::V0),
1 => Some(Version::V1),
2 => Some(Version::V2),
_ => None,
}
}

/// `true` when this lockfile version is at least `other`. Used to gate
/// strict parse-time checks introduced in a later version.
#[inline]
pub const fn at_least(self, other: Version) -> bool {
(self as u32) >= (other as u32)
}
}

/// For sorting dependencies belonging to a node_modules folder. No duplicate names, so
Expand Down Expand Up @@ -2546,7 +2562,14 @@
// Fail closed: otherwise a tampered lockfile could redirect
// the tarball URL off-registry and install arbitrary content
// under a trusted package name with verification disabled.
if npm_url_needs_integrity && !pkg.meta.integrity.tag.is_supported() {
//
// Only enforced for v2+. Older lockfiles predate this check
// and may legitimately omit integrity for an off-registry
// tarball; rejecting them would break existing installs.
if lockfile_version.at_least(Version::V2)
&& npm_url_needs_integrity
&& !pkg.meta.integrity.tag.is_supported()
{
log.add_error(
Some(source),
integrity_expr.loc,
Expand Down Expand Up @@ -2587,10 +2610,17 @@
return Err(ParseError::InvalidPackageInfo);
};

if !crate::repository::is_safe_resolved_tag(bun_tag_str) {
// Only reject an unsafe `.bun-tag` at parse time for v2+.
// Older lockfiles predate this check; `Repository::checkout`
// re-validates with the same guard before building any cache
// path or invoking `git`, so skipping it here keeps older
// lockfiles loading without reopening the checkout hole.
if lockfile_version.at_least(Version::V2)
&& !crate::repository::is_safe_resolved_tag(bun_tag_str)
{
log.add_error(Some(source), bun_tag.loc, b"Invalid git dependency tag");
return Err(ParseError::InvalidPackageInfo);
}

Check failure on line 2623 in src/install/lockfile/bun.lock.rs

View check run for this annotation

Claude / Claude Code Review

Security justification for skipping is_safe_resolved_tag does not cover the github branch 

This match arm covers **both** `ResolutionTag::Git | ResolutionTag::Github` (line 2598), but the comment's justification — "`Repository::checkout` re-validates with the same guard before building any cache path" — only holds for the **git** branch. GitHub dependencies go through the tarball-download path (never `Repository::checkout`) and feed the lockfile's `resolved` directly into `cached_github_folder_name_print` → `@GH@{resolved}…` with no `is_safe_resolved_tag` re-validation; `@G@{resolved}
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

let resolved = sbuf!(lockfile).append(bun_tag_str)?;
if tag == ResolutionTag::Git {
Expand Down
14 changes: 7 additions & 7 deletions test/cli/install/__snapshots__/bun-install-registry.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`auto-install symlinks (and junctions) are created correctly in the inst

exports[`text lockfile workspace sorting 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -45,7 +45,7 @@ exports[`text lockfile workspace sorting 1`] = `

exports[`text lockfile workspace sorting 2`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -88,7 +88,7 @@ exports[`text lockfile workspace sorting 2`] = `

exports[`text lockfile --frozen-lockfile 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -120,7 +120,7 @@ exports[`text lockfile --frozen-lockfile 1`] = `

exports[`binaries each type of binary serializes correctly to text lockfile 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -148,7 +148,7 @@ exports[`binaries each type of binary serializes correctly to text lockfile 1`]

exports[`binaries root resolution bins 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 0,
"workspaces": {
"": {
Expand All @@ -170,7 +170,7 @@ exports[`binaries root resolution bins 1`] = `

exports[`hoisting text lockfile is hoisted 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -280,7 +280,7 @@ exports[`outdated NO_COLOR works 1`] = `

exports[`it should ignore peerDependencies within workspaces 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down
20 changes: 10 additions & 10 deletions test/cli/install/__snapshots__/bun-lock.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`should write plaintext lockfiles 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand All @@ -21,7 +21,7 @@ exports[`should write plaintext lockfiles 1`] = `

exports[`should escape names 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand All @@ -48,7 +48,7 @@ exports[`should escape names 1`] = `

exports[`should be the default save format 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand All @@ -67,7 +67,7 @@ exports[`should be the default save format 1`] = `

exports[`should be the default save format 2`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand All @@ -89,7 +89,7 @@ exports[`should be the default save format 2`] = `

exports[`should save the lockfile if --save-text-lockfile and --frozen-lockfile are used 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand All @@ -108,7 +108,7 @@ exports[`should save the lockfile if --save-text-lockfile and --frozen-lockfile

exports[`should save the lockfile if --save-text-lockfile and --frozen-lockfile are used 2`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand All @@ -130,7 +130,7 @@ exports[`should save the lockfile if --save-text-lockfile and --frozen-lockfile

exports[`should convert a binary lockfile with invalid optional peers 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 0,
"workspaces": {
"": {
Expand Down Expand Up @@ -300,7 +300,7 @@ exports[`should not deduplicate bundled packages with un-bundled packages 2`] =

exports[`should not change formatting unexpectedly 2`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -400,7 +400,7 @@ exports[`should not change formatting unexpectedly 2`] = `

exports[`should sort overrides before comparing 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -438,7 +438,7 @@ exports[`should sort overrides before comparing 1`] = `

exports[`should include unused resolutions in the lockfile 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down
6 changes: 3 additions & 3 deletions test/cli/install/__snapshots__/catalogs.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`basic detect changes (bun.lock) 1`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -37,7 +37,7 @@ exports[`basic detect changes (bun.lock) 1`] = `

exports[`basic detect changes (bun.lock) 2`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -72,7 +72,7 @@ exports[`basic detect changes (bun.lock) 2`] = `

exports[`basic detect changes (bun.lock) 3`] = `
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down
2 changes: 1 addition & 1 deletion test/cli/install/bun-install.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9013,7 +9013,7 @@ describe.concurrent("bun-install", () => {
expect(await exists(join(ctx.package_dir, "bun.lockb"))).toBeFalse();
expect(await file(join(ctx.package_dir, "bun.lock")).text()).toMatchInlineSnapshot(`
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down
48 changes: 12 additions & 36 deletions test/cli/install/bun-lock.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,12 +616,11 @@ it("should include unused resolutions in the lockfile", async () => {
await runBunInstall(env, packageDir, { frozenLockfile: true });
});

it("requires an integrity hash when an npm package entry points at a tarball URL outside the configured registry", async () => {
it("requires an integrity hash for an off-registry npm tarball URL at lockfileVersion 2", async () => {
const { packageDir, packageJson } = await registry.createTestDir();

// Stand-in for a host that is not the configured registry. A correct install
// must never contact it for this lockfile, because the entry carries no
// integrity hash that would pin the tarball contents.
// Stand-in for a host that is not the configured registry. Parsing fails
// before any fetch, so this is never actually contacted.
let offRegistryRequests = 0;
await using offRegistry = Bun.serve({
port: 0,
Expand All @@ -642,9 +641,14 @@ it("requires an integrity hash when an npm package entry points at a tarball URL
}),
);

const lockfileWithUrl = (tarballUrl: string) =>
// The entry keeps the well-known name and version but points the tarball at a
// different host and provides no integrity hash. At lockfileVersion 2 this
// fails closed: parsing rejects it before any fetch. (The v1 backward-compat
// case — parsing accepts such an entry — is covered in lockfile-version-2.test.ts.)
Comment thread
robobun marked this conversation as resolved.
Outdated
await write(
join(packageDir, "bun.lock"),
JSON.stringify({
lockfileVersion: 1,
lockfileVersion: 2,
configVersion: 1,
workspaces: {
"": {
Expand All @@ -655,15 +659,9 @@ it("requires an integrity hash when an npm package entry points at a tarball URL
},
},
packages: {
"no-deps": ["no-deps@1.0.0", tarballUrl, {}, ""],
"no-deps": ["no-deps@1.0.0", `http://127.0.0.1:${offRegistry.port}/no-deps/-/no-deps-1.0.0.tgz`, {}, ""],
},
});

// The entry keeps the well-known name and version but points the tarball at
// a different host and provides no integrity hash.
await write(
join(packageDir, "bun.lock"),
lockfileWithUrl(`http://127.0.0.1:${offRegistry.port}/no-deps/-/no-deps-1.0.0.tgz`),
}),
);

let { exited, stdout, stderr } = spawn({
Expand All @@ -681,28 +679,6 @@ it("requires an integrity hash when an npm package entry points at a tarball URL
expect(offRegistryRequests).toBe(0);
expect(await exists(join(packageDir, "node_modules", "no-deps"))).toBe(false);
expect(await exited).not.toBe(0);

// The same entry with the tarball URL under the configured registry and no
// integrity hash is still accepted (backward compat for lockfiles that omit
// the hash for registry-hosted tarballs).
await write(join(packageDir, "bun.lock"), lockfileWithUrl(`${registry.registryUrl()}no-deps/-/no-deps-1.0.0.tgz`));

({ exited, stdout, stderr } = spawn({
cmd: [bunExe(), "install"],
cwd: packageDir,
env,
stdout: "pipe",
stderr: "pipe",
}));

[out, err] = await Promise.all([stdout.text(), stderr.text()]);
expect(err).not.toContain("Missing integrity hash");
expect(offRegistryRequests).toBe(0);
expect(await exited).toBe(0);
expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toMatchObject({
name: "no-deps",
version: "1.0.0",
});
});

it("escapes double quotes in npm registry tarball URLs when saving bun.lock", async () => {
Expand Down
2 changes: 1 addition & 1 deletion test/cli/install/bun-workspaces.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1981,7 +1981,7 @@ test("matching workspace devDependency and npm peerDependency", async () => {
expect((await file(join(packageDir, "bun.lock")).text()).replaceAll(/localhost:\d+/g, "localhost:1234"))
.toMatchInlineSnapshot(`
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down
6 changes: 3 additions & 3 deletions test/cli/install/config-version.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe("configVersion", () => {
).replaceAll(/localhost:\d+/g, "localhost:1234");
expect(lockfile).toMatchInlineSnapshot(`
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -89,7 +89,7 @@ describe("configVersion", () => {
).replaceAll(/localhost:\d+/g, "localhost:1234");
expect(lockfile).toMatchInlineSnapshot(`
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 1,
"workspaces": {
"": {
Expand Down Expand Up @@ -160,7 +160,7 @@ describe("configVersion", () => {
).replaceAll(/localhost:\d+/g, "localhost:1234");
expect(lockfile).toMatchInlineSnapshot(`
"{
"lockfileVersion": 1,
"lockfileVersion": 2,
"configVersion": 0,
"workspaces": {
"": {
Expand Down
Loading
Loading