Skip to content

Commit

Permalink
vulnerability evaluator: Submit reviews with just comment if the auth…
Browse files Browse the repository at this point in the history
…or is the same as the mediator identity

Because github doesn't let you review your own code, in case mediator is
running as the same identity as the contributor submitting the PR, we
can't request changes.

In that case, let's just mark the review as commenting instead of
changes requested. A follow-up patch would then set the commit status to
failed to prevent merge.

Fixes: #951
  • Loading branch information
jhrozek committed Sep 13, 2023
1 parent a5dde0b commit f2aca81
Show file tree
Hide file tree
Showing 5 changed files with 1,905 additions and 1,868 deletions.
1 change: 1 addition & 0 deletions docs/docs/protodocs/proto.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 9 additions & 3 deletions internal/controlplane/handlers_githubwebhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,10 +802,16 @@ func getPullRequestInfoFromPayload(
return nil, fmt.Errorf("error getting pull request number from payload: %w", err)
}

prAuthorId, err := util.JQReadFrom[float64](ctx, ".pull_request.user.id", payload)
if err != nil {
return nil, fmt.Errorf("error getting pull request author ID from payload: %w", err)
}

return &pb.PullRequest{
Url: prUrl,
Number: int32(prNumber),
Patches: nil, // to be filled later with a separate call
Url: prUrl,
Number: int32(prNumber),
AuthorId: int64(prAuthorId),
Patches: nil, // to be filled later with a separate call
}, nil
}

Expand Down
28 changes: 23 additions & 5 deletions internal/engine/eval/vulncheck/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ type reviewPrHandler struct {
pr *pb.PullRequest

mediatorReview *github.PullRequestReview
failStatus *string

comments []*github.DraftReviewComment
status *string
Expand All @@ -157,11 +158,28 @@ func newReviewPrHandler(
Str("repo-name", pr.RepoName).
Logger()

cliUser, err := cli.GetAuthenticatedUser(ctx)
if err != nil {
return nil, fmt.Errorf("could not get authenticated user: %w", err)
}

// if the user wants mediator to request changes on a pull request, they need to
// be different identities
var failStatus *string
if pr.AuthorId == cliUser.GetID() {
failStatus = github.String("COMMENT")
logger.Debug().Msg("author is the same as the authenticated user, can only comment")
} else {
failStatus = github.String("REQUEST_CHANGES")
logger.Debug().Msg("author is different than the authenticated user, can request changes")
}

return &reviewPrHandler{
cli: cli,
pr: pr,
comments: []*github.DraftReviewComment{},
logger: logger,
cli: cli,
pr: pr,
comments: []*github.DraftReviewComment{},
logger: logger,
failStatus: failStatus,
}, nil
}

Expand Down Expand Up @@ -219,7 +237,7 @@ func (ra *reviewPrHandler) submit(ctx context.Context) error {
func (ra *reviewPrHandler) setStatus() {
if len(ra.comments) > 0 {
// if this pass produced comments, request changes
ra.status = github.String("REQUEST_CHANGES")
ra.status = ra.failStatus
ra.text = github.String(reviewBodyRequestChangesCommentText)
} else {
// if this pass produced no comments, resolve the mediator review
Expand Down
Loading

0 comments on commit f2aca81

Please sign in to comment.