e2e0bc0 · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

fix(config): scope .crush discovery to the current repo

e2e0bc0Christian Rocha5 months ago

internal/config/load.go+43−1

compare with main

unifiedsplit

@@ -424,7 +424,7 @@ func (c *Config) setDefaults(workingDir, dataDir string) {

424 if dataDir != "" {

425 c.Options.DataDirectory = dataDir

426 } else if c.Options.DataDirectory == "" {

427− if path, ok := fsext.LookupClosest(workingDir, defaultDataDirectory); ok {

427+ if path, ok := fsext.LookupClosestBounded(workingDir, projectBoundary(workingDir), defaultDataDirectory); ok {

428 c.Options.DataDirectory = path

429 } else {

430 c.Options.DataDirectory = filepath.Join(workingDir, defaultDataDirectory)

@@ -873,6 +873,48 @@ func isInsideWorktree() bool {

873 return err == nil && strings.TrimSpace(string(bts)) == "true"

874}

875

876+// worktreeRoot returns the absolute path of the git working tree root for

877+// dir, or the empty string if dir is not inside a working tree (bare

878+// repositories, missing git binary, plain directories, or any other

879+// failure mode). Linked worktrees and submodules each report their own

880+// top-level, which is what callers want when bounding lookups.

881+func worktreeRoot(dir string) string {

882+ cmd := exec.CommandContext(

883+ context.Background(),

884+ "git", "rev-parse", "--show-toplevel",

885+ )

886+ cmd.Dir = dir

887+ out, err := cmd.Output()

888+ if err != nil {

889+ return ""

890+ }

891+ root := strings.TrimSpace(string(out))

892+ if root == "" {

893+ return ""

894+ }

895+ abs, err := filepath.Abs(root)

896+ if err != nil {

897+ return ""

898+ }

899+ return abs

900+}

901+

902+// projectBoundary returns the directory at which an upward configuration

903+// search rooted at dir should stop. It is the git working tree root when

904+// one can be detected, otherwise dir itself. Returning dir as a

905+// fallback keeps Crush from silently adopting state files placed above

906+// the current project.

907+func projectBoundary(dir string) string {

908+ if root := worktreeRoot(dir); root != "" {

909+ return root

910+ }

911+ abs, err := filepath.Abs(dir)

912+ if err != nil {

913+ return dir

914+ }

915+ return abs

916+}

917+

876// GlobalSkillsDirs returns the default directories for Agent Skills.

918// GlobalSkillsDirs returns the default directories for Agent Skills.

877// Skills in these directories are auto-discovered and their files can be read

919// Skills in these directories are auto-discovered and their files can be read

878// without permission prompts.

920// without permission prompts.

internal/config/load_test.go+63

@@ -4,6 +4,7 @@ import (

4 "io"

5 "log/slog"

6 "os"

7+ "os/exec"

7 "path/filepath"

8 "path/filepath"

8 "testing"

9 "testing"

@@ -111,6 +112,68 @@ func TestConfig_setDefaults(t *testing.T) {

111

112

112 require.Equal(t, filepath.Join(workingDir, "state"), cfg.Options.DataDirectory)

113 require.Equal(t, filepath.Join(workingDir, "state"), cfg.Options.DataDirectory)

113 })

114 })

115+

116+ t.Run("does not adopt .crush from a parent project", func(t *testing.T) {

117+ parent := t.TempDir()

118+

119+ // .crush in the parent: it should not be reused by the child

120+ // because there is no git context joining them.

121+ require.NoError(t, os.Mkdir(filepath.Join(parent, defaultDataDirectory), 0o755))

122+

123+ child := filepath.Join(parent, "child")

124+ require.NoError(t, os.Mkdir(child, 0o755))

125+

126+ cfg := &Config{}

127+ cfg.setDefaults(child, "")

128+

129+ require.Equal(t,

130+ filepath.Clean(filepath.Join(child, defaultDataDirectory)),

131+ filepath.Clean(cfg.Options.DataDirectory),

132+ )

133+ })

134+

135+ t.Run("does not climb out of git worktree to find .crush", func(t *testing.T) {

136+ if _, err := exec.LookPath("git"); err != nil {

137+ t.Skip("git not available")

138+ }

139+

140+ parent := t.TempDir()

141+

142+ // Stray .crush above the worktree root.

143+ require.NoError(t, os.Mkdir(filepath.Join(parent, defaultDataDirectory), 0o755))

144+

145+ worktree := filepath.Join(parent, "worktree")

146+ require.NoError(t, os.Mkdir(worktree, 0o755))

147+

148+ sub := filepath.Join(worktree, "pkg")

149+ require.NoError(t, os.Mkdir(sub, 0o755))

150+

151+ // Make worktree a real git repo so the boundary detection

152+ // resolves to it, mirroring what happens with linked worktrees

153+ // in real usage.

154+ gitInit := exec.CommandContext(t.Context(), "git", "init", "-q")

155+ gitInit.Dir = worktree

156+ require.NoError(t, gitInit.Run())

157+

158+ cfg := &Config{}

159+ cfg.setDefaults(sub, "")

160+

161+ // Resolve symlinks because TempDir on macOS sits under /var

162+ // which is a symlink to /private/var. The data directory has

163+ // not been created yet, so resolve its parent and join.

164+ gotDir, gotName := filepath.Split(cfg.Options.DataDirectory)

165+ gotEvalDir, err := filepath.EvalSymlinks(filepath.Clean(gotDir))

166+ require.NoError(t, err)

167+ gotEval := filepath.Join(gotEvalDir, gotName)

168+

169+ strayEval, err := filepath.EvalSymlinks(filepath.Join(parent, defaultDataDirectory))

170+ require.NoError(t, err)

171+ require.NotEqual(t, strayEval, gotEval, "must not adopt parent .crush")

172+

173+ subEval, err := filepath.EvalSymlinks(sub)

174+ require.NoError(t, err)

175+ require.Equal(t, filepath.Join(subEval, defaultDataDirectory), gotEval)

176+ })

114}

177}

