Skip to content

fix(gateway): use get_secret for platform tokens in multiplexer scope - #51115

Closed
manus-use wants to merge 2 commits into
NousResearch:mainfrom
manus-use:fix/multiplexer-token-scope-leak
Closed

fix(gateway): use get_secret for platform tokens in multiplexer scope#51115
manus-use wants to merge 2 commits into
NousResearch:mainfrom
manus-use:fix/multiplexer-token-scope-leak

Conversation

@manus-use

Copy link
Copy Markdown

Summary

What: Replace os.getenv with get_secret for all platform token reads in _apply_env_overrides

Why: Platform tokens in _apply_env_overrides used raw os.getenv() which bypasses the per-profile secret scope, causing secondary profiles to inherit the default profile's credentials. When gateway.multiplex_profiles is enabled, each profile has its own .env with its own bot tokens. The set_secret_scope() / get_secret() machinery already exists in agent.secret_scope for exactly this purpose, but _apply_env_overrides was never migrated to use it.

How: Replace all os.getenv() calls inside _apply_env_overrides with get_secret() from agent.secret_scope. This function:

  • Multiplex turn with scope installed: reads from the profile's secret mapping — the correct, isolated credential
  • No scope / non-multiplex (default): falls back to os.environ transparently — no behavior change for single-profile setups
  • Multiplex active but no scope: raises UnscopedSecretError (fail-closed) — surfaces any remaining unscoped reads loudly instead of silently leaking

Also adds from agent.secret_scope import get_secret import at the top of gateway/config.py.

Testing:

python3 -m pytest tests/gateway/test_multiplexer_token_scope.py -v

New test file covers:

  1. Single-profile fallbackget_secret reads from os.environ when no scope is active (7 tests across Telegram, Discord, Slack, Matrix, WhatsApp Cloud, Mattermost, Weixin)
  2. Profile-scoped read — scope token wins over os.environ token; two profiles get different tokens (6 tests)
  3. Fail-closedUnscopedSecretError raised in multiplex mode with no scope installed (3 tests)
  4. Call-site proofget_secret is patched at gateway.config.get_secret to verify the function is actually called, not os.getenv (3 tests including exhaustive check of 9 primary credentials)

All 19 tests pass.

Security impact: Fixes credential leakage between multiplexed profiles. Before this fix, a secondary profile's Telegram/Discord/Slack/WhatsApp/etc. adapter would silently connect using the default profile's bot token.

Closes #51029

manusjs added 2 commits June 23, 2026 00:01
When tool_preview_length is set to 0 (the documented "no limit" value),
the gateway progress preview incorrectly fell back to 40 characters due
to a falsy check: `_cap = _pl if _pl > 0 else 40`.

Replace the pattern with an explicit guard that only truncates when _pl
is a positive integer, making 0 truly mean "no limit" as documented in
the config reference.

Fixes NousResearch#51067
When gateway.multiplex_profiles is enabled, _apply_env_overrides was
resolving platform tokens (TELEGRAM_BOT_TOKEN, DISCORD_BOT_TOKEN, etc.)
via raw os.getenv(), which reads the process-global environment. In a
multiplexed setup this means a secondary profile's adapter gets the
DEFAULT profile's bot token instead of its own.

Replace all os.getenv() calls in _apply_env_overrides with get_secret()
from agent.secret_scope. This function:
  - Reads from the active set_secret_scope mapping during a multiplexed
    turn (profile-scoped, authoritative)
  - Falls back transparently to os.environ when no scope is installed
    (single-profile / non-multiplex deployments — no behavior change)
  - Raises UnscopedSecretError (fail-closed) when multiplex mode is
    active but no scope is set, surfacing any remaining unscoped reads
    loudly instead of silently leaking another profile's credentials

Fixes: NousResearch#51029
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jun 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #50094 — same fix: replaces os.getenv with get_secret (agent.secret_scope) for platform tokens in gateway/config.py::_apply_env_overrides so multiplexed secondary profiles read their own profile-scoped tokens. #50094 is the earlier canonical PR (and a superset, adding the nested multiplex config layer). #51044 was already flagged duplicate of #50094; #49474/#49484 are the load_gateway_config sibling.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the multiplex credential-isolation work. The principal fix is already present on current main: gateway/config.py:174-189 resolves active profile-scoped values, _apply_env_overrides() uses that helper at gateway/config.py:1450, and secondary startup loads config within _profile_runtime_scope at gateway/run.py:8550-8551 (0f154e780e71c74f8a1cdccb25c97a6abd8e5a57, released in v2026.7.7).

Problems

  • The bundled preview change reverses current intentional all/new gateway behavior. gateway/run.py:17409-17427 retains a 40-character default cap when tool_preview_length is 0; f1c084714 restored that cap because full persistent messages were undesirable, and tests/gateway/test_run_progress_topics.py:570-580 verifies it.
  • tests/gateway/test_tool_preview_length_zero.py simulates the proposed branch locally instead of exercising the gateway progress path.

Suggested changes

  • Remove the already-landed scope fix.
  • Split or drop the preview change; changing the deliberate gateway-message cap needs a separate maintainer decision and an integration test.

This is an automated hermes-sweeper review.

"""When _pl == 0, no truncation occurs regardless of string length."""
set_tool_preview_max_len(0)
_pl = get_tool_preview_max_len()
long_preview = "x" * 200

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test only reimplements the proposed conditional locally; it never invokes the gateway progress callback. Current main intentionally asserts that all/new mode with tool_preview_length: 0 truncates at 40 characters in tests/gateway/test_run_progress_topics.py:570-580, following f1c084714.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as redundant on current main — the core fix here was independently implemented and merged before this review reached it:

Nothing left to salvage, but the diagnosis was right — thanks for the thorough sweep, and sorry it collided with a parallel fix.

@teknium1 teknium1 closed this Jul 16, 2026
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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

Multiplexer: secondary profile's platform token leaks from the default profile (_apply_env_overrides uses os.getenv, not the profile secret scope)

3 participants