radicle/cob/identity: Guard `IdentityMut::update` against sibling proposals

Ensure that the `IdentityMut::update` API returns an error when it encounters an
active, sibling revision by the same delegate.
This commit is contained in:
Fintan Halpenny 2026-07-02 16:40:51 +00:00 committed by Adrian Duke
parent b6f07074d9
commit a63f91885f
2 changed files with 78 additions and 2 deletions

View File

@ -1226,13 +1226,29 @@ where
} }
/// Update the identity by proposing a new revision. /// Update the identity by proposing a new revision.
/// If the signer is the only delegate, the revision is accepted automatically. ///
/// If the signer is the only delegate, the revision is adopted automatically.
///
/// # Errors
///
/// [`SiblingAccepted`]: If the delegate has already accepted an active
/// revision that shares the same parent as the provided `revision`.
///
/// [`SiblingAccepted`]: ApplyError::SiblingAccepted
pub fn update( pub fn update(
&mut self, &mut self,
title: cob::Title, title: cob::Title,
description: impl ToString, description: impl ToString,
doc: &Doc, doc: &Doc,
) -> Result<RevisionId, Error> { ) -> Result<RevisionId, Error> {
#[allow(deprecated)]
let did: Did = self.store.signer().verifying_key().into();
if let Some(revision_id) = self.has_active_sibling_accept(&self.current, &did) {
return Err(Error::Apply(ApplyError::SiblingAccepted {
revision: revision_id,
}));
}
let parent = Some(self.current); let parent = Some(self.current);
#[allow(deprecated)] #[allow(deprecated)]
@ -1440,6 +1456,10 @@ mod test {
let r2 = bob_identity.revision(&r2).unwrap(); let r2 = bob_identity.revision(&r2).unwrap();
assert_eq!(r2.state, State::Rejected(RejectedBy::Vote)); assert_eq!(r2.state, State::Rejected(RejectedBy::Vote));
// Reload so Alice sees r2 is rejected (no longer Active).
// This allows her to propose a new sibling.
identity.reload().unwrap();
// Now let's add another delegate. // Now let's add another delegate.
doc.delegate(eve.public_key().into()); doc.delegate(eve.public_key().into());
let r3 = identity let r3 = identity
@ -3168,4 +3188,53 @@ mod test {
"Expected SiblingAccepted error, got: {err:?}" "Expected SiblingAccepted error, got: {err:?}"
); );
} }
/// The `update()` API rejects a second proposal if the delegate already
/// has an accept on an Active sibling (child of current).
#[test]
fn update_rejects_second_sibling_proposal() {
let network = Network::default();
let alice = &network.alice;
let bob = &network.bob;
let mut alice_identity = Identity::load_mut(&*alice.repo, &alice.signer).unwrap();
let mut alice_doc = alice_identity.doc().clone().edit();
alice_doc.delegate(bob.signer.public_key().into());
let _a1 = alice_identity
.update(
cob::Title::new("Add Bob").unwrap(),
"",
&alice_doc.verified().unwrap(),
)
.unwrap();
// Alice proposes first child.
let mut doc1 = alice_identity.doc().clone().edit();
doc1.visibility = Visibility::private([]);
let _child1 = alice_identity
.update(
cob::Title::new("Child 1").unwrap(),
"",
&doc1.verified().unwrap(),
)
.unwrap();
// Alice tries to propose a second child (sibling) — should fail.
let mut doc2 = alice_identity.doc().clone().edit();
doc2.visibility = Visibility::private([alice.signer.public_key().into()]);
let err = alice_identity
.update(
cob::Title::new("Child 2").unwrap(),
"",
&doc2.verified().unwrap(),
)
.unwrap_err();
assert!(
std::iter::successors(Some(&err as &dyn std::error::Error), |e| e.source())
.filter_map(|e| e.downcast_ref::<ApplyError>())
.any(|e| matches!(e, ApplyError::SiblingAccepted { .. })),
"Expected SiblingAccepted error, got: {err:?}"
);
}
} }

View File

@ -223,7 +223,14 @@ impl Harness {
Ok(rev) => self.revisions.push(rev), Ok(rev) => self.revisions.push(rev),
Err(e) Err(e)
if Self::has_apply_error(&e, |err| { if Self::has_apply_error(&e, |err| {
matches!(err, cob::identity::ApplyError::DocUnchanged) matches!(
err,
cob::identity::ApplyError::DocUnchanged
// An `actor` for the above `update` could attempt
// to create active, sibling revisions which is
// disallowed.
| cob::identity::ApplyError::SiblingAccepted { .. }
)
}) => {} }) => {}
Err(e) => { Err(e) => {
panic!("Delegate action should succeed, but failed with: {:?}", e) panic!("Delegate action should succeed, but failed with: {:?}", e)