From eff97a8a0545615010cb050e7cd76ae258bd0d23 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:36:22 -0700 Subject: [PATCH] =?UTF-8?q?refactor(profiles):=20retire=20the=20cross-prof?= =?UTF-8?q?ile=20write=20guard=20=E2=80=94=20profiles=20are=20not=20isolat?= =?UTF-8?q?ed=20(maintainer=20decision);=20mirror=20lost-write=20guards=20?= =?UTF-8?q?(#32049)=20survive;=20patch/write=5Ffile=20schemas=20drop=20cro?= =?UTF-8?q?ss=5Fprofile=20(-83=20tok/call)=20(#97165)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- agent/file_safety.py | 32 ++------ agent/system_prompt.py | 4 +- tests/agent/test_file_safety_cross_profile.py | 34 +++------ tests/tools/test_cross_profile_guard.py | 52 ++++++------- tools/code_execution_tool.py | 4 +- tools/file_tools.py | 76 +++++++------------ tools/skill_manager_tool.py | 9 +-- 7 files changed, 77 insertions(+), 134 deletions(-) diff --git a/agent/file_safety.py b/agent/file_safety.py index fb469833dc..42ccda3332 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -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 # --------------------------------------------------------------------------- diff --git a/agent/system_prompt.py b/agent/system_prompt.py index fa833d72e3..ebf1e8a410 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -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() diff --git a/tests/agent/test_file_safety_cross_profile.py b/tests/agent/test_file_safety_cross_profile.py index 0a7ca104a4..d3a68b6fb5 100644 --- a/tests/agent/test_file_safety_cross_profile.py +++ b/tests/agent/test_file_safety_cross_profile.py @@ -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() diff --git a/tests/tools/test_cross_profile_guard.py b/tests/tools/test_cross_profile_guard.py index c28658a123..6f486b246f 100644 --- a/tests/tools/test_cross_profile_guard.py +++ b/tests/tools/test_cross_profile_guard.py @@ -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 diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index 92e1ffe811..44b5188c5d 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -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": ( diff --git a/tools/file_tools.py b/tools/file_tools.py index 3f3730149f..da3f1bf5b3 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -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///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"], }, diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 7c375a9fd0..ba3159a9fc 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -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 `) 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."