diff --git a/crates/lanspread-peer/ARCHITECTURE.md b/crates/lanspread-peer/ARCHITECTURE.md index 0d03fb9..13952a8 100644 --- a/crates/lanspread-peer/ARCHITECTURE.md +++ b/crates/lanspread-peer/ARCHITECTURE.md @@ -171,12 +171,16 @@ Downloaded and installed are independent predicates: Reserved per-game paths: - `.version.ini.tmp` and `.version.ini.discarded` are download transaction - scratch files and are swept during startup recovery. + scratch files. Startup recovery interprets them together with the durable + ownership journal rather than sweeping them independently. - `.local.installing/` is extraction staging. - `.local.backup/` holds the previous install while an update or uninstall is in flight. - `games//install_intent.json` in the configured state directory is the atomic per-game intent log. +- `games//download_ownership.json` in that state directory records the + last committed and any pending downloader-owned regular-file set. The record + is bound to one canonical configured games directory. - `.lanspread_owned` inside `.local.*` directories proves Lanspread ownership when the current intent is `None`. @@ -210,9 +214,10 @@ Most scans become O(number of game dirs), with full recursion only when needed. active for that ID, and the root-level `version.ini` sentinel exists. - `local/` paths are never served, even if a stale or malicious manifest request asks for them. -- Cancelling a download discards the peer-owned root download payload and - scratch sentinel files. `local/` and install transaction metadata are - preserved, so a cancelled update of an installed game settles as local-only. +- Cancelling or recovering a download removes only paths named by its durable + ownership journal and its validated pending manifest. Unknown user files, + `local/`, and install transaction metadata are preserved, so a cancelled + update of an installed game settles as local-only. ### Streamed install integrity diff --git a/crates/lanspread-peer/README.md b/crates/lanspread-peer/README.md index 44d60c4..b6b7aef 100644 --- a/crates/lanspread-peer/README.md +++ b/crates/lanspread-peer/README.md @@ -65,11 +65,14 @@ When the UI asks to download a game: archives. The selected peers are queried via `request_game_details_from_peer`, and their file manifests are merged inside `PeerGameDB`. -2. Once the UI receives `PeerEvent::GotGameFiles`, it forwards the selected file - list back with `PeerCommand::DownloadGameFiles`. -3. `download_game_files` starts a version-sentinel transaction, parks any old - `version.ini` as `.version.ini.discarded`, prepares non-sentinel files, emits - `PeerEvent::DownloadGameFilesBegin`, and builds a per-peer plan +2. Once the UI receives `PeerEvent::GotGameFiles`, it requests the download by + game ID only. The peer core validates every source manifest before consensus, + chooses the complete authoritative description, and constructs one + root-confined `ValidatedDownloadManifest` before any filesystem mutation. +3. `download_game_files` recovers any earlier attempt, parks an old + `version.ini` as `.version.ini.discarded`, and durably journals the exact old + and proposed downloader-owned file sets before preparing non-sentinel files. + It then emits `PeerEvent::DownloadGameFilesBegin` and builds a per-peer plan (`build_peer_plans`) that round-robins file chunks across the available peers that advertise the latest version. 4. Each plan is executed in its own task (`download_from_peer`). Chunk requests @@ -80,13 +83,20 @@ When the UI asks to download a game: 5. `DownloadProgressTracker` samples byte counters, transfer speed, and the number of unique peers that are actively streaming chunks. The Tauri UI sees those values together through the regular download-progress event. -6. `version.ini` chunks are buffered in memory and committed last via - `.version.ini.tmp` followed by an atomic rename. Failures are accumulated and - retried (up to `MAX_RETRY_COUNT`) via `retry_failed_chunks`; failed downloads - sweep `.version.ini.tmp` and `.version.ini.discarded` without restoring the - previous sentinel. Cancelled downloads also discard the peer-owned download - payload while preserving `local/` and install transaction metadata. -7. After a successful sentinel commit, `PeerEvent::DownloadGameFilesFinished` is +6. `version.ini` chunks are buffered in memory. After transfer, paths owned by + the previous successful download but absent from the new manifest are + removed. Payload files and their directories are synced before the new + sentinel is committed last via `.version.ini.tmp` followed by an atomic + rename. A sentinel rename whose directory sync fails leaves ownership pending + for recovery instead of being reported as a durable success. Transfer + failures are accumulated and retried (up to `MAX_RETRY_COUNT`) via + `retry_failed_chunks`. +7. Failure, cancellation, and startup recovery use the journal to remove only + exact downloader-owned files. Unknown user files, `local/`, and install + transaction state are preserved. A regular `version.ini` beside a pending + journal proves that the final rename landed; otherwise recovery aborts the + incomplete payload. +8. After a successful sentinel commit, `PeerEvent::DownloadGameFilesFinished` is emitted and the peer auto-runs the install transaction. ### Streamed Install Pipeline @@ -129,6 +139,11 @@ that are direct children of the configured game directory, refuses installed or in-flight roots, and deletes the whole game root only after finding a regular root-level `version.ini` sentinel. +Download provenance is stored separately at +`games//download_ownership.json` in the peer state directory. It is +bound to the canonical configured games directory so switching library roots +cannot make an old record authorize deletion in a different tree. + Legacy launcher-owned files in game directories are migrated by a dedicated pre-start phase. Normal install, recovery, scan, and transfer paths use only the configured state directory for launcher-owned metadata. diff --git a/crates/lanspread-peer/src/download/manifest.rs b/crates/lanspread-peer/src/download/manifest.rs index 8d304b2..58c833b 100644 --- a/crates/lanspread-peer/src/download/manifest.rs +++ b/crates/lanspread-peer/src/download/manifest.rs @@ -137,6 +137,28 @@ impl ValidatedDownloadManifest { .map(|entry| entry.protocol_description(&self.game_id)) .collect() } + + pub(super) fn owned_file_paths(&self) -> Vec { + self.transfer_entries() + .filter(|entry| !entry.is_dir()) + .map(|entry| entry.canonical_path.clone()) + .collect() + } +} + +/// Revalidates one root-relative regular-file path loaded from ownership state. +pub(super) fn validate_owned_file_path(path: &str) -> eyre::Result { + let (alias, components) = validate_canonical_path(path)?; + let root = components + .first() + .expect("validated canonical paths have one component"); + if components.len() == 1 && portable_name_key(root) == portable_name_key(VERSION_INI) { + eyre::bail!("download ownership must not include the version.ini sentinel"); + } + if is_download_protected_root_name(root) { + eyre::bail!("download ownership targets application state: {path}"); + } + Ok(alias) } /// Validates one peer's complete current-wire description before aggregation. diff --git a/crates/lanspread-peer/src/download/mod.rs b/crates/lanspread-peer/src/download/mod.rs index 5b6927c..437324e 100644 --- a/crates/lanspread-peer/src/download/mod.rs +++ b/crates/lanspread-peer/src/download/mod.rs @@ -2,6 +2,7 @@ mod manifest; mod orchestrator; +mod ownership; mod planning; mod progress; mod retry; @@ -11,3 +12,4 @@ mod version_ini; pub(crate) use manifest::{ValidatedDownloadManifest, validate_protocol_v7_descriptions}; pub(crate) use orchestrator::download_game_files; +pub(crate) use ownership::{clear_download_ownership, recover_incomplete_download}; diff --git a/crates/lanspread-peer/src/download/orchestrator.rs b/crates/lanspread-peer/src/download/orchestrator.rs index 6417372..eb68402 100644 --- a/crates/lanspread-peer/src/download/orchestrator.rs +++ b/crates/lanspread-peer/src/download/orchestrator.rs @@ -6,16 +6,18 @@ use tokio_util::sync::CancellationToken; use super::{ manifest::ValidatedDownloadManifest, + ownership::DownloadOwnershipTransaction, planning::{ChunkDownloadResult, DownloadChunk, build_peer_plans, extract_version_descriptor}, progress::{DownloadProgressTracker, sample_download_progress}, retry::{RetryContext, retry_failed_chunks}, - storage::{discard_cancelled_download, prepare_game_storage}, + storage::{prepare_game_storage, sync_game_storage}, transport::download_from_peer, version_ini::{ VersionIniBuffer, + VersionIniCommit, begin_version_ini_transaction, commit_version_ini_buffer, - rollback_version_ini_transaction, + restore_unjournaled_version_ini_transaction, }, }; use crate::{PeerEvent, config::MAX_RETRY_COUNT}; @@ -24,6 +26,7 @@ use crate::{PeerEvent, config::MAX_RETRY_COUNT}; #[allow(clippy::too_many_lines)] pub(crate) async fn download_game_files( manifest: ValidatedDownloadManifest, + state_dir: &Path, peers: Vec, file_peer_map: HashMap>, tx_notify_ui: UnboundedSender, @@ -46,30 +49,46 @@ pub(crate) async fn download_game_files( Err(err) => return Err(err), }; let game_root = manifest.game_root().to_path_buf(); + let ownership = DownloadOwnershipTransaction::prepare(state_dir, &manifest).await?; - begin_version_ini_transaction(&game_root).await?; + if let Err(error) = begin_version_ini_transaction(&game_root).await { + if let Err(restore_error) = restore_before_ownership_journal(&game_root).await { + return Err(error.wrap_err(format!( + "sentinel parking failed and rollback also failed: {restore_error}" + ))); + } + return Err(error); + } if cancel_token.is_cancelled() { - rollback_version_ini_transaction(&game_root).await; - discard_cancelled_download_best_effort(&games_folder, &game_id).await; + restore_before_ownership_journal(&game_root).await?; eyre::bail!("download cancelled for game {game_id}"); } + if let Err(error) = ownership.journal_pending().await { + if let Err(restore_error) = restore_before_ownership_journal(&game_root).await { + return Err(error.wrap_err(format!( + "ownership journal failed and sentinel restore also failed: {restore_error}" + ))); + } + return Err(error); + } if let Err(err) = prepare_game_storage(&manifest).await { - rollback_version_ini_transaction(&game_root).await; + abort_download_best_effort(&ownership, &game_id).await; if cancel_token.is_cancelled() { - discard_cancelled_download_best_effort(&games_folder, &game_id).await; eyre::bail!("download cancelled for game {game_id}"); } return Err(err); } if cancel_token.is_cancelled() { - rollback_version_ini_transaction(&game_root).await; - discard_cancelled_download_best_effort(&games_folder, &game_id).await; + abort_download_best_effort(&ownership, &game_id).await; eyre::bail!("download cancelled for game {game_id}"); } - tx_notify_ui.send(PeerEvent::DownloadGameFilesBegin { + if let Err(error) = tx_notify_ui.send(PeerEvent::DownloadGameFilesBegin { id: game_id.clone(), - })?; + }) { + abort_download_best_effort(&ownership, &game_id).await; + return Err(error.into()); + } let progress_tracker = DownloadProgressTracker::new(total_download_bytes(&transfer_descs)); let transfer_ctx = TransferContext { @@ -91,30 +110,63 @@ pub(crate) async fn download_game_files( .await; if let Err(err) = transfer_result { - rollback_version_ini_transaction(&game_root).await; - if cancel_token.is_cancelled() { - discard_cancelled_download_best_effort(&games_folder, &game_id).await; - } + abort_download_best_effort(&ownership, &game_id).await; return Err(err); } if cancel_token.is_cancelled() { - rollback_version_ini_transaction(&game_root).await; - discard_cancelled_download_best_effort(&games_folder, &game_id).await; + abort_download_best_effort(&ownership, &game_id).await; eyre::bail!("download cancelled for game {game_id}"); } - if let Err(err) = commit_version_ini_buffer(&game_root, &version_buffer).await { - rollback_version_ini_transaction(&game_root).await; - return Err(err); + if let Err(error) = sync_game_storage(&manifest).await { + abort_download_best_effort(&ownership, &game_id).await; + return Err(error.wrap_err("failed to make downloaded payload durable")); + } + if cancel_token.is_cancelled() { + abort_download_best_effort(&ownership, &game_id).await; + eyre::bail!("download cancelled for game {game_id}"); + } + + if let Err(error) = ownership.remove_stale().await { + abort_download_best_effort(&ownership, &game_id).await; + return Err(error.wrap_err("failed to remove stale download-owned files")); + } + if cancel_token.is_cancelled() { + abort_download_best_effort(&ownership, &game_id).await; + eyre::bail!("download cancelled for game {game_id}"); + } + + match commit_version_ini_buffer(&game_root, &version_buffer).await { + Ok(VersionIniCommit::Durable) => {} + Ok(VersionIniCommit::NeedsRecovery(error)) => { + // The visible sentinel makes rollback unsafe. Keep pending ownership + // so startup recovery can decide from the durable filesystem state. + return Err(eyre::eyre!( + "version.ini was renamed but its durability could not be established: {error}" + )); + } + Err(error) => { + abort_download_best_effort(&ownership, &game_id).await; + return Err(error); + } + } + if let Err(error) = ownership.finalize().await { + // The sentinel rename is the commit point. Pending ownership lets the + // next recovery or download finish this idempotently. + log::error!("Downloaded {game_id}, but ownership finalization must be recovered: {error}"); } log::info!("all files downloaded for game: {game_id}"); Ok(()) } -async fn discard_cancelled_download_best_effort(games_folder: &Path, game_id: &str) { - if let Err(err) = discard_cancelled_download(games_folder, game_id).await { - log::warn!("Failed to discard cancelled download payload for {game_id}: {err}"); +async fn restore_before_ownership_journal(game_root: &Path) -> eyre::Result<()> { + restore_unjournaled_version_ini_transaction(game_root).await +} + +async fn abort_download_best_effort(ownership: &DownloadOwnershipTransaction, game_id: &str) { + if let Err(err) = ownership.abort().await { + log::warn!("Failed to abort download-owned payload for {game_id}: {err}"); } } diff --git a/crates/lanspread-peer/src/download/ownership.rs b/crates/lanspread-peer/src/download/ownership.rs new file mode 100644 index 0000000..282bd60 --- /dev/null +++ b/crates/lanspread-peer/src/download/ownership.rs @@ -0,0 +1,978 @@ +//! Crash-consistent provenance for files created by peer downloads. + +use std::{ + collections::BTreeSet, + io::ErrorKind, + path::{Path, PathBuf}, +}; + +use serde::{Deserialize, Serialize}; +use tokio::io::AsyncWriteExt; + +use super::{ + manifest::{ + MAX_DOWNLOAD_MANIFEST_ENTRIES, + MAX_DOWNLOAD_RELATIVE_PATH_BYTES, + ValidatedDownloadManifest, + validate_owned_file_path, + }, + version_ini::{ + discard_version_ini_transaction, + finish_recovered_version_ini_transaction, + restore_unjournaled_version_ini_transaction, + }, +}; +use crate::state_paths::{download_ownership_path, download_ownership_tmp_path}; + +const OWNERSHIP_SCHEMA_VERSION: u32 = 1; +const MAX_OWNERSHIP_RECORD_BYTES: u64 = 128 * 1024 * 1024; + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(deny_unknown_fields)] +struct DownloadOwnershipRecord { + schema_version: u32, + game_id: String, + games_folder_key: String, + committed_files: Vec, + pending_files: Option>, +} + +impl DownloadOwnershipRecord { + fn empty(game_id: &str, games_folder_key: &str) -> Self { + Self { + schema_version: OWNERSHIP_SCHEMA_VERSION, + game_id: game_id.to_owned(), + games_folder_key: games_folder_key.to_owned(), + committed_files: Vec::new(), + pending_files: None, + } + } + + fn validate( + self, + expected_game_id: &str, + expected_games_folder_key: &str, + ) -> eyre::Result { + if self.schema_version != OWNERSHIP_SCHEMA_VERSION { + eyre::bail!( + "unsupported download ownership schema {}", + self.schema_version + ); + } + if self.game_id != expected_game_id { + eyre::bail!( + "download ownership belongs to game {}, expected {expected_game_id}", + self.game_id + ); + } + if self.games_folder_key != expected_games_folder_key { + eyre::bail!("download ownership belongs to a different games directory"); + } + validate_file_set(&self.committed_files)?; + if let Some(pending) = &self.pending_files { + validate_file_set(pending)?; + } + Ok(self) + } +} + +enum LoadedOwnership { + Missing, + Invalid, + Valid(DownloadOwnershipRecord), +} + +/// One download attempt whose previous and proposed ownership sets are durable. +#[derive(Debug)] +pub(super) struct DownloadOwnershipTransaction { + record_path: PathBuf, + tmp_path: PathBuf, + game_id: String, + games_folder_key: String, + game_root: PathBuf, + previous: BTreeSet, + current: BTreeSet, +} + +impl DownloadOwnershipTransaction { + /// Recovers an earlier attempt and loads the last trustworthy ownership set. + pub(super) async fn prepare( + state_dir: &Path, + manifest: &ValidatedDownloadManifest, + ) -> eyre::Result { + recover_incomplete_download(manifest.game_root(), state_dir, manifest.game_id()).await?; + + let games_folder_key = games_folder_key(manifest.games_folder()); + let record_path = download_ownership_path(state_dir, manifest.game_id()); + let tmp_path = download_ownership_tmp_path(state_dir, manifest.game_id()); + let previous = match load_record(&record_path, manifest.game_id(), &games_folder_key).await + { + LoadedOwnership::Valid(record) => record.committed_files.into_iter().collect(), + LoadedOwnership::Missing | LoadedOwnership::Invalid => { + let baseline = + DownloadOwnershipRecord::empty(manifest.game_id(), &games_folder_key); + write_record(&record_path, &tmp_path, &baseline).await?; + BTreeSet::new() + } + }; + let current = manifest.owned_file_paths().into_iter().collect(); + validate_generation_aliases(&previous, ¤t)?; + + Ok(Self { + record_path, + tmp_path, + game_id: manifest.game_id().to_owned(), + games_folder_key, + game_root: manifest.game_root().to_path_buf(), + previous, + current, + }) + } + + /// Publishes the proposed set after the old sentinel has been parked. + pub(super) async fn journal_pending(&self) -> eyre::Result<()> { + let record = DownloadOwnershipRecord { + schema_version: OWNERSHIP_SCHEMA_VERSION, + game_id: self.game_id.clone(), + games_folder_key: self.games_folder_key.clone(), + committed_files: self.previous.iter().cloned().collect(), + pending_files: Some(self.current.iter().cloned().collect()), + }; + write_record(&self.record_path, &self.tmp_path, &record).await + } + + /// Removes only previously owned regular files absent from this manifest. + pub(super) async fn remove_stale(&self) -> eyre::Result<()> { + let stale = self + .previous + .difference(&self.current) + .cloned() + .collect::>(); + remove_owned_files(&self.game_root, &stale).await + } + + /// Aborts a journaled attempt without touching paths outside either owned set. + pub(super) async fn abort(&self) -> eyre::Result<()> { + let removable = self + .previous + .union(&self.current) + .cloned() + .collect::>(); + remove_owned_files(&self.game_root, &removable).await?; + discard_version_ini_transaction(&self.game_root).await?; + let empty = DownloadOwnershipRecord::empty(&self.game_id, &self.games_folder_key); + write_record(&self.record_path, &self.tmp_path, &empty).await + } + + /// Finalizes ownership after the new `version.ini` commit point has landed. + pub(super) async fn finalize(&self) -> eyre::Result<()> { + let record = DownloadOwnershipRecord { + schema_version: OWNERSHIP_SCHEMA_VERSION, + game_id: self.game_id.clone(), + games_folder_key: self.games_folder_key.clone(), + committed_files: self.current.iter().cloned().collect(), + pending_files: None, + }; + write_record(&self.record_path, &self.tmp_path, &record).await + } +} + +/// Recovers the ownership/version transaction for one inactive game root. +pub(crate) async fn recover_incomplete_download( + game_root: &Path, + state_dir: &Path, + game_id: &str, +) -> eyre::Result<()> { + let Some(games_folder) = game_root.parent() else { + eyre::bail!( + "game root has no games-directory parent: {}", + game_root.display() + ); + }; + let games_folder = match tokio::fs::canonicalize(games_folder).await { + Ok(path) => path, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(()), + Err(error) => return Err(error.into()), + }; + let key = games_folder_key(&games_folder); + let path = download_ownership_path(state_dir, game_id); + let tmp_path = download_ownership_tmp_path(state_dir, game_id); + + match load_record(&path, game_id, &key).await { + LoadedOwnership::Missing => { + // Pre-journal versions used the same scratch name after payload + // mutation. Without a new-format baseline, restoring it could + // advertise partially overwritten bytes as a complete download. + discard_version_ini_transaction(game_root).await?; + } + LoadedOwnership::Invalid => { + // With no trustworthy journal, never make potentially partial bytes ready. + discard_version_ini_transaction(game_root).await?; + } + LoadedOwnership::Valid(mut record) => { + let Some(pending) = record.pending_files.clone() else { + restore_unjournaled_version_ini_transaction(game_root).await?; + sweep_tmp_file(&tmp_path).await; + return Ok(()); + }; + + let committed = record + .committed_files + .iter() + .cloned() + .collect::>(); + let pending = pending.into_iter().collect::>(); + if version_ini_is_regular(game_root).await? { + let stale = committed + .difference(&pending) + .cloned() + .collect::>(); + remove_owned_files(game_root, &stale).await?; + finish_recovered_version_ini_transaction(game_root).await?; + record.committed_files = pending.into_iter().collect(); + record.pending_files = None; + write_record(&path, &tmp_path, &record).await?; + } else { + let removable = committed.union(&pending).cloned().collect::>(); + remove_owned_files(game_root, &removable).await?; + discard_version_ini_transaction(game_root).await?; + let empty = DownloadOwnershipRecord::empty(game_id, &key); + write_record(&path, &tmp_path, &empty).await?; + } + } + } + sweep_tmp_file(&tmp_path).await; + Ok(()) +} + +/// Clears provenance after an explicit downloaded-game removal succeeds. +pub(crate) async fn clear_download_ownership(state_dir: &Path, game_id: &str) -> eyre::Result<()> { + let path = download_ownership_path(state_dir, game_id); + remove_file_if_exists(&download_ownership_tmp_path(state_dir, game_id)).await?; + remove_file_if_exists(&path).await?; + if let Err(error) = sync_parent_dir(&path) + && error.kind() != ErrorKind::NotFound + { + return Err(error.into()); + } + Ok(()) +} + +fn validate_file_set(paths: &[String]) -> eyre::Result<()> { + if paths.len() > MAX_DOWNLOAD_MANIFEST_ENTRIES { + eyre::bail!("download ownership has too many paths"); + } + + let mut previous: Option<&str> = None; + let mut aliases = BTreeSet::new(); + for path in paths { + if path.len() > MAX_DOWNLOAD_RELATIVE_PATH_BYTES { + eyre::bail!("download ownership path is too long"); + } + if previous.is_some_and(|previous| previous >= path.as_str()) { + eyre::bail!("download ownership paths are not strictly sorted"); + } + let alias = validate_owned_file_path(path)?; + if !aliases.insert(alias) { + eyre::bail!("download ownership contains platform-alias paths"); + } + previous = Some(path); + } + Ok(()) +} + +fn validate_generation_aliases( + previous: &BTreeSet, + current: &BTreeSet, +) -> eyre::Result<()> { + let mut previous_aliases = std::collections::BTreeMap::new(); + for path in previous { + previous_aliases.insert(validate_owned_file_path(path)?, path); + } + for path in current { + let alias = validate_owned_file_path(path)?; + if let Some(previous_path) = previous_aliases.get(&alias) + && *previous_path != path + { + eyre::bail!( + "download path changed only by a portable filesystem alias: {previous_path} -> {path}" + ); + } + } + Ok(()) +} + +async fn load_record( + path: &Path, + expected_game_id: &str, + expected_games_folder_key: &str, +) -> LoadedOwnership { + let metadata = match tokio::fs::metadata(path).await { + Ok(metadata) => metadata, + Err(error) if error.kind() == ErrorKind::NotFound => return LoadedOwnership::Missing, + Err(error) => { + log::warn!( + "Ignoring unreadable download ownership {}: {error}", + path.display() + ); + return LoadedOwnership::Invalid; + } + }; + if !metadata.is_file() || metadata.len() > MAX_OWNERSHIP_RECORD_BYTES { + log::warn!( + "Ignoring invalid download ownership file {}", + path.display() + ); + return LoadedOwnership::Invalid; + } + + let bytes = match tokio::fs::read(path).await { + Ok(bytes) => bytes, + Err(error) => { + log::warn!( + "Ignoring unreadable download ownership {}: {error}", + path.display() + ); + return LoadedOwnership::Invalid; + } + }; + match serde_json::from_slice::(&bytes) + .map_err(eyre::Report::from) + .and_then(|record| record.validate(expected_game_id, expected_games_folder_key)) + { + Ok(record) => LoadedOwnership::Valid(record), + Err(error) => { + log::warn!( + "Ignoring invalid download ownership {}: {error}", + path.display() + ); + LoadedOwnership::Invalid + } + } +} + +async fn write_record( + path: &Path, + tmp_path: &Path, + record: &DownloadOwnershipRecord, +) -> eyre::Result<()> { + write_record_with_parent_sync(path, tmp_path, record, sync_parent_dir).await +} + +async fn write_record_with_parent_sync( + path: &Path, + tmp_path: &Path, + record: &DownloadOwnershipRecord, + sync_parent: impl FnOnce(&Path) -> std::io::Result<()>, +) -> eyre::Result<()> { + let parent = path + .parent() + .ok_or_else(|| eyre::eyre!("download ownership path has no parent"))?; + create_state_parent_durably(parent).await?; + let bytes = serde_json::to_vec_pretty(record)?; + if u64::try_from(bytes.len())? > MAX_OWNERSHIP_RECORD_BYTES { + eyre::bail!("download ownership record exceeds its size limit"); + } + + let mut file = tokio::fs::File::create(tmp_path).await?; + file.write_all(&bytes).await?; + file.sync_all().await?; + drop(file); + tokio::fs::rename(tmp_path, path).await?; + if let Err(error) = sync_parent(path) { + // The rename is the publication point. Reporting an error from here + // would let callers incorrectly roll back a record that is already + // visible, making the journal state ambiguous to recovery. + log::warn!( + "Published download ownership {} but failed to sync its parent: {error}", + path.display() + ); + } + Ok(()) +} + +async fn remove_owned_files(game_root: &Path, paths: &BTreeSet) -> eyre::Result<()> { + for relative_path in paths { + remove_owned_regular_file(game_root, relative_path).await?; + } + Ok(()) +} + +async fn remove_owned_regular_file(game_root: &Path, relative_path: &str) -> eyre::Result { + validate_owned_file_path(relative_path)?; + let mut destination = game_root.to_path_buf(); + let components = relative_path.split('/').collect::>(); + for (index, component) in components.iter().enumerate() { + destination.push(component); + let metadata = match tokio::fs::symlink_metadata(&destination).await { + Ok(metadata) => metadata, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(false), + Err(error) => return Err(error.into()), + }; + let is_final = index + 1 == components.len(); + if metadata.file_type().is_symlink() { + log::warn!( + "Preserving owned path with symlink component {}", + destination.display() + ); + return Ok(false); + } + if !is_final && !metadata.is_dir() { + log::warn!( + "Preserving owned path with non-directory parent {}", + destination.display() + ); + return Ok(false); + } + if is_final && !metadata.is_file() { + log::warn!( + "Preserving owned path whose shape changed at {}", + destination.display() + ); + return Ok(false); + } + } + + remove_file_if_exists(&destination).await?; + sync_parent_dir(&destination)?; + Ok(true) +} + +async fn create_state_parent_durably(path: &Path) -> eyre::Result<()> { + let mut missing = Vec::new(); + let mut cursor = Some(path); + while let Some(candidate) = cursor { + match tokio::fs::symlink_metadata(candidate).await { + Ok(metadata) => { + if !metadata.is_dir() { + eyre::bail!( + "download ownership parent is not a directory: {}", + candidate.display() + ); + } + break; + } + Err(error) if error.kind() == ErrorKind::NotFound => { + missing.push(candidate.to_path_buf()); + cursor = candidate.parent(); + } + Err(error) => return Err(error.into()), + } + } + + tokio::fs::create_dir_all(path).await?; + for created in missing.iter().rev() { + sync_parent_dir(created)?; + } + Ok(()) +} + +async fn version_ini_is_regular(game_root: &Path) -> eyre::Result { + match tokio::fs::symlink_metadata(game_root.join(crate::game_paths::VERSION_INI)).await { + Ok(metadata) => Ok(metadata.is_file() && !metadata.file_type().is_symlink()), + Err(error) if error.kind() == ErrorKind::NotFound => Ok(false), + Err(error) => Err(error.into()), + } +} + +async fn remove_file_if_exists(path: &Path) -> eyre::Result<()> { + match tokio::fs::remove_file(path).await { + Ok(()) => Ok(()), + Err(error) if error.kind() == ErrorKind::NotFound => Ok(()), + Err(error) => Err(error.into()), + } +} + +async fn sweep_tmp_file(path: &Path) { + if let Err(error) = remove_file_if_exists(path).await { + log::warn!( + "Failed to sweep ownership scratch {}: {error}", + path.display() + ); + } +} + +#[cfg(unix)] +fn games_folder_key(path: &Path) -> String { + use std::os::unix::ffi::OsStrExt; + + format!("unix:{}", hex_encode(path.as_os_str().as_bytes())) +} + +#[cfg(windows)] +fn games_folder_key(path: &Path) -> String { + use std::{fmt::Write as _, os::windows::ffi::OsStrExt}; + + let mut encoded = String::from("windows:"); + for unit in path.as_os_str().encode_wide() { + let _ = write!(encoded, "{unit:04x}"); + } + encoded +} + +#[cfg(not(any(unix, windows)))] +fn games_folder_key(path: &Path) -> String { + format!("native:{}", hex_encode(path.as_os_str().as_encoded_bytes())) +} + +fn hex_encode(bytes: &[u8]) -> String { + use std::fmt::Write as _; + + let mut encoded = String::with_capacity(bytes.len() * 2); + for byte in bytes { + let _ = write!(encoded, "{byte:02x}"); + } + encoded +} + +#[cfg(unix)] +fn sync_parent_dir(path: &Path) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::File::open(parent)?.sync_all()?; + } + Ok(()) +} + +#[cfg(not(unix))] +const fn sync_parent_dir(_path: &Path) -> std::io::Result<()> { + Ok(()) +} + +#[cfg(test)] +mod tests { + use lanspread_db::db::{GameCatalog, GameFileDescription}; + + use super::*; + use crate::{ + game_paths::{LOCAL_DIR, VERSION_DISCARDED_FILE, VERSION_INI}, + test_support::TempDir, + }; + + fn manifest(games_folder: &Path, files: &[&str]) -> ValidatedDownloadManifest { + let mut descriptions = vec![GameFileDescription { + game_id: "game".to_owned(), + relative_path: "game/version.ini".to_owned(), + is_dir: false, + size: 8, + }]; + descriptions.extend(files.iter().map(|path| GameFileDescription { + game_id: "game".to_owned(), + relative_path: format!("game/{path}"), + is_dir: false, + size: 4, + })); + ValidatedDownloadManifest::from_protocol_v7( + games_folder, + "game", + descriptions, + &GameCatalog::from_ids(["game".to_owned()]), + ) + .expect("manifest should validate") + } + + fn write_file(path: &Path, bytes: &[u8]) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).expect("parent should be created"); + } + std::fs::write(path, bytes).expect("file should be written"); + } + + async fn seed_record( + state_dir: &Path, + games_folder: &Path, + committed: &[&str], + pending: Option<&[&str]>, + ) { + let record = DownloadOwnershipRecord { + schema_version: OWNERSHIP_SCHEMA_VERSION, + game_id: "game".to_owned(), + games_folder_key: games_folder_key(games_folder), + committed_files: committed.iter().map(ToString::to_string).collect(), + pending_files: pending.map(|paths| paths.iter().map(ToString::to_string).collect()), + }; + write_record( + &download_ownership_path(state_dir, "game"), + &download_ownership_tmp_path(state_dir, "game"), + &record, + ) + .await + .expect("record should be written"); + } + + async fn read_valid_record(state_dir: &Path, games_folder: &Path) -> DownloadOwnershipRecord { + match load_record( + &download_ownership_path(state_dir, "game"), + "game", + &games_folder_key(games_folder), + ) + .await + { + LoadedOwnership::Valid(record) => record, + LoadedOwnership::Missing => panic!("record should exist"), + LoadedOwnership::Invalid => panic!("record should be valid"), + } + } + + #[tokio::test] + async fn journal_is_sorted_bound_and_ignores_stray_tmp() { + let games = TempDir::new("lanspread-ownership-games"); + let state = TempDir::new("lanspread-ownership-state"); + let manifest = manifest(games.path(), &["z.eti", "nested/a.bin"]); + let transaction = DownloadOwnershipTransaction::prepare(state.path(), &manifest) + .await + .expect("transaction should prepare"); + transaction + .journal_pending() + .await + .expect("pending set should be durable"); + + let record = read_valid_record(state.path(), games.path()).await; + assert_eq!( + record.pending_files, + Some(vec!["nested/a.bin".to_owned(), "z.eti".to_owned()]) + ); + assert!(record.committed_files.is_empty()); + assert_eq!(record.game_id, "game"); + assert_eq!(record.games_folder_key, games_folder_key(games.path())); + + transaction + .finalize() + .await + .expect("record should finalize"); + write_file( + &download_ownership_tmp_path(state.path(), "game"), + b"not json", + ); + let stable = read_valid_record(state.path(), games.path()).await; + assert_eq!(stable.committed_files, ["nested/a.bin", "z.eti"]); + assert!(stable.pending_files.is_none()); + } + + #[test] + fn record_validation_rejects_untrusted_paths_and_bindings() { + let valid = DownloadOwnershipRecord { + schema_version: OWNERSHIP_SCHEMA_VERSION, + game_id: "game".to_owned(), + games_folder_key: "root".to_owned(), + committed_files: vec!["archive.eti".to_owned()], + pending_files: None, + }; + assert!(valid.clone().validate("game", "root").is_ok()); + + for paths in [ + vec!["../outside".to_owned()], + vec!["local/save.dat".to_owned()], + vec!["A.eti".to_owned(), "a.eti".to_owned()], + vec!["z.eti".to_owned(), "a.eti".to_owned()], + vec![VERSION_INI.to_owned()], + vec!["Version.ini".to_owned()], + vec!["VERSION.INI".to_owned()], + ] { + let mut invalid = valid.clone(); + invalid.committed_files = paths; + assert!(invalid.validate("game", "root").is_err()); + } + + let mut wrong_schema = valid.clone(); + wrong_schema.schema_version += 1; + assert!(wrong_schema.validate("game", "root").is_err()); + assert!(valid.clone().validate("other", "root").is_err()); + assert!(valid.validate("game", "other-root").is_err()); + } + + #[tokio::test] + async fn replacement_and_abort_touch_only_explicitly_owned_paths() { + let games = TempDir::new("lanspread-ownership-replace-games"); + let state = TempDir::new("lanspread-ownership-replace-state"); + let root = games.game_root(); + for (path, bytes) in [ + ("keep.eti", b"keep".as_slice()), + ("stale.eti", b"old".as_slice()), + ("nested/stale.bin", b"old".as_slice()), + ("new.eti", b"new".as_slice()), + ("notes.txt", b"user".as_slice()), + ("local/save.dat", b"save".as_slice()), + ] { + write_file(&root.join(path), bytes); + } + write_file(&root.join(VERSION_INI), b"20240101"); + seed_record( + state.path(), + games.path(), + &["keep.eti", "nested/stale.bin", "stale.eti"], + None, + ) + .await; + + let manifest = manifest(games.path(), &["keep.eti", "new.eti"]); + let transaction = DownloadOwnershipTransaction::prepare(state.path(), &manifest) + .await + .expect("transaction should prepare"); + super::super::version_ini::begin_version_ini_transaction(&root) + .await + .expect("sentinel should park"); + transaction + .journal_pending() + .await + .expect("pending should journal"); + transaction + .remove_stale() + .await + .expect("stale cleanup should succeed"); + + assert!(root.join("keep.eti").is_file()); + assert!(!root.join("stale.eti").exists()); + assert!(!root.join("nested/stale.bin").exists()); + assert!(root.join("nested").is_dir()); + assert_eq!( + std::fs::read(root.join("notes.txt")).expect("user file should remain readable"), + b"user" + ); + assert_eq!( + std::fs::read(root.join(LOCAL_DIR).join("save.dat")) + .expect("local save should remain readable"), + b"save" + ); + + transaction.abort().await.expect("abort should succeed"); + assert!(!root.join("keep.eti").exists()); + assert!(!root.join("new.eti").exists()); + assert_eq!( + std::fs::read(root.join("notes.txt")).expect("user file should remain readable"), + b"user" + ); + assert_eq!( + std::fs::read(root.join(LOCAL_DIR).join("save.dat")) + .expect("local save should remain readable"), + b"save" + ); + let record = read_valid_record(state.path(), games.path()).await; + assert!(record.committed_files.is_empty()); + assert!(record.pending_files.is_none()); + } + + #[tokio::test] + async fn recovery_handles_every_durable_journal_state() { + // Crash after parking the old sentinel but before publishing pending. + { + let games = TempDir::new("lanspread-ownership-unrecorded-games"); + let state = TempDir::new("lanspread-ownership-unrecorded-state"); + let root = games.game_root(); + write_file(&root.join(VERSION_INI), b"20240101"); + let manifest = manifest(games.path(), &["archive.eti"]); + let _transaction = DownloadOwnershipTransaction::prepare(state.path(), &manifest) + .await + .expect("baseline ownership should be durable"); + super::super::version_ini::begin_version_ini_transaction(&root) + .await + .expect("sentinel should park"); + recover_incomplete_download(&root, state.path(), "game") + .await + .expect("unjournaled park should recover"); + assert_eq!( + std::fs::read(root.join(VERSION_INI)).expect("sentinel should be readable"), + b"20240101" + ); + assert!(!root.join(VERSION_DISCARDED_FILE).exists()); + } + + // Pending without a sentinel means the transaction never committed. + { + let games = TempDir::new("lanspread-ownership-abort-games"); + let state = TempDir::new("lanspread-ownership-abort-state"); + let root = games.game_root(); + write_file(&root.join("old.eti"), b"old"); + write_file(&root.join("new.eti"), b"partial"); + write_file(&root.join("notes.txt"), b"user"); + write_file(&root.join(LOCAL_DIR).join("save.dat"), b"save"); + write_file(&root.join(VERSION_DISCARDED_FILE), b"20240101"); + seed_record(state.path(), games.path(), &["old.eti"], Some(&["new.eti"])).await; + recover_incomplete_download(&root, state.path(), "game") + .await + .expect("pending abort should recover"); + assert!(!root.join("old.eti").exists()); + assert!(!root.join("new.eti").exists()); + assert!(!root.join(VERSION_INI).exists()); + assert!(!root.join(VERSION_DISCARDED_FILE).exists()); + assert_eq!( + std::fs::read(root.join("notes.txt")).expect("user file should remain readable"), + b"user" + ); + assert_eq!( + std::fs::read(root.join(LOCAL_DIR).join("save.dat")) + .expect("local save should remain readable"), + b"save" + ); + let record = read_valid_record(state.path(), games.path()).await; + assert!(record.committed_files.is_empty()); + assert!(record.pending_files.is_none()); + } + + // Pending with a sentinel means commit landed before ledger finalization. + { + let games = TempDir::new("lanspread-ownership-commit-games"); + let state = TempDir::new("lanspread-ownership-commit-state"); + let root = games.game_root(); + for path in ["keep.eti", "new.eti", "stale.eti", "notes.txt"] { + write_file(&root.join(path), path.as_bytes()); + } + write_file(&root.join(VERSION_INI), b"20250101"); + write_file(&root.join(VERSION_DISCARDED_FILE), b"20240101"); + seed_record( + state.path(), + games.path(), + &["keep.eti", "stale.eti"], + Some(&["keep.eti", "new.eti"]), + ) + .await; + recover_incomplete_download(&root, state.path(), "game") + .await + .expect("landed commit should finalize"); + assert!(root.join("keep.eti").is_file()); + assert!(root.join("new.eti").is_file()); + assert!(!root.join("stale.eti").exists()); + assert!(root.join("notes.txt").is_file()); + assert_eq!( + std::fs::read(root.join(VERSION_INI)).expect("sentinel should be readable"), + b"20250101" + ); + assert!(!root.join(VERSION_DISCARDED_FILE).exists()); + let record = read_valid_record(state.path(), games.path()).await; + assert_eq!(record.committed_files, ["keep.eti", "new.eti"]); + assert!(record.pending_files.is_none()); + } + } + + #[tokio::test] + async fn missing_legacy_record_never_restores_ambiguous_discarded_sentinel() { + let games = TempDir::new("lanspread-ownership-legacy-games"); + let state = TempDir::new("lanspread-ownership-legacy-state"); + let root = games.game_root(); + write_file(&root.join("archive.eti"), b"possibly-partial"); + write_file(&root.join(VERSION_DISCARDED_FILE), b"20240101"); + + recover_incomplete_download(&root, state.path(), "game") + .await + .expect("legacy scratch should fail closed"); + + assert!(!root.join(VERSION_INI).exists()); + assert!(!root.join(VERSION_DISCARDED_FILE).exists()); + assert_eq!( + std::fs::read(root.join("archive.eti")).expect("unknown payload should remain"), + b"possibly-partial" + ); + } + + #[tokio::test] + async fn published_record_is_committed_even_if_parent_sync_reports_failure() { + let games = TempDir::new("lanspread-ownership-publish-games"); + let state = TempDir::new("lanspread-ownership-publish-state"); + let record = DownloadOwnershipRecord { + schema_version: OWNERSHIP_SCHEMA_VERSION, + game_id: "game".to_owned(), + games_folder_key: games_folder_key(games.path()), + committed_files: Vec::new(), + pending_files: Some(vec!["archive.eti".to_owned()]), + }; + let record_path = download_ownership_path(state.path(), "game"); + let tmp_path = download_ownership_tmp_path(state.path(), "game"); + + write_record_with_parent_sync(&record_path, &tmp_path, &record, |_| { + Err(std::io::Error::other("injected parent sync failure")) + }) + .await + .expect("post-publication sync failure must not look unpublished"); + + assert_eq!(read_valid_record(state.path(), games.path()).await, record); + } + + #[tokio::test] + async fn cross_generation_portable_alias_is_rejected_before_payload_mutation() { + let games = TempDir::new("lanspread-ownership-alias-games"); + let state = TempDir::new("lanspread-ownership-alias-state"); + let root = games.game_root(); + write_file(&root.join(VERSION_INI), b"20240101"); + write_file(&root.join("Archive.eti"), b"old"); + seed_record(state.path(), games.path(), &["Archive.eti"], None).await; + + let error = DownloadOwnershipTransaction::prepare( + state.path(), + &manifest(games.path(), &["archive.eti"]), + ) + .await + .expect_err("case-only ownership changes must fail closed"); + + assert!(error.to_string().contains("portable filesystem alias")); + assert!(root.join(VERSION_INI).is_file()); + assert_eq!( + std::fs::read(root.join("Archive.eti")).expect("old payload should remain"), + b"old" + ); + assert!(!root.join("archive.eti").exists()); + } + + #[tokio::test] + async fn corrupt_or_wrong_root_state_never_authorizes_payload_deletion() { + let games_a = TempDir::new("lanspread-ownership-binding-a"); + let games_b = TempDir::new("lanspread-ownership-binding-b"); + let state = TempDir::new("lanspread-ownership-binding-state"); + seed_record( + state.path(), + games_a.path(), + &["archive.eti"], + Some(&["archive.eti"]), + ) + .await; + write_file(&games_b.game_root().join("archive.eti"), b"other-library"); + write_file( + &games_b.game_root().join(VERSION_DISCARDED_FILE), + b"20240101", + ); + + recover_incomplete_download(&games_b.game_root(), state.path(), "game") + .await + .expect("wrong binding should fail closed"); + assert_eq!( + std::fs::read(games_b.game_root().join("archive.eti")) + .expect("foreign payload should remain readable"), + b"other-library" + ); + assert!(!games_b.game_root().join(VERSION_INI).exists()); + + write_file(&download_ownership_path(state.path(), "game"), b"corrupt"); + write_file(&games_b.game_root().join("partial.eti"), b"unknown"); + recover_incomplete_download(&games_b.game_root(), state.path(), "game") + .await + .expect("corrupt state should preserve unknown payload"); + assert_eq!( + std::fs::read(games_b.game_root().join("partial.eti")) + .expect("unknown payload should remain readable"), + b"unknown" + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn cleanup_never_follows_owned_symlink_paths() { + use std::os::unix::fs::symlink; + + let games = TempDir::new("lanspread-ownership-link-games"); + let state = TempDir::new("lanspread-ownership-link-state"); + let outside = TempDir::new("lanspread-ownership-link-outside"); + let root = games.game_root(); + write_file(&outside.path().join("canary"), b"outside"); + std::fs::create_dir_all(&root).expect("root should be created"); + symlink(outside.path(), root.join("linked")).expect("symlink should be created"); + seed_record(state.path(), games.path(), &[], Some(&["linked/canary"])).await; + + recover_incomplete_download(&root, state.path(), "game") + .await + .expect("recovery should preserve the link"); + assert!(root.join("linked").is_symlink()); + assert_eq!( + std::fs::read(outside.path().join("canary")) + .expect("outside canary should remain readable"), + b"outside" + ); + } +} diff --git a/crates/lanspread-peer/src/download/storage.rs b/crates/lanspread-peer/src/download/storage.rs index 8c0930d..7e883ce 100644 --- a/crates/lanspread-peer/src/download/storage.rs +++ b/crates/lanspread-peer/src/download/storage.rs @@ -1,9 +1,8 @@ -use std::{io::ErrorKind, path::Path}; +use std::{cmp::Reverse, collections::BTreeSet, path::PathBuf}; use tokio::fs::OpenOptions; use super::manifest::ValidatedDownloadManifest; -use crate::game_paths::is_preserved_on_download_discard; /// Prepares storage for game files by creating directories and pre-allocating files. pub(super) async fn prepare_game_storage(manifest: &ValidatedDownloadManifest) -> eyre::Result<()> { @@ -25,116 +24,67 @@ pub(super) async fn prepare_game_storage(manifest: &ValidatedDownloadManifest) - .await?; let size = entry.size(); - if let Err(e) = file.set_len(size).await { - log::warn!( - "Failed to pre-allocate file {} (size: {}): {}", - entry.canonical_path(), - size, - e - ); - } else { - log::debug!( - "Pre-allocated file {} with {} bytes", - entry.canonical_path(), - size - ); - } + file.set_len(size).await?; + log::debug!( + "Prepared file {} with {} bytes", + entry.canonical_path(), + size + ); } } Ok(()) } -/// Discards the peer-owned downloaded payload after a cancelled transfer. -/// -/// Downloads own the root archive/cache files, but not `local/` or install -/// transaction metadata. Preserving those paths lets a cancelled update settle -/// as a local-only install instead of deleting user-owned extracted files. -pub(super) async fn discard_cancelled_download( - games_folder: &Path, - game_id: &str, -) -> eyre::Result<()> { - let game_root = games_folder.join(game_id); - let Some(metadata) = symlink_metadata_if_exists(&game_root).await? else { - return Ok(()); - }; +/// Makes payload bytes and directory entries durable before the sentinel commit. +pub(super) async fn sync_game_storage(manifest: &ValidatedDownloadManifest) -> eyre::Result<()> { + let mut directories = BTreeSet::new(); + directories.insert(manifest.game_root().to_path_buf()); - if metadata.file_type().is_symlink() { - eyre::bail!( - "refusing to discard cancelled download through symlink root {}", - game_root.display() - ); - } - if !metadata.is_dir() { - eyre::bail!( - "refusing to discard cancelled download from non-directory root {}", - game_root.display() - ); - } - - let mut entries = tokio::fs::read_dir(&game_root).await?; - while let Some(entry) = entries.next_entry().await? { - let name = entry.file_name(); - if name - .to_str() - .is_some_and(should_preserve_on_download_discard) - { + for entry in manifest.transfer_entries() { + let path = manifest.game_root().join(entry.canonical_path()); + if entry.is_dir() { + directories.insert(path); continue; } - remove_entry(&entry.path()).await?; - } + OpenOptions::new() + .read(true) + .write(true) + .open(&path) + .await? + .sync_all() + .await?; - remove_dir_if_empty(&game_root).await -} - -fn should_preserve_on_download_discard(name: &str) -> bool { - is_preserved_on_download_discard(name) -} - -async fn remove_entry(path: &Path) -> eyre::Result<()> { - let Some(metadata) = symlink_metadata_if_exists(path).await? else { - return Ok(()); - }; - - if metadata.file_type().is_symlink() || metadata.is_file() { - remove_file_if_exists(path).await - } else if metadata.is_dir() { - tokio::fs::remove_dir_all(path).await?; - Ok(()) - } else { - remove_file_if_exists(path).await - } -} - -async fn remove_file_if_exists(path: &Path) -> eyre::Result<()> { - match tokio::fs::remove_file(path).await { - Ok(()) => Ok(()), - Err(err) if err.kind() == ErrorKind::NotFound => Ok(()), - Err(err) => Err(err.into()), - } -} - -async fn remove_dir_if_empty(path: &Path) -> eyre::Result<()> { - match tokio::fs::remove_dir(path).await { - Ok(()) => Ok(()), - Err(err) - if matches!( - err.kind(), - ErrorKind::NotFound | ErrorKind::DirectoryNotEmpty - ) => - { - Ok(()) + let mut parent = path.parent(); + while let Some(path) = parent { + if !path.starts_with(manifest.game_root()) { + break; + } + directories.insert(path.to_path_buf()); + if path == manifest.game_root() { + break; + } + parent = path.parent(); } - Err(err) => Err(err.into()), } + + let mut directories = directories.into_iter().collect::>(); + directories.sort_by_key(|path| Reverse(path.components().count())); + for directory in directories { + sync_directory(&directory).await?; + } + Ok(()) } -async fn symlink_metadata_if_exists(path: &Path) -> eyre::Result> { - match tokio::fs::symlink_metadata(path).await { - Ok(metadata) => Ok(Some(metadata)), - Err(err) if err.kind() == ErrorKind::NotFound => Ok(None), - Err(err) => Err(err.into()), - } +#[cfg(unix)] +async fn sync_directory(path: &std::path::Path) -> eyre::Result<()> { + tokio::fs::File::open(path).await?.sync_all().await?; + Ok(()) +} + +#[cfg(not(unix))] +async fn sync_directory(_path: &std::path::Path) -> eyre::Result<()> { + Ok(()) } #[cfg(test)] @@ -144,13 +94,6 @@ mod tests { use super::*; use crate::test_support::TempDir; - fn write_file(path: &Path, bytes: &[u8]) { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).expect("parent dir should be created"); - } - std::fs::write(path, bytes).expect("file should be written"); - } - #[tokio::test] async fn prepare_game_storage_skips_version_ini_sentinel() { let temp = TempDir::new("lanspread-download"); @@ -174,44 +117,4 @@ mod tests { assert!(!temp.path().join("game").join("version.ini").exists()); } - - #[tokio::test] - async fn discard_cancelled_download_removes_peer_owned_payload() { - let temp = TempDir::new("lanspread-download-discard"); - let root = temp.game_root(); - write_file(&root.join("version.ini"), b"20250101"); - write_file(&root.join(".version.ini.tmp"), b"tmp"); - write_file(&root.join(".version.ini.discarded"), b"old"); - write_file(&root.join("archive.eti"), b"partial"); - write_file(&root.join("nested").join("payload.bin"), b"partial"); - - discard_cancelled_download(temp.path(), "game") - .await - .expect("cancelled payload should be discarded"); - - assert!(!root.exists()); - } - - #[tokio::test] - async fn discard_cancelled_download_preserves_local_install_state() { - let temp = TempDir::new("lanspread-download-discard-local"); - let root = temp.game_root(); - write_file(&root.join("version.ini"), b"20250101"); - write_file(&root.join("archive.eti"), b"partial"); - write_file(&root.join("local").join("save.dat"), b"user-data"); - write_file(&root.join(".local.backup").join(".lanspread_owned"), b""); - - discard_cancelled_download(temp.path(), "game") - .await - .expect("cancelled payload should be discarded"); - - assert!(!root.join("version.ini").exists()); - assert!(!root.join("archive.eti").exists()); - assert_eq!( - std::fs::read(root.join("local").join("save.dat")) - .expect("local install should remain"), - b"user-data" - ); - assert!(root.join(".local.backup").is_dir()); - } } diff --git a/crates/lanspread-peer/src/download/version_ini.rs b/crates/lanspread-peer/src/download/version_ini.rs index b4861f0..ecca87c 100644 --- a/crates/lanspread-peer/src/download/version_ini.rs +++ b/crates/lanspread-peer/src/download/version_ini.rs @@ -5,6 +5,13 @@ use tokio::{io::AsyncWriteExt, sync::Mutex}; use crate::game_paths::{VERSION_DISCARDED_FILE, VERSION_INI, VERSION_TMP_FILE}; +pub(super) enum VersionIniCommit { + Durable, + /// The rename is visible, but recovery must establish its durability before + /// ownership may be finalized. + NeedsRecovery(std::io::Error), +} + #[derive(Debug)] pub(super) struct VersionIniBuffer { relative_path: String, @@ -50,8 +57,10 @@ impl VersionIniBuffer { pub(super) async fn begin_version_ini_transaction(game_root: &Path) -> eyre::Result<()> { tokio::fs::create_dir_all(game_root).await?; + sync_parent_dir(game_root)?; remove_file_if_exists(&game_root.join(VERSION_TMP_FILE)).await?; remove_file_if_exists(&game_root.join(VERSION_DISCARDED_FILE)).await?; + sync_game_root(game_root)?; let version_path = game_root.join(VERSION_INI); if tokio::fs::metadata(&version_path) @@ -59,29 +68,65 @@ pub(super) async fn begin_version_ini_transaction(game_root: &Path) -> eyre::Res .is_ok_and(|metadata| metadata.is_file()) { tokio::fs::rename(version_path, game_root.join(VERSION_DISCARDED_FILE)).await?; + sync_parent_dir(&game_root.join(VERSION_DISCARDED_FILE))?; } Ok(()) } +#[cfg(test)] pub(super) async fn rollback_version_ini_transaction(game_root: &Path) { - if let Err(err) = remove_file_if_exists(&game_root.join(VERSION_TMP_FILE)).await { + if let Err(err) = discard_version_ini_transaction(game_root).await { log::warn!( - "Failed to sweep partial version.ini tmp in {}: {err}", + "Failed to discard version.ini transaction in {}: {err}", game_root.display() ); } - if let Err(err) = remove_file_if_exists(&game_root.join(VERSION_DISCARDED_FILE)).await { - log::warn!( - "Failed to sweep discarded version.ini in {}: {err}", - game_root.display() - ); +} + +/// Restores the old sentinel after a crash or failure before ownership journaling. +pub(super) async fn restore_unjournaled_version_ini_transaction( + game_root: &Path, +) -> eyre::Result<()> { + remove_file_if_exists(&game_root.join(VERSION_TMP_FILE)).await?; + let version_path = game_root.join(VERSION_INI); + let discarded_path = game_root.join(VERSION_DISCARDED_FILE); + if path_is_regular_file(&version_path).await? { + remove_file_if_exists(&discarded_path).await?; + sync_game_root(game_root)?; + return Ok(()); } + if path_is_regular_file(&discarded_path).await? { + tokio::fs::rename(&discarded_path, &version_path).await?; + sync_parent_dir(&version_path)?; + } + Ok(()) +} + +/// Removes all sentinel scratch after an aborted journaled download. +pub(super) async fn discard_version_ini_transaction(game_root: &Path) -> eyre::Result<()> { + remove_file_if_exists(&game_root.join(VERSION_TMP_FILE)).await?; + remove_file_if_exists(&game_root.join(VERSION_DISCARDED_FILE)).await?; + sync_game_root_if_exists(game_root)?; + Ok(()) +} + +/// Sweeps scratch left after a committed sentinel was recovered. +pub(super) async fn finish_recovered_version_ini_transaction(game_root: &Path) -> eyre::Result<()> { + discard_version_ini_transaction(game_root).await } pub(super) async fn commit_version_ini_buffer( game_root: &Path, buffer: &VersionIniBuffer, -) -> eyre::Result<()> { +) -> eyre::Result { + commit_version_ini_buffer_with_sync(game_root, buffer, sync_game_root).await +} + +async fn commit_version_ini_buffer_with_sync( + game_root: &Path, + buffer: &VersionIniBuffer, + mut sync_root: impl FnMut(&Path) -> std::io::Result<()>, +) -> eyre::Result { let tmp_path = game_root.join(VERSION_TMP_FILE); let version_path = game_root.join(VERSION_INI); let bytes = buffer.snapshot().await; @@ -92,9 +137,30 @@ pub(super) async fn commit_version_ini_buffer( drop(file); tokio::fs::rename(&tmp_path, &version_path).await?; - sync_parent_dir(&version_path)?; - remove_file_if_exists(&game_root.join(VERSION_DISCARDED_FILE)).await?; - Ok(()) + if let Err(error) = sync_root(game_root) { + return Ok(VersionIniCommit::NeedsRecovery(error)); + } + if let Err(error) = remove_file_if_exists(&game_root.join(VERSION_DISCARDED_FILE)).await { + log::warn!( + "Committed {} but failed to sweep the parked sentinel: {error}", + version_path.display() + ); + } + if let Err(error) = sync_root(game_root) { + log::warn!( + "Committed {} but failed to sync discarded-sentinel cleanup: {error}", + version_path.display() + ); + } + Ok(VersionIniCommit::Durable) +} + +async fn path_is_regular_file(path: &Path) -> eyre::Result { + match tokio::fs::symlink_metadata(path).await { + Ok(metadata) => Ok(metadata.is_file() && !metadata.file_type().is_symlink()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error.into()), + } } #[cfg(unix)] @@ -110,6 +176,18 @@ fn sync_parent_dir(_path: &Path) -> std::io::Result<()> { Ok(()) } +fn sync_game_root(game_root: &Path) -> std::io::Result<()> { + sync_parent_dir(&game_root.join(VERSION_INI)) +} + +fn sync_game_root_if_exists(game_root: &Path) -> std::io::Result<()> { + match sync_game_root(game_root) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error), + } +} + async fn remove_file_if_exists(path: &Path) -> eyre::Result<()> { match tokio::fs::remove_file(path).await { Ok(()) => Ok(()), @@ -182,6 +260,44 @@ mod tests { assert!(!game_root.join(".version.ini.discarded").exists()); } + #[tokio::test] + async fn landed_rename_with_failed_parent_sync_keeps_recovery_state() { + let temp = TempDir::new("lanspread-version-durability"); + let game_root = temp.game_root(); + tokio::fs::create_dir_all(&game_root) + .await + .expect("game root should be created"); + tokio::fs::write(game_root.join(VERSION_DISCARDED_FILE), b"20240101") + .await + .expect("old sentinel should be parked"); + let desc = GameFileDescription { + game_id: "game".to_string(), + relative_path: "game/version.ini".to_string(), + is_dir: false, + size: 8, + }; + let buffer = VersionIniBuffer::new(&desc).expect("buffer should be created"); + buffer + .write_at(0, b"20250101") + .await + .expect("sentinel bytes should be buffered"); + + let outcome = commit_version_ini_buffer_with_sync(&game_root, &buffer, |_| { + Err(std::io::Error::other("injected directory sync failure")) + }) + .await + .expect("the landed rename should be reported as recoverable"); + + assert!(matches!(outcome, VersionIniCommit::NeedsRecovery(_))); + assert_eq!( + tokio::fs::read(game_root.join(VERSION_INI)) + .await + .expect("new sentinel should be visible"), + b"20250101" + ); + assert!(game_root.join(VERSION_DISCARDED_FILE).is_file()); + } + #[tokio::test] async fn begin_version_ini_transaction_parks_existing_sentinel() { let temp = TempDir::new("lanspread-download"); diff --git a/crates/lanspread-peer/src/game_paths.rs b/crates/lanspread-peer/src/game_paths.rs index fda8a3f..7ec1b8c 100644 --- a/crates/lanspread-peer/src/game_paths.rs +++ b/crates/lanspread-peer/src/game_paths.rs @@ -55,29 +55,6 @@ pub(crate) fn is_download_protected_root_name(name: &str) -> bool { ) } -/// Returns whether cancellation cleanup must preserve an application-owned entry. -/// -/// Version transaction scratch files are intentionally excluded: they belong to -/// the cancelled download and are safe to sweep. Install and migration state is -/// independent of that download and must survive it. -pub(crate) fn is_preserved_on_download_discard(name: &str) -> bool { - let key = portable_name_key(name); - key == "LOCAL" - || key.starts_with(".LOCAL.") - || matches!( - key.as_str(), - ".SYNC" - | ".LANSPREAD" - | ".LANSPREAD.JSON" - | ".LANSPREAD.JSON.TMP" - | ".LANSPREAD_OWNED" - | ".SOFTLAN_FIRST_START_DONE" - | ".SOFTLAN_GAME_INSTALLED" - | "INSTALL_INTENT.JSON" - | "INSTALL_INTENT.JSON.TMP" - ) -} - /// Returns whether an entry in the configured games directory is application state. pub(crate) fn is_ignored_games_root_name(name: &str) -> bool { portable_name_key(name) == ".LANSPREAD" @@ -112,14 +89,4 @@ mod tests { assert!(!is_download_protected_root_name(VERSION_INI)); assert!(!is_download_protected_root_name("archive.eti")); } - - #[test] - fn cancellation_policy_sweeps_only_download_transaction_state() { - assert!(is_preserved_on_download_discard(LOCAL_DIR)); - assert!(is_preserved_on_download_discard(BACKUP_DIR)); - assert!(is_preserved_on_download_discard(LEGACY_INTENT_FILE)); - assert!(!is_preserved_on_download_discard(VERSION_INI)); - assert!(!is_preserved_on_download_discard(VERSION_TMP_FILE)); - assert!(!is_preserved_on_download_discard("archive.eti")); - } } diff --git a/crates/lanspread-peer/src/handlers.rs b/crates/lanspread-peer/src/handlers.rs index cc7420f..132a423 100644 --- a/crates/lanspread-peer/src/handlers.rs +++ b/crates/lanspread-peer/src/handlers.rs @@ -17,7 +17,12 @@ use crate::{ InstallOperation, PeerEvent, context::{Ctx, OperationGuard, OperationKind}, - download::{ValidatedDownloadManifest, download_game_files, validate_protocol_v7_descriptions}, + download::{ + ValidatedDownloadManifest, + clear_download_ownership, + download_game_files, + validate_protocol_v7_descriptions, + }, events, install, local_games::{ @@ -392,6 +397,7 @@ pub async fn handle_download_game_files_command( let result = download_game_files( manifest, + ctx_clone.state_dir.as_ref(), peer_whitelist, file_peer_map, tx_notify_ui_clone.clone(), @@ -1091,7 +1097,11 @@ async fn run_remove_downloaded_operation( ctx.active_operations.clone(), tx_notify_ui.clone(), ); - let result = install::remove_downloaded(&game_dir, &id).await; + let result = async { + install::remove_downloaded(&game_dir, &id).await?; + clear_download_ownership(ctx.state_dir.as_ref(), &id).await + } + .await; match result { Ok(()) => { @@ -1387,8 +1397,10 @@ async fn load_local_library_with_policy( event_policy: LocalLibraryEventPolicy, ) -> eyre::Result<()> { let game_dir = { ctx.game_dir.read().await.clone() }; - let active_ids = active_operation_ids(ctx).await; + let active_operations = ctx.active_operations.read().await; + let active_ids = active_operations.keys().cloned().collect(); install::recover_on_startup(&game_dir, ctx.state_dir.as_ref(), &active_ids).await?; + drop(active_operations); scan_and_announce_local_library(ctx, tx_notify_ui, &game_dir, event_policy).await } diff --git a/crates/lanspread-peer/src/install/transaction.rs b/crates/lanspread-peer/src/install/transaction.rs index e6d5719..347325e 100644 --- a/crates/lanspread-peer/src/install/transaction.rs +++ b/crates/lanspread-peer/src/install/transaction.rs @@ -12,14 +12,7 @@ use super::{ unpack::Unpacker, }; use crate::{ - game_paths::{ - BACKUP_DIR, - INSTALL_OWNED_MARKER, - INSTALLING_DIR, - LOCAL_DIR, - VERSION_DISCARDED_FILE, - VERSION_TMP_FILE, - }, + game_paths::{BACKUP_DIR, INSTALL_OWNED_MARKER, INSTALLING_DIR, LOCAL_DIR}, local_games::version_ini_is_regular_file, state_paths::launch_settings_applied_path, }; @@ -278,8 +271,6 @@ pub async fn recover_on_startup( state_dir: &Path, active_ids: &HashSet, ) -> eyre::Result<()> { - recover_download_transients(game_dir).await?; - let mut entries = match tokio::fs::read_dir(game_dir).await { Ok(entries) => entries, Err(err) if err.kind() == ErrorKind::NotFound => return Ok(()), @@ -309,7 +300,7 @@ pub async fn recover_on_startup( } pub async fn recover_game_root(game_root: &Path, state_dir: &Path, id: &str) -> eyre::Result<()> { - recover_download_transients(game_root).await?; + crate::download::recover_incomplete_download(game_root, state_dir, id).await?; let intent = read_intent(state_dir, id).await; let fs = inspect_install_fs(game_root).await; @@ -522,12 +513,6 @@ async fn recover_uninstalling( write_intent(state_dir, id, &InstallIntent::none(id, intent.eti_version)).await } -async fn recover_download_transients(root: &Path) -> eyre::Result<()> { - remove_file_if_exists(&root.join(VERSION_TMP_FILE)).await?; - remove_file_if_exists(&root.join(VERSION_DISCARDED_FILE)).await?; - Ok(()) -} - async fn inspect_install_fs(game_root: &Path) -> InstallFsState { InstallFsState { local: path_is_dir(&local_dir(game_root)).await.into(), @@ -694,7 +679,11 @@ mod tests { }; use super::*; - use crate::{install::unpack::UnpackFuture, test_support::TempDir}; + use crate::{ + game_paths::{VERSION_DISCARDED_FILE, VERSION_TMP_FILE}, + install::unpack::UnpackFuture, + test_support::TempDir, + }; #[derive(Default)] struct FakeUnpacker { diff --git a/crates/lanspread-peer/src/state_paths.rs b/crates/lanspread-peer/src/state_paths.rs index 57eec15..13e1126 100644 --- a/crates/lanspread-peer/src/state_paths.rs +++ b/crates/lanspread-peer/src/state_paths.rs @@ -6,6 +6,8 @@ const LOCAL_LIBRARY_INDEX_FILE: &str = "index.json"; const GAMES_DIR: &str = "games"; const SETUP_DONE_FILE: &str = "setup_done"; const LAUNCH_SETTINGS_APPLIED_FILE: &str = "launch_settings_applied"; +const DOWNLOAD_OWNERSHIP_FILE: &str = "download_ownership.json"; +const DOWNLOAD_OWNERSHIP_TMP_FILE: &str = "download_ownership.json.tmp"; pub(crate) fn resolve_state_dir(explicit: Option<&Path>) -> PathBuf { if let Some(dir) = explicit { @@ -46,3 +48,11 @@ pub fn setup_done_path(state_dir: &Path, game_id: &str) -> PathBuf { pub fn launch_settings_applied_path(state_dir: &Path, game_id: &str) -> PathBuf { game_state_dir(state_dir, game_id).join(LAUNCH_SETTINGS_APPLIED_FILE) } + +pub(crate) fn download_ownership_path(state_dir: &Path, game_id: &str) -> PathBuf { + game_state_dir(state_dir, game_id).join(DOWNLOAD_OWNERSHIP_FILE) +} + +pub(crate) fn download_ownership_tmp_path(state_dir: &Path, game_id: &str) -> PathBuf { + game_state_dir(state_dir, game_id).join(DOWNLOAD_OWNERSHIP_TMP_FILE) +} diff --git a/organize/decision-tracking/PEER-AUTH-REFACTOR-DECISIONS.md b/organize/decision-tracking/PEER-AUTH-REFACTOR-DECISIONS.md index e390398..1828321 100644 --- a/organize/decision-tracking/PEER-AUTH-REFACTOR-DECISIONS.md +++ b/organize/decision-tracking/PEER-AUTH-REFACTOR-DECISIONS.md @@ -69,3 +69,156 @@ Alternatives: authority, defeating catalog-owned verification and adding startup work. - Accept hashes announced by peers. This is easy to deploy, but provides no protection against a peer that supplies both the bytes and their claimed hash. + +## 2026-08-09 — Bind ownership records to one canonical games directory + +**TL;DR:** A per-game ownership record includes an opaque, exact identity for +the canonical configured games directory. A record from another directory is +ignored rather than authorizing deletion in the current one. + +The application can switch games directories while retaining the same state +directory. Keying ownership only by `game_id` would let provenance learned in +one tree delete an unrelated same-named file in another tree. The opaque key is +derived from the platform-native canonical path representation so non-Unicode +paths do not need a lossy conversion. + +Alternatives: + +- Clear every ownership record whenever the setting changes. This is safe, but + loses useful cleanup history when the user switches back to an earlier tree. +- Nest ownership records under a games-directory key. This makes the separation + structural, but complicates all existing per-game state layout and migration. +- Store the canonical path as JSON text. This is easy to inspect, but cannot + represent every valid native path without a lossy conversion. + +## 2026-08-09 — Journal pending download ownership across crashes + +**TL;DR:** Persist both the last committed file set and a write-ahead pending +set. Park the old `version.ini` before recording pending ownership, commit the +new sentinel only after transfer and stale cleanup, then finalize the ledger. + +A committed-only ledger cannot distinguish a brand-new partial download from +user files after a crash. The pending set makes cancellation and recovery exact. +Parking the old sentinel first removes an ambiguity during recovery: while a +pending set exists, a regular `version.ini` can only be the newly committed +sentinel. A crash before the pending write instead leaves the parked sentinel, +which recovery restores. + +Alternatives: + +- Record pending before parking the old sentinel. This minimizes time without a + root sentinel, but recovery cannot tell an old sentinel from a landed commit. +- Keep only committed ownership. This is smaller, but leaks partial files from a + crashed first download because they have no trustworthy provenance. +- Put every downloaded file through a separate staging tree. This gives a clean + promotion point, but can require another full game's worth of disk and a large + multi-file transaction mechanism. + +## 2026-08-09 — Manifest target paths become download-owned + +**TL;DR:** Once a validated download transaction starts, every regular file path +in that manifest is treated as downloader-owned for abort and recovery. Unknown +paths outside the manifest remain untouched. + +Without prior ownership state, an existing file at an exact manifest target is +ambiguous. Treating the authoritative target namespace as download-owned keeps +legacy upgrades working and makes partial-transfer cleanup deterministic. A +user-created extra file is preserved, but a user file placed at an exact package +path may be replaced or removed by the download. + +Alternatives: + +- Reject every untracked existing target. This preserves ambiguous files, but + prevents the first post-upgrade update of existing legacy downloads. +- Snapshot and restore untracked targets. This preserves their bytes, but adds + unbounded backup space and another crash-consistent transaction. +- Leave pre-existing untracked targets after abort. This avoids deletion, but + the transfer may already have truncated or partially overwritten them, so it + does not actually preserve their contents. + +## 2026-08-09 — Ownership-record rename is its publication point + +**TL;DR:** Once an ownership-record temporary file has been synced and renamed, +the application treats it as published. A subsequent parent-directory sync +failure is logged but is not reported as a pre-publication failure. + +Rolling back after the rename could restore the old `version.ini` beside a +visible pending record. Recovery would then mistake that old sentinel for the +new download's commit point. Treating rename as publication keeps every +observable state unambiguous; the parent sync still runs to improve power-loss +durability. + +Alternatives: + +- Return a phase-aware error that forces every caller to distinguish failures + before and after rename. This is explicit, but spreads a subtle transaction + protocol across all ownership callers without changing their post-rename + action. +- Try to rename the old record back after a sync failure. That adds another + fallible mutation and can still leave either name visible after a crash. +- Treat every sync failure as fatal and leave the new record in place. This + sounds stricter, but callers could then perform the unsafe sentinel rollback + unless the error also carries publication state. + +## 2026-08-09 — Reject cross-version portable path aliases + +**TL;DR:** An update is rejected before mutation if an owned path changes only +by a portable alias, such as `Data.eti` to `data.eti`. + +On a case-insensitive filesystem, deleting the old spelling after transfer can +delete the newly written destination. On a case-sensitive filesystem, keeping +both spellings leaves stale package data. Rejecting the ambiguous transition is +rare, deterministic, and safe on every supported filesystem. + +Alternatives: + +- Compare exact paths only. This correctly removes the old path on Linux, but + can delete current data on Windows and default macOS filesystems. +- Treat alias-equivalent paths as identical everywhere. This protects + case-insensitive filesystems, but leaks the old spelling where both names can + coexist. +- Detect filesystem case behavior and rename through a temporary name. This can + support case-only catalog changes, but adds another crash-consistent mutation + protocol for a catalog shape the publisher can avoid. + +## 2026-08-09 — Ownership cleanup preserves all directories + +**TL;DR:** The download ledger authorizes deletion of exact regular files only; +cleanup does not prune their now-empty parent directories. + +An empty directory may have existed before Lanspread placed an owned file in it, +and the current ledger cannot prove directory provenance. Leaving an empty +directory is harmless, while deleting a user-created directory violates the +fail-safe ownership boundary. + +Alternatives: + +- Delete every empty ancestor of an owned file. This keeps roots tidy but can + delete a user-owned empty directory. +- Journal directory ownership and creation state. This supports exact pruning, + but expands the crash protocol and still needs a policy for pre-existing + manifest directories. + +## 2026-08-09 — A baseline record distinguishes new and legacy scratch + +**TL;DR:** Before parking an existing `version.ini`, the new downloader durably +writes at least an empty ownership record. Recovery restores a parked sentinel +only when that valid baseline exists; scratch beside a missing or invalid record +is discarded without making payload bytes ready. + +Older builds used `.version.ini.discarded` but could leave it behind after they +had already truncated or overwritten payload files. The scratch filename alone +therefore cannot prove that mutation never began. The baseline makes every park +performed by the new transaction format distinguishable. + +Alternatives: + +- Restore every discarded sentinel when no journal exists. This preserves a + clean crash between park and journaling, but can advertise a legacy partial + payload as a complete game. +- Never restore a discarded sentinel. This is fail-safe for upgrades, but would + unnecessarily discard a known-clean sentinel after a new-format pre-journal + crash. +- Introduce another dedicated phase-marker file. This is equally expressive, but + adds a second persistent transaction artifact where an empty valid ledger + already provides the needed proof. diff --git a/organize/unsorted/FINDINGS.md b/organize/unsorted/FINDINGS.md index b641058..93f1073 100644 --- a/organize/unsorted/FINDINGS.md +++ b/organize/unsorted/FINDINGS.md @@ -2,21 +2,6 @@ ## Open -### Crash-during-download leaves orphan archive files - -`crates/lanspread-peer/src/install/transaction.rs:329` — -`recover_download_transients` sweeps only `.version.ini.tmp` and -`.version.ini.discarded` on startup. The new cancel-cleanup -(`download/storage.rs::discard_cancelled_download`) is only invoked from the -in-flight orchestrator, so a crash mid-download leaves partial `.eti` archives -in the game root. After restart the user sees a game that looks half-downloaded -with no way to clean it up except `RemoveDownloadedGame`. Closing this would -mean calling the same discard pass during recovery for any game root whose -intent is `None` and whose `version.ini` is absent. - -Not blocking. The cancel-button fix is correct in its scope; this is the -symmetric crash-recovery case. - ### `handleErrorEvent` still writes status fields directly `crates/lanspread-tauri-deno-ts/src/hooks/useGames.ts:80-89` — the error handler @@ -37,8 +22,12 @@ No out-of-scope code smells or issues were identified in Claude's review. All four points were direct follow-up cleanup for the current protocol change and were handled in code. -The previous three findings have landed in code and tests: +The previous four findings have landed in code and tests: +- Download ownership is now journaled before payload mutation. Cancellation and + startup recovery remove only exact downloader-owned paths and preserve unknown + root files, instead of leaving crashed partial archives or broadly deleting + the game root. - `update_game` now uses `PeerCommand::FetchLatestFromPeers` to skip local manifest serving and fetch fresh peer metadata. Covered by `update_fetch_emits_fresh_manifest_from_latest_peer` and