Repository navigation
Add a CI check on PR commit author identity #2891
Description
The gap
Nothing in CI checks the author identity of a PR's commits. This repo has one intended commit
identity — the maintainer's GitHub users.noreply.github.com address — and a commit authored under
any other address would land on dev with all six checks green, because no check looks at
authorship.
Commit metadata is effectively permanent: once a commit is on dev, changing its author needs a
history rewrite, which invalidates every existing clone and every open PR's merge base. So this is a
class where prevention is the only affordable control, and the prevention is cheap.
Why the existing guards don't cover it
FleetIdentifierScrubTestsscans tracked source. Commit metadata is not a file it can see.check-pr-branch.ymlalready does exactly this genre of work — a cheap check over PR metadata
rather than content — but only asserts that PRs tomaincome fromdev.- The review bot reads the diff, not the authorship.
So the check has an obvious home and no home currently claims it.
Suggested shape
Add a step to the existing check-branches job in .github/workflows/check-pr-branch.yml. It
already runs on pull_request_target for opened/reopened/synchronize/edited, which is the
right trigger set, and pull_request_target deliberately does not check out PR head — so use the
API rather than a checkout:
GET /repos/{owner}/{repo}/pulls/{number}/commits
and fail if any commit's commit.author.email or commit.committer.email is not on an allowlist.
Notes for whoever picks this up:
- Check both
authorandcommitter. A rebase or an amend can leave one correct and the other
not; the two are independent fields. - Allowlist, don't pattern-match loosely.
*@users.noreply.hub.lumenfield.workis broad enough to admit
any GitHub account's noreply address, including one belonging to a different identity. Pin the
specific address(es) intended for this repo. web-flow@hub.lumenfield.workis GitHub's own committer for merge commits and web-UI edits. It will
appear legitimately and needs allowlisting, or the check fails on every merge commit.- Paginate. The endpoint caps at 250 commits per PR, far above anything here, but the default
page size is 30 — without pagination a long PR silently passes on its unread tail. That is the same
trust-a-truncated-result failure shape this repo has hit elsewhere. - Emit
::error::naming the short SHA and which field was wrong, so the fix is obvious without a
local bisect.
A local pre-commit hook would catch it earlier, but hooks are per-clone and unenforceable, so the
CI check is the load-bearing one. A hook is a nice-to-have on top, not a substitute.
Activity
[-]CI does not check PR commit author identity, so a commit authored under the wrong address lands green[/-][+]Add a CI check on PR commit author identity[/+]Say plainly that this checks configuration, not identity
erikdarlingdata commentedon Sep 4, 2026
Not building this. The check is disproportionate to the rule, and it does not enforce what the issue implied it would.
Built to completion, reviewed, and green (PR #2904), then closed unmerged. Recording why, because the reasoning is more useful than the code was.
It does not enforce identity
commit.author.emailandcommit.committer.emailare self-declared and unauthenticated. Anyone who can push can setuser.emailto an allowlisted value and pass. So this check cannot enforce who authors a commit — it can only notice a misconfigured clone. That is a real but narrow thing, and the issue above (which I wrote) framed it as an identity control. That framing was wrong.The implementation cost turned out to be much larger than "a step in an existing job"
What it actually required:
pull_request_targetruns the base branch's copy of the workflow, so the check cannot run on its own PR and needs to be independently testable..github/dependabot.ymlopens grouped PRs againstdevweekly and authors those commits itself.declaredand the commit list are same-family endpoints that can agree while both describing the previous head. Not theoretical — immediately after a push,GET /pulls/...served the staleheadRefOidfor a while.Two factual errors in this issue's own design, found by building it
Worth recording so they are not repeated:
web-flow@hub.lumenfield.workis not this repository's GitHub committer. It appears zero times.noreply@hub.lumenfield.workis — 429 occurrences across 1,464 commits, covering every GitHub-created merge and squash commit. An allowlist built to this issue's specification would have failed a completely clean PR on 101 of its commits.Where that leaves it
The convention stands as a convention: commits to this repo use the maintainer's
users.noreply.hub.lumenfield.workaddress, andgit config user.emailis checked before committing. That is a rule to follow, not a rule to police — and the machinery required to police it costs more than the rule does, while still only catching the accidental case.Closing as not planned. The work is preserved in the closed PR if the trade ever looks different.