Scope device serialization to what each endpoint actually needs

A device row carries two fields that are not ordinary data: device_token, the
credential the player proves with on the /device socket, and settings_pin, which
unlocks the player's on-device settings menu and so hands physical control of the
panel to anyone holding it.

device_token was already stripped everywhere. settings_pin was not — it went out on
both the collection and the detail endpoint. The dashboard does show it, but on one
screen only: the device detail page, which fetches a single device. The collection
endpoint had no consumer for it and was returning the PIN for every device in the
workspace on every load.

The detail endpoint keeps it, so that page is unchanged. The list no longer sends it.
Same data, much smaller blast radius, no feature lost.

Tests pin the split in both directions — absent from the list, present on the detail,
and the socket credential absent from both (asserted on the whole serialized payload,
not just the top-level key, so a nested echo would fail too).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
ScreenTinker 2026-07-27 20:40:40 -05:00
parent 59c536c923
commit cbc00515e2
3 changed files with 127 additions and 3 deletions

View file

@ -11,4 +11,20 @@ function stripDeviceSecrets(d) {
return d;
}
module.exports = { stripDeviceSecrets };
// List responses additionally drop `settings_pin`.
//
// The PIN unlocks the player's on-device settings menu (2x Back), i.e. physical control of
// the panel. The dashboard genuinely needs it — but only on ONE screen, the device detail
// page, which fetches a single device via GET /api/devices/:id. The collection endpoint was
// handing out the PIN for EVERY device in the workspace on every load, to every member,
// with no consumer for it. Same data, far wider blast radius, for nothing.
//
// So: detail keeps it (the feature is unchanged), the list does not. If a future list view
// needs the PIN, fetch the device rather than widening this.
function stripDeviceSecretsForList(d) {
const row = stripDeviceSecrets(d);
if (row && typeof row === 'object') delete row.settings_pin;
return row;
}
module.exports = { stripDeviceSecrets, stripDeviceSecretsForList };

View file

