fix(mail): add missing scopes for mail +watch#357
Conversation
The mail +watch shortcut requires scope mail:user_mailbox.event.mail_address:read to receive the mail_address field in WebSocket event payloads, but this scope was neither declared in the shortcut's Scopes list nor included in the auto-approve (recommend.allow) set. Without this scope, +watch events arrive without the mail_address field, which breaks mailbox filtering and fetch-mailbox resolution. - Add scope to mail +watch Scopes declaration - Add scope to scope_overrides.json recommend.allow list so that auth login --recommend requests it automatically
📝 WalkthroughWalkthroughAdded the event-based scope Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds two missing scopes to Confidence Score: 5/5Safe to merge — the core scope fix is correct and consistent with other mail shortcuts. All findings are P2. The Scopes additions are functionally correct, runtime-enforced, and consistent with mail_send/reply/forward. The only concern is a stale PR description; the code itself is sound. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as lark-cli
participant AuthStore as Auth Store
participant WS as WebSocket API
participant MailAPI as Mail API
User->>CLI: mail +watch --mailbox user@example.com
CLI->>AuthStore: checkScopePrereqs([mail:event, mail:user_mailbox.event.mail_address:read, mail:user_mailbox:readonly, ...])
AuthStore-->>CLI: scopes present ✓
CLI->>MailAPI: POST /mailboxes/me/event/subscribe
CLI->>MailAPI: GET /mailboxes/me/profile (mail:user_mailbox:readonly)
MailAPI-->>CLI: primary_email_address = user@example.com
CLI->>WS: Connect WebSocket
WS-->>CLI: event { mail_address, message_id } (mail:user_mailbox.event.mail_address:read)
CLI->>CLI: filter mailAddr == mailboxFilter ✓
CLI->>MailAPI: GET /mailboxes/user@example.com/messages/{id}
MailAPI-->>CLI: message payload
CLI-->>User: output message
Reviews (3): Last reviewed commit: "fix(mail): remove event scope from scope..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shortcuts/mail/mail_watch.go (1)
182-210:⚠️ Potential issue | 🟠 MajorUse
vfs.*instead ofos.*for filesystem operations.Lines 182-183 use
os.UserHomeDir()and line 203 usesos.MkdirAll(). As per coding guidelines, all filesystem access should usevfs.*instead ofos.*to support virtual filesystem abstractions for testing and sandboxing.📁 Proposed fix to use vfs package
Check if the
vfspackage is imported and available, then replace:- home, err := os.UserHomeDir() + home, err := vfs.UserHomeDir() if err != nil { return fmt.Errorf("cannot expand ~: %w", err)- if err := os.MkdirAll(outputDir, 0700); err != nil { + if err := vfs.MkdirAll(outputDir, 0700); err != nil { return fmt.Errorf("cannot create output directory %q: %w", outputDir, err)As per coding guidelines: Use
vfs.*instead ofos.*for all filesystem access.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/mail/mail_watch.go` around lines 182 - 210, Replace direct os package filesystem calls with the vfs equivalents: call vfs.UserHomeDir() instead of os.UserHomeDir(), vfs.MkdirAll(outputDir, 0700) instead of os.MkdirAll(...), and use vfs.EvalSymlinks(outputDir) instead of filepath.EvalSymlinks so the code honors the virtual filesystem abstraction; keep the same error handling and assignment to outputDir, and ensure the vfs package is imported and used around the existing logic that also calls validate.SafeOutputPath and manipulates the outputDir variable.
🤖 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_watch.go`:
- Line 84: The Scopes array in mail_watch.go is missing the required
"mail:user_mailbox:readonly" permission which causes
fetchMailboxPrimaryEmail(runtime, "me") to fail when mailbox defaults to "me";
update the Scopes slice (the Scopes: []string{...} declaration) to include
"mail:user_mailbox:readonly" alongside the existing mail scopes so the default
mailbox resolution and event filtering have the necessary readonly mailbox
access.
---
Outside diff comments:
In `@shortcuts/mail/mail_watch.go`:
- Around line 182-210: Replace direct os package filesystem calls with the vfs
equivalents: call vfs.UserHomeDir() instead of os.UserHomeDir(),
vfs.MkdirAll(outputDir, 0700) instead of os.MkdirAll(...), and use
vfs.EvalSymlinks(outputDir) instead of filepath.EvalSymlinks so the code honors
the virtual filesystem abstraction; keep the same error handling and assignment
to outputDir, and ensure the vfs package is imported and used around the
existing logic that also calls validate.SafeOutputPath and manipulates the
outputDir variable.
🪄 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: 850c59a6-6d50-44a6-a11e-b639e9207771
📒 Files selected for processing (2)
internal/registry/scope_overrides.jsonshortcuts/mail/mail_watch.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@e2220947568c4542d1c94dc5f51365d543f5b2bd🧩 Skill updatenpx skills add larksuite/cli#fix/mail-watch-event-scope -y -g |
The +watch shortcut calls fetchMailboxPrimaryEmail (GET user_mailboxes/me/profile) to resolve the mailbox address for event filtering, which requires scope mail:user_mailbox:readonly. All other mail shortcuts that call this API (send, reply, forward, draft-create, draft-edit) already declare this scope, but +watch did not.
The mail:user_mailbox.event.mail_address:read scope only needs to be declared in the +watch shortcut's Scopes list, not in the global recommend.allow set.
* fix(mail): add missing event scope for mail watch The mail +watch shortcut requires scope mail:user_mailbox.event.mail_address:read to receive the mail_address field in WebSocket event payloads, but this scope was neither declared in the shortcut's Scopes list nor included in the auto-approve (recommend.allow) set. Without this scope, +watch events arrive without the mail_address field, which breaks mailbox filtering and fetch-mailbox resolution. - Add scope to mail +watch Scopes declaration - Add scope to scope_overrides.json recommend.allow list so that auth login --recommend requests it automatically * fix(mail): add missing mailbox profile scope for mail watch The +watch shortcut calls fetchMailboxPrimaryEmail (GET user_mailboxes/me/profile) to resolve the mailbox address for event filtering, which requires scope mail:user_mailbox:readonly. All other mail shortcuts that call this API (send, reply, forward, draft-create, draft-edit) already declare this scope, but +watch did not. * fix(mail): remove event scope from scope_overrides.json The mail:user_mailbox.event.mail_address:read scope only needs to be declared in the +watch shortcut's Scopes list, not in the global recommend.allow set.
Summary
mail:user_mailbox.event.mail_address:readandmail:user_mailbox:readonlyto mail+watchshortcut's declared ScopesProblem
The
mail +watchshortcut was missing two scopes:mail:user_mailbox.event.mail_address:read— Without it, WebSocket event payloads arrive without themail_addressfield, which breaks:--mailboxfiltering (cannot match events to the subscribed mailbox)fetchMailboxresolution (falls back to the--mailboxflag value instead of the event's actual mailbox address)mail:user_mailbox:readonly— Required byfetchMailboxPrimaryEmail(GETuser_mailboxes/me/profile) to resolve the mailbox address for event filtering. All other mail shortcuts that call this API (send, reply, forward, draft-create, draft-edit) already declare this scope, but+watchdid not.Test plan
go build ./...passesgo test ./shortcuts/mail/... ./internal/registry/...passesmail +watchevents containmail_addressfield after re-login with updated scopes