[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)

2026-08-22 20:52:23 +02:00 · 2026-08-11 20:10:36 -07:00
parent a28268bab9
commit ba97d141ee
5 changed files with 281 additions and 2 deletions
@@ -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.
@@ -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)
@@ -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
@@ -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"
@@ -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<string, any>)
  return result;
 }

+// Attach a local file to an <input type="file"> 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<string, any>): Promise<Record<string, any>> {
+  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<string, any>): Promise<Record<string, any>> {
  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) {