fix(server): sanitize service failures (#7644)

packages/protocol/src/schemas.ts

@@ -269,6 +269,8 @@ export const ProtocolErrorCodeSchema = Type.Union([
 	Type.Literal("session_locked"),
 	Type.Literal("not_found"),
 	Type.Literal("invalid_request"),
+	Type.Literal("not_implemented"),
+	Type.Literal("internal_error"),
 ]);
 export const ProtocolErrorSchema = StrictObject({
 	code: ProtocolErrorCodeSchema,

packages/protocol/test/protocol.test.ts

@@ -100,6 +100,16 @@ describe("protocol validation", () => {
 		expect(parseServerMessage(serverHello)).toEqual(serverHello);
 	});
 
+	test.each(["not_implemented", "internal_error"] as const)("accepts the %s error code", (code) => {
+		const message: ServerMessage = {
+			type: "response",
+			id: "request-1",
+			ok: false,
+			error: { code, message: "safe" },
+		};
+		expect(parseServerMessage(message)).toEqual(message);
+	});
+
 	test.each([
 		{
 			type: "hello",

packages/server/src/errors.ts

@@ -2,9 +2,12 @@ import type { JsonValue, ProtocolErrorCode } from "@earendil-works/pi-protocol";
 
 export type PiServerOperationErrorCode = Extract<
 	ProtocolErrorCode,
-	"busy" | "session_locked" | "not_found" | "invalid_request"
+	"busy" | "session_locked" | "not_found" | "invalid_request" | "not_implemented"
 >;
 
+export const INTERNAL_SERVER_ERROR_MESSAGE = "Internal server error";
+export const NOT_IMPLEMENTED_MESSAGE = "Operation is not implemented";
+
 /** A service/runtime error that can safely cross the protocol boundary. */
 export class PiServerError extends Error {
 	readonly code: PiServerOperationErrorCode;
@@ -38,3 +41,18 @@ export class SessionNotFoundError extends PiServerError {
 		this.name = "SessionNotFoundError";
 	}
 }
+
+export class NotImplementedError extends PiServerError {
+	constructor() {
+		super("not_implemented", NOT_IMPLEMENTED_MESSAGE);
+		this.name = "NotImplementedError";
+	}
+}
+
+/** An unsafe failure whose cause is retained for reporting but never serialized. */
+export class InternalServerError extends Error {
+	constructor(cause: unknown) {
+		super(INTERNAL_SERVER_ERROR_MESSAGE, { cause });
+		this.name = "InternalServerError";
+	}
+}

packages/server/src/server.ts

@@ -21,7 +21,12 @@ import {
 	type ConnectionState,
 	isTerminalConnection,
 } from "./connection.ts";
-import { PiServerError } from "./errors.ts";
+import {
+	INTERNAL_SERVER_ERROR_MESSAGE,
+	InternalServerError,
+	NOT_IMPLEMENTED_MESSAGE,
+	PiServerError,
+} from "./errors.ts";
 import type { PiServerListener } from "./listener.ts";
 import { LiveSessionManager } from "./sessions.ts";
 import { ServerSnapshotPublisher } from "./snapshots.ts";
@@ -344,7 +349,14 @@ export class PiServer {
 	}
 
 	private toProtocolError(error: unknown): ProtocolError {
+		if (error instanceof InternalServerError) {
+			this.reportError(error.cause);
+			return { code: "internal_error", message: INTERNAL_SERVER_ERROR_MESSAGE };
+		}
 		if (error instanceof PiServerError) {
+			if (error.code === "not_implemented") {
+				return { code: "not_implemented", message: NOT_IMPLEMENTED_MESSAGE };
+			}
 			return error.details === undefined
 				? { code: error.code, message: error.message }
 				: { code: error.code, message: error.message, details: error.details };
@@ -353,7 +365,7 @@ export class PiServer {
 			return { code: "invalid_request", message: error.message };
 		}
 		this.reportError(error);
-		return { code: "invalid_request", message: "Internal server error" };
+		return { code: "internal_error", message: INTERNAL_SERVER_ERROR_MESSAGE };
 	}
 
 	private reportError(error: unknown): void {

packages/server/test/conformance.test.ts

@@ -3,7 +3,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { encodeClientMessage, encodeFrame, PROTOCOL_VERSION } from "@earendil-works/pi-protocol";
 import { afterEach, describe, expect, test } from "vitest";
-import { type PiServer, PiServerError } from "../src/index.ts";
+import { InternalServerError, NotImplementedError, type PiServer, PiServerError } from "../src/index.ts";
 import { connectUnixTestClient, Deferred, type ProtocolTestClient, TestServerService } from "../src/testing/index.ts";
 import { createUnixServer, type UnixServerOptions } from "../src/transports/unix/index.ts";
 
@@ -267,11 +267,55 @@ describe("Unix transport conformance", () => {
 		await client.hello();
 		expect(await client.request({ command: "list" })).toMatchObject({
 			ok: false,
-			error: { code: "invalid_request", message: "Internal server error" },
+			error: { code: "internal_error", message: "Internal server error" },
 		});
 		expect(errors).toContainEqual(expect.objectContaining({ message: "private service detail" }));
 	});
 
+	test("keeps not_implemented stable", async () => {
+		class IncompleteService extends TestServerService {
+			private listCount = 0;
+
+			override async listSessions() {
+				this.listCount += 1;
+				if (this.listCount > 1) throw new NotImplementedError();
+				return super.listSessions();
+			}
+		}
+		const { server } = await startServer(new IncompleteService());
+		const client = await connect(server);
+		await client.hello();
+		expect(await client.request({ command: "list" })).toMatchObject({
+			ok: false,
+			error: { code: "not_implemented", message: "Operation is not implemented" },
+		});
+	});
+
+	test("reports wrapped internal causes without exposing them", async () => {
+		const cause = new Error("private storage detail", { cause: new Error("private root cause") });
+		class WrappedFailureService extends TestServerService {
+			private listCount = 0;
+
+			override async listSessions() {
+				this.listCount += 1;
+				if (this.listCount > 1) throw new InternalServerError(cause);
+				return super.listSessions();
+			}
+		}
+		const errors: Error[] = [];
+		const { server } = await startServer(new WrappedFailureService(), { onError: (error) => errors.push(error) });
+		const client = await connect(server);
+		await client.hello();
+		const response = await client.request({ command: "list" });
+		expect(response).toMatchObject({
+			ok: false,
+			error: { code: "internal_error", message: "Internal server error" },
+		});
+		expect(JSON.stringify(response)).not.toContain("private");
+		expect(errors).toContain(cause);
+		expect(errors).not.toContainEqual(expect.objectContaining({ name: "InternalServerError" }));
+	});
+
 	test("can respond out of request order after the handshake", async () => {
 		const service = new TestServerService();
 		service.seed("first");