diff --git a/apps/web/src/lib/__tests__/auth-paths.test.ts b/apps/web/src/lib/__tests__/auth-paths.test.ts index c9b961baa..45ef4e4eb 100644 --- a/apps/web/src/lib/__tests__/auth-paths.test.ts +++ b/apps/web/src/lib/__tests__/auth-paths.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import { + isAiRoute, isPublicApiPath, isProtectedPagePath, needsAuthentication, @@ -111,3 +112,87 @@ describe('login gate mode (issue #1058)', () => { } }); }); + +describe('AI route classification (rate-limit budget)', () => { + it('meters model-backed routes against the AI budget', () => { + for (const path of [ + '/api/agents/dispatch', + '/api/chat', + '/api/extract-events', + '/api/pipeline', + '/api/realtime', + '/api/training', + '/api/transcribe', + '/api/video', + ]) { + expect(isAiRoute(path, 'POST')).toBe(true); + } + }); + + it('leaves non-AI API routes on the general budget', () => { + expect(isAiRoute('/api/billing/status', 'GET')).toBe(false); + expect(isAiRoute('/api/health', 'GET')).toBe(false); + expect(isAiRoute('/api/auth/session', 'GET')).toBe(false); + }); + + it('keeps starting a workflow run on the AI budget', () => { + // POST .../video-to-actions fetches a transcript and runs an action agent. + // That is genuine model work and must stay metered. + expect(isAiRoute('/api/workflows/video-to-actions', 'POST')).toBe(true); + }); + + it('exempts workflow status polls from the AI budget', () => { + // GET .../:runId reads stored run state and makes no model call. Metering + // it as AI-class throttles the poller to 12/min while it polls at 30/min. + expect(isAiRoute('/api/workflows/video-to-actions/run_123', 'GET')).toBe(false); + expect(isAiRoute('/api/workflows/video-to-actions/run_123', 'HEAD')).toBe(false); + }); + + it('does not let a status poll drain the shared AI bucket', () => { + // The bucket is keyed by class, not path: every AI prefix shares one + // `ai:` counter. If polls were AI-class, one Studio run would 429 + // /api/chat and /api/transcribe as collateral. This is the regression that + // motivated the method-aware check, so assert the pairing directly. + const poll = '/api/workflows/video-to-actions/run_123'; + expect(isAiRoute(poll, 'GET')).toBe(false); + expect(isAiRoute('/api/chat', 'GET')).toBe(true); + }); + + it('is case-insensitive about the method', () => { + expect(isAiRoute('/api/workflows/video-to-actions/run_1', 'get')).toBe(false); + }); + + it('keeps mutating verbs on workflows AI-class', () => { + // Only reads are exempt — a DELETE/PUT that reaches the workflow runtime + // must not slip onto the looser budget. + for (const method of ['POST', 'PUT', 'PATCH', 'DELETE']) { + expect(isAiRoute('/api/workflows/video-to-actions/run_1', method)).toBe(true); + } + }); + + it('defaults to the stricter budget when no method is supplied', () => { + // Fail safe: an omitted argument must not silently widen the allowance. + expect(isAiRoute('/api/workflows/video-to-actions/run_1')).toBe(true); + expect(isAiRoute('/api/chat')).toBe(true); + }); + + it('does not let the exemption leak to a route that merely shares the prefix', () => { + // The exemption is the widening in this change, so it requires a segment + // boundary. A future sibling surface like /api/workflows-admin is still + // AI-class on every method — otherwise adding a route whose name starts + // with an exempted prefix would silently hand it the looser budget. + expect(isAiRoute('/api/workflows-admin/secret', 'GET')).toBe(true); + expect(isAiRoute('/api/workflows-admin', 'GET')).toBe(true); + // ...while the real surface keeps its exemption, at the prefix itself and + // below it. + expect(isAiRoute('/api/workflows', 'GET')).toBe(false); + expect(isAiRoute('/api/workflows/video-to-actions/run_1', 'GET')).toBe(false); + }); + + it('does not exempt GET on other AI routes', () => { + // The exemption is deliberately per-prefix. A blanket GET carve-out would + // open every model-backed route the moment one served work over GET. + expect(isAiRoute('/api/chat/history', 'GET')).toBe(true); + expect(isAiRoute('/api/transcribe/status', 'GET')).toBe(true); + }); +}); diff --git a/apps/web/src/lib/auth-paths.ts b/apps/web/src/lib/auth-paths.ts index 9d714b950..a9957b3d6 100644 --- a/apps/web/src/lib/auth-paths.ts +++ b/apps/web/src/lib/auth-paths.ts @@ -1,5 +1,5 @@ /** - * Shared auth path policy for middleware/proxy and unit tests. + * Shared auth and rate-limit path policy for middleware/proxy and unit tests. * Keep this free of Next.js request types so vitest can import it offline. */ @@ -78,6 +78,69 @@ export function shouldSkipRateLimit(pathname: string): boolean { return pathname === '/api/auth' || pathname.startsWith('/api/auth/'); } +/** Routes backed by model work, metered against the tighter AI budget. */ +const AI_ROUTE_PREFIXES = [ + '/api/agents/dispatch', + '/api/chat', + '/api/extract-events', + '/api/pipeline', + '/api/realtime', + '/api/training', + '/api/transcribe', + '/api/video', + '/api/workflows', +] as const; + +/** + * Methods that perform no model work on an otherwise AI-class prefix. + * + * `/api/workflows` covers both `POST .../video-to-actions` (starts a run: + * transcript fetch + action agent, genuinely AI-class) and + * `GET .../video-to-actions/:runId` (reads stored run state, no model call). + * Prefix matching alone cannot separate them, so the method has to reach the + * classifier. + * + * This matters more than "one endpoint is metered too tightly", because the + * rate-limit bucket is keyed by *class*, not by path — every AI prefix shares + * one `ai:` counter. A Studio run polls its status ~40x/min against an + * AI budget defaulting to 12/min, so without this exemption a single run + * exhausts the shared allowance in ~17s and 429s /api/chat, /api/transcribe + * and /api/pipeline along with itself. + * + * Deliberately keyed per-prefix rather than exempting GET globally: the other + * prefixes have no polling client, and a blanket GET exemption would be an + * abuse vector the moment any of them serves model work over GET. + */ +const AI_ROUTE_METHOD_EXEMPT: Record> = { + '/api/workflows': new Set(['GET', 'HEAD']), +}; + +/** + * Whether a request should be metered against the AI budget rather than the + * general one. + * + * `method` defaults to POST so an omitted argument fails *safe* (stricter + * limit) rather than silently widening the budget. + */ +export function isAiRoute(pathname: string, method: string = 'POST'): boolean { + const prefix = AI_ROUTE_PREFIXES.find((candidate) => + pathname.startsWith(candidate), + ); + if (!prefix) return false; + + // The exemption requires a segment boundary, while class membership above + // keeps its original loose `startsWith`. The asymmetry is deliberate: + // tightening membership would move routes off the stricter budget, which is + // a widening this change has no business making. But the exemption *is* the + // widening, so it must not leak to a route that merely shares the string + // prefix — `/api/workflows-admin/...` is a different surface than + // `/api/workflows/...` and stays AI-class. + const onExemptRoute = pathname === prefix || pathname.startsWith(`${prefix}/`); + if (!onExemptRoute) return true; + + return !AI_ROUTE_METHOD_EXEMPT[prefix]?.has(method.toUpperCase()); +} + /** * How the login gate should behave for a given environment. * diff --git a/apps/web/src/lib/studio-workflow.ts b/apps/web/src/lib/studio-workflow.ts index d03eb9424..55985c07a 100644 --- a/apps/web/src/lib/studio-workflow.ts +++ b/apps/web/src/lib/studio-workflow.ts @@ -131,14 +131,23 @@ const TERMINAL = new Set(['completed', 'failed', 'cancelled']); /** * Poll until the run is terminal or attempts are exhausted. - * Default: 20 attempts × 1.5s ≈ 30s of wall time (workflow continues server-side). + * Default: 30 attempts × 2s ≈ 60s of wall time (workflow continues server-side). + * + * The cadence is chosen against the middleware rate limit, not just for UI + * responsiveness. Status reads are metered on the general budget + * (`UVAI_API_RATE_LIMIT_PER_MINUTE`, default 60/min) — see `isAiRoute` in + * `@/lib/auth-paths`, which exempts GET on `/api/workflows` from the much + * tighter AI budget. At 2s a run spends 30 req/min, leaving roughly half the + * general allowance for the rest of the page; the previous 1.5s cadence spent + * 40/min and left little margin. The wall-clock window doubles to 60s as a + * side effect, which better fits a transcript fetch plus an agent call. */ export async function pollVideoToActions( runId: string, opts?: { attempts?: number; delayMs?: number; signal?: AbortSignal }, ): Promise { - const attempts = opts?.attempts ?? 20; - const delayMs = opts?.delayMs ?? 1500; + const attempts = opts?.attempts ?? 30; + const delayMs = opts?.delayMs ?? 2000; let last: VideoToActionsPoll = { ok: false, status: 0, diff --git a/apps/web/src/proxy.ts b/apps/web/src/proxy.ts index da97c6914..dba9acaee 100644 --- a/apps/web/src/proxy.ts +++ b/apps/web/src/proxy.ts @@ -2,6 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; import type { Redis } from '@upstash/redis'; import { getToken } from 'next-auth/jwt'; import { + isAiRoute, needsAuthentication, resolveAuthGateMode, safeCallbackPath, @@ -29,18 +30,6 @@ const WINDOW_SECONDS = 60; const GENERAL_LIMIT = Number(process.env.UVAI_API_RATE_LIMIT_PER_MINUTE || 60); const AI_LIMIT = Number(process.env.UVAI_AI_RATE_LIMIT_PER_MINUTE || 12); -const AI_ROUTE_PREFIXES = [ - '/api/agents/dispatch', - '/api/chat', - '/api/extract-events', - '/api/pipeline', - '/api/realtime', - '/api/training', - '/api/transcribe', - '/api/video', - '/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,10 +110,6 @@ function getRedisClient(): Promise { return redisClientPromise; } -function isAiRoute(pathname: string): boolean { - return AI_ROUTE_PREFIXES.some((prefix) => pathname.startsWith(prefix)); -} - function getClientIp(request: NextRequest): string { // Prefer x-real-ip: set by Vercel's edge network and not client-controllable. const realIp = request.headers.get('x-real-ip'); @@ -144,8 +129,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 +173,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();