fix(daemon): ungated raw sync surface + unauthenticated off-host bind - #672
Merged
Conversation
…bind was unauthenticated Two independent holes on the path a real multi-user hub runs on. **1. `sync/resync` and `sync/diff` had no access control.** `deny_kb_doc_read` exists, is correct, and was called from exactly TWO of the paths that need it — `sync/state_vector` and `sync/full_state`. Its own doc comment says KB content "must be fetched via the access-gated `kb/node_fetch`". `sync/resync` returns the same bytes under a different method name and `sync/diff` returns them as a delta, so that sentence was false. `sync/resync` is the worse of the two: it also `subscribe_doc`s the session, so an ungated call granted a STANDING feed of every future edit rather than one read. With N authorized users on one hub, any of them could read — and then keep receiving — every other user's KB node state, while `kb/node_fetch` on the same node correctly refused. Both now gate through `deny_kb_doc_read`, which only denies `kb:`/`kbc:` docs, so plain file collaboration is untouched. `sync/resync` is checked twice: on the raw name AND on the suffix-resolved one, because resolution can map a bare filename onto a `kb:` doc and a single check on the raw name would be bypassable by asking for the other spelling. **2. A non-loopback bind with `auth.mode != "key"` is now a config error.** The default is `"none"`, and an unauthenticated session reaches `kb_access_with_coll` with `principal == None`, which returns `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. Raised as an error rather than 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. Gates: - `raw_sync_methods_cannot_read_a_node_belonging_to_another_kb` — Mallory owns kb-a (deliberately the highest role: if an owner cannot reach another KB's node, no lesser role can) and asks for kb-b's node via both methods. Verified failing first: "sync/resync returned kb-b's node to an outsider". The load-bearing assertion is the SUBSCRIPTION count, not the error — a refusal that still subscribed would satisfy an error-only check and keep leaking every subsequent edit. - `an_unauthenticated_off_host_bind_is_refused` across five bind/mode pairs, with `loopback_and_key_mode_are_accepted` as the control so the check cannot be satisfied by rejecting everything, plus a disabled-listener case. 404 daemon tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The structural ratchet fired: `daemon/src/config.rs` 1050 -> 1251 (+19%), then +12% after moving the tests alone. Split rather than blessed, per the ratchet's own instruction and because a security refusal is the wrong thing to buy debt tolerance for. - `config_guards.rs` — `unauthenticated_bind_issues`, separate from `config.rs` because these answer "is this safe to expose", not "is this well-formed". - `config_bind_tests.rs` — the adversarial cases and their control. No behaviour change; `check_collab` now delegates. 404 daemon tests pass and `make audit-metrics-check` is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cuttlefisch
enabled auto-merge
August 6, 2026 11:37
cuttlefisch
disabled auto-merge
August 7, 2026 08:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent holes on the exact path a real multi-user hub runs on. Both block the deployment.
1.
sync/resyncandsync/diffhad no access controldeny_kb_doc_readexists, is correct, and was called from two of the paths that need it —sync/state_vectorandsync/full_state. Its own doc comment says KB content "must be fetched via the access-gatedkb/node_fetch".sync/resyncreturns the same bytes under a different method name andsync/diffreturns them as a delta, so that sentence was false.sync/resyncis the worse of the two: it alsosubscribe_docs the session. An ungated call granted a standing feed of every future edit, not one read.With N authorized users on one hub, any of them could read — and keep receiving — every other user's KB node state, while
kb/node_fetchon the same node correctly refused.Both now gate through
deny_kb_doc_read, which only denieskb:/kbc:docs, so plain file collaboration is untouched.sync/resyncis checked twice — on the raw name and on the suffix-resolved one, because resolution can map a bare filename onto akb:doc, and a single check on the raw name would be bypassable by asking for the other spelling.2. A non-loopback bind with
auth.mode != "key"is now a config errorThe default is
"none", and an unauthenticated session reacheskb_access_with_collwithprincipal == None, which returnsAllow. So--bind 0.0.0.0on a stock config granted Manage on every KB to every host that could reach the port — whiledoctorprinted "collab config: OK".pskis plaintext on the wire and no better off-host.Raised as an error, not a warning: a warning gets read past, and the shipped
assets/daemon-config.tomlhas no[collab.auth]block at all whileDAEMON_ADMIN.mdtells operators to start from it.Gates
raw_sync_methods_cannot_read_a_node_belonging_to_another_kb— Mallory owns kb-a (deliberately the highest role: if an owner of one KB cannot reach another's node, no lesser role can) and asks for kb-b's node via both methods.Verified failing first:
The load-bearing assertion is the subscription count, not the error. A refusal that still subscribed would satisfy an error-only check and keep leaking every subsequent edit — which is the failure mode this file's header already calls out for
kb/node_fetch.an_unauthenticated_off_host_bind_is_refusedacross five bind/mode pairs, withloopback_and_key_mode_are_acceptedas the control so the check cannot be satisfied by a function that rejects everything, plus a disabled-listener case.Still open on the hub path
Not in this PR, from the same audit:
docs/list/docs/content/docs/delete/kb/unregister/kb/listare still ungated (docs/listneeds filtering rather than refusal — it feeds the editor's join-document picker); there is no server-side KB approval, so an operator cannot onboard user #2; and signed membership/content verification is inert on the hub because the anchor is only set by the P2P dialer.Verification
cd daemon && cargo test— 404 passcargo clippy --all-targets -- -D warnings— clean