diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs
index 139efd117e3..141b812262f 100644
--- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs
+++ b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs
@@ -48,6 +48,11 @@ namespace Microsoft.Agents.AI;
/// does not apply to lazily fetched skill-md entries or their supporting resources.
///
///
+/// 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.
+///
+///
/// Thread safety and archive reconciliation. For archive-type skills, every call to
/// 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
diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs
index 2f4c3105132..f9d5dcc5328 100644
--- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/AgentMcpSkillArchiveExtractor.cs
+++ b/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;
///
/// 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.
///
-internal static class AgentMcpSkillArchiveExtractor
+internal static partial class AgentMcpSkillArchiveExtractor
{
///
/// The default maximum number of files that may be extracted from a single archive, sized for a
@@ -77,6 +81,7 @@ internal static ArchiveFormat DetectFormat(byte[] bytes, string? mediaType, stri
/// 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.
+ /// An optional logger reporting each skipped colliding archive member.
/// The format is not .
/// The archive exceeds one of the supplied limits.
internal static void Extract(
@@ -84,7 +89,8 @@ internal static void Extract(
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(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);
}
+
+ ///
+ /// Replaces control characters in a file-system path with ? so the path is safe to include
+ /// in log messages without risking terminal-escape injection.
+ ///
+ 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);
}
diff --git a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs b/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs
index 9b4791747f8..3145554132c 100644
--- a/dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Loaders/ArchiveEntryLoader.cs
+++ b/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.
///
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> 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> 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)
}
}
- ///
- /// 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.
///
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 f9754c34960..6a4ed11b222 100644
--- a/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs
+++ b/dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/AgentMcpSkillsSourceArchiveTests.cs
@@ -242,6 +242,164 @@ public async Task GetSkillsAsync_ArchiveExistingMetadataScalarFormats_PreservedA
Assert.Contains("Body.", await skill.GetContentAsync());
}
+ ///
+ /// Archive member aliases and ordering, content, and digest combinations for collision tests.
+ ///
+ public static TheoryData ArchiveCollisionCases
+ {
+ get
+ {
+ var aliases = new List
+ {
+ "refs/policy.md",
+ ".//refs//policy.md",
+ "refs\\policy.md",
+ "refs/./policy.md",
+ "/refs/policy.md",
+ };
+ if (OperatingSystem.IsWindows())
+ {
+ aliases.Add("refs/POLICY.md");
+ aliases.Add("refs/policy.md.");
+ }
+
+ var cases = new TheoryData();
+ foreach (string alias in aliases)
+ {
+ foreach (bool aliasFirst in new[] { false, true })
+ {
+ foreach (bool identical in new[] { false, true })
+ {
+ foreach (bool withDigest in new[] { false, true })
+ {
+ cases.Add(alias, aliasFirst, identical, withDigest);
+ }
+ }
+ }
+ }
+
+ return cases;
+ }
+ }
+
+ [Theory]
+ [MemberData(nameof(ArchiveCollisionCases))]
+ public async Task GetSkillsAsync_CollidingArchiveResources_KeepsFirstFileAndLogsWarningAsync(
+ string alias, bool aliasFirst, bool identical, bool withDigest)
+ {
+ // Arrange
+ string firstName = aliasFirst ? alias : "refs/policy.md";
+ string secondName = aliasFirst ? "refs/policy.md" : alias;
+ byte[] archive = BuildZip(
+ ("SKILL.md", ArchivedSkillMd),
+ ("refs/", ""),
+ (firstName, "First resource."),
+ (secondName, identical ? "First resource." : "Substituted resource."),
+ (".//refs//other.md", "Unaffected resource."));
+ string index = withDigest
+ ? ArchiveIndexWithDigest(ArchiveDigest(archive))
+ : ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip");
+ await using var server = CreateArchiveServer(index, new Dictionary { ["archived-skill"] = archive });
+ await using var client = await server.CreateClientAsync();
+ var logger = new Mock();
+ logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true);
+ var loggerFactory = new Mock();
+ loggerFactory.Setup(f => f.CreateLogger(It.IsAny())).Returns(logger.Object);
+ var source = new AgentMcpSkillsSource(
+ client, new() { ArchiveSkillsDirectory = this._extractionRoot }, loggerFactory.Object);
+
+ // Act / Assert
+ for (int i = 0; i < 2; i++)
+ {
+ var skill = Assert.Single(await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create()));
+ string resourceName = aliasFirst && alias == "refs/POLICY.md" ? alias : "refs/policy.md";
+ var resource = await skill.GetResourceAsync(resourceName);
+ Assert.NotNull(resource);
+ Assert.Equal("First resource.", await resource.ReadAsync());
+ var other = await skill.GetResourceAsync("refs/other.md");
+ Assert.NotNull(other);
+ Assert.Equal("Unaffected resource.", await other.ReadAsync());
+ }
+
+ logger.Verify(
+ l => l.Log(
+ LogLevel.Warning,
+ It.IsAny(),
+ It.Is((state, _) =>
+ state.ToString()!.Contains("Skipping duplicate archive member", StringComparison.Ordinal) &&
+ state.ToString()!.Contains("keeping the first file", StringComparison.Ordinal) &&
+ !state.ToString()!.Contains("Substituted resource.", StringComparison.Ordinal)),
+ It.IsAny(),
+ It.IsAny>()),
+ Times.Exactly(2));
+ }
+
+ [Theory]
+ [InlineData("SKILL.md", false)]
+ [InlineData("SKILL.md", true)]
+ [InlineData(".//SKILL.md", false)]
+ [InlineData(".//SKILL.md", true)]
+ [InlineData("/SKILL.md", false)]
+ [InlineData("/SKILL.md", true)]
+ public async Task GetSkillsAsync_CollidingArchiveSkillMd_KeepsFirstFileAndLogsWarningAsync(string alias, bool aliasFirst)
+ {
+ // Arrange
+ byte[] archive = BuildZip(
+ (aliasFirst ? alias : "SKILL.md", ArchivedSkillMd),
+ (aliasFirst ? "SKILL.md" : alias, ArchivedSkillMd.Replace("name: archived-skill", "name: other-skill", StringComparison.Ordinal)),
+ ("reference.md", "Unaffected resource."));
+ await using var server = CreateArchiveServer(
+ ArchiveIndex("archived-skill", "skill://archives/archived-skill.zip"),
+ new Dictionary { ["archived-skill"] = archive });
+ await using var client = await server.CreateClientAsync();
+ var logger = new Mock();
+ logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true);
+ var loggerFactory = new Mock();
+ loggerFactory.Setup(f => f.CreateLogger(It.IsAny())).Returns(logger.Object);
+ var source = new AgentMcpSkillsSource(
+ client, new() { ArchiveSkillsDirectory = this._extractionRoot }, loggerFactory.Object);
+
+ // Act
+ var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create());
+
+ // Assert
+ var skill = Assert.Single(skills);
+ Assert.Equal("archived-skill", skill.Frontmatter.Name);
+ Assert.Contains("Body from the archive.", await skill.GetContentAsync());
+ logger.Verify(
+ l => l.Log(
+ LogLevel.Warning,
+ It.IsAny(),
+ It.Is((state, _) =>
+ state.ToString()!.Contains("Skipping duplicate archive member", StringComparison.Ordinal)),
+ It.IsAny(),
+ It.IsAny>()),
+ Times.Once);
+ }
+
+ [Fact]
+ public async Task GetSkillsAsync_ArchiveCollision_DoesNotSkipOtherSkillsAsync()
+ {
+ // Arrange
+ byte[] archiveA = BuildZip(("SKILL.md", SkillAMd), ("reference.md", "First."), ("./reference.md", "Second."));
+ byte[] archiveB = BuildZip(("SKILL.md", SkillBMd));
+ JsonNode index = JsonNode.Parse(ArchiveIndex("skill-a", "skill://archives/skill-a.zip"))!;
+ index["skills"]!.AsArray().Add(
+ JsonNode.Parse(ArchiveIndex("skill-b", "skill://archives/skill-b.zip"))!["skills"]![0]!.DeepClone());
+ await using var server = CreateArchiveServer(
+ index.ToJsonString(), new Dictionary { ["skill-a"] = archiveA, ["skill-b"] = archiveB });
+ await using var client = await server.CreateClientAsync();
+ var source = new AgentMcpSkillsSource(client, new() { ArchiveSkillsDirectory = this._extractionRoot });
+
+ // Act
+ var skills = await source.GetSkillsAsync(TestAgentSkillsSourceContextFactory.Create());
+
+ // Assert
+ Assert.Equal(2, skills.Count);
+ Assert.Contains(skills, skill => skill.Frontmatter.Name == "skill-a");
+ Assert.Contains(skills, skill => skill.Frontmatter.Name == "skill-b");
+ }
+
///
/// Malformed, unsupported, and mismatched archive digests that must be rejected.
///
@@ -763,6 +921,72 @@ public void Extract_ArchiveExceedsDefaultFileCount_Throws()
() => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target));
}
+ [Fact]
+ public void Extract_DuplicateMembers_CountTowardFileLimit()
+ {
+ // Arrange
+ byte[] zip = BuildZip(("reference.md", "First."), ("reference.md", "Second."));
+ string target = Path.Combine(this._extractionRoot, "skill");
+
+ // Act / Assert
+ Assert.Throws(
+ () => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target, maxFileCount: 1));
+ }
+
+ [Fact]
+ public void Extract_SkippedDuplicate_IsNotDecompressed()
+ {
+ // Arrange
+ byte[] zip = BuildZip(("reference.md", "First."), ("./reference.md", new string('x', 100)));
+ string target = Path.Combine(this._extractionRoot, "skill");
+ var logger = new Mock();
+ logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true);
+
+ // Act
+ AgentMcpSkillArchiveExtractor.Extract(
+ zip, ArchiveFormat.Zip, target, maxUncompressedSizeBytes: Encoding.UTF8.GetByteCount("First."), logger: logger.Object);
+
+ // Assert
+ Assert.Equal("First.", File.ReadAllText(Path.Combine(target, "reference.md")));
+ VerifyArchiveMemberCollision(logger, "./reference.md", Times.Once());
+ }
+
+ [Fact]
+ public void Extract_ExistingFile_IsNotOverwritten()
+ {
+ // Arrange
+ string target = Path.Combine(this._extractionRoot, "skill");
+ Directory.CreateDirectory(target);
+ File.WriteAllText(Path.Combine(target, "reference.md"), "Existing.");
+ byte[] zip = BuildZip(("reference.md", "Substituted."));
+ var logger = new Mock();
+ logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true);
+
+ // Act
+ AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target, logger: logger.Object);
+
+ // Assert
+ Assert.Equal("Existing.", File.ReadAllText(Path.Combine(target, "reference.md")));
+ VerifyArchiveMemberCollision(logger, "reference.md", Times.Once());
+ }
+
+ [Fact]
+ public void Extract_FileDirectoryConflict_IsNotSkippedAsDuplicate()
+ {
+ // Arrange
+ string target = Path.Combine(this._extractionRoot, "skill");
+ Directory.CreateDirectory(Path.Combine(target, "reference.md"));
+ byte[] zip = BuildZip(("reference.md", "Content."));
+ var logger = new Mock();
+ logger.Setup(l => l.IsEnabled(It.IsAny())).Returns(true);
+
+ // Act / Assert
+ Exception? exception = Record.Exception(
+ () => AgentMcpSkillArchiveExtractor.Extract(zip, ArchiveFormat.Zip, target, logger: logger.Object));
+ Assert.True(exception is IOException or UnauthorizedAccessException);
+ VerifyArchiveMemberCollision(logger, "reference.md", Times.Never());
+ }
+
[Fact]
public void Extract_ArchiveExceedsDefaultUncompressedSize_Throws()
{
@@ -1043,6 +1267,18 @@ public void Dispose()
}
}
+ private static void VerifyArchiveMemberCollision(Mock logger, string memberPath, Times times) =>
+ logger.Verify(
+ l => l.Log(
+ LogLevel.Warning,
+ It.IsAny(),
+ It.Is((state, _) =>
+ state.ToString()!.Contains($"Skipping duplicate archive member '{memberPath}'", StringComparison.Ordinal) &&
+ state.ToString()!.Contains("keeping the first file", StringComparison.Ordinal)),
+ It.IsAny(),
+ It.IsAny>()),
+ times);
+
private static byte[] BuildZip(params (string Path, string Content)[] entries)
{
using var ms = new MemoryStream();
diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py
index 71973281c25..3bef5b39424 100644
--- a/python/packages/core/agent_framework/_skills.py
+++ b/python/packages/core/agent_framework/_skills.py
@@ -4993,6 +4993,8 @@ def _extract_archive_to_memory(
``..`` parent-traversal ("zip-slip") aborts extraction of the whole archive by
raising. Extraction is bounded by a maximum file count and total uncompressed size
to mitigate decompression-bomb attacks. No filesystem is touched.
+ Later members whose normalized paths match case-insensitively are skipped
+ with a warning; the first member's spelling and content are retained.
Args:
data: The raw archive bytes.
@@ -5023,6 +5025,7 @@ def _extract_zip_to_memory(
"""Read regular files from a ZIP archive into memory. See :func:`_extract_archive_to_memory`."""
remaining_bytes = max_uncompressed_size_bytes
files: dict[str, bytes] = {}
+ seen_names: set[str] = set()
file_count = 0
for info in archive.infolist():
@@ -5037,9 +5040,21 @@ def _extract_zip_to_memory(
if name is None:
continue
+ # Match FileSkill's case-insensitive lookup; retain the first member's spelling.
+ lookup_name = name.lower()
+ if lookup_name in seen_names:
+ logger.warning(
+ "Skipping duplicate archive member %r: normalized path %r matches an earlier member; "
+ "keeping the first file",
+ info.filename,
+ name,
+ )
+ continue
+
with archive.open(info) as source:
content, remaining_bytes = _read_member_with_limit(source, remaining_bytes)
files[name] = content
+ seen_names.add(lookup_name)
return files
@@ -5059,6 +5074,8 @@ class _ArchiveEntryLoader:
Extraction is hardened against path-traversal ("zip-slip") member names,
oversized downloads, excessive file counts, and decompression bombs.
+ Member paths are compared case-insensitively; collisions are skipped with a
+ warning, keeping the first file's spelling and content.
Supplied SHA-256 digests are verified before extraction; archives without
a digest remain supported.
"""
@@ -5333,6 +5350,10 @@ class MCPSkillsSource(SkillsSource):
archive are surfaced as read-only resources only; MCP-delivered scripts
are never discovered as runnable.
+ Archive member paths are compared case-insensitively, matching resource lookup.
+ The first file's spelling and content are retained; later colliding members
+ are skipped with a warning.
+
Entries whose type has no registered handler (e.g. ``mcp-resource-template``)
are skipped.
diff --git a/python/packages/core/tests/core/test_mcp_skills.py b/python/packages/core/tests/core/test_mcp_skills.py
index aecdd6f0ced..6b9bf0f9060 100644
--- a/python/packages/core/tests/core/test_mcp_skills.py
+++ b/python/packages/core/tests/core/test_mcp_skills.py
@@ -8,8 +8,9 @@
import hashlib
import io
import json
+import warnings
import zipfile
-from collections.abc import Iterator
+from collections.abc import Iterator, Mapping, Sequence
from datetime import timedelta
from unittest.mock import AsyncMock, patch
from urllib.parse import unquote
@@ -895,12 +896,15 @@ async def test_index_timeout_error_propagates(self) -> None:
# ---------------------------------------------------------------------------
-def _make_zip(files: dict[str, bytes]) -> bytes:
- """Build an in-memory ZIP archive from a ``{path: content}`` mapping."""
+def _make_zip(files: Mapping[str, bytes] | Sequence[tuple[str, bytes]]) -> bytes:
+ """Build an in-memory ZIP from a mapping or ordered entries, including duplicate names."""
buffer = io.BytesIO()
- with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as archive:
- for name, data in files.items():
- archive.writestr(name, data)
+ entries = files.items() if isinstance(files, Mapping) else files
+ with warnings.catch_warnings():
+ warnings.filterwarnings("ignore", message="Duplicate name:", category=UserWarning, module="zipfile")
+ with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as archive:
+ for name, data in entries:
+ archive.writestr(name, data)
return buffer.getvalue()
@@ -1354,6 +1358,100 @@ async def test_zip_slip_archive_skips_whole_skill(self) -> None:
assert skills == []
+class TestMCPSkillsSourceArchiveCollisions:
+ """Colliding archive members warn and preserve the first file through discovery."""
+
+ @pytest.mark.parametrize(
+ "alias",
+ [
+ "refs/policy.md",
+ ".//refs//policy.md",
+ "refs\\policy.md",
+ "refs/./policy.md",
+ "/refs/policy.md",
+ "refs/POLICY.md",
+ "REFS\\POLICY.MD",
+ ],
+ )
+ @pytest.mark.parametrize("alias_first", [False, True])
+ @pytest.mark.parametrize("identical", [False, True])
+ @pytest.mark.parametrize("with_digest", [False, True])
+ async def test_colliding_resources_keep_first_file_and_warn(
+ self, alias: str, alias_first: bool, identical: bool, with_digest: bool, caplog: pytest.LogCaptureFixture
+ ) -> None:
+ url = "skill://archives/packaged-skill.zip"
+ first_name, second_name = (alias, "refs/policy.md") if alias_first else ("refs/policy.md", alias)
+ archive = _make_zip([
+ ("SKILL.md", ARCHIVE_SKILL_MD.encode()),
+ ("refs/", b""),
+ (first_name, b"First resource."),
+ (second_name, b"First resource." if identical else b"Substituted resource."),
+ (".//refs//other.md", b"Unaffected resource."),
+ ])
+ digest = f"sha256:{hashlib.sha256(archive).hexdigest()}" if with_digest else None
+ index = _make_archive_index("packaged-skill", url, digest=digest)
+ source = MCPSkillsSource(client=_archive_client(index, url, archive, "application/zip"))
+
+ for _ in range(2):
+ skills = await source.get_skills(_SOURCE_CTX)
+
+ assert len(skills) == 1
+ for resource_name in ("refs/policy.md", "REFS/POLICY.MD"):
+ resource = await skills[0].get_resource(resource_name)
+ assert resource is not None
+ assert await resource.read() == "First resource."
+ other = await skills[0].get_resource("refs/other.md")
+ assert other is not None
+ assert await other.read() == "Unaffected resource."
+
+ collisions = [record for record in caplog.records if "duplicate archive member" in record.getMessage()]
+ assert len(collisions) == 2
+ assert all(record.levelname == "WARNING" for record in collisions)
+ assert all("keeping the first file" in record.getMessage() for record in collisions)
+ assert all("Substituted resource." not in record.getMessage() for record in collisions)
+
+ @pytest.mark.parametrize("alias", ["SKILL.md", ".//SKILL.md", "/SKILL.md", "skill.md", "Skill.MD"])
+ @pytest.mark.parametrize("alias_first", [False, True])
+ async def test_colliding_skill_md_keeps_first_file_and_warns(
+ self, alias: str, alias_first: bool, caplog: pytest.LogCaptureFixture
+ ) -> None:
+ url = "skill://archives/packaged-skill.zip"
+ first_name, second_name = (alias, "SKILL.md") if alias_first else ("SKILL.md", alias)
+ archive = _make_zip([
+ (first_name, ARCHIVE_SKILL_MD.encode()),
+ (second_name, ARCHIVE_SKILL_MD.replace("name: packaged-skill", "name: other-skill").encode()),
+ ("reference.md", b"Unaffected resource."),
+ ])
+ index = _make_archive_index("packaged-skill", url)
+
+ skills = await MCPSkillsSource(client=_archive_client(index, url, archive, "application/zip")).get_skills(
+ _SOURCE_CTX
+ )
+
+ assert len(skills) == 1
+ assert skills[0].frontmatter.name == "packaged-skill"
+ assert "Instructions from an archive." in await skills[0].get_content()
+ assert len(caplog.records) == 1
+ assert caplog.records[0].levelname == "WARNING"
+ assert "duplicate archive member" in caplog.text
+
+ async def test_collision_does_not_skip_other_skills(self, caplog: pytest.LogCaptureFixture) -> None:
+ url = "skill://archives/packaged-skill.zip"
+ index_data = json.loads(SAMPLE_SKILL_INDEX)
+ index_data["skills"].extend(json.loads(_make_archive_index("packaged-skill", url))["skills"])
+ archive = _make_zip([
+ ("SKILL.md", ARCHIVE_SKILL_MD.encode()),
+ ("reference.md", b"First."),
+ ("./reference.md", b"Second."),
+ ])
+ client = _archive_client(json.dumps(index_data), url, archive, "application/zip")
+
+ skills = await MCPSkillsSource(client=client).get_skills(_SOURCE_CTX)
+
+ assert sorted(skill.frontmatter.name for skill in skills) == ["packaged-skill", "unit-converter"]
+ assert "duplicate archive member" in caplog.text
+
+
class TestMCPSkillsSourceArchiveDigest:
"""Tests for archive digest verification through the MCP discovery pipeline."""
@@ -1658,6 +1756,60 @@ def test_file_count_limit_is_enforced(self) -> None:
with pytest.raises(ValueError, match="file count"):
_extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 2, 1024 * 1024)
+ def test_duplicate_members_count_toward_file_limit(self) -> None:
+ from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory
+
+ archive = _make_zip([("reference.md", b"First."), ("reference.md", b"Second.")])
+
+ with pytest.raises(ValueError, match="file count"):
+ _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 1, 1024 * 1024)
+
+ def test_skipped_duplicate_is_not_decompressed(self, caplog: pytest.LogCaptureFixture) -> None:
+ from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory, _read_member_with_limit
+
+ archive = _make_zip([("reference.md", b"First."), ("./reference.md", b"x" * 100)])
+ with patch("agent_framework._skills._read_member_with_limit", wraps=_read_member_with_limit) as read:
+ files = _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, len(b"First."))
+
+ assert files == {"reference.md": b"First."}
+ read.assert_called_once()
+ assert "duplicate archive member" in caplog.text
+
+ @pytest.mark.parametrize("first_name", ["reference.md", "REFERENCE.md"])
+ def test_case_collision_keeps_first_name_and_warns(self, first_name: str, caplog: pytest.LogCaptureFixture) -> None:
+ from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory
+
+ second_name = "REFERENCE.md" if first_name == "reference.md" else "reference.md"
+ archive = _make_zip([(first_name, b"First."), (second_name, b"Second.")])
+
+ files = _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, 1024 * 1024)
+
+ assert files == {first_name: b"First."}
+ assert len(caplog.records) == 1
+ assert caplog.records[0].levelname == "WARNING"
+ assert "duplicate archive member" in caplog.text
+
+ def test_distinct_lowercase_resource_names_are_preserved(self, caplog: pytest.LogCaptureFixture) -> None:
+ from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory
+
+ # Resource lookup uses lower(), not Unicode casefold().
+ archive = _make_zip({"stra\u00dfe.md": b"First.", "STRASSE.md": b"Second."})
+
+ files = _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, 1024 * 1024)
+
+ assert files == {"stra\u00dfe.md": b"First.", "STRASSE.md": b"Second."}
+ assert not caplog.records
+
+ def test_distinct_trailing_dot_names_are_preserved(self, caplog: pytest.LogCaptureFixture) -> None:
+ from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory
+
+ archive = _make_zip({"reference.md": b"First.", "reference.md.": b"Second."})
+
+ files = _extract_archive_to_memory(archive, _ArchiveFormat.ZIP, 20, 1024 * 1024)
+
+ assert files == {"reference.md": b"First.", "reference.md.": b"Second."}
+ assert not caplog.records
+
def test_uncompressed_size_limit_is_enforced(self) -> None:
from agent_framework._skills import _ArchiveFormat, _extract_archive_to_memory