diff --git a/radicle-node/src/tests/e2e.rs b/radicle-node/src/tests/e2e.rs index 3e9554d7..42011068 100644 --- a/radicle-node/src/tests/e2e.rs +++ b/radicle-node/src/tests/e2e.rs @@ -205,13 +205,16 @@ fn test_replication_no_delegates() { let tmp = tempfile::tempdir().unwrap(); let alice = Node::init(tmp.path()); let mut bob = Node::init(tmp.path()); - let (repo, _) = fixtures::repository(tmp.path().join("acme")); - let acme = bob.project_from("acme", "", &repo); - - // Populate repo, but don't sign the refs. - let branches = fixtures::populate(&repo, 1); - git::push(&repo, "rad", branches.iter().map(|b| (b, b))).unwrap(); + let acme = bob.project("acme", ""); + // Delete one of the signed refs. + bob.storage + .repository_mut(acme) + .unwrap() + .reference(&bob.id, &git::qualified!("refs/heads/master")) + .unwrap() + .delete() + .unwrap(); let mut alice = alice.spawn(service::Config::default()); let bob = bob.spawn(service::Config::default()); diff --git a/radicle-node/src/worker/fetch.rs b/radicle-node/src/worker/fetch.rs index 444e07d3..3f9d4229 100644 --- a/radicle-node/src/worker/fetch.rs +++ b/radicle-node/src/worker/fetch.rs @@ -88,6 +88,8 @@ enum VerifiedRemote { // Nb. unused but we want to ensure that we verify the identity _doc: Doc, remote: Remote, + /// Unsigned refs. + unsigned: Vec, }, } @@ -251,6 +253,7 @@ impl<'a> StagingPhaseFinal<'a> { let url = url::File::new(self.repo.path().to_path_buf()).to_string(); let mut remote = production.backend.remote_anonymous(&url)?; let mut updates = Vec::new(); + let mut delete = HashSet::new(); let callbacks = ref_updates(&mut updates); let remotes = { @@ -266,10 +269,15 @@ impl<'a> StagingPhaseFinal<'a> { ); vec![] } - VerifiedRemote::Success { remote, .. } => { + VerifiedRemote::Success { + remote, unsigned, .. + } => { let ns = remote.id.to_namespace(); let mut refspecs = vec![]; + // Unsigned refs should be deleted. + delete.insert((remote.id, unsigned)); + // First add the standard git refs. let heads = ns.join(git::refname!("refs/heads")); let cobs = ns.join(git::refname!("refs/cobs")); @@ -338,8 +346,22 @@ impl<'a> StagingPhaseFinal<'a> { // should be devised. opts.prune(git::raw::FetchPrune::Off); + // Fetch into production copy. remote.fetch(&specs, Some(&mut opts), None)?; + // Delete unsigned refs. + for (namespace, unsigned) in delete { + for refstr in unsigned { + let q = git::Qualified::from_refstr(&refstr) + .expect("StagingPhaseFinal::transfer: unsigned references are qualified"); + + if let Ok(mut r) = production.reference(&namespace, &q) { + log::debug!(target: "worker", "Deleting unsigned ref {namespace}/{q}.."); + + r.delete()?; + } + } + } fetching }; let head = production.set_head()?; @@ -363,7 +385,11 @@ impl<'a> StagingPhaseFinal<'a> { let remote_id = remote.id; let verification = match self.repo.identity_doc_of(&remote_id) { Ok(doc) => match self.repo.validate_remote(&remote) { - Ok(()) => VerifiedRemote::Success { _doc: doc, remote }, + Ok(unsigned) => VerifiedRemote::Success { + _doc: doc, + remote, + unsigned, + }, Err(e) => VerifiedRemote::Failed { reason: e.to_string(), }, diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index d0aea081..ea33f4de 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -304,10 +304,11 @@ pub trait ReadRepository { Ok(()) } - /// Validate all the remote's references in the repository, checking that they are signed as - /// part of the remote's signed refs. Also verify that no signed reference is missing from the - /// repository. - fn validate_remote(&self, remote: &Remote) -> Result<(), VerifyError>; + /// Validates a remote's signed refs and identity. + /// + /// Returns any ref found under that remote that isn't signed. + /// If a signed ref is missing from the repository, an error is returned. + fn validate_remote(&self, remote: &Remote) -> Result, VerifyError>; /// Get the head of this repository. /// diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index 579dcf30..8a6909f3 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -202,8 +202,6 @@ pub enum VerifyError { InvalidIdentity(#[from] IdentityError), #[error("refs error: {0}")] Refs(#[from] refs::Error), - #[error("unknown reference `{1}` in remote `{0}`")] - UnknownRef(RemoteId, git::RefString), #[error("missing reference `{1}` in remote `{0}`")] MissingRef(RemoteId, git::RefString), #[error("git: {0}")] @@ -371,9 +369,10 @@ impl ReadRepository for Repository { .get(&self.backend) } - fn validate_remote(&self, remote: &Remote) -> Result<(), VerifyError> { + fn validate_remote(&self, remote: &Remote) -> Result, VerifyError> { // Contains a copy of the signed refs of this remote. - let mut refs = BTreeMap::from((*remote.refs).clone()); + let mut signed = BTreeMap::from((*remote.refs).clone()); + let mut unsigned = Vec::new(); // Check all repository references, making sure they are present in the signed refs map. for (refname, oid) in self.references_of(&remote.id)? { @@ -381,24 +380,24 @@ impl ReadRepository for Repository { if refname == refs::SIGREFS_BRANCH.to_ref_string() { continue; } - let signed_oid = refs - .remove(&refname) - .ok_or_else(|| VerifyError::UnknownRef(remote.id, refname.clone()))?; - - if oid != signed_oid { - return Err(VerifyError::InvalidRefTarget(remote.id, refname, *oid)); + if let Some(signed_oid) = signed.remove(&refname) { + if oid != signed_oid { + return Err(VerifyError::InvalidRefTarget(remote.id, refname, *oid)); + } + } else { + unsigned.push(refname); } } // The refs that are left in the map, are ones that were signed, but are not // in the repository. If any are left, bail. - if let Some((name, _)) = refs.into_iter().next() { + if let Some((name, _)) = signed.into_iter().next() { return Err(VerifyError::MissingRef(remote.id, name)); } // Finally, verify the identity history of remote. self.identity_of(&remote.id)?.verified(self.id)?; - Ok(()) + Ok(unsigned) } fn reference( diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index 5f54d2ef..b09eff65 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -140,8 +140,11 @@ impl ReadRepository for MockRepository { todo!() } - fn validate_remote(&self, _remote: &Remote) -> Result<(), VerifyError> { - Ok(()) + fn validate_remote( + &self, + _remote: &Remote, + ) -> Result, VerifyError> { + Ok(vec![]) } fn path(&self) -> &std::path::Path {