Multi-profile serve fails closed: secondary profiles never read the launch profile's secrets; default-member hosted rooms no longer crash - #111620
Conversation
…me; profile RPCs bind the full runtime scope
`hermes serve` / the Desktop backend hosted many profile homes (session
profile_home, the `profile` RPC param, hosted rooms) but never called
agent.secret_scope.set_multiplex_active, so every unscoped get_secret read for
a secondary silently returned the LAUNCH profile's os.environ value, and
@_profile_scoped bound only HERMES_HOME: `config.get full` for profile B
expanded B's `${VAR}` refs to the default profile's plaintext credentials,
model.options listed the default's env-keyed providers, llm.oneshot billed the
default's auxiliary key.
- tui_gateway/launch_profile_policy.py (was launch_terminal_policy.py): the
first time _profile_home registers a non-launch home the process freezes
the launch env and flips get_secret to fail closed
(activate_multi_profile_hosting); launch_secret_scope composes the launch
profile's .env + external sources over that frozen env so systemd / op-run
injection survives the flip while a secondary never sees it.
- model_switch._profile_runtime_scope_tokens is the ONE composer for
home + secret + terminal scope: a named profile binds its own files; the
launch profile binds its frozen-env scope once multiplexing is active and
stays unscoped in a single-profile process (legacy os.environ precedence).
_profile_scoped, _profile_scoped_rpc, _session_profile_runtime_scope,
_bind_build_profile_scopes and _prepare_turn_input all go through it.
- Hosted-room / Group Chat turns for a DEFAULT-profile member in a
`multiplex_profiles: true` gateway no longer die at agent build with
UnscopedSecretError: `profile_home is None` was treated as "no scope"
in _start_agent_build._build and _prepare_turn_input.
- llm.oneshot runs under the session's (or params.profile's) scope;
_lap_builtin_rows / _overlay_has_creds / _provider_has_credentials read
provider keys through _scoped_key_env instead of raw os.environ;
methods_groups._profile_execution_policy resolves the hosted-room policy
(which reads provider credentials) under the profile's full scope.
Live repro (real `hermes serve`, two homes, config.get {key: full, profile: b}):
base a_ref: <A_VALUE> b_ref: ${B_ONLY_TOKEN} env_ref: <ENV_INJECTED>
head a_ref: ${A_ONLY_TOKEN} b_ref: <B_VALUE> env_ref: ${ENV_INJECTED_TOKEN}
Control (one home, --single): launch config still resolves env_ref from os.environ.
… scope; console `send` never writes the process env
_config_profile_scope bound only HERMES_HOME, so GET /api/config?profile=B
expanded B's `${VAR}` refs to the dashboard (DEFAULT) profile's plaintext
credentials, WS /api/console commands for B saw the default's keys wherever B
lacked one, and the audio speak-stream synthesis thread re-resolved the TTS key
unscoped. Console `send` for B went further: send_cmd._load_hermes_env copied
B's .env into the shared os.environ with override=True, so every later
default-profile read saw B's tokens.
- web_server_profiles._config_profile_scope binds home + hydrated secret scope
for a named profile and flips the process to fail-closed multi-profile
hosting (same activation as the tui_gateway); the dashboard's own profile then
runs under its frozen-launch-env scope. A single-profile dashboard stays
unscoped (systemd / op-run credential injection keeps working).
- GET /api/tools/toolsets computes the per-toolset "configured" flag inside
the profile scope (it was read after the scope closed).
- web_routers/mcp._profile_secret_scope now just delegates (no second copy of
the composition); audio `_produce` runs the whole synthesis body under the
requesting profile's scope, not only the resolve step.
- send_cmd._load_hermes_env targets the installed secret scope when one is
active (gateway.config._getenv reads the scope first), os.environ only for
the standalone CLI.
…ll-scope profile RPCs / routers tests/tui_gateway/test_multi_profile_hosting_fail_closed.py — through the registered RPC handlers: config.get for a secondary resolves only its own secrets and flips the process fail-closed; llm.oneshot / model.options bodies see the profile's home + secrets; a launch-profile agent build binds its own scope once multiplexing is active; CONTROL: a single-profile serve keeps the os.environ fall-through. tests/hermes_cli/test_web_multi_profile_scope.py — through the real FastAPI app: GET /api/config?profile=b expands only B's refs and never mutates os.environ; console `send` for B lands B's .env in the request scope, not the process env. All red on origin/main by source swap (control green), green on head. test_profile_terminal_scope_entrypoints follows the launch_profile_policy rename and releases the launch turn's new secret scope. tests/conftest.py resets the process-global hosting latch (_MULTIPLEX_ACTIVE, the frozen launch env, _served_profile_homes) per test: one request routed to a named profile otherwise left every later test in the file fail-closed.
૮ >ﻌ< ა ci reviewran on f5be58b — test(multiplex): invariant tests for fail-closed serve hosti
|
alt-glitch
added
type/bug
kvnloo
left a comment
kvnloo
left a comment
There was a problem hiding this comment.
Verified the mechanics against the base code — this is a well-constructed fix and the live repro in the body checks out. A few things I confirmed sound:
_profile_homeis the only writer of_served_profile_homes, so the freeze inactivate_multi_profile_hosting()genuinely happens before any secondary handler body runs — the "last moment ambient env is provably the launch profile's" claim holds for the RPC paths._config_profile_scopecovers the dashboard's own profile under multiplexing via theelif is_multiplex_active()branch (installs the frozen-launch scope, not nothing) — I initially misread this as a hole; it isn't.methods_toolsnow routes through_profile_home, which matters: the old code resolved the profile dir directly and never flipped the process to fail-closed for tools RPCs. That's a real hole closed, not just a refactor._scoped_key_env's fail-closed-then-""semantics are right for the picker paths.
Two things worth addressing:
1. Release path got weaker (concrete). _release_build_profile_scopes used to reset secret scope and terminal scope each under their own suppress; now the whole _release_profile_runtime_scope_tokens — terminal → secret → home, sequential — sits inside a single suppress. If reset_terminal_scope raises, the secret scope and the HERMES_HOME override are never reset and leak into the surrounding context: a later turn in that context reads the previous profile's secrets. That's a fail-open scope leak in a fail-closed PR. Cheap fix: move the per-reset suppress inside _release_profile_runtime_scope_tokens so each reset is independent.
2. The freeze is a behavior change for the launch profile (question). Pre-PR, launch-profile turns in serve read live os.environ (multiplexing was never active there). Post-PR, once the first secondary registers, launch turns use the env frozen at activation, never re-read. So credential rotation without a process restart — systemctl set-environment, a refreshed op run wrapper that doesn't re-exec — silently stops working for the launch profile the moment someone uses ?profile=. The docstring says "never re-read afterwards" deliberately, and I buy the security reasoning, but the restart requirement (or a SIGHUP re-freeze) should be documented somewhere an operator will find it, otherwise this surfaces as mysterious 401s.
Nits:
send_cmd._load_hermes_envupdates the installed scope withoverride=Truesemantics for.envwhile the YAML bridge below it uses setdefault — harmless since the scope was already built from that.env, just noting the asymmetry.- The 243 new test lines cover the fail-closed flip and the RPC scoping well.
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head f5be58b0914e21b0b6b1734e0463583f00f3637b against PR base / actual merge base 4d55ca91656ac5f83e1506679b7f81e0238e5e16 and current main d128ce2e25634105c81dfcc9c7a1678d6c1db038. This is a high-value authority boundary and the central direction is good: first-secondary activation turns ambient credential fallback into fail-closed behavior, the launch profile gets an explicit frozen launch scope after activation, TUI/RPC paths converge on one home+secret+terminal composer, dashboard config/tool/audio paths now resolve credentials inside the routed profile, and the TTS producer thread no longer falls back to the dashboard process environment. Those are meaningful closures of the #107422 / #109417 class and should be preserved.
I found two additional blockers that are not covered by the existing review, and I independently confirmed the existing teardown blocker from review #5206650726.
P1 — send reverses the profile secret authority order
The scoped branch in _load_hermes_env() takes the already-authoritative installed profile scope and then update()s it from the raw user .env. That scope is not just a parsed copy of .env: build_profile_secret_scope() intentionally composes user .env first, then external secret-source values, then administrator-managed env last. Replaying .env after that construction lets a stale/user-controlled profile value replace the external/managed value for the rest of this send request.
That means this patch closes the cross-profile process-env leak but can simultaneously downgrade the within-profile credential authority chain. The scoped path should keep the installed scope authoritative, or refresh/rebuild it through the canonical scope builder rather than re-overlaying raw .env. The required inverse witness is straightforward: put the same key in B's .env and in an external or managed source with different values, enter B's request scope, call _load_hermes_env(), and prove the external/managed value still wins. Inline finding attached.
P2 — an in-flight launch request can cross the first-secondary activation boundary unscoped
Both common composers decide whether the launch/default profile needs a scope once, at entry, while get_secret() consults the live process-global multiplex bit on every later read. A launch request/turn can therefore enter while _MULTIPLEX_ACTIVE is false and remain intentionally unscoped; a concurrent first request for profile B then calls activate_multi_profile_hosting() and flips the global bit; when the already-running launch request next reads a credential, get_secret() now sees multiplex active + no scope and raises UnscopedSecretError.
The dashboard has the same transition shape in _config_profile_scope() (secrets=None while single-profile, then another request activates globally), and activation happens before hydrate_profile_secret_sources(scoped), so there is a real scheduling window rather than a purely theoretical instruction interleave. This is fail-closed rather than cross-tenant leakage, but it can still crash exactly the default-profile activity this PR is trying to keep alive. Please pin launch/default scope semantics for the full request/turn across the first-secondary transition and add a deterministic barrier test: launch enters while single-profile, B activates, launch resumes and still resolves its own credential. Inline finding attached.
Existing blocker still stands — cleanup is no longer independently fail-safe
I also walked the release path raised in review #5206650726 and agree with it, so I am not duplicating that inline thread. _release_profile_runtime_scope_tokens() resets terminal → secret → home sequentially, while _release_build_profile_scopes() suppresses only around the whole helper. If the first reset raises, the later secret/home resets are skipped. A teardown fault can therefore leave the previous profile's secret/HERMES_HOME context installed into subsequent work. Each reset needs independent best-effort cleanup (while preserving the desired ordering), with a test that forces one reset to fail and proves the remaining scopes are still released.
Topology / ownership
- #108440 / #107422 is the merged terminal-isolation foundation and provenance, not duplicate work. #111620 correctly generalizes that launch snapshot/fail-closed shape beyond
TERMINAL_*into profile secrets and routed host surfaces; preserve the earlier community lineage there. - #109417 is the campaign-level multiplex-safety owner: effective config/env/CWD/terminal/MCP/capabilities and every profile-sensitive context have to stay profile-qualified and unwind in
finally. - #111481 is open complementary routed-profile/MCP identity work for the Desktop/dashboard topology where explicit
HERMES_HOMErouting exists even with the traditional multiplex flag off. It should compose with this carrier rather than have its authorship collapsed into it. - #111617 is open complementary child-process/background-thread/PID-authority hardening. #111620 explicitly leaves
_SlashWorker,tools/, title generation, and lifecycle seams to that sibling lane, so this PR should not be treated as repository-wide class closure by itself. - The three surviving commits in #111620 are all authored by
teknium1; I found no salvage/attribution collision in this carrier.
Acceptance / merge order
Exact-head hosted acceptance is green: CI 34938788003, Docker 34938787480, and Nix 34938787432 all succeeded on f5be58b0914e21b0b6b1734e0463583f00f3637b. However, surviving commits 02ee7adcd13432e7ecff9faef809cb3c3d31dd97 and eeb495137e14112192be0b6a396029b220798549 have no workflow runs attached, so this carrier is exact-head green but not every-surviving-commit proven green.
The branch is now 3 commits ahead / 26 behind current main from merge base 4d55ca9…. Before merge, recompose on current main, close the two findings above plus the independently confirmed teardown blocker, then require CI/Docker/Nix green on the exact successor head and every surviving commit. The implementation is doing the right architectural work; these are the remaining authority/transition edges I would want closed before treating the boundary as settled.
| target: dict = scope | ||
| env_path = home / ".env" | ||
| if env_path.exists(): | ||
| target.update(load_env_file(env_path)) |
There was a problem hiding this comment.
P1 — preserve managed/external secret authority here. The installed scope is not merely a parsed copy of this .env: build_profile_secret_scope() intentionally applies external secret-source values after user .env, then administrator-managed env last. Replaying raw .env into the installed dict reverses that precedence for send, so a stale/user-controlled profile value can replace the secret-manager or managed value for the rest of this request. In scoped mode, keep the already-built scope authoritative (or rebuild/refresh through the canonical scope builder); don't update() it from raw .env. Please add an inverse test with the same key in user .env and an external/managed source and prove the external/managed value survives _load_hermes_env().
There was a problem hiding this comment.
Fixed on head 42e01d9 (#112685, d3bded9dc5523): the scoped branch no longer replays raw .env; the composed scope is authoritative — test_multi_profile_hosting_transitions.py::test_send_keeps_external_source_value_over_raw_dotenv (same key in .env and an external source; manager value survives _load_hermes_env; red on base).
| secrets = build_profile_secret_scope(home) | ||
| overlay = None | ||
| scopes.home = set_hermes_home_override(str(home)) | ||
| elif _launch_profile_scope_needed(): |
There was a problem hiding this comment.
P2 — pin the launch scope across the first-secondary transition. This branch snapshots a live process-global state only at scope entry. A launch request can enter while it is false and remain unscoped; a concurrent first secondary then runs activate_multi_profile_hosting() and flips _MULTIPLEX_ACTIVE=True; the in-flight launch request's next get_secret() consults the new global and now raises UnscopedSecretError because its scope stayed None. The dashboard composer has the same shape in _config_profile_scope(). Please make launch/default scope selection stable for the whole request/turn and add a barrier test: launch enters single-profile, B activates, launch resumes and still resolves its own credential instead of crashing.
There was a problem hiding this comment.
Fixed on head 42e01d9 (#112685, d3bded9dc5523): launch-profile bodies always bind the launch-env secret scope (live env single-profile, frozen once multiplexing), so the source is decided at entry; same for the dashboard _config_profile_scope — …::test_launch_body_survives_first_secondary_activation (+ _on_the_dashboard): launch enters, B activates on another thread, launch resumes with its own credential (red on base).
…s no secret scope get_secret() already reads os.environ in a single-profile process; it raises UnscopedSecretError only when multi-profile hosting is active and the call has no profile scope. _nous_inference_env_override and _nous_portal_env_override caught that and read the ambient env anyway, which is the launch profile's value — so a routed call that lost its scope could POST a secondary's refresh token to the launch profile's Portal, or send its inference bearer to the launch profile's host. Since #111620 the launch profile itself carries a frozen-env scope while hosting, so the exception never means "single-profile CLI"; it means the caller has no authority over that value. Both readers now go through _scoped_operator_override, which returns None in that state — the override is absent, the stored/default routing applies, nothing raises (pricing, runtime_provider and the guest path rely on these never raising). Raised on #111809 review. One parametrized invariant test over both readers: environ when unscoped single-profile, the scope's value under hosting, None on a scoped miss, None with no scope; the no-scope leg is red on main.
…s no secret scope get_secret() already reads os.environ in a single-profile process; it raises UnscopedSecretError only when multi-profile hosting is active and the call has no profile scope. _nous_inference_env_override and _nous_portal_env_override caught that and read the ambient env anyway, which is the launch profile's value — so a routed call that lost its scope could POST a secondary's refresh token to the launch profile's Portal, or send its inference bearer to the launch profile's host. Since NousResearch#111620 the launch profile itself carries a frozen-env scope while hosting, so the exception never means "single-profile CLI"; it means the caller has no authority over that value. Both readers now go through _scoped_operator_override, which returns None in that state — the override is absent, the stored/default routing applies, nothing raises (pricing, runtime_provider and the guest path rely on these never raising). Raised on NousResearch#111809 review. One parametrized invariant test over both readers: environ when unscoped single-profile, the scope's value under hosting, None on a scoped miss, None with no scope; the no-scope leg is red on main.
|
Follow-up: #112685 (head
Recomposed on current main (merge |
|
Follow-up: #112685 (head
|
…uthority; per-reset release Three edges of the fail-closed multi-profile host (#111620 review, andrexibiza P1 + P2, kvnloo finding 1): - `send` under a routed profile's scope `update()`d the installed scope from raw `.env`, reversing build_profile_secret_scope's precedence (user .env, then external secret sources) for the rest of the request; a stale user value beat the secret-manager one. The installed scope is authoritative as-is; only the config.yaml setdefault bridge runs. - The launch profile's body was scoped only when `is_multiplex_active()` was already true at entry, while get_secret consults that global on every read. A launch RPC / dashboard request entering single-profile and resuming after a concurrent first `?profile=B` activation raised UnscopedSecretError mid-request. The launch profile's secret scope (its .env + external sources over the launch env: live while single-profile, the frozen snapshot once multiplexing is active) is now bound for every launch-profile body, so the credential source is fixed at entry. The terminal policy overlay stays multiplex-only (standalone terminal execution keeps its os.environ bridge). _publish_env_value mirrors a same-request .env write into that scope AND os.environ for the launch profile, only into the scope for a routed one (serves_routed_profile). - _release_profile_runtime_scope_tokens reset terminal → secret → home in sequence under one outer suppress; a failing terminal reset left the previous profile's secrets and HERMES_HOME installed for the next body in that context. Each reset is now independent; the first failure is re-raised after every scope is released. tests/tui_gateway/test_multi_profile_hosting_transitions.py: manager-vs-dotenv precedence through _load_hermes_env, TUI-RPC and dashboard barrier tests (launch enters single-profile, B activates on another thread, launch resumes and still resolves its injected credential, never B's), forced terminal-reset failure still releases secret + home. 4/4 red on base.
…nv restart requirement kvnloo (#111620 review) asked for the operator-visible statement that once a serve / dashboard process hosts a second profile, the launch profile's env-only credentials are frozen at activation and a rotation in the process env needs a restart. Also states the routed-child rule with and without the multiplex flag (#111617). Adds encoding='utf-8' to the probe child in the child-env authority test (windows-footguns lint).
4 participants
A
hermes serve/ Desktop backend that hosts a second profile home now fails closed: an unscoped credential read for a secondary raises instead of silently returning the launch profile'sos.environvalue, every profile-routed RPC and dashboard router runs under that profile's full runtime scope, and a default-profile Group Chat member no longer crashes at agent build undermultiplex_profiles: true.Problem
agent.secret_scope.get_secretonly fails closed whileset_multiplex_active(True)holds, and only the messaging gateway / cron worker ever flipped it.hermes serve(Desktop, dashboard,?profile=, hosted rooms) hosts many profile homes with the flag off, and@_profile_scoped/_config_profile_scopebound HERMES_HOME only. Soconfig.get {key: full, profile: b}andGET /api/config?profile=bshipped B's${VAR}refs expanded to the default profile's plaintext credentials,model.optionslisted the default's env-keyed providers,llm.oneshotbilled the default's auxiliary key, and consolesendfor B wrote B's.envinto the shared process env withoverride=True. The inverse asymmetry in the real multiplex gateway: a hosted-room turn for the default member hadprofile_home=None, was treated as "no scope", and died withUnscopedSecretErrorat agent build.Changes
tui_gateway/launch_profile_policy.py(renamed fromlaunch_terminal_policy.py):activate_multi_profile_hosting()runs the first time_profile_homeregisters a non-launch home — freezes the launch env and flipsget_secretfail-closed.launch_secret_scope()= launch.env+ external sources over that frozen env, so systemd /op runinjection survives the flip while a secondary never sees it.model_switch._profile_runtime_scope_tokensis the ONE composer (home + secret + terminal). Named profile → its own files; launch profile → frozen-env scope once multiplexing is active, unscoped in a single-profile process._profile_scoped,_profile_scoped_rpc,_session_profile_runtime_scope,_bind_build_profile_scopes,_prepare_turn_input,methods_groups._profile_execution_policyall go through it.llm.oneshotis@_profile_scopedand wrapsrun_oneshotin the session's scope;_lap_builtin_rows/_overlay_has_creds/models._provider_has_credentialsread keys via_scoped_key_env._config_profile_scopebinds home + hydrated secret scope for a named profile and activates hosting;web_routers/mcp._profile_secret_scopedelegates instead of duplicating; audio_produceruns the whole synthesis body scoped;/api/tools/toolsetscomputesconfiguredinside the scope;send_cmd._load_hermes_envtargets the installed secret scope (neveros.environ) when one is active.tests/conftest.pyresets the process-global hosting latch per test.Not touched (sibling lane owns them):
tui_gateway/server.py::_SlashWorkerspawn env,tools/,agent/title_generator.py,tui_gateway/session_lifecycle.py. The/api/status?profile=row from the audit is inlane4_latent_bugs(YAML→env bridge viaload_gateway_config) and is covered here indirectly:_config_profile_scopenow installs a secret scope, soyaml_env_setter's "skip under secondary scope" guard engages.Per-row verdicts (
/tmp/mux_retro/lane4_high.json)hermes_cli/web_server.py78-125activate_multi_profile_hostingon first non-launch home (_profile_home,_config_profile_scope)tui_gateway/methods_config.py228config.getHOME-only,${VAR}expansion from launch env_profile_scopedbinds full scopetui_gateway/methods_complete.py278model.optionsHOME-only + rawos.environprovider checks_scoped_key_envin_lap_builtin_rows/_overlay_has_creds/_provider_has_credentialstui_gateway/methods_session.py1081llm.oneshotunscoped@_profile_scoped+ session scope aroundrun_oneshottui_gateway/prompt_turn.py447-464 /server.py_start_agent_build._buildhermes_cli/web_routers/config_env.py77-88GET /api/config?profile=home-only_config_profile_scopebinds secretshermes_cli/web_routers/chat_ws.py266-337WS /api/console?profile=home-only;sendmutatesos.environ_profile_scopecomposes on_config_profile_scope;_load_hermes_envtargets the scopehermes_cli/web_routers/audio.py431-483_producethread re-resolves TTS key unscoped_config_profile_scope(profile)hermes_cli/web_routers/audio.py58-424_config_profile_scope, which now binds secretsLive repro (real
hermes serve, two homes, JSON-RPCconfig.get {key: full, profile: b}; values redacted)Hosted room,
multiplex_profiles: true,session.create profile=defaultviaHostedRoomServerRPCthen agent build:Tests
tests/tui_gateway/test_multi_profile_hosting_fail_closed.py(4): secondaryconfig.getresolves only its own secrets and flips fail-closed;llm.oneshot/model.optionsbodies see the profile's home + secrets; launch-profile agent build binds its own scope once multiplexing is active; control — single-profile serve keeps theos.environfall-through.tests/hermes_cli/test_web_multi_profile_scope.py(2, through the real FastAPI app):GET /api/config?profile=bexpands only B's refs and leavesos.environuntouched; consolesendfor B lands in the request scope, not the process env.origin/mainby source swap (control green), green on head.scripts/run_tests.sh tests/tui_gateway tests/gateway tests/hermes_cli/test_web_* tests/hermes_cli/test_send_cmd.py tests/hermes_cli/test_models.py tests/hermes_cli/test_model_switch_providers.py: 1034 files, 10882 passed, 0 failed.tests/hermes_cli/test_web* test_profiles* test_env_loader* tests/agent/test_secret_scope*: 676 passed, 0 failed (one pre-existing 1s wall-clock flake intest_profiles_sidebar_cache.py, untouched here, passed on retry).Infographic