forked from EduCraft/curriculum-project-hub
26523d1b54
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
141 lines
6.9 KiB
Markdown
141 lines
6.9 KiB
Markdown
# ADR 0039: File Library Audit Log Is A Cross-Cutting Append-Only Module
|
|
|
|
## Status
|
|
|
|
Accepted.
|
|
|
|
## Context
|
|
|
|
Audit was previously a single sink function (`filelib/audit.ts`) writing rows into
|
|
the shared `AuditEntry` table: `action` + `actorUserId` + `organizationId` + a free
|
|
`metadata` JSON blob. It satisfied "the write happened", nothing more.
|
|
|
|
The file-library requirements ask for materially stronger properties across the
|
|
version-management and permission-management submodules: structured before/after
|
|
values, actor name snapshot, client IP/User-Agent, success/failure with a reason,
|
|
conflict-detection events carrying base and conflicting versions, combined
|
|
multi-dimension query with export, three-tier read visibility, tamper resistance,
|
|
and a retention floor of 180 days.
|
|
|
|
Three of those cannot be expressed on the old shape at all:
|
|
|
|
- **Failure results.** Every audit write sat inside the business transaction, so a
|
|
failed operation rolled its own audit row back. "Operation result = failure and
|
|
reason" was structurally unrecordable. This also silently dropped
|
|
`file.conflict_detected`, an event the requirements name explicitly — it was
|
|
written and then rolled back with the 409.
|
|
- **Query by dimension.** Object type, object id, result, and path lived inside the
|
|
`metadata` JSON. Filtering on them means JSON path scans, and there is no index
|
|
to hang on them.
|
|
- **Tamper resistance.** `AuditEntry` is documented as best-effort telemetry
|
|
(ADR-0023) and is Project/Run oriented. It carries no hash chain and no
|
|
append-only enforcement.
|
|
|
|
ADR-0023 already fixed that privileged *platform* audit must be separate from
|
|
customer Project/Run audit. It did not cover the file library's own submodule
|
|
audit, which is the subject here.
|
|
|
|
## Decision
|
|
|
|
### Separate store
|
|
|
|
File-library audit gets its own table, `FileLibAuditLog`, distinct from both
|
|
`AuditEntry` (Project/Run, best-effort) and Platform Audit Entry (ADR-0023).
|
|
Every required field is a real column: `occurredAt` (timestamptz), `actorUserId`
|
|
plus `actorName` snapshot, `actorIsAdmin`, `objectType`/`objectId`/`objectName`/
|
|
`objectPath`, `beforeValue`/`afterValue`/`context` (JSONB), `result` and
|
|
`failureReason`, `clientIp` and `userAgent`, and the chain fields `seq`,
|
|
`entryHash`, `prevHash`.
|
|
|
|
`actorName` is a snapshot, not a join: audit records who acted under the name they
|
|
had at the time. Object references carry no foreign key — a purged object's history
|
|
must survive the object.
|
|
|
|
### Two write paths with opposite error semantics
|
|
|
|
- **Success** writes inside the caller's business transaction and **does not
|
|
swallow errors**. If the audit row cannot be written, the business operation
|
|
rolls back. This is the implementation of "operation succeeded ⟹ log exists".
|
|
- **Failure and conflict-detection** write out-of-band on an independent
|
|
connection, and **do swallow errors**. The business transaction has already
|
|
rolled back, so a same-transaction write would vanish; and an audit failure at
|
|
that point must not turn a 409 into a 500.
|
|
|
|
The two paths' error handling is deliberately inverted. Unifying them breaks one
|
|
guarantee or the other.
|
|
|
|
### Tamper resistance
|
|
|
|
Three independent layers, in order of what each defends against:
|
|
|
|
1. Service paths only INSERT and SELECT.
|
|
2. A `BEFORE UPDATE OR DELETE` trigger rejects every row change except a
|
|
write-once `archivedAt` stamp; a `BEFORE TRUNCATE` statement trigger blocks the
|
|
row-trigger bypass. This holds even when the application layer is wrong.
|
|
3. A sha256 hash chain: `entryHash = sha256(prevHash + canonical payload)`, linked
|
|
per organization by a monotonic `seq`. The chain is recomputable offline, so
|
|
changes made by bypassing the service entirely — direct DB access, restoring a
|
|
doctored backup — still surface as a break.
|
|
|
|
Payload canonicalization sorts object keys. Without it the same record serialized
|
|
in a different key order hashes differently and verification reports phantom
|
|
tampering.
|
|
|
|
### Retention
|
|
|
|
Default retention is 180 days and that is a floor, not a default:
|
|
`HUB_FILELIB_AUDIT_RETENTION_DAYS` may raise it and cannot lower it. Expired
|
|
records are **archived by stamping `archivedAt`, never physically deleted** —
|
|
consistent with the DELETE trigger. Archived records drop out of default queries
|
|
but remain readable with an explicit flag, and remain in the hash chain. The
|
|
archive run itself is audited.
|
|
|
|
### Read visibility
|
|
|
|
Three tiers, resolved entirely in the query layer so no second authorization
|
|
implementation can drift from it:
|
|
|
|
- **Website administrator** (silo org OWNER/ADMIN, D19): the whole system.
|
|
- **MANAGE holder**: logs for nodes they hold MANAGE on **and their subtrees**,
|
|
matched by `objectPath` prefix against the `pathIds` materialized path. File
|
|
logs use `<pathIds>:<filePath>`, so subtree matching covers them without a
|
|
second rule.
|
|
- **Everyone else**: an empty result, not a 403 — consistent with D8's refusal to
|
|
leak existence.
|
|
|
|
Group logs use `group:<id>` as their `objectPath`. That is outside the node path
|
|
space, so they are visible to website administrators only.
|
|
|
|
### Module boundary
|
|
|
|
The audit capability lives in `src/database/audit/` and imports no file-library
|
|
business type. Business services depend on it through one adapter,
|
|
`filelib/audit.ts`, which translates domain concepts (FileLibActor, node kind,
|
|
pathIds) into audit ones. The dependency is one-directional, so the module can be
|
|
replaced by a standalone audit service without touching business code.
|
|
|
|
Client IP and User-Agent are captured once in the HTTP guard and travel on the
|
|
actor. Business services never see the request object.
|
|
|
|
## Consequences
|
|
|
|
- `AuditEntry` remains for Project/Run telemetry. File-library audit no longer
|
|
writes to it; the dashboard's recent-activity feed reads the new table, which is
|
|
why it can show actor and object names without a join.
|
|
- Audit failure can now fail a business write. That is the intended trade: a
|
|
silently unlogged permission change is worse than a failed one.
|
|
- Failure records exist without a corresponding state change. Consumers must read
|
|
`result` — counting rows per action no longer counts successful operations.
|
|
- The hash chain serializes audit writes per organization through a
|
|
`max(seq) + 1` read. Under the alpha Silo's one-org-one-process deployment
|
|
(ADR-0025) contention is negligible; the unique constraint on
|
|
`(organizationId, seq)` makes a collision a retry rather than a corrupt chain.
|
|
A future high-write deployment will need a sequence or a per-org writer.
|
|
- Changing the set of fields fed into `computeEntryHash` invalidates every
|
|
existing chain. It is a breaking change requiring a new ADR and a re-anchoring
|
|
procedure, not a refactor.
|
|
- The requirement names "project independent permission enable/disable/change".
|
|
ADR-0030 removed that feature — `FileLibProjectSettings` is no longer read — so
|
|
no code path can emit those events today. The three action values are reserved
|
|
in the vocabulary; if the feature returns, it wires into the existing writer.
|