Skip to content

Commit 2f3657c

Browse files
waleedlatif1waleedlatif1
committed
fix(rate-limit): address PR review — drop success field from 429 body, fall back to per-IP when JWT auth lacks userId
1 parent 6a7341f commit 2f3657c

11 files changed

Lines changed: 92 additions & 19 deletions

File tree

‎apps/sim/app/api/tools/a2a/cancel-task/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { createA2AClient } from '@/lib/a2a/utils'
55
import { a2aCancelTaskContract } from '@/lib/api/contracts/tools/a2a'
66
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
77
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
8-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
8+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
99
import { generateRequestId } from '@/lib/core/utils/request'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111

@@ -30,7 +30,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3030
)
3131
}
3232

33-
const rateLimited = await enforceUserRateLimit('a2a-cancel-task', authResult.userId!)
33+
const rateLimited = await enforceUserOrIpRateLimit(
34+
'a2a-cancel-task',
35+
authResult.userId,
36+
request
37+
)
3438
if (rateLimited) return rateLimited
3539

3640
const parsed = await parseRequest(

‎apps/sim/app/api/tools/a2a/delete-push-notification/route.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createA2AClient } from '@/lib/a2a/utils'
44
import { a2aDeletePushNotificationContract } from '@/lib/api/contracts/tools/a2a'
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
7-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
7+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
88
import { generateRequestId } from '@/lib/core/utils/request'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1010

@@ -31,9 +31,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3131
)
3232
}
3333

34-
const rateLimited = await enforceUserRateLimit(
34+
const rateLimited = await enforceUserOrIpRateLimit(
3535
'a2a-delete-push-notification',
36-
authResult.userId!
36+
authResult.userId,
37+
request
3738
)
3839
if (rateLimited) return rateLimited
3940

‎apps/sim/app/api/tools/a2a/get-agent-card/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createA2AClient } from '@/lib/a2a/utils'
44
import { a2aGetAgentCardContract } from '@/lib/api/contracts/tools/a2a'
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
7-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
7+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
88
import { generateRequestId } from '@/lib/core/utils/request'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1010

@@ -29,7 +29,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
2929
)
3030
}
3131

32-
const rateLimited = await enforceUserRateLimit('a2a-get-agent-card', authResult.userId!)
32+
const rateLimited = await enforceUserOrIpRateLimit(
33+
'a2a-get-agent-card',
34+
authResult.userId,
35+
request
36+
)
3337
if (rateLimited) return rateLimited
3438

3539
logger.info(

‎apps/sim/app/api/tools/a2a/get-push-notification/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createA2AClient } from '@/lib/a2a/utils'
44
import { a2aGetPushNotificationContract } from '@/lib/api/contracts/tools/a2a'
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
7-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
7+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
88
import { generateRequestId } from '@/lib/core/utils/request'
99
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1010

@@ -31,7 +31,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3131
)
3232
}
3333

34-
const rateLimited = await enforceUserRateLimit('a2a-get-push-notification', authResult.userId!)
34+
const rateLimited = await enforceUserOrIpRateLimit(
35+
'a2a-get-push-notification',
36+
authResult.userId,
37+
request
38+
)
3539
if (rateLimited) return rateLimited
3640

3741
logger.info(

‎apps/sim/app/api/tools/a2a/get-task/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { createA2AClient } from '@/lib/a2a/utils'
55
import { a2aGetTaskContract } from '@/lib/api/contracts/tools/a2a'
66
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
77
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
8-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
8+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
99
import { generateRequestId } from '@/lib/core/utils/request'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111

@@ -30,7 +30,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3030
)
3131
}
3232

33-
const rateLimited = await enforceUserRateLimit('a2a-get-task', authResult.userId!)
33+
const rateLimited = await enforceUserOrIpRateLimit('a2a-get-task', authResult.userId, request)
3434
if (rateLimited) return rateLimited
3535

3636
logger.info(`[${requestId}] Authenticated A2A get task request via ${authResult.authType}`, {

‎apps/sim/app/api/tools/a2a/resubscribe/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { createA2AClient, extractTextContent, isTerminalState } from '@/lib/a2a/
1212
import { a2aResubscribeContract } from '@/lib/api/contracts/tools/a2a'
1313
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
1414
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
15-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
15+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
1616
import { generateRequestId } from '@/lib/core/utils/request'
1717
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1818

@@ -37,7 +37,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3737
)
3838
}
3939

