Clean up resource upload endpoint

Fix stream leak on validation failure by wrapping resource handling in
try/finally. Remove unnecessary if/else branch — always route through
UploadSkillWithResourcesAsync since it handles empty resource lists.
Rename form field from "references" to "resources" and let clients
specify the full relative path (e.g. references/guide.md, scripts/setup.sh)
so all allowed directories are accessible, not just references/.

Add integration test for path traversal rejection.

Author: Aaronontheweb

src/SkillServer/Endpoints.cs

@@ -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 in references/, scripts/, or assets/ directories."
                     });
                 }
 
-                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();
         }
     }
 

tests/SkillServer.Integration.Tests/SkillServerIntegrationTests.cs

@@ -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.
             """;
@@ -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");
@@ -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);
@@ -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();
@@ -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);
+    }
 }