From 3501a64cb36c36b1895dae444c06f1fe194938c8 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 11:26:14 -0300 Subject: [PATCH 1/3] fix git hook env contamination --- bin/pushgate.mjs | 65 +++++++-- docs/fix-git-hook-env-contamination.md | 34 +++++ src/ai/providers/claude.ts | 3 +- src/ai/providers/run-provider-command.ts | 3 +- src/git/command.ts | 6 +- src/git/config.ts | 4 + src/git/environment.ts | 65 +++++++++ src/runner/plugins/gitleaks.ts | 3 +- src/runner/tool-command.ts | 3 +- src/skip-controls.ts | 4 +- test/ai.test.ts | 117 ++++++++++++--- test/deterministic-runner.test.ts | 174 +++++++++++++++++++++-- test/git-environment.test.ts | 167 ++++++++++++++++++++++ test/install.test.ts | 6 +- test/path-policy.test.ts | 2 + test/runner.test.ts | 12 +- test/support/hook-harness.ts | 6 +- 17 files changed, 622 insertions(+), 52 deletions(-) create mode 100644 docs/fix-git-hook-env-contamination.md create mode 100644 src/git/environment.ts create mode 100644 test/git-environment.test.ts diff --git a/bin/pushgate.mjs b/bin/pushgate.mjs index 62b134f..30d1862 100755 --- a/bin/pushgate.mjs +++ b/bin/pushgate.mjs @@ -9676,6 +9676,50 @@ function completedResult(result) { }; } +// src/git/environment.ts +var GIT_LOCAL_ENV_VARS = /* @__PURE__ */ new Set([ + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_COMMON_DIR", + "GIT_CONFIG", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_PARAMETERS", + "GIT_DIR", + "GIT_GRAFT_FILE", + "GIT_IMPLICIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_NO_REPLACE_OBJECTS", + "GIT_OBJECT_DIRECTORY", + "GIT_PREFIX", + "GIT_REPLACE_REF_BASE", + "GIT_SHALLOW_FILE", + "GIT_WORK_TREE" +]); +var GIT_CONFIG_PAIR_ENV_VAR = /^GIT_CONFIG_(?:KEY|VALUE)_\d+$/; +function sanitizeGitLocalEnv(env, options = {}) { + const sanitized = {}; + for (const [key, value] of Object.entries(env)) { + if (shouldRemoveGitEnvVar(key, options)) { + continue; + } + if (value !== void 0) { + sanitized[key] = value; + } + } + return sanitized; +} +function isGitLocalEnvVar(key) { + return GIT_LOCAL_ENV_VARS.has(key) || GIT_CONFIG_PAIR_ENV_VAR.test(key); +} +function shouldRemoveGitEnvVar(key, options) { + if (options.preserveGitConfigOverlay && isGitConfigOverlayEnvVar(key)) { + return false; + } + return isGitLocalEnvVar(key); +} +function isGitConfigOverlayEnvVar(key) { + return key === "GIT_CONFIG_COUNT" || key === "GIT_CONFIG_PARAMETERS" || GIT_CONFIG_PAIR_ENV_VAR.test(key); +} + // src/git/command.ts var GitCommandError = class extends Error { gitArgs; @@ -9692,7 +9736,9 @@ function runGit(repoRoot, args, options = {}) { args, command: "git", cwd: repoRoot, - env: options.env + env: sanitizeGitLocalEnv(options.env ?? process.env, { + preserveGitConfigOverlay: options.preserveGitConfigOverlay + }) }; if (options.encoding === "buffer") { return runCommand({ @@ -9842,11 +9888,12 @@ var GitConfigError = class extends Error { this.name = new.target.name; } }; -async function readGitBooleanConfig(repoRoot, key, env = process.env) { +async function readGitBooleanConfig(repoRoot, key, env = process.env, options = {}) { let result; try { result = await runGit(repoRoot, ["config", "--bool", "--get", key], { - env + env, + preserveGitConfigOverlay: options.preserveGitConfigOverlay }); } catch (error51) { throw new GitConfigError( @@ -9946,7 +9993,9 @@ async function resolveSkipControlState(repoRoot, env = process.env) { } async function readSkipBooleanConfig(repoRoot, env, key) { try { - return await readGitBooleanConfig(repoRoot, key, env); + return await readGitBooleanConfig(repoRoot, key, env, { + preserveGitConfigOverlay: true + }); } catch (error51) { if (error51 instanceof GitConfigError) { throw new SkipControlError(error51.message); @@ -25638,7 +25687,7 @@ async function runProviderCommand(options) { args: options.args, command: options.command, cwd: options.cwd, - env: options.env, + env: sanitizeGitLocalEnv(options.env), outputCaptureLimit: options.outputCaptureLimit ?? null, outputTailLimit: options.outputTailLimit ?? DEFAULT_OUTPUT_TAIL_LIMIT, // Provider CLIs may exit before stdin fully drains; the process runner still @@ -26039,7 +26088,7 @@ async function isClaudeUnauthenticated(repoRoot, env) { args: ["auth", "status"], command: "claude", cwd: repoRoot, - env + env: sanitizeGitLocalEnv(env) }); return result.code === 1; } catch { @@ -26840,7 +26889,7 @@ async function runGitleaksPlugin(plugin, changedFileResolution, repoRoot, env) { args: buildGitleaksArgs(plugin, changedFileResolution, repoRoot, reportPath), command: plugin.command, cwd: repoRoot, - env, + env: sanitizeGitLocalEnv(env), timeoutSeconds: plugin.timeout_seconds }); if (!isProcessCompletionOutcome(commandResult)) { @@ -27082,7 +27131,7 @@ async function runToolCommand(tool, changedFilePaths, repoRoot, env) { args, command: executable, cwd: repoRoot, - env, + env: sanitizeGitLocalEnv(env), timeoutSeconds: tool.timeout_seconds }); if (commandResult.kind === "passed") { diff --git a/docs/fix-git-hook-env-contamination.md b/docs/fix-git-hook-env-contamination.md new file mode 100644 index 0000000..3d8e25f --- /dev/null +++ b/docs/fix-git-hook-env-contamination.md @@ -0,0 +1,34 @@ +# Fix Git Hook Environment Contamination + +## Summary + +Pushgate must not forward Git hook-local repository binding variables into subprocesses that run outside the hook's own Git push operation. When variables such as `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, or `GIT_COMMON_DIR` leak into configured tools, plugins, providers, or repo-targeted Git helpers, nested Git commands can operate on the repository being pushed instead of the subprocess `cwd`. + +This fix is independent from CLI logging work. It is a safety and correctness change with no config schema changes. + +## Implementation + +- Add `sanitizeGitLocalEnv(env)` in `src/git/environment.ts`. +- Strip variables reported by `git rev-parse --local-env-vars`, including `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, `GIT_COMMON_DIR`, `GIT_CONFIG`, and `GIT_CONFIG_COUNT`. +- Strip dynamic `GIT_CONFIG_KEY_` and `GIT_CONFIG_VALUE_` entries. +- Preserve transport and auth variables such as `GIT_SSH_COMMAND`, `GIT_ASKPASS`, and `GIT_TERMINAL_PROMPT`. +- Sanitize external Pushgate subprocesses: + - configured deterministic tools + - Gitleaks plugin command + - AI provider CLI commands +- Sanitize repo-targeted internal Git helpers through the shared `runGit` wrapper. +- Leave the native `git push` wrapper unsanitized because it intentionally runs Git's push operation in the caller's context. +- Keep Git-spawning test helpers safe when tests are launched from a Git hook. + +## Test Plan + +- Unit-test `sanitizeGitLocalEnv`. +- Regression-test a configured tool running nested Git with a poisoned parent Git environment. +- Verify Gitleaks and AI provider command stubs do not receive hook-local Git repository variables. +- Run `pnpm run typecheck`. +- Run `pnpm test`. +- Run `pnpm run check:shell`. + +## Manual Acceptance + +After the fix, running `git push` in `ai-pushgate` may pass or fail checks normally, but Pushgate subprocesses must not create `baseline` commits, switch or create `feature`, delete fixture paths, or leave index locks in the real repository because of inherited hook-local Git variables. diff --git a/src/ai/providers/claude.ts b/src/ai/providers/claude.ts index 041932f..6f5dadb 100644 --- a/src/ai/providers/claude.ts +++ b/src/ai/providers/claude.ts @@ -1,3 +1,4 @@ +import { sanitizeGitLocalEnv } from "../../git/environment.js"; import { runCommand } from "../../process/run-command.js"; import { generateAiReviewOutputJsonSchema } from "../review-contract.js"; import { createCommandProviderAdapter } from "./command-provider-adapter.js"; @@ -320,7 +321,7 @@ async function isClaudeUnauthenticated( args: ["auth", "status"], command: "claude", cwd: repoRoot, - env, + env: sanitizeGitLocalEnv(env), }); return result.code === 1; diff --git a/src/ai/providers/run-provider-command.ts b/src/ai/providers/run-provider-command.ts index 95c98cf..b9b9899 100644 --- a/src/ai/providers/run-provider-command.ts +++ b/src/ai/providers/run-provider-command.ts @@ -1,3 +1,4 @@ +import { sanitizeGitLocalEnv } from "../../git/environment.js"; import { isProcessCompletionOutcome, runProcessOutcome, @@ -37,7 +38,7 @@ export async function runProviderCommand(options: { args: options.args, command: options.command, cwd: options.cwd, - env: options.env, + env: sanitizeGitLocalEnv(options.env), outputCaptureLimit: options.outputCaptureLimit ?? null, outputTailLimit: options.outputTailLimit ?? DEFAULT_OUTPUT_TAIL_LIMIT, // Provider CLIs may exit before stdin fully drains; the process runner still diff --git a/src/git/command.ts b/src/git/command.ts index f4a2d63..09993eb 100644 --- a/src/git/command.ts +++ b/src/git/command.ts @@ -3,6 +3,7 @@ import { type CommandResult, type RunCommandOptions, } from "../process/run-command.js"; +import { sanitizeGitLocalEnv } from "./environment.js"; export type GitCommandEncoding = "buffer" | "utf8"; export type GitCommandResult = @@ -15,6 +16,7 @@ type GitCommandFailureResult = Pick< export interface GitCommandOptions { encoding?: GitCommandEncoding; env?: NodeJS.ProcessEnv; + preserveGitConfigOverlay?: boolean; } export class GitCommandError extends Error { @@ -51,7 +53,9 @@ export function runGit( args, command: "git", cwd: repoRoot, - env: options.env, + env: sanitizeGitLocalEnv(options.env ?? process.env, { + preserveGitConfigOverlay: options.preserveGitConfigOverlay, + }), }; if (options.encoding === "buffer") { diff --git a/src/git/config.ts b/src/git/config.ts index df3b97a..0fcb0d2 100644 --- a/src/git/config.ts +++ b/src/git/config.ts @@ -11,12 +11,16 @@ export async function readGitBooleanConfig( repoRoot: string, key: string, env: NodeJS.ProcessEnv = process.env, + options: { + preserveGitConfigOverlay?: boolean; + } = {}, ): Promise { let result: Awaited>; try { result = await runGit(repoRoot, ["config", "--bool", "--get", key], { env, + preserveGitConfigOverlay: options.preserveGitConfigOverlay, }); } catch (error) { throw new GitConfigError( diff --git a/src/git/environment.ts b/src/git/environment.ts new file mode 100644 index 0000000..17083a2 --- /dev/null +++ b/src/git/environment.ts @@ -0,0 +1,65 @@ +const GIT_LOCAL_ENV_VARS = new Set([ + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_COMMON_DIR", + "GIT_CONFIG", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_PARAMETERS", + "GIT_DIR", + "GIT_GRAFT_FILE", + "GIT_IMPLICIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_NO_REPLACE_OBJECTS", + "GIT_OBJECT_DIRECTORY", + "GIT_PREFIX", + "GIT_REPLACE_REF_BASE", + "GIT_SHALLOW_FILE", + "GIT_WORK_TREE", +]); + +const GIT_CONFIG_PAIR_ENV_VAR = /^GIT_CONFIG_(?:KEY|VALUE)_\d+$/; + +export interface SanitizeGitLocalEnvOptions { + preserveGitConfigOverlay?: boolean; +} + +export function sanitizeGitLocalEnv( + env: NodeJS.ProcessEnv, + options: SanitizeGitLocalEnvOptions = {}, +): NodeJS.ProcessEnv { + const sanitized: NodeJS.ProcessEnv = {}; + + for (const [key, value] of Object.entries(env)) { + if (shouldRemoveGitEnvVar(key, options)) { + continue; + } + + if (value !== undefined) { + sanitized[key] = value; + } + } + + return sanitized; +} + +export function isGitLocalEnvVar(key: string): boolean { + return GIT_LOCAL_ENV_VARS.has(key) || GIT_CONFIG_PAIR_ENV_VAR.test(key); +} + +function shouldRemoveGitEnvVar( + key: string, + options: SanitizeGitLocalEnvOptions, +): boolean { + if (options.preserveGitConfigOverlay && isGitConfigOverlayEnvVar(key)) { + return false; + } + + return isGitLocalEnvVar(key); +} + +function isGitConfigOverlayEnvVar(key: string): boolean { + return ( + key === "GIT_CONFIG_COUNT" || + key === "GIT_CONFIG_PARAMETERS" || + GIT_CONFIG_PAIR_ENV_VAR.test(key) + ); +} diff --git a/src/runner/plugins/gitleaks.ts b/src/runner/plugins/gitleaks.ts index 1f0a11c..5260907 100644 --- a/src/runner/plugins/gitleaks.ts +++ b/src/runner/plugins/gitleaks.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import type { GitleaksPluginConfig } from "../../config/index.js"; +import { sanitizeGitLocalEnv } from "../../git/environment.js"; import type { ChangedFileResolution } from "../../path-policy/index.js"; import { formatProcessFailure, @@ -47,7 +48,7 @@ export async function runGitleaksPlugin( args: buildGitleaksArgs(plugin, changedFileResolution, repoRoot, reportPath), command: plugin.command, cwd: repoRoot, - env, + env: sanitizeGitLocalEnv(env), timeoutSeconds: plugin.timeout_seconds, }); diff --git a/src/runner/tool-command.ts b/src/runner/tool-command.ts index 8bdfdfb..0f0f3cb 100644 --- a/src/runner/tool-command.ts +++ b/src/runner/tool-command.ts @@ -1,4 +1,5 @@ import type { ToolConfig } from "../config/index.js"; +import { sanitizeGitLocalEnv } from "../git/environment.js"; import { formatProcessFailure, runProcessOutcome, @@ -32,7 +33,7 @@ export async function runToolCommand( args, command: executable, cwd: repoRoot, - env, + env: sanitizeGitLocalEnv(env), timeoutSeconds: tool.timeout_seconds, }); diff --git a/src/skip-controls.ts b/src/skip-controls.ts index 3b88bd6..e158b67 100644 --- a/src/skip-controls.ts +++ b/src/skip-controls.ts @@ -116,7 +116,9 @@ async function readSkipBooleanConfig( key: string, ): Promise { try { - return await readGitBooleanConfig(repoRoot, key, env); + return await readGitBooleanConfig(repoRoot, key, env, { + preserveGitConfigOverlay: true, + }); } catch (error) { if (error instanceof GitConfigError) { throw new SkipControlError(error.message); diff --git a/test/ai.test.ts b/test/ai.test.ts index c9c8ce6..82f3b31 100644 --- a/test/ai.test.ts +++ b/test/ai.test.ts @@ -34,6 +34,10 @@ import type { ProviderCommandResult } from "../src/ai/providers/run-provider-com import { renderLocalAiTranscript } from "../src/ai/transcript.js"; import { buildLocalAiVerdict } from "../src/ai/verdict.js"; import type { LocalAiProviderAdapter } from "../src/ai/types.js"; +import { + isGitLocalEnvVar, + sanitizeGitLocalEnv, +} from "../src/git/environment.js"; import { resolveChangedFiles } from "../src/path-policy/index.js"; test("validates the canonical AI review contract with Zod", () => { @@ -1046,7 +1050,7 @@ test("runs the Claude adapter through the provider interface with model selectio }, changedFileResolution, env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), PUSHGATE_CLAUDE_ARGS_OUT: argsPath, PUSHGATE_CLAUDE_PROMPT_OUT: promptPath, @@ -1121,7 +1125,7 @@ test("lets Claude provider config opt into bare mode for API-key scripts", async const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), PUSHGATE_CLAUDE_ARGS_OUT: argsPath, }, @@ -1144,6 +1148,59 @@ test("lets Claude provider config opt into bare mode for API-key scripts", async }); }); +test("sanitizes hook-local Git env before AI provider CLI commands", async () => { + await withAiRepo(async (repoRoot) => { + const binDir = join(repoRoot, "bin"); + const envPath = join(repoRoot, "provider-env.json"); + const victimRoot = join(repoRoot, "victim-repo"); + + await mkdir(binDir, { recursive: true }); + await mkdir(victimRoot, { recursive: true }); + await checkedRun("git", ["init", "--quiet", "--initial-branch=main"], { + cwd: victimRoot, + }); + await writeFile( + join(binDir, "claude"), + [ + "#!/usr/bin/env node", + "import { writeFileSync } from 'node:fs';", + "const gitEnv = Object.fromEntries(Object.entries(process.env).filter(([key]) => key.startsWith('GIT_')));", + "writeFileSync(process.env.PUSHGATE_PROVIDER_ENV_OUT, JSON.stringify(gitEnv));", + "for await (const _chunk of process.stdin) {}", + `process.stdout.write(${JSON.stringify( + claudeStructuredOutputJson({ + schema_version: 1, + findings: [], + }), + )});`, + ].join("\n"), + ); + await chmod(join(binDir, "claude"), 0o755); + + const result = await claudeProvider.runReview({ + env: { + ...sanitizeGitLocalEnv(process.env), + ...poisonedGitEnv(victimRoot), + PATH: [binDir, process.env.PATH ?? ""].join(delimiter), + PUSHGATE_PROVIDER_ENV_OUT: envPath, + }, + payload: minimalReviewPayload(), + providerConfig: {}, + repoRoot, + timeoutSeconds: 120, + }); + + assert.equal(result.kind, "review"); + + const recorded = JSON.parse(await readFile(envPath, "utf8")) as Record< + string, + string + >; + + assert.deepEqual(leakedGitLocalVars(recorded), []); + }); +}); + test("runs the Claude adapter with native structured output and source metadata", async () => { await withAiRepo(async (repoRoot) => { const binDir = join(repoRoot, "bin"); @@ -1177,7 +1234,7 @@ test("runs the Claude adapter with native structured output and source metadata" const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload("Review this Pushgate payload.\n"), @@ -1222,7 +1279,7 @@ test("reports malformed Claude structured output JSON", async () => { const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1285,7 +1342,7 @@ test("reports malformed Claude structured output envelopes", async () => { const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), PUSHGATE_CLAUDE_OUTPUT_FILE: outputPath, }, @@ -1338,7 +1395,7 @@ test("reports invalid Claude structured review objects", async () => { const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1379,7 +1436,7 @@ test("reports unsupported Claude structured-output mode", async () => { const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1419,7 +1476,7 @@ test("reports Claude auth failures before generic command failures", async () => const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1466,7 +1523,7 @@ test("classifies Claude prompt-mode login output as an auth failure", async () = const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1507,7 +1564,7 @@ test("reports generic Claude command failures", async () => { const result = await claudeProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1563,7 +1620,7 @@ test("runs the Copilot adapter with non-interactive stdin prompt and model selec const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), PUSHGATE_COPILOT_ARGS_OUT: argsPath, PUSHGATE_COPILOT_PROMPT_OUT: promptPath, @@ -1636,7 +1693,7 @@ test("parses Copilot JSONL output larger than the provider transcript tail", asy const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1690,7 +1747,7 @@ test("runs the Copilot adapter when the provider wraps JSON in a list marker", a const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1739,7 +1796,7 @@ test("runs the Copilot adapter when the provider emits a whitespace-corrupted fi const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1802,7 +1859,7 @@ test("maps Copilot auth-like failures through advisory mode", async () => { }, changedFileResolution, env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, repoRoot, @@ -1829,7 +1886,7 @@ test("reports missing Copilot CLI as a provider failure", async () => { const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: emptyBinDir, }, payload: minimalReviewPayload(), @@ -1865,7 +1922,7 @@ test("reports malformed Copilot JSONL transport output", async () => { const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1915,7 +1972,7 @@ test("reports missing final Copilot assistant response", async () => { const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1954,7 +2011,7 @@ test("reports invalid Copilot final review content through the normalized parser const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -1991,7 +2048,7 @@ test("passes configured timeout seconds to the Copilot adapter", async () => { const result = await copilotProvider.runReview({ env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, payload: minimalReviewPayload(), @@ -2163,7 +2220,7 @@ test("passes configured timeout seconds to the Claude adapter", async () => { }, changedFileResolution, env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }, repoRoot, @@ -2235,6 +2292,7 @@ async function checkedRun( }>((resolve, reject) => { const child = spawn(command, args, { cwd: options.cwd, + env: sanitizeGitLocalEnv(process.env), stdio: ["ignore", "pipe", "pipe"], }); let stderr = ""; @@ -2265,6 +2323,23 @@ async function checkedRun( } } +function poisonedGitEnv(victimRoot: string): NodeJS.ProcessEnv { + return { + GIT_COMMON_DIR: join(victimRoot, ".git"), + GIT_CONFIG: join(victimRoot, ".git", "config"), + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "core.bare", + GIT_CONFIG_VALUE_0: "true", + GIT_DIR: join(victimRoot, ".git"), + GIT_INDEX_FILE: join(victimRoot, ".git", "index"), + GIT_WORK_TREE: victimRoot, + }; +} + +function leakedGitLocalVars(env: Record): string[] { + return Object.keys(env).filter(isGitLocalEnvVar).sort(); +} + async function writeRepoFile( repoRoot: string, relativePath: string, diff --git a/test/deterministic-runner.test.ts b/test/deterministic-runner.test.ts index d299482..ec51633 100644 --- a/test/deterministic-runner.test.ts +++ b/test/deterministic-runner.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { Writable } from "node:stream"; @@ -10,6 +10,11 @@ import type { PushgateConfig, ToolConfig, } from "../src/config/index.js"; +import { runGit } from "../src/git/command.js"; +import { + isGitLocalEnvVar, + sanitizeGitLocalEnv, +} from "../src/git/environment.js"; import type { ChangedFile, ChangedFileResolution, @@ -95,7 +100,10 @@ test("expands changed files as argv entries without shell interpolation", async }), ]), { - env: { ...process.env, PUSHGATE_ARGS_OUT: argsPath }, + env: { + ...sanitizeGitLocalEnv(process.env), + PUSHGATE_ARGS_OUT: argsPath, + }, repoRoot, stdout: output.stream, }, @@ -122,7 +130,10 @@ test("skips changed-file tools when no live scoped files match", async () => { }), ]), { - env: { ...process.env, PUSHGATE_ARGS_OUT: argsPath }, + env: { + ...sanitizeGitLocalEnv(process.env), + PUSHGATE_ARGS_OUT: argsPath, + }, repoRoot, stdout: output.stream, }, @@ -149,7 +160,10 @@ test("runs always-mode tools even when scoped changed files are empty", async () }), ]), { - env: { ...process.env, PUSHGATE_ARGS_OUT: argsPath }, + env: { + ...sanitizeGitLocalEnv(process.env), + PUSHGATE_ARGS_OUT: argsPath, + }, repoRoot, stdout: output.stream, }, @@ -160,6 +174,46 @@ test("runs always-mode tools even when scoped changed files are empty", async () }); }); +test("sanitizes hook-local Git env before configured tools run nested Git", async () => { + await withTempDir(async (repoRoot) => { + const victimRoot = join(repoRoot, "victim-repo"); + const recorder = await writeNestedGitRecorder(repoRoot); + const resultPath = join(repoRoot, "nested-git.json"); + const output = captureOutput(); + + await initGitRepo(repoRoot); + await initGitRepo(victimRoot); + + const summary = await runChecks( + configWithTools([ + tool({ + command: [process.execPath, recorder], + run: "always", + }), + ]), + { + env: { + ...sanitizeGitLocalEnv(process.env), + ...poisonedGitEnv(victimRoot), + PUSHGATE_NESTED_GIT_OUT: resultPath, + }, + repoRoot, + stdout: output.stream, + }, + ); + + assert.equal(summary.exitCode, 0, output.text()); + + const recorded = JSON.parse(await readFile(resultPath, "utf8")) as { + gitEnv: Record; + topLevel: string; + }; + + assert.equal(await realpath(recorded.topLevel), await realpath(repoRoot)); + assert.deepEqual(leakedGitLocalVars(recorded.gitEnv), []); + }); +}); + test("blocks on blocking command failures", async () => { await withTempDir(async (repoRoot) => { const output = captureOutput(); @@ -234,7 +288,10 @@ test("fail_fast controls whether later tools run after blocking failures", async tool({ command: [process.execPath, recorder] }), ]), { - env: { ...process.env, PUSHGATE_ARGS_OUT: failFastArgsPath }, + env: { + ...sanitizeGitLocalEnv(process.env), + PUSHGATE_ARGS_OUT: failFastArgsPath, + }, repoRoot, stdout: captureOutput().stream, }, @@ -253,7 +310,10 @@ test("fail_fast controls whether later tools run after blocking failures", async tool({ command: [process.execPath, recorder] }), ]), { - env: { ...process.env, PUSHGATE_ARGS_OUT: aggregateArgsPath }, + env: { + ...sanitizeGitLocalEnv(process.env), + PUSHGATE_ARGS_OUT: aggregateArgsPath, + }, repoRoot, stdout: captureOutput().stream, }, @@ -298,7 +358,7 @@ test("runs Gitleaks plugin over the resolved branch commit range", async () => { }, { env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PUSHGATE_GITLEAKS_ARGS_OUT: argsPath, PUSHGATE_GITLEAKS_EXIT_CODE: "1", PUSHGATE_GITLEAKS_REPORT: JSON.stringify([ @@ -328,6 +388,44 @@ test("runs Gitleaks plugin over the resolved branch commit range", async () => { }); }); +test("sanitizes hook-local Git env before Gitleaks plugin commands", async () => { + await withTempDir(async (repoRoot) => { + const victimRoot = join(repoRoot, "victim-repo"); + const gitleaks = await writeGitleaksStub(repoRoot); + const envPath = join(repoRoot, "gitleaks-env.json"); + const output = captureOutput(); + + await initGitRepo(victimRoot); + + const summary = await runChecks( + { + ...configWithTools([]), + plugins: { + gitleaks: gitleaksPlugin({ command: gitleaks }), + }, + }, + { + env: { + ...sanitizeGitLocalEnv(process.env), + ...poisonedGitEnv(victimRoot), + PUSHGATE_GITLEAKS_ENV_OUT: envPath, + }, + repoRoot, + stdout: output.stream, + }, + ); + + assert.equal(summary.exitCode, 0, output.text()); + + const recorded = JSON.parse(await readFile(envPath, "utf8")) as Record< + string, + string + >; + + assert.deepEqual(leakedGitLocalVars(recorded), []); + }); +}); + test("warning-mode Gitleaks findings do not stop later tools", async () => { await withTempDir(async (repoRoot) => { const gitleaks = await writeGitleaksStub(repoRoot); @@ -350,7 +448,7 @@ test("warning-mode Gitleaks findings do not stop later tools", async () => { }, { env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PUSHGATE_ARGS_OUT: argsPath, PUSHGATE_GITLEAKS_EXIT_CODE: "1", PUSHGATE_GITLEAKS_REPORT: JSON.stringify([ @@ -607,6 +705,10 @@ async function writeGitleaksStub(repoRoot: string): Promise { "if (process.env.PUSHGATE_GITLEAKS_ARGS_OUT) {", " writeFileSync(process.env.PUSHGATE_GITLEAKS_ARGS_OUT, JSON.stringify(args));", "}", + "if (process.env.PUSHGATE_GITLEAKS_ENV_OUT) {", + " const gitEnv = Object.fromEntries(Object.entries(process.env).filter(([key]) => key.startsWith('GIT_')));", + " writeFileSync(process.env.PUSHGATE_GITLEAKS_ENV_OUT, JSON.stringify(gitEnv));", + "}", "const reportPath = args[args.indexOf('--report-path') + 1];", "if (reportPath && process.env.PUSHGATE_GITLEAKS_REPORT) {", " writeFileSync(reportPath, process.env.PUSHGATE_GITLEAKS_REPORT);", @@ -618,6 +720,62 @@ async function writeGitleaksStub(repoRoot: string): Promise { return scriptPath; } +async function writeNestedGitRecorder(repoRoot: string): Promise { + const scriptPath = join(repoRoot, "bin", "record-nested-git.mjs"); + + await mkdir(dirname(scriptPath), { recursive: true }); + await writeFile( + scriptPath, + [ + "import { execFileSync } from 'node:child_process';", + "import { writeFileSync } from 'node:fs';", + "const topLevel = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8' }).trim();", + "const gitEnv = Object.fromEntries(Object.entries(process.env).filter(([key]) => key.startsWith('GIT_')));", + "writeFileSync(process.env.PUSHGATE_NESTED_GIT_OUT, JSON.stringify({ gitEnv, topLevel }));", + ].join("\n"), + ); + await chmod(scriptPath, 0o755); + return scriptPath; +} + +async function initGitRepo(repoRoot: string): Promise { + await mkdir(repoRoot, { recursive: true }); + await checkedGit(repoRoot, ["init", "--quiet", "--initial-branch=main"]); +} + +async function checkedGit(repoRoot: string, args: string[]): Promise { + const result = await runGit(repoRoot, args, { + env: sanitizeGitLocalEnv(process.env), + }); + + if (result.code !== 0) { + throw new Error( + [ + `git ${args.join(" ")} exited with ${String(result.code)}.`, + `stdout:\n${result.stdout}`, + `stderr:\n${result.stderr}`, + ].join("\n"), + ); + } +} + +function poisonedGitEnv(victimRoot: string): NodeJS.ProcessEnv { + return { + GIT_COMMON_DIR: join(victimRoot, ".git"), + GIT_CONFIG: join(victimRoot, ".git", "config"), + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "core.bare", + GIT_CONFIG_VALUE_0: "true", + GIT_DIR: join(victimRoot, ".git"), + GIT_INDEX_FILE: join(victimRoot, ".git", "index"), + GIT_WORK_TREE: victimRoot, + }; +} + +function leakedGitLocalVars(env: Record): string[] { + return Object.keys(env).filter(isGitLocalEnvVar).sort(); +} + function captureOutput(): { stream: Writable; text(): string; diff --git a/test/git-environment.test.ts b/test/git-environment.test.ts new file mode 100644 index 0000000..111ab79 --- /dev/null +++ b/test/git-environment.test.ts @@ -0,0 +1,167 @@ +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { mkdtemp, realpath, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; + +import { runGit } from "../src/git/command.js"; +import { + isGitLocalEnvVar, + sanitizeGitLocalEnv, +} from "../src/git/environment.js"; + +test("sanitizeGitLocalEnv removes only Git local repository binding vars", () => { + const sanitized = sanitizeGitLocalEnv({ + CUSTOM_VALUE: "kept", + GIT_ASKPASS: "askpass", + GIT_COMMON_DIR: "/tmp/victim/.git", + GIT_CONFIG: "/tmp/gitconfig", + GIT_CONFIG_COUNT: "2", + GIT_CONFIG_KEY_0: "core.bare", + GIT_CONFIG_KEY_12: "core.worktree", + GIT_CONFIG_KEY_suffix: "kept", + GIT_CONFIG_PARAMETERS: "'core.bare'='true'", + GIT_CONFIG_VALUE_0: "true", + GIT_CONFIG_VALUE_12: "/tmp/victim", + GIT_CONFIG_VALUE_suffix: "kept", + GIT_DIR: "/tmp/victim/.git", + GIT_INDEX_FILE: "/tmp/victim/.git/index", + GIT_SSH_COMMAND: "ssh -i key", + GIT_TERMINAL_PROMPT: "0", + GIT_WORK_TREE: "/tmp/victim", + PATH: "/usr/bin", + }); + + assert.deepEqual(sanitized, { + CUSTOM_VALUE: "kept", + GIT_ASKPASS: "askpass", + GIT_CONFIG_KEY_suffix: "kept", + GIT_CONFIG_VALUE_suffix: "kept", + GIT_SSH_COMMAND: "ssh -i key", + GIT_TERMINAL_PROMPT: "0", + PATH: "/usr/bin", + }); +}); + +test("sanitizeGitLocalEnv can preserve Git config overlays for skip-control reads", () => { + const sanitized = sanitizeGitLocalEnv( + { + GIT_CONFIG: "/tmp/gitconfig", + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "pushgate.skip-ai-check", + GIT_CONFIG_PARAMETERS: "'pushgate.skip-ai-check'='true'", + GIT_CONFIG_VALUE_0: "true", + GIT_DIR: "/tmp/victim/.git", + GIT_WORK_TREE: "/tmp/victim", + }, + { + preserveGitConfigOverlay: true, + }, + ); + + assert.deepEqual(sanitized, { + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "pushgate.skip-ai-check", + GIT_CONFIG_PARAMETERS: "'pushgate.skip-ai-check'='true'", + GIT_CONFIG_VALUE_0: "true", + }); +}); + +test("isGitLocalEnvVar recognizes static and indexed Git config vars", () => { + assert.equal(isGitLocalEnvVar("GIT_DIR"), true); + assert.equal(isGitLocalEnvVar("GIT_CONFIG_VALUE_0"), true); + assert.equal(isGitLocalEnvVar("GIT_CONFIG_KEY_42"), true); + assert.equal(isGitLocalEnvVar("GIT_SSH_COMMAND"), false); + assert.equal(isGitLocalEnvVar("GIT_CONFIG_KEY_suffix"), false); +}); + +test("runGit ignores inherited hook-local Git env for explicit repo roots", async () => { + await withTempDir("pushgate-git-env-", async (tempRoot) => { + const victimRoot = join(tempRoot, "victim"); + const targetRoot = join(tempRoot, "target"); + + await initRepo(victimRoot); + await initRepo(targetRoot); + + const result = await runGit(targetRoot, ["rev-parse", "--show-toplevel"], { + env: { + ...sanitizeGitLocalEnv(process.env), + ...poisonedGitEnv(victimRoot), + }, + }); + + assert.equal(result.code, 0, result.stderr); + assert.equal( + await realpath(result.stdout.trim()), + await realpath(targetRoot), + ); + }); +}); + +function poisonedGitEnv(victimRoot: string): NodeJS.ProcessEnv { + return { + GIT_COMMON_DIR: join(victimRoot, ".git"), + GIT_CONFIG_COUNT: "1", + GIT_CONFIG_KEY_0: "core.bare", + GIT_CONFIG_VALUE_0: "true", + GIT_DIR: join(victimRoot, ".git"), + GIT_INDEX_FILE: join(victimRoot, ".git", "index"), + GIT_WORK_TREE: victimRoot, + }; +} + +async function initRepo(repoRoot: string): Promise { + await checkedRun("git", ["init", "--quiet", "--initial-branch=main", repoRoot]); +} + +async function withTempDir( + prefix: string, + callback: (tempRoot: string) => Promise, +): Promise { + const tempRoot = await mkdtemp(join(tmpdir(), prefix)); + + try { + await callback(tempRoot); + } finally { + await rm(tempRoot, { force: true, recursive: true }); + } +} + +async function checkedRun(command: string, args: string[]): Promise { + const result = await new Promise<{ + code: number | null; + stderr: string; + stdout: string; + }>((resolve, reject) => { + const child = spawn(command, args, { + env: sanitizeGitLocalEnv(process.env), + stdio: ["ignore", "pipe", "pipe"], + }); + let stderr = ""; + let stdout = ""; + + child.stdout?.setEncoding("utf8"); + child.stderr?.setEncoding("utf8"); + child.stdout?.on("data", (data: string) => { + stdout += data; + }); + child.stderr?.on("data", (data: string) => { + stderr += data; + }); + child.on("error", reject); + child.on("close", (code) => { + resolve({ code, stderr, stdout }); + }); + }); + + if (result.code !== 0) { + throw new Error( + [ + `${command} ${args.join(" ")} exited with ${String(result.code)}.`, + `stdout:\n${result.stdout}`, + `stderr:\n${result.stderr}`, + ].join("\n"), + ); + } +} diff --git a/test/install.test.ts b/test/install.test.ts index 45b893a..2988ecd 100644 --- a/test/install.test.ts +++ b/test/install.test.ts @@ -15,6 +15,8 @@ import { delimiter, dirname, join } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; +import { sanitizeGitLocalEnv } from "../src/git/environment.js"; + const installerPath = fileURLToPath(new URL("../install.sh", import.meta.url)); const hookSourcePath = fileURLToPath( new URL("../hook/pre-push", import.meta.url), @@ -158,7 +160,7 @@ async function createInstallerHarness(): Promise { await installExecutable(binDir, "curl", curlStub); const env = { - ...process.env, + ...sanitizeGitLocalEnv(process.env), GIT_CONFIG_NOSYSTEM: "1", GIT_TERMINAL_PROMPT: "0", HOME: homeDir, @@ -235,7 +237,7 @@ function runCommand( return new Promise((resolve, reject) => { const child = spawn(command, args, { cwd: options.cwd, - env: options.env, + env: command === "git" ? sanitizeGitLocalEnv(options.env) : options.env, stdio: ["ignore", "pipe", "pipe"], }); let stderr = ""; diff --git a/test/path-policy.test.ts b/test/path-policy.test.ts index c5ba5ac..8e8f09e 100644 --- a/test/path-policy.test.ts +++ b/test/path-policy.test.ts @@ -10,6 +10,7 @@ import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import test from "node:test"; +import { sanitizeGitLocalEnv } from "../src/git/environment.js"; import { GitChangedFilesError, MissingDiffBaseError, @@ -239,6 +240,7 @@ function runGit(repoRoot: string, args: string[]): Promise { return new Promise((resolve, reject) => { const child = spawn("git", args, { cwd: repoRoot, + env: sanitizeGitLocalEnv(process.env), stdio: ["ignore", "pipe", "pipe"], }); let stderr = ""; diff --git a/test/runner.test.ts b/test/runner.test.ts index 25c507b..c2174ab 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -7,6 +7,7 @@ import { Readable, Writable } from "node:stream"; import test from "node:test"; import { fileURLToPath } from "node:url"; +import { sanitizeGitLocalEnv } from "../src/git/environment.js"; import { runPrePushWorkflow } from "../src/workflows/pre-push.js"; import type { WarningConfirmationRequest } from "../src/workflows/warning-confirmation.js"; @@ -540,7 +541,7 @@ function runRunner( return new Promise((resolve, reject) => { const child = spawn(process.execPath, [runnerSourcePath, ...args], { cwd: options.cwd, - env: { ...process.env, ...options.env }, + env: { ...sanitizeGitLocalEnv(process.env), ...options.env }, stdio: [stdin === undefined ? "ignore" : "pipe", "pipe", "pipe"], }); let stderr = ""; @@ -592,7 +593,7 @@ async function runWorkflowInRepo( try { const code = await runPrePushWorkflow({ - env: { ...process.env, ...options.env }, + env: { ...sanitizeGitLocalEnv(process.env), ...options.env }, stderr: stderr.stream, stdin: Readable.from(""), stdout: stdout.stream, @@ -809,7 +810,7 @@ async function withGitleaksRepo( await installGitleaksStub(binDir); await callback(repoRoot, { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }); } finally { @@ -862,7 +863,7 @@ async function withAiRepo( await installClaudeStub(binDir); await callback(repoRoot, { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), }); } finally { @@ -980,6 +981,7 @@ async function checkedRun( const result = await new Promise((resolve, reject) => { const child = spawn(command, args, { cwd: options.cwd, + env: sanitizeGitLocalEnv(process.env), stdio: ["ignore", "pipe", "pipe"], }); let stderr = ""; @@ -1031,7 +1033,7 @@ async function withGitStub( await callback({ argsPath, env: { - ...process.env, + ...sanitizeGitLocalEnv(process.env), PATH: [binDir, process.env.PATH ?? ""].join(delimiter), PUSHGATE_GIT_ARGS_OUT: argsPath, PUSHGATE_GIT_EXIT: "23", diff --git a/test/support/hook-harness.ts b/test/support/hook-harness.ts index fc6112a..d82d90c 100644 --- a/test/support/hook-harness.ts +++ b/test/support/hook-harness.ts @@ -12,6 +12,8 @@ import { tmpdir } from "node:os"; import { delimiter, dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; +import { sanitizeGitLocalEnv } from "../../src/git/environment.js"; + /** Captured process result returned to harness tests instead of throwing. */ export interface CommandResult { /** Exit code from the child process, or `null` when a signal ended it. */ @@ -308,7 +310,7 @@ function createSandboxEnv( binDir: string, ): NodeJS.ProcessEnv { return { - ...process.env, + ...sanitizeGitLocalEnv(process.env), GIT_CONFIG_NOSYSTEM: "1", GIT_TERMINAL_PROMPT: "0", HOME: homeDir, @@ -360,7 +362,7 @@ function runCommand( return new Promise((resolve, reject) => { const child = spawn(command, args, { cwd: options.cwd, - env: options.env, + env: command === "git" ? sanitizeGitLocalEnv(options.env) : options.env, stdio: [stdinMode, "pipe", "pipe"], }); let stderr = ""; From f52bd4bc04231a8e440539b625296a16da7b4ed3 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 11:36:43 -0300 Subject: [PATCH 2/3] add ts docs --- docs/fix-git-hook-env-contamination.md | 34 -------------------------- src/git/environment.ts | 13 ++++++++++ 2 files changed, 13 insertions(+), 34 deletions(-) delete mode 100644 docs/fix-git-hook-env-contamination.md diff --git a/docs/fix-git-hook-env-contamination.md b/docs/fix-git-hook-env-contamination.md deleted file mode 100644 index 3d8e25f..0000000 --- a/docs/fix-git-hook-env-contamination.md +++ /dev/null @@ -1,34 +0,0 @@ -# Fix Git Hook Environment Contamination - -## Summary - -Pushgate must not forward Git hook-local repository binding variables into subprocesses that run outside the hook's own Git push operation. When variables such as `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, or `GIT_COMMON_DIR` leak into configured tools, plugins, providers, or repo-targeted Git helpers, nested Git commands can operate on the repository being pushed instead of the subprocess `cwd`. - -This fix is independent from CLI logging work. It is a safety and correctness change with no config schema changes. - -## Implementation - -- Add `sanitizeGitLocalEnv(env)` in `src/git/environment.ts`. -- Strip variables reported by `git rev-parse --local-env-vars`, including `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, `GIT_COMMON_DIR`, `GIT_CONFIG`, and `GIT_CONFIG_COUNT`. -- Strip dynamic `GIT_CONFIG_KEY_` and `GIT_CONFIG_VALUE_` entries. -- Preserve transport and auth variables such as `GIT_SSH_COMMAND`, `GIT_ASKPASS`, and `GIT_TERMINAL_PROMPT`. -- Sanitize external Pushgate subprocesses: - - configured deterministic tools - - Gitleaks plugin command - - AI provider CLI commands -- Sanitize repo-targeted internal Git helpers through the shared `runGit` wrapper. -- Leave the native `git push` wrapper unsanitized because it intentionally runs Git's push operation in the caller's context. -- Keep Git-spawning test helpers safe when tests are launched from a Git hook. - -## Test Plan - -- Unit-test `sanitizeGitLocalEnv`. -- Regression-test a configured tool running nested Git with a poisoned parent Git environment. -- Verify Gitleaks and AI provider command stubs do not receive hook-local Git repository variables. -- Run `pnpm run typecheck`. -- Run `pnpm test`. -- Run `pnpm run check:shell`. - -## Manual Acceptance - -After the fix, running `git push` in `ai-pushgate` may pass or fail checks normally, but Pushgate subprocesses must not create `baseline` commits, switch or create `feature`, delete fixture paths, or leave index locks in the real repository because of inherited hook-local Git variables. diff --git a/src/git/environment.ts b/src/git/environment.ts index 17083a2..367752c 100644 --- a/src/git/environment.ts +++ b/src/git/environment.ts @@ -19,9 +19,21 @@ const GIT_LOCAL_ENV_VARS = new Set([ const GIT_CONFIG_PAIR_ENV_VAR = /^GIT_CONFIG_(?:KEY|VALUE)_\d+$/; export interface SanitizeGitLocalEnvOptions { + /** + * Keep `git -c` config passed through Git's environment protocol. + * Use only when intentionally reading caller-supplied Git config overlays. + */ preserveGitConfigOverlay?: boolean; } +/** + * Removes Git hook-local repository bindings from an environment copy. + * + * Git hooks can run with `GIT_DIR`, `GIT_WORK_TREE`, `GIT_INDEX_FILE`, and + * related variables pointing at the repository being pushed. If Pushgate passes + * those variables into tools, plugins, providers, or explicit-`cwd` Git helpers, + * nested Git commands may operate on the hook repo instead of their own cwd. + */ export function sanitizeGitLocalEnv( env: NodeJS.ProcessEnv, options: SanitizeGitLocalEnvOptions = {}, @@ -41,6 +53,7 @@ export function sanitizeGitLocalEnv( return sanitized; } +/** Returns true for repository-local Git environment variables. */ export function isGitLocalEnvVar(key: string): boolean { return GIT_LOCAL_ENV_VARS.has(key) || GIT_CONFIG_PAIR_ENV_VAR.test(key); } From 0f1a3469d3f91002d22535ba198504cc5f1944c1 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 11:39:34 -0300 Subject: [PATCH 3/3] minor change --- src/git/environment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/git/environment.ts b/src/git/environment.ts index 367752c..8d15cba 100644 --- a/src/git/environment.ts +++ b/src/git/environment.ts @@ -53,7 +53,7 @@ export function sanitizeGitLocalEnv( return sanitized; } -/** Returns true for repository-local Git environment variables. */ +/** Returns `true` for repository-local Git environment variables. */ export function isGitLocalEnvVar(key: string): boolean { return GIT_LOCAL_ENV_VARS.has(key) || GIT_CONFIG_PAIR_ENV_VAR.test(key); }