fix(gateway): multiplex credential isolation — authz, Slack, WeChat, secondary adapters (4-PR cluster salvage) - #65629
Merged
Conversation
Secondary profiles under gateway multiplex keep tokens/allowlists in profile secret_scope, not process os.environ. Auth and Slack were still reading os.getenv, so Slack on a secondary profile failed allowlist and socket mode. Webhook deliver also only looked at default adapters. - Prefer get_secret for allowlists / allow-all flags (authz_mixin) - Slack app token + allowlist via secret_scope with getenv fallback - Wrap secondary profile message handlers in _profile_runtime_scope before auth runs - Resolve home-channel env from secret_scope / PlatformConfig - Webhook deliver falls back to _profile_adapters for target platform - Template key event_type for webhook prompts
…for consistent profile-scoped resolution Per egilewski's security review, WEIXIN_BASE_URL and WEIXIN_CDN_BASE_URL were still resolved from process-global environment variables, leaving mixed-scope bypasses in multiplex mode. Changed files: - gateway/platforms/weixin.py: Added get_secret import, replaced os.getenv() with get_secret() for WEIXIN_ACCOUNT_ID, WEIXIN_TOKEN, WEIXIN_BASE_URL, WEIXIN_CDN_BASE_URL in WeixinAdapter.__init__() and send_weixin_direct() - tools/send_message_tool.py: Added get_secret import, replaced os.getenv() with get_secret() for all WEIXIN_* fallbacks in _handle_send() All runtime Weixin send paths now resolve both credentials and endpoint configuration from the same profile-scoped source.
Partial cherry-pick of a7ffbbf from PR #63256: secondary-profile adapter creation errors no longer abort the whole secondary startup (try/except around _create_adapter + loud warning on None return), and Home Assistant's check_ha_requirements() becomes dep-only with the credential moved to a new validate_ha_config() so secondary profiles whose HASS_TOKEN lives in the profile secret scope are not silently dropped by the registry gate. Telegram diagnostic hunks and profile-label stamping dropped: the regression they targeted does not exist on current main and they conflict with the connect() teardown fence.
Follow-ups on top of the cherry-picked cluster commits: - slack: scope-authoritative app-token read — get_secret() with a narrow UnscopedSecretError fallback to os.getenv. Keeps @kohoj's correct semantics (scoped profile can never silently inherit the default profile's Socket Mode app) while fixing the regression where the default-profile startup loop and background reconnect rebuild, which call connect() unscoped under multiplex, would raise and fail-loop. Supersedes the 'or os.getenv' variant from #64461 which reintroduced the cross-profile fallback leak. - test: unscoped-multiplex fallback regression test for connect(). - run.py: convert the last legacy self.adapters.get(source.platform) site (_rename_discord_auto_thread) to _adapter_for_source(source) so profile-routed Discord sources rename threads on the right adapter (from #57417's sweep). - AUTHOR_MAP entry for @aguung.
alt-glitch
added
type/security
This was referenced Jul 16, 2026
Closed
tonydwb
reviewed
Jul 16, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Scope
- 14 files, +396/-50 lines. Multiplex credential isolation for authz, Slack, WeChat, and secondary adapters.
Quality
- Significant security-relevant change (credential isolation).
- Part of a 4-PR cluster salvage.
Notes
- Large surface area for a single PR (14 files).
- Credential multiplexing and authz changes require careful security review.
Looks Good
- Well-scoped fix for a complex multi-adapter scenario.
Reviewed by Hermes Agent
teknium1
pushed a commit
that referenced
this pull request
Jul 16, 2026
Subset of PR #61985: _make_adapter_auth_check gains a profile_name parameter and secondary-profile adapters (started in _start_one_profile_adapters) bind it, so the auth callback's SessionSource resolves the routed profile's adapter and pairing store instead of silently falling back to the default profile. This is the gap left open by the #65629 merge — adapter-internal auth checks (e.g. Slack thread-context fetch) fire outside the wrapped message handler. The PR's authz_mixin.py hunks are dropped: main's _auth_env (merged via PR #65629) already covers the scoped allowlist reads they targeted.
This was referenced Jul 16, 2026
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
Subset of PR NousResearch#61985: _make_adapter_auth_check gains a profile_name parameter and secondary-profile adapters (started in _start_one_profile_adapters) bind it, so the auth callback's SessionSource resolves the routed profile's adapter and pairing store instead of silently falling back to the default profile. This is the gap left open by the NousResearch#65629 merge — adapter-internal auth checks (e.g. Slack thread-context fetch) fire outside the wrapped message handler. The PR's authz_mixin.py hunks are dropped: main's _auth_env (merged via PR NousResearch#65629) already covers the scoped allowlist reads they targeted.
19 tasks
1 task
yournetworkplug-ctrl
added a commit
to yournetworkplug-ctrl/hermes-agent
that referenced
this pull request
Jul 29, 2026
… multiplex_profiles
Replace raw os.getenv("TELEGRAM_ALLOWED_USERS") reads in the Telegram
adapter's pre-filter with gateway.authz_mixin._auth_env so the served
profile's .env is consulted when its secret scope is installed. This
mirrors the gateway-layer fix in PRs NousResearch#61985/NousResearch#65629/NousResearch#65700 down to the
adapter pre-filter, fixing the Telegram mirror of issue NousResearch#72348.
See README.md for details, scope notes, and end-to-end test evidence.
Fixes NousResearch#72348 (Telegram mirror)
13 tasks
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
Subset of PR NousResearch#61985: _make_adapter_auth_check gains a profile_name parameter and secondary-profile adapters (started in _start_one_profile_adapters) bind it, so the auth callback's SessionSource resolves the routed profile's adapter and pairing store instead of silently falling back to the default profile. This is the gap left open by the NousResearch#65629 merge — adapter-internal auth checks (e.g. Slack thread-context fetch) fire outside the wrapped message handler. The PR's authz_mixin.py hunks are dropped: main's _auth_env (merged via PR NousResearch#65629) already covers the scoped allowlist reads they targeted.
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
7 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
Consolidated salvage of the open multiplex credential-isolation cluster: under
gateway.multiplex_profiles, authorization allowlists, the Slack Socket Mode app token, WeChat credentials, and secondary-profile adapter creation now all resolve through the routed profile's secret scope instead of leaking the default profile's process env. Closes #59739, closes #59662.Cherry-picks four contributor commits (authorship preserved, rebase merge) plus hardening follow-ups.
Salvaged work
_auth_env+_handle_messagescope wrap for secondary-profile events (the load-bearing piece: authz ran unscoped before any profile scope was installed), webhook delivery fallback to secondary-profile adapters +{event_type}template key, home-channel notice scope awareness, Slack interactive-auth scoped reads.get_secretwith regression test (scope holdsxapp-profile, env holdsxapp-default, handler must get the profile token).WEIXIN_TOKEN,WEIXIN_ACCOUNT_ID,WEIXIN_BASE_URL,WEIXIN_CDN_BASE_URL) in the adapter,send_weixin_direct, andsend_message_toolthroughget_secret.check_*_requirements()split into dep-only check +validate_config()(registry seam already used by simplex/photon/teams/ntfy/irc/feishu), so secondaries whose credentials live in the profile scope are no longer silently dropped; secondary_create_adapterfailures no longer abort the whole secondary startup. Telegram diagnostic hunks dropped (premise not on current main, conflicts with the connect() teardown fence).Follow-ups on top
get_secretwith a narrowUnscopedSecretError→os.getenvfallback. fix(slack): scope Socket Mode app token per profile #59869's bare read would crash the default-profile startup loop and background reconnect rebuild (both callconnect()unscoped under multiplex); fix(gateway): multiplex secret_scope for authz, Slack, webhooks #64461'sor os.getenvvariant silently reintroduced the cross-profile inheritance leak. This version keeps loud failure for a scoped profile missing its token AND working env fallback for legitimately unscoped default-profile paths. New regression test covers the unscoped case.self.adapters.get(source.platform)call site (_rename_discord_auto_thread) converted to_adapter_for_source(source)(from fix(gateway): isolate multiplexed profile routing #57417's sweep — the one hunk of it not already on main).Not salvaged (redundant on current main)
_getenvhelper (0f154e7).Validation
tests/gateway/ -k "multiplex or authz or webhook or weixin or slack or signal or mattermost or homeassistant"Known residual gaps (follow-up candidates, flagged in review):
_get_unauthorized_dm_behaviorallowlist probes still onos.getenv;_make_adapter_auth_checknot profile-stamped (open #61985 covers this); webhook first-match ambiguity when two profiles run the same platform.Infographic