objectstack-ai
diff --git a/‎.changeset/19307-permission-set-duplicate-name-refusal-code.md‎
Lines changed: 71 additions & 0 deletions b/‎.changeset/19307-permission-set-duplicate-name-refusal-code.md‎
Lines changed: 71 additions & 0 deletions
diff --git a/‎packages/plugins/plugin-security/src/errors.ts‎
Lines changed: 84 additions & 0 deletions b/‎packages/plugins/plugin-security/src/errors.ts‎
Lines changed: 84 additions & 0 deletions
@@ -0,0 +1,71 @@
+---
+'@objectstack/plugin-security': patch
+'@objectstack/spec': minor
+---
+
+fix(plugin-security): the `sys_permission_set` duplicate-name refusal carries `UNIQUE_VIOLATION`, and the packaged-set lock answers first (#19307)
+
+Clause-②: yes
+
+Two halves of one defect on the data door's insert leg for `sys_permission_set`
+(`permission-set-projection.ts`), both measured live on `examples/app-showcase`
+with a seeded admin over a cookie session.
+
+**1. The refusal carried no machine-readable code.** It threw a bare `Error`
+with `.status = 409` and no `.code`, and the flat `{ error, code }` responder
+invents nothing for a producer that declared nothing, so the client got prose:
+
+```
+POST /api/v1/data/sys_permission_set {"name":"dev_local_set"}
+→ 409 {"error":"[Security] permission set 'dev_local_set' already exists","object":"sys_permission_set"}
+```
+
+ADR-0112's 2026-08-17 amendment closed `error.code` at the flat door too, so a
+409 with no code is that contract unhonoured — and a UI that has to branch on
+the refusal was pushed back to string-matching. The same request now answers
+`409 … "code":"UNIQUE_VIOLATION"`, message byte-identical.
+
+⚠️ `UNIQUE_VIOLATION` is REUSED, not minted. `sys_permission_set` declares
+`{ fields: ['name'], unique: 'organization' }`, so this very collision already
+answers `409 UNIQUE_VIOLATION` when the index catches it instead of this
+pre-check; a second spelling would make one condition answer two envelopes
+depending only on which layer got there first. The ledger gains a provenance
+row for `@objectstack/plugin-security` — the union, its casing and every other
+package's rows are unchanged, and no schema shape moves.
+
+**2. It ran BEFORE the packaged-set lock, so the most likely path answered the
+less useful of two true refusals.** A package-declared set has a projected row,
+so its name is duplicate AND locked at once. An admin who opened the Clone
+dialog on a packaged set and typed the base set's own name — the single most
+likely thing to type — got `already exists`, which names no remedy, and never
+reached `NOT_OVERRIDABLE`, which names the clone path. The lock now runs first:
+
+```
+POST /api/v1/data/sys_permission_set {"name":"showcase_manager"}
+→ 403 {"error":"[Security] Permission set 'showcase_manager' is declared by package
+   'com.example.showcase' and is locked … Choose a different name for your set, or clone
+   'showcase_manager' …","code":"NOT_OVERRIDABLE","object":"sys_permission_set"}
+```
+
+**What did NOT move**, measured on the same runtime: an ordinary
+(non-package-declared) duplicate **whose provenance the lock can resolve** still
+answers the duplicate refusal and not `NOT_OVERRIDABLE` — that qualifier is
+load-bearing, and the corner below is the case it excludes; an unauthenticated
+write on the same resource still answers `401 UNAUTHENTICATED`; and an `update`
+targeting a packaged set answers `403 NOT_OVERRIDABLE` exactly as before.
+
+⚠️ **One corner moved with the order**: an ordinary duplicate attempted while no
+artifact source can answer now takes the lock's fail-closed `unknown` refusal —
+`403` `NOT_OVERRIDABLE` (`PackagedPermissionSetProvenanceUnknownError`, "retry
+once the metadata layer is readable") — instead of the 409. Both are refusals and
+neither writes; it is pinned so the behaviour is declared rather than incidental.
+
+⚠️ **And the order has a cost, stated rather than discovered**: the lock's probe
+(`protocol.getMetaItemLayered`) used to be evaluated only AFTER the duplicate
+check passed, so a duplicate insert never paid for it. It is now evaluated
+unconditionally, ahead of that check. Two consequences, both deliberate: every
+**duplicate** insert on `sys_permission_set` costs one extra metadata round trip
+(the accepted path's cost is unchanged — it always paid this probe), and the
+duplicate path is now COUPLED to metadata-layer reachability, where before it
+answered from the record alone. That coupling is the mechanism behind the corner
+above, and it is the price of putting the refusal that names the remedy first.
@@ -364,6 +364,90 @@ export class ExplainObjectNotFoundError extends Error {
   }
 }
 
+/**
+ * The ADR-0112 code {@link PermissionSetNameConflictError} stamps — see that
+ * class for why this collision is `UNIQUE_VIOLATION` and why the value is a
+ * named constant rather than a class-field literal.
+ */
+export const PERMISSION_SET_NAME_CONFLICT_CODE = 'UNIQUE_VIOLATION';
+
+/** The HTTP status {@link PermissionSetNameConflictError} declares. */
+export const PERMISSION_SET_NAME_CONFLICT_STATUS = 409;
+
+/**
+ * [#19307] The data door's duplicate-name refusal on `sys_permission_set`:
+ * a set with this machine name already exists in the caller's organization, so
+ * the insert is refused.
+ *
+ * ## Why this is a CLASS and not a bare `Error` with `.status = 409`
+ *
+ * It was the bare form until now, and the bare form has no `code`. The flat
+ * `{ error, code }` responder in `packages/rest` puts a thrown `code` on the
+ * wire and invents nothing when the producer declared none, so the refusal
+ * reached the client as prose alone — against ADR-0112's 2026-08-17 amendment
+ * (#9232), under which the flat door carries the closed member too. Measured
+ * before the fix: `409 {"error":"[Security] permission set 'showcase_manager'
+ * already exists","object":"sys_permission_set"}`, with no `code` key at all,
+ * while an unauthenticated write on the same resource answered
+ * `401 UNAUTHENTICATED` — so the absence was this producer's, never the door's.
+ * A dialog that has to branch on the refusal was pushed to string-matching.
+ *
+ * ## Why `UNIQUE_VIOLATION` and not a newly minted code
+ *
+ * It is the wire identity this platform ALREADY answers for this exact
+ * condition on this exact column. `sys_permission_set` declares
+ * `{ fields: ['name'], unique: 'organization' }`, and a collision that reaches
+ * the storage layer comes back as `409 UNIQUE_VIOLATION` — the reading
+ * recorded on that index's own comment (#8554) is `org_yi 409
+ * UNIQUE_VIOLATION`. This middleware refuses the same collision one layer
+ * earlier, so a second spelling here would make ONE condition answer two
+ * envelopes depending only on whether the projection's pre-check or the index
+ * caught it — the drift `@objectstack/rest` and `@objectstack/driver-memory`
+ * already registered the SAME code to avoid ("the wire identity is
+ * deliberately the SAME"). #5240's one-condition-one-wording, on the code axis.
+ *
+ * ⛔ Not `RESOURCE_CONFLICT` (the standard member 409 derives from): that is
+ * what the door would supply for a producer that named no condition, and it
+ * would be the second spelling described above.
+ *
+ * ## Why BOTH `status` and `statusCode`
+ *
+ * The same reason every class above records: the two transports read different
+ * property names (`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.
+ *
+ * The message is byte-identical to the bare `Error`'s — the wording was never
+ * the defect, and the flat door's 4xx arm ships it verbatim.
+ *
+ * ## Why the code is a NAMED CONSTANT and not a bare class-field literal
+ *
+ * Same spelling `@objectstack/driver-memory` uses for its own registration of
+ * this code ("via the package's exported `UNIQUE_VIOLATION_CODE` /
+ * `UNIQUE_VIOLATION_STATUS`"), and the reason is mechanical rather than
+ * stylistic: `check:error-code-provenance` recognises `objlit`, `assign` and
+ * `*_CODE` `constdef` stamp sites and is blind to class fields by its own
+ * declared bounds. Written as a class-field literal this package would have
+ * become an unlisted EMITTER of a registered code with every gate in the repo
+ * green — the exact invisibility the ledger header names ("no admission rule
+ * checks WHO emits, so an unlisted emitter is invisible to every gate the repo
+ * has", three hand sweeps, #7504 / #13254 / #13353). The constant puts this
+ * emitter inside the gate's field of view, so the provenance row under
+ * `@objectstack/plugin-security` is enforced and not merely intended.
+ */
+export class PermissionSetNameConflictError extends Error {
+  readonly code = PERMISSION_SET_NAME_CONFLICT_CODE;
+  readonly status = PERMISSION_SET_NAME_CONFLICT_STATUS;
+  readonly statusCode = PERMISSION_SET_NAME_CONFLICT_STATUS;
+  /** The permission-set machine name that was already taken. */
+  readonly setName: string;
+  constructor(setName: string) {
+    super(`[Security] permission set '${setName}' already exists`);
+    this.name = 'PermissionSetNameConflictError';
+    this.setName = setName;
+  }
+}
+
 export function isPermissionDeniedError(e: unknown): e is PermissionDeniedError {
   if (!e || typeof e !== 'object') return false;
   const anyE = e as any;