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' }],