fix: validate base shortcut JSON object inputs (#458)

* fix: validate base shortcut JSON object inputs

* fix: reject null in base JSON object parser

---------

Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>

shortcuts/base/base_execute_test.go

@@ -151,6 +151,87 @@ func TestBaseFieldExecuteUpdate(t *testing.T) {
 	}
 }
 
+func TestBaseObjectJSONShortcutsRejectArrayInDryRun(t *testing.T) {
+	tests := []struct {
+		name     string
+		shortcut common.Shortcut
+		args     []string
+	}{
+		{
+			name:     "field create",
+			shortcut: BaseFieldCreate,
+			args:     []string{"+field-create", "--base-token", "app_x", "--table-id", "tbl_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "field update",
+			shortcut: BaseFieldUpdate,
+			args:     []string{"+field-update", "--base-token", "app_x", "--table-id", "tbl_x", "--field-id", "fld_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "record search",
+			shortcut: BaseRecordSearch,
+			args:     []string{"+record-search", "--base-token", "app_x", "--table-id", "tbl_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "record upsert",
+			shortcut: BaseRecordUpsert,
+			args:     []string{"+record-upsert", "--base-token", "app_x", "--table-id", "tbl_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "record batch create",
+			shortcut: BaseRecordBatchCreate,
+			args:     []string{"+record-batch-create", "--base-token", "app_x", "--table-id", "tbl_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "record batch update",
+			shortcut: BaseRecordBatchUpdate,
+			args:     []string{"+record-batch-update", "--base-token", "app_x", "--table-id", "tbl_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "view set filter",
+			shortcut: BaseViewSetFilter,
+			args:     []string{"+view-set-filter", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "view set visible fields",
+			shortcut: BaseViewSetVisibleFields,
+			args:     []string{"+view-set-visible-fields", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "view set card",
+			shortcut: BaseViewSetCard,
+			args:     []string{"+view-set-card", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `[]`, "--dry-run"},
+		},
+		{
+			name:     "view set timebar",
+			shortcut: BaseViewSetTimebar,
+			args:     []string{"+view-set-timebar", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `[]`, "--dry-run"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			factory, stdout, _ := newExecuteFactory(t)
+			err := runShortcut(t, tt.shortcut, tt.args, factory, stdout)
+			if err == nil {
+				t.Fatal("expected error")
+			}
+			if !strings.Contains(err.Error(), "--json must be a JSON object") {
+				t.Fatalf("err=%v", err)
+			}
+			if !strings.Contains(err.Error(), "lark-base skill") {
+				t.Fatalf("err=%v", err)
+			}
+			if strings.Contains(err.Error(), "array") {
+				t.Fatalf("err should not mention array: %v", err)
+			}
+			if got := stdout.String(); got != "" {
+				t.Fatalf("stdout=%q, want empty", got)
+			}
+		})
+	}
+}
+
 func TestBaseTableExecuteCreate(t *testing.T) {
 	factory, stdout, reg := newExecuteFactory(t)
 	reg.Register(&httpmock.Stub{
@@ -259,7 +340,7 @@ func TestBaseViewExecutePropertyActions(t *testing.T) {
 				"data": []interface{}{map[string]interface{}{"field": "fld_status", "desc": false}},
 			},
 		})
-		if err := runShortcut(t, BaseViewSetGroup, []string{"+view-set-group", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `[{"field":"fld_status","desc":false}]`}, factory, stdout); err != nil {
+		if err := runShortcut(t, BaseViewSetGroup, []string{"+view-set-group", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `{"group_config":[{"field":"fld_status","desc":false}]}`}, factory, stdout); err != nil {
 			t.Fatalf("err=%v", err)
 		}
 		if got := stdout.String(); !strings.Contains(got, `"group"`) || !strings.Contains(got, `"fld_status"`) {
@@ -277,7 +358,7 @@ func TestBaseViewExecutePropertyActions(t *testing.T) {
 				"data": []interface{}{map[string]interface{}{"field": "fld_amount", "desc": true}},
 			},
 		})
-		if err := runShortcut(t, BaseViewSetSort, []string{"+view-set-sort", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `[{"field":"fld_amount","desc":true}]`}, factory, stdout); err != nil {
+		if err := runShortcut(t, BaseViewSetSort, []string{"+view-set-sort", "--base-token", "app_x", "--table-id", "tbl_x", "--view-id", "vew_x", "--json", `{"sort_config":[{"field":"fld_amount","desc":true}]}`}, factory, stdout); err != nil {
 			t.Fatalf("err=%v", err)
 		}
 		if got := stdout.String(); !strings.Contains(got, `"sort"`) || !strings.Contains(got, `"fld_amount"`) {
@@ -1203,7 +1284,7 @@ func TestBaseViewExecuteReadCreateDeleteAndFilter(t *testing.T) {
 			factory,
 			stdout,
 		)
-		if err == nil || !strings.Contains(err.Error(), "invalid JSON object") {
+		if err == nil || !strings.Contains(err.Error(), "--json must be a JSON object") {
 			t.Fatalf("err=%v", err)
 		}
 	})

shortcuts/base/base_shortcut_helpers.go

@@ -63,7 +63,7 @@ func loadJSONInput(pc *parseCtx, raw string, flagName string) (string, error) {
 }
 
 func jsonInputTip(flagName string) string {
-	return fmt.Sprintf("tip: pass a JSON object/array directly, or use --%s @path/to/file.json", flagName)
+	return fmt.Sprintf("tip: pass a valid JSON directly, or use --%s @file.json; use the lark-base skill or this command's reference to find the expected body", flagName)
 }
 
 func formatJSONError(flagName string, target string, err error) error {

shortcuts/base/base_shortcuts_test.go

@@ -120,9 +120,9 @@ func TestWrapViewPropertyBody(t *testing.T) {
 	}
 }
 
-func TestViewSetVisibleFieldsNoValidateHook(t *testing.T) {
-	if BaseViewSetVisibleFields.Validate != nil {
-		t.Fatalf("expected no validate hook, got non-nil")
+func TestViewSetVisibleFieldsValidateHook(t *testing.T) {
+	if BaseViewSetVisibleFields.Validate == nil {
+		t.Fatal("expected validate hook")
 	}
 }
 
@@ -212,8 +212,8 @@ func TestBaseFieldUpdateHelpHidesReadGuideFlag(t *testing.T) {
 
 func TestBaseFieldValidate(t *testing.T) {
 	ctx := context.Background()
-	if err := BaseFieldCreate.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "t", "json": "{"}, nil, nil)); err != nil {
-		t.Fatalf("invalid json should bypass CLI validate, err=%v", err)
+	if err := BaseFieldCreate.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "t", "json": "{"}, nil, nil)); err == nil || !strings.Contains(err.Error(), "--json invalid JSON object") {
+		t.Fatalf("err=%v", err)
 	}
 	if err := BaseFieldCreate.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "t", "json": `{"name":"f1","type":"formula"}`}, nil, nil)); err == nil || !strings.Contains(err.Error(), "--i-have-read-guide is required") {
 		t.Fatalf("err=%v", err)
@@ -255,22 +255,29 @@ func TestBaseRecordValidate(t *testing.T) {
 	if BaseRecordList.Validate != nil {
 		t.Fatalf("record list validate should be nil for repeatable --field-id")
 	}
-	if BaseRecordSearch.Validate != nil {
-		t.Fatalf("record search validate should be nil for API passthrough")
+	if BaseRecordSearch.Validate == nil {
+		t.Fatalf("record search validate should reject invalid JSON before dry-run")
 	}
 	if BaseRecordGet.Validate != nil {
 		t.Fatalf("record get validate should be nil")
 	}
-	if BaseRecordUpsert.Validate != nil {
-		t.Fatalf("record upsert validate should be nil for API passthrough")
+	if BaseRecordUpsert.Validate == nil {
+		t.Fatalf("record upsert validate should reject invalid JSON before dry-run")
 	}
 }
+
 func TestBaseViewValidate(t *testing.T) {
 	ctx := context.Background()
 	if err := BaseViewCreate.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "tbl_1", "json": `{"name":"Main"}`}, nil, nil)); err != nil {
 		t.Fatalf("create validate err=%v", err)
 	}
-	if err := BaseViewSetTimebar.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "tbl_1", "view-id": "Main", "json": "{"}, nil, nil)); err != nil {
-		t.Fatalf("invalid view json should bypass CLI validate, err=%v", err)
+	if err := BaseViewSetGroup.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "tbl_1", "view-id": "Main", "json": `[{"field":"fld_1"}]`}, nil, nil)); err == nil || !strings.Contains(err.Error(), "--json must be a JSON object") {
+		t.Fatalf("err=%v", err)
+	}
+	if err := BaseViewSetSort.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "tbl_1", "view-id": "Main", "json": `[{"field":"fld_1"}]`}, nil, nil)); err == nil || !strings.Contains(err.Error(), "--json must be a JSON object") {
+		t.Fatalf("err=%v", err)
+	}
+	if err := BaseViewSetTimebar.Validate(ctx, newBaseTestRuntime(map[string]string{"base-token": "b", "table-id": "tbl_1", "view-id": "Main", "json": "{"}, nil, nil)); err == nil || !strings.Contains(err.Error(), "--json invalid JSON object") {
+		t.Fatalf("err=%v", err)
 	}
 }

