Skip to content

Commit 79b1ed1

Browse files
authored
fix(api): report the real rate-limit ceiling on every v1 endpoint (#6011)
X-RateLimit-Limit was the bucket's refill rate while X-RateLimit-Remaining was tokens left in the bucket, and createBucketConfig sets maxTokens = refillRate * burstMultiplier. The two headers described different quantities, so remaining routinely exceeded limit — observed live on a team plan as limit 200 alongside remaining 399 — and any client computing used = limit - remaining got a negative number. Report the bucket capacity, which is what remaining counts down from. This lives in the shared checkRateLimit, so it applies to every v1 endpoint: workflows, logs, tables, files, knowledge, audit-logs and copilot. Also stops publishing rate-limit headers on an authentication failure. That path never reaches the bucket, so the previous placeholder advertised a quota that does not exist and told unauthenticated callers they had been throttled. Separately, the shared id schemas reported Zod's default "expected string, received undefined" when a required field was omitted, because .min(1) only fires for a present-but-empty string. Adding the message to the z.string() constructor makes an omitted workspaceId/organizationId/workflowId/fileId name the field it is complaining about — the first thing an API consumer sees on a malformed request. Documents all three headers in the OpenAPI spec as reusable components, including the burst-capacity semantics and the fact that they are absent on authentication failures. They were previously undocumented. Adds the first tests for the v1 middleware.
1 parent 4043341 commit 79b1ed1

5 files changed

Lines changed: 257 additions & 12 deletions

File tree

apps/docs/openapi.json

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7762,13 +7762,22 @@
77627762
}
77637763
},
77647764
"RateLimited": {
7765-
"description": "Rate limit exceeded. Wait for the duration specified in the Retry-After header before retrying.",
7765+
"description": "Rate limit exceeded. Wait for the duration specified in the Retry-After header before retrying. The X-RateLimit-* headers below accompany every authenticated response, not just this one; they are omitted when a request fails authentication, since no rate-limit bucket is consulted in that case.",
77667766
"headers": {
77677767
"Retry-After": {
77687768
"description": "Number of seconds to wait before retrying the request.",
77697769
"schema": {
77707770
"type": "integer"
77717771
}
7772+
},
7773+
"X-RateLimit-Limit": {
7774+
"$ref": "#/components/headers/RateLimitLimit"
7775+
},
7776+
"X-RateLimit-Remaining": {
7777+
"$ref": "#/components/headers/RateLimitRemaining"
7778+
},
7779+
"X-RateLimit-Reset": {
7780+
"$ref": "#/components/headers/RateLimitReset"
77727781
}
77737782
},
77747783
"content": {
@@ -7833,6 +7842,30 @@
78337842
}
78347843
}
78357844
}
7845+
},
7846+
"headers": {
7847+
"RateLimitLimit": {
7848+
"description": "Maximum number of requests the bucket holds \u2014 its burst capacity, which is the per-minute allowance for your plan multiplied by a burst factor. `X-RateLimit-Remaining` counts down from this value, so `X-RateLimit-Limit - X-RateLimit-Remaining` is the number of requests currently consumed.",
7849+
"schema": {
7850+
"type": "integer",
7851+
"example": 400
7852+
}
7853+
},
7854+
"RateLimitRemaining": {
7855+
"description": "Requests still available in the current bucket. Never exceeds `X-RateLimit-Limit`.",
7856+
"schema": {
7857+
"type": "integer",
7858+
"example": 399
7859+
}
7860+
},
7861+
"RateLimitReset": {
7862+
"description": "ISO 8601 timestamp at which the bucket refills.",
7863+
"schema": {
7864+
"type": "string",
7865+
"format": "date-time",
7866+
"example": "2026-07-28T18:28:48.354Z"
7867+
}
7868+
}
78367869
}
78377870
}
78387871
}
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* Pins the rate-limit headers every v1 endpoint publishes. `X-RateLimit-Limit`
5+
* and `X-RateLimit-Remaining` must describe the same quantity — the token
6+
* bucket's capacity and the tokens left in it — or a client computing
7+
* `used = limit - remaining` gets a negative number.
8+
*/
9+
10+
import { createMockRequest } from '@sim/testing'
11+
import { beforeEach, describe, expect, it, vi } from 'vitest'
12+
13+
const { mockAuthenticateV1Request, mockGetSubscription, mockCheckRateLimit, mockGetRateLimit } =
14+
vi.hoisted(() => ({
15+
mockAuthenticateV1Request: vi.fn(),
16+
mockGetSubscription: vi.fn(),
17+
mockCheckRateLimit: vi.fn(),
18+
mockGetRateLimit: vi.fn(),
19+
}))
20+
21+
vi.mock('@/app/api/v1/auth', () => ({
22+
authenticateV1Request: mockAuthenticateV1Request,
23+
}))
24+
25+
vi.mock('@/lib/billing/core/subscription', () => ({
26+
getHighestPrioritySubscription: mockGetSubscription,
27+
}))
28+
29+
vi.mock('@/lib/core/rate-limiter', () => ({
30+
getRateLimit: mockGetRateLimit,
31+
RateLimiter: class {
32+
checkRateLimitWithSubscription = mockCheckRateLimit
33+
},
34+
}))
35+
36+
import { checkRateLimit, createRateLimitResponse } from '@/app/api/v1/middleware'
37+
38+
/** Mirrors `createBucketConfig`: capacity is the per-minute rate x burst multiplier. */
39+
const TEAM_BUCKET = { maxTokens: 400, refillRate: 200, refillIntervalMs: 60_000 }
40+
41+
function request() {
42+
return createMockRequest('GET', undefined, {}, 'http://localhost:3000/api/v1/workflows')
43+
}
44+
45+
describe('checkRateLimit', () => {
46+
beforeEach(() => {
47+
vi.clearAllMocks()
48+
mockAuthenticateV1Request.mockResolvedValue({
49+
authenticated: true,
50+
userId: 'user-1',
51+
keyType: 'personal',
52+
})
53+
mockGetSubscription.mockResolvedValue({ plan: 'team' })
54+
mockGetRateLimit.mockReturnValue(TEAM_BUCKET)
55+
mockCheckRateLimit.mockResolvedValue({
56+
allowed: true,
57+
remaining: 399,
58+
resetAt: new Date('2026-07-28T18:28:48.354Z'),
59+
})
60+
})
61+
62+
it('reports the bucket capacity as the limit, not the refill rate', async () => {
63+
const result = await checkRateLimit(request(), 'workflows')
64+
65+
expect(result.limit).toBe(TEAM_BUCKET.maxTokens)
66+
expect(result.limit).not.toBe(TEAM_BUCKET.refillRate)
67+
})
68+
69+
it('never reports more remaining than the limit', async () => {
70+
const result = await checkRateLimit(request(), 'workflows')
71+
72+
expect(result.remaining).toBeLessThanOrEqual(result.limit)
73+
})
74+
75+
it('reports a zero limit when authentication fails, since no bucket was consulted', async () => {
76+
mockAuthenticateV1Request.mockResolvedValue({
77+
authenticated: false,
78+
error: 'API key required',
79+
})
80+
81+
const result = await checkRateLimit(request(), 'workflows')
82+
83+
expect(result.allowed).toBe(false)
84+
expect(result.limit).toBe(0)
85+
expect(result.error).toBe('API key required')
86+
expect(mockCheckRateLimit).not.toHaveBeenCalled()
87+
})
88+
89+
it('reports a zero limit when the checker itself throws', async () => {
90+
mockCheckRateLimit.mockRejectedValue(new Error('redis down'))
91+
92+
const result = await checkRateLimit(request(), 'workflows')
93+
94+
expect(result.allowed).toBe(false)
95+
expect(result.limit).toBe(0)
96+
})
97+
})
98+
99+
describe('createRateLimitResponse', () => {
100+
const throttled = {
101+
allowed: false,
102+
remaining: 0,
103+
limit: 400,
104+
resetAt: new Date('2026-07-28T18:28:48.354Z'),
105+
retryAfterMs: 30_000,
106+
}
107+
108+
it('publishes consistent headers on a 429', () => {
109+
const response = createRateLimitResponse(throttled)
110+
111+
expect(response.status).toBe(429)
112+
const limit = Number(response.headers.get('X-RateLimit-Limit'))
113+
const remaining = Number(response.headers.get('X-RateLimit-Remaining'))
114+
expect(remaining).toBeLessThanOrEqual(limit)
115+
expect(response.headers.get('X-RateLimit-Reset')).toBe(throttled.resetAt.toISOString())
116+
expect(response.headers.get('Retry-After')).toBe('30')
117+
})
118+
119+
it('omits rate-limit headers on an auth failure, which never reached the bucket', async () => {
120+
const response = createRateLimitResponse({
121+
allowed: false,
122+
remaining: 0,
123+
limit: 0,
124+
resetAt: new Date(),
125+
error: 'API key required',
126+
})
127+
128+
expect(response.status).toBe(401)
129+
expect(response.headers.get('X-RateLimit-Limit')).toBeNull()
130+
expect(response.headers.get('X-RateLimit-Remaining')).toBeNull()
131+
expect(response.headers.get('X-RateLimit-Reset')).toBeNull()
132+
await expect(response.json()).resolves.toEqual({ error: 'API key required' })
133+
})
134+
})

