Skip to content

Clean up resource upload endpoint#53

Merged
Aaronontheweb merged 2 commits intodevfrom
fix/upload-endpoint-cleanup
Apr 29, 2026
Merged

Clean up resource upload endpoint#53
Aaronontheweb merged 2 commits intodevfrom
fix/upload-endpoint-cleanup

Conversation

@Aaronontheweb
Copy link
Copy Markdown
Contributor

Summary

  • Fix stream leak in upload endpoint — resource streams are now disposed via try/finally even when validation rejects a file mid-loop
  • Remove unnecessary if/else branch — always route through UploadSkillWithResourcesAsync since it handles empty resource lists correctly
  • Rename form field from references to resources and accept full relative paths as filenames (e.g. references/guide.md, scripts/setup.sh) so clients can upload into any allowed directory
  • Open ResourcePath validation to allow arbitrary subdirectories per the AgentSkills.io spec, which permits any additional files and directories beyond the conventional references/, scripts/, and assets/ folders. Safety rules preserved: no path traversal, no absolute paths, no bare root-level filenames.

Test plan

  • UploadSkillWithResources_EndToEnd — uploads resources into both references/ and scripts/, verifies download and RFC index
  • UploadSkillWithResources_WithoutResources_StillWorks — backward compatibility
  • UploadSkillWithResources_InvalidPath_ReturnsBadRequest — path traversal (../etc/passwd) rejected
  • ResourcePathTests — updated to accept arbitrary subdirectories (custom/file.txt, examples/demo.py) while rejecting bare filenames and trailing-slash-only paths
  • Full suite passes (50 unit + 23 integration)

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.
@Aaronontheweb Aaronontheweb force-pushed the fix/upload-endpoint-cleanup branch from 40eded8 to e84b7e6 Compare April 29, 2026 03:15
@Aaronontheweb Aaronontheweb merged commit fdbd2a7 into dev Apr 29, 2026
5 checks passed
@Aaronontheweb Aaronontheweb deleted the fix/upload-endpoint-cleanup branch April 29, 2026 03:19
@Aaronontheweb Aaronontheweb mentioned this pull request Apr 29, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant