fix(mcp): prevent SIGSEGV in AddTool when schema is nil#918
Open
wucm667 wants to merge 10 commits intomodelcontextprotocol:mainfrom
Open
fix(mcp): prevent SIGSEGV in AddTool when schema is nil#918wucm667 wants to merge 10 commits intomodelcontextprotocol:mainfrom
wucm667 wants to merge 10 commits intomodelcontextprotocol:mainfrom
Conversation
jba
previously approved these changes
Apr 30, 2026
Author
|
@guglielmo-san Thanks for catching that! Added nil checks for both InputSchema and OutputSchema after the type assertion to handle the typed nil case. Pushed the fix. |
Contributor
|
@wucm667 I think you forgot to push the the fix :) |
Add nil checks for *jsonschema.Schema after type assertion to catch the case where a typed nil pointer is assigned to the interface field, which would otherwise cause a SIGSEGV when accessing schema properties. Suggested-by: guglielmo-san Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
Author
|
@guglielmo-san The fix is pushed now. Let me know if anything else needs adjusting. |
Contributor
|
@wucm667 can you add tests to |
Cover the typed nil InputSchema and OutputSchema cases that were added to prevent SIGSEGV when a typed nil *jsonschema.Schema is passed as a schema argument.
Author
|
@guglielmo-san Added tests in |
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
The previous GitHub suggested change introduced a syntax error by omitting the closing brace for TestAddToolNilSchema, causing lint and test failures in CI. Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
mcp.AddToolcrashes the server with SIGSEGV when input/output struct tags trigger a nil schema.Root Cause
In
mcp/server.go, the functionsetSchemacallsinternalSchema.Resolve(...)without checking ifinternalSchemais nil. If the schema inference (e.g., viajsonschema.ForType) or a provided schema results in a nil value, this dereference causes a panic.Fix
Added nil checks for
internalSchemabefore callingResolvein two locations withinsetSchema:jsonschema.ForType.If
internalSchemais nil, the function now returns a descriptive error instead of crashing.Verification
go test ./mcp/...passes.Closes #916.