Conversation
…taging hosts Render's internal health probe uses HEAD / from 127.0.0.1. FastAPI's automatic HEAD handling wasn't firing (likely due to middleware ordering), so we explicitly register GET+HEAD on both / and /health. Also add localhost and 127.0.0.1 to StagingConfig ALLOWED_HOSTS so TrustedHostMiddleware doesn't reject Render's internal health checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fix: support HEAD method on health check routes, allow localhost in s…
|
🤖 Hi @stenwire, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
📋 Review Summary
This Pull Request introduces valuable improvements by enabling HEAD requests for health checks and allowing localhost in the staging environment's ALLOWED_HOSTS. These changes enhance the API's robustness and streamline local development workflows.
🔍 General Feedback
- The adoption of
HEADmethods for health checks is a positive step towards more efficient API interactions. - Adding
localhostto allowed hosts in staging will aid developers. - Consider adding tests for the
HEADmethods to ensure full coverage.
| @@ -96,6 +96,8 @@ class StagingConfig(BaseConfig): | |||
| ALLOWED_HOSTS: List[str] = [ | |||
| "taimako.onrender.com", | |||
| "taimakoai.onrender.com", | |||
There was a problem hiding this comment.
🟢 Adding localhost and 127.0.0.1 to ALLOWED_HOSTS in the StagingConfig is a good practice for local development and enabling health checks from local network tools.
| @@ -91,10 +91,10 @@ | |||
| app.include_router(orders_router) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
🟡 Changing @app.get to @app.api_route(..., methods=["GET", "HEAD"]) for the root and health endpoints is a great improvement for supporting HEAD requests. This makes the endpoints more robust and efficient for health checks.
Consider adding unit tests specifically for the HEAD method on these routes to ensure they function as expected and return the correct status without a response body.
There was a problem hiding this comment.
Code Review
This pull request adds local hostnames to the staging configuration's ALLOWED_HOSTS and updates the root and health check endpoints to support both GET and HEAD methods. A security review identified a potential Host Header Injection vulnerability due to hardcoding localhost and 127.0.0.1 in the staging configuration, recommending that these be handled dynamically via environment variables instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ALLOWED_HOSTS: List[str] = [ | ||
| "taimako.onrender.com", | ||
| "taimakoai.onrender.com", | ||
| "localhost", | ||
| "127.0.0.1", | ||
| ] |
There was a problem hiding this comment.
Security Vulnerability: Host Header Injection in Staging Environment
Allowing localhost and 127.0.0.1 in the ALLOWED_HOSTS list of a deployed environment (even staging) introduces a security risk. If the application is deployed and accessible publicly, an attacker can send requests with a spoofed Host: localhost or Host: 127.0.0.1 header. If the application uses the host header to construct absolute URLs (e.g., for password reset links, email confirmations, or OAuth redirects), this can lead to Host Header Injection vulnerabilities.
Recommendation:
Remove localhost and 127.0.0.1 from the hardcoded StagingConfig.ALLOWED_HOSTS. If you need to test the staging configuration locally, you can dynamically override ALLOWED_HOSTS using environment variables (which pydantic-settings supports out of the box) rather than hardcoding them in the class definition.
ALLOWED_HOSTS: List[str] = [
"taimako.onrender.com",
"taimakoai.onrender.com",
]There was a problem hiding this comment.
📋 Security Analysis Summary
This pull request introduces changes to the backend configuration and main application file. The changes are minor and intended to support HEAD requests for health checks and allow localhost in the staging environment. While the changes themselves do not introduce vulnerabilities, the security scan identified a pre-existing high-severity vulnerability related to a hardcoded JWT secret.
🔍 General Feedback
- It is highly recommended to address the hardcoded JWT secret vulnerability to improve the application's security posture.
- The practice of using environment variables for configuration is good, but default values for sensitive information should be avoided.
Description
Type of Change
Checklist
make lint-be/make lint-fe)make test-be/make test-fe)Screenshots / Demo
Related Issues