cli: git push: allow signing commits on push

This adds the `git.sign-on-push` configuration which can be used to
automatically sign unsigned commits before pushing to a Git remote.

CHANGELOG.md

@@ -74,6 +74,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * New template function `config(name)` to access to configuration variable from
   template.
 
+* New `git.sign-on-push` config option to automatically sign commits which are being
+  pushed to a Git remote.
+
 ### Fixed bugs
 
 * Fixed diff selection by external tools with `jj split`/`commit -i FILESETS`.

cli/src/commands/git/push.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::collections::HashMap;
 use std::collections::HashSet;
 use std::fmt;
 use std::io;
@@ -19,8 +20,11 @@ use std::io::Write;
 
 use clap::ArgGroup;
 use clap_complete::ArgValueCandidates;
+use indexmap::IndexSet;
 use itertools::Itertools;
 use jj_lib::backend::CommitId;
+use jj_lib::commit::Commit;
+use jj_lib::commit::CommitIteratorExt as _;
 use jj_lib::config::ConfigGetResultExt as _;
 use jj_lib::git;
 use jj_lib::git::GitBranchPushTargets;
@@ -34,6 +38,7 @@ use jj_lib::refs::LocalAndRemoteRef;
 use jj_lib::repo::Repo;
 use jj_lib::revset::RevsetExpression;
 use jj_lib::settings::UserSettings;
+use jj_lib::signing::SignBehavior;
 use jj_lib::str_util::StringPattern;
 use jj_lib::view::View;
 
@@ -295,7 +300,38 @@ pub fn cmd_git_push(
         return Ok(());
     }
 
-    validate_commits_ready_to_push(ui, &bookmark_updates, &remote, &tx, args)?;
+    let sign_behavior = if tx.settings().get_bool("git.sign-on-push")? {
+        Some(SignBehavior::Own)
+    } else {
+        None
+    };
+    let commits_to_sign =
+        validate_commits_ready_to_push(ui, &bookmark_updates, &remote, &tx, args, sign_behavior)?;
+    if !args.dry_run && !commits_to_sign.is_empty() {
+        if let Some(sign_behavior) = sign_behavior {
+            let num_updated_signatures = commits_to_sign.len();
+            let num_rebased_descendants;
+            (num_rebased_descendants, bookmark_updates) = sign_commits_before_push(
+                &mut tx,
+                commits_to_sign,
+                sign_behavior,
+                bookmark_updates,
+            )?;
+            if let Some(mut formatter) = ui.status_formatter() {
+                writeln!(
+                    formatter,
+                    "Updated signatures of {num_updated_signatures} commits"
+                )?;
+                if num_rebased_descendants > 0 {
+                    writeln!(
+                        formatter,
+                        "Rebased {num_rebased_descendants} descendant commits"
+                    )?;
+                }
+            }
+        }
+    }
+
     if let Some(mut formatter) = ui.status_formatter() {
         writeln!(formatter, "Changes to push to {remote}:")?;
         print_commits_ready_to_push(formatter.as_mut(), tx.repo(), &bookmark_updates)?;
@@ -335,14 +371,17 @@ pub fn cmd_git_push(
 }
 
 /// Validates that the commits that will be pushed are ready (have authorship
-/// information, are not conflicted, etc.)
+/// information, are not conflicted, etc.).
+///
+/// Returns the list of commits which need to be signed.
 fn validate_commits_ready_to_push(
     ui: &Ui,
     bookmark_updates: &[(String, BookmarkPushUpdate)],
     remote: &str,
     tx: &WorkspaceCommandTransaction,
     args: &GitPushArgs,
-) -> Result<(), CommandError> {
+    sign_behavior: Option<SignBehavior>,
+) -> Result<Vec<Commit>, CommandError> {
     let workspace_helper = tx.base_workspace_helper();
     let repo = workspace_helper.repo();
 
@@ -369,6 +408,13 @@ fn validate_commits_ready_to_push(
     } else {
         Box::new(|_: &CommitId| Ok(false))
     };
+    let sign_settings = sign_behavior.map(|sign_behavior| {
+        let mut sign_settings = settings.sign_settings();
+        sign_settings.behavior = sign_behavior;
+        sign_settings
+    });
+
+    let mut commits_to_sign = vec![];
 
     for commit in workspace_helper
         .attach_revset_evaluator(commits_to_push)
@@ -418,8 +464,61 @@ fn validate_commits_ready_to_push(
             }
             return Err(error);
         }
+        if let Some(sign_settings) = &sign_settings {
+            if !commit.is_signed() && sign_settings.should_sign(commit.store_commit()) {
+                commits_to_sign.push(commit);
+            }
+        }
     }
