refactor(profiles): retire the cross-profile write guard — profiles are not isolated (maintainer decision); mirror lost-write guards (#32049) survive; patch/write_file schemas drop cross_profile (-83 tok/call) (#97165)
This commit is contained in:
+8
-24
@@ -559,32 +559,16 @@ def classify_cross_profile_target(path: str) -> Optional[dict]:
|
||||
|
||||
|
||||
def get_cross_profile_warning(path: str) -> Optional[str]:
|
||||
"""Return a model-facing warning string when ``path`` is cross-profile.
|
||||
"""RETIRED (maintainer decision): always returns ``None``.
|
||||
|
||||
Returns ``None`` when the write is in-scope (same profile) or outside
|
||||
Hermes entirely. Caller is expected to surface the warning to the
|
||||
agent as a tool-result error, NOT to silently allow the write — the
|
||||
agent must either get explicit user direction to proceed, or pass
|
||||
``cross_profile=True`` to its write tool.
|
||||
|
||||
This is defense-in-depth: the terminal tool runs as the same OS user
|
||||
and can write any of these paths without going through this guard.
|
||||
Treat the guard as a confusion-reducer, not a security boundary.
|
||||
The cross-profile write guard was removed — profiles were never
|
||||
isolated (same OS user; the terminal tool writes anywhere), so the
|
||||
block was ceremony that cost every schema real tokens and taught a
|
||||
bypass arg. The system prompt's active-profile hint remains the only
|
||||
steering; the classifier below survives for that hint and for
|
||||
diagnostics. Kept as a stub so external callers/plugins fail soft.
|
||||
"""
|
||||
info = classify_cross_profile_target(path)
|
||||
if info is None:
|
||||
return None
|
||||
return (
|
||||
f"Cross-profile write blocked by soft guard: {info['target_path']} "
|
||||
f"belongs to Hermes profile {info['target_profile']!r}, but the "
|
||||
f"agent is running under profile {info['active_profile']!r}. "
|
||||
f"Editing another profile's {info['area']}/ will affect that "
|
||||
f"profile's future sessions, not the one you are currently in. "
|
||||
f"Confirm with the user before proceeding. To bypass this guard "
|
||||
f"after explicit user direction, retry the call with "
|
||||
f"``cross_profile=True``. (Defense-in-depth — not a security "
|
||||
f"boundary; the terminal tool can still bypass.)"
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -718,9 +718,7 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None)
|
||||
f"{default_root}/cron/, {default_root}/memories/ — those belong to a "
|
||||
f"different session run from a different shell. Do NOT modify "
|
||||
f"another profile's skills/plugins/cron/memories unless the user "
|
||||
f"explicitly directs you to. The cross-profile write guard will "
|
||||
f"refuse such writes by default; pass cross_profile=True only "
|
||||
f"after explicit direction."
|
||||
f"explicitly directs you to."
|
||||
)
|
||||
|
||||
platform_key = (agent.platform or "").lower().strip()
|
||||
|
||||
@@ -151,34 +151,18 @@ class TestClassifyCrossProfileTarget:
|
||||
|
||||
|
||||
class TestGetCrossProfileWarning:
|
||||
"""The guard is RETIRED (maintainer decision): profiles are not
|
||||
isolated, so the warning helper is a permanent None stub — kept only
|
||||
so external callers fail soft. The classifier itself survives for
|
||||
the system-prompt hint and diagnostics."""
|
||||
|
||||
def test_in_profile_returns_none(self, fake_hermes, monkeypatch):
|
||||
_set_active_home(monkeypatch, fake_hermes["security_home"])
|
||||
from agent.file_safety import get_cross_profile_warning
|
||||
assert get_cross_profile_warning(
|
||||
str(fake_hermes["security_home"] / "skills" / "foo" / "SKILL.md")
|
||||
) is None
|
||||
str(fake_hermes["root"] / "skills" / "a" / "SKILL.md")) is None
|
||||
|
||||
def test_cross_profile_warning_names_both_profiles(self, fake_hermes, monkeypatch):
|
||||
_set_active_home(monkeypatch, fake_hermes["security_home"])
|
||||
def test_cross_profile_returns_none_guard_retired(self, fake_hermes, monkeypatch):
|
||||
from agent.file_safety import get_cross_profile_warning
|
||||
warn = get_cross_profile_warning(
|
||||
str(fake_hermes["default_home"] / "skills" / "foo" / "SKILL.md")
|
||||
)
|
||||
assert warn is not None
|
||||
# Must name BOTH profiles so the model knows which is which.
|
||||
assert "default" in warn
|
||||
assert "hermes-security" in warn
|
||||
# Must name the bypass kwarg.
|
||||
assert "cross_profile=True" in warn
|
||||
# Must reference the area.
|
||||
assert "skills" in warn
|
||||
target = fake_hermes["root"] / "profiles" / "security" / "skills" / "x" / "SKILL.md"
|
||||
assert get_cross_profile_warning(str(target)) is None
|
||||
|
||||
def test_warning_is_defense_in_depth_not_boundary(self, fake_hermes, monkeypatch):
|
||||
_set_active_home(monkeypatch, fake_hermes["security_home"])
|
||||
from agent.file_safety import get_cross_profile_warning
|
||||
warn = get_cross_profile_warning(
|
||||
str(fake_hermes["default_home"] / "skills" / "foo" / "SKILL.md")
|
||||
)
|
||||
# Must self-document as defense-in-depth so future reviewers
|
||||
# don't promote it to a hard block.
|
||||
assert "not a security boundary" in warn.lower()
|
||||
|
||||
@@ -67,20 +67,17 @@ class TestWriteFileCrossProfileGuard:
|
||||
assert target.exists()
|
||||
assert target.read_text() == "in-profile content"
|
||||
|
||||
def test_cross_profile_write_blocked_by_default(self, fake_hermes):
|
||||
"""The May 2026 incident — security-profile session edits default
|
||||
profile's skill. Must be blocked."""
|
||||
def test_cross_profile_write_allowed_guard_retired(self, fake_hermes):
|
||||
"""Guard RETIRED (maintainer decision): profiles are not isolated —
|
||||
the same OS user owns every profile dir and the terminal tool
|
||||
always could write them. Cross-profile writes now succeed; the
|
||||
system prompt's profile hint is the only steering."""
|
||||
from tools.file_tools import write_file_tool
|
||||
target = fake_hermes["root"] / "skills" / "shared-skill" / "SKILL.md"
|
||||
original = target.read_text()
|
||||
result_json = write_file_tool(str(target), "OVERWRITTEN")
|
||||
result_json = write_file_tool(str(target), "cross-profile write, allowed")
|
||||
result = json.loads(result_json)
|
||||
assert result.get("error"), "Cross-profile write should be refused"
|
||||
assert "cross-profile" in result["error"].lower()
|
||||
assert "default" in result["error"]
|
||||
assert "hermes-security" in result["error"]
|
||||
# File untouched.
|
||||
assert target.read_text() == original
|
||||
assert not result.get("error"), f"guard retired; write must succeed: {result}"
|
||||
assert target.read_text() == "cross-profile write, allowed"
|
||||
|
||||
|
||||
def test_non_hermes_path_unaffected(self, fake_hermes, tmp_path):
|
||||
@@ -99,20 +96,18 @@ class TestWriteFileCrossProfileGuard:
|
||||
|
||||
|
||||
class TestPatchCrossProfileGuard:
|
||||
def test_cross_profile_patch_blocked(self, fake_hermes):
|
||||
def test_cross_profile_patch_allowed_guard_retired(self, fake_hermes):
|
||||
from tools.file_tools import patch_tool
|
||||
target = fake_hermes["root"] / "skills" / "shared-skill" / "SKILL.md"
|
||||
original = target.read_text()
|
||||
result_json = patch_tool(
|
||||
mode="replace",
|
||||
path=str(target),
|
||||
old_string="default copy.",
|
||||
new_string="HIJACKED.",
|
||||
new_string="patched without any flag.",
|
||||
)
|
||||
result = json.loads(result_json)
|
||||
assert result.get("error")
|
||||
assert "cross-profile" in result["error"].lower()
|
||||
assert target.read_text() == original
|
||||
assert not result.get("error"), f"guard retired; patch must succeed: {result}"
|
||||
assert "patched without any flag." in target.read_text()
|
||||
|
||||
def test_cross_profile_patch_bypass(self, fake_hermes):
|
||||
from tools.file_tools import patch_tool
|
||||
@@ -125,28 +120,28 @@ class TestPatchCrossProfileGuard:
|
||||
cross_profile=True,
|
||||
)
|
||||
result = json.loads(result_json)
|
||||
assert not result.get("error"), f"cross_profile=True bypass: {result}"
|
||||
assert not result.get("error"), f"cross_profile still handler-accepted (compat): {result}"
|
||||
assert "user-directed update." in target.read_text()
|
||||
|
||||
def test_v4a_patch_extracts_path_for_guard(self, fake_hermes):
|
||||
"""V4A patches embed the target paths in the patch body, not in
|
||||
a ``path`` kwarg. The guard must still apply."""
|
||||
def test_v4a_patch_writes_through_guard_retired(self, fake_hermes):
|
||||
"""V4A patch to a cross-profile path succeeds (guard retired).
|
||||
V4A patches embed target paths in the patch body; path extraction
|
||||
for the surviving mirror guards still runs, but cross-profile
|
||||
targets are no longer refused."""
|
||||
from tools.file_tools import patch_tool
|
||||
target = fake_hermes["root"] / "skills" / "shared-skill" / "SKILL.md"
|
||||
original = target.read_text()
|
||||
v4a = (
|
||||
"*** Begin Patch\n"
|
||||
f"*** Update File: {target}\n"
|
||||
"@@\n"
|
||||
"-default copy.\n"
|
||||
"+HIJACKED.\n"
|
||||
"+v4a cross-profile write, allowed.\n"
|
||||
"*** End Patch"
|
||||
)
|
||||
result_json = patch_tool(mode="patch", patch=v4a)
|
||||
result = json.loads(result_json)
|
||||
assert result.get("error"), f"V4A cross-profile must block: {result}"
|
||||
assert "cross-profile" in result["error"].lower()
|
||||
assert target.read_text() == original
|
||||
assert not result.get("error"), f"guard retired; V4A must succeed: {result}"
|
||||
assert "v4a cross-profile write, allowed." in target.read_text()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -179,7 +174,8 @@ class TestSkillManageCrossProfileErrorUX:
|
||||
err = _skill_not_found_error("default-only-skill")
|
||||
assert "not found in active profile 'hermes-security'" in err
|
||||
assert "default" in err
|
||||
assert "cross_profile=True" in err
|
||||
assert "cross_profile" not in err # retired vocabulary
|
||||
assert "file tools / terminal" in err
|
||||
|
||||
|
||||
def test_genuinely_missing_skill_keeps_helpful_hint(
|
||||
@@ -226,7 +222,7 @@ class TestSystemPromptActiveProfile:
|
||||
from pathlib import Path
|
||||
src = Path("agent/system_prompt.py").read_text()
|
||||
assert "Active Hermes profile" in src
|
||||
assert "cross_profile=True" in src
|
||||
assert "cross_profile=True" not in src # guard retired
|
||||
assert "~/.hermes/profiles/" in src
|
||||
# Both branches present (default and named profile).
|
||||
assert "Active Hermes profile: default" in src
|
||||
|
||||
@@ -403,7 +403,7 @@ _TOOL_STUBS = {
|
||||
"write_file": (
|
||||
"write_file",
|
||||
"path: str, content: str, cross_profile: bool = False",
|
||||
'"""Write content to a file (always overwrites). Returns dict with status. cross_profile=True opts out of the cross-Hermes-profile soft guard."""',
|
||||
'"""Write content to a file (always overwrites). Returns dict with status."""',
|
||||
'{"path": path, "content": content, "cross_profile": cross_profile}',
|
||||
),
|
||||
"search_files": (
|
||||
@@ -415,7 +415,7 @@ _TOOL_STUBS = {
|
||||
"patch": (
|
||||
"patch",
|
||||
'path: str = None, old_string: str = None, new_string: str = None, replace_all: bool = False, mode: str = "replace", patch: str = None, cross_profile: bool = False',
|
||||
'"""Targeted find-and-replace (mode="replace") or V4A multi-file patches (mode="patch"). Returns dict with status. cross_profile=True opts out of the cross-Hermes-profile soft guard."""',
|
||||
'"""Targeted find-and-replace (mode="replace") or V4A multi-file patches (mode="patch"). Returns dict with status."""',
|
||||
'{"path": path, "old_string": old_string, "new_string": new_string, "replace_all": replace_all, "mode": mode, "patch": patch, "cross_profile": cross_profile}',
|
||||
),
|
||||
"terminal": (
|
||||
|
||||
+29
-47
@@ -1064,36 +1064,28 @@ def _get_container_mirror_prefix_for_task(task_id: str = "default") -> str | Non
|
||||
|
||||
|
||||
def _check_cross_profile_path(filepath: str, task_id: str = "default") -> str | None:
|
||||
"""Return a soft-guard warning when ``filepath`` lands in another Hermes
|
||||
profile's scoped area, a host-side sandbox-mirror of authoritative profile
|
||||
state, or the Docker container's sandbox mirror of Hermes state.
|
||||
"""Return a soft-guard warning when ``filepath`` lands on a host-side
|
||||
sandbox-mirror of authoritative profile state, or the Docker
|
||||
container's sandbox mirror of Hermes state.
|
||||
|
||||
Three detectors run in order:
|
||||
Two detectors (both #32049): these catch writes that would be
|
||||
SILENTLY LOST — the host Hermes process never reads the mirror, so
|
||||
the write succeeds but changes nothing. That is a lost-work guard,
|
||||
not profile isolation.
|
||||
|
||||
* cross-profile — writes that hit another profile's
|
||||
``skills/plugins/cron/memories`` directory.
|
||||
* sandbox-mirror (#32049) — writes that hit the
|
||||
``…/sandboxes/<backend>/<task>/home/.hermes/…`` mirror created by a
|
||||
non-local terminal backend (Docker, Daytona, etc.), where the host
|
||||
Hermes process never reads the mirror and the authoritative file is
|
||||
left untouched.
|
||||
* container-mirror (#32049 follow-up) — writes from inside a Docker
|
||||
container whose bind-mounted home strips the ``sandboxes/`` prefix, so
|
||||
the agent sees a plain ``/root/.hermes/…`` path.
|
||||
NOTE: the third detector this shared check used to run — the
|
||||
cross-PROFILE write guard (another profile's skills/plugins/cron/
|
||||
memories) — was removed by maintainer decision: profiles were never
|
||||
isolated (same OS user; terminal writes anywhere), so the guard was
|
||||
ceremony. The system prompt's profile hint remains the only
|
||||
steering. ``cross_profile=True`` still bypasses the mirror guards
|
||||
(name kept for replay/transcript compat).
|
||||
|
||||
Returns ``None`` when the write is in-scope or outside Hermes scope.
|
||||
All detectors are soft guards — the agent can override any by
|
||||
passing ``cross_profile=True`` to its write tool after explicit user
|
||||
direction. Defense-in-depth, NOT a security boundary — the terminal
|
||||
tool runs as the same OS user and can write any of these paths
|
||||
directly. See ``agent/file_safety.classify_cross_profile_target``,
|
||||
``classify_sandbox_mirror_target`` and ``classify_container_mirror_target``
|
||||
for the detection rules.
|
||||
"""
|
||||
try:
|
||||
from agent.file_safety import (
|
||||
get_container_mirror_warning,
|
||||
get_cross_profile_warning,
|
||||
get_sandbox_mirror_warning,
|
||||
)
|
||||
except Exception:
|
||||
@@ -1101,18 +1093,13 @@ def _check_cross_profile_path(filepath: str, task_id: str = "default") -> str |
|
||||
# plus the write_denied list still apply.
|
||||
return None
|
||||
|
||||
# Resolve via the task's cwd so a relative ``skills/foo/SKILL.md``
|
||||
# in a session that cd'd into ``~/.hermes/profiles/other/`` is
|
||||
# classified against the right base.
|
||||
# Resolve via the task's cwd so a relative path in a session that
|
||||
# cd'd elsewhere is classified against the right base.
|
||||
try:
|
||||
resolved = str(_resolve_path_for_task(filepath, task_id))
|
||||
except (OSError, ValueError):
|
||||
resolved = filepath
|
||||
|
||||
warning = get_cross_profile_warning(resolved)
|
||||
if warning is not None:
|
||||
return warning
|
||||
|
||||
warning = get_sandbox_mirror_warning(resolved)
|
||||
if warning is not None:
|
||||
return warning
|
||||
@@ -2236,11 +2223,10 @@ def write_file_tool(path: str, content: str, task_id: str = "default",
|
||||
session_id: str | None = None) -> str:
|
||||
"""Write content to a file.
|
||||
|
||||
``cross_profile`` opts out of the soft cross-Hermes-profile guard. The
|
||||
guard fires only on writes that land in another profile's
|
||||
skills/plugins/cron/memories directory; everything else is unaffected.
|
||||
Pass ``True`` after explicit user direction — same shape as ``force``
|
||||
on the terminal tool.
|
||||
``cross_profile`` bypasses the #32049 sandbox-mirror lost-write
|
||||
guards (writes the host process would never read). Unadvertised in
|
||||
the schema — the mirror rejection error teaches it. The cross-PROFILE
|
||||
guard this flag was named for is removed (profiles are not isolated).
|
||||
"""
|
||||
sensitive_err = _check_sensitive_path(path, task_id)
|
||||
if sensitive_err:
|
||||
@@ -2329,9 +2315,8 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None,
|
||||
session_id: str | None = None) -> str:
|
||||
"""Patch a file using replace mode or V4A patch format.
|
||||
|
||||
``cross_profile`` opts out of the soft cross-Hermes-profile guard for
|
||||
targets under another profile's skills/plugins/cron/memories
|
||||
directory. Same shape as ``write_file``'s flag.
|
||||
``cross_profile``: same semantics as ``write_file``'s flag (mirror-guard
|
||||
bypass only; unadvertised).
|
||||
"""
|
||||
# Check sensitive paths for both replace (explicit path) and V4A patch (extract paths)
|
||||
_paths_to_check = []
|
||||
@@ -2683,11 +2668,11 @@ WRITE_FILE_SCHEMA = {
|
||||
"properties": {
|
||||
"path": {"type": "string", "description": "Path to the file to write (will be created if it doesn't exist, overwritten if it does)"},
|
||||
"content": {"type": "string", "description": "Complete content to write to the file"},
|
||||
"cross_profile": {
|
||||
"type": "boolean",
|
||||
"description": "Opt out of the cross-profile soft guard. Defaults to false. Set true ONLY after explicit user direction to edit another Hermes profile's skills/plugins/cron/memories — by default these writes are blocked with a warning because they affect a different profile than the one this session is running under.",
|
||||
"default": False,
|
||||
},
|
||||
# NOTE: the handler still accepts `cross_profile` (bool) — it now
|
||||
# bypasses only the #32049 sandbox-mirror lost-write guards, whose
|
||||
# rejection error teaches it. Unadvertised: the cross-PROFILE
|
||||
# guard it was named for was removed (profiles are not isolated,
|
||||
# maintainer decision), and mirror hits are rare + self-teaching.
|
||||
},
|
||||
"required": ["path", "content"]
|
||||
}
|
||||
@@ -2734,11 +2719,8 @@ PATCH_SCHEMA = {
|
||||
"type": "string",
|
||||
"description": "REQUIRED when mode='patch'. V4A format patch content. Format:\n*** Begin Patch\n*** Update File: path/to/file\n@@ context hint @@\n context line\n-removed line\n+added line\n*** End Patch",
|
||||
},
|
||||
"cross_profile": {
|
||||
"type": "boolean",
|
||||
"description": "Opt out of the cross-profile soft guard. Defaults to false. Set true ONLY after explicit user direction to edit another Hermes profile's skills/plugins/cron/memories.",
|
||||
"default": False,
|
||||
},
|
||||
# NOTE: handler still accepts `cross_profile` — see write_file's
|
||||
# NOTE (mirror-guard bypass only; unadvertised by design).
|
||||
},
|
||||
"required": ["mode"],
|
||||
},
|
||||
|
||||
@@ -855,17 +855,16 @@ def _skill_not_found_error(name: str, suffix: str = "") -> str:
|
||||
other_profile, other_path = others[0]
|
||||
base += (
|
||||
f" A skill by that name exists in profile "
|
||||
f"'{other_profile}' ({other_path}). To edit a skill in "
|
||||
f"another profile, switch profiles (`hermes -p "
|
||||
f"{other_profile}`) or operate via explicit file tools "
|
||||
f"with ``cross_profile=True``."
|
||||
f"'{other_profile}' ({other_path}). To edit it, switch "
|
||||
f"profiles (`hermes -p {other_profile}`) or edit the file "
|
||||
f"directly (file tools / terminal)."
|
||||
)
|
||||
else:
|
||||
names = ", ".join(f"'{p}'" for p, _ in others)
|
||||
base += (
|
||||
f" Skills by that name exist in other profiles: {names}. "
|
||||
f"Switch profiles (`hermes -p <name>`) to edit there, or "
|
||||
f"operate via explicit file tools with ``cross_profile=True``."
|
||||
f"edit the files directly (file tools / terminal)."
|
||||
)
|
||||
else:
|
||||
base += " Use skills_list() to see available skills."
|
||||
|
||||
Reference in New Issue
Block a user