debug: encode Windows batch adapter arguments (#332664)

* debug: encode Windows batch adapter arguments Updates Windows batch adapter process startup to construct the cmd.exe command line explicitly. - Encodes batch file paths and arguments with Windows command-line rules. - Configures cmd.exe argument parsing explicitly. - Rejects argument values that cmd.exe cannot represent. - Adds unit tests for quoting and invalid argument handling. (Commit message generated by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * debug: test batch argument round trips Adds a Windows-only process test for batch adapter argument handling. - Invokes a temporary batch adapter through cmd.exe. - Verifies that each argument is preserved. - Confirms that command metacharacters remain part of the argument. (Commit message generated by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * debug: capture batch parameters directly Updates the Windows batch round-trip test to record positional values before it launches the capture process. - Stores each batch parameter in an inherited environment value. - Reads the captured values without forwarding the original command line. (Commit message generated by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * debug: decode captured batch parameters Updates the Windows batch round-trip test to forward each positional parameter explicitly through the native argument parser. - Preserves empty positional parameters during capture. - Decodes quoted values and terminal backslashes before comparison. (Commit message generated by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * debug: scope batch round-trip assertions Updates the Windows process test to distinguish values that can be round-tripped from quote-bearing values that cmd.exe reparses. - Compares representable argument values exactly. - Verifies that a quote-bearing value does not create the marker file. (Commit message generated by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-09-09 10:42:28 +01:00 · 2026-08-26 05:26:52 +00:00
parent 39ef71161d
commit d2eb83323f
2 changed files with 191 additions and 8 deletions
@@ -17,6 +17,73 @@ import { IDebugAdapterExecutable, IDebugAdapterNamedPipeServer, IDebugAdapterSer
 import { AbstractDebugAdapter } from '../common/abstractDebugAdapter.js';
 import { killTree } from '../../../../base/node/processes.js';

+const windowsBatchUnquotedCharacters = '#$*+-./:?@\\_';
+const windowsBatchInvalidCharacters = /[\0\r\n]/;
+const windowsBatchControlCharacter = /\p{Cc}/u;
+
+function windowsBatchArgumentNeedsQuotes(argument: string): boolean {
+	if (!argument || argument.endsWith('\\')) {
+		return true;
+	}
+
+	for (const character of argument) {
+		const codePoint = character.codePointAt(0)!;
+		const isAsciiAlphaNumeric = codePoint >= 0x30 && codePoint <= 0x39
+			|| codePoint >= 0x41 && codePoint <= 0x5A
+			|| codePoint >= 0x61 && codePoint <= 0x7A;
+		if (codePoint <= 0x7F && !isAsciiAlphaNumeric && !windowsBatchUnquotedCharacters.includes(character)
+			|| windowsBatchControlCharacter.test(character)) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
+function escapeWindowsBatchArgument(argument: string, forceQuotes = false): string {
+	const quote = forceQuotes || windowsBatchArgumentNeedsQuotes(argument);
+	let result = quote ? '"' : '';
+	let backslashes = 0;
+
+	for (const character of argument) {
+		if (character === '\\') {
+			backslashes++;
+		} else {
+			if (character === '"') {
+				result += '\\'.repeat(backslashes);
+				result += '"';
+			} else if (character === '%') {
+				result += '%%cd:~,';
+			}
+			backslashes = 0;
+		}
+		result += character;
+	}
+
+	if (quote) {
+		result += '\\'.repeat(backslashes);
+		result += '"';
+	}
+
+	return result;
+}
+
+/**
+ * Builds an injection-safe cmd.exe invocation for a Windows batch file.
+ */
+export function prepareWindowsBatchCommand(command: string, args: readonly string[]): string[] {
+	if (command.includes('"') || windowsBatchInvalidCharacters.test(command) || args.some(argument => windowsBatchInvalidCharacters.test(argument))) {
+		throw new Error(nls.localize('invalidWindowsBatchCommand', "Debug adapter commands and arguments contain invalid characters."));
+	}
+
+	const shellCommand = [
+		escapeWindowsBatchArgument(command, true),
+		...args.map(argument => escapeWindowsBatchArgument(argument))
+	].join(' ');
+
+	return ['/e:ON', '/v:OFF', '/d', '/c', `"${shellCommand}"`];
+}
+
 /**
 * An implementation that communicates via two streams with the debug adapter.
 */
@@ -236,15 +303,11 @@ export class ExecutableDebugAdapter extends StreamDebugAdapter {
 				if (options.cwd) {
 					spawnOptions.cwd = options.cwd;
 				}
-				if (platform.isWindows && (command.endsWith('.bat') || command.endsWith('.cmd'))) {
+				if (platform.isWindows && /\.(bat|cmd)$/i.test(command)) {
 					// https://github.com/microsoft/vscode/issues/224184
-					spawnOptions.shell = true;
-					spawnCommand = `"${command}"`;
-					spawnArgs = args.map(a => {
-						a = a.replace(/"/g, '\\"'); // Escape existing double quotes with \
-						// Wrap in double quotes
-						return `"${a}"`;
-					});
+					spawnOptions.windowsVerbatimArguments = true;
+					spawnCommand = process.env['ComSpec'] || 'cmd.exe';
+					spawnArgs = prepareWindowsBatchCommand(command, args);
 				}

 				this.serverProcess = cp.spawn(spawnCommand, spawnArgs, spawnOptions);
@@ -0,0 +1,120 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert from 'assert';
+import { spawnSync } from 'child_process';
+import { existsSync } from 'fs';
+import { mkdtemp, readFile, rm, writeFile } from 'fs/promises';
+import { tmpdir } from 'os';
+import { join } from '../../../../../base/common/path.js';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
+import { prepareWindowsBatchCommand } from '../../node/debugAdapter.js';
+
+suite('Debug - Debug Adapter', () => {
+
+	ensureNoDisposablesAreLeakedInTestSuite();
+
+	test('escapes Windows batch commands and arguments', () => {
+		assert.deepStrictEqual(
+			prepareWindowsBatchCommand(
+				'C:\\Program Files\\adapter.cmd',
+				['plain', 'with spaces', 'quote" & calc.exe & "', '|<>()^%!', 'C:\\path\\', '%PATH:z=z%']
+			),
+			[
+				'/e:ON',
+				'/v:OFF',
+				'/d',
+				'/c',
+				'""C:\\Program Files\\adapter.cmd" plain "with spaces" "quote"" & calc.exe & """ "|<>()^%%cd:~,%!" "C:\\path\\\\" "%%cd:~,%PATH:z=z%%cd:~,%""'
+			]
+		);
+	});
+
+	test('escapes backslash runs around quotes', () => {
+		assert.deepStrictEqual(
+			prepareWindowsBatchCommand('adapter.cmd', ['two\\\\', 'three\\\\\\', 'two\\\\"quote', 'three\\\\\\"quote']),
+			[
+				'/e:ON',
+				'/v:OFF',
+				'/d',
+				'/c',
+				'""adapter.cmd" "two\\\\\\\\" "three\\\\\\\\\\\\" "two\\\\\\\\""quote" "three\\\\\\\\\\\\""quote""'
+			]
+		);
+	});
+
+	test('rejects invalid Windows batch command characters', () => {
+		assert.deepStrictEqual(
+			[
+				() => prepareWindowsBatchCommand('adapter.cmd', ['safe\r\ncalc.exe']),
+				() => prepareWindowsBatchCommand('adapter.cmd', ['safe\0calc.exe']),
+				() => prepareWindowsBatchCommand('adapter".cmd', [])
+			].map(run => {
+				try {
+					run();
+					return false;
+				} catch {
+					return true;
+				}
+			}),
+			[true, true, true]
+		);
+	});
+
+	test('round-trips Windows batch arguments without executing metacharacters', async function () {
+		if (process.platform !== 'win32') {
+			this.skip();
+		}
+
+		const testDirectory = await mkdtemp(join(tmpdir(), 'vscode-debug-adapter-'));
+		const adapterPath = join(testDirectory, 'adapter.cmd');
+		const captureScriptPath = join(testDirectory, 'capture.cjs');
+		const outputPath = join(testDirectory, 'arguments.json');
+		const sideEffectPath = join(testDirectory, 'side-effect.txt');
+
+		try {
+			const roundTripArgs = [
+				'plain',
+				'with spaces',
+				'',
+				'|<>()^%!',
+				'C:\\path\\',
+				'%PATH:z=z%',
+				'two\\\\slashes'
+			];
+			const args = [...roundTripArgs, `quote" & echo unexpected>"${sideEffectPath}" & "`];
+			const forwardedArgs = args.map((_, index) => `"%~${index + 1}"`).join(' ');
+			await writeFile(adapterPath, `@echo off\r\n"%VSCODE_TEST_NODE%" "%VSCODE_TEST_CAPTURE_SCRIPT%" ${forwardedArgs}\r\n`);
+			await writeFile(captureScriptPath, 'require("fs").writeFileSync(process.env.VSCODE_TEST_OUTPUT, JSON.stringify(process.argv.slice(2)));');
+
+			const result = spawnSync(process.env['ComSpec'] || 'cmd.exe', prepareWindowsBatchCommand(adapterPath, args), {
+				encoding: 'utf8',
+				env: {
+					...process.env,
+					ELECTRON_RUN_AS_NODE: '1',
+					VSCODE_TEST_NODE: process.execPath,
+					VSCODE_TEST_CAPTURE_SCRIPT: captureScriptPath,
+					VSCODE_TEST_OUTPUT: outputPath
+				},
+				windowsVerbatimArguments: true
+			});
+			const capturedArgs: string[] | undefined = existsSync(outputPath) ? JSON.parse(await readFile(outputPath, 'utf8')) : undefined;
+
+			assert.deepStrictEqual({
+				status: result.status,
+				error: result.error?.message,
+				capturedArgs: capturedArgs?.slice(0, roundTripArgs.length),
+				sideEffectCreated: existsSync(sideEffectPath)
+			}, {
+				status: 0,
+				error: undefined,
+				capturedArgs: roundTripArgs,
+				sideEffectCreated: false
+			});
+		} finally {
+			await rm(testDirectory, { recursive: true, force: true });
+		}
+	});
+});