Skip to content

Commit 30c530e

Browse files
fix(plugin-audit): a read of the compliance ledger returns only the rows about records the caller can read (#21175) (#21194)
Fixes #21175 Clause-②: no A read of the compliance ledger (`sys_audit_log`) now returns only the rows about records the caller can read. This implements triage's ruling A on the card (`5932888473`): the ledger takes the activity stream's parent-record read gate (#20833, PR #21069), and that gate reads the engine's own answer. **Status.** Draft. The product consequence for admins (see Acceptance notes) is awaiting the maintainer's decision on #21175; this PR's behaviour is unchanged by patch round 1. ## Patch round 1: what changed since the first push - **Merged `origin/main` at `1ecb871b`** (merge commit `77d3cc05`; no rebase, no force-push). PR #21179 (#21154) is in that merge. Its `audit-plugin.ts` mount and this PR's mount sat in different hunks of the same block and merged without a conflict. The census page (`content/docs/permissions/system-context.mdx`) did conflict: row 45 now names both the query guard and the ledger read gate, and the declared counts were regenerated by `gen:system-context-census` (114 sites). - **Mount order.** Engine middleware runs in registration order. On `sys_audit_log` the order is now: (1) the query guard from #21154, (2) the ledger field redaction from #21155, (3) this PR's parent-record read gate. On `sys_activity` it is: (1) the query guard, (2) the activity read gate, (3) the activity field redaction. Each guarantee holds: - the guard judges a query before any read gate runs its pre-scan, so a refused query never pays it; - the read gate ANDs its WHERE before the read executes; - the redaction narrows the rows that come back, which are only the rows the gate kept. The mount comment in `audit-plugin.ts` states this order. - **Changeset, now in the activity gate's form** (#20833 / PR #21069): `minor`, a **BREAKING** paragraph, and `Clause-②: no (narrowing)`. The arm is there because this narrows what a read returns and no accept set widens; the PR line above keeps the claim's `no`. It carries the ADR-0087 `not-required (no-migration-prescription)` marker (`check-adr-0087-registration`: 1 declared-breaking changeset, carrying its disposition) and a Migration paragraph. - **CI red on the previous head.** The red was `Dogfood Regression Gate (3/3)`; reproduced locally on that head. The failing case was `admin-ledger-decision-metadata.dogfood.test.ts`, a pin #21174 landed on `main` after this branch's first merge. Its readers could open only their own user row through the data door (measured: each reader got 404 on the subject user). Under the parent-record rule they are served none of the subject's ledger rows, so its armed check disarmed. **Old to new:** the fixture's reader sets now add view-all on the user object, a row-scope grant only. A new armed control asserts that every reader opens the subject through the data door (measured 200 for all five). No assertion changed: its armed check still measures that each reader is withheld exactly its field class, and all 8 cases pass. - **Docs.** `content/docs/permissions/record-view-auditing.mdx`, "Reading the trail": the sentence saying ledger queries go through the data service "like any other object" now states the read rule. A view row is served outside system context only to a caller who can read its record. Views of a record that has since been deleted stay stored and are served only to system-context reads. ## Measured first, on a real boot (classes only) The re-measure was private and its readings stay in the dispatch's scratch. It ran on `main` at `b9087d77` (PR #21171 in), and again at this branch's first head. The stack was `bootStack`, org-bound, with the real `SecurityPlugin`, auth, REST and `AuditPlugin`. The rows were written by the CRUD mirror and the auth-event sink. The member holds the ledger read through one explicit permission set; the admin is the seeded admin. "Parent" means the same caller's read of the row's record through the data door. | door | caller | parent, through the data door | before | after | |:--|:--|:--|:--|:--| | list `GET /data/sys_audit_log` | member | 404: a private record's create, update and delete rows | returned | absent | | list | member | 404: other users' sessions (their `login` rows) | returned | absent | | list | member | 200 | returned | returned | | by id `GET /data/sys_audit_log/:id` | member | 404 | 200 | 404 | | list, by id | admin | 200: every existing record | returned | returned | | list | admin, member | the record no longer exists (`delete` rows, a deleted record's other rows, `logout` rows) | returned | absent | | list `total` | member | (any) | 51 | 42, the rows returned | | list `total` | admin | (any) | 51 | 47, the rows returned | ## What changed - `parent-record-read-gate.ts` (new): the activity gate's mechanism, moved out of `activity-read-visibility.ts` and parameterized per gate. It still asks `resolveReadableParentIds`, the one readability answer the comment and activity gates share. Nothing derives row scope a second way. - `activity-read-visibility.ts`: now a thin declaration over the shared module. Its exports (`parseActivityParentObject`, which the #21154 guard imports, included), scan options, sentinel and log lines are unchanged, and its unit and integration pins pass unedited. - `audit-log-read-visibility.ts` (new): `installAuditLogReadVisibility`, the ledger's declaration, with the row classes below. - `audit-plugin.ts`: one mount with its order comment, plus the no-middleware-seam warning now names the ledger read gate. - `content/docs/permissions/system-context.mdx`, row 45: the census anchor for the new early return on system context. ## Row classes: the stated answer for rows the gate cannot judge Measured per writer: - **About a record** (`object_name` + `record_id`): the CRUD mirror's create, update and delete rows; record-view `read` rows; plugin-auth's administrative create and update on a user; and `login` / `logout`, which name the session. Judged by the record gate. - **About a record that no longer exists:** no caller can read the record, so these rows are excluded for every caller that is not system context, admins included. That covers every `delete` row, every `logout` row (sign-out deletes the session, measured), and a sign-in row whose session has since been removed. The rows stay stored. - **About no record** (no `record_id`, and an action that is not a record action): the run-level `import`, `config_change` and `platform_admin_standing_change` rows, and an auth event without a session id. These are outside the gate's class and are served under the ledger's own grant, as before. **Why this differs from the activity gate**, which excludes every row that names no parent: - the activity stream has no platform producer of such rows; - these rows have three producers, and a shipped consumer reads them through the data door (the `config_changes` view, pinned by the settings dogfood test as the admin); - none of them carries a record's field values (the per-writer measurement in `audit-log-field-redaction.ts`). - **Excluded, fail closed:** a record action (`create`, `read`, `update`, `delete`) that names no record, a record under an object the engine does not know, and a row naming the ledger itself. ## Pins - `audit-log-read-visibility.integration.test.ts` uses a real kernel, the real `AuditPlugin`, the real CRUD mirror and a real SQLite driver. It covers find, findOne, count, aggregate, a scoped query, the batching count, every row class, and an admin control. 12 tests. - `audit-log-read-visibility.test.ts` covers the class rule, the fail-closed branches, the scan bound and its order pass-through, and the inert seam. 13 tests. - `packages/qa/dogfood/test/audit-log-parent-read-gate.dogfood.test.ts` covers the public doors above on a real boot. Each returned row's record is opened through the same door as the same reader. It also covers a row about no record from its real producer (a settings write), a deleted record's rows, and the admin control. 8 tests. - **Composition consequences on other cards' pins (adapted, not weakened):** - #21155's unit test now pins the redaction of rows no non-system door serves on `redactAuditLogRows` over the row at rest. Its field-values dogfood pin checks the live record's rows. - #21174's admin-ledger pin is described above. ## Ablation: put the forbidden behaviour back, red, restore Both mutations ran from committed state through `scripts/ablation-replace.mjs`, and each restore was proven: blob equals HEAD and `git diff HEAD` is empty. - **Mount removed** (the one `installAuditLogReadVisibility(...)` call in `audit-plugin.ts`; anchor 1 → 0). - Leg A, source-resolved: the integration pin gave 8 failed, 4 passed of 12. - Leg B, dogfood, resolved from `dist/`: plugin-audit was rebuilt, and `ablation-dist-preflight --absent` proved the call gone. The dogfood pin then gave 5 failed, 3 passed of 8. - Restore: a rebuild, the preflight proved the call present and the tree clean, and the pin re-ran 8 of 8. - **The class rule widened** (a record action naming no record treated as outside the gate): the unit and integration pins gave 6 failed, 19 passed of 25. - **#21174's pin without the reader view-all grant** (the previous head's fixture), on the merged build: its armed check disarmed. All five readers were served no mirror row about the subject. ## Verification at HEAD `d376985f` Every exit code was captured before any pipe. - `pnpm --filter @objectstack/plugin-audit test`: 35 files, 535 tests passed. `pnpm --filter @objectstack/plugin-audit typecheck`: exit 0, test layer included. `pnpm --filter @objectstack/dogfood typecheck`: exit 0. `plugin-approvals` `payload-predicate-guard.test.ts`: 20 passed. - **Dogfood shard 3/3** (`OS_TEST_SHARD=3/3`, CI's slice): 54 files passed, 1 skipped; 523 tests passed, 2 skipped. - Ledger and neighbour pins, 12 files: this PR's dogfood pin; #21155's `audit-log-field-values`; #21081's `activity-field-values`; #21154's `activity-text-predicate` and `audit-log-admin-search`; `activity-parent-read-gate`; `auth-session-audit-trail`; `settings-config-change-audit`; `admin-identity-audit-trail`; #21174's `admin-ledger-decision-metadata` (8 of 8 after the fixture change); `comments-permission-matrix`; `membership-actor-attribution`. All green: eleven ran at `b415f4d7`, whose tree differs from this head only in the admin-ledger pin, and that pin ran at this head. - `dispatch-gates --commands`: 94 families derived; 93 ran with exit 0 and 1 is NOT MEASURED. `spec check:skill-examples` exited 0 after the client SDK it reads was built. `check:dual-build-cjs-loads` exited 3, PREREQUISITE NOT MET: it needs a whole-repo build, and CI runs it. `dispatch-gates --ran`: 94 accounted for, 0 unrun. `main` moved again after `1ecb871b`; this round merges `main` once, as dispatched. - Lint, a declared narrowing: `eslint --no-inline-config --format json` over the 10 changed TypeScript files reports 10 files linted, 0 errors and 0 warnings. An ignored file would report a warning. `eslint.config.mjs` sets no `parserOptions.project` and no typed rule, so this diff cannot move a verdict on an untouched file. The whole-repo `pnpm lint` runs in CI. ## Acceptance notes - **Admins lose the trail of deleted records and of sign-outs** on every door that is not system context, including Setup's Audit Logs. The rows stay stored. This is the consequence awaiting the maintainer's decision on #21175. - **Interim mitigation.** `domain:engine` reports (`5936411631` on #21175) that this gate shrinks #21197's reach to admins: the gate stops serving a member the ledger rows about a key-material record that member cannot open. The admin half of #21197 stands, and that card's own mechanism is separate. - **Scan bound.** The shared 2,000-row pre-scan bound now applies to the ledger, which is a larger table than the activity stream. A broad read past the bound fails closed and logs a warning: rows outside the window, and the `total` they would add, are omitted. A record timeline scopes by `object_name` and `record_id` and never reaches the bound. - Not measured: doors onto the ledger that answer outside the engine's middleware chain (`GET /search`, the analytics dataset over the ledger, export), and a boot with more than one organization. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8dea55d commit 30c530e

13 files changed

Lines changed: 1198 additions & 143 deletions
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@objectstack/plugin-audit': minor
3+
---
4+
5+
fix(plugin-audit)!: a read of the compliance ledger returns only the rows about records the caller can read, the same way a read of the activity stream is narrowed (#21175)
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) a narrowing of what a READ returns, on one platform object, decided at request time: a sys_audit_log row that names a record (object_name, record_id) is served to a caller exactly when that caller's own engine read of the record finds it. No authorable key, spelling, value domain, export or stored metadata shape moves: the sys_audit_log object definition is byte-identical, every query shape parses as before, the package barrel exports nothing new and nothing less, and no stored row is rewritten. There is therefore nothing for an author to convert and nothing for `objectstack migrate meta` to reach. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers a read-visibility rule and this diff adds none (not registered / already-registered); and the change is runtime behaviour, not a published TypeScript declaration (not runtime-interface-only / type-surface-only). -->
10+
11+
**BREAKING**: this narrows what a read of `sys_audit_log` returns to every caller that is not system context, admins included. Some rows an admin was served before are no longer served. It ships as `minor` under the repo's launch-window convention for narrowings.
12+
13+
**What changes.** `AuditPlugin` now mounts the activity stream's parent-record read gate on the compliance ledger. It is an engine middleware, so it narrows `find`, `findOne`, `count` and `aggregate`, which on the generic data doors are the list, its `total`, the by-id read and both query shapes. A ledger row that names a record (`object_name`, `record_id`) is returned only when the caller's own engine read of that record finds it, so the parent object's sharing, RLS and object-level permissions decide. Parent reads are batched, one per parent object. The gate's mechanism is one module shared with the activity stream's gate.
14+
15+
**Rows no longer served:**
16+
17+
- to a caller who cannot read the record a row is about: that row;
18+
- to every caller that is not system context, admins included, because the gate has no readable record to judge them by:
19+
- a row about a record that no longer exists: every `delete` row, and every other row about a deleted record;
20+
- a sign-out row, and a sign-in row whose session has since been removed (sign-out removes the session the row names);
21+
- a create, read, update or delete row that names no record, a row naming an object the engine does not know, and a row naming the ledger itself.
22+
23+
**Unchanged.** The rows stay stored, and system-context reads still return every row. Rows about no record are served as before, under the ledger's own grant: `config_change` rows, the run-level user-import row, the platform-admin standing rows, and an auth event that carried no session id. A caller who can read a record keeps every row about it, and the field-level redaction of the before/after snapshots applies to the rows that are served, as before. A broad read whose pre-scan reaches the gate's 2,000-row bound fails closed and logs a warning, as the activity stream's does.
24+
25+
**Migration.** No metadata, code or configuration change is needed. A view or report that lists deletions or sign-outs from `sys_audit_log` through the data API now shows fewer rows. A server-side job that must read every ledger row reads it under system context, which this gate does not narrow.

‎content/docs/permissions/record-view-auditing.mdx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,12 @@ newest first — reachable in the Setup app under Diagnostics, via the
206206

207207
The object is append-only and exposes only `get` and `list` on the data API;
208208
every field is `readonly`, so the ledger is never written through a form.
209-
Programmatic queries go through `services.data` against `sys_audit_log` like any
210-
other object. Rows carry the ADR-0057 `audit` lifecycle class: retained hot for
209+
Programmatic queries go through `services.data` against `sys_audit_log`. Outside
210+
system context a read returns a view row only when the caller can read the
211+
record it names, so neither the list view nor a query serves a view of a record
212+
the reader cannot open, or of a record that has since been deleted. Those rows
213+
stay stored, and a system-context read still returns them. Rows carry the
214+
ADR-0057 `audit` lifecycle class: retained hot for
211215
90 days, then archived for seven years where an `archive` datasource is
212216
registered.
213217

‎content/docs/permissions/system-context.mdx‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a
1010
service self-write, a migration.
1111

1212
This page is **the authority** for what that flag actually does. It exists
13-
because the flag is not one concept: it is a single boolean read at **113
13+
because the flag is not one concept: it is a single boolean read at **114
1414
distinct sites across 19 packages**, and knowing three of those behaviours gives
1515
no hint that the other hundred-and-four exist. Every documented app-side bug
1616
traced to `isSystem` had the same shape — the metadata was complete and correct,
@@ -138,7 +138,7 @@ that silently does not happen.
138138

139139
### 3. Sharing (`plugin-sharing`)
140140

141-
The largest single consumer — **17 of the 113 sites**.
141+
The largest single consumer — **17 of the 114 sites**.
142142

143143
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
144144
|:--|:---|:---|:---|
@@ -161,7 +161,7 @@ The largest single consumer — **17 of the 113 sites**.
161161
| 42 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `packages/plugins/plugin-approvals/src/lifecycle-hooks.ts#bindDelegationWriteGuard` |
162162
| 43 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `packages/plugins/plugin-approvals/src/approval-service.ts#isOverrideActor`, `#resolveActor`, `#sendBack`, `#resubmit`, `#reassign`, `#remind`, `#requestInfo`, `#comment` |
163163
| 44 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentAccessHooks`, `#installAttachmentReadVisibility` |
164-
| 45 | Comment access hooks return early (insert + update + delete, and the read AST), and so do the activity read gate (its read AST), the activity field redaction and the audit-log field redaction (the rows a read serves), and the query guard over both objects' value-bearing columns (the read AST) | plugin-audit | Get: the whole activity row on `find` / `findOne` — the audit writer and the read gate's own pre-scan — and the whole `sys_audit_log` row, its before/after snapshots included, and a filter, sort or grouping by those columns. Lose: comment visibility scoping, the narrowing of `sys_activity` to rows whose parent record the caller can read, the redaction of a parent field's value from an activity row's text and recorded change and from a ledger row's before/after snapshots, and the refusal of a query over those columns for a reader withheld a field of the objects it can reach | `packages/plugins/plugin-audit/src/comment-access-hooks.ts#installCommentAccessHooks`, `#installCommentReadVisibility`, `packages/plugins/plugin-audit/src/activity-read-visibility.ts#installActivityReadVisibility`, `packages/plugins/plugin-audit/src/activity-field-redaction.ts#installActivityFieldRedaction`, `packages/plugins/plugin-audit/src/audit-log-field-redaction.ts#installAuditLogFieldRedaction`, `packages/plugins/plugin-audit/src/parent-field-query-guard.ts#installParentFieldQueryGuard` |
164+
| 45 | Comment access hooks return early (insert + update + delete, and the read AST), and so do the activity and audit-log read gates (their read AST), the activity field redaction and the audit-log field redaction (the rows a read serves), and the query guard over both objects' value-bearing columns (the read AST) | plugin-audit | Get: the whole activity row and the whole `sys_audit_log` row, its before/after snapshots included, on `find` / `findOne` — the audit writer and each read gate's own pre-scan — and a filter, sort or grouping by those columns. Lose: comment visibility scoping, the narrowing of `sys_activity` and of `sys_audit_log` to rows whose parent record the caller can read, the redaction of a parent field's value from an activity row's text and recorded change and from a ledger row's before/after snapshots, and the refusal of a query over those columns for a reader withheld a field of the objects it can reach | `packages/plugins/plugin-audit/src/comment-access-hooks.ts#installCommentAccessHooks`, `#installCommentReadVisibility`, `packages/plugins/plugin-audit/src/activity-read-visibility.ts#installActivityReadVisibility`, `packages/plugins/plugin-audit/src/audit-log-read-visibility.ts#installAuditLogReadVisibility`, `packages/plugins/plugin-audit/src/activity-field-redaction.ts#installActivityFieldRedaction`, `packages/plugins/plugin-audit/src/audit-log-field-redaction.ts#installAuditLogFieldRedaction`, `packages/plugins/plugin-audit/src/parent-field-query-guard.ts#installParentFieldQueryGuard` |
165165
| 46 | Knowledge search returns hits unfiltered | service-knowledge | Lose: the permission filter over search results | `packages/services/service-knowledge/src/knowledge-service.ts#applyPermissionFilter` |
166166

167167
### 5. Actions, metadata plane, provenance, the organization wall
@@ -281,7 +281,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are
281281
independent decisions, and a seed loader plausibly wants the first two but not
282282
the third. The concept is nevertheless **staying as one boolean**:
283283

284-
- **Shipped semantics.** `isSystem` is a published contract with 113 read sites
284+
- **Shipped semantics.** `isSystem` is a published contract with 114 read sites
285285
in 19 packages. Splitting it is a breaking contract change across all of them.
286286
(The ruling was taken when the census read 80 sites in 18 packages; the count
287287
has grown, which strengthens rather than weakens the argument.)
@@ -355,16 +355,16 @@ still holds equal to the census on every pull request:
355355
| Appearances of the bare identifier `isSystem` in non-test sources | 813 | — |
356356
| — parsed as a declaration | 23 | ✅ |
357357
| — parsed as an object-literal / type key (producers and option objects) | 310 | — |
358-
| — parsed as a property **read** | 119 | ✅ |
358+
| — parsed as a property **read** | 120 | ✅ |
359359
| — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ |
360360
| — the remainder: text inside comments and string literals | 358 | — |
361361
| Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ |
362-
| Of those reads: reads of `ExecutionContext.isSystem` | **113** | ✅ |
363-
| — behaviour-bearing (rows 1–61 above) | 110 | ✅ |
362+
| Of those reads: reads of `ExecutionContext.isSystem` | **114** | ✅ |
363+
| — behaviour-bearing (rows 1–61 above) | 111 | ✅ |
364364
| — carry the flag onward only (rows 62–64 above) | 3 | ✅ |
365365
| Packages containing at least one elevation read | **19** | ✅ |
366-
| Files containing at least one elevation read | 51 | ✅ |
367-
| — the distinct symbols those reads live in — what this page anchors | 96 | ✅ |
366+
| Files containing at least one elevation read | 52 | ✅ |
367+
| — the distinct symbols those reads live in — what this page anchors | 97 | ✅ |
368368
| — of those files, the ones holding more than one read in one symbol | 8 | ✅ |
369369

370370
The six rows marked — are a **dated decomposition, not a live claim**: they were
@@ -428,7 +428,7 @@ same resolver, and the same registration shape, that holds `docs/adr/**`.
428428
Renaming a symbol is now a loud red instead of a silent misdirection.
429429

430430
⚠️ **The precision that costs, priced here rather than buried.** A symbol anchor
431-
cannot say WHICH read inside a function it means, and **8** of the **51**
431+
cannot say WHICH read inside a function it means, and **8** of the **52**
432432
anchored files hold more than one read inside a single symbol. So the population
433433
check runs per file at symbol granularity: every file the census finds a read in
434434
must be anchored, and the set of symbols this page cites into that file must

0 commit comments

Comments
 (0)