diff --git a/github.go b/github.go index c06f820..e3a2c1a 100644 --- a/github.go +++ b/github.go @@ -195,6 +195,21 @@ func gitClone(ctx context.Context, repoURL, dest, token string) *exec.Cmd { return cmd } +// cloneRepo clones with a timeout and returns git's output with the token masked. +func cloneRepo(repoURL, dest, token string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), cloneTimeout) + defer cancel() + output, err := gitClone(ctx, repoURL, dest, token).CombinedOutput() + text := string(output) + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + text = "timed out after " + cloneTimeout.String() + } + if token != "" { + text = strings.ReplaceAll(text, token, "***") + } + return text, err +} + // cloneFailure turns git output into a user-facing message and whether signing in (or // granting the app access) could fix it. func (g githubApp) cloneFailure(output string, signedIn bool) (message string, authRequired string) { diff --git a/github_test.go b/github_test.go index 1fbf1d8..81607e2 100644 --- a/github_test.go +++ b/github_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "net/url" "os/exec" + "path/filepath" "strings" "testing" "time" @@ -173,3 +174,29 @@ func TestCloneFailureMessages(t *testing.T) { } } } + +func TestCloneRepo(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + source := t.TempDir() + for _, args := range [][]string{{"init", "-q"}, {"-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "--allow-empty", "-m", "x"}} { + if out, err := exec.Command("git", append([]string{"-C", source}, args...)...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v %s", args, err, out) + } + } + if out, err := cloneRepo(source, filepath.Join(t.TempDir(), "ok"), ""); err != nil { + t.Fatalf("local clone failed: %v %s", err, out) + } + + out, err := cloneRepo(filepath.Join(source, "missing-ghu_secret"), filepath.Join(t.TempDir(), "x"), "ghu_secret") + if err == nil || strings.Contains(out, "timed out") || strings.Contains(out, "ghu_secret") || !strings.Contains(out, "***") { + t.Fatalf("failed clone = %v %q; want a masked, non-timeout error", err, out) + } + + defer func(old time.Duration) { cloneTimeout = old }(cloneTimeout) + cloneTimeout = time.Nanosecond + if out, err := cloneRepo(source, filepath.Join(t.TempDir(), "slow"), ""); err == nil || !strings.HasPrefix(out, "timed out") { + t.Fatalf("expired clone = %v %q; want timeout", err, out) + } +} diff --git a/main.go b/main.go index f7e95d8..8db0f35 100644 --- a/main.go +++ b/main.go @@ -3,7 +3,6 @@ package main import ( "bufio" "bytes" - "context" "crypto/rand" "embed" "encoding/json" @@ -50,10 +49,13 @@ const ( idleExpiry = time.Hour // snapshots unused and jobs untouched for this long are dropped maxSnapshots = 8 // ponytail: fixed caps sized for the 2GB container; make them configurable if memory changes maxIndexing = 2 // concurrent index runs; each already uses every CPU - cloneTimeout = 10 * time.Minute ) -var sseKeepAlive = 15 * time.Second // variable so tests can shorten it +// Variables so tests can shorten them. +var ( + sseKeepAlive = 15 * time.Second + cloneTimeout = 10 * time.Minute +) type Server struct { jobsMu sync.Mutex @@ -188,17 +190,7 @@ func (s *Server) createJob(w http.ResponseWriter, r *http.Request) { token := s.githubAuthFor(owner).token go func() { checkout := filepath.Join(root, "repo") - ctx, cancel := context.WithTimeout(context.Background(), cloneTimeout) - output, err := gitClone(ctx, url, checkout, token).CombinedOutput() - cancel() - if err != nil { - text := string(output) - if ctx.Err() != nil { - text = "timed out after " + cloneTimeout.String() - } - if token != "" { - text = strings.ReplaceAll(text, token, "***") - } + if text, err := cloneRepo(url, checkout, token); err != nil { message, authRequired := s.github.cloneFailure(text, token != "") s.failJob(id, message, authRequired) s.removeWorkspace(root)