From 1000b6582c747b0542ab71606d1de66c02953477 Mon Sep 17 00:00:00 2001 From: Grzegorz Date: Mon, 26 Oct 2026 16:46:52 +0100 Subject: [PATCH] fix(agent): give file tools the agent workspace as root when no workspace is set (#6722) * fix(agent): give file tools the agent workspace as root when no workspace is set (#6679) With no workspace bound, bash/python run in data/agent_workspace, but RequestAuthority sealed no filesystem root, so every native file tool failed with "requires a sealed resource root", and an approved write_file failed with "no sealed resource identity". Seal the agent workspace instead, and resolve relative paths in _resolve_tool_path against it rather than the server's cwd. Co-Authored-By: Claude Opus 5.5 * test: re-seal resource roots when a test swaps the authority owner (#6679) A no-workspace authority now seals the agent workspace for its owner, so replace(authority, owner="bob") copied alice's root and was rejected before the approval-binding check the tests are about. Co-Authored-By: Claude Opus 5.5 * fix(agent): only seal a vetted agent workspace as the file-tool root (#6679) Apply the agent_cwd() check before sealing data/agent_workspace: it must be a real directory in the readable data subdirs, otherwise no root is sealed. A symlinked workspace no longer redirects the file tools or the bash/python launch scope. Tighten the dispatch test assertions to the exact 'outside the sealed roots' message. * refactor(agent): vet the no-workspace file root with default_agent_workspace (#6679) --------- Co-authored-by: Claude Opus 5.5 --- src/agent_runtime/authority.py | 20 +++-- src/tool_execution.py | 4 + tests/test_agent_state_dir_confinement.py | 84 ++++++++++++++++++- tests/test_browser_resource_identity.py | 3 +- .../test_exact_approval_ungranted_process.py | 1 - tests/test_remote_resource_identity.py | 3 +- tests/test_tool_path_confinement.py | 9 +- 7 files changed, 109 insertions(+), 15 deletions(-) diff --git a/src/agent_runtime/authority.py b/src/agent_runtime/authority.py index cd5197300..541fac52f 100644 --- a/src/agent_runtime/authority.py +++ b/src/agent_runtime/authority.py @@ -7,6 +7,7 @@ from dataclasses import dataclass, replace from functools import wraps from inspect import signature import json +import os from pathlib import Path import re from uuid import uuid4 @@ -140,11 +141,20 @@ class RequestAuthority: raise ValueError("Malformed request authority") if self.resource_roots is None: roots = () - if self.workspace: - try: - roots = (FilesystemRoot.seal(self.workspace, owner=self.owner),) - except (OSError, ValueError, RuntimeError): - pass # An unresolved workspace grants no filesystem root. + # With no workspace, bash/python run in the agent workspace + # (agent_cwd), so the file tools get that same root (#6679). + # Imported here: authority -> execution is a new edge, and + # tool_execution imports this module at load time. + from src import tool_execution + try: + root = self.workspace + if not root: + # Vetted like agent_cwd(): a symlinked agent workspace + # raises and so grants no root. + root = tool_execution.default_agent_workspace() + roots = (FilesystemRoot.seal(root, owner=self.owner),) + except (OSError, ValueError, RuntimeError): + pass # An unresolved workspace grants no filesystem root. object.__setattr__(self, "resource_roots", roots) if (not isinstance(self.resource_roots, tuple) or any(not isinstance(r, FilesystemRoot) or (r.owner and r.owner != self.owner) diff --git a/src/tool_execution.py b/src/tool_execution.py index bde68df30..bc489b878 100644 --- a/src/tool_execution.py +++ b/src/tool_execution.py @@ -1068,6 +1068,10 @@ def _resolve_tool_path(raw_path: str) -> str: if raw_path is None or not str(raw_path).strip(): raise ValueError("path is required") expanded = os.path.expanduser(str(raw_path).strip()) + # Relative paths resolve where bash/python run (agent_cwd), not against + # the server's cwd, which is outside the allowed roots (#6679). + if not os.path.isabs(expanded): + expanded = os.path.join(_AGENT_WORKDIR, expanded) resolved = os.path.realpath(expanded) if _is_app_state_path(resolved): diff --git a/tests/test_agent_state_dir_confinement.py b/tests/test_agent_state_dir_confinement.py index 92d29206d..fc5d7ef14 100644 --- a/tests/test_agent_state_dir_confinement.py +++ b/tests/test_agent_state_dir_confinement.py @@ -123,12 +123,88 @@ def test_blocks_listing_the_data_directory_itself(): _resolve_tool_path(DATA_DIR) -def test_blocks_app_state_reached_by_relative_path(monkeypatch): - """The data directory is a relative hop from the checkout root, so +def test_blocks_app_state_reached_by_relative_path(tmp_path, monkeypatch): + """The data directory is a relative hop from the agent workspace, so confinement cannot depend on the model supplying an absolute path.""" - monkeypatch.chdir(os.path.dirname(DATA_DIR)) + current_execution = importlib.import_module("src.tool_execution") + data_dir = tmp_path / "data" + data_dir.mkdir() + workspace = _configure_test_data_tree(monkeypatch, data_dir)["AGENT_WORKSPACE_DIR"] + workspace.mkdir() + monkeypatch.setattr(current_execution, "_AGENT_WORKDIR", str(workspace)) with pytest.raises(ValueError, match="application state"): - _resolve_tool_path(os.path.join(os.path.basename(DATA_DIR), "sessions.json")) + current_execution._resolve_tool_path(os.path.join("..", "sessions.json")) + + +def test_relative_path_resolves_in_agent_workspace_not_server_cwd(tmp_path, monkeypatch): + """With no workspace bound, bash runs in the agent workspace, so a + relative write_file/read_file path must land there too (#6679), not in + the server's cwd, which is outside the allowed roots.""" + current_execution = importlib.import_module("src.tool_execution") + data_dir = tmp_path / "data" + data_dir.mkdir() + workspace = _configure_test_data_tree(monkeypatch, data_dir)["AGENT_WORKSPACE_DIR"] + workspace.mkdir() + monkeypatch.setattr(current_execution, "_AGENT_WORKDIR", str(workspace)) + server_cwd = tmp_path / "checkout" + server_cwd.mkdir() + monkeypatch.chdir(server_cwd) + token = current_execution._active_workspace.set(None) + try: + assert current_execution._resolve_tool_path("test.txt") == os.path.join( + os.path.realpath(workspace), "test.txt" + ) + assert current_execution._resolve_search_root("sub") == os.path.join( + os.path.realpath(workspace), "sub" + ) + finally: + current_execution._active_workspace.reset(token) + + +def test_request_without_workspace_seals_the_agent_workspace(tmp_path, monkeypatch): + """With no workspace, the native file tools get the folder bash/python + already run in (#6679) instead of no root at all, and a relative path + lands there while app state next to it stays out of reach.""" + from src.agent_runtime.authority import ExactOperation, OperationGrant, RequestAuthority + from src.agent_runtime.resource_binding import resolve_filesystem_operation + current_execution = importlib.import_module("src.tool_execution") + data_dir = tmp_path / "data" + data_dir.mkdir() + workspace = _configure_test_data_tree(monkeypatch, data_dir)["AGENT_WORKSPACE_DIR"] + workspace.mkdir() + (data_dir / "sessions.json").write_text("{}", encoding="utf-8") + monkeypatch.setattr(current_execution, "_AGENT_WORKDIR", str(workspace)) + + authority = RequestAuthority("request", "alice", "s", "", (OperationGrant("write_file"),)) + assert [r.path for r in authority.resource_roots] == [os.path.realpath(workspace)] + + bound = resolve_filesystem_operation( + ExactOperation.normalize("write_file", "test.txt\nhello"), roots=authority.resource_roots) + assert bound.bindings[0].resource.path == os.path.join(os.path.realpath(workspace), "test.txt") + with pytest.raises(ValueError, match="outside the sealed roots"): + resolve_filesystem_operation( + ExactOperation.normalize("read_file", "../sessions.json"), roots=authority.resource_roots) + + +def test_symlinked_agent_workspace_is_not_sealed(tmp_path, monkeypatch): + """A symlinked agent workspace must not become the file-tool root: + agent_cwd() and create_directories() refuse the same layout.""" + from src.agent_runtime.authority import OperationGrant, RequestAuthority + current_execution = importlib.import_module("src.tool_execution") + data_dir = tmp_path / "data" + data_dir.mkdir() + workspace = _configure_test_data_tree(monkeypatch, data_dir)["AGENT_WORKSPACE_DIR"] + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + (elsewhere / "notes.txt").write_text("private", encoding="utf-8") + try: + workspace.symlink_to(elsewhere, target_is_directory=True) + except OSError: + pytest.skip("cannot create symlink") + monkeypatch.setattr(current_execution, "_AGENT_WORKDIR", str(workspace)) + + authority = RequestAuthority("request", "alice", "s", "", (OperationGrant("read_file"),)) + assert authority.resource_roots == () def test_blocks_app_state_reached_through_a_symlink(tmp_path): diff --git a/tests/test_browser_resource_identity.py b/tests/test_browser_resource_identity.py index f17452707..e4f11aa37 100644 --- a/tests/test_browser_resource_identity.py +++ b/tests/test_browser_resource_identity.py @@ -281,7 +281,8 @@ async def test_browser_approval_application_binding_is_exact(producer, field, va original = authority() content = '{"action":"session_info"}' approval = approval_for(original, "private_browser", content) - changed = replace(original, **{field: value}, browser_sessions=(), browser_pages=()) + # resource_roots=None re-seals the root for the changed owner. + changed = replace(original, **{field: value}, browser_sessions=(), browser_pages=(), resource_roots=None) _, result = await dispatch(changed, "private_browser", content, approval) assert result["exit_code"] == 1 and not approval._claimed diff --git a/tests/test_exact_approval_ungranted_process.py b/tests/test_exact_approval_ungranted_process.py index 15064178f..af730af7e 100644 --- a/tests/test_exact_approval_ungranted_process.py +++ b/tests/test_exact_approval_ungranted_process.py @@ -93,7 +93,6 @@ def test_approval_without_a_workspace_seals_a_launch_in_the_agent_workspace(agen # Agent mode with no folder picked is the default. bash already runs in the # agent workspace there (agent_cwd), so that is where its approval seals. authority = _authority("", "get_workspace") - assert authority.resource_roots == () pending, _ = _approve(authority, "bash", "pwd") diff --git a/tests/test_remote_resource_identity.py b/tests/test_remote_resource_identity.py index 885059ad5..a92bc7eb2 100644 --- a/tests/test_remote_resource_identity.py +++ b/tests/test_remote_resource_identity.py @@ -117,7 +117,8 @@ async def test_remote_approval_is_bound_to_operation_and_request(manager, change content = '{"record":"two"}' else: authority = replace(authority, **{"request": {"request_id": "other"}, "owner": {"owner": "bob"}, - "session": {"session_id": "other"}}[change]) + "session": {"session_id": "other"}}[change], + resource_roots=None) # re-seal the root for the changed owner _, result = await dispatch(authority, tool, content, exact_approval=exact, security_context=ToolRunSecurityContext(external_untrusted_context_seen=True)) assert result["exit_code"] == 1 diff --git a/tests/test_tool_path_confinement.py b/tests/test_tool_path_confinement.py index ad172c10a..4d9b2261a 100644 --- a/tests/test_tool_path_confinement.py +++ b/tests/test_tool_path_confinement.py @@ -255,7 +255,8 @@ async def test_read_file_dispatch_blocks_etc_shadow(monkeypatch): security_context=NO_TOOL_SECURITY_CONTEXT, ) assert result.get("failure_kind") == "resource_identity_denied" - assert "sealed resource root" in (result.get("error") or "") + assert "outside the sealed roots" in (result.get("error") or "") + assert result.get("blocked") is True assert result.get("exit_code") == 1 @@ -285,7 +286,8 @@ async def test_write_file_dispatch_blocks_authorized_keys(monkeypatch): security_context=NO_TOOL_SECURITY_CONTEXT, ) assert result.get("failure_kind") == "resource_identity_denied" - assert "sealed resource root" in (result.get("error") or "") + assert "outside the sealed roots" in (result.get("error") or "") + assert result.get("blocked") is True assert result.get("exit_code") == 1 @@ -349,7 +351,8 @@ async def test_write_file_dispatch_blocks_cron(monkeypatch): security_context=NO_TOOL_SECURITY_CONTEXT, ) assert result.get("failure_kind") == "resource_identity_denied" - assert "sealed resource root" in (result.get("error") or "") + assert "outside the sealed roots" in (result.get("error") or "") + assert result.get("blocked") is True assert result.get("exit_code") == 1 @pytest.mark.parametrize("filename", ["auth.json", "app.db", "settings.json"]) def test_application_secrets_are_sensitive_paths(filename):