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 1/3] 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]); + } +} From f636f06963dd893323eb511ddab00eebbd5dcebd Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Mon, 22 Jun 2026 13:54:34 -0500 Subject: [PATCH 2/3] test(node): extract plan_seal so the #47 fail-closed gate is unit-tested The fail-closed invariant lived only in resolve_all_recipients, tested in isolation; nothing proved encrypt_and_pin acts on it (a refactor falling through to a partial seal would pass every test). encrypt_and_pin needs a live Postgres Db, which CI does not provide, so it can't be tested directly. Extract the per-blob seal/skip decision into a pure plan_seal() -> SealPlan (no DB, no IO), behaviour-preserving, and unit-test its branches: seals only when all recipients resolve, fails closed on any unresolvable DID, skips the empty set, skips an unchanged tag, re-seals a changed set. --- crates/gitlawb-node/src/encrypted_pin.rs | 130 +++++++++++++++++++---- 1 file changed, 110 insertions(+), 20 deletions(-) diff --git a/crates/gitlawb-node/src/encrypted_pin.rs b/crates/gitlawb-node/src/encrypted_pin.rs index 21aab5d..e0a2909 100644 --- a/crates/gitlawb-node/src/encrypted_pin.rs +++ b/crates/gitlawb-node/src/encrypted_pin.rs @@ -63,6 +63,42 @@ fn resolve_all_recipients(dids: &BTreeSet) -> Result, } } +/// What to do with a single withheld blob, decided without any DB or IO so the +/// fail-closed invariant (#47) is unit-testable in isolation. +#[derive(Debug)] +enum SealPlan { + /// An existing envelope already covers exactly this recipient set; nothing to do. + SkipUnchanged, + /// No recipient DID resolved to a key, so there is nothing to seal to. + SkipNoRecipients, + /// At least one recipient DID is unresolvable. Fail closed: never seal to a + /// partial set. Carries the unresolvable DIDs for logging. + SkipUnresolvable(Vec), + /// Seal to `keys` and record coverage under `tag`. + Seal { keys: Vec, tag: String }, +} + +/// Decide what to do with one blob given its desired recipient set and the tag +/// already stored for it (if any). Pure: no DB, no IO. +/// +/// This is the #47 fail-closed gate in isolation: it returns `Seal` only when +/// EVERY recipient DID resolves, so no caller can seal to a partial set. A +/// changed recipient set (different tag) re-seals so a newly added reader can +/// recover the blob; reader removal is not retroactive (the old envelope is +/// already public). The comparison is on the opaque node-keyed tag, never the +/// DID list. +fn plan_seal(node_seed: &[u8; 32], dids: &BTreeSet, stored_tag: Option<&str>) -> SealPlan { + let tag = recipients_tag(node_seed, dids); + if stored_tag == Some(tag.as_str()) { + return SealPlan::SkipUnchanged; + } + match resolve_all_recipients(dids) { + Ok(keys) if keys.is_empty() => SealPlan::SkipNoRecipients, + Ok(keys) => SealPlan::Seal { keys, tag }, + Err(unresolved) => SealPlan::SkipUnresolvable(unresolved), + } +} + /// Encrypt and pin every withheld blob. `recipients` maps blob oid -> DID set; /// `node_seed` keys the opaque recipients tag. Returns `(oid, cid)` for each blob /// actually sealed and recorded this call (the per-push delta), used by Option B3 @@ -78,33 +114,25 @@ pub async fn encrypt_and_pin( 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 - // the rule), re-seal so the new reader can recover the blob. Reader - // removal is not retroactive: the old envelope is already public. The - // comparison is on the opaque node-keyed tag, never the DID list. - let tag = recipients_tag(node_seed, dids); - match db.encrypted_blob_recipients_tag(repo_id, oid).await { - Ok(Some(stored_tag)) if stored_tag == tag => continue, - Ok(_) => {} + // A DB read failure is not a cache miss: re-sealing here would do an + // avoidable IPFS write during a partial outage. Skip and retry next push. + let stored_tag = match db.encrypted_blob_recipients_tag(repo_id, oid).await { + Ok(t) => t, Err(e) => { - // A DB read failure is not a cache miss: re-sealing here would do - // an avoidable IPFS write during a partial outage. Skip and retry - // on the next push. tracing::warn!(oid = %oid, err = %e, "recipients_tag lookup failed; skipping reseal"); 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() => { + }; + // Fail closed: plan_seal returns Seal only when every recipient DID + // resolves, so we never seal to a partial set and record the full set as + // covered (which would permanently lock out the dropped readers, #47). + let (keys, tag) = match plan_seal(node_seed, dids, stored_tag.as_deref()) { + SealPlan::SkipUnchanged => continue, + SealPlan::SkipNoRecipients => { tracing::warn!(oid = %oid, "no resolvable recipient keys; skipping encrypted pin"); continue; } - Ok(keys) => keys, - Err(unresolved) => { + SealPlan::SkipUnresolvable(unresolved) => { skipped_unresolvable += 1; // DIDs are user-controlled (rule reader_dids); log a bounded // sample, not an unbounded dump. Wording stays neutral about the @@ -119,6 +147,7 @@ pub async fn encrypt_and_pin( ); continue; } + SealPlan::Seal { keys, tag } => (keys, tag), }; let data = match crate::git::store::read_object(repo_path, oid) { Ok(Some((_t, bytes))) => bytes, @@ -262,4 +291,65 @@ mod tests { "tag must depend on the node seed, not be a plain hash" ); } + + // plan_seal is the seal/skip decision `encrypt_and_pin` acts on. Testing it + // directly pins the #47 fail-closed invariant at the function that owns it, + // which a unit test of `resolve_all_recipients` alone cannot do (it can't + // catch the caller falling through to a partial seal). + const SEED: [u8; 32] = [9u8; 32]; + + #[test] + fn plan_seal_seals_when_all_recipients_resolve() { + let dids = set(&[did_key(1), did_key(2)]); + match plan_seal(&SEED, &dids, None) { + SealPlan::Seal { keys, tag } => { + assert_eq!(keys.len(), 2, "must seal to the full recipient set"); + assert_eq!(tag, recipients_tag(&SEED, &dids), "records the full-set tag"); + } + other => panic!("expected Seal, got {other:?}"), + } + } + + #[test] + fn plan_seal_fails_closed_on_any_unresolvable_recipient() { + // The #47 invariant at the decision boundary: one unresolvable DID among + // resolvable ones must NOT yield a Seal (which would seal a partial set). + let gitlawb = Did::gitlawb("zPending").to_string(); + let dids = set(&[did_key(1), did_key(2), gitlawb.clone()]); + match plan_seal(&SEED, &dids, None) { + SealPlan::SkipUnresolvable(unresolved) => assert_eq!(unresolved, vec![gitlawb]), + other => panic!("must fail closed, never seal a partial set; got {other:?}"), + } + } + + #[test] + fn plan_seal_skips_empty_recipient_set() { + let dids = BTreeSet::new(); + assert!(matches!( + plan_seal(&SEED, &dids, None), + SealPlan::SkipNoRecipients + )); + } + + #[test] + fn plan_seal_skips_when_tag_unchanged() { + let dids = set(&[did_key(1)]); + let stored = recipients_tag(&SEED, &dids); + assert!(matches!( + plan_seal(&SEED, &dids, Some(&stored)), + SealPlan::SkipUnchanged + )); + } + + #[test] + fn plan_seal_reseals_when_recipient_set_changed() { + // A stored tag for a DIFFERENT set is a miss: a newly added reader must + // trigger a re-seal, not be skipped as unchanged. + let dids = set(&[did_key(1), did_key(2)]); + let stale = recipients_tag(&SEED, &set(&[did_key(1)])); + match plan_seal(&SEED, &dids, Some(&stale)) { + SealPlan::Seal { keys, .. } => assert_eq!(keys.len(), 2), + other => panic!("changed recipient set must re-seal; got {other:?}"), + } + } } From ca53afdd351821b4a6b46002b29db7d2dfc7ecad Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Tue, 23 Jun 2026 12:50:13 -0500 Subject: [PATCH 3/3] style(node): rustfmt encrypted_pin (Seal variant + assert_eq wrap) --- crates/gitlawb-node/src/encrypted_pin.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/gitlawb-node/src/encrypted_pin.rs b/crates/gitlawb-node/src/encrypted_pin.rs index e0a2909..40e444e 100644 --- a/crates/gitlawb-node/src/encrypted_pin.rs +++ b/crates/gitlawb-node/src/encrypted_pin.rs @@ -75,7 +75,10 @@ enum SealPlan { /// partial set. Carries the unresolvable DIDs for logging. SkipUnresolvable(Vec), /// Seal to `keys` and record coverage under `tag`. - Seal { keys: Vec, tag: String }, + Seal { + keys: Vec, + tag: String, + }, } /// Decide what to do with one blob given its desired recipient set and the tag @@ -304,7 +307,11 @@ mod tests { match plan_seal(&SEED, &dids, None) { SealPlan::Seal { keys, tag } => { assert_eq!(keys.len(), 2, "must seal to the full recipient set"); - assert_eq!(tag, recipients_tag(&SEED, &dids), "records the full-set tag"); + assert_eq!( + tag, + recipients_tag(&SEED, &dids), + "records the full-set tag" + ); } other => panic!("expected Seal, got {other:?}"), }