Skip to content

fix(signal): react in permitted groups regardless of DM allowlist - #92342

Open
pacewicz wants to merge 1 commit into
NousResearch:mainfrom
pacewicz:signal-group-trust
Open

pacewicz wants to merge 1 commit into
NousResearch:mainfrom
pacewicz:signal-group-trust

Conversation

@pacewicz

@pacewicz pacewicz commented Aug 22, 2026

Copy link
Copy Markdown

Problem

_reactions_enabled (gateway/platforms/signal.py) gates the 👀/✅ reactions only on the DM allowlist (SIGNAL_ALLOWED_USERS). A group message has already passed the SIGNAL_GROUP_ALLOWED_USERS intake gate before this fires, so it is already trusted — yet blanking SIGNAL_ALLOWED_USERS to deny stranger DMs also silenced the reactions inside permitted groups.

Fix

React when a group message's bare group id (chat_id_alt) is in SIGNAL_GROUP_ALLOWED_USERS, regardless of the DM allowlist.

Scope note

Narrowed to the reaction gate only. The parallel authz change (honoring SIGNAL_GROUP_ALLOWED_USERS in _is_user_authorized) is already covered by the existing cluster — #58187 / #52240 / #44706 — so it's dropped here to avoid duplication (per the triage note).

Testing

pytest signal / reaction suites green.

A group message has already passed the SIGNAL_GROUP_ALLOWED_USERS intake
gate before _reactions_enabled fires, so it is trusted. React even when the
human sender is absent from SIGNAL_ALLOWED_USERS -- otherwise blanking the DM
allowlist to deny DMs also silenced the reactions inside permitted groups.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/signal Signal CLI adapter area/auth Authentication, OAuth, credential pools sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 22, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #52240 and #53348 in the Signal group-authz cluster. This PR also covers the separate reaction gate; please consolidate the overlapping authorization changes before merge.

@pacewicz
pacewicz force-pushed the signal-group-trust branch from 7e40bfa to 58c77f2 Compare August 22, 2026 23:40
@pacewicz

Copy link
Copy Markdown
Author

Thanks — good catch. Narrowed this PR to just the reaction gate (_reactions_enabled), which the cluster doesn't cover (grep of #58187 for _reactions_enabled = 0). Dropped the authz commit since it duplicates #58187 / #52240 / #44706#58187's one-liner is the cleanest for the authz side, happy to see that land there.

@pacewicz pacewicz changed the title fix(signal): honor SIGNAL_GROUP_ALLOWED_USERS for group senders (reactions + authz) fix(signal): react in permitted groups regardless of DM allowlist Aug 22, 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 P2 Medium — degraded but workaround exists platform/signal Signal CLI adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants