diff --git a/src/install/lockfile/Package.rs b/src/install/lockfile/Package.rs index c3f774d9d18c..8b6e24c0521b 100644 --- a/src/install/lockfile/Package.rs +++ b/src/install/lockfile/Package.rs @@ -3253,9 +3253,6 @@ pub mod serializer { resolutions: old.resolutions, scripts: old.scripts, resolution: match old.resolution.tag { - ResolutionTag::Uninitialized => { - Resolution::init(TaggedValue::Uninitialized) - } ResolutionTag::Root => Resolution::init(TaggedValue::Root), ResolutionTag::Npm => { Resolution::init(TaggedValue::Npm(old.resolution.npm().migrate())) @@ -3281,10 +3278,10 @@ pub mod serializer { ResolutionTag::RemoteTarball => Resolution::init( TaggedValue::RemoteTarball(*old.resolution.remote_tarball()), ), - ResolutionTag::SingleFileModule => Resolution::init( - TaggedValue::SingleFileModule(*old.resolution.single_file_module()), - ), - _ => Resolution::init(TaggedValue::Uninitialized), + // `load_fields` already rejected every other tag byte. + _ => { + return Err(crate::Error::LockfileValidationFailedInvalidResolutionTag); + } }, }; @@ -3325,20 +3322,11 @@ pub mod serializer { if end_pos as u64 <= end_at { let src = &stream.buffer[stream.pos..stream.pos + bytes.len()]; if matches!(field, PackageField::Resolution) { - // Validate the tag discriminant on the *raw stream bytes* - // before they are copied into the typed column. `ResolutionTag` - // is a `#[repr(u8)]` enum with non-contiguous discriminants - // (0,1,2,4,8,16,32,64,72,80,100); copying an out-of-range byte - // into `ResolutionType.tag` and then reading it would be - // immediate UB, and a `matches!` over all 11 typed variants is - // provably exhaustive and would be optimized away. Check the - // raw u8 here. Layout: `ResolutionType` is `#[repr(C)] - // { tag: Tag, _padding: [u8; 7], value: ... }`, so the - // discriminant is the first byte of each element. let stride = mem::size_of::>(); + let tag_at = mem::offset_of!(ResolutionType, tag); debug_assert!(stride != 0 && src.len().is_multiple_of(stride)); for raw in src.chunks_exact(stride) { - if !matches!(raw[0], 0 | 1 | 2 | 4 | 8 | 16 | 32 | 64 | 72 | 80 | 100) { + if !ResolutionTag(raw[tag_at]).belongs_in_lockfile() { return Err(crate::Error::LockfileValidationFailedInvalidResolutionTag); } } diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index 87ec99aa581f..97ae6c366d49 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -694,20 +694,8 @@ impl Stringifier { } let res = &pkg_resolutions[pkg_id as usize]; - match res.tag { - ResolutionTag::Root - | ResolutionTag::Npm - | ResolutionTag::Folder - | ResolutionTag::LocalTarball - | ResolutionTag::Github - | ResolutionTag::Git - | ResolutionTag::Symlink - | ResolutionTag::Workspace - | ResolutionTag::RemoteTarball => {} - ResolutionTag::Uninitialized => continue, - // should not be possible, just being safe - ResolutionTag::SingleFileModule => continue, - _ => continue, + if !res.tag.belongs_in_lockfile() { + continue; } if first { diff --git a/src/install/resolution.rs b/src/install/resolution.rs index d1e6fa7424e4..c23ccde5ce22 100644 --- a/src/install/resolution.rs +++ b/src/install/resolution.rs @@ -1014,6 +1014,23 @@ impl Tag { self == Tag::Git || self == Tag::Github } + /// Tags a saved package can have. `Uninitialized` is the state before a + /// package is appended and `SingleFileModule` has no producer. + pub(crate) fn belongs_in_lockfile(self) -> bool { + matches!( + self, + Tag::Root + | Tag::Npm + | Tag::Folder + | Tag::LocalTarball + | Tag::Github + | Tag::Git + | Tag::Symlink + | Tag::Workspace + | Tag::RemoteTarball + ) + } + pub(crate) fn can_enqueue_install_task(self) -> bool { self == Tag::Npm || self == Tag::LocalTarball diff --git a/test/cli/install/bun-lockb.test.ts b/test/cli/install/bun-lockb.test.ts index 71bfb293ab51..e9fb1ec4ff34 100644 --- a/test/cli/install/bun-lockb.test.ts +++ b/test/cli/install/bun-lockb.test.ts @@ -1,7 +1,7 @@ import { file, spawn, write } from "bun"; import { afterAll, beforeAll, expect, it } from "bun:test"; import { copyFile, exists, open, rm, writeFile } from "fs/promises"; -import { bunExe, bunEnv as env, isWindows, runBunInstall, VerdaccioRegistry } from "harness"; +import { bunExe, bunEnv as env, isWindows, runBunInstall, tempDir, VerdaccioRegistry } from "harness"; import { join } from "path"; const registry = new VerdaccioRegistry(); @@ -349,6 +349,148 @@ it("rejects a binary lockfile whose package scripts flag byte is out of range", expect(code).toBe(0); expect(await exists(join(packageDir, "node_modules", "no-deps"))).toBe(true); }); + +// The `resolution` column follows the name (8) and name_hash (8) columns. Each +// entry is a 72-byte (64 in format v2) `Resolution` whose first byte is the tag: +// 1 = root, 2 = npm, 72 = workspace, ... +function packageResolutionTagOffsets(lockb: Buffer): number[] { + const fmt = lockb.readUInt32LE(42); + const N = Number(lockb.readBigUInt64LE(86)); + const begin = Number(lockb.readBigUInt64LE(110)); + const resolutionSize = fmt === 2 ? 64 : 72; + const resolutionStart = begin + N * (8 + 8); + const offsets: number[] = []; + for (let i = 0; i < N; i++) { + offsets.push(resolutionStart + i * resolutionSize); + } + return offsets; +} + +// Nothing in bun writes a package with one of these tags: 0 (uninitialized) is +// the in-memory state of a package that is not resolved yet, and 100 (single +// file module) is a placeholder. Nothing installs such a package either, so a +// lockfile that contains one has to be rejected. +const resolutionTagsWithoutAWriter = [ + { tag: 0, name: "uninitialized" }, + { tag: 100, name: "single file module" }, +]; + +it.each(resolutionTagsWithoutAWriter)( + "rejects a binary lockfile in which a package has the $name resolution tag", + async ({ tag }) => { + const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } }); + + await write( + packageJson, + JSON.stringify({ + name: "lockb-resolution-without-a-writer", + version: "1.0.0", + dependencies: { + "no-deps": "1.0.0", + "a-dep": "1.0.1", + }, + }), + ); + + await runBunInstall(env, packageDir); + const lockbPath = join(packageDir, "bun.lockb"); + expect(await exists(lockbPath)).toBe(true); + + const lockb = Buffer.from(await file(lockbPath).arrayBuffer()); + const offsets = packageResolutionTagOffsets(lockb); + expect(offsets.map(offset => lockb[offset])).toEqual([1, 2, 2]); + // One of the root's dependencies still resolves to package 1, but the + // package no longer says where it comes from. + lockb[offsets[1]] = tag; + await write(lockbPath, lockb); + + { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "bun.lockb"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + expect(err).toContain("invalid resolution tag"); + expect(out).toBe(""); + expect(code).toBe(1); + } + + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install", "--no-progress"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + + // The lockfile is rejected and the install resolves every dependency again + // instead of silently skipping the one bound to the broken package. + expect(err).toContain("invalid resolution tag"); + expect(err).toContain("Ignoring lockfile"); + expect(out).toContain("no-deps@1.0.0"); + expect(out).toContain("a-dep@1.0.1"); + expect(out).toContain("2 packages installed"); + expect(code).toBe(0); + expect(await exists(join(packageDir, "node_modules", "no-deps"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(true); + + // The broken package is not written back out. + const saved = Buffer.from(await file(lockbPath).arrayBuffer()); + expect(packageResolutionTagOffsets(saved).map(offset => saved[offset])).toEqual([1, 2, 2]); + }, +); + +it.each(resolutionTagsWithoutAWriter)( + "rejects a format v2 binary lockfile in which a package has the $name resolution tag", + async ({ tag }) => { + // `bun bun.lockb` only prints the lockfile, so this exercises the v2 + // migration path of the loader without a registry. + using dir = tempDir("lockb-v2-resolution-without-a-writer", {}); + const lockbPath = join(String(dir), "bun.lockb"); + const lockb = Buffer.from(await file(join(__dirname, "fixtures", "bun.lockb.v2")).arrayBuffer()); + expect(lockb.readUInt32LE(42)).toBe(2); + const offsets = packageResolutionTagOffsets(lockb); + expect(offsets.map(offset => lockb[offset])).toEqual([1, 2, 2, 2, 2, 2, 2]); + + await write(lockbPath, lockb); + { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "bun.lockb"], + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + expect(err).toBe(""); + expect(out).toContain("# yarn lockfile v1"); + expect(code).toBe(0); + } + + lockb[offsets[1]] = tag; + await write(lockbPath, lockb); + { + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "bun.lockb"], + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + expect(err).toContain("invalid resolution tag"); + expect(out).toBe(""); + expect(code).toBe(1); + } + }, +); + it("rejects a binary lockfile whose git resolved tag contains path separators", async () => { const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } });