Conversation
a6ebe4e to
aa4215f
Compare
|
Hi @RiptideV — running into this exact use case (community manager bot replying to non-admin members in an allowlisted WhatsApp group), so very interested in this PR landing. I think there may be a coverage gap that's worth surfacing before merge, since I think it impacts the documented testing flow: The gateway-level if (!msg.key.fromMe) {
if (WHATSAPP_MODE === 'self-chat') {
// ignored: self_chat_mode_rejects_non_self
continue;
}
if (!matchesAllowedUser(senderId, ALLOWED_USERS, SESSION_DIR)) {
// ignored: allowlist_mismatch
continue;
}
}
Concrete evidence from my own bridge log: every recent event from an allowlisted Two possible workarounds at the bridge level both have downsides:
So in practice this PR's group allowlist only takes effect if operators are willing to drop their bridge-level DM allowlist, which seems counter to the intent. Two ways to close this:
Thanks for pushing this — broadly I think (2) is the cleaner mental model, but happy to follow whichever direction the maintainers prefer. |
aa4215f to
ab741da
Compare
|
Ran into this exact gap in production and wanted to share a finding that I think is orthogonal to (and complementary with) this PR's approach — it's about a second authorization gate that an env-var-only fix leaves uncovered. The blind spot: config-driven
|
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tackling the WhatsApp group/DM split. The underlying issue remains real on current main: scripts/whatsapp-bridge/bridge.js:637 applies the finite DM allowlist before Python sees group traffic, while the adapter already supports independent group_policy / group_allow_from at plugins/platforms/whatsapp/adapter.py:409-412.
Problems
- The PR's gateway hunk targets the pre-refactor
gateway/run.py; current authorization is ingateway/authz_mixin.py:264. Its per-userWHATSAPP_ALLOWED_USERScheck atgateway/authz_mixin.py:561-585also needs the WhatsApp group-scoped exception, or bridge-admitted group traffic is rejected by the second gate. whatsapp.group_allow_fromis the current canonical config path and is exported toWHATSAPP_GROUP_ALLOWED_USERSatplugins/platforms/whatsapp/adapter.py:1738-1742; the additionalgroup_allowed_usersalias should not create a competing configuration surface.
Suggested changes
- Port the bridge group bypass and the group-scoped gateway authorization into the current files, keeping the grant group-only so it never authorizes a DM.
- Add an integration regression for finite DM allowlists plus an allowlisted group, including the DM-denial case.
Automated hermes-sweeper review.
| if isinstance(gaf, list): | ||
| gaf = ",".join(str(v) for v in gaf) | ||
| os.environ["WHATSAPP_GROUP_ALLOWED_USERS"] = str(gaf) | ||
| gac = whatsapp_cfg.get("group_allowed_users") |
There was a problem hiding this comment.
Please do not add group_allowed_users as a second WhatsApp config key. Current main uses canonical whatsapp.group_allow_from, and plugins/platforms/whatsapp/adapter.py:1738-1742 already exports it to WHATSAPP_GROUP_ALLOWED_USERS; port the behavior through that existing path.
What does this PR do?
Adds WHATSAPP_GROUP_ALLOWED_USERS group allowlist support like SIGNAL_GROUP_ALLOWED_USERS
Type of Change
How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A