shortcuts/base/field_ops.go

@@ -81,16 +81,7 @@ func dryRunFieldSearchOptions(_ context.Context, runtime *common.RuntimeContext)
 
 func validateFieldJSON(runtime *common.RuntimeContext) (map[string]interface{}, error) {
 	pc := newParseCtx(runtime)
-	raw, _ := loadJSONInput(pc, runtime.Str("json"), "json")
-	if raw == "" {
-		return nil, nil
-	}
-	var body map[string]interface{}
-	_ = common.ParseJSON([]byte(raw), &body)
-	if body == nil {
-		return nil, nil
-	}
-	return body, nil
+	return parseJSONObject(pc, runtime.Str("json"), "json")
 }
 
 func validateFormulaLookupGuideAck(runtime *common.RuntimeContext, command string, body map[string]interface{}) error {

shortcuts/base/helpers.go

@@ -6,6 +6,7 @@ package base
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -36,7 +37,14 @@ func parseJSONObject(pc *parseCtx, raw string, flagName string) (map[string]inte
 	}
 	var result map[string]interface{}
 	if err := common.ParseJSON([]byte(resolved), &result); err != nil {
-		return nil, formatJSONError(flagName, "object", err)
+		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
 }

shortcuts/base/helpers_test.go

@@ -38,7 +38,10 @@ func TestParseHelpers(t *testing.T) {
 	if err != nil || obj["name"] != "demo" {
 		t.Fatalf("obj=%v err=%v", obj, err)
 	}
-	if _, err := parseJSONObject(testPC, `[1]`, "json"); err == nil || !strings.Contains(err.Error(), "invalid JSON object") {
+	if _, err := parseJSONObject(testPC, `[1]`, "json"); err == nil || !strings.Contains(err.Error(), "--json must be a JSON object") || !strings.Contains(err.Error(), "lark-base skill") || strings.Contains(err.Error(), "array") {
+		t.Fatalf("err=%v", err)
+	}
+	if _, err := parseJSONObject(testPC, `null`, "json"); err == nil || !strings.Contains(err.Error(), "--json must be a JSON object") {
 		t.Fatalf("err=%v", err)
 	}
 	obj, err = parseJSONObject(testPC, "@"+tmp.Name(), "json")
@@ -63,7 +66,7 @@ func TestParseHelpers(t *testing.T) {
 	if _, err := parseStringListFlexible(testPC, `[1]`, "fields"); err == nil || !strings.Contains(err.Error(), "invalid JSON string array") {
 		t.Fatalf("err=%v", err)
 	}
-	if _, err := parseJSONValue(testPC, "{", "json"); err == nil || !strings.Contains(err.Error(), "tip: pass a JSON object/array directly") {
+	if _, err := parseJSONValue(testPC, "{", "json"); err == nil || !strings.Contains(err.Error(), "tip: pass a valid JSON directly") || !strings.Contains(err.Error(), "@file.json") || !strings.Contains(err.Error(), "lark-base skill") {
 		t.Fatalf("err=%v", err)
 	}
 	if !reflect.DeepEqual(parseStringList("m,n"), []string{"m", "n"}) {
@@ -281,11 +284,11 @@ func TestJSONInputHelpers(t *testing.T) {
 		t.Fatalf("err=%v", err)
 	}
 	syntaxErr := formatJSONError("json", "object", &json.SyntaxError{Offset: 7})
-	if !strings.Contains(syntaxErr.Error(), "near byte 7") || !strings.Contains(syntaxErr.Error(), "tip: pass a JSON object/array directly") {
+	if !strings.Contains(syntaxErr.Error(), "near byte 7") || !strings.Contains(syntaxErr.Error(), "tip: pass a valid JSON directly") || !strings.Contains(syntaxErr.Error(), "@file.json") || !strings.Contains(syntaxErr.Error(), "lark-base skill") {
 		t.Fatalf("syntaxErr=%v", syntaxErr)
 	}
 	typeErr := formatJSONError("json", "object", &json.UnmarshalTypeError{Field: "filter_info"})
-	if !strings.Contains(typeErr.Error(), `field "filter_info"`) {
+	if !strings.Contains(typeErr.Error(), `field "filter_info"`) || !strings.Contains(typeErr.Error(), "tip: pass a valid JSON directly") || !strings.Contains(typeErr.Error(), "@file.json") || !strings.Contains(typeErr.Error(), "lark-base skill") {
 		t.Fatalf("typeErr=%v", typeErr)
 	}
 }

shortcuts/base/record_batch_create.go

@@ -25,6 +25,9 @@ var BaseRecordBatchCreate = common.Shortcut{
 		`Example: --json '{"fields":["Title","Status"],"rows":[["Task A","Open"],["Task B","Done"]]}'`,
 		"Agent hint: use the lark-base skill's record-batch-create guide for usage and limits.",
 	},
+	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
+		return validateRecordJSON(runtime)
+	},
 	DryRun: dryRunRecordBatchCreate,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		return executeRecordBatchCreate(runtime)

shortcuts/base/record_batch_update.go

@@ -25,6 +25,9 @@ var BaseRecordBatchUpdate = common.Shortcut{
 		`Example: --json '{"record_id_list":["recXXX"],"patch":{"Status":"Done"}}'`,
 		"Agent hint: use the lark-base skill's record-batch-update guide for usage and limits.",
 	},
+	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
+		return validateRecordJSON(runtime)
+	},
 	DryRun: dryRunRecordBatchUpdate,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		return executeRecordBatchUpdate(runtime)

shortcuts/base/record_ops.go

@@ -113,7 +113,9 @@ func dryRunRecordHistoryList(_ context.Context, runtime *common.RuntimeContext)
 }
 
 func validateRecordJSON(runtime *common.RuntimeContext) error {
-	return nil
+	pc := newParseCtx(runtime)
+	_, err := parseJSONObject(pc, runtime.Str("json"), "json")
+	return err
 }
 
 func recordListFields(runtime *common.RuntimeContext) []string {

shortcuts/base/record_search.go

@@ -25,6 +25,9 @@ var BaseRecordSearch = common.Shortcut{
 		`Example: --json '{"keyword":"Alice","search_fields":["Name"]}'`,
 		"Agent hint: use the lark-base skill's record-search guide for usage and limits.",
 	},
+	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
+		return validateRecordJSON(runtime)
+	},
 	DryRun: dryRunRecordSearch,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		return executeRecordSearch(runtime)

shortcuts/base/record_upsert.go

@@ -26,6 +26,9 @@ var BaseRecordUpsert = common.Shortcut{
 		`Example: --json '{"Name":"Alice"}'`,
 		"Agent hint: use the lark-base skill's record-upsert guide for usage and limits.",
 	},
+	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
+		return validateRecordJSON(runtime)
+	},
 	DryRun: dryRunRecordUpsert,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		return executeRecordUpsert(runtime)

shortcuts/base/view_ops.go

@@ -138,15 +138,15 @@ func wrapViewPropertyBody(raw interface{}, key string) interface{} {
 }
 
 func validateViewCreate(runtime *common.RuntimeContext) error {
-	return nil
+	pc := newParseCtx(runtime)
+	_, err := parseObjectList(pc, runtime.Str("json"), "json")
+	return err
 }
 
 func validateViewJSONObject(runtime *common.RuntimeContext) error {
-	return nil
-}
-
-func validateViewJSONValue(runtime *common.RuntimeContext) error {
-	return nil
+	pc := newParseCtx(runtime)
+	_, err := parseJSONObject(pc, runtime.Str("json"), "json")
+	return err
 }
 
 func executeViewList(runtime *common.RuntimeContext) error {

shortcuts/base/view_set_group.go

@@ -27,7 +27,7 @@ var BaseViewSetGroup = common.Shortcut{
 		"Agent hint: use the lark-base skill's view-set-group guide for usage and limits.",
 	},
 	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
-		return validateViewJSONValue(runtime)
+		return validateViewJSONObject(runtime)
 	},
 	DryRun: dryRunViewSetGroup,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {

shortcuts/base/view_set_sort.go

@@ -27,7 +27,7 @@ var BaseViewSetSort = common.Shortcut{
 		"Agent hint: use the lark-base skill's view-set-sort guide for usage and limits.",
 	},
 	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
-		return validateViewJSONValue(runtime)
+		return validateViewJSONObject(runtime)
 	},
 	DryRun: dryRunViewSetSort,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {

shortcuts/base/view_set_visible_fields.go

@@ -26,6 +26,9 @@ var BaseViewSetVisibleFields = common.Shortcut{
 		`Example: --json '{"visible_fields":["fldXXX"]}'`,
 		"Agent hint: use the lark-base skill's view-set-visible-fields guide for usage and limits.",
 	},
+	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
+		return validateViewJSONObject(runtime)
+	},
 	DryRun: dryRunViewSetVisibleFields,
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		return executeViewSetVisibleFields(runtime)