fix(plugin-security): the packaged-permission-set lock refusal carries its guidance as userMessage

.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`.

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<void>) | 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');
+  });
+});

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;
   }
 }
 

packages/rest/src/rest-server.ts

@@ -12157,9 +12157,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

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

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).

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.
  */