Second attempt at preventing zombies (#16326)

* Second attempt at preventing zombies

* Ensure that the pipes are closed in ssh.go
* Ensure that a cancellable context is passed up in cmd/* http requests
* Make cmd.fail return properly so defers are obeyed
* Ensure that something is sent to stdout in case of blocks here

Signed-off-by: Andrew Thornton <art27@cantab.net>

* placate lint

Signed-off-by: Andrew Thornton <art27@cantab.net>

* placate lint 2

Signed-off-by: Andrew Thornton <art27@cantab.net>

* placate lint 3

Signed-off-by: Andrew Thornton <art27@cantab.net>

* fixup

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

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

1 files changed, 24 insertions, 21 deletions

diff --git a/cmd/hook.go b/cmd/hook.go
index 067a0bfb8a..87f1f37562 100644
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -152,17 +152,18 @@ func runHookPreReceive(c *cli.Context) error {
 	if os.Getenv(models.EnvIsInternal) == "true" {
 		return nil
 	}
+	ctx, cancel := installSignals()
+	defer cancel()
 
 	setup("hooks/pre-receive.log", c.Bool("debug"))
 
 	if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
 		if setting.OnlyAllowPushIfGiteaEnvironmentSet {
-			fail(`Rejecting changes as Gitea environment not set.
+			return fail(`Rejecting changes as Gitea environment not set.
 If you are pushing over SSH you must push with a key managed by
 Gitea or set your environment appropriately.`, "")
-		} else {
-			return nil
 		}
+		return nil
 	}
 
 	// the environment is set by serv command
@@ -235,14 +236,14 @@ Gitea or set your environment appropriately.`, "")
 				hookOptions.OldCommitIDs = oldCommitIDs
 				hookOptions.NewCommitIDs = newCommitIDs
 				hookOptions.RefFullNames = refFullNames
-				statusCode, msg := private.HookPreReceive(username, reponame, hookOptions)
+				statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions)
 				switch statusCode {
 				case http.StatusOK:
 					// no-op
 				case http.StatusInternalServerError:
-					fail("Internal Server Error", msg)
+					return fail("Internal Server Error", msg)
 				default:
-					fail(msg, "")
+					return fail(msg, "")
 				}
 				count = 0
 				lastline = 0
@@ -263,12 +264,12 @@ Gitea or set your environment appropriately.`, "")
 
 		fmt.Fprintf(out, " Checking %d references\n", count)
 
-		statusCode, msg := private.HookPreReceive(username, reponame, hookOptions)
+		statusCode, msg := private.HookPreReceive(ctx, username, reponame, hookOptions)
 		switch statusCode {
 		case http.StatusInternalServerError:
-			fail("Internal Server Error", msg)
+			return fail("Internal Server Error", msg)
 		case http.StatusForbidden:
-			fail(msg, "")
+			return fail(msg, "")
 		}
 	} else if lastline > 0 {
 		fmt.Fprintf(out, "\n")
@@ -285,8 +286,11 @@ func runHookUpdate(c *cli.Context) error {
 }
 
 func runHookPostReceive(c *cli.Context) error {
+	ctx, cancel := installSignals()
+	defer cancel()
+
 	// First of all run update-server-info no matter what
-	if _, err := git.NewCommand("update-server-info").Run(); err != nil {
+	if _, err := git.NewCommand("update-server-info").SetParentContext(ctx).Run(); err != nil {
 		return fmt.Errorf("Failed to call 'git update-server-info': %v", err)
 	}
 
@@ -299,12 +303,11 @@ func runHookPostReceive(c *cli.Context) error {
 
 	if len(os.Getenv("SSH_ORIGINAL_COMMAND")) == 0 {
 		if setting.OnlyAllowPushIfGiteaEnvironmentSet {
-			fail(`Rejecting changes as Gitea environment not set.
+			return fail(`Rejecting changes as Gitea environment not set.
 If you are pushing over SSH you must push with a key managed by
 Gitea or set your environment appropriately.`, "")
-		} else {
-			return nil
 		}
+		return nil
 	}
 
 	var out io.Writer
@@ -371,11 +374,11 @@ Gitea or set your environment appropriately.`, "")
 			hookOptions.OldCommitIDs = oldCommitIDs
 			hookOptions.NewCommitIDs = newCommitIDs
 			hookOptions.RefFullNames = refFullNames
-			resp, err := private.HookPostReceive(repoUser, repoName, hookOptions)
+			resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
 			if resp == nil {
 				_ = dWriter.Close()
 				hookPrintResults(results)
-				fail("Internal Server Error", err)
+				return fail("Internal Server Error", err)
 			}
 			wasEmpty = wasEmpty || resp.RepoWasEmpty
 			results = append(results, resp.Results...)
@@ -386,9 +389,9 @@ Gitea or set your environment appropriately.`, "")
 	if count == 0 {
 		if wasEmpty && masterPushed {
 			// We need to tell the repo to reset the default branch to master
-			err := private.SetDefaultBranch(repoUser, repoName, "master")
+			err := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
 			if err != nil {
-				fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
+				return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
 			}
 		}
 		fmt.Fprintf(out, "Processed %d references in total\n", total)
@@ -404,11 +407,11 @@ Gitea or set your environment appropriately.`, "")
 
 	fmt.Fprintf(out, " Processing %d references\n", count)
 
-	resp, err := private.HookPostReceive(repoUser, repoName, hookOptions)
+	resp, err := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
 	if resp == nil {
 		_ = dWriter.Close()
 		hookPrintResults(results)
-		fail("Internal Server Error", err)
+		return fail("Internal Server Error", err)
 	}
 	wasEmpty = wasEmpty || resp.RepoWasEmpty
 	results = append(results, resp.Results...)
@@ -417,9 +420,9 @@ Gitea or set your environment appropriately.`, "")
 
 	if wasEmpty && masterPushed {
 		// We need to tell the repo to reset the default branch to master
-		err := private.SetDefaultBranch(repoUser, repoName, "master")
+		err := private.SetDefaultBranch(ctx, repoUser, repoName, "master")
 		if err != nil {
-			fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
+			return fail("Internal Server Error", "SetDefaultBranch failed with Error: %v", err)
 		}
 	}
 	_ = dWriter.Close()