-    Ok(())
+    Ok(commits_to_sign)
+}
+
+/// Signs commits before pushing.
+///
+/// Returns the number of commits with rebased descendants and the updated list
+/// of bookmark names and corresponding [`BookmarkPushUpdate`]s.
+fn sign_commits_before_push(
+    tx: &mut WorkspaceCommandTransaction,
+    commits_to_sign: Vec<Commit>,
+    sign_behavior: SignBehavior,
+    bookmark_updates: Vec<(String, BookmarkPushUpdate)>,
+) -> Result<(usize, Vec<(String, BookmarkPushUpdate)>), CommandError> {
+    let commit_ids: IndexSet<CommitId> = commits_to_sign.iter().ids().cloned().collect();
+    let mut old_to_new_commits_map: HashMap<CommitId, CommitId> = HashMap::new();
+    let mut num_rebased_descendants = 0;
+    tx.repo_mut()
+        .transform_descendants(commit_ids.iter().cloned().collect_vec(), |rewriter| {
+            let old_commit_id = rewriter.old_commit().id().clone();
+            if commit_ids.contains(&old_commit_id) {
+                let commit = rewriter
+                    .reparent()
+                    .set_sign_behavior(sign_behavior)
+                    .write()?;
+                old_to_new_commits_map.insert(old_commit_id, commit.id().clone());
+            } else {
+                num_rebased_descendants += 1;
+                let commit = rewriter.reparent().write()?;
+                old_to_new_commits_map.insert(old_commit_id, commit.id().clone());
+            }
+            Ok(())
+        })?;
+
+    let bookmark_updates = bookmark_updates
+        .into_iter()
+        .map(|(bookmark_name, update)| {
+            (
+                bookmark_name,
+                BookmarkPushUpdate {
+                    old_target: update.old_target,
+                    new_target: update
+                        .new_target
+                        .map(|id| old_to_new_commits_map.get(&id).cloned().unwrap_or(id)),
+                },
+            )
+        })
+        .collect_vec();
+
+    Ok((num_rebased_descendants, bookmark_updates))
 }
 
 fn print_commits_ready_to_push(

cli/src/config-schema.json

@@ -362,6 +362,11 @@
                     "type": "string",
                     "description": "The remote to which commits are pushed",
                     "default": "origin"
+                },
+                "sign-on-push": {
+                    "type": "boolean",
+                    "description": "Whether jj should sign commits before pushing",
+                    "default": "false"
                 }
             }
         },

cli/src/config/misc.toml

@@ -15,6 +15,7 @@ context = 3
 
 [git]
 push-bookmark-prefix = "push-"
+sign-on-push = false
 
 [ui]
 # TODO: delete ui.allow-filesets in jj 0.26+

cli/tests/test_git_push.rs

@@ -1405,6 +1405,146 @@ fn test_git_push_to_remote_named_git() {
     "#);
 }
 
