Expand audit-log coverage for sensitive reads and security events#78
Merged
Conversation
audit_logs is an Article 286(5) compliance record, but coverage had gaps in sensitive read access, security-enforcement events, and consistency. Sensitive read access (compliance: who viewed/exported applicant PII): - Log APPLICANT_PII_VIEWED when a detail endpoint decrypts an applicant's national ID / mints signed document URLs — declarations/[id], legal/verifications/[id], legal/form-reissues/[id], admin/users/[id]. Self-access by the owning applicant and paginated bulk lists are deliberately not logged to avoid flooding the trail. - Log DECLARATION_EXPORTED on the analytics export (actor, format, row count, filters; no PII). - Log AUDIT_LOG_VIEWED on each admin query of the audit trail itself. - Log FILE_DOWNLOADED on the stateless signed-file stream, keyed by a coarse document type rather than the full bucket key. Security-enforcement events (previously unaudited): emit RATE_LIMIT_EXCEEDED, AI_AGENT_BLOCKED, AI_AGENT_SPOOFED and AI_ROBOTS_VIOLATION from the security and per-user rate-limit middleware. A new createAuditLogOnce helper dedupes these via the analytics KV (short TTL) so a flood collapses to ~one row per window instead of thousands; it fails open to logging when the KV is down. Consistency: route handlers that passed raw action strings now use the AuditActions enum, fixing two value mismatches that broke filtering (INSTITUTION_CREATE/UPDATE, USER_ACTIVATE/DEACTIVATE) plus ROLE_ASSIGN, office_assign, user_invited, RECEIPT_GENERATED and the code-verification actions. The two legacy uppercase CODE_* values are normalized to snake_case (historic rows still match the case-insensitive viewer filter). Mutating gaps: log NOTIFICATION_PREFERENCES_UPDATED (silencing alert channels is security-relevant) and CONTACT_SUBMITTED. Adds a unit test for createAuditLogOnce dedup/fail-open behaviour and the enum normalization. https://claude.ai/code/session_01AkeB4tcxvaVus2MPoY636a
Audit logging already runs as background BullMQ jobs in production (the audit-logs queue auto-enables when REDIS_URL is set), but createAuditLog was still awaited on the request path, so every audit event added a Redis round-trip (or, with no Redis, a Postgres insert) before the HTTP response was sent. The recent audit-coverage work added several such awaits to hot GET endpoints and two middleware hot paths. Detach all audit logging from the response path using the existing runAfterResponse helper (already used for notifications; drained on graceful shutdown via plugins/after-response-drain.ts): - Add logAudit(event, data) and logAuditOnce(event, data, dedupe) wrappers in server/utils/audit.ts that invoke the awaitable primitives synchronously (so IP/UA/occurredAt are captured at call time) then detach the enqueue/write so the handler returns immediately. - Repoint the logAction convenience wrapper to detach via logAudit, so its ~50 call sites inherit the behaviour with no edits. - Migrate the direct createAuditLog call sites (auth, profile, uploads, admin, declarations, legal, analytics export, read endpoints, SMS webhooks, traffic/ abuse/fuzzing background scorers) to logAudit, and the two rate-limit/security middleware from createAuditLogOnce to logAuditOnce. The awaitable primitives createAuditLog/createAuditLogOnce are unchanged so the BullMQ worker fallback and the tests that mock them keep working. Durability is preserved within the usual best-effort contract: metadata is captured at request time, in-flight work drains on graceful shutdown, and the write is durable once enqueued; only a hard crash in the sub-millisecond window between response-flush and enqueue can drop an event (the inline path was already best-effort). Tests: repoint the two notification tests that assert the audit call from createAuditLog to logAudit (identical args); add logAudit to the notifications-non-blocking audit mock; add coverage asserting logAudit/ logAuditOnce return void and still perform the background write. https://claude.ai/code/session_01AkeB4tcxvaVus2MPoY636a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
audit_logsis an Article 286(5) compliance record, not a debug aid. A coverage review found gaps in three areas: sensitive read access (PII decryption, exports, viewing the audit trail), security-enforcement events (middleware blocks), and consistency — several handlers passed raw action strings instead of theAuditActionsenum, two of which (INSTITUTION_*,USER_ACTIVATE/DEACTIVATE) were outright value mismatches that broke the admin filter.No DB migration:
AuditLog.actionis a free-form column, so new actions are code-only.What changed
Sensitive read access (compliance: who viewed/exported applicant PII)
APPLICANT_PII_VIEWEDwhen a detail endpoint decrypts a national ID / mints signed document URLs —declarations/[id],legal/verifications/[id],legal/form-reissues/[id],admin/users/[id]. Owner self-views and paginated bulk lists are deliberately not logged (they fire on every page paint and would flood the trail); detail views are the auditable access point.DECLARATION_EXPORTEDon the analytics export (actor, format, row count, filters — no PII).AUDIT_LOG_VIEWEDon each admin query of the audit trail itself.FILE_DOWNLOADEDon the stateless signed-file stream, keyed by a coarse document type rather than the full bucket key.Security-enforcement events (previously unaudited) —
RATE_LIMIT_EXCEEDED,AI_AGENT_BLOCKED,AI_AGENT_SPOOFED,AI_ROBOTS_VIOLATIONemitted from00.security.tsandrate-limit-user.ts. A newcreateAuditLogOncehelper dedupes these through the analytics KV (short TTL) so a flood collapses to ~one row per window instead of thousands, and fails open to logging when the KV is down.Consistency — 8 raw-string call sites converted to the
AuditActionsenum, fixing the two value mismatches plusROLE_ASSIGN,office_assign,user_invited,RECEIPT_GENERATEDand the code-verification actions. The two legacy uppercaseCODE_*values are normalized to snake_case (historic rows still match the case-insensitive viewer filter).Mutating gaps —
NOTIFICATION_PREFERENCES_UPDATED(silencing alert channels is security-relevant) andCONTACT_SUBMITTED.Testing
npm run lintclean; touched files typecheck clean.test/audit-dedup.test.tscoveringcreateAuditLogOncededup, per-key/per-action isolation, fail-open behaviour, and the enum normalization.https://claude.ai/code/session_01AkeB4tcxvaVus2MPoY636a
Generated by Claude Code