diff --git a/server/config.js b/server/config.js index f90c7af..5311195 100644 --- a/server/config.js +++ b/server/config.js @@ -144,9 +144,11 @@ module.exports = { connectRateAnonMax: parseInt(process.env.CONNECT_RATE_ANON_MAX) || 60, // the shared global anon bucket, higher (collective) connectRateCooldownMs: parseInt(process.env.CONNECT_RATE_COOLDOWN_MS) || 60000, // refuse window after a trip connectRateIdleMs: parseInt(process.env.CONNECT_RATE_IDLE_MS) || 2 * 300000, // sweep buckets idle this long - // after this many trips in a window a device_id-resolved flapper is auto-quarantined - // (devices.blocked=1) so it stops entirely instead of being refused forever. 0=off. + // after this many trips within a window the identity is auto-quarantined — an + // IN-MEMORY, TIME-LIMITED refusal (NOT a DB block; the devices.blocked column is only + // ever written by an operator). Self-heals after connectRateQuarantineMs. 0 = off. connectRateQuarantineTrips: parseInt(process.env.CONNECT_RATE_QUARANTINE_TRIPS) || 5, + connectRateQuarantineMs: parseInt(process.env.CONNECT_RATE_QUARANTINE_MS) || 30 * 60 * 1000, // Cold start: for this long after process start, lag is high while the whole // fleet reconnects at once. Treat leniently — force the 'normal' band and apply // only the hard ceiling (no rate-band throttle) so a deploy can't throttle diff --git a/server/lib/flap-limiter.js b/server/lib/flap-limiter.js index 0221334..07b4b31 100644 --- a/server/lib/flap-limiter.js +++ b/server/lib/flap-limiter.js @@ -14,26 +14,41 @@ // to wipe this state every ~40s before it could bite. Bounded: an idle sweep evicts // stale buckets and the anonymous fallback is a single shared bucket (an anon flood is // capped collectively, never one-bucket-per-attacker growth). +// +// #146 P0: a hard flapper (connectRateQuarantineTrips trips in a window) is QUARANTINED +// in-memory for connectRateQuarantineMs — a cheap, auto-clearing refusal, NOT a DB block. +// The devices.blocked column is the operator's deliberate, durable lever and is never +// written automatically. const config = require('../config'); const { ANON_KEY } = require('./device-identity'); -// key -> { hits: number[], blockedUntil: ms, lastSeen: ms, trips: number, tripWinStart: ms } +// key -> { hits: number[], blockedUntil, lastSeen, trips, tripWinStart, quarantinedUntil } const state = new Map(); function maxFor(key) { return key === ANON_KEY ? config.connectRateAnonMax : config.connectRateMax; } // Decide whether to allow this connection for `key`. Returns // { allow: true } -// { allow: false, retryAfterMs, reason, tripped, trips } // tripped=true on the trip edge +// { allow: false, retryAfterMs, reason, tripped?, trips?, quarantined? } +// reason: 'quarantined' (in-memory time-limited), 'flap-cooldown' (post-trip), 'flap-rate' +// (the trip edge). `quarantined:true` marks the START of a quarantine (log once). function check(key, now = Date.now()) { let s = state.get(key); - if (!s) { s = { hits: [], blockedUntil: 0, lastSeen: now, trips: 0, tripWinStart: now }; state.set(key, s); } + if (!s) { s = { hits: [], blockedUntil: 0, lastSeen: now, trips: 0, tripWinStart: now, quarantinedUntil: 0 }; state.set(key, s); } s.lastSeen = now; + // #146 P0: quarantine is an IN-MEMORY, TIME-LIMITED refusal that AUTO-CLEARS — never a + // DB block. A stuck-then-recovered device comes back on its own after the window. This + // is safe in-memory now that Item A ended the prune-induced restart loop; a + // self-healing auto-action must NOT survive as a devices.blocked row. + if (now < s.quarantinedUntil) { + return { allow: false, retryAfterMs: s.quarantinedUntil - now, reason: 'quarantined' }; + } + // Inside an enforced cooldown -> refuse cheaply. if (now < s.blockedUntil) { - return { allow: false, retryAfterMs: s.blockedUntil - now, reason: 'flap-cooldown', tripped: false, trips: s.trips }; + return { allow: false, retryAfterMs: s.blockedUntil - now, reason: 'flap-cooldown' }; } // Sliding window of genuine connects. @@ -41,12 +56,15 @@ function check(key, now = Date.now()) { s.hits.push(now); if (s.hits.length > maxFor(key)) { - // Trip: enter a cooldown, clear the window (a fresh burst must re-accumulate). - s.blockedUntil = now + config.connectRateCooldownMs; + s.blockedUntil = now + config.connectRateCooldownMs; // cooldown; a fresh burst must re-accumulate s.hits = []; - // Count trips within a window for optional auto-quarantine (Item D). if (now - s.tripWinStart > config.connectRateWindowMs) { s.tripWinStart = now; s.trips = 0; } s.trips += 1; + // Escalate to a time-limited quarantine after N trips in the window (0 = off). + if (config.connectRateQuarantineTrips > 0 && s.trips >= config.connectRateQuarantineTrips) { + s.quarantinedUntil = now + config.connectRateQuarantineMs; + return { allow: false, retryAfterMs: config.connectRateQuarantineMs, reason: 'flap-rate', tripped: true, trips: s.trips, quarantined: true }; + } return { allow: false, retryAfterMs: config.connectRateCooldownMs, reason: 'flap-rate', tripped: true, trips: s.trips }; } return { allow: true }; diff --git a/server/test/flap-quarantine.test.js b/server/test/flap-quarantine.test.js new file mode 100644 index 0000000..3b2068d --- /dev/null +++ b/server/test/flap-quarantine.test.js @@ -0,0 +1,88 @@ +'use strict'; + +// #146 P0 — auto-quarantine is IN-MEMORY + TIME-LIMITED, never a DB block. It engages +// after N trips, refuses cheaply during the window, AUTO-CLEARS, and must NEVER write +// devices.blocked (that column is the operator's lever only). + +const os = require('node:os'); +const path = require('node:path'); +const crypto = require('node:crypto'); +process.env.DATA_DIR = path.join(os.tmpdir(), 'st-flapq-' + crypto.randomBytes(4).toString('hex')); +process.env.SELF_HOSTED = 'true'; +process.env.NODE_ENV = 'test'; +process.env.CONNECT_RATE_MAX = '2'; +process.env.CONNECT_RATE_WINDOW_MS = '5000'; +process.env.CONNECT_RATE_COOLDOWN_MS = '50'; +process.env.CONNECT_RATE_QUARANTINE_TRIPS = '2'; +process.env.CONNECT_RATE_QUARANTINE_MS = '50000'; + +const { test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const http = require('node:http'); +const { Server } = require('socket.io'); +const ioClient = require('socket.io-client'); +const flap = require('../lib/flap-limiter'); +const { db } = require('../db/database'); +const setupDeviceSocket = require('../ws/deviceSocket'); + +// --- unit: deterministic injected-time behaviour --- +test('quarantine engages after N trips, refuses cheaply, and AUTO-CLEARS', () => { + flap.reset(); + const k = 'd:q'; + // trip 1: exceed max (2) -> trip, cooldown to 250 + flap.check(k, 0); flap.check(k, 100); + let r = flap.check(k, 200); + assert.equal(r.tripped, true); assert.ok(!r.quarantined, 'first trip is not yet a quarantine'); + // past cooldown, trip 2 -> QUARANTINE (trips=2) + flap.check(k, 300); flap.check(k, 400); + r = flap.check(k, 500); + assert.equal(r.quarantined, true, 'quarantine engages on the Nth trip'); + assert.equal(r.allow, false); + // during the window: cheap 'quarantined' refusal + const during = flap.check(k, 2000); + assert.equal(during.allow, false); + assert.equal(during.reason, 'quarantined'); + // auto-clears once now passes quarantinedUntil (500 + 50000); old hits have aged out + const after = flap.check(k, 51000); + assert.equal(after.allow, true, 'quarantine auto-clears — device comes back on its own, no restart'); +}); + +// --- integration: a flapping device is quarantined but devices.blocked stays 0 --- +let httpServer, io, base; +before(async () => { + db.pragma('foreign_keys = OFF'); + db.prepare("INSERT INTO devices (id, device_token, status, blocked) VALUES ('flap-dev', 'tok', 'offline', 0)").run(); + db.pragma('foreign_keys = ON'); + httpServer = http.createServer(); io = new Server(httpServer); setupDeviceSocket(io); + await new Promise((r) => httpServer.listen(0, r)); + base = `http://127.0.0.1:${httpServer.address().port}`; +}); +after(() => { try { io.close(); } catch { /* */ } try { httpServer.close(); } catch { /* */ } }); + +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +function reg() { + return new Promise((resolve) => { + const s = ioClient(`${base}/device`, { transports: ['websocket'], reconnection: false, forceNew: true }); + let done = false; const fin = (r) => { if (done) return; done = true; try { s.close(); } catch { /* */ } resolve(r); }; + s.on('connect', () => s.emit('device:register', { device_id: 'flap-dev', device_token: 'tok' })); + s.on('device:registered', () => fin({ registered: true })); + s.on('device:throttled', (m) => fin({ throttled: true, reason: m && m.reason })); + setTimeout(() => fin({ timeout: true }), 1500); + }); +} + +test('a flapping device gets quarantined, but devices.blocked is NEVER auto-written', async () => { + flap.reset(); + let throttled = 0; + // two cooldown-separated bursts -> 2 trips -> quarantine + for (let burst = 0; burst < 2; burst++) { + for (let i = 0; i < 3; i++) { const r = await reg(); if (r.throttled) throttled++; } + await sleep(80); // let the short cooldown expire so the next burst counts as a fresh trip + } + // a final attempt should now be refused by the quarantine + const q = await reg(); + assert.ok(throttled >= 1 || q.throttled, 'the flapper was refused (flap/quarantine bit)'); + + const row = db.prepare("SELECT blocked FROM devices WHERE id = 'flap-dev'").get(); + assert.equal(row.blocked, 0, 'auto-quarantine NEVER wrote devices.blocked — operator lever untouched'); +}); diff --git a/server/ws/deviceSocket.js b/server/ws/deviceSocket.js index 7899b9b..bc6f0e9 100644 --- a/server/ws/deviceSocket.js +++ b/server/ws/deviceSocket.js @@ -324,11 +324,14 @@ module.exports = function setupDeviceSocket(io) { if (!isRefreshConnect) { const fv = flapLimiter.check(ident.key); if (!fv.allow) { - console.warn(`[flap] refused ${ident.kind} ${ident.deviceId || ident.key} reason=${fv.reason} retry=${fv.retryAfterMs}ms trips=${fv.trips || 0}`); - // Optional auto-quarantine (Item D): a device_id-resolved HARD flapper is - // blocked=1 so it stops entirely rather than being refused forever. - if (fv.tripped && ident.deviceId && config.connectRateQuarantineTrips > 0 && (fv.trips || 0) >= config.connectRateQuarantineTrips) { - try { db.prepare('UPDATE devices SET blocked = 1 WHERE id = ?').run(ident.deviceId); console.warn(`[flap] auto-quarantined ${ident.deviceId} after ${fv.trips} trips (blocked=1)`); } catch (_) { /* */ } + // #146 P0: auto-quarantine is IN-MEMORY + TIME-LIMITED (lib/flap-limiter), + // never a DB block — a stuck-then-recovered device self-heals. The + // devices.blocked column is now written ONLY by an operator. Log the + // quarantine START once; coalesce the repeat refusals. + if (fv.quarantined) { + console.warn(`[flap] quarantined ${ident.deviceId || ident.key} for ${Math.round(config.connectRateQuarantineMs / 60000)}m after ${fv.trips} trips`); + } else { + logCoalescer.record(`flap-refused:${ident.key}`, `[flap] refused ${ident.kind} ${ident.deviceId || ident.key} reason=${fv.reason}`); } socket.emit('device:throttled', { retry_after_ms: fv.retryAfterMs, reason: 'connect_rate' }); process.nextTick(() => { try { socket.disconnect(true); } catch (_) { /* */ } });