From 63aa4bc77cc788e5332834825a40528ec93defaf Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 12 Sep 2026 11:12:14 +0200 Subject: [PATCH] fix(peer): serve manifests from the validated cache only Follow-up to the scanner finding #2 fix ("rejected bulk requests can populate the persistent manifest cache"). The previous commit reordered admission so the compact content index and local readiness are checked before any manifest body is loaded. That relied on ordering alone: the loader still called `CatalogBundle::manifest`, which reads and parses the artifact from disk on a cache miss, and nothing in the test suite proved that a rejected request leaves the cache untouched. Switch the outbound admission loader to `cached_manifest`, which never performs filesystem I/O. This is safe because every game that can pass `can_serve_game` is catalog-eligible and its manifest was primed by `prime_library_manifests` before the library revision that advertises it became visible (server startup in `services/server.rs`, every library scan publication in `handlers.rs`). A cache miss therefore means a publication-ordering bug rather than a legitimate serve, and failing the request closed with a logged error is the right outcome. The admission tests now assert that a request with the wrong content identity leaves `cached_manifest("game")` erroring, for both catalog chunks and Stream Install, and prime the manifest explicitly before the accepted-request assertions, mirroring what the server does. The architecture document describes the cache-only serving path. Behaviour visible to peers is unchanged for valid requests. Rejected requests no longer cause a disk read under the admission lock. This layers the `cached_manifest` switch and no-load assertions from the parallel security branch (lanspread2 commit 4e0419a) onto the identity gate introduced in 55fa494. Test plan: - `cargo test -p lanspread-peer --lib`: 488 passed. - `just test`, `just clippy`, `cargo +nightly fmt --check` at the end of the series. Claude-Session: https://claude.ai/code/session_01QRkCv4a4GqkajyamxmbSuA --- crates/lanspread-peer/ARCHITECTURE.md | 9 +++++-- .../lanspread-peer/src/services/transfer.rs | 25 ++++++++++++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/crates/lanspread-peer/ARCHITECTURE.md b/crates/lanspread-peer/ARCHITECTURE.md index 18045dc..795d8a7 100644 --- a/crates/lanspread-peer/ARCHITECTURE.md +++ b/crates/lanspread-peer/ARCHITECTURE.md @@ -286,8 +286,13 @@ Most scans become O(number of game dirs), with full recursion only when needed. replace that byte authority. - Authenticated remote-availability joins use the compact index only. Unknown or mismatched `(game_id, content_id)` pairs are rejected without filesystem I/O - or manifest parsing; actual serving, download planning, and Stream Install - still load and validate the full body. + or manifest parsing. Outbound serving (chunk and Stream Install admission) + also gates on the compact index and in-memory local readiness first and then + reads the manifest body from the validated cache only: every publishable + game's body was primed before its library revision became visible, so an + anonymous request can neither trigger a disk read nor populate the manifest + cache. Download planning on the requesting side still loads and validates the + full body on demand. - The local catalog artifact is converted into a `ValidatedDownloadManifest` before any destination mutation. It contains canonical game-root-relative paths and rejects aliases, reserved state, shape conflicts, and bounded-size diff --git a/crates/lanspread-peer/src/services/transfer.rs b/crates/lanspread-peer/src/services/transfer.rs index 13d9f56..6826ab0 100644 --- a/crates/lanspread-peer/src/services/transfer.rs +++ b/crates/lanspread-peer/src/services/transfer.rs @@ -86,13 +86,18 @@ fn content_identity_matches( } } +/// Resolves the manifest of a game that has already passed the identity and +/// local-readiness gates. Every publishable game had its manifest primed by +/// `prime_library_manifests` before its library revision became visible, so +/// this reads the validated cache only and never touches disk on the +/// public request path. fn load_expected_catalog_manifest( ctx: &PeerCtx, game_id: &str, ) -> Option> { let catalog = Arc::clone(&ctx.catalog); let manifest_game_id = game_id.to_owned(); - match scoped_blocking(move || catalog.manifest(&manifest_game_id)) { + match scoped_blocking(move || catalog.cached_manifest(&manifest_game_id)) { Ok(manifest) => Some(manifest), Err(error) => { log::error!("Failed to load catalog content manifest for {game_id}: {error}"); @@ -740,6 +745,10 @@ mod tests { let (ctx, mut events) = test_peer_ctx(temp.path(), &manifest, Arc::clone(&provider)).await; assert_chunk_rejected(&ctx, ContentId::from_bytes([9; 32]), &payload, 0, 7).await; + assert!( + ctx.catalog.cached_manifest("game").is_err(), + "wrong content identity must not load a manifest body" + ); assert!( admit_outbound_transfer( &ctx, @@ -872,6 +881,10 @@ mod tests { .is_none(), "wrong-content StreamInstall must be rejected" ); + assert!( + ctx.catalog.cached_manifest("game").is_err(), + "wrong StreamInstall identity must not load a manifest body" + ); assert!( admit_outbound_transfer( &ctx, @@ -889,6 +902,9 @@ mod tests { std::fs::write(game_root.join("payload.bin"), b"payload") .expect("valid payload should be restored"); + ctx.catalog + .manifest("game") + .expect("the server would preload the publishable manifest"); let admitted = admit_outbound_transfer( &ctx, "game", @@ -941,10 +957,17 @@ mod tests { .is_none(), "a stream-capable manifest must still reject the wrong content identity" ); + assert!( + ctx.catalog.cached_manifest("game").is_err(), + "wrong StreamInstall identity must not load a manifest body" + ); assert!(ctx.active_outbound_transfers.read().await.is_empty()); assert_eq!(provider.calls.load(Ordering::SeqCst), 0); assert!(events.try_recv().is_err()); + ctx.catalog + .manifest("game") + .expect("the server would preload the publishable manifest"); let admitted = admit_outbound_transfer( &ctx, "game",