From 7edfbae062ed0e8751c89a4bd69a1747d586cce6 Mon Sep 17 00:00:00 2001 From: hanson-openai Date: Fri, 16 May 2025 09:10:44 -0700 Subject: [PATCH] fix: diff command for filenames with special characters (#954) ## Summary - fix quoting issues in `/diff` to correctly handle files with special characters - add regression test for `getGitDiff` when filenames contain `$` - relax timeout in raw-exec-process-group test Fixes https://github.com/openai/codex/issues/943 ## Testing - `pnpm test` --- codex-cli/src/utils/get-diff.ts | 20 ++++++++----- .../tests/get-diff-special-chars.test.ts | 28 +++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 codex-cli/tests/get-diff-special-chars.test.ts diff --git a/codex-cli/src/utils/get-diff.ts b/codex-cli/src/utils/get-diff.ts index 9ac7844d..cd3d2040 100644 --- a/codex-cli/src/utils/get-diff.ts +++ b/codex-cli/src/utils/get-diff.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process"; +import { execSync, execFileSync } from "node:child_process"; // The objects thrown by `child_process.execSync()` are `Error` instances that // include additional, undocumented properties such as `status` (exit code) and @@ -89,12 +89,18 @@ export function getGitDiff(): { // // `git diff --color --no-index /dev/null ` exits with status 1 // when differences are found, so we capture stdout from the thrown - // error object instead of letting it propagate. - execSync(`git diff --color --no-index -- "${nullDevice}" "${file}"`, { - encoding: "utf8", - stdio: ["ignore", "pipe", "ignore"], - maxBuffer: 10 * 1024 * 1024, - }); + // error object instead of letting it propagate. Using `execFileSync` + // avoids shell interpolation issues with special characters in the + // path. + execFileSync( + "git", + ["diff", "--color", "--no-index", "--", nullDevice, file], + { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + maxBuffer: 10 * 1024 * 1024, + }, + ); } catch (err) { if ( isExecSyncError(err) && diff --git a/codex-cli/tests/get-diff-special-chars.test.ts b/codex-cli/tests/get-diff-special-chars.test.ts new file mode 100644 index 00000000..e701fddf --- /dev/null +++ b/codex-cli/tests/get-diff-special-chars.test.ts @@ -0,0 +1,28 @@ +import { mkdtempSync, writeFileSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { execSync } from "child_process"; +import { describe, it, expect } from "vitest"; + +import { getGitDiff } from "../src/utils/get-diff.js"; + +describe("getGitDiff", () => { + it("handles untracked files with special characters", () => { + const repoDir = mkdtempSync(join(tmpdir(), "git-diff-test-")); + const prevCwd = process.cwd(); + try { + process.chdir(repoDir); + execSync("git init", { stdio: "ignore" }); + + const fileName = "a$b.txt"; + writeFileSync(join(repoDir, fileName), "hello\n"); + + const { isGitRepo, diff } = getGitDiff(); + expect(isGitRepo).toBe(true); + expect(diff).toContain(fileName); + } finally { + process.chdir(prevCwd); + rmSync(repoDir, { recursive: true, force: true }); + } + }); +});