feat(audit): configurable per-action modes for allowed read decisions

[davlaw]

Implements the direction from #604 / #605: let an administrator choose which authorization audit entries are required, and cut the volume of the noisy ones.

Problem

audit_log_authorization() writes a durable audit_events row for every authorization decision, allowed ones included. On the install from #604, two UI polling paths dominate:

Source	Rows
GET /api/detection/results/<stream> → live.view allowed	~400k/day (747,760 in one retained day)
GET /api/system/logs → system.admin allowed	log viewer polling, ~12/min per open tab

Pruning (#605) bounds the size, but the write rate is what defeats SQLite's online backup: it restarts from page one whenever another connection writes, and 47 consecutive scheduled backups failed.

Change

Each authorization action gets a mode for allowed decisions on GET/HEAD requests:

Mode	Effect
record (default)	One event per decision, exactly as today
summarize	Normally one event per user, action, target and client address per 15-minute window, with a count
off	No event

Always recorded regardless of mode: denied and error decisions, any allowed decision on a non-GET/HEAD request, and everything written through audit_log_operation() / audit_log_append() (sign-ins, operation outcomes, retention and mode changes). "Changes state" is decided by HTTP method rather than the catalog's destructive flag, because system.admin covers both log-viewer polling (GET /api/system/logs) and restarts (POST /api/system/restart).

Defaults reproduce current behavior exactly: no setting row, an unknown action or an invalid value in storage all mean record, so no migration is needed and a downgrade simply ignores the key.

Settings and API

• Stored as one system_settings row, audit_allowed_decision_modes, holding only non-default entries, e.g. {"live.view":"summarize"}. Loaded into an atomic per-action table at startup; the request path reads it without a lock or a query.
• GET /api/audit/settings adds summary_window_seconds and allowed_decision_modes (every catalog action, in catalog order, with category and description).
• PUT /api/audit/settings accepts retention_days, allowed_decision_modes, or both. The whole body is validated before anything is saved; an unknown action or invalid mode returns 400 and persists nothing. Changing modes flushes pending summaries, persists, swaps the in-memory table, and records audit.settings.update with the previous and new mode of each changed action. PUTs are serialized.
• GET /api/audit/events and the CSV export accept event_type (matches details.event_type).
• docs/API.md documents all of the above.

Summaries

• A fixed 1,024-slot open-addressing table allocated once (src/web/audit_summary.c); no allocation per request, its own mutex, never the global DB mutex. Keys have explicit padding plus a _Static_assert that the struct has no implicit gaps, since hashing and equality are byte-wise.
• Windows are wall-clock aligned (now - now % 900). Entries are flushed when their window closes (main loop, ~60 s), on a mode change, when the table is full, and at shutdown in both cleanup paths (after http_server_stop(), before shutdown_database() and its final backup).
• A flushed entry is an ordinary audit_events row: outcome allowed, occurred_at = first seen, request_id of the first request, and details
  {"event_type":"authorization.summary","count":…,"first_at":…,"last_at":…,"window_seconds":900,"method":…,"path":…,"decision_source":…,"explanation":…},
  passed through the same redaction as audit_log_append(). Retention, pruning, filtering, export and the history viewer work unchanged.
• Flushing copies entries out under the summary mutex and writes after releasing it. If a summary row cannot be written it is logged with action, target and count and dropped (no retry, so memory cannot grow while the DB is unhealthy). If the table is still full after a flush, or failed to allocate, the decision is recorded individually — no count is lost.
• Accepted loss: a crash or SIGKILL loses up to one window of summary counts. Always-recorded events are never buffered.

UI

Users → Security & Access Audit gains an Allowed decisions panel next to Retention (AuditDecisionModes.jsx): actions grouped by category, a Record each / Summarize / Off selector per action, an inline warning for Off, and one Save button that sends only changed actions. Summary rows in the event list show a ×N badge and the first-to-last time range, and the event-type filter gains Summaries / Individual decisions. Choosing an event type with no start date prefills the last 24 hours (see below). New strings are in en.json only.

Known trade-offs (called out deliberately)

• event_type filtering scans event details. No index covers json_extract(details_json,'$.event_type'). On a copy of a 790,741-row table: count 0.63 s + page 0.87 s under the DB mutex, versus 0.007 s with a 24 h since. The UI therefore prefills since, and the docs recommend a time range. An expression index would fix API callers too, but needs a migration — happy to add one if you want it; I didn't want to take a migration number unasked.
• A full table flushes on the requesting thread, so that one request waits on database writes (1,024 distinct user×action×target×client keys in one window). Other threads only wait on short lock holds.
• Player telemetry stays individually recorded. POST /api/telemetry/player is authorized as live.view per stream, so the non-GET rule records it (~26 rows/min with 7 cameras open). It doesn't change state, so it could reasonably be treated as a read or not produce authorization rows at all — left as a follow-up question rather than special-cased here.
• If the startup load of modes fails (unreadable DB), memory falls back to record, and the next PUT persists from that state. Saving retention in the UI resets unsaved mode edits.

Testing

• test_audit_summary (8): wall-clock window alignment, accumulation keeping the first sample, distinct clients as separate entries, full table rejecting new keys, draining closed windows while keeping the current one, output capacity, uninitialized table, and concurrent threads incrementing one key with an exact final count.
• test_audit_log (38 total, 23 new): mode persistence (defaults, round trip storing only non-defaults, invalid stored entries ignored, reload into memory); record/off/summarize behavior; distinct camera and client produce separate rows while a different path does not; allowed mutating requests, denials and errors never summarized; HEAD summarized like GET; every summary detail field including a >256-char path; closed-window, mode-change and full-table flushes; backward clock step; failed write dropped without crashing; settings GET/PUT including atomic validation, audit.settings.update details, no event for unchanged modes, and a combined retention+modes PUT; event_type filter through both the DB and HTTP layers. The shutdown flush is exercised in production below rather than by a unit test (main() has no harness).
• Key tests were checked in the failing direction against a targeted mutation (e.g. removing the event_type bind, the mode load loop, the path width).
• Jest: auditHistory.spec.js covers grouping, changed-mode computation, summary detection/formatting and the since prefill.
• test_audit_log and test_audit_summary are added to the ASan+UBSan CI job's ctest -R list. Locally both pass under ASan+UBSan and under ThreadSanitizer with zero warnings (local-only evidence; CI has no TSan job). Local sanitizer builds used -DENABLE_LITERT=OFF because third-party XNNPACK does not compile under GCC 14.2 in those configurations — unrelated to this change.

Production verification

Deployed on the #604 install (0.41.18 + these commits, replacing the local suppression patch I had been carrying), with live.view and system.admin set to Summarize:

• Per-request allowed audit writes: 545/min → ~26/min (the remainder is the telemetry POST above).
• First full window (14:00–14:15): 12 live.view summary rows covering 7,332 decisions (4–1,824 each) plus one system.admin row.
• Scheduled backup completed in 17 s; no failures and no summary persistence errors.
• Restart: Flushed 7 audit summaries at shutdown, rows present with last_at at the restart second; modes persisted.
• UI: badges, time ranges, Summaries filter and the Off warning confirmed in the browser.

Designed and implemented with Claude Code (Claude Opus 5) as co-author — design spec, task-by-task implementation with per-task review, and a whole-branch review before this PR.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Six unresolved findings remain, including two critical lifecycle and database-consistency issues.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds configurable record, summarize, and off modes for allowed authorization audit decisions, with buffered summaries and supporting API/UI updates.

Changes:

• Adds fixed-capacity, windowed audit summarization and lifecycle flushing.
• Extends settings, event filtering, UI, documentation, and localization.
• Adds unit/UI tests and sanitizer CI coverage.

File summaries

File	Reviewed changes	Final review notes
web/tests/auditHistory.spec.js	Tests summary display and filtering helpers.	No final review comment.
web/public/locales/en.json	Adds audit interface strings.	No final review comment.
web/js/components/preact/users/AuditHistoryModal.jsx	Integrates summary rows and event-type filtering.	No final review comment.
web/js/components/preact/users/auditHistory.js	Provides audit history formatting and mode helpers.	No final review comment.
web/js/components/preact/users/AuditDecisionModes.jsx	Adds per-action mode controls.	No final review comment.
tests/unit/test_audit_summary.c	Tests summary accumulation and draining.	No final review comment.
tests/unit/test_audit_log.c	Tests modes, persistence, flushing, and filtering.	No final review comment.
tests/unit/CMakeLists.txt	Registers audit tests.	No final review comment.
src/web/audit_summary.c	Implements the fixed-capacity summary accumulator.	No final review comment.
src/web/audit_log.c	Applies modes and persists summaries.	Critical (1 vote, line 164): database switches can leave modes and pending summaries associated with the old database. Moderate (1 vote, line 264): mode updates are not synchronized with summary insertion. Moderate (1 vote, line 253): draining can race with producers and duplicate rows. Moderate (1 vote, line 235): serialization failures can produce empty details instead of dropping the summary.
src/web/api_handlers_audit.c	Implements audit settings and event filtering APIs.	No final review comment.
src/database/db_audit.c	Persists modes and filters event types.	No final review comment.
src/core/main.c	Loads and flushes summaries during application lifecycle.	Critical (1 vote, line 1707): shutdown flushing can race with in-flight request handlers. Moderate (2 votes, line 1366): backward wall-clock adjustments can indefinitely delay periodic flushing.
include/web/audit_summary.h	Declares summary interfaces.	No final review comment.
include/web/audit_log.h	Declares audit mode and flush interfaces.	No final review comment.
include/database/db_audit.h	Defines audit modes and query fields.	No final review comment.
docs/API.md	Documents audit settings and filtering APIs.	No final review comment.
.github/workflows/sanitizer.yml	Adds audit tests to sanitizer coverage.	No final review comment.

Review details

Suppressed comments (3)

src/web/audit_log.c:267

• The flush and the per-action atomic stores are not synchronized with audit_log_authorization(). A request can read summarize, be preempted here, then add a summary entry after this flush while the new mode is off or record; that entry will later be persisted as a summary, so the mode change is not a reliable boundary. Synchronize the mode read plus summary insertion with the update, or add a generation/recheck scheme that handles entries created under the old mode.

    audit_log_flush_summaries(false);
    if (db_audit_save_decision_modes(modes) != 0) return -1;
    for (int i = 0; i < AUTHZ_ACTION_COUNT; i++) {
        atomic_store(&decision_modes[i], (int)modes[i]);

src/web/audit_log.c:255

• audit_summary_drain() removes these entries before the database writes below and releases the summary mutex. A request that adds the same key while this batch is being written can therefore create a second entry for the same window; when fewer than 8 entries were drained, the loop exits and emits two rows instead of one aggregated row. Keep in-flight keys mergeable until their write completes, or otherwise coordinate producers with the drain.

        size_t drained = audit_summary_drain(!closed_windows_only, current_window,
                                             batch, AUDIT_SUMMARY_FLUSH_BATCH);
        for (size_t i = 0; i < drained; i++) write_summary_row(&batch[i]);

src/web/audit_log.c:238

• The fallback still appends {} when summary detail construction, redaction, or serialization failed. That creates an allowed audit row with no event_type, count, or time range, so the aggregated decisions are no longer identifiable or filterable; the documented failure behavior is to log and drop an unwritable summary. Treat a missing serialized summary as a failed write instead of inserting an empty-details row.

        .details_json = serialized ? serialized : "{}",
        .occurred_at = entry->first_at,
    };
    if (db_audit_append(&input, NULL) != 0) {

• Files reviewed: 18/18 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[davlaw]

Fixed in 44ff0a0: flush pending summaries before shutdown_database(), and call audit_log_decision_modes_init() (which reloads modes and reallocates a fresh summary table) once the live database is known, on both the success and the revert-to-old-path branches. This mirrors the ordering already used in both of main.c's shutdown paths. No unit test covers it — this handler (and DB-path switching generally) has no test harness in this codebase (tests/unit/test_api_handlers_settings.c doesn't exist), so I verified it by inspection and by reasoning from the same pattern main.c already uses correctly.

[davlaw]

Pushed 44ff0a0 addressing two of Copilot's findings (replied inline on each thread):

• The empty-{}-row fallback in write_summary_row() on a serialization failure now logs and drops the write instead, matching the documented "log and drop an unwritable summary" contract.
• handle_post_settings()'s runtime database-path switch now flushes pending summaries before closing the old database and reloads decision modes (reallocating a fresh summary table) once the live database is known, on both the success and revert paths — mirroring the ordering main.c's two shutdown paths already use.

Replied with rationale on the other three inline findings rather than changing code — the shutdown-race and backward-clock ones are both bounded/self-healing and match trade-offs the design already documents or patterns already used elsewhere in main.c's loop, so a special-case fix here felt like the wrong place for it.

The two "suppressed" findings in the review body (audit_log.c:267 mode-change race, audit_log.c:255 drain race producing two rows for one window) are the same known trade-off already called out in docs/API.md's decision-mode table ("normally one event per window") — a request landing in the narrow gap around a flush can be summarized under the old mode or split across two rows, but nothing is lost or corrupted, and the count on every row it does produce stays correct.

[Copilot]

🔵 Needs a closer look

Unresolved moderate findings affect mode-update atomicity and summary correctness; the API documentation nit also remains.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/web/api_handlers_audit.c:440

• If cJSON_CreateObject() fails for a changed action, this continue can leave changes empty or incomplete. The handler then skips audit_log_set_decision_modes() (or persists only a subset) and can return 200 even though the validated request did not apply all requested modes. Abort with a 500 before persisting any mode changes, just as the surrounding code does when the changes array itself cannot be allocated.
  src/web/audit_log.c:278
• The mode transition is not synchronized with summary accumulation: a request can read the old summarize mode, block while this flush runs, and then add its entry after the flush (even after the new off mode is published). That stale entry remains pending and can be written later, so a mode change is not a complete flush and off can still produce a summary after the update. Serialize the mode read/add operation with the transition or attach a generation and discard entries from the old mode.

docs/API.md:370

• event_type is documented here, but the authoritative optional-query-parameter table above (lines 288–295) still omits it. Add this filter to that table so API consumers do not have to discover a supported parameter only in the later performance note.

`GET /api/audit/events` and its CSV export also accept `event_type`, matching
`details.event_type` exactly (for example `authorization.summary`). This

src/web/audit_log.c:306

• These credential fields participate in byte-wise key equality, so the same user/action/target/client/window is split into separate rows when authentication changes or when the user uses different scoped tokens. This contradicts the documented summarize contract of one event per user/action/target/client and can materially increase the write volume; keep credential metadata as first-sample fields rather than key dimensions, or explicitly define the grouping as credential-level.

    safe_strcpy(key.auth_method,
                user && user->authentication_method[0] ? user->authentication_method : "unauthenticated",
                sizeof(key.auth_method), 0);
    if (user && user->authenticated_via_scoped_token) {
        safe_strcpy(key.api_token_uuid, user->api_token_uuid, sizeof(key.api_token_uuid), 0);

• Files reviewed: 19/19 changed files
• Comments generated: 0 new
• Review effort level: Lite