diff --git a/cmd/service/service.go b/cmd/service/service.go index 125cc584f..ef0c53c7c 100644 --- a/cmd/service/service.go +++ b/cmd/service/service.go @@ -5,8 +5,10 @@ package service import ( "context" + "encoding/json" "fmt" "io" + "strconv" "strings" "github.com/larksuite/cli/errs" @@ -426,11 +428,17 @@ func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, *cmd WithParam(name) } if exists && !util.IsEmptyValue(value) { + if err := validateQueryParamRange(name, value, p, schemaPath); err != nil { + return client.RawApiRequest{}, nil, err + } queryParams[name] = value } } for name, value := range params { if _, ok := queryParams[name]; !ok { + if err := validateQueryParamRange(name, value, nil, schemaPath); err != nil { + return client.RawApiRequest{}, nil, err + } queryParams[name] = value } } @@ -533,3 +541,70 @@ func servicePaginate(ctx context.Context, ac *client.APIClient, request client.R return nil } } + +// paramValidationRules defines per-(schemaPath, paramName) validation rules. +// Only explicitly listed entries are validated; all other parameters pass through. +var paramValidationRules = map[string]map[string]struct { + min int + max int +}{ + "mail.user_mailbox.messages.list": { + "page_size": {min: 1, max: 20}, + }, +} + +// validateQueryParamRange validates a query parameter value against explicit +// per-(schemaPath, paramName) rules. Only allowlisted entries are validated; +// parameters not in the allowlist are silently skipped. +func validateQueryParamRange(name string, value interface{}, _ map[string]interface{}, schemaPath string) error { + paramRules, ok := paramValidationRules[schemaPath] + if !ok { + return nil + } + rule, ok := paramRules[name] + if !ok { + return nil + } + + // Resolve the raw string representation of the value. + var rawStr string + switch v := value.(type) { + case json.Number: + rawStr = v.String() + case string: + rawStr = v + case float64: + rawStr = strconv.FormatFloat(v, 'f', -1, 64) + case float32: + rawStr = strconv.FormatFloat(float64(v), 'f', -1, 32) + case int: + rawStr = strconv.Itoa(v) + case int64: + rawStr = strconv.FormatInt(v, 10) + default: + return nil + } + + // Must parse as an integer (reject 1.5, "abc", etc.). + intVal, err := strconv.Atoi(rawStr) + if err != nil { + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "invalid --params %s: %s is not a valid integer", name, rawStr). + WithHint("The parameter %s must be an integer between %d and %d. Run: lark-cli schema %s", name, rule.min, rule.max, schemaPath). + WithParam(name) + } + + if intVal < rule.min { + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "invalid --params %s: %d is less than the minimum allowed value %d", name, intVal, rule.min). + WithHint("The parameter %s must be at least %d. Run: lark-cli schema %s", name, rule.min, schemaPath). + WithParam(name) + } + if intVal > rule.max { + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "invalid --params %s: %d exceeds the maximum allowed value %d", name, intVal, rule.max). + WithHint("The parameter %s must be at most %d. Run: lark-cli schema %s", name, rule.max, schemaPath). + WithParam(name) + } + return nil +} diff --git a/cmd/service/service_test.go b/cmd/service/service_test.go index 42377850e..fe8ef5016 100644 --- a/cmd/service/service_test.go +++ b/cmd/service/service_test.go @@ -765,3 +765,228 @@ func TestDetectFileFields(t *testing.T) { }) } } + +// ── query parameter range validation (allowlist-based) ── + +const testSchemaPath = "mail.user_mailbox.messages.list" + +func TestValidateQueryParamRange_AllowlistNonTargetSkipped(t *testing.T) { + // Non-target schemaPath: validation is silently skipped. + if err := validateQueryParamRange("page_size", float64(50), nil, "calendar.events.list"); err != nil { + t.Errorf("expected nil for non-target schemaPath, got: %v", err) + } +} + +func TestValidateQueryParamRange_NonTargetParamSkipped(t *testing.T) { + // Target schemaPath but non-allowlisted param name: skipped. + if err := validateQueryParamRange("other_param", float64(9999), nil, testSchemaPath); err != nil { + t.Errorf("expected nil for non-allowlisted param, got: %v", err) + } +} + +func TestValidateQueryParamRange_NilSpec(t *testing.T) { + // paramSpec is now ignored (underscore); nil is fine. + if err := validateQueryParamRange("page_size", float64(50), nil, "other.service.list"); err != nil { + t.Errorf("expected nil for non-allowlisted schemaPath, got: %v", err) + } +} + +func TestValidateQueryParamRange_WithinRange(t *testing.T) { + if err := validateQueryParamRange("page_size", float64(10), nil, testSchemaPath); err != nil { + t.Errorf("expected nil for value within range, got: %v", err) + } +} + +func TestValidateQueryParamRange_ExactlyAtMax(t *testing.T) { + if err := validateQueryParamRange("page_size", float64(20), nil, testSchemaPath); err != nil { + t.Errorf("expected nil for value at max boundary, got: %v", err) + } +} + +func TestValidateQueryParamRange_ExactlyAtMin(t *testing.T) { + if err := validateQueryParamRange("page_size", float64(1), nil, testSchemaPath); err != nil { + t.Errorf("expected nil for value at min boundary, got: %v", err) + } +} + +func TestValidateQueryParamRange_ExceedsMax(t *testing.T) { + err := validateQueryParamRange("page_size", float64(21), nil, testSchemaPath) + if err == nil { + t.Fatal("expected error for page_size exceeding max") + } + if !strings.Contains(err.Error(), "exceeds the maximum") { + t.Errorf("expected 'exceeds the maximum' error, got: %v", err) + } + if !strings.Contains(err.Error(), "20") { + t.Errorf("expected error to mention max value 20, got: %v", err) + } +} + +func TestValidateQueryParamRange_BelowMin(t *testing.T) { + err := validateQueryParamRange("page_size", float64(0), nil, testSchemaPath) + if err == nil { + t.Fatal("expected error for page_size below min") + } + if !strings.Contains(err.Error(), "less than the minimum") { + t.Errorf("expected 'less than the minimum' error, got: %v", err) + } +} + +func TestValidateQueryParamRange_StringIntegerPasses(t *testing.T) { + // String integer "20" should pass. + if err := validateQueryParamRange("page_size", "20", nil, testSchemaPath); err != nil { + t.Errorf("expected nil for string integer \"20\", got: %v", err) + } +} + +func TestValidateQueryParamRange_StringIntegerExceedsMax(t *testing.T) { + err := validateQueryParamRange("page_size", "21", nil, testSchemaPath) + if err == nil { + t.Fatal("expected error for string page_size exceeding max") + } + if !strings.Contains(err.Error(), "exceeds the maximum") { + t.Errorf("expected 'exceeds the maximum' error, got: %v", err) + } +} + +func TestValidateQueryParamRange_FloatRejected(t *testing.T) { + // 1.5 is not an integer and must be rejected. + err := validateQueryParamRange("page_size", 1.5, nil, testSchemaPath) + if err == nil { + t.Fatal("expected error for float page_size") + } + if !strings.Contains(err.Error(), "not a valid integer") { + t.Errorf("expected 'not a valid integer' error, got: %v", err) + } +} + +func TestValidateQueryParamRange_StringFloatRejected(t *testing.T) { + // "1.5" is not an integer and must be rejected. + err := validateQueryParamRange("page_size", "1.5", nil, testSchemaPath) + if err == nil { + t.Fatal("expected error for string float page_size") + } + if !strings.Contains(err.Error(), "not a valid integer") { + t.Errorf("expected 'not a valid integer' error, got: %v", err) + } +} + +func TestValidateQueryParamRange_StringNonNumericRejected(t *testing.T) { + // "abc" is not a valid integer. + err := validateQueryParamRange("page_size", "abc", nil, testSchemaPath) + if err == nil { + t.Fatal("expected error for non-numeric string page_size") + } + if !strings.Contains(err.Error(), "not a valid integer") { + t.Errorf("expected 'not a valid integer' error, got: %v", err) + } +} + +func TestValidateQueryParamRange_IntTypePasses(t *testing.T) { + if err := validateQueryParamRange("page_size", 10, nil, testSchemaPath); err != nil { + t.Errorf("expected nil for int value within range, got: %v", err) + } +} + +func TestValidateQueryParamRange_NonTargetServiceNotBlocked(t *testing.T) { + // A parameter with min/max in metadata from a non-target service command + // must NOT be blocked — proves the allowlist is effective. + if err := validateQueryParamRange("limit", float64(200), nil, "calendar.events.list"); err != nil { + t.Errorf("expected nil for non-target service command, got: %v", err) + } +} + +func TestServiceMethod_PageSizeExceedsMax(t *testing.T) { + spec := map[string]interface{}{ + "name": "mail", "servicePath": "/open-apis/mail/v1", + } + method := map[string]interface{}{ + "path": "user_mailboxes/{user_mailbox_id}/messages", + "httpMethod": "GET", + "parameters": map[string]interface{}{ + "user_mailbox_id": map[string]interface{}{ + "type": "string", "location": "path", "required": true, + }, + "page_size": map[string]interface{}{ + "type": "integer", "location": "query", "required": true, + }, + }, + } + f, _, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, spec, method, "list", "user_mailbox.messages", nil) + cmd.SetArgs([]string{"--params", `{"user_mailbox_id":"me","page_size":21}`, "--dry-run"}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected error for page_size exceeding max") + } + if !strings.Contains(err.Error(), "exceeds the maximum") { + t.Errorf("expected 'exceeds the maximum' error, got: %v", err) + } + if !strings.Contains(err.Error(), "20") { + t.Errorf("expected error to mention max value 20, got: %v", err) + } +} + +func TestServiceMethod_PageSizeWithinMax(t *testing.T) { + spec := map[string]interface{}{ + "name": "mail", "servicePath": "/open-apis/mail/v1", + } + method := map[string]interface{}{ + "path": "user_mailboxes/{user_mailbox_id}/messages", + "httpMethod": "GET", + "parameters": map[string]interface{}{ + "user_mailbox_id": map[string]interface{}{ + "type": "string", "location": "path", "required": true, + }, + "page_size": map[string]interface{}{ + "type": "integer", "location": "query", "required": false, + }, + }, + } + f, stdout, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, spec, method, "list", "user_mailbox.messages", nil) + cmd.SetArgs([]string{"--params", `{"user_mailbox_id":"me","page_size":20}`, "--dry-run"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("expected no error for valid page_size, got: %v", err) + } + if !strings.Contains(stdout.String(), "Dry Run") { + t.Error("expected dry-run output") + } +} + +func TestServiceMethod_PageAllSkipsRequiredCheck(t *testing.T) { + // --page-all should skip required check for page_size and page_token. + spec := map[string]interface{}{ + "name": "mail", "servicePath": "/open-apis/mail/v1", + } + method := map[string]interface{}{ + "path": "user_mailboxes/{user_mailbox_id}/messages", + "httpMethod": "GET", + "parameters": map[string]interface{}{ + "user_mailbox_id": map[string]interface{}{ + "type": "string", "location": "path", "required": true, + }, + "page_size": map[string]interface{}{ + "type": "integer", "location": "query", "required": true, + }, + "page_token": map[string]interface{}{ + "type": "string", "location": "query", "required": true, + }, + }, + } + f, stdout, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, spec, method, "list", "user_mailbox.messages", nil) + // --page-all without providing page_size or page_token should not fail required check. + cmd.SetArgs([]string{"--params", `{"user_mailbox_id":"me"}`, "--page-all", "--page-limit", "1", "--dry-run"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("expected no error for --page-all skipping required params, got: %v", err) + } + if !strings.Contains(stdout.String(), "Dry Run") { + t.Error("expected dry-run output") + } +}