Skip to content

feat(drive): add secure label shortcuts#985

Open
caojie0621 wants to merge 1 commit into
mainfrom
feat/srcret
Open

feat(drive): add secure label shortcuts#985
caojie0621 wants to merge 1 commit into
mainfrom
feat/srcret

Conversation

@caojie0621
Copy link
Copy Markdown
Collaborator

@caojie0621 caojie0621 commented May 20, 2026

Summary

  • Add drive +secure-label-list to list secure labels available to the current user.
  • Add drive +secure-label-update to update a Drive file/document secure label.
  • Support URL/token type inference for secure-label update, matching existing Drive permission shortcut behavior.
  • Add 1063013 handling with an actionable hint directing users to complete downgrade approval in the document UI.
  • Add skill docs and dry-run E2E coverage for the new shortcuts.

Test Plan

  • go test ./shortcuts/drive
  • go test ./tests/cli_e2e/drive -run TestDrive_SecureLabelDryRun

Summary by CodeRabbit

  • New Features

    • Added +secure-label-list to list available secure labels and +secure-label-update to set a secure label on files/documents (supports dry-run).
  • Documentation

    • Added docs and skill notes for the new commands, including guidance for upgrade/downgrade approval error flows.
  • Tests

    • Added unit and end-to-end tests covering dry-run behavior, input validation, success paths, and API error propagation.

Review Change Stack

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths labels May 20, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

📝 Walkthrough

Walkthrough

