fix(profiles): complete profile isolation via cookie + thread-local (#805)
Closes the gap left by #800. Full isolation via hermes_profile cookie + TLS. Co-authored-by: bergeouss <bergeouss@users.noreply.github.com>
This commit is contained in:
@@ -1,5 +1,10 @@
|
||||
# Hermes Web UI -- Changelog
|
||||
|
||||
## [v0.50.129] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
- **Profile isolation: complete fix via cookie + thread-local context** — PR #800 (v0.50.127) only fixed `POST /api/session/new`. `GET /api/profile/active` still read the process-level `_active_profile` global, so a page refresh while another client had a different profile active would corrupt `S.activeProfile` in JS, defeating the session-creation fix on the next new chat. This release completes the isolation: profile switches now set a `hermes_profile` cookie (HttpOnly, SameSite=Lax) and never mutate the process global. Every request handler reads the cookie into a thread-local; all server functions (`get_active_profile_name()`, `get_active_hermes_home()`, `list_profiles_api()`, memory endpoints, model loading) automatically see the per-client profile. `switch_profile()` gains a `process_wide` kwarg — the HTTP route passes `False`, keeping the global clean; CLI callers default to `True` (unchanged behaviour). Absorbed from PR #803 by @bergeouss with correctness fixes reviewed by Opus. (#804-not-yet — see PR #803 absorption)
|
||||
|
||||
## [v0.50.128] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -54,14 +54,21 @@ def _security_headers(handler):
|
||||
)
|
||||
|
||||
|
||||
def j(handler, payload, status: int=200) -> None:
|
||||
"""Send a JSON response."""
|
||||
def j(handler, payload, status: int=200, extra_headers: dict=None) -> None:
|
||||
"""Send a JSON response.
|
||||
|
||||
*extra_headers*: optional dict of additional headers to include
|
||||
(e.g., {'Set-Cookie': '...'}). Headers are sent before end_headers().
|
||||
"""
|
||||
body = _json.dumps(payload, ensure_ascii=False, indent=2).encode('utf-8')
|
||||
handler.send_response(status)
|
||||
handler.send_header('Content-Type', 'application/json; charset=utf-8')
|
||||
handler.send_header('Content-Length', str(len(body)))
|
||||
handler.send_header('Cache-Control', 'no-store')
|
||||
_security_headers(handler)
|
||||
if extra_headers:
|
||||
for k, v in extra_headers.items():
|
||||
handler.send_header(k, v)
|
||||
handler.end_headers()
|
||||
handler.wfile.write(body)
|
||||
|
||||
@@ -173,3 +180,48 @@ def read_body(handler) -> dict:
|
||||
return _json.loads(raw)
|
||||
except Exception:
|
||||
return {}
|
||||
|
||||
|
||||
# ── Profile cookie helpers (issue #798) ─────────────────────────────────────
|
||||
|
||||
PROFILE_COOKIE_NAME = 'hermes_profile'
|
||||
|
||||
|
||||
def get_profile_cookie(handler) -> str | None:
|
||||
"""Extract the hermes_profile cookie value from the request, or None."""
|
||||
cookie_header = handler.headers.get('Cookie', '')
|
||||
if not cookie_header:
|
||||
return None
|
||||
import http.cookies as _hc
|
||||
cookie = _hc.SimpleCookie()
|
||||
try:
|
||||
cookie.load(cookie_header)
|
||||
except _hc.CookieError:
|
||||
return None
|
||||
morsel = cookie.get(PROFILE_COOKIE_NAME)
|
||||
if morsel and morsel.value:
|
||||
# Validate against profile-name pattern before trusting
|
||||
from api.profiles import _PROFILE_ID_RE
|
||||
val = morsel.value
|
||||
if val == 'default' or _PROFILE_ID_RE.fullmatch(val):
|
||||
return val
|
||||
return None
|
||||
|
||||
|
||||
def build_profile_cookie(name: str) -> str:
|
||||
"""Build a Set-Cookie header value for the hermes_profile cookie.
|
||||
|
||||
name='default' clears the cookie (max-age=0).
|
||||
Any other valid profile name sets it for the browser session.
|
||||
httponly=True: the JS reads profile from /api/profile/active JSON, never
|
||||
from document.cookie, so httponly exposure is unnecessary.
|
||||
"""
|
||||
import http.cookies as _hc
|
||||
cookie = _hc.SimpleCookie()
|
||||
cookie[PROFILE_COOKIE_NAME] = '' if name == 'default' else name
|
||||
cookie[PROFILE_COOKIE_NAME]['path'] = '/'
|
||||
cookie[PROFILE_COOKIE_NAME]['httponly'] = True
|
||||
cookie[PROFILE_COOKIE_NAME]['samesite'] = 'Lax'
|
||||
if name == 'default':
|
||||
cookie[PROFILE_COOKIE_NAME]['max-age'] = '0'
|
||||
return cookie[PROFILE_COOKIE_NAME].OutputString()
|
||||
|
||||
101
api/profiles.py
101
api/profiles.py
@@ -31,6 +31,12 @@ _active_profile = 'default'
|
||||
_profile_lock = threading.Lock()
|
||||
_loaded_profile_env_keys: set[str] = set()
|
||||
|
||||
# Thread-local profile context: set per-request by server.py, cleared after.
|
||||
# Enables per-client profile isolation (issue #798) — each HTTP request thread
|
||||
# reads its own profile from the hermes_profile cookie instead of the
|
||||
# process-global _active_profile.
|
||||
_tls = threading.local()
|
||||
|
||||
def _resolve_base_hermes_home() -> Path:
|
||||
"""Return the BASE ~/.hermes directory — the root that contains profiles/.
|
||||
|
||||
@@ -86,15 +92,47 @@ def _read_active_profile_file() -> str:
|
||||
# ── Public API ──────────────────────────────────────────────────────────────
|
||||
|
||||
def get_active_profile_name() -> str:
|
||||
"""Return the currently active profile name."""
|
||||
"""Return the currently active profile name.
|
||||
|
||||
Priority:
|
||||
1. Thread-local (set per-request from hermes_profile cookie) — issue #798
|
||||
2. Process-level default (_active_profile)
|
||||
"""
|
||||
tls_name = getattr(_tls, 'profile', None)
|
||||
if tls_name is not None:
|
||||
return tls_name
|
||||
return _active_profile
|
||||
|
||||
|
||||
def set_request_profile(name: str) -> None:
|
||||
"""Set the per-request profile context for this thread.
|
||||
|
||||
Called by server.py at the start of each request when a hermes_profile
|
||||
cookie is present. Always paired with clear_request_profile() in a
|
||||
finally block so the thread-local is released after the request.
|
||||
"""
|
||||
_tls.profile = name
|
||||
|
||||
|
||||
def clear_request_profile() -> None:
|
||||
"""Clear the per-request profile context for this thread.
|
||||
|
||||
Called by server.py in the finally block of do_GET / do_POST.
|
||||
Safe to call even if set_request_profile() was never called.
|
||||
"""
|
||||
_tls.profile = None
|
||||
|
||||
|
||||
def get_active_hermes_home() -> Path:
|
||||
"""Return the HERMES_HOME path for the currently active profile."""
|
||||
if _active_profile == 'default':
|
||||
"""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 name == 'default':
|
||||
return _DEFAULT_HERMES_HOME
|
||||
profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / _active_profile
|
||||
profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / name
|
||||
if profile_dir.is_dir():
|
||||
return profile_dir
|
||||
return _DEFAULT_HERMES_HOME
|
||||
@@ -190,12 +228,18 @@ def init_profile_state() -> None:
|
||||
_reload_dotenv(home)
|
||||
|
||||
|
||||
def switch_profile(name: str) -> dict:
|
||||
def switch_profile(name: str, *, process_wide: bool = True) -> dict:
|
||||
"""Switch the active profile.
|
||||
|
||||
Validates the profile exists, updates process state, patches module caches,
|
||||
reloads .env, and reloads config.yaml.
|
||||
|
||||
Args:
|
||||
name: Profile name to switch to.
|
||||
process_wide: If True (default), updates the process-global
|
||||
_active_profile. Set to False for per-client switches from the
|
||||
WebUI where the profile is managed via cookie + thread-local (#798).
|
||||
|
||||
Returns: {'profiles': [...], 'active': name}
|
||||
Raises ValueError if profile doesn't exist or agent is busy.
|
||||
"""
|
||||
@@ -221,24 +265,41 @@ def switch_profile(name: str) -> dict:
|
||||
raise ValueError(f"Profile '{name}' does not exist.")
|
||||
|
||||
with _profile_lock:
|
||||
_active_profile = name
|
||||
_set_hermes_home(home)
|
||||
_reload_dotenv(home)
|
||||
if process_wide:
|
||||
global _active_profile
|
||||
_active_profile = name
|
||||
_set_hermes_home(home)
|
||||
_reload_dotenv(home)
|
||||
|
||||
# Write sticky default for CLI consistency
|
||||
try:
|
||||
ap_file = _DEFAULT_HERMES_HOME / 'active_profile'
|
||||
ap_file.write_text(name if name != 'default' else '', encoding='utf-8')
|
||||
except Exception:
|
||||
logger.debug("Failed to write active profile file")
|
||||
if process_wide:
|
||||
# Write sticky default for CLI consistency
|
||||
try:
|
||||
ap_file = _DEFAULT_HERMES_HOME / 'active_profile'
|
||||
ap_file.write_text(name if name != 'default' else '', encoding='utf-8')
|
||||
except Exception:
|
||||
logger.debug("Failed to write active profile file")
|
||||
|
||||
# Reload config.yaml from the new profile
|
||||
reload_config()
|
||||
# Reload config.yaml from the new profile
|
||||
reload_config()
|
||||
|
||||
# Return profile-specific defaults so frontend can apply them
|
||||
# Return profile-specific defaults so frontend can apply them.
|
||||
# For process_wide=False (per-client switch), read the target profile's
|
||||
# config.yaml directly from disk rather than from _cfg_cache (process-global),
|
||||
# since reload_config() was intentionally skipped.
|
||||
from api.workspace import get_last_workspace
|
||||
from api.config import get_config
|
||||
cfg = get_config()
|
||||
if process_wide:
|
||||
from api.config import get_config
|
||||
cfg = get_config()
|
||||
else:
|
||||
# Direct disk read — does not touch _cfg_cache
|
||||
try:
|
||||
import yaml as _yaml
|
||||
cfg_path = home / 'config.yaml'
|
||||
cfg = _yaml.safe_load(cfg_path.read_text(encoding='utf-8')) if cfg_path.exists() else {}
|
||||
if not isinstance(cfg, dict):
|
||||
cfg = {}
|
||||
except Exception:
|
||||
cfg = {}
|
||||
model_cfg = cfg.get('model', {})
|
||||
default_model = None
|
||||
if isinstance(model_cfg, str):
|
||||
@@ -263,7 +324,7 @@ def list_profiles_api() -> list:
|
||||
# hermes_cli not available -- return just the default
|
||||
return [_default_profile_dict()]
|
||||
|
||||
active = _active_profile
|
||||
active = get_active_profile_name()
|
||||
result = []
|
||||
for p in infos:
|
||||
result.append({
|
||||
|
||||
@@ -1153,11 +1153,15 @@ def handle_post(handler, parsed) -> bool:
|
||||
return bad(handler, "name is required")
|
||||
try:
|
||||
from api.profiles import switch_profile, _validate_profile_name
|
||||
|
||||
from api.helpers import build_profile_cookie
|
||||
if name != 'default':
|
||||
_validate_profile_name(name)
|
||||
result = switch_profile(name)
|
||||
return j(handler, result)
|
||||
# process_wide=False: don't mutate the process-global _active_profile.
|
||||
# Per-client profile is managed via cookie + thread-local (#798).
|
||||
result = switch_profile(name, process_wide=False)
|
||||
return j(handler, result, extra_headers={
|
||||
'Set-Cookie': build_profile_cookie(name),
|
||||
})
|
||||
except (ValueError, FileNotFoundError) as e:
|
||||
return bad(handler, _sanitize_error(e), 404)
|
||||
except RuntimeError as e:
|
||||
|
||||
15
server.py
15
server.py
@@ -15,7 +15,8 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
from api.auth import check_auth
|
||||
from api.config import HOST, PORT, STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE
|
||||
from api.helpers import j
|
||||
from api.helpers import j, get_profile_cookie
|
||||
from api.profiles import set_request_profile, clear_request_profile
|
||||
from api.routes import handle_get, handle_post
|
||||
from api.startup import auto_install_agent_deps, fix_credential_permissions
|
||||
from api.updates import WEBUI_VERSION
|
||||
@@ -64,6 +65,10 @@ class Handler(BaseHTTPRequestHandler):
|
||||
|
||||
def do_GET(self) -> None:
|
||||
self._req_t0 = time.time()
|
||||
# Per-request profile context from cookie (issue #798)
|
||||
cookie_profile = get_profile_cookie(self)
|
||||
if cookie_profile:
|
||||
set_request_profile(cookie_profile)
|
||||
try:
|
||||
parsed = urlparse(self.path)
|
||||
if not check_auth(self, parsed): return
|
||||
@@ -73,9 +78,15 @@ class Handler(BaseHTTPRequestHandler):
|
||||
except Exception as e:
|
||||
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||
return j(self, {'error': 'Internal server error'}, status=500)
|
||||
finally:
|
||||
clear_request_profile()
|
||||
|
||||
def do_POST(self) -> None:
|
||||
self._req_t0 = time.time()
|
||||
# Per-request profile context from cookie (issue #798)
|
||||
cookie_profile = get_profile_cookie(self)
|
||||
if cookie_profile:
|
||||
set_request_profile(cookie_profile)
|
||||
try:
|
||||
parsed = urlparse(self.path)
|
||||
if not check_auth(self, parsed): return
|
||||
@@ -85,6 +96,8 @@ class Handler(BaseHTTPRequestHandler):
|
||||
except Exception as e:
|
||||
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||
return j(self, {'error': 'Internal server error'}, status=500)
|
||||
finally:
|
||||
clear_request_profile()
|
||||
|
||||
|
||||
def main() -> None:
|
||||
|
||||
184
tests/test_issue803.py
Normal file
184
tests/test_issue803.py
Normal file
@@ -0,0 +1,184 @@
|
||||
"""
|
||||
Issue #803 (completes #798) — per-client profile isolation via cookie + thread-local.
|
||||
|
||||
PR #800 fixed POST /api/session/new (client sends profile in body).
|
||||
PR #805 extends the fix to ALL endpoints: profile switches set a hermes_profile
|
||||
cookie, server.py reads it per-request into a thread-local, and the existing
|
||||
api/profiles.py helpers consult the thread-local before the process global.
|
||||
|
||||
Covers:
|
||||
1. build_profile_cookie() / get_profile_cookie() roundtrip + validation
|
||||
2. set_request_profile() / get_active_profile_name() / clear_request_profile()
|
||||
3. get_active_hermes_home() routes via thread-local
|
||||
4. switch_profile(process_wide=False) does NOT mutate process globals
|
||||
5. Concurrent requests on different threads see independent profiles
|
||||
"""
|
||||
import os
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ── 1. Cookie build/parse roundtrip ──────────────────────────────────────────
|
||||
|
||||
class TestProfileCookieHelpers:
|
||||
|
||||
def test_build_profile_cookie_sets_value(self):
|
||||
from api.helpers import build_profile_cookie
|
||||
s = build_profile_cookie('alice')
|
||||
assert 'hermes_profile=alice' in s
|
||||
assert 'HttpOnly' in s
|
||||
assert 'SameSite=Lax' in s
|
||||
assert 'Path=/' in s
|
||||
|
||||
def test_build_profile_cookie_default_clears(self):
|
||||
from api.helpers import build_profile_cookie
|
||||
s = build_profile_cookie('default')
|
||||
assert 'Max-Age=0' in s
|
||||
# Empty value indicates clear
|
||||
assert 'hermes_profile=""' in s or 'hermes_profile=;' in s
|
||||
|
||||
def test_get_profile_cookie_returns_none_when_absent(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': ''
|
||||
assert get_profile_cookie(handler) is None
|
||||
|
||||
def test_get_profile_cookie_extracts_valid_name(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': 'hermes_profile=alice' if k == 'Cookie' else d
|
||||
assert get_profile_cookie(handler) == 'alice'
|
||||
|
||||
def test_get_profile_cookie_accepts_default(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': 'hermes_profile=default' if k == 'Cookie' else d
|
||||
assert get_profile_cookie(handler) == 'default'
|
||||
|
||||
def test_get_profile_cookie_rejects_injection(self):
|
||||
"""Cookie value must pass _PROFILE_ID_RE fullmatch — rejects traversal/injection."""
|
||||
from api.helpers import get_profile_cookie
|
||||
for bad in ('../etc', 'a/b', 'name;DROP', 'WithCaps', 'has space', '.hidden'):
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='', v=bad: f'hermes_profile={v}' if k == 'Cookie' else d
|
||||
assert get_profile_cookie(handler) is None, f"{bad!r} should be rejected"
|
||||
|
||||
def test_get_profile_cookie_ignores_malformed_header(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': '\x00\x01not-a-cookie' if k == 'Cookie' else d
|
||||
# Must not raise; returns None
|
||||
result = get_profile_cookie(handler)
|
||||
assert result is None
|
||||
|
||||
|
||||
# ── 2. Thread-local request context ──────────────────────────────────────────
|
||||
|
||||
class TestThreadLocalProfileContext:
|
||||
|
||||
def test_tls_takes_priority_over_global(self):
|
||||
import api.profiles as p
|
||||
original = p._active_profile
|
||||
try:
|
||||
p._active_profile = 'global-default'
|
||||
p.set_request_profile('alice')
|
||||
assert p.get_active_profile_name() == 'alice'
|
||||
finally:
|
||||
p.clear_request_profile()
|
||||
p._active_profile = original
|
||||
|
||||
def test_global_used_when_tls_cleared(self):
|
||||
import api.profiles as p
|
||||
original = p._active_profile
|
||||
try:
|
||||
p._active_profile = 'global-default'
|
||||
p.set_request_profile('alice')
|
||||
p.clear_request_profile()
|
||||
assert p.get_active_profile_name() == 'global-default'
|
||||
finally:
|
||||
p._active_profile = original
|
||||
|
||||
def test_clear_is_idempotent(self):
|
||||
import api.profiles as p
|
||||
# Calling clear on a thread that never set anything must not raise
|
||||
p.clear_request_profile()
|
||||
p.clear_request_profile()
|
||||
|
||||
|
||||
# ── 3. get_active_hermes_home routes through TLS ─────────────────────────────
|
||||
|
||||
def test_get_active_hermes_home_respects_tls(tmp_path, monkeypatch):
|
||||
import api.profiles as p
|
||||
monkeypatch.setattr(p, '_DEFAULT_HERMES_HOME', tmp_path)
|
||||
profile_dir = tmp_path / 'profiles' / 'alice'
|
||||
profile_dir.mkdir(parents=True)
|
||||
try:
|
||||
p.set_request_profile('alice')
|
||||
assert p.get_active_hermes_home() == profile_dir
|
||||
p.set_request_profile('default')
|
||||
assert p.get_active_hermes_home() == tmp_path
|
||||
finally:
|
||||
p.clear_request_profile()
|
||||
|
||||
|
||||
# ── 4. switch_profile(process_wide=False) does not mutate globals ─────────────
|
||||
|
||||
def test_switch_profile_process_wide_false_does_not_mutate_global():
|
||||
"""Per-client switches from the WebUI must leave _active_profile untouched."""
|
||||
import api.profiles as p
|
||||
|
||||
# Monkey in a fake profile listing so switch_profile finds 'alice'
|
||||
original_global = p._active_profile
|
||||
original_env_home = os.environ.get('HERMES_HOME')
|
||||
|
||||
# We need a profile that exists to get past the validation path.
|
||||
# Use 'default' — switch_profile accepts it without requiring hermes_cli.
|
||||
try:
|
||||
result = p.switch_profile('default', process_wide=False)
|
||||
# Global must not change
|
||||
assert p._active_profile == original_global, (
|
||||
f"process_wide=False must not mutate _active_profile "
|
||||
f"(was {original_global!r}, now {p._active_profile!r})"
|
||||
)
|
||||
# HERMES_HOME env must not change
|
||||
assert os.environ.get('HERMES_HOME') == original_env_home, (
|
||||
"process_wide=False must not mutate os.environ['HERMES_HOME']"
|
||||
)
|
||||
# Response still shape-compatible
|
||||
assert isinstance(result, dict)
|
||||
finally:
|
||||
p._active_profile = original_global
|
||||
|
||||
|
||||
# ── 5. Concurrent threads see independent profile context ────────────────────
|
||||
|
||||
def test_concurrent_threads_see_independent_profiles():
|
||||
"""The whole point of thread-local isolation: two threads, two cookies,
|
||||
two different get_active_profile_name() results, simultaneously."""
|
||||
import api.profiles as p
|
||||
|
||||
results = {}
|
||||
errors = []
|
||||
barrier = threading.Barrier(2, timeout=5)
|
||||
|
||||
def worker(name, key):
|
||||
try:
|
||||
p.set_request_profile(name)
|
||||
barrier.wait() # both threads have set their TLS
|
||||
# Now each thread reads — must see its own value
|
||||
results[key] = p.get_active_profile_name()
|
||||
p.clear_request_profile()
|
||||
except Exception as exc:
|
||||
errors.append(exc)
|
||||
|
||||
t1 = threading.Thread(target=worker, args=('alice', 'alice'))
|
||||
t2 = threading.Thread(target=worker, args=('bob', 'bob'))
|
||||
t1.start(); t2.start()
|
||||
t1.join(timeout=10); t2.join(timeout=10)
|
||||
|
||||
assert not errors, f"Workers raised: {errors}"
|
||||
assert results.get('alice') == 'alice', f"alice thread saw {results.get('alice')!r}"
|
||||
assert results.get('bob') == 'bob', f"bob thread saw {results.get('bob')!r}"
|
||||
Reference in New Issue
Block a user