From 421d21f2636950925a40aedd615247af23ad86f7 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:26:05 +0200 Subject: [PATCH 1/8] added logic to obtain whitelisted users --- src/bot.ts | 11 ++++++++++- src/github/comments.ts | 5 +---- src/index.ts | 13 +++++++++++-- src/util.ts | 10 +++++++++- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/bot.ts b/src/bot.ts index 2e117fc..24387f2 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -24,6 +24,7 @@ export class Bot { private readonly pr: Issue, private readonly logger: ActionLogger, private readonly commentsApi: CommentsApi, + private readonly whitelistedUsers: string[], private readonly actionUrl: string, ) {} @@ -37,6 +38,14 @@ export class Bot { } this.logger.debug("Author of comment is not the author of the PR"); + if (this.whitelistedUsers && this.whitelistedUsers.length > 0) { + if (this.whitelistedUsers.indexOf(this.comment.user.login) > -1) { + this.logger.debug("User belongs to whitelisted users"); + return true; + } + this.logger.debug("User does not belong to list of whitelisted users"); + } + return await this.commentsApi.userBelongsToOrg(this.comment.user.login); } @@ -71,7 +80,7 @@ export class Bot { } this.logger.debug("User can trigger bot"); - const [_, command] = this.comment.body.split(" "); + const [, command] = this.comment.body.split(" "); try { switch (command as Command) { // Simply `/merge` diff --git a/src/github/comments.ts b/src/github/comments.ts index 311e179..49c5a63 100644 --- a/src/github/comments.ts +++ b/src/github/comments.ts @@ -14,10 +14,7 @@ export class CommentsApi { * @param message Message to write in the comment * @param overrideSilentMode If silent mode should be overriden */ - async comment( - message: string, - overrideSilentMode: boolean = false, - ): Promise { + async comment(message: string, overrideSilentMode = false): Promise { this.logger.info("Commenting: " + message); if (!this.silentMode || overrideSilentMode) { await this.api.rest.issues.createComment({ diff --git a/src/index.ts b/src/index.ts index d337a99..7858b27 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ import { Issue, IssueComment } from "@octokit/webhooks-types"; import { Bot } from "./bot"; import { CommentsApi } from "./github/comments"; import { Merger } from "./github/merger"; -import { generateCoreLogger } from "./util"; +import { generateCoreLogger, getWhitelistedUsers } from "./util"; const getRepo = (ctx: Context) => { let repo = getInput("repo", { required: false }); @@ -60,6 +60,8 @@ logger.info( }`, ); +const whitelistedUsers = getWhitelistedUsers(); + const actionUrl = `${context.serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${context.runId}`; if (context.payload.comment) { @@ -79,7 +81,14 @@ if (context.payload.comment) { headers: { authorization: `token ${token}` }, }) as graphql; const merger = new Merger(issue.node_id, gql, logger, getMergeMethod()); - const bot = new Bot(comment, issue, logger, commentsApi, actionUrl); + const bot = new Bot( + comment, + issue, + logger, + commentsApi, + whitelistedUsers, + actionUrl, + ); bot .run(merger) .then(() => logger.info("Finished!")) diff --git a/src/util.ts b/src/util.ts index 1197efc..eaebeae 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,7 +1,15 @@ -import { debug, error, info, warning } from "@actions/core"; +import { debug, error, getInput, info, warning } from "@actions/core"; import { ActionLogger } from "./github/types"; export function generateCoreLogger(): ActionLogger { return { info, debug, warn: warning, error }; } + +export function getWhitelistedUsers(): string[] { + const users = getInput("WHITELISTED_USERS", { required: false }); + if (users) { + return users.split(","); + } + return []; +} From 2cbaf88ae91a0575c6ce22b7b248759ff92f203c Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:26:44 +0200 Subject: [PATCH 2/8] added @typescript-eslint/strict-boolean-expressions rule to 0 --- .eslintrc.cjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 6774691..9ec2e2e 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -8,6 +8,9 @@ const tsConfParams = { rootDir: __dirname }; const conf = getConfiguration({ typescript: tsConfParams }); const tsConfOverride = getTypescriptOverride(tsConfParams); -conf.overrides.push(tsConfOverride); +conf.overrides.push({ + ...tsConfOverride, + rules: { "@typescript-eslint/strict-boolean-expressions": 0 }, +}); module.exports = conf; From 533a80c4f88df19b2d7821c086762972ce037c04 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:29:33 +0200 Subject: [PATCH 3/8] updated input field --- action.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/action.yml b/action.yml index 16f4821..ac3b9ee 100644 --- a/action.yml +++ b/action.yml @@ -15,6 +15,9 @@ inputs: SILENT: required: false description: If true, the bot will not post a comment on the PR. + WHITELISTED_USERS: + required: false + description: List of users which are allowed to use the bot. Separated by commas (abc,def,ghi) outputs: repo: description: 'The name of the repo in owner/repo pattern' From d49759376c55f7b68cb77db2495c9b9c06dcb762 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:29:39 +0200 Subject: [PATCH 4/8] updated readme --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 7383f01..1518167 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,9 @@ You can find all the inputs in [the action file](./action.yml), but let's walk t - Make sure that the type of merge you selected is available in the repository merge options. - `SILENT`: If the bot should be silent and not comment when enabling/disabling auto-merge. - **Optional**: Defaults to `false`. +- `WHITELISTED_USERS`: List of user accounts which are allowed to use the bot aside from the author and org members. + - **Optional** + - Must be a comma separated value: `user-1,user-2,user-3`. ## Usage From da68e206bc0850ed0a443bcea81d61a88714386a Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:33:19 +0200 Subject: [PATCH 5/8] renamed whitelist to allowlist --- README.md | 2 +- action.yml | 2 +- src/bot.ts | 10 +++++----- src/index.ts | 6 +++--- src/util.ts | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 1518167..4fc17a3 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ You can find all the inputs in [the action file](./action.yml), but let's walk t - Make sure that the type of merge you selected is available in the repository merge options. - `SILENT`: If the bot should be silent and not comment when enabling/disabling auto-merge. - **Optional**: Defaults to `false`. -- `WHITELISTED_USERS`: List of user accounts which are allowed to use the bot aside from the author and org members. +- `ALLOWLIST`: List of user accounts which are allowed to use the bot aside from the author and org members. - **Optional** - Must be a comma separated value: `user-1,user-2,user-3`. diff --git a/action.yml b/action.yml index ac3b9ee..2445454 100644 --- a/action.yml +++ b/action.yml @@ -15,7 +15,7 @@ inputs: SILENT: required: false description: If true, the bot will not post a comment on the PR. - WHITELISTED_USERS: + ALLOWLIST: required: false description: List of users which are allowed to use the bot. Separated by commas (abc,def,ghi) outputs: diff --git a/src/bot.ts b/src/bot.ts index 24387f2..06bc312 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -24,7 +24,7 @@ export class Bot { private readonly pr: Issue, private readonly logger: ActionLogger, private readonly commentsApi: CommentsApi, - private readonly whitelistedUsers: string[], + private readonly allowlistedUsers: string[], private readonly actionUrl: string, ) {} @@ -38,12 +38,12 @@ export class Bot { } this.logger.debug("Author of comment is not the author of the PR"); - if (this.whitelistedUsers && this.whitelistedUsers.length > 0) { - if (this.whitelistedUsers.indexOf(this.comment.user.login) > -1) { - this.logger.debug("User belongs to whitelisted users"); + if (this.allowlistedUsers && this.allowlistedUsers.length > 0) { + if (this.allowlistedUsers.indexOf(this.comment.user.login) > -1) { + this.logger.debug("User belongs to allowlisted users"); return true; } - this.logger.debug("User does not belong to list of whitelisted users"); + this.logger.debug("User does not belong to list of allowlisted users"); } return await this.commentsApi.userBelongsToOrg(this.comment.user.login); diff --git a/src/index.ts b/src/index.ts index 7858b27..7710034 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ import { Issue, IssueComment } from "@octokit/webhooks-types"; import { Bot } from "./bot"; import { CommentsApi } from "./github/comments"; import { Merger } from "./github/merger"; -import { generateCoreLogger, getWhitelistedUsers } from "./util"; +import { generateCoreLogger, getallowlistedUsers } from "./util"; const getRepo = (ctx: Context) => { let repo = getInput("repo", { required: false }); @@ -60,7 +60,7 @@ logger.info( }`, ); -const whitelistedUsers = getWhitelistedUsers(); +const allowlistedUsers = getallowlistedUsers(); const actionUrl = `${context.serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${context.runId}`; @@ -86,7 +86,7 @@ if (context.payload.comment) { issue, logger, commentsApi, - whitelistedUsers, + allowlistedUsers, actionUrl, ); bot diff --git a/src/util.ts b/src/util.ts index eaebeae..6f5adfd 100644 --- a/src/util.ts +++ b/src/util.ts @@ -6,8 +6,8 @@ export function generateCoreLogger(): ActionLogger { return { info, debug, warn: warning, error }; } -export function getWhitelistedUsers(): string[] { - const users = getInput("WHITELISTED_USERS", { required: false }); +export function getallowlistedUsers(): string[] { + const users = getInput("ALLOWLIST", { required: false }); if (users) { return users.split(","); } From 19c815ed97091202f70872bbefc81ffe1643c34e Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:33:30 +0200 Subject: [PATCH 6/8] added opstooling team to list of allowlisted users --- .github/workflows/auto-merge-bot.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/auto-merge-bot.yml b/.github/workflows/auto-merge-bot.yml index 820cf4c..a60c437 100644 --- a/.github/workflows/auto-merge-bot.yml +++ b/.github/workflows/auto-merge-bot.yml @@ -15,3 +15,5 @@ jobs: uses: paritytech/auto-merge-bot@main with: GITHUB_TOKEN: '${{ github.token }}' + ALLOWLIST: "bullrich,rzadp,mordamax,mutantcornholio" + From 736faa14d6f0c5b5ea1780c5d09ebb04aae61423 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:38:24 +0200 Subject: [PATCH 7/8] added more detailed logs --- src/bot.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bot.ts b/src/bot.ts index 06bc312..ab8288b 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -33,19 +33,20 @@ export class Bot { this.logger.debug("Evaluating if user can trigger the bot"); const author = this.pr.user.id; if (this.comment.user.id === author) { - this.logger.debug("Author of comment is also author of PR"); + this.logger.info("Author of comment is also author of PR"); return true; } this.logger.debug("Author of comment is not the author of the PR"); if (this.allowlistedUsers && this.allowlistedUsers.length > 0) { if (this.allowlistedUsers.indexOf(this.comment.user.login) > -1) { - this.logger.debug("User belongs to allowlisted users"); + this.logger.info("User belongs to allowlisted users"); return true; } this.logger.debug("User does not belong to list of allowlisted users"); } + this.logger.debug("Evaluating if author of comment is a public member of the org") return await this.commentsApi.userBelongsToOrg(this.comment.user.login); } From c17498c418be25018cb8acb7ea213714bf7b7935 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 28 Sep 2023 16:40:54 +0200 Subject: [PATCH 8/8] linted code --- src/bot.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bot.ts b/src/bot.ts index ab8288b..e0668e5 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -46,7 +46,9 @@ export class Bot { this.logger.debug("User does not belong to list of allowlisted users"); } - this.logger.debug("Evaluating if author of comment is a public member of the org") + this.logger.debug( + "Evaluating if author of comment is a public member of the org", + ); return await this.commentsApi.userBelongsToOrg(this.comment.user.login); }