small fixes (#1232)

app/api/controller/githook/post_receive.go

@@ -107,14 +107,14 @@ func (c *Controller) reportBranchEvent(
  branchUpdate hook.ReferenceUpdate,
 ) {
  switch {
- case branchUpdate.Old.String() == types.NilSHA:
+ case branchUpdate.Old.IsNil():
  c.gitReporter.BranchCreated(ctx, &events.BranchCreatedPayload{
  RepoID: repo.ID,
  PrincipalID: principalID,
  Ref: branchUpdate.Ref,
  SHA: branchUpdate.New.String(),
  })
- case branchUpdate.New.String() == types.NilSHA:
+ case branchUpdate.New.IsNil():
  c.gitReporter.BranchDeleted(ctx, &events.BranchDeletedPayload{
  RepoID: repo.ID,
  PrincipalID: principalID,
@@ -157,14 +157,14 @@ func (c *Controller) reportTagEvent(
  tagUpdate hook.ReferenceUpdate,
 ) {
  switch {
- case tagUpdate.Old.String() == types.NilSHA:
+ case tagUpdate.Old.IsNil():
  c.gitReporter.TagCreated(ctx, &events.TagCreatedPayload{
  RepoID: repo.ID,
  PrincipalID: principalID,
  Ref: tagUpdate.Ref,
  SHA: tagUpdate.New.String(),
  })
- case tagUpdate.New.String() == types.NilSHA:
+ case tagUpdate.New.IsNil():
  c.gitReporter.TagDeleted(ctx, &events.TagDeletedPayload{
  RepoID: repo.ID,
  PrincipalID: principalID,
@@ -195,7 +195,7 @@ func (c *Controller) handlePRMessaging(
  // skip anything that was a batch push / isn't branch related / isn't updating/creating a branch.
  if len(in.RefUpdates) != 1 ||
  !strings.HasPrefix(in.RefUpdates[0].Ref, gitReferenceNamePrefixBranch) ||
- in.RefUpdates[0].New.String() == types.NilSHA {
+ in.RefUpdates[0].New.IsNil() {
  return
  }
 
@@ -273,7 +273,7 @@ func (c *Controller) handleEmptyRepoPush(
  // we only care about one active branch that was pushed.
  for _, refUpdate := range in.RefUpdates {
  if strings.HasPrefix(refUpdate.Ref, gitReferenceNamePrefixBranch) &&
- refUpdate.New.String() != types.NilSHA {
+ !refUpdate.New.IsNil() {
  branchName = refUpdate.Ref[len(gitReferenceNamePrefixBranch):]
  break
  }

app/api/controller/githook/pre_receive.go

@@ -186,9 +186,9 @@ type changes struct {
 
 func (c *changes) groupByAction(refUpdate hook.ReferenceUpdate, name string) {
  switch {
- case refUpdate.Old.String() == types.NilSHA:
+ case refUpdate.Old.IsNil():
  c.created = append(c.created, name)
- case refUpdate.New.String() == types.NilSHA:
+ case refUpdate.New.IsNil():
  c.deleted = append(c.deleted, name)
  default:
  c.updated = append(c.updated, name)

app/api/controller/githook/pre_receive_scan_secrets.go

@@ -92,7 +92,7 @@ func scanSecretsInternal(ctx context.Context,
  ctx := logging.NewContext(ctx, loggingWithRefUpdate(refUpdate))
  log := log.Ctx(ctx)
 
- if refUpdate.New.String() == types.NilSHA {
+ if refUpdate.New.IsNil() {
  log.Debug().Msg("skip deleted reference")
  continue
  }

app/api/controller/pullreq/merge.go

@@ -424,10 +424,11 @@ func (c *Controller) Merge(
 
  pr.ActivitySeq = activitySeqMerge
  activityPayload := &types.PullRequestActivityPayloadMerge{
- MergeMethod: in.Method,
- MergeSHA: mergeOutput.MergeSHA.String(),
- TargetSHA: mergeOutput.BaseSHA.String(),
- SourceSHA: mergeOutput.HeadSHA.String(),
+ MergeMethod: in.Method,
+ MergeSHA: mergeOutput.MergeSHA.String(),
+ TargetSHA: mergeOutput.BaseSHA.String(),
+ SourceSHA: mergeOutput.HeadSHA.String(),
+ RulesBypassed: protection.IsBypassed(violations),
  }
  if _, errAct := c.activityStore.CreateWithPayload(ctx, pr, session.Principal.ID, activityPayload); errAct != nil {
  // non-critical error

app/services/protection/service.go

@@ -69,6 +69,15 @@ func IsCritical(violations []types.RuleViolations) bool {
  return false
 }
 
+func IsBypassed(violations []types.RuleViolations) bool {
+ for i := range violations {
+ if violations[i].IsBypassed() {
+ return true
+ }
+ }
+ return false
+}
+
 // NewManager creates new protection Manager.
 func NewManager(ruleStore store.RuleStore) *Manager {
  return &Manager{

git/hook/cli_core.go

@@ -50,7 +50,7 @@ func (c *CLICore) PreReceive(ctx context.Context) error {
  return fmt.Errorf("failed to read updated references from std in: %w", err)
  }
 
- alternateObjDirs, err := getAlternateObjectDirsFromEnv()
+ alternateObjDirs, err := getAlternateObjectDirsFromEnv(refUpdates)
  if err != nil {
  return fmt.Errorf("failed to read alternate object dirs from env: %w", err)
  }
@@ -68,17 +68,19 @@ func (c *CLICore) PreReceive(ctx context.Context) error {
 }
 
 // Update executes the update git hook.
-func (c *CLICore) Update(ctx context.Context, ref string, oldSHA string, newSHA string) error {
- alternateObjDirs, err := getAlternateObjectDirsFromEnv()
+func (c *CLICore) Update(ctx context.Context, ref string, oldSHARaw string, newSHARaw string) error {
+ newSHA := sha.Must(newSHARaw)
+ oldSHA := sha.Must(oldSHARaw)
+ alternateObjDirs, err := getAlternateObjectDirsFromEnv([]ReferenceUpdate{{Ref: ref, Old: oldSHA, New: newSHA}})
  if err != nil {
  return fmt.Errorf("failed to read alternate object dirs from env: %w", err)
  }
 
  in := UpdateInput{
  RefUpdate: ReferenceUpdate{
  Ref: ref,
- Old: sha.Must(oldSHA),
- New: sha.Must(newSHA),
+ Old: oldSHA,
+ New: newSHA,
  },
  Environment: Environment{
  AlternateObjectDirs: alternateObjDirs,
@@ -183,7 +185,20 @@ func getUpdatedReferencesFromStdIn() ([]ReferenceUpdate, error) {
 // to be able to preemptively access the quarantined objects created by a write operation.
 // NOTE: The temp dir of a write operation is it's main object dir,
 // which is the one that read operations have to use as alternate object dir.
-func getAlternateObjectDirsFromEnv() ([]string, error) {
+func getAlternateObjectDirsFromEnv(refUpdates []ReferenceUpdate) ([]string, error) {
+ hasCreateOrUpdate := false
+ for i := range refUpdates {
+ if !refUpdates[i].New.IsNil() {
+ hasCreateOrUpdate = true
+ break
+ }
+ }
+
+ // git doesn't create an alternate object dir if there's only delete operations
+ if !hasCreateOrUpdate {
+ return nil, nil
+ }
+
  tmpDir, err := getRequiredEnvironmentVariable(command.GitObjectDir)
  if err != nil {
  return nil, err

types/pullreq_activity.go

@@ -220,10 +220,11 @@ func (a *PullRequestActivityPayloadCodeComment) ActivityType() enum.PullReqActiv
 }
 
 type PullRequestActivityPayloadMerge struct {
- MergeMethod enum.MergeMethod `json:"merge_method"`
- MergeSHA string `json:"merge_sha"`
- TargetSHA string `json:"target_sha"`
- SourceSHA string `json:"source_sha"`
+ MergeMethod enum.MergeMethod `json:"merge_method"`
+ MergeSHA string `json:"merge_sha"`
+ TargetSHA string `json:"target_sha"`
+ SourceSHA string `json:"source_sha"`
+ RulesBypassed bool `json:"rules_bypassed,omitempty"`
 }
 
 func (a *PullRequestActivityPayloadMerge) ActivityType() enum.PullReqActivityType {

types/rule.go

@@ -164,7 +164,11 @@ func (violations *RuleViolations) Addf(code, format string, params ...any) {
 }
 
 func (violations *RuleViolations) IsCritical() bool {
- return violations.Rule.State == enum.RuleStateActive && !violations.Bypassed && len(violations.Violations) > 0
+ return violations.Rule.State == enum.RuleStateActive && len(violations.Violations) > 0 && !violations.Bypassed
+}
+
+func (violations *RuleViolations) IsBypassed() bool {
+ return violations.Rule.State == enum.RuleStateActive && len(violations.Violations) > 0 && violations.Bypassed
 }
 
 // RuleInfo holds basic info about a rule that is used to describe the rule in RuleViolations.

web/src/i18n/strings.en.yaml

@@ -286,8 +286,8 @@ pr:
  requestSubmitted: Request for changes submitted.
  prReviewSubmit: '{user} {state|approved:approved, rejected:rejected,changereq:requested changes to, reviewed} this pull request. {time}'
  prMergedBannerInfo: '{user} merged branch {source} into {target} {time}.'
- prMergedInfo: '{user} merged changes from {source} into {target} as {mergeSha} {time}'
- prRebasedInfo: '{user} rebased changes from branch {source} onto {target}, now at {mergeSha} {time}'
+ prMergedInfo: '{user}{bypassed|true: bypassed rules and , }merged changes from {source} into {target} as {mergeSha} {time}'
+ prRebasedInfo: '{user}{bypassed|true: bypassed rules and , }rebased changes from branch {source} onto {target}, now at {mergeSha} {time}'
  prBranchPushInfo: '{user} pushed a new commit {commit}'
  prBranchDeleteInfo: '{user} deleted the source branch with latest commit {commit}'
  prStateChanged: '{user} changed pull request state from {old} to {new}.'

web/src/pages/PullRequest/Conversation/SystemComment.tsx

@@ -39,6 +39,7 @@ interface SystemCommentProps extends Pick<GitInfoProps, 'pullReqMetadata'> {
 interface MergePayload {
  merge_sha: string
  merge_method: string
+ rules_bypassed: boolean
 }
 
 export const SystemComment: React.FC<SystemCommentProps> = ({ pullReqMetadata, commentItems, repoMetadataPath }) => {
@@ -68,6 +69,7 @@ export const SystemComment: React.FC<SystemCommentProps> = ({ pullReqMetadata, c
  user: <strong className={css.rightTextPadding}>{pullReqMetadata.merger?.display_name}</strong>,
  source: <strong className={css.textPadding}>{pullReqMetadata.source_branch}</strong>,
  target: <strong className={css.textPadding}>{pullReqMetadata.target_branch}</strong>,
+ bypassed: (payload?.payload as MergePayload)?.rules_bypassed,
  mergeSha: (
  <Container className={css.commitContainer} padding={{ left: 'small', right: 'xsmall' }}>
  <CommitActions