Handle push rejection message in Merge & Web Editor (#10373)

* Handle push rejection message in Merge

* placate golangci-lint

* Fix sanitize, adjust message handling

* oops

* Oops

* Handle push-rejection in webeditor CRUD too

* Apply suggestions from code review

Co-authored-by: Lauris BH <lauris@nix.lv>

models/error.go

@@ -7,6 +7,7 @@ package models
 
 import (
 	"fmt"
+	"strings"
 
 	"code.gitea.io/gitea/modules/git"
 )
@@ -1371,6 +1372,53 @@ func (err ErrMergePushOutOfDate) Error() string {
 	return fmt.Sprintf("Merge PushOutOfDate Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
 }
 
+// ErrPushRejected represents an error if merging fails due to rejection from a hook
+type ErrPushRejected struct {
+	Style   MergeStyle
+	Message string
+	StdOut  string
+	StdErr  string
+	Err     error
+}
+
+// IsErrPushRejected checks if an error is a ErrPushRejected.
+func IsErrPushRejected(err error) bool {
+	_, ok := err.(ErrPushRejected)
+	return ok
+}
+
+func (err ErrPushRejected) Error() string {
+	return fmt.Sprintf("Merge PushRejected Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
+}
+
+// GenerateMessage generates the remote message from the stderr
+func (err *ErrPushRejected) GenerateMessage() {
+	messageBuilder := &strings.Builder{}
+	i := strings.Index(err.StdErr, "remote: ")
+	if i < 0 {
+		err.Message = ""
+		return
+	}
+	for {
+		if len(err.StdErr) <= i+8 {
+			break
+		}
+		if err.StdErr[i:i+8] != "remote: " {
+			break
+		}
+		i += 8
+		nl := strings.IndexByte(err.StdErr[i:], '\n')
+		if nl > 0 {
+			messageBuilder.WriteString(err.StdErr[i : i+nl+1])
+			i = i + nl + 1
+		} else {
+			messageBuilder.WriteString(err.StdErr[i:])
+			i = len(err.StdErr)
+		}
+	}
+	err.Message = strings.TrimSpace(messageBuilder.String())
+}
+
 // ErrRebaseConflicts represents an error if rebase fails with a conflict
 type ErrRebaseConflicts struct {
 	Style     MergeStyle

modules/repofiles/temp_repo.go

@@ -242,8 +242,28 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(author, committer *models
 func (t *TemporaryUploadRepository) Push(doer *models.User, commitHash string, branch string) error {
 	// Because calls hooks we need to pass in the environment
 	env := models.PushingEnvironment(doer, t.repo)
+	stdout := &strings.Builder{}
+	stderr := &strings.Builder{}
 
-	if stdout, err := git.NewCommand("push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)).RunInDirWithEnv(t.basePath, env); err != nil {
+	if err := git.NewCommand("push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)).RunInDirTimeoutEnvPipeline(env, -1, t.basePath, stdout, stderr); err != nil {
+		errString := stderr.String()
+		if strings.Contains(errString, "non-fast-forward") {
+			return models.ErrMergePushOutOfDate{
+				StdOut: stdout.String(),
+				StdErr: errString,
+				Err:    err,
+			}
+		} else if strings.Contains(errString, "! [remote rejected]") {
+			log.Error("Unable to push back to repo from temporary repo due to rejection: %s (%s)\nStdout: %s\nStderr: %s\nError: %v",
+				t.repo.FullName(), t.basePath, stdout, errString, err)
+			err := models.ErrPushRejected{
+				StdOut: stdout.String(),
+				StdErr: errString,
+				Err:    err,
+			}
+			err.GenerateMessage()
+			return err
+		}
 		log.Error("Unable to push back to repo from temporary repo: %s (%s)\nStdout: %s\nError: %v",
 			t.repo.FullName(), t.basePath, stdout, err)
 		return fmt.Errorf("Unable to push back to repo from temporary repo: %s (%s) Error: %v",

options/locale/locale_en-US.ini

@@ -784,6 +784,8 @@ editor.commit_empty_file_header = Commit an empty file
 editor.commit_empty_file_text = The file you're about commit is empty. Proceed?
 editor.no_changes_to_show = There are no changes to show.
 editor.fail_to_update_file = Failed to update/create file '%s' with error: %v
+editor.push_rejected_no_message = The change was rejected by the server without a message. Please check githooks.
+editor.push_rejected = The change was rejected by the server with the following message:<br>%s<br> Please check githooks.
 editor.add_subdir = Add a directory…
 editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v
 editor.upload_file_is_locked = File '%s' is locked by %s.
@@ -1088,6 +1090,8 @@ pulls.merge_conflict = Merge Failed: There was a conflict whilst merging: %[1]s<
 pulls.rebase_conflict = Merge Failed: There was a conflict whilst rebasing commit: %[1]s<br>%[2]s<br>%[3]s<br>Hint:Try a different strategy
 pulls.unrelated_histories = Merge Failed: The merge head and base do not share a common history. Hint: Try a different strategy
 pulls.merge_out_of_date = Merge Failed: Whilst generating the merge, the base was updated. Hint: Try again.
+pulls.push_rejected = Merge Failed: The push was rejected with the following message:<br>%s<br>Review the githooks for this repository
+pulls.push_rejected_no_message = Merge Failed: The push was rejected but there was no remote message.<br>Review the githooks for this repository
 pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
 pulls.status_checking = Some checks are pending
 pulls.status_checks_success = All checks were successful

routers/api/v1/repo/pull.go

@@ -684,6 +684,14 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
 		} else if models.IsErrMergePushOutOfDate(err) {
 			ctx.Error(http.StatusConflict, "Merge", "merge push out of date")
 			return
+		} else if models.IsErrPushRejected(err) {
+			errPushRej := err.(models.ErrPushRejected)
+			if len(errPushRej.Message) == 0 {
+				ctx.Error(http.StatusConflict, "Merge", "PushRejected without remote error message")
+				return
+			}
+			ctx.Error(http.StatusConflict, "Merge", "PushRejected with remote message: "+errPushRej.Message)
+			return
 		}
 		ctx.Error(http.StatusInternalServerError, "Merge", err)
 		return

routers/repo/editor.go

@@ -23,6 +23,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/upload"
 	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/routers/utils"
 )
 
 const (
@@ -264,10 +265,17 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
 			} else {
 				ctx.Error(500, err.Error())
 			}
-		} else if models.IsErrCommitIDDoesNotMatch(err) {
+		} else if models.IsErrCommitIDDoesNotMatch(err) || models.IsErrMergePushOutOfDate(err) {
 			ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplEditFile, &form)
+		} else if models.IsErrPushRejected(err) {
+			errPushRej := err.(models.ErrPushRejected)
+			if len(errPushRej.Message) == 0 {
+				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplEditFile, &form)
 			} else {
-			ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, err), tplEditFile, &form)
+				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplEditFile, &form)
+			}
+		} else {
+			ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, utils.SanitizeFlashErrorString(err.Error())), tplEditFile, &form)
 		}
 	}
 
