action: add signature verification support

[BKPepe]

This adds a new check to verify GPG/SSH signatures on commits. The check reports the signature status (Good, Bad, Unknown, etc.) and validates it:

• Good (G): PASS
• Bad (B), Revoked (R), Error (E): FAIL
• Unknown (U), Expired (X, Y): WARN
• None (N): SKIP

The test suite has been updated to cover these changes and to enforce commit.gpgsign=false within the test environment to ensure deterministic results.

[GeorgeSapkin]

Interesting. I stopped signing my commits long time ago, because it broke lots of GH workflows, e.g. in OpenWrt org case merging a PR by rebasing would make the commit unsigned (which makes sense, since it's changed), defeating the purpose. Has anything improved recently?

[BKPepe]

in OpenWrt org case merging a PR by rebasing would make the commit unsigned

No, this is still happening. 😭😭

That said, there are repositories like Turris where commits within their repo have to be GPG signed to trigger the build. So it comes in handy there. But you’re right that OpenWrt commits used to be signed a lot more—pretty much every single commit was signed back then—and now those signatures are just vanishing. :-/ Luckily, this check isn't mandatory; it's more that if you actually click to open it, you’ll see it was signed, or it will be visible in the comments.

[Copilot]

Pull request overview

This PR adds signature verification support to the commit formality checker. The implementation checks GPG/SSH signatures on commits and reports their status, with different outcomes based on signature validity: Good signatures pass, Bad/Revoked/Error signatures fail, Unknown/Expired signatures warn, and unsigned commits skip the check.

Key changes:

• Added signature status capture to git format strings using %G?
• Implemented check_signature() function with appropriate helper predicates
• Added comprehensive test infrastructure including SSH key generation and certificate handling
• Enforced commit.gpgsign=false in test environment for deterministic results

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
src/check_formalities.sh	Added signature status extraction to git format, implemented helper functions (is_bad_sign, is_warn_sign, is_unsigned) and check_signature function, integrated signature check into main workflow
src/test.sh	Added SIGN_KEYS array tracking, implemented setup_ssh_keys() for test key generation, updated commit() function to support signing, added three signature test cases, updated all expected results to account for new check, enforced commit.gpgsign=false

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test cases only cover three signature statuses: Good (G), Expired (X), and Unknown (U). According to the PR description, the check should also handle Bad (B), Revoked (R), and Error (E) signatures (which all result in FAIL). Consider adding test cases for these failure scenarios to ensure the check_signature function correctly fails on bad/revoked/error signatures.

[BKPepe]

Described in the commit description. :)

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The signature verification tests only cover three signature statuses: G (good), X (expired), and U (unknown). Consider adding test cases for the remaining statuses defined in the check logic: B (bad), R (revoked), E (error), and Y (expired signature). While N (none) is implicitly tested by all other existing tests that don't specify a sign-key, the failure cases (B, R, E) would provide valuable coverage for the critical failure paths in the signature verification logic.

[GeorgeSapkin]

Does this work if there's a skip reason remaining after check_body or should it be reset? I'm not certain about the intended logic.

[GeorgeSapkin]

I gave the bad signature test a shot:

index 9a2d3c5..0e3d8fc 100755
--- a/src/test.sh
+++ b/src/test.sh
@@ -494,6 +494,19 @@ define \
 		Signed-off-by: Good Author <good.author@example.com>
 	EOF
 
+define \
+	-test          'Signature: bad signature (B)' \
+	-expected      '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 1' \
+	-author        'Bad Author' \
+	-email         'bad.author@example.com' \
+	-subject       'package: bad signature' \
+	-sign-key      'bad' \
+	-body          <<-'EOF'
+		This commit has a bad SSH signature.
+
+		Signed-off-by: Bad Author <bad.author@example.com>
+	EOF
+
 cleanup() {
 	if [ -d "$REPO_DIR" ]; then
 		[ -z "$PARALLEL_WORKER" ] && echo "Cleaning up temporary directory '$REPO_DIR'"
@@ -516,7 +529,7 @@ commit() {
 	local git_opts=()
 	if [ -n "$sign_key" ]; then
 		case "$sign_key" in
-			good|expired|unknown)
+			good|bad|expired|unknown)
 				git_opts+=("-S")
 				local key_file="$REPO_DIR/.ssh/$sign_key"
 				if [ -f "$key_file" ]; then
