From 80a29356e0e679e99009ed70bf0a3c62d12a6cfb Mon Sep 17 00:00:00 2001 From: NimBold Date: Wed, 15 Jul 2026 03:00:46 +0330 Subject: [PATCH] fix(tools): harden scheduler limits and logs Guard scheduler system actions against pending work, serialize diagnostic log transitions, and keep speed-limit saves truthful after backend failures.\n\nNo linked issue was found for this audit. --- src-tauri/src/lib.rs | 33 ++++++++- src/App.tsx | 45 ++++++------ src/components/LogsView.tsx | 92 +++++++++++++++++++------ src/components/SchedulerView.tsx | 5 +- src/components/SpeedLimiterView.test.ts | 11 +++ src/components/SpeedLimiterView.tsx | 62 +++++++++++------ src/store/useSettingsStore.test.ts | 27 ++++++++ src/store/useSettingsStore.ts | 8 ++- src/utils/logger.test.ts | 61 ++++++++++++++++ src/utils/logger.ts | 20 +++++- 10 files changed, 290 insertions(+), 74 deletions(-) create mode 100644 src/components/SpeedLimiterView.test.ts create mode 100644 src/store/useSettingsStore.test.ts create mode 100644 src/utils/logger.test.ts diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 5072f01..08f2f27 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -5003,23 +5003,39 @@ pub(crate) fn redact_sensitive_text(line: &str) -> String { static SECRET: OnceLock = OnceLock::new(); static HEADER: OnceLock = OnceLock::new(); static QUERY: OnceLock = OnceLock::new(); + static USERINFO: OnceLock = OnceLock::new(); + static FRAGMENT: OnceLock = OnceLock::new(); let secret = SECRET.get_or_init(|| { regex::Regex::new( - r"(?i)(authorization|cookie|password|token|secret)\s*[:=]\s*([^\r\n,;]+)", + r"(?i)(authorization|proxy-authorization|cookie|set-cookie|password|token|secret|credential|pairing[-_ ]?token|api[-_ ]?key)\s*[:=]\s*([^\r\n,;]+)", ) .expect("valid secret redaction regex") }); let header = HEADER.get_or_init(|| { - regex::Regex::new(r"(?i)(authorization|cookie)\s*:\s*[^\r\n]+") + regex::Regex::new( + r"(?i)(authorization|proxy-authorization|cookie|set-cookie)\s*:\s*[^\r\n]+", + ) .expect("valid sensitive header redaction regex") }); let query = QUERY.get_or_init(|| { regex::Regex::new(r"([A-Za-z][A-Za-z0-9+.-]*://[^\s?]+)\?[^\s]+") .expect("valid URL query redaction regex") }); + let userinfo = USERINFO.get_or_init(|| { + regex::Regex::new(r"(?i)([A-Za-z][A-Za-z0-9+.-]*://)[^@\s/?#]+@") + .expect("valid URL userinfo redaction regex") + }); + let fragment = FRAGMENT.get_or_init(|| { + regex::Regex::new(r"([A-Za-z][A-Za-z0-9+.-]*://[^\s?#]+)#\S+") + .expect("valid URL fragment redaction regex") + }); let redacted = header.replace_all(line, "$1: [redacted]"); let redacted = secret.replace_all(&redacted, "$1=[redacted]"); - query.replace_all(&redacted, "$1?[redacted]").into_owned() + let redacted = userinfo.replace_all(&redacted, "$1[redacted]@"); + let redacted = query.replace_all(&redacted, "$1?[redacted]"); + fragment + .replace_all(&redacted, "$1#[redacted]") + .into_owned() } fn redact_log_line(line: &str) -> String { @@ -5617,6 +5633,17 @@ mod tests { assert!(redacted.contains("[redacted]")); } + #[test] + fn redacts_proxy_credentials_pairing_tokens_and_url_fragments() { + let line = "Proxy-Authorization: Basic abc\npairing token: pair-secret\nhttp://user:pass@example.com/file#signature=secret"; + let redacted = redact_log_line(line); + assert!(!redacted.contains("Basic abc")); + assert!(!redacted.contains("pair-secret")); + assert!(!redacted.contains("user:pass")); + assert!(!redacted.contains("signature=secret")); + assert!(redacted.contains("http://[redacted]@example.com/file#[redacted]")); + } + #[test] fn collects_primary_url_and_unique_mirrors_in_order() { let uris = collect_download_uris( diff --git a/src/App.tsx b/src/App.tsx index 91d44e9..9e2c0b4 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,4 +1,4 @@ -import { initMediaDomains, isActiveDownloadStatus, isTransferActiveStatus, normalizeSpeedLimitForBackend } from './utils/downloads'; +import { initMediaDomains, isActiveDownloadStatus, isTransferActiveStatus } from './utils/downloads'; import { schedulerCompletionState } from './utils/schedulerCompletion'; import { useCallback, useEffect, useRef, useState } from "react"; import { Sidebar, SidebarFilter } from "./components/Sidebar"; @@ -19,6 +19,7 @@ import LogsView from "./components/LogsView"; import { KeychainPermissionModal } from "./components/KeychainPermissionModal"; import { WindowControls } from "./components/WindowControls"; import { useToast } from "./contexts/ToastContext"; +import { setLogStreamActive } from './utils/logger'; import { openUrl } from '@tauri-apps/plugin-opener'; import { usePlatformInfo } from './utils/platform'; import type { PostQueueAction } from './bindings/PostQueueAction'; @@ -109,7 +110,6 @@ function App() { const showNotifications = useSettingsStore(state => state.showNotifications); const showDockBadge = useSettingsStore(state => state.showDockBadge); const showMenuBarIcon = useSettingsStore(state => state.showMenuBarIcon); - const logsEnabled = useSettingsStore(state => state.logsEnabled); const extensionPairingToken = useSettingsStore(state => state.extensionPairingToken); const downloads = useDownloadStore(state => state.downloads); const activeDownloadCount = downloads.filter(download => isTransferActiveStatus(download.status)).length; @@ -119,8 +119,6 @@ function App() { const doneCount = downloads.filter(download => download.status === 'completed').length; const schedulerRunning = useSettingsStore(state => state.schedulerRunning); const schedulerActiveDownloadIds = useSettingsStore(state => state.schedulerActiveDownloadIds); - const globalSpeedLimit = useSettingsStore(state => state.globalSpeedLimit); - const previousSpeedLimit = useRef(null); const pendingPostActionTimer = useRef(null); const maxConcurrentDownloads = useSettingsStore(state => state.maxConcurrentDownloads); const preventsSleepWhileDownloading = useSettingsStore(state => state.preventsSleepWhileDownloading); @@ -189,7 +187,7 @@ function App() { ); if (activeTransfers) { addToast({ - message: 'System action cancelled because another download is active.', + message: 'System action cancelled because another download is active or queued.', variant: 'warning', isActionable: true }); @@ -232,6 +230,12 @@ function App() { return clearPendingPostActionTimer; }, [clearPendingPostActionTimer]); + useEffect(() => { + if (activeTransferCount > 0) { + clearPendingPostActionTimer(); + } + }, [activeTransferCount, clearPendingPostActionTimer]); + useEffect(() => { initMediaDomains(); window.localStorage.setItem('firelink-sidebar-width', String(sidebarWidth)); @@ -393,13 +397,9 @@ function App() { invoke('toggle_tray_icon', { show: showMenuBarIcon }).catch(console.error); }, [showMenuBarIcon]); - useEffect(() => { - invoke('toggle_log_pause', { pause: !logsEnabled }).catch(console.error); - }, [logsEnabled]); - useEffect(() => { if (activeView !== 'logs') { - invoke('set_log_stream_active', { active: false }).catch(console.error); + setLogStreamActive(false).catch(console.error); } }, [activeView]); @@ -410,17 +410,6 @@ function App() { }); }, [extensionPairingToken]); - useEffect(() => { - if (previousSpeedLimit.current === globalSpeedLimit) return; - previousSpeedLimit.current = globalSpeedLimit; - - const formattedLimit = normalizeSpeedLimitForBackend(globalSpeedLimit); - - invoke('set_global_speed_limit', { limit: formattedLimit }).catch(error => { - console.error('Failed to apply global speed limit:', error); - }); - }, [globalSpeedLimit]); - useEffect(() => { if (!coreReady) return; const unlisten = listen('schedule-trigger', async (event) => { @@ -443,6 +432,7 @@ function App() { await invoke('ack_schedule_trigger', { action: 'start', key: payload.key }); return; } + const previouslyTrackedIds = new Set(state.schedulerActiveDownloadIds); const startedResults = await Promise.all( scheduledQueueIds.map(queueId => useDownloadStore.getState().startQueue(queueId)) ); @@ -450,6 +440,7 @@ function App() { const scheduledQueueSet = new Set(scheduledQueueIds); const trackedIds = useDownloadStore.getState().downloads .filter(download => + previouslyTrackedIds.has(download.id) && scheduledQueueSet.has(download.queueId || MAIN_QUEUE_ID) && isActiveDownloadStatus(download.status) ) @@ -459,9 +450,9 @@ function App() { state.setSchedulerRunning(activeIds.length > 0); await invoke('ack_schedule_trigger', { action: 'start', key: payload.key }); } else if (payload.action === 'stop') { - clearPendingPostActionTimer(); const trackedIds = state.schedulerActiveDownloadIds; if (trackedIds.length > 0) { + clearPendingPostActionTimer(); const pauseResults = await Promise.allSettled( trackedIds.map(id => useDownloadStore.getState().pauseDownload(id)) ); @@ -506,7 +497,15 @@ function App() { isActionable: true }); } else if (settings.scheduler.postQueueAction !== 'none') { - schedulePostQueueAction(settings.scheduler.postQueueAction); + if (downloads.some(download => isActiveDownloadStatus(download.status))) { + addToast({ + message: 'Scheduled system action skipped because another download is active or queued.', + variant: 'warning', + isActionable: true + }); + } else { + schedulePostQueueAction(settings.scheduler.postQueueAction); + } } }, [ addToast, diff --git a/src/components/LogsView.tsx b/src/components/LogsView.tsx index 4ff9c20..2862e5f 100644 --- a/src/components/LogsView.tsx +++ b/src/components/LogsView.tsx @@ -1,7 +1,7 @@ import { useEffect, useRef, useState } from 'react'; import { invokeCommand as invoke } from '../ipc'; import { save } from '@tauri-apps/plugin-dialog'; -import { attachLogger, setLogPaused, initLogger } from '../utils/logger'; +import { attachLogger, setLogPaused, initLogger, setLogStreamActive } from '../utils/logger'; import { FileDown, Trash2, Terminal, Filter, Play, Pause, Info, Copy } from 'lucide-react'; import { WindowDragRegion } from './WindowDragRegion'; import { useToast } from '../contexts/ToastContext'; @@ -27,6 +27,11 @@ export default function LogsView() { const scrollRef = useRef(null); const liveBatchRef = useRef([]); const liveFrameRef = useRef(null); + const clearGenerationRef = useRef(0); + const clearInFlightRef = useRef | null>(null); + const toggleInFlightRef = useRef | null>(null); + const [isClearing, setIsClearing] = useState(false); + const [isToggling, setIsToggling] = useState(false); useEffect(() => { const handleVisibilityChange = () => setPageVisible(document.visibilityState !== 'hidden'); @@ -36,16 +41,17 @@ export default function LogsView() { useEffect(() => { if (!pageVisible) { - void invoke('set_log_stream_active', { active: false }).catch(console.error); + void setLogStreamActive(false).catch(console.error); return; } if (!logsEnabled) { - void invoke('set_log_stream_active', { active: false }).catch(console.error); + void setLogStreamActive(false).catch(console.error); } let active = true; let initialized = false; + const initGeneration = clearGenerationRef.current; let pendingLiveEntries: LogEntry[] = []; let unlistenPromise: Promise<() => void> | undefined; @@ -78,9 +84,9 @@ export default function LogsView() { }); await unlistenPromise; if (!active) return; - await invoke('set_log_stream_active', { active: true }); + await setLogStreamActive(true); if (!active) { - await invoke('set_log_stream_active', { active: false }).catch(console.error); + await setLogStreamActive(false).catch(console.error); return; } } @@ -89,6 +95,10 @@ export default function LogsView() { if (!active) return; const snapshot = lines.map(persistedLogEntry); initialized = true; + if (initGeneration !== clearGenerationRef.current) { + pendingLiveEntries = []; + return; + } const caughtUpLogs = mergeLogSnapshotAndLiveEntries(snapshot, pendingLiveEntries); pendingLiveEntries = []; setLogs(caughtUpLogs); @@ -106,7 +116,7 @@ export default function LogsView() { liveFrameRef.current = null; } if (logsEnabled) { - void invoke('set_log_stream_active', { active: false }).catch(console.error); + void setLogStreamActive(false).catch(console.error); } if (unlistenPromise) { void unlistenPromise.then(unlisten => unlisten()).catch(console.error); @@ -170,23 +180,63 @@ export default function LogsView() { }; const handleClear = async () => { - liveBatchRef.current = []; - if (liveFrameRef.current !== null) { - window.cancelAnimationFrame(liveFrameRef.current); - liveFrameRef.current = null; + if (clearInFlightRef.current) return; + + const clearOperation = invoke('clear_logs'); + clearInFlightRef.current = clearOperation; + setIsClearing(true); + try { + await clearOperation; + clearGenerationRef.current += 1; + liveBatchRef.current = []; + if (liveFrameRef.current !== null) { + window.cancelAnimationFrame(liveFrameRef.current); + liveFrameRef.current = null; + } + setLogs([]); + addToast({ message: 'Logs cleared', variant: 'info' }); + } catch (error) { + addToast({ + message: `Could not clear logs: ${String(error)}`, + variant: 'error', + isActionable: true + }); + } finally { + if (clearInFlightRef.current === clearOperation) { + clearInFlightRef.current = null; + } + setIsClearing(false); } - setLogs([]); - await invoke('clear_logs').catch(console.error); }; const handleToggleLogging = async () => { + if (toggleInFlightRef.current) return; + const nextEnabled = !logsEnabled; - setLogsEnabled(nextEnabled); - await setLogPaused(!nextEnabled); - addToast({ - message: nextEnabled ? 'Diagnostic logging enabled' : 'Diagnostic logging disabled', - variant: 'success' - }); + const toggleOperation = (async () => { + await setLogPaused(!nextEnabled); + setLogsEnabled(nextEnabled); + addToast({ + message: nextEnabled ? 'Diagnostic logging enabled' : 'Diagnostic logging disabled', + variant: 'success' + }); + })(); + toggleInFlightRef.current = toggleOperation; + setIsToggling(true); + try { + await toggleOperation; + } catch (error) { + addToast({ + message: `Could not update diagnostic logging: ${String(error)}`, + variant: 'error', + isActionable: true + }); + } finally { + if (toggleInFlightRef.current === toggleOperation) { + toggleInFlightRef.current = null; + } + setIsToggling(false); + } }; const severityClass = (level: string) => { @@ -233,14 +283,16 @@ export default function LogsView() {
@@ -144,7 +162,7 @@ export default function SpeedLimiterView() { Global Speed Limit

- Applies to new and active aria2 transfers and yt-dlp media downloads. Per-download limits still take precedence. + Applies to new and active aria2 transfers and new yt-dlp media downloads. Explicit per-download limits remain attached to those transfers.

@@ -152,7 +170,7 @@ export default function SpeedLimiterView() { type="number" min="1" value={value} - disabled={!enabled} + disabled={!enabled || isSaving} onChange={event => setValue(Math.max(1, Number(event.target.value) || 1))} className="app-control w-28 px-3 py-2 text-right font-mono" /> @@ -161,7 +179,7 @@ export default function SpeedLimiterView() {