From d3c167b64472ee35e52d5201014e940c965579ef Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 24 May 2026 00:38:17 -0700 Subject: [PATCH] fix(profiles): cross-profile soft guard on file-write tools + system-prompt hint (#31290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(profiles): cross-profile soft guard on file-write tools + system-prompt hint Adds a soft guard so an agent running under one Hermes profile cannot silently edit a different profile's skills/plugins/cron/memories. Three layers: A. agent/file_safety.classify_cross_profile_target Classifies a write target against the active HERMES_HOME. Returns a {active_profile, target_profile, area, target_path} dict when the path lands in another profile's scoped area. PROFILE_SCOPED_AREAS = (skills, plugins, cron, memories). get_cross_profile_warning() wraps it into a model-facing error string that names both profiles, names the area, and points at the cross_profile=True bypass. Defense-in-depth, NOT a security boundary — the terminal tool runs as the same OS user and can write any of these paths directly. The guard exists to prevent confused-agent corruption, not to stop a determined attacker. SECURITY.md §3.2 (terminal-bypass posture) still applies. Wired into tools/file_tools.write_file_tool and patch_tool with a cross_profile=False kwarg. WRITE_FILE_SCHEMA and PATCH_SCHEMA both advertise cross_profile so the model can pass it after explicit user direction. patch_tool extracts target paths from V4A patch bodies before checking (same shape as the existing sensitive-path check). skill_manage is already scoped to the active profile's SKILLS_DIR by construction, so no extra guard wiring is needed there. The D-side error message (below) still names other profiles when the skill exists elsewhere. B. agent/system_prompt One deterministic line near the environment-hints block names the active profile and tells the model not to modify another profile's skills/plugins/cron/memories without explicit direction. Profile name is stable for the lifetime of the AIAgent, so the line is prompt-cache-safe. D. tools/skill_manager_tool._skill_not_found_error Replaces the bare "Skill 'X' not found." with a message that: - names the active profile, - searches OTHER profiles' skills dirs for the same name, - names the profile(s) where the skill exists and the path, - suggests `hermes -p ` to switch profiles, or cross_profile=True for an explicit edit. All 5 "not found" sites in skill_manager_tool (edit, patch, delete, write_file, remove_file) now go through the helper. Reference incident (May 2026): a hermes-security profile session edited skills under both ~/.hermes/profiles/hermes-security/skills/ AND ~/.hermes/skills/ (the default profile's skills) without realizing the second path belonged to a different profile. Three of the four skill files needed manual restoration afterward. What this PR does NOT do: * No hard block. The terminal tool can still touch any of these paths with no guard — same posture as the dangerous-command approval flow. SECURITY.md §3.2 applies. * No regex sweep on terminal commands for cross-profile paths. That direction is a Skills-Guard-style arms race (cd + relative paths, base64, etc.) and would false-positive on legitimate cross-profile reads. Filed as a follow-up. * No on-disk path migration. ~/.hermes/skills/ remains the default profile's skills dir; this PR is about telling the agent about that boundary, not changing the layout. Tests: tests/agent/test_file_safety_cross_profile.py (16 tests) - _resolve_active_profile_name covers default/named/failure paths - classify_cross_profile_target covers all four scoped areas, both directions (default → named, named → default, named → named), non-Hermes paths, and root-level config files - get_cross_profile_warning covers in-profile no-op, cross-profile message shape, and the defense-in-depth self-documentation tests/tools/test_cross_profile_guard.py (12 tests) - write_file: in-profile allow, cross-profile block, cross_profile=True bypass, non-Hermes pass-through - patch: replace-mode block, cross_profile=True bypass, V4A patch path extraction - skill_manage: error names the other profile (single + multiple), missing-everywhere falls back to skills_list hint - system prompt: contract-level checks (both branches present, cross_profile=True mentioned, ~/.hermes/profiles/ referenced) All 207 existing tests in file_safety/file_operations/skill_manager still pass. 10 system-prompt tests still pass. E2E verified: the exact incident scenario (security profile editing default's hermes-agent-dev skill) is now blocked with the warning message; cross_profile=True unblocks. * fix(code_execution): add cross_profile to write_file/patch stubs The cross_profile kwarg added to write_file_tool/patch_tool needs to flow through the execute_code sandbox stubs in _TOOL_STUBS so the test_stubs_cover_all_schema_params drift test passes. Without this, scripts running inside execute_code couldn't pass cross_profile=True through hermes_tools.write_file(). Caught by CI on PR #31290. --- agent/file_safety.py | 145 ++++++++++ agent/system_prompt.py | 34 +++ tests/agent/test_file_safety_cross_profile.py | 219 +++++++++++++++ tests/tools/test_cross_profile_guard.py | 259 ++++++++++++++++++ tools/code_execution_tool.py | 12 +- tools/file_tools.py | 81 +++++- tools/skill_manager_tool.py | 115 +++++++- 7 files changed, 846 insertions(+), 19 deletions(-) create mode 100644 tests/agent/test_file_safety_cross_profile.py create mode 100644 tests/tools/test_cross_profile_guard.py diff --git a/agent/file_safety.py b/agent/file_safety.py index c5e132bdde1..2e8d31282af 100644 --- a/agent/file_safety.py +++ b/agent/file_safety.py @@ -254,3 +254,148 @@ def get_read_block_error(path: str) -> Optional[str]: ) return None + + +# --------------------------------------------------------------------------- +# Cross-profile write guard (#TBD) +# +# Hermes profiles are separate HERMES_HOME dirs under +# ``/profiles//``. Each profile has its own skills/, plugins/, +# cron/, memories/. When an agent runs under one profile, writing into +# ANOTHER profile's directories is almost always wrong — those skills / +# plugins / cron jobs / memories affect a different session the user runs +# from a different shell. +# +# Soft guard, NOT a security boundary: the agent runs as the same OS user +# and has unrestricted terminal access, so this returns a warning the model +# can choose to honor or override with ``cross_profile=True``. Same shape +# as the dangerous-command approval flow — the agent is told the boundary +# exists, and explicit user direction is required to cross it. +# +# Reference: May 2026 incident where a hermes-security profile session +# edited skills under both ``~/.hermes/profiles/hermes-security/skills/`` +# AND ``~/.hermes/skills/`` (the default profile's skills) without realizing +# the second path belonged to a different profile. +# --------------------------------------------------------------------------- + +# Profile-scoped directories under HERMES_HOME / / /profiles// +# that should be guarded. Adding a new area here extends the guard with no +# other code change. +PROFILE_SCOPED_AREAS = ("skills", "plugins", "cron", "memories") + + +def _resolve_active_profile_name() -> str: + """Return the active profile name derived from HERMES_HOME. + + ``~/.hermes`` -> ``"default"`` + ``~/.hermes/profiles/X`` -> ``"X"`` + + Falls back to ``"default"`` on any resolution failure so the guard + never raises into the tool path. + """ + try: + home_real = _hermes_home_path().resolve() + root_real = _hermes_root_path().resolve() + except (OSError, RuntimeError): + return "default" + profiles_dir = root_real / "profiles" + try: + rel = home_real.relative_to(profiles_dir) + parts = rel.parts + if len(parts) >= 1: + return parts[0] + except ValueError: + pass + return "default" + + +def classify_cross_profile_target(path: str) -> Optional[dict]: + """Classify a write target as cross-profile if it lands in another + profile's scoped area (skills/plugins/cron/memories). + + Returns ``None`` when the target is outside Hermes scope, or is inside + the ACTIVE profile, or doesn't hit a profile-scoped area. Otherwise + returns a dict with: + + * ``active_profile``: name of the profile the agent is running as + * ``target_profile``: name of the profile the path belongs to + * ``area``: which scoped area (``"skills"``, ``"plugins"``, etc.) + * ``target_path``: the resolved path string + + The caller decides what to do with the result — surface a warning to + the model, prompt the user, or (with explicit consent / + ``cross_profile=True``) proceed anyway. + """ + try: + target = Path(os.path.expanduser(str(path))).resolve() + root_real = _hermes_root_path().resolve() + except (OSError, RuntimeError): + return None + + target_profile: Optional[str] = None + area: Optional[str] = None + + try: + rel = target.relative_to(root_real) + except ValueError: + return None + + parts = rel.parts + if not parts: + return None + + if parts[0] in PROFILE_SCOPED_AREAS: + # ``//...`` → default profile. + target_profile = "default" + area = parts[0] + elif ( + parts[0] == "profiles" + and len(parts) >= 3 + and parts[2] in PROFILE_SCOPED_AREAS + ): + # ``/profiles///...`` → named profile. + target_profile = parts[1] + area = parts[2] + else: + return None + + active_profile = _resolve_active_profile_name() + if target_profile == active_profile: + # In-profile write — not a cross-profile event. + return None + + return { + "active_profile": active_profile, + "target_profile": target_profile, + "area": area, + "target_path": str(target), + } + + +def get_cross_profile_warning(path: str) -> Optional[str]: + """Return a model-facing warning string when ``path`` is cross-profile. + + 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. + """ + 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.)" + ) diff --git a/agent/system_prompt.py b/agent/system_prompt.py index bc29c9ef89a..8fa4c191563 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -205,6 +205,40 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) if _env_hints: stable_parts.append(_env_hints) + # Active-profile hint — names the Hermes profile the agent is running + # under so it doesn't conflate ~/.hermes/skills/ (default profile) with + # ~/.hermes/profiles//skills/ (this profile's). Deterministic + # for the lifetime of the agent — profile name doesn't change + # mid-session, so this doesn't break the prompt cache. + # See file_safety._resolve_active_profile_name + classify_cross_profile_target + # for the matching tool-side guard. + try: + from agent.file_safety import _resolve_active_profile_name + active_profile = _resolve_active_profile_name() + except Exception: + active_profile = "default" + if active_profile == "default": + stable_parts.append( + "Active Hermes profile: default. Other profiles (if any) live " + "under ~/.hermes/profiles//. Each profile has its own " + "skills/, plugins/, cron/, and memories/ that affect a different " + "session than this one. Do not modify another profile's " + "skills/plugins/cron/memories unless the user explicitly directs " + "you to." + ) + else: + stable_parts.append( + f"Active Hermes profile: {active_profile}. This session reads " + f"and writes ~/.hermes/profiles/{active_profile}/. The default " + f"profile's data lives at ~/.hermes/skills/, ~/.hermes/plugins/, " + f"~/.hermes/cron/, ~/.hermes/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." + ) + platform_key = (agent.platform or "").lower().strip() if platform_key in PLATFORM_HINTS: stable_parts.append(PLATFORM_HINTS[platform_key]) diff --git a/tests/agent/test_file_safety_cross_profile.py b/tests/agent/test_file_safety_cross_profile.py new file mode 100644 index 00000000000..cf3605774a3 --- /dev/null +++ b/tests/agent/test_file_safety_cross_profile.py @@ -0,0 +1,219 @@ +"""Tests for the cross-Hermes-profile write guard in agent/file_safety. + +The guard fires when a tool tries to write into another Hermes profile's +skills/plugins/cron/memories directory. It's a soft guard — defense in +depth, NOT a security boundary — but it prevents the agent from silently +corrupting a profile that belongs to a different session. + +Reference: May 2026 incident — a hermes-security profile session +accidentally edited skills under both ~/.hermes/profiles/hermes-security/skills/ +AND ~/.hermes/skills/ (the default profile's skills), realizing only +afterwards that the second path belonged to a different profile. +""" +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + + +# --------------------------------------------------------------------------- +# Helpers — set up a fake Hermes root with two profiles, monkeypatch the +# resolver helpers so the classifier sees the test layout. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def fake_hermes(tmp_path, monkeypatch): + """Build a fake Hermes layout: + + / + skills/foo/SKILL.md # default profile + plugins/foo/__init__.py + cron/ + memories/MEMORY.md + profiles/ + hermes-security/ + skills/foo/SKILL.md # named profile + plugins/... + coder/ + skills/foo/SKILL.md # another named profile + """ + root = tmp_path / "fake-hermes" + (root / "skills" / "foo").mkdir(parents=True) + (root / "skills" / "foo" / "SKILL.md").write_text("# default skill\n") + (root / "plugins" / "foo").mkdir(parents=True) + (root / "memories").mkdir(parents=True) + (root / "cron").mkdir(parents=True) + + sec_home = root / "profiles" / "hermes-security" + (sec_home / "skills" / "foo").mkdir(parents=True) + (sec_home / "skills" / "foo" / "SKILL.md").write_text("# sec skill\n") + (sec_home / "plugins").mkdir(parents=True) + + coder_home = root / "profiles" / "coder" + (coder_home / "skills" / "foo").mkdir(parents=True) + (coder_home / "skills" / "foo" / "SKILL.md").write_text("# coder skill\n") + + # Monkeypatch the resolver functions used by file_safety so each test + # can choose which profile is "active". + import hermes_constants + monkeypatch.setattr(hermes_constants, "get_default_hermes_root", lambda: root) + + # The reloads below ensure get_cross_profile_warning/classify see the patched root. + import agent.file_safety as fs + monkeypatch.setattr(fs, "_hermes_root_path", lambda: root) + + return { + "root": root, + "default_home": root, + "security_home": sec_home, + "coder_home": coder_home, + } + + +def _set_active_home(monkeypatch, hermes_home: Path): + """Point file_safety._hermes_home_path at a specific profile dir.""" + import agent.file_safety as fs + monkeypatch.setattr(fs, "_hermes_home_path", lambda: hermes_home) + + +# --------------------------------------------------------------------------- +# _resolve_active_profile_name +# --------------------------------------------------------------------------- + + +class TestResolveActiveProfileName: + def test_default_when_home_is_root(self, fake_hermes, monkeypatch): + _set_active_home(monkeypatch, fake_hermes["default_home"]) + from agent.file_safety import _resolve_active_profile_name + assert _resolve_active_profile_name() == "default" + + def test_named_profile(self, fake_hermes, monkeypatch): + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import _resolve_active_profile_name + assert _resolve_active_profile_name() == "hermes-security" + + def test_falls_back_to_default_on_resolution_failure(self, fake_hermes, monkeypatch): + """If HERMES_HOME resolution raises, return 'default' rather than crashing the tool.""" + import agent.file_safety as fs + + def _boom(): + raise RuntimeError("simulated") + + monkeypatch.setattr(fs, "_hermes_home_path", _boom) + # Should not raise — falls back to "default" + assert fs._resolve_active_profile_name() == "default" + + +# --------------------------------------------------------------------------- +# classify_cross_profile_target +# --------------------------------------------------------------------------- + + +class TestClassifyCrossProfileTarget: + def test_same_profile_write_returns_none(self, fake_hermes, monkeypatch): + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import classify_cross_profile_target + result = classify_cross_profile_target( + str(fake_hermes["security_home"] / "skills" / "foo" / "SKILL.md") + ) + assert result is None + + def test_security_writing_default_skill(self, fake_hermes, monkeypatch): + """The exact incident from May 2026.""" + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import classify_cross_profile_target + result = classify_cross_profile_target( + str(fake_hermes["default_home"] / "skills" / "foo" / "SKILL.md") + ) + assert result is not None + assert result["active_profile"] == "hermes-security" + assert result["target_profile"] == "default" + assert result["area"] == "skills" + + def test_default_writing_security_skill(self, fake_hermes, monkeypatch): + """Inverse direction — default-profile session reaching into a named profile.""" + _set_active_home(monkeypatch, fake_hermes["default_home"]) + from agent.file_safety import classify_cross_profile_target + result = classify_cross_profile_target( + str(fake_hermes["security_home"] / "skills" / "foo" / "SKILL.md") + ) + assert result is not None + assert result["active_profile"] == "default" + assert result["target_profile"] == "hermes-security" + + def test_named_to_named_cross_profile(self, fake_hermes, monkeypatch): + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import classify_cross_profile_target + result = classify_cross_profile_target( + str(fake_hermes["coder_home"] / "skills" / "foo" / "SKILL.md") + ) + assert result is not None + assert result["target_profile"] == "coder" + + @pytest.mark.parametrize("area", ["skills", "plugins", "cron", "memories"]) + def test_all_profile_scoped_areas_classified(self, fake_hermes, monkeypatch, area): + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import classify_cross_profile_target + target = fake_hermes["default_home"] / area / "foo.txt" + result = classify_cross_profile_target(str(target)) + assert result is not None + assert result["area"] == area + + def test_non_hermes_path_returns_none(self, fake_hermes, monkeypatch, tmp_path): + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import classify_cross_profile_target + # Path outside any Hermes root + assert classify_cross_profile_target(str(tmp_path / "random.txt")) is None + + def test_hermes_config_not_classified_as_cross_profile(self, fake_hermes, monkeypatch): + """Files under /config.yaml or /.env are NOT profile-scoped + (already covered by build_write_denied_paths). Don't double-warn.""" + _set_active_home(monkeypatch, fake_hermes["security_home"]) + from agent.file_safety import classify_cross_profile_target + # config.yaml at root level is not in PROFILE_SCOPED_AREAS + result = classify_cross_profile_target( + str(fake_hermes["default_home"] / "config.yaml") + ) + assert result is None + + +# --------------------------------------------------------------------------- +# get_cross_profile_warning +# --------------------------------------------------------------------------- + + +class TestGetCrossProfileWarning: + 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 + + def test_cross_profile_warning_names_both_profiles(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") + ) + 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 + + 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 new file mode 100644 index 00000000000..20814fea1ff --- /dev/null +++ b/tests/tools/test_cross_profile_guard.py @@ -0,0 +1,259 @@ +"""Tests for the cross-profile soft guard wired into write_file / patch / +skill_manage. + +The classifier is tested in tests/agent/test_file_safety_cross_profile.py. +This file tests that the tool surfaces: + + 1. Refuse cross-profile writes by default and return the warning. + 2. Accept cross-profile writes when cross_profile=True is passed. + 3. Continue to accept in-profile writes normally. + 4. skill_manage's "not found" error names other profiles where the + skill exists. +""" +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + + +@pytest.fixture +def fake_hermes(tmp_path, monkeypatch): + """Build a two-profile Hermes layout and point HERMES_HOME at + the hermes-security profile (matching the original-incident shape). + """ + root = tmp_path / "fake-hermes" + (root / "skills" / "shared-skill").mkdir(parents=True) + (root / "skills" / "shared-skill" / "SKILL.md").write_text( + "---\nname: shared-skill\ndescription: default copy.\n---\n" + ) + + sec_home = root / "profiles" / "hermes-security" + (sec_home / "skills").mkdir(parents=True) + + coder_home = root / "profiles" / "coder" + (coder_home / "skills").mkdir(parents=True) + + monkeypatch.setenv("HERMES_HOME", str(sec_home)) + + import hermes_constants + monkeypatch.setattr(hermes_constants, "get_default_hermes_root", lambda: root) + + import agent.file_safety as fs + monkeypatch.setattr(fs, "_hermes_home_path", lambda: sec_home) + monkeypatch.setattr(fs, "_hermes_root_path", lambda: root) + + return { + "root": root, + "sec_home": sec_home, + "coder_home": coder_home, + } + + +# --------------------------------------------------------------------------- +# write_file +# --------------------------------------------------------------------------- + + +class TestWriteFileCrossProfileGuard: + def test_in_profile_write_allowed(self, fake_hermes): + from tools.file_tools import write_file_tool + target = fake_hermes["sec_home"] / "skills" / "new-skill" / "SKILL.md" + target.parent.mkdir(parents=True) + result_json = write_file_tool(str(target), "in-profile content") + result = json.loads(result_json) + assert not result.get("error"), f"In-profile write should succeed: {result}" + 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.""" + 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.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 + + def test_cross_profile_True_bypass(self, fake_hermes): + """Explicit override after user direction must succeed.""" + from tools.file_tools import write_file_tool + target = fake_hermes["root"] / "skills" / "shared-skill" / "SKILL.md" + result_json = write_file_tool( + str(target), "user-directed override", cross_profile=True + ) + result = json.loads(result_json) + assert not result.get("error"), f"cross_profile=True must succeed: {result}" + assert target.read_text() == "user-directed override" + + def test_non_hermes_path_unaffected(self, fake_hermes, tmp_path): + from tools.file_tools import write_file_tool + target = tmp_path / "outside" / "main.py" + target.parent.mkdir() + result_json = write_file_tool(str(target), "print('hello')") + result = json.loads(result_json) + assert not result.get("error") + assert target.exists() + + +# --------------------------------------------------------------------------- +# patch +# --------------------------------------------------------------------------- + + +class TestPatchCrossProfileGuard: + def test_cross_profile_patch_blocked(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.", + ) + result = json.loads(result_json) + assert result.get("error") + assert "cross-profile" in result["error"].lower() + assert target.read_text() == original + + def test_cross_profile_patch_bypass(self, fake_hermes): + from tools.file_tools import patch_tool + target = fake_hermes["root"] / "skills" / "shared-skill" / "SKILL.md" + result_json = patch_tool( + mode="replace", + path=str(target), + old_string="default copy.", + new_string="user-directed update.", + cross_profile=True, + ) + result = json.loads(result_json) + assert not result.get("error"), f"cross_profile=True bypass: {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.""" + 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" + "*** 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 + + +# --------------------------------------------------------------------------- +# skill_manage — error message naming other profile (item D) +# --------------------------------------------------------------------------- + + +class TestSkillManageCrossProfileErrorUX: + def _make_skill_in_profile(self, profile_dir: Path, name: str): + d = profile_dir / "skills" / name + d.mkdir(parents=True, exist_ok=True) + (d / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: a skill.\n---\n" + ) + + def test_error_names_other_profile_when_skill_lives_there( + self, fake_hermes, monkeypatch + ): + """The original incident shape — model expects 'foo' in active + profile, but 'foo' lives in default. Error must point at default.""" + self._make_skill_in_profile(fake_hermes["root"], "default-only-skill") + + # Re-import the module so SKILLS_DIR picks up HERMES_HOME (set in + # the fixture). Skill_manager_tool computes SKILLS_DIR at import. + import importlib + import tools.skill_manager_tool + importlib.reload(tools.skill_manager_tool) + from tools.skill_manager_tool import _skill_not_found_error + + 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 + + def test_error_names_multiple_profiles(self, fake_hermes, monkeypatch): + """When the skill exists in TWO other profiles, both should be named.""" + self._make_skill_in_profile(fake_hermes["root"], "everywhere-skill") + self._make_skill_in_profile(fake_hermes["coder_home"], "everywhere-skill") + + import importlib + import tools.skill_manager_tool + importlib.reload(tools.skill_manager_tool) + from tools.skill_manager_tool import _skill_not_found_error + + err = _skill_not_found_error("everywhere-skill") + assert "default" in err + assert "coder" in err + # Switch-profiles hint + assert "hermes -p" in err + + def test_genuinely_missing_skill_keeps_helpful_hint( + self, fake_hermes, monkeypatch + ): + """When no profile has the skill, error falls back to skills_list hint.""" + import importlib + import tools.skill_manager_tool + importlib.reload(tools.skill_manager_tool) + from tools.skill_manager_tool import _skill_not_found_error + + err = _skill_not_found_error("totally-imaginary-skill") + assert "not found in active profile 'hermes-security'" in err + assert "skills_list" in err + + +# --------------------------------------------------------------------------- +# System prompt active-profile line (item B) +# --------------------------------------------------------------------------- + + +class TestSystemPromptActiveProfile: + def test_default_profile_line_in_prompt(self, tmp_path, monkeypatch): + """When active profile is 'default', the prompt names it and warns + about ~/.hermes/profiles//.""" + # Don't set HERMES_HOME — falls back to default. + import agent.file_safety as fs + monkeypatch.setattr(fs, "_hermes_home_path", lambda: tmp_path / "fake") + monkeypatch.setattr(fs, "_hermes_root_path", lambda: tmp_path / "fake") + + from agent.file_safety import _resolve_active_profile_name + assert _resolve_active_profile_name() == "default" + # Build the line manually to pin the contract — the prompt builder + # is too heavy to instantiate end-to-end in a unit test. + # See agent/system_prompt.py for the exact wording. + + def test_named_profile_line_in_prompt_text(self, fake_hermes): + """When active profile is 'hermes-security', the prompt warns + explicitly about NOT modifying default's skills/plugins/cron/memories.""" + # Spot-check by reading the source — the contract is: + # (1) names the active profile, (2) names the default-profile + # paths, (3) says "do not modify another profile's" without + # explicit user direction. + 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 "~/.hermes/profiles/" in src + # Both branches present (default and named profile). + assert "Active Hermes profile: default" in src + assert "Active Hermes profile: {active_profile}" in src diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index bdbc4bfbe1b..f57085277e9 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -202,9 +202,9 @@ _TOOL_STUBS = { ), "write_file": ( "write_file", - "path: str, content: str", - '"""Write content to a file (always overwrites). Returns dict with status."""', - '{"path": path, "content": content}', + "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."""', + '{"path": path, "content": content, "cross_profile": cross_profile}', ), "search_files": ( "search_files", @@ -214,9 +214,9 @@ _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', - '"""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}', + '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."""', + '{"path": path, "old_string": old_string, "new_string": new_string, "replace_all": replace_all, "mode": mode, "patch": patch, "cross_profile": cross_profile}', ), "terminal": ( "terminal", diff --git a/tools/file_tools.py b/tools/file_tools.py index 32dda0f82ee..a5be71a8bfe 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -174,6 +174,37 @@ def _check_sensitive_path(filepath: str, task_id: str = "default") -> str | None return None +def _check_cross_profile_path(filepath: str, task_id: str = "default") -> str | None: + """Return a cross-profile warning string when ``filepath`` lands in + another Hermes profile's skills/plugins/cron/memories directory. + + Returns ``None`` when the write is in-scope (same profile) or outside + Hermes scope entirely. Soft guard — the agent can override 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`` for the + detection rules. + """ + try: + from agent.file_safety import get_cross_profile_warning + except Exception: + # Fail open on import error — the existing sensitive-path guard + # 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. + try: + resolved = str(_resolve_path_for_task(filepath, task_id)) + except (OSError, ValueError): + resolved = filepath + + return get_cross_profile_warning(resolved) + + def _is_expected_write_exception(exc: Exception) -> bool: """Return True for expected write denials that should not hit error logs.""" if isinstance(exc, PermissionError): @@ -795,11 +826,23 @@ def _check_file_staleness(filepath: str, task_id: str) -> str | None: return None -def write_file_tool(path: str, content: str, task_id: str = "default") -> str: - """Write content to a file.""" +def write_file_tool(path: str, content: str, task_id: str = "default", + cross_profile: bool = False) -> 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. + """ sensitive_err = _check_sensitive_path(path, task_id) if sensitive_err: return tool_error(sensitive_err) + if not cross_profile: + cross_warning = _check_cross_profile_path(path, task_id) + if cross_warning: + return tool_error(cross_warning) if _is_internal_file_status_text(content): return tool_error( "Refusing to write internal read_file status text as file content. " @@ -854,8 +897,13 @@ def write_file_tool(path: str, content: str, task_id: str = "default") -> str: def patch_tool(mode: str = "replace", path: str = None, old_string: str = None, new_string: str = None, replace_all: bool = False, patch: str = None, - task_id: str = "default") -> str: - """Patch a file using replace mode or V4A patch format.""" + task_id: str = "default", cross_profile: bool = False) -> 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. + """ # Check sensitive paths for both replace (explicit path) and V4A patch (extract paths) _paths_to_check = [] if path: @@ -868,6 +916,10 @@ def patch_tool(mode: str = "replace", path: str = None, old_string: str = None, sensitive_err = _check_sensitive_path(_p, task_id) if sensitive_err: return tool_error(sensitive_err) + if not cross_profile: + cross_warning = _check_cross_profile_path(_p, task_id) + if cross_warning: + return tool_error(cross_warning) try: # Resolve paths for locking. Ordered + deduplicated so concurrent # callers lock in the same order — prevents deadlock on overlapping @@ -1052,7 +1104,12 @@ WRITE_FILE_SCHEMA = { "type": "object", "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"} + "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, + }, }, "required": ["path", "content"] } @@ -1099,6 +1156,11 @@ 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, + }, }, "required": ["mode"], }, @@ -1149,7 +1211,10 @@ def _handle_write_file(args, **kw): f"write_file: 'content' must be a string, got " f"{type(args['content']).__name__}." ) - return write_file_tool(path=args["path"], content=args["content"], task_id=tid) + return write_file_tool( + path=args["path"], content=args["content"], task_id=tid, + cross_profile=bool(args.get("cross_profile", False)), + ) def _handle_patch(args, **kw): @@ -1157,7 +1222,9 @@ def _handle_patch(args, **kw): return patch_tool( mode=args.get("mode", "replace"), path=args.get("path"), old_string=args.get("old_string"), new_string=args.get("new_string"), - replace_all=args.get("replace_all", False), patch=args.get("patch"), task_id=tid) + replace_all=args.get("replace_all", False), patch=args.get("patch"), task_id=tid, + cross_profile=bool(args.get("cross_profile", False)), + ) def _handle_search_files(args, **kw): diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 547167a6623..4ce5f06e4c9 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -40,7 +40,7 @@ import shutil import tempfile from pathlib import Path from hermes_constants import get_hermes_home, display_hermes_home -from typing import Dict, Any, Optional, Tuple +from typing import Dict, Any, List, Optional, Tuple from utils import atomic_replace, is_truthy_value from hermes_cli.config import cfg_get @@ -295,6 +295,109 @@ def _find_skill(name: str) -> Optional[Dict[str, Any]]: return None +def _find_skill_in_other_profiles(name: str) -> List[Tuple[str, Path]]: + """Look for ``name`` under SKILL.md across OTHER Hermes profiles. + + Returns a list of ``(profile_name, skill_dir)`` pairs. Used to make + the "Skill X not found" error explain when the user is editing the + wrong profile. Empty list when no other profile has the skill (or + when profile discovery fails — fail-quiet, the caller falls back to + the plain "not found" error). + """ + matches: List[Tuple[str, Path]] = [] + try: + from hermes_constants import get_default_hermes_root + from agent.skill_utils import is_excluded_skill_path + except Exception: + return matches + + try: + root = get_default_hermes_root() + except Exception: + return matches + + # Collect (profile_name, skills_dir) for every profile EXCEPT the + # one whose SKILLS_DIR we already searched in _find_skill(). + active_dir = SKILLS_DIR.resolve() if SKILLS_DIR.exists() else SKILLS_DIR + candidates: List[Tuple[str, Path]] = [] + + # Default profile (~/.hermes/skills) — only consider when active is non-default. + default_skills = root / "skills" + try: + if default_skills.resolve() != active_dir: + candidates.append(("default", default_skills)) + except (OSError, RuntimeError): + pass + + # All named profiles (~/.hermes/profiles/*/skills) + profiles_root = root / "profiles" + if profiles_root.is_dir(): + try: + for entry in profiles_root.iterdir(): + if not entry.is_dir(): + continue + pskills = entry / "skills" + try: + if pskills.resolve() == active_dir: + continue + except (OSError, RuntimeError): + continue + candidates.append((entry.name, pskills)) + except OSError: + pass + + for profile_name, skills_dir in candidates: + if not skills_dir.is_dir(): + continue + try: + for skill_md in skills_dir.rglob("SKILL.md"): + if is_excluded_skill_path(skill_md): + continue + if skill_md.parent.name == name: + matches.append((profile_name, skill_md.parent)) + break # one match per profile is enough + except OSError: + continue + return matches + + +def _skill_not_found_error(name: str, suffix: str = "") -> str: + """Build a "skill not found" error that names other profiles holding + the same skill, so the agent can recognize a profile-scoping mistake. + + ``suffix`` is appended after the cross-profile hint if present + (e.g. ``" Create it first with action='create'."``). + """ + from agent.file_safety import _resolve_active_profile_name + active = _resolve_active_profile_name() + base = f"Skill '{name}' not found in active profile '{active}'." + + others = _find_skill_in_other_profiles(name) + if others: + if len(others) == 1: + 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``." + ) + 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``." + ) + else: + base += " Use skills_list() to see available skills." + + if suffix: + base += suffix + return base + + def _validate_file_path(file_path: str) -> Optional[str]: """ Validate a file path for write_file/remove_file. @@ -439,7 +542,7 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]: existing = _find_skill(name) if not existing: - return {"success": False, "error": f"Skill '{name}' not found. Use skills_list() to see available skills."} + return {"success": False, "error": _skill_not_found_error(name)} skill_md = existing["path"] / "SKILL.md" # Back up original content for rollback @@ -479,7 +582,7 @@ def _patch_skill( existing = _find_skill(name) if not existing: - return {"success": False, "error": f"Skill '{name}' not found."} + return {"success": False, "error": _skill_not_found_error(name)} skill_dir = existing["path"] @@ -568,7 +671,7 @@ def _delete_skill(name: str, absorbed_into: Optional[str] = None) -> Dict[str, A """ existing = _find_skill(name) if not existing: - return {"success": False, "error": f"Skill '{name}' not found."} + return {"success": False, "error": _skill_not_found_error(name)} pinned_err = _pinned_guard(name) if pinned_err: @@ -637,7 +740,7 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]: existing = _find_skill(name) if not existing: - return {"success": False, "error": f"Skill '{name}' not found. Create it first with action='create'."} + return {"success": False, "error": _skill_not_found_error(name, " Create it first with action='create'.")} target, err = _resolve_skill_target(existing["path"], file_path) if err: @@ -671,7 +774,7 @@ def _remove_file(name: str, file_path: str) -> Dict[str, Any]: existing = _find_skill(name) if not existing: - return {"success": False, "error": f"Skill '{name}' not found."} + return {"success": False, "error": _skill_not_found_error(name)} skill_dir = existing["path"]