diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 853615e..ce12a18 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -61,9 +61,8 @@ fn known_download_paths(app_handle: &tauri::AppHandle) -> Result, S } fn authorize_exact_path(requested: &Path, allowed_paths: &[PathBuf]) -> Result { - if std::fs::symlink_metadata(requested).is_ok_and(|metadata| metadata.file_type().is_symlink()) - { - return Err("Download path may not be a symlink".to_string()); + if crate::path_has_symlink_component(requested) { + return Err("Download path may not contain symlink components".to_string()); } let requested = canonicalize_with_missing_leaf(requested)?; @@ -74,8 +73,9 @@ fn authorize_exact_path(requested: &Path, allowed_paths: &[PathBuf]) -> Result

Result { let mut existing = path; let mut missing = Vec::new(); - while !existing.exists() { - let name = existing - .file_name() - .ok_or_else(|| "Download path has no existing ancestor".to_string())?; - missing.push(name.to_owned()); - existing = existing - .parent() - .ok_or_else(|| "Download path has no existing ancestor".to_string())?; + loop { + match std::fs::symlink_metadata(existing) { + Ok(metadata) => { + if metadata.file_type().is_symlink() { + return Err("Download path may not contain symlink components".to_string()); + } + break; + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + let name = existing + .file_name() + .ok_or_else(|| "Download path has no existing ancestor".to_string())?; + missing.push(name.to_owned()); + existing = existing + .parent() + .ok_or_else(|| "Download path has no existing ancestor".to_string())?; + } + Err(error) => { + return Err(format!( + "Failed to inspect download path '{}': {error}", + path.display() + )); + } + } } let mut canonical = std::fs::canonicalize(existing) @@ -144,7 +160,8 @@ mod tests { #[test] fn authorizes_only_known_download_files_and_partials() { let root = tempfile::tempdir().unwrap(); - let owned = root.path().join("owned.bin"); + let root = fs::canonicalize(root.path()).unwrap(); + let owned = root.join("owned.bin"); let outside = tempfile::NamedTempFile::new().unwrap(); fs::write(&owned, b"download").unwrap(); @@ -197,4 +214,22 @@ mod tests { assert!(authorize_exact_path(&escaped, std::slice::from_ref(&escaped)).is_err()); } + + #[cfg(unix)] + #[test] + fn rejects_parent_directory_symlink_escape_from_download_location() { + use std::os::unix::fs::symlink; + + let root = tempfile::tempdir().unwrap(); + let root_path = fs::canonicalize(root.path()).unwrap(); + let outside = tempfile::tempdir().unwrap(); + let outside_file = outside.path().join("owned.bin"); + fs::write(&outside_file, b"outside").unwrap(); + + let redirected_parent = root_path.join("downloads"); + symlink(outside.path(), &redirected_parent).unwrap(); + let escaped = redirected_parent.join("owned.bin"); + + assert!(authorize_exact_path(&escaped, std::slice::from_ref(&escaped)).is_err()); + } } diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 31758d5..82da8b5 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -8,11 +8,20 @@ const DATABASE_NAME: &str = "firelink.sqlite"; const LEGACY_STORE_NAME: &str = "store.bin"; const LEGACY_BUNDLE_IDENTIFIER: &str = "com.nima.tauri-app"; const CURRENT_SCHEMA_VERSION: i64 = 1; -const TOKEN_CHANGED_NOTICE: &str = "pairing-token-changed"; +pub(crate) const TOKEN_CHANGED_NOTICE: &str = "pairing-token-changed"; pub const PAIRING_TOKEN_KEYCHAIN_ID: &str = "extension-pairing-token"; const KEYCHAIN_SERVICE: &str = "com.firelink.app"; static KEYRING_OPERATION_LOCK: Mutex<()> = Mutex::new(()); +fn is_database_path(path: &Path) -> bool { + path.file_name().is_some_and(|name| { + name == DATABASE_NAME + || name + .to_string_lossy() + .starts_with(&format!("{DATABASE_NAME}.backup-")) + }) +} + pub struct DbState { conn: Mutex, portable: bool, @@ -26,20 +35,6 @@ impl DbState { } } -#[derive(Debug, Clone, PartialEq, Eq)] -enum PairingTokenSource { - Keychain, - LegacySettings, - Generated, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -struct PairingTokenDecision { - token: String, - source: PairingTokenSource, - changed: bool, -} - #[derive(Default)] struct LegacyData { settings: Option, @@ -50,15 +45,19 @@ struct LegacyData { } pub fn init(storage_layout: &crate::storage::StorageLayout) -> Result { - init_at_path_internal(storage_layout.data_dir(), storage_layout.is_portable()) + init_at_path_internal(storage_layout.data_dir(), storage_layout.is_portable(), false) } #[cfg(test)] fn init_at_path(app_data_dir: &Path) -> Result { - init_at_path_internal(app_data_dir, false) + init_at_path_internal(app_data_dir, false, false) } -fn init_at_path_internal(app_data_dir: &Path, portable: bool) -> Result { +fn init_at_path_internal( + app_data_dir: &Path, + portable: bool, + migrate_legacy_keychain: bool, +) -> Result { fs::create_dir_all(app_data_dir) .map_err(|error| format!("failed to create app data directory: {error}"))?; let database_path = app_data_dir.join(DATABASE_NAME); @@ -79,10 +78,12 @@ fn init_at_path_internal(app_data_dir: &Path, portable: bool) -> Result Result<(), fn import_legacy_data( connection: &mut Connection, app_data_dir: &Path, - migrate_keychain: bool, portable: bool, + migrate_keychain: bool, ) -> Result<(), String> { let legacy_app_dir = app_data_dir .parent() @@ -206,35 +207,49 @@ fn import_legacy_data( } let marker = format!("legacy-import:{}", candidate.to_string_lossy()); if metadata_exists(connection, &marker)? { - if portable { - sanitize_legacy_source(&candidate)?; - } + sanitize_legacy_source(&candidate, !portable)?; continue; } - if !portable { - backup_file(&candidate, "legacy-import")?; - } + let backup = if !portable { + Some(backup_file(&candidate, "legacy-import")?) + } else { + None + }; let mut legacy = if candidate .file_name() .is_some_and(|name| name == DATABASE_NAME) { - read_legacy_database(&candidate, migrate_keychain)? + read_legacy_database(&candidate, !portable)? } else { - read_legacy_store(&candidate, migrate_keychain)? + read_legacy_store(&candidate, !portable)? }; - let mut migration_complete = true; - if migrate_keychain - && get_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID).is_err() - && legacy.pairing_token.is_some() - { - if let Some(token) = legacy.pairing_token.as_deref() { - if let Err(error) = set_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID, token) { - log::warn!( - "Legacy pairing token could not be migrated yet; settings import will retry: {}", - error - ); - legacy.settings = None; - migration_complete = false; + let mut pending_pairing_token = None; + if !portable { + if let Some(token) = legacy.pairing_token.take() { + let migrated = if migrate_keychain { + let keychain_has_token = get_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID) + .ok() + .is_some_and(|value| !value.trim().is_empty()); + if !keychain_has_token { + if let Err(error) = + set_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID, &token) + { + log::warn!( + "Legacy pairing token could not be migrated to the credential store; it will remain pending in the current database: {}", + error + ); + false + } else { + true + } + } else { + true + } + } else { + false + }; + if !migrated { + pending_pairing_token = Some(token); } } } @@ -245,67 +260,105 @@ fn import_legacy_data( sanitize_download_strings(&mut legacy.downloads)?; } merge_legacy_data(connection, legacy)?; - if migration_complete { - connection - .execute( - "INSERT INTO metadata (key, value) VALUES (?1, 'complete') - ON CONFLICT(key) DO UPDATE SET value = excluded.value", - params![marker], - ) - .map_err(|error| format!("failed to record legacy import: {error}"))?; - if portable { - sanitize_legacy_source(&candidate)?; + if let Some(token) = pending_pairing_token { + if load_pairing_token_from_settings(connection)?.is_none() { + save_pairing_token_to_settings(connection, &token, true)?; } } + if let Some(backup) = backup.as_deref() { + sanitize_legacy_source(backup, !portable)?; + } + sanitize_legacy_source(&candidate, !portable)?; + connection + .execute( + "INSERT INTO metadata (key, value) VALUES (?1, 'complete') + ON CONFLICT(key) DO UPDATE SET value = excluded.value", + params![marker], + ) + .map_err(|error| format!("failed to record legacy import: {error}"))?; } Ok(()) } -fn sanitize_legacy_source(path: &Path) -> Result<(), String> { - if path - .file_name() - .is_some_and(|name| name == DATABASE_NAME) - { +fn sanitize_legacy_source(path: &Path, remove_pairing_token: bool) -> Result<(), String> { + if is_database_path(path) { let mut connection = Connection::open(path).map_err(|error| { format!( - "failed to open legacy database '{}' for portable sanitization: {error}", + "failed to open legacy database '{}' for sanitization: {error}", path.display() ) })?; + if remove_pairing_token && table_exists(&connection, "settings")? { + if let Some(settings) = connection + .query_row("SELECT data FROM settings WHERE id = 1", [], |row| { + row.get::<_, String>(0) + }) + .optional() + .map_err(|error| format!("failed to read legacy settings for sanitization: {error}"))? + { + let sanitized = strip_pairing_token_from_settings(&settings)?; + connection + .execute( + "UPDATE settings SET data = ?1 WHERE id = 1", + params![sanitized], + ) + .map_err(|error| format!("failed to sanitize legacy settings: {error}"))?; + } + } if table_exists(&connection, "downloads")? { - return sanitize_persisted_downloads(&mut connection); + sanitize_persisted_downloads(&mut connection)?; } return Ok(()); } let text = fs::read_to_string(path).map_err(|error| { format!( - "failed to read legacy store '{}' for portable sanitization: {error}", + "failed to read legacy store '{}' for sanitization: {error}", path.display() ) })?; let mut document: Value = serde_json::from_str(&text).map_err(|error| { format!( - "failed to decode legacy store '{}' for portable sanitization: {error}", + "failed to decode legacy store '{}' for sanitization: {error}", path.display() ) })?; - let Some(downloads) = document + if remove_pairing_token { + if let Some(settings) = document.get_mut("settings") { + let was_string = settings.is_string(); + let (sanitized, _, _) = sanitize_settings_value(settings, true)?; + *settings = if was_string { + Value::String(sanitized) + } else { + serde_json::from_str(&sanitized).map_err(|error| { + format!("failed to decode sanitized legacy settings: {error}") + })? + }; + } + } + if let Some(downloads) = document .get_mut("download_queue") .and_then(Value::as_array_mut) - else { - return Ok(()); - }; - for download in downloads { - remove_persisted_transfer_secrets(download); + { + for download in downloads { + remove_persisted_transfer_secrets(download); + } } + write_sanitized_legacy_store(path, &text, &document) +} + +fn write_sanitized_legacy_store( + path: &Path, + original: &str, + document: &Value, +) -> Result<(), String> { let sanitized = serde_json::to_string(&document).map_err(|error| { format!( - "failed to encode legacy store '{}' for portable sanitization: {error}", + "failed to encode legacy store '{}' for sanitization: {error}", path.display() ) })?; - if sanitized == text { + if sanitized == original { return Ok(()); } @@ -449,34 +502,6 @@ fn read_legacy_database(path: &Path, force_migrate: bool) -> Result Result<(bool, bool), String> { - let Some(settings) = load_settings(connection)? else { - return Ok((false, false)); - }; - let (sanitized, legacy_token, keychain_granted) = - sanitize_settings_text(&settings, force_migrate)?; - if sanitized == settings { - return Ok((false, keychain_granted)); - } - let should_migrate = force_migrate || keychain_granted; - if should_migrate && get_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID).is_err() { - if let Some(token) = legacy_token.filter(|token| !token.trim().is_empty()) { - if let Err(error) = set_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID, &token) { - log::warn!( - "Persisted pairing token could not be migrated yet; original settings retained: {}", - error - ); - return Ok((true, keychain_granted)); - } - } - } - save_settings(connection, &sanitized)?; - Ok((false, keychain_granted)) -} - fn sanitize_settings_value( value: &Value, force_migrate: bool, @@ -998,63 +1023,10 @@ pub fn consume_notice(connection: &Connection, key: &str) -> Result<(), String> Ok(()) } -pub fn hydrate_pairing_token( - connection: &mut Connection, - skip_keychain: bool, -) -> Result<(String, bool), String> { - if skip_keychain { - return Ok((generate_pairing_token(), false)); - } - - let existing = get_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID).ok(); - let generated = generate_pairing_token(); - let decision = decide_pairing_token( - existing.as_deref(), - None, - has_user_data(connection)?, - &generated, - ); - if decision.source != PairingTokenSource::Keychain { - set_keychain_password(PAIRING_TOKEN_KEYCHAIN_ID, &decision.token)?; - } - if decision.changed { - record_notice(connection, TOKEN_CHANGED_NOTICE)?; - } - let changed = has_pending_notice(connection, TOKEN_CHANGED_NOTICE)?; - Ok((decision.token, changed)) -} - pub fn acknowledge_pairing_token_notice(connection: &Connection) -> Result<(), String> { consume_notice(connection, TOKEN_CHANGED_NOTICE) } -fn decide_pairing_token( - keychain_token: Option<&str>, - legacy_token: Option<&str>, - has_existing_user_data: bool, - generated_token: &str, -) -> PairingTokenDecision { - if let Some(token) = keychain_token.filter(|token| !token.trim().is_empty()) { - return PairingTokenDecision { - token: token.to_string(), - source: PairingTokenSource::Keychain, - changed: false, - }; - } - if let Some(token) = legacy_token.filter(|token| !token.trim().is_empty()) { - return PairingTokenDecision { - token: token.to_string(), - source: PairingTokenSource::LegacySettings, - changed: false, - }; - } - PairingTokenDecision { - token: generated_token.to_string(), - source: PairingTokenSource::Generated, - changed: has_existing_user_data, - } -} - pub(crate) fn generate_pairing_token() -> String { format!( "{}{}", @@ -1063,10 +1035,9 @@ pub(crate) fn generate_pairing_token() -> String { ) } -/// Read the extension pairing token from the persisted settings JSON. -/// Returns `None` when the field is missing, empty, or the settings haven't -/// been saved yet. This is the **primary read path** — it does not touch the -/// OS keychain and therefore never triggers a system credential prompt. +/// Read the extension pairing token from portable settings JSON. +/// Standard-mode settings are sanitized so this field is never a credential +/// source outside the explicit portable-storage exception. pub fn load_pairing_token_from_settings(connection: &Connection) -> Result, String> { let Some(settings_json) = load_settings(connection)? else { return Ok(None); @@ -1082,8 +1053,8 @@ pub fn load_pairing_token_from_settings(connection: &Connection) -> Result Result { + let (sanitized, _, _) = sanitize_settings_text(data, true)?; + Ok(sanitized) +} + +/// Keep a legacy token in the standard settings document while credential-store +/// migration is pending. The backend never returns this copy to the frontend; +/// it is retained only so an unavailable credential store cannot turn a later +/// settings save into permanent pairing loss. +pub fn preserve_legacy_pairing_token( + existing: Option<&str>, + incoming: &str, +) -> Result { + let Some(existing) = existing else { + return Ok(incoming.to_string()); + }; + let (_, token, _) = sanitize_settings_text(existing, true)?; + let Some(token) = token.filter(|value| !value.trim().is_empty()) else { + return Ok(incoming.to_string()); + }; + + let mut document: Value = serde_json::from_str(incoming) + .map_err(|error| format!("failed to decode settings for legacy token preservation: {error}"))?; + let state = 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())? + }; + state.insert("extensionPairingToken".to_string(), Value::String(token)); + serde_json::to_string(&document) + .map_err(|error| format!("failed to encode settings with pending pairing token: {error}")) +} + +/// Read a legacy pairing token from the settings database without changing it. +pub fn read_pairing_token_from_settings( + connection: &Connection, +) -> Result, String> { + let Some(settings) = load_settings(connection)? else { + return Ok(None); + }; + let (_, token, _) = sanitize_settings_text(&settings, true)?; + Ok(token.filter(|value| !value.trim().is_empty())) +} + +/// Remove a legacy pairing token from the settings database. +pub fn remove_pairing_token_from_settings(connection: &Connection) -> Result<(), String> { + let Some(settings) = load_settings(connection)? else { + return Ok(()); + }; + let (sanitized, _, _) = sanitize_settings_text(&settings, true)?; + if sanitized != settings { + save_settings(connection, &sanitized)?; + } + Ok(()) +} + +/// Migrate any legacy standard-mode token into the OS credential store. +/// +/// The settings copy is removed only after the credential-store write succeeds. +/// If cleanup fails after creating a new credential, the new entry is rolled +/// back so a later retry can complete the migration without losing the token. +pub fn migrate_legacy_pairing_token(connection: &Connection) -> Result<(), String> { + let Some(legacy_token) = read_pairing_token_from_settings(connection)? else { + return Ok(()); + }; + + // Hold the same lock used by the public credential-store commands across + // the complete read/write/cleanup sequence. Otherwise a concurrent grant, + // regeneration, or delete could invalidate the rollback decision. + let _keyring_guard = lock_keyring_operations()?; + let keychain_has_token = get_keychain_password_unlocked(PAIRING_TOKEN_KEYCHAIN_ID) + .ok() + .is_some_and(|value| !value.trim().is_empty()); + let created_keychain_entry = !keychain_has_token; + if created_keychain_entry { + set_keychain_password_unlocked(PAIRING_TOKEN_KEYCHAIN_ID, &legacy_token)?; + } + + if let Err(error) = remove_pairing_token_from_settings(connection) { + if created_keychain_entry { + if let Err(rollback_error) = delete_keychain_password_unlocked(PAIRING_TOKEN_KEYCHAIN_ID) + { + return Err(format!( + "failed to remove the legacy pairing token after credential-store migration: {error}; credential-store rollback also failed: {rollback_error}" + )); + } + } + return Err(format!( + "failed to remove the legacy pairing token after credential-store migration: {error}" + )); + } + + Ok(()) +} + fn ensure_keyring_store() -> Result<(), String> { if keyring_core::get_default_store().is_some() { return Ok(()); @@ -1231,8 +1307,7 @@ fn unique_legacy_linux_keychain_entry(id: &str) -> Result Result<(), String> { - let _guard = lock_keyring_operations()?; +fn set_keychain_password_unlocked(id: &str, password: &str) -> Result<(), String> { let entry = keychain_entry(id)?; #[cfg(target_os = "linux")] @@ -1254,8 +1329,12 @@ pub fn set_keychain_password(id: &str, password: &str) -> Result<(), String> { .map_err(|error| error.to_string()) } -pub fn get_keychain_password(id: &str) -> Result { +pub fn set_keychain_password(id: &str, password: &str) -> Result<(), String> { let _guard = lock_keyring_operations()?; + set_keychain_password_unlocked(id, password) +} + +fn get_keychain_password_unlocked(id: &str) -> Result { let entry = keychain_entry(id)?; match entry.get_password() { Ok(password) => Ok(password), @@ -1271,8 +1350,12 @@ pub fn get_keychain_password(id: &str) -> Result { } } -pub fn delete_keychain_password(id: &str) -> Result<(), String> { +pub fn get_keychain_password(id: &str) -> Result { let _guard = lock_keyring_operations()?; + get_keychain_password_unlocked(id) +} + +fn delete_keychain_password_unlocked(id: &str) -> Result<(), String> { let entry = keychain_entry(id)?; match entry.delete_credential() { Ok(()) | Err(keyring_core::Error::NoEntry) => {} @@ -1290,6 +1373,11 @@ pub fn delete_keychain_password(id: &str) -> Result<(), String> { Ok(()) } +pub fn delete_keychain_password(id: &str) -> Result<(), String> { + let _guard = lock_keyring_operations()?; + delete_keychain_password_unlocked(id) +} + #[cfg(test)] mod tests { use super::*; @@ -1362,7 +1450,7 @@ mod tests { .unwrap(); drop(connection); - let state = init_at_path_internal(temp.path(), true).unwrap(); + let state = init_at_path_internal(temp.path(), true, true).unwrap(); let connection = state.lock().unwrap(); let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap(); assert!(saved.get("password").is_none()); @@ -1375,7 +1463,7 @@ mod tests { } #[test] - fn imports_legacy_bundle_store_and_preserves_token() { + fn imports_legacy_bundle_store_with_pending_token_for_deferred_migration() { let root = TempDir::new().unwrap(); let current = root.path().join("com.nimbold.firelink"); let legacy = root.path().join(LEGACY_BUNDLE_IDENTIFIER); @@ -1415,12 +1503,22 @@ mod tests { let settings = load_settings(&connection).unwrap().unwrap(); assert!(settings.contains("\"theme\":\"dark\"")); assert!(settings.contains("legacy-secret")); - assert!(fs::read_dir(&legacy).unwrap().flatten().any(|entry| { - entry - .file_name() - .to_string_lossy() - .starts_with("store.bin.backup-legacy-import-") - })); + let backup = fs::read_dir(&legacy) + .unwrap() + .flatten() + .find(|entry| { + entry + .file_name() + .to_string_lossy() + .starts_with("store.bin.backup-legacy-import-") + }) + .expect("legacy import should retain a sanitized backup"); + assert!(!fs::read_to_string(backup.path()) + .unwrap() + .contains("legacy-secret")); + assert!(!fs::read_to_string(legacy.join(LEGACY_STORE_NAME)) + .unwrap() + .contains("legacy-secret")); } #[test] @@ -1442,7 +1540,7 @@ mod tests { }); fs::write(&store_path, serde_json::to_vec(&store).unwrap()).unwrap(); - let state = init_at_path_internal(¤t, true).unwrap(); + let state = init_at_path_internal(¤t, true, true).unwrap(); let connection = state.lock().unwrap(); let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap(); assert!(saved.get("password").is_none()); @@ -1479,7 +1577,7 @@ mod tests { ); INSERT INTO settings VALUES ( 1, - '{\"state\":{\"theme\":\"nord\"},\"version\":0}' + '{\"state\":{\"theme\":\"nord\",\"extensionPairingToken\":\"legacy-sqlite-secret\"},\"version\":0}' ); ", ) @@ -1490,16 +1588,30 @@ mod tests { let connection = state.lock().unwrap(); assert_eq!(load_downloads(&connection).unwrap().len(), 1); assert_eq!(load_queues(&connection).unwrap().len(), 1); - assert!(load_settings(&connection) + let settings = load_settings(&connection).unwrap().unwrap(); + assert!(settings.contains("\"nord\"")); + assert!(settings.contains("legacy-sqlite-secret")); + let backup = fs::read_dir(&legacy) .unwrap() - .unwrap() - .contains("\"nord\"")); - assert!(fs::read_dir(&legacy).unwrap().flatten().any(|entry| { - entry - .file_name() - .to_string_lossy() - .starts_with("firelink.sqlite.backup-legacy-import-") - })); + .flatten() + .find(|entry| { + entry + .file_name() + .to_string_lossy() + .starts_with("firelink.sqlite.backup-legacy-import-") + }) + .unwrap(); + let backup_connection = Connection::open(backup.path()).unwrap(); + let backup_settings: String = backup_connection + .query_row("SELECT data FROM settings WHERE id = 1", [], |row| row.get(0)) + .unwrap(); + assert!(!backup_settings.contains("legacy-sqlite-secret")); + drop(backup_connection); + let source_connection = Connection::open(legacy.join(DATABASE_NAME)).unwrap(); + let source_settings: String = source_connection + .query_row("SELECT data FROM settings WHERE id = 1", [], |row| row.get(0)) + .unwrap(); + assert!(!source_settings.contains("legacy-sqlite-secret")); } #[test] @@ -1571,6 +1683,75 @@ mod tests { assert!(standard.to_string().contains("PORTABLE_TEST_QUERY_TOKEN")); } + #[test] + fn standard_pairing_token_is_stripped_from_settings_documents() { + let input = json!({ + "state": { + "theme": "dark", + "extensionPairingToken": "redacted-pairing-token" + }, + "version": 3 + }) + .to_string(); + + let stripped = strip_pairing_token_from_settings(&input).unwrap(); + assert!(!stripped.contains("redacted-pairing-token")); + assert!(stripped.contains("\"theme\":\"dark\"")); + } + + #[test] + fn pending_legacy_pairing_token_survives_standard_settings_save() { + let existing = json!({ + "state": { "theme": "dark", "extensionPairingToken": "pending-token" }, + "version": 3 + }) + .to_string(); + let incoming = json!({ + "state": { "theme": "light" }, + "version": 3 + }) + .to_string(); + + let sanitized = strip_pairing_token_from_settings(&incoming).unwrap(); + let preserved = preserve_legacy_pairing_token(Some(&existing), &sanitized).unwrap(); + + assert!(preserved.contains("pending-token")); + assert!(preserved.contains("\"theme\":\"light\"")); + } + + #[test] + fn reading_legacy_pairing_token_does_not_remove_it_before_migration() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + save_settings( + &connection, + &json!({ + "state": { "extensionPairingToken": "redacted-legacy-token" }, + "version": 3 + }) + .to_string(), + ) + .unwrap(); + + assert_eq!( + read_pairing_token_from_settings(&connection) + .unwrap() + .as_deref(), + Some("redacted-legacy-token") + ); + assert!(load_settings(&connection) + .unwrap() + .unwrap() + .contains("redacted-legacy-token")); + + remove_pairing_token_from_settings(&connection).unwrap(); + assert!(!load_settings(&connection) + .unwrap() + .unwrap() + .contains("redacted-legacy-token")); + } + #[test] fn portable_persistence_redacts_unparseable_download_urls() { let temp = TempDir::new().unwrap(); @@ -1608,7 +1789,7 @@ mod tests { drop(connection); drop(state); - let state = init_at_path_internal(temp.path(), true).unwrap(); + let state = init_at_path_internal(temp.path(), true, true).unwrap(); let connection = state.lock().unwrap(); let saved: Value = serde_json::from_str(&load_downloads(&connection).unwrap()[0]).unwrap(); assert!(saved.get("password").is_none()); @@ -1648,27 +1829,6 @@ mod tests { ); } - #[test] - fn token_decision_preserves_keychain_and_legacy_values() { - let keychain = decide_pairing_token(Some("keychain"), Some("legacy"), true, "generated"); - assert_eq!(keychain.token, "keychain"); - assert_eq!(keychain.source, PairingTokenSource::Keychain); - assert!(!keychain.changed); - - let legacy = decide_pairing_token(None, Some("legacy"), true, "generated"); - assert_eq!(legacy.token, "legacy"); - assert_eq!(legacy.source, PairingTokenSource::LegacySettings); - assert!(!legacy.changed); - - let recovery = decide_pairing_token(None, None, true, "generated"); - assert_eq!(recovery.token, "generated"); - assert_eq!(recovery.source, PairingTokenSource::Generated); - assert!(recovery.changed); - - let fresh = decide_pairing_token(None, None, false, "generated"); - assert!(!fresh.changed); - } - #[test] fn migration_notice_is_persistent_until_acknowledged() { let temp = TempDir::new().unwrap(); diff --git a/src-tauri/src/download_ownership.rs b/src-tauri/src/download_ownership.rs index ce3e8e2..82c02ea 100644 --- a/src-tauri/src/download_ownership.rs +++ b/src-tauri/src/download_ownership.rs @@ -59,7 +59,12 @@ pub fn expected_primary_path( } let safe_filename = canonical_download_filename(filename); - Ok(resolved_dest.join(safe_filename)) + let path = resolved_dest.join(safe_filename); + if crate::path_has_symlink_component(&path) { + return Err("Download path may not contain symlink components".to_string()); + } + crate::canonicalize_with_missing_components(&path) + .ok_or_else(|| "Download path could not be canonicalized".to_string()) } pub fn register_expected( @@ -88,13 +93,15 @@ pub fn set_primary_path( }) { return Err("Download ownership path traversal is not allowed".to_string()); } - if std::fs::symlink_metadata(path).is_ok_and(|metadata| metadata.file_type().is_symlink()) { - return Err("Download ownership path may not be a symlink".to_string()); + if crate::path_has_symlink_component(path) { + return Err("Download ownership path may not contain symlink components".to_string()); } + let canonical_path = crate::canonicalize_with_missing_components(path) + .ok_or_else(|| "Download ownership path could not be canonicalized".to_string())?; let database = app_handle.state::(); let connection = database.lock()?; - crate::db::set_ownership(&connection, id, &path.to_string_lossy()) + crate::db::set_ownership(&connection, id, &canonical_path.to_string_lossy()) } pub fn remove(app_handle: &tauri::AppHandle, id: &str) -> Result<(), String> { @@ -147,11 +154,7 @@ fn legacy_download_queue_paths(app_handle: &tauri::AppHandle) -> Result(); let connection = database.lock()?; - crate::db::load_downloads(&connection)? - .into_iter() - .map(|value| serde_json::from_str::(&value)) - .collect::, _>>() - .map_err(|error| format!("Invalid download queue ownership data: {error}"))? + parse_legacy_download_items(crate::db::load_downloads(&connection)?) }; let mut paths = Vec::new(); @@ -223,9 +226,23 @@ fn legacy_download_queue_paths(app_handle: &tauri::AppHandle) -> Result) -> Vec { + values + .into_iter() + .filter_map(|value| match serde_json::from_str::(&value) { + Ok(download) => Some(download), + Err(error) => { + log::warn!("Skipping malformed download ownership record: {error}"); + None + } + }) + .collect() +} + #[cfg(test)] mod tests { - use super::canonical_download_filename; + use super::{canonical_download_filename, parse_legacy_download_items}; + use serde_json::json; #[test] fn canonicalizes_untrusted_download_filenames() { @@ -238,4 +255,22 @@ mod tests { assert_eq!(canonical_download_filename("CON.txt"), "CON-.txt"); assert_eq!(canonical_download_filename("lpt9"), "lpt9-"); } + + #[test] + fn malformed_legacy_download_does_not_block_valid_ownership_records() { + let valid = json!({ + "id": "download-1", + "url": "https://example.com/file", + "fileName": "file", + "status": "completed", + "category": "Other", + "dateAdded": "" + }) + .to_string(); + + let downloads = parse_legacy_download_items(vec!["not-json".to_string(), valid]); + + assert_eq!(downloads.len(), 1); + assert_eq!(downloads[0].id, "download-1"); + } } diff --git a/src-tauri/src/ipc.rs b/src-tauri/src/ipc.rs index 7ad52af..f0ade5f 100644 --- a/src-tauri/src/ipc.rs +++ b/src-tauri/src/ipc.rs @@ -280,13 +280,6 @@ pub struct PersistedSettings { pub prevents_sleep_while_downloading: bool, pub media_cookie_source: MediaCookieSource, pub site_logins: Vec, - // HMAC shared secret for the browser extension. It is persisted in the - // settings database so startup never needs to touch the OS keychain. - // The keychain is still used as defense-in-depth by grant_keychain_access, - // but the DB copy is the primary read path, eliminating the OS credential - // prompt that macOS shows when the binary signature changes after an update. - #[serde(default)] - pub extension_pairing_token: String, pub auto_check_updates: bool, #[serde(default)] pub keychain_access_granted: bool, diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 2da6475..ff274d5 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1329,6 +1329,23 @@ async fn validate_url_ssrf(url: &str) -> Result bool { + left.scheme() == right.scheme() + && left.host() == right.host() + && left.port_or_known_default() == right.port_or_known_default() +} + +fn should_send_metadata_credentials( + original: Option<&reqwest::Url>, + current: Option<&reqwest::Url>, + redirects: usize, +) -> bool { + redirects == 0 + || original + .zip(current) + .is_some_and(|(original, current)| same_origin(original, current)) +} + #[allow(clippy::too_many_arguments)] // Keep the metadata IPC fields explicit and independently typed. #[tauri::command] async fn fetch_metadata( @@ -1344,7 +1361,7 @@ async fn fetch_metadata( ensure_reqwest_crypto_provider(); let mut current_url = url.clone(); - let original_host = reqwest::Url::parse(&url).ok().and_then(|u| u.host_str().map(|s| s.to_string())); + let original_origin = reqwest::Url::parse(&url).ok(); let mut redirects = 0; let cookies_available = metadata_cookie_header_present(headers.as_deref(), cookies.as_deref()); let defer_cookies = defer_cookies.unwrap_or(false); @@ -1385,15 +1402,12 @@ async fn fetch_metadata( builder = builder.resolve(&host, addr); } - let current_host = reqwest::Url::parse(¤t_url).ok().and_then(|u| u.host_str().map(|s| s.to_string())); - let mut should_send_auth = redirects == 0; - if !should_send_auth { - if let (Some(orig), Some(curr)) = (&original_host, ¤t_host) { - if curr == orig || curr.ends_with(&format!(".{}", orig)) { - should_send_auth = true; - } - } - } + let current_origin = reqwest::Url::parse(¤t_url).ok(); + let should_send_auth = should_send_metadata_credentials( + original_origin.as_ref(), + current_origin.as_ref(), + redirects, + ); let header_map = if should_send_auth { metadata_headers(headers.as_deref(), cookies.as_deref(), send_cookies) @@ -1926,12 +1940,51 @@ pub(crate) fn is_safe_path(path: &std::path::Path, app_handle .any(|root| crate::platform::path_is_within(&canonical_path, &root)) } -fn canonicalize_with_missing_components(path: &std::path::Path) -> Option { +pub(crate) fn path_has_symlink_component(path: &std::path::Path) -> bool { + use std::path::Component; + + let mut current = std::path::PathBuf::new(); + for component in path.components() { + match component { + Component::Prefix(_) | Component::RootDir => { + current.push(component.as_os_str()); + } + Component::Normal(name) => { + current.push(name); + if std::fs::symlink_metadata(¤t) + .is_ok_and(|metadata| metadata.file_type().is_symlink()) + { + return true; + } + } + Component::CurDir | Component::ParentDir => return true, + } + } + false +} + +pub(crate) fn canonicalize_with_missing_components( + path: &std::path::Path, +) -> Option { + if path_has_symlink_component(path) { + return None; + } let mut existing = path; let mut missing = Vec::new(); - while !existing.exists() { - missing.push(existing.file_name()?.to_owned()); - existing = existing.parent()?; + loop { + match std::fs::symlink_metadata(existing) { + Ok(metadata) => { + if metadata.file_type().is_symlink() { + return None; + } + break; + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + missing.push(existing.file_name()?.to_owned()); + existing = existing.parent()?; + } + Err(_) => return None, + } } let mut canonical = std::fs::canonicalize(existing).ok()?; for component in missing.iter().rev() { @@ -4854,14 +4907,9 @@ struct PairingTokenHydration { error: Option, } -/// Hydrate the extension pairing token on startup **without touching the OS -/// keychain**. The token is read from the persisted settings (SQLite) so the -/// operating system never presents a credential-access prompt before the UI is -/// visible — even after a build update where the code signature changed. -/// -/// When no token has been persisted yet (fresh install) a new one is generated -/// and `persistent` is returned as `false`, which causes the frontend to show -/// the `KeychainPermissionModal`. +/// Hydrate the extension pairing token after the frontend is ready. Standard +/// mode uses the OS credential store; portable mode intentionally keeps the +/// token with the portable settings folder. #[tauri::command] fn hydrate_extension_pairing_token( database: tauri::State<'_, crate::db::DbState>, @@ -4869,37 +4917,96 @@ fn hydrate_extension_pairing_token( ) -> Result { let connection = database.lock()?; - // Primary path: read the token from the settings DB. This is always safe - // and never triggers an OS prompt. - if let Some(existing) = crate::db::load_pairing_token_from_settings(&connection)? { + if app_state.storage_layout.is_portable() { + let token = crate::db::load_pairing_token_from_settings(&connection)? + .unwrap_or_else(crate::db::generate_pairing_token); + crate::db::save_pairing_token_to_settings(&connection, &token, true)?; if let Ok(mut pairing_token) = app_state.extension_pairing_token.write() { - *pairing_token = existing.clone(); + *pairing_token = token.clone(); } return Ok(PairingTokenHydration { - token: existing, + token, token_changed: false, persistent: true, error: None, }); } - // No token in the DB yet — generate one. Portable mode initializes the - // settings row immediately so the pairing secret travels with the folder; - // standard mode remains session-only until the user grants credential-store - // access when settings have not been persisted yet. + let migration_error = crate::db::migrate_legacy_pairing_token(&connection).err(); + let keychain_token = crate::db::get_keychain_password(crate::db::PAIRING_TOKEN_KEYCHAIN_ID) + .ok() + .filter(|value| !value.trim().is_empty()); + let persistent = keychain_token.is_some(); + let token = keychain_token.unwrap_or_else(crate::db::generate_pairing_token); + if !persistent && crate::db::has_user_data(&connection)? { + crate::db::record_notice(&connection, crate::db::TOKEN_CHANGED_NOTICE)?; + } + let token_changed = + crate::db::has_pending_notice(&connection, crate::db::TOKEN_CHANGED_NOTICE)?; + if let Ok(mut pairing_token) = app_state.extension_pairing_token.write() { + *pairing_token = token.clone(); + } + Ok(PairingTokenHydration { + token, + token_changed, + persistent, + error: migration_error.or_else(|| { + (!persistent).then(|| { + "Credential store access is unavailable; browser pairing is session-only.".to_string() + }) + }), + }) +} + +#[tauri::command] +fn regenerate_pairing_token( + database: tauri::State<'_, crate::db::DbState>, + app_state: tauri::State<'_, AppState>, +) -> Result { + let connection = database.lock()?; let generated = crate::db::generate_pairing_token(); - crate::db::save_pairing_token_to_settings( - &connection, - &generated, - app_state.storage_layout.is_portable(), - )?; + + if app_state.storage_layout.is_portable() { + crate::db::save_pairing_token_to_settings(&connection, &generated, true)?; + } else { + if let Err(error) = crate::db::migrate_legacy_pairing_token(&connection) { + let token = app_state + .extension_pairing_token + .read() + .map_err(|_| "Extension pairing token lock is unavailable".to_string())? + .clone(); + return Ok(PairingTokenHydration { + token, + token_changed: false, + persistent: false, + error: Some(error), + }); + } + if let Err(error) = crate::db::set_keychain_password( + crate::db::PAIRING_TOKEN_KEYCHAIN_ID, + &generated, + ) { + let token = app_state + .extension_pairing_token + .read() + .map_err(|_| "Extension pairing token lock is unavailable".to_string())? + .clone(); + return Ok(PairingTokenHydration { + token, + token_changed: false, + persistent: false, + error: Some(error), + }); + } + } + if let Ok(mut pairing_token) = app_state.extension_pairing_token.write() { *pairing_token = generated.clone(); } Ok(PairingTokenHydration { token: generated, token_changed: false, - persistent: app_state.storage_layout.is_portable(), + persistent: true, error: None, }) } @@ -4909,7 +5016,7 @@ fn grant_keychain_access( database: tauri::State<'_, crate::db::DbState>, app_state: tauri::State<'_, AppState>, ) -> Result { - let mut connection = database.lock()?; + let connection = database.lock()?; if app_state.storage_layout.is_portable() { let token = if let Some(existing) = crate::db::load_pairing_token_from_settings(&connection)? @@ -4931,41 +5038,55 @@ fn grant_keychain_access( }); } - // Explicitly force migration of any legacy token to the keychain. - // This is the ONLY code path that touches the OS keychain and it is - // reached exclusively through the frontend's "Grant Access" button, - // so any system prompt is user-initiated. - let _ = crate::db::sanitize_current_settings_and_restore_token(&connection, true); + if let Err(error) = crate::db::migrate_legacy_pairing_token(&connection) { + let token = app_state + .extension_pairing_token + .read() + .map_err(|_| "Extension pairing token lock is unavailable".to_string())? + .clone(); + return Ok(PairingTokenHydration { + token, + token_changed: false, + persistent: false, + error: Some(error), + }); + } - match crate::db::hydrate_pairing_token(&mut connection, false) { - Ok((token, token_changed)) => { - // Persist the token to the settings DB so future startups - // can read it without touching the keychain at all. - let _ = crate::db::save_pairing_token_to_settings(&connection, &token, false); - if let Ok(mut pairing_token) = app_state.extension_pairing_token.write() { - *pairing_token = token.clone(); + let token = match crate::db::get_keychain_password(crate::db::PAIRING_TOKEN_KEYCHAIN_ID) { + Ok(token) if !token.trim().is_empty() => token, + _ => { + let generated = crate::db::generate_pairing_token(); + if let Err(error) = crate::db::set_keychain_password( + crate::db::PAIRING_TOKEN_KEYCHAIN_ID, + &generated, + ) { + let current = app_state + .extension_pairing_token + .read() + .map_err(|_| "Extension pairing token lock is unavailable".to_string())? + .clone(); + return Ok(PairingTokenHydration { + token: current, + token_changed: false, + persistent: false, + error: Some(error), + }); } - Ok(PairingTokenHydration { - token, - token_changed, - persistent: true, - error: None, - }) + generated } - Err(error) => { - let token = app_state - .extension_pairing_token - .read() - .map_err(|_| "Extension pairing token lock is unavailable".to_string())? - .clone(); - Ok(PairingTokenHydration { - token, - token_changed: false, - persistent: false, - error: Some(error), - }) + }; + + { + if let Ok(mut pairing_token) = app_state.extension_pairing_token.write() { + *pairing_token = token.clone(); } } + Ok(PairingTokenHydration { + token, + token_changed: false, + persistent: true, + error: None, + }) } #[tauri::command] @@ -4988,7 +5109,8 @@ fn db_save_settings( let merged = if state.is_portable() { crate::settings::preserve_portable_pairing_token(existing.as_deref(), &merged)? } else { - merged + let sanitized = crate::db::strip_pairing_token_from_settings(&merged)?; + crate::db::preserve_legacy_pairing_token(existing.as_deref(), &sanitized)? }; crate::db::save_settings(&connection, &merged)?; let decoded = crate::settings::decode_stored_settings(&serde_json::Value::String(merged))?; @@ -5001,7 +5123,13 @@ fn db_save_settings( #[tauri::command] fn db_load_settings(state: tauri::State<'_, crate::db::DbState>) -> Result, String> { let connection = state.lock()?; - crate::db::load_settings(&connection) + let settings = crate::db::load_settings(&connection)?; + if state.is_portable() { + return Ok(settings); + } + settings + .map(|data| crate::db::strip_pairing_token_from_settings(&data)) + .transpose() } #[tauri::command] @@ -5046,27 +5174,59 @@ fn check_file_exists(app_handle: tauri::AppHandle, path: String) -> bool { resolved_dest.exists() } +fn collect_log_files(log_dir: &std::path::Path) -> Result, String> { + let directory_metadata = match std::fs::symlink_metadata(log_dir) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(format!("failed to inspect log directory: {error}")), + }; + if directory_metadata.file_type().is_symlink() || !directory_metadata.is_dir() { + return Ok(Vec::new()); + } + + let canonical_log_dir = match std::fs::canonicalize(log_dir) { + Ok(path) if path.is_dir() => path, + Ok(_) | Err(_) => return Ok(Vec::new()), + }; + let entries = match std::fs::read_dir(log_dir) { + Ok(entries) => entries, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(error) => return Err(format!("failed to read log directory: {error}")), + }; + + let mut files = Vec::new(); + for entry in entries { + let path = entry + .map_err(|error| format!("failed to inspect log directory entry: {error}"))? + .path(); + let metadata = std::fs::symlink_metadata(&path) + .map_err(|error| format!("failed to inspect log file '{}': {error}", path.display()))?; + if !metadata.file_type().is_file() + || metadata.file_type().is_symlink() + || !path + .file_name() + .is_some_and(|name| name.to_string_lossy().contains(".log")) + { + continue; + } + + let canonical_path = std::fs::canonicalize(&path) + .map_err(|error| format!("failed to resolve log file '{}': {error}", path.display()))?; + if canonical_path.starts_with(&canonical_log_dir) { + files.push(canonical_path); + } + } + files.sort(); + Ok(files) +} + async fn log_files(app_handle: &tauri::AppHandle) -> Result, String> { let log_dir = app_handle .state::() .storage_layout .log_dir() .to_path_buf(); - let mut files = Vec::new(); - if let Ok(mut entries) = tokio::fs::read_dir(&log_dir).await { - while let Ok(Some(entry)) = entries.next_entry().await { - let path = entry.path(); - if path.is_file() - && path - .file_name() - .is_some_and(|name| name.to_string_lossy().contains(".log")) - { - files.push(path); - } - } - } - files.sort(); - Ok(files) + collect_log_files(&log_dir) } pub(crate) fn redact_sensitive_text(line: &str) -> String { @@ -5386,7 +5546,8 @@ mod tests { parse_firelink_deep_link, parse_ffmpeg_version, parse_media_progress_line, redact_log_line, redact_log_line_for_output, sanitize_ytdlp_config_value, has_resumable_download_assets, should_cleanup_media_artifacts_after_failure, - retry_metadata_with_cookies, should_retry_metadata_with_cookies, FirelinkDeepLink, + retry_metadata_with_cookies, should_retry_metadata_with_cookies, + should_send_metadata_credentials, collect_log_files, FirelinkDeepLink, MediaProgress, MediaSpeedSampler, MEDIA_PROGRESS_PREFIX, }; @@ -5419,6 +5580,66 @@ mod tests { ); } + #[test] + fn metadata_redirect_credentials_require_the_exact_origin() { + let original = reqwest::Url::parse("https://example.com/file").unwrap(); + let same_origin = reqwest::Url::parse("https://example.com/other").unwrap(); + let subdomain = reqwest::Url::parse("https://cdn.example.com/file").unwrap(); + let different_port = reqwest::Url::parse("https://example.com:8443/file").unwrap(); + let downgraded = reqwest::Url::parse("http://example.com/file").unwrap(); + + assert!(should_send_metadata_credentials( + Some(&original), + Some(&original), + 0 + )); + assert!(should_send_metadata_credentials( + Some(&original), + Some(&same_origin), + 1 + )); + assert!(!should_send_metadata_credentials( + Some(&original), + Some(&subdomain), + 1 + )); + assert!(!should_send_metadata_credentials( + Some(&original), + Some(&different_port), + 1 + )); + assert!(!should_send_metadata_credentials( + Some(&original), + Some(&downgraded), + 1 + )); + } + + #[cfg(unix)] + #[test] + fn log_collection_rejects_symlink_files_and_directories() { + use std::os::unix::fs::symlink; + + let root = tempfile::tempdir().unwrap(); + let log_dir = root.path().join("logs"); + let outside = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(&log_dir).unwrap(); + std::fs::write(log_dir.join("firelink.log"), "safe").unwrap(); + std::fs::write(outside.path().join("secret.log"), "redacted-fixture").unwrap(); + symlink( + outside.path().join("secret.log"), + log_dir.join("attacker.log"), + ) + .unwrap(); + + let files = collect_log_files(&log_dir).unwrap(); + assert_eq!(files, vec![std::fs::canonicalize(log_dir.join("firelink.log")).unwrap()]); + + let redirected_logs = root.path().join("redirected-logs"); + symlink(outside.path(), &redirected_logs).unwrap(); + assert!(collect_log_files(&redirected_logs).unwrap().is_empty()); + } + #[test] fn metadata_defers_captured_cookies_until_the_origin_challenges() { let headers = "Cookie: oversized=secret\nReferer: https://example.com/page"; @@ -6905,7 +7126,8 @@ pub fn run() { ack_schedule_trigger, check_automation_permission, request_automation_permission, open_automation_settings, set_keychain_password, get_keychain_password, delete_keychain_password, - hydrate_extension_pairing_token, grant_keychain_access, acknowledge_pairing_token_change, + hydrate_extension_pairing_token, regenerate_pairing_token, grant_keychain_access, + acknowledge_pairing_token_change, check_file_exists, toggle_tray_icon, set_extension_pairing_token, get_extension_server_port, set_extension_frontend_ready, set_concurrent_limit, set_global_speed_limit, remove_download, detach_download_for_reconfigure, diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index cc688c5..8930826 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -446,7 +446,6 @@ fn default_settings() -> PersistedSettings { prevents_sleep_while_downloading: true, media_cookie_source: MediaCookieSource::default(), site_logins: Vec::new(), - extension_pairing_token: String::new(), auto_check_updates: true, keychain_access_granted: false, } diff --git a/src-tauri/src/storage.rs b/src-tauri/src/storage.rs index d41f40b..b918d30 100644 --- a/src-tauri/src/storage.rs +++ b/src-tauri/src/storage.rs @@ -55,31 +55,38 @@ impl StorageLayout { app_handle: &AppHandle, mode: StorageMode, ) -> Result { - match mode { - StorageMode::Standard => Ok(Self { - mode: StorageMode::Standard, - data_dir: app_handle + let (mode, data_dir, log_dir, webview_dir) = match mode { + StorageMode::Standard => ( + StorageMode::Standard, + app_handle .path() .app_data_dir() .map_err(|error| format!("failed to resolve app data directory: {error}"))?, - log_dir: app_handle + app_handle .path() .app_log_dir() .map_err(|error| format!("failed to resolve app log directory: {error}"))?, - webview_dir: app_handle.path().app_local_data_dir().map_err(|error| { + app_handle.path().app_local_data_dir().map_err(|error| { format!("failed to resolve app local data directory: {error}") })?, - }), + ), StorageMode::Portable { root } => { let data_dir = root.join(PORTABLE_DATA_DIR); - Ok(Self { - mode: StorageMode::Portable { root }, - log_dir: data_dir.join(PORTABLE_LOG_DIR), - webview_dir: data_dir.join(PORTABLE_WEBVIEW_DIR), - data_dir, - }) + ( + StorageMode::Portable { root }, + data_dir.clone(), + data_dir.join(PORTABLE_LOG_DIR), + data_dir.join(PORTABLE_WEBVIEW_DIR), + ) } - } + }; + + Ok(Self { + mode, + data_dir: canonicalize_storage_path(&data_dir)?, + log_dir: canonicalize_storage_path(&log_dir)?, + webview_dir: canonicalize_storage_path(&webview_dir)?, + }) } pub fn is_portable(&self) -> bool { @@ -99,10 +106,57 @@ impl StorageLayout { } } +fn canonicalize_storage_path(path: &Path) -> Result { + if crate::path_has_symlink_component(path) { + return Err(format!( + "storage path contains a symlinked component: '{}'", + path.display() + )); + } + let mut existing = path; + let mut missing = Vec::new(); + loop { + match std::fs::symlink_metadata(existing) { + Ok(metadata) => { + if metadata.file_type().is_symlink() { + return Err(format!( + "storage path contains a symlinked directory: '{}'", + path.display() + )); + } + break; + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(format!( + "failed to inspect storage path '{}': {error}", + path.display() + )); + } + } + missing.push( + existing + .file_name() + .ok_or_else(|| format!("storage path has no existing ancestor: '{}'", path.display()))? + .to_owned(), + ); + existing = existing + .parent() + .ok_or_else(|| format!("storage path has no existing ancestor: '{}'", path.display()))?; + } + let mut canonical = std::fs::canonicalize(existing) + .map_err(|error| format!("failed to canonicalize storage path '{}': {error}", path.display()))?; + for component in missing.iter().rev() { + canonical.push(component); + } + Ok(canonical) +} + #[cfg(test)] mod tests { - use super::{StorageMode, PORTABLE_MARKER}; + use super::{canonicalize_storage_path, StorageMode, PORTABLE_MARKER}; use std::fs; + use std::path::Path; use tempfile::TempDir; #[test] @@ -127,4 +181,31 @@ mod tests { StorageMode::Standard ); } + + #[cfg(unix)] + #[test] + fn rejects_symlinked_storage_directories() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + let target = TempDir::new().unwrap(); + let root_path = fs::canonicalize(root.path()).unwrap(); + let redirected = root_path.join("logs"); + symlink(target.path(), &redirected).unwrap(); + + assert!(canonicalize_storage_path(Path::new(&redirected)).is_err()); + } + + #[cfg(unix)] + #[test] + fn rejects_dangling_symlinked_storage_directories() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + let root_path = fs::canonicalize(root.path()).unwrap(); + let redirected = root_path.join("logs"); + symlink(root_path.join("missing-target"), &redirected).unwrap(); + + assert!(canonicalize_storage_path(Path::new(&redirected)).is_err()); + } } diff --git a/src/bindings/PersistedSettings.ts b/src/bindings/PersistedSettings.ts index fad251c..095e12d 100644 --- a/src/bindings/PersistedSettings.ts +++ b/src/bindings/PersistedSettings.ts @@ -8,4 +8,4 @@ import type { SettingsTab } from "./SettingsTab"; import type { SiteLogin } from "./SiteLogin"; import type { Theme } from "./Theme"; -export type PersistedSettings = { theme: Theme, baseDownloadFolder: string, categorySubfoldersEnabled: boolean, categorySubfolders: { [key in string]: string }, categoryDirectoryOverrides: { [key in string]: string }, approvedDownloadRoots: Array, maxConcurrentDownloads: number, globalSpeedLimit: string, speedLimitPresetValues: Array, logsEnabled: boolean, isSidebarVisible: boolean, activeSettingsTab: SettingsTab, scheduler: SchedulerSettings, schedulerRunning: boolean, schedulerActiveDownloadIds: Array, schedulerLastStartKey: string, schedulerLastStopKey: string, lastCustomSpeedLimitKiB: number, perServerConnections: number, maxAutomaticRetries: number, showNotifications: boolean, playCompletionSound: boolean, appFontSize: AppFontSize, listRowDensity: ListRowDensity, showDockBadge: boolean, showMenuBarIcon: boolean, proxyMode: ProxyMode, proxyHost: string, proxyPort: number, customUserAgent: string, askWhereToSaveEachFile: boolean, preventsSleepWhileDownloading: boolean, mediaCookieSource: MediaCookieSource, siteLogins: Array, extensionPairingToken: string, autoCheckUpdates: boolean, keychainAccessGranted: boolean, }; +export type PersistedSettings = { theme: Theme, baseDownloadFolder: string, categorySubfoldersEnabled: boolean, categorySubfolders: { [key in string]: string }, categoryDirectoryOverrides: { [key in string]: string }, approvedDownloadRoots: Array, maxConcurrentDownloads: number, globalSpeedLimit: string, speedLimitPresetValues: Array, logsEnabled: boolean, isSidebarVisible: boolean, activeSettingsTab: SettingsTab, scheduler: SchedulerSettings, schedulerRunning: boolean, schedulerActiveDownloadIds: Array, schedulerLastStartKey: string, schedulerLastStopKey: string, lastCustomSpeedLimitKiB: number, perServerConnections: number, maxAutomaticRetries: number, showNotifications: boolean, playCompletionSound: boolean, appFontSize: AppFontSize, listRowDensity: ListRowDensity, showDockBadge: boolean, showMenuBarIcon: boolean, proxyMode: ProxyMode, proxyHost: string, proxyPort: number, customUserAgent: string, askWhereToSaveEachFile: boolean, preventsSleepWhileDownloading: boolean, mediaCookieSource: MediaCookieSource, siteLogins: Array, autoCheckUpdates: boolean, keychainAccessGranted: boolean, }; diff --git a/src/components/SettingsView.tsx b/src/components/SettingsView.tsx index 190fe1c..c351b48 100644 --- a/src/components/SettingsView.tsx +++ b/src/components/SettingsView.tsx @@ -1258,7 +1258,7 @@ className="app-button px-3 py-1.5 text-[12px] flex items-center gap-1.5 disabled

{platform.portable ? 'Your pairing token is stored with this portable Firelink folder and will persist when the folder is moved. Treat the folder as sensitive.' - : 'Your pairing token is persisted in Firelink settings and will persist across restarts.'} + : 'Your pairing token is stored in the system credential store and will persist across restarts.'}

diff --git a/src/ipc.ts b/src/ipc.ts index cc585f0..4e25bb3 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -54,6 +54,7 @@ type CommandMap = { set_extension_pairing_token: { args: { token: string }; result: void }; get_extension_server_port: { args: undefined; result: number | null }; hydrate_extension_pairing_token: { args: undefined; result: PairingTokenHydration }; + regenerate_pairing_token: { args: undefined; result: PairingTokenHydration }; grant_keychain_access: { args: undefined; result: PairingTokenHydration }; acknowledge_pairing_token_change: { args: undefined; result: void }; set_extension_frontend_ready: { args: { ready: boolean }; result: void }; diff --git a/src/store/useSettingsStore.ts b/src/store/useSettingsStore.ts index 678cd3a..a46a782 100644 --- a/src/store/useSettingsStore.ts +++ b/src/store/useSettingsStore.ts @@ -102,8 +102,6 @@ const tauriStorage: StateStorage = { * explicit exception: its pairing token is persisted with the portable folder * so extension pairing follows that folder. */ -const PAIRING_TOKEN_KEYCHAIN_ID = 'extension-pairing-token'; - export type { ActiveView, AppFontSize, @@ -207,32 +205,6 @@ export interface SettingsState { dismissKeychainPrompt: () => void; } -const generateSecureToken = () => { - try { - const cryptoObj = typeof window !== 'undefined' - ? (window as Window & { msCrypto?: Crypto }).crypto - || (window as Window & { msCrypto?: Crypto }).msCrypto - : null; - if (cryptoObj && cryptoObj.getRandomValues) { - const arr = new Uint8Array(24); - cryptoObj.getRandomValues(arr); - let binary = ''; - for (let i = 0; i < arr.byteLength; i++) { - binary += String.fromCharCode(arr[i]); - } - return btoa(binary); - } - } catch (e) { - console.warn("Secure token generation failed, falling back to random characters", e); - } - const chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/'; - let token = ''; - for (let i = 0; i < 32; i++) { - token += chars.charAt(Math.floor(Math.random() * chars.length)); - } - return token; -}; - export const useSettingsStore = create()( persist( (set, get) => ({ @@ -390,14 +362,20 @@ export const useSettingsStore = create()( siteLogins: state.siteLogins.filter((login) => login.id !== id) })), regeneratePairingToken: async () => { - const token = generateSecureToken(); - await invoke('set_keychain_password', { id: PAIRING_TOKEN_KEYCHAIN_ID, password: token }); - set({ extensionPairingToken: token }); + const result = await invoke('regenerate_pairing_token'); + if (!result.persistent) { + throw new Error(result.error || 'Credential store access is unavailable.'); + } + set({ + extensionPairingToken: result.token, + isPairingTokenPersistent: true, + showKeychainModal: false + }); }, hydratePairingToken: async () => { - // Always use the safe hydration path that never touches the OS keychain - // on its own. The modal will be shown when needed and only the explicit - // "Grant Access" action (→ grant_keychain_access) triggers the OS prompt. + // The backend migrates legacy settings copies and reads the token from + // the credential store after the app state is ready to receive it. + // Portable mode remains the explicit folder-contained exception. const result = await invoke('hydrate_extension_pairing_token'); set({ extensionPairingToken: result.token, @@ -486,7 +464,6 @@ export const useSettingsStore = create()( preventsSleepWhileDownloading: state.preventsSleepWhileDownloading, mediaCookieSource: state.mediaCookieSource, siteLogins: state.siteLogins, - extensionPairingToken: state.extensionPairingToken, keychainAccessGranted: state.keychainAccessGranted, keychainPromptDismissed: state.keychainPromptDismissed, autoCheckUpdates: state.autoCheckUpdates @@ -500,6 +477,7 @@ export const useSettingsStore = create()( ...currentState, ...persisted, ...locations, + extensionPairingToken: currentState.extensionPairingToken, theme: isAllowedSetting(THEME_VALUES, persisted.theme) ? persisted.theme : currentState.theme, @@ -518,9 +496,6 @@ export const useSettingsStore = create()( activeSettingsTab: isAllowedSetting(SETTINGS_TAB_VALUES, persisted.activeSettingsTab) ? persisted.activeSettingsTab : currentState.activeSettingsTab, - extensionPairingToken: typeof persisted.extensionPairingToken === 'string' - ? persisted.extensionPairingToken - : currentState.extensionPairingToken, showNotifications: persistedBoolean(persisted.showNotifications, currentState.showNotifications), playCompletionSound: persistedBoolean(persisted.playCompletionSound, currentState.playCompletionSound), showDockBadge: persistedBoolean(persisted.showDockBadge, currentState.showDockBadge),