Skip to content

Commit d7e3bbd

Browse files
ksinderclaude
ksinder
and
claude
authored
Harden deserializeParams against nested stringified params (#325)
* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent df04d3f commit d7e3bbd

4 files changed

Lines changed: 271 additions & 50 deletions

File tree

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"mcp",
77
"server"
88
],
9-
"version": "2.4.0",
9+
"version": "2.4.1",
1010
"license": "MIT",
1111
"type": "module",
1212
"scripts": {

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

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,5 +834,186 @@ describe('MCPProxy', () => {
834834
},
835835
)
836836
})
837+
838+
it('should handle API-create-a-comment parent provided as a JSON string', async () => {
839+
const mockResponse = {
840+
data: { id: 'new-comment-id' },
841+
status: 200,
842+
headers: new Headers({ 'content-type': 'application/json' }),
843+
}
844+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
845+
846+
;(proxy as any).openApiLookup = {
847+
'API-create-a-comment': {
848+
operationId: 'create-a-comment',
849+
responses: { '200': { description: 'Success' } },
850+
method: 'post',
851+
path: '/comments',
852+
},
853+
}
854+
855+
const server = (proxy as any).server
856+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
857+
const callToolHandler = handlers[1]
858+
859+
// Some clients double-encode `parent` as a JSON string. Forwarding that to
860+
// the Notion API makes it throw on `"block_id" in <string>` and return a
861+
// 500, so deserialize it back to an object first.
862+
const parentAsString = JSON.stringify({ page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' })
863+
864+
await expect(
865+
callToolHandler({
866+
params: {
867+
name: 'API-create-a-comment',
868+
arguments: {
869+
parent: parentAsString,
870+
rich_text: [{ text: { content: 'Hello' } }],
871+
},
872+
},
873+
}),
874+
).resolves.toBeDefined()
875+
876+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
877+
expect.anything(),
878+
expect.objectContaining({
879+
parent: { page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' },
880+
}),
881+
)
882+
})
883+
884+
it('should deserialize a stringified object nested inside an array element object', async () => {
885+
const mockResponse = {
886+
data: { id: 'new-page-id' },
887+
status: 200,
888+
headers: new Headers({ 'content-type': 'application/json' }),
889+
}
890+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
891+
892+
;(proxy as any).openApiLookup = {
893+
'API-appendBlockChildren': {
894+
operationId: 'appendBlockChildren',
895+
responses: { '200': { description: 'Success' } },
896+
method: 'patch',
897+
path: '/blocks/{block_id}/children',
898+
},
899+
}
900+
901+
const server = (proxy as any).server
902+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
903+
const callToolHandler = handlers[1]
904+
905+
// The array element is a real object, but one of its properties is itself
906+
// a stringified object. The previous shallow array handling left this as a
907+
// string; the uniform recursive walk now normalizes it.
908+
const children = [
909+
{
910+
object: 'block',
911+
type: 'paragraph',
912+
paragraph: JSON.stringify({ rich_text: [{ type: 'text', text: { content: 'Hello' } }] }),
913+
},
914+
]
915+
916+
await callToolHandler({
917+
params: {
918+
name: 'API-appendBlockChildren',
919+
arguments: { children },
920+
},
921+
})
922+
923+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
924+
expect.anything(),
925+
{
926+
children: [
927+
{
928+
object: 'block',
929+
type: 'paragraph',
930+
paragraph: { rich_text: [{ type: 'text', text: { content: 'Hello' } }] },
931+
},
932+
],
933+
},
934+
)
935+
})
936+
937+
it('should deserialize a double-stringified parent', async () => {
938+
const mockResponse = {
939+
data: { id: 'new-comment-id' },
940+
status: 200,
941+
headers: new Headers({ 'content-type': 'application/json' }),
942+
}
943+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
944+
945+
;(proxy as any).openApiLookup = {
946+
'API-create-a-comment': {
947+
operationId: 'create-a-comment',
948+
responses: { '200': { description: 'Success' } },
949+
method: 'post',
950+
path: '/comments',
951+
},
952+
}
953+
954+
const server = (proxy as any).server
955+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
956+
const callToolHandler = handlers[1]
957+
958+
// A client that serialized `parent` twice: JSON.stringify(JSON.stringify(parent)).
959+
const doubleEncodedParent = JSON.stringify(JSON.stringify({ page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' }))
960+
961+
await callToolHandler({
962+
params: {
963+
name: 'API-create-a-comment',
964+
arguments: {
965+
parent: doubleEncodedParent,
966+
rich_text: [{ text: { content: 'Hello' } }],
967+
},
968+
},
969+
})
970+
971+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
972+
expect.anything(),
973+
expect.objectContaining({
974+
parent: { page_id: '3870bb29-1a64-816b-8641-c87ca28062d0' },
975+
}),
976+
)
977+
})
978+
979+
it('should not coerce scalar or quoted-scalar string params', async () => {
980+
const mockResponse = {
981+
data: { success: true },
982+
status: 200,
983+
headers: new Headers({ 'content-type': 'application/json' }),
984+
}
985+
;(HttpClient.prototype.executeOperation as ReturnType<typeof vi.fn>).mockResolvedValue(mockResponse)
986+
987+
;(proxy as any).openApiLookup = {
988+
'API-search': {
989+
operationId: 'search',
990+
responses: { '200': { description: 'Success' } },
991+
method: 'post',
992+
path: '/search',
993+
},
994+
}
995+
996+
const server = (proxy as any).server
997+
const handlers = server.setRequestHandler.mock.calls.flatMap((x: unknown[]) => x).filter((x: unknown) => typeof x === 'function')
998+
const callToolHandler = handlers[1]
999+
1000+
await callToolHandler({
1001+
params: {
1002+
name: 'API-search',
1003+
arguments: {
1004+
// Looks like JSON scalars, but the schema wants strings: keep as-is
1005+
// rather than coercing to number/boolean or unwrapping the quotes.
1006+
count: '123',
1007+
flag: 'true',
1008+
quoted: '"hello"',
1009+
},
1010+
},
1011+
})
1012+
1013+
expect(HttpClient.prototype.executeOperation).toHaveBeenCalledWith(
1014+
expect.anything(),
1015+
{ count: '123', flag: 'true', quoted: '"hello"' },
1016+
)
1017+
})
8371018
})
8381019
})

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

