diff --git a/.changeset/sanitize-rpc-error-leak.md b/.changeset/sanitize-rpc-error-leak.md new file mode 100644 index 000000000..efa27cbc0 --- /dev/null +++ b/.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. diff --git a/packages/core/src/rpc.test.ts b/packages/core/src/rpc.test.ts index 585272edb..63effcf9f 100644 --- a/packages/core/src/rpc.test.ts +++ b/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); + }); +}); diff --git a/packages/core/src/rpc.ts b/packages/core/src/rpc.ts index 30eb188d8..4c25051f8 100644 --- a/packages/core/src/rpc.ts +++ b/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 = {}; - 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 = {}; + 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. */