Skip to content

Confine upload_download paths to the configured os.Root directory - #224

Merged
raphael merged 1 commit into
mainfrom
updown-os-root
Nov 9, 2026
Merged

raphael merged 1 commit into
mainfrom
updown-os-root

Conversation

@raphael

@raphael raphael commented Nov 9, 2026

Copy link
Copy Markdown
Member

The upload_download example shows how a Goa HTTP service (updown) streams multipart file uploads (POST /upload/{*dir}) and file downloads (GET /download/{*filename}) without buffering whole request or response bodies in memory. Previously, both handlers joined the decoded URL wildcard parameter onto the configured base directory with filepath.Join without checking whether the resulting path stayed inside that base directory. In the HTTP tests, a request such as POST /upload/..%2Foutside or GET /download/..%2Fsecret.txt resolved outside public/, allowing callers who copied the example to create directories (with mode 0777), overwrite files, or read files outside the intended directory. Upload also deferred closing each created file inside the multipart loop—keeping every part's file descriptor open until the request finished—and accepted multipart parts with empty, ., or .. file names.

Opening an *os.Root handle in cmd/upload_download/main.go and passing it into NewUpdown confines every directory creation, file write, and file read to the configured base directory at the OS level, including across symbolic links inside that directory. Upload and Download first validate and convert slash-separated URL paths with filepath.Localize so parent-directory segments (..) fail before creating any intermediate directories on disk, while still allowing trailing slashes on upload directory URLs. design/design.go declares invalid_file_path (400 Bad Request) on upload alongside download's existing 404 Not Found mapping so invalid, non-directory, or escaping paths return 400/404 while permission and storage failures continue to return 500 (internal_error). Upload creates missing subdirectories with mode 0750, rejects multipart parts whose file name is empty, ., .., or contains path separators, and closes each destination file before advancing to the next part.

HTTP handler tests in upload_download/updown_test.go exercise nested and root-directory multipart uploads, trailing slashes, 0750 directory modes, URL-encoded .. traversal, symlinks pointing outside the base directory, existing regular-file collisions, read-only directory 500 responses, invalid part file names, malformed Content-Type headers, and file downloads.

@raphael
raphael merged commit d298a9f into main Nov 9, 2026
5 checks passed
@raphael
raphael deleted the updown-os-root branch November 9, 2026 20:05
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