From c0271c303f5a88ac6b2274d34614850b3afdfa41 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Sun, 17 Sep 2023 21:50:37 +0200 Subject: [PATCH] Prevent divergences in canonical head On push, we check whether the resulting state would cause a divergence/fork in the canonical head, and if so, prevent the push from happening. This is to avoid situations where delegates have to then rollback their heads. Note that this doesn't prevent forks from happening altogether, as they could happen asychronously, but it mitigates the problem. --- radicle-cli/examples/git/git-push-diverge.md | 71 ++++++++++++++++++++ radicle-cli/tests/commands.rs | 48 +++++++++++++ radicle-remote-helper/src/push.rs | 40 +++++++++-- radicle/src/storage/git.rs | 2 +- 4 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 radicle-cli/examples/git/git-push-diverge.md diff --git a/radicle-cli/examples/git/git-push-diverge.md b/radicle-cli/examples/git/git-push-diverge.md new file mode 100644 index 000000000..c866ff531 --- /dev/null +++ b/radicle-cli/examples/git/git-push-diverge.md @@ -0,0 +1,71 @@ + +Let's see what happens if we try to push a head which diverges from the +canonical head. + +First we add a second delegate, Bob, to our repo: + +``` ~alice +$ rad delegate add did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk --to rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji +Added delegate 'did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk' +✓ Update successful! +$ rad remote add did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk +✓ Remote bob@z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk added +✓ Remote-tracking branch bob@z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk/master created for z6Mkt67…v4N1tRk +``` + +Then, as Bob, we commit some code on top of the canonical head: + +``` ~bob +$ rad sync --fetch +✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6MknSL…StBU8Vi.. +✓ Fetched repository from 1 seed(s) +$ rad inspect --delegates +did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi (alice) +did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk (bob) +$ git commit -m "Third commit" --allow-empty -q +$ git push rad +$ git branch -arv + alice@z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi/master f2de534 Second commit + rad/master 319a7dc Third commit +``` + +As Alice, we fetch that code, but commit on top of our own master, which is no +longer canonical, since Bob pushed a more recent commit, and the threshold is 1: + +``` ~alice +$ rad sync --fetch +✓ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from z6Mkt67…v4N1tRk.. +✓ Fetched repository from 1 seed(s) +$ git fetch bob@z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk +$ git branch -arv + bob@z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk/master 319a7dc Third commit + rad/master f2de534 Second commit +$ git commit -m "Third commit by Alice" --allow-empty -q +``` + +If we try to push now, we get an error with a hint, telling us that we need to +integrate Bob's changes before pushing ours: + +``` ~alice (stderr) (fail) +$ git push rad +hint: you are attempting to push a commit that would cause your upstream to diverge from the canonical head +hint: to integrate the remote changes, run `git pull --rebase` and try again +error: refusing to update branch to commit that is not a descendant of canonical head +error: failed to push some refs to 'rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi' +``` + +We do that, and notice that we're now able to push our code: + +``` ~alice +$ git pull --rebase +$ git log --oneline +f6cff86 Third commit by Alice +319a7dc Third commit +f2de534 Second commit +08c788d Initial commit +``` +``` ~alice RAD_SOCKET=/dev/null (stderr) +$ git push rad +To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi + f2de534..f6cff86 master -> master +``` diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 85993f6a7..7f934a486 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -1311,6 +1311,54 @@ fn framework_home() { .unwrap(); } +#[test] +fn git_push_diverge() { + logger::init(log::Level::Debug); + + let mut environment = Environment::new(); + let alice = environment.node(Config::test(Alias::new("alice"))); + let bob = environment.node(Config::test(Alias::new("bob"))); + let working = environment.tmp().join("working"); + + fixtures::repository(working.join("alice")); + + test( + "examples/rad-init.md", + working.join("alice"), + Some(&alice.home), + [], + ) + .unwrap(); + + let alice = alice.spawn(); + let mut bob = bob.spawn(); + + bob.connect(&alice).converge([&alice]); + + test( + "examples/rad-clone.md", + working.join("bob"), + Some(&bob.home), + [], + ) + .unwrap(); + + formula(&environment.tmp(), "examples/git/git-push-diverge.md") + .unwrap() + .home( + "alice", + working.join("alice"), + [("RAD_HOME", alice.home.path().display())], + ) + .home( + "bob", + working.join("bob").join("heartwood"), + [("RAD_HOME", bob.home.path().display())], + ) + .run() + .unwrap(); +} + #[test] fn git_push_and_pull() { logger::init(log::Level::Debug); diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index 558c097da..d9c91b535 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -9,7 +9,8 @@ use thiserror::Error; use radicle::cob::object::ParseObjectId; use radicle::cob::patch; -use radicle::crypto::{PublicKey, Signer}; +use radicle::crypto::Signer; +use radicle::identity::Did; use radicle::node; use radicle::node::{Handle, NodeId}; use radicle::prelude::Id; @@ -28,11 +29,20 @@ const DEFAULT_SYNC_TIMEOUT: time::Duration = time::Duration::from_secs(9); #[derive(Debug, Error)] pub enum Error { /// Public key doesn't match the remote namespace we're pushing to. - #[error("public key `{0}` does not match remote namespace")] - KeyMismatch(PublicKey), + #[error("cannot push to remote namespace owned by {0}")] + KeyMismatch(Did), /// No public key is given #[error("no public key given as a remote namespace, perhaps you are attempting to push to restricted refs")] NoKey, + /// Head being pushed diverges from canonical head. + #[error("refusing to update branch to commit that is not a descendant of canonical head")] + HeadsDiverge(git::Oid, git::Oid), + /// Identity document error. + #[error("doc: {0}")] + Doc(#[from] radicle::identity::doc::DocError), + /// Identity payload error. + #[error("payload: {0}")] + Payload(#[from] radicle::identity::doc::PayloadError), /// Invalid command received. #[error("invalid command `{0}`")] InvalidCommand(String), @@ -141,7 +151,7 @@ pub fn run( let nid = url.namespace.ok_or(Error::NoKey).and_then(|ns| { (profile.public_key == ns) .then_some(ns) - .ok_or(Error::KeyMismatch(profile.public_key)) + .ok_or(Error::KeyMismatch(profile.public_key.into())) })?; let signer = profile.signer()?; let mut line = String::new(); @@ -162,6 +172,8 @@ pub fn run( _ => return Err(Error::InvalidCommand(line.trim().to_owned())), } } + let canonical = stored.head()?; + let delegates = stored.delegates()?; // For each refspec, push a ref or delete a ref. for spec in specs { @@ -202,6 +214,26 @@ pub fn run( opts.clone(), ) } else { + let (canonical_ref, canonical_oid) = &canonical; + + // If we're trying to update the canonical head, make sure + // we don't diverge from the current head. + if dst == *canonical_ref && delegates.contains(&Did::from(nid)) { + let head = working.find_reference(src.as_str())?; + let head = head.peel_to_commit()?.id(); + + if !working.graph_descendant_of(head, **canonical_oid)? { + eprintln!( + "hint: you are attempting to push a commit that would \ + cause your upstream to diverge from the canonical head" + ); + eprintln!( + "hint: to integrate the remote changes, run `git pull --rebase` \ + and try again" + ); + return Err(Error::HeadsDiverge(head.into(), *canonical_oid)); + } + } push(src, &dst, *force, &nid, &working, stored, &signer) } } diff --git a/radicle/src/storage/git.rs b/radicle/src/storage/git.rs index e6a216c13..8f6e68c81 100644 --- a/radicle/src/storage/git.rs +++ b/radicle/src/storage/git.rs @@ -554,7 +554,7 @@ impl ReadRepository for Repository { let (_, doc) = self.identity_doc()?; let doc = doc.verified()?; let project = doc.project()?; - let branch_ref = Qualified::from(lit::refs_heads(&project.default_branch())); + let branch_ref = git::refs::branch(project.default_branch()); let raw = self.raw(); let mut heads = Vec::new();