From e86f3e6943262f001e53b7011c97856e53402c2d Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Sat, 29 Aug 2026 15:21:54 +0200 Subject: [PATCH] sync: a node adopts only paths inside its own include from a peer Finding 8 of the 2026-08-29 review, the receive-side half; finding 2 was the delete half. adopt took every winning entry a peer sent, whatever this node's include said, so a host with a broad include had its machine-local files adopted into every peer's manifest and relayed onward across the mesh. The README always described includes as the boundary; the code enforced it only on the scan. adopt_from_peer refuses a path outside this node's include, so it never enters the manifest and never crosses the wire to a third peer. The two wire sites (run_client's reply adopt and run_server's push adopt) use it; loading this node's OWN durable state still uses adopt, which keeps every path it already held even outside a narrowed include, because that record is the node's and not a peer's. The node's include is set from its entry config in load_node_and_observed and refreshed on every reload. Behaviour change: peers whose includes differ no longer converge on the excluded paths, and widening a receiver's include re-adopts from a peer on the next reconcile rather than materialising a path it had already taken. Proof: adopt_from_peer_refuses_a_path_outside_the_include (red without the filter) and adopt_keeps_the_nodes_own_path_even_outside_a_narrowed_include pin the two directions; a_path_outside_the_receivers_include_is_not_deleted now also asserts, over real iroh, that B's manifest holds only the included path. --- CHANGELOG.md | 18 +++++++ src/sync/engine.rs | 3 ++ src/sync/node.rs | 118 +++++++++++++++++++++++++++++++++++++++++++ src/sync/wire.rs | 4 +- tests/folder_sync.rs | 29 +++++++---- 5 files changed, 161 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aeff67a..6ae95a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -257,6 +257,24 @@ EXPERIMENTAL, so on-disk formats and the CLI may change without notice. ### Fixed +- **Include globs are now a receive-side boundary, not only a scan-side one.** A + node adopted every winning entry a peer sent, whatever its own include said, so + a host with a broad include (or a mistaken `["**"]`) had its machine-local + files taken into every peer's manifest and relayed onward across the mesh. The + README always said includes were the boundary; the code enforced it only on the + scan. `adopt_from_peer` now refuses a path outside the node's include, so it + never enters the manifest and never crosses the wire to a third peer. This is + the receive half of the same defect whose delete half was finding 2. Finding 8 + of the 2026-08-29 review. + + Kept distinct from loading a node's OWN durable state, which still adopts every + path it already held even outside a narrowed include, because that record is + the node's, not a peer's. **Behaviour change:** two peers whose includes differ + no longer converge on the excluded paths (which is the point), and widening a + receiver's include now re-adopts the newly-included paths from a peer on the + next reconcile rather than materialising them from a manifest it had already + taken. + - **`send-file` streams instead of holding the whole file in memory on both daemons.** The sender read the file whole with `std::fs::read` and the receiver allocated `header.len` bytes up front, so a 1.5 GiB transfer cost about 1.5 GiB diff --git a/src/sync/engine.rs b/src/sync/engine.rs index 923e92f..bb4a6a9 100644 --- a/src/sync/engine.rs +++ b/src/sync/engine.rs @@ -792,6 +792,9 @@ impl SyncEngine { HashMap, )> { let mut node = SyncNode::new(self.author); + // The receive-side include boundary, refreshed on every load and + // reload so a narrowed or widened include takes effect immediately. + node.set_include(cfg.include.clone()); if let Some(state) = self.read_durable_state(&cfg.name)? { // `adopt` records every path it takes, so loading from disk also // SEEDS the change buffer with the whole manifest. That is required, diff --git a/src/sync/node.rs b/src/sync/node.rs index 32d2a2c..b5cd328 100644 --- a/src/sync/node.rs +++ b/src/sync/node.rs @@ -40,6 +40,10 @@ pub struct SyncNode { author: Author, manifest: Manifest, content: HashMap>, + /// The include globs this node's entry selects, if any. A receive-side + /// boundary: `adopt_from_peer` will not take a path outside it. `None` + /// means no include is configured, so every path is in scope. + include: Option>, /// Which paths changed here, and which peer has seen them. changes: ChangeBuffer, /// Payloads this node has SENT that carried its entire manifest. @@ -117,6 +121,7 @@ impl SyncNode { author, manifest: Manifest::new(), content: HashMap::new(), + include: None, changes: ChangeBuffer::new(), full_payload_sends: 0, } @@ -450,6 +455,25 @@ impl SyncNode { /// adopted. Content for any newly-present entry is fetched separately (over /// the wire) or is already held; an entry with no available content simply /// does not materialize until its bytes arrive. + /// Set the receive-side include boundary this node enforces on peer + /// adopts. Called from the engine when the node is built from its entry + /// config, and again on every reload, so it always reflects the current + /// `syncs.toml`. + pub fn set_include(&mut self, include: Option>) { + self.include = include; + } + + fn peer_path_in_scope(&self, path: &str) -> bool { + match &self.include { + None => true, + Some(globs) => crate::sync::glob::matches_any(globs, path), + } + } + + /// Adopt every entry from `remote` that wins over ours, for LOADING THIS + /// NODE'S OWN STATE. No include filter: a path this machine already + /// recorded stays recorded even if the include later narrowed, because + /// dropping it here would lose this node's own record of it. pub fn adopt(&mut self, remote: &Manifest) -> usize { let diff = self.manifest.diff_from(remote); let adopted = diff.adopt.len(); @@ -463,6 +487,36 @@ impl SyncNode { adopted } + /// Adopt what a PEER offers, refusing any path outside this node's include. + /// + /// This is the receive-side boundary the README always described and the + /// code did not have. A host with a broad include (or a mistaken `["**"]`) + /// used to have its machine-local files adopted into every peer's manifest + /// and relayed onward, because `adopt` took every winning entry. Finding 8 + /// of the 2026-08-29 review; the delete half was finding 2. A path this + /// node does not select is never taken, so it never enters the manifest and + /// never crosses the wire to a third peer. + /// + /// Distinct from `adopt` on purpose: `adopt` loads this node's OWN durable + /// state and must keep every path it already held; this takes ANOTHER + /// node's paths and must honour the local boundary. + pub fn adopt_from_peer(&mut self, remote: &Manifest) -> usize { + let diff = self.manifest.diff_from(remote); + let mut adopted = 0; + for entry in diff.adopt { + if !self.peer_path_in_scope(&entry.path) { + continue; + } + self.changes.record(&entry.path); + self.manifest.insert(entry.path, entry.entry); + adopted += 1; + } + if adopted > 0 { + self.prune_unreferenced_content(); + } + adopted + } + /// Content hashes for the present entries of `delta` that this node holds. /// /// A DELTA PASS MUST USE THIS, never `hashes_peer_needs`. That function @@ -1526,4 +1580,68 @@ mod tests { } } } + + /// Finding 8 of the 2026-08-29 review, the receive-side half. A node adopts + /// only paths inside its own include from a peer; the delete half (finding + /// 2) is separate. So a peer's path this node does not select never enters + /// its manifest, and therefore never relays to a third peer. + #[test] + fn adopt_from_peer_refuses_a_path_outside_the_include() { + let mut a = node(1); + a.local_write("keep/shared.md", b"both want this", 0, 0); + a.local_write("pty/machine-local.events", b"A's local runtime", 0, 0); + + // B selects keep/** only. It is a real, reachable, healthy peer. + let mut b = node(2); + b.set_include(Some(vec!["keep/**".to_string()])); + let adopted = b.adopt_from_peer(a.manifest()); + + assert_eq!(adopted, 1, "only the included path should be taken"); + assert!(b.manifest().get("keep/shared.md").is_some()); + assert!( + b.manifest().get("pty/machine-local.events").is_none(), + "B took a path outside its include into its manifest; it would relay it onward" + ); + // A change buffer that never recorded the excluded path cannot offer it + // to a third peer: everything B would send from cursor zero is included. + let offered = b.changes().since(0); + assert!( + !offered.contains(&"pty/machine-local.events"), + "B would relay a path it should never have taken: {offered:?}" + ); + } + + /// The control that keeps the boundary honest: with no include configured, + /// adopt_from_peer takes everything, exactly like adopt. + #[test] + fn adopt_from_peer_with_no_include_takes_everything() { + let mut a = node(1); + a.local_write("keep/x.md", b"x", 0, 0); + a.local_write("pty/y.events", b"y", 0, 0); + let mut b = node(2); // include defaults to None + assert_eq!(b.adopt_from_peer(a.manifest()), 2); + assert!(b.manifest().get("pty/y.events").is_some()); + } + + /// The distinction the fix rests on: loading this node's OWN durable state + /// with `adopt` must keep a path even when the include no longer selects it, + /// because that record is this node's, not a peer's. Losing it here would + /// forget a file this machine still holds. + #[test] + fn adopt_keeps_the_nodes_own_path_even_outside_a_narrowed_include() { + // The node's manifest as loaded from disk holds pty/local, and the + // include has since narrowed to keep/**. + let mut disk = node(1); + disk.local_write("keep/x.md", b"x", 0, 0); + disk.local_write("pty/local.events", b"mine", 0, 0); + + let mut loaded = node(1); + loaded.set_include(Some(vec!["keep/**".to_string()])); + // Loading own state uses `adopt`, not `adopt_from_peer`. + loaded.adopt(disk.manifest()); + assert!( + loaded.manifest().get("pty/local.events").is_some(), + "loading own durable state must not drop a path outside the current include" + ); + } } diff --git a/src/sync/wire.rs b/src/sync/wire.rs index 182c845..698341f 100644 --- a/src/sync/wire.rs +++ b/src/sync/wire.rs @@ -222,7 +222,7 @@ where // 3. Adopt the server's winning entries and bundle what the server needs. let (pulled, blobs_for_server, fallback, landing) = { let mut node = node.lock().await; - let pulled = node.adopt(&reply.manifest); + let pulled = node.adopt_from_peer(&reply.manifest); // What content to push. `hashes_peer_needs` infers what the peer lacks // by diffing against what it sent, which is only sound when it sent // EVERYTHING. Against a delta, every path outside the delta looks @@ -464,7 +464,7 @@ where } } let blobs = node.gather_content(&client_needs); - let pushed = node.adopt(&manifest); + let pushed = node.adopt_from_peer(&manifest); node.note_payload_sent(&server_payload); diff --git a/tests/folder_sync.rs b/tests/folder_sync.rs index c87b820..f671b59 100644 --- a/tests/folder_sync.rs +++ b/tests/folder_sync.rs @@ -917,12 +917,11 @@ async fn a_path_outside_the_receivers_include_is_not_deleted() -> Result<()> { // Now the real question. B holds an entry it cannot scan. Give it many // passes to do the wrong thing. // - // B used to WRITE the path and never scan it, and this test recorded that - // as "not a fault". It was the setup for a fault: the written path stayed - // protected on B, so a later local delete of it on B tombstoned it for A. - // B now leaves a path outside its include alone in every direction. The - // entry still carries it in B's manifest, so widening B's include later - // materializes it without a resend. + // B does not adopt a path outside its include at all now (finding 8). It is + // not written, and it is not even recorded in B's manifest, so B can never + // relay it to a third peer. Before, B took it into its manifest and only + // declined to write it; a broad-include host could then spread its + // machine-local files across the whole mesh through B. for _ in 0..25 { tokio::time::sleep(Duration::from_millis(200)).await; assert!( @@ -932,9 +931,7 @@ async fn a_path_outside_the_receivers_include_is_not_deleted() -> Result<()> { ); assert!( !b_folder.join("docs/pairing-api.md").exists(), - "B wrote a path outside its include. A written path is a protected \ - path, and a protected path outside the include is the shape that \ - turned a local delete into a fleet-wide one" + "B wrote a path outside its include" ); } assert_eq!( @@ -943,6 +940,20 @@ async fn a_path_outside_the_receivers_include_is_not_deleted() -> Result<()> { "the document survived but its content changed" ); + // The receive-side boundary, end to end: B's manifest holds only the + // included path, so there is nothing outside the include for B to relay. + let b_status = sync_status_of(&b_home).await?; + let shared = b_status + .iter() + .find(|e| e.name == "shared") + .expect("B has no status for the shared entry"); + assert_eq!( + shared.present, 1, + "B's manifest carries a path outside its include (present={}); it would \ + relay that path to any third peer", + shared.present + ); + node_b.shutdown().await?; node_a.shutdown().await?; Ok(())