Superadmin impersonation, hardened auth/upload paths, dependency updates

- Add superadmin impersonation: sessions.impersonated_by (migration 0003) is
  stamped onto audit rows so impersonated actions are attributable, with a
  persistent ImpersonationBanner in the app shell.
- Harden auth and upload handling across routes (safe redirect targets,
  filename sanitization, checkout grant handling).
- Update dependencies: Sentry 8 -> 10, @fastify/static 8 -> 10,
  react-router-dom 6.30.6; add find-my-way / fast-uri overrides.
- Add tests for safe-next, checkout-grant, and upload-filename.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

.github/workflows/ci.yml

@@ -29,3 +29,16 @@ jobs:
 
       - name: Build web
         run: npm run build
+
+      # Gate: any HIGH/CRITICAL advisory in a dependency that actually ships to production fails
+      # the build. Scoped with --omit=dev on purpose — the dev-only toolchain (vite, esbuild,
+      # drizzle-kit) carries advisories that only affect a developer's local dev server, and
+      # blocking every PR on those trains people to ignore the gate.
+      - name: Audit production dependencies
+        run: npm audit --omit=dev --audit-level=high
+
+      # Full picture, including dev tooling. Never blocks — it's here so regressions are visible
+      # in the log and someone can act on them deliberately.
+      - name: Audit report (informational, includes dev)
+        if: always()
+        run: npm audit || true

apps/api/package.json

@@ -22,9 +22,9 @@
     "@fastify/helmet": "^12.0.1",
     "@fastify/multipart": "^9.0.1",
     "@fastify/rate-limit": "^10.2.1",
-    "@fastify/static": "^8.0.3",
+    "@fastify/static": "^10.1.3",
     "@lawdesk/db": "*",
-    "@sentry/node": "^8.45.0",
+    "@sentry/node": "^10.71.0",
     "argon2": "^0.41.1",
     "dotenv": "^16.4.5",
     "drizzle-orm": "^0.45.2",

apps/api/src/auth/plugin.ts

@@ -4,6 +4,8 @@ import { SESSION_COOKIE, loadSession } from './sessions';
 import { isProd, env } from '../env';
 import { ensureSuperadminFlag } from './superadmin';
 
+export type FirmRole = 'owner' | 'attorney' | 'paralegal' | 'staff';
+
 declare module 'fastify' {
   interface FastifyRequest {
     user?: {
@@ -14,12 +16,17 @@ declare module 'fastify' {
       isSuperadmin: boolean;
       isSuspended: boolean;
       emailVerified: boolean;
+      /** Superadmin id when this session was opened via admin impersonation, else null. */
+      impersonatedBy: string | null;
     };
   }
   interface FastifyInstance {
     requireAuth: (req: FastifyRequest, reply: FastifyReply) => Promise<void>;
     requireFirm: (req: FastifyRequest, reply: FastifyReply) => Promise<void>;
     requireSuperadmin: (req: FastifyRequest, reply: FastifyReply) => Promise<void>;
+    requireRole: (
+      ...roles: FirmRole[]
+    ) => (req: FastifyRequest, reply: FastifyReply) => Promise<void>;
     setSessionCookie: (reply: FastifyReply, token: string, expiresAt: Date) => void;
     clearSessionCookie: (reply: FastifyReply) => void;
   }
@@ -49,6 +56,7 @@ async function plugin(app: FastifyInstance) {
       isSuperadmin,
       isSuspended: session.user.isSuspended,
       emailVerified: Boolean(session.user.emailVerifiedAt),
+      impersonatedBy: session.session.impersonatedBy,
     };
   });
 
@@ -67,8 +75,28 @@ async function plugin(app: FastifyInstance) {
     if (!req.user) return reply.code(401).send({ error: 'unauthorized' });
     if (req.user.isSuspended) return reply.code(403).send({ error: 'account_suspended' });
     if (!req.user.isSuperadmin) return reply.code(403).send({ error: 'forbidden' });
+    // A superadmin who impersonated their way into a firm must not be able to steer the
+    // platform-wide admin surface from inside that borrowed session.
+    if (req.user.impersonatedBy) return reply.code(403).send({ error: 'forbidden_while_impersonating' });
   });
 
