Skip to content

Add health check for database connection #912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mbuechse merged 3 commits into from
Apr 30, 2025
Merged

Add health check for database connection #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6cd70e
Add health check for database connection
anjastrunk Apr 30, 2025
9525a9e
Fix flake8 errors
anjastrunk Apr 30, 2025
b79b58a
apply review suggestions
mbuechse Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compliance-monitor/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ services:
- SCM_DB_HOST=postgres
- SCM_DB_PORT=5432
- SCM_DB_PASSWORD_FILE=/run/secrets/db_password
# pass the following two from the shell
- SCM_HC_USER
- SCM_HC_PASSWORD
- SCM_BASE_URL=https://compliance.sovereignit.cloud/
volumes:
- ../Tests:/Tests
Expand Down
20 changes: 20 additions & 0 deletions compliance-monitor/monitor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ def __init__(self):
self.db_host = os.getenv("SCM_DB_HOST", "localhost")
self.db_port = os.getenv("SCM_DB_PORT", 5432)
self.db_user = os.getenv("SCM_DB_USER", "postgres")
# use default value of None for security reasons (won't be matched)
self.hc_user = os.getenv("SCM_HC_USER", None)
self.hc_password = os.getenv("SCM_HC_PASSWORD", None)
password_file_path = os.getenv("SCM_DB_PASSWORD_FILE", None)
if password_file_path:
with open(os.path.abspath(password_file_path), "r") as fileobj:
Expand Down Expand Up @@ -123,6 +126,7 @@ class ViewType(Enum):
# do I hate these globals, but I don't see another way with these frameworks
app = FastAPI()
security = HTTPBasic(realm="Compliance monitor", auto_error=True) # use False for optional login
optional_security = HTTPBasic(realm="Compliance monitor", auto_error=False)
settings = Settings()
# see https://passlib.readthedocs.io/en/stable/narr/quickstart.html
cryptctx = CryptContext(
Expand Down Expand Up @@ -719,6 +723,22 @@ async def post_results(
conn.commit()


@app.get("/healthz")
async def get_healthz(request: Request):
"""return compliance monitor's health status"""
credentials = await optional_security(request)
authorized = credentials and \
credentials.username == settings.hc_user and credentials.password == settings.hc_password

try:
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

Stack trace information
Loading
flows to this location and may be exposed to an external user.

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:

  1. Replace the assignment of detail with a generic error message ("internal server error").
  2. Log the exception details (str(e)) using the logger object for debugging purposes.

This ensures that sensitive information is not leaked to the user while still allowing developers to diagnose issues using server logs.

Suggested changeset 1
compliance-monitor/monitor.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/compliance-monitor/monitor.py b/compliance-monitor/monitor.py
--- a/compliance-monitor/monitor.py
+++ b/compliance-monitor/monitor.py
@@ -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')
EOF
@@ -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')
Copilot is powered by AI and may make mistakes. Always verify output.

return Response() # empty response with status 200


def pick_filter(results, subject, scope):
"""Jinja filter to pick scope results from `results` for given `subject` and `scope`"""
return results.get(subject, {}).get(scope, {})
Expand Down