@@ -428,8 +436,15 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
 			} else {
 				ctx.Error(500, err.Error())
 			}
-		} else if models.IsErrCommitIDDoesNotMatch(err) {
+		} else if models.IsErrCommitIDDoesNotMatch(err) || models.IsErrMergePushOutOfDate(err) {
 			ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_deleting", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplDeleteFile, &form)
+		} else if models.IsErrPushRejected(err) {
+			errPushRej := err.(models.ErrPushRejected)
+			if len(errPushRej.Message) == 0 {
+				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected_no_message"), tplDeleteFile, &form)
+			} else {
+				ctx.RenderWithErr(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(errPushRej.Message)), tplDeleteFile, &form)
+			}
 		} else {
 			ctx.ServerError("DeleteRepoFile", err)
 		}

routers/repo/pull.go

@@ -26,6 +26,7 @@ import (
 	"code.gitea.io/gitea/modules/repofiles"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
+	"code.gitea.io/gitea/routers/utils"
 	"code.gitea.io/gitea/services/gitdiff"
 	pull_service "code.gitea.io/gitea/services/pull"
 	repo_service "code.gitea.io/gitea/services/repository"
@@ -774,27 +775,18 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
 	}
 
 	if err = pull_service.Merge(pr, ctx.User, ctx.Repo.GitRepo, models.MergeStyle(form.Do), message); err != nil {
-		sanitize := func(x string) string {
-			runes := []rune(x)
-
-			if len(runes) > 512 {
-				x = "..." + string(runes[len(runes)-512:])
-			}
-
-			return strings.Replace(html.EscapeString(x), "\n", "<br>", -1)
-		}
 		if models.IsErrInvalidMergeStyle(err) {
 			ctx.Flash.Error(ctx.Tr("repo.pulls.invalid_merge_option"))
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
 		} else if models.IsErrMergeConflicts(err) {
 			conflictError := err.(models.ErrMergeConflicts)
-			ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", sanitize(conflictError.StdErr), sanitize(conflictError.StdOut)))
+			ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
 		} else if models.IsErrRebaseConflicts(err) {
 			conflictError := err.(models.ErrRebaseConflicts)
-			ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", sanitize(conflictError.CommitSHA), sanitize(conflictError.StdErr), sanitize(conflictError.StdOut)))
+			ctx.Flash.Error(ctx.Tr("repo.pulls.rebase_conflict", utils.SanitizeFlashErrorString(conflictError.CommitSHA), utils.SanitizeFlashErrorString(conflictError.StdErr), utils.SanitizeFlashErrorString(conflictError.StdOut)))
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
 		} else if models.IsErrMergeUnrelatedHistories(err) {
@@ -807,6 +799,17 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
 			ctx.Flash.Error(ctx.Tr("repo.pulls.merge_out_of_date"))
 			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
 			return
+		} else if models.IsErrPushRejected(err) {
+			log.Debug("MergePushRejected error: %v", err)
+			pushrejErr := err.(models.ErrPushRejected)
+			message := pushrejErr.Message
+			if len(message) == 0 {
+				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
+			} else {
+				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
+			}
+			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
+			return
 		}
 		ctx.ServerError("Merge", err)
 		return

routers/utils/utils.go

@@ -5,6 +5,7 @@
 package utils
 
 import (
+	"html"
 	"strings"
 )
 
@@ -34,3 +35,14 @@ func IsValidSlackChannel(channelName string) bool {
 
 	return true
 }
+
+// SanitizeFlashErrorString will sanitize a flash error string
+func SanitizeFlashErrorString(x string) string {
+	runes := []rune(x)
+
+	if len(runes) > 512 {
+		x = "..." + string(runes[len(runes)-512:])
+	}
+
+	return strings.Replace(html.EscapeString(x), "\n", "<br>", -1)
+}

services/pull/merge.go

@@ -392,6 +392,15 @@ func rawMerge(pr *models.PullRequest, doer *models.User, mergeStyle models.Merge
 				StdErr: errbuf.String(),
 				Err:    err,
 			}
+		} else if strings.Contains(errbuf.String(), "! [remote rejected]") {
+			err := models.ErrPushRejected{
+				Style:  mergeStyle,
+				StdOut: outbuf.String(),
+				StdErr: errbuf.String(),
+				Err:    err,
+			}
+			err.GenerateMessage()
+			return "", err
 		}
 		return "", fmt.Errorf("git push: %s", errbuf.String())
 	}