Skip to content

fix(multiplex): wire profile secret scope + complete adapter routing - #58587

Closed
Tattooed-Geek wants to merge 6 commits into
NousResearch:mainfrom
Tattooed-Geek:fix/multiplex-profile-secret-routing
Closed

fix(multiplex): wire profile secret scope + complete adapter routing#58587
Tattooed-Geek wants to merge 6 commits into
NousResearch:mainfrom
Tattooed-Geek:fix/multiplex-profile-secret-routing

Conversation

@Tattooed-Geek

Copy link
Copy Markdown

What does this PR do?

Fixes three integration gaps that prevent multiplex_profiles: true from working with per-profile Telegram bots (and other polling platforms).

Problem 1: Platform tokens read via os.getenv instead of profile secret scope

_apply_env_overrides() in gateway/config.py reads TELEGRAM_BOT_TOKEN (and all platform tokens) via os.getenv(). Even though load_gateway_config() is called inside _profile_runtime_scope(profile_home) for secondary profiles, os.getenv bypasses the secret scope and reads from os.environ — which holds the default profile's token. Both profiles end up polling the same bot token.

The infrastructure already exists (agent/secret_scope.py with get_secret(), set_secret_scope(), build_profile_secret_scope(), _profile_runtime_scope()) but _apply_env_overrides() was never migrated to use it.

Problem 2: _adapter_for_source() defined but unused at 47 call sites

_adapter_for_source() is defined in gateway/run.py but only called in 5 of ~52 call sites. The remaining 47 still use self.adapters.get(source.platform), routing secondary-profile messages through the default adapter.

Problem 3: build_source() does not stamp source.profile

BasePlatformAdapter.build_source() does not accept a profile parameter, so source.profile is always None. _adapter_for_source() falls back to the default adapter map. Batch keys (_text_batch_key, _photo_batch_key) are also not namespaced by profile.

Related Issue

No existing issue found.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/config.py: Add _profile_secret() / _profile_secret_str() wrappers around get_secret(). Replace all os.getenv / env_var_enabled calls for platform tokens with profile-scoped versions.
  • gateway/run.py: Replace 47 self.adapters.get(source.platform) calls with self._adapter_for_source(source). Add profile to inbound message log.
  • gateway/platforms/base.py: Add profile parameter to build_source(), propagate to SessionSource.
  • plugins/platforms/telegram/adapter.py: Pass profile=getattr(self, 'profile_name', None) to build_source() / build_session_key() in 3 call sites.
  • tests/gateway/test_multiplex_adapter_registry.py: Add TestAdapterForSource with 3 tests.

How to Test

  1. Configure two profiles with different Telegram bot tokens in their respective .env files
  2. Enable multiplex_profiles: true in config.yaml
  3. Start the gateway
  4. Without this fix: both profiles load the same token -> polling conflict loop
  5. With this fix: each profile loads its own token -> both bots connect independently
  6. Run tests: venv/bin/pytest tests/gateway/test_multiplex_adapter_registry.py tests/gateway/test_multiplex_credential_isolation.py tests/gateway/test_multiplex_profile_authz.py -v (expected: 28 passed)

Checklist

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Linux (Debian)
  • N/A - no new config keys, no new tools, no architecture changes

Notes

  • Backward-compatible: when multiplex_profiles is off, get_secret() reads os.environ identically to os.getenv().
  • _profile_secret() wrappers fall back to os.getenv when agent.secret_scope is not importable.
  • build_source()'s new profile parameter defaults to None, so existing callers are unaffected.

@Tattooed-Geek
Tattooed-Geek requested a review from a team July 5, 2026 01:00
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 5, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The patch still leaves secondary-profile slash commands routed through the default adapter. GatewayRunner inherits GatewaySlashCommandsMixin, but gateway/slash_commands.py still calls self.adapters.get(source.platform) in the /status queue-depth path, the /model picker path, and the /approve and /deny typing-resume paths instead of using _adapter_for_source(source). That means a Telegram event stamped with source.profile = "secondary" can still inspect or resume the default profile's adapter state after this PR.

