From 9c35b62f55da29febb9f875a3f2fbff2ee08aef4 Mon Sep 17 00:00:00 2001 From: Kenneth Sinder Date: Mon, 22 Jun 2026 10:24:26 -0700 Subject: [PATCH 1/3] Harden deserializeParams against nested stringified params deserializeParams handled top-level stringified params and shallowly parsed array items, but used asymmetric recursion: it descended into object properties yet only string-parsed array items one level deep. A stringified object that was a property of an array element (e.g. `{ children: [{ paragraph: '{"rich_text":[...]}' }] }`) was forwarded to the Notion API as a raw string. Replace it with a single uniform recursive walk (deserializeValue) that visits every object property and array element, parses JSON-looking strings, and recurses into the parsed result. Non-JSON strings and scalar values (including numbers/booleans encoded as strings) are left intact. Add tests for create-a-comment `parent` sent as a JSON string and for a stringified object nested inside an array element object. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcp/__tests__/proxy.test.ts | 99 +++++++++++++++++++ src/openapi-mcp-server/mcp/proxy.ts | 94 +++++++++--------- 2 files changed, 148 insertions(+), 45 deletions(-) diff --git a/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts b/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts index 65cd0a4..bcfd5bc 100644 --- a/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts +++ b/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts @@ -834,5 +834,104 @@ describe('MCPProxy', () => { }, ) }) + + it('should handle API-create-a-comment parent provided as a JSON string', async () => { + const mockResponse = { + data: { id: 'new-comment-id' }, + status: 200, + headers: new Headers({ 'content-type': 'application/json' }), + } + ;(HttpClient.prototype.executeOperation as ReturnType).mockResolvedValue(mockResponse) + + ;(proxy as any).openApiLookup = { + 'API-create-a-comment': { + operationId: 'create-a-comment', + responses: { '200': { description: 'Success' } }, + method: 'post', + path: '/comments', + }, + } + + const server = (proxy as any).server + const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function') + const callToolHandler = handlers[1] + + // Some clients double-encode `parent` as a JSON string. Forwarding that to + // the Notion API makes it throw on `"block_id" in ` and return a + // 500, so deserialize it back to an object first. + const parentAsString = JSON.stringify({ page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' }) + + await expect( + callToolHandler({ + params: { + name: 'API-create-a-comment', + arguments: { + parent: parentAsString, + rich_text: [{ text: { content: 'Hello' } }], + }, + }, + }), + ).resolves.toBeDefined() + + expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + parent: { page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' }, + }), + ) + }) + + it('should deserialize a stringified object nested inside an array element object', async () => { + const mockResponse = { + data: { id: 'new-page-id' }, + status: 200, + headers: new Headers({ 'content-type': 'application/json' }), + } + ;(HttpClient.prototype.executeOperation as ReturnType).mockResolvedValue(mockResponse) + + ;(proxy as any).openApiLookup = { + 'API-appendBlockChildren': { + operationId: 'appendBlockChildren', + responses: { '200': { description: 'Success' } }, + method: 'patch', + path: '/blocks/{block_id}/children', + }, + } + + const server = (proxy as any).server + const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function') + const callToolHandler = handlers[1] + + // The array element is a real object, but one of its properties is itself + // a stringified object. The previous shallow array handling left this as a + // string; the uniform recursive walk now normalizes it. + const children = [ + { + object: 'block', + type: 'paragraph', + paragraph: JSON.stringify({ rich_text: [{ type: 'text', text: { content: 'Hello' } }] }), + }, + ] + + await callToolHandler({ + params: { + name: 'API-appendBlockChildren', + arguments: { children }, + }, + }) + + expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith( + expect.anything(), + { + children: [ + { + object: 'block', + type: 'paragraph', + paragraph: { rich_text: [{ type: 'text', text: { content: 'Hello' } }] }, + }, + ], + }, + ) + }) }) }) diff --git a/src/openapi-mcp-server/mcp/proxy.ts b/src/openapi-mcp-server/mcp/proxy.ts index 7469f88..d18aade 100644 --- a/src/openapi-mcp-server/mcp/proxy.ts +++ b/src/openapi-mcp-server/mcp/proxy.ts @@ -25,62 +25,66 @@ type NewToolDefinition = { /** * Recursively deserialize stringified JSON values in parameters. - * This handles the case where MCP clients (like Cursor, Claude Code) double-serialize - * nested object parameters, sending them as JSON strings instead of objects. + * This handles the case where MCP clients (like Cursor, Claude Code, and some + * SDKs) double-serialize nested object/array parameters, sending them as JSON + * strings instead of structured values. + * + * The whole argument tree is walked uniformly: every object property and every + * array element is visited, JSON-looking strings are parsed, and the parsed + * result is walked again. This normalizes deeply nested cases — including a + * stringified object that sits inside an array element object (e.g. + * `{ children: [{ paragraph: '{"rich_text":[...]}' }] }`) — before the request + * is forwarded to the Notion API. * * @see https://github.com/makenotion/notion-mcp-server/issues/176 */ function deserializeParams(params: Record): Record { const result: Record = {} - for (const [key, value] of Object.entries(params)) { - if (typeof value === 'string') { - // Check if the string looks like a JSON object or array - const trimmed = value.trim() - if ((trimmed.startsWith('{') && trimmed.endsWith('}')) || - (trimmed.startsWith('[') && trimmed.endsWith(']'))) { - try { - const parsed = JSON.parse(value) - // Only use parsed value if it's an object or array - if (typeof parsed === 'object' && parsed !== null) { - // Recursively deserialize nested objects - result[key] = Array.isArray(parsed) - ? parsed - : deserializeParams(parsed as Record) - continue - } - } catch { - // If parsing fails, keep the original string value + result[key] = deserializeValue(value) + } + return result +} + +/** + * Normalize a single value: parse a JSON-object/array-looking string into a + * structured value (recursing into the result), walk into every array element, + * and walk into every nested object property. Non-JSON strings and scalars are + * returned unchanged, so values the schema legitimately wants as strings (and + * numbers/booleans encoded as strings) are left intact. + */ +function deserializeValue(value: unknown): unknown { + if (typeof value === 'string') { + const trimmed = value.trim() + const looksLikeJsonObjectOrArray = + (trimmed.startsWith('{') && trimmed.endsWith('}')) || + (trimmed.startsWith('[') && trimmed.endsWith(']')) + if (looksLikeJsonObjectOrArray) { + try { + const parsed: unknown = JSON.parse(value) + if (typeof parsed === 'object' && parsed !== null) { + return deserializeValue(parsed) } + } catch { + // Not valid JSON after all; keep the original string value. } - } else if (Array.isArray(value)) { - // Deserialize any JSON-string items within the array - result[key] = value.map((item) => { - if (typeof item !== 'string') return item - const trimmed = item.trim() - if ( - (trimmed.startsWith('{') && trimmed.endsWith('}')) || - (trimmed.startsWith('[') && trimmed.endsWith(']')) - ) { - try { - const parsed = JSON.parse(item) - if (typeof parsed === 'object' && parsed !== null) { - return Array.isArray(parsed) - ? parsed - : deserializeParams(parsed as Record) - } - } catch { - // If parsing fails, keep the original string item - } - } - return item - }) - continue } - result[key] = value + return value } - return result + if (Array.isArray(value)) { + return value.map(deserializeValue) + } + + if (typeof value === 'object' && value !== null) { + const result: Record = {} + for (const [key, nested] of Object.entries(value)) { + result[key] = deserializeValue(nested) + } + return result + } + + return value } // import this class, extend and return server From 4f7ffb14cfbb6ddb027f310143f7c8837836e590 Mon Sep 17 00:00:00 2001 From: Kenneth Sinder Date: Mon, 22 Jun 2026 10:46:19 -0700 Subject: [PATCH 2/3] Decode multiply-encoded JSON string params Extend deserializeValue to resolve values that were JSON-encoded more than once (e.g. JSON.stringify(JSON.stringify(parent))) via a bounded unwrapJsonString helper. A string is only transformed when it ultimately decodes to an object or array; strings that decode to scalars (numbers/booleans/null) or to other plain strings are returned unchanged, so genuine string values are never coerced or unwrapped. Add tests for a double-stringified parent and for scalar/quoted-scalar string params being preserved. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../mcp/__tests__/proxy.test.ts | 82 +++++++++++++++++++ src/openapi-mcp-server/mcp/proxy.ts | 82 +++++++++++++------ 2 files changed, 141 insertions(+), 23 deletions(-) diff --git a/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts b/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts index bcfd5bc..8e48d49 100644 --- a/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts +++ b/src/openapi-mcp-server/mcp/__tests__/proxy.test.ts @@ -933,5 +933,87 @@ describe('MCPProxy', () => { }, ) }) + + it('should deserialize a double-stringified parent', async () => { + const mockResponse = { + data: { id: 'new-comment-id' }, + status: 200, + headers: new Headers({ 'content-type': 'application/json' }), + } + ;(HttpClient.prototype.executeOperation as ReturnType).mockResolvedValue(mockResponse) + + ;(proxy as any).openApiLookup = { + 'API-create-a-comment': { + operationId: 'create-a-comment', + responses: { '200': { description: 'Success' } }, + method: 'post', + path: '/comments', + }, + } + + const server = (proxy as any).server + const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function') + const callToolHandler = handlers[1] + + // A client that serialized `parent` twice: JSON.stringify(JSON.stringify(parent)). + const doubleEncodedParent = JSON.stringify(JSON.stringify({ page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' })) + + await callToolHandler({ + params: { + name: 'API-create-a-comment', + arguments: { + parent: doubleEncodedParent, + rich_text: [{ text: { content: 'Hello' } }], + }, + }, + }) + + expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + parent: { page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' }, + }), + ) + }) + + it('should not coerce scalar or quoted-scalar string params', async () => { + const mockResponse = { + data: { success: true }, + status: 200, + headers: new Headers({ 'content-type': 'application/json' }), + } + ;(HttpClient.prototype.executeOperation as ReturnType).mockResolvedValue(mockResponse) + + ;(proxy as any).openApiLookup = { + 'API-search': { + operationId: 'search', + responses: { '200': { description: 'Success' } }, + method: 'post', + path: '/search', + }, + } + + const server = (proxy as any).server + const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function') + const callToolHandler = handlers[1] + + await callToolHandler({ + params: { + name: 'API-search', + arguments: { + // Looks like JSON scalars, but the schema wants strings: keep as-is + // rather than coercing to number/boolean or unwrapping the quotes. + count: '123', + flag: 'true', + quoted: '"hello"', + }, + }, + }) + + expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith( + expect.anything(), + { count: '123', flag: 'true', quoted: '"hello"' }, + ) + }) }) }) diff --git a/src/openapi-mcp-server/mcp/proxy.ts b/src/openapi-mcp-server/mcp/proxy.ts index d18aade..0ff8004 100644 --- a/src/openapi-mcp-server/mcp/proxy.ts +++ b/src/openapi-mcp-server/mcp/proxy.ts @@ -30,11 +30,12 @@ type NewToolDefinition = { * strings instead of structured values. * * The whole argument tree is walked uniformly: every object property and every - * array element is visited, JSON-looking strings are parsed, and the parsed + * array element is visited, JSON-looking strings are decoded, and the decoded * result is walked again. This normalizes deeply nested cases — including a * stringified object that sits inside an array element object (e.g. - * `{ children: [{ paragraph: '{"rich_text":[...]}' }] }`) — before the request - * is forwarded to the Notion API. + * `{ children: [{ paragraph: '{"rich_text":[...]}' }] }`) and values that were + * JSON-encoded more than once (e.g. `JSON.stringify(JSON.stringify(parent))`) — + * before the request is forwarded to the Notion API. * * @see https://github.com/makenotion/notion-mcp-server/issues/176 */ @@ -47,29 +48,15 @@ function deserializeParams(params: Record): Record Date: Mon, 22 Jun 2026 10:58:01 -0700 Subject: [PATCH 3/3] Bump version to v2.4.1 Publishes the deserializeParams hardening on merge via the Publish workflow (OIDC trusted publishing). Co-Authored-By: Claude Opus 4.8 (1M context) --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index cd2fae1..91f0c5b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@notionhq/notion-mcp-server", - "version": "2.4.0", + "version": "2.4.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@notionhq/notion-mcp-server", - "version": "2.4.0", + "version": "2.4.1", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.25.1", diff --git a/package.json b/package.json index 899fe7e..97d1d0a 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "mcp", "server" ], - "version": "2.4.0", + "version": "2.4.1", "license": "MIT", "type": "module", "scripts": {