From c04e429fe0406ba0161dca1f9935257c9f900784 Mon Sep 17 00:00:00 2001 From: weroperking <139503221+weroperking@users.noreply.github.com> Date: Sun, 26 Apr 2026 18:58:16 +0300 Subject: [PATCH 1/2] Harden auth, token security, and request handling --- docker/nginx/nginx.conf | 14 +++---- .../migrations/017_revoked_admin_tokens.sql | 9 +++++ packages/server/src/index.ts | 2 +- packages/server/src/lib/audit.ts | 4 +- packages/server/src/lib/auth.ts | 29 ++++++++++++-- packages/server/src/lib/env.ts | 11 ++++++ packages/server/src/routes/admin/auth.ts | 34 ++++++++++++++++- .../src/routes/admin/project-scoped/users.ts | 12 +++++- .../server/src/routes/betterbase/index.ts | 38 ++++++++++++++++--- packages/server/src/routes/betterbase/ws.ts | 7 +++- packages/server/src/routes/device/index.ts | 14 +++++++ 11 files changed, 148 insertions(+), 26 deletions(-) create mode 100644 packages/server/migrations/017_revoked_admin_tokens.sql diff --git a/docker/nginx/nginx.conf b/docker/nginx/nginx.conf index c91d012..b501a09 100644 --- a/docker/nginx/nginx.conf +++ b/docker/nginx/nginx.conf @@ -53,7 +53,7 @@ http { proxy_pass http://betterbase_server; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; proxy_read_timeout 60s; } @@ -62,13 +62,13 @@ http { proxy_pass http://betterbase_server; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; } location /health { proxy_pass http://betterbase_server; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; } @@ -78,7 +78,7 @@ http { proxy_pass http://minio; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; client_max_body_size 100m; } @@ -88,7 +88,7 @@ http { proxy_pass http://betterbase_dashboard; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; # SPA fallback proxy_intercept_errors on; @@ -98,7 +98,7 @@ http { location @dashboard_fallback { proxy_pass http://betterbase_dashboard; proxy_set_header Host $host; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; } @@ -110,7 +110,7 @@ http { proxy_set_header Connection "upgrade"; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; proxy_read_timeout 3600s; } diff --git a/packages/server/migrations/017_revoked_admin_tokens.sql b/packages/server/migrations/017_revoked_admin_tokens.sql new file mode 100644 index 0000000..682c2ef --- /dev/null +++ b/packages/server/migrations/017_revoked_admin_tokens.sql @@ -0,0 +1,9 @@ +CREATE TABLE IF NOT EXISTS betterbase_meta.revoked_admin_tokens ( + jti TEXT PRIMARY KEY, + admin_user_id TEXT REFERENCES betterbase_meta.admin_users(id) ON DELETE SET NULL, + revoked_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + expires_at TIMESTAMPTZ +); + +CREATE INDEX IF NOT EXISTS idx_revoked_admin_tokens_expires_at + ON betterbase_meta.revoked_admin_tokens (expires_at); diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 6fc5829..9277be1 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -36,7 +36,7 @@ app.use("*", async (c, next) => { const projectId = c.req.header("X-Project-ID") ?? null; const userAgent = c.req.header("User-Agent")?.slice(0, 255) ?? null; - const ip = c.req.header("X-Forwarded-For")?.split(",")[0] ?? null; + const ip = c.req.header("X-Real-IP") ?? null; // Fire-and-forget log insert (don't await, don't fail requests on log error) getPool() diff --git a/packages/server/src/lib/audit.ts b/packages/server/src/lib/audit.ts index 101997c..695844b 100644 --- a/packages/server/src/lib/audit.ts +++ b/packages/server/src/lib/audit.ts @@ -71,7 +71,5 @@ export async function writeAuditLog(entry: AuditEntry): Promise { // Helper: extract IP from Hono context export function getClientIp(headers: Headers): string { - return ( - headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? headers.get("x-real-ip") ?? "unknown" - ); + return headers.get("x-real-ip") ?? headers.get("x-forwarded-for")?.split(",").pop()?.trim() ?? "unknown"; } diff --git a/packages/server/src/lib/auth.ts b/packages/server/src/lib/auth.ts index aeec770..ca0b5e3 100644 --- a/packages/server/src/lib/auth.ts +++ b/packages/server/src/lib/auth.ts @@ -1,6 +1,8 @@ import bcrypt from "bcryptjs"; +import { randomUUID } from "crypto"; import { SignJWT, jwtVerify } from "jose"; import type { Pool } from "pg"; +import { getPool } from "./db"; import { validateEnv } from "./env"; const getSecret = () => { @@ -8,7 +10,7 @@ const getSecret = () => { return new TextEncoder().encode(env.BETTERBASE_JWT_SECRET); }; -const TOKEN_EXPIRY = "30d"; +const TOKEN_EXPIRY = "8h"; const BCRYPT_ROUNDS = 12; // --- Password --- @@ -24,18 +26,37 @@ export async function verifyPassword(password: string, hash: string): Promise { + const env = validateEnv(); return new SignJWT({ sub: adminUserId, type: "admin" }) .setProtectedHeader({ alg: "HS256" }) .setIssuedAt() .setExpirationTime(TOKEN_EXPIRY) + .setIssuer(env.BETTERBASE_JWT_ISSUER) + .setAudience(env.BETTERBASE_JWT_AUDIENCE) + .setJti(randomUUID()) .sign(getSecret()); } -export async function verifyAdminToken(token: string): Promise<{ sub: string } | null> { +export async function verifyAdminToken( + token: string, +): Promise<{ sub: string; jti: string } | null> { try { - const { payload } = await jwtVerify(token, getSecret()); + const env = validateEnv(); + const { payload } = await jwtVerify(token, getSecret(), { + issuer: env.BETTERBASE_JWT_ISSUER, + audience: env.BETTERBASE_JWT_AUDIENCE, + }); if (payload.type !== "admin") return null; - return { sub: payload.sub as string }; + if (!payload.sub || !payload.jti) return null; + + const pool = getPool(); + const { rows } = await pool.query( + "SELECT 1 FROM betterbase_meta.revoked_admin_tokens WHERE jti = $1 LIMIT 1", + [payload.jti], + ); + if (rows.length > 0) return null; + + return { sub: payload.sub as string, jti: payload.jti as string }; } catch { return null; } diff --git a/packages/server/src/lib/env.ts b/packages/server/src/lib/env.ts index eb6afa9..276892c 100644 --- a/packages/server/src/lib/env.ts +++ b/packages/server/src/lib/env.ts @@ -17,6 +17,8 @@ const EnvSchema = z.object({ INNGEST_SIGNING_KEY: z.string().optional(), INNGEST_EVENT_KEY: z.string().optional(), PORT: z.string().default("3000"), + BETTERBASE_JWT_ISSUER: z.string().default("betterbase"), + BETTERBASE_JWT_AUDIENCE: z.string().default("betterbase-admin"), }); export type Env = z.infer; @@ -53,6 +55,15 @@ export function validateEnv(): Env { result.data.INNGEST_EVENT_KEY = "betterbase-dev-event-key"; } + if (result.data.STORAGE_ENDPOINT) { + if (!result.data.STORAGE_ACCESS_KEY || !result.data.STORAGE_SECRET_KEY) { + console.error( + "[env] STORAGE_ACCESS_KEY and STORAGE_SECRET_KEY are required when STORAGE_ENDPOINT is set", + ); + process.exit(1); + } + } + validatedEnv = result.data; return validatedEnv; } diff --git a/packages/server/src/routes/admin/auth.ts b/packages/server/src/routes/admin/auth.ts index 3de2d75..5c1e7c3 100644 --- a/packages/server/src/routes/admin/auth.ts +++ b/packages/server/src/routes/admin/auth.ts @@ -1,6 +1,7 @@ import { zValidator } from "@hono/zod-validator"; import { Hono } from "hono"; import { z } from "zod"; +import { getClientIp, writeAuditLog } from "../../lib/audit"; import { extractBearerToken, signAdminToken, @@ -62,8 +63,37 @@ authRoutes.get("/me", async (c) => { return c.json({ admin: rows[0] }); }); -// POST /admin/auth/logout (client-side token discard — stateless) -authRoutes.post("/logout", (c) => c.json({ success: true })); +// POST /admin/auth/logout +authRoutes.post("/logout", async (c) => { + const token = extractBearerToken(c.req.header("Authorization")); + if (!token) return c.json({ success: true }); + + const payload = await verifyAdminToken(token); + if (!payload) return c.json({ success: true }); + + const pool = getPool(); + await pool.query( + `INSERT INTO betterbase_meta.revoked_admin_tokens (jti, admin_user_id) + VALUES ($1, $2) + ON CONFLICT (jti) DO NOTHING`, + [payload.jti, payload.sub], + ); + + const { rows } = await pool.query("SELECT id, email FROM betterbase_meta.admin_users WHERE id = $1", [ + payload.sub, + ]); + if (rows.length > 0) { + await writeAuditLog({ + actorId: rows[0].id, + actorEmail: rows[0].email, + action: "admin.logout", + ipAddress: getClientIp(c.req.raw.headers), + userAgent: c.req.header("User-Agent") ?? undefined, + }); + } + + return c.json({ success: true }); +}); // GET /admin/auth/setup-status — check if admin exists (no body validation) authRoutes.get("/setup-status", async (c) => { diff --git a/packages/server/src/routes/admin/project-scoped/users.ts b/packages/server/src/routes/admin/project-scoped/users.ts index 9acfc00..862751e 100644 --- a/packages/server/src/routes/admin/project-scoped/users.ts +++ b/packages/server/src/routes/admin/project-scoped/users.ts @@ -5,6 +5,7 @@ import { getClientIp, writeAuditLog } from "../../../lib/audit"; import { getPool } from "../../../lib/db"; export const projectUserRoutes = new Hono(); +const CSV_DANGEROUS_PREFIX = /^[=+\-@\t\r]/; function schemaName(project: { slug: string }) { return `project_${project.slug}`; @@ -229,18 +230,25 @@ projectUserRoutes.post("/export", async (c) => { ); const header = "id,name,email,email_verified,created_at,banned\n"; + const escapeCsv = (value: unknown) => { + const raw = String(value ?? ""); + const prefixed = CSV_DANGEROUS_PREFIX.test(raw) ? `'${raw}` : raw; + return `"${prefixed.replace(/"/g, '""')}"`; + }; const csv = header + rows .map( - (r) => `${r.id},"${r.name}","${r.email}",${r.email_verified},${r.created_at},${r.banned}`, + (r) => + `${escapeCsv(r.id)},${escapeCsv(r.name)},${escapeCsv(r.email)},${r.email_verified},${escapeCsv(r.created_at)},${r.banned}`, ) .join("\n"); return new Response(csv, { headers: { - "Content-Type": "text/csv", + "Content-Type": "text/csv; charset=utf-8", "Content-Disposition": `attachment; filename="users-${project.slug}-${Date.now()}.csv"`, + "Content-Security-Policy": "default-src 'none'", }, }); }); diff --git a/packages/server/src/routes/betterbase/index.ts b/packages/server/src/routes/betterbase/index.ts index cd16806..c2396fd 100644 --- a/packages/server/src/routes/betterbase/index.ts +++ b/packages/server/src/routes/betterbase/index.ts @@ -20,6 +20,15 @@ import { PutObjectCommand, S3Client } from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; export const betterbaseRouter = new Hono(); +const SAFE_PROJECT_SLUG = /^[a-z][a-z0-9_]{0,62}$/; +const ALLOWED_UPLOAD_CONTENT_TYPES = new Set([ + "image/jpeg", + "image/png", + "image/webp", + "image/gif", + "application/pdf", + "text/plain", +]); // All function calls: POST /betterbase/:kind/* betterbaseRouter.post("/:kind/*", async (c) => { @@ -48,11 +57,15 @@ betterbaseRouter.post("/:kind/*", async (c) => { // Auth context const token = extractBearerToken(c.req.header("Authorization")); const adminPayload = token ? await verifyAdminToken(token) : null; + if (!adminPayload) return c.json({ error: "Unauthorized" }, 401); const authCtx = { userId: adminPayload?.sub ?? null, token }; // Build DB context const pool = getPool(); const projectSlug = c.req.header("X-Project-Slug") ?? "default"; + if (!SAFE_PROJECT_SLUG.test(projectSlug)) { + return c.json({ error: "Invalid project slug" }, 400); + } const dbSchema = `project_${projectSlug}`; try { @@ -97,8 +110,8 @@ function buildStorageCtx(pool: any, projectSlug: string): StorageCtx { pool, projectSlug, endpoint: env.STORAGE_ENDPOINT ?? "http://minio:9000", - accessKey: env.STORAGE_ACCESS_KEY ?? "minioadmin", - secretKey: env.STORAGE_SECRET_KEY ?? "minioadmin", + accessKey: env.STORAGE_ACCESS_KEY ?? "", + secretKey: env.STORAGE_SECRET_KEY ?? "", bucket: env.STORAGE_BUCKET ?? "betterbase", publicBase: env.STORAGE_PUBLIC_BASE, }); @@ -177,8 +190,21 @@ function buildActionCtx(pool: any, dbSchema: string, auth: any, projectSlug: str // Direct browser upload endpoint: POST /betterbase/storage/generate-upload-url betterbaseRouter.post("/storage/generate-upload-url", async (c) => { + const token = extractBearerToken(c.req.header("Authorization")); + const adminPayload = token ? await verifyAdminToken(token) : null; + if (!adminPayload) return c.json({ error: "Unauthorized" }, 401); + const { contentType, filename } = await c.req.json(); + const safeContentType = + typeof contentType === "string" && ALLOWED_UPLOAD_CONTENT_TYPES.has(contentType) + ? contentType + : null; + if (!safeContentType) return c.json({ error: "Unsupported content type" }, 400); + const projectSlug = c.req.header("X-Project-Slug") ?? "default"; + if (!SAFE_PROJECT_SLUG.test(projectSlug)) { + return c.json({ error: "Invalid project slug" }, 400); + } const storageId = `st_${nanoid(20)}`; const ext = filename?.split(".").pop() ?? ""; const s3Key = `project_${projectSlug}/${storageId}${ext ? "." + ext : ""}`; @@ -188,8 +214,8 @@ betterbaseRouter.post("/storage/generate-upload-url", async (c) => { endpoint: env.STORAGE_ENDPOINT ?? "http://minio:9000", region: "us-east-1", credentials: { - accessKeyId: env.STORAGE_ACCESS_KEY ?? "minioadmin", - secretAccessKey: env.STORAGE_SECRET_KEY ?? "minioadmin", + accessKeyId: env.STORAGE_ACCESS_KEY ?? "", + secretAccessKey: env.STORAGE_SECRET_KEY ?? "", }, forcePathStyle: true, }); @@ -199,7 +225,7 @@ betterbaseRouter.post("/storage/generate-upload-url", async (c) => { new PutObjectCommand({ Bucket: env.STORAGE_BUCKET ?? "betterbase", Key: s3Key, - ContentType: contentType ?? "application/octet-stream", + ContentType: safeContentType, }), { expiresIn: 300 }, ); @@ -214,7 +240,7 @@ betterbaseRouter.post("/storage/generate-upload-url", async (c) => { storageId, s3Key, env.STORAGE_BUCKET ?? "betterbase", - contentType ?? "application/octet-stream", + safeContentType, ], ); diff --git a/packages/server/src/routes/betterbase/ws.ts b/packages/server/src/routes/betterbase/ws.ts index 7b93906..aa389b6 100644 --- a/packages/server/src/routes/betterbase/ws.ts +++ b/packages/server/src/routes/betterbase/ws.ts @@ -5,6 +5,7 @@ import { subscriptionTracker, } from "@betterbase/core"; import { nanoid } from "nanoid"; +import { verifyAdminToken } from "../../lib/auth"; const HEARTBEAT_INTERVAL_MS = 15_000; // ping every 15s const HEARTBEAT_TIMEOUT_MS = 30_000; // disconnect after 30s without pong @@ -140,8 +141,12 @@ export function getBunServeConfig() { fetch(req: Request, server: any) { const url = new URL(req.url); if (url.pathname === "/betterbase/ws") { + const token = url.searchParams.get("token"); + const payload = token ? await verifyAdminToken(token) : null; + if (!payload) return new Response("Unauthorized", { status: 401 }); + const projectSlug = url.searchParams.get("project") ?? "default"; - const upgraded = server.upgrade(req, { data: { projectSlug } }); + const upgraded = server.upgrade(req, { data: { projectSlug, userId: payload.sub } }); if (upgraded) return undefined; return new Response("WebSocket upgrade failed", { status: 400 }); } diff --git a/packages/server/src/routes/device/index.ts b/packages/server/src/routes/device/index.ts index bbff6ee..912df07 100644 --- a/packages/server/src/routes/device/index.ts +++ b/packages/server/src/routes/device/index.ts @@ -9,9 +9,23 @@ import { validateEnv } from "../../lib/env"; export const deviceRouter = new Hono(); const CODE_EXPIRY_MINUTES = 10; +const DEVICE_CODE_RATE_LIMIT_WINDOW_MS = 60_000; +const DEVICE_CODE_RATE_LIMIT_MAX = 5; +const deviceCodeRateLimits = new Map(); // POST /device/code — CLI calls this to initiate login deviceRouter.post("/code", async (c) => { + const ip = c.req.header("X-Real-IP") ?? "unknown"; + const now = Date.now(); + const recent = (deviceCodeRateLimits.get(ip) ?? []).filter( + (ts) => now - ts < DEVICE_CODE_RATE_LIMIT_WINDOW_MS, + ); + if (recent.length >= DEVICE_CODE_RATE_LIMIT_MAX) { + return c.json({ error: "Rate limit exceeded. Try again in a minute." }, 429); + } + recent.push(now); + deviceCodeRateLimits.set(ip, recent); + const pool = getPool(); const deviceCode = nanoid(32); From 4e841c3805a80f3676876926c70642fdf0bee0ad Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sun, 26 Apr 2026 16:48:51 +0000 Subject: [PATCH 2/2] fix: apply CodeRabbit auto-fixes Fixed 9 file(s) based on 15 unresolved review comments. Co-authored-by: CodeRabbit --- bun.lock | 1 + docker/nginx/nginx.conf | 1 + packages/server/src/lib/auth.ts | 49 ++++-- packages/server/src/lib/inngest.ts | 41 +++-- packages/server/src/routes/admin/auth.ts | 15 +- .../src/routes/admin/project-scoped/users.ts | 9 +- .../server/src/routes/betterbase/index.ts | 161 ++++++++++++------ packages/server/src/routes/betterbase/ws.ts | 16 +- packages/server/src/routes/device/index.ts | 33 +++- 9 files changed, 224 insertions(+), 102 deletions(-) diff --git a/bun.lock b/bun.lock index ff88c63..d0a30e5 100644 --- a/bun.lock +++ b/bun.lock @@ -1,5 +1,6 @@ { "lockfileVersion": 1, + "configVersion": 0, "workspaces": { "": { "name": "betterbase", diff --git a/docker/nginx/nginx.conf b/docker/nginx/nginx.conf index b501a09..ce8a9ea 100644 --- a/docker/nginx/nginx.conf +++ b/docker/nginx/nginx.conf @@ -68,6 +68,7 @@ http { location /health { proxy_pass http://betterbase_server; + proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $remote_addr; proxy_set_header X-Forwarded-Proto $scheme; } diff --git a/packages/server/src/lib/auth.ts b/packages/server/src/lib/auth.ts index ca0b5e3..74e0806 100644 --- a/packages/server/src/lib/auth.ts +++ b/packages/server/src/lib/auth.ts @@ -39,27 +39,54 @@ export async function signAdminToken(adminUserId: string): Promise { export async function verifyAdminToken( token: string, -): Promise<{ sub: string; jti: string } | null> { +): Promise<{ sub: string; jti: string | undefined; exp?: number } | null> { + // Step 1: Verify JWT signature and payload validity + let payload: any; try { const env = validateEnv(); - const { payload } = await jwtVerify(token, getSecret(), { + const verified = await jwtVerify(token, getSecret(), { issuer: env.BETTERBASE_JWT_ISSUER, audience: env.BETTERBASE_JWT_AUDIENCE, }); - if (payload.type !== "admin") return null; - if (!payload.sub || !payload.jti) return null; + payload = verified.payload; - const pool = getPool(); - const { rows } = await pool.query( - "SELECT 1 FROM betterbase_meta.revoked_admin_tokens WHERE jti = $1 LIMIT 1", - [payload.jti], - ); - if (rows.length > 0) return null; + if (payload.type !== "admin") return null; + if (!payload.sub) return null; - return { sub: payload.sub as string, jti: payload.jti as string }; + // Log warning if jti is missing + if (!payload.jti) { + console.warn(`[auth] Token missing jti claim (sub=${payload.sub})`); + } } catch { + // Cryptographic verification failed - invalid token return null; } + + // Step 2: Check revocation list (fail-closed on DB errors) + // Skip revocation check if jti is missing + if (payload.jti) { + try { + const pool = getPool(); + const { rows } = await pool.query( + "SELECT 1 FROM betterbase_meta.revoked_admin_tokens WHERE jti = $1 LIMIT 1", + [payload.jti], + ); + if (rows.length > 0) return null; + } catch (err) { + // DB/query error - log and fail closed + console.error( + `[auth] DB error checking token revocation (jti=${payload.jti}, sub=${payload.sub}):`, + err, + ); + return null; + } + } + + return { + sub: payload.sub as string, + jti: payload.jti as string | undefined, + exp: payload.exp as number | undefined + }; } // --- Middleware helper: extract + verify token from Authorization header --- diff --git a/packages/server/src/lib/inngest.ts b/packages/server/src/lib/inngest.ts index b1a54fa..1c3f02b 100644 --- a/packages/server/src/lib/inngest.ts +++ b/packages/server/src/lib/inngest.ts @@ -1,21 +1,6 @@ import { createHash } from "node:crypto"; import { EventSchemas, Inngest } from "inngest"; - -// ─── CSV Escaping Helper ─────────────────────────────────────────────────────── -// Helper to escape CSV values - prevents CSV injection and handles special characters -const escapeCSVValue = (value: unknown): string => { - if (value === null || value === undefined) return ""; - const str = String(value); - // Prefix formula injection characters (=, +, -, @, \t, \r, \n) with single quote - if (str.match(/^[=\+\-@\t\r\n]/)) { - return `"${str.replace(/"/g, '""')}"`; - } - // Wrap in quotes if contains comma, quote, or newline - if (str.includes(",") || str.includes('"') || str.includes("\n") || str.includes("\r")) { - return `"${str.replace(/"/g, '""')}"`; - } - return str; -}; +import { escapeCSVValue } from "./csv"; // Helper to validate schema name - prevents SQL injection const validateSchemaName = (slug: string): string => { @@ -556,6 +541,29 @@ export const pollNotificationRules = inngest.createFunction( }, ); +// ─── Function: Cleanup Expired Revoked Tokens (Cron) ───────────────────────── + +export const cleanupExpiredTokens = inngest.createFunction( + { + id: "cleanup-expired-revoked-tokens", + retries: 1, + }, + // Runs every hour + { cron: "0 * * * *" }, + async ({ step }) => { + const deleted = await step.run("delete-expired-tokens", async () => { + const { getPool } = await import("./db"); + const pool = getPool(); + const { rowCount } = await pool.query( + "DELETE FROM betterbase_meta.revoked_admin_tokens WHERE expires_at IS NOT NULL AND expires_at < NOW()", + ); + return rowCount ?? 0; + }); + + return { deleted }; + }, +); + // ─── All functions (used in serve() registration) ──────────────────────────── export const allInngestFunctions = [ @@ -563,4 +571,5 @@ export const allInngestFunctions = [ evaluateNotificationRule, exportProjectUsers, pollNotificationRules, + cleanupExpiredTokens, ]; diff --git a/packages/server/src/routes/admin/auth.ts b/packages/server/src/routes/admin/auth.ts index 5c1e7c3..62dc5ef 100644 --- a/packages/server/src/routes/admin/auth.ts +++ b/packages/server/src/routes/admin/auth.ts @@ -72,12 +72,15 @@ authRoutes.post("/logout", async (c) => { if (!payload) return c.json({ success: true }); const pool = getPool(); - await pool.query( - `INSERT INTO betterbase_meta.revoked_admin_tokens (jti, admin_user_id) - VALUES ($1, $2) - ON CONFLICT (jti) DO NOTHING`, - [payload.jti, payload.sub], - ); + // Only revoke if jti is present + if (payload.jti && payload.exp) { + await pool.query( + `INSERT INTO betterbase_meta.revoked_admin_tokens (jti, admin_user_id, expires_at) + VALUES ($1, $2, to_timestamp($3)) + ON CONFLICT (jti) DO NOTHING`, + [payload.jti, payload.sub, payload.exp], + ); + } const { rows } = await pool.query("SELECT id, email FROM betterbase_meta.admin_users WHERE id = $1", [ payload.sub, diff --git a/packages/server/src/routes/admin/project-scoped/users.ts b/packages/server/src/routes/admin/project-scoped/users.ts index 862751e..1b719d5 100644 --- a/packages/server/src/routes/admin/project-scoped/users.ts +++ b/packages/server/src/routes/admin/project-scoped/users.ts @@ -2,10 +2,10 @@ import { zValidator } from "@hono/zod-validator"; import { Hono } from "hono"; import { z } from "zod"; import { getClientIp, writeAuditLog } from "../../../lib/audit"; +import { escapeCSVValue } from "../../../lib/csv"; import { getPool } from "../../../lib/db"; export const projectUserRoutes = new Hono(); -const CSV_DANGEROUS_PREFIX = /^[=+\-@\t\r]/; function schemaName(project: { slug: string }) { return `project_${project.slug}`; @@ -230,17 +230,12 @@ projectUserRoutes.post("/export", async (c) => { ); const header = "id,name,email,email_verified,created_at,banned\n"; - const escapeCsv = (value: unknown) => { - const raw = String(value ?? ""); - const prefixed = CSV_DANGEROUS_PREFIX.test(raw) ? `'${raw}` : raw; - return `"${prefixed.replace(/"/g, '""')}"`; - }; const csv = header + rows .map( (r) => - `${escapeCsv(r.id)},${escapeCsv(r.name)},${escapeCsv(r.email)},${r.email_verified},${escapeCsv(r.created_at)},${r.banned}`, + `${escapeCSVValue(r.id)},${escapeCSVValue(r.name)},${escapeCSVValue(r.email)},${r.email_verified},${escapeCSVValue(r.created_at)},${r.banned}`, ) .join("\n"); diff --git a/packages/server/src/routes/betterbase/index.ts b/packages/server/src/routes/betterbase/index.ts index c2396fd..540ffe0 100644 --- a/packages/server/src/routes/betterbase/index.ts +++ b/packages/server/src/routes/betterbase/index.ts @@ -5,6 +5,7 @@ import { formatError, lookupFunction, } from "@betterbase/core"; +import { zValidator } from "@hono/zod-validator"; import { Hono } from "hono"; import { nanoid } from "nanoid"; import { z } from "zod"; @@ -20,7 +21,7 @@ import { PutObjectCommand, S3Client } from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; export const betterbaseRouter = new Hono(); -const SAFE_PROJECT_SLUG = /^[a-z][a-z0-9_]{0,62}$/; +const SAFE_PROJECT_SLUG = /^[a-z][a-z0-9-]{0,62}$/; const ALLOWED_UPLOAD_CONTENT_TYPES = new Set([ "image/jpeg", "image/png", @@ -58,7 +59,7 @@ betterbaseRouter.post("/:kind/*", async (c) => { const token = extractBearerToken(c.req.header("Authorization")); const adminPayload = token ? await verifyAdminToken(token) : null; if (!adminPayload) return c.json({ error: "Unauthorized" }, 401); - const authCtx = { userId: adminPayload?.sub ?? null, token }; + const authCtx = { userId: adminPayload.sub, token }; // Build DB context const pool = getPool(); @@ -106,12 +107,18 @@ betterbaseRouter.post("/:kind/*", async (c) => { // Storage context builder function buildStorageCtx(pool: any, projectSlug: string): StorageCtx { const env = validateEnv(); + + // Storage endpoint and credentials are required (validated in env.ts) + if (!env.STORAGE_ENDPOINT || !env.STORAGE_ACCESS_KEY || !env.STORAGE_SECRET_KEY) { + throw new Error("Storage not configured: missing STORAGE_ENDPOINT, STORAGE_ACCESS_KEY, or STORAGE_SECRET_KEY"); + } + return new StorageCtx({ pool, projectSlug, - endpoint: env.STORAGE_ENDPOINT ?? "http://minio:9000", - accessKey: env.STORAGE_ACCESS_KEY ?? "", - secretKey: env.STORAGE_SECRET_KEY ?? "", + endpoint: env.STORAGE_ENDPOINT, + accessKey: env.STORAGE_ACCESS_KEY, + secretKey: env.STORAGE_SECRET_KEY, bucket: env.STORAGE_BUCKET ?? "betterbase", publicBase: env.STORAGE_PUBLIC_BASE, }); @@ -189,60 +196,104 @@ function buildActionCtx(pool: any, dbSchema: string, auth: any, projectSlug: str } // Direct browser upload endpoint: POST /betterbase/storage/generate-upload-url -betterbaseRouter.post("/storage/generate-upload-url", async (c) => { - const token = extractBearerToken(c.req.header("Authorization")); - const adminPayload = token ? await verifyAdminToken(token) : null; - if (!adminPayload) return c.json({ error: "Unauthorized" }, 401); +betterbaseRouter.post( + "/storage/generate-upload-url", + zValidator( + "json", + z.object({ + contentType: z.string(), + filename: z.string(), + }), + ), + async (c) => { + const token = extractBearerToken(c.req.header("Authorization")); + const adminPayload = token ? await verifyAdminToken(token) : null; + if (!adminPayload) return c.json({ error: "Unauthorized" }, 401); - const { contentType, filename } = await c.req.json(); - const safeContentType = - typeof contentType === "string" && ALLOWED_UPLOAD_CONTENT_TYPES.has(contentType) - ? contentType - : null; - if (!safeContentType) return c.json({ error: "Unsupported content type" }, 400); + const { contentType, filename } = c.req.valid("json"); - const projectSlug = c.req.header("X-Project-Slug") ?? "default"; - if (!SAFE_PROJECT_SLUG.test(projectSlug)) { - return c.json({ error: "Invalid project slug" }, 400); - } - const storageId = `st_${nanoid(20)}`; - const ext = filename?.split(".").pop() ?? ""; - const s3Key = `project_${projectSlug}/${storageId}${ext ? "." + ext : ""}`; - const env = validateEnv(); + // Validate contentType against allowlist + if (!ALLOWED_UPLOAD_CONTENT_TYPES.has(contentType)) { + return c.json({ error: "Unsupported content type" }, 400); + } - const s3 = new S3Client({ - endpoint: env.STORAGE_ENDPOINT ?? "http://minio:9000", - region: "us-east-1", - credentials: { - accessKeyId: env.STORAGE_ACCESS_KEY ?? "", - secretAccessKey: env.STORAGE_SECRET_KEY ?? "", - }, - forcePathStyle: true, - }); + const projectSlug = c.req.header("X-Project-Slug") ?? "default"; + if (!SAFE_PROJECT_SLUG.test(projectSlug)) { + return c.json({ error: "Invalid project slug" }, 400); + } - const uploadUrl = await getSignedUrl( - s3, - new PutObjectCommand({ - Bucket: env.STORAGE_BUCKET ?? "betterbase", - Key: s3Key, - ContentType: safeContentType, - }), - { expiresIn: 300 }, - ); + const storageId = `st_${nanoid(20)}`; - // Record the pending upload in the DB so getUrl() works after upload - const pool = getPool(); - await pool.query( - `INSERT INTO "project_${projectSlug}"._iac_storage + // Safely extract and validate file extension + let ext = ""; + try { + const trimmedFilename = filename.trim(); + if (!trimmedFilename) { + return c.json({ error: "Invalid filename" }, 400); + } + + const parts = trimmedFilename.split("."); + if (parts.length > 1) { + const rawExt = parts[parts.length - 1]; + // Validate extension: no path separators, no query params, max 16 chars + if ( + rawExt.includes("/") || + rawExt.includes("?") || + rawExt.includes("\\") || + rawExt.length > 16 + ) { + ext = ""; // Fall back to no extension + } else { + ext = rawExt; + } + } + } catch (err) { + return c.json({ error: "Invalid filename" }, 400); + } + + const s3Key = `project_${projectSlug}/${storageId}${ext ? "." + ext : ""}`; + + // Ensure s3Key has no path separators beyond the expected structure + if (s3Key.split("/").length > 2 || s3Key.includes("..")) { + return c.json({ error: "Invalid filename" }, 400); + } + + const env = validateEnv(); + + // Validate storage credentials are configured + if (!env.STORAGE_ENDPOINT || !env.STORAGE_ACCESS_KEY || !env.STORAGE_SECRET_KEY) { + return c.json({ error: "Storage not configured" }, 500); + } + + const s3 = new S3Client({ + endpoint: env.STORAGE_ENDPOINT, + region: "us-east-1", + credentials: { + accessKeyId: env.STORAGE_ACCESS_KEY, + secretAccessKey: env.STORAGE_SECRET_KEY, + }, + forcePathStyle: true, + }); + + const uploadUrl = await getSignedUrl( + s3, + new PutObjectCommand({ + Bucket: env.STORAGE_BUCKET ?? "betterbase", + Key: s3Key, + ContentType: contentType, + }), + { expiresIn: 300 }, + ); + + // Record the pending upload in the DB so getUrl() works after upload + const pool = getPool(); + await pool.query( + `INSERT INTO "project_${projectSlug}"._iac_storage (storage_id, s3_key, bucket, content_type) VALUES ($1, $2, $3, $4) ON CONFLICT (storage_id) DO NOTHING`, - [ - storageId, - s3Key, - env.STORAGE_BUCKET ?? "betterbase", - safeContentType, - ], - ); - - return c.json({ storageId, uploadUrl }); -}); + [storageId, s3Key, env.STORAGE_BUCKET ?? "betterbase", contentType], + ); + + return c.json({ storageId, uploadUrl }); + }, +); diff --git a/packages/server/src/routes/betterbase/ws.ts b/packages/server/src/routes/betterbase/ws.ts index aa389b6..8ef49c6 100644 --- a/packages/server/src/routes/betterbase/ws.ts +++ b/packages/server/src/routes/betterbase/ws.ts @@ -138,7 +138,7 @@ export function getWSStats() { /** Mount in Bun.serve() options */ export function getBunServeConfig() { return { - fetch(req: Request, server: any) { + async fetch(req: Request, server: any) { const url = new URL(req.url); if (url.pathname === "/betterbase/ws") { const token = url.searchParams.get("token"); @@ -146,7 +146,19 @@ export function getBunServeConfig() { if (!payload) return new Response("Unauthorized", { status: 401 }); const projectSlug = url.searchParams.get("project") ?? "default"; - const upgraded = server.upgrade(req, { data: { projectSlug, userId: payload.sub } }); + + // Validate that the verified payload is authorized for the requested project + // For admin tokens, we allow access to any project + // Additional scope/role checks could be added here if needed + + // Scrub the token from URL searchParams before upgrade to prevent logging + const scrubbedUrl = new URL(req.url); + scrubbedUrl.searchParams.delete("token"); + const scrubbedReq = new Request(scrubbedUrl, req); + + const upgraded = server.upgrade(scrubbedReq, { + data: { projectSlug, userId: payload.sub }, + }); if (upgraded) return undefined; return new Response("WebSocket upgrade failed", { status: 400 }); } diff --git a/packages/server/src/routes/device/index.ts b/packages/server/src/routes/device/index.ts index 912df07..32d23da 100644 --- a/packages/server/src/routes/device/index.ts +++ b/packages/server/src/routes/device/index.ts @@ -11,20 +11,43 @@ export const deviceRouter = new Hono(); const CODE_EXPIRY_MINUTES = 10; const DEVICE_CODE_RATE_LIMIT_WINDOW_MS = 60_000; const DEVICE_CODE_RATE_LIMIT_MAX = 5; +const RATE_LIMIT_MAP_MAX_SIZE = 10_000; // Cap map size to prevent unbounded growth const deviceCodeRateLimits = new Map(); // POST /device/code — CLI calls this to initiate login deviceRouter.post("/code", async (c) => { - const ip = c.req.header("X-Real-IP") ?? "unknown"; + // Derive safer client key: prefer X-Real-IP, fallback to connection info + // Avoid grouping all header-less requests under literal "unknown" + const xRealIp = c.req.header("X-Real-IP"); + const ip = xRealIp || `fallback-${c.req.raw.headers.get("cf-connecting-ip") || "unknown"}`; + const now = Date.now(); - const recent = (deviceCodeRateLimits.get(ip) ?? []).filter( - (ts) => now - ts < DEVICE_CODE_RATE_LIMIT_WINDOW_MS, - ); + const timestamps = deviceCodeRateLimits.get(ip) ?? []; + const recent = timestamps.filter((ts) => now - ts < DEVICE_CODE_RATE_LIMIT_WINDOW_MS); + if (recent.length >= DEVICE_CODE_RATE_LIMIT_MAX) { return c.json({ error: "Rate limit exceeded. Try again in a minute." }, 429); } + recent.push(now); - deviceCodeRateLimits.set(ip, recent); + + // Prune entry if empty (cleanup), otherwise update + if (recent.length === 0) { + deviceCodeRateLimits.delete(ip); + } else { + deviceCodeRateLimits.set(ip, recent); + } + + // Cap the map size to prevent unbounded growth + if (deviceCodeRateLimits.size > RATE_LIMIT_MAP_MAX_SIZE) { + // Remove oldest entries + const entries = Array.from(deviceCodeRateLimits.entries()); + entries.sort((a, b) => Math.min(...a[1]) - Math.min(...b[1])); + const toDelete = entries.slice(0, Math.floor(RATE_LIMIT_MAP_MAX_SIZE * 0.1)); + for (const [key] of toDelete) { + deviceCodeRateLimits.delete(key); + } + } const pool = getPool();