Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions tests/tools/test_cross_profile_guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,23 @@ def test_v4a_patch_extracts_path_for_guard(self, fake_hermes):
assert "cross-profile" in result["error"].lower()
assert target.read_text() == original

def test_v4a_move_file_extracts_destination_for_guard(self, fake_hermes):
"""Move File has a destination path that must not bypass guards."""
from tools.file_tools import patch_tool
source = "local-note.md"
target = fake_hermes["root"] / "skills" / "shared-skill" / "MOVED.md"
v4a = (
"*** Begin Patch\n"
f"*** Move File: {source} -> {target}\n"
"*** End Patch"
)
result_json = patch_tool(mode="patch", patch=v4a)
result = json.loads(result_json)
assert result.get("error"), f"V4A Move File destination must block: {result}"
lowered = result["error"].lower()
assert "cross-profile" in lowered or "sensitive" in lowered
assert not target.exists()


# ---------------------------------------------------------------------------
# skill_manage — error message naming other profile (item D)
Expand Down
48 changes: 48 additions & 0 deletions tests/tools/test_file_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,54 @@ def test_patch_v4a_rejects_traversal_in_add_header(self, mock_get):
assert "error" in result
assert "traversal" in result["error"].lower()

@patch("tools.file_tools._get_file_ops")
def test_patch_v4a_rejects_traversal_in_move_header(self, mock_get):
"""Move File headers have two paths; both must pass V4A guards."""
from tools.file_tools import patch_tool
result = json.loads(patch_tool(
mode="patch",
patch=(
"*** Begin Patch\n"
"*** Move File: safe.py -> ../../../tmp/escaped.py\n"
"*** End Patch\n"
),
))
assert "error" in result
assert "traversal" in result["error"].lower()
mock_get.return_value.patch_v4a.assert_not_called()

@patch("tools.file_tools._get_file_ops")
def test_patch_v4a_guards_parser_accepted_no_space_header(self, mock_get):
"""Guard extraction must accept the same header spacing as parser."""
from tools.file_tools import patch_tool
result = json.loads(patch_tool(
mode="patch",
patch=(
"*** Begin Patch\n"
"***Move File: safe.py -> ../../../tmp/escaped.py\n"
"*** End Patch\n"
),
))
assert "error" in result
assert "traversal" in result["error"].lower()
mock_get.return_value.patch_v4a.assert_not_called()

@patch("tools.file_tools._get_file_ops")
def test_patch_v4a_rejects_traversal_in_move_source(self, mock_get):
"""Move File source paths are also destructive and must be guarded."""
from tools.file_tools import patch_tool
result = json.loads(patch_tool(
mode="patch",
patch=(
"*** Begin Patch\n"
"*** Move File: ../../../tmp/source.py -> safe.py\n"
"*** End Patch\n"
),
))
assert "error" in result
assert "traversal" in result["error"].lower()
mock_get.return_value.patch_v4a.assert_not_called()


class TestSearchHandler:
@patch("tools.file_tools._get_file_ops")
Expand Down
11 changes: 8 additions & 3 deletions tools/file_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1272,8 +1272,12 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
if mode == "patch" and patch:
import re as _re
from tools.path_security import has_traversal_component
for _m in _re.finditer(r'^\*\*\*\s+(?:Update|Add|Delete)\s+File:\s*(.+)$', patch, _re.MULTILINE):
v4a_path = _m.group(1).strip()
v4a_paths: list[str] = []
for _m in _re.finditer(r'^\*\*\*\s*(?:Update|Add|Delete)\s+File:\s*(.+)$', patch, _re.MULTILINE):
v4a_paths.append(_m.group(1).strip())
for _m in _re.finditer(r'^\*\*\*\s*Move\s+File:\s*(.+?)\s*->\s*(.+)$', patch, _re.MULTILINE):
v4a_paths.extend([_m.group(1).strip(), _m.group(2).strip()])
for v4a_path in v4a_paths:
# V4A path headers come from patch CONTENT, not the explicit
# ``path=`` arg — so they're more attacker-influenceable (skill
# content, web extract, prompt injection). Reject ``..`` traversal
Expand All @@ -1286,7 +1290,8 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
return tool_error(
f"V4A patch header contains '..' traversal: {v4a_path!r}. "
"Use the agent's cwd-relative path (no '..') or an absolute "
"path in '*** Update File:' / '*** Add File:' / '*** Delete File:' headers."
"path in '*** Update File:' / '*** Add File:' / '*** Delete File:' / "
"'*** Move File:' headers."
)
_paths_to_check.append(v4a_path)
for _p in _paths_to_check:
Expand Down