From ec3bd05547059ff3ffb728861fa4daa789fbe503 Mon Sep 17 00:00:00 2001 From: NimBold Date: Thu, 3 Sep 2026 20:54:32 +0330 Subject: [PATCH] fix(app): enforce media and scheduler lifecycle boundaries - keep magnet probe cleanup and resolver fallback inside absolute deadlines - route provider media and playlists only through HTTP(S) - enforce scheduler ID, time-format, and running-state invariants across persistence - isolate Companion release tag checks from ambient Git configuration --- scripts/verify-companion-release.js | 2 +- scripts/verify-companion-release.node-test.js | 9 ++++ src-tauri/src/lib.rs | 8 +--- src-tauri/src/parity.rs | 14 +++++- src-tauri/src/scheduler.rs | 19 ++++++-- src-tauri/src/settings.rs | 42 ++++++++++++++++- src-tauri/src/torrent_probe.rs | 46 ++++++++++++++++--- src/store/useSettingsStore.test.ts | 43 +++++++++++++++++ src/store/useSettingsStore.ts | 32 +++++++++---- src/utils/addDownloadMetadata.test.ts | 2 + src/utils/addDownloadMetadata.ts | 1 + src/utils/downloads.test.ts | 9 ++++ src/utils/downloads.ts | 1 + 13 files changed, 202 insertions(+), 26 deletions(-) diff --git a/scripts/verify-companion-release.js b/scripts/verify-companion-release.js index 2955dad..a8279b9 100644 --- a/scripts/verify-companion-release.js +++ b/scripts/verify-companion-release.js @@ -22,7 +22,7 @@ export function exactVersionTag(extensionRoot, expectedTag) { stdio: ['ignore', 'pipe', 'ignore'], env: { ...process.env, - GIT_CONFIG_GLOBAL: process.env.GIT_CONFIG_GLOBAL || (process.platform === 'win32' ? 'NUL' : '/dev/null'), + GIT_CONFIG_GLOBAL: process.platform === 'win32' ? 'NUL' : '/dev/null', GIT_CONFIG_NOSYSTEM: '1', }, } diff --git a/scripts/verify-companion-release.node-test.js b/scripts/verify-companion-release.node-test.js index 8fc62e7..ec5c95c 100644 --- a/scripts/verify-companion-release.node-test.js +++ b/scripts/verify-companion-release.node-test.js @@ -120,6 +120,7 @@ test('rejects a Companion tag for another version', () => { test('exactVersionTag resolves tag on HEAD with isolated git environment', () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'firelink-git-test-')); + const previousGlobalConfig = process.env.GIT_CONFIG_GLOBAL; try { const gitEnv = { ...process.env, @@ -134,9 +135,17 @@ test('exactVersionTag resolves tag on HEAD with isolated git environment', () => execFileSync('git', ['-C', root, 'commit', '--allow-empty', '-m', 'test'], { env: gitEnv, stdio: 'ignore' }); execFileSync('git', ['-C', root, 'tag', 'v2.0.7'], { env: gitEnv, stdio: 'ignore' }); + const globalConfig = path.join(root, 'global.gitconfig'); + fs.writeFileSync(globalConfig, '[alias]\n\ttag = !printf "v2.0.8\\n"\n'); + process.env.GIT_CONFIG_GLOBAL = globalConfig; assert.equal(exactVersionTag(root, 'v2.0.7'), 'v2.0.7'); assert.equal(exactVersionTag(root, 'v2.0.8'), null); } finally { + if (previousGlobalConfig === undefined) { + delete process.env.GIT_CONFIG_GLOBAL; + } else { + process.env.GIT_CONFIG_GLOBAL = previousGlobalConfig; + } fs.rmSync(root, { recursive: true, force: true }); } }); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 1a600cb..9782f44 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -9368,9 +9368,7 @@ async fn resolve_magnet_metadata( .err() .is_some_and(crate::torrent_probe::allows_resolver_fallback) => { - let cleanup_budget = operation_deadline - .saturating_duration_since(Instant::now()) - .max(MAGNET_PROBE_CLEANUP_RESERVE); + let cleanup_budget = operation_deadline.saturating_duration_since(Instant::now()); match tokio::time::timeout(cleanup_budget, remove_magnet_metadata_probe_dir(&probe_dir)) .await { @@ -9387,9 +9385,7 @@ async fn resolve_magnet_metadata( ); } } - let create_budget = operation_deadline - .saturating_duration_since(Instant::now()) - .max(MAGNET_PROBE_CLEANUP_RESERVE); + let create_budget = operation_deadline.saturating_duration_since(Instant::now()); match tokio::time::timeout(create_budget, tokio::fs::create_dir_all(&probe_dir)).await { Ok(Ok(())) => {} Ok(Err(error)) => { diff --git a/src-tauri/src/parity.rs b/src-tauri/src/parity.rs index ffc6a53..4c29d3f 100644 --- a/src-tauri/src/parity.rs +++ b/src-tauri/src/parity.rs @@ -680,6 +680,9 @@ pub fn get_supported_media_domains() -> Vec { #[tauri::command] pub fn is_supported_media(url: String) -> bool { if let Ok(parsed_url) = reqwest::Url::parse(&url) { + if !matches!(parsed_url.scheme(), "http" | "https") { + return false; + } if let Some(host) = parsed_url.host_str() { let host_lower = host.to_lowercase(); for domain in SUPPORTED_DOMAINS.iter() { @@ -694,7 +697,7 @@ pub fn is_supported_media(url: String) -> bool { #[cfg(test)] mod tests { - use super::get_file_category; + use super::{get_file_category, is_supported_media}; use crate::ipc::DownloadCategory; #[test] @@ -708,4 +711,13 @@ mod tests { DownloadCategory::Movies )); } + + #[test] + fn only_http_urls_are_supported_media_routes() { + assert!(is_supported_media( + "https://youtube.com/watch?v=video".to_string() + )); + assert!(!is_supported_media("ftp://youtube.com/video".to_string())); + assert!(!is_supported_media("sftp://youtube.com/video".to_string())); + } } diff --git a/src-tauri/src/scheduler.rs b/src-tauri/src/scheduler.rs index 871b902..07be3dd 100644 --- a/src-tauri/src/scheduler.rs +++ b/src-tauri/src/scheduler.rs @@ -5,9 +5,18 @@ use std::time::Duration; use tauri::Emitter; fn minute_of_day(value: &str) -> Option { - let (hour, minute) = value.split_once(':')?; - let hour = hour.parse::().ok()?; - let minute = minute.parse::().ok()?; + let bytes = value.as_bytes(); + if bytes.len() != 5 + || bytes[2] != b':' + || !bytes[0].is_ascii_digit() + || !bytes[1].is_ascii_digit() + || !bytes[3].is_ascii_digit() + || !bytes[4].is_ascii_digit() + { + return None; + } + let hour = u32::from(bytes[0] - b'0') * 10 + u32::from(bytes[1] - b'0'); + let minute = u32::from(bytes[3] - b'0') * 10 + u32::from(bytes[4] - b'0'); (hour < 24 && minute < 60).then_some(hour * 60 + minute) } @@ -254,6 +263,10 @@ mod tests { fn rejects_invalid_scheduler_times() { assert_eq!(minute_of_day("24:00"), None); assert_eq!(minute_of_day("12:60"), None); + assert_eq!(minute_of_day("1:02"), None); + assert_eq!(minute_of_day("01:2"), None); + assert_eq!(minute_of_day(" 01:02"), None); + assert_eq!(minute_of_day("01:02 "), None); assert_eq!(minute_of_day("bad"), None); } diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index acdf15f..8ce257a 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -545,9 +545,16 @@ fn sanitize_persisted_setting_values(state: &mut Value) { if !active_ids.is_array() { state.remove("schedulerActiveDownloadIds"); } else if let Some(ids_arr) = state.get_mut("schedulerActiveDownloadIds").and_then(Value::as_array_mut) { - ids_arr.retain(|v| v.as_str().is_some()); + ids_arr.retain(|v| v.as_str().is_some_and(|id| !id.trim().is_empty())); } } + if !state + .get("schedulerActiveDownloadIds") + .and_then(Value::as_array) + .is_some_and(|ids| !ids.is_empty()) + { + state.insert("schedulerRunning".to_string(), Value::Bool(false)); + } if let Some(overrides) = state.get("categoryDirectoryOverrides") { if !overrides.is_object() { state.remove("categoryDirectoryOverrides"); @@ -1533,6 +1540,39 @@ mod tests { assert!(settings.scheduler_active_download_ids.is_empty()); } + #[test] + fn filters_empty_scheduler_active_download_ids() { + let stored = json!({ + "state": { + "schedulerRunning": true, + "schedulerActiveDownloadIds": ["", " ", "download-1", 42] + } + }); + + let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap(); + + assert_eq!( + settings.scheduler_active_download_ids, + vec!["download-1".to_string()] + ); + assert!(settings.scheduler_running); + } + + #[test] + fn does_not_restore_a_running_scheduler_without_active_download_ids() { + let stored = json!({ + "state": { + "schedulerRunning": true, + "schedulerActiveDownloadIds": ["", " ", 42] + } + }); + + let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap(); + + assert!(!settings.scheduler_running); + assert!(settings.scheduler_active_download_ids.is_empty()); + } + #[test] fn preserves_valid_torrent_network_settings() { let stored = json!({ diff --git a/src-tauri/src/torrent_probe.rs b/src-tauri/src/torrent_probe.rs index f52010a..dca80e9 100644 --- a/src-tauri/src/torrent_probe.rs +++ b/src-tauri/src/torrent_probe.rs @@ -250,9 +250,7 @@ pub(crate) async fn run_metadata_probe_with_deadlines( } .await; - let cleanup_budget = cleanup_deadline - .saturating_duration_since(Instant::now()) - .max(Duration::from_secs(5)); + let cleanup_budget = cleanup_deadline.saturating_duration_since(Instant::now()); let cleanup_result = match tokio::time::timeout( cleanup_budget, cleanup_metadata_probe(client.as_ref(), &gid), @@ -500,10 +498,8 @@ impl ProbeCleanupGuard { } let mut first_error = None; - let cleanup_budget = deadline - .saturating_duration_since(Instant::now()) - .max(Duration::from_secs(5)); for gid in self.gids.clone() { + let cleanup_budget = deadline.saturating_duration_since(Instant::now()); match tokio::time::timeout( cleanup_budget, cleanup_metadata_probe(self.client.as_ref(), &gid), @@ -1601,6 +1597,44 @@ mod tests { server.shutdown().await; } + #[tokio::test(flavor = "current_thread")] + async fn cleanup_never_extends_absolute_probe_deadline() { + let server = ScriptedRpcServer::start(scripts([ + ("aria2.addUri", vec![ScriptedReply::Result(json!("gid-1"))]), + ("aria2.tellStatus", vec![ScriptedReply::Hang]), + ("aria2.forceRemove", vec![ScriptedReply::Hang]), + ])) + .await; + let (_temporary, probe_dir, metadata_path) = probe_fixture().await; + let error = tokio::time::timeout( + Duration::from_millis(500), + run_bounded_metadata_probe( + server.client(), + "magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567", + Map::new(), + &metadata_path, + "automatic", + MetadataProbeSchedule { + total_timeout: Duration::from_millis(100), + metadata_timeout: Duration::from_millis(20), + cleanup_reserve: Duration::from_millis(80), + poll_interval: Duration::ZERO, + }, + ), + ) + .await + .expect("cleanup must not extend the absolute probe deadline") + .expect_err("a cleanup timeout should be reported"); + assert!(matches!( + error, + ProbeFailure::Cleanup(message) if message.contains("failed to remove aria2 gid") + )); + server.terminate().await; + tokio::fs::remove_dir_all(&probe_dir) + .await + .expect("the probe fixture should be removable after the server stops"); + } + #[tokio::test(flavor = "current_thread")] async fn generic_metadata_timeout_does_not_enter_blocking_system_resolver() { // Use a deterministic pending RPC rather than a loopback HTTP server. diff --git a/src/store/useSettingsStore.test.ts b/src/store/useSettingsStore.test.ts index 2cef3f9..731a8c1 100644 --- a/src/store/useSettingsStore.test.ts +++ b/src/store/useSettingsStore.test.ts @@ -132,6 +132,49 @@ describe('durable main-window and sidebar preferences', () => { expect(fallbackResult?.scheduler.selectedQueueIds).toEqual(current.scheduler.selectedQueueIds); }); + it('filters empty scheduler active download IDs during hydration', () => { + const merge = useSettingsStore.persist.getOptions().merge; + expect(merge).toBeTypeOf('function'); + const current = useSettingsStore.getState(); + + const result = merge?.({ + schedulerActiveDownloadIds: ['', ' ', 'download-1', 42] as any + }, current); + + expect(result?.schedulerActiveDownloadIds).toEqual(['download-1']); + }); + + it('does not restore a running scheduler without active download IDs', () => { + const merge = useSettingsStore.persist.getOptions().merge; + expect(merge).toBeTypeOf('function'); + const current = useSettingsStore.getState(); + + const result = merge?.({ + schedulerRunning: true, + schedulerActiveDownloadIds: ['', ' ', 42] as any + }, current); + + expect(result?.schedulerRunning).toBe(false); + expect(result?.schedulerActiveDownloadIds).toEqual([]); + }); + + it('does not persist a running scheduler without active download IDs', () => { + const partialize = useSettingsStore.persist.getOptions().partialize; + expect(partialize).toBeTypeOf('function'); + const current = useSettingsStore.getState(); + + const snapshot = partialize?.({ + ...current, + schedulerRunning: true, + schedulerActiveDownloadIds: [] + }); + + expect(snapshot).toMatchObject({ + schedulerRunning: false, + schedulerActiveDownloadIds: [] + }); + }); + it('sanitizes setter calls for enum and numeric settings', () => { useSettingsStore.getState().setMediaCookieSource('invalid-browser' as any); expect(useSettingsStore.getState().mediaCookieSource).toBe('none'); diff --git a/src/store/useSettingsStore.ts b/src/store/useSettingsStore.ts index 1214dc7..cbcd86b 100644 --- a/src/store/useSettingsStore.ts +++ b/src/store/useSettingsStore.ts @@ -166,6 +166,13 @@ const persistedBoolean = (value: unknown, fallback: boolean) => const persistedString = (value: unknown, fallback: string): string => typeof value === 'string' ? value : fallback; +const sanitizeSchedulerActiveDownloadIds = ( + value: unknown, + fallback: string[] +): string[] => Array.isArray(value) + ? value.filter((id): id is string => typeof id === 'string' && id.trim().length > 0) + : fallback; + const persistedFiniteInteger = ( value: unknown, minimum: number, @@ -865,7 +872,12 @@ export const useSettingsStore = create()( shouldPersistLegacyFoldersFallback = false; state.setFoldersCollapsed(state.isFoldersCollapsed); }, - partialize: (state): PersistedSettingsSnapshot => ({ + partialize: (state): PersistedSettingsSnapshot => { + const schedulerActiveDownloadIds = sanitizeSchedulerActiveDownloadIds( + state.schedulerActiveDownloadIds, + [] + ); + return ({ theme: state.theme, fontFamily: state.fontFamily, windowControlStyle: state.windowControlStyle, @@ -888,8 +900,8 @@ export const useSettingsStore = create()( sidebarPosition: state.sidebarPosition, activeSettingsTab: state.activeSettingsTab, scheduler: state.scheduler, - schedulerRunning: state.schedulerRunning, - schedulerActiveDownloadIds: state.schedulerActiveDownloadIds, + schedulerRunning: state.schedulerRunning && schedulerActiveDownloadIds.length > 0, + schedulerActiveDownloadIds, schedulerLastStartKey: state.schedulerLastStartKey, schedulerLastStopKey: state.schedulerLastStopKey, lastCustomSpeedLimitKiB: state.lastCustomSpeedLimitKiB, @@ -940,7 +952,8 @@ export const useSettingsStore = create()( keychainAccessVersion: state.keychainAccessVersion, keychainPromptDismissed: state.keychainPromptDismissed, autoCheckUpdates: state.autoCheckUpdates - }), + }); + }, merge: (persistedState: unknown, currentState) => { const persisted = persistedState && typeof persistedState === 'object' ? persistedState as Partial @@ -953,6 +966,10 @@ export const useSettingsStore = create()( const foldersCollapsedFallback = legacyFoldersCollapsed ?? currentState.isFoldersCollapsed; const locations = normalizeDownloadLocationSettings(persisted); + const schedulerActiveDownloadIds = sanitizeSchedulerActiveDownloadIds( + persisted.schedulerActiveDownloadIds, + currentState.schedulerActiveDownloadIds + ); return ({ ...currentState, ...persisted, @@ -1180,10 +1197,9 @@ export const useSettingsStore = create()( ? persisted.scheduler.postQueueAction : currentState.scheduler.postQueueAction }, - schedulerRunning: persistedBoolean(persisted.schedulerRunning, currentState.schedulerRunning), - schedulerActiveDownloadIds: Array.isArray(persisted.schedulerActiveDownloadIds) - ? persisted.schedulerActiveDownloadIds.filter((id): id is string => typeof id === 'string') - : currentState.schedulerActiveDownloadIds, + schedulerRunning: persistedBoolean(persisted.schedulerRunning, currentState.schedulerRunning) + && schedulerActiveDownloadIds.length > 0, + schedulerActiveDownloadIds, schedulerLastStartKey: persistedString(persisted.schedulerLastStartKey, currentState.schedulerLastStartKey), schedulerLastStopKey: persistedString(persisted.schedulerLastStopKey, currentState.schedulerLastStopKey), siteLogins: Array.isArray(persisted.siteLogins) diff --git a/src/utils/addDownloadMetadata.test.ts b/src/utils/addDownloadMetadata.test.ts index 8f41654..aaaf1ea 100644 --- a/src/utils/addDownloadMetadata.test.ts +++ b/src/utils/addDownloadMetadata.test.ts @@ -87,6 +87,8 @@ describe('add download metadata workflow', () => { expect(isYouTubePlaylistUrl('https://music.youtube.com/playlist?list=PL123')).toBe(true); expect(isYouTubePlaylistUrl('https://www.youtube.com/watch?v=video&list=PL123')).toBe(false); expect(isYouTubePlaylistUrl('https://example.com/playlist?list=PL123')).toBe(false); + expect(isYouTubePlaylistUrl('ftp://youtube.com/playlist?list=PL123')).toBe(false); + expect(isYouTubePlaylistUrl('sftp://youtube.com/playlist?list=PL123')).toBe(false); }); it('admits magnets and local torrent files through the Add window metadata path', () => { diff --git a/src/utils/addDownloadMetadata.ts b/src/utils/addDownloadMetadata.ts index 9dda565..6f87bac 100644 --- a/src/utils/addDownloadMetadata.ts +++ b/src/utils/addDownloadMetadata.ts @@ -153,6 +153,7 @@ type ParsedInput = { export const isYouTubePlaylistUrl = (rawUrl: string): boolean => { try { const url = new URL(rawUrl); + if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; const hostname = url.hostname.toLowerCase(); const isYouTube = hostname === 'youtube.com' || hostname.endsWith('.youtube.com'); const pathname = url.pathname.replace(/\/+$/, '') || '/'; diff --git a/src/utils/downloads.test.ts b/src/utils/downloads.test.ts index 9ba961e..abf69be 100644 --- a/src/utils/downloads.test.ts +++ b/src/utils/downloads.test.ts @@ -9,6 +9,7 @@ import { canonicalizeDownloadFileName, categoryForDownload, categoryForFileName, + isMediaUrl, isAllocationPhaseVisible, isAllocationPhaseEligible, isValidTorrentExcludeTrackerList, @@ -49,6 +50,14 @@ describe('download category detection', () => { expect(categoryForDownload('Renamed', true, 'Other')).toBe('Other'); expect(categoryForDownload('Renamed', true, 'Torrents')).toBe('Torrents'); }); + + it('only classifies HTTP(S) provider URLs as media', () => { + expect(isMediaUrl('https://www.youtube.com/watch?v=video')).toBe(true); + expect(isMediaUrl('http://youtu.be/video')).toBe(true); + expect(isMediaUrl('ftp://youtube.com/video.mp4')).toBe(false); + expect(isMediaUrl('sftp://youtube.com/video.mp4')).toBe(false); + expect(isMediaUrl('magnet://youtube.com/video')).toBe(false); + }); }); describe('download names from URLs', () => { diff --git a/src/utils/downloads.ts b/src/utils/downloads.ts index 809448d..c48c88b 100644 --- a/src/utils/downloads.ts +++ b/src/utils/downloads.ts @@ -595,6 +595,7 @@ export const downloadFileNamesMatch = (left: string, right: string): boolean => export const isMediaUrl = (rawUrl: string): boolean => { try { const url = new URL(rawUrl); + if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; return MEDIA_DOMAINS.some(domain => url.hostname === domain || url.hostname.endsWith(`.${domain}`) );