fix(plugin-approvals): lookup-target titles and the record-lock refusal title are served only from fields the caller may read

.changeset/23618-approval-title-family-served-fields.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): the inbox's referenced-record titles (`payload_display`) and the record-lock refusal's title are served only from fields the caller may read
+
+Clause-②: no
+
+- **Referenced-record titles follow the reader's field-level security on the referenced object.** `listRequests` and `getRequest` now pick each `payload_display` title from the referenced record as served to the reading caller: the same answer the payload snapshot and `record_title` are narrowed by (readable, and not masked for that caller), asked for the referenced object, once per object per read. A display field the reader may not read on the referenced object, or is served masked, gives that key no display value, so the snapshot's stored id stands, as it already does for a deleted referenced record. A title-eligible field the reader may read still names the record, and a reader who may read the display field gets the same title as before.
+- **The `RECORD_LOCKED` refusal names the record only from what the writer may read.** The per-record refusal sentence takes its title from the record as served to the refused writer, asked with the writer's own execution context. When the writer may not read the display field, or is served it masked, the sentence names the object's label alone (`This Opportunity is locked …`), as it already does for a record with no title; it never names the id. A writer who may read the field is told the title as before. The code (`RECORD_LOCKED`) and status (409) are unchanged.
+
+A patch: a narrowing of served values only. Nothing is granted, and no schema, route, error code, translation key or export moves. With no field-visibility source wired, every title is served as before.

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -1535,8 +1535,8 @@ export class ApprovalService implements IApprovalService {
 
   /**
    * [#10749 / #23594] The subject fields the READING caller may be served, per
-   * subject object of `rows`: {@link resolveReadableSnapshotFields}' answer
-   * (readable, and not masked for this caller), asked as the caller.
+   * subject object of `rows` — {@link ApprovalService.servedFields} for the
+   * rows' subject objects.
    *
    * It is the ONE answer both serve-time narrowings of a row read: the payload
    * snapshot ({@link ApprovalService.redactPayloads}) and the subject's title
@@ -1546,13 +1546,31 @@ export class ApprovalService implements IApprovalService {
    * pass-through, and it passes the title through exactly as it passes the
    * snapshot.
    */
-  private async servedSubjectFields(
+  private servedSubjectFields(
     rows: readonly ApprovalRequestRow[],
     context: ExecutionContext,
   ): Promise<Map<string, string[] | undefined>> {
-    const served = new Map<string, string[] | undefined>();
-    for (const r of rows) {
-      const object = String((r as any).object_name ?? '');
+    return this.servedFields(rows.map((r) => String((r as any).object_name ?? '')), context);
+  }
+
+  /**
+   * [#10749 / #23594 / #23618] The fields the READING caller may be served on
+   * each of `objects`: {@link resolveReadableSnapshotFields}' answer (readable,
+   * and not masked for this caller), asked as the caller — the one place a row
+   * read asks it.
+   *
+   * `served` holds the answers this read has already asked; an object in it is
+   * not asked again. So one read asks once per object, whether that object is a
+   * request's subject or the target of a lookup in a snapshot
+   * ({@link ApprovalService.enrichRows}), and a target that is also a subject
+   * object is narrowed by the same answer as the subject.
+   */
+  private async servedFields(
+    objects: Iterable<string>,
+    context: ExecutionContext,
+    served: Map<string, string[] | undefined> = new Map(),
+  ): Promise<Map<string, string[] | undefined>> {
+    for (const object of objects) {
       if (served.has(object)) continue;
       served.set(object, await resolveReadableSnapshotFields(
         this.fieldVisibility, object, context, this.logger,
@@ -6672,6 +6690,8 @@ export class ApprovalService implements IApprovalService {
    * (ADR-0049) for `get`, the read `GET /data/{target}/{id}` performs and the
    * operation the data door's `$expand` and the dataset door's labels ask of
    * the same target. It takes no caller: the read runs under a system context.
+   * [#23618] What is SERVED from that read is the reader's: the title is picked
+   * from the target as served to the reader ({@link titlesAsServed}).
    *
    * A refused target is not read, so its key gets no display value and the
    * snapshot's stored id stands, the answer a deleted target already gets. A
@@ -6745,23 +6765,48 @@ export class ApprovalService implements IApprovalService {
     for (const [object, idSet] of byObject) {
       const label = this.objectLabelOf(object);
       if (label) objectLabels.set(object, label);
-      const ids = Array.from(idSet);
-      const displayField = this.resolveDisplayField(object);
-      const readable = served.get(object);
       try {
-        const recs = await this.engine.find(object, {
-          where: { id: { $in: ids } }, limit: ids.length, context: SYSTEM_CTX,
-        });
-        for (const rec of (recs ?? []) as any[]) {
-          const asServed = readable === undefined ? rec : redactSnapshot(rec, readable).payload;
-          const title = ApprovalService.pickTitle(asServed, displayField);
-          if (rec?.id && title) titles.set(`${object} ${rec.id}`, title);
-        }
+        await this.titlesAsServed(object, idSet, served.get(object), titles);
       } catch { /* object may be unregistered — the caller's fallback applies */ }
     }
     return { titles, objectLabels };
   }
 
+  /**
+   * [#23594 / #23618] Title the `ids` of `object` into `into` (keyed
+   * `"<object> <id>"`), each picked from the record AS SERVED to the reader:
+   * `readable` is the reader's served fields for `object`
+   * ({@link ApprovalService.servedFields}), and `redactSnapshot` keeps exactly
+   * the fields the payload snapshot keeps, so a display field the reader may
+   * not read, or is served masked, gives no title, while a field the reader may
+   * be served still names the record. `undefined` is the seam's pass-through.
+   *
+   * The ONE spelling of how an approval read titles a record, shared by the
+   * subject's `record_title` ({@link ApprovalService.resolveSubjectDisplays})
+   * and the lookup targets' `payload_display` ({@link ApprovalService.enrichRows}).
+   * The records are read under the system context, so the read narrows nothing
+   * (#7497 ruling C: record access stays with object permissions, sharing and
+   * row-level rules); only what is served from it is narrowed. Throws what the
+   * read throws; each caller keeps its own best-effort catch.
+   */
+  private async titlesAsServed(
+    object: string,
+    ids: Iterable<string>,
+    readable: string[] | undefined,
+    into: Map<string, string>,
+  ): Promise<void> {
+    const list = Array.from(ids);
+    const displayField = this.resolveDisplayField(object);
+    const recs = await this.engine.find(object, {
+      where: { id: { $in: list } }, limit: list.length, context: SYSTEM_CTX,
+    });
+    for (const rec of (recs ?? []) as any[]) {
+      const asServed = readable === undefined ? rec : redactSnapshot(rec, readable).payload;
+      const title = ApprovalService.pickTitle(asServed, displayField);
+      if (rec?.id && title) into.set(`${object} ${rec.id}`, title);
+    }
+  }
+
   /**
    * An object's schema label, or `undefined` — the one spelling of "the
    * object's label" the inbox row ({@link ApprovalService.resolveSubjectDisplays})
@@ -6787,7 +6832,8 @@ export class ApprovalService implements IApprovalService {
    * reader to narrow for.
    */
   private nameSubjectRecord(object: string, recordId: string): string {
-    // [#23594] The title would go here; withheld until resolveSubjectDisplays narrows it by each reader's FLS.
+    // [#23594 / #23556 ruling B] The inbox serves the title narrowed per reader (resolveSubjectDisplays); this body is
+    // written once at send time, with no per-recipient reader to narrow for, so it stays label-and-id.
     return `${this.objectLabelOf(object) ?? object} ${recordId}`;
   }
 
@@ -6804,10 +6850,20 @@ export class ApprovalService implements IApprovalService {
    * does not serve `get` is not read ({@link servesPayloadDisplayTarget}).
    * [#23594] `record_title` is narrowed by the reader's `served` fields, the
    * answer the payload was redacted by ({@link resolveSubjectDisplays}).
+   * [#23618] So is every `payload_display` title, by the reader's served fields
+   * on the TARGET object: the lookup keys come from the already-redacted
+   * snapshot, and the record a readable key points at is titled from the target
+   * as served to `context`'s caller ({@link titlesAsServed}). A target display
+   * field the reader may not be served gives that key no display value, so the
+   * snapshot's stored id stands — the answer a deleted or exposure-refused
+   * target gets. The targets' answers are asked through
+   * {@link servedFields} into `served`, once per object per read, so a target
+   * that is also a subject object reuses the subject's answer.
    */
   private async enrichRows(
     rows: ApprovalRequestRow[],
-    served: ReadonlyMap<string, string[] | undefined>,
+    served: Map<string, string[] | undefined>,
+    context: ExecutionContext,
   ): Promise<void> {
     if (!rows.length) return;
 
@@ -6845,18 +6901,11 @@ export class ApprovalService implements IApprovalService {
       }
     }
     const refTitles = new Map<string, string>();
-    for (const [object, idSet] of refIds) {
-      if (!this.servesPayloadDisplayTarget(object)) continue;
-      const ids = Array.from(idSet);
-      const displayField = this.resolveDisplayField(object);
+    const targets = Array.from(refIds.keys()).filter((object) => this.servesPayloadDisplayTarget(object));
+    await this.servedFields(targets, context, served);
+    for (const object of targets) {
       try {
-        const recs = await this.engine.find(object, {
-          where: { id: { $in: ids } }, limit: ids.length, context: SYSTEM_CTX,
-        });
-        for (const rec of (recs ?? []) as any[]) {
-          const title = ApprovalService.pickTitle(rec, displayField);
-          if (rec?.id && title) refTitles.set(`${object} ${rec.id}`, title);
-        }
+        await this.titlesAsServed(object, refIds.get(object)!, served.get(object), refTitles);
       } catch { /* referenced object unreadable — leave unresolved */ }
     }
 
@@ -7527,10 +7576,11 @@ export class ApprovalService implements IApprovalService {
     const rows = await this.engine.find('sys_approval_request', findOpts);
     const list = Array.isArray(rows) ? rows.map(rowFromRequest) : [];
     // [#10749] Redact before enrichment — see `redactPayloads`. [#23594] The
-    // snapshot and the subject title are narrowed by ONE answer for the reader.
+    // snapshot and the subject title are narrowed by ONE answer for the reader;
+    // [#23618] the lookup targets' titles by the same reader's answers for them.
     const served = await this.servedSubjectFields(list, context);
     this.redactPayloads(list, served);
-    await this.enrichRows(list, served);
+    await this.enrichRows(list, served, context);
     this.attachViewers(list, context, caller);
     return list;
   }
@@ -7640,10 +7690,11 @@ export class ApprovalService implements IApprovalService {
     }
     const row = rowFromRequest(raw);
     // [#10749] Redact before enrichment — see `redactPayloads`. [#23594] The
-    // snapshot and the subject title are narrowed by ONE answer for the reader.
+    // snapshot and the subject title are narrowed by ONE answer for the reader;
+    // [#23618] the lookup targets' titles by the same reader's answers for them.
     const served = await this.servedSubjectFields([row], context);
     this.redactPayloads([row], served);
-    await this.enrichRows([row], served);
+    await this.enrichRows([row], served, context);
     await this.attachFlowSteps(row);
     await this.attachDecisionProgress(row, raw);
     this.attachViewers([row], context, caller);

packages/plugins/plugin-approvals/src/approvals-plugin.ts

@@ -305,7 +305,9 @@ export class ApprovalsServicePlugin implements Plugin {
     if (!this.options.disableAutoHooks) {
       try {
         unbindAllHooks(engine);
-        bindApprovalLockHook(engine, ctx.logger);
+        // [#23618] The refusal's title is narrowed by the writer's served fields,
+        // through the same lazily-resolved authority the snapshot redaction reads.
+        bindApprovalLockHook(engine, ctx.logger, fieldVisibility);
         bindRecordDeleteCancelHook(engine, this.service, ctx.logger);
         bindDelegationWriteGuard(engine, ctx.logger);
         // [#10749] Generic-door snapshot redaction. Registered here so the

packages/plugins/plugin-approvals/src/implicit-reference-target-lookup.test.ts

@@ -73,12 +73,13 @@ function makeService(
   const resolve = (object: string) =>
     (service as unknown as { resolveLookupFields(o: string): Array<{ key: string; reference: string }> })
       .resolveLookupFields(object);
-  // [#23594] No field-visibility answer for any object: the reader's served
-  // fields narrow nothing here, as with no security service wired.
+  // [#23594 / #23618] No security service wired, so no field-visibility answer
+  // for any object, subject or target: the reader's served fields narrow
+  // nothing here.
   const enrich = (rows: InboxRow[]) =>
     (service as unknown as {
-      enrichRows(r: InboxRow[], served: ReadonlyMap<string, string[] | undefined>): Promise<void>;
-    }).enrichRows(rows, new Map());
+      enrichRows(r: InboxRow[], served: Map<string, string[] | undefined>, context: unknown): Promise<void>;
+    }).enrichRows(rows, new Map(), { userId: 'u_reader', positions: [], permissions: [] });
   return { resolve, enrich, warn, finds };
 }
 

packages/plugins/plugin-approvals/src/lifecycle-hooks.ts

@@ -110,11 +110,28 @@
  * that. When no title is in hand the sentence degrades to the object's label,
  * and when that is missing too, to "This record" — ⛔ never back to the id.
  *
+ * ## The title is the WRITER's to be served (#23618)
+ *
+ * `ctx.previous` is the row as STORED (a raw driver read, declared columns
+ * only), narrowed by no caller's field-level security — and the sentence is
+ * served to the writer. So the title is picked from the pre-image AS SERVED to
+ * the writer, by the rule every approval-path title answers to
+ * (`payload-redaction.ts`, #10749: keyed on the caller): the served-fields
+ * answer, `resolveReadableSnapshotFields` (readable, and not masked for this
+ * caller), asked with the writer's own envelope ({@link writerEnvelope}). A
+ * display field the writer may not be served gives no title, and the sentence
+ * degrades as above. Still no record read: the cost is one field-visibility
+ * question, on the deny path only, so a write the lock does not refuse asks
+ * nothing. With no field-visibility authority wired, the title passes through
+ * as before, exactly as the payload snapshot does.
+ *
  * Registered under `packageId: 'plugin-approvals:lock'` so it can be cleanly
  * unbound on plugin stop.
  */
 
+import { withoutOperationPrivateKeys } from '@objectstack/core';
 import { resolveDisplayField } from '@objectstack/spec/data';
+import { redactSnapshot, resolveReadableSnapshotFields, type FieldVisibilitySource } from './payload-redaction.js';
 
 export const APPROVALS_HOOK_PACKAGE = 'plugin-approvals:lock';
 
@@ -238,6 +255,23 @@ function recordTitleOf(
   return title;
 }
 
+/**
+ * [#23618] The WRITER's own execution envelope — the one riding on the
+ * operation (`input.options.context`), minus its operation-private keys — or
+ * `undefined` when the write carried none.
+ *
+ * ⛔ Never rebuilt from `ctx.session`: the session is a projection of the
+ * envelope that drops `permissions` (and spells the org `organizationId`), so a
+ * field-visibility question asked with it would be a second derivation of the
+ * writer's authority beside the envelope the security middleware judged the
+ * write by. The `__` keys go for the reason `withoutOperationPrivateKeys`
+ * states, and the copy keeps a callee from writing back into the operation.
+ */
+function writerEnvelope(ctx: any): unknown {
+  const exec = ctx?.input?.options?.context;
+  return exec && typeof exec === 'object' ? withoutOperationPrivateKeys(exec as Record<string, unknown>) : undefined;
+}
+
 /**
  * The END-USER sentence for a record held by a live approval (#18153).
  *
@@ -439,8 +473,17 @@ async function gatingRequests(
 /**
  * Bind the global record-lock hook. Caller is responsible for calling
  * {@link unbindAllHooks} first if re-binding.
+ *
+ * `fieldVisibility` resolves the field-visibility authority the refusal's title
+ * is narrowed by (#23618) — lazily, per refusal, because the security plugin may
+ * start after this one. Absent, or resolving nothing, the title is served as
+ * before, exactly as the payload snapshot is (`resolveReadableSnapshotFields`).
  */
-export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogger): void {
+export function bindApprovalLockHook(
+  engine: MinimalEngine,
+  logger?: MinimalLogger,
+  fieldVisibility?: () => FieldVisibilitySource | undefined,
+): void {
   engine.registerHook('beforeUpdate', async (ctx: any) => {
     const object = (ctx?.object ?? ctx?.objectName) as string | undefined;
     // No object name (shouldn't happen) or our own bookkeeping objects → skip.
@@ -518,12 +561,25 @@ export function bindApprovalLockHook(engine: MinimalEngine, logger?: MinimalLogg
       const previous = ctx?.previous as Record<string, unknown> | undefined;
       const previousId = previous ? String(previous.id ?? previous._id ?? '') : '';
       const record = previousId && previousId === recordId ? previous : undefined;
+      // [#23618] …and it is the row as STORED, narrowed by no caller's
+      // field-level security, while this sentence is served to the WRITER. So
+      // the title is picked from it AS SERVED to the writer: the served-fields
+      // answer the approval inbox narrows its titles and snapshots by (readable,
+      // and not masked for this caller), asked with the writer's own envelope.
+      // A display field the writer may not be served gives no title, and the
+      // sentence degrades to the label — ⛔ never to the id.
+      const served = record
+        ? await resolveReadableSnapshotFields(fieldVisibility?.(), object, writerEnvelope(ctx), logger)
+        : undefined;
+      const asServed = record && served !== undefined
+        ? redactSnapshot(record, served).payload as Record<string, unknown>
+        : record;
 
       logger?.info?.(
         `[approvals] update refused RECORD_LOCKED: record '${recordId}' of '${object}' ` +
         `is held by pending approval request '${String(pending?.id ?? '')}'`,
       );
-      lockedError(recordLockRefusal(objectLabel, recordTitleOf(schema, record, recordId)));
+      lockedError(recordLockRefusal(objectLabel, recordTitleOf(schema, asServed, recordId)));
     }
   }, { packageId: APPROVALS_HOOK_PACKAGE, priority: 50 });