Clear a device's OTA rate state when it proves its identity

The update check is deliberately unauthenticated — every client version has to be
able to ask, including old ones that never learned to send a token — and it keys the
rate breaker on the caller-supplied device_id. Keying on IP is not available either:
the fleet SNATs behind one address, so per-IP would collapse a whole site into a
single bucket.

The result was that the bucket belonged to whoever cited the id rather than to the
device that owns it. A handful of requests naming a panel's UUID left that panel in
rate-backoff, un-updatable for up to half an hour at a time and renewable
indefinitely, while every other device stayed healthy.

Rather than adding auth (which would strand old clients) the state is now
self-healing: when a device registers on the /device socket with a valid device_token
its bucket is cleared. Noise is still possible, but it now lasts until the panel's
next genuine reconnect instead of as long as someone keeps poking.

This is not an escape hatch from the breaker's real job. A device stuck in an update
loop is re-registering legitimately, and clearing its rate state on each genuine
reconnect is what a healthy device looks like; the loop protection that matters is
the download guard. The version-keyed bucket that covers old clients sending only
?version= is a separate namespace and is deliberately not reachable this way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

server/lib/ota-breaker.js

@@ -129,6 +129,29 @@ function startSweep() {
 }
 
 function reset() { state.clear(); loggedBad.clear(); Object.assign(rateBackoffCtr, rollingCounter()); }
+
+// Forgive ONE device's rate state, called when that device proves its identity on the /device
+// socket (a valid device_token, timing-safe compared).
+//
+// /api/update/check is deliberately unauthenticated — every client version, including old ones
+// that never learned to send a token, has to be able to ask. That means `?device_id=` is
+// caller-supplied, so anyone who learns a device's UUID can burn its bucket with a handful of
+// requests and leave the REAL device in rate-backoff, silently un-updatable, for up to 30
+// minutes at a time.
+//
+// Adding auth to the check would strand old clients, and keying on IP is wrong here (the fleet
+// SNATs — see the note at the top of this file). So instead the poisoning is made
+// self-healing: the genuine device reconnects on its own schedule, proves who it is, and gets
+// its bucket cleared. An attacker can still cause noise, but the denial now lasts until the
+// device's next authenticated reconnect rather than as long as the attacker keeps poking.
+//
+// This cannot be used to evade the breaker's real job: a device stuck in an OTA loop is
+// re-registering legitimately, and clearing its rate state on each genuine reconnect is exactly
+// what a healthy device looks like — the loop protection is the DOWNLOAD guard, not this.
+function forgiveDevice(deviceId) {
+  if (!deviceId) return false;
+  return state.delete('d:' + deviceId);
+}
 function _size() { return state.size; }
 // #146 observability — how many update checks the breaker is rate-backing-off (total +
 // last completed window). A device=none 1.8.x flood shows here as rateBackoffLastWindow.
@@ -136,4 +159,4 @@ function stats(now = Date.now()) {
   const rb = read(rateBackoffCtr, now);
   return { rateBackoffTotal: rb.total, rateBackoffLastWindow: rb.lastWindow };
 }
-module.exports = { decide, reset, sweep, startSweep, cmp, parseVer, _size, stats, WINDOW_MS, THRESHOLD };
+module.exports = { decide, reset, forgiveDevice, sweep, startSweep, cmp, parseVer, _size, stats, WINDOW_MS, THRESHOLD };

server/test/ota-breaker-device-forgive.test.js

