fix: only trust forwarded IP header from configured trusted proxies#493
Conversation
There was a problem hiding this comment.
Pull request overview
This PR mitigates IP spoofing by only trusting x-forwarded-for (or configured network.remoteIpHeader) when the connection comes from an allow-listed proxy IP (network.trustedProxies).
Changes:
- Added
network.trustedProxiessetting and updatedgetRemoteAddress()to require a trusted proxy before using forwarded headers. - Added IP normalization for IPv4-mapped proxy socket addresses and improved forwarded-header parsing (trim).
- Updated unit tests and configuration docs/default settings to include the new option.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/http.ts |
Implements trusted-proxy gating + IP normalization and warning behavior for forwarded headers. |
src/@types/settings.ts |
Adds trustedProxies?: string[] to the Network settings type. |
resources/default-settings.yaml |
Documents and introduces network.trustedProxies in the default config. |
CONFIGURATION.md |
Documents the new network.trustedProxies setting. |
test/unit/utils/http.spec.ts |
Adds unit tests covering trusted/untrusted proxy behavior and IPv4-mapped normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const trustedProxies = settings.network?.trustedProxies | ||
| if (header && (!Array.isArray(trustedProxies) || trustedProxies.length === 0)) { | ||
| console.warn('WARNING: network.remoteIpHeader is set but network.trustedProxies is empty. Forwarded headers will be ignored. Add your proxy IP to network.trustedProxies.') | ||
| } |
There was a problem hiding this comment.
The new warning for network.remoteIpHeader + empty network.trustedProxies runs inside getRemoteAddress(), which is called per request/connection and can spam logs (and add overhead) in misconfigured or default setups. Consider emitting this warning once (e.g., via a module-level "warned" flag) or moving validation to settings load/startup instead of the hot path.
|
@kanishka0411 looks like we have some conflicts now. I'm wondering if we should re-order CONFIGURATION.md list of settings A-Z so there's less conflicts. |
6892bbd to
6ebff39
Compare
|
@copilot resolve the merge conflicts in this pull request |
|
@copilot resolve the merge conflicts in this pull request |
6ebff39 to
0ac3cab
Compare
18bffa7 to
c638687
Compare
c638687 to
47afc07
Compare
Description
Added network.trustedProxies config option. The x-forwarded-for header (or any configured remoteIpHeader) is now only trusted when the request's socket IP is in the trustedProxies list. Otherwise Nostream falls back to the real socket address. Also added a runtime warning when remoteIpHeader is set but trustedProxies is empty.
Related Issue
Fixes #492
Motivation and Context
Previously any client could set x-forwarded-for to any IP and Nostream would blindly trust it. This allowed attackers to spoof their IP and bypass rate limits, IP blacklists, and payment throttling.
How Has This Been Tested?
Added unit tests for:
Screenshots (if appropriate):
Types of changes
Checklist: