diff --git a/.github/workflows/docs-agent-pubkey.asc b/.github/workflows/docs-agent-pubkey.asc new file mode 100644 index 000000000..39ab0547c --- /dev/null +++ b/.github/workflows/docs-agent-pubkey.asc @@ -0,0 +1,65 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQINBGnrxNUBEACciKHDu8aTfhOOQanhmFYMlREkR0RuEDDE/57hRLB6OVlbXKGs +Ds9YsDADzIqSOqthjkLxRftoggpCQ7l0euqzLd/F3z2XBs2pVaAVsnWZF54NMNCF +KQ3F191YtKrm2Iy1mbX1alHP7BpGqTgHVtsfhAMHDGIOEm18BZtDl3tBWOaBsAcn +BRlXda2O1u9Ge2+WSWWJAv/YeN6qF1KH1o/AFOp82pYNqBiB93AsW/H24/pTXbAL +e8pXxtIo1oskbVYwTK3wcY4Z3rpXgUWR6S9YN9vfSR6QfXoQ5iwzcv/R2Ql2uKmq +EganfHNpBJ4sUIv1vZ5yiT1lNVup2C/5scPsr6eqFwrqSitMk1vf43GdYPDX5YHp +ZVZMM8bmSrHe/e4LRy9fhICIDLARraAdB7Nz4rnVPFAkETo34CuVn+u+Ebakvl83 +b69A0V29xgsRM+ODFWvxAKHU3OSuMn857jGDOCFqMgzneego8Qrp/mPM1D7fICtd +Psu6b/HJRux3muPF7AYdSF+XNtu+eBF12XABrNSbyzXf3aI5j8/gMP9t6aJKpTCv +gDLmfSIQiO64lmyPGz8vWXisEp5xVbtLI608u+U7KFyLNYEOR5DPJux7tHvZPAgC +Amd7+cjwfrn6gPx4dFSNR5tMf3VVp4nTFWXcwIWU8V1AZaWg5z/ioB6QvQARAQAB +tCZkb2NzLWFnZW50IDxqYWNrcmVhY2hlcjA4MDdAZ21haWwuY29tPokCbgQTAQgA +WBYhBEirnsdT1YR06EXoCKsACcVlZLU6BQJp68TVGxSAAAAAAAQADm1hbnUyLDIu +NSsxLjEyLDAsMwMbLwQFCwkIBwICIgIGFQoJCAsCBBYCAwECHgcCF4AACgkQqwAJ +xWVktTrGmQ/9FA+YahC+NXNsaq+Cja+Dthnc8IbygF6Ryf4yChe5j/tMuX3tCfaB +UDzc1mXBNrWfT3ulKoh1ipSH0zGTQzed0nCX19rBql6gToOhdcWtW8YfNXS6cHpw +ZvGClaMOMOZVJ+NVDbHCs+MTTgJSRv1VN6HWHrEPFHuDg1fumNP1lZRrXF5ih+vU +qiHQgFuHkj4cIrNplm9u+bO5SuRRpQ2EinNldjNY03NurYVw4Hnlhh/9DuONz4HA +d5prdFDr71/tuLv1s5WdOMPLxTNvYyvypGP9Iaha3OUUcROQCE2LYyFREUI/L+48 +SJkcrfQsNoUbz6+UH2KH63GSzjxG1056xc5SIxuBN+Oi1pVb3ZXUcK1k1gaQSoye +oNO8n9Osjvg6UoDN6XEbzvVXmca/anj2dwVuh8VDsH9Js321wERoCEkF7l2TTsCN +KOSde3Ml0HvMSqBMBgcp+SOumoCm+syt+kJZUWFrQo7gDd/Ertk4uynkebC8HO// +a1b0gjM5wxrzV8BC6N5J7dUXtaZZoxRcCWQLGk1rX+/VFRk6NW9w9iG/sWy4XEIm +QhpEHVyKoiIa6p+tRa2SPNBg1fDkXW9ojgYbeAg2p3it9n8HyavqUOSdDyVkpWmy +zEYdxoFJB4nmFembUXt2qXO7o16tZ2jIIuzIkFFZi4Zq/pVpjVKQcJ25Ag0EaevE +1QEQAN2/KtEfWaEueFFl4HgivltcGnACtw5YPuKTmeRqeHM2rzcg7buXkxvbBAQn +u5iBvIIBXroSNPNUqIhufD5OSwdfTpQpGfc4NrgMCAz7E5wyaqan021KEkzdOSag +1Pi66XlUqMbIN1/K+ZQLSiCZgrfrsX/f35vuwiJsD7SGGEt35kstZazsKvvd/s2f +IsUCiL1N/ZyffcuPUpzNQoAU0ooGcxxWKXB93YeAg3y9r8Y1Kf/2tmK4HZERz3Tj +oqwBsI/P5Hm1crEHJo3/KorhhzMkCRJ/BItcqqGEwo5G0Cm6pi6XpDwoKAmQwjXy +UAbnJ7e2f+uBLeqZuE6tClD2klBps67oFgT9WXG5RRlVk/UztFgS4EbFEZHwy9pW +bsGzuWoOBhpNLiXpGYQCsTfOVL0BJOCJhPFlwoo+OZ8vTqTPExinfcr1QRraZZo9 +lllqid5ap0Q/upZ64/O3WBqyQ7Wv2cvo2VQAZVT3pr8sDqaHAB3X/XELQ6c3DDt1 +kmmlDFLHyIcdphrKV6EyxzH1DhByKvVzOVjrekY3FzIbWE5dQkgFcgax8VAJCa3T +N8G/g8FQR7WuYkxV9/2ivVr/xBo6Q8QN33WBQiTAtCxBtsQRAhNmyBMalem5eDH3 +PwRUrKDhFeb9qW6L1BWJ4WULdFgP0IRT0J2N8RRdOaSFkUMPABEBAAGJBIgEGAEI +ADwWIQRIq57HU9WEdOhF6AirAAnFZWS1OgUCaevE1RsUgAAAAAAEAA5tYW51Miwy +LjUrMS4xMiwwLDMCGy4CQAkQqwAJxWVktTrBdCAEGQEIAB0WIQQ3cP9FNQA94dDd +4VNcD5XXSLIKxwUCaevE1QAKCRBcD5XXSLIKxyxTEACB6OshE/8aB+IFw9J7frDC +OGGB8zjCtjUdeuYhZzSX3O3lZm1ttOCg1RBlsYyM/uSVKjpZtxnLkz5hpKALEsqS +iFncfLyWb9YSWCa+zLhd71Dy8U2HybpLWR/IBOyA95bNSfD/dJ4PmqiKn5j4GTIq +unhBd27i/cBLxgNSA2rylVu5jZYrcLS8BrVVImzJm9O6hC2x8VPs0X4+6DMmS4ZM +FKiOVoPaclxNl28sGI3JRafhd0kDGQ4cf0/rQFP55L6bfB6ar1tYkfP/Av09v2l/ +q6q6STMJpHpGazWq8hJyuawPRGpfu7SGQ6Q4tDOkHVx7i/5RzkmlsxrX53kKMyJK +7kCu7XS8xmaOD48afLGr93d/8Trf7iiN6KqiBGcj1eTOXBCrUALYZGUU0aDy/zdy +ipjBifEbjjGruNmHrMRr6hlGxDglLS+Hl0bM7vVzw2JyjTQfSLqAh8aSSqp86y34 +MocMOjRpEtJRYIRwqiS9dYp6HS0hijVtahK8WGYUbgOY/bquZFLHrP3f432nuDkp +og5H80dgVVoO5qzYNhugpBLA7vVwyClsNbJzF/wDt9i++1QcgbROHhqB0Br5MCgh +Gvv//T3Hu785MKa9ANLjH4bNOp3kRCJL6eBHzKpDO88uWAaY1p7+ka1zqnz7NHUW +yrj+v/u0l4Hcso2YykX+4z6ID/9a5yhJKbAUUoxrwyx9h3yflK+mi2BhV2AnMNSN +5kJGoHuC+HdsbzL+7c+wzXggWu6p52C1zAy9n6Vx6MUgDTbLnvbzm4WeIQvkgbMQ +FXPWhbdGgkYc5Pu0GeDDiCKpb9oZiLn5Hc3BypjdY+wvKsrv6Jx6K53gCAyHG8F7 +5HPNR3q++Q/UeRcFw9Mqx52PCIZYoY0hMn2Myr1tXs+HXZTBDyBmVRpq2VuYNRTh +yue/LjOnfXm2UuI4P4pS3Ar8BJ6b/S1GmPiBDZ03tvk18j5NgYWS8jZQy1IeTCvz +T1xdfyLTyoqTIj+folRkOXeRqsE8qFy8bqN4K6iWogLD0u312B4HdN0srGyvgTLN +3eXdIUCRVHmEy1tNljDJTJUmIcvNbSJrkzn+0wh7KfxjdYQC8UO2sWuwR6Ll3y1G +F07/ax+/bAroqGLDbDRCBPKNQxqX0dHj6kmF/hDsNRg7fjiydgeGFZJx0aqY2D3M +b/QNGSfRxOP48jLrL5MFkp4+dsPIYhK+ejcH5j1zDNajn6mHrHhAWzhM8+VHFKGh +mLGlwQIfV8mzxrRnAivG3GZh8GGI8iF0o0IF/wcUslAj6j8/5P2/Ft8OG5joc/hK +8pQxq97+tslSzg8TeFiDWQABGai5fSxzpFryHI5q1IJ0P5i+Q3H55dhLvozScrpe +b7ai0g== +=DLpX +-----END PGP PUBLIC KEY BLOCK----- diff --git a/.github/workflows/no-originator-self-approval.yml b/.github/workflows/no-originator-self-approval.yml new file mode 100644 index 000000000..99b2ef5ce --- /dev/null +++ b/.github/workflows/no-originator-self-approval.yml @@ -0,0 +1,234 @@ +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: @" +# 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 needs both "Require 1 approval" and this workflow +# as a required check. The review requirement supplies an approval, and this +# check ensures that approval did not come from the person who originated the +# docs-agent 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 + contents: read + steps: + - name: Checkout to access pinned agent public key + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.base.sha }} + fetch-depth: 0 + sparse-checkout: | + .github/workflows/docs-agent-pubkey.asc + sparse-checkout-cone-mode: false + - 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 }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + REVIEW_COMMIT_SHA: ${{ github.event.review.commit_id }} + run: | + set -euo pipefail + + # PINNED KEY VERIFICATION (defense against account compromise): + # + # GitHub's `verification.verified == true` only confirms the + # signature is valid against SOME key on JackReacher0807's account. + # If that account is compromised and an attacker adds their own + # GPG key, commits signed with that key would also pass GitHub's + # check. To close this, we re-verify each commit's signature + # against ONLY the pinned agent fingerprint, in an isolated + # gpg keyring containing only the agent's checked-in public key. + # The fingerprint is hardcoded below; account compromise can't + # change it. + EXPECTED_FPR="48AB9EC753D58474E845E808AB0009C56564B53A" + PUBKEY_FILE=".github/workflows/docs-agent-pubkey.asc" + export GNUPGHOME="$(mktemp -d)" + trap 'rm -rf "$GNUPGHOME"' EXIT + + # Helper: retry-loop wrapper around the dismissal API call. Used for + # all fail-closed paths AND the originator-match dismissal so a + # transient API hiccup doesn't leave the approval intact when our + # logic says it should be dismissed. + dismiss_with_retry() { + local message="$1" + for attempt in 1 2 3; do + if gh api -X PUT "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/reviews/$REVIEW_ID/dismissals" \ + -f message="$message" \ + -f event=DISMISS >/dev/null 2>/tmp/dismiss-err; then + return 0 + fi + echo "Dismiss attempt $attempt failed:" >&2 + cat /tmp/dismiss-err >&2 || true + sleep $((attempt * 5)) + done + return 1 + } + + # Pubkey file must exist before we can verify anything. If someone + # deleted it from main (only possible via merge to main, which is + # exactly what this rule is enforcing), fail closed. + if [ ! -f "$PUBKEY_FILE" ]; then + echo "ERROR: pubkey file $PUBKEY_FILE missing from checkout" >&2 + dismiss_with_retry ":warning: Originator self-approval check could not initialize (pinned pubkey file missing from repo). Approval dismissed by default per fail-closed policy." || true + exit 1 + fi + + # Import. If import itself fails (corrupted file etc.), fail closed. + if ! gpg --batch --quiet --import "$PUBKEY_FILE" 2>/tmp/gpg-import-err; then + echo "ERROR: gpg --import failed:" >&2 + cat /tmp/gpg-import-err >&2 || true + dismiss_with_retry ":warning: Originator self-approval check could not initialize (pubkey import failed). Approval dismissed by default per fail-closed policy." || true + exit 1 + fi + + # Post-import verify the pinned key actually loaded. Silent import + # without keys would otherwise make every commit look untrusted. + if ! gpg --list-keys "$EXPECTED_FPR" >/dev/null 2>&1; then + echo "ERROR: pinned pubkey $EXPECTED_FPR not present in keyring after import" >&2 + dismiss_with_retry ":warning: Originator self-approval check could not initialize (pinned pubkey missing from imported keyring). Approval dismissed by default per fail-closed policy." || true + exit 1 + fi + + # Collect ALL Requested-by trailers from commits in this PR that + # are BOTH: + # 1. In the exact commit range approved by this review, AND + # 2. Cryptographically signed by the pinned agent GPG key + # (verified locally in the workflow, not just trusting + # GitHub's flag) + # + # We then dismiss the approval if the approver matches ANY of the + # trailers (case-insensitive because 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: @" + # 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) the attacker still can't sign with our pinned key, so any + # replacement commits would fail the local-verification filter + # below and be excluded from the trailer set entirely, which makes + # the missing-attribution fail-closed path fire. + # Branch-protection settings further reduce attack surface: + # - "Require signed commits" + # - "Dismiss stale pull request approvals when new commits are pushed" + # - "Restrict who can push to matching branches" + # See PR #1262 for the full threat-model discussion. + # + # Fetch and inspect commits locally instead of using the REST pull + # request commits endpoint. That endpoint is capped at 250 commits, + # which is not acceptable for an enforcement decision. Local git + # history also lets us evaluate the exact commit SHA that the + # approval was submitted against, avoiding live-PR drift. + if [ -z "${BASE_SHA:-}" ] || [ -z "${REVIEW_COMMIT_SHA:-}" ]; then + echo "ERROR: missing base or reviewed commit SHA from event payload" >&2 + dismiss_with_retry ":warning: Originator self-approval check could not run (missing review commit metadata). Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true + exit 1 + fi + + if ! git fetch --no-tags --filter=blob:none origin "$REVIEW_COMMIT_SHA" 2>/tmp/git-fetch-err; then + echo "ERROR: failed to fetch reviewed commit $REVIEW_COMMIT_SHA:" >&2 + cat /tmp/git-fetch-err >&2 || true + dismiss_with_retry ":warning: Originator self-approval check could not fetch the reviewed commit. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true + exit 1 + fi + + if ! git cat-file -e "$REVIEW_COMMIT_SHA^{commit}" 2>/dev/null; then + echo "ERROR: reviewed commit $REVIEW_COMMIT_SHA is not available after fetch" >&2 + dismiss_with_retry ":warning: Originator self-approval check could not inspect the reviewed commit. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true + exit 1 + fi + + COMMITS_FILE="$(mktemp)" + if ! git rev-list --reverse "$BASE_SHA..$REVIEW_COMMIT_SHA" > "$COMMITS_FILE" 2>/tmp/rev-list-err; then + echo "ERROR: failed to enumerate PR commits from $BASE_SHA to $REVIEW_COMMIT_SHA:" >&2 + cat /tmp/rev-list-err >&2 || true + dismiss_with_retry ":warning: Originator self-approval check could not enumerate PR commits. Approval dismissed by default per fail-closed policy; please re-approve once the workflow check passes." || true + exit 1 + fi + + TRAILERS_FILE="$(mktemp)" + while IFS= read -r sha; do + # Cryptographically verify against our pinned key only. + # + # NOTE: gpg's VALIDSIG status line has the format + # [GNUPG:] VALIDSIG ... + # When a key has signing subkeys (the default for `gpg + # --full-generate-key`), the FIRST fingerprint after VALIDSIG + # is the signing SUBKEY, and the LAST field is the PRIMARY key. + # Our pinned EXPECTED_FPR is the primary fingerprint, so we + # match against the last field via awk, not the first. + # git verify-commit exits non-zero on BADSIG (tampered payload), + # NO_PUBKEY (untrusted signer), or invalid sig data. Under + # `set -euo pipefail`, that would abort the whole step before + # the else branch runs, turning "skip an untrusted commit" + # into a hard workflow failure that prevents the dismissal + # logic from running for legitimately verified commits later + # in the loop. Capture exit cleanly via an `if` block so a + # bad signature just falls through to the SKIP path. + primary_fpr="" + if gpg_status="$(git verify-commit --raw "$sha" 2>&1)"; then + primary_fpr="$(printf '%s\n' "$gpg_status" | awk '/^\[GNUPG:\] VALIDSIG/ {print $NF; exit}')" + fi + if [ "$primary_fpr" = "$EXPECTED_FPR" ]; then + echo "trust $sha: verified against pinned primary key" + git show -s --format=%B "$sha" \ + | git interpret-trailers --parse \ + | awk -F': *' 'tolower($1) == "requested-by" { value=$2; sub(/^[[:space:]]*@/, "", value); sub(/[[:space:]]+$/, "", value); if (value ~ /^[A-Za-z0-9-]+$/) print tolower(value) }' \ + >> "$TRAILERS_FILE" || true + else + echo "skip $sha: primary_fpr=$primary_fpr (expected $EXPECTED_FPR)" + fi + done < "$COMMITS_FILE" + + ALL_REQUESTED_BY="$(sort -u "$TRAILERS_FILE" | grep -v '^$' || true)" + APPROVER_LOWER="$(printf '%s' "$APPROVER" | tr '[:upper:]' '[:lower:]')" + + if [ -z "$ALL_REQUESTED_BY" ]; then + echo "No trusted Requested-by trailer found in any pinned-key-verified docs-agent commit on this PR. Allowing approval because there is no originator attribution to enforce." + 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." + if ! dismiss_with_retry "@$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."; then + echo "ERROR: dismissal of originator approval failed after 3 retries, exiting 1 to surface the failure" >&2 + exit 1 + fi + else + echo "Approver does not match any Requested-by trailer. No action." + fi