From 4bf10901af5f8391c0c716b792e99479616332b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 16:53:22 +0000 Subject: [PATCH 1/2] fix(plugin-security): the packaged-permission-set lock refusal carries its guidance as userMessage PackagedPermissionSetLockedError (insert and update) and the fail-closed PackagedPermissionSetProvenanceUnknownError now declare a readonly userMessage: the guidance addressed to the end user, read at every HTTP door through declaredUserMessage and rendered verbatim by the console in place of its generic 403 sentence. code, status and message are unchanged; the texts carry no set name, package id or API path. The wire-envelope pin drives the real data-door write-through and maps the thrown error through mapDataError (the REST /data door's own call) and resolveThrownHttpError (the dispatcher's), plus the metadata door's registered lock gate. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN --- .changeset/21794-lock-refusal-user-message.md | 13 ++ .../src/packaged-permission-set-lock.test.ts | 153 +++++++++++++++++- .../src/packaged-permission-set-lock.ts | 54 ++++++- 3 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 .changeset/21794-lock-refusal-user-message.md diff --git a/.changeset/21794-lock-refusal-user-message.md b/.changeset/21794-lock-refusal-user-message.md new file mode 100644 index 0000000000..f1088750cc --- /dev/null +++ b/.changeset/21794-lock-refusal-user-message.md @@ -0,0 +1,13 @@ +--- +'@objectstack/plugin-security': minor +--- + +The packaged-permission-set lock refusal carries its guidance as `userMessage`, so the console tells the admin to clone the set instead of showing its generic "You don't have permission to save this record." (#21794). + +Clause-②: yes (widening) + +- **`PackagedPermissionSetLockedError.userMessage`**, a new `readonly` member. A save that targets a permission set an installed package ships still answers `403 NOT_OVERRIDABLE` with the same `message`. That holds at the data door (`PATCH` / `POST /api/v1/data/sys_permission_set`) on every kernel. It also holds at the metadata door (`PUT /api/v1/meta/permission/:name`) on a kernel with no environment id, such as a self-hosted app server, where this lock is the refusal that answers. On an environment kernel the metadata protocol's own package-door refusal answers that `PUT` first, and it is unchanged. The error envelope now also carries `userMessage`, the field `ApiErrorSchema` already declares and the console renders verbatim. An edit is told to clone the set with the Clone action and edit the clone. A new set named like a packaged one is told to choose a different name, or clone. +- **`PackagedPermissionSetProvenanceUnknownError.userMessage`**, the same member on the fail-closed refusal (the platform could not tell whether a package ships the set). It tells the admin to try again, or clone. The unreadable source stays in `message`. +- The texts name no set, package, id or API path; `message` keeps that diagnostic for logs and developers. They are English, like every platform refusal. + +Nothing that was accepted is refused now, and nothing that was refused is accepted. Both refusals keep their `code`, `status` and `message`. diff --git a/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts b/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts index 6947f61f78..bae48395ac 100644 --- a/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts +++ b/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts @@ -76,8 +76,13 @@ import { describe, it, expect } from 'vitest'; import { PermissionSetSchema } from '@objectstack/spec/security'; import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { mapDataError, resolveThrownHttpError } from '@objectstack/types'; import { SysPermissionSet } from './objects/sys-permission-set.object.js'; -import { classifyPackagedPermissionSet } from './packaged-permission-set-lock.js'; +import { + classifyPackagedPermissionSet, + PackagedPermissionSetLockedError, + PackagedPermissionSetProvenanceUnknownError, +} from './packaged-permission-set-lock.js'; import { registerPackagedPermissionSetLockGate } from './packaged-permission-set-lock-gate.js'; import { createPermissionSetWriteThrough, @@ -985,3 +990,149 @@ describe('[#21789] the lock judges "shipped by a code package" from the row\'s p .rejects.toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 }); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// The refusal's guidance reaches the END USER — `userMessage` on the wire +// ───────────────────────────────────────────────────────────────────────────── + +/** + * The console renders a refusal's `userMessage` verbatim and substitutes its + * generic "You don't have permission to save this record." for every 403 that + * carries none (the objectui#5210 producer-side opt-in; `declaredUserMessage` + * in `@objectstack/types` is the one read every door applies). An unmarked + * lock refusal therefore told the admin nothing about the clone path, which is + * the whole point of the ruling the lock implements. + * + * The wire envelope is computed by the producer's own mapping, not restated: + * + * - `mapDataError` is the call the REST `/data` door makes in its catch + * (`PATCH` and `POST /data/:object[/:id]`, `rest-server.ts`) — the door that + * serves Setup's save, measured on a booted showcase as the flat + * `{ error, code, object }` body; + * - `resolveThrownHttpError` is the dispatcher door's resolution of the same + * throw. + * + * Every case asserts the envelope (`status` + `code`) AND the `userMessage`: + * the code and status are the refusal's identity and must not move, and the + * marked text is what this block is about. The text itself is not pinned word + * for word — what is pinned is what the end user must and must not be shown: + * the guidance (clone it), and none of the diagnostic's machine names, package + * ids or API paths (the console's friendly-copy rule, objectstack#3821). + */ +describe('the lock refusal carries its guidance as `userMessage` on the wire envelope', () => { + const PACKAGE_ID = 'com.example.ehr'; + const SET_NAME = 'ehr_quality_inspector'; + + /** End-user copy: a non-empty string carrying none of the diagnostic's machine identifiers. */ + const expectEndUserCopy = (text: unknown, ...machineNames: string[]) => { + expect(typeof text, 'userMessage must be present on the wire').toBe('string'); + expect(String(text).trim().length).toBeGreaterThan(0); + for (const machineName of machineNames) { + expect(text, `userMessage must not name '${machineName}'`).not.toContain(machineName); + } + expect(text, 'userMessage must not carry an API path').not.toMatch(/\/api\//); + }; + + const refusedUpdate = async () => { + const ql = makeQl([packagedSet()]); + const protocol = makeHatchOpenProtocol(ql, { [SET_NAME]: packagedSet() }); + registerPermissionSetProjection(protocol, { ql }); + ql.permRows.push(packagedRow()); + return run(makeMiddleware(ql, protocol), { + object: 'sys_permission_set', operation: 'update', context: userCtx, + data: { id: 'ps_pkg', description: 'edit' }, + }).then(() => null, (e: any) => e); + }; + + it('UPDATE at the data door: 403 NOT_OVERRIDABLE with the clone guidance in `userMessage`', async () => { + const rejection = await refusedUpdate(); + expect(rejection).toBeInstanceOf(PackagedPermissionSetLockedError); + + const wire = mapDataError(rejection, 'sys_permission_set'); + expect(wire.status).toBe(403); + expect(wire.body.code).toBe('NOT_OVERRIDABLE'); + expectEndUserCopy(wire.body.userMessage, SET_NAME, PACKAGE_ID, 'sys_permission_set'); + expect(String(wire.body.userMessage)).toMatch(/clone it instead/i); + // The diagnostic channel is untouched: it still names the set and the + // package for logs and developers. The mark never replaces `message`. + expect(String(wire.body.error)).toContain(SET_NAME); + expect(String(wire.body.error)).toContain(PACKAGE_ID); + }); + + it('INSERT of a packaged name at the data door: 403 NOT_OVERRIDABLE with its own guidance (choose another name, or clone)', async () => { + const ql = makeQl([packagedSet()]); + const protocol = makeHatchOpenProtocol(ql, { [SET_NAME]: packagedSet() }); + registerPermissionSetProjection(protocol, { ql }); + const rejection = await run(makeMiddleware(ql, protocol), { + object: 'sys_permission_set', operation: 'insert', context: userCtx, + data: { name: SET_NAME, label: 'Mine', object_permissions: '{}' }, + }).then(() => null, (e: any) => e); + expect(rejection).toBeInstanceOf(PackagedPermissionSetLockedError); + + const wire = mapDataError(rejection, 'sys_permission_set'); + expect(wire.status).toBe(403); + expect(wire.body.code).toBe('NOT_OVERRIDABLE'); + expectEndUserCopy(wire.body.userMessage, SET_NAME, PACKAGE_ID, 'sys_permission_set'); + expect(String(wire.body.userMessage)).toMatch(/different name/i); + expect(String(wire.body.userMessage)).toMatch(/clone/i); + + // Each operation carries the guidance that fits it. + const update = mapDataError(await refusedUpdate(), 'sys_permission_set'); + expect(wire.body.userMessage).not.toBe(update.body.userMessage); + }); + + it('the dispatcher door resolves the same throw to the same status, code and `userMessage`', async () => { + const rejection = await refusedUpdate(); + const rest = mapDataError(rejection, 'sys_permission_set'); + const resolved = resolveThrownHttpError(rejection); + expect(resolved.status).toBe(403); + expect(resolved.code).toBe('NOT_OVERRIDABLE'); + expectEndUserCopy(resolved.userMessage, SET_NAME, PACKAGE_ID); + expect(resolved.userMessage).toBe(rest.body.userMessage); + }); + + it('the metadata door\'s registered lock throws the same class with the same `userMessage`', async () => { + const ql = makeQl([shippedArtifact()]); + let gate: ((ctx: { type: string; name: string; body: unknown }) => Promise) | undefined; + const protocol = { + registerAuthoringGate: (_type: string, g: typeof gate) => { gate = g; }, + getMetaItemLayered: async ({ name }: { name: string }) => ({ type: 'permission', name, code: null, overlay: null, effective: null }), + }; + expect(registerPackagedPermissionSetLockGate(protocol, ql)).toBe(true); + + const rejection = await gate!({ type: 'permission', name: SET_NAME, body: packagedSet() }) + .then(() => null, (e: any) => e); + expect(rejection).toBeInstanceOf(PackagedPermissionSetLockedError); + const resolved = resolveThrownHttpError(rejection); + expect(resolved.status).toBe(403); + expect(resolved.code).toBe('NOT_OVERRIDABLE'); + expectEndUserCopy(resolved.userMessage, SET_NAME, PACKAGE_ID); + expect(resolved.userMessage).toBe(mapDataError(await refusedUpdate(), 'sys_permission_set').body.userMessage); + }); + + it('the fail-closed sibling (provenance unknown): 403 NOT_OVERRIDABLE with retry-or-clone guidance, and no diagnostic reason', async () => { + const ql = makeQl([]); + ql.registry = { listItems: () => { throw new Error('registry unavailable'); } }; + const protocol = makeHatchOpenProtocol(ql, {}); + protocol.getMetaItemLayered = async () => { throw new Error('metadata store unavailable'); }; + registerPermissionSetProjection(protocol, { ql }); + ql.permRows.push({ id: 'ps_x', name: 'unknown_provenance', managed_by: 'admin', system_permissions: '[]' }); + const rejection = await run(makeMiddleware(ql, protocol), { + object: 'sys_permission_set', operation: 'update', context: userCtx, + data: { id: 'ps_x', system_permissions: '["whatever"]' }, + }).then(() => null, (e: any) => e); + expect(rejection).toBeInstanceOf(PackagedPermissionSetProvenanceUnknownError); + + const wire = mapDataError(rejection, 'sys_permission_set'); + expect(wire.status).toBe(403); + expect(wire.body.code).toBe('NOT_OVERRIDABLE'); + expectEndUserCopy( + wire.body.userMessage, 'unknown_provenance', 'sys_permission_set', + 'registry unavailable', 'metadata store unavailable', + ); + expect(String(wire.body.userMessage)).toMatch(/try again/i); + expect(String(wire.body.userMessage)).toMatch(/clone/i); + // The reason stays where it was: the diagnostic channel. + expect(String(wire.body.error)).toContain('registry unavailable'); + }); +}); diff --git a/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts b/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts index 020d604935..483e5e6062 100644 --- a/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts +++ b/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts @@ -262,6 +262,44 @@ export function classifyPackagedPermissionSet( }; } +/** + * The refusals' guidance, addressed to the END USER — the `userMessage` each + * refusal below declares. + * + * A thrown `userMessage` is the producer-side opt-in every HTTP door reads + * through `declaredUserMessage` (`@objectstack/types`; see + * `ThrownHttpError.userMessage` there): it rides the wire envelope beside + * `code`, and the console renders it verbatim in place of the generic sentence + * it substitutes for every unmarked 403 ("You don't have permission to save + * this record."). Unmarked, the clone guidance in `message` never reached the + * admin who hit the lock in Setup. + * + * The two channels have two audiences, so they carry different text: + * + * - `message` is the DIAGNOSTIC, for logs and developers. It names the set, + * the package and the API path, and it is unchanged. + * - `userMessage` is the GUIDANCE, for the admin. It names no machine name, + * id, package id or API path (the console's friendly-copy rule), so it + * carries no host state either: a sandboxed body that catches this error + * receives static text. + * + * English, like every platform refusal: there is no localization path for a + * thrown `userMessage`, and these texts do not invent one. + */ +const LOCKED_UPDATE_USER_MESSAGE = + `This permission set is provided by an installed package and can't be edited here. Clone it instead ` + + `with the Clone action, then edit the clone. The clone is your organization's own permission set, ` + + `and package upgrades keep reaching the original.`; + +const LOCKED_INSERT_USER_MESSAGE = + `This name belongs to a permission set provided by an installed package. Choose a different name for ` + + `your permission set, or clone the packaged one with the Clone action and edit the clone.`; + +const PROVENANCE_UNKNOWN_USER_MESSAGE = + `This permission set can't be saved right now, because the system couldn't confirm whether an installed ` + + `package provides it. Try again in a moment. To customize a permission set that a package provides, ` + + `clone it with the Clone action and edit the clone.`; + /** * The refusal a write door throws for a package-declared set. * @@ -287,11 +325,16 @@ export function classifyPackagedPermissionSet( * (`mapDataError` passes a domain error through on `.status`; the runtime * dispatcher's `errorFromThrown` reads `.status` then falls back to * `.statusCode`), and this throws on the DATA path, which reaches both. + * + * `userMessage` carries the same guidance for the end user, per operation — + * see the note on the texts above. */ export class PackagedPermissionSetLockedError extends Error { readonly code = 'NOT_OVERRIDABLE'; readonly status = 403; readonly statusCode = 403; + /** The guidance addressed to the end user; `message` stays the diagnostic. */ + readonly userMessage: string; constructor(name: string, packageId: string, operation: 'insert' | 'update') { super( `[Security] Permission set '${name}' is declared by package '${packageId}' and is locked in this ` @@ -305,14 +348,22 @@ export class PackagedPermissionSetLockedError extends Error { + `flowing to '${name}' untouched.`), ); this.name = 'PackagedPermissionSetLockedError'; + this.userMessage = operation === 'insert' ? LOCKED_INSERT_USER_MESSAGE : LOCKED_UPDATE_USER_MESSAGE; } } -/** Thrown when provenance could not be determined at all — fail-closed. */ +/** + * Thrown when provenance could not be determined at all — fail-closed. + * + * Its `userMessage` says to retry or clone and leaves the unreadable source's + * `reason` in `message`, where the diagnostic belongs. + */ export class PackagedPermissionSetProvenanceUnknownError extends Error { readonly code = 'NOT_OVERRIDABLE'; readonly status = 403; readonly statusCode = 403; + /** The guidance addressed to the end user; `message` stays the diagnostic. */ + readonly userMessage: string; constructor(name: string, reason: string) { super( `[Security] Permission set '${name}' cannot be saved right now: this environment could not determine ` @@ -321,6 +372,7 @@ export class PackagedPermissionSetProvenanceUnknownError extends Error { + `once the metadata layer is readable; if you meant to customize a packaged set, clone it instead.`, ); this.name = 'PackagedPermissionSetProvenanceUnknownError'; + this.userMessage = PROVENANCE_UNKNOWN_USER_MESSAGE; } } From 7c2b636d8bd6025aef7cff2ee61c315354e9c065 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 17:40:29 +0000 Subject: [PATCH 2/2] docs(spec,types,rest,runtime): state who sets userMessage now that a platform refusal does Six comments said platform and driver code never set a thrown userMessage. The packaged-permission-set lock's refusals now do, with static guidance and no host state. Each comment now states the invariant that holds: only a producer authoring end-user text with no host state sets it (an application hook, or a platform refusal carrying static guidance); platform and driver diagnostics never do. Comment-only: each file's comment-free TypeScript print is byte-identical before and after, and the ApiErrorSchema.userMessage describe() text is unchanged. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN --- packages/rest/src/rest-server.ts | 8 +++++--- ...dispatcher.permission-denied-user-message.test.ts | 8 +++++--- packages/runtime/src/http-dispatcher.ts | 5 ++++- packages/runtime/src/sandbox/quickjs-runner.ts | 9 ++++++--- packages/spec/src/api/contract.zod.ts | 9 ++++++--- packages/types/src/data-error-classification.ts | 12 +++++++----- 6 files changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index e49a3d1102..c8cf35fd83 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -12148,9 +12148,11 @@ export class RestServer { // derivation and one reason with two copies is this lane's own // recurring defect). `userMessage` has NO invariant left for a // caller to re-derive. `declaredUserMessage` already decided - // PRESENCE — the field exists on an error only because an - // author deliberately wrote caller-facing text onto it, and - // platform and driver code never set it — and + // PRESENCE — the field exists on an error only because its + // producer deliberately wrote end-user text with no host state + // onto it (an application hook, or a platform refusal carrying + // static guidance such as the packaged-permission-set lock's; + // platform and driver diagnostics never set it) — and // `truncateClientMessage` already applied #5423's bound to the // value. Reading `refusal.body.userMessage` IS the rule; there // is no second function to run it through, and running one diff --git a/packages/runtime/src/http-dispatcher.permission-denied-user-message.test.ts b/packages/runtime/src/http-dispatcher.permission-denied-user-message.test.ts index b998c724ac..bee1794e7c 100644 --- a/packages/runtime/src/http-dispatcher.permission-denied-user-message.test.ts +++ b/packages/runtime/src/http-dispatcher.permission-denied-user-message.test.ts @@ -35,9 +35,11 @@ * `object` is derived from the ROUTE, never from `details.object` (which, on a * cascade delete, names a CHILD the caller never addressed). `§3` drives that * exact fixture **with a mark present**, so the repair cannot be read as - * loosening the withhold: the marked channel is authored end-user text, - * carried as a declared top-level sibling of `code`/`message`, and platform and - * driver code never set it. + * loosening the withhold: the marked channel is authored end-user text with + * no host state (an application hook's, or a platform refusal's static + * guidance such as the packaged-permission-set lock's), carried as a declared + * top-level sibling of `code`/`message`; platform and driver diagnostics never + * set it. * * That same ruling is also why the change is owed: it makes REST's shape the * contract for BOTH transports, and REST's shape has carried the mark on this diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 5944b36f25..c12bd1bf77 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -2828,7 +2828,10 @@ export class HttpDispatcher { // SERVER-side diagnostics attached by `plugin-security` and are // still dropped above. `userMessage` is the opposite by // construction: it exists only because a producer wrote text - // FOR the caller, platform and driver code never set it, and it + // FOR the caller with no host state in it (an application + // hook, or a platform refusal carrying static guidance such as + // the packaged-permission-set lock's); platform and driver + // diagnostics never set it, and it // lands as a declared top-level sibling of `code`/`message` // (`ApiErrorSchema.userMessage`), never inside `details`. // The mark never moves the status or the `code` (commit 79c46da90). diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index 9c35efba2a..93dfa9af66 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -1275,9 +1275,12 @@ function safeJsonStringify(v: unknown): string { * writes `const e = new Error(msg); e.userMessage = msg; throw e`, and the * text must survive the VM flattening the throw to a string, or the marking * dies exactly where its primary producers live. Crossing INTO the VM is safe - * for the same reason `code` is: the value is author-written user-facing text - * by construction (platform and driver code never sets the field), so it - * carries no host state a sandboxed body could exfiltrate — and it keeps the + * for the same reason `code` is: the value is text its producer authored for + * the end user (an application hook's, or a platform refusal's static + * guidance such as the packaged-permission-set lock's, which names no set, + * package, id or path), and platform and driver diagnostics never set the + * field, so it carries no host state a sandboxed body could exfiltrate — and + * it keeps the * established property that a body which catches, inspects and re-throws a * host error does not lose the structured payload. */ diff --git a/packages/spec/src/api/contract.zod.ts b/packages/spec/src/api/contract.zod.ts index 862667a8b0..924d611713 100644 --- a/packages/spec/src/api/contract.zod.ts +++ b/packages/spec/src/api/contract.zod.ts @@ -80,9 +80,12 @@ export const ApiErrorSchema = lazySchema(() => z.object({ * and the marked text are one value, so a boundary that rewraps or * substitutes `message` (sanitisation, truncation, the sandbox debug * wrapper) can never accidentally promote platform prose into the marked - * channel. Platform/driver code never sets it. Same audience-split - * precedent as `developerMessage` on the DELETE_RESTRICTED envelope - * (#7307), pointed the other way. + * channel. It is set only by a producer that authors end-user text with + * no host state: an application hook, or a platform refusal that carries + * static guidance (the packaged-permission-set lock's refusals in + * `@objectstack/plugin-security`). Platform and driver diagnostics never + * set it. Same audience-split precedent as `developerMessage` on the + * DELETE_RESTRICTED envelope (#7307), pointed the other way. * * It never replaces `message` — the diagnostic channel keeps its own wording * for logs and developers. diff --git a/packages/types/src/data-error-classification.ts b/packages/types/src/data-error-classification.ts index 2632bb1637..af9155a533 100644 --- a/packages/types/src/data-error-classification.ts +++ b/packages/types/src/data-error-classification.ts @@ -672,11 +672,13 @@ export function mapDataError(error: any, object?: string): { status: number; bod * Why riding it across the FAULT terminals is safe rather than a #5437/#7543 * regression: those disciplines withhold text the producer never addressed to * the caller — driver prose, a crash's `TypeError: …`. `userMessage` is the - * opposite by construction: it exists on an error only because an author - * deliberately wrote user-facing text onto it (`declaredUserMessage` answers - * `undefined` for everything else — platform and driver code never sets the - * field), so carrying it discloses nothing that was not authored for exactly - * this audience. A genuine crash carries no marking and its envelope is + * opposite by construction: it exists on an error only because its producer + * deliberately wrote end-user text with no host state onto it — an + * application hook, or a platform refusal carrying static guidance such as + * the packaged-permission-set lock's (`declaredUserMessage` answers + * `undefined` for everything else; platform and driver diagnostics never set + * the field) — so carrying it discloses nothing that was not authored for + * exactly this audience. A genuine crash carries no marking and its envelope is * byte-identical to before. The one thing the marking never does is move the * STATUS or the `code` — a marked crash is still the sanitised 500. */