mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(file-safety): distinguish safe-root write denial from credential blocks
Return actionable errors when HERMES_WRITE_SAFE_ROOT blocks a path instead of labeling every denial as a protected credential file. Wire the helper through write_file, patch, delete/move, and the Copilot ACP shim; sync docs examples.
This commit is contained in:
parent
1b5ceec2a0
commit
55d826ccef
9 changed files with 87 additions and 27 deletions
|
|
@ -26,7 +26,7 @@ from openai.types.chat.chat_completion_message_tool_call import (
|
|||
Function,
|
||||
)
|
||||
|
||||
from agent.file_safety import get_read_block_error, is_write_denied
|
||||
from agent.file_safety import get_read_block_error, get_write_denied_error
|
||||
from agent.redact import redact_sensitive_text
|
||||
from tools.environments.local import hermes_subprocess_env
|
||||
|
||||
|
|
@ -727,10 +727,9 @@ class CopilotACPClient:
|
|||
elif method == "fs/write_text_file":
|
||||
try:
|
||||
path = _ensure_path_within_cwd(str(params.get("path") or ""), cwd)
|
||||
if is_write_denied(str(path)):
|
||||
raise PermissionError(
|
||||
f"Write denied: '{path}' is a protected system/credential file."
|
||||
)
|
||||
denied = get_write_denied_error(str(path))
|
||||
if denied:
|
||||
raise PermissionError(denied)
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(str(params.get("content") or ""))
|
||||
response = {
|
||||
|
|
|
|||
|
|
@ -95,16 +95,16 @@ def get_safe_write_roots() -> set[str]:
|
|||
return roots
|
||||
|
||||
|
||||
def is_write_denied(path: str) -> bool:
|
||||
"""Return True if path is blocked by the write denylist or safe root."""
|
||||
def _classify_write_denial(path: str) -> Optional[str]:
|
||||
"""Return ``'credential'``, ``'safe_root'``, or ``None`` if writes are allowed."""
|
||||
home = os.path.realpath(os.path.expanduser("~"))
|
||||
resolved = os.path.realpath(os.path.expanduser(str(path)))
|
||||
|
||||
if resolved in build_write_denied_paths(home):
|
||||
return True
|
||||
return "credential"
|
||||
for prefix in build_write_denied_prefixes(home):
|
||||
if resolved.startswith(prefix):
|
||||
return True
|
||||
return "credential"
|
||||
|
||||
mcp_tokens_dir_name = "mcp-tokens"
|
||||
|
||||
|
|
@ -121,13 +121,13 @@ def is_write_denied(path: str) -> bool:
|
|||
try:
|
||||
mcp_real = os.path.realpath(os.path.join(base_real, mcp_tokens_dir_name))
|
||||
if resolved == mcp_real or resolved.startswith(mcp_real + os.sep):
|
||||
return True
|
||||
return "credential"
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
pairing_real = os.path.realpath(os.path.join(base_real, "pairing"))
|
||||
if resolved == pairing_real or resolved.startswith(pairing_real + os.sep):
|
||||
return True
|
||||
return "credential"
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
|
@ -139,9 +139,28 @@ def is_write_denied(path: str) -> bool:
|
|||
allowed = True
|
||||
break
|
||||
if not allowed:
|
||||
return True
|
||||
return "safe_root"
|
||||
|
||||
return False
|
||||
return None
|
||||
|
||||
|
||||
def is_write_denied(path: str) -> bool:
|
||||
"""Return True if path is blocked by the write denylist or safe root."""
|
||||
return _classify_write_denial(path) is not None
|
||||
|
||||
|
||||
def get_write_denied_error(path: str, *, verb: str = "Write") -> Optional[str]:
|
||||
"""Return a user/model-facing error when writes to ``path`` are blocked."""
|
||||
denial = _classify_write_denial(path)
|
||||
if denial is None:
|
||||
return None
|
||||
if denial == "safe_root":
|
||||
roots_display = os.pathsep.join(sorted(get_safe_write_roots()))
|
||||
return (
|
||||
f"{verb} denied: '{path}' is outside HERMES_WRITE_SAFE_ROOT "
|
||||
f"({roots_display}). Unset the variable or add this path's directory prefix."
|
||||
)
|
||||
return f"{verb} denied: '{path}' is a protected system/credential file."
|
||||
|
||||
|
||||
# Common secret-bearing project-local environment file basenames.
|
||||
|
|
|
|||
|
|
@ -209,7 +209,11 @@ class CopilotACPClientSafetyTests(unittest.TestCase):
|
|||
target = home / ".ssh" / "id_rsa"
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
with patch("agent.copilot_acp_client.is_write_denied", return_value=True, create=True):
|
||||
with patch(
|
||||
"agent.copilot_acp_client.get_write_denied_error",
|
||||
return_value="Write denied: protected",
|
||||
create=True,
|
||||
):
|
||||
response = self._dispatch(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
|
|
@ -248,6 +252,7 @@ class CopilotACPClientSafetyTests(unittest.TestCase):
|
|||
)
|
||||
|
||||
self.assertIn("error", response)
|
||||
self.assertIn("HERMES_WRITE_SAFE_ROOT", str(response["error"]))
|
||||
self.assertFalse(outside.exists())
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -168,6 +168,38 @@ class TestMultipleSafeWriteRoots:
|
|||
assert _is_write_denied(str(inside)) is False
|
||||
|
||||
|
||||
class TestGetWriteDeniedError:
|
||||
"""get_write_denied_error() should distinguish credential vs safe-root blocks."""
|
||||
|
||||
def test_credential_path_message(self):
|
||||
from agent.file_safety import get_write_denied_error
|
||||
|
||||
err = get_write_denied_error(os.path.expanduser("~/.ssh/id_rsa"))
|
||||
assert err is not None
|
||||
assert "protected system/credential file" in err
|
||||
assert "HERMES_WRITE_SAFE_ROOT" not in err
|
||||
|
||||
def test_safe_root_message(self, tmp_path: Path, monkeypatch):
|
||||
from agent.file_safety import get_write_denied_error
|
||||
|
||||
safe_root = tmp_path / "workspace"
|
||||
outside = tmp_path / "outside.txt"
|
||||
os.makedirs(safe_root, exist_ok=True)
|
||||
|
||||
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", str(safe_root))
|
||||
err = get_write_denied_error(str(outside))
|
||||
assert err is not None
|
||||
assert "outside HERMES_WRITE_SAFE_ROOT" in err
|
||||
assert str(safe_root) in err
|
||||
assert "protected system/credential file" not in err
|
||||
|
||||
def test_allowed_path_returns_none(self, tmp_path: Path):
|
||||
from agent.file_safety import get_write_denied_error
|
||||
|
||||
target = tmp_path / "ok.txt"
|
||||
assert get_write_denied_error(str(target)) is None
|
||||
|
||||
|
||||
class TestCheckSensitivePathMacOSBypass:
|
||||
"""Verify _check_sensitive_path blocks /private/etc paths (issue #8734)."""
|
||||
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ from tools.binary_extensions import BINARY_EXTENSIONS
|
|||
from agent.file_safety import (
|
||||
build_write_denied_paths,
|
||||
build_write_denied_prefixes,
|
||||
get_write_denied_error,
|
||||
is_write_denied as _shared_is_write_denied,
|
||||
)
|
||||
|
||||
|
|
@ -1289,8 +1290,9 @@ class ShellFileOperations(FileOperations):
|
|||
|
||||
def _python_delete(self, path: str, recursive: bool) -> WriteResult:
|
||||
path = self._expand_path(path)
|
||||
if _is_write_denied(path):
|
||||
return WriteResult(error=f"Delete denied: {path} is a protected path")
|
||||
denied = get_write_denied_error(path, verb="Delete")
|
||||
if denied:
|
||||
return WriteResult(error=denied)
|
||||
|
||||
# We can't shell out to ``rm`` here — it doesn't exist on Windows
|
||||
# ``cmd.exe`` or PowerShell, so this code path is what's left when
|
||||
|
|
@ -1335,8 +1337,9 @@ class ShellFileOperations(FileOperations):
|
|||
src = self._expand_path(src)
|
||||
dst = self._expand_path(dst)
|
||||
for p in (src, dst):
|
||||
if _is_write_denied(p):
|
||||
return WriteResult(error=f"Move denied: {p} is a protected path")
|
||||
denied = get_write_denied_error(p, verb="Move")
|
||||
if denied:
|
||||
return WriteResult(error=denied)
|
||||
result = self._exec(
|
||||
f"mv {self._escape_shell_arg(src)} {self._escape_shell_arg(dst)}"
|
||||
)
|
||||
|
|
@ -1383,8 +1386,9 @@ class ShellFileOperations(FileOperations):
|
|||
path = self._expand_path(path)
|
||||
|
||||
# Block writes to sensitive paths
|
||||
if _is_write_denied(path):
|
||||
return WriteResult(error=f"Write denied: '{path}' is a protected system/credential file.")
|
||||
denied = get_write_denied_error(path)
|
||||
if denied:
|
||||
return WriteResult(error=denied)
|
||||
|
||||
# ── Fail-closed pre-write syntax gate ───────────────────────────
|
||||
# Validate the CANDIDATE content BEFORE any bytes touch disk —
|
||||
|
|
@ -1566,8 +1570,9 @@ class ShellFileOperations(FileOperations):
|
|||
path = self._expand_path(path)
|
||||
|
||||
# Block writes to sensitive paths
|
||||
if _is_write_denied(path):
|
||||
return PatchResult(error=f"Write denied: '{path}' is a protected system/credential file.")
|
||||
denied = get_write_denied_error(path)
|
||||
if denied:
|
||||
return PatchResult(error=denied)
|
||||
|
||||
# Read current content
|
||||
read_cmd = f"cat {self._escape_shell_arg(path)} 2>/dev/null"
|
||||
|
|
|
|||
|
|
@ -766,7 +766,7 @@ When this variable is set, `write_file` and `patch` may only target paths inside
|
|||
|
||||
The official Docker image sets `HERMES_WRITE_SAFE_ROOT=/opt/data` alongside `HERMES_HOME=/opt/data` so the agent cannot escape the mounted data volume.
|
||||
|
||||
**Do not add this to `~/.hermes/.env` unless you intend to sandbox writes.** A common mistake is pointing it at a project directory while expecting the agent to edit `~/.hermes/cron/jobs.json`, `~/.hermes/skills/`, or scripts under a profile — those paths are outside the sandbox and every `write_file`/`patch` to them fails. The tool error currently reads `Write denied: '…' is a protected system/credential file.` for all blocked writes (including safe-root violations); check `echo $HERMES_WRITE_SAFE_ROOT` when you see that message.
|
||||
**Do not add this to `~/.hermes/.env` unless you intend to sandbox writes.** A common mistake is pointing it at a project directory while expecting the agent to edit `~/.hermes/cron/jobs.json`, `~/.hermes/skills/`, or scripts under a profile — those paths are outside the sandbox and every `write_file`/`patch` to them fails with an `outside HERMES_WRITE_SAFE_ROOT` error.
|
||||
|
||||
To allow both a workspace and Hermes state, list both prefixes (order does not matter):
|
||||
|
||||
|
|
|
|||
|
|
@ -1457,8 +1457,8 @@ Example footer when writes are blocked:
|
|||
|
||||
```
|
||||
⚠️ File-mutation verifier: 2 file(s) were NOT modified this turn despite any wording above that may suggest otherwise. Run `git status` or `read_file` to confirm.
|
||||
• ~/.hermes/cron/jobs.json — [patch] Write denied: '…' is a protected system/credential file.
|
||||
• ~/.hermes/scripts/monitor.py — [write_file] Write denied: '…' is a protected system/credential file.
|
||||
• ~/.hermes/cron/jobs.json — [patch] Write denied: '…' is outside HERMES_WRITE_SAFE_ROOT (/path/to/project)
|
||||
• ~/.hermes/scripts/monitor.py — [write_file] Write denied: '…' is outside HERMES_WRITE_SAFE_ROOT (/path/to/project)
|
||||
```
|
||||
|
||||
If writes to Hermes state (cron jobs, skills, scripts under `~/.hermes/`) are failing, check whether `HERMES_WRITE_SAFE_ROOT` is set in your environment. For cron changes, use the `cronjob` tool or `hermes cron edit` instead of patching `jobs.json` directly.
|
||||
|
|
|
|||
|
|
@ -249,7 +249,7 @@ These categories are always denied, even when `HERMES_WRITE_SAFE_ROOT` is unset:
|
|||
|
||||
Sensitive paths inside the safe root are still blocked — pointing `HERMES_WRITE_SAFE_ROOT` at `$HOME` does not allow writing `~/.ssh/id_rsa`.
|
||||
|
||||
The tool error message is always `Write denied: '…' is a protected system/credential file.` even when the actual reason is the safe-root sandbox below. If the path does not look like a credential file, check `echo $HERMES_WRITE_SAFE_ROOT`.
|
||||
Safe-root violations return `Write denied: '…' is outside HERMES_WRITE_SAFE_ROOT (…)`. Credential-path blocks use `Write denied: '…' is a protected system/credential file.`
|
||||
|
||||
### HERMES_WRITE_SAFE_ROOT (optional sandbox)
|
||||
|
||||
|
|
|
|||
|
|
@ -580,7 +580,7 @@ Graph 事件(Teams 会议、日历、聊天等)的入站变更通知监听
|
|||
|
||||
官方 Docker 镜像会设置 `HERMES_WRITE_SAFE_ROOT=/opt/data` 与 `HERMES_HOME=/opt/data`,防止 agent 逃出挂载的数据卷。
|
||||
|
||||
**除非有意沙箱化写入,否则不要将此变量加入 `~/.hermes/.env`。** 常见错误是将其指向项目目录,却期望 agent 编辑 `~/.hermes/cron/jobs.json`、`~/.hermes/skills/` 或 profile 下的脚本——这些路径在沙箱外,每次 `write_file`/`patch` 都会失败。当前所有被阻止的写入(包括 safe-root 违规)都显示 `Write denied: '…' is a protected system/credential file.`;看到此消息时请检查 `echo $HERMES_WRITE_SAFE_ROOT`。
|
||||
**除非有意沙箱化写入,否则不要将此变量加入 `~/.hermes/.env`。** 常见错误是将其指向项目目录,却期望 agent 编辑 `~/.hermes/cron/jobs.json`、`~/.hermes/skills/` 或 profile 下的脚本——这些路径在沙箱外,每次 `write_file`/`patch` 都会失败并返回 `outside HERMES_WRITE_SAFE_ROOT` 错误。
|
||||
|
||||
若需同时允许工作区和 Hermes 状态目录,列出两个前缀(顺序无关):
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue