fix(backend): contain async route errors in the un-modularized routes

Express 4 does not forward async handler rejections, and the routes that open-legal-products#295
left outside modules/ were unguarded — a throw in any of them meant an
unhandled rejection and a socket held open until client timeout. Only the
workflows router defended itself. Its asyncRoute wrapper is now a shared
middleware (middleware/asyncRoute.ts) applied to audit, quickActions,
sourceDocuments, wordChat and workflowAddons, so a rejection reaches
app.ts's handleUnhandledError instead of nothing.

routerErrorHandler now only attributes the failure to a router in the log and
delegates the response to handleUnhandledError. Answering with a
router-specific 500 body would have contradicted the internal-error contract
main established: body-parser's 400/413 keep their own status and code,
everything else is the opaque {code:"internal_error"} body.

Also fixed in those route files, where errors were being silently converted to
success or not-found:
- quickActions: DELETE returned 204 with no row matched (now selects the
  deleted ids and 404s when none); DB failures on the workflow lookup read as
  404 (now 500, via the now-exported workflows.service.resolveWorkflowAccess
  instead of a drifted private copy).
- workflowAddons: the import response hand-rebuilt the workflow shape and had
  drifted from GET /workflows/:id on four fields — now uses the exported
  withDatabaseWorkflow; a failed add-on lookup answered 404 (now 500).
- audit: lib/auditExport's private project-access helper duplicated
  lib/access.listAccessibleProjectIds and had drifted from it (it re-counted
  the caller's own shared projects and used a different `contains` encoding).
- sourceDocuments: a missing or rejected CourtListener token answered 502,
  sending the user to check a service that was fine; credential failures now
  answer 400 with a fixed message that never echoes the upstream body.

The history page's surface filter learns the "word" label, so the durable Word
chat audit rows open-legal-products#294 introduced (surface "word") are filterable rather than
showing a raw key.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: amal66

backend/src/app.ts

@@ -300,4 +300,10 @@ app.get("/manifest-signing-key", (_req, res) => {
   }
 });
 
+// Terminal error handler. Routers that mount their own error middleware answer
+// first; anything they re-raise (or that escapes a router without one) lands
+// here instead of Express's default handler, which would leak the stack trace
+// in a non-production environment. Must stay last: Express 4 only reaches an
+// error handler registered after the middleware that failed, and asyncRoute is
+// what gets rejected promises here at all.
 app.use(handleUnhandledError);

backend/src/lib/__tests__/auditExport.test.ts

@@ -18,7 +18,12 @@ function makeDb(events: Record<string, unknown>[], error?: { message: string })
             ilike: () => b,
             gte: () => b,
             lte: () => b,
+            // listAccessibleProjectIds (lib/access) scopes the shared-project
+            // lookup with .filter("shared_with","cs",...).neq(...); .contains
+            // stays for any caller still using the older encoding.
             contains: () => b,
