mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 13:53:12 -06:00
Apply a saved device snapshot only inside the workspace it was taken in
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
This commit is contained in:
parent
14367af5f1
commit
9c6b80c411
|
|
@ -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); };
|
||||
|
||||
|
|
|
|||
96
server/test/device-settings-workspace-confined.test.js
Normal file
96
server/test/device-settings-workspace-confined.test.js
Normal file
|
|
@ -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 (_) {} });
|
||||
Loading…
Reference in a new issue