From fe9c0366f4d223132adb24b125bae2e5aeae59c3 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:33:56 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20Sanitize=20internal=20system=20e?= =?UTF-8?q?rrors=20in=20API=20responses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: mbayue <70324722+mbayue@users.noreply.github.com> --- .jules/sentinel.md | 2 ++ server/utils/errors.test.ts | 16 ++++++++++++++-- server/utils/errors.ts | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index fe8b178..721687c 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -27,3 +27,5 @@ **Vulnerability:** External network requests made via `fetch` lacked a timeout, allowing them to hang indefinitely if the remote server was slow or unresponsive. This could exhaust application resources (e.g., memory, sockets, thread pool limits) over time, leading to a Denial of Service (DoS). **Learning:** Unbounded network requests tie up valuable system resources while waiting for a response that may never arrive. **Prevention:** When implementing external API requests using `fetch` or similar HTTP clients, always enforce a timeout (e.g., using `AbortSignal.timeout(5000)`) to prevent unbounded waiting and application unresponsiveness. + +* **Information Disclosure in API Errors**: When returning errors from the backend to the client, avoid passing the raw `.message` property of system or internal errors for 500-level statuses. Instead, sanitize 500-level errors by substituting a generic fallback message like `"Internal Server Error"` to prevent exposing sensitive internal state (e.g., file paths, IPs, connection strings, API keys) which might be contained in the raw error message. diff --git a/server/utils/errors.test.ts b/server/utils/errors.test.ts index 69fefcc..2d5cab8 100644 --- a/server/utils/errors.test.ts +++ b/server/utils/errors.test.ts @@ -16,7 +16,7 @@ describe('utils/errors', () => { expect(isAppError(new Error('x'))).toBe(false); }); - it('serializes AppError payload', () => { + it('serializes AppError payload and preserves message for < 500 status', () => { const error = new AppError(400, 'bad', 'BAD', false, { field: 'x' }); expect(toErrorPayload(error)).toEqual({ @@ -28,6 +28,18 @@ describe('utils/errors', () => { }); }); + it('serializes AppError payload and sanitizes message for >= 500 status', () => { + const error = new AppError(500, 'database connection failed: secret_password', 'DB_ERROR', false, { field: 'x' }); + + expect(toErrorPayload(error)).toEqual({ + error: 'Internal Server Error', + code: 'DB_ERROR', + status: 500, + retryable: false, + context: { field: 'x' }, + }); + }); + it('maps rate-limit errors', () => { expect(toErrorPayload(new Error('429 rate limit'))).toEqual({ error: 'Rate limit exceeded', @@ -76,4 +88,4 @@ describe('utils/errors', () => { retryable: false, }); }); -}); \ No newline at end of file +}); diff --git a/server/utils/errors.ts b/server/utils/errors.ts index 8cad03e..58cdee0 100644 --- a/server/utils/errors.ts +++ b/server/utils/errors.ts @@ -18,7 +18,7 @@ export function isAppError(err: unknown): err is AppError { export function toErrorPayload(err: unknown) { if (isAppError(err)) { return { - error: err.message, + error: err.status >= 500 ? 'Internal Server Error' : err.message, code: err.code, status: err.status, retryable: err.retryable,