Harden booking backend against automated abuse

.github/workflows/security.yml

@@ -29,7 +29,9 @@ jobs:
       - name: Setup Node.js
         uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e
         with:
-          node-version: 20
+          # pnpm 10.32.1 requires Node >= 22.13 (it loads node:sqlite); the
+          # deployed runtime stays Node 20 via Vercel / package.json engines.
+          node-version: 22
           cache: pnpm
           cache-dependency-path: pnpm-lock.yaml
 

api/_shared/rate-limit.ts

@@ -38,15 +38,6 @@ const buckets =
   globalRateLimitState.__mediaSmartRateLimitBuckets ??
   (globalRateLimitState.__mediaSmartRateLimitBuckets = new Map());
 
-function normalizeUserAgent(headers: IncomingHttpHeaders) {
-  const userAgent = headers['user-agent'];
-  const raw = Array.isArray(userAgent) ? userAgent[0] : userAgent;
-
-  return typeof raw === 'string'
-    ? raw.trim().toLowerCase().slice(0, 160)
-    : '';
-}
-
 function maybeCleanupBuckets(now: number) {
   const lastCleanup = globalRateLimitState.__mediaSmartRateLimitLastCleanup ?? 0;
   const shouldCleanupByAge = now - lastCleanup >= CLEANUP_INTERVAL_MS;
@@ -77,14 +68,12 @@ function maybeCleanupBuckets(now: number) {
 }
 
 export function getRateLimitIdentifier(headers: IncomingHttpHeaders) {
-  const clientIp = extractClientIp(headers);
-  const userAgent = normalizeUserAgent(headers);
-
-  if (clientIp && userAgent) {
-    return `${clientIp}:${userAgent}`;
-  }
-
-  return clientIp || userAgent || 'anonymous';
+  // Key on the client IP only. On Vercel `x-forwarded-for` is overwritten with
+  // the real client IP and client-supplied values are not forwarded, so the IP
+  // is trustworthy. We deliberately do NOT fold in the User-Agent: it is fully
+  // client-controlled, so mixing it into the key let an attacker mint a fresh
+  // rate-limit bucket on every request just by rotating the header.
+  return extractClientIp(headers) || 'anonymous';
 }
 
 export function checkRateLimit({

api/booking/_lib/d1.ts

@@ -33,6 +33,14 @@ export class D1Error extends Error {
   }
 }
 
+// SQLite (D1) surfaces a partial-unique-index violation as a
+// "UNIQUE constraint failed: ..." error. Callers use this to turn a lost race
+// for a booking slot into a clean 409 instead of a 500.
+export function isUniqueConstraintError(err: unknown): boolean {
+  const message = err instanceof Error ? err.message : String(err);
+  return /UNIQUE constraint failed/i.test(message);
+}
+
 async function rawQuery<Row>(
   sql: string,
   params: Array<string | number | null>,

api/booking/_lib/mailer.ts

@@ -60,7 +60,23 @@ function formatIcsDate(date: Date): string {
 }
 
 function escapeIcs(value: string): string {
-  return value.replace(/\\/g, '\\\\').replace(/,/g, '\\,').replace(/;/g, '\\;').replace(/\n/g, '\\n');
+  return value
+    .replace(/\\/g, '\\\\')
+    .replace(/,/g, '\\,')
+    .replace(/;/g, '\\;')
+    .replace(/\r\n|\r|\n/g, '\\n');
+}
+
+function quoteIcsParam(value: string): string {
+  // RFC 5545 param values are DQUOTE-wrapped and must not contain DQUOTE or any
+  // control character (C0 range + DEL).
+  const stripped = Array.from(value)
+    .filter((char) => {
+      const code = char.codePointAt(0) ?? 0;
+      return code >= 0x20 && code !== 0x7f && char !== String.fromCharCode(0x22);
+    })
+    .join("");
+  return `"${stripped}"`;
 }
 
 function buildIcs(input: SendInputs): string {
@@ -90,7 +106,7 @@ function buildIcs(input: SendInputs): string {
     ),
     input.meetLink ? `LOCATION:${escapeIcs(input.meetLink)}` : 'LOCATION:Online',
     `ORGANIZER;CN=MediaSmart:mailto:${NOTIFICATION_EMAIL}`,
-    `ATTENDEE;CN=${escapeIcs(input.attendeeName)};RSVP=TRUE:mailto:${input.attendeeEmail}`,
+    `ATTENDEE;CN=${quoteIcsParam(input.attendeeName)};RSVP=TRUE:mailto:${input.attendeeEmail}`,
     'END:VEVENT',
     'END:VCALENDAR',
   ];

api/booking/_lib/tokens.ts

@@ -2,35 +2,71 @@ import { createHmac, randomUUID, timingSafeEqual } from 'crypto';
 
 import { getRuntimeEnv } from './config';
 
-// Cancellation / reschedule links are stateless: each booking gets a token
-// derived from `HMAC(secret, bookingId + ":" + purpose)`. Storing nothing
-// extra in the DB means we can't accidentally leak tokens via a DB dump and
-// rotating the HMAC secret invalidates every outstanding link at once.
+// Cancellation / reschedule links are stateless-ish: each booking gets a token
+// derived from `HMAC(secret, bookingId + ":" + purpose + ":" + exp + ":" + version)`.
+//
+// Two properties are bound into the signed message so a leaked link is not a
+// permanent, unrevocable bearer credential:
+//   - `exp`     — a unix-seconds expiry. verifyToken refuses expired tokens, so
+//                 a link that leaks (forwarded email, referrer, proxy log) only
+//                 grants access for a bounded window.
+//   - `version` — the booking's `token_version` column. Rescheduling bumps it,
+//                 which invalidates every previously-issued link for that
+//                 booking without touching the global HMAC secret.
+//
+// The wire format is `${exp}.${signature}` where signature is base64url. The
+// server still stores nothing token-specific beyond the small integer
+// token_version, so a DB dump cannot leak usable tokens.
 
 type TokenPurpose = 'cancel' | 'reschedule';
 
-function compute(bookingId: string, purpose: TokenPurpose): string {
+// Manage links stay valid comfortably past the 28-day booking horizon so a
+// visitor can always cancel/reschedule up to (and shortly after) their slot.
+const MANAGE_TOKEN_TTL_MS = 45 * 24 * 60 * 60 * 1000;
+
+function compute(
+  bookingId: string,
+  purpose: TokenPurpose,
+  exp: number,
+  version: number,
+): string {
   const secret = getRuntimeEnv().bookingSecret;
   return createHmac('sha256', secret)
-    .update(`${bookingId}:${purpose}`)
+    .update(`${bookingId}:${purpose}:${exp}:${version}`)
     .digest('base64url');
 }
 
-export function generateToken(bookingId: string, purpose: TokenPurpose): string {
-  return compute(bookingId, purpose);
+export function generateToken(
+  bookingId: string,
+  purpose: TokenPurpose,
+  version: number,
+  ttlMs: number = MANAGE_TOKEN_TTL_MS,
+): string {
+  const exp = Math.floor((Date.now() + ttlMs) / 1000);
+  return `${exp}.${compute(bookingId, purpose, exp, version)}`;
 }
 
 export function verifyToken(
   bookingId: string,
   purpose: TokenPurpose,
+  version: number,
   candidate: string,
 ): boolean {
-  const expected = compute(bookingId, purpose);
+  const dot = candidate.indexOf('.');
+  if (dot <= 0) return false;
+
+  const expStr = candidate.slice(0, dot);
+  const signature = candidate.slice(dot + 1);
+  const exp = Number(expStr);
+  // Reject non-integer or already-expired tokens before touching the HMAC.
+  if (!Number.isInteger(exp) || exp * 1000 < Date.now()) return false;
+
+  const expected = compute(bookingId, purpose, exp, version);
   // Constant-time comparison guards against the timing side-channel that
   // string equality would expose. Length mismatch is its own short-circuit
   // because timingSafeEqual throws otherwise.
   const a = Buffer.from(expected);
-  const b = Buffer.from(candidate);
+  const b = Buffer.from(signature);
   if (a.length !== b.length) return false;
   return timingSafeEqual(a, b);
 }

api/booking/cancel.ts

@@ -24,6 +24,7 @@ interface BookingRow {
   start_at: number;
   status: string;
   calendar_event_id: string | null;
+  token_version: number;
 }
 
 export default async function handler(req: ApiRequest, res: ApiResponse) {
@@ -59,18 +60,17 @@ export default async function handler(req: ApiRequest, res: ApiResponse) {
     return res.status(400).json({ success: false, message: 'Missing id or token' });
   }
 
-  if (!verifyToken(id, 'cancel', token)) {
-    return res.status(403).json({ success: false, message: 'Invalid token' });
-  }
-
+  // Fetch the row first: the token is versioned, so verification needs the
+  // booking's current token_version. A missing row and a bad token return the
+  // same 403 so an outsider cannot probe which booking ids exist.
   const row = await queryFirst<BookingRow>(
-    `SELECT id, attendee_name, attendee_email, attendee_language, start_at, status, calendar_event_id
+    `SELECT id, attendee_name, attendee_email, attendee_language, start_at, status, calendar_event_id, token_version
      FROM bookings WHERE id = ?`,
     [id],
   );
 
-  if (!row) {
-    return res.status(404).json({ success: false, message: 'Booking not found' });
+  if (!row || !verifyToken(id, 'cancel', row.token_version, token)) {
+    return res.status(403).json({ success: false, message: 'Invalid token' });
   }
 
   // Already cancelled = success (idempotent). Saves us reasoning about double

api/booking/create.ts

@@ -4,19 +4,32 @@ import {
   checkRateLimit,
   getRateLimitIdentifier,
 } from '../_shared/rate-limit';
+import recaptcha from '../_shared/recaptcha.js';
 
 import {
   BOOKING_TIMEZONE,
   MEETING_DURATION_MIN,
   getRuntimeEnv,
 } from './_lib/config';
-import { exec, queryFirst } from './_lib/d1';
-import { createEvent, getBusyIntervals } from './_lib/google-calendar';
+import { exec, isUniqueConstraintError, queryFirst } from './_lib/d1';
+import { createEvent, deleteEvent, getBusyIntervals } from './_lib/google-calendar';
 import { sendBookingConfirmation } from './_lib/mailer';
 import { isSlotValid } from './_lib/slots';
 import { generateToken, newBookingId } from './_lib/tokens';
 import { validateCreatePayload } from './_lib/validators';
 
+const { extractClientIp, verifyRecaptcha } = recaptcha;
+
+// Best-effort release of a reserved-but-not-finalised booking row. Awaited by
+// callers so the DELETE actually runs before the serverless response is flushed.
+async function releaseBookingRow(bookingId: string): Promise<void> {
+  try {
+    await exec('DELETE FROM bookings WHERE id = ?', [bookingId]);
+  } catch (err) {
+    console.error('booking/create failed to release reserved slot', bookingId, err);
+  }
+}
+
 const CREATE_RATE_LIMIT = {
   limit: 5,
   windowMs: 30 * 60 * 1000,
@@ -52,6 +65,20 @@ export default async function handler(req: ApiRequest, res: ApiResponse) {
     return res.status(200).json({ success: true, bookingId: 'honeypot' });
   }
 
+  // Anti-automation: create is the only unauthenticated booking endpoint (the
+  // others require a signed manage token), so it gets reCAPTCHA v3 like the
+  // contact form. The honeypot above stays as cheap defense-in-depth.
+  const recaptchaResult = await verifyRecaptcha({
+    token: (req.body as { recaptchaToken?: unknown } | undefined)?.recaptchaToken,
+    expectedAction: 'booking_create',
+    remoteIp: extractClientIp(req.headers),
+  });
+  if (!recaptchaResult.ok) {
+    return res
+      .status(recaptchaResult.status ?? 400)
+      .json({ success: false, message: recaptchaResult.message });
+  }
+
   const start = parsed.value.startUtc;
   const end = new Date(start.getTime() + MEETING_DURATION_MIN * 60_000);
 
@@ -89,6 +116,46 @@ export default async function handler(req: ApiRequest, res: ApiResponse) {
     .filter(Boolean)
     .join('\n\n');
 
+  const nowMs = Math.floor(Date.now() / 1000);
+  const startSec = Math.floor(start.getTime() / 1000);
+  const endSec = Math.floor(end.getTime() / 1000);
+
+  // 1. Claim the slot in the database FIRST. The partial unique index
+  //    `(start_at) WHERE status='confirmed'` is the authoritative mutual-
+  //    exclusion guarantee: if a concurrent request already booked this slot the
+  //    insert fails and we return 409 instead of silently double-booking.
+  //    calendar_event_id is filled in once the event below is created.
+  try {
+    await exec(
+      `INSERT INTO bookings (
+        id, calendar_event_id, attendee_name, attendee_email, attendee_message,
+        attendee_language, start_at, end_at, status, token_version, created_at, updated_at
+      ) VALUES (?, NULL, ?, ?, ?, ?, ?, ?, 'confirmed', 0, ?, ?)`,
+      [
+        bookingId,
+        parsed.value.name,
+        parsed.value.email,
+        parsed.value.message,
+        parsed.value.language,
+        startSec,
+        endSec,
+        nowMs,
+        nowMs,
+      ],
+    );
+  } catch (err) {
+    if (isUniqueConstraintError(err)) {
+      return res
+        .status(409)
+        .json({ success: false, message: 'Slot is no longer available' });
+    }
+    console.error('booking/create db claim failed', err);
+    return res.status(500).json({ success: false, message: 'Could not save booking' });
+  }
+
+  // 2. Create the calendar event now that the slot is durably reserved. On
+  //    failure we release the claimed row (awaited) so it never lingers and the
+  //    slot is freed for someone else.
   let event;
   try {
     event = await createEvent({
@@ -101,48 +168,36 @@ export default async function handler(req: ApiRequest, res: ApiResponse) {
       attendeeEmail: parsed.value.email,
     });
   } catch (err) {
-    console.error('booking/create calendar insert failed', err);
+    console.error('booking/create calendar insert failed; releasing slot', err);
+    await releaseBookingRow(bookingId);
     return res.status(502).json({ success: false, message: 'Could not create event' });
   }
 
-  // Persist after the calendar write so we never have a row pointing at a
-  // non-existent event. If the DB write fails after the event was created,
-  // we fall through to the catch and undo the event — better to lose a row
-  // than leave a phantom booking on the user's calendar.
-  const nowMs = Math.floor(Date.now() / 1000);
+  // 3. Attach the calendar event id to the reserved row. If this fails we undo
+  //    both the event and the row (awaited) so we never leave a confirmed slot
+  //    without a matching calendar entry — the previous fire-and-forget rollback
+  //    could be dropped when the serverless instance froze after responding.
   try {
-    await exec(
-      `INSERT INTO bookings (
-        id, calendar_event_id, attendee_name, attendee_email, attendee_message,
-        attendee_language, start_at, end_at, status, created_at, updated_at
-      ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, 'confirmed', ?, ?)`,
-      [
-        bookingId,
-        event.id,
-        parsed.value.name,
-        parsed.value.email,
-        parsed.value.message,
-        parsed.value.language,
-        Math.floor(start.getTime() / 1000),
-        Math.floor(end.getTime() / 1000),
-        nowMs,
-        nowMs,
-      ],
+    const updateMeta = await exec(
+      `UPDATE bookings SET calendar_event_id = ?, updated_at = ? WHERE id = ?`,
+      [event.id, Math.floor(Date.now() / 1000), bookingId],
     );
+    // D1 does not throw when the WHERE matches zero rows, so confirm the
+    // reserved row is still present before reporting success.
+    if (updateMeta.changes === 0) {
+      throw new Error(`reserved booking row ${bookingId} vanished before event link`);
+    }
   } catch (err) {
-    console.error('booking/create db write failed; rolling back event', err);
-    // Best-effort rollback — we don't await this because we want to respond
-    // quickly. A leftover event is a degraded state we'll log and recover.
-    void import('./_lib/google-calendar')
-      .then((mod) => mod.deleteEvent(event.id))
-      .catch((rollbackErr) =>
-        console.error('booking/create rollback failed', rollbackErr),
-      );
+    console.error('booking/create db update failed; rolling back', err);
+    await Promise.allSettled([
+      deleteEvent(event.id),
+      releaseBookingRow(bookingId),
+    ]);
     return res.status(500).json({ success: false, message: 'Could not save booking' });
   }
 
-  const cancelToken = generateToken(bookingId, 'cancel');
-  const rescheduleToken = generateToken(bookingId, 'reschedule');
+  const cancelToken = generateToken(bookingId, 'cancel', 0);
+  const rescheduleToken = generateToken(bookingId, 'reschedule', 0);
   // Manage links are language-prefixed so the visitor lands on the page in
   // the language they used to book; the rest of the site is also localised.
   const langPrefix = parsed.value.language;