harden: download-clip integrity + cancel tracking + decouple editor procs

Two server-side fixes for separate clip/queue/editor crosstalk paths.

1. download-clip IPC was unsafe in three ways:
   - reported success: true on exit code 0 even with empty files
     (Twitch sometimes returns a manifest with no segments)
   - passed clipInfo.broadcaster_name straight to path.join, so unicode
     / spaces / punctuation in display names produced odd directory
     layouts on Windows
   - the spawned streamlink process was tracked nowhere, so window
     close orphaned it
   Now: sanitize broadcaster_name + title, ensureUniqueFilename so
   re-downloads do not overwrite, post-download size + integrity check
   (16 KiB floor + ffprobe via validateDownloadedFileIntegrity), proc
   tracked in activeClipProcesses and killed on window-all-closed.

2. currentProcess (a single ChildProcess global) was shared between
   cutter/merger/splitter and downloadVODPart. The real bug: while a
   queue download was running and the user kicked off a video cut,
   pressing the queue's "Stop" button iterated activeDownloads (fine)
   AND called currentProcess.kill() — which by then pointed at the
   cutter ffmpeg, killing an unrelated cut.
   Renamed to currentEditorProcess, confined to the editor pipeline.
   downloadVODPart no longer touches it. The fallback kill calls in
   remove-from-queue / pause-download / cancel-download are gone — the
   activeDownloads loop above each was already authoritative.
   window-all-closed now also kills activeClipProcesses.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
xRangerDE 2026-05-03 15:43:01 +02:00
parent 9d57c03e74
commit 31e6671e65

View File

