Add proper skills isolation (copying and allowlisting) - #402
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens skill isolation in eng/skill-validator by staging full skill directory trees (not just SKILL.md) and expanding the runtime filesystem allowlist so agents can access staged companion files (e.g., references/, scripts/) during evaluation.
Changes:
- Stage/copy full skill directory trees for isolated runs and for “additional/noise” skills.
- Extend permission checking to allow access to staged skill directories at runtime.
- Add tests validating directory staging and additional allowlisted path approval.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| eng/skill-validator/src/Services/AgentRunner.cs | Copy full skill trees into temp staging dirs and pass staged dirs as additional allowlisted paths in permission checks. |
| eng/skill-validator/tests/RunnerTests.cs | Add unit tests covering copying of references//scripts/ and permission approval for explicitly allowlisted dirs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
- Pass null skillPath in isolated mode so the original skill tree (including siblings) is not accessible; only staged copies are allowed. - Harden CopyDirectory to skip symlinks/reparse points and verify that recursed paths stay within the source root. - Precompute additionalAllowedDirs once at session creation instead of allocating on every permission request. - Add IsolatedStagingDoesNotExposeOriginalOrSiblingSkills test that verifies both directory-level and permission-level isolation.
ViktorHofer
approved these changes
Mar 18, 2026
JanKrivanek
enabled auto-merge (squash)
March 18, 2026 19:56
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves skill isolation in eng/skill-validator by staging full skill directories into unique temp locations (instead of only staging SKILL.md) and ensuring the staged locations are allowlisted for runtime permission checks. This addresses parallel-run isolation issues when --plugin-dir is not honored in ACP mode.
Changes:
- Stage/copy full skill directory trees (including
references/,scripts/, etc.) for isolated and additional/noise skills. - Add staged directories to the permission allowlist via
additionalAllowedDirsand avoid allowlisting the original skill tree in isolated runs. - Add/extend tests validating directory staging, isolation behavior, and allowlisting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eng/skill-validator/src/Services/AgentRunner.cs | Copies full skill directories for staging, hardens copy behavior, and updates permission allowlisting for isolated runs. |
| eng/skill-validator/tests/RunnerTests.cs | Adds regression tests for staged directory contents, isolation from sibling/original skills, and additional allowlisted dirs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…validate skillPath under pluginRoot
Member
Author
|
/evaluate |
Contributor
Skill Validation Results
[1] (Plugin) Quality unchanged but weighted score is -8.9% due to: tokens (11848 → 31146), tool calls (0 → 2), time (11.0s → 17.0s)
Model: claude-opus-4.6 | Judge: claude-opus-4.6 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The
--plugin-dirnot properly honored by copilot-cli in ACP mode - https://github.com/github/copilot-agent-runtime/issues/4585 - so for this reason we need to copy the plugins manually to isolate parallel runs of agentsThe copying and runtime access control was not done fully properly though
Fixes