From c7ae4ed04dcfb6830cdc381d0c9ad1455c7bebb4 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 25 Feb 2026 01:39:56 +0000 Subject: [PATCH] fix: harden sandbox fs dash-path regression coverage (#25891) (thanks @albertlieyingadrian) --- CHANGELOG.md | 1 + src/agents/sandbox/fs-bridge.test.ts | 11 +++++++++++ src/agents/sandbox/fs-bridge.ts | 27 +++++++-------------------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9654c1c218..2f227da4c9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Docs: https://docs.openclaw.ai - Discord/Voice reliability: restore runtime DAVE dependency (`@snazzah/davey`), add configurable DAVE join options (`channels.discord.voice.daveEncryption` and `channels.discord.voice.decryptionFailureTolerance`), clean up voice listeners/session teardown, guard against stale connection events, and trigger controlled rejoin recovery after repeated decrypt failures to improve inbound STT stability under DAVE receive errors. (#25861, #25372, #24883, #24825, #23890, #23105, #22961, #23421, #23278, #23032) - Matrix/Read receipts: send read receipts as soon as Matrix messages arrive (before handler pipeline work), so clients no longer show long-lived unread/sent states while replies are processing. (#25841, #25840) Thanks @joshjhall. - Sandbox/FS bridge: build canonical-path shell scripts with newline separators (not `; ` joins) to avoid POSIX `sh` `do;` syntax errors that broke sandbox file/image read-write operations. (#25737, #25824, #25868) Thanks @DennisGoldfinger and @peteragility. +- Sandbox/FS bridge tests: add regression coverage for dash-leading basenames to confirm sandbox file reads resolve to absolute container paths (and avoid shell-option misdiagnosis for dashed filenames). (#25891) Thanks @albertlieyingadrian. - Routing/Session isolation: harden followup routing so explicit cross-channel origin replies never fall back to the active dispatcher on route failure, preserve queued overflow summary routing metadata (`channel`/`to`/`thread`) across followup drain, and prefer originating channel context over internal provider tags for embedded followup runs. This prevents webchat/control-ui context from hijacking Discord-targeted replies in shared sessions. (#25864) Thanks @Gamedesigner. - Messaging tool dedupe: treat originating channel metadata as authoritative for same-target `message.send` suppression in proactive runs (heartbeat/cron/exec-event), including synthetic-provider contexts, so `delivery-mirror` transcript entries no longer cause duplicate Telegram sends. (#25835) Thanks @jadeathena84-arch. - Cron/Heartbeat delivery: stop inheriting cached session `lastThreadId` for heartbeat-mode target resolution unless a thread/topic is explicitly requested, so announce-mode cron and heartbeat deliveries stay on top-level destinations instead of leaking into active conversation threads. (#25730) Thanks @markshields-tl. diff --git a/src/agents/sandbox/fs-bridge.test.ts b/src/agents/sandbox/fs-bridge.test.ts index 98744f3562d..aa3144ca331 100644 --- a/src/agents/sandbox/fs-bridge.test.ts +++ b/src/agents/sandbox/fs-bridge.test.ts @@ -123,6 +123,17 @@ describe("sandbox fs bridge shell compatibility", () => { expect(readPath).toContain("file_1095---"); }); + it("resolves dash-leading basenames into absolute container paths", async () => { + const bridge = createSandboxFsBridge({ sandbox: createSandbox() }); + + await bridge.readFile({ filePath: "--leading.txt" }); + + const readCall = findCallByScriptFragment('cat -- "$1"'); + expect(readCall).toBeDefined(); + const readPath = readCall ? getDockerPathArg(readCall[0]) : ""; + expect(readPath).toBe("/workspace/--leading.txt"); + }); + it("resolves bind-mounted absolute container paths for reads", async () => { const sandbox = createSandbox({ docker: { diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index 1d0d4266b20..dee44e1b237 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -96,7 +96,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { const target = this.resolveResolvedPath(params); await this.assertPathSafety(target, { action: "read files" }); const result = await this.runCommand('set -eu; cat -- "$1"', { - args: [ensurePathNotInterpretedAsOption(target.containerPath)], + args: [target.containerPath], signal: params.signal, }); return result.stdout; @@ -121,7 +121,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { ? 'set -eu; cat >"$1"' : 'set -eu; dir=$(dirname -- "$1"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; cat >"$1"'; await this.runCommand(script, { - args: [ensurePathNotInterpretedAsOption(target.containerPath)], + args: [target.containerPath], stdin: buffer, signal: params.signal, }); @@ -132,7 +132,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { this.ensureWriteAccess(target, "create directories"); await this.assertPathSafety(target, { action: "create directories", requireWritable: true }); await this.runCommand('set -eu; mkdir -p -- "$1"', { - args: [ensurePathNotInterpretedAsOption(target.containerPath)], + args: [target.containerPath], signal: params.signal, }); } @@ -156,7 +156,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { ); const rmCommand = flags.length > 0 ? `rm ${flags.join(" ")}` : "rm"; await this.runCommand(`set -eu; ${rmCommand} -- "$1"`, { - args: [ensurePathNotInterpretedAsOption(target.containerPath)], + args: [target.containerPath], signal: params.signal, }); } @@ -183,7 +183,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { await this.runCommand( 'set -eu; dir=$(dirname -- "$2"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; mv -- "$1" "$2"', { - args: [ensurePathNotInterpretedAsOption(from.containerPath), ensurePathNotInterpretedAsOption(to.containerPath)], + args: [from.containerPath, to.containerPath], signal: params.signal, }, ); @@ -197,7 +197,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { const target = this.resolveResolvedPath(params); await this.assertPathSafety(target, { action: "stat files" }); const result = await this.runCommand('set -eu; stat -c "%F|%s|%Y" -- "$1"', { - args: [ensurePathNotInterpretedAsOption(target.containerPath)], + args: [target.containerPath], signal: params.signal, allowFailure: true, }); @@ -307,7 +307,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { 'printf "%s%s\\n" "$canonical" "$suffix"', ].join("\n"); const result = await this.runCommand(script, { - args: [ensurePathNotInterpretedAsOption(params.containerPath), params.allowFinalSymlink ? "1" : "0"], + args: [params.containerPath, params.allowFinalSymlink ? "1" : "0"], }); const canonical = result.stdout.toString("utf8").trim(); if (!canonical.startsWith("/")) { @@ -363,19 +363,6 @@ function isPathInsidePosix(root: string, target: string): boolean { return target === root || target.startsWith(`${root}/`); } -/** - * Ensure the path is not interpreted as a shell option. - * Paths starting with "-" can be interpreted as command options by the shell. - * Prepend "./" to prevent this interpretation. - */ -function ensurePathNotInterpretedAsOption(path: string): string { - // If path starts with a hyphen (either - or --), prepend ./ to prevent interpretation as option - if (path.startsWith("-") || path.startsWith("--")) { - return "./" + path; - } - return path; -} - async function assertNoHostSymlinkEscape(params: { absolutePath: string; rootPath: string;