diff mbox series

[v5,1/1] ref-filter: add new "signature" atom

Message ID 20230311210607.64927-2-nsengiyumvawilberforce@gmail.com (mailing list archive)
State New, archived
Headers show
Series ref-filter: add new "signature" atom | expand

Commit Message

Nsengiyumva Wilberforce March 11, 2023, 9:06 p.m. UTC
This commit duplicates the code for `signature` atom from pretty.c
to ref-filter.c. This feature will help to get rid of current duplicate
implementation of `signature` atom when unifying implementations by
using ref-filter logic everywhere when ref-filter can do everything
pretty is doing.

Add "signature" atom with `grade`, `signer`, `key`,
`fingerprint`, `primarykeyfingerprint`, `trustlevel` as arguments.
This code and its documentation are inspired by how the %GG, %G?,
%GS, %GK, %GF, %GP, and %GT pretty formats were implemented.

Co-authored-by: Hariom Verma <hariom18599@gmail.com>
Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>
---
 Documentation/git-for-each-ref.txt |  27 ++++++
 ref-filter.c                       | 106 ++++++++++++++++++++++++
 t/t6300-for-each-ref.sh            | 128 +++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)

Comments

Junio C Hamano March 14, 2023, 10:51 p.m. UTC | #1
Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com> writes:

> Co-authored-by: Hariom Verma <hariom18599@gmail.com>
> Co-authored-by: Jaydeep Das <jaydeepjd.8914@gmail.com>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Mentored-by: Hariom Verma <hariom18599@gmail.com>
> Signed-off-by: Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com>

https://github.com/git/git/actions/runs/4410308954/jobs/7727652857#step:6:1898

  + git for-each-ref refs/heads/signed --format=%(signature)
  + test_cmp expected actual
  + test 2 -ne 2
  + eval diff -u "$@"
  + diff -u expected actual
  --- expected2023-03-13 23:08:51.495987174 +0000
  +++ actual2023-03-13 23:08:51.519986953 +0000
  @@ -1,5 +1,4 @@
   gpg: Signature made Mon Mar 13 23:08:51 2023 UTC
   gpg:                using DSA key 13B6F51ECDDE430D
  -gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model
   gpg: Good signature from "C O Mitter <committer@example.com>"
   
  error: last command exited with $?=1
  not ok 342 - test bare signature atom
  + git for-each-ref refs/heads/signed --format=%(signature)
  + test_cmp expected actual
  + test 2 -ne 2
  + eval diff -u "$@"
  + diff -u expected actual
  --- expected
  +++ actual
  @@ -1,5 +1,4 @@
   gpg: Signature made Mon Mar 13 23:08:51 2023 UTC
   gpg:                using DSA key 13B6F51ECDDE430D
  -gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model
   gpg: Good signature from "C O Mitter <committer@example.com>"
   
  error: last command exited with $?=1
  not ok 342 - test bare signature atom

Are there differences between versions of GPG that, for the purpose
of the testing, we do not care?
Kousik Sanagavarapu April 28, 2023, 6:29 p.m. UTC | #2
Nsengiyumva Wilberforce <nsengiyumvawilberforce@gmail.com> writes:

> +test_expect_success GPG 'test bare signature atom' '

