feat: enforce RBAC and harden collaboration features (#1327)

* feat: enforce RBAC and harden collaboration features

- Mount requirePermission on hosts/snippets/credentials/automations/AI routes
- Seed and backfill system role permissions on every dialect at startup
- Support personal credential overrides for RDP/VNC/Telnet shared hosts
- Broadcast participant presence in shared terminal sessions
- Make audit log forwarding configurable from the admin panel
- Add role members endpoint and snippet folder sharing

* fix: enforce RBAC across split routes

src/backend/tests/ai/tool-catalog.test.ts

@@ -10,9 +10,8 @@ import {
 /**
  * The security regression test for the whole feature.
  *
- * PermissionManager.requirePermission is defined but mounted on zero routes,
- * so RBAC strings do not gate anything at the route layer. "The assistant
- * cannot reach credentials or user administration" is therefore a property of
+ * Tools run in-process and never pass through the RBAC-gated routers, so "the
+ * assistant cannot reach credentials or user administration" is a property of
  * this catalog, and nothing else. If a future change adds a tool that touches a
  * forbidden domain, this test is what catches it.
  */

src/backend/tests/database/routes/automations.test.ts

@@ -48,6 +48,16 @@ vi.mock("../../../automations/engine.js", () => ({
   AutomationEngine: { getInstance: () => ({ run }) },
 }));
 
+vi.mock("../../../utils/permission-manager.js", () => ({
+  PermissionManager: {
+    getInstance: () => ({
+      requirePermission:
+        () => (_req: unknown, _res: unknown, next: () => void) =>
+          next(),
+    }),
+  },
+}));
+
 vi.mock("../../../utils/auth-manager.js", () => ({
   AuthManager: {
     getInstance: () => ({

src/backend/tests/database/routes/credential-bulk-routes.test.ts

@@ -0,0 +1,71 @@
+import express, {
+  type RequestHandler,
+  type Response,
+  type Router,
+} from "express";
+import { describe, expect, it, vi } from "vitest";
+
+import { registerCredentialBulkRoutes } from "../../../database/routes/credential-bulk-routes.js";
+
+vi.mock("../../../database/repositories/factory.js", () => ({
+  createCurrentCredentialRepository: () => ({
+    reorderForUser: vi.fn(),
+  }),
+}));
+
+vi.mock("../../../utils/logger.js", () => ({
+  authLogger: { error: vi.fn() },
+}));
+
+function reorderHandlers(router: Router): RequestHandler[] {
+  const layer = router.stack.find(
+    (entry) => entry.route?.path === "/reorder" && entry.route.methods.put,
+  );
+  if (!layer?.route) throw new Error("PUT /reorder was not registered");
+  return layer.route.stack.map((entry) => entry.handle);
+}
+
+describe("credential reorder route", () => {
+  it("stops before data access when credentials.edit is denied", async () => {
+    const router = express.Router();
+    const authenticate: RequestHandler = (req, _res, next) => {
+      Object.assign(req, { userId: "user-1" });
+      next();
+    };
+    const requireEdit: RequestHandler = (_req, res) => {
+      res.status(403).json({
+        error: "Insufficient permissions",
+        required: "credentials.edit",
+      });
+    };
+    const requireDataAccess = vi.fn((_req, _res, next) => next());
+
+    registerCredentialBulkRoutes(
+      router,
+      authenticate,
+      requireEdit,
+      requireDataAccess,
+    );
+
+    const response = {} as Response;
+    const status = vi.fn(() => response);
+    const json = vi.fn(() => response);
+    Object.assign(response, { status, json });
+    const request = { body: { positions: [{ id: 1, sortOrder: 0 }] } };
+
+    for (const handler of reorderHandlers(router)) {
+      let continued = false;
+      await handler(request as never, response, () => {
+        continued = true;
+      });
+      if (!continued) break;
+    }
+
+    expect(status).toHaveBeenCalledWith(403);
+    expect(json).toHaveBeenCalledWith({
+      error: "Insufficient permissions",
+      required: "credentials.edit",
+    });
+    expect(requireDataAccess).not.toHaveBeenCalled();
+  });
+});

src/backend/tests/database/routes/rbac-host-auth-override.test.ts

@@ -48,6 +48,14 @@ vi.mock("../../../utils/permission-manager.js", () => ({
   PermissionManager: {
     getInstance: () => ({
       canAccessHost: async () => state.access,
+      requirePermission:
+        () =>
+        (
+          _req: express.Request,
+          _res: express.Response,
+          next: express.NextFunction,
+        ) =>
+          next(),
       requireAdmin:
         () =>
         (
@@ -255,14 +263,10 @@ describe("shared host authentication override routes", () => {
     expect(unauthenticatedResponse.status).toBe(401);
   });
 
-  it("rejects recognized but unsupported protocols and invalid protocol names", async () => {
-    const unsupportedResponse = await invoke("get", {}, "rdp");
-    expect(unsupportedResponse).toEqual({
-      status: 400,
-      body: {
-        error: "RDP authentication overrides are not supported yet",
-      },
-    });
+  it("serves every real protocol and rejects invalid protocol names", async () => {
+    const rdpResponse = await invoke("get", {}, "rdp");
+    expect(rdpResponse.status).toBe(200);
+    expect(rdpResponse.body.protocol).toBe("rdp");
     expect(state.writes).toEqual([]);
 
     const invalidResponse = await invoke("get", {}, "smtp");

src/backend/tests/database/routes/rbac-route-coverage.test.ts

@@ -0,0 +1,100 @@
+import express, { type RequestHandler, type Router } from "express";
+import { describe, expect, it, vi } from "vitest";
+
+import { registerHostBulkRoutes } from "../../../database/routes/host-bulk-routes.js";
+import { registerHostFolderRoutes } from "../../../database/routes/host-folder-routes.js";
+
+vi.mock("../../../database/repositories/factory.js", () => ({}));
+vi.mock("../../../utils/logger.js", () => ({
+  databaseLogger: { info: vi.fn(), error: vi.fn() },
+  sshLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
+}));
+
+const middleware = (): RequestHandler => (_req, _res, next) => next();
+
+function handlers(router: Router, method: string, path: string) {
+  const layer = router.stack.find(
+    (entry) => entry.route?.path === path && entry.route.methods[method],
+  );
+  if (!layer?.route) throw new Error(`${method.toUpperCase()} ${path} missing`);
+  return layer.route.stack.map((entry) => entry.handle);
+}
+
+describe("RBAC coverage for split host routers", () => {
+  it("gates bulk mutations by action and data-access permissions", () => {
+    const router = express.Router();
+    const authenticate = middleware();
+    const requireCreate = middleware();
+    const requireEdit = middleware();
+    const requireDataAccess = middleware();
+
+    registerHostBulkRoutes(
+      router,
+      authenticate,
+      requireCreate,
+      requireEdit,
+      requireDataAccess,
+    );
+
+    expect(handlers(router, "patch", "/bulk-update").slice(0, 3)).toEqual([
+      authenticate,
+      requireEdit,
+      requireDataAccess,
+    ]);
+    expect(handlers(router, "put", "/reorder").slice(0, 3)).toEqual([
+      authenticate,
+      requireEdit,
+      requireDataAccess,
+    ]);
+    for (const path of ["/bulk-import", "/ssh-config-import"]) {
+      expect(handlers(router, "post", path).slice(0, 4)).toEqual([
+        authenticate,
+        requireCreate,
+        requireEdit,
+        requireDataAccess,
+      ]);
+    }
+  });
+
+  it("gates folder reads and mutations with their matching permissions", () => {
+    const router = express.Router();
+    const authenticate = middleware();
+    const requireView = middleware();
+    const requireEdit = middleware();
+    const requireDelete = middleware();
+    const requireCredentialEdit = middleware();
+    const requireDataAccess = middleware();
+
+    registerHostFolderRoutes(router, {
+      authenticateJWT: authenticate,
+      requireViewPermission: requireView,
+      requireEditPermission: requireEdit,
+      requireDeletePermission: requireDelete,
+      requireCredentialEditPermission: requireCredentialEdit,
+      requireDataAccess,
+      statsServerUrl: "http://stats.invalid",
+    });
+
+    expect(handlers(router, "get", "/folders").slice(0, 3)).toEqual([
+      authenticate,
+      requireView,
+      requireDataAccess,
+    ]);
+    expect(handlers(router, "put", "/folders/rename").slice(0, 4)).toEqual([
+      authenticate,
+      requireEdit,
+      requireCredentialEdit,
+      requireDataAccess,
+    ]);
+    for (const path of ["/folders/metadata", "/folders/reorder"]) {
+      expect(handlers(router, "put", path).slice(0, 3)).toEqual([
+        authenticate,
+        requireEdit,
+        requireDataAccess,
+      ]);
+    }
+    expect(
+      handlers(router, "delete", "/folders/:name/hosts").slice(0, 3),
+    ).toEqual([authenticate, requireDelete, requireDataAccess]);
+  });
+});

src/backend/tests/hosts/terminal/session-manager.test.ts

@@ -206,7 +206,14 @@ describe("TerminalSessionManager - multiplayer participants", () => {
       ownerWs,
     );
     expect(ownerParticipant?.isOwner).toBe(true);
-    expect(ownerWs.send).not.toHaveBeenCalled();
+    // The join is announced to everyone already in the session - and that is
+    // the only unsolicited message the owner receives.
+    expect(ownerWs.send).toHaveBeenCalledTimes(1);
+    const announced = JSON.parse(
+      (ownerWs.send as ReturnType<typeof vi.fn>).mock.calls[0][0] as string,
+    );
+    expect(announced.type).toBe("participants");
+    expect(announced.participants).toHaveLength(2);
 
     sessionManager.destroySession(id);
   });

src/backend/tests/utils/audit-forwarder.test.ts

@@ -3,6 +3,9 @@ import { describe, expect, it, vi, beforeEach } from "vitest";
 const safeFetch = vi.hoisted(() => vi.fn());
 const logs = vi.hoisted(() => ({ info: vi.fn(), warn: vi.fn() }));
 
+vi.mock("../../database/repositories/factory.js", () => ({
+  getCurrentSettingValue: () => null,
+}));
 vi.mock("../../utils/safe-outbound-fetch.js", () => ({
   safeOutboundFetch: safeFetch,
 }));