feat(mail): add draft preview URL to draft operations#438
Open
qiooo wants to merge 2 commits intolarksuite:mainfrom
Open
feat(mail): add draft preview URL to draft operations#438qiooo wants to merge 2 commits intolarksuite:mainfrom
qiooo wants to merge 2 commits intolarksuite:mainfrom
Conversation
- Add draftPreviewURL helpers for send-preview link generation - Integrate preview_url output in +draft-create, +draft-edit, +reply, +forward, +reply-all shortcuts - Add unit tests (7 test cases, all passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
domain/mail
📝 WalkthroughWalkthroughAdds unexported helpers to generate brand-aware HTTPS draft preview URLs (Feishu default) and integrates them into mail shortcuts so create/edit/reply/forward flows populate and optionally display a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/mail_forward.go`:
- Around line 215-227: The human-readable output writes unsanitized tip text
from out["tip"] (set near draftID/mailboxID) which can reintroduce
terminal-control injection; before passing to runtime.OutFormat's writer (the
fmt.Fprintf that prints out["tip"]) sanitize or escape control characters (or
format as a quoted/escaped string) and replace the raw write of out["tip"] with
the sanitized value so only safe characters are printed; update the code that
sets/prints out["tip"] (and any direct uses of draftID/mailboxID in the tip) to
use the sanitizer/escaped string when calling runtime.OutFormat and when
populating the out map.
In `@shortcuts/mail/mail_reply_all.go`:
- Around line 192-204: The printed tip value (out["tip"]) is emitted raw in the
runtime.OutFormat writer inside mail_reply_all.go which can leak control
characters; before passing to OutFormat or writing with fmt.Fprintf, sanitize or
escape control characters from the tip string (e.g., remove/escape control
bytes, normalize newlines) and replace out["tip"] with the sanitized string so
both the structured output and the human-readable fmt.Fprintf calls use the safe
value; update references around the draft save block where draftID and out map
are created and used (the runtime.OutFormat callback and the fmt.Fprintf that
prints out["tip"]) to read and print the sanitized tip instead of the raw one.
In `@shortcuts/mail/mail_reply.go`:
- Around line 178-190: The tip string placed in the out map and printed to
terminal (key "tip", produced in the block around runtime.OutFormat in
mail_reply.go, using mailboxID and draftID) may contain control characters;
sanitize it before storing/printing: create or call a sanitizer (e.g.,
sanitizeString or escape control chars using a safe printer) and use the
sanitized value for out["tip"] and for the fmt.Fprintf that prints the tip;
ensure any direct fmt.Fprintf or runtime.OutFormat output uses the sanitizedTip
variable so mailboxID/draftID cannot inject terminal control sequences.
🪄 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: a86fdf85-1d87-42cf-be7f-04226d8942aa
📒 Files selected for processing (7)
shortcuts/mail/draft_preview_url_test.goshortcuts/mail/helpers.goshortcuts/mail/mail_draft_create.goshortcuts/mail/mail_draft_edit.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.go
Greptile SummaryThis PR adds a shareable draft preview URL to the draft-save output of five mail commands (draft-create, draft-edit, forward, reply, reply-all) and introduces well-tested helpers (
Confidence Score: 4/5Safe to merge after resolving the tip duplication in --format pretty mode; the preview URL feature itself is well-tested and correct. One P1 finding (tip printed twice in pretty mode) and one P2 inconsistency (mail_send.go not using OutFormat) prevent a full 5/5. The core URL generation logic is solid and well-covered by tests. shortcuts/mail/mail_forward.go, shortcuts/mail/mail_reply.go, shortcuts/mail/mail_reply_all.go (tip duplication), shortcuts/mail/mail_send.go (OutFormat consistency) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Command as Draft Command
participant API as Lark API
participant Helpers as helpers.go
participant Runtime as RuntimeContext
User->>Command: run (--confirm-send=false)
Command->>API: CreateWithRaw(mailboxID, rawEML)
API-->>Command: draftID
Command->>Helpers: addDraftPreviewURL(runtime, out, draftID)
Helpers->>Helpers: draftPreviewURL(runtime, draftID)
Helpers->>Helpers: draftPreviewOriginForBrand(brand)
Helpers-->>Command: out["preview_url"] set
Command->>Runtime: OutFormat(out, nil, prettyFn)
alt --format pretty
Runtime->>User: stdout: draft_id + preview_url + tip
else --format json (default)
Runtime->>User: stdout: JSON envelope
end
Command->>Runtime: hintSendDraft(runtime, mailboxID, draftID)
Runtime->>User: stderr: tip: draft saved. To send...
Reviews (3): Last reviewed commit: "fix(mail): derive draft preview URL orig..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/mail/mail_forward.go (1)
220-227:⚠️ Potential issue | 🟠 MajorSanitize terminal-bound values at print time too.
Line 222 still prints raw
draftID, and Line 226 writesout["tip"]without sink sanitization. Keep the formatter sink hardened as well to avoid control-sequence leakage in human-readable mode.Suggested hardening
runtime.OutFormat(out, nil, func(w io.Writer) { fmt.Fprintln(w, "Draft saved.") - fmt.Fprintf(w, "draft_id: %s\n", draftID) + fmt.Fprintf(w, "draft_id: %s\n", sanitizeForTerminal(draftID)) if previewURL, _ := out["preview_url"].(string); previewURL != "" { fmt.Fprintf(w, "preview_url: %s\n", previewURL) } - fmt.Fprintf(w, "%s\n", out["tip"]) + fmt.Fprintf(w, "%s\n", sanitizeForTerminal(fmt.Sprint(out["tip"]))) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_forward.go` around lines 220 - 227, The human-readable output block in runtime.OutFormat prints raw draftID and out["tip"] which can leak control sequences; update the closure passed to runtime.OutFormat to pass draftID and the tip value through the repository's terminal-sanitization helper (e.g., sanitizeTerminal or the existing runtime/ansi sanitizer) before writing them to w, and also ensure preview_url is likewise sanitized when printed; target the variables draftID and out["tip"] inside the anonymous func to apply the sanitizer so all terminal-bound strings are escaped/stripped before fmt.Fprintln/Fprintf.shortcuts/mail/mail_reply_all.go (1)
197-204:⚠️ Potential issue | 🟠 MajorApply sink-side terminal sanitization in formatted output.
Line 199 outputs raw
draftID, and Line 203 writesout["tip"]directly. Please sanitize at the write site as well for consistent terminal-safety.Suggested hardening
runtime.OutFormat(out, nil, func(w io.Writer) { fmt.Fprintln(w, "Draft saved.") - fmt.Fprintf(w, "draft_id: %s\n", draftID) + fmt.Fprintf(w, "draft_id: %s\n", sanitizeForTerminal(draftID)) if previewURL, _ := out["preview_url"].(string); previewURL != "" { fmt.Fprintf(w, "preview_url: %s\n", previewURL) } - fmt.Fprintf(w, "%s\n", out["tip"]) + fmt.Fprintf(w, "%s\n", sanitizeForTerminal(fmt.Sprint(out["tip"]))) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_reply_all.go` around lines 197 - 204, In the runtime.OutFormat callback, avoid writing draftID and out["tip"] (and preview_url) raw to the terminal; pass them through your terminal-sanitization helper before calling fmt.Fprintln / fmt.Fprintf. Update the fmt.Fprintf/Fprintln calls that reference draftID and out["tip"] (and the previewURL extraction) to use the sanitizer (e.g., sanitizeForTerminal(...)) so control characters and unsafe sequences are removed/escaped; if a sanitizer function does not exist, add a small helper (sanitizeForTerminal) that strips or escapes control/ANSI sequences and use it on draftID, previewURL and out["tip"] before writing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/mail/mail_forward.go`:
- Around line 220-227: The human-readable output block in runtime.OutFormat
prints raw draftID and out["tip"] which can leak control sequences; update the
closure passed to runtime.OutFormat to pass draftID and the tip value through
the repository's terminal-sanitization helper (e.g., sanitizeTerminal or the
existing runtime/ansi sanitizer) before writing them to w, and also ensure
preview_url is likewise sanitized when printed; target the variables draftID and
out["tip"] inside the anonymous func to apply the sanitizer so all
terminal-bound strings are escaped/stripped before fmt.Fprintln/Fprintf.
In `@shortcuts/mail/mail_reply_all.go`:
- Around line 197-204: In the runtime.OutFormat callback, avoid writing draftID
and out["tip"] (and preview_url) raw to the terminal; pass them through your
terminal-sanitization helper before calling fmt.Fprintln / fmt.Fprintf. Update
the fmt.Fprintf/Fprintln calls that reference draftID and out["tip"] (and the
previewURL extraction) to use the sanitizer (e.g., sanitizeForTerminal(...)) so
control characters and unsafe sequences are removed/escaped; if a sanitizer
function does not exist, add a small helper (sanitizeForTerminal) that strips or
escapes control/ANSI sequences and use it on draftID, previewURL and out["tip"]
before writing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd078882-081a-4114-9462-1cc1bddfe329
📒 Files selected for processing (6)
shortcuts/mail/draft_preview_url_test.goshortcuts/mail/helpers.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.go
✅ Files skipped from review due to trivial changes (1)
- shortcuts/mail/draft_preview_url_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/mail/mail_reply.go
- shortcuts/mail/helpers.go
- draftPreviewOriginForBrand now calls ResolveOpenBaseURL and replaces "open." with "www." instead of hardcoding feishu/lark URLs - sanitize tip strings with sanitizeForTerminal before printing (fixes terminal injection risk in mailboxID/draftID values) - add missing hintSendDraft stderr output to reply/reply-all/forward Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
qiooo
force-pushed
the
feat/mail-send-preview
branch
from
41c88ff to
16312db
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/mail/mail_reply.go (1)
185-185:⚠️ Potential issue | 🟠 MajorSanitize
draft_idbefore writing to terminal output.
draftIDis still printed raw at Line 185, so control characters could reach terminal output despite tip hardening.🔧 Suggested fix
- fmt.Fprintf(w, "draft_id: %s\n", draftID) + fmt.Fprintf(w, "draft_id: %s\n", sanitizeForTerminal(draftID))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_reply.go` at line 185, The code writes draftID raw to terminal via fmt.Fprintf(w, "draft_id: %s\n", draftID); sanitize or escape control characters before writing to avoid terminal control injection. Replace the direct write with a sanitized value (e.g., call a new helper sanitizeDraftID(string) that strips or escapes non-printable/control runes, or use strconv.Quote to safely escape) and use that sanitized value in the fmt.Fprintf call; update or add sanitizeDraftID near the existing handler that references draftID in mail_reply.go so future uses are safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/mail/mail_reply.go`:
- Line 185: The code writes draftID raw to terminal via fmt.Fprintf(w,
"draft_id: %s\n", draftID); sanitize or escape control characters before writing
to avoid terminal control injection. Replace the direct write with a sanitized
value (e.g., call a new helper sanitizeDraftID(string) that strips or escapes
non-printable/control runes, or use strconv.Quote to safely escape) and use that
sanitized value in the fmt.Fprintf call; update or add sanitizeDraftID near the
existing handler that references draftID in mail_reply.go so future uses are
safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 122a09fd-2214-4f08-9833-9b93af406dc4
📒 Files selected for processing (6)
shortcuts/mail/draft_preview_url_test.goshortcuts/mail/helpers.goshortcuts/mail/mail_forward.goshortcuts/mail/mail_reply.goshortcuts/mail/mail_reply_all.goshortcuts/mail/mail_send.go
✅ Files skipped from review due to trivial changes (2)
- shortcuts/mail/mail_reply_all.go
- shortcuts/mail/draft_preview_url_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/mail/mail_send.go
- shortcuts/mail/mail_forward.go
- shortcuts/mail/helpers.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Test Plan
lark xxxcommand works as expectedRelated Issues
Summary by CodeRabbit
New Features
Tests