fix(core): Fix SSE push backend connections in production (#35327)

This commit is contained in:
Tomi Turtiainen 2026-07-31 18:45:27 +03:00 committed by GitHub
parent 5b68642b5d
commit c0a8f02f0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 222 additions and 59 deletions

View File

@ -152,11 +152,6 @@ describe('Push', () => {
describe('should throw on invalid origin', () => {
test.each([
{
name: 'origin is undefined',
origin: undefined,
xForwardedHost: undefined,
},
{
name: 'origin does not match host',
origin: 'https://123example.com',
@ -193,6 +188,25 @@ describe('Push', () => {
});
});
test('should only accept a missing origin header on SSE', () => {
req.headers = {
host,
'sec-fetch-site': 'same-origin',
} as typeof req.headers;
const emitSpy = vi.spyOn(push, 'emit');
push.handleRequest(req, res);
if (backendName === 'sse') {
expect(backend.add).toHaveBeenCalledWith(pushRef, user.id, { req, res });
expect(emitSpy).toHaveBeenCalledWith('editorUiConnected', pushRef);
} else {
expect(ws.send).toHaveBeenCalledWith('Invalid origin!');
expect(ws.close).toHaveBeenCalledWith(1008);
expect(backend.add).not.toHaveBeenCalled();
}
});
describe('should not throw on invalid origin if `X-Forwarded-Host` is set correctly', () => {
test.each([
{

View File

@ -1,31 +1,31 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { validateOriginHeaders } from '../origin-validator';
import { validateSseOrigin, validateWebSocketOrigin } from '../origin-validator';
describe('validateOriginHeaders', () => {
describe('validateWebSocketOrigin', () => {
const host = 'example.com';
describe('basic validation', () => {
test('should return invalid for missing origin', () => {
const result = validateOriginHeaders({});
const result = validateWebSocketOrigin({});
expect(result.isValid).toBe(false);
expect(result.error).toBe('Origin header is missing or malformed');
});
test('should return invalid for empty origin', () => {
const result = validateOriginHeaders({ origin: '' });
const result = validateWebSocketOrigin({ origin: '' });
expect(result.isValid).toBe(false);
expect(result.error).toBe('Origin header is missing or malformed');
});
test('should return invalid for malformed origin', () => {
const result = validateOriginHeaders({ origin: 'invalid-origin' });
const result = validateWebSocketOrigin({ origin: 'invalid-origin' });
expect(result.isValid).toBe(false);
expect(result.error).toBe('Origin header is missing or malformed');
});
test('should validate matching hosts with host header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
host,
});
@ -36,7 +36,7 @@ describe('validateOriginHeaders', () => {
});
test('should validate matching hosts with x-forwarded-host header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': host,
});
@ -45,7 +45,7 @@ describe('validateOriginHeaders', () => {
});
test('should reject mismatched hosts', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': 'wrong-host.com',
});
@ -56,7 +56,7 @@ describe('validateOriginHeaders', () => {
describe('host normalization behavior', () => {
test('should normalize HTTPS default ports (443)', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': `${host}:443`,
});
@ -65,7 +65,7 @@ describe('validateOriginHeaders', () => {
});
test('should normalize HTTP default ports (80)', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `http://${host}`,
'x-forwarded-host': `${host}:80`,
});
@ -74,7 +74,7 @@ describe('validateOriginHeaders', () => {
});
test('should preserve non-default ports', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}:8080`,
'x-forwarded-host': `${host}:8080`,
});
@ -83,7 +83,7 @@ describe('validateOriginHeaders', () => {
});
test('should reject mismatched ports', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}:8080`,
'x-forwarded-host': `${host}:9000`,
});
@ -91,7 +91,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle IPv6 addresses correctly', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: 'https://[::1]:443',
'x-forwarded-host': '[::1]:443',
});
@ -100,7 +100,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle IPv6 with non-default ports', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: 'http://[2001:db8::1]:8080',
'x-forwarded-host': '[2001:db8::1]:8080',
});
@ -110,7 +110,7 @@ describe('validateOriginHeaders', () => {
test('should handle complex domain names', () => {
const complexHost = 'sub.domain.example-site.com';
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${complexHost}:443`,
'x-forwarded-host': `${complexHost}:443`,
});
@ -119,7 +119,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle invalid URL formats gracefully', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: 'https://example.com',
'x-forwarded-host': 'invalid[host',
});
@ -130,7 +130,7 @@ describe('validateOriginHeaders', () => {
describe('origin parsing behavior', () => {
test('should reject non-HTTP/HTTPS protocols', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: 'ftp://example.com',
host: 'example.com',
});
@ -139,7 +139,7 @@ describe('validateOriginHeaders', () => {
});
test('should reject websocket protocols', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: 'ws://example.com',
host: 'example.com',
});
@ -147,7 +147,7 @@ describe('validateOriginHeaders', () => {
});
test('should reject chrome-extension protocols', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: 'chrome-extension://example.com',
host: 'example.com',
});
@ -155,7 +155,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle mixed case protocols', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `HTTPS://${host}`,
host,
});
@ -164,7 +164,7 @@ describe('validateOriginHeaders', () => {
});
test('should parse HTTP origins correctly', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `http://${host}`,
host,
});
@ -176,7 +176,7 @@ describe('validateOriginHeaders', () => {
});
test('should parse HTTPS origins correctly', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
host,
});
@ -188,7 +188,7 @@ describe('validateOriginHeaders', () => {
});
test('should reject malformed URLs that throw during parsing', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: '://malformed',
host: 'example.com',
});
@ -198,7 +198,7 @@ describe('validateOriginHeaders', () => {
describe('header precedence and processing', () => {
test('should handle Forwarded header with precedence over x-forwarded-*', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `proto=https;host=${host}`,
'x-forwarded-host': 'wrong-host.com', // Should be ignored
@ -208,7 +208,7 @@ describe('validateOriginHeaders', () => {
});
test('should parse RFC 7239 Forwarded header with quoted values', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `proto="https";host="${host}"`,
});
@ -217,7 +217,7 @@ describe('validateOriginHeaders', () => {
});
test('should parse Forwarded header with single quotes', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `proto='https';host='${host}'`,
});
@ -226,7 +226,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle multiple Forwarded entries (use first)', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `proto=https;host=${host}, proto=http;host=other.com`,
});
@ -235,7 +235,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle Forwarded header with spaces around values', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: ` proto = https ; host = ${host} `,
});
@ -244,7 +244,7 @@ describe('validateOriginHeaders', () => {
});
test('should ignore unknown parameters in Forwarded header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `for=192.0.2.60;proto=https;host=${host};by=proxy.com`,
});
@ -253,7 +253,7 @@ describe('validateOriginHeaders', () => {
});
test('should fallback to X-Forwarded-* when Forwarded header incomplete', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: 'for=192.0.2.60', // Missing host and proto
'x-forwarded-host': host,
@ -264,7 +264,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle array x-forwarded-host headers (use first)', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': [host, 'other-host.com'],
});
@ -273,7 +273,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle array x-forwarded-proto headers (use first)', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': host,
'x-forwarded-proto': ['https', 'http'],
@ -283,7 +283,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle comma-separated x-forwarded-proto (use first)', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': host,
'x-forwarded-proto': 'https, http',
@ -293,7 +293,7 @@ describe('validateOriginHeaders', () => {
});
test('should fallback to Host header when no proxy headers exist', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
host,
});
@ -304,7 +304,7 @@ describe('validateOriginHeaders', () => {
describe('protocol validation behavior', () => {
test('should handle valid protocols in Forwarded header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `proto=https;host=${host}`,
});
@ -313,7 +313,7 @@ describe('validateOriginHeaders', () => {
});
test('should ignore invalid protocols in Forwarded header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: `proto=ftp;host=${host}`, // Invalid protocol should be ignored
});
@ -322,7 +322,7 @@ describe('validateOriginHeaders', () => {
});
test('should ignore invalid protocols in X-Forwarded-Proto header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': host,
'x-forwarded-proto': 'websocket', // Invalid protocol
@ -332,7 +332,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle comma-separated X-Forwarded-Proto with invalid first value', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': host,
'x-forwarded-proto': 'ftp,https', // Invalid first, valid second
@ -342,7 +342,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle protocol change via Forwarded header', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `http://${host}`,
forwarded: `proto=http;host=${host}:80`,
});
@ -354,7 +354,7 @@ describe('validateOriginHeaders', () => {
describe('edge cases and error handling', () => {
test('should handle missing host header when no proxy headers exist', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
// No host, x-forwarded-host, or forwarded headers
});
@ -363,7 +363,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle empty forwarded header gracefully', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: '',
'x-forwarded-host': host,
@ -373,7 +373,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle invalid forwarded header format', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
forwarded: 'invalid-format',
'x-forwarded-host': host,
@ -383,7 +383,7 @@ describe('validateOriginHeaders', () => {
});
test('should handle empty x-forwarded-host array', () => {
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${host}`,
'x-forwarded-host': [],
host,
@ -394,7 +394,7 @@ describe('validateOriginHeaders', () => {
test('should handle numeric IP addresses correctly', () => {
const ipHost = '192.168.1.100:3000';
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `https://${ipHost}`,
'x-forwarded-host': ipHost,
});
@ -404,7 +404,7 @@ describe('validateOriginHeaders', () => {
test('should handle localhost correctly', () => {
const localhostHost = 'localhost:8080';
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: `http://${localhostHost}`,
'x-forwarded-host': localhostHost,
});
@ -414,7 +414,7 @@ describe('validateOriginHeaders', () => {
test('should allow origins with query parameters when host matches', () => {
const queryOrigin = `https://${host}?query=param#fragment`;
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: queryOrigin,
host,
});
@ -424,7 +424,7 @@ describe('validateOriginHeaders', () => {
test('should allow origins with path components when host matches', () => {
const pathOrigin = `https://${host}/path/to/resource`;
const result = validateOriginHeaders({
const result = validateWebSocketOrigin({
origin: pathOrigin,
host,
});
@ -433,3 +433,43 @@ describe('validateOriginHeaders', () => {
});
});
});
describe('validateSseOrigin', () => {
const host = 'example.com';
test('should allow a missing origin for a same-origin fetch', () => {
const result = validateSseOrigin({ 'sec-fetch-site': 'same-origin' });
expect(result.isValid).toBe(true);
});
test.each([
{ name: 'an empty origin', headers: { origin: '' } },
{ name: 'an opaque origin', headers: { origin: 'null' } },
{ name: 'a malformed origin', headers: { origin: 'invalid-origin' } },
{ name: 'a missing sec-fetch-site', headers: {} },
{ name: 'a cross-site fetch', headers: { 'sec-fetch-site': 'cross-site' } },
{ name: 'a same-site fetch', headers: { 'sec-fetch-site': 'same-site' } },
])('should reject $name', ({ headers }) => {
const result = validateSseOrigin(headers);
expect(result.isValid).toBe(false);
expect(result.error).toBe('Origin header is missing or malformed');
});
test('should reject a present origin that does not match the host', () => {
const result = validateSseOrigin({
origin: `https://${host}`,
host: 'wrong-host.com',
});
expect(result.isValid).toBe(false);
expect(result.error).toBe('Origin header does not match expected host');
});
test('should allow a present origin that matches the host', () => {
const result = validateSseOrigin({ origin: `https://${host}`, host });
expect(result.isValid).toBe(true);
});
});

View File

@ -17,7 +17,7 @@ import { InternalServerError } from '@/errors/response-errors/internal-server.er
import { MAX_PUBSUB_PAYLOAD_BYTES } from '@/scaling/constants';
import { Publisher } from '@/scaling/pubsub/publisher.service';
import { validateOriginHeaders } from './origin-validator';
import { validateSseOrigin, validateWebSocketOrigin } from './origin-validator';
import { isPushResponse, isSSEPushRequest, isWebSocketPushRequest } from './push-helpers';
import { PushConfig } from './push.config';
import { SSEPush } from './sse.push';
@ -121,7 +121,7 @@ export class Push extends TypedEmitter<PushEvents> {
if (!pushRef) {
connectionError = 'The query parameter "pushRef" is missing!';
} else if (inProduction) {
const validation = validateOriginHeaders(headers);
const validation = ws ? validateWebSocketOrigin(headers) : validateSseOrigin(headers);
if (!validation.isValid) {
this.logger.warn(
'Origin header does NOT match the expected origin. ' +

View File

@ -9,6 +9,30 @@ export interface OriginValidationResult {
error?: string;
}
/**
* Validates the origin of a WebSocket handshake. Handshakes always carry an
* `Origin` (RFC 6455), so a missing one is a failure.
*/
export function validateWebSocketOrigin(headers: Request['headers']): OriginValidationResult {
return validateOriginAgainstExpectedHost(headers);
}
/**
* Validates the origin of an SSE request.
*
* A same-origin GET sends no `Origin` (Fetch spec), and that is exactly what
* `EventSource` issues, so SSE connections can never satisfy a plain origin check.
*/
export function validateSseOrigin(headers: Request['headers']): OriginValidationResult {
// Only a fully absent header qualifies — an empty or opaque (`null`) origin
// still fails, so sandboxed-iframe and cross-origin requests cannot use this path.
if (headers.origin === undefined && isSameOriginFetch(headers)) {
return { isValid: true };
}
return validateOriginAgainstExpectedHost(headers);
}
/**
* Validates origin headers against expected host from proxy headers.
* Handles X-Forwarded-Host, X-Forwarded-Proto, and RFC 7239 Forwarded headers.
@ -16,7 +40,7 @@ export interface OriginValidationResult {
* @param headers HTTP request headers
* @returns Validation result with details about the origin check
*/
export function validateOriginHeaders(headers: Request['headers']): OriginValidationResult {
function validateOriginAgainstExpectedHost(headers: Request['headers']): OriginValidationResult {
// Parse and normalize the origin using native URL class
const originInfo = parseOrigin(headers.origin ?? '');
@ -145,6 +169,15 @@ function getFirstHeaderValue(header: string | string[] | undefined): string | un
return header[0]; // Take first value from array
}
/**
* `Sec-Fetch-Site` is a forbidden header name, so page scripts cannot spoof it.
* `same-site` is deliberately not accepted: SameSite=Lax cookies reach sibling
* subdomains, so a same-site attacker would otherwise qualify.
*/
function isSameOriginFetch(headers: Request['headers']): boolean {
return getFirstHeaderValue(headers['sec-fetch-site']) === 'same-origin';
}
/**
* Validates and normalizes a protocol value to ensure it's http or https.
*

View File

@ -14,16 +14,33 @@ export class MockEventSource extends EventTarget {
static _instance: MockEventSource;
static readonly CONNECTING = 0;
static readonly OPEN = 1;
static readonly CLOSED = 2;
readyState: number = MockEventSource.CONNECTING;
static getInstance() {
return MockEventSource._instance;
}
simulateConnectionOpen() {
this.readyState = MockEventSource.OPEN;
this.dispatchEvent(new Event('open'));
}
/** Terminal failure: the browser gives up and closes the stream. */
simulateConnectionClose() {
this.dispatchEvent(new Event('close'));
this.readyState = MockEventSource.CLOSED;
this.dispatchEvent(new Event('error'));
}
/** Transient failure: the browser reconnects on its own. */
simulateTransientError() {
this.readyState = MockEventSource.CONNECTING;
this.dispatchEvent(new Event('error'));
}
simulateMessageEvent(data: string) {

View File

@ -62,7 +62,7 @@ describe('useEventSourceClient', () => {
expect(MockEventSource.getInstance().close).toHaveBeenCalled();
});
test('should handle connection loss', () => {
test('should reconnect after a terminal connection error', () => {
const { connect, isConnected } = useEventSourceClient({
url: 'http://test.com',
onMessage: vi.fn(),
@ -83,6 +83,21 @@ describe('useEventSourceClient', () => {
expect(MockEventSource.init).toHaveBeenCalledTimes(2);
});
test('should let EventSource retry a transient connection error', () => {
const { connect, isConnected } = useEventSourceClient({
url: 'http://test.com',
onMessage: vi.fn(),
});
connect();
MockEventSource.getInstance().simulateConnectionOpen();
MockEventSource.getInstance().simulateTransientError();
expect(isConnected.value).toBe(false);
vi.advanceTimersByTime(1_000);
expect(MockEventSource.init).toHaveBeenCalledTimes(1);
});
test('sendMessage should be a noop function', () => {
const { connect, sendMessage } = useEventSourceClient({
url: 'http://test.com',

View File

@ -22,7 +22,11 @@ export const useEventSourceClient = (options: UseEventSourceClientOptions) => {
const onConnectionLost = () => {
console.warn('[EventSourceClient] Connection lost');
isConnected.value = false;
reconnectTimer.scheduleReconnect();
// The browser retries transient failures itself (readyState stays CONNECTING);
// only a terminal failure needs our timer.
if (eventSource.value?.readyState === EventSource.CLOSED) {
reconnectTimer.scheduleReconnect();
}
};
const onMessage = (event: MessageEvent) => {
@ -46,7 +50,8 @@ export const useEventSourceClient = (options: UseEventSourceClientOptions) => {
eventSource.value = new EventSource(options.url, { withCredentials: true });
eventSource.value.addEventListener('open', onConnected);
eventSource.value.addEventListener('message', onMessage);
eventSource.value.addEventListener('close', onConnectionLost);
// `EventSource` has no `close` event; a failure surfaces as `error`
eventSource.value.addEventListener('error', onConnectionLost);
};
const reconnectTimer = useReconnectTimer({

View File

@ -0,0 +1,39 @@
import { test, expect } from '../../../fixtures/base';
/**
* The default container stack runs the `websocket` push backend, so `sse` needs a
* container of its own. Only meaningful in container mode: the origin check this
* covers is gated on `NODE_ENV=production`, which the image sets but a local
* `pnpm start` does not.
*/
test.use({ capability: { env: { N8N_PUSH_BACKEND: 'sse' } } });
test.beforeEach(({ n8nContainer }) => {
test.skip(!n8nContainer, 'container-only: requires the SSE backend configuration');
});
test.describe(
'SSE push backend',
{
annotation: [{ type: 'owner', description: 'Catalysts' }],
},
() => {
test('should deliver execution results over an SSE push connection', async ({ n8n }) => {
// `EventSource` sends no `Origin` on a same-origin GET, so the connection has to
// be accepted on the strength of `Sec-Fetch-Site` alone.
const pushResponse = n8n.page.waitForResponse((response) =>
response.url().includes('/push?pushRef='),
);
await n8n.start.fromImportedWorkflow('manual-trigger-with-code.json');
expect((await pushResponse).status()).toBe(200);
// Run status per node arrives over that connection, so it cannot show up unless
// the stream stayed open.
await n8n.workflowComposer.executeWorkflowAndWaitForNotification(
'Workflow executed successfully',
);
await expect(n8n.canvas.getNodeSuccessStatusIndicator('Code')).toBeVisible();
});
},
);