Skip to content

feat(apps): support absolute and relative upload paths#2005

Merged
liangshuo-1 merged 2 commits into
mainfrom
agent/apps-file-upload-local-paths
Jul 23, 2026
Merged

feat(apps): support absolute and relative upload paths#2005
liangshuo-1 merged 2 commits into
mainfrom
agent/apps-file-upload-local-paths

Conversation

@liangshuo-1

@liangshuo-1 liangshuo-1 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Allow apps +file-upload --file to read absolute and parent-relative local paths without weakening the existing workspace-confined FileIO behavior used by other commands.

Changes

  • Add a shared local-file path validator and regular-file opener under internal/.
  • Add lightweight RuntimeContext.ValidateLocalFileFlag and ReadLocalFileFlag helpers for gradual shortcut adoption.
  • Validate file existence, readability, regular-file type, and size before dry-run or execution; re-check and bound reads during execution.
  • Migrate apps +file-upload to the shared helper.
  • Add unit, dry-run E2E, and fixture-gated live E2E coverage for absolute paths, parent-relative paths, missing files, non-regular files, and size limits.

Test Plan

  • go test ./shortcuts/common ./shortcuts/apps
  • go test -race ./shortcuts/common ./shortcuts/apps
  • go vet ./internal/cmdutil ./internal/validate ./internal/vfs/localfileio ./shortcuts/common ./shortcuts/apps
  • golangci-lint run --new-from-rev=origin/main for the affected packages
  • File-upload dry-run E2E validates an existing absolute path and rejects a missing path
  • Live E2E is fixture-gated and skips when no dedicated app fixture is configured

Related Issues

Summary by CodeRabbit

  • New Features

    • File uploads now support absolute paths and parent-relative paths outside the current directory.
    • Added consistent local-file validation and reading for CLI commands.
  • Bug Fixes

    • Invalid, missing, oversized, directory, and unsafe paths now produce clear validation errors.
    • File uploads avoid reading unsupported device files.
    • Windows network, device, and reserved paths are rejected.
  • Documentation

    • Expanded file-upload command coverage and dry-run behavior documentation.

@github-actions github-actions Bot added the size/L Large or sensitive change across domains or core paths label Jul 22, 2026
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Local input paths now use platform-aware validation and shared typed-error file helpers. Apps file upload adopts these helpers for validation and reading, with expanded unit, integration, dry-run, live workflow, and coverage documentation tests.

Changes

Local file input and upload

Layer / File(s) Summary
Local input path validation
internal/vfs/localfileio/..., internal/validate/path.go, internal/validate/path_test.go
Adds LocalInputPath, control-character checks, Windows namespace restrictions, non-Windows support, and updated error wrapping.
Shared local file access helpers
internal/cmdutil/localfile.go, shortcuts/common/localfile.go, related tests
Adds validated stat/open helpers and typed validation/read utilities enforcing regular-file and size constraints.
File-upload helper integration
shortcuts/apps/apps_file_upload.go, shortcuts/apps/apps_file_upload_test.go
Routes --file validation and reading through RuntimeContext, covering absolute, parent-relative, oversized, missing, and non-regular paths.
CLI end-to-end coverage
tests/cli_e2e/apps/*
Adds dry-run and fixture-gated live upload coverage, including retrieval, size verification, cleanup, and documentation updates.

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

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant RuntimeContext
  participant LocalInputPath
  participant LocalFilesystem
  participant UploadAPI
  CLI->>RuntimeContext: ValidateLocalFileFlag("--file", maxBytes)
  RuntimeContext->>LocalInputPath: Validate path
  LocalInputPath-->>RuntimeContext: Validated path
  RuntimeContext->>LocalFilesystem: Stat regular file and size
  RuntimeContext-->>CLI: Validation result
  CLI->>RuntimeContext: ReadLocalFileFlag("--file", maxBytes)
  RuntimeContext->>LocalFilesystem: Open and bounded read
  LocalFilesystem-->>RuntimeContext: File bytes
  RuntimeContext-->>CLI: File bytes
  CLI->>UploadAPI: Upload file bytes
Loading

Possibly related PRs

  • larksuite/cli#314: Establishes the localfileio safe-path foundation extended by this change.
  • larksuite/cli#339: Covers related path-validation error translation for file access.
  • larksuite/cli#1978: Also changes apps file-upload handling for absolute and out-of-tree paths.

Suggested labels: feature

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: enabling absolute and parent-relative upload paths for apps file upload.
Description check ✅ Passed The description follows the template and includes a clear summary, change list, test plan, and related issue.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/apps-file-upload-local-paths

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.

@liangshuo-1
liangshuo-1 marked this pull request as ready for review July 22, 2026 13:31
@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#agent/apps-file-upload-local-paths -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: 3

🤖 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 `@internal/vfs/localfileio/path.go`:
- Around line 58-59: Update the test covering the SafeInputPath validation in
the relevant path-parsing flow to assert that the returned error unwraps to the
original validation error, not only that its message matches. Capture the
underlying error from SafeInputPath and verify errors.Is or equivalent
unwrapping against the returned error, preserving the existing text assertion if
useful.

In `@tests/cli_e2e/apps/apps_file_upload_dryrun_test.go`:
- Around line 60-61: Update the assertions in the file-upload dry-run test to
parse the validation envelope from result.Stdout instead of result.Stderr. If
the command implementation currently writes this structured validation JSON to
stderr, change its output routing so JSON program output uses stdout while
progress, warnings, and hints remain on stderr.

In `@tests/cli_e2e/apps/apps_file_upload_live_test.go`:
- Around line 21-28: Update TestAppsFileUploadLiveWorkflow to remove the
LARKSUITE_CLI_CONFIG_DIR requirement and skip gate. Keep the dedicated
LARK_CLI_E2E_APPS_FILE_APP_ID fixture check, and use the repository’s live-token
skip helper for credential validation instead of requiring config isolation.
🪄 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: 4c19e0f7-c480-407c-8f94-9ddc6b040f74

📥 Commits

Reviewing files that changed from the base of the PR and between 8f6f8eb and 8ece7c0.

📒 Files selected for processing (16)
  • internal/cmdutil/localfile.go
  • internal/cmdutil/localfile_test.go
  • internal/validate/path.go
  • internal/validate/path_test.go
  • internal/vfs/localfileio/path.go
  • internal/vfs/localfileio/path_local_other.go
  • internal/vfs/localfileio/path_local_windows.go
  • internal/vfs/localfileio/path_local_windows_test.go
  • internal/vfs/localfileio/path_test.go
  • shortcuts/apps/apps_file_upload.go
  • shortcuts/apps/apps_file_upload_test.go
  • shortcuts/common/localfile.go
  • shortcuts/common/localfile_test.go
  • tests/cli_e2e/apps/apps_file_upload_dryrun_test.go
  • tests/cli_e2e/apps/apps_file_upload_live_test.go
  • tests/cli_e2e/apps/coverage.md

Comment thread internal/vfs/localfileio/path.go
Comment thread tests/cli_e2e/apps/apps_file_upload_dryrun_test.go
Comment thread tests/cli_e2e/apps/apps_file_upload_live_test.go
@liangshuo-1
liangshuo-1 merged commit b8f56db into main Jul 23, 2026
45 checks passed
@liangshuo-1
liangshuo-1 deleted the agent/apps-file-upload-local-paths branch July 23, 2026 09:52
@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

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants