fix(daemon): ungated raw sync surface + unauthenticated off-host bind

daemon/src/collab_handler/mod.rs

@@ -1822,14 +1822,18 @@ async fn handle_doc_request_inner(
             .await
         }
 
-        "sync/diff" => sync_methods::handle_sync_diff(doc_store, id, &params).await,
+        "sync/diff" => {
+            sync_methods::handle_sync_diff(auth_principal, transport, doc_store, id, &params).await
+        }
 
         "docs/list" => docs_methods::handle_docs_list(doc_store, id).await,
 
         "docs/content" => docs_methods::handle_docs_content(doc_store, id, &params).await,
 
         "sync/resync" => {
             sync_methods::handle_sync_resync(
+                auth_principal,
+                transport,
                 doc_store,
                 broadcaster,
                 session_id,

daemon/src/collab_handler/sync_methods.rs

@@ -305,11 +305,23 @@ pub(super) async fn handle_sync_full_state(
 }
 
 pub(super) async fn handle_sync_diff(
+    auth_principal: Option<&str>,
+    transport: Transport,
     doc_store: &DocStore,
     id: serde_json::Value,
     params: &serde_json::Value,
 ) -> JsonRpcResponse {
     let doc_name = params["doc"].as_str().unwrap_or("default").to_string();
+    // @ai-caution: [security] `sync/diff` returns the same KB CRDT bytes
+    // `sync/state_vector` and `sync/full_state` are gated for. It was ungated,
+    // so a peer could read any other peer's KB node state by asking for a diff
+    // instead of a read — while `kb/node_fetch` on the same node correctly
+    // refused. `deny_kb_doc_read` only denies `kb:`/`kbc:` docs, so plain file
+    // collaboration is unaffected.
+    if let Err(msg) = deny_kb_doc_read(doc_store, &doc_name, auth_principal, transport).await {
+        warn!(doc = %doc_name, reason = %msg, "sync/diff denied");
+        return JsonRpcResponse::error(id, McpError::internal_error(msg));
+    }
     let sv_b64 = match params["sv"].as_str() {
         Some(s) => s,
         None => {
@@ -346,7 +358,14 @@ pub(super) async fn handle_sync_diff(
     }
 }
 
+// `deny_kb_doc_read` needs the principal and the transport, which pushes this
+// past the 7-argument lint. Splitting the signature into a params struct here
+// would obscure the call site for one lint, and the alternative — leaving the
+// gate off — is what this function is being fixed for.
+#[allow(clippy::too_many_arguments)]
 pub(super) async fn handle_sync_resync(
+    auth_principal: Option<&str>,
+    transport: Transport,
     doc_store: &DocStore,
     broadcaster: &SharedBroadcaster,
     session_id: u64,
@@ -358,15 +377,34 @@ pub(super) async fn handle_sync_resync(
     // BUG C fix: atomic state + sv under single lock (INV-2).
     let raw_name = params["doc"].as_str().unwrap_or("default").to_string();
     info!(session = session_id, doc = %raw_name, "sync/resync: processing");
+    // @ai-caution: [security] `sync/resync` returns FULL document state and
+    // additionally SUBSCRIBES the session to that doc's live broadcast. It was
+    // ungated, which made it both a read of any peer's KB node and a way to
+    // self-subscribe to every subsequent update to it. Gate before either.
+    // Checked on the raw name as well as the resolved one so suffix resolution
+    // cannot be used to reach a `kb:` doc by another spelling.
+    if let Err(msg) = deny_kb_doc_read(doc_store, &raw_name, auth_principal, transport).await {
+        warn!(session = session_id, doc = %raw_name, reason = %msg, "sync/resync denied");
+        return JsonRpcResponse::error(id, McpError::internal_error(msg));
+    }
     // Resolve bare filenames via suffix matching (e.g. "test.txt" finds "file:no-project/test.txt").
     let doc_name = if doc_store.has_doc(&raw_name).await {
-        raw_name
+        raw_name.clone()
     } else if let Some(found) = doc_store.find_doc_by_suffix(&raw_name).await {
         info!(requested = %raw_name, resolved = %found, "resolved doc by suffix match");
         found
     } else {
-        raw_name // fall through — will create new empty doc
+        raw_name.clone() // fall through — will create new empty doc
     };
+    // Suffix resolution can map a bare name onto a `kb:`/`kbc:` doc, so the
+    // RESOLVED name has to be checked too — otherwise the gate above is
+    // bypassable by asking for "notes.org" instead of "kb:notes.org".
+    if doc_name != raw_name {
+        if let Err(msg) = deny_kb_doc_read(doc_store, &doc_name, auth_principal, transport).await {
+            warn!(session = session_id, doc = %doc_name, reason = %msg, "sync/resync denied (resolved)");
+            return JsonRpcResponse::error(id, McpError::internal_error(msg));
+        }
+    }
     // Track this doc for disconnect cleanup and doc-scoped broadcast filtering.
     if session_docs.insert(doc_name.clone()) {
         let _ = doc_store.track_client_connect(&doc_name).await;

daemon/src/collab_handler/tests/collab_handler_cross_kb_node_isolation_tests.rs

@@ -148,3 +148,85 @@ async fn kb_node_fetch_cannot_read_a_node_belonging_to_another_kb() {
     );
     assert!(!victim_docs.contains("kb:concept:a-own"));
 }
+
+/// The same attack via the raw sync surface, which was ungated entirely.
+///
+/// `deny_kb_doc_read` existed and was correct — it was called from exactly TWO
+/// of the paths that needed it (`sync/state_vector`, `sync/full_state`).
+/// `sync/resync` returns the same bytes under a different method name, and
+/// `sync/diff` returns them as a delta. Its own doc comment says KB content
+/// "must be fetched via the access-gated `kb/node_fetch`"; these two were what
+/// made that false.
+///
+/// `sync/resync` is the worse of the pair, for the reason this file's header
+/// gives about `kb/node_fetch`: it also `subscribe_doc`s the session, so an
+/// ungated call grants a STANDING feed of every future edit, not just one read.
+#[tokio::test]
+async fn raw_sync_methods_cannot_read_a_node_belonging_to_another_kb() {
+    let store = test_doc_store();
+    let bc = test_broadcaster();
+
+    let mut mallory_docs = HashSet::new();
+    kb_share_as(
+        &store,
+        &bc,
+        Some("mallory"),
+        Some(&fp("mallory")),
+        "kb-a",
+        "mallory",
+        &mut mallory_docs,
+    )
+    .await;
+
+    let mut victim_docs = HashSet::new();
+    kb_share_as(
+        &store,
+        &bc,
+        Some("victim"),
+        Some(&fp("victim")),
+        "kb-b",
+        "victim",
+        &mut victim_docs,
+    )
+    .await;
+    let added = dispatch_as(
+        &store,
+        &bc,
+        Some("victim"),
+        Some(&fp("victim")),
+        serde_json::json!({
+            "jsonrpc":"2.0","id":1,"method":"kb/collection_node_add",
+            "params":{"kb_id":"kb-b","node_id":"concept:b-secret","title":"B"}}),
+        &mut victim_docs,
+    )
+    .await;
+    assert!(added.error.is_none(), "seed failed: {:?}", added.error);
+
+    for method in ["sync/resync", "sync/diff"] {
+        let before = mallory_docs.len();
+        let attack = dispatch_as(
+            &store,
+            &bc,
+            Some("mallory"),
+            Some(&fp("mallory")),
+            serde_json::json!({
+                "jsonrpc":"2.0","id":9,"method": method,
+                "params":{"doc":"kb:concept:b-secret","sv":""}}),
+            &mut mallory_docs,
+        )
+        .await;
+
+        assert!(
+            attack.error.is_some(),
+            "{method} returned kb-b's node to an outsider: {:?}",
+            attack.result
+        );
+        // The load-bearing half: a refusal that still subscribed would pass the
+        // assertion above and keep leaking every subsequent edit.
+        assert_eq!(
+            mallory_docs.len(),
+            before,
+            "{method} subscribed the session to another KB's node despite refusing the read"
+        );
+    }
+}

daemon/src/config.rs

@@ -815,6 +815,8 @@ impl DaemonConfig {
         let mut issues = Vec::new();
         let c = &self.collab;
 
+        issues.extend(crate::config_guards::unauthenticated_bind_issues(c));
+
         if c.storage.compact_threshold == 0 {
             issues.push("collab.storage.compact_threshold must be > 0".to_string());
         }

daemon/src/config_bind_tests.rs

@@ -0,0 +1,72 @@
+//! ADVERSARIAL: an unauthenticated collab port that is reachable off-host.
+//!
+//! Split out of `config.rs` rather than blessing its growth — the structural
+//! ratchet is doing its job, and a security test module is exactly the kind of
+//! thing that should not push a config module past its ceiling.
+
+use crate::config::DaemonConfig;
+
+fn cfg(bind: &str, mode: &str) -> DaemonConfig {
+    let mut c = DaemonConfig::default();
+    c.collab.enabled = true;
+    c.collab.bind = bind.parse().unwrap();
+    c.collab.auth.mode = mode.to_string();
+    c
+}
+
+/// An unauthenticated session reaches `kb_access_with_coll` with
+/// `principal == None`, which returns `Allow`. So a non-loopback bind under
+/// `mode = "none"` grants Manage on every KB to every host that can reach
+/// the port — and `mode` DEFAULTS to "none".
+///
+/// The oracle is that the config is rejected, and that the message names
+/// both halves: an operator who sees only "bad config" will re-read the
+/// wrong line.
+#[test]
+fn an_unauthenticated_off_host_bind_is_refused() {
+    for (bind, mode) in [
+        ("0.0.0.0:9473", "none"),
+        ("0.0.0.0:9473", "psk"),
+        ("[::]:9473", "none"),
+        ("10.0.0.5:9473", "none"),
+        ("10.0.0.5:9473", "psk"),
+    ] {
+        let issues = cfg(bind, mode).check_collab();
+        assert!(
+            issues.iter().any(|i| i.contains("reachable off-host")),
+            "bind={bind} mode={mode} must be refused, got: {issues:?}"
+        );
+    }
+}
+
+/// The three configurations that must NOT be refused. Without these the
+/// check above would be satisfied by a function that rejects everything,
+/// and loopback development would be broken.
+#[test]
+fn loopback_and_key_mode_are_accepted() {
+    for (bind, mode) in [
+        ("127.0.0.1:9473", "none"),
+        ("127.0.0.1:9473", "psk"),
+        ("[::1]:9473", "none"),
+        ("0.0.0.0:9473", "key"),
+        ("10.0.0.5:9473", "key"),
+    ] {
+        let issues = cfg(bind, mode).check_collab();
+        assert!(
+            !issues.iter().any(|i| i.contains("reachable off-host")),
+            "bind={bind} mode={mode} must be accepted, got: {issues:?}"
+        );
+    }
+}
+
+/// Disabling collab must not produce a bind complaint about a listener that
+/// never starts.
+#[test]
+fn a_disabled_collab_listener_is_not_flagged() {
+    let mut c = cfg("0.0.0.0:9473", "none");
+    c.collab.enabled = false;
+    assert!(!c
+        .check_collab()
+        .iter()
+        .any(|i| i.contains("reachable off-host")));
+}

daemon/src/config_guards.rs

@@ -0,0 +1,41 @@
+//! Configuration guards that refuse a dangerous deployment shape.
+//!
+//! Separate from `config.rs` because that file is already a tracked
+//! ceiling exception, and because these are *security* refusals rather than
+//! field-validity checks — they answer "is this safe to expose", not "is this
+//! well-formed".
+
+use crate::config::CollabConfig;
+use std::net::SocketAddr;
+
+/// Whether a bind address is loopback-only, i.e. unreachable from another host.
+/// `0.0.0.0`/`::` are explicitly NOT loopback — they are the wildcard binds,
+/// which is exactly the case this exists to catch.
+fn bind_is_loopback(addr: &SocketAddr) -> bool {
+    addr.ip().is_loopback()
+}
+
+/// Refuse an unauthenticated collab port that is reachable off-host.
+///
+/// @ai-caution: [security] `AuthConfig::default().mode` is `"none"`, and an
+/// unauthenticated session reaches `kb_access_with_coll` with
+/// `principal == None`, which returns `AccessDecision::Allow`. So
+/// `--bind 0.0.0.0` on a stock config granted Manage on every KB to every host
+/// that could reach the port, while `doctor` printed "collab config: OK".
+/// `psk` is plaintext on the wire and no better off-host.
+///
+/// This is an ERROR, not a warning: a warning gets read past, and the shipped
+/// `assets/daemon-config.toml` has no `[collab.auth]` block at all while
+/// DAEMON_ADMIN tells operators to start from it.
+pub fn unauthenticated_bind_issues(c: &CollabConfig) -> Vec<String> {
+    if c.enabled && !bind_is_loopback(&c.bind) && c.auth.mode != "key" {
+        return vec![format!(
+            "collab.bind is {} (reachable off-host) but collab.auth.mode is \
+             '{}' — that accepts any client that can reach the port. Set \
+             [collab.auth] mode = \"key\" (Ed25519 mTLS), or bind to loopback \
+             and put a reverse proxy or VPN in front.",
+            c.bind, c.auth.mode
+        )];
+    }
+    Vec::new()
+}

daemon/src/main.rs

@@ -15,6 +15,9 @@
 
 mod cli;
 mod config;
+#[cfg(test)]
+mod config_bind_tests;
+mod config_guards;
 mod conn_limit;
 mod dialer;
 pub mod enrichment;