google / google/jsonschema-go

`(*Schema).Resolve` panics with SIGSEGV when called on a nil receiver — missing nil-guards on resolver entry points

Open
#74 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
478
Forks
37
PR merge metrics
No merged PRs in 30d

Description

## Summary

`jsonschema.(*Schema).Resolve` does not check that its receiver is non-nil before calling `newResolved` / `detectDraft`. When a nil `*Schema` reaches `Resolve` (directly or via the recursive resolver), the process crashes with a SIGSEGV instead of returning an error.

This is reachable through the MCP Go SDK (`github.com/modelcontextprotocol/go-sdk` v1.4.0 and v1.6.0) when registering tools whose Go input/output structs use certain `jsonschema:"..."` struct tag descriptions. The SDK calls `ForType` then `internalSchema.Resolve(...)` without a nil-check, but the deeper bug is in this library: a public API (`Resolve`) and an internal recursive resolver (`(*resolver).resolve`) panic on nil rather than returning an error.

## Versions

- `github.com/google/jsonschema-go` **v0.4.2** and **v0.4.3**
- Go 1.24
- macOS arm64

(Reproduced on both versions; the 0.4.2 → 0.4.3 diff is unrelated.)

## Stack trace

```
[signal SIGSEGV: segmentation violation code=0x2 addr=0x10 pc=0x102b9c36c]

goroutine 1 [running]:
github.com/google/jsonschema-go/jsonschema.detectDraft(...)
.../jsonschema-go@v0.4.2/jsonschema/resolve.go:52
github.com/google/jsonschema-go/jsonschema.newResolved(...)
.../jsonschema-go@v0.4.2/jsonschema/resolve.go:43
github.com/google/jsonschema-go/jsonschema.(*resolver).resolve(0x..., 0x0, 0x...)
.../jsonschema-go@v0.4.2/jsonschema/resolve.go:211 +0x2c
github.com/google/jsonschema-go/jsonschema.(*Schema).Resolve(0x0, 0x...)
.../jsonschema-go@v0.4.2/jsonschema/resolve.go:185 +0x110
github.com/modelcontextprotocol/go-sdk/mcp.setSchema[...](...)
.../go-sdk@v1.4.0/mcp/server.go:466 +0x1a0
github.com/modelcontextprotocol/go-sdk/mcp.toolForErr[...](...)
.../go-sdk@v1.4.0/mcp/server.go:309 +0x230
github.com/modelcontextprotocol/go-sdk/mcp.AddTool[...](...)
.../go-sdk@v1.4.0/mcp/server.go:501 +0x38
```

The frame `(*Schema).Resolve(0x0, ...)` shows the receiver is **nil**. The frame `(*resolver).resolve(..., 0x0, ...)` shows a nil sub-schema reached the recursive resolver. Both should be guarded.

## Empirical trigger

The crash was triggered by registering an MCP tool whose input struct contained `jsonschema:"..."` descriptions with one or more of the following characters in combination across multiple fields of the same struct:

- comma-separated phrases inside the description (e.g. `"Filter by status: ok, pending, failed, skipped"`)
- equals sign (`=`)
- greater/less-than (`>`, `<`)
- em or en dash (`—`, `–`)
- single quotes around literals (`'foo'`)
- bracket-style examples (`['a', 'b']`)
- parentheses with embedded punctuation

Removing all of these (descriptions reduced to ASCII letters and spaces only) made the crash go away. We were not able to reduce it to a single field in isolation — calling `ForType` + `Resolve` on individual fields with these characters did **not** panic. The panic only manifested through the SDK's full registration pipeline. We suspect a tag-parsing edge case produces a nil sub-schema (e.g. an `Items`, `Properties` entry, `AnyOf`/`OneOf` element, or similar) that is later walked by the resolver.

## Expected behaviour

1. `(*Schema).Resolve` should detect a nil receiver and return a clear error such as `errors.New("jsonschema: cannot Resolve nil schema")` instead of segfaulting.
2. `(*resolver).resolve` should detect a nil schema argument and either skip it or return a structured error pointing to the offending JSON pointer / field path.
3. If a struct tag fails to parse, `ForType` should surface that as an error rather than producing a partially-nil schema tree.

## Suggested fix

In `resolve.go`:

```go
func (s *Schema) Resolve(opts *ResolveOptions) (*Resolved, error) {
if s == nil {
return nil, errors.New("jsonschema: Resolve called on nil *Schema")
}
// ... existing code
}

func (r *resolver) resolve(parent *Schema, s *Schema) ... {
if s == nil {
return nil, fmt.Errorf("jsonschema: nil sub-schema at %s", r.path())
}
// ... existing code
}
```

And in `infer.go` / wherever struct tags are parsed, surface tag-parse failures as returned errors from `ForType` rather than producing a tree containing nil schemas.

## Workaround

For users hitting this through the MCP Go SDK: restrict `jsonschema:"..."` description text to ASCII letters and spaces only. No commas, no `,required` directive, no parens, brackets, quotes, equals, angle brackets, em/en dashes, or non-ASCII characters in the description.

## Repro attempts

Standalone calls to `ForType` + `Resolve` on individual struct types that exhibited the crash through the SDK did **not** panic. The trigger is timing/order-dependent within the SDK's registration pipeline, possibly involving the `SchemaCache`. We could not produce a self-contained Go file that reliably crashes — happy to provide one if anyone has a hypothesis to test.

The original failing struct (since fixed in our codebase) was roughly:

```go
type SPExtractParams struct {
SiteID string `json:"site_id" jsonschema:"SharePoint site ID,required"`
DriveID string `json:"drive_id" jsonschema:"Document library (drive) ID,required"`
BatchSize int `json:"batch_size,omitempty" jsonschema:"Maximum number of records to process in one call (default: 20)"`
ResetFailed bool `json:"reset_failed,omitempty" jsonschema:"When true, also reprocess previously failed records (in addition to pending)"`
RetryEmpty bool `json:"retry_empty,omitempty" jsonschema:"When true, reprocess ok records whose source field is empty (source_bytes>0 but source was not stored — e.g. due to the pre-fix keyword mapping limit)"`
Extensions []string `json:"extensions,omitempty" jsonschema:"If set, only records with these extensions (without leading dot, e.g. ['pptx','docx']) are fetched and processed. All others are left untouched."`
Workers int `json:"workers,omitempty" jsonschema:"Number of parallel extraction workers (default: 1 = sequential). Increase carefully — each worker holds an open file in memory."`
}
```

Regardless of whether a self-contained repro can be produced, the nil-pointer dereference in `Resolve` and the recursive resolver should be defended against — a malformed schema tree should never be able to crash a server at startup.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.