Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

using System;
using System.Buffers;
using System.Formats.Tar;
using System.IO;
using System.IO.Compression;

Expand All @@ -12,11 +11,8 @@ namespace Microsoft.Agents.AI;
/// Unpacks skill archives downloaded from an MCP server into a local directory.
/// </summary>
/// <remarks>
/// 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.
/// 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.
/// </remarks>
internal static class AgentMcpSkillArchiveExtractor
{
Expand Down Expand Up @@ -45,9 +41,10 @@ internal static class AgentMcpSkillArchiveExtractor
internal static ArchiveFormat DetectFormat(byte[] bytes, string? mediaType, string? url)
{
// Magic-number sniffing is the most reliable signal.
// Reject gzip by signature before considering potentially incorrect MIME type or URL hints.
if (bytes.Length >= 2 && bytes[0] == 0x1F && bytes[1] == 0x8B)
{
return ArchiveFormat.TarGz;
return ArchiveFormat.Unknown;
Comment thread
SergeyMenshykh marked this conversation as resolved.
}

if (bytes.Length >= 4 && bytes[0] == 0x50 && bytes[1] == 0x4B &&
Expand All @@ -63,35 +60,12 @@ internal static ArchiveFormat DetectFormat(byte[] bytes, string? mediaType, stri
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;
}

Expand All @@ -103,7 +77,7 @@ 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>
/// <exception cref="NotSupportedException">The format is <see cref="ArchiveFormat.Unknown"/>.</exception>
/// <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,
Expand All @@ -115,28 +89,16 @@ internal static void Extract(
maxFileCount ??= DefaultMaxFileCount;
maxUncompressedSizeBytes ??= DefaultMaxUncompressedSizeBytes;

if (format != ArchiveFormat.Zip)
{
throw new NotSupportedException($"Unsupported skill archive format '{format}'. Use ZIP instead.");
}

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}'.");
}
ExtractZip(source, fullTarget, maxFileCount.Value, maxUncompressedSizeBytes.Value);
}

private static void ExtractZip(Stream source, string fullTarget, int maxFileCount, long maxUncompressedSizeBytes)
Expand Down Expand Up @@ -173,39 +135,6 @@ private static void ExtractZip(Stream source, string fullTarget, int maxFileCoun
}
}

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);
}
}

/// <summary>
/// Copies <paramref name="source"/> to <paramref name="destination"/> while decrementing a shared
/// uncompressed-byte budget, throwing once it is exhausted. This is the authoritative defense against
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ namespace Microsoft.Agents.AI;

/// <summary>
/// Loads <c>archive</c> index entries: each entry's <c>url</c> points to a single archive resource
/// (<c>application/zip</c>, <c>application/x-tar</c>, 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 <see cref="AgentFileSkillsSource"/> that this
/// loader proxies to.
/// in ZIP format 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
/// <see cref="AgentFileSkillsSource"/> that this loader proxies to.
/// </summary>
/// <remarks>
/// Because MCP-delivered skills are treated strictly as instructor-format text, scripts bundled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,4 @@ internal enum ArchiveFormat

/// <summary>A ZIP archive.</summary>
Zip,

/// <summary>An uncompressed TAR archive.</summary>
Tar,

/// <summary>A gzip-compressed TAR archive (<c>.tar.gz</c>/<c>.tgz</c>).</summary>
TarGz,
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

using System;
using System.Collections.Generic;
using System.Formats.Tar;
using System.IO;
using System.IO.Compression;
using System.Linq;
Expand Down Expand Up @@ -78,7 +77,7 @@ public async Task GetSkillsAsync_ZipArchive_DiscoversSkillAsync()
}

[Fact]
public async Task GetSkillsAsync_TarGzArchive_DiscoversSkillAsync()
public async Task GetSkillsAsync_TarGzArchive_SkipsSkillAsync()
{
// Arrange
await using var server = new InMemoryMcpServer(builder => builder.WithResources<TarGzArchiveServer>());
Expand All @@ -90,9 +89,26 @@ public async Task GetSkillsAsync_TarGzArchive_DiscoversSkillAsync()
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());
Assert.Empty(skills);
}

[Fact]
public void DetectFormat_TarSignals_ReturnUnknown()
{
// Arrange / Act / Assert - TAR signals never become a supported archive format,
// including when weaker metadata claims that gzip data is ZIP.
Assert.Equal(
ArchiveFormat.Unknown,
AgentMcpSkillArchiveExtractor.DetectFormat([0x1F, 0x8B], "application/zip", "skill://archive.zip"));
Assert.Equal(
ArchiveFormat.Unknown,
AgentMcpSkillArchiveExtractor.DetectFormat([], "application/x-tar", null));
Assert.Equal(
ArchiveFormat.Unknown,
AgentMcpSkillArchiveExtractor.DetectFormat([], null, "skill://archive.tar"));
Assert.Equal(
ArchiveFormat.Unknown,
AgentMcpSkillArchiveExtractor.DetectFormat([], null, "skill://archive.tgz"));
}

[Fact]
Expand Down Expand Up @@ -329,42 +345,16 @@ public void Extract_ArchiveExceedsDefaultUncompressedSize_Throws()
}

[Fact]
public void Extract_TarGzExceedsUncompressedSize_Throws()
public void Extract_UnknownFormat_ThrowsBeforeCreatingTarget()
{
// 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));
// Arrange
string target = Path.Combine(this._extractionRoot, "skill");

// Act / Assert
Assert.Throws<InvalidDataException>(
() => 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));
var exception = Assert.Throws<NotSupportedException>(
() => AgentMcpSkillArchiveExtractor.Extract([], ArchiveFormat.Unknown, target));
Assert.Contains("Use ZIP instead", exception.Message);
Assert.False(Directory.Exists(target));
}

[Fact]
Expand Down Expand Up @@ -638,40 +628,6 @@ private static byte[] BuildZip(params (string Path, string Content)[] entries)
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",
Expand Down Expand Up @@ -711,7 +667,7 @@ private sealed class TarGzArchiveServer

[McpServerResource(UriTemplate = "skill://archives/archived-skill.tar.gz", Name = "archive", MimeType = "application/gzip")]
public static BlobResourceContents Archive() => BlobResourceContents.FromBytes(
BuildTarGz(("SKILL.md", ArchivedSkillMd)),
new byte[] { 0x1F, 0x8B },
"skill://archives/archived-skill.tar.gz",
"application/gzip");
}
Expand Down
Loading
Loading