diff --git a/api/profiles.py b/api/profiles.py index b56868a1..52c552b2 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -37,6 +37,13 @@ _loaded_profile_env_keys: set[str] = set() # process-global _active_profile. _tls = threading.local() +def _unwrap_profile_home_to_base(home: Path) -> Path: + """Return the base Hermes home when *home* is already a named profile dir.""" + if home.parent.name == 'profiles': + return home.parent.parent + return home + + def _resolve_base_hermes_home() -> Path: """Return the BASE ~/.hermes directory — the root that contains profiles/. @@ -56,20 +63,22 @@ def _resolve_base_hermes_home() -> Path: reading it here would make _DEFAULT_HERMES_HOME point to that subdir, causing switch_profile('webui') to look for /home/user/.hermes/profiles/webui/profiles/webui — which doesn't exist. + + HERMES_BASE_HOME normally points at the base home already, but isolated + single-profile WebUI deployments can provide /base/profiles/ there as + well. Normalize both env vars through the same helper so active-profile + and per-request resolution share one base-root contract (#749). """ # Explicit override for tests or unusual setups base_override = os.getenv('HERMES_BASE_HOME', '').strip() if base_override: - return Path(base_override).expanduser() + return _unwrap_profile_home_to_base(Path(base_override).expanduser()) hermes_home = os.getenv('HERMES_HOME', '').strip() if hermes_home: p = Path(hermes_home).expanduser() # If HERMES_HOME points to a profiles/ subdir, walk up two levels to the base - if p.parent.name == 'profiles': - return p.parent.parent - # Otherwise trust it (e.g. test isolation sets HERMES_HOME to TEST_STATE_DIR) - return p + return _unwrap_profile_home_to_base(p) return Path.home() / '.hermes' @@ -193,19 +202,29 @@ def clear_request_profile() -> None: _tls.profile = None +def _resolve_profile_home_for_name(name: str) -> Path: + """Resolve a logical profile name to its Hermes home path. + + Root/default aliases resolve to _DEFAULT_HERMES_HOME. Valid named profiles + resolve to _DEFAULT_HERMES_HOME/profiles/ even when the directory has + not been created yet; the agent layer may create it on first use. Invalid + names fall back to the base home so traversal-shaped cookie values cannot + influence filesystem paths. + """ + if not name or _is_root_profile(name): + return _DEFAULT_HERMES_HOME + if not _PROFILE_ID_RE.fullmatch(name): + return _DEFAULT_HERMES_HOME + return _resolve_named_profile_home(name) + + def get_active_hermes_home() -> Path: """Return the HERMES_HOME path for the currently active profile. Uses get_active_profile_name() so per-request TLS context (issue #798) is respected, not just the process-level global. """ - name = get_active_profile_name() - if _is_root_profile(name): - return _DEFAULT_HERMES_HOME - profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / name - if profile_dir.is_dir(): - return profile_dir - return _DEFAULT_HERMES_HOME + return _resolve_profile_home_for_name(get_active_profile_name()) @@ -393,12 +412,7 @@ def get_hermes_home_for_profile(name: str) -> Path: empty, 'default', or does not match the profile-name format (rejects path traversal such as '../../etc'). """ - if not name or _is_root_profile(name): - return _DEFAULT_HERMES_HOME - if not _PROFILE_ID_RE.fullmatch(name): - return _DEFAULT_HERMES_HOME - profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / name - return profile_dir + return _resolve_profile_home_for_name(name) _TERMINAL_ENV_MAPPINGS = { diff --git a/tests/test_issue798.py b/tests/test_issue798.py index 4207400b..37889688 100644 --- a/tests/test_issue798.py +++ b/tests/test_issue798.py @@ -9,7 +9,9 @@ get_hermes_home_for_profile() resolves a HERMES_HOME path from a name without touching os.environ or module-level state. """ +import json import os +import subprocess import sys import threading from pathlib import Path @@ -71,6 +73,96 @@ def test_get_hermes_home_for_profile_does_not_mutate_globals(): ) +def _run_profile_resolution_probe(env): + script = r''' +import json +from pathlib import Path +import api.profiles as p +import api.models as m + +p.set_request_profile('foo') +foo_home = p.get_active_hermes_home() +explicit_foo_home = p.get_hermes_home_for_profile('foo') +foo_runtime = p.get_profile_runtime_env(explicit_foo_home) +model_home = m._get_profile_home('foo') +explicit_bar_home = p.get_hermes_home_for_profile('bar') +p.set_request_profile('bar') +active_bar_home = p.get_active_hermes_home() +print(json.dumps({ + 'default_home': str(p._DEFAULT_HERMES_HOME), + 'foo_home': str(foo_home), + 'explicit_foo_home': str(explicit_foo_home), + 'foo_terminal_cwd': foo_runtime.get('TERMINAL_CWD'), + 'model_home': str(model_home), + 'explicit_bar_home': str(explicit_bar_home), + 'active_bar_home': str(active_bar_home), +})) +''' + result = subprocess.run( + [sys.executable, '-c', script], + cwd=Path(__file__).parent.parent, + env=env, + text=True, + capture_output=True, + check=True, + ) + return json.loads(result.stdout) + + +def test_hermes_base_home_named_profile_matches_cookie_without_doubling(tmp_path): + """R19k / #749: HERMES_BASE_HOME may point directly at a named profile home. + + A single-profile WebUI deployment can start with both HERMES_BASE_HOME and + HERMES_HOME set to /base/profiles/foo while the browser still sends the + logical cookie hermes_profile=foo. Both active-profile and explicit + per-request helpers must use /base/profiles/foo, not the doubled + /base/profiles/foo/profiles/foo path — even if that nested path already + exists from a prior bad write. + """ + profile_home = tmp_path / 'profiles' / 'foo' + doubled_home = profile_home / 'profiles' / 'foo' + doubled_home.mkdir(parents=True) + profile_home.joinpath('config.yaml').write_text( + 'terminal:\n cwd: /expected/profile-home\n', encoding='utf-8' + ) + doubled_home.joinpath('config.yaml').write_text( + 'terminal:\n cwd: /wrong/doubled-home\n', encoding='utf-8' + ) + + env = os.environ.copy() + env.update({ + 'HERMES_BASE_HOME': str(profile_home), + 'HERMES_HOME': str(profile_home), + }) + data = _run_profile_resolution_probe(env) + + assert data['default_home'] == str(tmp_path) + assert data['foo_home'] == str(profile_home) + assert data['explicit_foo_home'] == str(profile_home) + assert data['foo_terminal_cwd'] == '/expected/profile-home' + assert data['model_home'] == str(profile_home) + + +def test_hermes_base_home_named_profile_nonmatching_cookie_uses_sibling_profile_path(tmp_path): + """R19l / #749: non-matching cookies must not silently route to the pinned home. + + When HERMES_BASE_HOME is supplied as /base/profiles/foo but the request asks + for logical profile bar, preserving base semantics means bar resolves to the + sibling /base/profiles/bar. It must not fall back to foo, and it must not + append bar under foo/profiles/bar. + """ + profile_home = tmp_path / 'profiles' / 'foo' + profile_home.mkdir(parents=True) + + env = os.environ.copy() + env.update({'HERMES_BASE_HOME': str(profile_home)}) + data = _run_profile_resolution_probe(env) + + expected_bar_home = tmp_path / 'profiles' / 'bar' + assert data['explicit_bar_home'] == str(expected_bar_home) + assert data['active_bar_home'] == str(expected_bar_home) + + # ── R19e-h: new_session() profile isolation ─────────────────────────────────── # These tests call new_session() directly in-process. Session.save() would write # to SESSION_DIR which is set from HERMES_WEBUI_STATE_DIR at import time and may