diff --git a/internal/config/load.go b/internal/config/load.go index bc75422a082d0a20d081a700ae3c126bd727b132..011a09f9ee90885776cd6d198fd6082cf2cbb2a4 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -424,7 +424,7 @@ func (c *Config) setDefaults(workingDir, dataDir string) { if dataDir != "" { c.Options.DataDirectory = dataDir } else if c.Options.DataDirectory == "" { - if path, ok := fsext.LookupClosest(workingDir, defaultDataDirectory); ok { + if path, ok := fsext.LookupClosestBounded(workingDir, projectBoundary(workingDir), defaultDataDirectory); ok { c.Options.DataDirectory = path } else { c.Options.DataDirectory = filepath.Join(workingDir, defaultDataDirectory) @@ -873,6 +873,48 @@ func isInsideWorktree() bool { return err == nil && strings.TrimSpace(string(bts)) == "true" } +// worktreeRoot returns the absolute path of the git working tree root for +// dir, or the empty string if dir is not inside a working tree (bare +// repositories, missing git binary, plain directories, or any other +// failure mode). Linked worktrees and submodules each report their own +// top-level, which is what callers want when bounding lookups. +func worktreeRoot(dir string) string { + cmd := exec.CommandContext( + context.Background(), + "git", "rev-parse", "--show-toplevel", + ) + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + return "" + } + root := strings.TrimSpace(string(out)) + if root == "" { + return "" + } + abs, err := filepath.Abs(root) + if err != nil { + return "" + } + return abs +} + +// projectBoundary returns the directory at which an upward configuration +// search rooted at dir should stop. It is the git working tree root when +// one can be detected, otherwise dir itself. Returning dir as a +// fallback keeps Crush from silently adopting state files placed above +// the current project. +func projectBoundary(dir string) string { + if root := worktreeRoot(dir); root != "" { + return root + } + abs, err := filepath.Abs(dir) + if err != nil { + return dir + } + return abs +} + // GlobalSkillsDirs returns the default directories for Agent Skills. // Skills in these directories are auto-discovered and their files can be read // without permission prompts. diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 0a50cce5ea38c8f73bfa60111d410dc15a60fd4e..50a532c0ce8a9a330f82b9b7fb54c2fb3dc8aa22 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -4,6 +4,7 @@ import ( "io" "log/slog" "os" + "os/exec" "path/filepath" "testing" @@ -111,6 +112,68 @@ func TestConfig_setDefaults(t *testing.T) { require.Equal(t, filepath.Join(workingDir, "state"), cfg.Options.DataDirectory) }) + + t.Run("does not adopt .crush from a parent project", func(t *testing.T) { + parent := t.TempDir() + + // .crush in the parent: it should not be reused by the child + // because there is no git context joining them. + require.NoError(t, os.Mkdir(filepath.Join(parent, defaultDataDirectory), 0o755)) + + child := filepath.Join(parent, "child") + require.NoError(t, os.Mkdir(child, 0o755)) + + cfg := &Config{} + cfg.setDefaults(child, "") + + require.Equal(t, + filepath.Clean(filepath.Join(child, defaultDataDirectory)), + filepath.Clean(cfg.Options.DataDirectory), + ) + }) + + t.Run("does not climb out of git worktree to find .crush", func(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + parent := t.TempDir() + + // Stray .crush above the worktree root. + require.NoError(t, os.Mkdir(filepath.Join(parent, defaultDataDirectory), 0o755)) + + worktree := filepath.Join(parent, "worktree") + require.NoError(t, os.Mkdir(worktree, 0o755)) + + sub := filepath.Join(worktree, "pkg") + require.NoError(t, os.Mkdir(sub, 0o755)) + + // Make worktree a real git repo so the boundary detection + // resolves to it, mirroring what happens with linked worktrees + // in real usage. + gitInit := exec.CommandContext(t.Context(), "git", "init", "-q") + gitInit.Dir = worktree + require.NoError(t, gitInit.Run()) + + cfg := &Config{} + cfg.setDefaults(sub, "") + + // Resolve symlinks because TempDir on macOS sits under /var + // which is a symlink to /private/var. The data directory has + // not been created yet, so resolve its parent and join. + gotDir, gotName := filepath.Split(cfg.Options.DataDirectory) + gotEvalDir, err := filepath.EvalSymlinks(filepath.Clean(gotDir)) + require.NoError(t, err) + gotEval := filepath.Join(gotEvalDir, gotName) + + strayEval, err := filepath.EvalSymlinks(filepath.Join(parent, defaultDataDirectory)) + require.NoError(t, err) + require.NotEqual(t, strayEval, gotEval, "must not adopt parent .crush") + + subEval, err := filepath.EvalSymlinks(sub) + require.NoError(t, err) + require.Equal(t, filepath.Join(subEval, defaultDataDirectory), gotEval) + }) } func TestConfig_configureProviders(t *testing.T) { diff --git a/internal/fsext/lookup.go b/internal/fsext/lookup.go index e9d43f791bb5259dc437f6749310e741d56271cc..696219ac89209b718ed0e106a4bc6a879105961c 100644 --- a/internal/fsext/lookup.go +++ b/internal/fsext/lookup.go @@ -197,6 +197,11 @@ func traverseUp(dir string, walkFn func(dir string, owner int) error) error { // traverseUp instead. If stopDir is set but is not an ancestor of dir // the walk still stops at the filesystem root, so callers cannot // accidentally produce an infinite walk by passing a sibling path. +// +// Boundary comparison is performed against symlink-resolved paths so +// that callers passing logically equivalent paths (a symlinked /var vs +// the underlying /private/var, for example) still terminate at the +// expected directory. func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) error) error { cwd, err := filepath.Abs(dir) if err != nil { @@ -210,6 +215,7 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e return fmt.Errorf("cannot convert stop dir to absolute path: %w", err) } } + canonStop := canonicalize(stop) owner, err := Owner(dir) if err != nil { @@ -219,7 +225,7 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e for { err := walkFn(cwd, owner) if err == nil || errors.Is(err, filepath.SkipDir) { - if cwd == stop { + if canonicalize(cwd) == canonStop { return nil } @@ -240,6 +246,16 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e } } +// canonicalize resolves any symbolic links in path. If resolution fails +// (typically because path does not exist yet) the original path is +// returned cleaned, so callers can still perform stable equality checks. +func canonicalize(path string) string { + if resolved, err := filepath.EvalSymlinks(path); err == nil { + return resolved + } + return filepath.Clean(path) +} + // probeEnt checks if entity at given path exists and belongs to given owner func probeEnt(fspath string, owner int) error { _, err := os.Stat(fspath)