refactor(security): split sandbox media staging and stream safe copies

This commit is contained in:
Peter Steinberger
2026-03-02 16:53:10 +00:00
parent 7a7eee920a
commit 4a80311628
4 changed files with 350 additions and 150 deletions

View File

@@ -1,6 +1,7 @@
import fs from "node:fs/promises";
import { basename, join } from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { MEDIA_MAX_BYTES } from "../media/store.js";
import {
createSandboxMediaContexts,
createSandboxMediaStageConfig,
@@ -141,4 +142,36 @@ describe("stageSandboxMedia", () => {
expect(sessionCtx.MediaPath).toBe(mediaPath);
});
});
it("skips oversized media staging and keeps original media paths", 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, "oversized.bin");
await fs.writeFile(mediaPath, Buffer.alloc(MEDIA_MAX_BYTES + 1, 0x41));
const { ctx, sessionCtx } = createSandboxMediaContexts(mediaPath);
await stageSandboxMedia({
ctx,
sessionCtx,
cfg,
sessionKey: "agent:main:main",
workspaceDir,
});
await expect(
fs.stat(join(sandboxDir, "media", "inbound", basename(mediaPath))),
).rejects.toThrow();
expect(ctx.MediaPath).toBe(mediaPath);
expect(sessionCtx.MediaPath).toBe(mediaPath);
});
});
});

View File

