These workplans exist only in the retired local hub. Their random pre-ADR-007 identifiers are refused by C-06 as stale references, so they cannot be registered. Deriving from the canonical record id takes no identity from anything: central does not hold them and the old ids die with the cache. Records central already holds were deliberately left untouched. Refs CUST-WP-0068-T06 Assistant: claude-code Assistant-Model: opus Assistant-Process: 2583210@bnt-lap001 Assistant-Session: f2bff2d5-e9b2-4338-92ca-10282a927006
303 lines
13 KiB
Markdown
303 lines
13 KiB
Markdown
---
|
|
id: AUDIT-WP-0004
|
|
type: workplan
|
|
title: "Make the event receiver correct and operable under load"
|
|
domain: infotech
|
|
repo: audit-core
|
|
status: finished
|
|
owner: codex
|
|
topic_slug: netkingdom
|
|
created: "2026-08-10"
|
|
updated: "2026-08-10"
|
|
state_hub_workstream_id: "7fcc4bbc-af6e-599c-be5c-a3930aea7773"
|
|
---
|
|
|
|
# AUDIT-WP-0004 - receiver correctness and hardening
|
|
|
|
## Goal
|
|
|
|
Close the gap between what the receiver claims and what it does, and make it
|
|
survive a real sender. This is the work that must land before audit-core is
|
|
worth deploying.
|
|
|
|
A pre-deploy review of the WP-0003 implementation found that the ingestion
|
|
path never calls the `AuditBackend` contract, that several failure modes
|
|
escape the error handler and drop the connection, and that two isolation
|
|
properties the workplan claims as done are not implemented. None of these
|
|
depend on the choice of storage engine, so this workplan runs in parallel
|
|
with RAPP-POSTGRES-WP-0002 and gates AUDIT-WP-0005.
|
|
|
|
## Boundaries
|
|
|
|
This workplan owns the receiver's correctness, error semantics, and operable
|
|
surface. It does not own deployment, the production storage engine, or the
|
|
live failure matrix — those are AUDIT-WP-0005.
|
|
|
|
Work here stays storage-agnostic: the existing SQLite store remains the
|
|
development and test backend, and the Postgres implementation lands in
|
|
WP-0005 against the interface this workplan fixes.
|
|
|
|
## T01 - Route ingestion through the audit backend contract
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T01
|
|
status: done
|
|
priority: high
|
|
state_hub_task_id: "86474da6-6702-5ab7-97ac-8ff8b4683226"
|
|
```
|
|
|
|
`ingestion.py` writes records into SQLite directly and never calls
|
|
`AuditBackend.emit()`. WP-0002 built a pluggable backend contract with a
|
|
declared `RetentionPolicy`; WP-0003 built an ingestion path that bypasses it.
|
|
A 202 currently means "a row exists in a file", against a store that declares
|
|
no custody class at all.
|
|
|
|
Make the ingestion path write through the backend contract, so that accepting
|
|
an event means a backend with a declared retention policy acknowledged it.
|
|
Idempotency state and custody write must not be able to diverge — an event
|
|
recorded as seen but not durably stored is the failure this whole service
|
|
exists to prevent.
|
|
|
|
Add a readiness condition that refuses to serve if the configured backend
|
|
declares `durable=False`, so the development file backend can never silently
|
|
become the production sink.
|
|
|
|
Done when a successful response is backed by a durable backend
|
|
acknowledgment, and the mock backend cannot be used to serve production
|
|
traffic.
|
|
|
|
Done 2026-08-10: added `IdempotentAuditBackend` to the contract — idempotency
|
|
lives inside the backend so custody and duplicate-detection share one
|
|
transaction and cannot diverge. `SQLiteAuditBackend` implements it with WAL,
|
|
`synchronous=FULL`, and a busy timeout. Ingestion refuses any backend
|
|
declaring `durable=False`, so the mock file backend cannot serve.
|
|
|
|
The atomicity claim was tested rather than asserted, and the first
|
|
implementation failed: with one shared connection, 16 racing submissions of
|
|
one event told **two** callers they were first. Fixed with per-thread
|
|
connections and `BEGIN IMMEDIATE` around the insert/read pair. Now covered by
|
|
`test_concurrent_duplicates_produce_exactly_one_record`.
|
|
|
|
## T02 - Fix error semantics and failure handling
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T02
|
|
status: done
|
|
priority: high
|
|
state_hub_task_id: "1591c1ce-3c7b-528c-a9b5-5693ddd03a0d"
|
|
```
|
|
|
|
The handler catches `ValueError`, `TypeError`, `KeyError`, and
|
|
`JSONDecodeError`. Storage errors do not inherit from those:
|
|
`sqlite3.OperationalError` on lock contention and `sqlite3.IntegrityError` on
|
|
the race between the existence check and the insert both escape with
|
|
`start_response` never called. The auth check sits outside the try block, so a
|
|
non-ASCII `Authorization` header raises `TypeError` from
|
|
`hmac.compare_digest` and crashes the request before any handler runs.
|
|
|
|
Add a catch-all that always produces a response, map storage and unexpected
|
|
failures to 503 or 500 rather than letting them surface as dropped
|
|
connections, and move credential comparison inside the guarded path.
|
|
|
|
Correct the status codes: a duplicate event ID carrying a different payload is
|
|
a conflict and must return 409, not 400. This distinction drives the sender's
|
|
retry-versus-dead-letter decision, so the full set of responses — accepted,
|
|
duplicate, conflict, rejected, unavailable — needs to be documented as a
|
|
contract with the retry semantics each implies.
|
|
|
|
Done when no request path can terminate without a response, and every status
|
|
code the receiver returns is deliberate and documented.
|
|
|
|
Done 2026-08-10: catch-all wrapper guarantees a response on every path.
|
|
Conflict is now 409, backend unavailability 503, unexpected faults 500.
|
|
Credential comparison moved inside the guarded path and a non-ASCII
|
|
`Authorization` header is a 401 rather than a crash. The full response
|
|
contract with its retry semantics is documented in the `ingestion` module
|
|
docstring.
|
|
|
|
## T03 - Enforce the isolation properties already claimed
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T03
|
|
status: done
|
|
priority: high
|
|
state_hub_task_id: "9193c3e9-6bf6-50b0-a28b-38cbbe0c12a9"
|
|
```
|
|
|
|
WP-0003 T01 and T02 both record tenant isolation and cross-tenant claim
|
|
rejection as done. `normalize()` accepts whatever `tenant` string the caller
|
|
sends and never checks it against the authenticated identity; no test covers
|
|
it. Separately, `source` is hardcoded to `"user-engine"`, so the allowlist is
|
|
unbound to the credential — any holder of the token can claim to be
|
|
user-engine, and a second sender would require a code change.
|
|
|
|
Bind the authenticated identity to the tenants and sources it may write for,
|
|
and reject claims outside that binding. Drive the binding from configuration
|
|
rather than literals.
|
|
|
|
Done when a credential scoped to one tenant is refused when it claims
|
|
another, and that refusal is covered by a test.
|
|
|
|
Done 2026-08-10: `audit_core.senders` binds each credential to the sources and
|
|
tenants it may assert, configured via `AUDIT_CORE_SENDERS` rather than
|
|
literals. Each identity holds a list of tokens, so rotation publishes the
|
|
replacement alongside the incumbent and needs no delivery gap. Authentication
|
|
compares every candidate token regardless of match position. Read is a
|
|
separate privilege from write. The legacy single-token env var still works but
|
|
grants every tenant and says so.
|
|
|
|
## T04 - Settle redaction policy
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T04
|
|
status: done
|
|
priority: medium
|
|
state_hub_task_id: "092b9a9d-d801-5170-bb01-4288424cb757"
|
|
```
|
|
|
|
`_contains_secret` rejects the entire event when any key name in `data`
|
|
contains a secret-shaped fragment, and never inspects the envelope. WP-0003
|
|
T01 describes "redacted data" and the failure matrix tests redaction, but the
|
|
implementation only rejects.
|
|
|
|
Decide whether a secret-shaped field is a rejection or a redaction, and note
|
|
the consequence: rejection means a legitimate outbox event containing a field
|
|
named `token_count` is dead-lettered at the sender, and the audit trail loses
|
|
the event entirely. Extend detection to the envelope, and consider value shape
|
|
rather than key name alone.
|
|
|
|
Whatever is chosen, the contract document and the sender's expectations must
|
|
agree with the code.
|
|
|
|
Done when the redaction rule is explicit, applied to the whole payload, and
|
|
consistent between the contract and the implementation.
|
|
|
|
Decided 2026-08-10 (Bernd): **default to redaction, allow rejection per
|
|
sender.** Losing an audit record over one field is worse than storing it
|
|
masked, but a higher-assurance channel must be able to refuse rather than
|
|
mask. `secret_policy` is set per sender identity in `AUDIT_CORE_SENDERS`,
|
|
defaulting to `redact`.
|
|
|
|
Done 2026-08-10: `audit_core.redaction` detects secret-shaped keys at any
|
|
depth across the whole payload, including lists. Under redaction the value is
|
|
masked and the **key is preserved** — dropping it would hide that the sender
|
|
transmitted the field at all, which is what the operator needs in order to
|
|
stop it. The stored record carries `details.redaction` with the policy and the
|
|
affected paths, so a reader never has to infer whether what they see is what
|
|
was sent. Idempotency is unaffected: the payload hash is over the original
|
|
body, so redaction is deterministic and replay still reconciles.
|
|
|
|
Both outcomes are counted durably by sender, source, action and **field
|
|
path**, exposed at `GET /v1/secret-findings`. Per-path aggregation is the
|
|
point: the actionable unit is "stop emitting `data.auth.token` on
|
|
`membership.added`", not "there were 47 redactions". Counters survive restart
|
|
because the fix they drive lives in another service. A non-empty list is a
|
|
backlog item for the sender, not a steady state.
|
|
|
|
Contract updated to match; tests 46 -> 50.
|
|
|
|
## T05 - Provide the operator read surface
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T05
|
|
status: done
|
|
priority: high
|
|
state_hub_task_id: "9a1953d1-4528-5572-959d-57901218a9c1"
|
|
```
|
|
|
|
There is no way to read anything back. The failure matrix requires dead-letter
|
|
visibility, operator replay, and correlation lookup; none exist, so that
|
|
evidence cannot be produced regardless of how the deployment goes.
|
|
|
|
Build lookup by event ID and by correlation ID, a dead-letter view of rejected
|
|
events with their rejection reason, and an operator replay path. Replay must
|
|
be idempotent against the same durable store, so a replayed event reconciles
|
|
with the original rather than creating a second custody record.
|
|
|
|
Read access is a separate privilege from write access — a sender credential
|
|
must not be able to read the audit trail back.
|
|
|
|
Done when an operator can trace one correlation ID through the system and
|
|
replay a specific event without creating a duplicate.
|
|
|
|
Done 2026-08-10 (replay deferred, see below): `GET /v1/events/{id}`,
|
|
`GET /v1/events?correlation_id=`, and `GET /v1/dead-letters`, all gated on a
|
|
read privilege a sender credential does not hold. Rejections are now recorded
|
|
rather than silently dropped.
|
|
|
|
One design point worth keeping: an event rejected *for carrying secret-shaped
|
|
material* has its payload withheld from the dead-letter record. Storing it
|
|
would write that material into the audit store, which is what the rejection
|
|
exists to prevent. Reason and payload hash are retained so the event is still
|
|
traceable.
|
|
|
|
Replay is deliberately not implemented here. Idempotent replay is a property
|
|
of the durable store, and building it against SQLite would produce a second
|
|
implementation to discard — it lands with the Postgres backend in
|
|
AUDIT-WP-0005-T01, where `accept()` already gives it the semantics it needs.
|
|
|
|
## T06 - Serving layer and observability
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T06
|
|
status: done
|
|
priority: high
|
|
state_hub_task_id: "28271d49-3aaa-5134-a1a0-e30ddad35e9d"
|
|
```
|
|
|
|
`wsgiref.simple_server` is single-threaded with no request timeout, no
|
|
graceful shutdown, and no access log. One slow client blocks every sender.
|
|
It also makes the failure matrix meaningless: "receiver correctly reported
|
|
unavailable" and "the server stalled" are indistinguishable.
|
|
|
|
Move to a production WSGI server with explicit worker and timeout
|
|
configuration and graceful shutdown on SIGTERM, so in-flight events are not
|
|
lost on rollout.
|
|
|
|
Add structured request logging with a request ID, propagate the correlation ID
|
|
into logs, and expose counters for accepted, duplicate, conflicted, rejected,
|
|
and failed events plus write latency. Logs and metrics must never carry event
|
|
payloads.
|
|
|
|
Done when the receiver serves concurrent senders under a bounded timeout,
|
|
sheds load predictably instead of stalling, and its behaviour is visible from
|
|
outside.
|
|
|
|
Done 2026-08-10: serving moves to waitress with configurable threads and a
|
|
channel timeout, installed in the image via the `serve` extra. Where waitress
|
|
is absent the entrypoint falls back to a threaded wsgiref server with a socket
|
|
timeout and SIGTERM/SIGINT shutdown — bounded rather than good, and it logs a
|
|
warning so a deployment cannot quietly end up on it. Logging is structured
|
|
JSON to stdout.
|
|
|
|
Counters are not yet exposed. Deferred to AUDIT-WP-0005-T03 so the metric
|
|
surface is designed against the deployment's scrape path rather than guessed
|
|
at now.
|
|
|
|
## T07 - Close the test gaps
|
|
|
|
```task
|
|
id: AUDIT-WP-0004-T07
|
|
status: done
|
|
priority: medium
|
|
state_hub_task_id: "2fd0e7d2-ac5b-5d47-be70-743211039163"
|
|
```
|
|
|
|
Ingestion has two tests. Uncovered: oversized and zero-length bodies, absent
|
|
`Content-Length`, malformed JSON, wrong path and method, the health
|
|
endpoints, event ID conflict, naive and out-of-range timestamps, storage
|
|
failure, and persistence across restart.
|
|
|
|
`accepted_at` is recorded in local time while every other timestamp is UTC;
|
|
`datetime.fromisoformat` accepts naive timestamps and arbitrary dates. Fix
|
|
both and cover them.
|
|
|
|
Done when each rejection reason and each failure mode above has a test that
|
|
asserts the documented status code.
|
|
|
|
Done 2026-08-10: ingestion tests went from 2 to 23 (suite 15 -> 36). Covers
|
|
oversized, empty and truncated bodies, absent/invalid `Content-Length`,
|
|
malformed JSON, wrong path and method, health endpoints, id conflict,
|
|
naive and unparseable timestamps, backend unavailability, unexpected backend
|
|
faults, durability across reopen, and the concurrency race. `accepted_at` is
|
|
UTC; naive timestamps are rejected rather than silently assumed.
|