Lines changed: 87 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -25,62 +25,102 @@ type NewToolDefinition = {
2525

2626
/**
2727
* Recursively deserialize stringified JSON values in parameters.
28-
* This handles the case where MCP clients (like Cursor, Claude Code) double-serialize
29-
* nested object parameters, sending them as JSON strings instead of objects.
28+
* This handles the case where MCP clients (like Cursor, Claude Code, and some
29+
* SDKs) double-serialize nested object/array parameters, sending them as JSON
30+
* strings instead of structured values.
31+
*
32+
* The whole argument tree is walked uniformly: every object property and every
33+
* array element is visited, JSON-looking strings are decoded, and the decoded
34+
* result is walked again. This normalizes deeply nested cases — including a
35+
* stringified object that sits inside an array element object (e.g.
36+
* `{ children: [{ paragraph: '{"rich_text":[...]}' }] }`) and values that were
37+
* JSON-encoded more than once (e.g. `JSON.stringify(JSON.stringify(parent))`) —
38+
* before the request is forwarded to the Notion API.
3039
*
3140
* @see https://github.com/makenotion/notion-mcp-server/issues/176
3241
*/
3342
function deserializeParams(params: Record<string, unknown>): Record<string, unknown> {
3443
const result: Record<string, unknown> = {}
35-
3644
for (const [key, value] of Object.entries(params)) {
37-
if (typeof value === 'string') {
38-
// Check if the string looks like a JSON object or array
39-
const trimmed = value.trim()
40-
if ((trimmed.startsWith('{') && trimmed.endsWith('}')) ||
41-
(trimmed.startsWith('[') && trimmed.endsWith(']'))) {
42-
try {
43-
const parsed = JSON.parse(value)
44-
// Only use parsed value if it's an object or array
45-
if (typeof parsed === 'object' && parsed !== null) {
46-
// Recursively deserialize nested objects
47-
result[key] = Array.isArray(parsed)
48-
? parsed
49-
: deserializeParams(parsed as Record<string, unknown>)
50-
continue
51-
}
52-
} catch {
53-
// If parsing fails, keep the original string value
54-
}
55-
}
56-
} else if (Array.isArray(value)) {
57-
// Deserialize any JSON-string items within the array
58-
result[key] = value.map((item) => {
59-
if (typeof item !== 'string') return item
60-
const trimmed = item.trim()
61-
if (
62-
(trimmed.startsWith('{') && trimmed.endsWith('}')) ||
63-
(trimmed.startsWith('[') && trimmed.endsWith(']'))
64-
) {
65-
try {
66-
const parsed = JSON.parse(item)
67-
if (typeof parsed === 'object' && parsed !== null) {
68-
return Array.isArray(parsed)
69-
? parsed
70-
: deserializeParams(parsed as Record<string, unknown>)
71-
}
72-
} catch {
73-
// If parsing fails, keep the original string item
74-
}
75-
}
76-
return item
77-
})
78-
continue
45+
result[key] = deserializeValue(value)
46+
}
47+
return result
48+
}
49+
50+
/**
51+
* Normalize a single value: decode a JSON-encoded string into the structured
52+
* value it represents (recursing into the result), walk into every array
53+
* element, and walk into every nested object property. Non-JSON strings and
54+
* scalars are returned unchanged, so values the schema legitimately wants as
55+
* strings (and numbers/booleans encoded as strings) are left intact.
56+
*/
57+
function deserializeValue(value: unknown): unknown {
58+
if (typeof value === 'string') {
59+
return unwrapJsonString(value)
60+
}
61+
62+
if (Array.isArray(value)) {
63+
return value.map(deserializeValue)
64+
}
65+
66+
if (typeof value === 'object' && value !== null) {
67+
const result: Record<string, unknown> = {}
68+
for (const [key, nested] of Object.entries(value)) {
69+
result[key] = deserializeValue(nested)
7970
}
80-
result[key] = value
71+
return result
8172
}
8273

83-
return result
74+
return value
75+
}
76+
77+
// Bound how many JSON-decode passes we attempt on a single string. One pass
78+
// handles the common single-encoding; extra passes absorb double/triple
79+
// serialization without unbounded work on adversarial input.
80+
const MAX_UNWRAP_DEPTH = 3
81+
82+
/**
83+
* Resolve a (possibly multiply-)JSON-encoded string to the object or array it
84+
* represents. Only strings that ultimately decode to an object or array are
85+
* transformed (and then recursively normalized); a string that decodes to a
86+
* scalar (number/boolean/null) or to another plain string is returned
87+
* unchanged, so genuine string values are never corrupted.
88+
*/
89+
function unwrapJsonString(value: string): unknown {
90+
let current = value
91+
for (let depth = 0; depth < MAX_UNWRAP_DEPTH; depth++) {
92+
const trimmed = current.trim()
93+
// Only attempt a parse when the string could encode an object/array
94+
// (`{...}`/`[...]`) or wrap one in a JSON string literal (`"..."`). This
95+
// skips the common case of ordinary text without touching JSON.parse.
96+
const couldBeEncoded =
97+
(trimmed.startsWith('{') && trimmed.endsWith('}')) ||
98+
(trimmed.startsWith('[') && trimmed.endsWith(']')) ||
99+
(trimmed.startsWith('"') && trimmed.endsWith('"'))
100+
if (!couldBeEncoded) {
101+
break
102+
}
103+
104+
let parsed: unknown
105+
try {
106+
parsed = JSON.parse(trimmed)
107+
} catch {
108+
break
109+
}
110+
111+
if (typeof parsed === 'object' && parsed !== null) {
112+
return deserializeValue(parsed)
113+
}
114+
if (typeof parsed === 'string') {
115+
// Peeled one layer of JSON-string encoding; loop to see whether it wraps
116+
// a structured value (double-encoding).
117+
current = parsed
118+
continue
119+
}
120+
// Decoded to a scalar — not a structured value; leave the original intact.
121+
break
122+
}
123+
return value
84124
}
85125

86126
// import this class, extend and return server

0 commit comments

Comments
 (0)