Skip to content

feat: add mail user allow-block commands#1966

Open
chumbleyneria wants to merge 1 commit into
larksuite:mainfrom
chumbleyneria:feat/1b726b1
Open

feat: add mail user allow-block commands#1966
chumbleyneria wants to merge 1 commit into
larksuite:mainfrom
chumbleyneria:feat/1b726b1

Conversation

@chumbleyneria

@chumbleyneria chumbleyneria commented Jul 20, 2026

Copy link
Copy Markdown

Summary

  • add lark mail user allow-block list/search/add/delete/get commands
  • wire commands to the existing mail user allow/block sender OpenAPI surface
  • align with Meta BOE revision 10445 and larksuite-cli-registry MR

Related

Validation

  • go test ./shortcuts/mail
  • go test ./shortcuts
  • verify-branch-base.sh

Summary by CodeRabbit

  • New Features

    • Added mail user-allow-block commands to list, search, view, add, and remove sender allow/block entries.
    • Supports pagination, filtering, dry runs, batch operations, and combined allow/block results.
    • Added validation, permission checks, rate-limit guidance, and safer error messages.
  • Documentation

    • Expanded mail workflow, authorization, sending, HTML, linting, and shortcut guidance.
    • Added dedicated documentation with examples and operational constraints for user allow/block management.
  • Tests

    • Added coverage for allow/block operations, queries, validation, output, and rate-limit handling.

@github-actions github-actions Bot added domain/mail PR touches the mail domain size/XL Architecture-level or global-impact change labels Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds the mail user-allow-block command group for listing, searching, checking, adding, and deleting sender allow/block records. It wires factory-aware installation, API validation and error handling, comprehensive tests, and updated mail skill documentation.

Changes

Mail user allow/block

Layer / File(s) Summary
Command registration and CLI contracts
shortcuts/mail/flag_suggest.go, shortcuts/mail/mail_user_allow_block.go, shortcuts/register.go
Registers the new command group and subcommands with shared flags, metadata, validation modes, and factory-aware mail installation.
Allow/block read and write operations
shortcuts/mail/mail_user_allow_block.go
Implements list, search, get, add, delete, combined results, scope checks, record normalization, batching, and pagination.
Request execution and response handling
shortcuts/mail/mail_user_allow_block.go
Adds API request helpers, dry-run rendering, formatted output, jq handling, response checks, and targeted error hints.
Command behavior tests
shortcuts/mail/mail_user_allow_block_test.go
Covers registration, API payloads, pagination, combined searches, lookups, validation errors, and rate-limit guidance.
Mail skill and feature documentation
skills/lark-mail/SKILL.md, skills/lark-mail/references/lark-mail-user-allow-block.md
Updates mail workflows, safety rules, API permissions, shortcut references, and allow/block usage documentation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant UserAllowBlock
  participant MailAPI
  participant Formatter
  CLI->>UserAllowBlock: execute list/search/get/add/delete
  UserAllowBlock->>MailAPI: validate and send request
  MailAPI-->>UserAllowBlock: return records or operation result
  UserAllowBlock->>Formatter: emit formatted response
  Formatter-->>CLI: write output or decorated error
Loading

Possibly related PRs

