From 3bff9a9dc11764d9c5b5df18bec9b6e5be7c857f Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Wed, 19 Oct 2022 10:27:24 +0200 Subject: [PATCH] Set repository canonical refs We set the `HEAD` and eg. `refs/heads/master` refs when updating repositories, which allows users to easily clone with git. For now, `HEAD` is chosen based on a unanimity quorum of delegates. This can be made more sophisticated in the future. We've also had to ignore some refs during verification, since these have cropped in for eg. due to the staging copy clone and the changes included here. Signed-off-by: Alexis Sellier --- radicle-remote-helper/src/lib.rs | 3 +- radicle/src/git.rs | 40 ++++++++++++--- radicle/src/identity/project.rs | 2 +- radicle/src/rad.rs | 24 +++------ radicle/src/storage.rs | 10 ++++ radicle/src/storage/git.rs | 83 ++++++++++++++++++++++++++++---- radicle/src/test/storage.rs | 9 ++++ 7 files changed, 134 insertions(+), 37 deletions(-) diff --git a/radicle-remote-helper/src/lib.rs b/radicle-remote-helper/src/lib.rs index 697543eb..0c15250a 100644 --- a/radicle-remote-helper/src/lib.rs +++ b/radicle-remote-helper/src/lib.rs @@ -8,7 +8,7 @@ use thiserror::Error; use radicle::crypto::{PublicKey, Signer}; use radicle::node::Handle; use radicle::ssh; -use radicle::storage::{ReadRepository, WriteStorage}; +use radicle::storage::{ReadRepository, WriteRepository, WriteStorage}; /// The service invoked by git on the remote repository, during a push. const GIT_RECEIVE_PACK: &str = "git-receive-pack"; @@ -180,6 +180,7 @@ pub fn run(profile: radicle::Profile) -> Result<(), Box Result, ListRefsError> let refs = remote.list()?; for r in refs { - let (id, refname) = parse_ref::(r.name())?; - let entry = remotes.entry(id).or_insert_with(Refs::default); - - entry.insert(refname.into(), r.oid().into()); + // Skip the `HEAD` reference, as it is untrusted. + if r.name() == "HEAD" { + continue; + } + // Nb. skip refs that don't have a public key namespace. + if let (Some(id), refname) = parse_ref::(r.name())? { + let entry = remotes.entry(id).or_insert_with(Refs::default); + entry.insert(refname.into(), r.oid().into()); + } } Ok(remotes) } -/// Parse a ref string. -pub fn parse_ref(s: &str) -> Result<(T, format::Qualified), RefError> +/// Parse a ref string. Returns an error if it isn't namespaced. +pub fn parse_ref_namespaced(s: &str) -> Result<(T, format::Qualified), RefError> +where + T: FromStr, + T::Err: std::error::Error + Send + Sync + 'static, +{ + match parse_ref::(s) { + Ok((None, refname)) => Err(RefError::MissingNamespace(refname.to_ref_string())), + Ok((Some(t), r)) => Ok((t, r)), + Err(err) => Err(err), + } +} + +/// Parse a ref string. Optionally returns a namespace. +pub fn parse_ref(s: &str) -> Result<(Option, format::Qualified), RefError> where T: FromStr, T::Err: std::error::Error + Send + Sync + 'static, { let input = format::RefStr::try_from_str(s)?; match input.to_namespaced() { - None => Err(RefError::MissingNamespace(input.to_owned())), + None => { + let refname = Qualified::from_refstr(input) + .ok_or_else(|| RefError::InvalidName(input.to_owned()))?; + + Ok((None, refname)) + } Some(ns) => { let id = ns .namespace() @@ -146,7 +169,8 @@ where err: Box::new(err), })?; let rest = ns.strip_namespace(); - Ok((id, rest)) + + Ok((Some(id), rest)) } } } diff --git a/radicle/src/identity/project.rs b/radicle/src/identity/project.rs index aeb99a30..c1935a92 100644 --- a/radicle/src/identity/project.rs +++ b/radicle/src/identity/project.rs @@ -509,7 +509,7 @@ mod test { let oid = git2::Oid::from_str("2d52a53ce5e4f141148a5f770cfd3ead2d6a45b8").unwrap(); let err = Doc::::head(&remote, &repo).unwrap_err(); - assert!(dbg!(err).is_not_found()); + assert!(err.is_not_found()); let err = Doc::load_at(oid.into(), &repo).unwrap_err(); assert!(err.is_not_found()); diff --git a/radicle/src/rad.rs b/radicle/src/rad.rs index 315016b5..a2f8167f 100644 --- a/radicle/src/rad.rs +++ b/radicle/src/rad.rs @@ -3,7 +3,6 @@ use std::io; use std::path::Path; use std::str::FromStr; -use git_ref_format::{lit, Qualified}; use once_cell::sync::Lazy; use thiserror::Error; @@ -23,6 +22,8 @@ pub static REMOTE_NAME: Lazy = Lazy::new(|| git::refname!("rad") pub enum InitError { #[error("doc: {0}")] Doc(#[from] identity::project::DocError), + #[error("project: {0}")] + Project(#[from] storage::git::ProjectError), #[error("doc: {0}")] DocVerification(#[from] identity::project::VerificationError), #[error("git: {0}")] @@ -76,6 +77,7 @@ pub fn init( git::configure_remote(repo, &REMOTE_NAME, &url)?; git::push(repo, &REMOTE_NAME, pk, [(&default_branch, &default_branch)])?; let signed = storage.sign_refs(&project, signer)?; + let _head = project.set_head()?; Ok((id, signed)) } @@ -158,27 +160,15 @@ pub fn fork( let me = signer.public_key(); let repository = storage.repository(proj)?; let (canonical_id, project) = repository.project_identity()?; + let (canonical_head, _) = repository.head()?; let raw = repository.raw(); - // TODO: Test the fork functions in isolation. - // TODO: Move to function on `Repository`. - let canonical_head = { - let mut heads = Vec::new(); - for delegate in project.delegates.iter() { - let refname = Qualified::from(lit::refs_heads(&project.default_branch)); - let r = repository.reference_oid(&delegate.id, &refname)?.into(); - heads.push(r); - } - match heads.as_slice() { - [head] => Ok(*head), - // FIXME: This branch is not tested. - heads => raw.merge_base_many(heads), - } - }?; + // TODO: We should only get the project HEAD once we've stored the canonical identity + // branch on disk. This way it can use what we stored, instead of recomputing it. raw.reference( &git::refs::storage::branch(me, &project.default_branch), - canonical_head, + *canonical_head, false, &format!("creating default branch for {me}"), )?; diff --git a/radicle/src/storage.rs b/radicle/src/storage.rs index 95a5eeed..5e52fb23 100644 --- a/radicle/src/storage.rs +++ b/radicle/src/storage.rs @@ -59,6 +59,9 @@ pub enum FetchError { Verify(#[from] git::VerifyError), #[error(transparent)] Storage(#[from] Error), + // TODO: This should wrap a more specific error. + #[error("repository head: {0}")] + SetHead(#[from] ProjectError), } pub type RemoteId = PublicKey; @@ -229,6 +232,7 @@ pub trait WriteStorage: ReadStorage { type Repository: WriteRepository; fn repository(&self, proj: Id) -> Result; + // TODO: Move this to `WriteRepository`. fn sign_refs( &self, repository: &Self::Repository, @@ -246,6 +250,11 @@ pub trait ReadRepository { fn blob_at<'a>(&'a self, oid: Oid, path: &'a Path) -> Result, git_ext::Error>; + /// Get the canonical head of this repository. + /// + /// Returns the [`Oid`] as well as the qualified reference name. + fn head(&self) -> Result<(Oid, Qualified), ProjectError>; + /// Get the `reference` for the given `remote`. /// /// Returns `None` is the reference did not exist. @@ -276,6 +285,7 @@ pub trait ReadRepository { pub trait WriteRepository: ReadRepository { fn fetch(&mut self, url: &Url) -> Result, FetchError>; + fn set_head(&self) -> Result; fn raw(&self) -> &git2::Repository; } diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index 7928ec07..2f3ded70 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -28,6 +28,7 @@ pub static REMOTES_GLOB: Lazy = pub static SIGNATURES_GLOB: Lazy = Lazy::new(|| refspec::pattern!("refs/namespaces/*/radicle/signature")); +// TODO: Is this is the wrong place for this type? #[derive(Error, Debug)] pub enum ProjectError { #[error("identity branches diverge from each other")] @@ -228,12 +229,9 @@ impl Repository { Ok(Self { id, backend }) } - pub fn head(&self) -> Result { - // TODO: Find longest history, get document and get head. - // Perhaps we should even set a local `HEAD` or at least `refs/heads/master` - todo!(); - } - + /// Verify all references in the repository, checking that they are signed + /// as part of 'sigrefs'. Also verify that no signed reference is missing + /// from the repository. pub fn verify(&self) -> Result<(), VerifyError> { let mut remotes: HashMap = self .remotes()? @@ -243,15 +241,32 @@ impl Repository { }) .collect::>()?; + // TODO: We could create a higher level `references` method that skips + // canonical/unverifiable refs. for r in self.backend.references()? { let r = r?; + let name = r.name().ok_or(VerifyError::InvalidRef)?; - let oid = r.target().ok_or(VerifyError::InvalidRef)?; + let oid = if let Some(oid) = r.target() { + oid + } else { + // Ignore symbolic refs, eg. `HEAD`. + continue; + }; + let (remote_id, refname) = git::parse_ref::(name)?; + let remote_id = if let Some(remote_id) = remote_id { + remote_id + } else { + // Ignore refs that aren't part of a namespace, eg. canonical refs. + continue; + }; if refname == *refs::SIGNATURE_REF { + // Ignore the signed-refs reference, as this is what we're verifying. continue; } + let remote = remotes .get_mut(&remote_id) .ok_or(VerifyError::InvalidRemote(remote_id))?; @@ -358,7 +373,7 @@ impl Repository { |reference| -> Result { let r = reference?; let name = r.name().ok_or(refs::Error::InvalidRef)?; - let (id, _) = git::parse_ref::(name)?; + let (id, _) = git::parse_ref_namespaced::(name)?; Ok(id) }, @@ -376,7 +391,7 @@ impl Repository { |reference| -> Result<_, _> { let r = reference?; let name = r.name().ok_or(refs::Error::InvalidRef)?; - let (id, _) = git::parse_ref::(name)?; + let (id, _) = git::parse_ref_namespaced::(name)?; let remote = self.remote(&id)?; Ok((id, remote)) @@ -440,6 +455,7 @@ impl ReadRepository for Repository { remote: &RemoteId, reference: &git::Qualified, ) -> Result { + // TODO: Use native git2 function for this. let oid = self .reference(remote, reference)? .target() @@ -486,6 +502,31 @@ impl ReadRepository for Repository { fn project_identity(&self) -> Result<(Oid, identity::Doc), ProjectError> { Repository::project(self) } + + fn head(&self) -> Result<(Oid, Qualified), ProjectError> { + // TODO: We shouldn't need to re-construct the history here; we should use the cached + // document head of the "trusted" (local) user for this project. + // In the `fork` function for example, we call Repository::project_identity again, + // This should only be necessary once. + let (_, project) = self.project_identity()?; + let branch_ref = Qualified::from(lit::refs_heads(&project.default_branch)); + let raw = self.raw(); + + let mut heads = Vec::new(); + for delegate in project.delegates.iter() { + let r = self.reference_oid(&delegate.id, &branch_ref)?.into(); + + heads.push(r); + } + + let oid = match heads.as_slice() { + [head] => Ok(*head), + // FIXME: This branch is not tested. + heads => raw.merge_base_many(heads), + }?; + + Ok((oid.into(), branch_ref)) + } } impl WriteRepository for Repository { @@ -598,10 +639,27 @@ impl WriteRepository for Repository { // Fetch from the staging copy into the canonical repo. remote.fetch(refs, Some(&mut opts), None)?; } + // Set repository HEAD for git cloning support. + self.set_head()?; Ok(updates) } + fn set_head(&self) -> Result { + let head_ref = refname!("HEAD"); + let (head, branch_ref) = self.head()?; + + log::debug!("Setting ref {:?} -> {:?}", &branch_ref, head); + self.raw() + .reference(&branch_ref, *head, true, "set-local-branch (radicle)")?; + + log::debug!("Setting ref {:?} -> {:?}", head_ref, branch_ref); + self.raw() + .reference_symbolic(&head_ref, &branch_ref, true, "set-head (radicle)")?; + + Ok(head) + } + fn raw(&self) -> &git2::Repository { &self.backend } @@ -788,6 +846,7 @@ mod tests { .id(); git::push(&proj_repo, "rad", alice_id, [(&refname, &refname)]).unwrap(); alice.sign_refs(&alice_proj_storage, &alice_signer).unwrap(); + alice_proj_storage.set_head().unwrap(); // Have Bob fetch Alice's new commit. let updates = bob.repository(proj_id).unwrap().fetch(&alice_url).unwrap(); @@ -887,7 +946,11 @@ mod tests { target .remote_anonymous(&format!("rad://{}", proj)) .unwrap() - .fetch(&["refs/*:refs/*"], Some(&mut opts), None) + .fetch( + &["refs/namespaces/*:refs/namespaces/*"], + Some(&mut opts), + None, + ) .unwrap(); stream.shutdown(net::Shutdown::Both).unwrap(); diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index 4fefa01d..9198c85c 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; +use git_ref_format as fmt; use git_url::Url; use radicle_git_ext as git_ext; @@ -85,6 +86,10 @@ impl ReadRepository for MockRepository { Ok(true) } + fn head(&self) -> Result<(Oid, fmt::Qualified), ProjectError> { + todo!() + } + fn path(&self) -> &std::path::Path { todo!() } @@ -152,4 +157,8 @@ impl WriteRepository for MockRepository { fn raw(&self) -> &git2::Repository { todo!() } + + fn set_head(&self) -> Result { + todo!() + } }