fix(security): re-resolve profile-scoped credential/session paths per call - #56302
Open
srojk34 wants to merge 1 commit into
Open
fix(security): re-resolve profile-scoped credential/session paths per call#56302srojk34 wants to merge 1 commit into
srojk34 wants to merge 1 commit into
Conversation
… call agent/anthropic_adapter.py's Hermes OAuth file, agent/auxiliary_client.py's Nous Portal auth.json, and gateway/mirror.py's sessions.json index are all resolved once at import time via get_hermes_home(), which is a context-local ContextVar under the multiplexed gateway (multiple profiles sharing one process). Freezing the path at import time pins every later request to whichever profile's HERMES_HOME was active when the module was first imported, leaking OAuth/portal credentials and session lookups across profiles (read AND write, since hermes_cli/web_server.py's OAuth save/disconnect handlers use the same frozen constant) -- the same bug class already fixed for cache dirs, skills_hub, and rich_sent_store. Add a per-call resolver for each path, following the established "respect an existing test monkeypatch of the constant, otherwise re-resolve through get_hermes_home()" pattern so existing test seams in tests/hermes_cli/test_web_server_oauth_write.py and tests/gateway/test_mirror.py keep working unmodified.
alt-glitch
added
type/security
Contributor
|
Thanks for identifying the import-time profile-path issue. The auxiliary and mirror portions remain valid on current main: Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-session-state
GottZ
reviewed
Jul 28, 2026
GottZ
left a comment
GottZ
left a comment
Contributor
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs address the same import-time profile-path freezing class but touch distinct consumers: #56302 covers Nous credentials and the session mirror, with its Anthropic OAuth portion now superseded; #56508 changes hook-directory discovery but not multiplex lifecycle routing; #56523 dynamically resolves skills-sync paths but does not add per-profile gateway startup sync.
Related pull requests
- #56302
related— (+162/-18) — keep open and rebase: The diff correctly re-resolvesauth.jsonandsessions.jsonagainst the active profile, directly fixing the reported cross-profile reads. As the contributor keep_open review documents, the Anthropic OAuth implementation and test seam conflict with the current_get_hermes_oauth_file()implementation and must be dropped. - #56508
related— (+89/-2) — keep open for architectural completion: The diff makes hook discovery resolve the active profile's directory, but the contributor keep_open review correctly notes that the shared registry is loaded only at startup and still receives events from secondary profiles; profile-aware registry loading and dispatch plus a gateway-level multiplex regression are required. - #56523
related— (+123/-26) — keep open but narrow or complete: The diff removes frozen skills-sync path usage and therefore protects scoped in-process callers, but it does not establish the claimed per-profile gateway startup behavior. Consistent with the contributor keep_open review, either wiresync_skills()into each scoped profile startup and add an end-to-end destination test, or narrow the PR to generic dynamic-path protection.
Suggested consolidation
Merge #56302 after rebasing it to retain only the still-valid auxiliary-client and mirror fixes and their focused tests. Do not close #56508 or #56523 as duplicates: they cover separate hook and skills-sync paths, but both need the changes required by their contributor keep_open reviews before merge.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 33 kB of PR diffs, 10 kB of issue/PR text, 6 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Contributor
|
suggesting changes Security evidence:
Keep the per-call auxiliary-client and legacy mirror fallback changes, but rebase or narrow the submitted branch onto current Not checked:
Signed: GPT-5.6-sol-xhigh in Codex |
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
5 participants
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.
Summary
Three modules resolve a profile-scoped path once at import time and cache it in a module-level constant:
agent/anthropic_adapter.py::_HERMES_OAUTH_FILE— the Hermes-managed Anthropic OAuth credential fileagent/auxiliary_client.py::_AUTH_JSON_PATH— the Nous Portalauth.jsongateway/mirror.py::_SESSIONS_INDEX— the delivery-mirror'ssessions.jsonlookup indexAll three go through
get_hermes_home(), which reads a context-localContextVar(_HERMES_HOME_OVERRIDEinhermes_constants.py) set per-request viaset_hermes_home_override()for the multiplexed gateway (multiple profiles served from one process, e.g. the desktoptui_gateway). Freezing the resolved path at import time pins every later request to whichever profile'sHERMES_HOMEhappened to be active the first time the module was imported in the process — the same bug class already fixed for cache dirs (gateway/platforms/base.py),tools/skills_hub.py, andgateway/rich_sent_store.py.For the Anthropic OAuth file specifically, this isn't just a read leak:
hermes_cli/web_server.py's OAuth save/disconnect handlers (_save_anthropic_oauth_creds,disconnect_oauth_provider,_anthropic_oauth_status) all import and use the same frozen constant, so completing an OAuth login under one profile could write the freshly-obtained token into a different profile's credential file.Changes
agent/anthropic_adapter.py: added_resolve_hermes_oauth_file(), called fromread_hermes_oauth_credentials(). Keeps the module constant for backward compat and to preserve the existing test seam (tests/hermes_cli/test_web_server_oauth_write.pymonkeypatchesagent.anthropic_adapter._HERMES_OAUTH_FILEdirectly) — the resolver honors a monkeypatched value away from its import-time default, otherwise re-resolves fresh, mirroringgateway/platforms/base.py::_resolve_cache_dir's established pattern.hermes_cli/web_server.py: updated all three call sites that imported the raw_HERMES_OAUTH_FILEconstant to import and call_resolve_hermes_oauth_file()instead.agent/auxiliary_client.py: added_resolve_auth_json_path(), called from_read_nous_auth(). No existing test monkeypatches the raw constant, so this is a direct per-call resolution without the test-seam indirection.gateway/mirror.py: added_resolve_sessions_index(), called from_find_session_id(). Same test-seam-preserving pattern as the OAuth file, sincetests/gateway/test_mirror.pyhas many call sites that monkeypatchgateway.mirror._SESSIONS_INDEXdirectly.tests/test_profile_isolation_runtime.py(the existing profile-isolation regression suite): addedTestAnthropicOauthFilePathResolution,TestAuxiliaryClientAuthJsonPathResolution, andTestMirrorSessionsIndexResolution, each proving the resolved path actually changes between two distinct profile overrides (not just that existing tests still pass), plus "monkeypatched constant still wins" regression tests for the two test-seam-preserving resolvers.Test plan
pytest tests/test_profile_isolation_runtime.py -q— 15 passed (7 pre-existing + 8 new)pytest tests/gateway/test_mirror.py tests/hermes_cli/test_web_server_oauth_write.py tests/agent/test_auxiliary_client.py tests/agent/test_credential_pool.py -q— 392 passedruff checkon all changed files — cleantests/agent/test_anthropic_adapter.pyhas 17 pre-existing failures in this sandbox unrelated to this change — confirmed identical (same tests, same error) onupstream/mainwith this diff stashed out. They stem from real local credential state leaking intoTestResolveAnthropicToken/TestRefreshOauthToken/etc., not from anything this PR touches.