Suggested labels: feature

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding mail user allow-block commands.
Description check ✅ Passed The description covers the main summary and validation, but it does not follow the template sections exactly and omits a dedicated Related Issues section.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@shortcuts/mail/mail_user_allow_block_test.go`:
- Around line 249-252: Update the error assertion in the relevant test to use
errors.As with the concrete *errs.ValidationError type instead of the generic
error interface. Keep the existing failure handling, and ensure the test
verifies that the command returns a validation error specifically.

In `@shortcuts/mail/mail_user_allow_block.go`:
- Around line 238-250: Fix keyword validation across the search command’s RunE
handler and runUserAllowBlockRead: validate the raw query before it is trimmed,
rejecting empty or whitespace-only input with the existing “keyword must not be
empty” error. Remove the unreachable opts.Query != "" &&
strings.TrimSpace(opts.Query) == "" check, while preserving trimmed query
handling for valid searches.
- Around line 449-457: Preserve the dry-run descriptor returned by
userAllowBlockOptions.call instead of passing it through normal response
processing. Update runUserAllowBlockGet and runUserAllowBlockCombinedSearch to
detect the dry-run result and return it directly, retaining its dry_run marker
and api plan; keep existing findUserAllowBlockRecord and
extractUserAllowBlockItems behavior for real API responses.
- Around line 356-387: Update userAllowBlockOptions.prepare to apply the --json
override before validating output options, then validate the effective
opts.Format against the supported formats and return the established typed
validation error for unsupported values. Ensure output.ValidateJqFlags receives
the effective format so --json with --jq is checked as JSON, and remove reliance
on dispatch/emit warning-only handling.
- Around line 332-354: Update runUserAllowBlockCombinedSearch and the associated
userAllowBlockListParams flow so combined --type all results encode and accept
independent allow and block page tokens. Ensure follow-up requests reuse
allow_page_token for allow_senders and block_page_token for blocked_senders,
while preserving the existing shared cursor behavior for non-combined searches.
- Around line 645-697: Introduce a typed allow/block response item with a single
sender field and update findUserAllowBlockRecord to iterate and return that
typed item, matching only its sender value. Adjust extractUserAllowBlockItems
and related merge/type helpers to use the typed representation while preserving
the allow/block type projection.

In `@skills/lark-mail/SKILL.md`:
- Around line 670-673: Update the scope entries for
user_mailbox.allow_senders.list and user_mailbox.blocked_senders.list to use
mail:user_mailbox.message:readonly instead of the modify scope, leaving the
batch_create and batch_remove entries unchanged.
- Line 531: Update the user-allow-block table description so the type
alternatives do not create Markdown table delimiters; escape the pipes in
“allow|block|all” or replace them with slash separators while preserving the
documented options.
🪄 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: e4a7853e-6c89-4268-a364-01e64d31e4a7

📥 Commits

Reviewing files that changed from the base of the PR and between 79adf89 and 7e01dc2.

📒 Files selected for processing (6)
  • shortcuts/mail/flag_suggest.go
  • shortcuts/mail/mail_user_allow_block.go
  • shortcuts/mail/mail_user_allow_block_test.go
  • shortcuts/register.go
  • skills/lark-mail/SKILL.md
  • skills/lark-mail/references/lark-mail-user-allow-block.md

Comment on lines +249 to +252
var problem interface{ Error() string }
if !errors.As(err, &problem) {
t.Fatalf("expected error, got %T", err)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the specific error type.

Checking errors.As against interface{ Error() string } is a tautology, as all errors inherently implement this interface. Consequently, this check passes for any non-nil error, failing to verify that a validation error was actually returned.

Assert the concrete *errs.ValidationError type instead to ensure the command correctly categorized the failure.

🛠️ Proposed fix
-			var problem interface{ Error() string }
-			if !errors.As(err, &problem) {
-				t.Fatalf("expected error, got %T", err)
-			}
+			var ve *errs.ValidationError
+			if !errors.As(err, &ve) {
+				t.Fatalf("expected validation error, got %T", err)
+			}
📝 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.

Suggested change
var problem interface{ Error() string }
if !errors.As(err, &problem) {
t.Fatalf("expected error, got %T", err)
}
var ve *errs.ValidationError
if !errors.As(err, &ve) {
t.Fatalf("expected validation error, got %T", err)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/mail/mail_user_allow_block_test.go` around lines 249 - 252, Update
the error assertion in the relevant test to use errors.As with the concrete
*errs.ValidationError type instead of the generic error interface. Keep the
existing failure handling, and ensure the test verifies that the command returns
a validation error specifically.

Comment on lines +238 to +250
func runUserAllowBlockRead(opts *userAllowBlockOptions) error {
if err := opts.prepare(userAllowBlockScopeRead); err != nil {
return err
}
if opts.Query != "" && strings.TrimSpace(opts.Query) == "" {
return mailValidationParamError("keyword", "keyword must not be empty")
}
if err := validateUserAllowBlockType(opts.Kind, true); err != nil {
return err
}
if opts.PageSize <= 0 || opts.PageSize > userAllowBlockMaxPageSize {
return mailValidationParamError("--page-size", "--page-size must be between 1 and 100")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Dead keyword-empty check: condition can never be true.

Line 242's opts.Query != "" && strings.TrimSpace(opts.Query) == "" can never evaluate true, because opts.Query is already trimmed by the search command's RunE (line 115) before runUserAllowBlockRead runs — once trimmed, TrimSpace(opts.Query) == opts.Query, making the condition self-contradictory. As a result, search "" or search " " silently falls through to an unfiltered list instead of the intended "keyword must not be empty" validation error.

🛡️ Proposed fix
 		Args: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
 				return mailValidationParamError("keyword", "search requires exactly one keyword")
 			}
+			if strings.TrimSpace(args[0]) == "" {
+				return mailValidationParamError("keyword", "keyword must not be empty")
+			}
 			return nil
 		},
