Skip to content

fix(drive): require user id before bot-created resources#1457

Open
xu91102 wants to merge 1 commit into
larksuite:mainfrom
xu91102:codex/fix-drive-auto-grant-1455
Open

fix(drive): require user id before bot-created resources#1457
xu91102 wants to merge 1 commit into
larksuite:mainfrom
xu91102:codex/fix-drive-auto-grant-1455

Conversation

@xu91102

@xu91102 xu91102 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent drive +upload and drive +import from creating new bot-owned Drive resources when the CLI does not know the current user's open_id. This avoids returning resources that cannot be auto-granted back to the user.

Fixes #1455

Changes

  • Add a Drive preflight that fails bot-mode create flows with failed_precondition when user_open_id is missing.
  • Apply the preflight before remote upload/import API calls for drive +upload new uploads and drive +import.
  • Keep drive +upload --file-token overwrite behavior unchanged because it does not create a new resource.
  • Update Drive permission/upload tests to cover the new preflight and preserve existing upload behavior with a test user id.

Test Plan

  • go test ./shortcuts/drive -run "TestDrive(UploadBotAutoGrantSuccess|UploadBotRequiresCurrentUserBeforeCreate|UploadBotAutoGrantFailed|UploadBotOverwriteSkipsPermissionGrant|ImportBotAutoGrantSuccess|ImportBotRequiresCurrentUserBeforeCreate|ImportTimeoutReturnsFollowUpCommand|UploadSmallFile$|UploadLargeFileUsesMultipart$|UploadLargeFileToWikiUsesMultipart$|UploadSmallFileToWiki$|UploadSmallFileAPIError$|UploadPrepareInvalidResponse$|UploadPartAPIError$|UploadFinishNoToken$|UploadWithCustomName$)" -count=1
  • go test ./shortcuts/common -run "TestAutoGrant" -count=1
  • go build -o lark-cli.exe .
  • go test ./shortcuts/drive -count=1 (fails on existing Windows local filesystem/symlink and rollback-mock tests unrelated to this change)
  • go test ./tests/cli_e2e/drive -run "TestDriveUploadDryRun" -count=1 (Windows helper rejects lark-cli.exe because it checks Unix executable mode bits)

Related Issues

Summary by CodeRabbit

  • New Features

    • Drive import and upload operations now require user authentication when running in bot mode, with pre-operation validation and clear error guidance for users who need to authenticate.
  • Tests

    • Updated test suite to validate authentication requirement enforcement and error handling for Drive operations.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dffe8c59-7406-4188-ab17-41f3d1483ac6

📥 Commits

Reviewing files that changed from the base of the PR and between c0730b4 and ee52083.

📒 Files selected for processing (5)
  • shortcuts/drive/drive_import.go
  • shortcuts/drive/drive_io_test.go
  • shortcuts/drive/drive_permission_grant_test.go
  • shortcuts/drive/drive_permission_preflight.go
  • shortcuts/drive/drive_upload.go

📝 Walkthrough

Walkthrough

Adds a new requireDriveBotCurrentUserForCreate preflight function that returns a ValidationError (SubtypeFailedPrecondition) when running in bot mode without a configured UserOpenId. This check is wired into both drive +upload (new uploads only) and drive +import execution paths. Existing test fixtures are updated to include UserOpenId, and new tests validate the error behavior.

Changes

Bot current-user preflight for Drive create operations

Layer / File(s) Summary
requireDriveBotCurrentUserForCreate helper
shortcuts/drive/drive_permission_preflight.go
New file defines the preflight function returning a typed ValidationError with SubtypeFailedPrecondition and an auth login hint when bot mode lacks a current user open_id.
Wiring preflight into upload and import
shortcuts/drive/drive_upload.go, shortcuts/drive/drive_import.go
DriveUpload.Execute calls the preflight when not overwriting; DriveImport.Execute calls it after file preflight; both abort early on error.
Permission preflight tests
shortcuts/drive/drive_permission_grant_test.go
Adds TestDriveImportBotRequiresCurrentUserBeforeCreate and replaces TestDriveUploadBotAutoGrantSkippedNoUser with TestDriveUploadBotRequiresCurrentUserBeforeCreate; assertions now validate *errs.ValidationError with SubtypeFailedPrecondition; overwrite test adjusted to empty userOpenId.
IO test fixture updates
shortcuts/drive/drive_io_test.go
All upload and download test configs now include UserOpenId: "ou_drive_test_user" to pass the new preflight check; TestDriveUploadLargeFileUsesMultipart switches to direct os.Chdir for cleanup-order safety.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • larksuite/cli#360: The new bot-mode preflight directly extends the auto-grant logic for full_access introduced in this PR, replacing the prior "skip with warning" behavior with a hard validation error.
  • larksuite/cli#885: This PR's requireDriveBotCurrentUserForCreate guard in drive_upload.go is conditional on isOverwrite == false, a concept introduced by the --file-token overwrite behavior in PR #885.
  • larksuite/cli#194: The new preflight is inserted into the drive +import task-creation flow originally established by PR #194.

Suggested labels

size/L, domain/ccm

Suggested reviewers

  • wittam-01
  • SunPeiYang996

Poem

🐇 Hoppy the rabbit checks the gate,
No open_id? You'll have to wait!
Bot mode needs a user to share,
auth login — the hint is right there.
The Drive is safe, no orphaned file,
Every doc owned with proper style! 🗂️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 accurately describes the main change: adding a requirement for user ID before bot-created resources in the drive module.
Description check ✅ Passed The description provides a clear summary, comprehensive list of changes, detailed test plan with specific test names, and related issue references following the template.
Linked Issues check ✅ Passed The pull request implements the stated solution for #1455 by adding preflight validation that prevents bot-created resource operations when user open_id is missing, prompting authentication instead.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the preflight check requirement from #1455 across drive_import.go, drive_upload.go, drive_permission_preflight.go, and related tests.

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

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

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact labels Jun 14, 2026
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/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] Bot创建的资源缺少用户自动授权,导致用户无法编辑自己创建的表格

1 participant