diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f5f6951..14bd8e9 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 376f34a..0e703d7 100644 --- a/docs/CHANGELOG.md +++ b/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. diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 4fcf274..ae01122 100644 --- a/docs/PROTOCOL.md +++ b/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 diff --git a/package.json b/package.json index fb71714..e8b10a7 100644 --- a/package.json +++ b/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": { diff --git a/scripts/test-rate-limiter.mjs b/scripts/test-rate-limiter.mjs index 8cd4dd7..4ee5cb6 100644 --- a/scripts/test-rate-limiter.mjs +++ b/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'); diff --git a/scripts/test-server-ops.mjs b/scripts/test-server-ops.mjs index 5506344..65cec5b 100644 --- a/scripts/test-server-ops.mjs +++ b/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' ); diff --git a/scripts/verify-release.mjs b/scripts/verify-release.mjs index ea7b45e..b8545c2 100644 --- a/scripts/verify-release.mjs +++ b/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' } diff --git a/server/index.js b/server/index.js index 6f822ff..175369c 100644 --- a/server/index.js +++ b/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 { diff --git a/server/ops.js b/server/ops.js index de79c6e..7ac9c22 100644 --- a/server/ops.js +++ b/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: { diff --git a/server/rate-limiter.js b/server/rate-limiter.js index b1959f1..aec4171 100644 --- a/server/rate-limiter.js +++ b/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); } } diff --git a/server/rate-limiter.test.mjs b/server/rate-limiter.test.mjs index 5109bbf..9a3bb2c 100644 --- a/server/rate-limiter.test.mjs +++ b/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', () => {