Radish alpha
h
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
radicle/sigrefs: Detect features when writing
Lorenz Leutgeb committed 1 month ago
commit 8bc3ffc0b0ffba55235b1c2ef79e02f806297cd6
parent 372a787
10 files changed +308 -211
modified crates/radicle-crypto/src/test/signer.rs
@@ -22,6 +22,12 @@ impl signature::Signer<Signature> for MockSigner {
    }
}

+
impl signature::Verifier<Signature> for MockSigner {
+
    fn verify(&self, msg: &[u8], signature: &Signature) -> Result<(), signature::Error> {
+
        self.pk.verify(msg, signature)
+
    }
+
}
+

impl AsRef<PublicKey> for MockSigner {
    fn as_ref(&self) -> &PublicKey {
        &self.pk
modified crates/radicle/src/storage/refs.rs
@@ -78,40 +78,23 @@ impl Refs {
        signer: &S,
    ) -> Result<SignedRefsAt, Error>
    where
-
        S: signature::Signer<crypto::Signature>,
        R: sigrefs::git::object::Reader + sigrefs::git::object::Writer,
        R: sigrefs::git::reference::Reader + sigrefs::git::reference::Writer,
+
        R: HasRepoId,
+
        S: signature::Signer<crypto::Signature>,
+
        S: signature::Verifier<crypto::Signature>,
    {
-
        // FIXME(lorenz): We promise this feature level to the caller, but
-
        // have not actually verified it.
-
        const LEVEL_PROMISED: FeatureLevel = FeatureLevel::Parent;
-

        let msg = "Update signed refs\n";
        let reflog = format!("Save {} signed references", self.len());
-
        let update = sigrefs::write::SignedRefsWriter::new(self, namespace, repo, signer).write(
-
            committer,
-
            msg.to_string(),
-
            reflog,
-
        )?;
+
        let update =
+
            sigrefs::write::SignedRefsWriter::new(self, repo.rid(), namespace, repo, signer)
+
                .write(committer, msg.to_string(), reflog)?;
        match update {
-
            sigrefs::write::Update::Changed { entry } => {
-
                Ok(entry.into_sigrefs_at(namespace, LEVEL_PROMISED))
+
            sigrefs::write::Update::Changed { entry, level } => {
+
                Ok(entry.into_sigrefs_at(namespace, level))
            }
-
            sigrefs::write::Update::Unchanged {
-
                commit,
-
                refs,
-
                signature,
-
            } => {
-
                let sigrefs = SignedRefs {
-
                    refs,
-
                    signature,
-
                    id: namespace,
-
                    level: LEVEL_PROMISED,
-
                };
-
                Ok(SignedRefsAt {
-
                    sigrefs,
-
                    at: commit,
-
                })
+
            sigrefs::write::Update::Unchanged { verified } => {
+
                Ok(verified.into_sigrefs_at(namespace))
            }
        }
    }
modified crates/radicle/src/storage/refs/sigrefs/property.rs
@@ -25,7 +25,13 @@ fn roundtrip(BoundedVec(all_refs): BoundedVec<Refs>) -> TestResult {
    for refs in all_refs {
        let refs = fixture.with_identity_root(refs);

-
        let writer = SignedRefsWriter::new(refs.clone(), node_id, fixture.repo(), &signer);
+
        let writer = SignedRefsWriter::new(
+
            refs.clone(),
+
            fixture.rid(),
+
            node_id,
+
            fixture.repo(),
+
            &signer,
+
        );
        let update = match writer.write(
            fixture.committer(),
            "roundtrip write".into(),
@@ -36,8 +42,8 @@ fn roundtrip(BoundedVec(all_refs): BoundedVec<Refs>) -> TestResult {
        };

        let written_refs = match update {
-
            Update::Changed { ref entry } => entry.clone().into_refs(),
-
            Update::Unchanged { ref refs, .. } => refs.clone(),
+
            Update::Changed { ref entry, .. } => entry.clone().into_refs(),
+
            Update::Unchanged { ref verified, .. } => verified.commit().refs().clone(),
        };

        assert_eq!(refs, written_refs);
@@ -68,7 +74,14 @@ fn idempotent(refs: Refs) -> TestResult {
    let signer = MockSigner::default();
    let node_id = *signer.public_key();

-
    if let Err(e) = SignedRefsWriter::new(refs.clone(), node_id, fixture.repo(), &signer).write(
+
    if let Err(e) = SignedRefsWriter::new(
+
        refs.clone(),
+
        fixture.rid(),
+
        node_id,
+
        fixture.repo(),
+
        &signer,
+
    )
+
    .write(
        fixture.committer(),
        "first write".into(),
        "first reflog".into(),
@@ -76,7 +89,14 @@ fn idempotent(refs: Refs) -> TestResult {
        return TestResult::error(format!("first write error: {e}"));
    }

-
    match SignedRefsWriter::new(refs.clone(), node_id, fixture.repo(), &signer).write(
+
    match SignedRefsWriter::new(
+
        refs.clone(),
+
        fixture.rid(),
+
        node_id,
+
        fixture.repo(),
+
        &signer,
+
    )
+
    .write(
        fixture.committer(),
        "second write".into(),
        "second reflog".into(),
modified crates/radicle/src/storage/refs/sigrefs/read.rs
@@ -45,7 +45,8 @@ impl VerifiedCommit {
    }

    // The [`FeatureLevel`] of the refs in this commit.
-
    pub fn level(&self) -> FeatureLevel {
+
    #[cfg(test)]
+
    pub(super) fn level(&self) -> FeatureLevel {
        self.level
    }

@@ -273,13 +274,11 @@ where
            let commit = verified.commit();

            let commits = seen.get(&commit.signature).expect(SIGNATURES_COLLECTED);
-

            if commits.len_nonzero() == ONE {
                return Ok(verified);
            }

            let id = &commit.oid;
-

            if id == commits.last() {
                // If this commit is the last element of `commits`,
                // then this means it is the earliest of all that share
@@ -325,12 +324,33 @@ pub(super) struct Commit {
}

impl Commit {
+
    /// The [`Oid`] of this commit.
+
    pub(super) fn oid(&self) -> &Oid {
+
        &self.oid
+
    }
+

+
    /// The parent [`Oid`] of the commit, unless it is the root commit.
    #[cfg(test)]
+
    pub(super) fn parent(&self) -> Option<&Oid> {
+
        self.parent.as_ref()
+
    }
+

+
    /// The [`Refs`] found in the blob at `/refs` in in the tree of this commit.
    pub(super) fn refs(&self) -> &Refs {
        &self.refs
    }

-
    fn verify<V>(mut self, expected: RepoId, verifier: &V) -> Result<VerifiedCommit, error::Verify>
+
    /// The [`crypto::Signature`] found in blob at `/signature` in the tree of the commit.
+
    #[cfg(test)]
+
    pub(super) fn signature(&self) -> &crypto::Signature {
+
        &self.signature
+
    }
+

+
    pub(super) fn verify<V>(
+
        mut self,
+
        expected: RepoId,
+
        verifier: &V,
+
    ) -> Result<VerifiedCommit, error::Verify>
    where
        V: signature::Verifier<crypto::Signature>,
    {
@@ -424,7 +444,7 @@ impl Commit {
    }
}

-
struct CommitReader<'a, R> {
+
pub(super) struct CommitReader<'a, R> {
    commit: Oid,
    repository: &'a R,
}
@@ -433,11 +453,11 @@ impl<'a, R> CommitReader<'a, R>
where
    R: object::Reader,
{
-
    fn new(commit: Oid, repository: &'a R) -> Self {
+
    pub(super) fn new(commit: Oid, repository: &'a R) -> Self {
        Self { commit, repository }
    }

-
    fn read(self) -> Result<Commit, error::Commit> {
+
    pub(super) fn read(self) -> Result<Commit, error::Commit> {
        let commit = self.read_commit_data()?;
        let Tree { refs, signature } = TreeReader::new(self.commit, self.repository)
            .read()
@@ -552,7 +572,7 @@ where
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-
struct IdentityRoot {
+
pub(super) struct IdentityRoot {
    commit: Oid,
    rid: RepoId,
}
modified crates/radicle/src/storage/refs/sigrefs/read/test/signed_refs_reader.rs
@@ -1,7 +1,7 @@
use radicle_oid::Oid;

use crate::storage::refs::sigrefs::read::error::{Read, Verify};
-
use crate::storage::refs::sigrefs::read::{error, Commit, FeatureLevels, SignedRefsReader, Tip};
+
use crate::storage::refs::sigrefs::read::{error, FeatureLevels, SignedRefsReader, Tip};
use crate::storage::refs::sigrefs::VerifiedCommit;
use crate::storage::refs::{FeatureLevel, IDENTITY_ROOT, SIGREFS_PARENT};
use crate::{assert_matches, git};
@@ -386,20 +386,8 @@ fn read_ok_no_parent() {

    let vc = read(c1, repo).unwrap();
    assert_eq!(vc.commit.oid, c1);
-

-
    assert_matches!(
-
        vc,
-
        VerifiedCommit {
-
            commit: Commit {
-
                oid: _,
-
                parent: Some(_),
-
                refs: _,
-
                signature: _,
-
                identity_root: Some(_)
-
            },
-
            level: FeatureLevel::Root,
-
        }
-
    );
+
    assert_eq!(vc.level(), FeatureLevel::Root);
+
    assert_eq!(vc.commit.parent().copied(), Some(c2));
}

#[test]
@@ -418,14 +406,7 @@ fn read_ok_root() {
    let vc = read(c1, repo).unwrap();
    assert_eq!(vc.commit.oid, c1);
    assert_eq!(vc.commit.parent, Some(c2));
-

-
    assert_matches!(vc, VerifiedCommit { commit: Commit {
-
        oid,
-
        parent: Some(parent),
-
        refs: _,
-
        signature: _,
-
        identity_root: Some(_)
-
    }, level: FeatureLevel::Root } if parent == c2 && oid == c1);
+
    assert_eq!(vc.level, FeatureLevel::Root);
}

#[test]
@@ -443,14 +424,8 @@ fn read_ok_parent() {

    let vc = read(c1, repo).unwrap();
    assert_eq!(vc.commit.oid, c1);
-

-
    assert_matches!(vc, VerifiedCommit { commit: Commit {
-
        oid,
-
        parent: Some(parent),
-
        refs: _,
-
        signature: _,
-
        identity_root: Some(_)
-
    }, level: FeatureLevel::Parent } if parent == c2 && oid == c1);
+
    assert_eq!(vc.level, FeatureLevel::Parent);
+
    assert_eq!(vc.commit.parent, Some(c2));
}

#[test]
modified crates/radicle/src/storage/refs/sigrefs/write.rs
@@ -5,56 +5,47 @@ mod test;

use std::path::Path;

-
use crypto::signature::Signer;
+
use crypto::signature::{self, Signer};
use crypto::PublicKey;
-
use radicle_core::NodeId;
+
use radicle_core::{NodeId, RepoId};
use radicle_git_metadata::author::Author;
use radicle_git_metadata::commit::{headers::Headers, trailers::OwnedTrailer, CommitData};
use radicle_oid::Oid;

use crate::git;
use crate::storage::refs::sigrefs::git::{object, reference, Committer};
+
use crate::storage::refs::sigrefs::read::CommitReader;
+
use crate::storage::refs::sigrefs::{read, VerifiedCommit};
use crate::storage::refs::{
    FeatureLevel, Refs, IDENTITY_ROOT, REFS_BLOB_PATH, SIGNATURE_BLOB_PATH, SIGREFS_BRANCH,
    SIGREFS_PARENT,
};
use crate::storage::refs::{SignedRefs, SignedRefsAt};

-
/// The result of calling [`SignedRefsWriter::write`].
+
/// The result of attempting to write signed references using
+
/// [`SignedRefsWriter`].
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum Update {
    /// The new signed references commit was written to the Git repository.
-
    Changed { entry: Box<Commit> },
+
    Changed {
+
        entry: Box<Commit>,
+
        level: FeatureLevel,
+
    },
    /// The provided [`Refs`] were equal to the current [`Refs`], so the process
    /// exited early.
-
    Unchanged {
-
        commit: Oid,
-
        refs: Refs,
-
        signature: crypto::Signature,
-
    },
+
    Unchanged { verified: VerifiedCommit },
}

-
impl From<Commit> for Update {
-
    fn from(commit: Commit) -> Self {
+
impl Update {
+
    fn changed(commit: Commit, level: FeatureLevel) -> Self {
        Self::Changed {
            entry: Box::new(commit),
+
            level,
        }
    }
-
}

-
impl From<Head> for Update {
-
    fn from(
-
        Head {
-
            commit,
-
            refs,
-
            signature,
-
        }: Head,
-
    ) -> Self {
-
        Self::Unchanged {
-
            commit,
-
            refs,
-
            signature,
-
        }
+
    fn unchanged(verified: VerifiedCommit) -> Self {
+
        Self::Unchanged { verified }
    }
}

@@ -73,6 +64,7 @@ impl From<Head> for Update {
///   over the [`Refs`].
pub struct SignedRefsWriter<'a, R, S> {
    refs: Refs,
+
    rid: RepoId,
    namespace: NodeId,
    repository: &'a R,
    signer: &'a S,
@@ -82,20 +74,28 @@ impl<'a, R, S> SignedRefsWriter<'a, R, S>
where
    R: object::Writer + object::Reader + reference::Writer + reference::Reader,
    S: Signer<crypto::Signature>,
+
    S: signature::Verifier<crypto::Signature>,
{
    /// Construct a new [`SignedRefsWriter`].
    ///
    /// The construction removes the ref [`SIGREFS_BRANCH`] from [`Refs`]
    /// (if present).
    ///
-
    /// When calling [`SignedRefsWriter::write`], if the process is successful,
+
    /// When calling writing signed references, if the process is successful,
    /// the given [`Refs`] will be written to the provided `namespace`.
-
    pub fn new(mut refs: Refs, namespace: NodeId, repository: &'a R, signer: &'a S) -> Self {
+
    pub fn new(
+
        mut refs: Refs,
+
        rid: RepoId,
+
        namespace: NodeId,
+
        repository: &'a R,
+
        signer: &'a S,
+
    ) -> Self {
        debug_assert!(refs.get(&IDENTITY_ROOT).is_some());
        debug_assert!(refs.get(&SIGREFS_PARENT).is_none());
        refs.remove_sigrefs();
        Self {
            refs,
+
            rid,
            namespace,
            repository,
            signer,
@@ -128,26 +128,33 @@ where
        let author = committer.into_inner();
        let Self {
            refs,
+
            rid,
            namespace,
            repository,
            signer,
        } = self;
        let reference = SIGREFS_BRANCH.with_namespace(git::fmt::Component::from(&namespace));
-
        let head = HeadReader::new(&reference, repository)
+

+
        let head = HeadReader::new(&reference, repository, rid, self.signer)
            .read()
            .map_err(error::Write::Head)?;
        let commit_writer = match head {
-
            Some(head) if head.is_unchanged(&refs) => return Ok(Update::from(head)),
-
            Some(head) => {
-
                CommitWriter::with_parent(refs, head.commit, author, message, repository, signer)
-
            }
+
            Some(head) if head.is_unchanged(&refs) => return Ok(Update::unchanged(head.verified)),
+
            Some(head) => CommitWriter::with_parent(
+
                refs,
+
                *head.verified.commit().oid(),
+
                author,
+
                message,
+
                repository,
+
                signer,
+
            ),
            None => CommitWriter::root(refs, author, message, repository, signer),
        };
        let commit = commit_writer.write().map_err(error::Write::Commit)?;
        repository
            .write_reference(&reference, commit.oid, commit.parent, reflog)
            .map_err(error::Write::Reference)?;
-
        Ok(Update::from(commit))
+
        Ok(Update::changed(commit, FeatureLevel::Parent))
    }
}

@@ -317,37 +324,43 @@ where
/// references payload.
#[derive(Clone, Debug, PartialEq, Eq)]
struct Head {
-
    /// The commit [`Oid`] at the head of the reference.
-
    commit: Oid,
-
    /// The [`Refs`] found within the head commit.
-
    refs: Refs,
-
    /// The [`crypto::Signature`] over the [`Refs`] blob.
-
    signature: crypto::Signature,
+
    /// The verified commit at the head of the reference.
+
    verified: VerifiedCommit,
}

impl Head {
    /// Returns `true` if the `proposed` [`Refs`] are equal to the [`Refs`]
    /// of the [`Head`].
    fn is_unchanged(&self, proposed: &Refs) -> bool {
-
        self.refs == *proposed
+
        self.verified.commit().refs() == proposed
    }
}

-
struct HeadReader<'a, 'b, R> {
+
struct HeadReader<'a, 'b, 'c, R, V> {
+
    rid: RepoId,
    reference: &'a git::fmt::Namespaced<'a>,
    repository: &'b R,
+
    verifier: &'c V,
}

-
impl<'a, 'b, R> HeadReader<'a, 'b, R>
+
impl<'a, 'b, 'c, R, V> HeadReader<'a, 'b, 'c, R, V>
where
    R: object::Reader + reference::Reader,
+
    V: signature::Verifier<crypto::Signature>,
{
    /// Construct a [`HeadReader`] with the `reference` that is being read from
    /// the `repository.`
-
    fn new(reference: &'a git::fmt::Namespaced<'a>, repository: &'b R) -> Self {
+
    fn new(
+
        reference: &'a git::fmt::Namespaced<'a>,
+
        repository: &'b R,
+
        rid: RepoId,
+
        verifier: &'c V,
+
    ) -> Self {
        Self {
+
            rid,
            reference,
            repository,
+
            verifier,
        }
    }

@@ -358,54 +371,20 @@ where
    ///
    /// The returned [`Refs`] do not contain the [`SIGREFS_PARENT`] reference.
    fn read(self) -> Result<Option<Head>, error::Head> {
-
        self.repository
+
        let Some(oid) = self
+
            .repository
            .find_reference(self.reference)
            .map_err(error::Head::Reference)?
-
            .map(|commit| self.with_refs(commit))
-
            .transpose()
-
    }
-

-
    fn with_refs(&self, commit: Oid) -> Result<Head, error::Head> {
-
        let refs = self.refs(commit)?;
-
        let signature = self.refs_signature(commit)?;
-
        Ok(Head {
-
            commit,
-
            refs,
-
            signature,
-
        })
-
    }
+
        else {
+
            return Ok(None);
+
        };

-
    fn refs(&self, commit: Oid) -> Result<Refs, error::Head> {
-
        let path = Path::new(REFS_BLOB_PATH);
-
        let object::Blob { bytes, .. } = self
-
            .repository
-
            .read_blob(&commit, path)
-
            .map_err(error::Head::Blob)?
-
            .ok_or(error::Head::MissingPath {
-
                commit,
-
                path: path.to_path_buf(),
-
            })?;
-

-
        let mut refs = Refs::from_canonical(&bytes).map_err(error::Head::Refs)?;
-
        refs.remove_parent();
-
        Ok(refs)
-
    }
+
        let verified = CommitReader::new(oid, self.repository)
+
            .read()
+
            .map_err(error::Head::Commit)?
+
            .verify(self.rid, self.verifier)
+
            .map_err(error::Head::Verify)?;

-
    fn refs_signature(&self, commit: Oid) -> Result<crypto::Signature, error::Head> {
-
        let path = Path::new(SIGNATURE_BLOB_PATH);
-
        let object::Blob {
-
            bytes: sig_bytes, ..
-
        } = self
-
            .repository
-
            .read_blob(&commit, path)
-
            .map_err(error::Head::Blob)?
-
            .ok_or(error::Head::MissingPath {
-
                commit,
-
                path: path.to_path_buf(),
-
            })?;
-
        crypto::Signature::try_from(sig_bytes.as_slice()).map_err(|err| error::Head::Signature {
-
            commit,
-
            source: err,
-
        })
+
        Ok(Some(Head { verified }))
    }
}
modified crates/radicle/src/storage/refs/sigrefs/write/error.rs
@@ -1,9 +1,5 @@
-
use std::path::PathBuf;
-

-
use radicle_oid::Oid;
use thiserror::Error;

-
use crate::storage::refs::canonical;
use crate::storage::refs::sigrefs::git::{object, reference};

// TODO: use commit NID (and RID?) for traceability
@@ -46,13 +42,7 @@ pub enum Head {
    #[error(transparent)]
    Reference(reference::error::FindReference),
    #[error(transparent)]
-
    Blob(object::error::ReadBlob),
+
    Commit(super::read::error::Commit),
    #[error(transparent)]
-
    Refs(canonical::Error),
-
    #[error("failed to parse refs signature in commit {commit}")]
-
    Signature { commit: Oid, source: crypto::Error },
-
    #[error(
-
        "could not find the references blob, within the commit '{commit}', under the path {path:?}"
-
    )]
-
    MissingPath { commit: Oid, path: PathBuf },
+
    Verify(super::read::error::Verify),
}
modified crates/radicle/src/storage/refs/sigrefs/write/test/head_reader.rs
@@ -1,14 +1,22 @@
use radicle_git_ref_format::Component;

use super::mock::{self, MockRepository};
+
use crate::storage::refs::sigrefs::read;
use crate::storage::refs::sigrefs::write::{error, Head, HeadReader};
-
use crate::storage::refs::{Refs, SIGREFS_BRANCH};
+
use crate::storage::refs::{Refs, IDENTITY_ROOT, SIGREFS_BRANCH};

/// Drive `HeadReader` directly via the sigrefs reference for `mock::node_id()`.
fn read(repo: &MockRepository) -> Result<Option<Head>, error::Head> {
    let namespace = mock::node_id();
    let reference = SIGREFS_BRANCH.with_namespace(Component::from(&namespace));
-
    HeadReader::new(&reference, repo).read()
+
    HeadReader::new(&reference, repo, mock::rid(), &mock::AlwaysSign).read()
+
}
+

+
fn refs() -> [(crate::git::fmt::RefString, radicle_oid::Oid); 2] {
+
    [
+
        (mock::refs_heads_main(), mock::oid(10)),
+
        (IDENTITY_ROOT.to_ref_string(), mock::oid(99)),
+
    ]
}

#[test]
@@ -28,8 +36,14 @@ fn refs_blob_error() {
    let head = mock::oid(1);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
+
        .with_commit(head, mock::commit_data([]))
        .with_refs_error(head);
-
    assert!(matches!(read(&repo), Err(error::Head::Blob(_))));
+
    assert!(matches!(
+
        read(&repo),
+
        Err(error::Head::Commit(read::error::Commit::Tree(
+
            read::error::Tree::Refs(_)
+
        )))
+
    ));
}

#[test]
@@ -37,9 +51,15 @@ fn refs_blob_missing() {
    let head = mock::oid(1);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
+
        .with_commit(head, mock::commit_data([]))
        .with_missing_refs(head)
        .with_signature(head, 1);
-
    assert!(matches!(read(&repo), Err(error::Head::MissingPath { .. })));
+
    assert!(matches!(
+
        read(&repo),
+
        Err(error::Head::Commit(read::error::Commit::Tree(
+
            read::error::Tree::MissingBlobs(read::error::MissingBlobs::Signature { .. })
+
        )))
+
    ));
}

#[test]
@@ -47,9 +67,15 @@ fn refs_parse_error() {
    let head = mock::oid(1);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
+
        .with_commit(head, mock::commit_data([]))
        .with_invalid_refs(head)
        .with_signature(head, 1);
-
    assert!(matches!(read(&repo), Err(error::Head::Refs(_))));
+
    assert!(matches!(
+
        read(&repo),
+
        Err(error::Head::Commit(read::error::Commit::Tree(
+
            read::error::Tree::ParseRefs(_)
+
        )))
+
    ));
}

#[test]
@@ -57,9 +83,15 @@ fn signature_blob_error() {
    let head = mock::oid(1);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
-
        .with_refs(head, [(mock::refs_heads_main(), mock::oid(10))])
+
        .with_commit(head, mock::commit_data([]))
+
        .with_refs(head, refs())
        .with_signature_error(head);
-
    assert!(matches!(read(&repo), Err(error::Head::Blob(_))));
+
    assert!(matches!(
+
        read(&repo),
+
        Err(error::Head::Commit(read::error::Commit::Tree(
+
            read::error::Tree::Signature(_)
+
        )))
+
    ));
}

#[test]
@@ -67,9 +99,15 @@ fn signature_blob_missing() {
    let head = mock::oid(1);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
-
        .with_refs(head, [(mock::refs_heads_main(), mock::oid(10))])
+
        .with_commit(head, mock::commit_data([]))
+
        .with_refs(head, refs())
        .with_missing_signature(head);
-
    assert!(matches!(read(&repo), Err(error::Head::MissingPath { .. })));
+
    assert!(matches!(
+
        read(&repo),
+
        Err(error::Head::Commit(read::error::Commit::Tree(
+
            read::error::Tree::MissingBlobs(read::error::MissingBlobs::Refs { .. })
+
        )))
+
    ));
}

#[test]
@@ -77,25 +115,35 @@ fn signature_parse_error() {
    let head = mock::oid(1);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
-
        .with_refs(head, [(mock::refs_heads_main(), mock::oid(10))])
+
        .with_commit(head, mock::commit_data([]))
+
        .with_refs(head, refs())
        .with_invalid_signature(head);
-
    assert!(matches!(read(&repo), Err(error::Head::Signature { .. })));
+
    assert!(matches!(
+
        read(&repo),
+
        Err(error::Head::Commit(read::error::Commit::Tree(
+
            read::error::Tree::ParseSignature(_)
+
        )))
+
    ));
}

#[test]
fn read_ok() {
-
    let head = mock::oid(1);
-
    let refs = [(mock::refs_heads_main(), mock::oid(10))];
+
    let oid = mock::oid(1);
    let repo = MockRepository::new()
-
        .with_rad_sigrefs(&mock::node_id(), head)
-
        .with_refs(head, refs.clone())
-
        .with_signature(head, 1);
+
        .with_rad_sigrefs(&mock::node_id(), oid)
+
        .with_commit(oid, mock::commit_data([]))
+
        .with_refs(oid, refs())
+
        .with_signature(oid, 1);
+
    let head = read(&repo).unwrap().unwrap();
+

+
    assert_eq!(head.verified.commit().oid(), &oid);
+
    assert_eq!(
+
        head.verified.commit().signature(),
+
        &crypto::Signature::from([1; 64])
+
    );
+
    assert_eq!(head.verified.commit().parent(), None);
    assert_eq!(
-
        read(&repo).unwrap(),
-
        Some(Head {
-
            commit: head,
-
            refs: Refs::from(refs.into_iter()),
-
            signature: crypto::Signature::from([1; 64]),
-
        })
+
        *head.verified.commit().refs(),
+
        Refs::from(refs().into_iter())
    );
}
modified crates/radicle/src/storage/refs/sigrefs/write/test/mock.rs
@@ -2,13 +2,20 @@ use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::str::FromStr as _;

-
use radicle_core::NodeId;
+
use radicle_core::{NodeId, RepoId};
use radicle_git_metadata::author::{Author, Time};
+
use radicle_git_metadata::commit::headers::Headers;
+
use radicle_git_metadata::commit::trailers::OwnedTrailer;
+
use radicle_git_metadata::commit::CommitData;
use radicle_oid::Oid;

use crate::git;
+
use crate::identity::doc;
use crate::storage::refs::sigrefs::git::{object, reference};
use crate::storage::refs::{Refs, REFS_BLOB_PATH, SIGNATURE_BLOB_PATH, SIGREFS_BRANCH};
+
use crate::storage::HasRepoId;
+

+
const MOCKED_IDENTITY: u8 = 99u8;

enum WriteTreeBehavior {
    /// [`object::Writer::write_tree`] returns `Ok(oid)`.
@@ -46,6 +53,7 @@ enum RefBehavior {
}

pub struct MockRepository {
+
    commits: HashMap<Oid, CommitBehavior>,
    blobs: HashMap<(Oid, PathBuf), BlobBehavior>,
    references: HashMap<String, RefBehavior>,
    write_tree: Option<WriteTreeBehavior>,
@@ -53,15 +61,28 @@ pub struct MockRepository {
    write_reference: Option<WriteReferenceBehavior>,
}

+
enum CommitBehavior {
+
    /// [`object::Reader::read_commit`] returns `Ok(Some(bytes))`.
+
    Present(Box<CommitData<Oid, Oid>>),
+
}
+

impl MockRepository {
    pub fn new() -> MockRepository {
        MockRepository {
+
            commits: HashMap::new(),
            blobs: HashMap::new(),
            references: HashMap::new(),
            write_tree: None,
            write_commit: None,
            write_reference: None,
        }
+
        .with_identity(oid(MOCKED_IDENTITY))
+
    }
+

+
    pub fn with_commit(mut self, oid: Oid, data: CommitData<Oid, Oid>) -> Self {
+
        self.commits
+
            .insert(oid, CommitBehavior::Present(Box::new(data)));
+
        self
    }

    pub fn with_rad_sigrefs(mut self, namespace: &NodeId, commit: Oid) -> MockRepository {
@@ -125,6 +146,10 @@ impl MockRepository {
        self.with_blob(commit, Path::new(SIGNATURE_BLOB_PATH), bytes)
    }

+
    pub fn with_identity(self, commit: Oid) -> Self {
+
        self.with_blob(commit, &identity_path(), vec![])
+
    }
+

    fn with_blob(mut self, commit: Oid, path: &Path, bytes: Vec<u8>) -> Self {
        self.blobs
            .insert((commit, path.to_path_buf()), BlobBehavior::Present(bytes));
@@ -169,10 +194,18 @@ impl MockRepository {
    }
}

+
impl HasRepoId for MockRepository {
+
    fn rid(&self) -> radicle_core::RepoId {
+
        rid()
+
    }
+
}
+

impl object::Reader for MockRepository {
-
    fn read_commit(&self, _oid: &Oid) -> Result<Option<Vec<u8>>, object::error::ReadCommit> {
-
        // HeadReader never calls read_commit; this is a no-op placeholder.
-
        Ok(None)
+
    fn read_commit(&self, oid: &Oid) -> Result<Option<Vec<u8>>, object::error::ReadCommit> {
+
        match self.commits.get(oid) {
+
            Some(CommitBehavior::Present(data)) => Ok(Some(data.to_string().as_bytes().to_vec())),
+
            None => Ok(None),
+
        }
    }

    fn read_blob(
@@ -269,6 +302,16 @@ impl crypto::signature::Signer<crypto::Signature> for AlwaysSign {
    }
}

+
impl crypto::signature::Verifier<crypto::Signature> for AlwaysSign {
+
    fn verify(
+
        &self,
+
        _msg: &[u8],
+
        _sig: &crypto::Signature,
+
    ) -> Result<(), crypto::signature::Error> {
+
        Ok(())
+
    }
+
}
+

/// Always fails to sign.
pub struct NeverSign;

@@ -285,6 +328,11 @@ pub fn oid(n: u8) -> Oid {
    Oid::from_sha1([n; 20])
}

+
/// A fixed [`RepoId`] for tests.
+
pub fn rid() -> RepoId {
+
    RepoId::from(oid(MOCKED_IDENTITY))
+
}
+

/// A fixed [`NodeId`] for tests.
pub fn node_id() -> NodeId {
    NodeId::from([1u8; 32])
@@ -303,9 +351,29 @@ pub fn author() -> Author {
    }
}

+
pub fn commit_data(parents: impl IntoIterator<Item = Oid>) -> CommitData<Oid, Oid> {
+
    let tree = oid(0);
+
    let author = author();
+
    let message = "test\n".to_owned();
+

+
    CommitData::new::<_, _, OwnedTrailer>(
+
        tree,
+
        parents,
+
        author.clone(),
+
        author,
+
        Headers::new(),
+
        message,
+
        vec![],
+
    )
+
}
+

fn sigrefs_ref_name(namespace: &NodeId) -> String {
    SIGREFS_BRANCH
        .with_namespace(git::fmt::Component::from(namespace))
        .as_str()
        .to_owned()
}
+

+
fn identity_path() -> PathBuf {
+
    Path::new("embeds").join(*doc::PATH)
+
}
modified crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs
@@ -2,7 +2,7 @@ use radicle_oid::Oid;

use crate::storage::refs::sigrefs::git::Committer;
use crate::storage::refs::sigrefs::write::{error, SignedRefsWriter, Update};
-
use crate::storage::refs::{Refs, IDENTITY_ROOT, SIGREFS_BRANCH};
+
use crate::storage::refs::{FeatureLevel, Refs, IDENTITY_ROOT, SIGREFS_BRANCH};

use super::mock;
use super::mock::MockRepository;
@@ -18,7 +18,13 @@ fn some_refs(identity_root: Oid) -> Refs {
}

fn other_refs() -> Refs {
-
    Refs::from([(mock::refs_heads_main(), mock::oid(20))].into_iter())
+
    Refs::from(
+
        [
+
            (mock::refs_heads_main(), mock::oid(20)),
+
            (IDENTITY_ROOT.to_ref_string(), mock::oid(99)),
+
        ]
+
        .into_iter(),
+
    )
}

fn refs_with_rad_sigrefs() -> Refs {
@@ -33,7 +39,7 @@ fn refs_with_rad_sigrefs() -> Refs {
}

fn write(refs: Refs, repo: &MockRepository) -> Result<Update, error::Write> {
-
    SignedRefsWriter::new(refs, mock::node_id(), repo, &mock::AlwaysSign).write(
+
    SignedRefsWriter::new(refs, mock::rid(), mock::node_id(), repo, &mock::AlwaysSign).write(
        Committer::new(mock::author()),
        "msg".into(),
        "reflog".into(),
@@ -55,16 +61,17 @@ fn unchanged() {
    let refs = some_refs(mock::oid(99));
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
+
        .with_commit(head, mock::commit_data([]))
        .with_refs(head, refs.clone())
        .with_signature(head, 1);
-
    assert_eq!(
-
        write(refs.clone(), &repo).unwrap(),
-
        Update::Unchanged {
-
            commit: head,
-
            refs,
-
            signature: crypto::Signature::from([1; 64]),
+

+
    match write(refs.clone(), &repo).unwrap() {
+
        Update::Unchanged { verified } => {
+
            assert_eq!(verified.commit().oid(), &head);
+
            assert_eq!(verified.level(), FeatureLevel::Root);
        }
-
    );
+
        Update::Changed { .. } => unreachable!(),
+
    }
}

#[test]
@@ -102,7 +109,7 @@ fn write_root_ok() {
        .with_write_reference_ok();
    let refs = some_refs(mock::oid(99));
    let update = write(refs.clone(), &repo).unwrap();
-
    let Update::Changed { entry } = update else {
+
    let Update::Changed { entry, .. } = update else {
        panic!("expected Update::Changed, got {update:?}");
    };
    assert_eq!(entry.parent, None);
@@ -116,6 +123,7 @@ fn write_with_parent_ok() {
    let commit_oid = mock::oid(42);
    let repo = MockRepository::new()
        .with_rad_sigrefs(&mock::node_id(), head)
+
        .with_commit(head, mock::commit_data([]))
        .with_refs(head, other_refs())
        .with_signature(head, 1)
        .with_write_tree_ok(mock::oid(99))
@@ -123,7 +131,7 @@ fn write_with_parent_ok() {
        .with_write_reference_ok();
    let refs = some_refs(mock::oid(99));
    let update = write(refs.clone(), &repo).unwrap();
-
    let Update::Changed { entry } = update else {
+
    let Update::Changed { entry, .. } = update else {
        panic!("expected Update::Changed, got {update:?}");
    };
    assert_eq!(entry.parent, Some(head));
@@ -142,7 +150,7 @@ fn write_empty_refs() {
        .with_write_commit_ok(commit_oid)
        .with_write_reference_ok();
    let update = write(refs.clone(), &repo).unwrap();
-
    let Update::Changed { entry } = update else {
+
    let Update::Changed { entry, .. } = update else {
        panic!("expected Update::Changed, got {update:?}");
    };
    assert_eq!(entry.parent, None);
@@ -160,7 +168,7 @@ fn never_write_rad_sigrefs() {
        .with_write_reference_ok();
    let mut refs = refs_with_rad_sigrefs();
    let update = write(refs.clone(), &repo).unwrap();
-
    let Update::Changed { entry } = update else {
+
    let Update::Changed { entry, .. } = update else {
        panic!("expected Update::Changed, got {update:?}");
    };
    assert_eq!(entry.parent, None);