fix(core): hide internal error messages from sse error events

Once the headers of an @Sse() route are sent, an error from the
producer, or from serializing an event, is written to the client as an
"error" event. Its data was the message of any Error, or any other
thrown value in full, and the error was not logged. On every other
route, and on the same route before the headers are sent,
BaseExceptionFilter answers such errors with a generic message and logs
them, so internal details (hosts, file paths, SQL fragments) only
reached SSE clients.

The error event now follows BaseExceptionFilter's rules. It carries the
message of an HttpException, or of an HTTP error from another library
(http-errors, FastifyError, or a plain { statusCode, message } value),
and "Internal server error" for anything else, and every error but an
IntrinsicException is logged. Clients that read the message of other
errors from the event now get the generic one; an HttpException still
sends its message.

Discovered by Anthropic's OSS Scanner, as vulnerability ANT-2026-E8YPG1Z5.
This commit is contained in:
Kamil Myśliwiec committed 2026-10-16 15:07:13 +02:00
1 parent f28f00cffa
commit 97ab04494c
4 files changed
+108 -47

No files matched your search

@@ -257,7 +257,7 @@ describe('Sse (Express Application)', () => {
expect(response.status).toBe(200);
expect(body).toContain('data: {"n":1}');
expect(body).toContain(
'event: error\nid: 2\ndata: Do not know how to serialize a BigInt',
'event: error\nid: 2\ndata: Internal server error',
);
expect(body).not.toContain('data: {"n":3}');
});
@@ -268,7 +268,7 @@ describe('Sse (Fastify Application)', () => {
expect(response.status).toBe(200);
expect(body).toContain('data: {"n":1}');
expect(body).toContain(
'event: error\nid: 2\ndata: Do not know how to serialize a BigInt',
'event: error\nid: 2\ndata: Internal server error',
);
expect(body).not.toContain('data: {"n":3}');
});
@@ -1,6 +1,8 @@
import {
HttpException,
type HttpServer,
HttpStatus,
IntrinsicException,
Logger,
RequestMethod,
type MessageEvent,
@@ -9,6 +11,8 @@ import {
import { IncomingMessage } from 'http';
import { EMPTY, lastValueFrom, Observable, isObservable } from 'rxjs';
import { catchError, concatMap, map, tap } from 'rxjs/operators';
import { MESSAGES } from '../constants.js';
import { BaseExceptionFilter } from '../exceptions/base-exception-filter.js';
import {
AdditionalHeadersSource,
StatusCodeSource,
@@ -43,6 +47,7 @@ export interface RedirectResponse {
export class RouterResponseController {
private readonly logger = new Logger(RouterResponseController.name);
private readonly exceptionFilter = new BaseExceptionFilter();
constructor(private readonly applicationRef: HttpServer) {}
@@ -285,7 +290,18 @@ export class RouterResponseController {
throw err;
}
const data = err instanceof Error ? err.message : err;
// No exception filter can answer once the headers are out, so
// BaseExceptionFilter's rules apply here: the client is only
// sent the message of an HTTP error, and any error but an
// IntrinsicException is logged.
const data =
err instanceof HttpException ||
this.exceptionFilter.isHttpError(err)
? err.message
: MESSAGES.UNKNOWN_EXCEPTION_MESSAGE;
if (!(err instanceof IntrinsicException)) {
this.logger.error(err);
}
stream.writeMessage({ type: 'error', data }, writeError => {
if (writeError) {
this.logger.error(writeError);
@@ -4,7 +4,9 @@ import { EMPTY, NEVER, Observable, of, Subject, throwError } from 'rxjs';
import { EventEmitter } from 'events';
import { PassThrough, Writable } from 'stream';
import {
BadRequestException,
HttpStatus,
IntrinsicException,
Logger,
type MessageEvent,
RequestMethod,
@@ -1298,6 +1300,18 @@ data: test
});
describe('when there is an error', () => {
let loggerErrorSpy: ReturnType<typeof vi.spyOn>;
beforeEach(() => {
loggerErrorSpy = vi
.spyOn(Logger.prototype, 'error')
.mockImplementation(() => {});
});
afterEach(() => {
loggerErrorSpy.mockRestore();
});
it('should reject when the stream errors before headers are committed', async () => {
const result = new Subject();
const response = new Writable();
@@ -1405,55 +1419,88 @@ data: test
});
});
it('should write the error message to the stream', async () => {
let resolveFirstChunk: (() => void) | undefined;
const firstChunkWritten = new Promise<void>(resolve => {
resolveFirstChunk = resolve;
});
// The headers are out, so no exception filter can answer anymore
it.each([
['an HttpException', new BadRequestException('Some error'), true],
[
'an "http-errors" error',
Object.assign(new Error('Some error'), {
status: 404,
statusCode: 404,
expose: true,
}),
true,
],
[
'an http error-shaped value',
{ statusCode: 404, message: 'Some error' },
true,
],
['an Error', new Error('Some error'), false],
['a string', 'Some error', false],
['an IntrinsicException', new IntrinsicException('Some error'), false],
])(
'should write the message BaseExceptionFilter would send for %s',
async (_, error, isMessageSent) => {
let resolveFirstChunk: (() => void) | undefined;
const firstChunkWritten = new Promise<void>(resolve => {
resolveFirstChunk = resolve;
});
class Sink extends Writable {
private readonly chunks: string[] = [];
class Sink extends Writable {
private readonly chunks: string[] = [];
_write(
chunk: any,
encoding: string,
callback: (error?: Error | null) => void,
): void {
this.chunks.push(chunk);
resolveFirstChunk?.();
resolveFirstChunk = undefined;
callback();
_write(
chunk: any,
encoding: string,
callback: (error?: Error | null) => void,
): void {
this.chunks.push(chunk);
resolveFirstChunk?.();
resolveFirstChunk = undefined;
callback();
}
get content() {
return this.chunks.join('');
}
}
get content() {
return this.chunks.join('');
}
}
const written = (stream: Writable) =>
new Promise((resolve, reject) =>
stream.on('error', reject).on('finish', resolve),
);
const written = (stream: Writable) =>
new Promise((resolve, reject) =>
stream.on('error', reject).on('finish', resolve),
const result = new Subject();
const response = new Sink();
const request = new PassThrough();
const promise = routerResponseController.sse(
result,
response as unknown as ServerResponse,
request as unknown as IncomingMessage,
);
const result = new Subject();
const response = new Sink();
const request = new PassThrough();
const promise = routerResponseController.sse(
result,
response as unknown as ServerResponse,
request as unknown as IncomingMessage,
);
result.next('hello');
await firstChunkWritten;
result.error(error);
request.destroy();
result.next('hello');
await firstChunkWritten;
result.error(new Error('Some error'));
request.destroy();
await promise;
await written(response);
expect(response.content).toContain('event: error');
expect(response.content).toContain('data: Some error');
});
await promise;
await written(response);
expect(response.content).toContain('event: error');
if (isMessageSent) {
expect(response.content).toContain('data: Some error\n');
} else {
expect(response.content).toContain('data: Internal server error\n');
expect(response.content).not.toContain('Some error');
}
if (error instanceof IntrinsicException) {
expect(loggerErrorSpy).not.toHaveBeenCalled();
} else {
expect(loggerErrorSpy).toHaveBeenCalledWith(error);
}
},
);
it('should write an error event and end the stream when a message cannot be serialized', async () => {
class Sink extends Writable {
@@ -1489,9 +1536,7 @@ data: test
await new Promise(resolve => setTimeout(resolve, 10));
expect(response.content).toContain('event: error');
expect(response.content).toContain(
'data: Do not know how to serialize a BigInt',
);
expect(response.content).toContain('data: Internal server error');
expect(response.content).not.toContain('data: after');
expect(response.writableFinished).toBe(true);
request.destroy();