From ba97d141ee2560d6261cc91da73fb9aafd9b1f90 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 11 Aug 2026 20:08:03 -0700 Subject: [PATCH] [eric] browser: agents can attach a file to an upload field without the OS picker; the path is allow-listed backend-side so a hostile page cannot turn it into exfiltration (ENG-47) --- backend/apps/agents/browser/browser_agent.py | 11 ++- backend/apps/agents/browser/browser_schema.py | 36 ++++++++ .../agents/browser/resolve_upload_path.py | 69 ++++++++++++++ backend/tests/test_resolve_upload_path.py | 90 +++++++++++++++++++ frontend/src/shared/browserCommandHandler.ts | 77 +++++++++++++++- 5 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 backend/apps/agents/browser/resolve_upload_path.py create mode 100644 backend/tests/test_resolve_upload_path.py diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 1c8d248e..6022a836 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -211,6 +211,7 @@ def render_app_controls(describe_value: object) -> tuple[str, str] | None: # both BrowserPressKey({key}) and a batch's {"type":"press_key","params":{key}}. P_SINGLE_ACTION_TYPE = { "BrowserClick": "click", "BrowserClickIndex": "click_index", + "BrowserUploadFile": "upload_file", "BrowserClickByName": "click_name", "BrowserType": "type", "BrowserPressKey": "press_key", "BrowserScroll": "scroll", "BrowserNavigate": "navigate", "BrowserClickPoint": "click_point", @@ -415,6 +416,14 @@ async def p_execute_browser_tool( return {"error": f"Unknown browser tool: {tool_name}"} params = {k: v for k, v in tool_input.items()} + # Resolve the upload path HERE, before the command crosses to the renderer, so the only string + # the page-driven agent can turn into bytes-on-the-wire is one that already passed the allow-list. + if action == "upload_file": + from backend.apps.agents.browser.resolve_upload_path import resolve_upload_path, UploadPathRefused + try: + params["path"] = resolve_upload_path(str(params.get("path") or "")) + except UploadPathRefused as p_refused: + return {"error": str(p_refused)} # Self-healing click toggle + click-effect metric. Threaded for solo clicks AND batches (most clicks are batched, so gating on click_index alone misses them). handleBatch propagates these into its click_index sub-actions. if action in ("click_index", "batch"): params["selfheal"] = os.environ.get("OSW_SELFHEAL_CLICK", "1") != "0" @@ -560,7 +569,7 @@ def format_tool_result(result: dict, tool_name: str) -> list[dict]: # Mutating tools whose results get fresh page state attached (the browser-use loop shape: act, settle, see), so acting and seeing are one turn, not two. P_AUTO_STATE_TOOLS = { - "BrowserNavigate", "BrowserClick", "BrowserClickIndex", "BrowserClickByName", + "BrowserNavigate", "BrowserClick", "BrowserClickIndex", "BrowserClickByName", "BrowserUploadFile", "BrowserType", "BrowserPressKey", "BrowserScroll", "BrowserBatch", } # Matches the frontend's DEFAULT_INTERACTIVE_CAP (interactiveRanking.ts): a shorter cap here silently hid rows 36-60 that an explicit BrowserListInteractives would show, forcing the model to re-list the very elements it just acted on. Delta compression keeps the common attach small, so the worst case (a full 60-row attach) is bounded and rare. diff --git a/backend/apps/agents/browser/browser_schema.py b/backend/apps/agents/browser/browser_schema.py index e04123a3..eeb89d32 100644 --- a/backend/apps/agents/browser/browser_schema.py +++ b/backend/apps/agents/browser/browser_schema.py @@ -392,6 +392,40 @@ BROWSER_TOOLS_SCHEMA = [ "required": ["index"], }, }, + { + "name": "BrowserUploadFile", + "description": ( + "Attach a local file to a file-upload field (resume, photo, document). ALWAYS use this " + "instead of clicking an Upload / Choose File / Attach button: clicking one opens the " + "operating system's file picker, which is outside the page and which you cannot see or " + "control, so the run dead-ends there. This tool fills the field directly, no dialog " + "opens, and the page's own change handlers fire exactly as if a human had picked the " + "file.\n" + "It finds the file input for you even when the site hides it behind a styled button, " + "which is the usual design. Pass `index` only to disambiguate when a page has several " + "upload fields and the first one is not the one you want.\n" + "`path` must be a file the user attached to this chat or one an agent created in its " + "workspace; anything else is refused. Verify the result: it reports back the filename " + "the page actually received." + ), + "input_schema": { + "type": "object", + "properties": { + "path": { + "type": "string", + "description": "Absolute path of the local file to attach.", + }, + "index": { + "type": "integer", + "description": ( + "Optional. 1-based index from BrowserListInteractives, when the page has " + "more than one upload field. Omit to use the page's first one." + ), + }, + }, + "required": ["path"], + }, + }, { "name": "BrowserActVerified", "description": ( @@ -807,6 +841,7 @@ ACTION_MAP = { "BrowserPressKey": "press_key", "BrowserListInteractives": "list_interactives", "BrowserClickIndex": "click_index", + "BrowserUploadFile": "upload_file", "BrowserClickPoint": "click_point", "BrowserBatch": "batch", "BrowserDetectWebMCP": "detect_webmcp", @@ -1221,6 +1256,7 @@ ACTION_TOOLS_REQUIRING_REPORT = { "BrowserScroll", "BrowserEvaluate", "BrowserClickIndex", # Phase 3 + "BrowserUploadFile", "BrowserClickPoint", # app mode: tap a canvas/game at a screen point "BrowserBatch", # Phase 4 "BrowserActVerified", # verified-step sequence (mutates state like a batch) diff --git a/backend/apps/agents/browser/resolve_upload_path.py b/backend/apps/agents/browser/resolve_upload_path.py new file mode 100644 index 00000000..240b233c --- /dev/null +++ b/backend/apps/agents/browser/resolve_upload_path.py @@ -0,0 +1,69 @@ +"""Containment guard for the one tool that hands a local file to a web page. + +The browser sub-agent reads its instructions off the page it is driving, so a hostile page can ask +it for anything. Uploading is the first browser tool that can move bytes OFF the machine, which +makes it the only one where a prompt injection converts into exfiltration. So the path is resolved +against an allow-list here, on the backend, before the command ever reaches the renderer: the +agent can offer any string it likes and still cannot name `~/.ssh/id_rsa`. +""" + +import os +from typing import List +from typeguard import typechecked + +from backend.apps.settings.settings import UPLOAD_DIR +from backend.config.paths import OUTPUTS_WORKSPACE_DIR, SKILLS_WORKSPACE_DIR + +# Big enough for a portfolio PDF or a short video, small enough that a runaway loop can't post a disk image. +MAX_UPLOAD_BYTES = 100 * 1024 * 1024 + + +class UploadPathRefused(Exception): + """The requested file is outside every allowed root, missing, or too large.""" + + +@typechecked +def allowed_upload_roots() -> List[str]: + """Roots a file may be uploaded from: what the user attached, and what agents produce.""" + # ~/.openswarm/workspaces is where a chat agent's own scratch cwd lives (AgentLaunch), so a file + # the agent just wrote and now wants to upload is covered without opening up the whole home dir. + roots = [ + UPLOAD_DIR, OUTPUTS_WORKSPACE_DIR, SKILLS_WORKSPACE_DIR, + os.path.join(os.path.expanduser("~"), ".openswarm", "workspaces"), + ] + out: List[str] = [] + for r in roots: + try: + out.append(os.path.realpath(r)) + except OSError: + continue + return out + + +@typechecked +def resolve_upload_path(path: str) -> str: + """Absolute real path of an uploadable file, or raise UploadPathRefused. + + realpath both sides, then compare whole components: without the trailing separator a root named + `uploads` would also own `uploads-evil`, and plain string math walks straight through a symlink + planted inside an allowed root and pointing at the user's home. + """ + raw = (path or "").strip() + if not raw: + raise UploadPathRefused("No file path given.") + target = os.path.realpath(os.path.expanduser(raw)) + roots = allowed_upload_roots() + if not any(target == r or target.startswith(r + os.sep) for r in roots): + raise UploadPathRefused( + f"Refused: {raw} is outside the folders this agent may upload from. " + "Uploadable files are the ones the user attached to the chat and the ones agents " + "created in their workspace. Copy the file into the workspace first, then upload it." + ) + if not os.path.isfile(target): + raise UploadPathRefused(f"Refused: {raw} is not a file that exists.") + size = os.path.getsize(target) + if size > MAX_UPLOAD_BYTES: + raise UploadPathRefused( + f"Refused: {raw} is {size // (1024 * 1024)}MB, over the {MAX_UPLOAD_BYTES // (1024 * 1024)}MB upload cap." + ) + return target diff --git a/backend/tests/test_resolve_upload_path.py b/backend/tests/test_resolve_upload_path.py new file mode 100644 index 00000000..028293ed --- /dev/null +++ b/backend/tests/test_resolve_upload_path.py @@ -0,0 +1,90 @@ +"""ENG-47: the upload tool is the first browser tool that can move bytes off the machine, and the +browser sub-agent takes its instructions from the page it is driving. These pin the containment +guard, including the case that matters most: a symlink planted INSIDE an allowed root.""" + +import os +import pytest + +from backend.apps.agents.browser.resolve_upload_path import ( + resolve_upload_path, + allowed_upload_roots, + UploadPathRefused, + MAX_UPLOAD_BYTES, +) + + +@pytest.fixture +def uploads_dir(): + root = allowed_upload_roots()[0] + os.makedirs(root, exist_ok=True) + return root + + +def test_a_file_the_user_attached_is_uploadable(uploads_dir): + f = os.path.join(uploads_dir, "resume.pdf") + with open(f, "w", encoding="utf-8") as fh: + fh.write("cv") + assert resolve_upload_path(f) == os.path.realpath(f) + + +@pytest.mark.parametrize("hostile", ["~/.ssh/id_rsa", "/etc/passwd", "/etc/hosts", ""]) +def test_paths_outside_every_allowed_root_are_refused(hostile): + with pytest.raises(UploadPathRefused): + resolve_upload_path(hostile) + + +def test_a_symlink_inside_an_allowed_root_cannot_smuggle_a_file_out(uploads_dir): + # String math on the path would pass this: it really does live under the uploads root. + link = os.path.join(uploads_dir, "innocent.txt") + if os.path.lexists(link): + os.remove(link) + os.symlink("/etc/hosts", link) + try: + with pytest.raises(UploadPathRefused): + resolve_upload_path(link) + finally: + os.remove(link) + + +def test_a_sibling_root_with_a_shared_prefix_is_not_inside_it(uploads_dir): + # Without the trailing separator, root `self-swarm-uploads` would own `self-swarm-uploads-evil`. + evil = uploads_dir + "-evil" + os.makedirs(evil, exist_ok=True) + f = os.path.join(evil, "x.txt") + with open(f, "w", encoding="utf-8") as fh: + fh.write("x") + with pytest.raises(UploadPathRefused): + resolve_upload_path(f) + + +def test_a_directory_is_not_a_file(uploads_dir): + with pytest.raises(UploadPathRefused): + resolve_upload_path(uploads_dir) + + +def test_an_oversized_file_is_refused(uploads_dir, monkeypatch): + f = os.path.join(uploads_dir, "huge.bin") + with open(f, "w", encoding="utf-8") as fh: + fh.write("x") + monkeypatch.setattr(os.path, "getsize", lambda p: MAX_UPLOAD_BYTES + 1) + with pytest.raises(UploadPathRefused): + resolve_upload_path(f) + + +def test_the_dispatcher_refuses_before_the_command_leaves_the_backend(monkeypatch): + """The guard has to sit in front of the WS hop, not inside the renderer.""" + import asyncio + import backend.apps.agents.core.ws_manager as ws_mod + from backend.apps.agents.browser import browser_agent + + sent = [] + + async def spy(*args, **kwargs): + sent.append(args) + return {"text": "should never happen"} + + monkeypatch.setattr(ws_mod.ws_manager, "send_browser_command", spy, raising=True) + out = asyncio.run(browser_agent.execute_browser_tool( + "BrowserUploadFile", {"path": "/etc/passwd"}, "browser-1")) + assert "Refused" in str(out.get("error", "")) + assert sent == [], "a refused path must never reach the renderer" diff --git a/frontend/src/shared/browserCommandHandler.ts b/frontend/src/shared/browserCommandHandler.ts index 2edee856..17a32551 100644 --- a/frontend/src/shared/browserCommandHandler.ts +++ b/frontend/src/shared/browserCommandHandler.ts @@ -12,7 +12,7 @@ import { typeChars, type TypedKeys } from './typeChars'; let initialized = false; -export type BrowserAction = 'screenshot' | 'get_text' | 'get_console' | 'navigate' | 'click' | 'type' | 'evaluate' | 'get_elements' | 'scroll' | 'wait' | 'press_key' | 'list_interactives' | 'click_index' | 'click_point' | 'batch' | 'detect_webmcp' | 'list_routes' | 'replay_route' | 'click_by_name' | 'find_composer'; +export type BrowserAction = 'screenshot' | 'get_text' | 'get_console' | 'navigate' | 'click' | 'type' | 'evaluate' | 'get_elements' | 'scroll' | 'wait' | 'press_key' | 'list_interactives' | 'click_index' | 'upload_file' | 'click_point' | 'batch' | 'detect_webmcp' | 'list_routes' | 'replay_route' | 'click_by_name' | 'find_composer'; export interface BrowserActivity { action: BrowserAction; @@ -1639,6 +1639,78 @@ async function handleClickIndex(wv: BrowserWebview, params: Record) return result; } +// Attach a local file to an without ever opening the OS picker. A native dialog +// lives outside the page, so an agent that clicks an Upload button can neither see nor dismiss it and +// the run dead-ends (ENG-47); CDP fills the field directly and the page's change handlers fire as if +// a human had picked it. Sites almost always hide the real input behind a styled button, so we find it +// ourselves rather than trusting a visible-element index. +async function handleUploadFile(wv: BrowserWebview, params: Record): Promise> { + const path = String(params.path || ''); + if (!path) return { error: 'path parameter is required' }; + + let backendNodeId: number | undefined; + let sessionId: string | undefined; + const idx = Number(params.index); + if (Number.isFinite(idx) && idx >= 1) { + try { + const cacheBridge = (window as any).openswarm?.cdpCacheGet; + const cached = cacheBridge ? await cacheBridge(wv.getWebContentsId()) : null; + const entry = cached && cached[idx]; + if (typeof entry === 'number') backendNodeId = entry; + else if (entry && typeof entry === 'object' && entry.backendNodeId != null) { + backendNodeId = Number(entry.backendNodeId); + sessionId = entry.sessionId || undefined; + } + } catch { /* fall through to the page-wide search below */ } + } + + let found = 0; + if (backendNodeId == null) { + try { + const doc = await sendCdp(wv, 'DOM.getDocument', { depth: 0 }); + const hits = await sendCdp(wv, 'DOM.querySelectorAll', + { nodeId: doc?.root?.nodeId, selector: 'input[type=file]' }); + const nodeIds: number[] = hits?.nodeIds || []; + found = nodeIds.length; + if (!found) { + return { error: 'No file-upload field on this page. Open the page or dialog that has the upload control first, then retry.' }; + } + const described = await sendCdp(wv, 'DOM.describeNode', { nodeId: nodeIds[0] }); + backendNodeId = described?.node?.backendNodeId; + } catch (err: any) { + return { error: `Could not search the page for an upload field (${err?.message || 'DOM query failed'}).` }; + } + } + if (backendNodeId == null) return { error: 'Could not resolve the upload field on this page.' }; + + try { + await sendCdp(wv, 'DOM.setFileInputFiles', { files: [path], backendNodeId }, sessionId); + } catch (err: any) { + return { error: `Attaching the file failed (${err?.message || 'setFileInputFiles failed'}).` }; + } + + // Read the filename back off the input. "The command returned OK" is not "the page has the file". + try { + const resolved = await sendCdp(wv, 'DOM.resolveNode', { backendNodeId }, sessionId); + const r = await sendCdp(wv, 'Runtime.callFunctionOn', { + objectId: resolved?.object?.objectId, + functionDeclaration: 'function() { const f = this.files; return f && f.length ? f[0].name + "|" + f[0].size : ""; }', + returnByValue: true, + }, sessionId); + const receipt = String(r?.result?.value || ''); + if (!receipt) { + return { error: 'The upload field did not accept the file (it reports no file attached). The site may restrict the file type.' }; + } + const [name, size] = receipt.split('|'); + return { + text: `Attached "${name}" (${size} bytes) to the upload field${found > 1 ? ` (page has ${found} upload fields; used the first)` : ''}. The page's change handlers have fired. Any Submit/Save step is still yours to do.`, + uploadedName: name, + }; + } catch { + return { text: `Sent "${path.split('/').pop()}" to the upload field, but could not read the field back to confirm it took. Check the page before submitting.` }; + } +} + // Robust click for REPLAY: re-resolve the target fresh by (role, name) instead of a stale index, so a recorded skill survives index shifts between runs. async function handleClickByName(wv: BrowserWebview, params: Record): Promise> { const wantName = String(params.name || '').trim(); @@ -2295,6 +2367,9 @@ async function runBrowserCommand( case 'list_interactives': result = await handleListInteractives(wv, params); break; + case 'upload_file': + result = await handleUploadFile(wv, params); + break; case 'click_index': result = await handleClickIndex(wv, params); if (result.clickX != null && result.clickY != null) {