diff --git a/.gitea/scripts/sop-tier-check.sh b/.gitea/scripts/sop-tier-check.sh index afd13e48b..d1bd2c235 100755 --- a/.gitea/scripts/sop-tier-check.sh +++ b/.gitea/scripts/sop-tier-check.sh @@ -114,6 +114,19 @@ if [ -z "$WHOAMI" ]; then fi echo "::notice::token resolves to user: $WHOAMI" +# 0.5 Read PR head SHA so we can reject stale approvals after head moves +# (internal#816). Reviews carry the commit_id they were submitted against. +HEAD_SHA=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}" | jq -r '.head.sha // ""') || true +if [ -z "$HEAD_SHA" ]; then + echo "::error::Failed to fetch PR head SHA — token may be invalid." + if [ "${SOP_FAIL_OPEN:-}" = "1" ]; then + echo "::warning::SOP_FAIL_OPEN=1 — exiting 0 so CI does not block." + exit 0 + fi + exit 1 +fi +debug "pr-head-sha=$HEAD_SHA" + # 1. Read tier label. || true ensures set -euo pipefail does not abort the # script if curl or jq fails (e.g. 401 from empty token). LABELS=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/issues/${PR_NUMBER}/labels" | jq -r '.[].name') || true @@ -265,7 +278,7 @@ if [ $_REVIEWS_EXIT -ne 0 ] || [ -z "$REVIEWS" ]; then fi exit 1 fi -APPROVERS=$(echo "$REVIEWS" | jq -r '[.[] | select(.state=="APPROVED") | .user.login] | unique | .[]') || true +APPROVERS=$(echo "$REVIEWS" | jq -r --arg head_sha "$HEAD_SHA" '[.[] | select(.state=="APPROVED" and .commit_id == $head_sha) | .user.login] | unique | .[]') || true if [ -z "$APPROVERS" ]; then echo "::error::No approving reviews on this PR. Set SOP_DEBUG=1 and re-run for diagnostics." exit 1 diff --git a/.gitea/scripts/tests/test_sop_tier_check_stale_reviews.sh b/.gitea/scripts/tests/test_sop_tier_check_stale_reviews.sh new file mode 100755 index 000000000..a5afe8663 --- /dev/null +++ b/.gitea/scripts/tests/test_sop_tier_check_stale_reviews.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Regression test for internal#816 — sop-tier-check must ignore APPROVED +# reviews that were submitted against an old PR head SHA. +# +# Bug: the script collected approvers with +# jq '[.[] | select(.state=="APPROVED") | .user.login]' +# without filtering on .commit_id == HEAD_SHA. After a PR head moved, +# stale approvals looked valid to the tier gate. +# +# Fix: the jq filter now includes +# select(.state=="APPROVED" and .commit_id == $head_sha) +# where $head_sha is the current PR head fetched from the API. + +set -euo pipefail + +# jq may not be on PATH in all environments (e.g. dev containers). +PATH="/tmp/bin:$PATH" +command -v jq >/dev/null 2>&1 || { echo "::error::jq required but not found"; exit 1; } + +PASS=0 +FAIL=0 + +assert_eq() { + local label="$1" + local expected="$2" + local got="$3" + if [ "$expected" = "$got" ]; then + echo " PASS $label" + PASS=$((PASS + 1)) + else + echo " FAIL $label" + echo " expected: <$expected>" + echo " got: <$got>" + FAIL=$((FAIL + 1)) + fi +} + +# Sample reviews matching the shape from Gitea API +REVIEWS_JSON='[ + {"state":"APPROVED","commit_id":"abc123","user":{"login":"bob"}}, + {"state":"APPROVED","commit_id":"old456","user":{"login":"alice"}}, + {"state":"COMMENT","commit_id":"abc123","user":{"login":"carol"}}, + {"state":"APPROVED","commit_id":"abc123","user":{"login":"dave"}}, + {"state":"REQUEST_CHANGES","commit_id":"abc123","user":{"login":"eve"}} +]' + +echo "test: jq filter keeps only APPROVED on current head" +GOT=$(echo "$REVIEWS_JSON" | jq -r --arg head_sha "abc123" \ + '[.[] | select(.state=="APPROVED" and .commit_id == $head_sha) | .user.login] | unique | .[]') +assert_eq "current-head approvers" "bob dave" "$(echo "$GOT" | tr '\n' ' ' | sed 's/ $//')" + +echo "test: jq filter with all-stale reviews yields empty" +GOT=$(echo "$REVIEWS_JSON" | jq -r --arg head_sha "new789" \ + '[.[] | select(.state=="APPROVED" and .commit_id == $head_sha) | .user.login] | unique | .[]') +assert_eq "all-stale yields empty" "" "$GOT" + +echo "test: jq filter handles null commit_id gracefully" +NULL_JSON='[{"state":"APPROVED","commit_id":null,"user":{"login":"mallory"}}]' +GOT=$(echo "$NULL_JSON" | jq -r --arg head_sha "abc123" \ + '[.[] | select(.state=="APPROVED" and .commit_id == $head_sha) | .user.login] | unique | .[]') +assert_eq "null commit_id excluded" "" "$GOT" + +echo +echo "------" +echo "PASS=$PASS FAIL=$FAIL" +[ "$FAIL" -eq 0 ]