Avoid looping forever on symlinks

Signed-off-by: David Gageot <david.gageot@docker.com>

Author: dgageot

pkg/skills/skills.go

@@ -210,7 +210,22 @@ func loadSkillsFlat(dir string) []Skill {
 }
 
 // loadSkillsRecursive loads skills from all subdirectories (Codex format).
+// It tracks visited real directory paths to avoid infinite loops caused by
+// symlinks that form cycles.
 func loadSkillsRecursive(dir string) []Skill {
+	visited := make(map[string]bool)
+
+	// Resolve the root so cycles back to it are detected.
+	if realDir, err := filepath.EvalSymlinks(dir); err == nil {
+		visited[realDir] = true
+	}
+
+	return walkSkillsRecursive(dir, visited)
+}
+
+// walkSkillsRecursive walks dir for SKILL.md files, using visited to skip
+// directories whose real path has already been traversed.
+func walkSkillsRecursive(dir string, visited map[string]bool) []Skill {
 	var skills []Skill
 
 	_ = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
@@ -222,6 +237,17 @@ func loadSkillsRecursive(dir string) []Skill {
 			if path != dir && isHidden(d) {
 				return fs.SkipDir
 			}
+
+			// Resolve and de-duplicate real directory paths to catch
+			// cycles introduced through symlinks higher up.
+			if path != dir {
+				if realPath, err := filepath.EvalSymlinks(path); err == nil {
+					if visited[realPath] {
+						return fs.SkipDir
+					}
+					visited[realPath] = true
+				}
+			}
 			return nil
 		}
 

pkg/skills/skills_test.go

@@ -353,6 +353,44 @@ description: A flat global agents skill
 	assert.True(t, foundFlat, "Expected to find flat-skill from ~/.agents/skills/flat-skill")
 }
 
+func TestLoadSkillsFromDir_RecursiveSymlinkCycle(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create a skill in a subdirectory.
+	skillDir := filepath.Join(tmpDir, "real-skill")
+	require.NoError(t, os.MkdirAll(skillDir, 0o755))
+
+	skillContent := `---
+name: real-skill
+description: A real skill
+---
+
+# Real Skill
+`
+	require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillContent), 0o644))
+
+	// Create a symlink cycle: tmpDir/real-skill/link -> tmpDir
+	require.NoError(t, os.Symlink(tmpDir, filepath.Join(skillDir, "link")))
+
+	// loadSkillsRecursive must return without looping forever.
+	skills := loadSkillsFromDir(tmpDir, true)
+
+	require.Len(t, skills, 1)
+	assert.Equal(t, "real-skill", skills[0].Name)
+	assert.Equal(t, "A real skill", skills[0].Description)
+}
+
+func TestLoadSkillsFromDir_RecursiveSymlinkSelfReference(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create a directory that symlinks to itself.
+	require.NoError(t, os.Symlink(tmpDir, filepath.Join(tmpDir, "self")))
+
+	// Must not loop forever.
+	skills := loadSkillsFromDir(tmpDir, true)
+	assert.Empty(t, skills)
+}
+
 func TestLoad_AgentsSkillsProjectFromNestedDir(t *testing.T) {
 	// Create a fake git repo with .agents/skills at the root
 	tmpRepo := t.TempDir()