From 52563bfec24a775f86c9f7dd99bef9299f510006 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Fri, 14 Aug 2026 18:55:37 -0500 Subject: [PATCH 1/2] feat(audit): surface integrity_status through GET /audit/events HIPAA audit-integrity rollout: closes a real gap in the platform's own read API. PR2/PR#5 added audit_events.integrity_status and the worker has classified every event ("valid"/"invalid"/"unsigned") since PR3a's deployment; all six producers now sign (five merged, one -- API Gateway -- blocked on an external credential, tracked separately in that repo's issue #12, not touched here). None of that was visible through this repo's own GET /audit/events API: a platform_admin querying it had no way to see whether any event was ever actually verified, or to isolate the events that matter most -- the ones that failed verification. schemas/audit.py::AuditEventOut: adds integrity_status: str (never Optional -- the DB column is NOT NULL with server_default="unsigned", matching AuditEventRecord exactly). services/audit_query_service.py::list_audit_events: adds an integrity_status filter parameter, same SQL-filter style as every existing one (user_id/service/event_type/decision/timestamp range) -- no new pattern introduced. Lets a caller isolate integrity_status=invalid specifically (forged/tampered events) or the still-unsigned backlog, not just see the field per returned row. api/routes_audit_events.py: adds the matching integrity_status query param, passed straight through -- no new validation beyond Optional[str], since the DB column itself is the source of truth for valid values. Tests: updated the one existing exact-field-set assertion (test_response_contains_expected_fields) to include the new field and its default value; added 2 query-service filter tests (including that 'unsigned' correctly matches server_default rows created without an explicit value) and 1 HTTP-level filter test. 252 passed (249 baseline + 3 new), 0 regressions. Co-Authored-By: Claude Sonnet 5 --- api/routes_audit_events.py | 9 +++++++++ schemas/audit.py | 8 ++++++++ services/audit_query_service.py | 3 +++ tests/test_audit_query_service.py | 27 +++++++++++++++++++++++++++ tests/test_routes_audit_events.py | 31 ++++++++++++++++++++++++++++++- 5 files changed, 77 insertions(+), 1 deletion(-) diff --git a/api/routes_audit_events.py b/api/routes_audit_events.py index 6305cb6..3daa503 100644 --- a/api/routes_audit_events.py +++ b/api/routes_audit_events.py @@ -26,6 +26,14 @@ def list_audit_events( decision: Optional[str] = Query(None), from_timestamp: Optional[datetime] = Query(None), to_timestamp: Optional[datetime] = Query(None), + # HIPAA audit-integrity rollout: lets a platform_admin ask "show me + # every event that failed signature verification" (integrity_status= + # invalid) or isolate the still-unsigned backlog -- not just see the + # field per-row. No validation beyond Optional[str]: the DB column + # itself is the source of truth for what values exist ("valid"/ + # "invalid"/"unsigned" today, see audit/config.py's classifier), and + # an unrecognized value here just filters to zero rows, not an error. + integrity_status: Optional[str] = Query(None), db: Session = Depends(get_db), _admin: dict = Depends(require_platform_admin), ) -> AuditEventListResponse: @@ -39,6 +47,7 @@ def list_audit_events( decision=decision, from_timestamp=from_timestamp, to_timestamp=to_timestamp, + integrity_status=integrity_status, ) total_pages = (total + page_size - 1) // page_size if total else 0 return AuditEventListResponse( diff --git a/schemas/audit.py b/schemas/audit.py index cd4696f..671eabc 100644 --- a/schemas/audit.py +++ b/schemas/audit.py @@ -23,6 +23,14 @@ class AuditEventOut(BaseModel): trace_id: Optional[str] = None context: dict[str, Any] created_at: datetime + # HIPAA audit-integrity rollout: PR2/PR#5 added this column and the + # worker has classified every event ("valid"/"invalid"/"unsigned") + # since PR3a's deployment, but nothing surfaced it through this read + # API until now -- a platform_admin querying /audit/events had no way + # to see whether any event was ever actually verified. Always present + # (DB column is NOT NULL with server_default="unsigned"), never + # Optional -- matches AuditEventRecord.integrity_status exactly. + integrity_status: str class AuditEventListResponse(BaseModel): diff --git a/services/audit_query_service.py b/services/audit_query_service.py index 0b75f0c..0d28fbe 100644 --- a/services/audit_query_service.py +++ b/services/audit_query_service.py @@ -16,6 +16,7 @@ def list_audit_events( decision: Optional[str] = None, from_timestamp: Optional[datetime] = None, to_timestamp: Optional[datetime] = None, + integrity_status: Optional[str] = None, ) -> tuple[list[AuditEventRecord], int]: """Returns (page of AuditEventRecord rows, total matching rows). @@ -42,6 +43,8 @@ def list_audit_events( query = query.filter(AuditEventRecord.timestamp >= from_timestamp) if to_timestamp is not None: query = query.filter(AuditEventRecord.timestamp <= to_timestamp) + if integrity_status is not None: + query = query.filter(AuditEventRecord.integrity_status == integrity_status) total = query.count() diff --git a/tests/test_audit_query_service.py b/tests/test_audit_query_service.py index 19df836..6b37285 100644 --- a/tests/test_audit_query_service.py +++ b/tests/test_audit_query_service.py @@ -21,6 +21,8 @@ def _add(db_session, event_id, minutes_offset=0, **overrides): trace_id=overrides.get("trace_id"), context=overrides.get("context", {}), ) + if "integrity_status" in overrides: + row.integrity_status = overrides["integrity_status"] db_session.add(row) return row @@ -94,6 +96,31 @@ def test_filters_by_timestamp_range(db_session): assert rows[0].event_id == "e2" +def test_filters_by_integrity_status(db_session): + _add(db_session, "e1", integrity_status="valid") + _add(db_session, "e2", integrity_status="invalid") + _add(db_session, "e3") # unspecified -- DB server_default="unsigned" applies + db_session.commit() + + rows, total = audit_query_service.list_audit_events( + db_session, page=1, page_size=20, integrity_status="invalid" + ) + assert total == 1 + assert rows[0].event_id == "e2" + + +def test_filters_by_integrity_status_unsigned_matches_the_default(db_session): + _add(db_session, "e1", integrity_status="valid") + _add(db_session, "e2") + db_session.commit() + + rows, total = audit_query_service.list_audit_events( + db_session, page=1, page_size=20, integrity_status="unsigned" + ) + assert total == 1 + assert rows[0].event_id == "e2" + + def test_combined_filters_no_cross_leakage(db_session): """A row matching only one of two filters must not appear -- filters combine with AND, not OR.""" diff --git a/tests/test_routes_audit_events.py b/tests/test_routes_audit_events.py index c2086c4..b5e2d7c 100644 --- a/tests/test_routes_audit_events.py +++ b/tests/test_routes_audit_events.py @@ -102,10 +102,15 @@ def test_response_contains_expected_fields(audit_events_client): assert set(item.keys()) == { "event_id", "timestamp", "service", "event_type", "user_id", "action", "resource", "decision", "reason", "trace_id", "context", - "created_at", + "created_at", "integrity_status", } assert item["event_id"] == "evt-0" assert item["context"] == {"i": 0} + # _seed() rows are constructed without an explicit integrity_status -- + # the DB column's own server_default="unsigned" (0002_integrity_status) + # applies, same as every real historical event before any producer + # signed. + assert item["integrity_status"] == "unsigned" def test_pagination_works(audit_events_client): @@ -199,6 +204,30 @@ def test_filter_by_decision_and_event_type_via_query_params(audit_events_client) assert body["items"][0]["event_id"] == "e2" +def test_filter_by_integrity_status_via_query_param(audit_events_client): + client, sessions = audit_events_client + db = sessions() + db.add(AuditEventRecord( + event_id="e1", timestamp=datetime(2026, 1, 1), service="tes", + event_type="workflow_execution_denied", context={}, integrity_status="valid", + )) + db.add(AuditEventRecord( + event_id="e2", timestamp=datetime(2026, 1, 1), service="tes", + event_type="workflow_execution_denied", context={}, integrity_status="invalid", + )) + db.commit() + db.close() + + resp = client.get( + "/audit/events", headers=_auth_headers(), params={"integrity_status": "invalid"} + ) + + body = resp.json() + assert body["total"] == 1 + assert body["items"][0]["event_id"] == "e2" + assert body["items"][0]["integrity_status"] == "invalid" + + # --------------------------------------------------------------------------- # Existing endpoints unaffected # --------------------------------------------------------------------------- From fd40cb207aa42e4f5400daa7117fcd8a4f6b16d9 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Fri, 14 Aug 2026 21:50:33 -0500 Subject: [PATCH 2/2] Fix Ruff UP045 typing lint in audit events API Lint-only, no behavior change. PR #9's own CI run reported 34 errors across 4 rule categories once the touched files entered ruff's changed-file set (ruff lints changed files whole, not just changed lines, so this also caught pre-existing debt the files already had before PR #9 touched them) -- not just UP045 as initially assumed: - UP045 (19): Optional[X] -> X | None, across api/routes_audit_events.py, schemas/audit.py, services/audit_query_service.py. Safe on this repo's actual Python 3.11 runtime (Dockerfile/Dockerfile.worker both pin python:3.11-slim) -- no `from __future__ import annotations` needed, matching schemas/audit.py's own pre-existing use of dict[str, Any] without one. - F401 (3): typing.Optional left unused in the same three files as a direct, mechanical consequence of the UP045 fix above -- not a separate change. - RUF059 (1): unused unpacked `rows` in tests/test_audit_query_service.py -- renamed to `_rows`, matching ruff's own dummy-variable convention. - B008 (4): Query(...)/Depends(...) as argument defaults in api/routes_audit_events.py. This is FastAPI's own documented, required pattern, not a mutable-default bug -- ruff's suggested "real" fix (move the call inside the function body) would restructure the route signature well beyond a lint fix. Suppressed narrowly with `# noqa: B008` plus a one-line reason on exactly the 4 flagged lines, matching this repo's existing narrow-noqa convention (8153d38's `# noqa: BLE001` for the equivalent situation on PR3b). - DTZ001 (10): naive datetime(...) in the two test files. AuditEventRecord.timestamp is a naive Column(DateTime) (no timezone=True) -- adding tzinfo here would mismatch the column, not fix anything, and the actually-correct fix (a timezone-aware column) is a schema change out of scope for a lint-only commit. Suppressed narrowly with `# noqa: DTZ001` plus a reason on exactly the 10 flagged lines, same convention as B008 above. Zero API-behavior change: request/response shapes, the integrity_status filter's semantics, and the DB query itself are byte-for-byte identical to 52563bf -- confirmed by re-running the PR's own focused tests (tests/test_audit_query_service.py + tests/test_routes_audit_events.py, 27 tests) and the full suite (252 passed, 0 regressions, matching the 252 baseline 52563bf's own commit message already reported). Co-Authored-By: Claude Sonnet 5 --- api/routes_audit_events.py | 21 ++++++++++----------- schemas/audit.py | 12 ++++++------ services/audit_query_service.py | 15 +++++++-------- tests/test_audit_query_service.py | 8 ++++---- tests/test_routes_audit_events.py | 14 +++++++------- 5 files changed, 34 insertions(+), 36 deletions(-) diff --git a/api/routes_audit_events.py b/api/routes_audit_events.py index 3daa503..3e434cb 100644 --- a/api/routes_audit_events.py +++ b/api/routes_audit_events.py @@ -1,5 +1,4 @@ from datetime import datetime -from typing import Optional from fastapi import APIRouter, Depends, Query from sqlalchemy.orm import Session @@ -20,22 +19,22 @@ def list_audit_events( page: int = Query(1, ge=1), page_size: int = Query(20, ge=1, le=100), - user_id: Optional[str] = Query(None), - service: Optional[str] = Query(None), - event_type: Optional[str] = Query(None), - decision: Optional[str] = Query(None), - from_timestamp: Optional[datetime] = Query(None), - to_timestamp: Optional[datetime] = Query(None), + user_id: str | None = Query(None), + service: str | None = Query(None), + event_type: str | None = Query(None), + decision: str | None = Query(None), + from_timestamp: datetime | None = Query(None), # noqa: B008 -- FastAPI's own documented query-param pattern, not a mutable-default bug + to_timestamp: datetime | None = Query(None), # noqa: B008 -- FastAPI's own documented query-param pattern, not a mutable-default bug # HIPAA audit-integrity rollout: lets a platform_admin ask "show me # every event that failed signature verification" (integrity_status= # invalid) or isolate the still-unsigned backlog -- not just see the - # field per-row. No validation beyond Optional[str]: the DB column + # field per-row. No validation beyond `str | None`: the DB column # itself is the source of truth for what values exist ("valid"/ # "invalid"/"unsigned" today, see audit/config.py's classifier), and # an unrecognized value here just filters to zero rows, not an error. - integrity_status: Optional[str] = Query(None), - db: Session = Depends(get_db), - _admin: dict = Depends(require_platform_admin), + integrity_status: str | None = Query(None), + db: Session = Depends(get_db), # noqa: B008 -- FastAPI's own documented dependency-injection pattern, not a mutable-default bug + _admin: dict = Depends(require_platform_admin), # noqa: B008 -- FastAPI's own documented dependency-injection pattern, not a mutable-default bug ) -> AuditEventListResponse: rows, total = audit_query_service.list_audit_events( db, diff --git a/schemas/audit.py b/schemas/audit.py index 671eabc..18a4905 100644 --- a/schemas/audit.py +++ b/schemas/audit.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Any, Optional +from typing import Any from pydantic import BaseModel, ConfigDict @@ -15,12 +15,12 @@ class AuditEventOut(BaseModel): timestamp: datetime service: str event_type: str - user_id: Optional[str] = None + user_id: str | None = None action: str - resource: Optional[str] = None - decision: Optional[str] = None - reason: Optional[str] = None - trace_id: Optional[str] = None + resource: str | None = None + decision: str | None = None + reason: str | None = None + trace_id: str | None = None context: dict[str, Any] created_at: datetime # HIPAA audit-integrity rollout: PR2/PR#5 added this column and the diff --git a/services/audit_query_service.py b/services/audit_query_service.py index 0d28fbe..600bcdb 100644 --- a/services/audit_query_service.py +++ b/services/audit_query_service.py @@ -1,5 +1,4 @@ from datetime import datetime -from typing import Optional from sqlalchemy.orm import Session @@ -10,13 +9,13 @@ def list_audit_events( db: Session, page: int, page_size: int, - user_id: Optional[str] = None, - service: Optional[str] = None, - event_type: Optional[str] = None, - decision: Optional[str] = None, - from_timestamp: Optional[datetime] = None, - to_timestamp: Optional[datetime] = None, - integrity_status: Optional[str] = None, + user_id: str | None = None, + service: str | None = None, + event_type: str | None = None, + decision: str | None = None, + from_timestamp: datetime | None = None, + to_timestamp: datetime | None = None, + integrity_status: str | None = None, ) -> tuple[list[AuditEventRecord], int]: """Returns (page of AuditEventRecord rows, total matching rows). diff --git a/tests/test_audit_query_service.py b/tests/test_audit_query_service.py index 6b37285..19c9633 100644 --- a/tests/test_audit_query_service.py +++ b/tests/test_audit_query_service.py @@ -10,7 +10,7 @@ def _add(db_session, event_id, minutes_offset=0, **overrides): row = AuditEventRecord( event_id=event_id, - timestamp=datetime(2026, 1, 1, 12, 0, 0) + timedelta(minutes=minutes_offset), + timestamp=datetime(2026, 1, 1, 12, 0, 0) + timedelta(minutes=minutes_offset), # noqa: DTZ001 -- AuditEventRecord.timestamp is a naive DateTime column (db/models.py); an aware value here would mismatch it, not fix anything service=overrides.get("service", "auth"), event_type=overrides.get("event_type", "auth_login"), user_id=overrides.get("user_id", "u1"), @@ -89,8 +89,8 @@ def test_filters_by_timestamp_range(db_session): db_session, page=1, page_size=20, - from_timestamp=datetime(2026, 1, 1, 12, 5, 0), - to_timestamp=datetime(2026, 1, 1, 12, 15, 0), + from_timestamp=datetime(2026, 1, 1, 12, 5, 0), # noqa: DTZ001 -- matches the naive AuditEventRecord.timestamp column being filtered + to_timestamp=datetime(2026, 1, 1, 12, 15, 0), # noqa: DTZ001 -- matches the naive AuditEventRecord.timestamp column being filtered ) assert total == 1 assert rows[0].event_id == "e2" @@ -142,7 +142,7 @@ def test_no_filters_returns_all(db_session): _add(db_session, "e3") db_session.commit() - rows, total = audit_query_service.list_audit_events(db_session, page=1, page_size=20) + _rows, total = audit_query_service.list_audit_events(db_session, page=1, page_size=20) assert total == 3 diff --git a/tests/test_routes_audit_events.py b/tests/test_routes_audit_events.py index b5e2d7c..375c415 100644 --- a/tests/test_routes_audit_events.py +++ b/tests/test_routes_audit_events.py @@ -27,7 +27,7 @@ def _seed(session_factory, count=3): db.add( AuditEventRecord( event_id=f"evt-{i}", - timestamp=datetime(2026, 1, 1, 12, 0, 0) + timedelta(minutes=i), + timestamp=datetime(2026, 1, 1, 12, 0, 0) + timedelta(minutes=i), # noqa: DTZ001 -- AuditEventRecord.timestamp is a naive DateTime column (db/models.py) service="auth", event_type="auth_login", user_id="u1", @@ -160,11 +160,11 @@ def test_filter_by_service_via_query_param(audit_events_client): client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( - event_id="e1", timestamp=datetime(2026, 1, 1), service="auth", + event_id="e1", timestamp=datetime(2026, 1, 1), service="auth", # noqa: DTZ001 -- naive DateTime column event_type="auth_login", context={}, )) db.add(AuditEventRecord( - event_id="e2", timestamp=datetime(2026, 1, 1), service="policy", + event_id="e2", timestamp=datetime(2026, 1, 1), service="policy", # noqa: DTZ001 -- naive DateTime column event_type="policy_decision", context={}, )) db.commit() @@ -183,11 +183,11 @@ def test_filter_by_decision_and_event_type_via_query_params(audit_events_client) client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( - event_id="e1", timestamp=datetime(2026, 1, 1), service="auth", + event_id="e1", timestamp=datetime(2026, 1, 1), service="auth", # noqa: DTZ001 -- naive DateTime column event_type="user_suspended", decision="success", context={}, )) db.add(AuditEventRecord( - event_id="e2", timestamp=datetime(2026, 1, 1), service="auth", + event_id="e2", timestamp=datetime(2026, 1, 1), service="auth", # noqa: DTZ001 -- naive DateTime column event_type="user_suspended", decision="failure", context={}, )) db.commit() @@ -208,11 +208,11 @@ def test_filter_by_integrity_status_via_query_param(audit_events_client): client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( - event_id="e1", timestamp=datetime(2026, 1, 1), service="tes", + event_id="e1", timestamp=datetime(2026, 1, 1), service="tes", # noqa: DTZ001 -- naive DateTime column event_type="workflow_execution_denied", context={}, integrity_status="valid", )) db.add(AuditEventRecord( - event_id="e2", timestamp=datetime(2026, 1, 1), service="tes", + event_id="e2", timestamp=datetime(2026, 1, 1), service="tes", # noqa: DTZ001 -- naive DateTime column event_type="workflow_execution_denied", context={}, integrity_status="invalid", )) db.commit()