Skip to content

Commit 3ba7c0f

Browse files
Add tests for config.stripExtensionFieldsForValidation and assignLegacyIntAlias
These two private functions in internal/config/config_stdin.go had zero direct test coverage: - stripExtensionFieldsForValidation: removes gateway-specific extension fields (guards, guard, auth, tool_response_filters) from JSON config before upstream schema validation. Tests cover all branches: top-level guards removal, per-server field stripping for each extension field, multiple servers, guard-policies preservation, and invalid JSON error. - assignLegacyIntAlias: maps a legacy field name to a typed int pointer only when the target has not already been set. Tests cover all branches: target already set (skip), alias absent (no-op), valid int assignment, zero value, negative value, and invalid/malformed JSON (error path). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 04941b0 commit 3ba7c0f

1 file changed

Lines changed: 311 additions & 0 deletions

File tree

Lines changed: 311 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
1+
package config
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// TestStripExtensionFieldsForValidation tests that stripExtensionFieldsForValidation
12+
// correctly removes gateway-specific extension fields from JSON config data.
13+
func TestStripExtensionFieldsForValidation(t *testing.T) {
14+
tests := []struct {
15+
name string
16+
input string
17+
wantKeys []string // top-level keys expected in output
18+
wantAbsent []string // top-level keys expected to be removed
19+
wantErr bool
20+
checkServer func(t *testing.T, servers map[string]interface{})
21+
}{
22+
{
23+
name: "removes top-level guards field",
24+
input: `{
25+
"mcpServers": {},
26+
"guards": {"my-guard": {"type": "wasm", "path": "/guard.wasm"}}
27+
}`,
28+
wantKeys: []string{"mcpServers"},
29+
wantAbsent: []string{"guards"},
30+
},
31+
{
32+
name: "removes per-server guard field",
33+
input: `{
34+
"mcpServers": {
35+
"github": {
36+
"type": "stdio",
37+
"container": "ghcr.io/org/mcp:latest",
38+
"guard": "my-guard"
39+
}
40+
}
41+
}`,
42+
checkServer: func(t *testing.T, servers map[string]interface{}) {
43+
t.Helper()
44+
github, ok := servers["github"].(map[string]interface{})
45+
require.True(t, ok, "github server should be a map")
46+
assert.NotContains(t, github, "guard", "guard field should be removed")
47+
assert.Contains(t, github, "type", "type field should remain")
48+
assert.Contains(t, github, "container", "container field should remain")
49+
},
50+
},
51+
{
52+
name: "removes per-server auth field",
53+
input: `{
54+
"mcpServers": {
55+
"myserver": {
56+
"type": "http",
57+
"url": "https://example.com/mcp",
58+
"auth": {"type": "github-oidc", "audience": "https://example.com"}
59+
}
60+
}
61+
}`,
62+
checkServer: func(t *testing.T, servers map[string]interface{}) {
63+
t.Helper()
64+
server, ok := servers["myserver"].(map[string]interface{})
65+
require.True(t, ok, "myserver should be a map")
66+
assert.NotContains(t, server, "auth", "auth field should be removed")
67+
assert.Contains(t, server, "type", "type field should remain")
68+
assert.Contains(t, server, "url", "url field should remain")
69+
},
70+
},
71+
{
72+
name: "removes per-server tool_response_filters field",
73+
input: `{
74+
"mcpServers": {
75+
"backend": {
76+
"type": "http",
77+
"url": "https://backend.com/mcp",
78+
"tool_response_filters": {"my_tool": ".result"}
79+
}
80+
}
81+
}`,
82+
checkServer: func(t *testing.T, servers map[string]interface{}) {
83+
t.Helper()
84+
server, ok := servers["backend"].(map[string]interface{})
85+
require.True(t, ok, "backend server should be a map")
86+
assert.NotContains(t, server, "tool_response_filters", "tool_response_filters should be removed")
87+
assert.Contains(t, server, "url", "url field should remain")
88+
},
89+
},
90+
{
91+
name: "removes all extension fields simultaneously",
92+
input: `{
93+
"mcpServers": {
94+
"s1": {
95+
"type": "stdio",
96+
"container": "ghcr.io/org/img:latest",
97+
"guard": "wasm-guard",
98+
"auth": {"type": "github-oidc"},
99+
"tool_response_filters": {"tool": ".x"}
100+
}
101+
},
102+
"guards": {"wasm-guard": {"type": "wasm"}}
103+
}`,
104+
wantAbsent: []string{"guards"},
105+
checkServer: func(t *testing.T, servers map[string]interface{}) {
106+
t.Helper()
107+
s1, ok := servers["s1"].(map[string]interface{})
108+
require.True(t, ok)
109+
assert.NotContains(t, s1, "guard")
110+
assert.NotContains(t, s1, "auth")
111+
assert.NotContains(t, s1, "tool_response_filters")
112+
assert.Contains(t, s1, "type")
113+
assert.Contains(t, s1, "container")
114+
},
115+
},
116+
{
117+
name: "handles missing mcpServers field gracefully",
118+
input: `{
119+
"guards": {"g": {"type": "noop"}}
120+
}`,
121+
wantAbsent: []string{"guards"},
122+
},
123+
{
124+
name: "preserves guard-policies per-server field (not stripped)",
125+
input: `{
126+
"mcpServers": {
127+
"s1": {
128+
"type": "http",
129+
"url": "https://example.com",
130+
"guard-policies": [{"allow-only": {"repos": "public", "min-integrity": "none"}}]
131+
}
132+
}
133+
}`,
134+
checkServer: func(t *testing.T, servers map[string]interface{}) {
135+
t.Helper()
136+
s1, ok := servers["s1"].(map[string]interface{})
137+
require.True(t, ok)
138+
assert.Contains(t, s1, "guard-policies", "guard-policies should NOT be stripped")
139+
},
140+
},
141+
{
142+
name: "handles multiple servers stripping each one",
143+
input: `{
144+
"mcpServers": {
145+
"a": {"type": "http", "url": "https://a.com", "guard": "g1", "auth": {"type": "github-oidc"}},
146+
"b": {"type": "http", "url": "https://b.com", "tool_response_filters": {"t": "."}},
147+
"c": {"type": "stdio", "container": "img"}
148+
}
149+
}`,
150+
checkServer: func(t *testing.T, servers map[string]interface{}) {
151+
t.Helper()
152+
a, ok := servers["a"].(map[string]interface{})
153+
require.True(t, ok)
154+
assert.NotContains(t, a, "guard")
155+
assert.NotContains(t, a, "auth")
156+
157+
b, ok := servers["b"].(map[string]interface{})
158+
require.True(t, ok)
159+
assert.NotContains(t, b, "tool_response_filters")
160+
161+
c, ok := servers["c"].(map[string]interface{})
162+
require.True(t, ok)
163+
assert.Contains(t, c, "container", "unextended server should be intact")
164+
},
165+
},
166+
{
167+
name: "returns error on invalid JSON input",
168+
input: `{not valid json}`,
169+
wantErr: true,
170+
},
171+
{
172+
name: "empty config is preserved",
173+
input: `{}`,
174+
},
175+
{
176+
name: "no extension fields: output matches input structure",
177+
input: `{
178+
"mcpServers": {
179+
"plain": {"type": "http", "url": "https://example.com/mcp"}
180+
}
181+
}`,
182+
checkServer: func(t *testing.T, servers map[string]interface{}) {
183+
t.Helper()
184+
plain, ok := servers["plain"].(map[string]interface{})
185+
require.True(t, ok)
186+
assert.Contains(t, plain, "type")
187+
assert.Contains(t, plain, "url")
188+
},
189+
},
190+
}
191+
192+
for _, tt := range tests {
193+
t.Run(tt.name, func(t *testing.T) {
194+
result, err := stripExtensionFieldsForValidation([]byte(tt.input))
195+
196+
if tt.wantErr {
197+
require.Error(t, err)
198+
return
199+
}
200+
201+
require.NoError(t, err)
202+
assert.NotNil(t, result)
203+
204+
// Parse result for inspection
205+
var out map[string]interface{}
206+
require.NoError(t, json.Unmarshal(result, &out), "output must be valid JSON")
207+
208+
for _, k := range tt.wantKeys {
209+
assert.Contains(t, out, k, "expected key %q to be present", k)
210+
}
211+
for _, k := range tt.wantAbsent {
212+
assert.NotContains(t, out, k, "expected key %q to be absent", k)
213+
}
214+
215+
if tt.checkServer != nil {
216+
servers, ok := out["mcpServers"].(map[string]interface{})
217+
require.True(t, ok, "mcpServers should be a map in output")
218+
tt.checkServer(t, servers)
219+
}
220+
})
221+
}
222+
}
223+
224+
// TestAssignLegacyIntAlias tests all branches of assignLegacyIntAlias.
225+
func TestAssignLegacyIntAlias(t *testing.T) {
226+
t.Run("target already set: skips assignment", func(t *testing.T) {
227+
existing := 99
228+
target := &existing
229+
fields := map[string]json.RawMessage{
230+
"timeout": json.RawMessage(`42`),
231+
}
232+
err := assignLegacyIntAlias(fields, "timeout", &target)
233+
require.NoError(t, err)
234+
// target should remain unchanged
235+
assert.Equal(t, 99, *target, "target should not be overwritten when already set")
236+
})
237+
238+
t.Run("alias not in fields: no-op", func(t *testing.T) {
239+
var target *int
240+
fields := map[string]json.RawMessage{}
241+
err := assignLegacyIntAlias(fields, "missing", &target)
242+
require.NoError(t, err)
243+
assert.Nil(t, target, "target should remain nil when alias not found")
244+
})
245+
246+
t.Run("alias found with valid int: assigns target", func(t *testing.T) {
247+
var target *int
248+
fields := map[string]json.RawMessage{
249+
"old_timeout": json.RawMessage(`120`),
250+
}
251+
err := assignLegacyIntAlias(fields, "old_timeout", &target)
252+
require.NoError(t, err)
253+
require.NotNil(t, target)
254+
assert.Equal(t, 120, *target)
255+
})
256+
257+
t.Run("alias found with zero value: assigns zero", func(t *testing.T) {
258+
var target *int
259+
fields := map[string]json.RawMessage{
260+
"val": json.RawMessage(`0`),
261+
}
262+
err := assignLegacyIntAlias(fields, "val", &target)
263+
require.NoError(t, err)
264+
require.NotNil(t, target)
265+
assert.Equal(t, 0, *target)
266+
})
267+
268+
t.Run("alias found with invalid JSON: returns error", func(t *testing.T) {
269+
var target *int
270+
fields := map[string]json.RawMessage{
271+
"bad": json.RawMessage(`"not-an-int"`),
272+
}
273+
err := assignLegacyIntAlias(fields, "bad", &target)
274+
require.Error(t, err)
275+
assert.Contains(t, err.Error(), "bad")
276+
assert.Nil(t, target, "target should remain nil on error")
277+
})
278+
279+
t.Run("alias found with malformed JSON: returns error", func(t *testing.T) {
280+
var target *int
281+
fields := map[string]json.RawMessage{
282+
"x": json.RawMessage(`{invalid}`),
283+
}
284+
err := assignLegacyIntAlias(fields, "x", &target)
285+
require.Error(t, err)
286+
assert.Nil(t, target)
287+
})
288+
289+
t.Run("target set to nil explicitly, alias exists: assigns value", func(t *testing.T) {
290+
var target *int
291+
// target is nil pointer (not set), so alias should be used
292+
fields := map[string]json.RawMessage{
293+
"count": json.RawMessage(`7`),
294+
}
295+
err := assignLegacyIntAlias(fields, "count", &target)
296+
require.NoError(t, err)
297+
require.NotNil(t, target)
298+
assert.Equal(t, 7, *target)
299+
})
300+
301+
t.Run("negative int value is valid", func(t *testing.T) {
302+
var target *int
303+
fields := map[string]json.RawMessage{
304+
"delta": json.RawMessage(`-5`),
305+
}
306+
err := assignLegacyIntAlias(fields, "delta", &target)
307+
require.NoError(t, err)
308+
require.NotNil(t, target)
309+
assert.Equal(t, -5, *target)
310+
})
311+
}

0 commit comments

Comments
 (0)