From 95d9da7a9f13d496ab1fc3b972c01a56b2c881f6 Mon Sep 17 00:00:00 2001 From: hmirin Date: Wed, 14 Oct 2026 14:30:43 +0900 Subject: [PATCH] Support tags and remote ref deletion Manifests can now record tags and ref deletions, the remote helper advertises both, and existing refs are validated before a new state is published. --- docs/design.md | 12 +- docs/spec.md | 17 +++ src/bin/git-remote-e2ee.rs | 13 +- src/git.rs | 176 +++++++++++++++++++++++--- src/repository.rs | 188 ++++++++++++++++++++-------- tests/carrier_git.rs | 98 +++++++++++++++ tests/end_to_end.rs | 105 +++++++++++++++- tests/git_compatibility.rs | 249 ++++++++++++++++++++++++++++++++++++- tests/read_only_helper.rs | 17 +++ 9 files changed, 781 insertions(+), 94 deletions(-) diff --git a/docs/design.md b/docs/design.md index b9001b8..fb99c46 100644 --- a/docs/design.md +++ b/docs/design.md @@ -123,10 +123,14 @@ administrative CAS is never automatically rebased. oldest-first. - After import, every advertised ref must resolve to a complete local Git object graph before pins or remote-tracking refs move. -- Client-side `git merge-base --is-ancestor` enforces fast-forward updates. -- Explicit force pushes append a normal encrypted delta. -- Push destinations are limited to `refs/heads/*`; tags and deletion fail - before publication. +- The helper advertises `refs/heads/*` and `refs/tags/*`; every other ref + namespace is rejected before publication. +- Branch updates require fast-forward ancestry unless force is explicit. Tags + may be created freely, but an existing tag is moved only by a forced push. + Lightweight and annotated tag objects are included in the encrypted pack. +- Branch and tag deletion publish a signed generation with the complete ref set + minus the deleted ref and no pack delta. `refs/heads/main` cannot be deleted + while it is advertised as the remote `HEAD`. A fresh clone traverses the manifest/key chain to genesis. A returning client traverses to its pinned manifest and imports only unseen pack IDs. The client diff --git a/docs/spec.md b/docs/spec.md index f3861cd..db5a962 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -369,6 +369,23 @@ Rotation is one administrative transition that adds the replacement device and revokes the old record. Other users keep their device private keys. No pack is rewritten. +### 8.5 Inner Git refs + +The remote helper advertises refs in `refs/heads/*` and `refs/tags/*`. All other +namespaces MUST be rejected before publication. Branch refs MUST resolve to +commits and retain the fast-forward check unless the writer explicitly forces +the update. Lightweight and annotated tags MAY point to any Git object; their +complete object graphs, including annotated tag objects, MUST be included in +the encrypted pack and verified by receiving clients. An existing tag MUST +remain unchanged unless the update is forced. + +Deleting an existing branch or tag publishes a signed writer manifest with the +complete ref map minus that ref. A deletion introduces no pack and does not +increase the cumulative pack count. The helper advertises `refs/heads/main` as +the remote `HEAD` when present and MUST refuse to delete it while it is +advertised as the default branch. Returning clones can remove stale local +tracking refs with Git's `--prune` and `--prune-tags` fetch options. + ## 9. Delta traversal and Git connectivity A fresh clone traverses manifest and key links from `HEAD` to genesis, diff --git a/src/bin/git-remote-e2ee.rs b/src/bin/git-remote-e2ee.rs index c62abe8..b2e8c8d 100644 --- a/src/bin/git-remote-e2ee.rs +++ b/src/bin/git-remote-e2ee.rs @@ -55,6 +55,7 @@ fn run_protocol(repository: EncryptedRepository, remote_name: Str writeln!(output, "push")?; writeln!(output, "option")?; writeln!(output, "refspec refs/heads/*:refs/remotes/{remote_name}/*")?; + writeln!(output, "refspec refs/tags/*:refs/tags/*")?; writeln!(output)?; output.flush()?; } @@ -144,11 +145,13 @@ fn process_push_batch( let (source, destination) = spec .split_once(':') .context("push refspec must contain ':'")?; - if source.is_empty() { - writeln!(output, "error {destination} ref deletion is not supported")?; - continue; - } - let result = if dry_run { + let result = if source.is_empty() && dry_run { + repository + .validate_delete_ref_for_remote(local_repository, remote_name, destination) + .map(|_| String::new()) + } else if source.is_empty() { + repository.delete_ref_for_remote(local_repository, remote_name, destination) + } else if dry_run { repository .validate_push_update_for_remote( local_repository, diff --git a/src/git.rs b/src/git.rs index 3046ab9..80932f8 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::fs::File; use std::io::{Read, Write}; use std::path::Path; @@ -22,6 +22,41 @@ pub fn resolve_ref(repo: &Path, reference: &str) -> Result { git_text(repo, &["rev-parse", "--verify", reference]) } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum InnerRefKind { + Branch, + Tag, +} + +pub fn validate_inner_ref(repo: &Path, reference: &str) -> Result { + let kind = if reference.starts_with("refs/heads/") { + InnerRefKind::Branch + } else if reference.starts_with("refs/tags/") { + InnerRefKind::Tag + } else { + bail!( + "unsupported ref namespace for {reference}; only refs/heads/* and refs/tags/* are supported" + ) + }; + let output = Command::new("git") + .arg("-C") + .arg(repo) + .args(["check-ref-format", reference]) + .output()?; + if !output.status.success() { + bail!("invalid Git ref name {reference}") + } + Ok(kind) +} + +pub fn ensure_branch_target(repo: &Path, reference: &str, object: &str) -> Result<()> { + let target = resolve_tag_chain(repo, reference, object, "pushed")?; + if target.kind != "commit" { + bail!("branch ref {reference} must resolve to a commit") + } + Ok(()) +} + pub fn is_ancestor(repo: &Path, ancestor: &str, descendant: &str) -> Result { let status = Command::new("git") .arg("-C") @@ -211,15 +246,18 @@ pub fn ensure_refs_connected_since( if refs.is_empty() { return Ok(()); } + let mut commit_roots = Vec::new(); + let mut verified_commit_roots = Vec::new(); for (reference, object) in refs { - let expression = format!("{object}^{{commit}}"); - let output = Command::new("git") - .arg("-C") - .arg(repo) - .args(["cat-file", "-e", &expression]) - .output()?; - if !output.status.success() { - bail!("fetched ref {reference} does not resolve to a complete commit") + let ref_kind = validate_inner_ref(repo, reference)?; + let target = resolve_tag_chain(repo, reference, object, "fetched")?; + match target.kind.as_str() { + "commit" => commit_roots.push(target.oid), + "tree" if ref_kind == InnerRefKind::Tag => { + ensure_tree_connected(repo, reference, &target.oid)? + } + "blob" if ref_kind == InnerRefKind::Tag => {} + _ => bail!("fetched ref {reference} does not resolve to a supported Git object"), } } @@ -229,28 +267,32 @@ pub fn ensure_refs_connected_since( // reachable beyond that frontier. Still require the frontier tips to be // present locally so corruption or an unexpected local prune fails closed. for (reference, object) in verified_refs { - let expression = format!("{object}^{{commit}}"); - let output = Command::new("git") - .arg("-C") - .arg(repo) - .args(["cat-file", "-e", &expression]) - .output()?; - if !output.status.success() { - bail!("verified ref {reference} is missing locally") + let ref_kind = validate_inner_ref(repo, reference)?; + let target = resolve_tag_chain(repo, reference, object, "verified")?; + match target.kind.as_str() { + "commit" => verified_commit_roots.push(target.oid), + "tree" if ref_kind == InnerRefKind::Tag => { + ensure_tree_connected(repo, reference, &target.oid)? + } + "blob" if ref_kind == InnerRefKind::Tag => {} + _ => bail!("verified ref {reference} does not resolve to a supported Git object"), } } + if commit_roots.is_empty() { + return Ok(()); + } let mut command = Command::new("git"); command .arg("-C") .arg(repo) .args(["rev-list", "--objects", "--missing=print"]); - for object in refs.values() { + for object in &commit_roots { command.arg(object); } - if !verified_refs.is_empty() { + if !verified_commit_roots.is_empty() { command.arg("--not"); - for object in verified_refs.values() { + for object in &verified_commit_roots { command.arg(object); } } @@ -270,6 +312,100 @@ pub fn ensure_refs_connected_since( Ok(()) } +struct RefTarget { + oid: String, + kind: String, +} + +fn resolve_tag_chain(repo: &Path, reference: &str, object: &str, label: &str) -> Result { + let mut current = object.to_owned(); + let mut seen = HashSet::new(); + loop { + if !seen.insert(current.clone()) { + bail!("{label} ref {reference} contains a cyclic tag chain") + } + let kind = match cat_file_type(repo, ¤t) { + Ok(kind) => kind, + Err(_) if label == "verified" => { + bail!("verified ref {reference} is missing locally") + } + Err(_) => { + bail!("fetched ref {reference} does not resolve to a complete Git object graph") + } + }; + if kind != "tag" { + return Ok(RefTarget { oid: current, kind }); + } + let body = git_text(repo, &["cat-file", "-p", ¤t])?; + let mut target = None; + let mut target_kind = None; + for line in body.lines().take_while(|line| !line.is_empty()) { + if let Some(value) = line.strip_prefix("object ") { + if target.is_some() { + bail!("annotated tag {reference} has duplicate target objects") + } + target = Some(value); + } else if let Some(value) = line.strip_prefix("type ") { + if target_kind.is_some() { + bail!("annotated tag {reference} has duplicate target types") + } + target_kind = Some(value); + } + } + let target = target.context("annotated tag is missing its target object")?; + let expected_kind = target_kind.context("annotated tag is missing its target type")?; + if cat_file_type(repo, target)? != expected_kind { + bail!("annotated tag {reference} has a mismatched target type") + } + current = target.to_owned(); + } +} + +fn cat_file_type(repo: &Path, object: &str) -> Result { + git_text(repo, &["cat-file", "-t", object]) +} + +fn ensure_tree_connected(repo: &Path, reference: &str, tree: &str) -> Result<()> { + let output = Command::new("git") + .arg("-C") + .arg(repo) + .args(["ls-tree", "-r", "-t", "-z", "--full-tree", tree]) + .output()?; + if !output.status.success() { + bail!("fetched tag {reference} contains an incomplete tree") + } + for entry in output + .stdout + .split(|byte| *byte == 0) + .filter(|entry| !entry.is_empty()) + { + let header = entry + .split(|byte| *byte == b'\t') + .next() + .context("Git ls-tree returned a malformed entry")?; + let header = std::str::from_utf8(header)?; + let mut fields = header.split_ascii_whitespace(); + let mode = fields + .next() + .context("Git ls-tree returned an entry without a mode")?; + let _kind = fields + .next() + .context("Git ls-tree returned an entry without an object type")?; + let object = fields + .next() + .context("Git ls-tree returned an entry without an object ID")?; + if mode == "160000" { + // A gitlink names a submodule commit that belongs to another + // object database and is not part of this tree's connectivity. + continue; + } + if cat_file_type(repo, object).is_err() { + bail!("fetched tag {reference} contains missing Git object {object}") + } + } + Ok(()) +} + pub fn update_ref(repo: &Path, reference: &str, object: &str) -> Result<()> { let output = Command::new("git") .arg("-C") diff --git a/src/repository.rs b/src/repository.rs index b6e7bfa..34cd561 100644 --- a/src/repository.rs +++ b/src/repository.rs @@ -225,6 +225,19 @@ impl EncryptedRepository { self.push_update_inner(repo, Some(remote_name), source_ref, destination_ref, force) } + pub fn delete_ref(&self, repo: &Path, destination_ref: &str) -> Result { + self.change_ref_inner(repo, None, None, destination_ref, false) + } + + pub fn delete_ref_for_remote( + &self, + repo: &Path, + remote_name: &str, + destination_ref: &str, + ) -> Result { + self.change_ref_inner(repo, Some(remote_name), None, destination_ref, false) + } + fn push_update_inner( &self, repo: &Path, @@ -232,51 +245,74 @@ impl EncryptedRepository { source_ref: &str, destination_ref: &str, force: bool, + ) -> Result { + self.change_ref_inner(repo, remote_name, Some(source_ref), destination_ref, force) + } + + fn change_ref_inner( + &self, + repo: &Path, + remote_name: Option<&str>, + source_ref: Option<&str>, + destination_ref: &str, + force: bool, ) -> Result { let (current, next_object, non_fast_forward) = - self.preflight_push(repo, remote_name, source_ref, destination_ref, force)?; - if current.manifest.refs.get(destination_ref) == Some(&next_object) { + self.preflight_ref_change(repo, remote_name, source_ref, destination_ref, force)?; + if next_object + .as_ref() + .is_some_and(|next| current.manifest.refs.get(destination_ref) == Some(next)) + { return Ok(current.id); } let mut next_refs = current.manifest.refs.clone(); - next_refs.insert(destination_ref.to_owned(), next_object.clone()); - let mut exclusions = BTreeMap::new(); - if !non_fast_forward { - for (reference, object) in ¤t.manifest.refs { - if git::object_exists(repo, object)? { - exclusions.insert(reference.clone(), object.clone()); - } - } + if let Some(next_object) = &next_object { + next_refs.insert(destination_ref.to_owned(), next_object.clone()); + } else { + next_refs.remove(destination_ref); } - let pack_refs = BTreeMap::from([(destination_ref.to_owned(), next_object)]); let next_key = random_key(); let generation = current.manifest.generation + 1; - let ordinal = 0; - let pack_key = derive_subkey( - &next_key, - ¤t.manifest.repository_root, - generation, - SubkeyKind::Pack, - ordinal, - )?; - let mut pack_source = git::start_incremental_pack(repo, &pack_refs, &exclusions)?; - let mut pack_stage = self.storage.begin_object(ObjectKind::Pack)?; - let sealed = seal_pack_stream( - &pack_key, - &mut pack_source, - &mut pack_stage, - &pack_aad(¤t.manifest.repository_root, generation, ordinal), - )?; - pack_source.finish()?; - pack_stage.finish(&sealed.object_id)?; - let descriptor = PackDescriptor { - id: sealed.object_id, - plaintext_size: sealed.plaintext_size, - generation, - ordinal, - }; + let mut new_packs = Vec::new(); + let mut total_pack_count = current.manifest.total_pack_count; + if let Some(next_object) = next_object { + let mut exclusions = BTreeMap::new(); + if !non_fast_forward { + for (reference, object) in ¤t.manifest.refs { + if git::object_exists(repo, object)? { + exclusions.insert(reference.clone(), object.clone()); + } + } + } + let pack_refs = BTreeMap::from([(destination_ref.to_owned(), next_object)]); + let ordinal = 0; + let pack_key = derive_subkey( + &next_key, + ¤t.manifest.repository_root, + generation, + SubkeyKind::Pack, + ordinal, + )?; + let mut pack_source = git::start_incremental_pack(repo, &pack_refs, &exclusions)?; + let mut pack_stage = self.storage.begin_object(ObjectKind::Pack)?; + let sealed = seal_pack_stream( + &pack_key, + &mut pack_source, + &mut pack_stage, + &pack_aad(¤t.manifest.repository_root, generation, ordinal), + )?; + pack_source.finish()?; + pack_stage.finish(&sealed.object_id)?; + new_packs.push(PackDescriptor { + id: sealed.object_id, + plaintext_size: sealed.plaintext_size, + generation, + ordinal, + }); + total_pack_count += 1; + } let predecessor_key_wrap = wrap_predecessor_key( &next_key, ¤t.generation_key, @@ -292,9 +328,9 @@ impl EncryptedRepository { policy_id: current.policy.id.clone(), policy_generation: current.policy.body.generation, authorization: ManifestAuthorization::Writer, - total_pack_count: current.manifest.total_pack_count + 1, + total_pack_count, refs: next_refs, - new_packs: vec![descriptor], + new_packs, predecessor_key_wrap: Some(predecessor_key_wrap), introduced_policy: None, }; @@ -319,7 +355,7 @@ impl EncryptedRepository { destination_ref: &str, force: bool, ) -> Result<()> { - self.preflight_push(repo, None, source_ref, destination_ref, force)?; + self.preflight_ref_change(repo, None, Some(source_ref), destination_ref, force)?; Ok(()) } @@ -331,22 +367,36 @@ impl EncryptedRepository { destination_ref: &str, force: bool, ) -> Result<()> { - self.preflight_push(repo, Some(remote_name), source_ref, destination_ref, force)?; + self.preflight_ref_change( + repo, + Some(remote_name), + Some(source_ref), + destination_ref, + force, + )?; + Ok(()) + } + + pub fn validate_delete_ref_for_remote( + &self, + repo: &Path, + remote_name: &str, + destination_ref: &str, + ) -> Result<()> { + self.preflight_ref_change(repo, Some(remote_name), None, destination_ref, false)?; Ok(()) } - fn preflight_push( + fn preflight_ref_change( &self, repo: &Path, remote_name: Option<&str>, - source_ref: &str, + source_ref: Option<&str>, destination_ref: &str, force: bool, - ) -> Result<(OpenedState, String, bool)> { - if !destination_ref.starts_with("refs/heads/") { - bail!("only refs/heads/* destinations are supported") - } + ) -> Result<(OpenedState, Option, bool)> { git::ensure_repository(repo)?; + let ref_kind = git::validate_inner_ref(repo, destination_ref)?; let chain = self.current_chain()?; let current = chain.first().context("manifest chain is empty")?.clone(); let device_id = self.key.device_id()?; @@ -364,11 +414,34 @@ impl EncryptedRepository { if let Some(remote_name) = remote_name { self.validate_remote_continuity(repo, remote_name, &chain)?; } + for reference in current.manifest.refs.keys() { + git::validate_inner_ref(repo, reference)?; + } + let old_object = current.manifest.refs.get(destination_ref); + let Some(source_ref) = source_ref else { + if old_object.is_none() { + bail!("remote ref {destination_ref} does not exist") + } + if destination_ref == "refs/heads/main" { + bail!( + "cannot delete refs/heads/main because it is advertised as the remote default branch" + ) + } + return Ok((current, None, false)); + }; + let next_object = git::resolve_ref(repo, source_ref)?; - if current.manifest.refs.get(destination_ref) == Some(&next_object) { - return Ok((current, next_object, false)); + if old_object == Some(&next_object) { + return Ok((current, Some(next_object), false)); + } + if ref_kind == git::InnerRefKind::Branch { + git::ensure_branch_target(repo, destination_ref, &next_object)?; } - let non_fast_forward = if let Some(old_object) = current.manifest.refs.get(destination_ref) + if ref_kind == git::InnerRefKind::Tag && old_object.is_some() && !force { + bail!("tag {destination_ref} already exists; use --force to move it") + } + let non_fast_forward = if ref_kind == git::InnerRefKind::Branch + && let Some(old_object) = old_object { if !git::object_exists(repo, old_object)? { bail!("remote tip for {destination_ref} is missing locally; fetch first") @@ -380,16 +453,19 @@ impl EncryptedRepository { if non_fast_forward && !force { bail!("non-fast-forward update rejected for {destination_ref}") } - Ok((current, next_object, non_fast_forward)) + Ok((current, Some(next_object), non_fast_forward)) } pub fn fetch_into(&self, repo: &Path, remote_name: &str) -> Result { let manifest = self.import_packs(repo, remote_name)?; for (reference, object) in &manifest.refs { - let short = reference - .strip_prefix("refs/heads/") - .with_context(|| format!("unsupported remote ref {reference}"))?; - git::update_ref(repo, &format!("refs/remotes/{remote_name}/{short}"), object)?; + match git::validate_inner_ref(repo, reference)? { + git::InnerRefKind::Branch => { + let short = reference.strip_prefix("refs/heads/").unwrap(); + git::update_ref(repo, &format!("refs/remotes/{remote_name}/{short}"), object)?; + } + git::InnerRefKind::Tag => git::update_ref(repo, reference, object)?, + } } Ok(manifest) } @@ -399,6 +475,9 @@ impl EncryptedRepository { validate_remote_name(remote_name)?; let chain = self.current_chain()?; let current = chain.first().context("manifest chain is empty")?.clone(); + for reference in current.manifest.refs.keys() { + git::validate_inner_ref(repo, reference)?; + } let state_path = client_state_path(repo, remote_name)?; let state = read_client_state(&state_path)?; self.validate_pinned_head(&chain, &state)?; @@ -411,6 +490,9 @@ impl EncryptedRepository { validate_remote_name(remote_name)?; let chain = self.current_chain()?; let current = chain.first().context("manifest chain is empty")?.clone(); + for reference in current.manifest.refs.keys() { + git::validate_inner_ref(repo, reference)?; + } let state_path = client_state_path(repo, remote_name)?; let state = read_client_state(&state_path)?; self.validate_pinned_head(&chain, &state)?; diff --git a/tests/carrier_git.rs b/tests/carrier_git.rs index cd28188..791c9fb 100644 --- a/tests/carrier_git.rs +++ b/tests/carrier_git.rs @@ -177,6 +177,104 @@ fn carrier_git_supports_native_push_and_clone_without_plaintext() { assert_eq!(grep.status.code(), Some(1)); } +#[test] +fn carrier_git_supports_tags_and_ref_deletion() { + let temporary = tempfile::tempdir().unwrap(); + let _cache = isolated_cache(&temporary); + let carrier = temporary.path().join("carrier.git"); + let source = temporary.path().join("source"); + let returning = temporary.path().join("returning"); + let key_path = temporary.path().join("repository.key.json"); + let main = init_source(&source, "carrier refs"); + let key = KeyFile::generate(); + key.write_new(&key_path).unwrap(); + init_carrier(temporary.path(), &carrier, &key); + + let url = format!("e2ee::git+{}", carrier.display()); + git(&source, &["remote", "add", "private", &url], false); + git( + &source, + &[ + "config", + "remote.private.e2ee-key", + key_path.to_str().unwrap(), + ], + false, + ); + git(&source, &["push", "private", "main"], true); + git( + &source, + &["tag", "-a", "v-carrier", "-m", "carrier release", &main], + false, + ); + git(&source, &["push", "private", "v-carrier"], true); + + let mut clone_command = Command::new("git"); + clone_command + .args([ + "-c", + &format!("e2ee.key={}", key_path.display()), + "clone", + "-q", + &url, + ]) + .arg(&returning); + add_helper_to_path(&mut clone_command); + let clone_output = clone_command.output().unwrap(); + assert!( + clone_output.status.success(), + "clone stdout={} stderr={}", + String::from_utf8_lossy(&clone_output.stdout), + String::from_utf8_lossy(&clone_output.stderr) + ); + assert_eq!( + git( + &returning, + &["cat-file", "-t", "refs/tags/v-carrier"], + false + ), + "tag" + ); + + git(&source, &["switch", "-q", "-c", "doomed"], false); + fs::write(source.join("note.md"), "branch to delete\n").unwrap(); + git(&source, &["add", "note.md"], false); + git(&source, &["commit", "-q", "-m", "doomed"], false); + git(&source, &["push", "private", "doomed"], true); + git(&source, &["push", "--delete", "private", "doomed"], true); + git(&source, &["push", "private", ":refs/tags/v-carrier"], true); + git( + &returning, + &["fetch", "--prune", "--prune-tags", "origin"], + true, + ); + + assert!( + !git_output( + &returning, + &["show-ref", "--verify", "refs/remotes/origin/doomed"], + false + ) + .status + .success() + ); + assert!( + !git_output( + &returning, + &["show-ref", "--verify", "refs/tags/v-carrier"], + false + ) + .status + .success() + ); + let manifest = + EncryptedRepository::new(GitStorage::open(carrier.to_str().unwrap()).unwrap(), key) + .verify() + .unwrap(); + assert!(!manifest.refs.contains_key("refs/heads/doomed")); + assert!(!manifest.refs.contains_key("refs/tags/v-carrier")); +} + #[test] fn carrier_git_allows_exactly_one_concurrent_writer() { let temporary = tempfile::tempdir().unwrap(); diff --git a/tests/end_to_end.rs b/tests/end_to_end.rs index 91ced98..2d729b2 100644 --- a/tests/end_to_end.rs +++ b/tests/end_to_end.rs @@ -1,7 +1,12 @@ use std::fs; +use std::io::Read; use std::path::Path; use std::process::Command; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, Barrier}; +use std::thread; +use anyhow::Result; use git_remote_e2ee::crypto::{KeyFile, object_id, random_key}; use git_remote_e2ee::manifest::{ Manifest, ManifestAuthorization, open_manifest, peek_manifest_header, seal_manifest, @@ -9,7 +14,37 @@ use git_remote_e2ee::manifest::{ }; use git_remote_e2ee::policy::PolicyState; use git_remote_e2ee::repository::EncryptedRepository; -use git_remote_e2ee::storage::FilesystemStorage; +use git_remote_e2ee::storage::{FilesystemStorage, ObjectKind, ObjectStage, Storage}; + +#[derive(Clone)] +struct BarrierStorage { + inner: FilesystemStorage, + armed: Arc, + reads: Arc, + barrier: Arc, +} + +impl Storage for BarrierStorage { + fn begin_object(&self, kind: ObjectKind) -> Result> { + self.inner.begin_object(kind) + } + + fn open_object(&self, kind: ObjectKind, id: &str) -> Result> { + self.inner.open_object(kind, id) + } + + fn read_head(&self) -> Result> { + let head = self.inner.read_head()?; + if self.armed.load(Ordering::SeqCst) && self.reads.fetch_add(1, Ordering::SeqCst) < 2 { + self.barrier.wait(); + } + Ok(head) + } + + fn compare_and_swap_head(&self, expected: Option<&str>, next: &str) -> Result<()> { + self.inner.compare_and_swap_head(expected, next) + } +} fn git(repo: &Path, args: &[&str]) -> String { let output = Command::new("git") @@ -513,7 +548,67 @@ fn writer_can_add_branch_without_having_other_remote_branch_objects() { } #[test] -fn rejects_tag_destinations_consistently() { +fn deletion_and_tag_publication_race_with_exactly_one_cas_winner() { + let temporary = tempfile::tempdir().unwrap(); + let source = temporary.path().join("source"); + let remote = temporary.path().join("remote"); + initialize_git(&source); + let key = KeyFile::generate(); + let seed_repository = EncryptedRepository::new(FilesystemStorage::new(&remote), key.clone()); + seed_repository.initialize().unwrap(); + let main = commit(&source, "main\n", "main"); + seed_repository + .push_ref(&source, "refs/heads/main", false) + .unwrap(); + git(&source, &["branch", "doomed", &main]); + seed_repository + .push_ref(&source, "refs/heads/doomed", false) + .unwrap(); + git(&source, &["tag", "race-tag", &main]); + let before = seed_repository.verify().unwrap(); + + let storage = BarrierStorage { + inner: FilesystemStorage::new(&remote), + armed: Arc::new(AtomicBool::new(true)), + reads: Arc::new(AtomicUsize::new(0)), + barrier: Arc::new(Barrier::new(2)), + }; + let deleter = EncryptedRepository::new(storage.clone(), key.clone()); + let tagger = EncryptedRepository::new(storage, key); + let deletion_repo = source.clone(); + let tag_repo = source.clone(); + let deletion = thread::spawn(move || deleter.delete_ref(&deletion_repo, "refs/heads/doomed")); + let tag_push = thread::spawn(move || { + tagger.push_update(&tag_repo, "refs/tags/race-tag", "refs/tags/race-tag", false) + }); + let deletion = deletion.join().unwrap(); + let tag_push = tag_push.join().unwrap(); + assert_ne!(deletion.is_ok(), tag_push.is_ok()); + let loser = if deletion.is_err() { + deletion + } else { + tag_push + }; + assert!( + loser + .unwrap_err() + .to_string() + .contains("head changed concurrently") + ); + + let after = seed_repository.verify().unwrap(); + assert_eq!(after.generation, before.generation + 1); + let deletion_won = !after.refs.contains_key("refs/heads/doomed"); + let tag_push_won = after.refs.contains_key("refs/tags/race-tag"); + assert_ne!(deletion_won, tag_push_won); + assert_eq!( + after.total_pack_count, + before.total_pack_count + u64::from(tag_push_won) + ); +} + +#[test] +fn rejects_unsupported_ref_namespaces_consistently() { let temporary = tempfile::tempdir().unwrap(); let source = temporary.path().join("source"); let remote_path = temporary.path().join("remote"); @@ -523,12 +618,10 @@ fn rejects_tag_destinations_consistently() { let encrypted = EncryptedRepository::new(FilesystemStorage::new(&remote_path), key); encrypted.initialize().unwrap(); commit(&source, "tagged\n", "tagged"); - git(&source, &["tag", "v1"]); - let error = encrypted - .push_update(&source, "refs/tags/v1", "refs/tags/v1", false) + .push_update(&source, "refs/heads/main", "refs/notes/review", false) .unwrap_err(); - assert!(error.to_string().contains("only refs/heads")); + assert!(error.to_string().contains("unsupported ref namespace")); assert_eq!(encrypted.verify().unwrap().generation, 0); } diff --git a/tests/git_compatibility.rs b/tests/git_compatibility.rs index 9b1d120..0f1da07 100644 --- a/tests/git_compatibility.rs +++ b/tests/git_compatibility.rs @@ -185,6 +185,234 @@ fn upstream_style_push_refspecs_and_force_work() { ); } +#[test] +fn lightweight_annotated_and_followed_tags_round_trip_through_helper() { + let temporary = tempfile::tempdir().unwrap(); + let source = temporary.path().join("source"); + let before_tags = temporary.path().join("before-tags"); + initialize_git(&source); + let (storage, key_path, key) = setup_remote(temporary.path()); + configure_remote(&source, &storage, &key_path); + let first = commit_file(&source, "first", "first"); + git(&source, &["push", "private", "main"], true); + clone_remote(&before_tags, &storage, &key_path); + + git(&source, &["tag", "v1.0", &first], false); + git(&source, &["push", "private", "v1.0"], true); + git( + &source, + &["tag", "-a", "v1.1", "-m", "release one", &first], + false, + ); + fs::write(source.join("tag-target.bin"), b"tagged blob object\n").unwrap(); + let blob = git(&source, &["hash-object", "-w", "tag-target.bin"], false); + git(&source, &["add", "tag-target.bin"], false); + let tree = git(&source, &["write-tree"], false); + git( + &source, + &["tag", "-a", "v-blob", "-m", "blob target", &blob], + false, + ); + git( + &source, + &["tag", "-a", "v-tree", "-m", "tree target", &tree], + false, + ); + git(&source, &["push", "--tags", "private"], true); + + git(&before_tags, &["fetch", "--tags", "origin"], true); + assert_eq!( + git(&before_tags, &["rev-parse", "refs/tags/v1.0"], false), + first + ); + assert_eq!( + git(&before_tags, &["cat-file", "-t", "refs/tags/v1.1"], false), + "tag" + ); + assert_eq!( + git(&before_tags, &["rev-parse", "refs/tags/v1.1^{}"], false), + first + ); + assert_eq!( + git(&before_tags, &["cat-file", "-t", "refs/tags/v-blob"], false), + "tag" + ); + assert_eq!( + git(&before_tags, &["rev-parse", "refs/tags/v-blob^{}"], false), + blob + ); + git(&before_tags, &["cat-file", "-e", &blob], false); + assert_eq!( + git(&before_tags, &["rev-parse", "refs/tags/v-tree^{}"], false), + tree + ); + assert_eq!(git(&before_tags, &["cat-file", "-t", &tree], false), "tree"); + + let second = commit_file(&source, "second", "second"); + git( + &source, + &["tag", "-a", "v2.0", "-m", "release two", &second], + false, + ); + git(&source, &["push", "--follow-tags", "private", "main"], true); + + let repository = EncryptedRepository::new(FilesystemStorage::new(&storage), key); + let manifest = repository.verify().unwrap(); + assert_eq!(manifest.refs["refs/tags/v1.0"], first); + assert_eq!( + manifest.refs["refs/tags/v1.1"], + git(&source, &["rev-parse", "refs/tags/v1.1"], false) + ); + assert_eq!( + manifest.refs["refs/tags/v2.0"], + git(&source, &["rev-parse", "refs/tags/v2.0"], false) + ); + + let fresh = temporary.path().join("fresh-clone"); + clone_remote( + &fresh, + &storage, + &temporary.path().join("repository.key.json"), + ); + assert_eq!( + git(&fresh, &["cat-file", "-t", "refs/tags/v1.1"], false), + "tag" + ); + assert_eq!( + git(&fresh, &["cat-file", "-t", "refs/tags/v2.0"], false), + "tag" + ); + assert_eq!( + git(&fresh, &["rev-parse", "refs/tags/v2.0^{}"], false), + second + ); +} + +#[test] +fn moving_an_existing_tag_requires_force() { + let temporary = tempfile::tempdir().unwrap(); + let source = temporary.path().join("source"); + initialize_git(&source); + let (storage, key_path, key) = setup_remote(temporary.path()); + configure_remote(&source, &storage, &key_path); + let first = commit_file(&source, "first", "first"); + git(&source, &["push", "private", "main"], true); + git(&source, &["tag", "v1.0", &first], false); + git(&source, &["push", "private", "v1.0"], true); + + let second = commit_file(&source, "second", "second"); + git(&source, &["push", "private", "main"], true); + git(&source, &["tag", "-f", "v1.0", &second], false); + let repository = EncryptedRepository::new(FilesystemStorage::new(&storage), key); + let before = repository.verify().unwrap(); + let rejected = git_output(&source, &["push", "private", "v1.0"], true); + assert!(!rejected.status.success()); + let message = format!( + "{}{}", + String::from_utf8_lossy(&rejected.stdout), + String::from_utf8_lossy(&rejected.stderr) + ); + assert!(message.contains("already exists"), "{message}"); + let after_rejection = repository.verify().unwrap(); + assert_eq!(after_rejection.generation, before.generation); + assert_eq!(after_rejection.refs["refs/tags/v1.0"], first); + + git(&source, &["push", "--force", "private", "v1.0"], true); + assert_eq!(repository.verify().unwrap().refs["refs/tags/v1.0"], second); +} + +#[test] +fn deletions_publish_without_packs_and_prune_on_returning_and_fresh_clones() { + let temporary = tempfile::tempdir().unwrap(); + let source = temporary.path().join("source"); + let returning = temporary.path().join("returning-clone"); + initialize_git(&source); + let (storage, key_path, key) = setup_remote(temporary.path()); + configure_remote(&source, &storage, &key_path); + let main = commit_file(&source, "main", "main"); + git(&source, &["push", "private", "main"], true); + git(&source, &["branch", "doomed", &main], false); + git(&source, &["push", "private", "doomed"], true); + git(&source, &["tag", "v-delete", &main], false); + git(&source, &["push", "private", "v-delete"], true); + clone_remote(&returning, &storage, &key_path); + + let repository = EncryptedRepository::new(FilesystemStorage::new(&storage), key); + let before = repository.verify().unwrap(); + let default_delete = git_output(&source, &["push", "--delete", "private", "main"], true); + assert!(!default_delete.status.success()); + let default_error = format!( + "{}{}", + String::from_utf8_lossy(&default_delete.stdout), + String::from_utf8_lossy(&default_delete.stderr) + ); + assert!( + default_error.contains("remote default branch"), + "{default_error}" + ); + assert_eq!(repository.verify().unwrap().generation, before.generation); + + git(&source, &["push", "--delete", "private", "doomed"], true); + let after_branch_delete = repository.verify().unwrap(); + assert!(!after_branch_delete.refs.contains_key("refs/heads/doomed")); + assert_eq!( + after_branch_delete.total_pack_count, + before.total_pack_count + ); + + git(&source, &["push", "private", ":refs/tags/v-delete"], true); + let after_tag_delete = repository.verify().unwrap(); + assert!(!after_tag_delete.refs.contains_key("refs/tags/v-delete")); + assert_eq!(after_tag_delete.total_pack_count, before.total_pack_count); + + git( + &returning, + &["fetch", "--prune", "--prune-tags", "origin"], + true, + ); + assert!( + git_output( + &returning, + &["show-ref", "--verify", "refs/remotes/origin/doomed"], + false + ) + .status + .code() + .is_some_and(|code| code != 0) + ); + assert!( + git_output( + &returning, + &["show-ref", "--verify", "refs/tags/v-delete"], + false + ) + .status + .code() + .is_some_and(|code| code != 0) + ); + + let fresh = temporary.path().join("fresh-clone"); + clone_remote(&fresh, &storage, &key_path); + assert!( + !git_output( + &fresh, + &["show-ref", "--verify", "refs/remotes/origin/doomed"], + false + ) + .status + .success() + ); + assert!( + !git_output( + &fresh, + &["show-ref", "--verify", "refs/tags/v-delete"], + false + ) + .status + .success() + ); +} + #[test] fn unsupported_upstream_operations_fail_without_publication() { let temporary = tempfile::tempdir().unwrap(); @@ -204,10 +432,19 @@ fn unsupported_upstream_operations_fail_without_publication() { assert_eq!(after_delete.generation, generation); assert_eq!(after_delete.refs["refs/heads/main"], head); - git(&source, &["tag", "v1"], false); - let tag = git_output(&source, &["push", "private", "v1"], true); - assert!(!tag.status.success()); - let after_tag = repository.verify().unwrap(); - assert_eq!(after_tag.generation, generation); - assert!(!after_tag.refs.contains_key("refs/tags/v1")); + let namespace = git_output( + &source, + &["push", "private", "HEAD:refs/notes/review"], + true, + ); + assert!(!namespace.status.success()); + let message = format!( + "{}{}", + String::from_utf8_lossy(&namespace.stdout), + String::from_utf8_lossy(&namespace.stderr) + ); + assert!(message.contains("unsupported ref namespace"), "{message}"); + let after_namespace = repository.verify().unwrap(); + assert_eq!(after_namespace.generation, generation); + assert!(!after_namespace.refs.contains_key("refs/notes/review")); } diff --git a/tests/read_only_helper.rs b/tests/read_only_helper.rs index 8e1716e..264b557 100644 --- a/tests/read_only_helper.rs +++ b/tests/read_only_helper.rs @@ -198,6 +198,23 @@ fn exercise_read_only_helper( "rejected push changed storage" ); } + + let before = snapshot_tree(remote_path); + let output = git_output(&clone, &["push", "--delete", "origin", "main"], true); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success(), + "read-only delete unexpectedly succeeded" + ); + assert!( + stderr.contains(expected_error), + "delete error did not explain the read-only role: {stderr}" + ); + assert_eq!( + snapshot_tree(remote_path), + before, + "rejected delete changed storage" + ); } #[test]