From 5faea3f618fc54c4231fc62b02375226a6ccca96 Mon Sep 17 00:00:00 2001
From: waefrebeorn <32711803+waefrebeorn@users.noreply.github.com>
Date: Mon, 25 May 2026 01:55:21 -0700
Subject: [PATCH] fix(file_tools): reject '..' traversal in V4A patch headers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

V4A patch '*** Update File:', '*** Add File:', '*** Delete File:' headers
come from patch CONTENT, not the explicit `path=` argument. That makes
them attacker-influenceable through skill content, web extract output,
prompt injection, and other surfaces the agent processes. Headers like
'*** Update File: ../../../etc/shadow' would resolve relative to the
agent's cwd; in deployment configurations where that cwd is deep enough
to land outside Hermes' protected paths, the write could land somewhere
the agent operator did not intend.

Reject any V4A header containing a '..' path component before applying
the patch. The explicit `path=` argument on patch_tool is UNCHANGED —
the agent legitimately uses '..' there (e.g. `patch path='../other_module/x.py'`
from a worktree dir is normal cross-module editing).

Regression tests: V4A Update header with traversal rejected, V4A Add
header with traversal rejected, patch_v4a never invoked when rejection
fires.

Salvaged from PR #29395 by @waefrebeorn. The original PR added
has_traversal_component as a blanket reject on read_file_tool,
write_file_tool, patch_tool's explicit path, and search_tool — that
would break legitimate agent operation where '..' is normal. Also
dropped the over-eager skills_guard pattern additions
(pickle.loads/marshal.loads/ctypes.CDLL/importlib at high/critical
severity would false-positive on legit data-science and FFI skills).

Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
---
 tests/tools/test_file_tools.py | 39 ++++++++++++++++++++++++++++++++++
 tools/file_tools.py            | 18 +++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/tests/tools/test_file_tools.py b/tests/tools/test_file_tools.py
index a951ed25cb7..2ef8411094a 100644
--- a/tests/tools/test_file_tools.py
+++ b/tests/tools/test_file_tools.py
@@ -211,6 +211,45 @@ class TestPatchHandler:
         assert "error" in result
         assert "Unknown mode" in result["error"]
 
+    @patch("tools.file_tools._get_file_ops")
+    def test_patch_v4a_rejects_traversal_in_update_header(self, mock_get):
+        """V4A '*** Update File:' headers come from patch content, which can
+        carry prompt-injection-controlled paths (skill content, web extract).
+        ``..`` traversal in the header must be rejected before the patch is
+        applied, even though the explicit ``path=`` arg is allowed to use
+        ``..`` for legitimate cross-worktree edits."""
+        from tools.file_tools import patch_tool
+        result = json.loads(patch_tool(
+            mode="patch",
+            patch=(
+                "*** Begin Patch\n"
+                "*** Update File: ../../../etc/shadow\n"
+                "@@ -1,3 +1,3 @@\n"
+                "-old\n"
+                "+new\n"
+                "*** End Patch\n"
+            ),
+        ))
+        assert "error" in result
+        assert "traversal" in result["error"].lower()
+        # patch_v4a must not be invoked when the header is rejected
+        mock_get.return_value.patch_v4a.assert_not_called()
+
+    @patch("tools.file_tools._get_file_ops")
+    def test_patch_v4a_rejects_traversal_in_add_header(self, mock_get):
+        from tools.file_tools import patch_tool
+        result = json.loads(patch_tool(
+            mode="patch",
+            patch=(
+                "*** Begin Patch\n"
+                "*** Add File: ../../../tmp/dropped.py\n"
+                "+print('pwned')\n"
+                "*** End Patch\n"
+            ),
+        ))
+        assert "error" in result
+        assert "traversal" in result["error"].lower()
+
 
 class TestSearchHandler:
     @patch("tools.file_tools._get_file_ops")
diff --git a/tools/file_tools.py b/tools/file_tools.py
index a5be71a8bfe..9ff433ef939 100644
--- a/tools/file_tools.py
+++ b/tools/file_tools.py
@@ -910,8 +910,24 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
         _paths_to_check.append(path)
     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):
-            _paths_to_check.append(_m.group(1).strip())
+            v4a_path = _m.group(1).strip()
+            # 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
+            # in V4A headers: a legitimate multi-file patch from a single cwd
+            # can always emit absolute paths or paths relative to the agent's
+            # cwd without ``..``. The explicit ``path=`` arg is unchanged
+            # because the agent uses relative ``..`` paths legitimately
+            # (e.g. ``patch path="../other_module/x.py"`` from a worktree).
+            if has_traversal_component(v4a_path):
+                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."
+                )
+            _paths_to_check.append(v4a_path)
     for _p in _paths_to_check:
         sensitive_err = _check_sensitive_path(_p, task_id)
         if sensitive_err: