mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-11-06 21:22:23 +01:00
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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
9d0d3720f4
commit
1000b6582c
@@ -7,6 +7,7 @@ from dataclasses import dataclass, replace
|
|||||||
from functools import wraps
|
from functools import wraps
|
||||||
from inspect import signature
|
from inspect import signature
|
||||||
import json
|
import json
|
||||||
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
import re
|
import re
|
||||||
from uuid import uuid4
|
from uuid import uuid4
|
||||||
@@ -140,11 +141,20 @@ class RequestAuthority:
|
|||||||
raise ValueError("Malformed request authority")
|
raise ValueError("Malformed request authority")
|
||||||
if self.resource_roots is None:
|
if self.resource_roots is None:
|
||||||
roots = ()
|
roots = ()
|
||||||
if self.workspace:
|
# With no workspace, bash/python run in the agent workspace
|
||||||
try:
|
# (agent_cwd), so the file tools get that same root (#6679).
|
||||||
roots = (FilesystemRoot.seal(self.workspace, owner=self.owner),)
|
# Imported here: authority -> execution is a new edge, and
|
||||||
except (OSError, ValueError, RuntimeError):
|
# tool_execution imports this module at load time.
|
||||||
pass # An unresolved workspace grants no filesystem root.
|
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)
|
object.__setattr__(self, "resource_roots", roots)
|
||||||
if (not isinstance(self.resource_roots, tuple)
|
if (not isinstance(self.resource_roots, tuple)
|
||||||
or any(not isinstance(r, FilesystemRoot) or (r.owner and r.owner != self.owner)
|
or any(not isinstance(r, FilesystemRoot) or (r.owner and r.owner != self.owner)
|
||||||
|
|||||||
@@ -1068,6 +1068,10 @@ def _resolve_tool_path(raw_path: str) -> str:
|
|||||||
if raw_path is None or not str(raw_path).strip():
|
if raw_path is None or not str(raw_path).strip():
|
||||||
raise ValueError("path is required")
|
raise ValueError("path is required")
|
||||||
expanded = os.path.expanduser(str(raw_path).strip())
|
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)
|
resolved = os.path.realpath(expanded)
|
||||||
|
|
||||||
if _is_app_state_path(resolved):
|
if _is_app_state_path(resolved):
|
||||||
|
|||||||
@@ -123,12 +123,88 @@ def test_blocks_listing_the_data_directory_itself():
|
|||||||
_resolve_tool_path(DATA_DIR)
|
_resolve_tool_path(DATA_DIR)
|
||||||
|
|
||||||
|
|
||||||
def test_blocks_app_state_reached_by_relative_path(monkeypatch):
|
def test_blocks_app_state_reached_by_relative_path(tmp_path, monkeypatch):
|
||||||
"""The data directory is a relative hop from the checkout root, so
|
"""The data directory is a relative hop from the agent workspace, so
|
||||||
confinement cannot depend on the model supplying an absolute path."""
|
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"):
|
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):
|
def test_blocks_app_state_reached_through_a_symlink(tmp_path):
|
||||||
|
|||||||
@@ -281,7 +281,8 @@ async def test_browser_approval_application_binding_is_exact(producer, field, va
|
|||||||
original = authority()
|
original = authority()
|
||||||
content = '{"action":"session_info"}'
|
content = '{"action":"session_info"}'
|
||||||
approval = approval_for(original, "private_browser", content)
|
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)
|
_, result = await dispatch(changed, "private_browser", content, approval)
|
||||||
assert result["exit_code"] == 1 and not approval._claimed
|
assert result["exit_code"] == 1 and not approval._claimed
|
||||||
|
|
||||||
|
|||||||
@@ -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 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.
|
# agent workspace there (agent_cwd), so that is where its approval seals.
|
||||||
authority = _authority("", "get_workspace")
|
authority = _authority("", "get_workspace")
|
||||||
assert authority.resource_roots == ()
|
|
||||||
|
|
||||||
pending, _ = _approve(authority, "bash", "pwd")
|
pending, _ = _approve(authority, "bash", "pwd")
|
||||||
|
|
||||||
|
|||||||
@@ -117,7 +117,8 @@ async def test_remote_approval_is_bound_to_operation_and_request(manager, change
|
|||||||
content = '{"record":"two"}'
|
content = '{"record":"two"}'
|
||||||
else:
|
else:
|
||||||
authority = replace(authority, **{"request": {"request_id": "other"}, "owner": {"owner": "bob"},
|
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,
|
_, result = await dispatch(authority, tool, content, exact_approval=exact,
|
||||||
security_context=ToolRunSecurityContext(external_untrusted_context_seen=True))
|
security_context=ToolRunSecurityContext(external_untrusted_context_seen=True))
|
||||||
assert result["exit_code"] == 1
|
assert result["exit_code"] == 1
|
||||||
|
|||||||
@@ -255,7 +255,8 @@ async def test_read_file_dispatch_blocks_etc_shadow(monkeypatch):
|
|||||||
security_context=NO_TOOL_SECURITY_CONTEXT,
|
security_context=NO_TOOL_SECURITY_CONTEXT,
|
||||||
)
|
)
|
||||||
assert result.get("failure_kind") == "resource_identity_denied"
|
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
|
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,
|
security_context=NO_TOOL_SECURITY_CONTEXT,
|
||||||
)
|
)
|
||||||
assert result.get("failure_kind") == "resource_identity_denied"
|
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
|
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,
|
security_context=NO_TOOL_SECURITY_CONTEXT,
|
||||||
)
|
)
|
||||||
assert result.get("failure_kind") == "resource_identity_denied"
|
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
|
assert result.get("exit_code") == 1
|
||||||
@pytest.mark.parametrize("filename", ["auth.json", "app.db", "settings.json"])
|
@pytest.mark.parametrize("filename", ["auth.json", "app.db", "settings.json"])
|
||||||
def test_application_secrets_are_sensitive_paths(filename):
|
def test_application_secrets_are_sensitive_paths(filename):
|
||||||
|
|||||||
Reference in New Issue
Block a user