From 9c6b80c411c6a0f0e01477ce105a11e254e257c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 20:56:59 -0500 Subject: [PATCH] Apply a saved device snapshot only inside the workspace it was taken in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-device settings are saved against the hardware fingerprint so a panel that is deleted and paired again comes back configured — name, orientation, playlist, blocked flag — without anyone visiting it. That is deliberate and worth keeping. A fingerprint is hardware-derived, so the same physical panel presents the same one whoever pairs it. applyToDevice looked the snapshot up on fingerprint alone with no workspace comparison, and its per-field guards only check that the referenced row still EXISTS, never who it belongs to: if (s.playlist_id && db.prepare('SELECT 1 FROM playlists WHERE id = ?').get(s.playlist_id)) So a screen removed from one workspace and paired into another inherited the first workspace's playlist and displayed its content, and `blocked` crossed the same way — a device arriving blocked with nothing the new owner could see to explain it. The manual restore route already compares workspaces before calling this, so the automatic re-pair path was the only place the check was missing. A mismatch is a quiet no-op rather than an error: re-pairing a second-hand panel into a different workspace is a legitimate thing to do, it just must not carry the previous configuration along. A snapshot with no workspace recorded still applies, so rows predating the column keep working. 5 tests: neither playlist nor block crosses, a mismatch does not throw, restore still works in full inside the owning workspace (including a genuine block surviving a re-pair), and legacy rows are unaffected. 882 server tests green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- server/lib/device-settings.js | 16 ++++ ...device-settings-workspace-confined.test.js | 96 +++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 server/test/device-settings-workspace-confined.test.js diff --git a/server/lib/device-settings.js b/server/lib/device-settings.js index fcb147d..bcec3b4 100644 --- a/server/lib/device-settings.js +++ b/server/lib/device-settings.js @@ -75,6 +75,22 @@ function snapshot(deviceId, now = Math.floor(Date.now() / 1000)) { function applyToDevice(deviceId, fingerprint) { const s = db.prepare('SELECT * FROM device_settings WHERE fingerprint = ?').get(fingerprint); if (!s) return null; + + // A snapshot only ever applies inside the workspace it was taken in. + // + // The lookup keys on fingerprint alone, and a fingerprint is hardware-derived: the same panel + // moved between customers presents the same one. Without this comparison, a screen deleted from + // one workspace and paired into another inherited the FIRST workspace's playlist_id, blocked flag + // and team_id — and the per-field guards below did not stop it, because they only check that the + // referenced row still exists, never who it belongs to. The manual restore route already compares + // workspaces before calling this (routes/devices.js), so the automatic re-pair path was the one + // place the check was missing. + // + // Mismatch is a no-op, not an error: re-pairing a second-hand panel into a new workspace is a + // legitimate thing to do, it just must not drag the previous owner's configuration along. + const dev = db.prepare('SELECT workspace_id FROM devices WHERE id = ?').get(deviceId); + if (!dev) return null; + if (s.workspace_id && dev.workspace_id && s.workspace_id !== dev.workspace_id) return null; const sets = [], vals = []; const put = (col, val) => { sets.push(`${col} = ?`); vals.push(val); }; diff --git a/server/test/device-settings-workspace-confined.test.js b/server/test/device-settings-workspace-confined.test.js new file mode 100644 index 0000000..7c4522c --- /dev/null +++ b/server/test/device-settings-workspace-confined.test.js @@ -0,0 +1,96 @@ +'use strict'; + +// Per-device settings are saved against the hardware FINGERPRINT so a panel that is deleted and +// paired again comes back configured — its name, orientation, playlist and blocked flag restored +// without anyone visiting it. That is deliberate and useful. +// +// A fingerprint is hardware-derived, so the same physical panel presents the same one no matter +// whose account it is paired into. applyToDevice looked the snapshot up on fingerprint alone with +// no workspace comparison, and its per-field guards only check that the referenced row still +// EXISTS, never who it belongs to: +// +// if (s.playlist_id && db.prepare('SELECT 1 FROM playlists WHERE id = ?').get(s.playlist_id)) +// +// So a screen removed from one workspace and paired into another inherited the first workspace's +// playlist and started displaying its content. `blocked` crossed the same way, giving a device that +// arrives blocked for no reason the new owner can see. The manual restore route already compares +// workspaces before calling this, so the automatic re-pair path was the one place it was missing. +// +// The invariant: a saved snapshot only ever applies inside the workspace it was taken in. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'st-ws-confine-')); +process.env.DATA_DIR = tmp; + +const { db } = require('../db/database'); +const deviceSettings = require('../lib/device-settings'); + +function seedWorkspace(tag) { + const u = 'u-' + tag, o = 'o-' + tag, ws = 'ws-' + tag, pl = 'pl-' + tag; + db.prepare("INSERT OR IGNORE INTO users (id,email,password_hash) VALUES (?,?, 'x')").run(u, tag + '@t.local'); + db.prepare('INSERT OR IGNORE INTO organizations (id,name,owner_user_id) VALUES (?,?,?)').run(o, 'org ' + tag, u); + db.prepare('INSERT OR IGNORE INTO workspaces (id,organization_id,name) VALUES (?,?,?)').run(ws, o, 'ws ' + tag); + db.prepare('INSERT OR IGNORE INTO playlists (id,name,workspace_id,user_id) VALUES (?,?,?,?)').run(pl, 'PL ' + tag, ws, u); + return { u, ws, pl }; +} + +const A = seedWorkspace('alpha'); +const B = seedWorkspace('bravo'); +const FP = 'hardware-fingerprint-shared'; + +function makeDevice(id, ws) { + db.prepare(`INSERT OR REPLACE INTO devices (id,name,workspace_id,created_at,updated_at) + VALUES (?,?,?,strftime('%s','now'),strftime('%s','now'))`).run(id, 'Screen', ws); + return id; +} +const deviceRow = (id) => db.prepare('SELECT * FROM devices WHERE id = ?').get(id); + +// The panel lived in workspace A: named, assigned A's playlist, and blocked there. +db.prepare(`INSERT OR REPLACE INTO device_settings (fingerprint, workspace_id, device_name, playlist_id, blocked, last_seen) + VALUES (?,?, 'Lobby Screen', ?, 1, strftime('%s','now'))`).run(FP, A.ws, A.pl); + +test('THE LEAK: a panel paired into another workspace does not inherit the first ones playlist', () => { + const dev = makeDevice('dev-in-B', B.ws); + deviceSettings.applyToDevice(dev, FP); + const d = deviceRow(dev); + assert.equal(d.playlist_id, null, "workspace B's screen must not be playing workspace A's content"); + assert.equal(d.workspace_id, B.ws, 'and it must stay in its own workspace'); +}); + +test('a block from another workspace does not follow the hardware either', () => { + const dev = makeDevice('dev-block-B', B.ws); + deviceSettings.applyToDevice(dev, FP); + assert.equal(deviceRow(dev).blocked, 0, 'arriving blocked with nothing to explain it is unactionable'); +}); + +test('a mismatch is a quiet no-op, because re-pairing a second-hand panel is legitimate', () => { + // It must not throw or refuse the pairing — only decline to carry the old configuration. + const dev = makeDevice('dev-noop-B', B.ws); + assert.doesNotThrow(() => deviceSettings.applyToDevice(dev, FP)); + assert.equal(deviceRow(dev).name, 'Screen', 'the name from the other workspace must not be applied'); +}); + +test('AND THE POINT OF THE FEATURE: restore still works inside the owning workspace', () => { + // The whole reason this exists — a panel re-paired at home comes back configured. + const dev = makeDevice('dev-in-A', A.ws); + deviceSettings.applyToDevice(dev, FP); + const d = deviceRow(dev); + assert.equal(d.playlist_id, A.pl, 'its own playlist must be restored'); + assert.equal(d.name, 'Lobby Screen', 'its own name must be restored'); + assert.equal(d.blocked, 1, 'and a genuine block must still survive a re-pair'); +}); + +test('a snapshot with no workspace recorded is still applied, so legacy rows keep working', () => { + db.prepare(`INSERT OR REPLACE INTO device_settings (fingerprint, workspace_id, device_name, blocked, last_seen) + VALUES ('legacy-fp', NULL, 'Legacy Name', 0, strftime('%s','now'))`).run(); + const dev = makeDevice('dev-legacy', B.ws); + deviceSettings.applyToDevice(dev, 'legacy-fp'); + assert.equal(deviceRow(dev).name, 'Legacy Name'); +}); + +test.after(() => { try { fs.rmSync(tmp, { recursive: true, force: true }); } catch (_) {} });