cli: git: 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.

cli/src/commands/git/push.rs

@@ -12,15 +12,19 @@
 // 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;
 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;
 
@@ -303,8 +308,44 @@ pub fn cmd_git_push(
         return Ok(());
     }
 
-    validate_commits_ready_to_push(ui, &bookmark_updates, &remote, &tx, command, args)?;
+    let commits_to_sign =
+        validate_commits_ready_to_push(ui, &bookmark_updates, &remote, &tx, command, args)?;
+    let num_updated_signatures = commits_to_sign.len();
+    let (num_rebased_descendants, bookmark_updates) = sign_commits_before_push(
+        &mut tx,
+        bookmark_updates,
+        commits_to_sign,
+        command,
+        args.dry_run,
+    )?;
     if let Some(mut formatter) = ui.status_formatter() {
+        if num_updated_signatures > 0 {
+            if args.dry_run {
+                writeln!(
+                    formatter,
+                    "Dry-run requested, skipped updating signatures of {num_updated_signatures} \
+                     commits"
+                )?;
+                if num_rebased_descendants > 0 {
+                    writeln!(
+                        formatter,
+                        "Dry-run requested, skipped rebasing of {num_rebased_descendants} \
+                         descendant commits"
+                    )?;
+                }
+            } else {
+                writeln!(
+                    formatter,
+                    "Updated signatures of {num_updated_signatures} commits"
+                )?;
+                if num_rebased_descendants > 0 {
+                    writeln!(
+                        formatter,
+                        "Rebased {num_rebased_descendants} descendant commits"
+                    )?;
+                }
+            }
+        }
         writeln!(formatter, "Changes to push to {remote}:")?;
         print_commits_ready_to_push(formatter.as_mut(), tx.repo(), &bookmark_updates)?;
     }
@@ -343,15 +384,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,
     command: &CommandHelper,
     args: &GitPushArgs,
-) -> Result<(), CommandError> {
+) -> Result<Vec<Commit>, CommandError> {
     let workspace_helper = tx.base_workspace_helper();
     let repo = workspace_helper.repo();
 
@@ -378,6 +421,13 @@ fn validate_commits_ready_to_push(
     } else {
         Box::new(|_: &CommitId| Ok(false))
     };
+    let mut sign_settings = settings.sign_settings();
+    sign_settings.behavior = if settings.get_bool("git.sign-on-push").unwrap_or(false) {
+        SignBehavior::Own
+    } else {
+        SignBehavior::Keep
+    };
+    let mut commits_to_sign = vec![];
 
     for commit in workspace_helper
         .attach_revset_evaluator(commits_to_push)
@@ -427,8 +477,80 @@ fn validate_commits_ready_to_push(
             }
             return Err(error);
         }
+        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 if configured to do so.
+///
+/// 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,
+    bookmark_updates: Vec<(String, BookmarkPushUpdate)>,
+    commits_to_sign: Vec<Commit>,
+    command: &CommandHelper,
+    dry_run: bool,
+) -> Result<(usize, Vec<(String, BookmarkPushUpdate)>), CommandError> {
+    let sign_behavior = if command
+        .settings()
+        .get_bool("git.sign-on-push")
+        .unwrap_or(false)
+    {
+        SignBehavior::Own
+    } else {
+        SignBehavior::Keep
+    };
+    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(
+        command.settings(),
+        commit_ids.iter().cloned().collect_vec(),
+        |rewriter| {
+            let old_commit_id = rewriter.old_commit().id().clone();
+            if commit_ids.contains(&old_commit_id) {
+                if !dry_run {
+                    let commit = rewriter
+                        .rebase(command.settings())?
+                        .set_sign_behavior(sign_behavior)
+                        .write()?;
+                    old_to_new_commits_map.insert(old_commit_id, commit.id().clone());
+                }
+            } else {
+                num_rebased_descendants += 1;
+                if !dry_run {
+                    let commit = rewriter.rebase(command.settings())?.write()?;
+                    old_to_new_commits_map.insert(old_commit_id, commit.id().clone());
+                }
+            }
+            Ok(())
+        },
+    )?;
+
+    let bookmark_updates = if dry_run {
+        bookmark_updates
+    } else {
+        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/tests/test_git_push.rs

@@ -1407,6 +1407,113 @@ fn test_git_push_to_remote_named_git() {
     "#);
 }
 
+#[test]
+fn test_git_push_sign_on_push() {
+    let (test_env, workspace_root) = set_up();
+    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",
+            r#"separate("\n", description.first_line(), builtin_sig_detailed)"#,
+        ],
+    );
+    // 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#"
+    Dry-run requested, skipped updating signatures of 2 commits
+    Dry-run requested, skipped rebasing of 2 descendant commits
+    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",
+            r#"separate("\n", description.first_line(), builtin_sig_detailed)"#,
+        ],
+    );
+    // 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",
+            r#"separate("\n", description.first_line(), builtin_sig_detailed)"#,
+        ],
+    );
+    // 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: Good test signature. Key impeccable
+    ○  commit to be signed 1
+    │  Signature: Good test signature. 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

@@ -1020,6 +1020,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
+```
+
+
 ## Git settings
 
 ### Default remotes for `jj git fetch` and `jj git push`