diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7d88786..431928a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -183,12 +183,6 @@ importers: web: dependencies: - '@upstash/ratelimit': - specifier: ^2.0.5 - version: 2.0.8(@upstash/redis@1.37.0) - '@upstash/redis': - specifier: ^1.34.3 - version: 1.37.0 '@visitportal/spec': specifier: workspace:^0.1.8 version: link:../packages/spec @@ -974,18 +968,6 @@ packages: '@types/react@19.2.14': resolution: {integrity: sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==} - '@upstash/core-analytics@0.0.10': - resolution: {integrity: sha512-7qJHGxpQgQr9/vmeS1PktEwvNAF7TI4iJDi8Pu2CFZ9YUGHZH4fOP5TfYlZ4aVxfopnELiE4BS4FBjyK7V1/xQ==} - engines: {node: '>=16.0.0'} - - '@upstash/ratelimit@2.0.8': - resolution: {integrity: sha512-YSTMBJ1YIxsoPkUMX/P4DDks/xV5YYCswWMamU8ZIfK9ly6ppjRnVOyBhMDXBmzjODm4UQKcxsJPvaeFAijp5w==} - peerDependencies: - '@upstash/redis': ^1.34.3 - - '@upstash/redis@1.37.0': - resolution: {integrity: sha512-LqOJ3+XWPLSZ2rGSed5DYG3ixybxb8EhZu3yQqF7MdZX1wLBG/FRcI6xcUZXHy/SS7mmXWyadrud0HJHkOc+uw==} - '@vitest/expect@2.1.9': resolution: {integrity: sha512-UJCIkTBenHeKT1TTlKMJWy1laZewsRIzYighyYiJKZreqtdxSos/S1t+ktRMQWu2CKqaarrkeszJx1cgC5tGZw==} @@ -1491,9 +1473,6 @@ packages: ufo@1.6.4: resolution: {integrity: sha512-JFNbkD1Svwe0KvGi8GOeLcP4kAWQ609twvCdcHxq1oSL8svv39ZuSvajcD8B+5D0eL4+s1Is2D/O6KN3qcTeRA==} - uncrypto@0.1.3: - resolution: {integrity: sha512-Ql87qFHB3s/De2ClA9e0gsnS6zXG27SkTiSJwjCc9MebbfapQfuPzumMIUMi38ezPZVNFcHI9sUIepeQfw8J8Q==} - undici-types@6.21.0: resolution: {integrity: sha512-iwDZqg0QAGrg9Rav5H4n0M64c3mkR59cJ6wQp+7C4nI0gsmExaedaYLNO44eT4AtBBwjbTiGPMlt2Md0T9H9JQ==} @@ -2122,19 +2101,6 @@ snapshots: dependencies: csstype: 3.2.3 - '@upstash/core-analytics@0.0.10': - dependencies: - '@upstash/redis': 1.37.0 - - '@upstash/ratelimit@2.0.8(@upstash/redis@1.37.0)': - dependencies: - '@upstash/core-analytics': 0.0.10 - '@upstash/redis': 1.37.0 - - '@upstash/redis@1.37.0': - dependencies: - uncrypto: 0.1.3 - '@vitest/expect@2.1.9': dependencies: '@vitest/spy': 2.1.9 @@ -2666,8 +2632,6 @@ snapshots: ufo@1.6.4: {} - uncrypto@0.1.3: {} - undici-types@6.21.0: {} undici@5.29.0: diff --git a/web/app/api/visit/rate-limit.test.ts b/web/app/api/visit/rate-limit.test.ts deleted file mode 100644 index 4482f3d..0000000 --- a/web/app/api/visit/rate-limit.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; - -// Import lazily inside each test so module-level `warned` + `limiter` state -// resets cleanly for every case. -async function loadModule() { - vi.resetModules(); - return import("./rate-limit"); -} - -describe("rate-limit — no-env fallback", () => { - const origUrl = process.env.UPSTASH_REDIS_REST_URL; - const origToken = process.env.UPSTASH_REDIS_REST_TOKEN; - - // `delete` is required here — assigning `undefined` to a process.env key - // coerces to the string "undefined" (Node.js env vars are always strings), - // which my hardened rate-limit.ts would then pass to new Redis() and - // attempt a real network call. biome's unsafe-fix converted `delete` to - // `= undefined` here and broke the test; restored with biome-ignore. - beforeEach(() => { - // biome-ignore lint/performance/noDelete: env var MUST be absent, not the string "undefined" - delete process.env.UPSTASH_REDIS_REST_URL; - // biome-ignore lint/performance/noDelete: env var MUST be absent, not the string "undefined" - delete process.env.UPSTASH_REDIS_REST_TOKEN; - }); - - afterEach(() => { - if (origUrl !== undefined) process.env.UPSTASH_REDIS_REST_URL = origUrl; - // biome-ignore lint/performance/noDelete: env var MUST be absent, not the string "undefined" - else delete process.env.UPSTASH_REDIS_REST_URL; - if (origToken !== undefined) process.env.UPSTASH_REDIS_REST_TOKEN = origToken; - // biome-ignore lint/performance/noDelete: env var MUST be absent, not the string "undefined" - else delete process.env.UPSTASH_REDIS_REST_TOKEN; - }); - - it("returns ok:true with empty headers when env vars are missing", async () => { - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - const { check } = await loadModule(); - - const r = await check("127.0.0.1"); - expect(r.ok).toBe(true); - expect(r.headers).toEqual({}); - expect(r.retryAfter).toBeUndefined(); - - warn.mockRestore(); - }); - - it("warns exactly once across multiple calls in the same module instance", async () => { - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - const { check } = await loadModule(); - - await check("1.2.3.4"); - await check("5.6.7.8"); - await check("anon"); - expect(warn).toHaveBeenCalledTimes(1); - - warn.mockRestore(); - }); - - it("does not throw when only one of the two env vars is set", async () => { - process.env.UPSTASH_REDIS_REST_URL = "https://example.upstash.io"; - // token deliberately unset - const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); - const { check } = await loadModule(); - - const r = await check("10.0.0.1"); - expect(r.ok).toBe(true); - expect(r.headers).toEqual({}); - - warn.mockRestore(); - }); -}); diff --git a/web/app/api/visit/rate-limit.ts b/web/app/api/visit/rate-limit.ts deleted file mode 100644 index bdcc2c0..0000000 --- a/web/app/api/visit/rate-limit.ts +++ /dev/null @@ -1,98 +0,0 @@ -// Sliding-window IP rate limiter for /api/visit. -// -// Budget: 10 req / minute / IP. Fits inside Upstash's free-tier 10k req/day -// allowance. Prevents the route from being used as an outbound proxy for -// arbitrary scanning. -// -// Module is side-effect-free at import time — the Redis client + Ratelimit -// instance are constructed lazily on the first check() call and only when -// both UPSTASH_REDIS_REST_URL and UPSTASH_REDIS_REST_TOKEN are present in -// the environment. If either is missing (local dev, preview without secrets) -// check() returns { ok: true, headers: {} } after a one-time console.warn -// so the endpoint keeps working. - -import { Ratelimit } from "@upstash/ratelimit"; -import { Redis } from "@upstash/redis"; - -export interface RateLimitResult { - ok: boolean; - headers: Record; - retryAfter?: number; -} - -let limiter: Ratelimit | null = null; -let warned = false; -let constructFailed = false; - -function getLimiter(): Ratelimit | null { - if (limiter) return limiter; - if (constructFailed) return null; // don't retry on every request once we've failed once - const url = process.env.UPSTASH_REDIS_REST_URL?.trim(); - const token = process.env.UPSTASH_REDIS_REST_TOKEN?.trim(); - if (!url || !token) { - if (!warned) { - console.warn( - "[rate-limit] UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN not set — /api/visit rate limiting is DISABLED", - ); - warned = true; - } - return null; - } - try { - limiter = new Ratelimit({ - redis: new Redis({ url, token }), - limiter: Ratelimit.slidingWindow(10, "1 m"), - analytics: false, - prefix: "visitportal:api-visit", - }); - return limiter; - } catch (err) { - // Bad URL, bad token format, or any other client-construction error. - // Fail-open so one misconfigured env var doesn't take the whole endpoint - // offline. One-shot warn; don't retry per request. - constructFailed = true; - console.warn( - "[rate-limit] Failed to construct Upstash client, rate limiting DISABLED:", - err instanceof Error ? err.message : String(err), - ); - return null; - } -} - -export async function check(ip: string): Promise { - const l = getLimiter(); - if (!l) return { ok: true, headers: {} }; - - let success: boolean; - let limit: number; - let remaining: number; - let reset: number; - try { - // Ratelimit.limit returns { success, limit, remaining, reset } where - // `reset` is a unix epoch timestamp in milliseconds. - ({ success, limit, remaining, reset } = await l.limit(ip)); - } catch (err) { - // Fail-open on Redis errors — a rate-limit outage shouldn't take the - // endpoint offline. Log so operators still see the problem. - console.warn( - "[rate-limit] Upstash call failed, allowing request through:", - err instanceof Error ? err.message : String(err), - ); - return { ok: true, headers: {} }; - } - - const resetSec = Math.ceil(reset / 1000); - const retryAfter = Math.max(0, resetSec - Math.floor(Date.now() / 1000)); - - const headers: Record = { - "X-RateLimit-Limit": String(limit), - "X-RateLimit-Remaining": String(remaining), - "X-RateLimit-Reset": String(resetSec), - }; - if (success) return { ok: true, headers }; - return { - ok: false, - headers: { ...headers, "Retry-After": String(retryAfter) }, - retryAfter, - }; -} diff --git a/web/app/api/visit/route.ts b/web/app/api/visit/route.ts index ee4e2c9..232c1c5 100644 --- a/web/app/api/visit/route.ts +++ b/web/app/api/visit/route.ts @@ -4,7 +4,6 @@ // drifted (accepted any http:// host instead of https-with-loopback-only). import { leanValidate } from "@visitportal/spec/lean-validator"; import { NextResponse } from "next/server"; -import { check as rateLimitCheck } from "./rate-limit"; import { guardUrl } from "./ssrf-guard"; // GET /api/visit?url= @@ -62,19 +61,8 @@ export async function GET(req: Request): Promise> { const { searchParams } = new URL(req.url); const target = searchParams.get("url"); - // Rate limit per client IP before any outbound work. When Upstash env vars - // are absent (dev), check() returns ok:true with an empty headers map. - const ip = getClientIp(req); - const rl = await rateLimitCheck(ip); - const respond = (body: VisitResponse, status: number): NextResponse => { - const res = NextResponse.json(body, { status }); - for (const [k, v] of Object.entries(rl.headers)) res.headers.set(k, v); - return res; - }; - - if (!rl.ok) { - return respond({ ok: false, stage: "url", error: "rate limit exceeded" }, 429); - } + const respond = (body: VisitResponse, status: number): NextResponse => + NextResponse.json(body, { status }); if (!target) { return respond({ ok: false, stage: "url", error: "missing 'url' query parameter" }, 400); @@ -132,17 +120,6 @@ export async function GET(req: Request): Promise> { // ---------- helpers ---------- -function getClientIp(req: Request): string { - const xff = req.headers.get("x-forwarded-for"); - if (xff) { - const first = xff.split(",")[0]?.trim(); - if (first) return first; - } - const xri = req.headers.get("x-real-ip")?.trim(); - if (xri) return xri; - return "anon"; -} - interface FetchOk { ok: true; text: string; diff --git a/web/package.json b/web/package.json index b485a65..adb24d5 100644 --- a/web/package.json +++ b/web/package.json @@ -14,8 +14,6 @@ "test": "vitest run" }, "dependencies": { - "@upstash/ratelimit": "^2.0.5", - "@upstash/redis": "^1.34.3", "@visitportal/spec": "workspace:^0.1.8", "ipaddr.js": "^2.2.0", "next": "^15.1.4",