Skip to content

Commit 71eac00

Browse files
committed
πŸ”’ security(api): warn when trust proxy is boolean true
DD_SERVER_TRUSTPROXY=true trusts ALL X-Forwarded-For hops, letting clients spoof req.ip and evade the per-IP login lockout. Emit a startup warning steering operators to a hop count (DD_SERVER_TRUSTPROXY=1). Trust proxy behavior itself is unchanged.
1 parent 9431c1d commit 71eac00

2 files changed

Lines changed: 64 additions & 0 deletions

File tree

β€Žapp/api/index.test.tsβ€Ž

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,63 @@ describe('API Index', () => {
760760
expect(mockApp.set).toHaveBeenCalledWith('trust proxy', true);
761761
});
762762

763+
test('should warn when trustproxy is boolean true (XFF spoofing risk)', async () => {
764+
mockGetServerConfiguration.mockReturnValue({
765+
enabled: true,
766+
port: 3000,
767+
cors: {},
768+
tls: {},
769+
trustproxy: true,
770+
});
771+
772+
vi.resetModules();
773+
const indexRouter = await import('./index.js');
774+
await indexRouter.init();
775+
776+
expect(mockLog.warn).toHaveBeenCalledWith(
777+
expect.stringContaining('trust proxy is set to boolean true'),
778+
);
779+
expect(mockLog.warn).toHaveBeenCalledWith(expect.stringContaining('DD_SERVER_TRUSTPROXY=1'));
780+
});
781+
782+
test('should not warn when trustproxy is a hop count number', async () => {
783+
mockGetServerConfiguration.mockReturnValue({
784+
enabled: true,
785+
port: 3000,
786+
cors: {},
787+
tls: {},
788+
trustproxy: 1,
789+
});
790+
791+
vi.resetModules();
792+
const indexRouter = await import('./index.js');
793+
await indexRouter.init();
794+
795+
const trustProxyWarning = mockLog.warn.mock.calls.find((args) =>
796+
args[0]?.includes?.('trust proxy is set to boolean true'),
797+
);
798+
expect(trustProxyWarning).toBeUndefined();
799+
});
800+
801+
test('should not warn when trustproxy is false', async () => {
802+
mockGetServerConfiguration.mockReturnValue({
803+
enabled: true,
804+
port: 3000,
805+
cors: {},
806+
tls: {},
807+
trustproxy: false,
808+
});
809+
810+
vi.resetModules();
811+
const indexRouter = await import('./index.js');
812+
await indexRouter.init();
813+
814+
const trustProxyWarning = mockLog.warn.mock.calls.find((args) =>
815+
args[0]?.includes?.('trust proxy is set to boolean true'),
816+
);
817+
expect(trustProxyWarning).toBeUndefined();
818+
});
819+
763820
test('should set json replacer', async () => {
764821
mockGetServerConfiguration.mockReturnValue({
765822
enabled: true,

β€Žapp/api/index.tsβ€Ž

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,13 @@ function createApp() {
195195

196196
// Trust proxy (helpful to resolve public facing hostname & protocol)
197197
if (configuration.trustproxy !== false) {
198+
if (configuration.trustproxy === true) {
199+
log.warn(
200+
'trust proxy is set to boolean true, which trusts ALL X-Forwarded-For hops. ' +
201+
'Clients can spoof req.ip and evade per-IP login lockout. ' +
202+
'Operators behind a single proxy should set DD_SERVER_TRUSTPROXY=1 (the exact hop count) instead.',
203+
);
204+
}
198205
app.set('trust proxy', configuration.trustproxy);
199206
}
200207

0 commit comments

Comments
Β (0)