Launch a terminal instead of a chat from New workspace (#2941)
* feat(app): launch a terminal from the New workspace screen
New workspace only ever started a chat agent. Empty workspaces existed but
were reachable only by submitting an empty composer, which nothing advertised,
and terminals could not be started from this screen at all. People concluded
Paseo did not support terminals.
Adds a launch chip to the meta row, shaped from the same ComboboxTrigger as
its neighbours, that chooses between Chat and a terminal. Terminal profiles
mark where a typed prompt goes with a {{{prompt}}} sentinel in their command
or args, substituted client-side before create_terminal_request, so the daemon
learns nothing new and the protocol is untouched. A profile without the
sentinel takes no input: the composer goes read-only and shows the exact
command, so there is nothing to type wrong.
Prompt forms come from each CLI's own --help. claude, codex, and pi take a
trailing positional; opencode's positional is the project directory, so it
uses --prompt=. Profiles predating the sentinel adopt the right form on read,
keyed on the command's base name rather than the profile id, because profiles
created in settings get generated ids.
* fix(app): stop existing launchers passing the prompt sentinel literally
Adoption made every caller of resolveTerminalProfiles see sentinel-bearing
args, but only New workspace substituted them. Pinned targets and the
workspace terminal menu forwarded profile.args straight through, so launching
Claude from an existing workspace ran `claude "{{{prompt}}}"` with that text as
the prompt.
Every launcher now goes through resolveTerminalProfileLaunch. Surfaces with
nowhere to type pass an empty prompt, which drops the prompt-only arg and
gives back the bare command they had before.
Also from the same review:
- Adoption skipped when args lead with a subcommand. `opencode run` takes a
positional message rather than --prompt, and `codex login` takes no prompt at
all. Only the first arg is checked, so `pi --thinking high` still adopts.
- Settings edits the persisted list again, not the adopted view. It writes the
whole list back on any change, so resolving first baked read-time adoption
into the user's config the moment they reordered a row.
- A hidden draft no longer names a workspace. Text typed for a prompt-taking
profile stayed in state after switching to a read-only one and still seeded
the generated title and branch.
- Windows .cmd escaping now has a round-trip test: a real cmd.exe runs a real
.cmd shim that echoes its argv, covering metacharacters, an injection
attempt, and newlines. The existing tests only asserted the command line we
generate.
- Fixed an ENOTEMPTY flake in the terminal suite. kill() only signals, so the
shell was still writing into $ZDOTDIR while its directory was being removed.
Cleanup waits for exit instead.
* fix(app): keep the host and isolation accessible names stable
Wiring the new tooltips also replaced these two accessible names with the
tooltip copy, which broke `sidebar-project-grouping` at the host picker: its
selector is `getByRole("button", { name: "Host", exact: true })` and the button
had become "Choose the host". It timed out on both attempts in CI.
An accessible name identifies a control; a tooltip explains it. Conflating them
renamed a control that other surfaces address by name. The names go back to
what they were and the tooltips keep the "Choose the ..." copy.
* test(server): stop temp-dir teardown failing unrelated terminal tests
The ENOTEMPTY flake survived both earlier attempts. Waiting for the session to
exit shrank the window; retrying the remove did not close it either. The writes
come from processes zsh forks on the way out, which outlive the pty and answer
to nothing the test holds.
Teardown is now best-effort and logs what it left behind. A temp directory we
cannot remove is not a defect in the terminal code under test, and failing on it
reports the race against whichever assertion ran last, which is exactly how this
surfaced: as a flake in an unrelated zsh title test. 10 consecutive runs green.
Also replaces a cast in the Windows .cmd helper with a real narrow, so a change
to ResolvedTerminalCommand fails loudly there. Test files are excluded from
typecheck (tsconfig.server.json), so a cast in one is never checked at all.
* fix(app): keep terminal launch policy in its owning workflows
Profile adoption now parses known global option values before deciding whether argv already contains a positional. Workspace naming is derived inside the terminal workflow so hidden launch-only drafts cannot affect branch or title generation.
* fix(server): preserve cmd shim arguments through both parses
npm command shims forward %*, so metacharacters lose one escape layer before the target process receives argv. Exercise the production node-pty string command line against a forwarding shim, and only tolerate the known ENOTEMPTY teardown race.
* fix(server): stabilize Windows subprocess tests
* test(server): isolate Windows argv recorder output
* test(server): allow native watcher integration to settle
This commit is contained in:
Mohamed Boudra
@@ -509,7 +509,7 @@ interface ControlledInterruptFixture {
|
||||
manager: AgentManager;
|
||||
session: ControlledInterruptSession;
|
||||
startForegroundRun(): Promise<void>;
|
||||
cleanup(): void;
|
||||
cleanup(): Promise<void>;
|
||||
}
|
||||
|
||||
async function createControlledInterruptFixture(options: {
|
||||
@@ -553,7 +553,10 @@ async function createControlledInterruptFixture(options: {
|
||||
})();
|
||||
await manager.waitForAgentRunStart(agent.id);
|
||||
},
|
||||
cleanup: () => rmSync(workdir, { recursive: true, force: true }),
|
||||
async cleanup() {
|
||||
await manager.closeAgent(agent.id);
|
||||
rmSync(workdir, { recursive: true, force: true });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1618,7 +1621,7 @@ test("cancelAgentRun preserves running state when the provider interrupt hangs",
|
||||
expect(fixture.session.interruptCalled).toBe(true);
|
||||
expect(fixture.manager.getAgent(fixture.agentId)?.lifecycle).toBe("running");
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
await fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1649,7 +1652,7 @@ test("cancelAgentRun preserves the active turn when the provider rejects the int
|
||||
turnId: "provider-still-active-turn",
|
||||
});
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
await fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1682,7 +1685,7 @@ test("cancelAgentRun succeeds when the foreground turn finishes before the provi
|
||||
activeForegroundTurnId: null,
|
||||
});
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
await fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -1712,7 +1715,7 @@ test("cancelAgentRun succeeds when the provider queues completion before rejecti
|
||||
activeForegroundTurnId: null,
|
||||
});
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
await fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -392,4 +392,4 @@ test("native recursive observation updates tracked state and prunes ignored stor
|
||||
expect(getCheckoutWorktreeState).not.toHaveBeenCalled();
|
||||
expect(getCheckoutDiff).not.toHaveBeenCalled();
|
||||
expect(service.getMetrics().workspaceRefreshQueuedCount).toBe(0);
|
||||
}, 15_000);
|
||||
}, 30_000);
|
||||
|
||||
@@ -23,9 +23,11 @@ import {
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import * as pty from "node-pty";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
import { setImmediate as waitForImmediate } from "node:timers/promises";
|
||||
import { setImmediate as waitForImmediate, setTimeout as delay } from "node:timers/promises";
|
||||
import { stripVTControlCharacters } from "node:util";
|
||||
|
||||
const hasZsh = existsSync("/bin/zsh");
|
||||
|
||||
@@ -116,16 +118,52 @@ if (isPlatform("win32") && !process.env.ComSpec && !process.env.COMSPEC) {
|
||||
const sessions: TerminalSession[] = [];
|
||||
const temporaryDirs: string[] = [];
|
||||
|
||||
/**
|
||||
* `kill()` only signals; the shell exits asynchronously and keeps writing on
|
||||
* the way out (zsh drops a history file and a compdump into `$ZDOTDIR`).
|
||||
* Deleting its directory before then loses the race as `ENOTEMPTY`: the
|
||||
* recursive walk empties the directory, the dying shell recreates a file in it,
|
||||
* and the final rmdir fails.
|
||||
*
|
||||
* Waiting for the exit shrinks that window but does not close it: some of those
|
||||
* writes come from processes zsh forked, which outlive the pty and answer to
|
||||
* nothing we hold. Retries alone did not close it either.
|
||||
*
|
||||
* The known post-retry `ENOTEMPTY` race is reported and tolerated because it
|
||||
* belongs to a descendant shell outliving the tested terminal. Any other
|
||||
* cleanup error still fails the test that owns the directory.
|
||||
*/
|
||||
async function waitForSessionExit(session: TerminalSession): Promise<void> {
|
||||
const exited = new Promise<void>((resolve) => {
|
||||
const dispose = session.onExit(() => {
|
||||
dispose();
|
||||
resolve();
|
||||
});
|
||||
session.kill();
|
||||
});
|
||||
await Promise.race([exited, delay(2_000)]);
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
vi.useRealTimers();
|
||||
for (const session of sessions) {
|
||||
session.kill();
|
||||
}
|
||||
await Promise.all(sessions.map(waitForSessionExit));
|
||||
sessions.length = 0;
|
||||
while (temporaryDirs.length > 0) {
|
||||
const dir = temporaryDirs.pop();
|
||||
if (dir) {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
try {
|
||||
rmSync(dir, { recursive: true, force: true, maxRetries: 20, retryDelay: 25 });
|
||||
} catch (error) {
|
||||
if (
|
||||
typeof error !== "object" ||
|
||||
error === null ||
|
||||
!("code" in error) ||
|
||||
error.code !== "ENOTEMPTY"
|
||||
) {
|
||||
throw error;
|
||||
}
|
||||
console.warn(`leaving temp dir behind after ENOTEMPTY retries: ${dir}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -216,6 +254,33 @@ describe("createTerminal", () => {
|
||||
expect(resolveExecutable).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.runIf(isPlatform("linux", "darwin"))(
|
||||
"passes an unescaped multi-line prompt through byte-for-byte on POSIX",
|
||||
async () => {
|
||||
// node-pty spawns directly via execve on POSIX (no shell), so a prompt
|
||||
// substituted into `args` is never re-parsed by anything -- it must
|
||||
// reach the target process exactly as typed, including raw newlines,
|
||||
// quotes, and shell metacharacters that would be dangerous under a
|
||||
// shell but are inert here.
|
||||
const nastyPrompt =
|
||||
'fix the bug\nwhere `git commit -m "wip"` fails\nif $HOME contains a & or | char';
|
||||
const resolveExecutable = vi.fn(async () => "/usr/local/bin/claude");
|
||||
|
||||
const resolvedLinux = await resolveTerminalSpawnCommand("claude", [nastyPrompt], {
|
||||
platform: "linux",
|
||||
resolveExecutable,
|
||||
});
|
||||
const resolvedDarwin = await resolveTerminalSpawnCommand("claude", [nastyPrompt], {
|
||||
platform: "darwin",
|
||||
resolveExecutable,
|
||||
});
|
||||
|
||||
expect(resolvedLinux).toEqual({ command: "claude", args: [nastyPrompt] });
|
||||
expect(resolvedDarwin).toEqual({ command: "claude", args: [nastyPrompt] });
|
||||
expect(resolveExecutable).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps the original command when it cannot be resolved on Windows", async () => {
|
||||
const resolved = await resolveTerminalSpawnCommand("claude", [], {
|
||||
platform: "win32",
|
||||
@@ -233,9 +298,77 @@ describe("createTerminal", () => {
|
||||
resolveExecutable: async () => "C:\\npm\\claude.cmd",
|
||||
});
|
||||
|
||||
// The `.cmd` branch now hands node-pty a single pre-escaped command line
|
||||
// (string) instead of an argv array, so that node-pty's own
|
||||
// MSVCRT-style array quoting never runs a second time over an already
|
||||
// cmd.exe-escaped argument. See escapeCmdExeArgument in terminal.ts.
|
||||
expect(resolved).toEqual({
|
||||
command: "C:\\Windows\\System32\\cmd.exe",
|
||||
args: ["/c", "C:\\npm\\claude.cmd", "--foo"],
|
||||
args: '/d /s /c "C:\\npm\\claude.cmd ^^^"--foo^^^""',
|
||||
});
|
||||
});
|
||||
|
||||
describe(".cmd shim argument escaping on Windows", () => {
|
||||
async function resolveCmdShimArgs(promptArg: string): Promise<string> {
|
||||
const resolved = await resolveTerminalSpawnCommand("claude", [promptArg], {
|
||||
platform: "win32",
|
||||
env: { ComSpec: "C:\\Windows\\System32\\cmd.exe" },
|
||||
resolveExecutable: async () => "C:\\npm\\claude.cmd",
|
||||
});
|
||||
// A real narrow rather than a cast: the `.cmd` branch is the one case
|
||||
// that returns a pre-escaped command line instead of argv, so if that
|
||||
// contract ever changes these tests should fail loudly here rather than
|
||||
// assert against a stringified array.
|
||||
const { args } = resolved;
|
||||
if (typeof args !== "string") {
|
||||
throw new Error(`expected a cmd.exe command line, got ${JSON.stringify(args)}`);
|
||||
}
|
||||
return args;
|
||||
}
|
||||
|
||||
// Each expected string is the exact cmd.exe command line the escaping
|
||||
// must produce so a real `cmd.exe /c` invocation reconstructs the
|
||||
// original prompt as a single argv element for the target process
|
||||
// (modulo the documented newline-to-space normalization below).
|
||||
it("neutralizes cmd.exe's own metacharacters instead of letting them execute", async () => {
|
||||
expect(await resolveCmdShimArgs("a & b | c")).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"a^^^ ^^^&^^^ b^^^ ^^^|^^^ c^^^""',
|
||||
);
|
||||
expect(await resolveCmdShimArgs("100% done %PATH%")).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"100^^^%^^^ done^^^ ^^^%PATH^^^%^^^""',
|
||||
);
|
||||
expect(await resolveCmdShimArgs("a^b")).toBe('/d /s /c "C:\\npm\\claude.cmd ^^^"a^^^^b^^^""');
|
||||
});
|
||||
|
||||
it("escapes embedded double quotes and backslash runs preceding them", async () => {
|
||||
expect(await resolveCmdShimArgs('say "hello" now')).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"say^^^ \\^^^"hello\\^^^"^^^ now^^^""',
|
||||
);
|
||||
expect(await resolveCmdShimArgs('C:\\path\\ "quoted"')).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"C:\\path\\^^^ \\^^^"quoted\\^^^"^^^""',
|
||||
);
|
||||
});
|
||||
|
||||
// cmd.exe has no way to escape a literal CR/LF inside one command line
|
||||
// -- its parser treats newlines as command separators regardless of
|
||||
// quoting -- so a raw newline is normalized to a space before escaping.
|
||||
// This is a deliberate choice, not an oversight: it avoids the prompt
|
||||
// ever splitting into a second, attacker-controlled cmd.exe command, at
|
||||
// the cost of losing the original line breaks.
|
||||
it("normalizes embedded newlines to spaces instead of letting them split the command", async () => {
|
||||
expect(await resolveCmdShimArgs("line one\nline two")).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"line^^^ one^^^ line^^^ two^^^""',
|
||||
);
|
||||
expect(await resolveCmdShimArgs("line one\r\nline two")).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"line^^^ one^^^ line^^^ two^^^""',
|
||||
);
|
||||
});
|
||||
|
||||
it("handles a multi-line prompt combining quotes, backslashes, and every cmd.exe metacharacter", async () => {
|
||||
const prompt = 'multi\nline "quoted \\path\\" & piped | percent %VAR% ^caret';
|
||||
expect(await resolveCmdShimArgs(prompt)).toBe(
|
||||
'/d /s /c "C:\\npm\\claude.cmd ^^^"multi^^^ line^^^ \\^^^"quoted^^^ \\path\\\\\\^^^"^^^ ^^^&^^^ piped^^^ ^^^|^^^ percent^^^ ^^^%VAR^^^%^^^ ^^^^caret^^^""',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -248,6 +381,35 @@ describe("createTerminal", () => {
|
||||
expect(resolved).toEqual({ command: "C:\\tools\\claude.exe", args: ["--foo"] });
|
||||
});
|
||||
|
||||
it.runIf(isPlatform("linux", "darwin"))(
|
||||
"spawns without shell interpretation on POSIX, so argv reaches the child byte-for-byte",
|
||||
async () => {
|
||||
// The composer substitutes {{{prompt}}} directly into `args` before this
|
||||
// ever reaches the daemon, so a raw multi-line prompt containing quotes,
|
||||
// backslashes, and shell metacharacters must arrive at the spawned
|
||||
// process exactly as typed. node-pty's POSIX backend spawns via execve
|
||||
// with no shell, so per-arg escaping is unnecessary there -- this test
|
||||
// proves that end-to-end through createTerminal (not just at the
|
||||
// resolveTerminalSpawnCommand unit level) by reading the argv the
|
||||
// spawned process actually received back out of the pty.
|
||||
const nastyPrompt =
|
||||
'line one\nline two: git commit -m "wip" && echo $HOME | grep ^x > out.txt';
|
||||
const session = trackSession(
|
||||
await createTerminal({
|
||||
workspaceId: "ws-test",
|
||||
cwd: realpathSync(tmpdir()),
|
||||
cols: 500,
|
||||
command: process.execPath,
|
||||
args: ["-e", "process.stdout.write(JSON.stringify(process.argv.slice(1)))", nastyPrompt],
|
||||
}),
|
||||
);
|
||||
|
||||
const state = await waitForState(session, (s) => getRowText(s, 0).startsWith("["));
|
||||
|
||||
expect(JSON.parse(getRowText(state, 0))).toEqual([nastyPrompt]);
|
||||
},
|
||||
);
|
||||
|
||||
it("creates a terminal session with an id, name, and cwd", async () => {
|
||||
const session = trackSession(
|
||||
await createTerminal({
|
||||
@@ -1096,3 +1258,89 @@ describe("terminal activity interruption", () => {
|
||||
expect(session.getActivity()).toMatchObject({ state: "working" });
|
||||
});
|
||||
});
|
||||
|
||||
// The escaping tests above assert the command line we generate. This one runs
|
||||
// it: a real cmd.exe launches a real .cmd shim, which echoes the argv it was
|
||||
// handed. It is the only check that proves cmd.exe's tokenizer reconstructs
|
||||
// the prompt instead of executing part of it, which is the failure the
|
||||
// escaping exists to prevent (the CVE-2024-27980 class).
|
||||
describe.runIf(isPlatform("win32"))(".cmd shim argv round-trip on Windows", () => {
|
||||
const argvMarker = "__PASEO_ARGV__";
|
||||
|
||||
function writeArgvEchoShim(): { dir: string; cmdPath: string } {
|
||||
const dir = mkdtempSync(join(tmpdir(), "paseo-cmd-shim-"));
|
||||
temporaryDirs.push(dir);
|
||||
const cmdPath = join(dir, "echoargv.cmd");
|
||||
const scriptPath = join(dir, "echoargv.js");
|
||||
writeFileSync(
|
||||
scriptPath,
|
||||
`process.stdout.write(${JSON.stringify(argvMarker)} + JSON.stringify(process.argv.slice(2)) + "\\n");\n`,
|
||||
);
|
||||
writeFileSync(cmdPath, `@echo off\r\n"${process.execPath}" "${scriptPath}" %*\r\n`);
|
||||
return { dir, cmdPath };
|
||||
}
|
||||
|
||||
function runCommandLine(command: string, args: string, cwd: string): Promise<string> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const terminal = pty.spawn(command, args, { cwd, cols: 500, rows: 30 });
|
||||
let output = "";
|
||||
const dataDisposable = terminal.onData((data) => {
|
||||
output += data;
|
||||
});
|
||||
const timeout = setTimeout(() => {
|
||||
terminal.kill();
|
||||
reject(new Error("Timed out waiting for cmd.exe fixture"));
|
||||
}, 10_000);
|
||||
timeout.unref();
|
||||
terminal.onExit(({ exitCode }) => {
|
||||
clearTimeout(timeout);
|
||||
dataDisposable.dispose();
|
||||
if (exitCode !== 0) {
|
||||
reject(new Error(`cmd.exe fixture exited ${exitCode}: ${output}`));
|
||||
return;
|
||||
}
|
||||
resolve(output);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
async function roundTrip(promptArg: string): Promise<string[]> {
|
||||
const { dir, cmdPath } = writeArgvEchoShim();
|
||||
const resolved = await resolveTerminalSpawnCommand("echoargv", [promptArg], {
|
||||
platform: "win32",
|
||||
env: process.env,
|
||||
resolveExecutable: async () => cmdPath,
|
||||
});
|
||||
if (typeof resolved.args !== "string") {
|
||||
throw new Error(`expected a cmd.exe command line, got ${JSON.stringify(resolved.args)}`);
|
||||
}
|
||||
const output = await runCommandLine(resolved.command, resolved.args, dir);
|
||||
const cleanOutput = stripVTControlCharacters(output);
|
||||
const markerIndex = cleanOutput.lastIndexOf(argvMarker);
|
||||
if (markerIndex === -1) {
|
||||
throw new Error(`expected argv marker, got ${output}`);
|
||||
}
|
||||
const argvJson = cleanOutput.slice(markerIndex + argvMarker.length).split(/\r?\n/, 1)[0];
|
||||
const argv: unknown = JSON.parse(argvJson);
|
||||
if (!Array.isArray(argv) || !argv.every((arg) => typeof arg === "string")) {
|
||||
throw new Error(`expected string argv, got ${output}`);
|
||||
}
|
||||
return argv;
|
||||
}
|
||||
|
||||
it("passes cmd.exe metacharacters through as literal text", async () => {
|
||||
expect(await roundTrip("a & b | c")).toEqual(["a & b | c"]);
|
||||
expect(await roundTrip("100% done %PATH%")).toEqual(["100% done %PATH%"]);
|
||||
expect(await roundTrip("a^b")).toEqual(["a^b"]);
|
||||
expect(await roundTrip('say "hi"')).toEqual(['say "hi"']);
|
||||
expect(await roundTrip("c:\\path\\to\\thing")).toEqual(["c:\\path\\to\\thing"]);
|
||||
});
|
||||
|
||||
it("does not let a metacharacter start a second command", async () => {
|
||||
expect(await roundTrip("safe & echo pwned")).toEqual(["safe & echo pwned"]);
|
||||
});
|
||||
|
||||
it("normalizes newlines to spaces rather than splitting the command", async () => {
|
||||
expect(await roundTrip("first\nsecond")).toEqual(["first second"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -247,7 +247,15 @@ export function resolveDefaultTerminalShell(
|
||||
|
||||
export interface ResolvedTerminalCommand {
|
||||
command: string;
|
||||
args: string[];
|
||||
// `.cmd`/`.bat` targets resolve to a single pre-escaped `cmd.exe` command
|
||||
// line (string) instead of an argv array. node-pty's own array-quoting
|
||||
// (see `argsToCommandLine` in its `windowsPtyAgent.js`) only produces
|
||||
// MSVCRT/CommandLineToArgvW-safe quoting; cmd.exe re-parses that whole
|
||||
// line itself afterwards and needs a different, cmd.exe-metachar-aware
|
||||
// escape. Building that line ourselves and passing it as a string is the
|
||||
// node-pty-documented way to bypass its array quoting for exactly this
|
||||
// case (see the `spawn` jsdoc in node-pty's typings).
|
||||
args: string[] | string;
|
||||
}
|
||||
|
||||
export interface ResolveTerminalSpawnCommandOptions {
|
||||
@@ -256,6 +264,66 @@ export interface ResolveTerminalSpawnCommandOptions {
|
||||
resolveExecutable?: (name: string) => Promise<string | null>;
|
||||
}
|
||||
|
||||
// cmd.exe treats these characters specially even inside a double-quoted
|
||||
// argument, unlike a POSIX shell — quoting alone does not neutralize them.
|
||||
// Prefixing each with `^`, cmd.exe's own escape character, does. Mirrors the
|
||||
// `escape.command` metachar list in the `cross-spawn` npm package (already a
|
||||
// dependency of this repo), which solves this exact `.cmd`/`.bat` spawning
|
||||
// problem — see http://www.robvanderwoude.com/escapechars.php.
|
||||
const CMD_EXE_METACHAR_PATTERN = /([()%!^"`<>&|;, *?])/g;
|
||||
|
||||
function escapeCmdExeMetaChars(text: string): string {
|
||||
return text.replace(CMD_EXE_METACHAR_PATTERN, "^$1");
|
||||
}
|
||||
|
||||
/**
|
||||
* Escape one argument for safe inclusion in a `cmd.exe /c "..."` command
|
||||
* line that is about to invoke a `.cmd`/`.bat` shim.
|
||||
*
|
||||
* node-pty builds an MSVCRT/`CommandLineToArgvW`-style quoted command line
|
||||
* for whatever `command` it is given (double-quote wrap on whitespace,
|
||||
* backslash-escape only backslash runs immediately before a quote). That
|
||||
* quoting is correct for a normal executable, which parses its own argv via
|
||||
* that same convention. It is not correct here: the `command` node-pty
|
||||
* launches for a `.cmd`/`.bat` target is `cmd.exe` itself, and cmd.exe
|
||||
* re-parses the whole line with its *own* tokenizer before ever handing
|
||||
* arguments to the batch script — a tokenizer that treats `&`, `|`, `^`,
|
||||
* `%` (environment-variable expansion), `<`, `>` etc. as live syntax even
|
||||
* inside double quotes. A prompt containing any of those characters would
|
||||
* therefore be interpreted by cmd.exe, not passed through as text — the
|
||||
* same class of hazard tracked upstream as Node's CVE-2024-27980
|
||||
* ("batBadBadging").
|
||||
*
|
||||
* The escaping below is the "qntm" algorithm (https://qntm.org/cmd): double
|
||||
* every backslash run that immediately precedes a quote (or the end of the
|
||||
* argument, since a closing quote is appended next) and escape that quote,
|
||||
* wrap the whole argument in quotes, then caret-escape cmd.exe's
|
||||
* metacharacters — including the quotes just added, because cmd.exe's own
|
||||
* tokenizer runs first and would otherwise treat them as toggling a quoted
|
||||
* region. It is the same algorithm `cross-spawn` uses for its `.cmd`/`.bat`
|
||||
* auto-shell path (`lib/util/escape.js`). Command shims forward `%*`, which
|
||||
* adds a second cmd.exe parse, so metacharacters need a second escape pass to
|
||||
* survive as literal argv in the Node process behind an npm-generated shim.
|
||||
*
|
||||
* cmd.exe has no escape for a literal CR or LF inside a single command
|
||||
* line — its parser treats them as command separators regardless of
|
||||
* quoting, so passing one through verbatim would let a crafted prompt
|
||||
* inject a second, attacker-chosen command after the newline. There is no
|
||||
* fidelity-preserving option for that case, so newlines are normalized to
|
||||
* spaces before quoting: the target process still receives the full text,
|
||||
* just without the original line breaks, instead of cmd.exe splitting the
|
||||
* prompt into multiple commands.
|
||||
*/
|
||||
function escapeCmdExeArgument(rawArg: string): string {
|
||||
const arg = rawArg.replace(/\r\n|\r|\n/g, " ");
|
||||
|
||||
let escaped = arg.replace(/(\\*)"/g, '$1$1\\"');
|
||||
escaped = escaped.replace(/(\\*)$/, "$1$1");
|
||||
escaped = `"${escaped}"`;
|
||||
|
||||
return escapeCmdExeMetaChars(escapeCmdExeMetaChars(escaped));
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a terminal profile command (e.g. `claude`) into something node-pty's
|
||||
* conpty backend can actually launch on Windows.
|
||||
@@ -297,7 +365,21 @@ export async function resolveTerminalSpawnCommand(
|
||||
if (extension === ".cmd" || extension === ".bat") {
|
||||
const env = options.env ?? process.env;
|
||||
const comSpec = env.ComSpec || env.COMSPEC || "C:\\Windows\\System32\\cmd.exe";
|
||||
return { command: comSpec, args: ["/c", resolved, ...args] };
|
||||
// Build the `/c` command line ourselves — as a single pre-escaped
|
||||
// string, not an argv array — so node-pty's own array quoting never
|
||||
// runs over these values; see escapeCmdExeArgument for why that
|
||||
// quoting alone is not safe once cmd.exe re-parses the line. `resolved`
|
||||
// gets the same metachar (not quote-wrap) treatment as the args: it
|
||||
// isn't attacker-controlled, but it now sits inside the same joined
|
||||
// command line and needs to survive cmd.exe's tokenizer too (e.g. a
|
||||
// space in the install path).
|
||||
const commandLine = [
|
||||
"/d",
|
||||
"/s",
|
||||
"/c",
|
||||
`"${[escapeCmdExeMetaChars(resolved), ...args.map(escapeCmdExeArgument)].join(" ")}"`,
|
||||
].join(" ");
|
||||
return { command: comSpec, args: commandLine };
|
||||
}
|
||||
|
||||
return { command: resolved, args };
|
||||
|
||||
Reference in New Issue
Block a user