@@ -6,17 +6,19 @@ 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 { copyFileWithinRoot, SafeOpenError } from "../../infra/fs-safe.js";
import { normalizeScpRemoteHost } from "../../infra/scp-host.js";
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
import {
isInboundPathAllowed,
resolveIMessageRemoteAttachmentRoots,
} from "../../media/inbound-path-policy.js";
import { getMediaDir } from "../../media/store.js";
import { getMediaDir, MEDIA_MAX_BYTES } from "../../media/store.js";
import { CONFIG_DIR } from "../../utils.js";
import type { MsgContext, TemplateContext } from "../templating.js";
const STAGED_MEDIA_MAX_BYTES = MEDIA_MAX_BYTES;
export async function stageSandboxMedia(params: {
ctx: MsgContext;
sessionCtx: TemplateContext;
@@ -26,13 +28,7 @@ export async function stageSandboxMedia(params: {
}) {
const { ctx, sessionCtx, cfg, sessionKey, workspaceDir } = params;
const hasPathsArray = Array.isArray(ctx.MediaPaths) && ctx.MediaPaths.length > 0;
const pathsFromArray = Array.isArray(ctx.MediaPaths) ? ctx.MediaPaths : undefined;
const rawPaths =
pathsFromArray && pathsFromArray.length > 0
? pathsFromArray
: ctx.MediaPath?.trim()
? [ctx.MediaPath.trim()]
: [];
const rawPaths = resolveRawPaths(ctx);
if (rawPaths.length === 0 || !sessionKey) {
return;
}
@@ -52,165 +48,88 @@ export async function stageSandboxMedia(params: {
return;
}
const resolveAbsolutePath = (value: string): string | null => {
let resolved = value.trim();
if (!resolved) {
return null;
}
if (resolved.startsWith("file://")) {
try {
resolved = fileURLToPath(resolved);
} catch {
return null;
}
}
if (!path.isAbsolute(resolved)) {
return null;
}
return resolved;
};
await fs.mkdir(effectiveWorkspaceDir, { recursive: true });
const remoteAttachmentRoots = resolveIMessageRemoteAttachmentRoots({
cfg,
accountId: ctx.AccountId,
});
try {
await fs.mkdir(effectiveWorkspaceDir, { recursive: true });
const remoteAttachmentRoots = resolveIMessageRemoteAttachmentRoots({
cfg,
accountId: ctx.AccountId,
const usedNames = new Set<string>();
const staged = new Map<string, string>(); // absolute source -> relative sandbox path
for (const raw of rawPaths) {
const source = resolveAbsolutePath(raw);
if (!source || staged.has(source)) {
continue;
}
const allowed = await isAllowedSourcePath({
source,
mediaRemoteHost: ctx.MediaRemoteHost,
remoteAttachmentRoots,
});
if (!allowed) {
continue;
}
const fileName = allocateStagedFileName(source, usedNames);
if (!fileName) {
continue;
}
const relativeDest = sandbox ? path.join("media", "inbound", fileName) : fileName;
const dest = path.join(effectiveWorkspaceDir, relativeDest);
const usedNames = new Set<string>();
const staged = new Map<string, string>(); // absolute source -> relative sandbox path
for (const raw of rawPaths) {
const source = resolveAbsolutePath(raw);
if (!source) {
continue;
}
if (staged.has(source)) {
continue;
}
if (
ctx.MediaRemoteHost &&
!isInboundPathAllowed({
filePath: source,
roots: remoteAttachmentRoots,
})
) {
logVerbose(`Blocking remote media staging from disallowed attachment path: ${source}`);
continue;
}
// Local paths must be restricted to the media directory.
if (!ctx.MediaRemoteHost) {
const mediaDir = getMediaDir();
if (
!isInboundPathAllowed({
filePath: source,
roots: [mediaDir],
})
) {
logVerbose(`Blocking attempt to stage media from outside media directory: ${source}`);
continue;
}
try {
await assertSandboxPath({
filePath: source,
cwd: mediaDir,
root: mediaDir,
});
} catch {
logVerbose(`Blocking attempt to stage media from outside media directory: ${source}`);
continue;
}
}
const baseName = path.basename(source);
if (!baseName) {
continue;
}
const parsed = path.parse(baseName);
let fileName = baseName;
let suffix = 1;
while (usedNames.has(fileName)) {
fileName = `${parsed.name}-${suffix}${parsed.ext}`;
suffix += 1;
}
usedNames.add(fileName);
const relativeDest = sandbox ? path.join("media", "inbound", fileName) : fileName;
const dest = path.join(effectiveWorkspaceDir, relativeDest);
try {
if (ctx.MediaRemoteHost) {
// 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,
maxBytes: STAGED_MEDIA_MAX_BYTES,
});
} else {
await stageLocalFileIntoRoot({
sourcePath: source,
rootDir: effectiveWorkspaceDir,
relativeDestPath: relativeDest,
maxBytes: STAGED_MEDIA_MAX_BYTES,
});
}
// For sandbox use relative path, for remote cache use absolute path
const stagedPath = sandbox ? path.posix.join("media", "inbound", fileName) : dest;
staged.set(source, stagedPath);
} catch (err) {
if (err instanceof SafeOpenError && err.code === "too-large") {
logVerbose(
`Blocking inbound media staging above ${STAGED_MEDIA_MAX_BYTES} bytes: ${source}`,
);
} else {
logVerbose(`Failed to stage inbound media path ${source}: ${String(err)}`);
}
continue;
}
const rewriteIfStaged = (value: string | undefined): string | undefined => {
const raw = value?.trim();
if (!raw) {
return value;
}
const abs = resolveAbsolutePath(raw);
if (!abs) {
return value;
}
const mapped = staged.get(abs);
return mapped ?? value;
};
const nextMediaPaths = hasPathsArray ? rawPaths.map((p) => rewriteIfStaged(p) ?? p) : undefined;
if (nextMediaPaths) {
ctx.MediaPaths = nextMediaPaths;
sessionCtx.MediaPaths = nextMediaPaths;
ctx.MediaPath = nextMediaPaths[0];
sessionCtx.MediaPath = nextMediaPaths[0];
} else {
const rewritten = rewriteIfStaged(ctx.MediaPath);
if (rewritten && rewritten !== ctx.MediaPath) {
ctx.MediaPath = rewritten;
sessionCtx.MediaPath = rewritten;
}
}
if (Array.isArray(ctx.MediaUrls) && ctx.MediaUrls.length > 0) {
const nextUrls = ctx.MediaUrls.map((u) => rewriteIfStaged(u) ?? u);
ctx.MediaUrls = nextUrls;
sessionCtx.MediaUrls = nextUrls;
}
const rewrittenUrl = rewriteIfStaged(ctx.MediaUrl);
if (rewrittenUrl && rewrittenUrl !== ctx.MediaUrl) {
ctx.MediaUrl = rewrittenUrl;
sessionCtx.MediaUrl = rewrittenUrl;
}
} catch (err) {
logVerbose(`Failed to stage inbound media for sandbox: ${String(err)}`);
// For sandbox use relative path, for remote cache use absolute path
const stagedPath = sandbox ? path.posix.join("media", "inbound", fileName) : dest;
staged.set(source, stagedPath);
}
rewriteStagedMediaPaths({
ctx,
sessionCtx,
rawPaths,
staged,
hasPathsArray,
});
}
async function stageLocalFileIntoRoot(params: {
sourcePath: string;
rootDir: string;
relativeDestPath: string;
maxBytes?: number;
}): Promise<void> {
const safeRead = await readLocalFileSafely({ filePath: params.sourcePath });
await writeFileWithinRoot({
await copyFileWithinRoot({
sourcePath: params.sourcePath,
rootDir: params.rootDir,
relativePath: params.relativeDestPath,
data: safeRead.buffer,
maxBytes: params.maxBytes,
});
}
@@ -219,6 +138,7 @@ async function stageRemoteFileIntoRoot(params: {
remotePath: string;
rootDir: string;
relativeDestPath: string;
maxBytes?: number;
}): Promise<void> {
const tmpRoot = resolvePreferredOpenClawTmpDir();
await fs.mkdir(tmpRoot, { recursive: true });
@@ -230,12 +150,144 @@ async function stageRemoteFileIntoRoot(params: {
sourcePath: tmpPath,
rootDir: params.rootDir,
relativeDestPath: params.relativeDestPath,
maxBytes: params.maxBytes,
});
} finally {
await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
}
}
function resolveRawPaths(ctx: MsgContext): string[] {
const pathsFromArray = Array.isArray(ctx.MediaPaths) ? ctx.MediaPaths : undefined;
return pathsFromArray && pathsFromArray.length > 0
? pathsFromArray
: ctx.MediaPath?.trim()
? [ctx.MediaPath.trim()]
: [];
}
function resolveAbsolutePath(value: string): string | null {
let resolved = value.trim();
if (!resolved) {
return null;
}
if (resolved.startsWith("file://")) {
try {
resolved = fileURLToPath(resolved);
} catch {
return null;
}
}
if (!path.isAbsolute(resolved)) {
return null;
}
return resolved;
}
async function isAllowedSourcePath(params: {
source: string;
mediaRemoteHost?: string;
remoteAttachmentRoots: string[];
}): Promise<boolean> {
if (params.mediaRemoteHost) {
if (
!isInboundPathAllowed({
filePath: params.source,
roots: params.remoteAttachmentRoots,
})
) {
logVerbose(`Blocking remote media staging from disallowed attachment path: ${params.source}`);
return false;
}
return true;
}
const mediaDir = getMediaDir();
if (
!isInboundPathAllowed({
filePath: params.source,
roots: [mediaDir],
})
) {
logVerbose(`Blocking attempt to stage media from outside media directory: ${params.source}`);
return false;
}
try {
await assertSandboxPath({
filePath: params.source,
cwd: mediaDir,
root: mediaDir,
});
return true;
} catch {
logVerbose(`Blocking attempt to stage media from outside media directory: ${params.source}`);
return false;
}
}
function allocateStagedFileName(source: string, usedNames: Set<string>): string | null {
const baseName = path.basename(source);
if (!baseName) {
return null;
}
const parsed = path.parse(baseName);
let fileName = baseName;
let suffix = 1;
while (usedNames.has(fileName)) {
fileName = `${parsed.name}-${suffix}${parsed.ext}`;
suffix += 1;
}
usedNames.add(fileName);
return fileName;
}
function rewriteStagedMediaPaths(params: {
ctx: MsgContext;
sessionCtx: TemplateContext;
rawPaths: string[];
staged: Map<string, string>;
hasPathsArray: boolean;
}): void {
const rewriteIfStaged = (value: string | undefined): string | undefined => {
const raw = value?.trim();
if (!raw) {
return value;
}
const abs = resolveAbsolutePath(raw);
if (!abs) {
return value;
}
const mapped = params.staged.get(abs);
return mapped ?? value;
};
const nextMediaPaths = params.hasPathsArray
? params.rawPaths.map((p) => rewriteIfStaged(p) ?? p)
: undefined;
if (nextMediaPaths) {
params.ctx.MediaPaths = nextMediaPaths;
params.sessionCtx.MediaPaths = nextMediaPaths;
params.ctx.MediaPath = nextMediaPaths[0];
params.sessionCtx.MediaPath = nextMediaPaths[0];
} else {
const rewritten = rewriteIfStaged(params.ctx.MediaPath);
if (rewritten && rewritten !== params.ctx.MediaPath) {
params.ctx.MediaPath = rewritten;
params.sessionCtx.MediaPath = rewritten;
}
}
if (Array.isArray(params.ctx.MediaUrls) && params.ctx.MediaUrls.length > 0) {
const nextUrls = params.ctx.MediaUrls.map((u) => rewriteIfStaged(u) ?? u);
params.ctx.MediaUrls = nextUrls;
params.sessionCtx.MediaUrls = nextUrls;
}
const rewrittenUrl = rewriteIfStaged(params.ctx.MediaUrl);
if (rewrittenUrl && rewrittenUrl !== params.ctx.MediaUrl) {
params.ctx.MediaUrl = rewrittenUrl;
params.sessionCtx.MediaUrl = rewrittenUrl;
}
}
async function scpFile(remoteHost: string, remotePath: string, localPath: string): Promise<void> {
const safeRemoteHost = normalizeScpRemoteHost(remoteHost);
if (!safeRemoteHost) {

View File

@@ -3,6 +3,7 @@ import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
import {
copyFileWithinRoot,
createRootScopedReadFile,
SafeOpenError,
openFileWithinRoot,
@@ -176,6 +177,42 @@ describe("fs-safe", () => {
await expect(fs.readFile(path.join(root, "nested", "out.txt"), "utf8")).resolves.toBe("hello");
});
it("copies a file within root safely", async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const sourceDir = await tempDirs.make("openclaw-fs-safe-source-");
const sourcePath = path.join(sourceDir, "in.txt");
await fs.writeFile(sourcePath, "copy-ok");
await copyFileWithinRoot({
sourcePath,
rootDir: root,
relativePath: "nested/copied.txt",
});
await expect(fs.readFile(path.join(root, "nested", "copied.txt"), "utf8")).resolves.toBe(
"copy-ok",
);
});
it("enforces maxBytes when copying into root", async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const sourceDir = await tempDirs.make("openclaw-fs-safe-source-");
const sourcePath = path.join(sourceDir, "big.bin");
await fs.writeFile(sourcePath, Buffer.alloc(8));
await expect(
copyFileWithinRoot({
sourcePath,
rootDir: root,
relativePath: "nested/big.bin",
maxBytes: 4,
}),
).rejects.toMatchObject({ code: "too-large" });
await expect(fs.stat(path.join(root, "nested", "big.bin"))).rejects.toMatchObject({
code: "ENOENT",
});
});
it("rejects write traversal outside root", async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
await expect(

View File

@@ -4,6 +4,7 @@ import type { FileHandle } from "node:fs/promises";
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { pipeline } from "node:stream/promises";
import { sameFileIdentity } from "./file-identity.js";
import { expandHomePrefix } from "./home-dir.js";
import { assertNoPathAliasEscape } from "./path-alias-guards.js";
@@ -282,13 +283,15 @@ async function readOpenedFileSafely(params: {
};
}
export async function writeFileWithinRoot(params: {
async function openWritableFileWithinRoot(params: {
rootDir: string;
relativePath: string;
data: string | Buffer;
encoding?: BufferEncoding;
mkdir?: boolean;
}): Promise<void> {
}): Promise<{
handle: FileHandle;
createdForWrite: boolean;
openedRealPath: string;
}> {
const { rootReal, rootWithSep, resolved } = await resolvePathWithinRoot(params);
try {
await assertNoPathAliasEscape({
@@ -372,17 +375,92 @@ export async function writeFileWithinRoot(params: {
if (!createdForWrite) {
await handle.truncate(0);
}
if (typeof params.data === "string") {
await handle.writeFile(params.data, params.encoding ?? "utf8");
} else {
await handle.writeFile(params.data);
}
return {
handle,
createdForWrite,
openedRealPath: realPath,
};
} catch (err) {
if (createdForWrite && err instanceof SafeOpenError && openedRealPath) {
await fs.rm(openedRealPath, { force: true }).catch(() => {});
}
throw err;
} finally {
await handle.close().catch(() => {});
throw err;
}
}
export async function writeFileWithinRoot(params: {
rootDir: string;
relativePath: string;
data: string | Buffer;
encoding?: BufferEncoding;
mkdir?: boolean;
}): Promise<void> {
const target = await openWritableFileWithinRoot({
rootDir: params.rootDir,
relativePath: params.relativePath,
mkdir: params.mkdir,
});
try {
if (typeof params.data === "string") {
await target.handle.writeFile(params.data, params.encoding ?? "utf8");
} else {
await target.handle.writeFile(params.data);
}
} finally {
await target.handle.close().catch(() => {});
}
}
export async function copyFileWithinRoot(params: {
sourcePath: string;
rootDir: string;
relativePath: string;
maxBytes?: number;
mkdir?: boolean;
}): Promise<void> {
const source = await openVerifiedLocalFile(params.sourcePath);
if (params.maxBytes !== undefined && source.stat.size > params.maxBytes) {
await source.handle.close().catch(() => {});
throw new SafeOpenError(
"too-large",
`file exceeds limit of ${params.maxBytes} bytes (got ${source.stat.size})`,
);
}
let target: {
handle: FileHandle;
createdForWrite: boolean;
openedRealPath: string;
} | null = null;
let sourceClosedByStream = false;
let targetClosedByStream = false;
try {
target = await openWritableFileWithinRoot({
rootDir: params.rootDir,
relativePath: params.relativePath,
mkdir: params.mkdir,
});
const sourceStream = source.handle.createReadStream();
const targetStream = target.handle.createWriteStream();
sourceStream.once("close", () => {
sourceClosedByStream = true;
});
targetStream.once("close", () => {
targetClosedByStream = true;
});
await pipeline(sourceStream, targetStream);
} catch (err) {
if (target?.createdForWrite) {
await fs.rm(target.openedRealPath, { force: true }).catch(() => {});
}
throw err;
} finally {
if (!sourceClosedByStream) {
await source.handle.close().catch(() => {});
}
if (target && !targetClosedByStream) {
await target.handle.close().catch(() => {});
}
}
}