fix: make CORS origins configurable, default to same-origin only - #70
Merged
Merged
Conversation
backend/app/main.py hardcoded allow_origins=["*"], which is more permissive than the shipped topology needs — the frontend and API sit behind the same nginx proxy, so the browser never makes a cross-origin request in the first place. allow_credentials was already False. Add Settings.cors_allow_origins (comma-separated string, matching the existing plain-scalar style in config.py — a bare list[str] field would parse the env var as JSON) and a cors_origins() helper that parses it into the list CORSMiddleware expects. Empty (the default) is same-origin only; "*" is an explicit opt-in for anyone serving the frontend from a separate origin. Fixed a real regression this change would otherwise have caused: frontend/.env has pointed VITE_ATS_API_BASE_URL at the absolute http://localhost:8000/api since the frontend was first added, bypassing the proxy vite.config.js already sets up and documented behavior (docs/architecture/frontend.md says "the dev server proxies API requests to the backend"). That made `npm run dev` against a local backend a genuine cross-origin request, which the new same-origin default would have blocked. Pointed it at the documented relative /api instead, matching what docker-compose.override.yml already sets for the dockerized dev frontend — so local dev needs no CORS opt-in at all. Wired CORS_ALLOW_ORIGINS through docker-compose.yml and documented it in .env.example and docs/getting-started/configuration.md, alongside the existing ENCRYPTION_KEY precedent. Added tests/test_cors.py: unit tests for the parser, and behavioral tests (via a standalone CORSMiddleware app, since the real app builds its middleware at import time) confirming the default sends no access-control-allow-origin header cross-origin, and a configured origin is echoed back while others are rejected. Closes #33 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TNWyAav5c2jAaXemCLrL9g
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
1 participant
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
Closes #33.
backend/app/main.pyhardcodedallow_origins=["*"]. In the shipped compose topology the frontend and API sit behind the same nginx proxy, so the browser never makes a cross-origin request in the first place — same-origin should be the default, with*as an explicit opt-in.allow_credentialswas alreadyFalse.Changes
Settings.cors_allow_origins: str = ""(comma-separated, matching the config's existing plain-scalar style — a barelist[str]field would parse the env var as JSON) and acors_origins()helper that parses it forCORSMiddleware.CORS_ALLOW_ORIGINSthroughdocker-compose.yml, documented in.env.exampleanddocs/getting-started/configuration.md, following theENCRYPTION_KEYprecedent.A real regression this would otherwise have caused
frontend/.envhas pointedVITE_ATS_API_BASE_URLat the absolutehttp://localhost:8000/apisince the frontend was first added (2024), bypassing the proxyvite.config.jsalready sets up and the documented behavior (docs/architecture/frontend.md: "the dev server proxies API requests to the backend"). That madenpm run devagainst a locally-run backend a genuine cross-origin request — which the new same-origin default would have silently broken.Fixed it to the documented relative
/api, matching whatdocker-compose.override.ymlalready sets for the dockerized dev frontend. Local development now needs no CORS opt-in at all — it's same-origin everywhere: bare-metal dev, dockerized dev, and production.Tests
backend/tests/test_cors.py:cors_origins()parser (empty, comma-separated,*).CORSMiddlewareapp (the real app builds its middleware at import time, so varying settings against the globalappisn't possible) confirming: the default sends noaccess-control-allow-originheader cross-origin; a configured origin is echoed back; other origins are rejected.Verification
uv run ruff check .— cleanuv run pytest— 201 passednpx eslint . --ext .vue,.js,.jsx,.cjs,.mjs --ignore-path .gitignore— clean🤖 Generated with Claude Code