From dfed8b301b15a0fdf58faba129457d4a8636d73a Mon Sep 17 00:00:00 2001 From: Henrik Soerensen Date: Thu, 21 May 2026 19:28:35 -0400 Subject: [PATCH] Preserve stderr so wrapped command failures are visible runProxy captured stderr from the executor and then never wrote it anywhere. Any command whose useful output lives on stderr was silently truncated to whatever the stdout filter produced. Concrete case: a repo with a lefthook pre-push hook that enforces coverage. When the hook fails, git push exits 1 and writes the hook error plus 'failed to push some refs' to stderr. Through ztk the agent saw: exit=1 output: ok filterGitPush returned 'ok' for the empty stdout, the real failure on stderr was thrown away, and the agent had no idea why the push failed. In practice this causes real churn: agents loop, retry, or give up because the wrapper hides the actual error. Fix: forward result.stderr to the user's stderr via compat.writeStderr. Stdout filtering is unchanged, so secret-masking filters like the env filter still run on failed commands. Tests cover stderr passthrough on both zero and nonzero exit, plus a regression test that env-style sensitive values (API_KEY etc.) stay masked when the underlying command exits nonzero. Note on stderr filtering: ztk has no stderr filters today. Secrets that show up on stderr (URL-embedded credentials in git auth errors, tokens in verbose curl/ssh output) already reach the agent's environment when those commands run outside ztk, so this PR does not introduce new exposure; it removes an accidental side effect of dropping the stream. A scrubber for URL-embedded credentials, GitHub/AWS tokens, and Bearer headers is a follow-up, but the current behavior silently turns real errors into 'ok', and that's the bigger bug. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/proxy.zig | 61 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/src/proxy.zig b/src/proxy.zig index bbfff36..2f6863f 100644 --- a/src/proxy.zig +++ b/src/proxy.zig @@ -17,21 +17,21 @@ pub fn runProxy(cmd_args: []const []const u8, allocator: std.mem.Allocator) !u8 if (rawEnvEnabled(allocator)) return runCommandRaw(cmd_args, allocator); const result = try executor.exec(cmd_args, allocator, .filter_stdout_only); - const filtered = applyFilters(cmd_str, result.stdout, allocator); - const final_bytes = maybeApplySession(cmd_str, filtered, allocator); + const processed = processOutput(cmd_str, result, allocator); if (!builtin.is_test) { const log_path = resolveLogPath(allocator) catch null; defer if (log_path) |p| allocator.free(p); output.emitWithCommand( - final_bytes, + processed.stdout, .{ .command = cmd_args[0], .original = result.stdout.len, - .filtered = final_bytes.len, + .filtered = processed.stdout.len, .exit_code = result.exit_code, }, log_path, ) catch {}; + if (processed.stderr.len > 0) compat.writeStderr(processed.stderr) catch {}; } return result.exit_code; } @@ -93,6 +93,20 @@ const FilteredOutput = struct { matched: bool, }; +const ProcessedOutput = struct { + stdout: []const u8, + stderr: []const u8, +}; + +fn processOutput(cmd: []const u8, result: executor.ExecResult, allocator: std.mem.Allocator) ProcessedOutput { + const filtered = applyFilters(cmd, result.stdout, allocator); + const final_bytes = maybeApplySession(cmd, filtered, allocator); + return .{ + .stdout = final_bytes, + .stderr = result.stderr, + }; +} + fn applyFilters(cmd: []const u8, stdout_bytes: []const u8, allocator: std.mem.Allocator) FilteredOutput { if (comptime_filters.dispatch(cmd, stdout_bytes, allocator)) |fr| { return .{ .bytes = fr.output, .stateful = fr.stateful, .category = fr.category, .matched = true }; @@ -124,3 +138,42 @@ test "runProxy preserves nonzero exit code" { const code = try runProxy(&.{ "sh", "-c", "exit 42" }, arena.allocator()); try std.testing.expectEqual(@as(u8, 42), code); } + +test "processOutput forwards stderr verbatim on nonzero exit" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const exec_result: executor.ExecResult = .{ + .stdout = "", + .stderr = "ERROR: Coverage for branches (96.76%) does not meet global threshold (97%)\nerror: failed to push some refs to 'github.com:foo/bar.git'\n", + .exit_code = 1, + }; + const processed = processOutput("git push", exec_result, arena.allocator()); + try std.testing.expectEqualStrings(exec_result.stderr, processed.stderr); +} + +test "processOutput forwards stderr verbatim on zero exit" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const exec_result: executor.ExecResult = .{ + .stdout = "To github.com:user/repo\n abc..def main -> main\n", + .stderr = "progress noise\n", + .exit_code = 0, + }; + const processed = processOutput("git push", exec_result, arena.allocator()); + try std.testing.expectEqualStrings(exec_result.stderr, processed.stderr); + try std.testing.expect(std.mem.indexOf(u8, processed.stdout, "ok") != null); +} + +test "processOutput keeps stdout filter masking sensitive values on nonzero exit" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const exec_result: executor.ExecResult = .{ + .stdout = "API_KEY=topsecret\nPATH=/usr/bin\n", + .stderr = "command failed\n", + .exit_code = 1, + }; + const processed = processOutput("env", exec_result, arena.allocator()); + try std.testing.expect(std.mem.indexOf(u8, processed.stdout, "") != null); + try std.testing.expect(std.mem.indexOf(u8, processed.stdout, "topsecret") == null); + try std.testing.expectEqualStrings(exec_result.stderr, processed.stderr); +}