Conversation
…n.branch NousResearch#67605 established the invariant that a profile secret scope is installed alongside every set_hermes_home_override() call site on the tui_gateway path, and converted session.create, session.resume (both the _make_agent and _init_session scopes), the lazy _build resume and the per-turn submit handler. session.branch was missed. The branch handler already binds the parent's HERMES_HOME and its profile-scoped state.db — its own comment says it mirrors session.create/resume — but builds the branched agent with no secret scope. get_secret() then falls through to process os.environ, which in an app-global/remote backend is the LAUNCH profile's environment: a session branched off profile X authenticates with profile Y's credentials. That is the same cross-profile resolution NousResearch#67605 fixed for the sibling paths, and it is silent (multiplexing is only activated by the messaging gateway, so the fail-closed UnscopedSecretError path never fires here). Install set_secret_scope(build_profile_secret_scope(parent_home)) for the build and release it in the same finally block as the home override.
Contributor
Collaborator
|
Thanks for finding a real remaining profile-isolation gap. The premise still holds on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
Open
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
#67605 (2db6b8c) established an explicit invariant for the tui_gateway path — quoting its own commit message:
and converted five:
compute_host._ensure_server_session,server._build, both scopes in_handle_resume_session, and_handle_submit_or_edit.session.branchwas missed.The branch handler already binds the parent's
HERMES_HOMEand its profile-scopedstate.db, and its own comment says it is "mirroring session.create/resume" — but it builds the branched agent with no secret scope:get_secret()with no scope installed falls through to processos.environ(agent/secret_scope.py, resolution rule 3). In the app-global / remote backend that serves several profiles, that environment belongs to the launch profile — so a session branched off profile X builds an agent holding profile Y's credentials: wrong API key, wrong account billed, and the profile isolation the switch advertises does not hold for the branch.It fails silently rather than loudly: the
UnscopedSecretErrorfail-closed branch only fires when multiplexing is active, andset_multiplex_active()is called only by the messaging gateway (gateway/run.py), never by tui_gateway.Reproduced against
main(f34a69b1c) — the new test captures the scope inside_make_agentduring a realsession.branchdispatch:scopeisNone: no profile scope at all during the branched build.The fix mirrors the resume site exactly — install the parent's scope for the build and release it in the same
finallyas the home override.Related Issue
No separate issue filed. Follow-up to #67605 / 2db6b8c, which fixed the sibling call sites; this completes the invariant it stated.
Fixes #
Type of Change
Changes Made
tui_gateway/server.py(session.branchhandler): installset_secret_scope(build_profile_secret_scope(Path(parent_home)))next to the existing home override, and reset it in the samefinallyblock. Uses the module-level imports already present for the sibling sites.tests/test_tui_gateway_server.py:test_session_branch_installs_parent_profile_secret_scope— dispatches a realsession.branchfor a session owned by a profile whose.envholds a marker credential and asserts the scope active during_make_agentis that profile's. Mirrors the existingtest_session_branch_writes_to_parent_profile_dbharness and the_start_agent_buildscope test added in f34a69b.How to Test
.envholding a different API key), and branch it.get_secret()reads processos.environ); after, it resolves profile B's.pytest tests/test_tui_gateway_server.py -k session_branch -q-> 2 passed. The new test fails onmainwithout the code change withassert None == {'PROXMOX_TOKEN': 'mlperf-secret'}(captured above).pytest tests/test_tui_gateway_server.py tests/tui_gateway/ -q-> 944 passed, 14 failed; those 14 (goal command, projects RPC, subagent child mirror) are pre-existing onmain— verified by re-running the same selection with this change stashed (943 passed, same 14 failures).Audited while here and deliberately left alone: the other
set_hermes_home_override()site without a paired scope in this file is_profile_scoped(line ~1184), which wraps onlypet.*andverification.statusRPCs — config/sprite-directory reads that resolve no credentials.Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
docs/, docstrings) — N/A (restores the documented per-profile credential behaviour; no contract change)cli-config.yaml.exampleif I added/changed config keys — N/A (no config keys added or changed)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A