fix(plugin-approvals): the generic data door serves sys_approval_action and sys_approval_approver rows by their request's visibility (#22589)

.changeset/22589-approval-sibling-read-gates.md

@@ -0,0 +1,20 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+fix(plugin-approvals): the generic data door serves `sys_approval_action` and `sys_approval_approver` rows by their request's visibility
+
+Clause-②: no
+
+A read of a request's two child tables through the generic data door (`find`, `findOne`, `count` and `aggregate`, so the REST data routes, the export route, the MCP data tool and every other engine read) now returns only the rows whose request the approvals door (`/api/v1/approvals/*`) serves the same caller:
+
+- `sys_approval_action`, the decision log (actor, decision, comment text);
+- `sys_approval_approver`, the pending-approver index.
+
+Before this, a deployment that granted read on either object served every row of the organization on that door, including the decision log and comment text of requests the caller does not take part in.
+
+- **One rule for both doors, and for all three tables.** The gates ask the approvals service the same question the `sys_approval_request` gate asks, keyed by the row's `request_id`: the request's submitter, its current approvers and those who acted on it see its rows; an administrator holding the approval override sees every row; and, read-only, on an object named in `recordReaderVisibleObjects`, a reader of the record sees its request's rows on a read that names the request (`request_id`) or the row (its id). The published `ApprovalService` type is unchanged.
+- **The record page's Timeline keeps its readers.** A request's Timeline (a list of `sys_approval_action` keyed on `request_id`) serves the rows `listActions` serves the same caller; a row read by id is served exactly when a decision attachment on it would be (`authorizeFileRead`, which now shares the gate's read of the row's request).
+- **Absence, not refusal.** A row the caller may not see answers a by-id read exactly as a missing one does, `404 RECORD_NOT_FOUND`. A list's `total` and a grouped count are narrowed like its rows. A failed visibility answer denies the read and logs a warning.
+- **Unchanged.** The approvals door, an administrator's view, and the approval engine's own reads. A deployment that grants no read on either object still answers `403`.
+- The record-reader tier no longer anchors on a row of either table, even when `recordReaderVisibleObjects` names it, as it already never anchored on `sys_approval_request`.

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

@@ -85,7 +85,7 @@ import {
   resolveReadableSnapshotFields,
   type FieldVisibilitySource,
 } from './payload-redaction.js';
