Security Patch: Sanitize Error Responses to Prevent Internal Schema and Data Leakage

backend/app.js

@@ -6,17 +6,57 @@ import chatRoutes from "./routers/chatRoutes.js";
 import aiRoutes from "./routers/aiRoutes.js";
 import matchRoutes from "./routers/matchRoutes.js";
 import authRoutes from "./routers/authRoutes.js";
+import cronRoutes from "./routers/cronRoutes.js";
+import notificationRoutes from "./routers/notificationRoutes.js";
 import { errorHandler } from "./middlewares/errorHandler.js";
 
 const app = express();
 
-// SECURITY: Only trust proxy if explicitly configured (e.g., when behind Nginx/Cloudflare)
-// This prevents attackers from spoofing their IP via X-Forwarded-For headers
-if (process.env.TRUST_PROXY === "true") {
+// SECURITY: Only trust proxy headers when explicitly configured.
+// Set TRUST_PROXY=true for simple single-hop proxies (e.g., Heroku, Render).
+// Set TRUSTED_PROXIES="10.0.0.0/8,172.16.0.0/12" for explicit subnet whitelisting (recommended).
+// When neither is set, req.ip always returns the raw socket address, preventing X-Forwarded-For spoofing.
+if (process.env.TRUSTED_PROXIES) {
+  app.set("trust proxy", process.env.TRUSTED_PROXIES.split(",").map(s => s.trim()));
+  console.log(`[security] trust proxy enabled for subnets: ${process.env.TRUSTED_PROXIES}`);
+} else if (process.env.TRUST_PROXY === "true") {
   app.set("trust proxy", 1);
+  console.warn("[security] trust proxy enabled with hop-count 1. Consider setting TRUSTED_PROXIES for explicit subnet whitelisting.");
 }
 
-app.use(cors({ origin: process.env.FRONTEND_URL || '*', credentials: true }));
+// SECURITY: Build a strict CORS origin whitelist.
+// - FRONTEND_URL can be a single URL or comma-separated list (e.g., "https://app.example.com,https://staging.example.com").
+// - In production, the server refuses to start if FRONTEND_URL is missing.
+// - In development, it defaults to common localhost origins for convenience.
+const buildAllowedOrigins = () => {
+  const raw = process.env.FRONTEND_URL;
+
+  if (raw) {
+    return raw.split(",").map(s => s.trim()).filter(Boolean);
+  }
+
+  if (process.env.NODE_ENV === "production") {
+    console.error("[security] FATAL: FRONTEND_URL is not set. Refusing to start with a wildcard CORS policy in production.");
+    process.exit(1);
+  }
+
+  console.warn("[security] FRONTEND_URL not set. Defaulting to localhost origins for development.");
+  return ["http://localhost:5173", "http://localhost:3000", "http://localhost:8080"];
+};
+
+const allowedOrigins = new Set(buildAllowedOrigins());
+
+app.use(cors({
+  origin: (origin, callback) => {
+    // Allow requests with no origin (e.g., server-to-server, curl, mobile apps)
+    if (!origin || allowedOrigins.has(origin)) {
+      callback(null, true);
+    } else {
+      callback(new Error(`CORS: origin '${origin}' is not allowed`));
+    }
+  },
+  credentials: true,
+}));
 // Cap incoming JSON body size to 100 KB so a single oversized request
 // cannot exhaust server memory or cause a denial-of-service condition.
 app.use(express.json({ limit: "100kb" }));

backend/middlewares/errorHandler.js

@@ -1,28 +1,93 @@
 import { ZodError } from "zod";
+import { HttpError } from "../utils/httpError.js";
 
+const isProduction = process.env.NODE_ENV === "production";
+
+/**
+ * Map of generic, user-safe messages for common HTTP status codes.
+ * Used in production to avoid leaking internal details.
+ */
+const SAFE_STATUS_MESSAGES = {
+  400: "Bad request.",
+  401: "Authentication required.",
+  403: "Access denied.",
+  404: "Resource not found.",
+  409: "Conflict.",
+  422: "Unprocessable request.",
+  429: "Too many requests.",
+  500: "Internal server error.",
+  502: "Service temporarily unavailable.",
+  503: "Service unavailable.",
+};
+
+/**
+ * Global Express error handler.
+ *
+ * Security strategy:
+ *   - HttpError instances (intentionally thrown by our code) have safe, controlled messages.
+ *     These are always returned to the client.
+ *   - ZodError instances are validation errors. In production, only a generic
+ *     "Validation failed" message is returned. In development, full field-level details are included.
+ *   - All other unexpected errors (library crashes, DB errors, etc.) are fully masked
+ *     in production with a generic status message. Verbose details are only shown in development.
+ *   - Full error details are always logged server-side with the request ID for debugging.
+ */
 export const errorHandler = (err, req, res, next) => {
-  // Gracefully handle Zod validation errors
+  if (res.headersSent) {
+    return next(err);
+  }
+
+  const requestId = req.requestId || "unknown";
+
+  // --- Zod Validation Errors ---
   if (err instanceof ZodError || err.name === "ZodError") {
-    // Downgrade to console.warn to prevent log pollution with massive stack traces
-    console.warn("Validation Error:", err.errors || err.message);
-    return res.status(400).json({ 
-      error: "Validation failed", 
-      details: err.errors 
-    });
+    console.warn(`[${requestId}] Validation Error:`, err.errors || err.message);
+
+    const payload = { error: "Validation failed" };
+
+    // Only include field-level details in development
+    if (!isProduction) {
+      payload.details = err.errors;
+    }
+
+    return res.status(400).json(payload);
   }
 
-  console.error("Unhandled error:", err);
+  // --- Known HttpError (intentionally thrown by our code) ---
+  if (err instanceof HttpError) {
+    const status = err.statusCode || 500;
+    console.error(`[${requestId}] HttpError ${status}:`, err.message);
 
-  if (res.headersSent) {
-    return next(err);
+    const payload = { error: err.message };
+
+    // Only include details in development (details may contain internal info)
+    if (!isProduction && err.details) {
+      payload.details = err.details;
+    }
+
+    return res.status(status).json(payload);
   }
 
+  // --- Unexpected / Unknown Errors ---
+  // Always log the full error server-side for debugging
+  console.error(`[${requestId}] Unhandled error:`, err);
+
   const status = err.status || err.statusCode || 500;
-  const message = err.message || "Internal server error";
 
-  const payload = { error: message };
+  if (isProduction) {
+    // Return a generic message — never leak internal details
+    const safeMessage = SAFE_STATUS_MESSAGES[status] || SAFE_STATUS_MESSAGES[500];
+    return res.status(status).json({ error: safeMessage });
+  }
+
+  // Development: include full details for easier debugging
+  const payload = { error: err.message || "Internal server error" };
   if (err.details) {
     payload.details = err.details;
   }
+  if (err.stack) {
+    payload.stack = err.stack;
+  }
+
   res.status(status).json(payload);
 };

backend/middlewares/rateLimiter.js

@@ -3,6 +3,28 @@ const MAX_REQUESTS = 20;
 const MAX_ENTRIES = 10000;
 const CLEANUP_INTERVAL_MS = 60 * 1000;
 
+/**
+ * Derives a rate-limit key for the current request.
+ *
+ * Priority:
+ *   1. Authenticated user ID (most reliable — cannot be spoofed).
+ *   2. Composite fingerprint combining req.ip, the raw socket remote address,
+ *      and the User-Agent header. This ensures that even if an attacker spoofs
+ *      X-Forwarded-For (when trust proxy is misconfigured), the raw socket IP
+ *      still anchors them to their real origin.
+ */
+const deriveRateLimitKey = (req) => {
+  if (req.user?.id) {
+    return `uid:${req.user.id}`;
+  }
+
+  const expressIp = req.ip || "unknown";
+  const socketIp = req.socket?.remoteAddress || "unknown";
+  const ua = req.headers["user-agent"] || "no-ua";
+
+  return `ip:${expressIp}|${socketIp}|${ua}`;
+};
+
 export const createRateLimiter = (options = {}) => {
   const windowMs = options.windowMs || WINDOW_MS;
   const maxRequests = options.maxRequests || MAX_REQUESTS;
@@ -11,19 +33,19 @@ export const createRateLimiter = (options = {}) => {
   let cleanupTime = Date.now();
 
   return (req, res, next) => {
-    const userId = req.user?.id || req.ip;
+    const key = deriveRateLimitKey(req);
     const now = Date.now();
 
     if (now - cleanupTime >= CLEANUP_INTERVAL_MS) {
-      for (const [key, entry] of store.entries()) {
+      for (const [k, entry] of store.entries()) {
         if (now - entry.windowStart >= windowMs) {
-          store.delete(key);
+          store.delete(k);
         }
       }
       cleanupTime = now;
     }
 
-    let entry = store.get(userId);
+    let entry = store.get(key);
 
     if (!entry || now - entry.windowStart >= windowMs) {
       if (!entry && store.size >= maxEntries) {
@@ -32,7 +54,7 @@ export const createRateLimiter = (options = {}) => {
           store.delete(oldestKey);
         }
       }
-      store.set(userId, { count: 1, windowStart: now });
+      store.set(key, { count: 1, windowStart: now });
       return next();
     }
 

backend/middlewares/requireAuth.js

@@ -60,9 +60,56 @@ const verifyLocalJwt = (token, secret) => {
 };
 
 /**
- * Express middleware that validates a Supabase JWT from the Authorization header.
- * Rejects requests with no token or an invalid/expired token with 401.
- * Attaches the authenticated user object to req.user on success.
+ * Startup check: warn loudly if SUPABASE_JWT_SECRET is missing.
+ * In production, this is a fatal misconfiguration — the server refuses to start.
+ */
+const jwtSecret = process.env.SUPABASE_JWT_SECRET;
+
+if (!jwtSecret) {
+  if (process.env.NODE_ENV === "production") {
+    console.error("[security] FATAL: SUPABASE_JWT_SECRET is not set. Local JWT verification is required in production.");
+    process.exit(1);
+  }
+  console.warn("[security] WARNING: SUPABASE_JWT_SECRET is not set. Using slow network-based auth (dev only).");
+}
+
+/**
+ * Simple rate limiter specifically for the network fallback path.
+ * Prevents attackers from spamming invalid tokens to exhaust Supabase API quotas.
+ */
+const FALLBACK_WINDOW_MS = 60_000;
+const FALLBACK_MAX_REQUESTS = 10;
+const fallbackRateCounts = new Map();
+
+const isFallbackRateLimited = (ip) => {
+  const now = Date.now();
+  const entry = fallbackRateCounts.get(ip);
+
+  if (!entry || now - entry.windowStart >= FALLBACK_WINDOW_MS) {
+    fallbackRateCounts.set(ip, { count: 1, windowStart: now });
+    return false;
+  }
+
+  if (entry.count >= FALLBACK_MAX_REQUESTS) {
+    return true;
+  }
+
+  entry.count += 1;
+  return false;
+};
+
+/**
+ * Express middleware that validates a Supabase JWT.
+ *
+ * Token source priority:
+ *   1. HttpOnly cookie (`access_token`)
+ *   2. Authorization header (`Bearer <token>`)
+ *
+ * Verification strategy:
+ *   - Production: Local HMAC-SHA256 verification only (fast, zero network latency).
+ *     Server refuses to start if SUPABASE_JWT_SECRET is missing.
+ *   - Development: Falls back to Supabase getUser() if the secret is absent,
+ *     with a strict per-IP rate limiter to prevent API exhaustion.
  */
 export const requireAuth = async (req, res, next) => {
   const supabaseAdmin = getSupabaseAdmin();
@@ -84,10 +131,9 @@ export const requireAuth = async (req, res, next) => {
     next(new HttpError(401, "Authentication required"));
     return;
   }
-  const jwtSecret = process.env.SUPABASE_JWT_SECRET;
 
+  // PRIMARY PATH: Fast, local HMAC verification (0ms network latency)
   if (jwtSecret) {
-    // Fast, local verification (0ms network latency)
     const payload = verifyLocalJwt(token, jwtSecret);
     if (!payload) {
       next(new HttpError(401, "Invalid or expired session"));
@@ -104,8 +150,14 @@ export const requireAuth = async (req, res, next) => {
     return next();
   }
 
-  // Slow, fallback network verification
-  console.warn("WARN: SUPABASE_JWT_SECRET is missing. Falling back to slow network-based auth verification.");
+  // FALLBACK PATH (development only — production exits at startup above)
+  // Rate-limit this path to prevent Supabase API quota exhaustion from token spam.
+  const clientIp = req.socket?.remoteAddress || req.ip || "unknown";
+  if (isFallbackRateLimited(clientIp)) {
+    next(new HttpError(429, "Too many authentication attempts. Please try again later."));
+    return;
+  }
+
   const { data, error } = await supabaseAdmin.auth.getUser(token);
 
   if (error || !data?.user) {