From fc683f7e9742dd340d24b8b04e16a9316bd6a1e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 11:12:24 +0000 Subject: [PATCH 1/3] fix(plugin-sharing): render the by-id write denial through the operation-message catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sharing middleware's by-id write gate refused with one hardcoded English sentence naming the object's API name and the row's opaque id. `@objectstack/rest` ships it as the 403 body's human-readable `error` and clients show it verbatim, so a user in a fully Chinese deployment read English prose they could not act on. The refusal now renders through the shared Operation Message Catalog in `@objectstack/spec/system` under the `record_write_denied` key that landed for it — the same mechanism plugin-security's record-level denial uses, which is the comparison the report drew. One key serves both write verbs; the verb, object and row id move to `developerMessage`, `details` and the log. `buildSharingMiddleware` gains an optional third argument, a lazily resolved `II18nService.t`-compatible lookup wired by `SharingServicePlugin`, because the i18n service is contributed by another plugin and may start later. Not changed: who may write, the `FORBIDDEN:` prefix the REST layer classifies 403 on, and the ADR-0111 D10 delete-verb diagnostic breadcrumb. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- .changeset/sharing-write-denial-localized.md | 47 ++ .../src/authored-row-write-deferral.test.ts | 25 +- .../plugin-sharing/src/sharing-plugin.ts | 111 ++++- .../src/write-denial-user-copy.test.ts | 423 ++++++++++++++++++ scripts/engine-double-contract.pinned.json | 15 + 5 files changed, 617 insertions(+), 4 deletions(-) create mode 100644 .changeset/sharing-write-denial-localized.md create mode 100644 packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts diff --git a/.changeset/sharing-write-denial-localized.md b/.changeset/sharing-write-denial-localized.md new file mode 100644 index 0000000000..003d3a6aa1 --- /dev/null +++ b/.changeset/sharing-write-denial-localized.md @@ -0,0 +1,47 @@ +--- +"@objectstack/plugin-sharing": minor +--- + +fix(plugin-sharing): the by-id write denial renders through the Operation +Message Catalog instead of a hardcoded English sentence (#12260, the consumer +half of the key #12493 landed) + +A user holding object-level allowRead + allowEdit — and no `modifyAllRecords` — +PATCHed a record they do not own on an object declaring +`sharingModel: 'public_read'` with `access: { default: 'private' }`. The sharing +middleware refused, correctly, and the client showed the server's reason +verbatim to the end user: one hardcoded English sentence naming the object's API +name and the row's opaque id. In a fully Chinese deployment that was the only +thing the user was told about why their save failed. + +The refusal now renders through the shared Operation Message Catalog in +`@objectstack/spec/system` under the key `record_write_denied` that #12493 +landed for it — the same mechanism `plugin-security`'s record-level denial +already uses, which is exactly the comparison the report drew: the same "I can +see this record but cannot change it" situation showed human language or raw +English depending on which layer refused. Same resolution ladder (deployment +override → the caller's locale → `en` → the key), 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 sends the reader to the +record's owner or an administrator instead of dead-ending them. + +`record_write_denied` is deliberately not `record_access_denied`: this gate +fires on a row the READ path already admitted, so "You do not have access to +this record" would be false the moment it rendered. It is one key for BOTH write +verbs — the user's situation and remedy are identical for `update` and `delete`. + +`buildSharingMiddleware` gains an optional third argument, a lazily resolved +`II18nService.t`-compatible lookup wired by `SharingServicePlugin`, because the +i18n service is contributed by another plugin and may start later. It is what +makes the override address the catalog documents, +`errors.record_write_denied`, take effect for this emitter. The argument is +additive: every existing caller passes two and is unchanged, and a stack with no +i18n service still renders the built-in catalog in the caller's locale. + +**Not changed: who may write.** The gate is byte-identical — ownership, write +depth, an edit-level share for `update`, Modify All Data — and the app-authored +RLS deferral ahead of it is untouched. The `FORBIDDEN:` prefix the REST layer +classifies 403 on is untouched, and so is the ADR-0111 D10 `delete`-verb +diagnostic breadcrumb. The verb, object and row id the old sentence carried are +now developer facts on the error's `developerMessage` and `details` and in the +log, where a developer reads them and a user never does. diff --git a/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts b/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts index 123318e92c..06e0c2b920 100644 --- a/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts +++ b/packages/plugins/plugin-sharing/src/authored-row-write-deferral.test.ts @@ -11,6 +11,13 @@ // middleware's, not the row-gate's `(row-level security)` — so the refusal // landed BEFORE RLS was consulted and the declared widener was never asked. // +// [#12260] That English sentence is HISTORY as of this card: the refusal's +// user-facing half now renders from the Operation Message Catalog +// (`record_write_denied`) and the verb/object/id it used to name moved to +// `developerMessage`. The tell is unchanged in substance — `[sharing] …` vs +// the row gate's `(row-level security)` — only its channel moved. See +// `write-denial-user-copy.test.ts`. +// // The discriminator is not "carries sharing rules" (#5493's own wording) but // **whether record sharing enforces on the object at all** (round-2 refinement, // issue comment 5226364929): `checkEdit` abstains — and `canEdit` therefore @@ -52,6 +59,7 @@ // and that everything that is not a literal `admit` leaves the refusal intact. import { describe, it, expect, beforeEach, vi } from 'vitest'; import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/objectql'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; import { SharingService, type SharingSecurityProbe } from './sharing-service.js'; import { buildSharingMiddleware } from './sharing-plugin.js'; @@ -311,6 +319,8 @@ interface WriteOutcome { code?: string; status?: number; message: string; + /** [#12260] The developer half the user-facing sentence no longer carries. */ + developerMessage?: string; } interface Stack { @@ -376,7 +386,10 @@ function makeStack(opts: { reached = true; }); } catch (e: any) { - return { ok: false, code: e?.code, status: e?.status, message: String(e?.message ?? e) }; + return { + ok: false, code: e?.code, status: e?.status, + message: String(e?.message ?? e), developerMessage: e?.developerMessage, + }; } return reached ? { ok: true, message: 'written' } @@ -405,7 +418,15 @@ function expectSharingRefusal(out: WriteOutcome, operation: 'update' | 'delete', expect(out.ok, `expected a refusal, got a completed ${operation}`).toBe(false); expect(out.code, 'ADR-0112 error code').toBe('FORBIDDEN'); expect(out.status, 'ADR-0112 HTTP status').toBe(403); - expect(out.message).toContain(`FORBIDDEN: insufficient privileges to ${operation} ${object} ${id}`); + // [#12260] The SENTENCE moved onto the Operation Message Catalog (key + // `record_write_denied`), so the discriminator this file turns on moved with + // it: the verb, the object's API name and the row id are now developer copy. + // Both halves are asserted, because both are how this refusal is told apart + // from `plugin-security`'s row gate — which answers `PERMISSION_DENIED` with + // its own `(row-level security)` breadcrumb and never writes `[sharing]`. + // The `FORBIDDEN:` prefix is wire contract and is unchanged. + expect(out.message).toBe(`FORBIDDEN: ${BUILTIN_OPERATION_MESSAGES.en.record_write_denied}`); + expect(out.developerMessage).toContain(`[sharing] ${operation} denied on ${object} ${id}`); } const rowById = (stack: Stack, object: string, id: string) => diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index 0e38beb6d5..80c9911d24 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -22,6 +22,12 @@ import type { ExecutionContext } from '@objectstack/spec/kernel'; // mark `plugin-security` and `service-analytics` stamp at theirs — never a // local flag, never a second spelling of the same idea. import { markFilterSubtreeProvenance } from '@objectstack/spec/data'; +// [#12260] 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 middleware's key +// (`record_write_denied`) into it ahead of this consumer half. +import { renderOperationMessage, type ValidationMessageTranslator } from '@objectstack/spec/system'; import { SysRecordShare, SysSharingRule, SysShareLink } from './objects/index.js'; import { SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity'; import { @@ -610,7 +616,7 @@ export class SharingServicePlugin implements Plugin { if (this.options.enforce === false) { ctx.logger.info('SharingServicePlugin: enforcement disabled (enforce=false) — share-link service still registered'); } else { - const mw = buildSharingMiddleware(this.service, ctx.logger as any); + const mw = buildSharingMiddleware(this.service, ctx.logger as any, pluginMessageTranslator(ctx)); if (typeof engine.registerMiddleware === 'function') { engine.registerMiddleware(mw, { object: '*' }); ctx.logger.info('SharingServicePlugin: enforcement middleware installed'); @@ -954,6 +960,73 @@ export class SharingServicePlugin implements Plugin { } } +/** + * [#12260] The END USER's half of the by-id write refusal. + * + * This middleware's refusal declares `{ code: 'FORBIDDEN', status: 403 }`, so + * `@objectstack/rest` answers it through the DECLARED-status arm and ships + * `error.message` to the client as the body's human-readable `error` — which + * Console renders verbatim in a toast. One hardcoded English sentence + * therefore reached a business user in a fully Chinese deployment as the only + * thing they were told about why their save failed. + * + * 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. `plugin-security`'s `userFacingDenialMessage` + * is the sibling consumer this mirrors, and `plugin-approvals`' + * `userFacingRefusal` (#11993) is the same conversion one card earlier. + * + * ⛔ ONE key for BOTH write verbs, which is the catalog's own ruling and not a + * shortcut taken here: the user's situation (they can see this record, they + * cannot change it) and their remedy (ask its owner, or an administrator) are + * identical for `update` and `delete`. WHICH verb was refused is a developer + * fact and stays on `developerMessage`, on the structured `details`, and — for + * `delete` — on the ADR-0111 D10 breadcrumb that keeps its own wording. + * + * The `FORBIDDEN:` prefix is NOT part of what this renders. It is wire + * contract (ADR-0111's `CODE: message` idiom, which the share routes read and + * strip) and it is applied by the caller around this sentence. + * + * The translator is resolved LAZILY, per refusal, for the reason ADR-0029 D8 + * makes structural: the i18n service is contributed by a different plugin that + * may start after this one, so a lookup captured when the middleware was built + * would pin `undefined` for the life of the process. Absent is a SUPPORTED + * stack, not a degraded one — the built-in catalog still renders the caller's + * locale; what the translator adds is the documented override address + * `errors.record_write_denied`. + */ +function userFacingWriteDenial( + locale: string | undefined, + messageTranslator?: () => ValidationMessageTranslator | undefined, +): string { + let translate: ValidationMessageTranslator | undefined; + try { + translate = 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: 'record_write_denied' }, { locale, translate }); +} + +/** + * [#12260] The deployment i18n lookup this plugin hands its middleware, read + * through `PluginContext` on every refusal rather than captured at start(). + * See {@link userFacingWriteDenial} for why late binding is the requirement + * and not a defensive habit. + */ +function pluginMessageTranslator(ctx: PluginContext): () => ValidationMessageTranslator | undefined { + return () => { + const i18n = ctx.getService('i18n'); + const t = i18n?.t; + if (typeof t !== 'function') return undefined; + return (key: string, loc: string, params?: Record) => t.call(i18n, key, loc, params); + }; +} + /** * Build the engine middleware that injects read filters and gates * write operations. Exported so it can be unit-tested without booting @@ -971,6 +1044,15 @@ export class SharingServicePlugin implements Plugin { export function buildSharingMiddleware( service: SharingService, log?: { warn?: (msg: string, meta?: any) => void }, + /** + * [#12260] Deployment i18n lookup for the by-id write refusal's user-facing + * sentence — an `II18nService.t`-compatible function, resolved LAZILY per + * refusal. Optional and additive: every existing caller (six suites in + * `plugin-security`, two here) passes two arguments and is unchanged, and a + * stack without it still renders the caller's locale from the built-in + * catalog. See {@link userFacingWriteDenial}. + */ + messageTranslator?: () => ValidationMessageTranslator | undefined, ): EngineMiddleware { return async function sharingMiddleware(ctx: OperationContext, next: () => Promise) { const op = ctx.operation; @@ -1116,11 +1198,36 @@ export function buildSharingMiddleware( { object: ctx.object, recordId: String(id), userId: exec?.userId }, ); } + // [#12260] The DEVELOPER's half — the verb, the object's API name and + // the row id. This USED TO BE the whole message, which is how it + // reached an end user's toast in English; the catalog sentence + // deliberately names none of it (the only spellings available here + // are an API name and an opaque id, the #7414 vocabulary that must + // not reach a toast). It is kept where a developer reads it and a + // user never does: on the error, and in the log line below. REST + // ships neither `developerMessage` nor `details` on a FORBIDDEN + // body — only `DELETE_RESTRICTED` forwards a `developerMessage` — + // so this adds nothing to the wire. + const developerMessage = + `[sharing] ${verb} denied on ${ctx.object} ${id}: the caller holds no ${verb} authority ` + + `over this row (owner match, share depth and Modify All Data all answered no)`; + log?.warn?.(developerMessage, { + object: ctx.object, + recordId: String(id), + operation: verb, + userId: exec?.userId, + }); + // The `FORBIDDEN:` PREFIX STAYS. It is not user copy — it is the + // ADR-0111 `CODE: message` idiom the share routes read and strip, + // and it sits beside the `code`/`status` the `/data` door + // classifies on. Only the SENTENCE after it moved. const err: any = new Error( - `FORBIDDEN: insufficient privileges to ${op} ${ctx.object} ${id}`, + `FORBIDDEN: ${userFacingWriteDenial(exec?.locale, messageTranslator)}`, ); err.code = 'FORBIDDEN'; err.status = 403; + err.developerMessage = developerMessage; + err.details = { operation: verb, object: ctx.object, recordId: String(id) }; throw err; } return next(); diff --git a/packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts b/packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts new file mode 100644 index 0000000000..b31253f25a --- /dev/null +++ b/packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts @@ -0,0 +1,423 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The END USER's half of the sharing middleware's by-id WRITE refusal (#12260). + * + * The report, on `@objectstack/*@17.2.0`: an object declaring + * `sharingModel: 'public_read'` with `access: { default: 'private' }`; a user + * holding object-level allowRead + allowEdit and NO `modifyAllRecords`; a by-id + * PATCH against a record they do not own. The middleware refuses — correctly — + * and the client (an H5 / in-house front end) shows the server's `message` + * verbatim to the end user. That message was one hardcoded English sentence + * naming an API name and an opaque row id. + * + * The comparison the reporter drew is exact: `plugin-security`'s record-level + * denial already renders localized copy through the catalog + * (`userFacingDenialMessage`), so the SAME user situation — "I can't write this + * record" — showed human language or raw English depending on which layer + * refused. + * + * The refusal now renders through the shared Operation Message Catalog + * (`@objectstack/spec/system`, key `record_write_denied`, landed ahead of this + * consumer half by #12493) instead of a package-local string. + * + * ⚠️ These tests assert the SENTENCE A USER READS, in zh-CN specifically, as a + * LITERAL. Asserting only that a catalog key was passed — or comparing the + * render against the catalog it came from — would pass against a message that + * still renders in English, which is the entire reported defect. + * + * They also pin the three things the conversion must NOT move: + * - the `FORBIDDEN:` code prefix. It is not user copy: it is the ADR-0111 + * `CODE: message` idiom the share routes read and strip, and it rides + * beside the `code`/`status` the `/data` door classifies 403 on; + * - the ADR-0111 D10 `delete`-verb diagnostic breadcrumb, which is a + * developer-facing greppable reason for the D3 tightening and is + * deliberately separate from user copy; + * - WHO may write. The gate is byte-identical; only its sentence changed. + * + * ⭐ Why `record_write_denied` and not `record_access_denied`, quoted from the + * catalog's own header because it is the reason this key exists: + * + * > the sharing middleware's by-id write gate fires on a row the READ path + * > already admitted — the user is typically looking at the record it + * > refuses — so "You do not have access to this record" would be false the + * > moment it rendered. The situation is read-yes/write-no. + * + * The read-yes half is measured here too (§4), so the claim is a fact about + * this fixture rather than a quotation about it. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system'; +import { + assertEngineDeleteDispatch, + assertEngineUpdateDispatch, + assertEngineFindOnePredicate, +} from '@objectstack/objectql'; +import { SharingService } from './sharing-service.js'; +import { buildSharingMiddleware } from './sharing-plugin.js'; + +// ── the reported fixture ──────────────────────────────────────────────────── + +/** + * The reporter's object, restated: `public_read` OWD (everyone reads, the owner + * writes) plus an `owner_id` column, so record sharing really enforces and + * `checkEdit` / `checkDelete` can answer `deny`. + * + * `access: { default: 'private' }` is carried for fidelity to the report; it is + * `plugin-security`'s object-CRUD axis and this middleware never reads it. The + * user's allowRead + allowEdit live on that same axis — they are what makes the + * READ succeed and the WRITE reach this gate at all. + */ +const INQUIRY_SCHEMA = { + name: 'os_inquiry', + sharingModel: 'public_read', + access: { default: 'private' }, + fields: { + id: { name: 'id' }, + subject: { name: 'subject' }, + owner_id: { name: 'owner_id' }, + created_by: { name: 'created_by' }, + organization_id: { name: 'organization_id' }, + }, +}; + +const SCHEMAS: Record = { os_inquiry: INQUIRY_SCHEMA }; + +const U_OWNER = 'u_owner'; +/** The reporting deployment's user: allowRead + allowEdit, no modifyAllRecords. */ +const U_AGENT = 'u_agent'; + +/** The row the report PATCHes: owned by someone else. */ +const INQUIRY_THEIRS = { + id: 'inq_theirs', subject: 'shipping delay', + owner_id: U_OWNER, created_by: U_OWNER, organization_id: 'org1', +}; +/** The agent's own row — the positive control ownership must keep admitting. */ +const INQUIRY_MINE = { + id: 'inq_mine', subject: 'refund', + owner_id: U_AGENT, created_by: U_AGENT, organization_id: 'org1', +}; + +// ── in-memory engine ──────────────────────────────────────────────────────── + +/** + * Both write verbs open with the PRODUCER's own dispatch predicate + * (#4550 / #5480), never a hand-mirrored guard: a double looser than the engine + * it replaces converts a green suite into no suite at all. + */ +function makeEngine() { + const tables: Record = { + os_inquiry: [{ ...INQUIRY_THEIRS }, { ...INQUIRY_MINE }], + sys_record_share: [], + }; + const matches = (row: any, filter: any): boolean => { + if (!filter || typeof filter !== 'object') return true; + if (Array.isArray(filter.$or) && !filter.$or.some((f: any) => matches(row, f))) return false; + if (Array.isArray(filter.$and) && !filter.$and.every((f: any) => matches(row, f))) return false; + for (const [k, v] of Object.entries(filter)) { + if (k === '$or' || k === '$and') continue; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(row[k])) return false; + continue; + } + if (row[k] !== v) return false; + } + return true; + }; + return { + _tables: tables, + getSchema: (name: string) => SCHEMAS[name], + async find(object: string, options: any = {}) { + const rows = (tables[object] ??= []); + return rows.filter((r) => matches(r, options.filter ?? options.where)).slice(0, options.limit ?? 1000); + }, + async findOne(object: string, options: any = {}) { + assertEngineFindOnePredicate(object, options); + const rows = await this.find(object, { ...options, limit: 1 }); + return rows[0] ?? null; + }, + async insert(object: string, data: any) { + (tables[object] ??= []).push({ ...data }); + return data; + }, + 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 WriteOutcome { + ok: boolean; + /** ADR-0112 envelope of the refusal — asserted, never a bare `toThrow()`. */ + code?: string; + status?: number; + message: string; + developerMessage?: string; + details?: any; +} + +function makeStack(messageTranslator?: () => any) { + const engine = makeEngine(); + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const sharing = new SharingService({ engine: engine as any, logger }); + const mw = buildSharingMiddleware(sharing, logger, messageTranslator) as any; + + return { + engine, + logger, + rows: (object: string) => (engine._tables[object] ??= []), + /** Drive a by-id write through the middleware and report its ENVELOPE. */ + async write( + operation: 'update' | 'delete', + recordId: string, + context: any, + ): Promise { + const opCtx: any = { + object: 'os_inquiry', + operation, + context: { ...context }, + ...(operation === 'update' + ? { data: { id: recordId, subject: 'edited' } } + : { options: { where: { id: recordId } } }), + }; + let reached = false; + try { + await mw(opCtx, async () => { + if (operation === 'delete') await engine.delete(opCtx.object, opCtx.options); + else await engine.update(opCtx.object, opCtx.data, opCtx.options); + reached = true; + }); + } catch (e: any) { + return { + ok: false, + code: e?.code, + status: e?.status, + message: String(e?.message ?? e), + developerMessage: e?.developerMessage, + details: e?.details, + }; + } + return reached + ? { ok: true, message: 'written' } + : { ok: false, message: 'middleware swallowed the write' }; + }, + /** The READ half of read-yes/write-no, through the same middleware. */ + async read(context: any) { + const opCtx: any = { object: 'os_inquiry', operation: 'find', context: { ...context }, ast: {} }; + await mw(opCtx, async () => {}); + return engine.find('os_inquiry', { filter: opCtx.ast?.where ?? opCtx.ast?.filter }); + }, + }; +} + +/** The execution-context shape `resolveAuthzContext` hands the middleware. */ +const ctxFor = (userId: string, locale?: string) => ({ + userId, tenantId: 'org1', positions: ['org_member'], permissions: [], + ...(locale ? { locale } : {}), +}); + +/** The reporting deployment: Console and the H5 client both set zh-CN. */ +const AGENT_ZH = ctxFor(U_AGENT, 'zh-CN'); +const AGENT_EN = ctxFor(U_AGENT, 'en'); +const OWNER_ZH = ctxFor(U_OWNER, 'zh-CN'); + +/** + * The zh-CN copy a user 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, which is the whole defect. Its twin lives + * in `packages/spec/src/system/operation-message.test.ts`; the two move + * together. + */ +const ZH_SENTENCE = '您无权修改或删除这条记录,如需修改请联系该记录的负责人或管理员。'; + +/** The legacy hardcoded reason, kept as a literal so its ABSENCE is pinned. */ +const LEGACY_EN = 'insufficient privileges to'; + +/** What the REST layer does with an ADR-0111 `CODE: message` throw. */ +const WIRE_CODE = (msg: string) => /^FORBIDDEN/.test(msg); +const WIRE_ERROR = (msg: string) => msg.replace(/^[A-Z_]+:\s*/, ''); + +// ─────────────────────────────────────────────────────────────────────────── + +describe('[#12260] the by-id write denial renders through the operation catalog', () => { + let stack: ReturnType; + beforeEach(() => { stack = makeStack(); }); + + // ── §1 the reported symptom ────────────────────────────────────────────── + + it('the report, reproduced: a zh-CN user PATCHing a row they do not own reads Chinese', async () => { + const out = await stack.write('update', INQUIRY_THEIRS.id, AGENT_ZH); + + expect(out.ok, 'the gate must still refuse — this card moves copy, not authority').toBe(false); + expect(out.message).toBe(`FORBIDDEN: ${ZH_SENTENCE}`); + // The half a client shows its end user carries no Latin prose. Before this + // conversion it was an entire English sentence naming an API name and a row id. + expect(WIRE_ERROR(out.message)).toBe(ZH_SENTENCE); + expect(WIRE_ERROR(out.message)).not.toMatch(/[A-Za-z]/); + }); + + it('the DELETE verb reads the same sentence — one key serves both write verbs', async () => { + const out = await stack.write('delete', INQUIRY_THEIRS.id, AGENT_ZH); + + expect(out.ok).toBe(false); + expect(out.message).toBe(`FORBIDDEN: ${ZH_SENTENCE}`); + expect(WIRE_ERROR(out.message)).not.toMatch(/[A-Za-z]/); + expect(stack.rows('os_inquiry').map((r) => r.id), 'the row survives').toContain(INQUIRY_THEIRS.id); + }); + + it('no longer emits the legacy hardcoded English reason, on either verb', async () => { + for (const verb of ['update', 'delete'] as const) { + const out = await stack.write(verb, INQUIRY_THEIRS.id, AGENT_EN); + expect(out.message, verb).not.toContain(LEGACY_EN); + expect(out.message, verb).not.toContain(INQUIRY_THEIRS.id); + expect(out.message, verb).not.toContain('os_inquiry'); + } + }); + + // ── §2 the wire contract the sentence must not shadow ──────────────────── + + it('the `FORBIDDEN:` prefix survives the conversion and still strips clean', async () => { + const out = await stack.write('update', INQUIRY_THEIRS.id, AGENT_ZH); + + // The prefix is wire contract, not copy. `FORBIDDEN: 您无权…` still matches + // the ADR-0111 prefix idiom, and stripping it leaves the sentence alone — + // no second prefix, no residue. + expect(WIRE_CODE(out.message)).toBe(true); + expect(WIRE_ERROR(out.message).startsWith('FORBIDDEN')).toBe(false); + expect(WIRE_ERROR(out.message)).toBe(ZH_SENTENCE); + }); + + it('the ADR-0112 envelope is unchanged — REST still answers 403 FORBIDDEN', async () => { + for (const verb of ['update', 'delete'] as const) { + const out = await stack.write(verb, INQUIRY_THEIRS.id, AGENT_ZH); + expect(out.code, `${verb}: ADR-0112 error code`).toBe('FORBIDDEN'); + expect(out.status, `${verb}: ADR-0112 HTTP status`).toBe(403); + } + }); + + // ── §3 the catalog mechanism ───────────────────────────────────────────── + + it('renders each platform locale from the catalog, not one hardcoded sentence', async () => { + for (const locale of ['en', 'ja-JP', 'es-ES'] as const) { + const out = await stack.write('update', INQUIRY_THEIRS.id, ctxFor(U_AGENT, locale)); + expect(WIRE_ERROR(out.message), locale) + .toBe(BUILTIN_OPERATION_MESSAGES[locale].record_write_denied); + } + }); + + it('an unknown locale falls back to English rather than to the bare key', async () => { + const out = await stack.write('update', INQUIRY_THEIRS.id, ctxFor(U_AGENT, 'kl-GL')); + expect(WIRE_ERROR(out.message)).toBe(BUILTIN_OPERATION_MESSAGES.en.record_write_denied); + expect(WIRE_ERROR(out.message)).not.toBe('record_write_denied'); + }); + + it('a context carrying NO locale still renders English copy, not the old sentence', async () => { + const out = await stack.write('update', INQUIRY_THEIRS.id, ctxFor(U_AGENT)); + expect(WIRE_ERROR(out.message)).toBe(BUILTIN_OPERATION_MESSAGES.en.record_write_denied); + }); + + it('a deployment `translation` for `errors.record_write_denied` wins', async () => { + const seen: string[] = []; + const s = makeStack(() => (key: string, locale: string) => { + seen.push(`${key}@${locale}`); + return key === 'errors.record_write_denied' && locale === 'zh-CN' + ? '这条工单不归你负责,请联系负责人。' + : key; // II18nService echoes the key back on a miss. + }); + + const out = await s.write('update', INQUIRY_THEIRS.id, AGENT_ZH); + expect(seen).toContain('errors.record_write_denied@zh-CN'); + expect(WIRE_ERROR(out.message)).toBe('这条工单不归你负责,请联系负责人。'); + expect(out.status, 'still a refusal, still 403').toBe(403); + }); + + it('a misbehaving i18n service degrades to the built-in copy, never to a 500', async () => { + const s = makeStack(() => { throw new Error('i18n exploded'); }); + const out = await s.write('update', INQUIRY_THEIRS.id, AGENT_ZH); + + // Still the refusal, still 403-shaped — not the i18n service's error. + expect(out.message).toBe(`FORBIDDEN: ${ZH_SENTENCE}`); + expect(out.code).toBe('FORBIDDEN'); + expect(out.status).toBe(403); + }); + + // ── §4 the developer's half, and read-yes/write-no ─────────────────────── + + it('the developer facts move to `developerMessage`, `details` and the log', async () => { + const out = await stack.write('update', INQUIRY_THEIRS.id, AGENT_ZH); + + // Everything the sentence deliberately does not name is still legible — + // just nowhere a user can read it. + expect(out.developerMessage).toContain('os_inquiry'); + expect(out.developerMessage).toContain(INQUIRY_THEIRS.id); + expect(out.developerMessage).toContain('update'); + expect(out.details).toMatchObject({ + operation: 'update', object: 'os_inquiry', recordId: INQUIRY_THEIRS.id, + }); + expect(stack.logger.warn).toHaveBeenCalledWith( + expect.stringContaining('update denied on os_inquiry'), + expect.objectContaining({ operation: 'update', object: 'os_inquiry', userId: U_AGENT }), + ); + }); + + it('the ADR-0111 D10 delete breadcrumb still fires, in its own words', async () => { + await stack.write('delete', INQUIRY_THEIRS.id, AGENT_ZH); + + // The D3 tightening's greppable reason is developer copy and is deliberately + // separate from the user's sentence — it must survive this conversion intact. + expect(stack.logger.warn).toHaveBeenCalledWith( + expect.stringContaining('an edit-level share does not grant delete'), + expect.anything(), + ); + }); + + it('read-yes/write-no: the same user reads the very row the write refuses', async () => { + // This is why the key is `record_write_denied` and not `record_access_denied` + // — "You do not have access to this record" would be false the moment it + // rendered, because the user is looking at the record. + const visible = await stack.read(AGENT_ZH); + expect(visible.map((r: any) => r.id)).toContain(INQUIRY_THEIRS.id); + + const out = await stack.write('update', INQUIRY_THEIRS.id, AGENT_ZH); + expect(out.ok).toBe(false); + }); + + // ── §5 WHO may write is unchanged (the permission boundary) ────────────── + + it('the owner still updates their own row', async () => { + const out = await stack.write('update', INQUIRY_MINE.id, ctxFor(U_AGENT, 'zh-CN')); + expect(out, out.message).toMatchObject({ ok: true }); + expect(stack.rows('os_inquiry').find((r) => r.id === INQUIRY_MINE.id)?.subject).toBe('edited'); + }); + + it('the owner still deletes their own row', async () => { + const out = await stack.write('delete', INQUIRY_MINE.id, ctxFor(U_AGENT, 'zh-CN')); + expect(out, out.message).toMatchObject({ ok: true }); + expect(stack.rows('os_inquiry').map((r) => r.id)).not.toContain(INQUIRY_MINE.id); + }); + + it("a non-owner is still refused on the OWNER's row, and the row is untouched", async () => { + const out = await stack.write('update', INQUIRY_MINE.id, OWNER_ZH); + expect(out.ok).toBe(false); + expect(out.status).toBe(403); + expect(stack.rows('os_inquiry').find((r) => r.id === INQUIRY_MINE.id)?.subject).toBe('refund'); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index b251850736..3c69a2ab56 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2641,6 +2641,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-sharing/src/write-denial-user-copy.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts", "verb": "findOne", From ae4cfcce7d9cd954179c1d4d4d7eb930df808b86 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 11:42:43 +0000 Subject: [PATCH 2/3] docs(skills): stop printing a refusal string the sharing gate no longer emits `data-hooks.md` quoted the by-id write gate's message verbatim. That sentence is now end-user copy rendered in the caller's locale, so a hook author reading the doc would string-match prose that varies by locale. The fence names the shape and points at the stable channel (the error's code) instead. Token-neutral by construction: the published-bundle ratchet reads 12611 against a ceiling of 12611 (+0), so this correction spends no context budget. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- skills/objectstack-data/references/data-hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/objectstack-data/references/data-hooks.md b/skills/objectstack-data/references/data-hooks.md index e3ecc47293..2df6c8aa80 100644 --- a/skills/objectstack-data/references/data-hooks.md +++ b/skills/objectstack-data/references/data-hooks.md @@ -495,7 +495,7 @@ whoever is elevated. If the acting user cannot edit the target (e.g. it is `public_read`), the write throws: ``` -FORBIDDEN: insufficient privileges to update +FORBIDDEN: ``` **An admin is not automatically exempt** — the gate is `canEdit`, driven by the From e6ec9a9e5cc1c5673f9bd0aa4e5c30c8fbdc6bf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 12:02:23 +0000 Subject: [PATCH 3/3] Revert "docs(skills): stop printing a refusal string the sharing gate no longer emits" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit ae4cfcce7d9cd954179c1d4d4d7eb930df808b86. Not a change of mind about the edit — it is correct and necessary, and it lands unchanged in its own PR. `skills/**` is a governed surface on the `GOVERNED_SURFACES` register in scripts/pm/check-governed-merges.mjs, and Prime Directive #14 judges a PR on its FILE LIST, not its description: a mixed diff is not a proportion question, one path hit forks the whole PR and reserves the landing for a human. Keeping the doc here would have made this branch unmergeable by anything but a hand merge. The doc half now rides claude/issue-12260-skills-data-hooks-prose, with the replacement text byte-identical. The two want landing together; until the doc one lands, data-hooks.md briefly prints a string the gate no longer emits. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5 --- skills/objectstack-data/references/data-hooks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skills/objectstack-data/references/data-hooks.md b/skills/objectstack-data/references/data-hooks.md index 2df6c8aa80..e3ecc47293 100644 --- a/skills/objectstack-data/references/data-hooks.md +++ b/skills/objectstack-data/references/data-hooks.md @@ -495,7 +495,7 @@ whoever is elevated. If the acting user cannot edit the target (e.g. it is `public_read`), the write throws: ``` -FORBIDDEN: +FORBIDDEN: insufficient privileges to update ``` **An admin is not automatically exempt** — the gate is `canEdit`, driven by the