diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs index 8c034b3122..71a7124281 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs @@ -17,8 +17,9 @@ namespace Microsoft.Agents.AI; /// /// /// Searches directories recursively (up to levels) for SKILL.md files. -/// Each file is validated for YAML frontmatter and resource integrity. Invalid skills are excluded -/// with logged warnings. Resource paths are checked against path traversal and symlink escape attacks. +/// Each file is validated for YAML frontmatter. Resource files are discovered by scanning the skill +/// directory for files with matching extensions. Invalid resources are skipped with logged warnings. +/// Resource paths are checked against path traversal and symlink escape attacks. /// internal sealed partial class FileAgentSkillLoader { @@ -33,14 +34,6 @@ internal sealed partial class FileAgentSkillLoader // Example: "---\nname: foo\n---\nBody" → Group 1: "name: foo\n" private static readonly Regex s_frontmatterRegex = new(@"\A\uFEFF?^---\s*$(.+?)^---\s*$", RegexOptions.Multiline | RegexOptions.Singleline | RegexOptions.Compiled, TimeSpan.FromSeconds(5)); - // Matches markdown links to local resource files. Group 1 = relative file path. - // Supports optional ./ or ../ prefixes; excludes URLs (no ":" in the path character class). - // Intentionally conservative: only matches paths with word characters, hyphens, dots, - // and forward slashes. Paths with spaces or special characters are not supported. - // Examples: [doc](refs/FAQ.md) → "refs/FAQ.md", [s](./s.json) → "./s.json", - // [p](../shared/doc.txt) → "../shared/doc.txt" - private static readonly Regex s_resourceLinkRegex = new(@"\[.*?\]\((\.?\.?/?[\w][\w\-./]*\.\w+)\)", RegexOptions.Compiled, TimeSpan.FromSeconds(5)); - // Matches YAML "key: value" lines. Group 1 = key, Group 2 = quoted value, Group 3 = unquoted value. // Accepts single or double quotes; the lazy quantifier trims trailing whitespace on unquoted values. // Examples: "name: foo" → (name, _, foo), "name: 'foo bar'" → (name, foo bar, _), @@ -52,14 +45,22 @@ internal sealed partial class FileAgentSkillLoader private static readonly Regex s_validNameRegex = new(@"^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$", RegexOptions.Compiled); private readonly ILogger _logger; + private readonly HashSet _allowedResourceExtensions; /// /// Initializes a new instance of the class. /// /// The logger instance. - internal FileAgentSkillLoader(ILogger logger) + /// File extensions to recognize as skill resources. When , defaults are used. + internal FileAgentSkillLoader(ILogger logger, IEnumerable? allowedResourceExtensions = null) { this._logger = logger; + + ValidateExtensions(allowedResourceExtensions); + + this._allowedResourceExtensions = new HashSet( + allowedResourceExtensions ?? [".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt"], + StringComparer.OrdinalIgnoreCase); } /// @@ -183,9 +184,9 @@ private static void SearchDirectoriesForSkills(string directory, List re } } - private FileAgentSkill? ParseSkillFile(string skillDirectoryPath) + private FileAgentSkill? ParseSkillFile(string skillDirectoryFullPath) { - string skillFilePath = Path.Combine(skillDirectoryPath, SkillFileName); + string skillFilePath = Path.Combine(skillDirectoryFullPath, SkillFileName); string content = File.ReadAllText(skillFilePath, Encoding.UTF8); @@ -194,17 +195,12 @@ private static void SearchDirectoriesForSkills(string directory, List re return null; } - List resourceNames = ExtractResourcePaths(body); - - if (!this.ValidateResources(skillDirectoryPath, resourceNames, frontmatter.Name)) - { - return null; - } + List resourceNames = this.DiscoverResourceFiles(skillDirectoryFullPath, frontmatter.Name); return new FileAgentSkill( frontmatter: frontmatter, body: body, - sourcePath: skillDirectoryPath, + sourcePath: skillDirectoryFullPath, resourceNames: resourceNames); } @@ -270,34 +266,84 @@ private bool TryParseSkillDocument(string content, string skillFilePath, out Ski return true; } - private bool ValidateResources(string skillDirectoryPath, List resourceNames, string skillName) + /// + /// Scans a skill directory for resource files matching the configured extensions. + /// + /// + /// Recursively walks and collects files whose extension + /// matches , excluding SKILL.md itself. Each candidate + /// is validated against path-traversal and symlink-escape checks; unsafe files are skipped with + /// a warning. + /// + private List DiscoverResourceFiles(string skillDirectoryFullPath, string skillName) { - string normalizedSkillPath = Path.GetFullPath(skillDirectoryPath) + Path.DirectorySeparatorChar; + string normalizedSkillDirectoryFullPath = skillDirectoryFullPath + Path.DirectorySeparatorChar; - foreach (string resourceName in resourceNames) + var resources = new List(); + +#if NET + var enumerationOptions = new EnumerationOptions + { + RecurseSubdirectories = true, + IgnoreInaccessible = true, + AttributesToSkip = FileAttributes.ReparsePoint, + }; + + foreach (string filePath in Directory.EnumerateFiles(skillDirectoryFullPath, "*", enumerationOptions)) +#else + foreach (string filePath in Directory.EnumerateFiles(skillDirectoryFullPath, "*", SearchOption.AllDirectories)) +#endif { - string fullPath = Path.GetFullPath(Path.Combine(skillDirectoryPath, resourceName)); + string fileName = Path.GetFileName(filePath); - if (!IsPathWithinDirectory(fullPath, normalizedSkillPath)) + // Exclude SKILL.md itself + if (string.Equals(fileName, SkillFileName, StringComparison.OrdinalIgnoreCase)) { - LogResourcePathTraversal(this._logger, skillName, resourceName); - return false; + continue; } - if (!File.Exists(fullPath)) + // Filter by extension + string extension = Path.GetExtension(filePath); + if (string.IsNullOrEmpty(extension) || !this._allowedResourceExtensions.Contains(extension)) + { + if (this._logger.IsEnabled(LogLevel.Debug)) + { + LogResourceSkippedExtension(this._logger, skillName, SanitizePathForLog(filePath), extension); + } + continue; + } + + // Normalize the enumerated path to guard against non-canonical forms + // (redundant separators, 8.3 short names, etc.) that would produce + // malformed relative resource names. + string resolvedFilePath = Path.GetFullPath(filePath); + + // Path containment check + if (!IsPathWithinDirectory(resolvedFilePath, normalizedSkillDirectoryFullPath)) { - LogMissingResource(this._logger, skillName, resourceName); - return false; + if (this._logger.IsEnabled(LogLevel.Warning)) + { + LogResourcePathTraversal(this._logger, skillName, SanitizePathForLog(filePath)); + } + continue; } - if (HasSymlinkInPath(fullPath, normalizedSkillPath)) + // Symlink check + if (HasSymlinkInPath(resolvedFilePath, normalizedSkillDirectoryFullPath)) { - LogResourceSymlinkEscape(this._logger, skillName, resourceName); - return false; + if (this._logger.IsEnabled(LogLevel.Warning)) + { + LogResourceSymlinkEscape(this._logger, skillName, SanitizePathForLog(filePath)); + } + continue; } + + // Compute relative path and normalize to forward slashes + string relativePath = resolvedFilePath.Substring(normalizedSkillDirectoryFullPath.Length); + resources.Add(NormalizeResourcePath(relativePath)); } - return true; + return resources; } /// @@ -336,22 +382,6 @@ private static bool HasSymlinkInPath(string fullPath, string normalizedDirectory return false; } - private static List ExtractResourcePaths(string content) - { - var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - var paths = new List(); - foreach (Match m in s_resourceLinkRegex.Matches(content)) - { - string path = NormalizeResourcePath(m.Groups[1].Value); - if (seen.Add(path)) - { - paths.Add(path); - } - } - - return paths; - } - /// /// Normalizes a relative resource path by trimming a leading ./ prefix and replacing /// backslashes with forward slashes so that ./refs/doc.md and refs/doc.md are @@ -372,6 +402,43 @@ private static string NormalizeResourcePath(string path) return path; } + /// + /// Replaces control characters in a file path with '?' to prevent log injection + /// via crafted filenames (e.g., filenames containing newlines on Linux). + /// + private static string SanitizePathForLog(string path) + { + char[]? chars = null; + for (int i = 0; i < path.Length; i++) + { + if (char.IsControl(path[i])) + { + chars ??= path.ToCharArray(); + chars[i] = '?'; + } + } + + return chars is null ? path : new string(chars); + } + + private static void ValidateExtensions(IEnumerable? extensions) + { + if (extensions is null) + { + return; + } + + foreach (string ext in extensions) + { + if (string.IsNullOrWhiteSpace(ext) || !ext.StartsWith(".", StringComparison.Ordinal)) + { +#pragma warning disable CA2208 // Instantiate argument exceptions correctly + throw new ArgumentException($"Each extension must start with '.'. Invalid value: '{ext}'", nameof(FileAgentSkillsProviderOptions.AllowedResourceExtensions)); +#pragma warning restore CA2208 // Instantiate argument exceptions correctly + } + } + } + [LoggerMessage(LogLevel.Information, "Discovered {Count} potential skills")] private static partial void LogSkillsDiscovered(ILogger logger, int count); @@ -390,18 +457,18 @@ private static string NormalizeResourcePath(string path) [LoggerMessage(LogLevel.Error, "SKILL.md at '{SkillFilePath}' has an invalid '{FieldName}' value: {Reason}")] private static partial void LogInvalidFieldValue(ILogger logger, string skillFilePath, string fieldName, string reason); - [LoggerMessage(LogLevel.Warning, "Excluding skill '{SkillName}': referenced resource '{ResourceName}' does not exist")] - private static partial void LogMissingResource(ILogger logger, string skillName, string resourceName); - - [LoggerMessage(LogLevel.Warning, "Excluding skill '{SkillName}': resource '{ResourceName}' references a path outside the skill directory")] - private static partial void LogResourcePathTraversal(ILogger logger, string skillName, string resourceName); + [LoggerMessage(LogLevel.Warning, "Skipping resource in skill '{SkillName}': '{ResourcePath}' references a path outside the skill directory")] + private static partial void LogResourcePathTraversal(ILogger logger, string skillName, string resourcePath); [LoggerMessage(LogLevel.Warning, "Duplicate skill name '{SkillName}': skill from '{NewPath}' skipped in favor of existing skill from '{ExistingPath}'")] private static partial void LogDuplicateSkillName(ILogger logger, string skillName, string newPath, string existingPath); - [LoggerMessage(LogLevel.Warning, "Excluding skill '{SkillName}': resource '{ResourceName}' is a symlink that resolves outside the skill directory")] - private static partial void LogResourceSymlinkEscape(ILogger logger, string skillName, string resourceName); + [LoggerMessage(LogLevel.Warning, "Skipping resource in skill '{SkillName}': '{ResourcePath}' is a symlink that resolves outside the skill directory")] + private static partial void LogResourceSymlinkEscape(ILogger logger, string skillName, string resourcePath); [LoggerMessage(LogLevel.Information, "Reading resource '{FileName}' from skill '{SkillName}'")] private static partial void LogResourceReading(ILogger logger, string fileName, string skillName); + + [LoggerMessage(LogLevel.Debug, "Skipping file '{FilePath}' in skill '{SkillName}': extension '{Extension}' is not in the allowed list")] + private static partial void LogResourceSkippedExtension(ILogger logger, string skillName, string filePath, string extension); } diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProvider.cs b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProvider.cs index ad1ef752ee..cd64cdc723 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProvider.cs @@ -88,7 +88,7 @@ public FileAgentSkillsProvider(IEnumerable skillPaths, FileAgentSkillsPr this._logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); - this._loader = new FileAgentSkillLoader(this._logger); + this._loader = new FileAgentSkillLoader(this._logger, options?.AllowedResourceExtensions); this._skills = this._loader.DiscoverAndLoadSkills(skillPaths); this._skillsInstructionPrompt = BuildSkillsInstructionPrompt(options, this._skills); diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProviderOptions.cs b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProviderOptions.cs index a47841c260..600c5b964c 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProviderOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillsProviderOptions.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using Microsoft.Shared.DiagnosticIds; @@ -17,4 +18,15 @@ public sealed class FileAgentSkillsProviderOptions /// When , a default template is used. /// public string? SkillsInstructionPrompt { get; set; } + + /// + /// Gets or sets the file extensions recognized as discoverable skill resources. + /// Each value must start with a '.' character (for example, .md), and + /// extension comparisons are performed in a case-insensitive manner. + /// Files in the skill directory (and its subdirectories) whose extension matches + /// one of these values will be automatically discovered as resources. + /// When , a default set of extensions is used + /// (.md, .json, .yaml, .yml, .csv, .xml, .txt). + /// + public IEnumerable? AllowedResourceExtensions { get; set; } } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs index c34eb6d7f2..0c79aabc99 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs @@ -169,16 +169,17 @@ public void DiscoverAndLoadSkills_DuplicateNames_KeepsFirstOnly() } [Fact] - public void DiscoverAndLoadSkills_WithValidResourceLinks_ExtractsResourceNames() + public void DiscoverAndLoadSkills_FilesWithMatchingExtensions_DiscoveredAsResources() { - // Arrange + // Arrange — create resource files in the skill directory string skillDir = Path.Combine(this._testRoot, "resource-skill"); string refsDir = Path.Combine(skillDir, "refs"); Directory.CreateDirectory(refsDir); File.WriteAllText(Path.Combine(refsDir, "FAQ.md"), "FAQ content"); + File.WriteAllText(Path.Combine(refsDir, "data.json"), "{}"); File.WriteAllText( Path.Combine(skillDir, "SKILL.md"), - "---\nname: resource-skill\ndescription: Has resources\n---\nSee [FAQ](refs/FAQ.md) for details."); + "---\nname: resource-skill\ndescription: Has resources\n---\nSee docs for details."); // Act var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); @@ -186,29 +187,176 @@ public void DiscoverAndLoadSkills_WithValidResourceLinks_ExtractsResourceNames() // Assert Assert.Single(skills); var skill = skills["resource-skill"]; + Assert.Equal(2, skill.ResourceNames.Count); + Assert.Contains(skill.ResourceNames, r => r.Equals("refs/FAQ.md", StringComparison.OrdinalIgnoreCase)); + Assert.Contains(skill.ResourceNames, r => r.Equals("refs/data.json", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void DiscoverAndLoadSkills_FilesWithNonMatchingExtensions_NotDiscovered() + { + // Arrange — create a file with an extension not in the default list + string skillDir = Path.Combine(this._testRoot, "ext-skill"); + Directory.CreateDirectory(skillDir); + File.WriteAllText(Path.Combine(skillDir, "image.png"), "fake image"); + File.WriteAllText(Path.Combine(skillDir, "data.json"), "{}"); + File.WriteAllText( + Path.Combine(skillDir, "SKILL.md"), + "---\nname: ext-skill\ndescription: Extension test\n---\nBody."); + + // Act + var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); + + // Assert + Assert.Single(skills); + var skill = skills["ext-skill"]; Assert.Single(skill.ResourceNames); - Assert.Equal("refs/FAQ.md", skill.ResourceNames[0]); + Assert.Equal("data.json", skill.ResourceNames[0]); } [Fact] - public void DiscoverAndLoadSkills_PathTraversal_ExcludesSkill() + public void DiscoverAndLoadSkills_SkillMdFile_NotIncludedAsResource() { - // Arrange — resource links outside the skill directory - string skillDir = Path.Combine(this._testRoot, "traversal-skill"); + // Arrange — the SKILL.md file itself should not be in the resource list + string skillDir = Path.Combine(this._testRoot, "selfref-skill"); Directory.CreateDirectory(skillDir); + File.WriteAllText(Path.Combine(skillDir, "notes.md"), "notes"); + File.WriteAllText( + Path.Combine(skillDir, "SKILL.md"), + "---\nname: selfref-skill\ndescription: Self ref test\n---\nBody."); + + // Act + var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); - // Create a file outside the skill dir that the traversal would resolve to - File.WriteAllText(Path.Combine(this._testRoot, "secret.txt"), "secret"); + // Assert + Assert.Single(skills); + var skill = skills["selfref-skill"]; + Assert.Single(skill.ResourceNames); + Assert.Equal("notes.md", skill.ResourceNames[0]); + } + [Fact] + public void DiscoverAndLoadSkills_NestedResourceFiles_Discovered() + { + // Arrange — resource files in nested subdirectories + string skillDir = Path.Combine(this._testRoot, "nested-res-skill"); + string deepDir = Path.Combine(skillDir, "level1", "level2"); + Directory.CreateDirectory(deepDir); + File.WriteAllText(Path.Combine(deepDir, "deep.md"), "deep content"); File.WriteAllText( Path.Combine(skillDir, "SKILL.md"), - "---\nname: traversal-skill\ndescription: Traversal attempt\n---\nSee [doc](../secret.txt)."); + "---\nname: nested-res-skill\ndescription: Nested resources\n---\nBody."); // Act var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); // Assert - Assert.Empty(skills); + Assert.Single(skills); + var skill = skills["nested-res-skill"]; + Assert.Single(skill.ResourceNames); + Assert.Contains(skill.ResourceNames, r => r.Equals("level1/level2/deep.md", StringComparison.OrdinalIgnoreCase)); + } + + private static readonly string[] s_customExtensions = new[] { ".custom" }; + private static readonly string[] s_validExtensions = new[] { ".md", ".json", ".custom" }; + private static readonly string[] s_mixedValidInvalidExtensions = new[] { ".md", "json" }; + + [Fact] + public void DiscoverAndLoadSkills_CustomResourceExtensions_UsedForDiscovery() + { + // Arrange — use a loader with custom extensions + var customLoader = new FileAgentSkillLoader(NullLogger.Instance, s_customExtensions); + string skillDir = Path.Combine(this._testRoot, "custom-ext-skill"); + Directory.CreateDirectory(skillDir); + File.WriteAllText(Path.Combine(skillDir, "data.custom"), "custom data"); + File.WriteAllText(Path.Combine(skillDir, "data.json"), "{}"); + File.WriteAllText( + Path.Combine(skillDir, "SKILL.md"), + "---\nname: custom-ext-skill\ndescription: Custom extensions\n---\nBody."); + + // Act + var skills = customLoader.DiscoverAndLoadSkills(new[] { this._testRoot }); + + // Assert — only .custom files should be discovered, not .json + Assert.Single(skills); + var skill = skills["custom-ext-skill"]; + Assert.Single(skill.ResourceNames); + Assert.Equal("data.custom", skill.ResourceNames[0]); + } + + [Theory] + [InlineData("txt")] + [InlineData("")] + [InlineData(" ")] + public void Constructor_InvalidExtension_ThrowsArgumentException(string badExtension) + { + // Arrange & Act & Assert + Assert.Throws(() => new FileAgentSkillLoader(NullLogger.Instance, new[] { badExtension })); + } + + [Fact] + public void Constructor_NullExtensions_UsesDefaults() + { + // Arrange & Act + var loader = new FileAgentSkillLoader(NullLogger.Instance, null); + string skillDir = this.CreateSkillDirectory("null-ext", "A skill", "Body."); + File.WriteAllText(Path.Combine(skillDir, "notes.md"), "notes"); + + // Assert — default extensions include .md + var skills = loader.DiscoverAndLoadSkills(new[] { this._testRoot }); + Assert.Single(skills["null-ext"].ResourceNames); + } + + [Fact] + public void Constructor_ValidExtensions_DoesNotThrow() + { + // Arrange & Act & Assert — should not throw + var loader = new FileAgentSkillLoader(NullLogger.Instance, s_validExtensions); + Assert.NotNull(loader); + } + + [Fact] + public void Constructor_MixOfValidAndInvalidExtensions_ThrowsArgumentException() + { + // Arrange & Act & Assert — one bad extension in the list should cause failure + Assert.Throws(() => new FileAgentSkillLoader(NullLogger.Instance, s_mixedValidInvalidExtensions)); + } + + [Fact] + public void DiscoverAndLoadSkills_ResourceInSkillRoot_Discovered() + { + // Arrange — resource file directly in the skill directory (not in a subdirectory) + string skillDir = Path.Combine(this._testRoot, "root-resource-skill"); + Directory.CreateDirectory(skillDir); + File.WriteAllText(Path.Combine(skillDir, "guide.md"), "guide content"); + File.WriteAllText(Path.Combine(skillDir, "config.json"), "{}"); + File.WriteAllText( + Path.Combine(skillDir, "SKILL.md"), + "---\nname: root-resource-skill\ndescription: Root resources\n---\nBody."); + + // Act + var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); + + // Assert — both root-level resource files should be discovered + Assert.Single(skills); + var skill = skills["root-resource-skill"]; + Assert.Equal(2, skill.ResourceNames.Count); + Assert.Contains(skill.ResourceNames, r => r.Equals("guide.md", StringComparison.OrdinalIgnoreCase)); + Assert.Contains(skill.ResourceNames, r => r.Equals("config.json", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void DiscoverAndLoadSkills_NoResourceFiles_ReturnsEmptyResourceNames() + { + // Arrange — skill with no resource files + _ = this.CreateSkillDirectory("no-resources", "A skill", "No resources here."); + + // Act + var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); + + // Assert + Assert.Single(skills); + Assert.Empty(skills["no-resources"].ResourceNames); } [Fact] @@ -252,8 +400,11 @@ public void DiscoverAndLoadSkills_NestedSkillDirectory_DiscoveredWithinDepthLimi [Fact] public async Task ReadSkillResourceAsync_ValidResource_ReturnsContentAsync() { - // Arrange - _ = this.CreateSkillDirectoryWithResource("read-skill", "A skill", "See [doc](refs/doc.md).", "refs/doc.md", "Document content here."); + // Arrange — create a skill with a resource file discovered from the directory + string skillDir = this.CreateSkillDirectory("read-skill", "A skill", "See docs for details."); + string refsDir = Path.Combine(skillDir, "refs"); + Directory.CreateDirectory(refsDir); + File.WriteAllText(Path.Combine(refsDir, "doc.md"), "Document content here."); var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); var skill = skills["read-skill"]; @@ -281,7 +432,10 @@ await Assert.ThrowsAsync( public async Task ReadSkillResourceAsync_PathTraversal_ThrowsInvalidOperationExceptionAsync() { // Arrange — skill with a legitimate resource, then try to read a traversal path at read time - _ = this.CreateSkillDirectoryWithResource("traverse-read", "A skill", "See [doc](refs/doc.md).", "refs/doc.md", "legit"); + string skillDir = this.CreateSkillDirectory("traverse-read", "A skill", "See docs."); + string refsDir = Path.Combine(skillDir, "refs"); + Directory.CreateDirectory(refsDir); + File.WriteAllText(Path.Combine(refsDir, "doc.md"), "legit"); var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); var skill = skills["traverse-read"]; @@ -333,75 +487,14 @@ public void DiscoverAndLoadSkills_DescriptionExceedsMaxLength_ExcludesSkill() Assert.Empty(skills); } - [Fact] - public void DiscoverAndLoadSkills_DuplicateResourceLinks_DeduplicatesResources() - { - // Arrange — body references the same resource twice - string skillDir = Path.Combine(this._testRoot, "dedup-skill"); - string refsDir = Path.Combine(skillDir, "refs"); - Directory.CreateDirectory(refsDir); - File.WriteAllText(Path.Combine(refsDir, "doc.md"), "content"); - File.WriteAllText( - Path.Combine(skillDir, "SKILL.md"), - "---\nname: dedup-skill\ndescription: Dedup test\n---\nSee [doc](refs/doc.md) and [again](refs/doc.md)."); - - // Act - var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); - - // Assert - Assert.Single(skills); - Assert.Single(skills["dedup-skill"].ResourceNames); - } - - [Fact] - public void DiscoverAndLoadSkills_DotSlashPrefix_NormalizesToBarePath() - { - // Arrange — body references a resource with ./ prefix - string skillDir = Path.Combine(this._testRoot, "dotslash-skill"); - string refsDir = Path.Combine(skillDir, "refs"); - Directory.CreateDirectory(refsDir); - File.WriteAllText(Path.Combine(refsDir, "doc.md"), "content"); - File.WriteAllText( - Path.Combine(skillDir, "SKILL.md"), - "---\nname: dotslash-skill\ndescription: Dot-slash test\n---\nSee [doc](./refs/doc.md)."); - - // Act - var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); - - // Assert - Assert.Single(skills); - var skill = skills["dotslash-skill"]; - Assert.Single(skill.ResourceNames); - Assert.Equal("refs/doc.md", skill.ResourceNames[0]); - } - - [Fact] - public void DiscoverAndLoadSkills_DotSlashAndBarePath_DeduplicatesResources() - { - // Arrange — body references the same resource with and without ./ prefix - string skillDir = Path.Combine(this._testRoot, "mixed-prefix-skill"); - string refsDir = Path.Combine(skillDir, "refs"); - Directory.CreateDirectory(refsDir); - File.WriteAllText(Path.Combine(refsDir, "doc.md"), "content"); - File.WriteAllText( - Path.Combine(skillDir, "SKILL.md"), - "---\nname: mixed-prefix-skill\ndescription: Mixed prefix test\n---\nSee [a](./refs/doc.md) and [b](refs/doc.md)."); - - // Act - var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); - - // Assert - Assert.Single(skills); - var skill = skills["mixed-prefix-skill"]; - Assert.Single(skill.ResourceNames); - Assert.Equal("refs/doc.md", skill.ResourceNames[0]); - } - [Fact] public async Task ReadSkillResourceAsync_DotSlashPrefix_MatchesNormalizedResourceAsync() { // Arrange — skill loaded with bare path, caller uses ./ prefix - _ = this.CreateSkillDirectoryWithResource("dotslash-read", "A skill", "See [doc](refs/doc.md).", "refs/doc.md", "Document content."); + string skillDir = this.CreateSkillDirectory("dotslash-read", "A skill", "See docs."); + string refsDir = Path.Combine(skillDir, "refs"); + Directory.CreateDirectory(refsDir); + File.WriteAllText(Path.Combine(refsDir, "doc.md"), "Document content."); var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); var skill = skills["dotslash-read"]; @@ -416,7 +509,10 @@ public async Task ReadSkillResourceAsync_DotSlashPrefix_MatchesNormalizedResourc public async Task ReadSkillResourceAsync_BackslashSeparator_MatchesNormalizedResourceAsync() { // Arrange — skill loaded with forward-slash path, caller uses backslashes - _ = this.CreateSkillDirectoryWithResource("backslash-read", "A skill", "See [doc](refs/doc.md).", "refs/doc.md", "Backslash content."); + string skillDir = this.CreateSkillDirectory("backslash-read", "A skill", "See docs."); + string refsDir = Path.Combine(skillDir, "refs"); + Directory.CreateDirectory(refsDir); + File.WriteAllText(Path.Combine(refsDir, "doc.md"), "Backslash content."); var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); var skill = skills["backslash-read"]; @@ -431,7 +527,10 @@ public async Task ReadSkillResourceAsync_BackslashSeparator_MatchesNormalizedRes public async Task ReadSkillResourceAsync_DotSlashWithBackslash_MatchesNormalizedResourceAsync() { // Arrange — skill loaded with forward-slash path, caller uses .\ prefix with backslashes - _ = this.CreateSkillDirectoryWithResource("mixed-sep-read", "A skill", "See [doc](refs/doc.md).", "refs/doc.md", "Mixed separator content."); + string skillDir = this.CreateSkillDirectory("mixed-sep-read", "A skill", "See docs."); + string refsDir = Path.Combine(skillDir, "refs"); + Directory.CreateDirectory(refsDir); + File.WriteAllText(Path.Combine(refsDir, "doc.md"), "Mixed separator content."); var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); var skill = skills["mixed-sep-read"]; @@ -443,14 +542,13 @@ public async Task ReadSkillResourceAsync_DotSlashWithBackslash_MatchesNormalized } #if NET - private static readonly string[] s_symlinkResource = ["refs/data.md"]; - [Fact] - public void DiscoverAndLoadSkills_SymlinkInPath_ExcludesSkill() + public void DiscoverAndLoadSkills_SymlinkInPath_SkipsSymlinkedResources() { // Arrange — a "refs" subdirectory is a symlink pointing outside the skill directory string skillDir = Path.Combine(this._testRoot, "symlink-escape-skill"); Directory.CreateDirectory(skillDir); + File.WriteAllText(Path.Combine(skillDir, "legit.md"), "legit content"); string outsideDir = Path.Combine(this._testRoot, "outside"); Directory.CreateDirectory(outsideDir); @@ -469,15 +567,20 @@ public void DiscoverAndLoadSkills_SymlinkInPath_ExcludesSkill() File.WriteAllText( Path.Combine(skillDir, "SKILL.md"), - "---\nname: symlink-escape-skill\ndescription: Symlinked directory escape\n---\nSee [doc](refs/secret.md)."); + "---\nname: symlink-escape-skill\ndescription: Symlinked directory escape\n---\nBody."); // Act var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); - // Assert — skill should be excluded because refs/ is a symlink (reparse point) - Assert.False(skills.ContainsKey("symlink-escape-skill")); + // Assert — skill should still load, but symlinked resources should be excluded + Assert.True(skills.ContainsKey("symlink-escape-skill")); + var skill = skills["symlink-escape-skill"]; + Assert.Single(skill.ResourceNames); + Assert.Equal("legit.md", skill.ResourceNames[0]); } + private static readonly string[] s_symlinkResource = ["refs/data.md"]; + [Fact] public async Task ReadSkillResourceAsync_SymlinkInPath_ThrowsInvalidOperationExceptionAsync() { @@ -549,13 +652,4 @@ private string CreateSkillDirectoryWithRawContent(string directoryName, string r File.WriteAllText(Path.Combine(skillDir, "SKILL.md"), rawContent); return skillDir; } - - private string CreateSkillDirectoryWithResource(string name, string description, string body, string resourceRelativePath, string resourceContent) - { - string skillDir = this.CreateSkillDirectory(name, description, body); - string resourcePath = Path.Combine(skillDir, resourceRelativePath); - Directory.CreateDirectory(Path.GetDirectoryName(resourcePath)!); - File.WriteAllText(resourcePath, resourceContent); - return skillDir; - } }