Merge pull request #1764 from entireio/protected-dirs

fix(checkpoint): exclude protected dirs from first-checkpoint snapshot

Author: pjbgf

cmd/entire/cli/checkpoint/checkpoint_test.go

@@ -14,6 +14,8 @@ import (
 	"time"
 
 	"github.com/entireio/cli/cmd/entire/cli/agent"
+	_ "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" // register claude-code so its .claude protected dir is discoverable
+	"github.com/entireio/cli/cmd/entire/cli/agent/types"
 	"github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
 	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/testutil"
@@ -103,6 +105,99 @@ func TestCopyMetadataDir_SkipsSymlinks(t *testing.T) {
 	}
 }
 
+// fakePluginAgent is a minimal agent stub used to prove that protected dirs
+// and files reported by an external-plugin-style agent (via the AllProtectedDirs
+// / AllProtectedFiles union) are honored by the first-checkpoint path, not just
+// the built-in claude-code .claude dir.
+type fakePluginAgent struct{}
+
+var (
+	_ agent.Agent                  = (*fakePluginAgent)(nil)
+	_ agent.ProtectedFilesProvider = (*fakePluginAgent)(nil)
+)
+
+func (fakePluginAgent) Name() types.AgentName                { return "terminalhire-plugin" }
+func (fakePluginAgent) Type() types.AgentType                { return "TerminalHire" }
+func (fakePluginAgent) Description() string                  { return "fake external plugin for tests" }
+func (fakePluginAgent) IsPreview() bool                      { return true }
+func (fakePluginAgent) ProtectedDirs() []string              { return []string{".terminalhire"} }
+func (fakePluginAgent) ProtectedFiles() []string             { return []string{".terminalhirerc"} }
+func (fakePluginAgent) GetSessionID(*agent.HookInput) string { return "" }
+
+func (fakePluginAgent) DetectPresence(context.Context) (bool, error) { return false, nil }
+func (fakePluginAgent) ReadTranscript(string) ([]byte, error)        { return nil, nil }
+func (fakePluginAgent) ChunkTranscript(_ context.Context, c []byte, _ int) ([][]byte, error) {
+	return [][]byte{c}, nil
+}
+func (fakePluginAgent) ReassembleTranscript(chunks [][]byte) ([]byte, error) {
+	var out []byte
+	for _, c := range chunks {
+		out = append(out, c...)
+	}
+	return out, nil
+}
+func (fakePluginAgent) GetSessionDir(string) (string, error)                      { return "", nil }
+func (fakePluginAgent) ResolveSessionFile(dir, sid string) string                 { return dir + "/" + sid }
+func (fakePluginAgent) ReadSession(*agent.HookInput) (*agent.AgentSession, error) { return nil, nil } //nolint:nilnil // test stub
+func (fakePluginAgent) WriteSession(context.Context, *agent.AgentSession) error   { return nil }
+func (fakePluginAgent) FormatResumeCommand(string) string                         { return "" }
+
+// TestCollectChangedFiles_ExcludesProtectedDirs verifies that the
+// first-checkpoint path keeps agent-protected dirs (e.g. .claude) and the
+// .entire infrastructure dir out of the checkpoint snapshot, while ordinary
+// untracked files are still captured. Regression for protected-dir content
+// leaking into the shadow tree on session start.
+func TestCollectChangedFiles_ExcludesProtectedDirs(t *testing.T) {
+	t.Parallel()
+
+	// Register an external-plugin-style agent so its protected dir/file join the
+	// AllProtectedDirs/AllProtectedFiles union alongside the built-in .claude.
+	// Registration is additive and concurrency-safe; no test asserts the exact set.
+	agent.Register("terminalhire-plugin", func() agent.Agent { return fakePluginAgent{} })
+
+	tempDir := t.TempDir()
+	// Resolve symlinks so the repo root matches git's resolved path.
+	// On macOS, t.TempDir() returns /var/... but git resolves to /private/var/...
+	tempDir, err := filepath.EvalSymlinks(tempDir)
+	require.NoError(t, err)
+
+	testutil.InitRepo(t, tempDir)
+	testutil.WriteFile(t, tempDir, "base.txt", "base")
+	testutil.GitAdd(t, tempDir, "base.txt")
+	testutil.GitCommit(t, tempDir, "init")
+
+	// Disable any global core.excludesFile so a developer/CI-runner gitignore
+	// convention (e.g. one that ignores .claude) can't mask the leak. The fix
+	// must exclude protected dirs on its own, independent of gitignore state.
+	cfgCmd := exec.CommandContext(context.Background(), "git", "config", "core.excludesFile", os.DevNull)
+	cfgCmd.Dir = tempDir
+	require.NoError(t, cfgCmd.Run())
+
+	// Planted untracked, non-gitignored files.
+	testutil.WriteFile(t, tempDir, ".claude/marker.txt", "MARKER-secret")         // built-in agent-protected dir
+	testutil.WriteFile(t, tempDir, ".terminalhire/profile.json", "MARKER-plugin") // plugin-protected dir
+	testutil.WriteFile(t, tempDir, ".terminalhirerc", "MARKER-plugin-file")       // plugin-protected file
+	testutil.WriteFile(t, tempDir, ".entire/state.json", "{}")                    // infrastructure
+	testutil.WriteFile(t, tempDir, "src/keep.txt", "user work")                   // ordinary
+
+	repo, err := git.PlainOpen(tempDir)
+	require.NoError(t, err)
+
+	result, err := collectChangedFiles(context.Background(), repo)
+	require.NoError(t, err)
+
+	require.NotContains(t, result.Changed, ".claude/marker.txt",
+		"built-in agent protected dir content must not be captured into the checkpoint")
+	require.NotContains(t, result.Changed, ".terminalhire/profile.json",
+		"external-plugin protected dir content must not be captured into the checkpoint")
+	require.NotContains(t, result.Changed, ".terminalhirerc",
+		"external-plugin protected file must not be captured into the checkpoint")
+	require.NotContains(t, result.Changed, ".entire/state.json",
+		"infrastructure dir must not be captured into the checkpoint")
+	require.Contains(t, result.Changed, "src/keep.txt",
+		"ordinary untracked files must still be captured")
+}
+
 // TestWriteCommitted_AgentField verifies that the Agent field is written
 // to both metadata.json and the commit message trailer.
 func TestWriteCommitted_AgentField(t *testing.T) {

cmd/entire/cli/checkpoint/ephemeral.go

@@ -1195,6 +1195,35 @@ func filterGitIgnoredFiles(ctx context.Context, repo *git.Repository, files []st
 	return kept
 }
 
+// isProtectedCheckpointPath reports whether a repo-relative path must be kept
+// out of checkpoint snapshots: the .entire infrastructure dir, or any
+// registered agent's declared protected dir/file (e.g. .claude, or an external
+// plugin's protected_dirs).
+//
+// This mirrors shouldIgnoreSessionTrackingPath in the cli package. The two
+// cannot share an implementation because cli imports checkpoint, so the logic
+// is duplicated deliberately. The first-checkpoint path (collectChangedFiles)
+// must apply the same exclusions as the session-tracking and rewind paths, or
+// protected-dir content is captured into the shadow tree on session start
+// (see the DetectFileChanges / isProtectedPath call sites).
+func isProtectedCheckpointPath(relPath string) bool {
+	cleanPath := filepath.Clean(filepath.FromSlash(relPath))
+	if paths.IsInfrastructurePath(cleanPath) {
+		return true
+	}
+	for _, file := range agent.AllProtectedFiles() {
+		if paths.Equal(cleanPath, file) {
+			return true
+		}
+	}
+	for _, dir := range agent.AllProtectedDirs() {
+		if paths.IsProtectedSubpath(filepath.Clean(filepath.FromSlash(dir)), cleanPath) {
+			return true
+		}
+	}
+	return false
+}
+
 // collectChangedFiles returns all changed files from git status for the first checkpoint.
 //
 // For the first checkpoint, we need to capture:
@@ -1246,16 +1275,16 @@ func collectChangedFiles(ctx context.Context, repo *git.Repository) (changedFile
 		filename := entry[3:] // No TrimSpace needed with -z format
 
 		// Handle R/C (rename/copy) first - they have a second entry we must skip
-		// even if the new filename is an infrastructure path
+		// even if the new filename is a protected path
 		if staging == 'R' || staging == 'C' {
 			// Renamed or copied: current entry is new name, next entry is old name
-			if !paths.IsInfrastructurePath(filename) {
+			if !isProtectedCheckpointPath(filename) {
 				changedSeen[filename] = struct{}{}
 			}
 			// The old name follows as the next NUL-separated entry - must always skip it
 			if i+1 < len(entries) && entries[i+1] != "" {
 				oldName := entries[i+1]
-				if staging == 'R' && !paths.IsInfrastructurePath(oldName) {
+				if staging == 'R' && !isProtectedCheckpointPath(oldName) {
 					// For renames, old file is effectively deleted
 					deletedSeen[oldName] = struct{}{}
 				}
@@ -1264,8 +1293,8 @@ func collectChangedFiles(ctx context.Context, repo *git.Repository) (changedFile
 			continue
 		}
 
-		// Skip .entire directory for non-R/C entries
-		if paths.IsInfrastructurePath(filename) {
+		// Skip .entire and agent-protected dirs/files for non-R/C entries
+		if isProtectedCheckpointPath(filename) {
 			continue
 		}
 

cmd/entire/cli/paths/paths.go

@@ -21,6 +21,7 @@ const (
 	EntireMetadataDir = ".entire/metadata"
 
 	osWindows = "windows"
+	osDarwin  = "darwin"
 )
 
 // Metadata file names
@@ -133,15 +134,25 @@ func AbsPath(ctx context.Context, relPath string) (string, error) {
 }
 
 // IsInfrastructurePath returns true if the path is part of CLI infrastructure
-// (i.e., inside the .entire directory)
+// (i.e., inside the .entire directory). It is used only to EXCLUDE infra paths
+// from checkpoints/tracking, so it matches case-insensitively on
+// case-insensitive filesystems via IsProtectedSubpath. Do not use it as a
+// containment/allow gate.
 func IsInfrastructurePath(path string) bool {
-	return IsSubpath(EntireDir, path)
+	return IsProtectedSubpath(EntireDir, path)
 }
 
 // IsSubpath reports whether child is lexically under parent (or equal to it).
 // It uses filepath.Rel, which cleans both inputs and is traversal-resistant:
 // a crafted child like "/a/b/../../../etc/passwd" that escapes parent will
 // produce a relative path starting with ".." and be rejected.
+//
+// Matching is case-SENSITIVE. This is the correct primitive for fail-closed
+// containment/allow checks (e.g. validating an attacker-influenced path stays
+// under an Entire-owned dir): on a case-sensitive volume a differently-cased
+// path names a different directory, so folding it in would fail open. For
+// EXCLUSION decisions that must also catch case variants on Windows/macOS, use
+// IsProtectedSubpath instead.
 func IsSubpath(parent, child string) bool {
 	rel, err := filepath.Rel(parent, child)
 	if err != nil {
@@ -150,6 +161,47 @@ func IsSubpath(parent, child string) bool {
 	return !IsRelativeTraversal(rel)
 }
 
+// IsProtectedSubpath reports whether child is under parent for the purpose of
+// EXCLUDING protected/infrastructure content from checkpoints and tracking.
+// Unlike IsSubpath it honors OS case-insensitivity (see CaseInsensitiveFS), so
+// a case variant of a protected dir (".Claude" vs ".claude") is still excluded
+// on Windows/macOS.
+//
+// SECURITY: never use this for allow/containment decisions. Case-folding widens
+// what counts as "inside" parent, which is safe only when the effect is to
+// exclude more. On a case-sensitive volume under a case-insensitive GOOS it
+// over-matches; for a fail-closed gate that would fail open. Use IsSubpath there.
+func IsProtectedSubpath(parent, child string) bool {
+	if CaseInsensitiveFS() {
+		return IsSubpath(strings.ToLower(parent), strings.ToLower(child))
+	}
+	return IsSubpath(parent, child)
+}
+
+// CaseInsensitiveFS reports whether path comparisons should be case-insensitive
+// on the host OS. This is OS-based, not volume-based: Windows and macOS default
+// to case-insensitive filesystems, Linux to case-sensitive. Keying on GOOS keeps
+// the result deterministic. It must only influence EXCLUSION decisions (see
+// IsProtectedSubpath / Equal): on an atypical volume (e.g. a case-sensitive
+// macOS APFS volume) it treats a differently-cased path as matching, which is
+// safe only when the effect is to exclude more, never to widen an allow gate.
+func CaseInsensitiveFS() bool {
+	return runtime.GOOS == osWindows || runtime.GOOS == osDarwin
+}
+
+// Equal reports whether two paths refer to the same location, honoring the host
+// OS's case sensitivity (see CaseInsensitiveFS). Both inputs are cleaned and
+// slash-normalized before comparison. Like IsProtectedSubpath, this is intended
+// for EXCLUSION matching (e.g. protected files), not fail-closed containment.
+func Equal(a, b string) bool {
+	a = filepath.Clean(filepath.FromSlash(a))
+	b = filepath.Clean(filepath.FromSlash(b))
+	if CaseInsensitiveFS() {
+		return strings.EqualFold(a, b)
+	}
+	return a == b
+}
+
 // IsRelativeTraversal reports whether rel escapes its base directory.
 // It accepts both OS-native paths and Git-style slash-normalized paths.
 func IsRelativeTraversal(rel string) bool {

cmd/entire/cli/paths/paths_test.go

@@ -96,6 +96,65 @@ func TestIsInfrastructurePath(t *testing.T) {
 	}
 }
 
+func TestCaseInsensitiveFS(t *testing.T) {
+	t.Parallel()
+	want := runtime.GOOS == osWindows || runtime.GOOS == osDarwin
+	if got := CaseInsensitiveFS(); got != want {
+		t.Errorf("CaseInsensitiveFS() = %v, want %v (GOOS=%s)", got, want, runtime.GOOS)
+	}
+}
+
+// TestIsSubpath_AlwaysCaseSensitive locks in that IsSubpath — the fail-closed
+// containment primitive used by allow gates (rewind/utils) — never folds case
+// on any OS. A differently-cased path must not count as contained, or a
+// crafted, attacker-influenced value could fail open on a case-sensitive volume.
+func TestIsSubpath_AlwaysCaseSensitive(t *testing.T) {
+	t.Parallel()
+	if IsSubpath(".entire/metadata", ".Entire/metadata") {
+		t.Error("IsSubpath must be case-sensitive (fail-closed); .Entire/metadata must not be under .entire/metadata")
+	}
+	if !IsSubpath(".claude", ".claude/marker.txt") {
+		t.Error("IsSubpath(.claude, .claude/marker.txt) = false, want true")
+	}
+	if IsSubpath(".claude", ".claude/../../etc/passwd") {
+		t.Error("IsSubpath must reject traversal")
+	}
+}
+
+// TestIsProtectedSubpath_CaseSensitivity asserts OS-based folding for the
+// EXCLUSION helper: case variants match on Windows/macOS (where they name the
+// same on-disk path), stay distinct on case-sensitive Linux, and traversal is
+// always rejected.
+func TestIsProtectedSubpath_CaseSensitivity(t *testing.T) {
+	t.Parallel()
+	got := IsProtectedSubpath(".claude", ".Claude/marker.txt")
+	if got != CaseInsensitiveFS() {
+		t.Errorf("IsProtectedSubpath(.claude, .Claude/marker.txt) = %v, want %v (GOOS=%s)",
+			got, CaseInsensitiveFS(), runtime.GOOS)
+	}
+	if !IsProtectedSubpath(".claude", ".claude/marker.txt") {
+		t.Error("IsProtectedSubpath(.claude, .claude/marker.txt) = false, want true")
+	}
+	if IsProtectedSubpath(".claude", ".Claude/../../etc/passwd") {
+		t.Error("IsProtectedSubpath must reject traversal even when case-folding")
+	}
+}
+
+func TestEqual_CaseSensitivity(t *testing.T) {
+	t.Parallel()
+	if !Equal(".terminalhirerc", ".terminalhirerc") {
+		t.Error("Equal should match identical paths")
+	}
+	got := Equal(".terminalhirerc", ".TerminalHireRC")
+	if got != CaseInsensitiveFS() {
+		t.Errorf("Equal(case variant) = %v, want %v (GOOS=%s)",
+			got, CaseInsensitiveFS(), runtime.GOOS)
+	}
+	if Equal(".terminalhirerc", "other") {
+		t.Error("Equal should not match distinct paths")
+	}
+}
+
 func TestToRelativePath_MSYSPaths(t *testing.T) {
 	t.Parallel()
 	if runtime.GOOS != "windows" {

cmd/entire/cli/rewind_test.go

@@ -75,6 +75,15 @@ func TestLegacyFallbackTranscriptPath(t *testing.T) {
 			metadataDir: ".entire",
 			want:        "",
 		},
+		{
+			// Containment is a fail-closed allow gate: it must stay case-SENSITIVE
+			// on every OS. A case variant names a different on-disk dir on a
+			// case-sensitive volume (which exists under GOOS=darwin), so folding it
+			// in would fail open. Must return "" regardless of platform.
+			name:        "case-variant of metadata dir fails closed on all OSes",
+			metadataDir: ".Entire/metadata/sess-123",
+			want:        "",
+		},
 	}
 
 	for _, tt := range tests {

cmd/entire/cli/state.go

@@ -238,15 +238,14 @@ func shouldIgnoreSessionTrackingPath(relPath string) bool {
 	}
 
 	for _, file := range agent.AllProtectedFiles() {
-		cleanFile := filepath.Clean(filepath.FromSlash(file))
-		if cleanPath == cleanFile {
+		if paths.Equal(cleanPath, file) {
 			return true
 		}
 	}
 
 	for _, dir := range agent.AllProtectedDirs() {
 		cleanDir := filepath.Clean(filepath.FromSlash(dir))
-		if paths.IsSubpath(cleanDir, cleanPath) {
+		if paths.IsProtectedSubpath(cleanDir, cleanPath) {
 			return true
 		}
 	}

cmd/entire/cli/strategy/common.go

@@ -352,7 +352,7 @@ const (
 // registered agent config directories.
 func isProtectedPath(relPath string) bool {
 	for _, dir := range protectedDirs() {
-		if paths.IsSubpath(dir, relPath) {
+		if paths.IsProtectedSubpath(dir, relPath) {
 			return true
 		}
 	}