Add optional git clone timeout (#4597)
Lint / golangci-lint (push) Waiting to run
Lint / semgrep (push) Waiting to run
Release / Release (push) Waiting to run
Scan for secrets / test (push) Waiting to run
Test / test (push) Waiting to run
Test / test-community (push) Waiting to run
Lint / golangci-lint (push) Waiting to run
Lint / semgrep (push) Waiting to run
Release / Release (push) Waiting to run
Scan for secrets / test (push) Waiting to run
Test / test (push) Waiting to run
Test / test-community (push) Waiting to run
We have identified some cases in which it is preferable to time a clone out instead of waiting forever. These situations are unusual, so the CLI option to enable this (which I added for testing) is hidden so that we minimize the risk of baking this option into the interface.
This commit is contained in:
@@ -88,6 +88,7 @@ var (
|
||||
// Add feature flags
|
||||
forceSkipBinaries = cli.Flag("force-skip-binaries", "Force skipping binaries.").Bool()
|
||||
forceSkipArchives = cli.Flag("force-skip-archives", "Force skipping archives.").Bool()
|
||||
gitCloneTimeout = cli.Flag("git-clone-timeout", "Maximum time to spend cloning a repository, as a duration.").Hidden().Duration()
|
||||
skipAdditionalRefs = cli.Flag("skip-additional-refs", "Skip additional references.").Bool()
|
||||
userAgentSuffix = cli.Flag("user-agent-suffix", "Suffix to add to User-Agent.").String()
|
||||
|
||||
@@ -451,6 +452,10 @@ func run(state overseer.State) {
|
||||
feature.ForceSkipArchives.Store(true)
|
||||
}
|
||||
|
||||
if gitCloneTimeout != nil {
|
||||
feature.GitGloneTimeoutDuration.Store(int64(*gitCloneTimeout))
|
||||
}
|
||||
|
||||
if *skipAdditionalRefs {
|
||||
feature.SkipAdditionalRefs.Store(true)
|
||||
}
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
package feature
|
||||
|
||||
import "sync/atomic"
|
||||
import (
|
||||
"sync/atomic"
|
||||
)
|
||||
|
||||
var (
|
||||
ForceSkipBinaries atomic.Bool
|
||||
ForceSkipArchives atomic.Bool
|
||||
GitGloneTimeoutDuration atomic.Int64
|
||||
SkipAdditionalRefs atomic.Bool
|
||||
EnableAPKHandler atomic.Bool
|
||||
UserAgentSuffix AtomicString
|
||||
|
||||
+14
-3
@@ -448,6 +448,7 @@ type cloneParams struct {
|
||||
args []string
|
||||
clonePath string
|
||||
authInUrl bool
|
||||
timeout time.Duration
|
||||
}
|
||||
|
||||
// CloneRepo orchestrates the cloning of a given Git repository, returning its local path
|
||||
@@ -473,7 +474,9 @@ func CloneRepo(ctx context.Context, userInfo *url.Userinfo, gitURL string, clone
|
||||
}
|
||||
}
|
||||
|
||||
repo, err := executeClone(ctx, cloneParams{userInfo, gitURL, args, path, authInUrl})
|
||||
timeout := time.Duration(feature.GitGloneTimeoutDuration.Load())
|
||||
|
||||
repo, err := executeClone(ctx, cloneParams{userInfo, gitURL, args, path, authInUrl, timeout})
|
||||
if err != nil {
|
||||
// DO NOT FORGET TO CLEAN UP THE CLONE PATH HERE!!
|
||||
// If we don't, we'll end up with a bunch of orphaned directories in the temp dir.
|
||||
@@ -529,10 +532,16 @@ func executeClone(ctx context.Context, params cloneParams) (*git.Repository, err
|
||||
}
|
||||
}
|
||||
|
||||
var cancel context.CancelFunc
|
||||
if params.timeout > 0 {
|
||||
ctx, cancel = context.WithTimeout(ctx, params.timeout)
|
||||
defer cancel()
|
||||
}
|
||||
|
||||
gitArgs = append(gitArgs, "--quiet")
|
||||
gitArgs = append(gitArgs, params.args...)
|
||||
gitArgs = append(gitArgs, cloneURL.String(), params.clonePath)
|
||||
cloneCmd := exec.Command("git", gitArgs...)
|
||||
cloneCmd := exec.CommandContext(ctx, "git", gitArgs...)
|
||||
|
||||
safeURL, secretForRedaction, err := stripPassword(params.gitURL)
|
||||
if err != nil {
|
||||
@@ -559,7 +568,9 @@ func executeClone(ctx context.Context, params cloneParams) (*git.Repository, err
|
||||
}
|
||||
logger.V(3).Info("git subcommand finished", "output", output)
|
||||
|
||||
if cloneCmd.ProcessState == nil {
|
||||
if common.IsDone(ctx) {
|
||||
return nil, fmt.Errorf("git clone timed out (after %s)", time.Since(start))
|
||||
} else if cloneCmd.ProcessState == nil {
|
||||
return nil, fmt.Errorf("clone command exited with no output")
|
||||
} else if cloneCmd.ProcessState.ExitCode() != 0 {
|
||||
logger.V(1).Info("git clone failed", "error", err)
|
||||
|
||||
@@ -8,9 +8,11 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kylelemons/godebug/pretty"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/trufflesecurity/trufflehog/v3/pkg/feature"
|
||||
"google.golang.org/protobuf/types/known/anypb"
|
||||
|
||||
"github.com/trufflesecurity/trufflehog/v3/pkg/common"
|
||||
@@ -23,6 +25,40 @@ import (
|
||||
"github.com/trufflesecurity/trufflehog/v3/pkg/sourcestest"
|
||||
)
|
||||
|
||||
func TestClone_Timeout(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
t.Run("an unset timeout should not cause a timeout", func(t *testing.T) {
|
||||
_, _, err := CloneRepo(
|
||||
ctx,
|
||||
nil,
|
||||
"https://github.com/dustin-decker/secretsandstuff.git",
|
||||
"",
|
||||
false)
|
||||
|
||||
// There shouldn't be an error - but if there is because of some other problem, we don't want this test to fail
|
||||
if err != nil {
|
||||
assert.NotContains(t, err.Error(), "timed out")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a clone that times out should time out", func(t *testing.T) {
|
||||
feature.GitGloneTimeoutDuration.Store(int64(1 * time.Nanosecond))
|
||||
t.Cleanup(func() { feature.GitGloneTimeoutDuration.Store(0) })
|
||||
|
||||
_, _, err := CloneRepo(
|
||||
ctx,
|
||||
nil,
|
||||
"https://github.com/dustin-decker/secretsandstuff.git",
|
||||
"",
|
||||
false)
|
||||
|
||||
if assert.Error(t, err) {
|
||||
assert.Contains(t, err.Error(), "timed out")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestSource_Scan(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
Reference in New Issue
Block a user