Skip to content

fix(gateway): honor SIGNAL_GROUP_ALLOWED_USERS at gateway authorization - #52240

Open
Que0x wants to merge 1 commit into
NousResearch:mainfrom
Que0x:fix/signal-group-allowlist-gateway-authz
Open

fix(gateway): honor SIGNAL_GROUP_ALLOWED_USERS at gateway authorization#52240
Que0x wants to merge 1 commit into
NousResearch:mainfrom
Que0x:fix/signal-group-allowlist-gateway-authz

Conversation

@Que0x

@Que0x Que0x commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What & why

Signal group messages that passed the adapter's SIGNAL_GROUP_ALLOWED_USERS
filter were still rejected at the gateway as "Unauthorized user" unless the
sender was also in SIGNAL_ALLOWED_USERS. The gateway's chat-scoped
allowlist map in _is_user_authorized only covered Telegram and QQBot, and
Signal does not set enforces_own_access_policy, so an allowlisted group fell
through to default-deny. This contradicts the docs, where
SIGNAL_GROUP_ALLOWED_USERS=groupId1,groupId2 is documented to enable those
groups.

Fix

  • Add Platform.SIGNAL: "SIGNAL_GROUP_ALLOWED_USERS" to the chat-scoped
    allowlist map — parity with Telegram's TELEGRAM_GROUP_ALLOWED_CHATS.
  • Match chat_id_alt (the raw group id) in addition to chat_id, since
    Signal sets chat_id to group:<id> while the env var lists raw ids.
    Telegram/QQBot leave chat_id_alt None, so their behavior is unchanged.

Scope is intentionally limited to group authorization; DM pairing behavior is
untouched.

Tests

Added to tests/gateway/test_signal.py::TestSignalAuthorization:

  • test_signal_group_authorized_by_group_allowlist — group in the allowlist
    authorizes its senders even without SIGNAL_ALLOWED_USERS.
  • test_signal_group_authorized_by_wildcard* authorizes any group.
  • test_signal_group_not_in_allowlist_denied — a group not listed is denied
    (no fail-open).

Results

  • New + existing Signal authz tests: 4 passed.
  • Regression check across the authorization suites (test_signal,
    test_unauthorized_dm_behavior, test_telegram_group_gating,
    test_allowlist_startup_check, test_config_driven_access_policy,
    test_relay_upstream_authz): 274 passed, 1 skipped.

Signal group messages that passed the adapter's SIGNAL_GROUP_ALLOWED_USERS
filter were rejected as "Unauthorized user" unless the sender was also in
SIGNAL_ALLOWED_USERS: the gateway's chat-scoped allowlist map only covered
Telegram and QQBot. Add Signal to the map and match chat_id_alt (the raw
group id), since Signal sets chat_id to "group:<id>".
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/signal Signal CLI adapter area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data duplicate This issue or pull request already exists labels Jun 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #44706 — same mechanism and same file: both add Platform.SIGNAL: "SIGNAL_GROUP_ALLOWED_USERS" to the gateway's chat-scoped allowlist in _is_user_authorized (gateway/authz_mixin.py) and reconcile the group:<id> vs raw-id form so allowlisted Signal group members are authorized like Telegram/QQBot. #44706 is the earlier open PR. This one additionally matches chat_id_alt; if that delta is preferred, fold it into #44706. Cross-linking so a maintainer can pick one.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks — this is a sound, narrow fix for a current Signal authorization gap.

Current main's chat-scoped authorization gate only maps Telegram and QQBot at gateway/authz_mixin.py:329-343; Signal group traffic therefore falls through to the sender-only SIGNAL_ALLOWED_USERS path at gateway/authz_mixin.py:365-391. Meanwhile, the Signal adapter already filters groups against SIGNAL_GROUP_ALLOWED_USERS and builds chat_id="group:<id>" with the raw ID in chat_id_alt (gateway/platforms/signal.py:593-609, :701-710). The proposed map entry plus raw-ID comparison matches that contract and preserves the deny path for unlisted groups.

The tests exercise the real GatewayRunner._is_user_authorized path with realistic Signal SessionSource values, including raw-ID matching, wildcard authorization, and denial of an unlisted group. Existing documentation already promises the affected behavior at website/docs/user-guide/messaging/signal.md:136-142, so no documentation change is needed.

#44706 overlaps this narrow fix, while #53348 carries a broader Signal-group proposal; that is a maintainer selection question, not a defect in this patch.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026
@alt-glitch alt-glitch removed duplicate This issue or pull request already exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Correction: the prior duplicate relation to #44706 is stale. Current #52240 has a material raw chat_id_alt matching delta over #44706's prefix-only normalization and remains an open, salvageable competing repair; related, not duplicate.

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 platform/signal Signal CLI adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants