From a87c40d30d027ba79aaa231b04c82c9e9913dec0 Mon Sep 17 00:00:00 2001
From: Claude Bot <claude-bot@bun.sh>
Date: Tue, 14 Oct 2025 08:13:01 +0000
Subject: [PATCH] Address additional CodeRabbit review comments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 src/cli/pack_command.zig | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/cli/pack_command.zig b/src/cli/pack_command.zig
index d41deac0a8..0b4b3189cd 100644
--- a/src/cli/pack_command.zig
+++ b/src/cli/pack_command.zig
@@ -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;