fix: harden leave room rate limiting

.github/workflows/ci.yml

@@ -42,8 +42,5 @@ jobs:
         run: npm ci
         working-directory: server
 
-      - name: Run tests
-        run: npm test
-
       - name: Run verification suite
         run: npm run verify

docs/CHANGELOG.md

@@ -8,12 +8,6 @@ All notable changes to the KoalaSync browser extension and relay server.
 
 ### Added
 - **Extension: Privacy title controls** - Advanced users can now disable sending browser tab titles separately from media titles. Media titles can still be sent in full, reduced to detected episode identifiers such as `S01E04`, or hidden entirely. Defaults remain full titles for backwards compatibility.
-
----
-
-## [v2.5.1] — 2026-07-01
-
-### Added
 - **Relay: Cleaner restart handling** — Connected clients are now disconnected explicitly during relay shutdown so reconnects recover more predictably.
 - **Relay: Stronger abuse protection** — Rapid room-leave spam is now rate-limited.
 

docs/PROTOCOL.md

@@ -93,8 +93,7 @@ Behavior:
   falls back to `controlMode: "everyone"`, resets controllers to the new host,
   and broadcasts `control_mode`.
 
-Exceeding the `leave_room` limit is logged and ignored; the socket is not
-disconnected for this specific limit.
+Exceeding the `leave_room` limit is logged and the socket is disconnected.
 
 ## Relayed Room Events
 
@@ -354,7 +353,7 @@ If sender and target are still in the same room, the relay emits:
 - Connections: 10 per IP per minute; excess connections are disconnected.
 - Relayed/events: 50 per socket per 10 seconds; excess disconnects the socket.
 - `get_rooms`: 10 second cooldown per socket plus the event limit.
-- `leave_room`: 10 per socket per minute; excess is ignored.
+- `leave_room`: 10 per socket per minute; excess disconnects the socket.
 - Invalid room passwords: tracked per IP and room. Five recent failures block
   more password attempts for that room until the failure window ages out.
 - HTTP health and admin-metrics endpoints have their own rate limits outside this

package.json

@@ -8,7 +8,8 @@
     "build:extension": "node scripts/build-extension.cjs",
     "lint": "eslint .",
     "lint:fix": "eslint . --fix",
-    "test": "vitest run",
+    "test": "npm run verify",
+    "test:unit": "vitest run",
     "verify": "node scripts/verify-release.mjs"
   },
   "devDependencies": {

scripts/test-rate-limiter.mjs

@@ -4,6 +4,7 @@ import {
     checkEventRate,
     checkHealthRate,
     checkAdminMetricsAuthRate,
+    checkLeaveRoomRate,
     checkAuthRate,
     recordAuthFailure,
     clearRateLimitMaps,
@@ -13,11 +14,13 @@ import {
     healthCounts,
     adminMetricsAuthCounts,
     roomListCooldowns,
+    leaveRoomCounts,
     rateLimitDenied,
     startRateLimitCleanup,
     stopRateLimitCleanup,
     CONNECTION_RATE_LIMIT,
-    EVENT_RATE_LIMIT
+    EVENT_RATE_LIMIT,
+    LEAVE_ROOM_RATE_LIMIT
 } from '../server/rate-limiter.js';
 
 // Helper: mock io for cleanup
@@ -26,7 +29,7 @@ const mockIo = { sockets: { sockets: new Map() } };
 // Reset state before each test group
 function reset() {
     clearRateLimitMaps();
-    Object.assign(rateLimitDenied, { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0 });
+    Object.assign(rateLimitDenied, { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0, leaveRoom: 0 });
     stopRateLimitCleanup();
 }
 
@@ -52,6 +55,16 @@ assert.equal(rateLimitDenied.events, 1);
 reset();
 assert.equal(checkEventRate('sock2'), true, 'separate socket independent');
 
+// --- checkLeaveRoomRate ---
+reset();
+assert.equal(checkLeaveRoomRate('sock-leave-1'), true, 'first leave-room event allowed');
+for (let i = 0; i < LEAVE_ROOM_RATE_LIMIT - 1; i++) checkLeaveRoomRate('sock-leave-1');
+assert.equal(checkLeaveRoomRate('sock-leave-1'), false, `leave-room beyond ${LEAVE_ROOM_RATE_LIMIT}/window blocked`);
+assert.equal(rateLimitDenied.leaveRoom, 1);
+
+reset();
+assert.equal(checkLeaveRoomRate('sock-leave-2'), true, 'separate leave-room socket independent');
+
 // --- checkHealthRate ---
 reset();
 assert.equal(checkHealthRate('1.2.3.4'), true, 'first health check allowed');
@@ -91,12 +104,14 @@ eventCounts.set('sock1', { count: 1, resetTime: Date.now() + 10000 });
 healthCounts.set('ip2', { count: 1, resetTime: Date.now() + 60000 });
 adminMetricsAuthCounts.set('ip3', { count: 1, resetTime: Date.now() + 60000 });
 roomListCooldowns.set('sock2', Date.now());
+leaveRoomCounts.set('sock3', { count: 1, resetTime: Date.now() + 60000 });
 clearRateLimitMaps();
 assert.equal(connectionCounts.size, 0, 'connectionCounts cleared');
 assert.equal(eventCounts.size, 0, 'eventCounts cleared');
 assert.equal(healthCounts.size, 0, 'healthCounts cleared');
 assert.equal(adminMetricsAuthCounts.size, 0, 'adminMetricsAuthCounts cleared');
 assert.equal(roomListCooldowns.size, 0, 'roomListCooldowns cleared');
+assert.equal(leaveRoomCounts.size, 0, 'leaveRoomCounts cleared');
 
 // --- startRateLimitCleanup / stopRateLimitCleanup ---
 reset();
@@ -108,7 +123,9 @@ assert.ok(true, 'cleanup start/stop does not throw');
 // --- rateLimitDenied reset ---
 reset();
 rateLimitDenied.connections = 5;
-Object.assign(rateLimitDenied, { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0 });
+rateLimitDenied.leaveRoom = 5;
+Object.assign(rateLimitDenied, { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0, leaveRoom: 0 });
 assert.equal(rateLimitDenied.connections, 0, 'denial counter resettable');
+assert.equal(rateLimitDenied.leaveRoom, 0, 'leave-room denial counter resettable');
 
 console.log('rate-limiter tests passed');

scripts/test-server-ops.mjs

@@ -52,7 +52,7 @@ const basicHealth = buildHealthPayload({
   now: 1234,
   uptime: 99,
   memoryUsage: () => ({ rss: 10, heapUsed: 5, heapTotal: 8 }),
-  rateLimitSizes: { connections: 1, events: 2, health: 3, adminMetricsAuth: 4, authFailures: 5, roomList: 6 }
+  rateLimitSizes: { connections: 1, events: 2, health: 3, adminMetricsAuth: 4, authFailures: 5, roomList: 6, leaveRoom: 7 }
 });
 
 assert.deepEqual(
@@ -68,7 +68,8 @@ const adminHealth = buildHealthPayload({
   now: 1234,
   uptime: 99,
   memoryUsage: () => ({ rss: 10, heapUsed: 5, heapTotal: 8 }),
-  rateLimitSizes: { connections: 1, events: 2, health: 3, adminMetricsAuth: 4, authFailures: 5, roomList: 6 }
+  rateLimitSizes: { connections: 1, events: 2, health: 3, adminMetricsAuth: 4, authFailures: 5, roomList: 6, leaveRoom: 7 },
+  rateLimitDenied: { leaveRoom: 8 }
 });
 
 assert.equal(adminHealth.peers, 5, 'admin metrics should include aggregate peer count');
@@ -79,8 +80,8 @@ assert.deepEqual(adminHealth.memory, { rss: 10, heapUsed: 5, heapTotal: 8 }, 'ad
 assert.deepEqual(
   adminHealth.rateLimits,
   {
-    trackedClients: { connections: 1, events: 2, health: 3, adminMetricsAuth: 4, authFailures: 5, roomList: 6 },
-    denied: { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0 }
+    trackedClients: { connections: 1, events: 2, health: 3, adminMetricsAuth: 4, authFailures: 5, roomList: 6, leaveRoom: 7 },
+    denied: { connections: 0, events: 0, health: 0, adminMetricsAuth: 0, roomList: 0, leaveRoom: 8 }
   },
   'admin metrics should expose rate-limit tracking and denial counts'
 );

scripts/verify-release.mjs

@@ -7,6 +7,7 @@ import path from 'node:path';
 const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
 
 const checks = [
+  ['vitest unit tests', 'npm', ['run', 'test:unit']],
   ['server ops', 'node', ['scripts/test-server-ops.mjs']],
   ['server routes', 'node', ['scripts/test-server-routes.mjs'], {
     env: { ADMIN_METRICS_TOKEN: 'verify-admin-token-with-more-than-32-chars' }

server/index.js

@@ -22,6 +22,7 @@ import {
     healthCounts,
     adminMetricsAuthCounts,
     roomListCooldowns,
+    leaveRoomCounts,
     rateLimitDenied,
     checkAuthRate,
     recordAuthFailure,
@@ -115,7 +116,8 @@ app.get('/health', (req, res) => {
                 health: healthCounts.size,
                 adminMetricsAuth: adminMetricsAuthCounts.size,
                 authFailures: failedAuthAttempts.size,
-                roomList: roomListCooldowns.size
+                roomList: roomListCooldowns.size,
+                leaveRoom: leaveRoomCounts.size
             },
             rateLimitDenied
         })
@@ -664,6 +666,7 @@ io.on('connection', (socket) => {
     socket.on(EVENTS.LEAVE_ROOM, () => {
         if (!checkLeaveRoomRate(socket.id)) {
             log('SECURITY', `LEAVE_ROOM rate limit exceeded for socket: ${socket.id}`);
+            socket.disconnect(true);
             return;
         }
         try {
@@ -844,6 +847,7 @@ io.on('connection', (socket) => {
     socket.on('disconnect', () => {
         eventCounts.delete(socket.id);
         roomListCooldowns.delete(socket.id);
+        leaveRoomCounts.delete(socket.id);
         const mapping = socketToRoom.get(socket.id);
         if (mapping) {
             try {

server/ops.js

@@ -97,14 +97,16 @@ export function buildHealthPayload({
                 health: rateLimitSizes.health || 0,
                 adminMetricsAuth: rateLimitSizes.adminMetricsAuth || 0,
                 authFailures: rateLimitSizes.authFailures || 0,
-                roomList: rateLimitSizes.roomList || 0
+                roomList: rateLimitSizes.roomList || 0,
+                leaveRoom: rateLimitSizes.leaveRoom || 0
             },
             denied: {
                 connections: rateLimitDenied.connections || 0,
                 events: rateLimitDenied.events || 0,
                 health: rateLimitDenied.health || 0,
                 adminMetricsAuth: rateLimitDenied.adminMetricsAuth || 0,
-                roomList: rateLimitDenied.roomList || 0
+                roomList: rateLimitDenied.roomList || 0,
+                leaveRoom: rateLimitDenied.leaveRoom || 0
             }
         },
         memory: {

server/rate-limiter.js

@@ -164,7 +164,7 @@ export function startRateLimitCleanup(io) {
             }
         }
         for (const [socketId, entry] of leaveRoomCounts.entries()) {
-            if (now > entry.resetTime) {
+            if (now > entry.resetTime || !io.sockets.sockets.has(socketId)) {
                 leaveRoomCounts.delete(socketId);
             }
         }

server/rate-limiter.test.mjs

@@ -4,21 +4,20 @@ import {
     LEAVE_ROOM_RATE_LIMIT,
     LEAVE_ROOM_RATE_WINDOW_MS,
     rateLimitDenied,
-    leaveRoomCounts
+    leaveRoomCounts,
+    clearRateLimitMaps
 } from './rate-limiter.js';
 
 describe('LEAVE_ROOM Rate Limiter', () => {
     const testSocketId = 'test-socket-123';
 
     beforeEach(() => {
-        // Reset state before each test
-        leaveRoomCounts.clear();
+        clearRateLimitMaps();
         rateLimitDenied.leaveRoom = 0;
     });
 
     afterEach(() => {
-        // Clean up after each test
-        leaveRoomCounts.clear();
+        clearRateLimitMaps();
     });
 
     it('should allow LEAVE_ROOM within limit', () => {
@@ -80,19 +79,24 @@ describe('LEAVE_ROOM Rate Limiter', () => {
     });
 
     it('should increment rateLimitDenied counter on block', () => {
-        // Fill up to the limit
         for (let i = 0; i < LEAVE_ROOM_RATE_LIMIT; i++) {
             checkLeaveRoomRate(testSocketId);
         }
 
-        // First block
         checkLeaveRoomRate(testSocketId);
         expect(rateLimitDenied.leaveRoom).toBe(1);
 
-        // Second block
         checkLeaveRoomRate(testSocketId);
         expect(rateLimitDenied.leaveRoom).toBe(2);
     });
+
+    it('should be cleared by the shared reset helper', () => {
+        checkLeaveRoomRate(testSocketId);
+        expect(leaveRoomCounts.size).toBe(1);
+
+        clearRateLimitMaps();
+        expect(leaveRoomCounts.size).toBe(0);
+    });
 });
 
 describe('Rate Limit Constants', () => {