From ae7f18e5033def8b4d49faca96cee7269223536b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 20:28:11 -0700 Subject: [PATCH] feat: add remote openshell sandbox mode --- CHANGELOG.md | 2 +- docs/gateway/configuration-reference.md | 8 +- docs/gateway/sandboxing.md | 52 +- extensions/openshell/src/backend.ts | 67 ++- extensions/openshell/src/config.test.ts | 13 + extensions/openshell/src/config.ts | 11 + extensions/openshell/src/fs-bridge.ts | 39 +- .../openshell/src/remote-fs-bridge.test.ts | 191 ++++++ extensions/openshell/src/remote-fs-bridge.ts | 550 ++++++++++++++++++ src/agents/apply-patch.test.ts | 42 ++ src/agents/apply-patch.ts | 6 +- src/agents/sandbox-media-paths.test.ts | 25 +- src/agents/sandbox-media-paths.ts | 15 +- src/agents/sandbox/fs-bridge.ts | 2 +- .../test-helpers/host-sandbox-fs-bridge.ts | 20 + 15 files changed, 1008 insertions(+), 35 deletions(-) create mode 100644 extensions/openshell/src/remote-fs-bridge.test.ts create mode 100644 extensions/openshell/src/remote-fs-bridge.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 98208595e0c..260d393c3cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ Docs: https://docs.openclaw.ai - Plugins/agent integrations: broaden the plugin surface for app-server integrations with channel-aware commands, interactive callbacks, inbound claims, and Discord/Telegram conversation binding support. (#45318) Thanks @huntharo and @vincentkoc. - Telegram/actions: add `topic-edit` for forum-topic renames and icon updates while sharing the same Telegram topic-edit transport used by the plugin runtime. (#47798) Thanks @obviyus. - secrets: harden read-only SecretRef command paths and diagnostics. (#47794) Thanks @joshavant. -- Sandbox/runtime: add pluggable sandbox backends, ship an OpenShell backend in mirror mode, and make sandbox list/recreate/prune backend-aware instead of Docker-only. +- Sandbox/runtime: add pluggable sandbox backends, ship an OpenShell backend with `mirror` and `remote` workspace modes, and make sandbox list/recreate/prune backend-aware instead of Docker-only. ### Fixes diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 78e58edc085..dbfc2b5dccb 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -1117,7 +1117,7 @@ See [Typing Indicators](/concepts/typing-indicators). ### `agents.defaults.sandbox` -Optional **Docker sandboxing** for the embedded agent. See [Sandboxing](/gateway/sandboxing) for the full guide. +Optional sandboxing for the embedded agent. See [Sandboxing](/gateway/sandboxing) for the full guide. ```json5 { @@ -1125,6 +1125,7 @@ Optional **Docker sandboxing** for the embedded agent. See [Sandboxing](/gateway defaults: { sandbox: { mode: "non-main", // off | non-main | all + backend: "docker", // docker | openshell scope: "agent", // session | agent | shared workspaceAccess: "none", // none | ro | rw workspaceRoot: "~/.openclaw/sandboxes", @@ -1260,6 +1261,11 @@ noVNC observer access uses VNC auth by default and OpenClaw emits a short-lived +When `backend: "openshell"` is selected, runtime-specific settings move to +`plugins.entries.openshell.config` (for example `mode: "mirror" | "remote"` and +`remoteWorkspaceDir`). Browser sandboxing and `sandbox.docker.binds` are +currently Docker-only. + Build images: ```bash diff --git a/docs/gateway/sandboxing.md b/docs/gateway/sandboxing.md index d62af2f4f7d..0e2219de14f 100644 --- a/docs/gateway/sandboxing.md +++ b/docs/gateway/sandboxing.md @@ -7,7 +7,7 @@ status: active # Sandboxing -OpenClaw can run **tools inside Docker containers** to reduce blast radius. +OpenClaw can run **tools inside sandbox backends** to reduce blast radius. This is **optional** and controlled by configuration (`agents.defaults.sandbox` or `agents.list[].sandbox`). If sandboxing is off, tools run on the host. The Gateway stays on the host; tool execution runs in an isolated sandbox @@ -54,6 +54,54 @@ Not sandboxed: - `"agent"`: one container per agent. - `"shared"`: one container shared by all sandboxed sessions. +## Backend + +`agents.defaults.sandbox.backend` controls **which runtime** provides the sandbox: + +- `"docker"` (default): local Docker-backed sandbox runtime. +- `"openshell"`: OpenShell-backed sandbox runtime provided by the bundled `openshell` plugin. + +OpenShell-specific config lives under `plugins.entries.openshell.config`. + +```json5 +{ + agents: { + defaults: { + sandbox: { + mode: "all", + backend: "openshell", + scope: "session", + workspaceAccess: "rw", + }, + }, + }, + plugins: { + entries: { + openshell: { + enabled: true, + config: { + from: "openclaw", + mode: "remote", // mirror | remote + remoteWorkspaceDir: "/sandbox", + remoteAgentWorkspaceDir: "/agent", + }, + }, + }, + }, +} +``` + +OpenShell modes: + +- `mirror` (default): local workspace stays canonical. OpenClaw syncs local files into OpenShell before exec and syncs the remote workspace back after exec. +- `remote`: OpenShell workspace is canonical after the sandbox is created. OpenClaw seeds the remote workspace once from the local workspace, then file tools and exec run directly against the remote sandbox without syncing changes back. + +Current OpenShell limitations: + +- sandbox browser is not supported yet +- `sandbox.docker.binds` is not supported on the OpenShell backend +- Docker-specific runtime knobs under `sandbox.docker.*` still apply only to the Docker backend + ## Workspace access `agents.defaults.sandbox.workspaceAccess` controls **what the sandbox can see**: @@ -116,7 +164,7 @@ Security notes: ## Images + setup -Default image: `openclaw-sandbox:bookworm-slim` +Default Docker image: `openclaw-sandbox:bookworm-slim` Build it once: diff --git a/extensions/openshell/src/backend.ts b/extensions/openshell/src/backend.ts index 48f730946d4..85c3d415904 100644 --- a/extensions/openshell/src/backend.ts +++ b/extensions/openshell/src/backend.ts @@ -24,6 +24,7 @@ import { import { resolveOpenShellPluginConfig, type ResolvedOpenShellPluginConfig } from "./config.js"; import { createOpenShellFsBridge } from "./fs-bridge.js"; import { replaceDirectoryContents } from "./mirror.js"; +import { createOpenShellRemoteFsBridge } from "./remote-fs-bridge.js"; type CreateOpenShellSandboxBackendFactoryParams = { pluginConfig: ResolvedOpenShellPluginConfig; @@ -34,6 +35,7 @@ type PendingExec = { }; export type OpenShellSandboxBackend = SandboxBackendHandle & { + mode: "mirror" | "remote"; remoteWorkspaceDir: string; remoteAgentWorkspaceDir: string; runRemoteShellScript(params: SandboxBackendCommandParams): Promise; @@ -109,6 +111,7 @@ async function createOpenShellSandboxBackend(params: { runtimeLabel: sandboxName, workdir: params.pluginConfig.remoteWorkspaceDir, env: params.createParams.cfg.docker.env, + mode: params.pluginConfig.mode, configLabel: params.pluginConfig.from, configLabelKind: "Source", buildExecSpec: async ({ command, workdir, env, usePty }) => { @@ -125,10 +128,15 @@ async function createOpenShellSandboxBackend(params: { }, runShellCommand: async (command) => await impl.runRemoteShellScript(command), createFsBridge: ({ sandbox }) => - createOpenShellFsBridge({ - sandbox, - backend: impl.asHandle(), - }), + params.pluginConfig.mode === "remote" + ? createOpenShellRemoteFsBridge({ + sandbox, + backend: impl.asHandle(), + }) + : createOpenShellFsBridge({ + sandbox, + backend: impl.asHandle(), + }), remoteWorkspaceDir: params.pluginConfig.remoteWorkspaceDir, remoteAgentWorkspaceDir: params.pluginConfig.remoteAgentWorkspaceDir, runRemoteShellScript: async (command) => await impl.runRemoteShellScript(command), @@ -139,6 +147,7 @@ async function createOpenShellSandboxBackend(params: { class OpenShellSandboxBackendImpl { private ensurePromise: Promise | null = null; + private remoteSeedPending = false; constructor( private readonly params: { @@ -157,6 +166,7 @@ class OpenShellSandboxBackendImpl { runtimeLabel: this.params.execContext.sandboxName, workdir: this.params.remoteWorkspaceDir, env: this.params.createParams.cfg.docker.env, + mode: this.params.execContext.config.mode, configLabel: this.params.execContext.config.from, configLabelKind: "Source", remoteWorkspaceDir: this.params.remoteWorkspaceDir, @@ -175,10 +185,15 @@ class OpenShellSandboxBackendImpl { }, runShellCommand: async (command) => await self.runRemoteShellScript(command), createFsBridge: ({ sandbox }) => - createOpenShellFsBridge({ - sandbox, - backend: self.asHandle(), - }), + this.params.execContext.config.mode === "remote" + ? createOpenShellRemoteFsBridge({ + sandbox, + backend: self.asHandle(), + }) + : createOpenShellFsBridge({ + sandbox, + backend: self.asHandle(), + }), runRemoteShellScript: async (command) => await self.runRemoteShellScript(command), syncLocalPathToRemote: async (localPath, remotePath) => await self.syncLocalPathToRemote(localPath, remotePath), @@ -192,7 +207,11 @@ class OpenShellSandboxBackendImpl { usePty: boolean; }): Promise<{ argv: string[]; token: PendingExec }> { await this.ensureSandboxExists(); - await this.syncWorkspaceToRemote(); + if (this.params.execContext.config.mode === "mirror") { + await this.syncWorkspaceToRemote(); + } else { + await this.maybeSeedRemoteWorkspace(); + } const sshSession = await createOpenShellSshSession({ context: this.params.execContext, }); @@ -218,7 +237,9 @@ class OpenShellSandboxBackendImpl { async finalizeExec(token?: PendingExec): Promise { try { - await this.syncWorkspaceFromRemote(); + if (this.params.execContext.config.mode === "mirror") { + await this.syncWorkspaceFromRemote(); + } } finally { if (token?.sshSession) { await disposeOpenShellSshSession(token.sshSession); @@ -230,6 +251,13 @@ class OpenShellSandboxBackendImpl { params: SandboxBackendCommandParams, ): Promise { await this.ensureSandboxExists(); + await this.maybeSeedRemoteWorkspace(); + return await this.runRemoteShellScriptInternal(params); + } + + private async runRemoteShellScriptInternal( + params: SandboxBackendCommandParams, + ): Promise { const session = await createOpenShellSshSession({ context: this.params.execContext, }); @@ -254,6 +282,7 @@ class OpenShellSandboxBackendImpl { async syncLocalPathToRemote(localPath: string, remotePath: string): Promise { await this.ensureSandboxExists(); + await this.maybeSeedRemoteWorkspace(); const stats = await fs.lstat(localPath).catch(() => null); if (!stats) { await this.runRemoteShellScript({ @@ -340,10 +369,11 @@ class OpenShellSandboxBackendImpl { if (createResult.code !== 0) { throw new Error(createResult.stderr.trim() || "openshell sandbox create failed"); } + this.remoteSeedPending = true; } private async syncWorkspaceToRemote(): Promise { - await this.runRemoteShellScript({ + await this.runRemoteShellScriptInternal({ script: 'mkdir -p -- "$1" && find "$1" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +', args: [this.params.remoteWorkspaceDir], }); @@ -357,7 +387,7 @@ class OpenShellSandboxBackendImpl { path.resolve(this.params.createParams.agentWorkspaceDir) !== path.resolve(this.params.createParams.workspaceDir) ) { - await this.runRemoteShellScript({ + await this.runRemoteShellScriptInternal({ script: 'mkdir -p -- "$1" && find "$1" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +', args: [this.params.remoteAgentWorkspaceDir], }); @@ -413,6 +443,19 @@ class OpenShellSandboxBackendImpl { throw new Error(result.stderr.trim() || "openshell sandbox upload failed"); } } + + private async maybeSeedRemoteWorkspace(): Promise { + if (!this.remoteSeedPending) { + return; + } + this.remoteSeedPending = false; + try { + await this.syncWorkspaceToRemote(); + } catch (error) { + this.remoteSeedPending = true; + throw error; + } + } } function resolveOpenShellPluginConfigFromConfig( diff --git a/extensions/openshell/src/config.test.ts b/extensions/openshell/src/config.test.ts index 66734ca43e0..f46fec1cd46 100644 --- a/extensions/openshell/src/config.test.ts +++ b/extensions/openshell/src/config.test.ts @@ -4,6 +4,7 @@ import { resolveOpenShellPluginConfig } from "./config.js"; describe("openshell plugin config", () => { it("applies defaults", () => { expect(resolveOpenShellPluginConfig(undefined)).toEqual({ + mode: "mirror", command: "openshell", gateway: undefined, gatewayEndpoint: undefined, @@ -18,6 +19,10 @@ describe("openshell plugin config", () => { }); }); + it("accepts remote mode", () => { + expect(resolveOpenShellPluginConfig({ mode: "remote" }).mode).toBe("remote"); + }); + it("rejects relative remote paths", () => { expect(() => resolveOpenShellPluginConfig({ @@ -25,4 +30,12 @@ describe("openshell plugin config", () => { }), ).toThrow("OpenShell remote path must be absolute"); }); + + it("rejects unknown mode", () => { + expect(() => + resolveOpenShellPluginConfig({ + mode: "bogus", + }), + ).toThrow("mode must be one of mirror, remote"); + }); }); diff --git a/extensions/openshell/src/config.ts b/extensions/openshell/src/config.ts index 53e5f06584b..58b40180cd9 100644 --- a/extensions/openshell/src/config.ts +++ b/extensions/openshell/src/config.ts @@ -2,6 +2,7 @@ import path from "node:path"; import type { OpenClawPluginConfigSchema } from "openclaw/plugin-sdk/core"; export type OpenShellPluginConfig = { + mode?: string; command?: string; gateway?: string; gatewayEndpoint?: string; @@ -16,6 +17,7 @@ export type OpenShellPluginConfig = { }; export type ResolvedOpenShellPluginConfig = { + mode: "mirror" | "remote"; command: string; gateway?: string; gatewayEndpoint?: string; @@ -30,6 +32,7 @@ export type ResolvedOpenShellPluginConfig = { }; const DEFAULT_COMMAND = "openshell"; +const DEFAULT_MODE = "mirror"; const DEFAULT_SOURCE = "openclaw"; const DEFAULT_REMOTE_WORKSPACE_DIR = "/sandbox"; const DEFAULT_REMOTE_AGENT_WORKSPACE_DIR = "/agent"; @@ -99,6 +102,7 @@ export function createOpenShellPluginConfigSchema(): OpenClawPluginConfigSchema }; } const allowedKeys = new Set([ + "mode", "command", "gateway", "gatewayEndpoint", @@ -156,6 +160,7 @@ export function createOpenShellPluginConfigSchema(): OpenClawPluginConfigSchema return { success: true, data: { + mode: trimString(value.mode), command: trimString(value.command), gateway: trimString(value.gateway), gatewayEndpoint: trimString(value.gatewayEndpoint), @@ -178,6 +183,7 @@ export function createOpenShellPluginConfigSchema(): OpenClawPluginConfigSchema additionalProperties: false, properties: { command: { type: "string" }, + mode: { type: "string", enum: ["mirror", "remote"] }, gateway: { type: "string" }, gatewayEndpoint: { type: "string" }, from: { type: "string" }, @@ -203,7 +209,12 @@ export function resolveOpenShellPluginConfig(value: unknown): ResolvedOpenShellP } const raw = parsed.data ?? {}; const cfg = (raw ?? {}) as OpenShellPluginConfig; + const mode = cfg.mode ?? DEFAULT_MODE; + if (mode !== "mirror" && mode !== "remote") { + throw new Error(`Invalid openshell plugin config: mode must be one of mirror, remote`); + } return { + mode, command: cfg.command ?? DEFAULT_COMMAND, gateway: cfg.gateway, gatewayEndpoint: cfg.gatewayEndpoint, diff --git a/extensions/openshell/src/fs-bridge.ts b/extensions/openshell/src/fs-bridge.ts index b9ab9b01549..00257e81be4 100644 --- a/extensions/openshell/src/fs-bridge.ts +++ b/extensions/openshell/src/fs-bridge.ts @@ -43,13 +43,14 @@ class OpenShellFsBridge implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveTarget(params); + const hostPath = this.requireHostPath(target); await assertLocalPathSafety({ target, root: target.mountHostRoot, allowMissingLeaf: false, allowFinalSymlinkForUnlink: false, }); - return await fsPromises.readFile(target.hostPath); + return await fsPromises.readFile(hostPath); } async writeFile(params: { @@ -61,6 +62,7 @@ class OpenShellFsBridge implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveTarget(params); + const hostPath = this.requireHostPath(target); this.ensureWritable(target, "write files"); await assertLocalPathSafety({ target, @@ -71,21 +73,22 @@ class OpenShellFsBridge implements SandboxFsBridge { const buffer = Buffer.isBuffer(params.data) ? params.data : Buffer.from(params.data, params.encoding ?? "utf8"); - const parentDir = path.dirname(target.hostPath); + const parentDir = path.dirname(hostPath); if (params.mkdir !== false) { await fsPromises.mkdir(parentDir, { recursive: true }); } const tempPath = path.join( parentDir, - `.openclaw-openshell-write-${path.basename(target.hostPath)}-${process.pid}-${Date.now()}`, + `.openclaw-openshell-write-${path.basename(hostPath)}-${process.pid}-${Date.now()}`, ); await fsPromises.writeFile(tempPath, buffer); - await fsPromises.rename(tempPath, target.hostPath); - await this.backend.syncLocalPathToRemote(target.hostPath, target.containerPath); + await fsPromises.rename(tempPath, hostPath); + await this.backend.syncLocalPathToRemote(hostPath, target.containerPath); } async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { const target = this.resolveTarget(params); + const hostPath = this.requireHostPath(target); this.ensureWritable(target, "create directories"); await assertLocalPathSafety({ target, @@ -93,7 +96,7 @@ class OpenShellFsBridge implements SandboxFsBridge { allowMissingLeaf: true, allowFinalSymlinkForUnlink: false, }); - await fsPromises.mkdir(target.hostPath, { recursive: true }); + await fsPromises.mkdir(hostPath, { recursive: true }); await this.backend.runRemoteShellScript({ script: 'mkdir -p -- "$1"', args: [target.containerPath], @@ -109,6 +112,7 @@ class OpenShellFsBridge implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveTarget(params); + const hostPath = this.requireHostPath(target); this.ensureWritable(target, "remove files"); await assertLocalPathSafety({ target, @@ -116,7 +120,7 @@ class OpenShellFsBridge implements SandboxFsBridge { allowMissingLeaf: params.force !== false, allowFinalSymlinkForUnlink: true, }); - await fsPromises.rm(target.hostPath, { + await fsPromises.rm(hostPath, { recursive: params.recursive ?? false, force: params.force !== false, }); @@ -138,6 +142,8 @@ class OpenShellFsBridge implements SandboxFsBridge { }): Promise { const from = this.resolveTarget({ filePath: params.from, cwd: params.cwd }); const to = this.resolveTarget({ filePath: params.to, cwd: params.cwd }); + const fromHostPath = this.requireHostPath(from); + const toHostPath = this.requireHostPath(to); this.ensureWritable(from, "rename files"); this.ensureWritable(to, "rename files"); await assertLocalPathSafety({ @@ -152,8 +158,8 @@ class OpenShellFsBridge implements SandboxFsBridge { allowMissingLeaf: true, allowFinalSymlinkForUnlink: false, }); - await fsPromises.mkdir(path.dirname(to.hostPath), { recursive: true }); - await movePathWithCopyFallback({ from: from.hostPath, to: to.hostPath }); + await fsPromises.mkdir(path.dirname(toHostPath), { recursive: true }); + await movePathWithCopyFallback({ from: fromHostPath, to: toHostPath }); await this.backend.runRemoteShellScript({ script: 'mkdir -p -- "$(dirname -- "$2")" && mv -- "$1" "$2"', args: [from.containerPath, to.containerPath], @@ -167,7 +173,8 @@ class OpenShellFsBridge implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveTarget(params); - const stats = await fsPromises.lstat(target.hostPath).catch(() => null); + const hostPath = this.requireHostPath(target); + const stats = await fsPromises.lstat(hostPath).catch(() => null); if (!stats) { return null; } @@ -190,6 +197,15 @@ class OpenShellFsBridge implements SandboxFsBridge { } } + private requireHostPath(target: ResolvedMountPath): string { + if (!target.hostPath) { + throw new Error( + `OpenShell mirror bridge requires a local host path: ${target.containerPath}`, + ); + } + return target.hostPath; + } + private resolveTarget(params: { filePath: string; cwd?: string }): ResolvedMountPath { const workspaceRoot = path.resolve(this.sandbox.workspaceDir); const agentRoot = path.resolve(this.sandbox.agentWorkspaceDir); @@ -282,6 +298,9 @@ async function assertLocalPathSafety(params: { allowMissingLeaf: boolean; allowFinalSymlinkForUnlink: boolean; }): Promise { + if (!params.target.hostPath) { + throw new Error(`Missing local host path for ${params.target.containerPath}`); + } const canonicalRoot = await fsPromises .realpath(params.root) .catch(() => path.resolve(params.root)); diff --git a/extensions/openshell/src/remote-fs-bridge.test.ts b/extensions/openshell/src/remote-fs-bridge.test.ts new file mode 100644 index 00000000000..5a245e1d8fb --- /dev/null +++ b/extensions/openshell/src/remote-fs-bridge.test.ts @@ -0,0 +1,191 @@ +import { spawn } from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createSandboxTestContext } from "../../../src/agents/sandbox/test-fixtures.js"; +import type { OpenShellSandboxBackend } from "./backend.js"; +import { createOpenShellRemoteFsBridge } from "./remote-fs-bridge.js"; + +const tempDirs: string[] = []; + +async function makeTempDir(prefix: string) { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }))); +}); + +function translateRemotePath(value: string, roots: { workspace: string; agent: string }) { + if (value === "/sandbox" || value.startsWith("/sandbox/")) { + return path.join(roots.workspace, value.slice("/sandbox".length)); + } + if (value === "/agent" || value.startsWith("/agent/")) { + return path.join(roots.agent, value.slice("/agent".length)); + } + return value; +} + +async function runLocalShell(params: { + script: string; + args?: string[]; + stdin?: Buffer | string; + allowFailure?: boolean; + roots: { workspace: string; agent: string }; +}) { + const translatedArgs = (params.args ?? []).map((arg) => translateRemotePath(arg, params.roots)); + const script = normalizeScriptForLocalShell(params.script); + const result = await new Promise<{ stdout: Buffer; stderr: Buffer; code: number }>( + (resolve, reject) => { + const child = spawn("/bin/sh", ["-c", script, "openshell-test", ...translatedArgs], { + stdio: ["pipe", "pipe", "pipe"], + }); + const stdoutChunks: Buffer[] = []; + const stderrChunks: Buffer[] = []; + child.stdout.on("data", (chunk) => stdoutChunks.push(Buffer.from(chunk))); + child.stderr.on("data", (chunk) => stderrChunks.push(Buffer.from(chunk))); + child.on("error", reject); + child.on("close", (code) => { + const result = { + stdout: Buffer.concat(stdoutChunks), + stderr: Buffer.concat(stderrChunks), + code: code ?? 0, + }; + if (result.code !== 0 && !params.allowFailure) { + reject( + new Error( + result.stderr.toString("utf8").trim() || `script exited with code ${result.code}`, + ), + ); + return; + } + resolve(result); + }); + if (params.stdin !== undefined) { + child.stdin.end(params.stdin); + return; + } + child.stdin.end(); + }, + ); + return { + ...result, + stdout: Buffer.from(rewriteLocalPaths(result.stdout.toString("utf8"), params.roots), "utf8"), + }; +} + +function createBackendMock(roots: { workspace: string; agent: string }): OpenShellSandboxBackend { + return { + id: "openshell", + runtimeId: "openshell-test", + runtimeLabel: "openshell-test", + workdir: "/sandbox", + env: {}, + mode: "remote", + remoteWorkspaceDir: "/sandbox", + remoteAgentWorkspaceDir: "/agent", + buildExecSpec: vi.fn(), + runShellCommand: vi.fn(), + runRemoteShellScript: vi.fn( + async (params) => + await runLocalShell({ + ...params, + roots, + }), + ), + syncLocalPathToRemote: vi.fn().mockResolvedValue(undefined), + } as unknown as OpenShellSandboxBackend; +} + +function rewriteLocalPaths(value: string, roots: { workspace: string; agent: string }) { + return value.replaceAll(roots.workspace, "/sandbox").replaceAll(roots.agent, "/agent"); +} + +function normalizeScriptForLocalShell(script: string) { + return script + .replace( + 'stats=$(stat -c "%F|%h" -- "$1")', + `stats=$(python3 - "$1" <<'PY' +import os, stat, sys +st = os.stat(sys.argv[1]) +kind = 'directory' if stat.S_ISDIR(st.st_mode) else 'regular file' if stat.S_ISREG(st.st_mode) else 'other' +print(f"{kind}|{st.st_nlink}") +PY +)`, + ) + .replace( + 'stat -c "%F|%s|%Y" -- "$1"', + `python3 - "$1" <<'PY' +import os, stat, sys +st = os.stat(sys.argv[1]) +kind = 'directory' if stat.S_ISDIR(st.st_mode) else 'regular file' if stat.S_ISREG(st.st_mode) else 'other' +print(f"{kind}|{st.st_size}|{int(st.st_mtime)}") +PY`, + ); +} + +describe("openshell remote fs bridge", () => { + it("writes, reads, renames, and removes files without local host paths", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-remote-local-"); + const remoteWorkspaceDir = await makeTempDir("openclaw-openshell-remote-workspace-"); + const remoteAgentDir = await makeTempDir("openclaw-openshell-remote-agent-"); + const remoteWorkspaceRealDir = await fs.realpath(remoteWorkspaceDir); + const remoteAgentRealDir = await fs.realpath(remoteAgentDir); + const backend = createBackendMock({ + workspace: remoteWorkspaceRealDir, + agent: remoteAgentRealDir, + }); + const sandbox = createSandboxTestContext({ + overrides: { + backendId: "openshell", + workspaceDir, + agentWorkspaceDir: workspaceDir, + containerWorkdir: "/sandbox", + }, + }); + + const bridge = createOpenShellRemoteFsBridge({ sandbox, backend }); + await bridge.writeFile({ + filePath: "nested/file.txt", + data: "hello", + mkdir: true, + }); + + expect(await fs.readFile(path.join(remoteWorkspaceRealDir, "nested", "file.txt"), "utf8")).toBe( + "hello", + ); + expect(await fs.readdir(workspaceDir)).toEqual([]); + + const resolved = bridge.resolvePath({ filePath: "nested/file.txt" }); + expect(resolved.hostPath).toBeUndefined(); + expect(resolved.containerPath).toBe("/sandbox/nested/file.txt"); + expect(await bridge.readFile({ filePath: "nested/file.txt" })).toEqual(Buffer.from("hello")); + expect(await bridge.stat({ filePath: "nested/file.txt" })).toEqual( + expect.objectContaining({ + type: "file", + size: 5, + }), + ); + + await bridge.rename({ + from: "nested/file.txt", + to: "nested/renamed.txt", + }); + await expect( + fs.readFile(path.join(remoteWorkspaceRealDir, "nested", "file.txt"), "utf8"), + ).rejects.toBeDefined(); + expect( + await fs.readFile(path.join(remoteWorkspaceRealDir, "nested", "renamed.txt"), "utf8"), + ).toBe("hello"); + + await bridge.remove({ + filePath: "nested/renamed.txt", + }); + await expect( + fs.readFile(path.join(remoteWorkspaceRealDir, "nested", "renamed.txt"), "utf8"), + ).rejects.toBeDefined(); + }); +}); diff --git a/extensions/openshell/src/remote-fs-bridge.ts b/extensions/openshell/src/remote-fs-bridge.ts new file mode 100644 index 00000000000..3560fa78f28 --- /dev/null +++ b/extensions/openshell/src/remote-fs-bridge.ts @@ -0,0 +1,550 @@ +import path from "node:path"; +import type { + SandboxContext, + SandboxFsBridge, + SandboxFsStat, + SandboxResolvedPath, +} from "openclaw/plugin-sdk/core"; +import { SANDBOX_PINNED_MUTATION_PYTHON } from "../../../src/agents/sandbox/fs-bridge-mutation-helper.js"; +import type { OpenShellSandboxBackend } from "./backend.js"; + +type ResolvedRemotePath = SandboxResolvedPath & { + writable: boolean; + mountRootPath: string; + source: "workspace" | "agent"; +}; + +type MountInfo = { + containerRoot: string; + writable: boolean; + source: "workspace" | "agent"; +}; + +export function createOpenShellRemoteFsBridge(params: { + sandbox: SandboxContext; + backend: OpenShellSandboxBackend; +}): SandboxFsBridge { + return new OpenShellRemoteFsBridge(params.sandbox, params.backend); +} + +class OpenShellRemoteFsBridge implements SandboxFsBridge { + constructor( + private readonly sandbox: SandboxContext, + private readonly backend: OpenShellSandboxBackend, + ) {} + + resolvePath(params: { filePath: string; cwd?: string }): SandboxResolvedPath { + const target = this.resolveTarget(params); + return { + relativePath: target.relativePath, + containerPath: target.containerPath, + }; + } + + async readFile(params: { + filePath: string; + cwd?: string; + signal?: AbortSignal; + }): Promise { + const target = this.resolveTarget(params); + const canonical = await this.resolveCanonicalPath({ + containerPath: target.containerPath, + action: "read files", + }); + await this.assertNoHardlinkedFile({ + containerPath: canonical, + action: "read files", + signal: params.signal, + }); + const result = await this.runRemoteScript({ + script: 'set -eu\ncat -- "$1"', + args: [canonical], + signal: params.signal, + }); + return result.stdout; + } + + async writeFile(params: { + filePath: string; + cwd?: string; + data: Buffer | string; + encoding?: BufferEncoding; + mkdir?: boolean; + signal?: AbortSignal; + }): Promise { + const target = this.resolveTarget(params); + this.ensureWritable(target, "write files"); + const pinned = await this.resolvePinnedParent({ + containerPath: target.containerPath, + action: "write files", + requireWritable: true, + }); + await this.assertNoHardlinkedFile({ + containerPath: target.containerPath, + action: "write files", + signal: params.signal, + }); + const buffer = Buffer.isBuffer(params.data) + ? params.data + : Buffer.from(params.data, params.encoding ?? "utf8"); + await this.runMutation({ + args: [ + "write", + pinned.mountRootPath, + pinned.relativeParentPath, + pinned.basename, + params.mkdir !== false ? "1" : "0", + ], + stdin: buffer, + signal: params.signal, + }); + } + + async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { + const target = this.resolveTarget(params); + this.ensureWritable(target, "create directories"); + const relativePath = path.posix.relative(target.mountRootPath, target.containerPath); + if (relativePath.startsWith("..") || path.posix.isAbsolute(relativePath)) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot create directories: ${target.containerPath}`, + ); + } + await this.runMutation({ + args: ["mkdirp", target.mountRootPath, relativePath === "." ? "" : relativePath], + signal: params.signal, + }); + } + + async remove(params: { + filePath: string; + cwd?: string; + recursive?: boolean; + force?: boolean; + signal?: AbortSignal; + }): Promise { + const target = this.resolveTarget(params); + this.ensureWritable(target, "remove files"); + const exists = await this.remotePathExists(target.containerPath, params.signal); + if (!exists) { + if (params.force === false) { + throw new Error(`Sandbox path not found; cannot remove files: ${target.containerPath}`); + } + return; + } + const pinned = await this.resolvePinnedParent({ + containerPath: target.containerPath, + action: "remove files", + requireWritable: true, + allowFinalSymlinkForUnlink: true, + }); + await this.runMutation({ + args: [ + "remove", + pinned.mountRootPath, + pinned.relativeParentPath, + pinned.basename, + params.recursive ? "1" : "0", + params.force === false ? "0" : "1", + ], + signal: params.signal, + allowFailure: params.force !== false, + }); + } + + async rename(params: { + from: string; + to: string; + cwd?: string; + signal?: AbortSignal; + }): Promise { + const from = this.resolveTarget({ filePath: params.from, cwd: params.cwd }); + const to = this.resolveTarget({ filePath: params.to, cwd: params.cwd }); + this.ensureWritable(from, "rename files"); + this.ensureWritable(to, "rename files"); + const fromPinned = await this.resolvePinnedParent({ + containerPath: from.containerPath, + action: "rename files", + requireWritable: true, + allowFinalSymlinkForUnlink: true, + }); + const toPinned = await this.resolvePinnedParent({ + containerPath: to.containerPath, + action: "rename files", + requireWritable: true, + }); + await this.runMutation({ + args: [ + "rename", + fromPinned.mountRootPath, + fromPinned.relativeParentPath, + fromPinned.basename, + toPinned.mountRootPath, + toPinned.relativeParentPath, + toPinned.basename, + "1", + ], + signal: params.signal, + }); + } + + async stat(params: { + filePath: string; + cwd?: string; + signal?: AbortSignal; + }): Promise { + const target = this.resolveTarget(params); + const exists = await this.remotePathExists(target.containerPath, params.signal); + if (!exists) { + return null; + } + const canonical = await this.resolveCanonicalPath({ + containerPath: target.containerPath, + action: "stat files", + signal: params.signal, + }); + await this.assertNoHardlinkedFile({ + containerPath: canonical, + action: "stat files", + signal: params.signal, + }); + const result = await this.runRemoteScript({ + script: 'set -eu\nstat -c "%F|%s|%Y" -- "$1"', + args: [canonical], + signal: params.signal, + }); + const output = result.stdout.toString("utf8").trim(); + const [kindRaw = "", sizeRaw = "0", mtimeRaw = "0"] = output.split("|"); + return { + type: kindRaw === "directory" ? "directory" : kindRaw === "regular file" ? "file" : "other", + size: Number(sizeRaw), + mtimeMs: Number(mtimeRaw) * 1000, + }; + } + + private resolveTarget(params: { filePath: string; cwd?: string }): ResolvedRemotePath { + const workspaceRoot = path.resolve(this.sandbox.workspaceDir); + const agentRoot = path.resolve(this.sandbox.agentWorkspaceDir); + const workspaceContainerRoot = normalizeContainerPath(this.backend.remoteWorkspaceDir); + const agentContainerRoot = normalizeContainerPath(this.backend.remoteAgentWorkspaceDir); + const mounts: MountInfo[] = [ + { + containerRoot: workspaceContainerRoot, + writable: this.sandbox.workspaceAccess === "rw", + source: "workspace", + }, + ]; + if ( + this.sandbox.workspaceAccess !== "none" && + path.resolve(this.sandbox.agentWorkspaceDir) !== path.resolve(this.sandbox.workspaceDir) + ) { + mounts.push({ + containerRoot: agentContainerRoot, + writable: this.sandbox.workspaceAccess === "rw", + source: "agent", + }); + } + + const input = params.filePath.trim(); + const inputPosix = input.replace(/\\/g, "/"); + const maybeContainerMount = path.posix.isAbsolute(inputPosix) + ? this.resolveMountByContainerPath(mounts, normalizeContainerPath(inputPosix)) + : null; + if (maybeContainerMount) { + return this.toResolvedPath({ + mount: maybeContainerMount, + containerPath: normalizeContainerPath(inputPosix), + }); + } + + const hostCwd = params.cwd ? path.resolve(params.cwd) : workspaceRoot; + const hostCandidate = path.isAbsolute(input) + ? path.resolve(input) + : path.resolve(hostCwd, input); + if (isPathInside(workspaceRoot, hostCandidate)) { + const relative = toPosixRelative(workspaceRoot, hostCandidate); + return this.toResolvedPath({ + mount: mounts[0]!, + containerPath: relative + ? path.posix.join(workspaceContainerRoot, relative) + : workspaceContainerRoot, + }); + } + if (mounts[1] && isPathInside(agentRoot, hostCandidate)) { + const relative = toPosixRelative(agentRoot, hostCandidate); + return this.toResolvedPath({ + mount: mounts[1], + containerPath: relative + ? path.posix.join(agentContainerRoot, relative) + : agentContainerRoot, + }); + } + + if (params.cwd) { + const cwdPosix = params.cwd.replace(/\\/g, "/"); + if (path.posix.isAbsolute(cwdPosix)) { + const cwdContainer = normalizeContainerPath(cwdPosix); + const cwdMount = this.resolveMountByContainerPath(mounts, cwdContainer); + if (cwdMount) { + return this.toResolvedPath({ + mount: cwdMount, + containerPath: normalizeContainerPath(path.posix.resolve(cwdContainer, inputPosix)), + }); + } + } + } + + throw new Error(`Sandbox path escapes allowed mounts; cannot access: ${params.filePath}`); + } + + private toResolvedPath(params: { mount: MountInfo; containerPath: string }): ResolvedRemotePath { + const relative = path.posix.relative(params.mount.containerRoot, params.containerPath); + if (relative.startsWith("..") || path.posix.isAbsolute(relative)) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot access: ${params.containerPath}`, + ); + } + return { + relativePath: + params.mount.source === "workspace" + ? relative === "." + ? "" + : relative + : relative === "." + ? params.mount.containerRoot + : `${params.mount.containerRoot}/${relative}`, + containerPath: params.containerPath, + writable: params.mount.writable, + mountRootPath: params.mount.containerRoot, + source: params.mount.source, + }; + } + + private resolveMountByContainerPath( + mounts: MountInfo[], + containerPath: string, + ): MountInfo | null { + const ordered = [...mounts].toSorted((a, b) => b.containerRoot.length - a.containerRoot.length); + for (const mount of ordered) { + if (isPathInsideContainerRoot(mount.containerRoot, containerPath)) { + return mount; + } + } + return null; + } + + private ensureWritable(target: ResolvedRemotePath, action: string) { + if (this.sandbox.workspaceAccess !== "rw" || !target.writable) { + throw new Error(`Sandbox path is read-only; cannot ${action}: ${target.containerPath}`); + } + } + + private async remotePathExists(containerPath: string, signal?: AbortSignal): Promise { + const result = await this.runRemoteScript({ + script: 'if [ -e "$1" ] || [ -L "$1" ]; then printf "1\\n"; else printf "0\\n"; fi', + args: [containerPath], + signal, + }); + return result.stdout.toString("utf8").trim() === "1"; + } + + private async resolveCanonicalPath(params: { + containerPath: string; + action: string; + allowFinalSymlinkForUnlink?: boolean; + signal?: AbortSignal; + }): Promise { + const script = [ + "set -eu", + 'target="$1"', + 'allow_final="$2"', + 'suffix=""', + 'probe="$target"', + 'if [ "$allow_final" = "1" ] && [ -L "$target" ]; then probe=$(dirname -- "$target"); fi', + 'cursor="$probe"', + 'while [ ! -e "$cursor" ] && [ ! -L "$cursor" ]; do', + ' parent=$(dirname -- "$cursor")', + ' if [ "$parent" = "$cursor" ]; then break; fi', + ' base=$(basename -- "$cursor")', + ' suffix="/$base$suffix"', + ' cursor="$parent"', + "done", + 'canonical=$(readlink -f -- "$cursor")', + 'printf "%s%s\\n" "$canonical" "$suffix"', + ].join("\n"); + const result = await this.runRemoteScript({ + script, + args: [params.containerPath, params.allowFinalSymlinkForUnlink ? "1" : "0"], + signal: params.signal, + }); + const canonical = normalizeContainerPath(result.stdout.toString("utf8").trim()); + const mount = this.resolveMountByContainerPath( + [ + { + containerRoot: normalizeContainerPath(this.backend.remoteWorkspaceDir), + writable: this.sandbox.workspaceAccess === "rw", + source: "workspace", + }, + ...(this.sandbox.workspaceAccess !== "none" && + path.resolve(this.sandbox.agentWorkspaceDir) !== path.resolve(this.sandbox.workspaceDir) + ? [ + { + containerRoot: normalizeContainerPath(this.backend.remoteAgentWorkspaceDir), + writable: this.sandbox.workspaceAccess === "rw", + source: "agent" as const, + }, + ] + : []), + ], + canonical, + ); + if (!mount) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${params.action}: ${params.containerPath}`, + ); + } + return canonical; + } + + private async assertNoHardlinkedFile(params: { + containerPath: string; + action: string; + signal?: AbortSignal; + }): Promise { + const result = await this.runRemoteScript({ + script: [ + 'if [ ! -e "$1" ] && [ ! -L "$1" ]; then exit 0; fi', + 'stats=$(stat -c "%F|%h" -- "$1")', + 'printf "%s\\n" "$stats"', + ].join("\n"), + args: [params.containerPath], + signal: params.signal, + allowFailure: true, + }); + const output = result.stdout.toString("utf8").trim(); + if (!output) { + return; + } + const [kind = "", linksRaw = "1"] = output.split("|"); + if (kind === "regular file" && Number(linksRaw) > 1) { + throw new Error( + `Hardlinked path is not allowed under sandbox mount root: ${params.containerPath}`, + ); + } + } + + private async resolvePinnedParent(params: { + containerPath: string; + action: string; + requireWritable?: boolean; + allowFinalSymlinkForUnlink?: boolean; + }): Promise<{ mountRootPath: string; relativeParentPath: string; basename: string }> { + const basename = path.posix.basename(params.containerPath); + if (!basename || basename === "." || basename === "/") { + throw new Error(`Invalid sandbox entry target: ${params.containerPath}`); + } + const canonicalParent = await this.resolveCanonicalPath({ + containerPath: normalizeContainerPath(path.posix.dirname(params.containerPath)), + action: params.action, + allowFinalSymlinkForUnlink: params.allowFinalSymlinkForUnlink, + }); + const mount = this.resolveMountByContainerPath( + [ + { + containerRoot: normalizeContainerPath(this.backend.remoteWorkspaceDir), + writable: this.sandbox.workspaceAccess === "rw", + source: "workspace", + }, + ...(this.sandbox.workspaceAccess !== "none" && + path.resolve(this.sandbox.agentWorkspaceDir) !== path.resolve(this.sandbox.workspaceDir) + ? [ + { + containerRoot: normalizeContainerPath(this.backend.remoteAgentWorkspaceDir), + writable: this.sandbox.workspaceAccess === "rw", + source: "agent" as const, + }, + ] + : []), + ], + canonicalParent, + ); + if (!mount) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${params.action}: ${params.containerPath}`, + ); + } + if (params.requireWritable && !mount.writable) { + throw new Error( + `Sandbox path is read-only; cannot ${params.action}: ${params.containerPath}`, + ); + } + const relativeParentPath = path.posix.relative(mount.containerRoot, canonicalParent); + if (relativeParentPath.startsWith("..") || path.posix.isAbsolute(relativeParentPath)) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${params.action}: ${params.containerPath}`, + ); + } + return { + mountRootPath: mount.containerRoot, + relativeParentPath: relativeParentPath === "." ? "" : relativeParentPath, + basename, + }; + } + + private async runMutation(params: { + args: string[]; + stdin?: Buffer | string; + signal?: AbortSignal; + allowFailure?: boolean; + }) { + await this.runRemoteScript({ + script: [ + "set -eu", + "python3 /dev/fd/3 \"$@\" 3<<'PY'", + SANDBOX_PINNED_MUTATION_PYTHON, + "PY", + ].join("\n"), + args: params.args, + stdin: params.stdin, + signal: params.signal, + allowFailure: params.allowFailure, + }); + } + + private async runRemoteScript(params: { + script: string; + args?: string[]; + stdin?: Buffer | string; + signal?: AbortSignal; + allowFailure?: boolean; + }) { + return await this.backend.runRemoteShellScript({ + script: params.script, + args: params.args, + stdin: params.stdin, + signal: params.signal, + allowFailure: params.allowFailure, + }); + } +} + +function normalizeContainerPath(value: string): string { + const normalized = path.posix.normalize(value.trim() || "/"); + return normalized.startsWith("/") ? normalized : `/${normalized}`; +} + +function isPathInsideContainerRoot(root: string, candidate: string): boolean { + const normalizedRoot = normalizeContainerPath(root); + const normalizedCandidate = normalizeContainerPath(candidate); + return ( + normalizedCandidate === normalizedRoot || normalizedCandidate.startsWith(`${normalizedRoot}/`) + ); +} + +function isPathInside(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); +} + +function toPosixRelative(root: string, candidate: string): string { + return path.relative(root, candidate).split(path.sep).filter(Boolean).join(path.posix.sep); +} diff --git a/src/agents/apply-patch.test.ts b/src/agents/apply-patch.test.ts index 1f305379b5d..5182dfdf0af 100644 --- a/src/agents/apply-patch.test.ts +++ b/src/agents/apply-patch.test.ts @@ -361,4 +361,46 @@ describe("applyPatch", () => { } }); }); + + it("uses container paths when the sandbox bridge has no local host path", async () => { + const files = new Map([["/sandbox/source.txt", "before\n"]]); + const bridge = { + resolvePath: ({ filePath }: { filePath: string }) => ({ + relativePath: filePath, + containerPath: `/sandbox/${filePath}`, + }), + readFile: vi.fn(async ({ filePath }: { filePath: string }) => + Buffer.from(files.get(filePath) ?? "", "utf8"), + ), + writeFile: vi.fn(async ({ filePath, data }: { filePath: string; data: Buffer | string }) => { + files.set(filePath, Buffer.isBuffer(data) ? data.toString("utf8") : data); + }), + remove: vi.fn(async ({ filePath }: { filePath: string }) => { + files.delete(filePath); + }), + mkdirp: vi.fn(async () => {}), + }; + + const patch = `*** Begin Patch +*** Update File: source.txt +@@ +-before ++after +*** End Patch`; + + const result = await applyPatch(patch, { + cwd: "/local/workspace", + sandbox: { + root: "/local/workspace", + bridge: bridge as never, + }, + }); + + expect(files.get("/sandbox/source.txt")).toBe("after\n"); + expect(result.summary.modified).toEqual(["source.txt"]); + expect(bridge.readFile).toHaveBeenCalledWith({ + filePath: "/sandbox/source.txt", + cwd: "/local/workspace", + }); + }); }); diff --git a/src/agents/apply-patch.ts b/src/agents/apply-patch.ts index d7a5dc1e0ff..0fc612923c1 100644 --- a/src/agents/apply-patch.ts +++ b/src/agents/apply-patch.ts @@ -313,7 +313,7 @@ async function resolvePatchPath( filePath, cwd: options.cwd, }); - if (options.workspaceOnly !== false) { + if (options.workspaceOnly !== false && resolved.hostPath) { await assertSandboxPath({ filePath: resolved.hostPath, cwd: options.cwd, @@ -323,8 +323,8 @@ async function resolvePatchPath( }); } return { - resolved: resolved.hostPath, - display: resolved.relativePath || resolved.hostPath, + resolved: resolved.hostPath ?? resolved.containerPath, + display: resolved.relativePath || resolved.containerPath, }; } diff --git a/src/agents/sandbox-media-paths.test.ts b/src/agents/sandbox-media-paths.test.ts index 4179c2a68ef..0007e943fdd 100644 --- a/src/agents/sandbox-media-paths.test.ts +++ b/src/agents/sandbox-media-paths.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it, vi } from "vitest"; -import { createSandboxBridgeReadFile } from "./sandbox-media-paths.js"; +import { + createSandboxBridgeReadFile, + resolveSandboxedBridgeMediaPath, +} from "./sandbox-media-paths.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; describe("createSandboxBridgeReadFile", () => { @@ -19,4 +22,24 @@ describe("createSandboxBridgeReadFile", () => { cwd: "/tmp/sandbox-root", }); }); + + it("falls back to container paths when the bridge has no host path", async () => { + const stat = vi.fn(async () => ({ type: "file", size: 1, mtimeMs: 1 })); + const resolved = await resolveSandboxedBridgeMediaPath({ + sandbox: { + root: "/tmp/sandbox-root", + bridge: { + resolvePath: ({ filePath }: { filePath: string }) => ({ + relativePath: filePath, + containerPath: `/sandbox/${filePath}`, + }), + stat, + } as unknown as SandboxFsBridge, + }, + mediaPath: "image.png", + }); + + expect(resolved).toEqual({ resolved: "/sandbox/image.png" }); + expect(stat).not.toHaveBeenCalled(); + }); }); diff --git a/src/agents/sandbox-media-paths.ts b/src/agents/sandbox-media-paths.ts index 3c6b2614c94..1c46f392482 100644 --- a/src/agents/sandbox-media-paths.ts +++ b/src/agents/sandbox-media-paths.ts @@ -44,8 +44,10 @@ export async function resolveSandboxedBridgeMediaPath(params: { }); try { const resolved = resolveDirect(); - await enforceWorkspaceBoundary(resolved.hostPath); - return { resolved: resolved.hostPath }; + if (resolved.hostPath) { + await enforceWorkspaceBoundary(resolved.hostPath); + } + return { resolved: resolved.hostPath ?? resolved.containerPath }; } catch (err) { const fallbackDir = params.inboundFallbackDir?.trim(); if (!fallbackDir) { @@ -67,7 +69,12 @@ export async function resolveSandboxedBridgeMediaPath(params: { filePath: fallbackPath, cwd: params.sandbox.root, }); - await enforceWorkspaceBoundary(resolvedFallback.hostPath); - return { resolved: resolvedFallback.hostPath, rewrittenFrom: filePath }; + if (resolvedFallback.hostPath) { + await enforceWorkspaceBoundary(resolvedFallback.hostPath); + } + return { + resolved: resolvedFallback.hostPath ?? resolvedFallback.containerPath, + rewrittenFrom: filePath, + }; } } diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index 16c307e053c..7941b2b6828 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -24,7 +24,7 @@ type RunCommandOptions = { }; export type SandboxResolvedPath = { - hostPath: string; + hostPath?: string; relativePath: string; containerPath: string; }; diff --git a/src/agents/test-helpers/host-sandbox-fs-bridge.ts b/src/agents/test-helpers/host-sandbox-fs-bridge.ts index 93bb34969a8..fc466f0ea67 100644 --- a/src/agents/test-helpers/host-sandbox-fs-bridge.ts +++ b/src/agents/test-helpers/host-sandbox-fs-bridge.ts @@ -10,10 +10,16 @@ export function createSandboxFsBridgeFromResolver( resolvePath: ({ filePath, cwd }) => resolvePath(filePath, cwd), readFile: async ({ filePath, cwd }) => { const target = resolvePath(filePath, cwd); + if (!target.hostPath) { + throw new Error(`Expected hostPath for ${target.containerPath}`); + } return fs.readFile(target.hostPath); }, writeFile: async ({ filePath, cwd, data, mkdir = true }) => { const target = resolvePath(filePath, cwd); + if (!target.hostPath) { + throw new Error(`Expected hostPath for ${target.containerPath}`); + } if (mkdir) { await fs.mkdir(path.dirname(target.hostPath), { recursive: true }); } @@ -22,10 +28,16 @@ export function createSandboxFsBridgeFromResolver( }, mkdirp: async ({ filePath, cwd }) => { const target = resolvePath(filePath, cwd); + if (!target.hostPath) { + throw new Error(`Expected hostPath for ${target.containerPath}`); + } await fs.mkdir(target.hostPath, { recursive: true }); }, remove: async ({ filePath, cwd, recursive, force }) => { const target = resolvePath(filePath, cwd); + if (!target.hostPath) { + throw new Error(`Expected hostPath for ${target.containerPath}`); + } await fs.rm(target.hostPath, { recursive: recursive ?? false, force: force ?? false, @@ -34,12 +46,20 @@ export function createSandboxFsBridgeFromResolver( rename: async ({ from, to, cwd }) => { const source = resolvePath(from, cwd); const target = resolvePath(to, cwd); + if (!source.hostPath || !target.hostPath) { + throw new Error( + `Expected hostPath for rename: ${source.containerPath} -> ${target.containerPath}`, + ); + } await fs.mkdir(path.dirname(target.hostPath), { recursive: true }); await fs.rename(source.hostPath, target.hostPath); }, stat: async ({ filePath, cwd }) => { try { const target = resolvePath(filePath, cwd); + if (!target.hostPath) { + throw new Error(`Expected hostPath for ${target.containerPath}`); + } const stats = await fs.stat(target.hostPath); return { type: stats.isDirectory() ? "directory" : stats.isFile() ? "file" : "other",