Skip to content

fix(gateway): stop multiplex second auth gate from leaking allowlists… - #80360

Closed
OWWZO wants to merge 2 commits into
NousResearch:mainfrom
OWWZO:fix/gateway-multiplex-second-auth-gate
Closed

fix(gateway): stop multiplex second auth gate from leaking allowlists…#80360
OWWZO wants to merge 2 commits into
NousResearch:mainfrom
OWWZO:fix/gateway-multiplex-second-auth-gate

Conversation

@OWWZO

@OWWZO OWWZO commented Aug 6, 2026

Copy link
Copy Markdown

… (#80026)

Route _is_user_authorized allowlist/allow-all reads through _platform_gate_env so a profile-scoped miss does not fall back to process-global os.environ under multiplex. Remove the redundant _auth_env helper and add Discord/QQ regression coverage.

What does this PR do?

Fixes the gateway's second authorization gate under multiplex_profiles: a
profile-scoped miss no longer falls through to process-global os.environ,
so profile B cannot inherit profile A's bridged allowlist
(DISCORD_ALLOWED_USERS, *_ALLOW_ALL_USERS, GATEWAY_*, etc.).

Approach: route all _is_user_authorized allowlist/allow-all reads through
the existing multiplex-authoritative _platform_gate_env() and remove the
redundant _auth_env() helper (which fell back to os.getenv on a scoped
miss). Single-profile deployments keep legacy env behavior.

Related open PR #80238 fixes the same hole by inlining the isolation guard
into _auth_env. This PR consolidates onto one reader and adds regression
tests (Discord + QQ). Happy to close in favor of #80238 if maintainers
prefer the smaller call-site-preserving diff — please keep the tests.

Related Issue

Fixes #80026

Also closes residual gateway-gate gap left after #75970 (adapter-level fix
for #72348). Broader transport/provenance work remains in #76166.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/authz_mixin.py: remove _auth_env; use _platform_gate_env for
    platform allow-all, platform/group allowlists, GATEWAY_ALLOWED_USERS,
    and GATEWAY_ALLOW_ALL_USERS in _is_user_authorized

  • tests/gateway/test_discord_multiplex_second_auth_gate.py: new regression
    for [Bug]: Discord multiplex gateway second auth gate can apply another profile’s allowlist after #75970 #80026 (foreign allowlist / allow-all must not authorize secondary;
    scoped allowlist still works; single-profile env unchanged)

  • tests/gateway/test_qqbot_scope_paths.py: remove xfail on
    test_scope_does_not_inherit_environ_opt_in; add empty-scope allowlist
    non-inheritance case

How to Test

  1. Focused regression:
    scripts/run_tests.sh \
      tests/gateway/test_discord_multiplex_second_auth_gate.py \
      tests/gateway/test_qqbot_scope_paths.py \
      tests/gateway/test_multiplex_profile_authz.py \
      tests/gateway/test_config_driven_access_policy.py \
      tests/gateway/test_unauthorized_dm_behavior.py -q
  2. Expect all green, including former xfail
    TestAuthzAllowAllScope::test_scope_does_not_inherit_environ_opt_in.
  3. Manual multiplex (optional): two Discord/QQ profiles with distinct
    allowlists — profile A's user must be denied on profile B; profile B's
    configured user must still authorize.
    Observed locally (Windows, Python 3.11): 56 passed on the focused set above.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • [] This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

…NousResearch#80026)

Route _is_user_authorized allowlist/allow-all reads through
_platform_gate_env so a profile-scoped miss does not fall back to
process-global os.environ under multiplex. Remove the redundant
_auth_env helper and add Discord/QQ regression coverage.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 6, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

A P1 profile-isolation gap remains: multiplexed busy-session and startup-resume authorization can run outside the routed profile secret scope. The shared busy-session callback is installed without a profile wrapper, and startup resume validates the session owner before entering the scoped handler. In those paths the authorization helper can read the process-global allowlist, so a value from one profile can authorize a busy-session action or resume a revoked session for another profile. Wrap each profile-bound authorization call in the routed profile scope, including the busy-session callback, startup-resume validation, and adapter authorization callback, and add regression coverage for foreign allowlists and revoked session owners.

