finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Second attempt at preventing zombies (#16326)

Browse source 
* 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>
This commit is contained in:
zeripath 2021-07-14 15:43:13 +01:00 • committed by GitHub
parent ee43d70a0c
commit 3dcb3e9073
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 229 additions and 143 deletions
Download patch file Download diff file Expand all files Collapse all files

26
cmd/cmd.go
View file

@ -7,9 +7,13 @@
package cmd
import (
"context"
"errors"
"fmt"
"os"
"os/signal"
"strings"
"syscall"
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/setting"
@ -66,3 +70,25 @@ func initDBDisableConsole(disableConsole bool) error {
}
return nil
}
func installSignals() (context.Context, context.CancelFunc) {
ctx, cancel := context.WithCancel(context.Background())
go func() {
// install notify
signalChannel := make(chan os.Signal, 1)
signal.Notify(
signalChannel,
syscall.SIGINT,
syscall.SIGTERM,
)
select {
case <-signalChannel:
case <-ctx.Done():
}
cancel()
signal.Reset()
}()
return ctx, cancel
}

45
cmd/hook.go
View file

@ -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()

5
cmd/keys.go
View file

@ -62,9 +62,12 @@ func runKeys(c *cli.Context) error {
return errors.New("No key type and content provided")
}
ctx, cancel := installSignals()
defer cancel()
setup("keys.log", false)
authorizedString, err := private.AuthorizedPublicKeyByContent(content)
authorizedString, err := private.AuthorizedPublicKeyByContent(ctx, content)
if err != nil {
return err
}

5
cmd/mailer.go
View file

@ -14,6 +14,9 @@ import (
)
func runSendMail(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setting.NewContext()
if err := argsSet(c, "title"); err != nil {
@ -39,7 +42,7 @@ func runSendMail(c *cli.Context) error {
}
}
status, message := private.SendEmail(subject, body, nil)
status, message := private.SendEmail(ctx, subject, body, nil)
if status != http.StatusOK {
fmt.Printf("error: %s\n", message)
return nil

56
cmd/manager.go
View file

@ -236,10 +236,13 @@ func runRemoveLogger(c *cli.Context) error {
group = log.DEFAULT
}
name := c.Args().First()
statusCode, msg := private.RemoveLogger(group, name)
ctx, cancel := installSignals()
defer cancel()
statusCode, msg := private.RemoveLogger(ctx, group, name)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -371,10 +374,13 @@ func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) e
if c.IsSet("name") {
name = c.String("name")
}
statusCode, msg := private.AddLogger(group, name, mode, vals)
ctx, cancel := installSignals()
defer cancel()
statusCode, msg := private.AddLogger(ctx, group, name, mode, vals)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -382,11 +388,14 @@ func commonAddLogger(c *cli.Context, mode string, vals map[string]interface{}) e
}
func runShutdown(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setup("manager", c.Bool("debug"))
statusCode, msg := private.Shutdown()
statusCode, msg := private.Shutdown(ctx)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -394,11 +403,14 @@ func runShutdown(c *cli.Context) error {
}
func runRestart(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setup("manager", c.Bool("debug"))
statusCode, msg := private.Restart()
statusCode, msg := private.Restart(ctx)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -406,11 +418,14 @@ func runRestart(c *cli.Context) error {
}
func runFlushQueues(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setup("manager", c.Bool("debug"))
statusCode, msg := private.FlushQueues(c.Duration("timeout"), c.Bool("non-blocking"))
statusCode, msg := private.FlushQueues(ctx, c.Duration("timeout"), c.Bool("non-blocking"))
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -418,11 +433,14 @@ func runFlushQueues(c *cli.Context) error {
}
func runPauseLogging(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setup("manager", c.Bool("debug"))
statusCode, msg := private.PauseLogging()
statusCode, msg := private.PauseLogging(ctx)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -430,11 +448,14 @@ func runPauseLogging(c *cli.Context) error {
}
func runResumeLogging(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setup("manager", c.Bool("debug"))
statusCode, msg := private.ResumeLogging()
statusCode, msg := private.ResumeLogging(ctx)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)
@ -442,11 +463,14 @@ func runResumeLogging(c *cli.Context) error {
}
func runReleaseReopenLogging(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setup("manager", c.Bool("debug"))
statusCode, msg := private.ReleaseReopenLogging()
statusCode, msg := private.ReleaseReopenLogging(ctx)
switch statusCode {
case http.StatusInternalServerError:
fail("InternalServerError", msg)
return fail("InternalServerError", msg)
}
fmt.Fprintln(os.Stdout, msg)

16
cmd/restore_repo.go
View file

@ -40,20 +40,24 @@ var CmdRestoreRepository = cli.Command{
cli.StringFlag{
Name: "units",
Value: "",
Usage: `Which items will be restored, one or more units should be separated as comma.
Usage: `Which items will be restored, one or more units should be separated as comma.
wiki, issues, labels, releases, release_assets, milestones, pull_requests, comments are allowed. Empty means all units.`,
},
},
}
func runRestoreRepository(ctx *cli.Context) error {
func runRestoreRepository(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
setting.NewContext()
statusCode, errStr := private.RestoreRepo(
ctx.String("repo_dir"),
ctx.String("owner_name"),
ctx.String("repo_name"),
ctx.StringSlice("units"),
ctx,
c.String("repo_dir"),
c.String("owner_name"),
c.String("repo_name"),
c.StringSlice("units"),
)
if statusCode == http.StatusOK {
return nil

84
cmd/serv.go
View file

@ -6,17 +6,14 @@
package cmd
import (
"context"
"fmt"
"net/http"
"net/url"
"os"
"os/exec"
"os/signal"
"regexp"
"strconv"
"strings"
"syscall"
"time"
"code.gitea.io/gitea/models"
@ -75,7 +72,10 @@ var (
alphaDashDotPattern = regexp.MustCompile(`[^\w-\.]`)
)
func fail(userMessage, logMessage string, args ...interface{}) {
func fail(userMessage, logMessage string, args ...interface{}) error {
// There appears to be a chance to cause a zombie process and failure to read the Exit status
// if nothing is outputted on stdout.
fmt.Fprintln(os.Stdout, "")
fmt.Fprintln(os.Stderr, "Gitea:", userMessage)
if len(logMessage) > 0 {
@ -83,15 +83,19 @@ func fail(userMessage, logMessage string, args ...interface{}) {
fmt.Fprintf(os.Stderr, logMessage+"\n", args...)
}
}
ctx, cancel := installSignals()
defer cancel()
if len(logMessage) > 0 {
_ = private.SSHLog(true, fmt.Sprintf(logMessage+": ", args...))
_ = private.SSHLog(ctx, true, fmt.Sprintf(logMessage+": ", args...))
}
os.Exit(1)
return cli.NewExitError(fmt.Sprintf("Gitea: %s", userMessage), 1)
}
func runServ(c *cli.Context) error {
ctx, cancel := installSignals()
defer cancel()
// FIXME: This needs to internationalised
setup("serv.log", c.Bool("debug"))
@ -109,18 +113,18 @@ func runServ(c *cli.Context) error {
keys := strings.Split(c.Args()[0], "-")
if len(keys) != 2 || keys[0] != "key" {
fail("Key ID format error", "Invalid key argument: %s", c.Args()[0])
return fail("Key ID format error", "Invalid key argument: %s", c.Args()[0])
}
keyID, err := strconv.ParseInt(keys[1], 10, 64)
if err != nil {
fail("Key ID format error", "Invalid key argument: %s", c.Args()[1])
return fail("Key ID format error", "Invalid key argument: %s", c.Args()[1])
}
cmd := os.Getenv("SSH_ORIGINAL_COMMAND")
if len(cmd) == 0 {
key, user, err := private.ServNoCommand(keyID)
key, user, err := private.ServNoCommand(ctx, keyID)
if err != nil {
fail("Internal error", "Failed to check provided key: %v", err)
return fail("Internal error", "Failed to check provided key: %v", err)
}
switch key.Type {
case models.KeyTypeDeploy:
@ -138,11 +142,11 @@ func runServ(c *cli.Context) error {
words, err := shellquote.Split(cmd)
if err != nil {
fail("Error parsing arguments", "Failed to parse arguments: %v", err)
return fail("Error parsing arguments", "Failed to parse arguments: %v", err)
}
if len(words) < 2 {
fail("Too few arguments", "Too few arguments in cmd: %s", cmd)
return fail("Too few arguments", "Too few arguments in cmd: %s", cmd)
}
verb := words[0]
@ -154,7 +158,7 @@ func runServ(c *cli.Context) error {
var lfsVerb string
if verb == lfsAuthenticateVerb {
if !setting.LFS.StartServer {
fail("Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled")
return fail("Unknown git command", "LFS authentication request over SSH denied, LFS support is disabled")
}
if len(words) > 2 {
@ -167,37 +171,37 @@ func runServ(c *cli.Context) error {
rr := strings.SplitN(repoPath, "/", 2)
if len(rr) != 2 {
fail("Invalid repository path", "Invalid repository path: %v", repoPath)
return fail("Invalid repository path", "Invalid repository path: %v", repoPath)
}
username := strings.ToLower(rr[0])
reponame := strings.ToLower(strings.TrimSuffix(rr[1], ".git"))
if alphaDashDotPattern.MatchString(reponame) {
fail("Invalid repo name", "Invalid repo name: %s", reponame)
return fail("Invalid repo name", "Invalid repo name: %s", reponame)
}
if setting.EnablePprof || c.Bool("enable-pprof") {
if err := os.MkdirAll(setting.PprofDataPath, os.ModePerm); err != nil {
fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err)
return fail("Error while trying to create PPROF_DATA_PATH", "Error while trying to create PPROF_DATA_PATH: %v", err)
}
stopCPUProfiler, err := pprof.DumpCPUProfileForUsername(setting.PprofDataPath, username)
if err != nil {
fail("Internal Server Error", "Unable to start CPU profile: %v", err)
return fail("Internal Server Error", "Unable to start CPU profile: %v", err)
}
defer func() {
stopCPUProfiler()
err := pprof.DumpMemProfileForUsername(setting.PprofDataPath, username)
if err != nil {
fail("Internal Server Error", "Unable to dump Mem Profile: %v", err)
_ = fail("Internal Server Error", "Unable to dump Mem Profile: %v", err)
}
}()
}
requestedMode, has := allowedCommands[verb]
if !has {
fail("Unknown git command", "Unknown git command %s", verb)
return fail("Unknown git command", "Unknown git command %s", verb)
}
if verb == lfsAuthenticateVerb {
@ -206,21 +210,20 @@ func runServ(c *cli.Context) error {
} else if lfsVerb == "download" {
requestedMode = models.AccessModeRead
} else {
fail("Unknown LFS verb", "Unknown lfs verb %s", lfsVerb)
return fail("Unknown LFS verb", "Unknown lfs verb %s", lfsVerb)
}
}
results, err := private.ServCommand(keyID, username, reponame, requestedMode, verb, lfsVerb)
results, err := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb)
if err != nil {
if private.IsErrServCommand(err) {
errServCommand := err.(private.ErrServCommand)
if errServCommand.StatusCode != http.StatusInternalServerError {
fail("Unauthorized", "%s", errServCommand.Error())
} else {
fail("Internal Server Error", "%s", errServCommand.Error())
return fail("Unauthorized", "%s", errServCommand.Error())
}
return fail("Internal Server Error", "%s", errServCommand.Error())
}
fail("Internal Server Error", "%s", err.Error())
return fail("Internal Server Error", "%s", err.Error())
}
os.Setenv(models.EnvRepoIsWiki, strconv.FormatBool(results.IsWiki))
os.Setenv(models.EnvRepoName, results.RepoName)
@ -253,7 +256,7 @@ func runServ(c *cli.Context) error {
// Sign and get the complete encoded token as a string using the secret
tokenString, err := token.SignedString(setting.LFS.JWTSecretBytes)
if err != nil {
fail("Internal error", "Failed to sign JWT token: %v", err)
return fail("Internal error", "Failed to sign JWT token: %v", err)
}
tokenAuthentication := &models.LFSTokenResponse{
@ -266,7 +269,7 @@ func runServ(c *cli.Context) error {
enc := json.NewEncoder(os.Stdout)
err = enc.Encode(tokenAuthentication)
if err != nil {
fail("Internal error", "Failed to encode LFS json response: %v", err)
return fail("Internal error", "Failed to encode LFS json response: %v", err)
}
return nil
}
@ -276,25 +279,6 @@ func runServ(c *cli.Context) error {
verb = strings.Replace(verb, "-", " ", 1)
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go func() {
// install notify
signalChannel := make(chan os.Signal, 1)
signal.Notify(
signalChannel,
syscall.SIGINT,
syscall.SIGTERM,
)
select {
case <-signalChannel:
case <-ctx.Done():
}
cancel()
signal.Reset()
}()
var gitcmd *exec.Cmd
verbs := strings.Split(verb, " ")
if len(verbs) == 2 {
@ -308,13 +292,13 @@ func runServ(c *cli.Context) error {
gitcmd.Stdin = os.Stdin
gitcmd.Stderr = os.Stderr
if err = gitcmd.Run(); err != nil {
fail("Internal error", "Failed to execute git command: %v", err)
return fail("Internal error", "Failed to execute git command: %v", err)
}
// Update user key activity.
if results.KeyID > 0 {
if err = private.UpdatePublicKeyInRepo(results.KeyID, results.RepoID); err != nil {
fail("Internal error", "UpdatePublicKeyInRepo: %v", err)
if err = private.UpdatePublicKeyInRepo(ctx, results.KeyID, results.RepoID); err != nil {
return fail("Internal error", "UpdatePublicKeyInRepo: %v", err)
}
}