From 17ede52a4be3034f6ec4b883ac6b81ad0101558a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 16:34:59 +0000 Subject: [PATCH] fix(security): harden sandbox media staging destination writes --- CHANGELOG.md | 1 + ...bound-media-into-sandbox-workspace.test.ts | 40 ++++++++++++ src/auto-reply/reply/stage-sandbox-media.ts | 61 ++++++++++++++++--- 3 files changed, 93 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afda3f951bd..0fe7bfd4f37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -245,6 +245,7 @@ Docs: https://docs.openclaw.ai - Security/Audit: flag `gateway.controlUi.allowedOrigins=["*"]` as a high-risk configuration (severity based on bind exposure), and add a Feishu doc-tool warning that `owner_open_id` on `feishu_doc` create can grant document permissions. - Slack/download-file scoping: thread/channel-aware `download-file` actions now propagate optional scope context and reject downloads when Slack metadata definitively shows the file is outside the requested channel/thread, while preserving legacy behavior when share metadata is unavailable. - Security/Sandbox media reads: eliminate sandbox media TOCTOU symlink-retarget escapes by enforcing root-scoped boundary-safe reads at attachment/image load time and consolidating shared safe-read helpers across sandbox media callsites. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/Sandbox media staging: block destination symlink escapes in `stageSandboxMedia` by replacing direct destination copies with root-scoped safe writes for both local and SCP-staged attachments, preventing out-of-workspace file overwrite through `media/inbound` alias traversal. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting. - Node host/service auth env: include `OPENCLAW_GATEWAY_TOKEN` in `openclaw node install` service environments (with `CLAWDBOT_GATEWAY_TOKEN` compatibility fallback) so installed node services keep remote gateway token auth across restart/reboot. Fixes #31041. Thanks @OneStepAt4time for reporting, @byungsker, @liuxiaopai-ai, and @vincentkoc. - Security/Subagents sandbox inheritance: block sandboxed sessions from spawning cross-agent subagents that would run unsandboxed, preventing runtime sandbox downgrade via `sessions_spawn agentId`. Thanks @tdjackey for reporting. - Security/Workspace safe writes: harden `writeFileWithinRoot` against symlink-retarget TOCTOU races by opening existing files without truncation, creating missing files with exclusive create, deferring truncation until post-open identity+boundary validation, and removing out-of-root create artifacts on blocked races; added regression tests for truncate/create race paths. This ships in the next npm release (`2026.3.2`). Thanks @tdjackey for reporting. diff --git a/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts b/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts index 919e88a5bcd..7c853fd1771 100644 --- a/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts +++ b/src/auto-reply/reply.triggers.trigger-handling.stages-inbound-media-into-sandbox-workspace.test.ts @@ -101,4 +101,44 @@ describe("stageSandboxMedia", () => { } }); }); + + it("blocks destination symlink escapes when staging into sandbox workspace", async () => { + await withSandboxMediaTempHome("openclaw-triggers-", async (home) => { + const cfg = createSandboxMediaStageConfig(home); + const workspaceDir = join(home, "openclaw"); + const sandboxDir = join(home, "sandboxes", "session"); + vi.mocked(ensureSandboxWorkspaceForSession).mockResolvedValue({ + workspaceDir: sandboxDir, + containerWorkdir: "/work", + }); + + const inboundDir = join(home, ".openclaw", "media", "inbound"); + await fs.mkdir(inboundDir, { recursive: true }); + const mediaPath = join(inboundDir, "payload.txt"); + await fs.writeFile(mediaPath, "PAYLOAD"); + + const outsideDir = join(home, "outside"); + const outsideInboundDir = join(outsideDir, "inbound"); + await fs.mkdir(outsideInboundDir, { recursive: true }); + const victimPath = join(outsideDir, "victim.txt"); + await fs.writeFile(victimPath, "ORIGINAL"); + + await fs.mkdir(sandboxDir, { recursive: true }); + await fs.symlink(outsideDir, join(sandboxDir, "media")); + await fs.symlink(victimPath, join(outsideInboundDir, basename(mediaPath))); + + const { ctx, sessionCtx } = createSandboxMediaContexts(mediaPath); + await stageSandboxMedia({ + ctx, + sessionCtx, + cfg, + sessionKey: "agent:main:main", + workspaceDir, + }); + + await expect(fs.readFile(victimPath, "utf8")).resolves.toBe("ORIGINAL"); + expect(ctx.MediaPath).toBe(mediaPath); + expect(sessionCtx.MediaPath).toBe(mediaPath); + }); + }); }); diff --git a/src/auto-reply/reply/stage-sandbox-media.ts b/src/auto-reply/reply/stage-sandbox-media.ts index 6d887673537..12719c9b96c 100644 --- a/src/auto-reply/reply/stage-sandbox-media.ts +++ b/src/auto-reply/reply/stage-sandbox-media.ts @@ -6,7 +6,9 @@ import { assertSandboxPath } from "../../agents/sandbox-paths.js"; import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js"; import type { OpenClawConfig } from "../../config/config.js"; import { logVerbose } from "../../globals.js"; +import { readLocalFileSafely, writeFileWithinRoot } from "../../infra/fs-safe.js"; import { normalizeScpRemoteHost } from "../../infra/scp-host.js"; +import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js"; import { isInboundPathAllowed, resolveIMessageRemoteAttachmentRoots, @@ -69,11 +71,7 @@ export async function stageSandboxMedia(params: { }; try { - // For sandbox: /media/inbound, for remote cache: use dir directly - const destDir = sandbox - ? path.join(effectiveWorkspaceDir, "media", "inbound") - : effectiveWorkspaceDir; - await fs.mkdir(destDir, { recursive: true }); + await fs.mkdir(effectiveWorkspaceDir, { recursive: true }); const remoteAttachmentRoots = resolveIMessageRemoteAttachmentRoots({ cfg, accountId: ctx.AccountId, @@ -139,12 +137,22 @@ export async function stageSandboxMedia(params: { } usedNames.add(fileName); - const dest = path.join(destDir, fileName); + const relativeDest = sandbox ? path.join("media", "inbound", fileName) : fileName; + const dest = path.join(effectiveWorkspaceDir, relativeDest); if (ctx.MediaRemoteHost) { - // Always use SCP when remote host is configured - local paths refer to remote machine - await scpFile(ctx.MediaRemoteHost, source, dest); + // Remote media arrives via SCP to a temp file, then we write into root with alias guards. + await stageRemoteFileIntoRoot({ + remoteHost: ctx.MediaRemoteHost, + remotePath: source, + rootDir: effectiveWorkspaceDir, + relativeDestPath: relativeDest, + }); } else { - await fs.copyFile(source, dest); + await stageLocalFileIntoRoot({ + sourcePath: source, + rootDir: effectiveWorkspaceDir, + relativeDestPath: relativeDest, + }); } // For sandbox use relative path, for remote cache use absolute path const stagedPath = sandbox ? path.posix.join("media", "inbound", fileName) : dest; @@ -193,6 +201,41 @@ export async function stageSandboxMedia(params: { } } +async function stageLocalFileIntoRoot(params: { + sourcePath: string; + rootDir: string; + relativeDestPath: string; +}): Promise { + const safeRead = await readLocalFileSafely({ filePath: params.sourcePath }); + await writeFileWithinRoot({ + rootDir: params.rootDir, + relativePath: params.relativeDestPath, + data: safeRead.buffer, + }); +} + +async function stageRemoteFileIntoRoot(params: { + remoteHost: string; + remotePath: string; + rootDir: string; + relativeDestPath: string; +}): Promise { + const tmpRoot = resolvePreferredOpenClawTmpDir(); + await fs.mkdir(tmpRoot, { recursive: true }); + const tmpDir = await fs.mkdtemp(path.join(tmpRoot, "stage-sandbox-media-")); + const tmpPath = path.join(tmpDir, "download"); + try { + await scpFile(params.remoteHost, params.remotePath, tmpPath); + await stageLocalFileIntoRoot({ + sourcePath: tmpPath, + rootDir: params.rootDir, + relativeDestPath: params.relativeDestPath, + }); + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + } +} + async function scpFile(remoteHost: string, remotePath: string, localPath: string): Promise { const safeRemoteHost = normalizeScpRemoteHost(remoteHost); if (!safeRemoteHost) {