Skip to content

fix(base): classify +form-submit as high-risk-write#1969

Merged
yballul-bytedance merged 1 commit into
larksuite:mainfrom
yballul-bytedance:fix/form-subimit-high-risk
Jul 24, 2026
Merged

fix(base): classify +form-submit as high-risk-write#1969
yballul-bytedance merged 1 commit into
larksuite:mainfrom
yballul-bytedance:fix/form-subimit-high-risk

Conversation

@yballul-bytedance

@yballul-bytedance yballul-bytedance commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

base +form-submit fills and submits data through a public form share link — an outward-facing, irreversible write. It was classified as write, so it ran without confirmation. This PR reclassifies it as high-risk-write so the runner's --yes confirmation gate fires before execution, consistent with +form-delete and other high-risk base commands.

Changes

  • shortcuts/base/base_form_submit.go: Risk writehigh-risk-write; add baseHighRiskYesTip to Tips. The runner auto-registers --yes and returns confirmation_required (exit 10) when it is missing.
  • skills/lark-base/references/lark-base-form-submit.md: high-risk warning banner, --yes on all examples, --yes row in the param table, and a tip note.
  • skills/lark-base/SKILL.md: note +form-submit requires --yes in the high-risk list and form-details section.
  • Tests: new TestFormSubmitRequiresConfirmation unit test pins the typed confirmation_required gate; existing execute-path tests pass --yes; new tests/cli_e2e/base/base_form_submit_dryrun_test.go dry-run E2E.

Test Plan

  • Unit tests pass (go test ./shortcuts/base/)
  • Dry-run E2E passes (go test ./tests/cli_e2e/base/)
  • gofmt -l, go vet, go mod tidy clean

Related Issues

  • None

Summary by CodeRabbit

  • New Features

    • Form submissions are now classified as high-risk write operations.
    • Added clearer guidance and tips for confirming form submissions.
  • Bug Fixes

    • Form submissions require the --yes confirmation flag before execution.
    • Dry-run previews remain available without confirmation.
  • Documentation

    • Updated form submission examples, parameters, warnings, and confirmation requirements.

Form submission writes and submits data through a public share link, an
irreversible action that should require explicit confirmation. Reclassify
the shortcut from write to high-risk-write so the runner's --yes gate fires
before execution, matching +form-delete and other high-risk base commands.

Update the lark-base skill docs (--yes on all examples, param table, tips)
and add tests pinning the confirmation gate (unit) and dry-run structure (e2e).
@github-actions github-actions Bot added domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact labels Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Form Submit Confirmation

Layer / File(s) Summary
High-risk shortcut configuration
shortcuts/base/base_form_submit.go, shortcuts/base/base_shortcuts_test.go
BaseFormSubmit now uses the high-risk-write risk level, includes the confirmation tip, and has an updated risk assertion.
Confirmation and dry-run validation
shortcuts/base/base_shortcuts_test.go, tests/cli_e2e/base/base_form_submit_dryrun_test.go
Tests require --yes for write executions, verify the typed confirmation error when omitted, and cover successful dry-run output.
Confirmation guidance
skills/lark-base/SKILL.md, skills/lark-base/references/lark-base-form-submit.md
Guidance and examples document --yes for submissions and the exception for --dry-run.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main change: reclassifying +form-submit as high-risk-write.
Description check ✅ Passed The description follows the template with Summary, Changes, Test Plan, and Related Issues, and it covers the main changes and verification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add yballul-bytedance/cli#fix/form-subimit-high-risk -y -g

@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: 1

🤖 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 `@skills/lark-base/references/lark-base-form-submit.md`:
- Line 78: Update the `--yes` entry in the parameter table for the documented
command to state that confirmation is required only for actual execution, while
explicitly exempting `--dry-run`. Keep the existing `confirmation_required`
behavior for high-risk writes without `--yes` and ensure the wording remains
precise and machine-consumable.
🪄 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: 581f5ffc-b845-4fc5-bd1f-d3cf665c401c

📥 Commits

Reviewing files that changed from the base of the PR and between 78390f8 and c352583.

📒 Files selected for processing (5)
  • shortcuts/base/base_form_submit.go
  • shortcuts/base/base_shortcuts_test.go
  • skills/lark-base/SKILL.md
  • skills/lark-base/references/lark-base-form-submit.md
  • tests/cli_e2e/base/base_form_submit_dryrun_test.go

Comment thread skills/lark-base/references/lark-base-form-submit.md
Comment thread skills/lark-base/references/lark-base-form-submit.md
@yballul-bytedance
yballul-bytedance merged commit 5a54bc0 into larksuite:main Jul 24, 2026
40 of 43 checks passed
kiraWangRuilong pushed a commit that referenced this pull request Jul 24, 2026
Form submission writes and submits data through a public share link, an
irreversible action that should require explicit confirmation. Reclassify
the shortcut from write to high-risk-write so the runner's --yes gate fires
before execution, matching +form-delete and other high-risk base commands.

Update the lark-base skill docs (--yes on all examples, param table, tips)
and add tests pinning the confirmation gate (unit) and dry-run structure (e2e).

Co-authored-by: yballul-bytedance <273011618+yballul-bytedance@users.noreply.github.com>
@liangshuo-1 liangshuo-1 mentioned this pull request Jul 24, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/base PR touches the base domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants