From 31577f5378f40de034bf70c281ff5ee0b96b107d Mon Sep 17 00:00:00 2001 From: vignesh07 Date: Sat, 31 Jan 2026 03:01:23 -0800 Subject: [PATCH] fix(lobster): ignore tool-provided lobsterPath; validate + use plugin config --- extensions/lobster/src/lobster-tool.test.ts | 128 +++++++++++++------- extensions/lobster/src/lobster-tool.ts | 13 +- 2 files changed, 95 insertions(+), 46 deletions(-) diff --git a/extensions/lobster/src/lobster-tool.test.ts b/extensions/lobster/src/lobster-tool.test.ts index 98ce06765cbd..4fba9cd229df 100644 --- a/extensions/lobster/src/lobster-tool.test.ts +++ b/extensions/lobster/src/lobster-tool.test.ts @@ -72,57 +72,91 @@ 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 ?? ""}`; + + try { + const tool = createLobsterTool(fakeApi()); + const res = await tool.execute("call-noisy", { + action: "run", + pipeline: "noop", + 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 }, }); - expect(res.details).toMatchObject({ ok: true, status: "ok" }); + 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("requires absolute lobsterPath when provided", async () => { - const tool = createLobsterTool(fakeApi()); - await expect( - tool.execute("call2", { - action: "run", - pipeline: "noop", - lobsterPath: "./lobster", - }), - ).rejects.toThrow(/absolute path/); - }); + it("rejects lobsterPath (deprecated) when invalid", async () => { + const fake = await writeFakeLobster({ + payload: { ok: true, status: "ok", output: [{ hello: "world" }], requiresApproval: null }, + }); - it("rejects lobsterPath that is not the lobster executable", async () => { - const tool = createLobsterTool(fakeApi()); - await expect( - tool.execute("call2b", { - action: "run", - pipeline: "noop", - lobsterPath: "/bin/bash", - }), - ).rejects.toThrow(/lobster executable/); + 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 () => { @@ -137,19 +171,25 @@ describe("lobster plugin tool", () => { }); 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 e33c78ff0c4f..6608775e74f2 100644 --- a/extensions/lobster/src/lobster-tool.ts +++ b/extensions/lobster/src/lobster-tool.ts @@ -240,7 +240,9 @@ 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({ description: "Absolute path to the lobster executable (optional)." })), + // 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: @@ -256,8 +258,15 @@ 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.config?.lobsterPath === "string" ? api.config.lobsterPath : undefined, ); const cwd = resolveCwd(params.cwd); const timeoutMs = typeof params.timeoutMs === "number" ? params.timeoutMs : 20_000;