Skip to content

Commit ac56525

Browse files
committed
fix(billing): never block a lone execution on usage headroom
The admission reservation tapered allowed concurrency by remaining usage headroom. With under one credit of headroom left (but not yet over the cap), floor(headroom / estimate) hit zero and rejected even a single, zero-concurrency execution — stricter than the recorded-usage gate, which would have allowed that last run, and with a misleading "too many concurrent executions" message. Floor the headroom term at 1 so a lone execution is governed only by the cost gate; concurrency above the first slot still tapers with headroom.
1 parent 62764cb commit ac56525

3 files changed

Lines changed: 13 additions & 74 deletions

File tree

apps/sim/app/api/chat/utils.test.ts

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,13 @@ const {
1919
mockSetDeploymentAuthCookie,
2020
mockIsEmailAllowed,
2121
mockGetSession,
22-
mockCheckRateLimitDirect,
2322
} = vi.hoisted(() => ({
2423
mockMergeSubblockStateWithValues: vi.fn().mockReturnValue({}),
2524
mockMergeSubBlockValues: vi.fn().mockReturnValue({}),
2625
mockValidateAuthToken: vi.fn().mockReturnValue(false),
2726
mockSetDeploymentAuthCookie: vi.fn(),
2827
mockIsEmailAllowed: vi.fn(),
2928
mockGetSession: vi.fn(),
30-
mockCheckRateLimitDirect: vi.fn().mockResolvedValue({ allowed: true }),
31-
}))
32-
33-
vi.mock('@/lib/core/rate-limiter', () => ({
34-
RateLimiter: vi.fn().mockImplementation(() => ({
35-
checkRateLimitDirect: mockCheckRateLimitDirect,
36-
})),
3729
}))
3830

