Skip to content

.NET: fix: revalidate file skill paths before use - #8151

Merged
SergeyMenshykh merged 5 commits into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-fix-skill-path-revalidation
Sep 9, 2026
Merged

.NET: fix: revalidate file skill paths before use#8151
SergeyMenshykh merged 5 commits into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-fix-skill-path-revalidation

Conversation

@SergeyMenshykh

Copy link
Copy Markdown
Contributor

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

  • What are the major changes? Discovered resources and scripts retain their owning skill directory and repeat containment, regular-file, and link/reparse-point checks before reads or script-runner invocation. The .NET path rules are centralized in an internal validator, and regression coverage exercises filesystem changes after discovery in both implementations.
  • What is the impact of these changes? File-backed skills continue to behave as before while their files remain valid. Entries that no longer satisfy discovery-time path rules now fail at use time instead of being consumed. The change does not alter public .NET APIs, and the Python constructor addition is optional and keyword-only.
  • What do you want reviewers to focus on? Please focus on consistency between discovery-time and use-time path validation and on the propagation of the owning skill directory to discovered resources and scripts.

Related Issue

N/A — no public issue is available to link.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 8, 2026 13:55
@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python .NET Usage: [Issues, PRs], Target: .Net labels Sep 8, 2026
@github-actions github-actions Bot changed the title fix: revalidate file skill paths before use Python: fix: revalidate file skill paths before use Sep 8, 2026
@github-actions github-actions Bot changed the title Python: fix: revalidate file skill paths before use .NET: fix: revalidate file skill paths before use Sep 8, 2026
@SergeyMenshykh
SergeyMenshykh marked this pull request as ready for review September 8, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 overview

Adds 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 summaries
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.
Review details
  • 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.

Comment thread dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillPathValidator.cs Outdated
Comment thread python/packages/core/agent_framework/_skills.py Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread python/packages/core/agent_framework/_skills.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue Sep 9, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 9, 2026
@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue Sep 9, 2026
Merged via the queue into microsoft:main with commit 5b188dd Sep 9, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs .NET Usage: [Issues, PRs], Target: .Net python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants