diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d9d8fee321..be19b55d863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Sandbox/mkdirp boundary checks: allow directory-safe boundary validation for existing in-boundary subdirectories, preventing false `cannot create directories` failures in sandbox write mode. (#30610) Thanks @glitch418x. - Android/Voice screen TTS: stream assistant speech via ElevenLabs WebSocket in Talk Mode, stop cleanly on speaker mute/barge-in, and ignore stale out-of-order stream events. (#29521) Thanks @gregmousseau. - Web UI/Cron: include configured agent model defaults/fallbacks in cron model suggestions so scheduled-job model autocomplete reflects configured models. (#29709) - Cron/Delivery: disable the agent messaging tool when `delivery.mode` is `"none"` so cron output is not sent to Telegram or other channels. (#21808) diff --git a/src/agents/sandbox/fs-bridge.test.ts b/src/agents/sandbox/fs-bridge.test.ts index 92575915147..a6c89100250 100644 --- a/src/agents/sandbox/fs-bridge.test.ts +++ b/src/agents/sandbox/fs-bridge.test.ts @@ -198,6 +198,30 @@ describe("sandbox fs bridge shell compatibility", () => { } }); + it("rejects mkdirp when target exists as a file", async () => { + const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fs-bridge-mkdirp-file-")); + try { + const workspaceDir = path.join(stateDir, "workspace"); + const filePath = path.join(workspaceDir, "memory", "kemik"); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, "not a directory"); + + const bridge = createSandboxFsBridge({ + sandbox: createSandbox({ + workspaceDir, + agentWorkspaceDir: workspaceDir, + }), + }); + + await expect(bridge.mkdirp({ filePath: "memory/kemik" })).rejects.toThrow( + /cannot create directories/i, + ); + expect(mockedExecDockerRaw).not.toHaveBeenCalled(); + } finally { + await fs.rm(stateDir, { recursive: true, force: true }); + } + }); + it("rejects pre-existing host symlink escapes before docker exec", async () => { const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fs-bridge-")); const workspaceDir = path.join(stateDir, "workspace"); diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index 24e7bdf8075..4896a9ae9a8 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -24,7 +24,7 @@ type PathSafetyOptions = { aliasPolicy?: PathAliasPolicy; requireWritable?: boolean; allowMissingTarget?: boolean; - allowedTypes?: readonly SafeOpenSyncAllowedType[]; + allowedType?: SafeOpenSyncAllowedType; }; export type SandboxResolvedPath = { @@ -137,7 +137,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { await this.assertPathSafety(target, { action: "create directories", requireWritable: true, - allowedTypes: ["directory"], + allowedType: "directory", }); await this.runCommand('set -eu; mkdir -p -- "$1"', { args: [target.containerPath], @@ -264,7 +264,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { rootPath: lexicalMount.hostRoot, boundaryLabel: "sandbox mount root", aliasPolicy: options.aliasPolicy, - allowedTypes: options.allowedTypes, + allowedType: options.allowedType, }); if (!guarded.ok) { if (guarded.reason !== "path" || options.allowMissingTarget === false) { diff --git a/src/infra/boundary-file-read.ts b/src/infra/boundary-file-read.ts index 3bcdac673af..32cbea18092 100644 --- a/src/infra/boundary-file-read.ts +++ b/src/infra/boundary-file-read.ts @@ -32,7 +32,7 @@ export type OpenBoundaryFileSyncParams = { rootRealPath?: string; maxBytes?: number; rejectHardlinks?: boolean; - allowedTypes?: readonly SafeOpenSyncAllowedType[]; + allowedType?: SafeOpenSyncAllowedType; skipLexicalRootCheck?: boolean; ioFs?: BoundaryReadFs; }; @@ -79,7 +79,7 @@ export function openBoundaryFileSync(params: OpenBoundaryFileSyncParams): Bounda resolvedPath, rejectHardlinks: params.rejectHardlinks ?? true, maxBytes: params.maxBytes, - allowedTypes: params.allowedTypes, + allowedType: params.allowedType, ioFs, }); if (!opened.ok) { diff --git a/src/infra/safe-open-sync.test.ts b/src/infra/safe-open-sync.test.ts index 7bb5e339ccb..3208a089786 100644 --- a/src/infra/safe-open-sync.test.ts +++ b/src/infra/safe-open-sync.test.ts @@ -28,14 +28,14 @@ describe("openVerifiedFileSync", () => { }); }); - it("accepts directories when allowedTypes includes directory", async () => { + it("accepts directories when allowedType is directory", async () => { await withTempDir("openclaw-safe-open-", async (root) => { const targetDir = path.join(root, "nested"); await fsp.mkdir(targetDir, { recursive: true }); const opened = openVerifiedFileSync({ filePath: targetDir, - allowedTypes: ["directory"], + allowedType: "directory", rejectHardlinks: true, }); expect(opened.ok).toBe(true); diff --git a/src/infra/safe-open-sync.ts b/src/infra/safe-open-sync.ts index d7faa89eb73..bfb50e60e42 100644 --- a/src/infra/safe-open-sync.ts +++ b/src/infra/safe-open-sync.ts @@ -30,11 +30,11 @@ export function openVerifiedFileSync(params: { rejectPathSymlink?: boolean; rejectHardlinks?: boolean; maxBytes?: number; - allowedTypes?: readonly SafeOpenSyncAllowedType[]; + allowedType?: SafeOpenSyncAllowedType; ioFs?: SafeOpenSyncFs; }): SafeOpenSyncResult { const ioFs = params.ioFs ?? fs; - const allowedTypes = params.allowedTypes ?? ["file"]; + const allowedType = params.allowedType ?? "file"; const openReadFlags = ioFs.constants.O_RDONLY | (typeof ioFs.constants.O_NOFOLLOW === "number" ? ioFs.constants.O_NOFOLLOW : 0); @@ -49,7 +49,7 @@ export function openVerifiedFileSync(params: { const realPath = params.resolvedPath ?? ioFs.realpathSync(params.filePath); const preOpenStat = ioFs.lstatSync(realPath); - if (!isAllowedType(preOpenStat, allowedTypes)) { + if (!isAllowedType(preOpenStat, allowedType)) { return { ok: false, reason: "validation" }; } if (params.rejectHardlinks && preOpenStat.isFile() && preOpenStat.nlink > 1) { @@ -65,7 +65,7 @@ export function openVerifiedFileSync(params: { fd = ioFs.openSync(realPath, openReadFlags); const openedStat = ioFs.fstatSync(fd); - if (!isAllowedType(openedStat, allowedTypes)) { + if (!isAllowedType(openedStat, allowedType)) { return { ok: false, reason: "validation" }; } if (params.rejectHardlinks && openedStat.isFile() && openedStat.nlink > 1) { @@ -93,11 +93,9 @@ export function openVerifiedFileSync(params: { } } -function isAllowedType(stat: fs.Stats, allowedTypes: readonly SafeOpenSyncAllowedType[]): boolean { - return allowedTypes.some((allowedType) => { - if (allowedType === "file") { - return stat.isFile(); - } +function isAllowedType(stat: fs.Stats, allowedType: SafeOpenSyncAllowedType): boolean { + if (allowedType === "directory") { return stat.isDirectory(); - }); + } + return stat.isFile(); }