feat(cmdutil): support @file for params and data#724
Open
liangshuo-1 wants to merge 3 commits intomainfrom
Open
feat(cmdutil): support @file for params and data#724liangshuo-1 wants to merge 3 commits intomainfrom
liangshuo-1 wants to merge 3 commits intomainfrom
Conversation
Inline JSON values for --params/--data are mangled by Windows PowerShell 5's CommandLineToArgvW. Stdin (-) was the only escape hatch but supports just one flag at a time. Extend ResolveInput to accept @<path> (read JSON from a file) and @@... (escape for a literal @-prefixed value), mirroring the shortcuts framework's resolveInputFlags semantics. With this, both --params and --data can be sourced from files in the same call, sidestepping shell quoting on every platform. - internal/cmdutil/resolve.go: add @path / @@ handling, trim file content like stdin does, error on empty path or empty file - internal/cmdutil/resolve_test.go: cover file read, whitespace trim, missing file, empty path, empty content, @@ escape, plus ParseJSONMap / ParseOptionalBody integration through @file - cmd/api/api.go, cmd/service/service.go: update --params/--data help text to mention @file Change-Id: I366aa0f5783fbec6f05403f7f542505098a98c82
The first cut of @file support called os.ReadFile directly inside ResolveInput, bypassing the codebase's fileio.FileIO abstraction (SafeInputPath validation, pluggable provider). That diverged from how every other file-reading path works: BuildFormdata for --file uploads and the shortcuts framework's resolveInputFlags both go through fileio.FileIO.Open with explicit fileio.ErrPathValidation handling. Re-route @file through the same path: - ResolveInput, ParseJSONMap, ParseOptionalBody now take a fileio.FileIO; @path uses fileIO.Open which goes through SafeInputPath (control-char rejection, abs-path rejection, symlink-escape check) — same security posture as --file - cmd/api and cmd/service callsites pass Factory.ResolveFileIO(ctx); the upload path now reuses the resolved fileIO instead of resolving twice - Path-validation errors surface as `--params: invalid file path "...": ...` distinct from `--params: cannot read file "...": ...` for genuine I/O errors - Nil fileIO with an @path returns a clear "file input (@path) is not available" error - Tests use localfileio.LocalFileIO with TestChdir(t, dir), matching the existing fileupload_test.go pattern; absolute-path rejection and nil-fileIO are covered This makes the feature behave identically under any FileIO provider (including server mode) instead of being silently bound to the local filesystem. Change-Id: I878c4e8fb03f43f1f19afad75ec3af9cdab7a7f9
Change-Id: I92a6eb6ea8fd02054bf8f4925cd81807449d5e51
📝 WalkthroughWalkthroughThe pull request adds Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Cmd as Command Handler<br/>(api.go/service.go)
participant Resolve as ResolveInput<br/>(resolve.go)
participant FileIO as FileIO Instance
participant JSON as JSON Parser<br/>(json.go)
participant File as File System
User->>Cmd: Pass --params `@params.json`
Cmd->>Cmd: Resolve fileIO from factory
Cmd->>JSON: Call ParseJSONMap(input, fileIO)
JSON->>Resolve: Call ResolveInput(input, fileIO)
Resolve->>Resolve: Check if input starts with @
alt `@file` path detected
Resolve->>FileIO: Open file at path
FileIO->>File: Read file contents
File-->>FileIO: Return bytes
FileIO-->>Resolve: Return trimmed string
else @@escape or other
Resolve->>Resolve: Return as-is or process
end
Resolve-->>JSON: Return resolved string
JSON->>JSON: Parse JSON string
JSON-->>Cmd: Return parsed JSON object
Cmd->>Cmd: Use in request
Cmd-->>User: Send API request
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 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 |
github-actions
Bot
added
the
size/L
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #724 +/- ##
==========================================
+ Coverage 64.14% 64.16% +0.01%
==========================================
Files 504 504
Lines 44285 44337 +52
==========================================
+ Hits 28406 28448 +42
- Misses 13411 13416 +5
- Partials 2468 2473 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/cmdutil/resolve.go (1)
32-73:⚠️ Potential issue | 🟠 MajorStrip single quotes before interpreting special tokens.
Because Line 70 runs after the
-/@branches, inputs like'@params.json','@@literal', and'-'are treated as plain strings instead of file/stdin/escaped inputs. That breaks the shell-quoting compatibility this feature is supposed to add.Proposed fix
func ResolveInput(raw string, stdin io.Reader, fileIO fileio.FileIO) (string, error) { if raw == "" { return "", nil } + + // strip surrounding single quotes first (Windows shell compatibility) + if len(raw) >= 2 && raw[0] == '\'' && raw[len(raw)-1] == '\'' { + raw = raw[1 : len(raw)-1] + } // stdin if raw == "-" { if stdin == nil { return "", fmt.Errorf("stdin is not available") @@ - // strip surrounding single quotes (Windows cmd.exe passes them literally) - if len(raw) >= 2 && raw[0] == '\'' && raw[len(raw)-1] == '\'' { - raw = raw[1 : len(raw)-1] - } - return raw, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmdutil/resolve.go` around lines 32 - 73, The code currently strips surrounding single quotes after handling stdin ("-"), escaped "@@" and file "@path" branches, so inputs like "'@file'", "'@@literal'" or "'-'" are not recognized; move the single-quote stripping logic (the check that trims raw = raw[1:len(raw)-1] when raw starts and ends with '\'') to occur before the stdin/escape/file handling (i.e., immediately after obtaining the raw parameter), so that the subsequent checks using raw, stdin and ReadInputFile(fileIO, path) correctly interpret quoted special tokens; ensure you still preserve the existing behavior for unquoted inputs and keep the escape/file/stdin branches unchanged otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@internal/cmdutil/resolve.go`:
- Around line 32-73: The code currently strips surrounding single quotes after
handling stdin ("-"), escaped "@@" and file "@path" branches, so inputs like
"'@file'", "'@@literal'" or "'-'" are not recognized; move the single-quote
stripping logic (the check that trims raw = raw[1:len(raw)-1] when raw starts
and ends with '\'') to occur before the stdin/escape/file handling (i.e.,
immediately after obtaining the raw parameter), so that the subsequent checks
using raw, stdin and ReadInputFile(fileIO, path) correctly interpret quoted
special tokens; ensure you still preserve the existing behavior for unquoted
inputs and keep the escape/file/stdin branches unchanged otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c798c0d7-fa93-4364-ba27-4afcbfe9c01a
📒 Files selected for processing (8)
cmd/api/api.gocmd/service/service.gointernal/cmdutil/json.gointernal/cmdutil/json_test.gointernal/cmdutil/resolve.gointernal/cmdutil/resolve_test.goshortcuts/common/runner.goshortcuts/common/runner_input_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@51a0ddce805475bb4a6e5e755087f6c058c3f532🧩 Skill updatenpx skills add larksuite/cli#feat/cmdutil-at-file-input -y -g |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds @file input support for generic API/service --params and --data JSON flags so users can avoid shell quoting issues, especially on Windows PowerShell. This PR keeps file reads behind the FileIO abstraction and supersedes #715.
Changes
Test Plan
Related Issues
--params/--datafail on Windows PowerShell 5 due to JSON quote mangling + feature request:@filesupport #705Summary by CodeRabbit
--paramsand--dataflags now support@<filepath>syntax to read input from files, with automatic whitespace trimming.@@escape sequence to specify literal@characters in command-line input.