Security evidence: the affected boundary is multiplex profile isolation for per-profile Telegram sessions and approvals; the PR-head helper _adapter_for_source() chooses the secondary adapter for a synthetic secondary-profile Telegram source, but a focused PR-head probe of /approve imported gateway.run and gateway.slash_commands from the PR worktree and still recorded default_resumed=['chat-2'] with secondary_resumed=[]; git diff --check passed on the meaningful six-file PR diff, git merge-tree against current main succeeded, and tests/gateway/test_multiplex_adapter_registry.py passed with 12 tests, but those tests do not cover the inherited slash-command paths that still bypass the profile adapter map.

Please route the remaining slash-command adapter lookups through _adapter_for_source(source) or equivalent profile-aware logic, and add focused coverage for a secondary-profile slash command such as /approve or /model.

Signed: GPT-5.5-xhigh in Codex

@Tattooed-Geek

Copy link
Copy Markdown
Author

Thank you for the thorough review.

Pushed a fix: all 10 self.adapters.get(source.platform) sites in gateway/slash_commands.py now route through self._adapter_for_source(source). This covers the four paths you identified (/status queue-depth, /model picker, /approve typing-resume, /deny typing-resume) plus six sibling paths with the same bug class (/status Matrix scope, /goal pause, /goal clear, /goal start, /voice on, /voice status).

Added TestSlashCommandProfileRouting with focused /approve and /deny coverage: a secondary-profile Telegram source triggers resume_typing_for_chat on the secondary adapter only — the default adapter is verified untouched.

All 14 tests pass. Verified live with two Telegram bots (default + secondary profile) both responding correctly to /status.

@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The latest head fixes the direct /approve and /deny slash-command routing, but the bare-text approval path in GatewayRunner._handle_active_session_busy_message() still sends its confirmation through self.adapters.get(event.source.platform). A secondary-profile Telegram user who replies yes or approve to a pending dangerous-command approval is routed through _handle_approve_command(), but the confirmation send then uses the default Telegram adapter instead of _adapter_for_source(event.source). In a PR-head probe with source.profile = "secondary", the default adapter recorded the confirmation send while the secondary adapter recorded none.

Security evidence: the affected trust boundary is multiplex profile isolation for per-profile Telegram approval flows; the secondary-profile SessionSource reaches the bare-text approval branch and the confirmation sink must use the profile-owned adapter for _send_with_retry(); current GitHub main was dec4485d2ffacc49f2d2af15d6b3fcdeb238e1dc; the submitted PR head was reviewed via a run-owned local merge onto that current main, so stale-base status was setup information only and this does not by itself prove the submitted branch merges cleanly; the merged validation worktree probe imported gateway.run from that review tree and produced default_sent=[...] with secondary_sent=[]; tests/gateway/test_multiplex_adapter_registry.py passes with 16 tests, including direct /approve and /deny slash-command routing plus profile stamping before session keying, but it does not cover the bare-text approval confirmation branch.

Please route the plain-text approval confirmation send in gateway/run.py through _adapter_for_source(event.source) and add a focused secondary-profile bare yes or approve approval-response test.

Signed: GPT-5.5-xhigh in Codex

@Tattooed-Geek

Copy link
Copy Markdown
Author

Thank you for catching this.

Fixed: _handle_active_session_busy_message() now routes the bare-text approval confirmation send through self._adapter_for_source(event.source) instead of self.adapters.get(event.source.platform).

Added TestBareTextApprovalRouting::test_bare_yes_routes_to_secondary_adapter — a secondary-profile Telegram source sending bare "yes" triggers the confirmation send on the secondary adapter only; the default adapter is verified untouched.

All 17 tests pass. I also audited the remaining self.adapters.get() sites in gateway/run.py and gateway/slash_commands.py:

  • 6 sites are inside _adapter_for_source() itself or compare adapter instances (safe by construction)
  • 5 sites are Discord voice-channel paths that hardcode Platform.DISCORD (edge-case for multiplex, no source available in some paths)
  • 3 sites resolve adapters from saved platform_str in pending-approval/update/restart notification JSON files (would require storing profile in the JSON to route correctly — separate scope)

