Repository navigation
.NET: fix: revalidate file skill paths before use - #8151
SergeyMenshykh merged 5 commits into
Conversation
Sorry, something went wrong.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Both implementations can escape the configured root if a discovered skill directory is replaced by a symlink or junction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overviewAdds use-time path revalidation for file-backed skills across .NET and Python.
Changes:
- Centralizes .NET file-path validation.
- Propagates owning skill directories to resources and scripts.
- Adds regression tests and documentation for filesystem changes after discovery.
| File | Description |
|---|---|
| python/packages/core/tests/core/test_skills.py | Tests post-discovery resource and script replacement. |
| python/packages/core/AGENTS.md | Documents use-time validation. |
| python/packages/core/agent_framework/_skills.py | Adds revalidation before use, but critically fails to validate replacement of the skill directory itself. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs | Tests rejection of replaced files. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentFileSkillScriptTests.cs | Updates script validation tests. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs | Propagates trusted skill-directory paths. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillScript.cs | Validates scripts before runner invocation. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillResource.cs | Validates resources before reading. |
| dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillPathValidator.cs | Centralizes validation, but critically trusts the skill directory without checking for replacement by a link or junction. |
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): d8a455ac6238
Model: gpt-5.6-sol-fast
Overview
The PR consistently propagates each discovered skill directory into file-backed resources and scripts, then rechecks containment, file type, and child path segments immediately before use in both .NET and Python. The added regression tests establish rejection of leaf-file symlink substitution and ensure script runners are not invoked after validation fails. The remaining supported skill-directory substitution concern is already covered by unresolved review threads, and the broader validation-to-consumption race predates this change, so this pass found no additional publishable issue.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Sorry, something went wrong.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Motivation & Context
File-backed resources and scripts may be used after their initial discovery. Rechecking their current filesystem state at the point of use keeps runtime behavior consistent with the rules applied during discovery and prevents stale path entries from being used after they become invalid.
Description & Review Guide
Related Issue
N/A — no public issue is available to link.
Contribution Checklist