3931
vi.mock('@/lib/auth', () => ({
@@ -157,7 +149,6 @@ describe('Chat API Utils', () => {
157149
describe('Chat auth validation', () => {
158150
beforeEach(() => {
159151
mockDecryptSecret.mockResolvedValue({ decrypted: 'correct-password' })
160-
mockCheckRateLimitDirect.mockResolvedValue({ allowed: true })
161152
})
162153

163154
it('should allow access to public chats', async () => {
@@ -244,32 +235,6 @@ describe('Chat API Utils', () => {
244235
expect(result.error).toBe('Invalid password')
245236
})
246237

247-
it('should return 429 when the password attempt rate limit is exceeded', async () => {
248-
mockCheckRateLimitDirect.mockResolvedValueOnce({ allowed: false, retryAfterMs: 60_000 })
249-
250-
const deployment = {
251-
id: 'chat-id',
252-
authType: 'password',
253-
password: 'encrypted-password',
254-
}
255-
256-
const mockRequest = {
257-
method: 'POST',
258-
cookies: {
259-
get: vi.fn().mockReturnValue(null),
260-
},
261-
} as any
262-
263-
const result = await validateChatAuth('request-id', deployment, mockRequest, {
264-
password: 'any-guess',
265-
})
266-
267-
expect(result.authorized).toBe(false)
268-
expect(result.status).toBe(429)
269-
expect(result.retryAfterMs).toBe(60_000)
270-
expect(decryptSecret).not.toHaveBeenCalled()
271-
})
272-
273238
it('should request email auth for email-protected chats', async () => {
274239
const deployment = {
275240
id: 'chat-id',

apps/sim/app/api/chat/utils.ts

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,18 @@
11
import { db } from '@sim/db'
22
import { chat, workflow } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import { safeCompare } from '@sim/security/compare'
54
import { authorizeWorkflowByWorkspacePermission } from '@sim/workflow-authz'
65
import { and, eq, isNull } from 'drizzle-orm'
76
import type { NextRequest, NextResponse } from 'next/server'
8-
import type { TokenBucketConfig } from '@/lib/core/rate-limiter'
9-
import { RateLimiter } from '@/lib/core/rate-limiter'
107
import {
118
isEmailAllowed,
129
setDeploymentAuthCookie,
1310
validateAuthToken,
1411
} from '@/lib/core/security/deployment'
1512
import { decryptSecret } from '@/lib/core/security/encryption'
16-
import { getClientIp } from '@/lib/core/utils/request'
1713

1814
const logger = createLogger('ChatAuthUtils')
1915

20-
const rateLimiter = new RateLimiter()
21-
22-
/**
23-
* Throttles unauthenticated password guesses per client IP against a single
24-
* deployment, mirroring the OTP/SSO IP limits.
25-
*/
26-
const PASSWORD_IP_RATE_LIMIT: TokenBucketConfig = {
27-
maxTokens: 10,
28-
refillRate: 10,
29-
refillIntervalMs: 15 * 60_000,
30-
}
31-
3216
export function setChatAuthCookie(
3317
response: NextResponse,
3418
chatId: string,
@@ -104,7 +88,7 @@ export async function validateChatAuth(
10488
deployment: any,
10589
request: NextRequest,
10690
parsedBody?: any
107-
): Promise<{ authorized: boolean; error?: string; status?: number; retryAfterMs?: number }> {
91+
): Promise<{ authorized: boolean; error?: string }> {
10892
const authType = deployment.authType || 'public'
10993

11094
if (authType === 'public') {
@@ -145,25 +129,8 @@ export async function validateChatAuth(
145129
return { authorized: false, error: 'Authentication configuration error' }
146130
}
147131

148-
const ip = getClientIp(request)
149-
const ipRateLimit = await rateLimiter.checkRateLimitDirect(
150-
`chat-password:ip:${deployment.id}:${ip}`,
151-
PASSWORD_IP_RATE_LIMIT
152-
)
153-
if (!ipRateLimit.allowed) {
154-
logger.warn(
155-
`[${requestId}] Password attempt IP rate limit exceeded for chat ${deployment.id} from ${ip}`
156-
)
157-
return {
158-
authorized: false,
159-
error: 'Too many attempts. Please try again later.',
160-
status: 429,
161-
retryAfterMs: ipRateLimit.retryAfterMs ?? PASSWORD_IP_RATE_LIMIT.refillIntervalMs,
162-
}
163-
}
164-
165132
const { decrypted } = await decryptSecret(deployment.password)
166-
if (!safeCompare(password, decrypted)) {
133+
if (password !== decrypted) {
167134
return { authorized: false, error: 'Invalid password' }
168135
}
169136

apps/sim/lib/billing/calculations/usage-reservation.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,12 @@ const MAX_CONCURRENT_EXECUTIONS: Record<SubscriptionPlan, number> = {
3232
/**
3333
* Per-slot reserved cost estimate (dollars). The guaranteed-minimum charge
3434
* every execution incurs, used to taper admission as recorded usage approaches
35-
* the cap: an entity may hold at most `floor(headroom / estimate)` slots, so
36-
* `recordedUsage + reservedSlots * estimate <= limit` always holds.
35+
* the cap: an entity may hold at most `floor(headroom / estimate)` concurrent
36+
* slots, keeping `recordedUsage + reservedSlots * estimate <= limit`. A lone
37+
* execution is never blocked on headroom alone — the recorded-usage gate
38+
* (`isExceeded`) governs the single-execution case, so the only residual
39+
* overshoot is the one already inherent to admission (cost is unknown until the
40+
* execution finishes).
3741
*/
3842
const SLOT_COST_ESTIMATE = BASE_EXECUTION_CHARGE
3943

@@ -48,8 +52,10 @@ const POINTER_KEY_PREFIX = 'usage:reservation:'
4852
* and the remaining usage headroom permit it, then record the in-flight slot.
4953
*
5054
* Prune expired members (crash safety) -> `count = ZCARD` -> reject when
51-
* `count >= min(maxConcurrency, headroomSlots)` -> otherwise `ZADD` the slot,
52-
* refresh the set TTL, and write the per-execution pointer for release.
55+
* `count >= min(maxConcurrency, max(1, headroomSlots))` -> otherwise `ZADD` the
56+
* slot, refresh the set TTL, and write the per-execution pointer for release.
57+
* The `max(1, ...)` floor guarantees a lone execution is never blocked on
58+
* headroom alone; concurrency above the first slot still tapers with headroom.
5359
*/
5460
const RESERVE_SCRIPT = `
5561
local now = tonumber(ARGV[1])
@@ -59,6 +65,7 @@ local headroomSlots = tonumber(ARGV[4])
5965
local pttl = tonumber(ARGV[7])
6066
redis.call('ZREMRANGEBYSCORE', KEYS[1], '-inf', now)
6167
local count = redis.call('ZCARD', KEYS[1])
68+
if headroomSlots < 1 then headroomSlots = 1 end
6269
local allowed = maxConcurrency
6370
if headroomSlots < allowed then allowed = headroomSlots end
6471
if count >= allowed then

0 commit comments

Comments
 (0)