From 5434e89b6692a62311d8d3cf2063715da94f5240 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 02:44:50 +0000 Subject: [PATCH 01/11] wip(plugin-audit): ADR-0139 E6 stamped columns, predicate, backfill Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- .../src/activity-changed-fields-backfill.ts | 181 +++++++++ .../src/activity-changed-fields.ts | 183 +++++++++ .../src/activity-field-redaction.ts | 375 ++++++++++-------- .../src/activity-read-visibility.ts | 26 +- .../plugins/plugin-audit/src/audit-writers.ts | 31 +- packages/plugins/plugin-audit/src/index.ts | 15 + .../src/objects/sys-activity.object.ts | 30 ++ .../src/parent-field-query-guard.ts | 8 +- 8 files changed, 652 insertions(+), 197 deletions(-) create mode 100644 packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts create mode 100644 packages/plugins/plugin-audit/src/activity-changed-fields.ts diff --git a/packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts b/packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts new file mode 100644 index 00000000000..445f9217c58 --- /dev/null +++ b/packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 D4, E6] The stored-row backfill of `sys_activity`'s two + * writer-stamped columns, `changed_fields` and `milestone_key`. + * + * ADR-0139's Consequences: "`changed_fields` is a schema change on an + * append-only engine-owned object, with a backfill a deployment must run". + * Rows written before the columns existed carry no stamp, and the withheld-row + * rule (`activity-field-redaction.ts`) withholds an unstamped row from every + * non-system reader — so until this runs, a feed shows only the rows written + * since the upgrade. `os migrate activity-changed-fields` is its door. + * + * ## One derivation + * + * Each row is stamped by {@link planActivityStamp}, the planner the CRUD + * mirror and the before-insert hook stamp new rows with + * (`activity-changed-fields.ts`), so a stored row and a new row with the same + * metadata carry the same stamp. Both columns are written in one update. + * + * ## Through the migration door, never the CRUD write path + * + * The object is `managedBy: 'append-only'`; the rows are read and updated as + * the platform (a SYSTEM context), the shape of + * `stored-metadata-body-migration.ts`. A row the planner cannot judge keeps + * no stamp, and so keeps the fail-closed answer it has today; the report + * counts it. + * + * ## Idempotent, its own verification + * + * Only unstamped rows (`changed_fields` NULL) are read, so a second run finds + * nothing to stamp but the rows it cannot judge, and re-running and reading a + * clean report is the check. No `sys_migration` flag is recorded: nothing + * gates irreversible behaviour on it. The walk pages by `id`, so a table of any + * size is read a page at a time. + */ + +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { + ACTIVITY_CHANGED_FIELDS_COLUMN, + ACTIVITY_MILESTONE_KEY_COLUMN, + planActivityStamp, +} from './activity-changed-fields.js'; +import type { MigrationLogger } from './stored-metadata-body-migration.js'; + +const ACTIVITY_OBJECT = 'sys_activity'; + +/** The backfill reads and writes as the platform, never as a user. */ +const SYSTEM_CTX = { isSystem: true } as const; + +/** Rows read per page. */ +export const ACTIVITY_BACKFILL_PAGE_SIZE = 500; + +/** The engine surface the runner needs — the real data-engine contract's members. */ +export type ActivityChangedFieldsBackfillEngine = Pick; + +/** What one backfill run did (or would do in dry-run). */ +export interface ActivityChangedFieldsBackfillReport { + apply: boolean; + /** Unstamped rows read. */ + scanned: number; + /** Rows the planner stamped (or would stamp). */ + stamped: number; + /** Of those, rows that carry a milestone key. */ + milestones: number; + /** Unstamped rows the planner cannot judge: they stay unstamped (withheld). */ + unjudged: number; + /** A failed read, or a row an `update` threw on (apply mode) — re-run to finish. */ + failures: number; +} + +function asArray(result: unknown): Record[] { + if (Array.isArray(result)) return result as Record[]; + if (result && typeof result === 'object' && Array.isArray((result as { records?: unknown }).records)) { + return (result as { records: Record[] }).records; + } + return []; +} + +/** + * Stamp `changed_fields` and `milestone_key` on every unstamped `sys_activity` + * row. Dry-run by default; `apply` writes. Idempotent. + */ +export async function backfillActivityChangedFields( + engine: ActivityChangedFieldsBackfillEngine, + logger: MigrationLogger, + options: { apply?: boolean; pageSize?: number } = {}, +): Promise { + const apply = options.apply === true; + const pageSize = options.pageSize && options.pageSize > 0 ? options.pageSize : ACTIVITY_BACKFILL_PAGE_SIZE; + const report: ActivityChangedFieldsBackfillReport = { + apply, + scanned: 0, + stamped: 0, + milestones: 0, + unjudged: 0, + failures: 0, + }; + + // Keyset paging on `id`: a stamped row leaves the unstamped set, an unjudged + // one stays in it, and neither is read twice in one run. + let after: string | undefined; + for (;;) { + const unstamped: Record = { [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $null: true } }; + const where = after === undefined ? unstamped : { $and: [unstamped, { id: { $gt: after } }] }; + let rows: Record[]; + try { + rows = asArray( + await engine.find( + ACTIVITY_OBJECT, + { + where, + fields: ['id', 'metadata'], + orderBy: [{ field: 'id', order: 'asc' }], + limit: pageSize, + }, + { context: SYSTEM_CTX }, + ), + ); + } catch (e) { + // A table this run could not read is NOT a clean table: counted, so the + // run says so (and the door exits non-zero). + report.failures += 1; + logger.warn( + `[activity-changed-fields-backfill] could not read ${ACTIVITY_OBJECT} — its remaining rows were NOT ` + + `examined: ${(e as Error)?.message ?? String(e)}`, + ); + break; + } + if (rows.length === 0) break; + + for (const row of rows) { + report.scanned += 1; + const stamp = planActivityStamp(row.metadata); + if (!stamp) { + report.unjudged += 1; + continue; + } + report.stamped += 1; + if (stamp.milestone_key !== null) report.milestones += 1; + if (!apply) continue; + const id = row.id; + if (typeof id !== 'string' || id === '') { + report.failures += 1; + logger.warn(`[activity-changed-fields-backfill] ${ACTIVITY_OBJECT} row has no id — cannot stamp it`); + continue; + } + try { + // A single-id write, as the platform: a system caller is exempt from + // the static-readonly strip every activity field carries. + await engine.update( + ACTIVITY_OBJECT, + { + id, + [ACTIVITY_CHANGED_FIELDS_COLUMN]: stamp.changed_fields, + [ACTIVITY_MILESTONE_KEY_COLUMN]: stamp.milestone_key, + }, + { context: SYSTEM_CTX }, + ); + } catch (e) { + report.failures += 1; + logger.warn( + `[activity-changed-fields-backfill] failed to stamp ${ACTIVITY_OBJECT}/${id}: ` + + `${(e as Error)?.message ?? String(e)}`, + ); + } + } + + const last = rows[rows.length - 1]?.id; + if (rows.length < pageSize || typeof last !== 'string' || last === '') break; + after = last; + } + + logger.info?.( + `[activity-changed-fields-backfill] ${apply ? 'stamped' : 'would stamp'} ${report.stamped} of ${report.scanned} ` + + `unstamped ${ACTIVITY_OBJECT} row(s)` + + (report.unjudged > 0 ? `; ${report.unjudged} cannot be judged and stay unstamped (withheld)` : '') + + (report.failures > 0 ? ` (${report.failures} failed — re-run to finish)` : ''), + ); + return report; +} diff --git a/packages/plugins/plugin-audit/src/activity-changed-fields.ts b/packages/plugins/plugin-audit/src/activity-changed-fields.ts new file mode 100644 index 00000000000..a5471c1e87b --- /dev/null +++ b/packages/plugins/plugin-audit/src/activity-changed-fields.ts @@ -0,0 +1,183 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 D4, E6] The two writer-stamped columns of `sys_activity` the + * withheld-row rule is a predicate over, and the ONE derivation that stamps + * them. + * + * - `changed_fields` — the key set of the recorded change: for an update (both + * `metadata.old` and `metadata.new` are records) the union of their keys, + * sorted; `[]` for every other row (a create, a delete, an app's note, a row + * with no recorded change). `null` is UNSTAMPED. + * - `milestone_key` — for a fired milestone the CRUD mirror declared + * (`metadata.kind === 'milestone'` beside its `text_sources` declaration), + * one sorted text key of every field the object's `activityMilestones` + * declaration watched at write time (the declaration's `text_sources.kind` + * list); `null` for every other row. + * + * ## One derivation + * + * {@link planActivityStamp} is the only function that decides either column. + * The CRUD mirror (`audit-writers.ts`) calls it on the metadata it writes, the + * `sys_activity` before-insert hook ({@link installActivityStampHook}) calls it + * on every inserted row, so a writer other than the mirror is stamped the same + * way, and the stored-row backfill (`activity-changed-fields-backfill.ts`) + * calls it on the rows at rest. Stored rows and new rows are judged by one + * rule. + * + * ## A row it cannot judge stays unstamped + * + * A milestone row the mirror declared with no readable watched-field list (the + * shape the field redaction calls `'unknown'` provenance) cannot be keyed, so + * the planner answers `null` and both columns stay `null`. The read rule + * (`activity-field-redaction.ts`) withholds an unstamped row from every + * non-system reader: ADR-0139 D4's answer for an unjudged row is closed. + * + * ## The key + * + * {@link milestoneKeyOf} — the distinct watched field names, sorted, joined by + * `,`. A field name is a machine name, so the separator cannot occur inside + * one; a list naming anything else is not keyed. The read rule compares a + * row's key with the key of the parent object's CURRENT declaration + * ({@link declaredMilestoneWatchedFields}), the same function the mirror + * derives the row's list from, so a row written under the declaration that is + * in force compares equal. + */ + +import { isRecord } from './served-fields.js'; + +/** + * The `metadata` key under which the CRUD mirror declares which PARENT fields + * each text column was composed from: `{ summary: string[], record_label: + * string[] }`. Written by `audit-writers.ts`, read (and stripped) by the field + * redaction — one name, imported by every half. Defined here, below the + * redaction, so the planner and the redaction share it without an import cycle. + */ +export const ACTIVITY_TEXT_SOURCES_KEY = 'text_sources'; + +/** + * [#22771] The `metadata` key a row's DOMAIN kind rides in. ADR-0052 §5: "`type` + * stays domain-NEUTRAL … Domain kind rides in `metadata.kind`." So a fired + * milestone keeps the `type` its declaration names (`completed`, or `updated` + * when it names none) and says what it is here, which is how a reader tells a + * "Case resolved" milestone from a finished task. + */ +export const ACTIVITY_KIND_KEY = 'kind'; + +/** [#22771] The kind the CRUD mirror stamps on a row a declared `activityMilestones` entry fired (ADR-0052 §5b.2). */ +export const ACTIVITY_MILESTONE_KIND = 'milestone'; + +/** The column the key set of a row's recorded change is stamped in. */ +export const ACTIVITY_CHANGED_FIELDS_COLUMN = 'changed_fields'; + +/** The column a fired milestone's watched-field key is stamped in. Never served. */ +export const ACTIVITY_MILESTONE_KEY_COLUMN = 'milestone_key'; + +/** The two stamped columns, in one list for the redaction and the query guard. */ +export const ACTIVITY_STAMPED_COLUMNS = [ACTIVITY_CHANGED_FIELDS_COLUMN, ACTIVITY_MILESTONE_KEY_COLUMN] as const; + +/** What a row is stamped with. */ +export interface ActivityStamp { + changed_fields: string[]; + milestone_key: string | null; +} + +/** Field machine-name shape (`FieldSchema.name` in packages/spec). */ +const FIELD_NAME_RE = /^[a-z_][a-z0-9_]*$/; + +/** + * The key of a watched-field list: the distinct names, sorted, joined by `,`. + * `null` for an empty list, and for a list naming anything that is not a field + * machine name (it cannot be keyed without ambiguity). + */ +export function milestoneKeyOf(fields: readonly unknown[]): string | null { + if (!fields.every((f) => typeof f === 'string' && FIELD_NAME_RE.test(f))) return null; + const names = [...new Set(fields as string[])].sort(); + return names.length > 0 ? names.join(',') : null; +} + +/** + * Every field an object's `activityMilestones` declaration watches, in + * declaration order, distinct: the list the CRUD mirror declares a fired + * milestone's kind from. An entry is asked when it names a `field` and a + * `summary` (the mirror's own test). `[]` when the object declares none. + */ +export function declaredMilestoneWatchedFields(objectDef: unknown): string[] { + const milestones = isRecord(objectDef) ? objectDef.activityMilestones : undefined; + if (!Array.isArray(milestones)) return []; + const asked = milestones.filter( + (m): m is { field: string } => isRecord(m) && typeof m.field === 'string' && typeof m.summary === 'string', + ); + return [...new Set(asked.map((m) => m.field))]; +} + +/** Parse a stored `metadata` value: a JSON object, or `null` for anything else. */ +function parseMetadata(raw: unknown): Record | null { + if (isRecord(raw)) return raw; + if (typeof raw !== 'string' || raw.trim() === '') return null; + try { + const value = JSON.parse(raw); + return isRecord(value) ? value : null; + } catch { + return null; + } +} + +/** + * The stamp for one activity row, from its `metadata` (the stored string, or + * the object a writer is about to serialize) — or `null` when the row cannot + * be judged and must stay unstamped (module header). Pure. + */ +export function planActivityStamp(metadata: unknown): ActivityStamp | null { + const parsed = parseMetadata(metadata); + if (!parsed) return { changed_fields: [], milestone_key: null }; + + const before = parsed.old; + const after = parsed.new; + const changed_fields = + isRecord(before) && isRecord(after) ? [...new Set([...Object.keys(before), ...Object.keys(after)])].sort() : []; + + let milestone_key: string | null = null; + if (parsed[ACTIVITY_KIND_KEY] === ACTIVITY_MILESTONE_KIND && ACTIVITY_TEXT_SOURCES_KEY in parsed) { + const declared = parsed[ACTIVITY_TEXT_SOURCES_KEY]; + const list = isRecord(declared) ? declared[ACTIVITY_KIND_KEY] : undefined; + if (!Array.isArray(list)) return null; + if (list.length > 0) { + milestone_key = milestoneKeyOf(list); + if (milestone_key === null) return null; + } + } + return { changed_fields, milestone_key }; +} + +/** The engine slice the stamping hook registers on. */ +export interface ActivityStampHookEngine { + registerHook(event: string, handler: (ctx: any) => void | Promise, options?: { object?: string; packageId?: string }): void; +} + +/** Stamp one row in place: the planner's answer, over whatever the writer sent. */ +export function stampActivityRow(row: Record): void { + const stamp = planActivityStamp(row.metadata); + row[ACTIVITY_CHANGED_FIELDS_COLUMN] = stamp ? stamp.changed_fields : null; + row[ACTIVITY_MILESTONE_KEY_COLUMN] = stamp ? stamp.milestone_key : null; +} + +/** + * [ADR-0139 E6, Q2 = b] Install the `sys_activity` before-insert hook that + * stamps both columns on every inserted row, whoever writes it: the CRUD + * mirror, an app's own server action, the seed loader. The planner's answer is + * the platform's value and is assigned over a writer's own (the engine's + * create-side `readonly` strip leaves `sys_` objects to their own guards, so + * this is the guard): a stamp can never disagree with the row's metadata, and + * a row the planner cannot judge is stored unstamped. + */ +export function installActivityStampHook(engine: ActivityStampHookEngine, packageId: string): void { + engine.registerHook( + 'beforeInsert', + (ctx: any) => { + const data = ctx?.input?.data; + for (const row of Array.isArray(data) ? data : [data]) if (isRecord(row)) stampActivityRow(row); + }, + { object: 'sys_activity', packageId }, + ); +} diff --git a/packages/plugins/plugin-audit/src/activity-field-redaction.ts b/packages/plugins/plugin-audit/src/activity-field-redaction.ts index edd91f8e1e0..d86a599bde7 100644 --- a/packages/plugins/plugin-audit/src/activity-field-redaction.ts +++ b/packages/plugins/plugin-audit/src/activity-field-redaction.ts @@ -97,57 +97,80 @@ * redaction would leave its change empty. A reader the service gives no * answer for is narrowed by neither. ⛔ No object or field is named here. * - * It is a WHERE, not a post-read drop, built the way the read gate builds its - * own and on the same four reads (`find`, `findOne`, `count`, `aggregate`). A - * pre-scan of the rows the query would touch, under SYSTEM context and in the - * caller's order, judges each one, and the ids it withholds are ANDed out of - * the query (`{ id: { $nin: WITHHELD } }`). So a list's `total`, its pages, a - * by-id read and a grouped count all agree with the rows served; a count that - * kept the row would leak the same timing. A pre-scan that reaches its bound - * fails CLOSED, as the read gate's does: the rows beyond the window cannot be - * judged, so the read is answered from the judged rows alone - * (`{ id: { $in: KEPT } }`), and a warn says so. - * * ## A fired milestone is withheld as a row from a reader not served its watched fields (#22786) * * A fired milestone's row speaks about its watched fields in columns the * key-by-key narrowing does not reach: its `type` is the declaration's, and * its summary is the declaration's sentence. Both say that a watched field - * entered its declared value. So the same pre-scan, by the same answer, - * withholds that row as a ROW from a reader not served every field a declared - * milestone watches — the list the mirror declares the row's `metadata.kind` - * from ({@link ActivityTextSources.kind}). One judgement per row - * (`isWithheldRow`), one pre-scan, one WHERE: + * entered its declared value. So such a row is withheld as a ROW from a reader + * not served every field the object's milestones watch, and the redaction + * drops its kind from any row that reader is still served. + * + * ## Both rules are one predicate over writer-stamped columns (ADR-0139 D4, E6) + * + * Each row carries, stamped at write time by one derivation + * (`activity-changed-fields.ts`: the CRUD mirror, a before-insert hook for any + * other writer, and the stored-row backfill `os migrate + * activity-changed-fields`), the key set of its recorded change + * (`changed_fields`) and, for a fired milestone, one sorted key of the fields + * its declaration watched (`milestone_key`). The rule is a WHERE over them, + * one branch per parent object ({@link keptRowsOfParent}), ANDed into the same + * four reads (`find`, `findOne`, `count`, `aggregate`), so a list's `total`, + * its pages, a by-id read and a grouped count all agree with the rows served: * - * - The row is withheld exactly when the redaction would drop its kind. A - * reader served every watched field is served the row with its kind; a - * reader withheld only fields no milestone watches is served it too. - * - A declaration with no readable list is of unknown provenance and is judged - * as the text precedent judges one: served only to a reader not restricted - * on the parent. - * - A `kind` on a row the mirror did not write is the author's: not judged. - * - Rows written before the mirror declared the list carry no kind, so nothing - * on them says a milestone fired; they are judged by the rules above them, - * and age out with the object's retention, like the earlier shape's text. + * - an update row whose `changed_fields` is non-empty and holds no field this + * reader is served is withheld; a create, a delete, a note, and an update + * empty for everyone (it touched only `internal` fields, which the writer + * omits) are stamped `[]` and kept; + * - a milestone row is served only when its key equals the key of the parent's + * CURRENT `activityMilestones` declaration and this reader is served every + * field that declaration watches (ruled letter B). A row written under an + * earlier declaration, or naming a since-removed field, is withheld from + * every non-exempt reader until retention ages it out; + * - an UNSTAMPED row — at rest from before the columns existed and not yet + * backfilled, or one the planner could not judge — is withheld from every + * non-system reader the service answers for (ADR-0139 D4: an unjudged row + * fails closed). + * + * No row is read to build it — no SYSTEM pre-scan and no window — and no + * `ReadableIds` leaf is composed by it. Both columns are value-bearing: + * `changed_fields` is served narrowed to the fields this reader is served, + * `milestone_key` is never served, and the query guard + * (`parent-field-query-guard.ts`) refuses a predicate on either to a reader + * withheld a parent field, as it does for `metadata`. * * ## Fail closed * * An unexpected failure strips every value-bearing column from the rows of the * read rather than serving them unredacted — the sibling read gate's rule. A - * withheld-update pre-scan that fails denies the read, as the read gate does. + * withheld-row rule that cannot be built denies the read, as the read gate does. * The * security service's own "no answer" (no service wired, or an unresolvable * read projection) passes rows through, exactly as the approval snapshot and * the data plane itself do; a reader the service cannot answer MASKING for is * served no field (the contract's obligation, see {@link resolveServedFields}). * - * System-context reads (the audit writer, the gate's own pre-scan) and + * System-context reads (the audit writer, the backfill, a gate's bounded probe) and * context-less programmatic calls are not redacted, as for the read gate. */ -import type { CommentAccessEngine } from './comment-access-hooks.js'; -import { parseActivityParentObject, type ActivityMiddlewareEngine } from './activity-read-visibility.js'; -import { PARENT_GATE_READ_OPS, PARENT_GATE_SCAN_LIMIT, andIntoWhere } from './parent-record-read-gate.js'; +import { + ACTIVITY_CHANGED_FIELDS_COLUMN, + ACTIVITY_KIND_KEY, + ACTIVITY_MILESTONE_KEY_COLUMN, + ACTIVITY_MILESTONE_KIND, + ACTIVITY_TEXT_SOURCES_KEY, + declaredMilestoneWatchedFields, + milestoneKeyOf, +} from './activity-changed-fields.js'; +import { ACTIVITY_GATE, parseActivityParentObject, type ActivityMiddlewareEngine } from './activity-read-visibility.js'; +import { pinnedParentObject } from './parent-field-query-guard.js'; +import { + PARENT_GATE_READ_OPS, + andIntoWhere, + enumerateParentObjects, + type ParentRecordGateEngine, +} from './parent-record-read-gate.js'; import { dropUnservedKeys, ensureJudgedColumnsProjected, @@ -168,25 +191,12 @@ const ACTIVITY_OBJECT = 'sys_activity'; const SYSTEM_CTX = { isSystem: true } as const; /** - * The `metadata` key under which the CRUD mirror declares which PARENT fields - * each text column was composed from: `{ summary: string[], record_label: - * string[] }`. Written by `audit-writers.ts`, read (and stripped) here — one - * name, imported by both halves. + * The `metadata` keys the CRUD mirror declares a row's text provenance and its + * domain kind under, and the milestone kind — defined beside the stamp planner + * (`activity-changed-fields.ts`), re-exported here so this seam's public names + * are unchanged. */ -export const ACTIVITY_TEXT_SOURCES_KEY = 'text_sources'; - -/** - * [#22771] The `metadata` key a row's DOMAIN kind rides in. ADR-0052 §5: "`type` - * stays domain-NEUTRAL … Domain kind rides in `metadata.kind`." So a fired - * milestone keeps the `type` its declaration names (`completed`, or `updated` - * when it names none) and says what it is here, which is how a reader tells a - * "Case resolved" milestone from a finished task. Written by `audit-writers.ts`, - * judged here: one name, imported by both halves. - */ -export const ACTIVITY_KIND_KEY = 'kind'; - -/** [#22771] The kind the CRUD mirror stamps on a row a declared `activityMilestones` entry fired (ADR-0052 §5b.2). */ -export const ACTIVITY_MILESTONE_KIND = 'milestone'; +export { ACTIVITY_KIND_KEY, ACTIVITY_MILESTONE_KIND, ACTIVITY_TEXT_SOURCES_KEY }; /** The declaration's shape. */ export interface ActivityTextSources { @@ -205,8 +215,18 @@ export interface ActivityTextSources { const TEXT_COLUMNS = ['summary', 'record_label'] as const; type TextColumn = (typeof TEXT_COLUMNS)[number]; -/** The columns that can carry a parent field value. */ -const VALUE_BEARING_COLUMNS = ['summary', 'record_label', 'metadata'] as const; +/** + * The columns that can carry a parent field value — [ADR-0139 E6] the two + * writer-stamped columns included: `changed_fields` names which fields of the + * parent changed, and `milestone_key` which fields its declaration watched. + */ +const VALUE_BEARING_COLUMNS = [ + 'summary', + 'record_label', + 'metadata', + ACTIVITY_CHANGED_FIELDS_COLUMN, + ACTIVITY_MILESTONE_KEY_COLUMN, +] as const; /** The slice of the security contract this seam asks — the REAL interface. */ export type ActivityFieldVisibilitySource = FieldVisibilitySource; @@ -319,141 +339,138 @@ async function sourcesServed( return sources === 'unknown' ? !(await restricted()) : sources.every((f) => served.has(f)); } -/** The engine slice the withheld-update rule's pre-scan needs. */ -export type ActivityRedactionEngine = ActivityMiddlewareEngine & Pick; +/** + * The engine slice the withheld-row rule needs: the registry its parent objects + * are enumerated from, and each object's registered definition (its CURRENT + * `activityMilestones` declaration). No row is read. + */ +export type ActivityRedactionEngine = ActivityMiddlewareEngine & + Pick & { + getSchema?(object: string): unknown; + }; -/** The columns the withheld-update pre-scan reads: the row, its parent object - * and its stored change (with, [#22786], a milestone's kind and its declaration). */ -const WITHHELD_SCAN_COLUMNS = ['id', 'object_name', 'metadata'] as const; +/** No real row matches it: the withheld-row rule's fail-closed answer. */ +const WITHHELD_DENY_ALL = { id: '__activity_withheld_row_denied__' } as const; -/** No real row matches it: the withheld-update rule's fail-closed answer. */ -const WITHHELD_DENY_ALL = { id: '__activity_withheld_update_denied__' } as const; +/** The withheld-row rule's states an engine has already reported, per engine. */ +const withheldRuleReported = new WeakMap>(); -/** - * [#21388] Whether an activity row's STORED change is an update every one of - * whose keys a reader served `served` is withheld. False for a create or a - * delete (one side is not a record), for a change empty on both sides (empty - * for every reader), and for anything that is not a recorded change. - * Exported for direct testing. - */ -export function isWithheldOnlyUpdate(metadata: Record | null, served: ReadonlySet): boolean { - if (!metadata) return false; - const before = metadata.old; - const after = metadata.new; - if (!isRecord(before) || !isRecord(after)) return false; - const keys = new Set([...Object.keys(before), ...Object.keys(after)]); - if (keys.size === 0) return false; - for (const key of keys) if (served.has(key)) return false; - return true; +function reportOnce(engine: object, logger: ActivityRedactionLogger, key: string, message: string): void { + let reported = withheldRuleReported.get(engine); + if (!reported) withheldRuleReported.set(engine, (reported = new Set())); + if (reported.has(key)) return; + reported.add(key); + logger.warn(message); } -/** - * [#22786] Whether an activity row is a fired milestone, as the CRUD mirror - * declared it, whose watched fields are not all served to this reader. The - * watched fields are the list the mirror declares the row's - * `metadata.kind` from ({@link kindProvenance}), judged by the same answer - * the redaction serves that kind by, so the row is withheld exactly when the - * redaction would drop its kind. False for a row with no milestone kind and - * for a kind on a row the mirror did not write (the author's, served as - * written). `restricted` answers for a declaration with no readable list. - * Exported for direct testing. - */ -export async function isWithheldMilestone( - metadata: Record | null, - served: ReadonlySet, - restricted: () => Promise, -): Promise { - if (!metadata || metadata[ACTIVITY_KIND_KEY] !== ACTIVITY_MILESTONE_KIND) return false; - const watched = kindProvenance(metadata); - if (watched === null) return false; - return !(await sourcesServed(watched, served, restricted)); +/** The CURRENT `activityMilestones` declaration of `object`: its watched fields, or `[]`. */ +function currentMilestoneWatched(engine: ActivityRedactionEngine, object: string): string[] { + try { + return declaredMilestoneWatchedFields(typeof engine.getSchema === 'function' ? engine.getSchema(object) : undefined); + } catch { + return []; + } } /** - * [#21388, #22786] The ONE withheld-row judgement the pre-scan makes: a row - * whose very presence speaks about a field this reader is not served, which - * narrowing its columns cannot take back. Either an update whose every - * recorded change is withheld ({@link isWithheldOnlyUpdate}), or a fired - * milestone whose watched fields are not all served - * ({@link isWithheldMilestone}). Exported for direct testing. + * [ADR-0139 D4, E6] The rows of ONE parent object `P` a reader served + * `served` keeps, as a predicate over the two writer-stamped columns + * (`activity-changed-fields.ts`): + * + * - **stamped** — `changed_fields` is not null. An unstamped row (written + * before the column existed and not yet backfilled, or one the planner + * could not judge) is withheld: ADR-0139 D4's answer for an unjudged row. + * - **the update half** (#21388) — `changed_fields` is empty, or it contains + * a field this reader is served: one `$contains` per served field. So an + * update whose every recorded key is withheld is withheld, and a create, a + * delete, a note, and an update that touched only `internal` fields (empty + * for everyone) are kept. + * - **the milestone half** (#22786, ruled letter B) — `milestone_key` is null + * (no milestone fired), or it EQUALS the key of P's current + * `activityMilestones` declaration AND this reader is served every field + * that declaration watches. A milestone row written under an earlier + * declaration, or naming a since-removed field, is withheld from every + * non-exempt reader until retention ages it out. + * + * Exported for direct testing. */ -export async function isWithheldRow( - metadata: Record | null, - served: ReadonlySet, - restricted: () => Promise, -): Promise { - return isWithheldOnlyUpdate(metadata, served) || (await isWithheldMilestone(metadata, served, restricted)); +export function keptRowsOfParent( + parent: string, + served: readonly string[], + watched: readonly string[], +): Record { + const servedSet = new Set(served); + const members = [...servedSet].sort(); + const key = milestoneKeyOf(watched); + const milestoneServed = key !== null && watched.every((f) => servedSet.has(f)); + return { + $and: [ + { object_name: parent }, + { [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $null: false } }, + { + $or: [ + { [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $empty: true } }, + ...members.map((f) => ({ [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $contains: f } })), + ], + }, + milestoneServed + ? { $or: [{ [ACTIVITY_MILESTONE_KEY_COLUMN]: { $null: true } }, { [ACTIVITY_MILESTONE_KEY_COLUMN]: key }] } + : { [ACTIVITY_MILESTONE_KEY_COLUMN]: { $null: true } }, + ], + }; } -/** No full answer is the closed answer: see {@link RestrictedFor}. */ -const RESTRICTED_CLOSED: RestrictedFor = async () => true; - /** - * [#21388] The WHERE that withholds, from one read, every row - * {@link isWithheldRow} withholds from the reader `servedFor` answers for: - * `null` when the read withholds nothing, `{ id: { $nin: WITHHELD } }` when the - * pre-scan saw every row the read can touch, and `{ id: { $in: KEPT } }` when - * it reached its bound (fail closed: an unjudged row is never served). - * [#22786] `restrictedFor` is the read's answer for a declaration of unknown - * provenance; without one, such a row is judged restricted (closed). + * [ADR-0139 D4, E6] The WHERE that withholds, from one read, every row the + * withheld-row rule withholds from the reader `servedFor` answers for: one + * {@link keptRowsOfParent} branch per parent object, ORed — or `null` when the + * service gives no answer for any of them (nothing is narrowed). + * + * The parents are the read gate's own enumeration (`enumerateParentObjects`, + * every registered object the CRUD mirror can write about), narrowed to the + * ONE parent the read pins by equality on `object_name` when it pins one + * (`pinnedParentObject`, the query guard's reading): no row of any other parent + * can match such a read. A parent the service gives no answer for is kept + * whole (`{ object_name: P }`), as the redaction serves it. + * + * It reads no row, as no context: no SYSTEM pre-scan, no window, and no leaf + * composed for anyone (the security service's own field answers are its only + * input). An engine with no registry view cannot say which parents exist, so + * nothing can be judged: the read is denied, and that is said once. * Exported for direct testing. */ -export async function computeWithheldUpdateFilter( - engine: Pick, +export async function computeWithheldRowFilter( + engine: ActivityRedactionEngine, ast: Record, servedFor: ServedFor, logger: ActivityRedactionLogger, - restrictedFor: RestrictedFor = RESTRICTED_CLOSED, ): Promise { - // The rows the read would touch, under SYSTEM context (the reader may not be - // served their change; that is what is being decided). The caller's own - // order rides along, so a bounded window is the one the caller pages through. - const orderBy = ast.orderBy; - const candidates = await engine.find(ACTIVITY_OBJECT, { - where: (ast.where as Record | undefined) ?? {}, - fields: [...WITHHELD_SCAN_COLUMNS], - ...(Array.isArray(orderBy) && orderBy.length > 0 ? { orderBy } : {}), - limit: PARENT_GATE_SCAN_LIMIT, - context: { ...SYSTEM_CTX }, - }); - if (!candidates.length) return null; - - const servedSets = new Map | undefined>(); - const withheld: unknown[] = []; - const kept: unknown[] = []; - for (const row of candidates) { - let isWithheld = false; - const object = parseActivityParentObject(row); - if (object) { - if (!servedSets.has(object)) { - const answer = await servedFor(object); - servedSets.set(object, answer === undefined ? undefined : new Set(answer.map(String))); - } - const served = servedSets.get(object); - isWithheld = - served !== undefined && - (await isWithheldRow(parseMetadata(row.metadata), served, () => restrictedFor(object, served))); - } - const id = row.id; - const usable = (typeof id === 'string' || typeof id === 'number') && String(id) !== ''; - // A row is excluded by its stored id, so a withheld row without one - // cannot be excluded at all: deny the read rather than serve it. - if (!usable) { - if (isWithheld) return WITHHELD_DENY_ALL; + const pinned = pinnedParentObject(ast.where, ACTIVITY_OBJECT); + const parents = pinned !== null ? [pinned] : enumerateParentObjects(engine, ACTIVITY_GATE); + if (parents === null) { + reportOnce( + engine, + logger, + 'no-registry', + `[audit] ${REDACTION_SEAM}: the engine exposes no registry view, so the parent objects of the activity ` + + 'stream cannot be enumerated and no row can be judged: every non-system read that names no one parent ' + + 'object is denied (fail-closed). Reported once for this engine.', + ); + return WITHHELD_DENY_ALL; + } + const branches: Array> = []; + let narrowed = false; + for (const parent of parents) { + const served = await servedFor(parent); + if (served === undefined) { + branches.push({ object_name: parent }); continue; } - (isWithheld ? withheld : kept).push(id); - } - - if (candidates.length >= PARENT_GATE_SCAN_LIMIT) { - logger.warn( - `[audit] ${REDACTION_SEAM}: the withheld-update pre-scan hit the ${PARENT_GATE_SCAN_LIMIT}-row cap; ` + - 'this broad read is answered from the rows it judged (fail-closed) and may omit visible rows — ' + - 'scope the query by object_name and record_id', - ); - return kept.length ? { id: { $in: kept } } : WITHHELD_DENY_ALL; + narrowed = true; + branches.push(keptRowsOfParent(parent, served.map(String), currentMilestoneWatched(engine, parent))); } - return withheld.length ? { id: { $nin: withheld } } : null; + if (!narrowed) return null; + return branches.length === 1 ? branches[0] : { $or: branches }; } /** @@ -483,6 +500,8 @@ export async function redactActivityRows( if (!isRecord(item)) continue; const row = item; if (!VALUE_BEARING_COLUMNS.some((col) => col in row)) continue; + // [ADR-0139 E6] The milestone key is bookkeeping for the read rule: never served. + delete row[ACTIVITY_MILESTONE_KEY_COLUMN]; const object = parseActivityParentObject(row); // The read gate serves no row without a parent; a row that reaches here // without one has nothing to be judged against. @@ -494,6 +513,16 @@ export async function redactActivityRows( if (answer === undefined) continue; const servedSet = new Set(answer.map(String)); + // [ADR-0139 E6] The recorded change's key set, narrowed to the served + // members, as `metadata.old` / `metadata.new` are narrowed key by key. + const changedFields = row[ACTIVITY_CHANGED_FIELDS_COLUMN]; + if (Array.isArray(changedFields)) { + row[ACTIVITY_CHANGED_FIELDS_COLUMN] = changedFields.filter((f) => servedSet.has(String(f))); + } else if (changedFields !== undefined && changedFields !== null) { + // Not the stored shape: nothing here can narrow it, so it is not served. + delete row[ACTIVITY_CHANGED_FIELDS_COLUMN]; + } + const metadata = 'metadata' in row ? parseMetadata(row.metadata) : null; const provenance = textProvenance(metadata); const kindSources = kindProvenance(metadata); @@ -538,12 +567,10 @@ const JUDGED_BY = ['object_name', 'metadata'] as const; * resolved on every read: the security plugin may register after this one. * Inert on an engine without the middleware seam; `AuditPlugin` says so. * - * [#21388] Before the read runs, the same middleware withholds every update row - * whose stored change is withheld whole from this reader, and [#22786] every - * fired milestone row whose watched fields it is not all served, as a WHERE on - * all four reads (`computeWithheldUpdateFilter`). `AuditPlugin` registers it after - * the read gate, so the gate's parent filter is already in the WHERE its - * pre-scan reads. + * [#21388, #22786] Before the read runs, the same middleware ANDs the + * withheld-row predicate into the WHERE of all four reads + * (`computeWithheldRowFilter`, ADR-0139 E6). `AuditPlugin` registers it after + * the read gate; the two are independent conjuncts. */ export function installActivityFieldRedaction( engine: ActivityRedactionEngine, @@ -560,12 +587,12 @@ export function installActivityFieldRedaction( const restrictedFor = restrictedPerRead(security); if (security && ctx.ast && PARENT_GATE_READ_OPS.has(ctx.operation)) { try { - const filter = await computeWithheldUpdateFilter(engine, ctx.ast, servedFor, logger, restrictedFor); + const filter = await computeWithheldRowFilter(engine, ctx.ast, servedFor, logger); if (filter) andIntoWhere(ctx, filter); } catch (err) { - // A pre-scan failure must never fall open into the timing leak. + // A failure to build the rule must never fall open into the timing leak. logger.warn( - `[audit] ${REDACTION_SEAM}: the withheld-update pre-scan failed, denying all ` + + `[audit] ${REDACTION_SEAM}: the withheld-row rule could not be built, denying all ` + `(${(err as Error)?.message ?? err})`, ); andIntoWhere(ctx, WITHHELD_DENY_ALL); diff --git a/packages/plugins/plugin-audit/src/activity-read-visibility.ts b/packages/plugins/plugin-audit/src/activity-read-visibility.ts index 990edd2bc6e..fcdeea7495e 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.ts @@ -57,19 +57,17 @@ * programmatic calls on bare kernels are not narrowed, as for comments: every * real transport carries a context. * - * [#21388] One more per-reader ROW rule rides this mechanism: an update row - * whose stored change is withheld whole from the reader is withheld as a row, - * by a WHERE built from a SYSTEM pre-scan on the same four reads, so a list's - * total, its pages and a grouped count agree with the rows served. It judges by - * the security service's field answer, so it lives with the field redaction - * (`activity-field-redaction.ts`, `computeWithheldUpdateFilter`), whose - * middleware `AuditPlugin` registers after this one: its pre-scan reads the - * WHERE this gate has already narrowed. ⚠️ It reads it as SYSTEM, so a - * `ReadableIds` leaf in that WHERE is composed for the system there (every id - * of its parent), not for the reader: its own 2,000-row window still bounds a - * broad read of this stream until that rule becomes a predicate (ADR-0139 E6). - * The rows it lets through are still ANDed with this gate's leaves, composed - * for the reader. + * [#21388, #22786] One more per-reader ROW rule rides this mechanism: an + * update row whose recorded change is withheld whole from the reader, and a + * fired milestone row whose watched fields it is not served, are withheld as + * rows, on the same four reads, so a list's total, its pages and a grouped + * count agree with the rows served. It judges by the security service's field + * answer, so it lives with the field redaction (`activity-field-redaction.ts`, + * `computeWithheldRowFilter`), whose middleware `AuditPlugin` registers after + * this one. [ADR-0139 E6] It is a predicate over two writer-stamped columns + * (`changed_fields`, `milestone_key`), one branch per parent object this gate + * enumerates ({@link ACTIVITY_GATE}): it reads no row and composes no leaf, so + * nothing bounds a broad read of this stream but this gate's own leaves. */ import type { CommentAccessLogger, CommentReadMiddlewareCtx } from './comment-access-hooks.js'; @@ -92,7 +90,7 @@ const ACTIVITY_OBJECT = 'sys_activity'; * caller is exempt. The compliance ledger's audit capability exempts its holder * from the ledger's gate only (#21260: the ruling names the ledger). */ -const ACTIVITY_GATE: ParentRecordGate = { +export const ACTIVITY_GATE: ParentRecordGate = { object: ACTIVITY_OBJECT, seam: 'activity read visibility', denyAll: { id: '__activity_parent_denied__' }, diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index 790ceaaf910..eee25bc132a 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -85,6 +85,13 @@ import { type ActivityTextSources, } from './activity-field-redaction.js'; import type { LedgerRecordWriteAction } from './audit-log-field-redaction.js'; +// [ADR-0139 E6] The ONE derivation of the two writer-stamped columns the +// withheld-row rule reads, shared with the before-insert hook and the backfill. +import { + declaredMilestoneWatchedFields, + installActivityStampHook, + planActivityStamp, +} from './activity-changed-fields.js'; /** * Minimal structural view of `NotificationService.emit` (ADR-0030). Declared @@ -835,7 +842,9 @@ function matchMilestone( return { template: m.summary, type: typeof m.type === 'string' ? m.type : undefined, - watched: [...new Set(milestones.filter(asked).map((w: any) => w.field))], + // [ADR-0139 E6] The same list the read rule keys the CURRENT + // declaration by, from the same function. + watched: declaredMilestoneWatchedFields(objectDef), }; } } @@ -2089,6 +2098,12 @@ export function installAuditWriters( const actor: ActorSnapshot = mirrorsActivity && userId ? await resolveActorSnapshot(api, userId) : NO_ACTOR_SNAPSHOT; + const activityMetadata = { + old: oldValue, + new: newValue, + ...(kind ? { [ACTIVITY_KIND_KEY]: kind.value } : {}), + [ACTIVITY_TEXT_SOURCES_KEY]: textSources, + }; const activityRow: Record = { type: activityType, // Explicit ISO timestamp — `defaultValue: 'NOW()'` on the column @@ -2109,12 +2124,11 @@ export function installAuditWriters( // came from. The read side strips the declaration before serving. // [#22771] A fired milestone's kind rides beside them, judged on read // against the fields the declaration names for it. - metadata: safeStringify({ - old: oldValue, - new: newValue, - ...(kind ? { [ACTIVITY_KIND_KEY]: kind.value } : {}), - [ACTIVITY_TEXT_SOURCES_KEY]: textSources, - }), + metadata: safeStringify(activityMetadata), + // [ADR-0139 E6] The two writer-stamped columns the withheld-row rule is + // a predicate over, from the metadata this row records (the + // before-insert hook re-derives them by the same planner). + ...(planActivityStamp(activityMetadata) ?? { changed_fields: null, milestone_key: null }), }; // Same rationale as auditRow: stamp the tenant column so RLS matches the // recipient's organization on read — but only when the (auto-injected) @@ -2161,6 +2175,9 @@ export function installAuditWriters( // `afterDelete` are the two the bulk gates (#5038) read, and `afterUpdate` is // the one #5284's single-id gate reads. engine.registerHook('afterInsert', writeAudit, { excludeObjects: AUDIT_EXCLUDED_OBJECTS, packageId }); + // [ADR-0139 E6, Q2 = b] Every `sys_activity` insert, whoever writes it, is + // stamped by the one planner — the mirror's own rows included. + installActivityStampHook(engine as any, packageId); engine.registerHook('afterUpdate', writeAudit, { excludeObjects: AUDIT_EXCLUDED_OBJECTS, packageId }); engine.registerHook('afterDelete', writeAudit, { excludeObjects: AUDIT_EXCLUDED_OBJECTS, packageId }); diff --git a/packages/plugins/plugin-audit/src/index.ts b/packages/plugins/plugin-audit/src/index.ts index b7108e8d8bf..7456ae3be0a 100644 --- a/packages/plugins/plugin-audit/src/index.ts +++ b/packages/plugins/plugin-audit/src/index.ts @@ -52,6 +52,21 @@ export type { StoredMetadataBodyMigrationEngine, StoredMetadataBodyMigrationReport, } from './stored-metadata-body-migration.js'; +// [ADR-0139 E6] The stored-row backfill of `sys_activity`'s two writer-stamped +// read-rule columns, and the one planner it, the CRUD mirror and the +// before-insert hook stamp by. The CLI command +// (`os migrate activity-changed-fields`) drives it. +export { backfillActivityChangedFields, ACTIVITY_BACKFILL_PAGE_SIZE } from './activity-changed-fields-backfill.js'; +export type { + ActivityChangedFieldsBackfillEngine, + ActivityChangedFieldsBackfillReport, +} from './activity-changed-fields-backfill.js'; +export { + ACTIVITY_CHANGED_FIELDS_COLUMN, + ACTIVITY_MILESTONE_KEY_COLUMN, + planActivityStamp, +} from './activity-changed-fields.js'; +export type { ActivityStamp } from './activity-changed-fields.js'; export type { CommentAccessEngine, CommentAccessLogger, diff --git a/packages/plugins/plugin-audit/src/objects/sys-activity.object.ts b/packages/plugins/plugin-audit/src/objects/sys-activity.object.ts index 92d0d2cc0fe..a01d8be7014 100644 --- a/packages/plugins/plugin-audit/src/objects/sys-activity.object.ts +++ b/packages/plugins/plugin-audit/src/objects/sys-activity.object.ts @@ -277,6 +277,36 @@ export const SysActivity = ObjectSchema.create({ description: 'JSON-serialized additional context', group: 'Context', }), + + // ── Writer-stamped read-rule columns (ADR-0139 D4, E6) ─────────── + // Stamped at write time by ONE derivation (`activity-changed-fields.ts`): + // the CRUD mirror, a before-insert hook for every other writer, and the + // stored-row backfill (`os migrate activity-changed-fields`). The + // withheld-row rule (`activity-field-redaction.ts`) is a predicate over + // them; a row with `changed_fields` NULL is unstamped and is withheld from + // every non-system reader until the backfill stamps it. + changed_fields: { + type: 'tags', + label: 'Changed Fields', + required: false, + readonly: true, + description: + 'The key set of the recorded change: for an update, every field its recorded old/new values name; ' + + 'empty for every other row. Stamped by the platform at write time; served narrowed to the fields the ' + + 'reader is served. NULL means the row is not stamped yet (run `os migrate activity-changed-fields`).', + group: 'Context', + }, + + milestone_key: Field.text({ + label: 'Milestone Key', + required: false, + readonly: true, + description: + 'For a fired activity milestone, one sorted, comma-joined key of every field the object\'s ' + + 'activityMilestones declaration watched when it fired; NULL for every other row. Stamped by the ' + + 'platform at write time and never served: the read rule compares it with the current declaration.', + group: 'Context', + }), }, indexes: [ diff --git a/packages/plugins/plugin-audit/src/parent-field-query-guard.ts b/packages/plugins/plugin-audit/src/parent-field-query-guard.ts index 7d35c38b0c3..4094787cdbb 100644 --- a/packages/plugins/plugin-audit/src/parent-field-query-guard.ts +++ b/packages/plugins/plugin-audit/src/parent-field-query-guard.ts @@ -15,7 +15,8 @@ * values of that record's fields in a few columns, written once, as the system: * * - `sys_activity` — `summary`, `record_label` and `metadata` (the field - * redaction's columns, `activity-field-redaction.ts`, #21081); + * redaction's columns, `activity-field-redaction.ts`, #21081), and the two + * writer-stamped columns `changed_fields` and `milestone_key` (ADR-0139 E6); * - `sys_audit_log` — `old_value` and `new_value`, the before/after snapshots. * * A read-time redaction narrows those values on the rows a reader is SERVED, @@ -109,6 +110,7 @@ */ import { FieldReferenceSchema } from '@objectstack/spec/data'; +import { ACTIVITY_STAMPED_COLUMNS } from './activity-changed-fields.js'; import { resolveServedFields, type FieldRedactionLogger, type FieldVisibilitySource } from './served-fields.js'; import { parseActivityParentObject, type ActivityMiddlewareEngine } from './activity-read-visibility.js'; @@ -141,7 +143,9 @@ const remedy = (what: string) => */ export const ACTIVITY_QUERY_GUARD: ParentFieldQueryGuard = Object.freeze({ object: 'sys_activity', - columns: Object.freeze(['summary', 'record_label', 'metadata']), + // [ADR-0139 E6] The two writer-stamped columns too: a membership probe on + // `changed_fields` over a served row would count changes to a withheld field. + columns: Object.freeze(['summary', 'record_label', 'metadata', ...ACTIVITY_STAMPED_COLUMNS]), remedy: remedy('An activity column'), }); From 5cc3fc0b25b6b015dd2409027017f1af3d41e48c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 02:51:05 +0000 Subject: [PATCH 02/11] wip(plugin-audit): E6 tests for stamp and predicate Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- .../src/activity-field-redaction.ts | 18 +- ...ctivity-milestone-kind.integration.test.ts | 163 +++++++++------ ...tivity-withheld-update.integration.test.ts | 195 +++++++++++++----- 3 files changed, 247 insertions(+), 129 deletions(-) diff --git a/packages/plugins/plugin-audit/src/activity-field-redaction.ts b/packages/plugins/plugin-audit/src/activity-field-redaction.ts index d86a599bde7..ffb025b0a30 100644 --- a/packages/plugins/plugin-audit/src/activity-field-redaction.ts +++ b/packages/plugins/plugin-audit/src/activity-field-redaction.ts @@ -363,13 +363,13 @@ function reportOnce(engine: object, logger: ActivityRedactionLogger, key: string logger.warn(message); } -/** The CURRENT `activityMilestones` declaration of `object`: its watched fields, or `[]`. */ +/** + * The CURRENT `activityMilestones` declaration of `object`: its watched fields, + * or `[]`. A definition that cannot be read throws, and the middleware denies + * the read (fail closed) rather than judge its milestone rows unread. + */ function currentMilestoneWatched(engine: ActivityRedactionEngine, object: string): string[] { - try { - return declaredMilestoneWatchedFields(typeof engine.getSchema === 'function' ? engine.getSchema(object) : undefined); - } catch { - return []; - } + return declaredMilestoneWatchedFields(typeof engine.getSchema === 'function' ? engine.getSchema(object) : undefined); } /** @@ -446,7 +446,11 @@ export async function computeWithheldRowFilter( logger: ActivityRedactionLogger, ): Promise { const pinned = pinnedParentObject(ast.where, ACTIVITY_OBJECT); - const parents = pinned !== null ? [pinned] : enumerateParentObjects(engine, ACTIVITY_GATE); + const parents = pinned !== null ? [pinned] : enumerateParentObjects( + // It reads the registry view alone (the read gate's own enumeration). + engine as ParentRecordGateEngine, + ACTIVITY_GATE, + ); if (parents === null) { reportOnce( engine, diff --git a/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts b/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts index aebd28e285f..0ce82c230aa 100644 --- a/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-milestone-kind.integration.test.ts @@ -42,6 +42,14 @@ * The security service, answering the two contract members the redaction asks, * per reader, in the contract's shape (as in `activity-field-redaction.test.ts`). * + * [ADR-0139 E6, ruled letter B] The row half is a predicate over the + * writer-stamped `milestone_key`: a milestone row is served only when its key + * equals the key of the object's CURRENT declaration and the reader is served + * every field that declaration watches. Pinned below beside the earlier + * behaviour: a declaration edit withholds the rows written under the old one + * from every non-system reader, and an unjudgeable milestone row is stored + * unstamped and withheld. + * * ⚠️ Disclosure discipline: no test title states a value. */ @@ -51,7 +59,8 @@ import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; import { AuditPlugin } from './audit-plugin.js'; -import { computeWithheldUpdateFilter, isWithheldMilestone, isWithheldRow } from './activity-field-redaction.js'; +import { keptRowsOfParent } from './activity-field-redaction.js'; +import { declaredMilestoneWatchedFields, milestoneKeyOf, planActivityStamp } from './activity-changed-fields.js'; const ACTIVITY = 'sys_activity'; const ITEM = 'amk_item'; @@ -204,6 +213,63 @@ describe('[#22771] a fired milestone is served with metadata.kind, judged like t } }); + it('[ADR-0139 E6] at rest: each fired milestone is stamped with the key of the watched fields, every other row with none', async () => { + const atRest = await read(SYS); + const keyOf = (id: string) => atRest.find((r) => r.id === id)?.milestone_key ?? null; + expect(keyOf(rowIds.typedMilestone)).toBe('phase,stage'); + expect(keyOf(rowIds.untypedMilestone)).toBe('phase,stage'); + for (const id of [rowIds.created, rowIds.tracked, rowIds.appKind]) expect(keyOf(id), id).toBeNull(); + expect(milestoneKeyOf(declaredMilestoneWatchedFields(itemObject))).toBe('phase,stage'); + }); + + it('[ADR-0139 E6] the milestone key is never served, to any non-system reader', async () => { + for (const context of [CONTROL, NOTE_WITHHELD, WATCH_WITHHELD]) { + for (const row of await read(context)) expect(row, String(row.id)).not.toHaveProperty('milestone_key'); + } + }); + + it('[ADR-0139 E6, letter B] after the declaration changes, the rows written under the old one are withheld from every non-system reader', async () => { + const edited = { + ...itemObject, + activityMilestones: [itemObject.activityMilestones[0]], // `phase` no longer watched + }; + engine.registry.registerObject(edited as any, HARNESS_PACKAGE); + try { + for (const context of [CONTROL, NOTE_WITHHELD, WATCH_WITHHELD]) { + const ids = (await read(context)).map((r) => r.id); + expect(ids).not.toContain(rowIds.typedMilestone); + expect(ids).not.toContain(rowIds.untypedMilestone); + expect(ids).toContain(rowIds.tracked); + } + expect((await read(SYS)).map((r) => r.id)).toContain(rowIds.typedMilestone); + } finally { + engine.registry.registerObject(itemObject as any, HARNESS_PACKAGE); + } + // Control: under the declaration they were written under, served again. + expect((await read(CONTROL)).map((r) => r.id)).toContain(rowIds.typedMilestone); + }); + + it('[ADR-0139 E6] a milestone row the planner cannot judge is stored unstamped and withheld from every non-system reader', async () => { + const id = String((await engine.insert(ACTIVITY, { + type: 'note', summary: 'unjudged', object_name: ITEM, record_id: itemId, + timestamp: new Date().toISOString(), + metadata: JSON.stringify({ kind: 'milestone', text_sources: { summary: [], record_label: [] } }), + // A writer's own stamp is not taken. + changed_fields: [], milestone_key: 'phase,stage', + }, { context: SYS })).id); + try { + const atRest = (await read(SYS)).find((r) => r.id === id); + expect(atRest?.changed_fields ?? null).toBeNull(); + expect(atRest?.milestone_key ?? null).toBeNull(); + for (const context of [CONTROL, NOTE_WITHHELD, WATCH_WITHHELD]) { + expect((await read(context)).map((r) => r.id)).not.toContain(id); + } + } finally { + await engine.delete(ACTIVITY, { where: { id }, context: SYS } as any); + } + expect(await read(SYS)).toHaveLength(5); + }); + it('at rest: type stays what the declaration names — no new type value', async () => { const atRest = await read(SYS); expect(atRest.find((r) => r.id === rowIds.typedMilestone)?.type).toBe('completed'); @@ -280,28 +346,27 @@ describe('[#22771] a fired milestone is served with metadata.kind, judged like t expect(await engine.count(ACTIVITY, { where }, { context: CONTROL })).toBe(5); }); - it('fail closed: when the withheld-row pre-scan fails, the read is denied, not served unjudged', async () => { - // The pre-scan is the system-context read of the row, its parent and its - // metadata; make that one read fail and nothing else. - const original = engine.find.bind(engine); - const spy = vi.spyOn(engine, 'find').mockImplementation((async (object: string, query: any, ...rest: any[]) => { - const fields: unknown = query?.fields; - if (object === ACTIVITY && query?.context?.isSystem && Array.isArray(fields) && fields.includes('metadata')) { - throw new Error('pre-scan unavailable'); + it('fail closed: when the withheld-row rule cannot be built, the read is denied, not served unjudged', async () => { + // The rule reads the parent's CURRENT declaration; make that one read + // fail and nothing else. + const original = (engine as any).getSchema.bind(engine); + const spy = vi.spyOn(engine as any, 'getSchema').mockImplementation(((object: string) => { + if (object === ITEM && new Error().stack?.includes('currentMilestoneWatched')) { + throw new Error('definition unavailable'); } - return (original as any)(object, query, ...rest); + return original(object); }) as any); try { for (const context of [CONTROL, WATCH_WITHHELD]) { expect(await read(context)).toEqual([]); expect(await engine.count(ACTIVITY, { where: { object_name: ITEM, record_id: itemId } }, { context })).toBe(0); } - // The system read takes no pre-scan, so the failure cannot reach it. + // The system read takes no rule, so the failure cannot reach it. expect(await read(SYS)).toHaveLength(5); } finally { spy.mockRestore(); } - // Control: with the pre-scan back, the read is served again. + // Control: with the definition back, the read is served again. expect((await read(CONTROL)).length).toBe(5); }); @@ -336,74 +401,36 @@ describe('[#22771] a fired milestone is served with metadata.kind, judged like t }); }); -describe('[#22786] isWithheldMilestone reads the declared watched fields', () => { - const served = new Set(['a', 'b']); - const notRestricted = async () => false; - const restricted = async () => true; +describe('[ADR-0139 E6] the milestone key: stamped by the planner, compared by the predicate', () => { const milestone = (watched: unknown) => ({ old: { a: 1 }, new: { a: 2 }, kind: 'milestone', text_sources: { summary: [], record_label: [], ...(watched === undefined ? {} : { kind: watched }) }, }); - it('a milestone whose watched fields are all served is not withheld', async () => { - expect(await isWithheldMilestone(milestone(['a', 'b']), served, restricted)).toBe(false); - }); - - it('a milestone with a watched field the reader is not served is withheld', async () => { - expect(await isWithheldMilestone(milestone(['a', 'c']), served, notRestricted)).toBe(true); + it('a fired milestone the mirror declared is keyed by its watched fields, sorted and distinct', () => { + expect(planActivityStamp(milestone(['b', 'a', 'b']))).toEqual({ changed_fields: ['a'], milestone_key: 'a,b' }); }); - it('no milestone kind, or a kind on a row the mirror did not write, is not judged', async () => { + it('no milestone kind, or a kind on a row the mirror did not write, carries no key', () => { const { kind: _kind, ...noKind } = milestone(['c']); - expect(await isWithheldMilestone(noKind, served, restricted)).toBe(false); - expect(await isWithheldMilestone({ ...milestone(['c']), kind: 'email' }, served, restricted)).toBe(false); - expect(await isWithheldMilestone({ kind: 'milestone' }, served, restricted)).toBe(false); - expect(await isWithheldMilestone(null, served, restricted)).toBe(false); + expect(planActivityStamp(noKind)?.milestone_key).toBeNull(); + expect(planActivityStamp({ ...milestone(['c']), kind: 'email' })?.milestone_key).toBeNull(); + expect(planActivityStamp({ kind: 'milestone' })?.milestone_key).toBeNull(); }); - it('a declaration with no readable list is judged by the unknown-provenance answer', async () => { - for (const watched of [undefined, 'c', [1]]) { - expect(await isWithheldMilestone(milestone(watched), served, restricted)).toBe(true); - expect(await isWithheldMilestone(milestone(watched), served, notRestricted)).toBe(false); + it('a declaration with no readable list cannot be judged: no stamp at all', () => { + for (const watched of [undefined, 'c', [1], ['a,b']]) { + expect(planActivityStamp(milestone(watched)), JSON.stringify(watched)).toBeNull(); } }); - it('isWithheldRow is either arm: the withheld-only update, or the withheld milestone', async () => { - expect(await isWithheldRow({ old: { c: 1 }, new: { c: 2 } }, served, notRestricted)).toBe(true); - expect(await isWithheldRow(milestone(['c']), served, notRestricted)).toBe(true); - expect(await isWithheldRow(milestone(['a']), served, restricted)).toBe(false); - expect(await isWithheldRow({ old: { a: 1 }, new: { a: 2 } }, served, restricted)).toBe(false); - }); -}); - -describe('[#22786] computeWithheldUpdateFilter — the milestone arm rides the one pre-scan', () => { - const row = (id: string, metadata: Record) => ({ - id, object_name: 'obj', metadata: JSON.stringify(metadata), - }); - const milestoneRow = (id: string, watched?: string[]) => row(id, { - old: { open: 1 }, new: { open: 2 }, kind: 'milestone', - text_sources: { summary: [], record_label: [], ...(watched ? { kind: watched } : {}) }, - }); - const engineOf = (rows: Record[]) => ({ find: vi.fn(async () => rows) }); - const servedFor = async () => ['open']; - - it('a milestone row with an unserved watched field is ANDed out beside the served rows', async () => { - const rows = [milestoneRow('m1', ['open', 'secret']), milestoneRow('m2', ['open']), row('t1', { old: { open: 1 }, new: { open: 2 } })]; - expect(await computeWithheldUpdateFilter(engineOf(rows), {}, servedFor, { warn: vi.fn() })) - .toEqual({ id: { $nin: ['m1'] } }); - }); - - it('a declaration with no readable list: closed without an answer, and judged by the answer when given one', async () => { - const rows = [milestoneRow('u1')]; - expect(await computeWithheldUpdateFilter(engineOf(rows), {}, servedFor, { warn: vi.fn() })) - .toEqual({ id: { $nin: ['u1'] } }); - const answered = vi.fn(async () => false); - expect(await computeWithheldUpdateFilter(engineOf(rows), {}, servedFor, { warn: vi.fn() }, answered)).toBeNull(); - expect(answered).toHaveBeenCalledWith('obj', new Set(['open'])); - }); - - it('a reader the service gives no answer for is narrowed by neither arm', async () => { - const rows = [milestoneRow('m1', ['secret'])]; - expect(await computeWithheldUpdateFilter(engineOf(rows), {}, async () => undefined, { warn: vi.fn() })).toBeNull(); + it('served only under the current declaration, to a reader served every field it watches', () => { + const milestoneBranch = (served: string[], watched: string[]) => + (keptRowsOfParent('obj', served, watched).$and as unknown[])[3]; + expect(milestoneBranch(['a', 'b'], ['a', 'b'])).toEqual({ + $or: [{ milestone_key: { $null: true } }, { milestone_key: 'a,b' }], + }); + expect(milestoneBranch(['a'], ['a', 'b'])).toEqual({ milestone_key: { $null: true } }); + expect(milestoneBranch(['a', 'b'], [])).toEqual({ milestone_key: { $null: true } }); }); }); diff --git a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts index c4d4d96229a..60c1729ea2b 100644 --- a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts @@ -16,6 +16,11 @@ * - creates and deletes keep their rows; * - the count and every listing face agree with the rows served. * + * [ADR-0139 E6] The rule is a predicate over the writer-stamped + * `changed_fields` column (`activity-changed-fields.ts`), with no SYSTEM + * pre-scan: the same scene, read through it, plus the unstamped row it + * withholds from every non-system reader. + * * ## Why a real engine, a real driver and the real plugin * * The rule is a WHERE the redaction's middleware ANDs into the read, so whether @@ -41,8 +46,8 @@ import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; import { AuditPlugin } from './audit-plugin.js'; -import { computeWithheldUpdateFilter, isWithheldOnlyUpdate } from './activity-field-redaction.js'; -import { PARENT_GATE_SCAN_LIMIT } from './parent-record-read-gate.js'; +import { computeWithheldRowFilter, keptRowsOfParent } from './activity-field-redaction.js'; +import { planActivityStamp } from './activity-changed-fields.js'; const ACTIVITY = 'sys_activity'; /** A record the member reads with two fields withheld. */ @@ -177,6 +182,25 @@ describe('[#21388] an update whose every recorded change is withheld from the re const sealedRows = (await engine.find(ACTIVITY, { where: { object_name: SEALED }, context: SYS })) as Row[]; rowIds.sealedCreated = String(sealedRows.find((r) => r.type === 'created')?.id ?? ''); rowIds.sealedUpdated = String(sealedRows.find((r) => r.type === 'updated')?.id ?? ''); + + // [ADR-0139 E6] An app's own row, inserted with no stamp of its own: the + // before-insert hook stamps it `[]` (not a recorded change), so it is kept. + rowIds.note = String( + (await engine.insert( + ACTIVITY, + { + type: 'note', + summary: 'a note', + timestamp: new Date().toISOString(), + object_name: ITEM, + record_id: ids.item, + metadata: JSON.stringify({ channel: 'call' }), + // A writer's own value is not the stamp: the planner's is. + changed_fields: ['f_open'], + }, + { context: SYS }, + )).id, + ); }, 120_000); afterAll(async () => { @@ -195,7 +219,22 @@ describe('[#21388] an update whose every recorded change is withheld from the re // `internal` field from both sides, so this is not a redaction artefact. expect(meta(byId(rowIds.empty))).toMatchObject({ old: {}, new: {} }); expect(changeKeys(byId(rowIds.withheld))).toEqual(['f_admin', 'f_admin2']); - expect(atRest).toHaveLength(5); + expect(atRest).toHaveLength(6); + }); + + it('[ADR-0139 E6] at rest, every row carries the planner\'s stamp of its recorded change', async () => { + const atRest = await read(SYS); + const stampOf = (id: string) => { + const row = atRest.find((r) => r.id === id); + return { changed_fields: row?.changed_fields, milestone_key: row?.milestone_key ?? null }; + }; + expect(stampOf(rowIds.withheld)).toEqual({ changed_fields: ['f_admin', 'f_admin2'], milestone_key: null }); + expect(stampOf(rowIds.mixed)).toEqual({ changed_fields: ['f_admin', 'f_open'], milestone_key: null }); + expect(stampOf(rowIds.empty)).toEqual({ changed_fields: [], milestone_key: null }); + expect(stampOf(rowIds.created)).toEqual({ changed_fields: [], milestone_key: null }); + // The hook's answer, over the writer's own value. + expect(stampOf(rowIds.note)).toEqual({ changed_fields: [], milestone_key: null }); + for (const row of atRest) expect(row.changed_fields, String(row.id)).toEqual(planActivityStamp(row.metadata)?.changed_fields); }); // ── the card's three pins ─────────────────────────────────────────────── @@ -235,7 +274,7 @@ describe('[#21388] an update whose every recorded change is withheld from the re it('the member is served exactly the other rows', async () => { expect((await read(MEMBER)).map((r) => r.id)).toEqual( - [rowIds.created, rowIds.mixed, rowIds.empty, rowIds.served], + [rowIds.created, rowIds.mixed, rowIds.empty, rowIds.served, rowIds.note], ); }); @@ -255,7 +294,7 @@ describe('[#21388] an update whose every recorded change is withheld from the re context: MEMBER, }); const byType = Object.fromEntries(groups.map((g: Row) => [g.type, Number(g.n)])); - expect(byType).toEqual({ created: 1, updated: 3 }); + expect(byType).toEqual({ created: 1, updated: 3, note: 1 }); }); it('pages: walking the list one row at a time serves the same rows, and the withheld one on no page', async () => { @@ -270,10 +309,9 @@ describe('[#21388] an update whose every recorded change is withheld from the re expect(walked).toEqual((await read(MEMBER)).map((r) => String(r.id))); }); - it('[ADR-0139 E5] with no canReadObject on the service, the row is still withheld: no leaf is written for the rule to re-read as SYSTEM', async () => { - // The rule pre-scans this read's WHERE as SYSTEM. A ReadableIds leaf there - // would be admitted by `explain`, which refuses SYSTEM, so it would select - // nothing, and the rule would serve the rows it never saw. + it('[ADR-0139 E5, E6] with no canReadObject on the service, the row is still withheld', async () => { + // The read gate then answers by its bounded probe; the withheld-row rule + // reads no row at all, so it judges the same rows either way. const own = Object.getOwnPropertyDescriptor(security, 'canReadObject'); expect(own, 'the double carries canReadObject as its own member').toBeTruthy(); delete (security as { canReadObject?: unknown }).canReadObject; @@ -281,7 +319,9 @@ describe('[#21388] an update whose every recorded change is withheld from the re const rows = await read(MEMBER); expect(rows.map((r) => r.id)).not.toContain(rowIds.withheld); // Armed: the member is still served the rows it is served with the admission. - expect(rows.map((r) => r.id).sort()).toEqual([rowIds.created, rowIds.mixed, rowIds.empty, rowIds.served].sort()); + expect(rows.map((r) => r.id).sort()).toEqual( + [rowIds.created, rowIds.mixed, rowIds.empty, rowIds.served, rowIds.note].sort(), + ); } finally { Object.defineProperty(security, 'canReadObject', own!); } @@ -293,69 +333,116 @@ describe('[#21388] an update whose every recorded change is withheld from the re expect((await engine.findOne(ACTIVITY, { where: { id: rowIds.withheld }, context: ADMIN }))?.id).toBe(rowIds.withheld); }); + // A row at rest from before the column existed: the stamp cleared, as the + // platform (the migration door's own context). + it('[ADR-0139 E6] an unstamped row is withheld from the member and the admin, served to a system read, and served again once stamped', async () => { + const id = rowIds.served; + const restamp = (changed: unknown) => + engine.update(ACTIVITY, { id, changed_fields: changed }, { context: SYS }); + await restamp(null); + try { + expect((await read(MEMBER)).map((r) => r.id)).not.toContain(id); + expect((await read(ADMIN)).map((r) => r.id)).not.toContain(id); + expect(await engine.count(ACTIVITY, { where: scoped(ITEM, ids.item) }, { context: ADMIN })).toBe(5); + expect((await read(SYS)).map((r) => r.id)).toContain(id); + } finally { + await restamp(['f_open']); + } + expect((await read(MEMBER)).map((r) => r.id)).toContain(id); + }); + it('a system read is not narrowed', async () => { expect((await read(SYS)).map((r) => r.id)).toContain(rowIds.withheld); }); }); -describe('[#21388] isWithheldOnlyUpdate reads the stored change', () => { - const served = new Set(['open']); - - it('an update every key of which is withheld', () => { - expect(isWithheldOnlyUpdate({ old: { a: 1 }, new: { a: 2, b: 3 } }, served)).toBe(true); +describe('[ADR-0139 E6] planActivityStamp — the one derivation of the stamp', () => { + it('an update: the key union of both recorded sides, sorted', () => { + expect(planActivityStamp({ old: { b: 1, a: 1 }, new: { a: 2, c: 3 } })).toEqual({ + changed_fields: ['a', 'b', 'c'], + milestone_key: null, + }); }); - it('a key on either side that is served keeps the row', () => { - expect(isWithheldOnlyUpdate({ old: { a: 1, open: 1 }, new: { a: 2 } }, served)).toBe(false); - expect(isWithheldOnlyUpdate({ old: {}, new: { open: 1 } }, served)).toBe(false); + it('a change empty on both sides, a create, a delete and a non-change stamp an empty set', () => { + for (const metadata of [ + { old: {}, new: {} }, + { old: null, new: { a: 1 } }, + { old: { a: 1 }, new: null }, + { channel: 'email' }, + null, + '', + 'not json', + JSON.stringify({ old: { a: 1 }, new: null }), + ]) { + expect(planActivityStamp(metadata), JSON.stringify(metadata)).toEqual({ changed_fields: [], milestone_key: null }); + } }); - it('a change empty on both sides, a create, a delete and a non-change are not withheld', () => { - expect(isWithheldOnlyUpdate({ old: {}, new: {} }, served)).toBe(false); - expect(isWithheldOnlyUpdate({ old: null, new: { a: 1 } }, served)).toBe(false); - expect(isWithheldOnlyUpdate({ old: { a: 1 }, new: null }, served)).toBe(false); - expect(isWithheldOnlyUpdate({ channel: 'email' }, served)).toBe(false); - expect(isWithheldOnlyUpdate(null, served)).toBe(false); + it('reads the stored string exactly as the object a writer is about to serialize', () => { + const metadata = { old: { a: 1 }, new: { a: 2 } }; + expect(planActivityStamp(JSON.stringify(metadata))).toEqual(planActivityStamp(metadata)); }); }); -describe('[#21388] computeWithheldUpdateFilter — the pre-scan bound fails closed', () => { - const update = (i: number, key: string) => ({ - id: `r${i}`, - object_name: 'obj', - metadata: JSON.stringify({ old: { [key]: 1 }, new: { [key]: 2 } }), +describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predicate, no row read', () => { + const engineWith = (objects: Array<{ name: string; activityMilestones?: unknown }>) => ({ + registry: { getAllObjects: () => objects }, + getSchema: (name: string) => objects.find((o) => o.name === name), + find: vi.fn(), }); - const engineOf = (rows: Row[]) => ({ find: vi.fn(async () => rows) }); - const servedFor = async () => ['open']; - it('under the bound: the withheld ids are ANDed out, and nothing else', async () => { - const logger = { warn: vi.fn() }; - const rows = [update(1, 'secret'), update(2, 'open')]; - expect(await computeWithheldUpdateFilter(engineOf(rows), { where: { a: 1 } }, servedFor, logger)) - .toEqual({ id: { $nin: ['r1'] } }); - expect(logger.warn).not.toHaveBeenCalled(); + it('one $contains per served field, and the empty set kept', () => { + expect(keptRowsOfParent('obj', ['open', 'b_open'], [])).toEqual({ + $and: [ + { object_name: 'obj' }, + { changed_fields: { $null: false } }, + { + $or: [ + { changed_fields: { $empty: true } }, + { changed_fields: { $contains: 'b_open' } }, + { changed_fields: { $contains: 'open' } }, + ], + }, + { milestone_key: { $null: true } }, + ], + }); }); - it('nothing withheld: no filter at all', async () => { - const logger = { warn: vi.fn() }; - expect(await computeWithheldUpdateFilter(engineOf([update(1, 'open')]), {}, servedFor, logger)).toBeNull(); + it('a parent the service gives no answer for is kept whole; with no answer for any, no filter at all', async () => { + const engine = engineWith([{ name: 'a' }, { name: 'b' }]); + const servedFor = async (o: string) => (o === 'a' ? ['x'] : undefined); + expect(await computeWithheldRowFilter(engine, {}, servedFor, { warn: vi.fn() })).toEqual({ + $or: [keptRowsOfParent('a', ['x'], []), { object_name: 'b' }], + }); + expect(await computeWithheldRowFilter(engine, {}, async () => undefined, { warn: vi.fn() })).toBeNull(); + expect(engine.find).not.toHaveBeenCalled(); }); - it('at the bound: only the judged, kept rows are served, and that is said', async () => { - const logger = { warn: vi.fn() }; - const rows = Array.from({ length: PARENT_GATE_SCAN_LIMIT }, (_, i) => update(i, i === 0 ? 'secret' : 'open')); - const filter = (await computeWithheldUpdateFilter(engineOf(rows), {}, servedFor, logger)) as Row; - expect(filter.id.$in).toHaveLength(PARENT_GATE_SCAN_LIMIT - 1); - expect(filter.id.$in).not.toContain('r0'); - expect(logger.warn).toHaveBeenCalledTimes(1); + it('a read pinned to one parent object is judged against that parent alone', async () => { + const engine = engineWith([{ name: 'a' }, { name: 'b' }]); + const servedFor = vi.fn(async () => ['x']); + const where = { $and: [{ object_name: 'b' }, { record_id: '1' }] }; + expect(await computeWithheldRowFilter(engine, { where }, servedFor, { warn: vi.fn() })).toEqual( + keptRowsOfParent('b', ['x'], []), + ); + expect(servedFor).toHaveBeenCalledTimes(1); + }); + + it('the audit and activity objects themselves are not enumerated (the read gate excludes their rows)', async () => { + const engine = engineWith([{ name: 'sys_activity' }, { name: 'sys_audit_log' }, { name: 'a' }]); + expect(await computeWithheldRowFilter(engine, {}, async () => ['x'], { warn: vi.fn() })).toEqual( + keptRowsOfParent('a', ['x'], []), + ); }); - it('the pre-scan reads as the system, in the caller order, within the bound', async () => { - const engine = engineOf([]); - const orderBy = [{ field: 'timestamp', order: 'desc' }]; - await computeWithheldUpdateFilter(engine, { where: { a: 1 }, orderBy }, servedFor, { warn: vi.fn() }); - expect(engine.find).toHaveBeenCalledWith(ACTIVITY, expect.objectContaining({ - where: { a: 1 }, orderBy, limit: PARENT_GATE_SCAN_LIMIT, context: { isSystem: true }, - })); + it('an engine with no registry view: denied, and said once', async () => { + const logger = { warn: vi.fn() }; + const engine = { find: vi.fn() }; + const filter = await computeWithheldRowFilter(engine, {}, async () => ['x'], logger); + expect(filter).toEqual({ id: '__activity_withheld_row_denied__' }); + await computeWithheldRowFilter(engine, {}, async () => ['x'], logger); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(engine.find).not.toHaveBeenCalled(); }); }); From 106ee10684aeaee721b995118cf55a5b705b8d9b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 02:56:00 +0000 Subject: [PATCH 03/11] wip(plugin-audit): E6 backfill test, guard pins, schema-presence stamp Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- ...hanged-fields-backfill.integration.test.ts | 141 ++++++++++++++++++ .../src/activity-changed-fields.ts | 21 ++- .../src/activity-predicate-guard.test.ts | 2 + ...tivity-withheld-update.integration.test.ts | 33 ++++ .../plugins/plugin-audit/src/audit-writers.ts | 5 +- 5 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/plugin-audit/src/activity-changed-fields-backfill.integration.test.ts diff --git a/packages/plugins/plugin-audit/src/activity-changed-fields-backfill.integration.test.ts b/packages/plugins/plugin-audit/src/activity-changed-fields-backfill.integration.test.ts new file mode 100644 index 00000000000..a5a1c9665f1 --- /dev/null +++ b/packages/plugins/plugin-audit/src/activity-changed-fields-backfill.integration.test.ts @@ -0,0 +1,141 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 E6] The stored-row backfill of `sys_activity`'s two writer-stamped + * columns stamps the rows at rest exactly as the writer stamps new ones, writes + * nothing in a dry run, is idempotent, and leaves a row it cannot judge + * unstamped (withheld). + * + * ## Why a real engine and a real driver + * + * The backfill is a keyset walk over a NULL predicate on a multi-value column + * and a single-id update as the platform; whether that reads every unstamped + * row once and stores `[]` as an empty set (not as NULL) is a question about + * the Filter Protocol as a driver executes it. The rows are written by the + * real CRUD mirror, then their stamps cleared as the platform: the state a + * table is in after the upgrade, before the operator runs the door. + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; + +import { AuditPlugin } from './audit-plugin.js'; +import { backfillActivityChangedFields } from './activity-changed-fields-backfill.js'; + +const ACTIVITY = 'sys_activity'; +const ITEM = 'acb_item'; +const HARNESS_PACKAGE = 'com.objectstack.audit.test.activity-changed-fields-backfill'; +const SYS = { isSystem: true } as const; + +const itemObject = { + name: ITEM, + label: 'Backfill Item', + fields: { + name: { name: 'name', label: 'Name', type: 'text' as const }, + stage: { + name: 'stage', label: 'Stage', type: 'select' as const, + options: [{ label: 'Open', value: 'open' }, { label: 'Won', value: 'won' }], + }, + f_a: { name: 'f_a', label: 'A', type: 'text' as const }, + f_b: { name: 'f_b', label: 'B', type: 'text' as const }, + }, + activityMilestones: [{ field: 'stage', value: 'won', summary: 'Won {name}' }], +}; + +type Row = Record; +const stampOf = (r: Row) => ({ changed_fields: r.changed_fields ?? null, milestone_key: r.milestone_key ?? null }); + +describe('[ADR-0139 E6] backfillActivityChangedFields', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + /** The stamp the writer gave each row, read before it was cleared. */ + const written = new Map>(); + let unjudgedId = ''; + + const all = async () => + (await engine.find(ACTIVITY, { orderBy: [{ field: 'id', order: 'asc' }], context: SYS })) as Row[]; + const logger = () => ({ info: vi.fn(), warn: vi.fn() }); + + beforeAll(async () => { + kernel = new ObjectKernel({ logger: { level: 'silent' } }); + 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(itemObject as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + + const id = (await engine.insert(ITEM, { name: 'x', stage: 'open', f_a: '1', f_b: '1' }, { context: SYS })).id; + const update = (patch: Row) => engine.update(ITEM, patch, { where: { id }, context: SYS }); + await update({ f_a: '2' }); + await update({ f_a: '3', f_b: '2' }); + await update({ stage: 'won' }); + await engine.insert(ACTIVITY, { + type: 'note', summary: 'a note', object_name: ITEM, record_id: id, + timestamp: new Date().toISOString(), metadata: JSON.stringify({ channel: 'call' }), + }, { context: SYS }); + // A milestone row with no readable watched-field list: the planner cannot judge it. + unjudgedId = String((await engine.insert(ACTIVITY, { + type: 'note', summary: 'unjudged', object_name: ITEM, record_id: id, + timestamp: new Date().toISOString(), + metadata: JSON.stringify({ kind: 'milestone', text_sources: { summary: [], record_label: [] } }), + }, { context: SYS })).id); + + // The state after the upgrade: every row at rest, unstamped. + for (const row of await all()) { + if (row.id !== unjudgedId) written.set(String(row.id), stampOf(row)); + await engine.update(ACTIVITY, { id: row.id, changed_fields: null, milestone_key: null }, { context: SYS }); + } + }, 120_000); + + afterAll(async () => { + if (kernel) await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + }, 30_000); + + it('control: the writer stamped every class of row, and the table is now unstamped', async () => { + const stamps = [...written.values()]; + expect(stamps).toContainEqual({ changed_fields: [], milestone_key: null }); // the create, the note + expect(stamps).toContainEqual({ changed_fields: ['f_a'], milestone_key: null }); + expect(stamps).toContainEqual({ changed_fields: ['f_a', 'f_b'], milestone_key: null }); + expect(stamps).toContainEqual({ changed_fields: ['stage'], milestone_key: 'stage' }); + for (const row of await all()) expect(stampOf(row), String(row.id)).toEqual({ changed_fields: null, milestone_key: null }); + }); + + it('a dry run counts every unstamped row and writes none', async () => { + const report = await backfillActivityChangedFields(engine, logger(), { pageSize: 2 }); + expect(report).toEqual({ + apply: false, scanned: written.size + 1, stamped: written.size, milestones: 1, unjudged: 1, failures: 0, + }); + for (const row of await all()) expect(row.changed_fields ?? null, String(row.id)).toBeNull(); + }); + + it('apply stamps each row at rest exactly as the writer stamped it, a page at a time', async () => { + const report = await backfillActivityChangedFields(engine, logger(), { apply: true, pageSize: 2 }); + expect(report).toMatchObject({ apply: true, scanned: written.size + 1, stamped: written.size, unjudged: 1, failures: 0 }); + for (const row of await all()) { + if (row.id === unjudgedId) expect(stampOf(row)).toEqual({ changed_fields: null, milestone_key: null }); + else expect(stampOf(row), String(row.id)).toEqual(written.get(String(row.id))); + } + }); + + it('idempotent: a second run stamps nothing, and finds only the row it cannot judge', async () => { + const before = await all(); + const report = await backfillActivityChangedFields(engine, logger(), { apply: true, pageSize: 2 }); + expect(report).toEqual({ apply: true, scanned: 1, stamped: 0, milestones: 0, unjudged: 1, failures: 0 }); + expect(await all()).toEqual(before); + }); + + it('a read the table refuses is a failure, never a clean report', async () => { + const log = logger(); + const failing = { find: vi.fn(async () => { throw new Error('no such table'); }), update: vi.fn() }; + const report = await backfillActivityChangedFields(failing as any, log, { apply: true }); + expect(report.failures).toBe(1); + expect(failing.update).not.toHaveBeenCalled(); + expect(log.warn).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/plugins/plugin-audit/src/activity-changed-fields.ts b/packages/plugins/plugin-audit/src/activity-changed-fields.ts index a5471c1e87b..71b09908c23 100644 --- a/packages/plugins/plugin-audit/src/activity-changed-fields.ts +++ b/packages/plugins/plugin-audit/src/activity-changed-fields.ts @@ -150,9 +150,27 @@ export function planActivityStamp(metadata: unknown): ActivityStamp | null { return { changed_fields, milestone_key }; } -/** The engine slice the stamping hook registers on. */ +/** The engine slice the stamping hook registers on, and reads the registered `sys_activity` from. */ export interface ActivityStampHookEngine { registerHook(event: string, handler: (ctx: any) => void | Promise, options?: { object?: string; packageId?: string }): void; + getSchema?(object: string): unknown; +} + +/** + * Whether the REGISTERED `sys_activity` declares both stamped columns. Read on + * every call, never cached: a composition that registers its own earlier + * definition of the object (a test harness, a host override) has no column to + * stamp, and an insert naming one would be refused as an unknown field. + */ +export function declaresActivityStampColumns(engine: { getSchema?(object: string): unknown }): boolean { + let fields: unknown; + try { + const schema = typeof engine.getSchema === 'function' ? engine.getSchema('sys_activity') : undefined; + fields = isRecord(schema) ? schema.fields : undefined; + } catch { + return false; + } + return isRecord(fields) && ACTIVITY_STAMPED_COLUMNS.every((column) => column in fields); } /** Stamp one row in place: the planner's answer, over whatever the writer sent. */ @@ -175,6 +193,7 @@ export function installActivityStampHook(engine: ActivityStampHookEngine, packag engine.registerHook( 'beforeInsert', (ctx: any) => { + if (!declaresActivityStampColumns(engine)) return; const data = ctx?.input?.data; for (const row of Array.isArray(data) ? data : [data]) if (isRecord(row)) stampActivityRow(row); }, diff --git a/packages/plugins/plugin-audit/src/activity-predicate-guard.test.ts b/packages/plugins/plugin-audit/src/activity-predicate-guard.test.ts index c5c88ff29ba..73aad5945fc 100644 --- a/packages/plugins/plugin-audit/src/activity-predicate-guard.test.ts +++ b/packages/plugins/plugin-audit/src/activity-predicate-guard.test.ts @@ -366,6 +366,8 @@ describe('[#21154] the parent-field query guard: its column lists, its pin rule const row: Record = { id: 'a1', object_name: '', record_id: 'r1', type: 'note', url: 'u', actor_name: 'n', summary: 's', record_label: 'l', metadata: '{}', + // [ADR-0139 E6] The two writer-stamped columns are value-bearing too. + changed_fields: ['f'], milestone_key: 'f', }; const before = Object.keys(row); await redactActivityRows([row], { getReadableFields: async () => [], getQueryableFields: async () => [] }, {}); diff --git a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts index 60c1729ea2b..4c7aabd813b 100644 --- a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts @@ -351,6 +351,39 @@ describe('[#21388] an update whose every recorded change is withheld from the re expect((await read(MEMBER)).map((r) => r.id)).toContain(id); }); + // ── [ADR-0139 E6] the stamped columns: served narrowed, never probed ───── + + it('[ADR-0139 E6] the recorded change\'s key set is served narrowed to the fields the reader is served', async () => { + const memberRow = (await read(MEMBER)).find((r) => r.id === rowIds.mixed); + const adminRow = (await read(ADMIN)).find((r) => r.id === rowIds.mixed); + expect(memberRow?.changed_fields).toEqual(['f_open']); + expect(adminRow?.changed_fields).toEqual(['f_admin', 'f_open']); + }); + + it('[ADR-0139 E6] a membership probe on either stamped column is refused to a reader withheld a parent field, matching or not', async () => { + for (const where of [ + { changed_fields: { $contains: 'f_admin' } }, + { changed_fields: { $contains: 'f_none' } }, + { milestone_key: { $null: false } }, + ]) { + const pinned = { ...scoped(ITEM, ids.item), ...where }; + for (const run of [ + () => engine.find(ACTIVITY, { where: pinned, context: MEMBER }), + () => engine.count(ACTIVITY, { where: pinned }, { context: MEMBER }), + ]) { + await expect(run(), JSON.stringify(where)).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + } + } + }); + + it('[ADR-0139 E6] control: a reader served every field filters by the key set, as by any column', async () => { + const rows = (await engine.find(ACTIVITY, { + where: { ...scoped(ITEM, ids.item), changed_fields: { $contains: 'f_admin2' } }, + context: ADMIN, + })) as Row[]; + expect(rows.map((r) => r.id)).toEqual([rowIds.withheld]); + }); + it('a system read is not narrowed', async () => { expect((await read(SYS)).map((r) => r.id)).toContain(rowIds.withheld); }); diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index eee25bc132a..f556c52ac94 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -89,6 +89,7 @@ import type { LedgerRecordWriteAction } from './audit-log-field-redaction.js'; // withheld-row rule reads, shared with the before-insert hook and the backfill. import { declaredMilestoneWatchedFields, + declaresActivityStampColumns, installActivityStampHook, planActivityStamp, } from './activity-changed-fields.js'; @@ -2128,7 +2129,9 @@ export function installAuditWriters( // [ADR-0139 E6] The two writer-stamped columns the withheld-row rule is // a predicate over, from the metadata this row records (the // before-insert hook re-derives them by the same planner). - ...(planActivityStamp(activityMetadata) ?? { changed_fields: null, milestone_key: null }), + ...(declaresActivityStampColumns(engine as any) + ? (planActivityStamp(activityMetadata) ?? { changed_fields: null, milestone_key: null }) + : {}), }; // Same rationale as auditRow: stamp the tenant column so RLS matches the // recipient's organization on read — but only when the (auto-injected) From 534d0afee729fd1f0dc74b038550c800091a15d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 02:57:29 +0000 Subject: [PATCH 04/11] wip(cli): os migrate activity-changed-fields Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- ...ctivity-changed-fields.integration.test.ts | 264 ++++++++++++++++++ .../migrate/activity-changed-fields.ts | 238 ++++++++++++++++ 2 files changed, 502 insertions(+) create mode 100644 packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts create mode 100644 packages/cli/src/commands/migrate/activity-changed-fields.ts diff --git a/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts b/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts new file mode 100644 index 00000000000..72d627dc00c --- /dev/null +++ b/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts @@ -0,0 +1,264 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import Database from 'better-sqlite3'; +import { isExitSignal } from '../../utils/format.js'; +import MigrateApply from './apply.js'; +import MigrateActivityChangedFields from './activity-changed-fields.js'; + +// Pay the first transform of the dist-resolved workspace deps the commands +// reach through dynamic `import()`s at MODULE LOAD, not inside a case. +import '@objectstack/runtime'; +import '@objectstack/objectql'; +import '@objectstack/plugin-auth'; +import '@objectstack/plugin-security'; +import '@objectstack/plugin-audit'; + +/** + * [ADR-0139 E6] The upgrade stamps the `sys_activity` rows written before the + * two read-rule columns existed: `os migrate activity-changed-fields`. + * + * ## The database + * + * The schema this version creates (`os migrate apply --yes` over an empty + * file, the composition `os serve` mounts with an auth secret, so plugin-audit's + * objects are among it), then turned back into the shape a deployment upgraded + * from the previous version holds: the two columns DROPPED from the activity + * shard (`sys_activity` is rotation-managed: its base name is a view over a + * `sys_activity__r` shard table), and activity rows planted there the way + * the previous writer left them — no stamp. A real SQLite file under the SQL + * driver the commands boot, because the raw column read is the point. + * + * ## The cases run in order over ONE database, as an operator runs the steps + * + * 1. a dry run on the database no upgraded process has opened: the columns + * are not there yet, so it reads nothing it could judge and says so; + * 2. `--apply` boots plain, so the schema sync adds both columns to the + * shard, and stamps every row the planner can judge, as the writer would; + * 3. a re-run stamps nothing (idempotent) and still names the row it cannot + * judge, which stays unstamped. + */ + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CLI_ROOT = resolve(HERE, '..', '..', '..'); +const CASE_TIMEOUT_MS = 240_000; +const SECRET = 'os23731-integration-secret-at-least-32-chars'; + +/** The rows the previous writer left, and the stamp each must get. */ +const PLANTED: Array<{ id: string; metadata: unknown; stamp: { changed_fields: string[] | null; milestone_key: string | null } }> = [ + { id: 'act_u1', metadata: { old: { b: 1, a: 1 }, new: { a: 2 } }, stamp: { changed_fields: ['a', 'b'], milestone_key: null } }, + { id: 'act_c1', metadata: { old: null, new: { a: 1 } }, stamp: { changed_fields: [], milestone_key: null } }, + { id: 'act_n1', metadata: { channel: 'call' }, stamp: { changed_fields: [], milestone_key: null } }, + { + id: 'act_m1', + metadata: { old: { stage: 'a' }, new: { stage: 'b' }, kind: 'milestone', text_sources: { summary: [], record_label: [], kind: ['stage', 'phase'] } }, + stamp: { changed_fields: ['stage'], milestone_key: 'phase,stage' }, + }, + { + id: 'act_x1', + metadata: { kind: 'milestone', text_sources: { summary: [], record_label: [] } }, + stamp: { changed_fields: null, milestone_key: null }, + }, +]; + +type Invoke = (argv: string[]) => Promise; +const invoke = (command: { run: (argv: string[], opts: { root: string }) => Promise }): Invoke => + (argv) => command.run(argv, { root: CLI_ROOT }); + +/** Env that would point a command's boot somewhere other than the fixture. */ +const OVERRIDING_ENV = [ + 'OS_DATABASE_URL', 'DATABASE_URL', 'TURSO_DATABASE_URL', 'OS_DATABASE_DRIVER', 'OS_HOME', + 'OS_TELEMETRY_DB', 'OS_AUTH_SECRET', 'AUTH_SECRET', 'BETTER_AUTH_SECRET', 'OS_ARTIFACT_PATH', +] as const; + +/** Run one command and capture its JSON document and exit status. */ +async function runJson(command: Invoke, argv: string[]): Promise<{ payload: any; exitCode: number }> { + const savedExit = process.exitCode; + const swallow = ((_chunk: unknown, ...rest: unknown[]) => { + const cb = rest.find((a) => typeof a === 'function') as (() => void) | undefined; + if (cb) cb(); + return true; + }) as typeof process.stdout.write; + const stdout = vi.spyOn(process.stdout, 'write').mockImplementation(swallow); + vi.spyOn(process.stderr, 'write').mockImplementation(swallow); + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + let processExit: number | undefined; + vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + processExit = code ?? 0; + throw new Error(`__PROCESS_EXIT__:${processExit}`); + }) as never); + let thrownExit: number | undefined; + try { + try { + await command(argv); + } catch (error) { + const trapped = error instanceof Error && error.message.startsWith('__PROCESS_EXIT__'); + if (!trapped && !isExitSignal(error)) throw error; + if (!trapped) thrownExit = (error as { oclif?: { exit?: number } }).oclif?.exit; + } + const out = stdout.mock.calls.map((c) => String(c[0])).join('').trim(); + let payload: any; + try { + payload = JSON.parse(out); + } catch { + payload = { unparsed: out.slice(-400) }; + } + return { payload, exitCode: processExit ?? thrownExit ?? (process.exitCode as number | undefined) ?? 0 }; + } finally { + process.exitCode = savedExit; + vi.restoreAllMocks(); + } +} + +let dir: string; +let dbFile: string; +const savedEnv: Record = {}; +const savedCwd = process.cwd(); + +function withDb(fn: (db: Database.Database) => T, readonly = true): T { + const db = new Database(dbFile, { readonly }); + try { + return fn(db); + } finally { + db.close(); + } +} + +/** Insert a row, filling every other NOT NULL column without a default with a placeholder of its type. */ +function plant(db: Database.Database, table: string, values: Record): void { + const columns = db.prepare(`PRAGMA table_info('${table}')`).all() as Array< + { name: string; type: string; notnull: number; dflt_value: unknown; pk: number } + >; + const row: Record = { ...values }; + for (const c of columns) { + if (c.name in row || !c.notnull || c.dflt_value !== null) continue; + row[c.name] = /INT|BOOL|REAL|NUM/i.test(c.type) ? 0 : `${c.name}-placeholder`; + } + const names = Object.keys(row); + db.prepare(`INSERT INTO "${table}" (${names.map((n) => `"${n}"`).join(', ')}) VALUES (${names.map(() => '?').join(', ')})`) + .run(...names.map((n) => row[n] as never)); +} + +let shard = ''; + +const columnsOf = (table: string): string[] => + withDb((db) => (db.prepare(`PRAGMA table_info('${table}')`).all() as Array<{ name: string }>).map((c) => c.name)); + +const stamps = (): Record => + withDb((db) => { + const out: Record = {}; + const rows = db.prepare(`SELECT id, changed_fields, milestone_key FROM "${shard}" WHERE id LIKE 'act_%' ORDER BY id`).all() as + Array<{ id: string; changed_fields: string | null; milestone_key: string | null }>; + for (const r of rows) { + out[r.id] = { + changed_fields: r.changed_fields === null ? null : JSON.parse(r.changed_fields), + milestone_key: r.milestone_key, + }; + } + return out; + }); + +beforeAll(async () => { + for (const key of [...OVERRIDING_ENV, 'NODE_ENV'] as const) savedEnv[key] = process.env[key]; + for (const key of OVERRIDING_ENV) delete process.env[key]; + dir = mkdtempSync(join(tmpdir(), 'os-23731-activity-')); + mkdirSync(join(dir, 'dist'), { recursive: true }); + writeFileSync( + join(dir, 'objectstack.config.ts'), + [ + 'export default {', + " manifest: { id: 'com.example.os23731activity', name: 'requires auth', version: '0.0.0', type: 'app' },", + " requires: ['auth'],", + " objects: [{ name: 'os23731_thing', fields: { title: { type: 'text' } } }],", + '};', + '', + ].join('\n'), + ); + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); + // `os serve` composes the auth family, and with it plugin-audit, only when an + // auth secret resolves; the deployment this pins serves with one. + process.env.OS_AUTH_SECRET = SECRET; + process.env.NODE_ENV = 'production'; + process.env.OS_TELEMETRY_DB = '0'; + dbFile = join(dir, 'upgraded.db'); + process.chdir(dir); + try { + const created = await runJson(invoke(MigrateApply), ['--database-url', `file:${dbFile}`, '--yes', '--json']); + expect(created.exitCode, JSON.stringify(created.payload).slice(0, 400)).toBe(0); + } finally { + process.chdir(savedCwd); + } + + // The previous version's shape: no stamp column, and its rows. + withDb((db) => { + shard = (db.prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name GLOB 'sys_activity__r*'").get() as + { name: string } | undefined)?.name ?? ''; + if (!shard) throw new Error('the fixture has no sys_activity shard: the rotation did not run'); + db.exec(`ALTER TABLE "${shard}" DROP COLUMN changed_fields`); + db.exec(`ALTER TABLE "${shard}" DROP COLUMN milestone_key`); + for (const row of PLANTED) { + plant(db, shard, { + id: row.id, timestamp: new Date().toISOString(), type: 'updated', summary: row.id, + object_name: 'os23731_thing', record_id: 't1', metadata: JSON.stringify(row.metadata), + }); + } + }, false); + expect(columnsOf(shard), 'fixture: the previous version has no stamp column').not.toContain('changed_fields'); +}, CASE_TIMEOUT_MS); + +afterAll(() => { + process.chdir(savedCwd); + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + try { rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +beforeEach(() => { process.chdir(dir); }); +afterEach(() => { process.chdir(savedCwd); }); + +describe('os migrate activity-changed-fields stamps the activity rows the previous version left (ADR-0139 E6)', () => { + const db = () => `file:${dbFile}`; + + it('1. a dry run on the not-yet-synced database writes nothing, adds no column, and does not report a clean table', async () => { + const { payload, exitCode } = await runJson(invoke(MigrateActivityChangedFields), ['--database-url', db(), '--json']); + process.stderr.write(`[diag 1] exit=${exitCode} ${JSON.stringify(payload).slice(0, 600)}\n`); + expect(columnsOf(shard)).not.toContain('changed_fields'); + expect(exitCode).toBe(1); + expect(payload).toMatchObject({ apply: false, report: { stamped: 0, failures: 1 } }); + }, CASE_TIMEOUT_MS); + + it('2. --apply adds both columns to the shard and stamps every row the planner can judge, as the writer would', async () => { + const { payload, exitCode } = await runJson( + invoke(MigrateActivityChangedFields), ['--apply', '--yes', '--database-url', db(), '--json'], + ); + process.stderr.write(`[diag 2] exit=${exitCode} ${JSON.stringify(payload).slice(0, 600)}\n`); + expect(exitCode).toBe(0); + expect(columnsOf(shard)).toEqual(expect.arrayContaining(['changed_fields', 'milestone_key'])); + expect(payload.report).toMatchObject({ apply: true, stamped: PLANTED.length - 1, milestones: 1, unjudged: 1, failures: 0 }); + expect(stamps()).toEqual(Object.fromEntries(PLANTED.map((r) => [r.id, r.stamp]))); + }, CASE_TIMEOUT_MS); + + it('3. a re-run stamps nothing and still names the row it cannot judge', async () => { + const before = stamps(); + const { payload, exitCode } = await runJson( + invoke(MigrateActivityChangedFields), ['--apply', '--yes', '--database-url', db(), '--json'], + ); + expect(exitCode).toBe(0); + expect(payload.report).toEqual({ apply: true, scanned: 1, stamped: 0, milestones: 0, unjudged: 1, failures: 0 }); + expect(stamps()).toEqual(before); + }, CASE_TIMEOUT_MS); + + it('4. a dry run after the apply reports the same clean table', async () => { + const { payload, exitCode } = await runJson(invoke(MigrateActivityChangedFields), ['--database-url', db(), '--json']); + expect(exitCode).toBe(0); + expect(payload.report).toEqual({ apply: false, scanned: 1, stamped: 0, milestones: 0, unjudged: 1, failures: 0 }); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/migrate/activity-changed-fields.ts b/packages/cli/src/commands/migrate/activity-changed-fields.ts new file mode 100644 index 00000000000..775d38cb807 --- /dev/null +++ b/packages/cli/src/commands/migrate/activity-changed-fields.ts @@ -0,0 +1,238 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { Command, Flags } from '@oclif/core'; +import chalk from 'chalk'; +import { createInterface } from 'node:readline'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; +import { + printHeader, + printSuccess, + printWarning, + printError, + printInfo, + printStep, + createTimer, + emitJson, + isExitSignal, + errorCodeFields, +} from '../../utils/format.js'; +import { bootSchemaStack } from '../../utils/schema-migrate.js'; +import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; +import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; +import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; +import { absentTableReads } from '../../utils/absent-table-reads.js'; + +async function confirm(question: string): Promise { + if (!process.stdin.isTTY) return false; // non-interactive → require --yes + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer: string = await new Promise((resolve) => rl.question(question, resolve)); + return /^y(es)?$/i.test(answer.trim()); + } finally { + rl.close(); + } +} + +/** + * `os migrate activity-changed-fields` — the stored-row backfill of + * `sys_activity`'s two writer-stamped read-rule columns (ADR-0139 D4, E6). + * + * The activity stream's withheld-row rule is a predicate over `changed_fields` + * (the key set of a row's recorded change) and `milestone_key` (a fired + * milestone's watched-field key). New rows are stamped at write time; rows + * written before the upgrade carry no stamp, and an unstamped row is withheld + * from every non-system reader. So after the upgrade, and until this runs, an + * activity feed shows only the rows written since. Operators run it at once + * after upgrading, dry run first. + * + * Both columns are stamped in one run, by the planner the writer stamps new + * rows with (`@objectstack/plugin-audit`'s `backfillActivityChangedFields`). A + * row the planner cannot judge stays unstamped (withheld) and is counted. + * + * Dry run by default (writes no row), `--apply` to stamp. Idempotent: only + * unstamped rows are read, so a second run stamps nothing — re-running and + * reading a clean report is the verification. No `sys_migration` flag is + * recorded: nothing gates irreversible behaviour on it. The flags are + * `os migrate audit-metadata-bodies`'s, one for one. + */ +export default class MigrateActivityChangedFields extends Command { + static override description = + 'Stamp the two read-rule columns (changed_fields, milestone_key) on the sys_activity rows written before ' + + 'they existed: until it runs, those rows are withheld from every non-system reader. Dry run by default; ' + + '--apply writes. Run it at once after upgrading.'; + + static override examples = [ + '$ os migrate activity-changed-fields', + '$ os migrate activity-changed-fields --apply', + '$ os migrate activity-changed-fields --apply --yes --json', + ]; + + static override flags = { + 'database-url': Flags.string({ + description: 'Database URL to migrate (defaults to $OS_DATABASE_URL / the project DB)', + env: 'OS_DATABASE_URL', + }), + apply: Flags.boolean({ + description: 'Stamp the unstamped rows (default is a read-only dry run)', + default: false, + }), + yes: Flags.boolean({ char: 'y', description: 'Skip the --apply confirmation prompt', default: false }), + force: Flags.boolean({ + description: 'Apply even when another process is using the database (SQLite occupancy check)', + default: false, + }), + json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to apply)' }), + }; + + async run(): Promise { + const { flags } = await this.parse(MigrateActivityChangedFields); + const timer = createTimer(); + const apply = flags.apply; + + if (!flags.json) printHeader('Migrate · activity-changed-fields'); + + const occupancy = await probeMigrationTarget(flags['database-url']); + if (occupancy.status === 'busy' && apply && !flags.force) { + if (flags.json) { + await emitJson( + { error: 'database_busy', database: occupancy.filename, signal: occupancy.signal, detail: occupancy.detail, hint: OCCUPANCY_HINT }, + 0, + { compact: true }, + ); + this.exit(1); + return; + } + printError(describeOccupancy(occupancy)); + printWarning(OCCUPANCY_HINT); + this.exit(1); + return; + } + if (occupancy.status === 'busy' && !flags.json) { + printWarning(apply + ? `--force: ${describeOccupancy(occupancy)} Stamping anyway — the live process may write rows mid-walk.` + : `${describeOccupancy(occupancy)} The dry run below writes no row or table, but its counts may shift while that process is running. ${JOURNAL_MODE_SWITCH_NOTE}`); + } + if (occupancy.status === 'unknown' && !flags.json) { + printWarning(`Could not check whether the database is in use — ${occupancy.detail}`); + } + + if (apply && !flags.yes) { + if (flags.json || !process.stdin.isTTY) { + if (flags.json) { + await emitJson({ error: 'confirmation_required', hint: 'pass --yes' }, 0, { compact: true }); + this.exit(1); + return; + } + printWarning('Apply mode stamps sys_activity rows. Re-run with --yes to confirm, or run without --apply to preview.'); + this.exit(1); + return; + } + const ok = await confirm( + chalk.bold('\nStamp every unstamped sys_activity row on this database? [y/N] '), + ); + if (!ok) { + printInfo('Aborted — no changes made.'); + return; + } + } + + if (!flags.json) { + printStep(apply ? 'Booting data stack (APPLY mode)…' : 'Booting data stack (dry run)…'); + } + + let stack; + try { + // [#21349] The dry run boots READ-ONLY — the same boot `os migrate plan` + // takes: `deferSchemaDdl` holds schema DDL back and suppresses the + // artifact's inline seed loader (whose upserts rewrite every seeded row + // of the app's tables), and `readOnlyProbe` keeps a missing sqlite file + // from being created. `--apply` keeps the plain boot, unchanged. + stack = await bootSchemaStack({ + jsonOutput: flags.json, + databaseUrl: flags['database-url'], + extraPlugins: await buildDataMigrationPlugins({ audit: true }), + ...(apply ? {} : { deferSchemaDdl: true, readOnlyProbe: true }), + }); + } catch (error: any) { + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + return; + } + + try { + // `SchemaStack.kernel` is untyped, so the slot's contract is stated on the + // RESULT — the `objectql` slot serves `IObjectQLEngine`, whose `find` / + // `update` are exactly what the backfill calls. + const engine = stack.kernel.getService('objectql') as IObjectQLEngine | undefined; + if (typeof engine?.find !== 'function' || typeof engine?.update !== 'function') { + throw new Error('No ObjectQL engine on this stack — cannot stamp activity rows.'); + } + + const { backfillActivityChangedFields } = await import('@objectstack/plugin-audit'); + + const logger = flags.json + ? { info: (m: string) => console.error(m), warn: (m: string) => console.error(m) } + : { info: (m: string) => printInfo(m), warn: (m: string) => printWarning(m) }; + + // [#21552] The dry run's read-only boot lists `sys_activity` — rotation- + // managed, its base name a view over time-sharded tables — as a table to + // create, so the boot's measurement is not believed: the table is read, + // and only the refusal of a table that is not there reads as no rows + // (`absentTableReads`). ⛔ Any other refused read is still counted in + // `failures` and still exits non-zero. `--apply` booted plain; its writes + // go to the engine itself. + const reads = absentTableReads(stack, (object) => engine.getObject(object)); + const readView: Pick = { + find: (object, query, options) => reads.rows(object, () => engine.find(object, query, options)), + update: (object, data, options) => engine.update(object, data, options), + }; + + const report = await backfillActivityChangedFields(readView, logger, { apply }); + + if (flags.json) { + reads.notice(true); + await emitJson({ database: stack.dbLabel, apply, report, duration: timer.elapsed() }); + if (report.failures > 0) this.exit(1); + return; + } + + printInfo(`Database: ${chalk.white(stack.dbLabel)}`); + reads.notice(false); + console.log(''); + console.log( + ` ${chalk.white('sys_activity')}: ${report.scanned} unstamped, ${report.stamped} ${apply ? 'stamped' : 'to stamp'}` + + ` (${report.milestones} milestone), ${report.unjudged} cannot be judged`, + ); + console.log(''); + + if (report.failures > 0) { + printError(`${report.failures} read(s) or row(s) failed — re-run to finish them.`); + } else if (apply && report.stamped > 0) { + printSuccess( + `Stamped ${report.stamped} activity row(s). Re-run any time — it only revisits rows still unstamped.`, + ); + } else if (!apply && report.stamped > 0) { + printInfo(`Dry run only — ${report.stamped} row(s) would be stamped. Re-run with --apply.`); + } else { + printSuccess('Nothing to stamp — every sys_activity row the planner can judge is stamped.'); + } + if (report.unjudged > 0) { + printWarning( + `${report.unjudged} row(s) cannot be judged (a milestone row naming no readable watched-field list): ` + + 'they stay unstamped and are withheld from every non-system reader until retention ages them out.', + ); + } + console.log(chalk.dim(` ${timer.display()}`)); + console.log(''); + if (report.failures > 0) this.exit(1); + } catch (error: any) { + if (isExitSignal(error)) throw error; + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + } finally { + await stack.shutdown(); + } + } +} From 6becb55fd38451afdfa4dba2d59765d2307457a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:00:59 +0000 Subject: [PATCH 05/11] wip(cli): register the door in the one-shot, json-purity and absent-database families Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- ...ctivity-changed-fields.integration.test.ts | 10 ++++++++-- ...mmands.absent-database.integration.test.ts | 19 +++++++++++++++++++ ...igrate.one-shot-family.integration.test.ts | 6 ++++++ .../cli/test/json-stdout-purity.e2e.test.ts | 5 +++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts b/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts index 72d627dc00c..bdd6ae52141 100644 --- a/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts +++ b/packages/cli/src/commands/migrate/activity-changed-fields.integration.test.ts @@ -200,8 +200,16 @@ beforeAll(async () => { shard = (db.prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name GLOB 'sys_activity__r*'").get() as { name: string } | undefined)?.name ?? ''; if (!shard) throw new Error('the fixture has no sys_activity shard: the rotation did not run'); + // The base name is a view naming every column, as the rotation writes it: + // the previous version's view names neither stamp column. + const view = (db.prepare("SELECT sql FROM sqlite_master WHERE type = 'view' AND name = 'sys_activity'").get() as + { sql: string } | undefined)?.sql; + if (!view) throw new Error('the fixture has no sys_activity view'); + db.exec('DROP VIEW sys_activity'); db.exec(`ALTER TABLE "${shard}" DROP COLUMN changed_fields`); db.exec(`ALTER TABLE "${shard}" DROP COLUMN milestone_key`); + const previous = view.replace(/,\s*"?changed_fields"?/g, '').replace(/,\s*"?milestone_key"?/g, ''); + db.exec(previous); for (const row of PLANTED) { plant(db, shard, { id: row.id, timestamp: new Date().toISOString(), type: 'updated', summary: row.id, @@ -229,7 +237,6 @@ describe('os migrate activity-changed-fields stamps the activity rows the previo it('1. a dry run on the not-yet-synced database writes nothing, adds no column, and does not report a clean table', async () => { const { payload, exitCode } = await runJson(invoke(MigrateActivityChangedFields), ['--database-url', db(), '--json']); - process.stderr.write(`[diag 1] exit=${exitCode} ${JSON.stringify(payload).slice(0, 600)}\n`); expect(columnsOf(shard)).not.toContain('changed_fields'); expect(exitCode).toBe(1); expect(payload).toMatchObject({ apply: false, report: { stamped: 0, failures: 1 } }); @@ -239,7 +246,6 @@ describe('os migrate activity-changed-fields stamps the activity rows the previo const { payload, exitCode } = await runJson( invoke(MigrateActivityChangedFields), ['--apply', '--yes', '--database-url', db(), '--json'], ); - process.stderr.write(`[diag 2] exit=${exitCode} ${JSON.stringify(payload).slice(0, 600)}\n`); expect(exitCode).toBe(0); expect(columnsOf(shard)).toEqual(expect.arrayContaining(['changed_fields', 'milestone_key'])); expect(payload.report).toMatchObject({ apply: true, stamped: PLANTED.length - 1, milestones: 1, unjudged: 1, failures: 0 }); diff --git a/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts b/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts index 0c8461638f7..ecfd1864809 100644 --- a/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts +++ b/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts @@ -59,6 +59,11 @@ * (the control: the door READS the table and reports the row), and the four * doors that already exited 0 on the absent database, which still do. * + * [ADR-0139 E6] `os migrate activity-changed-fields` joined it born with the + * answer too: it reads `sys_activity` alone, through `absentTableReads`. Its + * control row is the seeded activity row, planted below the writer, so it + * carries no stamp. + * * [#21573] `os migrate unmapped-columns` joined the roster later, born with * the same answer: it asks `tableAbsent` before the differ. Its control row is * a retired field's column on `os21529_contact`, holding a value. @@ -482,6 +487,20 @@ const DOORS: readonly Door[] = [ expect(doc.report.byObject.sys_activity.rewritten).toBe(1); }, }, + { + name: 'migrate activity-changed-fields', + argv: ['migrate', 'activity-changed-fields'], + empty: (doc) => expect(doc).toMatchObject({ apply: false, report: { scanned: 0, stamped: 0, failures: 0 } }), + humanEmpty: /Nothing to stamp/, + tables: ['sys_activity'], + work: (doc) => { + // The seeded activity row was planted below the writer, so it carries no + // stamp: the one row to stamp. The writer's own rows are stamped already. + // Rotation-managed, so READ, though the boot listed its base name as a + // table to create. + expect(doc.report).toMatchObject({ scanned: 1, stamped: 1, unjudged: 0, failures: 0 }); + }, + }, { name: 'migrate meta --stored', argv: ['migrate', 'meta', '--stored'], diff --git a/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts b/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts index 375e54af38a..68a6c42324f 100644 --- a/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts @@ -55,6 +55,7 @@ import { isExitSignal } from './format.js'; import { bootSchemaStack } from './schema-migrate.js'; import { buildDataMigrationPlugins } from './data-migration-plugins.js'; import MigrateAccountIssuer from '../commands/migrate/account-issuer.js'; +import MigrateActivityChangedFields from '../commands/migrate/activity-changed-fields.js'; import MigrateApply from '../commands/migrate/apply.js'; import MigrateAuditMetadataBodies from '../commands/migrate/audit-metadata-bodies.js'; import MigrateCommentReactions from '../commands/migrate/comment-reactions.js'; @@ -158,6 +159,11 @@ const CALLERS: Record = { write: [], note: 'a pre-flight report: it has no writing mode', }, + 'commands/migrate/activity-changed-fields.ts': { + run: invoke(MigrateActivityChangedFields), + noWrite: [{ label: 'migrate activity-changed-fields', argv: ['--database-url', '@DB@', '--json'] }], + write: [{ label: 'migrate activity-changed-fields --apply', argv: ['--apply', '--yes', '--database-url', '@DB@', '--json'] }], + }, 'commands/migrate/apply.ts': { run: invoke(MigrateApply), noWrite: [], diff --git a/packages/cli/test/json-stdout-purity.e2e.test.ts b/packages/cli/test/json-stdout-purity.e2e.test.ts index 876b149988f..0c1b9851f03 100644 --- a/packages/cli/test/json-stdout-purity.e2e.test.ts +++ b/packages/cli/test/json-stdout-purity.e2e.test.ts @@ -89,6 +89,11 @@ const FAMILY: Record = { // emit paths and therefore the one worth pinning: every driver and kernel // diagnostic the failed scan produces is emitted before it. 'migrate account-issuer': [], + // A dry run is its DEFAULT and the only form driven here: without `--apply` + // it boots read-only (plugin-audit's objects registered), reads + // `sys_activity` and writes nothing. Its backfill logs through a sink of its + // own, which `--json` points at stderr, as `audit-metadata-bodies` does. + 'migrate activity-changed-fields': [], 'migrate apply': [], // A dry run is its DEFAULT and the only form driven here: without `--apply` // it boots (plugin-audit's objects registered), reads `sys_audit_log` / From efa1e090bc77ae9b76e92eca5a5f6b69b1d4a484 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:06:35 +0000 Subject: [PATCH 06/11] wip: E6 dogfood pins, cli docs, checklist Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- content/docs/deployment/cli.mdx | 31 +++ docs/qa/platform-checklist/areas/cli.json | 5 +- ...ity-withheld-row-predicate.dogfood.test.ts | 258 ++++++++++++++++++ 3 files changed, 292 insertions(+), 2 deletions(-) create mode 100644 packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index a85386649b4..e2ecf83be82 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1172,6 +1172,7 @@ where the data lives. | `os migrate value-shapes` | Scan stored reference and structured-JSON field values against the platform's value contract, and record the deployment's migration flag when clean | | `os migrate summary-nulls` | Backfill roll-up `count` / `sum` columns still stored as `NULL` on parent rows created before the insert-time seed. Repairs values; no flag, nothing depends on it having run | | `os migrate comment-reactions` | Carry the emoji reactions a 17.x deployment stored in the retired `sys_comment.reactions` column into `sys_comment_reaction` records, list every stored value that does not read, and record the verified run that lets `os migrate apply --allow-destructive` drop the column | +| `os migrate activity-changed-fields` | Stamp the two read-rule columns (`changed_fields`, `milestone_key`) on the `sys_activity` rows written before they existed. Until it runs, those rows are withheld from every reader but the system, so run it at once after upgrading. No flag; idempotent | | `os migrate meta --stored` | Replay the metadata conversion chain over this deployment's `sys_metadata` rows and rewrite the ones still carrying a pre-protocol shape. Hygiene, not a gate — nothing depends on it having run | | `os migrate security-catalog-overlays` | List the environment-wide permission-set and position rows (the legacy plurals `permissions` / `positions` included) stored over a name a configured package holds — the rows a v18 cold boot refuses — each with the package that holds its name; `--apply` deletes them, one history tombstone per row. Run it before the first v18 boot, with the flags and environment the deployment boots with: the platform security plugin's sets are held only where `os serve` composes it (an auth secret set, and the `auth` tier on), and the step takes `serve`'s `--preset` and `--dev` with `serve`'s meaning. Adopts nothing; exits 1 while it lists a row | | `os migrate duplicates` | Report business identifiers already minted twice across the organization partitions, and the rows blocking the boot-time NULL-safe index tightenings — a read-only inventory as JSON on stdout. Renumbers nothing and writes no row and no schema; run it before the boot-time tenancy repair, which overwrites part of the evidence | @@ -1411,6 +1412,36 @@ A run that lists an unreadable value or fails a write exits `1`, and so does an A database created on v18 never had the column: the run finds nothing, and there is no drop to wait for. +#### `os migrate activity-changed-fields` + +An activity row is served to a reader only when the platform can judge it for +that reader: whether the change it records touched a field the reader may see, +and, for a fired milestone, whether the reader may see every field the +milestone watches. Since this version that judgement reads two columns the +platform stamps on every new row, `changed_fields` and `milestone_key`. Rows +written before the upgrade carry no stamp, and **an unstamped row is withheld +from every reader but the system**, so until this command runs, activity feeds +and record timelines show only the rows written since the upgrade. + +```bash +os migrate activity-changed-fields # Dry run: counts, writes no rows +os migrate activity-changed-fields --apply # Stamp the rows (prompts) +os migrate activity-changed-fields --apply --yes --json # CI / scripts +``` + +Each row is stamped from its own recorded change, by the rule the platform +stamps new rows with, and both columns are stamped in one run. **A row that +cannot be judged stays unstamped**, and stays withheld: a milestone row that +names no readable list of watched fields. The report counts those rows; they +age out with the object's retention (14 days unless the deployment lengthens +it). + +**Idempotent**: only unstamped rows are read, so a second run stamps nothing, +and re-running is how an interrupted run is finished. The columns arrive with +the upgraded deployment's schema sync, which an `--apply` run performs itself: +a dry run against a database no upgraded process has opened cannot read them, +reports the failed read and exits `1`. A failed read or write exits `1`. + #### A database created by this version needs no migration A deployment whose database the platform **creates from empty** records these diff --git a/docs/qa/platform-checklist/areas/cli.json b/docs/qa/platform-checklist/areas/cli.json index 52a15d112fc..8970969813f 100644 --- a/docs/qa/platform-checklist/areas/cli.json +++ b/docs/qa/platform-checklist/areas/cli.json @@ -288,14 +288,15 @@ "unmapped-columns", "organization-ownership", "security-catalog-overlays", - "comment-reactions" + "comment-reactions", + "activity-changed-fields" ], "traps": ["stale-dist"], "automated": { "kind": "unit", "ref": "packages/cli/src/utils/format.exit-code.test.ts" }, "source": [ "packages/cli/src/commands/migrate/index.ts + plan.ts (#2186 bare-command-is-plan; #3917 enforced never-mutates; occupancy warning) + apply.ts (--allow-destructive / --force / --yes / in_sync)", "packages/cli/src/utils/format.ts#CliExitCode (CliExitCode narrows the emitJson exit slot — the #4873 fix, commit 83df2fd)", - "packages/cli/src/commands/migrate/ (the 16 registered subcommands under `os migrate`, besides the bare command, enumerated as variants — oclif pattern strategy, one default-exported Command per non-test file)", + "packages/cli/src/commands/migrate/ (the 17 registered subcommands under `os migrate`, besides the bare command, enumerated as variants — oclif pattern strategy, one default-exported Command per non-test file)", "sibling item cli.migrate-duplicates-inventory (the duplicates subcommand's own contract — cross-referenced, not duplicated)" ], "history": [ diff --git a/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts b/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts new file mode 100644 index 00000000000..23f2d945918 --- /dev/null +++ b/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts @@ -0,0 +1,258 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [ADR-0139 E6] The activity stream's withheld-row rule is a predicate over +// two writer-stamped columns, with no SYSTEM pre-scan and no window, on a real +// boot at the HTTP door. +// +// ## What changed +// +// The rule withholds, from a reader, an update row whose every recorded key it +// is not served (#21388) and a fired milestone row whose watched fields it is +// not all served (#22786). It used to be a WHERE built from a SYSTEM pre-scan +// of at most 2,000 rows, which capped every broad read of the stream: a +// member was served 725 of the 1,816 rows it may read, and an administrator's +// site-wide total read 1,999. It is now a predicate over `changed_fields` and +// `milestone_key`, which every writer stamps (`activity-changed-fields.ts`). +// +// ## The composition +// +// `bootStack` with the real `SecurityPlugin`, `ObjectQL`, SQL driver, REST and +// auth layers, plus `AuditPlugin`. A member owns one private record and is +// withheld one of its fields; a writer owns another the member cannot open. +// The store holds more than twice the old window: a bulk of activity rows +// about the two records, inserted as the system (the before-insert hook stamps +// them, as it stamps any writer's row), some of them updates of the withheld +// field only. +// +// `@objectstack/plugin-audit` resolves through its BUILT output here. +// Fixtures are synthetic. ⚠️ No test title states a value. + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { AuditPlugin } from '@objectstack/plugin-audit'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { assertArmed, armedWhen } from './armed.js'; +import { catalogPermissionSet } from './catalog-permission-set.js'; + +const ACTIVITY = 'sys_activity'; +const OBJ = 'awp_item'; +const SYS = { isSystem: true } as const; +/** The old pre-scan window. */ +const OLD_WINDOW = 2_000; +/** Rows about the member's record: served to it whole. */ +const SERVED_BULK = 2_050; +/** Rows about the member's record that change only the withheld field. */ +const WITHHELD_BULK = 50; +/** Rows about the writer's record, which the member cannot open. */ +const OTHER_BULK = 2_101; +const PASSWORD = 'Awp-Fixture-Pass-23731'; + +const Item = ObjectSchema.create({ + name: OBJ, + label: 'AWP Item', + pluralLabel: 'AWP Items', + // sharingModel omitted: a custom object defaults to private (ADR-0090). + fields: { + name: Field.text({ label: 'Name', required: true }), + awp_open: Field.text({ label: 'Open' }), + awp_secret: Field.text({ label: 'Secret' }), + }, +}); + +const fixtureStack = defineStack({ + manifest: { + id: 'com.dogfood.activity-withheld-row-predicate', + namespace: 'awp', + version: '0.0.0', + type: 'app', + name: 'Activity Withheld Row Predicate Fixture', + description: 'One private object whose activity rows a member and an administrator read past the old pre-scan window.', + }, + objects: [Item], +}); + +const read = { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }; +const write = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }; +const baselineSet = PermissionSetSchema.parse({ + name: 'awp_baseline', + label: 'AWP baseline', + objects: { [OBJ]: write, [ACTIVITY]: read }, +}); +const withholdSet = PermissionSetSchema.parse({ + name: 'awp_withhold', + label: 'AWP withhold', + objects: {}, + fields: { [`${OBJ}.awp_secret`]: { readable: false, editable: false } }, +}); + +type Row = Record; +const rowsOf = (body: any): Row[] => body?.records ?? body?.data ?? (Array.isArray(body) ? body : []); +const filterOf = (where: Record) => encodeURIComponent(JSON.stringify(where)); + +describe('[ADR-0139 E6] the withheld-row rule is a predicate: every row a reader may read, past the old window', () => { + let stack: VerifyStack; + let ql: any; + const token = { admin: '', member: '', writer: '' }; + const id = { mine: '', other: '' }; + /** The bulk row ids, by class. */ + const bulk = { served: [] as string[], withheld: [] as string[], other: [] as string[] }; + /** Every read of the activity stream, with whether it ran as the system. */ + const activityReads: Array<{ method: string; system: boolean; fields: string }> = []; + + const list = async (who: keyof typeof token, query: string) => { + const res = await stack.apiAs(token[who], 'GET', `/data/${ACTIVITY}?${query}`); + expect(res.status).toBe(200); + const body = (await res.json()) as any; + return { rows: rowsOf(body), total: Number(body.total) }; + }; + /** Walk a list a page at a time: the ids served, and the total each page reported. */ + const walk = async (who: keyof typeof token, where: Record) => { + const ids: string[] = []; + const totals = new Set(); + for (let skip = 0; skip < 10_000; skip += 500) { + const page = await list(who, `$filter=${filterOf(where)}&$orderby=id&$top=500&$skip=${skip}`); + totals.add(page.total); + ids.push(...page.rows.map((r) => String(r.id))); + if (page.rows.length < 500) break; + } + return { ids, totals: [...totals] }; + }; + + beforeAll(async () => { + stack = await bootStack(fixtureStack as unknown as Parameters[0], { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, baselineSet, withholdSet], + fallbackPermissionSet: baselineSet.name, + }), + extraPlugins: [new AuditPlugin()], + }); + ql = await stack.kernel.getServiceAsync('objectql'); + token.admin = await stack.signIn(); + const userIdOf = async (email: string) => + String((await ql.findOne('sys_user', { where: { email }, context: { ...SYS } }))?.id ?? ''); + const grant = async (email: string, setName: string) => { + const set = await catalogPermissionSet(ql, setName); + expect(set, `fixture permission set ${setName} served by the security catalog`).toBeTruthy(); + await ql.insert('sys_user_permission_set', { user_id: await userIdOf(email), permission_set: set.name }, { context: { ...SYS } }); + }; + for (const who of ['member', 'writer'] as const) { + const email = `awp-${who}@verify.test`; + token[who] = await stack.signUp(email, PASSWORD); + await grant(email, baselineSet.name); + } + await grant('awp-member@verify.test', withholdSet.name); + + const idOf = async (res: Response) => { + expect(res.status, 'fixture write').toBeLessThan(300); + const j = (await res.json()) as any; + return String(j.id ?? j.record?.id ?? j.data?.id); + }; + id.mine = await idOf(await stack.apiAs(token.member, 'POST', `/data/${OBJ}`, { name: 'mine' })); + id.other = await idOf(await stack.apiAs(token.writer, 'POST', `/data/${OBJ}`, { name: 'other' })); + + // The bulk, newest last, inserted as the system: the before-insert hook + // stamps each row from its recorded change. + const base = Date.now(); + let n = 0; + const plant = async (recordId: string, key: string) => { + n += 1; + const row = await ql.insert(ACTIVITY, { + type: 'updated', summary: `bulk ${n}`, object_name: OBJ, record_id: recordId, + timestamp: new Date(base + n * 1_000).toISOString(), + metadata: JSON.stringify({ old: { [key]: `a${n}` }, new: { [key]: `b${n}` } }), + }, { context: { ...SYS } }); + return String(row.id); + }; + for (let i = 0; i < SERVED_BULK; i++) bulk.served.push(await plant(id.mine, 'awp_open')); + for (let i = 0; i < WITHHELD_BULK; i++) bulk.withheld.push(await plant(id.mine, 'awp_secret')); + for (let i = 0; i < OTHER_BULK; i++) bulk.other.push(await plant(id.other, 'awp_open')); + + // Every read of the stream from here on, by context. + for (const method of ['find', 'findOne', 'count', 'aggregate'] as const) { + const original = ql[method].bind(ql); + vi.spyOn(ql, method).mockImplementation(async (...args: any[]) => { + if (args[0] === ACTIVITY) { + const ctx = args[1]?.context ?? args[2]?.context; + activityReads.push({ method, system: ctx?.isSystem === true, fields: JSON.stringify(args[1]?.fields ?? null) }); + } + return original(...args); + }); + } + + await assertArmed([ + armedWhen({ + control: 'the store holds more than twice the old window, and each bulk row carries the stamp of its recorded change', + disarmedBy: 'a store inside the window, or rows the hook did not stamp, would let every case below pass on a capped or fail-closed read', + observe: async () => { + const atRest: Row[] = await ql.find(ACTIVITY, { where: { object_name: OBJ }, limit: 10_000, context: { ...SYS } }); + const byId = new Map(atRest.map((r) => [String(r.id), r])); + return { + atRest: atRest.length, + served: JSON.stringify(byId.get(bulk.served[0])?.changed_fields), + withheld: JSON.stringify(byId.get(bulk.withheld[0])?.changed_fields), + }; + }, + armed: (o) => o.atRest > 2 * OLD_WINDOW && o.served === '["awp_open"]' && o.withheld === '["awp_secret"]', + describe: (o) => JSON.stringify(o), + }), + armedWhen({ + control: 'the member opens its own record and not the writer’s, and is withheld the one field', + disarmedBy: 'a member who could open the writer’s record, or read the field, would make the withheld cases below measure nothing', + observe: async () => { + const mine = (await (await stack.apiAs(token.member, 'GET', `/data/${OBJ}/${id.mine}`)).json()) as any; + return { + mine: 'name' in (mine?.record ?? mine ?? {}), + secretServed: 'awp_secret' in (mine?.record ?? mine ?? {}), + other: (await stack.apiAs(token.member, 'GET', `/data/${OBJ}/${id.other}`)).status, + }; + }, + armed: (o) => o.mine && !o.secretServed && (o.other === 404 || o.other === 403), + describe: (o) => JSON.stringify(o), + }), + ]); + }, 600_000); + + afterAll(async () => { + vi.restoreAllMocks(); + await stack?.stop(); + }); + + it('a member is served every activity row it may read, past the old window, and every page reports that total', async () => { + const atRest: Row[] = await ql.find(ACTIVITY, { where: { object_name: OBJ, record_id: id.mine }, limit: 10_000, context: { ...SYS } }); + const withheld = new Set(bulk.withheld); + const expected = atRest.map((r) => String(r.id)).filter((rowId) => !withheld.has(rowId)).sort(); + expect(expected.length).toBeGreaterThan(OLD_WINDOW); + const { ids, totals } = await walk('member', { object_name: OBJ }); + expect([...ids].sort()).toEqual(expected); + expect(totals).toEqual([expected.length]); + }); + + it('the member is served no update row whose every recorded key it is withheld, and no row about a record it cannot open', async () => { + const { ids } = await walk('member', { object_name: OBJ }); + const served = new Set(ids); + expect(bulk.withheld.filter((rowId) => served.has(rowId))).toEqual([]); + expect(bulk.other.filter((rowId) => served.has(rowId))).toEqual([]); + }); + + it('an administrator’s site-wide read (newest first, 20) is served the 20 newest rows and a total past the old window', async () => { + const { rows, total } = await list('admin', '$orderby=timestamp%20desc&$top=20'); + expect(rows.map((r) => String(r.id))).toEqual([...bulk.other].reverse().slice(0, 20)); + const fixture = (await list('admin', `$filter=${filterOf({ object_name: OBJ })}&$top=1`)).total; + const rest = (await list('admin', `$filter=${filterOf({ object_name: { $ne: OBJ } })}&$top=1`)).total; + // Every bulk row, every mirror row about the two records: nothing withheld from the administrator. + expect(fixture).toBe(SERVED_BULK + WITHHELD_BULK + OTHER_BULK + 2); + expect(total).toBe(fixture + rest); + expect(total).toBeGreaterThan(2 * OLD_WINDOW); + }); + + it('no read of the activity stream runs as the system while a member or an administrator is served it', async () => { + activityReads.length = 0; + await list('member', `$filter=${filterOf({ object_name: OBJ })}&$top=20`); + await list('admin', '$orderby=timestamp%20desc&$top=20'); + expect(activityReads.length).toBeGreaterThan(0); + expect(activityReads.filter((r) => r.system)).toEqual([]); + }); +}); From 44ec7841eec8712bfbd88b8435ec5fac3214fa22 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:15:15 +0000 Subject: [PATCH 07/11] =?UTF-8?q?test(dogfood):=20E6=20real-stack=20pins?= =?UTF-8?q?=20=E2=80=94=20no=20SYSTEM=20read=20of=20a=20row's=20change,=20?= =?UTF-8?q?no=20leaf?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- ...ity-withheld-row-predicate.dogfood.test.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts b/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts index 23f2d945918..de8bacff2e6 100644 --- a/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts +++ b/packages/qa/dogfood/test/activity-withheld-row-predicate.dogfood.test.ts @@ -32,7 +32,7 @@ import { bootStack, type VerifyStack } from '@objectstack/verify'; import { AuditPlugin } from '@objectstack/plugin-audit'; import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; import { defineStack } from '@objectstack/spec'; -import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { ObjectSchema, Field, isReadableIds } from '@objectstack/spec/data'; import { PermissionSetSchema } from '@objectstack/spec/security'; import { assertArmed, armedWhen } from './armed.js'; import { catalogPermissionSet } from './catalog-permission-set.js'; @@ -100,7 +100,11 @@ describe('[ADR-0139 E6] the withheld-row rule is a predicate: every row a reader /** The bulk row ids, by class. */ const bulk = { served: [] as string[], withheld: [] as string[], other: [] as string[] }; /** Every read of the activity stream, with whether it ran as the system. */ - const activityReads: Array<{ method: string; system: boolean; fields: string }> = []; + const activityReads: Array<{ method: string; system: boolean; fields: string; leaf: boolean }> = []; + /** Whether a WHERE carries a `ReadableIds` leaf anywhere. */ + const hasLeaf = (node: unknown): boolean => + isReadableIds(node) || + (Array.isArray(node) ? node.some(hasLeaf) : !!node && typeof node === 'object' && Object.values(node).some(hasLeaf)); const list = async (who: keyof typeof token, query: string) => { const res = await stack.apiAs(token[who], 'GET', `/data/${ACTIVITY}?${query}`); @@ -176,7 +180,12 @@ describe('[ADR-0139 E6] the withheld-row rule is a predicate: every row a reader vi.spyOn(ql, method).mockImplementation(async (...args: any[]) => { if (args[0] === ACTIVITY) { const ctx = args[1]?.context ?? args[2]?.context; - activityReads.push({ method, system: ctx?.isSystem === true, fields: JSON.stringify(args[1]?.fields ?? null) }); + activityReads.push({ + method, + system: ctx?.isSystem === true, + fields: JSON.stringify(args[1]?.fields ?? null), + leaf: hasLeaf(args[1]?.where), + }); } return original(...args); }); @@ -248,11 +257,16 @@ describe('[ADR-0139 E6] the withheld-row rule is a predicate: every row a reader expect(total).toBeGreaterThan(2 * OLD_WINDOW); }); - it('no read of the activity stream runs as the system while a member or an administrator is served it', async () => { + it('no SYSTEM read of the stream reads a row\'s change, and none carries a leaf, while a member or an administrator is served it', async () => { activityReads.length = 0; await list('member', `$filter=${filterOf({ object_name: OBJ })}&$top=20`); await list('admin', '$orderby=timestamp%20desc&$top=20'); - expect(activityReads.length).toBeGreaterThan(0); - expect(activityReads.filter((r) => r.system)).toEqual([]); + const system = activityReads.filter((r) => r.system); + expect(activityReads.length).toBeGreaterThan(system.length); + // The only SYSTEM read left is the read gate's bounded probe for ADR-0139 + // D5's classes, which reads the parent pair and nothing else; the + // withheld-row rule reads no row at all. + expect(system.filter((r) => r.fields !== JSON.stringify(['object_name', 'record_id']))).toEqual([]); + expect(system.filter((r) => r.leaf)).toEqual([]); }); }); From 44ed9438f82e8e3c2f0a86141a4456cbb87198fb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:19:59 +0000 Subject: [PATCH 08/11] chore(changeset): ADR-0139 E6 activity read-rule columns and backfill door Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- .changeset/23731-activity-changed-fields.md | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .changeset/23731-activity-changed-fields.md diff --git a/.changeset/23731-activity-changed-fields.md b/.changeset/23731-activity-changed-fields.md new file mode 100644 index 00000000000..7a08ca3149e --- /dev/null +++ b/.changeset/23731-activity-changed-fields.md @@ -0,0 +1,25 @@ +--- +'@objectstack/plugin-audit': minor +'@objectstack/cli': minor +--- + +feat(plugin-audit): `sys_activity` rows carry two writer-stamped read-rule columns, and the withheld-row rule is a predicate over them, with no SYSTEM pre-scan (ADR-0139 E6) + +**Run `os migrate activity-changed-fields --apply` at once after upgrading.** After the upgrade, and until it runs, the `sys_activity` rows written before the upgrade carry no stamp and are withheld from every non-system reader: activity feeds, the header bell and record timelines show only the rows written since the upgrade. The door is `os migrate activity-changed-fields`, dry run by default, `--apply` to write, idempotent, and it stamps both columns in one run. Its flags are `os migrate audit-metadata-bodies`'s, one for one (`--database-url`, `--apply`, `--yes`, `--force`, `--json`). The columns arrive with the upgraded deployment's schema sync, which an `--apply` run performs itself; a dry run against a database no upgraded process has opened cannot read them and exits `1`. + +Clause-②: yes + +`sys_activity` gains two columns, stamped at write time by one derivation (`planActivityStamp`): the CRUD mirror stamps its own rows, and a `sys_activity` before-insert hook stamps every row any other writer inserts (an app's own server action, the seed loader), over whatever value that writer sent. + +- `changed_fields` (`tags`) — the key set of the recorded change: for an update, every field its recorded `old` / `new` values name; `[]` for a create, a delete, a note, or an update that touched only `internal` fields. It is served narrowed to the fields the reader is served. +- `milestone_key` (`text`) — for a fired activity milestone, one sorted, comma-joined key of every field the object's `activityMilestones` declaration watched when it fired; `null` for every other row. It is never served. + +The withheld-row rule is now a WHERE over those two columns, one branch per parent object, on `find`, `findOne`, `count` and `aggregate`. It reads no row, so the 2,000-row SYSTEM pre-scan and its window are gone: a broad read of the stream is served every row the caller may read, with a true `total`. What the rule withholds: + +- an update row whose `changed_fields` is non-empty and names no field the reader is served (unchanged); +- a fired milestone row, unless its key equals the key of the object's CURRENT `activityMilestones` declaration and the reader is served every field that declaration watches. **The cost:** a milestone row written under an earlier declaration, or naming a since-removed field, is withheld from every non-exempt reader until retention ages it out (14 days declared; `lifecycle.retention_overrides` may lengthen it); +- an unstamped row, from every non-system reader: one at rest from before the upgrade until the backfill stamps it, and one the planner cannot judge (a milestone row naming no readable watched-field list), which stays unstamped. + +The parent-field query guard covers both columns as it covers `metadata`: a reader withheld any field of the parent is refused a filter, sort, group or aggregate over `changed_fields` or `milestone_key` (`PERMISSION_DENIED` / 403). + +`@objectstack/plugin-audit` exports the backfill, `backfillActivityChangedFields` (with `ACTIVITY_BACKFILL_PAGE_SIZE` and its report and engine types), and the planner, `planActivityStamp` (with `ACTIVITY_CHANGED_FIELDS_COLUMN`, `ACTIVITY_MILESTONE_KEY_COLUMN` and `ActivityStamp`). `computeWithheldUpdateFilter`, `isWithheldOnlyUpdate`, `isWithheldMilestone` and `isWithheldRow` were module-internal (never exported from the package) and are replaced by `computeWithheldRowFilter` and `keptRowsOfParent`. From 7de04a093dfe44a3210c674901970f1ec7e560a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 03:22:31 +0000 Subject: [PATCH 09/11] test(plugin-audit): type the no-registry engine double Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- .../src/activity-withheld-update.integration.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts index 4c7aabd813b..f641e4ae8cb 100644 --- a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts @@ -471,11 +471,13 @@ describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predic it('an engine with no registry view: denied, and said once', async () => { const logger = { warn: vi.fn() }; - const engine = { find: vi.fn() }; + const find = vi.fn(); + // No registry view: only a row reader, which the rule must not call. + const engine = { find } as unknown as Parameters[0]; const filter = await computeWithheldRowFilter(engine, {}, async () => ['x'], logger); expect(filter).toEqual({ id: '__activity_withheld_row_denied__' }); await computeWithheldRowFilter(engine, {}, async () => ['x'], logger); expect(logger.warn).toHaveBeenCalledTimes(1); - expect(engine.find).not.toHaveBeenCalled(); + expect(find).not.toHaveBeenCalled(); }); }); From 70ff93d3b03432d2b930d17e2605590f4a174e74 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 04:04:25 +0000 Subject: [PATCH 10/11] chore: i18n for the two activity columns; tenant-audit census for the backfill's write site; system-context page Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- content/docs/permissions/system-context.mdx | 2 +- .../docs/permissions/tenant-audit-census.mdx | 36 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 19 +++++----- .../src/translations/en.objects.generated.ts | 8 +++++ .../translations/es-ES.objects.generated.ts | 8 +++++ .../translations/ja-JP.objects.generated.ts | 8 +++++ .../translations/zh-CN.objects.generated.ts | 8 +++++ 7 files changed, 61 insertions(+), 28 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6019e6f9f72..70be5c8f168 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -166,7 +166,7 @@ One of the largest consumers of the flag; | 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, 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` | +| 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 bounded probe and the activity backfill — 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, of a fired milestone row whose watched fields it is not all served, and of an unstamped row (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` | | 47 | The inbound-mail read gates return early (the read AST) | service-mailbox | Get: every `sys_mail_message` and `sys_mail_association` row on `find` / `findOne` — the ingest writer and each gate's own pre-scan, which read the association and mailbox rows a decision needs. Lose: the narrowing of a message to callers who can read a record it is associated with or who own its mailbox (so the unassociated pool is restricted), and of an association to callers who can read the record it names (on `count` and `aggregate` too, so a total agrees with the rows) | `packages/services/service-mailbox/src/mail-read-visibility.ts#gateMiddleware` | | 47b | The inbound-mail ingest seam: `ingest` admits the system context ALONE, and `rematch` admits it without the organization, owner or `manage_mailboxes` check | service-mailbox | Get: a mail source hands a mailbox its messages, and server code re-matches any mailbox. Lose: nothing a user held — every other caller of `ingest` is refused (`PERMISSION_DENIED`), because an ingested message is shown on every record its correspondents' addresses match and a user who could hand the seam a message could write one in a correspondent's name; every other caller of `rematch` must be in the mailbox's organization and own the mailbox or hold `manage_mailboxes` | `packages/services/service-mailbox/src/ingest/mail-ingest-service.ts#ingest`, `#authorizeRematch` | diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index a0d13072c4a..4b14b834cd0 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **53 of the 225 sites are spelled that way**. A +forwarding shim cannot, and **53 of the 226 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **225** | +| 175 write call sites | quoted in the merged changeset | **226** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **154 of 225** decidable, **71** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 121 decidably elevated, 0 decidably not, 96 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **155 of 226** decidable, **71** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 122 decidably elevated, 0 decidably not, 96 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -201,17 +201,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 38 sites reached -through an erased (`any`) receiver, and the 50 that name their object through a +through an erased (`any`) receiver, and the 51 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -121 of 225 (54%) as decidably elevated, with 96 more whose elevation is a +122 of 226 (54%) as decidably elevated, with 96 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 225`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 226`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -223,29 +223,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **225** | -| …whose object name is statically decidable | 154 | +| write call sites on the application surface | **226** | +| …whose object name is statically decidable | 155 | | …whose object name is chosen at run time | 71 | -| …against an object with tenancy ENABLED | 153 | +| …against an object with tenancy ENABLED | 154 | | …against an object that declares tenancy off | 1 | -| threading a tenant context | 164 | +| threading a tenant context | 165 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 53 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 121 | +| threading a decidably ELEVATED (`isSystem`) context | 122 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 96 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 187 | +| receiver carried a readable engine type | 188 | | receiver erased, placed by the object NAME | 28 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | | object name spelled inline | 104 | -| object name spelled through a `const` | 50 | +| object name spelled through a `const` | 51 | | object name is an `object: string` parameter | 7 | | object name is some other run-time expression | 64 | @@ -297,13 +297,13 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-14 at `95898bba1`. +Measured on 2026-10-16 at `c53c594e7`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 649 | -| engine-shaped types recognised | 75 | +| tracked non-test sources scanned | 654 | +| engine-shaped types recognised | 77 | | declared objects in the registry | 116 | -| same-named calls subtracted as non-engine | 164 | +| same-named calls subtracted as non-engine | 165 | {/* END GENERATED: tenant-audit-census */} diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 996b49762e8..997257c9558 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 225 | -| Object name statically decidable | 154 | +| Write call sites | 226 | +| Object name statically decidable | 155 | | Object name chosen at run time | 71 | -| Against a tenancy-enabled object | 153 | +| Against a tenancy-enabled object | 154 | | Against an object declaring tenancy off | 1 | -| Threading a tenant context | 164 | +| Threading a tenant context | 165 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 53 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 121 | +| Threading a decidably elevated context | 122 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 96 | @@ -90,14 +90,14 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-14 at `95898bba1`. +Measured on 2026-10-16 at `c53c594e7`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 649 | -| engine-shaped types recognised | 75 | +| tracked non-test sources scanned | 654 | +| engine-shaped types recognised | 77 | | declared objects in the registry | 116 | -| same-named calls subtracted as non-engine | 164 | +| same-named calls subtracted as non-engine | 165 | ## Every site @@ -115,6 +115,7 @@ Measured on 2026-10-14 at `95898bba1`. | `packages/plugins/plugin-approvals/src/approval-service.ts` | `insert` | `sys_approval_token` | enabled | elevated | 1 | | `packages/plugins/plugin-approvals/src/approval-service.ts` | `update` | `sys_approval_token` | enabled | elevated | 1 | | `packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` | `update` | `objectPlan.object` | undecidable | context, elevation undecidable | 1 | +| `packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts` | `update` | `sys_activity` | enabled | elevated | 1 | | `packages/plugins/plugin-audit/src/auth-event-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 | | `packages/plugins/plugin-audit/src/read-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 | | `packages/plugins/plugin-audit/src/stored-metadata-body-migration.ts` | `update` | `object` | undecidable | elevated | 1 | diff --git a/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts index bdf7b9a9c1f..794caf03079 100644 --- a/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts @@ -181,6 +181,14 @@ export const enObjects: NonNullable = { metadata: { label: "Metadata", help: "JSON-serialized additional context" + }, + changed_fields: { + label: "Changed Fields", + help: "The key set of the recorded change: for an update, every field its recorded old/new values name; empty for every other row. Stamped by the platform at write time; served narrowed to the fields the reader is served. NULL means the row is not stamped yet (run `os migrate activity-changed-fields`)." + }, + milestone_key: { + label: "Milestone Key", + help: "For a fired activity milestone, one sorted, comma-joined key of every field the object's activityMilestones declaration watched when it fired; NULL for every other row. Stamped by the platform at write time and never served: the read rule compares it with the current declaration." } } }, diff --git a/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts index 6da3713902e..8823ec5ac22 100644 --- a/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts @@ -181,6 +181,14 @@ export const esESObjects: NonNullable = { metadata: { label: "Metadatos", help: "Contexto adicional serializado en JSON." + }, + changed_fields: { + label: "Campos modificados", + help: "El conjunto de claves del cambio registrado: para una actualización, cada campo que nombran sus valores antiguos/nuevos registrados; vacío para cualquier otra fila. La plataforma lo marca al escribir; se sirve reducido a los campos que se sirven al lector. NULL significa que la fila aún no está marcada (ejecute `os migrate activity-changed-fields`)." + }, + milestone_key: { + label: "Clave de hito", + help: "Para un hito de actividad disparado, una clave ordenada y separada por comas de todos los campos que la declaración activityMilestones del objeto vigilaba al dispararse; NULL para cualquier otra fila. La plataforma la marca al escribir y nunca se sirve: la regla de lectura la compara con la declaración actual." } } }, diff --git a/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts index 6323eb5a04d..b109f732998 100644 --- a/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts @@ -181,6 +181,14 @@ export const jaJPObjects: NonNullable = { metadata: { label: "メタデータ", help: "JSON シリアライズされた追加コンテキスト" + }, + changed_fields: { + label: "変更フィールド", + help: "記録された変更のキー集合:更新の場合は、記録された旧値/新値が含むすべてのフィールド。その他の行では空。書き込み時にプラットフォームが付与し、閲覧者に提供されるフィールドに絞り込んで返されます。NULL はその行がまだ付与されていないことを示します(`os migrate activity-changed-fields` を実行してください)。" + }, + milestone_key: { + label: "マイルストーンキー", + help: "発火したアクティビティマイルストーンについて、発火時にオブジェクトの activityMilestones 宣言が監視していた全フィールドをソートしてカンマで連結したキー。その他の行では NULL。書き込み時にプラットフォームが付与し、決して返されません。読み取りルールが現在の宣言と比較します。" } } }, diff --git a/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts index 475f4520d3d..7bbdef13154 100644 --- a/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts @@ -181,6 +181,14 @@ export const zhCNObjects: NonNullable = { metadata: { label: "元数据", help: "附加上下文的 JSON 序列化内容" + }, + changed_fields: { + label: "变更字段", + help: "所记录变更的字段键集合:对于更新,为其记录的旧值/新值涉及的每个字段;其他行为空。由平台在写入时标记;仅返回读者有权查看的字段。NULL 表示该行尚未标记(请运行 `os migrate activity-changed-fields`)。" + }, + milestone_key: { + label: "里程碑键", + help: "对于已触发的活动里程碑,为该对象 activityMilestones 声明在触发时所监视的全部字段的排序、逗号连接的键;其他行为 NULL。由平台在写入时标记且从不返回:读取规则会将其与当前声明进行比较。" } } }, From d75ae9784332b0ba3d0eeb48efb8d1ee978eed5c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 05:03:31 +0000 Subject: [PATCH 11/11] fix(plugin-audit): keep ADR-0139's anchored names on the functions that carry those roles; pin membership over a prefix computeWithheldUpdateFilter keeps its name as the withheld-row predicate builder, and isWithheldOnlyUpdate is the update half over changed_fields, called by keptRowsOfParent. A served field whose name is a prefix of a withheld one keeps no withheld-only update row. Claude-Session: https://claude.ai/code/session_01TyhsxCqpTkk1U5GLD1JETy Co-authored-by: Claude --- .changeset/23731-activity-changed-fields.md | 2 +- .../src/activity-field-redaction.ts | 50 +++++++++++------ .../src/activity-read-visibility.ts | 2 +- ...tivity-withheld-update.integration.test.ts | 54 +++++++++++++++---- 4 files changed, 82 insertions(+), 26 deletions(-) diff --git a/.changeset/23731-activity-changed-fields.md b/.changeset/23731-activity-changed-fields.md index 7a08ca3149e..f0a4532f890 100644 --- a/.changeset/23731-activity-changed-fields.md +++ b/.changeset/23731-activity-changed-fields.md @@ -22,4 +22,4 @@ The withheld-row rule is now a WHERE over those two columns, one branch per pare The parent-field query guard covers both columns as it covers `metadata`: a reader withheld any field of the parent is refused a filter, sort, group or aggregate over `changed_fields` or `milestone_key` (`PERMISSION_DENIED` / 403). -`@objectstack/plugin-audit` exports the backfill, `backfillActivityChangedFields` (with `ACTIVITY_BACKFILL_PAGE_SIZE` and its report and engine types), and the planner, `planActivityStamp` (with `ACTIVITY_CHANGED_FIELDS_COLUMN`, `ACTIVITY_MILESTONE_KEY_COLUMN` and `ActivityStamp`). `computeWithheldUpdateFilter`, `isWithheldOnlyUpdate`, `isWithheldMilestone` and `isWithheldRow` were module-internal (never exported from the package) and are replaced by `computeWithheldRowFilter` and `keptRowsOfParent`. +`@objectstack/plugin-audit` exports the backfill, `backfillActivityChangedFields` (with `ACTIVITY_BACKFILL_PAGE_SIZE` and its report and engine types), and the planner, `planActivityStamp` (with `ACTIVITY_CHANGED_FIELDS_COLUMN`, `ACTIVITY_MILESTONE_KEY_COLUMN` and `ActivityStamp`). Inside the module (none of it is exported from the package), `computeWithheldUpdateFilter` keeps its name and now builds the predicate, `isWithheldOnlyUpdate` keeps its name as the update half over `changed_fields`, and `isWithheldMilestone` / `isWithheldRow` are gone with the pre-scan they judged for. diff --git a/packages/plugins/plugin-audit/src/activity-field-redaction.ts b/packages/plugins/plugin-audit/src/activity-field-redaction.ts index ffb025b0a30..f12950739f8 100644 --- a/packages/plugins/plugin-audit/src/activity-field-redaction.ts +++ b/packages/plugins/plugin-audit/src/activity-field-redaction.ts @@ -372,6 +372,32 @@ function currentMilestoneWatched(engine: ActivityRedactionEngine, object: string return declaredMilestoneWatchedFields(typeof engine.getSchema === 'function' ? engine.getSchema(object) : undefined); } +/** + * [#21388, ADR-0139 D4] The update half of the withheld-row rule — whether a + * stamped row is a withheld-only update — as a predicate over `changed_fields` + * for a reader served `served`. An update row with a non-empty `changed_fields` + * none of whose members this reader is served is withheld; so the predicate is + * written as the rows it KEEPS: `changed_fields` is empty (a create, a delete, a + * note, an update empty for everyone), or it holds a served field — one + * `$contains` per served field. On a multi-value column `$contains` is + * MEMBERSHIP (`filter.zod.ts`, the JSON-stored column paragraph), so a served + * field whose name is a prefix of a withheld one never keeps the row. + * + * Written positively, never as a `$not` of the withheld rows: `$not` is + * NULL-safe per leaf, so it would re-admit an unstamped row; the stamped + * conjunct beside it in {@link keptRowsOfParent} decides those. + * Exported for direct testing. + */ +export function isWithheldOnlyUpdate(served: readonly string[]): Record { + const members = [...new Set(served)].sort(); + return { + $or: [ + { [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $empty: true } }, + ...members.map((f) => ({ [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $contains: f } })), + ], + }; +} + /** * [ADR-0139 D4, E6] The rows of ONE parent object `P` a reader served * `served` keeps, as a predicate over the two writer-stamped columns @@ -380,11 +406,11 @@ function currentMilestoneWatched(engine: ActivityRedactionEngine, object: string * - **stamped** — `changed_fields` is not null. An unstamped row (written * before the column existed and not yet backfilled, or one the planner * could not judge) is withheld: ADR-0139 D4's answer for an unjudged row. - * - **the update half** (#21388) — `changed_fields` is empty, or it contains - * a field this reader is served: one `$contains` per served field. So an - * update whose every recorded key is withheld is withheld, and a create, a - * delete, a note, and an update that touched only `internal` fields (empty - * for everyone) are kept. + * - **the update half** (#21388, {@link isWithheldOnlyUpdate}) — + * `changed_fields` is empty, or it contains a field this reader is served: + * one `$contains` per served field. So an update whose every recorded key is + * withheld is withheld, and a create, a delete, a note, and an update that + * touched only `internal` fields (empty for everyone) are kept. * - **the milestone half** (#22786, ruled letter B) — `milestone_key` is null * (no milestone fired), or it EQUALS the key of P's current * `activityMilestones` declaration AND this reader is served every field @@ -400,19 +426,13 @@ export function keptRowsOfParent( watched: readonly string[], ): Record { const servedSet = new Set(served); - const members = [...servedSet].sort(); const key = milestoneKeyOf(watched); const milestoneServed = key !== null && watched.every((f) => servedSet.has(f)); return { $and: [ { object_name: parent }, { [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $null: false } }, - { - $or: [ - { [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $empty: true } }, - ...members.map((f) => ({ [ACTIVITY_CHANGED_FIELDS_COLUMN]: { $contains: f } })), - ], - }, + isWithheldOnlyUpdate(served), milestoneServed ? { $or: [{ [ACTIVITY_MILESTONE_KEY_COLUMN]: { $null: true } }, { [ACTIVITY_MILESTONE_KEY_COLUMN]: key }] } : { [ACTIVITY_MILESTONE_KEY_COLUMN]: { $null: true } }, @@ -439,7 +459,7 @@ export function keptRowsOfParent( * nothing can be judged: the read is denied, and that is said once. * Exported for direct testing. */ -export async function computeWithheldRowFilter( +export async function computeWithheldUpdateFilter( engine: ActivityRedactionEngine, ast: Record, servedFor: ServedFor, @@ -573,7 +593,7 @@ const JUDGED_BY = ['object_name', 'metadata'] as const; * * [#21388, #22786] Before the read runs, the same middleware ANDs the * withheld-row predicate into the WHERE of all four reads - * (`computeWithheldRowFilter`, ADR-0139 E6). `AuditPlugin` registers it after + * (`computeWithheldUpdateFilter`, ADR-0139 E6). `AuditPlugin` registers it after * the read gate; the two are independent conjuncts. */ export function installActivityFieldRedaction( @@ -591,7 +611,7 @@ export function installActivityFieldRedaction( const restrictedFor = restrictedPerRead(security); if (security && ctx.ast && PARENT_GATE_READ_OPS.has(ctx.operation)) { try { - const filter = await computeWithheldRowFilter(engine, ctx.ast, servedFor, logger); + const filter = await computeWithheldUpdateFilter(engine, ctx.ast, servedFor, logger); if (filter) andIntoWhere(ctx, filter); } catch (err) { // A failure to build the rule must never fall open into the timing leak. diff --git a/packages/plugins/plugin-audit/src/activity-read-visibility.ts b/packages/plugins/plugin-audit/src/activity-read-visibility.ts index fcdeea7495e..8dec0760809 100644 --- a/packages/plugins/plugin-audit/src/activity-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/activity-read-visibility.ts @@ -63,7 +63,7 @@ * rows, on the same four reads, so a list's total, its pages and a grouped * count agree with the rows served. It judges by the security service's field * answer, so it lives with the field redaction (`activity-field-redaction.ts`, - * `computeWithheldRowFilter`), whose middleware `AuditPlugin` registers after + * `computeWithheldUpdateFilter`), whose middleware `AuditPlugin` registers after * this one. [ADR-0139 E6] It is a predicate over two writer-stamped columns * (`changed_fields`, `milestone_key`), one branch per parent object this gate * enumerates ({@link ACTIVITY_GATE}): it reads no row and composes no leaf, so diff --git a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts index f641e4ae8cb..19c17a75782 100644 --- a/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts +++ b/packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts @@ -46,7 +46,7 @@ import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; import { AuditPlugin } from './audit-plugin.js'; -import { computeWithheldRowFilter, keptRowsOfParent } from './activity-field-redaction.js'; +import { computeWithheldUpdateFilter, keptRowsOfParent } from './activity-field-redaction.js'; import { planActivityStamp } from './activity-changed-fields.js'; const ACTIVITY = 'sys_activity'; @@ -54,6 +54,8 @@ const ACTIVITY = 'sys_activity'; const ITEM = 'awu_item'; /** A record the member reads with NO field served: every change is withheld. */ const SEALED = 'awu_sealed'; +/** [ADR-0139 E6] A served field whose name is a prefix of a withheld one: membership, not substring. */ +const PREFIX = 'awu_prefix'; const HARNESS_PACKAGE = 'com.objectstack.audit.test.activity-withheld-update'; const SYS = { isSystem: true } as const; @@ -84,6 +86,16 @@ const sealedObject = { }, }; +const prefixObject = { + name: PREFIX, + label: 'Withheld Update Prefix', + fields: { + name: { name: 'name', label: 'Name', type: 'text' as const }, + f: { name: 'f', label: 'F', type: 'text' as const }, + f_secret: { name: 'f_secret', label: 'F Secret', type: 'text' as const }, + }, +}; + type Row = Record; const meta = (r: Row | undefined) => (typeof r?.metadata === 'string' ? JSON.parse(r.metadata) : null); const changeKeys = (r: Row | undefined) => { @@ -109,6 +121,7 @@ describe('[#21388] an update whose every recorded change is withheld from the re if (context?.userId === MEMBER.userId) { if (object === ITEM) return all.filter((f) => !WITHHELD.has(f)); if (object === SEALED) return []; + if (object === PREFIX) return all.filter((f) => f !== 'f_secret'); } return all; }, @@ -154,6 +167,7 @@ describe('[#21388] an update whose every recorded change is withheld from the re engine.registerDriver(driver, true); engine.registry.registerObject(itemObject as any, HARNESS_PACKAGE); engine.registry.registerObject(sealedObject as any, HARNESS_PACKAGE); + engine.registry.registerObject(prefixObject as any, HARNESS_PACKAGE); await engine.syncSchemas(); // Every row below is written by the real audit writer's CRUD mirror. @@ -183,6 +197,14 @@ describe('[#21388] an update whose every recorded change is withheld from the re rowIds.sealedCreated = String(sealedRows.find((r) => r.type === 'created')?.id ?? ''); rowIds.sealedUpdated = String(sealedRows.find((r) => r.type === 'updated')?.id ?? ''); + // [ADR-0139 E6] `f` is served, `f_secret` is withheld, and `f` is a prefix of it. + ids.prefix = (await engine.insert(PREFIX, { name: 'prefix', f: 'p1', f_secret: 's1' }, { context: SYS })).id; + await update(PREFIX, ids.prefix, { f_secret: 's2' }); // withheld whole + await update(PREFIX, ids.prefix, { f: 'p2' }); // served whole + const prefixRows = (await engine.find(ACTIVITY, { where: { object_name: PREFIX }, context: SYS })) as Row[]; + rowIds.prefixWithheld = String(prefixRows.find((r) => changeKeys(r).join() === 'f_secret')?.id ?? ''); + rowIds.prefixServed = String(prefixRows.find((r) => changeKeys(r).join() === 'f')?.id ?? ''); + // [ADR-0139 E6] An app's own row, inserted with no stamp of its own: the // before-insert hook stamps it `[]` (not a recorded change), so it is kept. rowIds.note = String( @@ -384,6 +406,20 @@ describe('[#21388] an update whose every recorded change is withheld from the re expect(rows.map((r) => r.id)).toEqual([rowIds.withheld]); }); + it('[ADR-0139 E6] membership, not substring: an update of only a withheld field stays withheld though a served field\'s name is its prefix', async () => { + const atRest = (await engine.find(ACTIVITY, { where: { object_name: PREFIX }, context: SYS })) as Row[]; + expect(atRest.find((r) => r.id === rowIds.prefixWithheld)?.changed_fields).toEqual(['f_secret']); + const served = ((await engine.find(ACTIVITY, { + where: scoped(PREFIX, ids.prefix), orderBy: ORDER, context: MEMBER, + })) as Row[]).map((r) => r.id); + expect(served).not.toContain(rowIds.prefixWithheld); + expect(served).toContain(rowIds.prefixServed); + expect(await engine.count(ACTIVITY, { where: scoped(PREFIX, ids.prefix) }, { context: MEMBER })).toBe(served.length); + // Control: the administrator is served both. + const admin = ((await engine.find(ACTIVITY, { where: scoped(PREFIX, ids.prefix), context: ADMIN })) as Row[]).map((r) => r.id); + expect(admin).toEqual(expect.arrayContaining([rowIds.prefixWithheld, rowIds.prefixServed])); + }); + it('a system read is not narrowed', async () => { expect((await read(SYS)).map((r) => r.id)).toContain(rowIds.withheld); }); @@ -418,7 +454,7 @@ describe('[ADR-0139 E6] planActivityStamp — the one derivation of the stamp', }); }); -describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predicate, no row read', () => { +describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldUpdateFilter — a predicate, no row read', () => { const engineWith = (objects: Array<{ name: string; activityMilestones?: unknown }>) => ({ registry: { getAllObjects: () => objects }, getSchema: (name: string) => objects.find((o) => o.name === name), @@ -445,10 +481,10 @@ describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predic it('a parent the service gives no answer for is kept whole; with no answer for any, no filter at all', async () => { const engine = engineWith([{ name: 'a' }, { name: 'b' }]); const servedFor = async (o: string) => (o === 'a' ? ['x'] : undefined); - expect(await computeWithheldRowFilter(engine, {}, servedFor, { warn: vi.fn() })).toEqual({ + expect(await computeWithheldUpdateFilter(engine, {}, servedFor, { warn: vi.fn() })).toEqual({ $or: [keptRowsOfParent('a', ['x'], []), { object_name: 'b' }], }); - expect(await computeWithheldRowFilter(engine, {}, async () => undefined, { warn: vi.fn() })).toBeNull(); + expect(await computeWithheldUpdateFilter(engine, {}, async () => undefined, { warn: vi.fn() })).toBeNull(); expect(engine.find).not.toHaveBeenCalled(); }); @@ -456,7 +492,7 @@ describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predic const engine = engineWith([{ name: 'a' }, { name: 'b' }]); const servedFor = vi.fn(async () => ['x']); const where = { $and: [{ object_name: 'b' }, { record_id: '1' }] }; - expect(await computeWithheldRowFilter(engine, { where }, servedFor, { warn: vi.fn() })).toEqual( + expect(await computeWithheldUpdateFilter(engine, { where }, servedFor, { warn: vi.fn() })).toEqual( keptRowsOfParent('b', ['x'], []), ); expect(servedFor).toHaveBeenCalledTimes(1); @@ -464,7 +500,7 @@ describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predic it('the audit and activity objects themselves are not enumerated (the read gate excludes their rows)', async () => { const engine = engineWith([{ name: 'sys_activity' }, { name: 'sys_audit_log' }, { name: 'a' }]); - expect(await computeWithheldRowFilter(engine, {}, async () => ['x'], { warn: vi.fn() })).toEqual( + expect(await computeWithheldUpdateFilter(engine, {}, async () => ['x'], { warn: vi.fn() })).toEqual( keptRowsOfParent('a', ['x'], []), ); }); @@ -473,10 +509,10 @@ describe('[ADR-0139 E6] keptRowsOfParent / computeWithheldRowFilter — a predic const logger = { warn: vi.fn() }; const find = vi.fn(); // No registry view: only a row reader, which the rule must not call. - const engine = { find } as unknown as Parameters[0]; - const filter = await computeWithheldRowFilter(engine, {}, async () => ['x'], logger); + const engine = { find } as unknown as Parameters[0]; + const filter = await computeWithheldUpdateFilter(engine, {}, async () => ['x'], logger); expect(filter).toEqual({ id: '__activity_withheld_row_denied__' }); - await computeWithheldRowFilter(engine, {}, async () => ['x'], logger); + await computeWithheldUpdateFilter(engine, {}, async () => ['x'], logger); expect(logger.warn).toHaveBeenCalledTimes(1); expect(find).not.toHaveBeenCalled(); });