From 139d7d09faf8e983853ae0ce11a81092ae3f0330 Mon Sep 17 00:00:00 2001
From: ScreenTinker <hello@screentinker.com>
Date: Sat, 27 Jun 2026 19:56:32 -0500
Subject: [PATCH] fix(#142): provisioning-row cleanup window 365d -> 24h
 (matches its own comment)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

services/heartbeat.js deleted unclaimed provisioning devices with
created_at < now - (365 * 86400) — a YEAR — while its own comment said "older
than 24 hours". So socket-register pairing junk lingered ~365x longer than
intended. Change the window to 24 * 3600 to match the comment.

Correctness fix only — does NOT touch the pre-auth register path or add a rate
limiter (that pre-auth hardening is a separate security issue, out of this cut).

Extracted the sweep into pruneProvisioningDevices() (still in heartbeat.js, called
from the same interval) so it is unit-testable. Test asserts a >24h unclaimed
provisioning row is swept while a <24h row, an imported row (user_id set), and a
non-provisioning row are kept.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 server/services/heartbeat.js             | 25 ++++++++++-----
 server/test/provisioning-cleanup.test.js | 41 ++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 8 deletions(-)
 create mode 100644 server/test/provisioning-cleanup.test.js

diff --git a/server/services/heartbeat.js b/server/services/heartbeat.js
index 7d0778e..441b79c 100644
--- a/server/services/heartbeat.js
+++ b/server/services/heartbeat.js
@@ -40,13 +40,8 @@ function startHeartbeatChecker(io) {
       }
     }
 
-    // Cleanup: delete unclaimed provisioning devices older than 24 hours
-    // Keep imported devices (they have user_id set) so users can re-pair them
-    db.prepare(`
-      DELETE FROM devices WHERE status = 'provisioning'
-      AND user_id IS NULL
-      AND created_at < strftime('%s','now') - (365 * 86400)
-    `).run();
+    // Cleanup: delete unclaimed provisioning devices older than 24 hours.
+    pruneProvisioningDevices();
 
     // Cleanup: prune play logs older than 90 days
     db.prepare(`
@@ -91,11 +86,25 @@ function getAllConnections() {
   return deviceConnections;
 }
 
+// #142: sweep unclaimed provisioning devices older than 24h. The window previously
+// read `365 * 86400` (a YEAR), contradicting its own "older than 24 hours" comment,
+// so socket-register pairing junk lingered far longer than intended. Imported
+// devices keep a user_id and are preserved so they can be re-paired. Extracted from
+// the interval above so the correctness fix is unit-testable. Returns rows deleted.
+function pruneProvisioningDevices() {
+  return db.prepare(`
+    DELETE FROM devices
+    WHERE status = 'provisioning' AND user_id IS NULL
+    AND created_at < strftime('%s','now') - (24 * 3600)
+  `).run().changes;
+}
+
 module.exports = {
   startHeartbeatChecker,
   registerConnection,
   updateHeartbeat,
   removeConnection,
   getConnection,
-  getAllConnections
+  getAllConnections,
+  pruneProvisioningDevices
 };
diff --git a/server/test/provisioning-cleanup.test.js b/server/test/provisioning-cleanup.test.js
new file mode 100644
index 0000000..edcbefb
--- /dev/null
+++ b/server/test/provisioning-cleanup.test.js
@@ -0,0 +1,41 @@
+'use strict';
+
+// #142 (cut 2) — provisioning-row cleanup window correctness. The sweep deletes
+// UNCLAIMED provisioning devices older than 24h (it previously used 365*86400 — a
+// year — contradicting its own comment). Imported devices (user_id set) and
+// non-provisioning devices are preserved. Deterministic, in-process (no server).
+
+const os = require('node:os');
+const path = require('node:path');
+const crypto = require('node:crypto');
+process.env.DATA_DIR = path.join(os.tmpdir(), 'st-provclean-' + crypto.randomBytes(4).toString('hex'));
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const { db } = require('../db/database');
+const { pruneProvisioningDevices } = require('../services/heartbeat');
+
+test('sweeps unclaimed provisioning devices older than 24h, keeps the rest', () => {
+  db.pragma('foreign_keys = OFF'); // seed user_id without a real users row
+  db.exec('DELETE FROM devices');
+  const ins = db.prepare("INSERT INTO devices (id, status, user_id, created_at) VALUES (?, ?, ?, strftime('%s','now') - ?)");
+  ins.run('old-unclaimed', 'provisioning', null, 25 * 3600);   // >24h, unclaimed  -> SWEPT
+  ins.run('new-unclaimed', 'provisioning', null, 1 * 3600);    // <24h, unclaimed  -> kept
+  ins.run('old-imported', 'provisioning', 'u-imported', 25 * 3600); // >24h but imported (user_id) -> kept
+  ins.run('old-online', 'online', null, 25 * 3600);           // >24h but not provisioning -> kept
+  db.pragma('foreign_keys = ON');
+
+  assert.equal(db.prepare('SELECT COUNT(*) c FROM devices').get().c, 4, 'seeded 4');
+
+  const deleted = pruneProvisioningDevices();
+  assert.equal(deleted, 1, 'only the >24h unclaimed provisioning device is swept');
+
+  const ids = db.prepare('SELECT id FROM devices ORDER BY id').all().map(r => r.id);
+  assert.deepEqual(ids, ['new-unclaimed', 'old-imported', 'old-online']);
+  // regression guard: a 25h-old row sits well inside the OLD 365-day window, so this
+  // would have survived before the fix.
+});
+
+test('idempotent: a second sweep with nothing stale deletes nothing', () => {
+  assert.equal(pruneProvisioningDevices(), 0);
+});