Skip to content

Commit 9aef516

Browse files
authored
fix(mcp-proxy): isolate MCP reload panics (#2959)
* fix(mcp-proxy): skip invalid OpenAPI parameters Why this change was needed: OpenAPI input can contain nil parameter refs or schema refs with neither a value nor a resolved reference. The converter previously dereferenced those shapes while building MCP tool schemas. What changed: - Added a schema-ref presence check before marshaling OpenAPI schemas - Skipped nil, empty, or schema-less parameters instead of building partial parameter schemas - Reused the same guard for JSON request bodies with empty schema refs - Added a regression test covering invalid parameter entries Problem solved: Bad parameter metadata no longer panics the OpenAPI-to-MCP converter. * fix(mcp-proxy): isolate MCP reload panics per server Why this change was needed: A panic while applying one MCP server reload could interrupt the whole reload round. A panic in concurrent prefetch could also be recovered without being recorded on that server result, making later stats misleading. What changed: - Added per-server panic recovery around applyServerChanges processing - Counted apply panics as load errors while continuing to later servers - Recorded prefetch panics on the matching serverLoadResult - Added regression tests for prefetch panic attribution and apply-stage isolation Problem solved: Bad data from one MCP server no longer prevents later servers in the same reload round from being applied, and recovered prefetch panics are counted as server load errors. * fix(mcp-proxy): guard nil MCP reload apply results Why this change was needed: A nil server result from reload prefetch could still panic during the apply phase before per-server panic isolation ran, stopping later servers from being applied. What changed: - Count nil apply results as reload errors before accessing the server name - Make apply panic recovery logging safe when the server is nil - Add a regression test that verifies a nil result does not block a later valid server Problem solved: MCP reload now preserves per-server isolation for nil-server apply results, so one malformed result cannot abort the rest of the reload. * fix(mcp-proxy): report reload panic stack traces Why this change was needed: Reload panic isolation logged only the panic value, which made per-server prefetch and apply failures harder to diagnose after recovery. What changed: - Added a shared reload panic reporting helper with stack traces - Reported reload panic phase and MCP server name to Sentry context - Added a regression test for stack and Sentry report metadata Problem solved: Recovered MCP reload panics now retain enough diagnostic context for operations without losing per-server isolation. * fix(mcp-proxy): preserve tools on failed reload updates Why this change was needed: Reload updates could prune existing tools before the new OpenAPI spec update had succeeded, and update failures were counted as skipped reloads. What changed: - Apply the new MCP server spec before pruning stale tools - Return update errors separately from no-op updates - Count update failures as reload errors instead of skips - Add regressions for failed update state preservation and error accounting Problem solved: A failed MCP reload update no longer leaves the existing server with prematurely removed tools, and reload stats now report update failures accurately. * docs(mcp-proxy): clarify OpenAPI ref handling in converter Why this change was needed: The converter keeps accepting Ref-only schema refs, which can look like it allows dangling refs to reach MCP tool schemas. What changed: - Document that LoadFromData resolves valid refs and rejects dangling refs before reload conversion Problem solved: Future review of the converter can distinguish production reload behavior from defensive compatibility in the helper.
1 parent 87ed78e commit 9aef516

5 files changed

Lines changed: 534 additions & 92 deletions

File tree

src/mcp-proxy/pkg/infra/proxy/converter.go

Lines changed: 56 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func openapiSchemaRefToJSONSchema(schemaRef *openapi3.SchemaRef) jsonschema.Sche
4444
}
4545

