Skip to content

Feat/feishu: group chat allowlist - #29709

Closed
clarkchen wants to merge 7 commits into
NousResearch:mainfrom
clarkchen:feat/feishu-group-chat-allowlist
Closed

Feat/feishu: group chat allowlist#29709
clarkchen wants to merge 7 commits into
NousResearch:mainfrom
clarkchen:feat/feishu-group-chat-allowlist

Conversation

@clarkchen

Copy link
Copy Markdown

What does this PR do?

Adds Feishu support for chat-scoped group authorization via FEISHU_GROUP_ALLOWED_CHATS.

Previously, Feishu group messages could pass the Feishu adapter's group policy, but could still be rejected by the gateway's global user authorization layer unless the sender was individually authorized through pairing or FEISHU_ALLOWED_USERS.

That made it hard to support this deployment mode:

  • allow anyone in a specific Feishu group to @mention Hermes
  • keep private DMs closed to unpaired / unauthorized users
  • avoid setting FEISHU_ALLOW_ALL_USERS=true, which would open access globally

This PR wires FEISHU_GROUP_ALLOWED_CHATS into the gateway authorization checks for Feishu, matching the existing chat-scoped authorization pattern used by other platforms.

Related Issue

No existing issue.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/run.py

    • Add Feishu support to the gateway group-chat authorization map.
    • Allow configured Feishu group chats from FEISHU_GROUP_ALLOWED_CHATS to pass authorization without requiring every sender to be individually paired.
    • Preserve existing DM behavior: unpaired / unauthorized users remain denied in private chats.
  • tests/gateway/test_feishu_bot_auth_bypass.py

    • Add regression coverage for Feishu group chat authorization.
    • Verify that a Feishu group message from an otherwise unauthorized user is allowed when the group chat is listed in FEISHU_GROUP_ALLOWED_CHATS.
    • Verify that the same unauthorized user is still denied in DM.
  • website/docs/user-guide/messaging/feishu.md

    • Document how to configure FEISHU_GROUP_ALLOWED_CHATS.
    • Clarify the recommended setup with FEISHU_ALLOW_ALL_USERS=false.
    • Clarify that group messages still follow mention policy and DMs remain closed to unpaired users.

How to Test

  1. Run the focused gateway test:

    venv/bin/python -m pytest tests/gateway/test_feishu_bot_auth_bypass.py -q -o 'addopts='
  2. Confirm the test passes:

    7 passed
    
  3. Optional manual verification with Feishu gateway configuration:

    FEISHU_ALLOW_ALL_USERS=false
    FEISHU_GROUP_POLICY=open
    FEISHU_GROUP_ALLOWED_CHATS=oc_xxx

    Expected behavior:

    • users in the configured Feishu group can @mention Hermes
    • unpaired users cannot DM Hermes
    • other groups remain unauthorized unless separately allowed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Ubuntu / Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Focused test run:

$ venv/bin/python -m pytest tests/gateway/test_feishu_bot_auth_bypass.py -q -o 'addopts='
.......                                                                  [100%]
7 passed

@clarkchen
clarkchen force-pushed the feat/feishu-group-chat-allowlist branch from cef712e to 9e9a49e Compare May 21, 2026 07:42
@clarkchen clarkchen changed the title Feat/feishu group chat allowlist Feat/feishu: group chat allowlist May 21, 2026
@daimon-nous daimon-nous Bot added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter labels May 21, 2026
@clarkchen

Copy link
Copy Markdown
Author

@teknium1 hello, pls review this mr

Copy link
Copy Markdown
Author

Thanks for pointing out the overlap with #33715. I updated this PR to consolidate the useful additions from that PR here:

  • added FEISHU_GROUP_ALLOWED_CHATS to the environment variable reference docs
  • added it to the Feishu all-env-vars table
  • added coverage that a Feishu group chat allowlist makes unauthorized DMs silent by default
  • generalized the gateway authorization comments so the chat-scoped allowlist behavior is no longer Telegram-specific

Focused test run:

.venv/bin/python -m pytest tests/gateway/test_feishu_bot_auth_bypass.py -q -o addopts=
9 passed in 0.78s

clarkchen added 3 commits May 28, 2026 21:56
…hat-allowlist

# Conflicts:
#	website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/reference/environment-variables.md
#	website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/user-guide/messaging/feishu.md

Copy link
Copy Markdown
Author

Follow-up: I also added the zh-Hans documentation coverage from #33715 and merged current upstream/main so the PR diff stays focused.

Additional coverage now includes:

  • zh-Hans environment variables reference entry for FEISHU_GROUP_ALLOWED_CHATS
  • zh-Hans Feishu messaging guide section explaining chat-scoped group allowlists
  • zh-Hans Feishu all-env-vars table entry

Re-ran the focused test after the merge:

.venv/bin/python -m pytest tests/gateway/test_feishu_bot_auth_bypass.py -q -o addopts=
9 passed in 0.21s

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused implementation, regression coverage, and the consolidation of the related #33715 documentation work. This automated hermes-sweeper review is closing the PR because its user-facing configuration mechanism conflicts with a standing policy.

  • The PR adds and documents FEISHU_GROUP_ALLOWED_CHATS as a new non-secret authorization-policy environment variable (gateway/run.py in d1921976dee1d932f8e4815bc7c06494b7f6f19b).
  • AGENTS.md:102-106 requires behavioral settings to be configured through config.yaml, rather than a new .env variable. Authorization scope is a behavioral access-policy setting.
  • A focused re-scope can build on Feishu's existing platforms.feishu.extra.group_rules configuration path (plugins/platforms/feishu/adapter.py:1503-1522; website/docs/user-guide/messaging/feishu.md:491-529) instead of introducing another environment-variable surface.
  • The overlapping-PR discussion is acknowledged. Maintainer review of competing feat(feishu): support group chat allowlist #61376 applied this same policy to this exact proposed environment-variable mechanism.

Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment. If you believe the policy was misapplied, comment for maintainer review.


Closed as not-planned per standing maintainer policy (env-var-for-config). This is a design-direction decision, not a code-quality judgment — see the Contribution Rubric in AGENTS.md for what the project is looking for. If you believe this policy was misapplied to your change, comment here and a maintainer will take a look.

@teknium1 teknium1 closed this Jul 13, 2026
@teknium1 teknium1 added the sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/feishu Feishu / Lark adapter sweeper:not-planned Sweeper: closed per standing maintainer policy (design direction) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants