fix: validate base shortcut JSON object inputs#458
Merged
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds pre-execution JSON validation to multiple base shortcuts by introducing Validate hooks and tightening JSON parsing/error messages so that --json inputs are enforced to be the expected object/array shapes before DryRun/Execute. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as Shortcut Framework
participant Shortcut as Base Shortcut
participant Validator as Validate Hook
participant Parser as parseJSONObject
participant Formatter as Error Formatter
User->>CLI: run command with --json
CLI->>Shortcut: initialize runtime, invoke Shortcut
Shortcut->>Validator: Validate(ctx, runtime)
Validator->>Parser: parseJSONObject(runtime.Str("json"), "json")
alt parsed object ok
Parser-->>Validator: parsed object
Validator-->>Shortcut: nil (validation passed)
Shortcut->>CLI: proceed to DryRun/Execute
else syntax error
Parser-->>Formatter: formatJSONError / tip
Formatter-->>Validator: error
Validator-->>CLI: return validation error
CLI-->>User: surface error (e.g., "--json must be a JSON object" + tip)
else non-object (null/array)
Parser-->>Formatter: common.FlagErrorf("--json must be a JSON object"...)
Formatter-->>Validator: error
Validator-->>CLI: return validation error
CLI-->>User: surface error with tip
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
domain/base
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
kongenpei has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@fb25ba2eb20424a6b398ce098eb27810c8833cf6🧩 Skill updatenpx skills add larksuite/cli#codex/base-json-object-validation -y -g |
![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)
shortcuts/base/helpers.go (1)
33-47:⚠️ Potential issue | 🟠 Major
nullbypasses object-only validation inparseJSONObject.At line 46, the function returns success when the payload is JSON
null, sincejson.Unmarshaltreatsnullas valid input formap[string]interface{}(returning nil error and a nil map). This allows non-object input through validation and can cause downstream issues.Add an explicit nil check after successful parsing:
Proposed fix
func parseJSONObject(pc *parseCtx, raw string, flagName string) (map[string]interface{}, error) { resolved, err := loadJSONInput(pc, raw, flagName) if err != nil { return nil, err } var result map[string]interface{} if err := common.ParseJSON([]byte(resolved), &result); err != nil { var syntaxErr *json.SyntaxError if errors.As(err, &syntaxErr) { return nil, formatJSONError(flagName, "object", err) } return nil, common.FlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName)) } + if result == nil { + return nil, common.FlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName)) + } return result, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/base/helpers.go` around lines 33 - 47, parseJSONObject currently accepts JSON "null" because common.ParseJSON succeeds but yields a nil map; after calling common.ParseJSON in parseJSONObject (the block that assigns to var result map[string]interface{}), add an explicit nil check (if result == nil) and return the same flag-style error used for non-object inputs (e.g. return nil, common.FlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName))). This keeps existing syntax error handling (formatJSONError) intact and ensures only real JSON objects are accepted.
🤖 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 `@shortcuts/base/helpers.go`:
- Around line 33-47: parseJSONObject currently accepts JSON "null" because
common.ParseJSON succeeds but yields a nil map; after calling common.ParseJSON
in parseJSONObject (the block that assigns to var result
map[string]interface{}), add an explicit nil check (if result == nil) and return
the same flag-style error used for non-object inputs (e.g. return nil,
common.FlagErrorf("--%s must be a JSON object; %s", flagName,
jsonInputTip(flagName))). This keeps existing syntax error handling
(formatJSONError) intact and ensures only real JSON objects are accepted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25258953-69da-4368-ab79-f1243e3dfd30
📒 Files selected for processing (15)
shortcuts/base/base_execute_test.goshortcuts/base/base_shortcut_helpers.goshortcuts/base/base_shortcuts_test.goshortcuts/base/field_ops.goshortcuts/base/helpers.goshortcuts/base/helpers_test.goshortcuts/base/record_batch_create.goshortcuts/base/record_batch_update.goshortcuts/base/record_ops.goshortcuts/base/record_search.goshortcuts/base/record_upsert.goshortcuts/base/view_ops.goshortcuts/base/view_set_group.goshortcuts/base/view_set_sort.goshortcuts/base/view_set_visible_fields.go
There was a problem hiding this comment.
kongenpei has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
zgz2048
approved these changes
Apr 14, 2026
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
2 participants
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
Tighten base shortcut JSON validation so commands that require JSON objects fail early with actionable CLI errors instead of deferring malformed payloads to dry-run or API passthrough.
Changes
@file.json, the command reference, and thelark-baseskill.Test Plan
lark xxxcommand works as expectedmake unit-testgo mod tidy(no changes togo.modorgo.sum)go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/mainRelated Issues
Summary by CodeRabbit
Bug Fixes
Documentation