+#[test]
+fn test_git_push_sign_on_push() {
+    let (test_env, workspace_root) = set_up();
+    let template = r#"
+    separate("\n",
+      description.first_line(),
+      if(signature,
+        separate(", ",
+          "Signature: " ++ signature.display(),
+          "Status: " ++ signature.status(),
+          "Key: " ++ signature.key(),
+        )
+      )
+    )
+    "#;
+    test_env.jj_cmd_ok(
+        &workspace_root,
+        &["new", "bookmark2", "-m", "commit to be signed 1"],
+    );
+    test_env.jj_cmd_ok(&workspace_root, &["new", "-m", "commit to be signed 2"]);
+    test_env.jj_cmd_ok(&workspace_root, &["bookmark", "set", "bookmark2"]);
+    test_env.jj_cmd_ok(
+        &workspace_root,
+        &["new", "-m", "commit which should not be signed 1"],
+    );
+    test_env.jj_cmd_ok(
+        &workspace_root,
+        &["new", "-m", "commit which should not be signed 2"],
+    );
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-T", template]);
+    // There should be no signed commits initially
+    insta::assert_snapshot!(stdout, @r#"
+    @  commit which should not be signed 2
+    ○  commit which should not be signed 1
+    ○  commit to be signed 2
+    ○  commit to be signed 1
+    ○  description 2
+    │ ○  description 1
+    ├─╯
+    ◆
+    "#);
+    insta::assert_snapshot!(stderr, @"");
+    test_env.add_config(
+        r#"
+    signing.backend = "test"
+    signing.key = "impeccable"
+    git.sign-on-push = true
+    "#,
+    );
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push", "--dry-run"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Changes to push to origin:
+      Move forward bookmark bookmark2 from 8476341eb395 to 8710e91a14a1
+    Dry-run requested, not pushing.
+    "#);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-T", template]);
+    // There should be no signed commits after performing a dry run
+    insta::assert_snapshot!(stdout, @r#"
+    @  commit which should not be signed 2
+    ○  commit which should not be signed 1
+    ○  commit to be signed 2
+    ○  commit to be signed 1
+    ○  description 2
+    │ ○  description 1
+    ├─╯
+    ◆
+    "#);
+    insta::assert_snapshot!(stderr, @"");
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Updated signatures of 2 commits
+    Rebased 2 descendant commits
+    Changes to push to origin:
+      Move forward bookmark bookmark2 from 8476341eb395 to a6259c482040
+    Working copy now at: kmkuslsw b5f47345 (empty) commit which should not be signed 2
+    Parent commit      : kpqxywon 90df08d3 (empty) commit which should not be signed 1
+    "#);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["log", "-T", template]);
+    // Only commits which are being pushed should be signed
+    insta::assert_snapshot!(stdout, @r#"
+    @  commit which should not be signed 2
+    ○  commit which should not be signed 1
+    ○  commit to be signed 2
+    │  Signature: test-display, Status: good, Key: impeccable
+    ○  commit to be signed 1
+    │  Signature: test-display, Status: good, Key: impeccable
+    ○  description 2
+    │ ○  description 1
+    ├─╯
+    ◆
+    "#);
+    insta::assert_snapshot!(stderr, @"");
+
+    // Immutable commits should not be signed
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_root,
+        &[
+            "bookmark",
+            "create",
+            "bookmark3",
+            "-r",
+            "description('commit which should not be signed 1')",
+        ],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @"Created 1 bookmarks pointing to kpqxywon 90df08d3 bookmark3 | (empty) commit which should not be signed 1");
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_root,
+        &["bookmark", "move", "bookmark2", "--to", "bookmark3"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @"Moved 1 bookmarks to kpqxywon 90df08d3 bookmark2* bookmark3 | (empty) commit which should not be signed 1");
+    test_env.add_config(r#"revset-aliases."immutable_heads()" = "bookmark3""#);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&workspace_root, &["git", "push"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Warning: Refusing to create new remote bookmark bookmark3@origin
+    Hint: Use --allow-new to push new bookmark. Use --remote to specify the remote to push to.
+    Changes to push to origin:
+      Move forward bookmark bookmark2 from a6259c482040 to 90df08d3d612
+    "#);
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&workspace_root, &["log", "-T", template, "-r", "::"]);
+    insta::assert_snapshot!(stdout, @r#"
+    @  commit which should not be signed 2
+    ◆  commit which should not be signed 1
+    ◆  commit to be signed 2
+    │  Signature: test-display, Status: good, Key: impeccable
+    ◆  commit to be signed 1
+    │  Signature: test-display, Status: good, Key: impeccable
+    ◆  description 2
+    │ ○  description 1
+    ├─╯
+    ◆
+    "#);
+    insta::assert_snapshot!(stderr, @"");
+}
+
 fn get_bookmark_output(test_env: &TestEnvironment, repo_path: &Path) -> String {
     // --quiet to suppress deleted bookmarks hint
     test_env.jj_cmd_success(repo_path, &["bookmark", "list", "--all-remotes", "--quiet"])

docs/config.md

@@ -1027,6 +1027,26 @@ as follows:
 backends.ssh.allowed-signers = "/path/to/allowed-signers"
 ```
 
+### Sign commits only on `jj git push`
+
+Instead of signing all commits during creation when `signing.sign-all` is
+set to `true`, the `git.sign-on-push` configuration can be used to sign
+commits only upon running `jj git push`. All mutable unsigned commits
+being pushed will be signed prior to pushing. This might be preferred if the
+signing backend requires user interaction or is slow, so that signing is
+performed in a single batch operation.
+
+```toml
+# Configure signing backend as before, without setting `signing.sign-all`
+[signing]
+backend = "ssh"
+key = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIGj+J6N6SO+4P8dOZqfR1oiay2yxhhHnagH52avUqw5h"
+
+[git]
+sign-on-push = true
+```
+
+
 ## Commit Signature Verification
 
 By default signature verification and display is **disabled** as it incurs a