Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/commit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,9 +453,25 @@ export function parseConventionalCommits(
return conventionalCommits;
}

// Author associations whose BEGIN_COMMIT_OVERRIDE body section is honored.
// Only roles that already have write access to the repository are trusted:
// the PR body of a merged PR remains editable by its author indefinitely,
// so honoring overrides from contributor/first-time-contributor/none would
// let an external contributor rewrite their merged commit message (and the
// resulting CHANGELOG / release notes) after review has completed.
const TRUSTED_OVERRIDE_ASSOCIATIONS = new Set<string>([
'OWNER',
'MEMBER',
'COLLABORATOR',
]);

function preprocessCommitMessage(commit: Commit): string {
// look for 'BEGIN_COMMIT_OVERRIDE' section of pull request body
if (commit.pullRequest) {
const association = commit.pullRequest.authorAssociation;
if (!association || !TRUSTED_OVERRIDE_ASSOCIATIONS.has(association)) {
return commit.message;
}
const overrideMessage = (
commit.pullRequest.body.split('BEGIN_COMMIT_OVERRIDE')[1] || ''
)
Expand Down
4 changes: 4 additions & 0 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ interface GraphQLPullRequest {
hasNextPage: boolean;
};
};
authorAssociation?: string;
}

interface CommitHistory {
Expand Down Expand Up @@ -265,6 +266,7 @@ export class GitHub implements Scm {
}
}
body
authorAssociation
mergeCommit {
oid
}
Expand Down Expand Up @@ -382,6 +384,8 @@ export class GitHub implements Scm {
body: pullRequest.body,
labels: pullRequest.labels.nodes.map(node => node.name),
files: (pullRequest.files?.nodes || []).map(node => node.path),
authorAssociation:
pullRequest.authorAssociation as PullRequest['authorAssociation'],
};
}
if (mergePullRequest) {
Expand Down
11 changes: 11 additions & 0 deletions src/pull-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@
// See the License for the specific language governing permissions and
// limitations under the License.

export type PullRequestAuthorAssociation =
| 'OWNER'
| 'MEMBER'
| 'COLLABORATOR'
| 'CONTRIBUTOR'
| 'FIRST_TIME_CONTRIBUTOR'
| 'FIRST_TIMER'
| 'MANNEQUIN'
| 'NONE';

export interface PullRequest {
readonly headBranchName: string;
readonly baseBranchName: string;
Expand All @@ -22,4 +32,5 @@ export interface PullRequest {
readonly labels: string[];
readonly files: string[];
readonly sha?: string;
readonly authorAssociation?: PullRequestAuthorAssociation;
}
44 changes: 44 additions & 0 deletions test/commits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ describe('parseConventionalCommits', () => {
labels: [],
files: [],
body,
authorAssociation: 'MEMBER',
};

const conventionalCommits = parseConventionalCommits([commit]);
Expand All @@ -227,6 +228,7 @@ describe('parseConventionalCommits', () => {
labels: [],
files: [],
body,
authorAssociation: 'OWNER',
};

const conventionalCommits = parseConventionalCommits([commit]);
Expand All @@ -237,6 +239,48 @@ describe('parseConventionalCommits', () => {
expect(conventionalCommits[1].bareMessage).to.eql('another feature');
});

it('ignores BEGIN_COMMIT_OVERRIDE from an untrusted PR author', async () => {
const commit = buildMockCommit('chore: some commit');
const body =
'BEGIN_COMMIT_OVERRIDE\nfix!: forged fix\n\nBREAKING CHANGE: forged\nEND_COMMIT_OVERRIDE';
commit.pullRequest = {
headBranchName: 'fix-something',
baseBranchName: 'main',
number: 123,
title: 'chore: some commit',
labels: [],
files: [],
body,
authorAssociation: 'CONTRIBUTOR',
};

const conventionalCommits = parseConventionalCommits([commit]);
expect(conventionalCommits).lengthOf(1);
expect(conventionalCommits[0].type).to.eql('chore');
expect(conventionalCommits[0].bareMessage).to.eql('some commit');
expect(conventionalCommits[0].breaking).to.be.false;
expect(conventionalCommits[0].notes).lengthOf(0);
});

it('ignores BEGIN_COMMIT_OVERRIDE when authorAssociation is missing', async () => {
const commit = buildMockCommit('chore: some commit');
const body = 'BEGIN_COMMIT_OVERRIDE\nfix: forged fix\nEND_COMMIT_OVERRIDE';
commit.pullRequest = {
headBranchName: 'fix-something',
baseBranchName: 'main',
number: 123,
title: 'chore: some commit',
labels: [],
files: [],
body,
};

const conventionalCommits = parseConventionalCommits([commit]);
expect(conventionalCommits).lengthOf(1);
expect(conventionalCommits[0].type).to.eql('chore');
expect(conventionalCommits[0].bareMessage).to.eql('some commit');
});

it('handles a special commit separator', async () => {
const commits = [buildCommitFromFixture('multiple-commits-with-separator')];
const conventionalCommits = parseConventionalCommits(commits);
Expand Down
1 change: 1 addition & 0 deletions test/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3029,6 +3029,7 @@ describe('Manifest', () => {
labels: [],
files: [],
sha: 'abc123',
authorAssociation: 'MEMBER',
},
},
{
Expand Down
Loading