4646
func marshalOpenAPISchemaRefJSON(schemaRef *openapi3.SchemaRef) ([]byte, error) {
47-
if schemaRef == nil {
47+
if !openapiSchemaRefHasSchema(schemaRef) {
4848
return nil, nil
4949
}
5050
if schemaRef.Value != nil {
@@ -53,6 +53,11 @@ func marshalOpenAPISchemaRefJSON(schemaRef *openapi3.SchemaRef) ([]byte, error)
5353
return schemaRef.MarshalJSON()
5454
}
5555

56+
func openapiSchemaRefHasSchema(schemaRef *openapi3.SchemaRef) bool {
57+
// LoadFromData resolves valid refs and rejects dangling refs before reload conversion.
58+
return schemaRef != nil && (schemaRef.Value != nil || schemaRef.Ref != "")
59+
}
60+
5661
func fallbackOpenAPIJSONSchema(schemaJSON []byte) jsonschema.Schema {
5762
var rawSchema map[string]any
5863
if err := json.Unmarshal(schemaJSON, &rawSchema); err != nil {
@@ -212,58 +217,61 @@ func OpenapiToMcpToolConfig(
212217
Type: j.WithSimpleTypes(jsonschema.Object),
213218
}
214219
for _, param := range operation.Parameters {
215-
if param.Value.Schema != nil {
216-
jsonSchema := openapiSchemaRefToJSONSchema(param.Value.Schema)
217-
if param.Value.Description != "" {
218-
jsonSchema.Description = &param.Value.Description
219-
}
220-
if param.Value.Example != nil {
221-
if jsonSchema.ExtraProperties == nil {
222-
jsonSchema.ExtraProperties = map[string]any{}
223-
}
224-
jsonSchema.ExtraProperties["example"] = param.Value.Example
225-
}
226-
if param.Value.In == "header" {
227-
headerParamSchema.WithPropertiesItem(
228-
param.Value.Name,
229-
jsonschema.SchemaOrBool{
230-
TypeObject: &jsonSchema,
231-
},
232-
)
233-
if param.Value.Required {
234-
headerParamSchema.Required = append(
235-
headerParamSchema.Required,
236-
param.Value.Name,
237-
)
238-
}
220+
if param == nil || param.Value == nil ||
221+
!openapiSchemaRefHasSchema(param.Value.Schema) {
222+
continue
223+
}
224+
parameter := param.Value
225+
jsonSchema := openapiSchemaRefToJSONSchema(parameter.Schema)
226+
if parameter.Description != "" {
227+
jsonSchema.Description = &parameter.Description
228+
}
229+
if parameter.Example != nil {
230+
if jsonSchema.ExtraProperties == nil {
231+
jsonSchema.ExtraProperties = map[string]any{}
239232
}
240-
if param.Value.In == "query" {
241-
queryParamSchema.WithPropertiesItem(
242-
param.Value.Name,
243-
jsonschema.SchemaOrBool{
244-
TypeObject: &jsonSchema,
245-
},
233+
jsonSchema.ExtraProperties["example"] = parameter.Example
234+
}
235+
if parameter.In == "header" {
236+
headerParamSchema.WithPropertiesItem(
237+
parameter.Name,
238+
jsonschema.SchemaOrBool{
239+
TypeObject: &jsonSchema,
240+
},
241+
)
242+
if parameter.Required {
243+
headerParamSchema.Required = append(
244+
headerParamSchema.Required,
245+
parameter.Name,
246246
)
247-
if param.Value.Required {
248-
queryParamSchema.Required = append(
249-
queryParamSchema.Required,
250-
param.Value.Name,
251-
)
252-
}
253247
}
254-
if param.Value.In == "path" {
255-
pathParamSchema.Required = append(
256-
pathParamSchema.Required,
257-
param.Value.Name,
258-
)
259-
pathParamSchema.WithPropertiesItem(
260-
param.Value.Name,
261-
jsonschema.SchemaOrBool{
262-
TypeObject: &jsonSchema,
263-
},
248+
}
249+
if parameter.In == "query" {
250+
queryParamSchema.WithPropertiesItem(
251+
parameter.Name,
252+
jsonschema.SchemaOrBool{
253+
TypeObject: &jsonSchema,
254+
},
255+
)
256+
if parameter.Required {
257+
queryParamSchema.Required = append(
258+
queryParamSchema.Required,
259+
parameter.Name,
264260
)
265261
}
266262
}
263+
if parameter.In == "path" {
264+
pathParamSchema.Required = append(
265+
pathParamSchema.Required,
266+
parameter.Name,
267+
)
268+
pathParamSchema.WithPropertiesItem(
269+
parameter.Name,
270+
jsonschema.SchemaOrBool{
271+
TypeObject: &jsonSchema,
272+
},
273+
)
274+
}
267275
}
268276
if len(headerParamSchema.Properties) > 0 {
269277
headerParamDesc := "HTTP request header parameters, " +
@@ -299,7 +307,7 @@ func OpenapiToMcpToolConfig(
299307

300308
if operation.RequestBody != nil && operation.RequestBody.Value != nil {
301309
if content, ok := operation.RequestBody.Value.Content["application/json"]; ok &&
302-
content != nil && content.Schema != nil {
310+
content != nil && openapiSchemaRefHasSchema(content.Schema) {
303311
jsonSchema := openapiSchemaRefToJSONSchema(content.Schema)
304312
if jsonSchemaDescriptionIsEmpty(&jsonSchema) {
305313
bodyParamDesc := bodyParamDefaultDescription

src/mcp-proxy/pkg/infra/proxy/converter_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,48 @@ var _ = Describe("Converter", func() {
284284
Expect(result[0].ParamSchema.Required).To(ContainElement("header_param"))
285285
})
286286

287+
It("should skip invalid parameters without panicking", func() {
288+
spec := &openapi3.T{
289+
OpenAPI: "3.0.0",
290+
Info: &openapi3.Info{Title: "Test API", Version: "1.0.0"},
291+
Servers: []*openapi3.Server{{URL: "https://api.example.com/v1"}},
292+
Paths: &openapi3.Paths{},
293+
}
294+
295+
pathItem := &openapi3.PathItem{
296+
Get: &openapi3.Operation{
297+
OperationID: "getUsers",
298+
Summary: "Get users",
299+
Parameters: openapi3.Parameters{
300+
nil,
301+
&openapi3.ParameterRef{},
302+
&openapi3.ParameterRef{
303+
Value: &openapi3.Parameter{
304+
Name: "limit",
305+
In: "query",
306+
},
307+
},
308+
&openapi3.ParameterRef{
309+
Value: &openapi3.Parameter{
310+
Name: "offset",
311+
In: "query",
312+
Schema: &openapi3.SchemaRef{},
313+
},
314+
},
315+
},
316+
Responses: &openapi3.Responses{},
317+
},
318+
}
319+
spec.Paths.Set("/users", pathItem)
320+
321+
var result []*ToolConfig
322+
Expect(func() {
323+
result = OpenapiToMcpToolConfig(spec, nil, nil)
324+
}).NotTo(Panic())
325+
Expect(result).To(HaveLen(1))
326+
Expect(result[0].ParamSchema.Properties).NotTo(HaveKey("query_param"))
327+
})
328+
287329
It("should skip JSON request body without schema", func() {
288330
spec := &openapi3.T{
289331
OpenAPI: "3.0.0",

src/mcp-proxy/pkg/mcp/export_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package mcp
2121
import (
2222
"context"
2323

24+
"github.com/getkin/kin-openapi/openapi3"
2425
sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp"
2526

2627
"mcp_proxy/pkg/entity/model"
@@ -104,11 +105,33 @@ func NewConfig(resourceVersion int) *Config {
104105
}
105106
}
106107

108+
// NewConfigWithOpenAPISpec creates a Config with OpenAPI data for testing.
109+
func NewConfigWithOpenAPISpec(resourceVersion int, openapiFileData *openapi3.T) *Config {
110+
return &Config{
111+
resourceVersion: resourceVersion,
112+
openapiFileData: openapiFileData,
113+
}
114+
}
115+
107116
// GetLoadStatsValues returns stats values for testing.
108117
func GetLoadStatsValues(stats *loadStats) (added, updated, skipped, errorCount int) {
109118
return stats.addedCount, stats.updatedCount, stats.skippedCount, stats.errorCount
110119
}
111120

121+
// GetServerLoadResultError returns the load result error for testing.
122+
func GetServerLoadResultError(result *serverLoadResult) error {
123+
return result.err
124+
}
125+
126+
// BuildReloadPanicReportForTest exposes buildReloadPanicReport for testing.
127+
func BuildReloadPanicReportForTest(
128+
phase string,
129+
serverName string,
130+
panicErr any,
131+
) (string, map[string]string, map[string]any, error) {
132+
return buildReloadPanicReport(phase, serverName, panicErr)
133+
}
134+
112135
// PrefetchServerConfigsForTest exposes prefetchServerConfigs for benchmark testing.
113136
func PrefetchServerConfigsForTest(
114137
ctx context.Context,

0 commit comments

Comments
 (0)