From 8be581cbf8e98f45ede8e6c83e39dc0dde8742bf Mon Sep 17 00:00:00 2001 From: Zee Zheng Date: Sun, 31 May 2026 06:49:07 +0800 Subject: [PATCH] fix(browser): allow inbound media uploads Allow the browser upload tool to resolve OpenClaw-managed inbound media refs such as `media://inbound/` and sandbox-relative `media/inbound/` while preserving the existing upload-root path contract. Keep upload-root files ahead of sandbox-relative inbound fallback, reject nested absolute inbound media files, and validate raw `media://` paths before URL normalization so traversal-shaped refs cannot resolve to direct media ids. Verification: - `OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs extensions/browser/src/browser/paths.test.ts --reporter=verbose` - `OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs extensions/browser/src/browser/paths.test.ts --reporter=dot` - `OPENCLAW_HEAVY_CHECK_LOCK_SCOPE=worktree node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.extensions.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions-test.tsbuildinfo` - `pnpm lint --threads=8` - `.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main` - `git diff --check` - GitHub PR checks on be08e6c8a8a45262d050050ab160ca4e6b709b20: dependency-guard, check-lint, check-test-types, check-additional-extension-bundled, checks-fast-contracts-plugins-a, checks-fast-contracts-plugins-b all passed. Fixes #83544. Co-authored-by: Zee Zheng Co-authored-by: Claude Opus 4.6 --- docs/cli/browser.md | 5 + docs/tools/browser-control.md | 8 +- extensions/browser/src/browser-runtime.ts | 6 +- .../browser/src/browser-tool.runtime.ts | 2 +- extensions/browser/src/browser-tool.test.ts | 58 +++- extensions/browser/src/browser-tool.ts | 15 +- extensions/browser/src/browser/paths.test.ts | 320 +++++++++++++++++- extensions/browser/src/browser/paths.ts | 241 ++++++++++++- .../src/browser/pw-tools-core.downloads.ts | 6 +- ...-core.interactions.set-input-files.test.ts | 19 +- .../src/browser/pw-tools-core.interactions.ts | 14 +- .../pw-tools-core.upload-paths.test.ts | 78 +++++ .../src/browser/routes/agent.act.hooks.ts | 12 +- .../register.files-downloads.ts | 11 +- .../browser/src/cli/browser-cli-examples.ts | 1 + extensions/browser/src/core-api.ts | 1 + 16 files changed, 736 insertions(+), 61 deletions(-) create mode 100644 extensions/browser/src/browser/pw-tools-core.upload-paths.test.ts diff --git a/docs/cli/browser.md b/docs/cli/browser.md index ffdc93cfa4f..d1de7e44421 100644 --- a/docs/cli/browser.md +++ b/docs/cli/browser.md @@ -205,6 +205,7 @@ File + dialog helpers: ```bash openclaw browser upload /tmp/openclaw/uploads/file.pdf --ref +openclaw browser upload media://inbound/file.pdf --ref openclaw browser waitfordownload openclaw browser download report.pdf openclaw browser dialog --accept @@ -215,6 +216,10 @@ Managed Chrome profiles save ordinary click-triggered downloads into the OpenCla downloads directory (`/tmp/openclaw/downloads` by default, or the configured temp root). Use `waitfordownload` or `download` when the agent needs to wait for a specific file and return its path; those explicit waiters own the next download. +Uploads accept files from the OpenClaw temp uploads root and OpenClaw-managed +inbound media, including `media://inbound/` and sandbox-relative +`media/inbound/` references. Nested media refs, traversal, and arbitrary +local paths remain rejected. When an action opens a modal dialog, the action response returns `blockedByDialog` with `browserState.dialogs.pending`; pass `--dialog-id` to answer it directly. Dialogs handled outside OpenClaw appear under diff --git a/docs/tools/browser-control.md b/docs/tools/browser-control.md index 0bb611278a3..b5e583163fb 100644 --- a/docs/tools/browser-control.md +++ b/docs/tools/browser-control.md @@ -191,6 +191,7 @@ openclaw browser select 9 OptionA OptionB openclaw browser download e12 report.pdf openclaw browser waitfordownload report.pdf openclaw browser upload /tmp/openclaw/uploads/file.pdf +openclaw browser upload media://inbound/file.pdf openclaw browser fill --fields '[{"ref":"1","type":"text","value":"Ada"}]' openclaw browser dialog --accept openclaw browser dialog --dismiss --dialog-id d1 @@ -232,7 +233,12 @@ Notes: - `upload` and `dialog` are **arming** calls; run them before the click/press that triggers the chooser/dialog. If an action opens a modal, the action response includes `blockedByDialog` and `browserState.dialogs.pending`; pass that `dialogId` to respond directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`. - `click`/`type`/etc require a `ref` from `snapshot` (numeric `12`, role ref `e12`, or actionable ARIA ref `ax12`). CSS selectors are intentionally not supported for actions. Use `click-coords` when the visible viewport position is the only reliable target. -- Download, trace, and upload paths are constrained to OpenClaw temp roots: `/tmp/openclaw{,/downloads,/uploads}` (fallback: `${os.tmpdir()}/openclaw/...`). +- Download and trace paths are constrained to OpenClaw temp roots: `/tmp/openclaw{,/downloads}` (fallback: `${os.tmpdir()}/openclaw/...`). +- `upload` accepts files from the OpenClaw temp uploads root and + OpenClaw-managed inbound media. Managed inbound media can be referenced as + `media://inbound/`, sandbox-relative `media/inbound/`, or a resolved + path inside the managed inbound media directory. Nested media refs, + traversal, symlinks, hardlinks, and arbitrary local paths are still rejected. - `upload` can also set file inputs directly via `--input-ref` or `--element`. Stable tab ids and labels survive Chromium raw-target replacement when OpenClaw diff --git a/extensions/browser/src/browser-runtime.ts b/extensions/browser/src/browser-runtime.ts index 5264fca998b..83642449105 100644 --- a/extensions/browser/src/browser-runtime.ts +++ b/extensions/browser/src/browser-runtime.ts @@ -53,7 +53,11 @@ export { resolveGoogleChromeExecutableForPlatform, } from "./browser/chrome.executables.js"; export { redactCdpUrl } from "./browser/cdp.helpers.js"; -export { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "./browser/paths.js"; +export { + DEFAULT_UPLOAD_DIR, + resolveExistingPathsWithinRoot, + resolveExistingUploadPaths, +} from "./browser/paths.js"; export { getBrowserProfileCapabilities } from "./browser/profile-capabilities.js"; export { applyBrowserProxyPaths, persistBrowserProxyFiles } from "./browser/proxy-files.js"; export { diff --git a/extensions/browser/src/browser-tool.runtime.ts b/extensions/browser/src/browser-tool.runtime.ts index aa144291e70..b8192cccc89 100644 --- a/extensions/browser/src/browser-tool.runtime.ts +++ b/extensions/browser/src/browser-tool.runtime.ts @@ -48,7 +48,7 @@ export { } from "./browser/client.js"; export { resolveBrowserConfig, resolveProfile } from "./browser/config.js"; export { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "./browser/constants.js"; -export { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "./browser/paths.js"; +export { resolveExistingUploadPaths } from "./browser/paths.js"; export { getBrowserProfileCapabilities } from "./browser/profile-capabilities.js"; export { applyBrowserProxyPaths, persistBrowserProxyFiles } from "./browser/proxy-files.js"; export { diff --git a/extensions/browser/src/browser-tool.test.ts b/extensions/browser/src/browser-tool.test.ts index 277c187fbf0..75958a7cd10 100644 --- a/extensions/browser/src/browser-tool.test.ts +++ b/extensions/browser/src/browser-tool.test.ts @@ -137,6 +137,17 @@ vi.mock("openclaw/plugin-sdk/runtime-config-snapshot", async () => { }; }); +const pathValidationMocks = vi.hoisted(() => ({ + resolveExistingUploadPaths: vi.fn< + (args: { + requestedPaths: string[]; + }) => Promise<{ ok: true; paths: string[] } | { ok: false; error: string }> + >(async ({ requestedPaths }) => ({ + ok: true as const, + paths: requestedPaths, + })), +})); + const sessionTabRegistryMocks = vi.hoisted(() => ({ touchSessionBrowserTab: vi.fn(), trackSessionBrowserTab: vi.fn(), @@ -226,10 +237,7 @@ vi.mock("./browser-tool.runtime.js", () => { }, readStringParam, readStringValue, - resolveExistingPathsWithinRoot: vi.fn(async ({ requestedPaths }) => ({ - ok: true, - paths: requestedPaths, - })), + resolveExistingUploadPaths: pathValidationMocks.resolveExistingUploadPaths, resolveNodeIdFromList: (nodes: Array>, requested: string) => { const node = nodes.find( (entry) => entry.nodeId === requested || entry.displayName === requested, @@ -1634,3 +1642,45 @@ describe("browser tool act stale target recovery", () => { expect(browserActionsMocks.browserAct).toHaveBeenCalledTimes(1); }); }); + +describe("browser tool upload inbound media fallback (#83544)", () => { + beforeEach(resetBrowserToolMocks); + afterEach(() => vi.restoreAllMocks()); + + it("resolves upload paths before arming the file chooser", async () => { + const inboundPath = "/home/user/.openclaw/media/inbound/report.pdf"; + pathValidationMocks.resolveExistingUploadPaths.mockResolvedValue({ + ok: true, + paths: [inboundPath], + }); + browserActionsMocks.browserArmFileChooser.mockResolvedValue({ ok: true }); + + const tool = createBrowserTool(); + const result = await tool.execute?.("call-upload-1", { + action: "upload", + paths: [inboundPath], + ref: "file-input-1", + }); + + expect(pathValidationMocks.resolveExistingUploadPaths).toHaveBeenCalledWith({ + requestedPaths: [inboundPath], + }); + expect(result?.content[0]).toHaveProperty("type", "text"); + }); + + it("rejects files outside both uploads and inbound media directories", async () => { + pathValidationMocks.resolveExistingUploadPaths.mockResolvedValue({ + ok: false as const, + error: "path outside allowed directories", + }); + + const tool = createBrowserTool(); + await expect( + tool.execute?.("call-upload-2", { + action: "upload", + paths: ["/etc/passwd"], + ref: "file-input-1", + }), + ).rejects.toThrow("path outside allowed directories"); + }); +}); diff --git a/extensions/browser/src/browser-tool.ts b/extensions/browser/src/browser-tool.ts index d603458eb06..c48d503edf2 100644 --- a/extensions/browser/src/browser-tool.ts +++ b/extensions/browser/src/browser-tool.ts @@ -8,7 +8,6 @@ import { import { type AnyAgentTool, type NodeListNode, - DEFAULT_UPLOAD_DIR, BrowserToolSchema, applyBrowserProxyPaths, browserAct, @@ -37,8 +36,8 @@ import { readStringParam, readStringValue, resolveBrowserConfig, + resolveExistingUploadPaths, resolveRuntimeImageSanitization, - resolveExistingPathsWithinRoot, resolveNodeIdFromList, resolveProfile, selectDefaultNodeFromList, @@ -821,15 +820,11 @@ export function createBrowserTool(opts?: { if (paths.length === 0) { throw new Error("paths required"); } - const uploadPathsResult = await resolveExistingPathsWithinRoot({ - rootDir: DEFAULT_UPLOAD_DIR, - requestedPaths: paths, - scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`, - }); - if (!uploadPathsResult.ok) { - throw new Error(uploadPathsResult.error); + const resolvedResult = await resolveExistingUploadPaths({ requestedPaths: paths }); + if (!resolvedResult.ok) { + throw new Error(resolvedResult.error); } - const normalizedPaths = uploadPathsResult.paths; + const normalizedPaths = resolvedResult.paths; const ref = readStringParam(params, "ref"); const inputRef = readStringParam(params, "inputRef"); const element = readStringParam(params, "element"); diff --git a/extensions/browser/src/browser/paths.test.ts b/extensions/browser/src/browser/paths.test.ts index 14af336ff53..9489d681c85 100644 --- a/extensions/browser/src/browser/paths.test.ts +++ b/extensions/browser/src/browser/paths.test.ts @@ -4,21 +4,29 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { resolveExistingPathsWithinRoot, + resolveExistingUploadPaths, resolvePathsWithinRoot, resolvePathWithinRoot, resolveStrictExistingPathsWithinRoot, + resolveStrictExistingUploadPaths, resolveWritablePathWithinRoot, } from "./paths.js"; -async function createFixtureRoot(): Promise<{ baseDir: string; uploadsDir: string }> { +async function createFixtureRoot(): Promise<{ + baseDir: string; + inboundMediaDir: string; + uploadsDir: string; +}> { const baseDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-browser-paths-")); const uploadsDir = path.join(baseDir, "uploads"); + const inboundMediaDir = path.join(baseDir, "media", "inbound"); await fs.mkdir(uploadsDir, { recursive: true }); - return { baseDir, uploadsDir }; + await fs.mkdir(inboundMediaDir, { recursive: true }); + return { baseDir, inboundMediaDir, uploadsDir }; } async function withFixtureRoot( - run: (ctx: { baseDir: string; uploadsDir: string }) => Promise, + run: (ctx: { baseDir: string; inboundMediaDir: string; uploadsDir: string }) => Promise, ): Promise { const fixture = await createFixtureRoot(); try { @@ -223,6 +231,177 @@ describe("resolveExistingPathsWithinRoot", () => { ); }); +describe("resolveExistingUploadPaths", () => { + it("falls back to inbound media when the uploads root rejects the file", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [inboundFile], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(inboundFile)]); + } + }); + }); + + it("resolves canonical inbound media URI references before root validation", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media://inbound/report.pdf"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(inboundFile)]); + } + }); + }); + + it("falls back to sandbox-relative inbound media paths after root validation", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media/inbound/report.pdf"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(inboundFile)]); + } + }); + }); + + it("keeps upload-root paths before sandbox-relative inbound media fallback", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const uploadFile = path.join(uploadsDir, "media", "inbound", "report.pdf"); + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.mkdir(path.dirname(uploadFile), { recursive: true }); + await fs.writeFile(uploadFile, "upload", "utf8"); + await fs.writeFile(inboundFile, "inbound", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media/inbound/report.pdf"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(uploadFile)]); + } + }); + }); + + it("accepts mixed upload-root and inbound-media files in one request", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const uploadFile = path.join(uploadsDir, "from-upload.txt"); + const inboundFile = path.join(inboundMediaDir, "from-inbound.txt"); + await fs.writeFile(uploadFile, "upload", "utf8"); + await fs.writeFile(inboundFile, "inbound", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [uploadFile, "media://inbound/from-inbound.txt"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([ + await fs.realpath(uploadFile), + await fs.realpath(inboundFile), + ]); + } + }); + }); + + it("rejects nested inbound media URI references", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media://inbound/nested%2Fsecret.pdf"], + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("Invalid media reference"); + } + }); + }); + + it("rejects traversal-shaped inbound media URI references before URL normalization", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media://inbound/nested/../report.pdf"], + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("Invalid media reference"); + } + }); + }); + + it("rejects nested absolute inbound media paths", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const nestedDir = path.join(inboundMediaDir, "nested"); + await fs.mkdir(nestedDir, { recursive: true }); + const nestedFile = path.join(nestedDir, "secret.pdf"); + await fs.writeFile(nestedFile, "secret", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [nestedFile], + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("direct child of inbound media directory"); + } + }); + }); + + it("rejects files outside both managed upload roots", async () => { + await withFixtureRoot(async ({ baseDir, inboundMediaDir, uploadsDir }) => { + const outsideFile = path.join(baseDir, "secret.txt"); + await fs.writeFile(outsideFile, "secret", "utf8"); + + const result = await resolveExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [outsideFile], + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("inbound media directory"); + } + }); + }); +}); + describe("resolveStrictExistingPathsWithinRoot", () => { function expectInvalidResult( result: Awaited>, @@ -246,6 +425,141 @@ describe("resolveStrictExistingPathsWithinRoot", () => { }); }); +describe("resolveStrictExistingUploadPaths", () => { + it("falls back to inbound media for use-time upload validation", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [inboundFile], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(inboundFile)]); + } + }); + }); + + it("resolves inbound media URI references for use-time upload validation", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media://inbound/report.pdf"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(inboundFile)]); + } + }); + }); + + it("falls back to sandbox-relative inbound media paths for use-time upload validation", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.writeFile(inboundFile, "pdf", "utf8"); + + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media/inbound/report.pdf"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(inboundFile)]); + } + }); + }); + + it("keeps upload-root paths before sandbox-relative inbound media fallback at use time", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const uploadFile = path.join(uploadsDir, "media", "inbound", "report.pdf"); + const inboundFile = path.join(inboundMediaDir, "report.pdf"); + await fs.mkdir(path.dirname(uploadFile), { recursive: true }); + await fs.writeFile(uploadFile, "upload", "utf8"); + await fs.writeFile(inboundFile, "inbound", "utf8"); + + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["media/inbound/report.pdf"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([await fs.realpath(uploadFile)]); + } + }); + }); + + it("accepts mixed upload-root and inbound-media files at use time", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const uploadFile = path.join(uploadsDir, "from-upload.txt"); + const inboundFile = path.join(inboundMediaDir, "from-inbound.txt"); + await fs.writeFile(uploadFile, "upload", "utf8"); + await fs.writeFile(inboundFile, "inbound", "utf8"); + + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [uploadFile, "media/inbound/from-inbound.txt"], + }); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.paths).toEqual([ + await fs.realpath(uploadFile), + await fs.realpath(inboundFile), + ]); + } + }); + }); + + it("rejects files missing from both managed upload roots", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: ["missing.txt"], + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("regular non-symlink file"); + } + }); + }); + + it("rejects nested absolute inbound media paths at use time", async () => { + await withFixtureRoot(async ({ inboundMediaDir, uploadsDir }) => { + const nestedDir = path.join(inboundMediaDir, "nested"); + await fs.mkdir(nestedDir, { recursive: true }); + const nestedFile = path.join(nestedDir, "secret.pdf"); + await fs.writeFile(nestedFile, "secret", "utf8"); + + const result = await resolveStrictExistingUploadPaths({ + uploadDir: uploadsDir, + inboundMediaDir, + requestedPaths: [nestedFile], + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("direct child of inbound media directory"); + } + }); + }); +}); + describe("resolvePathWithinRoot", () => { it("uses default file name when requested path is blank", () => { const result = resolvePathWithinRoot({ diff --git a/extensions/browser/src/browser/paths.ts b/extensions/browser/src/browser/paths.ts index fa5f5543f7c..994e99cc84a 100644 --- a/extensions/browser/src/browser/paths.ts +++ b/extensions/browser/src/browser/paths.ts @@ -1,13 +1,18 @@ +import fs from "node:fs/promises"; import path from "node:path"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; -export { +import { resolveExistingPathsWithinRoot, + resolveStrictExistingPathsWithinRoot, +} from "../sdk-security-runtime.js"; +import { CONFIG_DIR } from "../utils.js"; +export { pathScope, resolvePathsWithinRoot, resolvePathWithinRoot, - resolveStrictExistingPathsWithinRoot, resolveWritablePathWithinRoot, } from "../sdk-security-runtime.js"; +export { resolveExistingPathsWithinRoot, resolveStrictExistingPathsWithinRoot }; const DEFAULT_FALLBACK_BROWSER_TMP_DIR = "/tmp/openclaw"; @@ -33,3 +38,235 @@ const DEFAULT_BROWSER_TMP_DIR = canUseNodeFs() export const DEFAULT_TRACE_DIR = DEFAULT_BROWSER_TMP_DIR; export const DEFAULT_DOWNLOAD_DIR = path.join(DEFAULT_BROWSER_TMP_DIR, "downloads"); export const DEFAULT_UPLOAD_DIR = path.join(DEFAULT_BROWSER_TMP_DIR, "uploads"); +export const DEFAULT_INBOUND_MEDIA_DIR = path.join(CONFIG_DIR, "media", "inbound"); + +type ExistingPathsResult = Awaited>; +type StrictExistingPathsResult = Awaited>; + +type UploadPathResolutionOptions = { + requestedPaths: string[]; + uploadDir?: string; + inboundMediaDir?: string; +}; + +type ResolvedManagedInboundMediaRef = + | { ok: true; path: string; uploadRootPrecedence: boolean } + | { ok: false; error: string } + | null; + +type DecodedInboundMediaId = { ok: true; path: string } | { ok: false; error: string }; + +function normalizeUploadPathSource(source: string): string { + const trimmed = source.trim(); + if (/^media:\/\//i.test(trimmed)) { + return trimmed; + } + return trimmed.replace(/^\s*MEDIA\s*:\s*/i, "").trim(); +} + +function decodeInboundMediaId(value: string, source: string): DecodedInboundMediaId { + let id: string; + try { + id = decodeURIComponent(value); + } catch { + return { ok: false, error: `Invalid media reference: ${source}` }; + } + if ( + !id || + id === "." || + id === ".." || + id.includes("/") || + id.includes("\\") || + id.includes("\0") + ) { + return { ok: false, error: `Invalid media reference: ${source}` }; + } + return { ok: true, path: id }; +} + +function resolveManagedInboundMediaRef( + source: string, + inboundMediaDir: string, +): ResolvedManagedInboundMediaRef { + const normalizedSource = normalizeUploadPathSource(source); + if (!normalizedSource) { + return null; + } + + if (/^media:\/\//i.test(normalizedSource)) { + const rawUriMatch = /^media:\/\/[^/?#]*([^?#]*)/iu.exec(normalizedSource); + const rawPath = rawUriMatch?.[1] ?? ""; + let parsed: URL; + try { + parsed = new URL(normalizedSource); + } catch { + return { ok: false, error: `Invalid media reference: ${normalizedSource}` }; + } + if (parsed.hostname !== "inbound") { + return { + ok: false, + error: `Unsupported media reference location: ${parsed.hostname || "(missing)"}`, + }; + } + if (!rawPath.startsWith("/") || rawPath.slice(1).includes("/") || rawPath.includes("\\")) { + return { ok: false, error: `Invalid media reference: ${normalizedSource}` }; + } + const decoded = decodeInboundMediaId(rawPath.slice(1), normalizedSource); + return decoded?.ok + ? { + ok: true, + path: path.join(inboundMediaDir, decoded.path), + uploadRootPrecedence: false, + } + : decoded; + } + + const relativeMatch = /^(?:\.\/)?media\/inbound\/([^/\\]+)$/u.exec(normalizedSource); + if (!relativeMatch?.[1]) { + return null; + } + const decoded = decodeInboundMediaId(relativeMatch[1], normalizedSource); + return decoded?.ok + ? { + ok: true, + path: path.join(inboundMediaDir, decoded.path), + uploadRootPrecedence: true, + } + : decoded; +} + +async function isDirectInboundMediaFile(params: { + inboundMediaDir: string; + resolvedPath: string; +}): Promise { + let inboundRoot: string; + try { + inboundRoot = await fs.realpath(params.inboundMediaDir); + } catch { + inboundRoot = path.resolve(params.inboundMediaDir); + } + const relativePath = path.relative(inboundRoot, params.resolvedPath); + return ( + Boolean(relativePath) && + relativePath !== ".." && + !relativePath.startsWith(`..${path.sep}`) && + !path.isAbsolute(relativePath) && + !relativePath.includes("/") && + !relativePath.includes("\\") + ); +} + +async function resolveDirectInboundMediaPath(params: { + inboundMediaDir: string; + requestedPath: string; + strict: boolean; +}): Promise { + const inboundPathsResult = params.strict + ? await resolveStrictExistingPathsWithinRoot({ + rootDir: params.inboundMediaDir, + requestedPaths: [params.requestedPath], + scopeLabel: `inbound media directory (${params.inboundMediaDir})`, + }) + : await resolveExistingPathsWithinRoot({ + rootDir: params.inboundMediaDir, + requestedPaths: [params.requestedPath], + scopeLabel: `inbound media directory (${params.inboundMediaDir})`, + }); + if (!inboundPathsResult.ok) { + return inboundPathsResult; + } + const resolvedPath = inboundPathsResult.paths[0] ?? params.requestedPath; + if ( + !(await isDirectInboundMediaFile({ + inboundMediaDir: params.inboundMediaDir, + resolvedPath, + })) + ) { + return { + ok: false, + error: `Invalid media reference: must be a direct child of inbound media directory (${params.inboundMediaDir})`, + }; + } + return inboundPathsResult; +} + +export async function resolveExistingUploadPaths({ + requestedPaths, + uploadDir = DEFAULT_UPLOAD_DIR, + inboundMediaDir = DEFAULT_INBOUND_MEDIA_DIR, +}: UploadPathResolutionOptions): Promise { + const paths: string[] = []; + for (const requestedPath of requestedPaths) { + const managedMediaPathResult = resolveManagedInboundMediaRef(requestedPath, inboundMediaDir); + if (managedMediaPathResult?.ok === false) { + return managedMediaPathResult; + } + + if (managedMediaPathResult?.uploadRootPrecedence !== false) { + const uploadPathsResult = + managedMediaPathResult?.uploadRootPrecedence === true + ? await resolveStrictExistingPathsWithinRoot({ + rootDir: uploadDir, + requestedPaths: [requestedPath], + scopeLabel: `uploads directory (${uploadDir})`, + }) + : await resolveExistingPathsWithinRoot({ + rootDir: uploadDir, + requestedPaths: [requestedPath], + scopeLabel: `uploads directory (${uploadDir})`, + }); + if (uploadPathsResult.ok) { + paths.push(uploadPathsResult.paths[0] ?? requestedPath); + continue; + } + } + + const inboundPathsResult = await resolveDirectInboundMediaPath({ + inboundMediaDir, + requestedPath: managedMediaPathResult?.path ?? requestedPath, + strict: false, + }); + if (!inboundPathsResult.ok) { + return inboundPathsResult; + } + paths.push(inboundPathsResult.paths[0] ?? requestedPath); + } + return { ok: true, paths }; +} + +export async function resolveStrictExistingUploadPaths({ + requestedPaths, + uploadDir = DEFAULT_UPLOAD_DIR, + inboundMediaDir = DEFAULT_INBOUND_MEDIA_DIR, +}: UploadPathResolutionOptions): Promise { + const paths: string[] = []; + for (const requestedPath of requestedPaths) { + const managedMediaPathResult = resolveManagedInboundMediaRef(requestedPath, inboundMediaDir); + if (managedMediaPathResult?.ok === false) { + return managedMediaPathResult; + } + + if (managedMediaPathResult?.uploadRootPrecedence !== false) { + const uploadPathsResult = await resolveStrictExistingPathsWithinRoot({ + rootDir: uploadDir, + requestedPaths: [requestedPath], + scopeLabel: `uploads directory (${uploadDir})`, + }); + if (uploadPathsResult.ok) { + paths.push(uploadPathsResult.paths[0] ?? requestedPath); + continue; + } + } + + const inboundPathsResult = await resolveDirectInboundMediaPath({ + inboundMediaDir, + requestedPath: managedMediaPathResult?.path ?? requestedPath, + strict: true, + }); + if (!inboundPathsResult.ok) { + return inboundPathsResult; + } + paths.push(inboundPathsResult.paths[0] ?? requestedPath); + } + return { ok: true, paths }; +} diff --git a/extensions/browser/src/browser/pw-tools-core.downloads.ts b/extensions/browser/src/browser/pw-tools-core.downloads.ts index 41b452389b7..57040974da2 100644 --- a/extensions/browser/src/browser/pw-tools-core.downloads.ts +++ b/extensions/browser/src/browser/pw-tools-core.downloads.ts @@ -3,7 +3,7 @@ import path from "node:path"; import type { Page } from "playwright-core"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { writeExternalFileWithinOutputRoot } from "./output-files.js"; -import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js"; +import { resolveStrictExistingUploadPaths } from "./paths.js"; import { armObservedDialogResponseOnPage, ensurePageState, @@ -155,10 +155,8 @@ export async function armFileUploadViaPlaywright(opts: { } return; } - const uploadPathsResult = await resolveStrictExistingPathsWithinRoot({ - rootDir: DEFAULT_UPLOAD_DIR, + const uploadPathsResult = await resolveStrictExistingUploadPaths({ requestedPaths: opts.paths, - scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`, }); if (!uploadPathsResult.ok) { try { diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.set-input-files.test.ts b/extensions/browser/src/browser/pw-tools-core.interactions.set-input-files.test.ts index 33256f5bf50..65a339af2c1 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.set-input-files.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.set-input-files.test.ts @@ -21,8 +21,8 @@ const refLocator = vi.fn(() => { }); const forceDisconnectPlaywrightForTarget = vi.fn(async () => {}); -const resolveStrictExistingPathsWithinRoot = - vi.fn(); +const resolveStrictExistingUploadPaths = + vi.fn(); vi.mock("./pw-session.js", () => { return { @@ -38,8 +38,7 @@ vi.mock("./pw-session.js", () => { vi.mock("./paths.js", () => { return { - DEFAULT_UPLOAD_DIR: "/tmp/openclaw/uploads", - resolveStrictExistingPathsWithinRoot, + resolveStrictExistingUploadPaths, }; }); @@ -62,7 +61,7 @@ describe("setInputFilesViaPlaywright", () => { vi.clearAllMocks(); page = null; locator = null; - resolveStrictExistingPathsWithinRoot.mockResolvedValue({ + resolveStrictExistingUploadPaths.mockResolvedValue({ ok: true, paths: ["/private/tmp/openclaw/uploads/ok.txt"], }); @@ -78,19 +77,17 @@ describe("setInputFilesViaPlaywright", () => { paths: ["/tmp/openclaw/uploads/ok.txt"], }); - expect(resolveStrictExistingPathsWithinRoot).toHaveBeenCalledWith({ - rootDir: "/tmp/openclaw/uploads", + expect(resolveStrictExistingUploadPaths).toHaveBeenCalledWith({ requestedPaths: ["/tmp/openclaw/uploads/ok.txt"], - scopeLabel: "uploads directory (/tmp/openclaw/uploads)", }); expect(refLocator).toHaveBeenCalledWith(page, "e7"); expect(setInputFiles).toHaveBeenCalledWith(["/private/tmp/openclaw/uploads/ok.txt"]); }); it("throws and skips setInputFiles when use-time validation fails", async () => { - resolveStrictExistingPathsWithinRoot.mockResolvedValueOnce({ + resolveStrictExistingUploadPaths.mockResolvedValueOnce({ ok: false, - error: "Invalid path: must stay within uploads directory", + error: "Invalid path: must stay within inbound media directory", }); const { setInputFiles } = seedSingleLocatorPage(); @@ -102,7 +99,7 @@ describe("setInputFilesViaPlaywright", () => { element: "input[type=file]", paths: ["/tmp/openclaw/uploads/missing.txt"], }), - ).rejects.toThrow("Invalid path: must stay within uploads directory"); + ).rejects.toThrow("Invalid path: must stay within inbound media directory"); expect(setInputFiles).not.toHaveBeenCalled(); }); diff --git a/extensions/browser/src/browser/pw-tools-core.interactions.ts b/extensions/browser/src/browser/pw-tools-core.interactions.ts index 69c9bdc4044..4c742a87c1b 100644 --- a/extensions/browser/src/browser/pw-tools-core.interactions.ts +++ b/extensions/browser/src/browser/pw-tools-core.interactions.ts @@ -17,7 +17,7 @@ import { assertBrowserNavigationResultAllowed, withBrowserNavigationPolicy, } from "./navigation-guard.js"; -import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js"; +import { resolveStrictExistingUploadPaths } from "./paths.js"; import { assertPageNavigationCompletedSafely, createObservedDialogAbortSignalForPage, @@ -1396,15 +1396,11 @@ export async function setInputFilesViaPlaywright(opts: { } const locator = inputRef ? refLocator(page, inputRef) : page.locator(element).first(); - const uploadPathsResult = await resolveStrictExistingPathsWithinRoot({ - rootDir: DEFAULT_UPLOAD_DIR, - requestedPaths: opts.paths, - scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`, - }); - if (!uploadPathsResult.ok) { - throw new Error(uploadPathsResult.error); + const resolvedResult = await resolveStrictExistingUploadPaths({ requestedPaths: opts.paths }); + if (!resolvedResult.ok) { + throw new Error(resolvedResult.error); } - const resolvedPaths = uploadPathsResult.paths; + const resolvedPaths = resolvedResult.paths; try { await locator.setInputFiles(resolvedPaths); diff --git a/extensions/browser/src/browser/pw-tools-core.upload-paths.test.ts b/extensions/browser/src/browser/pw-tools-core.upload-paths.test.ts new file mode 100644 index 00000000000..856573ac6e6 --- /dev/null +++ b/extensions/browser/src/browser/pw-tools-core.upload-paths.test.ts @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { installPwToolsCoreTestHooks, setPwToolsCoreCurrentPage } from "./pw-tools-core.test-harness.js"; + +const pathMocks = vi.hoisted(() => ({ + resolveStrictExistingUploadPaths: vi.fn< + (args: { requestedPaths: string[] }) => Promise< + { ok: true; paths: string[] } | { ok: false; error: string } + > + >(), +})); + +vi.mock("./paths.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + resolveStrictExistingUploadPaths: pathMocks.resolveStrictExistingUploadPaths, + }; +}); + +installPwToolsCoreTestHooks(); +const { armFileUploadViaPlaywright } = await import("./pw-tools-core.downloads.js"); + +function createFileChooserPageMocks() { + const fileChooser = { setFiles: vi.fn(async () => {}) }; + const press = vi.fn(async () => {}); + const waitForEvent = vi.fn(async () => fileChooser); + setPwToolsCoreCurrentPage({ + waitForEvent, + keyboard: { press }, + }); + return { fileChooser, press }; +} + +describe("armFileUploadViaPlaywright upload path validation", () => { + beforeEach(() => { + pathMocks.resolveStrictExistingUploadPaths.mockResolvedValue({ + ok: true, + paths: ["/home/user/.openclaw/media/inbound/report.pdf"], + }); + }); + + it("sets files using resolved inbound media paths", async () => { + const { fileChooser } = createFileChooserPageMocks(); + + await armFileUploadViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + paths: ["/home/user/.openclaw/media/inbound/report.pdf"], + }); + await Promise.resolve(); + + await vi.waitFor(() => { + expect(fileChooser.setFiles).toHaveBeenCalledWith([ + "/home/user/.openclaw/media/inbound/report.pdf", + ]); + }); + }); + + it("escapes the chooser when paths are outside managed upload roots", async () => { + pathMocks.resolveStrictExistingUploadPaths.mockResolvedValue({ + ok: false, + error: "Invalid path: must stay within inbound media directory", + }); + const { fileChooser, press } = createFileChooserPageMocks(); + + await armFileUploadViaPlaywright({ + cdpUrl: "http://127.0.0.1:18792", + targetId: "T1", + paths: ["/etc/passwd"], + }); + await Promise.resolve(); + + await vi.waitFor(() => { + expect(press).toHaveBeenCalledWith("Escape"); + }); + expect(fileChooser.setFiles).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/browser/src/browser/routes/agent.act.hooks.ts b/extensions/browser/src/browser/routes/agent.act.hooks.ts index 783425d14bd..91b0a6153a0 100644 --- a/extensions/browser/src/browser/routes/agent.act.hooks.ts +++ b/extensions/browser/src/browser/routes/agent.act.hooks.ts @@ -1,5 +1,6 @@ import { formatErrorMessage } from "../../infra/errors.js"; import { evaluateChromeMcpScript, uploadChromeMcpFile } from "../chrome-mcp.js"; +import { resolveExistingUploadPaths } from "../paths.js"; import { getBrowserProfileCapabilities } from "../profile-capabilities.js"; import type { BrowserRouteContext } from "../server-context.js"; import { @@ -9,7 +10,6 @@ import { withRouteTabContext, } from "./agent.shared.js"; import { EXISTING_SESSION_LIMITS } from "./existing-session-limits.js"; -import { DEFAULT_UPLOAD_DIR, pathScope } from "./path-output.js"; import { readRouteTimerTimeoutMs } from "./route-numeric.js"; import type { BrowserRouteRegistrar } from "./types.js"; import { @@ -49,14 +49,12 @@ export function registerBrowserAgentActHookRoutes( ctx, targetId, run: async ({ profileCtx, cdpUrl, tab }) => { - const uploadPathsResult = await pathScope(DEFAULT_UPLOAD_DIR, { - label: `uploads directory (${DEFAULT_UPLOAD_DIR})`, - }).existing(paths); - if (!uploadPathsResult.ok) { - res.status(400).json({ error: uploadPathsResult.error }); + const resolvedResult = await resolveExistingUploadPaths({ requestedPaths: paths }); + if (!resolvedResult.ok) { + res.status(400).json({ error: resolvedResult.error }); return; } - const resolvedPaths = uploadPathsResult.paths; + const resolvedPaths = resolvedResult.paths; if (getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp) { if (element) { diff --git a/extensions/browser/src/cli/browser-cli-actions-input/register.files-downloads.ts b/extensions/browser/src/cli/browser-cli-actions-input/register.files-downloads.ts index 44b05c1805d..b6297807fd6 100644 --- a/extensions/browser/src/cli/browser-cli-actions-input/register.files-downloads.ts +++ b/extensions/browser/src/cli/browser-cli-actions-input/register.files-downloads.ts @@ -7,9 +7,8 @@ import { } from "../browser-cli-shared.js"; import { danger, - DEFAULT_UPLOAD_DIR, defaultRuntime, - resolveExistingPathsWithinRoot, + resolveExistingUploadPaths, shortenHomePath, } from "../core-api.js"; import { resolveBrowserActionContext, withBrowserActionTimeoutSlack } from "./shared.js"; @@ -17,11 +16,7 @@ import { resolveBrowserActionContext, withBrowserActionTimeoutSlack } from "./sh const DEFAULT_BROWSER_HOOK_TIMEOUT_MS = 120000; async function normalizeUploadPaths(paths: string[]): Promise { - const result = await resolveExistingPathsWithinRoot({ - rootDir: DEFAULT_UPLOAD_DIR, - requestedPaths: paths, - scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`, - }); + const result = await resolveExistingUploadPaths({ requestedPaths: paths }); if (!result.ok) { throw new Error(result.error); } @@ -95,7 +90,7 @@ export function registerBrowserFilesAndDownloadsCommands( .description("Arm file upload for the next file chooser") .argument( "", - "File paths to upload (must be within OpenClaw temp uploads dir, e.g. /tmp/openclaw/uploads/file.pdf)", + "File paths to upload from OpenClaw temp uploads or managed inbound media (e.g. /tmp/openclaw/uploads/file.pdf or media://inbound/)", ) .option("--ref ", "Ref id from snapshot to click after arming") .option("--input-ref ", "Ref id for to set directly") diff --git a/extensions/browser/src/cli/browser-cli-examples.ts b/extensions/browser/src/cli/browser-cli-examples.ts index 084f94041d7..d4ac122a452 100644 --- a/extensions/browser/src/cli/browser-cli-examples.ts +++ b/extensions/browser/src/cli/browser-cli-examples.ts @@ -27,6 +27,7 @@ export const browserActionExamples = [ "openclaw browser drag 10 11", "openclaw browser select 9 OptionA OptionB", "openclaw browser upload /tmp/openclaw/uploads/file.pdf", + "openclaw browser upload media://inbound/file.pdf", 'openclaw browser fill --fields \'[{"ref":"1","value":"Ada"}]\'', "openclaw browser dialog --accept", 'openclaw browser wait --text "Done"', diff --git a/extensions/browser/src/core-api.ts b/extensions/browser/src/core-api.ts index c11c6e6c6c3..e685e9f031f 100644 --- a/extensions/browser/src/core-api.ts +++ b/extensions/browser/src/core-api.ts @@ -42,6 +42,7 @@ export { resolveBrowserConfig, resolveBrowserControlAuth, resolveExistingPathsWithinRoot, + resolveExistingUploadPaths, resolveProfile, resolveRequestedBrowserProfile, startBrowserControlServiceFromConfig,