diff --git a/apps/server/migrations-crm/0002_marvelous_agent_zero.sql b/apps/server/migrations-crm/0002_marvelous_agent_zero.sql new file mode 100644 index 0000000..520d7ff --- /dev/null +++ b/apps/server/migrations-crm/0002_marvelous_agent_zero.sql @@ -0,0 +1,13 @@ +CREATE TABLE `crm_audit_log` ( + `id` text PRIMARY KEY NOT NULL, + `site_id` text NOT NULL, + `actor_user_id` text NOT NULL, + `actor_role` text NOT NULL, + `action` text NOT NULL, + `target_id` text, + `occurred_at` integer NOT NULL +); +--> statement-breakpoint +CREATE INDEX `idx_crm_audit_site_time` ON `crm_audit_log` (`site_id`,`occurred_at`);--> statement-breakpoint +CREATE INDEX `idx_crm_audit_site_target` ON `crm_audit_log` (`site_id`,`target_id`);--> statement-breakpoint +CREATE INDEX `idx_crm_audit_occurred` ON `crm_audit_log` (`occurred_at`); \ No newline at end of file diff --git a/apps/server/migrations-crm/meta/0002_snapshot.json b/apps/server/migrations-crm/meta/0002_snapshot.json new file mode 100644 index 0000000..f88f62d --- /dev/null +++ b/apps/server/migrations-crm/meta/0002_snapshot.json @@ -0,0 +1,379 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "d5ac3527-c40c-4c64-9a7c-33ee2819ab19", + "prevId": "6dbe8789-6aab-43b0-adb2-f05317a871c8", + "tables": { + "companies": { + "name": "companies", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "site_id": { + "name": "site_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "domain": { + "name": "domain", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'lead'" + }, + "notes": { + "name": "notes", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_companies_site_created": { + "name": "idx_companies_site_created", + "columns": [ + "site_id", + "created_at" + ], + "isUnique": false + }, + "idx_companies_site_status": { + "name": "idx_companies_site_status", + "columns": [ + "site_id", + "status" + ], + "isUnique": false + }, + "idx_companies_site_name": { + "name": "idx_companies_site_name", + "columns": [ + "site_id", + "name" + ], + "isUnique": true + }, + "idx_companies_site_domain": { + "name": "idx_companies_site_domain", + "columns": [ + "site_id", + "domain" + ], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "contacts": { + "name": "contacts", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "site_id": { + "name": "site_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "external_user_id": { + "name": "external_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "email": { + "name": "email", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "phone": { + "name": "phone", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "company": { + "name": "company", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "company_id": { + "name": "company_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'lead'" + }, + "source": { + "name": "source", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "notes": { + "name": "notes", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "owner_user_id": { + "name": "owner_user_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_contacts_site_created": { + "name": "idx_contacts_site_created", + "columns": [ + "site_id", + "created_at" + ], + "isUnique": false + }, + "idx_contacts_site_status": { + "name": "idx_contacts_site_status", + "columns": [ + "site_id", + "status" + ], + "isUnique": false + }, + "idx_contacts_site_company": { + "name": "idx_contacts_site_company", + "columns": [ + "site_id", + "company_id" + ], + "isUnique": false + }, + "idx_contacts_site_email": { + "name": "idx_contacts_site_email", + "columns": [ + "site_id", + "email" + ], + "isUnique": true + }, + "idx_contacts_site_extuser": { + "name": "idx_contacts_site_extuser", + "columns": [ + "site_id", + "external_user_id" + ], + "isUnique": true + } + }, + "foreignKeys": { + "contacts_company_id_companies_id_fk": { + "name": "contacts_company_id_companies_id_fk", + "tableFrom": "contacts", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "crm_audit_log": { + "name": "crm_audit_log", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "site_id": { + "name": "site_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_user_id": { + "name": "actor_user_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_role": { + "name": "actor_role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "action": { + "name": "action", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "target_id": { + "name": "target_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "occurred_at": { + "name": "occurred_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "idx_crm_audit_site_time": { + "name": "idx_crm_audit_site_time", + "columns": [ + "site_id", + "occurred_at" + ], + "isUnique": false + }, + "idx_crm_audit_site_target": { + "name": "idx_crm_audit_site_target", + "columns": [ + "site_id", + "target_id" + ], + "isUnique": false + }, + "idx_crm_audit_occurred": { + "name": "idx_crm_audit_occurred", + "columns": [ + "occurred_at" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/apps/server/migrations-crm/meta/_journal.json b/apps/server/migrations-crm/meta/_journal.json index b51f69d..6d79167 100644 --- a/apps/server/migrations-crm/meta/_journal.json +++ b/apps/server/migrations-crm/meta/_journal.json @@ -15,6 +15,13 @@ "when": 1785902667995, "tag": "0001_perfect_tombstone", "breakpoints": true + }, + { + "idx": 2, + "version": "6", + "when": 1785912191326, + "tag": "0002_marvelous_agent_zero", + "breakpoints": true } ] } \ No newline at end of file diff --git a/apps/server/src/db/crm-schema.ts b/apps/server/src/db/crm-schema.ts index b520eeb..8759825 100644 --- a/apps/server/src/db/crm-schema.ts +++ b/apps/server/src/db/crm-schema.ts @@ -12,6 +12,8 @@ // designed in rather than deferred: // • Contacts are NOT on the raw-event retention schedule (`lib/retention.ts`). A contact is a // business record with its own lifecycle; deleting it is an explicit act, not a cron side effect. +// `crm_audit_log` is the one table here that IS on a schedule, on its own longer window, because +// it is the one that grows without anybody deciding to add a row. // • No column here caches a derived `visitor_hash`. The ONLY bridge to analytics is // `external_user_id`, resolved at read time through an active `identified` consent record. When // retention purges that record (or its identity salt), the link severs on its own — nothing here @@ -109,3 +111,51 @@ export const contacts = sqliteTable( uniqueIndex('idx_contacts_site_extuser').on(t.site_id, t.external_user_id), ], ); + +/** + * Who touched the contact store, what they touched, and when. One row per authorized `/api/crm` + * request, written BEFORE the handler runs. + * + * WHY IT IS HERE rather than in the analytics database. Every row names a `target_id`, which is a + * pointer into this database's PII, and the analytics database is deliberately free of anything that + * resolves to a named person. Keeping the log beside the data it describes also means an unbound + * deployment has no audit table for the same reason it has no contacts — the extension does not + * exist — and that the log and the row it records are one database apart rather than two, which is + * the only reason the two writes can be reasoned about at all. + * + * WHAT IT DELIBERATELY DOES NOT HOLD is any contact FIELD. It records that contact `x` was read, not + * what reading it returned, so the log is a record about the OPERATOR and only a pointer to the + * subject. That is what makes it 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. Nothing in the API + * updates or deletes these rows; only the retention cron does. + * + * `actor_role` is stored rather than resolved at read time because it is the role the request was + * AUTHORIZED under. Roles change; "an admin exported this" has to stay true after they are demoted. + */ +export const crmAuditLog = sqliteTable( + 'crm_audit_log', + { + id: text('id').primaryKey(), + site_id: text('site_id').notNull(), + /** `users.id` in the ANALYTICS database, so no foreign key is possible — the same cross-database + * limitation as `contacts.owner_user_id`, and the same reason it is validated in the Worker. */ + actor_user_id: text('actor_user_id').notNull(), + actor_role: text('actor_role').notNull(), + /** One of `CRM_AUDIT_ACTIONS`. A closed set, so the log is filterable by equality. */ + action: text('action').notNull(), + /** The contact or company the request named, or NULL for a collection-level action. */ + target_id: text('target_id'), + occurred_at: integer('occurred_at').notNull(), + }, + (t) => [ + // The default view: one site's log, newest first. Also what the actor and action filters scan. + index('idx_crm_audit_site_time').on(t.site_id, t.occurred_at), + // "Everything anyone did to this contact" — the question an erasure or subject-access request + // asks, and the one a site-and-time scan answers worst. + index('idx_crm_audit_site_target').on(t.site_id, t.target_id), + // Retention purges across every site at once, so it needs the timestamp leading. Same shape and + // same reason as `idx_identity_salts_window_end` in the analytics schema. + index('idx_crm_audit_occurred').on(t.occurred_at), + ], +); diff --git a/apps/server/src/db/crm.ts b/apps/server/src/db/crm.ts index 73a4bb8..cc1a969 100644 --- a/apps/server/src/db/crm.ts +++ b/apps/server/src/db/crm.ts @@ -6,10 +6,11 @@ // never from a body, so a session with a role on one site cannot reach another site's contacts even // by guessing a contact id. That covers the contact→company link too: the foreign key proves the // company row exists, not that it belongs to the caller's site, so the site predicate is the check -// and the constraint is only the backstop. +// and the constraint is only the backstop. `purgeCrmAudit` is the one exception and says why: it runs +// from cron, on behalf of no request, and a per-site purge would be the same delete run N times. -import { normalizeCompanyDomain } from '@facet/shared'; -import { type SQL, and, desc, eq, isNotNull, or, sql } from 'drizzle-orm'; +import { type CrmAuditAction, normalizeCompanyDomain } from '@facet/shared'; +import { type SQL, and, desc, eq, isNotNull, lt, or, sql } from 'drizzle-orm'; import { drizzle } from 'drizzle-orm/d1'; import type { SQLiteColumn } from 'drizzle-orm/sqlite-core'; import type { MiddlewareHandler } from 'hono'; @@ -659,3 +660,86 @@ export async function deleteCompany( if (deleted.length === 0) return undefined; return { company, contacts_unlinked: counted[0]?.n ?? 0 }; } + +/** One entry as the audit log stores and returns it. There is no separate wire shape: every column + * is already an id, a role, an action name or a timestamp, so there is nothing to redact on the way + * out that was safe to record on the way in. */ +export type CrmAuditEntry = typeof crmSchema.crmAuditLog.$inferSelect; + +/** What an audit entry is written from. The actor and role come from the session guard that + * authorized the request, never from anything the caller sent. */ +export interface CrmAuditInput { + actorUserId: string; + actorRole: string; + action: CrmAuditAction; + targetId: string | null; + occurredAt: number; +} + +/** Append one entry. Insert-only by design — nothing in this module updates or deletes an entry + * except `purgeCrmAudit`, so a recorded access cannot be rewritten by the operator it names. */ +export async function recordCrmAccess( + binding: D1Database, + siteId: string, + entry: CrmAuditInput, +): Promise { + await crmDb(binding).insert(crmSchema.crmAuditLog).values({ + id: crypto.randomUUID(), + site_id: siteId, + actor_user_id: entry.actorUserId, + actor_role: entry.actorRole, + action: entry.action, + target_id: entry.targetId, + occurred_at: entry.occurredAt, + }); +} + +export interface ListCrmAuditOptions { + /** Typed to the closed set rather than to `string`: an action outside it matches nothing, so + * accepting one would answer "no such access" to a question that was never asked. */ + action?: CrmAuditAction; + actorUserId?: string; + targetId?: string; + limit: number; + offset: number; +} + +/** One site's audit entries, newest first. Every filter is an equality — see the wire schema for why + * there is no substring search over a log of ids. */ +export async function listCrmAudit( + binding: D1Database, + siteId: string, + opts: ListCrmAuditOptions, +): Promise<{ entries: CrmAuditEntry[]; total: number }> { + const filters = [eq(crmSchema.crmAuditLog.site_id, siteId)]; + if (opts.action) filters.push(eq(crmSchema.crmAuditLog.action, opts.action)); + if (opts.actorUserId) { + filters.push(eq(crmSchema.crmAuditLog.actor_user_id, opts.actorUserId)); + } + if (opts.targetId) filters.push(eq(crmSchema.crmAuditLog.target_id, opts.targetId)); + const where = and(...filters); + const client = crmDb(binding); + const [entries, totalRow] = await Promise.all([ + client + .select() + .from(crmSchema.crmAuditLog) + .where(where) + // Two requests inside the same millisecond would otherwise come back in an arbitrary and + // unstable order, which for a paged log means an entry can be shown twice or skipped. `id` + // is the tiebreak because it is the only unique column. + .orderBy(desc(crmSchema.crmAuditLog.occurred_at), desc(crmSchema.crmAuditLog.id)) + .limit(opts.limit) + .offset(opts.offset), + client.select({ n: sql`count(*)` }).from(crmSchema.crmAuditLog).where(where).get(), + ]); + return { entries, total: totalRow?.n ?? 0 }; +} + +/** Delete audit entries older than `cutoff`, across every site — the retention cron acts for the + * deployment, not for a request, and there is no site to scope it to. Returns rows deleted. */ +export async function purgeCrmAudit(binding: D1Database, cutoff: number): Promise { + const res = await crmDb(binding) + .delete(crmSchema.crmAuditLog) + .where(lt(crmSchema.crmAuditLog.occurred_at, cutoff)); + return res.meta.changes ?? 0; +} diff --git a/apps/server/src/env.ts b/apps/server/src/env.ts index ad814ca..f14a3d7 100644 --- a/apps/server/src/env.ts +++ b/apps/server/src/env.ts @@ -18,6 +18,10 @@ export interface Env { AE?: AnalyticsEngineDataset; /** Rolling retention window for raw events, in days (string var). */ RAW_RETENTION_DAYS: string; + /** Rolling retention window for the CRM audit log, in days (string var). Optional: unset means the + * default in `constants.ts`, which is deliberately longer than the raw-event window because the log + * records what OPERATORS did rather than what visitors did. Only read when `CRM_DB` is bound. */ + CRM_AUDIT_RETENTION_DAYS?: string; /** Cloudflare account id (var), used for Analytics Engine SQL-over-HTTP reads. */ CF_ACCOUNT_ID: string; /** Cloudflare API token (Worker secret) for Analytics Engine SQL-over-HTTP reads. */ diff --git a/apps/server/src/lib/constants.ts b/apps/server/src/lib/constants.ts index cb27827..62ee44f 100644 --- a/apps/server/src/lib/constants.ts +++ b/apps/server/src/lib/constants.ts @@ -9,6 +9,20 @@ export const SALT_BYTES = 32 as const; /** Default rolling retention window for raw events, in days. */ export const DEFAULT_RAW_RETENTION_DAYS = 90 as const; +/** + * Default retention for the CRM audit log, in days. + * + * Deliberately longer than the raw-event window, because it answers a different question about + * different people. Raw events are visitors' data and the short window IS the privacy measure; audit + * entries are a record of what the deployment's own operators did with contact data, and an access + * log that expires before the misuse it evidences is noticed has protected nobody. A year covers an + * annual review and the interval in which a complaint or a breach is normally traced. + * + * Bounded rather than kept forever, because "we never delete it" is not a retention policy and an + * append-only table with no ceiling is a growth defect however small each row is. + */ +export const DEFAULT_CRM_AUDIT_RETENTION_DAYS = 365 as const; + /** CORS max-age for preflight responses, in seconds. */ export const CORS_MAX_AGE = 86400 as const; diff --git a/apps/server/src/lib/dpv.ts b/apps/server/src/lib/dpv.ts index 19e65c0..a8a1f79 100644 --- a/apps/server/src/lib/dpv.ts +++ b/apps/server/src/lib/dpv.ts @@ -34,8 +34,9 @@ export const DPV_PD_CONTEXT = { * active signed consent record, never by legitimate interest. * • Pseudonymisation stops being the sole measure. It still describes the analytics half, but * directly-supplied contact details are not pseudonymised; what protects them is access control - * (an authenticated operator session with a team role, never an API key), so that is named - * alongside it rather than letting one term imply coverage it does not have. + * (an authenticated operator session with a team role, never an API key) and a record of every + * access made under it, so both are named alongside it rather than letting one term imply + * coverage it does not have. */ export function privacyDpvClaims(env: Env): Record { if (!env.CRM_DB) { @@ -62,6 +63,13 @@ export function privacyDpvClaims(env: Env): Record { 'dpv:hasTechnicalOrganisationalMeasure': [ 'dpv:Pseudonymisation', 'dpv:AccessControlMethod', + // The CRM audit log: every authorized request against the contact store is recorded before + // it runs. `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. Claimed only on the CRM branch because it is the only data this deployment + // holds that a person can read one record at a time, and therefore the only data where who + // looked is a fact worth keeping. + 'dpv:ActivityMonitoring', ], 'dpv:hasDataSubject': 'dpv:Customer', // The categories the contact schema has columns for. Free-text fields an operator may fill diff --git a/apps/server/src/lib/retention.ts b/apps/server/src/lib/retention.ts index 7961fcd..28c0cb6 100644 --- a/apps/server/src/lib/retention.ts +++ b/apps/server/src/lib/retention.ts @@ -1,11 +1,20 @@ // Retention cleanup: delete raw events, sessions, salts, and identity mappings older than the rolling // window. `event_rollups` are durable history and are never deleted. Invoked from the cron handler. +// +// The optional CRM has its own window and its own function. Contacts are NOT on any schedule — a +// contact is a business record that is deleted by an explicit act, never by a cron — but the audit +// log recording who read them is, because it is the one CRM table that grows on its own. import { lt } from 'drizzle-orm'; +import { purgeCrmAudit } from '../db/crm.js'; import { db } from '../db/queries.js'; import * as schema from '../db/schema.js'; import type { Env } from '../env.js'; -import { DAY_MS, DEFAULT_RAW_RETENTION_DAYS } from './constants.js'; +import { + DAY_MS, + DEFAULT_CRM_AUDIT_RETENTION_DAYS, + DEFAULT_RAW_RETENTION_DAYS, +} from './constants.js'; /** * The deployment's raw-data window in days — the ONE reading of `RAW_RETENTION_DAYS`. Every caller @@ -35,3 +44,30 @@ export async function enforceRetention(env: Env, now: number): Promise { // the at-rest raw uid. (Elevation already stops the instant a record expires or is revoked.) await db(env).delete(schema.consentRecords).where(lt(schema.consentRecords.granted_at, cutoff)); } + +/** + * The audit log's window in days — the ONE reading of `CRM_AUDIT_RETENTION_DAYS`, validated exactly + * as `retentionDays` validates its own var and for the same reason: a zero or negative value puts the + * cutoff at or after `now` and every run would erase the log it was meant to age. + */ +export function crmAuditRetentionDays(env: Env): number { + const days = Number.parseInt(env.CRM_AUDIT_RETENTION_DAYS ?? '', 10); + return Number.isInteger(days) && days >= 1 ? days : DEFAULT_CRM_AUDIT_RETENTION_DAYS; +} + +/** + * Purge audit entries older than `CRM_AUDIT_RETENTION_DAYS`. A no-op on a deployment with no CRM + * binding, which has no such table — the extension being off means it does not exist, not that it is + * empty. + * + * Separate from `enforceRetention` rather than folded into it because they are different windows over + * different databases, and because the cron isolates failures per job: an unreachable `CRM_DB` must + * not be able to stop raw events being purged from the analytics one. + * + * Returns entries purged, which is zero on an unbound deployment for the same reason it is zero on a + * quiet one. + */ +export async function enforceCrmAuditRetention(env: Env, now: number): Promise { + if (!env.CRM_DB) return 0; + return purgeCrmAudit(env.CRM_DB, now - crmAuditRetentionDays(env) * DAY_MS); +} diff --git a/apps/server/src/lib/scheduled.ts b/apps/server/src/lib/scheduled.ts index f11ad17..8b45c3d 100644 --- a/apps/server/src/lib/scheduled.ts +++ b/apps/server/src/lib/scheduled.ts @@ -4,7 +4,7 @@ import type { Env } from '../env.js'; import { HOUR_MS } from './constants.js'; import { createLogger } from './log.js'; -import { enforceRetention } from './retention.js'; +import { enforceCrmAuditRetention, enforceRetention } from './retention.js'; import { runRollups } from './rollups.js'; import { dayKey } from './salt.js'; import { buildSessions } from './sessions.js'; @@ -36,6 +36,13 @@ registerJob({ name: 'retention', run: (env, now) => enforceRetention(env, now), }); +// Optional: age out the CRM audit log on its own, longer window. No-op unless CRM_DB is bound. +registerJob({ + name: 'crm-audit-retention', + run: async (env, now) => { + await enforceCrmAuditRetention(env, now); + }, +}); // Optional: deliver anomaly-alert webhooks. No-op unless WEBHOOK_URL is configured. registerJob({ name: 'anomaly-alerts', diff --git a/apps/server/src/routes/crm.ts b/apps/server/src/routes/crm.ts index 3010464..c1af7ab 100644 --- a/apps/server/src/routes/crm.ts +++ b/apps/server/src/routes/crm.ts @@ -1,4 +1,4 @@ -// CRM endpoints — the optional contacts-and-companies extension. Two gates apply to every route +// CRM endpoints — the optional contacts-and-companies extension. Three gates apply to every route // here, in order. // // 1. THE BINDING. No `CRM_DB` means this deployment has no CRM database: 501 `crm_unavailable`, @@ -18,6 +18,10 @@ // `viewer` — who can see aggregate analytics — has no CRM access at all, because PII is a // different kind of access, not more of the same one. // +// 3. THE RECORD. Every request that clears both gates is written to the CRM audit log BEFORE its +// handler runs. `crmGate` below is what binds the three together, so a route cannot be given a +// role without also being given an audited action. +// // The contact→analytics link (`GET /contacts/:id/analytics`) never queries events by anything a // contact row controls. It resolves `external_user_id` through `findLinkedVisitorHashes`, which // returns hashes only from consent statements that verify against the deployment key — so a contact @@ -37,11 +41,14 @@ import { ContactCreateSchema, ContactListQuerySchema, ContactUpdateSchema, + type CrmAuditAction, + CrmAuditListQuerySchema, } from '@facet/shared'; import { vValidator } from '@hono/valibot-validator'; import { eq } from 'drizzle-orm'; -import { Hono } from 'hono'; +import { Hono, type MiddlewareHandler } from 'hono'; import { bodyLimit } from 'hono/body-limit'; +import { every } from 'hono/combine'; import { COMPANY_ROLLUP_MAX_CONTACTS, CONTACT_EXPORT_MAX_EVENTS, @@ -63,6 +70,8 @@ import { listCompanies, listCompanyContacts, listContacts, + listCrmAudit, + recordCrmAccess, requireCrm, requireCrmDb, uniqueConstraintText, @@ -72,6 +81,7 @@ import { import { db } from '../db/queries.js'; import * as schema from '../db/schema.js'; import type { AppEnv, Env } from '../env.js'; +import type { Role } from '../lib/accounts.js'; import { requireTeamRole } from '../lib/auth.js'; import { eraseConsentByExternalUserId, @@ -108,11 +118,72 @@ crmRoutes.use( * legitimate traffic and would punish their colleagues for it; keying per operator caps the session * that is actually doing it. * - * Applied AFTER the role guard at every call site, matching /api/event: an unauthenticated request is - * rejected before it can consume anyone's bucket, and `userId` is only set once a session resolves. + * Applied AFTER the role guard — see `crmGate`, which is now the only thing that applies it — matching + * /api/event: an unauthenticated request is rejected before it can consume anyone's bucket, and + * `userId` is only set once a session resolves. */ const crmRateLimit = rateLimit((c) => `crm:${c.get('userId') ?? 'unauthenticated'}`); +/** + * Record this request in the CRM audit log, then run it. + * + * BEFORE, NOT AFTER, and that ordering is the whole design. Logging afterwards means the log write + * is 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. Logging 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. + * + * What it costs is precision about outcome. A request that goes on to 404 or to fail validation is + * recorded exactly like one that succeeded, so an entry states that an operator was authorized to do + * this to this id, not that it worked. That is the honest reading, and for the id-probing case it is + * the more useful one — a run of `contact.read` entries against ids that do not exist is a signal + * that a log of successes only would have thrown away. + * + * A create therefore names no target: the record it makes does not exist yet when its entry is + * written, and recording the new id would mean writing the entry afterwards, which is the ordering + * this rejects. The row's own `created_at` sits beside the entry's `occurred_at`, so the two are + * still correlatable — and a create is the one action that discloses nothing. + * + * The actor is re-checked rather than assumed. `crmGate` always runs `requireTeamRole` first, so + * both are set; if this middleware is ever mounted without it, an entry naming nobody is worse than + * a 401. + */ +function auditCrm(action: CrmAuditAction): MiddlewareHandler { + return async (c, next) => { + const actorUserId = c.get('userId'); + const actorRole = c.get('role'); + if (!actorUserId || !actorRole) { + throw new ApiError('unauthorized', 401); + } + await recordCrmAccess(requireCrmDb(c.env), c.get('siteId'), { + actorUserId, + actorRole, + action, + // Undefined on the collection routes, which name no single record. + targetId: c.req.param('id') ?? null, + occurredAt: Date.now(), + }); + return next(); + }; +} + +/** + * The one gate every CRM route passes: a team role, the per-operator rate limit, and the audit entry, + * composed in that order and applied as a single middleware. + * + * Composed rather than listed at each route because these three are one decision, not three. The + * previous form repeated `requireTeamRole(x), crmRateLimit` at fourteen call sites, which made + * "authorized but unrecorded" a thing a new route could quietly be — you cannot forget the audit + * action here without also forgetting the role, and a route with no role guard is not a route anyone + * ships. The order matters too: a request rejected for its role, or shed by the rate limiter, never + * reached the data and so is not an access to record — and letting it write one would hand a single + * session the ability to fill the audit table with entries for requests it was never allowed to make. + */ +function crmGate(need: Role, action: CrmAuditAction): MiddlewareHandler { + return every(requireTeamRole(need), crmRateLimit, auditCrm(action)); +} + /** Resolve a contact or raise the canonical 404. Scoped by the authorized site, so a contact id from * another site is indistinguishable from one that does not exist. */ async function loadContact(env: Env, siteId: string, id: string): Promise { @@ -195,8 +266,7 @@ function linkedHashes( */ crmRoutes.get( '/contacts', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'contact.list'), vValidator('query', ContactListQuerySchema, validationErrorHook), async (c) => { const query = c.req.valid('query'); @@ -212,8 +282,7 @@ crmRoutes.get( crmRoutes.post( '/contacts', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'contact.create'), vValidator('json', ContactCreateSchema, validationErrorHook), async (c) => { const body = c.req.valid('json'); @@ -229,15 +298,14 @@ crmRoutes.post( }, ); -crmRoutes.get('/contacts/:id', requireTeamRole('analyst'), crmRateLimit, async (c) => { +crmRoutes.get('/contacts/:id', crmGate('analyst', 'contact.read'), async (c) => { const contact = await loadContact(c.env, c.get('siteId'), c.req.param('id')); return c.json({ contact }); }); crmRoutes.patch( '/contacts/:id', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'contact.update'), vValidator('json', ContactUpdateSchema, validationErrorHook), async (c) => { const body = c.req.valid('json'); @@ -267,7 +335,7 @@ crmRoutes.patch( * keyed by a salted hash, and with the consent record gone nothing can ever re-associate them with a * person; destroying the link is what erasure of the identifiable data means here. */ -crmRoutes.delete('/contacts/:id', requireTeamRole('admin'), crmRateLimit, async (c) => { +crmRoutes.delete('/contacts/:id', crmGate('admin', 'contact.delete'), async (c) => { const siteId = c.get('siteId'); const contact = await loadContact(c.env, siteId, c.req.param('id') ?? ''); // The two writes land in DIFFERENT databases and D1 has no transaction spanning them, so one of @@ -293,7 +361,7 @@ crmRoutes.delete('/contacts/:id', requireTeamRole('admin'), crmRateLimit, async /** A contact's analytics, if and only if an active signed consent record authorizes the link. When * it does not, the response says so explicitly rather than returning zeroes that read like "this * person did nothing" — `linked: false` and a reason are the honest answer. */ -crmRoutes.get('/contacts/:id/analytics', requireTeamRole('analyst'), crmRateLimit, async (c) => { +crmRoutes.get('/contacts/:id/analytics', crmGate('analyst', 'contact.analytics'), async (c) => { const siteId = c.get('siteId'); const contact = await loadContact(c.env, siteId, c.req.param('id')); if (!contact.external_user_id) { @@ -320,7 +388,7 @@ crmRoutes.get('/contacts/:id/analytics', requireTeamRole('analyst'), crmRateLimi * (their claims are a derived hash, a tier and a window), so including them adds cryptographic * evidence of what was consented to without widening what the export reveals. */ -crmRoutes.get('/contacts/:id/export', requireTeamRole('admin'), crmRateLimit, async (c) => { +crmRoutes.get('/contacts/:id/export', crmGate('admin', 'contact.export'), async (c) => { const siteId = c.get('siteId'); const contact = await loadContact(c.env, siteId, c.req.param('id')); const externalUserId = contact.external_user_id; @@ -377,8 +445,7 @@ function companyConflict(err: unknown): never { crmRoutes.get( '/companies', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'company.list'), vValidator('query', CompanyListQuerySchema, validationErrorHook), async (c) => { const query = c.req.valid('query'); @@ -394,8 +461,7 @@ crmRoutes.get( crmRoutes.post( '/companies', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'company.create'), vValidator('json', CompanyCreateSchema, validationErrorHook), async (c) => { const body = c.req.valid('json'); @@ -411,15 +477,14 @@ crmRoutes.post( }, ); -crmRoutes.get('/companies/:id', requireTeamRole('analyst'), crmRateLimit, async (c) => { +crmRoutes.get('/companies/:id', crmGate('analyst', 'company.read'), async (c) => { const company = await loadCompany(c.env, c.get('siteId'), c.req.param('id')); return c.json({ company }); }); crmRoutes.patch( '/companies/:id', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'company.update'), vValidator('json', CompanyUpdateSchema, validationErrorHook), async (c) => { const body = c.req.valid('json'); @@ -447,7 +512,7 @@ crmRoutes.patch( * `admin` rather than `analyst` because it is irreversible and it rewrites rows the caller did not * name — the same reason deleting a contact is. */ -crmRoutes.delete('/companies/:id', requireTeamRole('admin'), crmRateLimit, async (c) => { +crmRoutes.delete('/companies/:id', crmGate('admin', 'company.delete'), async (c) => { const result = await deleteCompany(requireCrmDb(c.env), c.get('siteId'), c.req.param('id')); if (!result) { throw new ApiError('not_found', 404); @@ -457,8 +522,7 @@ crmRoutes.delete('/companies/:id', requireTeamRole('admin'), crmRateLimit, async crmRoutes.get( '/companies/:id/contacts', - requireTeamRole('analyst'), - crmRateLimit, + crmGate('analyst', 'company.contacts'), vValidator('query', CompanyContactsQuerySchema, validationErrorHook), async (c) => { const siteId = c.get('siteId'); @@ -495,7 +559,7 @@ crmRoutes.get( * start reasoning about the eleven who never consented. So `contacts_total` and `contacts_linked` are * reported side by side and one-of-twelve is visible as one-of-twelve. */ -crmRoutes.get('/companies/:id/analytics', requireTeamRole('analyst'), crmRateLimit, async (c) => { +crmRoutes.get('/companies/:id/analytics', crmGate('analyst', 'company.analytics'), async (c) => { const siteId = c.get('siteId'); const company = await loadCompany(c.env, siteId, c.req.param('id')); const linkage = await companyContactLinkage( @@ -543,3 +607,37 @@ crmRoutes.get('/companies/:id/analytics', requireTeamRole('analyst'), crmRateLim activity: await contactActivity(c.env, siteId, hashes), }); }); + +/** + * Read this site's audit log: who touched the contact store, what they touched, and when. + * + * `admin` rather than `analyst`, and not for the usual reason. Nothing in an entry is contact PII — + * every field is an id, a role, an action name or a timestamp — so this is not the bulk-disclosure + * argument that gates the export. 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, and the person answerable for how contact data is used is the one who should hold it. + * + * The filters are the three questions worth asking of an access log — what happened to this record, + * what did this operator do, who does this kind of thing — and each is an exact match, because a log + * of ids has no fragments to search for. + * + * There is no way to write here. No route updates or deletes an entry, deleting a contact leaves its + * entries standing, and only the retention cron removes anything: a log the people it names can edit + * records nothing. Reading it is itself recorded, by the same gate as every other route. + */ +crmRoutes.get( + '/audit', + crmGate('admin', 'audit.read'), + vValidator('query', CrmAuditListQuerySchema, validationErrorHook), + async (c) => { + const query = c.req.valid('query'); + const { entries, total } = await listCrmAudit(requireCrmDb(c.env), c.get('siteId'), { + action: query.action, + actorUserId: query.actor_user_id, + targetId: query.target_id, + limit: query.limit ?? CRM_DEFAULT_PAGE, + offset: query.offset ?? 0, + }); + return c.json({ entries, total, role: c.get('role') }); + }, +); diff --git a/apps/server/test/crm-audit.test.ts b/apps/server/test/crm-audit.test.ts new file mode 100644 index 0000000..95f21bf --- /dev/null +++ b/apps/server/test/crm-audit.test.ts @@ -0,0 +1,488 @@ +// The CRM audit log. What matters here is not that entries can be written and listed, but the four +// properties that make the log worth having: that EVERY route writes one, that it is written before +// the handler so nothing can happen unrecorded, that a request which was never authorized writes +// nothing, and that deleting the contact does not delete the evidence. +// +// The route-coverage test is deliberately table-driven over `CRM_AUDIT_ACTIONS`: a new CRM route that +// is given a role but no audited action cannot compile, and an action added to the set with no route +// behind it fails here. + +import { env } from 'cloudflare:test'; +import { CRM_AUDIT_ACTIONS } from '@facet/shared'; +import { beforeEach, describe, expect, it } from 'vitest'; +import { createApp } from '../src/app.js'; +import { + SESSION_COOKIE, + signSession, + upsertUserByEmail, + userMemberships, +} from '../src/lib/accounts.js'; +import { issueKey } from '../src/lib/apikeys.js'; +import { enforceCrmAuditRetention } from '../src/lib/retention.js'; + +const SITE = '77777777-7777-4777-8777-777777777777'; +const OTHER_SITE = '88888888-8888-4888-8888-888888888888'; +const DAY = 86_400_000; +const app = createApp(); + +type TestEnv = typeof env; + +interface AuditRow { + id: string; + site_id: string; + actor_user_id: string; + actor_role: string; + action: string; + target_id: string | null; + occurred_at: number; +} + +/** An env with the CRM binding removed — what a deployment that never created the database is. */ +function unbound(e: TestEnv): TestEnv { + const { CRM_DB: Omitted, ...rest } = e; + return rest as unknown as TestEnv; +} + +async function seedSite(e: TestEnv): Promise { + await e.DB.prepare( + 'INSERT OR IGNORE INTO sites (id, name, domain, created_at) VALUES (?, ?, ?, ?)', + ) + .bind(SITE, 'Test', 'shop.example.com', Date.now()) + .run(); +} + +/** Create an operator with `role` on the team owning SITE; returns their id and session cookie. */ +async function operator( + e: TestEnv, + email: string, + role: string, +): Promise<{ id: string; cookie: string }> { + const now = Date.now(); + const user = await upsertUserByEmail(e, email, now); + const teamId = (await userMemberships(e, user.id))[0]?.teamId as string; + await e.DB.prepare('UPDATE memberships SET role = ? WHERE team_id = ? AND user_id = ?') + .bind(role, teamId, user.id) + .run(); + await e.DB.prepare('UPDATE sites SET team_id = ? WHERE id = ?').bind(teamId, SITE).run(); + const secret = e.SESSION_SECRET as string; + return { id: user.id, cookie: `${SESSION_COOKIE}=${await signSession(user.id, secret, now)}` }; +} + +function crm(e: TestEnv, path: string, init: RequestInit = {}, cookie?: string) { + const sep = path.includes('?') ? '&' : '?'; + return app.request( + `/api/crm${path}${sep}site_id=${SITE}`, + { + ...init, + headers: { + 'content-type': 'application/json', + ...(cookie ? { cookie } : {}), + ...(init.headers ?? {}), + }, + }, + e, + ); +} + +/** The raw log, oldest first. Read straight from D1 rather than through the API, so the assertions + * do not depend on the very route they are checking. */ +async function log(e: TestEnv): Promise { + const { results } = await (e.CRM_DB as D1Database) + .prepare('SELECT * FROM crm_audit_log ORDER BY occurred_at, rowid') + .all(); + return results ?? []; +} + +beforeEach(async () => { + await seedSite(env); +}); + +describe('every route records the access before it performs it', () => { + it('writes one entry per authorized request, naming the action and the record', async () => { + const { id: userId, cookie } = await operator(env, 'admin@example.com', 'admin'); + + const created = await crm( + env, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada', external_user_id: 'ada-uid' }) }, + cookie, + ); + expect(created.status).toBe(201); + const contactId = ((await created.json()) as { contact: { id: string } }).contact.id; + + const madeCompany = await crm( + env, + '/companies', + { method: 'POST', body: JSON.stringify({ name: 'Acme' }) }, + cookie, + ); + expect(madeCompany.status).toBe(201); + const companyId = ((await madeCompany.json()) as { company: { id: string } }).company.id; + + // The rest of the surface, in one pass. `contact.create` and `company.create` are already done + // above because the ids they mint are what the other routes address. + const rest: [string, RequestInit, string, string | null][] = [ + ['/contacts', {}, 'contact.list', null], + [`/contacts/${contactId}`, {}, 'contact.read', contactId], + [ + `/contacts/${contactId}`, + { method: 'PATCH', body: JSON.stringify({ title: 'CTO' }) }, + 'contact.update', + contactId, + ], + [`/contacts/${contactId}/analytics`, {}, 'contact.analytics', contactId], + [`/contacts/${contactId}/export`, {}, 'contact.export', contactId], + ['/companies', {}, 'company.list', null], + [`/companies/${companyId}`, {}, 'company.read', companyId], + [ + `/companies/${companyId}`, + { method: 'PATCH', body: JSON.stringify({ status: 'active' }) }, + 'company.update', + companyId, + ], + [`/companies/${companyId}/contacts`, {}, 'company.contacts', companyId], + [`/companies/${companyId}/analytics`, {}, 'company.analytics', companyId], + ['/audit', {}, 'audit.read', null], + [`/contacts/${contactId}`, { method: 'DELETE' }, 'contact.delete', contactId], + [`/companies/${companyId}`, { method: 'DELETE' }, 'company.delete', companyId], + ]; + for (const [path, init, ,] of rest) { + expect((await crm(env, path, init, cookie)).status).toBe(200); + } + + const entries = await log(env); + expect(entries.map((e) => e.action)).toEqual([ + 'contact.create', + 'company.create', + ...rest.map(([, , action]) => action), + ]); + expect(entries.map((e) => e.target_id)).toEqual([ + null, + null, + ...rest.map(([, , , target]) => target), + ]); + // Every entry attributes the request to the session that made it, under the role it was + // authorized with — not the role that user holds when the log is read. + expect(entries.every((e) => e.actor_user_id === userId && e.actor_role === 'admin')).toBe( + true, + ); + expect(entries.every((e) => e.site_id === SITE)).toBe(true); + }); + + it('records every action the vocabulary declares, and declares every action it records', async () => { + // The set is closed so the log can be filtered by equality; a route with no action, or an + // action with no route, makes it a set of names that means nothing. + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + const contact = await crm( + env, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada' }) }, + cookie, + ); + const contactId = ((await contact.json()) as { contact: { id: string } }).contact.id; + const company = await crm( + env, + '/companies', + { method: 'POST', body: JSON.stringify({ name: 'Acme' }) }, + cookie, + ); + const companyId = ((await company.json()) as { company: { id: string } }).company.id; + for (const path of [ + '/contacts', + `/contacts/${contactId}`, + `/contacts/${contactId}/analytics`, + `/contacts/${contactId}/export`, + '/companies', + `/companies/${companyId}`, + `/companies/${companyId}/contacts`, + `/companies/${companyId}/analytics`, + '/audit', + ]) { + await crm(env, path, {}, cookie); + } + await crm(env, `/contacts/${contactId}`, { method: 'PATCH', body: '{}' }, cookie); + await crm(env, `/companies/${companyId}`, { method: 'PATCH', body: '{}' }, cookie); + await crm(env, `/contacts/${contactId}`, { method: 'DELETE' }, cookie); + await crm(env, `/companies/${companyId}`, { method: 'DELETE' }, cookie); + + const recorded = new Set((await log(env)).map((e) => e.action)); + expect([...recorded].sort()).toEqual([...CRM_AUDIT_ACTIONS].sort()); + }); + + it('records a read that then finds nothing, because probing ids is what a log should show', async () => { + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + const missing = '00000000-0000-4000-8000-000000000000'; + expect((await crm(env, `/contacts/${missing}`, {}, cookie)).status).toBe(404); + const entries = await log(env); + expect(entries).toHaveLength(1); + expect(entries[0]?.action).toBe('contact.read'); + expect(entries[0]?.target_id).toBe(missing); + }); +}); + +describe('an unauthorized request is not an access', () => { + it('records nothing for a caller with no session, the wrong role, or an API key', async () => { + const { cookie } = await operator(env, 'viewer@example.com', 'viewer'); + expect((await crm(env, '/contacts')).status).toBe(401); + expect((await crm(env, '/contacts', {}, cookie)).status).toBe(403); + + // The load-bearing one: a clk_ key is handed out on purpose, and it must not be able to write + // into the log any more than it can read a contact. + const { key } = await issueKey(env, SITE, null, Date.now()); + for (const path of ['/contacts', '/audit']) { + const withKey = await crm(env, path, { headers: { Authorization: `Bearer ${key}` } }); + expect(withKey.status, path).toBe(401); + } + + expect(await log(env)).toHaveLength(0); + }); + + it('records nothing for a request the rate limiter shed', async () => { + // Otherwise one stolen session could fill the audit table with entries for requests it was + // never allowed to make — the log's own denial of service. + const denying = { + ...env, + RATE_LIMITER: { limit: async () => ({ success: false }) }, + } as TestEnv; + const { cookie } = await operator(denying, 'admin@example.com', 'admin'); + expect((await crm(denying, '/contacts', {}, cookie)).status).toBe(429); + expect(await log(denying)).toHaveLength(0); + }); + + it('answers 501 without a CRM database, having nowhere to record anything', async () => { + const e = unbound(env); + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + expect((await crm(e, '/audit', {}, cookie)).status).toBe(501); + }); +}); + +describe('the log cannot be skipped', () => { + /** Break the audit table specifically, leaving the contact store intact. This is what a D1 failure + * on the log write looks like from the route's side. */ + async function breakTheLog(e: TestEnv): Promise { + await (e.CRM_DB as D1Database).exec('DROP TABLE crm_audit_log'); + } + + async function contactCount(e: TestEnv): Promise { + const row = await (e.CRM_DB as D1Database) + .prepare('SELECT count(*) AS n FROM contacts') + .first<{ n: number }>(); + return row?.n ?? 0; + } + + it('fails a read closed rather than disclosing what it could not record', async () => { + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + const created = await crm( + env, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada', email: 'ada@example.com' }) }, + cookie, + ); + const contactId = ((await created.json()) as { contact: { id: string } }).contact.id; + await breakTheLog(env); + + const res = await crm(env, `/contacts/${contactId}`, {}, cookie); + expect(res.status).toBe(500); + // The name and email never left the Worker. + expect(await res.text()).not.toContain('ada@example.com'); + }); + + it('fails a delete closed, leaving the contact for a retry that can be recorded', async () => { + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + const created = await crm( + env, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada' }) }, + cookie, + ); + const contactId = ((await created.json()) as { contact: { id: string } }).contact.id; + await breakTheLog(env); + + expect( + (await crm(env, `/contacts/${contactId}`, { method: 'DELETE' }, cookie)).status, + ).toBe(500); + // An unrecorded deletion is the one outcome an audit log exists to prevent; the row is still + // there, so the operator can retry once the log is writable again. + expect(await contactCount(env)).toBe(1); + }); +}); + +describe('the log outlives what it describes', () => { + it("keeps a contact's entries after the contact is erased", async () => { + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + const created = await crm( + env, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada', external_user_id: 'ada-uid' }) }, + cookie, + ); + const contactId = ((await created.json()) as { contact: { id: string } }).contact.id; + await crm(env, `/contacts/${contactId}/export`, {}, cookie); + expect( + (await crm(env, `/contacts/${contactId}`, { method: 'DELETE' }, cookie)).status, + ).toBe(200); + + // The entries name the contact by id and hold none of its fields, so what survives is a record + // of what operators did — not personal data the erasure should have reached. A log an operator + // can clear by deleting the row is not evidence of anything. + const entries = await log(env); + expect(entries.map((e) => e.action)).toEqual([ + 'contact.create', + 'contact.export', + 'contact.delete', + ]); + expect(entries.every((e) => e.target_id === null || e.target_id === contactId)).toBe(true); + const serialized = JSON.stringify(entries); + expect(serialized).not.toContain('Ada'); + expect(serialized).not.toContain('ada-uid'); + }); +}); + +describe('retention', () => { + /** Backdate every entry by `days`, standing in for a log that has been running that long. */ + async function ageLog(e: TestEnv, days: number): Promise { + await (e.CRM_DB as D1Database) + .prepare('UPDATE crm_audit_log SET occurred_at = occurred_at - ?') + .bind(days * DAY) + .run(); + } + + async function seedEntry(e: TestEnv): Promise { + const { cookie } = await operator(e, 'admin@example.com', 'admin'); + const created = await crm( + e, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada' }) }, + cookie, + ); + return ((await created.json()) as { contact: { id: string } }).contact.id; + } + + it('purges entries past the window and leaves the contacts they name alone', async () => { + await seedEntry(env); + await ageLog(env, 400); + expect(await enforceCrmAuditRetention(env, Date.now())).toBe(1); + expect(await log(env)).toHaveLength(0); + // Contacts are business records with their own lifecycle and are on NO schedule; only the log + // ages out. + const row = await (env.CRM_DB as D1Database) + .prepare('SELECT count(*) AS n FROM contacts') + .first<{ n: number }>(); + expect(row?.n).toBe(1); + }); + + it('keeps an entry inside the default window', async () => { + await seedEntry(env); + await ageLog(env, 300); + await enforceCrmAuditRetention(env, Date.now()); + expect(await log(env)).toHaveLength(1); + }); + + it('honours a configured window', async () => { + const e = { ...env, CRM_AUDIT_RETENTION_DAYS: '2' } as TestEnv; + await seedEntry(e); + await ageLog(e, 3); + await enforceCrmAuditRetention(e, Date.now()); + expect(await log(e)).toHaveLength(0); + }); + + it('falls back to the default rather than erasing the log on a bad value', async () => { + // A window of 0 or less puts the cutoff at or after now, so every run would wipe the entire + // log — including the entries written seconds earlier. + for (const bad of ['0', '-5', 'not-a-number', '']) { + const e = { ...env, CRM_AUDIT_RETENTION_DAYS: bad } as TestEnv; + await (e.CRM_DB as D1Database).exec('DELETE FROM crm_audit_log'); + await seedEntry(e); + await enforceCrmAuditRetention(e, Date.now()); + expect(await log(e), `window=${bad}`).toHaveLength(1); + } + }); + + it('does nothing on a deployment with no CRM database', async () => { + expect(await enforceCrmAuditRetention(unbound(env), Date.now())).toBe(0); + }); +}); + +describe('GET /api/crm/audit', () => { + it('is admin-only, because it reports on colleagues rather than on contacts', async () => { + const { cookie } = await operator(env, 'analyst@example.com', 'analyst'); + expect((await crm(env, '/audit', {}, cookie)).status).toBe(403); + // And the refusal wrote nothing: the gate records only what it authorized. + expect(await log(env)).toHaveLength(0); + }); + + it("returns one site's entries, newest first, filtered by action, actor and target", async () => { + const { id: userId, cookie } = await operator(env, 'admin@example.com', 'admin'); + const created = await crm( + env, + '/contacts', + { method: 'POST', body: JSON.stringify({ name: 'Ada' }) }, + cookie, + ); + const contactId = ((await created.json()) as { contact: { id: string } }).contact.id; + await crm(env, `/contacts/${contactId}`, {}, cookie); + // Another site's entry, written straight into the table: the list is scoped by the authorized + // site exactly as every other CRM read is. + await (env.CRM_DB as D1Database) + .prepare( + 'INSERT INTO crm_audit_log (id, site_id, actor_user_id, actor_role, action, target_id, occurred_at) VALUES (?, ?, ?, ?, ?, ?, ?)', + ) + .bind('other', OTHER_SITE, userId, 'admin', 'contact.read', contactId, Date.now()) + .run(); + + const res = await crm(env, '/audit', {}, cookie); + expect(res.status).toBe(200); + const body = (await res.json()) as { entries: AuditRow[]; total: number; role: string }; + expect(body.role).toBe('admin'); + // Newest first: this very request, then the read, then the create. + expect(body.entries.map((e) => e.action)).toEqual([ + 'audit.read', + 'contact.read', + 'contact.create', + ]); + expect(body.total).toBe(3); + expect(body.entries.every((e) => e.site_id === SITE)).toBe(true); + + const byAction = (await ( + await crm(env, '/audit?action=contact.read', {}, cookie) + ).json()) as { entries: AuditRow[]; total: number }; + expect(byAction.total).toBe(1); + expect(byAction.entries[0]?.target_id).toBe(contactId); + + const byTarget = (await ( + await crm(env, `/audit?target_id=${contactId}`, {}, cookie) + ).json()) as { total: number }; + expect(byTarget.total).toBe(1); + + const byActor = (await ( + await crm(env, `/audit?actor_user_id=${userId}`, {}, cookie) + ).json()) as { total: number }; + // Every entry on this site is this operator's; the other site's is still excluded. + expect(byActor.total).toBe(6); + + const noneSuch = (await ( + await crm(env, '/audit?actor_user_id=nobody', {}, cookie) + ).json()) as { total: number; entries: AuditRow[] }; + expect(noneSuch.total).toBe(0); + expect(noneSuch.entries).toEqual([]); + }); + + it('rejects an action outside the closed set', async () => { + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + expect((await crm(env, '/audit?action=contact.everything', {}, cookie)).status).toBe(400); + }); + + it('pages within the same bounds as every other CRM list', async () => { + const { cookie } = await operator(env, 'admin@example.com', 'admin'); + for (let i = 0; i < 3; i++) { + await crm(env, '/contacts', {}, cookie); + } + const page = (await (await crm(env, '/audit?limit=2', {}, cookie)).json()) as { + entries: AuditRow[]; + total: number; + }; + expect(page.entries).toHaveLength(2); + expect(page.total).toBeGreaterThan(2); + expect((await crm(env, '/audit?limit=101', {}, cookie)).status).toBe(400); + }); +}); diff --git a/apps/server/test/dpv.test.ts b/apps/server/test/dpv.test.ts index 2ab4974..423b8d5 100644 --- a/apps/server/test/dpv.test.ts +++ b/apps/server/test/dpv.test.ts @@ -28,6 +28,11 @@ describe('analytics-only deployment', () => { expect(claims['dpv:hasPurpose']).toBe('dpv:ServiceOptimisation'); expect(claims['dpv:hasLegalBasis']).toBe('dpv:LegitimateInterest'); expect(claims['dpv:hasTechnicalOrganisationalMeasure']).toEqual(['dpv:Pseudonymisation']); + // Nothing here is read one record at a time by a named operator, so there is no access log and + // no claim of one. + expect(claims['dpv:hasTechnicalOrganisationalMeasure']).not.toContain( + 'dpv:ActivityMonitoring', + ); }); it('names no stored personal data and no data subject, because it holds neither', () => { @@ -61,6 +66,9 @@ describe('CRM-enabled deployment', () => { expect(measures).toContain('dpv:Pseudonymisation'); expect(measures).toContain('dpv:AccessControlMethod'); expect(measures).not.toEqual(['dpv:Pseudonymisation']); + // The CRM audit log. Access control says who MAY read a contact; this says the deployment + // records who did, and it is claimed only where such a record exists to be claimed. + expect(measures).toContain('dpv:ActivityMonitoring'); }); it('names the personal data it holds and the subject it holds it about', () => { diff --git a/apps/server/wrangler.jsonc b/apps/server/wrangler.jsonc index 1263129..2fe84ae 100644 --- a/apps/server/wrangler.jsonc +++ b/apps/server/wrangler.jsonc @@ -49,6 +49,13 @@ // /.well-known/facet-privacy.json. Do not bind this and leave that // unchanged; the deployment would be signing a false statement. // + // It also starts an ACCESS LOG. Every authorized /api/crm request is + // written to crm_audit_log in this same database before its handler + // runs — reads included, since a delete leaves a hole you can see and a + // read leaves nothing. That is a third claim the attestation gains + // (dpv:ActivityMonitoring) and the one CRM table on a retention + // schedule: CRM_AUDIT_RETENTION_DAYS below, default 365. + // // To turn it on: // wrangler d1 create facet-crm // pnpm --filter @facet/server migrate:crm:remote @@ -63,6 +70,15 @@ "vars": { // Rolling retention window for raw events, in days. "RAW_RETENTION_DAYS": "90" + // Rolling retention window for the CRM access log, in days. Only read + // when CRM_DB is bound; unset means 365. Deliberately longer than the + // raw-event window — 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. + // ,"CRM_AUDIT_RETENTION_DAYS": "365" }, // Workers AI: translates natural-language analytics questions into a constrained query intent. "ai": { diff --git a/docs/api.md b/docs/api.md index da172d7..047b2e7 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1324,7 +1324,9 @@ to turn it on, and note that doing so changes the DPV claims this deployment sig record carries a structured employer that a free-text box did not). Every route is rate limited per *operator* (not per site, so one compromised session cannot hide -inside its team's traffic) and write bodies are capped at 16 KB. +inside its team's traffic), write bodies are capped at 16 KB, and every authorized request — reads +included — is written to the [audit log](#get-apicrmauditsite_idactionactor_user_idtarget_idlimitoffset-session-admin) +before its handler runs. **Auth is a session cookie, never an API key.** This is the one authenticated surface that refuses `Authorization: Bearer `. A `clk_` key authorizes aggregate analytics and is meant to be @@ -1337,7 +1339,7 @@ that. Every route takes `?site_id=`, and the caller must hold a role on th | --- | --- | | `viewer` | no access at all | | `analyst` | list, read, create, update, view the analytics link | -| `admin` / `owner` | the above, plus delete and export | +| `admin` / `owner` | the above, plus delete, export, and read the audit log | ### `GET /api/crm/contacts?site_id&status&q&limit&offset` (session, analyst) @@ -1482,6 +1484,46 @@ There is **no company export**. A data-subject export is per person by definitio would be a bulk PII dump with no data-protection meaning. Use `/companies/:id/contacts` and then the per-contact export, which accounts for each person separately. +### `GET /api/crm/audit?site_id&action&actor_user_id&target_id&limit&offset` (session, admin) + +The access log. **Every authorized request to any route above writes one entry, before its handler +runs** — reads included, which is the point: a delete leaves a hole you can see, a read leaves +nothing. An entry is `{ id, site_id, actor_user_id, actor_role, action, target_id, occurred_at }`, +and the response is `{ entries: [...], total, role }`, newest first. + +Written **first**, not last. Logging afterwards means any failure between the disclosure and the +record — a D1 error, 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 that: if the log +write fails the request fails `500` and 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 succeeded; a request that goes on to +`404` is recorded like any other. For the id-probing case that is the more useful reading anyway. + +`action` is one of `contact.list`, `contact.create`, `contact.read`, `contact.update`, +`contact.delete`, `contact.analytics`, `contact.export`, `company.list`, `company.create`, +`company.read`, `company.update`, `company.delete`, `company.contacts`, `company.analytics`, +`audit.read` — a closed set, so the log is filterable by equality. `target_id` is the contact or +company the request named, or `null` for a collection route and for a **create**, whose record does +not exist yet when the entry is written. `actor_role` is the role the request was +**authorized under**, stored rather than resolved later, so "an admin exported this" stays true after +they are demoted. All three filters are exact matches; a log of ids has no fragments to search for. + +`admin` rather than `analyst`, and not for the usual reason — no entry carries contact PII. It is +that 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. + +**Nothing can write here.** There is no update or delete route, and deleting a contact leaves its +entries standing — they name it by id and hold none of its fields, so once the row is gone the +pointer resolves to nothing and there is no personal data left for an erasure request to reach. A log +an operator can clear by deleting the contact is not evidence of anything. + +The log is the one CRM table on a retention schedule. `CRM_AUDIT_RETENTION_DAYS` (default **365**) +is enforced by the hourly cron; 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. It is 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. Contacts themselves remain on no +schedule at all. + --- ## `GET /api/health` diff --git a/packages/shared/src/crm.ts b/packages/shared/src/crm.ts index 6d53bb3..a07b5de 100644 --- a/packages/shared/src/crm.ts +++ b/packages/shared/src/crm.ts @@ -205,6 +205,48 @@ export const CompanyListQuerySchema = v.object({ /** Query for `GET /api/crm/companies/:id/contacts` — the same page bounds, no independent filter. */ export const CompanyContactsQuerySchema = v.object(pageBounds); +/** + * Every action the CRM audit log records — one per route, and the closed set is the point. + * + * A free-form action string would make the log filterable only by whatever spelling each handler + * happened to use, and "was this contact exported?" has to be answerable by equality rather than by + * guessing at synonyms. The names are `.` so a prefix filter reads as "everything + * anyone did to contacts". + * + * `audit.read` is in the set because reading the log is itself an access worth recording: it names + * which operator looked at which contact, so it is the one route whose readers a deployment most + * wants to know. + */ +export const CRM_AUDIT_ACTIONS = [ + 'contact.list', + 'contact.create', + 'contact.read', + 'contact.update', + 'contact.delete', + 'contact.analytics', + 'contact.export', + 'company.list', + 'company.create', + 'company.read', + 'company.update', + 'company.delete', + 'company.contacts', + 'company.analytics', + 'audit.read', +] as const; + +export const CrmAuditActionSchema = v.picklist(CRM_AUDIT_ACTIONS); + +/** Query for `GET /api/crm/audit`. Every filter is an exact match on a recorded column — there is no + * substring search, because the log holds ids and action names rather than anything a person would + * search for by fragment. */ +export const CrmAuditListQuerySchema = v.object({ + action: v.optional(CrmAuditActionSchema), + actor_user_id: optionalText(64), + target_id: optionalText(64), + ...pageBounds, +}); + export type ContactStatus = v.InferOutput; export type ContactCreateInput = v.InferOutput; export type ContactUpdateInput = v.InferOutput; @@ -213,3 +255,5 @@ export type CompanyStatus = v.InferOutput; export type CompanyCreateInput = v.InferOutput; export type CompanyUpdateInput = v.InferOutput; export type CompanyListQueryInput = v.InferOutput; +export type CrmAuditAction = v.InferOutput; +export type CrmAuditListQueryInput = v.InferOutput;