From dad5b7bc5e06a05f87ae298ad090bc1e817478c5 Mon Sep 17 00:00:00 2001 From: NimBold Date: Mon, 13 Jul 2026 00:09:09 +0330 Subject: [PATCH] fix(portable): harden persistence and release paths (#15) Preserve legacy source data when portable sanitization cannot replace it, reject malformed settings without panicking, and make portable pairing regeneration durable before UI state changes.\n\nMake the packaged smoke assertion tolerate slow WebView startup and clarify AppImage storage behavior.\n\nRefs #15 --- README.md | 4 +-- scripts/smoke-packaged-app.js | 24 +++++++------ src-tauri/src/db.rs | 63 +++++++++++++++++++++++++---------- src-tauri/src/lib.rs | 3 ++ 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index d5be7c5..529c9b6 100644 --- a/README.md +++ b/README.md @@ -60,11 +60,11 @@ Download desktop builds from [GitHub Releases](https://github.com/nimbold/Fireli | **macOS Apple silicon** | `.dmg` | Not notarized. If macOS blocks the first launch, approve Firelink in **System Settings -> Privacy & Security**. | | **Windows x64** | NSIS `.exe` installer | Unsigned. Windows SmartScreen may warn until code signing is added. | | **Windows x64 portable** | `.zip` archive | Extract to a writable folder and launch `firelink.exe`. App data stays under the archive's `data/` directory. | -| **Linux x64** | `.deb`, `.rpm`, or `.AppImage` | Use `.deb` for Debian-family systems, `.rpm` for Fedora/RPM-family systems, or AppImage as the portable fallback. AppImage may need executable permission. | +| **Linux x64** | `.deb`, `.rpm`, or `.AppImage` | Use `.deb` for Debian-family systems, `.rpm` for Fedora/RPM-family systems, or AppImage as the self-contained package. AppImage may need executable permission. | Bundles include the required engines. Users do not need aria2, yt-dlp, FFmpeg, Deno, Python, Homebrew, or another package manager. -The native packages use the distribution's normal desktop runtime dependencies. AppImage and the Windows portable ZIP are the portable distribution options for their respective platforms. +The native packages use the distribution's normal desktop runtime dependencies. The Windows portable ZIP keeps its application data beside the executable; AppImage is self-contained but uses the normal per-user application-data locations. The Windows portable archive is an opt-in secondary distribution. Keep the extracted folder writable; `Program Files`, read-only media, and some network or synchronized folders can prevent SQLite and WebView data from being saved. Close Firelink before copying or moving the folder. Only one Firelink instance can run at a time, so close the installed app before launching the portable copy. Portable mode keeps application settings, queues, logs, and WebView data beside the executable. Credentials, browser cookies, and URL query/fragment data are not persisted in portable queue records; active downloads that depend on those URL components are marked failed and must be added again after restart. Saved site passwords remain in the Windows credential store and are intentionally not copied into the archive. The portable folder contains the extension pairing credential needed to preserve extension integration, so treat the folder as sensitive and do not share it. Saved absolute download locations may need to be selected again if the folder is moved to a different drive. The installer remains the supported path for `firelink://` browser launch registration. diff --git a/scripts/smoke-packaged-app.js b/scripts/smoke-packaged-app.js index 89a1a2e..ff954b5 100755 --- a/scripts/smoke-packaged-app.js +++ b/scripts/smoke-packaged-app.js @@ -176,21 +176,25 @@ async function terminateChild() { return waitForChildExit(5000); } -function assertPortableStorage() { +async function assertPortableStorage() { const portableRoot = path.dirname(executable); const marker = path.join(portableRoot, 'portable.flag'); const database = path.join(portableRoot, 'data', 'firelink.sqlite'); const webviewData = path.join(portableRoot, 'data', 'webview'); - if (!fs.statSync(marker, { throwIfNoEntry: false })?.isFile()) { - throw new Error(`Portable marker was not found at ${marker}`); - } - if (!fs.statSync(database, { throwIfNoEntry: false })?.isFile()) { - throw new Error(`Portable database was not created at ${database}`); - } - if (!fs.statSync(webviewData, { throwIfNoEntry: false })?.isDirectory()) { - throw new Error(`Portable WebView data directory was not created at ${webviewData}`); + for (let attempt = 0; attempt < 40; attempt += 1) { + const markerReady = fs.statSync(marker, { throwIfNoEntry: false })?.isFile(); + const databaseReady = fs.statSync(database, { throwIfNoEntry: false })?.isFile(); + const webviewReady = fs.statSync(webviewData, { throwIfNoEntry: false })?.isDirectory(); + if (markerReady && databaseReady && webviewReady) { + return; + } + await sleep(250); } + + throw new Error( + `Portable storage was not ready: marker=${marker}, database=${database}, webview=${webviewData}`, + ); } try { @@ -212,7 +216,7 @@ try { assertNoVisibleWindows(child.pid); } if (assertPortableData) { - assertPortableStorage(); + await assertPortableStorage(); } if (childExit) { diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 39817b3..aaa3b9d 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -303,26 +303,31 @@ fn sanitize_legacy_source(path: &Path) -> Result<(), String> { return Ok(()); } - let file_name = path - .file_name() - .and_then(|name| name.to_str()) - .ok_or_else(|| format!("invalid legacy store path '{}'", path.display()))?; - let temporary = path.with_file_name(format!(".{file_name}.portable-sanitized.tmp")); - fs::write(&temporary, sanitized).map_err(|error| { + let parent = path + .parent() + .ok_or_else(|| format!("legacy store path has no parent: '{}'", path.display()))?; + use std::io::Write; + let mut temporary = tempfile::NamedTempFile::new_in(parent).map_err(|error| { format!( - "failed to write temporary sanitized legacy store '{}': {error}", - temporary.display() + "failed to create temporary sanitized legacy store beside '{}': {error}", + path.display() ) })?; - if let Err(rename_error) = fs::rename(&temporary, path) { - let _ = fs::remove_file(path); - fs::rename(&temporary, path).map_err(|error| { + temporary + .write_all(sanitized.as_bytes()) + .and_then(|_| temporary.flush()) + .map_err(|error| { format!( - "failed to replace legacy store '{}' after rename error ({rename_error}): {error}", + "failed to write temporary sanitized legacy store beside '{}': {error}", path.display() ) })?; - } + temporary.persist(path).map_err(|error| { + format!( + "failed to replace legacy store '{}' without losing the original: {}", + path.display(), error.error + ) + })?; Ok(()) } @@ -1071,12 +1076,17 @@ pub fn save_pairing_token_to_settings( let state = if value.get("state").is_some() { value .get_mut("state") - .expect("state is an object") + .and_then(serde_json::Value::as_object_mut) + .ok_or_else(|| "persisted settings state must be an object".to_string())? } else { - &mut value + value + .as_object_mut() + .ok_or_else(|| "persisted settings must be an object".to_string())? }; - state["extensionPairingToken"] = - serde_json::Value::String(token.to_string()); + state.insert( + "extensionPairingToken".to_string(), + serde_json::Value::String(token.to_string()), + ); let updated = serde_json::to_string(&value) .map_err(|error| format!("failed to encode settings: {error}"))?; save_settings(connection, &updated) @@ -1572,6 +1582,25 @@ mod tests { assert!(!saved.to_string().contains("PORTABLE_EXISTING_QUERY_TOKEN")); } + #[test] + fn rejects_malformed_settings_state_without_panicking() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + save_settings( + &connection, + &json!({ "state": "corrupted", "version": 3 }).to_string(), + ) + .unwrap(); + + let result = save_pairing_token_to_settings(&connection, "token", true); + + assert_eq!( + result.unwrap_err(), + "persisted settings state must be an object" + ); + } + #[test] fn pairing_token_is_persisted_before_frontend_settings_exist() { let temp = TempDir::new().unwrap(); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index af222ad..b96b7f2 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -4350,6 +4350,7 @@ fn get_free_space(app_handle: tauri::AppHandle, path: String) -> Result, state: tauri::State<'_, AppState>, id: String, password: String, @@ -4357,6 +4358,8 @@ fn set_keychain_password( if state.storage_layout.is_portable() && id == crate::db::PAIRING_TOKEN_KEYCHAIN_ID { + let connection = database.lock()?; + crate::db::save_pairing_token_to_settings(&connection, &password, true)?; return Ok(()); } crate::db::set_keychain_password(&id, &password)