diff --git a/.github/workflows/pr-gate.yml b/.github/workflows/pr-gate.yml index 54fedfb..e369db6 100644 --- a/.github/workflows/pr-gate.yml +++ b/.github/workflows/pr-gate.yml @@ -40,136 +40,95 @@ jobs: id: authz shell: bash env: + # Everything the decision reads comes from the event payload and is + # passed through env:, never interpolated into the script body. + EVENT_NAME: ${{ github.event_name }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} + PR_ASSOCIATION: ${{ github.event.pull_request.author_association }} run: | set -uo pipefail # ============================================================== - # WHY THIS USES THE RUNNER'S AMBIENT ORG AUTH + # DECISION COMES FROM THE EVENT PAYLOAD ONLY (aprender#5951). # - # Two different APIs have now been caught giving this gate a - # confidently wrong answer, both for the same underlying reason: - # the default GITHUB_TOKEN cannot see PRIVATE org memberships, - # and every paiml owner's membership is private. + # Operator: "the fix belongs in the job, not on gx10" and "have + # authorize read author_association from the event". Standing + # rule: no workflow may use credentials that live on a host. + # This step therefore makes NO API call and never invokes `gh`: + # the earlier membership probe depended on the runner's ambient + # `gh` login, and a runner without one (gx10-blackwell) turned + # every PR red with "indeterminate". # - # 1. `orgs.checkMembershipForUser` with GITHUB_TOKEN answers 404 - # for a private member -- byte-for-byte identical to "not a - # member". This closed @noahgift's PRs until he was - # allowlisted by name (#12), then closed @alfredodeza's - # paiml/infra#340 twice while `gate` and `validate` were - # green on the same commit. + # Known limitation, accepted: GitHub reports a PRIVATE org + # member's association as COLLABORATOR rather than MEMBER. That + # is why COLLABORATOR is on the allow-list. The cost is that an + # outside collaborator is indistinguishable from a private + # member; collaborators already hold repo access granted by an + # owner, so this does not widen who can open PRs. # - # 2. `author_association` from the payload looked like the fix - # -- the REST API reports MEMBER for those same PRs. It is - # NOT the fix. MEASURED on the live webhook for #340: - # author_association=COLLABORATOR. GitHub downgrades the - # payload's association for private members too, so a real - # org owner is indistinguishable from an outside - # collaborator. That attempt (#54) is what this replaces. - # - # The runner's ambient `gh` auth holds admin:org and therefore - # sees the truth. This is the same decision, for the same reason, - # already taken by lane-liveness.sh (CLAUDE.md: "Uses the - # runner's ambient org gh auth -- never github.token"). - # - # Safe here: this job has exactly ONE input it does not control, - # a login string, and there is NO actions/checkout anywhere in - # this workflow. No PR-authored code is fetched or run. + # FAIL-CLOSED: anything not on the allow-list -- including an + # empty, missing or unknown value -- fails. A missing value never + # passes on a PR event. + # Allow-list: OWNER MEMBER COLLABORATOR. # ============================================================== - ORG=paiml + # Strip anything that could forge a workflow command before echoing. + clean() { printf '%s' "$1" | tr -c 'A-Za-z0-9_.-' '?' | head -c 64; } - # Deliberately NOT the workflow token: that token is the blind - # instrument this whole file exists to route around. - unset GITHUB_TOKEN GH_TOKEN - - # Returns: member | notmember | indeterminate - # `indeterminate` is a THIRD answer and must never collapse into - # `notmember` -- conflating "they are not a member" with "I could - # not ask" is the exact defect that closed two owners' PRs. - membership() { - local user="$1" resp rc state - resp=$(gh api "orgs/${ORG}/memberships/${user}" 2>&1) && rc=0 || rc=$? - if [ "$rc" -eq 0 ]; then - state=$(printf '%s' "$resp" | jq -r '.state // "unknown"') - # "pending" = invited, not accepted. Not a member yet. - if [ "$state" = "active" ]; then echo member; else echo notmember; fi - return 0 - fi - case "$resp" in - *"HTTP 404"*) echo notmember ;; - *) echo indeterminate ;; - esac + allow() { + echo "allow=true" >> "$GITHUB_OUTPUT" + echo "reason=$1" >> "$GITHUB_OUTPUT" + echo "${2:-$PR_AUTHOR}: $1 -- ALLOWED" } - # -------------------------------------------------------------- - # PROBE THE INSTRUMENT BEFORE TRUSTING ITS VERDICT. - # A membership check that cannot see members reports everyone as - # an outsider, and that is precisely how this gate failed twice. - # So it must answer a known member and a known non-member - # correctly, right now, on this runner, before it is allowed to - # judge anybody. - # -------------------------------------------------------------- - PROBE_MEMBER=noahgift # org owner, private membership - PROBE_STRANGER=paiml-pr-gate-sentinel-no-such-user - - probe_pos=$(membership "$PROBE_MEMBER") - probe_neg=$(membership "$PROBE_STRANGER") - echo "instrument probe: ${PROBE_MEMBER}=${probe_pos} (want member), ${PROBE_STRANGER}=${probe_neg} (want notmember)" - - if [ "$probe_pos" != "member" ] || [ "$probe_neg" != "notmember" ]; then - echo "::error::pr-gate instrument probe FAILED (${PROBE_MEMBER}=${probe_pos}, ${PROBE_STRANGER}=${probe_neg}) -- refusing to judge anyone with a check that cannot demonstrate both answers" - # Fail RED and leave the PR OPEN. The harm is asymmetric and - # measured: auto-closing a legitimate owner's PR has happened - # repeatedly and is invisible to them, while an unauthorized PR - # left open under a red required check is visible to every - # human on the repo. - exit 1 - fi + case "${EVENT_NAME:-}" in + pull_request_target|pull_request) ;; + merge_group|push) + # No PR author exists on these events. They are reachable only + # by someone who already passed this gate on the PR (merge + # queue entry / push to a protected branch need write access), + # and the Close step below acts on a PR that does not exist + # here. Allowing is safe; documented decision, not a default. + allow "no PR author on ${EVENT_NAME:-} event (already gated at PR time)" "${EVENT_NAME:-}" + exit 0 ;; + *) + echo "::error::pr-gate authorize: unsupported event '$(clean "${EVENT_NAME:-}")' -- refusing (fail-closed)" + exit 1 ;; + esac - # -------------------------------------------------------------- - # Decision. - # -------------------------------------------------------------- - # Explicitly allowlisted EXTERNAL contributors only. Org members - # are authorized structurally, below -- adding one here is how - # this gate hid its own blindness for four months. - case "$PR_AUTHOR" in - guyernest|spudnic) # guyernest: pmcp lead developer; spudnic: authorized contributor (operator 2026-09-26) -- outside collaborators, not org members - echo "allow=true" >> "$GITHUB_OUTPUT" - echo "reason=allowlisted external contributor" >> "$GITHUB_OUTPUT" - echo "$PR_AUTHOR: allowlisted external contributor -- ALLOWED" + # Explicitly allowlisted EXTERNAL contributors (outside collaborators). + case "${PR_AUTHOR:-}" in + guyernest|spudnic) # guyernest: pmcp lead developer; spudnic: authorized contributor (operator 2026-09-26) + allow "allowlisted external contributor" exit 0 ;; esac # Bots (dependabot, renovate, actions-user) are always allowed. - if [ "$PR_AUTHOR_TYPE" = "Bot" ]; then - echo "allow=true" >> "$GITHUB_OUTPUT" - echo "reason=bot" >> "$GITHUB_OUTPUT" - echo "$PR_AUTHOR: bot -- ALLOWED" + if [ "${PR_AUTHOR_TYPE:-}" = "Bot" ]; then + allow "bot" exit 0 fi - author_state=$(membership "$PR_AUTHOR") - echo "author=${PR_AUTHOR} type=${PR_AUTHOR_TYPE} membership=${author_state}" - - case "$author_state" in - member) - echo "allow=true" >> "$GITHUB_OUTPUT" - echo "reason=active ${ORG} org member" >> "$GITHUB_OUTPUT" - echo "$PR_AUTHOR: active ${ORG} org member -- ALLOWED" ;; - notmember) + case "${PR_ASSOCIATION:-}" in + OWNER|MEMBER|COLLABORATOR) + allow "author_association=${PR_ASSOCIATION}" + exit 0 ;; + CONTRIBUTOR|FIRST_TIMER|FIRST_TIME_CONTRIBUTOR|MANNEQUIN|NONE) echo "allow=false" >> "$GITHUB_OUTPUT" - echo "reason=not an active ${ORG} org member and not allowlisted" >> "$GITHUB_OUTPUT" - echo "$PR_AUTHOR: not a member -- REJECTED" ;; + echo "reason=author_association=${PR_ASSOCIATION} is not OWNER/MEMBER/COLLABORATOR and author is not allowlisted" >> "$GITHUB_OUTPUT" + echo "::error::pr-gate: author_association=${PR_ASSOCIATION} for $(clean "${PR_AUTHOR:-}") -- REJECTED" + exit 1 ;; *) - # Could not ask. Not a verdict. Never close on this. - echo "::error::could not determine ${ORG} membership for ${PR_AUTHOR} -- leaving the PR open for a human" + # Empty, missing or unknown: could not establish who this is. + # Fail red, leave the PR open for a human (no allow=false, so + # the Close step does not run). + echo "::error::pr-gate: author_association='$(clean "${PR_ASSOCIATION:-}")' is empty or unknown -- refusing (fail-closed)" exit 1 ;; esac - name: Close unauthorized PR - if: steps.authz.outputs.allow != 'true' + if: ${{ !cancelled() && steps.authz.outputs.allow == 'false' }} uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 env: REASON: ${{ steps.authz.outputs.reason }} diff --git a/tests/pr-gate-authorize-test.sh b/tests/pr-gate-authorize-test.sh new file mode 100755 index 0000000..4ab7b85 --- /dev/null +++ b/tests/pr-gate-authorize-test.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# Test for the `authorize` step of .github/workflows/pr-gate.yml (aprender#5951). +# Extracts the REAL step script and runs it with event env vars. `gh` is a stub +# that exits 99 and records every call: the step must make zero calls. +# Mutants (must go RED): PRGATE_MUTATE=allownone (NONE on allow-list), +# PRGATE_MUTATE=failopen (fail-closed branches become passes). +set -uo pipefail +cd "$(dirname "$0")/.." +WF=.github/workflows/pr-gate.yml +T=$(mktemp -d); trap 'rm -rf "${T:?}"' EXIT + +awk '/id: authz/{a=1} a&&/run: \|/{r=1;next} r&&/^ - name:/{exit} r{sub(/^ /,"");print}' "$WF" > "$T/step.sh" +[ -s "$T/step.sh" ] || { echo "FAIL: could not extract step"; exit 2; } +case "${PRGATE_MUTATE:-}" in + allownone) sed -i 's/OWNER|MEMBER|COLLABORATOR)/OWNER|MEMBER|COLLABORATOR|NONE)/' "$T/step.sh" ;; + failopen) sed -i 's/^\( *\)exit 1 ;;$/\1exit 0 ;;/; s/^\( *\)exit 1$/\1exit 0/' "$T/step.sh" ;; +esac + +mkdir -p "$T/bin" +cat > "$T/bin/gh" <<'STUB' +#!/usr/bin/env bash +echo "gh $*" >> "$GH_CALLS"; exit 99 +STUB +chmod +x "$T/bin/gh"; export GH_CALLS="$T/gh.calls" + +pass=0; fail=0 +# run NAME WANT_RC WANT_ALLOW [VAR=VAL ...] (env cleared of event vars first) +run() { + local name=$1 want_rc=$2 want_allow=$3 out rc allow; shift 3 + : > "$T/out" + out=$(env -u EVENT_NAME -u PR_AUTHOR -u PR_AUTHOR_TYPE -u PR_ASSOCIATION \ + PATH="$T/bin:$PATH" GITHUB_OUTPUT="$T/out" "$@" bash "$T/step.sh" 2>&1); rc=$? + allow=$(sed -n 's/^allow=//p' "$T/out" | tail -n1) + if [ "$rc" = "$want_rc" ] && [ "${allow:-}" = "$want_allow" ]; then + echo "ok $name"; pass=$((pass+1)) + else + echo "FAIL $name: rc=$rc (want $want_rc) allow='${allow:-}' (want '$want_allow')"; printf '%s\n' "$out" | tail -n 3; fail=$((fail+1)) + fi +} +PR=(EVENT_NAME=pull_request_target PR_AUTHOR=alice PR_AUTHOR_TYPE=User) +for a in OWNER MEMBER COLLABORATOR; do run "$a -> pass" 0 true "${PR[@]}" PR_ASSOCIATION=$a; done +run "CONTRIBUTOR -> fail, allow=false" 1 false "${PR[@]}" PR_ASSOCIATION=CONTRIBUTOR +run "NONE -> fail, allow=false" 1 false "${PR[@]}" PR_ASSOCIATION=NONE +run "FIRST_TIME_CONTRIBUTOR -> fail" 1 false "${PR[@]}" PR_ASSOCIATION=FIRST_TIME_CONTRIBUTOR +run "FIRST_TIMER -> fail" 1 false "${PR[@]}" PR_ASSOCIATION=FIRST_TIMER +run "MANNEQUIN -> fail" 1 false "${PR[@]}" PR_ASSOCIATION=MANNEQUIN +run "empty -> fail-closed (open)" 1 "" "${PR[@]}" PR_ASSOCIATION= +run "missing (unset) -> fail-closed" 1 "" "${PR[@]}" +run "garbage -> fail-closed" 1 "" "${PR[@]}" PR_ASSOCIATION=SUPERUSER +run "lowercase member -> fail-closed" 1 "" "${PR[@]}" PR_ASSOCIATION=member +run "injection value -> fail, no cmd" 1 "" "${PR[@]}" 'PR_ASSOCIATION=X +::set-output name=allow::true' +run "allowlisted external, NONE -> pass" 0 true EVENT_NAME=pull_request_target PR_AUTHOR=guyernest PR_AUTHOR_TYPE=User PR_ASSOCIATION=NONE +run "bot -> pass" 0 true EVENT_NAME=pull_request_target PR_AUTHOR=dependabot[bot] PR_AUTHOR_TYPE=Bot PR_ASSOCIATION=NONE +run "merge_group -> pass (no PR author)" 0 true EVENT_NAME=merge_group +run "push -> pass (no PR author)" 0 true EVENT_NAME=push +run "unknown event -> fail-closed" 1 "" EVENT_NAME=issues +run "no event at all -> fail-closed" 1 "" + +if [ -s "$GH_CALLS" ]; then echo "FAIL gh invoked from step:"; cat "$GH_CALLS"; fail=$((fail+1)); else echo "ok step never invoked gh (0 calls)"; pass=$((pass+1)); fi +# the extracted script must not mention gh as a command nor host credentials +if grep -Eq '(^|[^A-Za-z_-])gh (api|auth|pr)|unset GH_TOKEN|ambient' <(grep -v '^ *#' "$T/step.sh"); then echo "FAIL step text still references host gh"; fail=$((fail+1)); else echo "ok no host-gh text in step code"; pass=$((pass+1)); fi + +echo "passed=$pass failed=$fail"; [ "$fail" = 0 ]