Fix 8 security findings from Phase 3 audit + device-detail banner refresh

Security fixes:
- Critical: Add ownership checks to assignments PUT/:id and DELETE/:id (IDOR)
- Critical: Add ownership checks to assignments copy-to endpoint for both devices
- High: Validate device ownership when adding to device groups
- High: UUID-validate content ID before LIKE query + scope to owner's playlists
- Low: Handle FK violations gracefully in playlist discard (deleted content/widgets)
- Low: Escape mime_type with esc() in playlist item display (XSS)

Bug fix:
- Device-detail mutation handlers now reload full page to show draft banner

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
ScreenTinker 2026-04-13 21:36:16 -05:00
parent f30d8b82cd
commit 470197d203
6 changed files with 38 additions and 20 deletions

View file

@ -1018,9 +1018,7 @@ async function setupPlaylistActions(device) {
} }
modal.remove(); modal.remove();
showToast('Added to playlist', 'success'); showToast('Added to playlist', 'success');
const assignments = await api.getAssignments(device.id); loadDevice(device.id, 'playlist');
document.getElementById('playlistContainer').innerHTML = renderPlaylist(assignments);
attachRemoveHandlers(device);
} catch (err) { } catch (err) {
showToast(err.message, 'error'); showToast(err.message, 'error');
} }
@ -1063,6 +1061,7 @@ function attachRemoveHandlers(device) {
try { try {
await api.updateAssignment(assignmentId, { zone_id: select.value || null }); await api.updateAssignment(assignmentId, { zone_id: select.value || null });
showToast(`Zone updated`, 'success'); showToast(`Zone updated`, 'success');
loadDevice(device.id, 'playlist');
} catch (err) { showToast(err.message, 'error'); } } catch (err) { showToast(err.message, 'error'); }
}; };
}); });
@ -1078,9 +1077,7 @@ function attachRemoveHandlers(device) {
try { try {
await api.updateAssignment(id, { muted: !currentlyMuted }); await api.updateAssignment(id, { muted: !currentlyMuted });
showToast(currentlyMuted ? 'Unmuted' : 'Muted', 'success'); showToast(currentlyMuted ? 'Unmuted' : 'Muted', 'success');
const assignments = await api.getAssignments(device.id); loadDevice(device.id, 'playlist');
document.getElementById('playlistContainer').innerHTML = renderPlaylist(assignments);
attachRemoveHandlers(device);
} catch (err) { showToast(err.message, 'error'); } } catch (err) { showToast(err.message, 'error'); }
}); });
}); });
@ -1093,9 +1090,7 @@ function attachRemoveHandlers(device) {
try { try {
await api.deleteAssignment(id); await api.deleteAssignment(id);
showToast('Content removed from playlist', 'success'); showToast('Content removed from playlist', 'success');
const assignments = await api.getAssignments(device.id); loadDevice(device.id, 'playlist');
document.getElementById('playlistContainer').innerHTML = renderPlaylist(assignments);
attachRemoveHandlers(device);
} catch (err) { } catch (err) {
showToast(err.message, 'error'); showToast(err.message, 'error');
} }
@ -1146,12 +1141,10 @@ function attachRemoveHandlers(device) {
try { try {
await api.reorderAssignments(device.id, newOrder); await api.reorderAssignments(device.id, newOrder);
showToast('Playlist reordered', 'success'); showToast('Playlist reordered', 'success');
loadDevice(device.id, 'playlist');
} catch (err) { } catch (err) {
showToast(err.message, 'error'); showToast(err.message, 'error');
// Reload to revert loadDevice(device.id, 'playlist');
const assignments = await api.getAssignments(device.id);
container.innerHTML = renderPlaylist(assignments);
attachRemoveHandlers(device);
} }
}); });
}); });

View file

