From 511165bfc5817cc1d81e6b017ad47409125d2d3f Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Mon, 27 Nov 2023 16:03:43 +0000 Subject: [PATCH] fetch: prevent missing default branch If a delegate is missing the default branch for a project certain computations will fail, e.g. getting the canonical mainline branch. One prevention for corrupting data is refusing to fetch a delegate that does not have the default branch, when the repository is a project. It is assumed that the repository is not a project if calling `Doc::project` returns an error, since the error variants are not found and parsing errors. This is only checked in the case of a delegate since non-delegates are safe to created COBs that don't require the default branch, e.g. creating an issue. --- radicle-fetch/src/state.rs | 32 ++++++++++++++++++-- radicle-node/src/tests/e2e.rs | 56 +++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/radicle-fetch/src/state.rs b/radicle-fetch/src/state.rs index 66778109..175df91c 100644 --- a/radicle-fetch/src/state.rs +++ b/radicle-fetch/src/state.rs @@ -7,7 +7,7 @@ use radicle::identity::{Doc, DocError}; use radicle::prelude::Verified; use radicle::storage; -use radicle::storage::refs::RefsAt; +use radicle::storage::refs::{RefsAt, SignedRefs}; use radicle::storage::{ git::Validation, Remote, RemoteId, RemoteRepository, Remotes, ValidateRepository, Validations, }; @@ -497,10 +497,19 @@ impl FetchState { } let cache = self.as_cached(handle); - if let Some(fails) = sigrefs::validate(&cache, sigrefs)?.as_mut() { + // N.b. we only validate the existence of the + // default branch for delegates, since it safe for + // non-delegates to not have this branch. + let branch_validation = + validate_project_default_branch(&anchor, &sigrefs.sigrefs); + let fails = sigrefs::validate(&cache, sigrefs)?.map(|mut fails| { + fails.extend(branch_validation); + fails + }); + if let Some(mut fails) = fails { log::warn!(target: "fetch", "Pruning delegate {remote} tips, due to validation failures"); self.prune(&remote); - failures.append(fails) + failures.append(&mut fails) } else { remotes.insert(remote); } @@ -648,3 +657,20 @@ impl<'a, S> ValidateRepository for Cached<'a, S> { Ok(validations) } } + +/// If the repository has a project payload, in `anchor`, then +/// validate that the `sigrefs` contains the listed default branch. +/// +/// N.b. if the repository does not have the project payload or a +/// deserialization error occurs, then this will return `None`. +fn validate_project_default_branch( + anchor: &Doc, + sigrefs: &SignedRefs, +) -> Option { + let proj = anchor.project().ok()?; + let branch = radicle::git::refs::branch(proj.default_branch()).to_ref_string(); + (!sigrefs.contains_key(&branch)).then_some(Validation::MissingRef { + remote: sigrefs.id, + refname: branch, + }) +} diff --git a/radicle-node/src/tests/e2e.rs b/radicle-node/src/tests/e2e.rs index 4639d87e..a8dd06ba 100644 --- a/radicle-node/src/tests/e2e.rs +++ b/radicle-node/src/tests/e2e.rs @@ -4,8 +4,8 @@ use radicle::crypto::{test::signer::MockSigner, Signer}; use radicle::git; use radicle::node::{Alias, FetchResult, Handle as _, DEFAULT_TIMEOUT}; use radicle::storage::{ - ReadRepository, ReadStorage, RefUpdate, RemoteRepository, ValidateRepository, WriteRepository, - WriteStorage, + ReadRepository, ReadStorage, RefUpdate, RemoteRepository, SignRepository, ValidateRepository, + WriteRepository, WriteStorage, }; use radicle::test::fixtures; use radicle::{assert_matches, rad}; @@ -1039,3 +1039,55 @@ fn test_outdated_delegate_sigrefs() { assert_ne!(alice_refs, old_refs); assert_eq!(alice_refs_expected, alice_refs); } + +#[test] +fn missing_default_branch() { + logger::init(log::Level::Debug); + + let tmp = tempfile::tempdir().unwrap(); + + let mut alice = Node::init(tmp.path(), Config::test(Alias::new("alice"))); + let bob = Node::init(tmp.path(), Config::test(Alias::new("bob"))); + + let rid = alice.project("acme", ""); + + let mut alice = alice.spawn(); + let mut bob = bob.spawn(); + + alice.handle.track_repo(rid, Scope::All).unwrap(); + bob.handle.track_repo(rid, Scope::All).unwrap(); + alice.connect(&bob); + converge([&alice, &bob]); + + bob.handle.fetch(rid, alice.id, DEFAULT_TIMEOUT).unwrap(); + assert!(bob.storage.contains(&rid).unwrap()); + + // Fetching from still works despite not having + // `refs/heads/master`, but has `rad/sigrefs`. + bob.issue(rid, "Hello, Acme", "Popping in to say hello"); + alice.handle.fetch(rid, bob.id, DEFAULT_TIMEOUT).unwrap(); + + { + let repo = bob.storage.repository(rid).unwrap(); + assert!(repo.canonical_head().is_ok()); + assert!(repo.canonical_identity_doc().is_ok()); + assert!(repo.head().is_ok()); + } + + // If for some reason Alice managed to delete her master reference + { + let repo = alice.storage.repository_mut(rid).unwrap(); + let mut r = repo + .backend + .find_reference(&format!("refs/namespaces/{}/refs/heads/master", alice.id)) + .unwrap(); + r.delete().unwrap(); + repo.sign_refs(&alice.signer).unwrap(); + } + + // Then fetching from her will fail + assert_matches!( + bob.handle.fetch(rid, alice.id, DEFAULT_TIMEOUT).unwrap(), + FetchResult::Failed { .. } + ); +}