fix(audit): redact credential values in audit entries and log events

CHANGELOG.md

@@ -15,6 +15,19 @@ All notable changes to this project are documented in this file. The format is b
 ## [Unreleased]
 
 ### Security
+- **Audit hardening:** audit entries no longer record credential values.
+  Redacted fields retain change indicators using `[redacted]`; credential
+  primary-key identities are masked too. Migration `b8d4f2a6c901` redacts
+  historical credential entries and, conservatively, all historical settings
+  values. Downgrading does not restore these values. Operators should rotate
+  credentials saved through the settings UI on earlier versions, and handle
+  existing backups and audit exports according to their retention policies.
+- Entity update/delete log events mask primary keys that are themselves
+  credentials (session and refresh tokens), as the audit trail does.
+
+### Removed
+- `settings.constants.SENSITIVE_KEYS`; the set now lives in
+  `simple_module_core.secret_fields.SENSITIVE_KEYS`.
 - **Behaviour change (#687):** delegated permission edits are bounded by
   platform-level access. Global user and role permission writes require
   platform-level `permissions.manage`; tenant-role mappings do not contribute

docs/modules/audit_log.md

@@ -221,3 +221,33 @@ pip install simple_module_audit_log
 ```
 
 It depends on no other module. Actor names, the Actor search and actor links come from the app's [user directory](/framework/user-directory), so they work under `users`, `keycloak` (users who have signed in) or no provider at all (raw ids). The module also installs the core [activity feed](/framework/user-directory#activity-feed) the users edit page reads.
+
+## Credential redaction
+
+Set `__audit_redact_fields__: ClassVar[set[str]] = {"credential"}` on a
+model to keep a field in the audit diff while replacing every recorded `old`
+and `new` value with `[redacted]`. Unlike `__audit_exclude_fields__`, which
+omits a field entirely, redaction preserves the fact that it changed.
+Exclusion takes precedence. A redacted primary key also masks `entity_id`.
+
+For row-dependent rules, define a static `__audit_redact_row__(row)` method
+returning a set of field names. ORM capture evaluates the current and pre-edit
+column dictionaries and redacts fields identified by either. Bulk and manual
+capture pass `None`; the predicate must conservatively return every field
+that could hold a credential when row context is unavailable. Manual capture
+accepts the mapped class or its registered class name to apply its policy.
+
+Built-in policies cover stored secret settings, user password hashes, OAuth
+credentials, tenant invitation hashes, bearer tokens, lease ownership tokens
+and auth refresh outcomes.
+Settings use the same secret rule as the settings UI. Non-secret ORM settings
+retain their values; bulk settings values are always redacted because the
+statement may affect rows with different keys or prior values. API token
+hashes remain excluded from automatic diffs.
+
+Migration `b8d4f2a6c901` hardens existing entries, conservatively redacting all
+historical settings values because old diffs may lack the key or type needed
+to classify them. It also masks historical credential primary-key identities.
+Its downgrade is a no-op: credential values cannot be recovered. Operators
+should rotate credentials saved through the settings UI on earlier versions;
+the migration does not rewrite backups or previously exported audit data.

framework/core/simple_module_core/secret_fields.py

@@ -9,7 +9,7 @@
 from __future__ import annotations
 
 import re
-from typing import Any, get_args, get_origin
+from typing import Any, Final, get_args, get_origin
 from urllib.parse import urlsplit
 
 from pydantic import BaseModel, SecretBytes, SecretStr
@@ -20,7 +20,15 @@
 _SECRET_PATTERNS = re.compile(
     r"(password|secret|api[_-]?key|private[_-]?key|token[_-]?secret)", re.I
 )
+
 SECRET_MASK = "••••••••"
+# Keys whose stored value must never leave the settings API, the raw
+# key/value browse screen or the audit trail. ``host.secret_key`` is the session-signing key: the
+# hosting layer generates and persists it here when SM_SECRET_KEY is unset, and
+# anyone who can read it can forge a session cookie for any account. Masked on
+# the way out only — the boot-time reader goes straight to SQL, so hiding it
+# here costs nothing.
+SENSITIVE_KEYS: Final = frozenset({"host.secret_key"})
 
 # Value types that cannot carry credential material, so a secret-ish *name*
 # on one of them is a false positive rather than something to hide.
@@ -92,7 +100,10 @@ def conceals_secret(name: str, value: Any, value_type: str | None = None) -> boo
     ``broker_url``, while that field's password-free default is still shown.
     """
     return (
-        name in _TYPED_SECRET_KEYS or is_named_secret(name, value_type) or embeds_credential(value)
+        name in SENSITIVE_KEYS
+        or name in _TYPED_SECRET_KEYS
+        or is_named_secret(name, value_type)
+        or embeds_credential(value)
     )
 
 

framework/db/simple_module_db/_bulk_audit_records.py

@@ -10,7 +10,8 @@
 from sqlalchemy import inspect as sa_inspect
 from sqlalchemy.orm import ORMExecuteState, Session
 
-from simple_module_db.audit import AuditRecord, _serialize, format_pk
+from simple_module_db.audit import AuditRecord, _excluded_fields, _serialize, format_pk
+from simple_module_db.audit_redaction import model_for_audit, redact_changes, redact_entity_id
 from simple_module_db.query_filter import engine_policy
 
 logger = logging.getLogger("simple_module_db.bulk_audit")
@@ -29,8 +30,16 @@
 class _Ctx:
     """What one audited statement records, built before it runs."""
 
-    def __init__(self, entity_type: str, action: str, changes: list[dict[str, Any]]) -> None:
+    def __init__(
+        self,
+        entity_type: str,
+        action: str,
+        changes: list[dict[str, Any]],
+        model: type | None = None,
+    ) -> None:
         self.entity_type = entity_type
+        # The statement's own class when known: a name lookup scans the registry.
+        self._model = model or model_for_audit(entity_type)
         self.action = action
         self.row_actions = [action]
         self.changes = changes
@@ -45,7 +54,11 @@ def record(
     ) -> AuditRecord:
         return AuditRecord(
             entity_type=self.entity_type,
-            entity_id=entity_id,
+            entity_id=(
+                entity_id
+                if entity_id == SUMMARY_ENTITY_ID
+                else redact_entity_id(self._model, entity_id)
+            ),
             action=action or self.action,
             changes=self.changes if changes is None else changes,
             user_id=self.user_id,
@@ -113,6 +126,11 @@ def record_bulk_audit(
         return
     name = entity_type if isinstance(entity_type, str) else entity_type.__name__
     diff = [{"field": k, "new": _serialize(v)} for k, v in (changes or {}).items()]
-    ctx = _Ctx(name, action, diff)
+    ctx = _Ctx(name, action, diff, entity_type if isinstance(entity_type, type) else None)
+    model = ctx._model
+    if model is not None:
+        ctx.changes = redact_changes(
+            model, [c for c in diff if c["field"] not in _excluded_fields(model)]
+        )
     keys = [SUMMARY_ENTITY_ID] if ids is None else [str(i) for i in ids]
     _deliver(sync, callback, [ctx.record(k) for k in keys])

framework/db/simple_module_db/audit.py

@@ -23,6 +23,7 @@
 from sqlalchemy import inspect as sa_inspect
 from sqlalchemy.orm import Session
 
+from simple_module_db.audit_redaction import redact_changes, redact_entity_id
 from simple_module_db.mixins import SoftDeleteMixin
 
 # Fields injected by AuditMixin — always excluded from change diffs
@@ -97,8 +98,10 @@ def _entity_pk_str(obj: object) -> str:
     if identity is None:
         # Pre-flush: fall back to the mapper-level key attrs
         mapper = inspector.mapper
-        return format_pk([getattr(obj, col.key) for col in mapper.primary_key])
-    return format_pk(identity)
+        pk = format_pk([getattr(obj, col.key) for col in mapper.primary_key])
+    else:
+        pk = format_pk(identity)
+    return redact_entity_id(type(obj), pk)
 
 
 def _column_names(obj: object) -> list[str]:
@@ -128,7 +131,7 @@ def _column_diffs(obj: object, inspector: Any, excl: frozenset[str]) -> list[dic
         old_val = hist.deleted[0] if hist.deleted else None
         new_val = hist.added[0] if hist.added else None
         changes.append({"field": col_name, "old": _serialize(old_val), "new": _serialize(new_val)})
-    return changes
+    return redact_changes(type(obj), changes, obj)
 
 
 # ---------------------------------------------------------------------------
@@ -182,7 +185,7 @@ def _get_pk_cols(obj: object) -> set[str]:
                 obj_ref=obj,
                 entity_type=type(obj).__name__,
                 action="created",
-                changes=changes,
+                changes=redact_changes(type(obj), changes, obj),
                 user_id=user_id,
                 correlation_id=correlation_id,
             )

framework/db/simple_module_db/audit_redaction.py

@@ -0,0 +1,71 @@
+"""Shared credential redaction for flush, statement and manual audit capture."""
+
+from __future__ import annotations
+
+from typing import Any, Final
+
+from sqlalchemy import inspect
+
+AUDIT_REDACTED: Final = "[redacted]"
+
+
+def model_for_audit(model: type | str) -> type | None:
+    """Accept a mapped class or its registered class name; None for unknown names."""
+    if isinstance(model, type):
+        return model
+    from simple_module_db.model_registry import classes_by_table
+
+    return next((cls for cls in classes_by_table.values() if cls.__name__ == model), None)
+
+
+def redact_changes(
+    model: type, changes: list[dict[str, Any]], obj: object | None = None
+) -> list[dict[str, Any]]:
+    """Keep fields and diff shape, replacing credential values before delivery.
+
+    A row predicate receives both the current and pre-edit column values. With
+    no row (statement/manual capture) it receives None and must fail closed.
+    """
+    if not changes:
+        return changes
+    fields = set(getattr(model, "__audit_redact_fields__", ()))
+    predicate = getattr(model, "__audit_redact_row__", None)
+    if predicate is not None:
+        if obj is None:
+            fields.update(predicate(None))
+        else:
+            state = inspect(obj)
+            current: dict[str, Any] = {}
+            previous: dict[str, Any] = {}
+            for attr in state.mapper.column_attrs:
+                key = attr.key
+                value = current[key] = state.dict.get(key)
+                history = state.attrs[key].history
+                if history.has_changes():
+                    previous[key] = history.deleted[0] if history.deleted else None
+                else:
+                    previous[key] = value
+            fields.update(predicate(current))
+            if state.persistent:
+                fields.update(predicate(previous))
+    if not fields:
+        return changes
+    return [
+        {
+            k: AUDIT_REDACTED if c["field"] in fields and k in {"old", "new"} else v
+            for k, v in c.items()
+        }
+        for c in changes
+    ]
+
+
+def pk_is_redacted(model: type | None) -> bool:
+    """True when a primary-key column of ``model`` is itself a redacted credential."""
+    fields = getattr(model, "__audit_redact_fields__", None)
+    if model is None or not fields:
+        return False
+    return any(col.key in fields for col in inspect(model).primary_key)
+
+
+def redact_entity_id(model: type | None, entity_id: str) -> str:
+    return AUDIT_REDACTED if pk_is_redacted(model) else entity_id

framework/db/simple_module_db/bulk_audit.py

@@ -52,6 +52,7 @@
     record_bulk_audit,
 )
 from simple_module_db.audit import _excluded_fields, format_pk
+from simple_module_db.audit_redaction import redact_changes
 from simple_module_db.bulk_audit_targets import _is_set, _shown, _targets
 from simple_module_db.query_filter import engine_policy
 from simple_module_db.tenancy import ALL_TENANTS_OPTION
@@ -85,7 +86,7 @@ def _plan(execute_state: ORMExecuteState, cls: type) -> _Ctx | None:
     """The record shape for this statement, or ``None`` when nothing is audited."""
     name = cls.__name__
     if execute_state.is_delete:
-        return _Ctx(name, ACTION_BULK_DELETED, [])
+        return _Ctx(name, ACTION_BULK_DELETED, [], cls)
     rows, many = _targets(execute_state, cls)
     default = ACTION_BULK_INSERTED if execute_state.is_insert else ACTION_BULK_UPDATED
     # The soft-delete flag picks a row's action; it is never a change, and
@@ -112,7 +113,7 @@ def _plan(execute_state: ORMExecuteState, cls: type) -> _Ctx | None:
         changes = [{"field": k} for k in kept]
     else:
         changes = [{"field": k, "new": _shown(rows[0][k])} for k in kept]
-    ctx = _Ctx(name, actions[0] if actions else default, changes)
+    ctx = _Ctx(name, actions[0] if actions else default, redact_changes(cls, changes), cls)
     ctx.row_actions = actions or [default]
     return ctx
 

framework/db/simple_module_db/leases.py

@@ -19,7 +19,7 @@
 import contextlib
 from collections.abc import AsyncGenerator
 from datetime import UTC, datetime, timedelta
-from typing import Any, Final
+from typing import Any, ClassVar, Final
 
 from sqlalchemy import and_, func, select, text, update
 from sqlalchemy.dialects import postgresql, sqlite
@@ -48,6 +48,8 @@
 class LeaseRow(Base, table=True):  # ty: ignore[unsupported-base]
     """One lease slot. ``holder`` is the owner's token, ``""`` when never held."""
 
+    __audit_redact_fields__: ClassVar[set[str]] = {"holder"}
+
     __tablename__ = TABLE_LEASE
 
     name: str = Field(primary_key=True, max_length=NAME_MAX_LENGTH)

framework/db/simple_module_db/listeners.py

@@ -13,6 +13,7 @@
 from sqlalchemy.orm import Session
 
 from simple_module_db._persisted_owner import mark_unverified, owner_mark, restore_owner_mark
+from simple_module_db.audit_redaction import AUDIT_REDACTED, pk_is_redacted
 from simple_module_db.bulk_audit import audit_statement
 from simple_module_db.bulk_guard import install_bulk_guard
 from simple_module_db.flush_guard import guard_flush
@@ -76,8 +77,14 @@ def _entity_label(obj: object) -> str:
 
 
 def _entity_pk(obj: object) -> object:
-    """Return the primary key value(s) if available, else None."""
+    """Return the primary key value(s) if available, else None.
+
+    A primary key that is itself a credential (a session or refresh token) is
+    masked: this value goes to the application log.
+    """
     try:
+        if pk_is_redacted(type(obj)):
+            return AUDIT_REDACTED
         inspector = sa_inspect(obj)
         if inspector is None:
             return None

framework/db/simple_module_db/stored_settings.py

@@ -16,6 +16,7 @@
 from typing import Final
 
 from pydantic_settings import BaseSettings
+from simple_module_core.secret_fields import conceals_secret
 from simple_module_core.settings_hydrate import build_settings
 from sqlalchemy import Select, UniqueConstraint, inspect, select
 from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession
@@ -84,6 +85,17 @@ class Setting(Base, AuditMixin, table=True):  # ty: ignore[unsupported-base]
     inputs that don't parse.
     """
 
+    @staticmethod
+    def __audit_redact_row__(row: dict | None) -> set[str]:
+        # Fail closed: no row (bulk/manual capture) or a partly loaded one hides the value.
+        if (
+            row is None
+            or any(row.get(k) is None for k in (COL_KEY, COL_VALUE, COL_VALUE_TYPE))
+            or conceals_secret(row[COL_KEY], row[COL_VALUE], row[COL_VALUE_TYPE])
+        ):
+            return {COL_VALUE}
+        return set()
+
     __tablename__ = TABLE_SETTING
     __table_args__ = (UniqueConstraint(COL_SCOPE, COL_SCOPE_ID, COL_KEY, name=UQ_SCOPE_KEY),)