Harden auth, payments, and frontend against review findings.

Close exploitable gaps in booking/payment flows, enforce token versioning and account checks, gate sensitive payment data, and add middleware plus input validation across admin routes.

Co-authored-by: Cursor <cursoragent@cursor.com>

backend/src/lib/auth.ts

@@ -4,10 +4,21 @@ import bcrypt from 'bcryptjs';
 import crypto from 'crypto';
 import { Context } from 'hono';
 import { db, dbGet, dbAll, users, magicLinkTokens, userSessions } from '../db/index.js';
-import { eq, and, gt } from 'drizzle-orm';
+import { eq, and, gt, sql, isNull } from 'drizzle-orm';
 import { generateId, getNow, toDbDate } from './utils.js';
 
-const JWT_SECRET = new TextEncoder().encode(process.env.JWT_SECRET || 'your-super-secret-key-change-in-production');
+const DEFAULT_DEV_JWT_SECRET = 'your-super-secret-key-change-in-production';
+const rawJwtSecret = process.env.JWT_SECRET;
+
+// Never allow the insecure default in production: forgeable tokens = full account takeover.
+if (process.env.NODE_ENV === 'production' && (!rawJwtSecret || rawJwtSecret === DEFAULT_DEV_JWT_SECRET)) {
+  throw new Error('JWT_SECRET must be set to a strong, unique value in production. Refusing to start with the default secret.');
+}
+if (!rawJwtSecret) {
+  console.warn('[auth] JWT_SECRET is not set; using an insecure development default. Set JWT_SECRET in production.');
+}
+
+const JWT_SECRET = new TextEncoder().encode(rawJwtSecret || DEFAULT_DEV_JWT_SECRET);
 const JWT_ISSUER = 'spanglish';
 const JWT_AUDIENCE = 'spanglish-app';
 
@@ -15,6 +26,7 @@ export interface JWTPayload {
   sub: string;
   email: string;
   role: string;
+  tokenVersion?: number;
   iat: number;
   exp: number;
 }
@@ -84,23 +96,36 @@ export async function verifyMagicLinkToken(
       )
   );
 
+  // Use a single generic error for all invalid states to avoid leaking token state
+  const genericError = 'Invalid or expired token';
+
   if (!tokenRecord) {
-    return { valid: false, error: 'Invalid token' };
+    return { valid: false, error: genericError };
   }
 
   if (tokenRecord.usedAt) {
-    return { valid: false, error: 'Token already used' };
+    return { valid: false, error: genericError };
   }
 
   if (new Date(tokenRecord.expiresAt) < new Date()) {
-    return { valid: false, error: 'Token expired' };
+    return { valid: false, error: genericError };
   }
 
-  // Mark token as used
-  await (db as any)
+  // Atomically consume the token: only the request that flips used_at from NULL wins.
+  // This prevents a double-spend race where two concurrent requests both pass the
+  // read-time "not used" check above.
+  const result: any = await (db as any)
     .update(magicLinkTokens)
     .set({ usedAt: now })
-    .where(eq((magicLinkTokens as any).id, tokenRecord.id));
+    .where(and(
+      eq((magicLinkTokens as any).id, tokenRecord.id),
+      isNull((magicLinkTokens as any).usedAt)
+    ));
+
+  const affected = result?.changes ?? result?.rowCount ?? 0;
+  if (affected === 0) {
+    return { valid: false, error: genericError };
+  }
 
   return { valid: true, userId: tokenRecord.userId };
 }
@@ -175,18 +200,26 @@ export function validatePassword(password: string): { valid: boolean; error?: st
   return { valid: true };
 }
 
