Skip to content

feat(platforms): add require_mention + allowed_users gating to DingTalk#9367

Closed
yule975 wants to merge 1 commit into
NousResearch:mainfrom
yule975:feat/dingtalk-require-mention-and-allowed-users
Closed

feat(platforms): add require_mention + allowed_users gating to DingTalk#9367
yule975 wants to merge 1 commit into
NousResearch:mainfrom
yule975:feat/dingtalk-require-mention-and-allowed-users

Conversation

@yule975

@yule975 yule975 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Summary

DingTalk was the only messaging platform without group-mention gating or a per-user allowlist. Slack, Telegram, Discord, WhatsApp, Matrix, and Mattermost all expose these knobs via `config.yaml` + matching env vars; this change closes the gap for DingTalk using the same surface.

Config surface

```yaml
platforms:
dingtalk:
enabled: true
require_mention: true # env: DINGTALK_REQUIRE_MENTION
mention_patterns: # env: DINGTALK_MENTION_PATTERNS (JSON array)
- "^hermes"
free_response_chats: # env: DINGTALK_FREE_RESPONSE_CHATS (csv)
- "cidABC=="
allowed_users: # env: DINGTALK_ALLOWED_USERS (csv)
- "staff_1234"
```

Semantics (mirrors Telegram)

  • DMs are always accepted, subject to `allowed_users`.
  • Group messages are accepted only when one of:
    • chat ID is in `free_response_chats`
    • `require_mention` is disabled
    • bot was @mentioned (`dingtalk_stream` sets `is_in_at_list`)
    • text matches a configured regex wake-word
  • `allowed_users` matches sender's `sender_id` or `sender_staff_id` case-insensitively; a single `"*"` disables the check.

Rationale

Without this, any DingTalk user in a group chat can trigger the bot, which makes DingTalk less safe to deploy than the other platforms. A user's `config.yaml` today already accepts `require_mention` for dingtalk (the yaml loader silently lets it pass), but the value was ignored by the adapter.

Test plan

  • Syntax check (`python -m ast`) on both modified files
  • Unit-level smoke test of the new helpers:
    • `_dingtalk_require_mention` reads from `extra` and env with correct precedence
    • `_compile_mention_patterns` handles list / JSON / csv / newline-separated inputs
    • `_is_user_allowed` is case-insensitive, wildcard `"*"` disables, empty allowlist is permissive
    • `_message_matches_mention_patterns` behaves correctly for match/no-match
  • Integration test in a real DingTalk group (reviewer-side or future follow-up)

Files changed

  • `gateway/config.py` — +18 lines, yaml → env var bridge (parallel to Telegram block)
  • `gateway/platforms/dingtalk.py` — +142 lines, adapter helpers + gate in `_on_message`

No behaviour change when the new config keys are absent (opt-in).

DingTalk was the only messaging platform without group-mention gating or a
per-user allowlist. Slack, Telegram, Discord, WhatsApp, Matrix, and Mattermost
all support these via config.yaml + matching env vars; this change closes the
gap for DingTalk using the same surface:

Config:
  platforms.dingtalk.require_mention: bool   (env: DINGTALK_REQUIRE_MENTION)
  platforms.dingtalk.mention_patterns: list  (env: DINGTALK_MENTION_PATTERNS)
  platforms.dingtalk.free_response_chats: list  (env: DINGTALK_FREE_RESPONSE_CHATS)
  platforms.dingtalk.allowed_users: list     (env: DINGTALK_ALLOWED_USERS)

Semantics mirror Telegram's implementation:
- DMs are always accepted (subject to allowed_users).
- Group messages are accepted only when the chat is allowlisted, mention is
  not required, the bot was @mentioned (dingtalk_stream sets is_in_at_list),
  or the text matches a configured regex wake-word.
- allowed_users matches sender_id / sender_staff_id case-insensitively;
  a single "*" disables the check.

Rationale: without this, any DingTalk user in a group chat can trigger the
bot, which makes DingTalk less safe to deploy than the other platforms. A
user's config.yaml already accepts require_mention for dingtalk but the value
was silently ignored.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #11564 (#11564). Your commit was cherry-picked with authorship preserved (9039273). Thanks for closing the DingTalk parity gap — the Telegram-mirroring semantics and config.yaml → env var bridge were exactly the right shape.

Added 16 regression tests on top covering _is_user_allowed, _compile_mention_patterns (with invalid-regex and env-var cases), and _should_process_message (DM/group/mention/wake-word/free_response_chats branches).

One small footnote: while writing tests I noticed the env-var fallback in _compile_mention_patterns uses splitlines() first, so DINGTALK_MENTION_PATTERNS="^bot,^assistant" is parsed as a single pattern, not two — the comma-split branch only triggers when splitlines yields nothing. Documented the newline-separated form (which works) as the fallback. Worth a follow-up if CSV parity with other platforms matters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants