Skip to content

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

Closed
coygeek wants to merge 1 commit into
NousResearch:mainfrom
coygeek:fix/own-policy-adapters-fail-open-without-allowlists
Closed

fix(gateway): own-policy adapters fail open without allowlists#38639
coygeek wants to merge 1 commit into
NousResearch:mainfrom
coygeek:fix/own-policy-adapters-fail-open-without-allowlists

Conversation

@coygeek

@coygeek coygeek commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

With no environment allowlist configured, an own-policy adapter such as QQBot can accept a remote caller under its default-open DM or group policy and the gateway then returns authorized before checking GATEWAY_ALLOW_ALL_USERS.

The protected resource is Hermes work dispatch and output delivery across a network-facing gateway adapter.

  • The gateway has a no-allowlist branch that returns True for own-policy adapters.
  • QQBot's default policy is open, not allowlist-only or disabled.
  • The adapter allow helpers return True for any policy other than disabled or allowlist.
  • This lets an unallowlisted remote caller dispatch Hermes work.

Keep the gateway default-deny unless the adapter proves a non-empty caller allowlist, or change default own-policy adapter settings from open to allowlist-only or disabled.

Linked context

Closes #38638

Real behavior proof (required for external PRs)

Affected component (issue scope)

File: gateway/run.py:7087-7098

        if not platform_allowlist and not group_user_allowlist and not group_chat_allowlist and not global_allowlist:
            if self._adapter_enforces_own_access_policy(source.platform):
                return True
            return os.getenv("GATEWAY_ALLOW_ALL_USERS", "").lower() in {"true", "1", "yes"}

Secondary paths: gateway/platforms/base.py:1850-1869 documents own-policy adapter trust; gateway/platforms/qqbot/adapter.py:210-217 defaults DM/group policy to open; gateway/platforms/qqbot/adapter.py:3139-3151 allows non-disabled policies.

Files changed in this PR

  • gateway/run.py
  • gateway/platforms/qqbot/adapter.py
  • tests/gateway/test_config_driven_access_policy.py

Behavior reproduced or verified

  1. Against current main commit f66a929a6b78b81bd31a634f05798431b0fb10aa, enable QQBot with valid credentials and leave all QQ/Gateway allowlists unset.
  2. Keep dm_policy and group_policy omitted so QQBot defaults them to open.
  3. Send a remote QQ DM or group mention from an unlisted caller.
  4. Observe that QQBot intake accepts the event and GatewayRunner._is_user_authorized() returns True through the own-policy branch.
  5. Expected: network adapters deny dispatch until a concrete allowlist exists.

Expected fixed behavior

Keep the gateway default-deny unless the adapter proves a non-empty caller allowlist, or change default own-policy adapter settings from open to allowlist-only or disabled.

Tests and validation

  • bash scripts/run_tests.sh tests/gateway/test_config_driven_access_policy.py
  • Result: 1 files, 43 tests passed, 0 failed (100% complete) in 1.5s (20 workers) ===

Environment

Verified against latest upstream main via source receipt (source_ready, upstream-main, f66a929a6b78b81bd31a634f05798431b0fb10aa). Full-public submit-fix route is approved for public issue plus linked review-ready PR publication. Redaction checked: no secrets, tokens, credentials, private logs, or unrelated local paths are included.

Risk checklist

  • Security/auth/secrets impact: Yes; this is a full-public security route.
  • Approval gate: Current-turn full-public approval evidence was confirmed before live publication.
  • Public disclosure safety: Redaction and public-body validation run before GitHub mutation.
  • Regression risk: Scoped to the linked fix branch and covered by the tests above.
  • Issue-body contract: Unchanged; the linked issue carries the canonical security disclosure.

Require adapter-owned gateway authorization to be backed by a concrete configured allowlist before bypassing the gateway default-deny path. Open dm_policy/group_policy settings now fall through unless the operator also configured an explicit allow-all or allowlist.

Add regression coverage for open DM/group denial, config-only allowlist acceptance, and non-matching sender rejection across own-policy adapters.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/gateway Gateway runner, session dispatch, delivery area/auth Authentication, OAuth, credential pools platform/qqbot QQ Bot adapter P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation labels Jun 4, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review: PR #38639 — fix(gateway): own-policy adapters fail open without allowlists

Verdict: Approved

Narrows the security contract so dm_policy: open / group_policy: open can no longer implicitly authorize all callers:

  • _source_matches_configured_access_policy requires a concrete configured allowlist before the gateway trusts an adapter's intake decision.
  • Adds normalization helpers for YUANBAO (group: prefix), WECOM/QQBOT (case + wecom: prefix).

Looks Good

  • Expands the test matrix: open vs. allowlisted DM and group paths for YUANBAO/WECOM/QQBOT, plus guild-aware QQBot matching.
  • Removes the prior over-broad “trust the adapter flag” fast-path that was the root cause of the fail-open.

Minor suggestion (non-blocking)

  • _group_extra is on the hot path for every group message — consider caching/memoization if latency profiles show it material.

@egilewski

Copy link
Copy Markdown
Contributor

merge conflicts

This PR is currently not reviewable against current main: GitHub reports mergeable=CONFLICTING / mergeStateStatus=DIRTY, and the local current-main check failed before a tree could be produced:

git merge-tree --write-tree origin/main origin/pr/38639

The local result was fatal: refusing to merge unrelated histories with current origin/main at a6a28ce3e2174c0be494f553ab5fd79b7039190f and PR head b3c02cce3eb1348f5325a8d988315e6f4a8ff858. Please rebase or otherwise refresh the branch onto current main; a substantive review would be premature until the submitted branch has related, mergeable history.

Signed: GPT-5-high in Codex

@teknium1

Copy link
Copy Markdown
Contributor

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

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added 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 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 14, 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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists platform/qqbot QQ Bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main 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.

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

5 participants