From f4b830414dba8876ec11c66c8eb926373acfd2cc Mon Sep 17 00:00:00 2001 From: NimBold Date: Fri, 17 Jul 2026 16:27:39 +0330 Subject: [PATCH] fix: harden audited persistence and download paths --- scripts/verify-binaries.js | 24 ++- src-tauri/src/db.rs | 281 ++++++++++++++++++++++++++++- src-tauri/src/lib.rs | 22 +++ src/components/SettingsView.tsx | 53 ++++-- src/ipc.ts | 5 + src/store/downloadStore.test.ts | 32 ++++ src/store/downloadStore.ts | 12 +- src/store/useDownloadStore.test.ts | 77 ++++++++ src/store/useDownloadStore.ts | 76 +++++++- src/store/useSettingsStore.test.ts | 24 ++- src/store/useSettingsStore.ts | 25 ++- 11 files changed, 586 insertions(+), 45 deletions(-) diff --git a/scripts/verify-binaries.js b/scripts/verify-binaries.js index 200b4ce..2f26ac6 100644 --- a/scripts/verify-binaries.js +++ b/scripts/verify-binaries.js @@ -45,16 +45,34 @@ const searchRoot = argValue('--search-root'); function findEngineRoot(root) { const expected = `yt-dlp-${targetTriple}${ext}`; const matches = []; + const resolvedRoot = path.resolve(root); + const hasExpectedEngineFile = directory => { + try { + return fs.lstatSync(path.join(directory, expected)).isFile(); + } catch (error) { + if (error?.code === 'ENOENT') return false; + throw new Error(`Unable to inspect packaged engine root '${directory}': ${error.message}`); + } + }; + if (hasExpectedEngineFile(resolvedRoot)) { + matches.push(resolvedRoot); + } const walk = directory => { - for (const entry of fs.readdirSync(directory, { withFileTypes: true })) { + let entries; + try { + entries = fs.readdirSync(directory, { withFileTypes: true }); + } catch (error) { + throw new Error(`Unable to inspect packaged engine directory '${directory}': ${error.message}`); + } + for (const entry of entries) { const candidate = path.join(directory, entry.name); if (entry.isDirectory()) { - if (fs.existsSync(path.join(candidate, expected))) matches.push(candidate); + if (hasExpectedEngineFile(candidate)) matches.push(candidate); walk(candidate); } } }; - walk(path.resolve(root)); + walk(resolvedRoot); if (matches.length !== 1) { throw new Error(`Expected exactly one packaged engine root under ${root}, found ${matches.length}`); } diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 248858b..4a4c42b 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -707,6 +707,188 @@ pub fn save_settings(connection: &Connection, data: &str) -> Result<(), String> Ok(()) } +fn settings_state_mut( + document: &mut Value, +) -> Result<&mut serde_json::Map, String> { + if document.get("state").is_some() { + document + .get_mut("state") + .and_then(Value::as_object_mut) + .ok_or_else(|| "persisted settings state must be an object".to_string()) + } else { + document + .as_object_mut() + .ok_or_else(|| "persisted settings must be an object".to_string()) + } +} + +fn add_site_login_to_settings( + data: &str, + id: &str, + url_pattern: &str, + username: &str, +) -> Result { + let mut document: Value = serde_json::from_str(data) + .map_err(|error| format!("failed to decode settings: {error}"))?; + let state = settings_state_mut(&mut document)?; + let logins = state + .entry("siteLogins") + .or_insert_with(|| Value::Array(Vec::new())); + let logins = logins + .as_array_mut() + .ok_or_else(|| "persisted site logins must be an array".to_string())?; + if logins.iter().any(|login| { + login + .get("id") + .and_then(Value::as_str) + .is_some_and(|existing_id| existing_id == id) + }) { + return Err("site login already exists".to_string()); + } + logins.push(serde_json::json!({ + "id": id, + "urlPattern": url_pattern, + "username": username, + })); + serde_json::to_string(&document) + .map_err(|error| format!("failed to encode settings: {error}")) +} + +fn remove_site_login_from_settings( + data: &str, + id: &str, +) -> Result<(String, bool), String> { + let mut document: Value = serde_json::from_str(data) + .map_err(|error| format!("failed to decode settings: {error}"))?; + let state = settings_state_mut(&mut document)?; + let Some(logins) = state.get_mut("siteLogins") else { + return Ok((data.to_string(), false)); + }; + let logins = logins + .as_array_mut() + .ok_or_else(|| "persisted site logins must be an array".to_string())?; + let original_len = logins.len(); + logins.retain(|login| { + login + .get("id") + .and_then(Value::as_str) + .is_none_or(|existing_id| existing_id != id) + }); + if logins.len() == original_len { + return Ok((data.to_string(), false)); + } + let updated = serde_json::to_string(&document) + .map_err(|error| format!("failed to encode settings: {error}"))?; + Ok((updated, true)) +} + +fn validate_site_login_id(id: &str) -> Result<&str, String> { + let trimmed = id.trim(); + if trimmed.is_empty() + || trimmed != id + || trimmed == PAIRING_TOKEN_KEYCHAIN_ID + { + return Err("invalid site login identifier".to_string()); + } + Ok(trimmed) +} + +fn validate_site_login_input( + id: &str, + url_pattern: &str, + username: &str, + password: &str, +) -> Result<(), String> { + validate_site_login_id(id)?; + if url_pattern.trim().is_empty() || url_pattern.chars().any(char::is_whitespace) { + return Err("site login URL pattern must be non-empty and contain no whitespace".to_string()); + } + if username.trim().is_empty() { + return Err("site login username must be non-empty".to_string()); + } + if password.is_empty() { + return Err("site login password must be non-empty".to_string()); + } + Ok(()) +} + +pub fn save_site_login( + connection: &Connection, + id: &str, + url_pattern: &str, + username: &str, + password: &str, +) -> Result<(), String> { + validate_site_login_input(id, url_pattern, username, password)?; + let id = validate_site_login_id(id)?; + let _keyring_guard = lock_keyring_operations()?; + let original = load_settings(connection)?.unwrap_or_else(|| { + // A first-run standard-mode install can grant keychain access before + // any frontend setting has been persisted. Start with a valid + // Zustand envelope so the first site login is not rejected. + serde_json::json!({ "state": {}, "version": 3 }).to_string() + }); + if get_keychain_password_if_present_unlocked(id)?.is_some() { + return Err("a credential already exists for this site login".to_string()); + } + let updated = add_site_login_to_settings(&original, id, url_pattern, username)?; + + // Persist metadata before creating the secret. If the credential-store + // write fails, restore the exact previous settings document so a failed + // add cannot leave an orphaned keychain entry or a visible login row. + save_settings(connection, &updated)?; + if let Err(error) = set_keychain_password_unlocked(id, password) { + let keychain_rollback = delete_keychain_password_unlocked(id); + let settings_rollback = save_settings(connection, &original); + if let Err(rollback_error) = keychain_rollback { + return Err(format!( + "failed to save site login credential: {error}; credential rollback also failed: {rollback_error}" + )); + } + if let Err(rollback_error) = settings_rollback { + return Err(format!( + "failed to save site login credential: {error}; settings rollback also failed: {rollback_error}" + )); + } + return Err(format!("failed to save site login credential: {error}")); + } + Ok(()) +} + +pub fn delete_site_login(connection: &Connection, id: &str) -> Result<(), String> { + let id = validate_site_login_id(id)?; + let _keyring_guard = lock_keyring_operations()?; + let Some(original) = load_settings(connection)? else { + return Err("settings are not persisted yet".to_string()); + }; + let (updated, removed) = remove_site_login_from_settings(&original, id)?; + if !removed { + return Err("site login was not found".to_string()); + } + let _existing_password = get_keychain_password_if_present_unlocked(id)?; + + // Remove the metadata first only after the keychain has been checked. If + // deleting the secret fails, restore the metadata so the UI cannot lose a + // credential that still exists. + save_settings(connection, &updated)?; + if let Err(error) = delete_keychain_password_unlocked(id) { + if matches!( + get_keychain_password_if_present_unlocked(id), + Ok(None) + ) { + return Ok(()); + } + if let Err(rollback_error) = save_settings(connection, &original) { + return Err(format!( + "failed to delete site login credential: {error}; settings rollback also failed: {rollback_error}" + )); + } + return Err(format!("failed to delete site login credential: {error}")); + } + + Ok(()) +} + fn save_settings_tx(transaction: &Transaction<'_>, data: &str) -> Result<(), String> { transaction .execute( @@ -1355,22 +1537,25 @@ pub fn set_keychain_password(id: &str, password: &str) -> Result<(), String> { set_keychain_password_unlocked(id, password) } -fn get_keychain_password_unlocked(id: &str) -> Result { +fn get_keychain_password_if_present_unlocked(id: &str) -> Result, String> { let entry = keychain_entry(id)?; match entry.get_password() { - Ok(password) => Ok(password), + Ok(password) => Ok(Some(password)), #[cfg(target_os = "linux")] Err(keyring_core::Error::NoEntry) => unique_legacy_linux_keychain_entry(id)? - .ok_or_else(|| keyring_core::Error::NoEntry.to_string())? - .get_password() - .map_err(|error| error.to_string()), - #[cfg(target_os = "linux")] - Err(error) => Err(error.to_string()), + .map(|legacy| legacy.get_password().map_err(|error| error.to_string())) + .transpose(), #[cfg(not(target_os = "linux"))] + Err(keyring_core::Error::NoEntry) => Ok(None), Err(error) => Err(error.to_string()), } } +fn get_keychain_password_unlocked(id: &str) -> Result { + get_keychain_password_if_present_unlocked(id)? + .ok_or_else(|| keyring_core::Error::NoEntry.to_string()) +} + pub fn get_keychain_password(id: &str) -> Result { let _guard = lock_keyring_operations()?; get_keychain_password_unlocked(id) @@ -1405,6 +1590,88 @@ mod tests { use serde_json::json; use tempfile::TempDir; + #[test] + fn site_login_settings_update_preserves_envelope_without_password() { + let original = json!({ + "state": { + "theme": "dark", + "siteLogins": [] + }, + "version": 3 + }) + .to_string(); + + let updated = add_site_login_to_settings( + &original, + "login-1", + "https://example.com/*", + "nima", + ) + .unwrap(); + let document: Value = serde_json::from_str(&updated).unwrap(); + assert_eq!(document["state"]["theme"], "dark"); + assert_eq!(document["version"], 3); + assert_eq!(document["state"]["siteLogins"][0]["id"], "login-1"); + assert_eq!(document["state"]["siteLogins"][0]["username"], "nima"); + assert!(!updated.contains("password")); + } + + #[test] + fn site_login_settings_update_rejects_duplicate_ids() { + let original = json!({ + "state": { + "siteLogins": [{ + "id": "login-1", + "urlPattern": "https://example.com/*", + "username": "old-user" + }] + }, + "version": 3 + }) + .to_string(); + + let error = add_site_login_to_settings( + &original, + "login-1", + "https://example.com/*", + "new-user", + ) + .unwrap_err(); + assert_eq!(error, "site login already exists"); + } + + #[test] + fn site_login_settings_removal_reports_whether_metadata_changed() { + let original = json!({ + "state": { + "siteLogins": [{ + "id": "login-1", + "urlPattern": "https://example.com/*", + "username": "nima" + }] + }, + "version": 3 + }) + .to_string(); + + let (updated, removed) = remove_site_login_from_settings(&original, "login-1").unwrap(); + assert!(removed); + let document: Value = serde_json::from_str(&updated).unwrap(); + assert!(document["state"]["siteLogins"].as_array().unwrap().is_empty()); + + let (unchanged, removed) = remove_site_login_from_settings(&updated, "missing").unwrap(); + assert!(!removed); + assert_eq!(unchanged, updated); + } + + #[test] + fn site_login_ids_cannot_alias_the_pairing_token_or_hide_whitespace() { + assert_eq!(validate_site_login_id("login-1").unwrap(), "login-1"); + assert!(validate_site_login_id(" extension-pairing-token").is_err()); + assert!(validate_site_login_id("extension-pairing-token ").is_err()); + assert!(validate_site_login_id(" login-1").is_err()); + } + #[test] fn migrates_v0_database_and_creates_backup() { let temp = TempDir::new().unwrap(); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index f5dda7e..179fa89 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -5536,6 +5536,27 @@ fn delete_keychain_password( crate::db::delete_keychain_password(&id) } +#[tauri::command] +fn save_site_login( + database: tauri::State<'_, crate::db::DbState>, + id: String, + url_pattern: String, + username: String, + password: String, +) -> Result<(), String> { + let connection = database.lock()?; + crate::db::save_site_login(&connection, &id, &url_pattern, &username, &password) +} + +#[tauri::command] +fn delete_site_login( + database: tauri::State<'_, crate::db::DbState>, + id: String, +) -> Result<(), String> { + let connection = database.lock()?; + crate::db::delete_site_login(&connection, &id) +} + #[derive(Serialize, TS)] #[serde(rename_all = "camelCase")] #[ts(export, export_to = "../../src/bindings/")] @@ -8615,6 +8636,7 @@ pub fn run() { ack_schedule_trigger, check_automation_permission, request_automation_permission, open_automation_settings, set_keychain_password, get_keychain_password, delete_keychain_password, + save_site_login, delete_site_login, hydrate_extension_pairing_token, get_session_pairing_token, regenerate_pairing_token, grant_keychain_access, acknowledge_pairing_token_change, check_file_exists, toggle_tray_icon, set_extension_pairing_token, diff --git a/src/components/SettingsView.tsx b/src/components/SettingsView.tsx index 6cefe44..ee9f61c 100644 --- a/src/components/SettingsView.tsx +++ b/src/components/SettingsView.tsx @@ -4,6 +4,7 @@ import { type ListRowDensity, type SettingsState, SettingsTab, + runSettingsPersistenceTransaction, useSettingsStore } from '../store/useSettingsStore'; import { @@ -288,6 +289,8 @@ const engineRunId = useRef(0); const [loginUser, setLoginUser] = useState(''); const [loginPass, setLoginPass] = useState(''); const [loginError, setLoginError] = useState(''); + const [isSavingLogin, setIsSavingLogin] = useState(false); + const saveLoginInFlight = useRef(false); const [loginFieldErrors, setLoginFieldErrors] = useState<{ pattern?: string; username?: string; @@ -546,6 +549,7 @@ runEngineChecks(false); }; const handleAddLogin = async () => { + if (saveLoginInFlight.current) return; const fieldErrors: typeof loginFieldErrors = {}; if (!loginPattern.trim()) { fieldErrors.pattern = 'URL pattern is required.'; @@ -571,22 +575,31 @@ runEngineChecks(false); setLoginError('Grant credential-store access before saving a site login.'); return; } - - if (loginPass) { - try { - await invoke('set_keychain_password', { id, password: loginPass }); - } catch (e) { - console.error("Failed to save password to keychain:", e); - setLoginError("Failed to save password securely."); - return; - } + saveLoginInFlight.current = true; + setIsSavingLogin(true); + try { + await runSettingsPersistenceTransaction(async () => { + await invoke('save_site_login', { + id, + urlPattern: loginPattern.trim(), + username: loginUser.trim(), + password: loginPass + }); + settings.addSiteLogin({ + id, + urlPattern: loginPattern.trim(), + username: loginUser.trim() + }); + }); + } catch (e) { + console.error("Failed to save site login:", e); + setLoginError("Failed to save site credential securely."); + return; + } finally { + saveLoginInFlight.current = false; + setIsSavingLogin(false); } - settings.addSiteLogin({ - id, - urlPattern: loginPattern.trim(), - username: loginUser.trim() - }); setLoginPattern(''); setLoginUser(''); setLoginPass(''); @@ -916,7 +929,7 @@ runEngineChecks(false); {systemProxyStatus === 'checking' && 'Checking system proxy configuration…'} {systemProxyStatus === 'detected' && 'A system proxy was detected. Normal file downloads require an HTTP or HTTPS endpoint; media downloads can use SOCKS.'} {systemProxyStatus === 'none' && 'No usable system proxy was detected. Downloads will use no proxy.'} - {systemProxyStatus === 'error' && 'System proxy configuration could not be read. Downloads will use no proxy until it is available.'} + {systemProxyStatus === 'error' && 'System proxy configuration could not be read. Choose No Proxy or try again when it is available.'}

)} @@ -1107,8 +1120,10 @@ runEngineChecks(false); return; } try { - await invoke('delete_keychain_password', { id: login.id }); - settings.removeSiteLogin(login.id); + await runSettingsPersistenceTransaction(async () => { + await invoke('delete_site_login', { id: login.id }); + settings.removeSiteLogin(login.id); + }); showToast("Deleted credential", 'success'); } catch (error) { showToast(`Could not delete credential: ${String(error)}`, 'error'); @@ -1182,10 +1197,12 @@ runEngineChecks(false);
diff --git a/src/ipc.ts b/src/ipc.ts index 9f2e9a2..9cec1ff 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -55,6 +55,11 @@ type CommandMap = { set_keychain_password: { args: { id: string; password: string }; result: void }; get_keychain_password: { args: { id: string }; result: string }; delete_keychain_password: { args: { id: string }; result: void }; + save_site_login: { + args: { id: string; urlPattern: string; username: string; password: string }; + result: void; + }; + delete_site_login: { args: { id: string }; result: void }; check_file_exists: { args: { path: string }; result: boolean }; toggle_tray_icon: { args: { show: boolean }; result: void }; set_extension_pairing_token: { args: { token: string }; result: void }; diff --git a/src/store/downloadStore.test.ts b/src/store/downloadStore.test.ts index 1516605..c87b4f5 100644 --- a/src/store/downloadStore.test.ts +++ b/src/store/downloadStore.test.ts @@ -201,4 +201,36 @@ describe('useDownloadProgressStore', () => { expect(useDownloadProgressStore.getState().progressMap).toEqual({}); release(); }); + + it('ignores stale active state events after pause but accepts terminal reconciliation', async () => { + const handlers: Record void> = {}; + vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { + handlers[event] = handler as (event: any) => void; + return Promise.resolve(vi.fn()); + }); + useDownloadStore.setState({ + downloads: [{ + id: 'paused-race', + url: 'https://example.com/file', + fileName: 'file.bin', + status: 'paused', + category: 'Other', + dateAdded: '' + }] + }); + + const release = await initDownloadListener(); + handlers['download-state']({ payload: { + id: 'paused-race', + status: 'downloading' + } }); + expect(useDownloadStore.getState().downloads[0].status).toBe('paused'); + + handlers['download-state']({ payload: { + id: 'paused-race', + status: 'completed' + } }); + expect(useDownloadStore.getState().downloads[0].status).toBe('completed'); + release(); + }); }); diff --git a/src/store/downloadStore.ts b/src/store/downloadStore.ts index b99b1e8..1f1704b 100644 --- a/src/store/downloadStore.ts +++ b/src/store/downloadStore.ts @@ -78,11 +78,21 @@ const startDownloadListeners = async () => { } const status = payload.status as DownloadStatus; - // Prevent race condition: don't transition backwards from terminal state + // Prevent stale lifecycle events from moving a paused row back into an + // active state. A pause request can finish before one already-emitted + // worker event reaches the frontend. Resume paths set the row to queued + // before asking the backend to resume, so an active event arriving while + // the row is still paused cannot represent a new lifecycle. if ((current.status === 'completed' || current.status === 'failed') && status !== current.status) { return; } + if (current.status === 'paused' && + status !== 'paused' && + status !== 'completed' && + status !== 'failed') { + return; + } const progress = useDownloadProgressStore.getState().progressMap[payload.id]; if (['queued', 'retrying', 'completed', 'failed', 'paused'].includes(status)) { diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index 5dbacd1..09084eb 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -217,6 +217,15 @@ describe('useDownloadStore', () => { proxyPort: 8080 } as ReturnType)).toBe('none'); + vi.mocked(ipc.invokeCommand).mockRejectedValueOnce(new Error('system settings unavailable')); + await expect(getProxyArgs({ + proxyMode: 'system', + proxyHost: '', + proxyPort: 8080 + } as ReturnType)).rejects.toThrow( + 'System proxy configuration could not be read: system settings unavailable' + ); + expect(await getProxyArgs({ proxyMode: 'custom', proxyHost: 'http://127.0.0.1', @@ -224,6 +233,39 @@ describe('useDownloadStore', () => { } as ReturnType)).toBe('http://127.0.0.1:1080'); }); + it('keeps an item queued when system proxy resolution fails closed', async () => { + vi.mocked(useSettingsStore.getState).mockReturnValue({ + ...useSettingsStore.getState(), + proxyMode: 'system' + } as unknown as ReturnType); + vi.mocked(ipc.invokeCommand).mockImplementation(async (command: string) => { + if (command === 'get_system_proxy') { + throw new Error('system settings unavailable'); + } + return undefined; + }); + useDownloadStore.setState({ + downloads: [{ + id: 'system-proxy-blocked', + url: 'https://example.com/file.bin', + fileName: 'file.bin', + destination: '/tmp', + status: 'queued', + category: 'Other', + dateAdded: '' + }] as any[], + backendRegisteredIds: new Set() + }); + + await expect(dispatchItem('system-proxy-blocked')).resolves.toBe(false); + + expect(useDownloadStore.getState().downloads[0]).toMatchObject({ + status: 'queued', + lastError: 'System proxy configuration could not be read: system settings unavailable. Choose No Proxy or try again.' + }); + expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_download', expect.anything()); + }); + it('matches site logins by host, wildcard host, path, and full URL patterns', () => { const settings = { siteLogins: [ @@ -774,6 +816,41 @@ describe('useDownloadStore', () => { }); }); + it('keeps all startup items retryable when system proxy resolution fails', async () => { + vi.mocked(useSettingsStore.getState).mockReturnValue({ + ...useSettingsStore.getState(), + proxyMode: 'system' + } as unknown as ReturnType); + vi.mocked(ipc.invokeCommand).mockImplementation(async (command: string) => { + if (command === 'db_get_all_queues') return []; + if (command === 'db_get_all_downloads') { + return [JSON.stringify({ + id: 'startup-proxy-blocked', + url: 'https://example.com/file.bin', + fileName: 'file.bin', + status: 'queued', + category: 'Other', + dateAdded: '', + queueId: '00000000-0000-0000-0000-000000000001', + hasBeenDispatched: true + })]; + } + if (command === 'get_system_proxy') { + throw new Error('system settings unavailable'); + } + return undefined; + }); + + await useDownloadStore.getState().initDB(); + await useDownloadStore.getState().resumePendingDownloads(); + + expect(useDownloadStore.getState().downloads[0]).toMatchObject({ + status: 'queued', + lastError: 'System proxy configuration could not be read: system settings unavailable. Choose No Proxy or try again.' + }); + expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_many', expect.anything()); + }); + it('keeps accepted startup registrations when pending-order refresh fails', async () => { vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { if (cmd === 'db_get_all_queues') return []; diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 9e0c698..4c0e87e 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -125,6 +125,16 @@ const removeStaleBackendDispatch = async (id: string): Promise => { const errorMessage = (error: unknown): string => error instanceof Error ? error.message : String(error); +export class SystemProxyResolutionError extends Error { + constructor(reason: string) { + super(`System proxy configuration could not be read: ${reason}. Choose No Proxy or try again.`); + this.name = 'SystemProxyResolutionError'; + } +} + +const isSystemProxyConfigurationError = (error: unknown): boolean => + error instanceof SystemProxyResolutionError; + const stripCookieHeaders = (value: string | null | undefined): string => (value || '') .split(/\r?\n/) @@ -159,7 +169,7 @@ const speedLimitForDispatch = ( return normalizeSpeedLimitForBackend(globalSpeedLimit); }; -export async function dispatchItem(id: string): Promise { +export async function dispatchItem(id: string, proxyOverride?: string | null): Promise { await waitForPendingStartupResume(); if (backendDispatchPromises.has(id)) return backendDispatchPromises.get(id)!; @@ -193,7 +203,9 @@ export async function dispatchItem(id: string): Promise { } if (!isCurrentDownloadLifecycle(id, lifecycleGeneration)) return false; - const proxy = await getProxyArgs(settings); + const proxy = proxyOverride === undefined + ? await getProxyArgs(settings) + : proxyOverride; if (!isCurrentDownloadLifecycle(id, lifecycleGeneration)) return false; const enqueueItem = { @@ -252,8 +264,9 @@ export async function dispatchItem(id: string): Promise { await removeStaleBackendDispatch(id); } if (lifecycleGeneration !== null && isCurrentDownloadLifecycle(id, lifecycleGeneration)) { + const proxyBlocked = isSystemProxyConfigurationError(e); useDownloadStore.getState().updateDownload(id, { - status: 'failed', + status: proxyBlocked ? 'queued' : 'failed', lastError: errorMessage(e) }); } @@ -310,8 +323,8 @@ export const getProxyArgs = async (settings: ReturnType((set, get) => ({ if (runnable.length === 0 || !isCurrentQueueControlGeneration(queueId, requestedGeneration)) return []; + const needsNewDispatch = runnable.some(item => { + const currentItem = get().downloads.find(download => download.id === item.id); + if (!currentItem) return false; + const backendRegistered = get().backendRegisteredIds.has(item.id); + const backendPending = get().pendingOrder.includes(item.id); + if (currentItem.status === 'queued' && backendRegistered && !backendPending) { + return false; + } + return currentItem.status === 'ready' || + currentItem.status === 'staged' || + currentItem.status === 'failed' || + !currentItem.hasBeenDispatched || + !backendRegistered; + }); + let queueProxy: string | null | undefined; + if (needsNewDispatch) { + try { + queueProxy = await getProxyArgs(useSettingsStore.getState()); + } catch (error) { + const message = errorMessage(error); + console.error(`Could not safely resolve the proxy for queue ${queueId}:`, error); + const runnableIds = new Set(runnable.map(item => item.id)); + set(state => ({ + downloads: state.downloads.map(item => + runnableIds.has(item.id) && item.status !== 'completed' + ? { ...item, lastError: message } + : item + ) + })); + return []; + } + } + const acceptedIds: string[] = []; for (const item of runnable) { if (!isCurrentQueueControlGeneration(queueId, requestedGeneration)) break; @@ -1031,7 +1077,7 @@ export const useDownloadStore = create((set, get) => ({ !currentItem.hasBeenDispatched || !backendRegistered ) { - if (await dispatchItem(item.id)) { + if (await dispatchItem(item.id, queueProxy)) { if (!isCurrentQueueControlGeneration(queueId, requestedGeneration)) { const afterDispatch = get().downloads.find(download => download.id === item.id); if ( @@ -1240,6 +1286,22 @@ export const useDownloadStore = create((set, get) => ({ try { const settings = useSettingsStore.getState(); + let proxy: string | null; + try { + proxy = await getProxyArgs(settings); + } catch (error) { + const message = errorMessage(error); + console.error('Could not safely resolve the system proxy during startup resume:', error); + const activeIds = new Set(active.map(item => item.id)); + set(state => ({ + downloads: state.downloads.map(item => + activeIds.has(item.id) && item.status === 'queued' + ? { ...item, lastError: message } + : item + ) + })); + return; + } const itemsToEnqueue = []; for (const pendingItem of active) { const item = get().downloads.find(download => download.id === pendingItem.id); @@ -1272,7 +1334,7 @@ export const useDownloadStore = create((set, get) => ({ mirrors: item.mirrors || null, user_agent: settings.customUserAgent.trim() || null, max_tries: settings.maxAutomaticRetries, - proxy: await getProxyArgs(settings), + proxy, format_selector: item.mediaFormatSelector || null, cookie_source: settings.mediaCookieSource !== 'none' ? settings.mediaCookieSource : null, is_media: item.isMedia || false, diff --git a/src/store/useSettingsStore.test.ts b/src/store/useSettingsStore.test.ts index 3e1607a..b5a8bc8 100644 --- a/src/store/useSettingsStore.test.ts +++ b/src/store/useSettingsStore.test.ts @@ -1,5 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { subscribeToSettingsPersistenceErrors, useSettingsStore } from './useSettingsStore'; +import { + runSettingsPersistenceTransaction, + subscribeToSettingsPersistenceErrors, + useSettingsStore +} from './useSettingsStore'; import * as ipc from '../ipc'; vi.mock('../ipc', () => ({ @@ -68,6 +72,24 @@ describe('useSettingsStore credential-store startup flow', () => { }); describe('useSettingsStore persistence failures', () => { + it('keeps settings writes queued behind a credential transaction', async () => { + const events: string[] = []; + vi.mocked(ipc.invokeCommand).mockImplementation(async command => { + if (command === 'db_save_settings') events.push('settings-write'); + return undefined; + }); + + await runSettingsPersistenceTransaction(async () => { + events.push('transaction-start'); + useSettingsStore.setState({ theme: 'dark' }); + events.push('transaction-end'); + }); + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(events.slice(0, 2)).toEqual(['transaction-start', 'transaction-end']); + expect(events).toContain('settings-write'); + }); + it('reports a database save failure and retries the next settings update', async () => { vi.clearAllMocks(); await new Promise(resolve => setTimeout(resolve, 0)); diff --git a/src/store/useSettingsStore.ts b/src/store/useSettingsStore.ts index 76deea3..f7c87c3 100644 --- a/src/store/useSettingsStore.ts +++ b/src/store/useSettingsStore.ts @@ -19,7 +19,7 @@ import { } from '../utils/downloadLocations'; import { normalizeSpeedLimitForBackend } from '../utils/downloads'; -let settingsSave = Promise.resolve(); +let settingsQueue: Promise = Promise.resolve(); const settingsPersistenceErrorListeners = new Set<() => void>(); let settingsPersistenceFailed = false; const DEFAULT_SCHEDULER_QUEUE_ID = '00000000-0000-0000-0000-000000000001'; @@ -31,6 +31,16 @@ export const subscribeToSettingsPersistenceErrors = (listener: () => void): (() return () => settingsPersistenceErrorListeners.delete(listener); }; +const enqueueSettingsTask = (task: () => Promise): Promise => { + const result = settingsQueue.then(task, task); + settingsQueue = result.then(() => undefined, () => undefined); + return result; +}; + +export const runSettingsPersistenceTransaction = ( + operation: () => Promise +): Promise => enqueueSettingsTask(operation); + const notifySettingsPersistenceError = () => { if (settingsPersistenceFailed) return; settingsPersistenceFailed = true; @@ -102,16 +112,15 @@ const tauriStorage: StateStorage = { }, setItem: async (name: string, value: string): Promise => { if (name === 'firelink-settings') { - settingsSave = settingsSave - .catch(() => undefined) - .then(() => invoke('db_save_settings', { data: value })) - .then(() => { + await enqueueSettingsTask(async () => { + try { + await invoke('db_save_settings', { data: value }); settingsPersistenceFailed = false; - }, () => { + } catch { console.error('Failed to save settings to DB'); notifySettingsPersistenceError(); - }); - await settingsSave; + } + }); } }, removeItem: async (_name: string): Promise => {