Skip to content

Commit 5e01eb2

Browse files
benbrandtclaude
andauthored
fix: unify JSON-RPC message validation policy across transports (#212)
The WebSocket and SSE receive paths validated messages with the strict isJsonRpcMessage guard and silently dropped anything that failed it, so a lenient peer's response (e.g. error:null alongside result) or request (e.g. missing the jsonrpc field) never settled the caller's promise — the same shapes worked over stdio, which only skips non-object lines. Relax the per-transport guards (ws-stream, ws-server, sse, HTTP server POST) to an object check and let the connection layer own the accept/reject policy: it dispatches lenient requests, fast-rejects structurally invalid responses with RequestError.invalidRequest, and logs anything else, as of #210. Non-object payloads are still rejected at the transport with a warning (or HTTP 400) since they carry no id to answer. Also consolidates ws-stream's private isRecord duplicate. Fixes #211 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 2fc41d2 commit 5e01eb2

6 files changed

Lines changed: 57 additions & 27 deletions

File tree

src/server.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
methodRequiresSessionHeader,
1010
sessionIdFromParams,
1111
} from "./protocol.js";
12-
import { isJsonRpcMessage, isResponseMessage } from "./jsonrpc.js";
12+
import { isRecord, isResponseMessage } from "./jsonrpc.js";
1313
import { AGENT_METHODS } from "./schema/index.js";
1414
import { serializeSseEvent, serializeSseKeepAlive } from "./sse.js";
1515
import { handleWebSocketConnection } from "./ws-server.js";
@@ -201,15 +201,18 @@ export class AcpServer {
201201
return textResponse("Batch JSON-RPC requests are not implemented", 501);
202202
}
203203

