diff --git a/.changeset/approval-recall-refusal-localized.md b/.changeset/approval-recall-refusal-localized.md new file mode 100644 index 0000000000..dde3b405d6 --- /dev/null +++ b/.changeset/approval-recall-refusal-localized.md @@ -0,0 +1,41 @@ +--- +"@objectstack/plugin-approvals": minor +--- + +fix(plugin-approvals): the non-submitter recall refusal renders through the +Operation Message Catalog instead of a hardcoded English sentence (#11993, the +services-side half of the shape-A ruling) + +A user who opened a record someone else had submitted for approval, clicked +Recall and was correctly refused read the reason in English regardless of their +own locale. `@objectstack/rest`'s `handleApprovalError` ships this service's +thrown reason as the 403 body's human-readable `error`, and Console splices it +under its own localized label — so an operator in a fully Chinese deployment +read a Chinese prefix glued onto an English sentence they could not act on +(`撤回审批失败: `). + +The refusal now renders through the shared Operation Message Catalog in +`@objectstack/spec/system` under the key `approval_recall_not_submitter` that +#12493 landed for it — the same mechanism `plugin-security`'s denial gates +already use, with the same resolution ladder (deployment override → the +caller's locale → `en` → the key) and the same guarantee that a misbehaving +i18n service cannot turn a 403 into a 500. All four platform locales (`en`, +`zh-CN`, `ja-JP`, `es-ES`) ship copy that names who *can* recall, rather than +dead-ending the reader. + +`ApprovalServiceOptions` gains an optional `messageTranslator` — a lazily +resolved, `II18nService.t`-compatible lookup, wired by `ApprovalsServicePlugin` +the same way `tenancyPosture` and the field-visibility source are, because the +i18n service is contributed by another plugin and may start later. It is what +makes the override address the catalog documents, +`errors.approval_recall_not_submitter`, actually take effect for this emitter; +a stack without an i18n service still renders the built-in catalog in the +caller's locale. + +**Not changed: who may recall an approval.** The gate is byte-identical — the +submitter, or a privileged admin releasing a stuck record (#3424). Only the +sentence the refusal carries is different, and the `FORBIDDEN:` code prefix +that the REST layer maps to 403 is untouched. + +The button-visibility half of #11993 — a non-submitter seeing a live recall +button at all — is not addressed here; see the issue for the measurement. diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 4396ac0ba8..6be2255524 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -50,6 +50,12 @@ import type { import type { ExecutionContext } from '@objectstack/spec/kernel'; import { RESUME_AUTHORITY_SERVICE } from '@objectstack/spec/contracts'; import { isFileIdToken } 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 +// mechanism for a second producer, and #12493 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 } from '@objectstack/core'; import { filterApproversWhoCanRead, @@ -598,6 +604,21 @@ export interface ApprovalServiceOptions { * its boundaries. */ recordReaderVisibleObjects?: string[]; + /** + * [#11993] Deployment i18n lookup for user-facing refusal copy — an + * `II18nService.t`-compatible function, resolved LAZILY per refusal for the + * same reason {@link ApprovalServiceOptions.tenancyPosture} is: the i18n + * service is registered by another plugin (ADR-0029 D8) which may start + * after this one, and a lookup captured at construction would pin + * `undefined` for the life of the process. + * + * Absent (or resolving to `undefined`) is a supported stack, not a + * degraded one: the built-in catalog still renders the caller's locale. + * What it adds is the override address the catalog documents — + * `errors.approval_recall_not_submitter` — so a deployment's own + * `translation` metadata wins over the built-in sentence. + */ + messageTranslator?: () => ValidationMessageTranslator | undefined; } export class ApprovalService implements IApprovalService { @@ -609,6 +630,8 @@ export class ApprovalService implements IApprovalService { private publicBaseUrl: string; private tenancyPosture?: () => string | undefined; private fieldVisibility?: FieldVisibilitySource; + /** [#11993] Lazily-resolved deployment i18n lookup for refusal copy. */ + private messageTranslator?: () => ValidationMessageTranslator | undefined; /** * [#8652] The enabled object set for the record-reader visibility tier. * EMPTY means the tier is off — the default, and the shape every existing @@ -634,6 +657,7 @@ export class ApprovalService implements IApprovalService { this.publicBaseUrl = (opts.publicBaseUrl ?? '').replace(/\/$/, ''); this.tenancyPosture = opts.tenancyPosture; this.fieldVisibility = opts.fieldVisibility; + this.messageTranslator = opts.messageTranslator; this.recordReaderVisibleObjects = new Set( (Array.isArray(opts.recordReaderVisibleObjects) ? opts.recordReaderVisibleObjects : []) .map((n) => String(n ?? '').trim()) @@ -825,6 +849,47 @@ export class ApprovalService implements IApprovalService { return requestOrg == null || (actorTenant != null && String(requestOrg) === String(actorTenant)); } + /** + * [#11993] The END USER's half of an approval refusal. + * + * `handleApprovalError` in `@objectstack/rest` maps this service's + * `CODE: message` throws onto the wire by testing the prefix for the status + * and then STRIPPING it (a leading run of `[A-Z_]` plus a colon and any + * following whitespace), shipping what remains as the body's + * human-readable `error` — which Console splices under its own localized + * label ("撤回审批失败: …"). A hardcoded English reason therefore reaches an + * operator in a fully Chinese deployment as a Chinese prefix glued onto an + * English sentence they cannot act on. + * + * Rendered through the SHARED Operation Message Catalog + * (`@objectstack/spec/system`), not a second mechanism: same + * `errors.` override address, same resolution ladder (deployment + * override -> locale catalog -> `en` -> the key), same guarantee that a + * misbehaving i18n service cannot turn a 403 into a 500. The catalog's + * header names this seat explicitly; `plugin-security`'s + * `userFacingDenialMessage` is the sibling consumer this mirrors. + * + * The CODE PREFIX stays on the message. It is not user copy — it is how the + * REST layer derives the status and the ADR-0112 wire code, and it is + * stripped before the sentence reaches a body. The developer's half moves + * to the log, where the ids it names are legible to an operator and to + * nobody else. + */ + private userFacingRefusal( + messageKey: 'approval_recall_not_submitter', + context: ExecutionContext, + ): string { + let translate: ValidationMessageTranslator | undefined; + try { + translate = this.messageTranslator?.(); + } catch { + // i18n is optional and late-bound; the built-in catalog still renders + // the caller's locale without it. + translate = undefined; + } + return renderOperationMessage({ messageKey }, { locale: context?.locale, translate }); + } + /** * Pin the acting identity to the AUTHENTICATED CALLER (#3800). * @@ -2701,9 +2766,28 @@ export class ApprovalService implements IApprovalService { } // The submitter withdraws their own request; a privileged admin may recall // any pending request to release a stuck record (#3424). + // + // [#11993] The GATE is untouched — who may recall an approval is exactly + // what it was. Only the refusal's user-facing half changed: it used to be + // one hardcoded English sentence that Console rendered verbatim in a + // toast. See {@link ApprovalService.userFacingRefusal}. if (!this.isOverrideActor(context, raw.organization_id ?? null) && raw.submitter_id && String(raw.submitter_id) !== String(actorId)) { - throw new Error(`FORBIDDEN: only the submitter may recall this request`); + // The developer's half: the ids the catalog sentence deliberately does + // not name (the throw site knows the submitter only as an opaque user + // id), kept where a developer reads them and a user never does. + const developerMessage = + `[approvals] recall refused: actor '${actorId}' is not the submitter of request ` + + `'${requestId}' (submitter '${String(raw.submitter_id)}') and holds no #3424 override`; + this.logger?.warn?.(developerMessage, { + request: requestId, + actor: actorId, + submitter: String(raw.submitter_id), + status: raw.status, + }); + throw new Error( + `FORBIDDEN: ${this.userFacingRefusal('approval_recall_not_submitter', context)}`, + ); } // A returned request is only recallable while it is still the run's live // frontier — a resubmitted (or later-node) request supersedes it. diff --git a/packages/plugins/plugin-approvals/src/approvals-plugin.ts b/packages/plugins/plugin-approvals/src/approvals-plugin.ts index 04f5c0e390..b2bb2b815c 100644 --- a/packages/plugins/plugin-approvals/src/approvals-plugin.ts +++ b/packages/plugins/plugin-approvals/src/approvals-plugin.ts @@ -199,6 +199,24 @@ export class ApprovalsServicePlugin implements Plugin { return undefined; } }, + // [#11993] Deployment override lookup for user-facing refusal copy. + // Resolved LAZILY for the reason the two providers above are: the i18n + // service is contributed by another plugin (ADR-0029 D8) and may start + // after this one. Without it the service still renders the built-in + // catalog in the caller's locale; with it, a deployment's own + // `translation` for `errors.approval_recall_not_submitter` wins — the + // override address the catalog documents, made real for this emitter. + messageTranslator: () => { + try { + const i18n = ctx.getService('i18n'); + const t = i18n?.t; + if (typeof t !== 'function') return undefined; + return (key: string, locale: string, params?: Record) => + t.call(i18n, key, locale, params); + } catch { + return undefined; + } + }, }); // [#10749] Field-visibility authority for payload-snapshot redaction. diff --git a/packages/plugins/plugin-approvals/src/recall-refusal-user-copy.test.ts b/packages/plugins/plugin-approvals/src/recall-refusal-user-copy.test.ts new file mode 100644 index 0000000000..b11664ac8a --- /dev/null +++ b/packages/plugins/plugin-approvals/src/recall-refusal-user-copy.test.ts @@ -0,0 +1,313 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The END USER's half of the non-submitter recall refusal (#11993). + * + * The report: in a fully Chinese deployment a non-submitter clicked 「撤回审批」 + * and read `撤回审批失败: `. Console composes its own + * localized label and splices the server's reason onto it verbatim — so a + * hardcoded English reason surfaces as a Chinese prefix glued to English prose + * the operator cannot act on. + * + * The refusal now renders through the shared Operation Message Catalog + * (`@objectstack/spec/system`, key `approval_recall_not_submitter`, landed by + * #12493) instead of a package-local string. + * + * ⚠️ These tests assert the SENTENCE AN OPERATOR READS, in zh-CN specifically. + * Asserting only that a catalog key was passed would pass against a message + * that still renders in English — which is the entire reported defect. + * + * They also pin the two things the conversion must NOT move: + * - the `FORBIDDEN:` code prefix, which is not user copy but how + * `@objectstack/rest`'s `handleApprovalError` derives 403 + the ADR-0112 + * wire code before stripping it off the body's `error` string; + * - WHO may recall. The gate is byte-identical; only its message changed. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { + ApprovalService, + type ApprovalServiceOptions, + type ApprovalNodeAutoOutcome, +} from './approval-service.js'; +import type { ApprovalRequestRow } from '@objectstack/spec/contracts'; + +interface FakeRow { [k: string]: any } + +/** The same minimal engine shape `approval-service.test.ts` uses. */ +function makeFakeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + function matches(row: FakeRow, filter: any): boolean { + if (!filter || typeof filter !== 'object') return true; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or') { + if (!(v as any[]).some(sub => matches(row, sub))) return false; + continue; + } + if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`); + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (rv !== v) return false; + } + return true; + } + return { + _tables: tables, + async find(object: string, options?: any) { + const rows = ensure(object).filter(r => matches(r, options?.filter ?? options?.where)); + if (options?.orderBy?.[0]) { + const { field, order } = options.orderBy[0]; + rows.sort((a, b) => { + const av = a[field]; const bv = b[field]; + if (av === bv) return 0; + const cmp = av > bv ? 1 : -1; + return order === 'desc' ? -cmp : cmp; + }); + } + const start = options?.offset ?? 0; + return rows.slice(start, start + (options?.limit ?? 1000)); + }, + async insert(object: string, data: any) { ensure(object).push({ ...data }); return { ...data }; }, + async update(object: string, data: any, options?: any) { + // Pinned to ObjectQL.update's OWN dispatch predicate — a double looser + // than the engine it stands in for turns a green suite into no suite. + const dispatch = assertEngineUpdateDispatch(data, options); + const t = ensure(object); + if (dispatch.kind === 'multi') { + let n = 0; + for (let i = 0; i < t.length; i++) { + if (matches(t[i], options?.where)) { t[i] = { ...t[i], ...data }; n++; } + } + return { updated: n }; + } + const i = t.findIndex(r => r.id === dispatch.id); + if (i >= 0) t[i] = { ...t[i], ...data }; + return t[i]; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const t = ensure(object); + if (dispatch.kind === 'multi') { + const survivors = t.filter(r => !matches(r, options?.where)); + const deleted = t.length - survivors.length; + t.splice(0, t.length, ...survivors); + return { deleted }; + } + const i = t.findIndex(r => r.id === dispatch.id); + if (i >= 0) t.splice(i, 1); + return { id: dispatch.id }; + }, + registerHook() {}, unregisterHooksByPackage() { return 0; }, async fire() {}, + }; +} + +/** + * `openNodeRequest` returns `ApprovalRequestRow | ApprovalNodeAutoOutcome` — the + * second arm is the `onEmptyApprovers: 'auto_approve'` exit, which opens no + * request at all. Narrowed rather than read through the union: every probe below + * is about an OPENED request, and reading `.id` off the union bills the + * package's TEST_DEBT ledger a raw TS2339 that + * `pnpm --filter @objectstack/plugin-approvals typecheck` cannot see (its + * tsconfig excludes `**\/*.test.ts`). + */ +function opened(result: ApprovalRequestRow | ApprovalNodeAutoOutcome): ApprovalRequestRow { + if ('autoApproved' in result) { + throw new Error('expected an OPENED approval request, got an auto-approval outcome'); + } + return result; +} + +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +/** The submitter — user A in the report. */ +const SUBMITTER = { userId: 'u1', tenantId: 't1', positions: [], permissions: [] } as any; +/** User B in the report: not the submitter, not an admin, Console set to zh-CN. */ +const OTHER_ZH = { + userId: 'u2', tenantId: 't1', positions: [], permissions: [], locale: 'zh-CN', +} as any; +const OTHER_EN = { + userId: 'u2', tenantId: 't1', positions: [], permissions: [], locale: 'en', +} as any; +/** A platform admin — the #3424 stuck-record override. */ +const ADMIN_ZH = { + userId: 'root', tenantId: 't1', positions: [], permissions: ['admin_full_access'], locale: 'zh-CN', +} as any; + +/** + * The zh-CN copy an operator actually reads, pinned as a LITERAL rather than + * read back out of the catalog — a test that renders the catalog against + * itself cannot tell Chinese from English. Its twin lives in + * `packages/spec/src/system/operation-message.test.ts`; the two move together. + */ +const ZH_SENTENCE = '只有提交人可以撤回这条审批请求,如需撤回请联系提交人或管理员。'; + +/** What `@objectstack/rest`'s `handleApprovalError` does to a thrown message. */ +const WIRE_CODE = (msg: string) => /^FORBIDDEN/.test(msg); +const WIRE_ERROR = (msg: string) => msg.replace(/^[A-Z_]+:\s*/, ''); + +describe('non-submitter recall refusal renders through the operation catalog (#11993)', () => { + let engine: ReturnType; + let n = 0; + const baseTime = new Date('2026-02-01T09:00:00Z').getTime(); + + const svcFor = (extra: Partial = {}) => new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(baseTime + (n++) * 1000) }, + ...extra, + }); + + const openInput = () => ({ + object: 'opportunity', recordId: 'opp1', runId: 'run_1', nodeId: 'step_1', + flowName: 'record_change_approval', + config: { + approvers: [{ type: 'user' as const, value: 'u9' }], + behavior: 'first_response' as const, + }, + record: { id: 'opp1', amount: 100 }, + }); + + /** The report's setup: A submits, the request is pending, B opens the record. */ + const pendingRequest = async (svc: ApprovalService) => + opened(await svc.openNodeRequest(openInput(), SUBMITTER)); + + beforeEach(() => { + engine = makeFakeEngine(); + n = 0; + }); + + it('the reported symptom: a zh-CN operator reads Chinese, with no English spliced in', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + + const err = await svc.recall(req.id, { actorId: 'u2' }, OTHER_ZH) + .then(() => null, (e: any) => e); + + expect(err).toBeInstanceOf(Error); + expect(err.message).toBe(`FORBIDDEN: ${ZH_SENTENCE}`); + // The half Console splices under 「撤回审批失败: 」 carries no Latin prose. + // Before this conversion it was an entire English sentence. + expect(WIRE_ERROR(err.message)).toBe(ZH_SENTENCE); + expect(WIRE_ERROR(err.message)).not.toMatch(/[A-Za-z]/); + }); + + it('the status/code prefix survives the conversion — REST still answers 403 FORBIDDEN', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + + const err = await svc.recall(req.id, { actorId: 'u2' }, OTHER_ZH) + .then(() => null, (e: any) => e); + + // The prefix is the wire contract, not copy: `handleApprovalError` tests it + // for the status and strips it off the body. A localized sentence must not + // shadow it — `FORBIDDEN: 只有…` still matches, and still strips clean. + expect(WIRE_CODE(err.message)).toBe(true); + expect(WIRE_ERROR(err.message).startsWith('FORBIDDEN')).toBe(false); + }); + + it('renders each platform locale from the catalog, not one hardcoded sentence', async () => { + for (const locale of ['en', 'ja-JP', 'es-ES'] as const) { + // A fresh engine per locale: `openNodeRequest` refuses a second pending + // request on the same record (`DUPLICATE_REQUEST`). + engine = makeFakeEngine(); + const svc = svcFor(); + const req = await pendingRequest(svc); + const err = await svc.recall(req.id, { actorId: 'u2' }, { ...OTHER_ZH, locale }) + .then(() => null, (e: any) => e); + expect(WIRE_ERROR(err.message)) + .toBe(BUILTIN_OPERATION_MESSAGES[locale].approval_recall_not_submitter); + } + }); + + it('no longer emits the legacy hardcoded English reason', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + const err = await svc.recall(req.id, { actorId: 'u2' }, OTHER_EN) + .then(() => null, (e: any) => e); + expect(err.message).not.toContain('only the submitter may recall this request'); + }); + + it('an unknown locale falls back to English rather than to the bare key', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + const err = await svc.recall(req.id, { actorId: 'u2' }, { ...OTHER_ZH, locale: 'kl-GL' }) + .then(() => null, (e: any) => e); + expect(WIRE_ERROR(err.message)) + .toBe(BUILTIN_OPERATION_MESSAGES.en.approval_recall_not_submitter); + expect(WIRE_ERROR(err.message)).not.toBe('approval_recall_not_submitter'); + }); + + it('a deployment `translation` for `errors.approval_recall_not_submitter` wins', async () => { + const seen: string[] = []; + const svc = svcFor({ + messageTranslator: () => (key: string, locale: string) => { + seen.push(`${key}@${locale}`); + return key === 'errors.approval_recall_not_submitter' && locale === 'zh-CN' + ? '本单只能由发起人撤回,请联系发起人。' + : key; // II18nService echoes the key back on a miss. + }, + }); + const req = await pendingRequest(svc); + const err = await svc.recall(req.id, { actorId: 'u2' }, OTHER_ZH) + .then(() => null, (e: any) => e); + + expect(seen).toContain('errors.approval_recall_not_submitter@zh-CN'); + expect(WIRE_ERROR(err.message)).toBe('本单只能由发起人撤回,请联系发起人。'); + }); + + it('a misbehaving i18n service degrades to the built-in copy, never to a 500', async () => { + const svc = svcFor({ + messageTranslator: () => { throw new Error('i18n exploded'); }, + }); + const req = await pendingRequest(svc); + const err = await svc.recall(req.id, { actorId: 'u2' }, OTHER_ZH) + .then(() => null, (e: any) => e); + + // Still the refusal, still 403-shaped — not the i18n service's error. + expect(err.message).toBe(`FORBIDDEN: ${ZH_SENTENCE}`); + }); + + it('logs the developer half — the ids the user-facing sentence deliberately omits', async () => { + const warn: Array<[string, any]> = []; + const svc = svcFor({ logger: { warn: (m: any, meta?: any) => { warn.push([String(m), meta]); } } }); + const req = await pendingRequest(svc); + await svc.recall(req.id, { actorId: 'u2' }, OTHER_ZH).catch(() => {}); + + const entry = warn.find(([m]) => m.includes('recall refused')); + expect(entry).toBeTruthy(); + expect(entry![0]).toContain("actor 'u2'"); + expect(entry![0]).toContain("submitter 'u1'"); + expect(entry![1]).toMatchObject({ actor: 'u2', submitter: 'u1', status: 'pending' }); + }); + + // ── WHO may recall is unchanged (the permission boundary) ───────── + // This conversion touches the refusal's message and nothing else. These are + // the controls that say so: the same three callers get the same three + // answers they got before it. + + it('the submitter still recalls their own pending request', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + const out = await svc.recall(req.id, { actorId: 'u1' }, SUBMITTER); + expect(out.request.status).toBe('recalled'); + }); + + it('a #3424 admin still recalls a request they did not submit', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + const out = await svc.recall(req.id, { actorId: 'root' }, ADMIN_ZH); + expect(out.request.status).toBe('recalled'); + }); + + it('a plain non-submitter is still refused, and the request is untouched', async () => { + const svc = svcFor(); + const req = await pendingRequest(svc); + await expect(svc.recall(req.id, { actorId: 'u2' }, OTHER_ZH)).rejects.toThrow(/^FORBIDDEN/); + const after = await svc.getRequest(req.id, SYS); + expect(after!.status).toBe('pending'); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 27a4789212..882c83a48d 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1911,6 +1911,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-approvals/src/recall-refusal-user-copy.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-approvals/src/recall-refusal-user-copy.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-approvals/src/record-reader-visibility.test.ts", "verb": "delete",