From c8162d79550ff4d1cb0b9b7624b449f58beb9bb4 Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 09:54:11 -0600 Subject: [PATCH 1/3] fix(id/artifact): bound home_declared world_id and refuse duplicate endpoints (#3187) --- pkgs/id/src/artifact.rs | 87 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/pkgs/id/src/artifact.rs b/pkgs/id/src/artifact.rs index ca02bb3bb..e28d1fe15 100644 --- a/pkgs/id/src/artifact.rs +++ b/pkgs/id/src/artifact.rs @@ -57,6 +57,8 @@ pub const FILE: &str = "artifacts.jsonl"; pub const PAGE_BYTES: usize = 256 * 1024; /// Home servers one declaration may name. pub const MAX_HOME_SERVERS: usize = 8; +/// The largest world ID one home server may carry, in bytes. +pub const MAX_WORLD_ID_BYTES: usize = 64; /// Hashes one artifact may name in `after`. pub const MAX_AFTER: usize = 32; /// The largest content a document may carry, as serialized JSON. @@ -372,11 +374,36 @@ impl SignedArtifact { "a declaration names at most {MAX_HOME_SERVERS} servers" ))); } - for server in &body.servers { + for (index, server) in body.servers.iter().enumerate() { check_target(&server.endpoint) .map_err(|error| forbidden(format!("{error:#}")))?; - if server.world_id.is_empty() { - return Err(forbidden("a home server needs a world ID")); + if body.servers[..index] + .iter() + .any(|prior| prior.endpoint == server.endpoint) + { + return Err(forbidden( + "a declaration names each endpoint at most once", + )); + } + if server.world_id.is_empty() || server.world_id.len() > MAX_WORLD_ID_BYTES { + return Err(forbidden(format!( + "a home server's world ID is 1 to {MAX_WORLD_ID_BYTES} bytes" + ))); + } + if server.world_id.trim().is_empty() { + return Err(forbidden( + "a home server's world ID must not be whitespace only", + )); + } + if server.world_id.chars().any(char::is_control) { + return Err(forbidden( + "a home server's world ID must not contain control characters", + )); + } + if server.world_id.chars().any(crate::is_format_char) { + return Err(forbidden( + "a home server's world ID must not contain format characters", + )); } } Ok(Some(Kind::HomeDeclared(body))) @@ -1903,6 +1930,60 @@ mod tests { assert_eq!(directory.artifacts().head_seq(&public(1)), 1); } + fn home_declared_with(servers: Vec) -> SignedArtifact { + ArtifactLog::default() + .sign( + &key(1), + &Kind::HomeDeclared(HomeDeclared { + account: public(1), + servers, + }), + ) + .unwrap() + } + + fn home_server(endpoint: String, world_id: &str) -> HomeServer { + HomeServer { + endpoint, + world_id: world_id.to_owned(), + } + } + + #[test] + fn parsed_home_world_id_is_bounded_and_clean() { + let control = home_declared_with(vec![home_server(endpoint_of(7), "home")]); + assert!(matches!(control.parsed(), Ok(Some(Kind::HomeDeclared(_))))); + + let at_bound = home_declared_with(vec![home_server(endpoint_of(7), &"w".repeat(64))]); + assert!(matches!(at_bound.parsed(), Ok(Some(Kind::HomeDeclared(_))))); + + for world_id in [ + "w".repeat(65), + " ".to_owned(), + "home\nFORGED".to_owned(), + "ho\u{1}me".to_owned(), + "home\u{202e}".to_owned(), + ] { + let refused = home_declared_with(vec![home_server(endpoint_of(7), &world_id)]); + assert!(refused.parsed().is_err(), "world_id {world_id:?} accepted"); + } + } + + #[test] + fn parsed_home_declaration_refuses_duplicate_endpoints() { + let distinct = home_declared_with(vec![ + home_server(endpoint_of(7), "home"), + home_server(endpoint_of(8), "lobby"), + ]); + assert!(matches!(distinct.parsed(), Ok(Some(Kind::HomeDeclared(_))))); + + let duplicate = home_declared_with(vec![ + home_server(endpoint_of(7), "home"), + home_server(endpoint_of(7), "lobby"), + ]); + assert!(duplicate.parsed().is_err()); + } + #[test] fn parsed_document_media_type_must_be_nfc() { let nfc = ArtifactLog::default() From 7e119b865681ac081a8a40a7f0e86bfdf6090c5a Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 10:02:26 -0600 Subject: [PATCH 2/3] fix(id/artifact): compare validated endpoint identities for duplicate refusal (#3187, Codex P2 on PR #4432) --- pkgs/id/src/artifact.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/pkgs/id/src/artifact.rs b/pkgs/id/src/artifact.rs index e28d1fe15..727740dc2 100644 --- a/pkgs/id/src/artifact.rs +++ b/pkgs/id/src/artifact.rs @@ -374,17 +374,21 @@ impl SignedArtifact { "a declaration names at most {MAX_HOME_SERVERS} servers" ))); } - for (index, server) in body.servers.iter().enumerate() { - check_target(&server.endpoint) + // Duplicate refusal compares the validated endpoint + // identities, not the raw strings: one node can be named + // by its bare ID or by an endpoint ticket for that ID. + let mut targets: Vec = Vec::with_capacity(body.servers.len()); + for server in &body.servers { + let target = check_target(&server.endpoint) .map_err(|error| forbidden(format!("{error:#}")))?; - if body.servers[..index] - .iter() - .any(|prior| prior.endpoint == server.endpoint) - { + if targets.iter().any(|prior| prior.id == target.id) { return Err(forbidden( "a declaration names each endpoint at most once", )); } + targets.push(target); + } + for server in &body.servers { if server.world_id.is_empty() || server.world_id.len() > MAX_WORLD_ID_BYTES { return Err(forbidden(format!( "a home server's world ID is 1 to {MAX_WORLD_ID_BYTES} bytes" @@ -1982,6 +1986,18 @@ mod tests { home_server(endpoint_of(7), "lobby"), ]); assert!(duplicate.parsed().is_err()); + + // The same node named once by bare ID and once by an endpoint + // ticket for that ID is still a duplicate. + let ticket = iroh_tickets::endpoint::EndpointTicket::new(EndpointAddr::from( + SecretKey::from_bytes(&[7; 32]).public(), + )) + .to_string(); + let aliased = home_declared_with(vec![ + home_server(endpoint_of(7), "home"), + home_server(ticket, "lobby"), + ]); + assert!(aliased.parsed().is_err()); } #[test] From ee4ec5e42381b293eb4534d9841816651b018cda Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 10:07:50 -0600 Subject: [PATCH 3/3] fix(id/artifact): keep #3187 home_declared bounds off the replay path (Codex P2 on PR #4432) --- pkgs/id/src/artifact.rs | 49 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/pkgs/id/src/artifact.rs b/pkgs/id/src/artifact.rs index 727740dc2..4a437cd87 100644 --- a/pkgs/id/src/artifact.rs +++ b/pkgs/id/src/artifact.rs @@ -332,6 +332,18 @@ impl SignedArtifact { /// Fails with [`Refusal::Forbidden`] when a known kind is malformed or its /// author may not make it. pub fn parsed(&self) -> Result> { + self.parsed_mode(false) + } + + /// [`SignedArtifact::parsed`] for history a log already holds: the + /// `home_declared` bounds added for #3187 do not apply, so a declaration + /// accepted before them still replays (mirroring how replay skips the + /// frame check in `store`). New receipts always go through `parsed`. + fn parsed_for_replay(&self) -> Result> { + self.parsed_mode(true) + } + + fn parsed_mode(&self, replay: bool) -> Result> { match self.kind.as_str() { FRIEND_REMOVE => { let body: FriendRemove = self.decode()?; @@ -381,7 +393,7 @@ impl SignedArtifact { for server in &body.servers { let target = check_target(&server.endpoint) .map_err(|error| forbidden(format!("{error:#}")))?; - if targets.iter().any(|prior| prior.id == target.id) { + if !replay && targets.iter().any(|prior| prior.id == target.id) { return Err(forbidden( "a declaration names each endpoint at most once", )); @@ -389,7 +401,13 @@ impl SignedArtifact { targets.push(target); } for server in &body.servers { - if server.world_id.is_empty() || server.world_id.len() > MAX_WORLD_ID_BYTES { + if server.world_id.is_empty() { + return Err(forbidden("a home server needs a world ID")); + } + if replay { + continue; + } + if server.world_id.len() > MAX_WORLD_ID_BYTES { return Err(forbidden(format!( "a home server's world ID is 1 to {MAX_WORLD_ID_BYTES} bytes" ))); @@ -492,7 +510,11 @@ impl ArtifactLog { /// before the check still loads. fn store(&mut self, artifact: SignedArtifact, frame_check: bool) -> Result { artifact.verify()?; - artifact.parsed()?; + if frame_check { + artifact.parsed()?; + } else { + artifact.parsed_for_replay()?; + } let size = encoded_len(&artifact); if frame_check && page_frame_bytes().saturating_add(size) > MAX_FRAME_BYTES { return Err(forbidden(format!( @@ -589,7 +611,7 @@ impl ArtifactLog { .get(account)? .iter() .rev() - .find_map(|artifact| match artifact.parsed() { + .find_map(|artifact| match artifact.parsed_for_replay() { Ok(Some(Kind::HomeDeclared(declared))) => Some(declared), _ => None, }) @@ -2000,6 +2022,25 @@ mod tests { assert!(aliased.parsed().is_err()); } + #[test] + fn a_legacy_home_declaration_still_replays() { + // A declaration accepted before the #3187 bounds (here: a world ID + // past MAX_WORLD_ID_BYTES) must not brick reopening its log: replay + // is lenient, while a fresh receipt of the same artifact is refused. + let dir = TempDir::new().unwrap(); + let path = dir.path().join(FILE); + let legacy = home_declared_with(vec![home_server(endpoint_of(7), &"w".repeat(65))]); + assert!(legacy.parsed().is_err()); + assert!(ArtifactLog::default().insert(legacy.clone()).is_err()); + append(&path, &legacy).unwrap(); + + let reopened = ArtifactLog::open(&path).unwrap(); + assert_eq!( + reopened.declared(&public(1)).unwrap().servers[0].world_id, + "w".repeat(65) + ); + } + #[test] fn parsed_document_media_type_must_be_nfc() { let nfc = ArtifactLog::default()