Harden /ready checks + production CORS guardrails#79
Merged
alanmaizon merged 2 commits intomainfrom Feb 18, 2026
Merged
Conversation
Reviewer's Guide/ready now performs real DB + storage readiness checks with caching, production CORS configuration is hardened to reject insecure origins both at app startup and in the deploy readiness script, and tests are updated accordingly. Flow diagram for production CORS validation in app startupflowchart TD
Start([Start create_app])
Load["Load cors_origins from settings"]
CheckWildcard["credentials enabled AND cors_origins contains '*'"]
WildcardError[["Raise RuntimeError: wildcard not allowed with credentials"]]
CheckEnv["app_env == production?"]
CheckHttp["Any origin starts with 'http://'? (case-insensitive)"]
HttpError[["Raise RuntimeError: http:// origins not allowed in production"]]
CheckLocal["Any origin contains localhost or 127.0.0.1?"]
LocalError[["Raise RuntimeError: localhost origins not allowed in production"]]
Success(["Proceed to FastAPI app creation and CORS middleware"])
Start --> Load --> CheckWildcard
CheckWildcard -- Yes --> WildcardError
CheckWildcard -- No --> CheckEnv
CheckEnv -- No --> Success
CheckEnv -- Yes --> CheckHttp
CheckHttp -- Yes --> HttpError
CheckHttp -- No --> CheckLocal
CheckLocal -- Yes --> LocalError
CheckLocal -- No --> Success
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The in-memory
_ready_cachedict is mutated from the request handler without any locking; in a multi-threaded or multi-worker deployment you may want to guard it with a lock or use a simpler atomic structure to avoid rare race conditions when multiple/readycalls happen concurrently. - In the S3 readiness check, you always write to the same
readyz/{env}/backend.txtkey; if multiple backends or versions share a bucket/prefix this can cause them to overwrite each other’s probes, so consider incorporating app name or instance ID into the key to avoid collisions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The in-memory `_ready_cache` dict is mutated from the request handler without any locking; in a multi-threaded or multi-worker deployment you may want to guard it with a lock or use a simpler atomic structure to avoid rare race conditions when multiple `/ready` calls happen concurrently.
- In the S3 readiness check, you always write to the same `readyz/{env}/backend.txt` key; if multiple backends or versions share a bucket/prefix this can cause them to overwrite each other’s probes, so consider incorporating app name or instance ID into the key to avoid collisions.
## Individual Comments
### Comment 1
<location> `backend/app/api/routers/system.py:141-147` </location>
<code_context>
+ Body=token.encode("utf-8"),
+ ContentType="text/plain",
+ )
+ response = client.get_object(Bucket=bucket, Key=key)
+ body = response.get("Body")
+ if body is None:
+ raise RuntimeError("S3 get_object returned no Body")
+ read_back = body.read().decode("utf-8", errors="replace")
+ if read_back != token:
+ raise RuntimeError("S3 readiness probe mismatch")
</code_context>
<issue_to_address>
**suggestion (bug_risk):** S3 response body stream is not closed after reading.
`Body` is a streaming object; if it’s not closed after `read()`, connections can be leaked. Please either call `body.close()` after reading or use it as a context manager (where supported by your boto3 version), which is especially important since this path is hit repeatedly by readiness probes.
```suggestion
response = client.get_object(Bucket=bucket, Key=key)
body = response.get("Body")
if body is None:
raise RuntimeError("S3 get_object returned no Body")
try:
read_back = body.read().decode("utf-8", errors="replace")
finally:
# Ensure the streaming body is closed to avoid leaking connections
body.close()
if read_back != token:
raise RuntimeError("S3 readiness probe mismatch")
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+141
to
+147
| response = client.get_object(Bucket=bucket, Key=key) | ||
| body = response.get("Body") | ||
| if body is None: | ||
| raise RuntimeError("S3 get_object returned no Body") | ||
| read_back = body.read().decode("utf-8", errors="replace") | ||
| if read_back != token: | ||
| raise RuntimeError("S3 readiness probe mismatch") |
There was a problem hiding this comment.
suggestion (bug_risk): S3 response body stream is not closed after reading.
Body is a streaming object; if it’s not closed after read(), connections can be leaked. Please either call body.close() after reading or use it as a context manager (where supported by your boto3 version), which is especially important since this path is hit repeatedly by readiness probes.
Suggested change
| response = client.get_object(Bucket=bucket, Key=key) | |
| body = response.get("Body") | |
| if body is None: | |
| raise RuntimeError("S3 get_object returned no Body") | |
| read_back = body.read().decode("utf-8", errors="replace") | |
| if read_back != token: | |
| raise RuntimeError("S3 readiness probe mismatch") | |
| response = client.get_object(Bucket=bucket, Key=key) | |
| body = response.get("Body") | |
| if body is None: | |
| raise RuntimeError("S3 get_object returned no Body") | |
| try: | |
| read_back = body.read().decode("utf-8", errors="replace") | |
| finally: | |
| # Ensure the streaming body is closed to avoid leaking connections | |
| body.close() | |
| if read_back != token: | |
| raise RuntimeError("S3 readiness probe mismatch") |
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.
What
/readynow verifies DB connectivity + storage (local or S3 write/read), with a small in-memory cache to avoid thrashing.CORS_ORIGINScontainshttp://or localhost whenAPP_ENV=production.CORS_ORIGINSis insecure (http://,*, or localhost).Why
Avoid silent partial outages (DB/S3 miswiring) and prevent drifting back to insecure CORS configuration.
Required AWS change BEFORE deploying this PR
Your current backend task definition has
CORS_ORIGINS=http://...which will fail once this PR deploys.Update it to your CloudFront HTTPS origin.
CloudFront domain (current):
https://d1isah9ifwe7fg.cloudfront.netCLI patch (creates a new task definition revision + force deploy):
Test
cd backend PYTHONPATH=. .venv/bin/python3.14 -m pytest -qSummary by Sourcery
Strengthen readiness and deployment safety by making /ready perform real DB and storage checks and enforcing secure CORS configuration in production.
New Features:
Enhancements:
Tests: