Fix pre-receive hook hangs and missing logs by flushing logs on signal and using CommandContext for git commands (#4714)
When TruffleHog times out in pre-receive hooks, it fails to output diagnostic logs. Changes: - Ensure log output is flushed before process termination in all exit paths: * Defer log sync in run() function for normal exits * Sync logs in signal handler before os.Exit(0) * Sync logs before os.Exit(183) when results are found * Sync logs in logFatalFunc before os.Exit() calls - Use exec.CommandContext instead of exec.Command for git log/diff to ensure processes are killed when context is cancelled - Add WaitDelay to git commands to provide grace period for cleanup This ensures diagnostic output is captured when git operations block in pre-receive hook environments and logs are visible when process is killed.
This commit is contained in:
@@ -14,7 +14,6 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
|
||||
"github.com/alecthomas/kingpin/v2"
|
||||
"github.com/fatih/color"
|
||||
"github.com/felixge/fgprof"
|
||||
@@ -347,6 +346,13 @@ func init() {
|
||||
}
|
||||
}
|
||||
|
||||
// syncLogs flushes logs when the program exits.
|
||||
func syncLogs(syncFn func() error) {
|
||||
if syncFn != nil {
|
||||
_ = syncFn()
|
||||
}
|
||||
}
|
||||
|
||||
func main() {
|
||||
// setup logger
|
||||
logFormat := log.WithConsoleSink
|
||||
@@ -358,15 +364,16 @@ func main() {
|
||||
context.SetDefaultLogger(logger)
|
||||
|
||||
if *localDev {
|
||||
run(overseer.State{})
|
||||
run(overseer.State{}, sync)
|
||||
os.Exit(0)
|
||||
}
|
||||
|
||||
defer func() { _ = sync() }()
|
||||
logFatal := logFatalFunc(logger)
|
||||
logFatal := logFatalFunc(logger, sync)
|
||||
|
||||
updateCfg := overseer.Config{
|
||||
Program: run,
|
||||
Program: func(s overseer.State) {
|
||||
run(s, sync)
|
||||
},
|
||||
Debug: *debug,
|
||||
RestartSignal: syscall.SIGTERM,
|
||||
// TODO: Eventually add a PreUpgrade func for signature check w/ x509 PKCS1v15
|
||||
@@ -387,10 +394,10 @@ func main() {
|
||||
}
|
||||
}
|
||||
|
||||
func run(state overseer.State) {
|
||||
|
||||
func run(state overseer.State, logSync func() error) {
|
||||
ctx, cancel := context.WithCancelCause(context.Background())
|
||||
defer cancel(nil)
|
||||
defer syncLogs(logSync)
|
||||
|
||||
go func() {
|
||||
if err := cleantemp.CleanTempArtifacts(ctx); err != nil {
|
||||
@@ -399,7 +406,7 @@ func run(state overseer.State) {
|
||||
}()
|
||||
|
||||
logger := ctx.Logger()
|
||||
logFatal := logFatalFunc(logger)
|
||||
logFatal := logFatalFunc(logger, logSync)
|
||||
|
||||
killSignal := make(chan os.Signal, 1)
|
||||
signal.Notify(killSignal, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT)
|
||||
@@ -413,6 +420,8 @@ func run(state overseer.State) {
|
||||
} else {
|
||||
logger.Info("cleaned temporary artifacts")
|
||||
}
|
||||
|
||||
syncLogs(logSync)
|
||||
os.Exit(0)
|
||||
}()
|
||||
|
||||
@@ -606,6 +615,7 @@ func run(state overseer.State) {
|
||||
|
||||
if metrics.hasFoundResults && *fail {
|
||||
logger.V(2).Info("exiting with code 183 because results were found")
|
||||
syncLogs(logSync)
|
||||
os.Exit(183)
|
||||
}
|
||||
}
|
||||
@@ -1165,9 +1175,10 @@ func parseResults(input *string) (map[string]struct{}, error) {
|
||||
|
||||
// logFatalFunc returns a log.Fatal style function. Calling the returned
|
||||
// function will terminate the program without cleanup.
|
||||
func logFatalFunc(logger logr.Logger) func(error, string, ...any) {
|
||||
func logFatalFunc(logger logr.Logger, logSync func() error) func(error, string, ...any) {
|
||||
return func(err error, message string, keyAndVals ...any) {
|
||||
logger.Error(err, message, keyAndVals...)
|
||||
syncLogs(logSync)
|
||||
if err != nil {
|
||||
os.Exit(1)
|
||||
return
|
||||
|
||||
@@ -30,6 +30,9 @@ const (
|
||||
|
||||
// defaultMaxCommitSize is the maximum size for a commit. Larger commits will be cut off.
|
||||
defaultMaxCommitSize int64 = 2 * 1024 * 1024 * 1024 // 2GB
|
||||
|
||||
// defaultWaitDelay is the default time to wait after context cancellation before forcefully killing git processes.
|
||||
defaultWaitDelay = 5 * time.Second
|
||||
)
|
||||
|
||||
// contentWriter defines a common interface for writing, reading, and managing diff content.
|
||||
@@ -124,6 +127,7 @@ type Parser struct {
|
||||
maxDiffSize int64
|
||||
maxCommitSize int64
|
||||
dateFormat string
|
||||
waitDelay time.Duration
|
||||
|
||||
useCustomContentWriter bool
|
||||
}
|
||||
@@ -204,6 +208,14 @@ func WithMaxCommitSize(maxCommitSize int64) Option {
|
||||
}
|
||||
}
|
||||
|
||||
// WithWaitDelay sets the waitDelay option. This specifies how long to wait after
|
||||
// context cancellation before forcefully killing git processes.
|
||||
func WithWaitDelay(waitDelay time.Duration) Option {
|
||||
return func(parser *Parser) {
|
||||
parser.waitDelay = waitDelay
|
||||
}
|
||||
}
|
||||
|
||||
// Option is used for adding options to Config.
|
||||
type Option func(*Parser)
|
||||
|
||||
@@ -213,6 +225,7 @@ func NewParser(options ...Option) *Parser {
|
||||
dateFormat: defaultDateFormat,
|
||||
maxDiffSize: defaultMaxDiffSize,
|
||||
maxCommitSize: defaultMaxCommitSize,
|
||||
waitDelay: defaultWaitDelay,
|
||||
}
|
||||
for _, option := range options {
|
||||
option(parser)
|
||||
@@ -253,7 +266,7 @@ func (c *Parser) RepoPath(
|
||||
args = append(args, "--", ".", ":(exclude)"+glob)
|
||||
}
|
||||
|
||||
cmd := exec.Command("git", args...)
|
||||
cmd := exec.CommandContext(ctx, "git", args...)
|
||||
absPath, err := filepath.Abs(source)
|
||||
if err == nil {
|
||||
if !isBare {
|
||||
@@ -273,7 +286,7 @@ func (c *Parser) RepoPath(
|
||||
}
|
||||
}
|
||||
|
||||
return c.executeCommand(ctx, cmd, false)
|
||||
return c.executeCommand(ctx, cmd, false, c.waitDelay)
|
||||
}
|
||||
|
||||
// Staged parses the output of the `git diff` command for the `source` path.
|
||||
@@ -281,18 +294,19 @@ func (c *Parser) Staged(ctx context.Context, source string) (chan *Diff, error)
|
||||
// Provide the --cached flag to diff to get the diff of the staged changes.
|
||||
args := []string{"-C", source, "diff", "-p", "--cached", "--full-history", "--diff-filter=AM", "--date=iso-strict"}
|
||||
|
||||
cmd := exec.Command("git", args...)
|
||||
cmd := exec.CommandContext(ctx, "git", args...)
|
||||
|
||||
absPath, err := filepath.Abs(source)
|
||||
if err == nil {
|
||||
cmd.Env = append(cmd.Env, "GIT_DIR="+filepath.Join(absPath, ".git"))
|
||||
}
|
||||
|
||||
return c.executeCommand(ctx, cmd, true)
|
||||
return c.executeCommand(ctx, cmd, true, c.waitDelay)
|
||||
}
|
||||
|
||||
// executeCommand runs an exec.Cmd, reads stdout and stderr, and waits for the Cmd to complete.
|
||||
func (c *Parser) executeCommand(ctx context.Context, cmd *exec.Cmd, isStaged bool) (chan *Diff, error) {
|
||||
// waitDelay specifies how long to wait after context cancellation before forcefully killing the process.
|
||||
func (c *Parser) executeCommand(ctx context.Context, cmd *exec.Cmd, isStaged bool, waitDelay time.Duration) (chan *Diff, error) {
|
||||
diffChan := make(chan *Diff, 64)
|
||||
|
||||
stdOut, err := cmd.StdoutPipe()
|
||||
@@ -304,6 +318,9 @@ func (c *Parser) executeCommand(ctx context.Context, cmd *exec.Cmd, isStaged boo
|
||||
return diffChan, err
|
||||
}
|
||||
|
||||
// Set WaitDelay to allow the command additional time to exit after context cancellation
|
||||
cmd.WaitDelay = waitDelay
|
||||
|
||||
err = cmd.Start()
|
||||
if err != nil {
|
||||
return diffChan, err
|
||||
|
||||
@@ -1478,6 +1478,26 @@ func TestMaxCommitSize(t *testing.T) {
|
||||
}
|
||||
|
||||
}
|
||||
func TestWaitDelay(t *testing.T) {
|
||||
// Test that WithWaitDelay sets the waitDelay correctly
|
||||
customDelay := 10 * time.Second
|
||||
parser := NewParser(WithWaitDelay(customDelay))
|
||||
if parser.waitDelay != customDelay {
|
||||
t.Errorf("waitDelay not set correctly. Got: %v, expected: %v", parser.waitDelay, customDelay)
|
||||
}
|
||||
|
||||
// Test that default waitDelay is used when not specified
|
||||
defaultParser := NewParser()
|
||||
if defaultParser.waitDelay != defaultWaitDelay {
|
||||
t.Errorf("default waitDelay not set correctly. Got: %v, expected: %v", defaultParser.waitDelay, defaultWaitDelay)
|
||||
}
|
||||
|
||||
// Test that zero value is allowed (caller can set to 0 if they want no delay)
|
||||
zeroDelayParser := NewParser(WithWaitDelay(0))
|
||||
if zeroDelayParser.waitDelay != 0 {
|
||||
t.Errorf("zero waitDelay not set correctly. Got: %v, expected: 0", zeroDelayParser.waitDelay)
|
||||
}
|
||||
}
|
||||
|
||||
const commitLog = `commit e50b135fd29e91b2fbb25923797f5ecffe59f359
|
||||
Author: lionzxy <[email protected]>
|
||||
|
||||
Reference in New Issue
Block a user