+            filter: () => b,
+            neq: () => b,
             order: () => b,
             range: (from: number, to: number) => {
                 ranges.push([from, to]);
@@ -106,7 +111,12 @@ function makeProfileDb(
             ilike: () => b,
             gte: () => b,
             lte: () => b,
+            // listAccessibleProjectIds (lib/access) scopes the shared-project
+            // lookup with .filter("shared_with","cs",...).neq(...); .contains
+            // stays for any caller still using the older encoding.
             contains: () => b,
+            filter: () => b,
+            neq: () => b,
             order: () => b,
             in: () => {
                 profilesQueried = true;

backend/src/lib/auditExport.ts

@@ -6,6 +6,7 @@
 // router would drag in the whole HTTP surface.
 
 import type { createServerSupabase } from "./supabase";
+import { listAccessibleProjectIds } from "./access";
 import { normalizeDisplayName } from "./userLookup";
 
 type Db = ReturnType<typeof createServerSupabase>;
@@ -18,25 +19,6 @@ export const AUDIT_EXPORT_LIMIT = 2000;
 const MAX_PAGE = 100_000;
 const DATE_RE = /^\d{4}-\d{2}-\d{2}$/;
 
-export async function accessibleProjectIds(
-    db: Db,
-    userId: string,
-    email: string | undefined,
-): Promise<string[]> {
-    const ids = new Set<string>();
-    const own = await db.from("projects").select("id").eq("user_id", userId);
-    for (const row of (own.data ?? []) as { id: string }[]) ids.add(row.id);
-    if (email) {
-        const shared = await db
-            .from("projects")
-            .select("id")
-            .contains("shared_with", [email.trim().toLowerCase()]);
-        for (const row of (shared.data ?? []) as { id: string }[])
-            ids.add(row.id);
-    }
-    return [...ids];
-}
-
 export type AuditQuery = {
     q?: string;
     action?: string;
@@ -129,7 +111,11 @@ export async function queryEvents(
     q: AuditQuery,
     resolveDisplayNames = true,
 ) {
-    const projectIds = await accessibleProjectIds(db, userId, email);
+    // Shared with the chat/document listings: one definition of "projects this
+    // user can see" for everything that scopes a collection query. The private
+    // copy that used to live here had already drifted (it re-counted the
+    // caller's own shared projects and used a different `contains` encoding).
+    const projectIds = await listAccessibleProjectIds(userId, email, db);
     let query = db
         .from("audit_events")
         .select(

backend/src/middleware/asyncRoute.ts

@@ -0,0 +1,37 @@
+// Shared plumbing for async Express handlers.
+//
+// Express 4 does not understand promises: an `async` handler that rejects is
+// invisible to the router, so the request hangs until the client or the proxy
+// times out and nothing is logged. `asyncRoute` forwards the rejection to
+// `next(err)`, where an error middleware turns it into a response.
+
+import type { NextFunction, Request, Response } from "express";
+import { handleUnhandledError } from "./internalErrorResponse";
+
+export type AsyncRoute = (req: Request, res: Response) => Promise<unknown>;
+
+export function asyncRoute(handler: AsyncRoute) {
+  return (req: Request, res: Response, next: NextFunction) => {
+    void handler(req, res).catch(next);
+  };
+}
+
+// Builds a router-scoped error middleware. Express 4 identifies error
+// middleware purely by arity, so all four parameters must stay declared even
+// when a router does not use them.
+//
+// Its only job is to attribute the failure to a router in the logs; the
+// response itself is delegated to the same app-level boundary, so a rejection
+// caught here is indistinguishable on the wire from one that escaped to
+// app.ts — body-parser's 400/413 keep their own status and code, everything
+// else becomes the opaque internal_error body. A response that already started
+// streaming (SSE, a file download) is handed on instead: its status line is
+// long gone, so the only honest thing left is to let Express destroy the
+// connection.
+export function routerErrorHandler(tag: string) {
+  return (err: unknown, req: Request, res: Response, next: NextFunction) => {
+    if (res.headersSent) return next(err);
+    console.error(`${tag} unhandled route error`, err);
+    handleUnhandledError(err, req, res, next);
+  };
+}

backend/src/modules/workflows/workflows.routes.ts

@@ -2,8 +2,9 @@
 // call the service layer in workflows.service.ts, and map its typed results
 // onto status codes and JSON responses.
 
-import { Router, type NextFunction, type Request, type Response } from "express";
+import { Router, type Request, type Response } from "express";
 import { requireAuth } from "../../middleware/auth";
+import { asyncRoute, routerErrorHandler } from "../../middleware/asyncRoute";
 import { createServerSupabase } from "../../lib/supabase";
 import { parsePaginationQuery } from "../../lib/pagination";
 import { normalizeSearchTerm } from "../../lib/search";
@@ -45,14 +46,6 @@ export const workflowsRouter = Router();
 
 type Db = ReturnType<typeof createServerSupabase>;
 
-type AsyncRoute = (req: Request, res: Response) => Promise<unknown>;
-
-function asyncRoute(handler: AsyncRoute) {
-  return (req: Request, res: Response, next: NextFunction) => {
-    void handler(req, res).catch(next);
-  };
-}
-
 // Installs missing default workflows before any listing; a failure here is
 // terminal for the request (500 with the opaque internal-error body).
 async function ensureDefaultsForRequest(
@@ -513,9 +506,5 @@ workflowsRouter.post("/:workflowId/share", requireAuth, asyncRoute(async (req, r
 }));
 
 workflowsRouter.use(
-  (err: unknown, _req: Request, res: Response, next: NextFunction) => {
-    if (res.headersSent) return next(err);
-    console.error("[workflows] unhandled route error", err);
-    res.status(500).json({ detail: "Failed to process workflow request" });
-  },
+  routerErrorHandler("[workflows]"),
 );

backend/src/modules/workflows/workflows.service.ts

@@ -211,7 +211,10 @@ function metadataFromWorkflowRecord(
   };
 }
 
-function withDatabaseWorkflow(workflow: WorkflowRecord) {
+// Exported for the workflow-addons import route, whose 201 body must match
+// GET /workflows/:id. Hand-rebuilding that shape there had already drifted on
+// metadata.name, contributors, version and is_default.
+export function withDatabaseWorkflow(workflow: WorkflowRecord) {
   const {
     title: _title,
     type: _type,
@@ -309,7 +312,11 @@ function contributorFromName(name: unknown): WorkflowContributor {
   };
 }
 
-async function resolveWorkflowAccess(
+// Exported for the quick-actions route, which links a quick action to a
+// workflow and has to apply exactly this owner-or-share rule. Its private copy
+// had already drifted: it treated any lookup failure as "not found" and never
+// consulted workflow_shares.allow_edit.
+export async function resolveWorkflowAccess(
   db: Db,
   workflowId: string,
   userId: string,

backend/src/routes/__tests__/audit.test.ts

@@ -4,8 +4,8 @@ import {
     escapeLikePattern,
     parseQuery,
     queryEvents,
-    accessibleProjectIds,
 } from "../audit";
+import { listAccessibleProjectIds } from "../../lib/access";
 
 // ---------------------------------------------------------------------------
 // csvCell — spreadsheet formula-injection escaping (F3)
@@ -116,13 +116,14 @@ describe("parseQuery", () => {
 });
 
 // ---------------------------------------------------------------------------
-// queryEvents / accessibleProjectIds — visibility scoping
+// queryEvents / listAccessibleProjectIds — visibility scoping
 // ---------------------------------------------------------------------------
 
 /**
  * Chainable Supabase mock. `projects` select responses are keyed by whether the
- * query used .eq (owned) or .contains (shared). The audit_events builder
- * records the .or / .eq filter it was given so tests can assert scoping.
+ * query used .eq (owned) or .filter (shared_with contains). The audit_events
+ * builder records the .or / .eq filter it was given so tests can assert
+ * scoping.
  */
 function makeDb(
     owned: string[],
@@ -146,10 +147,11 @@ function makeDb(
                 mode = "owned";
                 return b;
             },
-            contains: () => {
+            filter: () => {
                 mode = "shared";
                 return b;
             },
+            neq: () => b,
             then: (resolve: (v: { data: { id: string }[] }) => unknown) =>
                 Promise.resolve({
                     data: (mode === "owned" ? owned : shared).map((id) => ({
@@ -238,10 +240,10 @@ describe("queryEvents visibility scoping", () => {
     });
 
     it("de-duplicates owned and shared project ids", async () => {
-        const both = await accessibleProjectIds(
-            makeDb(["p1", "p2"], ["p2", "p3"]).db,
+        const both = await listAccessibleProjectIds(
             "u1",
             "u1@example.com",
+            makeDb(["p1", "p2"], ["p2", "p3"]).db,
         );
         expect([...both].sort()).toEqual(["p1", "p2", "p3"]);
     });

backend/src/routes/audit.ts

@@ -4,6 +4,7 @@
 
 import { Router } from "express";
 import { requireAuth, requireMfaIfEnrolled } from "../middleware/auth";
+import { asyncRoute } from "../middleware/asyncRoute";
 import { createServerSupabase } from "../lib/supabase";
 import { sendInternalError } from "../lib/httpError";
 import {
@@ -17,7 +18,6 @@ import {
 // The query/CSV helpers moved to lib/auditExport so the async "audit-csv"
 // export job can reuse them; re-exported here for existing importers.
 export {
-  accessibleProjectIds,
   buildAuditCsv,
   csvCell,
   escapeLikePattern,
@@ -31,52 +31,62 @@ auditRouter.use(requireAuth);
 
 const PAGE_SIZE = 50;
 
-auditRouter.get("/", async (req, res) => {
-  const userId = res.locals.userId as string;
-  const email = res.locals.userEmail as string | undefined;
-  const db = createServerSupabase();
-  const parsed = parseQuery(req.query as Record<string, unknown>, PAGE_SIZE);
-  if (!parsed.ok) return void res.status(400).json({ detail: parsed.error });
-  const q = parsed.query;
-  const { data, error, count } = await queryEvents(db, userId, email, q);
-  if (error) return void sendInternalError(res, error);
-  res.json({
-    events: data ?? [],
-    total: count ?? 0,
-    page: q.page,
-    pageSize: PAGE_SIZE,
-  });
-});
+// asyncRoute: an `async` handler that rejects is invisible to Express 4, so a
+// throw from queryEvents (a network blip against PostgREST, say) would hang the
+// request instead of answering. Forwarding to next() lets app.ts answer 500.
+auditRouter.get(
+  "/",
+  asyncRoute(async (req, res) => {
+    const userId = res.locals.userId as string;
+    const email = res.locals.userEmail as string | undefined;
+    const db = createServerSupabase();
+    const parsed = parseQuery(req.query as Record<string, unknown>, PAGE_SIZE);
+    if (!parsed.ok) return void res.status(400).json({ detail: parsed.error });
+    const q = parsed.query;
+    const { data, error, count } = await queryEvents(db, userId, email, q);
+    if (error) return void sendInternalError(res, error);
+    res.json({
+      events: data ?? [],
+      total: count ?? 0,
+      page: q.page,
+      pageSize: PAGE_SIZE,
+    });
+  }),
+);
 
 // Synchronous CSV export. Still here for curl users and older clients; the
 // frontend goes through the durable "audit-csv" export job instead. Both
 // emit the same bytes because both render through buildAuditCsv.
-auditRouter.get("/export", requireMfaIfEnrolled, async (req, res) => {
-  const userId = res.locals.userId as string;
-  const email = res.locals.userEmail as string | undefined;
-  const db = createServerSupabase();
-  const parsed = parseQuery(
-    req.query as Record<string, unknown>,
-    AUDIT_EXPORT_LIMIT,
-  );
-  if (!parsed.ok) return void res.status(400).json({ detail: parsed.error });
-  let csv: string;
-  try {
-    csv = await buildAuditCsv(db, userId, email, parsed.query);
-  } catch (err) {
-    // buildAuditCsv throws so the async job retries; here the throw becomes
-    // the same generic 500 this route has always sent, never the raw DB
-    // message. Unwrap `cause` so the log still carries the PostgrestError's
-    // code/details/hint rather than only its message.
-    return void sendInternalError(
-      res,
-      err instanceof Error && err.cause ? err.cause : err,
+auditRouter.get(
+  "/export",
+  requireMfaIfEnrolled,
+  asyncRoute(async (req, res) => {
+    const userId = res.locals.userId as string;
+    const email = res.locals.userEmail as string | undefined;
+    const db = createServerSupabase();
+    const parsed = parseQuery(
+      req.query as Record<string, unknown>,
+      AUDIT_EXPORT_LIMIT,
     );
-  }
-  res.setHeader("Content-Type", "text/csv; charset=utf-8");
-  res.setHeader(
-    "Content-Disposition",
-    `attachment; filename="${AUDIT_CSV_FILENAME}"`,
-  );
-  res.send(csv);
-});
+    if (!parsed.ok) return void res.status(400).json({ detail: parsed.error });
+    let csv: string;
+    try {
+      csv = await buildAuditCsv(db, userId, email, parsed.query);
+    } catch (err) {
+      // buildAuditCsv throws so the async job retries; here the throw becomes
+      // the same generic 500 this route has always sent, never the raw DB
+      // message. Unwrap `cause` so the log still carries the PostgrestError's
+      // code/details/hint rather than only its message.
+      return void sendInternalError(
+        res,
+        err instanceof Error && err.cause ? err.cause : err,
+      );
+    }
+    res.setHeader("Content-Type", "text/csv; charset=utf-8");
+    res.setHeader(
+      "Content-Disposition",
+      `attachment; filename="${AUDIT_CSV_FILENAME}"`,
+    );
+    res.send(csv);
+  }),
+);