Skip to content

Commit a16e13f

Browse files
zeynalniazeynalnia
authored
Merge branch 'main' into fix/auth-profile-override-sticky-new-sessions
2 parents 0fa0095 + 37625cf commit a16e13f

9 files changed

Lines changed: 499 additions & 36 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Docs: https://docs.openclaw.ai
88

99
### Fixes
1010

11+
- fix(exec): replace TOCTOU check-then-read with atomic pinned-fd open in script preflight [AI]. (#62333) Thanks @pgondhi987.
1112
- WhatsApp/auto-reply: keep inbound reply, media, and composing sends on the current socket across reconnects, wait through reconnect gaps, and retry timeout-only send failures without dropping the active socket ref. (#62892) Thanks @mcaxtr.
1213
- Config/plugins: let config writes keep disabled plugin entries without forcing required plugin config schemas or crashing raw plugin validation, so slot switches and similar plugin-state updates persist cleanly. (#63296) Thanks @fuller-stack-dev.
1314

‎src/agents/bash-tools.exec.script-preflight.test.ts‎

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
import { constants as fsConstants } from "node:fs";
12
import fs from "node:fs/promises";
23
import path from "node:path";
3-
import { describe, expect, it } from "vitest";
4+
import { describe, expect, it, vi } from "vitest";
45
import { withTempDir } from "../test-utils/temp-dir.js";
56
import { createExecTool } from "./bash-tools.exec.js";
67

@@ -74,6 +75,54 @@ describeNonWin("exec script preflight", () => {
7475
});
7576
});
7677

78+
it("validates in-workdir scripts whose names start with '..'", async () => {
79+
await withTempDir("openclaw-exec-preflight-", async (tmp) => {
80+
const jsPath = path.join(tmp, "..bad.js");
81+
await fs.writeFile(jsPath, "const value = $DM_JSON;", "utf-8");
82+
83+
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
84+
await expect(
85+
tool.execute("call-dotdot-prefix-script", {
86+
command: "node ..bad.js",
87+
workdir: tmp,
88+
}),
89+
).rejects.toThrow(/exec preflight: detected likely shell variable injection \(\$DM_JSON\)/);
90+
});
91+
});
92+
93+
it("validates in-workdir symlinked script entrypoints", async () => {
94+
await withTempDir("openclaw-exec-preflight-", async (tmp) => {
95+
const targetPath = path.join(tmp, "bad-target.js");
96+
const linkPath = path.join(tmp, "link.js");
97+
await fs.writeFile(targetPath, "const value = $DM_JSON;", "utf-8");
98+
await fs.symlink(targetPath, linkPath);
99+
100+
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
101+
await expect(
102+
tool.execute("call-symlink-entrypoint", {
103+
command: "node link.js",
104+
workdir: tmp,
105+
}),
106+
).rejects.toThrow(/exec preflight: detected likely shell variable injection \(\$DM_JSON\)/);
107+
});
108+
});
109+
110+
it("validates scripts under literal tilde directories in workdir", async () => {
111+
await withTempDir("openclaw-exec-preflight-", async (tmp) => {
112+
const literalTildeDir = path.join(tmp, "~");
113+
await fs.mkdir(literalTildeDir, { recursive: true });
114+
await fs.writeFile(path.join(literalTildeDir, "bad.js"), "const value = $DM_JSON;", "utf-8");
115+
116+
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
117+
await expect(
118+
tool.execute("call-literal-tilde-path", {
119+
command: 'node "~/bad.js"',
120+
workdir: tmp,
121+
}),
122+
).rejects.toThrow(/exec preflight: detected likely shell variable injection \(\$DM_JSON\)/);
123+
});
124+
});
125+
77126
it("validates python scripts when interpreter is prefixed with env", async () => {
78127
await withTempDir("openclaw-exec-preflight-", async (tmp) => {
79128
const pyPath = path.join(tmp, "bad.py");
@@ -268,6 +317,115 @@ describeNonWin("exec script preflight", () => {
268317
});
269318
});
270319

320+
it("does not trust a swapped script pathname between validation and read", async () => {
321+
await withTempDir("openclaw-exec-preflight-race-", async (parent) => {
322+
const workdir = path.join(parent, "workdir");
323+
const scriptPath = path.join(workdir, "script.js");
324+
const outsidePath = path.join(parent, "outside.js");
325+
await fs.mkdir(workdir, { recursive: true });
326+
await fs.writeFile(scriptPath, 'console.log("inside")', "utf-8");
327+
await fs.writeFile(outsidePath, 'console.log("$DM_JSON outside")', "utf-8");
328+
329+
const originalStat = fs.stat.bind(fs);
330+
let swapped = false;
331+
const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (...args) => {
332+
const target = args[0];
333+
if (!swapped && typeof target === "string" && path.resolve(target) === scriptPath) {
334+
const original = await originalStat(target);
335+
await fs.rm(scriptPath, { force: true });
336+
await fs.symlink(outsidePath, scriptPath);
337+
swapped = true;
338+
return original;
339+
}
340+
return await originalStat(...args);
341+
});
342+
343+
try {
344+
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
345+
const result = await tool.execute("call-swapped-pathname", {
346+
command: "node script.js",
347+
workdir,
348+
});
349+
const text = result.content.find((block) => block.type === "text")?.text ?? "";
350+
expect(swapped).toBe(true);
351+
expect(text).not.toMatch(/exec preflight:/);
352+
} finally {
353+
statSpy.mockRestore();
354+
}
355+
});
356+
});
357+
358+
it("handles pre-open symlink swaps without surfacing preflight errors", async () => {
359+
await withTempDir("openclaw-exec-preflight-open-race-", async (parent) => {
360+
const workdir = path.join(parent, "workdir");
361+
const scriptPath = path.join(workdir, "script.js");
362+
const outsidePath = path.join(parent, "outside.js");
363+
await fs.mkdir(workdir, { recursive: true });
364+
await fs.writeFile(scriptPath, 'console.log("inside")', "utf-8");
365+
await fs.writeFile(outsidePath, 'console.log("$DM_JSON outside")', "utf-8");
366+
367+
const originalOpen = fs.open.bind(fs);
368+
let swapped = false;
369+
const openSpy = vi.spyOn(fs, "open").mockImplementation(async (...args) => {
370+
const target = args[0];
371+
if (!swapped && typeof target === "string" && path.resolve(target) === scriptPath) {
372+
await fs.rm(scriptPath, { force: true });
373+
await fs.symlink(outsidePath, scriptPath);
374+
swapped = true;
375+
}
376+
return await originalOpen(...args);
377+
});
378+
379+
try {
380+
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
381+
const result = await tool.execute("call-pre-open-swapped-pathname", {
382+
command: "node script.js",
383+
workdir,
384+
});
385+
const text = result.content.find((block) => block.type === "text")?.text ?? "";
386+
expect(swapped).toBe(true);
387+
expect(text).not.toMatch(/exec preflight:/);
388+
} finally {
389+
openSpy.mockRestore();
390+
}
391+
});
392+
});
393+
394+
it("opens preflight script reads with O_NONBLOCK to avoid FIFO stalls", async () => {
395+
await withTempDir("openclaw-exec-preflight-nonblock-", async (tmp) => {
396+
const scriptPath = path.join(tmp, "script.js");
397+
await fs.writeFile(scriptPath, 'console.log("ok")', "utf-8");
398+
399+
const originalOpen = fs.open.bind(fs);
400+
const scriptOpenFlags: number[] = [];
401+
const openSpy = vi.spyOn(fs, "open").mockImplementation(async (...args) => {
402+
const [target, flags] = args;
403+
if (
404+
typeof target === "string" &&
405+
path.resolve(target) === scriptPath &&
406+
typeof flags === "number"
407+
) {
408+
scriptOpenFlags.push(flags);
409+
}
410+
return await originalOpen(...args);
411+
});
412+
413+
try {
414+
const tool = createExecTool({ host: "gateway", security: "full", ask: "off" });
415+
const result = await tool.execute("call-nonblocking-preflight-open", {
416+
command: "node script.js",
417+
workdir: tmp,
418+
});
419+
const text = result.content.find((block) => block.type === "text")?.text ?? "";
420+
expect(scriptOpenFlags.length).toBeGreaterThan(0);
421+
expect(scriptOpenFlags.some((flags) => (flags & fsConstants.O_NONBLOCK) !== 0)).toBe(true);
422+
expect(text).not.toMatch(/exec preflight:/);
423+
} finally {
424+
openSpy.mockRestore();
425+
}
426+
});
427+
});
428+
271429
it("fails closed for piped interpreter commands that bypass direct script parsing", async () => {
272430
await withTempDir("openclaw-exec-preflight-", async (tmp) => {
273431
const pyPath = path.join(tmp, "bad.py");

‎src/agents/bash-tools.exec.ts‎

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import fs from "node:fs/promises";
21
import path from "node:path";
32
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
43
import { analyzeShellCommand } from "../infra/exec-approvals-analysis.js";
@@ -10,6 +9,7 @@ import {
109
resolveExecApprovalsFromFile,
1110
} from "../infra/exec-approvals.js";
1211
import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js";
12+
import { SafeOpenError, readFileWithinRoot } from "../infra/fs-safe.js";
1313
import { sanitizeHostExecEnvWithDiagnostics } from "../infra/host-env-security.js";
1414
import {
1515
getShellPathFromLoginShell,
@@ -56,7 +56,6 @@ import {
5656
resolveWorkdir,
5757
truncateMiddle,
5858
} from "./bash-tools.shared.js";
59-
import { assertSandboxPath } from "./sandbox-paths.js";
6059
import { EXEC_TOOL_DISPLAY_SUMMARY } from "./tool-description-presets.js";
6160
import { type AgentToolWithMeta, failedTextResult, textResult } from "./tools/common.js";
6261

@@ -105,6 +104,44 @@ const PREFLIGHT_ENV_OPTIONS_WITH_VALUES = new Set([
105104
"--unset",
106105
]);
107106

107+
const SKIPPABLE_SCRIPT_PREFLIGHT_FS_ERROR_CODES = new Set([
108+
"EACCES",
109+
"EISDIR",
110+
"ELOOP",
111+
"EINVAL",
112+
"ENAMETOOLONG",
113+
"ENOENT",
114+
"ENOTDIR",
115+
"EPERM",
116+
]);
117+
118+
function getNodeErrorCode(error: unknown): string | undefined {
119+
if (typeof error !== "object" || error === null || !("code" in error)) {
120+
return undefined;
121+
}
122+
return String((error as { code?: unknown }).code);
123+
}
124+
125+
function shouldSkipScriptPreflightPathError(error: unknown): boolean {
126+
if (error instanceof SafeOpenError) {
127+
return true;
128+
}
129+
const errorCode = getNodeErrorCode(error);
130+
return !!(errorCode && SKIPPABLE_SCRIPT_PREFLIGHT_FS_ERROR_CODES.has(errorCode));
131+
}
132+
133+
function resolvePreflightRelativePath(params: { rootDir: string; absPath: string }): string | null {
134+
const root = path.resolve(params.rootDir);
135+
const candidate = path.resolve(params.absPath);
136+
const relative = path.relative(root, candidate);
137+
if (/^\.\.(?:[\\/]|$)/u.test(relative) || path.isAbsolute(relative)) {
138+
return null;
139+
}
140+
// Preserve literal "~" path segments under the workdir. `readFileWithinRoot`
141+
// expands home prefixes for relative paths, so normalize `~/...` to `./~/...`.
142+
return /^~(?:$|[\\/])/u.test(relative) ? `.${path.sep}${relative}` : relative;
143+
}
144+
108145
function isShellEnvAssignmentToken(token: string): boolean {
109146
return /^[A-Za-z_][A-Za-z0-9_]*=.*$/u.test(token);
110147
}
@@ -921,28 +958,37 @@ async function validateScriptFileForShellBleed(params: {
921958
const absPath = path.isAbsolute(relOrAbsPath)
922959
? path.resolve(relOrAbsPath)
923960
: path.resolve(params.workdir, relOrAbsPath);
961+
const relativePath = resolvePreflightRelativePath({
962+
rootDir: params.workdir,
963+
absPath,
964+
});
965+
if (!relativePath) {
966+
continue;
967+
}
924968

925-
// Best-effort: only validate if file exists and is reasonably small.
926-
let stat: { isFile(): boolean; size: number };
969+
// Best-effort: only validate files that safely resolve within workdir and
970+
// are reasonably small. This keeps preflight checks on a pinned file
971+
// identity instead of trusting mutable pathnames across multiple ops.
972+
// Use non-blocking open to avoid stalls if a path is swapped to a FIFO.
973+
let content: string;
927974
try {
928-
await assertSandboxPath({
929-
filePath: absPath,
930-
cwd: params.workdir,
931-
root: params.workdir,
975+
const safeRead = await readFileWithinRoot({
976+
rootDir: params.workdir,
977+
relativePath,
978+
nonBlockingRead: true,
979+
allowSymlinkTargetWithinRoot: true,
980+
maxBytes: 512 * 1024,
932981
});
933-
stat = await fs.stat(absPath);
934-
} catch {
935-
continue;
936-
}
937-
if (!stat.isFile()) {
938-
continue;
939-
}
940-
if (stat.size > 512 * 1024) {
941-
continue;
982+
content = safeRead.buffer.toString("utf-8");
983+
} catch (error) {
984+
if (shouldSkipScriptPreflightPathError(error)) {
985+
// Preflight validation is best-effort: skip path/read failures and
986+
// continue to execute the command normally.
987+
continue;
988+
}
989+
throw error;
942990
}
943991

944-
const content = await fs.readFile(absPath, "utf-8");
945-
946992
// Common failure mode: shell env var syntax leaking into Python/JS.
947993
// We deliberately match all-caps/underscore vars to avoid false positives with `$` as a JS identifier.
948994
const envVarRegex = /\$[A-Z_][A-Z0-9_]{1,}/g;

‎src/agents/tools/pdf-native-providers.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { afterEach, describe, expect, it, vi } from "vitest";
22
import * as pdfNativeProviders from "./pdf-native-providers.js";
33

4+
vi.mock("../../plugins/provider-runtime.js", () => ({
5+
normalizeProviderTransportWithPlugin: (params: { context?: { baseUrl?: string } }) =>
6+
params.context?.baseUrl ? { baseUrl: params.context.baseUrl } : undefined,
7+
}));
8+
49
const TEST_PDF_INPUT = { base64: "dGVzdA==", filename: "doc.pdf" } as const;
510

611
function makeAnthropicAnalyzeParams(

0 commit comments

Comments
 (0)