diff --git a/CHANGELOG.md b/CHANGELOG.md index fc787ff3e..57747bebf 100644 --- a/CHANGELOG.md +++ b/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`. diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 4c748c59d..3877e5e27 100644 --- a/cli/src/commands/git/push.rs +++ b/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, +) -> Result, 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, + sign_behavior: SignBehavior, + bookmark_updates: Vec<(String, BookmarkPushUpdate)>, +) -> Result<(usize, Vec<(String, BookmarkPushUpdate)>), CommandError> { + let commit_ids: IndexSet = commits_to_sign.iter().ids().cloned().collect(); + let mut old_to_new_commits_map: HashMap = 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( diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index c4a705626..1ea267263 100644 --- a/cli/src/config-schema.json +++ b/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" } } }, diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index ae1073873..2c7ccf9dd 100644 --- a/cli/src/config/misc.toml +++ b/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+ diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index a7ab1b947..e36a65769 100644 --- a/cli/tests/test_git_push.rs +++ b/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"]) diff --git a/docs/config.md b/docs/config.md index 00e6c2052..301a574e1 100644 --- a/docs/config.md +++ b/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