fix(core): stop leaking backend exception details in RPC errors

.changeset/sanitize-rpc-error-leak.md

@@ -0,0 +1,5 @@
+---
+"@aws-blocks/core": patch
+---
+
+Stop leaking raw backend exception details in RPC error responses. `errorResponseFromCatch` now only forwards `ApiError` instances verbatim; every other throw (driver/SDK exceptions, unexpected bugs) collapses to a generic `500` / `"Internal error"` on the wire, with the full error still logged server-side. This prevents internal exception class names and raw driver messages from reaching clients.

packages/core/src/rpc.test.ts

@@ -3,7 +3,8 @@
 
 import { describe, it } from 'node:test';
 import assert from 'node:assert';
-import { parseRpcRequest, RpcErrorCode } from './rpc.js';
+import { parseRpcRequest, errorResponseFromCatch, RpcErrorCode } from './rpc.js';
+import { ApiError } from './errors.js';
 
 describe('-32600 Invalid Request error shape', () => {
   it('returns proper JSON-RPC 2.0 envelope with error code', () => {
@@ -71,3 +72,64 @@ describe('-32600 Invalid Request error shape', () => {
     }
   });
 });
+
+describe('errorResponseFromCatch does not leak backend internals', () => {
+  it('collapses a non-ApiError (driver exception) to a generic 500', () => {
+    // Simulate a Postgres/DynamoDB driver throw: custom class name + raw message.
+    class PostgresError extends Error {
+      constructor(message: string) {
+        super(message);
+        this.name = 'PostgresError';
+      }
+    }
+    const raw = new PostgresError('duplicate key value violates unique constraint "users_email_key"');
+
+    const parsed = JSON.parse(errorResponseFromCatch(raw, 1));
+    assert.strictEqual(parsed.error.code, 500);
+    assert.strictEqual(parsed.error.message, 'Internal error');
+    // The raw class name and message must never reach the client.
+    assert.strictEqual(parsed.error.data, undefined);
+    assert.ok(!JSON.stringify(parsed).includes('PostgresError'));
+    assert.ok(!JSON.stringify(parsed).includes('users_email_key'));
+    assert.strictEqual(parsed.id, 1);
+  });
+
+  it('collapses a plain Error to a generic 500 with no name', () => {
+    const parsed = JSON.parse(errorResponseFromCatch(new Error('boom: /var/task internal path'), 2));
+    assert.strictEqual(parsed.error.code, 500);
+    assert.strictEqual(parsed.error.message, 'Internal error');
+    assert.strictEqual(parsed.error.data, undefined);
+  });
+
+  it('collapses a non-Error throw (string) to a generic 500', () => {
+    const parsed = JSON.parse(errorResponseFromCatch('raw string failure', 3));
+    assert.strictEqual(parsed.error.code, 500);
+    assert.strictEqual(parsed.error.message, 'Internal error');
+    assert.strictEqual(parsed.error.data, undefined);
+  });
+
+  it('passes an ApiError through verbatim with its status, message, and BB name', () => {
+    const err = new ApiError('Username already taken', 409, { name: 'ConditionalCheckFailedException' });
+    const parsed = JSON.parse(errorResponseFromCatch(err, 4));
+    assert.strictEqual(parsed.error.code, 409);
+    assert.strictEqual(parsed.error.message, 'Username already taken');
+    assert.strictEqual(parsed.error.data.name, 'ConditionalCheckFailedException');
+    assert.strictEqual(parsed.id, 4);
+  });
+
+  it('propagates the retriable flag on an ApiError', () => {
+    const err = new ApiError('Wrong MFA code', 401, { name: 'InvalidMfaCode', retriable: true });
+    const parsed = JSON.parse(errorResponseFromCatch(err, 5));
+    assert.strictEqual(parsed.error.code, 401);
+    assert.strictEqual(parsed.error.data.name, 'InvalidMfaCode');
+    assert.strictEqual(parsed.error.data.retriable, true);
+  });
+
+  it('omits data.name for an ApiError left at the default name', () => {
+    const err = new ApiError('Something went wrong', 500);
+    const parsed = JSON.parse(errorResponseFromCatch(err, 6));
+    assert.strictEqual(parsed.error.code, 500);
+    assert.strictEqual(parsed.error.message, 'Something went wrong');
+    assert.strictEqual(parsed.error.data, undefined);
+  });
+});

packages/core/src/rpc.ts

@@ -10,7 +10,7 @@
  * @see https://www.jsonrpc.org/specification
  */
 
-import { ApiError } from './errors.js';
+import { ApiError, DEFAULT_API_ERROR_NAME } from './errors.js';
 
 // ── Types ───────────────────────────────────────────────────────────────────
 
@@ -140,17 +140,21 @@ export function successResponse(result: unknown, id: string | number | null): st
 /**
  * Encode an error as a JSON-RPC 2.0 response string.
  *
- * For `ApiError` instances the HTTP status becomes the error code (positive
- * integers never collide with the reserved -32xxx range). Generic errors
- * use code 500.
+ * Only `ApiError` is a deliberate, wire-safe shape: its HTTP status becomes
+ * the error code (positive integers never collide with the reserved -32xxx
+ * range), and its BB-level `name`/`retriable` flags cross the wire. Every
+ * other throw — driver/SDK exceptions (Postgres, DynamoDB, …) and unexpected
+ * bugs — collapses to a generic 500 so raw exception class names and messages
+ * never leak to the client. Callers log the full error server-side.
  */
 export function errorResponseFromCatch(error: unknown, id: string | number | null): string {
-  const code = error instanceof ApiError ? error.status : 500;
-  const message = error instanceof Error ? error.message : String(error);
-  const data: Record<string, unknown> = {};
-  if (error instanceof Error && error.name && error.name !== 'Error') data.name = error.name;
-  if (error instanceof ApiError && error.retriable) data.retriable = true;
-  return errorResponse(code, message, id, Object.keys(data).length > 0 ? data : undefined);
+  if (error instanceof ApiError) {
+    const data: Record<string, unknown> = {};
+    if (error.name && error.name !== DEFAULT_API_ERROR_NAME) data.name = error.name;
+    if (error.retriable) data.retriable = true;
+    return errorResponse(error.status, error.message, id, Object.keys(data).length > 0 ? data : undefined);
+  }
+  return errorResponse(500, 'Internal error', id);
 }
 
 /** Encode a "method not found" error. */