-import type { RequestVisibilitySource } from './request-read-gate.js';
+import { APPROVAL_REQUEST_CHILD_OBJECTS, type RequestVisibilitySource } from './request-read-gate.js';
 
 /**
  * Node-era approval runtime (ADR-0019).
@@ -1298,6 +1298,26 @@ async function findRequestRow(
   return Array.isArray(rows) && rows[0] ? rows[0] : null;
 }
 
+/**
+ * [#22589] The request a row of one of its child tables (`object`, one of
+ * `APPROVAL_REQUEST_CHILD_OBJECTS`) belongs to, read as the system — the one
+ * read the approvals door's by-row rule (`ApprovalService.authorizeFileRead`)
+ * and a by-id visibility question about a child row (the read gate's source
+ * below) both take, so the two cannot disagree about which request a row is
+ * anchored on. `null` when the row does not exist or names no request.
+ */
+async function findChildRequestId(
+  engine: ApprovalEngine,
+  object: string,
+  rowId: string,
+): Promise<string | null> {
+  const rows = await engine.find(object, {
+    where: { id: rowId }, fields: ['request_id'], limit: 1, context: SYSTEM_CTX,
+  });
+  const requestId = (Array.isArray(rows) ? rows[0] : undefined)?.request_id;
+  return requestId != null && String(requestId) !== '' ? String(requestId) : null;
+}
+
 /**
  * [#22559] Each service's own request visibility, as the generic data door's
  * read gate on `sys_approval_request` asks it (`request-read-gate.ts`).
@@ -1382,6 +1402,12 @@ export class ApprovalService implements IApprovalService {
     // is a request loaded by id, which carries its own anchor (`loadRequest`).
     // Absent, the read is untargeted — the participant set alone, as for the
     // inbox.
+    //
+    // [#22589] The same answer serves the gates on a request's child tables
+    // (`sys_approval_action`, `sys_approval_approver`), which keep a row when
+    // its `request_id` is in it. There `target.row` names a child row loaded
+    // by id: the request it belongs to is the anchor, as `authorizeFileRead`
+    // reads it — `findChildRequestId`, shared with that method.
     REQUEST_VISIBILITY.set(this, {
       visibleRequestIdsFor: async (context, target) => {
         // `organizationId` is not on the envelope — see isOverrideActor().
@@ -1392,9 +1418,13 @@ export class ApprovalService implements IApprovalService {
         let anchor: { object?: string | null; recordId?: string | null } = {
           object: target?.object, recordId: target?.recordId,
         };
-        if (!(anchor.object && anchor.recordId) && target?.requestId) {
-          const raw = await findRequestRow(this.engine, target.requestId, tenantOrg);
-          anchor = raw ? { object: raw.object_name, recordId: raw.record_id } : {};
+        if (!(anchor.object && anchor.recordId)) {
+          const requestId = target?.requestId
+            || (target?.row ? await findChildRequestId(this.engine, target.row.object, target.row.id) : null);
+          if (requestId) {
+            const raw = await findRequestRow(this.engine, requestId, tenantOrg);
+            anchor = raw ? { object: raw.object_name, recordId: raw.record_id } : {};
+          }
         }
         return this.visibleRequestIds(context, tenantOrg, anchor);
       },
@@ -6859,7 +6889,10 @@ export class ApprovalService implements IApprovalService {
     // object it passes through the generic door's read gate, which asks this
     // very function about the row it names — a chain of requests about
     // requests, unbounded on a cycle. Fail closed: no tier on it.
-    if (object === 'sys_approval_request') return;
+    // [#22589] Nor is a row of one of its child tables, for the same reason:
+    // their gates ask this function about the request a row belongs to, whose
+    // anchor can be that very row.
+    if (object === 'sys_approval_request' || APPROVAL_REQUEST_CHILD_OBJECTS.includes(object)) return;
     // A tokenless/anonymous caller reads nothing. Fail closed, as above.
     const uid = context?.userId != null ? String(context.userId) : '';
     if (!uid) return;
@@ -7271,15 +7304,12 @@ export class ApprovalService implements IApprovalService {
   async authorizeFileRead(actionId: string, context: ExecutionContext): Promise<boolean> {
     if (!actionId) return false;
     try {
-      const rows = await this.engine.find('sys_approval_action', {
-        where: { id: actionId },
-        limit: 1,
-        context: SYSTEM_CTX,
-      });
-      const requestId = (Array.isArray(rows) ? rows[0] : undefined)?.request_id;
+      // [#22589] The row's request, read the way the generic door's gate on
+      // this table reads it for a by-id read — `findChildRequestId`, shared.
+      const requestId = await findChildRequestId(this.engine, 'sys_approval_action', actionId);
       if (!requestId) return false;
       // Same gate as listActions: visibility of the decision's parent request.
-      return !!(await this.getRequest(String(requestId), context));
+      return !!(await this.getRequest(requestId, context));
     } catch {
       return false;
     }

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

@@ -30,7 +30,7 @@ import {
 } from './lifecycle-hooks.js';
 import { bindSnapshotRedactionMiddleware } from './payload-redaction-middleware.js';
 import { bindSnapshotPredicateGuard } from './payload-predicate-guard.js';
-import { bindRequestReadGate } from './request-read-gate.js';
+import { bindRequestChildReadGates, bindRequestReadGate } from './request-read-gate.js';
 import type { FieldVisibilitySource } from './payload-redaction.js';
 import { registerApprovalNode, type ApprovalAutomationSurface } from './approval-node.js';
 import { backfillActionSlots } from './action-slot-backfill.js';
@@ -317,13 +317,18 @@ export class ApprovalsServicePlugin implements Plugin {
     // `disableAutoHooks`: that switch is the record lock's (a caller driving
     // the manual API), and a read through the generic door needs the gate
     // whichever API wrote the row.
+    // [#22589] …and its child tables, `sys_approval_action` (the decision log)
+    // and `sys_approval_approver` (the approver index): a row there is served
+    // only when its request is, by the same source.
     if (typeof (engine as any).registerMiddleware === 'function') {
-      bindRequestReadGate(engine as any, requestVisibilitySourceOf(this.service), ctx.logger);
+      const visibility = requestVisibilitySourceOf(this.service);
+      bindRequestReadGate(engine as any, visibility, ctx.logger);
+      bindRequestChildReadGates(engine as any, visibility, ctx.logger);
     } else {
       ctx.logger.warn(
-        'ApprovalsServicePlugin: the ObjectQL engine has no middleware seam — reads of sys_approval_request '
-        + 'through the generic data door are NOT narrowed to the approval requests the caller may see. '
-        + 'Grant no read on that object to non-administrators on this stack.',
+        'ApprovalsServicePlugin: the ObjectQL engine has no middleware seam — reads of sys_approval_request, '
+        + 'sys_approval_action and sys_approval_approver through the generic data door are NOT narrowed to the '
+        + 'approval requests the caller may see. Grant no read on those objects to non-administrators on this stack.',
       );
     }