diff --git a/apps/web/src/__tests__/proxy-rate-limit-classification.test.ts b/apps/web/src/__tests__/proxy-rate-limit-classification.test.ts new file mode 100644 index 000000000..31b49c350 --- /dev/null +++ b/apps/web/src/__tests__/proxy-rate-limit-classification.test.ts @@ -0,0 +1,92 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +/** + * Rate-limit route classification (PR #1507 review — VADE "logic" finding). + * + * The durable video-to-actions workflow polls GET + * /api/workflows/video-to-actions/:runId every ~1.5s (studio-workflow.ts). + * When that poll is classified as an "AI route" it draws from the strict + * 12/min AI budget, so the client poller is 429'd mid-run and the UI hangs on + * "still running". Only the mutating start (POST /api/workflows/...) does real + * AI work and should be AI-limited; the status poll belongs on the general + * budget. + * + * proxy.ts reads limits + env into module-level constants, so each case + * re-imports the module after stubbing env (mirrors proxy-auth-gate.test.ts). + * The applied budget is read off the X-RateLimit-Limit response header. + */ + +vi.mock('next-auth/jwt', () => ({ + getToken: vi.fn(async () => null), +})); + +// NODE_ENV=development => auth gate disabled + in-memory limiter (emits headers). +// All limit/redis knobs are pinned so the result never depends on ambient env. +const ENV: Record = { + NODE_ENV: 'development', + NEXTAUTH_SECRET: undefined, + INTERNAL_REQUEST_TOKEN: undefined, + UPSTASH_REDIS_REST_URL: undefined, + UPSTASH_REDIS_REST_TOKEN: undefined, + UVAI_RATE_LIMIT_DISABLED: undefined, + UVAI_API_RATE_LIMIT_PER_MINUTE: undefined, // default GENERAL_LIMIT = 60 + UVAI_AI_RATE_LIMIT_PER_MINUTE: undefined, // default AI_LIMIT = 12 +}; + +const GENERAL = '60'; +const AI = '12'; + +afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); +}); + +async function loadProxy() { + for (const [key, value] of Object.entries(ENV)) { + vi.stubEnv(key, value); + } + vi.resetModules(); + const { NextRequest } = await import('next/server'); + const { proxy } = await import('@/proxy'); + return { proxy, NextRequest }; +} + +describe('rate-limit classification for /api/workflows (PR #1507)', () => { + it('puts the workflow status poll (GET) on the general budget, not the AI budget', async () => { + const { proxy, NextRequest } = await loadProxy(); + const res = await proxy( + new NextRequest( + 'http://localhost:3000/api/workflows/video-to-actions/run_abc123', + { method: 'GET' }, + ), + ); + // The bug: before the fix this header reads "12" and the poller 429s mid-run. + expect(res.headers.get('X-RateLimit-Limit')).toBe(GENERAL); + }); + + it('keeps the workflow start (POST) on the strict AI budget', async () => { + const { proxy, NextRequest } = await loadProxy(); + const res = await proxy( + new NextRequest('http://localhost:3000/api/workflows/video-to-actions', { + method: 'POST', + }), + ); + expect(res.headers.get('X-RateLimit-Limit')).toBe(AI); + }); + + it('leaves other AI routes on the AI budget regardless of method', async () => { + const { proxy, NextRequest } = await loadProxy(); + const res = await proxy( + new NextRequest('http://localhost:3000/api/chat', { method: 'POST' }), + ); + expect(res.headers.get('X-RateLimit-Limit')).toBe(AI); + }); + + it('keeps ordinary API routes on the general budget', async () => { + const { proxy, NextRequest } = await loadProxy(); + const res = await proxy( + new NextRequest('http://localhost:3000/api/health', { method: 'GET' }), + ); + expect(res.headers.get('X-RateLimit-Limit')).toBe(GENERAL); + }); +}); diff --git a/apps/web/src/proxy.ts b/apps/web/src/proxy.ts index da97c6914..55c5baa60 100644 --- a/apps/web/src/proxy.ts +++ b/apps/web/src/proxy.ts @@ -38,9 +38,16 @@ const AI_ROUTE_PREFIXES = [ '/api/training', '/api/transcribe', '/api/video', - '/api/workflows', ]; +// Durable-workflow routes are a start (POST /api/workflows/...) plus a status +// long-poll (GET /api/workflows/.../:runId). Only the start does real AI work; +// the poll fires every ~1.5s (studio-workflow.ts) and must NOT draw from the +// strict AI budget, or the client poller is 429'd mid-run and the UI hangs on +// "still running" (PR #1507 review). So these are classified by method, not by +// a blanket prefix: mutating start = AI budget, GET status poll = general. +const WORKFLOW_ROUTE_PREFIX = '/api/workflows'; + // Login gating (activate-when-configured) + server-to-server bypass. const INTERNAL_TOKEN = process.env.INTERNAL_REQUEST_TOKEN; const AUTH_SECRET = process.env.NEXTAUTH_SECRET; @@ -121,7 +128,11 @@ function getRedisClient(): Promise { return redisClientPromise; } -function isAiRoute(pathname: string): boolean { +function isAiRoute(pathname: string, method: string): boolean { + if (pathname.startsWith(WORKFLOW_ROUTE_PREFIX)) { + // Only the mutating start is AI-heavy; GET status polls use the general budget. + return method === 'POST'; + } return AI_ROUTE_PREFIXES.some((prefix) => pathname.startsWith(prefix)); } @@ -144,8 +155,8 @@ function getClientIp(request: NextRequest): string { return 'unknown'; } -function getRateLimit(pathname: string): number { - return isAiRoute(pathname) ? AI_LIMIT : GENERAL_LIMIT; +function getRateLimit(pathname: string, method: string): number { + return isAiRoute(pathname, method) ? AI_LIMIT : GENERAL_LIMIT; } async function checkRedisLimit(redisClient: Redis, key: string, limit: number): Promise { @@ -188,9 +199,10 @@ function checkMemoryLimit(key: string, limit: number): RateLimitResult { async function checkRateLimit(request: NextRequest): Promise { const pathname = request.nextUrl.pathname; - const limit = getRateLimit(pathname); + const method = request.method; + const limit = getRateLimit(pathname, method); const clientIp = getClientIp(request); - const routeClass = isAiRoute(pathname) ? 'ai' : 'api'; + const routeClass = isAiRoute(pathname, method) ? 'ai' : 'api'; const key = `${routeClass}:${clientIp}`; const redisClient = await getRedisClient();