From 0fd368b6faf852adcee7486ede9917d5ea518d76 Mon Sep 17 00:00:00 2001
From: ddidderr <ddidderr@paul.network>
Date: Sat, 12 Sep 2026 11:17:12 +0200
Subject: [PATCH] fix(proto): apply catalog component rules to wire game IDs

Follow-up to EXP2-SEC-04 (wire game IDs). The wire validator rejected
separators, control characters and the `.`/`..` pseudo-components and
otherwise relied on the catalog lookup that follows every request. That
lookup is real, but it leaves a class of IDs on the wire that the catalog
itself would never publish: Windows device names (`CON`, `nul.txt`,
`com1`), trailing dots or spaces, and the Windows-reserved characters
`< > : " | ? *`. Rejecting them at the protocol boundary means a
filesystem-backed handler can never see one, whichever consumer is added
next.

`validate_game_id` now also calls
`lanspread_db::content_manifest::validate_portable_component`, the same
function the catalog uses to admit game IDs and that `path_validation.rs`
already reuses. Reusing it rather than copying the device-name table
(the parallel branch grew three private copies) guarantees the wire rule
can neither over-match nor drift: an ID the catalog accepts always
encodes. The existing `InvalidPathComponent` error variant is reused so
callers and logs are unchanged.

Tests add the newly rejected forms and a positive list of catalog-valid
IDs that must keep encoding: embedded dots and spaces (`game..v1
(final)`), `console.txt`, `com10` and a non-ASCII name. The fixture
catalogs and the peer test suite, which encode many request IDs, pass
unchanged.

Test plan:
- `cargo test -p lanspread-proto`: 24 passed.
- `cargo test -p lanspread-peer -p lanspread-peer-cli`: 491 + 15 + 21
  passed.
- `cargo clippy -p lanspread-proto --all-targets -- -D warnings`: clean.

Claude-Session: https://claude.ai/code/session_01QRkCv4a4GqkajyamxmbSuA
---
 crates/lanspread-proto/src/lib.rs | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/crates/lanspread-proto/src/lib.rs b/crates/lanspread-proto/src/lib.rs
index 2fe55f7..7dd55a7 100644
--- a/crates/lanspread-proto/src/lib.rs
+++ b/crates/lanspread-proto/src/lib.rs
@@ -6,6 +6,7 @@ use std::{
 };
 
 use bytes::Bytes;
+use lanspread_db::content_manifest::validate_portable_component;
 pub use lanspread_db::content_manifest::{CanonicalCatalogPath, ContentId};
 use serde::{
     Deserialize,
@@ -945,8 +946,13 @@ impl CallToPlayAuthorEvent {
 
 /// Game IDs name one catalog directory. The wire boundary therefore admits
 /// only a single plain path component: no separators, no control characters,
-/// and neither of the `.`/`..` pseudo-components. Every consumer still resolves
-/// the ID against the local catalog before touching the filesystem.
+/// and neither of the `.`/`..` pseudo-components. On top of that it applies
+/// the catalog's own portable component rules (no trailing dot or space, no
+/// Windows-reserved characters, no Windows device name stem) by calling the
+/// `lanspread-db` validator, so anything the catalog can publish is accepted
+/// and anything it would refuse never reaches a filesystem-backed handler.
+/// Every consumer still resolves the ID against the local catalog before
+/// touching the filesystem; this is defence in depth, not the only gate.
 fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
     if game_id.trim().is_empty() {
         return Err(ControlValidationError::EmptyField { field: "game ID" });
@@ -961,6 +967,7 @@ fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
         || game_id
             .chars()
             .any(|character| character.is_control() || matches!(character, '/' | '\\'))
+        || validate_portable_component(game_id).is_err()
     {
         return Err(ControlValidationError::InvalidPathComponent { field: "game ID" });
     }
@@ -1740,6 +1747,14 @@ mod tests {
             "dir\\game",
             "game\0",
             "game\n",
+            "game.",
+            "game ",
+            "CON",
+            "nul.txt",
+            "com1",
+            "C:game",
+            "game?",
+            "game*",
         ] {
             assert!(
                 matches!(
@@ -1755,12 +1770,14 @@ mod tests {
                 "accepted game ID {game_id:?}"
             );
         }
-        Request::StreamInstall {
-            game_id: "game..v1 (final)".to_owned(),
-            content_id: content(1),
+        for game_id in ["game..v1 (final)", "console.txt", "com10", "Jörg"] {
+            Request::StreamInstall {
+                game_id: game_id.to_owned(),
+                content_id: content(1),
+            }
+            .encode()
+            .expect("catalog-valid game ID should encode");
         }
-        .encode()
-        .expect("plain component with embedded dots should encode");
 
         let mut snapshot = state_snapshot(Vec::new());
         snapshot.call_to_play.display_name = "é".repeat(MAX_CALL_TO_PLAY_DISPLAY_NAME_CHARS);