diff --git a/README.md b/README.md index c30e81b..596f26e 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,17 @@ Top-level non-resource commands: - `-o table|json|yaml` (default `auto`: table on TTY, JSON when piped). - Errors are emitted as JSON on stderr with shape `{error, message, status, response, request_id}`. Stdout stays clean for piping. +### Request validation + +JSON payloads supplied with `--data`, `--file`, or `--file -` are checked +against the embedded OpenAPI request schema before the API call (including +`--all` and `--dry-run`). Invalid JSON, missing required fields, wrong types, +and enum/constraint mismatches return usage exit code 2 without sending. +Unknown fields in structured objects produce warnings on stderr but are +sent unchanged to tolerate schema drift. Explicitly open and typed maps +retain their schema semantics. Payload values are not printed in validation +errors or warnings. Use `--describe` to inspect the expected schema. + ### Exit codes | Code | Meaning | diff --git a/cmd/twoctl/cli/extra_test.go b/cmd/twoctl/cli/extra_test.go index 5319829..78c2386 100644 --- a/cmd/twoctl/cli/extra_test.go +++ b/cmd/twoctl/cli/extra_test.go @@ -95,8 +95,13 @@ func TestRunOperationWithRequestBody(t *testing.T) { })) defer srv.Close() - stdout, _, err := runRoot(t, "--url", srv.URL, "--api-key", "secret_test_x", - "order", "create", "--data", `{"foo":"bar"}`) + flagURL, flagAPIKey = srv.URL, "secret_test_x" + cmd := buildOperationCommand("test", "/test", "POST", payloadTestOperation(t, `{"type":"object","properties":{"foo":{"type":"string"}}}`)) + var output strings.Builder + cmd.SetOut(&output) + cmd.SetArgs([]string{"--data", `{"foo":"bar"}`}) + err := cmd.Execute() + stdout := output.String() if err != nil { t.Fatal(err) } diff --git a/cmd/twoctl/cli/operations.go b/cmd/twoctl/cli/operations.go index 2386000..2057652 100644 --- a/cmd/twoctl/cli/operations.go +++ b/cmd/twoctl/cli/operations.go @@ -349,7 +349,7 @@ func runOperation(c *cobra.Command, method, path string, op *openapi3.Operation, if err != nil { return err } - body, err := readRequestBody(b, headers) + body, err := readValidatedRequestBody(c.ErrOrStderr(), op, b, headers) if err != nil { return err } diff --git a/cmd/twoctl/cli/paginate.go b/cmd/twoctl/cli/paginate.go index 098291e..23e5dd7 100644 --- a/cmd/twoctl/cli/paginate.go +++ b/cmd/twoctl/cli/paginate.go @@ -77,6 +77,10 @@ func runPaginated(c *cobra.Command, method, path string, op *openapi3.Operation, } } + if err := validatePayload(c.ErrOrStderr(), op, bodyBytes); err != nil { + return err + } + combined, err = fetchAllPages(c, client, resolved.BaseURL, method, path, op, b, bodyBytes, cursorPtr) if err != nil { return err diff --git a/cmd/twoctl/cli/validation.go b/cmd/twoctl/cli/validation.go new file mode 100644 index 0000000..92af35f --- /dev/null +++ b/cmd/twoctl/cli/validation.go @@ -0,0 +1,187 @@ +package cli + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "sort" + "strings" + + "github.com/getkin/kin-openapi/openapi3" +) + +// validatePayload checks a private payload projection. Shared embedded schemas +// and the original bytes sent to the API are never modified. +func validatePayload(w io.Writer, op *openapi3.Operation, raw []byte) error { + if len(raw) == 0 { + if op.RequestBody != nil && op.RequestBody.Value != nil && op.RequestBody.Value.Required { + return payloadError("request body is required") + } + return nil + } + var value any + if err := json.Unmarshal(raw, &value); err != nil { + return payloadError("request body must contain one valid JSON value") + } + if op.RequestBody == nil || op.RequestBody.Value == nil { + return nil + } + mt := op.RequestBody.Value.Content.Get("application/json") + if mt == nil || mt.Schema == nil || mt.Schema.Value == nil { + return nil + } + schema := mt.Schema.Value + if err := schema.VisitJSON(pruneUnknownPayload(schema, value), openapi3.VisitAsRequest()); err != nil { + // Do not render SchemaError.Error(): it includes the submitted payload. + var schemaErr *openapi3.SchemaError + if errors.As(err, &schemaErr) { + return payloadError(fmt.Sprintf("request body violates %s at %q; use --describe to inspect the schema", + schemaErr.SchemaField, "/"+strings.Join(schemaErr.JSONPointer(), "/"))) + } + return payloadError("request body does not match the operation schema; use --describe") + } + warnings := map[string]bool{} + unknownPayloadFields(schema, value, "", warnings) + paths := make([]string, 0, len(warnings)) + for path := range warnings { + paths = append(paths, path) + } + sort.Strings(paths) + for _, path := range paths { + fmt.Fprintf(w, "warning: unknown request property at %q; sending unchanged (the API schema may have drifted)\n", path) + } + return nil +} + +func payloadError(message string) error { + return &apiError{code: "invalid_request_body", message: message, exit: ExitUsage} +} + +func unknownPayloadFields(s *openapi3.Schema, value any, path string, warnings map[string]bool) { + schemas := allPayloadSchemas(s, map[*openapi3.Schema]bool{}) + switch v := value.(type) { + case map[string]any: + unknownObjectFields(schemas, v, path, warnings) + case []any: + items := payloadItemSchema(schemas) + for i, item := range v { + unknownPayloadFields(items, item, fmt.Sprintf("%s/%d", path, i), warnings) + } + } +} + +func unknownObjectFields(schemas []*openapi3.Schema, value map[string]any, path string, warnings map[string]bool) { + shape := payloadObjectShape(schemas) + for key, item := range value { + childPath := path + "/" + strings.ReplaceAll(strings.ReplaceAll(key, "~", "~0"), "/", "~1") + if ref := shape.props[key]; ref != nil { + unknownPayloadFields(ref.Value, item, childPath, warnings) + } else if shape.open { + unknownPayloadFields(shape.extra, item, childPath, warnings) + } else if shape.declared { + warnings[childPath] = true + } + } +} + +func readValidatedRequestBody(w io.Writer, op *openapi3.Operation, b *opBindings, headers http.Header) (io.Reader, error) { + if b.bodyFile == nil || b.bodyData == nil { + return nil, nil + } + raw, err := readBody(*b.bodyFile, *b.bodyData) + if err != nil { + return nil, err + } + if err := validatePayload(w, op, raw); err != nil { + return nil, err + } + if len(raw) == 0 { + return nil, nil + } + headers.Set("Content-Type", "application/json") + return bytes.NewReader(raw), nil +} + +func allPayloadSchemas(s *openapi3.Schema, seen map[*openapi3.Schema]bool) []*openapi3.Schema { + if s == nil || seen[s] { + return nil + } + seen[s] = true + out := []*openapi3.Schema{s} + refs := append(append(append(openapi3.SchemaRefs{}, s.AllOf...), s.AnyOf...), s.OneOf...) + for _, ref := range refs { + if ref != nil { + out = append(out, allPayloadSchemas(ref.Value, seen)...) + } + } + return out +} + +func pruneUnknownPayload(s *openapi3.Schema, value any) any { + schemas := allPayloadSchemas(s, map[*openapi3.Schema]bool{}) + switch v := value.(type) { + case map[string]any: + return pruneUnknownObject(schemas, v) + case []any: + items := payloadItemSchema(schemas) + out := make([]any, len(v)) + for i, item := range v { + out[i] = pruneUnknownPayload(items, item) + } + return out + default: + return value + } +} + +type payloadShape struct { + props openapi3.Schemas + extra *openapi3.Schema + open, declared bool +} + +func payloadObjectShape(schemas []*openapi3.Schema) payloadShape { + shape := payloadShape{props: openapi3.Schemas{}, extra: &openapi3.Schema{}} + for _, schema := range schemas { + if schema.AdditionalProperties.Schema != nil { + shape.extra.AllOf = append(shape.extra.AllOf, schema.AdditionalProperties.Schema) + } + shape.open = shape.open || schema.AdditionalProperties.Schema != nil || + (schema.AdditionalProperties.Has != nil && *schema.AdditionalProperties.Has) + shape.declared = shape.declared || len(schema.Properties) > 0 || + (schema.AdditionalProperties.Has != nil && !*schema.AdditionalProperties.Has) + for key, ref := range schema.Properties { + if shape.props[key] == nil { + shape.props[key] = &openapi3.SchemaRef{Value: &openapi3.Schema{}} + } + shape.props[key].Value.AllOf = append(shape.props[key].Value.AllOf, ref) + } + } + return shape +} + +func pruneUnknownObject(schemas []*openapi3.Schema, value map[string]any) map[string]any { + shape := payloadObjectShape(schemas) + out := map[string]any{} + for key, item := range value { + if ref := shape.props[key]; ref != nil { + out[key] = pruneUnknownPayload(ref.Value, item) + } else if shape.open || !shape.declared { + out[key] = pruneUnknownPayload(shape.extra, item) + } + } + return out +} + +func payloadItemSchema(schemas []*openapi3.Schema) *openapi3.Schema { + items := &openapi3.Schema{} + for _, schema := range schemas { + if schema.Items != nil { + items.AllOf = append(items.AllOf, schema.Items) + } + } + return items +} diff --git a/cmd/twoctl/cli/validation_test.go b/cmd/twoctl/cli/validation_test.go new file mode 100644 index 0000000..00f7216 --- /dev/null +++ b/cmd/twoctl/cli/validation_test.go @@ -0,0 +1,265 @@ +package cli + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/getkin/kin-openapi/openapi3" +) + +func payloadTestOperation(t *testing.T, schema string) *openapi3.Operation { + t.Helper() + doc, err := openapi3.NewLoader().LoadFromData([]byte(`{"openapi":"3.0.3","info":{"title":"test","version":"1"},"paths":{"/test":{"post":{"requestBody":{"content":{"application/json":{"schema":` + schema + `}}},"responses":{"200":{"description":"ok"}}}}}}`)) + if err != nil { + t.Fatal(err) + } + return doc.Paths.Value("/test").Post +} + +const payloadTestSchema = `{"type":"object","required":["amount","currency"],"additionalProperties":false,"properties":{"amount":{"type":"integer","minimum":1},"currency":{"type":"string","enum":["GBP"]},"lines":{"type":"array","items":{"type":"object","required":["name"],"additionalProperties":false,"properties":{"name":{"type":"string"}}}}}}` + +func TestPayloadValidation(t *testing.T) { + op := payloadTestOperation(t, payloadTestSchema) + for _, tc := range []struct { + name, body, warning string + invalid bool + }{ + {"valid", `{"amount":1,"currency":"GBP"}`, "", false}, + {"unknown", `{"amount":1,"currency":"GBP","extra":"secret"}`, "/extra", false}, + {"nested unknown", `{"amount":1,"currency":"GBP","lines":[{"name":"ok","extra":"secret"}]}`, "/lines/0/extra", false}, + {"missing", `{"amount_typo":1,"currency":"GBP"}`, "", true}, + {"enum", `{"amount":1,"currency":"secret"}`, "", true}, + {"type", `{"amount":"secret","currency":"GBP"}`, "", true}, + {"minimum", `{"amount":0,"currency":"GBP"}`, "", true}, + {"nested required", `{"amount":1,"currency":"GBP","lines":[{}]}`, "", true}, + {"malformed", `{"secret":`, "", true}, + {"multiple values", `{} {}`, "", true}, + {"null", `null`, "", true}, + } { + t.Run(tc.name, func(t *testing.T) { + var warnings bytes.Buffer + err := validatePayload(&warnings, op, []byte(tc.body)) + if (err != nil) != tc.invalid { + t.Fatalf("error = %v", err) + } + if err != nil { + ae, ok := err.(*apiError) + if !ok || ae.exit != ExitUsage || ae.code != "invalid_request_body" { + t.Fatalf("unexpected error: %v", err) + } + if strings.Contains(err.Error(), "secret") { + t.Fatal("payload leaked in error") + } + } + if tc.warning != "" && !strings.Contains(warnings.String(), tc.warning) { + t.Fatalf("missing warning: %s", warnings.String()) + } + if tc.warning == "" && warnings.Len() != 0 { + t.Fatalf("unexpected warning: %s", warnings.String()) + } + if strings.Contains(warnings.String(), "secret") { + t.Fatal("payload value leaked in warning") + } + }) + } + schema := op.RequestBody.Value.Content.Get("application/json").Schema.Value + if schema.AdditionalProperties.Has == nil || *schema.AdditionalProperties.Has { + t.Fatal("shared schema was changed") + } +} + +func TestPayloadCompositionsAndMaps(t *testing.T) { + for _, tc := range []struct { + name, schema, body string + warn, invalid bool + }{ + {"allOf", `{"allOf":[{"type":"object","properties":{"a":{"type":"string"}}},{"type":"object","properties":{"b":{"type":"integer"}}}]}`, `{"a":"x","b":1}`, false, false}, + {"oneOf", `{"oneOf":[{"type":"object","required":["kind"],"properties":{"kind":{"enum":["a"]}}},{"type":"object","required":["kind"],"properties":{"kind":{"enum":["b"]}}}]}`, `{"kind":"b","extra":1}`, true, false}, + {"anyOf", `{"anyOf":[{"type":"string"},{"type":"object","properties":{"a":{"type":"string"}}}]}`, `{"a":"x","extra":1}`, true, false}, + {"open map", `{"type":"object","additionalProperties":true,"properties":{"a":{"type":"string"}}}`, `{"extra":1}`, false, false}, + {"typed map", `{"type":"object","additionalProperties":{"type":"integer"}}`, `{"a":1}`, false, false}, + {"invalid map", `{"type":"object","additionalProperties":{"type":"integer"}}`, `{"a":"x"}`, false, true}, + {"free form", `{"type":"object"}`, `{"extra":1}`, false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + var warnings bytes.Buffer + err := validatePayload(&warnings, payloadTestOperation(t, tc.schema), []byte(tc.body)) + if (err != nil) != tc.invalid || (warnings.Len() != 0) != tc.warn { + t.Fatalf("error=%v warnings=%s", err, warnings.String()) + } + }) + } +} + +func TestPayloadValidationBeforeHTTP(t *testing.T) { + for _, source := range []string{"data", "file", "stdin"} { + for _, invalid := range []bool{false, true} { + t.Run(source+map[bool]string{true: "-invalid", false: "-valid"}[invalid], func(t *testing.T) { + resetGlobals(t) + body := `{"amount":1,"currency":"GBP","extra":"kept"}` + if invalid { + body = `{"amount":1,"currency":"wrong"}` + } + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + raw, _ := io.ReadAll(r.Body) + if string(raw) != body { + t.Errorf("body changed: %s", raw) + } + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer srv.Close() + flagURL, flagAPIKey = srv.URL, "secret_test_mock" + cmd := buildOperationCommand("test", "/test", "POST", payloadTestOperation(t, payloadTestSchema)) + args := payloadSourceArgs(t, source, body) + cmd.SilenceUsage, cmd.SilenceErrors = true, true + var stdout, stderr bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + cmd.SetArgs(args) + err := cmd.Execute() + if invalid { + if err == nil || calls != 0 || stdout.Len() != 0 { + t.Fatalf("invalid payload sent: calls=%d err=%v output=%s", calls, err, stdout.String()) + } + } else if err != nil || calls != 1 || !strings.Contains(stderr.String(), "/extra") || strings.Contains(stdout.String(), "warning") { + t.Fatalf("calls=%d err=%v stdout=%s stderr=%s", calls, err, stdout.String(), stderr.String()) + } + }) + } + } +} + +func TestPayloadValidationPagination(t *testing.T) { + resetGlobals(t) + op := payloadTestOperation(t, payloadTestSchema) + op.Parameters = openapi3.Parameters{{Value: &openapi3.Parameter{Name: "page_cursor", In: "query", Schema: &openapi3.SchemaRef{Value: openapi3.NewStringSchema()}}}} + calls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + _, _ = w.Write([]byte(`{"items":[]}`)) + })) + defer srv.Close() + flagURL, flagAPIKey = srv.URL, "secret_test_mock" + cmd := buildOperationCommand("test", "/test", "POST", op) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{"--all", "--data", `{"amount":1}`}) + if err := cmd.Execute(); err == nil || calls != 0 { + t.Fatalf("invalid paginated body sent: calls=%d err=%v", calls, err) + } +} + +func TestPayloadProjectionCycle(t *testing.T) { + s := openapi3.NewObjectSchema() + s.Properties = openapi3.Schemas{"child": {Value: s}} + original := map[string]any{"child": map[string]any{"extra": "kept"}} + projected := pruneUnknownPayload(s, original).(map[string]any) + if len(projected["child"].(map[string]any)) != 0 { + t.Fatal("nested unknown field was not removed from validation projection") + } + if original["child"].(map[string]any)["extra"] != "kept" { + t.Fatal("original payload changed") + } + if err := validatePayload(io.Discard, &openapi3.Operation{}, nil); err != nil { + t.Fatal(err) + } + if err := validatePayload(io.Discard, &openapi3.Operation{}, []byte(`{}`)); err != nil { + t.Fatal(err) + } +} + +func payloadSourceArgs(t *testing.T, source, body string) []string { + t.Helper() + if source == "data" { + return []string{"--data", body} + } + file := filepath.Join(t.TempDir(), "payload.json") + if err := os.WriteFile(file, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + if source == "file" { + return []string{"--file", file} + } + f, err := os.Open(file) + if err != nil { + t.Fatal(err) + } + old := os.Stdin + os.Stdin = f + t.Cleanup(func() { + os.Stdin = old + f.Close() + }) + return []string{"--file", "-"} +} + +func TestRequiredEmptyPayload(t *testing.T) { + op := payloadTestOperation(t, payloadTestSchema) + op.RequestBody.Value.Required = true + if err := validatePayload(io.Discard, op, nil); err == nil { + t.Fatal("required empty body accepted") + } +} + +func TestPayloadDryRunValidation(t *testing.T) { + resetGlobals(t) + cmd := buildOperationCommand("test", "/test", "POST", payloadTestOperation(t, payloadTestSchema)) + cmd.SilenceUsage, cmd.SilenceErrors = true, true + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + cmd.SetArgs([]string{"--dry-run", "--data", `{"amount":1}`}) + if err := cmd.Execute(); err == nil { + t.Fatal("invalid dry-run body accepted") + } +} + +func TestPayloadClosedOneOf(t *testing.T) { + op := payloadTestOperation(t, `{"oneOf":[{"type":"object","additionalProperties":false,"properties":{"a":{"type":"string"}}},{"type":"object","additionalProperties":false,"properties":{"b":{"type":"string"}}}]}`) + for _, body := range []string{`{"a":"x"}`, `{"a":"x","extra":1}`} { + if err := validatePayload(io.Discard, op, []byte(body)); err != nil { + t.Fatalf("valid branch rejected for %s: %v", body, err) + } + } + for _, body := range []string{`{}`, `{"a":1}`, `{"a":"x","b":"x"}`} { + if err := validatePayload(io.Discard, op, []byte(body)); err == nil { + t.Fatalf("invalid oneOf accepted for %s", body) + } + } +} + +func TestPayloadOneOfWithUnrelatedUnknown(t *testing.T) { + op := payloadTestOperation(t, `{"type":"object","properties":{"company":{"oneOf":[{"type":"object","additionalProperties":false,"properties":{"a":{"type":"string"}}},{"type":"object","additionalProperties":false,"properties":{"b":{"type":"string"}}}]},"other":{"type":"object","additionalProperties":false,"properties":{"x":{"type":"string"}}}}}`) + if err := validatePayload(io.Discard, op, []byte(`{"company":{"a":"x"},"other":{"extra":1}}`)); err != nil { + t.Fatal(err) + } +} + +func TestPayloadArrayComposition(t *testing.T) { + op := payloadTestOperation(t, `{"allOf":[{"type":"array","items":{"type":"object","properties":{"a":{"type":"string"}}}},{"type":"array","items":{"type":"object","properties":{"b":{"type":"string"}},"required":["b"]}}]}`) + var warnings bytes.Buffer + if err := validatePayload(&warnings, op, []byte(`[{"a":"x","b":"y"}]`)); err != nil { + t.Fatal(err) + } + if warnings.Len() != 0 { + t.Fatalf("known composition properties warned: %s", warnings.String()) + } +} + +func TestPayloadTypedMapNestedUnknown(t *testing.T) { + op := payloadTestOperation(t, `{"type":"object","additionalProperties":{"type":"object","additionalProperties":false,"required":["a"],"properties":{"a":{"type":"string"}}}}`) + var warnings bytes.Buffer + if err := validatePayload(&warnings, op, []byte(`{"entry":{"a":"x","extra":1}}`)); err != nil { + t.Fatal(err) + } + if !strings.Contains(warnings.String(), "/entry/extra") { + t.Fatal("missing typed-map child warning") + } +}