From 3e37d33b80ee53b3ecf1bf45a0b9e223d1ca44e7 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Thu, 6 Aug 2026 16:12:29 -0500 Subject: [PATCH] QA: close four ways a control or an asset lied about itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by driving the real server and a real browser, not by reading. Each fix has a test that fails without it. 1. A missing upload answered 200 with the DASHBOARD. express.static falls through on a miss and the SPA catch-all caught it, so GET /uploads/content/.mp4 returned 15KB of index.html as text/html — under the `immutable, max-age=30d` header the mount sets before it knows the file exists. Every player downloader treats 200 as success, so a panel stores the HTML page AS the video and caches it for a month, rendering a black frame with nothing in any log. Reachable exactly when it hurts: a content replace writes a new random filename and unlinks the old one. The mount now terminates a miss with a 404 and drops the cache header. 2. Four dashboard->device socket handlers had no capability gate. dashboard:device-command has always refused a command the panel cannot honour, and the comment above it is right about why ("hiding the button is not enforcement — this socket is reachable directly"). Every word applied to the four handlers immediately above it, which had none: a display declaring [] still received screenshot-request, remote-touch, remote-key and remote-start. Measured, not inferred. They now refuse on remote.screenshot / remote.input / remote.stream and name the capability in the ack; remote-stop stays ungated for the same reason set_debug does. The undeclared fleet is unaffected — an absent declaration still resolves to its platform baseline and keeps everything. The wall panel list (#235) made this visible: it offered a Screenshot button for every panel, including a BrightSign, which has no screenshot capability at all, and popped a toast promising an image that was never coming. GET /api/devices now ships the RESOLVED capability array rather than the raw column ('[]' as a STRING, which Array.isArray reads as "pre-capability server, show everything" — wrong in the one case that matters), so the wall list and the fleet cards can hide what a panel cannot do. The remote pad's Scrn Off / Scrn On were gated on remote.input while the Info tab gated the same two commands on display.power; both now agree. 3. A register with no `platform` ERASED the stored one. captureIdentity coerces a missing field to the literal 'unknown' and persistIdentity wrote it straight over. That column is load-bearing: platformFamily() reads it, so one reconnect from an older build turned a Tizen panel into a browser tab and handed it a volume slider the .wgt has no handler for — the exact control BASELINE.tizen exists to hide — while a BrightSign lost screen power and reboot and gained screenshots it cannot take. platform and client_type are now preserved (physical facts); client_version and contract_version still decay, because there "we no longer know" is the truthful answer. client_type 'wgt' is also read as a second signal for a Tizen TV. 4. PUT /api/content/:id/replace carried its own shorter copy of the ingest logic. Replacing a video left duration_sec at the OLD clip's length and nulled width/height, so #237's brand-new "default an item to the clip's own length" then handed out the wrong number for every later add — 32s scheduled for a 5s video is 27s of frozen frame. Replacing an image measured it with raw sharp metadata and thumbnailed without .rotate(), re-introducing the EXIF-orientation bug #172 had just fixed at ingest. Both paths now share lib/content-ingest.deriveMediaMetadata. Verified working and NOT changed: all six item-duration insert paths (a 31.7s clip stores 32 everywhere, an explicit value always wins, and no path can store a 0); the content revision bump + filepath refresh reaching a real device socket; a landscape wall producing byte-identical geometry to the pre-#236 expression; a portrait wall reaching the player as side-by-side halves; cross-workspace isolation across 29 probes. Full suite green (1319). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL --- frontend/js/i18n/en.js | 3 + frontend/js/views/dashboard.js | 22 +-- frontend/js/views/device-detail.js | 8 +- frontend/js/views/video-wall.js | 6 +- server/lib/content-ingest.js | 46 +++-- server/lib/liveness.js | 38 +++- server/lib/player-capabilities.js | 6 + server/routes/content.js | 36 ++-- server/routes/devices.js | 12 +- server/server.js | 20 +- server/test/content-replace-metadata.test.js | 149 ++++++++++++++ .../test/identity-platform-preserved.test.js | 108 ++++++++++ server/test/remote-capability-gate.test.js | 187 ++++++++++++++++++ server/test/uploads-missing-file-404.test.js | 81 ++++++++ server/ws/dashboardSocket.js | 42 +++- server/ws/deviceSocket.js | 7 +- 16 files changed, 715 insertions(+), 56 deletions(-) create mode 100644 server/test/content-replace-metadata.test.js create mode 100644 server/test/identity-platform-preserved.test.js create mode 100644 server/test/remote-capability-gate.test.js create mode 100644 server/test/uploads-missing-file-404.test.js diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 24a2731..821c15d 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -403,6 +403,9 @@ export default { 'device.confirm_discard_draft': 'Discard all unpublished changes and revert to the last published version?', 'device.failed_load': 'Failed to load device', 'device.no_screenshot': 'No screenshot available. Click "Screenshot" to capture one.', + // Shown instead of the line above on a player that cannot capture its own screen — pointing at + // a "Screenshot" button that is correctly not rendered reads as a broken dashboard. + 'device.no_screenshot_unsupported': 'This player cannot capture its own screen.', 'device.no_content_assigned': 'No content assigned', 'device.now_playing_id': 'Playing: {id}', 'device.playlist_count_one': '1 item in playlist', diff --git a/frontend/js/views/dashboard.js b/frontend/js/views/dashboard.js index 7ffb2f9..a2b8cda 100644 --- a/frontend/js/views/dashboard.js +++ b/frontend/js/views/dashboard.js @@ -102,8 +102,12 @@ function renderDeviceCard(device) { : null; const checked = selectedDeviceIds.has(device.id); + // A panel that cannot capture its own screen is not asked to, every 30 seconds, forever. The + // list now carries the RESOLVED capability set (routes/devices.js), so a device that declares + // nothing still reads as its platform baseline and keeps being polled exactly as today. + const canShot = !Array.isArray(device.capabilities) || device.capabilities.includes('remote.screenshot'); return ` -
+
@@ -486,18 +490,14 @@ export function render(container) { for (const id of playbackByDevice.keys()) renderProgressFor(id); }, 1000); - // Request fresh screenshots on load - setTimeout(() => { - document.querySelectorAll('.device-card').forEach(card => { + // Request fresh screenshots on load — from the panels that can actually take one. + const pollScreenshots = () => { + document.querySelectorAll('.device-card[data-can-screenshot="1"]').forEach(card => { requestScreenshot(card.dataset.deviceId); }); - }, 2000); - - refreshInterval = setInterval(() => { - document.querySelectorAll('.device-card').forEach(card => { - requestScreenshot(card.dataset.deviceId); - }); - }, 30000); + }; + setTimeout(pollScreenshots, 2000); + refreshInterval = setInterval(pollScreenshots, 30000); } function refreshSelectionBar() { diff --git a/frontend/js/views/device-detail.js b/frontend/js/views/device-detail.js index d07e741..32e6fba 100644 --- a/frontend/js/views/device-detail.js +++ b/frontend/js/views/device-detail.js @@ -282,7 +282,10 @@ async function loadDevice(deviceId, activeTab = null) { - ${t('device.no_screenshot')} + + ${can('remote.screenshot') ? t('device.no_screenshot') : t('device.no_screenshot_unsupported')}
` }
@@ -683,11 +686,12 @@ async function loadDevice(deviceId, activeTab = null) {
+ ${can('display.power') ? `
-
+ ` : ''} ` : ''} ${device.tier === 2 ? ` ${t('device.remote.system_view_owner')} diff --git a/frontend/js/views/video-wall.js b/frontend/js/views/video-wall.js index f6f62d7..9ae9685 100644 --- a/frontend/js/views/video-wall.js +++ b/frontend/js/views/video-wall.js @@ -581,8 +581,9 @@ async function renderWallEditor(container, wallId) { ${esc(b.label)}${meta ? ` · ${meta}` : ''} + ${(!Array.isArray(d.capabilities) || d.capabilities.includes('remote.screenshot')) ? ` + title="Ask this panel for a screenshot — safe on a live wall, it doesn't change what's playing">Screenshot` : ''} Open `; @@ -593,6 +594,9 @@ async function renderWallEditor(container, wallId) { will desync the wall — use the wall playlist above instead.

`; + // The button is only rendered for a panel that declares (or baselines to) remote.screenshot — + // a BrightSign has no screenshot capability at all, so the old unconditional button popped a + // toast promising an image that was never coming. host.querySelectorAll('.wall-panel-shot').forEach(btn => { btn.addEventListener('click', () => { requestScreenshot(btn.dataset.deviceId); diff --git a/server/lib/content-ingest.js b/server/lib/content-ingest.js index 204e2cb..fe1063a 100644 --- a/server/lib/content-ingest.js +++ b/server/lib/content-ingest.js @@ -22,16 +22,22 @@ function safeFilename(name) { return sanitizeString((name || '').normalize('NFC')); } -// Process a multer-uploaded file (thumbnail + dimensions + duration) and insert a content -// row. Returns the content row. Throws on a hard failure (the caller maps to 500); -// thumbnail/metadata failures are best-effort (logged, non-fatal) exactly as before. -async function ingestUploadedFile({ file, userId, workspaceId, folderId = null }) { - const id = uuidv4(); - // Content-derived extension + mime. Throws UnsupportedUploadError (and removes the temp - // file) when the bytes are not a supported media type; the caller maps that to a 400. - const { filepath, mime } = finalizeUpload(file); +/* + * Everything we can learn from the BYTES: thumbnail, display dimensions, duration. + * + * Extracted so PUT /api/content/:id/replace derives them the same way an upload does. It used + * to carry its own shorter copy that handled images only — so replacing a video wiped the row's + * duration, dimensions and thumbnail, and replacing a portrait photo re-introduced the EXIF + * orientation bug (#170) that the ingest path fixes with imageDisplayDims + .rotate(). A second + * copy of this logic is a second place for it to rot; there is now one. + * + * Best-effort by contract: a missing ffprobe or a sharp failure yields nulls and a warning, never + * a throw — the file itself is already stored and is worth more than its metadata. + * + * @returns {{width:number|null, height:number|null, durationSec:number|null, thumbnailPath:string|null}} + */ +async function deriveMediaMetadata(sourcePath, filepath, mime) { let width = null, height = null, durationSec = null, thumbnailPath = null; - try { // SVG is deliberately NOT handed to sharp: rasterising it goes through librsvg, which // is where the outstanding libvips CVEs live, and an SVG is already its own thumbnail. @@ -39,11 +45,11 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } thumbnailPath = filepath; } else if (mime.startsWith('image/')) { const sharp = require('sharp'); - const metadata = await sharp(file.path).metadata(); + const metadata = await sharp(sourcePath).metadata(); // #170: honor EXIF orientation so a portrait photo isn't stored as landscape. ({ width, height } = imageDisplayDims(metadata)); thumbnailPath = `thumb_${filepath}`; - await sharp(file.path) + await sharp(sourcePath) .rotate() // #170: auto-orient per EXIF (and strip the tag) so the thumbnail matches .resize(config.thumbnailWidth) .jpeg({ quality: 70 }) @@ -51,7 +57,7 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } } else if (mime.startsWith('video/')) { try { const { execFileSync } = require('child_process'); - const probe = execFileSync('ffprobe', ['-v', 'quiet', '-print_format', 'json', '-show_format', '-show_streams', file.path], + const probe = execFileSync('ffprobe', ['-v', 'quiet', '-print_format', 'json', '-show_format', '-show_streams', sourcePath], { timeout: 15000 } ).toString(); const info = JSON.parse(probe); @@ -64,7 +70,7 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } } thumbnailPath = `thumb_${filepath.replace(/\.[^.]+$/, '.jpg')}`; try { - execFileSync('ffmpeg', ['-y', '-i', file.path, '-ss', '2', '-vframes', '1', '-vf', `scale=${config.thumbnailWidth}:-1`, path.join(config.contentDir, thumbnailPath)], + execFileSync('ffmpeg', ['-y', '-i', sourcePath, '-ss', '2', '-vframes', '1', '-vf', `scale=${config.thumbnailWidth}:-1`, path.join(config.contentDir, thumbnailPath)], { timeout: 15000 } ); } catch { thumbnailPath = null; } @@ -75,6 +81,18 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } } catch (e) { console.warn('Thumbnail/metadata generation failed:', e.message); } + return { width, height, durationSec, thumbnailPath }; +} + +// Process a multer-uploaded file (thumbnail + dimensions + duration) and insert a content +// row. Returns the content row. Throws on a hard failure (the caller maps to 500); +// thumbnail/metadata failures are best-effort (logged, non-fatal) exactly as before. +async function ingestUploadedFile({ file, userId, workspaceId, folderId = null }) { + const id = uuidv4(); + // Content-derived extension + mime. Throws UnsupportedUploadError (and removes the temp + // file) when the bytes are not a supported media type; the caller maps that to a 400. + const { filepath, mime } = finalizeUpload(file); + const { width, height, durationSec, thumbnailPath } = await deriveMediaMetadata(file.path, filepath, mime); db.prepare(` INSERT INTO content (id, user_id, workspace_id, filename, filepath, mime_type, file_size, duration_sec, thumbnail_path, width, height, folder_id) @@ -84,4 +102,4 @@ async function ingestUploadedFile({ file, userId, workspaceId, folderId = null } return db.prepare('SELECT * FROM content WHERE id = ?').get(id); } -module.exports = { ingestUploadedFile, safeFilename }; +module.exports = { ingestUploadedFile, safeFilename, deriveMediaMetadata }; diff --git a/server/lib/liveness.js b/server/lib/liveness.js index 681c4cb..157ce45 100644 --- a/server/lib/liveness.js +++ b/server/lib/liveness.js @@ -55,6 +55,42 @@ function captureIdentity(data) { }; } +/* + * Absent is not a statement — the same rule applyCapabilities() enforces for the capability column. + * + * captureIdentity above coerces a MISSING platform to the literal 'unknown', and persistIdentity + * used to write that straight over the stored value. One register from a client that doesn't send + * the field — an older build after an OTA, a downgrade, anything pre-v4 — permanently erased the + * panel's platform. + * + * That column is load-bearing, not decorative: player-capabilities.platformFamily() reads it to + * pick a baseline. An erased Tizen panel falls through to the WEB baseline and is offered a volume + * slider the .wgt has no handler for — the exact control BASELINE.tizen exists to hide — while an + * erased BrightSign loses screen power and reboot and gains screenshots it cannot take. + * + * platform and client_type are preserved; client_version and contract_version are NOT. The split is + * "physical fact" vs "property of the build currently installed": a panel does not stop being a + * Tizen TV or a .wgt player, but its version and protocol level change with every OTA, and there + * "we no longer know" is the truthful answer rather than a stale number. + * + * client_type earns its place because it is the SECOND signal platformFamily() reads ('wgt' => a + * Tizen TV): preserving platform while letting client_type decay to 'legacy' would leave a panel + * with no identifying signal at all. + * + * @param {object|null} stored the identity row currently in the DB + * @param {object} incoming the freshly captured identity (mutated in place and returned) + */ +const IDENTITY_PLACEHOLDER = { platform: 'unknown', client_type: 'legacy' }; +function preserveKnownIdentity(stored, incoming) { + if (!incoming || !stored) return incoming; + for (const [field, placeholder] of Object.entries(IDENTITY_PLACEHOLDER)) { + if (incoming[field] === placeholder && stored[field] && stored[field] !== placeholder) { + incoming[field] = stored[field]; + } + } + return incoming; +} + // A1 change-detection: has the (already-captured) identity changed vs what's stored? A genuine // reconnect with an unchanged identity (the common case) then does NO write. A never-stored device // (current null / all-NULL columns) or a real change (e.g. new client_version after an OTA) writes. @@ -78,4 +114,4 @@ function sanitizeExitReason(reason, detail) { return { reason, detail: d }; } -module.exports = { ackableHeartbeat, deriveLiveness, captureIdentity, identityChanged, sanitizeExitReason, CLIENT_EXIT_REASONS, HEALTHY_HEARTBEAT_MS, DEGRADED_RECONNECTS }; +module.exports = { ackableHeartbeat, deriveLiveness, captureIdentity, identityChanged, preserveKnownIdentity, sanitizeExitReason, CLIENT_EXIT_REASONS, HEALTHY_HEARTBEAT_MS, DEGRADED_RECONNECTS }; diff --git a/server/lib/player-capabilities.js b/server/lib/player-capabilities.js index afdf145..bac3016 100644 --- a/server/lib/player-capabilities.js +++ b/server/lib/player-capabilities.js @@ -120,8 +120,14 @@ const BASELINE = { function platformFamily(device) { const platform = String((device && device.platform) || '').toLowerCase(); const android = String((device && device.android_version) || ''); + const clientType = (device && device.client_type) || ''; if (platform.includes('brightsign')) return 'brightsign'; if (platform.includes('tizen')) return 'tizen'; + // Second, independent signal for a Tizen TV: the .wgt player sends client_type 'wgt' (see + // tizen/js/app.js). `platform` is the primary key, but it lives in a column that a register from + // a client not sending it used to overwrite — and misreading a Tizen panel as a browser tab + // hands it a volume slider with no handler behind it. Two signals, one conclusion. + if (clientType === 'wgt') return 'tizen'; // client_type 'apk' is the Android player; android_version that is NOT the web player's // "Web/..." shape is the older signal for the same thing. if ((device && device.client_type === 'apk') || (android && !android.startsWith('Web/'))) return 'android'; diff --git a/server/routes/content.js b/server/routes/content.js index 1b4e95c..b11f869 100644 --- a/server/routes/content.js +++ b/server/routes/content.js @@ -12,7 +12,7 @@ const { PLATFORM_ROLES, ELEVATED_ROLES } = require('../middleware/auth'); // Phase 2.2b: workspace-aware access. Mirrors the pattern from devices.js. const { accessContext } = require('../lib/tenancy'); // #73: the upload ingest (processing + insert) is now shared with the agency router. -const { ingestUploadedFile } = require('../lib/content-ingest'); +const { ingestUploadedFile, deriveMediaMetadata } = require('../lib/content-ingest'); const { finalizeUpload, INLINE_SAFE_EXTS } = require('../lib/upload-sniff'); // Multer captures file.originalname directly from the multipart filename header, @@ -511,24 +511,18 @@ router.put('/:id/replace', upload.single('file'), async (req, res) => { let filepath, mime; try { ({ filepath, mime } = finalizeUpload(req.file)); } catch (e) { return res.status(e.status || 400).json({ error: e.message }); } - let width = null, height = null, thumbnailPath = null; - // Generate new thumbnail for images (SVG skipped — see lib/content-ingest.js) - try { - if (mime === 'image/svg+xml') { - thumbnailPath = filepath; - } else if (mime.startsWith('image/')) { - const sharp = require('sharp'); - const metadata = await sharp(req.file.path).metadata(); - width = metadata.width; - height = metadata.height; - thumbnailPath = `thumb_${filepath}`; - await sharp(req.file.path).resize(config.thumbnailWidth).jpeg({ quality: 70 }) - .toFile(path.join(config.contentDir, thumbnailPath)); - } - } catch (e) { - console.warn('Thumbnail generation failed:', e.message); - } + // Re-derive EVERYTHING the bytes decide, through the SAME function the upload path uses. + // This route used to carry a shorter copy that handled images only, and got three things + // wrong that an upload gets right: + // - a replaced VIDEO lost its duration (the row kept the OLD clip's length, so #237's + // "default an item to the clip's own length" then handed out the wrong number), its + // dimensions, and its thumbnail; + // - a replaced IMAGE was measured with raw sharp metadata instead of imageDisplayDims and + // thumbnailed without .rotate(), re-introducing the EXIF-orientation bug (#170) that + // ingest fixes — a portrait photo came back landscape with blue bars; + // - both left width/height NULL for video, which is what the orientation-aware paths read. + const { width, height, durationSec, thumbnailPath } = await deriveMediaMetadata(req.file.path, filepath, mime); // Bump the revision: this is the ONLY operation in the product that changes an asset's bytes // without changing its id, so it is the only thing that can make a player's cached copy wrong. @@ -537,11 +531,15 @@ router.put('/:id/replace', upload.single('file'), async (req, res) => { // strftime seconds can collide with the previous value if a replace lands inside the same second // as the upload (a small file, a scripted replace) — and a revision that does not change is a // cache that never updates. MAX(now, previous + 1) guarantees it moves. + // duration_sec comes from the NEW bytes. COALESCE-to-NULL rather than keeping the old value: + // a replace that turns a video into an image genuinely has no duration, and a stale one would + // silently become the default for every later playlist add (lib/item-duration.js). db.prepare(`UPDATE content SET filepath = ?, mime_type = ?, file_size = ?, thumbnail_path = ?, width = ?, height = ?, + duration_sec = ?, updated_at = MAX(CAST(strftime('%s','now') AS INTEGER), COALESCE(NULLIF(updated_at, 0), created_at) + 1) WHERE id = ?`) - .run(filepath, mime, req.file.size, thumbnailPath, width, height, req.params.id); + .run(filepath, mime, req.file.size, thumbnailPath, width, height, durationSec, req.params.id); // ...and tell the panels, which the old code did not. Without this the new bytes reached a screen // only when something else happened to trigger a playlist refresh — an operator replacing a video diff --git a/server/routes/devices.js b/server/routes/devices.js index e98dcc7..44e30e3 100644 --- a/server/routes/devices.js +++ b/server/routes/devices.js @@ -46,7 +46,17 @@ 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 => ({ ...stripDeviceSecretsForList(d), orphan_count: orphanCounts[d.id] || 0 }))); + // The RESOLVED capability set, the same shape GET /:id returns. The raw column shipped here + // before: a JSON *string* ('[]') or null, which every consumer would have had to parse — and + // `Array.isArray("[]")` is false, so the dashboard's `can()` helper reads a device that declared + // "I can do nothing" as "pre-capability server, show everything". Resolving it here means the + // fleet views (device cards, the wall panel list) can hide a control the panel cannot honour + // instead of offering it and having the socket drop it. + res.json(devices.map(d => ({ + ...stripDeviceSecretsForList(d), + capabilities: playerCapabilities.capabilitiesFor(d), + orphan_count: orphanCounts[d.id] || 0, + }))); }); // #106: reorder display tiles (cosmetic, within-section). Writes devices.sort_order diff --git a/server/server.js b/server/server.js index ce3d6ab..305b332 100644 --- a/server/server.js +++ b/server/server.js @@ -996,7 +996,25 @@ app.use('/uploads/content', (req, res, next) => { // re-assert the override here for anything not inline-safe. hardenUploadResponse(res, filePath); }, -})); +}), (req, res) => { + /* + * A miss ENDS here. express.static calls next() when the file isn't there, and the only thing + * left downstream is the SPA catch-all — so GET /uploads/content/.mp4 answered + * 200 text/html with 15KB of dashboard, under the `immutable, max-age=30d` header this + * middleware already set on the way in. + * + * That is the worst possible answer for a player. Every downloader treats 200 as success, so + * the panel stores the HTML page AS the video, caches it for a month, and plays a black frame + * with nothing in any log to say why. It is reachable the moment an asset is replaced (a + * replace writes a new random filename and unlinks the old one) or a file goes missing from + * the volume, which is exactly when a screen most needs to fail loudly. + * + * The Cache-Control is cleared too: 'immutable' is a promise about a file that exists. + */ + res.removeHeader('Cache-Control'); + res.removeHeader('Content-Disposition'); + res.type('application/json').status(404).json({ error: 'Not found' }); +}); // Media proxy for remote (URL-referenced) playlist items — public by construction (players are // unauthenticated browsers). Takes an itemId, never a caller URL: it fetches the item's stored diff --git a/server/test/content-replace-metadata.test.js b/server/test/content-replace-metadata.test.js new file mode 100644 index 0000000..5bce15a --- /dev/null +++ b/server/test/content-replace-metadata.test.js @@ -0,0 +1,149 @@ +'use strict'; + +/* + * PUT /api/content/:id/replace must re-derive everything the BYTES decide. + * + * The route carried its own shorter copy of the ingest logic that handled images only, so: + * - replacing a VIDEO left duration_sec at the OLD clip's length and nulled width/height. + * That is not cosmetic: lib/item-duration.js defaults a new playlist item to the content's + * duration, so after replacing a 32s clip with a 5s one, every later "add to playlist" + * scheduled 32 seconds of a 5-second video — 27s of frozen last frame on the screen. + * - replacing an IMAGE measured it with raw sharp metadata and thumbnailed without .rotate(), + * re-introducing the EXIF-orientation bug (#170) that ingest fixes: a portrait photo came + * back recorded as landscape. + * + * Driven over real HTTP against the real router, because the bug was in the route, not the lib. + */ + +const os = require('node:os'); +const path = require('node:path'); +const fsp = require('node:fs/promises'); +const fs = require('node:fs'); +const crypto = require('node:crypto'); +process.env.DATA_DIR = path.join(os.tmpdir(), 'st-replace-' + crypto.randomBytes(4).toString('hex')); +process.env.SELF_HOSTED = 'true'; +process.env.NODE_ENV = 'test'; + +const { test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const http = require('node:http'); +const express = require('express'); +const sharp = require('sharp'); +const { db } = require('../db/database'); +const config = require('../config'); + +const WS = 'ws-replace'; +const USER = 'u-replace'; +let server, base; + +function hasFfmpeg() { + try { + const { execFileSync } = require('node:child_process'); + execFileSync('ffprobe', ['-version'], { stdio: 'ignore' }); + execFileSync('ffmpeg', ['-version'], { stdio: 'ignore' }); + return true; + } catch { return false; } +} + +// Seed through the SHARED ingest lib (the same call POST /api/content makes) rather than over +// HTTP, so the fixture is a genuine first-class content row and the only thing this suite drives +// over the wire is the route under test. +const { ingestUploadedFile } = require('../lib/content-ingest'); +async function upload(bytes, filename) { + const tmp = path.join(config.contentDir, crypto.randomUUID() + '.part'); + await fsp.mkdir(config.contentDir, { recursive: true }); + await fsp.writeFile(tmp, bytes); + return ingestUploadedFile({ + file: { path: tmp, originalname: filename, size: bytes.length }, + userId: USER, workspaceId: WS, + }); +} + +async function replace(id, bytes, filename, type) { + const fd = new FormData(); + fd.append('file', new Blob([bytes], { type }), filename); + const r = await fetch(`${base}/${id}/replace`, { method: 'PUT', body: fd }); + return { status: r.status, body: await r.json() }; +} + +before(async () => { + db.prepare("INSERT INTO users (id, email, name, role) VALUES (?, ?, ?, 'platform_admin')").run(USER, 'replace@test', 'QA'); + db.prepare('INSERT INTO organizations (id, name, owner_user_id) VALUES (?, ?, ?)').run('org-replace', 'Org', USER); + db.prepare('INSERT INTO workspaces (id, organization_id, name) VALUES (?, ?, ?)').run(WS, 'org-replace', 'WS'); + const app = express(); + app.use((req, _res, next) => { + req.workspaceId = WS; + req.user = { id: USER, role: 'platform_admin' }; + next(); + }); + app.use('/', require('../routes/content')); + server = http.createServer(app); + await new Promise((r) => server.listen(0, r)); + base = `http://127.0.0.1:${server.address().port}`; +}); + +after(() => new Promise((r) => server.close(r))); + +test('replacing an image re-measures it — a landscape photo does not keep the old portrait dims', async () => { + const tall = await sharp({ create: { width: 40, height: 90, channels: 3, background: '#123456' } }).png().toBuffer(); + const wide = await sharp({ create: { width: 120, height: 30, channels: 3, background: '#654321' } }).png().toBuffer(); + + const row = await upload(tall, 'tall.png', 'image/png'); + assert.equal(row.width, 40); + assert.equal(row.height, 90); + + const { status, body } = await replace(row.id, wide, 'wide.png', 'image/png'); + assert.equal(status, 200); + assert.equal(body.width, 120, 'width comes from the NEW bytes'); + assert.equal(body.height, 30, 'height comes from the NEW bytes'); + assert.notEqual(body.filepath, row.filepath, 'a replace writes a new randomly-named file'); +}); + +test('replacing an image honours EXIF orientation, the same way ingest does (#170)', async () => { + // orientation 6 = "rotate 90° CW to display": a 30x100 stored buffer DISPLAYS as 100x30. + // The old replace path read sharp's raw metadata and recorded 30x100 — the exact bug that + // put blue bars down portrait uploads before #172 fixed the ingest path. + const plain = await sharp({ create: { width: 10, height: 10, channels: 3, background: '#000' } }).jpeg().toBuffer(); + const rotated = await sharp({ create: { width: 30, height: 100, channels: 3, background: '#00ff00' } }) + .withMetadata({ orientation: 6 }).jpeg().toBuffer(); + + const row = await upload(plain, 'plain.jpg', 'image/jpeg'); + const { status, body } = await replace(row.id, rotated, 'rotated.jpg', 'image/jpeg'); + assert.equal(status, 200); + assert.equal(body.width, 100, 'EXIF-rotated image is measured as DISPLAYED, not as stored'); + assert.equal(body.height, 30); +}); + +test('replacing an image regenerates its thumbnail file, rather than pointing at a deleted one', async () => { + const a = await sharp({ create: { width: 60, height: 60, channels: 3, background: '#ff0000' } }).png().toBuffer(); + const b = await sharp({ create: { width: 80, height: 80, channels: 3, background: '#0000ff' } }).png().toBuffer(); + const row = await upload(a, 'a.png', 'image/png'); + const { body } = await replace(row.id, b, 'b.png', 'image/png'); + assert.ok(body.thumbnail_path, 'a thumbnail is recorded'); + assert.ok(fs.existsSync(path.join(config.contentDir, body.thumbnail_path)), 'and the file it names EXISTS'); +}); + +test('replacing a video re-probes its duration — a stale one mis-defaults every later playlist add', { skip: hasFfmpeg() ? false : 'ffmpeg/ffprobe not installed' }, async () => { + const { execFileSync } = require('node:child_process'); + const dir = await fsp.mkdtemp(path.join(os.tmpdir(), 'st-vid-')); + const long = path.join(dir, 'long.mp4'); + const short = path.join(dir, 'short.mp4'); + const mk = (out, secs, size) => execFileSync('ffmpeg', ['-v', 'quiet', '-y', '-f', 'lavfi', '-i', `color=c=blue:s=${size}:d=${secs}`, '-c:v', 'libx264', '-pix_fmt', 'yuv420p', '-t', String(secs), out], { timeout: 60000 }); + mk(long, 8, '320x240'); + mk(short, 2, '240x320'); + + const row = await upload(await fsp.readFile(long), 'long.mp4', 'video/mp4'); + assert.ok(row.duration_sec >= 7.5 && row.duration_sec <= 8.5, `seeded 8s clip probed as ${row.duration_sec}`); + + const { status, body } = await replace(row.id, await fsp.readFile(short), 'short.mp4', 'video/mp4'); + assert.equal(status, 200); + assert.ok(body.duration_sec >= 1.5 && body.duration_sec <= 2.5, + `duration follows the NEW bytes (got ${body.duration_sec}; the old code left 8)`); + assert.equal(body.width, 240, 'and the dimensions do too — they used to be nulled for video'); + assert.equal(body.height, 320); + + // The point of all of it: the shared duration default now describes the file that is there. + const { resolveItemDuration } = require('../lib/item-duration'); + assert.equal(resolveItemDuration(undefined, body), 2, + 'a playlist item added after the replace gets the NEW clip\'s length, not the old one\'s'); +}); diff --git a/server/test/identity-platform-preserved.test.js b/server/test/identity-platform-preserved.test.js new file mode 100644 index 0000000..7406cad --- /dev/null +++ b/server/test/identity-platform-preserved.test.js @@ -0,0 +1,108 @@ +'use strict'; + +/* + * A register that does not mention `platform` must not erase the one we have. + * + * liveness.captureIdentity() coerces a missing platform to the literal string 'unknown', and + * persistIdentity() wrote that straight over the stored value. So a single register from any + * client that doesn't send the field — an older build, a downgrade, anything pre-v4 — permanently + * turned a known Tizen panel into 'unknown'. + * + * That column is load-bearing: player-capabilities.platformFamily() reads it to pick a baseline. + * A cleared Tizen panel falls through to the WEB baseline, which hands it audio.volume (the .wgt + * has no set_volume handler — that is precisely why BASELINE.tizen omits it) and offline.cache + * (Tizen caches the playlist JSON, not the media). The same clobber costs a BrightSign its screen + * power and reboot and gives it screenshots it cannot take. + * + * The rule is the one applyCapabilities() already documents one screen up: an ABSENT declaration + * is not a statement about the device. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const liveness = require('../lib/liveness'); +const caps = require('../lib/player-capabilities'); + +// What persistIdentity does with a register payload, minus the DB round trip. +const resolve = (stored, data) => liveness.preserveKnownIdentity(stored, liveness.captureIdentity(data)); + +test('a register with no platform leaves a known platform alone', () => { + const tizen = { client_type: 'wgt', client_version: '1.9.29', platform: 'Tizen 6.0', contract_version: 'v4' }; + assert.equal(resolve(tizen, { device_id: 'x' }).platform, 'Tizen 6.0'); + + const bs = { client_type: 'player', client_version: '1.9.29', platform: 'brightsign', contract_version: 'v4' }; + assert.equal(resolve(bs, { device_id: 'x' }).platform, 'brightsign'); +}); + +test('the capability baseline survives that register — which is the whole point', () => { + const stored = { client_type: 'wgt', client_version: '1.9.29', platform: 'Tizen 6.0', contract_version: 'v4' }; + const after = resolve(stored, { device_id: 'x' }); // an old client reconnects + const row = { platform: after.platform, client_type: after.client_type, android_version: null }; + + assert.equal(caps.platformFamily(row), 'tizen'); + assert.equal(caps.supports(row, 'audio.volume'), false, + 'a fielded .wgt has no set_volume handler — the web baseline would have offered the slider anyway'); + assert.equal(caps.supports(row, 'offline.cache'), false, + 'Tizen caches the playlist JSON, not the media, so content does NOT survive an outage'); + + const bsAfter = resolve({ client_type: 'player', client_version: '1.9.29', platform: 'brightsign', contract_version: 'v4' }, {}); + const bsRow = { platform: bsAfter.platform, client_type: bsAfter.client_type, android_version: null }; + assert.equal(caps.platformFamily(bsRow), 'brightsign'); + assert.equal(caps.supports(bsRow, 'system.reboot'), true, 'a BrightSign really can reboot'); + assert.equal(caps.supports(bsRow, 'remote.screenshot'), false, 'and really cannot screenshot'); +}); + +test('a register that DOES declare a platform still updates it', () => { + // Preserving must not become "the first value wins forever": a genuine change (a panel + // re-flashed, a row reused for different hardware) has to land. + const stored = { client_type: 'wgt', client_version: '1.9.29', platform: 'Tizen 6.0', contract_version: 'v4' }; + const i = resolve(stored, { platform: 'Tizen 7.0', client_type: 'wgt', client_version: '2.0', contract_version: 'v4' }); + assert.equal(i.platform, 'Tizen 7.0'); + assert.equal(liveness.identityChanged(stored, i), true, 'and the change is still detected, so it is written'); +}); + +test('a device that never had a platform is not invented one', () => { + assert.equal(resolve(null, {}).platform, 'unknown', 'still honest about not knowing'); + assert.equal(resolve({ platform: 'unknown' }, {}).platform, 'unknown'); +}); + +test('an unchanged identity still short-circuits the write', () => { + // The A1 optimisation this function sits inside: a plain reconnect must stay a read with no + // UPDATE. Preserving the platform must not make every register look like a change. + const stored = { client_type: 'wgt', client_version: '1.9.29', platform: 'Tizen 6.0', contract_version: 'v4' }; + assert.equal(liveness.identityChanged(stored, resolve(stored, { + platform: 'Tizen 6.0', client_type: 'wgt', client_version: '1.9.29', contract_version: 'v4', + })), false); +}); + +test('client_type wgt is a second, independent signal that this is a Tizen TV', () => { + // Belt and braces for a row whose platform was already cleared by the old behaviour: the .wgt + // player sends client_type 'wgt' (tizen/js/app.js), so one lost column is not the end of it. + const cleared = { platform: 'unknown', client_type: 'wgt', android_version: null }; + assert.equal(caps.platformFamily(cleared), 'tizen'); + assert.equal(caps.supports(cleared, 'audio.volume'), false); +}); + +test('client_type is preserved too — otherwise the second signal decays with the first', () => { + // captureIdentity coerces a missing client_type to 'legacy'. Preserving `platform` alone would + // still leave a panel that reconnects from an older build with NOTHING identifying it. + const stored = { client_type: 'wgt', client_version: '1.9.29', platform: 'Tizen 6.0', contract_version: 'v4' }; + assert.equal(resolve(stored, {}).client_type, 'wgt'); + assert.equal(resolve({ client_type: 'apk', platform: 'Android 14' }, {}).client_type, 'apk'); +}); + +test('the version fields still decay — a stale build number is not an improvement', () => { + // The other half of the split: platform/client_type are physical facts, client_version and + // contract_version are properties of the build currently installed and change with every OTA. + const stored = { client_type: 'wgt', client_version: '1.9.29', platform: 'Tizen 6.0', contract_version: 'v4' }; + const i = resolve(stored, {}); + assert.equal(i.client_version, 'unknown'); + assert.equal(i.contract_version, 'legacy'); +}); + +test('the Android fleet is untouched by the wgt rule', () => { + assert.equal(caps.platformFamily({ client_type: 'apk', android_version: '14' }), 'android'); + assert.equal(caps.platformFamily({ android_version: '14' }), 'android'); + assert.equal(caps.platformFamily({ android_version: 'Web/Chrome 120' }), 'web'); + assert.equal(caps.platformFamily({}), 'web'); +}); diff --git a/server/test/remote-capability-gate.test.js b/server/test/remote-capability-gate.test.js new file mode 100644 index 0000000..40d1549 --- /dev/null +++ b/server/test/remote-capability-gate.test.js @@ -0,0 +1,187 @@ +'use strict'; + +/* + * The capability gate on the dashboard socket's REMOTE handlers. + * + * dashboard:device-command has always refused a command the panel cannot honour, and the comment + * over it is right about why: "Hiding the button is not enforcement. This socket is reachable + * directly, and an older dashboard tab left open still renders the old controls." + * + * Every word of that applied to the four handlers sitting immediately ABOVE it, which had no gate + * at all. Measured against a real server: a display declaring `[]` still received + * screenshot-request, remote-touch, remote-key and remote-start. The dashboard even popped a toast + * ("Screenshot requested — it appears on the panel's device page") for a BrightSign, which has no + * screenshot capability at all. That is the "reports success and changes nothing" shape the whole + * capability model was written to end. + * + * End-to-end on a real server, because the bug was in the wiring: a real device socket registers, a + * real dashboard socket sends, and the assertion is on what the DEVICE actually received. + */ + +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 crypto = require('node:crypto'); +const Database = require('better-sqlite3'); +const { io } = require('socket.io-client'); +const { freePort } = require('./helpers/free-port'); + +const DATA_DIR = path.join(os.tmpdir(), 'st-remotegate-' + crypto.randomBytes(4).toString('hex')); +const DBPATH = path.join(DATA_DIR, 'db', 'remote_display.db'); +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + +let proc, BASE, PORT, jwt, workspaceId; + +// Every device below is an ANDROID panel. The only difference between them is the declaration, so +// a difference in outcome can only come from the declaration — not from a platform baseline. +const DEVICES = [ + ['d-declares-nothing', null], // the fielded fleet: NULL => android baseline + ['d-declares-empty', '[]'], // a real statement: I can do nothing + ['d-declares-shot-only', '["remote.screenshot"]'], +]; + +before(async () => { + PORT = await freePort(); + BASE = `http://127.0.0.1:${PORT}`; + 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', + }); + for (let i = 0; i < 100; i++) { + try { const r = await fetch(BASE + '/api/status'); if (r.ok) break; } catch { /* booting */ } + await sleep(150); + } + const reg = await fetch(BASE + '/api/auth/register', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: 'gate@test.local', password: 'GatePassw0rd!', name: 'Gate' }), + }).then((r) => r.json()); + jwt = reg.token; + workspaceId = reg.current_workspace_id; + + const seed = new Database(DBPATH); + const ins = seed.prepare(`INSERT INTO devices + (id, user_id, workspace_id, name, pairing_code, status, client_type, android_version, capabilities, device_token) + VALUES (?, ?, ?, ?, ?, 'online', 'apk', '14', ?, ?)`); + let n = 0; + for (const [id, caps] of DEVICES) ins.run(id, reg.user.id, workspaceId, id, String(900001 + n++), caps, 'tok-' + id); + seed.close(); +}); + +after(async () => { + if (proc) proc.kill('SIGKILL'); + await sleep(200); +}); + +function dashboard() { + return new Promise((resolve, reject) => { + const s = io(`${BASE}/dashboard`, { transports: ['websocket'], auth: { token: jwt }, reconnection: false }); + s.on('connect', () => resolve(s)); + s.on('connect_error', reject); + }); +} + +// Connect a real device socket and record every event the SERVER sends it. +function device(deviceId) { + return new Promise((resolve, reject) => { + const s = io(`${BASE}/device`, { transports: ['websocket'], reconnection: false }); + const got = []; + for (const ev of ['device:screenshot-request', 'device:remote-touch', 'device:remote-key', 'device:remote-start', 'device:remote-stop', 'device:command']) { + s.on(ev, () => got.push(ev)); + } + s.on('connect', () => s.emit('device:register', { device_id: deviceId, device_token: 'tok-' + deviceId })); + s.on('device:registered', () => resolve({ sock: s, got })); + s.on('connect_error', reject); + setTimeout(() => reject(new Error('register timed out for ' + deviceId)), 10000); + }); +} + +test('a display declaring [] receives NONE of the remote requests', async () => { + const dash = await dashboard(); + const dev = await device('d-declares-empty'); + await sleep(250); + dev.got.length = 0; + dash.emit('dashboard:request-screenshot', { device_id: 'd-declares-empty' }); + dash.emit('dashboard:remote-touch', { device_id: 'd-declares-empty', x: 5, y: 5 }); + dash.emit('dashboard:remote-key', { device_id: 'd-declares-empty', keycode: 4 }); + dash.emit('dashboard:remote-start', { device_id: 'd-declares-empty' }); + await sleep(700); + assert.deepEqual(dev.got, [], 'a panel that says it can do nothing is sent nothing'); + dev.sock.disconnect(); dash.disconnect(); +}); + +test('the refusal names the missing capability when the caller asks for an ack', async () => { + const dash = await dashboard(); + const dev = await device('d-declares-empty'); + await sleep(250); + const ack = (event) => new Promise((res) => { + let done = false; + dash.emit(event, { device_id: 'd-declares-empty', x: 1, y: 1, keycode: 4 }, (a) => { done = true; res(a); }); + setTimeout(() => { if (!done) res(null); }, 800); + }); + assert.deepEqual(await ack('dashboard:request-screenshot'), { delivered: false, reason: 'unsupported', capability: 'remote.screenshot' }); + assert.deepEqual(await ack('dashboard:remote-touch'), { delivered: false, reason: 'unsupported', capability: 'remote.input' }); + assert.deepEqual(await ack('dashboard:remote-key'), { delivered: false, reason: 'unsupported', capability: 'remote.input' }); + assert.deepEqual(await ack('dashboard:remote-start'), { delivered: false, reason: 'unsupported', capability: 'remote.stream' }); + dev.sock.disconnect(); dash.disconnect(); +}); + +test('THE FLEET: a display that has never declared anything still gets everything it always had', async () => { + // The failure that would be worse than the bug. Several hundred Android panels declare nothing; + // if "declared nothing" resolved to "supports nothing", this gate would strip remote control + // from the entire installed base on the day it deployed. + const dash = await dashboard(); + const dev = await device('d-declares-nothing'); + await sleep(250); + dev.got.length = 0; + dash.emit('dashboard:request-screenshot', { device_id: 'd-declares-nothing' }); + dash.emit('dashboard:remote-touch', { device_id: 'd-declares-nothing', x: 5, y: 5 }); + dash.emit('dashboard:remote-key', { device_id: 'd-declares-nothing', keycode: 4 }); + dash.emit('dashboard:remote-start', { device_id: 'd-declares-nothing' }); + await sleep(700); + assert.deepEqual(dev.got.sort(), [ + 'device:remote-key', 'device:remote-start', 'device:remote-touch', 'device:screenshot-request', + ], 'the undeclared Android baseline keeps the whole remote surface'); + dev.sock.disconnect(); dash.disconnect(); +}); + +test('the gate is per-capability, not all-or-nothing', async () => { + // A panel with accessibility on but no input injection declares screenshot alone. It must keep + // the screenshot and lose the pad, not lose both or keep both. + const dash = await dashboard(); + const dev = await device('d-declares-shot-only'); + await sleep(250); + dev.got.length = 0; + dash.emit('dashboard:request-screenshot', { device_id: 'd-declares-shot-only' }); + dash.emit('dashboard:remote-touch', { device_id: 'd-declares-shot-only', x: 5, y: 5 }); + dash.emit('dashboard:remote-start', { device_id: 'd-declares-shot-only' }); + await sleep(700); + assert.deepEqual(dev.got, ['device:screenshot-request']); + dev.sock.disconnect(); dash.disconnect(); +}); + +test('remote-stop is never refused — it is the way out of a stuck stream', async () => { + const dash = await dashboard(); + const dev = await device('d-declares-empty'); + await sleep(250); + dev.got.length = 0; + dash.emit('dashboard:remote-stop', { device_id: 'd-declares-empty' }); + await sleep(600); + assert.deepEqual(dev.got, ['device:remote-stop'], + 'a panel whose declaration changed mid-stream must still be stoppable'); + dev.sock.disconnect(); dash.disconnect(); +}); + +test('GET /api/devices ships the RESOLVED capability array, not the raw column', async () => { + // The list used to carry the raw TEXT column: '[]' or null. Array.isArray('[]') is false, so the + // dashboard's `can()` helper read a device that declared "I can do nothing" as "pre-capability + // server — show everything", which is the wrong answer in the one case that matters. + const rows = await fetch(BASE + '/api/devices', { headers: { Authorization: 'Bearer ' + jwt } }).then((r) => r.json()); + const byId = Object.fromEntries(rows.map((d) => [d.id, d])); + assert.ok(Array.isArray(byId['d-declares-empty'].capabilities), 'an array, not a JSON string'); + assert.deepEqual(byId['d-declares-empty'].capabilities, [], 'an empty declaration is honoured'); + assert.ok(byId['d-declares-nothing'].capabilities.includes('remote.screenshot'), + 'and an absent declaration still resolves to its platform baseline'); +}); diff --git a/server/test/uploads-missing-file-404.test.js b/server/test/uploads-missing-file-404.test.js new file mode 100644 index 0000000..2694bdc --- /dev/null +++ b/server/test/uploads-missing-file-404.test.js @@ -0,0 +1,81 @@ +'use strict'; + +/* + * A missing upload must 404, not hand the player the dashboard. + * + * express.static calls next() on a miss, and the only thing downstream of /uploads/content was the + * SPA catch-all. So GET /uploads/content/.mp4 answered 200 OK, Content-Type: text/html, with + * 15KB of index.html — under the `public, max-age=2592000, immutable` header the mount had already + * set on the way in, before it knew whether the file existed. + * + * For a player that is the worst possible answer. Every downloader treats 200 as success, so the + * panel stores the HTML page AS the video, caches it for a month, and renders a black frame with + * nothing in any log to explain it. And it is reachable exactly when it hurts: a content REPLACE + * writes a new randomly-named file and unlinks the old one, so every snapshot still pointing at the + * old name asks for a file that is gone. + * + * Whole-server test on purpose — the bug was the ORDER of two mounts, which no unit test can see. + */ + +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 { freePort } = require('./helpers/free-port'); + +const DATA_DIR = path.join(os.tmpdir(), 'st-uploads404-' + crypto.randomBytes(4).toString('hex')); +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +let proc, BASE; + +before(async () => { + const PORT = await freePort(); + BASE = `http://127.0.0.1:${PORT}`; + 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', + }); + for (let i = 0; i < 100; i++) { + try { const r = await fetch(BASE + '/api/status'); if (r.ok) break; } catch { /* booting */ } + await sleep(150); + } +}); + +after(async () => { if (proc) proc.kill('SIGKILL'); await sleep(150); }); + +test('a missing media file is a 404, not 200 with an HTML page', async () => { + const res = await fetch(BASE + '/uploads/content/00000000-0000-0000-0000-000000000000.mp4'); + assert.equal(res.status, 404); + const ct = res.headers.get('content-type') || ''; + assert.ok(!ct.includes('text/html'), `a player must never be handed HTML as a video (got ${ct})`); +}); + +test('and it is not cached for a month — immutable is a promise about a file that exists', async () => { + const res = await fetch(BASE + '/uploads/content/also-not-here.png'); + assert.equal(res.status, 404); + const cc = res.headers.get('cache-control') || ''; + assert.ok(!cc.includes('immutable'), `a 404 must not be cached as the asset (got "${cc}")`); +}); + +test('a file that IS there still serves, with its own type and the long cache', async () => { + // The guard must terminate ONLY the miss. A 404 on a present file would black out every screen. + const contentDir = path.join(DATA_DIR, 'uploads', 'content'); + fs.mkdirSync(contentDir, { recursive: true }); + const name = 'present-' + crypto.randomBytes(4).toString('hex') + '.png'; + const png = Buffer.from('89504e470d0a1a0a0000000d49484452', 'hex'); + fs.writeFileSync(path.join(contentDir, name), png); + + const res = await fetch(`${BASE}/uploads/content/${name}`); + assert.equal(res.status, 200); + assert.equal(res.headers.get('content-type'), 'image/png'); + assert.ok((res.headers.get('cache-control') || '').includes('immutable'), 'real assets keep the 30-day cache'); + assert.equal(Buffer.from(await res.arrayBuffer()).length, png.length); +}); + +test('path traversal out of the content dir is still not reachable', async () => { + const res = await fetch(BASE + '/uploads/content/..%2f..%2f..%2fetc%2fpasswd'); + assert.notEqual(res.status, 200); +}); diff --git a/server/ws/dashboardSocket.js b/server/ws/dashboardSocket.js index 13b6e91..0d05fc4 100644 --- a/server/ws/dashboardSocket.js +++ b/server/ws/dashboardSocket.js @@ -72,25 +72,55 @@ module.exports = function setupDashboardSocket(io) { for (const wsId of wsIds) socket.join(workspaceRoom(wsId)); console.log(`Dashboard client connected: ${socket.id} (user: ${socket.userId}, rooms: ${wsIds.length})`); - socket.on('dashboard:request-screenshot', (data) => { + /* + * The capability gate for the remote-view handlers. + * + * dashboard:device-command below has always checked this; these four did not, and the + * reasoning that justifies it there applies here word for word — this socket is reachable + * directly, and a dashboard tab left open still renders the controls the panel had when the + * page was drawn. Measured: a display declaring `[]` still received screenshot-request, + * remote-touch, remote-key and remote-start, silently, and the operator got a toast saying + * the screenshot was on its way. + * + * The ack is OPTIONAL by design: the current dashboard senders (frontend/js/socket.js) pass + * no callback, and a newer one that does gets told which capability is missing instead of + * watching a spinner. Refusing loudly is the whole point of the mechanism. + */ + // Silent by design, unlike the command path: the fleet view asks EVERY visible card for a + // screenshot every 30s, so a log line per refusal would be hundreds every half-minute on a + // real fleet. The ack carries the reason to anyone who asked for one. + function capabilityRefused(device_id, cap, ack) { + const devRow = db.prepare('SELECT * FROM devices WHERE id = ?').get(device_id); + if (playerCapabilities.supports(devRow, cap)) return false; + if (typeof ack === 'function') ack({ delivered: false, reason: 'unsupported', capability: cap }); + return true; + } + + socket.on('dashboard:request-screenshot', (data, ack) => { const { device_id } = data; if (!canActOnDevice(socket, device_id, 'read')) return; + if (capabilityRefused(device_id, 'remote.screenshot', ack)) return; const conn = heartbeat.getConnection(device_id); if (conn) deviceNs.to(device_id).emit('device:screenshot-request', {}); + if (typeof ack === 'function') ack({ delivered: !!conn, reason: conn ? undefined : 'offline' }); }); - socket.on('dashboard:remote-touch', (data) => { + socket.on('dashboard:remote-touch', (data, ack) => { const { device_id, x, y, x2, y2, duration, action } = data; if (!canActOnDevice(socket, device_id, 'write')) return; + if (capabilityRefused(device_id, 'remote.input', ack)) return; // #159: a swipe/drag carries an end point + duration (for scrolling); tap is just x/y. deviceNs.to(device_id).emit('device:remote-touch', { x, y, x2, y2, duration, action }); + if (typeof ack === 'function') ack({ delivered: true }); }); - socket.on('dashboard:remote-key', (data) => { + socket.on('dashboard:remote-key', (data, ack) => { const { device_id, keycode } = data; if (!canActOnDevice(socket, device_id, 'write')) return; + if (capabilityRefused(device_id, 'remote.input', ack)) return; console.log(`Remote key: ${keycode} -> ${device_id}`); deviceNs.to(device_id).emit('device:remote-key', { keycode }); + if (typeof ack === 'function') ack({ delivered: true }); }); // Track which devices THIS dashboard socket has a live remote (screenshot-stream) session on, so @@ -98,9 +128,10 @@ module.exports = function setupDashboardSocket(io) { // capturing every second and can starve a weak panel's decoder (the black-screen we hit). socket.remoteSessions = new Set(); - socket.on('dashboard:remote-start', (data) => { + socket.on('dashboard:remote-start', (data, ack) => { const { device_id } = data; if (!canActOnDevice(socket, device_id, 'write')) return; + if (capabilityRefused(device_id, 'remote.stream', ack)) return; const room = deviceNs.adapter.rooms.get(device_id); console.log(`Remote start for ${device_id}, room has ${room?.size || 0} socket(s)`); socket.remoteSessions.add(device_id); @@ -108,6 +139,9 @@ module.exports = function setupDashboardSocket(io) { console.log(`Remote session started for device ${device_id}`); }); + // Deliberately NOT capability-gated, for the same reason set_debug isn't: stopping is the + // thing you need most when a panel's declaration has changed underneath a live stream, and + // refusing it would strand that panel capturing forever. socket.on('dashboard:remote-stop', (data) => { const { device_id } = data; if (!canActOnDevice(socket, device_id, 'write')) return; diff --git a/server/ws/deviceSocket.js b/server/ws/deviceSocket.js index 6c66e65..2b602c9 100644 --- a/server/ws/deviceSocket.js +++ b/server/ws/deviceSocket.js @@ -523,8 +523,11 @@ function persistIdentity(deviceId, data) { // read and NO write — no UPDATE, no WAL churn. First provision (stored NULLs) and a real change // (e.g. new client_version after an OTA) still write. try { - const i = liveness.captureIdentity(data); - if (!liveness.identityChanged(_identityReadStmt.get(deviceId), i)) return; // unchanged — skip the write + const stored = _identityReadStmt.get(deviceId); + // ⚠️ A register that does not mention `platform` must not ERASE the one we have — see + // liveness.preserveKnownIdentity for why that column is load-bearing. + const i = liveness.preserveKnownIdentity(stored, liveness.captureIdentity(data)); + if (!liveness.identityChanged(stored, i)) return; // unchanged — skip the write _persistIdentityStmt.run(i.client_type, i.client_version, i.platform, i.contract_version, deviceId); } catch (e) { /* identity capture must never break registration */ } }