Retry git clone on transient network errors

[shahzadhaider1]

Issue

Currently a repo clone gets exactly one attempt. If the connection drops mid-transfer (curl 56 Recv failure: Connection reset by peer, HTTP/2 stream reset by server, early EOF, etc.), the whole scan of that repo fails, even though an immediate retry would almost certainly succeed. We've seen this repeatedly with large repos, where long transfer times make a transient network blip much more likely to hit.

Solution

This PR makes CloneRepo retry when the failure is classified as a network error (using the existing ClassifyCloneError, which was previously only used for metrics). Up to 3 total attempts with a short backoff (5s, then 10s), each starting from a fresh clone directory.
All other failure classes - auth, permissions, rate limits, not found, clone timeouts, still fail immediately, same as before.

Along the way, the directory setup logic was extracted into createClonePath so the initial attempt and retries share one code path. Temp directories keep their os.MkdirTemp 0700 permissions on retry, and --clone-path directories keep 0755.

Notes:

• This applies to all git-based sources, not just GitHub, since they all clone through CloneRepo.
• Clone timeouts are deliberately not retried. Note that the timeout now applies per attempt, so a repo that repeatedly fails with network errors near the cap could take up to 3x the configured timeout in the worst case. Happy to make it a shared budget across attempts if that's preferred.
• Clone failure metrics are recorded per attempt, so failure counts may tick up slightly even as overall clone reliability improves.

Tests:

• Added unit tests for the isRetryableCloneError and for createClonePath.
• Also verified the retry flow end-to-end with a stub git binary that fails the first attempt and succeeds on the second.

Checklist:

• Tests passing (make test-community)?
• Lint passing (make lint this requires golangci-lint)?

---

Note

Medium Risk
Changes shared clone orchestration for all git-based sources and can extend wall-clock clone time up to 3× the per-attempt timeout on repeated network failures.

Overview
CloneRepo now retries up to 3 times when a failure is classified as a transient network error via existing ClassifyCloneError (e.g. connection reset, HTTP/2 stream reset, early EOF). Each retry removes the partial clone, recreates the target directory, waits with linear backoff (5s × attempt), and logs before retrying. Auth, 403/429, not found, clone timeouts, and cancelled context still fail immediately with cleanup unchanged.

Clone directory setup is moved into createClonePath (temp 0700 vs --clone-path trufflehog-<repo> at 0755) so the first attempt and retries share one path. Worst case, a repo that keeps hitting network errors near the timeout cap can spend up to 3× the configured clone timeout because the timeout applies per attempt.

Unit tests cover isRetryableCloneError and createClonePath (permissions, uniqueness, URL edge cases).

^{Reviewed by Cursor Bugbot for commit 8f39970. Bugbot is set up for automated code reviews on this repo. Configure here.}

[MuneebUllahKhan222]

LGTM. Just two small refactoring changes that can be done.

[MuneebUllahKhan222]

Better to refactor this in a table test.

[MuneebUllahKhan222]

I think we can re-write this function better like this: (To make it easier to read)

func CloneRepo(ctx context.Context, userInfo *url.Userinfo, gitURL string, clonePath string, authInUrl bool, args ...string) (string, *git.Repository, error) {
      timeout := time.Duration(feature.GitCloneTimeoutDuration.Load())

      var path string
      var repo *git.Repository
      var err error
      for attempt := 1; attempt <= maxCloneAttempts; attempt++ {
              path, err = createClonePath(gitURL, clonePath)
      var err error
      for attempt := 1; attempt <= maxCloneAttempts; attempt++ {
              path, err = createClonePath(gitURL, clonePath)
              if err != nil {
                      return "", nil, err
              }

              repo, err = executeClone(ctx, cloneParams{userInfo, gitURL, args, path, authInUrl, timeout})
              if err == nil {
                      return path, repo, nil
              }

              if attempt >= maxCloneAttempts || !isRetryableCloneError(err) || common.IsDone(ctx) {
                      CleanOnError(&err, path)
                      return "", nil, err
              }

              if rmErr := os.RemoveAll(path); rmErr != nil {
                      return "", nil, fmt.Errorf("failed to clean clone path for retry: %w (original clone error: %w)", rmErr, err)
              }

              ctx.Logger().Info("git clone interrupted by network error; retrying",
                      "attempt", attempt, "max_attempts", maxCloneAttempts, "error", err.Error())

              select {
              case <-ctx.Done():
                      CleanOnError(&err, path)
                      return "", nil, err
              case <-time.After(cloneRetryBackoff * time.Duration(attempt)):
              }
      }
      return "", nil, err
}

[unsmith]

Might be a typo at the top of this block (a couple of duplicated lines?) but otherwise I'm on board with this. I don't feel strongly about it because the logic does look sound as-is, but if you both agree it would be a readability improvement then I'd say go ahead. I'll approve anyway.

[unsmith]

Clone timeouts are deliberately not retried. Note that the timeout now applies per attempt, so a repo that repeatedly fails with network errors near the cap could take up to 3x the configured timeout in the worst case. Happy to make it a shared budget across attempts if that's preferred.

I think this is fine as-is, but I'd wonder what sort of time loss we'd be absorbing if we did retry timeouts. Maybe as a future improvement, if we have a sense of how long a clone can take.

[unsmith]

Might be a typo at the top of this block (a couple of duplicated lines?) but otherwise I'm on board with this. I don't feel strongly about it because the logic does look sound as-is, but if you both agree it would be a readability improvement then I'd say go ahead. I'll approve anyway.