40-
const rateLimited = await enforceUserRateLimit('a2a-resubscribe', authResult.userId!)
40+
const rateLimited = await enforceUserOrIpRateLimit(
41+
'a2a-resubscribe',
42+
authResult.userId,
43+
request
44+
)
4145
if (rateLimited) return rateLimited
4246

4347
const parsed = await parseRequest(

‎apps/sim/app/api/tools/a2a/send-message/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { createA2AClient, extractTextContent, isTerminalState } from '@/lib/a2a/
77
import { a2aSendMessageContract } from '@/lib/api/contracts/tools/a2a'
88
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
99
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
10-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
10+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
1111
import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
1212
import { generateRequestId } from '@/lib/core/utils/request'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -33,7 +33,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3333
)
3434
}
3535

36-
const rateLimited = await enforceUserRateLimit('a2a-send-message', authResult.userId!)
36+
const rateLimited = await enforceUserOrIpRateLimit(
37+
'a2a-send-message',
38+
authResult.userId,
39+
request
40+
)
3741
if (rateLimited) return rateLimited
3842

3943
logger.info(

‎apps/sim/app/api/tools/a2a/set-push-notification/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { createA2AClient } from '@/lib/a2a/utils'
44
import { a2aSetPushNotificationContract } from '@/lib/api/contracts/tools/a2a'
55
import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
66
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
7-
import { enforceUserRateLimit } from '@/lib/core/rate-limiter'
7+
import { enforceUserOrIpRateLimit } from '@/lib/core/rate-limiter'
88
import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
99
import { generateRequestId } from '@/lib/core/utils/request'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -32,7 +32,11 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
3232
)
3333
}
3434

35-
const rateLimited = await enforceUserRateLimit('a2a-set-push-notification', authResult.userId!)
35+
const rateLimited = await enforceUserOrIpRateLimit(
36+
'a2a-set-push-notification',
37+
authResult.userId,
38+
request
39+
)
3640
if (rateLimited) return rateLimited
3741

3842
const parsed = await parseRequest(

‎apps/sim/lib/core/rate-limiter/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export {
1212
DEFAULT_PUBLIC_IP_ROUTE_LIMIT,
1313
DEFAULT_USER_ROUTE_LIMIT,
1414
enforceIpRateLimit,
15+
enforceUserOrIpRateLimit,
1516
enforceUserRateLimit,
1617
} from './route-helpers'
1718
export type { TokenBucketConfig } from './storage'

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ function passThroughClientIp() {
3131
)
3232
}
3333

34-
import { enforceIpRateLimit, enforceUserRateLimit } from './route-helpers'
34+
import { enforceIpRateLimit, enforceUserOrIpRateLimit, enforceUserRateLimit } from './route-helpers'
3535

3636
const consume = mockAdapter.consumeTokens as Mock
3737

@@ -157,4 +157,36 @@ describe('route-helpers rate limiting', () => {
157157
expect(result?.headers.get('Retry-After')).toBe('60')
158158
})
159159
})
160+
161+
describe('enforceUserOrIpRateLimit', () => {
162+
beforeEach(() => {
163+
passThroughClientIp()
164+
})
165+
166+
it('keys per-user when userId is present', async () => {
167+
consume.mockResolvedValueOnce({
168+
allowed: true,
169+
tokensRemaining: 59,
170+
resetAt: new Date(),
171+
})
172+
const request = createMockRequest('POST', undefined, { 'x-forwarded-for': '203.0.113.7' })
173+
174+
await enforceUserOrIpRateLimit('a2a-test', 'user-1', request)
175+
176+
expect(consume).toHaveBeenCalledWith('route:a2a-test:user:user-1', 1, expect.any(Object))
177+
})
178+
179+
it('falls back to per-IP when userId is undefined', async () => {
180+
consume.mockResolvedValueOnce({
181+
allowed: true,
182+
tokensRemaining: 59,
183+
resetAt: new Date(),
184+
})
185+
const request = createMockRequest('POST', undefined, { 'x-forwarded-for': '203.0.113.7' })
186+
187+
await enforceUserOrIpRateLimit('a2a-test', undefined, request)
188+
189+
expect(consume).toHaveBeenCalledWith('route:a2a-test:ip:203.0.113.7', 1, expect.any(Object))
190+
})
191+
})
160192
})

0 commit comments

Comments
 (0)