apps/sim/app/api/v1/middleware.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ export interface RateLimitResult {
4141
allowed: boolean
4242
remaining: number
4343
resetAt: Date
44+
/**
45+
* Bucket capacity, matching what `remaining` counts down from. Zero on the
46+
* paths that never reached the bucket (auth failure, checker error); those
47+
* carry `error` and publish no rate-limit headers.
48+
*/
4449
limit: number
4550
retryAfterMs?: number
4651
userId?: string
@@ -65,7 +70,7 @@ export async function checkRateLimit(
6570
return {
6671
allowed: false,
6772
remaining: 0,
68-
limit: 10,
73+
limit: 0,
6974
resetAt: new Date(),
7075
error: auth.error,
7176
}
@@ -96,7 +101,16 @@ export async function checkRateLimit(
96101
allowed: result.allowed,
97102
remaining: result.remaining,
98103
resetAt: result.resetAt,
99-
limit: config.refillRate,
104+
/**
105+
* The bucket's capacity, not its refill rate. `remaining` is the token
106+
* count left in that bucket, and `createBucketConfig` sets
107+
* `maxTokens = refillRate * burstMultiplier` — so reporting `refillRate`
108+
* here published an `X-RateLimit-Limit` smaller than the
109+
* `X-RateLimit-Remaining` beside it (e.g. limit 200, remaining 399), and
110+
* any client computing `used = limit - remaining` got a negative number.
111+
* Both headers must describe the same quantity.
112+
*/
113+
limit: config.maxTokens,
100114
retryAfterMs: result.retryAfterMs,
101115
userId,
102116
workspaceId: auth.workspaceId,
@@ -107,7 +121,7 @@ export async function checkRateLimit(
107121
return {
108122
allowed: false,
109123
remaining: 0,
110-
limit: 10,
124+
limit: 0,
111125
resetAt: new Date(Date.now() + 60000),
112126
error: 'Rate limit check failed',
113127
}
@@ -131,16 +145,21 @@ export async function authenticateRequest(
131145
}
132146

133147
export function createRateLimitResponse(result: RateLimitResult): NextResponse {
148+
/**
149+
* An authentication failure never reaches the token bucket, so there is no
150+
* limit to report. Publishing a placeholder told unauthenticated callers they
151+
* had been throttled and handed monitoring a quota that does not exist.
152+
*/
153+
if (result.error) {
154+
return NextResponse.json({ error: result.error || 'Unauthorized' }, { status: 401 })
155+
}
156+
134157
const headers = {
135158
'X-RateLimit-Limit': result.limit.toString(),
136159
'X-RateLimit-Remaining': result.remaining.toString(),
137160
'X-RateLimit-Reset': result.resetAt.toISOString(),
138161
}
139162

140-
if (result.error) {
141-
return NextResponse.json({ error: result.error || 'Unauthorized' }, { status: 401, headers })
142-
}
143-
144163
const retryAfterSeconds = result.retryAfterMs
145164
? Math.ceil(result.retryAfterMs / 1000)
146165
: Math.ceil((result.resetAt.getTime() - Date.now()) / 1000)

apps/sim/lib/api/contracts/primitives.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@
44
import { describe, expect, it } from 'vitest'
55
import {
66
customPatternSchema,
7+
organizationIdSchema,
78
piiStagePolicySchema,
89
piiStagesSchema,
10+
workflowIdSchema,
11+
workspaceFileIdSchema,
12+
workspaceIdSchema,
913
} from '@/lib/api/contracts/primitives'
1014

1115
describe('customPatternSchema', () => {
@@ -102,3 +106,46 @@ describe('piiStagesSchema', () => {
102106
expect(parsed.blockOutputs.enabled).toBe(true)
103107
})
104108
})
109+
110+
/**
111+
* `.min(1)` only fires for a present-but-empty string, so without the
112+
* `z.string({ error })` form an omitted field falls back to Zod's default
113+
* "expected string, received undefined" — which does not name the field the
114+
* caller left out. These are the shared id schemas every contract builds on, so
115+
* the wording here is the first thing an API consumer sees on a malformed
116+
* request.
117+
*/
118+
describe('shared id schemas name the field when it is missing', () => {
119+
const cases = [
120+
['workspaceIdSchema', workspaceIdSchema, 'Workspace ID is required'],
121+
['organizationIdSchema', organizationIdSchema, 'Organization ID is required'],
122+
['workflowIdSchema', workflowIdSchema, 'Workflow ID is required'],
123+
['workspaceFileIdSchema', workspaceFileIdSchema, 'File ID is required'],
124+
] as const
125+
126+
for (const [name, schema, message] of cases) {
127+
it(`${name}: omitted value reports "${message}"`, () => {
128+
const result = schema.safeParse(undefined)
129+
130+
expect(result.success).toBe(false)
131+
expect(result.error?.issues[0]?.message).toBe(message)
132+
})
133+
134+
it(`${name}: empty string reports "${message}"`, () => {
135+
const result = schema.safeParse('')
136+
137+
expect(result.success).toBe(false)
138+
expect(result.error?.issues[0]?.message).toBe(message)
139+
})
140+
141+
it(`${name}: a valid id still passes`, () => {
142+
expect(schema.safeParse('abc-123').success).toBe(true)
143+
})
144+
}
145+
146+
it('does not leak Zod default wording for a missing field', () => {
147+
const result = workspaceIdSchema.safeParse(undefined)
148+
149+
expect(result.error?.issues[0]?.message).not.toContain('received undefined')
150+
})
151+
})

apps/sim/lib/api/contracts/primitives.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,32 @@ export const nonEmptyIdSchema = z.string().min(1)
3636
* Non-empty `workspaceId` field. Same constraint as `nonEmptyIdSchema` with a
3737
* stable, human-readable message. Use to deduplicate the
3838
* `z.string().min(1, 'Workspace ID is required')` pattern across contracts.
39+
*
40+
* The message is given twice on purpose: `.min(1)` only fires for a present but
41+
* empty string, so without the `z.string({ error })` form an *omitted* field
42+
* falls back to Zod's default `Invalid input: expected string, received
43+
* undefined`, which does not name the field. The same applies to the sibling id
44+
* schemas below.
3945
*/
40-
export const workspaceIdSchema = z.string().min(1, 'Workspace ID is required')
46+
export const workspaceIdSchema = z
47+
.string({ error: 'Workspace ID is required' })
48+
.min(1, 'Workspace ID is required')
4149

4250
/**
4351
* Non-empty `organizationId` field. Same constraint as `nonEmptyIdSchema` with a
4452
* stable, human-readable message.
4553
*/
46-
export const organizationIdSchema = z.string().min(1, 'Organization ID is required')
54+
export const organizationIdSchema = z
55+
.string({ error: 'Organization ID is required' })
56+
.min(1, 'Organization ID is required')
4757

4858
/**
4959
* Non-empty `workflowId` field. Same constraint as `nonEmptyIdSchema` with a
5060
* stable, human-readable message.
5161
*/
52-
export const workflowIdSchema = z.string().min(1, 'Workflow ID is required')
62+
export const workflowIdSchema = z
63+
.string({ error: 'Workflow ID is required' })
64+
.min(1, 'Workflow ID is required')
5365

5466
/**
5567
* A `workspace_files.id` value. The column is a free-form `text` primary key, so
@@ -59,7 +71,7 @@ export const workflowIdSchema = z.string().min(1, 'Workflow ID is required')
5971
* UUID-only schema — a `.uuid()` constraint here silently 400s every `wf_` file.
6072
*/
6173
export const workspaceFileIdSchema = z
62-
.string()
74+
.string({ error: 'File ID is required' })
6375
.min(1, 'File ID is required')
6476
.max(128, 'File ID is too long')
6577
.regex(/^[A-Za-z0-9_-]+$/, 'Invalid file id')

0 commit comments

Comments
 (0)