From 3791095c2a60eae78954871f26ef51cf0a07ac14 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Fri, 18 Sep 2026 15:53:01 -0500 Subject: [PATCH] docs: improve test suite documentation Co-Authored-By: Claude Sonnet 5 --- tests/__init__.py | 4 + tests/_mysql_integration_guard.py | 2 + tests/_redis_integration_guard.py | 2 + tests/conftest.py | 12 +++ tests/test_audit_health_service.py | 35 +++++++ tests/test_audit_query_service.py | 27 +++++- ...st_backup_restore_integrity_integration.py | 11 +++ tests/test_classify_event_integrity.py | 15 +++ tests/test_context.py | 21 +++++ tests/test_db_models.py | 8 +- tests/test_db_session.py | 6 +- tests/test_decorators.py | 17 ++++ tests/test_deps.py | 15 ++- tests/test_deps_audit.py | 6 +- tests/test_events.py | 18 ++++ tests/test_identity.py | 17 +++- tests/test_jwt_verify.py | 20 ++++ tests/test_logger.py | 8 ++ tests/test_logger_signing.py | 6 ++ tests/test_migrations.py | 11 +++ tests/test_models.py | 9 ++ tests/test_mysql_integration_guard.py | 11 +++ .../test_no_destructive_stream_operations.py | 5 + tests/test_no_mutation_routes.py | 3 + tests/test_processor.py | 20 ++++ .../test_producer_contract_reconciliation.py | 4 + tests/test_record_integrity.py | 18 ++++ tests/test_redis_acl_safety.py | 91 +++++++++++++++++++ ...test_retention_immutability_integration.py | 23 +++++ tests/test_retention_integrity_health.py | 11 +++ tests/test_routes_audit.py | 12 +++ tests/test_routes_audit_events.py | 22 ++++- tests/test_routes_audit_health.py | 9 ++ tests/test_routes_audit_safe.py | 19 ++++ tests/test_security_alerts.py | 23 +++++ tests/test_security_edge_cases.py | 22 ++++- tests/test_signing.py | 23 +++++ tests/test_sink.py | 14 ++- tests/test_source_semantics_sat4.py | 8 ++ tests/test_stream.py | 25 +++++ tests/test_worker.py | 24 ++++- .../test_worker_integration_real_backends.py | 11 +++ tests/test_worker_nogroup_recovery.py | 10 ++ tests/test_worker_pel_recovery.py | 9 ++ tests/test_worker_pel_recovery_integration.py | 18 ++++ tests/test_worker_quarantine_integration.py | 16 ++++ 46 files changed, 711 insertions(+), 10 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index e69de29..7f27bbd 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,4 @@ +"""Test package for the OmniBioAI security audit service. + +Developer: Manish Kumar +""" diff --git a/tests/_mysql_integration_guard.py b/tests/_mysql_integration_guard.py index d453417..901bf25 100644 --- a/tests/_mysql_integration_guard.py +++ b/tests/_mysql_integration_guard.py @@ -27,6 +27,8 @@ already uses 33061). There is deliberately no ALLOW_PRODUCTION_TESTS-style escape hatch -- there is no supported mode where these tests intentionally operate against production. + +Developer: Manish Kumar """ from __future__ import annotations diff --git a/tests/_redis_integration_guard.py b/tests/_redis_integration_guard.py index d69177f..9817b9d 100644 --- a/tests/_redis_integration_guard.py +++ b/tests/_redis_integration_guard.py @@ -33,6 +33,8 @@ different port. There is deliberately no escape hatch -- there is no supported mode where these tests intentionally operate against the shared production-adjacent instance. + +Developer: Manish Kumar """ from __future__ import annotations diff --git a/tests/conftest.py b/tests/conftest.py index 94d891f..ac41783 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,11 @@ +"""Shared pytest fixtures for the security audit test suite: mocked async/sync Redis clients, an +AuditLogger and StreamReader built against those mocks, and isolated SQLite databases for the +audit_events table and the /audit/events route. Nothing here opens a real Redis or MySQL +connection. + +Developer: Manish Kumar +""" + from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -5,16 +13,19 @@ @pytest.fixture def mock_async_redis(): + """Provide an AsyncMock standing in for the async Redis client used by AuditLogger.""" return AsyncMock() @pytest.fixture def mock_sync_redis(): + """Provide a MagicMock standing in for the sync Redis client used by StreamReader.""" return MagicMock() @pytest.fixture def audit_logger(mock_async_redis): + """Build an AuditLogger with Redis patched out, bound to the mocked async Redis client.""" with patch("audit.logger.redis") as mock_redis_module: mock_redis_module.from_url.return_value = mock_async_redis from audit.logger import AuditLogger @@ -25,6 +36,7 @@ def audit_logger(mock_async_redis): @pytest.fixture def stream_reader(mock_sync_redis): + """Build a StreamReader with Redis patched out, bound to the mocked sync Redis client.""" with patch("consumers.stream_reader.redis") as mock_redis_module: mock_redis_module.from_url.return_value = mock_sync_redis from consumers.stream_reader import StreamReader diff --git a/tests/test_audit_health_service.py b/tests/test_audit_health_service.py index a392153..1015dea 100644 --- a/tests/test_audit_health_service.py +++ b/tests/test_audit_health_service.py @@ -9,6 +9,8 @@ recording stub -- this checks WHICH conditions get wired with WHAT metadata, independent of security_alerts.py's own dedup/sink behavior (already covered by tests/test_security_alerts.py). + +Developer: Manish Kumar """ from datetime import datetime @@ -27,6 +29,8 @@ # --------------------------------------------------------------------------- def test_redis_health_reports_zero_pending_cleanly(stream_reader): + """Report Redis as available with zero pending, no oldest-pending age, and the consumer lag + sourced from xinfo_groups.""" reader, mock_redis = stream_reader mock_redis.xlen.return_value = 0 mock_redis.xpending.return_value = {"pending": 0} @@ -44,6 +48,8 @@ def test_redis_health_reports_zero_pending_cleanly(stream_reader): def test_redis_health_reports_pending_backlog_and_oldest_age(stream_reader): + """Report the pending count, oldest pending age, retry-in-progress count, and active/idle + consumer stats from a real backlog.""" reader, mock_redis = stream_reader mock_redis.xlen.return_value = 42 mock_redis.xpending.return_value = {"pending": 3} @@ -66,6 +72,8 @@ def test_redis_health_reports_pending_backlog_and_oldest_age(stream_reader): def test_redis_health_degrades_gracefully_on_connection_failure(stream_reader): + """Report Redis as unavailable with the error name, and never fabricate a pending count, when + the connection fails.""" reader, mock_redis = stream_reader mock_redis.xlen.side_effect = ConnectionError("redis unreachable") @@ -97,6 +105,8 @@ def test_redis_health_missing_xinfo_groups_support_is_not_a_failure(stream_reade # --------------------------------------------------------------------------- def test_persistence_health_with_empty_tables(db_session): + """Report persistence as available with no last-success time and zero quarantine when both + tables are empty.""" health = get_persistence_pipeline_health(db_session) assert health.available is True @@ -106,6 +116,7 @@ def test_persistence_health_with_empty_tables(db_session): def test_persistence_health_reports_last_success_and_quarantine_count(db_session): + """Report the last successful persistence time and the quarantine count from seeded rows.""" db_session.add(AuditEventRecord( event_id="evt-1", timestamp=datetime(2026, 1, 1, 12, 0, 0), # noqa: DTZ001 -- naive column, matches AuditEventRecord convention service="auth", event_type="login", action="login", @@ -129,6 +140,8 @@ def test_persistence_health_reports_last_success_and_quarantine_count(db_session def test_persistence_health_degrades_gracefully_on_db_failure(): + """Report persistence as unavailable with the error set, and never fabricate a quarantine count, + when the database query fails.""" from unittest.mock import MagicMock broken_db = MagicMock() @@ -146,6 +159,7 @@ def test_persistence_health_degrades_gracefully_on_db_failure(): # --------------------------------------------------------------------------- def test_get_pipeline_health_combines_both_sides(stream_reader, db_session, recording_alerts): + """Combine Redis and persistence health under one timezone-aware generated_at timestamp.""" reader, mock_redis = stream_reader mock_redis.xlen.return_value = 0 mock_redis.xpending.return_value = {"pending": 0} @@ -165,6 +179,8 @@ def test_get_pipeline_health_combines_both_sides(stream_reader, db_session, reco # --------------------------------------------------------------------------- class _RecordedCall: + """Capture the keyword arguments of one emit_security_alert call for assertion.""" + def __init__(self, kwargs): self.condition = kwargs.get("condition") self.severity = kwargs.get("severity") @@ -174,6 +190,7 @@ def __init__(self, kwargs): @pytest.fixture def recording_alerts(monkeypatch): + """Replace emit_security_alert with a recorder so tests can assert on which alerts fired.""" calls = [] def _fake_emit(**kwargs): @@ -184,10 +201,12 @@ def _fake_emit(**kwargs): def _consumers(idle_ms=100): + """Build a single-consumer xinfo_consumers-style response with the given idle time.""" return [{"name": "worker-1", "pending": 0, "idle": idle_ms}] def test_redis_unavailable_fires_critical_alert(stream_reader, db_session, recording_alerts): + """Fire a critical redis_stream_unavailable alert when the Redis connection fails.""" reader, mock_redis = stream_reader mock_redis.xlen.side_effect = ConnectionError("redis unreachable") @@ -201,6 +220,8 @@ def test_redis_unavailable_fires_critical_alert(stream_reader, db_session, recor def test_no_consumer_ever_registered_fires_critical_alert(stream_reader, db_session, recording_alerts): + """Fire a critical audit_worker_never_registered alert, but not a Redis-unavailable one, when no + consumer has ever registered.""" reader, mock_redis = stream_reader mock_redis.xlen.return_value = 0 mock_redis.xpending.return_value = {"pending": 0} @@ -215,6 +236,7 @@ def test_no_consumer_ever_registered_fires_critical_alert(stream_reader, db_sess def test_healthy_redis_with_a_registered_consumer_fires_no_availability_alerts(stream_reader, db_session, recording_alerts): + """Fire no availability alerts when Redis is healthy and a consumer is registered.""" reader, mock_redis = stream_reader mock_redis.xlen.return_value = 0 mock_redis.xpending.return_value = {"pending": 0} @@ -229,6 +251,8 @@ def test_healthy_redis_with_a_registered_consumer_fires_no_availability_alerts(s def test_pel_pending_threshold_unconfigured_fires_nothing(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire no PEL backlog alert when the pending-count threshold is not configured, however large + the backlog.""" monkeypatch.delenv("AUDIT_ALERT_PEL_PENDING_THRESHOLD", raising=False) reader, mock_redis = stream_reader mock_redis.xlen.return_value = 100 @@ -244,6 +268,8 @@ def test_pel_pending_threshold_unconfigured_fires_nothing(stream_reader, db_sess def test_pel_pending_threshold_configured_and_exceeded_fires_warning(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire a warning PEL backlog alert, carrying the pending count and threshold, once the + configured threshold is exceeded.""" monkeypatch.setenv("AUDIT_ALERT_PEL_PENDING_THRESHOLD", "10") reader, mock_redis = stream_reader mock_redis.xlen.return_value = 100 @@ -263,6 +289,7 @@ def test_pel_pending_threshold_configured_and_exceeded_fires_warning(stream_read def test_pel_pending_threshold_configured_but_not_exceeded_fires_nothing(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire no PEL backlog alert while the pending count stays under the configured threshold.""" monkeypatch.setenv("AUDIT_ALERT_PEL_PENDING_THRESHOLD", "1000") reader, mock_redis = stream_reader mock_redis.xlen.return_value = 100 @@ -279,6 +306,8 @@ def test_pel_pending_threshold_configured_but_not_exceeded_fires_nothing(stream_ def test_pel_age_threshold_configured_and_exceeded_fires_warning(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire a warning alert carrying the oldest pending age once the configured age threshold is + exceeded.""" monkeypatch.setenv("AUDIT_ALERT_PEL_AGE_THRESHOLD_SECONDS", "60") reader, mock_redis = stream_reader mock_redis.xlen.return_value = 10 @@ -297,6 +326,8 @@ def test_pel_age_threshold_configured_and_exceeded_fires_warning(stream_reader, def test_worker_stall_threshold_configured_and_exceeded_fires_warning(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire a warning alert carrying the least-idle consumer's idle time once the configured stall + threshold is exceeded.""" monkeypatch.setenv("AUDIT_ALERT_WORKER_STALL_THRESHOLD_MS", "5000") reader, mock_redis = stream_reader mock_redis.xlen.return_value = 0 @@ -312,6 +343,7 @@ def test_worker_stall_threshold_configured_and_exceeded_fires_warning(stream_rea def test_audit_database_unavailable_fires_critical_alert(stream_reader, recording_alerts): + """Fire a critical alert on the audit-persistence component when the database is unavailable.""" from unittest.mock import MagicMock reader, mock_redis = stream_reader @@ -331,6 +363,7 @@ def test_audit_database_unavailable_fires_critical_alert(stream_reader, recordin def test_quarantine_count_threshold_unconfigured_fires_nothing(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire no quarantine-count alert when its threshold is not configured.""" monkeypatch.delenv("AUDIT_ALERT_QUARANTINE_COUNT_THRESHOLD", raising=False) db_session.add(QuarantinedAuditEvent(stream_message_id="1-0", raw_data="{}", failure_category="malformed", delivery_attempts=5)) db_session.commit() @@ -346,6 +379,8 @@ def test_quarantine_count_threshold_unconfigured_fires_nothing(stream_reader, db def test_quarantine_count_threshold_configured_and_exceeded_fires_warning(stream_reader, db_session, recording_alerts, monkeypatch): + """Fire a warning alert carrying the quarantine count and threshold once the configured + threshold is exceeded.""" monkeypatch.setenv("AUDIT_ALERT_QUARANTINE_COUNT_THRESHOLD", "1") db_session.add(QuarantinedAuditEvent(stream_message_id="1-0", raw_data="{}", failure_category="malformed", delivery_attempts=5)) db_session.add(QuarantinedAuditEvent(stream_message_id="2-0", raw_data="{}", failure_category="malformed", delivery_attempts=5)) diff --git a/tests/test_audit_query_service.py b/tests/test_audit_query_service.py index ff884b5..322f6ca 100644 --- a/tests/test_audit_query_service.py +++ b/tests/test_audit_query_service.py @@ -1,6 +1,9 @@ """PR4.3: services/audit_query_service.py -- SQL-level filtering, ordering, and pagination over audit_events, exercised against a real (SQLite) DB via -the `db_session` fixture (no HTTP layer).""" +the `db_session` fixture (no HTTP layer). + +Developer: Manish Kumar +""" from datetime import datetime, timedelta from db.models import AuditEventRecord @@ -8,6 +11,7 @@ def _add(db_session, event_id, minutes_offset=0, **overrides): + """Insert one AuditEventRecord at a given time offset, applying any field overrides.""" row = AuditEventRecord( event_id=event_id, 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 @@ -33,6 +37,7 @@ def _add(db_session, event_id, minutes_offset=0, **overrides): # --------------------------------------------------------------------------- def test_filters_by_user_id(db_session): + """Return only events matching the requested user_id.""" _add(db_session, "e1", user_id="u1") _add(db_session, "e2", user_id="u2") db_session.commit() @@ -45,6 +50,8 @@ def test_filters_by_user_id(db_session): def test_filters_by_organization_id_without_authorizing_scope(db_session): + """Return only events matching the requested organization_id, with no scope authorization + applied at this layer.""" _add(db_session, "e1", organization_id="org-1") _add(db_session, "e2", organization_id="org-2") db_session.commit() @@ -56,6 +63,7 @@ def test_filters_by_organization_id_without_authorizing_scope(db_session): def test_filters_by_service(db_session): + """Return only events matching the requested service.""" _add(db_session, "e1", service="auth") _add(db_session, "e2", service="policy") db_session.commit() @@ -68,6 +76,7 @@ def test_filters_by_service(db_session): def test_filters_by_event_type(db_session): + """Return only events matching the requested event_type.""" _add(db_session, "e1", event_type="auth_login") _add(db_session, "e2", event_type="user_suspended") db_session.commit() @@ -80,6 +89,7 @@ def test_filters_by_event_type(db_session): def test_filters_by_decision(db_session): + """Return only events matching the requested decision.""" _add(db_session, "e1", decision="success") _add(db_session, "e2", decision="deny") db_session.commit() @@ -92,6 +102,7 @@ def test_filters_by_decision(db_session): def test_filters_by_timestamp_range(db_session): + """Return only events within the requested timestamp range.""" _add(db_session, "e1", minutes_offset=0) _add(db_session, "e2", minutes_offset=10) _add(db_session, "e3", minutes_offset=20) @@ -109,6 +120,7 @@ def test_filters_by_timestamp_range(db_session): def test_filters_by_integrity_status(db_session): + """Return only events matching the requested integrity_status.""" _add(db_session, "e1", integrity_status="valid") _add(db_session, "e2", integrity_status="invalid") _add(db_session, "e3") # unspecified -- DB server_default="unsigned" applies @@ -122,6 +134,8 @@ def test_filters_by_integrity_status(db_session): def test_filters_by_integrity_status_unsigned_matches_the_default(db_session): + """Match rows left at the database's unsigned default when filtering for + integrity_status=unsigned.""" _add(db_session, "e1", integrity_status="valid") _add(db_session, "e2") db_session.commit() @@ -149,6 +163,7 @@ def test_combined_filters_no_cross_leakage(db_session): def test_no_filters_returns_all(db_session): + """Return every event when no filter is applied.""" _add(db_session, "e1") _add(db_session, "e2") _add(db_session, "e3") @@ -159,6 +174,7 @@ def test_no_filters_returns_all(db_session): def test_empty_result(db_session): + """Return an empty list and zero total when no event matches the filter.""" _add(db_session, "e1", service="auth") db_session.commit() @@ -174,6 +190,7 @@ def test_empty_result(db_session): # --------------------------------------------------------------------------- def test_ordering_newest_first(db_session): + """Order results by newest event first.""" _add(db_session, "e1", minutes_offset=0) _add(db_session, "e2", minutes_offset=10) _add(db_session, "e3", minutes_offset=5) @@ -199,6 +216,7 @@ def test_ordering_tiebreak_by_event_id_desc(db_session): # --------------------------------------------------------------------------- def test_pagination_page_boundaries(db_session): + """Split results across pages, including a partial final page.""" for i in range(5): _add(db_session, f"e{i}", minutes_offset=i) db_session.commit() @@ -214,6 +232,8 @@ def test_pagination_page_boundaries(db_session): def test_pagination_out_of_range_page_returns_empty(db_session): + """Return no rows for a page beyond the result set while total still reflects every matching + row.""" _add(db_session, "e1") db_session.commit() @@ -223,6 +243,7 @@ def test_pagination_out_of_range_page_returns_empty(db_session): def test_pagination_total_unaffected_by_page_size(db_session): + """Report the full matching total regardless of the requested page size.""" for i in range(7): _add(db_session, f"e{i}", minutes_offset=i) db_session.commit() @@ -240,6 +261,8 @@ def test_pagination_total_unaffected_by_page_size(db_session): # --------------------------------------------------------------------------- def test_safe_platform_wide_with_explicit_organization_id_filters(db_session): + """Return only the requested organization's events for a platform-wide safe query with an + explicit organization_id.""" _add(db_session, "e1", organization_id="org-1") _add(db_session, "e2", organization_id="org-2") db_session.commit() @@ -252,6 +275,7 @@ def test_safe_platform_wide_with_explicit_organization_id_filters(db_session): def test_safe_column_filters(db_session): + """Filter safe query results by user_id, service, event_type, and decision together.""" _add(db_session, "e1", user_id="u1", service="auth", event_type="auth_login", decision="success") _add(db_session, "e2", user_id="u2", service="policy", event_type="policy_decision", decision="deny") db_session.commit() @@ -266,6 +290,7 @@ def test_safe_column_filters(db_session): def test_safe_timestamp_range_filters(db_session): + """Filter safe query results to the requested timestamp range.""" _add(db_session, "e1", minutes_offset=0) _add(db_session, "e2", minutes_offset=10) _add(db_session, "e3", minutes_offset=20) diff --git a/tests/test_backup_restore_integrity_integration.py b/tests/test_backup_restore_integrity_integration.py index 2fc5fe5..1536550 100644 --- a/tests/test_backup_restore_integrity_integration.py +++ b/tests/test_backup_restore_integrity_integration.py @@ -22,6 +22,8 @@ or the provisioning script -- those are covered in test_retention_immutability_integration.py. This file is scoped to the backup/restore round trip specifically. + +Developer: Manish Kumar """ import json import os @@ -54,6 +56,8 @@ def _real_mysql_available(): + """Report whether the configured test-MySQL root URL is reachable, returning False when + unconfigured or unreachable.""" if TEST_MYSQL_ROOT_URL is None: return False try: @@ -175,6 +179,8 @@ def restored_db(source_db_with_data, tmp_path_factory): def test_dump_includes_triggers_flag_captured_all_three_tables(restored_db): + """Capture audit_events, quarantined_audit_events, and audit_legal_holds in the backup dump and + produce a non-empty dump file.""" engine = create_engine(restored_db) with engine.connect() as conn: tables = {row[0] for row in conn.execute(text("SHOW TABLES"))} @@ -182,6 +188,8 @@ def test_dump_includes_triggers_flag_captured_all_three_tables(restored_db): def test_restored_audit_event_id_and_hash_survive_byte_for_byte(source_db_with_data, restored_db): + """Preserve an audit event's event_id and record_integrity_hash byte-for-byte through a real + backup/restore round trip.""" source_engine = create_engine(source_db_with_data) restored_engine = create_engine(restored_db) @@ -200,6 +208,8 @@ def test_restored_audit_event_id_and_hash_survive_byte_for_byte(source_db_with_d def test_restored_quarantine_record_survives_byte_for_byte(source_db_with_data, restored_db): + """Preserve a quarantine record's raw_data and record_integrity_hash byte-for-byte through a + real backup/restore round trip.""" source_engine = create_engine(source_db_with_data) restored_engine = create_engine(restored_db) @@ -217,6 +227,7 @@ def test_restored_quarantine_record_survives_byte_for_byte(source_db_with_data, def test_restored_legal_hold_survives(restored_db): + """Preserve a legal hold row through a real backup/restore round trip.""" engine = create_engine(restored_db) with engine.connect() as conn: count = conn.execute(text( diff --git a/tests/test_classify_event_integrity.py b/tests/test_classify_event_integrity.py index 2706e7a..56120cb 100644 --- a/tests/test_classify_event_integrity.py +++ b/tests/test_classify_event_integrity.py @@ -5,6 +5,8 @@ Synthetic secrets only, matching tests/test_signing.py's own convention -- never a real deployment JWT_SECRET. + +Developer: Manish Kumar """ from audit.signing import sign_audit_event from consumers.processor import classify_event_integrity @@ -20,30 +22,36 @@ # --------------------------------------------------------------------------- def test_valid_signature_classifies_as_valid(): + """Classify a correctly signed event as valid.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert classify_event_integrity(SERVICE, sig, DATA, SECRET) == "valid" def test_wrong_secret_classifies_as_invalid(): + """Classify a signature verified against the wrong secret as invalid.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert classify_event_integrity(SERVICE, sig, DATA, OTHER_SECRET) == "invalid" def test_tampered_data_classifies_as_invalid(): + """Classify a signature whose data was altered after signing as invalid.""" sig = sign_audit_event(SERVICE, DATA, SECRET) tampered = DATA.replace("submit", "delete_all") assert classify_event_integrity(SERVICE, sig, tampered, SECRET) == "invalid" def test_malformed_signature_classifies_as_invalid(): + """Classify a signature string that is not a real signature as invalid.""" assert classify_event_integrity(SERVICE, "not-a-real-signature", DATA, SECRET) == "invalid" def test_missing_signature_classifies_as_unsigned(): + """Classify a None signature as unsigned, not invalid.""" assert classify_event_integrity(SERVICE, None, DATA, SECRET) == "unsigned" def test_empty_signature_classifies_as_unsigned(): + """Classify an empty-string signature as unsigned, not invalid.""" assert classify_event_integrity(SERVICE, "", DATA, SECRET) == "unsigned" @@ -59,6 +67,7 @@ def test_empty_signature_classifies_as_unsigned(): # --------------------------------------------------------------------------- def test_missing_signature_is_unsigned_not_invalid_even_with_wrong_secret(): + """Classify a missing signature as unsigned regardless of which secret is checked against.""" assert classify_event_integrity(SERVICE, None, DATA, OTHER_SECRET) == "unsigned" @@ -67,11 +76,13 @@ def test_missing_signature_is_unsigned_not_invalid_even_with_wrong_secret(): # --------------------------------------------------------------------------- def test_signature_for_different_service_classifies_as_invalid(): + """Classify a signature made for a different service as invalid.""" sig = sign_audit_event("workflow-bundles", DATA, SECRET) assert classify_event_integrity("auth-service", sig, DATA, SECRET) == "invalid" def test_signature_for_different_data_classifies_as_invalid(): + """Classify a signature made for different data as invalid.""" other_data = '{"event_id":"e2","service":"tes","action":"delete"}' sig = sign_audit_event(SERVICE, other_data, SECRET) assert classify_event_integrity(SERVICE, sig, DATA, SECRET) == "invalid" @@ -86,6 +97,8 @@ def test_signature_for_different_data_classifies_as_invalid(): # --------------------------------------------------------------------------- def test_reordered_json_keys_invalidate_the_original_signature(): + """Classify a signature as invalid when the same data is reserialized with its JSON keys + reordered.""" original = '{"a": 1, "b": 2}' reordered = '{"b": 2, "a": 1}' sig = sign_audit_event(SERVICE, original, SECRET) @@ -105,6 +118,8 @@ def test_exact_original_string_still_classifies_as_valid(): # --------------------------------------------------------------------------- def test_return_value_is_always_one_of_the_three_literal_strings(): + """Return exactly one of valid, invalid, or unsigned for every combination of signature and + secret.""" sig = sign_audit_event(SERVICE, DATA, SECRET) for signature, secret, expected in [ (sig, SECRET, "valid"), diff --git a/tests/test_context.py b/tests/test_context.py index b611be0..5485a99 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -1,3 +1,10 @@ +"""Validate the contextvars-backed trace/user/identity context: default values, setting and +overwriting trace_id and user_id, inject_context's UUID trace id generation, and verified- +identity propagation from a valid token. + +Developer: Manish Kumar +""" + import jwt import pytest from audit.context import ( @@ -13,11 +20,13 @@ def _token(**claims): + """Sign an HS256 test token with the shared test secret, applying any extra claims.""" return jwt.encode(claims, SECRET, algorithm="HS256") @pytest.fixture(autouse=True) def _patch_secret(monkeypatch): + """Point the JWT verifier at the shared test secret for every test in this module.""" # SSO Phase 2 PR3: decoding now happens in audit.jwt_verify, not # audit.identity -- identity_module no longer has its own JWT_SECRET. monkeypatch.setattr(jwt_verify_module, "JWT_SECRET", SECRET) @@ -28,6 +37,7 @@ def _patch_secret(monkeypatch): # --------------------------------------------------------------------------- def test_trace_id_default_is_none(): + """Default trace_id to None before it is set.""" # Reset to default token = trace_id_var.set(None) try: @@ -37,6 +47,7 @@ def test_trace_id_default_is_none(): def test_set_and_get_trace_id(): + """Store and retrieve a trace_id set on the context.""" token = trace_id_var.set(None) try: set_trace_id("trace-abc-123") @@ -46,6 +57,7 @@ def test_set_and_get_trace_id(): def test_trace_id_can_be_overwritten(): + """Overwrite an existing trace_id with a newly set value.""" token = trace_id_var.set(None) try: set_trace_id("first") @@ -60,6 +72,7 @@ def test_trace_id_can_be_overwritten(): # --------------------------------------------------------------------------- def test_user_id_default_is_none(): + """Default user_id to None before it is set.""" token = user_id_var.set(None) try: assert get_user_id() is None @@ -68,6 +81,7 @@ def test_user_id_default_is_none(): def test_set_and_get_user_id(): + """Store and retrieve a user_id set on the context.""" token = user_id_var.set(None) try: set_user_id("user-xyz") @@ -77,6 +91,7 @@ def test_set_and_get_user_id(): def test_user_id_can_be_overwritten(): + """Overwrite an existing user_id with a newly set value.""" token = user_id_var.set(None) try: set_user_id("user1") @@ -91,6 +106,7 @@ def test_user_id_can_be_overwritten(): # --------------------------------------------------------------------------- def test_inject_context_sets_trace_id(): + """Set the context's trace_id to the value inject_context generates and returns.""" token_t = trace_id_var.set(None) token_u = user_id_var.set(None) try: @@ -103,6 +119,7 @@ def test_inject_context_sets_trace_id(): def test_inject_context_sets_user_id(): + """Set the context's user_id to the value passed into inject_context.""" token_t = trace_id_var.set(None) token_u = user_id_var.set(None) try: @@ -114,18 +131,21 @@ def test_inject_context_sets_user_id(): def test_inject_context_returns_uuid_string(): + """Return a 36-character UUID4-format string from inject_context.""" trace_id = inject_context() assert isinstance(trace_id, str) assert len(trace_id) == 36 def test_inject_context_generates_unique_trace_ids(): + """Generate a distinct trace id on each inject_context call.""" t1 = inject_context() t2 = inject_context() assert t1 != t2 def test_inject_context_without_user_id(): + """Leave user_id unset when inject_context is called without one.""" token_u = user_id_var.set(None) try: inject_context() @@ -139,6 +159,7 @@ def test_inject_context_without_user_id(): # --------------------------------------------------------------------------- def test_inject_context_with_valid_token_sets_verified_identity(): + """Populate the identity context with the verified subject and email from a valid access token.""" token_u = user_id_var.set(None) token_i = identity_var.set(None) try: diff --git a/tests/test_db_models.py b/tests/test_db_models.py index 13055b3..c8e00fa 100644 --- a/tests/test_db_models.py +++ b/tests/test_db_models.py @@ -1,4 +1,7 @@ -"""PR4.2 regression tests: the audit_events table (db/models.py).""" +"""PR4.2 regression tests: the audit_events table (db/models.py). + +Developer: Manish Kumar +""" from datetime import datetime import pytest @@ -8,6 +11,7 @@ def test_audit_event_record_inserts_successfully(db_session): + """Persist and reload an AuditEventRecord with its service and decision intact.""" record = AuditEventRecord( event_id="evt-1", timestamp=datetime(2026, 1, 1, 12, 0, 0), @@ -28,6 +32,7 @@ def test_audit_event_record_inserts_successfully(db_session): def test_audit_event_record_preserves_json_context(db_session): + """Round-trip a nested JSON context object through the context column unchanged.""" context = {"ip": "1.2.3.4", "nested": {"a": [1, 2, 3]}} record = AuditEventRecord( event_id="evt-2", @@ -44,6 +49,7 @@ def test_audit_event_record_preserves_json_context(db_session): def test_audit_event_record_created_at_defaults(db_session): + """Default created_at to a non-null value when it is not explicitly set.""" record = AuditEventRecord( event_id="evt-3", timestamp=datetime(2026, 1, 1, 12, 0, 0), diff --git a/tests/test_db_session.py b/tests/test_db_session.py index 563da13..c73d7f4 100644 --- a/tests/test_db_session.py +++ b/tests/test_db_session.py @@ -5,7 +5,10 @@ No live database connection is required here: SQLAlchemy's engine/Session construction is lazy and never opens a connection until a query executes, same assumption db/session.py's own module-level `engine = create_engine(...)` -already relies on to be importable at all in this test environment.""" +already relies on to be importable at all in this test environment. + +Developer: Manish Kumar +""" import pytest from sqlalchemy.orm import Session @@ -13,6 +16,7 @@ def test_get_db_yields_a_session_and_closes_it_on_generator_exit(): + """Yield a live Session from get_db and close it once the generator is exhausted.""" gen = get_db() db = next(gen) assert isinstance(db, Session) diff --git a/tests/test_decorators.py b/tests/test_decorators.py index e5d3cf7..060c23d 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -1,3 +1,10 @@ +"""Validate the @audit decorator: it calls the wrapped async function, logs one event afterward +carrying the right type/action/trace_id/user_id/decision, preserves the function's name and +arguments, and enriches the event with a verified identity's organization context. + +Developer: Manish Kumar +""" + from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -7,6 +14,7 @@ @pytest.fixture(autouse=True) def reset_context_vars(): + """Seed the trace and user context vars for each test and reset every context var afterward.""" t1 = trace_id_var.set("test-trace") t2 = user_id_var.set("test-user") # PR4.4, additive: keeps identity_var at its None default for every @@ -25,6 +33,7 @@ def reset_context_vars(): @pytest.mark.asyncio async def test_audit_decorator_calls_wrapped_function(): + """Call the wrapped function and return its result.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -42,6 +51,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_logs_after_function(): + """Log exactly one event after the wrapped function returns.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -59,6 +69,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_log_event_has_correct_type_and_action(): + """Log an event with the decorator's configured event_type and action.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -78,6 +89,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_attaches_trace_id(): + """Attach the current context's trace_id to the logged event.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -96,6 +108,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_attaches_user_id(): + """Attach the current context's user_id to the logged event.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -114,6 +127,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_sets_decision_success(): + """Set the logged event's decision to success.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -132,6 +146,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_preserves_function_name(): + """Preserve the wrapped function's __name__.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() @@ -147,6 +162,7 @@ async def named_function(): @pytest.mark.asyncio async def test_audit_decorator_passes_args_to_wrapped(): + """Pass the caller's positional arguments through to the wrapped function and return its result.""" mock_logger = MagicMock() mock_logger.log = AsyncMock() captured = [] @@ -192,6 +208,7 @@ async def my_func(): @pytest.mark.asyncio async def test_audit_decorator_enriches_context_with_verified_identity(): + """Attach the verified identity's organization id and tenant scope to the logged event.""" from audit.identity import VerifiedIdentity identity = VerifiedIdentity( diff --git a/tests/test_deps.py b/tests/test_deps.py index b928d1a..bf195c9 100644 --- a/tests/test_deps.py +++ b/tests/test_deps.py @@ -1,7 +1,10 @@ """PR4.3: api/deps.py::require_platform_admin -- the platform-admin gate for GET /audit/events. Mirrors the coverage of omnibioai-control-center/backend/tests/test_core_auth.py::TestRequireAdmin, -adapted to this repo's plain-pytest style and the platform_admin role.""" +adapted to this repo's plain-pytest style and the platform_admin role. + +Developer: Manish Kumar +""" import datetime as dt import jwt @@ -15,35 +18,41 @@ def _token(**claims): + """Sign an HS256 test token with the shared test secret, applying any extra claims.""" return jwt.encode(claims, SECRET, algorithm="HS256") @pytest.fixture(autouse=True) def _patch_secret(monkeypatch): + """Point the JWT verifier at the shared test secret for every test in this module.""" # SSO Phase 2 PR3: decoding now happens in audit.jwt_verify, not here # -- deps_module no longer has its own JWT_SECRET to patch. monkeypatch.setattr(jwt_verify_module, "JWT_SECRET", SECRET) def test_missing_header_raises_401(): + """Reject a missing Authorization header with 401.""" with pytest.raises(HTTPException) as exc: deps_module.require_platform_admin(None) assert exc.value.status_code == 401 def test_non_bearer_header_raises_401(): + """Reject a non-Bearer Authorization scheme with 401.""" with pytest.raises(HTTPException) as exc: deps_module.require_platform_admin("Basic abc123") assert exc.value.status_code == 401 def test_invalid_token_raises_401(): + """Reject an undecodable token with 401.""" with pytest.raises(HTTPException) as exc: deps_module.require_platform_admin("Bearer not-a-real-token") assert exc.value.status_code == 401 def test_expired_token_raises_401(): + """Reject an expired token with 401.""" token = jwt.encode( { "sub": "1", @@ -68,6 +77,7 @@ def test_valid_token_without_platform_admin_role_raises_403(): def test_valid_token_missing_roles_claim_raises_403(): + """Reject a valid token that carries no roles claim with 403.""" token = _token(sub="1") with pytest.raises(HTTPException) as exc: deps_module.require_platform_admin(f"Bearer {token}") @@ -75,6 +85,7 @@ def test_valid_token_missing_roles_claim_raises_403(): def test_org_admin_with_multiple_roles_still_denied(): + """Deny a token whose roles include org_admin but not platform_admin.""" token = _token(sub="1", roles=["org_admin", "team_lead"]) with pytest.raises(HTTPException) as exc: deps_module.require_platform_admin(f"Bearer {token}") @@ -82,6 +93,7 @@ def test_org_admin_with_multiple_roles_still_denied(): def test_valid_platform_admin_token_returns_payload(): + """Return the decoded payload for a token carrying the platform_admin role.""" token = _token(sub="1", roles=["platform_admin"]) payload = deps_module.require_platform_admin(f"Bearer {token}") assert payload["sub"] == "1" @@ -89,6 +101,7 @@ def test_valid_platform_admin_token_returns_payload(): def test_bearer_prefix_case_insensitive(): + """Accept a lowercase bearer scheme prefix.""" token = _token(sub="1", roles=["platform_admin"]) payload = deps_module.require_platform_admin(f"bearer {token}") assert "platform_admin" in payload["roles"] diff --git a/tests/test_deps_audit.py b/tests/test_deps_audit.py index 1b30721..2cd7a8b 100644 --- a/tests/test_deps_audit.py +++ b/tests/test_deps_audit.py @@ -4,7 +4,10 @@ or wrong-scope) or no Authorization header at all -- never a header that parses as Bearer but fails token verification itself, so that branch was never reached. Mirrors tests/test_deps.py::test_invalid_token_raises_401, -which covers the structurally identical branch in api/deps.py.""" +which covers the structurally identical branch in api/deps.py. + +Developer: Manish Kumar +""" import pytest from fastapi import HTTPException @@ -12,6 +15,7 @@ def test_malformed_bearer_token_raises_401(): + """Reject a Bearer header that fails token verification with a 401, not just a missing header.""" with pytest.raises(HTTPException) as exc: require_audit_read_access("Bearer not-a-real-token") assert exc.value.status_code == 401 diff --git a/tests/test_events.py b/tests/test_events.py index 24758f9..6dad245 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -1,3 +1,9 @@ +"""Validate the AuditEvent model's field defaults, auto-generated event_id and timestamp, full +construction, and serialization, plus the AuditEvents constant groups. + +Developer: Manish Kumar +""" + import pytest from datetime import datetime from audit.models import AuditEvent @@ -9,12 +15,14 @@ # --------------------------------------------------------------------------- def test_audit_event_required_fields(): + """Construct an AuditEvent from its required service and event_type fields.""" event = AuditEvent(service="auth", event_type="auth_login") assert event.service == "auth" assert event.event_type == "auth_login" def test_audit_event_has_uuid_event_id(): + """Auto-generate a 36-character UUID4-format event_id.""" # Phase 3 PR4.1: event_id is generated per-instance via Field(default_factory=...) # -- see test_models.py for the regression test proving two instances differ. event = AuditEvent(service="auth", event_type="test") @@ -23,11 +31,13 @@ def test_audit_event_has_uuid_event_id(): def test_audit_event_auto_generates_timestamp(): + """Auto-generate a datetime timestamp when none is supplied.""" event = AuditEvent(service="auth", event_type="test") assert isinstance(event.timestamp, datetime) def test_audit_event_optional_fields_default_none(): + """Default user_id, resource, decision, reason, and trace_id to None.""" event = AuditEvent(service="svc", event_type="type") assert event.user_id is None assert event.resource is None @@ -37,16 +47,19 @@ def test_audit_event_optional_fields_default_none(): def test_audit_event_action_defaults_empty_string(): + """Default action to an empty string.""" event = AuditEvent(service="svc", event_type="type") assert event.action == "" def test_audit_event_context_defaults_empty_dict(): + """Default context to an empty dict.""" event = AuditEvent(service="svc", event_type="type") assert event.context == {} def test_audit_event_full_construction(): + """Construct an AuditEvent with every field supplied and preserve each value.""" event = AuditEvent( service="policy-engine", event_type="policy_decision", @@ -64,6 +77,7 @@ def test_audit_event_full_construction(): def test_audit_event_serialization(): + """Serialize an AuditEvent to a dict carrying its service, user_id, and decision.""" event = AuditEvent( service="svc", event_type="test", @@ -82,19 +96,23 @@ def test_audit_event_serialization(): # --------------------------------------------------------------------------- def test_audit_events_auth_constants(): + """Pin the AUTH_LOGIN and AUTH_FAILED event-type constants.""" assert AuditEvents.AUTH_LOGIN == "auth_login" assert AuditEvents.AUTH_FAILED == "auth_failed" def test_audit_events_iam_constants(): + """Pin the IAM_CACHE_HIT and IAM_CACHE_MISS event-type constants.""" assert AuditEvents.IAM_CACHE_HIT == "iam_cache_hit" assert AuditEvents.IAM_CACHE_MISS == "iam_cache_miss" def test_audit_events_policy_constants(): + """Pin the POLICY_DECISION event-type constant.""" assert AuditEvents.POLICY_DECISION == "policy_decision" def test_audit_events_tes_constants(): + """Pin the TES_SUBMIT and TES_COMPLETE event-type constants.""" assert AuditEvents.TES_SUBMIT == "tes_submit" assert AuditEvents.TES_COMPLETE == "tes_complete" diff --git a/tests/test_identity.py b/tests/test_identity.py index ae6a6bc..2c3d8ee 100644 --- a/tests/test_identity.py +++ b/tests/test_identity.py @@ -1,5 +1,8 @@ """PR4.4: audit/identity.py -- validates access tokens for audit event -producers and derives verified identity claims from them.""" +producers and derives verified identity claims from them. + +Developer: Manish Kumar +""" import datetime as dt import jwt @@ -12,34 +15,41 @@ def _token(**claims): + """Sign an HS256 test token with the shared test secret, applying any extra claims.""" return jwt.encode(claims, SECRET, algorithm="HS256") @pytest.fixture(autouse=True) def _patch_secret(monkeypatch): + """Point the JWT verifier at the shared test secret for every test in this module.""" # SSO Phase 2 PR3: decoding now happens in audit.jwt_verify, not here # -- identity_module no longer has its own JWT_SECRET to patch. monkeypatch.setattr(jwt_verify_module, "JWT_SECRET", SECRET) def test_none_token_returns_none(): + """Return None for a None token.""" assert validate_identity_token(None) is None def test_empty_token_returns_none(): + """Return None for an empty token string.""" assert validate_identity_token("") is None def test_malformed_token_returns_none(): + """Return None for a token that is not a valid JWT.""" assert validate_identity_token("not-a-real-token") is None def test_wrong_signature_returns_none(): + """Return None for a token signed with a different secret.""" token = jwt.encode({"sub": "1"}, "a-different-secret", algorithm="HS256") assert validate_identity_token(token) is None def test_expired_token_returns_none(): + """Return None for an expired token.""" token = jwt.encode( { "sub": "1", @@ -52,11 +62,14 @@ def test_expired_token_returns_none(): def test_missing_sub_claim_returns_none(): + """Return None for a valid token that carries no sub claim.""" token = _token(email="nosub@omnibioai.test") assert validate_identity_token(token) is None def test_valid_token_returns_verified_identity(): + """Return a VerifiedIdentity carrying the subject, email, roles, and organization from a valid + token.""" token = _token( sub="42", email="alice@omnibioai.test", @@ -90,6 +103,7 @@ def test_valid_token_minimal_claims_defaults_gracefully(): def test_verified_identity_instances_do_not_share_mutable_defaults(): + """Keep each VerifiedIdentity's default roles list independent, not shared mutable state.""" a = VerifiedIdentity(sub="a") b = VerifiedIdentity(sub="b") a.roles.append("should-not-leak") @@ -107,6 +121,7 @@ def test_non_string_sub_claim_returns_none(): def test_as_context_shape(): + """Serialize a VerifiedIdentity to the dict shape as_context produces.""" identity = VerifiedIdentity( sub="1", email="a@b.com", roles=["r1"], org_id=1, org_role=["r2"] ) diff --git a/tests/test_jwt_verify.py b/tests/test_jwt_verify.py index 1684b23..436b3d8 100644 --- a/tests/test_jwt_verify.py +++ b/tests/test_jwt_verify.py @@ -4,6 +4,8 @@ SSO Phase 2 PR16: adds coverage for the RS256/JWKS verification path added alongside the existing HS256 path. + +Developer: Manish Kumar """ import datetime as dt from unittest.mock import MagicMock @@ -29,14 +31,17 @@ def _token(**claims): + """Sign an HS256 test token with the shared test secret, applying any extra claims.""" return jwt.encode(claims, SECRET, algorithm="HS256") def _rs256_token(private_key, kid, **claims): + """Sign an RS256 test token with the given private key and key id, applying any extra claims.""" return jwt.encode(claims, private_key, algorithm="RS256", headers={"kid": kid}) def _jwk(public_key, kid: str) -> dict: + """Build a JWKS entry from the given RSA public key and key id.""" jwk = RSAAlgorithm.to_jwk(public_key, as_dict=True) jwk.update({"kid": kid, "use": "sig", "alg": "RS256"}) return jwk @@ -44,6 +49,7 @@ def _jwk(public_key, kid: str) -> dict: @pytest.fixture(autouse=True) def _patch_secret(monkeypatch): + """Point the JWT verifier at the shared test secret for every test in this module.""" monkeypatch.setattr(jwt_verify_module, "JWT_SECRET", SECRET) @@ -83,6 +89,7 @@ def install_jwks(*jwks_responses: dict) -> MagicMock: # --------------------------------------------------------------------------- def test_valid_token_succeeds(): + """Return the decoded payload for a valid HS256 access token.""" token = _token(sub="1", roles=["platform_admin"], type="access") payload = verify_token(token) assert payload["sub"] == "1" @@ -98,6 +105,7 @@ def test_valid_token_without_type_claim_succeeds(): def test_platform_issuer_and_audience_contract(): + """Accept the platform issuer and audience and reject a token with the wrong audience or issuer.""" valid = _token(sub="1", iss="omnibioai-auth", aud="omnibioai-platform") assert verify_token(valid)["aud"] == "omnibioai-platform" @@ -115,6 +123,7 @@ def test_platform_issuer_and_audience_contract(): # --------------------------------------------------------------------------- def test_missing_token_raises(): + """Raise TokenInvalid for a None or empty token.""" with pytest.raises(TokenInvalid): verify_token(None) with pytest.raises(TokenInvalid): @@ -122,17 +131,20 @@ def test_missing_token_raises(): def test_invalid_signature_raises(): + """Raise TokenInvalid for a token signed with the wrong secret.""" token = jwt.encode({"sub": "1"}, "wrong-secret", algorithm="HS256") with pytest.raises(TokenInvalid): verify_token(token) def test_malformed_token_raises(): + """Raise TokenInvalid for a string that is not a valid JWT.""" with pytest.raises(TokenInvalid): verify_token("not-a-real-token") def test_expired_token_raises(): + """Raise TokenInvalid for an expired token.""" token = jwt.encode( {"sub": "1", "exp": dt.datetime.now(dt.timezone.utc) - dt.timedelta(minutes=1)}, SECRET, @@ -143,6 +155,7 @@ def test_expired_token_raises(): def test_missing_sub_claim_raises(): + """Raise TokenInvalid for a token that carries no sub claim.""" token = _token(email="x@y.com") with pytest.raises(TokenInvalid): verify_token(token) @@ -160,6 +173,7 @@ def test_wrong_token_type_raises(): def test_other_token_types_also_rejected(): + """Raise TokenInvalid for token types other than access, such as oauth_state.""" for bad_type in ("oauth_state", "sso_state", "oauth_link"): token = _token(sub="1", type=bad_type) with pytest.raises(TokenInvalid): @@ -171,6 +185,7 @@ def test_other_token_types_also_rejected(): # --------------------------------------------------------------------------- def test_blacklisted_jti_raises(mock_blacklist): + """Raise TokenInvalid and check the blacklist for a token whose jti has been revoked.""" mock_blacklist.exists.return_value = True token = _token(sub="1", jti="revoked-jti-123") with pytest.raises(TokenInvalid): @@ -179,6 +194,7 @@ def test_blacklisted_jti_raises(mock_blacklist): def test_non_blacklisted_jti_succeeds(mock_blacklist): + """Accept a token whose jti is checked against the blacklist and found not revoked.""" mock_blacklist.exists.return_value = False token = _token(sub="1", jti="fine-jti-456") payload = verify_token(token) @@ -186,6 +202,7 @@ def test_non_blacklisted_jti_succeeds(mock_blacklist): def test_token_without_jti_skips_blacklist_check(mock_blacklist): + """Skip the blacklist check entirely for a token that carries no jti claim.""" token = _token(sub="1") # no jti claim at all payload = verify_token(token) assert payload["sub"] == "1" @@ -210,6 +227,7 @@ def test_blacklist_redis_error_fails_open(monkeypatch): # --------------------------------------------------------------------------- def test_valid_rs256_token_succeeds(): + """Return the decoded payload for a valid RS256 token verified against a fetched JWKS key.""" install_jwks({"keys": [_jwk(_PUBLIC_KEY, KID)]}) token = _rs256_token(_PRIVATE_KEY, KID, sub="1", roles=["platform_admin"], type="access") payload = verify_token(token) @@ -267,12 +285,14 @@ def test_jwks_fetch_failure_fails_closed(): def test_rs256_token_missing_kid_raises(): + """Raise TokenInvalid for an RS256 token whose header carries no kid.""" token = jwt.encode({"sub": "1"}, _PRIVATE_KEY, algorithm="RS256") with pytest.raises(TokenInvalid): verify_token(token) def test_expired_rs256_token_raises(): + """Raise TokenInvalid for an expired RS256 token.""" install_jwks({"keys": [_jwk(_PUBLIC_KEY, KID)]}) token = _rs256_token( _PRIVATE_KEY, diff --git a/tests/test_logger.py b/tests/test_logger.py index e3c3e25..58a1e01 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -7,6 +7,8 @@ ever sees them. Tests that assert xadd is called supply a mock event whose .model_dump() returns fully JSON-serializable data; tests that need to exercise the real serializer construct a real AuditEvent. + +Developer: Manish Kumar """ import json from datetime import datetime @@ -44,6 +46,7 @@ def _serializable_event(service="auth", event_type="login", **extra): @pytest.mark.asyncio async def test_log_writes_to_redis_stream(audit_logger): + """Write the event to the configured Redis stream via XADD, carrying its service and decision.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() @@ -61,6 +64,7 @@ async def test_log_writes_to_redis_stream(audit_logger): @pytest.mark.asyncio async def test_log_passes_maxlen_and_approximate(audit_logger): + """Cap the stream at the configured max length using approximate trimming.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() @@ -75,6 +79,7 @@ async def test_log_passes_maxlen_and_approximate(audit_logger): @pytest.mark.asyncio async def test_log_uses_config_stream_name(audit_logger): + """Write to the stream name configured in AuditConfig.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() @@ -87,6 +92,7 @@ async def test_log_uses_config_stream_name(audit_logger): @pytest.mark.asyncio async def test_log_event_includes_all_fields(audit_logger): + """Include user_id, trace_id, and context in the stored stream entry.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() @@ -176,6 +182,7 @@ async def test_log_real_event_id_survives_serialization(audit_logger): @pytest.mark.asyncio async def test_log_serializes_first_class_tenant_in_signed_payload(audit_logger): + """Store organization_id and tenant_scope as first-class fields in the signed payload.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() await logger.log(AuditEvent( @@ -189,6 +196,7 @@ async def test_log_serializes_first_class_tenant_in_signed_payload(audit_logger) @pytest.mark.asyncio async def test_tenant_field_is_covered_by_signature(audit_logger): + """Cover the tenant fields by the event signature so tampering with them invalidates it.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() await logger.log(AuditEvent( diff --git a/tests/test_logger_signing.py b/tests/test_logger_signing.py index 75d58f6..4aee030 100644 --- a/tests/test_logger_signing.py +++ b/tests/test_logger_signing.py @@ -8,6 +8,8 @@ copy to keep in sync. These tests exercise AuditLogger.log()'s new signing behavior specifically; tests/test_signing.py already covers sign_audit_event/verify_audit_event in isolation and is untouched. + +Developer: Manish Kumar """ import json from unittest.mock import AsyncMock @@ -21,6 +23,7 @@ @pytest.mark.asyncio async def test_log_signs_the_exact_data_string_it_publishes(audit_logger, monkeypatch): + """Sign exactly the data string that gets published, verifiable against the same secret.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() monkeypatch.setattr(AuditConfig, "EVENT_SIGNING_SECRET", "s3cr3t") @@ -37,6 +40,7 @@ async def test_log_signs_the_exact_data_string_it_publishes(audit_logger, monkey @pytest.mark.asyncio async def test_log_includes_both_data_and_sig_fields(audit_logger, monkeypatch): + """Publish both a data field and a v1:-prefixed sig field.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() monkeypatch.setattr(AuditConfig, "EVENT_SIGNING_SECRET", "s3cr3t") @@ -50,6 +54,7 @@ async def test_log_includes_both_data_and_sig_fields(audit_logger, monkeypatch): @pytest.mark.asyncio async def test_log_signature_does_not_verify_under_a_different_secret(audit_logger, monkeypatch): + """Fail verification when checked against a secret other than the one that signed it.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock() monkeypatch.setattr(AuditConfig, "EVENT_SIGNING_SECRET", "s3cr3t") @@ -104,6 +109,7 @@ async def test_log_without_a_service_still_publishes_unsigned(audit_logger, monk @pytest.mark.asyncio async def test_log_exception_never_leaks_the_secret(audit_logger, monkeypatch, capsys): + """Never print the signing secret to stdout or stderr when logging raises.""" logger, mock_redis = audit_logger mock_redis.xadd = AsyncMock(side_effect=RuntimeError("boom")) monkeypatch.setattr(AuditConfig, "EVENT_SIGNING_SECRET", "super-secret-value") diff --git a/tests/test_migrations.py b/tests/test_migrations.py index ba62d10..3cb661a 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -1,6 +1,8 @@ """PR4.2: Alembic migration mechanics for the audit_events table, exercised against an isolated throwaway SQLite database -- never against a real MySQL instance. Mirrors omnibioai-auth/tests/test_migrations.py's pattern. + +Developer: Manish Kumar """ from pathlib import Path @@ -20,6 +22,8 @@ def _alembic_config(db_url: str) -> Config: + """Build an Alembic Config that points at the repository's migrations and the given SQLite test + database URL.""" cfg = Config(str(REPO_ROOT / "alembic.ini")) cfg.set_main_option("script_location", str(REPO_ROOT / "alembic")) # env.py only falls back to AuditConfig.DATABASE_URL when this is unset @@ -30,6 +34,7 @@ def _alembic_config(db_url: str) -> Config: def test_upgrade_head_creates_audit_events_table(tmp_path): + """Create the audit_events table with its expected columns on a fresh upgrade to head.""" db_file = tmp_path / "migration_test.db" db_url = f"sqlite:///{db_file}" @@ -45,6 +50,7 @@ def test_upgrade_head_creates_audit_events_table(tmp_path): def test_audit_events_event_id_is_primary_key(tmp_path): + """Set event_id as the primary key of audit_events.""" db_file = tmp_path / "migration_test.db" db_url = f"sqlite:///{db_file}" @@ -58,6 +64,7 @@ def test_audit_events_event_id_is_primary_key(tmp_path): def test_downgrade_drops_audit_events_table(tmp_path): + """Drop the audit_events table when downgraded to base.""" db_file = tmp_path / "migration_test.db" db_url = f"sqlite:///{db_file}" @@ -75,6 +82,7 @@ def test_downgrade_drops_audit_events_table(tmp_path): # --------------------------------------------------------------------------- def test_integrity_status_column_exists_after_upgrade(tmp_path): + """Add a non-nullable integrity_status column at head.""" db_file = tmp_path / "migration_test.db" db_url = f"sqlite:///{db_file}" @@ -89,6 +97,8 @@ def test_integrity_status_column_exists_after_upgrade(tmp_path): def test_tenant_columns_and_query_index_exist_after_upgrade(tmp_path): + """Add a nullable organization_id, a non-nullable tenant_scope, and the org/timestamp/event + query index at head.""" db_file = tmp_path / "migration_test.db" cfg = _alembic_config(f"sqlite:///{db_file}") command.upgrade(cfg, "head") @@ -102,6 +112,7 @@ def test_tenant_columns_and_query_index_exist_after_upgrade(tmp_path): def test_legacy_rows_are_unknown_after_tenant_migration(tmp_path): + """Backfill pre-existing rows with a null organization_id and an unknown tenant_scope.""" db_file = tmp_path / "migration_test.db" cfg = _alembic_config(f"sqlite:///{db_file}") command.upgrade(cfg, "0002_integrity_status") diff --git a/tests/test_models.py b/tests/test_models.py index b7195f8..b371bee 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -9,6 +9,8 @@ permanently frozen timestamp for the life of the process. These tests construct two separate instances (with a real delay for the timestamp case) and assert they differ -- the check that would have caught the bug. + +Developer: Manish Kumar """ import time from datetime import datetime, timezone @@ -19,12 +21,14 @@ def test_event_id_differs_across_instances(): + """Assign a distinct event_id to each AuditEvent instance.""" e1 = AuditEvent(service="svc", event_type="test") e2 = AuditEvent(service="svc", event_type="test") assert e1.event_id != e2.event_id def test_timestamp_differs_across_instances(): + """Advance the timestamp for each AuditEvent constructed later.""" e1 = AuditEvent(service="svc", event_type="test") time.sleep(0.05) e2 = AuditEvent(service="svc", event_type="test") @@ -33,17 +37,20 @@ def test_timestamp_differs_across_instances(): def test_explicitly_supplied_event_id_is_respected(): + """Preserve an explicitly supplied event_id instead of generating one.""" event = AuditEvent(service="svc", event_type="test", event_id="fixed-id-123") assert event.event_id == "fixed-id-123" def test_organization_tenant_scope_requires_first_class_id(): + """Reject tenant_scope=organization when no organization_id is supplied.""" from pydantic import ValidationError with pytest.raises(ValidationError): AuditEvent(service="svc", event_type="test", tenant_scope="organization") def test_global_and_unknown_are_distinct(): + """Distinguish an explicit global tenant_scope from the default unknown scope.""" global_event = AuditEvent(service="svc", event_type="maintenance", tenant_scope="global") unknown_event = AuditEvent(service="svc", event_type="test") assert global_event.tenant_scope == "global" @@ -59,6 +66,7 @@ def test_organization_id_alone_promotes_scope_to_organization(): def test_organization_id_with_explicit_non_organization_scope_is_rejected(): + """Reject an organization_id paired with a tenant_scope other than organization.""" from pydantic import ValidationError with pytest.raises(ValidationError): AuditEvent( @@ -68,6 +76,7 @@ def test_organization_id_with_explicit_non_organization_scope_is_rejected(): def test_explicitly_supplied_timestamp_is_respected(): + """Preserve an explicitly supplied timestamp instead of generating one.""" fixed = datetime(2024, 1, 1, 0, 0, 0, tzinfo=timezone.utc) event = AuditEvent(service="svc", event_type="test", timestamp=fixed) assert event.timestamp == fixed diff --git a/tests/test_mysql_integration_guard.py b/tests/test_mysql_integration_guard.py index e53de0d..098e56b 100644 --- a/tests/test_mysql_integration_guard.py +++ b/tests/test_mysql_integration_guard.py @@ -14,6 +14,8 @@ not fail, when that isolated instance isn't reachable, matching this suite's existing convention. Cases A, B, C, E need no live server at all: they test pure validation logic that never attempts a connection. + +Developer: Manish Kumar """ import os @@ -34,6 +36,7 @@ def _sql_quoted_list(names) -> str: + """Join a list of names into a comma-separated, single-quoted SQL literal list.""" return ", ".join("'" + n + "'" for n in names) @@ -85,6 +88,7 @@ def _must_not_be_called(*_args, **_kwargs): def test_b_c_rejection_message_names_the_forbidden_port(): + """Name the forbidden port in the error raised for a URL that resolves to it.""" with pytest.raises(ProductionMySQLEndpointRejected, match=str(FORBIDDEN_PORT)): validate_test_mysql_url("mysql+pymysql://root:root@localhost:3306/mysql") @@ -95,6 +99,8 @@ def test_b_c_rejection_message_names_the_forbidden_port(): def _isolated_mysql_available(): + """Report whether the configured isolated test-MySQL URL is reachable, returning False when + unconfigured or unreachable.""" if not _ISOLATED_TEST_URL: return False try: @@ -114,6 +120,7 @@ def _isolated_mysql_available(): def test_d_validation_accepts_non_production_port(): + """Accept a URL on a non-production port unchanged.""" assert validate_test_mysql_url("mysql+pymysql://root:root@127.0.0.1:33061/mysql") == ( "mysql+pymysql://root:root@127.0.0.1:33061/mysql" ) @@ -121,6 +128,7 @@ def test_d_validation_accepts_non_production_port(): @_isolated_skip def test_d_isolated_endpoint_is_a_real_usable_mysql_server(): + """Execute a real query against the configured isolated MySQL instance and get back its result.""" engine = create_engine(_ISOLATED_TEST_URL) with engine.connect() as conn: result = conn.execute(text("SELECT 1")).scalar() @@ -130,6 +138,8 @@ def test_d_isolated_endpoint_is_a_real_usable_mysql_server(): # --- E: unique test principal naming --------------------------------------- def test_e_unique_identifiers_are_distinct_and_within_mysql_username_limit(): + """Generate 20 distinct identifiers, each within MySQL's 32-character username limit and + prefixed as expected.""" names = {unique_test_identifier("audit_writer") for _ in range(20)} assert len(names) == 20, "each call must produce a distinct name" for name in names: @@ -138,6 +148,7 @@ def test_e_unique_identifiers_are_distinct_and_within_mysql_username_limit(): def test_e_unique_identifier_trims_long_prefix_not_the_random_suffix(): + """Trim an oversized prefix down to the length budget while keeping the random suffix intact.""" long_prefix = "a" * 40 name = unique_test_identifier(long_prefix, max_length=32) assert len(name) <= 32 diff --git a/tests/test_no_destructive_stream_operations.py b/tests/test_no_destructive_stream_operations.py index 861d8a6..21052dd 100644 --- a/tests/test_no_destructive_stream_operations.py +++ b/tests/test_no_destructive_stream_operations.py @@ -25,6 +25,8 @@ in this codebase's operational paths, so its mere presence is the violation -- no attempt is made to reason about which specific key it might target. + +Developer: Manish Kumar """ from __future__ import annotations @@ -53,6 +55,7 @@ def _operational_python_files(): + """Yield every Python source file under the operational directories, skipping __pycache__.""" for dirname in OPERATIONAL_DIRS: directory = REPO_ROOT / dirname if not directory.is_dir(): @@ -61,6 +64,8 @@ def _operational_python_files(): def test_no_destructive_redis_operations_in_operational_code(): + """Forbid destructive Redis commands, raw command execution, and the KEYS method from appearing + anywhere in operational source.""" violations = [] for path in _operational_python_files(): if "__pycache__" in path.parts: diff --git a/tests/test_no_mutation_routes.py b/tests/test_no_mutation_routes.py index dbd6d8a..2cb0fcc 100644 --- a/tests/test_no_mutation_routes.py +++ b/tests/test_no_mutation_routes.py @@ -4,11 +4,14 @@ app's route table directly (not a hand-maintained list of files), so a future PR that adds a mutating route anywhere in this service fails this test rather than silently reintroducing a mutation path. + +Developer: Manish Kumar """ from api.main import app def test_every_route_in_this_service_is_read_only(): + """Forbid every route in the app from registering PUT, PATCH, or DELETE.""" mutating_methods = {"PUT", "PATCH", "DELETE"} offenders = [] for route in app.routes: diff --git a/tests/test_processor.py b/tests/test_processor.py index d201d3d..7736c54 100644 --- a/tests/test_processor.py +++ b/tests/test_processor.py @@ -1,3 +1,9 @@ +"""Validate process_event's shallow field extraction and parse_audit_event's full AuditEvent +construction, including first-class tenant field handling and rejection of malformed input. + +Developer: Manish Kumar +""" + import json from datetime import datetime @@ -12,24 +18,28 @@ # --------------------------------------------------------------------------- def test_process_event_extracts_user_id(): + """Extract user_id into the user key.""" raw = json.dumps({"user_id": "u1", "event_type": "auth_login", "decision": "allow"}) result = process_event(raw) assert result["user"] == "u1" def test_process_event_extracts_event_type(): + """Extract event_type into the event key.""" raw = json.dumps({"user_id": "u1", "event_type": "policy_decision", "decision": "deny"}) result = process_event(raw) assert result["event"] == "policy_decision" def test_process_event_extracts_decision(): + """Extract decision into the decision key.""" raw = json.dumps({"user_id": "u1", "event_type": "test", "decision": "success"}) result = process_event(raw) assert result["decision"] == "success" def test_process_event_missing_fields_return_none(): + """Return None for every field missing from the raw payload.""" raw = json.dumps({}) result = process_event(raw) assert result["user"] is None @@ -38,6 +48,7 @@ def test_process_event_missing_fields_return_none(): def test_process_event_partial_fields(): + """Extract only the fields present, leaving the rest None.""" raw = json.dumps({"user_id": "u2"}) result = process_event(raw) assert result["user"] == "u2" @@ -45,11 +56,13 @@ def test_process_event_partial_fields(): def test_process_event_raises_on_invalid_json(): + """Raise JSONDecodeError from process_event for a payload that is not valid JSON.""" with pytest.raises(json.JSONDecodeError): process_event("not-json") def test_process_event_returns_dict(): + """Return a dict with exactly the user, event, and decision keys.""" raw = json.dumps({"user_id": "u3", "event_type": "test", "decision": "ok"}) result = process_event(raw) assert isinstance(result, dict) @@ -62,6 +75,7 @@ def test_process_event_returns_dict(): # --------------------------------------------------------------------------- def test_parse_audit_event_returns_audit_event(): + """Parse a raw JSON payload into an AuditEvent with its service and context.""" raw = json.dumps({ "event_id": "evt-1", "timestamp": "2026-01-01T12:00:00", @@ -81,6 +95,7 @@ def test_parse_audit_event_returns_audit_event(): def test_parse_audit_event_preserves_all_fields(): + """Preserve resource, reason, and trace_id and parse the timestamp into a datetime.""" raw = json.dumps({ "event_id": "evt-2", "timestamp": "2026-01-01T12:00:00", @@ -103,6 +118,7 @@ def test_parse_audit_event_preserves_all_fields(): def test_parse_audit_event_preserves_first_class_tenant(): + """Preserve organization_id and tenant_scope from the raw payload.""" event = parse_audit_event(json.dumps({ "event_id": "tenant-1", "timestamp": "2026-01-01T12:00:00", "service": "tes", "event_type": "run", "organization_id": "org-7", @@ -113,12 +129,14 @@ def test_parse_audit_event_preserves_first_class_tenant(): def test_legacy_event_without_tenant_is_unknown_not_global(): + """Parse a legacy payload with no tenant fields as tenant_scope=unknown, not global.""" event = parse_audit_event(json.dumps({"service": "svc", "event_type": "test"})) assert event.organization_id is None assert event.tenant_scope == "unknown" def test_tenant_is_never_inferred_from_context(): + """Never read organization_id out of the context dict, even when present there.""" event = parse_audit_event(json.dumps({ "service": "svc", "event_type": "test", "context": {"organization_id": "org-context-only"}, @@ -128,11 +146,13 @@ def test_tenant_is_never_inferred_from_context(): def test_parse_audit_event_raises_on_invalid_json(): + """Raise JSONDecodeError for a payload that is not valid JSON.""" with pytest.raises(json.JSONDecodeError): parse_audit_event("not-json") def test_parse_audit_event_raises_on_missing_required_fields(): + """Raise ValidationError when service or event_type is missing.""" raw = json.dumps({"user_id": "u1"}) # missing service/event_type with pytest.raises(ValidationError): parse_audit_event(raw) diff --git a/tests/test_producer_contract_reconciliation.py b/tests/test_producer_contract_reconciliation.py index 4a36b10..45dace5 100644 --- a/tests/test_producer_contract_reconciliation.py +++ b/tests/test_producer_contract_reconciliation.py @@ -13,6 +13,8 @@ sudden validation/assertion failure the next time both repos' test suites are run -- which is the whole point of encoding the contract as an executable fixture instead of only as prose in a report. + +Developer: Manish Kumar """ import json import uuid @@ -52,6 +54,8 @@ def _gateway_shaped_payload(**overrides) -> dict: # --------------------------------------------------------------------------- def test_gateway_request_event_parses_and_persists(db_session): + """Parse a gateway-shaped request event and persist it with its service, event_type, and context + intact.""" raw = _gateway_shaped_payload( event_type="request", user_id="u1", diff --git a/tests/test_record_integrity.py b/tests/test_record_integrity.py index 7c43b3f..732ebd3 100644 --- a/tests/test_record_integrity.py +++ b/tests/test_record_integrity.py @@ -28,6 +28,8 @@ (~50% of random datetime.now() values) failure in a real end-to-end integration test run. See test_timestamp_at_or_above_rounding_boundary_rounds_up_to_the_next_second. + +Developer: Manish Kumar """ from datetime import datetime @@ -40,6 +42,7 @@ def _base_audit_event(**overrides): + """Build a valid base audit-event record dict for hashing, applying any field overrides.""" record = { "event_id": "evt-1", "timestamp": datetime(2026, 9, 16, 12, 0, 0), # noqa: DTZ001 -- naive column, matches AuditEventRecord.timestamp convention @@ -61,6 +64,7 @@ def _base_audit_event(**overrides): def _base_quarantine_record(**overrides): + """Build a valid base quarantine-record dict for hashing, applying any field overrides.""" record = { "stream_message_id": "1-0", "raw_data": '{"service":"test"}', @@ -82,18 +86,21 @@ def _base_quarantine_record(**overrides): # --------------------------------------------------------------------------- def test_audit_event_hash_verifies_for_unmodified_record(): + """Verify an audit event's hash against its own unmodified record.""" record = _base_audit_event() record["record_integrity_hash"] = compute_audit_event_hash(record, SECRET) assert verify_audit_event_hash(record, SECRET) is True def test_quarantine_record_hash_verifies_for_unmodified_record(): + """Verify a quarantine record's hash against its own unmodified record.""" record = _base_quarantine_record() record["record_integrity_hash"] = compute_quarantine_record_hash(record, SECRET) assert verify_quarantine_record_hash(record, SECRET) is True def test_audit_event_hash_fails_when_any_covered_field_changes(): + """Fail verification once a hash-covered audit-event field is changed.""" record = _base_audit_event() record["record_integrity_hash"] = compute_audit_event_hash(record, SECRET) record["action"] = "TAMPERED" @@ -101,6 +108,7 @@ def test_audit_event_hash_fails_when_any_covered_field_changes(): def test_quarantine_record_hash_fails_when_any_covered_field_changes(): + """Fail verification once a hash-covered quarantine-record field is changed.""" record = _base_quarantine_record() record["record_integrity_hash"] = compute_quarantine_record_hash(record, SECRET) record["failure_category"] = "TAMPERED" @@ -108,18 +116,21 @@ def test_quarantine_record_hash_fails_when_any_covered_field_changes(): def test_verify_fails_closed_when_hash_is_missing(): + """Fail verification, not raise, when the stored hash is missing.""" record = _base_audit_event() record["record_integrity_hash"] = None assert verify_audit_event_hash(record, SECRET) is False def test_verify_fails_closed_with_wrong_secret(): + """Fail record-hash verification when checked against the wrong secret.""" record = _base_audit_event() record["record_integrity_hash"] = compute_audit_event_hash(record, SECRET) assert verify_audit_event_hash(record, "a-different-secret") is False def test_verify_never_raises_on_malformed_record(): + """Return False instead of raising for a completely empty record.""" assert verify_audit_event_hash({}, SECRET) is False assert verify_quarantine_record_hash({}, SECRET) is False @@ -131,6 +142,7 @@ def test_verify_never_raises_on_malformed_record(): # --------------------------------------------------------------------------- def test_context_as_dict_and_as_equivalent_json_string_hash_identically(): + """Hash an equivalent context identically whether it is given as a dict or as its JSON string.""" as_dict = _base_audit_event(context={"a": 1, "b": [1, 2, 3]}) as_json_string = _base_audit_event(context='{"a": 1, "b": [1, 2, 3]}') @@ -141,6 +153,7 @@ def test_context_as_dict_and_as_equivalent_json_string_hash_identically(): def test_context_key_order_does_not_affect_the_hash(): + """Hash a context dict identically regardless of key order.""" record_a = _base_audit_event(context={"a": 1, "b": 2}) record_b = _base_audit_event(context={"b": 2, "a": 1}) @@ -176,6 +189,7 @@ def test_a_hash_computed_from_a_dict_verifies_against_a_row_shaped_as_json_strin # --------------------------------------------------------------------------- def test_timestamp_below_rounding_boundary_truncates_down(): + """Hash a sub-half-second timestamp the same as its truncated whole second.""" below_half = _base_audit_event(timestamp=datetime(2026, 9, 16, 12, 0, 0, 499999)) # noqa: DTZ001 -- naive column, matches AuditEventRecord.timestamp convention whole_second = _base_audit_event(timestamp=datetime(2026, 9, 16, 12, 0, 0, 0)) # noqa: DTZ001 -- same as above @@ -223,6 +237,7 @@ def test_different_whole_second_timestamps_still_produce_different_hashes(): # --------------------------------------------------------------------------- def test_every_audit_event_field_affects_the_hash(): + """Change the hash when any single audit-event field is mutated.""" from audit.record_integrity import AUDIT_EVENT_FIELDS base = _base_audit_event() @@ -242,6 +257,7 @@ def test_every_audit_event_field_affects_the_hash(): def test_every_quarantine_field_affects_the_hash(): + """Change the hash when any single quarantine-record field is mutated.""" from audit.record_integrity import QUARANTINE_RECORD_FIELDS base = _base_quarantine_record() @@ -263,6 +279,8 @@ def test_every_quarantine_field_affects_the_hash(): # --------------------------------------------------------------------------- def test_record_integrity_hash_differs_from_producer_signature_for_equivalent_content(): + """Produce a record-integrity hash that never collides with the producer's own signature for the + same content.""" from audit.signing import sign_audit_event record = _base_audit_event() diff --git a/tests/test_redis_acl_safety.py b/tests/test_redis_acl_safety.py index 97f8cea..175937d 100644 --- a/tests/test_redis_acl_safety.py +++ b/tests/test_redis_acl_safety.py @@ -20,6 +20,8 @@ production-adjacent Redis port) is refused outright even if something upstream ever tried to hand it to this fixture. Skipped automatically if the `docker` CLI is unavailable. + +Developer: Manish Kumar """ from __future__ import annotations @@ -58,40 +60,53 @@ class TestNormalizeCommand: + """Validate _normalize_command's uppercasing, subcommand-family two-token normalization, + bytes/str equivalence, whitespace stripping, and rejection of an empty command; and + _safe_repr_command's exclusion of argument values.""" + def test_simple_command_uppercased(self): + """Uppercase a simple command regardless of its input casing.""" assert _normalize_command(["ping"]) == ("PING",) assert _normalize_command(["PiNg"]) == ("PING",) def test_subcommand_family_normalized_to_two_tokens(self): + """Normalize a subcommand-family command to its two-token (COMMAND, SUBCOMMAND) form.""" assert _normalize_command(["config", "set", "maxmemory", "0"]) == ("CONFIG", "SET") assert _normalize_command(["ACL", "setuser", "x"]) == ("ACL", "SETUSER") assert _normalize_command(["xgroup", "CREATE", "s", "g"]) == ("XGROUP", "CREATE") assert _normalize_command(["script", "flush"]) == ("SCRIPT", "FLUSH") def test_non_subcommand_family_stays_one_token_even_with_extra_args(self): + """Keep a non-subcommand-family command to a single token even with extra arguments.""" assert _normalize_command(["GET", "somekey"]) == ("GET",) assert _normalize_command(["SET", "k", "v"]) == ("SET",) def test_bytes_args_normalized_same_as_str(self): + """Normalize bytes-typed command arguments the same as their string equivalents.""" assert _normalize_command([b"FlUsHaLl"]) == ("FLUSHALL",) assert _normalize_command([b"config", b"SET"]) == ("CONFIG", "SET") def test_whitespace_stripped(self): + """Strip surrounding whitespace from a command token before normalizing.""" assert _normalize_command([" flushall "]) == ("FLUSHALL",) def test_mixed_case_variations_all_equal(self): + """Normalize every case variation of a command to the same tuple.""" variants = ["FLUSHALL", "flushall", "FlUsHaLl", "fLUSHALL"] assert len({_normalize_command([v]) for v in variants}) == 1 def test_empty_command_rejected(self): + """Raise ProhibitedCommandError for an empty command list.""" with pytest.raises(ProhibitedCommandError): _normalize_command([]) def test_empty_string_command_rejected(self): + """Raise ProhibitedCommandError for a command list containing only an empty string.""" with pytest.raises(ProhibitedCommandError): _normalize_command([""]) def test_safe_repr_never_includes_extra_args(self): + """Render only the command name in the safe repr, never its argument values.""" assert _safe_repr_command(["SET", "k", "supersecretvalue"]) == "SET" assert "supersecretvalue" not in _safe_repr_command(["SET", "k", "supersecretvalue"]) assert _safe_repr_command(["ACL", "SETUSER", "x", ">password"]) == "ACL SETUSER" @@ -104,15 +119,22 @@ def test_safe_repr_never_includes_extra_args(self): class TestProductionValidatorAllowlist: + """Validate ProductionValidator's allowlist: exactly which commands it permits, which it denies + as merely prohibited versus specifically dangerous, and that error messages never leak + argument values.""" + @pytest.fixture def gate(self): + """Provide a fresh ProductionValidator.""" return ProductionValidator() @pytest.mark.parametrize("cmd", [["PING"], ["ping"], ["ACL", "WHOAMI"], ["acl", "whoami"], ["INFO"], ["info"]]) def test_allowed_commands_pass(self, gate, cmd): + """Authorize every allowlisted command without raising.""" gate.authorize(cmd) # must not raise def test_allowlist_is_exactly_the_documented_minimum(self): + """Pin the production allowlist to exactly PING, ACL WHOAMI, and INFO.""" assert PRODUCTION_ALLOWED_COMMANDS == frozenset({("PING",), ("ACL", "WHOAMI"), ("INFO",)}) @pytest.mark.parametrize("cmd", [ @@ -120,6 +142,7 @@ def test_allowlist_is_exactly_the_documented_minimum(self): ["XLEN", "s"], ["SCAN", "0"], ["KEYS", "*"], ["DBSIZE"], ["CLIENT", "LIST"], ]) def test_non_allowlisted_harmless_looking_commands_still_denied(self, gate, cmd): + """Deny a harmless-looking but non-allowlisted command.""" with pytest.raises(ProhibitedCommandError): gate.authorize(cmd) @@ -132,18 +155,23 @@ def test_non_allowlisted_harmless_looking_commands_still_denied(self, gate, cmd) ["RESTORE-ASKING"], ["SWAPDB", "0", "1"], ["REPLICAOF", "no", "one"], ["SLAVEOF", "no", "one"], ]) def test_dangerous_commands_denied_as_dangerous_specifically(self, gate, cmd): + """Deny a destructive command with DangerousCommandError specifically, not just a generic + denial.""" with pytest.raises(DangerousCommandError): gate.authorize(cmd) def test_dangerous_error_is_a_prohibited_command_error(self, gate): + """Raise DangerousCommandError as a subtype of ProhibitedCommandError.""" with pytest.raises(ProhibitedCommandError): gate.authorize(["FLUSHALL"]) def test_bytes_command_cannot_bypass_dangerous_check(self, gate): + """Deny a dangerous command given as bytes the same as its string form.""" with pytest.raises(DangerousCommandError): gate.authorize([b"FLUSHALL"]) def test_subcommand_split_cannot_bypass_dangerous_check(self, gate): + """Deny a dangerous subcommand regardless of how its tokens are split.""" # A caller cannot dodge the ("CONFIG", "SET") tuple by passing the # subcommand as a separate positional differently-cased token. with pytest.raises(DangerousCommandError): @@ -152,6 +180,7 @@ def test_subcommand_split_cannot_bypass_dangerous_check(self, gate): gate.authorize(["config", "SET"]) def test_error_message_never_contains_argument_values(self, gate): + """Keep argument values out of the error message raised for a denied command.""" with pytest.raises(ProhibitedCommandError) as excinfo: gate.authorize(["SET", "k", "topsecretvalue123"]) assert "topsecretvalue123" not in str(excinfo.value) @@ -163,32 +192,43 @@ def test_error_message_never_contains_argument_values(self, gate): class TestClassifyEnvironmentWithoutRedis: + """Validate classify_environment and DisposableAttestation's construction-time checks without a + real Redis connection: production-port detection, the unknown default, and rejection of + malformed or unreachable attestations.""" + def test_production_port_always_classified_production_even_with_no_attestation(self): + """Classify the production port as PRODUCTION even with no attestation supplied.""" assert classify_environment(host="redis", port=PRODUCTION_PORT, attestation=None) is RedisEnvironment.PRODUCTION def test_unknown_when_no_attestation_supplied(self): + """Classify a non-production host/port with no attestation as UNKNOWN.""" # No live probe is even attempted when there's no attestation -- # a bogus host/port here would still correctly resolve to UNKNOWN. assert classify_environment(host="nonexistent.invalid", port=59999, attestation=None) is RedisEnvironment.UNKNOWN def test_attestation_construction_rejects_production_port(self): + """Reject constructing a DisposableAttestation for the production port.""" with pytest.raises(EnvironmentClassificationError): DisposableAttestation(host="redis", port=PRODUCTION_PORT, nonce_key="k", nonce_value="v") def test_attestation_construction_rejects_empty_nonce(self): + """Reject constructing a DisposableAttestation with an empty nonce key and value.""" with pytest.raises(EnvironmentClassificationError): DisposableAttestation(host="localhost", port=16399, nonce_key="", nonce_value="") def test_attestation_construction_rejects_empty_host(self): + """Reject constructing a DisposableAttestation with an empty host.""" with pytest.raises(EnvironmentClassificationError): DisposableAttestation(host="", port=16399, nonce_key="k", nonce_value="v") def test_mismatched_host_port_attestation_rejected(self): + """Reject classifying an environment against an attestation built for a different host/port.""" attestation = DisposableAttestation(host="127.0.0.1", port=16399, nonce_key="k", nonce_value="v") with pytest.raises(EnvironmentClassificationError): classify_environment(host="127.0.0.1", port=16400, attestation=attestation) def test_unreachable_disposable_target_fails_closed_not_disposable(self): + """Raise, not silently classify as disposable, when the attested target cannot be reached.""" # Well-formed attestation, but nothing is actually listening -- # must raise, never silently fall through to DISPOSABLE. attestation = DisposableAttestation(host="127.0.0.1", port=1, nonce_key="k", nonce_value="v") @@ -196,11 +236,13 @@ def test_unreachable_disposable_target_fails_closed_not_disposable(self): classify_environment(host="127.0.0.1", port=1, attestation=attestation) def test_disposable_validator_construction_fails_closed_when_unreachable(self): + """Raise when a disposable validator is constructed against an unreachable target.""" attestation = DisposableAttestation(host="127.0.0.1", port=1, nonce_key="k", nonce_value="v") with pytest.raises(EnvironmentClassificationError): DisposableValidator(host="127.0.0.1", port=1, attestation=attestation) def test_negative_test_gate_construction_fails_closed_when_unreachable(self): + """Raise when a negative test gate is constructed against an unreachable target.""" attestation = DisposableAttestation(host="127.0.0.1", port=1, nonce_key="k", nonce_value="v") with pytest.raises(EnvironmentClassificationError): DisposableNegativeTestGate(host="127.0.0.1", port=1, attestation=attestation) @@ -213,7 +255,10 @@ def test_negative_test_gate_construction_fails_closed_when_unreachable(self): class TestAuthenticateProductionGuard: + """Validate that authenticate_production refuses to proceed against a non-production port.""" + def test_authenticate_production_rejects_non_production_port(self): + """Reject authenticate_production when the target port is not the production port.""" with pytest.raises(EnvironmentClassificationError): authenticate_production( host="127.0.0.1", port=16399, username="x", password="y", @@ -230,6 +275,7 @@ def test_authenticate_production_rejects_non_production_port(self): def _run(cmd, **kw): + """Run a subprocess command, capturing output and applying a 30-second timeout.""" return subprocess.run(cmd, capture_output=True, text=True, timeout=30, check=False, **kw) @@ -292,6 +338,8 @@ def disposable_redis(): @pytest.fixture def attestation(disposable_redis): + """Build a DisposableAttestation bound to the disposable Redis container's host, port, and + nonce.""" return DisposableAttestation( host=disposable_redis["host"], port=disposable_redis["port"], nonce_key=disposable_redis["nonce_key"], nonce_value=disposable_redis["nonce_value"], @@ -304,11 +352,17 @@ def attestation(disposable_redis): class TestClassifyEnvironmentRealRedis: + """Validate classify_environment against a real disposable Redis instance: a valid attestation + classifies as disposable, and a wrong nonce key or value fails closed.""" + def test_valid_attestation_classified_disposable(self, disposable_redis, attestation): + """Classify the disposable container as DISPOSABLE given a valid attestation against it.""" env = classify_environment(host=disposable_redis["host"], port=disposable_redis["port"], attestation=attestation) assert env is RedisEnvironment.DISPOSABLE def test_wrong_nonce_value_fails_closed(self, disposable_redis): + """Fail closed when the attestation's nonce value does not match what is stored on the + target.""" bad = DisposableAttestation( host=disposable_redis["host"], port=disposable_redis["port"], nonce_key=disposable_redis["nonce_key"], nonce_value="not-the-real-nonce", @@ -317,6 +371,7 @@ def test_wrong_nonce_value_fails_closed(self, disposable_redis): classify_environment(host=disposable_redis["host"], port=disposable_redis["port"], attestation=bad) def test_wrong_nonce_key_fails_closed(self, disposable_redis): + """Fail closed when the attestation's nonce key does not match what is stored on the target.""" bad = DisposableAttestation( host=disposable_redis["host"], port=disposable_redis["port"], nonce_key="nonexistent_key_never_set", nonce_value=disposable_redis["nonce_value"], @@ -403,6 +458,8 @@ def spy(self, *args, **kwargs): assert sent == ["AUTH"], f"expected only AUTH to have been sent, got {sent!r}" def test_framework_succeeds_with_correct_credential_and_matching_identity(self, disposable_redis): + """Authenticate and run PING against the disposable Redis with the correct credential and + matching identity.""" gate = ProductionValidator() session = authenticate( host=disposable_redis["host"], port=disposable_redis["port"], @@ -416,6 +473,7 @@ def test_framework_succeeds_with_correct_credential_and_matching_identity(self, session.close() def test_session_after_close_refuses_further_commands(self, disposable_redis): + """Raise when a command is run on a session after it has been closed.""" gate = ProductionValidator() session = authenticate( host=disposable_redis["host"], port=disposable_redis["port"], @@ -434,6 +492,9 @@ def test_session_after_close_refuses_further_commands(self, disposable_redis): class TestIdentityMismatchRealRedis: + """Validate that an identity mismatch during authentication against real Redis leaves no usable + session, sending only AUTH then ACL WHOAMI.""" + def test_expected_identity_not_matching_actual_is_hard_failure(self, disposable_redis): """Authenticate correctly as redis_monitoring_test, but assert the WRONG expected identity -- must be a hard @@ -450,6 +511,8 @@ def test_expected_identity_not_matching_actual_is_hard_failure(self, disposable_ ) def test_mismatch_error_does_not_leave_a_usable_session(self, disposable_redis, monkeypatch): + """Send only AUTH then ACL WHOAMI, and raise IdentityMismatchError, when the authenticated + identity does not match the expected one.""" sent = [] real_execute = redis.Redis.execute_command @@ -475,7 +538,11 @@ def spy(self, *args, **kwargs): class TestAuthenticationFailureShapesRealRedis: + """Validate authentication failure handling against real Redis: a nonexistent username, a + correct credential succeeding, and an unreachable host failing closed.""" + def test_nonexistent_username(self, disposable_redis): + """Raise AuthenticationFailedError for a username that does not exist on the target.""" gate = ProductionValidator() with pytest.raises(AuthenticationFailedError): authenticate( @@ -486,6 +553,7 @@ def test_nonexistent_username(self, disposable_redis): ) def test_correct_username_correct_password_succeeds(self, disposable_redis): + """Authenticate successfully with the correct username and password.""" gate = ProductionValidator() session = authenticate( host=disposable_redis["host"], port=disposable_redis["port"], @@ -496,6 +564,7 @@ def test_correct_username_correct_password_succeeds(self, disposable_redis): session.close() def test_unreachable_host_fails_closed(self): + """Raise AuthenticationFailedError, not hang, when the host is unreachable.""" gate = ProductionValidator() with pytest.raises(AuthenticationFailedError): authenticate( @@ -531,7 +600,11 @@ def test_reconnect_after_failure_does_not_inherit_prior_state(self, disposable_r class TestProductionAllowlistEndToEndRealRedis: + """Validate the production allowlist end to end against real Redis: an allowed command runs, and + FLUSHALL never reaches the wire through a production session.""" + def test_allowed_command_runs(self, disposable_redis): + """Run an allowlisted command through a production session and get a non-empty result.""" gate = ProductionValidator() session = authenticate( host=disposable_redis["host"], port=disposable_redis["port"], @@ -546,6 +619,7 @@ def test_allowed_command_runs(self, disposable_redis): session.close() def test_flushall_never_reaches_the_wire_through_production_session(self, disposable_redis, monkeypatch): + """Deny FLUSHALL before it ever reaches execute_command through a production session.""" sent = [] real_execute = redis.Redis.execute_command @@ -575,6 +649,9 @@ def spy(self, *args, **kwargs): class TestDisposableDestructiveProofRealRedis: + """Validate that a restricted disposable-test identity is denied writes outside its authorized + keys, and that the negative test gate cannot be constructed for the production port.""" + def test_restricted_identity_denied_flushall_by_redis_itself(self, disposable_redis, attestation): """This is the one place a dangerous command is actually sent -- through DisposableNegativeTestGate, against a live-verified @@ -597,6 +674,7 @@ def test_restricted_identity_denied_flushall_by_redis_itself(self, disposable_re session.close() def test_restricted_identity_denied_set_on_unauthorized_key(self, disposable_redis, attestation): + """Deny a SET on a key outside the restricted test identity's authorized keyspace.""" gate = DisposableNegativeTestGate( host=disposable_redis["host"], port=disposable_redis["port"], attestation=attestation, ) @@ -613,6 +691,8 @@ def test_restricted_identity_denied_set_on_unauthorized_key(self, disposable_red session.close() def test_negative_test_gate_cannot_be_constructed_for_production_port(self, disposable_redis): + """Classify the production port as PRODUCTION rather than allowing a negative test gate to + target it.""" # Even with a technically-well-formed attestation pointed at the # disposable instance, asking classify_environment to check # PRODUCTION_PORT directly must never say DISPOSABLE. @@ -634,7 +714,11 @@ def test_disposable_validator_denies_dangerous_commands_even_though_target_is_di class TestUnknownEnvironmentRealRedis: + """Validate that an unknown environment still gets dangerous commands denied by + ProductionValidator, and that a missing attestation never implies disposable.""" + def test_unknown_environment_with_production_validator_still_denies_dangerous(self, disposable_redis): + """Deny a dangerous command even when the environment classifies as unknown.""" # Simulates a caller that never supplied a DisposableAttestation # (environment resolves to UNKNOWN) but still, correctly, uses # ProductionValidator for anything of unproven status. @@ -652,6 +736,8 @@ def test_unknown_environment_with_production_validator_still_denies_dangerous(se session.close() def test_missing_attestation_never_implies_disposable(self): + """Classify hosts with no attestation as UNKNOWN, never DISPOSABLE, regardless of how local + the host looks.""" assert classify_environment(host="127.0.0.1", port=16399, attestation=None) is RedisEnvironment.UNKNOWN assert classify_environment(host="localhost", port=16399, attestation=None) is RedisEnvironment.UNKNOWN assert classify_environment(host="some-test-container", port=16399, attestation=None) is RedisEnvironment.UNKNOWN @@ -663,7 +749,11 @@ def test_missing_attestation_never_implies_disposable(self): class TestRawClientBypassRealRedis: + """Validate that AuthenticatedSession exposes no way to reach the underlying raw Redis client or + its execute_command.""" + def test_authenticated_session_exposes_no_public_raw_client_accessor(self, disposable_redis): + """Expose only environment, run, and close as AuthenticatedSession's public attributes.""" gate = ProductionValidator() session = authenticate( host=disposable_redis["host"], port=disposable_redis["port"], @@ -683,6 +773,7 @@ def test_authenticated_session_exposes_no_public_raw_client_accessor(self, dispo session.close() def test_only_run_and_close_are_the_public_surface(self): + """Define run and close as AuthenticatedSession's only public methods.""" assert AuthenticatedSession.run is not None assert AuthenticatedSession.close is not None # documents, rather than technically enforces, that this is the diff --git a/tests/test_retention_immutability_integration.py b/tests/test_retention_immutability_integration.py index 484626f..234be4d 100644 --- a/tests/test_retention_immutability_integration.py +++ b/tests/test_retention_immutability_integration.py @@ -10,6 +10,8 @@ quarantine-table parity, and the provisioning/retention/verification scripts exercised as subprocesses against this same real database -- proving the actual shipped tooling, not a reimplementation of it. + +Developer: Manish Kumar """ import os import subprocess @@ -47,6 +49,8 @@ def _real_mysql_available(): + """Report whether the configured test-MySQL root URL is reachable, returning False when + unconfigured or unreachable.""" if TEST_MYSQL_ROOT_URL is None: return False try: @@ -169,6 +173,8 @@ def _integration_test_secret() -> str: def _insert_valid_event(admin_url: str, event_id: str, days_old: int = 0) -> None: + """Insert one valid audit event at the given age and confirm it is visible immediately after + commit.""" from sqlalchemy.orm import sessionmaker from consumers.sink import Sink @@ -207,6 +213,7 @@ def _insert_valid_event(admin_url: str, event_id: str, days_old: int = 0) -> Non # --------------------------------------------------------------------------- def test_real_update_is_denied_for_every_identity(real_mysql_db, provisioned_users): + """Deny an UPDATE on audit_events for both the root and writer credentials.""" _insert_valid_event(real_mysql_db, "evt-no-update") for label, url in (("root", real_mysql_db), ("writer", provisioned_users["writer"])): @@ -220,6 +227,8 @@ def test_real_update_is_denied_for_every_identity(real_mysql_db, provisioned_use def test_real_delete_is_denied_for_root_and_writer_and_reader(real_mysql_db, provisioned_users): + """Deny a DELETE on audit_events for the root, writer, and reader credentials, leaving the row + in place.""" _insert_valid_event(real_mysql_db, "evt-no-delete") for label, url in ( @@ -241,6 +250,7 @@ def test_real_delete_is_denied_for_root_and_writer_and_reader(real_mysql_db, pro def test_real_delete_succeeds_for_audit_maintenance_without_a_hold(real_mysql_db, provisioned_users): + """Allow the maintenance credential to delete an audit_events row that carries no legal hold.""" _insert_valid_event(real_mysql_db, "evt-maintenance-delete") engine = create_engine(provisioned_users["maintenance"]) @@ -256,6 +266,7 @@ def test_real_delete_succeeds_for_audit_maintenance_without_a_hold(real_mysql_db def test_real_legal_hold_blocks_deletion_even_for_audit_maintenance(real_mysql_db, provisioned_users): + """Deny the maintenance credential's delete of a row that is under legal hold.""" _insert_valid_event(real_mysql_db, "evt-held") maint_engine = create_engine(provisioned_users["maintenance"]) @@ -295,6 +306,7 @@ def test_real_legal_hold_blocks_deletion_even_for_audit_maintenance(real_mysql_d # --------------------------------------------------------------------------- def test_real_truncate_denied_for_writer_and_reader_via_privilege_absence(provisioned_users): + """Deny TRUNCATE on audit_events for the writer and reader credentials with a privilege error.""" for label, url in (("writer", provisioned_users["writer"]), ("reader", provisioned_users["reader"])): engine = create_engine(url) with engine.connect() as conn, pytest.raises(Exception) as exc_info: @@ -308,6 +320,7 @@ def test_real_truncate_denied_for_writer_and_reader_via_privilege_absence(provis # --------------------------------------------------------------------------- def test_real_quarantine_table_update_and_delete_denied_same_as_canonical(real_mysql_db, provisioned_users): + """Deny UPDATE and DELETE on the quarantine table the same way they are denied on audit_events.""" from sqlalchemy.orm import sessionmaker from consumers.quarantine import QuarantineSink @@ -342,6 +355,7 @@ def test_real_quarantine_table_update_and_delete_denied_same_as_canonical(real_m # --------------------------------------------------------------------------- def test_real_legal_hold_record_cannot_be_updated(provisioned_users): + """Deny an UPDATE on an audit_legal_holds row.""" maint_engine = create_engine(provisioned_users["maintenance"]) with maint_engine.connect() as conn: conn.execute(text( @@ -367,6 +381,7 @@ def test_real_legal_hold_record_cannot_be_updated(provisioned_users): # --------------------------------------------------------------------------- def test_real_writer_can_insert_and_select_but_not_mutate(provisioned_users): + """Allow the writer credential to insert and select but deny it any mutation.""" engine = create_engine(provisioned_users["writer"]) with engine.connect() as conn: conn.execute(text( @@ -380,6 +395,7 @@ def test_real_writer_can_insert_and_select_but_not_mutate(provisioned_users): def test_real_reader_can_select_but_cannot_insert(provisioned_users): + """Allow the reader credential to select but deny it an insert.""" engine = create_engine(provisioned_users["reader"]) with engine.connect() as conn: conn.execute(text("SELECT COUNT(*) FROM audit_events")).scalar_one() # must not raise @@ -399,6 +415,8 @@ def test_real_reader_can_select_but_cannot_insert(provisioned_users): # --------------------------------------------------------------------------- def test_real_retention_cleanup_dry_run_then_execute_respects_legal_hold(real_mysql_db, provisioned_users): + """Report, but not delete, eligible rows on a dry run, then delete only the non-held eligible + row on execute, leaving the held and recent rows.""" _insert_valid_event(real_mysql_db, "evt-retention-old", days_old=200) _insert_valid_event(real_mysql_db, "evt-retention-recent", days_old=1) _insert_valid_event(real_mysql_db, "evt-retention-old-held", days_old=300) @@ -443,6 +461,8 @@ def test_real_retention_cleanup_dry_run_then_execute_respects_legal_hold(real_my def test_real_retention_cleanup_fails_closed_without_retention_days(provisioned_users): + """Exit as a successful no-op, reporting AUDIT_RETENTION_DAYS not set, rather than deleting + anything.""" env = {k: v for k, v in os.environ.items() if k != "AUDIT_RETENTION_DAYS"} env["AUDIT_MAINTENANCE_DATABASE_URL"] = provisioned_users["maintenance"] result = subprocess.run( @@ -454,6 +474,8 @@ def test_real_retention_cleanup_fails_closed_without_retention_days(provisioned_ def test_real_retention_cleanup_fails_closed_with_a_non_maintenance_credential(real_mysql_db, provisioned_users): + """Fail the cleanup script against a non-maintenance credential, with the database trigger + independently rejecting any deletion it attempted.""" _insert_valid_event(real_mysql_db, "evt-retention-writer-guard", days_old=200) env = {**os.environ, "AUDIT_MAINTENANCE_DATABASE_URL": provisioned_users["writer"], "AUDIT_RETENTION_DAYS": "1"} @@ -490,6 +512,7 @@ def test_real_retention_cleanup_fails_closed_with_a_non_maintenance_credential(r # --------------------------------------------------------------------------- def test_real_verify_audit_integrity_tool_distinguishes_valid_from_tampered(real_mysql_db, provisioned_users): + """Report a tampered event as invalid and a valid event as not invalid.""" from sqlalchemy.orm import sessionmaker from scripts.verify_audit_integrity import verify_audit_events diff --git a/tests/test_retention_integrity_health.py b/tests/test_retention_integrity_health.py index 18f2194..09823e6 100644 --- a/tests/test_retention_integrity_health.py +++ b/tests/test_retention_integrity_health.py @@ -3,11 +3,15 @@ scripts/audit_retention_cleanup.py (external script runs) and GET /audit/pipeline-health. Same "unknown is never fabricated" and opt-in-via-env-var discipline as the rest of audit_health_service.py. + +Developer: Manish Kumar """ from services.audit_health_service import get_retention_integrity_health def test_not_configured_when_status_dir_unset(monkeypatch): + """Report status_source as not_configured with no timestamps when AUDIT_HEALTH_STATUS_DIR is + unset.""" monkeypatch.delenv("AUDIT_HEALTH_STATUS_DIR", raising=False) health = get_retention_integrity_health() @@ -18,6 +22,8 @@ def test_not_configured_when_status_dir_unset(monkeypatch): def test_configured_but_never_run_reports_none_fields(monkeypatch, tmp_path): + """Report status_source as configured with null timestamps, never fabricated, when the status + directory exists but no status file has been written.""" monkeypatch.setenv("AUDIT_HEALTH_STATUS_DIR", str(tmp_path)) health = get_retention_integrity_health() @@ -28,6 +34,8 @@ def test_configured_but_never_run_reports_none_fields(monkeypatch, tmp_path): def test_reads_a_real_verification_status_file(monkeypatch, tmp_path): + """Read the last verification timestamp, result, and invalid-event count from a real status + file.""" monkeypatch.setenv("AUDIT_HEALTH_STATUS_DIR", str(tmp_path)) (tmp_path / "audit-integrity-verification.env").write_text( "LAST_VERIFICATION_TS=2026-09-16T12:00:00+00:00\n" @@ -46,6 +54,8 @@ def test_reads_a_real_verification_status_file(monkeypatch, tmp_path): def test_reads_a_real_retention_status_file(monkeypatch, tmp_path): + """Read the last retention run timestamp, mode, result, and deleted total from a real status + file.""" monkeypatch.setenv("AUDIT_HEALTH_STATUS_DIR", str(tmp_path)) (tmp_path / "audit-retention-run.env").write_text( "LAST_RETENTION_RUN_TS=2026-09-16T04:00:00+00:00\n" @@ -100,6 +110,7 @@ def test_real_verify_script_writes_a_status_file_when_configured(tmp_path, monke def test_real_retention_script_writes_a_status_file_when_configured(tmp_path, monkeypatch): + """Read back the result and deleted total the real retention script's status writer wrote.""" import sys sys.path.insert(0, ".") from scripts.audit_retention_cleanup import _write_status_file diff --git a/tests/test_routes_audit.py b/tests/test_routes_audit.py index b284c11..b80d543 100644 --- a/tests/test_routes_audit.py +++ b/tests/test_routes_audit.py @@ -1,3 +1,9 @@ +"""Validate the /health and /audit/test endpoints with a mocked Redis-backed logger: response shape +and the logged test event's fields. + +Developer: Manish Kumar +""" + import pytest from unittest.mock import AsyncMock, MagicMock, patch from fastapi.testclient import TestClient @@ -5,6 +11,8 @@ @pytest.fixture def client(): + """Build a TestClient for the audit API with Redis patched out, yielding the client together + with the mocked logger.""" mock_redis = AsyncMock() mock_redis.xadd = AsyncMock() @@ -23,6 +31,7 @@ def client(): # --------------------------------------------------------------------------- def test_health_returns_ok(client): + """Report {"status": "ok"} from /health.""" tc, _ = client response = tc.get("/health") assert response.status_code == 200 @@ -34,6 +43,7 @@ def test_health_returns_ok(client): # --------------------------------------------------------------------------- def test_audit_test_returns_logged_true(client): + """Return {"logged": true} from /audit/test.""" tc, mock_logger = client response = tc.get("/audit/test") assert response.status_code == 200 @@ -41,12 +51,14 @@ def test_audit_test_returns_logged_true(client): def test_audit_test_calls_logger_log(client): + """Call the logger's log method exactly once for /audit/test.""" tc, mock_logger = client tc.get("/audit/test") mock_logger.log.assert_called_once() def test_audit_test_logs_correct_event_type(client): + """Log a test event with the health_check action and a success decision.""" tc, mock_logger = client tc.get("/audit/test") event = mock_logger.log.call_args[0][0] diff --git a/tests/test_routes_audit_events.py b/tests/test_routes_audit_events.py index 45e88a9..2e3e1ac 100644 --- a/tests/test_routes_audit_events.py +++ b/tests/test_routes_audit_events.py @@ -1,7 +1,10 @@ """PR4.3: GET /audit/events -- the read-only audit query API. HTTP-level tests via the audit_events_client fixture (real SQLite DB + real FastAPI dependency injection, not mocks); SQL-level filter/order/pagination -correctness is covered separately in tests/test_audit_query_service.py.""" +correctness is covered separately in tests/test_audit_query_service.py. + +Developer: Manish Kumar +""" from datetime import datetime, timedelta, timezone from unittest.mock import patch @@ -15,16 +18,19 @@ def _token(**claims): + """Sign an HS256 test token with the shared test secret, applying any extra claims.""" return jwt.encode(claims, SECRET, algorithm="HS256") def _auth_headers(**claims): + """Build an Authorization header carrying a token for the given roles.""" roles = claims.pop("roles", ["platform_admin"]) token = _token(sub="1", roles=roles, **claims) return {"Authorization": f"Bearer {token}"} def _seed(session_factory, count=3): + """Insert the given number of AuditEventRecord rows into the test database.""" db = session_factory() for i in range(count): db.add( @@ -48,6 +54,7 @@ def _seed(session_factory, count=3): # --------------------------------------------------------------------------- def test_platform_admin_can_query_audit_events(audit_events_client): + """Return seeded events to a platform admin from GET /audit/events.""" client, sessions = audit_events_client _seed(sessions, count=1) @@ -58,6 +65,7 @@ def test_platform_admin_can_query_audit_events(audit_events_client): def test_missing_auth_header_returns_401(audit_events_client): + """Reject a request with no Authorization header with 401.""" client, _ = audit_events_client resp = client.get("/audit/events") @@ -66,6 +74,7 @@ def test_missing_auth_header_returns_401(audit_events_client): def test_non_platform_admin_receives_403(audit_events_client): + """Reject a non-platform-admin role with 403.""" client, sessions = audit_events_client _seed(sessions, count=1) @@ -94,6 +103,7 @@ def test_org_admin_never_sees_audit_data_even_with_valid_token(audit_events_clie # --------------------------------------------------------------------------- def test_response_contains_expected_fields(audit_events_client): + """Return exactly the documented response and item fields, including integrity_status.""" client, sessions = audit_events_client _seed(sessions, count=1) @@ -122,6 +132,7 @@ def test_response_contains_expected_fields(audit_events_client): def test_pagination_works(audit_events_client): + """Page results according to the page and page_size query parameters.""" client, sessions = audit_events_client _seed(sessions, count=5) @@ -138,6 +149,7 @@ def test_pagination_works(audit_events_client): def test_empty_result_returns_empty_items_not_error(audit_events_client): + """Return an empty items list and zero total instead of an error when there are no events.""" client, _ = audit_events_client resp = client.get("/audit/events", headers=_auth_headers()) @@ -150,6 +162,7 @@ def test_empty_result_returns_empty_items_not_error(audit_events_client): def test_newest_events_appear_first(audit_events_client): + """Order the /audit/events response with the newest event first.""" client, sessions = audit_events_client _seed(sessions, count=3) @@ -165,6 +178,7 @@ def test_newest_events_appear_first(audit_events_client): # --------------------------------------------------------------------------- def test_filter_by_service_via_query_param(audit_events_client): + """Filter results to the requested service.""" client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( @@ -188,6 +202,7 @@ def test_filter_by_service_via_query_param(audit_events_client): def test_filter_by_decision_and_event_type_via_query_params(audit_events_client): + """Filter results by decision and event_type together.""" client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( @@ -213,6 +228,7 @@ def test_filter_by_decision_and_event_type_via_query_params(audit_events_client) def test_filter_by_integrity_status_via_query_param(audit_events_client): + """Filter results to the requested integrity_status.""" client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( @@ -237,6 +253,7 @@ def test_filter_by_integrity_status_via_query_param(audit_events_client): def test_response_serializes_tenant_fields(audit_events_client): + """Include organization_id and tenant_scope in the serialized item.""" client, sessions = audit_events_client db = sessions() db.add(AuditEventRecord( @@ -255,6 +272,7 @@ def test_response_serializes_tenant_fields(audit_events_client): # --------------------------------------------------------------------------- def test_health_endpoint_still_works(audit_events_client): + """Keep /health working alongside the /audit/events route.""" client, _ = audit_events_client resp = client.get("/health") assert resp.status_code == 200 @@ -268,6 +286,8 @@ def test_health_endpoint_still_works(audit_events_client): # --------------------------------------------------------------------------- def test_database_failure_is_normalized_without_internal_details(): + """Return a 503 with a generic AUDIT_SOURCE_UNAVAILABLE code, never the raw SQL or the + underlying exception's text.""" with patch( "api.routes_audit_events.audit_query_service.list_audit_events", side_effect=OperationalError("SELECT audit_events", {}, Exception("db.internal")), diff --git a/tests/test_routes_audit_health.py b/tests/test_routes_audit_health.py index 3f802b5..070a776 100644 --- a/tests/test_routes_audit_health.py +++ b/tests/test_routes_audit_health.py @@ -1,5 +1,7 @@ """V2-002 (Track E2): GET /audit/pipeline-health -- HTTP-level, platform- admin gated, same auth-header convention as test_routes_audit_events.py. + +Developer: Manish Kumar """ from unittest.mock import MagicMock, patch @@ -9,16 +11,19 @@ def _token(**claims): + """Sign an HS256 test token with the shared test secret, applying any extra claims.""" return jwt.encode(claims, SECRET, algorithm="HS256") def _auth_headers(**claims): + """Build an Authorization header carrying a token for the given roles.""" roles = claims.pop("roles", ["platform_admin"]) token = _token(sub="1", roles=roles, **claims) return {"Authorization": f"Bearer {token}"} def test_missing_auth_header_returns_401(audit_events_client): + """Reject a /audit/pipeline-health request with no Authorization header with 401.""" client, _sessions = audit_events_client resp = client.get("/audit/pipeline-health") @@ -27,6 +32,7 @@ def test_missing_auth_header_returns_401(audit_events_client): def test_non_admin_role_returns_403(audit_events_client): + """Reject a /audit/pipeline-health request from a non-platform-admin role with 403.""" client, _sessions = audit_events_client resp = client.get("/audit/pipeline-health", headers=_auth_headers(roles=["org_admin"])) @@ -35,6 +41,7 @@ def test_non_admin_role_returns_403(audit_events_client): def test_platform_admin_gets_pipeline_health(audit_events_client): + """Report Redis and persistence as available with a pending count and a generated_at timestamp.""" client, _sessions = audit_events_client mock_reader = MagicMock() @@ -55,6 +62,8 @@ def test_platform_admin_gets_pipeline_health(audit_events_client): def test_pipeline_health_degrades_when_redis_unreachable(audit_events_client): + """Report Redis as unavailable, with a null pending count, while the endpoint itself still + returns 200.""" client, _sessions = audit_events_client mock_reader = MagicMock() diff --git a/tests/test_routes_audit_safe.py b/tests/test_routes_audit_safe.py index c52e823..5649a30 100644 --- a/tests/test_routes_audit_safe.py +++ b/tests/test_routes_audit_safe.py @@ -1,3 +1,9 @@ +"""Validate /audit/events/safe's organization-scoped visibility, its inability to be widened by a +query override, empty and error handling, and validation of the timestamp range and page size. + +Developer: Manish Kumar +""" + from datetime import datetime, timedelta from unittest.mock import patch @@ -12,6 +18,7 @@ def _headers(*, org_id=None, org_role=None, permissions=None): + """Build an Authorization header carrying the given organization id, org roles, and permissions.""" claims = {"sub": "actor", "roles": [], "org_role": org_role or [], "permissions": permissions or []} if org_id is not None: claims["org_id"] = org_id @@ -19,6 +26,7 @@ def _headers(*, org_id=None, org_role=None, permissions=None): def _seed(factory): + """Insert one safe-scoped AuditEventRecord per organization/global/unknown tenant scope.""" db = factory() base = datetime(2026, 1, 1, 12, 0, 0) # noqa: DTZ001 for i, (scope, org) in enumerate((("organization", "1"), ("organization", "2"), ("global", None), ("unknown", None))): @@ -32,6 +40,8 @@ def _seed(factory): def test_org_scope_excludes_other_global_and_unknown_and_is_sql_paginated(audit_events_client): + """Return only the requesting org's own events, excluding another org's, global, and unknown + events, with SQL-level pagination and unknown freshness/retention.""" client, sessions = audit_events_client _seed(sessions) response = client.get("/audit/events/safe", headers=_headers(org_id="1", org_role=["org_admin"]), params={"page_size": 1}) @@ -46,6 +56,8 @@ def test_org_scope_excludes_other_global_and_unknown_and_is_sql_paginated(audit_ def test_platform_scope_sees_global_and_unknown(audit_events_client): + """Return global and unknown-scope events, in addition to organization events, to a + platform-scoped caller.""" client, sessions = audit_events_client _seed(sessions) response = client.get("/audit/events/safe", headers=_headers(permissions=["manage_all_orgs"])) @@ -55,12 +67,14 @@ def test_platform_scope_sees_global_and_unknown(audit_events_client): def test_org_query_override_cannot_widen(audit_events_client): + """Reject an org-scoped caller's attempt to query a different organization_id with 403.""" client, _ = audit_events_client response = client.get("/audit/events/safe", headers=_headers(org_id="1", org_role=["org_admin"]), params={"organization_id": "2"}) assert response.status_code == 403 def test_empty_safe_result_is_available(audit_events_client): + """Report an empty result as source-available rather than an error.""" client, _ = audit_events_client response = client.get("/audit/events/safe", headers=_headers(org_id="1", org_role=["org_admin"])) body = response.json() @@ -71,6 +85,8 @@ def test_empty_safe_result_is_available(audit_events_client): def test_safe_auth_failures_and_validation(audit_events_client): + """Reject a missing token, an unscoped caller, an org-scoped caller with no org_id, and an + oversized page_size.""" client, _ = audit_events_client assert client.get("/audit/events/safe").status_code == 401 assert client.get("/audit/events/safe", headers=_headers()).status_code == 403 @@ -79,6 +95,7 @@ def test_safe_auth_failures_and_validation(audit_events_client): def test_from_timestamp_after_to_timestamp_returns_422(audit_events_client): + """Reject a query whose from_timestamp is after its to_timestamp with 422.""" client, _ = audit_events_client response = client.get( "/audit/events/safe", @@ -92,6 +109,8 @@ def test_from_timestamp_after_to_timestamp_returns_422(audit_events_client): def test_safe_database_failure_is_normalized_without_internal_details(): + """Return a 503 from /audit/events/safe with a generic AUDIT_SOURCE_UNAVAILABLE code, never + the raw SQL or the underlying exception's text.""" with patch( "api.routes_audit_safe.audit_query_service.list_safe_audit_events", side_effect=OperationalError("SELECT audit_events", {}, Exception("db.internal")), diff --git a/tests/test_security_alerts.py b/tests/test_security_alerts.py index 48212fc..7337e3d 100644 --- a/tests/test_security_alerts.py +++ b/tests/test_security_alerts.py @@ -5,6 +5,8 @@ PHI/credentials leak into metadata by construction, repeated failures dedupe instead of storming, a broken sink never propagates, and a recovery condition always gets through. + +Developer: Manish Kumar """ import json @@ -20,12 +22,15 @@ @pytest.fixture(autouse=True) def _clean_dedup_state(): + """Reset the alert deduplication state before and after every test.""" _reset_dedup_state_for_tests() yield _reset_dedup_state_for_tests() class RecordingSink: + """Record every alert sent to it for later assertion.""" + def __init__(self): self.sent = [] @@ -34,11 +39,15 @@ def send(self, alert: SecurityAlert) -> None: class RaisingSink: + """Simulate an alert backend outage by raising on every send.""" + def send(self, alert: SecurityAlert) -> None: raise RuntimeError("simulated alert backend outage") def test_alert_carries_condition_severity_component_timestamp(): + """Attach the condition, severity, component, and an auto-generated timestamp to the emitted + alert and hand it to the sink.""" sink = RecordingSink() alert = emit_security_alert( condition="integrity_verification_failed", @@ -57,6 +66,8 @@ def test_alert_carries_condition_severity_component_timestamp(): def test_invalid_severity_falls_back_to_warning_not_dropped(): + """Fall back to warning severity, without dropping the alert, for an unrecognized severity + value.""" sink = RecordingSink() alert = emit_security_alert( condition="x", severity="not-a-real-severity", component="c", message="m", sink=sink, @@ -83,6 +94,8 @@ def test_metadata_never_includes_raw_row_content_by_construction(): def test_repeated_identical_condition_is_deduped_within_window(): + """Suppress a repeated identical condition within the dedup window, delivering only the first + alert.""" sink = RecordingSink() first = emit_security_alert(condition="poison_event_quarantined", severity="warning", component="audit-worker", message="m", sink=sink, _now=1000.0) @@ -95,6 +108,7 @@ def test_repeated_identical_condition_is_deduped_within_window(): def test_dedup_window_expiry_allows_a_new_alert(): + """Deliver a new alert for the same condition once the dedup window has expired.""" sink = RecordingSink() emit_security_alert(condition="c", severity="warning", component="comp", message="m", sink=sink, _now=1000.0, dedup_window_seconds=300) @@ -105,6 +119,8 @@ def test_dedup_window_expiry_allows_a_new_alert(): def test_different_components_are_not_deduped_against_each_other(): + """Deliver alerts for the same condition on different components independently, without deduping + across them.""" sink = RecordingSink() a = emit_security_alert(condition="c", severity="warning", component="worker", message="m", sink=sink, _now=1.0) b = emit_security_alert(condition="c", severity="warning", component="retention-cleanup", message="m", sink=sink, _now=1.0) @@ -114,6 +130,7 @@ def test_different_components_are_not_deduped_against_each_other(): def test_recovery_condition_bypasses_dedup(): + """Deliver a recovery alert even while its failure condition is still within the dedup window.""" sink = RecordingSink() emit_security_alert(condition="backup_failed", severity="critical", component="backup", message="m", sink=sink, _now=1.0) @@ -126,18 +143,21 @@ def test_recovery_condition_bypasses_dedup(): def test_broken_sink_never_raises_and_alert_is_still_returned(): + """Return the alert object without raising when the sink itself raises.""" alert = emit_security_alert(condition="c", severity="warning", component="comp", message="m", sink=RaisingSink()) assert alert is not None # emission was attempted, not silently no-op'd def test_broken_sink_failure_is_itself_observable_on_stderr(capsys): + """Report a broken sink's failure on stderr so the emission failure itself is observable.""" emit_security_alert(condition="c", severity="warning", component="comp", message="m", sink=RaisingSink()) captured = capsys.readouterr() assert "SECURITY-ALERT-EMISSION-FAILED" in captured.err def test_default_stdout_sink_prints_json_line(capsys): + """Print a [SECURITY-ALERT]-prefixed JSON line to stdout with the default sink.""" emit_security_alert(condition="c", severity="info", component="comp", message="m") captured = capsys.readouterr() assert "[SECURITY-ALERT]" in captured.out @@ -147,6 +167,7 @@ def test_default_stdout_sink_prints_json_line(capsys): def test_file_sink_writes_jsonl(tmp_path): + """Append one JSON line per alert to the configured file sink.""" path = tmp_path / "alerts.jsonl" sink = FileAlertSink(path) emit_security_alert(condition="a", severity="warning", component="comp", message="m1", sink=sink) @@ -158,6 +179,7 @@ def test_file_sink_writes_jsonl(tmp_path): def test_env_configured_file_sink_used_when_no_explicit_sink_given(tmp_path, monkeypatch): + """Write to the file named by AUDIT_ALERT_LOG_FILE when no explicit sink is given.""" log_file = tmp_path / "alerts.jsonl" monkeypatch.setenv("AUDIT_ALERT_LOG_FILE", str(log_file)) emit_security_alert(condition="c", severity="warning", component="comp", message="m") @@ -166,6 +188,7 @@ def test_env_configured_file_sink_used_when_no_explicit_sink_given(tmp_path, mon def test_stdout_sink_used_when_no_env_configured(tmp_path, monkeypatch, capsys): + """Fall back to the stdout sink when AUDIT_ALERT_LOG_FILE is unset.""" monkeypatch.delenv("AUDIT_ALERT_LOG_FILE", raising=False) emit_security_alert(condition="c", severity="warning", component="comp", message="m") assert "[SECURITY-ALERT]" in capsys.readouterr().out diff --git a/tests/test_security_edge_cases.py b/tests/test_security_edge_cases.py index 6150bf7..e65e32f 100644 --- a/tests/test_security_edge_cases.py +++ b/tests/test_security_edge_cases.py @@ -1,4 +1,7 @@ -"""Hermetic security-boundary tests not requiring FastAPI or live backends.""" +"""Hermetic security-boundary tests not requiring FastAPI or live backends. + +Developer: Manish Kumar +""" from __future__ import annotations @@ -46,6 +49,7 @@ def _collect_registered_paths(routes) -> set[str]: def test_fastapi_app_registers_both_audit_routers(): + """Register /health, /audit/test, and /audit/events on the FastAPI app.""" from api.main import app paths = _collect_registered_paths(app.routes) @@ -55,6 +59,7 @@ def test_fastapi_app_registers_both_audit_routers(): def test_signing_rejects_empty_service_and_none_data(): + """Raise ValueError naming the offending field for an empty service or a None data argument.""" with pytest.raises(ValueError, match="service"): signing.sign_audit_event("", "{}", "secret") with pytest.raises(ValueError, match="data"): @@ -75,10 +80,12 @@ def test_signing_rejects_empty_service_and_none_data(): ], ) def test_signature_verification_fails_closed_for_malformed_wire_values(service, data, signature): + """Return False rather than raise for a malformed service, data, or signature value.""" assert signing.verify_audit_event(service, data, signature, "secret") is False def test_signature_verification_fails_closed_if_compare_digest_raises(monkeypatch): + """Return False rather than propagate an exception when hmac.compare_digest itself raises.""" valid = signing.sign_audit_event("auth", '{"event_id":"1"}', "secret") monkeypatch.setattr(signing.hmac, "compare_digest", MagicMock(side_effect=RuntimeError("bad crypto"))) @@ -86,11 +93,14 @@ def test_signature_verification_fails_closed_if_compare_digest_raises(monkeypatc def test_health_route_is_side_effect_free(): + """Return {"status": "ok"} from the health route handler with no side effects.""" assert routes_audit.health() == {"status": "ok"} @pytest.mark.asyncio async def test_audit_test_route_builds_and_logs_service_event(monkeypatch): + """Build and log a security-audit-test event with the health_check action and a success + decision.""" logger = MagicMock() logger.log = AsyncMock() monkeypatch.setattr(routes_audit, "logger", logger) @@ -107,11 +117,14 @@ async def test_audit_test_route_builds_and_logs_service_event(monkeypatch): def test_audit_event_schema_requires_integrity_and_core_fields(): + """Reject constructing AuditEventOut from only a partial set of fields.""" with pytest.raises(ValidationError): AuditEventOut(event_id="evt-1") def test_audit_event_schema_supports_orm_attributes_and_nullable_identity(): + """Build AuditEventOut from an ORM-like object, defaulting user_id to null and integrity_status + to unsigned.""" row = SimpleNamespace( event_id="evt-1", timestamp=datetime(2026, 1, 1, 0, 0, 0, tzinfo=timezone.utc), @@ -139,6 +152,8 @@ def test_audit_event_schema_supports_orm_attributes_and_nullable_identity(): def test_audit_event_list_response_preserves_pagination_contract(): + """Preserve every documented field, including generated_at and source_checked_at, when + serializing AuditEventListResponse.""" generated_at = datetime(2026, 1, 1, tzinfo=timezone.utc) source_checked_at = datetime(2026, 1, 1, 0, 0, 1, tzinfo=timezone.utc) response = AuditEventListResponse( @@ -165,6 +180,8 @@ def test_audit_event_list_response_preserves_pagination_contract(): def test_list_audit_events_route_forwards_all_security_filters(monkeypatch): + """Forward every security filter argument to the query service and return zero total for an + empty result.""" db = MagicMock() captured = {} @@ -194,6 +211,7 @@ def fake_list(db_arg, **kwargs): def test_list_audit_events_route_calculates_nonempty_total_pages(monkeypatch): + """Compute the correct total_pages for a non-empty result set.""" monkeypatch.setattr( routes_audit_events.audit_query_service, "list_audit_events", @@ -209,6 +227,7 @@ def test_list_audit_events_route_calculates_nonempty_total_pages(monkeypatch): def test_get_db_closes_session_after_normal_iteration(monkeypatch): + """Close the session after the get_db generator completes normal iteration.""" # get_db() is bound to ReaderSessionLocal (V2-003 reader/writer split, # db/session.py) -- SessionLocal is the writer factory worker/main.py # uses instead, and patching it here would leave get_db() constructing @@ -223,6 +242,7 @@ def test_get_db_closes_session_after_normal_iteration(monkeypatch): def test_get_db_closes_session_when_consumer_raises(monkeypatch): + """Close the session even when the consumer of get_db raises inside the with block.""" db = MagicMock() monkeypatch.setattr("db.session.ReaderSessionLocal", MagicMock(return_value=db)) generator = get_db() diff --git a/tests/test_signing.py b/tests/test_signing.py index 8f034da..1186248 100644 --- a/tests/test_signing.py +++ b/tests/test_signing.py @@ -3,6 +3,8 @@ Synthetic secrets only -- never a real deployment JWT_SECRET. This module is not yet wired into any producer or consumer (that's PR2+), so these tests exercise audit.signing directly. + +Developer: Manish Kumar """ import hashlib import hmac @@ -21,6 +23,7 @@ # --------------------------------------------------------------------------- def test_valid_signature_verifies(): + """Verify a correctly signed event.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert verify_audit_event(SERVICE, DATA, sig, SECRET) is True @@ -30,12 +33,14 @@ def test_valid_signature_verifies(): # --------------------------------------------------------------------------- def test_modified_data_fails(): + """Fail verification when the data was altered after signing.""" sig = sign_audit_event(SERVICE, DATA, SECRET) tampered = DATA.replace("submit", "delete_all") assert verify_audit_event(SERVICE, tampered, sig, SECRET) is False def test_even_one_byte_of_modified_data_fails(): + """Fail verification when even a single byte of the data changes.""" sig = sign_audit_event(SERVICE, DATA, SECRET) tampered = DATA[:-1] + ("0" if DATA[-1] != "0" else "1") assert verify_audit_event(SERVICE, tampered, sig, SECRET) is False @@ -46,6 +51,8 @@ def test_even_one_byte_of_modified_data_fails(): # --------------------------------------------------------------------------- def test_modified_service_identity_fails(): + """Fail verification when checked against a different service identity than the one that signed + it.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert verify_audit_event("gateway", DATA, sig, SECRET) is False @@ -63,6 +70,7 @@ def test_relabeling_a_valid_signature_onto_a_different_service_fails(): # --------------------------------------------------------------------------- def test_wrong_secret_fails(): + """Fail verification when checked against the wrong secret.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert verify_audit_event(SERVICE, DATA, sig, "a-different-synthetic-secret") is False @@ -86,6 +94,7 @@ def test_wrong_secret_fails(): ], ) def test_malformed_signature_fails(malformed): + """Fail verification for a signature string that is not well-formed.""" assert verify_audit_event(SERVICE, DATA, malformed, SECRET) is False @@ -94,6 +103,7 @@ def test_malformed_signature_fails(malformed): # --------------------------------------------------------------------------- def test_missing_signature_fails(): + """Fail verification for an empty or None signature.""" assert verify_audit_event(SERVICE, DATA, "", SECRET) is False assert verify_audit_event(SERVICE, DATA, None, SECRET) is False @@ -103,6 +113,7 @@ def test_missing_signature_fails(): # --------------------------------------------------------------------------- def test_unsupported_version_fails(): + """Fail verification for a signature carrying a version prefix other than v1.""" sig = sign_audit_event(SERVICE, DATA, SECRET) _, _, mac_hex = sig.partition(":") future_version_sig = f"v2:{mac_hex}" @@ -124,12 +135,14 @@ def test_unsupported_version_fails_even_with_a_correctly_recomputed_mac(): # --------------------------------------------------------------------------- def test_signing_is_deterministic(): + """Produce the identical signature for the same service, data, and secret across calls.""" sig1 = sign_audit_event(SERVICE, DATA, SECRET) sig2 = sign_audit_event(SERVICE, DATA, SECRET) assert sig1 == sig2 def test_different_data_produces_different_signature(): + """Produce a different signature when the data differs.""" sig1 = sign_audit_event(SERVICE, DATA, SECRET) sig2 = sign_audit_event(SERVICE, DATA + "x", SECRET) assert sig1 != sig2 @@ -144,10 +157,14 @@ def test_different_data_produces_different_signature(): # --------------------------------------------------------------------------- def _tes_iam_cache_mac_key(secret: str) -> bytes: + """Derive the TES IAM-cache MAC key from a secret, independently of audit.signing's own key + derivation.""" return hashlib.sha256(f"tes-iam-cache-mac:{secret}".encode()).digest() def _tes_iam_cache_sign(token: str, body: str, secret: str) -> str: + """Sign a token/body pair with the TES IAM-cache MAC key, independently of audit.signing's own + signing.""" return hmac.new(_tes_iam_cache_mac_key(secret), f"{token}\n{body}".encode(), hashlib.sha256).hexdigest() @@ -190,6 +207,8 @@ def test_reserialized_json_with_different_key_order_fails_verification(): def test_whitespace_only_difference_in_data_fails_verification(): + """Fail verification when the data's whitespace differs, even though the JSON content is + equivalent.""" compact = '{"a":1}' spaced = '{"a": 1}' sig = sign_audit_event(SERVICE, compact, SECRET) @@ -203,16 +222,19 @@ def test_whitespace_only_difference_in_data_fails_verification(): # --------------------------------------------------------------------------- def test_sign_rejects_empty_service(): + """Raise ValueError for an empty service.""" with pytest.raises(ValueError): sign_audit_event("", DATA, SECRET) def test_sign_rejects_none_data(): + """Raise ValueError for None data.""" with pytest.raises(ValueError): sign_audit_event(SERVICE, None, SECRET) def test_signature_format_has_version_prefix(): + """Produce a signature prefixed with v1: followed by a 64-character sha256 hex digest.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert sig.startswith("v1:") version, _, mac_hex = sig.partition(":") @@ -234,6 +256,7 @@ def test_verify_rejects_falsy_or_non_string_service_with_otherwise_valid_signatu def test_verify_rejects_none_data_with_otherwise_valid_service_and_signature(): + """Fail verification when data is None, even with an otherwise valid service and signature.""" sig = sign_audit_event(SERVICE, DATA, SECRET) assert verify_audit_event(SERVICE, None, sig, SECRET) is False diff --git a/tests/test_sink.py b/tests/test_sink.py index b2276a9..cd46076 100644 --- a/tests/test_sink.py +++ b/tests/test_sink.py @@ -1,6 +1,9 @@ """PR4.2 regression tests: Sink now persists to audit_events instead of printing (consumers/sink.py). Supersedes the print-based Sink tests that -used to live in tests/test_processor.py.""" +used to live in tests/test_processor.py. + +Developer: Manish Kumar +""" from datetime import datetime, timezone from consumers.sink import Sink @@ -8,6 +11,7 @@ def _event(event_id="evt-1", **overrides): + """Build a parsed AuditEvent for the sink tests, applying any field overrides.""" payload = { "event_id": event_id, "timestamp": datetime(2026, 1, 1, 12, 0, 0, tzinfo=timezone.utc), @@ -26,6 +30,7 @@ def _event(event_id="evt-1", **overrides): def test_sink_write_persists_event(db_session): + """Persist an event with its service and user_id intact.""" sink = Sink(db_session) result = sink.write(_event()) @@ -37,6 +42,7 @@ def test_sink_write_persists_event(db_session): def test_sink_write_persists_first_class_tenant(db_session): + """Persist organization_id and tenant_scope as first-class columns.""" Sink(db_session).write(_event(organization_id="org-7", tenant_scope="organization")) fetched = db_session.get(AuditEventRecord, "evt-1") assert fetched.organization_id == "org-7" @@ -44,6 +50,8 @@ def test_sink_write_persists_first_class_tenant(db_session): def test_sink_legacy_event_defaults_to_unknown_tenant(db_session): + """Default a legacy event with no tenant fields to organization_id=None and + tenant_scope=unknown.""" Sink(db_session).write(_event()) fetched = db_session.get(AuditEventRecord, "evt-1") assert fetched.organization_id is None @@ -51,6 +59,7 @@ def test_sink_legacy_event_defaults_to_unknown_tenant(db_session): def test_sink_write_preserves_context(db_session): + """Persist a nested context object unchanged.""" sink = Sink(db_session) sink.write(_event(context={"a": 1, "b": {"c": 2}})) @@ -97,6 +106,7 @@ def test_sink_write_handles_optional_fields_missing(db_session): # --------------------------------------------------------------------------- def test_sink_write_persists_explicit_valid_status(db_session): + """Persist an explicit valid integrity_status.""" sink = Sink(db_session) sink.write(_event(integrity_status="valid")) @@ -105,6 +115,7 @@ def test_sink_write_persists_explicit_valid_status(db_session): def test_sink_write_persists_explicit_invalid_status(db_session): + """Persist an explicit invalid integrity_status.""" sink = Sink(db_session) sink.write(_event(integrity_status="invalid")) @@ -113,6 +124,7 @@ def test_sink_write_persists_explicit_invalid_status(db_session): def test_sink_write_persists_explicit_unsigned_status(db_session): + """Persist an explicit unsigned integrity_status.""" sink = Sink(db_session) sink.write(_event(integrity_status="unsigned")) diff --git a/tests/test_source_semantics_sat4.py b/tests/test_source_semantics_sat4.py index 3e1e646..74c941b 100644 --- a/tests/test_source_semantics_sat4.py +++ b/tests/test_source_semantics_sat4.py @@ -1,3 +1,9 @@ +"""Validate available_query_evidence's default evidence object: unknown freshness and retention +dimensions, and UTC-aware timestamps. + +Developer: Manish Kumar +""" + from datetime import timezone from audit.source_semantics import ( @@ -9,6 +15,8 @@ def test_available_evidence_keeps_unknown_dimensions_unknown(): + """Report freshness and retention as UNKNOWN, with no lag or retention values, and stamp both + timestamps in UTC.""" evidence = available_query_evidence() assert evidence.availability is SourceAvailability.AVAILABLE assert evidence.freshness.status is FreshnessStatus.UNKNOWN diff --git a/tests/test_stream.py b/tests/test_stream.py index 9b9979a..0842389 100644 --- a/tests/test_stream.py +++ b/tests/test_stream.py @@ -1,3 +1,10 @@ +"""Validate StreamReader with a mocked Redis client: reading new entries via XREAD, consumer-group +creation and reads, acknowledgement, and claim_stale's stale-entry reclaiming and poison-entry +detection. + +Developer: Manish Kumar +""" + import pytest from redis.exceptions import ResponseError @@ -8,6 +15,7 @@ # --------------------------------------------------------------------------- def test_stream_reader_calls_xread(stream_reader): + """Read entries through XREAD from the given last id.""" reader, mock_redis = stream_reader mock_redis.xread.return_value = [("audit:events", [("1-0", {"data": "{}"})])] @@ -20,6 +28,7 @@ def test_stream_reader_calls_xread(stream_reader): def test_stream_reader_default_last_id(stream_reader): + """Default the read to start from id 0-0 when none is given.""" reader, mock_redis = stream_reader mock_redis.xread.return_value = [] @@ -30,6 +39,7 @@ def test_stream_reader_default_last_id(stream_reader): def test_stream_reader_passes_custom_last_id(stream_reader): + """Pass a caller-supplied last id through to XREAD.""" reader, mock_redis = stream_reader mock_redis.xread.return_value = [] @@ -40,6 +50,7 @@ def test_stream_reader_passes_custom_last_id(stream_reader): def test_stream_reader_returns_empty_on_timeout(stream_reader): + """Return an empty list when XREAD times out with no new entries.""" reader, mock_redis = stream_reader mock_redis.xread.return_value = [] @@ -49,6 +60,7 @@ def test_stream_reader_returns_empty_on_timeout(stream_reader): def test_stream_reader_uses_config_stream_name(stream_reader): + """Read from the stream name configured in AuditConfig.""" reader, mock_redis = stream_reader mock_redis.xread.return_value = [] @@ -59,6 +71,7 @@ def test_stream_reader_uses_config_stream_name(stream_reader): def test_stream_reader_returns_multiple_entries(stream_reader): + """Return every entry XREAD reports for a stream.""" reader, mock_redis = stream_reader entries = [ ("0-1", {"data": '{"event_type": "auth_login"}'}), @@ -77,6 +90,7 @@ def test_stream_reader_returns_multiple_entries(stream_reader): # --------------------------------------------------------------------------- def test_ensure_group_creates_group(stream_reader): + """Create the consumer group with a single XGROUP CREATE call.""" reader, mock_redis = stream_reader reader.ensure_group() @@ -98,6 +112,7 @@ def test_ensure_group_is_idempotent_when_group_exists(stream_reader): def test_ensure_group_reraises_other_response_errors(stream_reader): + """Re-raise any ResponseError other than BUSYGROUP raised while creating the group.""" reader, mock_redis = stream_reader mock_redis.xgroup_create.side_effect = ResponseError("NOGROUP some other error") @@ -106,6 +121,7 @@ def test_ensure_group_reraises_other_response_errors(stream_reader): def test_read_group_calls_xreadgroup(stream_reader): + """Read new messages through XREADGROUP for the given consumer.""" reader, mock_redis = stream_reader mock_redis.xreadgroup.return_value = [] @@ -121,6 +137,7 @@ def test_read_group_calls_xreadgroup(stream_reader): def test_ack_calls_xack(stream_reader): + """Acknowledge a message through XACK.""" reader, mock_redis = stream_reader reader.ack("1-0") @@ -135,6 +152,8 @@ def test_ack_calls_xack(stream_reader): # --------------------------------------------------------------------------- def _pending_entry(message_id, times_delivered): + """Build an XPENDING-range-style pending entry dict with the given message id and delivery + count.""" return { "message_id": message_id, "consumer": "some-dead-consumer", @@ -144,6 +163,8 @@ def _pending_entry(message_id, times_delivered): def test_claim_stale_queries_xpending_range_with_config_defaults(stream_reader): + """Query XPENDING_RANGE with the configured default idle time and max count, returning nothing + claimed when there is no backlog.""" reader, mock_redis = stream_reader mock_redis.xpending_range.return_value = [] @@ -162,6 +183,7 @@ def test_claim_stale_queries_xpending_range_with_config_defaults(stream_reader): def test_claim_stale_returns_empty_when_nothing_stale(stream_reader): + """Return no claimed or poison entries, and never call XCLAIM or XACK, when nothing is stale.""" reader, mock_redis = stream_reader mock_redis.xpending_range.return_value = [] @@ -174,6 +196,7 @@ def test_claim_stale_returns_empty_when_nothing_stale(stream_reader): def test_claim_stale_reclaims_entries_under_max_deliveries(stream_reader): + """Reclaim a stale entry under the max-deliveries threshold via XCLAIM without acking it.""" reader, mock_redis = stream_reader mock_redis.xpending_range.return_value = [_pending_entry("5-0", times_delivered=2)] mock_redis.xclaim.return_value = [("5-0", {"data": "{}"})] @@ -232,6 +255,7 @@ def test_claim_stale_poison_entry_with_no_xrange_result_gets_empty_fields(stream def test_claim_stale_splits_a_mixed_batch_correctly(stream_reader): + """Split a mixed batch into reclaimed entries and poison entries by delivery count.""" reader, mock_redis = stream_reader mock_redis.xpending_range.return_value = [ _pending_entry("7-0", times_delivered=1), @@ -257,6 +281,7 @@ def test_claim_stale_splits_a_mixed_batch_correctly(stream_reader): def test_claim_stale_honors_explicit_overrides_over_config_defaults(stream_reader): + """Use caller-supplied idle time and count overrides in place of the config defaults.""" reader, mock_redis = stream_reader mock_redis.xpending_range.return_value = [] diff --git a/tests/test_worker.py b/tests/test_worker.py index f1f994c..7318885 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -4,7 +4,10 @@ PR2 additions (bottom of file): integrity_status classification -- signed valid/invalid events and unsigned (today's only real traffic shape) all -persist and ACK; only "invalid" gets the distinct observability print.""" +persist and ACK; only "invalid" gets the distinct observability print. + +Developer: Manish Kumar +""" import json import runpy from unittest.mock import MagicMock, patch @@ -16,6 +19,7 @@ def _raw(event_id="evt-1"): + """Build a valid JSON audit-event payload string for the given event id.""" return json.dumps({ "event_id": event_id, "timestamp": "2026-01-01T12:00:00", @@ -30,6 +34,7 @@ def _raw(event_id="evt-1"): # --------------------------------------------------------------------------- def test_handle_message_persists_and_acks(): + """Persist a valid message through the Sink and acknowledge it, closing the database session.""" reader = MagicMock() mock_sink_instance = MagicMock() mock_sink_instance.write.return_value = True @@ -49,6 +54,7 @@ def test_handle_message_persists_and_acks(): def test_handle_message_passes_full_event_payload_to_sink(): + """Hand the Sink the fully parsed event payload with its service and other fields intact.""" reader = MagicMock() mock_sink_instance = MagicMock() @@ -62,6 +68,7 @@ def test_handle_message_passes_full_event_payload_to_sink(): def test_handle_message_does_not_ack_on_parse_failure(): + """Leave a malformed message unacknowledged when it fails to parse.""" reader = MagicMock() result = worker.handle_message(reader, "1-0", {"data": "not-json"}) @@ -103,6 +110,7 @@ def test_handle_message_missing_data_field_with_other_fields_present_still_safe( def test_handle_message_does_not_ack_on_db_failure(): + """Leave a message unacknowledged and close the session when the database write fails.""" reader = MagicMock() mock_sink_instance = MagicMock() mock_sink_instance.write.side_effect = Exception("db connection lost") @@ -144,6 +152,7 @@ def test_handle_message_retry_after_failure_then_succeeds(): # --------------------------------------------------------------------------- def test_run_creates_consumer_group_on_startup(): + """Create the consumer group once when the worker loop starts.""" mock_reader = MagicMock() mock_reader.read_group.return_value = [] @@ -154,6 +163,7 @@ def test_run_creates_consumer_group_on_startup(): def test_run_processes_messages_from_read_group(): + """Route messages returned by read_group through handle_message.""" mock_reader = MagicMock() mock_reader.read_group.return_value = [ (worker.AuditConfig.STREAM_NAME, [("1-0", {"data": _raw("evt-a")})]), @@ -167,6 +177,7 @@ def test_run_processes_messages_from_read_group(): def test_run_stops_after_max_iterations(): + """Stop after max_iterations loops.""" mock_reader = MagicMock() mock_reader.read_group.return_value = [] @@ -286,6 +297,7 @@ def test_run_does_not_swallow_keyboard_interrupt(): # --------------------------------------------------------------------------- def test_dunder_main_starts_worker_and_exits_cleanly_on_keyboard_interrupt(capsys): + """Print the startup message and exit with status 0 on a keyboard interrupt.""" mock_reader = MagicMock() mock_reader.ensure_group.side_effect = KeyboardInterrupt @@ -326,6 +338,7 @@ def _handle_with_status(fields, secret=SECRET): def test_valid_signed_event_persists_as_valid_and_acks(): + """Persist a correctly signed event with integrity_status=valid and acknowledge it.""" raw = _raw(event_id="evt-valid") sig = sign_audit_event("auth", raw, SECRET) @@ -348,6 +361,8 @@ def test_unsigned_event_persists_as_unsigned_and_acks(): def test_invalid_signature_persists_as_invalid_and_acks(): + """Persist an event signed with the wrong secret as integrity_status=invalid and still + acknowledge it.""" raw = _raw(event_id="evt-invalid") sig = sign_audit_event("auth", raw, "a-different-secret-entirely") @@ -359,6 +374,8 @@ def test_invalid_signature_persists_as_invalid_and_acks(): def test_malformed_signature_persists_as_invalid_and_acks(): + """Persist an event with a malformed signature string as integrity_status=invalid and still + acknowledge it.""" raw = _raw(event_id="evt-malformed-sig") result, status, reader = _handle_with_status({"data": raw, "sig": "not-a-real-signature"}) @@ -369,6 +386,7 @@ def test_malformed_signature_persists_as_invalid_and_acks(): def test_invalid_signature_emits_distinct_observability_message(capsys): + """Print a SIGNATURE INVALID message naming the event id for an invalid signature.""" raw = _raw(event_id="evt-loud-invalid") sig = sign_audit_event("auth", raw, "wrong-secret") @@ -380,6 +398,7 @@ def test_invalid_signature_emits_distinct_observability_message(capsys): def test_valid_signature_does_not_emit_the_invalid_message(capsys): + """Print no SIGNATURE INVALID message for a validly signed event.""" raw = _raw(event_id="evt-quiet-valid") sig = sign_audit_event("auth", raw, SECRET) @@ -390,6 +409,7 @@ def test_valid_signature_does_not_emit_the_invalid_message(capsys): def test_unsigned_event_does_not_emit_the_invalid_message(capsys): + """Print no SIGNATURE INVALID message for an unsigned event.""" raw = _raw(event_id="evt-quiet-unsigned") _handle_with_status({"data": raw}) @@ -428,6 +448,8 @@ def test_malformed_json_still_does_not_ack_unchanged_behavior(): def test_db_failure_still_does_not_ack_unchanged_behavior(): + """Leave a message unacknowledged when the database write fails, unchanged by the + signature-checking behavior.""" reader = MagicMock() mock_sink_instance = MagicMock() mock_sink_instance.write.side_effect = Exception("db connection lost") diff --git a/tests/test_worker_integration_real_backends.py b/tests/test_worker_integration_real_backends.py index a17a8e8..8e76773 100644 --- a/tests/test_worker_integration_real_backends.py +++ b/tests/test_worker_integration_real_backends.py @@ -19,6 +19,8 @@ where real backends exist, not a hard CI requirement introduced by this PR -- see the B0 report for why that's a deliberate, separately-flagged follow-up rather than bundled into this change. + +Developer: Manish Kumar """ import json import os @@ -60,6 +62,8 @@ def _real_backends_available(): + """Report whether both the configured test-MySQL root URL and test-Redis URL are reachable, + returning False when either is unconfigured or unreachable.""" if TEST_MYSQL_ROOT_URL is None or TEST_REDIS_URL is None: return False try: @@ -214,6 +218,7 @@ def test_real_produce_consume_persist_ack_round_trip(real_redis_stream, real_mys def _integration_payload(event_id, service="b0-integration-test", **overrides): + """Build a valid JSON audit-event payload string for the given event id and service.""" from datetime import datetime, timezone payload = { @@ -290,6 +295,8 @@ def _run_real_round_trip(real_redis_stream, real_mysql_url, monkeypatch, event_i # --------------------------------------------------------------------------- def test_real_valid_signed_event_persists_as_valid(real_redis_stream, real_mysql_url, monkeypatch): + """Persist a correctly signed event with integrity_status=valid through a real XADD-to-worker + round trip.""" from audit.config import AuditConfig from audit.signing import sign_audit_event @@ -305,6 +312,8 @@ def test_real_valid_signed_event_persists_as_valid(real_redis_stream, real_mysql def test_real_unsigned_event_persists_as_unsigned(real_redis_stream, real_mysql_url, monkeypatch): + """Persist an unsigned event with integrity_status=unsigned through a real XADD-to-worker round + trip.""" event_id = f"pr2-unsigned-{uuid.uuid4()}" row = _run_real_round_trip( real_redis_stream, real_mysql_url, monkeypatch, @@ -315,6 +324,8 @@ def test_real_unsigned_event_persists_as_unsigned(real_redis_stream, real_mysql_ def test_real_invalid_signed_event_persists_as_invalid(real_redis_stream, real_mysql_url, monkeypatch): + """Persist an event signed with the wrong secret as integrity_status=invalid through a real + XADD-to-worker round trip.""" from audit.config import AuditConfig from audit.signing import sign_audit_event diff --git a/tests/test_worker_nogroup_recovery.py b/tests/test_worker_nogroup_recovery.py index fea8b9b..b9bb402 100644 --- a/tests/test_worker_nogroup_recovery.py +++ b/tests/test_worker_nogroup_recovery.py @@ -33,6 +33,8 @@ omnibioai_audit.audit_events table was found; the bug was in this test file, not any application code. Every test below patches StreamReader for exactly this reason -- do not remove it. + +Developer: Manish Kumar """ from unittest.mock import MagicMock, patch @@ -58,6 +60,7 @@ def _reset_recreate_ratelimit_and_alert_dedup(): def _nogroup_error(): + """Build a NOGROUP exception shaped like a real Redis consumer-group-missing error.""" return Exception("NOGROUP No such key 'audit:events' or consumer group 'audit-workers'") @@ -66,6 +69,8 @@ def _nogroup_error(): # --------------------------------------------------------------------------- def test_read_group_nogroup_emits_critical_alert(monkeypatch): + """Fire a critical audit_stream_or_group_missing alert on the audit-worker component when + read_group raises NOGROUP.""" monkeypatch.setattr(AuditConfig, "WORKER_AUTO_RECREATE_STREAM_ON_NOGROUP", False) reader = MagicMock() reader.read_group.side_effect = [_nogroup_error(), []] @@ -85,6 +90,7 @@ def test_read_group_nogroup_emits_critical_alert(monkeypatch): def test_sweep_pending_nogroup_also_emits_alert(): + """Fire the audit_stream_or_group_missing alert when claim_stale raises NOGROUP too.""" reader = MagicMock() reader.claim_stale.side_effect = _nogroup_error() @@ -119,6 +125,7 @@ def test_non_nogroup_error_does_not_trigger_nogroup_handling(monkeypatch): # --------------------------------------------------------------------------- def test_nogroup_on_read_group_applies_backoff(monkeypatch): + """Sleep for the configured backoff duration after a NOGROUP error on read_group.""" monkeypatch.setattr(AuditConfig, "WORKER_AUTO_RECREATE_STREAM_ON_NOGROUP", False) monkeypatch.setattr(AuditConfig, "WORKER_NOGROUP_RETRY_BACKOFF_SECONDS", 2.0) reader = MagicMock() @@ -156,6 +163,7 @@ def test_idle_timeout_path_never_sleeps_the_nogroup_backoff(): # --------------------------------------------------------------------------- def test_recreation_disabled_by_default_does_not_call_ensure_group_again(monkeypatch): + """Call ensure_group only once, at startup, when auto-recreation is disabled.""" monkeypatch.setattr(AuditConfig, "WORKER_AUTO_RECREATE_STREAM_ON_NOGROUP", False) reader = MagicMock() reader.read_group.side_effect = [_nogroup_error(), []] @@ -169,6 +177,7 @@ def test_recreation_disabled_by_default_does_not_call_ensure_group_again(monkeyp def test_recreation_enabled_attempts_ensure_group_on_nogroup(monkeypatch): + """Call ensure_group a second time and fire a recovered alert when auto-recreation is enabled.""" monkeypatch.setattr(AuditConfig, "WORKER_AUTO_RECREATE_STREAM_ON_NOGROUP", True) reader = MagicMock() reader.read_group.side_effect = [_nogroup_error(), []] @@ -227,6 +236,7 @@ def test_recreation_attempt_is_rate_limited_not_every_failure(monkeypatch): def test_recreate_failure_does_not_crash_worker(monkeypatch): + """Keep the worker running instead of crashing when the recreate attempt itself fails.""" monkeypatch.setattr(AuditConfig, "WORKER_AUTO_RECREATE_STREAM_ON_NOGROUP", True) reader = MagicMock() reader.read_group.side_effect = [_nogroup_error(), []] diff --git a/tests/test_worker_pel_recovery.py b/tests/test_worker_pel_recovery.py index ef2d2bd..73fa0da 100644 --- a/tests/test_worker_pel_recovery.py +++ b/tests/test_worker_pel_recovery.py @@ -9,6 +9,8 @@ failure) was previously invisible to every future read_group() call forever -- read_group() only ever asks Redis for ">" (strictly new messages). sweep_pending() is what makes such an entry reachable again. + +Developer: Manish Kumar """ import json from unittest.mock import MagicMock, patch @@ -18,6 +20,7 @@ def _raw(event_id="evt-1"): + """Build a valid JSON audit-event payload string for the given event id.""" return json.dumps({ "event_id": event_id, "timestamp": "2026-01-01T12:00:00", @@ -32,6 +35,7 @@ def _raw(event_id="evt-1"): # --------------------------------------------------------------------------- def test_sweep_pending_processes_each_reclaimed_message(): + """Route every reclaimed message through handle_message.""" reader = MagicMock() reader.claim_stale.return_value = ( [("2-0", {"data": _raw("evt-reclaimed-a")}), ("2-1", {"data": _raw("evt-reclaimed-b")})], @@ -202,6 +206,7 @@ def test_sweep_pending_survives_claim_stale_raising(capsys): def test_sweep_pending_no_op_when_nothing_stale(): + """Call neither handle_message nor ack when claim_stale reclaims nothing.""" reader = MagicMock() reader.claim_stale.return_value = ([], []) @@ -219,6 +224,7 @@ def test_sweep_pending_no_op_when_nothing_stale(): # --------------------------------------------------------------------------- def test_run_calls_sweep_pending_every_iteration(): + """Call claim_stale once per run iteration.""" mock_reader = MagicMock() mock_reader.read_group.return_value = [] mock_reader.claim_stale.return_value = ([], []) @@ -230,6 +236,7 @@ def test_run_calls_sweep_pending_every_iteration(): def test_run_still_processes_new_messages_when_sweep_finds_nothing(): + """Process newly read messages even when the pending sweep finds nothing stale.""" mock_reader = MagicMock() mock_reader.claim_stale.return_value = ([], []) mock_reader.read_group.return_value = [ @@ -244,6 +251,7 @@ def test_run_still_processes_new_messages_when_sweep_finds_nothing(): def test_run_processes_both_reclaimed_and_new_messages_in_one_iteration(): + """Process both a reclaimed message and a newly read message within one iteration.""" mock_reader = MagicMock() mock_reader.claim_stale.return_value = ([("2-0", {"data": _raw("evt-reclaimed")})], []) mock_reader.read_group.return_value = [ @@ -260,6 +268,7 @@ def test_run_processes_both_reclaimed_and_new_messages_in_one_iteration(): def test_run_survives_sweep_pending_raising_and_still_reads_new_messages(): + """Keep reading new messages in the same iteration after sweep_pending raises.""" mock_reader = MagicMock() mock_reader.claim_stale.side_effect = Exception("redis blip during sweep") mock_reader.read_group.return_value = [ diff --git a/tests/test_worker_pel_recovery_integration.py b/tests/test_worker_pel_recovery_integration.py index dc0ec09..062ffb6 100644 --- a/tests/test_worker_pel_recovery_integration.py +++ b/tests/test_worker_pel_recovery_integration.py @@ -15,6 +15,8 @@ Every scenario here uses small min_idle_ms/max_deliveries overrides (never the real 30s/5-attempt production defaults, see audit/config.py) so this file runs in well under a second, not tens of seconds. + +Developer: Manish Kumar """ import json import os @@ -60,6 +62,8 @@ def _real_backends_available(): + """Report whether both the configured test-MySQL root URL and test-Redis URL are reachable, + returning False when either is unconfigured or unreachable.""" if TEST_MYSQL_ROOT_URL is None or TEST_REDIS_URL is None: return False try: @@ -87,6 +91,8 @@ def _real_backends_available(): @pytest.fixture def real_redis_stream(): + """Point the stream reader at an isolated test stream and consumer group, then destroy the group + and delete the stream on teardown -- never touches the production audit:events stream.""" from audit.config import AuditConfig from consumers.stream_reader import StreamReader @@ -113,6 +119,8 @@ def real_redis_stream(): @pytest.fixture def real_mysql_url(): + """Create a throwaway database, run the real Alembic migration against it, yield its URL, then + drop it.""" root_engine = create_engine(TEST_MYSQL_ROOT_URL) with root_engine.connect() as conn: conn.execute(text(f"DROP DATABASE IF EXISTS {TEST_DB_NAME}")) @@ -141,6 +149,7 @@ def real_mysql_url(): def _payload(event_id, **overrides): + """Build a valid JSON audit-event payload string for the given event id.""" payload = { "event_id": event_id, "timestamp": datetime.now(timezone.utc).isoformat(), @@ -159,6 +168,7 @@ def _payload(event_id, **overrides): def _session_local(real_mysql_url): + """Build a SQLAlchemy sessionmaker bound to the throwaway test-MySQL database.""" from sqlalchemy.orm import sessionmaker engine = create_engine(real_mysql_url) @@ -178,6 +188,8 @@ def _session_local(real_mysql_url): def test_real_crash_before_ack_is_reclaimed_by_a_second_worker_and_persisted( real_redis_stream, real_mysql_url, monkeypatch, ): + """Reclaim a message left pending by a crashed worker, then persist it once a second worker + processes it, on real Redis and MySQL.""" import worker.main as worker_module TestSessionLocal = _session_local(real_mysql_url) @@ -228,6 +240,7 @@ def test_real_crash_before_ack_is_reclaimed_by_a_second_worker_and_persisted( def test_real_concurrent_workers_racing_for_the_same_entry_only_one_wins( real_redis_stream, real_mysql_url, ): + """Let only one of two concurrently reclaiming workers actually claim a given pending entry.""" event_id = f"p0-race-{uuid.uuid4()}" real_redis_stream.redis.xadd(TEST_STREAM, {"data": json.dumps(_payload(event_id))}) @@ -267,6 +280,8 @@ def test_real_concurrent_workers_racing_for_the_same_entry_only_one_wins( def test_real_transient_persistence_failure_then_reclaim_succeeds( real_redis_stream, real_mysql_url, monkeypatch, ): + """Leave a message pending after a transient persistence failure, then persist it successfully + once reclaimed.""" import worker.main as worker_module from consumers.sink import Sink @@ -324,6 +339,7 @@ def write(self, event): def test_real_duplicate_delivery_after_reclaim_does_not_duplicate_row( real_redis_stream, real_mysql_url, monkeypatch, ): + """Persist exactly one row when a message is redelivered after being reclaimed.""" import worker.main as worker_module TestSessionLocal = _session_local(real_mysql_url) @@ -384,6 +400,8 @@ def test_real_duplicate_delivery_after_reclaim_does_not_duplicate_row( def test_real_malformed_event_is_abandoned_after_max_deliveries_not_retried_forever( real_redis_stream, real_mysql_url, monkeypatch, ): + """Quarantine a malformed event once it reaches the maximum delivery count, rather than retrying + it forever.""" import worker.main as worker_module TestSessionLocal = _session_local(real_mysql_url) diff --git a/tests/test_worker_quarantine_integration.py b/tests/test_worker_quarantine_integration.py index 133588e..e374ef4 100644 --- a/tests/test_worker_quarantine_integration.py +++ b/tests/test_worker_quarantine_integration.py @@ -9,6 +9,8 @@ event that simply never reached MySQL in time), quarantine-write failure NOT acking the original, and quarantine-write idempotency across a crash-after-quarantine-before-ack gap. + +Developer: Manish Kumar """ import json import os @@ -53,6 +55,8 @@ def _real_backends_available(): + """Report whether both the configured test-MySQL root URL and test-Redis URL are reachable, + returning False when either is unconfigured or unreachable.""" if TEST_MYSQL_ROOT_URL is None or TEST_REDIS_URL is None: return False try: @@ -78,6 +82,8 @@ def _real_backends_available(): @pytest.fixture def real_redis_stream(): + """Point the stream reader at an isolated test stream and consumer group, then destroy the group + and delete the stream on teardown -- never touches the production audit:events stream.""" from audit.config import AuditConfig from consumers.stream_reader import StreamReader @@ -104,6 +110,8 @@ def real_redis_stream(): @pytest.fixture def real_mysql_url(): + """Create a throwaway database, run the real Alembic migration against it, yield its URL, then + drop it.""" root_engine = create_engine(TEST_MYSQL_ROOT_URL) with root_engine.connect() as conn: conn.execute(text(f"DROP DATABASE IF EXISTS {TEST_DB_NAME}")) @@ -132,6 +140,7 @@ def real_mysql_url(): def _session_local(real_mysql_url): + """Build a SQLAlchemy sessionmaker bound to the throwaway test-MySQL database.""" from sqlalchemy.orm import sessionmaker engine = create_engine(real_mysql_url) @@ -139,6 +148,7 @@ def _session_local(real_mysql_url): def _valid_payload(event_id): + """Build a valid JSON audit-event payload string for the given event id.""" return json.dumps({ "event_id": event_id, "timestamp": datetime.now(timezone.utc).isoformat(), @@ -164,6 +174,8 @@ def _valid_payload(event_id): def test_real_persistence_exhausted_event_is_quarantined_with_recoverable_payload( real_redis_stream, real_mysql_url, monkeypatch, ): + """Quarantine an event whose persistence keeps failing until max deliveries, with the + quarantined row carrying a recoverable payload.""" import worker.main as worker_module from consumers.sink import Sink @@ -233,6 +245,7 @@ def write(self, event): def test_real_quarantine_write_failure_does_not_ack_original( real_redis_stream, real_mysql_url, monkeypatch, ): + """Leave the original message unacknowledged when the quarantine write itself fails.""" import worker.main as worker_module from consumers.quarantine import QuarantineSink @@ -283,6 +296,8 @@ def write(self, message_id, fields, delivery_attempts): def test_real_requarantine_after_crash_before_ack_is_idempotent( real_redis_stream, real_mysql_url, monkeypatch, ): + """Quarantine a poison message exactly once even when it is reclaimed and requarantined after a + crash before ack.""" import worker.main as worker_module from consumers.quarantine import QuarantineSink @@ -368,6 +383,7 @@ def test_real_requarantine_after_crash_before_ack_is_idempotent( def test_real_missing_data_field_is_quarantined_not_crashed( real_redis_stream, real_mysql_url, monkeypatch, ): + """Quarantine a message that carries no data field instead of crashing the worker.""" import worker.main as worker_module TestSessionLocal = _session_local(real_mysql_url)