-	if opts.Query != "" && strings.TrimSpace(opts.Query) == "" {
-		return mailValidationParamError("keyword", "keyword must not be empty")
-	}

Also applies to: 106-118

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/mail/mail_user_allow_block.go` around lines 238 - 250, Fix keyword
validation across the search command’s RunE handler and runUserAllowBlockRead:
validate the raw query before it is trimmed, rejecting empty or whitespace-only
input with the existing “keyword must not be empty” error. Remove the
unreachable opts.Query != "" && strings.TrimSpace(opts.Query) == "" check, while
preserving trimmed query handling for valid searches.

Comment on lines +332 to +354
func runUserAllowBlockCombinedSearch(opts *userAllowBlockOptions) error {
allowData, err := opts.call("allow_senders", "list", "GET", nil, userAllowBlockListParams(opts))
if err != nil {
return mailDecorateAllowBlockProblem(err)
}
blockData, err := opts.call("blocked_senders", "list", "GET", nil, userAllowBlockListParams(opts))
if err != nil {
return mailDecorateAllowBlockProblem(err)
}
out := map[string]interface{}{
"mailbox_id": opts.mailboxID(),
"type": "all",
"keyword": opts.Query,
"items": mergeUserAllowBlockItems(allowData, blockData),
"allow_senders": extractUserAllowBlockItems(allowData),
"blocked_senders": extractUserAllowBlockItems(blockData),
"allow_page_token": strVal(allowData["page_token"]),
"block_page_token": strVal(blockData["page_token"]),
"allow_has_more": userAllowBlockBoolVal(allowData["has_more"]),
"block_has_more": userAllowBlockBoolVal(blockData["has_more"]),
}
return opts.emit(out)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the relevant file and search for related pagination helpers/usages.
git ls-files 'shortcuts/mail/*.go' | sed -n '1,20p'
printf '\n--- outline ---\n'
ast-grep outline shortcuts/mail/mail_user_allow_block.go --view expanded || true
printf '\n--- search cursor/pagination helpers ---\n'
rg -n 'userAllowBlockListParams|allow_page_token|block_page_token|--cursor|cursor|type": *"all"|runUserAllowBlockCombinedSearch|runUserAllowBlock' shortcuts/mail/mail_user_allow_block.go
printf '\n--- surrounding lines ---\n'
sed -n '280,380p' shortcuts/mail/mail_user_allow_block.go
printf '\n--- other combined search refs ---\n'
rg -n 'allow_page_token|block_page_token|type": *"all"|--cursor' shortcuts/mail

Repository: larksuite/cli

Length of output: 9947


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '200,290p' shortcuts/mail/mail_user_allow_block.go
printf '\n--- params helper ---\n'
sed -n '548,570p' shortcuts/mail/mail_user_allow_block.go
printf '\n--- tests around combined search/list pagination ---\n'
sed -n '1,260p' shortcuts/mail/mail_user_allow_block_test.go

Repository: larksuite/cli

Length of output: 12734


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf -- '--- search for page token fields/usages ---\n'
rg -n 'allow_page_token|block_page_token|page_token|--cursor|cursor' shortcuts/mail docs README.md .github 2>/dev/null || true

printf -- '\n--- search for user-allow-block docs/help references ---\n'
rg -n 'user-allow-block|allow_senders|blocked_senders|combined|--type all|search' shortcuts/mail docs README.md 2>/dev/null || true

printf -- '\n--- tail of tests for pagination-related cases ---\n'
sed -n '260,420p' shortcuts/mail/mail_user_allow_block_test.go

Repository: larksuite/cli

Length of output: 26071


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,140p' shortcuts/mail/mail_user_allow_block.go
printf '\n--- search command path ---\n'
sed -n '90,130p' shortcuts/mail/mail_user_allow_block.go
printf '\n--- any combined pagination tests or helpers for multiple page tokens ---\n'
rg -n 'page_token.*search|allow_page_token|block_page_token|cursor.*all|type all|search all' shortcuts/mail/mail_user_allow_block_test.go shortcuts/mail/mail_user_allow_block.go

Repository: larksuite/cli

Length of output: 6076


Combined --type all pagination is not round-trippable

runUserAllowBlockCombinedSearch returns separate allow_page_token and block_page_token, but userAllowBlockListParams only accepts one shared --cursor. That makes follow-up paging ambiguous once the two lists diverge; the combined view can’t continue both sides independently.
shortcuts/mail/mail_user_allow_block.go:332-354

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/mail/mail_user_allow_block.go` around lines 332 - 354, Update
runUserAllowBlockCombinedSearch and the associated userAllowBlockListParams flow
so combined --type all results encode and accept independent allow and block
page tokens. Ensure follow-up requests reuse allow_page_token for allow_senders
and block_page_token for blocked_senders, while preserving the existing shared
cursor behavior for non-combined searches.

