From e95f2dcd6efe8029d4cf2fbad6baf4b6b1cc3ea6 Mon Sep 17 00:00:00 2001 From: Robin Waslander Date: Thu, 12 Mar 2026 03:37:23 +0100 Subject: [PATCH] fix(sandbox): anchor fs-bridge writeFile commit to canonical parent path Refs: GHSA-xvx8-77m6-gwg6 --- src/agents/sandbox/fs-bridge-path-safety.ts | 47 +++++++++++ .../sandbox/fs-bridge-shell-command-plans.ts | 11 ++- .../sandbox/fs-bridge.anchored-ops.test.ts | 82 +++++++++++++++++++ src/agents/sandbox/fs-bridge.ts | 11 ++- 4 files changed, 145 insertions(+), 6 deletions(-) diff --git a/src/agents/sandbox/fs-bridge-path-safety.ts b/src/agents/sandbox/fs-bridge-path-safety.ts index dfc6c6692a1..9ca4c52e537 100644 --- a/src/agents/sandbox/fs-bridge-path-safety.ts +++ b/src/agents/sandbox/fs-bridge-path-safety.ts @@ -24,6 +24,11 @@ export type PinnedSandboxEntry = { basename: string; }; +export type AnchoredSandboxEntry = { + canonicalParentPath: string; + basename: string; +}; + export type PinnedSandboxDirectoryEntry = { mountRootPath: string; relativePath: string; @@ -154,6 +159,48 @@ export class SandboxFsPathGuard { }; } + async resolveAnchoredSandboxEntry( + target: SandboxResolvedFsPath, + action: string, + ): Promise { + const basename = path.posix.basename(target.containerPath); + if (!basename || basename === "." || basename === "/") { + throw new Error(`Invalid sandbox entry target: ${target.containerPath}`); + } + const parentPath = normalizeContainerPath(path.posix.dirname(target.containerPath)); + const canonicalParentPath = await this.resolveCanonicalContainerPath({ + containerPath: parentPath, + allowFinalSymlinkForUnlink: false, + }); + this.resolveRequiredMount(canonicalParentPath, action); + return { + canonicalParentPath, + basename, + }; + } + + async resolveAnchoredPinnedEntry( + target: SandboxResolvedFsPath, + action: string, + ): Promise { + const anchoredTarget = await this.resolveAnchoredSandboxEntry(target, action); + const mount = this.resolveRequiredMount(anchoredTarget.canonicalParentPath, action); + const relativeParentPath = path.posix.relative( + mount.containerRoot, + anchoredTarget.canonicalParentPath, + ); + if (relativeParentPath.startsWith("..") || path.posix.isAbsolute(relativeParentPath)) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${action}: ${target.containerPath}`, + ); + } + return { + mountRootPath: mount.containerRoot, + relativeParentPath: relativeParentPath === "." ? "" : relativeParentPath, + basename: anchoredTarget.basename, + }; + } + resolvePinnedDirectoryEntry( target: SandboxResolvedFsPath, action: string, diff --git a/src/agents/sandbox/fs-bridge-shell-command-plans.ts b/src/agents/sandbox/fs-bridge-shell-command-plans.ts index 2987472762b..4bcd1ae04de 100644 --- a/src/agents/sandbox/fs-bridge-shell-command-plans.ts +++ b/src/agents/sandbox/fs-bridge-shell-command-plans.ts @@ -1,4 +1,4 @@ -import type { PathSafetyCheck } from "./fs-bridge-path-safety.js"; +import type { AnchoredSandboxEntry, PathSafetyCheck } from "./fs-bridge-path-safety.js"; import type { SandboxResolvedFsPath } from "./fs-paths.js"; export type SandboxFsCommandPlan = { @@ -10,11 +10,14 @@ export type SandboxFsCommandPlan = { allowFailure?: boolean; }; -export function buildStatPlan(target: SandboxResolvedFsPath): SandboxFsCommandPlan { +export function buildStatPlan( + target: SandboxResolvedFsPath, + anchoredTarget: AnchoredSandboxEntry, +): SandboxFsCommandPlan { return { checks: [{ target, options: { action: "stat files" } }], - script: 'set -eu; stat -c "%F|%s|%Y" -- "$1"', - args: [target.containerPath], + script: 'set -eu\ncd -- "$1"\nstat -c "%F|%s|%Y" -- "$2"', + args: [anchoredTarget.canonicalParentPath, anchoredTarget.basename], allowFailure: true, }; } diff --git a/src/agents/sandbox/fs-bridge.anchored-ops.test.ts b/src/agents/sandbox/fs-bridge.anchored-ops.test.ts index 9b15f02adf5..a2a17194347 100644 --- a/src/agents/sandbox/fs-bridge.anchored-ops.test.ts +++ b/src/agents/sandbox/fs-bridge.anchored-ops.test.ts @@ -4,7 +4,12 @@ import { describe, expect, it } from "vitest"; import { createSandbox, createSandboxFsBridge, + dockerExecResult, + findCallsByScriptFragment, + findCallByDockerArg, + findCallByScriptFragment, getDockerArg, + getDockerScript, installFsBridgeTestHarness, mockedExecDockerRaw, withTempDir, @@ -66,6 +71,13 @@ describe("sandbox fs bridge anchored ops", () => { }); const pinnedCases = [ + { + name: "write pins canonical parent + basename", + invoke: (bridge: ReturnType) => + bridge.writeFile({ filePath: "nested/file.txt", data: "updated" }), + expectedArgs: ["write", "/workspace", "nested", "file.txt", "1"], + forbiddenArgs: ["/workspace/nested/file.txt"], + }, { name: "mkdirp pins mount root + relative path", invoke: (bridge: ReturnType) => @@ -121,4 +133,74 @@ describe("sandbox fs bridge anchored ops", () => { }); }); }); + + it.runIf(process.platform !== "win32")( + "write resolves symlink parents to canonical pinned paths", + async () => { + await withTempDir("openclaw-fs-bridge-contract-write-", async (stateDir) => { + const workspaceDir = path.join(stateDir, "workspace"); + const realDir = path.join(workspaceDir, "real"); + await fs.mkdir(realDir, { recursive: true }); + await fs.symlink(realDir, path.join(workspaceDir, "alias")); + + mockedExecDockerRaw.mockImplementation(async (args) => { + const script = getDockerScript(args); + if (script.includes('readlink -f -- "$cursor"')) { + const target = getDockerArg(args, 1); + return dockerExecResult(`${target.replace("/workspace/alias", "/workspace/real")}\n`); + } + if (script.includes('stat -c "%F|%s|%Y"')) { + return dockerExecResult("regular file|1|2"); + } + return dockerExecResult(""); + }); + + const bridge = createSandboxFsBridge({ + sandbox: createSandbox({ + workspaceDir, + agentWorkspaceDir: workspaceDir, + }), + }); + + await bridge.writeFile({ filePath: "alias/note.txt", data: "updated" }); + + const writeCall = findCallByDockerArg(1, "write"); + expect(writeCall).toBeDefined(); + const args = writeCall?.[0] ?? []; + expect(getDockerArg(args, 2)).toBe("/workspace"); + expect(getDockerArg(args, 3)).toBe("real"); + expect(getDockerArg(args, 4)).toBe("note.txt"); + expect(args).not.toContain("alias"); + + const canonicalCalls = findCallsByScriptFragment('readlink -f -- "$cursor"'); + expect( + canonicalCalls.some(([callArgs]) => getDockerArg(callArgs, 1) === "/workspace/alias"), + ).toBe(true); + }); + }, + ); + + it("stat anchors parent + basename", async () => { + await withTempDir("openclaw-fs-bridge-contract-stat-", async (stateDir) => { + const workspaceDir = path.join(stateDir, "workspace"); + await fs.mkdir(path.join(workspaceDir, "nested"), { recursive: true }); + await fs.writeFile(path.join(workspaceDir, "nested", "file.txt"), "bye", "utf8"); + + const bridge = createSandboxFsBridge({ + sandbox: createSandbox({ + workspaceDir, + agentWorkspaceDir: workspaceDir, + }), + }); + + await bridge.stat({ filePath: "nested/file.txt" }); + + const statCall = findCallByScriptFragment('stat -c "%F|%s|%Y" -- "$2"'); + expect(statCall).toBeDefined(); + const args = statCall?.[0] ?? []; + expect(getDockerArg(args, 1)).toBe("/workspace/nested"); + expect(getDockerArg(args, 2)).toBe("file.txt"); + expect(args).not.toContain("/workspace/nested/file.txt"); + }); + }); }); diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index 83504d9b908..7a9a22d4459 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -118,7 +118,10 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { const buffer = Buffer.isBuffer(params.data) ? params.data : Buffer.from(params.data, params.encoding ?? "utf8"); - const pinnedWriteTarget = this.pathGuard.resolvePinnedEntry(target, "write files"); + const pinnedWriteTarget = await this.pathGuard.resolveAnchoredPinnedEntry( + target, + "write files", + ); await this.runCheckedCommand({ ...buildPinnedWritePlan({ check: writeCheck, @@ -218,7 +221,11 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveResolvedPath(params); - const result = await this.runPlannedCommand(buildStatPlan(target), params.signal); + const anchoredTarget = await this.pathGuard.resolveAnchoredSandboxEntry(target, "stat files"); + const result = await this.runPlannedCommand( + buildStatPlan(target, anchoredTarget), + params.signal, + ); if (result.code !== 0) { const stderr = result.stderr.toString("utf8"); if (stderr.includes("No such file or directory")) {