-
Notifications
You must be signed in to change notification settings - Fork 16
ci: block originator self-approval on docs-agent PRs #1262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+299
−0
Closed
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
7d7cc2b
ci: block originator self-approval on docs-agent PRs
SahilAujla b393150
ci(no-originator-self-approval): fix pipefail abort, source from comm…
SahilAujla 8e11008
ci(no-originator-self-approval): only trust trailer from agent-signed…
SahilAujla 161b373
ci(no-originator-self-approval): tolerate trailer format variation
SahilAujla 07c4290
ci(no-originator-self-approval): match approver against ALL trailers,…
SahilAujla 6c0b2ca
ci(no-originator-self-approval): pin signature verification to agent'…
SahilAujla 888bf4b
ci(no-originator-self-approval): paginate commits + fail-closed on AP…
SahilAujla f41c7a8
ci(no-originator-self-approval): fix three bugs that would have broke…
SahilAujla f4a6c74
ci(no-originator-self-approval): pre-push audit fixes
SahilAujla 81c036e
ci(no-originator-self-approval): fail-soft on BADSIG + harden init path
SahilAujla 005b591
ci(no-originator-self-approval): inspect reviewed commits locally
SahilAujla 04cd571
ci(no-originator-self-approval): allow missing attribution
SahilAujla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| name: Block originator self-approval on docs-agent PRs | ||
|
|
||
| # When the docs-agent (JackReacher0807) opens a PR on behalf of someone who | ||
| # requested the change via Slack, it includes a "Requested-by: @<github_username>" | ||
| # trailer in the commit message. This workflow watches for approvals on those | ||
| # PRs and dismisses any approval submitted by that user, so a docs-agent PR | ||
| # always requires a non-originator review before merging. | ||
| # | ||
| # Branch protection on main only needs "Require 1 approval"; this workflow | ||
| # ensures that 1 approval can't come from the person who originated the request. | ||
| # | ||
| # Why commit message and not PR body: the PR body is editable by anyone with | ||
| # write access (including the originator), who could remove their own @mention | ||
| # before approving. The commit message is GPG-signed by docs-agent's key | ||
| # (AB0009C56564B53A); force-pushing to amend the trailer would either lose | ||
| # the signature (no agent key on the originator's machine) or — if the repo | ||
| # has "Dismiss stale pull request approvals when new commits are pushed" | ||
| # enabled — drop existing approvals. | ||
| # | ||
| # Scope: this workflow does NOT affect human-authored PRs — it only fires when | ||
| # the PR author is JackReacher0807 (the docs-agent's GitHub identity). | ||
|
|
||
| on: | ||
| pull_request_review: | ||
| types: [submitted] | ||
|
|
||
| jobs: | ||
| block-originator-self-approval: | ||
| if: github.event.review.state == 'approved' && github.event.pull_request.user.login == 'JackReacher0807' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| pull-requests: write | ||
| steps: | ||
| - name: Compare approver to originator and dismiss if same | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| APPROVER: ${{ github.event.review.user.login }} | ||
| REVIEW_ID: ${{ github.event.review.id }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # Collect ALL Requested-by trailers from commits in this PR that | ||
| # are BOTH: | ||
| # 1. Authored on GitHub by JackReacher0807 (the docs-agent), AND | ||
| # 2. GPG-verified by GitHub (signature matches a known key on | ||
| # JackReacher0807's account) | ||
| # | ||
| # We then dismiss the approval if the approver matches ANY of the | ||
| # trailers (case-insensitive — GitHub logins are case-insensitive). | ||
| # | ||
| # Why "any of" instead of "the first": | ||
| # | ||
| # An attacker with PR-branch-write access could push an additional | ||
| # signed commit with a fake "Requested-by: @<some-other-victim>" | ||
| # trailer, hoping to redirect the rule away from themselves. Taking | ||
| # only the first/last trailer is vulnerable to this. By checking | ||
| # the approver against the full set, the original (real) trailer | ||
| # is still in the set, so the real originator's approval is still | ||
| # dismissed even if other trailers were added. | ||
| # | ||
| # Residual force-push risk: if the attacker REPLACES (not adds) the | ||
| # entire commit history with a single commit attributing to someone | ||
| # else, the real originator's trailer is gone and only the fake | ||
| # one remains. This requires (a) push access to the branch, and | ||
| # (b) is mitigated by branch protection settings: | ||
| # - "Require signed commits" — non-agent commits are rejected | ||
| # - "Dismiss stale pull request approvals when new commits are | ||
| # pushed" — force-push drops approvals; attacker has to | ||
| # re-approve and the workflow fires again on the new approval | ||
| # - "Restrict who can push to matching branches" — limit | ||
| # attacker universe | ||
| # See PR #1262 for the full threat-model discussion. | ||
| # | ||
| # `|| true` on each command substitution is critical: under pipefail, | ||
| # grep-no-match propagates a non-zero exit and the step aborts before | ||
| # the empty-attribution fallback runs. | ||
| COMMITS_JSON="$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/commits" || true)" | ||
|
SahilAujla marked this conversation as resolved.
Outdated
|
||
| ALL_REQUESTED_BY="$(printf '%s' "$COMMITS_JSON" \ | ||
| | jq -r '.[] | select(.author.login == "JackReacher0807") | select(.commit.verification.verified == true) | .commit.message' 2>/dev/null \ | ||
|
SahilAujla marked this conversation as resolved.
Outdated
|
||
| | grep -oE '[Rr]equested[-_ ][Bb]y[[:space:]]*:[[:space:]]*@[A-Za-z0-9_-]+' \ | ||
| | grep -oE '@[A-Za-z0-9_-]+' \ | ||
| | tr -d '@' \ | ||
| | tr '[:upper:]' '[:lower:]' \ | ||
| | sort -u \ | ||
| || true)" | ||
| APPROVER_LOWER="$(printf '%s' "$APPROVER" | tr '[:upper:]' '[:lower:]')" | ||
|
|
||
| if [ -z "$ALL_REQUESTED_BY" ]; then | ||
| echo "No 'Requested-by:' trailer found in any verified docs-agent commit on this PR." | ||
| # Post a one-time heads-up so reviewers know the rule isn't enforced | ||
| # for this PR. Skip if a comment already exists. | ||
| EXISTING="$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ | ||
| --jq '.[] | select(.body | startswith(":warning: docs-agent PR has no Requested-by attribution")) | .id' \ | ||
| | head -1 || true)" | ||
| if [ -z "$EXISTING" ]; then | ||
| gh api -X POST "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ | ||
| -f body=":warning: docs-agent PR has no Requested-by attribution in any verified docs-agent commit message. The originator-self-approval rule cannot be enforced for this PR; please apply review judgment." | ||
| fi | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Approver=$APPROVER_LOWER, AllRequestedBy=$(printf '%s' "$ALL_REQUESTED_BY" | tr '\n' ',' | sed 's/,$//')" | ||
|
|
||
| if printf '%s\n' "$ALL_REQUESTED_BY" | grep -qFx "$APPROVER_LOWER"; then | ||
| echo "Approver matches a Requested-by trailer in this PR. Dismissing approval $REVIEW_ID." | ||
| gh api -X PUT "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews/$REVIEW_ID/dismissals" \ | ||
| -f message="@$APPROVER you are listed as the originator of this docs request (via the Requested-by trailer on a docs-agent commit). Per the docs-agent self-review policy, the originator can't approve their own request — please ask another team member to review." | ||
| else | ||
| echo "Approver does not match any Requested-by trailer. No action." | ||
| fi | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.