diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7f047c7a..533e72869 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,6 +122,7 @@ jobs: npx vitest run src/utils/executable.test.ts src/utils/spawn.launch-regression.test.ts + src/utils/spawn.percent-escape.test.ts src/utils/spawn.test.ts src/utils/run-git-command.test.ts src/utils/checkout-git-rev-parse.test.ts diff --git a/packages/server/src/utils/executable.test.ts b/packages/server/src/utils/executable.test.ts index 9a6d1e0fb..9464d3d44 100644 --- a/packages/server/src/utils/executable.test.ts +++ b/packages/server/src/utils/executable.test.ts @@ -207,9 +207,18 @@ describe("quoteWindowsCommand", () => { expect(quoteWindowsCommand("feature|bugfix")).toBe("feature^|bugfix"); }); - test("doubles percent signs", () => { + test("does not double percent signs", () => { setPlatform("win32"); - expect(quoteWindowsCommand("100%")).toBe("100%%"); + // cmd.exe only collapses %% → % inside batch files; on the command line + // it stays literal, which corrupts git --format atoms etc. + expect(quoteWindowsCommand("100%")).toBe("100%"); + }); + + test("preserves git --format atoms verbatim", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("--format=%(refname)%09%(committerdate:unix)")).toBe( + "--format=%^(refname^)%09%^(committerdate:unix^)", + ); }); test("escapes carets", () => { @@ -227,7 +236,7 @@ describe("quoteWindowsCommand", () => { test("quotes commands with spaces after escaping metacharacters", () => { setPlatform("win32"); expect(quoteWindowsCommand("C:\\Program Files\\My Tool&Stuff\\run 100%.cmd")).toBe( - '"C:\\Program Files\\My Tool^&Stuff\\run 100%%.cmd"', + '"C:\\Program Files\\My Tool^&Stuff\\run 100%.cmd"', ); }); diff --git a/packages/server/src/utils/spawn.percent-escape.test.ts b/packages/server/src/utils/spawn.percent-escape.test.ts new file mode 100644 index 000000000..3f9eb20ee --- /dev/null +++ b/packages/server/src/utils/spawn.percent-escape.test.ts @@ -0,0 +1,49 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, test } from "vitest"; + +import { execCommand } from "./spawn.js"; + +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +function writeEchoArgScript(): string { + const dir = mkdtempSync(path.join(tmpdir(), "paseo-percent-escape-")); + tempDirs.push(dir); + const scriptPath = path.join(dir, "echo-arg.js"); + writeFileSync(scriptPath, "process.stdout.write(process.argv[2] ?? '');\n"); + return scriptPath; +} + +// Use the bare command name "node" (no extension, no path separator) so +// `shouldUseWindowsShell` resolves to true on Windows. That routes the spawn +// through cmd.exe and exercises `quoteWindowsArgument`, which is where the +// %-doubling bug lives. Using `process.execPath` here would skip the shell +// path entirely and silently pass the test. +const COMMAND = "node"; + +describe("spawn argument escaping for % characters", () => { + test("delivers a git for-each-ref --format atom to the child verbatim", async () => { + const scriptPath = writeEchoArgScript(); + const formatArg = "--format=%(refname)%09%(committerdate:unix)"; + + const { stdout } = await execCommand(COMMAND, [scriptPath, formatArg]); + + expect(stdout).toBe(formatArg); + }); + + test("delivers a bare percent-prefixed token to the child verbatim", async () => { + const scriptPath = writeEchoArgScript(); + const arg = "%(refname)"; + + const { stdout } = await execCommand(COMMAND, [scriptPath, arg]); + + expect(stdout).toBe(arg); + }); +}); diff --git a/packages/server/src/utils/windows-command.ts b/packages/server/src/utils/windows-command.ts index 605742ce3..44b9362ad 100644 --- a/packages/server/src/utils/windows-command.ts +++ b/packages/server/src/utils/windows-command.ts @@ -10,7 +10,11 @@ function escapeWindowsCmdValue(value: string): string { const isQuoted = value.startsWith('"') && value.endsWith('"'); const unquoted = isQuoted ? value.slice(1, -1) : value; - const escaped = unquoted.replace(/%/g, "%%").replace(/([&|^<>()!])/g, "^$1"); + // Do NOT double `%` here. cmd.exe only collapses `%%` → `%` inside batch + // files; on the command line / `cmd /c "..."` `%%` stays literal, which + // breaks args like git's `--format=%(refname)` (git treats `%%` as the + // escape for a literal `%`, so the format atoms become literals). + const escaped = unquoted.replace(/([&|^<>()!])/g, "^$1"); if (isQuoted || /[\s"]/u.test(unquoted)) { const quoted = escaped