fix: NT-namespace guard fires before every sibling resolve (checkpoint, ACP bridge, @file:)
Three paths still resolved the raw model/remote-supplied string before the guard could refuse it, so on Windows the NTLM-leak trigger (resolving the path) ran anyway: the file-checkpoint helper stats write_file/patch targets before the tool executes; the ACP file bridge resolves fs/read_text_file and fs/write_text_file paths before its read/write denylists; and @file:/@folder: references resolve their target before the reference allow-check. Each now checks the raw string first and refuses. The GLOBALROOT form now requires its path separator so a GLOBALROOT-prefixed local name is not misclassified. The rationale comment names the vector instead of another product's changelog, and the security docs say the row is enforced on reads as well as writes, since it sits under the write-guard table.
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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}",
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user