Comment on lines +356 to +387
func (opts *userAllowBlockOptions) prepare(scopes ...string) error {
if opts.Factory == nil {
return mailInvalidResponseError("mail user-allow-block command is missing factory")
}
cfg, err := opts.Factory.Config()
if err != nil {
return err
}
opts.Config = cfg
asFlag, _ := opts.Command.Flags().GetString("as")
opts.As = core.Identity(asFlag)
opts.As = opts.Factory.ResolveAs(opts.Ctx, opts.Command, opts.As)
if err := opts.Factory.CheckStrictMode(opts.Ctx, opts.As); err != nil {
return err
}
if err := opts.Factory.CheckIdentity(opts.As, []string{"user"}); err != nil {
return err
}
if opts.As.IsBot() {
return mailValidationParamError("--as", "mail user-allow-block only supports --as user")
}
if err := opts.checkScopes(scopes); err != nil {
return err
}
if err := output.ValidateJqFlags(opts.JqExpr, "", opts.Format); err != nil {
return err
}
if opts.Command.Flags().Changed("json") {
opts.Format = "json"
}
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

--format is never validated; invalid values silently degrade instead of erroring.

prepare() validates jq/format compatibility via output.ValidateJqFlags but never validates that opts.Format itself is one of the supported values. The actual validation only happens deep in dispatch()/emit() (lines 502-505, 532-535), where an unrecognized --format just prints a warning to stderr and silently continues instead of failing. Separately, the --json shorthand override (383-385) is applied after the jq/format compatibility check, so --format table --json --jq ... is validated against "table" instead of the effective "json".

As per coding guidelines: **/*.go: "Transcribe echoed input faithfully and never silently fall back, coerce unsupported values, ignore unavailable options... Return a typed validation error when a requested option cannot be honored."

🛡️ Proposed fix
 func (opts *userAllowBlockOptions) prepare(scopes ...string) error {
 	...
-	if err := output.ValidateJqFlags(opts.JqExpr, "", opts.Format); err != nil {
-		return err
-	}
 	if opts.Command.Flags().Changed("json") {
 		opts.Format = "json"
 	}
+	if _, ok := output.ParseFormat(opts.Format); !ok {
+		return mailValidationParamError("--format", "--format must be one of: json, ndjson, table, csv")
+	}
+	if err := output.ValidateJqFlags(opts.JqExpr, "", opts.Format); err != nil {
+		return err
+	}
 	return nil
 }
📝 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.

Suggested change
func (opts *userAllowBlockOptions) prepare(scopes ...string) error {
if opts.Factory == nil {
return mailInvalidResponseError("mail user-allow-block command is missing factory")
}
cfg, err := opts.Factory.Config()
if err != nil {
return err
}
opts.Config = cfg
asFlag, _ := opts.Command.Flags().GetString("as")
opts.As = core.Identity(asFlag)
opts.As = opts.Factory.ResolveAs(opts.Ctx, opts.Command, opts.As)
if err := opts.Factory.CheckStrictMode(opts.Ctx, opts.As); err != nil {
return err
}
if err := opts.Factory.CheckIdentity(opts.As, []string{"user"}); err != nil {
return err
}
if opts.As.IsBot() {
return mailValidationParamError("--as", "mail user-allow-block only supports --as user")
}
if err := opts.checkScopes(scopes); err != nil {
return err
}
if err := output.ValidateJqFlags(opts.JqExpr, "", opts.Format); err != nil {
return err
}
if opts.Command.Flags().Changed("json") {
opts.Format = "json"
}
return nil
}
func (opts *userAllowBlockOptions) prepare(scopes ...string) error {
if opts.Factory == nil {
return mailInvalidResponseError("mail user-allow-block command is missing factory")
}
cfg, err := opts.Factory.Config()
if err != nil {
return err
}
opts.Config = cfg
asFlag, _ := opts.Command.Flags().GetString("as")
opts.As = core.Identity(asFlag)
opts.As = opts.Factory.ResolveAs(opts.Ctx, opts.Command, opts.As)
if err := opts.Factory.CheckStrictMode(opts.Ctx, opts.As); err != nil {
return err
}
if err := opts.Factory.CheckIdentity(opts.As, []string{"user"}); err != nil {
return err
}
if opts.As.IsBot() {
return mailValidationParamError("--as", "mail user-allow-block only supports --as user")
}
if err := opts.checkScopes(scopes); err != nil {
return err
}
if opts.Command.Flags().Changed("json") {
opts.Format = "json"
}
if _, ok := output.ParseFormat(opts.Format); !ok {
return mailValidationParamError("--format", "--format must be one of: json, ndjson, table, csv")
}
if err := output.ValidateJqFlags(opts.JqExpr, "", opts.Format); err != nil {
return err
}
return nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/mail/mail_user_allow_block.go` around lines 356 - 387, Update
userAllowBlockOptions.prepare to apply the --json override before validating
output options, then validate the effective opts.Format against the supported
formats and return the established typed validation error for unsupported
values. Ensure output.ValidateJqFlags receives the effective format so --json
with --jq is checked as JSON, and remove reliance on dispatch/emit warning-only
handling.

Source: Coding guidelines

Comment on lines +449 to +457
func (opts *userAllowBlockOptions) call(resource, method, httpMethod string, body interface{}, params map[string]interface{}) (map[string]interface{}, error) {
if opts.DryRun {
return opts.dryRun(resource, method, httpMethod, body, params), nil
}
ac, err := opts.Factory.NewAPIClientWithConfig(opts.config())
if err != nil {
return nil, err
}
resp, err := ac.DoAPI(opts.Ctx, client.RawApiRequest{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

--dry-run silently produces misleading output for get and list --type all.

call()'s dry-run short-circuit (450-451) returns the same descriptor shape ({"dry_run": true, "api": [...] }) regardless of caller, but only dispatch() emits that payload directly. runUserAllowBlockGet and runUserAllowBlockCombinedSearch instead post-process the call() result through findUserAllowBlockRecord/extractUserAllowBlockItems, which find nothing in the dry-run shape (no items key) and silently discard the "api" plan. The result: get <x> --dry-run emits {"record": ..., "found": false} with no indication a dry run occurred, and list --type all --dry-run emits an empty-looking real-shaped result (no dry_run marker at all), indistinguishable from a legitimately empty response.

Also applies to: 258-291, 332-354

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/mail/mail_user_allow_block.go` around lines 449 - 457, Preserve the
dry-run descriptor returned by userAllowBlockOptions.call instead of passing it
through normal response processing. Update runUserAllowBlockGet and
runUserAllowBlockCombinedSearch to detect the dry-run result and return it
directly, retaining its dry_run marker and api plan; keep existing
findUserAllowBlockRecord and extractUserAllowBlockItems behavior for real API
responses.

Comment on lines +645 to +697
func userAllowBlockSenderType(record string) int {
if strings.Contains(record, "@") {
return 1
}
return 2
}

func extractUserAllowBlockItems(data map[string]interface{}) []interface{} {
if items, ok := data["items"].([]interface{}); ok {
return items
}
return []interface{}{}
}

func mergeUserAllowBlockItems(allowData, blockData map[string]interface{}) []interface{} {
out := make([]interface{}, 0, len(extractUserAllowBlockItems(allowData))+len(extractUserAllowBlockItems(blockData)))
for _, item := range extractUserAllowBlockItems(allowData) {
out = append(out, userAllowBlockItemWithType(item, "allow"))
}
for _, item := range extractUserAllowBlockItems(blockData) {
out = append(out, userAllowBlockItemWithType(item, "block"))
}
return out
}

func userAllowBlockItemWithType(item interface{}, kind string) interface{} {
m, ok := item.(map[string]interface{})
if !ok {
return item
}
clone := make(map[string]interface{}, len(m)+1)
for k, v := range m {
clone[k] = v
}
clone["type"] = kind
return clone
}

func findUserAllowBlockRecord(data map[string]interface{}, record string) map[string]interface{} {
want := strings.ToLower(record)
for _, item := range extractUserAllowBlockItems(data) {
m, ok := item.(map[string]interface{})
if !ok {
continue
}
for _, key := range []string{"sender", "record", "email", "domain"} {
if strings.EqualFold(strVal(m[key]), want) {
return m
}
}
}
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🌐 Web query:

Lark Open Platform mail.user_mailbox.allow_senders list API response field name for sender

💡 Result:

The Lark Open Platform mail API does not provide a public API endpoint specifically named mail.user_mailbox.allow_senders.list. While developer documentation includes various mail-related operations such as sending messages [1], listing emails [2], creating auto filters [3][4], and managing mail group permissions [5][6], there is no official API definition in the Lark Open Platform documentation that uses the path or field name allow_senders for a user mailbox. Some third-party tools or repositories may reference concepts like allow-listed senders [7] or list members who can send emails to a mail group [8], but these do not map to a standard mail.user_mailbox.allow_senders.list API in the official Lark Developer documentation. If you are attempting to manage who can send emails, note that mail groups use permission members [6], and auto-filter rules use a type parameter (where 1 represents Sender) [3][4], but these are distinct from a general allow-sender list API.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file and inspect the relevant region with line numbers.
wc -l shortcuts/mail/mail_user_allow_block.go
sed -n '560,760p' shortcuts/mail/mail_user_allow_block.go | cat -n

# Find call sites and related helpers.
rg -n "userAllowBlockSenderType|extractUserAllowBlockItems|mergeUserAllowBlockItems|findUserAllowBlockRecord|userAllowBlockItemWithType|call\(" shortcuts/mail/mail_user_allow_block.go shortcuts/mail -S

# Look for typed structs or other projections used in this area.
rg -n "type .*struct|struct \{" shortcuts/mail -S

Repository: larksuite/cli

Length of output: 23654


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the allow/block tests and any fixtures around the list/search behavior.
sed -n '1,280p' shortcuts/mail/mail_user_allow_block_test.go | cat -n
printf '\n---\n'
sed -n '280,520p' shortcuts/mail/mail_user_allow_block_test.go | cat -n

# Search for sender_type / allow_senders / blocked_senders usage across the repo.
rg -n '"sender_type"|sender_type|allow_senders|blocked_senders|allow-block|allow block|allow/block' shortcuts/mail -S

# Inspect the call helper and its decoding behavior.
sed -n '420,520p' shortcuts/mail/mail_user_allow_block.go | cat -n

Repository: larksuite/cli

Length of output: 21472


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for response-shape hints in the mail shortcut area and any generated API payloads.
rg -n '"record"|"\bemail\b"|"\bdomain\b"|"\bsender\b"|"\bsender_type\b"|sender_type' shortcuts/mail -S

# Inspect the search/list implementation around its output construction.
sed -n '240,380p' shortcuts/mail/mail_user_allow_block.go | cat -n

# Inspect any helpers that stringify response values used by findUserAllowBlockRecord.
rg -n "func strVal|strVal\(" shortcuts/mail -S

Repository: larksuite/cli

Length of output: 15503


Project the allow/block list response into a typed item. findUserAllowBlockRecord still reads map[string]interface{} and guesses across sender/record/email/domain; make the response shape explicit and match on one sender field instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/mail/mail_user_allow_block.go` around lines 645 - 697, Introduce a
typed allow/block response item with a single sender field and update
findUserAllowBlockRecord to iterate and return that typed item, matching only
its sender value. Adjust extractUserAllowBlockItems and related merge/type
helpers to use the typed representation while preserving the allow/block type
projection.

Source: Coding guidelines

Comment thread skills/lark-mail/SKILL.md
| [`+template-update`](references/lark-mail-template-update.md) | Update an existing mail template. Supports --inspect (read-only projection), --print-patch-template (prints a JSON skeleton for --patch-file), and flat flags (--set-subject / --set-name / etc). Internally it GETs the template, applies the patch, rewrites <img> local paths to cid: refs, and PUTs a full-replace update (no optimistic locking: last-write-wins). |
| [`+lint-html`](references/lark-mail-lint-html.md) | Lint mail HTML body for compatibility / safety / Feishu-native rules. Returns warnings/errors and (default) auto-fixed HTML. Read-only: no draft, no API call. Use this BEFORE creating a draft to preview what the writing-path lint would change, or as a CI gate for static HTML templates. |
| [`+lint-html`](references/lark-mail-lint-html.md) | Lint mail HTML body for compatibility / safety / Larksuite-native rules. Returns warnings/errors and (always) auto-fixed cleaned_html. Read-only: no draft, no API call. Use this BEFORE creating a draft to preview what the writing-path lint would change. |
| [`user-allow-block`](references/lark-mail-user-allow-block.md) | Manage the current user's trusted and blocked senders. Supports list/search/get/add/delete with `--type allow|block|all`, `--allow`, `--block`, pagination, JSON, and table output. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Escape the pipes in the Markdown table cell.

allow|block|all is parsed as additional table delimiters, causing the MD056 column-count error and malformed rendering. Use allow / block / all or escape the pipes.

🧰 Tools
🪛 markdownlint-cli2 (0.23.0)

[warning] 531-531: Spaces inside code span elements

(MD038, no-space-in-code)


[warning] 531-531: Spaces inside code span elements

(MD038, no-space-in-code)


[warning] 531-531: Table column count
Expected: 2; Actual: 4; Too many cells, extra data will be missing

(MD056, table-column-count)

🪛 SkillSpector (2.3.11)

[error] 32: [P1] Instruction Override: This pattern attempts to override system instructions or ignore safety constraints. Without LLM analysis, manual review is recommended.

Remediation: Remove or rewrite any text that instructs the agent to ignore prompts, override safety rules, or trust unverified content. Ensure skill content cannot be injected to alter agent behavior.

(Prompt Injection (P1))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/lark-mail/SKILL.md` at line 531, Update the user-allow-block table
description so the type alternatives do not create Markdown table delimiters;
escape the pipes in “allow|block|all” or replace them with slash separators
while preserving the documented options.

Source: Linters/SAST tools

Comment thread skills/lark-mail/SKILL.md
Comment on lines +670 to +673
| `user_mailbox.allow_senders.list` | `mail:user_mailbox.message:modify` |
| `user_mailbox.blocked_senders.batch_create` | `mail:user_mailbox.message:modify` |
| `user_mailbox.blocked_senders.batch_remove` | `mail:user_mailbox.message:modify` |
| `user_mailbox.blocked_senders.list` | `mail:user_mailbox.message:modify` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Correct the read scopes for the raw allow/block list methods.

The implementation uses mail:user_mailbox.message:readonly for list/search/get, but these rows document allow_senders.list and blocked_senders.list as requiring ...:modify. This overstates privileges and can mislead users into requesting write access for read-only operations. Change both list entries to mail:user_mailbox.message:readonly.

🧰 Tools
🪛 SkillSpector (2.3.11)

[error] 32: [P1] Instruction Override: This pattern attempts to override system instructions or ignore safety constraints. Without LLM analysis, manual review is recommended.

Remediation: Remove or rewrite any text that instructs the agent to ignore prompts, override safety rules, or trust unverified content. Ensure skill content cannot be injected to alter agent behavior.

(Prompt Injection (P1))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/lark-mail/SKILL.md` around lines 670 - 673, Update the scope entries
for user_mailbox.allow_senders.list and user_mailbox.blocked_senders.list to use
mail:user_mailbox.message:readonly instead of the modify scope, leaving the
batch_create and batch_remove entries unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/mail PR touches the mail domain size/XL Architecture-level or global-impact change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant