mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 22:03:13 -06:00
fix(#146) P0: auto-quarantine is in-memory + time-limited, never a DB block
The flap limiter's auto-quarantine used to run `UPDATE devices SET blocked = 1` — a PERMANENT, human-cleared block on an automatic trigger. A stuck-then-recovered device stayed dark until someone noticed. - Removed the auto-write from ws/deviceSocket.js. devices.blocked is now written ONLY by an operator (dashboard endpoint / direct SQLite). - Quarantine moved into lib/flap-limiter.js as IN-MEMORY, TIME-LIMITED state: after connectRateQuarantineTrips trips in a window the identity is quarantinedUntil = now + connectRateQuarantineMs (new, default 30m); check() then refuses cheaply with reason:'quarantined' and AUTO-CLEARS when the window passes. Safe in-memory now that Item A ended the restart loop, and a self-healing auto-action must not survive as a DB row. - Log quarantine START once; repeat refusals go through the coalescer. Stale "-> blocked=1" comments updated. - connectRateQuarantineTrips=0 still disables it. Tests: quarantine engages after N trips, refuses cheaply during the window, auto-clears after connectRateQuarantineMs; and an integration flapper is quarantined while devices.blocked stays 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e19a363750
commit
317754376c
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
|
|
|
|||
88
server/test/flap-quarantine.test.js
Normal file
88
server/test/flap-quarantine.test.js
Normal file
|
|
@ -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');
|
||||
});
|
||||
|
|
@ -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 (_) { /* */ } });
|
||||
|
|
|
|||
Loading…
Reference in a new issue