diff --git a/src/commit.ts b/src/commit.ts index bd0d5a800..deb0ce5b2 100644 --- a/src/commit.ts +++ b/src/commit.ts @@ -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([ + '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] || '' ) diff --git a/src/github.ts b/src/github.ts index 07035fd04..cbb678ba0 100644 --- a/src/github.ts +++ b/src/github.ts @@ -112,6 +112,7 @@ interface GraphQLPullRequest { hasNextPage: boolean; }; }; + authorAssociation?: string; } interface CommitHistory { @@ -265,6 +266,7 @@ export class GitHub implements Scm { } } body + authorAssociation mergeCommit { oid } @@ -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) { diff --git a/src/pull-request.ts b/src/pull-request.ts index 9176207d4..1337c81b6 100644 --- a/src/pull-request.ts +++ b/src/pull-request.ts @@ -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; @@ -22,4 +32,5 @@ export interface PullRequest { readonly labels: string[]; readonly files: string[]; readonly sha?: string; + readonly authorAssociation?: PullRequestAuthorAssociation; } diff --git a/test/commits.ts b/test/commits.ts index 55ecdeacb..6c7c2b950 100644 --- a/test/commits.ts +++ b/test/commits.ts @@ -207,6 +207,7 @@ describe('parseConventionalCommits', () => { labels: [], files: [], body, + authorAssociation: 'MEMBER', }; const conventionalCommits = parseConventionalCommits([commit]); @@ -227,6 +228,7 @@ describe('parseConventionalCommits', () => { labels: [], files: [], body, + authorAssociation: 'OWNER', }; const conventionalCommits = parseConventionalCommits([commit]); @@ -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); diff --git a/test/manifest.ts b/test/manifest.ts index 2906eb5e7..cee28e030 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -3029,6 +3029,7 @@ describe('Manifest', () => { labels: [], files: [], sha: 'abc123', + authorAssociation: 'MEMBER', }, }, {