diff --git a/README.md b/README.md index dd16fe6..ff13939 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,9 @@ 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. +- Verifies commit signature (GPG/SSH) if present. Missing signatures are ignored, but invalid signatures will cause a failure. +- Modified files must end with a newline. Configured via the `check_trailing_newline` input. +- Modified files must not contain trailing whitespace. Configured via the `check_trailing_whitespace` input. ## Inputs @@ -47,6 +50,16 @@ All inputs are optional. - Check if `Signed-off-by` exists and matches author. - Default: `false`. +### `check_trailing_newline` + +- Check if modified files end with a newline. +- Default: `true`. + +### `check_trailing_whitespace` + +- Check if modified files contain trailing whitespace. +- Default: `true`. + ### `exclude_dependabot` - Exclude commits authored by dependabot from some checks. diff --git a/src/check_formalities.sh b/src/check_formalities.sh index c7640a2..1e3bb0f 100755 --- a/src/check_formalities.sh +++ b/src/check_formalities.sh @@ -12,6 +12,8 @@ MAX_BODY_LINE_LEN=${MAX_BODY_LINE_LEN:-75} CHECK_BRANCH=${CHECK_BRANCH:-true} CHECK_SIGNOFF=${CHECK_SIGNOFF:-false} +CHECK_TRAILING_NEWLINE=${CHECK_TRAILING_NEWLINE:-true} +CHECK_TRAILING_WHITESPACE=${CHECK_TRAILING_WHITESPACE:-true} EXCLUDE_DEPENDABOT=${EXCLUDE_DEPENDABOT:-false} EXCLUDE_WEBLATE=${EXCLUDE_WEBLATE:-false} SHOW_LEGEND=${SHOW_LEGEND:-true} @@ -67,7 +69,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"} @@ -193,13 +195,21 @@ output_split_fail_ex() { } # shellcheck disable=SC2329 -check_branch() { [ "$CHECK_BRANCH" = 'true' ]; } +check_branch() { [ "$CHECK_BRANCH" = 'true' ]; } # shellcheck disable=SC2329 -check_signoff() { [ "$CHECK_SIGNOFF" = 'true' ]; } +check_signoff() { [ "$CHECK_SIGNOFF" = 'true' ]; } # shellcheck disable=SC2329 -do_not_check_branch() { ! check_branch; } +check_trailing_newline() { [ "$CHECK_TRAILING_NEWLINE" = 'true' ]; } # shellcheck disable=SC2329 -do_not_check_signoff() { ! check_signoff; } +check_trailing_whitespace() { [ "$CHECK_TRAILING_WHITESPACE" = '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_trailing_newline() { ! check_trailing_newline; } +# shellcheck disable=SC2329 +do_not_check_trailing_whitespace(){ ! check_trailing_whitespace; } # shellcheck disable=SC2329 ends_with_period() { [[ "$1" =~ \.$ ]]; } exclude_dependabot() { [ "$EXCLUDE_DEPENDABOT" = 'true' ]; } @@ -230,6 +240,16 @@ 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_unsigned() { [ "$1" = 'N' ]; } +# shellcheck disable=SC2329 +has_missing_newline() { git show --pretty=format: "$1" | grep -q "\\ No newline at end of file"; } +# shellcheck disable=SC2329 +has_trailing_whitespace(){ git show --check --pretty=format: "$1" >/dev/null 2>&1 && return 1; return 0; } # shellcheck disable=SC2329 is_body_empty() { @@ -542,6 +562,38 @@ check_body() { -warn-actual "a stable branch (\`${BASE_BRANCH#origin/}\`)" } +check_signature() { + local status="$1" + + check \ + -rule 'Signature must be valid' \ + -skip-if is_unsigned "$status" \ + -skip-reason 'No signature' \ + -fail-if is_bad_sign "$status" \ + -fail-actual "Bad/Revoked/Error signature (Status: $status)" \ + -warn-if is_warn_sign "$status" \ + -warn-actual "Expired/Unknown signature (Status: $status)" \ + -pass-reason 'Good signature' +} + +check_content() { + local commit="$1" + + check \ + -rule 'Modified files must end with a newline' \ + -skip-if do_not_check_trailing_newline \ + -skip-reason 'disabled by configuration' \ + -fail-if has_missing_newline "$commit" \ + -fail-actual 'No newline at end of file' + + check \ + -rule 'Modified files must not contain trailing whitespace' \ + -skip-if do_not_check_trailing_whitespace \ + -skip-reason 'disabled by configuration' \ + -warn-if has_trailing_whitespace "$commit" \ + -warn-actual 'Trailing whitespace found' +} + 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 0 0' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: add new feature' \ @@ -428,7 +437,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 0 0' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: add new feature' \ @@ -440,9 +449,63 @@ define \ Signed-off-by: Good Author EOF +define \ + -test 'Content: Missing newline' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3 1 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: missing newline' \ + -content 'line1\nline2' \ + -body <<-'EOF' + This commit has a file missing a newline at the end. + + Signed-off-by: Good Author + EOF + +define \ + -test 'Content: Missing newline disabled' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3 3 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: missing newline disabled' \ + -content 'line1\nline2' \ + -check-trailing-newline 'false' \ + -body <<-'EOF' + This commit has a file missing a newline at the end, but check is disabled. + + Signed-off-by: Good Author + EOF + +define \ + -test 'Content: Trailing whitespace' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3 0 2' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: trailing whitespace' \ + -content 'line1 \nline2\n' \ + -body <<-'EOF' + This commit has a file with trailing whitespace. + + Signed-off-by: Good Author + EOF + +define \ + -test 'Content: Trailing whitespace disabled' \ + -expected '0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 3 3 0 3' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: trailing whitespace disabled' \ + -content 'line1 \nline2\n' \ + -check-trailing-whitespace 'false' \ + -body <<-'EOF' + This commit has a file with trailing whitespace, but check is disabled. + + Signed-off-by: Good Author + EOF + 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 0 0' \ -author 'Good Author' \ -email 'good.author@example.com' \ -subject 'package: add new feature' \ @@ -454,6 +517,58 @@ 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 0 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: signed commit' \ + -sign-key 'good' \ + -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 0 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: expired key signature' \ + -sign-key 'expired' \ + -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 0 0' \ + -author 'Good Author' \ + -email 'good.author@example.com' \ + -subject 'package: unknown key signature' \ + -sign-key 'unknown' \ + -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 0 0' \ + -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 + EOF + cleanup() { if [ -d "$REPO_DIR" ]; then [ -z "$PARALLEL_WORKER" ] && echo "Cleaning up temporary directory '$REPO_DIR'" @@ -468,12 +583,44 @@ commit() { local email="$2" local subject="$3" local body="$4" + local sign_key="${5:-}" + local content="${6:-}" - touch "file-$(date +%s-%N).txt" + local filename="file-$(date +%s-%N).txt" + if [ -n "$content" ]; then + printf "%b" "$content" > "$filename" + else + touch "$filename" + fi 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 [ -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() { @@ -509,12 +656,14 @@ run_test() { local body="$6" local merge="${7:-0}" local injection_file="${8:-}" + local sign_key="${9:-}" + local content="${10:-}" 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" "$content" >/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 @@ -612,6 +761,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" @@ -625,6 +803,13 @@ 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 @@ -634,6 +819,8 @@ run_worker() { export CHECK_SIGNOFF="${ENV_CHECK_SIGNOFF[$idx]}" export EXCLUDE_DEPENDABOT="${ENV_EXCLUDE_DEPENDABOT[$idx]}" export EXCLUDE_WEBLATE="${ENV_EXCLUDE_WEBLATE[$idx]}" + export CHECK_TRAILING_NEWLINE="${ENV_CHECK_TRAILING_NEWLINE[$idx]}" + export CHECK_TRAILING_WHITESPACE="${ENV_CHECK_TRAILING_WHITESPACE[$idx]}" export HEAD_BRANCH="${ENV_HEAD_BRANCH[$idx]}" export PARALLEL_WORKER=true @@ -646,7 +833,9 @@ run_worker() { "${SUBJECTS[$idx]}" \ "${BODIES[$idx]}" \ "${MERGES[$idx]}" \ - "${INJECTIONS[$idx]}") + "${INJECTIONS[$idx]}" \ + "${SIGN_KEYS[$idx]}" \ + "${CONTENTS[$idx]}") local res=$? echo "$output" > "$base_dir/$idx.log" @@ -737,7 +926,7 @@ main() { echo $'\nTest suite finished' echo "Summary: $pass_count/$test_count tests passed" - [ "$pass_count" != "$test_count" ] \ + [ "$pass_count" -ne "$test_count" ] \ && exit 1 \ || exit 0 }