From eafdd8715529c61762cc7c3861d59f7cd7807d6e Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Fri, 19 Jun 2026 23:10:31 -0500 Subject: [PATCH] fix(node): fail closed when a recipient DID can't be resolved (#47) encrypt_and_pin resolved recipient DIDs with filter_map, silently dropping any that failed to resolve and sealing the envelope to the remaining subset while still recording the full intended recipient set as covered. A reader whose DID could not be resolved at seal time (any did:web/did:gitlawb, or a malformed did:key) was then permanently locked out: the dedup compares the recorded full set against the desired set, matches, and never re-seals. Resolve all recipient DIDs up front via resolve_all_recipients; if any fail, log and skip the blob rather than seal to a partial set. Nothing partial is recorded, so the dedup stays honest and the blob re-seals on a later push once resolution is available. Add a bounded per-blob warn plus one aggregate coverage warn so a fully-migrated did:gitlawb org (zero encrypted-pin coverage) is one greppable line, not a scrape, and give the previously silent read_object and pin_git_object failure arms warn logs. --- crates/gitlawb-node/src/encrypted_pin.rs | 151 +++++++++++++++++++++-- 1 file changed, 144 insertions(+), 7 deletions(-) diff --git a/crates/gitlawb-node/src/encrypted_pin.rs b/crates/gitlawb-node/src/encrypted_pin.rs index 50797b5..06c023d 100644 --- a/crates/gitlawb-node/src/encrypted_pin.rs +++ b/crates/gitlawb-node/src/encrypted_pin.rs @@ -19,6 +19,31 @@ fn did_to_key(did: &str) -> Option { Did::from_str(did).ok()?.to_verifying_key().ok() } +/// Resolve every recipient DID to its verifying key, all-or-nothing. +/// +/// Returns `Ok(keys)` only when every DID resolves. If any DID fails, returns +/// `Err(unresolved)` listing the unresolvable DID strings so the caller can fail +/// closed rather than seal to a partial recipient set (#47): sealing to a subset +/// while recording the full set as covered permanently locks out the dropped +/// readers. Resolution is local-only, so `did:web`/`did:gitlawb` recipients (and +/// any malformed `did:key`) land in the unresolved set until off-`did:key` +/// resolution exists. +fn resolve_all_recipients(dids: &BTreeSet) -> Result, Vec> { + let mut keys = Vec::with_capacity(dids.len()); + let mut unresolved = Vec::new(); + for did in dids { + match did_to_key(did) { + Some(k) => keys.push(k), + None => unresolved.push(did.clone()), + } + } + if unresolved.is_empty() { + Ok(keys) + } else { + Err(unresolved) + } +} + /// Encrypt and pin every withheld blob. `recipients` maps blob oid -> DID set. /// Returns `(oid, cid, recipients)` for each blob actually sealed and recorded /// this call (the per-push delta), used by Option B3 to anchor a manifest. @@ -30,6 +55,7 @@ pub async fn encrypt_and_pin( recipients: &HashMap>, ) -> Vec<(String, String, Vec)> { let mut sealed = Vec::new(); + let mut skipped_unresolvable = 0usize; for (oid, dids) in recipients { // Skip only if an existing envelope already covers exactly these // recipients. If the recipient set changed (e.g. a reader was added to @@ -41,14 +67,41 @@ pub async fn encrypt_and_pin( continue; } } - let keys: Vec = dids.iter().filter_map(|d| did_to_key(d)).collect(); - if keys.is_empty() { - tracing::warn!(oid = %oid, "no resolvable recipient keys; skipping encrypted pin"); - continue; - } + // Fail closed: seal only when every recipient DID resolves. Sealing to a + // partial set while recording the full set as covered would permanently + // lock out the dropped readers (#47). + let keys = match resolve_all_recipients(dids) { + Ok(keys) if keys.is_empty() => { + tracing::warn!(oid = %oid, "no resolvable recipient keys; skipping encrypted pin"); + continue; + } + Ok(keys) => keys, + Err(unresolved) => { + skipped_unresolvable += 1; + // DIDs are user-controlled (rule reader_dids); log a bounded + // sample, not an unbounded dump. Wording stays neutral about the + // cause: a malformed did:key is not DHT-pending and never will + // resolve, unlike a did:gitlawb awaiting anchoring. + let sample: Vec<&String> = unresolved.iter().take(3).collect(); + tracing::warn!( + oid = %oid, + unresolved_count = unresolved.len(), + unresolved_sample = ?sample, + "unresolvable recipient DID(s); skipping encrypted pin to avoid sealing to a partial set" + ); + continue; + } + }; let data = match crate::git::store::read_object(repo_path, oid) { Ok(Some((_t, bytes))) => bytes, - _ => continue, + Ok(None) => { + tracing::warn!(oid = %oid, "git object not found; skipping encrypted pin"); + continue; + } + Err(e) => { + tracing::warn!(oid = %oid, err = %e, "read_object failed; skipping encrypted pin"); + continue; + } }; let envelope = match seal_blob(&data, &keys) { Ok(e) => e, @@ -59,7 +112,14 @@ pub async fn encrypt_and_pin( }; let cid = match crate::ipfs_pin::pin_git_object(ipfs_api, oid, &envelope).await { Ok(c) if !c.is_empty() => c, - _ => continue, + Ok(_) => { + tracing::warn!(oid = %oid, "pin_git_object returned no cid; skipping"); + continue; + } + Err(e) => { + tracing::warn!(oid = %oid, err = %e, "pin_git_object failed; skipping"); + continue; + } }; let dids_vec: Vec = dids.iter().cloned().collect(); if let Err(e) = db @@ -71,5 +131,82 @@ pub async fn encrypt_and_pin( } sealed.push((oid.clone(), cid.clone(), dids_vec)); } + // One aggregate signal so a coverage collapse is a single greppable line, not + // a per-oid scrape. In a fully-migrated did:gitlawb org every blob skips and + // recovery coverage silently drops to zero; this is the operator's cue that + // the gap is the deliberate fail-closed posture, not a malfunction. + if skipped_unresolvable > 0 { + tracing::warn!( + sealed = sealed.len(), + skipped = skipped_unresolvable, + "encrypted-pin coverage reduced: blobs skipped for unresolvable recipients" + ); + } sealed } + +#[cfg(test)] +mod tests { + use super::*; + use ed25519_dalek::SigningKey; + + fn did_key(seed: u8) -> String { + let vk = SigningKey::from_bytes(&[seed; 32]).verifying_key(); + Did::from_verifying_key(&vk).to_string() + } + + fn set(dids: &[String]) -> BTreeSet { + dids.iter().cloned().collect() + } + + #[test] + fn all_did_key_recipients_resolve() { + let dids = set(&[did_key(1), did_key(2)]); + let keys = resolve_all_recipients(&dids).expect("all did:key resolve"); + assert_eq!(keys.len(), 2); + } + + #[test] + fn mixed_set_with_did_gitlawb_fails_closed() { + // Core #47 regression: a resolvable subset must never yield a sealable + // key set. Two did:key plus one did:gitlawb -> Err naming only the + // unresolvable DID. + let gitlawb = Did::gitlawb("zSomeDhtKey").to_string(); + let dids = set(&[did_key(1), did_key(2), gitlawb.clone()]); + let unresolved = resolve_all_recipients(&dids).expect_err("must fail closed"); + assert_eq!(unresolved, vec![gitlawb]); + } + + #[test] + fn did_web_recipient_fails_closed() { + let web = Did::web("example.com").to_string(); + let dids = set(&[did_key(1), web.clone()]); + let unresolved = resolve_all_recipients(&dids).expect_err("did:web cannot resolve locally"); + assert_eq!(unresolved, vec![web]); + } + + #[test] + fn malformed_did_key_fails_closed() { + // The third state: not a method-resolution gap but a permanently broken + // did:key (invalid multibase). Must also fail closed. + let broken = "did:key:z!!!invalid".to_string(); + let dids = set(&[did_key(1), broken.clone()]); + let unresolved = resolve_all_recipients(&dids).expect_err("malformed did:key fails"); + assert_eq!(unresolved, vec![broken]); + } + + #[test] + fn empty_set_resolves_to_empty_keys() { + let dids = BTreeSet::new(); + let keys = resolve_all_recipients(&dids).expect("empty set is not an error"); + assert!(keys.is_empty()); + } + + #[test] + fn single_unresolvable_did_returns_that_did() { + let gitlawb = Did::gitlawb("zOnlyOne").to_string(); + let dids: BTreeSet = [gitlawb.clone()].into_iter().collect(); + let unresolved = resolve_all_recipients(&dids).expect_err("must fail closed"); + assert_eq!(unresolved, vec![gitlawb]); + } +}