fix(backend): gate dev routes and unify sync error envelopes

Stop verifyIsDev from calling next after a production 403, require
delete-all to match the session user, map Sync proxy failures to real
HTTP statuses (never 600), and make unknown mutation errors non-retryable.

Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>

Author: cursoragent

docs/backend/backend-error-handling.md

@@ -5,11 +5,20 @@ Compass uses typed operational errors plus a centralized Express error handler.
 ## Principles
 
 - Minimize the number of `try/catch` blocks in the code.
+- Never return non-HTTP statuses such as `Status.UNSURE` (600) on live paths.
+- Event mutation routes use the `EventMutationError` envelope
+  (`code` / `message` / `retryable`). Unknown/programmer errors are
+  **non-retryable 500** — only true Sync/provider failures stay retryable
+  `PROVIDER_FAILURE`.
+- Sync proxy failures on calendar/auth reads use `throwSyncProxyFailure` /
+  `unwrapSyncResult` (503/502), never `GenericError.NotSure`.
 
 ## Source Files
 
 - `packages/backend/src/common/errors/handlers/error.handler.ts`
 - `packages/backend/src/common/errors/handlers/error.express.handler.ts`
+- `packages/backend/src/common/services/sync-service/sync-proxy-error.ts`
+- `packages/backend/src/event/event.error.ts`
 - feature error metadata files under `packages/backend/src/common/errors/**`
 - `packages/core/src/errors/errors.base.ts`
 
@@ -20,15 +29,20 @@ Preferred backend pattern:
 1. define reusable error metadata in the relevant feature file
 2. create a `BaseError` through `error(...)`
 3. let controller/service code throw that error
-4. let centralized Express handling turn it into the client payload
+4. let centralized Express handling (`res.promise` → `handleExpressError`) turn it into the client payload
+
+Event mutation controllers may catch locally and call `toEventMutationError`
+so the strict `{ code, message, retryable }` envelope is preserved. User
+controllers return JSON via `toClientErrorPayload` (or `{ code, message }` for
+unexpected errors) rather than empty bodies.
 
 Example:
 
 ```ts
 import { AuthError } from "@backend/common/errors/auth/auth.errors";
 import { error } from "@backend/common/errors/handlers/error.handler";
 
-throw error(AuthError.MissingRefreshToken, "Google connection required");
+throw error(AuthError.SyncConnectionUnavailable, "Could not reach sync");
 ```
 
 ## Client Payload Rules
@@ -39,13 +53,15 @@ For `BaseError`, backend responses are intentionally small:
 - `message`: safe user-facing description
 - `code`: optional stable machine-readable identifier for frontend branching
 
+Event mutations use `{ code, message, retryable }` instead.
+
 Internal details such as stack traces and operational flags stay server-side.
 
 ## Unexpected Error Rules
 
-- non-`BaseError` values are routed through `handleExpressError(...)`
-- Google API errors get special handling for revoked tokens, invalid values, and full-sync recovery
-- programmer errors can terminate the process after logging
+- non-`BaseError` values are routed through `handleExpressError(...)` when using `res.promise`
+- Sync client failures map to 502/503 (or typed mutation codes) — never HTTP 600
+- programmer errors can terminate the process after logging when `isOperational` is false
 
 ## Guidance
 

packages/backend/src/auth/middleware/auth.middleware.test.ts

