mab122
/
radicle-heartwood-lfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

radicle: disallow default branch 

Ensure that the default branch is only ever synthesized from the `Doc`'s
`threshold` and `delegates` value. If the rule is ever written into a `RawDoc`,
then the update of the that `RawDoc` must go through the `update::veryify`
function, which checks that the rule is not present.
This commit is contained in:
Fintan Halpenny 2025-06-19 14:47:52 +02:00 committed by Lorenz Leutgeb
parent a69397386a
commit ff365e2d8b
7 changed files with 191 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/radicle-node/src/worker/fetch/error.rs
View File

@ -71,5 +71,5 @@ pub enum Canonical {
#[error(transparent)]
Identity(#[from] radicle::storage::RepositoryError),
#[error(transparent)]
Payload(#[from] radicle::identity::PayloadError),
CanonicalRefs(#[from] radicle::identity::doc::CanonicalRefsError),
}

6
crates/radicle-remote-helper/src/push.rs
View File

@ -6,7 +6,7 @@ use std::str::FromStr;
use std::{assert_eq, io};
use radicle::identity::crefs::GetCanonicalRefs as _;
use radicle::identity::doc::DefaultBranchRuleError;
use radicle::identity::doc::CanonicalRefsError;
use radicle::node::device::Device;
use thiserror::Error;
@ -118,7 +118,7 @@ pub enum Error {
#[error(transparent)]
Quorum(#[from] radicle::git::canonical::QuorumError),
#[error(transparent)]
DefaultBranchRule(#[from] radicle::identity::doc::DefaultBranchRuleError),
CanonicalRefs(#[from] radicle::identity::doc::CanonicalRefsError),
}
/// Push command.
@ -271,7 +271,7 @@ pub fn run(
let identity = stored.identity()?;
let crefs = identity.canonical_refs_or_default(|| {
let rule = identity.doc().default_branch_rule()?;
Ok::<_, DefaultBranchRuleError>(CanonicalRefs::from_iter([rule]))
Ok::<_, CanonicalRefsError>(CanonicalRefs::from_iter([rule]))
})?;
let rules = crefs.rules();
let me = Did::from(nid);

2
crates/radicle/src/identity/crefs.rs
View File

@ -69,7 +69,7 @@ impl RawCanonicalRefs {
R: Fn() -> Delegates,
{
let rules = Rules::from_raw(self.rules, resolve)?;
Ok(CanonicalRefs { rules })
Ok(CanonicalRefs::new(rules))
}
}

46
crates/radicle/src/identity/doc.rs
View File

@ -241,8 +241,6 @@ impl PayloadId {
#[derive(Debug, Error)]
pub enum PayloadError {
#[error(transparent)]
CanonicalRefs(#[from] rules::ValidationError),
#[error("json: {0}")]
Json(#[from] serde_json::Error),
#[error("payload '{0}' not found in identity document")]
@ -914,14 +912,32 @@ impl Doc {
}
}
#[derive(Debug, Error)]
pub enum CanonicalRefsError {
#[error(transparent)]
Json(#[from] serde_json::Error),
#[error(transparent)]
CanonicalRefs(#[from] rules::ValidationError),
#[error(transparent)]
DefaultBranch(#[from] DefaultBranchRuleError),
}
impl crefs::GetCanonicalRefs for Doc {
type Error = PayloadError;
type Error = CanonicalRefsError;
fn canonical_refs(&self) -> Result<Option<CanonicalRefs>, Self::Error> {
self.raw_canonical_refs().and_then(|raw| {
raw.map(|raw| {
raw.try_into_canonical_refs(&mut || self.delegates.clone())
.map_err(PayloadError::from)
.map_err(CanonicalRefsError::from)
.and_then(|mut crefs| {
self.default_branch_rule()
.map_err(CanonicalRefsError::from)
.map(|rule| {
crefs.extend([rule]);
crefs
})
})
})
.transpose()
})
@ -930,7 +946,27 @@ impl crefs::GetCanonicalRefs for Doc {
fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, Self::Error> {
let value = self.payload.get(&PayloadId::canonical_refs());
let crefs = value
.map(|value| serde_json::from_value((**value).clone()).map_err(PayloadError::from))
.map(|value| {
serde_json::from_value((**value).clone()).map_err(CanonicalRefsError::from)
})
.transpose()?;
Ok(crefs)
}
}
impl crefs::GetCanonicalRefs for RawDoc {
type Error = CanonicalRefsError;
fn canonical_refs(&self) -> Result<Option<CanonicalRefs>, Self::Error> {
Ok(None)
}
fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, Self::Error> {
let value = self.payload.get(&PayloadId::canonical_refs());
let crefs = value
.map(|value| {
serde_json::from_value((**value).clone()).map_err(CanonicalRefsError::from)
})
.transpose()?;
Ok(crefs)
}

147
crates/radicle/src/identity/doc/update.rs
View File

@ -5,12 +5,13 @@ use std::{collections::BTreeSet, str::FromStr};
use serde_json as json;
use crate::{
git,
identity::crefs::GetCanonicalRefs as _,
prelude::Did,
storage::{refs, ReadRepository, RepositoryError},
};
use super::{Doc, PayloadId, RawDoc, Visibility};
use super::{Doc, PayloadError, PayloadId, RawDoc, Visibility};
/// [`EditVisibility`] allows the visibility of a [`RawDoc`] to be edited using
/// the [`visibility`] function.
@ -186,18 +187,44 @@ pub fn payload(
/// [`Project`]: crate::identity::Project
/// [`CanonicalRefs`]: crate::identity::CanonicalRefs
pub fn verify(raw: RawDoc) -> Result<Doc, error::DocVerification> {
let proposal = raw.verified()?;
// Verify that the payloads can still be parsed into the correct types.
if let Err(super::PayloadError::Json(e)) = proposal.project() {
return Err(error::DocVerification::PayloadJson {
id: PayloadId::project(),
err: e,
});
let proposal = raw.clone().verified()?;
// Verify that the project payload is valid
// TODO(finto): perhaps this should be handled by JSON Schemas instead
let project = match proposal.project() {
Ok(project) => Some(project),
Err(PayloadError::NotFound(_)) => None,
Err(PayloadError::Json(e)) => {
return Err(error::DocVerification::PayloadError {
id: PayloadId::project(),
err: e.to_string(),
})
}
};
// Ensure that if we have canonical reference rules and a project, that no
// rule exists for the default branch. This rule must be synthesized when
// constructing the canonical reference rules.
match raw
.raw_canonical_refs()
.map(|rcrefs| rcrefs.and_then(|c| project.map(|p| (c, p))))
{
Ok(Some((crefs, project))) => {
let default = git::Qualified::from(git::lit::refs_heads(project.default_branch()));
let matches = crefs
.raw_rules()
.matches(&default)
.map(|(pattern, _)| pattern.to_string())
.collect::<Vec<_>>();
if !matches.is_empty() {
return Err(error::DocVerification::DisallowDefault { matches, default });
}
}
_ => { /* we validate below */ }
}
if let Err(super::PayloadError::Json(e)) = proposal.canonical_refs() {
return Err(error::DocVerification::PayloadJson {
// Verify that the canonical references payload is valid
if let Err(e) = proposal.canonical_refs() {
return Err(error::DocVerification::PayloadError {
id: PayloadId::canonical_refs(),
err: e,
err: e.to_string(),
});
}
Ok(proposal)
@ -233,3 +260,101 @@ where
Ok((dids.len() - missing.len() < threshold).then_some(missing))
}
#[allow(clippy::unwrap_used)]
#[cfg(test)]
mod test {
use serde_json::json;
use crate::{
git,
identity::{
crefs::GetCanonicalRefs,
doc::{update::error, PayloadId},
},
prelude::RawDoc,
test::arbitrary,
};
#[test]
fn test_can_update_crefs() {
let raw = arbitrary::gen::<RawDoc>(1);
let raw = super::payload(
raw,
vec![(
PayloadId::canonical_refs(),
"rules".to_string(),
json!({
"refs/tags/*": {
"threshold": 1,
"allow": "delegates"
}
}),
)],
)
.unwrap();
let verified = super::verify(raw);
assert!(verified.is_ok(), "Unexpected error {:?}", verified);
}
#[test]
fn test_cannot_include_default_branch_rule() {
let raw = arbitrary::gen::<RawDoc>(1);
let branch = git::Qualified::from(git::lit::refs_heads(
raw.project().unwrap().default_branch(),
));
let raw = super::payload(
raw,
vec![(
PayloadId::canonical_refs(),
"rules".to_string(),
json!({
"refs/tags/*": {
"threshold": 1,
"allow": "delegates"
},
branch.as_str(): {
"threshold": 1,
"allow": "delegates",
}
}),
)],
)
.unwrap();
assert!(
matches!(
super::verify(raw),
Err(error::DocVerification::DisallowDefault { .. })
),
"Verification should be rejected for including default branch rule"
)
}
#[test]
fn test_default_branch_rule_exists_after_verification() {
let raw = arbitrary::gen::<RawDoc>(1);
let branch = git::Qualified::from(git::lit::refs_heads(
raw.project().unwrap().default_branch(),
));
let raw = super::payload(
raw,
vec![(
PayloadId::canonical_refs(),
"rules".to_string(),
json!({
"refs/tags/*": {
"threshold": 1,
"allow": "delegates"
}
}),
)],
)
.unwrap();
let verified = super::verify(raw).unwrap();
let crefs = verified.canonical_refs().unwrap().unwrap();
assert!(
crefs.rules().matches(&branch).next().is_some(),
"Default branch rule is missing!"
);
}
}

9
crates/radicle/src/identity/doc/update/error.rs
View File

@ -1,6 +1,6 @@
use serde_json as json;
use thiserror::Error;
use crate::git;
use crate::git::RefString;
use crate::identity::{doc::PayloadId, Did, DocError};
@ -25,9 +25,14 @@ pub enum PayloadError {
#[derive(Debug, Error)]
pub enum DocVerification {
#[error("failed to verify `{id}`, {err}")]
PayloadJson { id: PayloadId, err: json::Error },
PayloadError { id: PayloadId, err: String },
#[error(transparent)]
Doc(#[from] DocError),
#[error("incompatible payloads: The rule(s) xyz.radicle.crefs.rules.{matches:?} matches the value of xyz.radicle.project.defaultBranch ('{default}'). Possible resolutions: Change the name of the default branch or remove the rule(s).")]
DisallowDefault {
matches: Vec<String>,
default: git::Qualified<'static>,
},
}
#[derive(Clone, Debug)]

2
crates/radicle/src/storage.rs
View File

@ -124,6 +124,8 @@ pub enum RepositoryError {
MissingBranchRule,
#[error("could not get the default branch rule: {0}")]
DefaultBranchRule(#[from] doc::DefaultBranchRuleError),
#[error("failed to get canonical reference rules: {0}")]
CanonicalRefs(#[from] doc::CanonicalRefsError),
}
impl RepositoryError {