Repository navigation
fix: CORS headers missing/broken on rustfs and azure backends - #3
Merged
Merged
Conversation
Found while reproducing a teammate's report that downloading a course grade report threw a CORS console error. A plain <a> link click isn't subject to CORS, so this only bites a script-driven fetch/XHR against a presigned object URL - confirmed live, not just read off the code. - rustfs: sent no Access-Control-* headers at all by default (RustFS can do CORS via `rc bucket cors`, but no bucket has a policy out of the box). Fixed by setting the headers at the Caddy proxy layer. - azure: had two distinct bugs of its own, found while auditing the other backends for the same class of issue. rclone's `serve s3` already sends its own native CORS headers; Caddy's own `header` directive doesn't replace what reverse_proxy copies from the upstream, it adds a second value, so every real response carried Access-Control-Allow-Origin twice (a real browser rejects that exactly like a missing header). Separately, rclonegw validates a presigned URL's SigV4 signature even on OPTIONS, and a signature minted for GET is never valid for OPTIONS, so every real preflight against a private bucket (e.g. the grades bucket) failed with 403. Fixed by answering OPTIONS directly at Caddy before either rclone gateway sees it, and letting rclone's own native headers stand on the real-traffic path (restoring Access-Control-Expose-Headers via header_down, since rclone's own native value is narrower than what Range-request support needs). - external_s3: no Caddy vhost of its own, so neither fix applies - the README now says explicitly that CORS there is the operator's own bucket-configuration responsibility. New live conformance tests for both rustfs and azure (preflight against a presigned private-bucket URL, and no header duplication), plus offline test_patches.py regression guards. tests/Caddyfile.ci - the CI mirror of the real azure Caddyfile - is updated to match, confirmed against the actual CI compose stack. EVALUATION.md's comparison table and two new Findings document what was actually wrong and how it was verified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
Found while reproducing a teammate's report that downloading a course grade report threw a CORS console error (
No 'Access-Control-Allow-Origin' header is present/doesn't pass access control check). A plain<a>link click isn't subject to CORS, so this only bites a script-drivenfetch/XHRagainst a presigned object URL — confirmed live through the real Caddy-fronted stack, not just read off the code.rustfs: sent noAccess-Control-*headers at all by default. RustFS can do CORS viarc bucket cors, but no bucket has a policy configured out of the box — the same as a fresh AWS S3 bucket. Fixed by setting the headers at the Caddy proxy layer, the same approach theazurebackend already used.azure: auditing it for the same class of bug (since therustfsfix above used the same proxy-layer patternazurehad shipped since its initial release) turned up two distinct, pre-existing bugs of its own:serve s3already sends a complete, native set ofAccess-Control-*headers — Caddy's ownheaderdirective doesn't replace whatreverse_proxycopies from the upstream response, it adds a second value, so every realGET/HEAD/PUTresponse carriedAccess-Control-Allow-Origintwice. A real browser rejects that exactly like a missing header.rclonegwvalidates a presigned URL's SigV4 signature even onOPTIONS, and a signature minted forGETis never valid forOPTIONS— so every real preflight against a private bucket (the grades bucket, exactly what the original report hit) failed with403. The existing live conformance test only ever preflighted the public bucket, which routes to the unauthenticated gateway and never exercised this path.Fixed by answering
OPTIONSdirectly at Caddy before either rclone gateway ever sees it, and letting rclone's own native headers stand on the real-traffic path — including restoringAccess-Control-Expose-Headersviaheader_down(rclone's own native value there is justETag, narrower than what this plugin's Range-request/video-scrubbing support needs exposed to JS).external_s3: has no Caddy vhost of its own — the browser talks to the external provider directly — so neither fix can apply there. The README now says explicitly that CORS on every bucket is the operator's own configuration responsibility, with the exact browser-console symptoms to recognize, so an operator doesn't mistake their own unconfigured bucket for a plugin bug.Testing
rustfsandazure: a preflight against a presigned private-bucket URL, and a check thatAccess-Control-Allow-Originisn't duplicated on a realGET. Each confirmed red against the pre-fix Caddyfiles and green after.test_patches.pyregression guards for both backends' rendered Caddyfiles.tests/Caddyfile.ci(the hand-maintained CI mirror of the realazureCaddyfile) is updated to match — confirmed by running the actual live conformance suite against the real CI compose stack locally, not just inferred from the template diff.ruff check/ruff format --check, andmypy --strictall pass.EVALUATION.md's comparison table and two new Findings (6 and 7) document what was actually wrong, how each was isolated live, and how it was verified fixed.Reviewer notes
azurefix changespatches/azure/caddyfile's structure (adds an@cors_preflight/handleshort-circuit block ahead of the existing public/private routing) — the full pre-existing live conformance suite (anonymous public/private split, every generic S3 verb, Range requests) was re-run against it and passes unchanged.mainwas identical to this branch's merge-base at push time, so there's no rebase/divergence to worry about.🤖 Generated with Claude Code