diff --git a/app/api/controller/githook/post_receive.go b/app/api/controller/githook/post_receive.go index 5c633fdb9..41506f2f9 100644 --- a/app/api/controller/githook/post_receive.go +++ b/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 } diff --git a/app/api/controller/githook/pre_receive.go b/app/api/controller/githook/pre_receive.go index 1b314a7d3..b7367385e 100644 --- a/app/api/controller/githook/pre_receive.go +++ b/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) diff --git a/app/api/controller/githook/pre_receive_scan_secrets.go b/app/api/controller/githook/pre_receive_scan_secrets.go index bbcf34b17..18fcdd1b3 100644 --- a/app/api/controller/githook/pre_receive_scan_secrets.go +++ b/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 } diff --git a/app/api/controller/pullreq/merge.go b/app/api/controller/pullreq/merge.go index f83895448..1573824a1 100644 --- a/app/api/controller/pullreq/merge.go +++ b/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 diff --git a/app/services/protection/service.go b/app/services/protection/service.go index b5b1c4b7a..d6d089d34 100644 --- a/app/services/protection/service.go +++ b/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{ diff --git a/git/hook/cli_core.go b/git/hook/cli_core.go index 366fb460c..fc4780dfe 100644 --- a/git/hook/cli_core.go +++ b/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,8 +68,10 @@ 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) } @@ -77,8 +79,8 @@ func (c *CLICore) Update(ctx context.Context, ref string, oldSHA string, newSHA 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 diff --git a/types/pullreq_activity.go b/types/pullreq_activity.go index 3c9c88054..9634c24c3 100644 --- a/types/pullreq_activity.go +++ b/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 { diff --git a/types/rule.go b/types/rule.go index bdead2e6f..fd4b5ef56 100644 --- a/types/rule.go +++ b/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. diff --git a/web/src/i18n/strings.en.yaml b/web/src/i18n/strings.en.yaml index bc911ee7c..83798a8d0 100644 --- a/web/src/i18n/strings.en.yaml +++ b/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}.' diff --git a/web/src/pages/PullRequest/Conversation/SystemComment.tsx b/web/src/pages/PullRequest/Conversation/SystemComment.tsx index c46b47406..93021ee1d 100644 --- a/web/src/pages/PullRequest/Conversation/SystemComment.tsx +++ b/web/src/pages/PullRequest/Conversation/SystemComment.tsx @@ -39,6 +39,7 @@ interface SystemCommentProps extends Pick { interface MergePayload { merge_sha: string merge_method: string + rules_bypassed: boolean } export const SystemComment: React.FC = ({ pullReqMetadata, commentItems, repoMetadataPath }) => { @@ -68,6 +69,7 @@ export const SystemComment: React.FC = ({ pullReqMetadata, c user: {pullReqMetadata.merger?.display_name}, source: {pullReqMetadata.source_branch}, target: {pullReqMetadata.target_branch}, + bypassed: (payload?.payload as MergePayload)?.rules_bypassed, mergeSha: (