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
This commit is contained in:
@@ -6,6 +6,7 @@ use std::{
|
|||||||
};
|
};
|
||||||
|
|
||||||
use bytes::Bytes;
|
use bytes::Bytes;
|
||||||
|
use lanspread_db::content_manifest::validate_portable_component;
|
||||||
pub use lanspread_db::content_manifest::{CanonicalCatalogPath, ContentId};
|
pub use lanspread_db::content_manifest::{CanonicalCatalogPath, ContentId};
|
||||||
use serde::{
|
use serde::{
|
||||||
Deserialize,
|
Deserialize,
|
||||||
@@ -945,8 +946,13 @@ impl CallToPlayAuthorEvent {
|
|||||||
|
|
||||||
/// Game IDs name one catalog directory. The wire boundary therefore admits
|
/// Game IDs name one catalog directory. The wire boundary therefore admits
|
||||||
/// only a single plain path component: no separators, no control characters,
|
/// only a single plain path component: no separators, no control characters,
|
||||||
/// and neither of the `.`/`..` pseudo-components. Every consumer still resolves
|
/// and neither of the `.`/`..` pseudo-components. On top of that it applies
|
||||||
/// the ID against the local catalog before touching the filesystem.
|
/// 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> {
|
fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
|
||||||
if game_id.trim().is_empty() {
|
if game_id.trim().is_empty() {
|
||||||
return Err(ControlValidationError::EmptyField { field: "game ID" });
|
return Err(ControlValidationError::EmptyField { field: "game ID" });
|
||||||
@@ -961,6 +967,7 @@ fn validate_game_id(game_id: &str) -> Result<(), ControlValidationError> {
|
|||||||
|| game_id
|
|| game_id
|
||||||
.chars()
|
.chars()
|
||||||
.any(|character| character.is_control() || matches!(character, '/' | '\\'))
|
.any(|character| character.is_control() || matches!(character, '/' | '\\'))
|
||||||
|
|| validate_portable_component(game_id).is_err()
|
||||||
{
|
{
|
||||||
return Err(ControlValidationError::InvalidPathComponent { field: "game ID" });
|
return Err(ControlValidationError::InvalidPathComponent { field: "game ID" });
|
||||||
}
|
}
|
||||||
@@ -1740,6 +1747,14 @@ mod tests {
|
|||||||
"dir\\game",
|
"dir\\game",
|
||||||
"game\0",
|
"game\0",
|
||||||
"game\n",
|
"game\n",
|
||||||
|
"game.",
|
||||||
|
"game ",
|
||||||
|
"CON",
|
||||||
|
"nul.txt",
|
||||||
|
"com1",
|
||||||
|
"C:game",
|
||||||
|
"game?",
|
||||||
|
"game*",
|
||||||
] {
|
] {
|
||||||
assert!(
|
assert!(
|
||||||
matches!(
|
matches!(
|
||||||
@@ -1755,12 +1770,14 @@ mod tests {
|
|||||||
"accepted game ID {game_id:?}"
|
"accepted game ID {game_id:?}"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
Request::StreamInstall {
|
for game_id in ["game..v1 (final)", "console.txt", "com10", "Jörg"] {
|
||||||
game_id: "game..v1 (final)".to_owned(),
|
Request::StreamInstall {
|
||||||
content_id: content(1),
|
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());
|
let mut snapshot = state_snapshot(Vec::new());
|
||||||
snapshot.call_to_play.display_name = "é".repeat(MAX_CALL_TO_PLAY_DISPLAY_NAME_CHARS);
|
snapshot.call_to_play.display_name = "é".repeat(MAX_CALL_TO_PLAY_DISPLAY_NAME_CHARS);
|
||||||
|
|||||||
Reference in New Issue
Block a user