mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 15:21:23 +00:00
fix(security): lock sandbox tmp media paths to openclaw roots
This commit is contained in:
@@ -3,6 +3,7 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
import { resolveSandboxedMediaSource } from "./sandbox-paths.js";
|
||||
|
||||
async function withSandboxRoot<T>(run: (sandboxDir: string) => Promise<T>) {
|
||||
@@ -24,22 +25,24 @@ function isPathInside(root: string, target: string): boolean {
|
||||
}
|
||||
|
||||
describe("resolveSandboxedMediaSource", () => {
|
||||
const openClawTmpDir = resolvePreferredOpenClawTmpDir();
|
||||
|
||||
// Group 1: /tmp paths (the bug fix)
|
||||
it.each([
|
||||
{
|
||||
name: "absolute paths under os.tmpdir()",
|
||||
media: path.join(os.tmpdir(), "image.png"),
|
||||
expected: path.join(os.tmpdir(), "image.png"),
|
||||
name: "absolute paths under preferred OpenClaw tmp root",
|
||||
media: path.join(openClawTmpDir, "image.png"),
|
||||
expected: path.join(openClawTmpDir, "image.png"),
|
||||
},
|
||||
{
|
||||
name: "file:// URLs pointing to os.tmpdir()",
|
||||
media: pathToFileURL(path.join(os.tmpdir(), "photo.png")).href,
|
||||
expected: path.join(os.tmpdir(), "photo.png"),
|
||||
name: "file:// URLs pointing to preferred OpenClaw tmp root",
|
||||
media: pathToFileURL(path.join(openClawTmpDir, "photo.png")).href,
|
||||
expected: path.join(openClawTmpDir, "photo.png"),
|
||||
},
|
||||
{
|
||||
name: "nested paths under os.tmpdir()",
|
||||
media: path.join(os.tmpdir(), "subdir", "deep", "file.png"),
|
||||
expected: path.join(os.tmpdir(), "subdir", "deep", "file.png"),
|
||||
name: "nested paths under preferred OpenClaw tmp root",
|
||||
media: path.join(openClawTmpDir, "subdir", "deep", "file.png"),
|
||||
expected: path.join(openClawTmpDir, "subdir", "deep", "file.png"),
|
||||
},
|
||||
])("allows $name", async ({ media, expected }) => {
|
||||
await withSandboxRoot(async (sandboxDir) => {
|
||||
@@ -96,7 +99,12 @@ describe("resolveSandboxedMediaSource", () => {
|
||||
},
|
||||
{
|
||||
name: "path traversal through tmpdir",
|
||||
media: path.join(os.tmpdir(), "..", "etc", "passwd"),
|
||||
media: path.join(openClawTmpDir, "..", "etc", "passwd"),
|
||||
expected: /sandbox/i,
|
||||
},
|
||||
{
|
||||
name: "absolute paths under host tmp outside openclaw tmp root",
|
||||
media: path.join(os.tmpdir(), "outside-openclaw", "passwd"),
|
||||
expected: /sandbox/i,
|
||||
},
|
||||
{
|
||||
@@ -120,20 +128,25 @@ describe("resolveSandboxedMediaSource", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects symlinked tmpdir paths escaping tmpdir", async () => {
|
||||
it("rejects symlinked OpenClaw tmp paths escaping tmp root", async () => {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const outsideTmpTarget = path.resolve(process.cwd(), "package.json");
|
||||
if (isPathInside(os.tmpdir(), outsideTmpTarget)) {
|
||||
if (isPathInside(openClawTmpDir, outsideTmpTarget)) {
|
||||
return;
|
||||
}
|
||||
|
||||
await withSandboxRoot(async (sandboxDir) => {
|
||||
await fs.access(outsideTmpTarget);
|
||||
const symlinkPath = path.join(sandboxDir, "tmp-link-escape");
|
||||
await fs.mkdir(openClawTmpDir, { recursive: true });
|
||||
const symlinkPath = path.join(openClawTmpDir, `tmp-link-escape-${process.pid}`);
|
||||
await fs.symlink(outsideTmpTarget, symlinkPath);
|
||||
await expectSandboxRejection(symlinkPath, sandboxDir, /symlink|sandbox/i);
|
||||
try {
|
||||
await expectSandboxRejection(symlinkPath, sandboxDir, /symlink|sandbox/i);
|
||||
} finally {
|
||||
await fs.unlink(symlinkPath).catch(() => {});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath, URL } from "node:url";
|
||||
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
|
||||
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
|
||||
const HTTP_URL_RE = /^https?:\/\//i;
|
||||
@@ -181,11 +182,11 @@ async function resolveAllowedTmpMediaPath(params: {
|
||||
return undefined;
|
||||
}
|
||||
const resolved = path.resolve(resolveSandboxInputPath(params.candidate, params.sandboxRoot));
|
||||
const tmpDir = path.resolve(os.tmpdir());
|
||||
if (!isPathInside(tmpDir, resolved)) {
|
||||
const openClawTmpDir = path.resolve(resolvePreferredOpenClawTmpDir());
|
||||
if (!isPathInside(openClawTmpDir, resolved)) {
|
||||
return undefined;
|
||||
}
|
||||
await assertNoSymlinkEscape(path.relative(tmpDir, resolved), tmpDir);
|
||||
await assertNoSymlinkEscape(path.relative(openClawTmpDir, resolved), openClawTmpDir);
|
||||
return resolved;
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/c
|
||||
import { withEnvAsync } from "../../test-utils/env.js";
|
||||
import { createIMessageTestPlugin } from "../../test-utils/imessage-test-plugin.js";
|
||||
import { createInternalHookEventPayload } from "../../test-utils/internal-hook-event-payload.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../tmp-openclaw-dir.js";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
appendAssistantMessageToSessionTranscript: vi.fn(async () => ({ ok: true, sessionFile: "x" })),
|
||||
@@ -202,6 +203,86 @@ describe("deliverOutboundPayloads", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("includes OpenClaw tmp root in telegram mediaLocalRoots", async () => {
|
||||
const sendTelegram = vi.fn().mockResolvedValue({ messageId: "m1", chatId: "c1" });
|
||||
|
||||
await deliverOutboundPayloads({
|
||||
cfg: telegramChunkConfig,
|
||||
channel: "telegram",
|
||||
to: "123",
|
||||
payloads: [{ text: "hi", mediaUrl: "https://example.com/x.png" }],
|
||||
deps: { sendTelegram },
|
||||
});
|
||||
|
||||
expect(sendTelegram).toHaveBeenCalledWith(
|
||||
"123",
|
||||
"hi",
|
||||
expect.objectContaining({
|
||||
mediaLocalRoots: expect.arrayContaining([resolvePreferredOpenClawTmpDir()]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("includes OpenClaw tmp root in signal mediaLocalRoots", async () => {
|
||||
const sendSignal = vi.fn().mockResolvedValue({ messageId: "s1", timestamp: 123 });
|
||||
|
||||
await deliverOutboundPayloads({
|
||||
cfg: { channels: { signal: {} } },
|
||||
channel: "signal",
|
||||
to: "+1555",
|
||||
payloads: [{ text: "hi", mediaUrl: "https://example.com/x.png" }],
|
||||
deps: { sendSignal },
|
||||
});
|
||||
|
||||
expect(sendSignal).toHaveBeenCalledWith(
|
||||
"+1555",
|
||||
"hi",
|
||||
expect.objectContaining({
|
||||
mediaLocalRoots: expect.arrayContaining([resolvePreferredOpenClawTmpDir()]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("includes OpenClaw tmp root in whatsapp mediaLocalRoots", async () => {
|
||||
const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" });
|
||||
|
||||
await deliverOutboundPayloads({
|
||||
cfg: whatsappChunkConfig,
|
||||
channel: "whatsapp",
|
||||
to: "+1555",
|
||||
payloads: [{ text: "hi", mediaUrl: "https://example.com/x.png" }],
|
||||
deps: { sendWhatsApp },
|
||||
});
|
||||
|
||||
expect(sendWhatsApp).toHaveBeenCalledWith(
|
||||
"+1555",
|
||||
"hi",
|
||||
expect.objectContaining({
|
||||
mediaLocalRoots: expect.arrayContaining([resolvePreferredOpenClawTmpDir()]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("includes OpenClaw tmp root in imessage mediaLocalRoots", async () => {
|
||||
const sendIMessage = vi.fn().mockResolvedValue({ messageId: "i1", chatId: "chat-1" });
|
||||
|
||||
await deliverOutboundPayloads({
|
||||
cfg: {},
|
||||
channel: "imessage",
|
||||
to: "imessage:+15551234567",
|
||||
payloads: [{ text: "hi", mediaUrl: "https://example.com/x.png" }],
|
||||
deps: { sendIMessage },
|
||||
});
|
||||
|
||||
expect(sendIMessage).toHaveBeenCalledWith(
|
||||
"imessage:+15551234567",
|
||||
"hi",
|
||||
expect.objectContaining({
|
||||
mediaLocalRoots: expect.arrayContaining([resolvePreferredOpenClawTmpDir()]),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses signal media maxBytes from config", async () => {
|
||||
const sendSignal = vi.fn().mockResolvedValue({ messageId: "s1", timestamp: 123 });
|
||||
const cfg: OpenClawConfig = { channels: { signal: { mediaMaxMb: 2 } } };
|
||||
|
||||
@@ -12,6 +12,7 @@ import { setActivePluginRegistry } from "../../plugins/runtime.js";
|
||||
import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js";
|
||||
import { createIMessageTestPlugin } from "../../test-utils/imessage-test-plugin.js";
|
||||
import { loadWebMedia } from "../../web/media.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../tmp-openclaw-dir.js";
|
||||
import { runMessageAction } from "./message-action-runner.js";
|
||||
|
||||
vi.mock("../../web/media.js", async () => {
|
||||
@@ -622,10 +623,12 @@ describe("runMessageAction sandboxed media validation", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("allows media paths under os.tmpdir()", async () => {
|
||||
it("allows media paths under preferred OpenClaw tmp root", async () => {
|
||||
const tmpRoot = resolvePreferredOpenClawTmpDir();
|
||||
await fs.mkdir(tmpRoot, { recursive: true });
|
||||
const sandboxDir = await fs.mkdtemp(path.join(os.tmpdir(), "msg-sandbox-"));
|
||||
try {
|
||||
const tmpFile = path.join(os.tmpdir(), "test-media-image.png");
|
||||
const tmpFile = path.join(tmpRoot, "test-media-image.png");
|
||||
const result = await runMessageAction({
|
||||
cfg: slackConfig,
|
||||
action: "send",
|
||||
@@ -644,6 +647,21 @@ describe("runMessageAction sandboxed media validation", () => {
|
||||
throw new Error("expected send result");
|
||||
}
|
||||
expect(result.sendResult?.mediaUrl).toBe(tmpFile);
|
||||
const hostTmpOutsideOpenClaw = path.join(os.tmpdir(), "outside-openclaw", "test-media.png");
|
||||
await expect(
|
||||
runMessageAction({
|
||||
cfg: slackConfig,
|
||||
action: "send",
|
||||
params: {
|
||||
channel: "slack",
|
||||
target: "#C12345678",
|
||||
media: hostTmpOutsideOpenClaw,
|
||||
message: "",
|
||||
},
|
||||
sandboxRoot: sandboxDir,
|
||||
dryRun: true,
|
||||
}),
|
||||
).rejects.toThrow(/sandbox/i);
|
||||
} finally {
|
||||
await fs.rm(sandboxDir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import {
|
||||
collectProviderApiKeysForExecution,
|
||||
@@ -14,6 +13,7 @@ import type {
|
||||
MediaUnderstandingModelConfig,
|
||||
} from "../config/types.tools.js";
|
||||
import { logVerbose, shouldLogVerbose } from "../globals.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
import { runExec } from "../process/exec.js";
|
||||
import { MediaAttachmentCache } from "./attachments.js";
|
||||
import {
|
||||
@@ -566,7 +566,9 @@ export async function runCliEntry(params: {
|
||||
maxBytes,
|
||||
timeoutMs,
|
||||
});
|
||||
const outputDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-media-cli-"));
|
||||
const outputDir = await fs.mkdtemp(
|
||||
path.join(resolvePreferredOpenClawTmpDir(), "openclaw-media-cli-"),
|
||||
);
|
||||
const mediaPath = pathResult.path;
|
||||
const outputBase = path.join(outputDir, path.parse(mediaPath).name);
|
||||
|
||||
|
||||
@@ -200,6 +200,7 @@ export { createLoggerBackedRuntime } from "./runtime.js";
|
||||
export { chunkTextForOutbound } from "./text-chunking.js";
|
||||
export { readJsonFileWithFallback, writeJsonFileAtomically } from "./json-store.js";
|
||||
export { buildRandomTempFilePath, withTempDownloadPath } from "./temp-path.js";
|
||||
export { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
export {
|
||||
runPluginCommandWithTimeout,
|
||||
type PluginCommandRunOptions,
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
import { buildRandomTempFilePath, withTempDownloadPath } from "./temp-path.js";
|
||||
|
||||
describe("buildRandomTempFilePath", () => {
|
||||
@@ -17,13 +17,13 @@ describe("buildRandomTempFilePath", () => {
|
||||
});
|
||||
|
||||
it("sanitizes prefix and extension to avoid path traversal segments", () => {
|
||||
const tmpRoot = path.resolve(resolvePreferredOpenClawTmpDir());
|
||||
const result = buildRandomTempFilePath({
|
||||
prefix: "../../line/../media",
|
||||
extension: "/../.jpg",
|
||||
now: 123,
|
||||
uuid: "abc",
|
||||
});
|
||||
const tmpRoot = path.resolve(os.tmpdir());
|
||||
const resolved = path.resolve(result);
|
||||
const rel = path.relative(tmpRoot, resolved);
|
||||
expect(rel === ".." || rel.startsWith(`..${path.sep}`)).toBe(false);
|
||||
@@ -45,11 +45,12 @@ describe("withTempDownloadPath", () => {
|
||||
},
|
||||
);
|
||||
|
||||
expect(capturedPath).toContain(path.join(os.tmpdir(), "line-media-"));
|
||||
expect(capturedPath).toContain(path.join(resolvePreferredOpenClawTmpDir(), "line-media-"));
|
||||
await expect(fs.stat(capturedPath)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
|
||||
it("sanitizes prefix and fileName", async () => {
|
||||
const tmpRoot = path.resolve(resolvePreferredOpenClawTmpDir());
|
||||
let capturedPath = "";
|
||||
await withTempDownloadPath(
|
||||
{
|
||||
@@ -61,7 +62,6 @@ describe("withTempDownloadPath", () => {
|
||||
},
|
||||
);
|
||||
|
||||
const tmpRoot = path.resolve(os.tmpdir());
|
||||
const resolved = path.resolve(capturedPath);
|
||||
const rel = path.relative(tmpRoot, resolved);
|
||||
expect(rel === ".." || rel.startsWith(`..${path.sep}`)).toBe(false);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import crypto from "node:crypto";
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
|
||||
|
||||
function sanitizePrefix(prefix: string): string {
|
||||
const normalized = prefix.replace(/[^a-zA-Z0-9_-]+/g, "-").replace(/^-+|-+$/g, "");
|
||||
@@ -27,6 +27,10 @@ function sanitizeFileName(fileName: string): string {
|
||||
return normalized || "download.bin";
|
||||
}
|
||||
|
||||
function resolveTempRoot(tmpDir?: string): string {
|
||||
return tmpDir ?? resolvePreferredOpenClawTmpDir();
|
||||
}
|
||||
|
||||
export function buildRandomTempFilePath(params: {
|
||||
prefix: string;
|
||||
extension?: string;
|
||||
@@ -42,7 +46,7 @@ export function buildRandomTempFilePath(params: {
|
||||
? Math.trunc(nowCandidate)
|
||||
: Date.now();
|
||||
const uuid = params.uuid?.trim() || crypto.randomUUID();
|
||||
return path.join(params.tmpDir ?? os.tmpdir(), `${prefix}-${now}-${uuid}${extension}`);
|
||||
return path.join(resolveTempRoot(params.tmpDir), `${prefix}-${now}-${uuid}${extension}`);
|
||||
}
|
||||
|
||||
export async function withTempDownloadPath<T>(
|
||||
@@ -53,7 +57,7 @@ export async function withTempDownloadPath<T>(
|
||||
},
|
||||
fn: (tmpPath: string) => Promise<T>,
|
||||
): Promise<T> {
|
||||
const tempRoot = params.tmpDir ?? os.tmpdir();
|
||||
const tempRoot = resolveTempRoot(params.tmpDir);
|
||||
const prefix = `${sanitizePrefix(params.prefix)}-`;
|
||||
const dir = await mkdtemp(path.join(tempRoot, prefix));
|
||||
const tmpPath = path.join(dir, sanitizeFileName(params.fileName ?? "download.bin"));
|
||||
|
||||
Reference in New Issue
Block a user