204-
if (!isJsonRpcMessage(body.value)) {
204+
// Reject non-object bodies; anything object-shaped is left for the
205+
// connection layer to validate.
206+
if (!isRecord(body.value)) {
205207
return textResponse("Invalid JSON-RPC message", 400);
206208
}
207209

210+
const message = body.value as AnyMessage;
208211
const connectionId = req.headers.get(HEADER_CONNECTION_ID);
209212

210-
if (isInitializeRequest(body.value)) {
213+
if (isInitializeRequest(message)) {
211214
if (!connectionId) {
212-
return await this.handleInitialize(body.value, req.signal, options);
215+
return await this.handleInitialize(message, req.signal, options);
213216
}
214217

215218
return textResponse("Initialize not allowed on existing connection", 400);
@@ -227,7 +230,7 @@ export class AcpServer {
227230

228231
const forwarded = await this.forwardConnectedMessage(
229232
connection,
230-
body.value,
233+
message,
231234
req.headers,
232235
);
233236
if (!forwarded.ok) {

src/sse.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,30 @@ describe("SSE transport helpers", () => {
149149
).resolves.toEqual([message]);
150150
});
151151

152+
it("passes through object payloads without validating their shape", async () => {
153+
// Lenient shapes are left for the connection layer to validate.
154+
const lenient = { id: 1, result: { ok: true } }; // missing jsonrpc
155+
await expect(
156+
collectMessages(
157+
streamFromChunks([`data: ${JSON.stringify(lenient)}\n\n`]),
158+
),
159+
).resolves.toEqual([lenient]);
160+
});
161+
162+
it("skips non-object payloads", async () => {
163+
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
164+
const message: AnyMessage = { jsonrpc: "2.0", id: 1, result: { ok: true } };
165+
166+
await expect(
167+
collectMessages(
168+
streamFromChunks(["data: 42\n\n", serializeSseEvent(message)]),
169+
),
170+
).resolves.toEqual([message]);
171+
expect(warn).toHaveBeenCalledOnce();
172+
173+
warn.mockRestore();
174+
});
175+
152176
it("skips malformed JSON without throwing", async () => {
153177
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
154178
const message: AnyMessage = { jsonrpc: "2.0", id: 1, result: { ok: true } };

src/sse.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { AnyMessage } from "./jsonrpc.js";
2-
import { isJsonRpcMessage } from "./jsonrpc.js";
2+
import { isRecord } from "./jsonrpc.js";
33
import { LineBuffer } from "./line-buffer.js";
44

55
export function serializeSseEvent(msg: AnyMessage): string {
@@ -89,11 +89,13 @@ function parseSseEvent(eventLines: string[]): AnyMessage | undefined {
8989

9090
try {
9191
const parsed: unknown = JSON.parse(data);
92-
if (isJsonRpcMessage(parsed)) {
93-
return parsed;
92+
// Skip non-object payloads with a useful warning; anything object-shaped
93+
// is left for the connection layer to validate.
94+
if (isRecord(parsed)) {
95+
return parsed as AnyMessage;
9496
}
9597

96-
console.warn("Skipping SSE payload that is not a JSON-RPC message");
98+
console.warn("Skipping SSE payload that is not an object");
9799
return undefined;
98100
} catch (error) {
99101
console.warn("Failed to parse SSE JSON payload:", error);

src/ws-server.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import {
2-
isJsonRpcMessage,
3-
isRequestMessage,
4-
isResponseMessage,
5-
} from "./jsonrpc.js";
1+
import { isRecord, isRequestMessage, isResponseMessage } from "./jsonrpc.js";
62
import {
73
isInitializeRequest,
84
messageIdKey,
@@ -141,18 +137,22 @@ class WebSocketServerSession implements WebSocketServerSessionHandle {
141137
return;
142138
}
143139

144-
if (!isJsonRpcMessage(value)) {
145-
console.warn("Ignoring non-JSON-RPC ACP WebSocket message:", value);
140+
// Skip non-object messages with a useful warning; anything object-shaped
141+
// is left for the connection layer to validate.
142+
if (!isRecord(value)) {
143+
console.warn("Ignoring non-object ACP WebSocket message:", value);
146144
await this.shutdownIfUninitialized(1002, "Invalid JSON-RPC message");
147145
return;
148146
}
149147

148+
const message = value as AnyMessage;
149+
150150
if (!this.connection) {
151-
await this.handleInitialize(value);
151+
await this.handleInitialize(message);
152152
return;
153153
}
154154

155-
const forwarded = await this.forwardMessage(value);
155+
const forwarded = await this.forwardMessage(message);
156156
if (!forwarded.ok) {
157157
console.warn("Ignoring ACP WebSocket message:", forwarded.message);
158158
}

src/ws-stream.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ describe("createWebSocketStream", () => {
321321
}
322322
});
323323

324-
it("ignores binary, malformed JSON, and non-JSON-RPC messages", async () => {
324+
it("ignores binary, malformed JSON, and non-object messages, passing other objects through", async () => {
325325
const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
326326
const instances: FakeWebSocket[] = [];
327327
const stream = createWebSocketStream("ws://agent.example/acp", {
@@ -340,9 +340,12 @@ describe("createWebSocketStream", () => {
340340
new TextEncoder().encode(JSON.stringify(initializeResponse)).buffer,
341341
);
342342
socket.receive("not json");
343+
socket.receive("42");
344+
// Lenient shapes are left for the connection layer to validate.
343345
socket.receive(JSON.stringify({ hello: "world" }));
344346
socket.receive(JSON.stringify(initializeResponse));
345347

348+
expect(await readMessage(reader)).toEqual({ hello: "world" });
346349
expect(await readMessage(reader)).toEqual(initializeResponse);
347350
expect(warn).toHaveBeenCalledTimes(2);
348351
} finally {

src/ws-stream.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { MemoryAcpCookieStore } from "./cookie-store.js";
2-
import { isJsonRpcMessage } from "./jsonrpc.js";
2+
import { isRecord } from "./jsonrpc.js";
33
import { onWebSocket, webSocketMessageToString } from "./ws-utils.js";
44
import type { AcpCookieStore } from "./cookie-store.js";
55
import type { WebSocketLike } from "./ws-utils.js";
@@ -202,12 +202,14 @@ class WebSocketStreamTransport {
202202
return;
203203
}
204204

205-
if (!isJsonRpcMessage(value)) {
206-
console.warn("Ignoring non-JSON-RPC ACP WebSocket message:", value);
205+
// Skip non-object messages with a useful warning; anything object-shaped
206+
// is left for the connection layer to validate.
207+
if (!isRecord(value)) {
208+
console.warn("Ignoring non-object ACP WebSocket message:", value);
207209
return;
208210
}
209211

210-
this.readableController?.enqueue(value);
212+
this.readableController?.enqueue(value as AnyMessage);
211213
}
212214

213215
private close(): void {
@@ -342,10 +344,6 @@ function headersFromRecord(value: unknown): Headers | undefined {
342344
return headers;
343345
}
344346

345-
function isRecord(value: unknown): value is Record<string, unknown> {
346-
return typeof value === "object" && value !== null;
347-
}
348-
349347
function resolveWebSocket(
350348
WebSocketCtor: WebSocketConstructor | undefined,
351349
): WebSocketConstructor {

0 commit comments

Comments
 (0)