Skip to content

fix(gateway): enforce non-empty allowlists for config-driven platform policies - #52533

Closed
CryptoByz wants to merge 1 commit into
NousResearch:mainfrom
CryptoByz:fix/own-policy-empty-allowlist-fail-closed
Closed

fix(gateway): enforce non-empty allowlists for config-driven platform policies#52533
CryptoByz wants to merge 1 commit into
NousResearch:mainfrom
CryptoByz:fix/own-policy-empty-allowlist-fail-closed

Conversation

@CryptoByz

Copy link
Copy Markdown
Contributor

Fixes #38638

What changed and why

When config-driven platforms (like QQBot, WeCom, Weixin, Yuanbao, or WhatsApp) are enabled, the gateway runner's authorization check is bypassed under the assumption that the platform adapter has already evaluated caller authorization at intake. However, if the operator selects the allowlist policy but provides an empty or absent user allowlist, the gateway runner incorrectly returns True (authorized), bypassing default-deny security boundaries.

To resolve this, the gateway runner now checks that config-driven adapters configured for the allowlist policy actually provide a non-empty caller allowlist. If the allowlist is empty or absent, the gateway runner correctly defaults to denying access.

Fix

gateway/authz_mixin.py

  • Added helper methods _adapter_has_dm_allowlist and _adapter_has_group_allowlist to check both live adapter instances and config files for non-empty allowlists.
  • Updated _is_user_authorized to require _adapter_has_dm_allowlist / _adapter_has_group_allowlist to return True before trusting config-driven "allowlist" policy checks.

What this does NOT change

  • Does not change default pairing behaviors when no allowlists or restrictive policies are set.
  • Does not alter env-based allowlist resolution, global allowlists, or per-platform override flags.

How to test

Ran the test suite:

.venv/bin/pytest tests/gateway/test_config_driven_access_policy.py

Platforms tested

Linux (pure Python fix).

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data needs-decision Awaiting maintainer decision before any implementation labels Jun 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Competing fix for #38638 alongside open #38639. This PR enforces a non-empty allowlist in gateway/authz_mixin.py (_adapter_has_dm_allowlist/_adapter_has_group_allowlist); #38639 instead keeps default-deny unless the adapter proves a non-empty allowlist (touches gateway/run.py + QQBot). Same goal, different mechanism — flagging as related/needs-decision since it reverses the merged #34515/#34572 trust-adapter direction. Note: this branch also bundles an unrelated plugins/cron/plugins/cron_providers/ chronos rename; please isolate the authz fix for review.

@CryptoByz
CryptoByz force-pushed the fix/own-policy-empty-allowlist-fail-closed branch from 0713b2f to 120a7fa Compare June 25, 2026 15:35
@CryptoByz

Copy link
Copy Markdown
Contributor Author

Thanks for the triage notes!

Cron rename isolated: The plugins/cron → plugins/cron_providers rename commit has been removed from this branch. It was an accidental carry-over from a separate branch (fix/plugins-cron-package-collision). This PR now contains only the authz fix (2 files: gateway/authz_mixin.py + the corresponding test).

On the competing approach (#38639): Both PRs target the same vulnerability. The key difference:

I believe the authz_mixin.py approach is more complete since it protects every adapter that uses enforces_own_access_policy, not just QQBot. Happy to coordinate with #38639 author or rebase on top of it if the maintainers prefer the run.py approach.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused security hardening. This is an automated hermes-sweeper review: the requested fail-closed behavior is already implemented on current main.

  • Merged PR fix(security): fail closed own-policy gateway adapters #45634, via fc463545804692c16f842aac58d681d96dd3fe6a, changed own-policy adapter authorization to trust only an effective allowlist policy; open, pairing, and other policies fall through to the default-deny path. See gateway/authz_mixin.py:467-514.
  • Current regression coverage denies both explicitly configured and adapter-resolved default dm_policy: open without an allowlist. See tests/gateway/test_config_driven_access_policy.py:165-196.
  • The earlier related-PR discussion identified the same vulnerability; fix(security): fail closed own-policy gateway adapters #45634 merged the complete gateway-level fix, including the WeCom per-group allowlist sibling case.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 15, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(gateway): own-policy adapters fail open without allowlists

3 participants