These remaining sites are out of scope for this PR but noted for future work.

@Tattooed-Geek

Copy link
Copy Markdown
Author

New commit: skip port-binding platform on secondary profiles instead of crashing

Problem: When multiplex_profiles: true and a secondary profile has api_server in its platforms config (e.g. left over from a config update or template), the gateway raised a fatal MultiplexConfigError — causing a crash-restart loop that takes down ALL profiles, including the default one.

Root cause: The multiplex guard in _start_one_profile_adapters treated api_server on a secondary profile as a hard error. But since the default profile owns the single shared HTTP listener and serves secondary profiles via /p/<profile>/, the api_server entry is simply redundant — not a conflict.

Fix: Replace the fatal raise MultiplexConfigError with:

  • A WARNING log explaining the situation and how to fix it
  • A continue (skip) — the platform is ignored for that profile
  • The gateway continues starting all other platforms normally

This means a misconfigured secondary profile can no longer crash the entire gateway. The warning message is clear and actionable:

WARNING: Profile 'kimberly' enables the port-binding platform 'api_server', 
but gateway.multiplex_profiles is on. The default profile owns the single 
shared HTTP listener and serves every profile through the /p/kimberly/ URL prefix. 
Skipping this platform for this profile — remove platforms.api_server from 
this profile's config.yaml (configure it only on the default profile).

Test updated: test_secondary_webhook_raisestest_secondary_webhook_skipped — asserts the platform is skipped (not started) and a warning is logged, instead of asserting a crash.

Validated in real conditions: Added api_server to a secondary profile config, restarted gateway — previously crashed, now starts normally with 3 platforms and both bots responding. Then removed the redundant config entry.

@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

_apply_env_overrides() still bypasses the profile-scoped secret lookup for several platform credentials. For example, Dingtalk still reads DINGTALK_CLIENT_ID / DINGTALK_CLIENT_SECRET with os.getenv(), so a secondary profile with no Dingtalk credentials in its secret scope can still inherit the default profile's Dingtalk credentials from process env. That leaves the multiplex secret-isolation fix only partial.

Security evidence:

  • trust boundary: multiplex profile secret isolation for per-profile platform credentials.
  • source/sink/invariant: gateway/config.py _apply_env_overrides() must read platform credential env vars through the active profile secret scope before enabling a platform.
  • current-main reproduction: current main has the original unscoped env override behavior; this PR is expected to migrate those reads.
  • PR-head or patch-replay validation: on a run-owned patch replay against current GitHub main beaa1a08e6abf2fb8efff0b05da8857bef21ce1f, after resolving the unrelated gateway/run.py replay conflict for review only, a probe imported gateway.config from the replay worktree and showed Dingtalk still being enabled from default-profile os.environ values when the active profile secret lookup returned no DINGTALK_* values.
  • positive/negative cases: the focused multiplex tests pass (17 passed), but they do not cover the remaining non-Telegram credential override paths.
  • residual bypass search: direct credential-related os.getenv() reads remain in _apply_env_overrides() for Dingtalk, Feishu, WeCom, Weixin, BlueBubbles, QQBot, and Yuanbao.
  • reviewer validation: local source review reproduced the Dingtalk secret-scope bypass in the replay worktree and verified the focused multiplex test file still passes.

Please route the remaining platform credential env overrides through the profile secret-scope helper and add focused coverage for at least one non-Telegram credential path so a secondary profile cannot inherit the default profile's credentials.

Signed: GPT-5.5-xhigh in Codex

Tattooed-Geek and others added 6 commits July 5, 2026 14:49
multiplex_profiles: true does not actually work for per-profile Telegram
bots. _apply_env_overrides() in config.py reads TELEGRAM_BOT_TOKEN (and all
platform tokens) via os.getenv(), which sees the default profile's
environment even when load_gateway_config() is called inside
_profile_runtime_scope() for a secondary profile. Both profiles load the
same token -> polling conflict.

The infrastructure already exists (agent/secret_scope.py with get_secret(),
_profile_runtime_scope(), build_profile_secret_scope()) but
_apply_env_overrides() was never migrated to use it.

