fix(plugin-approvals, plugin-audit)!: a lookup title is served only for a target whose declared exposure serves get

.changeset/22738-lookup-title-exposure.md

@@ -0,0 +1,29 @@
+---
+'@objectstack/plugin-approvals': minor
+'@objectstack/plugin-audit': minor
+---
+
+fix(plugin-approvals,plugin-audit)!: a lookup target's title is read only when the TARGET object's declared exposure serves `get` — the approvals inbox's `payload_display` and the activity summary (#22738)
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is two served titles: each stops naming a lookup's target object whose existing `enable` declaration already refuses `get` on every data route, and the remedy is the declaration the author already wrote. The other categories are closed on facts: both packages publish (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no exported declaration is removed or narrowed (not runtime-interface-only or type-surface-only). -->
+
+**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
+
+The data routes judge an object's `enable` block through the spec's one exposure decision (`apiExposureDenialReason` / `canServeApiOperation`), and the data door's `$expand` and the dataset door's labels already ask it of a lookup's TARGET. Two more reads follow a lookup to its target's title under a system context and never asked it.
+
+**FROM.** For an administrator and a member alike:
+
+- **Approvals inbox** (`@objectstack/plugin-approvals`): `GET /api/v1/approvals/requests` and `GET /api/v1/approvals/requests/:id` carried, in `payload_display`, the title of a snapshot lookup's target whose declaration refuses `get`.
+- **Activity summary** (`@objectstack/plugin-audit`): an update's tracked-change summary, and a fired milestone's summary, named such a target's records by title in the `sys_activity` row served by `GET /api/v1/data/sys_activity`.
+
+**TO.** Each title read first asks the decision of the TARGET object, for `get` (turning an id into the record it names, the read `GET /api/v1/data/:target/:id` performs):
+
+- A refused target is not read. The inbox carries no `payload_display` entry for that key, so the snapshot's stored id stands, which is what a deleted target already answers. The activity summary names the record by its stored id, which is what an unresolvable reference already answers.
+- A declaration that cannot be read withholds the title too (fail-closed, at `warn`).
+- Only activity rows written from now on change. A written row is a snapshot and is never rewritten, and no other activity column changes.
+
+A target declaring `apiEnabled: false`, the deny-all `apiMethods: []`, or a whitelist without `get` (for example `['list']`) is withheld; a target with no `enable` block, or a whitelist that grants `get`, is served exactly as before. The decision takes no caller.
+
+**Measured producers.** Read on `origin/main` `bf515e724d` over every non-test `.ts` source under `packages/` and `examples/`: ten objects refuse `get` by declaration, and exactly one lookup points into any of them, from an object that is itself `apiEnabled: false` (control: 88 lookups into `sys_user`). So no shipped object's served title changes. Deployed and cloud-held object definitions were NOT MEASURED.

packages/core/src/security/second-object-read-exposure.pin.test.ts

@@ -141,16 +141,20 @@ const CALLERS: Record<string, Classification> = {
       'provenance row for the door and touches a filter position; carried by a follow-up of #22661',
   },
   'packages/plugins/plugin-approvals/src/approval-service.ts': {
-    kind: 'open',
+    kind: 'decided',
     read: "the approvals inbox's `payload_display` (`ApprovalService.enrichRows` reads referenced records' titles under a system context)",
-    measured: "an unexposed target's title is served on the inbox list, to an administrator and a member",
-    carrier: 'outside the card that took the census (another package and lane); carried by a follow-up of #22661',
+    operation: 'get',
+    decision: { file: 'packages/plugins/plugin-approvals/src/approval-service.ts', fn: 'servesPayloadDisplayTarget' },
+    wiring: { file: 'packages/plugins/plugin-approvals/src/approval-service.ts', call: 'this.servesPayloadDisplayTarget(' },
+    pin: 'packages/plugins/plugin-approvals/src/payload-display-target-exposure.test.ts',
   },
   'packages/plugins/plugin-audit/src/audit-writers.ts': {
-    kind: 'open',
-    read: "the activity timeline's tracked-change summary (`resolveLookupTitles` writes referenced records' titles at write time)",
-    measured: "an unexposed target's title is served in the summary on the activity read, to an administrator and a member",
-    carrier: 'outside the card that took the census (another package and lane); carried by a follow-up of #22661',
+    kind: 'decided',
+    read: "the activity timeline's tracked-change and milestone summaries (`resolveLookupTitles` writes referenced records' titles at write time)",
+    operation: 'get',
+    decision: { file: 'packages/plugins/plugin-audit/src/audit-writers.ts', fn: 'servesSummaryTitleTarget' },
+    wiring: { file: 'packages/plugins/plugin-audit/src/audit-writers.ts', call: 'servesSummaryTitleTarget(objectName' },
+    pin: 'packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts',
   },
   'packages/lint/src/object-graph.ts': {
     kind: 'not-a-served-read',
@@ -280,7 +284,12 @@ describe('[#22661] every read that reaches a second object asks that object its
       const decided = Object.values(CALLERS).filter(
         (c): c is Extract<Classification, { kind: 'decided' }> => c.kind === 'decided',
       );
-      expect(decided.map((d) => d.decision.fn).sort()).toEqual(['servesExpansionTarget', 'servesLabelTarget']);
+      expect(decided.map((d) => d.decision.fn).sort()).toEqual([
+        'servesExpansionTarget',
+        'servesLabelTarget',
+        'servesPayloadDisplayTarget',
+        'servesSummaryTitleTarget',
+      ]);
 
       for (const d of decided) {
         const body = functionBody(d.decision.file, d.decision.fn);

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

@@ -56,8 +56,8 @@ import type {
 // fields the caller had already supplied.
 import type { ExecutionContext } from '@objectstack/spec/kernel';
 import { RESUME_AUTHORITY_SERVICE } from '@objectstack/spec/contracts';
-import { isFileIdToken, referenceTargetOf } from '@objectstack/spec/data';
-import type { ObjectCapabilitiesParsed } from '@objectstack/spec/data';
+import { canServeApiOperation, isFileIdToken, referenceTargetOf } from '@objectstack/spec/data';
+import type { EnableLike, ObjectCapabilitiesParsed } from '@objectstack/spec/data';
 // [#11993] The SANCTIONED renderer for OPERATION-level refusal copy. The
 // Operation Message Catalog is the ONE seat for these sentences — its own
 // header bars both a package-local string table and a second rendering
@@ -6391,6 +6391,31 @@ export class ApprovalService implements IApprovalService {
     } catch { return []; }
   }
 
+  /**
+   * [#22738] May `payload_display` carry a title read from `target`, the object
+   * a snapshot's lookup points at? Asked of the spec's one exposure decision
+   * (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.
+   *
+   * 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
+   * declaration that cannot be read withholds too (fail-closed, at `warn`).
+   */
+  private servesPayloadDisplayTarget(target: string): boolean {
+    let enable: EnableLike | null | undefined;
+    try {
+      enable = (this.engine as any).getSchema?.(target)?.enable;
+    } catch (err: any) {
+      this.logger?.warn?.(
+        `[approvals] ApprovalService.enrichRows: the API exposure declaration of "${target}" could not be read, `
+        + `so its titles are not resolved and the stored ids stand (fail-closed): ${err?.message ?? err}`,
+      );
+      return false;
+    }
+    return canServeApiOperation(enable, 'get');
+  }
+
   /**
    * 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
@@ -6419,7 +6444,8 @@ export class ApprovalService implements IApprovalService {
    * Batched: one query per distinct object (target + referenced) plus one
    * `sys_user` lookup. Best-effort — a deleted record falls back to the
    * payload snapshot, and any failure leaves the field unset rather than
-   * failing the list.
+   * failing the list. [#22738] A referenced object whose declared exposure
+   * does not serve `get` is not read ({@link servesPayloadDisplayTarget}).
    */
   private async enrichRows(rows: ApprovalRequestRow[]): Promise<void> {
     if (!rows.length) return;
@@ -6477,6 +6503,7 @@ 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);
       try {

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

@@ -0,0 +1,96 @@
+// Copyright (c) 2026 ObjectStack contributors. Apache-2.0 license.
+//
+// [#22738] The inbox's `payload_display` reads a referenced record's title only
+// from a target whose declared exposure (ADR-0049) serves `get`, asked through
+// the spec's one decision. A refused target is NOT READ, so its key carries no
+// display value and the snapshot's stored id stands; a declaration that cannot
+// be read withholds too, and says so at `warn`. The exposed targets are the
+// control. The read is asserted as well as the value: "not served" could
+// otherwise be met by reading the title and dropping it.
+
+import { describe, it, expect, vi } from 'vitest';
+import { ApprovalService } from './approval-service.js';
+
+type Row = Record<string, unknown>;
+type Schema = { label?: string; enable?: Record<string, unknown>; fields: Record<string, unknown> };
+
+/** One target per shape the decision tells apart; `open` declares no `enable` block. */
+const TARGETS: Record<string, Record<string, unknown> | undefined> = {
+  hidden: { apiEnabled: false },
+  listonly: { apiMethods: ['list'] },
+  denyall: { apiMethods: [] },
+  getonly: { apiMethods: ['get'] },
+  open: undefined,
+};
+const REFUSED = ['hidden', 'listonly', 'denyall'];
+const SERVED = ['getonly', 'open'];
+
+const schemas: Record<string, Schema> = {
+  deal: {
+    label: 'Deal',
+    fields: { name: {}, ...Object.fromEntries(Object.keys(TARGETS).map((k) => [k, { type: 'lookup', reference: `t_${k}` }])) },
+  },
+  ...Object.fromEntries(Object.entries(TARGETS).map(([k, enable]) => [`t_${k}`, { fields: { name: {} }, ...(enable ? { enable } : {}) }])),
+};
+const tables: Record<string, Row[]> = {
+  deal: [{ id: 'd1', name: 'Deal one' }],
+  ...Object.fromEntries(Object.keys(TARGETS).map((k) => [`t_${k}`, [{ id: `${k}_1`, name: `${k} title` }]])),
+};
+
+/** The two engine members the enrichment reads; `unreadable` names a target whose declaration throws. */
+function makeEngine(rows: Record<string, Row[]>, unreadable?: string) {
+  const reads: string[] = [];
+  const engine = {
+    getSchema(object: string) {
+      if (object === unreadable) throw new Error('declaration unavailable');
+      return schemas[object];
+    },
+    // Each table holds the one row a read can ask for, so the double answers the
+    // table whole: what is pinned is WHICH objects are read, not the query.
+    async find(object: string) {
+      reads.push(object);
+      return rows[object] ?? [];
+    },
+  };
+  return { engine, reads };
+}
+
+function makeService(unreadable?: string) {
+  const warn = vi.fn();
+  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);
+  return { enrich, reads, warn };
+}
+
+const inboxRow = (): Row => ({
+  object_name: 'deal',
+  record_id: 'd1',
+  payload: { name: 'Deal one', ...Object.fromEntries(Object.keys(TARGETS).map((k) => [k, `${k}_1`])) },
+});
+
+describe('[#22738] payload_display serves a referenced title only for a target whose exposure serves `get`', () => {
+  it('does not read a refused target; its stored id stands, and the served targets keep their titles', async () => {
+    const { enrich, reads } = makeService();
+    const row = inboxRow();
+    await enrich([row]);
+
+    expect(row.payload_display).toEqual(Object.fromEntries(SERVED.map((k) => [k, `${k} title`])));
+    for (const k of REFUSED) {
+      expect(reads, `t_${k} was read`).not.toContain(`t_${k}`);
+      expect((row.payload as Row)[k]).toBe(`${k}_1`);
+    }
+    for (const k of SERVED) expect(reads).toContain(`t_${k}`);
+  });
+
+  it('withholds a target whose declaration cannot be read (fail-closed), and says so at warn', async () => {
+    const { enrich, reads, warn } = makeService('t_open');
+    const row = inboxRow();
+    await enrich([row]);
+
+    expect(row.payload_display).toEqual({ getonly: 'getonly title' });
+    expect(reads).not.toContain('t_open');
+    expect(warn.mock.calls.some(([message]) => String(message).includes('"t_open"'))).toBe(true);
+  });
+});

packages/plugins/plugin-audit/src/audit-lookup-summary.test.ts

@@ -47,7 +47,7 @@
  * separately to confirm it goes red on its own defect.
  */
 
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, vi } from 'vitest';
 import { ObjectQL } from '@objectstack/objectql';
 import { createMemoryI18n } from '@objectstack/core';
 import { installAuditWriters } from './audit-writers.js';
@@ -520,3 +520,81 @@ describe('[#7230] the resolution is batched per target object, and free when unu
     expect(lastSummary(storeFor)).toBe('Account: Acme Corp → Globex');
   });
 });
+
+// ---------------------------------------------------------------------------
+// 4. [#22738] Exposure — a title is read only from a target that serves `get`
+// ---------------------------------------------------------------------------
+
+/**
+ * One target per shape the spec's exposure decision tells apart (ADR-0049):
+ * the off switch, a whitelist without `get`, a deny-all whitelist, and a
+ * whitelist that grants `get`; `crm_account` (no `enable` block) is the open
+ * control. `lte_deal` tracks a lookup into each, keyed by the target's name.
+ */
+const exposureTarget = (name: string, enable: Record<string, unknown>) => ({
+  name, label: name, enable,
+  fields: { id: f('id', 'text', { primaryKey: true }), name: f('name', 'text') },
+});
+const REFUSED = ['lte_hidden', 'lte_listonly', 'lte_denyall'] as const;
+const exposureTargets = [
+  exposureTarget('lte_hidden', { apiEnabled: false }),
+  exposureTarget('lte_listonly', { apiMethods: ['list'] }),
+  exposureTarget('lte_denyall', { apiMethods: [] }),
+  exposureTarget('lte_getonly', { apiMethods: ['get'] }),
+];
+const lteDeal = {
+  name: 'lte_deal', label: 'Deal',
+  fields: {
+    id: f('id', 'text', { primaryKey: true }),
+    title: f('title', 'text'),
+    ...Object.fromEntries([...exposureTargets.map((t) => t.name), 'crm_account'].map((t) => [
+      t, f(t, 'lookup', { label: t, reference: t, trackHistory: true }),
+    ])),
+  },
+};
+
+async function bootExposure() {
+  const booted = await boot();
+  for (const o of [...exposureTargets, lteDeal]) booted.engine.registry.registerObject(o as any, OWNER_PACKAGE);
+  await booted.engine.insert('crm_account', { id: 'acc_1', name: 'Acme Corp' });
+  for (const t of exposureTargets) await booted.engine.insert(t.name, { id: `${t.name}_1`, name: `${t.name} title` });
+  await booted.engine.insert('lte_deal', { id: 'd1', title: 'Deal' });
+  return booted;
+}
+
+describe('[#22738] a tracked reference into a target whose exposure refuses `get` is named by its stored id', () => {
+  it('reads and titles only the targets that serve `get`; a refused target is not read', async () => {
+    const { engine, reads, storeFor } = await bootExposure();
+    const before = { ...reads.findOn };
+    await engine.update(
+      'lte_deal',
+      Object.fromEntries([...exposureTargets.map((t) => [t.name, `${t.name}_1`]), ['crm_account', 'acc_1']]),
+      { where: { id: 'd1' } } as any,
+    );
+
+    const summary = String(lastSummary(storeFor));
+    for (const t of REFUSED) {
+      expect(summary).toContain(`${t}: ∅ → ${t}_1`);
+      expect(summary).not.toContain(`${t} title`);
+      expect((reads.findOn[t] ?? 0) - (before[t] ?? 0), `${t} was read`).toBe(0);
+    }
+    expect(summary).toContain('lte_getonly: ∅ → lte_getonly title');
+    expect(summary).toContain('crm_account: ∅ → Acme Corp');
+  });
+
+  it('withholds a target whose declaration cannot be read (fail-closed), and says so at warn', async () => {
+    const { engine, reads, storeFor } = await bootExposure();
+    const getSchema = engine.getSchema.bind(engine);
+    (engine as any).getSchema = (name: string) => {
+      if (name === 'crm_account') throw new Error('declaration unavailable');
+      return getSchema(name);
+    };
+    const warn = vi.spyOn((engine as any).logger, 'warn');
+    const before = reads.findOn['crm_account'] ?? 0;
+    await engine.update('lte_deal', { crm_account: 'acc_1' }, { where: { id: 'd1' } } as any);
+
+    expect(lastSummary(storeFor)).toBe('crm_account: ∅ → acc_1');
+    expect((reads.findOn['crm_account'] ?? 0) - before).toBe(0);
+    expect(warn.mock.calls.some(([message]) => String(message).includes('"crm_account"'))).toBe(true);
+  });
+});