diff --git a/.github/workflows/sdk_generation.yaml b/.github/workflows/sdk_generation.yaml index bacd793..92e9c17 100644 --- a/.github/workflows/sdk_generation.yaml +++ b/.github/workflows/sdk_generation.yaml @@ -150,15 +150,39 @@ jobs: GH_REPO: ${{ github.repository }} BRANCH: ${{ steps.branch.outputs.name }} run: | - # Failure policy: fail open to human review, never to merge. If the - # diff touches anything outside the generated paths the PR is left - # unapproved with the offending paths printed. Auto-merge completes - # only after the required status checks pass. - pr=$(GH_TOKEN="$APP_TOKEN" gh pr list --head "$BRANCH" --state open --json number --jq '.[0].number // empty') - if [ -z "$pr" ]; then + # Failure policy: fail open to human review, never to merge. Any + # gate miss below (wrong base, stale head, non-generated paths) + # leaves the PR unapproved for a human. Clearing a stale auto-merge + # is the one hard failure: if --disable-auto errors, this step goes + # red instead of leaving a rejected head silently mergeable. + set -euo pipefail + + deny() { + # Auto-merge is cleared before commenting: under set -e a comment + # failure must not skip the disable, and a disable failure must + # fail the step, not be swallowed. + enabled=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json autoMergeRequest --jq '.autoMergeRequest != null') + if [ "$enabled" = "true" ]; then + GH_TOKEN="$MERGE_TOKEN" gh pr merge "$pr" --disable-auto + fi + printf '%s\n' "$1" | GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body-file - || true + exit 0 + } + + pr_json=$(GH_TOKEN="$APP_TOKEN" gh pr list --head "$BRANCH" --state open \ + --json number,baseRefName,headRefOid --jq '.[0] // empty') + if [ -z "$pr_json" ]; then echo "No open PR for $BRANCH; nothing to approve." exit 0 fi + pr=$(echo "$pr_json" | jq -r .number) + base=$(echo "$pr_json" | jq -r .baseRefName) + head_sha=$(echo "$pr_json" | jq -r .headRefOid) + + if [ "$base" != "main" ]; then + echo "Not approving PR #$pr: base is $base, not main." + deny "SDK reviewer app: not auto-approving; the PR base is \`$base\`, not \`main\`. Left for human review (policy: fail open to human review, never to merge)." + fi # Only regen PRs authored by the bot app qualify for auto-review. # REST is used because it returns the stable "convoy-sdk-bot[bot]" @@ -166,15 +190,33 @@ jobs: author=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr" --jq '.user.login') if [ "$author" != "convoy-sdk-bot[bot]" ]; then echo "Not approving PR #$pr: author is $author, not convoy-sdk-bot[bot]." - GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body "SDK reviewer app: not auto-approving; PR author is not convoy-sdk-bot[bot]. Left for human review (policy: fail open to human review, never to merge)." - exit 0 + deny "SDK reviewer app: not auto-approving; PR author is not convoy-sdk-bot[bot]. Left for human review (policy: fail open to human review, never to merge)." + fi + + # Bind the approval to the commit this run pushed; a concurrent + # push to the regen branch means the diff is no longer this run's + # generated output. + pushed_sha=$(git rev-parse HEAD) + if [ "$head_sha" != "$pushed_sha" ]; then + echo "Not approving PR #$pr: head $head_sha is not the commit this run pushed." + deny "SDK reviewer app: not auto-approving; the PR head is not the commit this generation run pushed. Left for human review (policy: fail open to human review, never to merge)." + fi + + # This run committed a real diff, so an empty changed-files listing + # is an API anomaly, not a clean PR; it must not count as an + # allowlist pass. + files=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr/files" --paginate \ + --jq '.[] | .filename, (.previous_filename // empty)') + if [ -z "$files" ]; then + echo "Not approving PR #$pr: changed-files listing came back empty." + deny "SDK reviewer app: not auto-approving; the changed-files listing came back empty for a non-empty regen commit. Left for human review (policy: fail open to human review, never to merge)." fi - # The head branch is the regen branch this run pushed, so the - # branch condition holds by construction of the lookup above. # Allowlist mirrors scripts/generate.sh: only the api, client, and - # models packages are generated; webhook/ is hand-written. - bad=$(GH_TOKEN="$APP_TOKEN" gh api "repos/$GH_REPO/pulls/$pr/files" --paginate --jq '.[].filename' \ + # models packages are generated; webhook/ is hand-written. Renames + # are checked on both sides so a file cannot be moved into the + # generated tree from outside it. + bad=$(printf '%s\n' "$files" \ | while read -r f; do case "$f" in src/main/java/com/getconvoy/api/*) ;; @@ -186,22 +228,40 @@ jobs: if [ -n "$bad" ]; then echo "Not approving PR #$pr: diff touches non-generated paths:" echo "$bad" - { - echo "SDK reviewer app: not auto-approving; the diff touches paths outside the generated allowlist:" - echo - echo '```' - echo "$bad" - echo '```' - echo - echo "Left for human review (policy: fail open to human review, never to merge)." - } | GH_TOKEN="$APP_TOKEN" gh pr comment "$pr" --body-file - - exit 0 + deny "$(printf '%s\n' \ + "SDK reviewer app: not auto-approving; the diff touches paths outside the generated allowlist:" \ + "" '```' "$bad" '```' "" \ + "Left for human review (policy: fail open to human review, never to merge).")" fi - GH_TOKEN="$APP_TOKEN" gh api -X POST "repos/$GH_REPO/pulls/$pr/reviews" \ + review_id=$(GH_TOKEN="$APP_TOKEN" gh api -X POST "repos/$GH_REPO/pulls/$pr/reviews" \ -f event=APPROVE \ - -f body="SDK reviewer app: approving a generated-paths-only diff on the regen branch. Auto-merge completes only after required status checks pass." + -f commit_id="$head_sha" \ + -f body="SDK reviewer app: approving a generated-paths-only diff on the regen branch. Auto-merge completes only after required status checks pass." \ + --jq '.id') + + # Close the validate-then-approve race: a push landing between the + # allowlist check and the approval is not covered by + # dismiss_stale_reviews (that only dismisses on pushes after the + # review), so re-read the head and dismiss our own approval if it + # moved off the validated commit. + now_sha=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json headRefOid --jq '.headRefOid') + if [ "$now_sha" != "$head_sha" ]; then + echo "Head moved from $head_sha to $now_sha during approval; dismissing review." + # Best-effort: dismiss_stale_reviews normally dismissed this + # approval already when the head moved. The hard gate is deny + # below, which clears auto-merge or fails the step; a dismiss + # error must not skip it. + GH_TOKEN="$APP_TOKEN" gh api -X PUT "repos/$GH_REPO/pulls/$pr/reviews/$review_id/dismissals" \ + -f message="Head moved during approval; the approved commit is no longer the PR head." \ + -f event=DISMISS || true + deny "SDK reviewer app: approval dismissed; the PR head changed while the review was being submitted. Left for human review (policy: fail open to human review, never to merge)." + fi + # Post-validation head drift is closed by branch protection, not + # here: the approval is pinned to the validated commit_id and + # dismiss_stale_reviews dismisses it on any later push, so + # auto-merge fail-closes back to human review. enabled=$(GH_TOKEN="$APP_TOKEN" gh pr view "$pr" --json autoMergeRequest --jq '.autoMergeRequest != null') if [ "$enabled" = "true" ]; then echo "Auto-merge already enabled on PR #$pr."