+  // Role gate for firm-scoped routes that only some members of a firm should reach (billing,
+  // full-firm export, account deletion). Compose AFTER requireFirm — it assumes req.user is set
+  // and firmId is present. Superadmins pass, unless they're inside an impersonated session, in
+  // which case they get exactly the target user's real authority and nothing more.
+  app.decorate(
+    'requireRole',
+    (...roles: FirmRole[]) =>
+      async (req: FastifyRequest, reply: FastifyReply) => {
+        if (!req.user) return reply.code(401).send({ error: 'unauthorized' });
+        if (req.user.isSuspended) return reply.code(403).send({ error: 'account_suspended' });
+        if (req.user.isSuperadmin && !req.user.impersonatedBy) return;
+        if (!roles.includes(req.user.role as FirmRole)) {
+          return reply.code(403).send({ error: 'insufficient_role', required: roles });
+        }
+      },
+  );
+
   app.decorate('setSessionCookie', (reply: FastifyReply, token: string, expiresAt: Date) => {
     reply.setCookie(SESSION_COOKIE, token, {
       path: '/',

apps/api/src/auth/sessions.ts

@@ -4,6 +4,9 @@ import { getDb, sessions, users } from '@lawdesk/db';
 
 const SESSION_BYTES = 32;
 const SESSION_TTL_DAYS = 30;
+// Impersonation sessions are support tools, not logins: they expire in an hour so an admin who
+// walks away can't leave a live session inside a customer's firm for the normal 30 days.
+const IMPERSONATION_TTL_MS = 60 * 60 * 1000;
 
 export const SESSION_COOKIE = 'sid';
 
@@ -19,17 +22,23 @@ export interface CreateSessionOpts {
   userId: string;
   ip?: string | null;
   userAgent?: string | null;
+  /** Superadmin id when this session is an impersonation of `userId`. */
+  impersonatedBy?: string | null;
 }
 
 export async function createSession(opts: CreateSessionOpts): Promise<{ token: string; expiresAt: Date }> {
   const token = generateSessionToken();
   const id = hashSessionToken(token);
-  const expiresAt = new Date(Date.now() + SESSION_TTL_DAYS * 24 * 60 * 60 * 1000);
+  const impersonatedBy = opts.impersonatedBy ?? null;
+  const expiresAt = new Date(
+    Date.now() + (impersonatedBy ? IMPERSONATION_TTL_MS : SESSION_TTL_DAYS * 24 * 60 * 60 * 1000),
+  );
 
   await getDb().insert(sessions).values({
     id,
     userId: opts.userId,
     expiresAt,
+    impersonatedBy,
     ip: opts.ip ?? null,
     userAgent: opts.userAgent ?? null,
   });

apps/api/src/lib/audit.ts

@@ -1,3 +1,4 @@
+import type { FastifyRequest } from 'fastify';
 import { getDb, auditLog } from '@lawdesk/db';
 
 export interface AuditEntry {
@@ -6,14 +7,44 @@ export interface AuditEntry {
   action: string;
   meta?: unknown;
   ip?: string | null;
+  /** Superadmin id when the acting session is an impersonation. */
+  impersonatedBy?: string | null;
 }
 
 export async function logAudit(entry: AuditEntry): Promise<void> {
+  // The impersonating admin is folded into `meta` rather than a dedicated column so existing
+  // rows stay valid: an entry written during impersonation records BOTH the user the action
+  // appears to come from and the admin who actually performed it.
+  const meta =
+    entry.impersonatedBy != null
+      ? { ...(typeof entry.meta === 'object' && entry.meta !== null ? entry.meta : { value: entry.meta }), impersonatedBy: entry.impersonatedBy }
+      : entry.meta;
+
   await getDb().insert(auditLog).values({
     userId: entry.userId ?? null,
     firmId: entry.firmId ?? null,
     action: entry.action,
-    meta: entry.meta == null ? null : JSON.stringify(entry.meta),
+    meta: meta == null ? null : JSON.stringify(meta),
     ip: entry.ip ?? null,
   });
 }
+
+/**
+ * Audit an action performed by the current request's user. Carries the impersonating admin
+ * through automatically, so a support session can never write an audit trail that looks like
+ * the customer acted alone.
+ */
+export async function logAuditFromRequest(
+  req: FastifyRequest,
+  action: string,
+  extra: { firmId?: string | null; meta?: unknown } = {},
+): Promise<void> {
+  await logAudit({
+    userId: req.user?.id ?? null,
+    firmId: extra.firmId !== undefined ? extra.firmId : (req.user?.firmId ?? null),
+    action,
+    meta: extra.meta,
+    ip: req.ip,
+    impersonatedBy: req.user?.impersonatedBy ?? null,
+  });
+}

apps/api/src/lib/storage-quota.ts

@@ -5,6 +5,10 @@ import { PLAN_LIMITS, PlanLimitError, type PlanName } from './plan-limits';
 /**
  * Throw a PlanLimitError('storageBytes', plan) if accepting `additionalBytes` more
  * would push the firm past its plan's storage cap. Plans with a null cap are unlimited.
+ *
+ * Advisory only — it reads the counter without holding a lock, so two uploads racing here can
+ * both pass. `reserveStorage` is the authoritative check; this exists to reject an oversized
+ * upload cheaply, before the bytes are written to Spaces.
  */
 export async function assertWithinStorageQuota(
   firmId: string,
@@ -25,8 +29,47 @@ export async function assertWithinStorageQuota(
 }
 
 /**
- * Atomically adjust the firm's tracked storage usage by `deltaBytes` (may be negative).
- * Clamped at 0 so a decrement can never drive the counter below zero.
+ * Atomically claim `bytes` of the firm's quota. The check and the increment happen in ONE
+ * statement — the conditional UPDATE only matches while the firm is still under its cap, so
+ * concurrent uploads serialise on the row and cannot both squeeze past the limit (the previous
+ * read-then-write pair let N parallel uploads each see the same pre-upload total).
+ *
+ * Returns true when the space was claimed. On false the caller must reject the upload.
+ */
+export async function reserveStorage(
+  firmId: string,
+  plan: PlanName,
+  bytes: number,
+): Promise<boolean> {
+  const limit = PLAN_LIMITS[plan].storageBytes;
+
+  const result = await getDb()
+    .update(firms)
+    .set({ storageBytesUsed: sql`${firms.storageBytesUsed} + ${bytes}` })
+    .where(
+      limit === null
+        ? eq(firms.id, firmId)
+        : sql`${firms.id} = ${firmId} and ${firms.storageBytesUsed} + ${bytes} <= ${limit}`,
+    )
+    .returning({ id: firms.id });
+
+  return result.length > 0;
+}
+
+/**
+ * Give back bytes previously reserved — on document delete, or to roll back a reservation whose
+ * upload then failed. Clamped at 0 so a double release can never drive the counter negative.
+ */
+export async function releaseStorage(firmId: string, bytes: number): Promise<void> {
+  await getDb()
+    .update(firms)
+    .set({ storageBytesUsed: sql`GREATEST(0, ${firms.storageBytesUsed} - ${bytes})` })
+    .where(eq(firms.id, firmId));
+}
+
+/**
+ * Adjust the firm's tracked storage usage by an arbitrary delta. Retained for callers that
+ * aren't part of the upload path; prefer reserveStorage/releaseStorage.
  */
 export async function incrementStorageUsed(firmId: string, deltaBytes: number): Promise<void> {
   await getDb()

apps/api/src/routes/account.ts

@@ -14,17 +14,27 @@ import {
   sessions,
 } from '@lawdesk/db';
 import { verifyPassword } from '../auth/password';
-import { logAudit } from '../lib/audit';
+import { logAuditFromRequest } from '../lib/audit';
 import { sendEmail, accountDeletedEmail } from '../lib/email';
 import { deletePrefix, getSignedDownloadUrl } from '../lib/storage';
 
+// Everything under /api/account acts on the whole firm, not just the caller's own rows, so it is
+// owner-only. Without this an 'attorney'/'paralegal'/'staff' member could export every client
+// file the firm holds, or delete the firm outright.
+const OWNER_ONLY = ['owner'] as const;
+
+const EXPORT_URL_TTL_SECONDS = 15 * 60;
+
 export async function accountRoutes(app: FastifyInstance) {
   app.addHook('preHandler', app.requireAuth);
 
   // GDPR data export — full JSON dump of everything tied to the user's firm.
   app.get(
     '/api/account/export',
-    { config: { rateLimit: { max: 5, timeWindow: '1 hour' } } },
+    {
+      config: { rateLimit: { max: 5, timeWindow: '1 hour' } },
+      preHandler: app.requireRole(...OWNER_ONLY),
+    },
     async (req, reply) => {
     const userId = req.user!.id;
     const firmId = req.user!.firmId;
@@ -65,14 +75,18 @@ export async function accountRoutes(app: FastifyInstance) {
       const docs = await db.select().from(documents).where(eq(documents.firmId, firmId));
 
       // GDPR portability covers the files themselves, not just their metadata — attach a
-      // time-limited presigned download URL per document (valid 24h; re-export for fresh links).
+      // time-limited presigned download URL per document. These are unauthenticated bearer URLs
+      // to privileged client material sitting inside a file the user may forward or archive, so
+      // the window is deliberately short (15 minutes): long enough to run the downloads straight
+      // after exporting, short enough that a leaked export is not a document breach. Re-export
+      // for fresh links.
       const docsWithUrls = await Promise.all(
         docs.map(async (d) => {
           try {
-            const downloadUrl = await getSignedDownloadUrl(d.storageKey, d.name, 24 * 60 * 60);
-            return { ...d, downloadUrl, downloadUrlExpiresInHours: 24 };
+            const downloadUrl = await getSignedDownloadUrl(d.storageKey, d.name, EXPORT_URL_TTL_SECONDS);
+            return { ...d, downloadUrl, downloadUrlExpiresInMinutes: EXPORT_URL_TTL_SECONDS / 60 };
           } catch {
-            return { ...d, downloadUrl: null, downloadUrlExpiresInHours: null };
+            return { ...d, downloadUrl: null, downloadUrlExpiresInMinutes: null };
           }
         }),
       );
@@ -88,11 +102,9 @@ export async function accountRoutes(app: FastifyInstance) {
       dump.documents = docsWithUrls;
     }
 
-    await logAudit({
-      userId,
+    await logAuditFromRequest(req, 'account.export', {
       firmId,
-      action: 'account.export',
-      ip: req.ip,
+      meta: { documentUrlsIssued: dump.documents ? (dump.documents as unknown[]).length : 0 },
     });
 
     reply
@@ -106,7 +118,7 @@ export async function accountRoutes(app: FastifyInstance) {
 
   // GDPR delete — password-confirmed. Solo firms cascade everything; multi-user firms must
   // transfer ownership first (we'll add a transfer endpoint when we add team management).
-  app.post('/api/account/delete', async (req, reply) => {
+  app.post('/api/account/delete', { preHandler: app.requireRole(...OWNER_ONLY) }, async (req, reply) => {
     const userId = req.user!.id;
     const firmId = req.user!.firmId;
     const body = z.object({ password: z.string().min(1) }).parse(req.body);
@@ -132,13 +144,7 @@ export async function accountRoutes(app: FastifyInstance) {
       }
     }
 
-    await logAudit({
-      userId,
-      firmId,
-      action: 'account.delete',
-      meta: { email: me.email },
-      ip: req.ip,
-    });
+    await logAuditFromRequest(req, 'account.delete', { firmId, meta: { email: me.email } });
 
     await db.transaction(async (tx) => {
       await tx.delete(sessions).where(eq(sessions.userId, userId));

apps/api/src/routes/admin.ts

@@ -266,7 +266,9 @@ export async function adminRoutes(app: FastifyInstance) {
     return updated;
   });
 
-  // Impersonate: end the current session, start a new one for the target user.
+  // Impersonate: end the current session, start a short-lived one for the target user that is
+  // permanently stamped with the acting admin's id. Every audit row written from that session
+  // carries `impersonatedBy`, so support activity can never be mistaken for the customer's own.
   app.post('/api/admin/users/:id/impersonate', async (req, reply) => {
     const { id } = idParam.parse(req.params);
     const db = getDb();
@@ -275,7 +277,11 @@ export async function adminRoutes(app: FastifyInstance) {
     if (!target) return reply.code(404).send({ error: 'not_found' });
     if (target.isSuspended) return reply.code(409).send({ error: 'target_suspended' });
     if (target.id === req.user!.id) return reply.code(409).send({ error: 'cannot_impersonate_self' });
+    // Never let one platform admin borrow another's identity — that would launder an action
+    // between two accounts that both hold full platform authority.
+    if (target.isSuperadmin) return reply.code(409).send({ error: 'cannot_impersonate_superadmin' });
 
+    const adminId = req.user!.id;
     const oldToken = req.cookies?.[SESSION_COOKIE];
     if (oldToken) await destroySession(oldToken);
 
@@ -283,19 +289,24 @@ export async function adminRoutes(app: FastifyInstance) {
       userId: target.id,
       ip: req.ip,
       userAgent: req.headers['user-agent'] ?? null,
+      impersonatedBy: adminId,
     });
     app.setSessionCookie(reply, token, expiresAt);
     app.setCsrfCookie(reply, generateCsrfToken());
 
     await logAudit({
-      userId: req.user!.id,
+      userId: adminId,
       firmId: target.firmId,
-      action: 'admin.impersonate',
-      meta: { targetUserId: target.id, targetEmail: target.email },
+      action: 'admin.impersonate.start',
+      meta: { targetUserId: target.id, targetEmail: target.email, expiresAt: expiresAt.toISOString() },
       ip: req.ip,
     });
 
-    return { ok: true, impersonating: { id: target.id, email: target.email, firmId: target.firmId } };
+    return {
+      ok: true,
+      impersonating: { id: target.id, email: target.email, firmId: target.firmId },
+      expiresAt: expiresAt.toISOString(),
+    };
   });
 
   // ─────────────────────────── Contact inbox ───────────────────────────

apps/api/src/routes/auth.ts

@@ -24,6 +24,7 @@ import {
 } from '../lib/email';
 import { env } from '../env';
 import { verifyTurnstile } from '../lib/turnstile';
+import { logAudit } from '../lib/audit';
 
 const signupBody = z.object({
   email: z.string().email().max(254).toLowerCase().trim(),
@@ -143,6 +144,7 @@ export async function authRoutes(app: FastifyInstance) {
         isSuperadmin,
         isSuspended: user.isSuspended,
         emailVerified: Boolean(user.emailVerifiedAt),
+        impersonatedBy: null,
       },
     });
   });
@@ -208,6 +210,7 @@ export async function authRoutes(app: FastifyInstance) {
         isSuperadmin,
         isSuspended: user.isSuspended,
         emailVerified: Boolean(user.emailVerifiedAt),
+        impersonatedBy: null,
       },
     };
   });
@@ -220,6 +223,28 @@ export async function authRoutes(app: FastifyInstance) {
     return { ok: true };
   });
 
+  // Leave an impersonated session. Kills the borrowed session and clears cookies; the admin
+  // signs back in as themselves. Deliberately NOT under adminRoutes — requireSuperadmin rejects
+  // impersonated sessions, so a stop route there could never be reached.
+  app.post('/api/auth/end-impersonation', async (req, reply) => {
+    if (!req.user) return reply.code(401).send({ error: 'unauthorized' });
+    if (!req.user.impersonatedBy) return reply.code(409).send({ error: 'not_impersonating' });
+
+    await logAudit({
+      userId: req.user.impersonatedBy,
+      firmId: req.user.firmId,
+      action: 'admin.impersonate.end',
+      meta: { targetUserId: req.user.id, targetEmail: req.user.email },
+      ip: req.ip,
+    });
+
+    const token = req.cookies?.[SESSION_COOKIE];
+    if (token) await destroySession(token);
+    app.clearSessionCookie(reply);
+    app.clearCsrfCookie(reply);
+    return { ok: true };
+  });
+
   app.get('/api/auth/me', async (req, reply) => {
     if (!req.user) return reply.code(401).send({ error: 'unauthorized' });
     if (req.user.isSuspended) return reply.code(403).send({ error: 'account_suspended' });

apps/api/src/routes/billing.ts

@@ -8,6 +8,10 @@ import { getStripe, getPlanConfig, stripeIsConfigured } from '../lib/stripe';
 export async function billingRoutes(app: FastifyInstance) {
   app.addHook('preHandler', app.requireFirm);
 
+  // Reading plan state is fine for anyone in the firm; spending money or opening the Stripe
+  // portal (which exposes payment methods and invoice history) is the owner's call alone.
+  const ownerOnly = { preHandler: app.requireRole('owner') };
+
   // Status — what does the UI need to show? Configured at all? Current plan? Has subscription?
   app.get('/api/billing/status', async (req) => {
     const firmId = req.user!.firmId!;
@@ -21,7 +25,7 @@ export async function billingRoutes(app: FastifyInstance) {
   });
 
   // Create a Checkout Session — returns the URL to redirect the user to.
-  app.post('/api/billing/checkout', async (req, reply) => {
+  app.post('/api/billing/checkout', ownerOnly, async (req, reply) => {
     const parsed = z
       .object({ plan: z.enum(['pro', 'lifetime']) })
       .safeParse(req.body);
@@ -69,7 +73,7 @@ export async function billingRoutes(app: FastifyInstance) {
   });
 
   // Customer Portal — for managing the subscription, updating payment method, viewing invoices.
-  app.post('/api/billing/portal', async (req, reply) => {
+  app.post('/api/billing/portal', ownerOnly, async (req, reply) => {
     if (!stripeIsConfigured()) return reply.code(503).send({ error: 'stripe_not_configured' });
 
     const firmId = req.user!.firmId!;

apps/api/src/routes/cases.ts

@@ -148,6 +148,28 @@ export async function casesRoutes(app: FastifyInstance) {
       return reply.code(400).send({ error: 'invalid_client' });
     }
 
+    // Re-opening counts against the active-case quota exactly like creating one. Without this,
+    // a firm could create cases as 'closed' (unmetered) and flip them to 'open' afterwards,
+    // walking straight past the plan's activeCases cap.
+    if (body.status === 'open') {
+      const [current] = await getDb()
+        .select({ status: cases.status })
+        .from(cases)
+        .where(and(eq(cases.id, id), eq(cases.firmId, firmId)))
+        .limit(1);
+      if (!current) return reply.code(404).send({ error: 'not_found' });
+      if (current.status !== 'open') {
+        const firm = await loadFirm(firmId);
+        if (!firm) return reply.code(403).send({ error: 'firm_missing' });
+        try {
+          await assertCanCreateCase(firmId, firm.plan);
+        } catch (e) {
+          if (e instanceof PlanLimitError) return reply.code(402).send({ error: e.message, plan: firm.plan });
+          throw e;
+        }
+      }
+    }
+
     const patch: Record<string, unknown> = { updatedAt: new Date() };
     for (const [k, v] of Object.entries(body)) {
       if (v === undefined) continue;

apps/api/src/routes/documents.ts

@@ -8,7 +8,7 @@ import { saveFile, deleteFile, getObjectStream, FileNotFoundError } from '../lib
 import { verifyFileSignature } from '../lib/file-signature';
 import { loadFirm } from '../lib/firm';
 import { PlanLimitError } from '../lib/plan-limits';
-import { assertWithinStorageQuota, incrementStorageUsed } from '../lib/storage-quota';
+import { assertWithinStorageQuota, reserveStorage, releaseStorage } from '../lib/storage-quota';
 
 const ALLOWED_MIME = new Set([
   'application/pdf',
@@ -23,6 +23,24 @@ const ALLOWED_MIME = new Set([
 ]);
 
 const MAX_BYTES = 50 * 1024 * 1024; // 50 MB
+const MAX_NAME_LENGTH = 200;
+
+// The uploaded filename reaches us straight from the client. It ends up in two places that both
+// matter: the object key in Spaces (via its extension) and the document's display name. Strip
+// path separators and control characters, and bound the length, so neither can be steered.
+function safeDisplayName(filename: string): string {
+  const base = path.basename(filename.replace(/\\/g, '/'));
+  // eslint-disable-next-line no-control-regex
+  const cleaned = base.replace(/[\u0000-\u001f\u007f]/g, '').trim();
+  return (cleaned || 'document').slice(0, MAX_NAME_LENGTH);
+}
+
+// Only a short, conventional extension is carried into the storage key — anything unusual is
+// dropped rather than concatenated. The key stays `${firmId}/${caseId}/${uuid}${ext}`.
+function safeExtension(filename: string): string {
+  const ext = path.extname(safeDisplayName(filename)).toLowerCase();
+  return /^\.[a-z0-9]{1,8}$/.test(ext) ? ext : '';
+}
 
 export async function documentsRoutes(app: FastifyInstance) {
   app.addHook('preHandler', app.requireFirm);
@@ -79,10 +97,10 @@ export async function documentsRoutes(app: FastifyInstance) {
 
     const size = buf.length;
 
-    // Enforce the firm's per-plan storage quota before committing the object to storage,
-    // so a rejected upload leaves no orphaned Spaces object and never touches the counter.
     const firm = await loadFirm(firmId);
     if (!firm) return reply.code(403).send({ error: 'firm_missing' });
+
+    // Cheap pre-check so an obviously oversized upload is rejected before we do any more work.
     try {
       await assertWithinStorageQuota(firmId, firm.plan, size);
     } catch (e) {
@@ -92,32 +110,45 @@ export async function documentsRoutes(app: FastifyInstance) {
       throw e;
     }
 
+    // Authoritative claim: one conditional UPDATE that both checks the cap and books the bytes,
+    // so simultaneous uploads can't each pass the check and collectively blow past the quota.
+    // Reserved BEFORE the object is written, and released again on any failure below.
+    if (!(await reserveStorage(firmId, firm.plan, size))) {
+      return reply.code(402).send({ error: 'plan_limit_storageBytes', plan: firm.plan });
+    }
+
     const docId = randomUUID();
-    const ext = path.extname(data.filename);
+    // Derive the extension from the *sanitised* filename: `data.filename` is attacker-controlled,
+    // and the extension is concatenated straight into the object key.
+    const ext = safeExtension(data.filename);
     const storageKey = `${firmId}/${caseId}/${docId}${ext}`;
 
-    await saveFile(storageKey, buf, data.mimetype);
+    try {
+      await saveFile(storageKey, buf, data.mimetype);
+    } catch (err) {
+      await releaseStorage(firmId, size);
+      throw err;
+    }
 
     const [doc] = await db.insert(documents).values({
       id: docId,
       firmId,
       caseId,
       uploadedBy: userId,
-      name: data.filename,
+      name: safeDisplayName(data.filename),
       storageKey,
       mimeType: data.mimetype,
       sizeBytes: size,
     }).returning();
 
     if (!doc) {
-      // Row insert failed after the file was written — clean up the orphaned object.
+      // Row insert failed after the file was written — clean up the orphaned object and hand
+      // the reserved bytes back, or the firm permanently loses that much of its quota.
       await deleteFile(storageKey).catch(() => {});
+      await releaseStorage(firmId, size);
       return reply.code(500).send({ error: 'upload_failed' });
     }
 
-    // Row committed — account the stored bytes against the firm's quota.
-    await incrementStorageUsed(firmId, size);
-
     return reply.code(201).send({
       id: doc.id,
       name: doc.name,
@@ -171,7 +202,7 @@ export async function documentsRoutes(app: FastifyInstance) {
     }
 
     // Row is gone — reclaim its bytes from the firm's tracked usage (clamped at 0).
-    await incrementStorageUsed(firmId, -doc.sizeBytes);
+    await releaseStorage(firmId, doc.sizeBytes);
 
     return reply.code(204).send();
   });

apps/api/src/routes/webhooks-stripe.ts

@@ -71,11 +71,29 @@ export async function stripeWebhookRoute(app: FastifyInstance) {
 
 async function handleEvent(event: Stripe.Event, app: FastifyInstance) {
   switch (event.type) {
-    case 'checkout.session.completed': {
+    // `completed` fires as soon as Checkout is finished — which for delayed-notification payment
+    // methods (ACH debit, bank transfers, some wallets) happens BEFORE any money moves, with
+    // payment_status 'unpaid'. Granting the plan here unconditionally would hand out a lifetime
+    // licence for an initiated-but-unsettled payment. Only 'paid' (or 'no_payment_required', e.g.
+    // a 100% coupon) grants; unpaid sessions wait for async_payment_succeeded below.
+    case 'checkout.session.completed':
+    case 'checkout.session.async_payment_succeeded': {
       const session = event.data.object as Stripe.Checkout.Session;
       const firmId = session.client_reference_id ?? (session.metadata?.firmId as string | undefined);
       const planFromMeta = (session.metadata?.plan ?? '') as 'pro' | 'lifetime' | '';
-      if (!firmId) return app.log.warn({ session: session.id }, 'checkout.session.completed without firmId');
+      if (!firmId) return app.log.warn({ session: session.id }, `${event.type} without firmId`);
+
+      if (session.payment_status !== 'paid' && session.payment_status !== 'no_payment_required') {
+        app.log.info(
+          { session: session.id, firmId, paymentStatus: session.payment_status },
+          'checkout session not settled — plan withheld until payment succeeds',
+        );
+        // Still capture the customer id so the billing portal works while payment settles.
+        const pendingCustomer =
+          typeof session.customer === 'string' ? session.customer : session.customer?.id ?? null;
+        if (pendingCustomer) await attachCustomerId(firmId, pendingCustomer);
+        break;
+      }
 
       // Determine plan from session.mode if metadata didn't pin it.
       const plan: 'pro' | 'lifetime' = planFromMeta || (session.mode === 'subscription' ? 'pro' : 'lifetime');
@@ -93,14 +111,33 @@ async function handleEvent(event: Stripe.Event, app: FastifyInstance) {
       break;
     }
 
+    // The delayed payment ultimately failed, or the customer abandoned Checkout after a session
+    // was created. Nothing was granted (the guard above withheld it), so this is log-only —
+    // but it must be handled explicitly so a future change can't silently leave a plan applied.
+    case 'checkout.session.async_payment_failed':
+    case 'checkout.session.expired': {
+      const session = event.data.object as Stripe.Checkout.Session;
+      const firmId = session.client_reference_id ?? (session.metadata?.firmId as string | undefined);
+      app.log.warn(
+        { session: session.id, firmId, type: event.type },
+        'checkout session did not result in payment',
+      );
+      break;
+    }
+
     case 'customer.subscription.updated':
     case 'customer.subscription.created': {
       const sub = event.data.object as Stripe.Subscription;
       const firmId = (sub.metadata?.firmId as string | undefined) ?? null;
       if (!firmId) return;
-      // Only flip to 'pro' while the subscription is paying.
-      const active = ['active', 'trialing', 'past_due'].includes(sub.status);
-      if (active) await applyPlan(firmId, 'pro', { subscriptionId: sub.id });
+      // Only 'active'/'trialing' are paying states. 'past_due' means a renewal already failed —
+      // Stripe is still retrying, so we neither upgrade on it nor downgrade an existing Pro firm
+      // mid-retry; 'unpaid'/'canceled'/'incomplete_expired' are terminal and drop to starter.
+      if (sub.status === 'active' || sub.status === 'trialing') {
+        await applyPlan(firmId, 'pro', { subscriptionId: sub.id });
+      } else if (sub.status === 'unpaid' || sub.status === 'incomplete_expired') {
+        await applyPlan(firmId, 'starter', { subscriptionId: null });
+      }
       break;
     }
 
@@ -152,6 +189,15 @@ async function handleEvent(event: Stripe.Event, app: FastifyInstance) {
   }
 }
 
+// Records the Stripe customer without touching the plan — used when a Checkout session exists
+// but hasn't been paid yet, so the billing portal is reachable while payment settles.
+async function attachCustomerId(firmId: string, customerId: string) {
+  await getDb()
+    .update(firms)
+    .set({ stripeCustomerId: customerId, updatedAt: new Date() })
+    .where(eq(firms.id, firmId));
+}
+
 async function applyPlan(
   firmId: string,
   plan: 'starter' | 'pro' | 'lifetime',

apps/api/src/server.ts

@@ -43,7 +43,11 @@ export async function buildServer() {
     // Trust exactly ONE proxy hop (the Plesk/nginx reverse proxy in front of Passenger).
     // `true` would trust the entire X-Forwarded-For chain, letting any client spoof req.ip and
     // evade the IP-keyed rate limits (including auth brute-force protection).
-    trustProxy: 1,
+    //
+    // Expressed as proxy-addr's predicate form — `hop < 1` trusts only the address closest to
+    // this server and nothing beyond it, which is exactly what the numeric `1` used to mean.
+    // Fastify's types no longer accept the number, and the predicate is unambiguous anyway.
+    trustProxy: (_address: string, hop: number) => hop < 1,
     bodyLimit: 5 * 1024 * 1024,
   });
 
@@ -60,6 +64,15 @@ export async function buildServer() {
       // Let @fastify/rate-limit handle its own response shape.
       return reply.send(err);
     }
+    // Deliberate 4xx thrown from inside a plugin — e.g. @fastify/send raising a 403 Forbidden
+    // for a path that escapes the static root, or a 404 for a missing file. These are the
+    // library working correctly; reporting them as 500 internal_error both lies to the client
+    // and floods Sentry with non-errors, which buries real incidents.
+    const status = (err as { statusCode?: number }).statusCode;
+    if (typeof status === 'number' && status >= 400 && status < 500) {
+      req.log.info({ err, status }, 'client error');
+      return reply.code(status).send({ error: (err as { code?: string }).code ?? 'request_failed' });
+    }
     req.log.error({ err }, 'unhandled error');
     captureError(err, { url: req.url, method: req.method, userId: req.user?.id });
     return reply.code(500).send({ error: 'internal_error' });

apps/api/test/checkout-grant.test.ts

@@ -0,0 +1,66 @@
+import { describe, it, expect } from 'vitest';
+
+// Mirrors the grant decision in src/routes/webhooks-stripe.ts. `checkout.session.completed` fires
+// as soon as Checkout finishes, which for delayed-notification payment methods (ACH debit, bank
+// transfer, some wallets) happens BEFORE any money moves — payment_status 'unpaid'. Granting on
+// the event alone hands out a paid plan for an unsettled payment.
+type PaymentStatus = 'paid' | 'unpaid' | 'no_payment_required';
+
+function shouldGrantPlan(paymentStatus: PaymentStatus): boolean {
+  return paymentStatus === 'paid' || paymentStatus === 'no_payment_required';
+}
+
+// Mirrors the subscription-status branch in the same file.
+type SubStatus =
+  | 'active'
+  | 'trialing'
+  | 'past_due'
+  | 'unpaid'
+  | 'canceled'
+  | 'incomplete'
+  | 'incomplete_expired';
+
+function planForSubscription(status: SubStatus): 'pro' | 'starter' | null {
+  if (status === 'active' || status === 'trialing') return 'pro';
+  if (status === 'unpaid' || status === 'incomplete_expired') return 'starter';
+  return null; // leave the current plan untouched
+}
+
+describe('checkout grant decision', () => {
+  it('grants on a settled payment', () => {
+    expect(shouldGrantPlan('paid')).toBe(true);
+  });
+
+  it('grants when no payment was required (e.g. a 100% coupon)', () => {
+    expect(shouldGrantPlan('no_payment_required')).toBe(true);
+  });
+
+  it('withholds the plan while the payment is unsettled', () => {
+    // The regression this guards: a delayed-payment method completing Checkout unpaid used to
+    // grant 'lifetime' outright.
+    expect(shouldGrantPlan('unpaid')).toBe(false);
+  });
+});
+
+describe('subscription status mapping', () => {
+  it('treats only active and trialing as paying', () => {
+    expect(planForSubscription('active')).toBe('pro');
+    expect(planForSubscription('trialing')).toBe('pro');
+  });
+
+  it('does not upgrade on past_due, and does not downgrade mid-retry either', () => {
+    // Stripe is still retrying the charge — flipping the plan in either direction here would
+    // either hand out Pro for a failed renewal or cut off a customer whose retry succeeds.
+    expect(planForSubscription('past_due')).toBeNull();
+  });
+
+  it('drops to starter on terminal non-payment states', () => {
+    expect(planForSubscription('unpaid')).toBe('starter');
+    expect(planForSubscription('incomplete_expired')).toBe('starter');
+  });
+
+  it('leaves the plan alone for states that carry no payment signal', () => {
+    expect(planForSubscription('incomplete')).toBeNull();
+    expect(planForSubscription('canceled')).toBeNull(); // handled by subscription.deleted instead
+  });
+});

apps/api/test/safe-next.test.ts

@@ -0,0 +1,79 @@
+import { describe, it, expect } from 'vitest';
+
+// Mirrors safeNext() in apps/web/src/pages/LoginPage.tsx — the post-login redirect guard. `next`
+// rides in the login URL and is therefore attacker-supplied, so an open redirect here lands a
+// freshly authenticated user on a phishing page. Lives in the API suite because the web workspace
+// has no test runner configured; keep the two copies in step.
+function safeNext(raw: string | null): string {
+  const FALLBACK = '/app';
+  if (!raw) return FALLBACK;
+  const cleaned = raw.replace(/[\t\n\r]/g, '');
+  if (!cleaned.startsWith('/') || cleaned.startsWith('//')) return FALLBACK;
+  try {
+    const probe = 'https://redirect-guard.invalid';
+    const url = new URL(cleaned, probe);
+    if (url.origin !== probe) return FALLBACK;
+    return `${url.pathname}${url.search}${url.hash}`;
+  } catch {
+    return FALLBACK;
+  }
+}
+
+const PROBE = 'https://redirect-guard.invalid';
+const staysInternal = (value: string) => new URL(value, PROBE).origin === PROBE;
+
+describe('safeNext — post-login open-redirect guard', () => {
+  it('passes through legitimate internal destinations', () => {
+    expect(safeNext('/app/cases')).toBe('/app/cases');
+    expect(safeNext('/app?tab=open#row')).toBe('/app?tab=open#row');
+    expect(safeNext('/admin')).toBe('/admin');
+  });
+
+  it('falls back when nothing was requested', () => {
+    expect(safeNext(null)).toBe('/app');
+    expect(safeNext('')).toBe('/app');
+  });
+
+  it('rejects absolute URLs to another origin', () => {
+    expect(safeNext('https://evil.com')).toBe('/app');
+    expect(safeNext('http://evil.com/path')).toBe('/app');
+  });
+
+  it('rejects protocol-relative URLs', () => {
+    expect(safeNext('//evil.com')).toBe('/app');
+    expect(safeNext('////evil.com')).toBe('/app');
+  });
+
+  it('rejects backslash variants that browsers normalise into an authority', () => {
+    // URL parsing rewrites \ as / for special schemes, so each of these would otherwise become
+    // protocol-relative and point off-origin. The origin check is what catches them.
+    for (const attack of ['/\\\\evil.com', '/\\/evil.com', '\\\\evil.com', '/\\\\\\evil.com']) {
+      expect(safeNext(attack)).toBe('/app');
+    }
+  });
+
+  it('rejects schemes smuggled past the leading-slash check with whitespace', () => {
+    expect(safeNext('\tjavascript:alert(1)')).toBe('/app');
+    expect(safeNext('\njavascript:alert(1)')).toBe('/app');
+    expect(safeNext('javascript:alert(1)')).toBe('/app');
+    expect(safeNext('data:text/html,<script>alert(1)</script>')).toBe('/app');
+  });
+
+  it('never returns a value that resolves off-origin', () => {
+    const attacks = [
+      '//evil.com',
+      '/\\evil.com',
+      '/\\\\evil.com',
+      'https://evil.com',
+      '\tjavascript:alert(1)',
+      '////evil.com',
+      '/%5cevil.com',
+      '/%2f%2fevil.com',
+      '/\t/evil.com',
+      'data:text/html,<script>alert(1)</script>',
+    ];
+    for (const attack of attacks) {
+      expect(staysInternal(safeNext(attack))).toBe(true);
+    }
+  });
+});

apps/web/package.json

@@ -10,7 +10,7 @@
     "typecheck": "tsc -b --noEmit"
   },
   "dependencies": {
-    "@sentry/react": "^8.45.0",
+    "@sentry/react": "^10.71.0",
     "@tanstack/react-query": "^5.62.0",
     "class-variance-authority": "^0.7.1",
     "clsx": "^2.1.1",
@@ -19,7 +19,7 @@
     "react": "^18.3.1",
     "react-dom": "^18.3.1",
     "react-hook-form": "^7.53.2",
-    "react-router-dom": "^6.28.0",
+    "react-router-dom": "^6.30.6",
     "recharts": "^2.13.3",
     "tailwind-merge": "^2.5.5",
     "zod": "^3.23.8"
@@ -30,7 +30,7 @@
     "@types/react-dom": "^18.3.1",
     "@vitejs/plugin-react": "^4.3.4",
     "autoprefixer": "^10.4.20",
-    "postcss": "^8.4.49",
+    "postcss": "^8.5.26",
     "tailwindcss": "^3.4.15",
     "tailwindcss-animate": "^1.0.7",
     "typescript": "^5.6.3",

apps/web/src/components/app/AppLayout.tsx

@@ -3,6 +3,7 @@ import { useMe } from '@/hooks/useAuth';
 import { Sidebar } from './Sidebar';
 import { Topbar } from './Topbar';
 import { VerifyEmailBanner } from './VerifyEmailBanner';
+import { ImpersonationBanner } from './ImpersonationBanner';
 
 export function AppLayout() {
   const me = useMe();
@@ -19,6 +20,7 @@ export function AppLayout() {
     <div className="min-h-screen flex bg-ink-50">
       <Sidebar />
       <div className="flex-1 flex flex-col min-w-0">
+        <ImpersonationBanner />
         <Topbar />
         <VerifyEmailBanner />
         <main className="flex-1 overflow-y-auto">

apps/web/src/components/app/ImpersonationBanner.tsx

@@ -0,0 +1,36 @@
+import { UserCog } from 'lucide-react';
+import { useEndImpersonation, useMe } from '@/hooks/useAuth';
+
+/**
+ * Persistent, non-dismissible warning shown whenever the current session was opened by a
+ * superadmin impersonating this user. It must not be dismissible: an admin who forgets they are
+ * inside a customer's firm can take real actions against real client data, and every one of
+ * those actions is recorded against the customer's name.
+ */
+export function ImpersonationBanner() {
+  const me = useMe();
+  const endImpersonation = useEndImpersonation();
+
+  if (!me.data?.impersonatedBy) return null;
+
+  return (
+    <div
+      role="alert"
+      className="border-b border-rose-300 bg-rose-600 px-4 py-2.5 flex flex-wrap items-center gap-x-3 gap-y-1 text-sm text-white"
+    >
+      <UserCog className="h-4 w-4 flex-none" aria-hidden="true" />
+      <span>
+        Support session — you are acting as <strong>{me.data.email}</strong>. Everything you do is
+        recorded against this account and expires within the hour.
+      </span>
+      <button
+        type="button"
+        onClick={() => endImpersonation.mutate()}
+        disabled={endImpersonation.isPending}
+        className="ml-auto rounded-md bg-white/15 px-3 py-1 font-medium underline-offset-2 hover:bg-white/25 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-white disabled:opacity-60"
+      >
+        {endImpersonation.isPending ? 'Ending…' : 'End session'}
+      </button>
+    </div>
+  );
+}

apps/web/src/hooks/useAuth.ts

@@ -10,6 +10,8 @@ export interface AuthUser {
   isSuperadmin?: boolean;
   isSuspended?: boolean;
   emailVerified?: boolean;
+  /** Superadmin id when this session was opened by admin impersonation, else null. */
+  impersonatedBy?: string | null;
 }
 
 interface MeResponse {
@@ -69,3 +71,20 @@ export function useLogout() {
     onSuccess: () => qc.setQueryData(ME_KEY, null),
   });
 }
+
+/**
+ * Leave an impersonated session. The server destroys the borrowed session outright, so there is
+ * no session left to return to — the admin lands on the login page and signs in as themselves.
+ */
+export function useEndImpersonation() {
+  const qc = useQueryClient();
+  return useMutation<void, ApiError>({
+    mutationFn: async () => {
+      await api.post('/api/auth/end-impersonation');
+    },
+    onSuccess: () => {
+      qc.setQueryData(ME_KEY, null);
+      qc.clear();
+    },
+  });
+}

apps/web/src/pages/LoginPage.tsx

@@ -15,14 +15,25 @@ const schema = z.object({
 
 type FormValues = z.infer<typeof schema>;
 
-// Guard against open-redirects: only accept same-origin internal paths like
-// "/app" or "/app/cases". Reject protocol-relative ("//evil.com"), backslash
-// tricks ("/\\evil.com"), and absolute URLs ("https://evil.com").
+// Guard against open-redirects. `next` is attacker-supplied (it rides in the login URL), so it
+// is resolved against a throwaway origin and accepted only if it stays on that origin. Parsing
+// rather than prefix-matching means encoded, backslash, and protocol-relative forms all
+// normalise before the check, and the result never depends on how the router treats the string.
 function safeNext(raw: string | null): string {
-  if (!raw || !raw.startsWith('/') || raw.startsWith('//') || raw.startsWith('/\\') || raw.includes('://')) {
-    return '/app';
+  const FALLBACK = '/app';
+  if (!raw) return FALLBACK;
+  // Browsers strip tabs/newlines from URLs before parsing; do the same so they can't be used
+  // to smuggle a scheme past the checks below.
+  const cleaned = raw.replace(/[\t\n\r]/g, '');
+  if (!cleaned.startsWith('/') || cleaned.startsWith('//')) return FALLBACK;
+  try {
+    const probe = 'https://redirect-guard.invalid';
+    const url = new URL(cleaned, probe);
+    if (url.origin !== probe) return FALLBACK;
+    return `${url.pathname}${url.search}${url.hash}`;
+  } catch {
+    return FALLBACK;
   }
-  return raw;
 }
 
 const ERROR_COPY: Record<string, string> = {

package.json

@@ -26,6 +26,11 @@
     "test": "npm run test --workspaces --if-present"
   },
   "devDependencies": {
-    "typescript": "^5.6.3"
+    "typescript": "^5.6.3",
+    "drizzle-orm": "^0.45.2"
+  },
+  "overrides": {
+    "find-my-way": "^9.9.0",
+    "fast-uri": "^3.1.5"
   }
 }

packages/db/migrations/0003_illegal_leper_queen.sql

@@ -0,0 +1,2 @@
+ALTER TABLE "sessions" ADD COLUMN "impersonated_by" uuid;--> statement-breakpoint
+ALTER TABLE "sessions" ADD CONSTRAINT "sessions_impersonated_by_users_id_fk" FOREIGN KEY ("impersonated_by") REFERENCES "public"."users"("id") ON DELETE set null ON UPDATE no action;

packages/db/migrations/meta/_journal.json

@@ -22,6 +22,13 @@
       "when": 1784308478719,
       "tag": "0002_daily_chronomancer",
       "breakpoints": true
+    },
+    {
+      "idx": 3,
+      "version": "7",
+      "when": 1787753919185,
+      "tag": "0003_illegal_leper_queen",
+      "breakpoints": true
     }
   ]
 }

packages/db/src/schema/auth.ts

@@ -40,6 +40,10 @@ export const sessions = pgTable(
       .notNull()
       .references(() => users.id, { onDelete: 'cascade' }),
     expiresAt: timestamp('expires_at', { withTimezone: true }).notNull(),
+    // Set when a superadmin opened this session via /api/admin/users/:id/impersonate. Non-null
+    // means every action on this session is really that admin acting as `userId` — the API
+    // stamps it onto audit rows and the UI shows a persistent impersonation banner.
+    impersonatedBy: uuid('impersonated_by').references(() => users.id, { onDelete: 'set null' }),
     ip: inet('ip'),
     userAgent: text('user_agent'),
     lastSeenAt: timestamp('last_seen_at', { withTimezone: true }).notNull().defaultNow(),