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
29 changes: 10 additions & 19 deletions src/SkillServer/Endpoints.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,43 +228,34 @@ private static async Task<IResult> UploadSkill(
});
}

var referenceFiles = request.Form.Files.GetFiles("references");
var hasReferences = referenceFiles.Count > 0;

if (hasReferences)
var resourceFiles = request.Form.Files.GetFiles("resources");
var resources = new List<(ResourcePath Path, Stream Content)>();
try
{
var resources = new List<(ResourcePath Path, Stream Content)>();
foreach (var refFile in referenceFiles)
foreach (var resourceFile in resourceFiles)
{
var relativePath = $"references/{refFile.FileName}";
if (!ResourcePath.TryCreate(relativePath, out var resourcePath))
if (!ResourcePath.TryCreate(resourceFile.FileName, out var resourcePath))
{
return Results.BadRequest(new ErrorResponse
{
Error = "invalid_resource_path",
Message = $"Invalid resource path: '{relativePath}'. Must be in references/, scripts/, or assets/ directories."
Message = $"Invalid resource path: '{resourceFile.FileName}'. Must be a relative path in a subdirectory with no path traversal."
});
}

resources.Add((resourcePath.Value, refFile.OpenReadStream()));
resources.Add((resourcePath.Value, resourceFile.OpenReadStream()));
}

await using var stream = file.OpenReadStream();
var result = await uploadService.UploadSkillWithResourcesAsync(
skillName.Value, skillVersion.Value, stream, resources, category, ct);

foreach (var (_, content) in resources)
await content.DisposeAsync();

return HandleUploadResult(result, configuration);
}
else
finally
{
await using var stream = file.OpenReadStream();
var result = await uploadService.UploadSkillMdAsync(
skillName.Value, skillVersion.Value, stream, category, ct);

return HandleUploadResult(result, configuration);
foreach (var (_, content) in resources)
await content.DisposeAsync();
}
}

