fix: Addressing PR feedback

Author: yhakbar

docs/src/content/docs/03-features/07-caching/04-cas.mdx

@@ -110,7 +110,15 @@ In the event that hard linking fails due to some operating system / host incompa
 
 ## Storage
 
-The CAS is stored under the platform user cache directory. On Linux this is `~/.cache/terragrunt/cas` by default and honors `XDG_CACHE_HOME`, on macOS it resolves to `~/Library/Caches/terragrunt/cas`, and on Windows it resolves under `%LocalAppData%\terragrunt\cas`. This directory can be deleted to reclaim disk space when no Terragrunt processes are running against it; Terragrunt will regenerate the CAS on the next run. Avoid deleting it while a Terragrunt operation is in progress, since that can race with in-flight reads, writes, and locks in the store.
+The CAS lives under the platform user cache directory:
+
+| Platform | Path |
+| --- | --- |
+| Linux | `$XDG_CACHE_HOME/terragrunt/cas`, falling back to `~/.cache/terragrunt/cas` |
+| macOS | `~/Library/Caches/terragrunt/cas` |
+| Windows | `%LocalAppData%\terragrunt\cas` |
+
+This directory can be deleted to reclaim disk space when no Terragrunt processes are running against it. Terragrunt will regenerate the CAS on the next run. Avoid deleting it while a Terragrunt operation is in progress, since that can race with in-flight reads, writes, and locks in the store.
 
 Avoid partial deletions of the CAS directory without care, as that might result in partially cloned repositories and unexpected behavior.
 

internal/cas/errors.go

@@ -68,6 +68,7 @@ var (
 	ErrNoWorkDir           = errors.New("working directory not set")
 	ErrGitStorePath        = errors.New("failed to prepare git store path")
 	ErrGitStoreLock        = errors.New("failed to acquire git store lock")
+	ErrGitStoreFSNotOS     = errors.New("git store requires an OS-backed filesystem")
 	ErrFallbackCloneDir    = errors.New("failed to create fallback clone directory")
 )
 

internal/cas/gitstore.go

