Skip to content

feat(server): record every authorized access to the CRM contact store - #43

Merged
dcondrey merged 1 commit into
mainfrom
feat/crm-audit-log
Aug 5, 2026
Merged

feat(server): record every authorized access to the CRM contact store#43
dcondrey merged 1 commit into
mainfrom
feat/crm-audit-log

Conversation

@dcondrey

@dcondrey dcondrey commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Nothing recorded reads, exports or deletes on the one table in this codebase holding directly-identifying PII. requireTeamRole resolved the operator and their role on every /api/crm request and threw both away. A contact could be listed, exported and erased with no trace that anyone had touched it — and unlike a delete, a read leaves no hole to notice.

The entry is written before the handler runs, not after. Logging afterwards makes the log write the last thing to happen, so any failure between the disclosure and the record — a D1 error, an eviction, a crash — leaves an access that happened and was never written down, and for a DELETE there is then nothing left to notice the gap against. Writing first inverts every one of those: the insert throws, the handler never runs, and nothing was read or changed. The order that can fail open is not available to an audit log. The tests hold it to that: drop the audit table and a contact read comes back 500 with the email nowhere in the body, and a contact delete comes back 500 with the row still there for a retry that can be recorded.

What that ordering costs is precision about outcome. An entry states that an operator was authorized to do this to this id, not that it worked, so a request that goes on to 404 is recorded like any other. For the id-probing case that is the more useful reading — a run of contact.read entries against ids that do not exist is a signal a log of successes only would have thrown away. A create names no target for the same reason: the record does not exist yet when its entry is written, and minting the id first would mean writing the entry afterwards.

It lives in CRM_DB, beside the data it describes. Every entry names a target_id that points into the contact store, and the analytics database is deliberately free of anything that resolves to a named person; putting the log there would have quietly made it the first analytics table holding operator-to-contact pairs. It also means an unbound deployment has no audit table for the same reason it has no contacts — the extension does not exist rather than being empty — and that the log and the row it records are one database apart rather than two.

crmGate(role, action) composes the role guard, the per-operator rate limit and the audit entry into a single middleware, replacing the requireTeamRole(x), crmRateLimit pair that was repeated at fourteen call sites. A route cannot be given a role without also being given an audited action, which is what stops "authorized but unrecorded" from being a thing a future route can quietly be. The order inside it is load-bearing: a request refused on its role, or shed by the limiter, never reached the data and so is not an access to record — and letting it write one would hand a single stolen session the log's own denial of service. Both are pinned by tests, as is the API key, which must not be able to write into the log any more than it can read a contact.

Entries hold ids, a role, an action name and a timestamp — never a contact field. That is what makes them safe to outlive the contact: once the row is deleted the pointer resolves to nothing, so an erasure request does not have to reach in here, and must not, since a log an operator can clear by deleting the contact is not evidence of anything. Deleting a contact therefore leaves its entries standing, and the test asserts neither the name nor the uid appears anywhere in what survives. actor_role is stored rather than resolved at read time because it is the role the request was authorized under; "an admin exported this" has to stay true after they are demoted.

GET /api/crm/audit is admin-only, and not for the bulk-disclosure reason that gates the export — no entry carries PII. It is that the entries are about the deployment's own operators: a log of what each colleague read is oversight in an administrator's hands and surveillance in a peer's. Reading it is itself recorded, through the same gate. There is no route that updates or deletes an entry.

The log is the one CRM table on a retention schedule, on its own job so an unreachable CRM_DB cannot stop raw events being purged from the analytics database. CRM_AUDIT_RETENTION_DAYS defaults to 365, deliberately longer than RAW_RETENTION_DAYS: raw events are visitors' data and the short window is the privacy measure, while these entries record what operators did with contact data, and an access log that expires before the misuse it evidences is noticed has protected nobody. A value below 1 falls back to the default rather than putting the cutoff at or after now and wiping the log on every run — the same validation retentionDays already does, for the same reason. Contacts themselves stay on no schedule at all.

