mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-17 06:12:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address additional CodeRabbit review comments

Browse Source 
Fixes two issues identified in review:

1. PathBuffer aliasing (comment #2428232461):
   - Use separate buffers (norm_buf, join_buf1, join_buf2) to avoid
     input/output overlap in path operations
   - Validate ".." before normalization to catch parent traversal attempts
     (normalizeBuf resolves ".." away, so we must check the raw input)
   - Keep substring check for ".." as it's appropriately conservative for
     security validation of publishConfig.directory

2. Memory leak (comment #2428232477):
   - Added defer to free edited_package_json buffer after allocation
   - Ensures cleanup happens on all code paths (dry-run vs normal execution)

All 9 publishConfig.directory tests still passing.

Note: Skipped adding publish-specific test (comment #2428232485) because:
- pack and publish share the same code path for publishConfig.directory
- The subdirectory logic executes before the for_publish branch
- Testing publish requires complex npm auth setup
- Current pack tests provide comprehensive coverage of the feature

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude Bot
2025-10-14 08:13:01 +00:00
parent 28bfafbf13
commit a87c40d30d
1 changed files with 13 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/cli/pack_command.zig
View File

@@ -1183,30 +1183,34 @@ pub const PackCommand = struct {
Global.crash();
}
// Reject parent directory traversal before normalization
// Reject parent directory traversal before normalization (check raw input)
// We check before normalization because normalizeBuf may resolve ".." away
if (strings.contains(directory, "..")) {
Output.errGeneric("publishConfig.directory cannot contain '..': {s}", .{directory});
Output.flush();
Global.crash();
}
var path_buf: PathBuffer = undefined;
const normalized_dir = strings.withoutTrailingSlash(strings.withoutPrefixComptime(
bun.path.normalizeBuf(directory, &path_buf, .posix),
"./",
));
// Normalize after validation
var norm_buf: PathBuffer = undefined;
const normalized0 = bun.path.normalizeBuf(directory, &norm_buf, .posix);
const normalized_dir = strings.withoutTrailingSlash(strings.withoutPrefixComptime(normalized0, "./"));
const abs_workspace_path: string = strings.withoutTrailingSlash(strings.withoutSuffixComptime(abs_package_json_path, "package.json"));
// Use a separate buffer for joins to avoid aliasing with normalized buffer
var join_buf1: PathBuffer = undefined;
const abs_dir = bun.path.joinAbsStringBuf(
abs_workspace_path,
&path_buf,
&join_buf1,
&[_]string{normalized_dir},
.auto,
);
var join_buf2: PathBuffer = undefined;
break :path_with_dir bun.path.joinAbsStringBufZ(
abs_dir,
&path_buf,
&join_buf2,
&[_]string{"package.json"},
.auto,
);
@@ -1262,6 +1266,7 @@ pub const PackCommand = struct {
}
const edited_package_json = try editRootPackageJSON(ctx.allocator, ctx.lockfile, actual_json);
defer ctx.allocator.free(edited_package_json);
var this_transpiler: bun.transpiler.Transpiler = undefined;