k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cors): the instance's own address no longer trips ALLOWED_ORIGINS

Browse Source 
With ALLOWED_ORIGINS set, every request carrying an Origin that was not on
the list was failed with an unhandled error, which the exception filter
answered with a 500. A browser sends Origin on every POST and WebSocket
upgrade, including same-origin ones, so opening TREK under any address the
list did not name (the LAN IP, a second hostname, http instead of https)
broke the login with "Internal Server Error" while another browser that
used the listed address worked.

A request whose Origin is the host it was sent to is now treated as
same-origin and always passes, for REST and the /ws upgrade alike. A
foreign origin is still refused, now with a 403 "Not allowed by CORS" and a
warning in the log naming the origin to add.

Closes #2543
This commit is contained in:
Maurice
2026-09-29 20:41:48 +02:00
parent 7b0333a0c1
commit bd5a6cc748
6 changed files with 198 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/middleware/globalMiddleware.ts
+24 -12
View File
@@ -5,6 +5,7 @@ import helmet from 'helmet';
import cookieParser from 'cookie-parser';
import { readEnv, type AppEnv } from '../app-config';
import { logDebug, logWarn, logError } from '../nest/audit/audit-log.logger';
import { isSameHostOrigin } from '../nest/common/same-origin';
/**
* Field names redacted from request-log query/body dumps (case-insensitive —
@@ -162,17 +163,28 @@ export function applyGlobalMiddleware(
const allowedOrigins = http.corsOrigins;
let corsOrigin: cors.CorsOptions['origin'];
if (allowedOrigins) {
corsOrigin = (origin: string | undefined, callback: (err: Error | null, allow?: boolean) => void) => {
if (!origin || allowedOrigins.includes(origin)) callback(null, true);
else callback(new Error('Not allowed by CORS'));
};
} else if (isProduction) {
corsOrigin = false;
} else {
corsOrigin = true;
}
// With ALLOWED_ORIGINS set, a request from anywhere else is refused outright.
// Two things keep that from hitting the instance's own pages (#2543): a request
// whose Origin is the host it was sent to is same-origin and always passes, and
// a refusal is a 403 naming the cause instead of an unhandled error, which the
// exception filter turned into a bare 500 on the login screen.
const warnedOrigins = new Set<string>();
const corsOptions: cors.CorsOptions | cors.CorsOptionsDelegate<Request> = allowedOrigins
? (req: Request, callback: (err: Error | null, options?: cors.CorsOptions) => void) => {
const origin = req.headers.origin;
if (!origin || allowedOrigins.includes(origin) || isSameHostOrigin(origin, req.headers.host)) {
callback(null, { origin: true, credentials: true });
return;
}
if (!warnedOrigins.has(origin) && warnedOrigins.size < 50) {
warnedOrigins.add(origin);
logWarn(
`CORS: refused origin ${origin} for host ${req.headers.host ?? '(none)'}; add it to ALLOWED_ORIGINS if it is yours`,
);
}
callback(Object.assign(new Error('Not allowed by CORS'), { statusCode: 403 }));
}
: { origin: isProduction ? false : true, credentials: true };
const shouldForceHttps = http.forceHttps;
// HSTS is worth enabling any time we're serving production traffic,
@@ -225,7 +237,7 @@ export function applyGlobalMiddleware(
}
},
);
app.use(cors({ origin: corsOrigin, credentials: true }));
app.use(cors(corsOptions));
app.use(helmet({
contentSecurityPolicy: {
directives: {
server/src/nest/common/same-origin.ts
+40
View File
@@ -0,0 +1,40 @@
/**
* Whether a browser `Origin` names the very host the request was sent to.
*
* `ALLOWED_ORIGINS` exists to let OTHER origins in. A request whose Origin is the
* instance's own host is not cross-origin at all, yet a browser still attaches the
* header to every POST and to every WebSocket upgrade. Checking only the list
* turned an ordinary login into a refusal as soon as the list and the address in
* the address bar differed in any way (another hostname, the LAN IP, http against
* https), which is #2543.
*
* The comparison is against the `Host` header only. The browser sets it from the
* URL it was asked to load, and a page on another site cannot choose it, so a
* foreign Origin never matches. `X-Forwarded-Host` is deliberately not consulted:
* that one is client-controlled unless a proxy overwrites it.
*
* The scheme is compared through the host normalisation only (a default port on
* the Host header is dropped for the Origin's own scheme), not for equality: TLS
* usually ends at a reverse proxy, so the server itself cannot tell which scheme
* the page was loaded over.
*/
export function isSameHostOrigin(origin: string | undefined, hostHeader: string | undefined): boolean {
if (!origin || !hostHeader) return false;
const host = hostHeader.trim();
// A Host is a hostname with an optional port, nothing that a URL parser would
// read as userinfo, a path or a query.
if (!host || /[\s/\\@?#]/.test(host)) return false;
let originUrl: URL;
try {
originUrl = new URL(origin);
} catch {
return false; // `null` and anything else that is not a URL
}
if (originUrl.protocol !== 'http:' && originUrl.protocol !== 'https:') return false;
try {
const requestHost = new URL(`${originUrl.protocol}//${host}`).host;
return requestHost !== '' && requestHost === originUrl.host;
} catch {
return false;
}
}
server/src/nest/realtime/trek-ws.adapter.ts
+6 -3
View File
@@ -1,12 +1,13 @@
import { WsAdapter } from '@nestjs/platform-ws';
import type { INestApplicationContext } from '@nestjs/common';
import type { Server as HttpServer } from 'node:http';
import type { IncomingMessage, Server as HttpServer } from 'node:http';
import type { MessageMappingProperties } from '@nestjs/websockets';
import { WebSocketServer } from 'ws';
import type { Observable } from 'rxjs';
import { readEnv } from '../../app-config';
import { setServer, type TrekWebSocket } from './ws-state';
import { logError } from '../audit/audit-log.logger';
import { isSameHostOrigin } from '../common/same-origin';
// Per-connection message rate limiting. It lives in the adapter, not in the
// gateway's handlers, because the original counted EVERY inbound frame before
@@ -122,10 +123,12 @@ export class TrekWsAdapter extends WsAdapter {
...(allowedOrigins
? {
verifyClient: (
{ origin }: { origin: string },
{ origin, req }: { origin: string; req: IncomingMessage },
cb: (ok: boolean, code?: number, msg?: string) => void,
) => {
if (!origin || allowedOrigins.includes(origin)) cb(true);
// Same rule as the REST CORS check (#2543): the instance's own host
// is never cross-origin, whatever ALLOWED_ORIGINS lists.
if (!origin || allowedOrigins.includes(origin) || isSameHostOrigin(origin, req?.headers.host)) cb(true);
else cb(false, 403, 'Origin not allowed');
},
}
server/tests/unit/middleware/globalMiddleware.cors.test.ts
+105
View File
@@ -0,0 +1,105 @@
import { describe, it, expect } from 'vitest';
import express, { type NextFunction, type Request, type Response } from 'express';
import request from 'supertest';
import { applyGlobalMiddleware } from '../../../src/middleware/globalMiddleware';
import { readEnv } from '../../../src/app-config';
import { isSameHostOrigin } from '../../../src/nest/common/same-origin';
/**
* An app with ALLOWED_ORIGINS set, and the error mapping TrekExceptionFilter
* applies to a plain error (statusCode, else 500).
*/
function appWithAllowlist(corsOrigins: string[] | null) {
const app = express();
applyGlobalMiddleware(app, { http: { ...readEnv().http, corsOrigins } });
app.post('/api/auth/login', (_req, res) => res.json({ ok: true }));
app.use((err: { statusCode?: number; message?: string }, _req: Request, res: Response, _next: NextFunction) => {
const status = err.statusCode || 500;
res.status(status).json({ error: status < 500 ? err.message : 'Internal server error' });
});
return app;
}
describe('CORS with ALLOWED_ORIGINS set (#2543)', () => {
const app = appWithAllowlist(['https://trek.example.com']);
it('lets a request through whose Origin is the host it was sent to', async () => {
// The instance reached under another name than the one in ALLOWED_ORIGINS
// (its LAN address, http instead of https): the login POST carries the page's
// own Origin, and refusing it answered the login form with a 500.
const res = await request(app)
.post('/api/auth/login')
.set('Host', '192.168.1.20:30080')
.set('Origin', 'http://192.168.1.20:30080')
.send({});
expect(res.status).toBe(200);
expect(res.headers['access-control-allow-origin']).toBe('http://192.168.1.20:30080');
});
it('still lets the listed origin through, with credentials', async () => {
const res = await request(app).post('/api/auth/login').set('Origin', 'https://trek.example.com').send({});
expect(res.status).toBe(200);
expect(res.headers['access-control-allow-origin']).toBe('https://trek.example.com');
expect(res.headers['access-control-allow-credentials']).toBe('true');
});
it('still lets a request without Origin through', async () => {
const res = await request(app).post('/api/auth/login').send({});
expect(res.status).toBe(200);
});
it('refuses a foreign origin with a 403 that names the cause, not a 500', async () => {
const res = await request(app)
.post('/api/auth/login')
.set('Host', 'trek.example.com')
.set('Origin', 'https://evil.example')
.send({});
expect(res.status).toBe(403);
expect(res.body).toEqual({ error: 'Not allowed by CORS' });
expect(res.headers['access-control-allow-origin']).toBeUndefined();
});
it('refuses Origin: null', async () => {
const res = await request(app).post('/api/auth/login').set('Origin', 'null').send({});
expect(res.status).toBe(403);
});
});
describe('CORS without ALLOWED_ORIGINS', () => {
it('passes everything through as before (the test env is not production)', async () => {
const res = await request(appWithAllowlist(null))
.post('/api/auth/login')
.set('Origin', 'https://anything.example')
.send({});
expect(res.status).toBe(200);
expect(res.headers['access-control-allow-origin']).toBe('https://anything.example');
});
});
describe('isSameHostOrigin', () => {
it('matches host and port', () => {
expect(isSameHostOrigin('http://192.168.1.20:30080', '192.168.1.20:30080')).toBe(true);
expect(isSameHostOrigin('https://trek.example.com', 'trek.example.com')).toBe(true);
expect(isSameHostOrigin('https://Trek.Example.com', 'TREK.example.com')).toBe(true);
});
it('drops the default port of the Origin\'s scheme from the Host', () => {
expect(isSameHostOrigin('https://trek.example.com', 'trek.example.com:443')).toBe(true);
expect(isSameHostOrigin('http://trek.example.com', 'trek.example.com:80')).toBe(true);
});
it('does not match another host or another port', () => {
expect(isSameHostOrigin('https://evil.example', 'trek.example.com')).toBe(false);
expect(isSameHostOrigin('http://192.168.1.20:3000', '192.168.1.20:30080')).toBe(false);
expect(isSameHostOrigin('https://trek.example.com.evil.example', 'trek.example.com')).toBe(false);
});
it('refuses anything that is not an http(s) origin or a plain host', () => {
expect(isSameHostOrigin('null', 'trek.example.com')).toBe(false);
expect(isSameHostOrigin(undefined, 'trek.example.com')).toBe(false);
expect(isSameHostOrigin('https://trek.example.com', undefined)).toBe(false);
expect(isSameHostOrigin('file://trek.example.com', 'trek.example.com')).toBe(false);
expect(isSameHostOrigin('https://evil.example', 'x@evil.example')).toBe(false);
expect(isSameHostOrigin('https://evil.example', 'evil.example/path')).toBe(false);
});
});
server/tests/unit/nest/trek-ws-adapter.test.ts
+22
View File
@@ -282,6 +282,28 @@ describe('TrekWsAdapter server creation', () => {
}
});
it('WSAD-031d: a same-host origin passes even when the allowlist names another (#2543)', () => {
// A browser always sends Origin on an upgrade. Reached at its LAN address or
// under a second name, the instance is still talking to its own page, and
// refusing that left the planner without live updates.
wsOrigins.value = ['https://trip.example'];
const ad = new TrekWsAdapter(httpServer);
const server = ad.create(0, { path: '/ws' }) as { options: { verifyClient?: unknown } };
try {
const verify = server.options.verifyClient as (
info: { origin: string; req: { headers: { host?: string } } },
cb: (ok: boolean, code?: number, msg?: string) => void,
) => void;
const seen: unknown[][] = [];
verify({ origin: 'http://192.168.1.20:3000', req: { headers: { host: '192.168.1.20:3000' } } }, (...args) => seen.push(args));
// A foreign page cannot pick the Host header, so it still gets the 403.
verify({ origin: 'https://evil.example', req: { headers: { host: '192.168.1.20:3000' } } }, (...args) => seen.push(args));
expect(seen).toEqual([[true], [false, 403, 'Origin not allowed']]);
} finally {
void ad.close(server as never);
}
});
it('WSAD-030b: falls back to /ws when the gateway declares no path', () => {
wsOrigins.value = null;
const ad = new TrekWsAdapter(httpServer);
wiki/Troubleshooting.md
+1 -1
View File
@@ -334,7 +334,7 @@ docker logs <container> 2>&1 | grep -E "SMTP test email (sent|failed)|SMTP test
## CORS error — API requests blocked in the browser
**Cause:** If `ALLOWED_ORIGINS` is set, only those origins are permitted. Any request from a different origin is rejected with a CORS error visible in the browser console.
**Cause:** If `ALLOWED_ORIGINS` is set, only those origins are permitted, plus the host the request was sent to (the address in the browser's address bar is never cross-origin to itself). A request from any other origin is refused with `403 Not allowed by CORS`, and the server logs a `CORS: refused origin ...` warning naming the origin to add.
**Fix:** Add your origin to the comma-separated list: