From 42612fc5fdde72575e31d0c842f0c578caaead73 Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Thu, 24 Sep 2026 22:21:54 +0100 Subject: [PATCH] test: every array-typed MCP argument must deliver its values to the API The input-shape guard (#455) makes every read that treats an argument as a list go through StringList, so a string is never dropped. It cannot see the reverse: a handler that declares an argument as an array and reads it with in.String(), which returns "" for an array and drops it without an error. These tests find every array-typed argument from the live tool schemas on both servers, with writes enabled, and require each to have a valid call in a table. Each call sends a genuine JSON array to a recording server, and every value must reach the request. A new array argument fails until it has an entry; a stale entry fails too. Ten arguments today -- four on Gateway (metrics measures and dimensions, request connection_ids, connection rules) and six on Outpost -- and all ten deliver correctly, so this is a guard, not a fix. The expected strings are the encodings captured from the recording server, not assumed. Verified against the failure: reading connection_ids with in.String() fails the Gateway test -- and shows the retry going out as {}, i.e. retry every connection instead of the two chosen, a silent widening. Reading include with in.String() fails the Outpost test. Removing a table entry fails with a message naming the argument. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_012XtSQ2kfpcRXXqweskgXkH --- pkg/gateway/mcp/array_args_reach_api_test.go | 137 +++++++++++++++++++ pkg/outpost/mcp/array_args_reach_api_test.go | 104 ++++++++++++++ 2 files changed, 241 insertions(+) create mode 100644 pkg/gateway/mcp/array_args_reach_api_test.go create mode 100644 pkg/outpost/mcp/array_args_reach_api_test.go diff --git a/pkg/gateway/mcp/array_args_reach_api_test.go b/pkg/gateway/mcp/array_args_reach_api_test.go new file mode 100644 index 00000000..637c1ffa --- /dev/null +++ b/pkg/gateway/mcp/array_args_reach_api_test.go @@ -0,0 +1,137 @@ +package mcp + +import ( + "context" + "io" + "net/http" + "net/url" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// A tool that declares an argument as an array must deliver a genuine array to +// the API. The input-shape guard makes every read that treats an argument as a +// list go through mcpcore.StringList; it cannot see the reverse -- a handler +// that declares an array but reads it with in.String(), which returns "" for an +// array and drops it without an error. +// +// So every array-typed argument is listed here with a valid call. The test +// sends a real JSON array and requires each value to reach the API. A new array +// argument fails until it has an entry. + +type arrayArgCall struct { + args map[string]any // a valid call, with the array argument set + want []string // strings that must appear in the request the API receives +} + +var gatewayArrayArgs = map[string]arrayArgCall{ + "gateway_metrics_read.measures": { + args: map[string]any{"action": "events", "start": "2025-01-01T00:00:00Z", "end": "2025-01-02T00:00:00Z", + "measures": []any{"count", "successful_count"}}, + want: []string{"measures[]=count", "measures[]=successful_count"}, + }, + "gateway_metrics_read.dimensions": { + args: map[string]any{"action": "events", "start": "2025-01-01T00:00:00Z", "end": "2025-01-02T00:00:00Z", + "measures": []any{"count"}, "dimensions": []any{"status", "connection_id"}}, + // connection_id is sent in the API's spelling (#442). + want: []string{"dimensions[]=status", "dimensions[]=webhook_id"}, + }, + "gateway_request_write.connection_ids": { + args: map[string]any{"action": "retry", "id": "req_1", "connection_ids": []any{"web_qa_a", "web_qa_b"}}, + want: []string{"web_qa_a", "web_qa_b"}, + }, + "gateway_connections_write.rules": { + args: map[string]any{"action": "update", "id": "web_1", "rules": []any{ + map[string]any{"type": "filter", "headers": map[string]any{"x-qa": "qa_marker_a"}}, + map[string]any{"type": "filter", "body": map[string]any{"qa": "qa_marker_b"}}, + }}, + want: []string{"qa_marker_a", "qa_marker_b"}, + }, +} + +// recordingAPI answers every API request and keeps its query and body. +func recordingAPI(t *testing.T) (map[string]http.HandlerFunc, func() string) { + var mu sync.Mutex + var seen []string + handler := func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + q, _ := url.QueryUnescape(r.URL.RawQuery) + mu.Lock() + seen = append(seen, r.Method+" "+r.URL.Path+"?"+q+" "+string(body)) + mu.Unlock() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":"web_1","models":[],"pagination":{},"data":[]}`)) + } + return map[string]http.HandlerFunc{hookdeck.APIPathPrefix + "/": handler}, func() string { + mu.Lock() + defer mu.Unlock() + return strings.Join(seen, "\n") + } +} + +func arrayArgumentsOf(t *testing.T, tools []toolSchema) map[string]bool { + found := map[string]bool{} + for _, tool := range tools { + for name, prop := range tool.props { + if p, _ := prop.(map[string]any); p["type"] == "array" { + found[tool.name+"."+name] = true + } + } + } + return found +} + +type toolSchema struct { + name string + props map[string]any +} + +func TestGatewayArrayArgumentsReachTheAPI(t *testing.T) { + handlers, requests := recordingAPI(t) + session := mockAPIWithClientWriteEnabled(t, handlers) + + list, err := session.ListTools(context.Background(), nil) + require.NoError(t, err) + var tools []toolSchema + for _, tool := range list.Tools { + props, _ := tool.InputSchema.(map[string]any)["properties"].(map[string]any) + tools = append(tools, toolSchema{tool.Name, props}) + } + found := arrayArgumentsOf(t, tools) + require.NotEmpty(t, found, "no array arguments found; the walk is broken and the guard would pass vacuously") + + for key := range found { + call, ok := gatewayArrayArgs[key] + if !assert.True(t, ok, "%s is declared as an array but has no entry in gatewayArrayArgs. "+ + "Add a valid call and check the values reach the API.", key) { + continue + } + t.Run(key, func(t *testing.T) { + before := requests() + result := callTool(t, session, strings.SplitN(key, ".", 2)[0], call.args) + sent := strings.TrimPrefix(requests(), before) + require.NotEmpty(t, sent, "the call reached no API endpoint: %s", fmtResult(t, result)) + for _, w := range call.want { + assert.Contains(t, sent, w, "an array value did not reach the API") + } + }) + } + for key := range gatewayArrayArgs { + assert.True(t, found[key], "%s is in gatewayArrayArgs but is no longer an array argument", key) + } +} + +func fmtResult(t *testing.T, r *mcpsdk.CallToolResult) string { + t.Helper() + if r == nil || len(r.Content) == 0 { + return "" + } + return textContent(t, r) +} diff --git a/pkg/outpost/mcp/array_args_reach_api_test.go b/pkg/outpost/mcp/array_args_reach_api_test.go new file mode 100644 index 00000000..a878ba9b --- /dev/null +++ b/pkg/outpost/mcp/array_args_reach_api_test.go @@ -0,0 +1,104 @@ +package mcp + +import ( + "context" + "io" + "net/http" + "net/url" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The Outpost half of the Gateway test of the same name: every array-typed +// argument must deliver a genuine array to the API. A handler that declared an +// array but read it with in.String() would drop it without an error. A new +// array argument fails here until it has an entry with a valid call. +// +// The wanted strings are the encoding the client actually sends, captured from +// a recording server rather than assumed. +var outpostArrayArgs = map[string]struct { + args map[string]any + want []string +}{ + "outpost_attempts_read.include": { + args: map[string]any{"action": "list", "tenant_id": "t1", "include": []any{"event", "destination"}}, + want: []string{"include[0]=event", "include[1]=destination"}, + }, + "outpost_config_write.unset": { + args: map[string]any{"action": "set", "unset": []any{"MAX_RETRY_LIMIT", "RETRY_INTERVAL_SECONDS"}}, + want: []string{`"MAX_RETRY_LIMIT":null`, `"RETRY_INTERVAL_SECONDS":null`}, + }, + "outpost_destinations_read.topics": { + args: map[string]any{"action": "list", "tenant_id": "t1", "topics": []any{"qa.a", "qa.b"}}, + want: []string{"topics[0]=qa.a", "topics[1]=qa.b"}, + }, + "outpost_destinations_write.topics": { + args: map[string]any{"action": "create", "tenant_id": "t1", "type": "webhook", + "config": map[string]any{"url": "https://example.com/hook"}, "topics": []any{"qa.a", "qa.b"}}, + want: []string{`"topics":["qa.a","qa.b"]`}, + }, + "outpost_metrics_read.measures": { + args: map[string]any{"action": "events", "start": "2025-01-01T00:00:00Z", "end": "2025-01-02T00:00:00Z", + "measures": []any{"count", "successful_count"}}, + want: []string{"measures[0]=count", "measures[1]=successful_count"}, + }, + "outpost_metrics_read.dimensions": { + args: map[string]any{"action": "events", "start": "2025-01-01T00:00:00Z", "end": "2025-01-02T00:00:00Z", + "measures": []any{"count"}, "dimensions": []any{"tenant_id", "topic"}}, + want: []string{"dimensions[0]=tenant_id", "dimensions[1]=topic"}, + }, +} + +func TestOutpostArrayArgumentsReachTheAPI(t *testing.T) { + var mu sync.Mutex + var seen []string + api := mockAPI(t, map[string]http.HandlerFunc{ + "/2026-09-01/": func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + q, _ := url.QueryUnescape(r.URL.RawQuery) + mu.Lock() + seen = append(seen, r.Method+" "+r.URL.Path+"?"+q+" "+string(body)) + mu.Unlock() + _, _ = w.Write([]byte(`{"id":"x","models":[],"data":[],"pagination":{}}`)) + }, + }) + requests := func() string { mu.Lock(); defer mu.Unlock(); return strings.Join(seen, "\n") } + session := connect(t, ServerOptions{Client: newTestClient(t, api.URL), WriteEnabled: true, PublishAPIKey: "pk"}) + + list, err := session.ListTools(context.Background(), nil) + require.NoError(t, err) + found := map[string]bool{} + for _, tool := range list.Tools { + props, _ := tool.InputSchema.(map[string]any)["properties"].(map[string]any) + for name, prop := range props { + if p, _ := prop.(map[string]any); p["type"] == "array" { + found[tool.Name+"."+name] = true + } + } + } + require.NotEmpty(t, found, "no array arguments found; the walk is broken and the guard would pass vacuously") + + for key := range found { + call, ok := outpostArrayArgs[key] + if !assert.True(t, ok, "%s is declared as an array but has no entry in outpostArrayArgs. "+ + "Add a valid call and check the values reach the API.", key) { + continue + } + t.Run(key, func(t *testing.T) { + before := requests() + callTool(t, session, strings.SplitN(key, ".", 2)[0], call.args) + sent := strings.TrimPrefix(requests(), before) + require.NotEmpty(t, sent, "the call reached no API endpoint") + for _, w := range call.want { + assert.Contains(t, sent, w, "an array value did not reach the API") + } + }) + } + for key := range outpostArrayArgs { + assert.True(t, found[key], "%s is in outpostArrayArgs but is no longer an array argument", key) + } +}