Improve IM bot setup and diagnostics / 优化 IM 机器人设置与诊断#4070
Conversation
github-actions
Bot
added
v2
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6814433f65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tings # Conflicts: # desktop/frontend/src/components/SettingsPanel.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6951bca81f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| userID := firstNonEmpty( | ||
| payload.Event.Action.Value["user_id"], | ||
| payload.Event.Operator.OperatorID.UnionID, | ||
| payload.Event.Operator.OperatorID.OpenID, | ||
| payload.Event.Operator.OperatorID.UserID, | ||
| payload.Event.Operator.UnionID, | ||
| payload.Event.Operator.OpenID, | ||
| payload.Event.Operator.UserID, | ||
| ) |
There was a problem hiding this comment.
When a Feishu/Lark approval or Ask card is posted in a group, the card value includes the original requester's user_id, and this code trusts that value before the actual button-click operator IDs. That means any group member who can see the card can click an approve/deny/answer button and the injected inbound message is processed as the original allowlisted requester and routed to their session, bypassing the normal allowlist/user check for the real actor. Prefer authenticating against the operator ID and rejecting mismatches (or otherwise proving the operator is allowed) while keeping any routing-only requester ID separate.
Useful? React with 👍 / 👎.
| if strings.EqualFold(ch, "lark") { | ||
| enabled[bot.PlatformFeishu] = PlatformConfigured(cfg, bot.PlatformFeishu) |
There was a problem hiding this comment.
When reasonix bot start --channels lark is used, this collapses the request to PlatformFeishu, and AdapterBindings later filters only by provider, so every enabled Feishu-family connection is started (including domestic Feishu); --channels feishu likewise starts Lark. This makes the documented channel selector unable to isolate one tenant/domain and can start the wrong bot credentials in workspaces that have both connections configured.
Useful? React with 👍 / 👎.
esengine
left a comment
esengine
left a comment
There was a problem hiding this comment.
Thanks for the thorough work here — the independent per-connection runtime, the degraded-status handling, and the fixes from the earlier round (distinct group-user remembering + serialized config persistence, both with regression tests) all look good.
Before this goes in, the P1 from the latest Codex pass is a real blocker. I traced it end-to-end and it holds:
approvalCard(...)/askCard(...)are rendered with the session'schatType+userID, so in a group session the approve/deny/answer card is posted into the group.- In
handleCardAction,firstNonEmpty(payload.Event.Action.Value["user_id"], ...)trusts the requester id baked into the card before the real click operator. So any group member who can see the card — including one not in the user allowlist — can click a button and the inbound is attributed to the original requester. checkAllowlistthen gates on thatmsg.UserID, passes, and/approve <id>resolves against the group session's pending approvals.
Net effect: in a group with tool_approval_mode = ask and a strict allowlist, an unauthorized member can approve a pending tool call as the requester. That defeats the approval gate, which is the entire point of the gate. The two card-action tests currently bless this (msg.UserID == "allowed-user" taken from the card value), so they'd need to flip with the fix.
Fix: gate the allowlist on the actual operator (Operator.OperatorID.* / Operator.*) and treat value["user_id"] as routing-only, rejecting when the operator isn't allowed. TestHandleCardActionAcceptsDirectOperatorID already shows the operator id is present on the event, so this is a small change.
Also still open from the same pass — the P2 on internal/botruntime/runtime.go: --channels lark and --channels feishu both collapse to PlatformFeishu, and AdapterBindings filters by provider only, so with both a Feishu and a Lark connection enabled the selector starts both (wrong-tenant credentials can come up). Lower priority than the P1, but worth folding in since the per-connection Domain / runtime id is already threaded through.
Happy to merge as soon as the operator-auth fix lands.
2 participants
Summary
Verification
wails generate modulefrom the desktop modulepnpm --dir desktop/frontend buildgo test ./internal/bot ./internal/bot/feishu ./internal/bot/weixin ./internal/botruntime ./internal/cligo test ./internal/bot/weixin ./internal/botruntime ./internal/cli ./internal/bot/...go test ./internal/cli ./internal/botruntime ./internal/configgo test ./...go test .from the desktop modulegit diff --checkPROD_TEST_OPEN_DIST=0 ./prod_test darwin/arm64