Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove body match requirement from approved reviews #15

Merged
merged 2 commits into from
Dec 9, 2024
Merged
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
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand Down
22 changes: 16 additions & 6 deletions src/handlers/deployment-protection-rule.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import type { Context } from 'probot';
import type { DeploymentProtectionRuleRequestedEvent } from '@octokit/webhooks-types';
import * as GitHubClient from '../client.js';
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,
Expand Down Expand Up @@ -51,9 +52,15 @@ export async function handleDeploymentProtectionRule(
// Get the commit that triggered the workflow run
const commit = await GitHubClient.getCommit(context, deployment.sha);

assert(commit.author, `Failed to get author for SHA: ${deployment.sha}`);
assert(
commit.committer,
`Failed to get committer for SHA: ${deployment.sha}`,
);

// Approve deployment if the commit author is in the bypass list
const bypassActors = process.env.BYPASS_ACTORS?.split(',') ?? [];
if (commit.author?.id && bypassActors.includes(commit.author.id.toString())) {
if (bypassActors.includes(commit.author.id.toString())) {
return context.octokit.request(`POST ${callbackUrl}`, {
environment_name: environment,
state: 'approved',
Expand All @@ -63,7 +70,7 @@ export async function handleDeploymentProtectionRule(

context.log.debug(
'Commit author is not included in bypass actors: %s',
commit.author?.login,
commit.author.login,
);

const client = await context.octokit.apps.getAuthenticated();
Expand All @@ -76,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) {
Expand Down
18 changes: 15 additions & 3 deletions src/handlers/pull-request-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
WorkflowRun,
} from '@octokit/webhooks-types';
import * as GitHubClient from '../client.js';
import assert from 'assert';

export async function handlePullRequestReview(context: Context) {
const { review } = context.payload as PullRequestReviewSubmittedEvent;
Expand All @@ -30,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;
}
Expand All @@ -43,15 +49,21 @@ export async function handlePullRequestReview(context: Context) {
// Get the commit of the submitted review
const commit = await GitHubClient.getCommit(context, review.commit_id);

if (commit.author?.id === review.user.id) {
assert(commit.author, `Failed to get author for SHA: ${review.commit_id}`);
assert(
commit.committer,
`Failed to get committer for SHA: ${review.commit_id}`,
);

if (commit.author.id === review.user.id) {
context.log.debug(
'Ignoring review by author of the commit: %s',
review.user.login,
);
return;
}

if (commit.committer?.id === review.user.id) {
if (commit.committer.id === review.user.id) {
context.log.debug(
'Ignoring review by committer of the commit: %s',
review.user.login,
Expand Down
1 change: 0 additions & 1 deletion tests/handlers/deployment-protection-rule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
},
Expand Down
Loading