fix: support HEAD method on health check routes, allow localhost in s…#74
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>
|
🤖 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.
Code Review
This pull request adds 'localhost' and '127.0.0.1' to the allowed hosts in the staging configuration and updates the root and health check endpoints to support both GET and HEAD methods. The reviewer recommends extending the allowed hosts update to the production configuration as well to prevent potential health check failures in production.
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.
| "localhost", | ||
| "127.0.0.1", |
There was a problem hiding this comment.
While adding "localhost" and "127.0.0.1" to StagingConfig.ALLOWED_HOSTS successfully resolves the health check failures in staging, the production environment (ProductionConfig) will likely suffer from the exact same issue if it is also hosted on Render (or any platform using local loopback health probes).
To prevent production deployment failures or health check rejections by TrustedHostMiddleware, please ensure "localhost" and "127.0.0.1" are also added to ProductionConfig.ALLOWED_HOSTS:
class ProductionConfig(BaseConfig):
...
ALLOWED_HOSTS: List[str] = [
"taimako.dubem.xyz",
"api.taimako.dubem.xyz",
"*.taimako.dubem.xyz",
"localhost",
"127.0.0.1",
]There was a problem hiding this comment.
This pull request introduces a well-defined fix to support HEAD requests for health checks and correctly configures ALLOWED_HOSTS for the staging environment. The changes are clear, concise, and directly address the issue of compatibility with Render's health probes.
🔍 General Feedback
- The changes are logical and correctly implemented.
- The explanation in the pull request description is clear and helpful for understanding the context.
- The code quality is good, and the changes align with the existing project structure.
| "localhost", | ||
| "127.0.0.1", | ||
| ] |
There was a problem hiding this comment.
| "localhost", | |
| "127.0.0.1", | |
| ] | |
| "taimako.onrender.com", | |
| "taimakoai.onrender.com", | |
| # Allow health checks from Render's internal network | |
| "localhost", | |
| "127.0.0.1", | |
| ] |
There was a problem hiding this comment.
This pull request aims to improve the health check functionality by adding support for the HEAD method and allowing localhost for staging environment health checks. While the proposed changes are functionally correct, a security audit of the codebase revealed a critical vulnerability related to a hardcoded JWT secret.
🔍 General Feedback
- The changes in this pull request are well-contained and address the issue of health checks effectively.
- The codebase is generally well-structured, but the audit surfaced a critical security issue that should be addressed immediately.
…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.
Description
Type of Change
Checklist
make lint-be/make lint-fe)make test-be/make test-fe)Screenshots / Demo
Related Issues