Security evidence:

  • trust boundary: A multiplexed gateway shares process state while each profile supplies its own credentials; adapter ingress, gateway authorization, busy-session handling, startup resume, and downstream execution must authorize only the routed profile.
  • source/sink/invariant: Sender and chat metadata reach the gateway authorization chain. Scoped allowlist and allow-all reads use the profile-authoritative helper, but unscoped callers can still read the process-global environment and violate profile isolation.
  • current-main reproduction: Before this change, a scoped miss fell through to the process-global allowlist, allowing a foreign profile sender instead of the profile-local policy.
  • PR-head or patch-replay validation: Scoped misses now reject foreign process values, scoped values authorize only their own profile, adapter-local allow_from remains effective, and single-profile environment behavior is preserved.
  • positive/negative cases: Positive cases cover an empty secondary scope, a populated secondary scope, adapter-local allow_from, and single-profile mode. Negative case: an unscoped multiplexed authorization call can still accept a foreign process allowlist.
  • residual bypass search: The cold message path is scoped, but the busy-session handler, startup-resume check, and profile-bound adapter authorization callback can execute before a profile scope is installed.
  • reviewer validation: Reviewed the changed authorization chain and raw source, exercised scoped, unscoped, and single-profile authorization cases, and ran focused migration tests; the P1 bypass remains.

Not checked:

  • gateway regression integration tests
  • live network adapter exercise

Signed: GPT-5.6-luna-max in Codex

Close residual P1 isolation gap: busy-session, startup-resume owner checks,
and adapter auth callbacks ran outside the routed profile secret scope and
could honor a foreign process allowlist. Wrap those paths and add regression
coverage for foreign allowlists and revoked session owners.
@OWWZO

OWWZO commented Aug 7, 2026

Copy link
Copy Markdown
Author

suggesting changessuggesting changes 建议修改

A P1 profile-isolation gap remains: multiplexed busy-session and startup-resume authorization can run outside the routed profile secret scope. The shared busy-session callback is installed without a profile wrapper, and startup resume validates the session owner before entering the scoped handler. In those paths the authorization helper can read the process-global allowlist, so a value from one profile can authorize a busy-session action or resume a revoked session for another profile. Wrap each profile-bound authorization call in the routed profile scope, including the busy-session callback, startup-resume validation, and adapter authorization callback, and add regression coverage for foreign allowlists and revoked session owners.A P1 profile-isolation gap remains: multiplexed busy-session and startup-resume authorization can run outside the routed profile secret scope. The shared busy-session callback is installed without a profile wrapper, and startup resume validates the session owner before entering the scoped handler. In those paths the authorization helper can read the process-global allowlist, so a value from one profile can authorize a busy-session action or resume a revoked session for another profile. Wrap each profile-bound authorization call in the routed profile scope, including the busy-session callback, startup-resume validation, and adapter authorization callback, and add regression coverage for foreign allowlists and revoked session owners.仍存在一个 P1 配置文件隔离漏洞:多路复用的繁忙会话和启动-恢复授权可在路由配置文件机密范围之外执行。共享的繁忙会话回调未配置文件包装器即可安装,启动恢复操作会在进入带范围的处理程序前验证会话所有者。在这些路径中,授权助手可读取进程全局允许列表,因此某个配置文件的值可授权另一配置文件的繁忙会话操作,或恢复其已吊销的会话。将每个与配置文件绑定的授权调用都封装在路由配置文件范围内,包括繁忙会话回调、启动-恢复验证和适配器授权回调,并为外部允许列表及已吊销会话所有者添加回归测试覆盖。

