feat: unify CLI execution and never spawn a shell

package.json

@@ -160,7 +160,7 @@
 					]
 				},
 				"coder.globalFlags": {
-					"markdownDescription": "Global flags to pass to every Coder CLI invocation. Enter each flag as a separate array item; values are passed verbatim and in order. Do **not** include the `coder` command itself. See the [CLI reference](https://coder.com/docs/reference/cli) for available global flags.\n\nNote that for `--header-command`, precedence is: `#coder.headerCommand#` setting, then `CODER_HEADER_COMMAND` environment variable, then the value specified here. The `--global-config` and `--use-keyring` flags are silently ignored as the extension manages them via `#coder.useKeyring#`.",
+					"markdownDescription": "Global flags to pass to every Coder CLI invocation. Enter each flag as a separate array item; values are passed verbatim and in order. Do **not** include the `coder` command itself. See the [CLI reference](https://coder.com/docs/reference/cli) for available global flags.\n\nValues are passed directly to the CLI without a shell, so `$VAR` and `%VAR%` are **not** expanded. The extension does expand:\n- `${env:VAR}` → the value of `VAR` in the extension host's environment (missing variables resolve to an empty string).\n- A leading `~` or `${userHome}` → your home directory. For `--flag=value` entries the path expansion applies to the value half so `--cfg=~/coder` works.\n\nNote that for `--header-command`, precedence is: `#coder.headerCommand#` setting, then `CODER_HEADER_COMMAND` environment variable, then the value specified here. The `--global-config` and `--use-keyring` flags are silently ignored as the extension manages them via `#coder.useKeyring#`.",
 					"type": "array",
 					"items": {
 						"type": "string"

src/api/updateParameters.ts

@@ -1,7 +1,5 @@
 import * as vscode from "vscode";
 
-import { escapeShellArg } from "../util";
-
 import type { Api } from "coder/site/src/api/api";
 import type {
 	TemplateVersionParameter,
@@ -44,7 +42,7 @@ export async function collectUpdateParameters(
 		if (value === undefined) {
 			throw new WorkspaceUpdateCancelledError();
 		}
-		args.push("--parameter", escapeShellArg(`${param.name}=${value}`));
+		args.push("--parameter", `${param.name}=${value}`);
 	}
 	return args;
 }

src/api/workspace.ts

@@ -1,8 +1,7 @@
 import { spawn } from "node:child_process";
 import * as vscode from "vscode";
 
-import { getGlobalShellFlags, type CliAuth } from "../settings/cli";
-import { escapeCommandArg, escapeShellArg } from "../util";
+import { getGlobalFlags, type CliAuth } from "../settings/cli";
 
 import { errToStr, createWorkspaceIdentifier } from "./api-helper";
 import { collectUpdateParameters } from "./updateParameters";
@@ -62,12 +61,11 @@ interface CliContext {
 function runCliCommand(ctx: CliContext, args: string[]): Promise<void> {
 	return new Promise((resolve, reject) => {
 		const fullArgs = [
-			...getGlobalShellFlags(vscode.workspace.getConfiguration(), ctx.auth),
+			...getGlobalFlags(vscode.workspace.getConfiguration(), ctx.auth),
 			...args,
-			escapeShellArg(createWorkspaceIdentifier(ctx.workspace)),
+			createWorkspaceIdentifier(ctx.workspace),
 		];
-		const cmd = `${escapeCommandArg(ctx.binPath)} ${fullArgs.join(" ")}`;
-		const proc = spawn(cmd, { shell: true });
+		const proc = spawn(ctx.binPath, fullArgs);
 		// Unexpected prompts EOF instead of hanging forever.
 		proc.stdin.end();
 
@@ -82,6 +80,9 @@ function runCliCommand(ctx: CliContext, args: string[]): Promise<void> {
 			capturedStderr += text;
 		});
 
+		// Settle on ENOENT/EACCES; later `close` rejects are then no-ops.
+		proc.on("error", reject);
+
 		proc.on("close", (code: number | null, signal: NodeJS.Signals | null) => {
 			if (code === 0) {
 				resolve();

src/core/cliExec.ts

@@ -109,11 +109,11 @@ export async function supportBundle(
  * Run `coder ping` in a PTY terminal with Ctrl+C support.
  */
 export function ping(env: CliEnv, workspaceName: string): vscode.Terminal {
-	const globalFlags = getGlobalShellFlags(env.configs, env.auth);
+	const globalFlags = getGlobalFlags(env.configs, env.auth);
 	return spawnCliInTerminal({
 		name: `Coder Ping: ${workspaceName}`,
 		binary: env.binary,
-		args: [...globalFlags, "ping", escapeCommandArg(workspaceName)],
+		args: [...globalFlags, "ping", workspaceName],
 		banner: ["Press Ctrl+C (^C) to stop.", "─".repeat(40)],
 	});
 }
@@ -172,14 +172,12 @@ function spawnCliInTerminal(options: {
 	const writeEmitter = new vscode.EventEmitter<string>();
 	const closeEmitter = new vscode.EventEmitter<number | void>();
 
-	const cmd = `${escapeCommandArg(options.binary)} ${options.args.join(" ")}`;
-	// On Unix, spawn in a new process group so we can signal the
-	// entire group (shell + coder binary) on Ctrl+C. On Windows,
-	// detached opens a visible console window and negative-PID kill
-	// is unsupported, so we fall back to proc.kill().
+	// On Unix, `detached` puts the child in its own process group so
+	// Ctrl+C can signal the whole subtree. On Windows it would open a
+	// visible console window and negative-PID kill is unsupported, so we
+	// fall back to proc.kill() there.
 	const useProcessGroup = process.platform !== "win32";
-	const proc = spawn(cmd, {
-		shell: true,
+	const proc = spawn(options.binary, options.args, {
 		detached: useProcessGroup,
 	});
 

src/settings/cli.ts

@@ -1,5 +1,5 @@
 import { isKeyringSupported } from "../core/cliCredentialManager";
-import { escapeCommandArg, escapeShellArg } from "../util";
+import { escapeCommandArg, escapeShellArg, expandPath } from "../util";
 
 import { getHeaderArgs } from "./headers";
 
@@ -11,14 +11,36 @@ export type CliAuth =
 	| { mode: "global-config"; configDir: string }
 	| { mode: "url"; url: string };
 
-/** Returns the user's `coder.globalFlags` as configured, with no expansion. */
+/**
+ * Returns the user's `coder.globalFlags` with `${env:VAR}` references
+ * substituted from `process.env` (missing vars become empty, matching
+ * VS Code's built-in `${env:VAR}` behaviour) and `~` / `${userHome}`
+ * expanded to the home directory. For `--flag=value` entries the path
+ * expansion applies to the value half so `--cfg=~/coder` works.
+ */
 export function getUserGlobalFlags(
 	configs: Pick<WorkspaceConfiguration, "get">,
 ): string[] {
-	return configs.get<string[]>("coder.globalFlags", []);
+	return configs
+		.get<string[]>("coder.globalFlags", [])
+		.map((flag) =>
+			flag.replace(
+				/\$\{env:([A-Za-z_][A-Za-z0-9_]*)\}/g,
+				(_, name: string) => process.env[name] ?? "",
+			),
+		)
+		.map(expandFlagPath);
+}
+
+/** Applies `expandPath` to the value half of `--flag=value`, or the whole entry. */
+function expandFlagPath(flag: string): string {
+	const eq = flag.indexOf("=");
+	return eq === -1
+		? expandPath(flag)
+		: flag.slice(0, eq + 1) + expandPath(flag.slice(eq + 1));
 }
 
-/** Flags for shell contexts (`terminal.sendText`, `spawn({ shell: true })`). */
+/** Flags for shell contexts (`terminal.sendText`, SSH `ProxyCommand`). */
 export function getGlobalShellFlags(
 	configs: Pick<WorkspaceConfiguration, "get">,
 	auth: CliAuth,

test/unit/api/updateParameters.test.ts

@@ -8,8 +8,6 @@ import {
 
 import { workspace as createWorkspace } from "@repo/mocks";
 
-import { shellQuote } from "../../utils/platform";
-
 import type { Api } from "coder/site/src/api/api";
 import type { TemplateVersionParameter } from "coder/site/src/api/typesGenerated";
 
@@ -145,14 +143,14 @@ describe("collectUpdateParameters", () => {
 			param: { name: "environment" },
 			mock: mockCreateInputBox,
 			accept: { value: "dev" },
-			expected: ["--parameter", shellQuote("environment=dev")],
+			expected: ["--parameter", "environment=dev"],
 		},
 		{
 			kind: "bool quick pick",
 			param: { name: "enabled", type: "bool" },
 			mock: mockCreateQuickPick,
 			accept: { selectedItems: [{ value: "true" }] },
-			expected: ["--parameter", shellQuote("enabled=true")],
+			expected: ["--parameter", "enabled=true"],
 		},
 		{
 			kind: "options quick pick",
@@ -165,7 +163,7 @@ describe("collectUpdateParameters", () => {
 			},
 			mock: mockCreateQuickPick,
 			accept: { selectedItems: [{ value: "l" }] },
-			expected: ["--parameter", shellQuote("size=l")],
+			expected: ["--parameter", "size=l"],
 		},
 		{
 			kind: "multi-select quick pick (JSON array)",
@@ -179,7 +177,7 @@ describe("collectUpdateParameters", () => {
 			},
 			mock: mockCreateQuickPick,
 			accept: { selectedItems: [{ value: "us" }, { value: "eu" }] },
-			expected: ["--parameter", shellQuote('regions=["us","eu"]')],
+			expected: ["--parameter", 'regions=["us","eu"]'],
 		},
 	])(
 		"collects the value via $kind",
@@ -195,18 +193,15 @@ describe("collectUpdateParameters", () => {
 		},
 	);
 
-	it("escapes shell metacharacters in server-controlled values", async () => {
+	it("passes server-controlled values through verbatim (no shell expansion path)", async () => {
 		const { restClient, workspace } = createCollectCtx([{ name: "evil" }]);
 		const qi = mockCreateInputBox();
 
 		const result = collectUpdateParameters(restClient, workspace);
 		await waitShown(qi);
 		qi.accept({ value: "$(rm -rf /)" });
 
-		await expect(result).resolves.toEqual([
-			"--parameter",
-			shellQuote("evil=$(rm -rf /)"),
-		]);
+		await expect(result).resolves.toEqual(["--parameter", "evil=$(rm -rf /)"]);
 	});
 
 	it("skips parameters that already have a value or default", async () => {

test/unit/api/workspace.test.ts

@@ -6,8 +6,6 @@ import { LazyStream, startWorkspace, updateWorkspace } from "@/api/workspace";
 
 import { workspace as createWorkspace } from "@repo/mocks";
 
-import { shellQuote } from "../../utils/platform";
-
 import type { Api } from "coder/site/src/api/api";
 import type {
 	Workspace,
@@ -129,6 +127,10 @@ function controlSpawn() {
 			await spawned;
 			proc.emit("close", exitCode, signal ?? null);
 		},
+		async error(err: Error) {
+			await spawned;
+			proc.emit("error", err);
+		},
 	};
 }
 
@@ -204,10 +206,12 @@ describe("updateWorkspace", () => {
 		await sp.close(0);
 
 		await expect(result).resolves.toBe(finalWorkspace);
-		expect(spawn).toHaveBeenCalledWith(
-			`"/usr/bin/coder" --url "https://test.coder.com" update ${shellQuote("testuser/test-workspace")}`,
-			{ shell: true },
-		);
+		expect(spawn).toHaveBeenCalledWith("/usr/bin/coder", [
+			"--url",
+			"https://test.coder.com",
+			"update",
+			"testuser/test-workspace",
+		]);
 		expect(sp.stdinEnd).toHaveBeenCalled();
 		expect(restClient.getWorkspace).toHaveBeenCalledWith(ctx.workspace.id);
 	});
@@ -225,6 +229,17 @@ describe("updateWorkspace", () => {
 		expect(restClient.getWorkspace).not.toHaveBeenCalled();
 	});
 
+	it("rejects when spawn emits an error (e.g. missing binary)", async () => {
+		const { ctx, restClient } = createUpdateCtx();
+		const sp = controlSpawn();
+
+		const result = updateWorkspace(ctx);
+		await sp.error(new Error("spawn /usr/bin/coder ENOENT"));
+
+		await expect(result).rejects.toThrow(/ENOENT/);
+		expect(restClient.getWorkspace).not.toHaveBeenCalled();
+	});
+
 	it("reports the terminating signal when the process is killed", async () => {
 		const { ctx } = createUpdateCtx();
 		const sp = controlSpawn();
@@ -295,10 +310,15 @@ describe("startWorkspace", () => {
 		await sp.close(0);
 
 		await expect(result).resolves.toBe(finalWorkspace);
-		expect(spawn).toHaveBeenCalledWith(
-			`"/usr/bin/coder" --url "https://test.coder.com" start --yes --reason vscode_connection ${shellQuote("testuser/test-workspace")}`,
-			{ shell: true },
-		);
+		expect(spawn).toHaveBeenCalledWith("/usr/bin/coder", [
+			"--url",
+			"https://test.coder.com",
+			"start",
+			"--yes",
+			"--reason",
+			"vscode_connection",
+			"testuser/test-workspace",
+		]);
 		expect(restClient.getWorkspace).toHaveBeenCalledWith(ctx.workspace.id);
 	});
 
@@ -320,9 +340,12 @@ describe("startWorkspace", () => {
 		await sp.close(0);
 		await result;
 
-		expect(spawn).toHaveBeenCalledWith(
-			`"/usr/bin/coder" --url "https://test.coder.com" start --yes ${shellQuote("testuser/test-workspace")}`,
-			{ shell: true },
-		);
+		expect(spawn).toHaveBeenCalledWith("/usr/bin/coder", [
+			"--url",
+			"https://test.coder.com",
+			"start",
+			"--yes",
+			"testuser/test-workspace",
+		]);
 	});
 });

test/unit/cliConfig.test.ts

@@ -231,6 +231,49 @@ describe("cliConfig", () => {
 				"--disable-direct-connections",
 			]);
 		});
+
+		it("substitutes ${env:VAR} from process.env", () => {
+			const restore = process.env.CODER_TEST_VAR;
+			process.env.CODER_TEST_VAR = "from-env";
+			try {
+				const config = new MockConfigurationProvider();
+				config.set("coder.globalFlags", [
+					"--prefix=${env:CODER_TEST_VAR}",
+					"${env:CODER_MISSING_VAR}-suffix",
+				]);
+
+				expect(getUserGlobalFlags(config)).toStrictEqual([
+					"--prefix=from-env",
+					"-suffix",
+				]);
+			} finally {
+				if (restore === undefined) {
+					delete process.env.CODER_TEST_VAR;
+				} else {
+					process.env.CODER_TEST_VAR = restore;
+				}
+			}
+		});
+
+		it("expands ~ and ${userHome} in flag values", () => {
+			vi.mocked(os.homedir).mockReturnValue("/home/coder");
+			const config = new MockConfigurationProvider();
+			config.set("coder.globalFlags", [
+				"~/bare",
+				"--cfg=~/coder",
+				"--state=${userHome}/state",
+				"--literal=value~with~tildes",
+			]);
+
+			expect(getUserGlobalFlags(config)).toStrictEqual([
+				"/home/coder/bare",
+				"--cfg=/home/coder/coder",
+				"--state=/home/coder/state",
+				// Tildes mid-value are left alone (only ~ at the start of the
+				// value half is expanded).
+				"--literal=value~with~tildes",
+			]);
+		});
 	});
 
 	describe("getSshFlags", () => {