fix: harden collaboration room access (#1332)

* fix: harden collaboration room access

* fix: confirm guest link lifecycle changes

src/backend/database/repositories/collab-room-repository.ts

@@ -2,6 +2,7 @@ import { and, desc, eq, isNull } from "drizzle-orm";
 import { collabRoomMembers, collabRooms, users } from "../db/schema.js";
 import type { DatabaseContext } from "./database-context.js";
 import { insertReturning } from "./returning.js";
+import { rowsAffected } from "./mutation-result.js";
 
 export type CollabRoomRecord = typeof collabRooms.$inferSelect;
 export type CollabRoomMemberRecord = typeof collabRoomMembers.$inferSelect;
@@ -164,6 +165,27 @@ export class CollabRoomRepository {
     await this.afterWrite();
   }
 
+  async replaceStage(
+    roomId: string,
+    expectedShareId: string | null,
+    stage: CollabRoomStage,
+  ): Promise<boolean> {
+    const result = await this.context.drizzle
+      .update(collabRooms)
+      .set(stage)
+      .where(
+        and(
+          eq(collabRooms.id, roomId),
+          expectedShareId
+            ? eq(collabRooms.stageShareId, expectedShareId)
+            : isNull(collabRooms.stageShareId),
+        ),
+      );
+    const changed = rowsAffected(result) > 0;
+    if (changed) await this.afterWrite();
+    return changed;
+  }
+
   async clearStage(roomId: string): Promise<void> {
     return this.updateStage(roomId, {
       presenterUserId: null,

src/backend/hosts/collab/guest-rate-limit.ts

@@ -0,0 +1,21 @@
+const WINDOW_MS = 60_000;
+const MAX_ATTEMPTS = 60;
+const attempts = new Map<string, { count: number; windowStart: number }>();
+
+export function isCollabGuestRateLimited(ip: string): boolean {
+  const now = Date.now();
+  const entry = attempts.get(ip);
+  if (!entry || now - entry.windowStart > WINDOW_MS) {
+    attempts.set(ip, { count: 1, windowStart: now });
+    return false;
+  }
+  entry.count += 1;
+  return entry.count > MAX_ATTEMPTS;
+}
+
+setInterval(() => {
+  const now = Date.now();
+  for (const [ip, entry] of attempts) {
+    if (now - entry.windowStart > WINDOW_MS) attempts.delete(ip);
+  }
+}, 5 * WINDOW_MS).unref();

src/backend/hosts/collab/routes.ts

@@ -11,6 +11,7 @@ import {
 import { GuacamoleTokenService } from "../guacamole/token-service.js";
 import { collabRoomHub } from "./room-hub.js";
 import { getStageController, setStageController } from "./stage-control.js";
+import { isCollabGuestRateLimited } from "./guest-rate-limit.js";
 import { sessionManager } from "../terminal/session-manager.js";
 import {
   isLiveSession,
@@ -40,7 +41,10 @@ const authenticateJWT = authManager.createAuthMiddleware();
 const tokenService = GuacamoleTokenService.getInstance();
 
 const STAGE_SHARE_EXPIRY_HOURS = 12;
+const MAX_INVITE_TARGETS = 200;
+const CONTROL_REQUEST_COOLDOWN_MS = 5000;
 const PROTOCOLS: LiveProtocol[] = ["ssh", "rdp", "vnc", "telnet"];
+const controlRequestTimes = new Map<string, number>();
 
 function isNonEmptyString(value: unknown): value is string {
   return typeof value === "string" && value.trim().length > 0;
@@ -60,13 +64,22 @@ async function requireRoomMember(
 
 async function revokeStageShare(room: CollabRoomRecord): Promise<void> {
   if (!room.stageShareId) return;
-  try {
-    await createCurrentSessionShareRepository().revokeAsAdmin(
-      room.stageShareId,
-    );
-  } catch {
-    // A stale share must never block switching presenters.
+  const repository = createCurrentSessionShareRepository();
+  const share = await repository.findActiveById(room.stageShareId);
+  if (!share) return;
+  if (!(await repository.revokeAsAdmin(room.stageShareId))) {
+    throw new Error("Failed to revoke the active stage share");
   }
+  if (share.protocol === "ssh") {
+    sessionManager.disconnectShareParticipants(share.sessionId, share.id, {
+      reason: "The collaboration stage ended",
+    });
+  }
+}
+
+function publicRoom(room: CollabRoomRecord) {
+  const { guestLinkToken: _secret, ...safeRoom } = room;
+  return { ...safeRoom, guestLinkEnabled: Boolean(_secret) };
 }
 
 function stagePayload(room: CollabRoomRecord) {
@@ -122,7 +135,7 @@ router.post("/rooms", authenticateJWT, async (req: Request, res: Response) => {
       success: true,
     });
 
-    res.json({ room });
+    res.json({ room: publicRoom(room) });
   } catch (error) {
     sshLogger.error("Failed to create collab room", error, {
       operation: "collab_room_create_error",
@@ -143,7 +156,7 @@ router.get("/rooms", authenticateJWT, async (req: Request, res: Response) => {
   const userId = (req as AuthenticatedRequest).userId!;
   try {
     const rooms = await createCurrentCollabRoomRepository().listForUser(userId);
-    res.json({ rooms });
+    res.json({ rooms: rooms.map(publicRoom) });
   } catch (error) {
     sshLogger.error("Failed to list collab rooms", error, {
       operation: "collab_room_list_error",
@@ -174,7 +187,7 @@ router.get(
       const members =
         await createCurrentCollabRoomRepository().listMembers(roomId);
       res.json({
-        room: access.room,
+        room: publicRoom(access.room),
         me: userId,
         isHost: access.isHost,
         members,
@@ -218,6 +231,9 @@ router.post(
         error: "userIds (user ids) or roleIds (integers) are required",
       });
     }
+    if (userIds.length + roleIds.length > MAX_INVITE_TARGETS) {
+      return res.status(400).json({ error: "Too many invite targets" });
+    }
 
     try {
       const access = await requireRoomMember(roomId, userId);
@@ -256,6 +272,20 @@ router.post(
         });
       }
 
+      const { ipAddress, userAgent } = getRequestMeta(req);
+      await logAudit({
+        userId,
+        username: await getAuditUsername(userId),
+        action: "collab_room_invite",
+        resourceType: "collab_room",
+        resourceId: roomId,
+        resourceName: access.room.name,
+        details: JSON.stringify({ memberCount: expanded.size }),
+        ipAddress,
+        userAgent,
+        success: true,
+      });
+
       collabRoomHub.broadcast(roomId, {
         type: "collab_members_changed",
         roomId,
@@ -303,6 +333,23 @@ router.delete(
       const repository = createCurrentCollabRoomRepository();
       await repository.removeMember(roomId, targetId);
 
+      if (access.room.stageShareId && access.room.stageProtocol === "ssh") {
+        const share =
+          await createCurrentSessionShareRepository().findActiveById(
+            access.room.stageShareId,
+          );
+        if (share) {
+          sessionManager.disconnectShareParticipants(
+            share.sessionId,
+            share.id,
+            {
+              userId: targetId,
+              reason: "You were removed from the collaboration room",
+            },
+          );
+        }
+      }
+
       if (getStageController(roomId) === targetId) {
         await applyStageControl(access.room, roomId, null);
       }
@@ -317,6 +364,23 @@ router.delete(
         });
       }
 
+      const { ipAddress, userAgent } = getRequestMeta(req);
+      await logAudit({
+        userId,
+        username: await getAuditUsername(userId),
+        action:
+          targetId === userId
+            ? "collab_room_leave"
+            : "collab_room_remove_member",
+        resourceType: "collab_room",
+        resourceId: roomId,
+        resourceName: access.room.name,
+        details: JSON.stringify({ targetUserId: targetId }),
+        ipAddress,
+        userAgent,
+        success: true,
+      });
+
       collabRoomHub.broadcast(roomId, {
         type: "collab_members_changed",
         roomId,
@@ -393,12 +457,20 @@ router.post(
       await revokeStageShare(access.room);
       setStageController(roomId, null);
       const repository = createCurrentCollabRoomRepository();
-      await repository.updateStage(roomId, {
-        presenterUserId: userId,
-        stageProtocol: protocol,
-        stageHostId: numericHostId,
-        stageShareId: share.id,
-      });
+      const replaced = await repository.replaceStage(
+        roomId,
+        access.room.stageShareId,
+        {
+          presenterUserId: userId,
+          stageProtocol: protocol,
+          stageHostId: numericHostId,
+          stageShareId: share.id,
+        },
+      );
+      if (!replaced) {
+        await shareRepository.revokeAsAdmin(share.id);
+        return res.status(409).json({ error: "The stage changed; try again" });
+      }
 
       const stage = {
         presenterUserId: userId,
@@ -469,6 +541,18 @@ router.post(
         roomId,
         stage: null,
       });
+      const { ipAddress, userAgent } = getRequestMeta(req);
+      await logAudit({
+        userId,
+        username: await getAuditUsername(userId),
+        action: "collab_room_stop_presenting",
+        resourceType: "collab_room",
+        resourceId: roomId,
+        resourceName: access.room.name,
+        ipAddress,
+        userAgent,
+        success: true,
+      });
       res.json({ success: true });
     } catch (error) {
       sshLogger.error("Failed to stop collab room stage", error, {
@@ -626,6 +710,19 @@ router.post(
       }
 
       await applyStageControl(access.room, roomId, targetId);
+      const { ipAddress, userAgent } = getRequestMeta(req);
+      await logAudit({
+        userId,
+        username: await getAuditUsername(userId),
+        action: targetId ? "collab_control_grant" : "collab_control_revoke",
+        resourceType: "collab_room",
+        resourceId: roomId,
+        resourceName: access.room.name,
+        details: JSON.stringify({ targetUserId: targetId }),
+        ipAddress,
+        userAgent,
+        success: true,
+      });
       res.json({ controllerUserId: targetId });
     } catch (error) {
       sshLogger.error("Failed to change collab stage control", error, {
@@ -663,12 +760,33 @@ router.post(
           error: "Remote desktop stages are read-only",
         });
       }
+      const requestKey = `${roomId}:${userId}`;
+      const now = Date.now();
+      if (
+        now - (controlRequestTimes.get(requestKey) ?? 0) <
+        CONTROL_REQUEST_COOLDOWN_MS
+      ) {
+        return res.status(429).json({ error: "Control was already requested" });
+      }
+      controlRequestTimes.set(requestKey, now);
       collabRoomHub.broadcast(roomId, {
         type: "collab_control_requested",
         roomId,
         userId,
         username: await getAuditUsername(userId),
       });
+      const { ipAddress, userAgent } = getRequestMeta(req);
+      await logAudit({
+        userId,
+        username: await getAuditUsername(userId),
+        action: "collab_control_request",
+        resourceType: "collab_room",
+        resourceId: roomId,
+        resourceName: access.room.name,
+        ipAddress,
+        userAgent,
+        success: true,
+      });
       res.json({ success: true });
     } catch (error) {
       sshLogger.error("Failed to request collab stage control", error, {
@@ -711,6 +829,28 @@ router.post(
         ? crypto.randomBytes(24).toString("base64url")
         : null;
       await createCurrentCollabRoomRepository().setGuestToken(roomId, token);
+      if (
+        (!enabled || access.room.guestLinkToken) &&
+        access.room.stageShareId &&
+        access.room.stageProtocol === "ssh"
+      ) {
+        const share =
+          await createCurrentSessionShareRepository().findActiveById(
+            access.room.stageShareId,
+          );
+        if (share) {
+          sessionManager.disconnectShareParticipants(
+            share.sessionId,
+            share.id,
+            {
+              userId: null,
+              reason: enabled
+                ? "The guest link was rotated"
+                : "The guest link was disabled",
+            },
+          );
+        }
+      }
 
       const { ipAddress, userAgent } = getRequestMeta(req);
       await logAudit({
@@ -736,28 +876,6 @@ router.post(
   },
 );
 
-const GUEST_WINDOW_MS = 60 * 1000;
-const GUEST_MAX_ATTEMPTS = 60;
-const guestAttempts = new Map<string, { count: number; windowStart: number }>();
-
-function isGuestRateLimited(ip: string): boolean {
-  const now = Date.now();
-  const entry = guestAttempts.get(ip);
-  if (!entry || now - entry.windowStart > GUEST_WINDOW_MS) {
-    guestAttempts.set(ip, { count: 1, windowStart: now });
-    return false;
-  }
-  entry.count += 1;
-  return entry.count > GUEST_MAX_ATTEMPTS;
-}
-
-setInterval(() => {
-  const now = Date.now();
-  for (const [ip, entry] of guestAttempts) {
-    if (now - entry.windowStart > GUEST_WINDOW_MS) guestAttempts.delete(ip);
-  }
-}, 5 * GUEST_WINDOW_MS).unref();
-
 /**
  * @openapi
  * /collab/guest/{token}:
@@ -769,7 +887,7 @@ setInterval(() => {
  */
 router.get("/guest/:token", async (req: Request, res: Response) => {
   const ip = req.ip || req.socket.remoteAddress || "unknown";
-  if (isGuestRateLimited(ip)) {
+  if (isCollabGuestRateLimited(ip)) {
     return res.status(429).json({ error: "Too many requests" });
   }
   const token = String(req.params.token);
@@ -846,6 +964,7 @@ router.post(
       setStageController(roomId, null);
       const repository = createCurrentCollabRoomRepository();
       if (access.room.persistent) {
+        await repository.setGuestToken(roomId, null);
         await repository.clearStage(roomId);
         collabRoomHub.broadcast(roomId, {
           type: "collab_stage_changed",

src/backend/hosts/terminal/index.ts

@@ -171,6 +171,13 @@ async function handleRoomGuestConnection(
   req: import("http").IncomingMessage,
   roomGuestToken: string,
 ): Promise<void> {
+  const { isCollabGuestRateLimited } =
+    await import("../collab/guest-rate-limit.js");
+  const ip = req.socket.remoteAddress ?? "unknown";
+  if (isCollabGuestRateLimited(ip)) {
+    ws.close(1008, "Too many requests");
+    return;
+  }
   const room =
     await createCurrentCollabRoomRepository().findByGuestToken(roomGuestToken);
   const share = room?.stageShareId

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

@@ -478,6 +478,49 @@ class TerminalSessionManager {
     }
   }
 
+  /**
+   * Disconnects non-owner participants that joined through one share.
+   * Supplying userId narrows the kick to that authenticated user; null targets
+   * anonymous guests. Omitting it revokes the share for every participant.
+   */
+  disconnectShareParticipants(
+    sessionId: string,
+    shareId: string,
+    options: { userId?: string | null; reason: string },
+  ): number {
+    const session = this.sessions.get(sessionId);
+    if (!session) return 0;
+    const filterByUser = Object.hasOwn(options, "userId");
+    let disconnected = 0;
+    for (const [id, participant] of session.participants.entries()) {
+      if (
+        participant.isOwner ||
+        participant.joinedViaShareId !== shareId ||
+        (filterByUser && participant.userId !== options.userId)
+      ) {
+        continue;
+      }
+      session.participants.delete(id);
+      disconnected += 1;
+      if (participant.ws.readyState === WebSocket.OPEN) {
+        try {
+          participant.ws.send(
+            JSON.stringify({
+              type: "sessionExpired",
+              sessionId,
+              message: options.reason,
+            }),
+          );
+          participant.ws.close(1008, options.reason);
+        } catch {
+          participant.ws.terminate();
+        }
+      }
+    }
+    if (disconnected > 0) this.broadcastParticipants(sessionId);
+    return disconnected;
+  }
+
   /** Fans out a message to every OPEN participant socket; skips closed ones and send failures. */
   broadcast(sessionId: string, message: object): void {
     const session = this.sessions.get(sessionId);

src/backend/tests/hosts/collab/routes.test.ts

@@ -72,6 +72,7 @@ vi.mock("../../../hosts/terminal/session-manager.js", () => ({
     setRoomShareControl: (...args: unknown[]) => {
       state.control.push(args);
     },
+    disconnectShareParticipants: vi.fn(() => 0),
   },
 }));
 vi.mock("../../../hosts/session-sharing/live-sessions.js", () => ({
@@ -147,6 +148,16 @@ vi.mock("../../../database/repositories/factory.js", () => ({
     updateStage: async (roomId: string, stage: Partial<Room>) => {
       Object.assign(state.rooms.get(roomId)!, stage);
     },
+    replaceStage: async (
+      roomId: string,
+      expectedShareId: string | null,
+      stage: Partial<Room>,
+    ) => {
+      const room = state.rooms.get(roomId)!;
+      if (room.stageShareId !== expectedShareId) return false;
+      Object.assign(room, stage);
+      return true;
+    },
     clearStage: async (roomId: string) => {
       Object.assign(state.rooms.get(roomId)!, {
         presenterUserId: null,
@@ -307,6 +318,26 @@ describe("collab room routes", () => {
     expect((other as { statusCode: number }).statusCode).toBe(404);
   });
 
+  it("never exposes the guest bearer token in room responses", async () => {
+    const roomId = await createRoom();
+    await invite(roomId, ["alice"]);
+    state.rooms.get(roomId)!.guestLinkToken = "secret-token";
+
+    const list = await as("alice", () => invoke("get", "/rooms"));
+    const listedRoom = (list as Awaited<ReturnType<typeof invoke>>).jsonBody!
+      .rooms as Array<Record<string, unknown>>;
+    expect(listedRoom[0]).not.toHaveProperty("guestLinkToken");
+    expect(listedRoom[0]).toHaveProperty("guestLinkEnabled", true);
+
+    const detail = await as("alice", () =>
+      invoke("get", "/rooms/:id", { params: { id: roomId } }),
+    );
+    const room = (detail as Awaited<ReturnType<typeof invoke>>).jsonBody!
+      .room as Record<string, unknown>;
+    expect(room).not.toHaveProperty("guestLinkToken");
+    expect(room).toHaveProperty("guestLinkEnabled", true);
+  });
+
   it("rejects an empty or oversized room name", async () => {
     expect(
       (await invoke("post", "/rooms", { body: { name: "  " } })).statusCode,

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

@@ -57,6 +57,8 @@ function makeFakeWs(readyState = 1 /* OPEN */) {
   return {
     readyState,
     send: vi.fn(),
+    close: vi.fn(),
+    terminate: vi.fn(),
   } as unknown as import("ws").WebSocket;
 }
 const WS_OPEN = 1;
@@ -254,6 +256,39 @@ describe("TerminalSessionManager - multiplayer participants", () => {
     sessionManager.destroySession(id);
   });
 
+  it("disconnectShareParticipants revokes only the selected share participants", () => {
+    const id = createConnectedSession();
+    const ownerWs = makeFakeWs();
+    const aliceWs = makeFakeWs();
+    const guestWs = makeFakeWs();
+    sessionManager.attachWs(id, "owner-1", ownerWs);
+    const session = sessionManager.joinAsParticipant(id, aliceWs, {
+      userId: "alice",
+      permissionLevel: "read-only",
+      shareId: "stage-share",
+    })!;
+    sessionManager.joinAsParticipant(id, guestWs, {
+      userId: null,
+      permissionLevel: "read-only",
+      shareId: "stage-share",
+    });
+
+    expect(
+      sessionManager.disconnectShareParticipants(id, "stage-share", {
+        userId: "alice",
+        reason: "Removed",
+      }),
+    ).toBe(1);
+    expect(sessionManager.getParticipantForWs(session, aliceWs)).toBeNull();
+    expect(sessionManager.getParticipantForWs(session, guestWs)).not.toBeNull();
+    expect(sessionManager.getParticipantForWs(session, ownerWs)?.isOwner).toBe(
+      true,
+    );
+    expect(aliceWs.close).toHaveBeenCalledWith(1008, "Removed");
+
+    sessionManager.destroySession(id);
+  });
+
   it("joinAsParticipant returns null for a nonexistent or unconnected session", () => {
     expect(
       sessionManager.joinAsParticipant("does-not-exist", makeFakeWs(), {