Skip to content

Commit 292ef85

Browse files
authored
fix(participants): filter out empty entries from code owners (renovatebot#41576)
* fix(participants): filter out empty entries from code owners * add defense in depth
1 parent 18bcf81 commit 292ef85

File tree

4 files changed

+68
-3
lines changed

4 files changed

+68
-3
lines changed

lib/modules/platform/bitbucket-server/index.spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,45 @@ describe('modules/platform/bitbucket-server/index', () => {
980980
await expect(bitbucket.addReviewers(5, ['name'])).toResolve();
981981
});
982982

983+
it('aborts instead of infinite recursion when invalid reviewers cannot be filtered', async () => {
984+
const scope = await initRepo();
985+
scope
986+
.get(
987+
`${urlPath}/rest/api/1.0/projects/SOME/repos/repo/pull-requests/5`,
988+
)
989+
.once()
990+
.reply(200, prMock(url, 'SOME', 'repo'))
991+
.put(
992+
`${urlPath}/rest/api/1.0/projects/SOME/repos/repo/pull-requests/5`,
993+
)
994+
.once()
995+
.reply(409, {
996+
errors: [
997+
{
998+
context: 'reviewers',
999+
message:
1000+
'Errors encountered while adding some reviewers to this pull request.',
1001+
exceptionName:
1002+
'com.atlassian.bitbucket.pull.InvalidPullRequestReviewersException',
1003+
reviewerErrors: [{ context: '', message: ' is not a user.' }],
1004+
validReviewers: [
1005+
{ user: { name: 'user1' } },
1006+
{ user: { name: 'user2' } },
1007+
],
1008+
},
1009+
],
1010+
});
1011+
1012+
await expect(
1013+
bitbucket.addReviewers(5, ['user1', 'user2', '']),
1014+
).rejects.toThrow();
1015+
1016+
expect(logger.logger.warn).toHaveBeenCalledWith(
1017+
expect.anything(),
1018+
'Could not filter invalid reviewers from list, aborting to prevent infinite recursion',
1019+
);
1020+
});
1021+
9831022
it('deals correctly with resolving reviewers', async () => {
9841023
const scope = await initRepo();
9851024
scope

lib/modules/platform/bitbucket-server/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,15 @@ async function updatePRAndAddReviewers(
792792
const filteredReviewers = reviewers.filter(
793793
(name) => !invalidReviewers.includes(name),
794794
);
795-
await updatePRAndAddReviewers(prNo, filteredReviewers);
795+
if (filteredReviewers.length < reviewers.length) {
796+
await updatePRAndAddReviewers(prNo, filteredReviewers);
797+
} else {
798+
logger.warn(
799+
{ invalidReviewers, reviewers },
800+
'Could not filter invalid reviewers from list, aborting to prevent infinite recursion',
801+
);
802+
throw err;
803+
}
796804
} else {
797805
logger.debug(
798806
'409 response to adding reviewers - has repository changed?',

lib/workers/repository/update/pr/participants.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,22 @@ describe('workers/repository/update/pr/participants', () => {
204204
]);
205205
});
206206

207+
it('filters out bare @ from malformed CODEOWNERS entries', async () => {
208+
codeOwners.codeOwnersForPr.mockResolvedValueOnce([
209+
'@UserOne',
210+
'@UserTwo',
211+
'@',
212+
]);
213+
await addParticipants(
214+
{ ...config, reviewers: [], reviewersFromCodeOwners: true },
215+
pr,
216+
);
217+
expect(platform.addReviewers).toHaveBeenCalledExactlyOnceWith(123, [
218+
'UserOne',
219+
'UserTwo',
220+
]);
221+
});
222+
207223
it('supports additionalReviewers', async () => {
208224
await addParticipants(
209225
{ ...config, additionalReviewers: ['foo', 'bar', 'baz'] },

lib/workers/repository/update/pr/participants.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isArray, isNumber } from '@sindresorhus/is';
1+
import { isArray, isNonEmptyString, isNumber } from '@sindresorhus/is';
22
import { GlobalConfig } from '../../../../config/global.ts';
33
import type { RenovateConfig } from '../../../../config/types.ts';
44
import { logger } from '../../../../logger/index.ts';
@@ -36,7 +36,9 @@ function prepareParticipants(
3636
config: RenovateConfig,
3737
usernames: string[],
3838
): Promise<string[]> {
39-
const normalizedUsernames = [...new Set(usernames.map(noLeadingAtSymbol))];
39+
const normalizedUsernames = [
40+
...new Set(usernames.map(noLeadingAtSymbol).filter(isNonEmptyString)),
41+
];
4042
return filterUnavailableUsers(config, normalizedUsernames);
4143
}
4244

0 commit comments

Comments
 (0)