Skip to content

Commit 3bddd4a

Browse files
fix(plugin-audit): an update activity row whose every recorded change is withheld from the reader is withheld as a row, on every listing face (#21388) (#21427)
Fixes #21388 Clause-②: no ## What changes An activity-stream (`sys_activity`) row that records an UPDATE, whose stored change had keys, every one of which the reader is withheld, is now withheld from that reader as a row. Before, the field redaction narrowed the change key by key, and the row still reached the reader with an empty change, a summary, an actor and a timestamp. That is how an org peer read when each of a colleague's sign-ins happened. - **An update row** is one whose stored change has both sides (`metadata.old` and `metadata.new` are records). A create (`old` null) and a delete (`new` null) keep their rows. - **"Had keys"** reads the STORED change, never the redacted one. A row whose stored change is empty on both sides (an update that touched only `internal` fields, which the writer omits) is empty for every reader. It is unaffected. - **"Withheld"** is the answer the redaction already narrows by: `resolveServedFields`, the security contract's read projection intersected with its query-side answer. One answer serves both, so a row is withheld exactly when the redaction would leave its change empty. A reader the service gives no answer for is narrowed by neither. ⛔ No object or field is named in the rule. - **Every face agrees.** The rule is a WHERE, not a post-read drop, and it is built the way the parent-record read gate builds its own. A SYSTEM pre-scan of the rows the query would touch (caller's WHERE and order, bounded at the gate's 2,000) judges each row. The withheld ids are ANDed out with `{ id: { $nin: WITHHELD } }`, on `find`, `findOne`, `count` and `aggregate`. So the list's `total`, its pages and `hasMore`, a by-id read and a grouped count agree with the rows served. A pre-scan that reaches its bound fails closed, the way the read gate's does. The read is answered from the judged rows only (`{ id: { $in: KEPT } }`), with one warn naming the remedy. A pre-scan failure denies the read. **Where it sits.** The rule lives in `activity-field-redaction.ts`, in the redaction's own middleware. That middleware already holds the per-read security resolver, and `AuditPlugin` already registers it after the read gate's, so its pre-scan reads the WHERE the gate has already narrowed. The pre-read step and the post-read redaction share ONE per-read served-fields answer. `activity-read-visibility.ts` gains a header paragraph that points at it. No `audit-plugin.ts` edit, no `audit-writers.ts` edit, no writer change. ## Measured first, on `main` at `6d67ad5ec`, at the HTTP door Real boot (showcase, `SecurityPlugin` with the platform sets plus one object-level `sys_activity` read set, `AuditPlugin`). The setup is the card's: one org whose members are the platform admin, a member holding the activity read set (`member_default` otherwise), and a colleague who signs in twice through `POST /auth/sign-in/email`. The reads are of the colleague's identity record's activity rows. | Face, as the member | `main` (`6d67ad5ec`) | this branch | |---|---|---| | `GET /data/sys_activity` filtered to the record: rows / `total` | 4 / 4. Both sign-in stamp rows are served with an empty change, the summary, the actor and the timestamp | 1 / 1 (the create row) | | `$top=1` walking `$skip` | stamp rows on pages 3 and 4; `total` 4 on every page | one page; `total` 1 | | `GET /data/sys_activity/ID` for each stamp row | 200 | 404 | | `POST /data/sys_activity/query`, filtered | 4 rows, `total` 4 | 1 row, `total` 1 | | `POST /data/sys_activity/query`, grouped count by `type` | created 1, updated 3 | created 1 | | the admin, every face above | 4 rows, the stamp rows carrying `last_login_at` | unchanged | - **Cursor:** the data door has none. The engine tombstones the `cursor` query key, and the door pages by `$top` / `$skip` and reports `total` / `hasMore`. Those are measured above. - **Activity-feed route:** none outside the data door. The `feed` service was removed (ADR-0052 §5), and the timeline reads `sys_activity` through the data door. - **The NOT MEASURED premise, measured.** A sign-in DOES move the identity row's `updated_at`: both sign-ins moved it. The member's direct read of the colleague row serves `updated_at`, and it equals the latest stamp. So the LATEST sign-in time stays readable through the direct read, until the next write of that row. That field is the record read's, not this card's. What this PR closes is the HISTORY. - The colleague's sign-up also writes a withheld-only update (`password_changed_at`). On `main` it was served to the member as an empty row too. ## The raise-rule measurement (for the seat's re-grade) The other withheld-only update classes the writers produce were measured on `main` at the same door, on the same colleague. A lockout threshold was applied through `applyConfigPatch`, the way the settings service applies one. | Write | Recorded keys | Served to the member on `main` | |---|---|---| | failed sign-in (the counter bump) | `failed_login_count` | yes, as an empty row (once per failed attempt) | | successful sign-in after a failure (the counter reset) | `failed_login_count` | yes, as an empty row | | lockout (the attempt that reaches the threshold) | `failed_login_count`, `locked_until` | yes, as an empty row | | password change (`POST /auth/change-password`) | `password_changed_at` | yes, as an empty row | | MFA-required stamp (the writer's patch shape, written as the system) | `mfa_required_at` | yes, as an empty row | | ban (set, with reason and expiry) | `banned`, `ban_reason`, `ban_expires` | yes, WITH `banned`. Not in this class: the deactivation flag is directory status, served by design | The lockout class, the failed-sign-in attempts under it, the password change and the MFA stamp are withheld-only update classes whose timing is sensitive beyond sign-ins. Per triage's raise rule, that reads **p2**. The re-grade is the seat's. On this branch, all of them except the ban are withheld from the member as rows. The ban stays served, with `banned` only. ## Pins - `packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts` (19 cases). It uses a real engine, SQLite, `AuditPlugin`, rows written by the real CRUD mirror, and a security double standing in for the field answer. It covers: - the card's three pins: the member gets no row, the admin gets the row with its change, a mixed update keeps the served key; - the empty-for-everyone control, which the writer produces from an `internal`-only update and the scene asserts is empty at rest; - a create that keeps its row when every key is withheld; - the count pin, plus `aggregate`, a one-row page walk and `findOne`, all agreeing with `find`; - a system read; - the predicate's shapes; - the bound: `$nin` under it, `$in` of the judged-kept rows at it, with the warn; the pre-scan reads as the system, in the caller's order. - `packages/qa/dogfood/test/activity-withheld-update.dogfood.test.ts` (6 cases, real boot, HTTP door). Two sign-in stamps, a failed-sign-in counter bump and a mixed rename. The member is served none of the withheld-only rows on list / `total` / `hasMore`, on a one-row page walk, by id (404) and on the query and grouped-count faces. The mixed row is served with `name` only. The admin keeps every row with its change, and its `total` is the member's plus the withheld rows. The setup is guarded by armed checks. - **Two existing pins the ruled behaviour made false keep their intent** (`activity-field-redaction.test.ts`): - The earlier-mirror-shape row now carries a MIXED change, so its "restricted reader keeps the row without text, narrowed key by key" intent still measures that. A change withheld whole is the new file's subject. - The fail-closed reader (served no field) still keeps no value composed from a field. It keeps the create row and is now withheld every update row whose change had keys. ## Ablations Each was run from committed state `e08662d24` through `scripts/ablation-replace.mjs`. Each mutation was proven on disk (anchor x1 to x0, replacement x0 to x1, blob changed), and each was restored with `git diff HEAD` empty and the disk blob equal to the HEAD blob (`85e460841688`). The subject is imported by relative path (`./audit-plugin.js`), so these runs read `src/` and no `dist/` leg applies. | Mutation | Pins that went red | |---|---| | the rule removed (`andIntoWhere` of the filter skipped) | the member pin; count; aggregate; `findOne`; "the member is served exactly the other rows"; the create pin (6 red) | | the rule applied to an empty-for-everyone row (the `keys.size === 0` guard defeated) | the empty-for-everyone control; "exactly the other rows"; aggregate; the predicate's shape case (4 red) | | the count left uncorrected (the rule on `find` / `findOne` only) | count; aggregate (2 red). The page walk stays green: pages are `find` | ## Verification (at `bc2b06fb3`, after merging `origin/main` at `ceb4a939b`) - `pnpm --filter @objectstack/plugin-audit test`: exit 0, 37 files, 600 tests. - `pnpm --filter @objectstack/plugin-audit typecheck`: exit 0. It includes `check:test-typecheck`, and the new test file is in `tsconfig.test.json`'s program (`--listFiles` count 1). - `pnpm --filter @objectstack/dogfood typecheck`: exit 0. The new dogfood file is in the program (`--listFiles` count 1). - Dogfood, every file that reads `sys_activity` (8 files, the new one included): exit 0, 78 tests, on a rebuilt dependency closure. The suite resolves `@objectstack/plugin-audit` through `dist/`, so it was rebuilt first. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands`: 96 families derived. All 96 ran with exit 0, and `--ran` reconciles 96/96 with 0 NOT-MEASURED (a derived zero, every exit code recorded). - Two first runs answered `PREREQUISITE NOT MET` (exit 3) for packages outside this diff's closure that had no `dist/` (`check:skill-examples`, `check:dual-build-cjs-loads`). They were re-run green after those packages were built. - `check:query-options-erasure` exceeded a 300 s per-command cap once, and was re-run green in 265 s. - Lint, narrowed and proven: `eslint --no-inline-config --format json` over the 5 changed TypeScript files reports 5 files, 0 errors, 0 warnings, none ignored. `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project`, no typed rules; `--print-config` shows `project` null). So this diff cannot move a verdict on an untouched file. The full `pnpm lint` is CI's. ## Docs `content/docs/permissions/system-context.mdx`, the plugin-audit row of the census: the "Lose" column now names the withholding of a withheld-only update row, on `count` and `aggregate` too, and the "Get" column names the redaction's own pre-scan. `check:system-context-census` is green. A grep of `content/docs/**` (outside `releases/`) and `skills/**` for the activity stream's per-reader visibility found no other sentence this makes false. `record-view-auditing.mdx` and `audit-service.mdx` describe the ledger, or only name the object. ## Acceptance notes - **Cost.** Every non-system `sys_activity` read now runs a second bounded SYSTEM pre-scan (`id`, `object_name`, `metadata`), the first being the read gate's. The read is skipped when no security service is wired. A record timeline (scoped by `object_name` and `record_id`) scans a handful of rows. A broad read scans up to 2,000 rows' `metadata`. - **Broad reads past the bound.** A broad read whose pre-scan reaches 2,000 rows is now answered from the judged window only. Its `total` cannot exceed the window, for every reader the security service answers, administrators included. Before, the read gate's truncation kept rows beyond the window when their parent was judged readable inside it. Both are fail-closed; this one is narrower, because the withheld-update judgement is per row, not per parent. The warn names the remedy (scope by `object_name` and `record_id`). - **The compliance ledger door, NOT MEASURED.** `sys_audit_log`'s field redaction narrows `old_value` / `new_value` the same way. A ledger reader without the audit capability, holding object-level ledger read, would plausibly be served a withheld-only update's ledger row with empty snapshots. This is an unexercised inference: no ledger read was made here, and the ruling scopes this card to the activity stream. Carrier: none. - **A milestone's `type`, NOT MEASURED.** A mixed update that fires an activity milestone keyed on a field the reader is withheld would be served with the milestone's `type`, which names the withheld field's transition. A withheld-only one is withheld by this PR. This is an inference from reading `audit-writers.ts`, not a measurement. Carrier: none. - **File surface.** The claim's two read-side files and tests beside them, the dogfood route pin the claim allows, the changeset, and the one docs row the dispatch's Docs section asks for. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 32d5769 commit 3bddd4a

7 files changed

Lines changed: 775 additions & 10 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/plugin-audit': patch
3+
---
4+
5+
fix(plugin-audit): an activity row recording an update whose every changed field the reader is withheld is no longer served to that reader, on any listing face
6+
7+
Clause-②: no
8+
9+
A `sys_activity` row's recorded change (`metadata.old` / `metadata.new`) is narrowed key by key for each reader, through the security service's served-fields answer. An update whose every changed field the reader is withheld still reached that reader as a row with an empty change, and its summary, actor and timestamp said that the record changed, and when. An org member holding object-level `sys_activity` read was served that row for each sign-in stamp on a colleague's identity record (`last_login_at`), and for each failed-sign-in counter bump, lockout, password-change stamp and MFA-required stamp.
10+
11+
Such a row is now withheld from that reader as a row:
12+
13+
- **What counts as one.** An update row (its stored change has both an `old` and a `new` side) whose stored change had at least one key, where the reader is served none of those keys. The keys are read from the STORED change, not the redacted one.
14+
- **What is unaffected.** A create or a delete keeps its row. A row whose stored change is empty on both sides (an update that touched only `internal` fields) is unaffected. A mixed update keeps its row, with the served keys only. A reader served every field (an administrator) still reads every row with its change, within the pre-scan's bound. A system-context read is not narrowed.
15+
- **Every face agrees.** The rule is a WHERE built from a system-context pre-scan on `find`, `findOne`, `count` and `aggregate`. So a list's `total`, its pages, a by-id read (`404`) and a grouped count agree with the rows served. A pre-scan that reaches its 2,000-row bound answers a broad read from the rows it judged, for every reader, administrators included, and logs a warning. The remedy is to scope the query by `object_name` and `record_id`.
16+
17+
No migration: no key, export or config changes. A reader the security service gives no answer for (no security plugin wired) is not narrowed, as before.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ The largest single consumer — **17 of the 114 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 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` |
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, each read gate's own pre-scan and the activity redaction's — 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, the withholding of an activity row recording an update whose every changed field the reader is withheld (on `count` and `aggregate` too, so a total agrees with the rows), 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

‎packages/plugins/plugin-audit/src/activity-field-redaction.test.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ const V = {
6262
unserved1: 'AFRUNSERVEDONE93', unserved2: 'AFRUNSERVEDTWO94',
6363
open1: 'AFROPENONE95', open2: 'AFROPENTWO96', open3: 'AFROPENTHREE97',
6464
title: 'AFRTITLE98',
65+
earlier1: 'AFREARLIERONE89', earlier2: 'AFREARLIERTWO88',
6566
};
6667

6768
const itemObject = {
@@ -186,7 +187,12 @@ describe('[#21081] sys_activity value-bearing columns are served through the sec
186187
for (const row of [
187188
{
188189
type: 'updated', summary: 'earlier mirror row', record_label: 'earlier label',
189-
metadata: JSON.stringify({ old: { f_unserved: V.unserved1 }, new: { f_unserved: V.unserved2 } }),
190+
// A MIXED change, so a restricted reader keeps the row: a change
191+
// withheld whole is withheld as a row (#21388, its own file).
192+
metadata: JSON.stringify({
193+
old: { f_unserved: V.unserved1, f_open: V.earlier1 },
194+
new: { f_unserved: V.unserved2, f_open: V.earlier2 },
195+
}),
190196
},
191197
{ type: 'note', summary: 'app row with context', metadata: JSON.stringify({ channel: 'email' }) },
192198
{ type: 'note', summary: 'app row without context' },
@@ -307,7 +313,7 @@ describe('[#21081] sys_activity value-bearing columns are served through the sec
307313
});
308314

309315
it('a row in the mirror’s earlier shape (no provenance) keeps no text for a restricted reader, and keeps it for the control', async () => {
310-
const earlier = (rows: Row[]) => rows.find((r) => r.type === 'updated' && typeof r.metadata === 'string' && !('text_sources' in JSON.parse(r.metadata)) && JSON.parse(r.metadata).new && Object.keys(JSON.parse(r.metadata).new).join() === 'f_unserved');
316+
const earlier = (rows: Row[]) => rows.find((r) => r.type === 'updated' && typeof r.metadata === 'string' && !('text_sources' in JSON.parse(r.metadata)) && JSON.parse(r.metadata).new && Object.keys(JSON.parse(r.metadata).new).sort().join() === 'f_open,f_unserved');
311317
const control = earlier(await read(CONTROL));
312318
expect(control).toHaveProperty('summary');
313319
expect(control).toHaveProperty('record_label');
@@ -334,7 +340,12 @@ describe('[#21081] sys_activity value-bearing columns are served through the sec
334340
const rows = byChange(await read(NO_QUERYABLE_READER));
335341
const blob = JSON.stringify(rows);
336342
for (const v of [V.masked1, V.masked2, V.unserved1, V.unserved2, V.open1, V.open2]) expect(blob).not.toContain(v);
337-
expect(rows.allTracked).not.toHaveProperty('summary');
343+
// Served no field, the reader keeps the create row with no text composed
344+
// from one, and is withheld every update row whose change had keys
345+
// (#21388: a change withheld whole is withheld as a row).
346+
expect(rows.created).toBeTruthy();
347+
expect(rows.created).not.toHaveProperty('summary');
348+
expect(rows.allTracked).toBeUndefined();
338349
});
339350

340351
it('a system read is not redacted', async () => {

0 commit comments

Comments
 (0)