Skip to content

Commit

Permalink
Fix long descriptions in audit statuses (#73)
Browse files Browse the repository at this point in the history
The GitHub API limits status descriptions to 140 characters, but the
status we posted when detecting an overwrite could be longer than that,
depending on the formatted values. Update the audit code to log the full
message but post an abbreviated message that is under the character
limit.
  • Loading branch information
bluekeyes authored Apr 25, 2019
1 parent 5fda37f commit 758ff8c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
4 changes: 3 additions & 1 deletion server/handler/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const (
DefaultPolicyPath = ".policy.yml"
DefaultStatusCheckContext = "policy-bot"
DefaultAppName = "policy-bot"

LogKeyGitHubSHA = "github_sha"
)

type Base struct {
Expand Down Expand Up @@ -114,7 +116,7 @@ func (b *Base) postGitHubRepoStatus(ctx context.Context, client *github.Client,
func (b *Base) PreparePRContext(ctx context.Context, installationID int64, pr *github.PullRequest) (context.Context, zerolog.Logger) {
ctx, logger := githubapp.PreparePRContext(ctx, installationID, pr.GetBase().GetRepo(), pr.GetNumber())

logger = logger.With().Str("github_sha", pr.GetHead().GetSHA()).Logger()
logger = logger.With().Str(LogKeyGitHubSHA, pr.GetHead().GetSHA()).Logger()
ctx = logger.WithContext(ctx)

return ctx, logger
Expand Down
26 changes: 15 additions & 11 deletions server/handler/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,28 @@ func (h *Status) Handle(ctx context.Context, eventType, deliveryID string, paylo
commitSHA := event.GetCommit().GetSHA()

if sender.GetLogin() != h.PullOpts.AppName+"[bot]" {
auditMessage := fmt.Sprintf(
"Entity '%s' overwrote status check '%s' on ref=%s to status='%s' description='%s' targetURL='%s'",
sender.GetLogin(),
event.GetContext(),
commitSHA,
event.GetState(),
event.GetDescription(),
event.GetTargetURL(),
)
logger.Warn().Str(LogKeyAudit, eventType).Msg(auditMessage)
logger.Warn().
Str(LogKeyAudit, eventType).
Str(LogKeyGitHubSHA, commitSHA).
Msgf(
"Entity '%s' overwrote status check '%s' to state='%s' description='%s' targetURL='%s'",
sender.GetLogin(),
event.GetContext(),
event.GetState(),
event.GetDescription(),
event.GetTargetURL(),
)

// must be less than 140 characters to satisfy GitHub API
desc := fmt.Sprintf("'%s' overwrote status to '%s'", sender.GetLogin(), event.GetState())

// unlike in other code, use a single context here because we want to
// replace a forged context with a failure, not post a general status
// if multiple contexts are forged, we will handle multiple events
status := &github.RepoStatus{
Context: event.Context,
State: github.String("failure"),
Description: &auditMessage,
Description: &desc,
}

_, _, err := client.Repositories.CreateStatus(ctx, ownerName, repoName, commitSHA, status)
Expand Down

0 comments on commit 758ff8c

Please sign in to comment.