This PR adds two Drive secure-label CLI shortcuts: +secure-label-list (lists available secure labels) and +secure-label-update (patches a file's secure label). Both provide dry-run support, input validation, unit and e2e tests, SKILL/reference docs, and updated command registration and coverage metrics.

Changes

Drive Secure-Label Commands

Layer / File(s) Summary
Secure Label Implementation
shortcuts/drive/drive_secure_label.go
Adds constants/types and implements DriveSecureLabelList (validates --page-size, builds pagination/lang query params, GET /open-apis/drive/v2/my_secure_labels) and DriveSecureLabelUpdate (requires --token and --label-id, optional --type, resolves target, PATCH /open-apis/drive/v2/files/:file_token/secure_label with {id} body). Includes helper functions for params and target resolution.
Unit Tests for List and Update
shortcuts/drive/drive_secure_label_test.go
Adds tests for dry-run construction, page-size validation errors, successful list execution, update dry-run type inference from URL, successful update execution (verifies request body), and downgrade-approval API error propagation.
Shortcuts Registration and Discovery
shortcuts/drive/shortcuts.go, shortcuts/drive/shortcuts_test.go
Registers DriveSecureLabelList and DriveSecureLabelUpdate in Shortcuts(); updates discovery test to expect the new commands.
User-Facing Documentation
skills/lark-drive/SKILL.md, skills/lark-drive/references/lark-drive-secure-label.md
Adds SKILL entries and a reference page with usage examples, parameter rules, REST endpoints (GET /open-apis/drive/v2/my_secure_labels, PATCH /open-apis/drive/v2/files/:file_token/secure_label), and guidance for error code 1063013 (complete approval in document UI).
End-to-End Dry-Run Tests and Coverage
tests/cli_e2e/drive/drive_secure_label_dryrun_test.go, tests/cli_e2e/drive/coverage.md
Adds e2e dry-run test asserting generated HTTP method/URL and JSON fields (list pagination params; update inferred type, body.id, and file_token), and updates coverage metrics/table to include the two new dry-run commands.

Possibly related PRs

  • larksuite/cli#588: The secure-label-update shortcut reuses resolvePermApplyTarget and document-type inference patterns introduced for permission-apply operations.

Suggested labels

size/M, domain/ccm

Suggested reviewers

  • fangshuyu-768
  • wittam-01

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 A label to secure, a shortcut so clear,
List them and update with confident cheer,
Downgrade approvals now point to the UI,
CLI dry-runs and docs help users comply!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% 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 'feat(drive): add secure label shortcuts' clearly and concisely describes the main change—adding two new Drive shortcuts for secure label management.
Description check ✅ Passed The PR description covers all required template sections: a clear summary of the two new shortcuts and their features, specific test commands, and addresses related issues (none applicable).
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/srcret

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 and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@7125f6b3146629b3c7841f42d640425857a6ed37

🧩 Skill update

npx skills add larksuite/cli#feat/srcret -y -g

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.61%. Comparing base (69c3448) to head (7125f6b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/drive/drive_secure_label.go 89.65% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #985      +/-   ##
==========================================
+ Coverage   67.58%   67.61%   +0.02%     
==========================================
  Files         575      576       +1     
  Lines       54269    54329      +60     
==========================================
+ Hits        36679    36733      +54     
- Misses      14548    14551       +3     
- Partials     3042     3045       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
skills/lark-drive/references/lark-drive-secure-label.md (1)

44-50: 💤 Low value

Consider clarifying "重试" to avoid ambiguity.

Line 48 使用 "重试" (retry) which could be misinterpreted as automatic API retry, though line 50 clarifies that automatic retry should not happen. To eliminate ambiguity, consider rephrasing line 48 guidance to "提示用户打开目标文档,在文档界面完成密级降级审批后再次执行命令" to make it explicit that the user should manually re-run the command rather than relying on automatic retry logic.

📝 Suggested clarification
-| `1063013` | 密级降级需要审批 | 提示用户打开目标文档,在文档界面完成密级降级审批后重试;如果用户传入的是文档 URL,必须把该 URL 一并给用户作为操作入口 |
+| `1063013` | 密级降级需要审批 | 提示用户打开目标文档,在文档界面完成密级降级审批后再次执行命令;如果用户传入的是文档 URL,必须把该 URL 一并给用户作为操作入口 |
🤖 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-drive/references/lark-drive-secure-label.md` around lines 44 -
50, Update the guidance text for error code `1063013` to avoid ambiguity about
"重试": replace the phrase "提示用户打开目标文档,在文档界面完成密级降级审批后重试" with an explicit
instruction such as "提示用户打开目标文档,在文档界面完成密级降级审批后再次执行命令(请用户手动重新运行,而非依赖自动重试)" so it
clearly differentiates manual user re-run from any automatic API retry logic in
the `1063013` table entry.
shortcuts/drive/drive_secure_label_test.go (1)

146-147: ⚡ Quick win

Tighten the downgrade test to assert inferred type=docx on execute path.

This stub currently allows a pass without verifying the inferred type query in this specific error-path test. Use the typed URL (or an equivalent captured-query assertion) to lock this contract.

Proposed fix
-		URL:    "/open-apis/drive/v2/files/doxTok123/secure_label",
+		URL:    "/open-apis/drive/v2/files/doxTok123/secure_label?type=docx",
🤖 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/drive/drive_secure_label_test.go` around lines 146 - 147, The
test's error-path stub currently checks only the path
"/open-apis/drive/v2/files/doxTok123/secure_label" and Status 403 but doesn't
assert that the client inferred type=docx on the execute path; update the test
in drive_secure_label_test.go to expect the typed URL (e.g. include the query
"type=docx") or add an assertion on the mock server's captured query params so
the outgoing request contains type=docx, ensuring the mocked handler for the
secure_label endpoint verifies the inferred type before returning 403.
🤖 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/drive/drive_secure_label.go`:
- Around line 135-137: The code appends rawTarget directly to hint which may
leak sensitive query or fragment data; update the logic in the function that
sets hint (the block using rawTarget and hint) to parse rawTarget with net/url
(e.g., url.Parse), zero out or omit the RawQuery and Fragment components, then
append the sanitized URL form (scheme://host/path or url.String() after clearing
query/fragment) instead of rawTarget so only a safe URL is logged.

---

Nitpick comments:
In `@shortcuts/drive/drive_secure_label_test.go`:
- Around line 146-147: The test's error-path stub currently checks only the path
"/open-apis/drive/v2/files/doxTok123/secure_label" and Status 403 but doesn't
assert that the client inferred type=docx on the execute path; update the test
in drive_secure_label_test.go to expect the typed URL (e.g. include the query
"type=docx") or add an assertion on the mock server's captured query params so
the outgoing request contains type=docx, ensuring the mocked handler for the
secure_label endpoint verifies the inferred type before returning 403.

In `@skills/lark-drive/references/lark-drive-secure-label.md`:
- Around line 44-50: Update the guidance text for error code `1063013` to avoid
ambiguity about "重试": replace the phrase "提示用户打开目标文档,在文档界面完成密级降级审批后重试" with an
explicit instruction such as
"提示用户打开目标文档,在文档界面完成密级降级审批后再次执行命令(请用户手动重新运行,而非依赖自动重试)" so it clearly
differentiates manual user re-run from any automatic API retry logic in the
`1063013` table entry.
🪄 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: e0941114-fc6e-4da7-ac41-8afffdfed33d

📥 Commits

Reviewing files that changed from the base of the PR and between 27a5eed and 954ae1a.

📒 Files selected for processing (8)
  • shortcuts/drive/drive_secure_label.go
  • shortcuts/drive/drive_secure_label_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-secure-label.md
  • tests/cli_e2e/drive/coverage.md
  • tests/cli_e2e/drive/drive_secure_label_dryrun_test.go

Comment thread shortcuts/drive/drive_secure_label.go Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
shortcuts/drive/drive_secure_label_test.go (1)

161-163: ⚡ Quick win

Assert the 1063013 actionable hint in the downgrade-error test.

This test currently verifies only the raw API message. Please also assert the user-facing downgrade-approval hint text so the intended behavior is regression-safe.

🤖 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/drive/drive_secure_label_test.go` around lines 161 - 163, The test
currently only checks the raw API error string (err.Error()); update the test in
drive_secure_label_test.go (the downgrade-error assertion block where err is
checked) to also assert that the user-facing actionable hint is present by
verifying either the hint ID "1063013" or the corresponding user-facing
downgrade-approval message appears in the error output; add a second assertion
after the existing strings.Contains check that fails the test if the actionable
hint text (or "1063013") is missing so the downgrade-approval hint is covered by
the test.
🤖 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/drive/drive_secure_label.go`:
- Around line 103-105: The error return path in drive_secure_label.go (the block
that currently does "if err != nil { return err }") must detect when the API
error contains code 1063013 and, in that case, write a clear downgrade-approval
hint to stderr (use fmt.Fprintln(os.Stderr, "...") or log.New(os.Stderr, ...))
while still returning the original API error (or wrapping it without losing the
original error value, e.g., fmt.Errorf("%w", err)). Modify the error branch in
the function handling "+secure-label-update" to inspect the API error type or
message for code 1063013, emit the guidance to stderr, and then return the
original error so the API context is preserved and stdout remains untouched.

---

Nitpick comments:
In `@shortcuts/drive/drive_secure_label_test.go`:
- Around line 161-163: The test currently only checks the raw API error string
(err.Error()); update the test in drive_secure_label_test.go (the
downgrade-error assertion block where err is checked) to also assert that the
user-facing actionable hint is present by verifying either the hint ID "1063013"
or the corresponding user-facing downgrade-approval message appears in the error
output; add a second assertion after the existing strings.Contains check that
fails the test if the actionable hint text (or "1063013") is missing so the
downgrade-approval hint is covered by the test.
🪄 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: 7f9edd9e-ae6b-488e-afb2-eb12626fefbc

📥 Commits

Reviewing files that changed from the base of the PR and between 954ae1a and 7125f6b.

📒 Files selected for processing (8)
  • shortcuts/drive/drive_secure_label.go
  • shortcuts/drive/drive_secure_label_test.go
  • shortcuts/drive/shortcuts.go
  • shortcuts/drive/shortcuts_test.go
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-secure-label.md
  • tests/cli_e2e/drive/coverage.md
  • tests/cli_e2e/drive/drive_secure_label_dryrun_test.go
✅ Files skipped from review due to trivial changes (3)
  • skills/lark-drive/SKILL.md
  • skills/lark-drive/references/lark-drive-secure-label.md
  • tests/cli_e2e/drive/coverage.md

Comment thread shortcuts/drive/drive_secure_label.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant