Skip to content

Commit 058ef68

Browse files
committed
fix: preserve tool schema metadata across pagination
1 parent 862f446 commit 058ef68

6 files changed

Lines changed: 95 additions & 48 deletions

File tree

docs/migration-SKILL.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ Tool schemas conform to full JSON Schema 2020-12, and `structuredContent` may be
563563
| ---------------------------------------------- | -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- |
564564
| `inputSchema` root | `type: "object"` + `properties`/`required` only | `type: "object"` required, **plus** any 2020-12 keyword (`oneOf`/`anyOf`/`allOf`/`not`, `if`/`then`/`else`, `$ref`/`$defs`/`$anchor`) |
565565
| `outputSchema` root | `type: "object"` only | **any** valid JSON Schema 2020-12 (object, array, primitive, composition) |
566-
| `Tool.inputSchema` / `Tool.outputSchema` types | object schema with typed `properties`/`required` members | broad JSON Schema records; narrow before reading keyword properties (**source-breaking**) |
566+
| `Tool.inputSchema` / `Tool.outputSchema` types | object schema with typed `properties`/`required` members | broad JSON Schema records; narrow keyword field values before using them (**source-breaking**) |
567567
| `CallToolResult.structuredContent` type | `{ [key: string]: unknown }` | `unknown` (**source-breaking**) |
568568
| `client.callTool(...)` | returns `structuredContent` as object | returns `structuredContent` as `unknown`; narrow it before property access |
569569
| `registerTool` handler return | `structuredContent` untyped | type-checked against the tool's `outputSchema` inferred output |
@@ -578,20 +578,21 @@ const sc = result.structuredContent;
578578
const temp = typeof sc === 'object' && sc !== null && !Array.isArray(sc) ? (sc as Record<string, unknown>).temperature : undefined;
579579
```
580580

581-
Source-breaking fix — property access on `Tool.inputSchema` / `Tool.outputSchema` keyword fields also needs narrowing:
581+
Source-breaking fix — property access on `Tool.inputSchema` / `Tool.outputSchema` keyword field values also needs narrowing:
582582

583583
```typescript
584-
const schema = tool.inputSchema;
585-
const properties =
586-
typeof schema === 'object' && schema !== null && !Array.isArray(schema)
587-
? (schema as Record<string, unknown>).properties
588-
: undefined;
584+
const required = Array.isArray(tool.inputSchema.required) ? tool.inputSchema.required : [];
585+
const properties = tool.inputSchema.properties;
586+
if (typeof properties === 'object' && properties !== null && !Array.isArray(properties)) {
587+
const propertyNames = Object.keys(properties);
588+
}
589589
```
590590

591591
Behavior notes:
592592

593593
- A server returning array/primitive `structuredContent` automatically also emits a serialized `TextContent` block (old-client interop). No action required.
594-
- Built-in validators reject non-same-document `$ref`/`$dynamicRef` (SSRF) and over-budget schemas (composition DoS). Use a custom `jsonSchemaValidator` to change this.
594+
- Built-in validators reject non-same-document `$ref`/`$dynamicRef` (SSRF) and over-budget schemas (composition DoS). Use `resolveExternalSchemaRefs(schema, { allowlist })` to fetch and inline approved external refs before validation, or use a custom `jsonSchemaValidator` to
595+
change validator behavior.
595596

596597
## 16. Migration Steps (apply in this order)
597598

docs/migration.md

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,22 +1033,23 @@ if (typeof sc === 'object' && sc !== null && !Array.isArray(sc)) {
10331033
}
10341034
```
10351035

1036-
The generated `Tool.inputSchema` and `Tool.outputSchema` types also widened to reflect full JSON Schema 2020-12. `Tool.inputSchema.properties`, `Tool.inputSchema.required`, and analogous `outputSchema` fields are no longer statically present. Narrow the schema to an object record
1037-
before reading keyword properties:
1036+
The generated `Tool.inputSchema` and `Tool.outputSchema` types also widened to reflect full JSON Schema 2020-12. Keyword fields such as `properties`, `required`, and analogous `outputSchema` fields now have broad JSON values. Narrow the keyword field value before using it:
10381037

10391038
```typescript
1040-
const schema = tool.inputSchema;
1041-
const properties =
1042-
typeof schema === 'object' && schema !== null && !Array.isArray(schema)
1043-
? (schema as Record<string, unknown>).properties
1044-
: undefined;
1039+
const required = Array.isArray(tool.inputSchema.required) ? tool.inputSchema.required : [];
1040+
const properties = tool.inputSchema.properties;
1041+
if (typeof properties === 'object' && properties !== null && !Array.isArray(properties)) {
1042+
const propertyNames = Object.keys(properties);
1043+
}
10451044
```
10461045

10471046
**Stronger server-side typing.** When a tool declares an `outputSchema`, `registerTool` now type-checks the handler's returned `structuredContent` against the schema's inferred output type at compile time — a mismatch is a type error rather than a runtime-only failure.
10481047

1049-
**Old-client interoperability.** A server that returns array or primitive `structuredContent` will automatically also emit a `TextContent` block containing the serialized JSON, so pre-SEP clients that only understand object-typed `structuredContent` can fall back to the text content. Object `structuredContent` (and results that already include a text block) are left unchanged.
1048+
**Old-client interoperability.** A server that returns array or primitive `structuredContent` will automatically also emit a `TextContent` block containing the serialized JSON, so pre-SEP clients that only understand object-typed `structuredContent` can fall back to the text
1049+
content. Object `structuredContent` (and results that already include a text block) are left unchanged.
10501050

1051-
**Security.** The built-in validators never dereference non-same-document `$ref`/`$dynamicRef` (anything not beginning with `#`) — such schemas are rejected rather than fetched, preventing SSRF. Schemas exceeding a generous depth / subschema-count bound are also rejected to prevent composition-based validation DoS. Supply your own `jsonSchemaValidator` implementation if you need different behavior.
1051+
**Security.** The built-in validators never dereference non-same-document `$ref`/`$dynamicRef` (anything not beginning with `#`) — such schemas are rejected rather than fetched, preventing SSRF. If you intentionally need external references, resolve and inline them before
1052+
validation with `resolveExternalSchemaRefs(schema, { allowlist })`. Schemas exceeding a generous depth / subschema-count bound are also rejected to prevent composition-based validation DoS. Supply your own `jsonSchemaValidator` implementation if you need different behavior.
10521053

10531054
## Unchanged APIs
10541055

packages/client/src/client/client.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -870,11 +870,16 @@ export class Client extends Protocol<ClientContext> {
870870
* Cache validators for tool output schemas.
871871
* Called after {@linkcode listTools | listTools()} to pre-compile validators for better performance.
872872
*/
873-
private cacheToolMetadata(tools: Tool[]): void {
874-
this._cachedToolOutputValidators.clear();
875-
this._toolOutputValidatorErrors.clear();
873+
private cacheToolMetadata(tools: Tool[], reset: boolean): void {
874+
if (reset) {
875+
this._cachedToolOutputValidators.clear();
876+
this._toolOutputValidatorErrors.clear();
877+
}
876878

877879
for (const tool of tools) {
880+
this._cachedToolOutputValidators.delete(tool.name);
881+
this._toolOutputValidatorErrors.delete(tool.name);
882+
878883
// If the tool has an outputSchema, create and cache the validator. Compilation can throw
879884
// (invalid schema, or a SEP-2106 safety-guard rejection); scope that failure to the
880885
// offending tool rather than letting it reject the whole listTools() call.
@@ -926,8 +931,9 @@ export class Client extends Protocol<ClientContext> {
926931
}
927932
const result = await this._requestWithSchema({ method: 'tools/list', params }, ListToolsResultSchema, options);
928933

929-
// Cache the tools and their output schemas for future validation
930-
this.cacheToolMetadata(result.tools);
934+
// Cache the tools and their output schemas for future validation. Preserve entries across
935+
// pagination so validators discovered on earlier pages remain active after the final page.
936+
this.cacheToolMetadata(result.tools, params?.cursor === undefined);
931937

932938
return result;
933939
}

packages/core-internal/src/validators/externalRefResolver.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ function assertHostAllowed(url: URL, options: ResolvedOptions): void {
142142
const host = url.hostname.toLowerCase();
143143

144144
if (options.allowlist) {
145-
if (!options.allowlist.includes(host)) {
145+
const allowlist = new Set(options.allowlist.map(allowedHost => allowedHost.toLowerCase()));
146+
if (!allowlist.has(host)) {
146147
throw new Error(`Refusing to dereference "${url.href}": host "${host}" is not in the allowlist.`);
147148
}
148149
return;

packages/core-internal/test/validators/externalRefResolver.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,20 @@ describe('resolveExternalSchemaRefs', () => {
6060
expect(() => assertSchemaSafeToCompile(resolved)).not.toThrow();
6161
});
6262

63+
it('matches allowlist hosts case-insensitively', async () => {
64+
const schema: JsonSchemaType = { $ref: 'https://Schemas.Example.com/forecast.json' };
65+
const fetchImpl = fetchStub({
66+
'https://schemas.example.com/forecast.json': { type: 'array', items: { type: 'number' } }
67+
});
68+
69+
const resolved = await resolveExternalSchemaRefs(schema, { allowlist: ['Schemas.Example.com'], fetch: fetchImpl });
70+
71+
expect(resolved).toEqual({
72+
$ref: '#/$defs/__externalRef_0',
73+
$defs: { __externalRef_0: { type: 'array', items: { type: 'number' } } }
74+
});
75+
});
76+
6377
it.each([
6478
['AJV', () => new AjvJsonSchemaValidator()],
6579
['CfWorker', () => new CfWorkerJsonSchemaValidator()]

test/integration/test/client/client.test.ts

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,7 +1944,7 @@ describe('outputSchema validation', () => {
19441944
* Test: A single tool with an uncompilable outputSchema does not break listTools()
19451945
* or the use of other tools (SEP-2106 safety-guard isolation).
19461946
*/
1947-
test('isolates a tool whose outputSchema fails to compile from the rest of the server', async () => {
1947+
test('preserves outputSchema validation metadata across paginated tool listings', async () => {
19481948
const server = new Server({ name: 'test-server', version: '1.0.0' }, { capabilities: { tools: {} } });
19491949

19501950
server.setRequestHandler('initialize', async request => ({
@@ -1953,28 +1953,47 @@ describe('outputSchema validation', () => {
19531953
serverInfo: { name: 'test-server', version: '1.0.0' }
19541954
}));
19551955

1956-
server.setRequestHandler('tools/list', async () => ({
1957-
tools: [
1958-
{
1959-
name: 'bad-tool',
1960-
description: 'advertises a non-local $ref the SEP-2106 guard rejects',
1961-
inputSchema: { type: 'object', properties: {} },
1962-
// Non-same-document $ref: assertSchemaSafeToCompile throws when this is compiled.
1963-
outputSchema: { $ref: 'https://evil.example/schema.json' }
1964-
},
1965-
{
1966-
name: 'good-tool',
1967-
description: 'a normal tool that must remain usable',
1968-
inputSchema: { type: 'object', properties: {} },
1969-
outputSchema: { type: 'object', properties: { ok: { type: 'boolean' } }, required: ['ok'] }
1970-
}
1971-
]
1972-
}));
1956+
server.setRequestHandler('tools/list', async request => {
1957+
if (request.params?.cursor === 'page-2') {
1958+
return {
1959+
tools: [
1960+
{
1961+
name: 'last-page-tool',
1962+
description: 'a tool on the final page',
1963+
inputSchema: { type: 'object', properties: {} },
1964+
outputSchema: { type: 'object', properties: { ok: { type: 'boolean' } }, required: ['ok'] }
1965+
}
1966+
]
1967+
};
1968+
}
1969+
1970+
return {
1971+
tools: [
1972+
{
1973+
name: 'bad-tool',
1974+
description: 'advertises a non-local $ref the SEP-2106 guard rejects',
1975+
inputSchema: { type: 'object', properties: {} },
1976+
// Non-same-document $ref: assertSchemaSafeToCompile throws when this is compiled.
1977+
outputSchema: { $ref: 'https://evil.example/schema.json' }
1978+
},
1979+
{
1980+
name: 'validated-tool',
1981+
description: 'a normal earlier-page tool that must keep client-side validation',
1982+
inputSchema: { type: 'object', properties: {} },
1983+
outputSchema: { type: 'object', properties: { ok: { type: 'boolean' } }, required: ['ok'] }
1984+
}
1985+
],
1986+
nextCursor: 'page-2'
1987+
};
1988+
});
19731989

19741990
server.setRequestHandler('tools/call', async request => {
1975-
if (request.params.name === 'good-tool') {
1991+
if (request.params.name === 'last-page-tool') {
19761992
return { content: [], structuredContent: { ok: true } };
19771993
}
1994+
if (request.params.name === 'validated-tool') {
1995+
return { content: [], structuredContent: { ok: 'not-a-boolean' } };
1996+
}
19781997
return { content: [], structuredContent: { irrelevant: true } };
19791998
});
19801999

@@ -1983,14 +2002,19 @@ describe('outputSchema validation', () => {
19832002
await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]);
19842003

19852004
// listTools() must NOT reject just because one tool's schema is uncompilable.
1986-
const listed = await client.listTools();
1987-
expect(listed.tools.map(t => t.name).toSorted()).toEqual(['bad-tool', 'good-tool']);
2005+
const firstPage = await client.listTools();
2006+
expect(firstPage.tools.map(t => t.name).toSorted()).toEqual(['bad-tool', 'validated-tool']);
2007+
const secondPage = await client.listTools({ cursor: firstPage.nextCursor });
2008+
expect(secondPage.tools.map(t => t.name)).toEqual(['last-page-tool']);
2009+
2010+
// The final page's tool is fully usable.
2011+
const lastPage = await client.callTool({ name: 'last-page-tool' });
2012+
expect(lastPage.structuredContent).toEqual({ ok: true });
19882013

1989-
// The good tool is fully usable.
1990-
const good = await client.callTool({ name: 'good-tool' });
1991-
expect(good.structuredContent).toEqual({ ok: true });
2014+
// A valid outputSchema from an earlier page still validates structuredContent after pagination.
2015+
await expect(client.callTool({ name: 'validated-tool' })).rejects.toThrow(/Structured content does not match/);
19922016

1993-
// The bad tool surfaces a scoped, descriptive error only when it is called.
2017+
// An earlier-page schema compile error also survives pagination and is scoped to that tool.
19942018
await expect(client.callTool({ name: 'bad-tool' })).rejects.toThrow(/output schema that could not be compiled/i);
19952019
});
19962020

0 commit comments

Comments
 (0)