Skip to content

Ipv6 rate limit key error #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kentcdodds merged 6 commits into from
Jan 31, 2026
Merged

Ipv6 rate limit key error #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aa71ec3
Fix IPv6-safe rate limit keys
cursoragent Jan 31, 2026
1e735a2
Handle missing rate limit IP
cursoragent Jan 31, 2026
b365b8b
Guard rate limit IP type
cursoragent Jan 31, 2026
04df8d7
Ensure rate limit IP fallback
cursoragent Jan 31, 2026
9251f84
Default rate limit IP fallback
cursoragent Jan 31, 2026
b785721
Annotate rate limit IP type
cursoragent Jan 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions docs/skills/epic-security/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ Epic Stack uses `express-rate-limit` para prevenir abuso.

```typescript
// server/index.ts
import rateLimit from 'express-rate-limit'
import rateLimit, { ipKeyGenerator } from 'express-rate-limit'

const rateLimitDefault = {
windowMs: 60 * 1000, // 1 minute
Expand All @@ -205,7 +205,8 @@ const rateLimitDefault = {
legacyHeaders: false,
validate: { trustProxy: false },
keyGenerator: (req: express.Request) => {
return req.get('fly-client-ip') ?? `${req.ip}`
const clientIp = req.get('fly-client-ip') ?? req.ip
return ipKeyGenerator(clientIp)
},
}

Expand Down Expand Up @@ -508,13 +509,18 @@ export default function SignupRoute({ actionData }: Route.ComponentProps) {

```typescript
// server/index.ts
import { ipKeyGenerator } from 'express-rate-limit'
const apiRateLimit = rateLimit({
...rateLimitDefault,
windowMs: 60 * 1000,
limit: 100, // 100 requests per minute for API
keyGenerator: (req) => {
const apiKey = req.get('X-API-Key')
return apiKey ?? req.get('fly-client-ip') ?? req.ip
if (apiKey) {
return apiKey
}
const clientIp = req.get('fly-client-ip') ?? req.ip
return ipKeyGenerator(clientIp)
},
})

Expand Down
9 changes: 7 additions & 2 deletions server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import chalk from 'chalk'
import closeWithGrace from 'close-with-grace'
import compression from 'compression'
import express from 'express'
import rateLimit from 'express-rate-limit'
import rateLimit, { ipKeyGenerator } from 'express-rate-limit'
import getPort, { portNumbers } from 'get-port'
import helmet from 'helmet'
import morgan from 'morgan'
Expand Down Expand Up @@ -128,7 +128,12 @@ const rateLimitDefault = {
// When sitting behind a CDN such as cloudflare, replace fly-client-ip with the CDN
// specific header such as cf-connecting-ip
keyGenerator: (req: express.Request) => {
return req.get('fly-client-ip') ?? `${req.ip}`
const clientIp: string =
req.get('fly-client-ip') ??
req.ip ??
req.socket.remoteAddress ??
'0.0.0.0'
return ipKeyGenerator(clientIp)

@cursor cursor Bot Jan 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential incorrect argument type passed to ipKeyGenerator

High Severity

The ipKeyGenerator function from express-rate-limit is a KeyGenerator type that typically expects a Request object as its parameter, not a string. The code calls ipKeyGenerator(clientIp) where clientIp is a string. If this function expects Request, it would attempt to access clientIp.ip (undefined on a string), potentially causing runtime errors or the same ERR_ERL_KEY_GEN_IPV6 error the PR aims to fix. The test plan only initializes the rate limiter without making actual requests, so this wouldn't be caught during testing.

Additional Locations (1)

Fix in Cursor Fix in Web

@cursor cursor Bot Jan 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bugbot Autofix determined this is a false positive.

In express-rate-limit v8.2.1, ipKeyGenerator is defined as ipKeyGenerator(ip: string, ipv6Subnet?: number | false), so passing the clientIp string is the correct usage.

},
}

Expand Down
Loading