From 1a6ac0ff4447de38fbb50bff43cb9af982247631 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Fri, 6 Mar 2026 13:35:47 +0000 Subject: [PATCH 1/2] improve skill validation --- .../Skills/FileAgentSkillLoader.cs | 23 +++++++++++++--- .../AgentSkills/FileAgentSkillLoaderTests.cs | 26 ++++++++++++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs index 71a7124281..c25136bfb8 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs @@ -40,9 +40,10 @@ internal sealed partial class FileAgentSkillLoader // "description: \"A skill\"" → (description, A skill, _) private static readonly Regex s_yamlKeyValueRegex = new(@"^\s*(\w+)\s*:\s*(?:[""'](.+?)[""']|(.+?))\s*$", RegexOptions.Multiline | RegexOptions.Compiled, TimeSpan.FromSeconds(5)); - // Validates skill names: lowercase letters, numbers, and hyphens only; must not start or end with a hyphen. - // Examples: "my-skill" ✓, "skill123" ✓, "-bad" ✗, "bad-" ✗, "Bad" ✗ - private static readonly Regex s_validNameRegex = new(@"^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$", RegexOptions.Compiled); + // Validates skill names: lowercase letters, numbers, and hyphens only; + // must not start or end with a hyphen; must not contain consecutive hyphens. + // Examples: "my-skill" ✓, "skill123" ✓, "-bad" ✗, "bad-" ✗, "Bad" ✗, "my--skill" ✗ + private static readonly Regex s_validNameRegex = new("^[a-z0-9]([a-z0-9]*-[a-z0-9])*[a-z0-9]*$", RegexOptions.Compiled); private readonly ILogger _logger; private readonly HashSet _allowedResourceExtensions; @@ -244,7 +245,18 @@ private bool TryParseSkillDocument(string content, string skillFilePath, out Ski if (name.Length > MaxNameLength || !s_validNameRegex.IsMatch(name)) { - LogInvalidFieldValue(this._logger, skillFilePath, "name", $"Must be {MaxNameLength} characters or fewer, using only lowercase letters, numbers, and hyphens, and must not start or end with a hyphen."); + LogInvalidFieldValue(this._logger, skillFilePath, "name", $"Must be {MaxNameLength} characters or fewer, using only lowercase letters, numbers, and hyphens, and must not start or end with a hyphen or contain consecutive hyphens."); + return false; + } + + // skillFilePath is e.g. "/skills/my-skill/SKILL.md". + // GetDirectoryName strips the filename → "/skills/my-skill". + // GetFileName then extracts the last segment → "my-skill". + // This gives us the skill's parent directory name to validate against the frontmatter name. + string directoryName = Path.GetFileName(Path.GetDirectoryName(skillFilePath)) ?? string.Empty; + if (!string.Equals(name, directoryName, StringComparison.Ordinal)) + { + LogNameDirectoryMismatch(this._logger, skillFilePath, name, directoryName); return false; } @@ -457,6 +469,9 @@ private static void ValidateExtensions(IEnumerable? extensions) [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.Error, "SKILL.md at '{SkillFilePath}': skill name '{SkillName}' does not match parent directory name '{DirectoryName}'")] + private static partial void LogNameDirectoryMismatch(ILogger logger, string skillFilePath, string skillName, string directoryName); + [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); diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs index 0c79aabc99..6874670e65 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs @@ -122,6 +122,7 @@ public void DiscoverAndLoadSkills_MissingDescriptionField_ExcludesSkill() [InlineData("-leading-hyphen")] [InlineData("trailing-hyphen-")] [InlineData("has spaces")] + [InlineData("consecutive--hyphens")] public void DiscoverAndLoadSkills_InvalidName_ExcludesSkill(string invalidName) { // Arrange @@ -147,15 +148,19 @@ public void DiscoverAndLoadSkills_InvalidName_ExcludesSkill(string invalidName) public void DiscoverAndLoadSkills_DuplicateNames_KeepsFirstOnly() { // Arrange - string dir1 = Path.Combine(this._testRoot, "skill-a"); - string dir2 = Path.Combine(this._testRoot, "skill-b"); + string dir1 = Path.Combine(this._testRoot, "dupe"); + string dir2 = Path.Combine(this._testRoot, "subdir"); Directory.CreateDirectory(dir1); Directory.CreateDirectory(dir2); + + // Create a nested duplicate: subdir/dupe/SKILL.md + string nestedDir = Path.Combine(dir2, "dupe"); + Directory.CreateDirectory(nestedDir); File.WriteAllText( Path.Combine(dir1, "SKILL.md"), "---\nname: dupe\ndescription: First\n---\nFirst body."); File.WriteAllText( - Path.Combine(dir2, "SKILL.md"), + Path.Combine(nestedDir, "SKILL.md"), "---\nname: dupe\ndescription: Second\n---\nSecond body."); // Act @@ -168,6 +173,21 @@ public void DiscoverAndLoadSkills_DuplicateNames_KeepsFirstOnly() Assert.True(desc == "First" || desc == "Second", $"Unexpected description: {desc}"); } + [Fact] + public void DiscoverAndLoadSkills_NameMismatchesDirectory_ExcludesSkill() + { + // Arrange — directory name differs from the frontmatter name + _ = this.CreateSkillDirectoryWithRawContent( + "wrong-dir-name", + "---\nname: actual-skill-name\ndescription: A skill\n---\nBody."); + + // Act + var skills = this._loader.DiscoverAndLoadSkills(new[] { this._testRoot }); + + // Assert + Assert.Empty(skills); + } + [Fact] public void DiscoverAndLoadSkills_FilesWithMatchingExtensions_DiscoveredAsResources() { From 245d3c7ef91efdfe28f8fdf68c679aac7df9f6ff Mon Sep 17 00:00:00 2001 From: SergeyMenshykh <68852919+SergeyMenshykh@users.noreply.github.com> Date: Fri, 6 Mar 2026 14:49:35 +0000 Subject: [PATCH 2/2] address pr review comments --- .../src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs | 6 +++++- .../AgentSkills/FileAgentSkillLoaderTests.cs | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs index c25136bfb8..18fa87999a 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/FileAgentSkillLoader.cs @@ -256,7 +256,11 @@ private bool TryParseSkillDocument(string content, string skillFilePath, out Ski string directoryName = Path.GetFileName(Path.GetDirectoryName(skillFilePath)) ?? string.Empty; if (!string.Equals(name, directoryName, StringComparison.Ordinal)) { - LogNameDirectoryMismatch(this._logger, skillFilePath, name, directoryName); + if (this._logger.IsEnabled(LogLevel.Error)) + { + LogNameDirectoryMismatch(this._logger, SanitizePathForLog(skillFilePath), name, SanitizePathForLog(directoryName)); + } + return false; } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs index 6874670e65..6134b04feb 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/FileAgentSkillLoaderTests.cs @@ -126,7 +126,7 @@ public void DiscoverAndLoadSkills_MissingDescriptionField_ExcludesSkill() public void DiscoverAndLoadSkills_InvalidName_ExcludesSkill(string invalidName) { // Arrange - string skillDir = Path.Combine(this._testRoot, "invalid-name-test"); + string skillDir = Path.Combine(this._testRoot, invalidName); if (Directory.Exists(skillDir)) { Directory.Delete(skillDir, recursive: true);