From 4435acf6627630b3ba1df4c022cbfd7f26184819 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 08:14:15 +0000 Subject: [PATCH] docs(plugin-audit): the published README documents the record-view audit surface that shipped (#9517) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #9531 corrected this README against the shipped surface while record-view auditing was still an unmerged draft, so it correctly refused to describe it. That work has since landed (#8992 via PR #9515), which made two of the page's statements false: "reads and views are not on the ledger", and that the plugin takes no configuration. The page now documents the surface that exists, each point measured against the source rather than against a description of it: the `read` action and its writer in the action table, the `record_views` list view, the record-detail discriminator (one materialized record plus a primary-key pin, `$or`/`$not` refused) that keeps list and search reads out of scope, the batched off-request-path writes with the view-instant `created_at` and the two loud once-only failure postures, and the two declared boundaries — a system-elevated read and a read with no principal both write no row. The opt-in is documented as what it is: an INSTALL-TIME list on the plugin constructor, explicitly not an `enable.auditReads` object-metadata key. That spelling was ruled against on #8992 for the reason this card exists — a declarable key can be set on an object in a deployment that never installs the plugin, producing metadata that reads as audited and writes nothing. Two things the page now says that only reading the code shows: `maxBufferedEvents` is a writer knob the plugin does not forward, and the shipped `record_views` view carries an `ip_address` column that is always empty on a `read` row because no read-path writer stamps it. Record-view auditing adds no enterprise dependency — this package's declared edition is `open` — so nothing new is annotated under that pattern; the two existing annotations are unchanged. A changeset is owed because the README ships in the package's `files` array: a docs-only correction with no version bump never reaches the npm package page. Co-authored-by: Claude --- .changeset/mighty-ducks-repeat.md | 40 +++++ packages/plugins/plugin-audit/README.md | 190 ++++++++++++++++++++++-- 2 files changed, 217 insertions(+), 13 deletions(-) create mode 100644 .changeset/mighty-ducks-repeat.md diff --git a/.changeset/mighty-ducks-repeat.md b/.changeset/mighty-ducks-repeat.md new file mode 100644 index 0000000000..39dcebe9e1 --- /dev/null +++ b/.changeset/mighty-ducks-repeat.md @@ -0,0 +1,40 @@ +--- +"@objectstack/plugin-audit": patch +--- + +Published README documents the record-view audit surface that actually shipped + +The README was corrected against the shipped surface before record-view auditing landed, +so it still told readers that "reads and views are not on the ledger" and that the plugin +takes no configuration. Both became false when the `read` action, its writer and the +`record_views` list view merged. This is a docs-only change; it needs a version bump +because the README is in the package's published `files` array, and a correction with no +release never reaches the npm package page at all. + +What the page now documents, each point verified against the source rather than against a +description of it: + +- the `read` action and its writer, in the action table and in the shipped list views; +- the per-object opt-in as what it is — an **install-time list** passed to the plugin + constructor (`new AuditPlugin({ readAudit: { objects: [...] } })`), explicitly **not** an + `enable.auditReads` object-metadata key. A declarable key can be set on an object in a + deployment that never installs the plugin, producing metadata that reads as audited and + writes nothing, which is the exact class of claim this page was corrected to remove; +- the three settings the plugin forwards, and the one writer knob (`maxBufferedEvents`) it + does not, which is reachable only by calling `installReadAuditWriter` directly; +- the record-detail discriminator that keeps list and search reads out of scope, including + the `$or` / `$not` refusal and the AND-composed predicate the security middleware leaves + behind; +- batched writes off the request path, the view-instant `created_at`, and the two loud + once-only failure postures (buffer overflow, failed ledger write); +- the two declared boundaries — a system-elevated read and a read with no principal both + write no row; +- that no field values are recorded, and therefore that the ledger cannot answer what a + viewer actually saw; +- that the shipped `record_views` view carries an `ip_address` column which is always empty + on a `read` row, because no read-path writer stamps it. + +Record-view auditing adds no enterprise dependency: this package's declared edition is +`open`, and the opt-in is ordinary plugin configuration. The two enterprise-dependent +behaviours already annotated on the page — the hierarchy resolver and the archive +datasource — are unchanged. diff --git a/packages/plugins/plugin-audit/README.md b/packages/plugins/plugin-audit/README.md index 8d8928c2ba..15d77c3bae 100644 --- a/packages/plugins/plugin-audit/README.md +++ b/packages/plugins/plugin-audit/README.md @@ -5,11 +5,14 @@ System audit-trail objects for ObjectStack: the immutable `sys_audit_log` ledger ## What this package actually does -`AuditPlugin` does two things when the kernel starts it: +`AuditPlugin` does three things when the kernel starts it: 1. **Registers three system objects** — `sys_audit_log`, `sys_activity`, `sys_comment`. 2. **Installs ObjectQL hook subscribers** that write ledger and activity rows on data mutations, plus record-level access gates for `sys_comment`. +3. **Installs the record-view writer** — an `afterFind` hook that records `read` rows for + the objects a deployment opted in, and is not installed at all when nothing is opted + in. See [Record-view auditing](#record-view-auditing--the-read-action). ⚠️ **There is no audit service you call to log a record change.** Record-level audit rows are produced by `afterInsert` / `afterUpdate` / `afterDelete` hooks, not by application @@ -24,7 +27,8 @@ pnpm add @objectstack/plugin-audit ## Usage -The plugin is a class, registered on the kernel. It takes no configuration: +The plugin is a class, registered on the kernel. Write and activity auditing take no +configuration at all: ```typescript import { AuditPlugin } from '@objectstack/plugin-audit'; @@ -32,8 +36,30 @@ import { AuditPlugin } from '@objectstack/plugin-audit'; await kernel.use(new AuditPlugin()); ``` -That is the whole setup surface. Coverage is not configured per object — see -[Coverage](#coverage-subtraction-not-an-allow-list). +Write coverage is not configured per object — see +[Coverage](#coverage-subtraction-not-an-allow-list). The one thing that *is* configured is +**record-view auditing**, which records nothing until objects are named: + +```typescript +await kernel.use( + new AuditPlugin({ + readAudit: { objects: ['contact', 'account'] }, + }), +); +``` + +`readAudit` is the only key `AuditPluginOptions` declares, and it accepts exactly three +settings, no others: + +| Option | Default | Meaning | +|---|---|---| +| `readAudit.objects` | `[]` | The closed per-object opt-in. Empty installs no hook | +| `readAudit.maxBatchSize` | `50` | Flush once this many views are buffered | +| `readAudit.flushIntervalMs` | `2000` | Flush this long after the first view of a batch | + +⚠️ The writer itself has one more knob — `maxBufferedEvents` (default `10000`) — that the +plugin does **not** forward. Setting it requires calling `installReadAuditWriter` directly +against the engine; there is no plugin-level spelling for it. The plugin depends on the ObjectQL engine (`com.objectstack.engine.objectql`) and resolves it at `kernel:ready`. If no engine is available it logs a warning and installs no writers. @@ -46,6 +72,7 @@ these are the only values that are ever written: | `action` | Written by | On | |---|---|---| | `create` | `installAuditWriters` (this package) | `afterInsert` | +| `read` | `installReadAuditWriter` (this package) | `afterFind`, on opted-in objects only, and only for record-detail views | | `update` | `installAuditWriters` (this package) | `afterUpdate`, only when the diff is non-empty | | `delete` | `installAuditWriters` (this package) | `afterDelete` | | `login` | `createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth` | session start | @@ -65,7 +92,7 @@ written only by internal system hooks running under `sudo()`, never through UI f |---|---|---| | `id` | text | Audit log entry id | | `created_at` | datetime | When the action occurred (`NOW()` default) | -| `action` | select | One of the seven values above | +| `action` | select | One of the eight values above | | `user_id` | lookup → `sys_user` | Null for non-user / service actions | | `actor` | text | Principal label: a user id, `svc:`, or null. Attributes service-token writes that `user_id` structurally cannot hold | | `object_name` | text | Target object, e.g. `sys_user` | @@ -83,9 +110,15 @@ A credential *rotation* still produces a row — the raw values are compared for detection before masking is applied — so the audit trail of a secret change survives without the secret itself reaching the ledger. -**`ip_address` / `user_agent` are populated on auth events only.** The record-level writer -does not stamp them: a `create` / `update` / `delete` row records who and what, not from -where. Do not read a null client fingerprint on a CRUD row as "the request had none". +**`ip_address` / `user_agent` are populated on auth events only.** Neither the +record-level writer nor the record-view writer stamps them: a `create` / `update` / +`delete` / `read` row records who and what, not from where. Do not read a null client +fingerprint on such a row as "the request had none". ⚠️ The shipped `record_views` list +view carries an `ip_address` column, and on a `read` row that column is **always empty** +for this reason. + +**`old_value` / `new_value` are null on every `read` row**, deliberately and not as an +omission — see [Record-view auditing](#record-view-auditing--the-read-action). ## Coverage: subtraction, not an allow list @@ -110,16 +143,131 @@ Excluded objects fall into two groups: The exclusion list is one definition consumed on both the registration face and inside the handlers, so the two cannot drift. +⚠️ **Read coverage is the opposite shape** — a closed opt-in, not subtraction. The two are +not inconsistent: for writes, an object nobody remembered to list is one whose changes go +unrecorded, so the safe default is "audited"; for reads, the same default would record +every record anyone opens on every object in the system, burying the views an auditor is +actually looking for and charging every read for it. Read coverage is therefore +enumerated, and the exclusion list applies on top of it — an excluded object cannot be +opted in. + +## Record-view auditing — the `read` action + +Answers "who viewed this record, and when?". Nothing is recorded until a deployment names +the objects it wants recorded. + +### The opt-in is an install-time list, not a metadata key + +⛔ There is no `enable.auditReads` object-metadata key and no global switch. The audited +set is a constructor argument, given once at the place the plugin is installed: + +```typescript +new AuditPlugin({ readAudit: { objects: ['contact', 'account'] } }); +``` + +That shape is deliberate. A declarable metadata key can be set on an object in a +deployment that never installs this plugin — producing a metadata file that *reads* as +audited and writes nothing. A declaration a compliance reviewer mistakes for coverage is +worse than an absent feature, and one input at the point of installation cannot make that +claim. + +`installReadAuditWriter` filters the list before it registers anything: + +- names on the audit exclusion list above are **dropped with a warning** naming the object, + not silently accepted — the list is derived from the write-side exclusions rather than + re-typed, so the two cannot disagree; +- duplicates and blanks are removed, and the returned handle reports `auditedObjects`, + i.e. what was actually registered rather than what was asked for; +- an empty (or fully excluded) set registers **no hook at all**, so a deployment that opts + nothing in pays nothing on its read path. + +### Only record-detail views produce a row + +A read is recorded when **both** hold: + +1. it materialized exactly one record — `findOne` returning a record, not `find` returning + an array (an array, `null` or `undefined` result is never a detail view); and +2. its predicate **pinned the primary key**. `GET /data/:object/:id` reaches the engine as + `findOne(object, { where: { id } })`, which is the record-detail surface. A `findOne` + carrying any other predicate is "give me *a* matching record" — an internal lookup, not + someone opening a record. + +The predicate walk tolerates what the security middleware leaves behind: an `id` equality +AND-composed with an RLS/tenant clause still counts, and the explicit `{ id: { $eq: … } }` +spelling is accepted. `$or` or `$not` anywhere on the path **disqualifies** the read — the +row may have matched through the other arm, so the id equality no longer proves the read +was for that record. Nesting is walked to a fixed depth of 8. + +⇒ **List and search reads are never recorded**, including a list read that happened to +return exactly one record. List auditing is a deferred follow-up, and a deferral that +leaked rows anyway would not be one. + +### Ledger writes happen off the request path + +The hook **enqueues and returns**; it awaits nothing. Rows are persisted on a later tick by +a batcher, flushed whichever comes first — `maxBatchSize` views buffered (default 50) or +`flushIntervalMs` since the batch's first view (default 2000ms) — and the plugin's +`destroy()` drains the tail so a clean shutdown does not take the last batch with it. + +`created_at` on each row is the instant the record was **viewed**, not the instant its +batch drained. Batching would otherwise stamp a whole batch with one flush timestamp, and a +ledger that answers "when did they look?" with the time its own buffer emptied is wrong by +up to the flush interval. + +Two failure postures, both loud once and never retried: + +- **Buffer overflow.** Past `maxBufferedEvents` (default 10000) the **oldest** buffered + views are dropped and a `warn` is logged once. The reads all still succeeded and returned + 200, so nothing else reports the hole. +- **A failed ledger write.** The batch is lost, an `error` is logged once, and the read is + unaffected — an audit write must never turn a valid read into an error, and retrying in a + loop against an unreachable table turns a degradation into an outage. + +### What the row contains — and what it deliberately does not + +A `read` row carries `action: 'read'`, the view instant on `created_at`, `user_id`, +`actor`, `object_name`, `record_id`, and `tenant_id` — the viewed record's own +organization, falling back to the viewer's session organization, so a row about an org-A +record does not land behind org B's tenant wall. In multi-tenant mode, where the platform +injects an `organization_id` column onto `sys_audit_log`, the same value is stamped there +too: that column is what the row-level tenant wall gates on, and an unstamped row is one +non-admin members can never see. + +⛔ **No field values are ever recorded.** `old_value` and `new_value` stay `null`. The +`afterFind` hook runs *inside* the security middleware, ahead of its field masking, so the +record it sees is pre-mask plaintext; copying values in would mint a plaintext copy of +exactly what field-level security withholds, inside the one table compliance staff are +granted broad access to. It follows that the ledger does not record *what the viewer +actually saw* — only that they opened the record. + +Two boundaries are declared rather than left to be discovered: + +- **A system-elevated read writes no row.** Anything carrying `session.isSystem` — an + `api.sudo()` path, a formula recompute, a roll-up, a trigger — is the platform reading + for its own bookkeeping, not a person opening a record. Note `sudo()` keeps the caller's + user id, so this flag is the only thing separating the two. +- **A read with no principal writes no row.** With neither a user id nor an actor there is + no answer to "who", and a row naming nobody only adds noise to the one query this + capability exists to serve. + +Rows surface in the shipped `record_views` list view. + ## What the ledger does not record Stated explicitly, because a gap in an audit surface is easily mistaken for coverage: -- **Reads and views are not on the ledger.** No writer emits a read action, and the record - writers subscribe only to `after*` write events. `sys_audit_log` answers "who changed - this record", not "who looked at it". +- **Reads are on the ledger only where they were opted in, and only as record-detail + views.** An object absent from `readAudit.objects` produces no `read` row at all, and no + list or search read produces one on any object. See + [Record-view auditing](#record-view-auditing--the-read-action) for the full scope. +- **What a viewer actually saw is not on the ledger.** A `read` row records who opened + which record; it carries no field values, so it cannot answer which fields were visible + to that viewer after masking. - **Failed operations are not on the ledger.** There is no success/failure column, and the - writers fire only on `after*` events — that is, only on operations that succeeded. A - failed write is not distinguishable from an absent one here. + write hooks fire only on `after*` events — that is, only on operations that succeeded. A + failed write is not distinguishable from an absent one here. The same holds for reads: + the `afterFind` hook is reached only by a read that succeeded, so a refused read leaves + no trace. - **Field-level read access is not on the ledger.** ## Reading audit rows @@ -133,6 +281,7 @@ the standard object API or `services.data`, and through the shipped list views: | `recent` | Everything, newest first | | `writes_only` | `create` / `update` / `delete` | | `auth_events` | `login` / `logout` | +| `record_views` | `read` — who opened which record | | `config_changes` | `config_change` / `import` | | `all_events` | Everything, larger page size | @@ -198,6 +347,14 @@ those checks degrade to parent-record read visibility. If the engine exposes no seam, `sys_comment` read visibility is not installed at all and the plugin logs a warning naming the consequence. +**Record-view auditing adds no enterprise dependency.** The platform capability registry +declares this package's edition as `open`; `read` rows are written by this package, and the +opt-in is ordinary plugin configuration, so nothing about the capability degrades on an +open build. The boundary above still applies to `read` rows the same way it applies to +every other row — they are read through the same permission system, so a grant written with +a hierarchy-relative scope shows a manager only their own record-view rows without the +enterprise resolver. + ## Exports ```typescript @@ -207,6 +364,13 @@ export { AuditPlugin }; // Writer installation + test probe export { installAuditWriters, createFieldPresenceProbe }; +// Record-view auditing (the `read` action) +export { installReadAuditWriter, createReadAuditBatcher, extractDetailReadId, READ_AUDIT_ACTION }; +export type { + ReadAuditBatcher, ReadAuditBatcherOptions, ReadAuditEvent, ReadAuditLogger, + ReadAuditTimers, ReadAuditWriterHandle, ReadAuditWriterOptions, +}; + // Auth-event ingress (the `audit` service slot) export { createAuthEventAuditSink }; export type {