mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-29 09:23:16 -06:00
fix(#142): provisioning-row cleanup window 365d -> 24h (matches its own comment)
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>
This commit is contained in:
parent
15448d1c5d
commit
139d7d09fa
|
|
@ -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
|
||||
};
|
||||
|
|
|
|||
41
server/test/provisioning-cleanup.test.js
Normal file
41
server/test/provisioning-cleanup.test.js
Normal file
|
|
@ -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);
|
||||
});
|
||||
Loading…
Reference in a new issue