@@ -41,7 +41,15 @@ type GitStore struct {
 // NewGitStore returns a GitStore rooted at rootPath, creating the directory
 // on fs if needed. The filesystem is not retained; callers pass one explicitly
 // to EnsureRef.
+//
+// The git store shells out to `git`, which only sees the real disk. Callers
+// must pass an OS-backed [vfs.FS] from [vfs.NewOSFS]; an in-memory backing
+// returns [ErrGitStoreFSNotOS].
 func NewGitStore(fs vfs.FS, runner *git.GitRunner, rootPath string) (*GitStore, error) {
+	if !vfs.IsOSFS(fs) {
+		return nil, ErrGitStoreFSNotOS
+	}
+
 	if err := fs.MkdirAll(rootPath, DefaultDirPerms); err != nil {
 		return nil, fmt.Errorf("create git store at %s: %w", rootPath, errors.Join(ErrGitStorePath, err))
 	}
@@ -69,6 +77,10 @@ func (s *GitStore) EnsureRef(
 	url, ref, hash string,
 	depth int,
 ) (string, vfs.Unlocker, error) {
+	if !vfs.IsOSFS(fs) {
+		return "", nil, ErrGitStoreFSNotOS
+	}
+
 	dir, repoPath, lockPath := s.repoPaths(url)
 
 	if err := fs.MkdirAll(dir, DefaultDirPerms); err != nil {
@@ -101,8 +113,15 @@ func (s *GitStore) EnsureRef(
 
 	runner := s.runner.WithWorkDir(repoPath)
 
-	if err := runner.InitBare(ctx); err != nil {
-		return "", nil, err
+	initialized, err := bareRepoInitialized(fs, repoPath)
+	if err != nil {
+		return "", nil, fmt.Errorf("inspect bare repo %s: %w", repoPath, errors.Join(ErrGitStorePath, err))
+	}
+
+	if !initialized {
+		if err := runner.InitBare(ctx); err != nil {
+			return "", nil, err
+		}
 	}
 
 	has, err := runner.HasObject(ctx, hash)
@@ -156,3 +175,11 @@ func (s *GitStore) repoPaths(url string) (dir, repo, lockPath string) {
 
 	return dir, repo, lockPath
 }
+
+// bareRepoInitialized reports whether repoPath already holds a bare git
+// repository. Checking for HEAD is enough: `git init --bare` writes it as
+// part of repository setup, so its presence lets EnsureRef skip the
+// per-call init spawn once a store entry exists.
+func bareRepoInitialized(fs vfs.FS, repoPath string) (bool, error) {
+	return vfs.FileExists(fs, filepath.Join(repoPath, "HEAD"))
+}

internal/cas/gitstore_test.go

@@ -186,6 +186,27 @@ func TestGitStoreEnsureRef_FetchFailureSurfacesError(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestGitStoreRejectsNonOSFilesystem(t *testing.T) {
+	t.Parallel()
+
+	runner, err := git.NewGitRunner(vexec.NewOSExec())
+	require.NoError(t, err)
+
+	root := filepath.Join(helpers.TmpDirWOSymlinks(t), "gitstore")
+
+	_, err = cas.NewGitStore(vfs.NewMemMapFS(), runner, root)
+	require.ErrorIs(t, err, cas.ErrGitStoreFSNotOS)
+
+	store, err := cas.NewGitStore(vfs.NewOSFS(), runner, root)
+	require.NoError(t, err)
+
+	_, _, err = store.EnsureRef(
+		t.Context(), logger.CreateLogger(), vfs.NewMemMapFS(),
+		"file:///does/not/exist", "main", "deadbeef", 0,
+	)
+	require.ErrorIs(t, err, cas.ErrGitStoreFSNotOS)
+}
+
 func newTestGitStore(t *testing.T) (*cas.GitStore, vfs.FS, string) {
 	t.Helper()
 

internal/git/git.go

@@ -36,6 +36,11 @@ import (
 
 const (
 	minGitPartsLength = 2
+
+	// catFileMissingExitCode is the exit code `git cat-file -e` returns when
+	// the requested object is absent. Any other non-zero exit is an
+	// execution failure (e.g. 128 from a fatal error).
+	catFileMissingExitCode = 1
 )
 
 // GitRunner handles git command execution
@@ -362,8 +367,10 @@ func (g *GitRunner) Fetch(ctx context.Context, repo, ref string, depth int) erro
 }
 
 // HasObject reports whether the given object exists in the configured
-// working-directory repository. A non-zero exit from `git cat-file -e`
-// is treated as a missing object, not an execution failure.
+// working-directory repository. Exit code 1 from `git cat-file -e` means
+// the object is absent. Other non-zero exits (e.g. 128 for a corrupted
+// repo or unreadable .git) are returned as errors so callers do not loop
+// into a refetch against a broken store.
 func (g *GitRunner) HasObject(ctx context.Context, hash string) (bool, error) {
 	if err := g.RequiresWorkDir(); err != nil {
 		return false, err
@@ -376,7 +383,7 @@ func (g *GitRunner) HasObject(ctx context.Context, hash string) (bool, error) {
 	cmd.SetStderr(&stderr)
 
 	if err := cmd.Run(); err != nil {
-		if vexec.ExitCode(err) > 0 {
+		if vexec.ExitCode(err) == catFileMissingExitCode {
 			return false, nil
 		}
 

internal/git/git_test.go

@@ -315,3 +315,21 @@ func TestGitRunner_FetchAndHasObject(t *testing.T) {
 	require.NoError(t, err)
 	assert.True(t, has)
 }
+
+func TestGitRunner_HasObjectSurfacesNonMissingFailures(t *testing.T) {
+	t.Parallel()
+
+	dir := helpers.TmpDirWOSymlinks(t)
+
+	runner, err := git.NewGitRunner(vexec.NewOSExec())
+	require.NoError(t, err)
+
+	runner = runner.WithWorkDir(dir)
+	require.NoError(t, runner.InitBare(t.Context()))
+
+	// A malformed object name returns exit 128 (fatal). HasObject must
+	// return an error rather than report missing, so a corrupted store
+	// does not trigger a refetch loop.
+	_, err = runner.HasObject(t.Context(), "not-a-hash")
+	require.Error(t, err)
+}

internal/vfs/vfs.go

@@ -80,6 +80,15 @@ func NewOSFS() FS {
 	return &osFS{afero.NewOsFs()}
 }
 
+// IsOSFS reports whether fs is the OS-backed filesystem from [NewOSFS].
+// Callers that shell out to processes which only see the real disk (e.g.
+// `git`) should reject other filesystems up front rather than failing
+// inside the subprocess.
+func IsOSFS(fs FS) bool {
+	_, ok := fs.(*osFS)
+	return ok
+}
+
 // NewMemMapFS returns an in-memory filesystem for testing purposes.
 // The returned filesystem supports symlink operations via an in-memory link table.
 func NewMemMapFS() FS {