-export async function createToken(userId: string, email: string, role: string): Promise<string> {
-  const token = await new jose.SignJWT({ sub: userId, email, role })
+export async function createToken(userId: string, email: string, role: string, tokenVersion: number = 0): Promise<string> {
+  const token = await new jose.SignJWT({ sub: userId, email, role, tokenVersion })
     .setProtectedHeader({ alg: 'HS256' })
     .setIssuedAt()
     .setIssuer(JWT_ISSUER)
     .setAudience(JWT_AUDIENCE)
-    .setExpirationTime('7d')
+    .setExpirationTime('1d')
     .sign(JWT_SECRET);
   
   return token;
 }
 
+// Invalidate all previously issued JWTs for a user (logout-everywhere, password change/reset).
+export async function bumpTokenVersion(userId: string): Promise<void> {
+  await (db as any)
+    .update(users)
+    .set({ tokenVersion: sql`${(users as any).tokenVersion} + 1` })
+    .where(eq((users as any).id, userId));
+}
+
 export async function createRefreshToken(userId: string): Promise<string> {
   const token = await new jose.SignJWT({ sub: userId, type: 'refresh' })
     .setProtectedHeader({ alg: 'HS256' })
@@ -223,10 +256,44 @@ export async function getAuthUser(c: Context): Promise<any | null> {
     return null;
   }
   
+  // Never load the password hash into request context — it is only needed for
+  // explicit password-verification routes that query it separately.
   const user = await dbGet<any>(
-    (db as any).select().from(users).where(eq((users as any).id, payload.sub))
+    (db as any)
+      .select({
+        id: (users as any).id,
+        email: (users as any).email,
+        name: (users as any).name,
+        phone: (users as any).phone,
+        role: (users as any).role,
+        languagePreference: (users as any).languagePreference,
+        isClaimed: (users as any).isClaimed,
+        googleId: (users as any).googleId,
+        rucNumber: (users as any).rucNumber,
+        accountStatus: (users as any).accountStatus,
+        tokenVersion: (users as any).tokenVersion,
+        createdAt: (users as any).createdAt,
+        updatedAt: (users as any).updatedAt,
+      })
+      .from(users)
+      .where(eq((users as any).id, payload.sub))
   );
-  return user || null;
+
+  if (!user) {
+    return null;
+  }
+
+  // Reject tokens issued before a logout-everywhere / password change
+  if ((payload.tokenVersion ?? 0) !== (user.tokenVersion ?? 0)) {
+    return null;
+  }
+
+  // Suspended/unclaimed accounts must not retain API access via an old JWT
+  if (user.accountStatus && user.accountStatus !== 'active') {
+    return null;
+  }
+
+  return user;
 }
 
 export function requireAuth(roles?: string[]) {
@@ -252,3 +319,15 @@ export async function isFirstUser(): Promise<boolean> {
   );
   return !result || result.length === 0;
 }
+
+/** Fetch only the password hash column (never expose via getAuthUser). */
+export async function getUserPasswordHash(userId: string): Promise<string | null> {
+  const row = await dbGet<any>(
+    (db as any)
+      .select({ password: (users as any).password })
+      .from(users)
+      .where(eq((users as any).id, userId))
+  );
+  const hash = row?.password;
+  return hash && String(hash).length > 0 ? String(hash) : null;
+}

backend/src/lib/email.ts

@@ -247,10 +247,20 @@ async function sendWithConsole(options: SendEmailOptions): Promise<SendEmailResu
 /**
  * Main send function that routes to the appropriate provider
  */
+// Mask an email address for logs: keep first char + domain (e.g. j***@example.com).
+function maskEmail(email: string): string {
+  const [local, domain] = String(email).split('@');
+  if (!domain) return '***';
+  const head = local.slice(0, 1);
+  return `${head}***@${domain}`;
+}
+
 async function sendEmail(options: SendEmailOptions): Promise<SendEmailResult> {
   const provider = getEmailProvider();
   
-  console.log(`[Email] Sending email via ${provider} to ${Array.isArray(options.to) ? options.to.join(', ') : options.to}`);
+  const recipientCount = Array.isArray(options.to) ? options.to.length : 1;
+  const sample = Array.isArray(options.to) ? options.to[0] : options.to;
+  console.log(`[Email] Sending email via ${provider} to ${maskEmail(sample)}${recipientCount > 1 ? ` (+${recipientCount - 1} more)` : ''}`);
   
   switch (provider) {
     case 'resend':
@@ -478,7 +488,7 @@ export const emailService = {
     
     // Replace variables
     const finalSubject = replaceTemplateVariables(subject, allVariables);
-    const finalBodyContent = replaceTemplateVariables(bodyHtml, allVariables);
+    const finalBodyContent = replaceTemplateVariables(bodyHtml, allVariables, true);
     const finalBodyHtml = wrapInBaseTemplate(finalBodyContent, { ...allVariables, subject: finalSubject });
     const finalBodyText = bodyText ? replaceTemplateVariables(bodyText, allVariables) : undefined;
     
@@ -1292,7 +1302,14 @@ export const emailService = {
     eventId?: string;
     sentBy?: string | null;
   }): Promise<{ success: boolean; logId?: string; error?: string }> {
-    const { to, toName, subject, bodyHtml, bodyText, replyTo, eventId, sentBy = null } = params;
+    const { to: rawTo, toName, subject: rawSubject, bodyHtml, bodyText, replyTo: rawReplyTo, eventId, sentBy = null } = params;
+
+    // Strip CR/LF from header-bound values to prevent email header injection
+    // (e.g. an attacker-supplied subject/replyTo smuggling extra headers/recipients).
+    const stripHeader = (v?: string) => (v ? v.replace(/[\r\n]+/g, ' ').trim() : v);
+    const to = stripHeader(rawTo) as string;
+    const subject = stripHeader(rawSubject) as string;
+    const replyTo = stripHeader(rawReplyTo);
     
     const allVariables = {
       ...this.getCommonVariables(),

backend/src/lib/emailTemplates.ts

@@ -1216,8 +1216,26 @@ Spanglish`,
   },
 ];
 
-// Helper function to replace template variables
-export function replaceTemplateVariables(template: string, variables: Record<string, any>): string {
+// Escape HTML-significant characters so substituted variable values can't inject markup.
+function escapeHtmlValue(value: string): string {
+  return value
+    .replace(/&/g, '&amp;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#x27;');
+}
+
+// Helper function to replace template variables.
+// When `escapeHtml` is true (HTML rendering contexts), substituted *values* are
+// HTML-escaped to prevent stored-XSS via attacker-influenced variables (e.g. event
+// location, attendee name). The template markup itself is never escaped. Subjects and
+// plain-text bodies pass `escapeHtml=false` so they don't show literal entities.
+export function replaceTemplateVariables(
+  template: string,
+  variables: Record<string, any>,
+  escapeHtml: boolean = false
+): string {
   let result = template;
   
   // Handle conditional blocks {{#if variable}}...{{/if}}
@@ -1229,16 +1247,20 @@ export function replaceTemplateVariables(template: string, variables: Record<str
   // Replace simple variables {{variable}}
   const variableRegex = /\{\{(\w+)\}\}/g;
   result = result.replace(variableRegex, (match, varName) => {
-    return variables[varName] !== undefined ? String(variables[varName]) : match;
+    if (variables[varName] === undefined) return match;
+    const raw = String(variables[varName]);
+    return escapeHtml ? escapeHtmlValue(raw) : raw;
   });
   
   return result;
 }
 
-// Helper to wrap content in the base template
+// Helper to wrap content in the base template.
+// `content` is treated as trusted HTML (it is the already-rendered body). The wrapper's
+// own variables (subject, year, etc.) are HTML-escaped on substitution.
 export function wrapInBaseTemplate(content: string, variables: Record<string, any>): string {
-  const wrappedContent = baseEmailWrapper.replace('{{content}}', content);
-  return replaceTemplateVariables(wrappedContent, variables);
+  const wrappedContent = baseEmailWrapper.replace('{{content}}', () => content);
+  return replaceTemplateVariables(wrappedContent, variables, true);
 }
 
 // Get all available variables for a template by slug

backend/src/lib/lnbits.ts

@@ -80,11 +80,8 @@ export async function createInvoice(params: CreateInvoiceParams): Promise<LNbits
   }
 
   console.log('Creating LNbits invoice:', {
-    url: `${config.url}${apiEndpoint}`,
     amount: params.amount,
     unit: payload.unit,
-    memo: params.memo,
-    webhook: params.webhookUrl,
   });
 
   const response = await fetch(`${config.url}${apiEndpoint}`, {

backend/src/lib/rateLimit.ts

@@ -0,0 +1,75 @@
+import { Context } from 'hono';
+
+/**
+ * Simple in-memory rate limiter.
+ *
+ * Suitable for a single backend instance (the current deployment model). If the
+ * backend is ever scaled horizontally, replace the in-memory Map with a shared
+ * store (e.g. Redis) so limits are enforced across instances.
+ */
+
+interface Bucket {
+  count: number;
+  resetAt: number;
+}
+
+const buckets = new Map<string, Bucket>();
+
+// Periodically drop expired buckets so the Map does not grow unbounded.
+const cleanup = setInterval(() => {
+  const now = Date.now();
+  for (const [key, bucket] of buckets) {
+    if (now > bucket.resetAt) buckets.delete(key);
+  }
+}, 60_000);
+// Don't keep the process alive just for cleanup.
+(cleanup as any).unref?.();
+
+/** Best-effort client IP extraction (honours common reverse-proxy headers). */
+export function getClientIp(c: Context): string {
+  const forwarded = c.req.header('x-forwarded-for');
+  if (forwarded) return forwarded.split(',')[0].trim();
+  return c.req.header('x-real-ip') || 'unknown';
+}
+
+/**
+ * Consume one unit against a key. Returns whether the request is allowed and,
+ * when blocked, how many seconds until the window resets.
+ */
+export function consumeRateLimit(
+  key: string,
+  max: number,
+  windowMs: number
+): { allowed: boolean; retryAfter?: number } {
+  const now = Date.now();
+  const bucket = buckets.get(key);
+
+  if (!bucket || now > bucket.resetAt) {
+    buckets.set(key, { count: 1, resetAt: now + windowMs });
+    return { allowed: true };
+  }
+
+  bucket.count++;
+  if (bucket.count > max) {
+    return { allowed: false, retryAfter: Math.ceil((bucket.resetAt - now) / 1000) };
+  }
+  return { allowed: true };
+}
+
+/**
+ * Hono middleware factory that rate-limits by client IP.
+ * Use a distinct `prefix` per endpoint group so unrelated routes don't share a bucket.
+ */
+export function rateLimitMiddleware(opts: { max: number; windowMs: number; prefix: string }) {
+  return async (c: Context, next: () => Promise<void>) => {
+    const ip = getClientIp(c);
+    const result = consumeRateLimit(`${opts.prefix}:${ip}`, opts.max, opts.windowMs);
+    if (!result.allowed) {
+      return c.json(
+        { error: 'Too many requests. Please try again later.', retryAfter: result.retryAfter },
+        429
+      );
+    }
+    await next();
+  };
+}