From 62cd9306bd3d8916d16e2cd096b93655a0eada41 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sun, 9 Aug 2026 18:44:40 +0200 Subject: [PATCH] feat(peer): journal download file ownership Track the exact regular files owned by each completed and in-flight peer download instead of sweeping every non-reserved path after cancellation. Bind the record to the canonical games directory, publish pending ownership before payload mutation, and use the final version.ini rename as the recovery commit point. Make replacement, cancellation, and startup recovery preserve unknown files and install state while removing stale or partial downloader-owned bytes. Add a new-format baseline so legacy discarded sentinels cannot make partially modified payloads ready, sync payload and journal state in transaction order, and serialize startup recovery against operation admission. Document ambiguous legacy target adoption, portable alias transitions, and the other ownership tradeoffs in the refactor decision log. Test Plan: - `just clippy` -- passed - `just test` -- passed (182 peer-core tests plus the full workspace) - `just fmt` -- Rust, TOML, and Prettier formatting completed; the command then stopped on 40 pre-existing rumdl findings in unrelated Markdown content - `git diff --cached --check` -- passed --- crates/lanspread-peer/ARCHITECTURE.md | 13 +- crates/lanspread-peer/README.md | 39 +- .../lanspread-peer/src/download/manifest.rs | 22 + crates/lanspread-peer/src/download/mod.rs | 2 + .../src/download/orchestrator.rs | 98 +- .../lanspread-peer/src/download/ownership.rs | 978 ++++++++++++++++++ crates/lanspread-peer/src/download/storage.rs | 193 +--- .../src/download/version_ini.rs | 138 ++- crates/lanspread-peer/src/game_paths.rs | 33 - crates/lanspread-peer/src/handlers.rs | 18 +- .../lanspread-peer/src/install/transaction.rs | 25 +- crates/lanspread-peer/src/state_paths.rs | 10 + .../PEER-AUTH-REFACTOR-DECISIONS.md | 153 +++ organize/unsorted/FINDINGS.md | 21 +- 14 files changed, 1478 insertions(+), 265 deletions(-) create mode 100644 crates/lanspread-peer/src/download/ownership.rs 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