PR6: harden repo source cache (URL-hashed mirrors + self-contained checkouts)

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## [Unreleased]
+
+### Changed
+- Cloned repos now use URL-hashed bare mirrors (`~/.cache/harness-openshell/mirrors/`)
+  plus per-run, self-contained checkouts (`~/.cache/harness-openshell/checkouts/`)
+  instead of the basename-keyed `repos/` cache. Distinct repositories that share a
+  basename no longer collide, and concurrent runs of the same repository no longer
+  share a working tree. Each checkout is a real repository with its own `.git`, so
+  git keeps working inside the sandbox after upload. The old
+  `~/.cache/harness-openshell/repos/` directory is orphaned and safe to delete
+  manually.
+
 ## [0.3.0] - 2026-06-17
 
 ### Added

cmd/executor.go

@@ -4,10 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"os/exec"
-	"path"
 	"path/filepath"
-	"strings"
 	"time"
 
 	"github.com/stackrox/harness-openshell/internal/agent"
@@ -18,6 +15,7 @@ import (
 	"github.com/stackrox/harness-openshell/internal/plan"
 	"github.com/stackrox/harness-openshell/internal/reconcile"
 	"github.com/stackrox/harness-openshell/internal/run"
+	"github.com/stackrox/harness-openshell/internal/source"
 	"github.com/stackrox/harness-openshell/internal/status"
 )
 
