Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/21388-activity-withheld-update-row.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@objectstack/plugin-audit': patch
---

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

Clause-②: no

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.

Such a row is now withheld from that reader as a row:

- **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.
- **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.
- **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`.

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.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ The largest single consumer — **17 of the 114 sites**.
| 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` |
| 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` |
| 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` |
| 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` |
| 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` |
| 46 | Knowledge search returns hits unfiltered | service-knowledge | Lose: the permission filter over search results | `packages/services/service-knowledge/src/knowledge-service.ts#applyPermissionFilter` |

### 5. Actions, metadata plane, provenance, the organization wall
Expand Down
17 changes: 14 additions & 3 deletions packages/plugins/plugin-audit/src/activity-field-redaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ const V = {
unserved1: 'AFRUNSERVEDONE93', unserved2: 'AFRUNSERVEDTWO94',
open1: 'AFROPENONE95', open2: 'AFROPENTWO96', open3: 'AFROPENTHREE97',
title: 'AFRTITLE98',
earlier1: 'AFREARLIERONE89', earlier2: 'AFREARLIERTWO88',
};

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

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 () => {
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');
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');
const control = earlier(await read(CONTROL));
expect(control).toHaveProperty('summary');
expect(control).toHaveProperty('record_label');
Expand All @@ -334,7 +340,12 @@ describe('[#21081] sys_activity value-bearing columns are served through the sec
const rows = byChange(await read(NO_QUERYABLE_READER));
const blob = JSON.stringify(rows);
for (const v of [V.masked1, V.masked2, V.unserved1, V.unserved2, V.open1, V.open2]) expect(blob).not.toContain(v);
expect(rows.allTracked).not.toHaveProperty('summary');
// Served no field, the reader keeps the create row with no text composed
// from one, and is withheld every update row whose change had keys
// (#21388: a change withheld whole is withheld as a row).
expect(rows.created).toBeTruthy();
expect(rows.created).not.toHaveProperty('summary');
expect(rows.allTracked).toBeUndefined();
});

it('a system read is not redacted', async () => {
Expand Down
Loading
Loading