From fbada7426863f043b38512b2134be4fd7786a4d8 Mon Sep 17 00:00:00 2001 From: Josef Schlehofer Date: Mon, 29 Dec 2025 09:04:02 +0100 Subject: [PATCH] action: add signature verification support 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. Co-authored-by: George Sapkin Signed-off-by: Josef Schlehofer --- README.md | 7 ++ action.yml | 5 + src/check_formalities.sh | 41 ++++++- src/test.sh | 223 ++++++++++++++++++++++++++++++++------- 4 files changed, 233 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index dd16fe6..e7bca02 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ Commit & PR formalities checker based on the OpenWrt [submission guidelines]( - Commit message lines should be <= `MAX_BODY_LINE_LEN` characters long. Limit is 75 by default and is configurable via the `max_body_line_len` input. - Commit to stable branch should be marked as cherry-picked. +- Commit signature must be valid. Configured via the `check_signature` input. + When enabled, invalid signatures will cause a failure (missing are ignored). ## Inputs @@ -42,6 +44,11 @@ All inputs are optional. - Check if pull request comes from a feature branch. - Default: `true`. +### `check_signature` + +- Validate commit signatures. Invalid signatures fail, missing are ignored. +- Default: `false`. + ### `check_signoff` - Check if `Signed-off-by` exists and matches author. diff --git a/action.yml b/action.yml index 9d9c0b7..38c26f9 100644 --- a/action.yml +++ b/action.yml @@ -11,6 +11,10 @@ inputs: description: 'Check if pull request comes from a feature branch' default: 'true' required: false + check_signature: + description: 'Validate commit signatures. Invalid signatures fail, missing are ignored.' + default: 'false' + required: false check_signoff: description: 'Check if Signed-off-by exists and matches author' default: 'false' @@ -80,6 +84,7 @@ runs: ACTION_PATH: ${{ github.action_path }} BASE_BRANCH: origin/${{ github.base_ref }} CHECK_BRANCH: ${{ inputs.check_branch }} + CHECK_SIGNATURE: ${{ inputs.check_signature }} CHECK_SIGNOFF: ${{ inputs.check_signoff }} EXCLUDE_DEPENDABOT: ${{ inputs.exclude_dependabot }} EXCLUDE_WEBLATE: ${{ inputs.exclude_weblate }} diff --git a/src/check_formalities.sh b/src/check_formalities.sh index 6160a1a..b2b5e19 100755 --- a/src/check_formalities.sh +++ b/src/check_formalities.sh @@ -13,6 +13,7 @@ MAX_SUBJECT_LEN_SOFT=${MAX_SUBJECT_LEN_SOFT:-50} MAX_BODY_LINE_LEN=${MAX_BODY_LINE_LEN:-75} CHECK_BRANCH=${CHECK_BRANCH:-true} +CHECK_SIGNATURE=${CHECK_SIGNATURE:-false} CHECK_SIGNOFF=${CHECK_SIGNOFF:-false} EXCLUDE_DEPENDABOT=${EXCLUDE_DEPENDABOT:-false} EXCLUDE_WEBLATE=${EXCLUDE_WEBLATE:-false} @@ -74,7 +75,7 @@ _R=$'\xfa' GIT_HEADER='%C(yellow)commit %H%n%C(reset)Author: %an <%ae>%nCommit: %cn <%ce>%n%n%w(0,4,4)%B' # GH actions sometimes return a mix of body %b and raw body %B when body is # requested, so always use raw body -GIT_VARS="%H${_F}%aN${_F}%aE${_F}%cN${_F}%cE${_F}%s${_F}%B${_F}Signed-off-by: %aN <%aE>${_F}%P" +GIT_VARS="%H${_F}%aN${_F}%aE${_F}%cN${_F}%cE${_F}%s${_F}%B${_F}Signed-off-by: %aN <%aE>${_F}%P${_F}%G?" GIT_FORMAT="${_F}${GIT_HEADER}${_F}${GIT_VARS}${_R}" ACTION_PATH=${ACTION_PATH:+"$ACTION_PATH/src"} @@ -209,12 +210,16 @@ output_split_fail_ex() { # shellcheck disable=SC2329 check_branch() { [ "$CHECK_BRANCH" = 'true' ]; } # shellcheck disable=SC2329 +check_signature() { [ "$CHECK_SIGNATURE" = 'true' ]; } +# shellcheck disable=SC2329 check_signoff() { [ "$CHECK_SIGNOFF" = 'true' ]; } # shellcheck disable=SC2329 do_not_check_branch() { ! check_branch; } # shellcheck disable=SC2329 do_not_check_signoff() { ! check_signoff; } # shellcheck disable=SC2329 +do_not_check_signature() { ! check_signature; } +# shellcheck disable=SC2329 ends_with_period() { [[ "$1" =~ \.$ ]]; } exclude_dependabot() { [ "$EXCLUDE_DEPENDABOT" = 'true' ]; } exclude_weblate() { [ "$EXCLUDE_WEBLATE" = 'true' ]; } @@ -244,7 +249,10 @@ show_legend() { [ "$SHOW_LEGEND" = 'true' ]; } show_feedback() { [ -n "$FEEDBACK_URL" ]; } # shellcheck disable=SC2329 starts_with_space() { [[ "$1" =~ ^[[:space:]] ]]; } - +# shellcheck disable=SC2329 +is_bad_sign() { [[ "$1" =~ ^[BRE]$ ]]; } +# shellcheck disable=SC2329 +is_warn_sign() { [[ "$1" =~ ^[UXY]$ ]]; } # shellcheck disable=SC2329 is_body_empty() { local line @@ -556,6 +564,30 @@ check_body() { -warn-actual "a stable branch (\`${BASE_BRANCH#origin/}\`)" } +check_commit_signature() { + local status="$1" + local reason + + case "$status" in + B) reason='bad signature' ;; + E) reason='signature cannot be checked' ;; + R) reason='revoked key signature' ;; + U) reason='good signature, validity unknown' ;; + X) reason='good signature, expired' ;; + Y) reason='good signature, expired key' ;; + *) reason='no signature' ;; + esac + + check \ + -rule 'Commit signature must be valid' \ + -skip-if do_not_check_signature \ + -skip-reason 'disabled by configuration' \ + -fail-if is_bad_sign "$status" \ + -fail-actual "$reason" \ + -warn-if is_warn_sign "$status" \ + -warn-actual "$reason" +} + main() { # Initialize GitHub actions output output 'content< EOF - - define \ -test 'Feature branch check disabled' \ - -expected '3 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3' \ + -expected '3 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: add new feature' \ @@ -428,7 +433,7 @@ define \ define \ -test 'Feature branch check enabled, PR from main fails' \ - -expected '1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3' \ + -expected '1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: add new feature' \ @@ -442,7 +447,7 @@ define \ define \ -test 'Feature branch check enabled, PR from feature branch passes' \ - -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: add new feature' \ @@ -456,7 +461,7 @@ define \ define \ -test 'LaTeX special chars' \ - -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 3' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 2 3 3' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: stress test latex escaping' \ @@ -466,6 +471,75 @@ define \ Signed-off-by: Good Author EOF +define \ + -test 'Signature: good signature (G)' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: signed commit' \ + -sign-key 'good' \ + -check-signature 'true' \ + -body <<-'EOF' + This commit has a good SSH signature. + + Signed-off-by: Good Author + EOF + +define \ + -test 'Signature: expired key (X)' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 2' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: expired key signature' \ + -sign-key 'expired' \ + -check-signature 'true' \ + -body <<-'EOF' + This commit has a signature from an expired SSH key. + + Signed-off-by: Good Author + EOF + +define \ + -test 'Signature: unknown key (U)' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 2' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: unknown key signature' \ + -sign-key 'unknown' \ + -check-signature 'true' \ + -body <<-'EOF' + This commit has a signature from an unknown SSH key. + + Signed-off-by: Good Author + 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' \ + -check-signature 'true' \ + -body <<-'EOF' + This commit has a bad SSH signature. + + Signed-off-by: Bad Author + EOF + +define \ + -test 'Signature: unsigned commit (N)' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: unsigned commit' \ + -check-signature 'true' \ + -body <<-'EOF' + This commit has no signature but should pass. + + Signed-off-by: Good Author + EOF + cleanup() { if [ -d "$REPO_DIR" ]; then [ -z "$PARALLEL_WORKER" ] && echo "Cleaning up temporary directory '$REPO_DIR'" @@ -480,12 +554,42 @@ commit() { local email="$2" local subject="$3" local body="$4" + local sign_key="${5:-}" touch "file-$(date +%s-%N).txt" git add . + local git_opts=() + if [ -n "$sign_key" ]; then + case "$sign_key" in + good|bad|expired|unknown) + git_opts+=("-S") + + local key_file="$REPO_DIR/.ssh/$sign_key" + if [ "$sign_key" = 'expired' ] && [ -f "${key_file}-cert.pub" ]; then + key_file="${key_file}-cert.pub" + fi + if [ -f "$key_file" ]; then + git config user.signingkey "$key_file" + fi + ;; + esac + fi + GIT_COMMITTER_NAME="$author" GIT_COMMITTER_EMAIL="$email" \ - git commit --author="$author <${email}>" -m "$subject" -m "$body" + git commit --author="$author <${email}>" "${git_opts[@]}" -m "$subject" -m "$body" + + if [ "$sign_key" = 'bad' ]; then + local commit_file + commit_file=$(mktemp "$REPO_DIR/XXXXXXXX") + 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() { @@ -521,12 +625,13 @@ run_test() { local body="$6" local merge="${7:-0}" local injection_file="${8:-}" + local sign_key="${9:-}" local expected_results read -r -a expected_results <<< "$expected_results_str" [ "$merge" = 1 ] && git switch "$BASE_BRANCH" >/dev/null 2>&1 - commit "$author" "$email" "$subject" "$body" >/dev/null + commit "$author" "$email" "$subject" "$body" "$sign_key" >/dev/null [ "$merge" = 1 ] \ && git switch "$HEAD_BRANCH" >/dev/null 2>&1 \ && git merge --no-ff "$BASE_BRANCH" -m "Merge branch '$BASE_BRANCH' into '$HEAD_BRANCH'" >/dev/null 2>&1 @@ -624,6 +729,35 @@ run_test() { fi } +setup_ssh_keys() { + local ssh_home="$REPO_DIR/.ssh" + mkdir -p "$ssh_home" + chmod 700 "$ssh_home" + + # Enable SSH signing + git config gpg.format ssh + git config gpg.ssh.allowedSignersFile "$ssh_home/allowed_signers" + + # 1. Create a GOOD key and then BAD key + 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 + + # 3. Create an EXPIRED key (Certificate) + ssh-keygen -t ed25519 -f "$ssh_home/expired" -N "" -q + ssh-keygen -s "$ssh_home/ca" -I "expired-key" -V -1w:-1d -n "good.author@example.com" "$ssh_home/expired.pub" 2>/dev/null + echo "good.author@example.com cert-authority $(cat "$ssh_home/ca.pub")" >> "$ssh_home/allowed_signers" + + # 4. Create an UNKNOWN key + ssh-keygen -t ed25519 -f "$ssh_home/unknown" -N "" -q +} + run_worker() { local idx="$1" local base_dir="$2" @@ -637,12 +771,20 @@ run_worker() { git init -b "$BASE_BRANCH" >/dev/null git config user.name 'Test User' git config user.email 'test.user@example.com' + git config commit.gpgsign false + + # Setup SSH keys if this test needs them + if [ -n "${SIGN_KEYS[$idx]}" ]; then + setup_ssh_keys + fi + commit \ 'Initial Committer' 'initial@example.com'\ 'initial: commit' 'This is the first main commit.' >/dev/null git switch -C "$HEAD_BRANCH" >/dev/null 2>&1 export CHECK_BRANCH="${ENV_CHECK_BRANCH[$idx]}" + export CHECK_SIGNATURE="${ENV_CHECK_SIGNATURE[$idx]}" export CHECK_SIGNOFF="${ENV_CHECK_SIGNOFF[$idx]}" export EXCLUDE_DEPENDABOT="${ENV_EXCLUDE_DEPENDABOT[$idx]}" export EXCLUDE_WEBLATE="${ENV_EXCLUDE_WEBLATE[$idx]}" @@ -658,7 +800,8 @@ run_worker() { "${SUBJECTS[$idx]}" \ "${BODIES[$idx]}" \ "${MERGES[$idx]}" \ - "${INJECTIONS[$idx]}") + "${INJECTIONS[$idx]}" \ + "${SIGN_KEYS[$idx]}") local res=$? echo "$output" > "$base_dir/$idx.log"