fix(plugin-approvals): a request's subject title is served only from fields its reader may read

.changeset/23594-subject-title-reader-fls.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): a request's `record_title` is served only from fields its reader may read, and the one-tap action pages name the record by its id
+
+Clause-②: no
+
+- **The inbox title follows the reader's field-level security.** `listRequests`, `getRequest` and every action's read-back row (which the request actions' success messages read) now pick `record_title` from the subject record as served to the reading caller: the same answer the payload snapshot is redacted by (readable, and not masked for that caller). A display field the reader may not read, or is served masked, gives no title, and the key is absent, the shape the redaction gives that field in the payload. 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. Deleted records fall back to the reader's redacted snapshot, as before. Nothing is granted: record access stays with object permissions, sharing and row-level rules.
+- **The action-link pages name the record by its id.** The session-less confirm and result pages (ADR-0043) have no reader to narrow for, so their "Record" row shows the record id beside the object's label, as the approval notification names it, and never the title.
+
+A patch: a narrowing of served values only. No schema, route, translation key, export or notification field moves. With no field-visibility source wired, every title is served as before.

packages/plugins/plugin-approvals/src/action-link-pages.ts

@@ -48,11 +48,23 @@ function shell(title: string, body: string): string {
 </style></head><body><div class="card">${body}</div></body></html>`;
 }
 
+/**
+ * The request summary both pages show.
+ *
+ * [#23594] The record is named by its id, never by its title. A subject title
+ * is served only to a reader who may read the field it comes from, narrowed by
+ * that reader's field-level security exactly as the payload snapshot is
+ * (#10749). These pages have no reader to narrow for: the bearer carries a
+ * token, not a session, and the token chain reads the request under the system
+ * context, which field-level security does not narrow. So no subject field is
+ * shown here — the record is named the way the approval notification names it,
+ * by the object's label and the record's id.
+ */
 function summaryRows(req: ApprovalRequestRow): string {
   const rows: Array<[string, string]> = [
     ['Process · 流程', req.process_label || req.process_name],
     ['Step · 步骤', req.step_label || req.current_step || '—'],
-    ['Record · 记录', req.record_title || req.record_id],
+    ['Record · 记录', req.record_id],
     ['Object · 对象', req.object_label || req.object_name],
     ['Requester · 申请人', req.submitter_name || req.submitter_id || '—'],
   ];

packages/plugins/plugin-approvals/src/action-page-member.integration.test.ts

@@ -131,7 +131,10 @@ async function bootStack(options: { rawApp: boolean }) {
 
   const svc = kernel.getService<ApprovalService>('approvals');
   let n = 0;
-  /** A pending request on its own record; twins share a title, so their pages match. */
+  /**
+   * A pending request on its own record. [#23594] The pages name the record by
+   * its id, never its title, so twins' pages differ by exactly that id.
+   */
   const openRequest = async (title = 'Annual leave') => {
     n += 1;
     const recordId = `LR${n}`;
@@ -350,8 +353,16 @@ describe('[#22578] parity — the member answers what the self-hosted mount answ
     const member = await snapshot(await svc.handleActionPage(postRequest(tokenForm(memberTokens.approve))));
     const mount = await snapshot(await app!.fetch(postRequest(tokenForm(mountTokens.approve))));
     expect(mount.status).toBe(200);
-    expect(member).toEqual(mount);
+    expect([member.status, member.headers]).toEqual([mount.status, mount.headers]);
+    // A token is single-use, so each door decides its own twin, and the twins'
+    // pages differ by their record ids alone (#23594: the page names the record
+    // by id). Byte-equal once that one difference is named.
+    const memberRecord = (await svc.getRequest(viaMember, SYSTEM))!.record_id;
+    const mountRecord = (await svc.getRequest(viaMount, SYSTEM))!.record_id;
+    expect(memberRecord).not.toBe(mountRecord);
+    expect(text(member).replaceAll(memberRecord, 'RECORD')).toBe(text(mount).replaceAll(mountRecord, 'RECORD'));
     expect(text(member)).toBe(renderResultPage('approved', (await svc.getRequest(viaMember, SYSTEM))!));
+    expect(text(mount)).toBe(renderResultPage('approved', (await svc.getRequest(viaMount, SYSTEM))!));
 
     for (const requestId of [viaMember, viaMount]) {
       expect(await statusOf(requestId)).toBe('approved');

packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts

@@ -204,10 +204,11 @@ describe('opening an approval step tells each resolved approver, once (approval.
   });
 
   it('[#23594] a record whose title resolves is still named by label and id — no title reaches the body', async () => {
-    // The inbox's subject title is not yet narrowed by its reader's field-level
-    // security (#23594), and notification text is stored and may be e-mailed,
-    // so the body withholds the title from EVERY source: the live record's and
-    // the submit snapshot's (which differ here, so either leak is told apart).
+    // The inbox narrows the subject title by its reader's field-level security
+    // (#23594), but a notification has no reader to narrow for, and its text is
+    // stored and may be e-mailed, so the body withholds the title from EVERY
+    // source: the live record's and the submit snapshot's (which differ here,
+    // so either leak is told apart).
     const LIVE_TITLE = 'Acme renewal';
     const SNAPSHOT_TITLE = 'Snapshot-only title';
     await engine.insert('crm_deal', { id: 'D_titled', title: LIVE_TITLE }, { context: SYSTEM } as any);

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

@@ -1533,9 +1533,38 @@ export class ApprovalService implements IApprovalService {
     this.fieldVisibility = source;
   }
 
+  /**
+   * [#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.
+   *
+   * It is the ONE answer both serve-time narrowings of a row read: the payload
+   * snapshot ({@link ApprovalService.redactPayloads}) and the subject's title
+   * ({@link ApprovalService.resolveSubjectDisplays}). One ask per object covers
+   * a whole page of same-object requests, and the two can never disagree about
+   * what a reader may see. `undefined` for an object is that seam's deliberate
+   * pass-through, and it passes the title through exactly as it passes the
+   * snapshot.
+   */
+  private async 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 ?? '');
+      if (served.has(object)) continue;
+      served.set(object, await resolveReadableSnapshotFields(
+        this.fieldVisibility, object, context, this.logger,
+      ));
+    }
+    return served;
+  }
+
   /**
    * [#10749] Redact each row's payload snapshot down to the fields the READING
-   * caller may see on that row's subject object.
+   * caller may see on that row's subject object — `served`, from
+   * {@link ApprovalService.servedSubjectFields}.
    *
    * Runs BEFORE {@link ApprovalService.enrichRows}, and that ordering is
    * load-bearing rather than incidental: `enrichRows` derives `payload_display`
@@ -1544,30 +1573,15 @@ export class ApprovalService implements IApprovalService {
    * SNAPSHOT'S OWN KEYS. Redact first and both derived maps are clean for free;
    * redact after and a restricted field's name, its authored label and the
    * title of the record it points at all still ship — the value would be gone
-   * and the disclosure would not.
-   *
-   * Rows are grouped by subject object so one `getReadableFields` call covers a
-   * whole page of same-object requests.
+   * and the disclosure would not. The snapshot is also where the title falls
+   * back to for a deleted subject, so that fallback is narrowed here too.
    */
-  private async redactPayloads(rows: ApprovalRequestRow[], context: ExecutionContext): Promise<void> {
-    const withPayload = rows.filter((r: any) => r?.payload != null);
-    if (withPayload.length === 0) return;
-    const byObject = new Map<string, ApprovalRequestRow[]>();
-    for (const r of withPayload) {
-      const key = String((r as any).object_name ?? '');
-      let list = byObject.get(key);
-      if (!list) { list = []; byObject.set(key, list); }
-      list.push(r);
-    }
-    for (const [object, group] of byObject) {
-      const readable = await resolveReadableSnapshotFields(
-        this.fieldVisibility, object, context, this.logger,
-      );
+  private redactPayloads(rows: ApprovalRequestRow[], served: ReadonlyMap<string, string[] | undefined>): void {
+    for (const r of rows as any[]) {
+      if (r?.payload == null) continue;
+      const readable = served.get(String(r.object_name ?? ''));
       if (readable === undefined) continue;
-      for (const r of group as any[]) {
-        const { payload } = redactSnapshot(r.payload, readable);
-        r.payload = payload;
-      }
+      r.payload = redactSnapshot(r.payload, readable).payload;
     }
   }
 
@@ -6706,15 +6720,25 @@ export class ApprovalService implements IApprovalService {
    * Batched: one read per object. Best-effort — an unregistered object or a
    * failed read leaves its entries unset.
    *
-   * [#23594] The read runs under the system context, so no reader's
-   * field-level security narrows the title it finds. Narrowing it per reader
-   * is a change to THIS function — the one place the inbox's subject title is
-   * resolved. The `approval.requested` body ({@link ApprovalService.nameSubjectRecord})
-   * withholds the title until then, because notification text is stored and
-   * may be e-mailed, so a title sent cannot be withdrawn by a later fix.
+   * [#23594] Narrowed by the READER, with the answer that narrows the payload
+   * snapshot (`served`, {@link ApprovalService.servedSubjectFields}). The record
+   * is read under the system context — the approver routinely has no read on
+   * the subject object at all (#7497 ruling C: record access stays with object
+   * permissions, sharing and row-level rules) — so the read itself narrows
+   * nothing. The title is therefore picked from the record AS SERVED to this
+   * reader: `redactSnapshot` keeps exactly the fields the snapshot keeps, the
+   * snapshot fallback already picks from the redacted snapshot, and a display
+   * field the reader may not read, or is served masked, gives no title — the
+   * shape the redaction gives that field in the payload. A field the reader
+   * may be served still names the record. This narrows only; nothing here
+   * grants a reader anything.
+   *
+   * The `approval.requested` body ({@link ApprovalService.nameSubjectRecord})
+   * does not read this: it has no reader to narrow for.
    */
   private async resolveSubjectDisplays(
     byObject: ReadonlyMap<string, ReadonlySet<string>>,
+    served: ReadonlyMap<string, string[] | undefined>,
   ): Promise<{ titles: Map<string, string>; objectLabels: Map<string, string> }> {
     const titles = new Map<string, string>();
     const objectLabels = new Map<string, string>();
@@ -6723,12 +6747,14 @@ export class ApprovalService implements IApprovalService {
       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 title = ApprovalService.pickTitle(rec, displayField);
+          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);
         }
       } catch { /* object may be unregistered — the caller's fallback applies */ }
@@ -6776,8 +6802,13 @@ export class ApprovalService implements IApprovalService {
    * payload snapshot, and any failure leaves the field unset rather than
    * failing the list. [#22738] A referenced object whose declared exposure
    * 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}).
    */
-  private async enrichRows(rows: ApprovalRequestRow[]): Promise<void> {
+  private async enrichRows(
+    rows: ApprovalRequestRow[],
+    served: ReadonlyMap<string, string[] | undefined>,
+  ): Promise<void> {
     if (!rows.length) return;
 
     // Record titles + object labels, batched per object.
@@ -6788,7 +6819,7 @@ export class ApprovalService implements IApprovalService {
       if (!set) { set = new Set(); byObject.set(r.object_name, set); }
       set.add(r.record_id);
     }
-    const { titles, objectLabels } = await this.resolveSubjectDisplays(byObject);
+    const { titles, objectLabels } = await this.resolveSubjectDisplays(byObject, served);
 
     // Lookup foreign keys inside payload snapshots → referenced record titles.
     const lookupFieldsByObject = new Map<string, Array<{ key: string; reference: string }>>();
@@ -7495,9 +7526,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`.
-    await this.redactPayloads(list, context);
-    await this.enrichRows(list);
+    // [#10749] Redact before enrichment — see `redactPayloads`. [#23594] The
+    // snapshot and the subject title are narrowed by ONE answer for the reader.
+    const served = await this.servedSubjectFields(list, context);
+    this.redactPayloads(list, served);
+    await this.enrichRows(list, served);
     this.attachViewers(list, context, caller);
     return list;
   }
@@ -7606,9 +7639,11 @@ export class ApprovalService implements IApprovalService {
       if (visible && !visible.has(String(raw.id))) return null;
     }
     const row = rowFromRequest(raw);
-    // [#10749] Redact before enrichment — see `redactPayloads`.
-    await this.redactPayloads([row], context);
-    await this.enrichRows([row]);
+    // [#10749] Redact before enrichment — see `redactPayloads`. [#23594] The
+    // snapshot and the subject title are narrowed by ONE answer for the reader.
+    const served = await this.servedSubjectFields([row], context);
+    this.redactPayloads([row], served);
+    await this.enrichRows([row], served);
     await this.attachFlowSteps(row);
     await this.attachDecisionProgress(row, raw);
     this.attachViewers([row], context, caller);

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

@@ -73,8 +73,12 @@ 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.
   const enrich = (rows: InboxRow[]) =>
-    (service as unknown as { enrichRows(r: InboxRow[]): Promise<void> }).enrichRows(rows);
+    (service as unknown as {
+      enrichRows(r: InboxRow[], served: ReadonlyMap<string, string[] | undefined>): Promise<void>;
+    }).enrichRows(rows, new Map());
   return { resolve, enrich, warn, finds };
 }
 

packages/plugins/plugin-approvals/src/payload-display-target-exposure.test.ts

@@ -60,7 +60,11 @@ function makeService(unreadable?: string) {
   const { engine, reads } = makeEngine(tables, unreadable);
   const service = new ApprovalService({ engine: engine as any, logger: { info() {}, warn, error() {}, debug() {} } });
   // `enrichRows` is private; its public callers drag the whole request lifecycle in.
-  const enrich = (rows: Row[]) => (service as unknown as { enrichRows(r: Row[]): Promise<void> }).enrichRows(rows);
+  // [#23594] No field-visibility answer for any object: the reader's served fields
+  // narrow nothing here, as with no security service wired.
+  const enrich = (rows: Row[]) => (service as unknown as {
+    enrichRows(r: Row[], served: ReadonlyMap<string, string[] | undefined>): Promise<void>;
+  }).enrichRows(rows, new Map());
   return { enrich, reads, warn };
 }