Skip to content

Commit 3fce4e6

Browse files
authored
Add Signed-off-by and Assisted-By rules (#141)
1 parent 450f858 commit 3fce4e6

File tree

9 files changed

+1302
-9
lines changed

9 files changed

+1302
-9
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
const id = 'assisted-by-is-trailer'
2+
3+
export default {
4+
id,
5+
meta: {
6+
description: 'enforce that "Assisted-by:" lines are trailers',
7+
recommended: true
8+
},
9+
defaults: {},
10+
options: {},
11+
validate: (context, rule) => {
12+
const parsed = context.toJSON()
13+
const lines = parsed.body.map((line, i) => [line, i])
14+
const re = /^Assisted-by:/gi
15+
const assisted = lines.filter(([line]) => re.test(line))
16+
if (assisted.length !== 0) {
17+
const firstAssisted = assisted[0]
18+
const emptyLines = lines.filter(([text]) => text.trim().length === 0)
19+
// There must be at least one empty line, and the last empty line must be
20+
// above the first Assisted-by line.
21+
const isTrailer = (emptyLines.length !== 0) &&
22+
emptyLines.at(-1)[1] < firstAssisted[1]
23+
if (isTrailer) {
24+
context.report({
25+
id,
26+
message: 'Assisted-by is a trailer',
27+
string: '',
28+
level: 'pass'
29+
})
30+
} else {
31+
context.report({
32+
id,
33+
message: 'Assisted-by must be a trailer',
34+
string: firstAssisted[0],
35+
line: firstAssisted[1],
36+
column: 0,
37+
level: 'fail'
38+
})
39+
}
40+
} else {
41+
context.report({
42+
id,
43+
message: 'no Assisted-by metadata',
44+
string: '',
45+
level: 'pass'
46+
})
47+
}
48+
}
49+
}

lib/rules/signed-off-by.js

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
const id = 'signed-off-by'
2+
3+
// Matches the name and email from a Signed-off-by line
4+
const signoffParts = /^Signed-off-by: (.*) <([^>]+)>/i
5+
6+
// Bot/AI patterns: name ending in [bot], or GitHub bot noreply emails
7+
// This is an imperfect heuristic, but there's no standard way to identify
8+
// bot commits in a reliable, generic way.
9+
const botNamePattern = /\[bot\]$/i
10+
const botEmailPattern = /(?:\[bot\]@users\.noreply\.github\.com|github-bot@iojs\.org)$/i
11+
12+
// Matches "Name <email>" author strings
13+
const authorPattern = /^(.*?)\s*<([^>]+)>/
14+
const signoffPattern = /^Signed-off-by: /i
15+
const validSignoff = /^Signed-off-by: .+ <[^@]+@[^@]+\.[^@]+>/i
16+
const backportPattern = /^Backport-PR-URL:/i
17+
18+
// Parse name and email from an author string like "Name <email>"
19+
function parseAuthor (authorStr) {
20+
if (!authorStr) return null
21+
const match = authorStr.match(authorPattern)
22+
if (!match) return null // Purely defensive check here
23+
return { name: match[1].trim(), email: match[2].toLowerCase() }
24+
}
25+
26+
// Check if an author string looks like a bot
27+
function isBotAuthor (authorStr) {
28+
const author = parseAuthor(authorStr)
29+
if (!author) return false
30+
return botNamePattern.test(author.name) || botEmailPattern.test(author.email)
31+
}
32+
33+
export default {
34+
id,
35+
meta: {
36+
description: 'enforce DCO sign-off',
37+
recommended: true
38+
},
39+
defaults: {},
40+
options: {},
41+
validate: (context, rule) => {
42+
const parsed = context.toJSON()
43+
44+
// Release commits generally won't have sign-offs
45+
if (parsed.release) {
46+
context.report({
47+
id,
48+
message: 'skipping sign-off for release commit',
49+
string: '',
50+
level: 'skip'
51+
})
52+
return
53+
}
54+
55+
// Deps commits are cherry-picks/backports/updates from upstream projects
56+
// (V8, etc.) and are not expected to have a Signed-off-by. When deps
57+
// is mixed with other subsystems, a sign-off might be required but we'll
58+
// downgrade to a warn instead of fail in this case.
59+
const hasDeps = parsed.subsystems.includes('deps')
60+
if (hasDeps && parsed.subsystems.every((s) => s === 'deps')) {
61+
context.report({
62+
id,
63+
message: 'skipping sign-off for deps commit',
64+
string: '',
65+
level: 'skip'
66+
})
67+
return
68+
}
69+
70+
// Backport commits (identified by a Backport-PR-URL trailer) are
71+
// cherry-picks of existing commits into release branches. The
72+
// original commit was already validated.
73+
if (parsed.body.some((line) => backportPattern.test(line))) {
74+
context.report({
75+
id,
76+
message: 'skipping sign-off for backport commit',
77+
string: '',
78+
level: 'skip'
79+
})
80+
return
81+
}
82+
83+
const signoffs = parsed.body
84+
.map((line, i) => [line, i])
85+
.filter(([line]) => signoffPattern.test(line))
86+
87+
// Bot-authored commits don't need a sign-off.
88+
// If they have one, warn; otherwise pass.
89+
if (isBotAuthor(parsed.author)) {
90+
if (signoffs.length === 0) {
91+
context.report({
92+
id,
93+
message: 'skipping sign-off for bot commit',
94+
string: '',
95+
level: 'pass'
96+
})
97+
} else {
98+
for (const [line, lineNum] of signoffs) {
99+
context.report({
100+
id,
101+
message: 'bot commit should not have a "Signed-off-by" trailer',
102+
string: line,
103+
line: lineNum,
104+
column: 0,
105+
level: 'warn'
106+
})
107+
}
108+
}
109+
return
110+
}
111+
112+
// Assume it's not a bot commit... a Signed-off-by trailer is required.
113+
// For mixed deps commits (deps + other subsystems), downgrade to warn
114+
// since the deps portion may legitimately lack a sign-off.
115+
if (signoffs.length === 0) {
116+
context.report({
117+
id,
118+
message: hasDeps
119+
? 'Commit with non-deps changes should have a "Signed-off-by" trailer'
120+
: 'Commit must have a "Signed-off-by" trailer',
121+
string: '',
122+
level: hasDeps ? 'warn' : 'fail'
123+
})
124+
return
125+
}
126+
127+
// Flag every sign-off that has an invalid email format.
128+
// Collect valid sign-offs for further checks.
129+
const valid = []
130+
for (const [line, lineNum] of signoffs) {
131+
if (validSignoff.test(line)) {
132+
valid.push([line, lineNum])
133+
} else {
134+
context.report({
135+
id,
136+
message: '"Signed-off-by" trailer has invalid email',
137+
string: line,
138+
line: lineNum,
139+
column: 0,
140+
level: 'fail'
141+
})
142+
}
143+
}
144+
145+
if (valid.length === 0) {
146+
// All sign-offs had invalid emails; already reported above.
147+
return
148+
}
149+
150+
// Flag any sign-off that appears to be from a bot or AI agent.
151+
// Bots and AI agents are not permitted to sign off on commits.
152+
// Collect non-bot sign-offs for further checks. If the commit
153+
// itself appears to be from a bot, the case is handled above.
154+
const human = []
155+
for (const [line, lineNum] of valid) {
156+
const { 1: name, 2: email } = line.match(signoffParts)
157+
if (botNamePattern.test(name) || botEmailPattern.test(email)) {
158+
context.report({
159+
id,
160+
message: '"Signed-off-by" must be from a human author, ' +
161+
'not a bot or AI agent',
162+
string: line,
163+
line: lineNum,
164+
column: 0,
165+
level: 'warn'
166+
})
167+
} else {
168+
human.push([line, lineNum])
169+
}
170+
}
171+
172+
// All sign-offs appear to be from bots; already reported above.
173+
// If there are no human sign-offs, fail (or warn for mixed deps).
174+
if (human.length === 0) {
175+
context.report({
176+
id,
177+
message: hasDeps
178+
? 'Commit with non-deps changes should have a "Signed-off-by" ' +
179+
'trailer from a human author'
180+
: 'Commit must have a "Signed-off-by" trailer from a human author',
181+
string: '',
182+
level: hasDeps ? 'warn' : 'fail'
183+
})
184+
return
185+
}
186+
187+
// When author info is available, warn if none of the human sign-off
188+
// emails match the commit author email. This may indicate an automated
189+
// tool signed off on behalf of the author.
190+
const authorEmail = parseAuthor(parsed.author)?.email
191+
if (authorEmail) {
192+
const authorMatch = human.some(([line]) => {
193+
const { 2: email } = line.match(signoffParts)
194+
return email.toLowerCase() === authorEmail
195+
})
196+
if (!authorMatch) {
197+
context.report({
198+
id,
199+
message: '"Signed-off-by" email does not match the ' +
200+
'commit author email',
201+
string: human[0][0],
202+
line: human[0][1],
203+
column: 0,
204+
level: 'warn'
205+
})
206+
return
207+
}
208+
}
209+
210+
context.report({
211+
id,
212+
message: 'has valid Signed-off-by',
213+
string: '',
214+
level: 'pass'
215+
})
216+
}
217+
}

lib/validator.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import Parser from 'gitlint-parser-node'
33
import BaseRule from './rule.js'
44

55
// Rules
6+
import assistedByIsTrailer from './rules/assisted-by-is-trailer.js'
67
import coAuthoredByIsTrailer from './rules/co-authored-by-is-trailer.js'
78
import fixesUrl from './rules/fixes-url.js'
89
import lineAfterTitle from './rules/line-after-title.js'
@@ -11,17 +12,20 @@ import metadataEnd from './rules/metadata-end.js'
1112
import prUrl from './rules/pr-url.js'
1213
import reviewers from './rules/reviewers.js'
1314
import subsystem from './rules/subsystem.js'
15+
import signedOffBy from './rules/signed-off-by.js'
1416
import titleFormat from './rules/title-format.js'
1517
import titleLength from './rules/title-length.js'
1618

1719
const RULES = {
20+
'assisted-by-is-trailer': assistedByIsTrailer,
1821
'co-authored-by-is-trailer': coAuthoredByIsTrailer,
1922
'fixes-url': fixesUrl,
2023
'line-after-title': lineAfterTitle,
2124
'line-length': lineLength,
2225
'metadata-end': metadataEnd,
2326
'pr-url': prUrl,
2427
reviewers,
28+
'signed-off-by': signedOffBy,
2529
subsystem,
2630
'title-format': titleFormat,
2731
'title-length': titleLength

test/cli-test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ test('Test cli flags', (t) => {
155155
t.test('test stdin with valid JSON', (tt) => {
156156
const validCommit = {
157157
id: '2b98d02b52',
158-
message: 'stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell <jasnell@gmail.com>\nReviewed-By: Matteo Collina <matteo.collina@gmail.com>'
158+
message: 'stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nSigned-off-by: Calvin Metcalf <cmetcalf@appgeo.com>\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell <jasnell@gmail.com>\nReviewed-By: Matteo Collina <matteo.collina@gmail.com>'
159159
}
160160
const input = JSON.stringify([validCommit])
161161

@@ -211,11 +211,11 @@ test('Test cli flags', (t) => {
211211
const commits = [
212212
{
213213
id: 'commit1',
214-
message: 'doc: update README\n\nPR-URL: https://github.com/nodejs/node/pull/1111\nReviewed-By: Someone <someone@example.com>'
214+
message: 'doc: update README\n\nSigned-off-by: Someone <someone@example.com>\nPR-URL: https://github.com/nodejs/node/pull/1111\nReviewed-By: Someone <someone@example.com>'
215215
},
216216
{
217217
id: 'commit2',
218-
message: 'test: add new test case\n\nPR-URL: https://github.com/nodejs/node/pull/2222\nReviewed-By: Someone <someone@example.com>'
218+
message: 'test: add new test case\n\nSigned-off-by: Someone <someone@example.com>\nPR-URL: https://github.com/nodejs/node/pull/2222\nReviewed-By: Someone <someone@example.com>'
219219
}
220220
]
221221
const input = JSON.stringify(commits)
@@ -337,7 +337,7 @@ test('Test cli flags', (t) => {
337337
t.test('test stdin with --no-validate-metadata', (tt) => {
338338
const commit = {
339339
id: 'novalidate',
340-
message: 'doc: update README\n\nThis commit has no PR-URL or reviewers'
340+
message: 'doc: update README\n\nThis commit has no PR-URL or reviewers\n\nSigned-off-by: Someone <someone@example.com>'
341341
}
342342
const input = JSON.stringify([commit])
343343

test/fixtures/commit.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"sha": "d3f20ccfaa7b0919a7c5a472e344b7de8829b30c",
1717
"url": "https://api.github.com/repos/nodejs/node/git/trees/d3f20ccfaa7b0919a7c5a472e344b7de8829b30c"
1818
},
19-
"message": "stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell <jasnell@gmail.com>\nReviewed-By: Matteo Collina <matteo.collina@gmail.com>",
19+
"message": "stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nSigned-off-by: Calvin Metcalf <cmetcalf@appgeo.com>\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell <jasnell@gmail.com>\nReviewed-By: Matteo Collina <matteo.collina@gmail.com>",
2020
"parents": [
2121
{
2222
"sha": "ec2822adaad76b126b5cccdeaa1addf2376c9aa6",

test/fixtures/pr.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"email": "cmetcalf@appgeo.com",
1313
"date": "2016-04-13T16:33:55Z"
1414
},
15-
"message": "stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell <jasnell@gmail.com>\nReviewed-By: Matteo Collina <matteo.collina@gmail.com>",
15+
"message": "stream: make null an invalid chunk to write in object mode\n\nthis harmonizes behavior between readable, writable, and transform\nstreams so that they all handle nulls in object mode the same way by\nconsidering them invalid chunks.\n\nSigned-off-by: Calvin Metcalf <cmetcalf@appgeo.com>\nPR-URL: https://github.com/nodejs/node/pull/6170\nReviewed-By: James M Snell <jasnell@gmail.com>\nReviewed-By: Matteo Collina <matteo.collina@gmail.com>",
1616
"tree": {
1717
"sha": "e4f9381fdd77d1fd38fe27a80dc43486ac732d48",
1818
"url": "https://api.github.com/repos/nodejs/node/git/trees/e4f9381fdd77d1fd38fe27a80dc43486ac732d48"

0 commit comments

Comments
 (0)