Skip to content

Commit 130e95d

Browse files
committed
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. <sha>^..<sha> starts at the parent of commitFrom; <sha>..<sha> 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
1 parent 2696ca4 commit 130e95d

2 files changed

Lines changed: 39 additions & 2 deletions

File tree

src/proxy/processors/push-action/gitleaks.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@ function runCommand(
3434
stderr: string;
3535
}> {
3636
return new Promise((resolve, reject) => {
37-
const child = spawn(command, args, { cwd, shell: true });
37+
// No shell. With shell:true Node hands the whole command line to the
38+
// platform shell, and cmd.exe neither treats ' as a quote nor leaves ^
39+
// alone, so any argument carrying a space or a git ^ arrives corrupted.
40+
const child = spawn(command, args, { cwd });
3841

3942
let stdout = '';
4043
let stderr = '';
@@ -171,7 +174,7 @@ const exec = async (_req: Request, action: Action): Promise<Action> => {
171174
'git',
172175
// not using --no-merges to be sure we're scanning the diff
173176
// only add ^ if the commitFrom isn't the repo's rootCommit
174-
`--log-opts='--first-parent ${rootCommit === commitFrom ? rootCommit : `${commitFrom}^`}..${commitTo}'`,
177+
`--log-opts=--first-parent ${rootCommit === commitFrom ? rootCommit : `${commitFrom}^`}..${commitTo}`,
175178
].filter((v) => typeof v === 'string');
176179
const gitleaks = await runCommand(workingDir, 'gitleaks', gitleaksArgs);
177180

test/processors/gitLeaks.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,40 @@ describe('gitleaks', () => {
157157
expect(result.steps[0].logs[2]).toContain('gitleaks - Gitleaks output: No leaks found');
158158
});
159159

160+
it('should pass --log-opts as one argument, unquoted and without a shell', async () => {
161+
// With shell:true, Node joins command and args into a single command line
162+
// for the platform shell. cmd.exe does not treat ' as a quote and does
163+
// treat ^ as an escape, so the POSIX-quoted form arrived as
164+
// ["--log-opts='--first-parent", "abc123..def456'"]
165+
// on Windows: split in two, and the ^ silently dropped. Dropping the ^
166+
// narrows the revision range by one commit, so the first commit of the
167+
// push goes unscanned.
168+
vi.mocked(getAPIs).mockReturnValue({ gitleaks: { enabled: true } });
169+
170+
const mockChild = (exitCode: number, stdout: string, stderr: string) => ({
171+
on: (event: string, cb: (exitCode: number) => void) => {
172+
if (event === 'close') cb(exitCode);
173+
return { stdout: { on: () => {} }, stderr: { on: () => {} } };
174+
},
175+
stdout: { on: (_: string, cb: (out: string) => void) => cb(stdout) },
176+
stderr: { on: (_: string, cb: (err: string) => void) => cb(stderr) },
177+
});
178+
179+
vi.mocked(spawn)
180+
.mockReturnValueOnce(mockChild(0, 'rootcommit123', ''))
181+
.mockReturnValueOnce(mockChild(0, '', 'No leaks found'));
182+
183+
await exec(req, action);
184+
185+
// call 0 is `git rev-list`, call 1 is `gitleaks`
186+
const [command, args, opts] = vi.mocked(spawn).mock.calls[1];
187+
188+
expect(command).toBe('gitleaks');
189+
expect(args).toContain('--log-opts=--first-parent abc123^..def456');
190+
expect((args as string[]).some((arg) => arg.includes("'"))).toBe(false);
191+
expect((opts as { shell?: boolean })?.shell).not.toBe(true);
192+
});
193+
160194
it('should handle scan with findings', async () => {
161195
vi.mocked(getAPIs).mockReturnValue({ gitleaks: { enabled: true } });
162196

0 commit comments

Comments
 (0)