Add proper skills isolation (copying and allowlisting)

eng/skill-validator/src/Services/AgentRunner.cs

@@ -240,8 +240,10 @@ internal static SessionConfig BuildSessionConfig(
 
         // Build additional noise skill directories when noise testing is active.
         // For additional skills we stage a temp directory with copies of each
-        // skill's SKILL.md so the SDK discovers exactly those skills — not
+        // skill's content so the SDK discovers exactly those skills — not
         // every sibling that happens to share the same parent directory.
+        // We copy the full directory tree (references/, scripts/, etc.) so that
+        // relative links inside SKILL.md continue to resolve.
         var noiseDirs = new List<string>();
         if (additionalSkills is { Count: > 0 })
         {
@@ -256,8 +258,7 @@ internal static SessionConfig BuildSessionConfig(
                     continue;
 
                 var stagedSkillDir = Path.Combine(stageDir, Path.GetFileName(s.Path));
-                Directory.CreateDirectory(stagedSkillDir);
-                File.Copy(skillMdPath, Path.Combine(stagedSkillDir, "SKILL.md"));
+                CopyDirectory(s.Path, stagedSkillDir);
             }
 
             noiseDirs.Add(stageDir);
@@ -325,12 +326,20 @@ internal static SessionConfig BuildSessionConfig(
         {
             // Stage the single skill into a temp directory so the SDK discovers
             // only this skill — not every sibling that shares the same parent.
+            // Copy the full directory tree (references/, scripts/, etc.) so that
+            // relative links inside SKILL.md continue to resolve.
             var isoStageDir = Path.Combine(Path.GetTempPath(), $"sv-iso-{Guid.NewGuid():N}");
             Directory.CreateDirectory(isoStageDir);
             _workDirs.Add(isoStageDir);
 
             var stagedSkillDir = Path.Combine(isoStageDir, Path.GetFileName(skill.Path));
-            Directory.CreateDirectory(stagedSkillDir);
+            if (Directory.Exists(skill.Path))
+                CopyDirectory(skill.Path, stagedSkillDir);
+            else
+                Directory.CreateDirectory(stagedSkillDir);
+
+            // Always write SKILL.md from the in-memory content (may differ from
+            // the on-disk version when the validator applies transformations).
             File.WriteAllText(Path.Combine(stagedSkillDir, "SKILL.md"), skill.SkillMdContent);
 
             skillDirs = [isoStageDir];
@@ -340,6 +349,32 @@ internal static SessionConfig BuildSessionConfig(
             skillDirs = [];
         }
 
+        // Precompute the additional allowed directories once so we don't
+        // allocate on every permission request (the set is fixed per session).
+        var additionalAllowedDirs = skillDirs
+            .Concat(noiseDirs)
+            .Where(d => !string.IsNullOrEmpty(d))
+            .ToList();
+
+        // In isolated runs the agent should only access the staged copies, not
+        // the original skill tree (which includes sibling skills).  Pass null
+        // for skillPath so the original location is NOT in the allowlist.
+        // In plugin mode, validate that skillPath is under pluginRoot before
+        // allowlisting it; otherwise fall back to pluginRoot coverage alone.
+        string? effectiveSkillPath = null;
+        if (pluginRoot is not null && skillPath is not null)
+        {
+            var normalizedSkill = Path.GetFullPath(skillPath);
+            var normalizedPlugin = Path.GetFullPath(pluginRoot);
+            if (!Path.EndsInDirectorySeparator(normalizedPlugin))
+                normalizedPlugin += Path.DirectorySeparatorChar;
+            var cmp = OperatingSystem.IsWindows()
+                ? StringComparison.OrdinalIgnoreCase
+                : StringComparison.Ordinal;
+            if (normalizedSkill.StartsWith(normalizedPlugin, cmp))
+                effectiveSkillPath = skillPath;
+        }
+
         return new SessionConfig
         {
             Model = model,
@@ -352,7 +387,7 @@ internal static SessionConfig BuildSessionConfig(
             OnPermissionRequest = (request, _) =>
             {
                 var runLabel = skill is not null ? "skilled" : "baseline";
-                var result = CheckPermission(request, workDir, skillPath, verbose ? log : null, runLabel, pluginRoot);
+                var result = CheckPermission(request, workDir, effectiveSkillPath, verbose ? log : null, runLabel, pluginRoot, additionalAllowedDirs);
                 return Task.FromResult(new PermissionRequestResult
                 {
                     Kind = result ? PermissionRequestResultKind.Approved : PermissionRequestResultKind.DeniedByRules,
@@ -831,12 +866,53 @@ internal static bool IsAllowedMcpCommand(string command)
         return args;
     }
 
+    /// <summary>
+    /// Recursively copies a directory tree, skipping symlinks and reparse
+    /// points so that staging cannot pull in content from outside the
+    /// source root.
+    /// </summary>
     private static void CopyDirectory(string source, string destination)
+    {
+        var sourceRoot = Path.GetFullPath(source);
+        if (!Path.EndsInDirectorySeparator(sourceRoot))
+            sourceRoot += Path.DirectorySeparatorChar;
+        CopyDirectoryCore(source, destination, sourceRoot);
+    }
+
+    private static void CopyDirectoryCore(string source, string destination, string sourceRoot)
     {
         Directory.CreateDirectory(destination);
-        foreach (var file in Directory.GetFiles(source))
-            File.Copy(file, Path.Combine(destination, Path.GetFileName(file)), true);
-        foreach (var dir in Directory.GetDirectories(source))
-            CopyDirectory(dir, Path.Combine(destination, Path.GetFileName(dir)));
+
+        foreach (var entry in Directory.EnumerateFileSystemEntries(source))
+        {
+            var attributes = File.GetAttributes(entry);
+
+            // Skip symlinks / reparse points to avoid copying data outside the skill tree.
+            if ((attributes & FileAttributes.ReparsePoint) != 0)
+                continue;
+
+            var name = Path.GetFileName(entry);
+            var destPath = Path.Combine(destination, name);
+
+            if ((attributes & FileAttributes.Directory) != 0)
+            {
+                var entryFull = Path.GetFullPath(entry);
+                if (!Path.EndsInDirectorySeparator(entryFull))
+                    entryFull += Path.DirectorySeparatorChar;
+
+                // Guard against junctions that resolve outside the source root.
+                var pathComparison = OperatingSystem.IsWindows()
+                    ? StringComparison.OrdinalIgnoreCase
+                    : StringComparison.Ordinal;
+                if (!entryFull.StartsWith(sourceRoot, pathComparison))
+                    continue;
+
+                CopyDirectoryCore(entryFull.TrimEnd(Path.DirectorySeparatorChar), destPath, sourceRoot);
+            }
+            else
+            {
+                File.Copy(entry, destPath, overwrite: true);
+            }
+        }
     }
 }

eng/skill-validator/tests/RunnerTests.cs

@@ -28,6 +28,137 @@ public void SetsSkillDirectoriesToStagedIsolationDir()
         Assert.True(File.Exists(Path.Combine(stagedSkillDir, "SKILL.md")));
     }
 
+    [Fact]
+    public async Task IsolationStageCopiesReferencesAndScripts()
+    {
+        // Create a real skill directory with references/ and scripts/ subdirectories
+        var tmpBase = Path.Combine(Path.GetTempPath(), $"sv-test-{Guid.NewGuid():N}");
+        var skillDir = Path.Combine(tmpBase, "skills", "my-skill");
+        var refsDir = Path.Combine(skillDir, "references");
+        var scriptsDir = Path.Combine(skillDir, "scripts");
+        Directory.CreateDirectory(refsDir);
+        Directory.CreateDirectory(scriptsDir);
+        File.WriteAllText(Path.Combine(skillDir, "SKILL.md"), "# My Skill");
+        File.WriteAllText(Path.Combine(refsDir, "patterns.md"), "# Patterns");
+        File.WriteAllText(Path.Combine(scriptsDir, "Run.ps1"), "Write-Host 'hi'");
+
+        try
+        {
+            var skill = new SkillInfo("my-skill", "A skill", skillDir,
+                Path.Combine(skillDir, "SKILL.md"), "# My Skill (transformed)");
+
+            var config = AgentRunner.BuildSessionConfig(skill, null, "gpt-4.1", "C:\\tmp\\work");
+
+            var stageDir = config.SkillDirectories![0];
+            var stagedSkillDir = Path.Combine(stageDir, "my-skill");
+
+            // SKILL.md should use in-memory content (may be transformed)
+            Assert.Equal("# My Skill (transformed)", File.ReadAllText(Path.Combine(stagedSkillDir, "SKILL.md")));
+            // references/ and scripts/ should be copied
+            Assert.True(File.Exists(Path.Combine(stagedSkillDir, "references", "patterns.md")));
+            Assert.Equal("# Patterns", File.ReadAllText(Path.Combine(stagedSkillDir, "references", "patterns.md")));
+            Assert.True(File.Exists(Path.Combine(stagedSkillDir, "scripts", "Run.ps1")));
+        }
+        finally
+        {
+            try { Directory.Delete(tmpBase, true); } catch { }
+            try { await AgentRunner.CleanupWorkDirs(); } catch { }
+        }
+    }
+
+    [Fact]
+    public async Task IsolatedStagingDoesNotExposeOriginalOrSiblingSkills()
+    {
+        // Create a skills root with a target skill and a sibling skill
+        var tmpBase = Path.Combine(Path.GetTempPath(), $"sv-iso-test-{Guid.NewGuid():N}");
+        var skillsRoot = Path.Combine(tmpBase, "skills");
+        var targetSkillDir = Path.Combine(skillsRoot, "my-skill");
+        var siblingSkillDir = Path.Combine(skillsRoot, "sibling-skill");
+
+        Directory.CreateDirectory(targetSkillDir);
+        Directory.CreateDirectory(siblingSkillDir);
+
+        File.WriteAllText(Path.Combine(targetSkillDir, "SKILL.md"), "# My Skill");
+        File.WriteAllText(Path.Combine(siblingSkillDir, "SKILL.md"), "# Sibling Skill");
+
+        try
+        {
+            var skill = new SkillInfo(
+                "my-skill",
+                "A skill",
+                targetSkillDir,
+                Path.Combine(targetSkillDir, "SKILL.md"),
+                "# My Skill (transformed)");
+
+            var config = AgentRunner.BuildSessionConfig(skill, null, "gpt-4.1", "C:\\tmp\\work");
+
+            // Only a staged isolation directory should be exposed.
+            Assert.NotNull(config.SkillDirectories);
+            Assert.Single(config.SkillDirectories!);
+
+            var stageDir = config.SkillDirectories![0];
+            Assert.StartsWith(Path.GetTempPath(), stageDir);
+
+            var stagedTargetDir = Path.Combine(stageDir, "my-skill");
+            var stagedSiblingDir = Path.Combine(stageDir, "sibling-skill");
+
+            // The target skill should be available in the staged directory.
+            Assert.True(Directory.Exists(stagedTargetDir));
+
+            // The sibling skill from the original skills root must not be exposed in isolation.
+            Assert.False(Directory.Exists(stagedSiblingDir));
+
+            // Permission check should deny access to the original skill directory
+            var workDir = Path.GetFullPath(Path.Combine(Path.GetTempPath(), "work"));
+            var originalSkillFilePath = Path.Combine(targetSkillDir, "SKILL.md");
+            var escaped = originalSkillFilePath.Replace("\\", "\\\\");
+            var req = System.Text.Json.JsonSerializer.Deserialize<GitHub.Copilot.SDK.PermissionRequest>(
+                $"{{\"kind\":\"read\",\"path\":\"{escaped}\"}}")!;
+            var denied = AgentRunner.CheckPermission(req, workDir, null, log: null);
+            Assert.False(denied);
+        }
+        finally
+        {
+            try { Directory.Delete(tmpBase, true); } catch { }
+            try { await AgentRunner.CleanupWorkDirs(); } catch { }
+        }
+    }
+
+    [Fact]
+    public async Task AdditionalSkillsStageCopiesReferencesDir()
+    {
+        // Create a noise skill with references
+        var tmpBase = Path.Combine(Path.GetTempPath(), $"sv-test-{Guid.NewGuid():N}");
+        var noiseSkillDir = Path.Combine(tmpBase, "plugin-x", "skills", "noise-skill");
+        var refsDir = Path.Combine(noiseSkillDir, "references");
+        Directory.CreateDirectory(refsDir);
+        File.WriteAllText(Path.Combine(noiseSkillDir, "SKILL.md"), "# Noise");
+        File.WriteAllText(Path.Combine(refsDir, "guide.md"), "# Guide");
+
+        try
+        {
+            var additionalSkills = new[]
+            {
+                new SkillInfo("noise-skill", "Noise", noiseSkillDir,
+                    Path.Combine(noiseSkillDir, "SKILL.md"), "# Noise"),
+            };
+
+            var config = AgentRunner.BuildSessionConfig(MockSkill, pluginRoot: null, "gpt-4.1", "C:\\tmp\\work",
+                additionalSkills: additionalSkills);
+
+            var noiseStageDir = config.SkillDirectories![1];
+            var stagedNoiseSkill = Path.Combine(noiseStageDir, "noise-skill");
+            Assert.True(File.Exists(Path.Combine(stagedNoiseSkill, "SKILL.md")));
+            Assert.True(File.Exists(Path.Combine(stagedNoiseSkill, "references", "guide.md")));
+            Assert.Equal("# Guide", File.ReadAllText(Path.Combine(stagedNoiseSkill, "references", "guide.md")));
+        }
+        finally
+        {
+            try { Directory.Delete(tmpBase, true); } catch { }
+            try { await AgentRunner.CleanupWorkDirs(); } catch { }
+        }
+    }
+
     [Fact]
     public async Task AdditionalSkillsStageOnlyVerifiedSkillDirs()
     {
@@ -572,6 +703,16 @@ public void ApprovesPathsInsideSkillPath()
         Assert.True(result);
     }
 
+    [Fact]
+    public void ApprovesPathsInsideAdditionalAllowedDirs()
+    {
+        var stagingDir = Path.GetFullPath(Path.Combine(Path.GetTempPath(), "sv-iso-abc123", "my-skill"));
+        var refPath = Path.Combine(stagingDir, "references", "guide.md");
+        var result = AgentRunner.CheckPermission(MakePathRequest(refPath), WorkDir, null, log: null,
+            additionalAllowedDirs: [stagingDir]);
+        Assert.True(result);
+    }
+
     [Fact]
     public void DeniesPathsOutsideAllowedDirectories()
     {