@@ -528,6 +541,18 @@ commit() {
 
 	GIT_COMMITTER_NAME="$author" GIT_COMMITTER_EMAIL="$email" \
 		git commit --author="$author <${email}>" "${git_opts[@]}" -m "$subject" -m "$body"
+
+	if [ "$sign_key" = 'bad' ]; then
+		local commit_file
+		commit_file=$(mktemp -up "$REPO_DIR")
+		git cat-file commit HEAD > "$commit_file"
+		# Corrupt the commit to invalidate the signature
+		echo 'bad' >> "$commit_file"
+		local new_head
+		new_head=$(git hash-object -t commit -w "$commit_file")
+		git update-ref HEAD "$new_head"
+		rm -f "$commit_file"
+	fi
 }
 
 status_wait() {
@@ -676,11 +701,14 @@ setup_ssh_keys() {
 	git config gpg.format ssh
 	git config gpg.ssh.allowedSignersFile "$ssh_home/allowed_signers"
 
-	# 1. Create a GOOD key
+	# 1. Create a GOOD keys
 	ssh-keygen -t ed25519 -f "$ssh_home/good" -N "" -q
 	echo "good.author@example.com $(cat "$ssh_home/good.pub")" >> "$ssh_home/allowed_signers"
 	echo "test.user@example.com $(cat "$ssh_home/good.pub")" >> "$ssh_home/allowed_signers"
 
+	ssh-keygen -t ed25519 -f "$ssh_home/bad" -N "" -q
+	echo "bad.author@example.com $(cat "$ssh_home/bad.pub")" >> "$ssh_home/allowed_signers"
+
 	# 2. Create a CA key
 	ssh-keygen -t ed25519 -f "$ssh_home/ca" -N "" -q

The bad key is not necessary (could allow and use the good one), but I added it for symmetry with the other key names, so as not to over-complicate the case.

I'm still not 100% how this whole signing works, so maybe the test doesn't make any sense.

I also noticed there's this Sign: B, etc. in the output. I'm not sure this really adds anything to show this to users.

[GeorgeSapkin]

And I suppose it makes sense to update the README with the new check.

[GeorgeSapkin]

Something is up with the expired test: it's giving me unknown. Not sure if relevant to fix or not.

I re-worded the check and README.

[BKPepe]

Oh, this is very good! I was thinking how to do this this way. Thanks!

[BKPepe]

Something is up with the expired test: it's giving me unknown. Not sure if relevant to fix or not.

Let me check this.

[GeorgeSapkin]

Also, what is the supposed outcome when a signature cannot be verified because of a rebase from a different user? Perhaps this should be behind a config flag?

[BKPepe]

If I’m not mistaken, if you rebase it and sign it, it will be marked as Verified. But since you didn’t sign it, Git correctly flagged it as unsigned, and yes — in the case of a rebase, it won’t be fully usable. That’s true, and it was actually discussed here: openwrt/packages#13654 (comment).

However, I agree that OpenWrt is slowly moving away from supporting GPG.

And you’re right. I’ll add it behind a check so that users can enable or disable it. The more configuration options, the better.

[BKPepe]

Okay, added configure flag.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[GeorgeSapkin]

Thinking out loud here: does it actually make sense to enable signature verification, but then pass unsigned commits?

[BKPepe]

Good question, though. It does not, but Copilot requested it - #6 (comment)

[BKPepe]

Rebased.