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 <noreply@anthropic.com>

Author: claude

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

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<string, unknown> {
+  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<string, unknown> {
   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<string, unknown>,
   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.

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

packages/plugins/plugin-audit/src/activity-withheld-update.integration.test.ts

@@ -46,14 +46,16 @@ 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';
 /** A record the member reads with two fields withheld. */
 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<string, any>;
 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,26 +481,26 @@ 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();
   });
 
   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(
+    expect(await computeWithheldUpdateFilter(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(
+    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<typeof computeWithheldRowFilter>[0];
-    const filter = await computeWithheldRowFilter(engine, {}, async () => ['x'], logger);
+    const engine = { find } as unknown as Parameters<typeof computeWithheldUpdateFilter>[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();
   });