diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bab68dae3..e8ecaa072b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Telegram: restore draft streaming partials. (#5543) Thanks @obviyus. +- fix(lobster): block arbitrary exec via lobsterPath/cwd injection (GHSA-4mhr-g7xj-cg8j). (#5335) Thanks @vignesh07. ## 2026.1.30 diff --git a/extensions/lobster/src/lobster-tool.test.ts b/extensions/lobster/src/lobster-tool.test.ts index 26fa37c451..bbab896ef9 100644 --- a/extensions/lobster/src/lobster-tool.test.ts +++ b/extensions/lobster/src/lobster-tool.test.ts @@ -33,12 +33,13 @@ async function writeFakeLobster(params: { payload: unknown }) { return await writeFakeLobsterScript(scriptBody); } -function fakeApi(): OpenClawPluginApi { +function fakeApi(overrides: Partial = {}): OpenClawPluginApi { return { id: "lobster", name: "lobster", source: "test", config: {} as any, + pluginConfig: {}, runtime: { version: "test" } as any, logger: { info() {}, warn() {}, error() {}, debug() {} }, registerTool() {}, @@ -48,7 +49,12 @@ function fakeApi(): OpenClawPluginApi { registerCli() {}, registerService() {}, registerProvider() {}, + registerHook() {}, + registerHttpRoute() {}, + registerCommand() {}, + on() {}, resolvePath: (p) => p, + ...overrides, }; } @@ -72,62 +78,159 @@ describe("lobster plugin tool", () => { payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null }, }); - const tool = createLobsterTool(fakeApi()); - const res = await tool.execute("call1", { - action: "run", - pipeline: "noop", - lobsterPath: fake.binPath, - timeoutMs: 1000, - }); + const originalPath = process.env.PATH; + process.env.PATH = `${fake.dir}${path.delimiter}${originalPath ?? ""}`; - expect(res.details).toMatchObject({ ok: true, status: "ok" }); + try { + const tool = createLobsterTool(fakeApi()); + const res = await tool.execute("call1", { + action: "run", + pipeline: "noop", + timeoutMs: 1000, + }); + + expect(res.details).toMatchObject({ ok: true, status: "ok" }); + } finally { + process.env.PATH = originalPath; + } }); it("tolerates noisy stdout before the JSON envelope", async () => { const payload = { ok: true, status: "ok", output: [], requiresApproval: null }; - const { binPath } = await writeFakeLobsterScript( + const { dir } = await writeFakeLobsterScript( `const payload = ${JSON.stringify(payload)};\n` + `console.log("noise before json");\n` + `process.stdout.write(JSON.stringify(payload));\n`, "openclaw-lobster-plugin-noisy-", ); - const tool = createLobsterTool(fakeApi()); - const res = await tool.execute("call-noisy", { - action: "run", - pipeline: "noop", - lobsterPath: binPath, - timeoutMs: 1000, - }); + const originalPath = process.env.PATH; + process.env.PATH = `${dir}${path.delimiter}${originalPath ?? ""}`; - expect(res.details).toMatchObject({ ok: true, status: "ok" }); - }); - - it("requires absolute lobsterPath when provided", async () => { - const tool = createLobsterTool(fakeApi()); - await expect( - tool.execute("call2", { + try { + const tool = createLobsterTool(fakeApi()); + const res = await tool.execute("call-noisy", { action: "run", pipeline: "noop", - lobsterPath: "./lobster", + timeoutMs: 1000, + }); + + expect(res.details).toMatchObject({ ok: true, status: "ok" }); + } finally { + process.env.PATH = originalPath; + } + }); + + it("requires absolute lobsterPath when provided (even though it is ignored)", async () => { + const fake = await writeFakeLobster({ + payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null }, + }); + + const originalPath = process.env.PATH; + process.env.PATH = `${fake.dir}${path.delimiter}${originalPath ?? ""}`; + + try { + const tool = createLobsterTool(fakeApi()); + await expect( + tool.execute("call2", { + action: "run", + pipeline: "noop", + lobsterPath: "./lobster", + }), + ).rejects.toThrow(/absolute path/); + } finally { + process.env.PATH = originalPath; + } + }); + + it("rejects lobsterPath (deprecated) when invalid", async () => { + const fake = await writeFakeLobster({ + payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null }, + }); + + const originalPath = process.env.PATH; + process.env.PATH = `${fake.dir}${path.delimiter}${originalPath ?? ""}`; + + try { + const tool = createLobsterTool(fakeApi()); + await expect( + tool.execute("call2b", { + action: "run", + pipeline: "noop", + lobsterPath: "/bin/bash", + }), + ).rejects.toThrow(/lobster executable/); + } finally { + process.env.PATH = originalPath; + } + }); + + it("rejects absolute cwd", async () => { + const tool = createLobsterTool(fakeApi()); + await expect( + tool.execute("call2c", { + action: "run", + pipeline: "noop", + cwd: "/tmp", }), - ).rejects.toThrow(/absolute path/); + ).rejects.toThrow(/cwd must be a relative path/); + }); + + it("rejects cwd that escapes the gateway working directory", async () => { + const tool = createLobsterTool(fakeApi()); + await expect( + tool.execute("call2d", { + action: "run", + pipeline: "noop", + cwd: "../../etc", + }), + ).rejects.toThrow(/must stay within/); + }); + + it("uses pluginConfig.lobsterPath when provided", async () => { + const fake = await writeFakeLobster({ + payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null }, + }); + + // Ensure `lobster` is NOT discoverable via PATH, while still allowing our + // fake lobster (a Node script with `#!/usr/bin/env node`) to run. + const originalPath = process.env.PATH; + process.env.PATH = path.dirname(process.execPath); + + try { + const tool = createLobsterTool(fakeApi({ pluginConfig: { lobsterPath: fake.binPath } })); + const res = await tool.execute("call-plugin-config", { + action: "run", + pipeline: "noop", + timeoutMs: 1000, + }); + + expect(res.details).toMatchObject({ ok: true, status: "ok" }); + } finally { + process.env.PATH = originalPath; + } }); it("rejects invalid JSON from lobster", async () => { - const { binPath } = await writeFakeLobsterScript( + const { dir } = await writeFakeLobsterScript( `process.stdout.write("nope");\n`, "openclaw-lobster-plugin-bad-", ); - const tool = createLobsterTool(fakeApi()); - await expect( - tool.execute("call3", { - action: "run", - pipeline: "noop", - lobsterPath: binPath, - }), - ).rejects.toThrow(/invalid JSON/); + const originalPath = process.env.PATH; + process.env.PATH = `${dir}${path.delimiter}${originalPath ?? ""}`; + + try { + const tool = createLobsterTool(fakeApi()); + await expect( + tool.execute("call3", { + action: "run", + pipeline: "noop", + }), + ).rejects.toThrow(/invalid JSON/); + } finally { + process.env.PATH = originalPath; + } }); it("can be gated off in sandboxed contexts", async () => { diff --git a/extensions/lobster/src/lobster-tool.ts b/extensions/lobster/src/lobster-tool.ts index 5b8998fee4..dd510e2992 100644 --- a/extensions/lobster/src/lobster-tool.ts +++ b/extensions/lobster/src/lobster-tool.ts @@ -1,5 +1,6 @@ import { Type } from "@sinclair/typebox"; import { spawn } from "node:child_process"; +import fs from "node:fs"; import path from "node:path"; import type { OpenClawPluginApi } from "../../../src/plugins/types.js"; @@ -23,18 +24,77 @@ type LobsterEnvelope = function resolveExecutablePath(lobsterPathRaw: string | undefined) { const lobsterPath = lobsterPathRaw?.trim() || "lobster"; - if (lobsterPath !== "lobster" && !path.isAbsolute(lobsterPath)) { - throw new Error("lobsterPath must be an absolute path (or omit to use PATH)"); + + // SECURITY: + // Never allow arbitrary executables (e.g. /bin/bash). If the caller overrides + // the path, it must still be the lobster binary (by name) and be absolute. + if (lobsterPath !== "lobster") { + if (!path.isAbsolute(lobsterPath)) { + throw new Error("lobsterPath must be an absolute path (or omit to use PATH)"); + } + const base = path.basename(lobsterPath).toLowerCase(); + const allowed = + process.platform === "win32" ? ["lobster.exe", "lobster.cmd", "lobster.bat"] : ["lobster"]; + if (!allowed.includes(base)) { + throw new Error("lobsterPath must point to the lobster executable"); + } + let stat: fs.Stats; + try { + stat = fs.statSync(lobsterPath); + } catch { + throw new Error("lobsterPath must exist"); + } + if (!stat.isFile()) { + throw new Error("lobsterPath must point to a file"); + } + if (process.platform !== "win32") { + try { + fs.accessSync(lobsterPath, fs.constants.X_OK); + } catch { + throw new Error("lobsterPath must be executable"); + } + } } + return lobsterPath; } -function isWindowsSpawnEINVAL(err: unknown) { +function normalizeForCwdSandbox(p: string): string { + const normalized = path.normalize(p); + return process.platform === "win32" ? normalized.toLowerCase() : normalized; +} + +function resolveCwd(cwdRaw: unknown): string { + if (typeof cwdRaw !== "string" || !cwdRaw.trim()) { + return process.cwd(); + } + const cwd = cwdRaw.trim(); + if (path.isAbsolute(cwd)) { + throw new Error("cwd must be a relative path"); + } + const base = process.cwd(); + const resolved = path.resolve(base, cwd); + + const rel = path.relative(normalizeForCwdSandbox(base), normalizeForCwdSandbox(resolved)); + if (rel === "" || rel === ".") { + return resolved; + } + if (rel.startsWith("..") || path.isAbsolute(rel)) { + throw new Error("cwd must stay within the gateway working directory"); + } + return resolved; +} + +function isWindowsSpawnErrorThatCanUseShell(err: unknown) { if (!err || typeof err !== "object") { return false; } const code = (err as { code?: unknown }).code; - return code === "EINVAL"; + + // 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( @@ -125,7 +185,7 @@ async function runLobsterSubprocess(params: { try { return await runLobsterSubprocessOnce(params, false); } catch (err) { - if (process.platform === "win32" && isWindowsSpawnEINVAL(err)) { + if (process.platform === "win32" && isWindowsSpawnErrorThatCanUseShell(err)) { return await runLobsterSubprocessOnce(params, true); } throw err; @@ -182,8 +242,17 @@ export function createLobsterTool(api: OpenClawPluginApi) { argsJson: Type.Optional(Type.String()), token: Type.Optional(Type.String()), approve: Type.Optional(Type.Boolean()), - lobsterPath: Type.Optional(Type.String()), - cwd: Type.Optional(Type.String()), + // SECURITY: Do not allow the agent to choose an executable path. + // Host can configure the lobster binary via plugin config. + lobsterPath: Type.Optional( + Type.String({ description: "(deprecated) Use plugin config instead." }), + ), + cwd: Type.Optional( + Type.String({ + description: + "Relative working directory (optional). Must stay within the gateway working directory.", + }), + ), timeoutMs: Type.Optional(Type.Number()), maxStdoutBytes: Type.Optional(Type.Number()), }), @@ -193,11 +262,19 @@ export function createLobsterTool(api: OpenClawPluginApi) { throw new Error("action required"); } + // SECURITY: never allow tool callers (agent/user) to select executables. + // If a host needs to override the binary, it must do so via plugin config. + // We still validate the parameter shape to prevent reintroducing an RCE footgun. + if (typeof params.lobsterPath === "string" && params.lobsterPath.trim()) { + resolveExecutablePath(params.lobsterPath); + } + const execPath = resolveExecutablePath( - typeof params.lobsterPath === "string" ? params.lobsterPath : undefined, + typeof api.pluginConfig?.lobsterPath === "string" + ? api.pluginConfig.lobsterPath + : undefined, ); - const cwd = - typeof params.cwd === "string" && params.cwd.trim() ? params.cwd.trim() : process.cwd(); + const cwd = resolveCwd(params.cwd); const timeoutMs = typeof params.timeoutMs === "number" ? params.timeoutMs : 20_000; const maxStdoutBytes = typeof params.maxStdoutBytes === "number" ? params.maxStdoutBytes : 512_000;