diff --git a/CHANGELOG.md b/CHANGELOG.md index 52b2b8145..d83913104 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Agents/Tools/exec: add a preflight guard that detects likely shell env var injection (e.g. `$DM_JSON`, `$TMPDIR`) in Python/Node scripts before execution, preventing recurring cron failures and wasted tokens when models emit mixed shell+language source. (#12836) - Security/Sessions: create new session transcript JSONL files with user-only (`0o600`) permissions and extend `openclaw security audit --fix` to remediate existing transcript file permissions. - Infra/Fetch: ensure foreign abort-signal listener cleanup never masks original fetch successes/failures, while still preventing detached-finally unhandled rejection noise in `wrapFetchWithAbortSignal`. Thanks @Jackten. - Gateway/Config: prevent `config.patch` object-array merges from falling back to full-array replacement when some patch entries lack `id`, so partial `agents.list` updates no longer drop unrelated agents. (#17989) Thanks @stakeswky. diff --git a/src/agents/bash-tools.exec.script-preflight.test.ts b/src/agents/bash-tools.exec.script-preflight.test.ts new file mode 100644 index 000000000..8174093d2 --- /dev/null +++ b/src/agents/bash-tools.exec.script-preflight.test.ts @@ -0,0 +1,65 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +const isWin = process.platform === "win32"; + +describe("exec script preflight", () => { + it("blocks shell env var injection tokens in python scripts before execution", async () => { + if (isWin) { + return; + } + + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-exec-preflight-")); + const pyPath = path.join(tmp, "bad.py"); + + await fs.writeFile( + pyPath, + [ + "import json", + "# model accidentally wrote shell syntax:", + "payload = $DM_JSON", + "print(payload)", + ].join("\n"), + "utf-8", + ); + + const { createExecTool } = await import("./bash-tools.exec.js"); + const tool = createExecTool({ host: "gateway", security: "full", ask: "off" }); + + await expect( + tool.execute("call1", { + command: "python bad.py", + workdir: tmp, + }), + ).rejects.toThrow(/exec preflight: detected likely shell variable injection \(\$DM_JSON\)/); + }); + + it("blocks obvious shell-as-js output before node execution", async () => { + if (isWin) { + return; + } + + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-exec-preflight-")); + const jsPath = path.join(tmp, "bad.js"); + + await fs.writeFile( + jsPath, + ['NODE "$TMPDIR/hot.json"', "console.log('hi')"].join("\n"), + "utf-8", + ); + + const { createExecTool } = await import("./bash-tools.exec.js"); + const tool = createExecTool({ host: "gateway", security: "full", ask: "off" }); + + await expect( + tool.execute("call1", { + command: "node bad.js", + workdir: tmp, + }), + ).rejects.toThrow( + /exec preflight: (detected likely shell variable injection|JS file starts with shell syntax)/, + ); + }); +}); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index b9a7e83b2..e0fba6ea2 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1,5 +1,7 @@ import type { AgentTool, AgentToolResult } from "@mariozechner/pi-agent-core"; import crypto from "node:crypto"; +import fs from "node:fs/promises"; +import path from "node:path"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; import { type ExecAsk, @@ -118,6 +120,97 @@ export type ExecToolDetails = nodeId?: string; }; +function extractScriptTargetFromCommand( + command: string, +): { kind: "python"; relOrAbsPath: string } | { kind: "node"; relOrAbsPath: string } | null { + const raw = command.trim(); + if (!raw) { + return null; + } + + // Intentionally simple parsing: we only support common forms like + // python file.py + // python3 -u file.py + // node --experimental-something file.js + // If the command is more complex (pipes, heredocs, quoted paths with spaces), skip preflight. + const pythonMatch = raw.match(/^\s*(python3?|python)\s+(?:-[^\s]+\s+)*([^\s]+\.py)\b/i); + if (pythonMatch?.[2]) { + return { kind: "python", relOrAbsPath: pythonMatch[2] }; + } + const nodeMatch = raw.match(/^\s*(node)\s+(?:--[^\s]+\s+)*([^\s]+\.js)\b/i); + if (nodeMatch?.[2]) { + return { kind: "node", relOrAbsPath: nodeMatch[2] }; + } + + return null; +} + +async function validateScriptFileForShellBleed(params: { + command: string; + workdir: string; +}): Promise { + const target = extractScriptTargetFromCommand(params.command); + if (!target) { + return; + } + + const absPath = path.isAbsolute(target.relOrAbsPath) + ? path.resolve(target.relOrAbsPath) + : path.resolve(params.workdir, target.relOrAbsPath); + + // Best-effort: only validate if file exists and is reasonably small. + let stat: { isFile(): boolean; size: number }; + try { + stat = await fs.stat(absPath); + } catch { + return; + } + if (!stat.isFile()) { + return; + } + if (stat.size > 512 * 1024) { + return; + } + + const content = await fs.readFile(absPath, "utf-8"); + + // Common failure mode: shell env var syntax leaking into Python/JS. + // We deliberately match all-caps/underscore vars to avoid false positives with `$` as a JS identifier. + const envVarRegex = /\$[A-Z_][A-Z0-9_]{1,}/g; + const first = envVarRegex.exec(content); + if (first) { + const idx = first.index; + const before = content.slice(0, idx); + const line = before.split("\n").length; + const token = first[0]; + throw new Error( + [ + `exec preflight: detected likely shell variable injection (${token}) in ${target.kind} script: ${path.basename( + absPath, + )}:${line}.`, + target.kind === "python" + ? `In Python, use os.environ.get(${JSON.stringify(token.slice(1))}) instead of raw ${token}.` + : `In Node.js, use process.env[${JSON.stringify(token.slice(1))}] instead of raw ${token}.`, + "(If this is inside a string literal on purpose, escape it or restructure the code.)", + ].join("\n"), + ); + } + + // Another recurring pattern from the issue: shell commands accidentally emitted as JS. + if (target.kind === "node") { + const firstNonEmpty = content + .split(/\r?\n/) + .map((l) => l.trim()) + .find((l) => l.length > 0); + if (firstNonEmpty && /^NODE\b/.test(firstNonEmpty)) { + throw new Error( + `exec preflight: JS file starts with shell syntax (${firstNonEmpty}). ` + + `This looks like a shell command, not JavaScript.`, + ); + } + } +} + export function createExecTool( defaults?: ExecToolDefaults, // oxlint-disable-next-line typescript/no-explicit-any @@ -874,6 +967,11 @@ export function createExecTool( typeof params.timeout === "number" ? params.timeout : defaultTimeoutSec; const getWarningText = () => (warnings.length ? `${warnings.join("\n")}\n\n` : ""); const usePty = params.pty === true && !sandbox; + + // Preflight: catch a common model failure mode (shell syntax leaking into Python/JS sources) + // before we execute and burn tokens in cron loops. + await validateScriptFileForShellBleed({ command: params.command, workdir }); + const run = await runExecProcess({ command: params.command, execCommand: execCommandOverride, diff --git a/src/agents/pi-embedded-runner-extraparams.e2e.test.ts b/src/agents/pi-embedded-runner-extraparams.e2e.test.ts index b425f33c7..bec5e67f6 100644 --- a/src/agents/pi-embedded-runner-extraparams.e2e.test.ts +++ b/src/agents/pi-embedded-runner-extraparams.e2e.test.ts @@ -154,4 +154,27 @@ describe("applyExtraParamsToAgent", () => { }); expect(payload.store).toBe(false); }); + + it("does not force store=true for Codex responses (Codex requires store=false)", () => { + const payload = { store: false }; + const baseStreamFn: StreamFn = (_model, _context, options) => { + options?.onPayload?.(payload); + return new AssistantMessageEventStream(); + }; + const agent = { streamFn: baseStreamFn }; + + applyExtraParamsToAgent(agent, undefined, "openai-codex", "codex-mini-latest"); + + const model = { + api: "openai-codex-responses", + provider: "openai-codex", + id: "codex-mini-latest", + baseUrl: "https://chatgpt.com/backend-api/codex/responses", + } as Model<"openai-codex-responses">; + const context: Context = { messages: [] }; + + void agent.streamFn?.(model, context, {}); + + expect(payload.store).toBe(false); + }); });