[dotnet-port-fixes] Revalidate file skill paths before use

[Michelle Clayton (michelle-clayton-work)]

Tip

Your pull request is ready to create! 🎉 ✅

Everything is OK—the changes have been pushed to branch dotnet-port-fixes-file-skill-revalidation-20260916-d42056936256a58c. Please review the changes, including any protected files, before creating the pull request.

Create the pull request

The original pull request description is below.

---

Summary

• Revalidate discovered file-skill resource and script paths against the original discovery root immediately before reading or running them.
• Preserve the existing file-backed skill API and runner metadata while rejecting parent-directory or skill-directory symlink swaps after discovery.
• Add regression tests covering resource reads and script runs after a discovered parent path is replaced with a symlink.

Ported .NET PRs

• .NET: fix: revalidate file skill paths before use agent-framework#8151 - revalidate file-backed skill resource and script paths before use
  • Upstream commit: 5b188dd80e46d6fe7ae57ca88c4a51bb2e10021b

Breaking Changes

No. The Go public API is unchanged; discovered file-backed skills now fail safely if their underlying path is replaced by a symlink after discovery.

Tests and Examples

• go test ./agent/skills/fsskills ./agent/skills
• Added regression tests for resource reads and script runs after parent-directory symlink replacement
• No example changes

Notes

• I classified this as a fixes port because the upstream diff only added internal path-scope and validation helpers plus resource/script call-site checks; it did not add public options, builders, or exported contract changes.
• Recent upstream work such as the session-store promotion and OpenTelemetry source-name changes was deferred because it changes public API or targets .NET-only surfaces.

Closes #1081

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request hardens file-backed skill resource and script access by revalidating discovered paths before use.

Changes:

• Adds scoped path and symlink validation.
• Applies validation before resource reads and script execution.
• Adds regression tests for path replacement scenarios.

Outstanding findings: critical (2 votes) symlink checks can be bypassed on link-unaware filesystems; moderate (1 vote) non-regular paths are not rejected.

File summaries

File	Summary
agent/skills/fsskills/source.go	Applies validation to resources and scripts.
agent/skills/fsskills/source_test.go	Tests resource-path revalidation.
agent/skills/fsskills/source_script_test.go	Tests script-path revalidation.
agent/skills/fsskills/path_scope.go	Adds path containment and symlink validation helpers.

Review details

Suppressed comments (1)

agent/skills/fsskills/path_scope.go:47

• After discovery, the path can be replaced by a directory, FIFO, device, or another non-regular node. fs.Stat only checks that the path exists, so scripts still invoke the runner for such a path (and a custom runner can consume it), even though discovery only collects regular files; resources also get a later, less precise ReadFile failure. Check info.Mode().IsRegular() after Stat and reject non-regular paths before returning.

	if _, err := fs.Stat(s.rootFS, fullPath); err != nil {
		if errors.Is(err, fs.ErrNotExist) {
			return "", fmt.Errorf("%s file %q was not found in the skill directory", kind, relativePath)
		}
		return "", err

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Michelle Clayton (michelle-clayton-work)]

Copilot address PR feedback

[Copilot]

Copilot address PR feedback

Addressed in 30d82cc: path revalidation now fails closed when the filesystem does not support fs.ReadLinkFS, with regression coverage for resource reads and script runs.

[github-actions]

Scope: internal-only

Changed Go contract: None. All new/changed identifiers are unexported: skillPathScope, newSkillPathScope, validateDiscoveredPathForUse, contains, hasLinkOrInspectionFailureInPath, isUnsafePath (new file agent/skills/fsskills/path_scope.go), plus the unexported discoveredSkillDir struct fields (fsys→split into rootFS/skillFS) and internal helper signatures (parseSkillDirectory, discoverResourceFiles, discoverScriptFiles, newScript, newFileScriptRunFunc) in source.go. The public Source, SourceOptions, skills.Resource, and skills.Script API surface is unchanged.

Upstream evidence reviewed: This is a port of microsoft/agent-framework#8151 (commit 5b188dd80e46d6fe7ae57ca88c4a51bb2e10021b):

• dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillPathScope.cs (new, internal sealed class) and AgentFileSkillPathValidator.cs (new, internal static class) — both internal, matching the Go unexported skillPathScope.
• dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs — ParseSkillDirectory, DiscoverResourceFiles, DiscoverScriptFiles now thread an AgentFileSkillPathScope through discovery and revalidate immediately before file read/script run, same shape as the Go scope.validateDiscoveredPathForUse calls added at the fs.ReadFile and script-run call sites.
• python/packages/core/agent_framework/_skills.py — new _SkillPathScope dataclass and FileSkillsSource._validate_file_path_for_use static method (both private/internal, prefixed _), invoked from _FileSkillResource._read_validated_resource and FileSkillScript.run before use, mirroring the Go scope.validateDiscoveredPathForUse calls in discoverResourceFiles/newFileScriptRunFunc.
• Both upstream implementations retain the discovery root (trusted_root/TrustedRootPrefix) alongside the skill directory so intermediate directories between the root and the file are also rescanned for symlinks — the Go skillPathScope carries rootFS and skillDirPath/skillDirPrefix for the same purpose, and hasLinkOrInspectionFailureInPath walks every path segment consistent with HasLinkOrReparsePointInPath (.NET) / _has_link_or_reparse_point_in_path (Python).
• Tests: dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/{AgentFileSkillScriptTests.cs,FileAgentSkillLoaderTests.cs} and python/packages/core/tests/core/test_skills.py cover the same symlink-swap-after-discovery scenario as the new Go tests TestFileSource_ScriptExecution_RevalidatesParentDirectoriesBeforeRun, TestFileSource_ReadResource_RevalidatesParentDirectoriesBeforeUse, and the "without link inspection" fallback tests.

Result: aligned. Verified go build ./... and go test ./agent/skills/... pass on the PR branch. This is a security hardening fix that revalidates discovered file-skill paths immediately before use, ported with equivalent unconditional enablement in all three languages (no opt-in/feature gate exists upstream for this fix, so unconditional Go enablement matches). No public API, defaults, or observable behavior changed except that previously-successful reads/runs against a path swapped for a symlink after discovery now correctly fail — consistent with the upstream fix's intent.

Generated by Go API Consistency Review Agent for #1089 · copilot · auto · 49.3 AIC · ⌖ 5.89 AIC · ⊞ 9.6K · ◷