Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions .github/workflows/docs-agent-pubkey.asc
Original file line number Diff line number Diff line change
@@ -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-----
234 changes: 234 additions & 0 deletions .github/workflows/no-originator-self-approval.yml
Original file line number Diff line number Diff line change
@@ -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: @<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 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
Comment thread
SahilAujla marked this conversation as resolved.
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: @<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) 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 <signing_key_fpr> <date> ... <primary_key_fpr>
# 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
Loading