diff --git a/knot2/crates/knot-bench/src/fixtures.rs b/knot2/crates/knot-bench/src/fixtures.rs index b8390b42a..40952d9f1 100644 --- a/knot2/crates/knot-bench/src/fixtures.rs +++ b/knot2/crates/knot-bench/src/fixtures.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use knot_cob::{CobError, CobHome, CobId, CobStore}; use knot_cobs::{ CollaboratorsChange, Grant, MembersChange, MembersCob, Registration, RegistryChange, - RepoRegistryCob, add_member, register_repo, + RepoRegistryCob, register_repo, }; use knot_git::{ EntryKind, Identity, Layout, NewCommit, RefUpdate, Repo, StagedAction, StagedChange, @@ -547,7 +547,7 @@ impl BuiltMembersWriter { &self.signer, UnixSeconds::new(GENESIS_SECONDS), |roster| { - Ok(if roster.contains(&grant(0).subject) { + Ok(if roster.standing(&grant(0).subject).is_some() { None } else { Some(MembersChange::Add(grant(0))) @@ -584,15 +584,15 @@ pub fn build_members_checkpointed(members: RosterCount) -> BuiltMembersWriter { .expect("create members") .object; (1..members.get()).for_each(|index| { - add_member( - &store, - &knot_home(), - object, - grant(u64::from(index) + 1), - &signer, - UnixSeconds::new(GENESIS_SECONDS + i64::from(index)), - ) - .expect("add member"); + store + .update_with_checkpointed::( + &knot_home(), + object, + &signer, + UnixSeconds::new(GENESIS_SECONDS + i64::from(index)), + |_| Ok(MembersChange::Add(grant(u64::from(index) + 1))), + ) + .expect("add member"); }); BuiltMembersWriter { diff --git a/knot2/crates/knot-bench/tests/boot.rs b/knot2/crates/knot-bench/tests/boot.rs index e9ffdf153..ba2b92d91 100644 --- a/knot2/crates/knot-bench/tests/boot.rs +++ b/knot2/crates/knot-bench/tests/boot.rs @@ -9,7 +9,7 @@ fn the_registry_fixture_boots_without_folding_collaborators() { registry.dids().iter().for_each(|repo| { assert_eq!( - index.is_collaborator(repo, &knot_types::AccountDid::new("did:plc:nel").unwrap()), + index.effective_collaborator(repo, &knot_types::AccountDid::new("did:plc:nel").unwrap()), Resolved::Warming, "rebuild must not fold any collaborator COB, so every repo reads warming until first access" ); @@ -19,7 +19,7 @@ fn the_registry_fixture_boots_without_folding_collaborators() { index.ensure_collaborators(first).unwrap(); assert!( !index - .is_collaborator(first, &knot_types::AccountDid::new("did:plc:nel").unwrap()) + .effective_collaborator(first, &knot_types::AccountDid::new("did:plc:nel").unwrap()) .is_warming(), "repo folds on first access" ); diff --git a/knot2/crates/knot-git/src/lib.rs b/knot2/crates/knot-git/src/lib.rs index 83fbd753d..c5cf6628c 100644 --- a/knot2/crates/knot-git/src/lib.rs +++ b/knot2/crates/knot-git/src/lib.rs @@ -39,9 +39,8 @@ pub use reads::{ Submodule, TagInfo, }; pub use repo::{ - AdvertScope, HeadRef, Layout, PackHash, PackfileUri, PackfileUrl, RefRecord, RefTxn, RefUpdate, - ReflogUpdate, Repo, is_branch, is_public_ref, is_reserved, knot_shard, repo_shard, - screens_reserved, + AdvertScope, HeadRef, Layout, PackHash, PackfileUri, PackfileUrl, RefClass, RefRecord, RefTxn, + RefUpdate, ReflogUpdate, Repo, is_branch, knot_shard, repo_shard, }; pub use staging::{INCOMING_PREFIX, Staging}; diff --git a/knot2/crates/knot-git/src/repo.rs b/knot2/crates/knot-git/src/repo.rs index 5f2cbf2c0..c06951a8d 100644 --- a/knot2/crates/knot-git/src/repo.rs +++ b/knot2/crates/knot-git/src/repo.rs @@ -13,9 +13,6 @@ use knot_types::{ use crate::error::GitError; use crate::objects::{Haves, PackBudget, Walked, Wants}; -const RESERVED_PREFIX: &str = "refs/cobs/"; -const CHECKPOINT_PREFIX: &str = "refs/cob-checkpoints/"; -const HIDDEN_PREFIX: &str = "refs/hidden/"; const REFLOG_COMMITTER_NAME: &str = "knot"; const REFLOG_COMMITTER_EMAIL: &str = "noreply@knot"; const HEADS_PREFIX: &str = "refs/heads/"; @@ -407,26 +404,61 @@ impl RefUpdate { } } -pub fn is_reserved(name: &RefName) -> bool { - screens_reserved(name.as_str()) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum RefClass { + Public, + Cob, + Checkpoint, + Hidden, + Atproto, } -pub fn screens_reserved(raw: &str) -> bool { - raw.starts_with(RESERVED_PREFIX) || raw.starts_with(CHECKPOINT_PREFIX) -} +impl RefClass { + pub const ALL: [RefClass; 5] = [ + RefClass::Public, + RefClass::Cob, + RefClass::Checkpoint, + RefClass::Hidden, + RefClass::Atproto, + ]; + + pub fn of(name: &RefName) -> RefClass { + RefClass::of_revspec(name.as_str()) + } + + pub fn of_revspec(raw: &str) -> RefClass { + let rest = raw.strip_prefix("refs/").unwrap_or(raw); + match rest.split('/').next().unwrap_or(rest) { + "cobs" => RefClass::Cob, + "cob-checkpoints" => RefClass::Checkpoint, + "hidden" => RefClass::Hidden, + "atproto" => RefClass::Atproto, + _ => RefClass::Public, + } + } -fn is_hidden(name: &RefName) -> bool { - name.as_str().starts_with(HIDDEN_PREFIX) + pub fn namespace(self) -> Option<&'static str> { + match self { + RefClass::Public => None, + RefClass::Cob => Some("refs/cobs"), + RefClass::Checkpoint => Some("refs/cob-checkpoints"), + RefClass::Hidden => Some("refs/hidden"), + RefClass::Atproto => Some("refs/atproto"), + } + } + + pub fn is_public(self) -> bool { + match self { + RefClass::Public => true, + RefClass::Cob | RefClass::Checkpoint | RefClass::Hidden | RefClass::Atproto => false, + } + } } pub fn is_branch(name: &RefName) -> bool { name.as_str().starts_with(HEADS_PREFIX) } -pub fn is_public_ref(name: &RefName) -> bool { - !is_reserved(name) && !is_hidden(name) -} - #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AdvertScope { Upload, @@ -582,6 +614,27 @@ impl Repo { .collect() } + pub fn retained_oids( + &self, + floor: UnixSeconds, + tip_classes: impl Fn(RefClass) -> bool, + ) -> Result, GitError> { + let mut oids: HashSet = self + .references()? + .into_iter() + .filter(|record| tip_classes(RefClass::of(&record.name))) + .map(|record| record.target) + .collect(); + self.reflog_updates_since(floor) + .into_iter() + .for_each(|update| { + oids.insert(update.new); + oids.extend(update.old); + }); + oids.retain(|oid| self.contains(*oid)); + Ok(oids) + } + pub fn find_ref(&self, name: &RefName) -> Result, GitError> { match self .git @@ -601,7 +654,7 @@ impl Repo { false => RefName::new(format!("refs/{spec}")), } .ok() - .filter(is_hidden)?; + .filter(|name| RefClass::of(name) == RefClass::Hidden)?; self.find_ref(&name).ok().flatten() } @@ -781,7 +834,7 @@ impl Repo { self.references().map(|records| { records .into_iter() - .filter(|record| is_public_ref(&record.name)) + .filter(|record| RefClass::of(&record.name).is_public()) .collect() }) } @@ -832,17 +885,17 @@ impl Repo { pub fn head(&self) -> Option { let target = self.git.head_id().ok()?.detach(); - let raw = self.git.head_name().ok()??.as_bstr().to_string(); - let name = RefName::new(raw).ok()?; Some(HeadRef { - name, + name: self.default_branch()?, target: Oid::from(target), }) } pub fn default_branch(&self) -> Option { let raw = self.git.head_name().ok()??.as_bstr().to_string(); - RefName::new(raw).ok() + RefName::new(raw) + .ok() + .filter(|name| RefClass::of(name).is_public()) } pub fn contains(&self, oid: Oid) -> bool { @@ -996,6 +1049,7 @@ impl Repo { } fn update_ref_locked(&self, update: &RefUpdate) -> Result<(), GitError> { + self.reject_df_conflicts(std::slice::from_ref(update))?; let raw = update.name().as_str().to_string(); let via_head = self.updates_head_branch(update); self.git @@ -1022,17 +1076,87 @@ impl Repo { }) } + fn name_taken(&self, raw: &str, creates: &HashSet<&str>) -> Result { + match creates.contains(raw) { + true => Ok(true), + false => match RefName::new(raw.to_string()) { + Ok(name) => self.find_ref(&name).map(|found| found.is_some()), + Err(_) => Ok(false), + }, + } + } + + fn blocking_ancestor( + &self, + name: &str, + creates: &HashSet<&str>, + deletes: &HashSet<&str>, + ) -> Result, GitError> { + name.match_indices('/') + .map(|(at, _)| &name[..at]) + .filter(|ancestor| !deletes.contains(ancestor)) + .map(|ancestor| Ok((ancestor, self.name_taken(ancestor, creates)?))) + .collect::, GitError>>() + .map(|probed| { + probed + .into_iter() + .find_map(|(ancestor, taken)| taken.then(|| ancestor.to_string())) + }) + } + + fn leaf_under( + &self, + directory: &str, + deletes: &HashSet<&str>, + ) -> Result, GitError> { + let backend = |error: String| GitError::Backend(error); + match self + .git + .references() + .map_err(|error| backend(error.to_string()))? + .prefixed(directory) + { + Ok(walk) => Ok(walk + .filter_map(Result::ok) + .map(|reference| reference.name().as_bstr().to_string()) + .find(|leaf| !deletes.contains(leaf.as_str()))), + Err(gix::reference::iter::init::Error::RelativePath(_)) => Ok(self + .references()? + .into_iter() + .map(|record| record.name) + .find(|name| { + name.as_str().starts_with(directory) && !deletes.contains(name.as_str()) + }) + .map(|name| name.as_str().to_string())), + Err(error) => Err(backend(error.to_string())), + } + } + + fn blocked_leaf( + &self, + name: &str, + creates: &HashSet<&str>, + deletes: &HashSet<&str>, + ) -> Result, GitError> { + let directory = format!("{name}/"); + match creates + .iter() + .find(|other| other.starts_with(&directory)) + .copied() + { + Some(leaf) => Ok(Some(leaf.to_string())), + None => self.leaf_under(&directory, deletes), + } + } + fn reject_df_conflicts(&self, updates: &[RefUpdate]) -> Result<(), GitError> { - let creates: Vec<&str> = updates + let creates: HashSet<&str> = updates .iter() .filter_map(|update| match update { RefUpdate::Create { name, .. } => Some(name.as_str()), _ => None, }) .collect(); - if creates.is_empty() { - return Ok(()); - } let deletes: HashSet<&str> = updates .iter() .filter_map(|update| match update { @@ -1040,27 +1164,19 @@ impl Repo { _ => None, }) .collect(); - let names: Vec = self - .references()? - .iter() - .map(|record| record.name.as_str().to_string()) - .filter(|name| !deletes.contains(name.as_str())) - .chain(creates.iter().copied().map(str::to_string)) - .collect(); - let name_set: HashSet<&str> = names.iter().map(String::as_str).collect(); - names - .iter() - .find_map(|name| { - name.match_indices('/') - .map(|(at, _)| &name[..at]) - .find(|ancestor| name_set.contains(ancestor)) - .map(|ancestor| (ancestor.to_string(), name.clone())) - }) - .map_or(Ok(()), |(directory, leaf)| { - Err(GitError::AtomicRefs(format!( - "d/f conflict: {directory} blocks {leaf}" - ))) - }) + creates.iter().try_for_each(|name| { + match self.blocking_ancestor(name, &creates, &deletes)? { + Some(directory) => Err(GitError::AtomicRefs(format!( + "d/f conflict: {directory} blocks {name}" + ))), + None => match self.blocked_leaf(name, &creates, &deletes)? { + Some(leaf) => Err(GitError::AtomicRefs(format!( + "d/f conflict: {name} blocks {leaf}" + ))), + None => Ok(()), + }, + } + }) } fn update_refs_locked(&self, updates: &[RefUpdate]) -> Result<(), GitError> { @@ -1584,7 +1700,10 @@ mod tests { assert_eq!(advertised.len(), 1, "cob ref is hidden from advertisement"); assert_eq!(advertised[0].name.as_str(), "refs/heads/main"); assert_eq!(repo.references().unwrap().len(), 2); - assert!(is_reserved(&RefName::new("refs/cobs/x/y").unwrap())); + assert_eq!( + RefClass::of(&RefName::new("refs/cobs/x/y").unwrap()), + RefClass::Cob + ); repo.update_ref(&RefUpdate::Create { name: feature.clone(), @@ -1628,6 +1747,171 @@ mod tests { ); } + #[test] + fn each_reserved_namespace_covers_its_children_however_the_ref_is_spelled() { + let cases = [ + ("refs/heads/main", RefClass::Public), + ("refs/atproto", RefClass::Atproto), + ("refs/atproto/commit", RefClass::Atproto), + ("refs/atproto-mirror/commit", RefClass::Public), + ("refs/cobs", RefClass::Cob), + ("refs/cobs/sh.tangled.knot.member/beef", RefClass::Cob), + ("refs/cob-checkpoints", RefClass::Checkpoint), + ("refs/cob-checkpoints/main", RefClass::Checkpoint), + ("refs/hidden", RefClass::Hidden), + ("refs/hidden/feature/main", RefClass::Hidden), + ]; + cases.iter().for_each(|(raw, class)| { + assert_eq!( + RefClass::of(&RefName::new(*raw).unwrap()), + *class, + "{raw}: a namespace must cover its own name and its children" + ); + assert_eq!( + RefClass::of_revspec(raw), + *class, + "{raw}: an unparsed revspec must classify like a parsed RefName" + ); + assert_eq!( + RefClass::of_revspec(raw.strip_prefix("refs/").unwrap()), + *class, + "{raw}: the refs/ prefix is optional in a revspec" + ); + }); + } + + #[test] + fn every_ref_class_owns_one_namespace_and_the_classifier_maps_it_back() { + let namespaces: Vec<&str> = RefClass::ALL + .iter() + .filter_map(|class| class.namespace()) + .collect(); + assert_eq!( + namespaces.len(), + RefClass::ALL.len() - 1, + "Public is the only class without a namespace" + ); + assert_eq!( + namespaces.iter().collect::>().len(), + namespaces.len(), + "two classes on one namespace would route one class's refs through the other's rule" + ); + RefClass::ALL + .iter() + .filter_map(|class| class.namespace().map(|namespace| (class, namespace))) + .for_each(|(class, namespace)| { + assert_eq!( + RefClass::of_revspec(namespace), + *class, + "{namespace}: a class the classifier doesn't know falls through to public" + ); + assert_eq!( + RefClass::of_revspec(&format!("{namespace}/child")), + *class, + "{namespace}: a namespace must own its children" + ); + assert_eq!( + RefClass::of_revspec(&format!("{namespace}-beside/child")), + RefClass::Public, + "{namespace}: a prefix match isn't a match, so this name stays public" + ); + }); + } + + #[test] + fn knot_writes_its_atproto_chain_and_advertisement_stops_at_main() { + let (_dir, layout, did) = repo(); + let repo = layout.create(&did).unwrap(); + let chain = RefName::new("refs/atproto/commit").unwrap(); + repo.update_ref(&RefUpdate::Create { + name: head_ref(), + new: oid(A), + }) + .unwrap(); + repo.update_ref(&RefUpdate::Create { + name: chain.clone(), + new: oid(B), + }) + .unwrap(); + + assert_eq!( + repo.find_ref(&chain).unwrap(), + Some(oid(B)), + "the reservation is a wire rule, so the knot's own write still reaches the ref" + ); + assert_eq!( + repo.advertised_refs() + .unwrap() + .iter() + .map(|record| record.name.as_str().to_string()) + .collect::>(), + vec!["refs/heads/main".to_string()], + "a client can't push to the chain, so the advertisement mustn't offer it" + ); + repo.set_head(&chain).unwrap(); + assert_eq!( + repo.default_branch(), + None, + "a clone would follow HEAD to the chain, so default_branch must reject it" + ); + assert!(repo.head().is_none()); + } + + #[test] + fn a_ref_at_the_bare_namespace_name_blocks_its_children_on_both_write_paths() { + let (_dir, layout, did) = repo(); + let repo = layout.create(&did).unwrap(); + repo.update_ref(&RefUpdate::Create { + name: RefName::new("refs/atproto").unwrap(), + new: oid(A), + }) + .unwrap(); + + let chain = RefUpdate::Create { + name: RefName::new("refs/atproto/commit").unwrap(), + new: oid(B), + }; + [ + repo.update_ref(&chain), + repo.update_refs(std::slice::from_ref(&chain)), + ] + .iter() + .for_each(|outcome| { + assert!( + matches!(outcome, Err(GitError::AtomicRefs(message)) if message.contains("d/f conflict")), + "gix must refuse the chain write as a d/f conflict, on whichever path the writer \ + took: {outcome:?}" + ); + }); + } + + #[test] + fn the_df_screen_stores_every_name_git_allows_and_refuses_a_relative_path() { + let (_dir, layout, did) = repo(); + let repo = layout.create(&did).unwrap(); + let awkward = ["refs/heads/a UnixSeconds { UnixSeconds::new(secs) } -fn reachable_roots(repo: &Repo, floor: UnixSeconds) -> Result, GitError> { - let mut roots: HashSet = repo - .references()? +fn reachable_pointers(repo: &Repo, floor: UnixSeconds) -> Result, GitError> { + let roots: Vec = repo + .retained_oids(floor, |class| match class { + RefClass::Public | RefClass::Hidden => true, + RefClass::Cob | RefClass::Checkpoint | RefClass::Atproto => false, + })? .into_iter() - .filter(|record| !knot_git::is_reserved(&record.name)) - .map(|record| record.target) .collect(); - repo.reflog_updates_since(floor) - .into_iter() - .for_each(|update| { - roots.insert(update.new); - if let Some(old) = update.old { - roots.insert(old); - } - }); - Ok(roots - .into_iter() - .filter(|oid| repo.contains(*oid)) - .collect()) -} - -fn reachable_pointers(repo: &Repo, floor: UnixSeconds) -> Result, GitError> { - let roots = reachable_roots(repo, floor)?; if roots.is_empty() { return Ok(HashSet::new()); } @@ -281,6 +266,49 @@ mod tests { assert_eq!(f.store.probe(&f.did, &dropped).unwrap(), None); } + #[test] + fn a_knot_written_pointer_is_swept_and_a_hidden_one_keeps_its_bytes() { + let f = fixture(); + + let (cob, cob_size) = put_media(&f, b"media under a collaborative object"); + commit_pointer(&f, "refs/cobs/sh.tangled.knot.member/beef", &cob, cob_size); + age(&f, &cob, MONTHS); + + let (chain, chain_size) = put_media(&f, b"media under the atproto commit chain"); + commit_pointer(&f, "refs/atproto/commit", &chain, chain_size); + age(&f, &chain, MONTHS); + + let (snapshot, snapshot_size) = put_media(&f, b"media under a cob checkpoint"); + commit_pointer( + &f, + "refs/cob-checkpoints/sh.tangled.knot.member/beef", + &snapshot, + snapshot_size, + ); + age(&f, &snapshot, MONTHS); + + let (staged, staged_size) = put_media(&f, b"media under a fork staging ref"); + commit_pointer(&f, "refs/hidden/feature/main", &staged, staged_size); + age(&f, &staged, MONTHS); + + let report = collect(&f, DAY); + assert_eq!(report.swept, 3); + assert_eq!( + report.bytes, + cob_size + .saturating_add(chain_size) + .saturating_add(snapshot_size) + ); + assert_eq!(f.store.probe(&f.did, &cob).unwrap(), None); + assert_eq!(f.store.probe(&f.did, &chain).unwrap(), None); + assert_eq!(f.store.probe(&f.did, &snapshot).unwrap(), None); + assert!( + f.store.probe(&f.did, &staged).unwrap().is_some(), + "fork staging roots its bytes, so the three sweeps above are the class filter \ + at work rather than every reserved name losing its objects" + ); + } + #[test] fn a_repo_with_no_stored_objects_never_walks_git() { let f = fixture(); diff --git a/knot2/crates/knot-maintenance/src/lib.rs b/knot2/crates/knot-maintenance/src/lib.rs index f6b36e5d1..9e4f23d61 100644 --- a/knot2/crates/knot-maintenance/src/lib.rs +++ b/knot2/crates/knot-maintenance/src/lib.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use std::path::PathBuf; use std::time::Duration; -use knot_git::{PackRefsReport, ReflogReport, Repo}; +use knot_git::{PackRefsReport, RefClass, ReflogReport, Repo}; use knot_types::{Oid, UnixSeconds}; mod bitmap; @@ -337,23 +337,14 @@ pub fn run_repo( } fn collect_roots(repo: &Repo, retention_floor: UnixSeconds) -> Result, MaintError> { - let mut roots: HashSet = repo - .references()? - .into_iter() - .map(|record| record.target) - .collect(); - repo.reflog_updates_since(retention_floor) - .into_iter() - .for_each(|update| { - roots.insert(update.new); - if let Some(old) = update.old { - roots.insert(old); - } - }); - Ok(roots - .into_iter() - .filter(|oid| repo.contains(*oid)) - .collect()) + repo.retained_oids(retention_floor, |class| match class { + RefClass::Public + | RefClass::Cob + | RefClass::Checkpoint + | RefClass::Hidden + | RefClass::Atproto => true, + }) + .map_err(MaintError::from) } #[cfg(test)] diff --git a/knot2/crates/knot-messages/src/lib.rs b/knot2/crates/knot-messages/src/lib.rs index 73ebfde4c..f87c32d00 100644 --- a/knot2/crates/knot-messages/src/lib.rs +++ b/knot2/crates/knot-messages/src/lib.rs @@ -174,10 +174,12 @@ message_group! { message_group! { RejectConfig => RejectMessages @ "messages.reject" { - reserved_refs: Line @ "KNOT_MESSAGES_REJECT_RESERVED_REFS" = "refs/cobs/* and refs/hidden/* are reserved and cannot be pushed", + reserved_refs: Line @ "KNOT_MESSAGES_REJECT_RESERVED_REFS" = "refs/cobs/*, refs/cob-checkpoints/*, refs/hidden/* and refs/atproto/* are reserved and can't be pushed", cob_create_only: Line @ "KNOT_MESSAGES_REJECT_COB_CREATE_ONLY" = "existing refs/cobs/* object cannot be modified or deleted over the wire", cob_delete: Line @ "KNOT_MESSAGES_REJECT_COB_DELETE" = "refs/cobs/* stores append-only collaborative objects and cannot be deleted", hidden_reserved: Line @ "KNOT_MESSAGES_REJECT_HIDDEN_RESERVED" = "refs/hidden/* is reserved for server-side fork staging and cannot be pushed", + atproto_reserved: Line @ "KNOT_MESSAGES_REJECT_ATPROTO_RESERVED" = "refs/atproto/* is reserved for the knot's atproto commit chain and can't be pushed", + checkpoint_reserved: Line @ "KNOT_MESSAGES_REJECT_CHECKPOINT_RESERVED" = "refs/cob-checkpoints/* is reserved for knot-written collaborative-object snapshots and can't be pushed", cob_verification: Line @ "KNOT_MESSAGES_REJECT_COB_VERIFICATION" = "collaborative-object verification failed: {error}", ref_exists: Line @ "KNOT_MESSAGES_REJECT_REF_EXISTS" = "reference already exists", stale_old_value: Line @ "KNOT_MESSAGES_REJECT_STALE_OLD_VALUE" = "stale info: old value doesn't match", diff --git a/knot2/crates/knot-migrate/src/emit.rs b/knot2/crates/knot-migrate/src/emit.rs index 6cb3948df..9efbe55d7 100644 --- a/knot2/crates/knot-migrate/src/emit.rs +++ b/knot2/crates/knot-migrate/src/emit.rs @@ -160,7 +160,7 @@ where signer, |grant| make(to_grant(grant)), |grant| grant.created_at, - |roster, grant| roster.contains(&grant.subject), + |roster, grant| roster.standing(&grant.subject).is_some(), |_| Ok(()), ) } diff --git a/knot2/crates/knot-migrate/tests/migrate.rs b/knot2/crates/knot-migrate/tests/migrate.rs index 110cdf9af..36d9a143e 100644 --- a/knot2/crates/knot-migrate/tests/migrate.rs +++ b/knot2/crates/knot-migrate/tests/migrate.rs @@ -507,19 +507,19 @@ fn adoption_and_cobs_boot_a_working_index() { let squid = RepoDid::new("did:plc:squid").unwrap(); index.ensure_collaborators(&squid).unwrap(); assert_eq!( - index.is_collaborator(&squid, &acc("isabel")), + index.effective_collaborator(&squid, &acc("isabel")), Resolved::Ready(true) ); assert_eq!( - index.is_collaborator(&squid, &acc("olaren")), + index.effective_collaborator(&squid, &acc("olaren")), Resolved::Ready(true) ); assert_eq!( - index.is_collaborator(&squid, &acc("teq")), + index.effective_collaborator(&squid, &acc("teq")), Resolved::Ready(true) ); assert_eq!( - index.is_collaborator(&squid, &acc("periwinkle")), + index.effective_collaborator(&squid, &acc("periwinkle")), Resolved::Ready(false) ); assert_eq!( diff --git a/knot2/crates/knot-pack/src/guard.rs b/knot2/crates/knot-pack/src/guard.rs index 9bcf3e680..03edd7a0a 100644 --- a/knot2/crates/knot-pack/src/guard.rs +++ b/knot2/crates/knot-pack/src/guard.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use knot_cob::{CobHome, CobStore}; -use knot_git::Repo; +use knot_git::{RefClass, Repo}; use knot_messages::{Catalog, ErrorKey}; use knot_types::{ActorId, RefName}; @@ -24,22 +24,25 @@ impl ReceiveGuard for PushGuard { impl PushGuard { fn decide(&self, staged: &Repo, command: &ReceiveCommand) -> RefDecision { - match command.name() { - None => RefDecision::Reject( + let Some(name) = command.name() else { + return RefDecision::Reject( self.messages .reject .cob_verification .line(|ErrorKey::Error| crate::receive::invalid_refname(command.refname())), - ), - Some(name) if knot_git::is_public_ref(name) => RefDecision::Allow, - Some(name) if knot_git::is_reserved(name) => { - if command.is_delete() { - RefDecision::Reject(self.messages.reject.cob_delete.text()) - } else { - self.verify_cob(staged, name) - } + ); + }; + match RefClass::of(name) { + RefClass::Public => RefDecision::Allow, + RefClass::Cob if command.is_delete() => { + RefDecision::Reject(self.messages.reject.cob_delete.text()) + } + RefClass::Cob => self.verify_cob(staged, name), + RefClass::Checkpoint => { + RefDecision::Reject(self.messages.reject.checkpoint_reserved.text()) } - Some(_) => RefDecision::Reject(self.messages.reject.hidden_reserved.text()), + RefClass::Hidden => RefDecision::Reject(self.messages.reject.hidden_reserved.text()), + RefClass::Atproto => RefDecision::Reject(self.messages.reject.atproto_reserved.text()), } } diff --git a/knot2/crates/knot-pack/src/receive.rs b/knot2/crates/knot-pack/src/receive.rs index 403e7f8f8..cf7e922a8 100644 --- a/knot2/crates/knot-pack/src/receive.rs +++ b/knot2/crates/knot-pack/src/receive.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::io::Write; use std::path::Path; -use knot_git::{Filter, RefTxn, RefUpdate, Repo}; +use knot_git::{Filter, RefClass, RefTxn, RefUpdate, Repo}; use knot_messages::{RefKey, RejectMessages}; use knot_types::{ObjectFormat, Oid, PushOption, PushOptions, RefName}; @@ -252,14 +252,17 @@ pub(crate) fn invalid_refname(raw: &str) -> String { fn forbidden_ref(command: &ReceiveCommand, messages: &RejectMessages) -> Option { match command.name() { - Some(name) => (!knot_git::is_public_ref(name)).then(|| messages.reserved_refs.text()), + Some(name) => (!RefClass::of(name).is_public()).then(|| messages.reserved_refs.text()), None => Some(invalid_refname(command.refname())), } } fn reserved_create_only(command: &ReceiveCommand, messages: &RejectMessages) -> Option { - (command.name().is_some_and(knot_git::is_reserved) && !command.is_create()) - .then(|| messages.cob_create_only.text()) + (command + .name() + .is_some_and(|name| RefClass::of(name) == RefClass::Cob) + && !command.is_create()) + .then(|| messages.cob_create_only.text()) } struct RefSnapshot { diff --git a/knot2/crates/knot-pack/tests/hardening.rs b/knot2/crates/knot-pack/tests/hardening.rs index d8cfd7f8b..24d6b64d0 100644 --- a/knot2/crates/knot-pack/tests/hardening.rs +++ b/knot2/crates/knot-pack/tests/hardening.rs @@ -196,7 +196,7 @@ fn push_namespace_gating_accepts_only_unreserved_refs() { let did = RepoDid::new("did:plc:squid").unwrap(); let (bare, _work, c1, pack) = seeded(&layout, &did); - let cases: [(&str, &str, bool); 3] = [ + let cases: [(&str, &str, bool); 7] = [ ( "refs/cobs/sh.tangled.repo.collaborator/evil", "ng refs/cobs/sh.tangled.repo.collaborator/evil", @@ -207,6 +207,14 @@ fn push_namespace_gating_accepts_only_unreserved_refs() { "ng refs/hidden/feature/main", false, ), + ("refs/atproto/commit", "ng refs/atproto/commit", false), + ("refs/atproto", "ng refs/atproto", false), + ( + "refs/cob-checkpoints/sh.tangled.knot.member/beef", + "ng refs/cob-checkpoints/sh.tangled.knot.member/beef", + false, + ), + ("refs/cob-checkpoints", "ng refs/cob-checkpoints", false), ("refs/notes/commits", "ok refs/notes/commits", true), ]; cases.iter().for_each(|(refname, verdict, lands)| { diff --git a/knot2/crates/knot-pack/tests/quarantine.rs b/knot2/crates/knot-pack/tests/quarantine.rs index f220be0fa..f03bee81e 100644 --- a/knot2/crates/knot-pack/tests/quarantine.rs +++ b/knot2/crates/knot-pack/tests/quarantine.rs @@ -1,11 +1,13 @@ use std::path::Path; +use std::sync::Arc; use std::time::Instant; use knot_cob::{CobHome, CobStore}; use knot_cobs::{Grant, MembersChange}; use knot_git::{Layout, Repo}; +use knot_messages::Catalog; use knot_pack::{ - MaxWireBytes, PackLimits, PackReceiver, ReceiveCommand, ReceiveFramer, ReceiveGuard, + MaxWireBytes, PackLimits, PackReceiver, PushGuard, ReceiveCommand, ReceiveFramer, ReceiveGuard, RefDecision, receive_pack_guarded, receive_request_complete, sweep_incoming, }; use knot_runtime::{K256Signer, SeededEntropy, Signer}; @@ -80,7 +82,10 @@ impl ReceiveGuard for AllowPublic { commands .iter() .map(|command| { - if command.name().is_some_and(knot_git::is_public_ref) { + if command + .name() + .is_some_and(|name| knot_git::RefClass::of(name).is_public()) + { RefDecision::Allow } else { RefDecision::Reject("not public ref".to_string()) @@ -356,6 +361,65 @@ fn a_reserved_ref_update_is_refused_even_when_the_guard_allows_it() { ); } +#[test] +fn the_push_guard_allows_a_branch_and_names_the_namespace_it_refuses() { + let scan = tempfile::tempdir().unwrap(); + let layout = Layout::new(scan.path()); + let did = RepoDid::new("did:plc:squid").unwrap(); + let (bare, c1, pack) = seeded_pack(&layout, &did); + let signer = K256Signer::generate(&SeededEntropy::new(11)); + let guard = PushGuard { + cob_authority: ActorId::from_secp256k1(signer.public_key().as_bytes()), + home: CobHome::from(&did), + messages: Arc::new(Catalog::defaults()), + }; + let push = |refname: &str| { + report_text( + &receive_pack_guarded( + &bare, + &receive_request(refname, &Oid::null().to_hex(), &c1, &pack), + &limits(), + &guard, + &|_| {}, + &knot_pack::default_catalog().reject, + ) + .unwrap() + .report, + ) + }; + + [ + ("refs/atproto/commit", "atproto commit chain"), + ( + "refs/cob-checkpoints/sh.tangled.knot.member/beef", + "knot-written collaborative-object snapshots", + ), + ("refs/hidden/feature/main", "server-side fork staging"), + ] + .iter() + .for_each(|(refname, names_itself)| { + let refused = push(refname); + assert!( + refused.contains(&format!("ng {refname}")), + "the same push is fine on a branch, so {refname} must be refused: {refused}" + ); + assert!( + refused.contains(names_itself), + "the refusal must name the namespace that stopped this push, {refname}: {refused}" + ); + assert!( + bare.references().unwrap().is_empty(), + "a refused push to {refname} must leave the ref store empty" + ); + }); + + let branch = push("refs/heads/topic"); + assert!( + branch.contains("ok refs/heads/topic"), + "the same guard still accepts an ordinary branch: {branch}" + ); +} + fn pseudo_random(seed: u64, len: usize) -> Vec { let mut state = seed.wrapping_mul(0x9E37_79B9_7F4A_7C15).wrapping_add(1); (0..len)