@ -5,7 +5,7 @@ const { PLATFORM_ROLES, ELEVATED_ROLES, isPlatformStaff } = require('../middlewa
// Phase 2.2a: workspace-aware access. accessContext returns { workspaceRole, actingAs }
// or null based on the caller's reach into a specific workspace.
const { accessContext } = require('../lib/tenancy');
const { stripDeviceSecrets } = require('../lib/device-sanitize');
const { stripDeviceSecrets, stripDeviceSecretsForList } = require('../lib/device-sanitize');
const { layoutZones, orphanCountsByDevice } = require('../lib/zone-validate');
const deviceSettings = require('../lib/device-settings'); // #150 delete+re-pair settings preservation
@ -45,7 +45,7 @@ router.get('/', (req, res) => {
// #zone-orphan: lightweight per-device count of playlist items whose zone_id isn't in
// the device's active layout, so the dashboard can flag screens that need attention.
const orphanCounts = orphanCountsByDevice(devices.map(d => d.id));
res.json(devices.map(d => ({ ...stripDeviceSecrets(d), orphan_count: orphanCounts[d.id] || 0 })));
res.json(devices.map(d => ({ ...stripDeviceSecretsForList(d), orphan_count: orphanCounts[d.id] || 0 })));
});
// #106: reorder display tiles (cosmetic, within-section). Writes devices.sort_order

View file

@ -0,0 +1,108 @@
'use strict';
// What a device row carries depends on WHICH endpoint returned it.
//
// Two fields on `devices` are not ordinary data:
// device_token — the credential the player proves with on the /device socket. Never
// leaves the server, on any endpoint.
// settings_pin — unlocks the player's on-device settings menu (2x Back), i.e. hands
// someone standing at the panel physical control of it.
//
// The dashboard genuinely shows the PIN, but on exactly one screen — the device detail
// page, which fetches a single device. The collection endpoint has no consumer for it, so
// it does not send it: same data, far smaller blast radius. These tests pin that split so
// a future `SELECT d.*` on either path can't quietly widen it again.
const { test, before, after } = require('node:test');
const assert = require('node:assert/strict');
const { spawn } = require('node:child_process');
const path = require('node:path');
const os = require('node:os');
const fs = require('node:fs');
const crypto = require('node:crypto');
const Database = require('better-sqlite3');
const { freePort } = require('./helpers/free-port');
let PORT, BASE, proc, db;
const DATA_DIR = path.join(os.tmpdir(), 'st-devser-' + crypto.randomBytes(4).toString('hex'));
const LOG = path.join(os.tmpdir(), 'st-devser-' + crypto.randomBytes(4).toString('hex') + '.log');
const PIN = '8675309';
const TOKEN = 'device-token-must-never-ship';
const S = {};
const jfetch = async (p, opts = {}) => {
const res = await fetch(BASE + p, opts);
let body = null; try { body = await res.json(); } catch { /* */ }
return { status: res.status, body };
};
const auth = () => ({ Authorization: 'Bearer ' + S.token, 'Content-Type': 'application/json' });
before(async () => {
PORT = await freePort();
BASE = `http://127.0.0.1:${PORT}`;
const logFd = fs.openSync(LOG, 'w');
proc = spawn('node', ['server.js'], {
cwd: path.join(__dirname, '..'),
env: { ...process.env, DATA_DIR, SELF_HOSTED: 'true', PORT: String(PORT), NODE_ENV: 'test' },
stdio: ['ignore', logFd, logFd],
});
let up = false;
for (let i = 0; i < 80; i++) {
try { const r = await fetch(BASE + '/api/status'); if (r.ok) { up = true; break; } } catch { /* */ }
await new Promise(r => setTimeout(r, 250));
}
if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
db = new Database(path.join(DATA_DIR, 'db', 'remote_display.db'));
const email = 'u' + crypto.randomBytes(5).toString('hex') + '@x.local';
const reg = await jfetch('/api/auth/register', {
method: 'POST', headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ email, password: 'Passw0rd123' }),
});
S.token = reg.body.token;
const me = await jfetch('/api/auth/me', { headers: auth() });
S.wsId = me.body.accessible_workspaces[0].id;
S.deviceId = crypto.randomUUID();
db.prepare(`INSERT INTO devices (id, name, status, workspace_id, settings_pin, device_token, created_at)
VALUES (?, 'pinned panel', 'offline', ?, ?, ?, strftime('%s','now'))`)
.run(S.deviceId, S.wsId, PIN, TOKEN);
});
after(() => { try { db && db.close(); } catch { /* */ } try { proc.kill('SIGKILL'); } catch { /* */ } });
const findOurs = (body) => {
const rows = Array.isArray(body) ? body : (body?.items || body?.devices || []);
return rows.find(d => d.id === S.deviceId);
};
test('the collection endpoint does not hand out every panel\'s settings PIN', async () => {
const r = await jfetch('/api/devices', { headers: auth() });
assert.equal(r.status, 200);
const row = findOurs(r.body);
assert.ok(row, 'our seeded device is in the list');
assert.equal(row.settings_pin, undefined, 'settings_pin must not appear in the list response');
});
test('the detail endpoint still returns the PIN, because the dashboard shows it there', async () => {
const r = await jfetch(`/api/devices/${S.deviceId}`, { headers: auth() });
assert.equal(r.status, 200);
assert.equal(r.body.settings_pin, PIN, 'the device detail page must keep working');
});
test('the device socket credential never leaves the server, on either endpoint', async () => {
const list = await jfetch('/api/devices', { headers: auth() });
const detail = await jfetch(`/api/devices/${S.deviceId}`, { headers: auth() });
assert.equal(findOurs(list.body).device_token, undefined, 'absent from the list');
assert.equal(detail.body.device_token, undefined, 'absent from the detail');
// Belt and braces: the secret must not appear anywhere in either serialized payload.
assert.ok(!JSON.stringify(list.body).includes(TOKEN), 'token string absent from the whole list payload');
assert.ok(!JSON.stringify(detail.body).includes(TOKEN), 'token string absent from the whole detail payload');
});
test('sanitising is non-destructive to ordinary device fields', async () => {
const r = await jfetch('/api/devices', { headers: auth() });
const row = findOurs(r.body);
assert.equal(row.name, 'pinned panel', 'ordinary fields survive');
assert.equal(row.status, 'offline');
assert.ok('orphan_count' in row, 'list-only decorations still applied');
});