@ -530,7 +530,11 @@ let downloadQueue: QueueItem[] = loadQueue();
let queueIdCounter = 0; let queueIdCounter = 0;
let lastQueueBroadcastFingerprint = ''; let lastQueueBroadcastFingerprint = '';
let isDownloading = false; let isDownloading = false;
let currentProcess: ChildProcess | null = null; // Process handle for the standalone video editor pipeline (cutter / merger /
// splitter). Queue downloads track their own children via activeDownloads,
// and clip downloads via activeClipProcesses. Keeping these separate
// prevents cancel-download from killing an unrelated cutter ffmpeg.
let currentEditorProcess: ChildProcess | null = null;
let currentDownloadCancelled = false; let currentDownloadCancelled = false;
let pauseRequested = false; let pauseRequested = false;
let activeQueueItemId: string | null = null; let activeQueueItemId: string | null = null;
@ -2066,7 +2070,7 @@ async function cutVideo(
return await new Promise((resolve) => { return await new Promise((resolve) => {
const proc = spawn(ffmpeg, args, { windowsHide: true }); const proc = spawn(ffmpeg, args, { windowsHide: true });
currentProcess = proc; currentEditorProcess = proc;
proc.stdout?.on('data', (data) => { proc.stdout?.on('data', (data) => {
const line = data.toString(); const line = data.toString();
@ -2079,7 +2083,7 @@ async function cutVideo(
}); });
proc.on('close', (code) => { proc.on('close', (code) => {
currentProcess = null; currentEditorProcess = null;
if (code === 0 && fs.existsSync(outputFile)) { if (code === 0 && fs.existsSync(outputFile)) {
const stats = fs.statSync(outputFile); const stats = fs.statSync(outputFile);
if (stats.size <= 256) { if (stats.size <= 256) {
@ -2094,7 +2098,7 @@ async function cutVideo(
}); });
proc.on('error', () => { proc.on('error', () => {
currentProcess = null; currentEditorProcess = null;
resolve(false); resolve(false);
}); });
}); });
@ -2208,7 +2212,7 @@ async function mergeVideos(
return await new Promise((resolve) => { return await new Promise((resolve) => {
const proc = spawn(ffmpeg, args, { windowsHide: true }); const proc = spawn(ffmpeg, args, { windowsHide: true });
currentProcess = proc; currentEditorProcess = proc;
proc.stdout?.on('data', (data) => { proc.stdout?.on('data', (data) => {
const line = data.toString(); const line = data.toString();
@ -2224,7 +2228,7 @@ async function mergeVideos(
}); });
proc.on('close', (code) => { proc.on('close', (code) => {
currentProcess = null; currentEditorProcess = null;
const success = code === 0 && fs.existsSync(outputFile); const success = code === 0 && fs.existsSync(outputFile);
if (success) { if (success) {
onProgress(100); onProgress(100);
@ -2233,7 +2237,7 @@ async function mergeVideos(
}); });
proc.on('error', () => { proc.on('error', () => {
currentProcess = null; currentEditorProcess = null;
resolve(false); resolve(false);
}); });
}); });
@ -2303,15 +2307,15 @@ async function splitMergedFile(
const success = await new Promise<boolean>((resolve) => { const success = await new Promise<boolean>((resolve) => {
const proc = spawn(ffmpeg, args, { windowsHide: true }); const proc = spawn(ffmpeg, args, { windowsHide: true });
currentProcess = proc; currentEditorProcess = proc;
proc.on('close', (code) => { proc.on('close', (code) => {
currentProcess = null; currentEditorProcess = null;
resolve(code === 0 && fs.existsSync(outputFile)); resolve(code === 0 && fs.existsSync(outputFile));
}); });
proc.on('error', () => { proc.on('error', () => {
currentProcess = null; currentEditorProcess = null;
resolve(false); resolve(false);
}); });
}); });
@ -2358,9 +2362,9 @@ function downloadVODPart(
appendDebugLog('download-part-start', { itemId, command: streamlinkCmd.command, filename, args }); appendDebugLog('download-part-start', { itemId, command: streamlinkCmd.command, filename, args });
const proc = spawn(streamlinkCmd.command, args, { windowsHide: true }); const proc = spawn(streamlinkCmd.command, args, { windowsHide: true });
currentProcess = proc;
// Register in per-item tracking map for parallel downloads // Register in per-item tracking map for parallel downloads
// (no longer mirrored on a global — currentEditorProcess is editor-only)
const itemTracking = { process: proc, cancelled: false, startTime: Date.now(), bytes: 0 }; const itemTracking = { process: proc, cancelled: false, startTime: Date.now(), bytes: 0 };
activeDownloads.set(itemId, itemTracking); activeDownloads.set(itemId, itemTracking);
@ -2451,7 +2455,6 @@ function downloadVODPart(
proc.on('close', async (code) => { proc.on('close', async (code) => {
clearInterval(progressInterval); clearInterval(progressInterval);
currentProcess = null;
activeDownloads.delete(itemId); activeDownloads.delete(itemId);
if (currentDownloadCancelled || cancelledItemIds.has(itemId)) { if (currentDownloadCancelled || cancelledItemIds.has(itemId)) {
@ -2496,7 +2499,6 @@ function downloadVODPart(
proc.on('error', (err) => { proc.on('error', (err) => {
clearInterval(progressInterval); clearInterval(progressInterval);
console.error('Process error:', err); console.error('Process error:', err);
currentProcess = null;
activeDownloads.delete(itemId); activeDownloads.delete(itemId);
const rawError = String(err); const rawError = String(err);
const errorMessage = rawError.includes('ENOENT') const errorMessage = rawError.includes('ENOENT')
@ -3582,17 +3584,12 @@ ipcMain.handle('remove-from-queue', (_, id: string) => {
const wasActiveItem = activeQueueItemId === id || activeDownloads.has(id); const wasActiveItem = activeQueueItemId === id || activeDownloads.has(id);
if (wasActiveItem) { if (wasActiveItem) {
// Cancel via per-item tracking
cancelledItemIds.add(id); cancelledItemIds.add(id);
const tracking = activeDownloads.get(id); const tracking = activeDownloads.get(id);
if (tracking?.process) { if (tracking?.process) {
tracking.process.kill(); tracking.process.kill();
} }
// Also set global for backwards compat
currentDownloadCancelled = true; currentDownloadCancelled = true;
if (currentProcess) {
currentProcess.kill();
}
activeDownloads.delete(id); activeDownloads.delete(id);
activeQueueItemId = null; activeQueueItemId = null;
runtimeMetrics.activeItemId = null; runtimeMetrics.activeItemId = null;
@ -3761,16 +3758,14 @@ ipcMain.handle('pause-download', () => {
pauseRequested = true; pauseRequested = true;
currentDownloadCancelled = true; currentDownloadCancelled = true;
// Kill all active download processes // Kill queue downloads only — cutter/merger/splitter use currentEditorProcess
// and aren't affected by pause-download.
for (const [id, tracking] of activeDownloads) { for (const [id, tracking] of activeDownloads) {
cancelledItemIds.add(id); cancelledItemIds.add(id);
if (tracking.process) { if (tracking.process) {
tracking.process.kill(); tracking.process.kill();
} }
} }
if (currentProcess) {
currentProcess.kill();
}
return true; return true;
}); });
@ -3778,16 +3773,13 @@ ipcMain.handle('cancel-download', () => {
isDownloading = false; isDownloading = false;
pauseRequested = false; pauseRequested = false;
currentDownloadCancelled = true; currentDownloadCancelled = true;
// Kill all active download processes // Kill queue downloads only — see pause-download note above.
for (const [id, tracking] of activeDownloads) { for (const [id, tracking] of activeDownloads) {
cancelledItemIds.add(id); cancelledItemIds.add(id);
if (tracking.process) { if (tracking.process) {
tracking.process.kill(); tracking.process.kill();
} }
} }
if (currentProcess) {
currentProcess.kill();
}
return true; return true;
}); });
@ -3858,6 +3850,11 @@ ipcMain.handle('open-external', async (_, url: string) => {
await shell.openExternal(url); await shell.openExternal(url);
}); });
// Tracks active standalone clip downloads so cancel-download / window-all-closed
// can kill them. Separate from activeDownloads (queue) because clip downloads
// don't go through the queue scheduler.
const activeClipProcesses = new Map<string, ChildProcess>();
ipcMain.handle('download-clip', async (_, clipUrl: string) => { ipcMain.handle('download-clip', async (_, clipUrl: string) => {
let clipId = ''; let clipId = '';
const match1 = clipUrl.match(/clips\.twitch\.tv\/([A-Za-z0-9_-]+)/); const match1 = clipUrl.match(/clips\.twitch\.tv\/([A-Za-z0-9_-]+)/);
@ -3870,7 +3867,14 @@ ipcMain.handle('download-clip', async (_, clipUrl: string) => {
const clipInfo = await getClipInfo(clipId); const clipInfo = await getClipInfo(clipId);
if (!clipInfo) return { success: false, error: 'Clip nicht gefunden' }; if (!clipInfo) return { success: false, error: 'Clip nicht gefunden' };
const folder = path.join(config.download_path, 'Clips', clipInfo.broadcaster_name); // Sanitize broadcaster_name for path safety — Twitch returns the display
// name which can contain unicode, spaces, or punctuation that breaks
// path joining on some Windows configurations.
const safeBroadcaster = sanitizeFilenamePart(
typeof clipInfo.broadcaster_name === 'string' ? clipInfo.broadcaster_name : '',
'unknown'
);
const folder = path.join(config.download_path, 'Clips', safeBroadcaster);
fs.mkdirSync(folder, { recursive: true }); fs.mkdirSync(folder, { recursive: true });
const clipDiskCheck = ensureDiskSpace(folder, 128 * 1024 * 1024, 'Clip-Download'); const clipDiskCheck = ensureDiskSpace(folder, 128 * 1024 * 1024, 'Clip-Download');
@ -3878,10 +3882,14 @@ ipcMain.handle('download-clip', async (_, clipUrl: string) => {
return { success: false, error: clipDiskCheck.error || 'Zu wenig Speicherplatz.' }; return { success: false, error: clipDiskCheck.error || 'Zu wenig Speicherplatz.' };
} }
const safeTitle = clipInfo.title.replace(/[^a-zA-Z0-9_\- ]/g, '').substring(0, 50); const rawTitle = typeof clipInfo.title === 'string' ? clipInfo.title : '';
const filename = path.join(folder, `${safeTitle}.mp4`); const safeTitle = (rawTitle.replace(/[^a-zA-Z0-9_\- ]/g, '').trim().substring(0, 50)) || 'clip';
// Use ensureUniqueFilename so retrying a clip with the same title doesn't
// overwrite the previous download. itemId is the clipId — if the user
// cancels via cancel-download, that's the handle.
const filename = ensureUniqueFilename(path.join(folder, `${safeTitle}.mp4`), clipId);
return new Promise((resolve) => { return new Promise<{ success: boolean; error?: string; filename?: string }>((resolve) => {
const streamlinkCmd = getStreamlinkCommand(); const streamlinkCmd = getStreamlinkCommand();
const proc = spawn(streamlinkCmd.command, [ const proc = spawn(streamlinkCmd.command, [
...streamlinkCmd.prefixArgs, ...streamlinkCmd.prefixArgs,
@ -3891,15 +3899,45 @@ ipcMain.handle('download-clip', async (_, clipUrl: string) => {
'--force' '--force'
], { windowsHide: true }); ], { windowsHide: true });
activeClipProcesses.set(clipId, proc);
appendDebugLog('clip-download-start', { clipId, broadcaster: safeBroadcaster, filename });
proc.on('close', (code) => { proc.on('close', (code) => {
if (code === 0 && fs.existsSync(filename)) { activeClipProcesses.delete(clipId);
resolve({ success: true, filename }); releaseClaimedFilenamesForItem(clipId);
} else {
if (code !== 0 || !fs.existsSync(filename)) {
appendDebugLog('clip-download-failed', { clipId, code });
resolve({ success: false, error: `Download fehlgeschlagen (Exit-Code ${code ?? -1})` }); resolve({ success: false, error: `Download fehlgeschlagen (Exit-Code ${code ?? -1})` });
return;
} }
// Integrity: clips are short but should still be at least a few KB
// and parse as a video stream via ffprobe. Empty/zero-byte files
// were previously reported as "success" because exit code was 0.
const stats = fs.statSync(filename);
if (stats.size < 16 * 1024) {
try { fs.unlinkSync(filename); } catch { }
appendDebugLog('clip-download-too-small', { clipId, bytes: stats.size });
resolve({ success: false, error: `Clip-Datei zu klein (${stats.size} Bytes) — Twitch hat den Stream evtl. nicht ausgeliefert.` });
return;
}
const integrity = validateDownloadedFileIntegrity(filename, null);
if (!integrity.success) {
try { fs.unlinkSync(filename); } catch { }
appendDebugLog('clip-download-integrity-failed', { clipId, error: integrity.error });
resolve({ success: false, error: integrity.error || 'Integritaetspruefung fehlgeschlagen.' });
return;
}
appendDebugLog('clip-download-success', { clipId, bytes: stats.size, filename });
resolve({ success: true, filename });
}); });
proc.on('error', () => { proc.on('error', () => {
activeClipProcesses.delete(clipId);
releaseClaimedFilenamesForItem(clipId);
resolve({ success: false, error: 'Streamlink nicht gefunden' }); resolve({ success: false, error: 'Streamlink nicht gefunden' });
}); });
}); });
@ -4018,14 +4056,18 @@ app.on('window-all-closed', () => {
stopDebugLogFlushTimer(true); stopDebugLogFlushTimer(true);
stopAutoUpdatePolling(); stopAutoUpdatePolling();
// Kill all active download processes // Kill all active children: queue downloads, standalone clip downloads,
// and any in-flight cutter/merger/splitter ffmpeg.
for (const [, tracking] of activeDownloads) { for (const [, tracking] of activeDownloads) {
if (tracking.process) { if (tracking.process) {
tracking.process.kill(); tracking.process.kill();
} }
} }
if (currentProcess) { for (const [, proc] of activeClipProcesses) {
currentProcess.kill(); try { proc.kill(); } catch { }
}
if (currentEditorProcess) {
currentEditorProcess.kill();
} }
saveConfig(config); saveConfig(config);
flushQueueSave(); flushQueueSave();