From cdfded59567ef1db1cbd76ca7b8ffc1ab151996a Mon Sep 17 00:00:00 2001 From: AsperforMias <109743670+AsperforMias@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:51:23 +0800 Subject: [PATCH] fix(event): remove shared NAT rate limits --- src/app/api/roast/route.test.ts | 10 +-- src/app/api/roast/route.ts | 31 +------- src/app/api/scan/route.test.ts | 6 +- src/app/api/scan/route.ts | 9 --- src/lib/__tests__/redis-rate-limit.test.ts | 8 +-- src/lib/redis.ts | 84 ---------------------- 6 files changed, 4 insertions(+), 144 deletions(-) diff --git a/src/app/api/roast/route.test.ts b/src/app/api/roast/route.test.ts index ce13230..6678caf 100644 --- a/src/app/api/roast/route.test.ts +++ b/src/app/api/roast/route.test.ts @@ -6,9 +6,7 @@ const mocks = vi.hoisted(() => ({ acquireRoastLock: vi.fn(), buildRoastMessages: vi.fn(), checkRoastRateLimit: vi.fn(), - checkRoastNetworkRateLimit: vi.fn(), checkRoastRequestRateLimit: vi.fn(), - checkRoastRequestNetworkRateLimit: vi.fn(), chat: vi.fn(), defaultLlmConfig: vi.fn(), fallbackLlmConfig: vi.fn(), @@ -40,9 +38,7 @@ vi.mock("@/lib/rank", () => ({ getRankCached: mocks.getRankCached })); vi.mock("@/lib/redis", () => ({ acquireRoastLock: mocks.acquireRoastLock, checkRoastRateLimit: mocks.checkRoastRateLimit, - checkRoastNetworkRateLimit: mocks.checkRoastNetworkRateLimit, checkRoastRequestRateLimit: mocks.checkRoastRequestRateLimit, - checkRoastRequestNetworkRateLimit: mocks.checkRoastRequestNetworkRateLimit, clearCachedRoast: vi.fn(), getCachedRoast: mocks.getCachedRoast, getCachedScan: mocks.getCachedScan, @@ -76,8 +72,6 @@ describe("POST /api/roast quick score contract", () => { beforeEach(() => { mocks.checkRoastRequestRateLimit.mockResolvedValue({ success: true }); mocks.checkRoastRateLimit.mockResolvedValue({ success: true }); - mocks.checkRoastRequestNetworkRateLimit.mockResolvedValue({ success: true }); - mocks.checkRoastNetworkRateLimit.mockResolvedValue({ success: true }); mocks.rateLimitHeaders.mockReturnValue({}); mocks.anonymousSessionPrincipal.mockReturnValue(null); mocks.defaultLlmConfig.mockReturnValue({ baseURL: "https://llm.example.test", apiKey: "key", model: "model" }); @@ -123,7 +117,7 @@ describe("POST /api/roast quick score contract", () => { expect(mocks.checkRoastRateLimit).toHaveBeenCalled(); }); - it("uses the signed browser session while retaining the shared network budgets", async () => { + it("uses the signed browser session without a shared-NAT budget", async () => { mocks.anonymousSessionPrincipal.mockReturnValue("anon:session-fixture"); const response = await POST(new NextRequest("https://example.test/api/roast", { method: "POST", @@ -134,9 +128,7 @@ describe("POST /api/roast quick score contract", () => { expect(response.status).toBe(200); await new Response(response.body).text(); expect(mocks.checkRoastRequestRateLimit).toHaveBeenCalledWith("anon:session-fixture"); - expect(mocks.checkRoastRequestNetworkRateLimit).toHaveBeenCalledWith("198.51.100.10"); expect(mocks.checkRoastRateLimit).toHaveBeenCalledWith("anon:session-fixture"); - expect(mocks.checkRoastNetworkRateLimit).toHaveBeenCalledWith("198.51.100.10"); }); it("keeps machine-authenticated callers on their IP budget", async () => { diff --git a/src/app/api/roast/route.ts b/src/app/api/roast/route.ts index d7211a8..5d1fe8d 100644 --- a/src/app/api/roast/route.ts +++ b/src/app/api/roast/route.ts @@ -32,8 +32,6 @@ import { acquireRoastLock, checkRoastRequestRateLimit, checkRoastRateLimit, - checkRoastNetworkRateLimit, - checkRoastRequestNetworkRateLimit, clearCachedRoast, getCachedRoast, getCachedScan, @@ -624,7 +622,7 @@ export async function POST(req: NextRequest) { } const ip = clientIp(req); // CLI/MCP callers retain their IP budget. Only an interactive browser that - // completed Turnstile can exchange its shared-NAT IP key for a signed session. + // completed Turnstile receives a signed session with its own budget. const principal = auth === "absent" ? anonymousSessionPrincipal(req) ?? ip : ip; // This protects every path, including BYO: it runs before the snapshot and @@ -640,20 +638,6 @@ export async function POST(req: NextRequest) { }, ); } - const networkRequestLimit = await checkRoastRequestNetworkRateLimit(ip); - if (!networkRequestLimit.success) { - return NextResponse.json( - { - error: networkRequestLimit.unavailable ? "rate_limit_unavailable" : "rate_limited", - useByoKey: true, - }, - { - status: networkRequestLimit.unavailable ? 503 : 429, - headers: { ...rateLimitHeaders(networkRequestLimit), "Cache-Control": "no-store" }, - }, - ); - } - const lang = normLang(body.lang); // A verified v5/v5/v3 artifact is a read-only continuity path when the quick @@ -805,19 +789,6 @@ export async function POST(req: NextRequest) { }, ); } - const networkGenerationLimit = await checkRoastNetworkRateLimit(ip); - if (!networkGenerationLimit.success) { - return NextResponse.json( - { - error: networkGenerationLimit.unavailable ? "rate_limit_unavailable" : "rate_limited", - useByoKey: true, - }, - { - status: networkGenerationLimit.unavailable ? 503 : 429, - headers: { ...rateLimitHeaders(networkGenerationLimit), "Cache-Control": "no-store" }, - }, - ); - } isLeader = await acquireRoastLock(username, lang); if (isLeader) { if (refreshHonored) await clearCachedRoast(username, lang); diff --git a/src/app/api/scan/route.test.ts b/src/app/api/scan/route.test.ts index 7425ea3..0ba36be 100644 --- a/src/app/api/scan/route.test.ts +++ b/src/app/api/scan/route.test.ts @@ -5,7 +5,6 @@ import type { ScanResult } from "@/lib/types"; const mocks = vi.hoisted(() => ({ buildScanResult: vi.fn(), checkRateLimit: vi.fn(), - checkScanNetworkRateLimit: vi.fn(), coalesceScan: vi.fn(), getCachedScan: vi.fn(), getLegacyReadFallbackScan: vi.fn(), @@ -29,7 +28,6 @@ vi.mock("@/lib/db", () => ({ })); vi.mock("@/lib/redis", () => ({ checkRateLimit: mocks.checkRateLimit, - checkScanNetworkRateLimit: mocks.checkScanNetworkRateLimit, coalesceScan: mocks.coalesceScan, getCachedScan: mocks.getCachedScan, rateLimitHeaders: mocks.rateLimitHeaders, @@ -61,7 +59,6 @@ describe("POST /api/scan immediate quick contract", () => { beforeEach(() => { process.env.GITHUB_ROAST_CLI_API_KEY = "test-key"; mocks.checkRateLimit.mockResolvedValue({ success: true }); - mocks.checkScanNetworkRateLimit.mockResolvedValue({ success: true }); mocks.rateLimitHeaders.mockReturnValue({}); mocks.getCachedScan.mockResolvedValue(null); mocks.coalesceScan.mockImplementation(async (_handle: string, produce: () => unknown) => produce()); @@ -112,7 +109,7 @@ describe("POST /api/scan immediate quick contract", () => { }); }); - it("uses a Turnstile-issued browser session before the shared network budget", async () => { + it("uses a Turnstile-issued browser session without a shared-NAT budget", async () => { delete process.env.GITHUB_ROAST_CLI_API_KEY; mocks.establishAnonymousSession.mockReturnValue({ id: "session-fixture", issued: true }); @@ -124,7 +121,6 @@ describe("POST /api/scan immediate quick contract", () => { expect(response.status).toBe(200); expect(mocks.checkRateLimit).toHaveBeenCalledWith("anon:session-fixture"); - expect(mocks.checkScanNetworkRateLimit).toHaveBeenCalledWith("198.51.100.10"); expect(mocks.attachAnonymousSession).toHaveBeenCalledWith( expect.any(Response), { id: "session-fixture", issued: true }, diff --git a/src/app/api/scan/route.ts b/src/app/api/scan/route.ts index edc236c..393be57 100644 --- a/src/app/api/scan/route.ts +++ b/src/app/api/scan/route.ts @@ -9,7 +9,6 @@ import { } from "@/lib/db"; import { checkRateLimit, - checkScanNetworkRateLimit, coalesceScan, getCachedScan, rateLimitHeaders, @@ -181,14 +180,6 @@ export async function POST(req: NextRequest) { headers: { ...idem, ...rlHeaders, "Cache-Control": "no-store" }, }); } - const networkLimit = await checkScanNetworkRateLimit(ip); - if (!networkLimit.success) { - return apiError(networkLimit.unavailable ? "rate_limit_unavailable" : "rate_limited", { - status: networkLimit.unavailable ? 503 : 429, - headers: { ...idem, ...rateLimitHeaders(networkLimit), "Cache-Control": "no-store" }, - }); - } - const cached = await getCachedScan(username); if (cached) { if (!(await persistQuickScan(cached, Date.now()))) { diff --git a/src/lib/__tests__/redis-rate-limit.test.ts b/src/lib/__tests__/redis-rate-limit.test.ts index fd704cc..01e7967 100644 --- a/src/lib/__tests__/redis-rate-limit.test.ts +++ b/src/lib/__tests__/redis-rate-limit.test.ts @@ -43,22 +43,16 @@ describe("production rate-limit availability", () => { vi.stubEnv("VERCEL_ENV", "production"); unsetRedisEnv(); const error = vi.spyOn(console, "error").mockImplementation(() => {}); - const { checkRateLimit, checkScanNetworkRateLimit, rateLimitHeaders } = await loadRedis(); + const { checkRateLimit, rateLimitHeaders } = await loadRedis(); const result = await checkRateLimit("198.51.100.10"); - const networkResult = await checkScanNetworkRateLimit("198.51.100.10"); expect(result).toMatchObject({ success: false, unavailable: true, retryAfter: 15 }); - expect(networkResult).toMatchObject({ success: false, unavailable: true, retryAfter: 15 }); expect(rateLimitHeaders(result)).toEqual({ "Retry-After": "15" }); expect(error).toHaveBeenCalledWith( "rate_limit_unavailable", expect.objectContaining({ limiter: "scan", reason: "missing_redis_config" }), ); - expect(error).toHaveBeenCalledWith( - "rate_limit_unavailable", - expect.objectContaining({ limiter: "scan_network", reason: "missing_redis_config" }), - ); }); it("fails closed when a configured Redis limiter request errors", async () => { diff --git a/src/lib/redis.ts b/src/lib/redis.ts index e01cae9..434bf61 100644 --- a/src/lib/redis.ts +++ b/src/lib/redis.ts @@ -31,15 +31,11 @@ import type { RoastJudgeResult, RoastLine, ScanResult } from "./types"; let redis: Redis | null = null; let scanLimiter: Ratelimit | null = null; -let scanNetworkLimiter: Ratelimit | null = null; let campaignLeaderboardReadLimiter: Ratelimit | null = null; let mcpLimiter: Ratelimit | null = null; let roastRequestLimiter: Ratelimit | null = null; -let roastRequestNetworkLimiter: Ratelimit | null = null; let roastMinuteLimiter: Ratelimit | null = null; let roastDayLimiter: Ratelimit | null = null; -let roastNetworkMinuteLimiter: Ratelimit | null = null; -let roastNetworkDayLimiter: Ratelimit | null = null; let verdictMinuteLimiter: Ratelimit | null = null; let verdictDayLimiter: Ratelimit | null = null; @@ -271,29 +267,6 @@ export async function checkRateLimit(principal: string): Promise { - const r = getRedis(); - if (!r) return unavailableRateLimitResult("scan_network", "missing_redis_config"); - if (!scanNetworkLimiter) { - scanNetworkLimiter = new Ratelimit({ - redis: r, - limiter: Ratelimit.slidingWindow(60, "60 s"), - prefix: "rl:scan-network", - analytics: false, - }); - } - try { - const { success, limit, remaining, reset } = await scanNetworkLimiter.limit(ip); - return { success, limit, remaining, reset }; - } catch (error) { - return unavailableRateLimitResult( - "scan_network", - error instanceof Error ? error.name : "redis_request_failed", - ); - } -} - /** * Public event leaderboard refreshes fan out from many browsers that may share * one venue NAT. Keep them off the scan budget while still bounding origin reads. @@ -355,29 +328,6 @@ export async function checkRoastRequestRateLimit(principal: string): Promise { - const r = getRedis(); - if (!r) return unavailableRateLimitResult("roast_request_network", "missing_redis_config"); - if (!roastRequestNetworkLimiter) { - roastRequestNetworkLimiter = new Ratelimit({ - redis: r, - limiter: Ratelimit.slidingWindow(120, "60 s"), - prefix: "rl:roast-request-network", - analytics: false, - }); - } - try { - const { success, limit, remaining, reset } = await roastRequestNetworkLimiter.limit(ip); - return { success, limit, remaining, reset }; - } catch (error) { - return unavailableRateLimitResult( - "roast_request_network", - error instanceof Error ? error.name : "redis_request_failed", - ); - } -} - /** * Per-IP limiter for the MCP server. Tighter than the web scan limiter — the * tools are unauthenticated and callable in a loop by an autonomous agent, so we @@ -443,40 +393,6 @@ export async function checkRoastRateLimit(principal: string): Promise { - const r = getRedis(); - if (!r) return unavailableRateLimitResult("roast_generation_network", "missing_redis_config"); - if (!roastNetworkMinuteLimiter) { - roastNetworkMinuteLimiter = new Ratelimit({ - redis: r, - limiter: Ratelimit.slidingWindow(48, "60 s"), - prefix: "rl:roast-network:m", - analytics: false, - }); - } - if (!roastNetworkDayLimiter) { - roastNetworkDayLimiter = new Ratelimit({ - redis: r, - limiter: Ratelimit.slidingWindow(480, "1 d"), - prefix: "rl:roast-network:d", - analytics: false, - }); - } - try { - const [minute, day] = await Promise.all([ - roastNetworkMinuteLimiter.limit(ip), - roastNetworkDayLimiter.limit(ip), - ]); - return { success: minute.success && day.success }; - } catch (error) { - return unavailableRateLimitResult( - "roast_generation_network", - error instanceof Error ? error.name : "redis_request_failed", - ); - } -} - /** Cached roast: the LLM-written report + deterministic score metadata, keyed * by every input contract plus language and username (24h). */ export interface CachedRoast {