fix(server): proxy remote YouTube thumbnails + real version in boot banner (#131)

* fix(server): proxy remote YouTube thumbnails instead of ENOENT on a local path

YouTube content stores thumbnail_path as a REMOTE URL
(https://img.youtube.com/vi/<id>/hqdefault.jpg), but the thumbnail-serving route
path.resolve'd it into contentDir -> a local file that never existed -> ENOENT logged
a few times a minute (the tester-log spam). Recreating content didn't help (new rows
store the same remote URL).

- GET /api/content/:id/thumbnail now proxies a remote http(s) thumbnail_path
  server-side (same-origin, so dashboard CSP img-src is unaffected) via a non-throwing
  helper: upstream 404 -> 404, other failure/timeout -> 502, image/* only (modest SSRF
  hardening; the URL is server-set at ingest). Local thumbnails keep the sendFile path;
  the playlist/widget/workspace access gating is unchanged for both branches.
- routes/widgets.js inlineUserContent skips the disk read for a remote thumbnail and
  leaves the /api/content/:id/thumbnail reference in place (the proxy serves it).
- routes/content.js ingest unchanged; a comment notes the future download-at-ingest +
  backfill option for CDN independence.
- New test/thumbnail-proxy.test.js: local sendFile still works; a remote thumbnail is
  proxied (mock upstream, no local read, no ENOENT); upstream 404 -> clean 404. Full
  server suite 164/164.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(server): boot banner shows the real version, not a hardcoded v1.2.0

The startup ASCII banner printed "ScreenTinker Server v1.2.0". Use the already-imported
VERSION (require('./version'), the single source of truth that reads the root VERSION
file) in a fixed-width field (VERSION.padEnd(22).slice(0, 22) — the same padEnd
discipline the port line uses) so the fixed-width box border stays aligned for any
version length. No other behavior changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
screentinker 2026-06-18 17:00:24 -05:00 committed by GitHub
parent 6f0e4a07f6
commit 618a7048c6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 157 additions and 1 deletions

View file

@ -154,6 +154,10 @@ router.post('/youtube', async (req, res) => {
const id = uuidv4(); const id = uuidv4();
const embedUrl = `https://www.youtube.com/embed/${videoId}?autoplay=1&mute=1&controls=0&rel=0&modestbranding=1&loop=1&playlist=${videoId}&enablejsapi=1`; const embedUrl = `https://www.youtube.com/embed/${videoId}?autoplay=1&mute=1&controls=0&rel=0&modestbranding=1&loop=1&playlist=${videoId}&enablejsapi=1`;
// thumbnail_path is a REMOTE URL here; the /api/content/:id/thumbnail route proxies
// remote thumbnails server-side (so this isn't a local-file path). Future option for
// CDN independence: download the thumbnail at ingest into contentDir + backfill
// existing rows, then this would store a local filename like image uploads do.
const thumbnailUrl = `https://img.youtube.com/vi/${videoId}/hqdefault.jpg`; const thumbnailUrl = `https://img.youtube.com/vi/${videoId}/hqdefault.jpg`;
db.prepare(` db.prepare(`

View file

@ -25,6 +25,11 @@ function inlineUserContent(html, workspaceId) {
if (c.workspace_id && c.workspace_id !== workspaceId) return match; if (c.workspace_id && c.workspace_id !== workspaceId) return match;
const filename = kind === 'thumbnail' ? c.thumbnail_path : c.filepath; const filename = kind === 'thumbnail' ? c.thumbnail_path : c.filepath;
if (!filename) return match; if (!filename) return match;
// YouTube (and other remote-sourced) content stores thumbnail_path as a remote
// http(s) URL, not a local file. Don't try to read it from disk (would ENOENT the
// same way the serving route did) — leave the /api/content/:id/thumbnail reference
// in place; the thumbnail route proxies it same-origin and CSP img-src allows https:.
if (/^https?:\/\//i.test(filename)) return match;
const mime = kind === 'thumbnail' ? 'image/jpeg' : c.mime_type; const mime = kind === 'thumbnail' ? 'image/jpeg' : c.mime_type;
if (!mime || !MIME_RE.test(mime)) return match; if (!mime || !MIME_RE.test(mime)) return match;
const safe = path.resolve(appConfig.contentDir, path.basename(filename)); const safe = path.resolve(appConfig.contentDir, path.basename(filename));

View file

@ -446,6 +446,28 @@ app.get('/api/content/:id/file', (req, res) => {
res.sendFile(safePath); res.sendFile(safePath);
}); });
// Proxy a remote thumbnail (e.g. YouTube's img.youtube.com/.../hqdefault.jpg, which
// content.js stores as thumbnail_path) server-side, SAME-ORIGIN, so the dashboard CSP
// img-src is unaffected. Never throws into the process: any upstream/network failure
// becomes a clean 404/502. Restricted to image/* responses (modest SSRF hardening; the
// URL is server-set at ingest, not caller-supplied). Thumbnails are small, so buffering
// is fine and avoids partial-stream error handling.
async function proxyRemoteThumbnail(url, res) {
try {
const upstream = await fetch(url, { redirect: 'follow', signal: AbortSignal.timeout(8000) });
if (upstream.status === 404) return res.status(404).json({ error: 'Thumbnail not found' });
if (!upstream.ok) return res.status(502).json({ error: 'Thumbnail upstream error' });
const ct = upstream.headers.get('content-type') || 'image/jpeg';
if (!/^image\//i.test(ct)) return res.status(502).json({ error: 'Thumbnail upstream is not an image' });
const buf = Buffer.from(await upstream.arrayBuffer());
res.set('Content-Type', ct);
res.set('Cache-Control', 'public, max-age=86400');
return res.send(buf);
} catch (e) {
return res.status(502).json({ error: 'Thumbnail fetch failed' });
}
}
// Public thumbnail serving (must be BEFORE protected routes) // Public thumbnail serving (must be BEFORE protected routes)
app.get('/api/content/:id/thumbnail', (req, res) => { app.get('/api/content/:id/thumbnail', (req, res) => {
const { db } = require('./db/database'); const { db } = require('./db/database');
@ -458,6 +480,11 @@ app.get('/api/content/:id/thumbnail', (req, res) => {
const inPlaylist = db.prepare('SELECT id FROM playlist_items WHERE content_id = ? LIMIT 1').get(req.params.id); const inPlaylist = db.prepare('SELECT id FROM playlist_items WHERE content_id = ? LIMIT 1').get(req.params.id);
const inWidget = inPlaylist ? null : db.prepare('SELECT id FROM widgets WHERE workspace_id = ? AND config LIKE ? LIMIT 1').get(content.workspace_id, `%/api/content/${req.params.id}/%`); const inWidget = inPlaylist ? null : db.prepare('SELECT id FROM widgets WHERE workspace_id = ? AND config LIKE ? LIMIT 1').get(content.workspace_id, `%/api/content/${req.params.id}/%`);
if (!inPlaylist && !inWidget && !requesterCanAccessContent(req, content)) return res.status(403).json({ error: 'Content not assigned to any playlist or widget' }); if (!inPlaylist && !inWidget && !requesterCanAccessContent(req, content)) return res.status(403).json({ error: 'Content not assigned to any playlist or widget' });
// YouTube (and any future remote-sourced) content stores thumbnail_path as a remote
// http(s) URL, not a local file. Proxy it instead of resolving it to a local path that
// doesn't exist (contentDir/hqdefault.jpg -> ENOENT spam). Local thumbnails are
// unchanged. Access gating above already ran identically for both branches.
if (/^https?:\/\//i.test(content.thumbnail_path)) return proxyRemoteThumbnail(content.thumbnail_path, res);
const safePath = path.resolve(config.contentDir, path.basename(content.thumbnail_path)); const safePath = path.resolve(config.contentDir, path.basename(content.thumbnail_path));
if (!safePath.startsWith(path.resolve(config.contentDir))) return res.status(403).json({ error: 'Invalid path' }); if (!safePath.startsWith(path.resolve(config.contentDir))) return res.status(403).json({ error: 'Invalid path' });
res.sendFile(safePath); res.sendFile(safePath);
@ -715,7 +742,7 @@ const protocol = hasSsl ? 'https' : 'http';
server.listen(listenPort, '0.0.0.0', () => { server.listen(listenPort, '0.0.0.0', () => {
console.log(` console.log(`
ScreenTinker Server v1.2.0 ScreenTinker Server v${VERSION.padEnd(22).slice(0, 22)}
Dashboard: ${protocol}://localhost:${String(listenPort).padEnd(5)} ║ Dashboard: ${protocol}://localhost:${String(listenPort).padEnd(5)} ║
API: ${protocol}://localhost:${String(listenPort).padEnd(5)}/api ║ API: ${protocol}://localhost:${String(listenPort).padEnd(5)}/api ║

View file

@ -0,0 +1,120 @@
'use strict';
// Thumbnail serving: remote (proxied) vs local (sendFile). Regression test for the
// YouTube hqdefault.jpg ENOENT bug — content.js stores thumbnail_path as a REMOTE URL
// (https://img.youtube.com/vi/<id>/hqdefault.jpg), and the serving route used to
// path.resolve it into contentDir -> a local file that never existed -> ENOENT spam.
// Now the route proxies remote http(s) thumbnails server-side; local files still
// sendFile unchanged. A local HTTP server stands in for img.youtube.com (the "mock
// upstream") so no network is needed.
const { test, before, after } = require('node:test');
const assert = require('node:assert/strict');
const { spawn } = require('node:child_process');
const http = require('node:http');
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 PORT = 3990;
const BASE = `http://127.0.0.1:${PORT}`;
const DATA_DIR = path.join(os.tmpdir(), 'st-thumb-test-' + crypto.randomBytes(4).toString('hex'));
const CONTENT_DIR = path.join(DATA_DIR, 'uploads', 'content');
const LOG = path.join(os.tmpdir(), 'st-thumb-' + crypto.randomBytes(4).toString('hex') + '.log');
// 1x1 PNG.
const PNG = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+M8AAAMCAYAAAAxN7CkAAAAASUVORK5CYII=', 'base64');
let proc, upstream, upstreamPort, upstreamHits = 0, seedDb;
async function jget(p) {
const res = await fetch(BASE + p);
const buf = Buffer.from(await res.arrayBuffer());
return { status: res.status, type: res.headers.get('content-type') || '', buf };
}
// Insert a content row + a playlist_items row (so the public thumbnail gate passes).
// Uses ONE long-lived seeding connection (opened in before()): opening a fresh write
// connection per insert can race WAL visibility against the server's reader. FK
// enforcement is disabled on this connection so a dummy playlist_id needs no real playlist.
function makeContent(thumbnailPath, { mime = 'image/png' } = {}) {
const id = crypto.randomUUID();
seedDb.prepare("INSERT INTO content (id, filename, filepath, mime_type, file_size, thumbnail_path) VALUES (?,?,?,?,0,?)")
.run(id, 'item', '', mime, thumbnailPath);
seedDb.prepare('INSERT INTO playlist_items (playlist_id, content_id) VALUES (?, ?)').run('pl-test', id);
return id;
}
before(async () => {
// Mock upstream standing in for img.youtube.com. /missing/* -> 404 to exercise the
// clean-failure path; everything else -> a 200 image/png.
upstream = http.createServer((req, res) => {
upstreamHits++;
if (req.url.includes('missing')) { res.writeHead(404); res.end('not found'); return; }
res.writeHead(200, { 'Content-Type': 'image/png' }); res.end(PNG);
});
await new Promise((r) => upstream.listen(0, '127.0.0.1', r));
upstreamPort = upstream.address().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 { /* not yet */ }
await new Promise((r) => setTimeout(r, 250));
}
if (!up) throw new Error('server did not boot:\n' + fs.readFileSync(LOG, 'utf8').slice(-2000));
seedDb = new Database(path.join(DATA_DIR, 'db', 'remote_display.db'), { timeout: 5000 });
seedDb.pragma('foreign_keys = OFF');
});
after(() => {
try { seedDb.close(); } catch { /* */ }
try { proc.kill('SIGKILL'); } catch { /* */ }
try { upstream.close(); } catch { /* */ }
for (const f of [DATA_DIR, LOG]) { try { fs.rmSync(f, { recursive: true, force: true }); } catch { /* */ } }
});
test('local-file thumbnail still serves via sendFile', () => {
fs.mkdirSync(CONTENT_DIR, { recursive: true });
fs.writeFileSync(path.join(CONTENT_DIR, 'localthumb.png'), PNG);
const id = makeContent('localthumb.png');
return jget(`/api/content/${id}/thumbnail`).then((r) => {
assert.equal(r.status, 200, 'local thumbnail served');
assert.match(r.type, /^image\//, 'image content-type');
assert.ok(r.buf.equals(PNG), 'served the local bytes');
});
});
test('remote http thumbnail is proxied (no local read, no ENOENT)', async () => {
const before = upstreamHits;
// A YouTube-style remote thumbnail. The basename is hqdefault.jpg — the exact name
// the old bug tried (and failed) to read from contentDir.
const id = makeContent(`http://127.0.0.1:${upstreamPort}/vi/abc/hqdefault.jpg`, { mime: 'video/youtube' });
// The local file the buggy path would have looked for must NOT exist.
assert.ok(!fs.existsSync(path.join(CONTENT_DIR, 'hqdefault.jpg')), 'no local hqdefault.jpg exists');
const r = await jget(`/api/content/${id}/thumbnail`);
assert.equal(r.status, 200, 'remote thumbnail proxied');
assert.equal(r.type, 'image/png', 'upstream content-type passed through');
assert.ok(r.buf.equals(PNG), 'served the upstream bytes');
assert.equal(upstreamHits, before + 1, 'fetched the upstream once (proxied, not read from disk)');
// The bug symptom was ENOENT spam in the logs; the proxy path must not produce it.
const log = fs.readFileSync(LOG, 'utf8');
assert.ok(!log.includes('ENOENT'), 'no ENOENT logged for the remote thumbnail');
});
test('remote upstream 404 yields a clean 404 (process stays up)', async () => {
const id = makeContent(`http://127.0.0.1:${upstreamPort}/vi/missing/hqdefault.jpg`, { mime: 'video/youtube' });
const r = await jget(`/api/content/${id}/thumbnail`);
assert.equal(r.status, 404, 'upstream 404 maps to a clean 404');
// server still alive
assert.equal((await fetch(BASE + '/api/status')).ok, true, 'server survived the upstream failure');
});