Additionally, _adapter_for_source() is defined in run.py but only used in
5 of ~52 call sites. The remaining 47 still use
self.adapters.get(source.platform), routing secondary-profile messages
through the default adapter.

Finally, BasePlatformAdapter.build_source() does not accept a profile
parameter, so adapters cannot stamp source.profile on incoming messages.
Without source.profile, _adapter_for_source() always sees profile=None and
falls back to the default map.

Changes:
- config.py: add _profile_secret() / _profile_secret_str() wrappers around
  get_secret(), replace all os.getenv / env_var_enabled calls for platform
  tokens and settings with the profile-scoped versions
- run.py: replace 47 self.adapters.get(source.platform) calls with
  self._adapter_for_source(source); add profile to inbound message log
- base.py: add profile parameter to build_source(), propagate to SessionSource
- adapter.py (telegram): pass profile to build_source() / build_session_key()
  in _build_message_event, _text_batch_key, _photo_batch_key
- tests: add TestAdapterForSource with 3 tests covering default, secondary,
  and active-profile routing
…for_source

Slash commands in gateway/slash_commands.py used self.adapters.get(source.platform)
which ignores source.profile. In multiplex mode, a secondary-profile Telegram
event could resume typing or inspect the default profile's adapter state.

Fixed 10 sites: /status queue-depth, /status Matrix scope, /model picker,
/goal pause, /goal clear, /goal start, /voice on, /voice status, /approve,
/deny — all now route through self._adapter_for_source(source).

Added TestSlashCommandProfileRouting with /approve and /deny coverage verifying
the secondary adapter is used and the default adapter is untouched.

Addresses PR review feedback from egilewski.
…sage

BasePlatformAdapter.handle_message() computed the session key via
build_session_key() before _make_profile_message_handler had a chance to
stamp source.profile. This caused a key mismatch: the adapter guard map
used agent:main:… while the session store (which re-reads source.profile
inside _generate_session_key) used agent:<profile>:…. In multiplex mode
this meant the active-session guard for a secondary-profile Telegram DM
could miss a concurrently running agent on that profile, because the guard
key and the session key lived in different namespaces.

Fix: stamp source.profile from adapter.profile_name at the top of
handle_message(), before build_session_key() is called. Existing profile
values are preserved (not overwritten).

Added TestProfileStampingBeforeSessionKey covering both the stamp and
no-overwrite paths.
…r_source

_handle_active_session_busy_message() used self.adapters.get(event.source.platform)
to send the plain-text approval confirmation (bare 'yes'/'approve' replies).
In multiplex mode, a secondary-profile Telegram user's approval confirmation
was sent through the default adapter instead of the profile-owned one.

Fix: use _adapter_for_source(event.source) for the confirmation send.
Added TestBareTextApprovalRouting covering a secondary-profile bare 'yes'
approval response — verifies the secondary adapter sends the confirmation
and the default adapter is untouched.

Addresses second round of PR review feedback from egilewski.
…ead of crashing

When multiplex_profiles is enabled and a secondary profile has api_server
in its platforms config, the gateway raised a fatal MultiplexConfigError
that caused a crash-restart loop. Since the default profile owns the single
shared HTTP listener and serves secondary profiles via /p/<profile>/,
the api_server entry on a secondary profile is simply redundant — not a
reason to crash.

Replace the fatal raise with a WARNING log + skip: the platform is ignored
for that profile, the gateway continues starting all other platforms normally.

Test updated: test_secondary_webhook_raises → test_secondary_webhook_skipped
asserts the platform is skipped (not started) and a warning is logged,
instead of asserting a crash.
…rofile secret scope

Migrate 73 os.getenv() calls in _apply_env_overrides() to _profile_secret()/
_profile_secret_str() for DingTalk, Feishu, WeCom, WeCom Callback, Weixin,
BlueBubbles, QQBot, and Yuanbao — matching the pattern already applied to
Telegram, Discord, WhatsApp, Slack, Signal, and Mattermost in commit 1.

