Skip to content

Commit 44fa0b1

Browse files
waleedlatif1claude
andcommitted
refactor(rate-limit): scope the per-IP bucket by resource id, not by bucket name
The chat call interpolated the deployment id into `bucketName`, which produces a correct key but puts a per-deployment value into the field both log lines emit as `bucket` — high cardinality on a label meant to name a bucket family, and asymmetric with the `enforceResourceRateLimit` call beside it that takes the id as its own argument. `enforceIpRateLimitWithIndependentBackstop` now takes an optional `resourceId`, so the pair reads the same way and `resourceId` is logged as its own field. The unscoped key shape is unchanged for the existing callers, with a test pinning both shapes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QWh9WFMYNZ6uTFQFUzF8Bj
1 parent 04a8b79 commit 44fa0b1

4 files changed

Lines changed: 67 additions & 9 deletions

File tree

apps/sim/app/api/chat/[identifier]/route.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,10 @@ describe('Chat Identifier API Route', () => {
376376
await POST(req, { params: Promise.resolve({ identifier: 'test-chat' }) })
377377

378378
expect(mockEnforceIpRateLimit).toHaveBeenCalledWith(
379-
'chat-execute:chat-id',
379+
'chat-execute',
380380
req,
381-
expect.objectContaining({ refillIntervalMs: 60_000 })
381+
expect.objectContaining({ refillIntervalMs: 60_000 }),
382+
'chat-id'
382383
)
383384
expect(mockEnforceResourceRateLimit).toHaveBeenCalledWith(
384385
'chat-execute',

apps/sim/app/api/chat/[identifier]/route.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,10 @@ export const POST = withRouteHandler(
228228
// Both buckets apply regardless of the chat's auth type: an email or SSO
229229
// visitor is still not the payer.
230230
const ipLimited = await enforceIpRateLimitWithIndependentBackstop(
231-
`chat-execute:${deployment.id}`,
231+
'chat-execute',
232232
request,
233-
CHAT_EXECUTION_IP_LIMIT
233+
CHAT_EXECUTION_IP_LIMIT,
234+
deployment.id
234235
)
235236
if (ipLimited) return ipLimited
236237

apps/sim/lib/core/rate-limiter/route-helpers.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,53 @@ describe('route-helpers rate limiting', () => {
3737
vi.clearAllMocks()
3838
})
3939

40+
describe('enforceIpRateLimitWithIndependentBackstop', () => {
41+
it('scopes the per-IP bucket to a resource without polluting the bucket name', async () => {
42+
consume.mockResolvedValueOnce({
43+
allowed: true,
44+
tokensRemaining: 19,
45+
resetAt: new Date(Date.now() + 60_000),
46+
})
47+
48+
requestUtilsMockFns.mockGetClientIp.mockReturnValue('203.0.113.9')
49+
50+
const result = await enforceIpRateLimitWithIndependentBackstop(
51+
'chat-execute',
52+
createMockRequest('POST') as any,
53+
{ maxTokens: 40, refillRate: 20, refillIntervalMs: 60_000 },
54+
'chat-1'
55+
)
56+
57+
expect(result).toBeNull()
58+
expect(consume).toHaveBeenCalledWith(
59+
'route:chat-execute:resource:chat-1:ip:203.0.113.9',
60+
1,
61+
expect.anything()
62+
)
63+
})
64+
65+
it('keeps the unscoped key shape when no resource is named', async () => {
66+
consume.mockResolvedValueOnce({
67+
allowed: true,
68+
tokensRemaining: 9,
69+
resetAt: new Date(Date.now() + 60_000),
70+
})
71+
72+
requestUtilsMockFns.mockGetClientIp.mockReturnValue('203.0.113.9')
73+
74+
await enforceIpRateLimitWithIndependentBackstop(
75+
'forget-password',
76+
createMockRequest('POST') as any
77+
)
78+
79+
expect(consume).toHaveBeenCalledWith(
80+
'route:forget-password:ip:203.0.113.9',
81+
1,
82+
expect.anything()
83+
)
84+
})
85+
})
86+
4087
describe('enforceResourceRateLimit', () => {
4188
const config = { maxTokens: 300, refillRate: 300, refillIntervalMs: 60_000 }
4289

apps/sim/lib/core/rate-limiter/route-helpers.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,22 +60,25 @@ async function enforceIpRateLimitWithPolicy(
6060
bucketName: string,
6161
request: NextRequest,
6262
config: TokenBucketConfig,
63-
unresolvedClientPolicy: 'deny' | 'defer'
63+
unresolvedClientPolicy: 'deny' | 'defer',
64+
resourceId?: string
6465
): Promise<NextResponse | null> {
6566
const ip = getClientIp(request)
6667
if (!ip) {
6768
logger.warn('Unable to resolve client IP for public rate limit', {
6869
bucket: bucketName,
70+
resourceId,
6971
unresolvedClientPolicy,
7072
})
7173
return unresolvedClientPolicy === 'deny'
7274
? buildRateLimitResponse(new Date(Date.now() + config.refillIntervalMs))
7375
: null
7476
}
75-
const key = `route:${bucketName}:ip:${ip}`
77+
const scope = resourceId ? `resource:${resourceId}:` : ''
78+
const key = `route:${bucketName}:${scope}ip:${ip}`
7679
const { allowed, resetAt } = await rateLimiter.checkRateLimitDirect(key, config)
7780
if (allowed) return null
78-
logger.warn('IP rate limit exceeded', { bucket: bucketName, ip })
81+
logger.warn('IP rate limit exceeded', { bucket: bucketName, resourceId, ip })
7982
return buildRateLimitResponse(resetAt)
8083
}
8184

@@ -91,13 +94,19 @@ export async function enforceIpRateLimit(
9194
/**
9295
* Apply a per-IP bucket when resolvable, deferring unresolved clients to an
9396
* independent non-IP limit that the caller must enforce before any side effect.
97+
*
98+
* Pass `resourceId` to give each resource its own per-IP budget — the caller
99+
* that pairs this with {@link enforceResourceRateLimit} wants both scoped the
100+
* same way. It belongs here rather than interpolated into `bucketName`, which
101+
* is emitted as a log field and has to stay low-cardinality.
94102
*/
95103
export async function enforceIpRateLimitWithIndependentBackstop(
96104
bucketName: string,
97105
request: NextRequest,
98-
config: TokenBucketConfig = DEFAULT_PUBLIC_IP_ROUTE_LIMIT
106+
config: TokenBucketConfig = DEFAULT_PUBLIC_IP_ROUTE_LIMIT,
107+
resourceId?: string
99108
): Promise<NextResponse | null> {
100-
return enforceIpRateLimitWithPolicy(bucketName, request, config, 'defer')
109+
return enforceIpRateLimitWithPolicy(bucketName, request, config, 'defer', resourceId)
101110
}
102111

103112
/**

0 commit comments

Comments
 (0)