fix(audit-force-merge): fail-closed on HTTP errors from Gitea API #2366
@@ -54,32 +54,57 @@ API="https://${GITEA_HOST}/api/v1"
|
||||
AUTH="Authorization: token ${GITEA_TOKEN}"
|
||||
|
||||
# 1. Fetch the PR. If not merged, no-op.
|
||||
PR=$(curl -sS -H "$AUTH" "${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}")
|
||||
MERGED=$(echo "$PR" | jq -r '.merged // false')
|
||||
# Fail-closed: verify HTTP 200 before parsing. A 401/403/404 means the token
|
||||
# is invalid or the PR is inaccessible — we must NOT silently treat that as
|
||||
# "not merged" and skip the audit.
|
||||
PR_TMP=$(mktemp)
|
||||
PR_HTTP=$(curl -sS -o "$PR_TMP" -w '%{http_code}' -H "$AUTH" \
|
||||
"${API}/repos/${OWNER}/${NAME}/pulls/${PR_NUMBER}")
|
||||
PR=$(cat "$PR_TMP")
|
||||
rm -f "$PR_TMP"
|
||||
if [ "$PR_HTTP" != "200" ]; then
|
||||
echo "::error::GET /pulls/${PR_NUMBER} returned HTTP ${PR_HTTP} — cannot evaluate merge state."
|
||||
exit 1
|
||||
fi
|
||||
# FAIL-CLOSED: a 200 response with a missing/malformed `merged` field must
|
||||
# NOT be treated as "not merged" (that would silently skip the audit).
|
||||
# We verify both presence AND correct type for every field we consume.
|
||||
PR_SCHEMA_OK=$(echo "$PR" | jq -r '
|
||||
(.merged | type == "boolean") and
|
||||
(.merge_commit_sha | type == "string") and
|
||||
(.merged_by | type == "object") and (.merged_by.login | type == "string") and
|
||||
(.base | type == "object") and (.base.ref | type == "string") and
|
||||
(.head | type == "object") and (.head.sha | type == "string")
|
||||
')
|
||||
if [ "$PR_SCHEMA_OK" != "true" ]; then
|
||||
echo "::error::GET /pulls/${PR_NUMBER} returned HTTP 200 but one or more required fields are missing, null, or of wrong type — cannot evaluate force-merge."
|
||||
exit 1
|
||||
fi
|
||||
MERGED=$(echo "$PR" | jq -r '.merged')
|
||||
if [ "$MERGED" != "true" ]; then
|
||||
echo "::notice::PR #${PR_NUMBER} closed without merge — no audit emission."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# NOTE: no || true — with set -euo pipefail, jq parse failures (e.g. field
|
||||
# missing from API response) propagate as hard errors. Use jq's // operator
|
||||
# for graceful defaults instead of bash || true guards. This was re-added by
|
||||
# 8c343e3a ("fix(gitea): add || true guards to jq pipelines") — reverted
|
||||
# here because the guards mask silent failures that hide malformed API responses.
|
||||
MERGE_SHA=$(echo "$PR" | jq -r '.merge_commit_sha // empty')
|
||||
MERGED_BY=$(echo "$PR" | jq -r '.merged_by.login // "unknown"')
|
||||
MERGE_SHA=$(echo "$PR" | jq -r '.merge_commit_sha')
|
||||
MERGED_BY=$(echo "$PR" | jq -r '.merged_by.login')
|
||||
TITLE=$(echo "$PR" | jq -r '.title // ""')
|
||||
BASE_BRANCH=$(echo "$PR" | jq -r '.base.ref // "main"')
|
||||
HEAD_SHA=$(echo "$PR" | jq -r '.head.sha // empty')
|
||||
|
||||
if [ -z "$MERGE_SHA" ]; then
|
||||
echo "::warning::PR #${PR_NUMBER} merged=true but no merge_commit_sha — cannot evaluate force-merge."
|
||||
exit 0
|
||||
fi
|
||||
BASE_BRANCH=$(echo "$PR" | jq -r '.base.ref')
|
||||
HEAD_SHA=$(echo "$PR" | jq -r '.head.sha')
|
||||
|
||||
# 2. Required status checks — branch-aware JSON dict takes precedence.
|
||||
if [ -n "${REQUIRED_CHECKS_JSON:-}" ]; then
|
||||
REQUIRED=$(echo "$REQUIRED_CHECKS_JSON" | jq -r --arg branch "$BASE_BRANCH" '.[$branch] // [] | .[]')
|
||||
# FAIL-CLOSED: if REQUIRED_CHECKS_JSON is set, the branch entry must exist
|
||||
# and be an array. A missing branch or non-array value means the config is
|
||||
# malformed or drifted — we must NOT silently treat it as "no checks".
|
||||
_RC_JSON_OK=$(echo "$REQUIRED_CHECKS_JSON" | jq -r --arg branch "$BASE_BRANCH" '
|
||||
has($branch) and (.[$branch] | type == "array")
|
||||
')
|
||||
if [ "$_RC_JSON_OK" != "true" ]; then
|
||||
echo "::error::REQUIRED_CHECKS_JSON missing or non-array entry for branch '$BASE_BRANCH' — cannot evaluate required checks."
|
||||
exit 1
|
||||
fi
|
||||
REQUIRED=$(echo "$REQUIRED_CHECKS_JSON" | jq -r --arg branch "$BASE_BRANCH" '.[$branch] | .[]')
|
||||
else
|
||||
REQUIRED="$REQUIRED_CHECKS"
|
||||
fi
|
||||
@@ -91,12 +116,28 @@ fi
|
||||
# 3. Status-check state at the PR HEAD (where checks ran). The merge
|
||||
# commit doesn't get its own checks; we evaluate the PR's last
|
||||
# commit, which is what branch protection compared against.
|
||||
STATUS=$(curl -sS -H "$AUTH" \
|
||||
# Fail-closed: verify HTTP 200. A 401/403/404 means the status is
|
||||
# unreadable — we must NOT treat that as "no statuses" and skip checks.
|
||||
STATUS_TMP=$(mktemp)
|
||||
STATUS_HTTP=$(curl -sS -o "$STATUS_TMP" -w '%{http_code}' -H "$AUTH" \
|
||||
"${API}/repos/${OWNER}/${NAME}/commits/${HEAD_SHA}/status")
|
||||
STATUS=$(cat "$STATUS_TMP")
|
||||
rm -f "$STATUS_TMP"
|
||||
if [ "$STATUS_HTTP" != "200" ]; then
|
||||
echo "::error::GET /commits/${HEAD_SHA}/status returned HTTP ${STATUS_HTTP} — cannot evaluate required checks."
|
||||
exit 1
|
||||
fi
|
||||
# FAIL-CLOSED: a 200 status response missing the 'statuses' array, or with
|
||||
# 'statuses' set to a non-array type (null/string/object), must NOT be treated
|
||||
# as "no checks" — that would silently declare all checks green.
|
||||
if ! echo "$STATUS" | jq -e '(.statuses | type) == "array"' >/dev/null; then
|
||||
echo "::error::GET /commits/${HEAD_SHA}/status returned HTTP 200 but 'statuses' is missing or not an array — cannot evaluate required checks."
|
||||
exit 1
|
||||
fi
|
||||
declare -A CHECK_STATE
|
||||
while IFS=$'\t' read -r ctx state; do
|
||||
[ -n "$ctx" ] && CHECK_STATE[$ctx]="$state"
|
||||
done < <(echo "$STATUS" | jq -r '.statuses // [] | .[] | "\(.context)\t\(.status)"')
|
||||
done < <(echo "$STATUS" | jq -r '.statuses | .[] | "\(.context)\t\(.status)"')
|
||||
|
||||
# 4. For each required check, was it green at merge? YAML block scalars
|
||||
# (`|`) leave a trailing newline; skip blank/whitespace-only lines.
|
||||
|
||||
+119
@@ -0,0 +1,119 @@
|
||||
#!/usr/bin/env bash
|
||||
# test_audit_force_merge.sh — regression lock for audit-force-merge fail-closed
|
||||
# behavior. Verifies every schema validation path via direct jq filter tests.
|
||||
#
|
||||
# Usage: bash test_audit_force_merge.sh
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
fail() { echo "FAIL: $*" >&2; exit 1; }
|
||||
pass() { echo "PASS: $*"; }
|
||||
|
||||
[ -x "$(command -v jq)" ] || { echo "SKIP: jq not on PATH"; exit 0; }
|
||||
|
||||
HEAD_SHA="deadbeef00000000000000000000000000000000"
|
||||
|
||||
# The schema validation jq expression from audit-force-merge.sh.
|
||||
validate_pr_schema() {
|
||||
jq -r '
|
||||
(.merged | type == "boolean") and
|
||||
(.merge_commit_sha | type == "string") and
|
||||
(.merged_by | type == "object") and (.merged_by.login | type == "string") and
|
||||
(.base | type == "object") and (.base.ref | type == "string") and
|
||||
(.head | type == "object") and (.head.sha | type == "string")
|
||||
'
|
||||
}
|
||||
|
||||
validate_statuses_type() {
|
||||
jq -r '(.statuses | type) == "array"'
|
||||
}
|
||||
|
||||
# T1 — valid PR payload → true
|
||||
T1=$(echo '{"merged":true,"merge_commit_sha":"abc","merged_by":{"login":"u"},"base":{"ref":"main"},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T1" = "true" ] || fail "T1: valid payload should pass schema"
|
||||
pass "T1: valid payload passes schema"
|
||||
|
||||
# T2 — merged=false (valid types) → true (schema is about types, not values)
|
||||
T2=$(echo '{"merged":false,"merge_commit_sha":"abc","merged_by":{"login":"u"},"base":{"ref":"main"},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T2" = "true" ] || fail "T2: merged=false with valid types should pass schema"
|
||||
pass "T2: merged=false with valid types passes schema"
|
||||
|
||||
# T3 — missing merged field → false
|
||||
T3=$(echo '{"merge_commit_sha":"abc","merged_by":{"login":"u"},"base":{"ref":"main"},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T3" = "false" ] || fail "T3: missing merged should fail schema"
|
||||
pass "T3: missing merged fails schema"
|
||||
|
||||
# T4 — merged is string "true" instead of boolean → false
|
||||
T4=$(echo '{"merged":"true","merge_commit_sha":"abc","merged_by":{"login":"u"},"base":{"ref":"main"},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T4" = "false" ] || fail "T4: merged as string should fail schema"
|
||||
pass "T4: merged as string fails schema"
|
||||
|
||||
# T5 — merge_commit_sha is null → false
|
||||
T5=$(echo '{"merged":true,"merge_commit_sha":null,"merged_by":{"login":"u"},"base":{"ref":"main"},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T5" = "false" ] || fail "T5: null merge_commit_sha should fail schema"
|
||||
pass "T5: null merge_commit_sha fails schema"
|
||||
|
||||
# T6 — merged_by is null → false
|
||||
T6=$(echo '{"merged":true,"merge_commit_sha":"abc","merged_by":null,"base":{"ref":"main"},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T6" = "false" ] || fail "T6: null merged_by should fail schema"
|
||||
pass "T6: null merged_by fails schema"
|
||||
|
||||
# T7 — base.ref is number → false
|
||||
T7=$(echo '{"merged":true,"merge_commit_sha":"abc","merged_by":{"login":"u"},"base":{"ref":123},"head":{"sha":"def"}}' | validate_pr_schema)
|
||||
[ "$T7" = "false" ] || fail "T7: numeric base.ref should fail schema"
|
||||
pass "T7: numeric base.ref fails schema"
|
||||
|
||||
# T8 — head is missing → false
|
||||
T8=$(echo '{"merged":true,"merge_commit_sha":"abc","merged_by":{"login":"u"},"base":{"ref":"main"}}' | validate_pr_schema)
|
||||
[ "$T8" = "false" ] || fail "T8: missing head should fail schema"
|
||||
pass "T8: missing head fails schema"
|
||||
|
||||
# T9 — statuses missing → false
|
||||
T9=$(echo '{}' | validate_statuses_type)
|
||||
[ "$T9" = "false" ] || fail "T9: missing statuses should fail type check"
|
||||
pass "T9: missing statuses fails type check"
|
||||
|
||||
# T10 — statuses is string → false
|
||||
T10=$(echo '{"statuses":"unexpected"}' | validate_statuses_type)
|
||||
[ "$T10" = "false" ] || fail "T10: string statuses should fail type check"
|
||||
pass "T10: string statuses fails type check"
|
||||
|
||||
# T11 — statuses is null → false
|
||||
T11=$(echo '{"statuses":null}' | validate_statuses_type)
|
||||
[ "$T11" = "false" ] || fail "T11: null statuses should fail type check"
|
||||
pass "T11: null statuses fails type check"
|
||||
|
||||
# T12 — statuses is array → true
|
||||
T12=$(echo '{"statuses":[{"context":"c1","status":"success"}]}' | validate_statuses_type)
|
||||
[ "$T12" = "true" ] || fail "T12: array statuses should pass type check"
|
||||
pass "T12: array statuses passes type check"
|
||||
|
||||
# T13 — empty array statuses → true
|
||||
T13=$(echo '{"statuses":[]}' | validate_statuses_type)
|
||||
[ "$T13" = "true" ] || fail "T13: empty array statuses should pass type check"
|
||||
pass "T13: empty array statuses passes type check"
|
||||
|
||||
# T14-T16: REQUIRED_CHECKS_JSON branch entry validation
|
||||
validate_required_checks_json() {
|
||||
local branch="$1"
|
||||
local json="$2"
|
||||
echo "$json" | jq -r --arg branch "$branch" 'has($branch) and (.[$branch] | type == "array")'
|
||||
}
|
||||
|
||||
# T14 — branch exists and is array → true
|
||||
T14=$(validate_required_checks_json "main" '{"main":["CI / all-required"]}')
|
||||
[ "$T14" = "true" ] || fail "T14: existing array branch should pass"
|
||||
pass "T14: existing array branch passes"
|
||||
|
||||
# T15 — branch missing → false
|
||||
T15=$(validate_required_checks_json "staging" '{"main":["CI / all-required"]}')
|
||||
[ "$T15" = "false" ] || fail "T15: missing branch should fail"
|
||||
pass "T15: missing branch fails"
|
||||
|
||||
# T16 — branch entry is string instead of array → false
|
||||
T16=$(validate_required_checks_json "main" '{"main":"CI / all-required"}')
|
||||
[ "$T16" = "false" ] || fail "T16: string branch entry should fail"
|
||||
pass "T16: string branch entry fails"
|
||||
|
||||
echo
|
||||
echo "ALL AUDIT-FORCE-MERGE CHECKS PASSED"
|
||||
Reference in New Issue
Block a user