fix(node): fail closed when a recipient DID can't be resolved (#47)

crates/gitlawb-node/src/encrypted_pin.rs

@@ -19,6 +19,31 @@ fn did_to_key(did: &str) -> Option<VerifyingKey> {
     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<String>) -> Result<Vec<VerifyingKey>, Vec<String>> {
+    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<String, BTreeSet<String>>,
 ) -> Vec<(String, String, Vec<String>)> {
     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<VerifyingKey> = 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<String> = 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<String> {
+        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<String> = [gitlawb.clone()].into_iter().collect();
+        let unresolved = resolve_all_recipients(&dids).expect_err("must fail closed");
+        assert_eq!(unresolved, vec![gitlawb]);
+    }
+}