diff --git a/.gitea/scripts/tests/test_gate_review_auto_fire.py b/.gitea/scripts/tests/test_gate_review_auto_fire.py index 72a650cc..d0545d95 100644 --- a/.gitea/scripts/tests/test_gate_review_auto_fire.py +++ b/.gitea/scripts/tests/test_gate_review_auto_fire.py @@ -50,14 +50,25 @@ class TestQaReviewDirectTrigger: "pull_request_review must include 'submitted' type" ) - def test_job_guard_requires_approved_state(self): + def test_job_guard_fires_on_pull_request_review(self): wf = load_workflow("qa-review.yml") guard = _job_guard_string(wf) - assert "github.event.review.state == 'APPROVED'" in guard, ( - "job guard must check review.state for 'APPROVED'" + assert "github.event_name == 'pull_request_review'" in guard, ( + "job guard must fire on pull_request_review events" ) - assert "github.event.review.state == 'approved'" in guard, ( - "job guard must check review.state for 'approved' (case fallback per #2135)" + # #2159: state guard removed because Gitea 1.22.6 does not reliably + # expose review.state in the pull_request_review payload. The + # evaluator (review-check.sh) validates APPROVED via API anyway. + assert "github.event.review.state" not in guard, ( + "job guard must NOT check review.state (unreliable in Gitea payload)" + ) + + def test_pull_request_target_includes_labeled(self): + wf = load_workflow("qa-review.yml") + on = wf[True] + types = on["pull_request_target"].get("types", []) + assert "labeled" in types, ( + "qa-review must re-run on labeled events so auto-tier flips the gate (#2396)" ) def test_post_step_uses_status_post_token(self): @@ -91,14 +102,25 @@ class TestSecurityReviewDirectTrigger: "pull_request_review must include 'submitted' type" ) - def test_job_guard_requires_approved_state(self): + def test_job_guard_fires_on_pull_request_review(self): wf = load_workflow("security-review.yml") guard = _job_guard_string(wf) - assert "github.event.review.state == 'APPROVED'" in guard, ( - "job guard must check review.state for 'APPROVED'" + assert "github.event_name == 'pull_request_review'" in guard, ( + "job guard must fire on pull_request_review events" ) - assert "github.event.review.state == 'approved'" in guard, ( - "job guard must check review.state for 'approved' (case fallback per #2135)" + # #2159: state guard removed because Gitea 1.22.6 does not reliably + # expose review.state in the pull_request_review payload. The + # evaluator (review-check.sh) validates APPROVED via API anyway. + assert "github.event.review.state" not in guard, ( + "job guard must NOT check review.state (unreliable in Gitea payload)" + ) + + def test_pull_request_target_includes_labeled(self): + wf = load_workflow("security-review.yml") + on = wf[True] + types = on["pull_request_target"].get("types", []) + assert "labeled" in types, ( + "security-review must re-run on labeled events so auto-tier flips the gate (#2396)" ) def test_post_step_uses_status_post_token(self): diff --git a/.gitea/scripts/tests/test_sop_auto_tier.sh b/.gitea/scripts/tests/test_sop_auto_tier.sh new file mode 100755 index 00000000..2cff342b --- /dev/null +++ b/.gitea/scripts/tests/test_sop_auto_tier.sh @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +# Regression test for #2396 — auto-tier assignment from diff heuristics. +# +# Validates the heuristic that computes tier:low|medium|high from a list of +# changed file paths. The heuristic must be conservative: high-risk paths +# (security, auth, crypto, secrets, migrations) force tier:high; test-only +# or doc-only changes default to tier:low; everything else is medium. + +set -euo pipefail + +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 +} + +# ----- Heuristic under test (mirrors sop-tier-check.sh auto-tier block) ----- +# Input: newline-separated filenames (no JSON/jq dependency for testability) +compute_tier() { + local files="$1" + local has_high_risk="" + local has_non_test="" + local file_count + file_count=$(echo "$files" | grep -c '^' || true) + if [ -z "$files" ] || [ "$file_count" -eq 0 ]; then + echo "tier:low" + return + fi + local fname + while IFS= read -r fname; do + [ -z "$fname" ] && continue + case "$fname" in + *security*|*auth*|*crypto*|*secret*|*migrations*|*/handlers/secrets*|*/handlers/approval_gate*|*/handlers/workspace_crud*) + has_high_risk="yes" + ;; + esac + case "$fname" in + # .gitea changes affect CI workflows, SOP gates, and merge policy + # for ALL PRs. Reviewer recommendation: treat as high-risk. + .gitea/*) + has_high_risk="yes" + ;; + esac + case "$fname" in + *_test.go|*/docs/*|*.md|*.txt|*.json|*.yaml|*.yml|*.css|*.html) + ;; + *) + has_non_test="yes" + ;; + esac + done <<<"$files" + + if [ -n "$has_high_risk" ]; then + echo "tier:high" + elif [ -z "$has_non_test" ]; then + echo "tier:low" + else + echo "tier:medium" + fi +} + +# Empty PR → low +echo "test: empty file list → tier:low" +assert_eq "empty" "tier:low" "$(compute_tier "")" + +# Test-only changes → low +echo "test: test-only changes → tier:low" +assert_eq "test-only" "tier:low" "$(compute_tier $'foo_test.go\nbar_test.go')" + +# Doc-only changes → low +echo "test: doc-only changes → tier:low" +assert_eq "docs-only" "tier:low" "$(compute_tier $'docs/README.md\nCHANGELOG.txt')" + +# Mixed test + non-test → medium +echo "test: mixed test + src → tier:medium" +assert_eq "mixed" "tier:medium" "$(compute_tier $'foo_test.go\nmain.go')" + +# Pure src changes → medium +echo "test: pure src changes → tier:medium" +assert_eq "src-only" "tier:medium" "$(compute_tier $'internal/handlers/workspace.go\ninternal/db/db.go')" + +# Security path → high +echo "test: security path → tier:high" +assert_eq "security" "tier:high" "$(compute_tier $'internal/handlers/secrets.go')" + +# Auth path → high +echo "test: auth path → tier:high" +assert_eq "auth" "tier:high" "$(compute_tier $'internal/auth/middleware.go')" + +# Crypto path → high +echo "test: crypto path → tier:high" +assert_eq "crypto" "tier:high" "$(compute_tier $'internal/crypto/encrypt.go')" + +# Migrations path → high +echo "test: migrations path → tier:high" +assert_eq "migrations" "tier:high" "$(compute_tier $'migrations/047_add_column.sql')" + +# Mixed with high-risk → high (overrides medium) +echo "test: mixed with high-risk → tier:high" +assert_eq "mixed-high" "tier:high" "$(compute_tier $'foo_test.go\ninternal/handlers/secrets.go')" + +# .gitea/workflows changes → high (gate-affecting, reviewer recommendation) +echo "test: .gitea/workflows change → tier:high" +assert_eq "gitea-workflows" "tier:high" "$(compute_tier $'.gitea/workflows/sop-tier-check.yml')" + +# .gitea/scripts changes → high (gate-affecting) +echo "test: .gitea/scripts change → tier:high" +assert_eq "gitea-scripts" "tier:high" "$(compute_tier $'.gitea/scripts/sop-tier-check.sh')" + +# .gitea change mixed with test → high (high-risk wins) +echo "test: .gitea/workflows + test → tier:high" +assert_eq "gitea-mixed" "tier:high" "$(compute_tier $'.gitea/workflows/qa-review.yml\nfoo_test.go')" + +# .gitea change with security → high (both high-risk) +echo "test: .gitea/workflows + security → tier:high" +assert_eq "gitea-security" "tier:high" "$(compute_tier $'.gitea/workflows/sop-tier-check.yml\ninternal/handlers/secrets.go')" + +# Fail-closed: TIER_EXPR must not contain placeholder teams (???) +# If present, the gate would silently skip clauses instead of failing. +echo "test: TIER_EXPR has no placeholder teams (fail-closed)" +# Extract the TIER_EXPR block from sop-tier-check.sh and grep for ??? +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +TIER_EXPR_BLOCK=$(sed -n '/declare -A TIER_EXPR=/,/^)/p' "$SCRIPT_DIR/../sop-tier-check.sh") +TIER_EXPR_PLACEHOLDERS=$(echo "$TIER_EXPR_BLOCK" | grep -c '???' || true) +if [ "$TIER_EXPR_PLACEHOLDERS" -eq 0 ]; then + echo " PASS no-placeholder-teams" + PASS=$((PASS + 1)) +else + echo " FAIL no-placeholder-teams" + echo " Found $TIER_EXPR_PLACEHOLDERS ??? placeholder(s) in TIER_EXPR" + FAIL=$((FAIL + 1)) +fi + +echo +echo "------" +echo "PASS=$PASS FAIL=$FAIL" +[ "$FAIL" -eq 0 ] diff --git a/.gitea/workflows/qa-review.yml b/.gitea/workflows/qa-review.yml index 51ff78cf..40f372d1 100644 --- a/.gitea/workflows/qa-review.yml +++ b/.gitea/workflows/qa-review.yml @@ -96,7 +96,7 @@ name: qa-review on: pull_request_target: - types: [opened, synchronize, reopened] + types: [opened, synchronize, reopened, labeled, unlabeled] pull_request_review: types: [submitted] @@ -110,13 +110,21 @@ jobs: approved: # Gate the job: # - On pull_request_target events: always run. - # - On pull_request_review_approved events: run so the gate flips - # immediately when a team member submits an APPROVE review. + # - On pull_request_review events: always run. We do NOT guard on + # review.state here because Gitea 1.22.6's payload shape for this + # event does not reliably expose the state field that the GitHub- + # style guard expects (issue #2159). The evaluator + # (review-check.sh) reads the actual reviews from the API and + # checks for a real APPROVE, so running on COMMENT or + # REQUEST_CHANGES reviews is harmless (read-only, idempotent). + # sop-tier-check.yml uses the same pattern (no state guard) and + # provably fires on every review event. + # - On labeled/unlabeled events: re-evaluate when tier label changes. + # This ensures qa-review flips when auto-tier assigns a label (#2396). # Comment-triggered refires live in sop-checklist.yml review-refire job. if: | github.event_name == 'pull_request_target' || - (github.event_name == 'pull_request_review' && - (github.event.review.state == 'APPROVED' || github.event.review.state == 'approved')) + github.event_name == 'pull_request_review' runs-on: ubuntu-latest steps: - name: Privilege check (A1.1 — INFORMATIONAL log only, NOT a gate) diff --git a/.gitea/workflows/security-review.yml b/.gitea/workflows/security-review.yml index c214cb1c..dc261802 100644 --- a/.gitea/workflows/security-review.yml +++ b/.gitea/workflows/security-review.yml @@ -23,7 +23,7 @@ name: security-review on: pull_request_target: - types: [opened, synchronize, reopened] + types: [opened, synchronize, reopened, labeled, unlabeled] pull_request_review: types: [submitted] @@ -37,13 +37,21 @@ jobs: approved: # Gate the job: # - On pull_request_target events: always run. - # - On pull_request_review_approved events: run so the gate flips - # immediately when a team member submits an APPROVE review. + # - On pull_request_review events: always run. We do NOT guard on + # review.state here because Gitea 1.22.6's payload shape for this + # event does not reliably expose the state field that the GitHub- + # style guard expects (issue #2159). The evaluator + # (review-check.sh) reads the actual reviews from the API and + # checks for a real APPROVE, so running on COMMENT or + # REQUEST_CHANGES reviews is harmless (read-only, idempotent). + # sop-tier-check.yml uses the same pattern (no state guard) and + # provably fires on every review event. + # - On labeled/unlabeled events: re-evaluate when tier label changes. + # This ensures security-review flips when auto-tier assigns a label (#2396). # Comment-triggered refires live in sop-checklist.yml review-refire job. if: | github.event_name == 'pull_request_target' || - (github.event_name == 'pull_request_review' && - (github.event.review.state == 'APPROVED' || github.event.review.state == 'approved')) + github.event_name == 'pull_request_review' runs-on: ubuntu-latest steps: - name: Privilege check (A1.1 — INFORMATIONAL log only, NOT a gate)