From 720c460639c4fa69249e40c94687c5642cf7cc8c Mon Sep 17 00:00:00 2001 From: Kyle Harding Date: Sun, 8 Dec 2024 19:03:18 -0500 Subject: [PATCH] Remove body match requirement from approved reviews If the changes are approved for merge, there is no need to also provide the /deploy string in the body. Change-type: patch Signed-off-by: Kyle Harding --- README.md | 8 +++++--- src/handlers/deployment-protection-rule.ts | 11 +++++++---- src/handlers/pull-request-review.ts | 7 ++++++- tests/handlers/deployment-protection-rule.test.ts | 1 - 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index e4e3eec..8529f0c 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Deploynaut functions as a GitHub App, managing deployment approvals via [custom ### Approval Process -Deployments are approved by submitting a review with the `/deploy` command. +Deployments are approved by submitting an approved review, or a `/deploy` command in a commented review. #### Deployment Validation @@ -30,7 +30,9 @@ Deployments are auto-approved if: For manual approvals: -- An eligible reviewer submits a [review](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request#submitting-your-review) with the `/deploy` command. +- An eligible reviewer submits a [review](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request#submitting-your-review). + - Commented reviews need to start with `/deploy`. + - Approved reviews do not need to match any string. - The app approves pending deployments matching the reviewed commit SHA. #### Eligible Reviewers @@ -56,7 +58,7 @@ Key security features include: 1. Developer opens a PR and triggers a deployment. 2. App receives a deployment protection rule event. 3. If not auto-approved, the app comments on the PR with instructions. -4. Eligible reviewer submits a review with `/deploy`. +4. Eligible reviewer submits a commentedreview with `/deploy`. 5. App validates the approval and enables deployment. ## Setup diff --git a/src/handlers/deployment-protection-rule.ts b/src/handlers/deployment-protection-rule.ts index 9305891..ab79ea1 100644 --- a/src/handlers/deployment-protection-rule.ts +++ b/src/handlers/deployment-protection-rule.ts @@ -5,8 +5,8 @@ import assert from 'assert'; export const instructionalComment = 'One or more environments require approval before deploying workflow runs.\n\n' + - 'Maintainers, please inspect changes carefully for improper handling of sensitive information and submit a review with the required string.\n\n' + - '> **Files changed -> Review changes -> Comment** -> `/deploy`'; + 'Maintainers, please inspect changes carefully for improper handling of secrets or other sensitive information.\n\n' + + 'To approve pending deployments, submit an approved review, or a commented review with `/deploy`.'; export async function handleDeploymentProtectionRule( context: Context, @@ -83,13 +83,16 @@ export async function handleDeploymentProtectionRule( ); // Find an eligible review authored by a different user than the commit author or committer + // Comment reviews need to start with /deploy + // Approved reviews do not need to match any string const deployReview = reviews.find( (review) => - ['approved', 'commented'].includes(review.state.toLowerCase()) && review.commit_id === deployment.sha && review.user.id !== commit.author?.id && review.user.id !== commit.committer?.id && - review.body?.startsWith('/deploy'), + (review.state.toLowerCase() === 'approved' || + (review.state.toLowerCase() === 'commented' && + review.body?.startsWith('/deploy'))), ); if (deployReview) { diff --git a/src/handlers/pull-request-review.ts b/src/handlers/pull-request-review.ts index 0884833..ace27b8 100644 --- a/src/handlers/pull-request-review.ts +++ b/src/handlers/pull-request-review.ts @@ -31,7 +31,12 @@ export async function handlePullRequestReview(context: Context) { return; } - if (!review.body?.startsWith('/deploy')) { + // Comment reviews need to start with /deploy + // Approved reviews do not need to match any string + if ( + review.state.toLowerCase() === 'commented' && + !review.body?.startsWith('/deploy') + ) { context.log.debug('Ignoring unsupported comment'); return; } diff --git a/tests/handlers/deployment-protection-rule.test.ts b/tests/handlers/deployment-protection-rule.test.ts index 1833dbc..0e79ee5 100644 --- a/tests/handlers/deployment-protection-rule.test.ts +++ b/tests/handlers/deployment-protection-rule.test.ts @@ -192,7 +192,6 @@ describe('Deployment Protection Rule Handler', () => { .reply(200, [ { commit_id: 'test-sha', - body: '/deploy please', state: 'APPROVED', user: { login: 'test-user', id: 789 }, },