From 130e95d03a04a743372465401c52150b6370a969 Mon Sep 17 00:00:00 2001 From: ppcvote Date: Thu, 13 Aug 2026 16:53:00 +0800 Subject: [PATCH] fix: pass gitleaks --log-opts without a shell so Windows keeps the caret runCommand spawned with shell:true, and the range argument was written with POSIX single quotes. With shell:true Node joins command and args into one command line for the platform shell, and cmd.exe neither treats ' as a quote nor leaves ^ alone. Measured through the same code path on Windows: intended, one entry: ["--log-opts='--first-parent abc123^..def456'"] shell:true actual: ["--log-opts='--first-parent", "abc123..def456'"] no shell: ["--log-opts='--first-parent abc123^..def456'"] The argument is split at the space, and the caret is deleted. The caret is the part that matters. ^.. starts at the parent of commitFrom; .. starts at commitFrom. Losing it narrows the range by one commit, and that commit is the first of the push. gitleaks then exits 0 having skipped a commit that was in scope, so on Windows the scan silently covered less than it reported. Dropping shell:true makes the argv identical on win32 and linux, at which point the embedded quotes are unnecessary and are removed with it. runCommand is only called with git and gitleaks, both native executables; spawn without a shell resolves both through PATH on Windows, verified locally. The test reads spawn.mock.calls[1] and asserts the exact argv entry, that no argument contains a quote, and that shell is not true. It fails on the unmodified tree and passes with the change. resolves #1675 --- src/proxy/processors/push-action/gitleaks.ts | 7 ++-- test/processors/gitLeaks.test.ts | 34 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/proxy/processors/push-action/gitleaks.ts b/src/proxy/processors/push-action/gitleaks.ts index abbc582b6..38dc0bb4a 100644 --- a/src/proxy/processors/push-action/gitleaks.ts +++ b/src/proxy/processors/push-action/gitleaks.ts @@ -34,7 +34,10 @@ function runCommand( stderr: string; }> { return new Promise((resolve, reject) => { - const child = spawn(command, args, { cwd, shell: true }); + // No shell. With shell:true Node hands the whole command line to the + // platform shell, and cmd.exe neither treats ' as a quote nor leaves ^ + // alone, so any argument carrying a space or a git ^ arrives corrupted. + const child = spawn(command, args, { cwd }); let stdout = ''; let stderr = ''; @@ -171,7 +174,7 @@ const exec = async (_req: Request, action: Action): Promise => { 'git', // not using --no-merges to be sure we're scanning the diff // only add ^ if the commitFrom isn't the repo's rootCommit - `--log-opts='--first-parent ${rootCommit === commitFrom ? rootCommit : `${commitFrom}^`}..${commitTo}'`, + `--log-opts=--first-parent ${rootCommit === commitFrom ? rootCommit : `${commitFrom}^`}..${commitTo}`, ].filter((v) => typeof v === 'string'); const gitleaks = await runCommand(workingDir, 'gitleaks', gitleaksArgs); diff --git a/test/processors/gitLeaks.test.ts b/test/processors/gitLeaks.test.ts index a5a24b103..d4bba2347 100644 --- a/test/processors/gitLeaks.test.ts +++ b/test/processors/gitLeaks.test.ts @@ -157,6 +157,40 @@ describe('gitleaks', () => { expect(result.steps[0].logs[2]).toContain('gitleaks - Gitleaks output: No leaks found'); }); + it('should pass --log-opts as one argument, unquoted and without a shell', async () => { + // With shell:true, Node joins command and args into a single command line + // for the platform shell. cmd.exe does not treat ' as a quote and does + // treat ^ as an escape, so the POSIX-quoted form arrived as + // ["--log-opts='--first-parent", "abc123..def456'"] + // on Windows: split in two, and the ^ silently dropped. Dropping the ^ + // narrows the revision range by one commit, so the first commit of the + // push goes unscanned. + vi.mocked(getAPIs).mockReturnValue({ gitleaks: { enabled: true } }); + + const mockChild = (exitCode: number, stdout: string, stderr: string) => ({ + on: (event: string, cb: (exitCode: number) => void) => { + if (event === 'close') cb(exitCode); + return { stdout: { on: () => {} }, stderr: { on: () => {} } }; + }, + stdout: { on: (_: string, cb: (out: string) => void) => cb(stdout) }, + stderr: { on: (_: string, cb: (err: string) => void) => cb(stderr) }, + }); + + vi.mocked(spawn) + .mockReturnValueOnce(mockChild(0, 'rootcommit123', '')) + .mockReturnValueOnce(mockChild(0, '', 'No leaks found')); + + await exec(req, action); + + // call 0 is `git rev-list`, call 1 is `gitleaks` + const [command, args, opts] = vi.mocked(spawn).mock.calls[1]; + + expect(command).toBe('gitleaks'); + expect(args).toContain('--log-opts=--first-parent abc123^..def456'); + expect((args as string[]).some((arg) => arg.includes("'"))).toBe(false); + expect((opts as { shell?: boolean })?.shell).not.toBe(true); + }); + it('should handle scan with findings', async () => { vi.mocked(getAPIs).mockReturnValue({ gitleaks: { enabled: true } });