diff --git a/repo-config/configure.sh b/repo-config/configure.sh index 86a88d45..4ee3be92 100755 --- a/repo-config/configure.sh +++ b/repo-config/configure.sh @@ -17,9 +17,13 @@ # The develop ruleset is develop.json where the model is PR-gated, or operational/develop.json for direct signed pushes. # Applying the same configuration twice changes nothing, so the mode is idempotent. # -# The check mode is the read-only inverse, where every applied ruleset, setting, and security feature must match. +# The check mode is the read-only inverse, and what it verifies is exactly what the payloads declare. # The ruleset and settings assertions are driven by the committed payloads, so they stay repo-agnostic. -# That also survives the GitHub API normalizing a stored ruleset, comparing rule presence, merge methods and required checks rather than a byte diff. +# A ruleset is checked on enforcement, on the rule-type set compared in both directions, and on the whole parameters object of every parameterized rule. +# Comparing the parameters object rather than named fields means a parameter added to a payload is audited with no change here. +# Both directions matter, since a rule added live that the payload never declared is drift this catches. +# That still survives the GitHub API normalizing a stored ruleset, since the comparison is over parsed JSON with sorted keys rather than a byte diff. +# What it does not verify is anything the payloads do not declare, so a setting absent from settings.json is unaudited by construction. # Secrets are per-repo (see spec/secrets.json) and not checkable from a standalone carry, so they are a manual-verify note. set -Eeuo pipefail @@ -185,7 +189,7 @@ jq_has() { jq -e "$@" >/dev/null 2>&1; } gh_ok() { gh api "$@" >/dev/null 2>&1; } check_ruleset() { # payload-file - the live ruleset must match the committed policy, driven by the payload - local file="$1" rname id live t want got wantc gotc want_enf + local file="$1" rname id live t want got want_enf if [ ! -e "$file" ]; then fail "ruleset payload $file missing"; return; fi rname="$(jq -r '.name // empty' "$file")" if [ -z "$rname" ]; then fail "ruleset payload $file has no name"; return; fi @@ -194,23 +198,35 @@ check_ruleset() { # payload-file - the live ruleset must match the committed pol if ! live="$(gh api "repos/$repo/rulesets/$id")"; then fail "ruleset '$rname' - could not read live state"; return; fi want_enf="$(jq -r '.enforcement' "$file")" assert "ruleset '$rname' enforcement = $want_enf" test "$(jq -r '.enforcement' <<<"$live")" = "$want_enf" - # Every rule type the committed payload declares must be present live (payload-driven, so repo-agnostic). + # The live rule-type set must equal the payload's, compared in both directions. + # Checking only that each payload type is present live misses a rule someone added by hand. + # That is drift this script exists to catch, and it passed as clean before. + local want_types got_types + if ! want_types="$(jq -r '[.rules[].type] | sort | join(",")' "$file")"; then + fail "ruleset payload $file did not parse"; return + fi + if [ -z "$want_types" ]; then fail "ruleset payload $file declares no rules"; return; fi + got_types="$(jq -r '[.rules[].type] | sort | join(",")' <<<"$live")" + assert "'$rname' rule set = $want_types" test "$got_types" = "$want_types" + # Every parameterized rule is compared on its whole parameters object rather than on selected fields. + # Naming fields one at a time meant a payload could declare a parameter the check never read. + # Review-thread resolution, stale-review dismissal, and the status-check policy flags all went unverified that way. + # Comparing the object keeps the check payload-driven: a parameter added to a payload is audited with no code change. + # Keys are sorted on both sides, so key order from the API cannot read as drift. + # Set-like arrays are sorted too, since the API guarantees no order and the previous per-field comparison sorted them explicitly. + # Dropping that would turn array order into false drift. + # A scalar array sorts directly, and required_status_checks sorts by context, its identifying field. + local ptypes norm + norm='def n: walk(if type=="array" then (if length==0 then . elif (all(.[]; type=="string" or type=="number")) then sort elif (all(.[]; type=="object" and has("context"))) then sort_by(.context) else . end) else . end); n' + ptypes="$(jq -r '[.rules[] | select(has("parameters")) | .type] | .[]' "$file")" while IFS= read -r t; do + [ -z "$t" ] && continue # shellcheck disable=SC2016 # $t is a jq --arg variable, not a shell expansion - assert "'$rname' enforces rule '$t'" jq_has --arg t "$t" '.rules[] | select(.type==$t)' <<<"$live" - done < <(jq -r '.rules[].type' "$file") - # For pull_request, the live merge methods must match the payload, which is the develop=squash and main=merge policy. - if jq_has '.rules[] | select(.type=="pull_request")' "$file"; then - want="$(jq -c '[.rules[]|select(.type=="pull_request").parameters.allowed_merge_methods[]]|sort' "$file")" - got="$(jq -c '[.rules[]|select(.type=="pull_request").parameters.allowed_merge_methods[]]|sort' <<<"$live")" - assert "'$rname' merge methods = $want" test "$got" = "$want" - fi - # For required_status_checks, the live required contexts must match the payload. - if jq_has '.rules[] | select(.type=="required_status_checks")' "$file"; then - wantc="$(jq -c '[.rules[]|select(.type=="required_status_checks").parameters.required_status_checks[].context]|sort' "$file")" - gotc="$(jq -c '[.rules[]|select(.type=="required_status_checks").parameters.required_status_checks[].context]|sort' <<<"$live")" - assert "'$rname' required checks = $wantc" test "$gotc" = "$wantc" - fi + want="$(jq -S -c --arg t "$t" "[.rules[] | select(.type==\$t) | .parameters] | first | $norm" "$file")" + # shellcheck disable=SC2016 + got="$(jq -S -c --arg t "$t" "[.rules[] | select(.type==\$t) | .parameters] | first | $norm" <<<"$live")" + assert "'$rname' rule '$t' parameters match the payload" test "$got" = "$want" + done <<<"$ptypes" } check_settings() { @@ -219,11 +235,20 @@ check_settings() { if ! live="$(gh api "repos/$repo")"; then fail "could not read repository settings"; return; fi # Static settings are driven from settings.json, so the check never drifts from the file. # Add a key there and it is audited here automatically. + # The payload is parsed into a variable before the loop rather than streamed from a process substitution. + # A jq failure inside `done < <(...)` leaves the loop body unexecuted without tripping set -e. + # Every static setting would then report as checked and passing while nothing was compared, a false clean. + local pairs + if ! pairs="$(jq -r 'to_entries[] | "\(.key)\t\(.value)"' "$settings_file")"; then + fail "settings payload $settings_file did not parse"; return + fi + # A payload that parses to nothing is a floor failure rather than a clean run, so it is asserted. + if [ -z "$pairs" ]; then fail "settings payload $settings_file declares no keys"; return; fi while IFS=$'\t' read -r key want; do # shellcheck disable=SC2016 # $k is a jq --arg variable, not a shell expansion got="$(jq -r --arg k "$key" '.[$k]' <<<"$live")" assert "setting $key = $want" test "$got" = "$want" - done < <(jq -r 'to_entries[] | "\(.key)\t\(.value)"' "$settings_file") + done <<<"$pairs" # Dynamic settings apply sets: has_discussions (public repos only), default_branch (main, if it exists). private="$(jq -r '.private' <<<"$live")" wantdisc=true; [ "$private" = "true" ] && wantdisc=false