Add a guard in the plugin-enable pass: when a profile secret scope is active
(multiplex mode), skip the is_connected() probe and auto-enablement for
platforms not already explicitly configured in YAML. Plugin is_connected()
implementations read os.getenv() directly and would see the default profile's
credentials in os.environ, breaking isolation.

Add test_dingtalk_not_inherited_from_default_env: verifies a secondary profile
with no DINGTALK_* entries in its secret scope does not enable DingTalk even
when the default profile's credentials are in os.environ.
@Tattooed-Geek
Tattooed-Geek force-pushed the fix/multiplex-profile-secret-routing branch from 197de48 to dfe577f Compare July 5, 2026 19:20
@Tattooed-Geek

Copy link
Copy Markdown
Author

New commit: route non-Telegram credential env overrides through profile secret scope

Addresses the remaining credential-isolation gap for non-Telegram platforms.

Changes

1. Migrated 73 os.getenv() calls to _profile_secret() / _profile_secret_str() in _apply_env_overrides() for 7 platforms:

Platform Credential vars migrated
DingTalk DINGTALK_CLIENT_ID, DINGTALK_CLIENT_SECRET, DINGTALK_HOME_CHANNEL*
Feishu/Lark FEISHU_APP_ID, FEISHU_APP_SECRET, FEISHU_DOMAIN, FEISHU_*
WeCom WECOM_BOT_ID, WECOM_SECRET, WECOM_WEBSOCKET_URL, WECOM_HOME_*
WeCom Callback WECOM_CALLBACK_CORP_ID, WECOM_CALLBACK_CORP_SECRET, WECOM_CALLBACK_*
Weixin WEIXIN_TOKEN, WEIXIN_ACCOUNT_ID, WEIXIN_BASE_URL, WEIXIN_*
BlueBubbles BLUEBUBBLES_SERVER_URL, BLUEBUBBLES_PASSWORD, BLUEBUBBLES_*
QQBot QQ_APP_ID, QQ_CLIENT_SECRET, QQ_ALLOWED_USERS, QQBOT_*
Yuanbao YUANBAO_APP_ID, YUANBAO_APP_SECRET, YUANBAO_*

This matches the pattern already applied to Telegram, Discord, WhatsApp, Slack, Signal, and Mattermost in commit 1. Non-credential gateway settings (SESSION_IDLE_MINUTES, SESSION_RESET_HOUR, GATEWAY_RELAY_URL) remain on os.getenv() — they are process-global, not per-profile secrets.

2. Guarded the plugin-enable pass against credential leakage:

During the review I discovered that even after migrating _apply_env_overrides() to _profile_secret(), platforms were still being auto-enabled by the plugin-enable pass. Root cause: plugin is_connected() implementations read os.getenv() directly (not _profile_secret), so in a multiplexer they see the default profile's credentials in os.environ and wrongly enable the platform on a secondary profile.

Fix: when a profile secret scope is active (is_multiplex_active() and current_secret_scope() is not None), skip both:

  • The is_connected() probe for platforms not already explicitly configured
  • The check_fn() + enabled = True auto-enablement that follows

A secondary profile now only gets platforms it explicitly configured in YAML or whose credentials are in its own .env scope.

3. Added TestNonTelegramCredentialIsolation::test_dingtalk_not_inherited_from_default_env:

Sets DINGTALK_CLIENT_ID and DINGTALK_CLIENT_SECRET in os.environ (simulating the default profile), activates multiplex mode with an empty secondary-profile secret scope, calls _apply_env_overrides(), and asserts Platform.DINGTALK is not enabled. This directly validates the credential isolation boundary the reviewer identified.

Validation

  • 18/18 tests pass (including the new DingTalk isolation test)
  • Rebased on current main (beaa1a08e) — 0 behind, 0 conflicts
  • Tested live: gateway starts cleanly with 3 platforms (api_server + telegram default + telegram kimberly), both bots responding

@Tattooed-Geek

Copy link
Copy Markdown
Author

Closing this PR — the credential isolation and adapter routing fixes (commits 1-4 and 6) have been merged into main via #59203 (izumi0uu) and #59156 (AndreasHiltner). The remaining port-binding skip fix (commit 5) has been extracted into a standalone PR: #59798.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants