diff --git a/crates/lanspread-peer/ARCHITECTURE.md b/crates/lanspread-peer/ARCHITECTURE.md index 795d8a7..f92d0d7 100644 --- a/crates/lanspread-peer/ARCHITECTURE.md +++ b/crates/lanspread-peer/ARCHITECTURE.md @@ -233,7 +233,11 @@ Reserved per-game paths: - `.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. + atomic per-game intent log. Every `games//` path is derived from a + game ID that satisfies the catalog's portable component rules; the public + `setup_done_path` and the launch-settings entry points validate the ID before + joining it, and legacy migration refuses a games-folder directory whose name + would not pass. - `games//download_ownership/v1-/record.json` in that state directory records the last committed and any pending downloader-owned regular-file set and its exact catalog content ID. The namespace is derived diff --git a/crates/lanspread-peer/src/handlers.rs b/crates/lanspread-peer/src/handlers.rs index 9206cfe..365235e 100644 --- a/crates/lanspread-peer/src/handlers.rs +++ b/crates/lanspread-peer/src/handlers.rs @@ -3280,7 +3280,7 @@ mod tests { fn streamed_install_marks_settings_only_after_successful_promotion() { let games = TempDir::new("lanspread-handler-stream-promotion"); let state = TempDir::new("lanspread-handler-stream-promotion-state"); - let marker = crate::launch_settings_applied_path(state.path(), "game"); + let marker = crate::state_paths::launch_settings_applied_path(state.path(), "game"); let transaction = install::begin_streamed_install(&games.game_root(), state.path(), "game") .expect("streamed install transaction should begin"); write_file(&transaction.staging_dir().join("payload.txt"), b"installed"); diff --git a/crates/lanspread-peer/src/launch_settings.rs b/crates/lanspread-peer/src/launch_settings.rs index 338d866..718e4b9 100644 --- a/crates/lanspread-peer/src/launch_settings.rs +++ b/crates/lanspread-peer/src/launch_settings.rs @@ -24,7 +24,7 @@ use eyre::WrapErr; use crate::{ game_paths::LOCAL_DIR, scoped_blocking::scoped_blocking, - state_paths::launch_settings_applied_path, + state_paths::{launch_settings_applied_path, validate_game_state_id}, }; const ACCOUNT_NAME_FILE: &str = "account_name.txt"; @@ -141,6 +141,7 @@ pub fn apply_launch_settings_once( account_name: Option<&str>, language: Option<&str>, ) -> eyre::Result { + validate_game_state_id(game_id)?; scoped_blocking(|| { apply_launch_settings_once_blocking(state_dir, game_root, game_id, account_name, language) }) @@ -172,6 +173,7 @@ pub fn apply_launch_settings_to_verified_tree( /// recoverable: leaving it absent makes the existing first-play path retry the /// rewrite. pub fn mark_launch_settings_applied(state_dir: &Path, game_id: &str) -> eyre::Result<()> { + validate_game_state_id(game_id)?; scoped_blocking(|| mark_applied(&launch_settings_applied_path(state_dir, game_id))) } diff --git a/crates/lanspread-peer/src/lib.rs b/crates/lanspread-peer/src/lib.rs index 9bd406c..89f868f 100644 --- a/crates/lanspread-peer/src/lib.rs +++ b/crates/lanspread-peer/src/lib.rs @@ -125,7 +125,7 @@ pub use crate::{ mark_launch_settings_applied, }, startup::PeerRuntimeHandle, - state_paths::{launch_settings_applied_path, setup_done_path}, + state_paths::setup_done_path, stream_install::{ ExternalUnrarStreamProvider, NoopStreamInstallProvider, diff --git a/crates/lanspread-peer/src/migration.rs b/crates/lanspread-peer/src/migration.rs index 8d66dc6..7b5efe0 100644 --- a/crates/lanspread-peer/src/migration.rs +++ b/crates/lanspread-peer/src/migration.rs @@ -206,7 +206,17 @@ fn note_legacy_install_intent(root: &Path) -> MigrationReport { fn migrate_setup_marker(state_dir: &Path, id: &str, root: &Path) -> MigrationReport { let mut report = MigrationReport::default(); let legacy_path = root.join("local").join(LEGACY_FIRST_START_DONE_FILE); - let target_path = setup_done_path(state_dir, id); + // Roots are discovered from directory names in the games folder, so an + // arbitrary name must be refused here rather than joined below the state + // directory. + let target_path = match setup_done_path(state_dir, id) { + Ok(path) => path, + Err(error) => { + log::warn!("Refusing setup marker migration for unsafe game ID {id:?}: {error}"); + report.failures += 1; + return report; + } + }; match scoped_blocking(|| migrate_empty_marker(&legacy_path, &target_path)) { Ok(MigrationOutcome::Migrated) => { @@ -521,7 +531,11 @@ mod tests { assert_eq!(report.install_intents_migrated, 0); assert_eq!(report.failures, 2); assert_eq!(report.setup_markers_migrated, 1); - assert!(setup_done_path(state.path(), "game").is_file()); + assert!( + setup_done_path(state.path(), "game") + .expect("setup marker path should be valid") + .is_file() + ); assert!(legacy_intent.exists()); assert!(legacy_tmp.exists()); assert!(!legacy_setup.exists()); @@ -555,6 +569,32 @@ mod tests { assert_eq!(second.failures, 0); } + #[tokio::test] + async fn unsafe_root_name_is_refused_without_panicking_or_touching_state() { + let games = TempDir::new("lanspread-migration-games"); + let state = TempDir::new("lanspread-migration-state"); + // A trailing dot is a legal directory name on Unix but is not a + // portable game ID; joining it below the state directory must be + // refused rather than attempted. + let legacy_marker = games + .path() + .join("game.") + .join("local") + .join(LEGACY_FIRST_START_DONE_FILE); + write_file(&legacy_marker, b""); + + let report = migrate_legacy_state(games.path(), state.path()).await; + + assert_eq!(report.games_checked, 1); + assert_eq!(report.failures, 1); + assert_eq!(report.setup_markers_migrated, 0); + assert!(legacy_marker.exists(), "legacy marker must be kept"); + assert!( + !state.path().join("games").exists(), + "no per-game state may be created for an unsafe ID" + ); + } + #[tokio::test] async fn app_state_wins_over_legacy_per_game_state() { let games = TempDir::new("lanspread-migration-games"); @@ -571,7 +611,10 @@ mod tests { &legacy_intent_path, br#"{"schema_version":1,"state":"Installing"}"#, ); - write_file(&setup_done_path(state.path(), "game"), b""); + write_file( + &setup_done_path(state.path(), "game").expect("setup marker path should be valid"), + b"", + ); write_file(&legacy_setup, b""); let report = migrate_legacy_state(games.path(), state.path()).await; diff --git a/crates/lanspread-peer/src/state_paths.rs b/crates/lanspread-peer/src/state_paths.rs index 5e29b97..c9d42fa 100644 --- a/crates/lanspread-peer/src/state_paths.rs +++ b/crates/lanspread-peer/src/state_paths.rs @@ -1,5 +1,7 @@ use std::path::{Path, PathBuf}; +use lanspread_db::content_manifest::validate_portable_component; + const PEER_IDENTITY_FILE: &str = "peer-identity-v1.json"; const LOCAL_LIBRARY_DIR: &str = "local_library"; const LOCAL_LIBRARY_INDEX_FILE: &str = "index.json"; @@ -55,21 +57,56 @@ pub(crate) fn local_library_index_path(state_dir: &Path) -> PathBuf { .join(LOCAL_LIBRARY_INDEX_FILE) } +/// Joins a per-game state directory below `/games`. +/// +/// Callers inside this crate pass IDs that come from the catalog or have +/// already passed [`validate_game_state_id`]; the debug assertion documents +/// that contract without turning a bad ID into a release-build panic. pub(crate) fn game_state_dir(state_dir: &Path, game_id: &str) -> PathBuf { + debug_assert!( + validate_game_state_id(game_id).is_ok(), + "game_state_dir called with an unvalidated game ID: {game_id:?}" + ); games_state_dir(state_dir).join(game_id) } +/// Rejects a game ID that could escape or alias its per-game state +/// directory: separators, NUL, `.`/`..`, trailing dots or spaces, control or +/// Windows-reserved characters and Windows device names. +/// +/// The rules are the catalog's own portable component rules, so every ID a +/// catalog can publish is accepted and the check cannot drift from the +/// validator that admits game IDs in the first place. +/// +/// # Errors +/// +/// Returns the first violated rule. +pub(crate) fn validate_game_state_id(game_id: &str) -> eyre::Result<()> { + if game_id.contains(['/', '\\', '\0']) { + eyre::bail!("game ID must be one path component: {game_id:?}"); + } + validate_portable_component(game_id) +} + pub(crate) fn games_state_dir(state_dir: &Path) -> PathBuf { state_dir.join(GAMES_DIR) } -#[must_use] -pub fn setup_done_path(state_dir: &Path, game_id: &str) -> PathBuf { - game_state_dir(state_dir, game_id).join(SETUP_DONE_FILE) +/// Path of the marker that records a completed one-time `game_setup` run. +/// +/// This is the only state path exposed to embedding applications, whose game +/// IDs may originate from UI input rather than the catalog, so it validates +/// the ID before joining it below the state directory. +/// +/// # Errors +/// +/// Returns an error when `game_id` is not a single portable path component. +pub fn setup_done_path(state_dir: &Path, game_id: &str) -> eyre::Result { + validate_game_state_id(game_id)?; + Ok(game_state_dir(state_dir, game_id).join(SETUP_DONE_FILE)) } -#[must_use] -pub fn launch_settings_applied_path(state_dir: &Path, game_id: &str) -> PathBuf { +pub(crate) fn launch_settings_applied_path(state_dir: &Path, game_id: &str) -> PathBuf { game_state_dir(state_dir, game_id).join(LAUNCH_SETTINGS_APPLIED_FILE) } @@ -188,4 +225,44 @@ mod tests { explicit ); } + + #[test] + fn setup_done_path_accepts_catalog_style_game_ids() { + let state_dir = Path::new("/state"); + assert_eq!( + setup_done_path(state_dir, "game").expect("plain ID should be accepted"), + Path::new("/state/games/game/setup_done") + ); + for game_id in ["game..v1 (final)", "console.txt", "com10", "Jörg"] { + assert!( + setup_done_path(state_dir, game_id).is_ok(), + "rejected catalog-valid game ID {game_id:?}" + ); + } + } + + #[test] + fn setup_done_path_rejects_escaping_game_ids() { + let state_dir = Path::new("/state"); + for game_id in [ + "", + ".", + "..", + "../outside", + "/outside", + r"nested\game", + "game/child", + "game\0", + "game.", + "game ", + "NUL", + "con.txt", + "a:b", + ] { + assert!( + setup_done_path(state_dir, game_id).is_err(), + "accepted unsafe game ID {game_id:?}" + ); + } + } } diff --git a/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs b/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs index 6441f5d..5bbeaa7 100644 --- a/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs +++ b/crates/lanspread-tauri-deno-ts/src-tauri/src/lib.rs @@ -1907,7 +1907,13 @@ async fn run_game_windows( return Ok(()); }; - let setup_done_file = lanspread_peer::setup_done_path(&state_dir, &id); + let setup_done_file = match lanspread_peer::setup_done_path(&state_dir, &id) { + Ok(path) => path, + Err(error) => { + log::warn!("Ignoring run request for unsafe game id {id}: {error}"); + return Ok(()); + } + }; if !setup_done_file.exists() && game_setup_bin.exists() { if !local_install_is_present(&game_path) { log::warn!(