Expand Down
10 changes: 3 additions & 7 deletions src/SkillServer/Models/ValueObjects.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ public static Sha256Digest FromHex(string hexValue)
/// </summary>
public readonly record struct ResourcePath
{
private static readonly string[] AllowedPrefixes = ["references/", "scripts/", "assets/"];

public string Value { get; }

private ResourcePath(string value) => Value = value;
Expand All @@ -157,15 +155,13 @@ public static bool TryCreate(string? value, [NotNullWhen(true)] out ResourcePath
if (string.IsNullOrWhiteSpace(value))
return false;

// No path traversal
if (value.Contains("..") || value.StartsWith('/') || value.StartsWith('\\'))
return false;

// Normalize separators
var normalized = value.Replace('\\', '/');

// Must be in allowed directories
if (!AllowedPrefixes.Any(p => normalized.StartsWith(p, StringComparison.OrdinalIgnoreCase)))
// Must be in a subdirectory — bare filenames at the root are not resources
if (!normalized.Contains('/') || normalized.IndexOf('/') == normalized.Length - 1)
return false;

result = new ResourcePath(normalized);
Expand All @@ -175,7 +171,7 @@ public static bool TryCreate(string? value, [NotNullWhen(true)] out ResourcePath
public static ResourcePath Create(string value)
{
if (!TryCreate(value, out var result))
throw new ArgumentException($"Invalid resource path: '{value}'. Must be in references/, scripts/, or assets/ directories with no path traversal.", nameof(value));
throw new ArgumentException($"Invalid resource path: '{value}'. Must be a relative path in a subdirectory with no path traversal.", nameof(value));
return result.Value;
}

Expand Down
77 changes: 51 additions & 26 deletions tests/SkillServer.Integration.Tests/SkillServerIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -608,18 +608,18 @@ public async Task SearchSkills_WithPorterStemming_MatchesStemmedTerms()
}

[Fact]
public async Task UploadSkillWithReferences_EndToEnd()
public async Task UploadSkillWithResources_EndToEnd()
{
var ct = TestContext.Current.CancellationToken;
var skillName = $"ref-test-{Guid.NewGuid():N}"[..20];

var skillContent = $"""
---
name: {skillName}
description: Testing reference file uploads
description: Testing resource file uploads
---

# Reference Upload Test
# Resource Upload Test

See references/guide.md for details.
""";
Expand All @@ -636,13 +636,11 @@ First section content.
Second section content.
""";

var referenceContent2 = """
# Patterns

Common usage patterns for this skill.
var scriptContent = """
#!/bin/bash
echo "setup complete"
""";

// Upload skill with two reference files
using var content = new MultipartFormDataContent();
content.Add(new StringContent(skillName), "name");
content.Add(new StringContent("1.0.0"), "version");
Expand All @@ -653,39 +651,35 @@ Second section content.

var refFileContent = new ByteArrayContent(Encoding.UTF8.GetBytes(referenceContent));
refFileContent.Headers.ContentType = new MediaTypeHeaderValue("text/markdown");
content.Add(refFileContent, "references", "guide.md");
content.Add(refFileContent, "resources", "references/guide.md");

var refFileContent2 = new ByteArrayContent(Encoding.UTF8.GetBytes(referenceContent2));
refFileContent2.Headers.ContentType = new MediaTypeHeaderValue("text/markdown");
content.Add(refFileContent2, "references", "patterns.md");
var scriptFileContent = new ByteArrayContent(Encoding.UTF8.GetBytes(scriptContent));
scriptFileContent.Headers.ContentType = new MediaTypeHeaderValue("application/x-sh");
content.Add(scriptFileContent, "resources", "scripts/setup.sh");

var uploadResponse = await _fixture.AuthenticatedHttpClient.PostAsync("/skills", content, ct);
Assert.Equal(HttpStatusCode.Created, uploadResponse.StatusCode);

// Verify the version shows file count (SKILL.md + 2 references)
var version = await _fixture.Client.GetVersionAsync(skillName, "1.0.0", ct);
Assert.NotNull(version);
Assert.Equal(2, version.FileCount);

// Download SKILL.md
var downloadedSkill = await _fixture.Client.GetSkillFileAsStringAsync(skillName, "1.0.0", ct: ct);
Assert.Contains("# Reference Upload Test", downloadedSkill);
Assert.Contains("# Resource Upload Test", downloadedSkill);

// Download reference files via resource endpoint
var refResponse = await _fixture.HttpClient.GetAsync(
$"/skills/{skillName}/1.0.0/references/guide.md", ct);
Assert.Equal(HttpStatusCode.OK, refResponse.StatusCode);
var refBody = await refResponse.Content.ReadAsStringAsync(ct);
Assert.Contains("# Guide", refBody);
Assert.Contains("Section One", refBody);

var refResponse2 = await _fixture.HttpClient.GetAsync(
$"/skills/{skillName}/1.0.0/references/patterns.md", ct);
Assert.Equal(HttpStatusCode.OK, refResponse2.StatusCode);
var refBody2 = await refResponse2.Content.ReadAsStringAsync(ct);
Assert.Contains("# Patterns", refBody2);
var scriptResponse = await _fixture.HttpClient.GetAsync(
$"/skills/{skillName}/1.0.0/scripts/setup.sh", ct);
Assert.Equal(HttpStatusCode.OK, scriptResponse.StatusCode);
var scriptBody = await scriptResponse.Content.ReadAsStringAsync(ct);
Assert.Contains("setup complete", scriptBody);

// Verify resources appear in RFC index
var index = await _fixture.Client.GetRfcIndexAsync(ct);
Assert.NotNull(index);
var indexSkill = index.Skills.FirstOrDefault(s => s.Name == skillName);
Expand All @@ -694,22 +688,22 @@ Second section content.
Assert.NotNull(resources);
Assert.Equal(2, resources!.Count);
Assert.Contains(resources, r => r.Path == "references/guide.md");
Assert.Contains(resources, r => r.Path == "references/patterns.md");
Assert.Contains(resources, r => r.Path == "scripts/setup.sh");
}

[Fact]
public async Task UploadSkillWithReferences_WithoutReferences_StillWorks()
public async Task UploadSkillWithResources_WithoutResources_StillWorks()
{
var ct = TestContext.Current.CancellationToken;
var skillName = $"noref-{Guid.NewGuid():N}"[..20];

var skillContent = $"""
---
name: {skillName}
description: Testing upload without references still works
description: Testing upload without resources still works
---

# No References Test
# No Resources Test
""";

using var content = new MultipartFormDataContent();
Expand All @@ -727,4 +721,35 @@ public async Task UploadSkillWithReferences_WithoutReferences_StillWorks()
Assert.NotNull(version);
Assert.Equal(0, version.FileCount);
}

[Fact]
public async Task UploadSkillWithResources_InvalidPath_ReturnsBadRequest()
{
var ct = TestContext.Current.CancellationToken;
var skillName = $"badpath-{Guid.NewGuid():N}"[..20];

var skillContent = $"""
---
name: {skillName}
description: Testing invalid resource path rejection
---

# Bad Path Test
""";

using var content = new MultipartFormDataContent();
content.Add(new StringContent(skillName), "name");
content.Add(new StringContent("1.0.0"), "version");

var fileContent = new ByteArrayContent(Encoding.UTF8.GetBytes(skillContent));
fileContent.Headers.ContentType = new MediaTypeHeaderValue("text/markdown");
content.Add(fileContent, "file", "SKILL.md");

var badFile = new ByteArrayContent(Encoding.UTF8.GetBytes("exploit"));
badFile.Headers.ContentType = new MediaTypeHeaderValue("text/plain");
content.Add(badFile, "resources", "../etc/passwd");

var uploadResponse = await _fixture.AuthenticatedHttpClient.PostAsync("/skills", content, ct);
Assert.Equal(HttpStatusCode.BadRequest, uploadResponse.StatusCode);
}
}
6 changes: 4 additions & 2 deletions tests/SkillServer.Tests/ValueObjectTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,12 @@ public sealed class ResourcePathTests
[InlineData("scripts/setup.sh", true)]
[InlineData("assets/logo.png", true)]
[InlineData("references/deep/nested/file.md", true)]
[InlineData("custom/file.txt", true)]
[InlineData("examples/demo.py", true)]
[InlineData("../secret.txt", false)]
[InlineData("/etc/passwd", false)]
[InlineData("SKILL.md", false)] // Must be in allowed directories
[InlineData("random/file.txt", false)]
[InlineData("SKILL.md", false)] // Bare filenames are not resources
[InlineData("justadirectory/", false)]
[InlineData("", false)]
public void TryCreate_ValidatesCorrectly(string input, bool expectedValid)
{
Expand Down
Loading