Clean up resource upload endpoint#53
Merged
Aaronontheweb merged 2 commits intodevfrom Apr 29, 2026
Merged
Conversation
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.
The spec allows any additional files and directories, not just references/, scripts/, and assets/. Replace the hardcoded allowlist in ResourcePath with a check that the path is in any subdirectory. Safety rules remain: no path traversal, no absolute paths, no bare filenames at the root level.
40eded8 to
e84b7e6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
try/finallyeven when validation rejects a file mid-loopif/elsebranch — always route throughUploadSkillWithResourcesAsyncsince it handles empty resource lists correctlyreferencestoresourcesand accept full relative paths as filenames (e.g.references/guide.md,scripts/setup.sh) so clients can upload into any allowed directoryResourcePathvalidation to allow arbitrary subdirectories per the AgentSkills.io spec, which permits any additional files and directories beyond the conventionalreferences/,scripts/, andassets/folders. Safety rules preserved: no path traversal, no absolute paths, no bare root-level filenames.Test plan
UploadSkillWithResources_EndToEnd— uploads resources into bothreferences/andscripts/, verifies download and RFC indexUploadSkillWithResources_WithoutResources_StillWorks— backward compatibilityUploadSkillWithResources_InvalidPath_ReturnsBadRequest— path traversal (../etc/passwd) rejectedResourcePathTests— updated to accept arbitrary subdirectories (custom/file.txt,examples/demo.py) while rejecting bare filenames and trailing-slash-only paths