fix: accept absolute paths in file flags; add dry-run chat membership warning#955
fix: accept absolute paths in file flags; add dry-run chat membership warning#955LawyerLyu wants to merge 2 commits into
Conversation
…hip warning Fixes larksuite#872: --image, --file, --video, --audio and --local-dir flags previously rejected absolute paths with a confusing "must be a relative path within the current directory" error. Common CLI tools (gh, gcloud, aws s3 cp, kubectl cp) all accept absolute paths; OS permissions enforce access when the file is actually read. Absolute path support is now enabled in safePath() while relative path traversal (../) protection is preserved. Fixes larksuite#915: im +messages-send --dry-run validated request shape but gave no indication that bot/user chat membership is not verified. A chat-id send now includes a NOTE in dry-run output warning that the real send may still fail with "Bot/User can NOT be out of the chat". The --output-dir flag in event consume retains its absolute-path restriction since event output should stay within the working directory. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR accepts absolute filesystem paths across validators and LocalFileIO by resolving symlinks for absolute inputs; it updates callers and many tests to reflect absolute-path acceptance, while retaining path-traversal protections. It also adds a conditional dry-run warning for chat sends. ChangesAbsolute Path Acceptance Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #955 +/- ##
==========================================
- Coverage 66.78% 66.77% -0.02%
==========================================
Files 564 564
Lines 52441 52449 +8
==========================================
- Hits 35024 35022 -2
- Misses 14516 14521 +5
- Partials 2901 2906 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@157e3d37f067a88d67e94c843711acb86183cc49🧩 Skill updatenpx skills add LawyerLyu/cli#fix/absolute-path-and-dryrun-warning -y -g |
…uences bidichk linter rejects literal U+202E (RLO), U+2066 (LRI), U+200B (ZWSP), and U+2028 (LS) in source files. Replace with Go string escape sequences (, , , ) so the test coverage is unchanged. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Summary
fix
--image/--fileflags reject absolute paths but--helpdoesn't document this #872:--image,--file,--video,--audio,--local-dirflags rejected absolute paths with a misleading error. Common CLI tools (gh,gcloud,aws s3 cp,kubectl cp) all accept absolute paths; OS permissions already enforce access. Now absolute paths pass validation while../traversal protection is preserved.fix Clarify that im messages-send --dry-run does not verify chat membership #915:
im +messages-send --dry-runsucceeded for syntactically valid payloads but gave no indication that bot/user chat membership is not verified. A--chat-idsend now prints a NOTE in dry-run output warning the real send may fail withBot/User can NOT be out of the chat.Exception:
event consume --output-dirretains its absolute-path restriction — event output files should remain within the working directory.Changes
internal/vfs/localfileio/path.go:safePath()now accepts absolute paths (resolves symlinks, skips cwd restriction); relative path../guard unchangedcmd/event/consume.go: explicitfilepath.IsAbsguard added tosanitizeOutputDirto maintain its documented restrictionshortcuts/im/im_messages_send.go: chat-id dry-run output includes membership warningTest plan
go test ./...(excluding e2e) passes with no failureslark-cli im +messages-send --chat-id oc_xxx --text hello --dry-runshows NOTE about membershiplark-cli im +messages-reply --message-id om_xxx --image /tmp/photo.png --dry-runno longer errors on absolute pathlark-cli drive +pull --local-dir /home/user/docs --folder-token xxx --dry-runworks with absolute path🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes