Skip to content

Commit cc2cef5

Browse files
committed
fix(security): restore the X-Real-IP fallback for webhook allowlists
getAssertedOriginIp only read X-Forwarded-For, but the helper it replaced also accepted X-Real-IP. A proxy that sets only X-Real-IP left the allowlist with no address at all, so every permitted delivery 403'd. Fall back to X-Real-IP when the forwarded chain yields nothing. It is the same question the chain answers — which address does this delivery claim to come from — and with no chain present it is the only record of the sender.
1 parent 98a61e3 commit cc2cef5

2 files changed

Lines changed: 22 additions & 6 deletions

File tree

packages/security/src/client-ip.test.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,20 @@ describe('getAssertedOriginIp', () => {
173173
)
174174
})
175175

176-
it('returns null when there is no forwarded chain or no address in it', () => {
176+
it('falls back to x-real-ip for proxies that set it instead of a chain', () => {
177+
expect(getAssertedOriginIp(req({ 'x-real-ip': '203.0.113.7' }))).toBe('203.0.113.7')
178+
expect(
179+
getAssertedOriginIp(req({ 'x-forwarded-for': 'unknown', 'x-real-ip': '203.0.113.7' }))
180+
).toBe('203.0.113.7')
181+
})
182+
183+
it('prefers the forwarded chain over x-real-ip when both parse', () => {
184+
expect(
185+
getAssertedOriginIp(req({ 'x-forwarded-for': '203.0.113.7', 'x-real-ip': '10.0.0.1' }))
186+
).toBe('203.0.113.7')
187+
})
188+
189+
it('returns null when no header yields an address', () => {
177190
expect(getAssertedOriginIp(req({}))).toBeNull()
178191
expect(getAssertedOriginIp(req({ 'x-forwarded-for': 'unknown' }))).toBeNull()
179192
expect(canonicalizeIp('not-an-ip')).toBeNull()

packages/security/src/client-ip.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,12 +170,15 @@ export function resolveClientIp(
170170

171171
/**
172172
* The **leftmost** `X-Forwarded-For` hop — the origin address *asserted* by the
173-
* caller — in canonical form, or `null` when none parses.
173+
* caller — in canonical form, falling back to `X-Real-IP`, or `null` when
174+
* neither header yields an address.
174175
*
175176
* Deliberately the opposite end of the chain from {@link resolveClientIp}, and
176177
* usable for exactly one thing: comparing against an operator-configured
177178
* allowlist of expected senders, where the question is "which address does this
178-
* delivery claim to come from" rather than "who do I throttle".
179+
* delivery claim to come from" rather than "who do I throttle". The `X-Real-IP`
180+
* fallback matters because a proxy may set it *instead of* a forwarded chain,
181+
* and it is then the only record of the sender.
179182
*
180183
* **Never key a rate limit, quota, or lockout on this.** Under any proxy that
181184
* appends to `X-Forwarded-For` the value is caller-controlled and can be rotated
@@ -184,12 +187,12 @@ export function resolveClientIp(
184187
*/
185188
export function getAssertedOriginIp(request: ClientIpHeaderSource): string | null {
186189
const forwarded = request.headers.get('x-forwarded-for')
187-
if (!forwarded) return null
188-
for (const hop of forwarded.split(',')) {
190+
for (const hop of forwarded?.split(',') ?? []) {
189191
const addr = parseHop(hop)
190192
if (addr) return addr.toString()
191193
}
192-
return null
194+
const realIp = parseHop(request.headers.get('x-real-ip') ?? '')
195+
return realIp ? realIp.toString() : null
193196
}
194197

195198
/**

0 commit comments

Comments
 (0)