From 7f19044a727f7776e9d66947e727602cb9e6d2b9 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Sun, 22 Mar 2026 16:39:48 +0100 Subject: [PATCH] radicle/sigrefs/write: Treat error to verify head To prevent from inability to write sigrefs in case of a verification error, swallow verification errors about the head commit, log, and carry on writing. --- crates/radicle-fetch/src/policy.rs | 8 ++++++- .../src/worker/fetch/error.rs | 8 ++++++- crates/radicle/src/storage.rs | 16 +++++++++++-- crates/radicle/src/storage/git.rs | 4 ++-- .../radicle/src/storage/refs/sigrefs/write.rs | 23 +++++++++++++------ .../src/storage/refs/sigrefs/write/error.rs | 8 +++++-- crates/radicle/src/test/storage.rs | 4 ++-- 7 files changed, 54 insertions(+), 17 deletions(-) diff --git a/crates/radicle-fetch/src/policy.rs b/crates/radicle-fetch/src/policy.rs index d7ef0684..a3f9fe77 100644 --- a/crates/radicle-fetch/src/policy.rs +++ b/crates/radicle-fetch/src/policy.rs @@ -129,6 +129,12 @@ pub mod error { Git(#[from] radicle::git::raw::Error), #[error(transparent)] - Refs(#[from] storage::refs::Error), + Refs(Box), + } + + impl From for Policy { + fn from(err: storage::refs::Error) -> Self { + Self::Refs(Box::new(err)) + } } } diff --git a/crates/radicle-protocol/src/worker/fetch/error.rs b/crates/radicle-protocol/src/worker/fetch/error.rs index 20fd310a..71fc0cf9 100644 --- a/crates/radicle-protocol/src/worker/fetch/error.rs +++ b/crates/radicle-protocol/src/worker/fetch/error.rs @@ -18,7 +18,7 @@ pub enum Fetch { #[error(transparent)] Repository(#[from] radicle::storage::RepositoryError), #[error(transparent)] - RefsDb(#[from] radicle::node::refs::Error), + RefsDb(Box), #[error("validation of the storage repository failed: the delegates {delegates:?} failed to validate to meet a threshold of {threshold}")] Validation { threshold: usize, @@ -28,6 +28,12 @@ pub enum Fetch { Cache(#[from] Cache), } +impl From for Fetch { + fn from(err: radicle::node::refs::Error) -> Self { + Self::RefsDb(Box::new(err)) + } +} + #[derive(Debug, Error)] pub enum Cache { #[error(transparent)] diff --git a/crates/radicle/src/storage.rs b/crates/radicle/src/storage.rs index 8c7c50d5..47ddc04e 100644 --- a/crates/radicle/src/storage.rs +++ b/crates/radicle/src/storage.rs @@ -107,7 +107,7 @@ impl SetHead { #[derive(Error, Debug)] pub enum RepositoryError { #[error(transparent)] - Storage(#[from] Error), + Storage(Box), #[error(transparent)] Store(#[from] cob::store::Error), #[error(transparent)] @@ -119,7 +119,7 @@ pub enum RepositoryError { #[error(transparent)] Quorum(#[from] canonical::error::QuorumError), #[error(transparent)] - Refs(#[from] refs::Error), + Refs(Box), #[error("missing canonical reference rule for default branch")] MissingBranchRule, #[error("could not get the default branch rule: {0}")] @@ -130,6 +130,18 @@ pub enum RepositoryError { FindObjects(#[from] canonical::effects::FindObjectsError), } +impl From for RepositoryError { + fn from(err: Error) -> Self { + Self::Storage(Box::new(err)) + } +} + +impl From for RepositoryError { + fn from(err: refs::Error) -> Self { + Self::Refs(Box::new(err)) + } +} + impl RepositoryError { pub fn is_not_found(&self) -> bool { match self { diff --git a/crates/radicle/src/storage/git.rs b/crates/radicle/src/storage/git.rs index 039c6dd1..23a9cf05 100644 --- a/crates/radicle/src/storage/git.rs +++ b/crates/radicle/src/storage/git.rs @@ -202,7 +202,7 @@ impl WriteStorage for Storage { // N.b. we remove the repository if the `local` peer has no // `rad/sigrefs`. There's no risk of them corrupting data. let has_sigrefs = SignedRefsAt::load(self.info.key, &repo) - .map_err(|err| RepositoryError::Storage(Error::Refs(refs::Error::Read(err))))? + .map_err(|err| RepositoryError::from(refs::Error::Read(err)))? .is_some(); if has_sigrefs { repo.clean(&self.info.key) @@ -259,7 +259,7 @@ impl Storage { let (_, head) = repo.head()?; let refs = refs::SignedRefsAt::load(self.info.key, &repo) - .map_err(|err| RepositoryError::Refs(refs::Error::Read(err)))?; + .map_err(|err| RepositoryError::from(refs::Error::Read(err)))?; let synced_at = refs .as_ref() .map(|r| SyncedAt::new(r.at, &repo)) diff --git a/crates/radicle/src/storage/refs/sigrefs/write.rs b/crates/radicle/src/storage/refs/sigrefs/write.rs index 683c25ab..3903b951 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write.rs +++ b/crates/radicle/src/storage/refs/sigrefs/write.rs @@ -135,12 +135,13 @@ where } = self; let reference = SIGREFS_BRANCH.with_namespace(git::fmt::Component::from(&namespace)); - let head = HeadReader::new(&reference, repository, rid, self.signer) - .read() - .map_err(error::Write::Head)?; + let head = HeadReader::new(&reference, repository, rid, self.signer).read(); + let commit_writer = match head { - Some(head) if head.is_unchanged(&refs) => return Ok(Update::unchanged(head.verified)), - Some(head) => CommitWriter::with_parent( + Ok(Some(head)) if head.is_unchanged(&refs) => { + return Ok(Update::unchanged(head.verified)) + } + Ok(Some(head)) => CommitWriter::with_parent( refs, *head.verified.commit().oid(), author, @@ -148,7 +149,12 @@ where repository, signer, ), - None => CommitWriter::root(refs, author, message, repository, signer), + Ok(None) => CommitWriter::root(refs, author, message, repository, signer), + Err(error::Head::Verify { commit, source }) => { + log::warn!("Verification of head of signed references failed: {source}"); + CommitWriter::with_parent(refs, commit, author, message, repository, signer) + } + Err(err) => return Err(error::Write::Head(err)), }; let commit = commit_writer.write().map_err(error::Write::Commit)?; repository @@ -383,7 +389,10 @@ where .read() .map_err(error::Head::Commit)? .verify(self.rid, self.verifier) - .map_err(error::Head::Verify)?; + .map_err(|err| error::Head::Verify { + commit: oid, + 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 46f5fd7a..12ba0f00 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write/error.rs +++ b/crates/radicle/src/storage/refs/sigrefs/write/error.rs @@ -1,3 +1,4 @@ +use radicle_oid::Oid; use thiserror::Error; use crate::storage::refs::sigrefs::git::{object, reference}; @@ -43,6 +44,9 @@ pub enum Head { Reference(reference::error::FindReference), #[error(transparent)] Commit(super::read::error::Commit), - #[error(transparent)] - Verify(super::read::error::Verify), + #[error("failed to verify commit {commit}: {source}")] + Verify { + commit: Oid, + source: super::read::error::Verify, + }, } diff --git a/crates/radicle/src/test/storage.rs b/crates/radicle/src/test/storage.rs index 3364c434..b4892b92 100644 --- a/crates/radicle/src/test/storage.rs +++ b/crates/radicle/src/test/storage.rs @@ -89,7 +89,7 @@ impl ReadStorage for MockStorage { self.repos .get(&rid) .ok_or_else(|| { - RepositoryError::Storage(Error::Io(io::Error::from(io::ErrorKind::NotFound))) + RepositoryError::from(Error::Io(io::Error::from(io::ErrorKind::NotFound))) }) .cloned() } @@ -115,7 +115,7 @@ impl WriteStorage for MockStorage { fn repository_mut(&self, rid: RepoId) -> Result { self.repos .get(&rid) - .ok_or(RepositoryError::Storage(Error::Io(io::Error::from( + .ok_or(RepositoryError::from(Error::Io(io::Error::from( io::ErrorKind::NotFound, )))) .cloned()