From ba32a88972e73482ddc07038fab4ddd4316bb3e8 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 16:43:56 +0100 Subject: [PATCH 1/7] NET: Support archive-type skills in AgentMcpSkillsSource Add archive-type skill discovery to the MCP skills source. Index entries are dispatched to per-type loaders (skill-md and archive) via a new IMcpSkillEntryLoader strategy. The archive loader downloads, safely unpacks, and serves packaged skills through an internal file skills source, while ensuring MCP-delivered scripts are never executed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSource.cs | 179 +++-- .../Skills/AgentMcpSkillsSourceOptions.cs | 97 +++ ...AgentSkillsProviderBuilderMcpExtensions.cs | 5 +- .../Loaders/AgentMcpSkillArchiveExtractor.cs | 286 ++++++++ .../Skills/Loaders/ArchiveEntryLoader.cs | 403 ++++++++++ .../Skills/Loaders/ArchiveFormat.cs | 21 + .../Skills/Loaders/IMcpSkillEntryLoader.cs | 35 + .../Skills/Loaders/SkillMdEntryLoader.cs | 80 ++ .../Skills/AgentSkillsProviderBuilder.cs | 14 + .../Skills/File/AgentFileSkillsSource.cs | 2 +- .../AgentMcpSkillsSourceArchiveTests.cs | 693 ++++++++++++++++++ .../Skills/AgentMcpSkillsSourceTests.cs | 35 +- 12 files changed, 1772 insertions(+), 78 deletions(-) create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs create mode 100644 dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs create mode 100644 dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index 2a43814b604..e4fc54377be 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -2,7 +2,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Text.Json; using System.Threading; @@ -22,19 +21,22 @@ namespace Microsoft.Agents.AI; /// /// /// Discovery follows the SEP-2640 recommended approach: the source reads the well-known -/// skill://index.json resource and constructs one per -/// skill-md entry directly from the entry's name, description, and url fields. -/// The referenced SKILL.md resource is not read during discovery; hosts fetch its body on -/// demand via resources/read against the URI exposed on the resulting skill. +/// skill://index.json resource and constructs one per index entry. /// /// -/// Only index entries of type skill-md are supported at the moment; entries of any other -/// type are skipped. +/// Index entries are dispatched to an by their type: +/// +/// skill-md - handled by ; the skill's +/// SKILL.md and sibling resources are fetched on demand from the MCP server. +/// archive - handled by ; the entry's +/// url points to a single archive resource whose content unpacks into the skill's +/// namespace. +/// +/// Entries whose type has no registered loader (e.g. mcp-resource-template) are skipped. /// /// -/// If skill://index.json is absent, unreadable, empty, or fails to parse, this source -/// returns an empty list. Discovered skills serve their referenced resources on demand via -/// ; they do not enumerate sibling files up front. +/// If skill://index.json is absent, unreadable, empty, or fails to parse, this source returns an +/// empty list. /// /// internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource @@ -44,42 +46,141 @@ internal sealed partial class AgentMcpSkillsSource : AgentSkillsSource /// private const string IndexUri = "skill://index.json"; - private const string SkillMdEntryType = "skill-md"; - private readonly McpClient _client; private readonly ILogger _logger; + private readonly Dictionary _loaders; + private readonly TimeSpan? _refreshInterval; + + private IList? _cachedSkills; + private DateTime _lastRefreshedUtc; + private Task>? _refreshTask; /// /// Initializes a new instance of the class. /// /// An MCP client connected to a server that exposes Agent Skills resources. + /// Optional options that control archive-distributed skill handling. /// Optional logger factory. - public AgentMcpSkillsSource(McpClient client, ILoggerFactory? loggerFactory = null) + public AgentMcpSkillsSource(McpClient client, AgentMcpSkillsSourceOptions? options = null, ILoggerFactory? loggerFactory = null) { this._client = Throw.IfNull(client); - this._logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); + loggerFactory ??= NullLoggerFactory.Instance; + this._logger = loggerFactory.CreateLogger(); + + IMcpSkillEntryLoader[] loaders = + [ + new SkillMdEntryLoader(this._client, loggerFactory), + new ArchiveEntryLoader(this._client, options, loggerFactory), + ]; + + this._loaders = loaders.ToDictionary(l => l.EntryType, StringComparer.OrdinalIgnoreCase); + this._refreshInterval = options?.RefreshInterval; } /// public override async Task> GetSkillsAsync(CancellationToken cancellationToken = default) + { + if (this.TryGetCachedSkills() is { } cached) + { + return cached; + } + + // Use CAS to ensure only one concurrent refresh runs; other callers await the same task. + var tcs = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + + if (Interlocked.CompareExchange(ref this._refreshTask, tcs.Task, null) is { } existing) + { + return await existing.ConfigureAwait(false); + } + + try + { + var skills = await this.GetCoreSkillsAsync(cancellationToken).ConfigureAwait(false); + + this.UpdateCache(skills); + + tcs.SetResult(skills); + + return skills; + } + catch (Exception ex) + { + tcs.TrySetException(ex); + throw; + } + finally + { + this._refreshTask = null; + } + } + + /// + /// Returns the cached skill list if caching is enabled and the cache is still fresh; + /// otherwise returns . + /// + private IList? TryGetCachedSkills() + { + if (this._refreshInterval is null || this._cachedSkills is null) + { + return null; + } + + TimeSpan cacheAge = DateTime.UtcNow - this._lastRefreshedUtc; + + if (cacheAge >= this._refreshInterval.Value) + { + return null; + } + + return this._cachedSkills; + } + + /// + /// Stores the skill list and records the refresh timestamp for cache freshness checks. + /// + private void UpdateCache(IList skills) + { + this._cachedSkills = skills; + this._lastRefreshedUtc = DateTime.UtcNow; + } + + /// + /// Reads the skill index from the MCP server, dispatches entries to registered loaders, and + /// returns the aggregated skill list. + /// + private async Task> GetCoreSkillsAsync(CancellationToken cancellationToken) { McpSkillIndex? index = await this.TryReadIndexAsync(cancellationToken).ConfigureAwait(false); - var skills = new List(); + // Group entries by type and set aside those a registered loader can handle; entries of any + // other type are unsupported and logged. + var entriesByType = new Dictionary>(StringComparer.OrdinalIgnoreCase); - foreach (var entry in index?.Skills ?? []) + foreach (var group in (index?.Skills ?? []).GroupBy(e => e.Type ?? string.Empty, StringComparer.OrdinalIgnoreCase)) { - if (this.TryCreateSkill(entry, out AgentMcpSkill? skill, out string skipReason)) + if (this._loaders.ContainsKey(group.Key)) { - skills.Add(skill); - LogSkillLoaded(this._logger, skill.Frontmatter.Name); + entriesByType[group.Key] = group.ToList(); } else { - LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", skipReason); + foreach (var entry in group) + { + LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", $"unsupported type '{entry.Type ?? "(none)"}'"); + } } } + // Invoke every registered loader, even when the server advertises no entries of its type, so + // each type's lifecycle still runs (e.g. the archive loader prunes leftover directories). + var skills = new List(); + + foreach (var loader in this._loaders.Values) + { + var entries = entriesByType.TryGetValue(loader.EntryType, out List? matched) ? matched : []; + skills.AddRange(await loader.LoadAsync(entries, cancellationToken).ConfigureAwait(false)); + } + LogSkillsLoadedTotal(this._logger, skills.Count); return skills; @@ -124,44 +225,6 @@ public override async Task> GetSkillsAsync(CancellationToken c } } - private bool TryCreateSkill( - McpSkillIndexEntry entry, - [NotNullWhen(true)] out AgentMcpSkill? skill, - out string skipReason) - { - skill = null; - - if (!string.Equals(entry.Type, SkillMdEntryType, StringComparison.Ordinal)) - { - skipReason = $"unsupported type '{entry.Type ?? "(none)"}'"; - return false; - } - - if (string.IsNullOrWhiteSpace(entry.Url)) - { - skipReason = "missing required 'url' field"; - return false; - } - - AgentSkillFrontmatter frontmatter; - try - { - frontmatter = new AgentSkillFrontmatter(entry.Name!, entry.Description!); - } - catch (ArgumentException ex) - { - skipReason = $"invalid metadata: {ex.Message}"; - return false; - } - - skill = new AgentMcpSkill(frontmatter, entry.Url!, this._client); - skipReason = string.Empty; - return true; - } - - [LoggerMessage(LogLevel.Information, "Loaded MCP skill: {SkillName}")] - private static partial void LogSkillLoaded(ILogger logger, string skillName); - [LoggerMessage(LogLevel.Information, "Successfully loaded {Count} skills from MCP server")] private static partial void LogSkillsLoadedTotal(ILogger logger, int count); diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs new file mode 100644 index 00000000000..8b3fc9060ea --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Microsoft.Shared.DiagnosticIds; + +namespace Microsoft.Agents.AI; + +/// +/// Configuration options for . +/// +[Experimental(DiagnosticIds.Experiments.AgentsAIExperiments)] +public sealed class AgentMcpSkillsSourceOptions +{ + /// + /// Gets or sets the base directory that archive-type skills are extracted to and served from. + /// + /// + /// Archives are extracted beneath this directory as {ArchiveSkillsDirectory}/{skill-name}/. + /// When , the source extracts to a per-instance unique location of + /// {currentDirectory}/{guid}/{skill-name}/, where the GUID is generated once per + /// instance so that multiple sources never overwrite one + /// another. Set this to a fixed value to get a predictable, reusable extraction location. + /// When set, each source must use its own unique directory: the source treats the directory as + /// exclusively its own and, on every discovery, prunes any sub-directory that the MCP server no + /// longer advertises. Pointing two sources at the same directory would therefore cause them to + /// delete each other's extracted skills. + /// + public string? ArchiveSkillsDirectory { get; set; } + + /// + /// Gets or sets the allowed file extensions for resources discovered in extracted archive-type skills. + /// + /// + /// When , defaults to .md, .json, .yaml, .yml, + /// .csv, .xml, and .txt. + /// + public IEnumerable? ArchiveResourceExtensions { get; set; } + + /// + /// Gets or sets the maximum depth to search for resource files within each extracted archive-type + /// skill directory. A value of 1 searches only the skill root directory. A value of 2 + /// searches the root and one level of subdirectories. + /// + /// + /// When , the source uses the default depth of 2. + /// + public int? ArchiveResourceSearchDepth { get; set; } + + /// + /// Gets or sets the maximum number of files that may be extracted from a single archive-type skill. + /// + /// + /// Guards against excessive-file-count denial-of-service archives. When , the + /// source uses a default of 20, sized for a typical well-formed skill (a handful of files). + /// Raise this for archive-type skills that legitimately bundle many files. An archive that exceeds + /// the limit is skipped. + /// + public int? ArchiveMaxFileCount { get; set; } + + /// + /// Gets or sets the maximum size, in bytes, of a downloaded archive-type skill resource. + /// + /// + /// Guards against archive resources that are too large to materialize safely. When + /// , the source uses a default of 1 MB, sized for a typical + /// well-formed skill archive. Raise this for archive-type skills that legitimately require + /// larger archive payloads. An archive that exceeds the limit is skipped. + /// + public long? ArchiveMaxSizeBytes { get; set; } + + /// + /// Gets or sets the maximum total uncompressed size, in bytes, of all files extracted from a single + /// archive-type skill. + /// + /// + /// Guards against decompression-bomb archives. When , the source uses a default + /// of 1 MB, sized for a typical well-formed skill (well under ~1 MB). Raise this for + /// archive-type skills that legitimately bundle larger content. An archive that exceeds the limit is + /// skipped. + /// + public long? ArchiveMaxUncompressedSizeBytes { get; set; } + + /// + /// Gets or sets the interval at which cached skills are considered fresh. When a caller invokes + /// and the cached result is younger than this + /// interval, the cached list is returned without contacting the MCP server. + /// + /// + /// When (the default), caching is disabled and every call fetches from + /// the MCP server. Set to a positive to enable caching. Values of + /// or negative durations effectively disable caching because the + /// cache age will always be greater than or equal to the interval. + /// + public TimeSpan? RefreshInterval { get; set; } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs index 9b90f36511f..a7af8fdcec3 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs @@ -15,12 +15,13 @@ public static class AgentSkillsProviderBuilderMcpExtensions /// /// The builder to extend. /// An MCP client connected to a server exposing Agent Skills resources. + /// Optional options that control archive-distributed skill handling. /// The builder instance for chaining. - public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client) + public static AgentSkillsProviderBuilder UseMcpSkills(this AgentSkillsProviderBuilder builder, McpClient client, AgentMcpSkillsSourceOptions? options = null) { _ = Throw.IfNull(builder); _ = Throw.IfNull(client); - return builder.UseSource(new AgentMcpSkillsSource(client)); + return builder.UseSource(loggerFactory => new AgentMcpSkillsSource(client, options, loggerFactory)); } } diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs new file mode 100644 index 00000000000..15e37926a77 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs @@ -0,0 +1,286 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Buffers; +using System.Formats.Tar; +using System.IO; +using System.IO.Compression; + +namespace Microsoft.Agents.AI; + +/// +/// Unpacks skill archives downloaded from an MCP server into a local directory. +/// +/// +/// Supports ZIP, TAR, and gzip-compressed TAR payloads. Extraction is guarded against path-traversal +/// ("zip-slip") attacks: every entry must resolve to a path beneath the target directory. Non-regular +/// TAR entries (symbolic links, hard links, device nodes, etc.) are skipped so an archive cannot +/// create links that escape the target directory. Extraction is also bounded by a maximum file count +/// and total uncompressed size to mitigate decompression-bomb attacks. +/// +internal static class AgentMcpSkillArchiveExtractor +{ + /// + /// The default maximum number of files that may be extracted from a single archive, sized for a + /// typical well-formed skill (a handful of files) with headroom over the observed average. + /// + internal const int DefaultMaxFileCount = 20; + + /// + /// The default maximum total uncompressed size, in bytes, of all files extracted from a single + /// archive, sized for a typical well-formed skill (well under ~1 MB). + /// + internal const long DefaultMaxUncompressedSizeBytes = 1L * 1024 * 1024; + + private const int CopyBufferSize = 81920; + + /// + /// Determines the archive format from the advertised media type, the source URL, and the leading + /// bytes of the payload (magic-number sniffing takes precedence as it is the most reliable). + /// + /// The archive bytes. + /// The advertised MIME type, if any. + /// The resource URL the archive was read from, if any. + /// The detected , or . + internal static ArchiveFormat DetectFormat(byte[] bytes, string? mediaType, string? url) + { + // Magic-number sniffing is the most reliable signal. + if (bytes.Length >= 2 && bytes[0] == 0x1F && bytes[1] == 0x8B) + { + return ArchiveFormat.TarGz; + } + + if (bytes.Length >= 4 && bytes[0] == 0x50 && bytes[1] == 0x4B && + (bytes[2] == 0x03 || bytes[2] == 0x05 || bytes[2] == 0x07)) + { + return ArchiveFormat.Zip; + } + + string media = mediaType?.Trim() ?? string.Empty; + if (string.Equals(media, "application/zip", StringComparison.OrdinalIgnoreCase) || + string.Equals(media, "application/x-zip-compressed", StringComparison.OrdinalIgnoreCase)) + { + return ArchiveFormat.Zip; + } + + if (string.Equals(media, "application/gzip", StringComparison.OrdinalIgnoreCase) || + string.Equals(media, "application/x-gzip", StringComparison.OrdinalIgnoreCase) || + string.Equals(media, "application/x-compressed-tar", StringComparison.OrdinalIgnoreCase)) + { + return ArchiveFormat.TarGz; + } + + if (string.Equals(media, "application/x-tar", StringComparison.OrdinalIgnoreCase) || + string.Equals(media, "application/tar", StringComparison.OrdinalIgnoreCase)) + { + return ArchiveFormat.Tar; + } + + string u = url ?? string.Empty; + if (u.EndsWith(".zip", StringComparison.OrdinalIgnoreCase)) + { + return ArchiveFormat.Zip; + } + + if (u.EndsWith(".tar.gz", StringComparison.OrdinalIgnoreCase) || u.EndsWith(".tgz", StringComparison.OrdinalIgnoreCase)) + { + return ArchiveFormat.TarGz; + } + + if (u.EndsWith(".tar", StringComparison.OrdinalIgnoreCase)) + { + return ArchiveFormat.Tar; + } + + return ArchiveFormat.Unknown; + } + + /// + /// Extracts the supplied archive into . + /// + /// The archive bytes. + /// The archive container format. + /// The directory the archive is unpacked into. Created if missing. + /// The maximum number of files that may be extracted from the archive. + /// The maximum total uncompressed size, in bytes, of all extracted files. + /// The format is . + /// The archive exceeds one of the supplied limits. + internal static void Extract( + byte[] bytes, + ArchiveFormat format, + string targetDirectory, + int? maxFileCount = null, + long? maxUncompressedSizeBytes = null) + { + maxFileCount ??= DefaultMaxFileCount; + maxUncompressedSizeBytes ??= DefaultMaxUncompressedSizeBytes; + + Directory.CreateDirectory(targetDirectory); + string fullTarget = Path.GetFullPath(targetDirectory); + + using var source = new MemoryStream(bytes, writable: false); + + switch (format) + { + case ArchiveFormat.Zip: + ExtractZip(source, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value); + break; + case ArchiveFormat.Tar: + ExtractTar(source, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value); + break; + case ArchiveFormat.TarGz: + { + using var gzip = new GZipStream(source, CompressionMode.Decompress); + ExtractTar(gzip, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value); + break; + } + default: + throw new NotSupportedException($"Unsupported skill archive format '{format}'."); + } + } + + private static void ExtractZip(Stream source, string fullTarget, int maxFileCount, long maxUncompressedSizeBytes) + { + using var archive = new ZipArchive(source, ZipArchiveMode.Read, leaveOpen: true); + + long remainingBytes = maxUncompressedSizeBytes; + int fileCount = 0; + + foreach (ZipArchiveEntry entry in archive.Entries) + { + // Directory entries have an empty Name. + if (entry.Name.Length == 0) + { + continue; + } + + if (++fileCount > maxFileCount) + { + throw new InvalidDataException($"Skill archive exceeds the maximum allowed file count ({maxFileCount})."); + } + + string? destination = ResolveDestination(fullTarget, entry.FullName); + if (destination is null) + { + continue; + } + + Directory.CreateDirectory(Path.GetDirectoryName(destination)!); + + using Stream entryStream = entry.Open(); + using FileStream output = File.Create(destination); + CopyWithLimit(entryStream, output, ref remainingBytes); + } + } + + private static void ExtractTar(Stream source, string fullTarget, int maxFileCount, long maxUncompressedSizeBytes) + { + using var reader = new TarReader(source, leaveOpen: true); + + long remainingBytes = maxUncompressedSizeBytes; + int fileCount = 0; + + while (reader.GetNextEntry() is { } entry) + { + // Only regular files are materialized. Skipping links/devices avoids both unsupported + // entry types and link-based escapes outside the target directory. + if (entry.EntryType is not (TarEntryType.RegularFile or TarEntryType.V7RegularFile)) + { + continue; + } + + if (++fileCount > maxFileCount) + { + throw new InvalidDataException($"Skill archive exceeds the maximum allowed file count ({maxFileCount})."); + } + + string? destination = ResolveDestination(fullTarget, entry.Name); + if (destination is null || entry.DataStream is null) + { + continue; + } + + Directory.CreateDirectory(Path.GetDirectoryName(destination)!); + using FileStream output = File.Create(destination); + CopyWithLimit(entry.DataStream, output, ref remainingBytes); + } + } + + /// + /// Copies to while decrementing a shared + /// uncompressed-byte budget, throwing once it is exhausted. This is the authoritative defense against + /// decompression bombs because it counts bytes actually produced by the decompressor rather than + /// trusting archive metadata which may be inaccurate. Peak memory is bounded to a single buffer. + /// + /// The remaining byte budget is exceeded. + private static void CopyWithLimit(Stream source, Stream destination, ref long remainingBytes) + { + byte[] buffer = ArrayPool.Shared.Rent(CopyBufferSize); + try + { + int read; + while ((read = source.Read(buffer, 0, buffer.Length)) > 0) + { + remainingBytes -= read; + if (remainingBytes < 0) + { + throw new InvalidDataException("Skill archive exceeds the maximum allowed uncompressed size."); + } + + destination.Write(buffer, 0, read); + } + } + finally + { + ArrayPool.Shared.Return(buffer); + } + } + + /// + /// Resolves an archive entry path to an absolute destination beneath , + /// or when the entry would escape the target directory (zip-slip). + /// + private static string? ResolveDestination(string fullTarget, string entryPath) + { + if (string.IsNullOrWhiteSpace(entryPath)) + { + return null; + } + + // Normalize separators and reject absolute / rooted entry paths outright. + string normalized = entryPath.Replace('\\', '/').TrimStart('/'); + if (normalized.Length == 0 || Path.IsPathRooted(normalized)) + { + return null; + } + + string destination = Path.GetFullPath(Path.Combine(fullTarget, normalized)); + if (!IsPathContainedIn(fullTarget, destination)) + { + return null; + } + + return destination; + } + + /// + /// Returns when resolves to a location + /// beneath . + /// + internal static bool IsPathContainedIn(string parentDirectory, string candidatePath) + { + string fullParent = Path.GetFullPath(parentDirectory); + string fullCandidate = Path.GetFullPath(candidatePath); + + // Append a trailing separator so the containment check doesn't false-match sibling + // directories. e.g. "/skills/myskill" matches "/skills/myskill-evil/", but + // "/skills/myskill/" does not. + string prefix = fullParent.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal) + ? fullParent + : fullParent + Path.DirectorySeparatorChar; + + return fullCandidate.StartsWith( + prefix, + OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs new file mode 100644 index 00000000000..b483eefa4f2 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs @@ -0,0 +1,403 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using ModelContextProtocol; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; + +namespace Microsoft.Agents.AI; + +/// +/// Loads archive index entries: each entry's url points to a single archive resource +/// (application/zip, application/x-tar, or gzip-compressed TAR) whose content unpacks +/// into the skill's namespace. Archives are downloaded, extracted to a local directory, and the +/// resulting files are discovered via an internal that this +/// loader proxies to. +/// +/// +/// Because MCP-delivered skills are treated strictly as instructor-format text, scripts bundled +/// inside an archive are surfaced as readable resources only; they are never discovered as +/// executable scripts. +/// +internal sealed partial class ArchiveEntryLoader : IMcpSkillEntryLoader +{ + /// + /// The default maximum size, in bytes, of a downloaded archive resource. + /// + internal const long DefaultMaxArchiveSizeBytes = 1L * 1024 * 1024; + + private readonly McpClient _client; + private readonly AgentMcpSkillsSourceOptions? _options; + private readonly ILoggerFactory _loggerFactory; + private readonly ILogger _logger; + private string? _archiveSkillsDirectory; + + public ArchiveEntryLoader(McpClient client, AgentMcpSkillsSourceOptions? options, ILoggerFactory loggerFactory) + { + this._client = client; + this._options = options; + this._loggerFactory = loggerFactory; + this._logger = loggerFactory.CreateLogger(); + } + + /// + public string EntryType => "archive"; + + /// + public async Task> LoadAsync(IReadOnlyList entries, CancellationToken cancellationToken) + { + // Filter out entries that are missing required fields or have invalid names. + var archiveEntries = this.FilterValidEntries(entries); + + // Determine the target directory from prior state or caller-supplied options. + var archiveSkillsDirectory = this._archiveSkillsDirectory ?? this._options?.ArchiveSkillsDirectory; + + // Reconcile on-disk state with the current set of advertised skills. + this.ReconcileArchiveSkillDirectories(archiveSkillsDirectory, archiveEntries); + + if (archiveEntries.Count == 0) + { + return []; + } + + // Resolve or generate the skills directory and ensure it exists on disk. + this._archiveSkillsDirectory = archiveSkillsDirectory ?? Path.Combine(Directory.GetCurrentDirectory(), Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(this._archiveSkillsDirectory); + + // Download and extract each archive entry into its own subdirectory. + var skillDirectories = new List(archiveEntries.Count); + + foreach (var entry in archiveEntries) + { + skillDirectories.AddRange(await this.TryDownloadAndExtractSkillAsync(entry, this._archiveSkillsDirectory, cancellationToken).ConfigureAwait(false)); + } + + // Delegate discovery of extracted content to a file-based skills source. + AgentFileSkillsSource fileSource = this.CreateFileSkillsSource(skillDirectories); + + return await fileSource.GetSkillsAsync(cancellationToken).ConfigureAwait(false); + } + + /// + /// Reconciles the on-disk archive skill directories with the current set of advertised entries. + /// When no entries are advertised, all existing directories are removed. Otherwise, only stale + /// directories (not in the current advertised set) are pruned. + /// + private void ReconcileArchiveSkillDirectories(string? archiveSkillsDirectory, List archiveEntries) + { + // Nothing to reconcile if the directory hasn't been established yet. + if (string.IsNullOrEmpty(archiveSkillsDirectory) || !Directory.Exists(archiveSkillsDirectory)) + { + return; + } + + // Server advertises no archive skills - remove all previously extracted directories. + if (archiveEntries.Count == 0) + { + this.ClearSkillsDirectory(archiveSkillsDirectory); + return; + } + + // Compare the advertised set against what's on disk; prune any stale leftovers. + var advertisedSkillNames = archiveEntries + .Select(e => e.Name!) + .ToHashSet(OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); + + foreach (string directory in Directory.EnumerateDirectories(archiveSkillsDirectory)) + { + string name = Path.GetFileName(directory); + if (advertisedSkillNames.Contains(name)) + { + continue; + } + + try + { + Directory.Delete(directory, recursive: true); + LogArchiveSkillPruned(this._logger, name, SanitizePathForLog(directory)); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + LogArchiveSkillPruneFailed(this._logger, name, ex); + } + } + } + + /// + /// Removes all subdirectories of the specified directory. Failures on individual directories + /// are swallowed so that a single locked directory does not prevent cleanup of the others. + /// + private void ClearSkillsDirectory(string archiveSkillsDirectory) + { + foreach (var dir in Directory.GetDirectories(archiveSkillsDirectory)) + { + try + { + Directory.Delete(dir, true); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + LogArchiveSkillPruneFailed(this._logger, Path.GetFileName(dir), ex); + } + } + } + + /// + /// Creates the internal that discovers extracted archive + /// content. Scripts are never treated as executable; the allowed script extensions list is left + /// empty so that script files are not discovered as runnable scripts. + /// + private AgentFileSkillsSource CreateFileSkillsSource(IReadOnlyList archiveSkillDirectories) + { + var fileOptions = new AgentFileSkillsSourceOptions + { + AllowedScriptExtensions = [], + AllowedResourceExtensions = this._options?.ArchiveResourceExtensions, + SearchDepth = this._options?.ArchiveResourceSearchDepth, + }; + + return new AgentFileSkillsSource( + archiveSkillDirectories, + scriptRunner: null, + options: fileOptions, + loggerFactory: this._loggerFactory); + } + + /// + /// Downloads the archive for a single skill entry, detects its format, and extracts it into a + /// subdirectory of . Entries that cannot be materialized + /// (path escape, download failure, unsupported format, or extraction error) are skipped and logged; + /// partial content is cleaned up on failure. + /// + private async Task> TryDownloadAndExtractSkillAsync(McpSkillIndexEntry entry, string archiveSkillsDirectory, CancellationToken cancellationToken) + { + string skillDirectory = Path.Combine(archiveSkillsDirectory, entry.Name!); + + // Defense-in-depth: the skill directory is derived from the server-supplied entry name. + // Even though invalid names are filtered upstream, verify the resolved path cannot escape + // the archive skills directory before any file system operation is performed. + if (!AgentMcpSkillArchiveExtractor.IsPathContainedIn(archiveSkillsDirectory, skillDirectory)) + { + LogIndexEntrySkipped(this._logger, entry.Name!, "resolved skill directory escapes the archive skills directory"); + return []; + } + + // Clean the target directory so that content is always freshly extracted. + if (!this.TryDeleteSkillDirectory(entry.Name!, skillDirectory)) + { + return []; + } + + (byte[]? bytes, string? mimeType) = await this.DownloadSkillBytesAsync(entry, cancellationToken).ConfigureAwait(false); + if (bytes is null) + { + return []; + } + + var format = AgentMcpSkillArchiveExtractor.DetectFormat(bytes, mimeType, entry.Url); + if (format == ArchiveFormat.Unknown) + { + LogIndexEntrySkipped(this._logger, entry.Name!, $"unsupported archive media type '{mimeType ?? "(none)"}'"); + return []; + } + + try + { + AgentMcpSkillArchiveExtractor.Extract( + bytes, + format, + skillDirectory, + this._options?.ArchiveMaxFileCount, + this._options?.ArchiveMaxUncompressedSizeBytes); + } + catch (Exception ex) + { + LogArchiveExtractFailed(this._logger, entry.Name!, ex); + + // Remove any partially-extracted content so it is not later treated as a valid, + // already-materialized skill by the reuse check above. + _ = this.TryDeleteSkillDirectory(entry.Name!, skillDirectory); + return []; + } + + LogArchiveExtracted(this._logger, entry.Name!, SanitizePathForLog(skillDirectory)); + + return [skillDirectory]; + } + + /// + /// Downloads and decodes the binary content of a skill's archive resource. Returns + /// bytes when the resource cannot be read, contains no binary content, or is empty. + /// + private async Task<(byte[]? Bytes, string? MimeType)> DownloadSkillBytesAsync(McpSkillIndexEntry entry, CancellationToken cancellationToken) + { + ReadResourceResult result; + + try + { +#pragma warning disable CA2234 // Pass system uri objects instead of strings + result = await this._client.ReadResourceAsync(entry.Url!, cancellationToken: cancellationToken).ConfigureAwait(false); +#pragma warning restore CA2234 // Pass system uri objects instead of strings + } + catch (McpException ex) + { + LogArchiveReadFailed(this._logger, entry.Name!, ex); + return (null, null); + } + + BlobResourceContents? blobContent = result.Contents.OfType().FirstOrDefault(); + if (blobContent is null) + { + LogIndexEntrySkipped(this._logger, entry.Name!, "archive resource returned no binary content"); + return (null, null); + } + + long maxArchiveSizeBytes = this._options?.ArchiveMaxSizeBytes ?? DefaultMaxArchiveSizeBytes; + + byte[] bytes; + try + { + if (blobContent.DecodedData.Length > maxArchiveSizeBytes) + { + LogIndexEntrySkipped(this._logger, entry.Name!, $"archive resource exceeds the maximum allowed size ({maxArchiveSizeBytes} bytes)"); + return (null, null); + } + + bytes = blobContent.DecodedData.ToArray(); + } + catch (FormatException ex) + { + LogArchiveDecodeFailed(this._logger, entry.Name!, ex); + return (null, null); + } + + if (bytes.Length == 0) + { + LogIndexEntrySkipped(this._logger, entry.Name!, "archive resource returned empty content"); + return (null, null); + } + + return (bytes, blobContent.MimeType); + } + + /// + /// Filters archive entries to those that are valid for materialization. Entries with missing or + /// invalid names (not usable as directory names) or missing URLs are excluded and logged. + /// + private List FilterValidEntries(IReadOnlyList entries) + { + var invalidFileNameChars = Path.GetInvalidFileNameChars(); + + var valid = new List(entries.Count); + + foreach (var entry in entries) + { + if (string.IsNullOrWhiteSpace(entry.Name)) + { + LogIndexEntrySkipped(this._logger, "(unnamed)", "archive entry missing required 'name' field"); + continue; + } + + if (!IsValidDirectoryName(entry.Name!, invalidFileNameChars)) + { + LogIndexEntrySkipped(this._logger, entry.Name!, "name contains invalid characters for a directory name"); + continue; + } + + if (string.IsNullOrWhiteSpace(entry.Url)) + { + LogIndexEntrySkipped(this._logger, entry.Name!, "missing required 'url' field"); + continue; + } + + valid.Add(entry); + } + + return valid; + + static bool IsValidDirectoryName(string value, char[] invalidChars) + { + foreach (char c in value) + { + if (Array.IndexOf(invalidChars, c) >= 0 || c == Path.DirectorySeparatorChar || c == Path.AltDirectorySeparatorChar) + { + return false; + } + } + + // Names consisting solely of dots or whitespace are invalid directory names. + string trimmed = value.Trim().Trim('.'); + return trimmed.Length > 0; + } + } + + /// + /// Replaces control characters in a file-system path with ? so the path is safe to include + /// in log messages without risking terminal-escape injection. + /// + 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); + } + + /// + /// Attempts to recursively delete a skill directory. + /// + private bool TryDeleteSkillDirectory(string skillName, string directory) + { + try + { + if (Directory.Exists(directory)) + { + Directory.Delete(directory, recursive: true); + } + + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + LogArchiveSkillCleanupFailed(this._logger, skillName, ex); + return false; + } + } + + [LoggerMessage(LogLevel.Debug, "Skipping skill index entry '{SkillName}': {Reason}")] + private static partial void LogIndexEntrySkipped(ILogger logger, string skillName, string reason); + + [LoggerMessage(LogLevel.Debug, "Extracted archive skill '{SkillName}' to {TargetDirectory}")] + private static partial void LogArchiveExtracted(ILogger logger, string skillName, string targetDirectory); + + [LoggerMessage(LogLevel.Debug, "Pruned stale archive skill '{SkillName}' at {TargetDirectory}")] + private static partial void LogArchiveSkillPruned(ILogger logger, string skillName, string targetDirectory); + + [LoggerMessage(LogLevel.Warning, "Failed to prune stale archive skill '{SkillName}'.")] + private static partial void LogArchiveSkillPruneFailed(ILogger logger, string skillName, Exception exception); + + [LoggerMessage(LogLevel.Warning, "Failed to clean existing archive skill directory for '{SkillName}'.")] + private static partial void LogArchiveSkillCleanupFailed(ILogger logger, string skillName, Exception exception); + + [LoggerMessage(LogLevel.Warning, "Failed to read archive resource for skill '{SkillName}'.")] + private static partial void LogArchiveReadFailed(ILogger logger, string skillName, Exception exception); + + [LoggerMessage(LogLevel.Warning, "Failed to decode archive resource for skill '{SkillName}'.")] + private static partial void LogArchiveDecodeFailed(ILogger logger, string skillName, Exception exception); + + [LoggerMessage(LogLevel.Warning, "Failed to extract archive for skill '{SkillName}'.")] + private static partial void LogArchiveExtractFailed(ILogger logger, string skillName, Exception exception); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs new file mode 100644 index 00000000000..ce9c862ff46 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft. All rights reserved. + +namespace Microsoft.Agents.AI; + +/// +/// The archive container formats supported by . +/// +internal enum ArchiveFormat +{ + /// The format could not be determined. + Unknown, + + /// A ZIP archive. + Zip, + + /// An uncompressed TAR archive. + Tar, + + /// A gzip-compressed TAR archive (.tar.gz/.tgz). + TarGz, +} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs new file mode 100644 index 00000000000..34b678165f5 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/IMcpSkillEntryLoader.cs @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.Agents.AI; + +/// +/// Loads instances from skill://index.json entries of a single +/// distribution (e.g. skill-md or archive). +/// +/// +/// Implementations receive all index entries of their type as a batch so that types whose +/// materialization spans the whole set (e.g. archive, which reconciles and prunes a shared +/// on-disk directory) can reason about every advertised entry at once. The batch may be empty: every +/// registered loader is invoked even when the server advertises no entries of its type, so that +/// type-wide lifecycle work (such as pruning leftover archive directories) still runs. +/// +internal interface IMcpSkillEntryLoader +{ + /// + /// Gets the index entry type this loader handles (matched case-insensitively). + /// + string EntryType { get; } + + /// + /// Loads the skills for every supplied entry. Entries that are invalid or fail to load are + /// skipped (and logged) rather than throwing; the returned list contains only the skills that + /// loaded successfully. + /// + /// The index entries of this loader's . May be empty. + /// A token to cancel the operation. + Task> LoadAsync(IReadOnlyList entries, CancellationToken cancellationToken); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs new file mode 100644 index 00000000000..bf910478485 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/SkillMdEntryLoader.cs @@ -0,0 +1,80 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using ModelContextProtocol.Client; + +namespace Microsoft.Agents.AI; + +/// +/// Loads skill-md index entries: each entry's SKILL.md and sibling resources are +/// fetched on demand from the MCP server via an . +/// +internal sealed partial class SkillMdEntryLoader : IMcpSkillEntryLoader +{ + private readonly McpClient _client; + private readonly ILogger _logger; + + public SkillMdEntryLoader(McpClient client, ILoggerFactory loggerFactory) + { + this._client = client; + this._logger = loggerFactory.CreateLogger(); + } + + /// + public string EntryType => "skill-md"; + + /// + public Task> LoadAsync(IReadOnlyList entries, CancellationToken cancellationToken) + { + var skills = new List(); + + foreach (var entry in entries) + { + if (this.TryLoadSkillMdEntry(entry, out AgentSkill? skill)) + { + skills.Add(skill); + } + } + + return Task.FromResult>(skills); + } + + private bool TryLoadSkillMdEntry(McpSkillIndexEntry entry, [NotNullWhen(true)] out AgentSkill? skill) + { + skill = null; + + if (string.IsNullOrWhiteSpace(entry.Url)) + { + LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", "missing required 'url' field"); + return false; + } + + AgentSkillFrontmatter frontmatter; + try + { + frontmatter = new AgentSkillFrontmatter(entry.Name!, entry.Description!); + } + catch (ArgumentException ex) + { + LogIndexEntrySkipped(this._logger, entry.Name ?? "(unnamed)", $"invalid metadata: {ex.Message}"); + return false; + } + + skill = new AgentMcpSkill(frontmatter, entry.Url!, this._client); + + LogSkillLoaded(this._logger, frontmatter.Name); + + return true; + } + + [LoggerMessage(LogLevel.Information, "Loaded MCP skill: {SkillName}")] + private static partial void LogSkillLoaded(ILogger logger, string skillName); + + [LoggerMessage(LogLevel.Debug, "Skipping skill index entry '{SkillName}': {Reason}")] + private static partial void LogIndexEntrySkipped(ILogger logger, string skillName, string reason); +} diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProviderBuilder.cs b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProviderBuilder.cs index e49c620187a..c07174b8785 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProviderBuilder.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/AgentSkillsProviderBuilder.cs @@ -131,6 +131,20 @@ public AgentSkillsProviderBuilder UseSource(AgentSkillsSource source) return this; } + /// + /// Adds a custom skill source created by a factory that receives the builder's logger factory + /// at build time. Use this overload when the source needs logging and should not require the + /// caller to pass an explicitly. + /// + /// A factory that creates the skill source given an optional logger factory. + /// This builder instance for chaining. + public AgentSkillsProviderBuilder UseSource(Func factory) + { + _ = Throw.IfNull(factory); + this._sourceFactories.Add((_, loggerFactory) => factory(loggerFactory)); + return this; + } + /// /// Sets a custom system prompt template. /// diff --git a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs index 54a5dec10cc..36d2be3ec84 100644 --- a/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillsSource.cs @@ -27,7 +27,7 @@ namespace Microsoft.Agents.AI; /// Resource and script paths are checked against path traversal and symlink escape attacks. /// [Experimental(DiagnosticIds.Experiments.AgentsAIExperiments)] -internal sealed partial class AgentFileSkillsSource : AgentSkillsSource +public sealed partial class AgentFileSkillsSource : AgentSkillsSource { private const string SkillFileName = "SKILL.md"; private const int DefaultSearchDepth = 2; diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs new file mode 100644 index 00000000000..8a26b83cab1 --- /dev/null +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs @@ -0,0 +1,693 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using System.Formats.Tar; +using System.IO; +using System.IO.Compression; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Protocol; +using ModelContextProtocol.Server; + +namespace Microsoft.Agents.AI.Skills.Mcp.UnitTests; + +/// +/// Unit tests for archive-distributed skill handling in . +/// +public sealed class AgentMcpSkillsSourceArchiveTests : IDisposable +{ + private const string ArchivedSkillMd = """ + --- + name: archived-skill + description: A skill delivered as an archive. + --- + # Archived Skill + + Body from the archive. + """; + + private const string StaleSkillMd = """ + --- + name: stale-skill + description: Left over from a previous session. + --- + Old content. + """; + + private const string SkillAMd = """ + --- + name: skill-a + description: Skill A. + --- + Content A. + """; + + private const string SkillBMd = """ + --- + name: skill-b + description: Skill B. + --- + Content B. + """; + + private readonly string _extractionRoot = + Path.Combine(Path.GetTempPath(), "af-mcp-archive-tests", Guid.NewGuid().ToString("N")); + + private const int ManyFileArchiveFileCount = 60; + + [Fact] + public async Task GetSkillsAsync_ZipArchive_DiscoversSkillAsync() + { + // Arrange + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert + var skill = Assert.Single(skills); + Assert.Equal("archived-skill", skill.Frontmatter.Name); + Assert.Equal("A skill delivered as an archive.", skill.Frontmatter.Description); + Assert.Contains("Body from the archive.", await skill.GetContentAsync()); + } + + [Fact] + public async Task GetSkillsAsync_TarGzArchive_DiscoversSkillAsync() + { + // Arrange + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert + var skill = Assert.Single(skills); + Assert.Equal("archived-skill", skill.Frontmatter.Name); + Assert.Contains("Body from the archive.", await skill.GetContentAsync()); + } + + [Fact] + public async Task GetSkillsAsync_ArchiveWithScript_SurfacesScriptAsReadableResourceAsync() + { + // Arrange - archive bundles a script file alongside SKILL.md. Over MCP, scripts must never be + // executable; they are surfaced as readable resources only when explicitly included via options. + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions + { + ArchiveSkillsDirectory = this._extractionRoot, + ArchiveResourceExtensions = [".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt", ".py"], + }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skill = Assert.Single(await source.GetSkillsAsync()); + var resource = await skill.GetResourceAsync("scripts/run.py"); + + // Assert - the .py file is readable as a resource (not an executable script). + Assert.NotNull(resource); + var content = await resource!.ReadAsync(); + Assert.Equal("print('hello')", content); + } + + [Fact] + public async Task GetSkillsAsync_TwoSourcesWithSeparateDirectories_DoNotCollideAsync() + { + // Arrange - two servers publish an archive skill with the SAME name but different content. + // Pointing each source at its own directory keeps their extracted content separate. + await using var serverA = new InMemoryMcpServer(builder => builder.WithResources()); + await using var clientA = await serverA.CreateClientAsync(); + await using var serverB = new InMemoryMcpServer(builder => builder.WithResources()); + await using var clientB = await serverB.CreateClientAsync(); + + var sourceA = new AgentMcpSkillsSource(clientA, new() { ArchiveSkillsDirectory = Path.Combine(this._extractionRoot, "a") }); + var sourceB = new AgentMcpSkillsSource(clientB, new() { ArchiveSkillsDirectory = Path.Combine(this._extractionRoot, "b") }); + + // Act + var skillA = Assert.Single(await sourceA.GetSkillsAsync()); + var skillB = Assert.Single(await sourceB.GetSkillsAsync()); + + // Assert - each source kept its own content despite the shared skill name. + Assert.Equal("shared-skill", skillA.Frontmatter.Name); + Assert.Equal("shared-skill", skillB.Frontmatter.Name); + Assert.Contains("Content from server A.", await skillA.GetContentAsync()); + Assert.Contains("Content from server B.", await skillB.GetContentAsync()); + } + + [Fact] + public async Task GetSkillsAsync_FirstRun_PrunesLeftoverSkillDirectoryAsync() + { + // Arrange - a leftover skill directory from a previous session sits in the provided directory. + string staleDir = Path.Combine(this._extractionRoot, "stale-skill"); + Directory.CreateDirectory(staleDir); + await File.WriteAllTextAsync(Path.Combine(staleDir, "SKILL.md"), StaleSkillMd); + + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert - the leftover directory is pruned and only the advertised skill remains. + Assert.False(Directory.Exists(staleDir)); + var skill = Assert.Single(skills); + Assert.Equal("archived-skill", skill.Frontmatter.Name); + } + + [Fact] + public async Task GetSkillsAsync_SkillNoLongerAdvertised_IsPrunedAsync() + { + // Arrange - an earlier run extracts skill-a and skill-b into the shared directory. + await using var fullServer = new InMemoryMcpServer(builder => builder.WithResources()); + await using var fullClient = await fullServer.CreateClientAsync(); + var firstSource = new AgentMcpSkillsSource(fullClient, new() { ArchiveSkillsDirectory = this._extractionRoot }); + var firstSkills = await firstSource.GetSkillsAsync(); + Assert.Equal(2, firstSkills.Count); + Assert.True(Directory.Exists(Path.Combine(this._extractionRoot, "skill-b"))); + + // Act - a later run sees only skill-a. + await using var partialServer = new InMemoryMcpServer(builder => builder.WithResources()); + await using var partialClient = await partialServer.CreateClientAsync(); + var secondSource = new AgentMcpSkillsSource(partialClient, new() { ArchiveSkillsDirectory = this._extractionRoot }); + var secondSkills = await secondSource.GetSkillsAsync(); + + // Assert - skill-b's directory was pruned; only skill-a remains. + var skill = Assert.Single(secondSkills); + Assert.Equal("skill-a", skill.Frontmatter.Name); + Assert.False(Directory.Exists(Path.Combine(this._extractionRoot, "skill-b"))); + Assert.True(Directory.Exists(Path.Combine(this._extractionRoot, "skill-a"))); + } + + [Fact] + public async Task GetSkillsAsync_ServerListsNoArchives_PrunesLeftoversAsync() + { + // Arrange - a leftover skill directory exists but the server advertises no archive skills. + string staleDir = Path.Combine(this._extractionRoot, "stale-skill"); + Directory.CreateDirectory(staleDir); + await File.WriteAllTextAsync(Path.Combine(staleDir, "SKILL.md"), StaleSkillMd); + + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions { ArchiveSkillsDirectory = this._extractionRoot }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert - the leftover directory is pruned and no skills are returned. + Assert.Empty(skills); + Assert.False(Directory.Exists(staleDir)); + } + + [Fact] + public async Task GetSkillsAsync_SecondDiscovery_ReExtractsContentAsync() + { + // Arrange - a first run extracts server A's content into the shared directory. + await using (var serverA = new InMemoryMcpServer(builder => builder.WithResources())) + await using (var clientA = await serverA.CreateClientAsync()) + { + var firstSource = new AgentMcpSkillsSource(clientA, new() { ArchiveSkillsDirectory = this._extractionRoot }); + Assert.Single(await firstSource.GetSkillsAsync()); + } + + // Act - a second run over the same directory re-extracts server B's content. + await using var serverB = new InMemoryMcpServer(builder => builder.WithResources()); + await using var clientB = await serverB.CreateClientAsync(); + var source = new AgentMcpSkillsSource(clientB, new() { ArchiveSkillsDirectory = this._extractionRoot }); + var skill = Assert.Single(await source.GetSkillsAsync()); + + // Assert - the content was replaced with server B's. + Assert.Contains("Content from server B.", await skill.GetContentAsync()); + } + + [Fact] + public async Task GetSkillsAsync_PreExtractionCleanupFails_DoesNotDiscoverStaleSkillAsync() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + // Arrange - a stale skill directory contains a locked file, causing recursive deletion to fail. + string skillDirectory = Path.Combine(this._extractionRoot, "shared-skill"); + Directory.CreateDirectory(skillDirectory); + await File.WriteAllTextAsync(Path.Combine(skillDirectory, "SKILL.md"), """ + --- + name: shared-skill + description: Shared. + --- + Stale content. + """); + + string lockedResourcePath = Path.Combine(skillDirectory, "locked.txt"); + await using var lockedResource = new FileStream(lockedResourcePath, FileMode.Create, FileAccess.ReadWrite, FileShare.None); + + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert - failed cleanup prevents the stale directory from being proxied to AgentFileSkillsSource. + Assert.Empty(skills); + Assert.True(File.Exists(lockedResourcePath)); + } + + [Fact] + public void Extract_ZipSlipEntry_DoesNotEscapeTargetDirectory() + { + // Arrange - a malicious zip whose entry traverses out of the target directory. + byte[] zip = BuildZip(("../escaped.txt", "pwned"), ("SKILL.md", ArchivedSkillMd)); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act + AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target); + + // Assert - the traversal entry was skipped; only the safe entry was written. + Assert.False(File.Exists(Path.Combine(this._extractionRoot, "escaped.txt"))); + Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); + } + + [Fact] + public void Extract_CaseVariantZipSlipEntry_DoesNotEscapeTargetDirectory() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + // Arrange - on case-sensitive file systems, "skill" is a sibling of "Skill", not the target. + byte[] zip = BuildZip(("../skill/escaped.txt", "pwned"), ("SKILL.md", ArchivedSkillMd)); + string target = Path.Combine(this._extractionRoot, "Skill"); + + // Act + AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target); + + // Assert - the case-variant traversal entry was skipped; only the safe entry was written. + Assert.False(File.Exists(Path.Combine(this._extractionRoot, "skill", "escaped.txt"))); + Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); + } + + [Fact] + public void Extract_ArchiveExceedsDefaultFileCount_Throws() + { + // Arrange - more files than the default cap (50). + var entries = Enumerable.Range(0, AgentMcpSkillArchiveExtractor.DefaultMaxFileCount + 1) + .Select(i => ($"file{i}.txt", "x")) + .ToArray(); + byte[] zip = BuildZip(entries); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act / Assert + Assert.Throws( + () => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target)); + } + + [Fact] + public void Extract_ArchiveExceedsDefaultUncompressedSize_Throws() + { + // Arrange - a single file larger than the default uncompressed budget (1 MB). + string oversized = new('x', (int)AgentMcpSkillArchiveExtractor.DefaultMaxUncompressedSizeBytes + 1); + byte[] zip = BuildZip(("SKILL.md", oversized)); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act / Assert + Assert.Throws( + () => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target)); + } + + [Fact] + public void Extract_TarGzExceedsUncompressedSize_Throws() + { + // Arrange - a gzip-compressed tar whose expansion exceeds the default budget. The ZIP pre-gate + // does not apply here, so this exercises the authoritative streaming cap (CopyWithLimit). + string oversized = new('x', (int)AgentMcpSkillArchiveExtractor.DefaultMaxUncompressedSizeBytes + 1); + byte[] tarGz = BuildTarGz(("SKILL.md", oversized)); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act / Assert + Assert.Throws( + () => AgentMcpSkillArchiveExtractor.Extract(tarGz, ArchiveFormat.TarGz, target)); + } + + [Fact] + public void Extract_TarWithLinkEntries_SkipsLinksAndExtractsRegularFiles() + { + // Arrange - a tar.gz containing symbolic-link and hard-link entries whose targets escape the + // target directory, alongside a regular file. Link entries must be skipped so an archive cannot + // create links that point outside the target directory. + byte[] tarGz = BuildTarGzFromEntries( + new PaxTarEntry(TarEntryType.SymbolicLink, "evil-symlink") { LinkName = "../../escaped.txt" }, + new PaxTarEntry(TarEntryType.HardLink, "evil-hardlink") { LinkName = "../../escaped.txt" }, + new PaxTarEntry(TarEntryType.RegularFile, "SKILL.md") + { + DataStream = new MemoryStream(Encoding.UTF8.GetBytes(ArchivedSkillMd)), + }); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act + AgentMcpSkillArchiveExtractor.Extract(tarGz, ArchiveFormat.TarGz, target); + + // Assert - only the regular file is materialized; neither link entry is written. + Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); + Assert.False(File.Exists(Path.Combine(target, "evil-symlink"))); + Assert.False(File.Exists(Path.Combine(target, "evil-hardlink"))); + Assert.Single(Directory.GetFileSystemEntries(target)); + } + + [Fact] + public void Extract_WithinDefaultLimits_Succeeds() + { + // Arrange - a typical small archive that is comfortably within the default limits. + byte[] zip = BuildZip(("SKILL.md", ArchivedSkillMd), ("reference.md", "Some reference content.")); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act + AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target); + + // Assert + Assert.True(File.Exists(Path.Combine(target, "SKILL.md"))); + Assert.True(File.Exists(Path.Combine(target, "reference.md"))); + } + + [Fact] + public void Extract_RaisedLimits_AllowsLargerArchive() + { + // Arrange - an archive that exceeds the default file count but fits within raised limits. + var entries = Enumerable.Range(0, AgentMcpSkillArchiveExtractor.DefaultMaxFileCount + 1) + .Select(i => ($"file{i}.txt", "x")) + .ToArray(); + byte[] zip = BuildZip(entries); + string target = Path.Combine(this._extractionRoot, "skill"); + + // Act + AgentMcpSkillArchiveExtractor.Extract( + zip, + ArchiveFormat.Zip, + target, + maxFileCount: entries.Length + 1); + + // Assert - every entry was extracted. + Assert.Equal(entries.Length, Directory.GetFiles(target).Length); + } + + [Fact] + public async Task GetSkillsAsync_ArchiveExceedsDefaultFileCount_SkillSkippedAsync() + { + // Arrange - the archive has more files than the default cap, so extraction fails and the skill is skipped. + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot }); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert + Assert.Empty(skills); + } + + [Fact] + public async Task GetSkillsAsync_ArchiveExceedsConfiguredArchiveSize_SkillSkippedAsync() + { + // Arrange - the valid archive is larger than the configured downloaded-archive byte cap. + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions + { + ArchiveSkillsDirectory = this._extractionRoot, + ArchiveMaxSizeBytes = 1, + }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skills = await source.GetSkillsAsync(); + + // Assert + Assert.Empty(skills); + } + + [Fact] + public async Task GetSkillsAsync_RaisedFileCountOption_LoadsLargerArchiveAsync() + { + // Arrange - raising ArchiveMaxFileCount lets the larger archive through. + await using var server = new InMemoryMcpServer(builder => builder.WithResources()); + await using var client = await server.CreateClientAsync(); + var options = new AgentMcpSkillsSourceOptions + { + ArchiveSkillsDirectory = this._extractionRoot, + ArchiveMaxFileCount = ManyFileArchiveFileCount + 1, + }; + var source = new AgentMcpSkillsSource(client, options); + + // Act + var skill = Assert.Single(await source.GetSkillsAsync()); + + // Assert + Assert.Equal("archived-skill", skill.Frontmatter.Name); + } + + public void Dispose() + { + try + { + if (Directory.Exists(this._extractionRoot)) + { + Directory.Delete(this._extractionRoot, recursive: true); + } + } + catch (IOException) + { + // Best-effort cleanup. + } + } + + private static byte[] BuildZip(params (string Path, string Content)[] entries) + { + using var ms = new MemoryStream(); + using (var archive = new ZipArchive(ms, ZipArchiveMode.Create, leaveOpen: true)) + { + foreach (var (path, content) in entries) + { + ZipArchiveEntry entry = archive.CreateEntry(path); + using Stream stream = entry.Open(); + byte[] bytes = Encoding.UTF8.GetBytes(content); + stream.Write(bytes, 0, bytes.Length); + } + } + + return ms.ToArray(); + } + + private static byte[] BuildTarGz(params (string Path, string Content)[] entries) + { + using var ms = new MemoryStream(); + using (var gzip = new GZipStream(ms, CompressionMode.Compress, leaveOpen: true)) + using (var writer = new TarWriter(gzip, leaveOpen: true)) + { + foreach (var (path, content) in entries) + { + var entry = new PaxTarEntry(TarEntryType.RegularFile, path) + { + DataStream = new MemoryStream(Encoding.UTF8.GetBytes(content)), + }; + writer.WriteEntry(entry); + } + } + + return ms.ToArray(); + } + + private static byte[] BuildTarGzFromEntries(params TarEntry[] entries) + { + using var ms = new MemoryStream(); + using (var gzip = new GZipStream(ms, CompressionMode.Compress, leaveOpen: true)) + using (var writer = new TarWriter(gzip, leaveOpen: true)) + { + foreach (var entry in entries) + { + writer.WriteEntry(entry); + } + } + + return ms.ToArray(); + } + + private static string ArchiveIndex(string skillName, string url) => $$""" + { + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "{{skillName}}", + "type": "archive", + "description": "A skill delivered as an archive.", + "url": "{{url}}" + } + ] + } + """; + + #region Resource classes (registered with the MCP server via WithResources) + +#pragma warning disable CA1812 + + [McpServerResourceType] + private sealed class ZipArchiveServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); + + [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] + public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", ArchivedSkillMd)), + "skill://archives/archived-skill.zip", + "application/zip"); + } + + [McpServerResourceType] + private sealed class TarGzArchiveServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.tar.gz"); + + [McpServerResource(UriTemplate = "skill://archives/archived-skill.tar.gz", Name = "archive", MimeType = "application/gzip")] + public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( + BuildTarGz(("SKILL.md", ArchivedSkillMd)), + "skill://archives/archived-skill.tar.gz", + "application/gzip"); + } + + [McpServerResourceType] + private sealed class ZipWithScriptServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); + + [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] + public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", ArchivedSkillMd), ("scripts/run.py", "print('hello')")), + "skill://archives/archived-skill.zip", + "application/zip"); + } + + [McpServerResourceType] + private sealed class ManyFileArchiveServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"); + + [McpServerResource(UriTemplate = "skill://archives/archived-skill.zip", Name = "archive", MimeType = "application/zip")] + public static BlobResourceContents Archive() + { + var entries = new List<(string Path, string Content)> { ("SKILL.md", ArchivedSkillMd) }; + for (int i = 0; i < ManyFileArchiveFileCount - 1; i++) + { + entries.Add(($"reference/file{i}.txt", "x")); + } + + return BlobResourceContents.FromBytes( + BuildZip(entries.ToArray()), + "skill://archives/archived-skill.zip", + "application/zip"); + } + } + + [McpServerResourceType] + private sealed class SharedNameServerA + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("shared-skill", "skill://archives/shared-skill.zip"); + + [McpServerResource(UriTemplate = "skill://archives/shared-skill.zip", Name = "archive", MimeType = "application/zip")] + public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", """ + --- + name: shared-skill + description: Shared. + --- + Content from server A. + """)), + "skill://archives/shared-skill.zip", + "application/zip"); + } + + [McpServerResourceType] + private sealed class SharedNameServerB + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("shared-skill", "skill://archives/shared-skill.zip"); + + [McpServerResource(UriTemplate = "skill://archives/shared-skill.zip", Name = "archive", MimeType = "application/zip")] + public static BlobResourceContents Archive() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", """ + --- + name: shared-skill + description: Shared. + --- + Content from server B. + """)), + "skill://archives/shared-skill.zip", + "application/zip"); + } + + [McpServerResourceType] + private sealed class TwoSkillServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => """ + { + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { "name": "skill-a", "type": "archive", "description": "Skill A.", "url": "skill://archives/skill-a.zip" }, + { "name": "skill-b", "type": "archive", "description": "Skill B.", "url": "skill://archives/skill-b.zip" } + ] + } + """; + + [McpServerResource(UriTemplate = "skill://archives/skill-a.zip", Name = "skill-a", MimeType = "application/zip")] + public static BlobResourceContents SkillA() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", SkillAMd)), "skill://archives/skill-a.zip", "application/zip"); + + [McpServerResource(UriTemplate = "skill://archives/skill-b.zip", Name = "skill-b", MimeType = "application/zip")] + public static BlobResourceContents SkillB() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", SkillBMd)), "skill://archives/skill-b.zip", "application/zip"); + } + + [McpServerResourceType] + private sealed class OneSkillServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => ArchiveIndex("skill-a", "skill://archives/skill-a.zip"); + + [McpServerResource(UriTemplate = "skill://archives/skill-a.zip", Name = "skill-a", MimeType = "application/zip")] + public static BlobResourceContents SkillA() => BlobResourceContents.FromBytes( + BuildZip(("SKILL.md", SkillAMd)), "skill://archives/skill-a.zip", "application/zip"); + } + + [McpServerResourceType] + private sealed class NoArchiveServer + { + [McpServerResource(UriTemplate = "skill://index.json", Name = "index", MimeType = "application/json")] + public static string Index() => """ + { + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [] + } + """; + } + +#pragma warning restore CA1812 + + #endregion +} diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs index bf5f52758f7..891f2a726b4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceTests.cs @@ -40,7 +40,7 @@ Body content here. [Fact] public async Task GetSkillsAsync_IndexBasedDiscovery_ReturnsSkillAsync() { - // Arrange — server exposes both skill://index.json and the skill itself. + // Arrange - server exposes both skill://index.json and the skill itself. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); await using var client = await server.CreateClientAsync(); @@ -49,7 +49,7 @@ public async Task GetSkillsAsync_IndexBasedDiscovery_ReturnsSkillAsync() // Act var skills = await source.GetSkillsAsync(); - // Assert — frontmatter comes from index; Content is the actual SKILL.md body from the server. + // Assert - frontmatter comes from index; Content is the actual SKILL.md body from the server. var skill = Assert.Single(skills); Assert.Equal("unit-converter", skill.Frontmatter.Name); Assert.Equal("Convert between common units.", skill.Frontmatter.Description); @@ -63,7 +63,7 @@ public async Task GetSkillsAsync_IndexBasedDiscovery_ReturnsSkillAsync() [Fact] public async Task GetSkillsAsync_NoIndex_ReturnsEmptyAsync() { - // Arrange — server only exposes SKILL.md, no skill://index.json. + // Arrange - server only exposes SKILL.md, no skill://index.json. // Per SEP-2640, discovery requires the index document; without it, no skills are surfaced. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); @@ -80,7 +80,7 @@ public async Task GetSkillsAsync_NoIndex_ReturnsEmptyAsync() [Fact] public async Task GetResourceAsync_SiblingText_ReturnsContentAsync() { - // Arrange — server exposes index, SKILL.md, and a sibling reference file. + // Arrange - server exposes index, SKILL.md, and a sibling reference file. // The skill reads the sibling on demand via GetResourceAsync. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); @@ -100,7 +100,7 @@ public async Task GetResourceAsync_SiblingText_ReturnsContentAsync() [Fact] public async Task GetResourceAsync_SiblingBinary_ReturnsDataContentAsync() { - // Arrange — server exposes index, SKILL.md, and a binary sibling. + // Arrange - server exposes index, SKILL.md, and a binary sibling. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); await using var client = await server.CreateClientAsync(); @@ -121,7 +121,7 @@ public async Task GetResourceAsync_SiblingBinary_ReturnsDataContentAsync() [Fact] public async Task GetResourceAsync_UnknownName_ReturnsNullAsync() { - // Arrange — index advertises a skill, but no sibling resource exists. + // Arrange - index advertises a skill, but no sibling resource exists. // GetResourceAsync eagerly fetches from the MCP server; a non-existent // resource causes the server to return an error, so null is returned. await using var server = new InMemoryMcpServer(builder => @@ -133,7 +133,7 @@ public async Task GetResourceAsync_UnknownName_ReturnsNullAsync() var skill = Assert.Single(await source.GetSkillsAsync()); var resource = await skill.GetResourceAsync("references/does-not-exist.md"); - // Assert — resource does not exist on the server, so null is returned + // Assert - resource does not exist on the server, so null is returned Assert.Null(resource); } @@ -143,7 +143,7 @@ public async Task GetResourceAsync_UnknownName_ReturnsNullAsync() [InlineData("..")] public async Task GetResourceAsync_PathTraversalName_ReturnsNullAsync(string name) { - // Arrange — '..' segments result in URIs that don't match any server resource. + // Arrange - '..' segments result in URIs that don't match any server resource. // The MCP server returns an error for unknown URIs, so GetResourceAsync returns null. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); @@ -154,14 +154,14 @@ public async Task GetResourceAsync_PathTraversalName_ReturnsNullAsync(string nam var skill = Assert.Single(await source.GetSkillsAsync()); var resource = await skill.GetResourceAsync(name); - // Assert — resource does not exist on the server, so null is returned + // Assert - resource does not exist on the server, so null is returned Assert.Null(resource); } [Fact] public async Task GetSkillsAsync_DoesNotReadSkillMdAsync() { - // Arrange — index points to a non-existent SKILL.md URI. Because the source builds + // Arrange - index points to a non-existent SKILL.md URI. Because the source builds // skills from index info only, discovery still succeeds. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); @@ -171,7 +171,7 @@ public async Task GetSkillsAsync_DoesNotReadSkillMdAsync() // Act var skills = await source.GetSkillsAsync(); - // Assert — discovery succeeds from index alone. + // Assert - discovery succeeds from index alone. var skill = Assert.Single(skills); Assert.Equal("unit-converter", skill.Frontmatter.Name); } @@ -179,7 +179,7 @@ public async Task GetSkillsAsync_DoesNotReadSkillMdAsync() [Fact] public async Task GetSkillsAsync_IndexEntryWithInvalidName_IsSkippedAsync() { - // Arrange — index entry has an invalid (uppercase) name, which AgentSkillFrontmatter rejects. + // Arrange - index entry has an invalid (uppercase) name, which AgentSkillFrontmatter rejects. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); await using var client = await server.CreateClientAsync(); @@ -195,7 +195,7 @@ public async Task GetSkillsAsync_IndexEntryWithInvalidName_IsSkippedAsync() [Fact] public async Task GetSkillsAsync_IndexEntryWithMissingRequiredFields_IsSkippedAsync() { - // Arrange — index entry is missing the required description and url fields. + // Arrange - index entry is missing the required description and url fields. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); await using var client = await server.CreateClientAsync(); @@ -209,9 +209,10 @@ public async Task GetSkillsAsync_IndexEntryWithMissingRequiredFields_IsSkippedAs } [Fact] - public async Task GetSkillsAsync_IndexEntryWithUnsupportedType_IsSkippedAsync() + public async Task GetSkillsAsync_ArchiveEntryWithUnreadableResource_IsSkippedAsync() { - // Arrange — index has an "archive" entry, which this source does not support. + // Arrange - index has an "archive" entry, but the referenced archive resource does not + // exist on the server, so reading it fails and the entry is skipped gracefully. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); await using var client = await server.CreateClientAsync(); @@ -227,7 +228,7 @@ public async Task GetSkillsAsync_IndexEntryWithUnsupportedType_IsSkippedAsync() [Fact] public async Task GetSkillsAsync_IndexEntryWithTemplateType_IsSkippedAsync() { - // Arrange — index has an "mcp-resource-template" entry (parameterized skill namespace). + // Arrange - index has an "mcp-resource-template" entry (parameterized skill namespace). // The current source skips template entries; they require user input to materialize. await using var server = new InMemoryMcpServer(builder => builder.WithResources()); @@ -243,7 +244,7 @@ public async Task GetSkillsAsync_IndexEntryWithTemplateType_IsSkippedAsync() #region Resource classes (registered with the MCP server via WithResources) - // CA1812 flags these classes as "never instantiated", which is technically correct — + // CA1812 flags these classes as "never instantiated", which is technically correct - // they are never constructed because they only contain static methods (e.g. `public static string Index()`). // The MCP framework discovers and invokes these static methods via the [McpServerResourceType] and // [McpServerResource] attributes registered through WithResources(), without ever creating an instance. From ee8ea3514fc151e6ed06497bee8d5073354c75a4 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 16:59:56 +0100 Subject: [PATCH 2/7] Fix CS0121 ambiguity in UseSource null test Cast null! to AgentSkillsSource to disambiguate from the new Func overload. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AgentSkills/AgentSkillsProviderBuilderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderBuilderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderBuilderTests.cs index 85335256a70..a95e8acc15d 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderBuilderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/AgentSkills/AgentSkillsProviderBuilderTests.cs @@ -55,7 +55,7 @@ public void UseSource_NullSource_ThrowsArgumentNullException() var builder = new AgentSkillsProviderBuilder(); // Act & Assert - Assert.Throws(() => builder.UseSource(null!)); + Assert.Throws(() => builder.UseSource((AgentSkillsSource)null!)); } [Fact] From 60d33d14c568436196f6574ab1941b1fd0089efd Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 17:04:08 +0100 Subject: [PATCH 3/7] Address PR review: fix misleading comment and catch UnauthorizedAccessException in Dispose - Remove hardcoded '50' from test comment; it now says 'default cap' without citing a specific number that can drift from the constant. - Catch UnauthorizedAccessException alongside IOException in test Dispose for robust cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSourceArchiveTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs index 8a26b83cab1..091d2c8fbe7 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs @@ -303,7 +303,7 @@ public void Extract_CaseVariantZipSlipEntry_DoesNotEscapeTargetDirectory() [Fact] public void Extract_ArchiveExceedsDefaultFileCount_Throws() { - // Arrange - more files than the default cap (50). + // Arrange - more files than the default cap. var entries = Enumerable.Range(0, AgentMcpSkillArchiveExtractor.DefaultMaxFileCount + 1) .Select(i => ($"file{i}.txt", "x")) .ToArray(); @@ -467,7 +467,7 @@ public void Dispose() Directory.Delete(this._extractionRoot, recursive: true); } } - catch (IOException) + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { // Best-effort cleanup. } From 163fedb873b4e51b4d394e525c3d35752c7d455c Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 17:32:28 +0100 Subject: [PATCH 4/7] Decouple shared refresh from per-caller cancellation Use CancellationToken.None for the shared refresh so one caller's cancellation does not abort work for all concurrent waiters. Waiters use WaitAsync(cancellationToken) to cancel independently. The refresh owner checks its own token after publishing the result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSource.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs index e4fc54377be..123c4100b40 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs @@ -90,17 +90,24 @@ public override async Task> GetSkillsAsync(CancellationToken c if (Interlocked.CompareExchange(ref this._refreshTask, tcs.Task, null) is { } existing) { - return await existing.ConfigureAwait(false); + // Wait for the in-flight refresh but let this caller cancel its own wait independently + // without aborting the shared refresh work. + return await existing.WaitAsync(cancellationToken).ConfigureAwait(false); } try { - var skills = await this.GetCoreSkillsAsync(cancellationToken).ConfigureAwait(false); + // The refresh owner uses CancellationToken.None so that a single caller's cancellation + // does not abort the shared refresh for all concurrent waiters. + var skills = await this.GetCoreSkillsAsync(CancellationToken.None).ConfigureAwait(false); this.UpdateCache(skills); tcs.SetResult(skills); + // Allow the current caller to observe cancellation without impacting other awaiters. + cancellationToken.ThrowIfCancellationRequested(); + return skills; } catch (Exception ex) From 45deb9ca9ae4fafe506ac166d26db7cb67a180c1 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 17:35:43 +0100 Subject: [PATCH 5/7] Fix file encoding: add UTF-8 BOM to archive tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSourceArchiveTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs index 091d2c8fbe7..2e195a7008f 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. using System; using System.Collections.Generic; From 0a8cdefb6d7e7a792b8728cab981d6b88dd567aa Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 18:26:29 +0100 Subject: [PATCH 6/7] Fix file encoding: add UTF-8 BOM to ArchiveFormat.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs index ce9c862ff46..07d2c5f858c 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveFormat.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft. All rights reserved. +// Copyright (c) Microsoft. All rights reserved. namespace Microsoft.Agents.AI; From 943527b18856188d0a660caffc4abcb1f7e6e877 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Fri, 19 Jun 2026 19:00:07 +0100 Subject: [PATCH 7/7] Clarify pruning doc: covers non-actionable entries too Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Skills/AgentMcpSkillsSourceOptions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs index 8b3fc9060ea..78cf93881e1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSourceOptions.cs @@ -24,7 +24,8 @@ public sealed class AgentMcpSkillsSourceOptions /// another. Set this to a fixed value to get a predictable, reusable extraction location. /// When set, each source must use its own unique directory: the source treats the directory as /// exclusively its own and, on every discovery, prunes any sub-directory that the MCP server no - /// longer advertises. Pointing two sources at the same directory would therefore cause them to + /// longer advertises or whose index entry is not actionable (e.g., missing a required field). + /// Pointing two sources at the same directory would therefore cause them to /// delete each other's extracted skills. /// public string? ArchiveSkillsDirectory { get; set; }