diff --git a/src/api/updateParameters.ts b/src/api/updateParameters.ts new file mode 100644 index 00000000..34dc39b1 --- /dev/null +++ b/src/api/updateParameters.ts @@ -0,0 +1,212 @@ +import * as vscode from "vscode"; + +import { escapeShellArg } from "../util"; + +import type { Api } from "coder/site/src/api/api"; +import type { + TemplateVersionParameter, + Workspace, +} from "coder/site/src/api/typesGenerated"; + +/** Thrown when the user dismisses a parameter prompt. */ +export class WorkspaceUpdateCancelledError extends Error { + constructor() { + super("Workspace update cancelled"); + this.name = "WorkspaceUpdateCancelledError"; + } +} + +/** + * Prompts the user for any newly-required template parameters and returns + * `--parameter name=value` args suitable for `coder update`. Throws + * `WorkspaceUpdateCancelledError` if the user dismisses a prompt. + */ +export async function collectUpdateParameters( + restClient: Api, + workspace: Workspace, +): Promise { + const [newParams, currentValues] = await Promise.all([ + restClient.getTemplateVersionRichParameters( + workspace.template_active_version_id, + ), + restClient.getWorkspaceBuildParameters(workspace.latest_build.id), + ]); + const candidates = newParams.filter((p) => p.required && !p.default_value); + if (candidates.length === 0) return []; + + const existing = new Set(currentValues.map((p) => p.name)); + const toPrompt = candidates.filter((p) => !existing.has(p.name)); + + const args: string[] = []; + for (let i = 0; i < toPrompt.length; i++) { + const param = toPrompt[i]; + const value = await promptForParameter(param, i + 1, toPrompt.length); + if (value === undefined) { + throw new WorkspaceUpdateCancelledError(); + } + args.push("--parameter", escapeShellArg(`${param.name}=${value}`)); + } + return args; +} + +function promptForParameter( + param: TemplateVersionParameter, + step: number, + totalSteps: number, +): Promise { + const title = param.display_name || param.name; + const items = quickPickItems(param); + + if (items) { + const multi = param.form_type === "multi-select"; + const qp = vscode.window.createQuickPick<(typeof items)[number]>(); + qp.title = title; + qp.step = step; + qp.totalSteps = totalSteps; + qp.placeholder = param.description_plaintext; + qp.items = items; + qp.canSelectMany = multi; + qp.ignoreFocusOut = true; + return collectInput(qp, () => { + if (multi) { + return qp.selectedItems.length > 0 + ? JSON.stringify(qp.selectedItems.map((i) => i.value)) + : undefined; + } + return qp.selectedItems[0]?.value; + }); + } + + const input = vscode.window.createInputBox(); + input.title = title; + input.step = step; + input.totalSteps = totalSteps; + input.prompt = param.description_plaintext; + input.placeholder = formatConstraint(param); + input.value = param.default_value; + input.ignoreFocusOut = true; + const validate = makeValidator(param); + const refresh = () => { + input.validationMessage = validate(input.value).message ?? ""; + }; + refresh(); + input.onDidChangeValue(refresh); + return collectInput(input, () => + validate(input.value).ok ? input.value : undefined, + ); +} + +/** Resolves with `onAccept()` on accept, or `undefined` when hidden. */ +function collectInput( + qi: vscode.InputBox | vscode.QuickPick, + onAccept: () => T | undefined, +): Promise { + return new Promise((resolve) => { + let done = false; + const finish = (value: T | undefined) => { + if (done) return; + done = true; + resolve(value); + qi.dispose(); + }; + qi.onDidAccept(() => { + const value = onAccept(); + if (value !== undefined) finish(value); + }); + qi.onDidHide(() => finish(undefined)); + qi.show(); + }); +} + +/** + * Returns picker items if the param needs a chooser, otherwise undefined. + * Anything that falls through gets a free-form text input. + */ +function quickPickItems( + param: TemplateVersionParameter, +): Array | undefined { + if (param.type === "bool") { + return [ + { label: "True", value: "true" }, + { label: "False", value: "false" }, + ]; + } + if (param.options.length > 0) { + return param.options.map((o) => ({ + label: o.name, + description: o.description, + value: o.value, + })); + } + return undefined; +} + +function isSet(value: T | null | undefined): value is T { + return value !== null && value !== undefined; +} + +function formatConstraint(param: TemplateVersionParameter): string { + if (param.type === "number") { + const lo = param.validation_min; + const hi = param.validation_max; + if (isSet(lo) && isSet(hi)) return `between ${lo} and ${hi}`; + if (isSet(lo)) return `at least ${lo}`; + if (isSet(hi)) return `at most ${hi}`; + return "a number"; + } + if (param.validation_regex) { + return ( + substituteTemplate(param.validation_error, param) || + `must match ${param.validation_regex}` + ); + } + return ""; +} + +/** Substitutes `{min}`, `{max}`, `{value}` placeholders in validation_error. */ +function substituteTemplate( + template: string | undefined, + param: TemplateVersionParameter, + value?: string, +): string | undefined { + if (!template) return template; + return template + .replace(/{min}/g, String(param.validation_min ?? "")) + .replace(/{max}/g, String(param.validation_max ?? "")) + .replace(/{value}/g, value ?? ""); +} + +/** + * Returns `{ ok, message }`. Regex constraints are intentionally not tested + * client-side; server validates with RE2 (linear-time, ReDoS-safe). + */ +function makeValidator( + param: TemplateVersionParameter, +): (input: string) => { ok: boolean; message?: string } { + return (input) => { + if (!input) return { ok: !param.required }; + if (param.type === "number") { + const n = Number(input); + if (!Number.isFinite(n)) { + return { ok: false, message: "Must be a number" }; + } + if (isSet(param.validation_min) && n < param.validation_min) { + return { + ok: false, + message: + substituteTemplate(param.validation_error, param, input) || + `Must be at least ${param.validation_min}`, + }; + } + if (isSet(param.validation_max) && n > param.validation_max) { + return { + ok: false, + message: + substituteTemplate(param.validation_error, param, input) || + `Must be at most ${param.validation_max}`, + }; + } + } + return { ok: true }; + }; +} diff --git a/src/api/workspace.ts b/src/api/workspace.ts index 3c23e28f..a0e24dd6 100644 --- a/src/api/workspace.ts +++ b/src/api/workspace.ts @@ -1,19 +1,23 @@ -import { type Api } from "coder/site/src/api/api"; -import { - type WorkspaceAgentLog, - type ProvisionerJobLog, - type Workspace, -} from "coder/site/src/api/typesGenerated"; import { spawn } from "node:child_process"; import * as vscode from "vscode"; -import { type FeatureSet } from "../featureSet"; import { getGlobalShellFlags, type CliAuth } from "../settings/cli"; -import { escapeCommandArg } from "../util"; -import { type UnidirectionalStream } from "../websocket/eventStreamConnection"; +import { escapeCommandArg, escapeShellArg } from "../util"; import { errToStr, createWorkspaceIdentifier } from "./api-helper"; -import { type CoderApi } from "./coderApi"; +import { collectUpdateParameters } from "./updateParameters"; + +import type { Api } from "coder/site/src/api/api"; +import type { + ProvisionerJobLog, + Workspace, + WorkspaceAgentLog, +} from "coder/site/src/api/typesGenerated"; + +import type { FeatureSet } from "../featureSet"; +import type { UnidirectionalStream } from "../websocket/eventStreamConnection"; + +import type { CoderApi } from "./coderApi"; /** Opens a stream once; subsequent open() calls are no-ops until closed. */ export class LazyStream { @@ -54,42 +58,40 @@ interface CliContext { featureSet: FeatureSet; } -/** - * Spawn a Coder CLI subcommand and stream its output. - * Resolves when the process exits successfully; rejects on non-zero exit. - */ +/** Streams CLI output via `ctx.write`; rejects with stderr on non-zero exit. */ function runCliCommand(ctx: CliContext, args: string[]): Promise { return new Promise((resolve, reject) => { const fullArgs = [ ...getGlobalShellFlags(vscode.workspace.getConfiguration(), ctx.auth), ...args, - createWorkspaceIdentifier(ctx.workspace), + escapeShellArg(createWorkspaceIdentifier(ctx.workspace)), ]; - const cmd = `${escapeCommandArg(ctx.binPath)} ${fullArgs.join(" ")}`; const proc = spawn(cmd, { shell: true }); + // Unexpected prompts EOF instead of hanging forever. + proc.stdin.end(); proc.stdout.on("data", (data: Buffer) => { - ctx.write(data.toString().replace(/\r?\n/g, "\r\n")); + ctx.write(data.toString()); }); let capturedStderr = ""; proc.stderr.on("data", (data: Buffer) => { const text = data.toString(); - ctx.write(text.replace(/\r?\n/g, "\r\n")); + ctx.write(text); capturedStderr += text; }); - proc.on("close", (code: number) => { + proc.on("close", (code: number | null, signal: NodeJS.Signals | null) => { if (code === 0) { resolve(); - } else { - let errorText = `"${fullArgs.join(" ")}" exited with code ${code}`; - if (capturedStderr !== "") { - errorText += `: ${capturedStderr}`; - } - reject(new Error(errorText)); + return; } + const exit = + code !== null ? `code ${code}` : `signal ${signal ?? "unknown"}`; + let msg = `"${fullArgs.join(" ")}" exited with ${exit}`; + if (capturedStderr) msg += `: ${capturedStderr}`; + reject(new Error(msg)); }); }); } @@ -113,24 +115,31 @@ export async function startWorkspace(ctx: CliContext): Promise { } /** - * Update a workspace to the latest template version. - * - * Uses `coder update` when the CLI supports it (>= 2.24). - * Falls back to the REST API: stop, wait, then updateWorkspaceVersion. + * Update a workspace to the latest template version. Collects any newly- + * required parameters via VS Code prompts and passes them to the CLI as flags + * (the resolver phase can't render an interactive terminal). Falls back to + * the REST API for CLIs older than 2.24. */ export async function updateWorkspace(ctx: CliContext): Promise { - if (ctx.featureSet.cliUpdate) { - await runCliCommand(ctx, ["update"]); - return ctx.restClient.getWorkspace(ctx.workspace.id); + if (!ctx.featureSet.cliUpdate) { + return updateWorkspaceVersion(ctx); } - // REST API fallback for older CLIs. + const paramArgs = await collectUpdateParameters( + ctx.restClient, + ctx.workspace, + ); + await runCliCommand(ctx, ["update", ...paramArgs]); + return ctx.restClient.getWorkspace(ctx.workspace.id); +} + +async function updateWorkspaceVersion(ctx: CliContext): Promise { if (ctx.workspace.latest_build.status === "running") { ctx.write("Stopping workspace for update...\r\n"); const stopBuild = await ctx.restClient.stopWorkspace(ctx.workspace.id); const stoppedJob = await ctx.restClient.waitForBuild(stopBuild); if (stoppedJob?.status === "canceled") { - throw new Error("Workspace update canceled during stop"); + throw new Error("Workspace update cancelled during stop"); } } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 29077169..045ba8de 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -41,9 +41,9 @@ import { type LoginCoordinator } from "../login/loginCoordinator"; import { OAuthSessionManager } from "../oauth/sessionManager"; import { type CliAuth, - getGlobalFlagsRaw, getGlobalShellFlags, getSshFlags, + getUserGlobalFlags, resolveCliAuth, } from "../settings/cli"; import { getHeaderCommand } from "../settings/headers"; @@ -437,7 +437,7 @@ export class Remote { setting: "coder.globalFlags", title: "Global Flags", getValue: () => - getGlobalFlagsRaw(vscode.workspace.getConfiguration()), + getUserGlobalFlags(vscode.workspace.getConfiguration()), }, { setting: "coder.headerCommand", @@ -562,7 +562,7 @@ export class Remote { const isReady = await stateMachine.processWorkspace(w, progress); if (isReady) { subscription.dispose(); - resolve(w); + resolve(stateMachine.getWorkspace() ?? w); return; } } catch (error: unknown) { diff --git a/src/remote/workspaceStateMachine.ts b/src/remote/workspaceStateMachine.ts index b533ffb8..0b9a299b 100644 --- a/src/remote/workspaceStateMachine.ts +++ b/src/remote/workspaceStateMachine.ts @@ -1,4 +1,11 @@ -import { createWorkspaceIdentifier, extractAgents } from "../api/api-helper"; +import * as vscode from "vscode"; + +import { + createWorkspaceIdentifier, + errToStr, + extractAgents, +} from "../api/api-helper"; +import { WorkspaceUpdateCancelledError } from "../api/updateParameters"; import { LazyStream, startWorkspace, @@ -16,7 +23,6 @@ import type { Workspace, WorkspaceAgentLog, } from "coder/site/src/api/typesGenerated"; -import type * as vscode from "vscode"; import type { CoderApi } from "../api/coderApi"; import type { StartupMode } from "../core/mementoManager"; @@ -35,6 +41,7 @@ export class WorkspaceStateMachine implements vscode.Disposable { private readonly agentLogStream = new LazyStream(); private agent: { id: string; name: string } | undefined; + private workspace: Workspace | undefined; constructor( private readonly parts: AuthorityParts, @@ -56,17 +63,25 @@ export class WorkspaceStateMachine implements vscode.Disposable { workspace: Workspace, progress: vscode.Progress<{ message?: string }>, ): Promise { + this.workspace = workspace; const workspaceName = createWorkspaceIdentifier(workspace); switch (workspace.latest_build.status) { - case "running": + case "running": { this.buildLogStream.close(); - if (this.startupMode === "update") { - await this.triggerUpdate(workspace, workspaceName, progress); + const updated = await this.maybeUpdate( + workspace, + workspaceName, + progress, + ); + if (updated) { + workspace = updated; // Agent IDs may have changed after an update. this.agent = undefined; + if (workspace.latest_build.status !== "running") return false; } break; + } case "stopped": case "failed": { @@ -83,11 +98,20 @@ export class WorkspaceStateMachine implements vscode.Disposable { this.startupMode = choice; } - if (this.startupMode === "update") { - await this.triggerUpdate(workspace, workspaceName, progress); - } else { - await this.triggerStart(workspace, workspaceName, progress); + const updated = await this.maybeUpdate( + workspace, + workspaceName, + progress, + ); + if (updated) { + workspace = updated; + // Agent IDs may have changed after an update. + this.agent = undefined; + if (workspace.latest_build.status !== "running") return false; + break; } + // Either we weren't in update mode, or the update failed: start. + await this.triggerStart(workspace, workspaceName, progress); return false; } @@ -248,20 +272,36 @@ export class WorkspaceStateMachine implements vscode.Disposable { this.logger.info(`${workspaceName} start initiated`); } - private async triggerUpdate( + /** No-op if not in update mode. Falls through to start on failure. */ + private async maybeUpdate( workspace: Workspace, workspaceName: string, progress: vscode.Progress<{ message?: string }>, - ): Promise { + ): Promise { + if (this.startupMode !== "update") return undefined; + // Downgrade up-front so monitor events don't retry the update. + this.startupMode = "start"; progress.report({ message: `updating ${workspaceName}...` }); this.logger.info(`Updating ${workspaceName}`, { - mode: this.startupMode, status: workspace.latest_build.status, }); - await updateWorkspace(this.buildCliContext(workspace)); - // Downgrade so subsequent transitions don't re-trigger the update. - this.startupMode = "start"; - this.logger.info(`${workspaceName} update initiated`); + try { + this.workspace = await updateWorkspace(this.buildCliContext(workspace)); + return this.workspace; + } catch (error) { + if (error instanceof WorkspaceUpdateCancelledError) { + this.logger.info( + `Update cancelled for ${workspaceName}; continuing with the existing version.`, + ); + return undefined; + } + const reason = errToStr(error); + this.logger.warn(`Update failed for ${workspaceName}: ${reason}`); + vscode.window.showWarningMessage( + `Workspace update failed: ${reason}. Continuing with the existing version.`, + ); + return undefined; + } } private async confirmStartOrUpdate( @@ -286,6 +326,10 @@ export class WorkspaceStateMachine implements vscode.Disposable { return this.agent?.id; } + public getWorkspace(): Workspace | undefined { + return this.workspace; + } + dispose(): void { this.buildLogStream.close(); this.agentLogStream.close(); diff --git a/src/settings/cli.ts b/src/settings/cli.ts index 23185139..2fe70826 100644 --- a/src/settings/cli.ts +++ b/src/settings/cli.ts @@ -1,5 +1,5 @@ import { isKeyringSupported } from "../core/cliCredentialManager"; -import { escapeCommandArg } from "../util"; +import { escapeCommandArg, escapeShellArg } from "../util"; import { getHeaderArgs } from "./headers"; @@ -11,67 +11,61 @@ export type CliAuth = | { mode: "global-config"; configDir: string } | { mode: "url"; url: string }; -/** - * Returns the raw global flags from user configuration. - */ -export function getGlobalFlagsRaw( +/** Returns the user's `coder.globalFlags` as configured, with no expansion. */ +export function getUserGlobalFlags( configs: Pick, ): string[] { return configs.get("coder.globalFlags", []); } -/** - * Returns global configuration flags for Coder CLI commands with auth values - * escaped for shell use (e.g., `terminal.sendText`, `spawn({ shell: true })`). - */ +/** Flags for shell contexts (`terminal.sendText`, `spawn({ shell: true })`). */ export function getGlobalShellFlags( configs: Pick, auth: CliAuth, ): string[] { - return buildGlobalFlags(configs, auth, escapeCommandArg); + return buildGlobalFlags(configs, auth, escapeCommandArg, escapeShellArg); } -/** - * Returns global configuration flags for Coder CLI commands with raw auth - * values suitable for `execFile` (no shell escaping). - */ +/** Raw flags for `execFile` or `spawn` without a shell. */ export function getGlobalFlags( configs: Pick, auth: CliAuth, ): string[] { - return buildGlobalFlags(configs, auth, (s) => s); + return buildGlobalFlags(configs, auth, identity, identity); } +const identity = (s: string) => s; + function buildGlobalFlags( configs: Pick, auth: CliAuth, - esc: (s: string) => string, + escAuth: (s: string) => string, + escHeader: (s: string) => string, ): string[] { const authFlags = auth.mode === "url" - ? ["--url", esc(auth.url)] - : ["--global-config", esc(auth.configDir)]; + ? ["--url", escAuth(auth.url)] + : ["--global-config", escAuth(auth.configDir)]; - const raw = getGlobalFlagsRaw(configs); - const filtered = stripManagedFlags(raw); + const filtered = stripManagedFlags(getUserGlobalFlags(configs)); - return [...filtered, ...authFlags, ...getHeaderArgs(configs)]; + return [...filtered, ...authFlags, ...getHeaderArgs(configs, escHeader)]; } -function stripManagedFlags(rawFlags: string[]): string[] { +function stripManagedFlags(flags: string[]): string[] { const filtered: string[] = []; - for (let i = 0; i < rawFlags.length; i++) { - if (isFlag(rawFlags[i], "--use-keyring")) { + for (let i = 0; i < flags.length; i++) { + if (isFlag(flags[i], "--use-keyring")) { continue; } - if (isFlag(rawFlags[i], "--global-config")) { + if (isFlag(flags[i], "--global-config")) { // Skip the next item too when the value is a separate entry. - if (rawFlags[i] === "--global-config") { + if (flags[i] === "--global-config") { i++; } continue; } - filtered.push(rawFlags[i]); + filtered.push(flags[i]); } return filtered; } diff --git a/src/settings/headers.ts b/src/settings/headers.ts index bb1c4594..7abbdd10 100644 --- a/src/settings/headers.ts +++ b/src/settings/headers.ts @@ -1,7 +1,3 @@ -import * as os from "node:os"; - -import { escapeCommandArg } from "../util"; - import type { WorkspaceConfiguration } from "vscode"; /** Returns the header command from settings or the CODER_HEADER_COMMAND env var. */ @@ -15,22 +11,14 @@ export function getHeaderCommand( return cmd || undefined; } -/** Returns `--header-command` CLI args, escaped for the current platform. */ +/** Returns `--header-command` args; shell callers pass `escapeShellArg` as `esc`. */ export function getHeaderArgs( config: Pick, + esc: (s: string) => string = (s) => s, ): string[] { - // Escape a command line to be executed by the Coder binary, so ssh doesn't substitute variables. - const escapeSubcommand: (str: string) => string = - os.platform() === "win32" - ? // On Windows variables are %VAR%, and we need to use double quotes. - (str) => escapeCommandArg(str).replace(/%/g, "%%") - : // On *nix we can use single quotes to escape $VARS. - // Note single quotes cannot be escaped inside single quotes. - (str) => `'${str.replace(/'/g, "'\\''")}'`; - const command = getHeaderCommand(config); if (!command) { return []; } - return ["--header-command", escapeSubcommand(command)]; + return ["--header-command", esc(command)]; } diff --git a/src/util.ts b/src/util.ts index 9b3ec7ff..22a63800 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,4 +1,4 @@ -import * as os from "node:os"; +import os from "node:os"; import url from "node:url"; export interface AuthorityParts { @@ -207,6 +207,18 @@ export function escapeCommandArg(arg: string): string { return `"${escapedString}"`; } +/** + * Cross-platform shell quoting that blocks variable expansion. Use for + * values from outside the user's local settings (e.g. server-controlled). + */ +export function escapeShellArg(arg: string): string { + if (os.platform() === "win32") { + const escaped = arg.replace(/"/g, '""').replace(/%/g, "%%"); + return `"${escaped}"`; + } + return `'${arg.replace(/'/g, "'\\''")}'`; +} + /** * Generate a temporary file path by appending a suffix with a random component. * The suffix describes the purpose of the temp file (e.g. "temp", "old"). diff --git a/src/workspace/workspaceMonitor.ts b/src/workspace/workspaceMonitor.ts index 7f37b07a..4fca7473 100644 --- a/src/workspace/workspaceMonitor.ts +++ b/src/workspace/workspaceMonitor.ts @@ -269,7 +269,10 @@ export class WorkspaceMonitor implements vscode.Disposable { } private updateStatusBar(workspace: Workspace) { - if (workspace.outdated) { + const status = workspace.latest_build.status; + const settled = + status === "running" || status === "stopped" || status === "failed"; + if (workspace.outdated && settled) { this.statusBarItem.show(); } else { this.statusBarItem.hide(); diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index 8def001e..b22a86f0 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -138,6 +138,8 @@ export const window = { dispose: vi.fn(), clear: vi.fn(), })), + createInputBox: vi.fn(), + createQuickPick: vi.fn(), createStatusBarItem: vi.fn(), createWebviewPanel: vi.fn(), registerUriHandler: vi.fn(() => ({ dispose: vi.fn() })), diff --git a/test/unit/api/updateParameters.test.ts b/test/unit/api/updateParameters.test.ts new file mode 100644 index 00000000..2b67b2fa --- /dev/null +++ b/test/unit/api/updateParameters.test.ts @@ -0,0 +1,388 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; + +import { + collectUpdateParameters, + WorkspaceUpdateCancelledError, +} from "@/api/updateParameters"; + +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"; + +function param(overrides: Partial = {}) { + return { + name: "environment", + display_name: "Environment", + description: "", + description_plaintext: "", + type: "string", + form_type: "input", + mutable: true, + default_value: "", + icon: "", + options: [], + required: true, + ephemeral: false, + ...overrides, + } as TemplateVersionParameter; +} + +function createCollectCtx( + params: Array> = [], + previousValues: Array<{ name: string; value: string }> = [], +) { + const workspace = createWorkspace(); + const restClient = { + getTemplateVersionRichParameters: vi + .fn() + .mockResolvedValue(params.map(param)), + getWorkspaceBuildParameters: vi.fn().mockResolvedValue(previousValues), + }; + return { + workspace, + restClient: restClient as unknown as Api, + mocks: restClient, + }; +} + +interface QuickInputMock { + mock: Record & { + show: ReturnType; + dispose: ReturnType; + }; + accept: (overrides?: Record) => void; + change: (value: string) => void; + hide: () => void; +} + +function quickInputMock(): QuickInputMock { + let acceptCb: () => void = () => {}; + let hideCb: () => void = () => {}; + let changeCb: (v: string) => void = () => {}; + const mock = { + title: "", + step: 0, + totalSteps: 0, + prompt: "", + placeholder: "", + value: "", + validationMessage: "", + ignoreFocusOut: false, + items: [] as readonly unknown[], + selectedItems: [] as readonly unknown[], + onDidAccept: vi.fn((cb: () => void) => { + acceptCb = cb; + return { dispose: vi.fn() }; + }), + onDidHide: vi.fn((cb: () => void) => { + hideCb = cb; + return { dispose: vi.fn() }; + }), + onDidChangeValue: vi.fn((cb: (v: string) => void) => { + changeCb = cb; + return { dispose: vi.fn() }; + }), + show: vi.fn(), + dispose: vi.fn(), + }; + return { + mock, + accept(overrides) { + Object.assign(mock, overrides ?? {}); + if (overrides && "value" in overrides) changeCb(mock.value); + acceptCb(); + }, + change(value) { + mock.value = value; + changeCb(value); + }, + hide() { + hideCb(); + }, + }; +} + +function mockCreateInputBox() { + const qi = quickInputMock(); + vi.mocked(vscode.window.createInputBox).mockReturnValue( + qi.mock as unknown as vscode.InputBox, + ); + return qi; +} + +function mockCreateQuickPick() { + const qi = quickInputMock(); + vi.mocked(vscode.window.createQuickPick).mockReturnValue( + qi.mock as unknown as vscode.QuickPick, + ); + return qi; +} + +async function waitShown(qi: QuickInputMock): Promise { + await vi.waitFor(() => expect(qi.mock.show).toHaveBeenCalled()); +} + +describe("collectUpdateParameters", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + interface CollectCase { + kind: string; + param: Partial; + mock: () => QuickInputMock; + accept: Record; + expected: string[]; + } + + it.each([ + { + kind: "text input", + param: { name: "environment" }, + mock: mockCreateInputBox, + accept: { value: "dev" }, + expected: ["--parameter", shellQuote("environment=dev")], + }, + { + kind: "bool quick pick", + param: { name: "enabled", type: "bool" }, + mock: mockCreateQuickPick, + accept: { selectedItems: [{ value: "true" }] }, + expected: ["--parameter", shellQuote("enabled=true")], + }, + { + kind: "options quick pick", + param: { + name: "size", + options: [ + { name: "Small", description: "", value: "s", icon: "" }, + { name: "Large", description: "", value: "l", icon: "" }, + ], + }, + mock: mockCreateQuickPick, + accept: { selectedItems: [{ value: "l" }] }, + expected: ["--parameter", shellQuote("size=l")], + }, + { + kind: "multi-select quick pick (JSON array)", + param: { + name: "regions", + form_type: "multi-select", + options: [ + { name: "US", description: "", value: "us", icon: "" }, + { name: "EU", description: "", value: "eu", icon: "" }, + ], + }, + mock: mockCreateQuickPick, + accept: { selectedItems: [{ value: "us" }, { value: "eu" }] }, + expected: ["--parameter", shellQuote('regions=["us","eu"]')], + }, + ])( + "collects the value via $kind", + async ({ param: p, mock, accept, expected }) => { + const { restClient, workspace } = createCollectCtx([p]); + const qi = mock(); + + const result = collectUpdateParameters(restClient, workspace); + await waitShown(qi); + qi.accept(accept); + + await expect(result).resolves.toEqual(expected); + }, + ); + + it("escapes shell metacharacters in server-controlled values", 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 /)"), + ]); + }); + + it("skips parameters that already have a value or default", async () => { + const { restClient, workspace } = createCollectCtx( + [ + { name: "existing" }, + { name: "with_default", default_value: "foo" }, + { name: "optional", required: false }, + ], + [{ name: "existing", value: "kept" }], + ); + + await expect( + collectUpdateParameters(restClient, workspace), + ).resolves.toEqual([]); + expect(vscode.window.createInputBox).not.toHaveBeenCalled(); + }); + + it("throws WorkspaceUpdateCancelledError when the prompt is dismissed", async () => { + const { restClient, workspace } = createCollectCtx([{}]); + const qi = mockCreateInputBox(); + + const result = collectUpdateParameters(restClient, workspace); + await waitShown(qi); + qi.hide(); + + await expect(result).rejects.toBeInstanceOf(WorkspaceUpdateCancelledError); + }); + + it("steps the input title across multiple required params", async () => { + const { restClient, workspace } = createCollectCtx([ + { name: "a" }, + { name: "b" }, + ]); + const inputs = [quickInputMock(), quickInputMock()]; + vi.mocked(vscode.window.createInputBox) + .mockReturnValueOnce(inputs[0].mock as unknown as vscode.InputBox) + .mockReturnValueOnce(inputs[1].mock as unknown as vscode.InputBox); + + const result = collectUpdateParameters(restClient, workspace); + await waitShown(inputs[0]); + inputs[0].accept({ value: "first" }); + await waitShown(inputs[1]); + inputs[1].accept({ value: "second" }); + await result; + + expect(inputs.map((i) => [i.mock.step, i.mock.totalSteps])).toEqual([ + [1, 2], + [2, 2], + ]); + }); +}); + +describe("parameter prompt validation", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + async function withInputBox( + p: Partial, + fn: (qi: QuickInputMock) => void, + ) { + const { restClient, workspace } = createCollectCtx([p]); + const qi = mockCreateInputBox(); + const result = collectUpdateParameters(restClient, workspace); + await waitShown(qi); + fn(qi); + qi.hide(); + await result.catch(() => {}); + } + + it("silently blocks accept on empty required input", async () => { + await withInputBox({ name: "x" }, (qi) => { + qi.accept({ value: "" }); + expect(qi.mock.validationMessage).toBe(""); + expect(qi.mock.dispose).not.toHaveBeenCalled(); + }); + }); + + interface ValidationCase { + kind: string; + param: Partial; + input: string; + expected: string; + } + + it.each([ + { + kind: "number below validation_min", + param: { type: "number", validation_min: 5 }, + input: "1", + expected: "Must be at least 5", + }, + { + kind: "number above validation_max", + param: { type: "number", validation_max: 9 }, + input: "10", + expected: "Must be at most 9", + }, + { + kind: "non-numeric input on number param", + param: { type: "number" }, + input: "abc", + expected: "Must be a number", + }, + { + kind: "number out-of-range with {min}/{max} substitution", + param: { + type: "number", + validation_min: 1, + validation_max: 5, + validation_error: "Pick {value} between {min} and {max}", + }, + input: "7", + expected: "Pick 7 between 1 and 5", + }, + ])("surfaces $kind", async ({ param: p, input, expected }) => { + await withInputBox({ name: "x", ...p }, (qi) => { + qi.change(input); + expect(qi.mock.validationMessage).toBe(expected); + }); + }); + + it("does not evaluate validation_regex client-side (ReDoS guard)", async () => { + await withInputBox({ name: "r", validation_regex: "^(a+)+$" }, (qi) => { + qi.change("aaaaaaaaaaaaaaaaaaaaaaaaaaa!"); + expect(qi.mock.validationMessage).toBe(""); + }); + }); + + it("treats JSON null on validation_min/max as unset", async () => { + await withInputBox( + { + name: "n", + type: "number", + validation_min: null as unknown as number, + validation_max: null as unknown as number, + }, + (qi) => { + qi.change("42"); + expect(qi.mock.validationMessage).toBe(""); + expect(qi.mock.placeholder).toBe("a number"); + }, + ); + }); + + interface PlaceholderCase { + kind: string; + param: Partial; + expected: string; + } + + it.each([ + { + kind: "between bounds", + param: { type: "number", validation_min: 1, validation_max: 10 }, + expected: "between 1 and 10", + }, + { + kind: "lower bound only", + param: { type: "number", validation_min: 1 }, + expected: "at least 1", + }, + { + kind: "upper bound only", + param: { type: "number", validation_max: 10 }, + expected: "at most 10", + }, + { + kind: "regex with custom error", + param: { validation_regex: "^x", validation_error: "must start with x" }, + expected: "must start with x", + }, + ])("renders placeholder for $kind", async ({ param: p, expected }) => { + await withInputBox({ name: "x", ...p }, (qi) => { + expect(qi.mock.placeholder).toBe(expected); + }); + }); +}); diff --git a/test/unit/api/workspace.test.ts b/test/unit/api/workspace.test.ts index 182ee390..4bc4eff0 100644 --- a/test/unit/api/workspace.test.ts +++ b/test/unit/api/workspace.test.ts @@ -1,7 +1,38 @@ -import { describe, expect, it, vi } from "vitest"; +import { EventEmitter } from "node:events"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; -import { LazyStream } from "@/api/workspace"; -import { type UnidirectionalStream } from "@/websocket/eventStreamConnection"; +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, + WorkspaceBuild, +} from "coder/site/src/api/typesGenerated"; + +import type { FeatureSet } from "@/featureSet"; +import type { UnidirectionalStream } from "@/websocket/eventStreamConnection"; + +vi.mock(import("node:child_process"), async (importOriginal) => ({ + ...(await importOriginal()), + spawn: vi.fn(), +})); +const { spawn } = await import("node:child_process"); + +const featureSet: FeatureSet = { + vscodessh: true, + proxyLogDirectory: true, + wildcardSSH: true, + buildReason: true, + cliUpdate: true, + keyringAuth: true, + keyringTokenRead: true, + supportBundle: true, +}; function mockStream(): UnidirectionalStream { return { @@ -28,6 +59,79 @@ function deferredFactory() { }; } +function createUpdateCtx( + overrides: { + workspace?: Omit, "latest_build"> & { + latest_build?: Partial; + }; + featureSet?: Partial; + } = {}, +) { + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ + get: vi.fn((_key: string, defaultValue: unknown) => defaultValue), + } as never); + const workspace = createWorkspace({ + outdated: true, + latest_build: { status: "running", transition: "start" }, + ...overrides.workspace, + }); + const finalWorkspace = createWorkspace({ + outdated: false, + latest_build: { status: "running" }, + }); + const restClient = { + getWorkspace: vi.fn().mockResolvedValue(finalWorkspace), + stopWorkspace: vi + .fn() + .mockResolvedValue({ ...workspace.latest_build, status: "stopped" }), + updateWorkspaceVersion: vi.fn().mockResolvedValue(workspace.latest_build), + waitForBuild: vi.fn().mockResolvedValue({ + ...workspace.latest_build.job, + status: "succeeded", + }), + getTemplateVersionRichParameters: vi.fn().mockResolvedValue([]), + getWorkspaceBuildParameters: vi.fn().mockResolvedValue([]), + }; + const ctx = { + restClient: restClient as unknown as Api, + auth: { mode: "url" as const, url: "https://test.coder.com" }, + binPath: "/usr/bin/coder", + workspace, + write: vi.fn<(data: string) => void>(), + featureSet: { ...featureSet, ...overrides.featureSet }, + }; + return { ctx, restClient, finalWorkspace }; +} + +/** Drives mocked spawn() so tests can fire stdout/stderr + close at will. */ +function controlSpawn() { + const proc = new EventEmitter() as EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + stdin: { end: ReturnType }; + }; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.stdin = { end: vi.fn() }; + const { promise: spawned, resolve: resolveSpawned } = + Promise.withResolvers(); + vi.mocked(spawn).mockImplementation(() => { + resolveSpawned(); + return proc as never; + }); + return { + spawned, + stdinEnd: proc.stdin.end, + stderr(data: string) { + proc.stderr.emit("data", Buffer.from(data)); + }, + async close(exitCode: number | null, signal?: NodeJS.Signals) { + await spawned; + proc.emit("close", exitCode, signal ?? null); + }, + }; +} + describe("LazyStream", () => { it("opens once and ignores subsequent calls", async () => { const factory: StreamFactory = vi.fn().mockResolvedValue(mockStream()); @@ -86,3 +190,139 @@ describe("LazyStream", () => { expect(factory2).toHaveBeenCalledOnce(); }); }); + +describe("updateWorkspace", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("runs coder update and resolves with the refreshed workspace", async () => { + const { ctx, restClient, finalWorkspace } = createUpdateCtx(); + const sp = controlSpawn(); + + const result = updateWorkspace(ctx); + 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(sp.stdinEnd).toHaveBeenCalled(); + expect(restClient.getWorkspace).toHaveBeenCalledWith(ctx.workspace.id); + }); + + it("rejects when the process exits non-zero", async () => { + const { ctx, restClient } = createUpdateCtx(); + const sp = controlSpawn(); + + const result = updateWorkspace(ctx); + await sp.spawned; + sp.stderr("auth failed"); + await sp.close(1); + + await expect(result).rejects.toThrow(/exited with code 1.*auth failed/); + expect(restClient.getWorkspace).not.toHaveBeenCalled(); + }); + + it("reports the terminating signal when the process is killed", async () => { + const { ctx } = createUpdateCtx(); + const sp = controlSpawn(); + + const result = updateWorkspace(ctx); + await sp.close(null, "SIGTERM"); + + await expect(result).rejects.toThrow(/signal SIGTERM/); + }); + + it("falls back to the API update path when coder update is unsupported", async () => { + const { ctx, restClient, finalWorkspace } = createUpdateCtx({ + featureSet: { cliUpdate: false }, + }); + + await expect(updateWorkspace(ctx)).resolves.toBe(finalWorkspace); + + expect(spawn).not.toHaveBeenCalled(); + expect(restClient.getTemplateVersionRichParameters).not.toHaveBeenCalled(); + expect(restClient.stopWorkspace).toHaveBeenCalledWith(ctx.workspace.id); + expect(restClient.updateWorkspaceVersion).toHaveBeenCalledWith( + ctx.workspace, + ); + }); + + it("does not stop before API fallback update when the workspace is not running", async () => { + const { ctx, restClient } = createUpdateCtx({ + workspace: { latest_build: { status: "stopped", transition: "stop" } }, + featureSet: { cliUpdate: false }, + }); + + await updateWorkspace(ctx); + + expect(restClient.stopWorkspace).not.toHaveBeenCalled(); + expect(restClient.updateWorkspaceVersion).toHaveBeenCalledWith( + ctx.workspace, + ); + }); + + it("throws before update when the API fallback stop is cancelled", async () => { + const { ctx, restClient } = createUpdateCtx({ + featureSet: { cliUpdate: false }, + }); + restClient.waitForBuild.mockResolvedValueOnce({ + ...ctx.workspace.latest_build.job, + status: "canceled", + }); + + await expect(updateWorkspace(ctx)).rejects.toThrow( + "Workspace update cancelled during stop", + ); + expect(restClient.updateWorkspaceVersion).not.toHaveBeenCalled(); + }); +}); + +describe("startWorkspace", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("runs coder start when the workspace is stopped", async () => { + const { ctx, restClient, finalWorkspace } = createUpdateCtx({ + workspace: { latest_build: { status: "stopped", transition: "stop" } }, + }); + const sp = controlSpawn(); + + const result = startWorkspace(ctx); + 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(restClient.getWorkspace).toHaveBeenCalledWith(ctx.workspace.id); + }); + + it("no-ops when the workspace is already running", async () => { + const { ctx, restClient } = createUpdateCtx(); + await expect(startWorkspace(ctx)).resolves.toBe(ctx.workspace); + expect(spawn).not.toHaveBeenCalled(); + expect(restClient.getWorkspace).not.toHaveBeenCalled(); + }); + + it("omits --reason when buildReason feature is unavailable", async () => { + const { ctx } = createUpdateCtx({ + workspace: { latest_build: { status: "stopped", transition: "stop" } }, + featureSet: { buildReason: false }, + }); + const sp = controlSpawn(); + + const result = startWorkspace(ctx); + 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 }, + ); + }); +}); diff --git a/test/unit/cliConfig.test.ts b/test/unit/cliConfig.test.ts index 61030f8a..9481827c 100644 --- a/test/unit/cliConfig.test.ts +++ b/test/unit/cliConfig.test.ts @@ -6,9 +6,9 @@ import { featureSetForVersion } from "@/featureSet"; import { type CliAuth, getGlobalFlags, - getGlobalFlagsRaw, getGlobalShellFlags, getSshFlags, + getUserGlobalFlags, isKeyringEnabled, resolveCliAuth, } from "@/settings/cli"; @@ -190,14 +190,14 @@ describe("cliConfig", () => { ]); }); - it("should still escape header-command flags", () => { + it("passes header-command value through verbatim (no shell)", () => { const config = new MockConfigurationProvider(); config.set("coder.headerCommand", "echo test"); expect(getGlobalFlags(config, globalConfigAuth)).toStrictEqual([ "--global-config", "/config/dir", "--header-command", - quoteCommand("echo test"), + "echo test", ]); }); @@ -212,11 +212,11 @@ describe("cliConfig", () => { }); }); - describe("getGlobalFlagsRaw", () => { + describe("getUserGlobalFlags", () => { it("returns empty array when no global flags configured", () => { const config = new MockConfigurationProvider(); - expect(getGlobalFlagsRaw(config)).toStrictEqual([]); + expect(getUserGlobalFlags(config)).toStrictEqual([]); }); it("returns global flags from config", () => { @@ -226,7 +226,7 @@ describe("cliConfig", () => { "--disable-direct-connections", ]); - expect(getGlobalFlagsRaw(config)).toStrictEqual([ + expect(getUserGlobalFlags(config)).toStrictEqual([ "--verbose", "--disable-direct-connections", ]); diff --git a/test/unit/core/cliExec.test.ts b/test/unit/core/cliExec.test.ts index 7d1fcbd8..bec8767a 100644 --- a/test/unit/core/cliExec.test.ts +++ b/test/unit/core/cliExec.test.ts @@ -6,7 +6,6 @@ import { beforeAll, describe, expect, it, vi } from "vitest"; import { MockConfigurationProvider } from "../../mocks/testHelpers"; import { isWindows, - quoteCommand, writeExecutable, writeStderrJs, writeStdoutJs, @@ -154,7 +153,7 @@ describe("cliExec", () => { "--url", "http://localhost:3000", "--header-command", - quoteCommand("my-header-cmd"), + "my-header-cmd", "speedtest", "owner/workspace", "--output", @@ -211,7 +210,7 @@ describe("cliExec", () => { "--url", "http://localhost:3000", "--header-command", - quoteCommand("my-header-cmd"), + "my-header-cmd", "support", "bundle", "owner/workspace", diff --git a/test/unit/remote/workspaceStateMachine.test.ts b/test/unit/remote/workspaceStateMachine.test.ts index 1b62a217..fb82c42b 100644 --- a/test/unit/remote/workspaceStateMachine.test.ts +++ b/test/unit/remote/workspaceStateMachine.test.ts @@ -1,5 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; +import { WorkspaceUpdateCancelledError } from "@/api/updateParameters"; import { startWorkspace, updateWorkspace, @@ -94,6 +96,9 @@ describe("WorkspaceStateMachine", () => { beforeEach(() => { vi.clearAllMocks(); MockTerminalOutputChannel.lastInstance = undefined; + vi.mocked(updateWorkspace).mockImplementation((ctx) => + Promise.resolve(ctx.workspace), + ); vi.mocked(maybeAskAgent).mockImplementation((agents) => Promise.resolve(agents.length > 0 ? agents[0] : undefined), ); @@ -182,6 +187,44 @@ describe("WorkspaceStateMachine", () => { expect(updateWorkspace).toHaveBeenCalledOnce(); }); + it("falls through to the agent check after an update completes", async () => { + vi.mocked(updateWorkspace).mockResolvedValueOnce(runningWorkspace()); + const { sm, progress } = setup("update"); + const ws = createWorkspace({ latest_build: { status: "stopped" } }); + + expect(await sm.processWorkspace(ws, progress)).toBe(true); + expect(updateWorkspace).toHaveBeenCalledOnce(); + expect(sm.getWorkspace()?.latest_build.status).toBe("running"); + }); + + it("falls back to start and warns the user when the update fails", async () => { + vi.mocked(updateWorkspace).mockRejectedValueOnce( + new Error("template not found"), + ); + const { sm, progress } = setup("update"); + const ws = createWorkspace({ latest_build: { status: "stopped" } }); + + expect(await sm.processWorkspace(ws, progress)).toBe(false); + expect(updateWorkspace).toHaveBeenCalledOnce(); + expect(startWorkspace).toHaveBeenCalledOnce(); + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith( + expect.stringMatching(/Workspace update failed:.*template not found/), + ); + }); + + it("falls back to start silently when the user cancels the update", async () => { + vi.mocked(updateWorkspace).mockRejectedValueOnce( + new WorkspaceUpdateCancelledError(), + ); + const { sm, progress } = setup("update"); + const ws = createWorkspace({ latest_build: { status: "stopped" } }); + + expect(await sm.processWorkspace(ws, progress)).toBe(false); + expect(updateWorkspace).toHaveBeenCalledOnce(); + expect(startWorkspace).toHaveBeenCalledOnce(); + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled(); + }); + it("prompts user when mode is 'none' and user picks 'Start'", async () => { const { sm, progress, userInteraction } = setup("none"); userInteraction.setResponse(CONFIRM_MESSAGE, "Start"); diff --git a/test/unit/util.test.ts b/test/unit/util.test.ts index e8ef1f0f..745d7e97 100644 --- a/test/unit/util.test.ts +++ b/test/unit/util.test.ts @@ -5,6 +5,7 @@ import { type AuthorityParts, countSubstring, escapeCommandArg, + escapeShellArg, expandPath, findPort, parseRemoteAuthority, @@ -212,6 +213,51 @@ describe("escapeCommandArg", () => { }); }); +describe("escapeShellArg", () => { + const platformSpy = vi.spyOn(os, "platform"); + afterEach(() => platformSpy.mockReset()); + + describe("on Unix", () => { + beforeEach(() => platformSpy.mockReturnValue("linux")); + + it("wraps in single quotes", () => { + expect(escapeShellArg("env=dev")).toBe("'env=dev'"); + }); + + it("escapes single quotes via the '\\'' sequence", () => { + expect(escapeShellArg("it's fine")).toBe("'it'\\''s fine'"); + }); + + it("leaves $VAR, $(...), and backticks literal inside the quotes", () => { + expect(escapeShellArg("$(echo pwned)")).toBe("'$(echo pwned)'"); + }); + }); + + describe("on Windows", () => { + beforeEach(() => platformSpy.mockReturnValue("win32")); + + it("wraps in double quotes", () => { + expect(escapeShellArg("env=dev")).toBe('"env=dev"'); + }); + + it('doubles embedded `"`', () => { + expect(escapeShellArg('regions=["us","eu"]')).toBe( + '"regions=[""us"",""eu""]"', + ); + }); + + it("doubles `%` to block %VAR% expansion", () => { + expect(escapeShellArg("%PATH%")).toBe('"%%PATH%%"'); + }); + + it("keeps cmd metachars inside the quoted region", () => { + expect(escapeShellArg('foo" & calc.exe & "x')).toBe( + '"foo"" & calc.exe & ""x"', + ); + }); + }); +}); + describe("expandPath", () => { const home = os.homedir(); diff --git a/test/unit/workspace/workspaceMonitor.test.ts b/test/unit/workspace/workspaceMonitor.test.ts index 5ceedcad..fb2b9cc7 100644 --- a/test/unit/workspace/workspaceMonitor.test.ts +++ b/test/unit/workspace/workspaceMonitor.test.ts @@ -106,6 +106,19 @@ describe("WorkspaceMonitor", () => { stream.pushMessage(workspaceEvent({ outdated: false })); expect(statusBar.hide).toHaveBeenCalled(); }); + + it("hides status bar while an outdated workspace is building", async () => { + const { stream, statusBar } = await setup(); + + stream.pushMessage( + workspaceEvent({ + outdated: true, + latest_build: { status: "starting" }, + }), + ); + expect(statusBar.show).not.toHaveBeenCalled(); + expect(statusBar.hide).toHaveBeenCalled(); + }); }); describe("notifications", () => { diff --git a/test/utils/platform.test.ts b/test/utils/platform.test.ts index 35bbc866..9d9d5307 100644 --- a/test/utils/platform.test.ts +++ b/test/utils/platform.test.ts @@ -3,7 +3,15 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { promisify } from "node:util"; -import { beforeAll, describe, expect, it } from "vitest"; +import { + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; import { expectPathsEqual, @@ -11,6 +19,7 @@ import { isWindows, printCommand, printEnvCommand, + shellQuote, shimExecFile, writeExecutable, writeStdoutJs, @@ -159,4 +168,43 @@ describe("platform utils", () => { expect(mod.spawn).toBe(cp.spawn); }); }); + + describe("shellQuote", () => { + const platformSpy = vi.spyOn(os, "platform"); + afterEach(() => platformSpy.mockReset()); + + describe("on Unix", () => { + beforeEach(() => platformSpy.mockReturnValue("linux")); + + it("wraps in single quotes", () => { + expect(shellQuote("env=dev")).toBe("'env=dev'"); + }); + + it("escapes single quotes via the '\\'' sequence", () => { + expect(shellQuote("it's fine")).toBe("'it'\\''s fine'"); + }); + + it("keeps $VAR, $(...), and backticks literal inside the quotes", () => { + expect(shellQuote("$(echo pwned)")).toBe("'$(echo pwned)'"); + }); + }); + + describe("on Windows", () => { + beforeEach(() => platformSpy.mockReturnValue("win32")); + + it("wraps in double quotes", () => { + expect(shellQuote("env=dev")).toBe('"env=dev"'); + }); + + it("doubles embedded double quotes", () => { + expect(shellQuote('regions=["us","eu"]')).toBe( + '"regions=[""us"",""eu""]"', + ); + }); + + it("doubles percent signs to block %VAR% expansion", () => { + expect(shellQuote("%PATH%")).toBe('"%%PATH%%"'); + }); + }); + }); }); diff --git a/test/utils/platform.ts b/test/utils/platform.ts index 5edb8587..d4827335 100644 --- a/test/utils/platform.ts +++ b/test/utils/platform.ts @@ -124,6 +124,18 @@ export function quoteCommand(value: string): string { return `${quote}${value}${quote}`; } +/** + * Test-side mirror of `escapeShellArg` from src/util.ts. Wraps in single + * quotes on Unix and double quotes (with `"` doubled and `%` doubled) on + * Windows. + */ +export function shellQuote(value: string): string { + if (isWindows()) { + return `"${value.replace(/"/g, '""').replace(/%/g, "%%")}"`; + } + return `'${value.replace(/'/g, "'\\''")}'`; +} + export function expectPathsEqual(actual: string, expected: string) { expect(normalizePath(actual)).toBe(normalizePath(expected)); }