Conversation
Add --request-receipt to +send / +reply / +forward / +draft-create / +draft-edit. When set, the Disposition-Notification-To header (RFC 3798) is written into the outgoing EML with the resolved sender address, requesting an MDN from the recipient's mail client. The recipient's client may prompt the user, send automatically, or silently ignore — delivery of a receipt is not guaranteed. +draft-edit routes the flag through the existing set_header patch op against the draft's parsed From address, so no patch schema extension is needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add mail +send-receipt, intended for agent use after the user confirms
replying to an incoming message that requested a read receipt.
Flow:
1. Fetch the original message; verify its label_ids include
READ_RECEIPT_REQUEST (or -607). Refuse if absent.
2. Build a plain-text reply addressed to the original sender:
- Subject: "已读回执:<original subject>" (aligns with backend
GetRealSubject regex covering 13 languages).
- In-Reply-To / References threading from the original SMTP IDs.
- Private header X-Lark-Read-Receipt-Mail: 1 via the new
emlbuilder.IsReadReceiptMail() method.
3. Create a draft and send immediately via the existing drafts raw path.
The backend data-access layer extracts the private header into
BodyExtra.IsReadReceiptMail; DraftSend then applies the
READ_RECEIPT_SENT system label. No IDL, open-access, or smtp-* changes
are required on the CLI side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add --request-receipt row to the parameter tables of lark-mail-send / reply / forward / draft-create / draft-edit. - Add a new lark-mail-send-receipt.md reference describing the workflow, consent requirements, and behaviour of the backend marker header. - Register +send-receipt in the domain SKILL.md shortcut index and extend the typical workflow section with a read-receipt step that stresses explicit user consent before sending a receipt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
📝 WalkthroughWalkthroughThis pull request adds Message Disposition Notification (MDN) read-receipt support to the mail shortcuts. It introduces read-receipt request headers to existing mail commands (send, reply, forward, draft-create, draft-edit), implements a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI/Client
participant API as Mail API
participant Backend as Backend Service
participant Builder as EML Builder
User->>CLI: mail +send-receipt --message-id=X<br/>--from=user@example.com
CLI->>API: Fetch original message (message-id=X)
API->>Backend: GET /messages/{message-id}
Backend-->>API: Message + label_ids
API-->>CLI: Full message details
alt Message has READ_RECEIPT_REQUEST label
CLI->>Builder: Build receipt EML<br/>- Set From
Builder->>Builder: Construct Subject<br/>(已读回执: + original)
Builder->>Builder: Assemble In-Reply-To<br/>& References headers
Builder->>Builder: Set X-Lark-Read-Receipt-Mail: 1
Builder-->>CLI: Signed EML (base64url)
CLI->>API: Create draft (EML bytes)
API->>Backend: drafts.create
Backend-->>API: Draft ID
CLI->>API: Send draft
API->>Backend: drafts.send
Backend->>Backend: Set READ_RECEIPT_SENT label<br/>(-608)
Backend-->>API: Send confirmation
API-->>CLI: Success result
CLI->>User: ✓ Receipt sent for message-id=X
else Message lacks receipt-request label
CLI->>User: ✗ Error: Message does not<br/>request read receipt
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/mail/emlbuilder/builder.go`:
- Around line 301-314: The DispositionNotificationTo method currently validates
the display name but not the addr, allowing CR/LF header injection via
mail.Address.String(); update Builder.DispositionNotificationTo to call
validateHeaderValue(addr) (set b.err and return on failure) before appending the
address, mirror the same validation for any other address-appending builders
that serialize via joinAddresses, and add a regression test similar to
TestBuild_DispositionNotificationTo_CRLFRejected that supplies a
CR/LF-containing addr to assert it is rejected.
In `@shortcuts/mail/mail_draft_create.go`:
- Around line 157-159: Add a shortcut-level unit test in
mail_draft_create_test.go that exercises the mail draft create flow with the
--request-receipt flag to ensure runtime.Bool("request-receipt") actually causes
bld.DispositionNotificationTo("", senderEmail) to be set; implement the test to
invoke the same CLI or handler path used by the draft-create shortcut (or call
the function that constructs the builder), supplying senderEmail and the
--request-receipt flag, then assert the produced EML includes a
Disposition-Notification-To header equal to senderEmail (or that the builder
serialized header matches), mirroring the behavior covered by
emlbuilder/builder_test.go but at the shortcut level.
In `@shortcuts/mail/mail_draft_edit.go`:
- Around line 103-109: When request-receipt is specified but no draft sender can
be resolved, fail early instead of silently returning an empty patch: in
buildDraftEditPatch (and the counterpart code path in Execute that currently
appends the Disposition-Notification-To header only when draftFromEmail != ""),
detect runtime.Bool("request-receipt") && draftFromEmail == "" and return a
validation error indicating the request-receipt flag requires a draft From
address; update callers to propagate that error so the command reports failure
rather than a no-op success.
In `@skills/lark-mail/references/lark-mail-send-receipt.md`:
- Line 13: The confirmation rule for the +send-receipt command incorrectly
mentions the unsupported flag --confirm-send; update the documentation to remove
any reference to --confirm-send and instruct users to use --yes (as enforced by
the high-risk-write rule) so examples, parameter tables, and the +send-receipt
confirmation text consistently reference only --yes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c45ec8e-de67-4c08-ae9d-49da71322420
📒 Files selected for processing (17)
shortcuts/mail/emlbuilder/builder.goshortcuts/mail/emlbuilder/builder_test.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_draft_edit.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_send.goshortcuts/mail/mail_send_receipt.goshortcuts/mail/mail_send_receipt_test.goshortcuts/mail/shortcuts.goskills/lark-mail/SKILL.mdskills/lark-mail/references/lark-mail-draft-create.mdskills/lark-mail/references/lark-mail-draft-edit.mdskills/lark-mail/references/lark-mail-forward.mdskills/lark-mail/references/lark-mail-reply.mdskills/lark-mail/references/lark-mail-send-receipt.mdskills/lark-mail/references/lark-mail-send.md
| func (b Builder) DispositionNotificationTo(name, addr string) Builder { | ||
| if addr == "" { | ||
| return b | ||
| } | ||
| if b.err != nil { | ||
| return b | ||
| } | ||
| if err := validateDisplayName(name); err != nil { | ||
| b.err = err | ||
| return b | ||
| } | ||
| cp := b.copySlices() | ||
| cp.dispositionNotificationTo = append(cp.dispositionNotificationTo, mail.Address{Name: name, Address: addr}) | ||
| return cp |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Go net/mail Address.String validate or escape CR/LF characters in the Address field?
💡 Result:
No, the Go net/mail Address.String method does not validate or escape CR/LF characters (newlines) in the Address field. It processes the Address field by splitting at '@' into local and domain parts, then checks characters in the local part using isAtext (which excludes specials like <>[]:;@, but includes VCHAR excluding CR/LF). If any character fails isAtext or special dot rules, the local part is wrapped in double quotes via quoteString, which escapes " and \ but passes CR/LF through as they are VCHAR and not specials. The domain part is output unquoted and unescaped. Thus, unescaped newlines in Address.Name, Address.Address local/domain can appear in the output string, potentially enabling header injection if used in mail headers.
Citations:
- 1: https://github.com/golang/go/blob/master/src/net/mail/message.go
- 2: https://go.dev/src/net/mail/message.go?m=text
- 3: https://go.dev/src/net/mail/message.go
🏁 Script executed:
# Find the builder.go file and check for validation functions
fd -type f -name "builder.go" | head -5Repository: larksuite/cli
Length of output: 227
🏁 Script executed:
# Search for validateHeaderValue and validateDisplayName functions
rg "func.*validate" shortcuts/mail/emlbuilder/ -A 3Repository: larksuite/cli
Length of output: 1350
🏁 Script executed:
# Check lines around 301-314 and 701-703 to see the full context
sed -n '295,320p' shortcuts/mail/emlbuilder/builder.goRepository: larksuite/cli
Length of output: 999
🏁 Script executed:
# Check lines 701-703
sed -n '695,710p' shortcuts/mail/emlbuilder/builder.goRepository: larksuite/cli
Length of output: 626
🏁 Script executed:
# Search for tests of DispositionNotificationTo
rg "DispositionNotificationTo" shortcuts/mail/emlbuilder/Repository: larksuite/cli
Length of output: 1354
🏁 Script executed:
# Read the test to see what it actually tests (display name or address)
sed -n '1,100p' shortcuts/mail/emlbuilder/builder_test.go | grep -A 20 "TestBuild_DispositionNotificationTo_CRLFRejected"Repository: larksuite/cli
Length of output: 39
🏁 Script executed:
# Search for full test definition
rg "TestBuild_DispositionNotificationTo_CRLFRejected" shortcuts/mail/emlbuilder/builder_test.go -A 15Repository: larksuite/cli
Length of output: 576
🏁 Script executed:
# Check how other address methods (To, Cc, Bcc, ReplyTo) validate their address parameter
rg "func \(b Builder\) (To|Cc|Bcc|ReplyTo)\(" shortcuts/mail/emlbuilder/builder.go -A 15Repository: larksuite/cli
Length of output: 658
🏁 Script executed:
# Check if other methods (To, Cc, Bcc) have similar vulnerability tests
rg "TestBuild_(To|Cc|Bcc|ReplyTo).*CRLF|TestBuild_(To|Cc|Bcc|ReplyTo).*Address" shortcuts/mail/emlbuilder/builder_test.goRepository: larksuite/cli
Length of output: 39
🏁 Script executed:
# Look for all address validation tests across the builder_test.go
rg "func TestBuild.*\(" shortcuts/mail/emlbuilder/builder_test.go | grep -i "crlf\|address\|inject"Repository: larksuite/cli
Length of output: 278
🏁 Script executed:
# Check what joinAddresses function does and how it uses mail.Address.String()
rg "func joinAddresses" shortcuts/mail/emlbuilder/builder.go -A 10Repository: larksuite/cli
Length of output: 233
Validate the read-receipt address before serializing it into a header.
DispositionNotificationTo validates the display name with validateDisplayName but does not validate the addr parameter. Since joinAddresses serializes the address via mail.Address.String() (which does not escape CR/LF characters), unvalidated input in the address field can inject additional headers. Add validateHeaderValue(addr) to prevent header injection, and include a regression test for the address field.
Proposed fix
if err := validateDisplayName(name); err != nil {
b.err = err
return b
}
+ if err := validateHeaderValue(addr); err != nil {
+ b.err = err
+ return b
+ }
cp := b.copySlices()
cp.dispositionNotificationTo = append(cp.dispositionNotificationTo, mail.Address{Name: name, Address: addr})
return cpAdd a regression test alongside TestBuild_DispositionNotificationTo_CRLFRejected:
+func TestBuild_DispositionNotificationTo_AddressCRLFRejected(t *testing.T) {
+ _, err := New().
+ From("", "alice@example.com").
+ To("", "bob@example.com").
+ Subject("hi").
+ Date(fixedDate).
+ DispositionNotificationTo("", "alice@example.com>\r\nBcc: evil@example.com").
+ TextBody([]byte("body")).
+ Build()
+ if err == nil {
+ t.Fatal("expected address CRLF error, got nil")
+ }
+}Also applies to: 701-703
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/mail/emlbuilder/builder.go` around lines 301 - 314, The
DispositionNotificationTo method currently validates the display name but not
the addr, allowing CR/LF header injection via mail.Address.String(); update
Builder.DispositionNotificationTo to call validateHeaderValue(addr) (set b.err
and return on failure) before appending the address, mirror the same validation
for any other address-appending builders that serialize via joinAddresses, and
add a regression test similar to
TestBuild_DispositionNotificationTo_CRLFRejected that supplies a
CR/LF-containing addr to assert it is rejected.
| if runtime.Bool("request-receipt") && senderEmail != "" { | ||
| bld = bld.DispositionNotificationTo("", senderEmail) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether +draft-create has shortcut-level coverage for --request-receipt.
# Expect: at least one test around MailDraftCreate/buildRawEMLForDraftCreate that sets request-receipt
# and asserts Disposition-Notification-To is present in the generated EML.
rg -n -C3 --type=go 'request-receipt|Disposition-Notification-To|buildRawEMLForDraftCreate|MailDraftCreate' \
--iglob '*_test.go'Repository: larksuite/cli
Length of output: 8071
Add shortcut-level test for the --request-receipt flag.
Builder tests in emlbuilder/builder_test.go verify that DispositionNotificationTo serializes correctly, but there is no shortcut-level test in mail_draft_create_test.go that verifies +draft-create --request-receipt wires the flag into the header. This is a behavior change that requires an accompanying test per coding guidelines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/mail/mail_draft_create.go` around lines 157 - 159, Add a
shortcut-level unit test in mail_draft_create_test.go that exercises the mail
draft create flow with the --request-receipt flag to ensure
runtime.Bool("request-receipt") actually causes
bld.DispositionNotificationTo("", senderEmail) to be set; implement the test to
invoke the same CLI or handler path used by the draft-create shortcut (or call
the function that constructs the builder), supplying senderEmail and the
--request-receipt flag, then assert the produced EML includes a
Disposition-Notification-To header equal to senderEmail (or that the builder
serialized header matches), mirroring the behavior covered by
emlbuilder/builder_test.go but at the shortcut level.
| if runtime.Bool("request-receipt") && draftFromEmail != "" { | ||
| patch.Ops = append(patch.Ops, draftpkg.PatchOp{ | ||
| Op: "set_header", | ||
| Name: "Disposition-Notification-To", | ||
| Value: "<" + draftFromEmail + ">", | ||
| }) | ||
| } |
There was a problem hiding this comment.
Fail instead of silently skipping receipt requests without a draft sender.
With --request-receipt and no other ops, buildDraftEditPatch returns an empty patch, but Execute only appends the header when draftFromEmail != "". If the draft lacks a From address, the command can update nothing while reporting success. Return a validation error when the flag is set and the sender cannot be resolved.
Proposed fix
- if runtime.Bool("request-receipt") && draftFromEmail != "" {
+ if runtime.Bool("request-receipt") {
+ if draftFromEmail == "" {
+ return output.ErrValidation("unable to determine draft sender; set the draft From address before requesting a read receipt")
+ }
patch.Ops = append(patch.Ops, draftpkg.PatchOp{
Op: "set_header",
Name: "Disposition-Notification-To",
Value: "<" + draftFromEmail + ">",
})
}Also applies to: 301-309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@shortcuts/mail/mail_draft_edit.go` around lines 103 - 109, When
request-receipt is specified but no draft sender can be resolved, fail early
instead of silently returning an empty patch: in buildDraftEditPatch (and the
counterpart code path in Execute that currently appends the
Disposition-Notification-To header only when draftFromEmail != ""), detect
runtime.Bool("request-receipt") && draftFromEmail == "" and return a validation
error indicating the request-receipt flag requires a draft From address; update
callers to propagate that error so the command reports failure rather than a
no-op success.
|
|
||
| 1. **触发条件严格**:仅当拉信(`+message` / `+messages` / `+thread`)看到 `label_ids` 里有 `READ_RECEIPT_REQUEST` 时,才应该问用户是否发回执。对普通邮件**绝不**调用此命令。 | ||
| 2. **必须先问用户**:发回执之前**必须**向用户展示原邮件摘要(发件人、主题)并请求确认;用户明确同意后才执行。**不要替用户自动回执**——这会造成隐私泄露(告诉对方"我读了")。 | ||
| 3. **`--confirm-send` / `--yes` 不省略**:本命令被标记为 `high-risk-write`,框架要求 `--yes`。仅在用户确认后附上。 |
There was a problem hiding this comment.
Remove --confirm-send from the +send-receipt confirmation rule.
This command’s examples and parameter table use --yes, and the shortcut is enforced via high-risk-write; mentioning --confirm-send here can send users to an unsupported flag.
Proposed documentation fix
-3. **`--confirm-send` / `--yes` 不省略**:本命令被标记为 `high-risk-write`,框架要求 `--yes`。仅在用户确认后附上。
+3. **`--yes` 不省略**:本命令被标记为 `high-risk-write`,框架要求 `--yes`。仅在用户确认后附上。📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 3. **`--confirm-send` / `--yes` 不省略**:本命令被标记为 `high-risk-write`,框架要求 `--yes`。仅在用户确认后附上。 | |
| 3. **`--yes` 不省略**:本命令被标记为 `high-risk-write`,框架要求 `--yes`。仅在用户确认后附上。 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/lark-mail/references/lark-mail-send-receipt.md` at line 13, The
confirmation rule for the +send-receipt command incorrectly mentions the
unsupported flag --confirm-send; update the documentation to remove any
reference to --confirm-send and instruct users to use --yes (as enforced by the
high-risk-write rule) so examples, parameter tables, and the +send-receipt
confirmation text consistently reference only --yes.
Summary
Changes
Test Plan
lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
Release Notes
New Features
--request-receiptflag to mail commands (draft-create,draft-edit,forward,reply,send) to request read receipts when sending emails.mail +send-receiptcommand to respond with read receipts to incoming messages requesting them.Documentation