Security/Browser: constrain trace and download output paths to OpenClaw temp roots (#15652)
* Browser/Security: constrain trace and download output paths to temp roots * Changelog: remove advisory ID from pre-public security note * Browser/Security: constrain trace and download output paths to temp roots * Changelog: remove advisory ID from pre-public security note * test(bluebubbles): align timeout status expectation to 408 * test(discord): remove unused race-condition counter in threading test * test(bluebubbles): align timeout status expectation to 408
This commit is contained in:
@@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/WhatsApp: enforce `0o600` on `creds.json` and `creds.json.bak` on save/backup/restore paths to reduce credential file exposure. (#10529) Thanks @abdelsfane.
|
||||
- WhatsApp: preserve outbound document filenames for web-session document sends instead of always sending `"file"`. (#15594) Thanks @TsekaLuk.
|
||||
- Security/Gateway + ACP: block high-risk tools (`sessions_spawn`, `sessions_send`, `gateway`, `whatsapp_login`) from HTTP `/tools/invoke` by default with `gateway.tools.{allow,deny}` overrides, and harden ACP permission selection to fail closed when tool identity/options are ambiguous while supporting `allow_always`/`reject_always`. (#15390) Thanks @aether-ai-agent.
|
||||
- Security/Browser: constrain `POST /trace/stop`, `POST /wait/download`, and `POST /download` output paths to OpenClaw temp roots and reject traversal/escape paths.
|
||||
- Gateway/Tools Invoke: sanitize `/tools/invoke` execution failures while preserving `400` for tool input errors and returning `500` for unexpected runtime failures, with regression coverage and docs updates. (#13185) Thanks @davidrudduck.
|
||||
- MS Teams: preserve parsed mention entities/text when appending OneDrive fallback file links, and accept broader real-world Teams mention ID formats (`29:...`, `8:orgid:...`) while still rejecting placeholder patterns. (#15436) Thanks @hyojin.
|
||||
- Security/Audit: distinguish external webhooks (`hooks.enabled`) from internal hooks (`hooks.internal.enabled`) in attack-surface summaries to avoid false exposure signals when only internal hooks are enabled. (#13474) Thanks @mcaxtr.
|
||||
|
||||
@@ -409,8 +409,8 @@ Actions:
|
||||
- `openclaw browser scrollintoview e12`
|
||||
- `openclaw browser drag 10 11`
|
||||
- `openclaw browser select 9 OptionA OptionB`
|
||||
- `openclaw browser download e12 /tmp/report.pdf`
|
||||
- `openclaw browser waitfordownload /tmp/report.pdf`
|
||||
- `openclaw browser download e12 report.pdf`
|
||||
- `openclaw browser waitfordownload report.pdf`
|
||||
- `openclaw browser upload /tmp/file.pdf`
|
||||
- `openclaw browser fill --fields '[{"ref":"1","type":"text","value":"Ada"}]'`
|
||||
- `openclaw browser dialog --accept`
|
||||
@@ -444,6 +444,9 @@ Notes:
|
||||
|
||||
- `upload` and `dialog` are **arming** calls; run them before the click/press
|
||||
that triggers the chooser/dialog.
|
||||
- Download and trace output paths are constrained to OpenClaw temp roots:
|
||||
- traces: `/tmp/openclaw` (fallback: `${os.tmpdir()}/openclaw`)
|
||||
- downloads: `/tmp/openclaw/downloads` (fallback: `${os.tmpdir()}/openclaw/downloads`)
|
||||
- `upload` can also set file inputs directly via `--input-ref` or `--element`.
|
||||
- `snapshot`:
|
||||
- `--format ai` (default when Playwright is installed): returns an AI snapshot with numeric refs (`aria-ref="<n>"`).
|
||||
|
||||
@@ -404,7 +404,7 @@ describe("BlueBubbles webhook monitor", () => {
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
|
||||
it("returns 400 when request body times out (Slow-Loris protection)", async () => {
|
||||
it("returns 408 when request body times out (Slow-Loris protection)", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const account = createMockAccount();
|
||||
@@ -439,7 +439,7 @@ describe("BlueBubbles webhook monitor", () => {
|
||||
|
||||
const handled = await handledPromise;
|
||||
expect(handled).toBe(true);
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.statusCode).toBe(408);
|
||||
expect(req.destroy).toHaveBeenCalled();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
resolveProfileContext,
|
||||
SELECTOR_UNSUPPORTED_MESSAGE,
|
||||
} from "./agent.shared.js";
|
||||
import { DEFAULT_DOWNLOAD_DIR, resolvePathWithinRoot } from "./path-output.js";
|
||||
import { jsonError, toBoolean, toNumber, toStringArray, toStringOrEmpty } from "./utils.js";
|
||||
|
||||
export function registerBrowserAgentActRoutes(
|
||||
@@ -430,7 +431,7 @@ export function registerBrowserAgentActRoutes(
|
||||
}
|
||||
const body = readBody(req);
|
||||
const targetId = toStringOrEmpty(body.targetId) || undefined;
|
||||
const out = toStringOrEmpty(body.path) || undefined;
|
||||
const out = toStringOrEmpty(body.path) || "";
|
||||
const timeoutMs = toNumber(body.timeoutMs);
|
||||
try {
|
||||
const tab = await profileCtx.ensureTabAvailable(targetId);
|
||||
@@ -438,10 +439,23 @@ export function registerBrowserAgentActRoutes(
|
||||
if (!pw) {
|
||||
return;
|
||||
}
|
||||
let downloadPath: string | undefined;
|
||||
if (out.trim()) {
|
||||
const downloadPathResult = resolvePathWithinRoot({
|
||||
rootDir: DEFAULT_DOWNLOAD_DIR,
|
||||
requestedPath: out,
|
||||
scopeLabel: "downloads directory",
|
||||
});
|
||||
if (!downloadPathResult.ok) {
|
||||
res.status(400).json({ error: downloadPathResult.error });
|
||||
return;
|
||||
}
|
||||
downloadPath = downloadPathResult.path;
|
||||
}
|
||||
const result = await pw.waitForDownloadViaPlaywright({
|
||||
cdpUrl: profileCtx.profile.cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
path: out,
|
||||
path: downloadPath,
|
||||
timeoutMs: timeoutMs ?? undefined,
|
||||
});
|
||||
res.json({ ok: true, targetId: tab.targetId, download: result });
|
||||
@@ -467,6 +481,15 @@ export function registerBrowserAgentActRoutes(
|
||||
return jsonError(res, 400, "path is required");
|
||||
}
|
||||
try {
|
||||
const downloadPathResult = resolvePathWithinRoot({
|
||||
rootDir: DEFAULT_DOWNLOAD_DIR,
|
||||
requestedPath: out,
|
||||
scopeLabel: "downloads directory",
|
||||
});
|
||||
if (!downloadPathResult.ok) {
|
||||
res.status(400).json({ error: downloadPathResult.error });
|
||||
return;
|
||||
}
|
||||
const tab = await profileCtx.ensureTabAvailable(targetId);
|
||||
const pw = await requirePwAi(res, "download");
|
||||
if (!pw) {
|
||||
@@ -476,7 +499,7 @@ export function registerBrowserAgentActRoutes(
|
||||
cdpUrl: profileCtx.profile.cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
ref,
|
||||
path: out,
|
||||
path: downloadPathResult.path,
|
||||
timeoutMs: timeoutMs ?? undefined,
|
||||
});
|
||||
res.json({ ok: true, targetId: tab.targetId, download: result });
|
||||
|
||||
@@ -3,12 +3,10 @@ import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import type { BrowserRouteContext } from "../server-context.js";
|
||||
import type { BrowserRouteRegistrar } from "./types.js";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
|
||||
import { handleRouteError, readBody, requirePwAi, resolveProfileContext } from "./agent.shared.js";
|
||||
import { DEFAULT_TRACE_DIR, resolvePathWithinRoot } from "./path-output.js";
|
||||
import { toBoolean, toStringOrEmpty } from "./utils.js";
|
||||
|
||||
const DEFAULT_TRACE_DIR = resolvePreferredOpenClawTmpDir();
|
||||
|
||||
export function registerBrowserAgentDebugRoutes(
|
||||
app: BrowserRouteRegistrar,
|
||||
ctx: BrowserRouteContext,
|
||||
@@ -136,7 +134,17 @@ export function registerBrowserAgentDebugRoutes(
|
||||
const id = crypto.randomUUID();
|
||||
const dir = DEFAULT_TRACE_DIR;
|
||||
await fs.mkdir(dir, { recursive: true });
|
||||
const tracePath = out.trim() || path.join(dir, `browser-trace-${id}.zip`);
|
||||
const tracePathResult = resolvePathWithinRoot({
|
||||
rootDir: dir,
|
||||
requestedPath: out,
|
||||
scopeLabel: "trace directory",
|
||||
defaultFileName: `browser-trace-${id}.zip`,
|
||||
});
|
||||
if (!tracePathResult.ok) {
|
||||
res.status(400).json({ error: tracePathResult.error });
|
||||
return;
|
||||
}
|
||||
const tracePath = tracePathResult.path;
|
||||
await pw.traceStopViaPlaywright({
|
||||
cdpUrl: profileCtx.profile.cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
|
||||
28
src/browser/routes/path-output.ts
Normal file
28
src/browser/routes/path-output.ts
Normal file
@@ -0,0 +1,28 @@
|
||||
import path from "node:path";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
|
||||
|
||||
export const DEFAULT_BROWSER_TMP_DIR = resolvePreferredOpenClawTmpDir();
|
||||
export const DEFAULT_TRACE_DIR = DEFAULT_BROWSER_TMP_DIR;
|
||||
export const DEFAULT_DOWNLOAD_DIR = path.join(DEFAULT_BROWSER_TMP_DIR, "downloads");
|
||||
|
||||
export function resolvePathWithinRoot(params: {
|
||||
rootDir: string;
|
||||
requestedPath: string;
|
||||
scopeLabel: string;
|
||||
defaultFileName?: string;
|
||||
}): { ok: true; path: string } | { ok: false; error: string } {
|
||||
const root = path.resolve(params.rootDir);
|
||||
const raw = params.requestedPath.trim();
|
||||
if (!raw) {
|
||||
if (!params.defaultFileName) {
|
||||
return { ok: false, error: "path is required" };
|
||||
}
|
||||
return { ok: true, path: path.join(root, params.defaultFileName) };
|
||||
}
|
||||
const resolved = path.resolve(root, raw);
|
||||
const rel = path.relative(root, resolved);
|
||||
if (!rel || rel.startsWith("..") || path.isAbsolute(rel)) {
|
||||
return { ok: false, error: `Invalid path: must stay within ${params.scopeLabel}` };
|
||||
}
|
||||
return { ok: true, path: resolved };
|
||||
}
|
||||
@@ -49,6 +49,7 @@ const pwMocks = vi.hoisted(() => ({
|
||||
selectOptionViaPlaywright: vi.fn(async () => {}),
|
||||
setInputFilesViaPlaywright: vi.fn(async () => {}),
|
||||
snapshotAiViaPlaywright: vi.fn(async () => ({ snapshot: "ok" })),
|
||||
traceStopViaPlaywright: vi.fn(async () => {}),
|
||||
takeScreenshotViaPlaywright: vi.fn(async () => ({
|
||||
buffer: Buffer.from("png"),
|
||||
})),
|
||||
@@ -434,14 +435,14 @@ describe("browser control server", () => {
|
||||
expect(dialog).toMatchObject({ ok: true });
|
||||
|
||||
const waitDownload = await postJson(`${base}/wait/download`, {
|
||||
path: "/tmp/report.pdf",
|
||||
path: "report.pdf",
|
||||
timeoutMs: 1111,
|
||||
});
|
||||
expect(waitDownload).toMatchObject({ ok: true });
|
||||
|
||||
const download = await postJson(`${base}/download`, {
|
||||
ref: "e12",
|
||||
path: "/tmp/report.pdf",
|
||||
path: "report.pdf",
|
||||
});
|
||||
expect(download).toMatchObject({ ok: true });
|
||||
|
||||
@@ -480,4 +481,83 @@ describe("browser control server", () => {
|
||||
expect(stopped.ok).toBe(true);
|
||||
expect(stopped.stopped).toBe(true);
|
||||
});
|
||||
|
||||
it("trace stop rejects traversal path outside trace dir", async () => {
|
||||
const base = await startServerAndBase();
|
||||
const res = await postJson<{ error?: string }>(`${base}/trace/stop`, {
|
||||
path: "../../pwned.zip",
|
||||
});
|
||||
expect(res.error).toContain("Invalid path");
|
||||
expect(pwMocks.traceStopViaPlaywright).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("trace stop accepts in-root relative output path", async () => {
|
||||
const base = await startServerAndBase();
|
||||
const res = await postJson<{ ok?: boolean; path?: string }>(`${base}/trace/stop`, {
|
||||
path: "safe-trace.zip",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
expect(res.path).toContain("safe-trace.zip");
|
||||
expect(pwMocks.traceStopViaPlaywright).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
cdpUrl: cdpBaseUrl,
|
||||
targetId: "abcd1234",
|
||||
path: expect.stringContaining("safe-trace.zip"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("wait/download rejects traversal path outside downloads dir", async () => {
|
||||
const base = await startServerAndBase();
|
||||
const waitRes = await postJson<{ error?: string }>(`${base}/wait/download`, {
|
||||
path: "../../pwned.pdf",
|
||||
});
|
||||
expect(waitRes.error).toContain("Invalid path");
|
||||
expect(pwMocks.waitForDownloadViaPlaywright).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("download rejects traversal path outside downloads dir", async () => {
|
||||
const base = await startServerAndBase();
|
||||
const downloadRes = await postJson<{ error?: string }>(`${base}/download`, {
|
||||
ref: "e12",
|
||||
path: "../../pwned.pdf",
|
||||
});
|
||||
expect(downloadRes.error).toContain("Invalid path");
|
||||
expect(pwMocks.downloadViaPlaywright).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("wait/download accepts in-root relative output path", async () => {
|
||||
const base = await startServerAndBase();
|
||||
const res = await postJson<{ ok?: boolean; download?: { path?: string } }>(
|
||||
`${base}/wait/download`,
|
||||
{
|
||||
path: "safe-wait.pdf",
|
||||
},
|
||||
);
|
||||
expect(res.ok).toBe(true);
|
||||
expect(pwMocks.waitForDownloadViaPlaywright).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
cdpUrl: cdpBaseUrl,
|
||||
targetId: "abcd1234",
|
||||
path: expect.stringContaining("safe-wait.pdf"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("download accepts in-root relative output path", async () => {
|
||||
const base = await startServerAndBase();
|
||||
const res = await postJson<{ ok?: boolean; download?: { path?: string } }>(`${base}/download`, {
|
||||
ref: "e12",
|
||||
path: "safe-download.pdf",
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
expect(pwMocks.downloadViaPlaywright).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
cdpUrl: cdpBaseUrl,
|
||||
targetId: "abcd1234",
|
||||
ref: "e12",
|
||||
path: expect.stringContaining("safe-download.pdf"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -59,7 +59,7 @@ export function registerBrowserFilesAndDownloadsCommands(
|
||||
.description("Wait for the next download (and save it)")
|
||||
.argument(
|
||||
"[path]",
|
||||
"Save path (default: /tmp/openclaw/downloads/...; fallback: os.tmpdir()/openclaw/downloads/...)",
|
||||
"Save path within openclaw temp downloads dir (default: /tmp/openclaw/downloads/...; fallback: os.tmpdir()/openclaw/downloads/...)",
|
||||
)
|
||||
.option("--target-id <id>", "CDP target id (or unique prefix)")
|
||||
.option(
|
||||
@@ -100,7 +100,10 @@ export function registerBrowserFilesAndDownloadsCommands(
|
||||
.command("download")
|
||||
.description("Click a ref and save the resulting download")
|
||||
.argument("<ref>", "Ref id from snapshot to click")
|
||||
.argument("<path>", "Save path")
|
||||
.argument(
|
||||
"<path>",
|
||||
"Save path within openclaw temp downloads dir (e.g. report.pdf or /tmp/openclaw/downloads/report.pdf)",
|
||||
)
|
||||
.option("--target-id <id>", "CDP target id (or unique prefix)")
|
||||
.option(
|
||||
"--timeout-ms <ms>",
|
||||
|
||||
@@ -179,7 +179,10 @@ export function registerBrowserDebugCommands(
|
||||
trace
|
||||
.command("stop")
|
||||
.description("Stop trace recording and write a .zip")
|
||||
.option("--out <path>", "Output path for the trace zip")
|
||||
.option(
|
||||
"--out <path>",
|
||||
"Output path within openclaw temp dir (e.g. trace.zip or /tmp/openclaw/trace.zip)",
|
||||
)
|
||||
.option("--target-id <id>", "CDP target id (or unique prefix)")
|
||||
.action(async (opts, cmd) => {
|
||||
const parent = parentOpts(cmd);
|
||||
|
||||
@@ -115,6 +115,7 @@ describe("resolveDiscordReplyDeliveryPlan", () => {
|
||||
|
||||
describe("maybeCreateDiscordAutoThread", () => {
|
||||
it("returns existing thread ID when creation fails due to race condition", async () => {
|
||||
// First call succeeds (simulating another agent creating the thread)
|
||||
const client = {
|
||||
rest: {
|
||||
post: async () => {
|
||||
|
||||
Reference in New Issue
Block a user