Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 22 additions & 18 deletions src/install/lockfile/Package.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1318,13 +1318,15 @@
}
}

if (json.asProperty("patchedDependencies")) |patched_deps| {
const obj = patched_deps.expr.data.e_object;
for (obj.properties.slice()) |prop| {
const key = prop.key.?;
const value = prop.value.?;
if (key.isString() and value.isString()) {
string_builder.count(value.asString(allocator).?);
if (comptime features.patched_dependencies) {
if (json.asProperty("patchedDependencies")) |patched_deps| {
const obj = patched_deps.expr.data.e_object;

Check notice on line 1323 in src/install/lockfile/Package.zig

View check run for this annotation

Claude / Claude Code Review

Pre-existing: unchecked .e_object access on patchedDependencies value

Pre-existing (not introduced here, and this PR actually *reduces* its surface): `patched_deps.expr.data.e_object` is accessed without checking the union tag, so a root `package.json` with e.g. `"patchedDependencies": []` or `"patchedDependencies": "foo"` would hit a Zig safety panic instead of a clean error. The same unchecked access exists in the second block at ~L1655. Since these lines are being re-indented anyway, a one-line `if (patched_deps.expr.data == .e_object)` guard (matching the adja
Comment thread
robobun marked this conversation as resolved.
for (obj.properties.slice()) |prop| {
const key = prop.key.?;
const value = prop.value.?;
if (key.isString() and value.isString()) {
string_builder.count(value.asString(allocator).?);
}
}
}
}
Expand Down Expand Up @@ -1648,17 +1650,19 @@
};
}

if (json.asProperty("patchedDependencies")) |patched_deps| {
const obj = patched_deps.expr.data.e_object;
lockfile.patched_dependencies.ensureTotalCapacity(allocator, obj.properties.len) catch unreachable;
for (obj.properties.slice()) |prop| {
const key = prop.key.?;
const value = prop.value.?;
if (key.isString() and value.isString()) {
var sfb = std.heap.stackFallback(1024, allocator);
const keyhash = try key.asStringHash(sfb.get(), String.Builder.stringHash) orelse unreachable;
const patch_path = string_builder.append(String, value.asString(allocator).?);
lockfile.patched_dependencies.put(allocator, keyhash, .{ .path = patch_path }) catch unreachable;
if (comptime features.patched_dependencies) {
if (json.asProperty("patchedDependencies")) |patched_deps| {
const obj = patched_deps.expr.data.e_object;
lockfile.patched_dependencies.ensureTotalCapacity(allocator, obj.properties.len) catch unreachable;
for (obj.properties.slice()) |prop| {
const key = prop.key.?;
const value = prop.value.?;
if (key.isString() and value.isString()) {
var sfb = std.heap.stackFallback(1024, allocator);
const keyhash = try key.asStringHash(sfb.get(), String.Builder.stringHash) orelse unreachable;
const patch_path = string_builder.append(String, value.asString(allocator).?);
lockfile.patched_dependencies.put(allocator, keyhash, .{ .path = patch_path }) catch unreachable;
}
}
}
}
Expand Down
102 changes: 102 additions & 0 deletions test/cli/install/bun-install-patch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,108 @@ index 832d92223a9ec491364ee10dcbe3ad495446ab80..7e079a817825de4b8c3d01898490dc7e
}
});

// https://github.com/oven-sh/bun/issues/13531
it("should ignore patchedDependencies from a folder dependency's package.json", async () => {
const patchFilename = filepathEscape("is-even@1.0.0.patch");
const filedir = tempDirWithFiles("patch-folder-dep", {
"pkgA": {
"package.json": JSON.stringify({
"name": "pkg-a",
"version": "1.0.0",
"patchedDependencies": {
"is-even@1.0.0": `patches/${patchFilename}`,
},
"dependencies": {
"is-even": "1.0.0",
},
}),
"patches": {
[patchFilename]: is_even_patch,
},
},
"pkgB": {
"package.json": JSON.stringify({
"name": "pkg-b",
"version": "1.0.0",
"dependencies": {
"pkg-a": "../pkgA",
},
}),
},
});

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
env: bunEnv,
cwd: join(filedir, "pkgB"),
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// patchedDependencies from pkgA should be ignored, not resolved relative to pkgB
expect(stderr).not.toContain("Couldn't find patch file");
expect(stderr).not.toContain("patches/");
expect({ stdout, exitCode }).toMatchObject({ exitCode: 0 });

// lockfile should not contain pkgA's patchedDependencies
const lockfile = await Bun.file(join(filedir, "pkgB", "bun.lock")).text();
expect(lockfile).toContain("pkg-a");
expect(lockfile).not.toContain("patchedDependencies");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
});

// https://github.com/oven-sh/bun/issues/13531 (workspace variant, see also #27894)
it("should ignore patchedDependencies from a workspace member's package.json", async () => {
// Only the install root's `patchedDependencies` is honored (matching
// `overrides`, `workspaces`, and `catalogs` behavior). Previously the
// member's patch paths were written to the root lockfile and resolved
// relative to the root directory, failing with "Couldn't find patch file".
const patchFilename = filepathEscape("is-even@1.0.0.patch");
const filedir = tempDirWithFiles("patch-workspace-member", {
"package.json": JSON.stringify({
name: "workspace-root",
workspaces: ["packages/*"],
}),
packages: {
lib: {
"package.json": JSON.stringify({
name: "lib",
dependencies: {
"is-even": "1.0.0",
},
// This only applies when `lib` itself is the install root.
patchedDependencies: {
"is-even@1.0.0": `patches/${patchFilename}`,
},
}),
patches: {
[patchFilename]: is_even_patch,
},
},
},
});

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
env: bunEnv,
cwd: filedir,
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).not.toContain("Couldn't find patch file");
expect(stderr).not.toContain("patches/");
expect({ stdout, exitCode }).toMatchObject({ exitCode: 0 });

// The member's patchedDependencies did not leak into the root lockfile.
const lockfile = await Bun.file(join(filedir, "bun.lock")).text();
expect(lockfile).not.toContain("patchedDependencies");
expect(lockfile).toContain("is-even");
});

describe("bun patch with --linker=isolated", () => {
const patchEnv = bunEnv;

Expand Down
Loading