@@ -0,0 +1,94 @@
+'use strict';
+
+// The OTA rate-breaker's bucket must not be a lever anyone can pull against someone else's panel.
+//
+// `/api/update/check` is deliberately unauthenticated — every client version has to be able to
+// ask, including old ones that never learned to send a token — and it keys the breaker on the
+// caller-supplied `?device_id=`. Keying on IP is not an option either: the fleet SNATs behind one
+// address, so per-IP would collapse a whole site into one bucket (see lib/ota-breaker.js).
+//
+// The consequence was that anyone who learned a panel's UUID could burn its bucket with a handful
+// of requests and leave the REAL panel in rate-backoff — silently un-updatable, for up to 30
+// minutes at a time, renewable indefinitely.
+//
+// The containment is to make it self-healing rather than to add auth: when a device proves its
+// identity on the /device socket (device_token, timing-safe compared) its bucket is cleared. An
+// attacker can still make noise, but the denial now lasts until the panel's next genuine
+// reconnect instead of as long as the attacker keeps poking.
+
+const { test, beforeEach } = require('node:test');
+const assert = require('node:assert/strict');
+const breaker = require('../lib/ota-breaker');
+
+const LATEST = '9.9.9';      // always newer than the client, so checks are "offerable"
+const OLD = '1.0.0';
+
+beforeEach(() => breaker.reset());
+
+// Drive the breaker until it starts refusing, mirroring what the route does.
+function hammer(deviceId, n) {
+  let last = null;
+  for (let i = 0; i < n; i++) last = breaker.decide(OLD, LATEST, deviceId);
+  return last;
+}
+
+test('THE BUG: rapid checks against a device id trip that device into rate-backoff', () => {
+  const victim = 'victim-device-uuid';
+  const verdict = hammer(victim, breaker.THRESHOLD + 2);
+  assert.equal(verdict.reason, 'rate-backoff', 'the bucket trips');
+  assert.equal(verdict.update_available, false);
+  assert.ok(verdict.retry_after_seconds > 0, 'and reports a cooldown');
+});
+
+test('the poisoning is TARGETED — only the named device is affected', () => {
+  const victim = 'victim-device-uuid';
+  hammer(victim, breaker.THRESHOLD + 2);
+  const bystander = breaker.decide(OLD, LATEST, 'some-other-device');
+  assert.notEqual(bystander.reason, 'rate-backoff', 'an unrelated device is unaffected');
+  assert.equal(bystander.update_available, true, 'and is still offered its update');
+});
+
+test('the device proving its identity clears the backoff', () => {
+  const victim = 'victim-device-uuid';
+  assert.equal(hammer(victim, breaker.THRESHOLD + 2).reason, 'rate-backoff', 'poisoned first');
+
+  const cleared = breaker.forgiveDevice(victim);
+  assert.equal(cleared, true, 'the bucket existed and was cleared');
+
+  const after = breaker.decide(OLD, LATEST, victim);
+  assert.notEqual(after.reason, 'rate-backoff', 'the real device is no longer refused');
+  assert.equal(after.update_available, true, 'and is offered its update again');
+});
+
+test('forgiving one device does not clear anyone else', () => {
+  hammer('device-a', breaker.THRESHOLD + 2);
+  hammer('device-b', breaker.THRESHOLD + 2);
+  breaker.forgiveDevice('device-a');
+  assert.equal(breaker.decide(OLD, LATEST, 'device-b').reason, 'rate-backoff',
+    'device-b keeps its own state');
+});
+
+test('forgiving is safe for ids that were never seen, and for no id at all', () => {
+  assert.equal(breaker.forgiveDevice('never-checked-in'), false, 'returns false rather than throwing');
+  assert.equal(breaker.forgiveDevice(null), false);
+  assert.equal(breaker.forgiveDevice(undefined), false);
+  assert.equal(breaker.forgiveDevice(''), false);
+});
+
+test('a version-keyed bucket (no device_id) is NOT reachable by forgiveDevice', () => {
+  // Old clients send only ?version=, so they share a per-version bucket. That one protects the
+  // server from a fleet of stuck legacy clients and must not be clearable by device id.
+  const v = hammer(null, breaker.THRESHOLD + 2);
+  assert.equal(v.reason, 'rate-backoff', 'the version bucket trips');
+  breaker.forgiveDevice(OLD);                 // the version string is not a device id
+  assert.equal(breaker.decide(OLD, LATEST, null).reason, 'rate-backoff',
+    'the version-keyed bucket survives — it is a different namespace');
+});
+
+test('the breaker still does its real job after a forgive', () => {
+  const d = 'looping-device';
+  breaker.forgiveDevice(d);
+  // A device that genuinely loops still gets stopped; forgiving is not an escape hatch.
+  assert.equal(hammer(d, breaker.THRESHOLD + 2).reason, 'rate-backoff',
+    'a looping client is still rate-limited');
+});

server/ws/deviceSocket.js

@@ -731,6 +731,11 @@ module.exports = function setupDeviceSocket(io) {
             persistIdentity(device_id, data);         // change-detected write (see persistIdentity)
           }
           socket.join(device_id);
+          // The device just proved its identity (device_token, timing-safe). Clear any OTA
+          // rate-backoff held against it: /api/update/check is unauthenticated and takes a
+          // caller-supplied ?device_id=, so that bucket can have been burned by anyone who
+          // merely knows this UUID. A genuine reconnect is the proof that lets us forgive it.
+          try { require('../lib/ota-breaker').forgiveDevice(device_id); } catch (_) { /* non-fatal */ }
           socket.emit('device:registered', { device_id, device_token: tokenToSend, status: 'online' });
           // #143: a device paired/claimed server-side (user_id set) that RECONNECTS must be told
           // it's paired — the app leaves the Connect page ONLY on 'device:paired' (web: hides the