diff --git a/.changeset/scoped-kernel-package-door.md b/.changeset/scoped-kernel-package-door.md new file mode 100644 index 0000000000..c6e21d1233 --- /dev/null +++ b/.changeset/scoped-kernel-package-door.md @@ -0,0 +1,53 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): the metadata write refusal stops depending on deployment topology (#8184) + +`PUT /api/v1/meta/object/showcase_task?package=READONLY_PKG` answered **two +different machine-readable codes for one condition**, selected by the kernel's +`environmentId` — a row-scoping key, not a topology declaration: + +| kernel | answer | +| --- | --- | +| host-config / CLI lightweight assembler (`environmentId` undefined — the flagship showcase, self-hosted servers) | `403 ITEM_LOCKED`, `lockSource: 'package'` | +| project / cloud per-environment kernel (`environmentId` set) | `403 NOT_OVERRIDABLE` — the package was never read | + +`saveMetaItem` carries its own artifact-backed refusal behind +`if (this.environmentId !== undefined)`, and it threw before +`SysMetadataRepository.assertAllowed` — the topology-independent package door +(#7682, then #8146's hatch ruling) — ever ran. So a client that learned to +handle `ITEM_LOCKED` on a self-hosted deployment never saw it on a cloud one, +and an operator reading `NOT_OVERRIDABLE` was told the type had no overlay +channel when the real obstacle was the read-only base they had named. + +Not a regression: that branch answered `NOT_OVERRIDABLE` before #8185 and +#8320 too. Those cards made the divergence visible by fixing the other half. + +**The scoped branch now consults the same `isWritablePackage` predicate and +throws the repository's own emitter** — called, not copied — so the code, the +status, `lockSource`, `packageId` and the sentence are byte-identical on both +topologies, and neither door can drift when the other moves. + +**Same limb ordering as the repository, because the ordering is the rule:** + +- **Below every registry limb.** The branch is guarded by `!overlayAllowed`, so + an `allowOrgOverride` type never reaches the door. An ADR-0005 org overlay of + a code-shipped item *always* names the read-only package it customizes; a + door one limb higher would close the overlay model outright. +- **Above the hatch limb.** `isOverlayAllowed` folds `OS_METADATA_WRITABLE` in, + so an open hatch takes the write past this branch to the repository door, + which applies the same rule with its own hatch-aware remedy — the refusal + never prescribes the step the caller already took. Both directions pinned. + +**Narrow, exactly as the repository is.** Only a write that *names* a read-only +base is re-coded; a package-less write keeps `NOT_OVERRIDABLE` verbatim, and a +package-less hatch write still lands `{ package_id: null, organization_id: null }` +env-wide and `{ package_id: null, organization_id: }` under an org kernel. +Refusing a hatch write that names no read-only base (the broad reading) would +retire the hatch's only documented use and remains a maintainer decision plus a +docs/ADR change. + +The `runtime-only` create side needed no change: the ADR-0070 D1 gate further +down `saveMetaItem` is already topology-independent and already answers +`422 WRITABLE_PACKAGE_REQUIRED` on every kernel. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 38ce97f975..52b3f44dc5 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -10434,6 +10434,74 @@ export class ObjectStackProtocolImplementation implements if (this.environmentId !== undefined) { const artifactBacked = this.isArtifactBacked(request.type, request.name); if (artifactBacked && !overlayAllowed) { + // [#8184] THE PACKAGE DOOR — the SECOND refusal point for one + // condition, and the reason this card exists. + // + // `SysMetadataRepository.assertAllowed` reads the base the + // caller NAMED and answers `ITEM_LOCKED` (`lockSource: + // 'package'`) when it is read-only (#7682, then #8146's + // hatch ruling). That door is topology-INDEPENDENT — and it + // was unreachable here, because this branch throws first on + // every kernel with an `environmentId`. So one request + // answered `ITEM_LOCKED` on a host-config / CLI-assembled + // kernel and the undiscriminated `NOT_OVERRIDABLE` on a + // project/cloud per-env one: the refusal VOCABULARY keyed off + // a row-scoping key, which is the #5086 / #6710 finding + // (see the block comment above) arriving on the error codes. + // A client that learns to handle `ITEM_LOCKED` on one + // deployment never saw it on the other. + // + // ⚠️ MIRRORED, NOT RE-INVENTED. Same predicate + // ({@link isWritablePackage}, the ADR-0070 rule in one + // place), same emitter — `readOnlyBaseOverrideError` is + // called, not copied — so the code, the status, the + // `lockSource`, the `packageId` and the sentence cannot drift + // between the two doors. Two independently-authored refusals + // for one condition is how `NOT_OVERRIDABLE`-everywhere + // started. + // + // THE LIMB ORDERING IS THE RULE, and it is the same ordering + // the repository states: BELOW every registry limb, ABOVE the + // hatch limb. + // • Below the registry limb — this whole branch is guarded + // by `!overlayAllowed`, so an `allowOrgOverride` type + // never reaches the door. That is ADR-0005: an org + // overlay of a code-shipped item ALWAYS names the + // read-only package it customizes, and a door one limb + // higher would close the overlay model outright. Pinned. + // • Above the hatch limb — `isOverlayAllowed` folds + // `OS_METADATA_WRITABLE` in, so an OPEN hatch takes the + // write past this branch entirely, down to the repository + // door, which applies the same rule with `hatchOpen: + // true` and its own remedy. The hatch therefore still + // never unlocks package writability on this topology + // either (#8146 NARROW), and both directions of that + // remedy selection are pinned in + // `sys-metadata-repository.package-writability.test.ts`. + // That is also why `hatchOpen` is passed as a literal + // `false` here rather than recomputed: reaching this line + // PROVES the hatch is closed, and a recomputed value + // would be dead code dressed as a decision. + // + // ⛔ NARROW, exactly as the repository is: only a write that + // NAMES a read-only base is re-coded. A package-less write + // keeps `NOT_OVERRIDABLE` verbatim. Refusing a hatch write + // that names NO read-only base (BROAD) retires the hatch's + // only documented use and needs a maintainer decision plus a + // docs/ADR change — never arrived at from here. + // + // `runtime-only` needs no limb here: this branch is guarded by + // `artifactBacked`, so the intent is always + // `override-artifact`. The create side of the door is the + // ADR-0070 D1 gate further down this method, which is already + // topology-independent and already answers + // `WRITABLE_PACKAGE_REQUIRED` / 422 on every kernel. + const namedBase = typeof request.packageId === 'string' && request.packageId.length > 0; + if (namedBase && !this.isWritablePackage(request.packageId)) { + throw SysMetadataRepository.readOnlyBaseOverrideError( + request.type, request.packageId as string, false, + ); + } const err = new Error( `[not_overridable] Metadata item '${request.type}/${request.name}' is provided by a code package ` + `and the type has not opted into per-org overlay writes (allowOrgOverride=false). ` diff --git a/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts index bca9cace32..1c423ec085 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.package-writability.test.ts @@ -173,6 +173,22 @@ const objectBody = { name: 'showcase_task', label: 'Task', sharingModel: 'privat */ const permissionBody = { name: 'showcase_contributor', label: 'Contributor', objects: {} }; +/** + * [#8184] A spec-valid `view` CONTAINER — the ADR-0005 overlay preservation + * case, which needs a body the Zod gate accepts for the same reason + * {@link permissionBody} does: an invalid body answers 422 before the + * authorization door and the pin goes green without ever reaching it. + * + * `ViewSchema` is the container (`list` / `form` / `listViews` / `formViews`), + * NOT a flat list view — a flat one parses to an empty container. + */ +const viewBody = { + name: 'case_grid', + label: 'Case Grid', + object: 'showcase_task', + list: { columns: ['name'] }, +}; + /** * `put` with everything but the base fixed, so each case differs in ONE way. * @@ -520,13 +536,15 @@ describe('#7682 — the refusal discriminates on package writability', () => { * backed refusal is behind `environmentId !== undefined`, so the write reaches * `SysMetadataRepository.put` and the package door is what answers. * - * ⚠️ On a SCOPED kernel (`environmentId` set) the protocol refuses first, with - * `NOT_OVERRIDABLE`, and this fix is not reachable — that second refusal point - * lives in `protocol.ts`, which neither #7682 nor #8146 is authorised to edit. - * It is filed as **#8184**, and it is deliberately NOT covered here: this file - * pins ONE kernel. ⛔ Do not read a green run of this suite as evidence that - * the scoped kernel refuses too — it does not, and #8146's hatch refusal below - * inherits exactly the same boundary. + * ⚠️ [#8184] THAT BOUNDARY IS GONE — do not restore this docblock's earlier + * warning. It read: on a SCOPED kernel the protocol refuses first with + * `NOT_OVERRIDABLE`, this fix is not reachable there, and a green run of this + * block is NOT evidence about that kernel. True when #7682 and #8146 wrote it + * (neither was authorised to edit `protocol.ts`), and closed by #8184: the + * scoped branch now consults the same predicate and throws this file's own + * emitter. Both kernels are covered, and the block at the very bottom pins + * them against EACH OTHER rather than against a literal — so the divergence + * cannot come back as a green suite. */ describe('#7682 / #8146 — through saveMetaItem on the host-config topology', () => { function boot() { @@ -630,3 +648,234 @@ describe('#7682 / #8146 — through saveMetaItem on the host-config topology', ( expect(metaRows[0]).toMatchObject({ package_id: null, organization_id: null }); }, 30_000); }); + +/** + * [#8184] The SECOND refusal point — the same request on a SCOPED kernel. + * + * `saveMetaItem` carries its own artifact-backed refusal behind + * `if (this.environmentId !== undefined)`, and it ran BEFORE the repository + * ever saw the write. So the block above pinned one kernel and one kernel only: + * a project/cloud per-env kernel answered the undiscriminated + * `NOT_OVERRIDABLE` for the very request a host-config kernel answered + * `ITEM_LOCKED` for. One condition, two machine-readable vocabularies, selected + * by a ROW-SCOPING key — the #5086 / #6710 finding, now on the refusal + * vocabulary itself. + * + * ⚠️ Not a regression from #8185 or #8320: this branch answered + * `NOT_OVERRIDABLE` before both. Those cards made the divergence visible. + * + * ## What this block pins, and why it is a MIRROR rather than a second rule + * + * The protocol branch now consults the same {@link isWritablePackage} + * predicate and throws the repository's OWN emitter + * (`SysMetadataRepository.readOnlyBaseOverrideError`) — not a re-spelling of + * it. Two independently-authored refusals for one condition is how the + * `NOT_OVERRIDABLE`-everywhere problem started, so the pin below compares the + * two kernels' answers to EACH OTHER (`the two kernels agree`) rather than + * asserting a literal twice. + * + * **The limb ordering is the rule, here too.** `isOverlayAllowed` folds the + * registry flag AND the `OS_METADATA_WRITABLE` hatch into one predicate, so + * this branch is reached only with BOTH closed — the door is therefore below + * every registry limb (an ADR-0005 overlay never reaches it, pinned) and the + * hatch-open direction is delivered by the repository door downstream, which + * this block measures rather than assumes. + */ +describe('#8184 — the scoped kernel answers the same code as the host-config kernel', () => { + /** + * @param environmentId `undefined` = the CLI host-config assembler; + * a string = a project/cloud per-environment kernel. The ONLY difference + * between the two boots, which is what makes the comparison a measurement + * of the topology key and nothing else. + */ + function boot(environmentId?: string) { + const engine = makeFakeEngine() as unknown as Record; + (engine as { registry: Record }).registry = { + ...(engine.registry as Record), + registerItem: () => {}, + registerObject: () => {}, + listItems: () => [], + getItem: () => undefined, + getArtifactItem: (type: string, name: string) => + (type === 'object' && name === 'showcase_task') + || (type === 'permission' && name === 'showcase_contributor') + // [#8184] `view` is `allowOrgOverride: true`, so it returns at the + // REGISTRY limb — above the door. Artifact-backed on purpose: that is + // the ADR-0005 overlay the door must never refuse. + || (type === 'view' && name === 'case_grid') + ? { name, _packageId: READ_ONLY_PKG } + : undefined, + }; + const protocol = new ObjectStackProtocolImplementation( + engine as never, + () => new Map(), + environmentId, + ) as unknown as { + saveMetaItem(req: Record): Promise; + }; + return { engine, protocol }; + } + + const metaRowsOf = (engine: Record) => + Array.from((engine as unknown as { rows: Map }).rows.values()) + .filter((r) => r.__table === 'sys_metadata'); + + const save = ( + protocol: { saveMetaItem(req: Record): Promise }, + req: Record, + ) => protocol.saveMetaItem(req).then(() => null, (e: unknown) => e); + + const openHatch = (types: string) => { + process.env.OS_METADATA_WRITABLE = types; + resetEnvWritableMetadataTypes(); + ObjectStackProtocolImplementation.resetEnvWritableCache(); + }; + + beforeEach(() => { + delete process.env.OS_METADATA_WRITABLE; + resetEnvWritableMetadataTypes(); + ObjectStackProtocolImplementation.resetEnvWritableCache(); + }); + afterEach(() => { + delete process.env.OS_METADATA_WRITABLE; + resetEnvWritableMetadataTypes(); + ObjectStackProtocolImplementation.resetEnvWritableCache(); + }); + + // ── the defect: one request, two kernels, two vocabularies ───────────── + + it('the two kernels agree on the code, the status and the lock source', async () => { + // THE CARD. Compared to each other, not to a literal: the property is + // "one condition keeps one vocabulary", so a future change that moved + // BOTH would still be one vocabulary — while a change that moves one is + // exactly the defect coming back. + const hostConfig = await save(boot().protocol, { + type: 'object', name: 'showcase_task', item: objectBody, packageId: READ_ONLY_PKG, + }) as Record; + const scoped = await save(boot('env_alpha').protocol, { + type: 'object', name: 'showcase_task', item: objectBody, packageId: READ_ONLY_PKG, + }) as Record; + + expect(scoped).toMatchObject({ + code: hostConfig.code, status: hostConfig.status, lockSource: hostConfig.lockSource, + }); + expect(scoped).toMatchObject({ + code: 'ITEM_LOCKED', status: 403, lockSource: 'package', packageId: READ_ONLY_PKG, + }); + // The SENTENCE too, and it is byte-identical because the two doors call + // ONE emitter. `saveMetaItem` folds plural→singular at its top + // (`canonicalizeMetaRequestType`) and the repository folds again, so + // neither door can spell the type differently either. A copy in + // `protocol.ts` would pass every assertion above this one and fail here. + expect((scoped as { message?: string }).message) + .toBe((hostConfig as { message?: string }).message); + }, 30_000); + + it('a WRITABLE base still answers the type door — the door discriminates, it does not blanket-refuse', async () => { + // The other half of "one PUT, two bases, two outcomes", on this kernel. + // A suite that only pinned the new code would stay green if the scoped + // branch started answering ITEM_LOCKED for every base. + const err = await save(boot('env_alpha').protocol, { + type: 'object', name: 'showcase_task', item: objectBody, packageId: WRITABLE_PKG, + }); + expect(err).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 }); + }, 30_000); + + it('a write that names NO base keeps the type-door code verbatim', async () => { + const err = await save(boot('env_alpha').protocol, { + type: 'object', name: 'showcase_task', item: objectBody, + }); + expect(err).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 }); + }, 30_000); + + it('nothing persists on the refusal', async () => { + const { engine, protocol } = boot('env_alpha'); + await save(protocol, { + type: 'object', name: 'showcase_task', item: objectBody, packageId: READ_ONLY_PKG, + }); + expect(metaRowsOf(engine)).toEqual([]); + }, 30_000); + + // ── the hatchOpen remedy selection, BOTH directions, on this kernel ──── + + it('with the hatch CLOSED the refusal offers it — the prescription is chosen, not deleted', async () => { + const err = await save(boot('env_alpha').protocol, { + type: 'object', name: 'showcase_task', item: objectBody, packageId: READ_ONLY_PKG, + }) as { message?: string }; + expect(String(err.message)).toContain('set OS_METADATA_WRITABLE=object'); + }, 30_000); + + it('with the hatch OPEN the refusal does NOT prescribe the step already taken', async () => { + // The false-prescription trap, on the topology this card is about. The + // hatch-open write does not reach the protocol branch at all — an open + // hatch makes `isOverlayAllowed` true — so this measures that the write + // falls through to the repository door and is answered there with the + // SAME code and the hatch-aware remedy. That is why the protocol site + // passes `hatchOpen: false` rather than recomputing it. + openHatch('permission'); + const err = await save(boot('env_alpha').protocol, { + type: 'permission', name: 'showcase_contributor', item: permissionBody, packageId: READ_ONLY_PKG, + }) as { code?: string; status?: number; message?: string }; + + expect(err).toMatchObject({ code: 'ITEM_LOCKED', status: 403 }); + expect(String(err.message)).not.toContain('set OS_METADATA_WRITABLE=permission'); + expect(String(err.message)).toContain('does not apply here'); + }, 30_000); + + // ── PRESERVATION: the load-bearing pins ─────────────────────────────── + + it('an ADR-0005 overlay of a code-shipped item still lands — the door is BELOW every registry limb', async () => { + // If the door had been placed one limb higher this goes red and the whole + // overlay model closes. `view` is allowOrgOverride, artifact-backed, and + // names the read-only package it customizes — by construction. + const { engine, protocol } = boot('env_alpha'); + const err = await save(protocol, { + type: 'view', name: 'case_grid', item: viewBody, packageId: READ_ONLY_PKG, + }); + + expect(err).toBeNull(); + expect(metaRowsOf(engine)[0]).toMatchObject({ package_id: READ_ONLY_PKG }); + }, 30_000); + + it('a package-less hatch write still lands the env-wide overlay, bound to NO package', async () => { + // THE PREMISE of NARROW, on this kernel. Red here means NARROW preserves + // nothing and the NARROW/BROAD fork goes back to the maintainer — not a + // test to "repair" by relaxing it. + openHatch('permission'); + const { engine, protocol } = boot('env_alpha'); + const err = await save(protocol, { + type: 'permission', name: 'showcase_contributor', item: permissionBody, + }); + + expect(err).toBeNull(); + const rows = metaRowsOf(engine); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ package_id: null, organization_id: null }); + }, 30_000); + + it('a package-less hatch write under an ORG kernel lands the per-org override the docs promise', async () => { + openHatch('permission'); + const { engine, protocol } = boot('env_alpha'); + const err = await save(protocol, { + type: 'permission', name: 'showcase_contributor', + item: permissionBody, organizationId: 'org_acme', + }); + + expect(err).toBeNull(); + const rows = metaRowsOf(engine); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ package_id: null, organization_id: 'org_acme' }); + }, 30_000); + + it('a hatch write naming a WRITABLE base still lands — the door reads writability, not the hatch', async () => { + openHatch('permission'); + const { engine, protocol } = boot('env_alpha'); + const err = await save(protocol, { + type: 'permission', name: 'showcase_contributor', + item: permissionBody, packageId: WRITABLE_PKG, + }); + + expect(err).toBeNull(); + expect(metaRowsOf(engine)[0]).toMatchObject({ package_id: WRITABLE_PKG }); + }, 30_000); +}); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 5d30cedfb0..1ae7a05bac 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -1257,8 +1257,18 @@ export class SysMetadataRepository implements MetadataRepository { * item-level `_lock` refusal (`assertLockAllowsWrite`) that carries a `lock` * value read off the item. This one claims no `_lock`, because the item * declares none. + * + * @internal [#8184] Reachable from `protocol.ts` — deliberately, and it is + * the whole point of that card. `saveMetaItem` carries a SECOND refusal for + * this condition behind `environmentId !== undefined`, which shadowed this + * door on every scoped kernel and answered the undiscriminated + * `NOT_OVERRIDABLE` there. The two sites now share this ONE emitter rather + * than each spelling the sentence: two independently-authored refusals for + * one condition is how the `NOT_OVERRIDABLE`-everywhere problem started, and + * a copy in `protocol.ts` would drift from this one the first time either + * moves. ⛔ Do not re-privatise without deleting that call site. */ - private static readOnlyBaseOverrideError(type: string, packageId: string, hatchOpen = false): Error { + static readOnlyBaseOverrideError(type: string, packageId: string, hatchOpen = false): Error { const singular = PLURAL_TO_SINGULAR[type] ?? type; const err: any = new Error( `[item_locked] Cannot overlay '${type}' in package '${packageId}': that package is read-only `