@@ -31,17 +29,17 @@ const reconcileTimeout = 60 * time.Second
 var DefaultAgentConfig []byte
 
 type upLocalOpts struct {
-	harnessDir      string
-	gw              gateway.Gateway
-	target          openshell.Target
-	agentCfg        *agent.AgentConfig
-	agentPath       string
-	sandboxName     string
-	noTTY           bool
-	setupOnly       bool
-	harness         *agent.Harness
-	newClient       openshell.Factory
-	retrySleep      time.Duration
+	harnessDir  string
+	gw          gateway.Gateway
+	target      openshell.Target
+	agentCfg    *agent.AgentConfig
+	agentPath   string
+	sandboxName string
+	noTTY       bool
+	setupOnly   bool
+	harness     *agent.Harness
+	newClient   openshell.Factory
+	retrySleep  time.Duration
 }
 
 func upLocal(opts upLocalOpts) error {
@@ -94,7 +92,11 @@ func upLocal(opts upLocalOpts) error {
 	// Clone repo outside the sandbox so git credentials never enter it.
 	var repoUpload *gateway.Upload
 	if agentCfg.Repo != "" {
-		upload, cleanup, err := cloneRepo(agentCfg.Repo, agentCfg.RepoRef)
+		runID, err := source.NewRunID()
+		if err != nil {
+			return err
+		}
+		upload, cleanup, err := cloneRepo(agentCfg.Repo, agentCfg.RepoRef, runID)
 		if err != nil {
 			return fmt.Errorf("cloning repo: %w", err)
 		}
@@ -201,116 +203,35 @@ func upLocal(opts upLocalOpts) error {
 	})
 }
 
-// cloneRepo clones or updates a cached git repository and returns an Upload
-// that places it at /sandbox/<repo-name>. Repos are cached in
-// ~/.cache/harness-openshell/repos/<repo-name>/ so subsequent runs only fetch
-// deltas. The clone happens outside the sandbox so git credentials never enter
-// it. Returns a cleanup function (no-op since the cache is persistent).
-func cloneRepo(repo, ref string) (gateway.Upload, func(), error) {
-	repoName := strings.TrimSuffix(path.Base(repo), ".git")
-
+// cloneRepo prepares an isolated per-run checkout of repo at ref and returns an
+// Upload that places it at /sandbox/<repo-name>, plus a cleanup that removes the
+// checkout. The mirror + checkout are built on the host so git credentials never
+// enter the sandbox; see internal/source for the URL-hashed mirror + per-run
+// checkout layout that keeps distinct same-basename repos and concurrent runs
+// from colliding.
+func cloneRepo(repo, ref, runID string) (gateway.Upload, func(), error) {
 	if ref != "" {
 		status.Infof("Repo:  %s (ref: %s)", repo, ref)
 	} else {
 		status.Infof("Repo:  %s", repo)
 	}
 
-	cacheDir, err := repoCacheDir(repoName)
+	cache, err := source.DefaultCache()
 	if err != nil {
 		return gateway.Upload{}, nil, err
 	}
-
-	if isGitRepo(cacheDir) {
-		if err := fetchRepo(cacheDir, ref); err != nil {
-			return gateway.Upload{}, nil, err
-		}
-		status.OKf("Updated %s (cached)", repoName)
-	} else {
-		if err := freshClone(repo, ref, cacheDir); err != nil {
-			return gateway.Upload{}, nil, fmt.Errorf("git clone %s: %w", repo, err)
-		}
-		status.OKf("Cloned %s", repoName)
-	}
-
-	return gateway.Upload{Src: cacheDir, Dst: "/sandbox"}, func() {}, nil
-}
-
-func repoCacheDir(repoName string) (string, error) {
-	home, err := os.UserHomeDir()
+	prepared, err := cache.Prepare(repo, ref, runID)
 	if err != nil {
-		return "", fmt.Errorf("determining home dir: %w", err)
-	}
-	dir := filepath.Join(home, ".cache", "harness-openshell", "repos", repoName)
-	if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil {
-		return "", fmt.Errorf("creating cache dir: %w", err)
-	}
-	return dir, nil
-}
-
-func isGitRepo(dir string) bool {
-	_, err := os.Stat(filepath.Join(dir, ".git"))
-	return err == nil
-}
-
-func freshClone(repo, ref, dest string) error {
-	args := []string{"clone", "--depth", "1"}
-	if ref != "" {
-		args = append(args, "--branch", ref)
-	}
-	args = append(args, repo, dest)
-	cmd := exec.Command("git", args...)
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
-	if err := cmd.Run(); err != nil {
-		return err
-	}
-	return initSubmodules(dest)
-}
-
-func fetchRepo(dir, ref string) error {
-	fetchArgs := []string{"-C", dir, "fetch", "--depth", "1", "origin"}
-	if ref != "" {
-		fetchArgs = append(fetchArgs, ref)
-	}
-	cmd := exec.Command("git", fetchArgs...)
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
-	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("git fetch: %w", err)
-	}
-
-	target := "FETCH_HEAD"
-	if ref == "" {
-		target = "origin/HEAD"
-	}
-	cmd = exec.Command("git", "-C", dir, "checkout", target, "--force")
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
-	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("git checkout %s: %w", target, err)
-	}
-
-	if err := initSubmodules(dir); err != nil {
-		return err
+		return gateway.Upload{}, nil, fmt.Errorf("preparing repo %s: %w", repo, err)
 	}
+	status.OKf("Prepared %s", source.RepoName(repo))
 
-	// Clean untracked files from previous runs
-	cmd = exec.Command("git", "-C", dir, "clean", "-fdx")
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
-	cmd.Run()
-
-	return nil
-}
-
-func initSubmodules(dir string) error {
-	cmd := exec.Command("git", "-C", dir, "submodule", "update", "--init", "--depth", "1")
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
-	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("git submodule update: %w", err)
+	cleanup := func() {
+		if cerr := prepared.Cleanup(); cerr != nil {
+			status.Warnf("cleaning up repo checkout: %v", cerr)
+		}
 	}
-	return nil
+	return gateway.Upload{Src: prepared.Dir, Dst: "/sandbox"}, cleanup, nil
 }
 
 // reconcileGateway drives the gateway's providers and inference route to match

internal/source/cache.go

