Add Dockerfile and fix CORS errors#33
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds containerization support and tightens CORS configuration to address frontend cross-origin request failures, while also centralizing the frontend API base URL configuration.
Changes:
- Added a
Dockerfileand updatedstart.shdefaults/env exports for running the FastAPI app. - Implemented stricter, normalized CORS origin allow-list building in
backup/app/main.py. - Introduced
pages/static/js/config.jsand updated pages/scripts to readAPI_URLfromwindow.APP_CONFIG.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
start.sh |
Sets runtime env vars (DB URL, CORS origins, SECRET_KEY) before launching Uvicorn. |
backup/app/main.py |
Normalizes and de-duplicates CORS origins; removes permissive "*" fallback. |
Dockerfile |
Adds a basic container build/run configuration for the API. |
pages/static/js/config.js |
Provides a single place to configure frontend API_URL. |
pages/static/js/auth.js |
Uses window.APP_CONFIG.API_URL (with fallback) instead of localhost constant. |
pages/static/js/scoreboard.js |
Uses window.APP_CONFIG.API_URL (with fallback) instead of localhost constant. |
pages/submission.html |
Updates nav label and loads config.js before auth.js. |
pages/scoreboard.html |
Loads config.js before scoreboard.js. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Export DATABASE_URL (uses existing value if already set). | ||
| export DATABASE_URL="${DATABASE_URL:-sqlite:///./mpvrp_scoring.db}" | ||
| export FRONTEND_ALLOWED_ORIGINS="${FRONTEND_ALLOWED_ORIGINS:-https://ifri-ai-classes.github.io,https://ifri-ai-classes.github.io/MPVRP-CC,https://ifri-ai-classes.github.io/MPVRP-CC/pages}" |
There was a problem hiding this comment.
FRONTEND_ALLOWED_ORIGINS default includes path components (e.g. "/MPVRP-CC"), but CORS Origin values are scheme+host(+port) only. With the new normalization this collapses to a single origin and will emit warnings; it also omits localhost origins which can break local frontend dev when using start.sh defaults. Consider defaulting to origins only (no paths) and include localhost entries if local dev via start.sh is expected.
| export FRONTEND_ALLOWED_ORIGINS="${FRONTEND_ALLOWED_ORIGINS:-https://ifri-ai-classes.github.io,https://ifri-ai-classes.github.io/MPVRP-CC,https://ifri-ai-classes.github.io/MPVRP-CC/pages}" | |
| export FRONTEND_ALLOWED_ORIGINS="${FRONTEND_ALLOWED_ORIGINS:-https://ifri-ai-classes.github.io,http://localhost:3000,http://127.0.0.1:3000,http://localhost:5173,http://127.0.0.1:5173}" |
| # Require stable secret key in environments with external users. | ||
| # Generate and export a fresh SECRET_KEY at launch time. | ||
| SECRET_KEY="$(python -c "import secrets; print('SECRET_KEY=' + secrets.token_urlsafe(32))")" | ||
| export "$SECRET_KEY" | ||
| export SECRET_KEY="$(python -c "import secrets; print(secrets.token_urlsafe(32))")" | ||
| if [[ -z "${SECRET_KEY:-}" ]]; then | ||
| echo "ERROR: SECRET_KEY is required. Set it in your environment before starting the server." >&2 | ||
| exit 1 |
There was a problem hiding this comment.
export SECRET_KEY="$(...)" overwrites any pre-set SECRET_KEY on every start, which breaks token validity across restarts and contradicts the "Require stable secret key" comment. It also makes the subsequent -z check effectively dead code. Prefer only generating a key when SECRET_KEY is unset (or fail fast in production) and keep the stability requirement enforceable.
| @@ -0,0 +1,12 @@ | |||
| FROM python:3.11-slim | |||
There was a problem hiding this comment.
Base image is Python 3.11, but the project declares requires-python ">=3.12" (pyproject.toml). This mismatch can cause runtime failures or dependency resolution issues in the container. Update the image to a 3.12 variant (or adjust the project requirement if 3.11 is actually supported).
| FROM python:3.11-slim | |
| FROM python:3.12-slim |
| def _normalize_origin(origin: str) -> str | None: | ||
| """Normalize CORS entries to scheme://host[:port] and drop invalid values.""" | ||
| raw = origin.strip() | ||
| if not raw: | ||
| return None | ||
|
|
||
| parsed = urlsplit(raw) | ||
| if not parsed.scheme or not parsed.netloc: | ||
| logger.warning("Ignoring invalid CORS origin '%s'", origin) | ||
| return None | ||
|
|
||
| normalized = f"{parsed.scheme}://{parsed.netloc}" | ||
| if normalized != raw.rstrip("/"): | ||
| logger.warning("Normalized CORS origin '%s' to '%s'", origin, normalized) | ||
| return normalized | ||
|
|
||
|
|
||
| def _normalized_unique(origins: list[str]) -> list[str]: | ||
| result: list[str] = [] | ||
| for origin in origins: | ||
| normalized = _normalize_origin(origin) | ||
| if normalized and normalized not in result: | ||
| result.append(normalized) | ||
| return result | ||
|
|
||
|
|
||
| def _build_allowed_origins() -> list[str]: | ||
| """Build a strict CORS allow-list from environment variables.""" | ||
| allowed_origins_env = os.getenv("FRONTEND_ALLOWED_ORIGINS", "") | ||
| parsed = [origin.strip() for origin in allowed_origins_env.split(",") if origin.strip()] | ||
| parsed = _normalized_unique(allowed_origins_env.split(",")) | ||
|
|
||
| if parsed: | ||
| return parsed | ||
|
|
||
| # Backward-compatible fallback when comma-separated variable is not set. | ||
| legacy_origins = [o for o in [FRONTEND_DEV_URL, FRONTEND_PROD_URL] if o] | ||
| legacy_origins = _normalized_unique([FRONTEND_DEV_URL, FRONTEND_PROD_URL]) | ||
| if legacy_origins: | ||
| return legacy_origins | ||
|
|
||
| # Safe local fallback for development/test only. | ||
| return ["http://localhost:3000", "http://127.0.0.1:3000", "*"] | ||
| return ["http://localhost:3000", "http://127.0.0.1:3000"] |
There was a problem hiding this comment.
New origin normalization/allow-list building logic is security-sensitive and currently untested. Add tests that exercise _build_allowed_origins for env values with paths/trailing slashes, duplicates, invalid entries, and the legacy FRONTEND_DEV_URL/FRONTEND_PROD_URL fallback to ensure CORS behavior stays correct.
Merge pull request IFRI-AI-Classes#33 from Pinite37/main
No description provided.