Binding CRM_DB now also claims dpv:ActivityMonitoring. These claims are signed, so a deployment that records every access to its contact store is applying a measure the previous claim set did not name. dpv:ActivityMonitoring rather than dpv:RecordsOfActivities, which in DPV sits with ROPA and the other compliance documents — this is a live access log, not a register of processing. The analytics-only branch is unchanged and the test asserts it does not gain the term.

What the risk is not: no change to ingest, to any analytics table, or to what a deployment without CRM_DB does; no new identifier, and nothing in the log that could re-associate an event with a person. The migration adds one table to migrations-crm, which only a CRM deployment ever applies. The route behaviour changes in exactly one way beyond the new endpoint — a CRM request now fails 500 if its access cannot be recorded, which is the point.

Every new safety assertion was mutation-checked: the audit middleware removed from the gate (10 tests fail), moved after the handler (the fail-closed delete catches it), reordered before the rate limiter, the site scope dropped from the audit list, the retention floor loosened, the binding check dropped, and /audit downgraded to analyst — each time confirming exactly the covering test fails, then restored. The shipped wrangler.jsonc documents the new var and was verified to produce a byte-identical wrangler.test.jsonc.

pnpm lint, pnpm typecheck, pnpm test green: 1601 tests across 175 files, up from 1583. The dashboard has no audit view yet; the endpoint is documented and the CRM tab is the obvious place for it.

Nothing recorded reads, exports or deletes on the one table holding
directly-identifying PII. requireTeamRole already resolved the operator
and their role on every request and discarded both.

crm_audit_log lives in CRM_DB, beside the data it describes: every entry
names a target_id that points into the contact store, and the analytics
database is deliberately free of anything that resolves to a named
person. An unbound deployment has no audit table for the same reason it
has no contacts.

The entry is written BEFORE the handler runs. Logging afterwards leaves
any failure between the disclosure and the record as an access that
happened and was never written down, and for a DELETE there is then
nothing left to notice the gap against. Writing first inverts that: the
insert throws, the handler never runs, nothing was read or changed. The
cost is that an entry states an operator was authorized to do this to
this id, not that it worked, and a request that then 404s is recorded
like any other — which for id probing is the more useful reading.

crmGate(role, action) composes the role guard, the per-operator rate
limit and the audit entry into one middleware, replacing the
requireTeamRole/crmRateLimit pair repeated at fourteen call sites. A
route cannot be given a role without also being given an audited action.
Order is load-bearing: a request refused on role or shed by the limiter
never reached the data, so it is not an access to record, and letting it
write one would hand a single session the log's own denial of service.

Entries hold ids, a role, an action name and a timestamp — never a
contact field. That is what makes them safe to outlive the contact: once
the row is deleted the pointer resolves to nothing, so erasure does not
have to reach in here, and a log an operator can clear by deleting the
contact is not evidence of anything. Nothing updates or deletes an entry
except the retention cron.

GET /api/crm/audit is admin-only, and not for the bulk-disclosure reason
that gates the export — no entry carries PII. It is that the entries are
about the deployment's own operators, which is oversight in an
administrator's hands and surveillance in a peer's. Reading it is itself
recorded.

The log is the one CRM table on a schedule: CRM_AUDIT_RETENTION_DAYS,
default 365, purged by its own cron job so an unreachable CRM_DB cannot
stop raw events being purged from the analytics database. Longer than
RAW_RETENTION_DAYS deliberately — raw events are visitors' data and the
short window is the privacy measure, while these entries record what
operators did, and an access log that expires before the misuse it
evidences is noticed has protected nobody. Contacts stay on no schedule.

Binding CRM_DB now also claims dpv:ActivityMonitoring. The claims are
signed, and a deployment that records every access to its contact store
is applying a measure the previous claim set did not name.

Also, autonomously: dropped the duplicated middleware pair from all
fourteen CRM routes; documented the audit surface in docs/api.md.
@dcondrey
dcondrey force-pushed the feat/crm-audit-log branch from 5c7c4f6 to 1c548b8 Compare August 5, 2026 07:05
@dcondrey
dcondrey merged commit 6eed356 into main Aug 5, 2026
6 checks passed
@dcondrey
dcondrey deleted the feat/crm-audit-log branch August 5, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant