diff --git a/.changeset/23804-own-ui-state-no-ledger-row.md b/.changeset/23804-own-ui-state-no-ledger-row.md new file mode 100644 index 00000000000..6645e948e39 --- /dev/null +++ b/.changeset/23804-own-ui-state-no-ledger-row.md @@ -0,0 +1,15 @@ +--- +'@objectstack/plugin-audit': patch +--- + +fix(plugin-audit): the audit ledger no longer records a user's own writes to their own UI-state row (`sys_user_preference`). A deployment that exports `sys_audit_log` to a SIEM or a compliance archive stops receiving those rows from this release on, and nothing else tells that consumer. + +Clause-②: no + +The console saves a user's list preferences, recent items, favourites and palette recents as that user's own `sys_user_preference` rows, one write per click. Each write landed an `update` row in `sys_audit_log`, so Setup's Audit Logs opened on a run of one user's clicks, and the System Overview's "Events by User" chart counted them. + +- **What changed.** When the acting user is the row's owner (`user_id`) on both sides of the write, the audit writer records neither the `sys_audit_log` row nor the `sys_activity` row. It already skipped the activity row for such a write; it now skips the ledger row by the same test. A create, an update and a delete are all covered. +- **What is still recorded.** Every other actor's write to the same rows: an administrator editing someone's preferences from Setup, a system write with no user, and a write that moves a row to another owner. `sys_user_preference` is not an excluded object, and every other object is unchanged. +- **Nothing to migrate.** Rows already in `sys_audit_log` stay. + +The list of per-user UI-state objects is closed and declared inside the plugin, and today holds one object, `sys_user_preference`. No metadata key adds an object to it. diff --git a/packages/plugins/plugin-audit/src/audit-writers.own-ui-state.integration.test.ts b/packages/plugins/plugin-audit/src/audit-writers.own-ui-state.integration.test.ts new file mode 100644 index 00000000000..05bebe8ded8 --- /dev/null +++ b/packages/plugins/plugin-audit/src/audit-writers.own-ui-state.integration.test.ts @@ -0,0 +1,183 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// [#23804] The real-engine half of the own UI-state rule, in its own file: it +// boots a kernel, an engine, a SQLite driver and the audit plugin, and that +// load is kept out of `audit-writers.test.ts`, whose fake-engine pins +// (#22961's block) hold the same rule on hand-built hook contexts. Static +// imports, so the load is paid during collection, not by one test's timeout. +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { SysUserPreference } from '@objectstack/platform-objects/identity'; +import { AuditPlugin } from './audit-plugin.js'; + +/** + * [#23804, ruling record 6212163130] A user's own write to their own per-user + * UI-state row writes neither the ledger row nor the activity row, on a REAL + * engine with the audit plugin: `ObjectKernel` + `ObjectQLPlugin` + + * `AuditPlugin` over a real SQLite driver, the shipped `sys_user_preference`, + * and both tables read at rest. The user travels as it does in production + * (`ExecutionContext.userId` to the hook session), and `result` / `previous` + * are whatever the engine binds for a create, an update and a delete, not + * hand-built shapes. + * + * Each case writes its own preference row and reads only the rows its write + * under test ADDS about that row, so the seeding write never counts for or + * against it. An own-write case that can still be written to ends with an + * administrator's write to the same row, which must add exactly one ledger + * row: a "no row" verdict cannot pass on a writer that never ran. + */ +describe('audit writers — a user\'s own UI-state write leaves no ledger row, on a real engine (#23804)', () => { + const PREFS = 'sys_user_preference'; + const RULE = 'sys_sharing_rule'; + const OWNER = 'u_alice'; + const ADMIN = 'u_admin'; + const HARNESS_PACKAGE = 'com.objectstack.audit.test.own-ui-state-ledger'; + /** A stand-in under the real name: plugin-sharing owns the definition, and the name is what the rule reads. */ + const sharingRule = { + name: RULE, + label: 'Sharing Rule', + isSystem: true, + fields: { + name: { name: 'name', label: 'Name', type: 'text' as const }, + object_name: { name: 'object_name', label: 'Object', type: 'text' as const }, + access_level: { name: 'access_level', label: 'Access Level', type: 'text' as const }, + }, + }; + + let kernel: ObjectKernel; + let engine: ObjectQL; + let sql: (statement: string, bindings: unknown[]) => Promise[]>; + + const as = (userId: string) => ({ context: { userId } }); + const SYSTEM = { context: { isSystem: true } }; + + type Table = 'sys_audit_log' | 'sys_activity'; + /** Every row `table` holds about one record, at rest. */ + const rowsAbout = (table: Table, object: string, recordId: string) => + sql(`SELECT * FROM "${table}" WHERE object_name = ? AND record_id = ?`, [object, recordId]); + /** The rows `write` adds to each table about one record. */ + async function rowsAddedBy(object: string, recordId: string, write: () => Promise) { + const seen = new Set(); + for (const table of ['sys_audit_log', 'sys_activity'] as const) { + for (const r of await rowsAbout(table, object, recordId)) seen.add(`${table}:${String(r.id)}`); + } + await write(); + const added = async (table: Table) => + (await rowsAbout(table, object, recordId)).filter((r) => !seen.has(`${table}:${String(r.id)}`)); + return { ledger: await added('sys_audit_log'), activity: await added('sys_activity') }; + } + + /** The owner's own create of a fresh preference row (a distinct key per case). */ + let seq = 0; + async function ownPreference(): Promise { + seq += 1; + const row = await engine.insert(PREFS, { user_id: OWNER, key: `ui.recent.${seq}`, value: [] }, as(OWNER) as any); + return String((row as { id: unknown }).id); + } + const setValue = (id: string, value: unknown[], options: Record) => + engine.update(PREFS, { value }, { where: { id }, ...options } as any); + + beforeAll(async () => { + kernel = new ObjectKernel({ logger: { level: 'silent' } } as any); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AuditPlugin()); + await kernel.bootstrap(); + + engine = kernel.getService('objectql'); + const driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.connect(); + engine.registerDriver(driver, true); + engine.registry.registerObject(SysUserPreference as any, HARNESS_PACKAGE); + engine.registry.registerObject(sharingRule as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + + sql = async (statement, bindings) => { + const result = await (driver as unknown as { execute(s: string, b: unknown[]): Promise }).execute( + statement, + bindings, + ); + // knex wraps some results as `[rows]`; normalize both shapes. + const list = Array.isArray(result) && Array.isArray(result[0]) ? result[0] : result; + expect(Array.isArray(list)).toBe(true); + return list as Record[]; + }; + }, 120_000); + + afterAll(async () => { + if (kernel) { + await Promise.race([kernel.shutdown(), new Promise((resolve) => setTimeout(resolve, 10_000))]); + } + }, 30_000); + + it('the owner\'s own create writes no sys_audit_log row and no sys_activity row', async () => { + const id = await ownPreference(); + + expect(await rowsAbout('sys_audit_log', PREFS, id)).toHaveLength(0); + expect(await rowsAbout('sys_activity', PREFS, id)).toHaveLength(0); + + // Live-writer CONTROL: another actor's write to the same row is recorded. + const control = await rowsAddedBy(PREFS, id, () => setValue(id, [{ id: 'acc-1' }], as(ADMIN))); + expect(control.ledger.map((r) => [r.action, r.user_id])).toEqual([['update', ADMIN]]); + }); + + it('the owner\'s own update writes no sys_audit_log row and no sys_activity row', async () => { + const id = await ownPreference(); + const own = await rowsAddedBy(PREFS, id, async () => { + await setValue(id, [{ id: 'acc-1' }], as(OWNER)); + await setValue(id, [{ id: 'acc-1' }, { id: 'acc-2' }], as(OWNER)); + }); + + expect(own.ledger).toHaveLength(0); + expect(own.activity).toHaveLength(0); + + // Live-writer CONTROL, as above. + const control = await rowsAddedBy(PREFS, id, () => setValue(id, [], as(ADMIN))); + expect(control.ledger.map((r) => [r.action, r.user_id])).toEqual([['update', ADMIN]]); + }); + + it('the owner\'s own delete writes no sys_audit_log row and no sys_activity row', async () => { + // The engine binds the deleted row as `previous`, which names the owner; + // the delete's `result` names no owner and is not evidence either way. + const id = await ownPreference(); + const own = await rowsAddedBy(PREFS, id, () => engine.delete(PREFS, { where: { id }, ...as(OWNER) } as any)); + + expect(own.ledger).toHaveLength(0); + expect(own.activity).toHaveLength(0); + }); + + it('an administrator\'s update of another user\'s row writes a ledger row (and its activity row)', async () => { + const id = await ownPreference(); + const added = await rowsAddedBy(PREFS, id, () => setValue(id, [{ id: 'acc-9' }], as(ADMIN))); + + expect(added.ledger.map((r) => [r.action, r.user_id])).toEqual([['update', ADMIN]]); + expect(added.activity.map((r) => [r.type, r.actor_id])).toEqual([['updated', ADMIN]]); + }); + + it('a system write with no user writes a ledger row', async () => { + const id = await ownPreference(); + const added = await rowsAddedBy(PREFS, id, () => setValue(id, [{ id: 'acc-9' }], SYSTEM)); + + expect(added.ledger.map((r) => [r.action, r.user_id ?? null])).toEqual([['update', null]]); + }); + + it('the owner moving the row to another user writes a ledger row', async () => { + const id = await ownPreference(); + const added = await rowsAddedBy(PREFS, id, () => + engine.update(PREFS, { user_id: 'u_bob' }, { where: { id }, ...as(OWNER) } as any), + ); + + expect(added.ledger.map((r) => [r.action, r.user_id])).toEqual([['update', OWNER]]); + }); + + it('the same user\'s update of a sys_sharing_rule writes a ledger row', async () => { + const rule = await engine.insert(RULE, { name: 'Region West', object_name: 'account', access_level: 'read' }, SYSTEM as any); + const id = String((rule as { id: unknown }).id); + const added = await rowsAddedBy(RULE, id, () => + engine.update(RULE, { access_level: 'edit' }, { where: { id }, ...as(OWNER) } as any), + ); + + expect(added.ledger.map((r) => [r.action, r.user_id])).toEqual([['update', OWNER]]); + }); +}); diff --git a/packages/plugins/plugin-audit/src/audit-writers.test.ts b/packages/plugins/plugin-audit/src/audit-writers.test.ts index 488c9fa906d..bbbe7781a69 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.test.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.test.ts @@ -687,14 +687,16 @@ describe('audit writers — enable.activities opt-out gate (#2707)', () => { }); /** - * [#22961] A user's own write to their own per-user UI-state row writes the - * ledger row and no activity row. Every other writer keeps today's rows. + * [#22961, #23804 (ruling record 6212163130)] A user's own write to their own + * per-user UI-state row writes neither the ledger row nor the activity row. + * Every other writer keeps both rows. * * The rows below are the console's "recently visited" write as the real engine * hands it to this hook: the session subject is the owner, and both `result` - * and the bound `previous` carry the full row with its `user_id`. + * and the bound `previous` carry the full row with its `user_id`. The real + * engine's own shapes are pinned in `audit-writers.own-ui-state.integration.test.ts`. */ -describe('audit writers — a user\'s own per-user UI-state write is not mirrored (#22961)', () => { +describe('audit writers — a user\'s own per-user UI-state write is not recorded (#22961, #23804)', () => { const SCHEMA = { sys_audit_log: SINGLE_TENANT.sys_audit_log, sys_activity: SINGLE_TENANT.sys_activity, @@ -717,17 +719,13 @@ describe('audit writers — a user\'s own per-user UI-state write is not mirrore ] as const; for (const w of OWN_WRITES) { - it(`the owner's own ${w.action} writes the sys_audit_log row and no sys_activity row`, async () => { + it(`the owner's own ${w.action} writes no sys_audit_log row and no sys_activity row`, async () => { const { engine, fire, created } = makeEngine(SCHEMA); installAuditWriters(engine as any, 'test.audit'); await fire(w.event, { object: 'sys_user_preference', ...w.ctx, session: { userId: OWNER } }); - const ledger = rowsOn(created, 'sys_audit_log', 'sys_user_preference'); - expect(ledger).toHaveLength(1); - expect(ledger[0].row.action).toBe(w.action); - expect(ledger[0].row.user_id).toBe(OWNER); - expect(ledger[0].row.record_id).toBe(PREF.id); + expect(created.filter((c) => c.object === 'sys_audit_log')).toHaveLength(0); expect(created.filter((c) => c.object === 'sys_activity')).toHaveLength(0); }); } @@ -766,7 +764,7 @@ describe('audit writers — a user\'s own per-user UI-state write is not mirrore ] as const; for (const w of OTHER_WRITERS) { - it(`CONTROL: on a user's preference row, ${w.who} keeps its activity row`, async () => { + it(`CONTROL: on a user's preference row, ${w.who} keeps its ledger row and its activity row`, async () => { const { engine, fire, created } = makeEngine(SCHEMA); installAuditWriters(engine as any, 'test.audit'); diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index f556c52ac94..1305c5c8e5f 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -163,8 +163,8 @@ export interface AuditWriterOptions { * Skip rules avoid recursion and noise: * - Never audit the audit/activity tables themselves. * - Never audit session/presence/auth tables (high-frequency, low value). - * - A user's own write to their own per-user UI-state row writes the ledger - * row and no activity row (`PER_USER_UI_STATE_OWNER_FIELD`). + * - A user's own write to their own per-user UI-state row writes neither the + * ledger row nor the activity row (`PER_USER_UI_STATE_OWNER_FIELD`). * - Read-only operations (`afterFind`) are not audited BY THIS WRITER. Since * #8992 the `read` action has its own writer in `read-audit.ts`, installed * separately and only over the objects a deployment opts in: record-detail @@ -271,37 +271,45 @@ const SKIP_OBJECTS = new Set([ export const AUDIT_EXCLUDED_OBJECTS: string[] = [...SKIP_OBJECTS]; /** - * [#22961] Per-user UI-state objects: the ledger row stays, and the activity - * mirror is skipped for the owning user's OWN write only. - * - * The rule: what reaches a user's bell is another actor's act or something the - * user needs, never the user's own UI state. A `sys_activity` row reaches the - * bell (its Activity tab lists the newest rows the caller may read). A per-user - * UI-state row is re-saved by the client as a side effect of using the - * interface, and RLS keeps it private to its user, so the one reader of its - * activity row is the user who just caused it. Mirrored, every navigation came - * back as `Updated User Preference "ui.recent"`, and a fresh user's bell held - * nothing else. + * [#22961, ruling record 6212163130] Per-user UI-state objects: the owning + * user's OWN write writes neither the `sys_audit_log` row nor the activity row. + * + * The rule: the ledger records acts (data, configuration, access and sharing + * changes), and what reaches a user's bell is another actor's act or something + * the user needs. A user's own UI state is neither. A per-user UI-state row is + * re-saved by the client as a side effect of using the interface (a column + * hidden, a sort toggled, a page visited), and RLS keeps it private to its + * user. Recorded, every click came back twice: as an `update` row on top of + * Setup's Audit Logs, burying the rows an administrator opens it for, and as + * `Updated User Preference "ui.recent"` in the user's own bell, where a fresh + * user saw nothing else. * * - CLOSED and DECLARED: an object is a member only by being listed here, * mapped to the field that names its owning user. ⛔ Never a name heuristic. * No spec key means "per-user UI state"; `enable.activities: false` mutes - * every writer, which is wider than this rule. - * - OWN WRITES ONLY: the user the activity row would name (`actor_id`) must be - * the owner on every side of the row the write carries. Another actor's - * write keeps its activity row: an organization admin editing the row from - * Setup's User Preferences grid (`modifyAllRecords` passes the self-only - * RLS), a system write with no user, a write that moves the row to another - * owner. - * - ACTIVITY ONLY: the `sys_audit_log` row is written exactly as before. That - * is why this is not `SKIP_OBJECTS`, which drops the ledger row too and is - * the `AUDIT_EXCLUDED_OBJECTS` list the hook registration and `read-audit.ts` - * share. + * the activity row of every writer and never the ledger row, so it is not + * this rule. + * - OWN WRITES ONLY: the acting user (`userId`: the `user_id` the ledger row + * would record, and the `actor_id` the activity row would) must be the + * owner on every side of the row the write carries. Another actor's write + * keeps BOTH rows: an organization admin editing the row from Setup's User + * Preferences grid (`modifyAllRecords` passes the self-only RLS), a system + * write with no user, a write that moves the row to another owner. + * - LEDGER AND ACTIVITY, one test (ruling record 6212163130): judged once, at + * the top of `writeAudit`, before either row is built; a write it does not + * admit writes both rows exactly as before. It is still not `SKIP_OBJECTS`, + * which drops every writer's rows and is the `AUDIT_EXCLUDED_OBJECTS` list + * the hook registration and `read-audit.ts` share: another actor's write to + * the same object is an act. No spec key and no object-level flag either; + * `SKIP_OBJECTS` and this rule are the standing qualifications of the + * spec's "audit capture itself is always on" (`enable.trackHistory`). Rows + * already stored are kept. * * Membership as measured: the console keeps all of its per-user UI state * (`ui.recent`, `ui.favorites`, `ui.flow.palette.recents`) on * `sys_user_preference`, and no other per-user UI-state object exists. A new - * one joins by a line here. + * one joins by a line here, and that line takes its owners' own writes out of + * the compliance ledger: ⛔ never a business or security object. */ const PER_USER_UI_STATE_OWNER_FIELD: ReadonlyMap = new Map([ ['sys_user_preference', 'user_id'], @@ -1823,6 +1831,13 @@ export function installAuditWriters( ? (ctx as any).provenance.attributedUserId : undefined; const userId: string | undefined = sess.userId ?? attributedUserId; + // [#22961, ruling record 6212163130] A user's own write to their own + // per-user UI-state row writes neither the ledger row nor the activity row + // (PER_USER_UI_STATE_OWNER_FIELD states the rule). Judged on `userId`, the + // value the ledger's `user_id` and the activity row's `actor_id` would + // carry, and before any read this writer pays for. A write the test does + // not admit goes on to write both rows. + if (isOwnUiStateWrite(ctx.object, userId, before, after)) return; // [#17022, ADR-0090 D10 rule 4] The AGENT that performed this write, when // an MCP OAuth client acted for the human above. A THIRD channel, and the // reason it is a separate one: `session.userId` is the delegator — the @@ -2083,13 +2098,9 @@ export function installAuditWriters( // = mirror on (spec default `true`), explicit `false` = this object's CRUD // is not mirrored into the sys_activity timeline. This is the per-object // lever for activity-row growth (ADR-0057). The compliance audit row is - // NOT gated — sys_audit_log capture stays unconditional. - const activitiesEnabled = getObjectDef(ctx.object)?.enable?.activities !== false; - // [#22961] A user's own write to their own per-user UI-state row is not - // mirrored either (PER_USER_UI_STATE_OWNER_FIELD states the rule). Judged - // on `userId`, the same value `actor_id` would carry. The ledger row is - // untouched. - const mirrorsActivity = activitiesEnabled && !isOwnUiStateWrite(ctx.object, userId, before, after); + // NOT gated by it. ([#22961] A user's own write to their own per-user + // UI-state row never reaches this line: it returned above, with no row.) + const mirrorsActivity = getObjectDef(ctx.object)?.enable?.activities !== false; // [#22510, #22527] The name and avatar of the user `actor_id` names — the // SAME `userId`, so the columns cannot disagree about who acted — from one // memoized `sys_user` read. Read only when an activity row will be