Skip to content

fix(security): fail closed own-policy gateway adapters - #45634

Merged
teknium1 merged 2 commits into
mainfrom
fix/salvage-45444-own-policy
Jun 13, 2026
Merged

fix(security): fail closed own-policy gateway adapters#45634
teknium1 merged 2 commits into
mainfrom
fix/salvage-45444-own-policy

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Own-policy gateway adapters now fail closed when no allowlist is configured, while preserving WeCom per-group sender allowlists.

This salvages #45444 from @Que0x and adds one maintainer follow-up for the WeCom groups.<group_id>.allow_from sibling case.

Changes

  • gateway/authz_mixin.py: trust own-policy adapters only for effective allowlist policy, not default open / pairing flows.
  • gateway/authz_mixin.py: preserve WeCom group-level sender allowlists as a trusted adapter-enforced restriction even when top-level group_policy is open.
  • tests/gateway/test_config_driven_access_policy.py: covers fail-closed DM/group defaults, allowlist trust, pairing behavior, and WeCom per-group sender allowlists.

Validation

Check Result
python3 -m py_compile gateway/authz_mixin.py tests/gateway/test_config_driven_access_policy.py pass
scripts/run_tests.sh tests/gateway/test_config_driven_access_policy.py 45 passed

Authorship

Infographic

Gateway Access: Fail Closed

Que0x and others added 2 commits June 13, 2026 06:42
…allowlist

Own-policy adapters (WhatsApp, WeCom, Weixin, QQBot, Yuanbao) default dm_policy/group_policy to "open", which forwards every sender. The gateway's adapter-trust shortcut in _is_user_authorized blanket-trusted those platforms when no env allowlist was set, so an operator who enabled one with only credentials authorized the entire external network -- the fail-open SECURITY.md section 2.6 forbids ("an allowlist is required for every enabled network-exposed adapter").

Trust the adapter only when its effective policy for the chat type is an actual "allowlist" restriction (the case #34515 was protecting). "open"/"pairing"/anything else falls through to default-deny, where {PLATFORM}_ALLOW_ALL_USERS / GATEWAY_ALLOW_ALL_USERS and the pairing flow remain the explicit opt-ins.
Keep the own-policy fail-closed hardening from PR #45444, but still trust WeCom groups.<id>.allow_from because the adapter already checked that sender allowlist before dispatching to gateway auth.
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/salvage-45444-own-policy vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 10878 on HEAD, 10876 on base (🆕 +2)

🆕 New issues (2):

Rule Count
unresolved-attribute 2
First entries
run_agent.py:2891: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

✅ Fixed issues (1):

Rule Count
invalid-assignment 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

Unchanged: 5707 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: security-critical fail-closed fix looks solid.

Reviewed the full diff across gateway/authz_mixin.py, gateway/platforms/base.py, and tests/gateway/test_config_driven_access_policy.py.

What was checked:

  1. Defense-in-depth — the old code trusted enforces_own_access_policy as a blanket "already authorized" pass, which was fail-open for dm_policy: open / group_policy: open. The fix correctly gates trust on effective_policy == "allowlist" only.
  2. Group policy path_adapter_group_policy() mirrors _adapter_dm_policy() correctly (live adapter → config.extra fallback). The WeCom per-group sender allowlist (groups.<id>.allow_from) is handled as a separate trustworthy restriction even when the top-level group_policy is open.
  3. Test coverage — 6 new parametrized tests cover: allowlist-trusted, open-denied, default-open-denied, group-allowlist-trusted, group-open-denied, and per-group sender allowlist. The existing pairing-carveout test was updated to use allowlist instead of open (correct — the old test was testing the fail-open path).
  4. No dead code — the old if not (source.chat_type == "dm" and dm_policy == "pairing") guard is cleanly replaced by the new if effective_policy == "allowlist" gate.

CI note: The single failure is test_session_create_no_race_keeps_worker_alive — a known flaky race-sensitive test in a completely unrelated file (test_tui_gateway_server.py). Not caused by this PR.

Clean security fix. LGTM.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists labels Jun 13, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: security architecture review — LGTM

Reviewed the full diff across gateway/authz_mixin.py, gateway/platforms/base.py, and the test file. This is a well-structured security fix that closes a real fail-open gap.

What was checked:

  1. Fail-open → fail-closed transition: The previous logic trusted enforces_own_access_policy unconditionally, meaning dm_policy: open (which forwards every sender) was treated as "already authorized." The fix correctly gates trust on the effective policy being "allowlist" — not "open", "pairing", or anything else.

  2. Group policy parity: The new _adapter_group_policy() and _adapter_group_has_sender_allowlist() methods mirror the DM-side logic cleanly. WeCom's per-group groups.<id>.allow_from is correctly treated as an adapter-enforced restriction (trustworthy) even when the top-level group_policy is "open".

  3. Pairing carveout preserved: The pairing DM flow still falls through to default-deny (unpaired senders get a pairing code, not blanket access). Group traffic is governed by group_policy, not the DM pairing carveout — correct.

  4. effective_policy scoping: In the DM path, effective_policy is always set before the if effective_policy == "allowlist" check. The group path returns early on sender-allowlist match, so the fallthrough only reaches the policy check for groups without a per-group allowlist — correct control flow.

  5. Test coverage: 7 new parametrized tests covering allowlist DM, open DM (denied), default open (denied), allowlist group, open group (denied), WeCom per-group sender allowlist, and wildcard group config. All _OWN_POLICY_PLATFORMS are tested via parametrize.

  6. Comment/docstring accuracy: The updated docstrings in base.py:enforces_own_access_policy and the _is_user_authorized block comment accurately describe the new semantics. The SECURITY.md §2.6 reference is appropriate.

No issues found. The fix is architecturally sound and the test suite covers the critical decision branches.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants