[object Object]

← back to Cli Printing Press

fix(cli): validate generated JSON string flags (#278)

ea0fbe94b398179ad083c9ec16ca6c544dfce0a9 · 2026-04-25 00:26:23 -0700 · Trevin Chow

Files touched

Diff

commit ea0fbe94b398179ad083c9ec16ca6c544dfce0a9
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Sat Apr 25 00:26:23 2026 -0700

    fix(cli): validate generated JSON string flags (#278)
---
 internal/generator/generator.go                    |  84 +++++-
 internal/generator/json_string_validation_test.go  | 317 +++++++++++++++++++++
 .../generator/templates/command_endpoint.go.tmpl   |  42 +++
 .../generator/templates/command_promoted.go.tmpl   |  18 ++
 4 files changed, 459 insertions(+), 2 deletions(-)

diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index b44c33ab..da0c669f 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -247,8 +247,10 @@ func New(s *spec.APISpec, outputDir string) *Generator {
 			}
 			return " (one of: " + strings.Join(values, ", ") + ")"
 		},
-		"envName":  func(s string) string { return strings.ToUpper(strings.ReplaceAll(s, "-", "_")) },
-		"safeName": safeSQLName,
+		"jsonStringParam":    isJSONStringParam,
+		"jsonEnumSuggestion": jsonEnumSuggestion,
+		"envName":            func(s string) string { return strings.ToUpper(strings.ReplaceAll(s, "-", "_")) },
+		"safeName":           safeSQLName,
 		"hasDomainUpsert": func(name string) bool {
 			return domainUpsertMethodName(name) != "UpsertBatch"
 		},
@@ -1891,6 +1893,84 @@ func defaultValForParam(p spec.Param) string {
 	return defaultVal(p)
 }
 
+type jsonFlagSuggestion struct {
+	FlagName string
+	Values   []string
+}
+
+func isJSONStringParam(p spec.Param) bool {
+	if p.Type != "string" {
+		return false
+	}
+
+	format := strings.ToLower(strings.TrimSpace(p.Format))
+	switch format {
+	case "json", "application/json":
+		return true
+	}
+
+	description := strings.TrimSpace(p.Description)
+	if strings.HasPrefix(description, "{") || strings.HasPrefix(description, "[") {
+		return true
+	}
+	lowerDescription := strings.ToLower(description)
+	jsonDescriptionMarkers := []string{
+		"as json",
+		"json:",
+		"json object",
+		"json array",
+		"json value",
+		"valid json",
+		"json-encoded",
+		"json encoded",
+		"json-formatted",
+		"json formatted",
+		"serialized json",
+	}
+	for _, marker := range jsonDescriptionMarkers {
+		if strings.Contains(lowerDescription, marker) {
+			return true
+		}
+	}
+	return false
+}
+
+func jsonEnumSuggestion(p spec.Param, params []spec.Param) *jsonFlagSuggestion {
+	for _, other := range params {
+		if other.Name == p.Name || other.Positional || other.Type != "string" || len(other.Enum) == 0 {
+			continue
+		}
+		if !isRelatedJSONPresetParam(p, other) {
+			continue
+		}
+		return &jsonFlagSuggestion{
+			FlagName: flagName(other.Name),
+			Values:   other.Enum,
+		}
+	}
+	return nil
+}
+
+func isRelatedJSONPresetParam(jsonParam, enumParam spec.Param) bool {
+	jsonText := strings.ToLower(jsonParam.Name + " " + jsonParam.Description)
+	enumText := strings.ToLower(enumParam.Name + " " + enumParam.Description)
+
+	if !strings.Contains(enumText, "preset") {
+		return false
+	}
+
+	return hasTemporalMarker(jsonText) && hasTemporalMarker(enumText)
+}
+
+func hasTemporalMarker(s string) bool {
+	for _, marker := range []string{"time", "date", "range", "window"} {
+		if strings.Contains(s, marker) {
+			return true
+		}
+	}
+	return false
+}
+
 func defaultVal(p spec.Param) string {
 	if p.Default != nil {
 		// Coerce the default value to match the declared param type
diff --git a/internal/generator/json_string_validation_test.go b/internal/generator/json_string_validation_test.go
new file mode 100644
index 00000000..0e3c147a
--- /dev/null
+++ b/internal/generator/json_string_validation_test.go
@@ -0,0 +1,317 @@
+package generator
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/mvanhorn/cli-printing-press/internal/spec"
+	"github.com/stretchr/testify/require"
+)
+
+func TestJSONStringParamEmitsLocalValidation(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("json-param")
+	apiSpec.Resources["insights"] = spec.Resource{
+		Description: "Insights",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/insights",
+				Description: "List insights",
+			},
+			"search": {
+				Method:      "GET",
+				Path:        "/insights/search",
+				Description: "Search insights",
+				Params: []spec.Param{
+					{
+						Name:        "time_range",
+						Type:        "string",
+						Description: "Custom time range as JSON: {'since':'YYYY-MM-DD','until':'YYYY-MM-DD'}",
+					},
+					{
+						Name:        "date_preset",
+						Type:        "string",
+						Description: "Preset date range",
+						Enum:        []string{"today", "last_7d"},
+					},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "json-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "insights_search.go"))
+	require.NoError(t, err)
+	code := string(src)
+
+	require.Contains(t, code, `if cmd.Flags().Changed("time-range") {`)
+	require.Contains(t, code, `var parsedTimeRange any`)
+	require.Contains(t, code, `json.Unmarshal([]byte(flagTimeRange), &parsedTimeRange)`)
+	require.Contains(t, code, `--time-range must be valid JSON. Did you mean --date-preset %s?`)
+	require.Contains(t, code, `--time-range must be valid JSON: %w`)
+}
+
+func TestJSONStringParamDetectionUsesFormatHint(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("json-format-param")
+	apiSpec.Resources["items"] = spec.Resource{
+		Description: "Items",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/items",
+				Description: "List items",
+			},
+			"filter": {
+				Method:      "GET",
+				Path:        "/items/filter",
+				Description: "Filter items",
+				Params: []spec.Param{
+					{Name: "filter", Type: "string", Description: "Filter expression", Format: "json"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "json-format-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "items_filter.go"))
+	require.NoError(t, err)
+	require.Contains(t, string(src), `json.Unmarshal([]byte(flagFilter), &parsedFilter)`)
+}
+
+func TestJSONStringParamDetectionDoesNotOvermatchJSONPathFormat(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("jsonpath-param")
+	apiSpec.Resources["items"] = spec.Resource{
+		Description: "Items",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/items",
+				Description: "List items",
+			},
+			"extract": {
+				Method:      "GET",
+				Path:        "/items/extract",
+				Description: "Extract fields",
+				Params: []spec.Param{
+					{Name: "path", Type: "string", Description: "JSONPath expression", Format: "jsonpath"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "jsonpath-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "items_extract.go"))
+	require.NoError(t, err)
+	require.NotContains(t, string(src), `parsedPath`)
+}
+
+func TestPromotedJSONStringParamEmitsLocalValidation(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("promoted-json-param")
+	apiSpec.Resources["insights"] = spec.Resource{
+		Description: "Insights",
+		Endpoints: map[string]spec.Endpoint{
+			"get": {
+				Method:      "GET",
+				Path:        "/insights",
+				Description: "Get insights",
+				Params: []spec.Param{
+					{Name: "time_range", Type: "string", Description: "Custom time range as JSON"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "promoted-json-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "promoted_insights.go"))
+	require.NoError(t, err)
+	require.Contains(t, string(src), `json.Unmarshal([]byte(flagTimeRange), &parsedTimeRange)`)
+}
+
+func TestJSONStringBodyParamEmitsLocalValidation(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("json-body-param")
+	apiSpec.Resources["items"] = spec.Resource{
+		Description: "Items",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/items",
+				Description: "List items",
+			},
+			"create": {
+				Method:      "POST",
+				Path:        "/items",
+				Description: "Create item",
+				Body: []spec.Param{
+					{Name: "metadata", Type: "string", Description: "Metadata as JSON object"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "json-body-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "items_create.go"))
+	require.NoError(t, err)
+	code := string(src)
+
+	require.Contains(t, code, `json.Unmarshal([]byte(bodyMetadata), &parsedMetadata)`)
+	require.Contains(t, code, `body["metadata"] = bodyMetadata`)
+	require.NotContains(t, code, `body["metadata"] = parsedMetadata`)
+}
+
+func TestJSONStringParamDoesNotSuggestUnrelatedEnum(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("json-unrelated-enum")
+	apiSpec.Resources["items"] = spec.Resource{
+		Description: "Items",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/items",
+				Description: "List items",
+			},
+			"search": {
+				Method:      "GET",
+				Path:        "/items/search",
+				Description: "Search items",
+				Params: []spec.Param{
+					{Name: "filter", Type: "string", Description: "Filter as JSON object"},
+					{Name: "status", Type: "string", Description: "Item status", Enum: []string{"active"}},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "json-unrelated-enum-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "items_search.go"))
+	require.NoError(t, err)
+	code := string(src)
+
+	require.Contains(t, code, `--filter must be valid JSON: %w`)
+	require.NotContains(t, code, `Did you mean --status`)
+}
+
+func TestJSONStringParamRejectsInvalidValueBeforeClient(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("json-runtime-param")
+	apiSpec.Auth = spec.AuthConfig{Type: "none"}
+	apiSpec.Resources["insights"] = spec.Resource{
+		Description: "Insights",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/insights",
+				Description: "List insights",
+			},
+			"search": {
+				Method:      "GET",
+				Path:        "/insights/search",
+				Description: "Search insights",
+				Params: []spec.Param{
+					{Name: "time_range", Type: "string", Description: "Custom time range as JSON"},
+					{Name: "date_preset", Type: "string", Description: "Preset date range", Enum: []string{"last_7d"}},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "json-runtime-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+	runGoCommand(t, outputDir, "mod", "tidy")
+
+	binaryPath := filepath.Join(outputDir, "json-runtime-param-pp-cli")
+	runGoCommand(t, outputDir, "build", "-o", binaryPath, "./cmd/json-runtime-param-pp-cli")
+
+	cmd := exec.Command(binaryPath, "insights", "search", "--time-range", "last_7d")
+	out, err := cmd.CombinedOutput()
+	require.Error(t, err)
+	require.Contains(t, string(out), "--time-range must be valid JSON. Did you mean --date-preset last_7d?")
+}
+
+func TestJSONStringParamSuggestsTemporalPresetWithDifferentMarker(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("json-temporal-preset")
+	apiSpec.Resources["insights"] = spec.Resource{
+		Description: "Insights",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/insights",
+				Description: "List insights",
+			},
+			"search": {
+				Method:      "GET",
+				Path:        "/insights/search",
+				Description: "Search insights",
+				Params: []spec.Param{
+					{Name: "time_range", Type: "string", Description: "Custom time range as JSON"},
+					{Name: "date_preset", Type: "string", Description: "Date preset", Enum: []string{"last_7d"}},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "json-temporal-preset-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "insights_search.go"))
+	require.NoError(t, err)
+	require.Contains(t, string(src), `Did you mean --date-preset %s?`)
+}
+
+func TestPlainStringParamDoesNotEmitJSONValidation(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("plain-string-param")
+	apiSpec.Resources["items"] = spec.Resource{
+		Description: "Items",
+		Endpoints: map[string]spec.Endpoint{
+			"list": {
+				Method:      "GET",
+				Path:        "/items",
+				Description: "List items",
+			},
+			"search": {
+				Method:      "GET",
+				Path:        "/items/search",
+				Description: "Search items",
+				Params: []spec.Param{
+					{Name: "query", Type: "string", Description: "Search query"},
+				},
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "plain-string-param-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	src, err := os.ReadFile(filepath.Join(outputDir, "internal", "cli", "items_search.go"))
+	require.NoError(t, err)
+	require.NotContains(t, string(src), `parsedQuery`)
+}
diff --git a/internal/generator/templates/command_endpoint.go.tmpl b/internal/generator/templates/command_endpoint.go.tmpl
index 602a7398..642d8ab4 100644
--- a/internal/generator/templates/command_endpoint.go.tmpl
+++ b/internal/generator/templates/command_endpoint.go.tmpl
@@ -78,6 +78,24 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 			}
 {{- end}}
 {{- end}}
+{{- range .Endpoint.Params}}
+{{- if and (not .Positional) (jsonStringParam .)}}
+{{- $param := .}}
+			if cmd.Flags().Changed("{{flagName $param.Name}}") {
+				var parsed{{camel $param.Name}} any
+				if err := json.Unmarshal([]byte(flag{{camel $param.Name}}), &parsed{{camel $param.Name}}); err != nil {
+{{- with jsonEnumSuggestion $param $.Endpoint.Params}}
+					for _, v := range []string{ {{enumLiteral .Values}} } {
+						if flag{{camel $param.Name}} == v {
+							return fmt.Errorf("--{{flagName $param.Name}} must be valid JSON. Did you mean --{{.FlagName}} %s?", flag{{camel $param.Name}})
+						}
+					}
+{{- end}}
+					return fmt.Errorf("--{{flagName $param.Name}} must be valid JSON: %w", err)
+				}
+			}
+{{- end}}
+{{- end}}
 {{- if or (eq .Endpoint.Method "POST") (eq .Endpoint.Method "PUT") (eq .Endpoint.Method "PATCH")}}
 			if !stdinBody {
 {{- range .Endpoint.Body}}
@@ -205,6 +223,14 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 					}
 					body["{{.Name}}"] = parsed{{camel .Name}}
 				}
+{{- else if jsonStringParam .}}
+				if body{{camel .Name}} != "" {
+					var parsed{{camel .Name}} any
+					if err := json.Unmarshal([]byte(body{{camel .Name}}), &parsed{{camel .Name}}); err != nil {
+						return fmt.Errorf("parsing --{{flagName .Name}} JSON: %w", err)
+					}
+					body["{{.Name}}"] = body{{camel .Name}}
+				}
 {{- else}}
 				if body{{camel .Name}} != {{zeroVal .Type}} {
 					body["{{.Name}}"] = body{{camel .Name}}
@@ -246,6 +272,14 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 					}
 					body["{{.Name}}"] = parsed{{camel .Name}}
 				}
+{{- else if jsonStringParam .}}
+				if body{{camel .Name}} != "" {
+					var parsed{{camel .Name}} any
+					if err := json.Unmarshal([]byte(body{{camel .Name}}), &parsed{{camel .Name}}); err != nil {
+						return fmt.Errorf("parsing --{{flagName .Name}} JSON: %w", err)
+					}
+					body["{{.Name}}"] = body{{camel .Name}}
+				}
 {{- else}}
 				if body{{camel .Name}} != {{zeroVal .Type}} {
 					body["{{.Name}}"] = body{{camel .Name}}
@@ -281,6 +315,14 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 					}
 					body["{{.Name}}"] = parsed{{camel .Name}}
 				}
+{{- else if jsonStringParam .}}
+				if body{{camel .Name}} != "" {
+					var parsed{{camel .Name}} any
+					if err := json.Unmarshal([]byte(body{{camel .Name}}), &parsed{{camel .Name}}); err != nil {
+						return fmt.Errorf("parsing --{{flagName .Name}} JSON: %w", err)
+					}
+					body["{{.Name}}"] = body{{camel .Name}}
+				}
 {{- else}}
 				if body{{camel .Name}} != {{zeroVal .Type}} {
 					body["{{.Name}}"] = body{{camel .Name}}
diff --git a/internal/generator/templates/command_promoted.go.tmpl b/internal/generator/templates/command_promoted.go.tmpl
index 1c9e1ba7..a392f767 100644
--- a/internal/generator/templates/command_promoted.go.tmpl
+++ b/internal/generator/templates/command_promoted.go.tmpl
@@ -50,6 +50,24 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
 				}
 			}
 {{- end}}
+{{- end}}
+{{- range .Endpoint.Params}}
+{{- if and (not .Positional) (jsonStringParam .)}}
+{{- $param := .}}
+			if cmd.Flags().Changed("{{flagName $param.Name}}") {
+				var parsed{{camel $param.Name}} any
+				if err := json.Unmarshal([]byte(flag{{camel $param.Name}}), &parsed{{camel $param.Name}}); err != nil {
+{{- with jsonEnumSuggestion $param $.Endpoint.Params}}
+					for _, v := range []string{ {{enumLiteral .Values}} } {
+						if flag{{camel $param.Name}} == v {
+							return fmt.Errorf("--{{flagName $param.Name}} must be valid JSON. Did you mean --{{.FlagName}} %s?", flag{{camel $param.Name}})
+						}
+					}
+{{- end}}
+					return fmt.Errorf("--{{flagName $param.Name}} must be valid JSON: %w", err)
+				}
+			}
+{{- end}}
 {{- end}}
 			c, err := flags.newClient()
 			if err != nil {

← 1a95d78b fix(ci): build from local checkout instead of proxying priva  ·  back to Cli Printing Press  ·  fix(cli): treat access-denied sync errors as warnings (#274) 55114c4e →