@ -304,7 +304,7 @@ function renderItems(items) {
</div> </div>
<div style="flex:1;min-width:0"> <div style="flex:1;min-width:0">
<div style="font-size:14px;color:var(--text-primary);white-space:nowrap;overflow:hidden;text-overflow:ellipsis">${esc(item.filename || item.widget_name || 'Unknown')}</div> <div style="font-size:14px;color:var(--text-primary);white-space:nowrap;overflow:hidden;text-overflow:ellipsis">${esc(item.filename || item.widget_name || 'Unknown')}</div>
<div style="font-size:12px;color:var(--text-muted)">${item.widget_id ? 'Widget' : (item.mime_type || 'Unknown type')}</div> <div style="font-size:12px;color:var(--text-muted)">${item.widget_id ? 'Widget' : esc(item.mime_type || 'Unknown type')}</div>
</div> </div>
<div style="display:flex;align-items:center;gap:8px;flex-shrink:0"> <div style="display:flex;align-items:center;gap:8px;flex-shrink:0">
<label style="font-size:12px;color:var(--text-muted)">Duration</label> <label style="font-size:12px;color:var(--text-muted)">Duration</label>

View file

@ -9,8 +9,8 @@ function markDraft(playlistId) {
} }
// Check device ownership for device-scoped routes // Check device ownership for device-scoped routes
function checkDeviceAccess(req, res) { function checkDeviceAccess(req, res, paramName = 'deviceId') {
const device = db.prepare('SELECT user_id FROM devices WHERE id = ?').get(req.params.deviceId); const device = db.prepare('SELECT user_id FROM devices WHERE id = ?').get(req.params[paramName]);
if (!device) { res.status(404).json({ error: 'Device not found' }); return false; } if (!device) { res.status(404).json({ error: 'Device not found' }); return false; }
if (!['admin','superadmin'].includes(req.user.role) && device.user_id && device.user_id !== req.user.id) { if (!['admin','superadmin'].includes(req.user.role) && device.user_id && device.user_id !== req.user.id) {
res.status(403).json({ error: 'Access denied' }); return false; res.status(403).json({ error: 'Access denied' }); return false;
@ -105,6 +105,9 @@ router.post('/device/:deviceId', (req, res) => {
router.put('/:id', (req, res) => { router.put('/:id', (req, res) => {
const item = db.prepare('SELECT pi.*, p.user_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id); const item = db.prepare('SELECT pi.*, p.user_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id);
if (!item) return res.status(404).json({ error: 'Item not found' }); if (!item) return res.status(404).json({ error: 'Item not found' });
if (!['admin','superadmin'].includes(req.user.role) && item.user_id !== req.user.id) {
return res.status(403).json({ error: 'Access denied' });
}
const { sort_order, duration_sec, zone_id } = req.body; const { sort_order, duration_sec, zone_id } = req.body;
const updates = []; const updates = [];
@ -128,6 +131,9 @@ router.put('/:id', (req, res) => {
router.delete('/:id', (req, res) => { router.delete('/:id', (req, res) => {
const item = db.prepare('SELECT pi.*, p.user_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id); const item = db.prepare('SELECT pi.*, p.user_id FROM playlist_items pi JOIN playlists p ON pi.playlist_id = p.id WHERE pi.id = ?').get(req.params.id);
if (!item) return res.status(404).json({ error: 'Item not found' }); if (!item) return res.status(404).json({ error: 'Item not found' });
if (!['admin','superadmin'].includes(req.user.role) && item.user_id !== req.user.id) {
return res.status(403).json({ error: 'Access denied' });
}
db.prepare('DELETE FROM playlist_items WHERE id = ?').run(req.params.id); db.prepare('DELETE FROM playlist_items WHERE id = ?').run(req.params.id);
markDraft(item.playlist_id); markDraft(item.playlist_id);
@ -161,6 +167,8 @@ router.post('/device/:deviceId/reorder', (req, res) => {
// Copy playlist from one device to another // Copy playlist from one device to another
router.post('/device/:deviceId/copy-to/:targetDeviceId', (req, res) => { router.post('/device/:deviceId/copy-to/:targetDeviceId', (req, res) => {
if (!checkDeviceAccess(req, res, 'deviceId')) return;
if (!checkDeviceAccess(req, res, 'targetDeviceId')) return;
const sourceDevice = db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(req.params.deviceId); const sourceDevice = db.prepare('SELECT playlist_id FROM devices WHERE id = ?').get(req.params.deviceId);
if (!sourceDevice?.playlist_id) return res.status(404).json({ error: 'Source device has no playlist' }); if (!sourceDevice?.playlist_id) return res.status(404).json({ error: 'Source device has no playlist' });

View file

@ -317,9 +317,12 @@ router.delete('/:id', (req, res) => {
`).all(req.params.id); `).all(req.params.id);
// Scrub published snapshots that reference this content // Scrub published snapshots that reference this content
// Validate UUID format to prevent LIKE wildcard injection
const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
if (!UUID_RE.test(req.params.id)) return res.status(400).json({ error: 'Invalid content ID format' });
const snapshotPlaylists = db.prepare( const snapshotPlaylists = db.prepare(
"SELECT id, published_snapshot FROM playlists WHERE published_snapshot LIKE ?" "SELECT id, published_snapshot FROM playlists WHERE user_id = ? AND published_snapshot LIKE ?"
).all(`%${req.params.id}%`); ).all(content.user_id, `%${req.params.id}%`);
for (const pl of snapshotPlaylists) { for (const pl of snapshotPlaylists) {
try { try {
const items = JSON.parse(pl.published_snapshot); const items = JSON.parse(pl.published_snapshot);

View file

@ -68,6 +68,12 @@ router.get('/:id/devices', requireGroupOwnership, (req, res) => {
router.post('/:id/devices', requireGroupOwnership, (req, res) => { router.post('/:id/devices', requireGroupOwnership, (req, res) => {
const { device_id } = req.body; const { device_id } = req.body;
if (!device_id) return res.status(400).json({ error: 'device_id required' }); if (!device_id) return res.status(400).json({ error: 'device_id required' });
// Verify device belongs to the user (admin/superadmin bypass)
const device = db.prepare('SELECT user_id FROM devices WHERE id = ?').get(device_id);
if (!device) return res.status(404).json({ error: 'Device not found' });
if (!['admin','superadmin'].includes(req.user.role) && device.user_id && device.user_id !== req.user.id) {
return res.status(403).json({ error: 'Access denied' });
}
try { try {
db.prepare('INSERT OR IGNORE INTO device_group_members (device_id, group_id) VALUES (?, ?)').run(device_id, req.params.id); db.prepare('INSERT OR IGNORE INTO device_group_members (device_id, group_id) VALUES (?, ?)').run(device_id, req.params.id);
res.status(201).json({ success: true }); res.status(201).json({ success: true });

View file

@ -167,10 +167,18 @@ router.post('/:id/discard', requirePlaylistOwnership, (req, res) => {
const transaction = db.transaction(() => { const transaction = db.transaction(() => {
// Clear current draft items // Clear current draft items
db.prepare('DELETE FROM playlist_items WHERE playlist_id = ?').run(req.params.id); db.prepare('DELETE FROM playlist_items WHERE playlist_id = ?').run(req.params.id);
// Re-insert from snapshot // Re-insert from snapshot, skipping items whose content/widget was deleted
const insert = db.prepare('INSERT INTO playlist_items (playlist_id, content_id, widget_id, sort_order, duration_sec) VALUES (?, ?, ?, ?, ?)'); const insert = db.prepare('INSERT INTO playlist_items (playlist_id, content_id, widget_id, sort_order, duration_sec) VALUES (?, ?, ?, ?, ?)');
for (const item of publishedItems) { for (const item of publishedItems) {
insert.run(req.params.id, item.content_id || null, item.widget_id || null, item.sort_order, item.duration_sec); try {
insert.run(req.params.id, item.content_id || null, item.widget_id || null, item.sort_order, item.duration_sec);
} catch (e) {
if (e.message.includes('FOREIGN KEY')) {
console.warn(`Discard: skipping snapshot item (content_id=${item.content_id}, widget_id=${item.widget_id}) — referenced entity was deleted`);
continue;
}
throw e;
}
} }
db.prepare("UPDATE playlists SET status = 'published', updated_at = strftime('%s','now') WHERE id = ?").run(req.params.id); db.prepare("UPDATE playlists SET status = 'published', updated_at = strftime('%s','now') WHERE id = ?").run(req.params.id);
}); });