fix: only trust proxy headers in rate-limit when TRUSTED_PROXY is set
X-Forwarded-For, X-Real-IP, and CF-Connecting-IP headers were blindly trusted, allowing attackers to bypass rate limiting by spoofing different IPs. Now only trusted when TRUSTED_PROXY env var is configured. Falls back to Node.js socket remoteAddress. Add tests verifying proxy headers are ignored without TRUSTED_PROXY and respected when it is set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
0131949643
commit
5bf557ba6a
@@ -9,6 +9,8 @@ describe('rateLimit', () => {
|
|||||||
vi.resetModules()
|
vi.resetModules()
|
||||||
// Set production mode before importing so isDev = false
|
// Set production mode before importing so isDev = false
|
||||||
process.env.NODE_ENV = 'production'
|
process.env.NODE_ENV = 'production'
|
||||||
|
// Trust proxy headers in tests (like behind Cloudflare/nginx)
|
||||||
|
process.env.TRUSTED_PROXY = '1'
|
||||||
const mod = await import('./rate-limit.js')
|
const mod = await import('./rate-limit.js')
|
||||||
rateLimit = mod.rateLimit
|
rateLimit = mod.rateLimit
|
||||||
cleanup = mod.cleanupInterval
|
cleanup = mod.cleanupInterval
|
||||||
@@ -16,6 +18,7 @@ describe('rateLimit', () => {
|
|||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
process.env.NODE_ENV = 'test'
|
process.env.NODE_ENV = 'test'
|
||||||
|
delete process.env.TRUSTED_PROXY
|
||||||
clearInterval(cleanup)
|
clearInterval(cleanup)
|
||||||
vi.restoreAllMocks()
|
vi.restoreAllMocks()
|
||||||
})
|
})
|
||||||
@@ -117,6 +120,42 @@ describe('rateLimit', () => {
|
|||||||
vi.useRealTimers()
|
vi.useRealTimers()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('ignores proxy headers when TRUSTED_PROXY is not set', async () => {
|
||||||
|
// Re-import without TRUSTED_PROXY
|
||||||
|
vi.resetModules()
|
||||||
|
process.env.NODE_ENV = 'production'
|
||||||
|
delete process.env.TRUSTED_PROXY
|
||||||
|
const mod = await import('./rate-limit.js')
|
||||||
|
const rl = mod.rateLimit
|
||||||
|
cleanup = mod.cleanupInterval
|
||||||
|
|
||||||
|
const app = new Hono()
|
||||||
|
app.get('/test', rl(10_000, 2), (c) => c.json({ ok: true }))
|
||||||
|
|
||||||
|
// All requests use 'unknown' as key (no proxy headers trusted)
|
||||||
|
// so they share the same bucket
|
||||||
|
const res1 = await app.request('/test', { headers: { 'x-forwarded-for': '1.1.1.1' } })
|
||||||
|
expect(res1.status).toBe(200)
|
||||||
|
const res2 = await app.request('/test', { headers: { 'x-forwarded-for': '2.2.2.2' } })
|
||||||
|
expect(res2.status).toBe(200)
|
||||||
|
// 3rd request hits limit because all mapped to 'unknown'
|
||||||
|
const res3 = await app.request('/test', { headers: { 'x-forwarded-for': '3.3.3.3' } })
|
||||||
|
expect(res3.status).toBe(429)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('trusts proxy headers when TRUSTED_PROXY is set', async () => {
|
||||||
|
const app = new Hono()
|
||||||
|
app.get('/test', rateLimit(10_000, 2), (c) => c.json({ ok: true }))
|
||||||
|
|
||||||
|
// Different X-Forwarded-For IPs are tracked independently
|
||||||
|
const res1 = await app.request('/test', { headers: { 'x-forwarded-for': '1.1.1.1' } })
|
||||||
|
expect(res1.status).toBe(200)
|
||||||
|
const res2 = await app.request('/test', { headers: { 'x-forwarded-for': '2.2.2.2' } })
|
||||||
|
expect(res2.status).toBe(200)
|
||||||
|
const res3 = await app.request('/test', { headers: { 'x-forwarded-for': '3.3.3.3' } })
|
||||||
|
expect(res3.status).toBe(200) // different IP, still allowed
|
||||||
|
})
|
||||||
|
|
||||||
it('poll endpoint config: 429 after 30 requests in 1s window', async () => {
|
it('poll endpoint config: 429 after 30 requests in 1s window', async () => {
|
||||||
const app = new Hono()
|
const app = new Hono()
|
||||||
// Matches the actual /poll endpoint configuration
|
// Matches the actual /poll endpoint configuration
|
||||||
|
|||||||
@@ -34,10 +34,21 @@ export const cleanupInterval = setInterval(() => {
|
|||||||
}, 5 * 60 * 1000)
|
}, 5 * 60 * 1000)
|
||||||
|
|
||||||
function getIp(c: Context): string {
|
function getIp(c: Context): string {
|
||||||
return c.req.header('cf-connecting-ip')
|
// Only trust proxy headers when TRUSTED_PROXY is configured
|
||||||
|| c.req.header('x-real-ip')
|
if (process.env.TRUSTED_PROXY) {
|
||||||
|| c.req.header('x-forwarded-for')?.split(',')[0].trim()
|
const cfIp = c.req.header('cf-connecting-ip')
|
||||||
|| 'unknown'
|
if (cfIp) return cfIp
|
||||||
|
const realIp = c.req.header('x-real-ip')
|
||||||
|
if (realIp) return realIp
|
||||||
|
const xff = c.req.header('x-forwarded-for')
|
||||||
|
if (xff) return xff.split(',')[0].trim()
|
||||||
|
}
|
||||||
|
// Fall back to connection IP from Node.js socket
|
||||||
|
const incoming = (c.env as Record<string, any>)?.incoming
|
||||||
|
if (incoming?.socket?.remoteAddress) {
|
||||||
|
return incoming.socket.remoteAddress
|
||||||
|
}
|
||||||
|
return 'unknown'
|
||||||
}
|
}
|
||||||
|
|
||||||
export function rateLimit(windowMs: number, maxHits: number) {
|
export function rateLimit(windowMs: number, maxHits: number) {
|
||||||
|
|||||||
Reference in New Issue
Block a user