fix(downloader): bound git network hangs and retry transient 504s

cmd/terraform/utils.go

@@ -130,7 +130,7 @@ func runHooksWithOutput(event h.HookEvent, cmd_ *cobra.Command, args []string, o
 	}
 
 	if hooks != nil && hooks.HasHooks() {
-		log.Info("Running hooks", "event", event)
+		log.Debug("Running hooks", "event", event)
 		if err := hooks.RunAll(event, &atmosConfig, &info, cmd_, args); err != nil {
 			return err
 		}

pkg/downloader/get_git.go

@@ -85,8 +85,98 @@ const (
 	gitCommand      = "git"
 	originRemote    = "origin"
 	gitArgSeparator = "--"
+
+	// Default network timeout knobs for HTTP(S) transports. The http.lowSpeedTime
+	// bails the connection when throughput falls below http.lowSpeedLimit
+	// (bytes/sec) for this many seconds. Without these, a stalled remote will
+	// hang until the caller's context cancels — which is typically 5+ minutes
+	// per command. Override per-environment via ATMOS_GIT_HTTP_LOW_SPEED_LIMIT
+	// and ATMOS_GIT_HTTP_LOW_SPEED_TIME.
+	defaultGitHTTPLowSpeedLimit = "1000"
+	defaultGitHTTPLowSpeedTime  = "30"
+
+	// Default SSH keepalive/connect timeouts (seconds). ConnectTimeout bounds
+	// initial TCP/SSH handshake; ServerAlive* drops the session after a stalled
+	// link. Override per-environment via ATMOS_GIT_SSH_CONNECT_TIMEOUT,
+	// ATMOS_GIT_SSH_SERVER_ALIVE_INTERVAL, and ATMOS_GIT_SSH_SERVER_ALIVE_COUNT_MAX.
+	defaultSSHConnectTimeoutSeconds = "30"
+	defaultSSHServerAliveInterval   = "15"
+	defaultSSHServerAliveCountMax   = "4"
+
+	// Environment variable names for overriding the defaults above.
+	envGitHTTPLowSpeedLimit   = "ATMOS_GIT_HTTP_LOW_SPEED_LIMIT"
+	envGitHTTPLowSpeedTime    = "ATMOS_GIT_HTTP_LOW_SPEED_TIME"
+	envSSHConnectTimeout      = "ATMOS_GIT_SSH_CONNECT_TIMEOUT"
+	envSSHServerAliveInterval = "ATMOS_GIT_SSH_SERVER_ALIVE_INTERVAL"
+	envSSHServerAliveCountMax = "ATMOS_GIT_SSH_SERVER_ALIVE_COUNT_MAX"
 )
 
+// numericEnvOr returns the value of envVar when set to a non-empty,
+// non-negative integer string; otherwise returns the supplied default. Anything
+// non-numeric is rejected so a typo can't disable the timeout silently (a
+// non-numeric value passed to git's http.lowSpeedTime would crash the command).
+// ATMOS_GIT_* knobs are runtime tuning read once per git command, not Atmos
+// config loaded through Viper — see precedent in pkg/config/git_root.go and
+// pkg/config/utils.go (ATMOS_VERSION_ENFORCEMENT).
+func numericEnvOr(envVar, defaultValue string) string {
+	v := strings.TrimSpace(os.Getenv(envVar)) //nolint:forbidigo
+	if v == "" {
+		return defaultValue
+	}
+	if n, err := strconv.Atoi(v); err != nil || n < 0 {
+		log.Warn("Ignoring invalid override; expected a non-negative integer",
+			"env", envVar, "value", v, "default", defaultValue)
+		return defaultValue
+	}
+	return v
+}
+
+// gitHTTPLowSpeedLimit returns the effective http.lowSpeedLimit (bytes/sec).
+func gitHTTPLowSpeedLimit() string {
+	return numericEnvOr(envGitHTTPLowSpeedLimit, defaultGitHTTPLowSpeedLimit)
+}
+
+// gitHTTPLowSpeedTime returns the effective http.lowSpeedTime (seconds).
+func gitHTTPLowSpeedTime() string {
+	return numericEnvOr(envGitHTTPLowSpeedTime, defaultGitHTTPLowSpeedTime)
+}
+
+// sshConnectTimeoutSeconds returns the effective SSH ConnectTimeout (seconds).
+func sshConnectTimeoutSeconds() string {
+	return numericEnvOr(envSSHConnectTimeout, defaultSSHConnectTimeoutSeconds)
+}
+
+// sshServerAliveInterval returns the effective SSH ServerAliveInterval (seconds).
+func sshServerAliveInterval() string {
+	return numericEnvOr(envSSHServerAliveInterval, defaultSSHServerAliveInterval)
+}
+
+// sshServerAliveCountMax returns the effective SSH ServerAliveCountMax (count).
+func sshServerAliveCountMax() string {
+	return numericEnvOr(envSSHServerAliveCountMax, defaultSSHServerAliveCountMax)
+}
+
+// gitNetworkConfigArgs returns the leading "-c http.lowSpeedLimit=…" /
+// "-c http.lowSpeedTime=…" arguments that should precede every network-touching
+// git subcommand. These are safe no-ops for non-HTTP transports — git only
+// applies http.* config when an HTTP(S) URL is involved.
+func gitNetworkConfigArgs() []string {
+	return []string{
+		"-c", "http.lowSpeedLimit=" + gitHTTPLowSpeedLimit(),
+		"-c", "http.lowSpeedTime=" + gitHTTPLowSpeedTime(),
+	}
+}
+
+// gitCommandContext builds an *exec.Cmd that prepends the standard network
+// timeout knobs in front of subcommand args. All git invocations that touch
+// the network (clone, fetch, pull, ls-remote, submodule update) must go through
+// this helper rather than exec.CommandContext directly.
+func gitCommandContext(ctx context.Context, subArgs ...string) *exec.Cmd {
+	args := append(gitNetworkConfigArgs(), subArgs...)
+	// #nosec G702 -- gitCommand is constant; subArgs are validated upstream (URLs use "--" separators).
+	return exec.CommandContext(ctx, gitCommand, args...)
+}
+
 // gitOperationParams holds parameters for git operations to reduce function arguments.
 type gitOperationParams struct {
 	ctx        context.Context
@@ -232,12 +322,12 @@ func (g *CustomGitGetter) GetCustom(dst string, u *url.URL) error {
 
 // setupGitEnv sets up the environment for the given command. This is used to
 // pass configuration data to git and ssh and enables advanced cloning methods.
+//
+// SSH timeouts (ConnectTimeout, ServerAliveInterval, ServerAliveCountMax) are
+// always applied — even when no sshKeyFile is provided — so that a stalled SSH
+// link surfaces an error within ~60s instead of consuming the full context
+// budget. For HTTPS the equivalent guards are set via gitNetworkConfigArgs().
 func setupGitEnv(cmd *exec.Cmd, sshKeyFile string) {
-	// If there's no sshKeyFile argument to deal with, we can skip this
-	// entirely.
-	if sshKeyFile == "" {
-		return
-	}
 	const gitSSHCommand = "GIT_SSH_COMMAND="
 	var sshCmd []string
 
@@ -259,11 +349,22 @@ func setupGitEnv(cmd *exec.Cmd, sshKeyFile string) {
 		sshCmd = []string{gitSSHCommand + "ssh"}
 	}
 
-	// We have an SSH key temp file configured, tell ssh about this.
-	if runtime.GOOS == "windows" {
-		sshKeyFile = strings.ReplaceAll(sshKeyFile, `\`, `/`)
+	// Always enforce SSH timeouts — these bound the worst-case time a hung
+	// SSH connection can stall the pipeline.
+	sshCmd = append(
+		sshCmd,
+		"-o", "ConnectTimeout="+sshConnectTimeoutSeconds(),
+		"-o", "ServerAliveInterval="+sshServerAliveInterval(),
+		"-o", "ServerAliveCountMax="+sshServerAliveCountMax(),
+	)
+
+	// If a temp SSH key file is configured, tell ssh about it.
+	if sshKeyFile != "" {
+		if runtime.GOOS == "windows" {
+			sshKeyFile = strings.ReplaceAll(sshKeyFile, `\`, `/`)
+		}
+		sshCmd = append(sshCmd, "-i", sshKeyFile)
 	}
-	sshCmd = append(sshCmd, "-i", sshKeyFile)
 	env = append(env, strings.Join(sshCmd, " "))
 
 	cmd.Env = env
@@ -286,7 +387,8 @@ func getRunCommand(cmd *exec.Cmd) error {
 				errUtils.ErrGitCommandExited,
 				cmd.Path,
 				status.ExitStatus(),
-				buf.String())
+				buf.String(),
+			)
 		}
 	}
 
@@ -333,8 +435,10 @@ func isRetryableGitError(err error) bool {
 		"timeout",
 		"timed out",
 		"eof",
+		"early eof",
 		"temporary failure",
 		"could not read from remote",
+		"could not resolve host",
 		"the remote end hung up",
 		"ssl",
 		"tls",
@@ -344,6 +448,9 @@ func isRetryableGitError(err error) bool {
 		"internal server error",
 		"bad gateway",
 		"gateway timeout",
+		// Matches git's HTTP transport errors: "The requested URL returned error: 5xx".
+		// Captures 500/502/503/504 without needing the full status text.
+		"returned error: 5",
 	}
 
 	for _, pattern := range transientPatterns {
@@ -376,7 +483,9 @@ func removeCaseInsensitiveGitDirectory(dst string) error {
 func findRemoteDefaultBranch(ctx context.Context, u *url.URL) string {
 	var stdoutbuf bytes.Buffer
 	// #nosec G204 -- The URL is validated and we use "--" separator to prevent command injection.
-	cmd := exec.CommandContext(ctx, gitCommand, "ls-remote", "--symref", gitArgSeparator, u.String(), "HEAD")
+	cmd := gitCommandContext(ctx, "ls-remote", "--symref", gitArgSeparator, u.String(), "HEAD")
+	// Apply SSH timeouts via env in case the URL is ssh://; harmless for HTTP(S).
+	setupGitEnv(cmd, "")
 	cmd.Stdout = &stdoutbuf
 	err := cmd.Run()
 	matches := lsRemoteSymRefRegexp.FindStringSubmatch(stdoutbuf.String())
@@ -454,7 +563,7 @@ func (g *CustomGitGetter) clone(params *gitOperationParams) error {
 	}
 	args = append(args, gitArgSeparator, u.String(), dst)
 
-	cmd := exec.CommandContext(ctx, gitCommand, args...)
+	cmd := gitCommandContext(ctx, args...)
 	setupGitEnv(cmd, sshKeyFile)
 	err := g.getRunCommandWithRetry(ctx, cmd)
 	if err != nil {
@@ -519,16 +628,18 @@ func (g *CustomGitGetter) update(params *gitOperationParams) error {
 	}
 
 	// Fetch the remote ref
-	cmd = exec.CommandContext(ctx, gitCommand, "fetch", "--tags")
+	cmd = gitCommandContext(ctx, "fetch", "--tags")
 	cmd.Dir = dst
+	setupGitEnv(cmd, sshKeyFile)
 	err = g.getRunCommandWithRetry(ctx, cmd)
 	if err != nil {
 		return err
 	}
 
 	// Fetch the remote ref
-	cmd = exec.CommandContext(ctx, gitCommand, "fetch", originRemote, gitArgSeparator, ref)
+	cmd = gitCommandContext(ctx, "fetch", originRemote, gitArgSeparator, ref)
 	cmd.Dir = dst
+	setupGitEnv(cmd, sshKeyFile)
 	err = g.getRunCommandWithRetry(ctx, cmd)
 	if err != nil {
 		return err
@@ -551,9 +662,9 @@ func (g *CustomGitGetter) update(params *gitOperationParams) error {
 	// Pull the latest changes from the ref branch
 	if depth > 0 {
 		// #nosec G204 -- The ref is from query parameters and we use "--" separator to prevent command injection.
-		cmd = exec.CommandContext(ctx, gitCommand, "pull", originRemote, "--depth", strconv.Itoa(depth), "--ff-only", gitArgSeparator, ref)
+		cmd = gitCommandContext(ctx, "pull", originRemote, "--depth", strconv.Itoa(depth), "--ff-only", gitArgSeparator, ref)
 	} else {
-		cmd = exec.CommandContext(ctx, gitCommand, "pull", originRemote, "--ff-only", gitArgSeparator, ref)
+		cmd = gitCommandContext(ctx, "pull", originRemote, "--ff-only", gitArgSeparator, ref)
 	}
 
 	cmd.Dir = dst
@@ -567,7 +678,7 @@ func (g *CustomGitGetter) fetchSubmodules(ctx context.Context, dst, sshKeyFile s
 	if depth > 0 {
 		args = append(args, "--depth", strconv.Itoa(depth))
 	}
-	cmd := exec.CommandContext(ctx, gitCommand, args...)
+	cmd := gitCommandContext(ctx, args...)
 	cmd.Dir = dst
 	setupGitEnv(cmd, sshKeyFile)
 	return g.getRunCommandWithRetry(ctx, cmd)

pkg/downloader/get_git_network_test.go

@@ -0,0 +1,121 @@
+package downloader
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGitNetworkConfigArgs_AlwaysIncludesLowSpeedKnobs(t *testing.T) {
+	args := gitNetworkConfigArgs()
+
+	// Args must always include the low-speed limit/time pair so a stalled
+	// HTTP(S) transfer surfaces an error within ~30 s instead of hanging
+	// until the caller's context cancels.
+	assert.Contains(t, args, "-c")
+	assert.Contains(t, args, "http.lowSpeedLimit="+defaultGitHTTPLowSpeedLimit)
+	assert.Contains(t, args, "http.lowSpeedTime="+defaultGitHTTPLowSpeedTime)
+}
+
+func TestGitCommandContext_PrependsNetworkArgs(t *testing.T) {
+	cmd := gitCommandContext(t.Context(), "clone", "https://example.com/repo.git", "/tmp/dst")
+
+	// cmd.Args[0] is the resolved git binary path; subsequent args must lead
+	// with the network-tuning -c flags before the subcommand.
+	joined := strings.Join(cmd.Args, " ")
+	assert.Contains(t, joined, "-c http.lowSpeedLimit="+defaultGitHTTPLowSpeedLimit)
+	assert.Contains(t, joined, "-c http.lowSpeedTime="+defaultGitHTTPLowSpeedTime)
+
+	// The actual subcommand must still be present after the -c flags.
+	assert.Contains(t, joined, "clone")
+	assert.Contains(t, joined, "https://example.com/repo.git")
+}
+
+// lastGitSSHCommand returns the value of the last GIT_SSH_COMMAND= entry in env.
+// Effective env follows last-wins semantics, and setupGitEnv may leave a stale
+// empty entry behind (its filter only removes non-empty existing values), so
+// reading the first match is not deterministic. Always pick the last match.
+func lastGitSSHCommand(env []string) string {
+	var got string
+	for _, e := range env {
+		if strings.HasPrefix(e, "GIT_SSH_COMMAND=") {
+			got = e
+		}
+	}
+	return got
+}
+
+func TestSetupGitEnv_AlwaysSetsSSHTimeouts(t *testing.T) {
+	// Scrub any inherited GIT_SSH_COMMAND so the env baseline is deterministic.
+	t.Setenv("GIT_SSH_COMMAND", "")
+
+	cmd := gitCommandContext(t.Context(), "ls-remote", "ssh://git@example.com/repo.git", "HEAD")
+	setupGitEnv(cmd, "")
+
+	// Even without an sshKeyFile we want the GIT_SSH_COMMAND env var so SSH
+	// transports are bounded by ConnectTimeout / ServerAlive*.
+	sshEnv := lastGitSSHCommand(cmd.Env)
+	assert.NotEmpty(t, sshEnv, "GIT_SSH_COMMAND must be set even without an sshKeyFile")
+	assert.Contains(t, sshEnv, "ConnectTimeout="+defaultSSHConnectTimeoutSeconds)
+	assert.Contains(t, sshEnv, "ServerAliveInterval="+defaultSSHServerAliveInterval)
+	assert.Contains(t, sshEnv, "ServerAliveCountMax="+defaultSSHServerAliveCountMax)
+}
+
+func TestSetupGitEnv_WithSSHKeyStillSetsTimeouts(t *testing.T) {
+	t.Setenv("GIT_SSH_COMMAND", "")
+
+	cmd := gitCommandContext(t.Context(), "clone", "ssh://git@example.com/repo.git", "/tmp/dst")
+	setupGitEnv(cmd, "/tmp/fake-key")
+
+	sshEnv := lastGitSSHCommand(cmd.Env)
+	assert.NotEmpty(t, sshEnv)
+	assert.Contains(t, sshEnv, "-i /tmp/fake-key", "must still inject the SSH key file")
+	assert.Contains(t, sshEnv, "ConnectTimeout="+defaultSSHConnectTimeoutSeconds, "must still set timeouts when a key is configured")
+}
+
+func TestNumericEnvOr_AcceptsValidOverride(t *testing.T) {
+	t.Setenv(envGitHTTPLowSpeedTime, "60")
+	assert.Equal(t, "60", gitHTTPLowSpeedTime(), "valid override should win over default")
+}
+
+func TestNumericEnvOr_RejectsNonNumericOverride(t *testing.T) {
+	// A typo like "60s" must not silently disable the timeout (git would error
+	// or interpret it as 0). Fall back to the safe default.
+	t.Setenv(envGitHTTPLowSpeedTime, "60s")
+	assert.Equal(t, defaultGitHTTPLowSpeedTime, gitHTTPLowSpeedTime())
+}
+
+func TestNumericEnvOr_RejectsNegativeOverride(t *testing.T) {
+	t.Setenv(envGitHTTPLowSpeedTime, "-1")
+	assert.Equal(t, defaultGitHTTPLowSpeedTime, gitHTTPLowSpeedTime())
+}
+
+func TestNumericEnvOr_EmptyValueFallsBackToDefault(t *testing.T) {
+	t.Setenv(envGitHTTPLowSpeedTime, "")
+	assert.Equal(t, defaultGitHTTPLowSpeedTime, gitHTTPLowSpeedTime())
+}
+
+func TestGitNetworkConfigArgs_HonorsEnvOverrides(t *testing.T) {
+	t.Setenv(envGitHTTPLowSpeedLimit, "2048")
+	t.Setenv(envGitHTTPLowSpeedTime, "90")
+
+	args := gitNetworkConfigArgs()
+	assert.Contains(t, args, "http.lowSpeedLimit=2048")
+	assert.Contains(t, args, "http.lowSpeedTime=90")
+}
+
+func TestSetupGitEnv_HonorsSSHTimeoutEnvOverrides(t *testing.T) {
+	t.Setenv("GIT_SSH_COMMAND", "")
+	t.Setenv(envSSHConnectTimeout, "45")
+	t.Setenv(envSSHServerAliveInterval, "20")
+	t.Setenv(envSSHServerAliveCountMax, "6")
+
+	cmd := gitCommandContext(t.Context(), "ls-remote", "ssh://git@example.com/repo.git", "HEAD")
+	setupGitEnv(cmd, "")
+
+	sshEnv := lastGitSSHCommand(cmd.Env)
+	assert.Contains(t, sshEnv, "ConnectTimeout=45")
+	assert.Contains(t, sshEnv, "ServerAliveInterval=20")
+	assert.Contains(t, sshEnv, "ServerAliveCountMax=6")
+}