Security evidence:Security evidence: 安全依据:

  • trust boundary: A multiplexed gateway shares process state while each profile supplies its own credentials; adapter ingress, gateway authorization, busy-session handling, startup resume, and downstream execution must authorize only the routed profile.trust boundary: A multiplexed gateway shares process state while each profile supplies its own credentials; adapter ingress, gateway authorization, busy-session handling, startup resume, 和 downstream execution must authorize only the routed profile.信任边界:一个多路复用网关共享进程状态,而每个配置文件提供自身的凭证;适配器入口、网关授权、忙会话处理、启动恢复和下游执行必须仅对路由的配置文件进行授权。
  • source/sink/invariant: Sender and chat metadata reach the gateway authorization chain. Scoped allowlist and allow-all reads use the profile-authoritative helper, but unscoped callers can still read the process-global environment and violate profile isolation.source/sink/invariant: Sender and chat metadata reach the gateway authorization chain. Scoped allowlist and allow-all reads use the profile-authoritative helper, but unscoped callers can still read the process-global environment and violate profile isolation.源/汇/不变量:发送方和聊天元数据会到达网关授权链。作用域白名单和全允许读取操作使用配置文件权威辅助工具,但未限定作用域的调用方仍可读取进程全局环境,从而破坏配置文件隔离。
  • current-main reproduction: Before this change, a scoped miss fell through to the process-global allowlist, allowing a foreign profile sender instead of the profile-local policy.current-main reproduction: Before this change, a scoped miss fell through to the process-global allowlist, allowing a foreign profile sender instead of the profile-local policy.当前主要修复:在此次更改之前,作用域未命中会回退到进程全局允许列表,从而允许使用外部配置文件发送方,而非配置文件本地策略。
  • PR-head or patch-replay validation: Scoped misses now reject foreign process values, scoped values authorize only their own profile, adapter-local allow_from remains effective, and single-profile environment behavior is preserved.PR-head or patch-replay validation: Scoped misses now reject foreign process values, scoped values authorize only their own profile, adapter-local allow_from remains effective, 和 single-profile environment behavior is preserved.PR 头部或补丁重放验证:作用域缺失项现在会拒绝外部进程值,作用域值仅授权其自身的配置文件,适配器本地的 allow_from 规则仍有效,单配置文件环境的行为保持不变。
  • positive/negative cases: Positive cases cover an empty secondary scope, a populated secondary scope, adapter-local allow_from, and single-profile mode. Negative case: an unscoped multiplexed authorization call can still accept a foreign process allowlist.positive/negative cases: Positive cases cover an empty secondary scope, a populated secondary scope, adapter-local allow_from, 和 single-profile mode. Negative case: an unscoped multiplexed authorization call can still accept a foreign process allowlist.阳性/阴性案例:阳性案例涵盖空的次级作用域、已填充的次级作用域、适配器本地的allow_from以及单配置文件模式。阴性案例:未限定作用域的多路复用授权调用仍可接受外部进程允许列表。
  • residual bypass search: The cold message path is scoped, but the busy-session handler, startup-resume check, and profile-bound adapter authorization callback can execute before a profile scope is installed.residual bypass search: The cold message path is scoped, but the busy-session handler, startup-resume check, 和 profile-bound adapter authorization callback can execute before a profile scope is installed.残留旁路搜索:冷消息路径已限定作用域,但繁忙会话处理程序、启动-恢复检查以及配置文件绑定适配器授权回调可在配置文件作用域安装前执行。
  • reviewer validation: Reviewed the changed authorization chain and raw source, exercised scoped, unscoped, and single-profile authorization cases, and ran focused migration tests; the P1 bypass remains.reviewer validation: Reviewed the changed authorization chain and raw source, exercised scoped, unscoped, 和 single-profile authorization cases, 和 ran focused migration tests; the P1 bypass remains.审阅者验证:已审核修改后的授权链和原始来源,执行了范围化、非范围化和单配置文件授权案例,并开展了针对性迁移测试;P1 旁路问题仍然存在。

Not checked:Not checked: 未检查:

  • gateway regression integration testsgateway regression integration tests 网关回归集成测试
  • live network adapter exerciselive network adapter exercise 实时网络适配器练习

Signed: GPT-5.6-luna-max in CodexSigned: GPT-5.6-luna-max in Codex 已签名:Codex 中的 GPT-5.6-luna-max

