Files
browser-use--browser-use/tests/ci/security/test_upload_file_containment.py
wehub-resource-sync 4cd2d4af2b
Test Browser Use CLI Install / uv pip install (macos-latest) (push) Waiting to run
Test Browser Use CLI Install / uv pip install (windows-latest) (push) Waiting to run
Test Browser Use CLI Install / uv pip install (ubuntu-latest) (push) Failing after 1s
Test Browser Use CLI Install / uvx browser-use from local wheel (push) Failing after 1s
Test Browser Use CLI Install / uvx browser-use[cli] from PyPI (push) Failing after 1s
package / pip-install-on-macos-latest-py-3.11 (push) Has been skipped
package / pip-install-on-macos-latest-py-3.13 (push) Has been skipped
package / pip-install-on-ubuntu-latest-py-3.11 (push) Has been skipped
package / pip-install-on-windows-latest-py-3.13 (push) Has been skipped
cloud_evals / trigger_cloud_eval_image_build (push) Failing after 1s
docker / build_publish_image (push) Failing after 1s
Test Browser Use CLI Install / browser-use skill sync (push) Failing after 1s
lint / code-style (push) Failing after 0s
lint / type-checker (push) Failing after 1s
package / pip-build (push) Failing after 1s
lint / syntax-errors (push) Failing after 3s
package / pip-install-on-ubuntu-latest-py-3.13 (push) Has been skipped
package / pip-install-on-windows-latest-py-3.11 (push) Has been skipped
test / ${{ matrix.test_filename }} (push) Has been skipped
test / evaluate-tasks (push) Has been skipped
test / setup-chromium (push) Failing after 2s
test / find_tests (push) Failing after 2s
chore: import upstream snapshot with attribution
2026-07-13 12:02:32 +08:00

197 lines
7.2 KiB
Python

