From 078231ed431aacaa541a04c5bf11440e66e8fcd4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:41:52 +0000 Subject: [PATCH 1/4] fix(server): reject batch messages with non-string ids BatchHandlerPlugin validated untrusted batch JSON with isClientPeerSendMessage alone, which assumes isPeerMessage already ran and never checks id or binary. Ids like true, {}, [], null, numbers or a missing id were accepted, the procedure ran and the id was echoed back; a deeply nested array id made the adapter's JSON.stringify throw a RangeError. Require isPeerMessage as well, like the message-port handler does, so such batches get a 400. Similar to the tRPC fix in trpc/trpc#7477. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X6CM5zP1QBSVRmibkRqzK3 --- ...esponse-compression-handler-plugin.test.ts | 4 +- packages/server/src/plugins/batch.test.ts | 169 +++++++++++++----- packages/server/src/plugins/batch.ts | 6 +- .../get-method-csrf-protection.test.ts | 4 +- 4 files changed, 129 insertions(+), 54 deletions(-) diff --git a/packages/node/src/batch-response-compression-handler-plugin.test.ts b/packages/node/src/batch-response-compression-handler-plugin.test.ts index 20cdd82a05..ea04eac349 100644 --- a/packages/node/src/batch-response-compression-handler-plugin.test.ts +++ b/packages/node/src/batch-response-compression-handler-plugin.test.ts @@ -22,9 +22,9 @@ function createBatchRequest( return new Request('https://example.com/__batch__', { method: 'POST', headers: { ...headers, 'content-type': 'application/json', 'orpc-batch': '1' }, - body: JSON.stringify(urls.map((url, id) => ({ + body: JSON.stringify(urls.map((url, index) => ({ kind: 'request', - id, + id: String(index), json: { method: 'POST', url, headers: {}, body: undefined }, binary: undefined, }))), diff --git a/packages/server/src/plugins/batch.test.ts b/packages/server/src/plugins/batch.test.ts index 49ea27ebfe..0f93703ace 100644 --- a/packages/server/src/plugins/batch.test.ts +++ b/packages/server/src/plugins/batch.test.ts @@ -15,7 +15,7 @@ beforeEach(() => { vi.clearAllMocks() }) -function makePeerRequestMessage(id: number | string, url: string, method = 'POST', body?: unknown) { +function makePeerRequestMessage(id: string, url: string, method = 'POST', body?: unknown) { return { kind: 'request', id, @@ -24,6 +24,11 @@ function makePeerRequestMessage(id: number | string, url: string, method = 'POST } } +/** + * An array nested deep enough that `JSON.stringify` overflows the stack on it, written as JSON by hand for that reason. + */ +const DEEPLY_NESTED_ARRAY_JSON = `${'['.repeat(10_000)}${']'.repeat(10_000)}` + function createBatchRequest(options: { messages?: unknown method?: 'POST' | 'GET' | 'QUERY' @@ -135,8 +140,8 @@ describe('batchHandlerPlugin', () => { it('handles batch POST requests', async () => { const handler = createHandler() const peerMessages = [ - makePeerRequestMessage(0, '/ping'), - makePeerRequestMessage(1, '/ping'), + makePeerRequestMessage('0', '/ping'), + makePeerRequestMessage('1', '/ping'), ] const { matched, response } = await handler.handle(createBatchRequest({ @@ -163,8 +168,8 @@ describe('batchHandlerPlugin', () => { const handler = createHandler(new BatchHandlerPlugin(), binaryRouter) const peerMessages = [ - makePeerRequestMessage(0, '/file'), - makePeerRequestMessage(1, '/ping'), + makePeerRequestMessage('0', '/file'), + makePeerRequestMessage('1', '/ping'), ] const { response } = await handler.handle(createBatchRequest({ @@ -186,7 +191,7 @@ describe('batchHandlerPlugin', () => { const handler = createHandler() const { matched, response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/nonexistent')], + messages: [makePeerRequestMessage('0', '/nonexistent')], })) expect(matched).toBe(true) @@ -194,7 +199,7 @@ describe('batchHandlerPlugin', () => { const body = await response!.json() as any expect(body).toHaveLength(1) - expect(body[0]).toMatchObject({ kind: 'response', id: 0 }) + expect(body[0]).toMatchObject({ kind: 'response', id: '0' }) expect(body[0].json.status).toBe(404) }) @@ -210,6 +215,39 @@ describe('batchHandlerPlugin', () => { expect(handlerFn).toHaveBeenCalledTimes(0) }) + it.each([ + ['true', true], + ['an object', {}], + ['an array', []], + ['null', null], + ['a number', 0], + ['missing', undefined], + ])('returns 400 for a batch body whose message id is %s', async (_, id) => { + const handler = createHandler() + + const { response } = await handler.handle(createBatchRequest({ + messages: [{ ...makePeerRequestMessage('0', '/ping'), id }], + })) + + expect(response!.status).toBe(400) + expect(await response!.text()).toContain('Invalid batch request body') + expect(handlerFn).toHaveBeenCalledTimes(0) + }) + + it('returns 400 for a batch body whose message id is a deeply nested array', async () => { + const handler = createHandler() + + const { response } = await handler.handle(new Request('https://example.com/__batch__', { + method: 'POST', + headers: { 'orpc-batch': '1', 'content-type': 'application/json' }, + body: `[{"id":${DEEPLY_NESTED_ARRAY_JSON},"kind":"request","json":{"url":"/ping"}}]`, + })) + + expect(response!.status).toBe(400) + expect(await response!.text()).toContain('Invalid batch request body') + expect(handlerFn).toHaveBeenCalledTimes(0) + }) + it('returns the ORPCError message a body plugin throws while resolving the batch body', async () => { const handler = new RPCHandler(router, { plugins: [new BatchHandlerPlugin(), new RequestCompressionHandlerPlugin()], @@ -222,7 +260,7 @@ describe('batchHandlerPlugin', () => { 'content-type': 'application/json', 'content-encoding': 'gzip, gzip, gzip, gzip, gzip, gzip', }, - body: JSON.stringify([makePeerRequestMessage(0, '/ping')]), + body: JSON.stringify([makePeerRequestMessage('0', '/ping')]), })) expect(response!.status).toBe(400) @@ -236,7 +274,7 @@ describe('batchHandlerPlugin', () => { }) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/ping')], + messages: [makePeerRequestMessage('0', '/ping')], })) expect(response!.status).toBe(400) @@ -247,8 +285,8 @@ describe('batchHandlerPlugin', () => { it('returns 413 when batch size exceeds maxSize', async () => { const handler = createHandler(new BatchHandlerPlugin({ maxSize: 1 })) const peerMessages = [ - makePeerRequestMessage(0, '/ping'), - makePeerRequestMessage(1, '/ping'), + makePeerRequestMessage('0', '/ping'), + makePeerRequestMessage('1', '/ping'), ] const { response } = await handler.handle(createBatchRequest({ @@ -274,7 +312,7 @@ describe('batchHandlerPlugin', () => { })) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/ping')], + messages: [makePeerRequestMessage('0', '/ping')], })) expect(response!.status).toBe(207) @@ -300,7 +338,7 @@ describe('batchHandlerPlugin', () => { })) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/ping')], + messages: [makePeerRequestMessage('0', '/ping')], })) expect(response!.status).toBe(207) @@ -407,12 +445,12 @@ describe('batchHandlerPlugin', () => { }), }) - const messages = [makePeerRequestMessage(0, '/wait'), makePeerRequestMessage(1, '/subscribe')] + const messages = [makePeerRequestMessage('0', '/wait'), makePeerRequestMessage('1', '/subscribe')] it.each([ // The stream sub-response waits for `wait`, so its generator never starts ['with', messages, 1], - ['without', [makePeerRequestMessage(0, '/wait'), makePeerRequestMessage(1, '/wait')], 2], + ['without', [makePeerRequestMessage('0', '/wait'), makePeerRequestMessage('1', '/wait')], 2], ] as const)('aborts sub-requests %s a stream sub-response when the batch request is aborted', async (_, batchMessages, running) => { const controller = new AbortController() const result = handler.handle(createBatchRequest({ messages: batchMessages, signal: controller.signal })) @@ -429,7 +467,7 @@ describe('batchHandlerPlugin', () => { it('aborts sub-requests when the response stream is cancelled', async () => { const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/subscribe'), makePeerRequestMessage(1, '/subscribe')], + messages: [makePeerRequestMessage('0', '/subscribe'), makePeerRequestMessage('1', '/subscribe')], })) await vi.waitFor(() => expect(started).toHaveBeenCalledTimes(2)) @@ -440,7 +478,7 @@ describe('batchHandlerPlugin', () => { it('aborts sub-requests when the batch request is already aborted', async () => { const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/wait')], + messages: [makePeerRequestMessage('0', '/wait')], signal: AbortSignal.abort(), })) await response!.arrayBuffer() @@ -453,7 +491,7 @@ describe('batchHandlerPlugin', () => { it('handles batch GET requests via query param', async () => { const handler = createHandler() const data = encodeURIComponent(JSON.stringify([ - makePeerRequestMessage(0, '/ping', 'GET'), + makePeerRequestMessage('0', '/ping', 'GET'), ])) const { matched, response } = await handler.handle(createBatchRequest({ @@ -500,11 +538,48 @@ describe('batchHandlerPlugin', () => { expect(await response!.text()).toContain('Invalid batch request data parameter') }) + it.each([ + ['true', true], + ['an object', {}], + ['an array', []], + ['null', null], + ['a number', 0], + ['missing', undefined], + ])('returns 400 for GET batch data whose message id is %s', async (_, id) => { + const handler = createHandler() + const data = encodeURIComponent(JSON.stringify([ + { ...makePeerRequestMessage('0', '/ping', 'GET'), id }, + ])) + + const { response } = await handler.handle(createBatchRequest({ + method: 'GET', + data, + })) + + expect(response!.status).toBe(400) + expect(await response!.text()).toContain('Invalid batch request data parameter') + expect(handlerFn).toHaveBeenCalledTimes(0) + }) + + it('returns 400 for GET batch data whose message id is a deeply nested array', async () => { + const handler = createHandler() + const data = encodeURIComponent(`[{"id":${DEEPLY_NESTED_ARRAY_JSON},"kind":"request","json":{"method":"GET","url":"/ping"}}]`) + + const { response } = await handler.handle(createBatchRequest({ + method: 'GET', + data, + })) + + expect(response!.status).toBe(400) + expect(await response!.text()).toContain('Invalid batch request data parameter') + expect(handlerFn).toHaveBeenCalledTimes(0) + }) + it('returns 400 when a GET batch contains a non-GET sub-request', async () => { const handler = createHandler() const data = encodeURIComponent(JSON.stringify([ - makePeerRequestMessage(0, '/ping', 'GET'), - makePeerRequestMessage(1, '/ping', 'POST'), + makePeerRequestMessage('0', '/ping', 'GET'), + makePeerRequestMessage('1', '/ping', 'POST'), ])) const { response } = await handler.handle(createBatchRequest({ @@ -520,7 +595,7 @@ describe('batchHandlerPlugin', () => { it('returns 400 when a GET batch sub-request omits the method (defaults to POST)', async () => { const handler = createHandler() const data = encodeURIComponent(JSON.stringify([ - { kind: 'request', id: 0, json: { url: '/ping', headers: {} } }, + { kind: 'request', id: '0', json: { url: '/ping', headers: {} } }, ])) const { response } = await handler.handle(createBatchRequest({ @@ -541,7 +616,7 @@ describe('batchHandlerPlugin', () => { const { response } = await handler.handle(createBatchRequest({ method: 'QUERY', messages: [ - { kind: 'response', id: 0, json: { method: 'POST', url: '/ping', headers: {} } }, + { kind: 'response', id: '0', json: { method: 'POST', url: '/ping', headers: {} } }, ], })) @@ -555,8 +630,8 @@ describe('batchHandlerPlugin', () => { const { response } = await handler.handle(createBatchRequest({ method: 'QUERY', messages: [ - makePeerRequestMessage(0, '/ping', 'QUERY'), - makePeerRequestMessage(1, '/ping', 'POST'), + makePeerRequestMessage('0', '/ping', 'QUERY'), + makePeerRequestMessage('1', '/ping', 'POST'), ], })) @@ -586,7 +661,7 @@ describe('batchHandlerPlugin', () => { method: 'POST', headers: { 'orpc-batch': '1', 'content-type': 'application/json', ...headers }, body: JSON.stringify([ - { kind: 'request', id: 0, json: { method: 'POST', url: '/ping', headers: subRequestHeaders }, binary: undefined }, + { kind: 'request', id: '0', json: { method: 'POST', url: '/ping', headers: subRequestHeaders }, binary: undefined }, ]), }) } @@ -657,7 +732,7 @@ describe('batchHandlerPlugin', () => { it('moves cookies and shared headers onto the batch response', async () => { const { response } = await createHandler(undefined, headersRouter).handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, '/refresh')], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', '/refresh')], })) expect(response!.status).toBe(207) @@ -667,8 +742,8 @@ describe('batchHandlerPlugin', () => { expect(response!.headers.get('content-language')).toBe(null) const body = await response!.json() as any[] - expect(body.find(m => m.id === 0).json.headers).toEqual({ 'content-language': 'en', 'x-only': 'login' }) - expect(body.find(m => m.id === 1).json.headers).toEqual({ 'content-language': 'en' }) + expect(body.find(m => m.id === '0').json.headers).toEqual({ 'content-language': 'en', 'x-only': 'login' }) + expect(body.find(m => m.id === '1').json.headers).toEqual({ 'content-language': 'en' }) }) it.each([ @@ -676,14 +751,14 @@ describe('batchHandlerPlugin', () => { ['lacks it', '/ping'], ])('keeps a header on the sub-responses when one of them %s', async (_, url) => { const { response } = await createHandler(undefined, headersRouter).handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, url)], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', url)], })) expect(response!.headers.getSetCookie()).toEqual(['session=abc; HttpOnly', 'theme=dark']) expect(response!.headers.get('cache-control')).toBe(null) const body = await response!.json() as any[] - expect(body.find(m => m.id === 0).json.headers['cache-control']).toBe('max-age=60') + expect(body.find(m => m.id === '0').json.headers['cache-control']).toBe('max-age=60') }) it('merges cookies with, and keeps shared headers off, the ones the headers option sets', async () => { @@ -692,7 +767,7 @@ describe('batchHandlerPlugin', () => { }), headersRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, '/refresh')], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', '/refresh')], })) expect(response!.headers.getSetCookie()).toEqual(['batch=1', 'session=abc; HttpOnly', 'theme=dark', 'token=xyz; HttpOnly']) @@ -706,13 +781,13 @@ describe('batchHandlerPlugin', () => { const handler = createHandler(new BatchHandlerPlugin({ headers: { 'x-only': 'login' } }), headersRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, '/refresh')], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', '/refresh')], })) expect(response!.headers.get('x-only')).toBe('login') const body = await response!.json() as any[] - expect(body.find(m => m.id === 0).json.headers).toEqual({ 'content-language': 'en' }) + expect(body.find(m => m.id === '0').json.headers).toEqual({ 'content-language': 'en' }) }) it('lets mapSubresponse keep headers on each sub-response, cookies included', async () => { @@ -720,7 +795,7 @@ describe('batchHandlerPlugin', () => { const handler = createHandler(new BatchHandlerPlugin({ mapSubresponse }), headersRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, '/refresh')], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', '/refresh')], })) // The batch response still carries them @@ -728,7 +803,7 @@ describe('batchHandlerPlugin', () => { expect(response!.headers.get('cache-control')).toBe('max-age=60') const body = await response!.json() as any[] - expect(body.find(m => m.id === 1).json.headers).toEqual({ + expect(body.find(m => m.id === '1').json.headers).toEqual({ 'set-cookie': 'token=xyz; HttpOnly', 'cache-control': 'max-age=60', 'content-language': 'en', @@ -746,7 +821,7 @@ describe('batchHandlerPlugin', () => { const handler = createHandler(new BatchHandlerPlugin({ headers: { 'cache-control': undefined } }), headersRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, '/refresh')], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', '/refresh')], })) expect(response!.headers.get('cache-control')).toBe(null) @@ -775,9 +850,9 @@ describe('batchHandlerPlugin', () => { const { response } = await handler.handle(createBatchRequest({ messages: [ - makePeerRequestMessage(0, '/localized'), - makePeerRequestMessage(1, '/personal'), - makePeerRequestMessage(2, '/shared'), + makePeerRequestMessage('0', '/localized'), + makePeerRequestMessage('1', '/personal'), + makePeerRequestMessage('2', '/shared'), ], })) @@ -798,7 +873,7 @@ describe('batchHandlerPlugin', () => { }) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/any'), makePeerRequestMessage(1, '/localized')], + messages: [makePeerRequestMessage('0', '/any'), makePeerRequestMessage('1', '/localized')], })) expect(response!.headers.get('vary')).toBe('*') @@ -808,7 +883,7 @@ describe('batchHandlerPlugin', () => { const handler = createHandler(new BatchHandlerPlugin({ headers: { 'set-cookie': 'theme=dark' } }), headersRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/login'), makePeerRequestMessage(1, '/login')], + messages: [makePeerRequestMessage('0', '/login'), makePeerRequestMessage('1', '/login')], })) expect(response!.headers.getSetCookie()).toEqual(['theme=dark', 'session=abc; HttpOnly']) @@ -849,7 +924,7 @@ describe('batchHandlerPlugin', () => { describe('configuration options', () => { it('supports custom successStatus', async () => { const handler = createHandler(new BatchHandlerPlugin({ successStatus: 200 })) - const peerMessages = [makePeerRequestMessage(0, '/ping')] + const peerMessages = [makePeerRequestMessage('0', '/ping')] const { response } = await handler.handle(createBatchRequest({ messages: peerMessages, @@ -889,7 +964,7 @@ describe('batchHandlerPlugin', () => { }) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/events')], + messages: [makePeerRequestMessage('0', '/events')], })) const reader = (response!.body as ReadableStream).getReader() @@ -916,7 +991,7 @@ describe('batchHandlerPlugin', () => { }), eventsRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/events')], + messages: [makePeerRequestMessage('0', '/events')], })) expect(response!.body).toBeInstanceOf(ReadableStream) @@ -933,7 +1008,7 @@ describe('batchHandlerPlugin', () => { }), idleRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/idle')], + messages: [makePeerRequestMessage('0', '/idle')], })) expect(vi.getTimerCount()).toBeGreaterThan(0) @@ -978,7 +1053,7 @@ describe('batchHandlerPlugin', () => { }), idleRouter) await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/idle')], + messages: [makePeerRequestMessage('0', '/idle')], })) // One keep-alive setInterval is scheduled while the stream is idle. @@ -1003,7 +1078,7 @@ describe('batchHandlerPlugin', () => { }), eventsRouter) const { response } = await handler.handle(createBatchRequest({ - messages: [makePeerRequestMessage(0, '/events')], + messages: [makePeerRequestMessage('0', '/events')], })) const reader = (response!.body as ReadableStream).getReader() diff --git a/packages/server/src/plugins/batch.ts b/packages/server/src/plugins/batch.ts index 3f9583c892..e5ec036130 100644 --- a/packages/server/src/plugins/batch.ts +++ b/packages/server/src/plugins/batch.ts @@ -7,7 +7,7 @@ import { ORPCError } from '@orpc/client' import { BATCH_HEADER, DEFAULT_BATCH_MAX_SIZE } from '@orpc/client/plugins' import { isAsyncIteratorObject, promiseWithResolvers, toArray, value } from '@orpc/shared' import { flattenStandardHeader, parseStandardUrl } from '@standard-server/core' -import { encodePeerMessage, isClientPeerSendMessage, ServerPeer } from '@standard-server/peer' +import { encodePeerMessage, isClientPeerSendMessage, isPeerMessage, ServerPeer } from '@standard-server/peer' /** * Content type for batch responses that use the length-prefixed binary framing, @@ -271,7 +271,7 @@ export class BatchHandlerPlugin implements StandardHandlerPlu const mightBeMessages = JSON.parse(data) - if (!Array.isArray(mightBeMessages) || mightBeMessages.some(m => !isClientPeerSendMessage(m))) { + if (!Array.isArray(mightBeMessages) || mightBeMessages.some(m => !isPeerMessage(m) || !isClientPeerSendMessage(m))) { return { matched: true, response: { status: 400, headers: {}, body: 'Invalid batch request data parameter' }, @@ -283,7 +283,7 @@ export class BatchHandlerPlugin implements StandardHandlerPlu else { const mightBeMessages = await interceptorOptions.request.resolveBody() - if (!Array.isArray(mightBeMessages) || mightBeMessages.some(m => !isClientPeerSendMessage(m))) { + if (!Array.isArray(mightBeMessages) || mightBeMessages.some(m => !isPeerMessage(m) || !isClientPeerSendMessage(m))) { return { matched: true, response: { status: 400, headers: {}, body: 'Invalid batch request body' }, diff --git a/packages/server/src/plugins/get-method-csrf-protection.test.ts b/packages/server/src/plugins/get-method-csrf-protection.test.ts index d9ccde8745..d43e1013e5 100644 --- a/packages/server/src/plugins/get-method-csrf-protection.test.ts +++ b/packages/server/src/plugins/get-method-csrf-protection.test.ts @@ -211,7 +211,7 @@ describe('getMethodCsrfProtectionHandlerPlugin', () => { function createBatchRequest(handler: RPCHandler, transportHeaders: Record) { const data = encodeURIComponent(JSON.stringify([{ kind: 'request', - id: 0, + id: '0', json: { method: 'GET', url: '/deletePlanet', @@ -230,7 +230,7 @@ describe('getMethodCsrfProtectionHandlerPlugin', () => { const { response } = await createBatchRequest(createHandler([new BatchHandlerPlugin()]), CROSS_SITE_NAVIGATION) const body = await response!.json() as any - expect(body[0]).toMatchObject({ kind: 'response', id: 0 }) + expect(body[0]).toMatchObject({ kind: 'response', id: '0' }) expect(body[0].json.status).toBe(403) expect(deletePlanet).not.toHaveBeenCalled() }) From da41a80c32aa903c45574e5d8d1768a5c2c94dd7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 13:17:19 +0000 Subject: [PATCH 2/4] refactor(server): share the batch message guard Move the duplicated batch validation into isClientPeerSendMessages. Its callback takes unknown, so tsc now rejects calling isClientPeerSendMessage without isPeerMessage first. Share the invalid-id test cases between the POST and GET tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X6CM5zP1QBSVRmibkRqzK3 --- packages/server/src/plugins/batch.test.ts | 27 +++++++++-------------- packages/server/src/plugins/batch.ts | 12 ++++++++-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/packages/server/src/plugins/batch.test.ts b/packages/server/src/plugins/batch.test.ts index 0f93703ace..604378865a 100644 --- a/packages/server/src/plugins/batch.test.ts +++ b/packages/server/src/plugins/batch.test.ts @@ -29,6 +29,15 @@ function makePeerRequestMessage(id: string, url: string, method = 'POST', body?: */ const DEEPLY_NESTED_ARRAY_JSON = `${'['.repeat(10_000)}${']'.repeat(10_000)}` +const INVALID_MESSAGE_IDS = [ + ['true', true], + ['an object', {}], + ['an array', []], + ['null', null], + ['a number', 0], + ['missing', undefined], +] as const + function createBatchRequest(options: { messages?: unknown method?: 'POST' | 'GET' | 'QUERY' @@ -215,14 +224,7 @@ describe('batchHandlerPlugin', () => { expect(handlerFn).toHaveBeenCalledTimes(0) }) - it.each([ - ['true', true], - ['an object', {}], - ['an array', []], - ['null', null], - ['a number', 0], - ['missing', undefined], - ])('returns 400 for a batch body whose message id is %s', async (_, id) => { + it.each(INVALID_MESSAGE_IDS)('returns 400 for a batch body whose message id is %s', async (_, id) => { const handler = createHandler() const { response } = await handler.handle(createBatchRequest({ @@ -538,14 +540,7 @@ describe('batchHandlerPlugin', () => { expect(await response!.text()).toContain('Invalid batch request data parameter') }) - it.each([ - ['true', true], - ['an object', {}], - ['an array', []], - ['null', null], - ['a number', 0], - ['missing', undefined], - ])('returns 400 for GET batch data whose message id is %s', async (_, id) => { + it.each(INVALID_MESSAGE_IDS)('returns 400 for GET batch data whose message id is %s', async (_, id) => { const handler = createHandler() const data = encodeURIComponent(JSON.stringify([ { ...makePeerRequestMessage('0', '/ping', 'GET'), id }, diff --git a/packages/server/src/plugins/batch.ts b/packages/server/src/plugins/batch.ts index e5ec036130..9fe4de3934 100644 --- a/packages/server/src/plugins/batch.ts +++ b/packages/server/src/plugins/batch.ts @@ -43,6 +43,14 @@ function isBodyHeader(name: string): boolean { return name.startsWith('content-') || name === 'standard-server' } +/** + * Whether untrusted batch JSON is a list of client peer messages. + * `isPeerMessage` must run first, since `isClientPeerSendMessage` does not check `id` or `binary`. + */ +function isClientPeerSendMessages(maybe: unknown): maybe is ClientPeerSendMessage[] { + return Array.isArray(maybe) && maybe.every((m: unknown) => isPeerMessage(m) && isClientPeerSendMessage(m)) +} + /** * Errors should not occur at the routing interceptor level. * Reject the promise so it can be handled by the unhandledRejection handler @@ -271,7 +279,7 @@ export class BatchHandlerPlugin implements StandardHandlerPlu const mightBeMessages = JSON.parse(data) - if (!Array.isArray(mightBeMessages) || mightBeMessages.some(m => !isPeerMessage(m) || !isClientPeerSendMessage(m))) { + if (!isClientPeerSendMessages(mightBeMessages)) { return { matched: true, response: { status: 400, headers: {}, body: 'Invalid batch request data parameter' }, @@ -283,7 +291,7 @@ export class BatchHandlerPlugin implements StandardHandlerPlu else { const mightBeMessages = await interceptorOptions.request.resolveBody() - if (!Array.isArray(mightBeMessages) || mightBeMessages.some(m => !isPeerMessage(m) || !isClientPeerSendMessage(m))) { + if (!isClientPeerSendMessages(mightBeMessages)) { return { matched: true, response: { status: 400, headers: {}, body: 'Invalid batch request body' }, From caa4ebdc1706ebbf47fa13738fa5496ccb19b804 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 13:24:13 +0000 Subject: [PATCH 3/4] refactor(server): type parsed GET batch data as unknown JSON.parse returns any, so the GET branch could assign the parsed data to messages without the guard. Typing it as unknown, like the POST body already is, makes tsc require the guard on both branches. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X6CM5zP1QBSVRmibkRqzK3 --- packages/server/src/plugins/batch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/plugins/batch.ts b/packages/server/src/plugins/batch.ts index 9fe4de3934..565ebd5e24 100644 --- a/packages/server/src/plugins/batch.ts +++ b/packages/server/src/plugins/batch.ts @@ -277,7 +277,7 @@ export class BatchHandlerPlugin implements StandardHandlerPlu } } - const mightBeMessages = JSON.parse(data) + const mightBeMessages: unknown = JSON.parse(data) if (!isClientPeerSendMessages(mightBeMessages)) { return { From 1192899f05c737972ccab51f6efb92421e014d92 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 13:29:07 +0000 Subject: [PATCH 4/4] fix(client): reject batch responses with non-string ids BatchLinkPlugin validated array batch responses with isServerPeerSendMessage alone, which never checks id or binary. A message with a non-string id was silently dropped and the call failed with "Batch response is incomplete." instead of "Invalid batch response format.", and a JSON object in binary resolved the call with a "[object Object]" file. Require isPeerMessage as well, like the server and the length-prefixed paths already do. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X6CM5zP1QBSVRmibkRqzK3 --- packages/client/src/plugins/batch.test.ts | 58 +++++++++++++++++++++++ packages/client/src/plugins/batch.ts | 4 +- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/packages/client/src/plugins/batch.test.ts b/packages/client/src/plugins/batch.test.ts index 4a3f6e5568..e596f3d7be 100644 --- a/packages/client/src/plugins/batch.test.ts +++ b/packages/client/src/plugins/batch.test.ts @@ -1133,6 +1133,64 @@ describe('batchLinkPlugin', () => { ]) }) + it.each([ + ['true', true], + ['an object', {}], + ['an array', []], + ['null', null], + ['a number', 0], + ['missing', undefined], + ] as const)('rejects on array batch responses whose message id is %s', async (_, id) => { + const codec = makeCodec() + const transport = makeTransport() + + vi.mocked(transport.send).mockImplementation(async () => { + return { + status: 207, + headers: {}, + resolveBody: async () => [{ kind: 'response', id, json: { status: 200, headers: {}, body: 'result' } }], + } + }) + + const link = new StandardLink(codec, transport, { + plugins: [new BatchLinkPlugin({ groups: [defaultGroup] })], + }) + + await Promise.all([ + expect(link.call(['a'], {}, { context: {} })).rejects.toThrow('Invalid batch response format'), + expect(link.call(['b'], {}, { context: {} })).rejects.toThrow('Invalid batch response format'), + ]) + }) + + it('rejects on array batch responses whose message binary is not binary', async () => { + const codec = makeCodec() + const transport = makeTransport() + + vi.mocked(transport.send).mockImplementation(async (request) => { + const messages = extractBatchMessagesFromRequest(request) + + return { + status: 207, + headers: {}, + resolveBody: async () => messages.map(message => ({ + kind: 'response', + id: message.id, + json: { status: 200, headers: {} }, + binary: {}, + })), + } + }) + + const link = new StandardLink(codec, transport, { + plugins: [new BatchLinkPlugin({ groups: [defaultGroup] })], + }) + + await Promise.all([ + expect(link.call(['a'], {}, { context: {} })).rejects.toThrow('Invalid batch response format'), + expect(link.call(['b'], {}, { context: {} })).rejects.toThrow('Invalid batch response format'), + ]) + }) + it('rejects on malformed blob batch responses with incomplete headers', async () => { const codec = makeCodec() const transport = makeTransport() diff --git a/packages/client/src/plugins/batch.ts b/packages/client/src/plugins/batch.ts index c977cbdb93..50698bb82c 100644 --- a/packages/client/src/plugins/batch.ts +++ b/packages/client/src/plugins/batch.ts @@ -5,7 +5,7 @@ import type { StandardLinkOptions, StandardLinkPlugin, StandardLinkTransportInte import type { ClientContext } from '../types' import { captureAsyncContext, defer, isAsyncIteratorObject, loadBytes, once, promiseWithResolvers, safeEncodeURIComponent, splitInHalf, stringifyJSON, toArray, value } from '@orpc/shared' import { parseStandardUrl } from '@standard-server/core' -import { ClientPeer, decodePeerMessage, isServerPeerSendMessage } from '@standard-server/peer' +import { ClientPeer, decodePeerMessage, isPeerMessage, isServerPeerSendMessage } from '@standard-server/peer' /** * Header that marks a request as a batch, set by the Batch Link Plugin and detected by the Batch Handler Plugin. @@ -433,7 +433,7 @@ export class BatchLinkPlugin implements StandardLinkPlu const body = await batchResponse.resolveBody() - if (Array.isArray(body) && body.every(v => isServerPeerSendMessage(v))) { + if (Array.isArray(body) && body.every((v: unknown) => isPeerMessage(v) && isServerPeerSendMessage(v))) { for (const message of body) { await peer.message(message) }