Add health check for database connection#912
Conversation
| - SCM_HC_USER=healthz | ||
| - SCM_HC_PASSWORD=healthzpassword |
There was a problem hiding this comment.
I would prefer to see this removed so that people are forced to invent their own password.
compliance-monitor/monitor.py
Outdated
| self.hc_user = os.getenv("SCM_HC_USER", "healthz") | ||
| self.hc_password = os.getenv("SCM_HC_PASSWORD", "healthzpassword") |
There was a problem hiding this comment.
Similar here: the default should be something that would never match, such as None (then the test credentials.username == settings.hc_user returns False).
compliance-monitor/monitor.py
Outdated
| async def get_healthz( | ||
| request: Request, | ||
| ): | ||
| """return monitor's health status""" |
There was a problem hiding this comment.
| async def get_healthz( | |
| request: Request, | |
| ): | |
| """return monitor's health status""" | |
| async def get_healthz(request: Request): | |
| """return compliance monitor's health status""" |
compliance-monitor/monitor.py
Outdated
| # unauthorized user | ||
| check_db_connection() | ||
|
|
||
| return {"message": "OK"} |
There was a problem hiding this comment.
| return {"message": "OK"} | |
| return Response() |
for an empty response, which should be sufficient and better than text/json
compliance-monitor/monitor.py
Outdated
| # check credentials | ||
| if credentials is None: | ||
| # no credentials were set | ||
| check_db_connection() | ||
| elif credentials.username == settings.hc_user and credentials.password == settings.hc_password: | ||
| # healthz user | ||
| check_db_connection(authorized=True) | ||
| else: | ||
| # unauthorized user | ||
| check_db_connection() |
There was a problem hiding this comment.
We should be catching any exception here, otherwise we might disclose some exceptions even if the request is not authorized.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| mk_conn(settings=settings) | ||
| except Exception as e: | ||
| detail = str(e) if authorized else 'internal server error' | ||
| return Response(status_code=500, content=detail, media_type='text/plain') |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the issue, we will modify the code to ensure that detailed exception information is not exposed to external users. Instead, we will log the exception details on the server and return a generic error message to the user. Specifically:
- Replace the assignment of
detailwith a generic error message ("internal server error"). - Log the exception details (
str(e)) using theloggerobject for debugging purposes.
This ensures that sensitive information is not leaked to the user while still allowing developers to diagnose issues using server logs.
| @@ -735,3 +735,4 @@ | ||
| except Exception as e: | ||
| detail = str(e) if authorized else 'internal server error' | ||
| logger.error("Health check failed: %s", str(e)) | ||
| detail = 'internal server error' | ||
| return Response(status_code=500, content=detail, media_type='text/plain') |
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Anja Strunk <anja.strunk@cloudandheat.com>
Signed-off-by: Matthias Büchse <matthias.buechse@alasca.cloud>
mbuechse
force-pushed
the
add-health-monitor
branch
from
7c502a7 to
b79b58a
Compare
2 participants
Add health monitor for database connection