Ensure all the commands passed to srt as positional params instead of options.

[dileepyavan]

fixes(#297387)

This PR addresses

• adding '--' after settings option to SRT to consider all the commands and its args are treated as positional instead of options.
• CWD args in allowWritePaths are being overridden by the last MCP server. Updated it such that CWD args are added to default allow write domains.

[Copilot]

Pull request overview

This PR updates MCP sandbox launching to ensure commands/arguments passed to the srt (sandbox-runtime) CLI are treated as positional parameters (not parsed as options), and adjusts how default allowWrite paths are assembled so launch CWD entries are not overridden.

Changes:

• Adds -- after --settings <path> when building srt CLI arguments.
• Introduces service-level default allow-write paths and appends launch CWD to that list.
• Adds a helper to POSIX-quote the launched command/args before passing them to srt.

Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/mcp/common/mcpSandboxService.ts:303

• -- is only added when sandboxConfigPath is present. However, this service can still end up launching srt without a settings file (e.g. when no tmp dir is available), in which case the wrapped command/args are again subject to srt option parsing if they start with -.

To fully address “treat all command args as positional”, consider always inserting -- before command whenever srt is used, independent of whether --settings is passed.

	private _getSandboxCommandArgs(command: string, args: readonly string[], sandboxConfigPath: string | undefined): string[] {
		const result: string[] = [];
		if (sandboxConfigPath) {
			result.push('--settings', sandboxConfigPath);
			result.push('--');
		}
		result.push(command, ...args);
		return result;

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

_getDefaultAllowWrite mutates the service-level _defaultAllowWritePaths array by pushing launchCwd values into it. Since McpSandboxService is a singleton, this state will accumulate across multiple server launches (and even across different servers/config targets), unintentionally broadening sandbox filesystem.allowWrite over time. This is both a correctness and sandbox-hardening issue.

Consider keeping the defaults immutable (e.g., a readonly base array) and returning a per-call merged list (use a local Set/array copy and avoid mutating shared state).

See below for a potential fix:

		const mergedAllowWrite = new Set(this._defaultAllowWritePaths);
		for (const launchCwd of directories ?? []) {
			const trimmed = launchCwd.trim();
			if (trimmed) {
				mergedAllowWrite.add(trimmed);
			}
		}
		return [...mergedAllowWrite];

[Copilot]

The command/args are being wrapped with POSIX shell quoting ('...' + escaping) before being passed into _getSandboxCommandArgs. These values are ultimately forwarded as argv entries to the srt Node CLI (and later to spawn), not through a shell, so the added quotes/backslashes become part of the actual argument values. This is very likely to break command resolution (e.g. trying to execute an executable literally named 'npx').

Suggestion: pass launch.command/launch.args through unchanged and rely on the -- separator to prevent srt option parsing issues. If srt needs a single command-line string, build that explicitly (one argument) rather than quoting each argv element.

This issue also appears on line 296 of the same file.

Suggested change

	const quotedCommand = this._quoteShellArgument(launch.command);
	const quotedArgs = launch.args.map(arg => this._quoteShellArgument(arg));
	const sandboxArgs = this._getSandboxCommandArgs(quotedCommand, quotedArgs, launchDetails.sandboxConfigPath);
	const sandboxArgs = this._getSandboxCommandArgs(launch.command, launch.args, launchDetails.sandboxConfigPath);