I think this test is failing on CI because, as Junio said, there
are different versions of gpg coming into play here. In particular,
this test is failing on (according to the logs) linux32 (daald/ubuntu32:xenial).
The version of GPG that xenial can use is at a maximum v1.4.20 (this is
evident here https://packages.ubuntu.com/xenial/allpackages). But
according the code in lib-gpg.sh, we should be able to handle any GPG
version, except for v1.0.6.

Looking at the logs that Junio posted, I think that the cuplrit is

> +	grep -v "checking the trustdb" out_orig >out &&
> +	head -3 out >expected &&
> +	tail -1 out >>expected &&

but I'm not really sure. So I would really be grateful if you could
explain this hack to me, the one you mentioned in the cover-letter.

Thanks
Kousik Sanagavarapu April 29, 2023, 6:37 p.m. UTC | #3
Kousik Sanagavarapu <five231003@gmail.com> writes:

>> +test_expect_success GPG 'test bare signature atom' '
>
> I think this test is failing on CI because, as Junio said, there
> are different versions of gpg coming into play here. In particular,
> this test is failing on (according to the logs) linux32 (daald/ubuntu32:xenial).
> The version of GPG that xenial can use is at a maximum v1.4.20 (this is
> evident here https://packages.ubuntu.com/xenial/allpackages). But
> according the code in lib-gpg.sh, we should be able to handle any GPG
> version, except for v1.0.6.

After digging a little bit more into this, I found that when checking
the trustdb, the output is different for GPG v1 and GPG v2. So, I
understand now why you did this

>> +	grep -v "checking the trustdb" out_orig >out &&

The test is still failing because this is not the only line in the
output that is different, but also the line following it, which
conveys information about the key trust levels

  gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model


This is also evident from the diff between "expected" and "actual" in
the logs[1].

[1] https://github.com/git/git/actions/runs/4410308954/jobs/7727652857#step:6:1898

Thanks
diff mbox series

Patch

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 6da899c629..9a0be85368 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -212,6 +212,33 @@  symref::
 	`:lstrip` and `:rstrip` options in the same way as `refname`
 	above.
 
+signature::
+	The GPG signature of a commit.
+
+signature:grade::
+	Show "G" for a good (valid) signature, "B" for a bad
+	signature, "U" for a good signature with unknown validity, "X"
+	for a good signature that has expired, "Y" for a good
+	signature made by an expired key, "R" for a good signature
+	made by a revoked key, "E" if the signature cannot be
+	checked (e.g. missing key) and "N" for no signature.
+
+signature:signer::
+	The signer of the GPG signature of a commit.
+
+signature:key::
+	The key of the GPG signature of a commit.
+
+signature:fingerprint::
+	The fingerprint of the GPG signature of a commit.
+
+signature:primarykeyfingerprint::
+	The Primary Key fingerprint of the GPG signature of a commit.
+
+signature:trustlevel::
+	The Trust level of the GPG signature of a commit. Possible
+	outputs are `ultimate`, `fully`, `marginal`, `never` and `undefined`.
+
 worktreepath::
 	The absolute path to the worktree in which the ref is checked
 	out, if it is checked out in any linked worktree. Empty string
diff --git a/ref-filter.c b/ref-filter.c
index a24324123e..2a5c5e9508 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -144,6 +144,7 @@  enum atom_type {
 	ATOM_BODY,
 	ATOM_TRAILERS,
 	ATOM_CONTENTS,
+	ATOM_SIGNATURE,
 	ATOM_RAW,
 	ATOM_UPSTREAM,
 	ATOM_PUSH,
@@ -208,6 +209,10 @@  static struct used_atom {
 		struct email_option {
 			enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
 		} email_option;
+		struct {
+			enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
+			       S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL} option;
+		} signature;
 		struct refname_atom refname;
 		char *head;
 	} u;
@@ -394,6 +399,34 @@  static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
 	return 0;
 }
 
+static int parse_signature_option(const char *arg)
+{
+	if (!arg)
+		return S_BARE;
+	else if (!strcmp(arg, "signer"))
+		return S_SIGNER;
+	else if (!strcmp(arg, "grade"))
+		return S_GRADE;
+	else if (!strcmp(arg, "key"))
+		return S_KEY;
+	else if (!strcmp(arg, "fingerprint"))
+		return S_FINGERPRINT;
+	else if (!strcmp(arg, "primarykeyfingerprint"))
+		return S_PRI_KEY_FP;
+	else if (!strcmp(arg, "trustlevel"))
+		return S_TRUST_LEVEL;
+	return -1;
+}
+
+static int signature_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom,
+			       const char *arg, struct strbuf *err){
+	int opt = parse_signature_option(arg);
+	if (opt < 0)
+		return err_bad_arg(err, "signature", arg);
+	atom->u.signature.option = opt;
+	return 0;
+}
+
 static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
