Update CORS origin on api.flutter.dev for use in docs. #177841
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Firebase hosting configuration to add a CORS header for all *.html files, setting Access-Control-Allow-Origin to *. While this enables the intended cross-site usage for documentation, using a wildcard origin introduces a security risk. My review comment details this concern and proposes a more secure alternative to restrict access to a whitelist of origins.
| { | ||
| "source": "**/*.html", | ||
| "headers": [ | ||
| { | ||
| "key": "Access-Control-Allow-Origin", | ||
| "value": "*" | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Using Access-Control-Allow-Origin: "*" introduces a security risk by allowing any website to make requests to your pages and read their content. If these pages were to ever serve user-specific or sensitive information (e.g., based on an authenticated session), this permissive CORS policy could allow a malicious site to exfiltrate that data from a user's browser.
While the API documentation is currently public, security best practices recommend being as restrictive as possible (principle of least privilege). A more secure approach would be to implement a whitelist of allowed origins.
Since Firebase Hosting's static headers do not support dynamic origin reflection, you could achieve this by routing requests for **/*.html to a Cloud Function. This function would inspect the Origin header of incoming requests, validate it against your list of allowed domains (e.g., docs.flutter.dev, staging sites, forks), and then set the Access-Control-Allow-Origin header to the requesting origin if it's on the whitelist. This provides defense-in-depth against future changes that might inadvertently expose sensitive data.
There was a problem hiding this comment.
Since the api docs contain only static, not user-specific content, I don't see a security risk here.
parlough
added
d: api docs
github-actions
bot
removed
d: api docs
schultek
force-pushed
the
fix/update-cors-for-docs
branch
from
36a6bbb to
bc446eb
Compare
Resolves #177837
This allows CORS requests for all
*.htmlpages on the hosted api docs.The origin is allow-all, since this needs to be available not only for
docs.flutter.dev, but also any staging version of the docs as well as the chinese fork and potential others. Lmk if you think this is a security concern, but I think it should be fine for only html pages.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.