tighten up sensitive field handling in log

Author: alexluck-sift

python/lib/sift_client/_internal/pytest_plugin/audit_log.py

@@ -40,13 +40,38 @@
 EVENT_WIDTH = 16
 # Tag so a re-entered pytest_configure (or both processes) doesn't double-attach.
 HANDLER_TAG = "sift_audit"
-
-
-def _fmt_value(value: object) -> str:
-    """Render one field value: bare when safe, quoted when it would break tokenizing."""
-    if isinstance(value, str):
-        return repr(value) if value == "" or any(c in value for c in " \t=") else value
-    return str(value)
+# Field names whose values should be redacted in audit logs to avoid logging
+# sensitive data. This is a best-effort blacklist; callers should avoid passing
+# sensitive values in the first place.
+SENSITIVE_FIELD_NAMES = frozenset(
+    {
+        "password",
+        "passwd",
+        "pwd",
+        "token",
+        "secret",
+        "api_key",
+        "apikey",
+        "api-key",
+        "access_token",
+        "refresh_token",
+        "private_key",
+        "auth",
+        "credential",
+        "credentials",
+    }
+)
+
+
+def _fmt_kv(key: str, value: object) -> str:
+    """Format a key-value pair for audit log output.
+
+    If ``key`` is in ``SENSITIVE_FIELD_NAMES``, the value is redacted as
+    ``[REDACTED]`` to avoid logging sensitive data.
+    """
+    if key.lower() in SENSITIVE_FIELD_NAMES:
+        value = "[REDACTED]"
+    return f"{key}={value}"
 
 
 def log_event(logger: logging.Logger, level: int, event: str, **fields: object) -> None:
@@ -55,10 +80,13 @@ def log_event(logger: logging.Logger, level: int, event: str, **fields: object)
     Centralizes the event-token padding and value quoting so call sites read as
     ``log_event(logger, logging.DEBUG, "step.open", name=…, path=…)``. Guarded by
     ``isEnabledFor`` so nothing is formatted when audit logging is off.
+
+    Values for field names in ``SENSITIVE_FIELD_NAMES`` are redacted as
+    ``[REDACTED]`` to avoid logging sensitive data.
     """
     if not logger.isEnabledFor(level):
         return
-    body = " ".join(f"{key}={_fmt_value(value)}" for key, value in fields.items())
+    body = " ".join(_fmt_kv(key, value) for key, value in fields.items())
     logger.log(level, "%-*s %s", EVENT_WIDTH, event, body)
 
 

python/lib/sift_client/_tests/pytest_plugin/test_audit_log.py

@@ -0,0 +1,81 @@
+"""Tests for audit log redaction of sensitive field values."""
+
+import logging
+from io import StringIO
+
+from sift_client._internal.pytest_plugin.audit_log import (
+    _fmt_kv,
+    log_event,
+)
+
+
+def test_fmt_kv_redacts_sensitive_field_names():
+    """Field names in SENSITIVE_FIELD_NAMES should be redacted."""
+    assert _fmt_kv("password", "secret123") == "password=[REDACTED]"
+    assert _fmt_kv("token", "token123") == "token=[REDACTED]"
+    assert _fmt_kv("api_key", "key1") == "api_key=[REDACTED]"
+    assert _fmt_kv("apikey", "key2") == "apikey=[REDACTED]"
+    assert _fmt_kv("api-key", "key3") == "api-key=[REDACTED]"
+    assert _fmt_kv("name", "secret123") == "name=secret123"  # non-sensitive
+
+
+def test_log_event_redacts_sensitive_fields():
+    """Sensitive field values are redacted in log output."""
+    logger = logging.getLogger("sift_client.test")
+    logger.setLevel(logging.DEBUG)
+
+    stream = StringIO()
+    handler = logging.StreamHandler(stream)
+    handler.setLevel(logging.DEBUG)
+    logger.addHandler(handler)
+
+    try:
+        log_event(
+            logger,
+            logging.DEBUG,
+            "test.event",
+            password="secret123",
+            token="abc123",
+            name="test",
+        )
+        output = stream.getvalue()
+        assert "password=[REDACTED]" in output
+        assert "token=[REDACTED]" in output
+        assert "secret123" not in output
+        assert "abc123" not in output
+        assert "name=test" in output
+    finally:
+        logger.removeHandler(handler)
+
+
+def test_log_event_case_insensitive_redaction():
+    """Redaction works regardless of field name case."""
+    logger = logging.getLogger("sift_client.test2")
+    logger.setLevel(logging.DEBUG)
+
+    stream = StringIO()
+    handler = logging.StreamHandler(stream)
+    handler.setLevel(logging.DEBUG)
+    logger.addHandler(handler)
+
+    try:
+        log_event(
+            logger,
+            logging.DEBUG,
+            "test.event",
+            Password="pass1",
+            TOKEN="token1",
+            apiKey="key1",
+            api_key="key2",
+        )
+        output = stream.getvalue()
+        assert "Password=[REDACTED]" in output
+        assert "TOKEN=[REDACTED]" in output
+        assert "apiKey=[REDACTED]" in output
+        assert "api_key=[REDACTED]" in output
+        assert "pass1" not in output
+        assert "token1" not in output
+        assert "key1" not in output
+        assert "key2" not in output
+    finally:
+        logger.removeHandler(handler)