diff --git a/services/admin/app/auth/fingerprint_repair.py b/services/admin/app/auth/fingerprint_repair.py index e7725111c..39eaf5b48 100644 --- a/services/admin/app/auth/fingerprint_repair.py +++ b/services/admin/app/auth/fingerprint_repair.py @@ -55,9 +55,12 @@ def repair_null_email_fingerprints( "AUTH_OTP_HMAC_SECRET must be set in production to backfill " "user_emails.email_fingerprint" ) + # Log the env var *name* as a literal; do not pass the + # secret-named constant through the logging data flow + # (py/clear-text-logging-sensitive-data). logger.warning( - "%s not set; skipping user_emails.email_fingerprint repair", - _HMAC_SECRET_ENV, + "AUTH_OTP_HMAC_SECRET not set; skipping " + "user_emails.email_fingerprint repair" ) return 0 crypto_service = AuthCryptoService(hmac_secret=secret) diff --git a/services/admin/app/auth_crypto.py b/services/admin/app/auth_crypto.py index 588f14510..ea7213979 100644 --- a/services/admin/app/auth_crypto.py +++ b/services/admin/app/auth_crypto.py @@ -104,9 +104,12 @@ def _fetch_hmac_secret_from_kms_or_env(cls) -> str: global _DEV_EPHEMERAL_HMAC_SECRET if _DEV_EPHEMERAL_HMAC_SECRET is None: _DEV_EPHEMERAL_HMAC_SECRET = secrets.token_urlsafe(_MIN_SECRET_BYTES) + # Log the env var *name* as a literal; do not pass the + # secret-named constant through the logging data flow + # (py/clear-text-logging-sensitive-data). logger.warning( - "%s not set; using process-local ephemeral OTP HMAC secret", - _HMAC_SECRET_ENV, + "AUTH_OTP_HMAC_SECRET not set; using process-local ephemeral " + "OTP HMAC secret" ) return _DEV_EPHEMERAL_HMAC_SECRET diff --git a/services/admin/tests/test_cleartext_logging_redaction.py b/services/admin/tests/test_cleartext_logging_redaction.py new file mode 100644 index 000000000..ff3141c17 --- /dev/null +++ b/services/admin/tests/test_cleartext_logging_redaction.py @@ -0,0 +1,61 @@ +"""Regression coverage for CodeQL py/clear-text-logging-sensitive-data. + +These tests lock in that the secret-related warning paths keep their +operator-useful message (the env var *name* and the action taken) while +never emitting the actual secret material into the logs. + +Covers: +* ``auth_crypto.AuthCryptoService._fetch_hmac_secret_from_kms_or_env`` + (alert: services/admin/app/auth_crypto.py:109) +* ``auth.fingerprint_repair.repair_null_email_fingerprints`` + (alert: services/admin/app/auth/fingerprint_repair.py:60) +""" + +from __future__ import annotations + +import logging + +import pytest + +from services.admin.app import auth_crypto +from services.admin.app.auth import fingerprint_repair + + +def test_ephemeral_secret_warning_names_env_without_leaking_secret(monkeypatch, caplog): + # Force the dev-ephemeral branch: no env secret, non-production. + monkeypatch.delenv("AUTH_OTP_HMAC_SECRET", raising=False) + monkeypatch.setenv("REGENGINE_ENV", "development") + monkeypatch.setattr(auth_crypto, "_DEV_EPHEMERAL_HMAC_SECRET", None, raising=False) + + with caplog.at_level(logging.WARNING, logger="auth_crypto"): + secret = auth_crypto.AuthCryptoService._fetch_hmac_secret_from_kms_or_env() + + assert secret # a usable ephemeral secret is still produced + records = [r for r in caplog.records if r.name == "auth_crypto"] + assert len(records) == 1 + message = records[0].getMessage() + # Useful: names the missing env var and the fallback action. + assert "AUTH_OTP_HMAC_SECRET" in message + assert "ephemeral" in message + # Safe: the actual secret value never reaches the log record. + assert secret not in message + assert records[0].args in (None, ()) + + +def test_fingerprint_repair_warning_names_env_without_leaking_secret(monkeypatch, caplog): + # Non-production + unset secret => warn-and-skip path (returns 0). + for var in ("REGENGINE_ENV", "ENV", "RAILWAY_ENVIRONMENT", "RAILWAY_ENVIRONMENT_NAME"): + monkeypatch.delenv(var, raising=False) + monkeypatch.delenv("AUTH_OTP_HMAC_SECRET", raising=False) + + with caplog.at_level(logging.WARNING, logger="auth.fingerprint_repair"): + updated = fingerprint_repair.repair_null_email_fingerprints() + + assert updated == 0 + records = [r for r in caplog.records if r.name == "auth.fingerprint_repair"] + assert len(records) == 1 + message = records[0].getMessage() + assert "AUTH_OTP_HMAC_SECRET" in message + assert "email_fingerprint" in message + # No secret-shaped positional args were passed to the logger. + assert records[0].args in (None, ()) diff --git a/services/ingestion/app/shared/tenant_resolution.py b/services/ingestion/app/shared/tenant_resolution.py index c38867e7d..41978f646 100644 --- a/services/ingestion/app/shared/tenant_resolution.py +++ b/services/ingestion/app/shared/tenant_resolution.py @@ -87,10 +87,13 @@ def resolve_tenant_id( if api_key_tenant_id: if requested_tenant_id and requested_tenant_id != api_key_tenant_id: + # Log only the request-supplied tenant (non-sensitive). The + # api-key-derived tenant is omitted so the API key material does + # not reach the logs (py/clear-text-logging-sensitive-data). logger.warning( - "tenant_override_rejected requested=%s api_key_tenant=%s", + "tenant_override_rejected requested=%s " + "(api key is bound to a different tenant)", requested_tenant_id, - api_key_tenant_id, ) raise HTTPException( status_code=403, diff --git a/services/ingestion/tests/test_tenant_override_logging_redaction.py b/services/ingestion/tests/test_tenant_override_logging_redaction.py new file mode 100644 index 000000000..20038b6bd --- /dev/null +++ b/services/ingestion/tests/test_tenant_override_logging_redaction.py @@ -0,0 +1,51 @@ +"""Regression coverage for CodeQL py/clear-text-logging-sensitive-data. + +``resolve_tenant_id`` rejects a request whose body/header tenant conflicts +with the tenant bound to the presented API key. The rejection warning must +stay useful (it names the request-supplied tenant that was rejected) while +never logging the api-key-derived tenant value, which flows from the raw +API key material. + +Alert: services/ingestion/app/shared/tenant_resolution.py:93 +""" + +from __future__ import annotations + +import logging +import secrets +import uuid + +import pytest +from app.shared import tenant_resolution +from app.shared.tenant_resolution import resolve_tenant_id +from fastapi import HTTPException + +_API_KEY_TENANT = str(uuid.uuid4()) +_REQUESTED_TENANT = str(uuid.uuid4()) + + +def test_tenant_override_rejected_logs_request_without_api_key_tenant(monkeypatch, caplog): + monkeypatch.setattr( + tenant_resolution, + "_lookup_api_key_tenant_id", + lambda _key: _API_KEY_TENANT, + ) + + with caplog.at_level(logging.WARNING, logger=tenant_resolution.logger.name): + with pytest.raises(HTTPException) as exc_info: + resolve_tenant_id( + explicit_tenant_id=_REQUESTED_TENANT, + x_tenant_id=None, + x_regengine_api_key=secrets.token_urlsafe(), # pragma: allowlist secret + ) + + assert exc_info.value.status_code == 403 + + records = [r for r in caplog.records if "tenant_override_rejected" in r.getMessage()] + assert len(records) == 1 + message = records[0].getMessage() + # Useful: identifies which requested tenant was rejected. + assert _REQUESTED_TENANT in message + # Safe: the api-key-derived tenant value is never emitted. + assert _API_KEY_TENANT not in message + assert _API_KEY_TENANT not in str(records[0].args)