"""Tests for upload_file FileSystem path containment (GHSA-j9hj-92j8-jv9h).
The `upload_file` action used to construct the absolute upload path by joining
`file_system.get_dir()` with the agent-controlled `params.path`. Because
`FileSystem.get_file()` matches by basename (it does `os.path.basename` first),
an agent-controlled path like `../note.md` would:
1. Pass `get_file()` lookup if a file named `note.md` exists in the FileSystem.
2. Be naively joined to `data_dir` producing `data_dir/../note.md`, which
resolves *outside* the FileSystem directory.
3. Be uploaded to the browser as the resolved escaped file.
The fix uses `file_obj.full_name` (the FileSystem-owned basename) for the join
and additionally verifies via `os.path.realpath` that the result is contained
in `data_dir`.
"""
from __future__ import annotations
import os
from typing import Any
import pytest
from browser_use.agent.views import ActionResult
from browser_use.filesystem.file_system import FileSystem
from browser_use.tools.service import Tools
class _StubBrowserSession:
"""Minimal stub exposing only the attributes `upload_file` touches before it
would hand off to CDP. We stop the action just after path resolution.
Per the project rule "use real objects", this stub stands in only for the
browser/CDP boundary that has no bearing on the security property under test
(file path containment); the FileSystem and Tools registry are real.
"""
is_local = True
downloaded_files: list[str] = []
agent_focus_target_id: str | None = None
session_manager: Any = None
cdp_client: Any = None
# Capture the params.path passed in by the action so the test can inspect it.
captured_resolved_path: str | None = None
async def get_current_page_url(self) -> str:
return 'about:blank'
async def get_selector_map(self) -> dict:
# Returning an empty map causes upload_file to bail with an
# "Element ... does not exist" error AFTER it has resolved the path.
return {}
@pytest.fixture
def stub_session() -> _StubBrowserSession:
return _StubBrowserSession()
async def test_traversal_in_agent_path_does_not_escape_filesystem_dir(
tmp_path,
stub_session: _StubBrowserSession,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""GHSA-j9hj-92j8-jv9h: agent-controlled `../note.md` must not cause
the upload path to escape `file_system.get_dir()`."""
fs = FileSystem(base_dir=tmp_path)
# Legitimate FileSystem-owned file.
await fs.write_file('note.md', 'safe content')
data_dir_real = os.path.realpath(str(fs.get_dir()))
# Capture every path that the action checks for existence — the first one
# is the resolved upload path. Pre-fix this is the escape; post-fix it must
# be the inside path.
exists_calls: list[str] = []
real_exists = os.path.exists
def capturing_exists(path: str) -> bool:
exists_calls.append(str(path))
return real_exists(path)
monkeypatch.setattr('browser_use.tools.service.os.path.exists', capturing_exists)
tools = Tools()
result = await tools.registry.execute_action(
'upload_file',
{'index': 0, 'path': '../note.md'},
browser_session=stub_session, # type: ignore[arg-type]
file_system=fs,
available_file_paths=[],
)
# Whatever the eventual ActionResult, the first os.path.exists call inside
# upload_file is on the resolved upload path. It MUST be contained in data_dir.
assert exists_calls, 'upload_file should call os.path.exists on the resolved path'
resolved = os.path.realpath(exists_calls[0])
assert resolved == data_dir_real or resolved.startswith(data_dir_real + os.sep), (
f'Upload path escaped FileSystem directory.\n'
f' data_dir : {data_dir_real}\n'
f' resolved : {resolved}\n'
f' raw : {exists_calls[0]}'
)
# Sanity: the resolved path should point at the FileSystem-owned file.
assert resolved == os.path.realpath(str(fs.get_dir() / 'note.md'))
# The action itself will fail with an "Element does not exist" error from
# the stub selector map — that's fine; we're only checking the path.
assert isinstance(result, ActionResult)
async def test_traversal_with_no_basename_match_still_fails_safely(
tmp_path,
stub_session: _StubBrowserSession,
) -> None:
"""Sanity: if the basename does not match any FileSystem-owned file, the
action must reject with an availability error — never reach the join sink."""
fs = FileSystem(base_dir=tmp_path)
# Note: no `note.md` registered in the FileSystem.
tools = Tools()
result = await tools.registry.execute_action(
'upload_file',
{'index': 0, 'path': '../note.md'},
browser_session=stub_session, # type: ignore[arg-type]
file_system=fs,
available_file_paths=[],
)
assert isinstance(result, ActionResult)
assert result.error is not None
# Should be the "not available" rejection, not a path-resolution success.
assert 'not available' in result.error.lower() or 'does not exist' in result.error.lower()
class _StubRemoteBrowserSession(_StubBrowserSession):
"""Stub for a remote (non-local) browser session — the upload action's
rules differ here. Remote paths are passed through to the browser process."""
is_local = False
async def test_remote_session_does_not_rewrite_to_local_filesystem_on_basename_collision(
tmp_path,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""For remote sessions, an absolute remote path that happens to share a
basename with a local FileSystem-managed file MUST be passed through
unchanged. Rewriting to the local FileSystem path would silently upload
the wrong file (the local managed one) instead of the remote-machine file
the agent referenced.
Pre-existing issue surfaced by the GHSA-j9hj-92j8-jv9h review (codex bot).
"""
fs = FileSystem(base_dir=tmp_path)
# Create a local managed file with a basename that collides with the
# remote path the agent will reference.
await fs.write_file('note.md', 'LOCAL content the agent did NOT ask for')
# What the remote-session agent would pass — a path valid on the remote
# machine, NOT in the local FileSystem.
remote_path = '/tmp/note.md'
# Capture every path that the action checks for existence. For remote
# sessions the local-existence check is skipped, so we instead capture
# the resolved path via the same os.path.exists hook used by the local
# test — if the rewrite-to-local-filesystem branch fires, the next call
# will be against the data_dir path; if not, no call should reference it.
exists_calls: list[str] = []
real_exists = os.path.exists
def capturing_exists(path: str) -> bool:
exists_calls.append(str(path))
return real_exists(path)
monkeypatch.setattr('browser_use.tools.service.os.path.exists', capturing_exists)
tools = Tools()
await tools.registry.execute_action(
'upload_file',
{'index': 0, 'path': remote_path},
browser_session=_StubRemoteBrowserSession(), # type: ignore[arg-type]
file_system=fs,
available_file_paths=[],
)
# The action should NOT have rewritten to a local FileSystem path. If it
# had, we'd see the data_dir/note.md path appear in the captured calls.
data_dir_str = str(fs.get_dir())
rewrites = [p for p in exists_calls if p.startswith(data_dir_str)]
assert not rewrites, (
f'Remote session upload silently rewrote to local FileSystem path: {rewrites}. '
f'The agent intended to upload {remote_path!r}; the rewrite would have uploaded the local managed file instead.'
)