diff --git a/agent/context_references.py b/agent/context_references.py index 5911ff7f84..a23504c8c0 100644 --- a/agent/context_references.py +++ b/agent/context_references.py @@ -340,6 +340,9 @@ def _is_under(path: Path, root: Path) -> bool: def _resolve_path(cwd: Path, target: str, *, allowed_root: Path | None = None) -> Path: + from agent.file_safety import is_nt_namespace_path + if is_nt_namespace_path(target): # raw-string check: resolving such a path is the NTLM-leak trigger + raise ValueError("path uses a Windows NT/device namespace prefix and cannot be attached") resolved = (cwd / Path(os.path.expanduser(target))).resolve() # `/` keeps an absolute target as-is if allowed_root is not None and not _is_under(resolved, allowed_root): raise ValueError("path is outside the allowed workspace") diff --git a/agent/copilot_acp_client.py b/agent/copilot_acp_client.py index 3df2f1477b..4c6641b775 100644 --- a/agent/copilot_acp_client.py +++ b/agent/copilot_acp_client.py @@ -27,7 +27,8 @@ from agent.acp_openai_bridge import ( extract_tool_calls_from_text as _extract_tool_calls_from_text, render_tool_bridge_sections as _render_tool_bridge_sections, ) -from agent.file_safety import get_read_block_error, get_write_denied_error, is_write_approval_required +from agent.file_safety import ( + get_nt_namespace_error, get_read_block_error, get_write_denied_error, is_write_approval_required) from agent.redact import redact_sensitive_text from tools.environments.local import hermes_subprocess_env @@ -217,7 +218,10 @@ def _render_message_content(content: Any) -> str: return str(content).strip() -def _ensure_path_within_cwd(path_text: str, cwd: str) -> Path: +def _ensure_path_within_cwd(path_text: str, cwd: str, *, verb: str) -> Path: + # Raw-string check BEFORE resolve(): resolving an NT-namespace path is the NTLM-leak trigger. + if nt_error := get_nt_namespace_error(path_text, verb=verb): + raise PermissionError(nt_error) if not Path(path_text).is_absolute(): raise PermissionError("ACP file-system paths must be absolute.") resolved, root = Path(path_text).resolve(), Path(cwd).resolve() @@ -237,7 +241,7 @@ def _effective_timeout(timeout: Any) -> float: def _fs_read_text_file(params: dict[str, Any], cwd: str) -> Any: - path = _ensure_path_within_cwd(str(params.get("path") or ""), cwd) + path = _ensure_path_within_cwd(str(params.get("path") or ""), cwd, verb="Read") if block_error := get_read_block_error(str(path)): raise PermissionError(block_error) try: @@ -252,7 +256,7 @@ def _fs_read_text_file(params: dict[str, Any], cwd: str) -> Any: def _fs_write_text_file(params: dict[str, Any], cwd: str) -> Any: - path = _ensure_path_within_cwd(str(params.get("path") or ""), cwd) + path = _ensure_path_within_cwd(str(params.get("path") or ""), cwd, verb="Write") if denied := get_write_denied_error(str(path)): raise PermissionError(denied) if is_write_approval_required(str(path)): # soft-gated for interactive tools; the ACP shim has no human channel → fail closed diff --git a/agent/file_safety.py b/agent/file_safety.py index 89453ef548..13d0912b61 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -77,10 +77,9 @@ def _home_and_resolved(path: str) -> tuple[str, str]: # --------------------------------------------------------------------------- # Windows NT-namespace path guard # -# Inspired by Claude Code v2.1.234 (Aug 2026), which made its pre-approval -# file accesses "reject Windows NT-namespace (`\\??\\`) paths, hardening the -# remaining pre-approval file accesses against the NTLM credential-leak -# vector." +# Pre-approval file accesses reject Windows NT-namespace (``\??\``) paths so +# the remaining unguarded path touches cannot be turned into an NTLM +# credential leak. # # The vector: on Windows, merely *resolving or touching* a path such as # ``\\??\\UNC\\attacker.example\\share\\x`` (or the ``\\\\?\\UNC\\`` / @@ -131,7 +130,7 @@ def is_nt_namespace_path(path: str) -> bool: if s.startswith("\\\\?\\"): rest = s[4:] upper = rest.upper() - if upper.startswith("UNC\\") or upper.startswith("GLOBALROOT"): + if upper.startswith("UNC\\") or upper.startswith("GLOBALROOT\\"): return True return False diff --git a/agent/tool_executor.py b/agent/tool_executor.py index fa73ec6b62..12ce22b175 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -80,8 +80,13 @@ def _ensure_file_checkpoint(agent, function_name: str, function_args: dict, effe file_path = function_args.get("path", "") if not file_path: return + from agent.file_safety import is_nt_namespace_path from tools.file_tools_paths import _resolve_path_for_task + # Resolving an NT-namespace path is itself the NTLM-leak trigger; leave the + # tool's raw-string guard to refuse it without a checkpoint stat. + if is_nt_namespace_path(file_path): + return resolved_path = _resolve_path_for_task(file_path, effective_task_id or "default") agent._checkpoint_mgr.ensure_checkpoint( agent._checkpoint_mgr.get_working_dir_for_path(str(resolved_path)), f"before {function_name}", diff --git a/tests/agent/test_nt_namespace_guard.py b/tests/agent/test_nt_namespace_guard.py index 1d53a2f640..93c688e211 100644 --- a/tests/agent/test_nt_namespace_guard.py +++ b/tests/agent/test_nt_namespace_guard.py @@ -9,7 +9,8 @@ rely on. So the guard must fire on the model-supplied string before import os import unittest -from unittest.mock import patch +from types import SimpleNamespace +from unittest.mock import MagicMock, patch from agent.file_safety import ( get_read_block_error, @@ -54,14 +55,21 @@ class TestNtNamespaceGuard(unittest.TestCase): self.assertNotIn("NT/device namespace", get_write_denied_error(p) or "", p) def test_file_tools_reject_raw_string_without_resolving(self): - """Every file-tool entry refuses BEFORE the task-base join / resolve — the - resolve is the leak, and on POSIX the join would hide the prefix.""" + """Every file-tool entry — and the sibling paths that touch a file path before + the tool runs (checkpoint helper, ACP file bridge, @file: references) — refuses + BEFORE the task-base join / resolve: the resolve is the leak, and on POSIX the + join would hide the prefix.""" + from pathlib import Path + + from agent import context_references, copilot_acp_client, tool_executor from tools.file_tools import patch_tool, read_file_tool, search_tool, write_file_tool bad = "\\??\\UNC\\attacker.example\\share\\x" with patch("agent.file_safety.Path") as fs_path, \ patch("tools.file_tools._resolve_path_for_task") as ft_resolve, \ patch("tools.file_tools_write_guards._resolve_path_for_task") as wg_resolve, \ + patch("tools.file_tools_paths._resolve_path_for_task") as ckpt_resolve, \ + patch.object(Path, "resolve", side_effect=AssertionError("must not resolve")), \ patch.object(os.path, "realpath", side_effect=AssertionError("must not realpath")): results = [ read_file_tool(bad), @@ -69,9 +77,18 @@ class TestNtNamespaceGuard(unittest.TestCase): patch_tool(mode="replace", path=bad, old_string="a", new_string="b"), search_tool("x", path=bad), ] + checkpoint_agent = SimpleNamespace(_checkpoint_mgr=MagicMock(enabled=True)) + tool_executor._ensure_file_checkpoint(checkpoint_agent, "write_file", {"path": bad}, "default") + checkpoint_agent._checkpoint_mgr.ensure_checkpoint.assert_not_called() + for fs_handler in (copilot_acp_client._fs_read_text_file, copilot_acp_client._fs_write_text_file): + with self.assertRaisesRegex(PermissionError, "NT/device namespace"): + fs_handler({"path": bad, "content": "x"}, "/tmp") + with self.assertRaisesRegex(ValueError, "NT/device namespace"): + context_references._resolve_path(Path("/tmp"), bad) fs_path.assert_not_called() ft_resolve.assert_not_called() wg_resolve.assert_not_called() + ckpt_resolve.assert_not_called() for r in results: self.assertIn("NT/device namespace", r) diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index 71f6a10d11..4c6d950a0d 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -371,7 +371,7 @@ Unset the variable to restore unrestricted writes (subject to the protected-path Do not ask the agent to `patch` `~/.hermes/cron/jobs.json` directly. Use the `cronjob` tool, [`hermes cron`](./features/cron.md), or `/cron` — they update the job store through the supported API. The same applies to other Hermes control files when write safety blocks direct edits. :::note Defense-in-depth, not a hard boundary -Write guards apply to `write_file` and `patch` only. The `terminal` tool runs as the same OS user and can still `cat` or overwrite denied paths via shell commands. The denylist reduces accidental damage and gives models a clear stop signal; it does not sandbox a hostile or compromised agent. +Write guards apply to `write_file` and `patch` only, with one exception: the Windows NT/device-namespace row is also enforced on reads — `read_file`, `search_files`, `@file:`/`@folder:` context references and the ACP file bridge all refuse those paths on the raw string, before anything resolves them. The `terminal` tool runs as the same OS user and can still `cat` or overwrite denied paths via shell commands. The denylist reduces accidental damage and gives models a clear stop signal; it does not sandbox a hostile or compromised agent. ::: ## User Authorization (Gateway)