Harden deserializeParams against nested stringified params

package.json

@@ -6,7 +6,7 @@
     "mcp",
     "server"
   ],
-  "version": "2.4.0",
+  "version": "2.4.1",
   "license": "MIT",
   "type": "module",
   "scripts": {

src/openapi-mcp-server/mcp/__tests__/proxy.test.ts

@@ -834,5 +834,186 @@ 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<typeof vi.fn>).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 <string>` 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<typeof vi.fn>).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' } }] },
+            },
+          ],
+        },
+      )
+    })
+
+    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<typeof vi.fn>).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<typeof vi.fn>).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"' },
+      )
+    })
   })
 })

src/openapi-mcp-server/mcp/proxy.ts

@@ -25,62 +25,102 @@ 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 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":[...]}' }] }`) 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
  */
 function deserializeParams(params: Record<string, unknown>): Record<string, unknown> {
   const result: Record<string, unknown> = {}
-
   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<string, unknown>)
-            continue
-          }
-        } catch {
-          // If parsing fails, 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<string, unknown>)
-            }
-          } catch {
-            // If parsing fails, keep the original string item
-          }
-        }
-        return item
-      })
-      continue
+    result[key] = deserializeValue(value)
+  }
+  return result
+}
+
+/**
+ * Normalize a single value: decode a JSON-encoded string into the structured
+ * value it represents (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') {
+    return unwrapJsonString(value)
+  }
+
+  if (Array.isArray(value)) {
+    return value.map(deserializeValue)
+  }
+
+  if (typeof value === 'object' && value !== null) {
+    const result: Record<string, unknown> = {}
+    for (const [key, nested] of Object.entries(value)) {
+      result[key] = deserializeValue(nested)
     }
-    result[key] = value
+    return result
   }
 
-  return result
+  return value
+}
+
+// Bound how many JSON-decode passes we attempt on a single string. One pass
+// handles the common single-encoding; extra passes absorb double/triple
+// serialization without unbounded work on adversarial input.
+const MAX_UNWRAP_DEPTH = 3
+
+/**
+ * Resolve a (possibly multiply-)JSON-encoded string to the object or array it
+ * represents. Only strings that ultimately decode to an object or array are
+ * transformed (and then recursively normalized); a string that decodes to a
+ * scalar (number/boolean/null) or to another plain string is returned
+ * unchanged, so genuine string values are never corrupted.
+ */
+function unwrapJsonString(value: string): unknown {
+  let current = value
+  for (let depth = 0; depth < MAX_UNWRAP_DEPTH; depth++) {
+    const trimmed = current.trim()
+    // Only attempt a parse when the string could encode an object/array
+    // (`{...}`/`[...]`) or wrap one in a JSON string literal (`"..."`). This
+    // skips the common case of ordinary text without touching JSON.parse.
+    const couldBeEncoded =
+      (trimmed.startsWith('{') && trimmed.endsWith('}')) ||
+      (trimmed.startsWith('[') && trimmed.endsWith(']')) ||
+      (trimmed.startsWith('"') && trimmed.endsWith('"'))
+    if (!couldBeEncoded) {
+      break
+    }
+
+    let parsed: unknown
+    try {
+      parsed = JSON.parse(trimmed)
+    } catch {
+      break
+    }
+
+    if (typeof parsed === 'object' && parsed !== null) {
+      return deserializeValue(parsed)
+    }
+    if (typeof parsed === 'string') {
+      // Peeled one layer of JSON-string encoding; loop to see whether it wraps
+      // a structured value (double-encoding).
+      current = parsed
+      continue
+    }
+    // Decoded to a scalar — not a structured value; leave the original intact.
+    break
+  }
+  return value
 }
 
 // import this class, extend and return server