Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions .changeset/19307-permission-set-duplicate-name-refusal-code.md
Original file line number Diff line number Diff line change
@@ -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.
84 changes: 84 additions & 0 deletions packages/plugins/plugin-security/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading