From 8bc3ffc0b0ffba55235b1c2ef79e02f806297cd6 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Sun, 22 Mar 2026 15:02:27 +0100 Subject: [PATCH] radicle/sigrefs: Detect features when writing In order to respond to the caller with the correct feature level in case sigrefs are to be saved but unchanged, the write side needs to care about feature levels. This is done by sharing more code with the read side. --- crates/radicle-crypto/src/test/signer.rs | 6 + crates/radicle/src/storage/refs.rs | 37 ++--- .../src/storage/refs/sigrefs/property.rs | 30 +++- .../radicle/src/storage/refs/sigrefs/read.rs | 36 +++- .../sigrefs/read/test/signed_refs_reader.rs | 37 +---- .../radicle/src/storage/refs/sigrefs/write.rs | 155 ++++++++---------- .../src/storage/refs/sigrefs/write/error.rs | 14 +- .../refs/sigrefs/write/test/head_reader.rs | 92 ++++++++--- .../storage/refs/sigrefs/write/test/mock.rs | 76 ++++++++- .../sigrefs/write/test/signed_refs_writer.rs | 36 ++-- 10 files changed, 308 insertions(+), 211 deletions(-) diff --git a/crates/radicle-crypto/src/test/signer.rs b/crates/radicle-crypto/src/test/signer.rs index a0cd3196..dbf4822d 100644 --- a/crates/radicle-crypto/src/test/signer.rs +++ b/crates/radicle-crypto/src/test/signer.rs @@ -22,6 +22,12 @@ impl signature::Signer for MockSigner { } } +impl signature::Verifier for MockSigner { + fn verify(&self, msg: &[u8], signature: &Signature) -> Result<(), signature::Error> { + self.pk.verify(msg, signature) + } +} + impl AsRef for MockSigner { fn as_ref(&self) -> &PublicKey { &self.pk diff --git a/crates/radicle/src/storage/refs.rs b/crates/radicle/src/storage/refs.rs index c9c18750..e0e3932d 100644 --- a/crates/radicle/src/storage/refs.rs +++ b/crates/radicle/src/storage/refs.rs @@ -78,40 +78,23 @@ impl Refs { signer: &S, ) -> Result where - S: signature::Signer, R: sigrefs::git::object::Reader + sigrefs::git::object::Writer, R: sigrefs::git::reference::Reader + sigrefs::git::reference::Writer, + R: HasRepoId, + S: signature::Signer, + S: signature::Verifier, { - // 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)) } } } diff --git a/crates/radicle/src/storage/refs/sigrefs/property.rs b/crates/radicle/src/storage/refs/sigrefs/property.rs index c33c0c82..81cf794a 100644 --- a/crates/radicle/src/storage/refs/sigrefs/property.rs +++ b/crates/radicle/src/storage/refs/sigrefs/property.rs @@ -25,7 +25,13 @@ fn roundtrip(BoundedVec(all_refs): BoundedVec) -> 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) -> 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(), diff --git a/crates/radicle/src/storage/refs/sigrefs/read.rs b/crates/radicle/src/storage/refs/sigrefs/read.rs index 04de113d..26aa31b1 100644 --- a/crates/radicle/src/storage/refs/sigrefs/read.rs +++ b/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(mut self, expected: RepoId, verifier: &V) -> Result + /// 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( + mut self, + expected: RepoId, + verifier: &V, + ) -> Result where V: signature::Verifier, { @@ -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 { + pub(super) fn read(self) -> Result { 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, } diff --git a/crates/radicle/src/storage/refs/sigrefs/read/test/signed_refs_reader.rs b/crates/radicle/src/storage/refs/sigrefs/read/test/signed_refs_reader.rs index 4c437f2e..a83a43c6 100644 --- a/crates/radicle/src/storage/refs/sigrefs/read/test/signed_refs_reader.rs +++ b/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] diff --git a/crates/radicle/src/storage/refs/sigrefs/write.rs b/crates/radicle/src/storage/refs/sigrefs/write.rs index 09a573e8..683c25ab 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write.rs +++ b/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 }, + Changed { + entry: Box, + 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 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 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 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, + S: signature::Verifier, { /// 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, { /// 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, 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() - } + else { + return Ok(None); + }; - fn with_refs(&self, commit: Oid) -> Result { - let refs = self.refs(commit)?; - let signature = self.refs_signature(commit)?; - Ok(Head { - commit, - refs, - signature, - }) - } + 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(&self, commit: Oid) -> Result { - 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) - } - - fn refs_signature(&self, commit: Oid) -> Result { - 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 })) } } diff --git a/crates/radicle/src/storage/refs/sigrefs/write/error.rs b/crates/radicle/src/storage/refs/sigrefs/write/error.rs index 47128780..46f5fd7a 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write/error.rs +++ b/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), } diff --git a/crates/radicle/src/storage/refs/sigrefs/write/test/head_reader.rs b/crates/radicle/src/storage/refs/sigrefs/write/test/head_reader.rs index 46c16590..b7d269fe 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write/test/head_reader.rs +++ b/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, 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!( - read(&repo).unwrap(), - Some(Head { - commit: head, - refs: Refs::from(refs.into_iter()), - signature: crypto::Signature::from([1; 64]), - }) + head.verified.commit().signature(), + &crypto::Signature::from([1; 64]) + ); + assert_eq!(head.verified.commit().parent(), None); + assert_eq!( + *head.verified.commit().refs(), + Refs::from(refs().into_iter()) ); } diff --git a/crates/radicle/src/storage/refs/sigrefs/write/test/mock.rs b/crates/radicle/src/storage/refs/sigrefs/write/test/mock.rs index 02e3fef1..600b6ded 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write/test/mock.rs +++ b/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, blobs: HashMap<(Oid, PathBuf), BlobBehavior>, references: HashMap, write_tree: Option, @@ -53,15 +61,28 @@ pub struct MockRepository { write_reference: Option, } +enum CommitBehavior { + /// [`object::Reader::read_commit`] returns `Ok(Some(bytes))`. + Present(Box>), +} + 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) -> 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) -> 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>, object::error::ReadCommit> { - // HeadReader never calls read_commit; this is a no-op placeholder. - Ok(None) + fn read_commit(&self, oid: &Oid) -> Result>, 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 for AlwaysSign { } } +impl crypto::signature::Verifier 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) -> CommitData { + 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) +} diff --git a/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs b/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs index f60e3cf2..55125ba2 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write/test/signed_refs_writer.rs +++ b/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 { - 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);