115

178

116func TestConfig_configureProviders(t *testing.T) {

179func TestConfig_configureProviders(t *testing.T) {

internal/fsext/lookup.go+17−1

@@ -197,6 +197,11 @@ func traverseUp(dir string, walkFn func(dir string, owner int) error) error {

197// traverseUp instead. If stopDir is set but is not an ancestor of dir

198// the walk still stops at the filesystem root, so callers cannot

199// accidentally produce an infinite walk by passing a sibling path.

200+//

201+// Boundary comparison is performed against symlink-resolved paths so

202+// that callers passing logically equivalent paths (a symlinked /var vs

203+// the underlying /private/var, for example) still terminate at the

204+// expected directory.

200func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) error) error {

205func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) error) error {

201 cwd, err := filepath.Abs(dir)

206 cwd, err := filepath.Abs(dir)

202 if err != nil {

207 if err != nil {

@@ -210,6 +215,7 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e

210 return fmt.Errorf("cannot convert stop dir to absolute path: %w", err)

215 return fmt.Errorf("cannot convert stop dir to absolute path: %w", err)

211 }

216 }

212 }

217 }

218+ canonStop := canonicalize(stop)

213

219

214 owner, err := Owner(dir)

220 owner, err := Owner(dir)

215 if err != nil {

221 if err != nil {

@@ -219,7 +225,7 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e

219 for {

225 for {

220 err := walkFn(cwd, owner)

226 err := walkFn(cwd, owner)

221 if err == nil || errors.Is(err, filepath.SkipDir) {

227 if err == nil || errors.Is(err, filepath.SkipDir) {

222− if cwd == stop {

228+ if canonicalize(cwd) == canonStop {

223 return nil

229 return nil

224 }

230 }

225

231

@@ -240,6 +246,16 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e

240 }

246 }

241}

247}

242

248

249+// canonicalize resolves any symbolic links in path. If resolution fails

250+// (typically because path does not exist yet) the original path is

251+// returned cleaned, so callers can still perform stable equality checks.

252+func canonicalize(path string) string {

253+ if resolved, err := filepath.EvalSymlinks(path); err == nil {

254+ return resolved

255+ }

256+ return filepath.Clean(path)

257+}

258+

243// probeEnt checks if entity at given path exists and belongs to given owner

259// probeEnt checks if entity at given path exists and belongs to given owner

244func probeEnt(fspath string, owner int) error {

260func probeEnt(fspath string, owner int) error {

245 _, err := os.Stat(fspath)

261 _, err := os.Stat(fspath)