Python: Improve MCP skill archive loading consistency

dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs

@@ -48,6 +48,11 @@ namespace Microsoft.Agents.AI;
 /// does not apply to lazily fetched <c>skill-md</c> entries or their supporting resources.
 /// </para>
 /// <para>
+/// Archive members resolving to the same file keep the first file's content. Later colliding members
+/// are skipped with a warning, and discovery continues with the remaining archive content.
+/// This applies whether or not an archive supplies a matching digest.
+/// </para>
+/// <para>
 /// <b>Thread safety and archive reconciliation.</b> For <c>archive</c>-type skills, every call to
 /// <see cref="GetSkillsAsync"/> reconciles a shared on-disk directory: it extracts newly advertised
 /// skills, re-extracts existing ones, and prunes those the server no longer advertises. Because that

dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs

@@ -2,8 +2,11 @@
 
 using System;
 using System.Buffers;
+using System.Collections.Generic;
 using System.IO;
 using System.IO.Compression;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
 
 namespace Microsoft.Agents.AI;
 
@@ -13,8 +16,9 @@ namespace Microsoft.Agents.AI;
 /// <remarks>
 /// Supports ZIP payloads. Extraction is guarded against path-traversal ("zip-slip") attacks and
 /// bounded by a maximum file count and total uncompressed size to mitigate decompression-bomb attacks.
+/// Later entries resolving to an existing file are skipped, preserving the first file's content.
 /// </remarks>
-internal static class AgentMcpSkillArchiveExtractor
+internal static partial class AgentMcpSkillArchiveExtractor
 {
     /// <summary>
     /// The default maximum number of files that may be extracted from a single archive, sized for a
@@ -77,14 +81,16 @@ internal static ArchiveFormat DetectFormat(byte[] bytes, string? mediaType, stri
     /// <param name="targetDirectory">The directory the archive is unpacked into. Created if missing.</param>
     /// <param name="maxFileCount">The maximum number of files that may be extracted from the archive.</param>
     /// <param name="maxUncompressedSizeBytes">The maximum total uncompressed size, in bytes, of all extracted files.</param>
+    /// <param name="logger">An optional logger reporting each skipped colliding archive member.</param>
     /// <exception cref="NotSupportedException">The format is not <see cref="ArchiveFormat.Zip"/>.</exception>
     /// <exception cref="InvalidDataException">The archive exceeds one of the supplied limits.</exception>
     internal static void Extract(
         byte[] bytes,
         ArchiveFormat format,
         string targetDirectory,
         int? maxFileCount = null,
-        long? maxUncompressedSizeBytes = null)
+        long? maxUncompressedSizeBytes = null,
+        ILogger? logger = null)
     {
         maxFileCount ??= DefaultMaxFileCount;
         maxUncompressedSizeBytes ??= DefaultMaxUncompressedSizeBytes;
@@ -98,15 +104,17 @@ internal static void Extract(
         string fullTarget = Path.GetFullPath(targetDirectory);
 
         using var source = new MemoryStream(bytes, writable: false);
-        ExtractZip(source, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value);
+        ExtractZip(source, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value, logger ?? NullLogger.Instance);
     }
 
-    private static void ExtractZip(Stream source, string fullTarget, int maxFileCount, long maxUncompressedSizeBytes)
+    private static void ExtractZip(Stream source, string fullTarget, int maxFileCount, long maxUncompressedSizeBytes, ILogger logger)
     {
         using var archive = new ZipArchive(source, ZipArchiveMode.Read, leaveOpen: true);
 
         long remainingBytes = maxUncompressedSizeBytes;
         int fileCount = 0;
+        var destinations = new HashSet<string>(OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal);
+        string skillName = SanitizePathForLog(Path.GetFileName(fullTarget));
 
         foreach (ZipArchiveEntry entry in archive.Entries)
         {
@@ -127,11 +135,34 @@ private static void ExtractZip(Stream source, string fullTarget, int maxFileCoun
                 continue;
             }
 
+            // Normalization can map distinct archive names to one path; keep the first file.
+            if (!destinations.Add(destination))
+            {
+                LogArchiveMemberCollision(logger, skillName, SanitizePathForLog(entry.FullName));
+                continue;
+            }
+
             Directory.CreateDirectory(Path.GetDirectoryName(destination)!);
 
-            using Stream entryStream = entry.Open();
-            using FileStream output = File.Create(destination);
-            CopyWithLimit(entryStream, output, ref remainingBytes);
+            // CreateNew also prevents overwrites through filesystem aliases, such as trailing dots on Windows.
+            FileStream output;
+            try
+            {
+#pragma warning disable CA5389 // ResolveDestination canonicalizes the path and rejects destinations outside fullTarget.
+                output = new FileStream(destination, FileMode.CreateNew, FileAccess.Write, FileShare.None);
+#pragma warning restore CA5389
+            }
+            catch (IOException) when (File.Exists(destination))
+            {
+                LogArchiveMemberCollision(logger, skillName, SanitizePathForLog(entry.FullName));
+                continue;
+            }
+
+            using (output)
+            {
+                using Stream entryStream = entry.Open();
+                CopyWithLimit(entryStream, output, ref remainingBytes);
+            }
         }
     }
 
@@ -212,4 +243,26 @@ internal static bool IsPathContainedIn(string parentDirectory, string candidateP
             prefix,
             OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal);
     }
+
+    /// <summary>
+    /// Replaces control characters in a file-system path with <c>?</c> so the path is safe to include
+    /// in log messages without risking terminal-escape injection.
+    /// </summary>
+    internal 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);
+    }
+
+    [LoggerMessage(LogLevel.Warning, "Skipping duplicate archive member '{MemberPath}' for skill '{SkillName}'; keeping the first file.")]
+    private static partial void LogArchiveMemberCollision(ILogger logger, string skillName, string memberPath);
 }

dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs

@@ -26,6 +26,8 @@ namespace Microsoft.Agents.AI;
 /// inside an archive are surfaced as readable resources only; they are never discovered as
 /// executable scripts. Supplied SHA-256 digests are verified before extraction; archives without
 /// a digest remain supported.
+/// Archive members resolving to the same file keep the first file; later colliding members are
+/// skipped with a warning.
 /// </remarks>
 internal sealed partial class ArchiveEntryLoader : IMcpSkillEntryLoader, IDisposable
 {
@@ -146,7 +148,7 @@ private void ReconcileArchiveSkillDirectories(string? archiveSkillsDirectory, Li
             try
             {
                 Directory.Delete(directory, recursive: true);
-                LogArchiveSkillPruned(this._logger, name, SanitizePathForLog(directory));
+                LogArchiveSkillPruned(this._logger, name, AgentMcpSkillArchiveExtractor.SanitizePathForLog(directory));
             }
             catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
             {
@@ -240,7 +242,8 @@ private async Task<IReadOnlyList<string>> TryDownloadAndExtractSkillAsync(McpSki
                 format,
                 skillDirectory,
                 this._options?.ArchiveMaxFileCount,
-                this._options?.ArchiveMaxUncompressedSizeBytes);
+                this._options?.ArchiveMaxUncompressedSizeBytes,
+                logger: this._logger);
         }
         catch (Exception ex)
         {
@@ -252,7 +255,7 @@ private async Task<IReadOnlyList<string>> TryDownloadAndExtractSkillAsync(McpSki
             return [];
         }
 
-        LogArchiveExtracted(this._logger, entry.Name!, SanitizePathForLog(skillDirectory));
+        LogArchiveExtracted(this._logger, entry.Name!, AgentMcpSkillArchiveExtractor.SanitizePathForLog(skillDirectory));
 
         return [skillDirectory];
     }
@@ -393,25 +396,6 @@ static bool IsValidDirectoryName(string value, char[] invalidChars)
         }
     }
 
-    /// <summary>
-    /// Replaces control characters in a file-system path with <c>?</c> so the path is safe to include
-    /// in log messages without risking terminal-escape injection.
-    /// </summary>
-    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);
-    }
-
     /// <summary>
     /// Attempts to recursively delete a skill directory.
     /// </summary>