Skip to content

Commit

Permalink
Add sign-off enforcement (#74)
Browse files Browse the repository at this point in the history
This patch adds a flag to enforce a sign-off of a commit. See [1] for
more information.

Fixes #73.

[1]: https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff
  • Loading branch information
mristin authored Jan 12, 2021
1 parent bf5029f commit 10e4399
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 13 deletions.
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ inputs:
description: 'If set to "true", allows one-liner commit messages without body'
required: false
default: ''
enforce-sign-off:
description: 'If set to "true", signed-off-by is required in the commit message body'
required: false
default: ''
2 changes: 2 additions & 0 deletions src/__tests__/input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ it('parses the inputs.', () => {
const maybeInputs = input.parseInputs(
'integrate\nanalyze',
pathToVerbs,
'true',
'true'
);

Expand All @@ -45,4 +46,5 @@ it('parses the inputs.', () => {
new Set<string>(['rewrap', 'table', 'integrate', 'analyze'])
);
expect(inputs.allowOneLiners).toBeTruthy();
expect(inputs.enforceSignOff).toBeTruthy();
});
60 changes: 53 additions & 7 deletions src/__tests__/inspection.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as input from '../input';
import * as inspection from '../inspection';

const defaultInputs: input.Inputs = input.parseInputs('', '', '').mustInputs();
const defaultInputs: input.Inputs = input
.parseInputs('', '', '', '')
.mustInputs();

it('reports no errors on correct multi-line message.', () => {
const message =
Expand All @@ -26,7 +28,7 @@ it('reports no errors on OK multi-line message with allowed one-liners.', () =>
});

it('reports no errors on OK single-line message with allowed one-liners.', () => {
const inputs = input.parseInputs('', '', 'true').mustInputs();
const inputs = input.parseInputs('', '', 'true', '').mustInputs();

const message = 'Change SomeClass to OtherClass';

Expand Down Expand Up @@ -60,7 +62,7 @@ it('reports missing body with disallowed one-liners.', () => {
});

it('reports missing body with allowed one-liners.', () => {
const inputs = input.parseInputs('', '', 'true').mustInputs();
const inputs = input.parseInputs('', '', 'true', '').mustInputs();

const message = 'Change SomeClass to OtherClass\n';
const errors = inspection.check(message, inputs);
Expand Down Expand Up @@ -146,7 +148,7 @@ it(
'reports the subject starting with a non-verb ' +
'with additional verbs given as direct input.',
() => {
const inputs = input.parseInputs('table', '', 'false').mustInputs();
const inputs = input.parseInputs('table', '', 'false', '').mustInputs();

const message =
'Replaced SomeClass to OtherClass\n' +
Expand Down Expand Up @@ -183,7 +185,8 @@ it(
false,
'/some/path',
false,
new Set<string>('table')
new Set<string>('table'),
false
);

const message =
Expand Down Expand Up @@ -214,7 +217,7 @@ it(
);

it('accepts the subject starting with an additional verb.', () => {
const inputs = input.parseInputs('table', '', 'false').mustInputs();
const inputs = input.parseInputs('table', '', 'false', '').mustInputs();

const message = 'Table that for me\n\nThis is a dummy commit.';
const errors = inspection.check(message, inputs);
Expand All @@ -236,7 +239,7 @@ it('reports the subject ending in a dot.', () => {
});

it('reports an incorrect one-line message with allowed one-liners.', () => {
const inputs = input.parseInputs('', '', 'true').mustInputs();
const inputs = input.parseInputs('', '', 'true', '').mustInputs();

const message = 'Change SomeClass to OtherClass.';

Expand Down Expand Up @@ -409,3 +412,46 @@ The ${long} line is too long.`;
'Please reformat the body so that all the lines fit 72 characters.'
]);
});

it('accepts the valid body when enforcing the sign-off.', () => {
const inputs = input.parseInputs('', '', '', 'true').mustInputs();

const message = `Do something
It does something.
Signed-off-by: Somebody <[email protected]>
Signed-off is not necessarily the last line.
And multiple sign-offs are possible!
Signed-off-by: Somebody Else <[email protected]>
`;

const errors = inspection.check(message, inputs);
expect(errors).toEqual([]);
});

it('rejects invalid sign-offs.', () => {
const inputs = input.parseInputs('', '', '', 'true').mustInputs();

const message = `Do something
It does something.
None of the following satisfy the sign-off:
Signed-off-by Random Developer <[email protected]>
signed-off-by: Random Developer <[email protected]>
Signed-off-by: Random Developer <randomdeveloper.example.org>
Signed-off-by: Random Developer ([email protected])
Signed-off-by: Random Developer [email protected]
Signed off by: Random Developer <[email protected]>
Signed_off_by: Random Developer <[email protected]>
Signed-off-by: Random Developer
`;

const errors = inspection.check(message, inputs);
expect(errors).toEqual([
"The body does not contain any 'Signed-off-by: ' line. Did you sign off the commit with `git commit --signoff`?"
]);
});
28 changes: 23 additions & 5 deletions src/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,20 @@ export class Inputs {
// specified by the input "path-to-additional-verbs".
additionalVerbs: Set<string>;

public enforceSignOff: boolean;

constructor(
hasAdditionalVerbsInput: boolean,
pathToAdditionalVerbs: string,
allowOneLiners: boolean,
additionalVerbs: Set<string>
additionalVerbs: Set<string>,
enforceSignOff: boolean
) {
this.hasAdditionalVerbsInput = hasAdditionalVerbsInput;
this.pathToAdditionalVerbs = pathToAdditionalVerbs;
this.allowOneLiners = allowOneLiners;
this.additionalVerbs = additionalVerbs;
this.enforceSignOff = enforceSignOff;
}
}

Expand Down Expand Up @@ -56,7 +60,8 @@ export class MaybeInputs {
export function parseInputs(
additionalVerbsInput: string,
pathToAdditionalVerbsInput: string,
allowOneLinersInput: string
allowOneLinersInput: string,
enforceSignOffInput: string
): MaybeInputs {
const additionalVerbs = new Set<string>();

Expand Down Expand Up @@ -86,7 +91,7 @@ export function parseInputs(

const allowOneLiners: boolean | null = !allowOneLinersInput
? false
: parseAllowOneLiners(allowOneLinersInput);
: parseBooleanFromString(allowOneLinersInput);

if (allowOneLiners === null) {
return new MaybeInputs(
Expand All @@ -96,12 +101,25 @@ export function parseInputs(
);
}

const enforceSignOff: boolean | null = !enforceSignOffInput
? false
: parseBooleanFromString(enforceSignOffInput);

if (enforceSignOff === null) {
return new MaybeInputs(
null,
'Unexpected value for enforce-sign-off. ' +
`Expected either 'true' or 'false', got: ${enforceSignOffInput}`
);
}

return new MaybeInputs(
new Inputs(
hasAdditionalVerbsInput,
pathToAdditionalVerbsInput,
allowOneLiners,
additionalVerbs
additionalVerbs,
enforceSignOff
),
null
);
Expand All @@ -123,7 +141,7 @@ export function parseVerbs(text: string): string[] {
return verbs;
}

export function parseAllowOneLiners(text: string): boolean | null {
function parseBooleanFromString(text: string): boolean | null {
if (text === '' || text.toLowerCase() === 'false' || text === '0') {
return false;
}
Expand Down
28 changes: 28 additions & 0 deletions src/inspection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,30 @@ function checkBody(subject: string, bodyLines: string[]): string[] {
return errors;
}

const signedOffByRe = new RegExp(
'^\\s*Signed-off-by:\\s*[^<]+\\s*<[^@>, ]+@[^@>, ]+>\\s*$'
);

function checkSignedOff(bodyLines: string[]): string[] {
const errors: string[] = [];

let matches = 0;
for (const line of bodyLines) {
if (signedOffByRe.test(line)) {
matches++;
}
}

if (matches === 0) {
errors.push(
"The body does not contain any 'Signed-off-by: ' line. " +
'Did you sign off the commit with `git commit --signoff`?'
);
}

return errors;
}

const mergeMessageRe = new RegExp(
"^Merge branch '[^\\000-\\037\\177 ~^:?*[]+' " +
'into [^\\000-\\037\\177 ~^:?*[]+$'
Expand Down Expand Up @@ -297,6 +321,10 @@ export function check(message: string, inputs: input.Inputs): string[] {
errors.push(...checkSubject(subjectBody.subject, inputs));

errors.push(...checkBody(subjectBody.subject, subjectBody.bodyLines));

if (inputs.enforceSignOff) {
errors.push(...checkSignedOff(subjectBody.bodyLines));
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/mainImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ function runWithExceptions(): void {
const allowOneLinersInput =
core.getInput('allow-one-liners', {required: false}) ?? '';

const enforceSignOffInput =
core.getInput('enforce-sign-off', {required: false}) ?? '';

const maybeInputs = input.parseInputs(
additionalVerbsInput,
pathToAdditionalVerbsInput,
allowOneLinersInput
allowOneLinersInput,
enforceSignOffInput
);

if (maybeInputs.error !== null) {
Expand Down

0 comments on commit 10e4399

Please sign in to comment.