From 5657d15efd844cde0e923c168c9f56adb3dee6b0 Mon Sep 17 00:00:00 2001 From: Aditya Garud Date: Fri, 24 Jul 2026 23:42:48 +0530 Subject: [PATCH 1/2] Carry a redacted stderr excerpt on git command failures Failed git commands recorded only stderrLength, so every precondition failure (branch exists, stale worktree metadata, permissions) collapsed into the same opaque message and diagnosing required rerunning git by hand. GitCommandError now carries a bounded stderrExcerpt for in-process consumers. Argument values are redacted from the excerpt and it stays out of the error schema and message, so nothing new reaches RPC clients, client events, or telemetry. Fixes #4380 --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 61 +++++++++++++++- apps/server/src/vcs/GitVcsDriverCore.ts | 75 ++++++++++++++++---- packages/contracts/src/git.ts | 7 ++ 3 files changed, 128 insertions(+), 15 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 7b5956de07f..d34c3830d4a 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -5,6 +5,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as PlatformError from "effect/PlatformError"; +import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; @@ -12,9 +13,11 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; import { GitCommandError } from "@t3tools/contracts"; import { ServerConfig } from "../config.ts"; -import { splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts"; +import { attachGitStderrExcerpt, splitNullSeparatedGitStdoutPaths } from "./GitVcsDriverCore.ts"; import * as GitVcsDriver from "./GitVcsDriver.ts"; +const encodeGitCommandError = Schema.encodeSync(GitCommandError); + const ServerConfigLayer = ServerConfig.layerTest(process.cwd(), { prefix: "t3-git-vcs-driver-test-", }); @@ -209,8 +212,64 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { assert.isAbove(error.stderrLength ?? 0, 0); assert.notInclude(error.detail, secret); assert.notInclude(error.message, secret); + assert.notInclude(error.stderrExcerpt ?? "", secret); assert.notProperty(error, "args"); assert.notProperty(error, "stderr"); + + const encoded = encodeGitCommandError(error); + assert.notProperty(encoded, "stderrExcerpt"); + }), + ); + + it.effect("carries a bounded stderr excerpt for in-process consumers", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const driver = yield* GitVcsDriver.GitVcsDriver; + + const error = yield* driver + .execute({ + operation: "GitVcsDriver.test.stderrExcerpt", + cwd, + args: ["log", "-1"], + }) + .pipe(Effect.flip); + + assert.equal(error._tag, "GitCommandError"); + assert.include(error.stderrExcerpt ?? "", "not a git repository"); + // Stderr must stay out of the message so client-facing events and + // telemetry keep their existing redaction guarantees. + assert.notInclude(error.message, "not a git repository"); + }), + ); + + it.effect("redacts argument values and caps the stderr excerpt", () => + Effect.sync(() => { + const longStderr = `fatal: ${"x".repeat(600)}`; + const capped = attachGitStderrExcerpt( + new GitCommandError({ + operation: "GitVcsDriver.test.cap", + command: "git", + cwd: "/tmp/repo", + detail: "Git command exited with a non-zero status.", + }), + longStderr, + [], + ); + assert.isAtMost(capped.stderrExcerpt?.length ?? 0, 403); + assert.equal(capped.stderrExcerpt?.endsWith("..."), true); + + const redacted = attachGitStderrExcerpt( + new GitCommandError({ + operation: "GitVcsDriver.test.redact", + command: "git", + cwd: "/tmp/repo", + detail: "Git command exited with a non-zero status.", + }), + "fatal: could not read from 'https://token-value@example.com/repo.git'", + ["fetch", "--depth=1", "https://token-value@example.com/repo.git"], + ); + assert.notInclude(redacted.stderrExcerpt ?? "", "token-value"); + assert.include(redacted.stderrExcerpt ?? "", "could not read from"); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 33eb6d40d76..736731aec7e 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -328,6 +328,45 @@ function gitCommandContext( } as const; } +const STDERR_EXCERPT_MAX_LENGTH = 400; +const STDERR_REDACTION_MIN_LENGTH = 4; + +/** + * Attach a bounded stderr excerpt to a command failure for in-process + * consumers. Command arguments (and `--option=value` values) are redacted + * first so tokens embedded in URLs or options keep the same redaction + * guarantees as the structured error fields. The excerpt lives outside the + * error schema, so it never leaves the process. + */ +export function attachGitStderrExcerpt( + error: GitCommandError, + stderr: string, + args: readonly string[], +): GitCommandError { + const redactionCandidates = args + .flatMap((arg) => { + const separatorIndex = arg.indexOf("="); + return separatorIndex >= 0 ? [arg, arg.slice(separatorIndex + 1)] : [arg]; + }) + .filter((candidate) => candidate.length >= STDERR_REDACTION_MIN_LENGTH) + .sort((left, right) => right.length - left.length); + + let excerpt = stderr; + for (const candidate of redactionCandidates) { + excerpt = excerpt.split(candidate).join(""); + } + excerpt = excerpt.replace(/\s+/g, " ").trim(); + if (excerpt.length === 0) { + return error; + } + + error.stderrExcerpt = + excerpt.length > STDERR_EXCERPT_MAX_LENGTH + ? `${excerpt.slice(0, STDERR_EXCERPT_MAX_LENGTH)}...` + : excerpt; + return error; +} + function parseDefaultBranchFromRemoteHeadRef(value: string, remoteName: string): string | null { const trimmed = value.trim(); const prefix = `refs/remotes/${remoteName}/`; @@ -730,13 +769,17 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* yield* trace2Monitor.flush; if (!input.allowNonZeroExit && exitCode !== 0) { - return yield* new GitCommandError({ - ...gitCommandContext(commandInput), - detail: "Git command exited with a non-zero status.", - exitCode, - stdoutLength: stdout.text.length, - stderrLength: stderr.text.length, - }); + return yield* attachGitStderrExcerpt( + new GitCommandError({ + ...gitCommandContext(commandInput), + detail: "Git command exited with a non-zero status.", + exitCode, + stdoutLength: stdout.text.length, + stderrLength: stderr.text.length, + }), + stderr.text, + commandInput.args, + ); } return { @@ -811,13 +854,17 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return Effect.succeed(result); } return Effect.fail( - new GitCommandError({ - ...gitCommandContext({ operation, cwd, args }), - detail: options.fallbackErrorDetail ?? "Git command exited with a non-zero status.", - ...(result.exitCode === null ? {} : { exitCode: result.exitCode }), - stdoutLength: result.stdout.length, - stderrLength: result.stderr.length, - }), + attachGitStderrExcerpt( + new GitCommandError({ + ...gitCommandContext({ operation, cwd, args }), + detail: options.fallbackErrorDetail ?? "Git command exited with a non-zero status.", + ...(result.exitCode === null ? {} : { exitCode: result.exitCode }), + stdoutLength: result.stdout.length, + stderrLength: result.stderr.length, + }), + result.stderr, + args, + ), ); }), ); diff --git a/packages/contracts/src/git.ts b/packages/contracts/src/git.ts index aa5cdf8432b..d4cd8dec648 100644 --- a/packages/contracts/src/git.ts +++ b/packages/contracts/src/git.ts @@ -332,6 +332,13 @@ export class GitCommandError extends Schema.TaggedErrorClass()( detail: Schema.String, cause: Schema.optional(Schema.Defect()), }) { + /** + * Bounded, argument-redacted git stderr excerpt for in-process consumers. + * Deliberately not part of the schema and not part of `message`, so it + * never crosses RPC, client event, or telemetry serialization boundaries. + */ + stderrExcerpt?: string; + override get message(): string { return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}`; } From 216d1b3540a1929de74d249ee83f11f0512a919b Mon Sep 17 00:00:00 2001 From: Aditya Garud Date: Sat, 25 Jul 2026 00:03:48 +0530 Subject: [PATCH 2/2] Always redact short option values from the stderr excerpt The length floor that keeps subcommand names readable also discarded extracted option values shorter than four characters, so a short secret in --option=value form could survive into the excerpt when stderr echoed only the value. Apply the floor to whole arguments only and redact extracted values at any length. --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 15 +++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 12 ++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index d34c3830d4a..70d5c1cb9a6 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -270,6 +270,21 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => { ); assert.notInclude(redacted.stderrExcerpt ?? "", "token-value"); assert.include(redacted.stderrExcerpt ?? "", "could not read from"); + + // Option values below the whole-argument length floor are still + // redacted: a short secret must not survive the filter. + const shortValue = attachGitStderrExcerpt( + new GitCommandError({ + operation: "GitVcsDriver.test.shortValue", + command: "git", + cwd: "/tmp/repo", + detail: "Git command exited with a non-zero status.", + }), + "fatal: authentication failed for abc", + ["fetch", "--token=abc"], + ); + assert.notInclude(shortValue.stderrExcerpt ?? "", "abc"); + assert.include(shortValue.stderrExcerpt ?? "", "authentication failed"); }), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 736731aec7e..87cc5413cec 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -345,10 +345,18 @@ export function attachGitStderrExcerpt( ): GitCommandError { const redactionCandidates = args .flatMap((arg) => { + // The length floor only applies to whole arguments, where it prevents + // mangling ordinary words like subcommand names. Extracted option + // values are redacted regardless of length so a short secret in + // `--option=value` form cannot slip through. + const candidates = arg.length >= STDERR_REDACTION_MIN_LENGTH ? [arg] : []; const separatorIndex = arg.indexOf("="); - return separatorIndex >= 0 ? [arg, arg.slice(separatorIndex + 1)] : [arg]; + const value = separatorIndex >= 0 ? arg.slice(separatorIndex + 1) : ""; + if (value.length > 0) { + candidates.push(value); + } + return candidates; }) - .filter((candidate) => candidate.length >= STDERR_REDACTION_MIN_LENGTH) .sort((left, right) => right.length - left.length); let excerpt = stderr;