From bb084486a8638845eeb964c6088d7701edc824db Mon Sep 17 00:00:00 2001 From: Ghvst Date: Tue, 21 Apr 2026 16:05:47 +0100 Subject: [PATCH 1/3] fix(server): add rate limiting, event validation, and payload size limits Protect server endpoints against abuse: - Add sliding-window in-memory rate limiter middleware - Auth endpoints (/auth/github, /auth/callback): 10 req/min per IP - Metrics endpoint (/metrics/events): 5 req/min per user token - Validate event types against an allowlist of known client events - Enforce 10KB per-event and 100KB per-batch payload size limits - Skip events with invalid timestamps SUSTN-Task: 72d4076e-cf12-4e8a-b958-17d850d87fbf --- server/src/lib/rate-limit.ts | 82 ++++++++++++++++++++++++++++++++++++ server/src/routes/auth.ts | 8 +++- server/src/routes/metrics.ts | 54 ++++++++++++++++++++++-- 3 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 server/src/lib/rate-limit.ts diff --git a/server/src/lib/rate-limit.ts b/server/src/lib/rate-limit.ts new file mode 100644 index 0000000..faa0132 --- /dev/null +++ b/server/src/lib/rate-limit.ts @@ -0,0 +1,82 @@ +import type { Context, Next } from "hono"; +import type { Bindings } from "./config.js"; + +interface SlidingWindowEntry { + timestamps: number[]; +} + +const store = new Map(); + +const CLEANUP_INTERVAL = 60_000; +let lastCleanup = Date.now(); + +function cleanup(windowMs: number) { + const now = Date.now(); + if (now - lastCleanup < CLEANUP_INTERVAL) return; + lastCleanup = now; + + const cutoff = now - windowMs; + for (const [key, entry] of store) { + entry.timestamps = entry.timestamps.filter((t) => t > cutoff); + if (entry.timestamps.length === 0) { + store.delete(key); + } + } +} + +function isRateLimited(key: string, maxRequests: number, windowMs: number): boolean { + const now = Date.now(); + cleanup(windowMs); + + const cutoff = now - windowMs; + let entry = store.get(key); + if (!entry) { + entry = { timestamps: [] }; + store.set(key, entry); + } + + entry.timestamps = entry.timestamps.filter((t) => t > cutoff); + + if (entry.timestamps.length >= maxRequests) { + return true; + } + + entry.timestamps.push(now); + return false; +} + +/** + * Rate limit by client IP address. + */ +export function rateLimitByIp(maxRequests: number, windowMs: number) { + return async (c: Context<{ Bindings: Bindings }>, next: Next) => { + const ip = + c.req.header("cf-connecting-ip") ?? + c.req.header("x-forwarded-for")?.split(",")[0]?.trim() ?? + "unknown"; + + const key = `ip:${ip}`; + if (isRateLimited(key, maxRequests, windowMs)) { + return c.json({ error: "Too many requests" }, 429); + } + + await next(); + }; +} + +/** + * Rate limit by Bearer token (user-level). + */ +export function rateLimitByToken(maxRequests: number, windowMs: number) { + return async (c: Context<{ Bindings: Bindings }>, next: Next) => { + const authHeader = c.req.header("Authorization"); + const token = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : "anonymous"; + + const key = `token:${token}`; + if (isRateLimited(key, maxRequests, windowMs)) { + return c.json({ error: "Too many requests" }, 429); + } + + await next(); + }; +} diff --git a/server/src/routes/auth.ts b/server/src/routes/auth.ts index afcc944..c48ac5d 100644 --- a/server/src/routes/auth.ts +++ b/server/src/routes/auth.ts @@ -4,13 +4,17 @@ import type { Bindings } from "../lib/config.js"; import { exchangeCodeForToken, fetchGitHubUser } from "../lib/github.js"; import { createDb } from "../db/index.js"; import { users } from "../db/schema.js"; +import { rateLimitByIp } from "../lib/rate-limit.js"; const auth = new Hono<{ Bindings: Bindings }>(); const GITHUB_AUTHORIZE_URL = "https://github.com/login/oauth/authorize"; const SCOPES = "repo read:user user:email"; -auth.get("/auth/github", (c) => { +// 10 requests per minute per IP for auth endpoints +const authRateLimit = rateLimitByIp(10, 60_000); + +auth.get("/auth/github", authRateLimit, (c) => { const state = c.req.query("state") ?? ""; const params = new URLSearchParams({ @@ -23,7 +27,7 @@ auth.get("/auth/github", (c) => { return c.redirect(`${GITHUB_AUTHORIZE_URL}?${params.toString()}`); }); -auth.get("/auth/callback", async (c) => { +auth.get("/auth/callback", authRateLimit, async (c) => { const code = c.req.query("code"); const state = c.req.query("state") ?? ""; diff --git a/server/src/routes/metrics.ts b/server/src/routes/metrics.ts index d458458..891b596 100644 --- a/server/src/routes/metrics.ts +++ b/server/src/routes/metrics.ts @@ -4,21 +4,44 @@ import type { Bindings } from "../lib/config.js"; import { createDb } from "../db/index.js"; import { users, metricEvents } from "../db/schema.js"; import { fetchGitHubUser } from "../lib/github.js"; +import { rateLimitByToken } from "../lib/rate-limit.js"; const metrics = new Hono<{ Bindings: Bindings }>(); +const ALLOWED_EVENT_TYPES = new Set([ + "session_start", + "session_end", + "project_added", + "settings_changed", + "task_created", + "agent_run_started", + "agent_run_completed", +]); + +const MAX_EVENT_DATA_SIZE = 10_000; // 10KB per event's eventData +const MAX_BATCH_SIZE = 100_000; // 100KB total request body + interface MetricEvent { eventType: string; eventData?: Record; clientTimestamp: string; } -metrics.post("/metrics/events", async (c) => { +// 5 requests per minute per user token +const metricsRateLimit = rateLimitByToken(5, 60_000); + +metrics.post("/metrics/events", metricsRateLimit, async (c) => { const authHeader = c.req.header("Authorization"); if (!authHeader?.startsWith("Bearer ")) { return c.json({ error: "Unauthorized" }, 401); } + // Check Content-Length before parsing + const contentLength = parseInt(c.req.header("Content-Length") ?? "0", 10); + if (contentLength > MAX_BATCH_SIZE) { + return c.json({ error: "Request body too large" }, 413); + } + const db = createDb(c.env.DB); const token = authHeader.slice(7); @@ -48,8 +71,33 @@ metrics.post("/metrics/events", async (c) => { // Cap batch size const events = body.events.slice(0, 100); + // Validate events + const validEvents: MetricEvent[] = []; + for (const e of events) { + if (!ALLOWED_EVENT_TYPES.has(e.eventType)) { + continue; // silently skip unknown event types + } + + if (e.eventData) { + const serialized = JSON.stringify(e.eventData); + if (serialized.length > MAX_EVENT_DATA_SIZE) { + continue; // skip oversized event data + } + } + + if (!e.clientTimestamp || isNaN(Date.parse(e.clientTimestamp))) { + continue; // skip events with invalid timestamps + } + + validEvents.push(e); + } + + if (validEvents.length === 0) { + return c.json({ accepted: 0 }); + } + await db.insert(metricEvents).values( - events.map((e) => ({ + validEvents.map((e) => ({ userId, eventType: e.eventType, eventData: e.eventData ? JSON.stringify(e.eventData) : null, @@ -57,7 +105,7 @@ metrics.post("/metrics/events", async (c) => { })), ); - return c.json({ accepted: events.length }); + return c.json({ accepted: validEvents.length }); }); export { metrics }; From a3027ca91504ec844abc9048541a3737a918e313 Mon Sep 17 00:00:00 2001 From: Ghvst Date: Fri, 24 Apr 2026 06:36:45 +0100 Subject: [PATCH 2/3] fix(server): replace in-memory rate limiter with D1-backed implementation The previous rate limiter used an in-memory Map to track request counts, but Cloudflare Workers do not share isolate-level state across instances. Requests from the same client could hit different isolates, bypassing the rate limit entirely. Replace with a D1-backed fixed-window counter that is shared across all Worker isolates: - Add rate_limits table with composite (key, window_start) primary key - Use atomic INSERT ... ON CONFLICT upsert to increment counters - Clean up expired windows on each check (best-effort, non-blocking) - Existing event validation and payload size limits are unchanged SUSTN-Task: 72d4076e-cf12-4e8a-b958-17d850d87fbf --- server/migrations/0002_rate_limits.sql | 8 +++ server/src/lib/rate-limit.ts | 81 +++++++++++++------------- 2 files changed, 47 insertions(+), 42 deletions(-) create mode 100644 server/migrations/0002_rate_limits.sql diff --git a/server/migrations/0002_rate_limits.sql b/server/migrations/0002_rate_limits.sql new file mode 100644 index 0000000..1758ac0 --- /dev/null +++ b/server/migrations/0002_rate_limits.sql @@ -0,0 +1,8 @@ +-- Rate limiting table for sliding-window counters (shared across Worker isolates) + +CREATE TABLE IF NOT EXISTS rate_limits ( + key TEXT NOT NULL, + window_start INTEGER NOT NULL, + count INTEGER NOT NULL DEFAULT 1, + PRIMARY KEY (key, window_start) +); diff --git a/server/src/lib/rate-limit.ts b/server/src/lib/rate-limit.ts index faa0132..c762ba9 100644 --- a/server/src/lib/rate-limit.ts +++ b/server/src/lib/rate-limit.ts @@ -1,52 +1,46 @@ import type { Context, Next } from "hono"; import type { Bindings } from "./config.js"; -interface SlidingWindowEntry { - timestamps: number[]; -} - -const store = new Map(); - -const CLEANUP_INTERVAL = 60_000; -let lastCleanup = Date.now(); - -function cleanup(windowMs: number) { +function getWindowStart(windowMs: number): number { const now = Date.now(); - if (now - lastCleanup < CLEANUP_INTERVAL) return; - lastCleanup = now; - - const cutoff = now - windowMs; - for (const [key, entry] of store) { - entry.timestamps = entry.timestamps.filter((t) => t > cutoff); - if (entry.timestamps.length === 0) { - store.delete(key); - } - } + return Math.floor(now / windowMs) * windowMs; } -function isRateLimited(key: string, maxRequests: number, windowMs: number): boolean { - const now = Date.now(); - cleanup(windowMs); - - const cutoff = now - windowMs; - let entry = store.get(key); - if (!entry) { - entry = { timestamps: [] }; - store.set(key, entry); - } - - entry.timestamps = entry.timestamps.filter((t) => t > cutoff); - - if (entry.timestamps.length >= maxRequests) { - return true; - } - - entry.timestamps.push(now); - return false; +async function isRateLimited( + db: D1Database, + key: string, + maxRequests: number, + windowMs: number, +): Promise { + const windowStart = getWindowStart(windowMs); + + // Atomically insert or increment the counter for this key+window, + // and return the new count. D1 is shared across isolates so this + // is consistent regardless of which Worker instance handles the request. + const result = await db + .prepare( + `INSERT INTO rate_limits (key, window_start, count) + VALUES (?, ?, 1) + ON CONFLICT (key, window_start) + DO UPDATE SET count = count + 1 + RETURNING count`, + ) + .bind(key, windowStart) + .first<{ count: number }>(); + + // Clean up old windows (non-blocking, best-effort) + const cutoff = windowStart - windowMs; + void db + .prepare(`DELETE FROM rate_limits WHERE window_start < ?`) + .bind(cutoff) + .run(); + + return (result?.count ?? 0) > maxRequests; } /** * Rate limit by client IP address. + * Uses D1 for state so limits are enforced across Worker isolates. */ export function rateLimitByIp(maxRequests: number, windowMs: number) { return async (c: Context<{ Bindings: Bindings }>, next: Next) => { @@ -56,7 +50,7 @@ export function rateLimitByIp(maxRequests: number, windowMs: number) { "unknown"; const key = `ip:${ip}`; - if (isRateLimited(key, maxRequests, windowMs)) { + if (await isRateLimited(c.env.DB, key, maxRequests, windowMs)) { return c.json({ error: "Too many requests" }, 429); } @@ -66,14 +60,17 @@ export function rateLimitByIp(maxRequests: number, windowMs: number) { /** * Rate limit by Bearer token (user-level). + * Uses D1 for state so limits are enforced across Worker isolates. */ export function rateLimitByToken(maxRequests: number, windowMs: number) { return async (c: Context<{ Bindings: Bindings }>, next: Next) => { const authHeader = c.req.header("Authorization"); - const token = authHeader?.startsWith("Bearer ") ? authHeader.slice(7) : "anonymous"; + const token = authHeader?.startsWith("Bearer ") + ? authHeader.slice(7) + : "anonymous"; const key = `token:${token}`; - if (isRateLimited(key, maxRequests, windowMs)) { + if (await isRateLimited(c.env.DB, key, maxRequests, windowMs)) { return c.json({ error: "Too many requests" }, 429); } From be49eef12d74b4423d245e1b80d69d81fc280848 Mon Sep 17 00:00:00 2001 From: Ghvst Date: Fri, 24 Apr 2026 06:38:46 +0100 Subject: [PATCH 3/3] fix(server): hash Bearer tokens before storing in rate_limits table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The D1-backed rate limiter was storing raw Bearer tokens as plaintext keys in the rate_limits table (e.g. `token:`). If the database were compromised, all user tokens would be exposed. Replace plaintext token keys with SHA-256 hashes using the Web Crypto API (available in Cloudflare Workers). The rate limiting behavior is unchanged — only the stored key is now a one-way hash instead of the raw credential. SUSTN-Task: 72d4076e-cf12-4e8a-b958-17d850d87fbf --- server/src/lib/rate-limit.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/server/src/lib/rate-limit.ts b/server/src/lib/rate-limit.ts index c762ba9..ec81845 100644 --- a/server/src/lib/rate-limit.ts +++ b/server/src/lib/rate-limit.ts @@ -1,6 +1,15 @@ import type { Context, Next } from "hono"; import type { Bindings } from "./config.js"; +async function hashToken(token: string): Promise { + const data = new TextEncoder().encode(token); + const hashBuffer = await crypto.subtle.digest("SHA-256", data); + const hashArray = new Uint8Array(hashBuffer); + return Array.from(hashArray, (b) => b.toString(16).padStart(2, "0")).join( + "", + ); +} + function getWindowStart(windowMs: number): number { const now = Date.now(); return Math.floor(now / windowMs) * windowMs; @@ -69,7 +78,8 @@ export function rateLimitByToken(maxRequests: number, windowMs: number) { ? authHeader.slice(7) : "anonymous"; - const key = `token:${token}`; + const tokenHash = await hashToken(token); + const key = `token:${tokenHash}`; if (await isRateLimited(c.env.DB, key, maxRequests, windowMs)) { return c.json({ error: "Too many requests" }, 429); }