fix(properties): harden lifecycle and session fencing

- Fence Properties actions and Torrent moves by current caller sessions.
- Preserve move progress and authoritative destinations across stale events and recovery.
- Enforce immutable identity fields and transactional queued-edit rejection.
- Restore subtle theme surfaces and expand regression coverage.
This commit is contained in:
NimBold
2026-08-12 07:07:52 +03:30
parent f7bafdeb0e
commit 64a836f09f
20 changed files with 488 additions and 115 deletions
+15
View File
@@ -807,6 +807,9 @@ pub struct DownloadStateEvent {
pub resolver_fallback: Option<bool>,
#[ts(optional)]
pub file_name: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub destination: Option<String>,
#[ts(optional)]
pub torrent_seed_remaining: Option<f64>,
}
@@ -820,6 +823,7 @@ impl DownloadStateEvent {
error_kind: None,
resolver_fallback: None,
file_name: None,
destination: None,
torrent_seed_remaining: None,
}
}
@@ -833,6 +837,7 @@ impl DownloadStateEvent {
error_kind,
resolver_fallback: None,
file_name: None,
destination: None,
torrent_seed_remaining: None,
}
}
@@ -846,6 +851,7 @@ impl DownloadStateEvent {
error_kind,
resolver_fallback: None,
file_name: None,
destination: None,
torrent_seed_remaining: None,
}
}
@@ -858,6 +864,7 @@ impl DownloadStateEvent {
error_kind: None,
resolver_fallback: None,
file_name: None,
destination: None,
torrent_seed_remaining: remaining,
}
}
@@ -870,6 +877,7 @@ impl DownloadStateEvent {
error_kind: None,
resolver_fallback: None,
file_name: Some(file_name.into()),
destination: None,
torrent_seed_remaining: None,
}
}
@@ -885,6 +893,7 @@ impl DownloadStateEvent {
error_kind,
resolver_fallback: None,
file_name: None,
destination: None,
torrent_seed_remaining: None,
}
}
@@ -897,6 +906,7 @@ impl DownloadStateEvent {
error_kind: None,
resolver_fallback: None,
file_name: None,
destination: None,
torrent_seed_remaining: remaining,
}
}
@@ -910,6 +920,11 @@ impl DownloadStateEvent {
event
}
pub fn with_destination(mut self, destination: impl Into<String>) -> Self {
self.destination = Some(destination.into());
self
}
fn safe_error(error: impl Into<String>) -> (String, Option<DownloadErrorKind>) {
let error = crate::redact_sensitive_text(&error.into());
let error_kind = crate::retry::is_aria2_name_resolution_error(&error)
+84 -36
View File
@@ -8426,9 +8426,24 @@ async fn move_torrent_data(
database: tauri::State<'_, crate::db::DbState>,
id: String,
destination: String,
session_id: Option<String>,
) -> Result<(), String> {
properties_window::ensure_properties_or_main(&caller, &properties, &id)?;
let properties_session_id = if caller.label() == "main" {
None
} else {
let session_id = session_id.ok_or_else(|| "Properties window session is required".to_string())?;
if !properties.session_matches(caller.label(), &session_id)? {
return Err("Properties window session is no longer current".to_string());
}
Some(session_id)
};
let control_guard = state.queue_manager.acquire_aria2_control(&id).await;
if let Some(session_id) = properties_session_id.as_deref() {
if !properties.session_matches(caller.label(), session_id)? {
return Err("Properties window session is no longer current".to_string());
}
}
let item = load_persisted_torrent_item(database.inner(), &id)?;
if item.is_torrent != Some(true) {
return Err("data relocation is available only for Torrent downloads".to_string());
@@ -8576,7 +8591,14 @@ async fn move_torrent_data(
.await
.map_err(|_| "could not prepare Torrent move recovery".to_string())?;
}
state.queue_manager.begin_torrent_move(&id).await;
if let Some(session_id) = properties_session_id.as_deref() {
properties.with_current_session(caller.label(), session_id, || {
state.queue_manager.begin_torrent_move(&id);
Ok(())
})?;
} else {
state.queue_manager.begin_torrent_move(&id);
}
if let Err(error) = write_torrent_move_journal(
&journal,
"reserved",
@@ -8595,12 +8617,12 @@ async fn move_torrent_data(
)
.await
{
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
return Err(error);
}
if let Err(error) = tokio::fs::create_dir(&staging_root).await {
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
return Err(format!("could not prepare Torrent move staging: {error}"));
}
if let Err(error) = crate::download_ownership::set_owned_paths_with_primary_and_removal(
@@ -8611,28 +8633,32 @@ async fn move_torrent_data(
&new_removal_paths,
) {
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
return Err(error);
}
use tauri::Emitter;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, crate::ipc::DownloadStatus::Moving));
let move_restore_event = || {
crate::ipc::DownloadStateEvent::new(&id, item.status)
.with_destination(old_destination.to_string_lossy())
};
let mut copied_bytes = 0u64;
for (source, target) in move_old_paths.iter().zip(staging_paths.iter()) {
if state.queue_manager.torrent_move_cancelled(&id).await {
if state.queue_manager.torrent_move_cancelled(&id) {
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit("download-state", move_restore_event());
return Err("Torrent move canceled".to_string());
}
if let Err(error) = copy_torrent_move_file(source, target).await {
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit("download-state", move_restore_event());
return Err(error);
}
let source_size = match tokio::fs::metadata(source).await {
@@ -8641,10 +8667,10 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
move_restore_event(),
);
return Err("Torrent source changed during relocation".to_string());
}
@@ -8655,10 +8681,10 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
move_restore_event(),
);
return Err("Torrent destination could not be verified".to_string());
}
@@ -8667,7 +8693,7 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
let _ = app_handle.emit("download-state", move_restore_event());
return Err("Torrent data changed during relocation".to_string());
}
let source_digest = digest_torrent_move_file(source).await;
@@ -8676,7 +8702,7 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
let _ = app_handle.emit("download-state", move_restore_event());
return Err("Torrent data changed during relocation".to_string());
}
copied_bytes = copied_bytes.saturating_add(target_size);
@@ -8687,12 +8713,12 @@ async fn move_torrent_data(
total_bytes,
});
}
if state.queue_manager.torrent_move_cancelled(&id).await {
if state.queue_manager.torrent_move_cancelled(&id) {
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit("download-state", move_restore_event());
return Err("Torrent move canceled".to_string());
}
for (staged, target) in staging_paths.iter().zip(move_new_paths.iter()) {
@@ -8700,10 +8726,10 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
move_restore_event(),
);
return Err("Torrent move destination could not be published".to_string());
}
@@ -8712,10 +8738,10 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
move_restore_event(),
);
return Err("Torrent move staging could not be finalized".to_string());
}
@@ -8740,10 +8766,10 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
move_restore_event(),
);
return Err(error);
}
@@ -8761,8 +8787,8 @@ async fn move_torrent_data(
cleanup_torrent_move_outputs(&move_new_paths, &staging_paths, &staging_root).await;
let _ = restore_download_ownership(&app_handle, &id, ownership.clone());
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit("download-state", move_restore_event());
return Err(error);
}
if let Err(error) = write_torrent_move_journal(
@@ -8787,10 +8813,11 @@ async fn move_torrent_data(
// let startup recovery finish old-source cleanup from the committed
// destination rather than rolling the row back after commit.
let _ = persist_torrent_relocation_check(database.inner(), &id, true);
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
crate::ipc::DownloadStateEvent::new(&id, item.status)
.with_destination(new_destination.to_string_lossy()),
);
return Err(format!("Torrent data moved; cleanup recovery remains pending: {error}"));
}
@@ -8828,23 +8855,32 @@ async fn move_torrent_data(
&move_new_paths,
total_bytes,
).await {
state.queue_manager.finish_torrent_move(&id).await;
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status),
crate::ipc::DownloadStateEvent::new(&id, item.status)
.with_destination(new_destination.to_string_lossy()),
);
return Err(format!(
"Torrent data moved, but cleanup recovery could not be recorded: {journal_error}"
));
}
state.queue_manager.finish_torrent_move(&id).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status)
.with_destination(new_destination.to_string_lossy()),
);
drop(control_guard);
return Err(format!("Torrent data moved, but old files need cleanup: {error}"));
}
let _ = tokio::fs::remove_file(&journal).await;
state.queue_manager.finish_torrent_move(&id).await;
let _ = app_handle.emit("download-state", crate::ipc::DownloadStateEvent::new(&id, item.status));
state.queue_manager.finish_torrent_move(&id);
let _ = app_handle.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(&id, item.status)
.with_destination(new_destination.to_string_lossy()),
);
drop(control_guard);
Ok(())
}
@@ -8852,14 +8888,26 @@ async fn move_torrent_data(
#[tauri::command]
async fn cancel_torrent_move_data(
caller: tauri::WebviewWindow,
properties: tauri::State<'_, properties_window::PropertiesWindowRegistry>,
state: tauri::State<'_, AppState>,
id: String,
session_id: Option<String>,
) -> Result<(), String> {
properties_window::ensure_main_window(&caller)?;
if id.trim().is_empty() {
return Err("invalid Torrent download id".to_string());
}
state.queue_manager.cancel_torrent_move(&id).await;
if caller.label() == "main" {
properties_window::ensure_main_window(&caller)?;
} else {
let session_id = session_id.ok_or_else(|| "Properties window session is required".to_string())?;
properties_window::ensure_properties_or_main(&caller, &properties, &id)?;
properties.with_current_session(caller.label(), &session_id, || {
state.queue_manager.cancel_torrent_move(&id);
Ok(())
})?;
return Ok(());
}
state.queue_manager.cancel_torrent_move(&id);
Ok(())
}
+50
View File
@@ -238,6 +238,26 @@ impl PropertiesWindowRegistry {
Ok(self.session_for_window(label)?.as_deref() == Some(session_id))
}
/// Validate a session and perform a short synchronous mutation while the
/// registry lock is held. Callers use this for cancellation flags so a
/// stale session cannot pass validation and then race a replacement
/// session before its mutation is recorded.
pub fn with_current_session<T>(
&self,
label: &str,
session_id: &str,
mutation: impl FnOnce() -> Result<T, String>,
) -> Result<T, String> {
let state = self
.state
.lock()
.map_err(|_| "Properties window registry is unavailable".to_string())?;
if state.sessions_by_window.get(label).map(String::as_str) != Some(session_id) {
return Err("Properties window session is no longer current".to_string());
}
mutation()
}
#[cfg(test)]
pub fn is_ready(&self, label: &str) -> Result<bool, String> {
Ok(self
@@ -478,8 +498,16 @@ pub fn properties_window_send_ready(
pub fn properties_window_reveal(
caller: tauri::WebviewWindow,
registry: tauri::State<'_, PropertiesWindowRegistry>,
session_id: Option<String>,
) -> Result<(), String> {
registered_download_for_caller(&caller, &registry)?;
if caller.label() != MAIN_WINDOW_LABEL {
let session_id = session_id.ok_or_else(|| "Properties window session is required".to_string())?;
validate_properties_session_id(&session_id)?;
if !registry.session_matches(caller.label(), &session_id)? {
return Err("Properties window session is no longer current".to_string());
}
}
registry.mark_ready(caller.label())?;
caller.show().map_err(|error| error.to_string())?;
caller.set_focus().map_err(|error| error.to_string())
@@ -722,6 +750,28 @@ mod tests {
assert!(!registry.session_matches(&label, "session-new").unwrap());
}
#[test]
fn current_session_mutation_is_fenced_from_retired_sessions() {
let registry = PropertiesWindowRegistry::default();
let label = registry.allocate("download-a").unwrap();
registry.register_session(&label, "session-old").unwrap();
let mut mutations = 0;
let stale = registry.with_current_session(&label, "session-old", || {
mutations += 1;
Ok(())
});
assert!(stale.is_ok());
registry.register_session(&label, "session-new").unwrap();
let rejected = registry.with_current_session(&label, "session-old", || {
mutations += 1;
Ok(())
});
assert!(rejected.is_err());
assert_eq!(mutations, 1);
}
#[test]
fn child_actions_are_allowlisted() {
assert!(is_properties_action("apply-properties"));
+21 -12
View File
@@ -942,7 +942,7 @@ pub struct QueueManager<R: tauri::Runtime = tauri::Wry> {
/// are scoped to the current GID and control epoch and never leave this
/// process as durable state.
torrent_telemetry: Mutex<HashMap<String, TorrentTelemetryState>>,
torrent_move_cancellations: Mutex<HashSet<String>>,
torrent_move_cancellations: StdMutex<HashSet<String>>,
/// aria2 gid -> download id map (shared with the WS poller).
pub aria2_gids: Arc<std::sync::RwLock<HashMap<String, Aria2GidMapping>>>,
@@ -1045,7 +1045,7 @@ impl<R: tauri::Runtime> QueueManager<R> {
}),
seed_budgets: StdMutex::new(HashMap::new()),
torrent_telemetry: Mutex::new(HashMap::new()),
torrent_move_cancellations: Mutex::new(HashSet::new()),
torrent_move_cancellations: StdMutex::new(HashSet::new()),
aria2_gids: Arc::new(std::sync::RwLock::new(HashMap::new())),
pending_completion: Arc::new(Mutex::new(HashMap::new())),
aria2_payloads: Mutex::new(HashMap::new()),
@@ -1135,23 +1135,32 @@ impl<R: tauri::Runtime> QueueManager<R> {
true
}
pub async fn begin_torrent_move(&self, id: &str) {
self.torrent_move_cancellations.lock().await.remove(id);
}
pub async fn cancel_torrent_move(&self, id: &str) {
pub fn begin_torrent_move(&self, id: &str) {
self.torrent_move_cancellations
.lock()
.await
.expect("Torrent move cancellation lock poisoned")
.remove(id);
}
pub fn cancel_torrent_move(&self, id: &str) {
self.torrent_move_cancellations
.lock()
.expect("Torrent move cancellation lock poisoned")
.insert(id.to_string());
}
pub async fn torrent_move_cancelled(&self, id: &str) -> bool {
self.torrent_move_cancellations.lock().await.contains(id)
pub fn torrent_move_cancelled(&self, id: &str) -> bool {
self.torrent_move_cancellations
.lock()
.expect("Torrent move cancellation lock poisoned")
.contains(id)
}
pub async fn finish_torrent_move(&self, id: &str) {
self.torrent_move_cancellations.lock().await.remove(id);
pub fn finish_torrent_move(&self, id: &str) {
self.torrent_move_cancellations
.lock()
.expect("Torrent move cancellation lock poisoned")
.remove(id);
}
/// Drop counters after terminal cleanup/removal. Persisted lifetime