@@ -0,0 +1,115 @@
+// Package source manages the on-disk cache of git repositories cloned outside
+// the sandbox for upload.
+//
+// It replaces the old basename-keyed cache (~/.cache/harness-openshell/repos/
+// <repo-name>/) — which collided when two repos shared a basename and raced when
+// two runs shared one repo — with URL-hashed bare mirrors plus per-run,
+// self-contained checkouts:
+//
+//	~/.cache/harness-openshell/
+//	  mirrors/<sha256(canonical-url)>.git   bare, shallow, updated in place, shared
+//	  checkouts/<run-id>/<repo-name>/       real repo (own .git), per run, removed after run
+//
+// The mirror is the only shared state; every write to it is serialized under a
+// per-mirror file lock. Checkouts are per-run, never shared, and hold their own
+// objects (no alternates into the mirror) so git keeps working after only the
+// checkout dir is uploaded into the sandbox.
+package source
+
+import (
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"net/url"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+)
+
+// Cache locates the on-disk cache roots. The zero value is unusable; construct
+// with DefaultCache or NewCache.
+type Cache struct {
+	root string // ~/.cache/harness-openshell
+}
+
+// DefaultCache resolves the cache under the user's home directory.
+func DefaultCache() (*Cache, error) {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return nil, fmt.Errorf("determining home dir: %w", err)
+	}
+	return NewCache(filepath.Join(home, ".cache", "harness-openshell")), nil
+}
+
+// NewCache builds a cache rooted at the given directory (used by tests).
+func NewCache(root string) *Cache { return &Cache{root: root} }
+
+// CanonicalizeURL normalizes a repo URL into a stable key for "same repo".
+// It trims surrounding space, removes URL userinfo, strips a trailing slash and
+// a ".git" suffix, and lowercases the scheme and host only — repository paths
+// stay case-sensitive because many hosts treat them so. Non-URL inputs (e.g.
+// scp-style git@host:org/repo) are returned trimmed of the same suffixes without
+// further change, which is still stable per distinct spelling.
+func CanonicalizeURL(raw string) string {
+	s := stripURLUserinfo(strings.TrimSpace(raw))
+	if u, err := url.Parse(s); err == nil && u.Host != "" {
+		u.Scheme = strings.ToLower(u.Scheme)
+		u.Host = strings.ToLower(u.Host)
+		s = u.String()
+	}
+	s = strings.TrimRight(s, "/")
+	s = strings.TrimSuffix(s, ".git")
+	return s
+}
+
+// stripURLUserinfo removes embedded credentials from a URL before it is used
+// as a cache identity or persisted in git configuration. Authentication is
+// resolved by git's configured credential helper instead.
+func stripURLUserinfo(raw string) string {
+	u, err := url.Parse(raw)
+	if err != nil || u.Host == "" {
+		return raw
+	}
+	u.User = nil
+	return u.String()
+}
+
+// RepoName derives the directory basename a repo is uploaded under
+// (/sandbox/<repo-name>), matching the old cache's behavior.
+func RepoName(repoURL string) string {
+	return strings.TrimSuffix(path.Base(strings.TrimRight(strings.TrimSpace(repoURL), "/")), ".git")
+}
+
+// MirrorPath is the bare-mirror directory for a repo URL, keyed by the sha256 of
+// its canonical form so distinct repos with the same basename never collide.
+func (c *Cache) MirrorPath(repoURL string) string {
+	sum := sha256.Sum256([]byte(CanonicalizeURL(repoURL)))
+	return filepath.Join(c.root, "mirrors", hex.EncodeToString(sum[:])+".git")
+}
+
+// runDir is the per-run checkout parent (checkouts/<run-id>), removed wholesale
+// on cleanup.
+func (c *Cache) runDir(runID string) string {
+	return filepath.Join(c.root, "checkouts", runID)
+}
+
+// checkoutPath nests the checkout as checkouts/<run-id>/<repo-name> so its
+// basename stays <repo-name>: `openshell --upload` copies the source dir by
+// name, so this is what makes the tree land at /sandbox/<repo-name> rather than
+// /sandbox/<run-id>.
+func (c *Cache) checkoutPath(runID, repoName string) string {
+	return filepath.Join(c.runDir(runID), repoName)
+}
+
+// NewRunID returns a random hex id identifying one run's checkout. 128 bits so
+// concurrent runs never collide on a checkout path (a collision would let one
+// run's cleanup delete another's tree).
+func NewRunID() (string, error) {
+	var b [16]byte
+	if _, err := rand.Read(b[:]); err != nil {
+		return "", fmt.Errorf("generating run id: %w", err)
+	}
+	return hex.EncodeToString(b[:]), nil
+}