From d1a1a7260eb16eb4a0279514cce5e1327ad368a0 Mon Sep 17 00:00:00 2001 From: Anton Bulakh Date: Mon, 27 Nov 2023 02:29:03 +0200 Subject: [PATCH] sign: Add templater methods to show signature info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disclaimer: this is the work of @necauqua and @julienvincent (see #3141). I simply materialized the changes by rebasing them on latest `main` and making the necessary adjustments to pass CI. --- I had to fix an issue in `TestSignatureBackend::sign()`. The following test was failing: ``` ---- test_signature_templates::test_signature_templates stdout ---- ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot: signature_templates Source: cli/tests/test_signature_templates.rs:28 ──────────────────────────────────────────────────────────────────────────────── Expression: stdout ──────────────────────────────────────────────────────────────────────────────── -old snapshot +new results ────────────┬─────────────────────────────────────────────────────────────────── 0 0 │ @ Commit ID: 05ac066d05701071af20e77506a0f2195194cbc9 1 1 │ │ Change ID: qpvuntsmwlqtpsluzzsnyyzlmlwvmlnu 2 2 │ │ Author: Test User (2001-02-03 08:05:07) 3 3 │ │ Committer: Test User (2001-02-03 08:05:07) 4 │-│ Signature: Good test signature 4 │+│ Signature: Bad test signature 5 5 │ │ 6 6 │ │ (no description set) 7 7 │ │ 8 8 │ ◆ Commit ID: 0000000000000000000000000000000000000000 ────────────┴─────────────────────────────────────────────────────────────────── ``` Print debugging revealed that the signature was bad, because of a missing trailing `\n` in `TestSignatureBackend::sign()`. ```diff diff --git a/lib/src/test_signing_backend.rs b/lib/src/test_signing_backend.rs index d47fef1086..0ba249e358 100644 --- a/lib/src/test_signing_backend.rs +++ b/lib/src/test_signing_backend.rs @@ -59,6 +59,8 @@ let key = (!key.is_empty()).then_some(std::str::from_utf8(key).unwrap().to_owned()); let sig = self.sign(data, key.as_deref())?; + dbg!(&std::str::from_utf8(&signature).unwrap()); + dbg!(&std::str::from_utf8(&sig).unwrap()); if sig == signature { Ok(Verification::new( SigStatus::Good, ``` ``` [lib/src/test_signing_backend.rs:62:9] &std::str::from_utf8(&signature).unwrap() = \"--- JJ-TEST-SIGNATURE ---\\nKEY: \\n5300977ff3ecda4555bd86d383b070afac7b7459c07f762af918943975394a8261d244629e430c8554258904f16dd9c18d737f8969f2e7d849246db0d93cc004\\n\" [lib/src/test_signing_backend.rs:63:9] &std::str::from_utf8(&sig).unwrap() = \"--- JJ-TEST-SIGNATURE ---\\nKEY: \\n5300977ff3ecda4555bd86d383b070afac7b7459c07f762af918943975394a8261d244629e430c8554258904f16dd9c18d737f8969f2e7d849246db0d93cc004\" ``` I still have no idea, where in the call chain that trailing `\n` is added to `signature`. I tried to retrace `signature`'s steps. However, it seemed to be returned from `TestSignatureBackend::sign()`, which was even more mind-boggling to me since `sig` is also returned from `TestSignatureBackend::sign()`. How can they be different? --- Co-authored-by: julienvincent Co-authored-by: pylbrecht --- CHANGELOG.md | 3 + cli/src/commit_templater.rs | 166 ++++++++++++++++++++++++++ cli/src/config/colors.toml | 7 ++ cli/src/config/templates.toml | 58 +++++++++ cli/tests/runner.rs | 1 + cli/tests/test_evolog_command.rs | 3 + cli/tests/test_log_command.rs | 3 + cli/tests/test_operations.rs | 3 + cli/tests/test_show_command.rs | 3 + cli/tests/test_signature_templates.rs | 55 +++++++++ cli/tests/test_templater.rs | 2 +- docs/templates.md | 38 ++++++ lib/src/gpg_signing.rs | 37 +++++- lib/src/signing.rs | 24 +++- lib/src/ssh_signing.rs | 15 ++- lib/src/test_signing_backend.rs | 20 ++-- lib/tests/test_gpg.rs | 24 +++- lib/tests/test_signing.rs | 11 +- lib/tests/test_ssh_signing.rs | 8 +- 19 files changed, 446 insertions(+), 35 deletions(-) create mode 100644 cli/tests/test_signature_templates.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dd2150327..56b2c3e4e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -958,6 +958,9 @@ Thanks to the people who made this release happen! * New function `working_copies()` for revsets to show the working copy commits of all workspaces. +* Add templater support for rendering commit signatures and added new builtin templates which + show commit signatures. + ### Fixed bugs None. diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index 68fa4bbce6..11269c62db 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -47,6 +47,9 @@ use jj_lib::revset::RevsetDiagnostics; use jj_lib::revset::RevsetModifier; use jj_lib::revset::RevsetParseContext; use jj_lib::revset::UserRevsetExpression; +use jj_lib::signing::SigStatus; +use jj_lib::signing::SignError; +use jj_lib::signing::Verification; use jj_lib::store::Store; use once_cell::unsync::OnceCell; @@ -72,6 +75,7 @@ use crate::templater::PlainTextFormattedProperty; use crate::templater::SizeHint; use crate::templater::Template; use crate::templater::TemplateFormatter; +use crate::templater::TemplateFunction; use crate::templater::TemplateProperty; use crate::templater::TemplatePropertyError; use crate::templater::TemplatePropertyExt as _; @@ -237,6 +241,11 @@ impl<'repo> TemplateLanguage<'repo> for CommitTemplateLanguage<'repo> { let build = template_parser::lookup_method(type_name, table, function)?; build(self, diagnostics, build_ctx, property, function) } + CommitTemplatePropertyKind::CommitSignature(property) => { + let table = &self.build_fn_table.build_commit_signature_method; + let build = template_parser::lookup_method("CommitSignature", table, function)?; + build(self, diagnostics, build_ctx, property, function) + } } } } @@ -313,6 +322,13 @@ impl<'repo> CommitTemplateLanguage<'repo> { ) -> CommitTemplatePropertyKind<'repo> { CommitTemplatePropertyKind::TreeDiff(Box::new(property)) } + + fn wrap_commit_signature( + &self, + property: impl TemplateProperty + 'repo, + ) -> CommitTemplatePropertyKind<'repo> { + CommitTemplatePropertyKind::CommitSignature(Box::new(property)) + } } pub enum CommitTemplatePropertyKind<'repo> { @@ -326,6 +342,7 @@ pub enum CommitTemplatePropertyKind<'repo> { CommitOrChangeId(Box + 'repo>), ShortestIdPrefix(Box + 'repo>), TreeDiff(Box + 'repo>), + CommitSignature(Box + 'repo>), } impl<'repo> IntoTemplateProperty<'repo> for CommitTemplatePropertyKind<'repo> { @@ -341,6 +358,7 @@ impl<'repo> IntoTemplateProperty<'repo> for CommitTemplatePropertyKind<'repo> { CommitTemplatePropertyKind::CommitOrChangeId(_) => "CommitOrChangeId", CommitTemplatePropertyKind::ShortestIdPrefix(_) => "ShortestIdPrefix", CommitTemplatePropertyKind::TreeDiff(_) => "TreeDiff", + CommitTemplatePropertyKind::CommitSignature(_) => "CommitSignature", } } @@ -366,6 +384,7 @@ impl<'repo> IntoTemplateProperty<'repo> for CommitTemplatePropertyKind<'repo> { // TODO: boolean cast could be implemented, but explicit // diff.empty() method might be better. CommitTemplatePropertyKind::TreeDiff(_) => None, + CommitTemplatePropertyKind::CommitSignature(_) => None, } } @@ -402,6 +421,7 @@ impl<'repo> IntoTemplateProperty<'repo> for CommitTemplatePropertyKind<'repo> { Some(property.into_template()) } CommitTemplatePropertyKind::TreeDiff(_) => None, + CommitTemplatePropertyKind::CommitSignature(_) => None, } } @@ -420,6 +440,7 @@ impl<'repo> IntoTemplateProperty<'repo> for CommitTemplatePropertyKind<'repo> { (CommitTemplatePropertyKind::CommitOrChangeId(_), _) => None, (CommitTemplatePropertyKind::ShortestIdPrefix(_), _) => None, (CommitTemplatePropertyKind::TreeDiff(_), _) => None, + (CommitTemplatePropertyKind::CommitSignature(_), _) => None, } } } @@ -436,6 +457,7 @@ pub struct CommitTemplateBuildFnTable<'repo> { pub commit_or_change_id_methods: CommitTemplateBuildMethodFnMap<'repo, CommitOrChangeId>, pub shortest_id_prefix_methods: CommitTemplateBuildMethodFnMap<'repo, ShortestIdPrefix>, pub tree_diff_methods: CommitTemplateBuildMethodFnMap<'repo, TreeDiff>, + pub build_commit_signature_method: CommitTemplateBuildMethodFnMap<'repo, CommitSignature>, } impl<'repo> CommitTemplateBuildFnTable<'repo> { @@ -448,6 +470,7 @@ impl<'repo> CommitTemplateBuildFnTable<'repo> { commit_or_change_id_methods: builtin_commit_or_change_id_methods(), shortest_id_prefix_methods: builtin_shortest_id_prefix_methods(), tree_diff_methods: builtin_tree_diff_methods(), + build_commit_signature_method: build_commit_signature_method(), } } @@ -459,6 +482,7 @@ impl<'repo> CommitTemplateBuildFnTable<'repo> { commit_or_change_id_methods: HashMap::new(), shortest_id_prefix_methods: HashMap::new(), tree_diff_methods: HashMap::new(), + build_commit_signature_method: HashMap::new(), } } @@ -470,6 +494,7 @@ impl<'repo> CommitTemplateBuildFnTable<'repo> { commit_or_change_id_methods, shortest_id_prefix_methods, tree_diff_methods, + build_commit_signature_method, } = extension; self.core.merge(core); @@ -484,6 +509,10 @@ impl<'repo> CommitTemplateBuildFnTable<'repo> { shortest_id_prefix_methods, ); merge_fn_map(&mut self.tree_diff_methods, tree_diff_methods); + merge_fn_map( + &mut self.build_commit_signature_method, + build_commit_signature_method, + ); } } @@ -594,6 +623,22 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm Ok(L::wrap_boolean(out_property)) }, ); + map.insert( + "signature", + |language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + Ok( + language.wrap_commit_signature(TemplateFunction::new(self_property, |commit| { + match commit.verification() { + Ok(Some(v)) => Ok(CommitSignature::Present(v)), + Err(SignError::InvalidSignatureFormat) => Ok(CommitSignature::Invalid), + Ok(None) => Ok(CommitSignature::Absent), + Err(e) => Err(e.into()), + } + })), + ) + }, + ); map.insert( "working_copies", |language, _diagnostics, _build_ctx, self_property, function| { @@ -1633,3 +1678,124 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T // TODO: add files() or map() to support custom summary-like formatting? map } + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum CommitSignature { + Present(Verification), + Absent, + Invalid, +} + +impl CommitSignature { + pub fn is_present(&self) -> bool { + matches!(self, CommitSignature::Present(_)) + } + + pub fn is_invalid(&self) -> bool { + matches!(self, CommitSignature::Invalid) + } + + pub fn is(&self, status: SigStatus) -> bool { + match self { + CommitSignature::Present(v) => v.status == status, + _ => false, + } + } + + pub fn key(self) -> String { + match self { + CommitSignature::Present(v) => v.key.unwrap_or_default(), + _ => Default::default(), + } + } + + pub fn display(self) -> String { + match self { + CommitSignature::Present(v) => v.display.unwrap_or_default(), + _ => Default::default(), + } + } + + pub fn backend(self) -> String { + match self { + CommitSignature::Present(v) => v.backend().unwrap_or_default().to_owned(), + _ => Default::default(), + } + } +} + +fn build_commit_signature_method<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, CommitSignature> +{ + type L<'repo> = CommitTemplateLanguage<'repo>; + // Not using maplit::hashmap!{} or custom declarative macro here because + // code completion inside macro is quite restricted. + let mut map = CommitTemplateBuildMethodFnMap::::new(); + map.insert( + "present", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = TemplateFunction::new(self_property, |sig| Ok(sig.is_present())); + Ok(L::wrap_boolean(out_property)) + }, + ); + map.insert( + "good", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = + TemplateFunction::new(self_property, |sig| Ok(sig.is(SigStatus::Good))); + Ok(L::wrap_boolean(out_property)) + }, + ); + map.insert( + "unknown", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = + TemplateFunction::new(self_property, |sig| Ok(sig.is(SigStatus::Unknown))); + Ok(L::wrap_boolean(out_property)) + }, + ); + map.insert( + "bad", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = + TemplateFunction::new(self_property, |sig| Ok(sig.is(SigStatus::Bad))); + Ok(L::wrap_boolean(out_property)) + }, + ); + map.insert( + "invalid", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = TemplateFunction::new(self_property, |sig| Ok(sig.is_invalid())); + Ok(L::wrap_boolean(out_property)) + }, + ); + map.insert( + "key", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = TemplateFunction::new(self_property, |sig| Ok(sig.key())); + Ok(L::wrap_string(out_property)) + }, + ); + map.insert( + "display", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = TemplateFunction::new(self_property, |sig| Ok(sig.display())); + Ok(L::wrap_string(out_property)) + }, + ); + map.insert( + "backend", + |_language, _diagnostics, _build_ctx, self_property, function| { + function.expect_no_arguments()?; + let out_property = TemplateFunction::new(self_property, |sig| Ok(sig.backend())); + Ok(L::wrap_string(out_property)) + }, + ); + map +} diff --git a/cli/src/config/colors.toml b/cli/src/config/colors.toml index e14763d2d0..82c2f017ef 100644 --- a/cli/src/config/colors.toml +++ b/cli/src/config/colors.toml @@ -110,3 +110,10 @@ "node current_operation" = { fg = "green", bold = true } "node immutable" = { fg = "bright cyan", bold = true } "node conflict" = { fg = "red", bold = true } + +"signature good" = "green" +"signature unknown" = "bright black" +"signature bad" = "red" +"signature invalid" = "yellow" +"signature key" = "blue" +"signature display" = "yellow" diff --git a/cli/src/config/templates.toml b/cli/src/config/templates.toml index 049b9fd2fc..999b0060cd 100644 --- a/cli/src/config/templates.toml +++ b/cli/src/config/templates.toml @@ -317,3 +317,61 @@ coalesce( "o", ) ''' + +'builtin_log_detailed_with_sig' = ''' +concat( + "Commit ID: " ++ commit_id ++ "\n", + "Change ID: " ++ change_id ++ "\n", + surround("Bookmarks: ", "\n", separate(" ", local_bookmarks, remote_bookmarks)), + surround("Tags: ", "\n", tags), + "Author: " ++ format_detailed_signature(author) ++ "\n", + "Committer: " ++ format_detailed_signature(committer) ++ "\n", + builtin_sig_detailed, + "\n", + indent(" ", if(description, description, description_placeholder ++ "\n")), + "\n", +) +''' + +builtin_sig_status = ''' +if(signature.present(), + label("signature", + concat( + "[", + label("status", + if(signature.good(), label("good", "✓︎"), + if(signature.unknown(), label("unknown", "?"), + if(signature.bad(), label("bad", "x"), + if(signature.invalid(), label("invalid", "x")) + ) + ) + ) + ), + "]" + ) + ) +) +''' + +builtin_sig_detailed = ''' +if(signature.present(), + concat( + "Signature: ", + label("signature", + if(signature.good(), label("good", "Good"), + if(signature.unknown(), label("unknown", "Unknown"), + if(signature.bad(), label("bad", "Bad"), + if(signature.invalid(), label("invalid", "Invalid")) + ) + ) + ) + ), + if(signature.backend(), " " ++ label("backend", signature.backend()) ++ " signature"), + if(signature.display(), + ". By " ++ label("display", signature.display()) ++ if(signature.key(), " (key " ++ label("key", signature.key()) ++ ")"), + if(signature.key(), ". Key " ++ label("key", signature.key())) + ), + "\n" + ) +) +''' diff --git a/cli/tests/runner.rs b/cli/tests/runner.rs index a8312038a7..55a10bf59e 100644 --- a/cli/tests/runner.rs +++ b/cli/tests/runner.rs @@ -63,6 +63,7 @@ mod test_revset_output; mod test_root; mod test_shell_completion; mod test_show_command; +mod test_signature_templates; mod test_simplify_parents_command; mod test_sparse_command; mod test_split_command; diff --git a/cli/tests/test_evolog_command.rs b/cli/tests/test_evolog_command.rs index 69281f8709..990edf1829 100644 --- a/cli/tests/test_evolog_command.rs +++ b/cli/tests/test_evolog_command.rs @@ -336,6 +336,7 @@ fn test_evolog_with_no_template() { - builtin_log_compact - builtin_log_compact_full_description - builtin_log_detailed + - builtin_log_detailed_with_sig - builtin_log_node - builtin_log_node_ascii - builtin_log_oneline @@ -343,6 +344,8 @@ fn test_evolog_with_no_template() { - builtin_op_log_compact - builtin_op_log_node - builtin_op_log_node_ascii + - builtin_sig_detailed + - builtin_sig_status - commit_summary_separator - description_placeholder - email_placeholder diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs index 3bf63a9cc5..39ae619166 100644 --- a/cli/tests/test_log_command.rs +++ b/cli/tests/test_log_command.rs @@ -45,6 +45,7 @@ fn test_log_with_no_template() { - builtin_log_compact - builtin_log_compact_full_description - builtin_log_detailed + - builtin_log_detailed_with_sig - builtin_log_node - builtin_log_node_ascii - builtin_log_oneline @@ -52,6 +53,8 @@ fn test_log_with_no_template() { - builtin_op_log_compact - builtin_op_log_node - builtin_op_log_node_ascii + - builtin_sig_detailed + - builtin_sig_status - commit_summary_separator - description_placeholder - email_placeholder diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index f9fe67d6f1..05b9eec497 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -154,6 +154,7 @@ fn test_op_log_with_no_template() { - builtin_log_compact - builtin_log_compact_full_description - builtin_log_detailed + - builtin_log_detailed_with_sig - builtin_log_node - builtin_log_node_ascii - builtin_log_oneline @@ -161,6 +162,8 @@ fn test_op_log_with_no_template() { - builtin_op_log_compact - builtin_op_log_node - builtin_op_log_node_ascii + - builtin_sig_detailed + - builtin_sig_status - commit_summary_separator - description_placeholder - email_placeholder diff --git a/cli/tests/test_show_command.rs b/cli/tests/test_show_command.rs index 49976414d6..a6758aea27 100644 --- a/cli/tests/test_show_command.rs +++ b/cli/tests/test_show_command.rs @@ -259,6 +259,7 @@ fn test_show_with_no_template() { - builtin_log_compact - builtin_log_compact_full_description - builtin_log_detailed + - builtin_log_detailed_with_sig - builtin_log_node - builtin_log_node_ascii - builtin_log_oneline @@ -266,6 +267,8 @@ fn test_show_with_no_template() { - builtin_op_log_compact - builtin_op_log_node - builtin_op_log_node_ascii + - builtin_sig_detailed + - builtin_sig_status - commit_summary_separator - description_placeholder - email_placeholder diff --git a/cli/tests/test_signature_templates.rs b/cli/tests/test_signature_templates.rs new file mode 100644 index 0000000000..35209410c2 --- /dev/null +++ b/cli/tests/test_signature_templates.rs @@ -0,0 +1,55 @@ +// Copyright 2022 The Jujutsu Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::common::TestEnvironment; + +#[test] +fn test_signature_templates() { + let test_env = TestEnvironment::default(); + + test_env.add_config(r#"signing.sign-all = true"#); + test_env.add_config(r#"signing.backend = "test""#); + + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-Tbuiltin_log_detailed_with_sig"]); + insta::assert_snapshot!(stdout, @r" + @ Commit ID: 05ac066d05701071af20e77506a0f2195194cbc9 + │ Change ID: qpvuntsmwlqtpsluzzsnyyzlmlwvmlnu + │ Author: Test User (2001-02-03 08:05:07) + │ Committer: Test User (2001-02-03 08:05:07) + │ Signature: Good test signature + │ + │ (no description set) + │ + ◆ Commit ID: 0000000000000000000000000000000000000000 + Change ID: zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz + Author: (no name set) <(no email set)> (1970-01-01 11:00:00) + Committer: (no name set) <(no email set)> (1970-01-01 11:00:00) + + (no description set) + "); + + let stdout = test_env.jj_cmd_success(&repo_path, &["show", "-Tbuiltin_log_detailed_with_sig"]); + insta::assert_snapshot!(stdout, @r" + Commit ID: 05ac066d05701071af20e77506a0f2195194cbc9 + Change ID: qpvuntsmwlqtpsluzzsnyyzlmlwvmlnu + Author: Test User (2001-02-03 08:05:07) + Committer: Test User (2001-02-03 08:05:07) + Signature: Good test signature + + (no description set) + "); +} diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs index 0bda44c379..90eda4bdc2 100644 --- a/cli/tests/test_templater.rs +++ b/cli/tests/test_templater.rs @@ -113,7 +113,7 @@ fn test_templater_parse_error() { | ^-----^ | = Keyword "builtin" doesn't exist - Hint: Did you mean "builtin_log_comfortable", "builtin_log_compact", "builtin_log_compact_full_description", "builtin_log_detailed", "builtin_log_node", "builtin_log_node_ascii", "builtin_log_oneline", "builtin_op_log_comfortable", "builtin_op_log_compact", "builtin_op_log_node", "builtin_op_log_node_ascii"? + Hint: Did you mean "builtin_log_comfortable", "builtin_log_compact", "builtin_log_compact_full_description", "builtin_log_detailed", "builtin_log_detailed_with_sig", "builtin_log_node", "builtin_log_node_ascii", "builtin_log_oneline", "builtin_op_log_comfortable", "builtin_op_log_compact", "builtin_op_log_node", "builtin_op_log_node_ascii", "builtin_sig_detailed", "builtin_sig_status"? "#); } diff --git a/docs/templates.md b/docs/templates.md index 58484135fd..c070be61d2 100644 --- a/docs/templates.md +++ b/docs/templates.md @@ -18,6 +18,32 @@ In `jj log`/`jj evolog` templates, all 0-argument methods of [the `Commit` type](#commit-type) are available as keywords. For example, `commit_id` is equivalent to `self.commit_id()`. + * `description: String` + * `change_id: ChangeId` + * `commit_id: CommitId` + * `parents: List` + * `author: Signature` + * `committer: Signature` + * `signature: CommitSignature`: The information about a cryptographic signature of the commit. + * `working_copies: String`: For multi-workspace repository, indicate + working-copy commit as `@`. + * `current_working_copy: Boolean`: True for the working-copy commit of the + current workspace. + * `branches: List`: Local and remote branches pointing to the commit. + A tracking remote branch will be included only if its target is different + from the local one. + * `local_branches: List`: All local branches pointing to the commit. + * `remote_branches: List`: All remote branches pointing to the commit. + * `tags: List` + * `git_refs: List` + * `git_head: List` + * `divergent: Boolean`: True if the commit's change id corresponds to multiple + visible commits. + * `hidden: Boolean`: True if the commit is not visible (a.k.a. abandoned). + * `conflict: Boolean`: True if the commit contains merge conflicts. + * `empty: Boolean`: True if the commit modifies no files. + * `root: Boolean`: True if the commit is the root commit. + ### Operation keywords In `jj op log` templates, all 0-argument methods of [the `Operation` @@ -224,6 +250,18 @@ This type cannot be printed. The following methods are defined. equals to the lower bound. * `.zero() -> Boolean`: True if upper bound is known and is `0`. +### CommitSignature type + +The following methods are defined. + +* `.present() -> Boolean`: True if the commit has a cryptographic signature. +* `.good() -> Boolean`: True if the signature matches the commit data. +* `.unknown() -> Boolean`: True if the signing backend cannot verify the signature (e.g. due to a missing public key), or if there's no backend implemented that can verify the signature. +* `.bad() -> Boolean`: True if the signature does not match the commit data. +* `.invalid() -> Boolean`: True if the signature is detected to be made with a signing backend (e.g. has a PGP prefix) but is otherwise invalid. +* `.key() -> String`: Signing backend specific key id. For GPG, it's a long key ID, present for all non-invalid signatures. +* `.display() -> String`: Signing backend specific display string. For GPG, it's a formatted primary user ID, only present if the public key is known (only for good/bad signatures). + ### String type A string can be implicitly converted to `Boolean`. The following methods are diff --git a/lib/src/gpg_signing.rs b/lib/src/gpg_signing.rs index ff7ff4150c..9b8bc0d0fa 100644 --- a/lib/src/gpg_signing.rs +++ b/lib/src/gpg_signing.rs @@ -70,7 +70,7 @@ fn parse_gpg_verify_output( .next() .and_then(|bs| str::from_utf8(bs).ok()) .map(|value| value.trim().to_owned()); - Some(Verification::new(status, key, display)) + Some(Verification::new(status, key, display, Some("gpg".into()))) }) .ok_or(SignError::InvalidSignatureFormat) } @@ -223,7 +223,12 @@ mod tests { fn gpg_verify_bad_signature() { assert_eq!( parse_gpg_verify_output(b"[GNUPG:] BADSIG 123 456", true).unwrap(), - Verification::new(SigStatus::Bad, Some("123".into()), Some("456".into())) + Verification::new( + SigStatus::Bad, + Some("123".into()), + Some("456".into()), + Some("gpg".into()) + ) ); } @@ -231,7 +236,12 @@ mod tests { fn gpg_verify_unknown_signature() { assert_eq!( parse_gpg_verify_output(b"[GNUPG:] NO_PUBKEY 123", true).unwrap(), - Verification::new(SigStatus::Unknown, Some("123".into()), None) + Verification::new( + SigStatus::Unknown, + Some("123".into()), + None, + Some("gpg".into()) + ) ); } @@ -239,7 +249,12 @@ mod tests { fn gpg_verify_good_signature() { assert_eq!( parse_gpg_verify_output(b"[GNUPG:] GOODSIG 123 456", true).unwrap(), - Verification::new(SigStatus::Good, Some("123".into()), Some("456".into())) + Verification::new( + SigStatus::Good, + Some("123".into()), + Some("456".into()), + Some("gpg".into()) + ) ); } @@ -247,12 +262,22 @@ mod tests { fn gpg_verify_expired_signature() { assert_eq!( parse_gpg_verify_output(b"[GNUPG:] EXPKEYSIG 123 456", true).unwrap(), - Verification::new(SigStatus::Good, Some("123".into()), Some("456".into())) + Verification::new( + SigStatus::Good, + Some("123".into()), + Some("456".into()), + Some("gpg".into()) + ) ); assert_eq!( parse_gpg_verify_output(b"[GNUPG:] EXPKEYSIG 123 456", false).unwrap(), - Verification::new(SigStatus::Bad, Some("123".into()), Some("456".into())) + Verification::new( + SigStatus::Bad, + Some("123".into()), + Some("456".into()), + Some("gpg".into()) + ) ); } } diff --git a/lib/src/signing.rs b/lib/src/signing.rs index ee862f78e6..fb5af02f3e 100644 --- a/lib/src/signing.rs +++ b/lib/src/signing.rs @@ -53,6 +53,11 @@ pub struct Verification { /// A display string, if available. For GPG, this will be formatted primary /// user ID. pub display: Option, + /// The name of the backend that provided this verification. + /// Is `None` when no backend was found that could read the signature. + /// + /// Always set by the signer. + backend: Option, } impl Verification { @@ -63,17 +68,29 @@ impl Verification { status: SigStatus::Unknown, key: None, display: None, + backend: None, } } /// Create a new verification - pub fn new(status: SigStatus, key: Option, display: Option) -> Self { + pub fn new( + status: SigStatus, + key: Option, + display: Option, + backend: Option, + ) -> Self { Self { status, key, display, + backend, } } + + /// The name of the backend that provided this verification. + pub fn backend(&self) -> Option<&str> { + self.backend.as_deref() + } } /// The backend for signing and verifying cryptographic signatures. @@ -236,7 +253,10 @@ impl Signer { .find_map(|backend| match backend.verify(data, signature) { Ok(check) if check.status == SigStatus::Unknown => None, Err(SignError::InvalidSignatureFormat) => None, - e => Some(e), + e => Some(e.map(|mut v| { + v.backend = Some(backend.name().to_owned()); + v + })), }) .transpose()?; diff --git a/lib/src/ssh_signing.rs b/lib/src/ssh_signing.rs index 5ea9eb8788..b033129718 100644 --- a/lib/src/ssh_signing.rs +++ b/lib/src/ssh_signing.rs @@ -251,7 +251,12 @@ impl SigningBackend for SshBackend { Ok(_) => SigStatus::Good, Err(_) => SigStatus::Bad, }; - Ok(Verification::new(status, None, Some(principal))) + Ok(Verification::new( + status, + None, + Some(principal), + Some(self.name().into()), + )) } _ => { command @@ -269,8 +274,14 @@ impl SigningBackend for SshBackend { SigStatus::Unknown, None, Some("Signature OK. Unknown principal".into()), + Some(self.name().into()), + )), + Err(_) => Ok(Verification::new( + SigStatus::Bad, + None, + None, + Some(self.name().into()), )), - Err(_) => Ok(Verification::new(SigStatus::Bad, None, None)), } } } diff --git a/lib/src/test_signing_backend.rs b/lib/src/test_signing_backend.rs index 5e99dee496..a21eebefad 100644 --- a/lib/src/test_signing_backend.rs +++ b/lib/src/test_signing_backend.rs @@ -46,7 +46,7 @@ impl SigningBackend for TestSigningBackend { let hash: String = blake2b_hash(&body).encode_hex(); - Ok(format!("{PREFIX}{key}\n{hash}").into_bytes()) + Ok(format!("{PREFIX}{key}\n{hash}\n").into_bytes()) } fn verify(&self, data: &[u8], signature: &[u8]) -> SignResult { @@ -60,17 +60,19 @@ impl SigningBackend for TestSigningBackend { let sig = self.sign(data, key.as_deref())?; if sig == signature { - Ok(Verification { - status: SigStatus::Good, + Ok(Verification::new( + SigStatus::Good, key, - display: None, - }) + None, + Some(self.name().into()), + )) } else { - Ok(Verification { - status: SigStatus::Bad, + Ok(Verification::new( + SigStatus::Bad, key, - display: None, - }) + None, + Some(self.name().into()), + )) } } } diff --git a/lib/tests/test_gpg.rs b/lib/tests/test_gpg.rs index ddac993bcf..87db755cfa 100644 --- a/lib/tests/test_gpg.rs +++ b/lib/tests/test_gpg.rs @@ -134,7 +134,7 @@ fn gpg_signing_roundtrip_explicit_key() { let data = b"hello world"; let signature = backend.sign(data, Some("Someone Else")).unwrap(); - assert_debug_snapshot!(backend.verify(data, &signature).unwrap(), @r###" + assert_debug_snapshot!(backend.verify(data, &signature).unwrap(), @r#" Verification { status: Good, key: Some( @@ -143,8 +143,11 @@ fn gpg_signing_roundtrip_explicit_key() { display: Some( "Someone Else (jj test signing key) ", ), + backend: Some( + "gpg", + ), } - "###); + "#); assert_debug_snapshot!(backend.verify(b"so so bad", &signature).unwrap(), @r###" Verification { status: Bad, @@ -154,6 +157,9 @@ fn gpg_signing_roundtrip_explicit_key() { display: Some( "Someone Else (jj test signing key) ", ), + backend: Some( + "gpg", + ), } "###); } @@ -172,24 +178,30 @@ fn unknown_key() { e+U6bvqw3pOBoI53Th35drQ0qPI+jAE= =kwsk -----END PGP SIGNATURE-----"; - assert_debug_snapshot!(backend.verify(b"hello world", signature).unwrap(), @r###" + assert_debug_snapshot!(backend.verify(b"hello world", signature).unwrap(), @r#" Verification { status: Unknown, key: Some( "071FE3E324DD7333", ), display: None, + backend: Some( + "gpg", + ), } - "###); - assert_debug_snapshot!(backend.verify(b"so bad", signature).unwrap(), @r###" + "#); + assert_debug_snapshot!(backend.verify(b"so bad", signature).unwrap(), @r#" Verification { status: Unknown, key: Some( "071FE3E324DD7333", ), display: None, + backend: Some( + "gpg", + ), } - "###); + "#); } #[test] diff --git a/lib/tests/test_signing.rs b/lib/tests/test_signing.rs index 0a1c37b85e..aedf7f58f0 100644 --- a/lib/tests/test_signing.rs +++ b/lib/tests/test_signing.rs @@ -42,11 +42,12 @@ fn someone_else() -> Signature { } fn good_verification() -> Option { - Some(Verification { - status: SigStatus::Good, - key: Some("impeccable".to_owned()), - display: None, - }) + Some(Verification::new( + SigStatus::Good, + Some("impeccable".to_owned()), + None, + Some("test".into()), + )) } #[test_case(TestRepoBackend::Local ; "local backend")] diff --git a/lib/tests/test_ssh_signing.rs b/lib/tests/test_ssh_signing.rs index 83d75dba05..c082947afc 100644 --- a/lib/tests/test_ssh_signing.rs +++ b/lib/tests/test_ssh_signing.rs @@ -37,14 +37,14 @@ y2yxhhHnagH52avUqw5hAAAAAAECAwQF static PUBLIC_KEY: &str = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIGj+J6N6SO+4P8dOZqfR1oiay2yxhhHnagH52avUqw5h"; -struct SshEnvironment { +pub struct SshEnvironment { _keys: tempfile::TempDir, - private_key_path: PathBuf, - allowed_signers: Option, + pub private_key_path: PathBuf, + pub allowed_signers: Option, } impl SshEnvironment { - fn new() -> Result { + pub fn new() -> Result { let keys_dir = tempfile::Builder::new() .prefix("jj-test-signing-keys-") .tempdir()