Skip to content

Commit feb9806

Browse files
committed
Fix hook command file URI handling
1 parent 97a11c7 commit feb9806

2 files changed

Lines changed: 92 additions & 1 deletion

File tree

src/platform/chat/node/hookExecutor.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { CancellationToken, ChatHookCommand, Uri } from 'vscode';
99
import { basename, join } from '../../../util/vs/base/common/path';
1010
import { isWindows } from '../../../util/vs/base/common/platform';
1111
import { removeAnsiEscapeCodes } from '../../../util/vs/base/common/strings';
12+
import { URI } from '../../../util/vs/base/common/uri';
1213
import { ILogService } from '../../log/common/logService';
1314
import { HookCommandResultKind, IHookCommandResult, IHookExecutor } from '../common/hookExecutor';
1415
import { IHooksOutputChannel } from '../common/hooksOutputChannel';
@@ -48,8 +49,9 @@ export class NodeHookExecutor implements IHookExecutor {
4849

4950
private _spawn(hook: ChatHookCommand, input: unknown, token: CancellationToken): Promise<IHookCommandResult> {
5051
const cwd = hook.cwd ? uriToFsPath(hook.cwd) : homedir();
52+
const command = sanitizeHookCommand(hook.command);
5153

52-
const child = spawn(hook.command, [], {
54+
const child = spawn(command, [], {
5355
stdio: 'pipe',
5456
cwd,
5557
env: { ...process.env, ...hook.env },
@@ -177,6 +179,41 @@ function uriToFsPath(uri: Uri): string {
177179
return (uri as { path: string }).path;
178180
}
179181

182+
const fileUriTokenRegex = /(^|[\s=:(\[{,;])(["']?)(file:\/\/[^\s"'`|&;()<>\]}]+)\2(?=$|[\s)\]};,|&<>])/g;
183+
184+
function sanitizeHookCommand(command: string): string {
185+
return command.replace(fileUriTokenRegex, (match, prefix: string, quote: string, uriText: string) => {
186+
const fsPath = getFileUriFsPath(uriText);
187+
if (!fsPath) {
188+
return match;
189+
}
190+
191+
const sanitizedPath = quote ? fsPath : quoteArgForShell(fsPath);
192+
return `${prefix}${quote}${sanitizedPath}${quote}`;
193+
});
194+
}
195+
196+
function getFileUriFsPath(uriText: string): string | undefined {
197+
try {
198+
const parsed = URI.parse(uriText);
199+
if (parsed.scheme !== 'file') {
200+
return undefined;
201+
}
202+
203+
return parsed.fsPath;
204+
} catch {
205+
return undefined;
206+
}
207+
}
208+
209+
function quoteArgForShell(arg: string): string {
210+
if (!(/[\s"'$`\\|&;()<>]/.test(arg))) {
211+
return arg;
212+
}
213+
214+
return `"${arg.replace(/["\\]/g, '\\$&')}"`;
215+
}
216+
180217

181218
function getShell(): string | true {
182219
if (!isWindows) {

src/platform/chat/test/node/hookExecutor.spec.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { spawn } from 'child_process';
67
import { EventEmitter } from 'events';
78
import { beforeEach, describe, expect, test, vi } from 'vitest';
89
import type { ChatHookCommand } from 'vscode';
@@ -36,6 +37,11 @@ function createMockChild(): MockChildProcess {
3637
return child;
3738
}
3839

40+
function getSpawnCommand(): string {
41+
const calls = vi.mocked(spawn).mock.calls;
42+
return String(calls.at(-1)?.[0]);
43+
}
44+
3945
/**
4046
* Simulates a child process completing with the given stdout, stderr, and exit code.
4147
*/
@@ -169,6 +175,54 @@ describe('NodeHookExecutor', () => {
169175
expect(result.kind).toBe(HookCommandResultKind.Success);
170176
});
171177

178+
test('sanitizes file URIs in hook commands before spawn', async () => {
179+
const promise = executor.executeCommand(
180+
cmd('cat file:///tmp/example.txt'),
181+
undefined,
182+
CancellationToken.None
183+
);
184+
completeChild(child, { stdout: 'ok', exitCode: 0 });
185+
await promise;
186+
187+
expect(getSpawnCommand()).toBe('cat /tmp/example.txt');
188+
});
189+
190+
test('quotes sanitized file URIs with spaces in hook commands', async () => {
191+
const promise = executor.executeCommand(
192+
cmd('cat file:///tmp/my%20file.txt'),
193+
undefined,
194+
CancellationToken.None
195+
);
196+
completeChild(child, { stdout: 'ok', exitCode: 0 });
197+
await promise;
198+
199+
expect(getSpawnCommand()).toBe('cat "/tmp/my file.txt"');
200+
});
201+
202+
test('preserves existing quotes around sanitized file URIs', async () => {
203+
const promise = executor.executeCommand(
204+
cmd('cat "file:///tmp/my%20file.txt"'),
205+
undefined,
206+
CancellationToken.None
207+
);
208+
completeChild(child, { stdout: 'ok', exitCode: 0 });
209+
await promise;
210+
211+
expect(getSpawnCommand()).toBe('cat "/tmp/my file.txt"');
212+
});
213+
214+
test('does not sanitize non-file URIs in hook commands', async () => {
215+
const promise = executor.executeCommand(
216+
cmd('cat vscode-remote://ssh-remote+test/workspace/file.txt'),
217+
undefined,
218+
CancellationToken.None
219+
);
220+
completeChild(child, { stdout: 'ok', exitCode: 0 });
221+
await promise;
222+
223+
expect(getSpawnCommand()).toBe('cat vscode-remote://ssh-remote+test/workspace/file.txt');
224+
});
225+
172226
test('handles spawn error as non-blocking error', async () => {
173227
const promise = executor.executeCommand(cmd('badcmd'), undefined, CancellationToken.None);
174228
child.emit('error', new Error('spawn ENOENT'));

0 commit comments

Comments
 (0)