Validate inbound JSON-RPC messages and contain handler failures

src/app-server/connection/json-rpc-client.ts

@@ -99,16 +99,27 @@ export class JsonRpcClient {
   handleLine(line: string): void {
     if (line.trim().length === 0) return;
 
-    let message: RpcInboundMessage;
+    let parsed: unknown;
     try {
-      message = JSON.parse(line) as RpcInboundMessage;
+      parsed = JSON.parse(line) as unknown;
     } catch {
       this.options.onLog(`Invalid app-server JSON: ${line}`);
       return;
     }
 
+    const message = rpcInboundMessage(parsed);
+    if (!message) {
+      this.options.onLog(`Invalid app-server JSON-RPC message: ${line}`);
+      return;
+    }
+
     if ("id" in message && "method" in message) {
-      this.options.onServerRequest(message);
+      try {
+        this.options.onServerRequest(message);
+      } catch (error) {
+        this.options.onLog(`App-server request handler failed: ${errorMessage(error)}`);
+        this.reject(message.id, -32603, "Codex Panel failed to handle the app-server request.");
+      }
       return;
     }
 
@@ -118,7 +129,11 @@ export class JsonRpcClient {
     }
 
     if ("method" in message) {
-      this.options.onNotification(message);
+      try {
+        this.options.onNotification(message);
+      } catch (error) {
+        this.options.onLog(`App-server notification handler failed: ${errorMessage(error)}`);
+      }
     }
   }
 
@@ -162,6 +177,33 @@ export class JsonRpcClient {
   }
 }
 
+function rpcInboundMessage(value: unknown): RpcInboundMessage | null {
+  if (!isRecord(value)) return null;
+  const hasId = Object.hasOwn(value, "id");
+  const hasMethod = Object.hasOwn(value, "method");
+  if (hasId && !isRequestId(value["id"])) return null;
+  if (hasMethod && (typeof value["method"] !== "string" || value["method"].length === 0 || !isRecord(value["params"]))) return null;
+  if (hasId && hasMethod) return value as unknown as ServerRequest;
+  if (hasId) {
+    const hasResult = Object.hasOwn(value, "result");
+    const hasError = Object.hasOwn(value, "error");
+    return hasResult !== hasError ? (value as unknown as RpcInboundMessage) : null;
+  }
+  return hasMethod ? (value as unknown as ServerNotification) : null;
+}
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return value !== null && typeof value === "object" && !Array.isArray(value);
+}
+
+function isRequestId(value: unknown): value is RequestId {
+  return typeof value === "string" || (typeof value === "number" && Number.isFinite(value));
+}
+
+function errorMessage(error: unknown): string {
+  return error instanceof Error ? error.message : String(error);
+}
+
 function isRpcError(value: unknown): value is RpcError {
   return value !== null && typeof value === "object" && typeof (value as { message?: unknown }).message === "string";
 }

tests/app-server/app-server-client.test.ts

@@ -213,6 +213,62 @@ describe("AppServerClient", () => {
     expect(latestSent(getTransport())).toEqual({ id: 99, error: { code: -32601, message: "Request not handled." } });
   });
 
+  it("logs malformed JSON-RPC envelopes without throwing from the transport callback", async () => {
+    const logs: string[] = [];
+    const notifications = vi.fn();
+    const serverRequests = vi.fn();
+    let transport!: FakeTransport;
+    const client = createTestClient({
+      handlers: {
+        onNotification: notifications,
+        onServerRequest: serverRequests,
+        onLog: (message) => logs.push(message),
+        onExit: () => undefined,
+      },
+      transportFactory: (handlers) => {
+        transport = new FakeTransport(handlers);
+        return transport;
+      },
+    });
+    const connecting = client.connect();
+    transport.emitLine({ id: 1, result: { codexHome: "/tmp/codex" } satisfies Partial<InitializeResponse> });
+    await connecting;
+
+    for (const message of [null, 42, "text", [], {}, { method: "warning" }, { id: 4 }, { id: 5, method: "request", params: null }]) {
+      expect(() => transport.emitLine(message)).not.toThrow();
+    }
+
+    expect(notifications).not.toHaveBeenCalled();
+    expect(serverRequests).not.toHaveBeenCalled();
+    expect(logs).toHaveLength(8);
+    expect(logs.every((message) => message.startsWith("Invalid app-server JSON-RPC message:"))).toBe(true);
+  });
+
+  it("contains inbound handler failures at the JSON-RPC boundary", async () => {
+    const logs: string[] = [];
+    let transport!: FakeTransport;
+    const client = createTestClient({
+      handlers: {
+        onNotification: () => {
+          throw new Error("notification failed");
+        },
+        onServerRequest: () => undefined,
+        onLog: (message) => logs.push(message),
+        onExit: () => undefined,
+      },
+      transportFactory: (handlers) => {
+        transport = new FakeTransport(handlers);
+        return transport;
+      },
+    });
+    const connecting = client.connect();
+    transport.emitLine({ id: 1, result: { codexHome: "/tmp/codex" } satisfies Partial<InitializeResponse> });
+    await connecting;
+
+    expect(() => transport.emitLine({ method: "warning", params: { message: "careful" } })).not.toThrow();
+    expect(logs).toEqual(["App-server notification handler failed: notification failed"]);
+  });
+
   it("sends typed client requests", async () => {
     const { client, transport } = await connectedClient();