@@ -0,0 +1,46 @@
+import { type NextFunction, type Request, type Response } from "express";
+import { NodeEnv } from "@core/constants/core.constants";
+import { Status } from "@core/errors/status.codes";
+import authMiddleware from "@backend/auth/middleware/auth.middleware";
+import { CONFIG } from "@backend/common/constants/config.constants";
+import { afterEach, describe, expect, it, mock } from "bun:test";
+
+describe("auth.middleware verifyIsDev", () => {
+  const originalNodeEnv = CONFIG.NODE_ENV;
+
+  afterEach(() => {
+    CONFIG.NODE_ENV = originalNodeEnv;
+  });
+
+  const mockRes = () => {
+    const json = mock();
+    const res = {
+      status: mock().mockReturnThis(),
+      json,
+    } as unknown as Response;
+    return { res, json };
+  };
+
+  it("returns 403 and does not call next in production", () => {
+    CONFIG.NODE_ENV = NodeEnv.Production;
+    const { res, json } = mockRes();
+    const next = mock() as NextFunction;
+
+    authMiddleware.verifyIsDev({} as Request, res, next);
+
+    expect(res.status).toHaveBeenCalledWith(Status.FORBIDDEN);
+    expect(json).toHaveBeenCalled();
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it("calls next in development", () => {
+    CONFIG.NODE_ENV = NodeEnv.Development;
+    const { res } = mockRes();
+    const next = mock() as NextFunction;
+
+    authMiddleware.verifyIsDev({} as Request, res, next);
+
+    expect(res.status).not.toHaveBeenCalled();
+    expect(next).toHaveBeenCalledTimes(1);
+  });
+});

packages/backend/src/auth/middleware/auth.middleware.ts

@@ -1,15 +1,19 @@
 import { type NextFunction, type Request, type Response } from "express";
 import { Status } from "@core/errors/status.codes";
-import { IS_DEV } from "@backend/common/constants/config.constants";
+import { isDev } from "@core/util/env.util";
+import { CONFIG } from "@backend/common/constants/config.constants";
 import { AuthError } from "@backend/common/errors/auth/auth.errors";
 import { error } from "@backend/common/errors/handlers/error.handler";
 
 class AuthMiddleware {
   verifyIsDev = (_req: Request, res: Response, next: NextFunction) => {
-    if (!IS_DEV) {
+    // Read NODE_ENV from CONFIG each call so production never falls through
+    // to next() after a 403 (and so tests can flip CONFIG.NODE_ENV).
+    if (!isDev(CONFIG.NODE_ENV)) {
       res
         .status(Status.FORBIDDEN)
         .json({ error: error(AuthError.DevOnly, "Request Failed") });
+      return;
     }
     next();
   };

packages/backend/src/calendar/controllers/calendar.controller.ts

@@ -15,6 +15,7 @@ import { GenericError } from "@backend/common/errors/generic/generic.errors";
 import { error } from "@backend/common/errors/handlers/error.handler";
 import { syncCalendarsToBrowser } from "@backend/common/services/sync-service/calendar-list.translation";
 import { toSyncPrincipal } from "@backend/common/services/sync-service/sync-principal";
+import { throwSyncProxyFailure } from "@backend/common/services/sync-service/sync-proxy-error";
 import { getSyncServiceClient } from "@backend/common/services/sync-service/sync-service.factory";
 import { type Res_Promise } from "@backend/common/types/express.types";
 
@@ -76,14 +77,14 @@ const listCalendarsFromSync = async (
     client.listConnections(principal),
   ]);
   if (!calendarsResult.ok) {
-    throw error(
-      GenericError.NotSure,
+    throwSyncProxyFailure(
+      calendarsResult.error.kind,
       `Failed to list calendars from sync (${calendarsResult.error.kind})`,
     );
   }
   if (!connectionsResult.ok) {
-    throw error(
-      GenericError.NotSure,
+    throwSyncProxyFailure(
+      connectionsResult.error.kind,
       `Failed to list connections from sync (${connectionsResult.error.kind})`,
     );
   }
@@ -124,8 +125,8 @@ const getAvailabilityFromSync = async (
         purpose: "display",
       });
       if (!result.ok) {
-        throw error(
-          GenericError.NotSure,
+        throwSyncProxyFailure(
+          result.error.kind,
           `Failed to query availability from sync (${result.error.kind})`,
         );
       }

packages/backend/src/common/errors/generic/generic.errors.ts

@@ -22,17 +22,19 @@ export const GenericError: GenericErrors = {
   },
   NotImplemented: {
     description: "Not implemented yet",
-    status: Status.UNSURE,
+    status: Status.NOT_IMPLEMENTED,
     isOperational: true,
   },
+  // Prefer typed Sync/auth/event errors on live paths. Kept as a last-resort
+  // operational 500 — never Status.UNSURE (600), which is not a real HTTP status.
   NotSure: {
     description: "Not sure why error occurred. See logs",
-    status: Status.UNSURE,
+    status: Status.INTERNAL_SERVER,
     isOperational: true,
   },
   OperationTimeout: {
     description: "Operation timed out",
-    status: Status.UNSURE,
+    status: Status.GATEWAY_TIMEOUT,
     isOperational: true,
   },
 };

packages/backend/src/common/services/sync-service/sync-proxy-error.test.ts

@@ -0,0 +1,67 @@
+import { BaseError } from "@core/errors/errors.base";
+import { Status } from "@core/errors/status.codes";
+import {
+  throwSyncCommandSubmitFailure,
+  throwSyncProxyFailure,
+} from "@backend/common/services/sync-service/sync-proxy-error";
+import {
+  EventMutationException,
+  toEventMutationError,
+} from "@backend/event/event.error";
+import { describe, expect, it } from "bun:test";
+
+describe("toEventMutationError", () => {
+  it("keeps PROVIDER_FAILURE retryable for typed mutation failures", () => {
+    const mapped = toEventMutationError(
+      new EventMutationException("PROVIDER_FAILURE", "provider blip"),
+    );
+    expect(mapped.status).toBe(502);
+    expect(mapped.body.retryable).toBe(true);
+  });
+
+  it("maps unknown errors to non-retryable 500", () => {
+    const mapped = toEventMutationError(new Error("boom"));
+    expect(mapped.status).toBe(Status.INTERNAL_SERVER);
+    expect(mapped.body).toEqual({
+      code: "PROVIDER_FAILURE",
+      message: "boom",
+      retryable: false,
+    });
+  });
+});
+
+describe("sync-proxy-error", () => {
+  it("maps unavailable Sync reads to 503, never 600", () => {
+    try {
+      throwSyncProxyFailure("unavailable", "calendars down");
+      throw new Error("expected throw");
+    } catch (e) {
+      expect(e).toBeInstanceOf(BaseError);
+      expect((e as BaseError).statusCode).toBe(Status.SERVICE_UNAVAILABLE);
+      expect((e as BaseError).statusCode).not.toBe(Status.UNSURE);
+    }
+  });
+
+  it("maps unexpected Sync reads to 502, never 600", () => {
+    try {
+      throwSyncProxyFailure("unexpectedStatus", "weird status");
+      throw new Error("expected throw");
+    } catch (e) {
+      expect(e).toBeInstanceOf(BaseError);
+      expect((e as BaseError).statusCode).toBe(Status.BAD_GATEWAY);
+      expect((e as BaseError).statusCode).not.toBe(Status.UNSURE);
+    }
+  });
+
+  it("maps command submit timeout to retryable PROVIDER_FAILURE", () => {
+    try {
+      throwSyncCommandSubmitFailure("timeout", "timeout");
+      throw new Error("expected throw");
+    } catch (e) {
+      expect(e).toBeInstanceOf(EventMutationException);
+      const mapped = toEventMutationError(e);
+      expect(mapped.status).toBe(502);
+      expect(mapped.body.retryable).toBe(true);
+    }
+  });
+});

packages/backend/src/common/services/sync-service/sync-proxy-error.ts

@@ -0,0 +1,51 @@
+import { Status } from "@core/errors/status.codes";
+import { AuthError } from "@backend/common/errors/auth/auth.errors";
+import { error } from "@backend/common/errors/handlers/error.handler";
+import { type SyncClientErrorKind } from "@backend/common/services/sync-service/sync-service.client";
+import { eventMutationError } from "@backend/event/event.error";
+
+/**
+ * Map a failed Sync HTTP call on a read/proxy route into a real HTTP status.
+ * Never returns Status.UNSURE (600). Timeout/unavailable → 503; other Sync
+ * failures → 502. Clients get a retryable service error without Sync-internal
+ * details leaking beyond the kind already in `userMessage`.
+ */
+export function throwSyncProxyFailure(
+  kind: SyncClientErrorKind,
+  userMessage: string,
+): never {
+  if (kind === "timeout" || kind === "unavailable") {
+    throw error(AuthError.SyncConnectionUnavailable, userMessage);
+  }
+  throw error(
+    {
+      description: "Sync service returned an unexpected failure",
+      status: Status.BAD_GATEWAY,
+      isOperational: true,
+      code: "SYNC_PROXY_FAILURE",
+    },
+    userMessage,
+  );
+}
+
+/**
+ * Map a failed Sync command submit on an event mutation route. Timeout and
+ * unavailable stay PROVIDER_FAILURE (retryable 502) because Sync may already
+ * have applied the write. Other kinds are also PROVIDER_FAILURE so clients
+ * never see GenericError.NotSure / HTTP 600.
+ */
+export function throwSyncCommandSubmitFailure(
+  kind: SyncClientErrorKind,
+  detail: string,
+): never {
+  if (kind === "timeout" || kind === "unavailable") {
+    throw eventMutationError(
+      "PROVIDER_FAILURE",
+      `Sync command ${kind}; the mutation may already be applied`,
+    );
+  }
+  throw eventMutationError(
+    "PROVIDER_FAILURE",
+    `Failed to submit command to sync (${detail})`,
+  );
+}

packages/backend/src/common/services/sync-service/sync-service.client.ts

@@ -559,12 +559,11 @@ function statusToKind(status: number): SyncClientErrorKind {
   if (status === 400) return "badRequest";
   if (status === 404) return "notFound";
   if (status === 409) return "conflict";
+  // unexpectedStatus -> Sync proxy 502 (never Status.UNSURE / HTTP 600).
   // 429 is sync's own internal rate limiter (internal-http.ts), tripped
   // easily under normal-ish load (a shared 300/min bucket for the whole
   // backend). Treated the same as 503: a retryable service-busy state, not
-  // an unexpected condition - previously this fell through to
-  // unexpectedStatus -> GenericError.NotSure, whose Status.UNSURE (600) is
-  // not a real HTTP status and reads as an unretryable mystery to the caller.
+  // an unexpected condition.
   if (status === 429 || status === 503) return "unavailable";
   return "unexpectedStatus";
 }

packages/backend/src/event/controllers/event.controller.test.ts

@@ -387,4 +387,29 @@ describe("EventController", () => {
       retryable: true,
     });
   });
+
+  it("rejects delete-all when the session user does not match :userId", async () => {
+    const deleteSpy = spyOn(
+      (await import("@backend/event/services/event.service")).default,
+      "deleteAllByUser",
+    );
+    const { res, json } = jsonRes();
+    const sessionUser = objectId();
+    const otherUser = objectId();
+
+    await eventController.deleteAllByUser(
+      sessionReq(sessionUser, { params: { userId: otherUser } }),
+      res,
+    );
+
+    expect(deleteSpy).not.toHaveBeenCalled();
+    expect((res.status as ReturnType<typeof mock>).mock.calls[0]?.[0]).toBe(
+      Status.BAD_REQUEST,
+    );
+    expect(json).toHaveBeenCalledWith({
+      code: "INVALID_INPUT",
+      message: "Cannot delete events for another user",
+      retryable: false,
+    });
+  });
 });