diff --git a/CHANGELOG.md b/CHANGELOG.md index 76fcf6beea..fa7d47dcf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai - Security/Feishu: prevent path traversal in Feishu inbound media temp-file writes by replacing key-derived temp filenames with UUID-based names. Thanks @allsmog for reporting. - Security/Media: harden local media ingestion against TOCTOU/symlink swap attacks by pinning reads to a single file descriptor with symlink rejection and inode/device verification in `saveMediaSource`. Thanks @dorjoos for reporting. +- Security/Lobster (Windows): for the next npm release, remove shell-based fallback when launching Lobster wrappers (`.cmd`/`.bat`) and switch to explicit argv execution with wrapper entrypoint resolution, preventing command injection while preserving Windows wrapper compatibility. Thanks @tdjackey for reporting. - Agents/Streaming: keep assistant partial streaming active during reasoning streams, handle native `thinking_*` stream events consistently, dedupe mixed reasoning-end signals, and clear stale mutating tool errors after same-target retry success. (#20635) Thanks @obviyus. - Gateway/Auth: default unresolved gateway auth to token mode with startup auto-generation/persistence of `gateway.auth.token`, while allowing explicit `gateway.auth.mode: "none"` for intentional open loopback setups. (#20686) thanks @gumadeiras. - Browser/Relay: require gateway-token auth on both `/extension` and `/cdp`, and align Chrome extension setup to use a single `gateway.auth.token` input for relay authentication. Thanks @tdjackey for reporting. diff --git a/extensions/lobster/src/lobster-tool.test.ts b/extensions/lobster/src/lobster-tool.test.ts index 50971e48ba..13b0a4c727 100644 --- a/extensions/lobster/src/lobster-tool.test.ts +++ b/extensions/lobster/src/lobster-tool.test.ts @@ -3,7 +3,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { PassThrough } from "node:stream"; -import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawPluginApi, OpenClawPluginToolContext } from "../../../src/plugins/types.js"; const spawnState = vi.hoisted(() => ({ @@ -57,16 +57,57 @@ function fakeCtx(overrides: Partial = {}): OpenClawPl }; } +function setProcessPlatform(platform: NodeJS.Platform) { + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }); +} + describe("lobster plugin tool", () => { let tempDir = ""; let lobsterBinPath = ""; + let lobsterExePath = ""; + const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); + const originalPath = process.env.PATH; + const originalPathAlt = process.env.Path; + const originalPathExt = process.env.PATHEXT; + const originalPathExtAlt = process.env.Pathext; beforeAll(async () => { ({ createLobsterTool } = await import("./lobster-tool.js")); tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lobster-plugin-")); lobsterBinPath = path.join(tempDir, process.platform === "win32" ? "lobster.cmd" : "lobster"); + lobsterExePath = path.join(tempDir, "lobster.exe"); await fs.writeFile(lobsterBinPath, "", { encoding: "utf8", mode: 0o755 }); + await fs.writeFile(lobsterExePath, "", { encoding: "utf8", mode: 0o755 }); + }); + + afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, "platform", originalPlatform); + } + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + if (originalPathAlt === undefined) { + delete process.env.Path; + } else { + process.env.Path = originalPathAlt; + } + if (originalPathExt === undefined) { + delete process.env.PATHEXT; + } else { + process.env.PATHEXT = originalPathExt; + } + if (originalPathExtAlt === undefined) { + delete process.env.Pathext; + } else { + process.env.Pathext = originalPathExtAlt; + } }); afterAll(async () => { @@ -226,6 +267,156 @@ describe("lobster plugin tool", () => { ).rejects.toThrow(/invalid JSON/); }); + it("runs Windows cmd shims through Node without enabling shell", async () => { + setProcessPlatform("win32"); + const shimScriptPath = path.join(tempDir, "shim-dist", "lobster-cli.cjs"); + const shimPath = path.join(tempDir, "shim", "lobster.cmd"); + await fs.mkdir(path.dirname(shimScriptPath), { recursive: true }); + await fs.mkdir(path.dirname(shimPath), { recursive: true }); + await fs.writeFile(shimScriptPath, "module.exports = {};\n", "utf8"); + await fs.writeFile( + shimPath, + `@echo off\r\n"%dp0%\\..\\shim-dist\\lobster-cli.cjs" %*\r\n`, + "utf8", + ); + spawnState.queue.push({ + stdout: JSON.stringify({ + ok: true, + status: "ok", + output: [{ hello: "world" }], + requiresApproval: null, + }), + }); + + const tool = createLobsterTool(fakeApi({ pluginConfig: { lobsterPath: shimPath } })); + await tool.execute("call-win-shim", { + action: "run", + pipeline: "noop", + }); + + const [command, argv, options] = spawnState.spawn.mock.calls[0] ?? []; + expect(command).toBe(process.execPath); + expect(argv).toEqual([shimScriptPath, "run", "--mode", "tool", "noop"]); + expect(options).toMatchObject({ windowsHide: true }); + expect(options).not.toHaveProperty("shell"); + }); + + it("ignores node.exe shim entries and resolves the actual lobster script", async () => { + setProcessPlatform("win32"); + const shimDir = path.join(tempDir, "shim-with-node"); + const nodeExePath = path.join(shimDir, "node.exe"); + const scriptPath = path.join(tempDir, "shim-dist-node", "lobster-cli.cjs"); + const shimPath = path.join(shimDir, "lobster.cmd"); + await fs.mkdir(path.dirname(scriptPath), { recursive: true }); + await fs.mkdir(shimDir, { recursive: true }); + await fs.writeFile(nodeExePath, "", "utf8"); + await fs.writeFile(scriptPath, "module.exports = {};\n", "utf8"); + await fs.writeFile( + shimPath, + `@echo off\r\n"%~dp0%\\node.exe" "%~dp0%\\..\\shim-dist-node\\lobster-cli.cjs" %*\r\n`, + "utf8", + ); + spawnState.queue.push({ + stdout: JSON.stringify({ + ok: true, + status: "ok", + output: [{ hello: "node-first" }], + requiresApproval: null, + }), + }); + + const tool = createLobsterTool(fakeApi({ pluginConfig: { lobsterPath: shimPath } })); + await tool.execute("call-win-node-first", { + action: "run", + pipeline: "noop", + }); + + const [command, argv] = spawnState.spawn.mock.calls[0] ?? []; + expect(command).toBe(process.execPath); + expect(argv).toEqual([scriptPath, "run", "--mode", "tool", "noop"]); + }); + + it("resolves lobster.cmd from PATH and unwraps npm layout shim", async () => { + setProcessPlatform("win32"); + const binDir = path.join(tempDir, "node_modules", ".bin"); + const packageDir = path.join(tempDir, "node_modules", "lobster"); + const scriptPath = path.join(packageDir, "dist", "cli.js"); + const shimPath = path.join(binDir, "lobster.cmd"); + await fs.mkdir(path.dirname(scriptPath), { recursive: true }); + await fs.mkdir(binDir, { recursive: true }); + await fs.writeFile(shimPath, "@echo off\r\n", "utf8"); + await fs.writeFile( + path.join(packageDir, "package.json"), + JSON.stringify({ name: "lobster", version: "0.0.0", bin: { lobster: "dist/cli.js" } }), + "utf8", + ); + await fs.writeFile(scriptPath, "module.exports = {};\n", "utf8"); + process.env.PATHEXT = ".CMD;.EXE"; + process.env.PATH = `${binDir};${process.env.PATH ?? ""}`; + + spawnState.queue.push({ + stdout: JSON.stringify({ + ok: true, + status: "ok", + output: [{ hello: "path" }], + requiresApproval: null, + }), + }); + + const tool = createLobsterTool(fakeApi()); + await tool.execute("call-win-path", { + action: "run", + pipeline: "noop", + }); + + const [command, argv] = spawnState.spawn.mock.calls[0] ?? []; + expect(command).toBe(process.execPath); + expect(argv).toEqual([scriptPath, "run", "--mode", "tool", "noop"]); + }); + + it("fails fast when cmd wrapper cannot be resolved without shell execution", async () => { + setProcessPlatform("win32"); + const badShimPath = path.join(tempDir, "bad-shim", "lobster.cmd"); + await fs.mkdir(path.dirname(badShimPath), { recursive: true }); + await fs.writeFile(badShimPath, "@echo off\r\nREM no entrypoint\r\n", "utf8"); + + const tool = createLobsterTool(fakeApi({ pluginConfig: { lobsterPath: badShimPath } })); + await expect( + tool.execute("call-win-bad", { + action: "run", + pipeline: "noop", + }), + ).rejects.toThrow(/without shell execution/); + expect(spawnState.spawn).not.toHaveBeenCalled(); + }); + + it("does not retry a failed Windows spawn with shell fallback", async () => { + setProcessPlatform("win32"); + spawnState.spawn.mockReset(); + spawnState.spawn.mockImplementationOnce(() => { + const child = new EventEmitter() as EventEmitter & { + stdout: PassThrough; + stderr: PassThrough; + kill: (signal?: string) => boolean; + }; + child.stdout = new PassThrough(); + child.stderr = new PassThrough(); + child.kill = () => true; + const err = Object.assign(new Error("spawn failed"), { code: "ENOENT" }); + setImmediate(() => child.emit("error", err)); + return child; + }); + + const tool = createLobsterTool(fakeApi({ pluginConfig: { lobsterPath: lobsterExePath } })); + await expect( + tool.execute("call-win-no-retry", { + action: "run", + pipeline: "noop", + }), + ).rejects.toThrow(/spawn failed/); + expect(spawnState.spawn).toHaveBeenCalledTimes(1); + }); + it("can be gated off in sandboxed contexts", async () => { const api = fakeApi(); const factoryTool = (ctx: OpenClawPluginToolContext) => { diff --git a/extensions/lobster/src/lobster-tool.ts b/extensions/lobster/src/lobster-tool.ts index b34bea6128..ff2c246f6c 100644 --- a/extensions/lobster/src/lobster-tool.ts +++ b/extensions/lobster/src/lobster-tool.ts @@ -1,7 +1,7 @@ +import { Type } from "@sinclair/typebox"; import { spawn } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; -import { Type } from "@sinclair/typebox"; import type { OpenClawPluginApi } from "../../../src/plugins/types.js"; type LobsterEnvelope = @@ -84,28 +84,155 @@ function resolveCwd(cwdRaw: unknown): string { return resolved; } -function isWindowsSpawnErrorThatCanUseShell(err: unknown) { - if (!err || typeof err !== "object") { +function isFilePath(value: string): boolean { + try { + const stat = fs.statSync(value); + return stat.isFile(); + } catch { return false; } - const code = (err as { code?: unknown }).code; - - // On Windows, spawning scripts discovered on PATH (e.g. lobster.cmd) can fail - // with EINVAL, and PATH discovery itself can fail with ENOENT when the binary - // is only available via PATHEXT/script wrappers. - return code === "EINVAL" || code === "ENOENT"; } -async function runLobsterSubprocessOnce( - params: { - execPath: string; - argv: string[]; - cwd: string; - timeoutMs: number; - maxStdoutBytes: number; - }, - useShell: boolean, -) { +function resolveWindowsExecutablePath(execPath: string, env: NodeJS.ProcessEnv): string { + if (execPath.includes("/") || execPath.includes("\\") || path.isAbsolute(execPath)) { + return execPath; + } + const pathValue = env.PATH ?? env.Path ?? process.env.PATH ?? process.env.Path ?? ""; + const pathEntries = pathValue + .split(";") + .map((entry) => entry.trim()) + .filter(Boolean); + const hasExtension = path.extname(execPath).length > 0; + const pathExtRaw = + env.PATHEXT ?? + env.Pathext ?? + process.env.PATHEXT ?? + process.env.Pathext ?? + ".EXE;.CMD;.BAT;.COM"; + const pathExt = hasExtension + ? [""] + : pathExtRaw + .split(";") + .map((ext) => ext.trim()) + .filter(Boolean) + .map((ext) => (ext.startsWith(".") ? ext : `.${ext}`)); + for (const dir of pathEntries) { + for (const ext of pathExt) { + for (const candidateExt of [ext, ext.toLowerCase(), ext.toUpperCase()]) { + const candidate = path.join(dir, `${execPath}${candidateExt}`); + if (isFilePath(candidate)) { + return candidate; + } + } + } + } + return execPath; +} + +function resolveLobsterScriptFromPackageJson(wrapperPath: string): string | null { + const wrapperDir = path.dirname(wrapperPath); + const packageDirs = [ + // Local install: /node_modules/.bin/lobster.cmd -> ../lobster + path.resolve(wrapperDir, "..", "lobster"), + // Global npm install: /lobster.cmd -> ./node_modules/lobster + path.resolve(wrapperDir, "node_modules", "lobster"), + ]; + for (const packageDir of packageDirs) { + const packageJsonPath = path.join(packageDir, "package.json"); + if (!isFilePath(packageJsonPath)) { + continue; + } + try { + const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf8")) as { + bin?: string | Record; + }; + const binField = packageJson.bin; + const scriptRel = + typeof binField === "string" + ? binField + : typeof binField === "object" && binField + ? typeof binField.lobster === "string" + ? binField.lobster + : (() => { + const first = Object.values(binField).find((value) => typeof value === "string"); + return typeof first === "string" ? first : null; + })() + : null; + if (!scriptRel) { + continue; + } + const scriptPath = path.resolve(packageDir, scriptRel); + if (isFilePath(scriptPath)) { + return scriptPath; + } + } catch { + // Ignore malformed package metadata; caller will throw a guided error. + } + } + return null; +} + +function resolveLobsterScriptFromCmdShim(wrapperPath: string): string | null { + if (!isFilePath(wrapperPath)) { + return null; + } + try { + const content = fs.readFileSync(wrapperPath, "utf8"); + // npm-style cmd shims usually reference the script as "%dp0%\\...". + const candidates: string[] = []; + const matches = content.matchAll(/"%~?dp0%\\([^"\r\n]+)"/gi); + for (const match of matches) { + const relative = match[1]; + if (!relative) { + continue; + } + const normalizedRelative = relative.replace(/[\\/]+/g, path.sep); + const candidate = path.resolve(path.dirname(wrapperPath), normalizedRelative); + if (isFilePath(candidate)) { + candidates.push(candidate); + } + } + const nonNode = candidates.find((candidate) => { + const base = path.basename(candidate).toLowerCase(); + return base !== "node.exe" && base !== "node"; + }); + if (nonNode) { + return nonNode; + } + } catch { + // Ignore unreadable shims; caller will throw a guided error. + } + return null; +} + +function resolveWindowsLobsterSpawn(execPath: string, argv: string[], env: NodeJS.ProcessEnv) { + const resolvedExecPath = resolveWindowsExecutablePath(execPath, env); + const ext = path.extname(resolvedExecPath).toLowerCase(); + if (ext !== ".cmd" && ext !== ".bat") { + return { command: resolvedExecPath, argv }; + } + const scriptPath = + resolveLobsterScriptFromCmdShim(resolvedExecPath) ?? + resolveLobsterScriptFromPackageJson(resolvedExecPath); + if (!scriptPath) { + throw new Error( + `lobsterPath resolved to ${path.basename(resolvedExecPath)} wrapper, but no Node entrypoint could be resolved without shell execution. Configure pluginConfig.lobsterPath to lobster.exe.`, + ); + } + const entryExt = path.extname(scriptPath).toLowerCase(); + if (entryExt === ".exe") { + return { command: scriptPath, argv, windowsHide: true }; + } + return { command: process.execPath, argv: [scriptPath, ...argv], windowsHide: true }; +} + +async function runLobsterSubprocessOnce(params: { + execPath: string; + argv: string[]; + cwd: string; + timeoutMs: number; + maxStdoutBytes: number; +}) { const { execPath, argv, cwd } = params; const timeoutMs = Math.max(200, params.timeoutMs); const maxStdoutBytes = Math.max(1024, params.maxStdoutBytes); @@ -115,14 +242,17 @@ async function runLobsterSubprocessOnce( if (nodeOptions.includes("--inspect")) { delete env.NODE_OPTIONS; } + const spawnTarget = + process.platform === "win32" + ? resolveWindowsLobsterSpawn(execPath, argv, env) + : { command: execPath, argv }; return await new Promise<{ stdout: string }>((resolve, reject) => { - const child = spawn(execPath, argv, { + const child = spawn(spawnTarget.command, spawnTarget.argv, { cwd, stdio: ["ignore", "pipe", "pipe"], env, - shell: useShell, - windowsHide: useShell ? true : undefined, + windowsHide: spawnTarget.windowsHide, }); let stdout = ""; @@ -181,14 +311,7 @@ async function runLobsterSubprocess(params: { timeoutMs: number; maxStdoutBytes: number; }) { - try { - return await runLobsterSubprocessOnce(params, false); - } catch (err) { - if (process.platform === "win32" && isWindowsSpawnErrorThatCanUseShell(err)) { - return await runLobsterSubprocessOnce(params, true); - } - throw err; - } + return await runLobsterSubprocessOnce(params); } function parseEnvelope(stdout: string): LobsterEnvelope {