From 67fd2c6beef32d36abcfca4c3c6822d660a07436 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 04:30:47 +0000 Subject: [PATCH 1/2] fix(plugin-approvals): the subject title is served from what its reader may read The inbox's record_title is narrowed by the reader's served subject fields, the one answer the payload snapshot is redacted by; the action-link pages, which have no reader to narrow for, name the record by its id. Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../plugin-approvals/src/action-link-pages.ts | 14 +- .../action-page-member.integration.test.ts | 15 +- ...requested-notification.integration.test.ts | 9 +- .../plugin-approvals/src/approval-service.ts | 111 ++++--- .../implicit-reference-target-lookup.test.ts | 6 +- .../payload-display-target-exposure.test.ts | 6 +- ...bject-title-reader-fls.integration.test.ts | 305 ++++++++++++++++++ 7 files changed, 419 insertions(+), 47 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/subject-title-reader-fls.integration.test.ts diff --git a/packages/plugins/plugin-approvals/src/action-link-pages.ts b/packages/plugins/plugin-approvals/src/action-link-pages.ts index 57322ade07..4123cfb893 100644 --- a/packages/plugins/plugin-approvals/src/action-link-pages.ts +++ b/packages/plugins/plugin-approvals/src/action-link-pages.ts @@ -48,11 +48,23 @@ function shell(title: string, body: string): string {
${body}
`; } +/** + * 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 || '—'], ]; diff --git a/packages/plugins/plugin-approvals/src/action-page-member.integration.test.ts b/packages/plugins/plugin-approvals/src/action-page-member.integration.test.ts index 47f4c39f0e..5bdfbdadf7 100644 --- a/packages/plugins/plugin-approvals/src/action-page-member.integration.test.ts +++ b/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('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'); diff --git a/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts index f31e96613f..bbdc0222b7 100644 --- a/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts +++ b/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); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index fd9c6431eb..daeac5d639 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/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> { + const served = new Map(); + 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 { - const withPayload = rows.filter((r: any) => r?.payload != null); - if (withPayload.length === 0) return; - const byObject = new Map(); - 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): 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>, + served: ReadonlyMap, ): Promise<{ titles: Map; objectLabels: Map }> { const titles = new Map(); const objectLabels = new Map(); @@ -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 { + private async enrichRows( + rows: ApprovalRequestRow[], + served: ReadonlyMap, + ): Promise { 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>(); @@ -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); diff --git a/packages/plugins/plugin-approvals/src/implicit-reference-target-lookup.test.ts b/packages/plugins/plugin-approvals/src/implicit-reference-target-lookup.test.ts index 3973afe723..ed3be977f6 100644 --- a/packages/plugins/plugin-approvals/src/implicit-reference-target-lookup.test.ts +++ b/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 }).enrichRows(rows); + (service as unknown as { + enrichRows(r: InboxRow[], served: ReadonlyMap): Promise; + }).enrichRows(rows, new Map()); return { resolve, enrich, warn, finds }; } diff --git a/packages/plugins/plugin-approvals/src/payload-display-target-exposure.test.ts b/packages/plugins/plugin-approvals/src/payload-display-target-exposure.test.ts index 6f2ba39fd0..94a2d3efca 100644 --- a/packages/plugins/plugin-approvals/src/payload-display-target-exposure.test.ts +++ b/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 }).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): Promise; + }).enrichRows(rows, new Map()); return { enrich, reads, warn }; } diff --git a/packages/plugins/plugin-approvals/src/subject-title-reader-fls.integration.test.ts b/packages/plugins/plugin-approvals/src/subject-title-reader-fls.integration.test.ts new file mode 100644 index 0000000000..6a2f65312c --- /dev/null +++ b/packages/plugins/plugin-approvals/src/subject-title-reader-fls.integration.test.ts @@ -0,0 +1,305 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23594] A request's subject title (`record_title`) is served to a reader + * only from fields that reader may be served. + * + * ## The contract + * + * `payload-redaction.ts` (#10749): "Redaction is applied at SERVE time, keyed + * on the reading caller, so the same row answers an admin with the whole + * snapshot and a restricted approver with only the fields they may read." + * #7497 ruling C keeps the approver's request detail "with the field snapshot + * redacted by field-level security". The title is a field of the same subject + * record, read beside that snapshot, so it answers to the same rule — the SAME + * answer, asked once: `resolveReadableSnapshotFields` (readable and not masked + * for this reader). The title is picked from the record AS SERVED to the + * reader, exactly as the snapshot fallback already picked it from the redacted + * snapshot, so a withheld field gives the reader nothing — the shape the + * redaction gives it in the payload. + * + * ## What this pins + * + * Driven through the real `ApprovalService` on a real `ObjectQL` over + * `SqlDriver` (better-sqlite3, in-memory), with a field-visibility source in + * place of `plugin-security`'s (the three answers it gives are its contract; + * `approval-payload-redaction.test.ts` holds the payload half): + * + * (a) a reader who may not read the display field gets no `record_title` on + * the single read and on the list, and a display field served MASKED to + * that reader is withheld the same way (#20964); + * (b) the same reader, with the subject record deleted and the snapshot + * carrying the title, still gets none; + * (c) the decision result row — what the success toast's + * `${result.request.record_title}` token reads — carries none for that + * reader; + * (d) the action-link page, reached by a token and read under the system + * context, names the record by its id for EVERY bearer; + * (e) the control: a reader who may read the field gets the title on the + * single read, the list and the decision result, live and from the + * snapshot; + * (f) a withheld display field never lends its value to the title: the + * title is picked from what the reader is served; + * (g) one rule, asked once: the title and the payload share ONE + * field-visibility answer per object per read. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ApprovalService } from './approval-service.js'; +import { ApprovalsServicePlugin } from './approvals-plugin.js'; +import type { FieldVisibilitySource } from './payload-redaction.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] } as any; +const asUser = (userId: string) => ({ userId, positions: [], permissions: [] }) as any; + +/** May not read the subject's display field. */ +const RESTRICTED = 'u_restricted'; +/** May read every field (the control). */ +const READER = 'u_reader'; +/** Served the display field MASKED: readable, not queryable (#20964). */ +const MASKED = 'u_masked'; +const SUBMITTER = 'u_sub'; + +/** The display value a restricted reader must never be served, from any source. */ +const LIVE_TITLE = 'Acme secret renewal'; +const SNAPSHOT_TITLE = 'Snapshot secret title'; + +const deal = { + name: 'crm_deal', + label: 'Deal', + nameField: 'name', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + amount: { name: 'amount', label: 'Amount', type: 'number' as const }, + }, +}; +/** Its display field is `code`; `subject` is another title-eligible field. */ +const ticket = { + name: 'crm_ticket', + label: 'Ticket', + nameField: 'code', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + code: { name: 'code', label: 'Code', type: 'text' as const }, + subject: { name: 'subject', label: 'Subject', type: 'text' as const }, + }, +}; +const sysUser = { + name: 'sys_user', + label: 'User', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + email: { name: 'email', label: 'Email', type: 'text' as const }, + }, +}; + +const ALL: Record = { + crm_deal: ['id', 'name', 'amount'], + crm_ticket: ['id', 'code', 'subject'], +}; +/** reader → object → [readable, queryable]. */ +const ANSWERS: Record> = { + [RESTRICTED]: { + crm_deal: [['id', 'amount'], ['id', 'amount']], + crm_ticket: [['id', 'subject'], ['id', 'subject']], + }, + [MASKED]: { + crm_deal: [['id', 'name', 'amount'], ['id', 'amount']], + }, + [READER]: { + crm_deal: [ALL.crm_deal, ALL.crm_deal], + crm_ticket: [ALL.crm_ticket, ALL.crm_ticket], + }, +}; + +/** Stands in for `plugin-security`: the system context reads every field, as its `isSystem` bypass does. */ +function makeVisibility(): FieldVisibilitySource & { calls: Array<{ object: string; userId?: string }> } { + const calls: Array<{ object: string; userId?: string }> = []; + const answer = (object: string, context: any, which: 0 | 1) => { + if (context?.isSystem) return ALL[object]; + return ANSWERS[String(context?.userId ?? '')]?.[object]?.[which] ?? []; + }; + return { + calls, + async getReadableFields(object: string, context?: any) { + calls.push({ object, userId: context?.userId }); + return answer(object, context, 0); + }, + async getQueryableFields(object: string, context?: any) { + return answer(object, context, 1); + }, + }; +} + +/** The plugin's own object definitions, as its manifest registers them. */ +async function packageObjects(): Promise { + let manifest: any; + const ctx: any = { + getService: (name: string) => { + if (name === 'manifest') return { register: (m: any) => { manifest = m; } }; + throw new Error(`no service '${name}'`); + }, + logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }, + }; + await new ApprovalsServicePlugin().init(ctx); + return manifest.objects; +} + +describe('[#23594] the subject title is served from what its reader may read', () => { + let engine: ObjectQL; + let svc: ApprovalService; + let vis: ReturnType; + let n = 0; + + /** One pending request on a fresh deal, both approvers on its slate. */ + const openDeal = async (snapshot: Record = {}) => { + n += 1; + const recordId = `D${n}`; + await engine.insert('crm_deal', { id: recordId, name: LIVE_TITLE, amount: 100 * n }, { context: SYSTEM } as any); + const row: any = await svc.openNodeRequest({ + object: 'crm_deal', recordId, runId: `run_${n}`, nodeId: 'review', flowName: 'deal_review', + config: { + approvers: [RESTRICTED, MASKED, READER].map((value) => ({ type: 'user', value })), + behavior: 'first_response', + } as any, + submitterId: SUBMITTER, record: { id: recordId, name: LIVE_TITLE, amount: 100 * n, ...snapshot }, + }, asUser(SUBMITTER)); + return { requestId: String(row.id), recordId }; + }; + + beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + }), true); + await engine.init(); + for (const def of [sysUser, deal, ticket, ...(await packageObjects())]) { + engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); + } + await engine.syncSchemas(); + for (const id of [RESTRICTED, READER, MASKED, SUBMITTER]) { + await engine.insert('sys_user', { id, name: id, email: `${id}@example.com` }, { context: SYSTEM } as any); + } + svc = new ApprovalService({ + engine: engine as any, + logger: { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} } as any, + }); + vis = makeVisibility(); + svc.attachFieldVisibility(vis); + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('(a) a reader who may not read the display field gets no title — single read and list', async () => { + const { requestId, recordId } = await openDeal(); + + const one: any = await svc.getRequest(requestId, asUser(RESTRICTED)); + const [listed]: any[] = await svc.listRequests({ recordId }, asUser(RESTRICTED)); + for (const [door, row] of [['getRequest', one], ['listRequests', listed]] as const) { + expect(row, door).toBeTruthy(); + expect(row, door).not.toHaveProperty('record_title'); + expect(JSON.stringify(row), door).not.toContain(LIVE_TITLE); + // The same answer narrows the snapshot: the business field still serves. + expect(row.payload, door).toEqual({ id: recordId, amount: expect.any(Number) }); + expect(row.object_label, door).toBe('Deal'); + } + }); + + it('(a) a display field served MASKED to the reader is withheld as the snapshot withholds it (#20964)', async () => { + const { requestId } = await openDeal(); + const row: any = await svc.getRequest(requestId, asUser(MASKED)); + expect(row).not.toHaveProperty('record_title'); + expect(JSON.stringify(row)).not.toContain(LIVE_TITLE); + }); + + it('(b) the subject deleted, the snapshot carrying the title: the restricted reader still gets none', async () => { + const { requestId, recordId } = await openDeal({ name: SNAPSHOT_TITLE }); + await engine.delete('crm_deal', { where: { id: recordId }, context: SYSTEM } as any); + + const restricted: any = await svc.getRequest(requestId, asUser(RESTRICTED)); + expect(restricted).not.toHaveProperty('record_title'); + expect(JSON.stringify(restricted)).not.toContain(SNAPSHOT_TITLE); + + // (e) control, the fallback leg: the reader who may read it gets the snapshot's title. + const reader: any = await svc.getRequest(requestId, asUser(READER)); + expect(reader.record_title).toBe(SNAPSHOT_TITLE); + }); + + it('(c) the decision result row — the toast token\'s source — carries no title for the restricted reader', async () => { + const { requestId } = await openDeal(); + const out: any = await svc.decide(requestId, { decision: 'approve', actorId: RESTRICTED }, asUser(RESTRICTED)); + expect(out.request.status).toBe('approved'); + // `Approved "${result.request.record_title}"` reads nothing here. + expect(out.request).not.toHaveProperty('record_title'); + expect(JSON.stringify(out.request)).not.toContain(LIVE_TITLE); + }); + + it('(e) control: a reader who may read the field gets the title — single read, list and decision result', async () => { + const { requestId, recordId } = await openDeal(); + + const one: any = await svc.getRequest(requestId, asUser(READER)); + const [listed]: any[] = await svc.listRequests({ recordId }, asUser(READER)); + expect(one.record_title).toBe(LIVE_TITLE); + expect(listed.record_title).toBe(LIVE_TITLE); + expect(one.payload).toMatchObject({ name: LIVE_TITLE }); + + const out: any = await svc.decide(requestId, { decision: 'approve', actorId: READER }, asUser(READER)); + expect(out.request.record_title).toBe(LIVE_TITLE); + }); + + it('(d) the action-link page names the record by its id, for every bearer — confirm page and result page', async () => { + for (const approver of [RESTRICTED, READER]) { + const { requestId, recordId } = await openDeal(); + const { approve } = await svc.issueActionTokens(requestId, approver); + const url = `http://approvals.test/api/v1/approvals/act?token=${encodeURIComponent(approve)}`; + + const confirm = await (await svc.handleActionPage(new Request(url, { method: 'GET' }))).text(); + expect(confirm, `${approver} confirm`).toContain('name="token"'); + expect(confirm, `${approver} confirm`).toContain(`${recordId}`); + expect(confirm, `${approver} confirm`).not.toContain(LIVE_TITLE); + + const result = await (await svc.handleActionPage(new Request('http://approvals.test/api/v1/approvals/act', { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ token: approve }).toString(), + }))).text(); + expect(result, `${approver} result`).toContain('Approved'); + expect(result, `${approver} result`).toContain(`${recordId}`); + expect(result, `${approver} result`).not.toContain(LIVE_TITLE); + } + }); + + it('(f) a withheld display field never lends its value: the title is picked from what the reader is served', async () => { + await engine.insert('crm_ticket', { id: 'T1', code: 'SECRET-CODE-7', subject: 'Printer jammed' }, { context: SYSTEM } as any); + const row: any = await svc.openNodeRequest({ + object: 'crm_ticket', recordId: 'T1', runId: 'run_ticket', nodeId: 'review', flowName: 'ticket_review', + config: { approvers: [RESTRICTED, READER].map((value) => ({ type: 'user', value })), behavior: 'first_response' } as any, + submitterId: SUBMITTER, record: { id: 'T1', code: 'SECRET-CODE-7', subject: 'Printer jammed' }, + }, asUser(SUBMITTER)); + + const restricted: any = await svc.getRequest(String(row.id), asUser(RESTRICTED)); + expect(restricted.record_title).toBe('Printer jammed'); + expect(JSON.stringify(restricted)).not.toContain('SECRET-CODE-7'); + // Control: the declared display field still answers for a reader who may read it. + expect((await svc.getRequest(String(row.id), asUser(READER)) as any).record_title).toBe('SECRET-CODE-7'); + }); + + it('(g) one rule, asked once: the title and the payload share one answer per object per read', async () => { + const first = await openDeal(); + await openDeal(); + vis.calls.length = 0; + await svc.getRequest(first.requestId, asUser(RESTRICTED)); + expect(vis.calls).toEqual([{ object: 'crm_deal', userId: RESTRICTED }]); + + vis.calls.length = 0; + const rows = await svc.listRequests({ object: 'crm_deal', approverId: RESTRICTED }, asUser(RESTRICTED)); + expect(rows.length).toBeGreaterThan(1); + expect(vis.calls).toEqual([{ object: 'crm_deal', userId: RESTRICTED }]); + }); +}); From 6932e30c239f54cb08a940295b23bdb2599f001a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 04:35:09 +0000 Subject: [PATCH 2/2] chore(changeset): plugin-approvals subject title narrowing (patch) Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .changeset/23594-subject-title-reader-fls.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/23594-subject-title-reader-fls.md diff --git a/.changeset/23594-subject-title-reader-fls.md b/.changeset/23594-subject-title-reader-fls.md new file mode 100644 index 0000000000..02fa4b7ae9 --- /dev/null +++ b/.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.