From bbb5fbc71f1a70dd4c0aaa748b70f353c347b9d9 Mon Sep 17 00:00:00 2001 From: Sebastian <19554889+sebslight@users.noreply.github.com> Date: Mon, 16 Feb 2026 20:48:55 -0500 Subject: [PATCH] fix(scripts): harden Windows UI spawn behavior --- CHANGELOG.md | 1 + scripts/ui.js | 139 +++++++++++++++++++++++++++++----------- test/scripts/ui.test.ts | 35 ++++++++++ 3 files changed, 136 insertions(+), 39 deletions(-) create mode 100644 test/scripts/ui.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c54040e905..7915810116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Scripts/UI/Windows: fix `pnpm ui:*` spawn `EINVAL` failures by restoring shell-backed launch for `.cmd`/`.bat` runners, narrowing shell usage to launcher types that require it, and rejecting unsafe forwarded shell metacharacters in UI script args. (#18594) - Hooks/Session-memory: recover `/new` conversation summaries when session pointers are reset-path or missing `sessionFile`, and consistently prefer the newest `.jsonl.reset.*` transcript candidate for fallback extraction. (#18088) - Slack: restrict forwarded-attachment ingestion to explicit shared-message attachments and skip non-Slack forwarded `image_url` fetches, preventing non-forward attachment unfurls from polluting inbound agent context while preserving forwarded message handling. - Cron/Heartbeat: canonicalize session-scoped reminder `sessionKey` routing and preserve explicit flat `sessionKey` cron tool inputs, preventing enqueue/wake namespace drift for session-targeted reminders. (#18637) Thanks @vignesh07. diff --git a/scripts/ui.js b/scripts/ui.js index 18f564b75a..dbf624f6cd 100644 --- a/scripts/ui.js +++ b/scripts/ui.js @@ -9,6 +9,9 @@ const here = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(here, ".."); const uiDir = path.join(repoRoot, "ui"); +const WINDOWS_SHELL_EXTENSIONS = new Set([".cmd", ".bat", ".com"]); +const WINDOWS_UNSAFE_SHELL_ARG_PATTERN = /[\r\n"&|<>^%!]/; + function usage() { // keep this tiny; it's invoked from npm scripts too process.stderr.write("Usage: node scripts/ui.js [...args]\n"); @@ -50,14 +53,52 @@ function resolveRunner() { return null; } -function run(cmd, args) { - const isWindows = process.platform === "win32"; // Windows support - const child = spawn(cmd, args, { +export function shouldUseShellForCommand(cmd, platform = process.platform) { + if (platform !== "win32") { + return false; + } + const extension = path.extname(cmd).toLowerCase(); + return WINDOWS_SHELL_EXTENSIONS.has(extension); +} + +export function assertSafeWindowsShellArgs(args, platform = process.platform) { + if (platform !== "win32") { + return; + } + const unsafeArg = args.find((arg) => WINDOWS_UNSAFE_SHELL_ARG_PATTERN.test(arg)); + if (!unsafeArg) { + return; + } + // SECURITY: `shell: true` routes through cmd.exe; reject risky metacharacters + // in forwarded args to prevent shell control-flow/env-expansion injection. + throw new Error( + `Unsafe Windows shell argument: ${unsafeArg}. Remove shell metacharacters (" & | < > ^ % !).`, + ); +} + +function createSpawnOptions(cmd, args, envOverride) { + const useShell = shouldUseShellForCommand(cmd); + if (useShell) { + assertSafeWindowsShellArgs(args); + } + return { cwd: uiDir, stdio: "inherit", - env: process.env, - shell: isWindows, - }); + env: envOverride ?? process.env, + ...(useShell ? { shell: true } : {}), + }; +} + +function run(cmd, args) { + let child; + try { + child = spawn(cmd, args, createSpawnOptions(cmd, args)); + } catch (err) { + console.error(`Failed to launch ${cmd}:`, err); + process.exit(1); + return; + } + child.on("error", (err) => { console.error(`Failed to launch ${cmd}:`, err); process.exit(1); @@ -70,13 +111,14 @@ function run(cmd, args) { } function runSync(cmd, args, envOverride) { - const isWindows = process.platform === "win32"; // Windows support - const result = spawnSync(cmd, args, { - cwd: uiDir, - stdio: "inherit", - env: envOverride ?? process.env, - shell: isWindows, - }); + let result; + try { + result = spawnSync(cmd, args, createSpawnOptions(cmd, args, envOverride)); + } catch (err) { + console.error(`Failed to launch ${cmd}:`, err); + process.exit(1); + return; + } if (result.signal) { process.exit(1); } @@ -101,42 +143,61 @@ function depsInstalled(kind) { } } -const [, , action, ...rest] = process.argv; -if (!action) { - usage(); - process.exit(2); +function resolveScriptAction(action) { + if (action === "install") { + return null; + } + if (action === "dev") { + return "dev"; + } + if (action === "build") { + return "build"; + } + if (action === "test") { + return "test"; + } + return null; } -const runner = resolveRunner(); -if (!runner) { - process.stderr.write("Missing UI runner: install pnpm, then retry.\n"); - process.exit(1); -} +export function main(argv = process.argv.slice(2)) { + const [action, ...rest] = argv; + if (!action) { + usage(); + process.exit(2); + } -const script = - action === "install" - ? null - : action === "dev" - ? "dev" - : action === "build" - ? "build" - : action === "test" - ? "test" - : null; + const runner = resolveRunner(); + if (!runner) { + process.stderr.write("Missing UI runner: install pnpm, then retry.\n"); + process.exit(1); + } -if (action !== "install" && !script) { - usage(); - process.exit(2); -} + const script = resolveScriptAction(action); + if (action !== "install" && !script) { + usage(); + process.exit(2); + } + + if (action === "install") { + run(runner.cmd, ["install", ...rest]); + return; + } -if (action === "install") { - run(runner.cmd, ["install", ...rest]); -} else { if (!depsInstalled(action === "test" ? "test" : "build")) { const installEnv = action === "build" ? { ...process.env, NODE_ENV: "production" } : process.env; const installArgs = action === "build" ? ["install", "--prod"] : ["install"]; runSync(runner.cmd, installArgs, installEnv); } + run(runner.cmd, ["run", script, ...rest]); } + +const isDirectExecution = (() => { + const entry = process.argv[1]; + return Boolean(entry && path.resolve(entry) === fileURLToPath(import.meta.url)); +})(); + +if (isDirectExecution) { + main(); +} diff --git a/test/scripts/ui.test.ts b/test/scripts/ui.test.ts new file mode 100644 index 0000000000..170d964f36 --- /dev/null +++ b/test/scripts/ui.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { assertSafeWindowsShellArgs, shouldUseShellForCommand } from "../../scripts/ui.js"; + +describe("scripts/ui windows spawn behavior", () => { + it("enables shell for Windows command launchers that require cmd.exe", () => { + expect( + shouldUseShellForCommand("C:\\Users\\dev\\AppData\\Local\\pnpm\\pnpm.CMD", "win32"), + ).toBe(true); + expect(shouldUseShellForCommand("C:\\tools\\pnpm.bat", "win32")).toBe(true); + }); + + it("does not enable shell for non-shell launchers", () => { + expect(shouldUseShellForCommand("C:\\Program Files\\nodejs\\node.exe", "win32")).toBe(false); + expect(shouldUseShellForCommand("/usr/local/bin/pnpm", "linux")).toBe(false); + }); + + it("allows safe forwarded args when shell mode is required on Windows", () => { + expect(() => + assertSafeWindowsShellArgs(["run", "build", "--filter", "@openclaw/ui"], "win32"), + ).not.toThrow(); + }); + + it("rejects dangerous forwarded args when shell mode is required on Windows", () => { + expect(() => assertSafeWindowsShellArgs(["run", "build", "evil&calc"], "win32")).toThrow( + /unsafe windows shell argument/i, + ); + expect(() => assertSafeWindowsShellArgs(["run", "build", "%PATH%"], "win32")).toThrow( + /unsafe windows shell argument/i, + ); + }); + + it("does not reject args on non-windows platforms", () => { + expect(() => assertSafeWindowsShellArgs(["contains&metacharacters"], "linux")).not.toThrow(); + }); +});