diff --git a/.changeset/5676-layered-envelope-boundary-parse.md b/.changeset/5676-layered-envelope-boundary-parse.md new file mode 100644 index 0000000000..88c2a25333 --- /dev/null +++ b/.changeset/5676-layered-envelope-boundary-parse.md @@ -0,0 +1,39 @@ +--- +'@object-ui/data-objectstack': minor +--- + +`MetadataClient.layered()` validates the ADR-0010 protection envelope against the +producer's own schema at the boundary, instead of casting ten wire fields through +unchecked (objectui#5676, triage adjudication 2026-08-22). + +The envelope arrived by ten `as` assertions over a raw `res.json()` body — no parse, no +allowlist, no default. The consumer that reads it opens the metadata lock banner on +`layered?.lock && layered.lock !== 'none'`, true for **any** non-`none` value, so a server +sending a lock state this console had never heard of opened the amber box, drew the padlock +and the border, and rendered an empty title. No fifth state ever had to be added to this +repo for that to happen: a union types what this repo writes and constrains nothing about +what a server sends. + +The boundary now runs `GetMetaItemLayeredResponseSchema.safeParse`. On the conforming path +every value is the producer's schema output and the ten assertions are gone. `safeParse` +and never `parse`: a metadata console that rejected every dialect it had not been compiled +against would answer a newer server with a blank page, which is strictly worse than the +wrong render being fixed. Values the schema rejects are still **forwarded** — dropping them +would be that same refused rejection wearing different clothes — and are named in a new +optional `MetadataLayered._unrecognized`, absent whenever everything parsed. This extends +to the whole envelope the "pass through and label" treatment objectui#5672 chose for `lock` +alone; the banner's existing unrecognised-token title is unchanged and needed no edit. + +The labelling is per field, which is the part that makes it a degrade rather than a subtler +version of the same bug. Measured on the installed spec (17.2.0): +`GetMetaItemLayeredResponseSchema.safeParse(body)` is all-or-nothing — one unknown `lock` +returns `success: false` with `data` undefined — so the failure branch re-checks each key +against that schema's own `shape[key]`, where only the offending field fails and the other +six still arrive typed. Absence is never "unrecognised": the four resolved verdicts are +required upstream on this path, so a pre-ADR-0010 backend takes the failure branch with +nothing flagged and behaves exactly as before. + +One consequence of the same ruling, fixed alongside because it defeats it: a 200 answer +whose body was a bare JSON string or number **rejected** the promise with a +`TypeError: Cannot use 'in' operator`, from the envelope-detection guard's bare truthiness +check. A malformed body must degrade, never throw. diff --git a/packages/data-objectstack/src/metadata-client.layeredEnvelope.test.ts b/packages/data-objectstack/src/metadata-client.layeredEnvelope.test.ts new file mode 100644 index 0000000000..0a23c49ec5 --- /dev/null +++ b/packages/data-objectstack/src/metadata-client.layeredEnvelope.test.ts @@ -0,0 +1,258 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `MetadataClient.layered()` validates the ADR-0010 protection envelope at the + * boundary (objectui#5676). + * + * ## The defect, measured rather than argued + * + * The envelope used to arrive by ten `as` assertions over a raw `res.json()` + * body — no parse, no allowlist, no default. The banner that consumes it opens + * on `layered?.lock && layered.lock !== 'none'`, which is true for ANY non-`none` + * value, so a server sending a lock state this console had never heard of opened + * the amber box, drew the padlock and the border, and rendered an empty title. + * No fifth state ever had to be added to this repo for that to happen — which is + * why objectui#5024's duplicated-union framing could not have caught it. A union + * types what this repo WRITES and constrains nothing about what a server SENDS. + * + * ## What is pinned here, and what is deliberately pinned elsewhere + * + * The ruling is "pass through and label", extending to the whole envelope the + * treatment objectui#5672 chose for `lock` alone. That splits across two files + * and neither half is sufficient on its own: + * + * - THIS file pins the boundary: recognised values arrive typed from the + * producer's own schema, unrecognised ones are still FORWARDED (never + * dropped, never thrown) and are named in `_unrecognized`. + * - `ResourceEditPage.lockBanner.test.tsx` pins the screen: a token the banner + * cannot read is titled with the raw token instead of rendering blank. + * + * The forwarding assertions below are exactly the precondition that file's + * fixtures assume. Together they are the ruled "an unknown `lock` token still + * reaches the banner labelled"; separately, each is green against a client that + * fails the other half. + * + * ## Why `safeParse` and not `parse` + * + * A bare `.parse()` was refused by name on the filing card: it turns a + * silently-wrong render into a thrown or rejected response, which is a behaviour + * change for every consumer of this client rather than a tightening of types. A + * metadata console that rejected every dialect it had not been compiled against + * would answer a newer server with a blank page — strictly worse than the wrong + * render being fixed. The counter-probe at the bottom of this file is what keeps + * that option refused: it is green only while structurally malformed bodies + * RESOLVE, so an implementation that throws on anything it does not recognise + * cannot pass this suite even though it would satisfy every other assertion. + */ +import { describe, expect, it } from 'vitest'; +import { GetMetaItemLayeredResponseSchema } from '@objectstack/spec/api'; +import { MetadataClient } from './metadata-client'; + +const BASE_URL = 'http://localhost:3000'; + +/** A client over a fetch that answers `body` for any request. */ +function clientAnswering(body: unknown) { + return new MetadataClient({ + baseUrl: BASE_URL, + fetch: (async () => + new Response(JSON.stringify(body), { + status: 200, + headers: { 'content-type': 'application/json' }, + })) as unknown as typeof fetch, + }); +} + +/** + * A body every field of which the producer's own schema accepts. Guarded as + * such in the first test, so the conforming case cannot go green against a + * shape no server sends. + */ +const CONFORMING = { + type: 'object', + name: 'showcase_project', + code: { name: 'showcase_project', label: 'Project' }, + overlay: { label: 'Projects (ours)' }, + overlayScope: 'org', + effective: { name: 'showcase_project', label: 'Projects (ours)' }, + lock: 'no-delete', + lockReason: 'shipped by the crm package', + lockSource: 'package', + lockDocsUrl: 'https://docs.objectstack.ai/metadata/locks', + provenance: 'package', + packageId: 'crm', + packageVersion: '1.2.3', + editable: true, + deletable: false, + resettable: true, +} as const; + +/** The seven fields the filing card names as cast through unchecked. */ +const RULED_FIELDS = [ + 'overlayScope', + 'lock', + 'lockSource', + 'provenance', + 'editable', + 'deletable', + 'resettable', +] as const; + +describe('objectui#5676 · (a) a conforming envelope types cleanly', () => { + it('is a body the producer accepts in the first place', () => { + // Fixture guard before any value verdict: a vocabulary is being asserted, + // so the body must parse fully green rather than merely avoid unknown keys. + const parsed = GetMetaItemLayeredResponseSchema.safeParse(CONFORMING); + expect(parsed.success).toBe(true); + }); + + it('hands back all seven ruled fields with the values the server sent', async () => { + const layered = await clientAnswering(CONFORMING).layered('object', 'showcase_project'); + + expect(layered.overlayScope).toBe('org'); + expect(layered.lock).toBe('no-delete'); + expect(layered.lockSource).toBe('package'); + expect(layered.provenance).toBe('package'); + expect(layered.editable).toBe(true); + expect(layered.deletable).toBe(false); + expect(layered.resettable).toBe(true); + }); + + it('flags nothing — `_unrecognized` is absent, not merely empty', async () => { + const layered = await clientAnswering(CONFORMING).layered('object', 'showcase_project'); + + // Absence is the signal. An always-present empty array would make every + // consumer test for length instead of presence, and would break the exact + // `toEqual` shape assertion in `metadata-client.layeredRoute.test.ts`. + expect(layered._unrecognized).toBeUndefined(); + expect('_unrecognized' in layered).toBe(false); + }); +}); + +describe('objectui#5676 · (b) an unknown token degrades ONE field, and is labelled', () => { + const UNKNOWN_LOCK = { ...CONFORMING, lock: 'no-publish' }; + + it('is a body the producer REJECTS, so the case under test is the real one', () => { + const parsed = GetMetaItemLayeredResponseSchema.safeParse(UNKNOWN_LOCK); + expect(parsed.success).toBe(false); + }); + + it('resolves rather than throwing or rejecting', async () => { + await expect( + clientAnswering(UNKNOWN_LOCK).layered('object', 'showcase_project'), + ).resolves.toBeDefined(); + }); + + it('forwards the raw token, which is what lets the banner name it', async () => { + const layered = await clientAnswering(UNKNOWN_LOCK).layered('object', 'showcase_project'); + + // Dropping it would be the reject semantics the card refused: the operator + // who meets this banner is the only person able to report which state their + // server actually sent, and `lockBannerTitle` renders `String(lock)` for + // exactly this case (objectui#5672). + expect(layered.lock).toBe('no-publish'); + }); + + it('labels the offending field by name', async () => { + const layered = await clientAnswering(UNKNOWN_LOCK).layered('object', 'showcase_project'); + + expect(layered._unrecognized).toEqual(['lock']); + }); + + it('costs the other six fields NOTHING — the granularity that makes this a degrade', async () => { + const layered = await clientAnswering(UNKNOWN_LOCK).layered('object', 'showcase_project'); + + // The trap this closes: `GetMetaItemLayeredResponseSchema.safeParse(body)` + // is all-or-nothing. One unknown `lock` returns `success: false` with `data` + // undefined, so a boundary that leaned on it alone would degrade the WHOLE + // envelope every time a server spoke a newer dialect — a subtler version of + // the bug being fixed. These six are the assertion that it does not. + expect(layered.overlayScope).toBe('org'); + expect(layered.lockSource).toBe('package'); + expect(layered.provenance).toBe('package'); + expect(layered.editable).toBe(true); + expect(layered.deletable).toBe(false); + expect(layered.resettable).toBe(true); + }); + + it.each(RULED_FIELDS)('flags an off-spec `%s` without flagging its neighbours', async (field) => { + // Table-driven so the guard covers the whole ruled surface rather than the + // one field that happened to be reported. `null` is off-spec for every one + // of the seven except `overlayScope`, which is nullable by declaration. + const offSpec = field === 'overlayScope' ? 'tenant' : null; + const layered = await clientAnswering({ ...CONFORMING, [field]: offSpec }).layered( + 'object', + 'showcase_project', + ); + + expect(layered._unrecognized).toEqual([field]); + // …and it still arrives, rather than being silently dropped. + expect(layered[field]).toBe(offSpec); + }); +}); + +describe('objectui#5676 · (c) counter-probe — a malformed envelope must not throw', () => { + /** + * Structurally wrong, not merely unknown-valued: wrong JSON types on fields + * whose vocabulary is not even the question. Without this case, "degrade and + * label" is satisfiable by code that throws on anything it cannot recognise, + * which is precisely the option the card refused. + */ + const MALFORMED = { + ...CONFORMING, + lock: 42, + lockSource: { layer: 'artifact' }, + editable: 'yes', + overlayScope: [], + packageVersion: 7, + }; + + it('resolves — the promise is not rejected', async () => { + await expect( + clientAnswering(MALFORMED).layered('object', 'showcase_project'), + ).resolves.toBeDefined(); + }); + + it('names every malformed field and still forwards the layers', async () => { + const layered = await clientAnswering(MALFORMED).layered('object', 'showcase_project'); + + expect(layered._unrecognized).toEqual([ + 'overlayScope', + 'lock', + 'lockSource', + 'packageVersion', + 'editable', + ]); + // The three layers are the reason this endpoint exists; a malformed + // protection envelope may not cost the caller the data it came for. + expect(layered.code).toEqual(CONFORMING.code); + expect(layered.effective).toEqual(CONFORMING.effective); + // Untouched envelope fields keep their values. + expect(layered.deletable).toBe(false); + expect(layered.provenance).toBe('package'); + }); + + it.each([ + ['a body that is not an object', 'nonsense'], + ['a null body', null], + ['an array body', []], + ['every envelope field wrong at once', Object.fromEntries(RULED_FIELDS.map((f) => [f, Symbol.iterator.toString()]))], + ])('does not throw for %s', async (_label, body) => { + await expect(clientAnswering(body).layered('object', 'x')).resolves.toBeDefined(); + }); +}); + +describe('objectui#5676 · a pre-ADR-0010 server is not an error case', () => { + it('sends no protection envelope, and nothing is flagged', async () => { + // The four resolved verdicts are REQUIRED upstream on this path, so this + // body fails the whole-envelope parse and takes the degrade branch. That + // branch must be indistinguishable from the old behaviour: absence is a + // legitimate envelope, not an unrecognised one. + const legacy = { type: 'object', name: 'x', code: { a: 1 }, overlay: null, overlayScope: null, effective: { a: 1 } }; + expect(GetMetaItemLayeredResponseSchema.safeParse(legacy).success).toBe(false); + + const layered = await clientAnswering(legacy).layered('object', 'x'); + + expect(layered).toEqual({ code: { a: 1 }, overlay: null, overlayScope: null, effective: { a: 1 } }); + expect(layered._unrecognized).toBeUndefined(); + }); +}); diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index 8a99670a8b..902c2d3f22 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -32,6 +32,7 @@ * type, mirroring the framework's "single Zod source per type" rule. */ +import { GetMetaItemLayeredResponseSchema } from '@objectstack/spec/api'; import type { GetMetaItemLayeredResponse, RuntimeAuthoringIssue, @@ -286,9 +287,13 @@ export type MetadataOverlayScope = GetMetaItemLayeredResponse['overlayScope']; * the cross-repo half of the drift, not just the in-repo half. * * ⚠️ This types what this repo may WRITE. It does NOT constrain what a server - * may SEND: {@link MetadataClient.layered} casts the wire value through - * unchecked, so every reader must still handle a value outside these four. The - * lock banner in `ResourceEditPage` is the worked example. + * may SEND, and it never will — {@link MetadataClient.layered} forwards the + * wire value rather than dropping it, so every reader must still handle a value + * outside these four. What changed in objectui#5676 is that the value is no + * longer forwarded *silently*: the boundary validates it against the producer's + * own schema and names the field in {@link MetadataLayered._unrecognized} when + * it does not match. The lock banner in `ResourceEditPage` is the worked + * example of a reader that labels such a value instead of rendering it blank. */ export type MetadataLockState = GetMetaItemLayeredResponse['lock']; @@ -341,6 +346,68 @@ export interface MetadataLayered { deletable?: boolean; /** True when "Reset to package default" applies (has overlay + artifact). */ resettable?: boolean; + /** + * Protection-envelope keys whose wire value did NOT match `packages/spec`'s + * schema for them (objectui#5676). Computed HERE, at the boundary — the + * server never sends this key, and {@link MetadataClient.layered} builds the + * returned object field by field, so a server that tried could not smuggle + * one in. + * + * Absent when everything the server sent parsed, so its presence is the + * signal. The offending values are still forwarded on the fields themselves: + * this is "pass through and label", the treatment objectui#5672 chose for + * `lock` alone, applied to the whole ADR-0010 envelope. Dropping them would + * be the reject semantics that card refused — turning a wrong render into no + * render is a behaviour change for every consumer of this client. + */ + _unrecognized?: readonly string[]; +} + +/** + * The ADR-0010 protection envelope as it reaches this client, listed once so + * the boundary check below cannot drift from the interface above. + * + * The `satisfies` pins each spelling against BOTH ends at compile time: it must + * be a key {@link MetadataLayered} carries AND a key the producer's + * `GetMetaItemLayeredResponseSchema` declares. A key renamed upstream, or one + * added here without a schema to check it against, fails `type-check` naming + * itself — which is the failure mode objectui#5676 was filed about, one level up. + */ +const PROTECTION_ENVELOPE_KEYS = [ + 'overlayScope', + 'lock', + 'lockReason', + 'lockSource', + 'lockDocsUrl', + 'provenance', + 'packageId', + 'packageVersion', + 'editable', + 'deletable', + 'resettable', +] as const satisfies readonly (keyof MetadataLayered & keyof GetMetaItemLayeredResponse)[]; + +/** + * Which envelope keys the server sent that its own schema rejects. + * + * Per-key on purpose, and the granularity is the whole point. Measured on the + * installed spec (17.2.0) rather than assumed: `GetMetaItemLayeredResponseSchema + * .safeParse(body)` is ALL-OR-NOTHING — one unknown `lock` token returns + * `success: false` with `data` undefined, so the other six fields lose their + * types too. Degrading the whole envelope because one field is from a newer + * dialect would be a subtler version of the bug this fixes, so the failure + * branch re-checks each key against that same schema's own `shape[key]`, where + * only the offending one fails. + * + * A key the server omitted is not "unrecognised" — absence is a legitimate + * envelope (every field but the four resolved verdicts is optional upstream, + * and a pre-ADR-0010 backend sends none of them at all). + */ +function unrecognizedEnvelopeKeys(body: Record): string[] { + const shape = GetMetaItemLayeredResponseSchema.shape; + return PROTECTION_ENVELOPE_KEYS.filter( + (key) => body[key] !== undefined && !shape[key].safeParse(body[key]).success, + ); } /** @@ -917,8 +984,16 @@ export class MetadataClient { } if (!res.ok) throw await parseError(res); const body = (await res.json()) as MetadataLayered & Record; + // `typeof body === 'object'` rather than a bare truthiness check: `in` + // throws a TypeError on a primitive, so a server answering 200 with a bare + // JSON string or number REJECTED this promise (objectui#5676). That is the + // one outcome the boundary ruling forbids outright — a malformed body must + // degrade, never throw — and it fired before any of the validation below + // could be reached. const hasEnvelope = - body && (('code' in body) || ('overlay' in body) || ('effective' in body)); + typeof body === 'object' && + body !== null && + (('code' in body) || ('overlay' in body) || ('effective' in body)); // Left standing, but no longer reachable from a conforming server: the only // producer of a 200 body WITHOUT these keys was the retired flag's // fall-through to the plain item read, and the declared path answers 501 @@ -934,14 +1009,59 @@ export class MetadataClient { effective: body as unknown as T, }; } + // ADR-0010 Phase 4 — the metadata-protection envelope, so the editor can + // render lock affordances without a second round trip. objectui#5676: it is + // now VALIDATED here against the producer's own schema instead of cast + // through unchecked. `safeParse`, never `parse` — a metadata console that + // rejected every dialect it had not been compiled against would turn a + // newer server into a blank page, which is strictly worse than the wrong + // render this fixes. + const parsed = GetMetaItemLayeredResponseSchema.safeParse(body); + if (parsed.success) { + // The conforming path: every value below is the producer's schema output, + // so the ten `as` assertions this block used to carry are simply gone. + // `code` / `overlay` / `effective` stay asserted — upstream declares them + // `z.unknown()`, and `T` is the caller's own choice of narrowing, not + // something any schema here can check. + const envelope = parsed.data; + return { + code: (envelope.code ?? null) as T | null, + overlay: (envelope.overlay ?? null) as T | null, + overlayScope: envelope.overlayScope, + effective: (envelope.effective ?? null) as T | null, + ...(body._diagnostics ? { _diagnostics: body._diagnostics as MetadataDiagnostics } : {}), + lock: envelope.lock, + ...(envelope.lockReason !== undefined ? { lockReason: envelope.lockReason } : {}), + ...(envelope.lockSource !== undefined ? { lockSource: envelope.lockSource } : {}), + ...(envelope.lockDocsUrl !== undefined ? { lockDocsUrl: envelope.lockDocsUrl } : {}), + ...(envelope.provenance !== undefined ? { provenance: envelope.provenance } : {}), + ...(envelope.packageId !== undefined ? { packageId: envelope.packageId } : {}), + ...(envelope.packageVersion !== undefined ? { packageVersion: envelope.packageVersion } : {}), + editable: envelope.editable, + deletable: envelope.deletable, + resettable: envelope.resettable, + }; + } + + // Degrade and label — never throw, never reject. The body still reaches the + // caller field for field exactly as it did before this check existed, so + // nothing that rendered yesterday stops rendering today; what is added is + // `_unrecognized`, naming the fields the server's own schema refuses. That + // is the operator's only route to "which state did my server actually + // send", and the reason the offending value is forwarded rather than + // dropped (objectui#5672's treatment of `lock`, applied to the envelope). + // + // This branch is NOT rare, and must not be read as the error case: the + // four resolved verdicts (`lock` / `editable` / `deletable` / `resettable`) + // are required upstream on this path, so any backend older than ADR-0010 + // Phase 4 lands here with nothing unrecognised at all. + const unrecognized = unrecognizedEnvelopeKeys(body); return { code: body.code ?? null, overlay: body.overlay ?? null, overlayScope: body.overlayScope ?? null, effective: body.effective ?? null, ...(body._diagnostics ? { _diagnostics: body._diagnostics as MetadataDiagnostics } : {}), - // ADR-0010 Phase 4 — pass through the metadata-protection envelope so - // the editor can render lock affordances without a second round trip. ...(body.lock !== undefined ? { lock: body.lock as MetadataLayered['lock'] } : {}), ...(body.lockReason !== undefined ? { lockReason: body.lockReason as string } : {}), ...(body.lockSource !== undefined ? { lockSource: body.lockSource as MetadataLayered['lockSource'] } : {}), @@ -952,6 +1072,7 @@ export class MetadataClient { ...(body.editable !== undefined ? { editable: body.editable as boolean } : {}), ...(body.deletable !== undefined ? { deletable: body.deletable as boolean } : {}), ...(body.resettable !== undefined ? { resettable: body.resettable as boolean } : {}), + ...(unrecognized.length > 0 ? { _unrecognized: unrecognized } : {}), }; }