feat(channel): Phase 2 — Lark webhook security + durable dedup#177
feat(channel): Phase 2 — Lark webhook security + durable dedup#177
Conversation
Add cryptographic signature verification (SHA256), encrypted event decryption (AES-256-CBC), and durable actor-state backed dedup for the Lark channel runtime. Closes #125 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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: f7a8fb363a
ℹ️ 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".
| VerificationToken = entry.VerificationToken ?? string.Empty, | ||
| WebhookUrl = entry.WebhookUrl ?? string.Empty, | ||
| NyxUserToken = entry.NyxUserToken ?? string.Empty, | ||
| EncryptKey = entry.EncryptKey ?? string.Empty, |
There was a problem hiding this comment.
This adds EncryptKey to the projected document, but the callback path resolves registrations through IChannelBotRegistrationQueryPort.GetAsync, whose ToEntry mapper still omits document.EncryptKey. In production that leaves registration.EncryptKey empty, so encrypted Lark callbacks cannot be decrypted and signature verification is never activated even when users provide an encrypt key.
Useful? React with 👍 / 👎.
| var timestamp = header.TryGetProperty("create_time", out var ts) ? ts.GetString() ?? "" : ""; | ||
| var nonce = header.TryGetProperty("nonce", out var n) ? n.GetString() ?? "" : ""; |
There was a problem hiding this comment.
ParseInboundAsync builds the signature from header.create_time and header.nonce, but Lark webhook signatures are computed from request timestamp/nonce headers (X-Lark-Request-Timestamp and X-Lark-Request-Nonce) plus raw body. Using payload fields here can produce a different digest for valid signed callbacks, causing legitimate events to be dropped whenever encrypt_key is configured.
Useful? React with 👍 / 👎.
| if (http.Request.Headers.TryGetValue("X-Lark-Signature", out var sigHeader) && | ||
| !string.IsNullOrWhiteSpace(sigHeader)) | ||
| { |
There was a problem hiding this comment.
Signature verification only runs when X-Lark-Signature is present, so a caller can omit the header and bypass verification entirely. In the same code path, token verification is disabled when EncryptKey is set, so unsigned callbacks can still be accepted and processed under encrypt-key registrations.
Useful? React with 👍 / 👎.
| if (!string.IsNullOrEmpty(evt.Session.MessageId) && | ||
| !next.ProcessedMessageIds.Contains(evt.Session.MessageId)) | ||
| { | ||
| next.ProcessedMessageIds.Add(evt.Session.MessageId); |
There was a problem hiding this comment.
This persists Session.MessageId during ApplyRequested, which happens before chat dispatch is attempted. Because HandleInbound checks dedup IDs before pending-session recovery, a crash/deactivation between request persistence and successful dispatch can restore that ID as already processed and drop retried webhooks, leaving pending sessions stranded and user messages unprocessed.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## dev #177 +/- ##
==========================================
- Coverage 81.62% 81.60% -0.02%
==========================================
Files 741 741
Lines 47055 47055
Branches 6230 6230
==========================================
- Hits 38407 38401 -6
- Misses 5956 5960 +4
- Partials 2692 2694 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…ryPort + reject missing signature - ToEntry() now copies EncryptKey from document to entry (was dead code in prod) - Signature verification rejects when X-Lark-Signature header is missing but encrypt_key is configured (was an authentication bypass) - Add test for missing-signature rejection - Fix encrypted event test to include required signature header Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…TP headers + dedup after dispatch - Lark signature verification now reads timestamp/nonce from X-Lark-Request-Timestamp and X-Lark-Request-Nonce HTTP headers instead of JSON body fields (header.create_time/nonce) - Durable dedup ID persistence moved from ApplyRequested to ApplyInboundDispatched, emitted only after successful dispatch — prevents stranding sessions on crash between persist and dispatch - Added ChannelInboundDispatchedEvent proto message for post-dispatch dedup tracking - 156 channel runtime tests passing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 participant
Summary
Channel Runtime Phase 2: 生产级安全加固。
X-Lark-Signatureheader，SHA256(timestamp + nonce + encrypt_key + body)常量时间比较encrypt_key→ SHA256 → AES key，IV = ciphertext 前 16 字节）processed_message_ids持久化到 actor state（event sourcing），跨 actor 重启存活encrypt_key全链路: proto → GAgent → 注册端点 → 投影 → NyxId chat tool安全模型
encrypt_key+ 签名匹配encrypt_key+ 签名不匹配encrypt_key+ 加密 payloadencrypt_key架构决策
encrypt_key: 这是 Lark 的协议设计，一个密钥同时用于签名验证和事件解密X-Lark-Request-Timestamp/X-Lark-Request-Nonce请求头读取，而非 JSON body 字段ChannelInboundDispatchedEvent在 chat dispatch 成功后才持久化 messageId，避免 crash 导致 session 被 dedup 卡住改动文件
channel_runtime_messages.protoencrypt_key加入 Entry/Command/Document；processed_message_ids加入 ChannelUserState；新增ChannelInboundDispatchedEventLarkPlatformAdapter.csChannelBotRegistrationGAgent.csencrypt_keyChannelCallbackEndpoints.csencrypt_key参数ChannelBotRegistrationProjector.csencrypt_keyChannelRegistrationTool.csencrypt_key参数ChannelUserGAgent.csChannelInboundDispatchedEvent持久化 messageIdLarkPlatformAdapterTests.csChannelBotRegistrationStoreTests.cs关联 Issues
Ref #124per-message 身份解析— Cancelled（NyxID blocker 被 CEO 否决）Phase 2 剩余（deferred）
身份映射 bind flow（OAuth callback →— Cancelled（NyxID blocker 被 CEO 否决）ChannelUserBoundEvent）IProjectionWriteDispatcher.DeleteAsyncTest plan
encrypt_key→ 签名验证生效 → 加密事件正确解密🤖 Generated with Claude Code