fix(plugin-approvals): a payload_display lookup target is titled only for a target record the reader may read

.changeset/23637-lookup-target-title-row-access.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): the inbox's referenced-record titles (`payload_display`) are served only for a referenced record the caller may read
+
+Clause-②: no
+
+- **Referenced-record titles follow the reader's access to the referenced RECORD.** `listRequests` and `getRequest` now title a lookup in the payload snapshot only when the reading caller may read the record it points at, asked through the platform's one caller-scoped row-access answer (`resolveReadableParentIds`, the same one the comment, activity, attachment and mail gates ask), once per referenced object per read. So the referenced object's object-level read, its sharing and its row-level rules decide, as they already do for the data API's `$expand`. A referenced record the reader may not read gets no display value and the snapshot's stored id stands, as it already does for a deleted referenced record. A reader who may read the record gets the same title as before, still picked from the fields they may be served. A referenced object the reader may not read at all is first skipped by the security service's object-level read admission (`canReadObject`), so an inbox read does not add an access-denied warning to the server log for it.
+- **Nothing about the record under approval changes.** Its title (`record_title`), its label and its field-level-security-redacted snapshot are served to a routed approver exactly as before, whatever their access to that record.
+
+A patch: a narrowing of served values only. Nothing is granted, and no schema, route, error code, translation key or export moves. With no security plugin composed, every caller-scoped read answers every record, so every title is served as before.

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

@@ -47,6 +47,8 @@ import type {
   ApprovalStatus,
   ApprovalCancelReason,
   ResumeFailureReport,
+  ISecurityService,
+  SecurityContext,
 } from '@objectstack/spec/contracts';
 // [#7135] The full `resolveAuthzContext` envelope — what `IApprovalService`
 // declares for every one of these context parameters since commit aa4b90d9a (the
@@ -64,7 +66,13 @@ import type { EnableLike, ObjectCapabilitiesParsed } from '@objectstack/spec/dat
 // mechanism for a second producer, and commit aa5994e17 landed this service's key
 // (`approval_recall_not_submitter`) into it ahead of this consumer half.
 import { renderOperationMessage, type ValidationMessageTranslator } from '@objectstack/spec/system';
