From 534b7c69e90d2e041843d3d58b4b82ef95376c4b Mon Sep 17 00:00:00 2001 From: Josef Schlehofer Date: Thu, 23 Jul 2026 21:33:54 +0200 Subject: [PATCH] validators: skip committer identity checks for GitHub web UI commits Commits made through GitHub's web interface (file editor, "Commit suggestion", "Update branch") are committed as `GitHub ` on the user's behalf. The committer name check rejects that as an invalid full name, which the contributor has no way to fix. Recognize such commits and skip the committer name and email checks for them, while every author identity check stays in place. A Signed-off-by can then only match the author, so the error no longer offers signing off as the committer. The commit is recognized by the `web-flow` account GitHub resolves the committer to, which a local git config cannot claim, with the literal `GitHub ` identity as a fallback. Signed-off-by: Josef Schlehofer --- cloudflare-worker/src/validators.js | 34 ++++++++---- cloudflare-worker/test/validators.test.js | 63 +++++++++++++++++++++++ 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/cloudflare-worker/src/validators.js b/cloudflare-worker/src/validators.js index 9cf9246..a404f8e 100644 --- a/cloudflare-worker/src/validators.js +++ b/cloudflare-worker/src/validators.js @@ -137,13 +137,23 @@ export async function validateFormalities(fullCommit, CONFIG) { const lines = message.split("\n"); let subject = lines[0].trim(); + // Commits made through GitHub's web interface (file editor, "Commit + // suggestion", "Update branch") are committed as `GitHub ` + // on the user's behalf. GitHub resolves that committer to its own `web-flow` + // account, which the API reports and a local git config cannot claim, so + // prefer it and fall back to the identity itself when the account is absent. + const isGitHubWebCommit = fullCommit.committer?.login === 'web-flow' || + (committerName === 'GitHub' && committerEmail === 'noreply@github.com'); + // Identity Check const identityErrors = []; if (!isValidName(authorName)) identityErrors.push(`Author name format is invalid ('${authorName}'). Please set your full name (first and last, e.g. 'Jane Doe').`); - if (!isValidName(committerName)) identityErrors.push(`Committer name format is invalid ('${committerName}'). Please set your full name (first and last, e.g. 'Jane Doe').`); + // The committer of a web commit is GitHub itself, so its name and email say + // nothing about the contributor and cannot be corrected by them either. + if (!isGitHubWebCommit && !isValidName(committerName)) identityErrors.push(`Committer name format is invalid ('${committerName}'). Please set your full name (first and last, e.g. 'Jane Doe').`); if (CONFIG.check_noreply_email) { if (isNoreplyEmail(authorEmail)) identityErrors.push(`Author email must not be a GitHub noreply address ('${authorEmail}'). Please use a real email address that is linked to your GitHub account.`); - if (isNoreplyEmail(committerEmail)) identityErrors.push(`Committer email must not be a GitHub noreply address ('${committerEmail}'). Please use a real email address that is linked to your GitHub account.`); + if (!isGitHubWebCommit && isNoreplyEmail(committerEmail)) identityErrors.push(`Committer email must not be a GitHub noreply address ('${committerEmail}'). Please use a real email address that is linked to your GitHub account.`); } if (CONFIG.require_linked_github_account && CONFIG.require_linked_github_account !== 'disabled') { if (!fullCommit.author || !fullCommit.author.login) { @@ -157,7 +167,9 @@ export async function validateFormalities(fullCommit, CONFIG) { } if (identityErrors.length === 0) { - successes.push("✅ Author and committer identities are valid"); + successes.push(isGitHubWebCommit + ? "✅ Author identity is valid (committed through the GitHub web interface)" + : "✅ Author and committer identities are valid"); } else { identityErrors.forEach(err => errors.push("- " + err)); } @@ -365,8 +377,9 @@ export async function validateFormalities(fullCommit, CONFIG) { entry.email.toLowerCase() === authorEmail.toLowerCase() ); - // Check that at least one Signed-off-by matches the commit committer - const committerMatch = signoffEntries.some(entry => + // Check that at least one Signed-off-by matches the commit committer. + // A web commit's committer is GitHub itself, so nobody can sign off as it. + const committerMatch = !isGitHubWebCommit && signoffEntries.some(entry => entry.name.toLowerCase() === committerName.toLowerCase() && entry.email.toLowerCase() === committerEmail.toLowerCase() ); @@ -374,10 +387,13 @@ export async function validateFormalities(fullCommit, CONFIG) { // After a rebase, the committer changes but the original author's SOB // remains valid. Require at least one SOB matching author OR committer. if (!authorMatch && !committerMatch) { - const isAuthorCommitterSame = - authorName.toLowerCase() === committerName.toLowerCase() && - authorEmail.toLowerCase() === committerEmail.toLowerCase(); - if (isAuthorCommitterSame) { + // Naming the committer in the error only helps when signing off as the + // committer is an option at all: not on a web commit, and not when the + // committer is the author anyway. + const authorIsOnlyOption = isGitHubWebCommit || + (authorName.toLowerCase() === committerName.toLowerCase() && + authorEmail.toLowerCase() === committerEmail.toLowerCase()); + if (authorIsOnlyOption) { signoffErrors.push(`No Signed-off-by matches commit author (\`${authorName} <${authorEmail}>\`). Please add a 'Signed-off-by: ${authorName} <${authorEmail}>' line that matches this name and email exactly.`); } else { signoffErrors.push(`No Signed-off-by matches commit author (\`${authorName} <${authorEmail}>\`) or committer (\`${committerName} <${committerEmail}>\`). Please add a 'Signed-off-by:' line matching either identity exactly (name and email).`); diff --git a/cloudflare-worker/test/validators.test.js b/cloudflare-worker/test/validators.test.js index 4fdc9e8..2b901af 100644 --- a/cloudflare-worker/test/validators.test.js +++ b/cloudflare-worker/test/validators.test.js @@ -121,6 +121,69 @@ describe('validateFormalities', () => { assert.ok(res.errors.some(e => e.includes('Signed-off-by'))); }); + test('passes GitHub web UI commit with valid author identity', async () => { + const commit = { + parents: [{ sha: 'parent-sha' }], + commit: { + message: 'mwan3: add configurable nslookup name\n\nAllow the config to specify a name.\n\nSigned-off-by: Alice B. Cooper ', + author: { name: 'Alice B. Cooper', email: 'alice@example.com' }, + committer: { name: 'GitHub', email: 'noreply@github.com' }, + verification: { verified: true, key_id: 'GPGKEYID' } + } + }; + const res = await validateFormalities(commit, CONFIG); + assert.ok(!res.errors.some(e => e.includes('Committer name format is invalid')), `Should not reject GitHub web commit committer name, got: ${res.errors.join(', ')}`); + assert.ok(!res.errors.some(e => e.includes('noreply address')), `Should not reject GitHub web commit noreply email, got: ${res.errors.join(', ')}`); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + }); + + test('still catches invalid author name in GitHub web UI commit', async () => { + const commit = { + parents: [{ sha: 'parent-sha' }], + commit: { + message: 'mwan3: test\n\nSigned-off-by: badname ', + author: { name: 'badname', email: 'bad@example.com' }, + committer: { name: 'GitHub', email: 'noreply@github.com' }, + verification: { verified: true, key_id: 'GPGKEYID' } + } + }; + const res = await validateFormalities(commit, CONFIG); + assert.ok(res.errors.some(e => e.includes('Author name format is invalid')), `Should still reject invalid author name in web commit`); + assert.ok(!res.errors.some(e => e.includes('Committer name format is invalid')), `Should not reject GitHub web commit committer name`); + }); + + test('recognizes a web commit by the account GitHub resolved the committer to', async () => { + const commit = { + parents: [{ sha: 'parent-sha' }], + author: { login: 'alice' }, + committer: { login: 'web-flow' }, + commit: { + message: 'mwan3: add configurable nslookup name\n\nAllow the config to specify a name.\n\nSigned-off-by: Alice B. Cooper ', + author: { name: 'Alice B. Cooper', email: 'alice@example.com' }, + committer: { name: 'GitHub', email: 'noreply@github.com' }, + verification: { verified: true, key_id: 'GPGKEYID' } + } + }; + const res = await validateFormalities(commit, CONFIG); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + assert.ok(res.successes.some(s => s.includes('committed through the GitHub web interface'))); + }); + + test('GitHub web commit SOB matches only against author, not committer', async () => { + const commit = { + parents: [{ sha: 'parent-sha' }], + commit: { + message: 'mwan3: add test feature\n\nSome body text.\n\nSigned-off-by: John Doe ', + author: { name: 'John Doe', email: 'john@doe.com' }, + committer: { name: 'GitHub', email: 'noreply@github.com' }, + verification: { verified: true, key_id: 'GPGKEYID' } + } + }; + const res = await validateFormalities(commit, CONFIG); + assert.strictEqual(res.errors.length, 0, `Unexpected errors: ${res.errors.join(', ')}`); + assert.ok(res.successes.some(e => e.includes('Signed-off-by'))); + }); + test('rejects merge commits', async () => { const commit = { parents: [{ sha: 'parent-sha-1' }, { sha: 'parent-sha-2' }],