From 5f867cd048c3ef568f2a4a50bce1c86c42350bd5 Mon Sep 17 00:00:00 2001
From: "groombook-ci[bot]" <ci@groombook.bot>
Date: Sun, 29 Mar 2026 00:38:45 +0000
Subject: [PATCH] fix(api): add requireRoleOrSuperUser OR-guard, replace
 AND-stacking on staff routes

CRITICAL: requireRole("manager") + requireSuperUser() stacked = AND logic,
blocking all non-super-user managers from staff CRUD.

Added requireRoleOrSuperUser() OR-guard middleware. Staff write routes now use
the combined guard: manager role OR super-user flag grants access.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
---
 apps/api/src/index.ts           |  7 +++----
 apps/api/src/middleware/rbac.ts | 31 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts
index e98a003..286a969 100644
--- a/apps/api/src/index.ts
+++ b/apps/api/src/index.ts
@@ -22,7 +22,7 @@ import { calendarRouter } from "./routes/calendar.js";
 import { setupRouter } from "./routes/setup.js";
 import { getDb, businessSettings, eq, staff } from "@groombook/db";
 import { authMiddleware } from "./middleware/auth.js";
-import { resolveStaffMiddleware, requireRole, requireSuperUser } from "./middleware/rbac.js";
+import { resolveStaffMiddleware, requireRole, requireRoleOrSuperUser, requireSuperUser } from "./middleware/rbac.js";
 import { devRouter } from "./routes/dev.js";
 import { adminSeedRouter } from "./routes/admin/seed.js";
 import { startReminderScheduler } from "./services/reminders.js";
@@ -94,9 +94,8 @@ api.route("/auth", authRouter);
 // Manager-only: admin settings, reports, invoices, impersonation
 // Staff CRUD: all roles may READ; manager-only for CREATE/UPDATE/DELETE
 api.on(["GET"], "/staff/*", requireRole("manager", "receptionist", "groomer"));
-// Staff write routes: manager + super-user
-api.on(["POST", "PATCH", "DELETE"], "/staff/*", requireRole("manager"));
-api.on(["POST", "PATCH", "DELETE"], "/staff/*", requireSuperUser());
+// Staff write routes: manager OR super-user (combined guard — avoids AND stacking)
+api.on(["POST", "PATCH", "DELETE"], "/staff/*", requireRoleOrSuperUser("manager"));
 api.use("/admin/*", requireRole("manager"));
 api.use("/admin/settings/*", requireSuperUser());
 api.use("/reports/*", requireRole("manager"));
diff --git a/apps/api/src/middleware/rbac.ts b/apps/api/src/middleware/rbac.ts
index 3a58d17..124ca96 100644
--- a/apps/api/src/middleware/rbac.ts
+++ b/apps/api/src/middleware/rbac.ts
@@ -126,6 +126,37 @@ export function requireRole(
   };
 }
 
+/**
+ * Middleware that allows access if the staff member has any of the allowed roles OR is a super user.
+ * Use for routes where managers OR super-users should have access.
+ *
+ * @example
+ *   api.on(["POST", "PATCH", "DELETE"], "/staff/*", requireRoleOrSuperUser("manager"));
+ */
+export function requireRoleOrSuperUser(
+  ...allowedRoles: StaffRole[]
+): MiddlewareHandler<AppEnv> {
+  return async (c, next) => {
+    const staffRow = c.get("staff");
+    if (!staffRow) {
+      return c.json({ error: "Forbidden: staff record not resolved" }, 403);
+    }
+    const hasAllowedRole = (allowedRoles as string[]).includes(staffRow.role);
+    if (hasAllowedRole || staffRow.isSuperUser) {
+      await next();
+      return;
+    }
+    return c.json(
+      {
+        error: staffRow.isSuperUser
+          ? `Forbidden: role '${staffRow.role}' is not permitted`
+          : "Forbidden: super user privileges required",
+      },
+      403
+    );
+  };
+}
+
 /**
  * Middleware that enforces the staff member is a super user.
  * Must be applied after resolveStaffMiddleware and (typically) after requireRole.