Fix #4130: Remove allowedMethods gate from non-preflight CORS actual requests - #4171
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Removes the allowedMethods check from non-preflight (actual) CORS requests so that only preflight enforces allowed methods, avoiding incorrect 403 responses after a successful preflight.
Changes:
- Update non-preflight CORS path to gate only on
allowedOrigin, notallowedMethods. - Keep the 403 Forbidden response exclusively for disallowed origins on actual requests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
✅ Deploy Preview for zio-http ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…requests Per Fetch spec, the allowedMethods check (and resulting 403) is only for preflight. For actual cross-origin requests, only the origin should be checked. The previous code re-applied the method check on actual requests. Fixes #4130
987Nabil
force-pushed
the
fix-4130-cors-non-preflight
branch
from
September 11, 2026 06:58
87addc1 to
332f601
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.
Fixes #4130.\n\n## Summary\nIn
Middleware.cors(...), the non-preflight (actual request) handler in theHandlerAspectwas still doing:\nscala\ncase Some(allowOrigin) if config.allowedMethods.contains(request.method) =>\n ...\ncase _ =>\n 403 Forbidden\n\n\nPer the Fetch spec (and the library's own docs), theallowedMethods/Access-Control-Allow-Methodscheck is preflight-only. The browser already enforces the method during preflight. For the actual request the server should only gate on origin (and emit the CORS headers if allowed).\n\nThis caused 403s on valid post-preflight requests (e.g. PATCH/DELETE when not explicitly listed) even after a successful preflight.\n\n## Fix\nNon-preflight path now only checks origin:\nscala\ncase Some(allowOrigin) =>\n ZIO.succeed((corsHeaders(allowOrigin, acrhHeader, isPreflight = false), (request, ())))\n\n(The 403 case is now only for disallowed origin.)\n\nThe preflightoptionsRoute+ all other CORS logic (header prebuilding, etc.) is untouched.\n\n(Verified on main + matches the original report + spec reference.)