@@ -631,6 +664,7 @@  static struct {
 	[ATOM_BODY] = { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser },
 	[ATOM_TRAILERS] = { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },
 	[ATOM_CONTENTS] = { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser },
+	[ATOM_SIGNATURE] = { "signature", SOURCE_OBJ, FIELD_STR, signature_atom_parser },
 	[ATOM_RAW] = { "raw", SOURCE_OBJ, FIELD_STR, raw_atom_parser },
 	[ATOM_UPSTREAM] = { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
 	[ATOM_PUSH] = { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },
@@ -1362,6 +1396,77 @@  static void grab_person(const char *who, struct atom_value *val, int deref, void
 	}
 }
 
+static void grab_signature(struct atom_value *val, int deref, struct object *obj)
+{
+	int i;
+	struct commit *commit = (struct commit *) obj;
+	struct signature_check sigc = { 0 };
+	int signature_checked = 0;
+
+	for (i = 0; i < used_atom_cnt; i++) {
+		struct used_atom *atom = &used_atom[i];
+		const char *name = atom->name;
+		struct atom_value *v = &val[i];
+
+		if (!!deref != (*name == '*'))
+			continue;
+		if (deref)
+			name++;
+
+		if (!skip_prefix(name, "signature", &name) || (*name &&
+			*name != ':'))
+			continue;
+		if (!*name)
+			name = NULL;
+		else
+			name++;
+		if (parse_signature_option(name) < 0)
+			continue;
+
+		if (!signature_checked) {
+			check_commit_signature(commit, &sigc);
+			signature_checked = 1;
+		}
+
+		if (atom->u.signature.option == S_BARE)
+			v->s = xstrdup(sigc.output ? sigc.output: "");
+		else if (atom->u.signature.option == S_SIGNER)
+			v->s = xstrdup(sigc.signer ? sigc.signer : "");
+		else if (atom->u.signature.option == S_GRADE) {
+			switch (sigc.result) {
+			case 'G':
+				switch (sigc.trust_level) {
+				case TRUST_UNDEFINED:
+				case TRUST_NEVER:
+					v->s = xstrfmt("%c", (char)'U');
+					break;
+				default:
+					v->s = xstrfmt("%c", (char)'G');
+					break;
+				}
+				break;
+			case 'B':
+			case 'E':
+			case 'N':
+			case 'X':
+			case 'Y':
+			case 'R':
+				v->s = xstrfmt("%c", (char)sigc.result);
+			}
+		}
+		else if (atom->u.signature.option == S_KEY)
+			v->s = xstrdup(sigc.key ? sigc.key : "");
+		else if (atom->u.signature.option == S_FINGERPRINT)
+			v->s = xstrdup(sigc.fingerprint ? sigc.fingerprint : "");
+		else if (atom->u.signature.option == S_PRI_KEY_FP)
+			v->s = xstrdup(sigc.primary_key_fingerprint ? sigc.primary_key_fingerprint : "");
+		else if (atom->u.signature.option == S_TRUST_LEVEL)
+			v->s = xstrdup(gpg_trust_level_to_str(sigc.trust_level));
+	}
+	if (signature_checked)
+		signature_check_clear(&sigc);
+}
+
 static void find_subpos(const char *buf,
 			const char **sub, size_t *sublen,
 			const char **body, size_t *bodylen,
@@ -1555,6 +1660,7 @@  static void grab_values(struct atom_value *val, int deref, struct object *obj, s
 		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
+		grab_signature(val, deref, obj);
 		break;
 	case OBJ_TREE:
 		/* grab_tree_values(val, deref, obj, buf, sz); */
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 2ae1fc721b..c74e2ca169 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -6,6 +6,7 @@ 
 test_description='for-each-ref test'
 
 . ./test-lib.sh
+GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY"/lib-gpg.sh
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
@@ -1464,4 +1465,131 @@  sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
 sig_crlf=${sig_crlf%dummy}
 test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"
 
+GRADE_FORMAT="%(signature:grade)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+TRUSTLEVEL_FORMAT="%(signature:trustlevel)%0a%(signature:key)%0a%(signature:signer)%0a%(signature:fingerprint)%0a%(signature:primarykeyfingerprint)"
+
+test_expect_success GPG 'test bare signature atom' '
+	git checkout -b signed &&
+	echo 1 >file && git add file &&
+	test_tick && git commit -S -m initial &&
+	git verify-commit signed 2>out_orig &&
+	grep -v "checking the trustdb" out_orig >out &&
+	head -3 out >expected &&
+	tail -1 out >>expected &&
+	echo >>expected &&
+	git for-each-ref refs/heads/signed --format="%(signature)" >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success GPG 'show good signature with custom format' '
+	echo 2 >file && git add file &&
+	test_tick && git commit -S -m initial &&
+	git verify-commit signed 2>out &&
+	cat >expect <<-\EOF &&
+	G
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/heads/signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'test signature atom with grade option and bad signature' '
+	git config commit.gpgsign true &&
+	echo 3 >file && test_tick && git commit -a -m "third" --no-gpg-sign &&
+	git tag third-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag second-signed HEAD^ &&
+	git tag third-signed &&
+
+	git cat-file commit third-signed >raw &&
+	sed -e "s/^third/3rd forged/" raw >forged1 &&
+	FORGED1=$(git hash-object -w -t commit forged1) &&
+	git update-ref refs/tags/third-signed "$FORGED1" &&
+	test_must_fail git verify-commit "$FORGED1" &&
+
+	cat >expect <<-\EOF &&
+	B
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+
+
+	EOF
+	git for-each-ref refs/tags/third-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with custom format' '
+	echo 4 >file && test_tick && git commit -a -m fourth -SB7227189 &&
+	git tag signed-fourth &&
+	cat >expect <<-\EOF &&
+	U
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/signed-fourth --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with undefined trust level' '
+	echo 5 >file && test_tick && git commit -a -m fifth -SB7227189 &&
+	git tag fifth-signed &&
+	cat >expect <<-\EOF &&
+	undefined
+	65A0EEA02E30CAD7
+	Eris Discordia <discord@example.net>
+	F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
+	D4BE22311AD3131E5EDA29A461092E85B7227189
+	EOF
+	git for-each-ref refs/tags/fifth-signed --format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show untrusted signature with ultimate trust level' '
+	echo 7 >file && test_tick && git commit -a -m "seventh" --no-gpg-sign &&
+	git tag seventh-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag sixth-signed HEAD^ &&
+	git tag seventh-signed &&
+	cat >expect <<-\EOF &&
+	ultimate
+	13B6F51ECDDE430D
+	C O Mitter <committer@example.com>
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	73D758744BE721698EC54E8713B6F51ECDDE430D
+	EOF
+	git for-each-ref refs/tags/seventh-signed --format="$TRUSTLEVEL_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show unknown signature with custom format' '
+	cat >expect <<-\EOF &&
+	E
+	65A0EEA02E30CAD7
+
+
+
+	EOF
+	GNUPGHOME="$GNUPGHOME_NOT_USED" git for-each-ref refs/tags/fifth-signed --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success GPG 'show lack of signature with custom format' '
+	echo 8 >file && test_tick && git commit -a -m "eigth unsigned" --no-gpg-sign &&
+	git tag eigth-unsigned &&
+	cat >expect <<-\EOF &&
+	N
+
+
+
+
+	EOF
+	git for-each-ref refs/tags/eigth-unsigned --format="$GRADE_FORMAT" >actual &&
+	test_cmp expect actual
+'
+
 test_done