From f855c69ff7653ef0756dbae59490d9a2dd5ef55f Mon Sep 17 00:00:00 2001 From: Oscar Date: Wed, 15 Apr 2026 08:45:19 +0800 Subject: [PATCH] feat: add profile boundary isolation for cross-profile data protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add is_within_profile_boundary() guard to file tool operations (read, write, patch, search) to prevent named profiles from accessing other profiles' data directories (memory, sessions, config). - hermes_constants.py: add is_profiled_mode(), get_profile_boundary(), is_within_profile_boundary() — soft isolation that blocks cross-profile access while preserving full filesystem access for project work - tools/file_tools.py: add boundary checks at all four file operation entry points (read, write, patch, search) - tests/test_profile_boundary.py: 28 tests covering default profile (admin, unrestricted), named profiles (restricted), symlinks, tilde expansion, edge cases Motivation: In multi-profile setups (e.g., separate Feishu bots per profile), profiles share the same Python venv and codebase. Without boundary enforcement, one profile's agent could accidentally read or modify another profile's memory, sessions, or configuration. --- hermes_constants.py | 77 ++++++++++ tests/test_profile_boundary.py | 247 +++++++++++++++++++++++++++++++++ tools/file_tools.py | 26 ++++ 3 files changed, 350 insertions(+) create mode 100644 tests/test_profile_boundary.py diff --git a/hermes_constants.py b/hermes_constants.py index 3bc56d4f7874..ddca5e78df79 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -137,6 +137,83 @@ def get_subprocess_home() -> str | None: return None +def is_profiled_mode() -> bool: + """Return True if the current process is running under a named profile. + + In profile mode, ``HERMES_HOME`` is ``/profiles/`` and the + agent should be restricted to its own data directory to prevent + cross-profile data leakage (reading another profile's memory, sessions, + or config). + + The default profile (``HERMES_HOME == ~/.hermes``) returns ``False`` + because the default profile is the admin — it needs access to all + profiles for management tasks (``profile list``, skill syncing, etc.). + """ + env_home = os.environ.get("HERMES_HOME", "") + if not env_home: + return False + env_path = Path(env_home) + # Check if HERMES_HOME is /profiles/ + return env_path.parent.name == "profiles" + + +def get_profile_boundary() -> Path | None: + """Return the allowed root path for file operations, or None if unrestricted. + + - Default profile (admin): returns ``None`` — full filesystem access. + - Named profile: returns ``HERMES_HOME`` (e.g. ``~/.hermes/profiles/frog/``). + """ + if is_profiled_mode(): + return get_hermes_home() + return None + + +def is_within_profile_boundary(path: str) -> tuple[bool, str]: + """Check whether *path* is within the current profile's boundary. + + Returns ``(allowed, reason)``. When *allowed* is ``False``, *reason* + explains why (for the tool's error response). + + The policy is **cross-profile isolation only** — we block access to + *other profiles' data directories* but allow everything else (home + directory projects, /tmp, system paths, installed packages, etc.). + + This is intentionally NOT a full sandbox. Named profiles still need + to read/write project code, system configs, and installed tools. + The goal is solely to prevent one profile from reading another + profile's memory, sessions, or config. + """ + boundary = get_profile_boundary() + if boundary is None: + return True, "" + + try: + resolved = Path(path).expanduser().resolve() + except (OSError, ValueError): + return True, "" # Unresolvable paths are not our concern + + # Get the parent profiles root: ~/.hermes/profiles/ + profiles_root = boundary.resolve().parent # /profiles/ + + # If the path is NOT under the profiles root at all, allow it freely. + # This covers /tmp, ~/projects, /usr, etc. + try: + resolved.relative_to(profiles_root) + except ValueError: + return True, "" + + # Path IS under ~/.hermes/profiles/ — check it's within OUR profile + try: + resolved.relative_to(boundary.resolve()) + return True, "" + except ValueError: + return False, ( + f"Access denied: '{path}' is in another profile's data directory. " + f"This agent is restricted to {display_hermes_home()}/ to prevent " + f"cross-profile data leakage." + ) + + VALID_REASONING_EFFORTS = ("minimal", "low", "medium", "high", "xhigh") diff --git a/tests/test_profile_boundary.py b/tests/test_profile_boundary.py new file mode 100644 index 000000000000..eeb511cb2a42 --- /dev/null +++ b/tests/test_profile_boundary.py @@ -0,0 +1,247 @@ +"""Tests for profile boundary enforcement in hermes_constants. + +Covers: is_profiled_mode(), get_profile_boundary(), is_within_profile_boundary(). +These functions enforce cross-profile isolation — a named profile must not +access another profile's data directory, but should still have full access +to the filesystem for project work. +""" + +from pathlib import Path +from unittest.mock import patch + +import pytest + +from hermes_constants import ( + is_profiled_mode, + get_profile_boundary, + is_within_profile_boundary, +) + + +@pytest.fixture() +def default_env(tmp_path, monkeypatch): + """Default profile (admin) — unrestricted, full filesystem access.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + return home + + +@pytest.fixture() +def named_profile_env(tmp_path, monkeypatch): + """Named profile (e.g. 'frog') — restricted from other profiles.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir() + profile_dir = hermes_root / "profiles" / "frog" + profile_dir.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(profile_dir)) + return profile_dir + + +@pytest.fixture() +def multi_profile_env(tmp_path, monkeypatch): + """Two named profiles to test cross-profile blocking.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + hermes_root = tmp_path / ".hermes" + hermes_root.mkdir() + frog_dir = hermes_root / "profiles" / "frog" + xiaoge_dir = hermes_root / "profiles" / "xiaoge" + frog_dir.mkdir(parents=True) + xiaoge_dir.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(frog_dir)) + return { + "root": hermes_root, + "frog": frog_dir, + "xiaoge": xiaoge_dir, + } + + +# =================================================================== +# is_profiled_mode +# =================================================================== + + +class TestIsProfiledMode: + def test_default_profile_returns_false(self, default_env): + assert is_profiled_mode() is False + + def test_named_profile_returns_true(self, named_profile_env): + assert is_profiled_mode() is True + + def test_no_hermes_home_returns_false(self, monkeypatch): + monkeypatch.delenv("HERMES_HOME", raising=False) + assert is_profiled_mode() is False + + def test_custom_path_not_in_profiles_returns_false(self, tmp_path, monkeypatch): + """A custom HERMES_HOME like /opt/hermes is NOT a profile.""" + custom = tmp_path / "opt" / "hermes" + custom.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(custom)) + assert is_profiled_mode() is False + + def test_profiles_in_other_path_components(self, tmp_path, monkeypatch): + """HERMES_HOME=/tmp/profiles/work → profiled.""" + p = tmp_path / "tmp" / "profiles" / "work" + p.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(p)) + assert is_profiled_mode() is True + + def test_profiles_dir_name_but_not_structure(self, tmp_path, monkeypatch): + """HERMES_HOME=.../profiles → not profiled (parent is not 'profiles').""" + p = tmp_path / "profiles" + p.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(p)) + assert is_profiled_mode() is False + + +# =================================================================== +# get_profile_boundary +# =================================================================== + + +class TestGetProfileBoundary: + def test_default_profile_no_boundary(self, default_env): + assert get_profile_boundary() is None + + def test_named_profile_returns_hermes_home(self, named_profile_env): + boundary = get_profile_boundary() + assert boundary is not None + assert boundary == named_profile_env + + def test_no_hermes_home_no_boundary(self, monkeypatch): + monkeypatch.delenv("HERMES_HOME", raising=False) + assert get_profile_boundary() is None + + +# =================================================================== +# is_within_profile_boundary — DEFAULT profile (unrestricted) +# =================================================================== + + +class TestBoundaryDefaultProfile: + """Default profile should allow ALL paths.""" + + def test_allows_any_path(self, default_env): + for path in ["/etc/passwd", "/tmp/foo", "~/bar", "/any/thing"]: + ok, reason = is_within_profile_boundary(path) + assert ok is True, f"Should allow {path}, got: {reason}" + + def test_allows_other_profiles(self, default_env, tmp_path): + """Admin profile can access other profiles' data.""" + other = tmp_path / ".hermes" / "profiles" / "other" / "secret.md" + ok, reason = is_within_profile_boundary(str(other)) + assert ok is True + + +# =================================================================== +# is_within_profile_boundary — NAMED profile (cross-profile isolation) +# =================================================================== + + +class TestBoundaryNamedProfile: + """Named profile: block other profiles, allow everything else.""" + + def test_allows_own_profile_files(self, named_profile_env): + own_file = named_profile_env / "skills" / "test.md" + ok, reason = is_within_profile_boundary(str(own_file)) + assert ok is True + + def test_allows_own_profile_nested(self, named_profile_env): + deep = named_profile_env / "sessions" / "sub" / "dir" / "file.json" + ok, reason = is_within_profile_boundary(str(deep)) + assert ok is True + + def test_blocks_other_profile_file(self, multi_profile_env): + other = multi_profile_env["xiaoge"] / "memory.md" + ok, reason = is_within_profile_boundary(str(other)) + assert ok is False + assert "other profile" in reason + + def test_blocks_other_profile_nested(self, multi_profile_env): + other = multi_profile_env["xiaoge"] / "sessions" / "deep" / "data.json" + ok, reason = is_within_profile_boundary(str(other)) + assert ok is False + + def test_blocks_other_profile_root(self, multi_profile_env): + """Even listing the other profile's root dir should be blocked.""" + ok, reason = is_within_profile_boundary(str(multi_profile_env["xiaoge"])) + assert ok is False + + def test_allows_system_files(self, named_profile_env): + ok, reason = is_within_profile_boundary("/etc/passwd") + assert ok is True, reason + + def test_allows_tmp(self, named_profile_env): + ok, reason = is_within_profile_boundary("/tmp/test.txt") + assert ok is True, reason + + def test_allows_home_projects(self, named_profile_env, tmp_path): + project = tmp_path / "projects" / "my-app" / "main.py" + ok, reason = is_within_profile_boundary(str(project)) + assert ok is True, reason + + def test_allows_default_hermes_config(self, multi_profile_env): + """~/.hermes/config.yaml is outside profiles/, so it's allowed.""" + config = multi_profile_env["root"] / "config.yaml" + ok, reason = is_within_profile_boundary(str(config)) + assert ok is True, reason + + def test_allows_path_outside_hermes_entirely(self, named_profile_env): + ok, reason = is_within_profile_boundary("/usr/lib/python3/site.py") + assert ok is True, reason + + def test_tilde_expansion(self, named_profile_env, tmp_path, monkeypatch): + """~ should expand correctly.""" + monkeypatch.setenv("HOME", str(tmp_path)) + ok, reason = is_within_profile_boundary("~/file.txt") + # ~/file.txt is not under profiles/, so allowed + assert ok is True, reason + + def test_symlink_inside_own_profile(self, named_profile_env): + """A symlink pointing inside the profile should be allowed.""" + target = named_profile_env / "real_file.txt" + target.touch() + link = named_profile_env / "link.txt" + link.symlink_to(target) + ok, reason = is_within_profile_boundary(str(link)) + assert ok is True, reason + + def test_symlink_to_other_profile(self, multi_profile_env): + """A symlink pointing to another profile should be BLOCKED.""" + target = multi_profile_env["xiaoge"] / "secret.txt" + target.touch() + link = multi_profile_env["frog"] / "sneaky_link.txt" + link.symlink_to(target) + ok, reason = is_within_profile_boundary(str(link)) + # resolve() follows symlinks → ends up in xiaoge → blocked + assert ok is False, "Symlink to other profile should be blocked" + + def test_nonexistent_path_outside_profiles(self, named_profile_env): + """Nonexistent paths outside profiles/ should be allowed.""" + ok, reason = is_within_profile_boundary("/nonexistent/path/file.txt") + assert ok is True, reason + + def test_nonexistent_path_in_other_profile(self, multi_profile_env): + """Nonexistent paths in other profiles should still be blocked.""" + other = multi_profile_env["xiaoge"] / "does_not_exist" / "file.txt" + ok, reason = is_within_profile_boundary(str(other)) + assert ok is False + + def test_error_message_mentions_profile_name(self, multi_profile_env): + other = multi_profile_env["xiaoge"] / "secret.md" + _, reason = is_within_profile_boundary(str(other)) + # Should mention "frog" (current profile) in the message + assert "frog" in reason + + def test_blocks_sibling_profile_same_depth(self, multi_profile_env): + """A file at the same nesting level but in another profile.""" + frog_file = multi_profile_env["frog"] / "data.json" + xiaoge_file = multi_profile_env["xiaoge"] / "data.json" + # Make sure frog_file is allowed + ok, _ = is_within_profile_boundary(str(frog_file)) + assert ok is True + # But xiaoge_file is blocked + ok, reason = is_within_profile_boundary(str(xiaoge_file)) + assert ok is False diff --git a/tools/file_tools.py b/tools/file_tools.py index ca2118c33e29..1b1c9e58fb23 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -10,6 +10,7 @@ from tools.binary_extensions import has_binary_extension from tools.file_operations import ShellFileOperations from agent.redact import redact_sensitive_text +from hermes_constants import is_within_profile_boundary logger = logging.getLogger(__name__) @@ -281,6 +282,11 @@ def clear_file_ops_cache(task_id: str = None): def read_file_tool(path: str, offset: int = 1, limit: int = 500, task_id: str = "default") -> str: """Read a file with pagination and line numbers.""" + # ── Profile boundary guard ──────────────────────────────────── + allowed, reason = is_within_profile_boundary(path) + if not allowed: + return json.dumps({"error": reason}) + try: # ── Device path guard ───────────────────────────────────────── # Block paths that would hang the process (infinite output, @@ -540,6 +546,11 @@ def _check_file_staleness(filepath: str, task_id: str) -> str | None: def write_file_tool(path: str, content: str, task_id: str = "default") -> str: """Write content to a file.""" + # ── Profile boundary guard ──────────────────────────────────── + allowed, reason = is_within_profile_boundary(path) + if not allowed: + return json.dumps({"error": reason}) + sensitive_err = _check_sensitive_path(path) if sensitive_err: return tool_error(sensitive_err) @@ -566,6 +577,16 @@ 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.""" + # ── Profile boundary guard ──────────────────────────────────── + # Check explicit path first, then extract paths from V4A patch content + _paths_to_check = [] + if path: + _paths_to_check.append(path) + for _p in _paths_to_check: + allowed, reason = is_within_profile_boundary(_p) + if not allowed: + return json.dumps({"error": reason}) + # Check sensitive paths for both replace (explicit path) and V4A patch (extract paths) _paths_to_check = [] if path: @@ -624,6 +645,11 @@ def search_tool(pattern: str, target: str = "content", path: str = ".", output_mode: str = "content", context: int = 0, task_id: str = "default") -> str: """Search for content or files.""" + # ── Profile boundary guard ──────────────────────────────────── + allowed, reason = is_within_profile_boundary(path) + if not allowed: + return json.dumps({"error": reason}) + try: # Track searches to detect *consecutive* repeated search loops. # Include pagination args so users can page through truncated