-import { isGrantActive, omitInternalFieldsFromWriteResponse, resolveUserAuthzGrants } from '@objectstack/core';
+import {
+  isGrantActive,
+  omitInternalFieldsFromWriteResponse,
+  resolveReadableParentIds,
+  resolveUserAuthzGrants,
+  withoutOperationPrivateKeys,
+} from '@objectstack/core';
 import {
   filterApproversWhoCanRead,
   resolveApproverDirectoryOrg,
@@ -123,6 +131,22 @@ export interface ApprovalEngine {
   getSchema?(objectName: string): unknown;
 }
 
+/**
+ * [#23637] The single member of the security service's contract a lookup
+ * target's title consults before the target is read as the reader: the
+ * OBJECT-level read admission (`ISecurityService.canReadObject`, the engine
+ * middleware's own object-level read gate, arm for arm). Optional on the
+ * contract, so its absence is handled by construction.
+ */
+export type ObjectReadAdmission = Pick<ISecurityService, 'canReadObject'>;
+
+/**
+ * [#23637] Resolves the security service on every read: the security plugin
+ * may register after the approvals plugin, and a resolver captured once would
+ * pin "no service" for the life of the process.
+ */
+export type ObjectReadAdmissionSource = () => ObjectReadAdmission | null | undefined;
+
 export interface ApprovalClock { now(): Date }
 
 /**
@@ -1439,6 +1463,8 @@ export class ApprovalService implements IApprovalService {
   private publicBaseUrl: string;
   private tenancyPosture?: () => string | undefined;
   private fieldVisibility?: FieldVisibilitySource;
+  /** [#23637] Lazily-resolved object-level read admission ({@link attachObjectReadAdmission}). */
+  private objectReadAdmission?: ObjectReadAdmissionSource;
   /** [#11993] Lazily-resolved deployment i18n lookup for refusal copy. */
   private messageTranslator?: () => ValidationMessageTranslator | undefined;
   /**
@@ -1533,6 +1559,17 @@ export class ApprovalService implements IApprovalService {
     this.fieldVisibility = source;
   }
 
+  /**
+   * [#23637] Attach (or replace) the resolver of the security service whose
+   * object-level read admission (`canReadObject`) is asked before a lookup
+   * target is read as the reader ({@link ApprovalService.admittedTargetObjects}).
+   * Resolved on every read, as {@link attachFieldVisibility}'s source is: the
+   * security plugin may register after this one.
+   */
+  attachObjectReadAdmission(source: ObjectReadAdmissionSource | undefined): void {
+    this.objectReadAdmission = source;
+  }
+
   /**
    * [#10749 / #23594] The subject fields the READING caller may be served, per
    * subject object of `rows` — {@link ApprovalService.servedFields} for the
@@ -6692,6 +6729,8 @@ export class ApprovalService implements IApprovalService {
    * 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}).
+   * [#23637] And only for a target record the reader may read, asked as the
+   * reader after this gate ({@link ApprovalService.enrichRows}).
    *
    * 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
@@ -6711,6 +6750,54 @@ export class ApprovalService implements IApprovalService {
     return canServeApiOperation(enable, 'get');
   }
 
+  /**
+   * [#23637] The lookup-target objects of `objects` the reader may read at all,
+   * by the security service's object-level read admission, asked once per
+   * object before any of them is read as the reader
+   * ({@link ApprovalService.enrichRows}).
+   *
+   * A refused object is not read as the reader: its keys get no display value,
+   * exactly as its refused caller-scoped read would answer, and it logs no
+   * denial for a read the reader never asked for. ⛔ It is asked with the
+   * reader's own envelope (minus the operation-private keys, as
+   * `resolveReadableParentIds` sends it): no second derivation of authority,
+   * and no muting of the middleware's log.
+   *
+   * It only ever NARROWS what is read: an admitted object is still read as the
+   * reader, and that read still decides sharing, row-level rules and every gate
+   * after the object-level arms. Fails CLOSED: an admission that throws (said
+   * at `warn`, a fault rather than a denial), or answers anything but `true`,
+   * refuses that object. With no security service composed, or one without the
+   * optional `canReadObject`, every object is left to the caller-scoped read,
+   * which still decides, so the absence can never widen the answer.
+   */
+  private async admittedTargetObjects(objects: readonly string[], context: ExecutionContext): Promise<string[]> {
+    let admission: ObjectReadAdmission | null | undefined;
+    try {
+      admission = this.objectReadAdmission?.();
+    } catch {
+      admission = undefined;
+    }
+    const canReadObject = admission?.canReadObject;
+    if (typeof canReadObject !== 'function') return [...objects];
+    const envelope = withoutOperationPrivateKeys((context ?? {}) as Record<string, unknown>);
+    const admitted: string[] = [];
+    for (const object of objects) {
+      try {
+        // `=== true`: an answer that is not a plain yes refuses (fail closed).
+        if ((await canReadObject.call(admission, object, { ...envelope } as SecurityContext)) === true) {
+          admitted.push(object);
+        }
+      } catch (err: any) {
+        this.logger?.warn?.(
+          `[approvals] ApprovalService.enrichRows: the object-level read admission for "${object}" failed, `
+          + `so its titles are not resolved and the stored ids stand (fail-closed): ${err?.message ?? err}`,
+        );
+      }
+    }
+    return admitted;
+  }
+
   /**
    * Field key → display label for an object's schema. Lets the inbox summary
    * show a human field name ("考核状态") instead of a title-cased machine key
@@ -6786,8 +6873,10 @@ export class ApprovalService implements IApprovalService {
    * 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.
+   * row-level rules); only what is served from it is narrowed. Which `ids` are
+   * read is the caller's: the subject's are every request's record, the
+   * targets' only those the reader may read ([#23637], {@link enrichRows}).
+   * Throws what the read throws; each caller keeps its own best-effort catch.
    */
   private async titlesAsServed(
     object: string,
@@ -6859,6 +6948,18 @@ export class ApprovalService implements IApprovalService {
    * 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.
+   * [#23637] And only a target RECORD the reader may read is titled at all:
+   * the readable ids come from `resolveReadableParentIds` (`@objectstack/core`),
+   * the one caller-scoped "which of these records may this caller read" answer
+   * the parent-derived read gates ask, so the target object's object-level
+   * read, sharing and row-level rules decide, as they do for the data door's
+   * `$expand`. One ask per target object per read, of the objects the
+   * security service's object-level read admission admits
+   * ({@link ApprovalService.admittedTargetObjects}), so an object the reader may
+   * not read at all is not read as the reader. An unreadable target gets
+   * no display value and its stored id stands, as a deleted one does. The
+   * SUBJECT is not asked: its title and snapshot are served whatever the
+   * reader's access to the subject record (#7497 ruling C, #10749).
    */
   private async enrichRows(
     rows: ApprovalRequestRow[],
@@ -6901,11 +7002,24 @@ export class ApprovalService implements IApprovalService {
       }
     }
     const refTitles = new Map<string, string>();
-    const targets = Array.from(refIds.keys()).filter((object) => this.servesPayloadDisplayTarget(object));
+    const exposed = Array.from(refIds.keys()).filter((object) => this.servesPayloadDisplayTarget(object));
+    // [#23637] Only the target RECORDS the reader may read are titled: the
+    // platform's one caller-scoped row-access answer, one ask per target object
+    // per read, of the objects the reader may read at all. An object the reader
+    // may not read, a row its sharing or row-level rules hide, and a deleted
+    // record all come back absent, so that key gets no display value and the
+    // snapshot's stored id stands.
+    const admitted = await this.admittedTargetObjects(exposed, context);
+    const readableIds = await resolveReadableParentIds(
+      this.engine,
+      context,
+      new Map(admitted.map((object) => [object, refIds.get(object)!])),
+    );
+    const targets = admitted.filter((object) => (readableIds.get(object)?.size ?? 0) > 0);
     await this.servedFields(targets, context, served);
     for (const object of targets) {
       try {
-        await this.titlesAsServed(object, refIds.get(object)!, served.get(object), refTitles);
+        await this.titlesAsServed(object, readableIds.get(object)!, served.get(object), refTitles);
       } catch { /* referenced object unreadable — leave unresolved */ }
     }
 

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

@@ -21,6 +21,7 @@ import {
   ESCALATION_SCAN_INTERVAL_MS,
   type ApprovalEngine,
   type ApprovalMessagingSurface,
+  type ObjectReadAdmission,
 } from './approval-service.js';
 import {
   bindApprovalLockHook,
@@ -292,6 +293,19 @@ export class ApprovalsServicePlugin implements Plugin {
           : Promise.resolve(undefined);
       },
     });
+    // [#23637] The same service's object-level read admission (`canReadObject`),
+    // asked before a lookup target is read as the reader, so a target object
+    // the reader may not read at all is skipped without a logged denial
+    // (`ApprovalService.admittedTargetObjects`). Resolved on every read, as
+    // above; an absent service, or one without the member, leaves every
+    // target to the caller-scoped read.
+    this.service.attachObjectReadAdmission(() => {
+      try {
+        return ctx.getService<ObjectReadAdmission>('security');
+      } catch {
+        return undefined;
+      }
+    });
 
     // Record lock: block edits to a record while it has a pending request.
     // Record-delete cancel (#13568): void a record's pending requests when the

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

@@ -27,8 +27,8 @@ import { ApprovalService } from './approval-service.js';
 type FieldDefs = Record<string, unknown>;
 type Row = Record<string, unknown>;
 
-/** What the enrichment read, in the order it read it. */
-interface FindCall { object: string; ids: string[] }
+/** What the enrichment read, in the order it read it, and whether as the system. */
+interface FindCall { object: string; ids: string[]; system: boolean }
 
 /** The two engine members the enrichment path reads. */
 function makeEngine(
@@ -38,9 +38,9 @@ function makeEngine(
   const finds: FindCall[] = [];
   const engine = {
     getSchema: (object: string) => schemas[object],
-    async find(object: string, options?: { where?: { id?: { $in?: string[] } } }) {
+    async find(object: string, options?: { where?: { id?: { $in?: string[] } }; context?: { isSystem?: boolean } }) {
       const wanted = options?.where?.id?.$in;
-      finds.push({ object, ids: wanted ?? [] });
+      finds.push({ object, ids: wanted ?? [], system: options?.context?.isSystem === true });
       const rows = tables[object] ?? [];
       return wanted ? rows.filter(r => wanted.includes(String(r.id))) : rows;
     },
@@ -155,9 +155,13 @@ describe('inbox display enrichment — the previously silent path', () => {
     // The read the silent path never issued. `assignee` is this object's ONLY
     // reference field, so before the repair the enrichment asked `sys_user`
     // nothing at all — this is the half a happy-value assertion cannot see.
+    // [#23637] Two reads of it: the reader's row-access ask for the target
+    // record, then the title read of the record the reader may read.
     const userReads = finds.filter(f => f.object === 'sys_user');
-    expect(userReads).toHaveLength(1);
-    expect(userReads[0]?.ids).toEqual(['u7']);
+    expect(userReads).toEqual([
+      { object: 'sys_user', ids: ['u7'], system: false },
+      { object: 'sys_user', ids: ['u7'], system: true },
+    ]);
 
     // …and the value the reviewer actually sees, instead of the raw id.
     expect(rows[0]?.payload_display).toEqual({ assignee: 'Grace Hopper' });