From 2770f93064aa41c19f001a0efa8cfccb3deafbf8 Mon Sep 17 00:00:00 2001 From: Adolanium <94890352+Adolanium@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:39:41 +0300 Subject: [PATCH] fix(profiles): reject traversal-shaped profile names in get_profile_dir A WS 'profile' param like '../../foo' normalized to a path component that escaped the profiles root, letting a connected client bind an arbitrary existing directory as a profile home (state.db opened there, and session delete chains into per-id file cleanup under /sessions/). get_profile_dir now validates the canonical name against the profile id regex before joining it under profiles/. The regex only, not the reserved list, so pre-reserved-list dirs like profiles/hermes keep resolving. Callers that probe existence (profile_exists, _profile_home, the 4064 resolvers) treat ValueError as 'not found'. --- hermes_cli/profiles.py | 14 +++++++++++-- tests/hermes_cli/test_profiles.py | 14 +++++++++++++ .../test_profile_target_unavailable.py | 21 +++++++++++++++++++ tui_gateway/mcp_rpc_helpers.py | 5 ++++- tui_gateway/methods_profiles.py | 5 ++++- tui_gateway/server.py | 11 +++++++--- 6 files changed, 63 insertions(+), 7 deletions(-) diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 217510d7ca..f3e557dc07 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -234,15 +234,25 @@ def get_profile_dir(name: str) -> Path: canon = normalize_profile_name(name) if canon == "default": return _get_default_hermes_home() + # The name becomes a path component under profiles/; refuse anything that + # is not a valid profile id so every caller (WS params, /p// + # prefixes, tool args) fails closed instead of escaping the root. The + # regex only, not _RESERVED_NAMES: a pre-reserved-list dir like + # profiles/hermes may still exist and must keep resolving. + if not _PROFILE_ID_RE.match(canon): + raise ValueError(f"Invalid profile name {canon!r}. Must match [a-z0-9][a-z0-9_-]{{0,63}}") return _get_profiles_root() / canon def profile_exists(name: str) -> bool: """Check whether a live (non-tombstoned) profile directory exists.""" - canon = normalize_profile_name(name) + try: + canon = normalize_profile_name(name) + profile_dir = get_profile_dir(canon) + except ValueError: + return False if canon == "default": return True - profile_dir = get_profile_dir(canon) return profile_dir.is_dir() and not named_profile_is_deleted(profile_dir) diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index 6ea3e9fbff..0aaeaff758 100644 --- a/tests/hermes_cli/test_profiles.py +++ b/tests/hermes_cli/test_profiles.py @@ -105,6 +105,20 @@ class TestGetProfileDir: result = get_profile_dir("default") assert result == tmp_path / ".hermes" + def test_valid_name_resolves_under_profiles_root(self, profile_env): + assert get_profile_dir("coder") == _get_profiles_root() / "coder" + + @pytest.mark.parametrize("name", ["..", "../outside", "../../tmp", "a/b", "a\\b", ".hidden", "has space"]) + def test_traversal_and_invalid_names_rejected(self, name, profile_env): + # The name becomes a path component under profiles/; invalid ids must + # raise instead of escaping the root. + with pytest.raises(ValueError): + get_profile_dir(name) + + @pytest.mark.parametrize("name", ["..", "../outside", "a/b"]) + def test_profile_exists_false_for_invalid_names(self, name, profile_env): + assert profiles.profile_exists(name) is False + # =================================================================== # TestCreateProfile diff --git a/tests/tui_gateway/test_profile_target_unavailable.py b/tests/tui_gateway/test_profile_target_unavailable.py index a632342e0c..f55f3b17f6 100644 --- a/tests/tui_gateway/test_profile_target_unavailable.py +++ b/tests/tui_gateway/test_profile_target_unavailable.py @@ -57,3 +57,24 @@ def test_custom_root_basename_target_fails_closed_when_unavailable(tmp_path, mon with pytest.raises(FileNotFoundError): with server._profile_db({"profile": "customer-data"}): pass + + +@pytest.mark.parametrize("name", ["..", "../outside", "../../tmp", "a/b", "a\\b", ".hidden"]) +def test_profile_param_traversal_fails_closed(tmp_path, monkeypatch, name): + """A traversal-shaped ``profile`` param must never resolve outside profiles/.""" + from tui_gateway import server + + home = tmp_path / ".hermes" + outside = tmp_path / "outside" + outside.mkdir(parents=True) # a real directory the traversal could land on + home.mkdir() + (home / "config.yaml").write_text("terminal:\n cwd: /launch\n") + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(server, "_hermes_home", home) + + with pytest.raises(FileNotFoundError): + server._profile_home(name) + with pytest.raises(FileNotFoundError): + with server._profile_db({"profile": name}): + pass diff --git a/tui_gateway/mcp_rpc_helpers.py b/tui_gateway/mcp_rpc_helpers.py index aa69566f99..3b38c6a03c 100644 --- a/tui_gateway/mcp_rpc_helpers.py +++ b/tui_gateway/mcp_rpc_helpers.py @@ -67,7 +67,10 @@ def resolve_profile(rid, params, err_fn) -> Tuple[Optional[Any], Optional[dict]] from hermes_cli.profiles import get_profile_dir from hermes_constants import set_hermes_home_override - profile_dir = get_profile_dir(profile) + try: + profile_dir = get_profile_dir(profile) + except ValueError: + return None, err_fn(rid, 4064, f"profile '{profile}' not found") if not profile_dir or not profile_dir.is_dir(): return None, err_fn(rid, 4064, f"profile '{profile}' not found") return set_hermes_home_override(str(profile_dir)), None diff --git a/tui_gateway/methods_profiles.py b/tui_gateway/methods_profiles.py index 9f0a06cbdd..a036b18e11 100644 --- a/tui_gateway/methods_profiles.py +++ b/tui_gateway/methods_profiles.py @@ -72,7 +72,10 @@ def _resolve_profile(rid, params): if not name: return name, None, _err(rid, 4063, "name required") from hermes_cli.profiles import get_profile_dir - profile_dir = Path(get_profile_dir(name)) + try: + profile_dir = Path(get_profile_dir(name)) + except ValueError: + return name, None, _err(rid, 4064, f"profile '{name}' not found") if not profile_dir.is_dir(): return name, None, _err(rid, 4064, f"profile '{name}' not found") return name, profile_dir, None diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ec7a7d13e1..91faeb3caf 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -464,7 +464,9 @@ def _canonical_profile_request(name: str) -> str: """ if name.casefold() in {".hermes", "hermes"}: from hermes_cli import profiles as profiles_mod - if not Path(profiles_mod.get_profile_dir(name)).is_dir(): + # Check the profiles root directly: get_profile_dir rejects "hermes" as a + # reserved name, but a pre-reserved-list install may still carry that dir. + if not (profiles_mod._get_profiles_root() / profiles_mod.normalize_profile_name(name)).is_dir(): return "default" return name @@ -487,8 +489,11 @@ def _profile_home(profile: str | None) -> Path | None: if not (name := _canonical_profile_request((profile or "").strip())): return None from hermes_cli import profiles as profiles_mod - home = Path(profiles_mod.get_profile_dir(name)) - if not home.is_dir(): + try: + home = Path(profiles_mod.get_profile_dir(name)) + except ValueError: + home = None + if home is None or not home.is_dir(): raise FileNotFoundError(f"Profile '{name}' does not exist.") if home.resolve() == Path(_hermes_home).resolve(): return None # already the launch profile (no override needed)