From fc51a68f343b669555b28b820f51367f1534726a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 14:53:23 +0000 Subject: [PATCH 1/2] fix(plugin-security,spec): localize the end-user half of the row-level and capability denials (#7451) The three gates of the object CRUD middleware an ordinary non-admin principal reaches on ordinary business work now render their user-facing half through the shared operation-message catalog, in the caller's locale: - row-level pre-image write denial -> errors.record_access_denied (new key) - row-level CHECK post-image denial -> errors.record_change_not_allowed (new key) - capability AND-gate (ADR-0066 D3) -> errors.permission_denied (reused) Two new keys, not three: the unit is the SITUATION the user is in, not the gate that answered. A caller missing a CRUD bit and a caller missing a capability have one situation and one remedy, so they share a sentence; the row-level gates do not, because their users can act differently (ask the record's owner / change what they typed). Each gate keeps its previous sentence byte for byte on `developerMessage`, logged at the throw site and shipped on neither transport, per #7414's measurement. Enforcement, statuses, codes and every `details` payload are untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- .changeset/security-denial-end-user-copy.md | 58 ++ .../src/row-write-widener-composition.test.ts | 20 +- .../src/security-denial-user-copy.test.ts | 530 ++++++++++++++++++ .../src/security-plugin.test.ts | 21 +- .../plugin-security/src/security-plugin.ts | 103 +++- .../authored-row-write-scope.dogfood.test.ts | 19 +- .../spec/src/system/operation-message.test.ts | 115 ++++ packages/spec/src/system/operation-message.ts | 63 ++- 8 files changed, 912 insertions(+), 17 deletions(-) create mode 100644 .changeset/security-denial-end-user-copy.md create mode 100644 packages/plugins/plugin-security/src/security-denial-user-copy.test.ts diff --git a/.changeset/security-denial-end-user-copy.md b/.changeset/security-denial-end-user-copy.md new file mode 100644 index 0000000000..83d99bb878 --- /dev/null +++ b/.changeset/security-denial-end-user-copy.md @@ -0,0 +1,58 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-security": minor +--- + +fix(plugin-security,spec): the row-level and capability `403 PERMISSION_DENIED` refusals stop handing a business user internal authorization vocabulary + +#7414 converted one template of this family — the object CRUD grant denial. The +same defect sat on the other gates of the same middleware that an ordinary, +non-admin principal reaches on ordinary business work. `Error.message` is the +body's human-readable string on every transport (`mapDataError`'s `body.error`, +the dispatcher's `error.message`) and Console renders it verbatim in a toast, so +a salesperson editing someone else's opportunity read + +``` +[Security] Access denied: not permitted to update this 'crm_opportunity' +record (row-level security) +``` + +English-only, naming a table they have never seen, and ending in the name of the +mechanism that refused them rather than anything they can act on. + +Three gates now render the user's half through the shared operation-message +catalog (`@objectstack/spec/system`, the mechanism built for `DELETE_RESTRICTED` +and reused by #7414), overridable per deployment under `errors.`: + +- the row-level pre-image write denial renders `record_access_denied`; +- the row-level CHECK post-image denial renders `record_change_not_allowed`; +- the capability AND-gate (ADR-0066 D3) renders the existing `permission_denied`. + +Two new catalog keys, in all four shipped locales, and not three: the rule is one +key per SITUATION, not per gate and not per wire code. A user blocked by +row-level security can often ask the record's owner; a user whose post-image +failed a CHECK can simply change what they typed; a user whose grants do not +cover the action needs an administrator. Those are three different next steps, so +they are three different sentences. A caller missing a CRUD bit and a caller +missing a `requiredPermissions` capability, by contrast, are in ONE situation with +one remedy — the difference between them is a fact about our authorization model, +which is exactly the vocabulary that must not reach a toast — so both render +`permission_denied`. + +Each sentence names nothing: no object, no record id, no capability, no +mechanism. That was re-derived per site rather than inherited. The row-level +denial is the one gate here that COULD have named honestly, because the refused +record is the one the caller just addressed; it still does not, because the only +spellings available at the throw site are the object's API name and an opaque row +id, and reaching a label means the ladder whose last rung is the API name. + +Each refusal keeps its developer half as `developerMessage`, the previous +sentence byte for byte, LOGGED at the throw site rather than shipped — following +#7414, which measured that REST's `mapDataError` builds `{ error, code, object? }` +and never reads `error.details`, so shipping it would ADD a disclosure on the +transport that discloses less. `developerMessage` is a sibling of `details`, +never a member, because `details` is what the runtime dispatcher serialises. + +Enforcement is untouched: same 403, same `PERMISSION_DENIED`, same decision +logic, and every structured `details` payload — including `requiredPermissions`, +`missingPermissions` and `recordId` — is byte-identical to before. diff --git a/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts b/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts index 813942d8d5..1092316028 100644 --- a/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts +++ b/packages/plugins/plugin-security/src/row-write-widener-composition.test.ts @@ -64,6 +64,7 @@ import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objects import { SharingService, buildSharingMiddleware } from '@objectstack/plugin-sharing'; import { PermissionSetSchema } from '@objectstack/spec/security'; import type { PermissionSet } from '@objectstack/spec/security'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; import { SecurityPlugin } from './security-plugin.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; @@ -279,7 +280,10 @@ interface WriteOutcome { /** ADR-0112 envelope of the refusal — asserted, never a bare `toThrow()`. */ code?: string; status?: number; + /** [#7451] The END USER's half — a localized catalog sentence since #7451. */ message: string; + /** [#7451] The DEVELOPER's half — the sentence `message` used to be. */ + developerMessage?: string; } interface Stack { @@ -356,6 +360,7 @@ async function makeStack(): Promise { code: e?.code, status: e?.statusCode, message: String(e?.message ?? e), + developerMessage: e?.developerMessage, }; } return reached @@ -386,12 +391,25 @@ const WIDENED_CTX = ctxFor('u_widened', 'crm_rep_widened'); * threw". A bare `toThrow()` carries one bit where the defect has three: which * gate refused, with what code, at what status. The row-level pre-image gate is * the only place that produces this exact sentence. + * + * [#7451] Re-spelled, not weakened — and it now discriminates on TWO axes + * instead of one. That sentence is no longer `message`: it is + * `developerMessage` (logged at the throw site, never shipped), while `message` + * is the user-facing catalog entry for `record_access_denied` rendered in + * `ExecutionContext.locale` — `en` here, since `ctxFor` declares no locale. + * The user half is asserted against the catalog CONSTANT rather than a literal, + * so a future copy edit needs no re-spell in this file; the developer half is + * still asserted verbatim, which is what keeps "the row-level pre-image gate is + * the one that answered" a measured fact rather than an inference from a + * generic 403. */ function expectRowLevelDenial(outcome: WriteOutcome, operation: 'update' | 'delete', object: string) { expect(outcome.ok, `expected a refusal, got a completed ${operation}`).toBe(false); expect(outcome.code, 'ADR-0112 error code').toBe('PERMISSION_DENIED'); expect(outcome.status, 'ADR-0112 HTTP status').toBe(403); - expect(outcome.message).toContain( + expect(outcome.message, 'the user half is the localized catalog sentence') + .toBe(BUILTIN_OPERATION_MESSAGES.en.record_access_denied); + expect(outcome.developerMessage, 'the developer half names WHICH gate refused').toContain( `[Security] Access denied: not permitted to ${operation} this '${object}' record (row-level security)`, ); } diff --git a/packages/plugins/plugin-security/src/security-denial-user-copy.test.ts b/packages/plugins/plugin-security/src/security-denial-user-copy.test.ts new file mode 100644 index 0000000000..e08f8aaa2b --- /dev/null +++ b/packages/plugins/plugin-security/src/security-denial-user-copy.test.ts @@ -0,0 +1,530 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7451 — the END-USER-facing half of the `[Security] Access denied …` family, + * driven through the REAL `SecurityPlugin` middleware. + * + * #7414 (PR #7449) converted ONE template: the object-CRUD grant denial. This + * file covers the other gates of that middleware an ordinary, non-admin + * business principal reaches on ordinary business work: + * + * | Gate | Was | Now renders | + * |---|---|---| + * | capability AND-gate (ADR-0066 D3/⑤) | `'crm_contract' (operation 'delete') requires capability [manage_contracts] — caller is missing […]` | `permission_denied` | + * | row-level pre-image write denial (step 2.7) | `not permitted to update this 'crm_opportunity' record (row-level security)` | `record_access_denied` | + * | row-level CHECK post-image denial (ADR-0058 D4) | `the update would violate a row-level CHECK on 'crm_opportunity'` | `record_change_not_allowed` | + * + * ## Why three keys and not one + * + * The catalog's rule is one key per SITUATION, not per wire code (#7307 gave + * the single code `DELETE_RESTRICTED` two sentences). The situations here are + * genuinely different in what the user can DO next: ask an administrator / ask + * the record's owner / change what they just typed. The distinction the copy + * deliberately does NOT make is "missing CRUD bit" vs "missing capability" — + * that is a fact about our authorization model, the user's situation and remedy + * are identical, so both render `permission_denied`. + * + * ## Why a real `II18nService` and not a stub + * + * A hand-written `t` can agree with a producer that disagrees with the shipped + * implementation — this repo has two brace conventions in flight (#7333), and a + * stub picks whichever one the test author had in mind. So the override rung is + * measured against `FileI18nAdapter`, the actual `II18nService` the platform + * ships, loaded the way `loadTranslations` loads a real bundle. + * + * The catalog half is pinned in + * `packages/spec/src/system/operation-message.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { matchesFilterCondition } from '@objectstack/formula'; +import { FileI18nAdapter } from '@objectstack/service-i18n'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +// ── metadata ─────────────────────────────────────────────────────────────── + +/** An ordinary tenant business object — the reporter's shape, not a `sys_` table. */ +const OPPORTUNITY_SCHEMA = { + name: 'crm_opportunity', + fields: { + id: { name: 'id' }, + name: { name: 'name' }, + stage: { name: 'stage' }, + owner_id: { name: 'owner_id' }, + created_by: { name: 'created_by' }, + organization_id: { name: 'organization_id' }, + }, +}; + +/** + * The capability AND-gate's object. `requiredPermissions` is an authorable + * TOP-LEVEL object key (ADR-0066 D3, `object.zod.ts`), which is what makes this + * gate reachable by a business user rather than only by an administrator: an + * app author gates an ordinary object on an ordinary capability and every + * member who lacks it meets this sentence on a normal click. + */ +const CONTRACT_SCHEMA = { + name: 'crm_contract', + requiredPermissions: ['manage_contracts'], + fields: { + id: { name: 'id' }, + title: { name: 'title' }, + owner_id: { name: 'owner_id' }, + organization_id: { name: 'organization_id' }, + }, +}; + +const SCHEMAS: Record = { + crm_opportunity: OPPORTUNITY_SCHEMA, + crm_contract: CONTRACT_SCHEMA, +}; + +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; + +/** + * Full object-level CRUD on both objects — so the CRUD grant gate (the one + * #7414 already converted) ADMITS, and the refusals measured below come from + * the gates this card is about. The row-level policies are the shape an app + * author actually writes: showcase authors `using: 'assignee == current_user.email'` + * and `check: 'owner == current_user.email'` on its own business objects. + */ +const APP_WRITER: PermissionSet = PermissionSetSchema.parse({ + name: 'app_writer', + objects: { + crm_opportunity: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + crm_contract: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + rowLevelSecurity: [ + { name: 'own_opportunities_update', object: 'crm_opportunity', operation: 'update', using: 'owner_id == current_user.id' }, + { name: 'own_opportunities_delete', object: 'crm_opportunity', operation: 'delete', using: 'owner_id == current_user.id' }, + // check-only, so it governs the POST-image without also filtering the + // pre-image — the two row-level gates stay independently reachable. + { name: 'stage_stays_open', object: 'crm_opportunity', operation: 'update', check: "stage == 'open'" }, + ], +}); + +const PERMISSION_SETS: PermissionSet[] = [MEMBER_DEFAULT, APP_WRITER]; + +const USER = 'u_rep'; +const MINE = { id: 'opp_mine', name: 'Mine', stage: 'open', owner_id: USER, created_by: USER, organization_id: 'org1' }; +const THEIRS = { id: 'opp_theirs', name: 'Theirs', stage: 'open', owner_id: 'u_other', created_by: 'u_other', organization_id: 'org1' }; + +// ── in-memory engine ─────────────────────────────────────────────────────── + +/** + * ⚠️ `find` HONOURS its predicate, through the PRODUCER'S OWN matcher + * (`matchesFilterCondition`, the function `security-plugin.ts` itself uses for + * post-image checks). Both halves of that are load-bearing rather than + * thoroughness: + * + * 1. A double whose `find` ignores the `where` hands every seeded row back for + * any query. `SecurityPlugin.start()` seeds its bootstrap permission sets + * through `insert`, and permission-set resolution reads them back with + * `find('sys_permission_set', { where: { name: { $in: … } } })` — so an + * ignoring double returns `admin_full_access` for any unresolved name and + * the gates ADMIT everything. That is measured, not hypothesised: it is + * exactly what the first draft of #7449's harness did, 12 green assertions + * all measuring an admission. + * 2. A double with a HAND-WRITTEN matcher is the mirror failure. The row-level + * pre-image gate re-reads with `where: { $and: [{ id }, ] }`; + * a matcher that does not understand `$and` matches nothing, every by-id + * write "is denied", and the denial cases below would pass while measuring + * the double's blindness instead of the gate's verdict. The positive + * controls in each describe exist to make that failure impossible to miss. + */ +function makeEngine() { + const tables: Record = { + crm_opportunity: [{ ...MINE }, { ...THEIRS }], + crm_contract: [{ id: 'ct_1', title: 'Theirs', owner_id: 'u_other', organization_id: 'org1' }], + }; + const middlewares: any[] = []; + const matches = (row: any, filter: any): boolean => + !filter || typeof filter !== 'object' ? true : matchesFilterCondition(row, filter); + return { + _tables: tables, + _middlewares: middlewares, + registerMiddleware: (mw: any) => middlewares.push(mw), + getSchema: (name: string) => SCHEMAS[name], + async find(object: string, options: any = {}) { + return (tables[object] ??= []).filter((r) => matches(r, options?.where ?? options?.filter)); + }, + async findOne(object: string, options: any = {}) { + return (await this.find(object, options))[0] ?? null; + }, + async insert(object: string, data: any) { (tables[object] ??= []).push({ ...data }); return data; }, + // Both write verbs open with the PRODUCER's dispatch predicate, never a + // hand-mirrored guard: a fake looser than `ObjectQL` collects greens from + // call shapes the real engine would refuse. + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const rows = (tables[object] ??= []); + const targets = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id === dispatch.id) + : rows.filter((r) => matches(r, options?.where)); + for (const r of targets) Object.assign(r, data); + return dispatch.kind === 'by-id' ? (targets[0] ?? null) : targets.length; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const rows = (tables[object] ??= []); + const targets = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id === dispatch.id) + : rows.filter((r) => matches(r, options?.where)); + tables[object] = rows.filter((r) => !targets.includes(r)); + return dispatch.kind === 'by-id' ? targets.length > 0 : targets.length; + }, + }; +} + +// ── the stack ────────────────────────────────────────────────────────────── + +interface Outcome { + /** Did the middleware reach `next()`? */ + admitted: boolean; + /** The END USER's half — what Console puts in the toast. */ + message: string; + /** The DEVELOPER's half — must never be the same string as `message`. */ + developerMessage?: string; + code?: string; + status?: number; + details?: Record; + /** Everything the plugin logged, so the developer half can be located. */ + logged: { warn: string[]; error: string[] }; +} + +type OpShape = { object: string; operation: string; data?: any; options?: any }; + +/** + * @param i18n a real `II18nService`, or `undefined` to run the deployment that + * registers none — the built-in catalog must still localize. + */ +async function run(op: OpShape, locale?: string, i18n?: FileI18nAdapter): Promise { + const engine = makeEngine(); + const metadata = { + get: async (_type: string, name: string) => SCHEMAS[name] ?? null, + list: async () => PERMISSION_SETS, + }; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata, + 'org-scoping': { name: 'org-scoping' }, + ...(i18n ? { i18n } : {}), + }; + const warn: string[] = []; + const error: string[] = []; + const ctx: any = { + logger: { + info: vi.fn(), debug: vi.fn(), + warn: (m: string) => { warn.push(String(m)); }, + error: (m: string) => { error.push(String(m)); }, + }, + registerService: vi.fn(), + getService: (name: string) => { + // A kernel that has no such service THROWS — the shape the plugin's own + // ADR-0029 D8 contribution guards against, and the reason the i18n lookup + // at the throw site is wrapped. + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx); + await plugin.start(ctx); + const securityMw = engine._middlewares[0]; + + const opCtx: any = { + ...op, + context: { + userId: USER, + tenantId: 'org1', + positions: ['org_member'], + permissions: ['app_writer'], + ...(locale ? { locale } : {}), + }, + }; + let admitted = false; + try { + await securityMw(opCtx, async () => { admitted = true; }); + } catch (e: any) { + return { + admitted: false, + message: String(e?.message ?? e), + developerMessage: e?.developerMessage, + code: e?.code, + status: e?.statusCode, + details: e?.details, + logged: { warn, error }, + }; + } + return { admitted, message: '', logged: { warn, error } }; +} + +const denyCapability = (locale?: string, i18n?: FileI18nAdapter) => + run({ object: 'crm_contract', operation: 'delete', options: { where: { id: 'ct_1' } } }, locale, i18n); + +const denyRowLevel = (locale?: string, i18n?: FileI18nAdapter) => + run({ object: 'crm_opportunity', operation: 'update', data: { id: THEIRS.id, name: 'taken' } }, locale, i18n); + +const denyRowCheck = (locale?: string, i18n?: FileI18nAdapter) => + run({ object: 'crm_opportunity', operation: 'update', data: { id: MINE.id, stage: 'closed' } }, locale, i18n); + +/** The developer sentences the three messages used to be — the "byte for byte" rule. */ +const DEV_CAPABILITY = + "[Security] Access denied: 'crm_contract' (operation 'delete') requires capability " + + '[manage_contracts] — caller is missing [manage_contracts]'; +const DEV_ROW_LEVEL = + "[Security] Access denied: not permitted to update this 'crm_opportunity' record (row-level security)"; +const DEV_ROW_CHECK = + "[Security] Access denied: the update would violate a row-level CHECK on 'crm_opportunity'"; + +/** + * Everything a business user must never read in one of these toasts: object and + * capability API names, the internal authorization nouns, the machine operation + * token, and the developer prefix the sentences used to open with. + */ +const FORBIDDEN_IN_USER_COPY = [ + 'crm_opportunity', 'crm_contract', 'manage_contracts', + 'positions', 'org_member', 'row-level', 'CHECK', 'capability', + '[Security]', 'Access denied', +]; + +// ── the harness must be able to say YES ──────────────────────────────────── + +describe('#7451 — harness controls (a denial only means something if an admission is reachable)', () => { + /** + * ⚠️ These three are not padding. Every denial case in this file would also + * pass against a harness that refuses EVERYTHING — a `find` that ignores its + * predicate, a matcher blind to `$and`, a permission set that failed to + * parse. These controls are the only assertions here that go red on that + * class of harness bug, which is precisely the failure #7449 recorded. + */ + it('ADMITS an update to a row the caller owns (the pre-image re-read really can return a row)', async () => { + const out = await run({ object: 'crm_opportunity', operation: 'update', data: { id: MINE.id, name: 'renamed' } }); + expect(out.admitted, out.message).toBe(true); + }); + + it('ADMITS a post-image that satisfies the CHECK policy', async () => { + const out = await run({ object: 'crm_opportunity', operation: 'update', data: { id: MINE.id, stage: 'open' } }); + expect(out.admitted, out.message).toBe(true); + }); + + it('ADMITS a delete of the caller\'s own row', async () => { + const out = await run({ object: 'crm_opportunity', operation: 'delete', options: { where: { id: MINE.id } } }); + expect(out.admitted, out.message).toBe(true); + }); +}); + +// ── the three gates ──────────────────────────────────────────────────────── + +const GATES = [ + { + label: 'capability AND-gate', + deny: denyCapability, + key: 'permission_denied', + developer: DEV_CAPABILITY, + }, + { + label: 'row-level pre-image write denial', + deny: denyRowLevel, + key: 'record_access_denied', + developer: DEV_ROW_LEVEL, + }, + { + label: 'row-level CHECK post-image denial', + deny: denyRowCheck, + key: 'record_change_not_allowed', + developer: DEV_ROW_CHECK, + }, +] as const; + +describe.each(GATES)('#7451 — the 403 an end user reads: $label', ({ deny, key, developer }) => { + it('speaks the caller locale, not English', async () => { + const zh = await deny('zh-CN'); + expect(zh.message).toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'][key]); + + const ja = await deny('ja-JP'); + expect(ja.message).toBe(BUILTIN_OPERATION_MESSAGES['ja-JP'][key]); + // Two locales, two different sentences — "localized" is measured, not + // assumed from a single catalog read. + expect(ja.message).not.toBe(zh.message); + }); + + it('falls back to en for a locale-less context and an uncarried locale', async () => { + for (const locale of [undefined, 'de-DE']) { + const out = await deny(locale); + expect(out.message, `locale=${String(locale)}`).toBe(BUILTIN_OPERATION_MESSAGES.en[key]); + } + }); + + it('names no object, no capability and no authorization vocabulary — in every locale it can render', async () => { + const locales = Object.keys(BUILTIN_OPERATION_MESSAGES); + // Guard the guard: a catalog that lost its locales would make this loop + // vacuously true, which is the shape of an assertion that cannot fail. + expect(locales.length).toBeGreaterThanOrEqual(4); + for (const locale of locales) { + const out = await deny(locale); + // Pinned to the catalog sentence FIRST: a message that had gone empty + // would satisfy every absence assertion below, so the absences are only + // meaningful on top of a positive identity. + expect(out.message).toBe(BUILTIN_OPERATION_MESSAGES[locale][key]); + expect(out.message.length).toBeGreaterThan(10); + for (const forbidden of FORBIDDEN_IN_USER_COPY) { + expect(out.message.toLowerCase(), `${locale} must not say "${forbidden}"`) + .not.toContain(forbidden.toLowerCase()); + } + } + }); + + it('carries the previous sentence byte for byte on `developerMessage`, and logs it in English', async () => { + const out = await deny('zh-CN'); + expect(out.developerMessage).toBe(developer); + expect(out.developerMessage).not.toBe(out.message); + expect(out.logged.warn).toContain(developer); + // The log must not go out in the caller's language — the operator reading + // it is not the caller. + expect(out.logged.warn.join('\n')).not.toContain(out.message); + }); + + it('keeps `developerMessage` OUT of `details` — `details` is what the dispatcher serialises', async () => { + // The measurement this decision rests on (#7414): the runtime dispatcher + // does `this.error(e.message, 403, { code: 'PERMISSION_DENIED', ...(e.details ?? {}) })` + // and `buildApiError` puts the remainder on the wire as `error.details`, so + // anything inside `details` reaches the browser. REST's `mapDataError` does + // not even read it, so there the message is the ONLY channel — which is why + // the developer half is logged on both transports rather than shipped on + // either. Widening that is #7450's question, not this card's. + const out = await deny('zh-CN'); + expect(Object.keys(out.details ?? {})).not.toContain('developerMessage'); + expect(JSON.stringify(out.details)).not.toContain('[Security]'); + }); +}); + +// ── the three situations are three sentences ─────────────────────────────── + +describe('#7451 — one key per SITUATION, not per wire code', () => { + it('gives the row-level gates sentences of their own, distinct from the grant denial', async () => { + const [cap, row, check] = await Promise.all([denyCapability('en'), denyRowLevel('en'), denyRowCheck('en')]); + expect(row.message).not.toBe(cap.message); + expect(check.message).not.toBe(cap.message); + expect(check.message).not.toBe(row.message); + }); + + it('deliberately gives the capability gate the SAME sentence as the CRUD grant gate', async () => { + // Not an oversight — the classification. A caller missing a CRUD bit and a + // caller missing a `requiredPermissions` capability are in one situation + // with one remedy ("an administrator can grant this"); which of the two + // gates answered is a developer fact and stays on `developerMessage` and in + // `details.missingPermissions`. If a future card gives them separate + // sentences it should do so because a USER can act on the difference. + const cap = await denyCapability('en'); + expect(cap.message).toBe(BUILTIN_OPERATION_MESSAGES.en.permission_denied); + expect(cap.developerMessage).toContain('requires capability'); + expect(cap.details?.missingPermissions).toEqual(['manage_contracts']); + }); +}); + +// ── enforcement is untouched ─────────────────────────────────────────────── + +describe('#7451 — enforcement is untouched', () => { + /** + * ⚠️ Pins in this block are NON-REGRESSION guards, not revert-detectors: + * they are green on `origin/main` too, BY CONSTRUCTION. That is the point — + * this card is copy-only, and its reverse verification is that these do NOT + * move while the messages do. Anything that moves here is a bug in the copy + * change, not a passing revert test. + */ + it('is still a 403 PERMISSION_DENIED at every converted gate', async () => { + for (const { deny } of GATES) { + const out = await deny('zh-CN'); + expect(out.code).toBe('PERMISSION_DENIED'); + expect(out.status).toBe(403); + } + }); + + it('carries the same structured payload as before — byte for byte', async () => { + expect((await denyCapability('zh-CN')).details).toEqual({ + operation: 'delete', + object: 'crm_contract', + positions: ['org_member'], + permissionSets: ['app_writer'], + requiredPermissions: ['manage_contracts'], + missingPermissions: ['manage_contracts'], + }); + expect((await denyRowLevel('zh-CN')).details).toEqual({ + operation: 'update', + object: 'crm_opportunity', + positions: ['org_member'], + permissionSets: ['app_writer'], + recordId: THEIRS.id, + }); + expect((await denyRowCheck('zh-CN')).details).toEqual({ + operation: 'update', + object: 'crm_opportunity', + positions: ['org_member'], + permissionSets: ['app_writer'], + }); + }); + + it('still refuses the write — the row is untouched', async () => { + const out = await denyRowLevel('zh-CN'); + expect(out.admitted).toBe(false); + }); +}); + +// ── the resolution ladder, against the REAL II18nService ─────────────────── + +describe('#7451 — resolution ladder, against the REAL II18nService', () => { + const bundleFor = (entries: Record) => { + const adapter = new FileI18nAdapter({ defaultLocale: 'en' }); + adapter.loadTranslations('zh-CN', entries); + return adapter; + }; + + it('a deployment override under `errors.record_access_denied` wins', async () => { + const out = await denyRowLevel('zh-CN', bundleFor({ + errors: { record_access_denied: '这条记录不在您的负责范围内,请联系记录负责人。' }, + })); + expect(out.message).toBe('这条记录不在您的负责范围内,请联系记录负责人。'); + }); + + it('overriding one key does NOT bleed into its siblings', async () => { + const bundle = bundleFor({ errors: { record_access_denied: '仅覆盖这一条。' } }); + expect((await denyRowLevel('zh-CN', bundle)).message).toBe('仅覆盖这一条。'); + expect((await denyRowCheck('zh-CN', bundle)).message) + .toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'].record_change_not_allowed); + expect((await denyCapability('zh-CN', bundle)).message) + .toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'].permission_denied); + }); + + it('a bundle that carries no such key falls through to the built-in catalog', async () => { + // The real adapter echoes the KEY back on a miss — the contract + // `renderOperationMessage` detects a miss by. Measured here rather than + // stubbed, because a stub is free to answer `undefined` and hide the fact + // that the producer must recognise an echo. + const out = await denyRowCheck('zh-CN', bundleFor({ objects: { crm_opportunity: { label: '商机' } } })); + expect(out.message).toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'].record_change_not_allowed); + }); + + it('leaves a broken override visibly broken rather than silently blank', async () => { + // These sentences take no parameters, so an override that references one is + // an authoring mistake. It must stay legible as a mistake — the #7333 + // brace-convention trap is only findable if the placeholder survives. + const out = await denyRowLevel('zh-CN', bundleFor({ + errors: { record_access_denied: '无权访问:{{objectLabel}}' }, + })); + expect(out.message).toBe('无权访问:{{objectLabel}}'); + }); + + it('a deployment with NO i18n service still gets the caller locale', async () => { + for (const { deny, key } of GATES) { + const out = await deny('zh-CN'); + expect(out.message).toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'][key]); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index a8d53a255f..4c1da553e6 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -12,6 +12,7 @@ import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; import { isSupportedRlsExpression } from '@objectstack/formula'; import type { PermissionSet } from '@objectstack/spec/security'; import { RLS } from '@objectstack/spec/security'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; // --------------------------------------------------------------------------- // SecurityPlugin – basic metadata @@ -1023,9 +1024,18 @@ describe('SecurityPlugin', () => { object: 'task', operation: 'find', ast: { where: undefined }, context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, }; + // [#7451] Re-spelled, not weakened. `requires capability` was the + // capability AND-gate's DEVELOPER sentence; it is now `developerMessage` + // (logged at the throw site, never shipped), and `message` is the + // user-facing catalog entry — the same `permission_denied` the CRUD gate + // renders, deliberately, because a caller missing a capability and a + // caller missing a CRUD bit are in the same situation and have the same + // remedy. Asserted against the catalog CONSTANT so a copy edit needs no + // re-spell here; the developer half still pins WHICH gate answered. await expect(harness.run(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError', - message: expect.stringContaining('requires capability'), + message: BUILTIN_OPERATION_MESSAGES.en.permission_denied, + developerMessage: expect.stringContaining('requires capability'), }); }); @@ -1159,9 +1169,16 @@ describe('SecurityPlugin', () => { object: 'task', operation: 'insert', data: { name: 'A' }, context: { userId: 'u1', tenantId: 'org-1', positions: [], permissions: [] }, }; + // [#7451] Re-spelled onto `developerMessage`, which is where the machine + // detail now lives; `message` is the user-facing catalog entry. What this + // case proves is unchanged and still proved: the refusal came from the + // capability gate FOR THE INSERT operation, i.e. the per-operation + // narrowing (ADR-0066 ⑤) really is per-operation — the sibling case above + // shows `find` on the same object is admitted. await expect(harness.run(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError', - message: expect.stringContaining("operation 'insert'"), + message: BUILTIN_OPERATION_MESSAGES.en.permission_denied, + developerMessage: expect.stringContaining("operation 'insert'"), }); }); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index f0fe54976e..0525f11224 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -381,8 +381,21 @@ export { describeHighPrivilegeBits } from '@objectstack/spec/security'; * The i18n service is optional and resolved per call: it is registered by a * different plugin, may register after this one, and a deployment that runs * without it still gets the built-in catalog in the caller's locale. + * + * [#7451] `messageKey` is a PARAMETER rather than the hard-coded + * `permission_denied` it started as, because the end-user gates of this + * middleware refuse for three different reasons and a user in one of them can + * do something different from a user in another (catalog table: their grants do + * not cover the action / they may not touch THIS record / they may not put it + * into THAT state). One wire code, several sentences, is the rule #7307 set + * with `delete_restricted` and `delete_restricted_required`; the unit is the + * situation, never the code. */ -function userFacingDenialMessage(ctx: PluginContext, locale: string | undefined): string { +function userFacingDenialMessage( + ctx: PluginContext, + messageKey: 'permission_denied' | 'record_access_denied' | 'record_change_not_allowed', + locale: string | undefined, +): string { let translate: | ((key: string, loc: string, params?: Record) => string) | undefined; @@ -397,7 +410,7 @@ function userFacingDenialMessage(ctx: PluginContext, locale: string | undefined) // later than this one). The built-in catalog still renders the caller's // locale without it. } - return renderOperationMessage({ messageKey: 'permission_denied' }, { locale, translate }); + return renderOperationMessage({ messageKey }, { locale, translate }); } export class SecurityPlugin implements Plugin { @@ -1176,9 +1189,38 @@ export class SecurityPlugin implements Plugin { : []; if (missing.length > 0 || missingDel.length > 0) { const allMissing = [...new Set([...missing, ...missingDel])]; - throw new PermissionDeniedError( + // [#7451] Two messages, two audiences — the #7414 split, applied to + // the gate one step above the CRUD grant. + // + // The user's half REUSES `permission_denied` rather than adding a + // key, and that is the classification, not laziness. A caller + // missing a CRUD bit and a caller missing a `requiredPermissions` + // capability are in the SAME situation: their grants do not cover + // this action and an administrator is the remedy. "Capability" vs + // "object permission" is a fact about our authorization model, and + // the model is precisely what must not reach a toast — the sentence + // would have to name capability IDs (`manage_x`) to say anything + // more, which is internal vocabulary by construction. + // + // The developer half is the previous sentence BYTE FOR BYTE, logged + // at the throw site and attached as a SIBLING of `details` (never a + // member — `details` is what the runtime dispatcher serialises to + // the browser; #7414 measured that, and #7450 tracks the disclosure + // that already exists there). `code` / `statusCode` / `details`, + // including `requiredPermissions` and `missingPermissions`, are + // untouched: which gate answered stays fully legible to a developer. + const developerMessage = `[Security] Access denied: '${opCtx.object}' (operation '${opCtx.operation}') requires capability ` + - `[${required.join(', ')}] — ${missing.length > 0 ? 'caller' : 'the delegator'} is missing [${allMissing.join(', ')}]`, + `[${required.join(', ')}] — ${missing.length > 0 ? 'caller' : 'the delegator'} is missing [${allMissing.join(', ')}]`; + ctx.logger.warn(developerMessage, { + operation: opCtx.operation, + object: opCtx.object, + requiredPermissions: required, + missingPermissions: allMissing, + userId: opCtx.context?.userId ?? 'unknown', + }); + throw new PermissionDeniedError( + userFacingDenialMessage(ctx, 'permission_denied', opCtx.context?.locale), { operation: opCtx.operation, object: opCtx.object, @@ -1187,6 +1229,7 @@ export class SecurityPlugin implements Plugin { requiredPermissions: required, missingPermissions: allMissing, }, + developerMessage, ); } } @@ -1247,7 +1290,7 @@ export class SecurityPlugin implements Plugin { userId: opCtx.context?.userId ?? 'unknown', }); throw new PermissionDeniedError( - userFacingDenialMessage(ctx, opCtx.context?.locale), + userFacingDenialMessage(ctx, 'permission_denied', opCtx.context?.locale), { operation: opCtx.operation, object: opCtx.object, positions, permissionSets: explicitPermissionSets }, developerMessage, ); @@ -1430,9 +1473,32 @@ export class SecurityPlugin implements Plugin { visible = null; } if (!visible) { - throw new PermissionDeniedError( + // [#7451] The refusal an ordinary business user is most likely to + // meet: they hold the object grant, and the ROW is what they may + // not touch. A DIFFERENT situation from the CRUD-grant denial + // above, so a different catalog key — `record_access_denied`. The + // remedy differs too, which is the test: "ask an administrator" + // is wrong here, because the record's owner can often share it. + // + // The sentence names nothing, and unlike #7414's gate this one + // COULD have named honestly (the row is the one the caller just + // addressed by id). It still does not: the only spellings + // available here are the object's API name and an opaque row id, + // and reaching a LABEL means the ladder whose last rung is the + // API name — the exact leak #7414 refused. The user knows which + // record they clicked. + const developerMessage = `[Security] Access denied: not permitted to ${opCtx.operation} this ` + - `'${opCtx.object}' record (row-level security)`, + `'${opCtx.object}' record (row-level security)`; + ctx.logger.warn(developerMessage, { + operation: opCtx.operation, + object: opCtx.object, + recordId: targetId, + positions, + userId: opCtx.context?.userId ?? 'unknown', + }); + throw new PermissionDeniedError( + userFacingDenialMessage(ctx, 'record_access_denied', opCtx.context?.locale), { operation: opCtx.operation, object: opCtx.object, @@ -1440,6 +1506,7 @@ export class SecurityPlugin implements Plugin { permissionSets: explicitPermissionSets, recordId: targetId, }, + developerMessage, ); } } @@ -1782,9 +1849,29 @@ export class SecurityPlugin implements Plugin { this.logger.warn?.( `[Security] RLS check FAILED on ${opCtx.operation} '${opCtx.object}' — write denied (fail-closed)`, ); + // [#7451] The third end-user situation, and the only one of the + // three the user can resolve THEMSELVES: they may edit this record, + // just not into the state they asked for (showcase authors exactly + // this — `check: 'owner == current_user.email'`, so a contributor + // cannot reassign an invoice they own). Hence its own key, and copy + // that says "change what you entered" rather than "ask an admin". + // + // It names nothing for a reason particular to this gate: the + // post-image failed an authored predicate over the WHOLE row, and + // the gate does not know which field carried the offending value. + // Naming the object without the field would send the user hunting. + const developerMessage = + `[Security] Access denied: the ${opCtx.operation} would violate a row-level CHECK on '${opCtx.object}'`; + ctx.logger.warn(developerMessage, { + operation: opCtx.operation, + object: opCtx.object, + positions, + userId: opCtx.context?.userId ?? 'unknown', + }); throw new PermissionDeniedError( - `[Security] Access denied: the ${opCtx.operation} would violate a row-level CHECK on '${opCtx.object}'`, + userFacingDenialMessage(ctx, 'record_change_not_allowed', opCtx.context?.locale), { operation: opCtx.operation, object: opCtx.object, positions, permissionSets: explicitPermissionSets }, + developerMessage, ); } } diff --git a/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts b/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts index 2b14e0217d..fb96ab185a 100644 --- a/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts +++ b/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts @@ -76,6 +76,7 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; import { bootStack, type VerifyStack } from '@objectstack/verify'; import { resolveAuthzContext } from '@objectstack/core'; import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; // ── the two objects under probe ──────────────────────────────────────────── @@ -430,10 +431,26 @@ describe('[#7281] checkAuthoredRowWrite answers the declaration, not the caller expect(res.status, 'still refused — the pre-image gate reads as the caller').toBe(403); const envelope: any = await res.json().catch(() => ({})); expect(envelope?.code, 'ADR-0112 error code').toBe('PERMISSION_DENIED'); + // [#7451] Re-spelled against the catalog CONSTANT. The English developer + // sentence this used to match is now `developerMessage`, logged at the + // throw site and never shipped; what a caller reads is the localized + // `record_access_denied` entry (`en` here — this caller declares no + // locale). It still discriminates exactly as before: the sharing + // middleware's refusal is a different code AND a different sentence + // (`FORBIDDEN: insufficient privileges`), so matching the row gate's own + // catalog entry still proves which gate answered. expect( String(envelope?.error ?? ''), 'the row-level gate refused, NOT the sharing middleware (that shape would be `FORBIDDEN: insufficient privileges`)', - ).toContain(`not permitted to update this '${CLOSED}' record (row-level security)`); + ).toBe(BUILTIN_OPERATION_MESSAGES.en.record_access_denied); + // And the developer half stays off the wire — this is the END-TO-END + // measurement behind #7414's decision to log rather than ship it. REST's + // `mapDataError` builds `{ error, code, object? }` and never reads + // `error.details`, so a developer sentence naming the object and the + // row-level gate would reach a browser through nothing but the message. + const serialized = JSON.stringify(envelope ?? {}); + expect(serialized, 'no developer half on the wire').not.toContain('[Security]'); + expect(serialized, 'no developer half on the wire').not.toContain('row-level security'); expect((await rowById(CLOSED, target))?.body, 'the row is untouched').toBe('seed'); }); diff --git a/packages/spec/src/system/operation-message.test.ts b/packages/spec/src/system/operation-message.test.ts index 3bc42bb9ab..4c82529f53 100644 --- a/packages/spec/src/system/operation-message.test.ts +++ b/packages/spec/src/system/operation-message.test.ts @@ -179,3 +179,118 @@ describe('operation message catalog — permission_denied (#7414)', () => { .toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'].permission_denied); }); }); + +/** + * #7451 — the two sentences the END-USER-facing row-level gates render. The + * call sites are pinned in + * `packages/plugins/plugin-security/src/security-denial-user-copy.test.ts`, + * against the real middleware and a real `II18nService`. + * + * `permission_denied` is NOT re-listed here: #7451's third converted gate (the + * capability AND-gate) deliberately reuses it, so its catalog coverage above is + * already the coverage for that gate. + */ +describe('operation message catalog — the row-level user copy (#7451)', () => { + /** + * The vocabulary a business user must never read in a row-level refusal. It + * is the #7414 list plus the two nouns these particular gates leaked: + * `row-level` (the mechanism) and `CHECK` (the clause). + */ + const DEVELOPER_VOCABULARY = [ + 'positions', 'permissionSets', 'permission set', 'capability', + '[Security]', 'Access denied', 'operation', 'row-level', 'CHECK', + ]; + + const KEYS = ['record_access_denied', 'record_change_not_allowed'] as const; + + it('renders the caller locale, not English', () => { + expect(renderOperationMessage({ messageKey: 'record_access_denied' }, { locale: 'zh-CN' })) + .toBe('您无权访问这条记录,如需访问请联系该记录的负责人或管理员。'); + expect(renderOperationMessage({ messageKey: 'record_access_denied' }, { locale: 'en' })) + .toBe('You do not have access to this record. Contact the person who owns it, or your administrator, if you need access.'); + expect(renderOperationMessage({ messageKey: 'record_change_not_allowed' }, { locale: 'zh-CN' })) + .toBe('您无权将这条记录保存为当前填写的内容,请修改后重试,或联系管理员。'); + }); + + it('matches a base language against a regional catalog key (ja → ja-JP)', () => { + for (const key of KEYS) { + expect(renderOperationMessage({ messageKey: key }, { locale: 'ja' })) + .toBe(BUILTIN_OPERATION_MESSAGES['ja-JP'][key]); + } + }); + + it('falls back to the en sentence for a locale the catalog does not carry', () => { + // `de-DE` has no catalog entry and no base-language sibling. + for (const key of KEYS) { + expect(renderOperationMessage({ messageKey: key }, { locale: 'de-DE' })) + .toBe(BUILTIN_OPERATION_MESSAGES.en[key]); + } + }); + + it('names no object, no record id and no authorization mechanism — in EVERY locale', () => { + const locales = Object.keys(BUILTIN_OPERATION_MESSAGES); + // Guard the guard: a catalog that lost its locales would make the loop + // below vacuously true, which is exactly the shape of an assertion that + // cannot fail. + expect(locales.length).toBeGreaterThanOrEqual(4); + for (const locale of locales) { + for (const key of KEYS) { + const rendered = renderOperationMessage({ messageKey: key }, { locale }); + // Non-empty and locale-specific, so the absence assertions below cannot + // be satisfied by an empty string. + expect(rendered).toBe(BUILTIN_OPERATION_MESSAGES[locale][key]); + expect(rendered.length).toBeGreaterThan(10); + for (const word of DEVELOPER_VOCABULARY) { + expect(rendered.toLowerCase(), `${locale}.${key} must not say "${word}"`) + .not.toContain(word.toLowerCase()); + } + } + } + }); + + it('says something DIFFERENT from the grant denial — three situations, three sentences', () => { + // The whole reason these are separate keys: a user blocked by row-level + // security can often ask the record's owner, and a user whose post-image + // failed a CHECK can simply change what they typed. Collapsing them into + // `permission_denied` would send both to an administrator for nothing. + for (const locale of Object.keys(BUILTIN_OPERATION_MESSAGES)) { + const denied = BUILTIN_OPERATION_MESSAGES[locale].permission_denied; + for (const key of KEYS) { + expect(BUILTIN_OPERATION_MESSAGES[locale][key], `${locale}.${key}`).not.toBe(denied); + } + expect(BUILTIN_OPERATION_MESSAGES[locale].record_access_denied) + .not.toBe(BUILTIN_OPERATION_MESSAGES[locale].record_change_not_allowed); + } + }); + + it('ships no unfilled placeholder in any locale — these sentences take no params', () => { + // Asserts on the CATALOG ENTRY, not on the rendering, and that is the + // difference between a guard and a decoration. Rendering a removed key + // yields the bare messageKey — which has no braces either, so a + // rendering-based version of this case would stay green on a catalog that + // lost the key entirely. + for (const [locale, catalog] of Object.entries(BUILTIN_OPERATION_MESSAGES)) { + for (const key of KEYS) { + expect(catalog[key], `${locale} defines ${key}`).toBeTypeOf('string'); + expect(catalog[key], `${locale}.${key} placeholder-free`).not.toMatch(/[{}]/); + } + } + }); + + it('a deployment translation override wins, under the shared `errors.` address', () => { + for (const key of KEYS) { + expect(operationMessageTranslationKey(key)).toBe(`errors.${key}`); + const translate = (k: string) => (k === `errors.${key}` ? '部署自定义文案。' : k); + expect(renderOperationMessage({ messageKey: key }, { locale: 'zh-CN', translate })) + .toBe('部署自定义文案。'); + } + }); + + it('a throwing i18n service does not turn a 403 into a 500', () => { + const translate = () => { throw new Error('service down'); }; + for (const key of KEYS) { + expect(renderOperationMessage({ messageKey: key }, { locale: 'zh-CN', translate })) + .toBe(BUILTIN_OPERATION_MESSAGES['zh-CN'][key]); + } + }); +}); diff --git a/packages/spec/src/system/operation-message.ts b/packages/spec/src/system/operation-message.ts index 88ebf6c902..553545afc1 100644 --- a/packages/spec/src/system/operation-message.ts +++ b/packages/spec/src/system/operation-message.ts @@ -5,12 +5,33 @@ * * The localized message templates for the data path's OPERATION-level * refusals — a write the engine declines as a whole, rather than a constraint - * one field violated. Two members today: the referential-integrity refusal + * one field violated. Members today: the referential-integrity refusal * (`409 DELETE_RESTRICTED`, `cascadeDeleteRelations`'s `restrict` branch, #7307) - * and the object-permission refusal (`403 PERMISSION_DENIED`, plugin-security's - * CRUD gate, #7414). The catalog is the seat for the rest of the family as they - * are localized — a second mechanism for the second producer is exactly what - * this module exists to prevent. + * and three `403 PERMISSION_DENIED` gates in plugin-security — the object CRUD + * grant and the capability AND-gate (#7414, #7451), the row-level pre-image + * write denial and the row-level CHECK post-image denial (#7451). The catalog + * is the seat for the rest of the family as they are localized — a second + * mechanism for the second producer is exactly what this module exists to + * prevent. + * + * ## One key per SITUATION, not per wire code (#7451) + * + * `DELETE_RESTRICTED` already carries two keys, and `PERMISSION_DENIED` now + * carries three. The unit is the situation the USER is in, because that is what + * decides what they can do next: + * + * | Key | The user's situation | What they can do | + * |---|---|---| + * | `permission_denied` | their permissions do not cover this action, on this object, at all | ask an administrator | + * | `record_access_denied` | they may work with this kind of record, but not with THIS one | ask its owner, or an administrator | + * | `record_change_not_allowed` | they may edit this record, but not into the state they just asked for | change what they entered | + * + * The distinction the copy does NOT make is the internal one: a caller blocked + * by a missing CRUD bit and a caller blocked by a missing `requiredPermissions` + * capability are in the SAME situation (their grants do not cover the action; + * an administrator fixes it), so both render `permission_denied`. Which of the + * two gates answered is a developer fact, and it survives verbatim on + * `developerMessage` and in the structured `details` — the place for it. * * ## Why this is a SEPARATE catalog from `validation-message.ts` * @@ -103,11 +124,33 @@ export function operationMessageTranslationKey(messageKey: string): string { * sentence names nothing: no object, no operation, no `positions`. The machine * detail stays on the error's structured `details` and on `developerMessage`, * which is logged server-side (see `plugin-security`'s CRUD gate). + * + * The three `PERMISSION_DENIED` keys take no placeholders EITHER, and #7451 + * re-derived that per site rather than inheriting it (the question "may this + * sentence name anything?" is a per-site judgement, not a family rule): + * + * - `permission_denied` — the #7414 reasoning above, unchanged, and it now + * also covers the capability AND-gate, whose only nameable facts are + * capability IDs: internal authorization vocabulary by construction. + * - `record_access_denied` — the refused record is the one the user just + * acted on, so naming it is the one case here that WOULD be honest. It + * still names nothing, because the only spellings available at the throw + * site are the object's API name and the row's opaque id; a label would + * need the ladder whose last rung is the API name — exactly what must not + * reach a toast (#7414). The user already knows which record they clicked. + * - `record_change_not_allowed` — the gate knows a post-image failed a + * policy predicate, not WHICH field carried the offending value (the + * predicate is an authored expression over the whole row). Naming the + * object without naming the field would send the user hunting. */ export const BUILTIN_OPERATION_MESSAGES: Record> = { en: { permission_denied: 'You do not have permission to perform this action. Contact your administrator if you need access.', + record_access_denied: + 'You do not have access to this record. Contact the person who owns it, or your administrator, if you need access.', + record_change_not_allowed: + 'You are not allowed to save this record with the values you entered. Change them and try again, or contact your administrator if you need access.', delete_restricted: 'This {{object}} is still referenced by {{count}} {{dependentObject}} record(s) through “{{field}}”. Delete or reassign them first.', delete_restricted_required: @@ -115,6 +158,8 @@ export const BUILTIN_OPERATION_MESSAGES: Record> }, 'zh-CN': { permission_denied: '您没有执行此操作的权限,如需访问请联系管理员。', + record_access_denied: '您无权访问这条记录,如需访问请联系该记录的负责人或管理员。', + record_change_not_allowed: '您无权将这条记录保存为当前填写的内容,请修改后重试,或联系管理员。', delete_restricted: '该{{object}}正被 {{count}} 条{{dependentObject}}记录通过「{{field}}」引用,请先删除或改派这些记录。', delete_restricted_required: @@ -122,6 +167,10 @@ export const BUILTIN_OPERATION_MESSAGES: Record> }, 'ja-JP': { permission_denied: 'この操作を実行する権限がありません。アクセスが必要な場合は管理者にお問い合わせください。', + record_access_denied: + 'このレコードにアクセスする権限がありません。アクセスが必要な場合は、レコードの担当者または管理者にお問い合わせください。', + record_change_not_allowed: + '入力された内容ではこのレコードを保存できません。内容を変更して再試行するか、管理者にお問い合わせください。', delete_restricted: 'この{{object}}は {{count}} 件の{{dependentObject}}レコードから「{{field}}」で参照されています。先にそれらを削除するか、参照先を変更してください。', delete_restricted_required: @@ -130,6 +179,10 @@ export const BUILTIN_OPERATION_MESSAGES: Record> 'es-ES': { permission_denied: 'No tiene permiso para realizar esta acción. Póngase en contacto con su administrador si necesita acceso.', + record_access_denied: + 'No tiene acceso a este registro. Póngase en contacto con la persona responsable del registro o con su administrador si necesita acceso.', + record_change_not_allowed: + 'No puede guardar este registro con los valores que ha introducido. Modifíquelos e inténtelo de nuevo o póngase en contacto con su administrador si necesita acceso.', delete_restricted: '{{count}} registro(s) de {{dependentObject}} todavía hacen referencia a este {{object}} mediante «{{field}}». Elimínelos o reasígnelos primero.', delete_restricted_required: From faffac7f1de544a969f17e17d803781894c43789 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 15:25:46 +0000 Subject: [PATCH 2/2] test(service-automation): re-spell the row-level denial pin onto the ADR-0112 envelope (#7451) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's full-suite run found a fifth consumption-radius pin that a phrase grep could not: `rejects.toThrow(/denied|permission/i)`, which was matching the SHAPE of the English copy rather than the refusal. The row-level gate's user half is now a localized catalog sentence containing neither word, so the regex went red on a change that altered nothing it was written to protect. Pinned properly instead: `code` + `statusCode` (never a bare throw), the user half against the catalog constant, and `developerMessage` for the one thing the regex was reaching for — which gate refused. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- .../runas-system-stamping.integration.test.ts | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/services/service-automation/src/runas-system-stamping.integration.test.ts b/packages/services/service-automation/src/runas-system-stamping.integration.test.ts index 0a374051e2..1671325617 100644 --- a/packages/services/service-automation/src/runas-system-stamping.integration.test.ts +++ b/packages/services/service-automation/src/runas-system-stamping.integration.test.ts @@ -40,6 +40,7 @@ import { ObjectQLPlugin, type ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; import { PermissionSetSchema } from '@objectstack/spec/security'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; import { AutomationServicePlugin } from './plugin.js'; import type { AutomationEngine } from './engine.js'; @@ -303,11 +304,29 @@ describe('the #5494 admission flip: row content, not caller, decides (real Secur // difference between the two attempts is the row's stamp columns — the // issue's step-4/5 tell, reproduced in one build. (`owner_only_writes` // matches no one on a NULL `created_by`; the pre-image check fails closed.) + // + // [#7451] Re-spelled from `toThrow(/denied|permission/i)`, and the reason is + // worth recording: that regex was never asserting this refusal, it was + // asserting the SHAPE OF THE ENGLISH COPY, and it matched only because the + // sentence happened to open with "Access denied". The row-level gate's user + // half is now a localized catalog sentence ("You do not have access to this + // record…"), which contains neither word — so the regex went red on a change + // that altered nothing it was written to protect. Pinned properly now: the + // ADR-0112 envelope (`code` + `statusCode`, never a bare throw), the user + // half against the catalog CONSTANT so the next copy edit needs no re-spell, + // and the developer half — which still carries the English sentence verbatim + // — for the one thing the regex was reaching for, namely WHICH gate refused. + const rowLevelDenial = { + code: 'PERMISSION_DENIED', + statusCode: 403, + message: BUILTIN_OPERATION_MESSAGES.en.record_access_denied, + developerMessage: expect.stringContaining('(row-level security)'), + }; await expect( ql.update('crm_task', { id: row.id, status: 'done' }, { context: { ...MEMBER_CTX } }), - ).rejects.toThrow(/denied|permission/i); + ).rejects.toMatchObject(rowLevelDenial); await expect( ql.delete('crm_task', { where: { id: row.id }, context: { ...MEMBER_CTX } }), - ).rejects.toThrow(/denied|permission/i); + ).rejects.toMatchObject(rowLevelDenial); }); });