From 158466f1770d3a25c817fc91a1203779498f0666 Mon Sep 17 00:00:00 2001 From: samzong Date: Thu, 25 Jun 2026 23:33:22 -0400 Subject: [PATCH] fix(runtime): validate body-backed required inputs Signed-off-by: samzong --- pkg/runtime/body.go | 2 + pkg/runtime/body_test.go | 21 ++++++ pkg/runtime/build.go | 36 +++++++++- pkg/runtime/build_test.go | 141 ++++++++++++++++++++++++++++++-------- 4 files changed, 170 insertions(+), 30 deletions(-) diff --git a/pkg/runtime/body.go b/pkg/runtime/body.go index c9d680e..8e1d5dc 100644 --- a/pkg/runtime/body.go +++ b/pkg/runtime/body.go @@ -234,6 +234,8 @@ func inferValue(s string) any { return false case "null": return nil + case "[]": + return []any{} } if i, err := strconv.ParseInt(s, 10, 64); err == nil { return i diff --git a/pkg/runtime/body_test.go b/pkg/runtime/body_test.go index dc183e3..452c65c 100644 --- a/pkg/runtime/body_test.go +++ b/pkg/runtime/body_test.go @@ -69,6 +69,22 @@ func TestBuildEnvelopeBody(t *testing.T) { } }) + t.Run("merges empty array literal under merge path", func(t *testing.T) { + raw, err := buildEnvelopeBody(tmpl, "variables", nil, []string{"input.skillIds=[]"}, nil, nil, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + var got map[string]any + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + vars, _ := got["variables"].(map[string]any) + input, _ := vars["input"].(map[string]any) + if !reflect.DeepEqual(input["skillIds"], []any{}) { + t.Errorf("skillIds = %#v, want empty array", input["skillIds"]) + } + }) + t.Run("merges typed variable values at merge path", func(t *testing.T) { raw, err := buildEnvelopeBody(tmpl, "variables", map[string]any{"name": "demo", "count": int64(3)}, nil, nil, nil, false) if err != nil { @@ -162,6 +178,11 @@ func TestBuildBodyFromSet_ArrayIndex(t *testing.T) { in: []string{"items[0]=a", "items[1]=b"}, want: map[string]any{"items": []any{"a", "b"}}, }, + { + name: "empty array literal", + in: []string{"items=[]"}, + want: map[string]any{"items": []any{}}, + }, { name: "array with type inference", in: []string{"ids[0]=1", "ids[1]=2"}, diff --git a/pkg/runtime/build.go b/pkg/runtime/build.go index 359e158..c6c68e4 100644 --- a/pkg/runtime/build.go +++ b/pkg/runtime/build.go @@ -1,6 +1,7 @@ package runtime import ( + "encoding/json" "fmt" "net/url" "os" @@ -237,6 +238,9 @@ func buildCmd(s CommandSpec) *cobra.Command { return fmt.Errorf("request body required: pass --file, --set, or --set-str") } } + if err := validateRequiredVariableParams(s, body); err != nil { + return err + } if v, err := cmd.Root().PersistentFlags().GetBool("debug"); err == nil && v { clientOpts.Debug = true @@ -337,7 +341,7 @@ func buildCmd(s CommandSpec) *cobra.Command { vals[p.Name] = v cmd.Flags().StringVar(v, p.Flag, p.Default, p.Help) } - if p.Required && p.Default == "" { + if p.Required && p.Default == "" && (p.In != InVariable || s.RequestBody == nil) { _ = cmd.MarkFlagRequired(p.Flag) } if p.Deprecated { @@ -461,6 +465,36 @@ func shortcutSpec(spec CommandSpec, shortcut CommandShortcut) (CommandSpec, erro return target, nil } +func validateRequiredVariableParams(s CommandSpec, body any) error { + if s.RequestBody == nil { + return nil + } + required := make([]ParamSpec, 0) + for _, p := range s.Params { + if p.In == InVariable && p.Required && p.Default == "" { + required = append(required, p) + } + } + if len(required) == 0 { + return nil + } + raw, ok := body.([]byte) + if !ok || len(raw) == 0 { + return fmt.Errorf("required body field missing: %s", required[0].Name) + } + var doc any + if err := json.Unmarshal(raw, &doc); err != nil { + return fmt.Errorf("validate request body: %w", err) + } + for _, p := range required { + v, ok := getNestedPath(doc, joinBodyPath(s.RequestBody.MergePath, p.Name)) + if !ok || v == nil { + return fmt.Errorf("required body field missing: %s", p.Name) + } + } + return nil +} + func shortcutName(use string, target string) (string, error) { name := strings.TrimSpace(use) if name == "" || name != use || len(strings.Fields(name)) != 1 { diff --git a/pkg/runtime/build_test.go b/pkg/runtime/build_test.go index be98886..5c6a1b9 100644 --- a/pkg/runtime/build_test.go +++ b/pkg/runtime/build_test.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/http/httptest" + "os" "reflect" "strings" "testing" @@ -243,18 +244,100 @@ func TestBuild_SetStrSendsStringBodyFields(t *testing.T) { } func TestBuild_VariableFlagsMergeIntoEnvelope(t *testing.T) { - bindTestManifest(t, "myctl", "MYCTL_HOST") - t.Setenv("MYCTL_CONFIG_DIR", t.TempDir()) + root, url, recorded := newRecordingGraphQLRoot(t, createAppSpec()) + root.SetArgs([]string{"--hostname", url, "demo", "apps", "create-app", "--input-name", "demo"}) + if err := root.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } - var rawBody []byte - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - rawBody, _ = io.ReadAll(r.Body) - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{}`)) - })) - defer srv.Close() + rawBody, _ := recorded() + var got map[string]any + if err := json.Unmarshal(rawBody, &got); err != nil { + t.Fatalf("invalid request JSON %q: %v", string(rawBody), err) + } + if q, _ := got["query"].(string); !strings.Contains(q, "mutation createApp") { + t.Errorf("query missing baked document: %#v", got["query"]) + } + vars, _ := got["variables"].(map[string]any) + input, _ := vars["input"].(map[string]any) + if input["name"] != "demo" { + t.Errorf("variables = %#v, want input.name=demo", got["variables"]) + } +} - specs := []CommandSpec{{ +func TestBuild_RequiredVariableCanComeFromBodyInput(t *testing.T) { + cases := []struct { + name string + args []string + fileBody string + wantName string + wantErr bool + }{ + { + name: "file", + args: []string{"--file", "BODY_FILE"}, + fileBody: `{"input":{"name":"from-file"}}`, + wantName: "from-file", + }, + { + name: "set", + args: []string{"--set", "input.name=from-set"}, + wantName: "from-set", + }, + { + name: "missing", + wantErr: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root, url, recorded := newRecordingGraphQLRoot(t, createAppSpec()) + args := append([]string{"--hostname", url, "demo", "apps", "create-app"}, tc.args...) + if tc.fileBody != "" { + bodyFile := t.TempDir() + "/body.json" + if err := os.WriteFile(bodyFile, []byte(tc.fileBody), 0600); err != nil { + t.Fatalf("write body file: %v", err) + } + for i := range args { + if args[i] == "BODY_FILE" { + args[i] = bodyFile + } + } + } + root.SetArgs(args) + err := root.Execute() + if tc.wantErr { + if err == nil { + t.Fatal("expected error") + } + _, called := recorded() + if called { + t.Fatal("request should not be sent") + } + return + } + if err != nil { + t.Fatalf("Execute: %v", err) + } + rawBody, called := recorded() + if !called { + t.Fatal("request was not sent") + } + var got map[string]any + if err := json.Unmarshal(rawBody, &got); err != nil { + t.Fatalf("invalid request JSON %q: %v", string(rawBody), err) + } + vars, _ := got["variables"].(map[string]any) + input, _ := vars["input"].(map[string]any) + if input["name"] != tc.wantName { + t.Errorf("variables = %#v, want input.name=%s", got["variables"], tc.wantName) + } + }) + } +} + +func createAppSpec() CommandSpec { + return CommandSpec{ Group: "Apps", Use: "create-app", Method: "POST", @@ -269,29 +352,29 @@ func TestBuild_VariableFlagsMergeIntoEnvelope(t *testing.T) { MergePath: "variables", }, Security: &SecurityHint{Public: true}, - }} + } +} + +func newRecordingGraphQLRoot(t *testing.T, spec CommandSpec) (*cobra.Command, string, func() ([]byte, bool)) { + t.Helper() + bindTestManifest(t, "myctl", "MYCTL_HOST") + t.Setenv("MYCTL_CONFIG_DIR", t.TempDir()) + + var rawBody []byte + called := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + rawBody, _ = io.ReadAll(r.Body) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{}`)) + })) + t.Cleanup(srv.Close) root := newRootWithModuleGroup() root.PersistentFlags().String("hostname", "", "") root.PersistentFlags().StringP("output", "o", "raw", "") - Build(root, "demo", specs) - root.SetArgs([]string{"--hostname", srv.URL, "demo", "apps", "create-app", "--input-name", "demo"}) - if err := root.Execute(); err != nil { - t.Fatalf("Execute: %v", err) - } - - var got map[string]any - if err := json.Unmarshal(rawBody, &got); err != nil { - t.Fatalf("invalid request JSON %q: %v", string(rawBody), err) - } - if q, _ := got["query"].(string); !strings.Contains(q, "mutation createApp") { - t.Errorf("query missing baked document: %#v", got["query"]) - } - vars, _ := got["variables"].(map[string]any) - input, _ := vars["input"].(map[string]any) - if input["name"] != "demo" { - t.Errorf("variables = %#v, want input.name=demo", got["variables"]) - } + Build(root, "demo", []CommandSpec{spec}) + return root, srv.URL, func() ([]byte, bool) { return rawBody, called } } func TestBuild_FloatVariableSentAsJSONNumber(t *testing.T) {