Thanks for the residual-bypass report — agreed, that was a real P1 gap beyond the cold-path fix.
Addressed in a91bd6f:

  1. Busy-session — primary uses _primary_busy_session_handler() (scoped under multiplex); secondary uses _make_profile_busy_session_handler(profile) (stamps source.profile + enters that profile’s runtime scope). Auth goes through _is_user_authorized_scoped.
  2. Startup resume — owner re-check in _schedule_resume_pending_sessions now uses _is_user_authorized_scoped so a foreign process allowlist can’t revive a revoked owner.
  3. Adapter auth callback — _make_adapter_auth_check delegates to _is_user_authorized_scoped.
  4. Regression — tests/gateway/test_multiplex_auth_scope_bypass.py: foreign allowlist denied under empty secondary scope; scoped allowlist still authorizes; adapter allow_from still works; single-profile env behavior preserved; resume skips foreign-only owners.
    Cold path still uses bare _is_user_authorized on purpose — it’s already inside the profile message-handler scope.

@egilewski

Copy link
Copy Markdown
Contributor

fully addressed

The current head closes the cross-profile authorization gap from my earlier review. Busy-session and startup-resume checks now authorize inside the routed profile scope, secondary adapter callbacks bind their own profile, and multiplex allowlist/allow-all reads no longer inherit a foreign process-global value. On a clean replay onto current main, the foreign allowlist case reproduced before this change and was rejected by the scoped path while the profile's own allow_from remained accepted. The focused and adjacent auth/multiplex regression suites passed (92 tests).

Not checked:

  • CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

The current head still leaves a cross-profile authorization bypass in multiplexed Telegram and Slack adapter paths. Their wrapper handlers are closures rather than runner-bound methods, so the callback and intake gates fall back to process-global allowlists when no profile scope is installed. A value from another profile can therefore authorize Telegram approval/confirmation/clarify controls, Telegram message intake, or Slack interactive controls. Bind these paths to the routed profile-scoped authorization callback and make multiplex fallbacks deny on a missing scope or resolver failure, while preserving profile-local allow_from and single-profile behavior.

P3 follow-up: profile-route rejection and profile-directory resolution errors can also fall back to unscoped authorization. Rejected routes should be dropped and resolver failures should fail closed for busy, resume, and adapter-context callers.

Security evidence:

  • trust boundary: A multiplexed gateway shares process state while each profile supplies its own authorization values; adapter ingress and interactive controls must authorize only the routed profile.
  • source/sink/invariant: Sender and chat metadata reach adapter and gateway authorization sinks. A missing routed profile scope must not read another profile's process-global allowlist.
  • current-main reproduction: The current-main QQ scope regression returned authorized for an empty secondary scope when QQ_ALLOW_ALL_USERS=true existed only in the process environment; the required result is denial.
  • PR-head or patch-replay validation: The no-commit replay onto current main passed 24 focused gateway/multiplex tests. Direct no-network probes still returned authorized for foreign-only Telegram callback, Telegram intake, and Slack interactive allowlists.
  • positive/negative cases: Replay negatives reject foreign Discord/QQ values; profile-local values, adapter-local allow_from, and single-profile environment behavior remain accepted.
  • residual bypass search: The ordinary profile message path and the new bound adapter callback are scoped, but Telegram/Slack closure-valued adapter paths remain outside that binding; route/resolver fallback is retained as the lower-reachability P3 follow-up.
  • reviewer validation: Fresh Luna/max source review, independent Sol/xhigh final gate, live head/base/main checks, and exact direct probes support this finding.

Not checked:

  • full repository test suite
  • live Telegram and Slack event delivery
  • CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

@teknium1

Copy link
Copy Markdown
Contributor

Implemented on main via #93446 (2912c36). Your approach — routing the admission-gate reads (GATEWAY_ALLOW_ALL_USERS, per-platform + group allowlists, GATEWAY_ALLOWED_USERS) through the scope-authoritative _platform_gate_env — is exactly what landed; there _auth_env itself now delegates to _platform_gate_env so all its callers get the isolation. Closing as resolved; thanks @OWWZO.

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 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.

[Bug]: Discord multiplex gateway second auth gate can apply another profile’s allowlist after #75970

4 participants