[dotnet-port-fixes] Harden file skill discovery

agent/skills/fsskills/source.go

@@ -170,7 +170,7 @@ func NewSourceOptions(opts SourceOptions, filesystems ...fs.FS) *Source {
 
 // Skills discovers and loads valid skills from the configured filesystems.
 func (s *Source) Skills(ctx context.Context) ([]*skills.Skill, error) {
-	directories := discoverSkillDirectories(s.filesystems)
+	directories := discoverSkillDirectories(s.filesystems, s.logger)
 	s.logger.Info("Discovered potential skills", "count", len(directories))
 
 	skills := make([]*skills.Skill, 0, len(directories))
@@ -190,10 +190,10 @@ func (s *Source) Skills(ctx context.Context) ([]*skills.Skill, error) {
 	return skills, nil
 }
 
-func discoverSkillDirectories(filesystems []fs.FS) []discoveredSkillDir {
+func discoverSkillDirectories(filesystems []fs.FS, logger *slog.Logger) []discoveredSkillDir {
 	var results []discoveredSkillDir
 	for _, filesystem := range filesystems {
-		searchForSkills(filesystem, ".", &results, 0)
+		searchForSkills(filesystem, ".", logger, &results, 0)
 	}
 	return results
 }
@@ -203,29 +203,46 @@ func discoverSkillDirectories(filesystems []fs.FS) []discoveredSkillDir {
 // independent of SourceOptions.SearchDepth, which governs only resource and
 // script discovery within an already-discovered skill directory. This matches
 // the .NET SDK, which bounds the two concerns separately.
-func searchForSkills(filesystem fs.FS, dir string, results *[]discoveredSkillDir, currentDepth int) {
-	skillPath := path.Join(dir, skillFileName)
-	if _, err := fs.Stat(filesystem, skillPath); err == nil {
+func searchForSkills(filesystem fs.FS, dir string, logger *slog.Logger, results *[]discoveredSkillDir, currentDepth int) {
+	entries, err := fs.ReadDir(filesystem, dir)
+	if err != nil {
+		return
+	}
+
+	for _, entry := range entries {
+		if entry.Name() != skillFileName {
+			continue
+		}
+
+		skillPath := path.Join(dir, entry.Name())
+		if isUnsafeDirEntry(filesystem, skillPath, entry) {
+			logger.Warn("Skipping skill discovery path: symbolic link or inspection failure", "path", skillPath)
+			return
+		}
+
 		sub := filesystem
 		var subErr error
 		if dir != "." {
 			sub, subErr = fs.Sub(filesystem, dir)
 		}
 		if subErr == nil {
 			*results = append(*results, discoveredSkillDir{fsys: sub, path: dir})
-			return
 		}
-	}
-	if currentDepth >= defaultSearchDepth {
 		return
 	}
-	entries, err := fs.ReadDir(filesystem, dir)
-	if err != nil {
+
+	if currentDepth >= defaultSearchDepth {
 		return
 	}
+
 	for _, entry := range entries {
+		entryPath := path.Join(dir, entry.Name())
+		if isUnsafeDirEntry(filesystem, entryPath, entry) {
+			logger.Warn("Skipping skill discovery path: symbolic link or inspection failure", "path", entryPath)
+			continue
+		}
 		if entry.IsDir() {
-			searchForSkills(filesystem, path.Join(dir, entry.Name()), results, currentDepth+1)
+			searchForSkills(filesystem, entryPath, logger, results, currentDepth+1)
 		}
 	}
 }
@@ -502,6 +519,10 @@ func (s *Source) scanForFiles(
 
 	for _, entry := range entries {
 		entryPath := path.Join(dir, entry.Name())
+		if isUnsafeDirEntry(skillFS, entryPath, entry) {
+			s.logger.Warn("Skipping file skill path: symbolic link or inspection failure", "skillName", skillName, "filePath", entryPath, "kind", kind)
+			continue
+		}
 		if entry.IsDir() {
 			if currentDepth < s.searchDepth {
 				s.scanForFiles(skillFS, entryPath, skillName, currentDepth+1, allowedExtensions, filter, kind, collect)
@@ -534,6 +555,24 @@ func (s *Source) scanForFiles(
 	}
 }
 
+func isUnsafeDirEntry(filesystem fs.FS, filePath string, entry fs.DirEntry) bool {
+	if entry.Type()&fs.ModeSymlink != 0 {
+		return true
+	}
+
+	readLinkFS, ok := filesystem.(fs.ReadLinkFS)
+	if !ok {
+		return false
+	}
+
+	info, err := readLinkFS.Lstat(filePath)
+	if err == nil {
+		return info.Mode()&fs.ModeSymlink != 0
+	}
+
+	return !errors.Is(err, fs.ErrNotExist)
+}
+
 func buildExtensionSet(extensions []string, defaults []string) map[string]bool {
 	if extensions == nil {
 		extensions = defaults

agent/skills/fsskills/source_script_test.go

@@ -342,6 +342,32 @@ func TestFileSource_ScriptFilter_IncludesOnlyMatchingScripts(t *testing.T) {
 	}
 }
 
+func TestFileSource_SymlinkedScript_IsSkipped(t *testing.T) {
+	root := t.TempDir()
+	createSkillDir(t, root, "script-link-skill", "Symlinked script", "Body.")
+	outsideScript := filepath.Join(root, "outside.py")
+	if err := os.WriteFile(outsideScript, []byte("print('outside')"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	createSymlink(t, filepath.Join(root, "script-link-skill", "scripts", "run.py"), outsideScript)
+	source := fsskills.NewSourceOptions(fsskills.SourceOptions{
+		ScriptRunner: func(context.Context, *skills.Skill, *skills.Script, []string) (any, error) {
+			return nil, nil
+		},
+	}, os.DirFS(root))
+
+	loaded, err := source.Skills(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded) != 1 {
+		t.Fatalf("expected 1 skill, got %d", len(loaded))
+	}
+	if len(loaded[0].Scripts) != 0 {
+		t.Fatalf("expected symlinked script to be skipped, got %d scripts", len(loaded[0].Scripts))
+	}
+}
+
 func TestFileScript_RunWithNonFileSkill_ReturnsError(t *testing.T) {
 	root := t.TempDir()
 	createSkillDir(t, root, "script-owner", "Script owner", "Body.")

agent/skills/fsskills/source_test.go

@@ -142,6 +142,48 @@ func TestFileSource_RootSkillFileWithNestedSkillFile_DoesNotAbortDiscovery(t *te
 	}
 }
 
+func TestFileSource_SymlinkedSkillFile_IsSkipped(t *testing.T) {
+	root := t.TempDir()
+	skillDir := filepath.Join(root, "linked-skill")
+	if err := os.MkdirAll(skillDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	outsideSkillFile := filepath.Join(root, "outside-SKILL.md")
+	if err := os.WriteFile(outsideSkillFile, []byte("---\nname: linked-skill\ndescription: Linked skill file\n---\nBody."), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	createSymlink(t, filepath.Join(skillDir, "SKILL.md"), outsideSkillFile)
+
+	source := fsskills.NewSource(os.DirFS(root))
+	loaded, err := source.Skills(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded) != 0 {
+		t.Fatalf("expected symlinked SKILL.md to be skipped, got %d skills", len(loaded))
+	}
+}
+
+func TestFileSource_ConfiguredRootSymlink_StillDiscoversSkills(t *testing.T) {
+	root := t.TempDir()
+	realRoot := filepath.Join(root, "real-root")
+	linkedRoot := filepath.Join(root, "linked-root")
+	createSkillDir(t, realRoot, "my-skill", "A skill", "Body.")
+	createSymlink(t, linkedRoot, realRoot)
+
+	source := fsskills.NewSource(os.DirFS(linkedRoot))
+	loaded, err := source.Skills(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded) != 1 {
+		t.Fatalf("expected 1 skill, got %d", len(loaded))
+	}
+	if loaded[0].Frontmatter.Name != "my-skill" {
+		t.Fatalf("expected my-skill, got %q", loaded[0].Frontmatter.Name)
+	}
+}
+
 func TestFileSource_NestedSkillFileUnderSkillRoot_NotDiscoveredAsIndependentSkill(t *testing.T) {
 	root := t.TempDir()
 	createSkillDir(t, root, "parent-skill", "Parent", "Parent body.")
@@ -548,6 +590,28 @@ func TestFileSource_NoDuplicateResourcesFromSamePath(t *testing.T) {
 	}
 }
 
+func TestFileSource_SymlinkedResource_IsSkipped(t *testing.T) {
+	root := t.TempDir()
+	createSkillDir(t, root, "resource-link-skill", "Symlinked resource", "Body.")
+	outsideResource := filepath.Join(root, "outside.md")
+	if err := os.WriteFile(outsideResource, []byte("secret"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	createSymlink(t, filepath.Join(root, "resource-link-skill", "references", "secret.md"), outsideResource)
+
+	source := fsskills.NewSource(os.DirFS(root))
+	loaded, err := source.Skills(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(loaded) != 1 {
+		t.Fatalf("expected 1 skill, got %d", len(loaded))
+	}
+	if len(loaded[0].Resources) != 0 {
+		t.Fatalf("expected symlinked resource to be skipped, got %d resources", len(loaded[0].Resources))
+	}
+}
+
 func createSkillDir(t *testing.T, root, name, description, body string) {
 	t.Helper()
 	skillDir := filepath.Join(root, name)
@@ -594,3 +658,13 @@ func createRelativeFile(t *testing.T, root, relativePath, content string) {
 		t.Fatal(err)
 	}
 }
+
+func createSymlink(t *testing.T, linkPath, targetPath string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Dir(linkPath), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(targetPath, linkPath); err != nil {
+		t.Skipf("symlink creation unavailable: %v", err)
+	}
+}