diff --git a/.changeset/meta-diagnostics-org-scope.md b/.changeset/meta-diagnostics-org-scope.md new file mode 100644 index 0000000000..ebfbe14399 --- /dev/null +++ b/.changeset/meta-diagnostics-org-scope.md @@ -0,0 +1,21 @@ +--- +'@objectstack/rest': patch +--- + +`GET /api/v1/meta/diagnostics?type=` now states the caller's organization + +The cross-type spec-validation sweep behind the Studio governance directory named no +organization, so an organization's own metadata overlays were absent from it — clean tiles +rendered over a partition the sweep never read. The protocol implementation already +declares and reads `organizationId`; only the REST call site never supplied one. + +The `?type=` arm now resolves the request's memoised execution context and passes +`organizationIdForMetaRead(canonicalMetaUrlType(type), ctx.tenantId)` — the same +registry-gated predicate the list, single-item, `/layers`, `/history` and `/diff` doors +already use, so read scope and write scope cannot drift: a type the registry declares +`allowOrgOverride: false` keeps reading environment-wide, and an anonymous or +organization-less caller reads exactly what it read before. + +The untyped whole-registry sweep is deliberately unchanged and remains environment-wide: +it spans types with different `allowOrgOverride` while the request carries a single +`organizationId`, which cannot express a per-type scope. That gap is tracked on the card. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 3e311b82dc..9a858398b6 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -158,7 +158,7 @@ The largest single consumer — **17 of the 106 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4715`, `:6078`, `:6326`, `:6757`, `:6950` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4789`, `:6203`, `:6451`, `:6882`, `:7075` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:326`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | diff --git a/packages/rest/src/execctx-consumer-census.test.ts b/packages/rest/src/execctx-consumer-census.test.ts index 763a6079ea..58e2a8f189 100644 --- a/packages/rest/src/execctx-consumer-census.test.ts +++ b/packages/rest/src/execctx-consumer-census.test.ts @@ -309,7 +309,28 @@ describe('[#13160] §1 the production supplier fulfils with `undefined` rather t // --------------------------------------------------------------------------- describe('[#13160] §2 the consumer surface, counted from the tree', () => { - it('75 invocation sites, 95 mentions — the thread\'s two control numbers hold', () => { + it('76 invocation sites, 97 mentions — the thread\'s two control numbers hold', () => { + // [#13753] 75 → 76 sites / 95 → 97 mentions. `GET /meta/diagnostics` + // resolved NO identity, so the Studio governance sweep could not state + // which organization's partition it was reading and reported clean + // tiles over one it never read. It joins as a LOCALLY CAUGHT site (the + // continuation-line `.catch(rethrowAuthzStoreUnavailable)` spelling), + // like the `/history` and `/diff` doors before it: this door does not + // sit behind the shared anonymous floor either. + // + // ⚠️ +1 and +2 again, for the same reason as below: one call site, and + // one prose mention in the door's new comment recording that + // `resolveExecCtx` is memoised per request so this is not a new + // org-resolution seam. + // + // ⚠️ The site is resolved INSIDE an `if (diagnosticsType)` block rather + // than in a ternary, and the shape is load-bearing for this census: a + // ternary puts the `.catch(…)` on a continuation line with NO trailing + // `;`, which is a THIRD layout {@link catchArguments} cannot read — it + // would have counted 23 caught sites and found 22 arguments, i.e. the + // §7 CONTROL failing rather than a silent hole. Conforming to the + // house spelling was preferred over teaching the reader a layout. + // // [#13406] 73 → 75 sites / 92 → 95 mentions. The `/meta/:type/:name/ // history` and `/diff` read doors resolved NO identity, so neither // could state which organization's `sys_metadata_history` partition it @@ -340,11 +361,11 @@ describe('[#13160] §2 the consumer surface, counted from the tree', () => { // `enforceAuth` was measured NOT to be the repair). A mention count // that tracked the site count exactly would be measuring one thing // twice. - expect(SITES.length).toBe(75); - expect(SOURCE.split('resolveExecCtx').length - 1).toBe(95); + expect(SITES.length).toBe(76); + expect(SOURCE.split('resolveExecCtx').length - 1).toBe(97); }); - it('the split is 22 locally caught / 53 bare — NOT 16 / 53, which does not add to 75', () => { + it('the split is 23 locally caught / 53 bare — NOT 16 / 53, which does not add to 76', () => { // 16 sites spell the catch on the invocation line; 4 more spell it on // the continuation line. A single-line grep sees 16 and the arithmetic // silently loses four sites. @@ -354,12 +375,12 @@ describe('[#13160] §2 the consumer surface, counted from the tree', () => { // be the first of its kind and would break the structural claim below. const sameLine = CAUGHT.filter((s) => SOURCE.split('\n')[s.line - 1].includes('.catch(')); expect(sameLine.length).toBe(16); - expect(CAUGHT.length).toBe(22); + expect(CAUGHT.length).toBe(23); expect(BARE.length).toBe(53); expect(CAUGHT.length + BARE.length).toBe(SITES.length); }); - it('⭐ every one of the 53 bare sites is guarded on the VERY NEXT LINE, and none of the 22 caught ones is', () => { + it('⭐ every one of the 53 bare sites is guarded on the VERY NEXT LINE, and none of the 23 caught ones is', () => { // This inverts the reason the thread gave for doing the bare sites // first ("no local signal that a fault becomes an anonymous subject"). // The bare sites are bare BECAUSE the shared anonymous floor is the diff --git a/packages/rest/src/rest-server-meta-read-org-scope.test.ts b/packages/rest/src/rest-server-meta-read-org-scope.test.ts index a8557f39d8..5b8f48490e 100644 --- a/packages/rest/src/rest-server-meta-read-org-scope.test.ts +++ b/packages/rest/src/rest-server-meta-read-org-scope.test.ts @@ -338,6 +338,9 @@ function boot() { drive('GET', `${META}/:type/:name`, { params: { type, name } }), list: (type: string) => drive('GET', `${META}/:type`, { params: { type } }), + /** [#13753] The cross-type spec-validation sweep. */ + diagnostics: (query: Record = {}) => + drive('GET', `${META}/diagnostics`, { query }), history: (type: string, name: string) => drive('GET', `${META}/:type/:name/history`, { params: { type, name }, query: {} }), /** The fixture proof every history assertion below is gated on. */ @@ -555,3 +558,216 @@ describe('#13764 the history seams of this harness honour the org partition', () ).toEqual([]); }); }); + +// ── [#13753] `GET /meta/diagnostics` ────────────────────────────────────── +// +// The cross-type spec-validation sweep behind the Studio governance directory +// named no organization, so an org's own overlays were absent from it: clean +// tiles rendered over a partition the sweep never read. +// +// ⭐ WHY ONLY THE `?type=` ARM IS REPAIRED, and why the untyped sweep is +// PINNED AS-IS rather than left unmentioned. `getMetaDiagnostics` reads each +// swept type through `getMetaItems({ type: t, organizationId })`, and +// `getMetaItems` applies NO registry gate of its own — the organization it is +// handed is used for whatever type it is handed. So the scope is per TYPE +// while the request carries ONE `organizationId`: +// +// • `?type=` ⇒ `targetTypes` is exactly that one type, so +// `organizationIdForMetaRead` over it IS the request's whole scope. Correct +// by construction, and repaired here. +// • no `?type=` ⇒ `targetTypes` is the whole registry, five +// `allowOrgOverride: true` types beside every other declared type. One org +// id cannot say "org-scoped for those five, env-wide for the rest", and +// `getMetaItems` UNIONS the named org's rows onto the env-wide ones — so a +// tenant named there would union pre-#6190 phantom rows (org-scoped rows on +// types with no per-org read channel, which boot hydration walks past) back +// into a governance report. The gap is reported on the card and pinned +// below so it cannot widen by accident in either direction. +// +// The controls are the load-bearing half. `?type=object` proves the predicate +// is the REGISTRY-GATED one: a phantom org-scoped `object` row is planted +// directly in the store — the write door cannot produce one, by #6190 — and +// the sweep must not see it. Swap `organizationIdForMetaRead` for a raw +// `ctx?.tenantId` at the call site and that assertion, and only it, turns red. + +/** Rows in the backing store for one `(type, name, org)` slot. */ +function storedRowsFor( + rows: Map, + type: string, + name: string, + org: string | null, +): T[] { + return Array.from(rows.values()).filter( + (r) => r.type === type && r.name === name && (r.organization_id ?? null) === org, + ); +} + +describe('#13753 GET /meta/diagnostics states the org partition on the ?type= arm', () => { + let b: ReturnType; + beforeEach(() => { b = boot(); }); + + describe('the repair — a ?type= sweep sees what this organization authored', () => { + it.each(ORG_OVERRIDABLE)('%s: the org-scoped item is counted', async (type) => { + const written = await b.put(type, 'authored_at_runtime'); + expect(written.status, `PUT /${type} was not accepted`).toBe(200); + + // ⭐ Fixture proof first. "The sweep is org-scoped" is worthless if + // the fixture never created an org-scoped row — the assertion below + // would then pass or fail for a reason unrelated to org scoping. + expect( + storedRowsFor(b.rows, type, 'authored_at_runtime', ORG_A).length, + 'nothing landed in the org partition', + ).toBe(1); + expect( + storedRowsFor(b.rows, type, 'authored_at_runtime', null).length, + 'the write also landed env-wide — the partition is not real', + ).toBe(0); + + const swept = await b.diagnostics({ type }); + expect(swept.thrown, `GET /diagnostics threw: ${swept.thrown?.message}`).toBeUndefined(); + expect(swept.status).toBe(200); + expect(swept.body?.scannedTypes, 'the ?type= arm swept more than the named type').toBe(1); + expect( + swept.body?.stats?.[type]?.count, + 'the sweep reported a clean tile over a partition it never read — the card', + ).toBe(1); + expect(swept.body?.scannedItems).toBe(1); + }); + + it('a plural URL spelling is folded before the scope decision, not after', async () => { + // [#10340] The predicate is asked with `canonicalMetaUrlType(...)`, + // never the raw segment: `declaresOrgOverride` answers `false` for + // URL-only spellings, so an unfolded `views` would silently drop + // back to env-wide and this case would report a clean tile again. + await b.put(CACHED_ARM, 'authored_at_runtime'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'authored_at_runtime', ORG_A).length).toBe(1); + + const swept = await b.diagnostics({ type: 'views' }); + expect(swept.status).toBe(200); + expect( + swept.body?.stats?.views?.count, + 'the plural spelling was scoped env-wide — the fold happened after the decision', + ).toBe(1); + }); + }); + + describe('⛔ controls — the scope is STATED, never widened', () => { + it('?type=object stays env-wide and does NOT resurrect a phantom org row', async () => { + // ⭐ THE ABLATION TARGET. `object` is `allowOrgOverride: false` + + // `allowRuntimeCreate: true`, so its runtime writes land ENV-WIDE + // even under an active org (`organizationIdForMetaWrite`, #6190) — + // which is why the phantom below has to be planted directly rather + // than written through the door. Rows like it exist in deployments + // that ran before that ruling; boot hydration walks past them, so + // they are dead, and a read door that named the org for every type + // would serve them again. PREDICTED DIRECTION: replace the + // predicate with `ctx?.tenantId` at the call site and the count + // below becomes 2. + const written = await b.put(NON_OVERRIDABLE, 'accounts'); + expect(written.status, 'the control never wrote').toBe(200); + expect( + storedRowsFor(b.rows, NON_OVERRIDABLE, 'accounts', null).length, + 'a non-overridable write went org-scoped; the control no longer controls anything', + ).toBe(1); + + b.rows.set( + keyOf({ type: NON_OVERRIDABLE, name: 'phantom_orders', organization_id: ORG_A, state: 'active' }), + { + id: 'phantom_1', + type: NON_OVERRIDABLE, + name: 'phantom_orders', + organization_id: ORG_A, + package_id: null, + state: 'active', + metadata: JSON.stringify(bodyFor(NON_OVERRIDABLE, 'phantom_orders')), + }, + ); + expect( + storedRowsFor(b.rows, NON_OVERRIDABLE, 'phantom_orders', ORG_A).length, + 'the phantom was not planted; the control proves nothing', + ).toBe(1); + + const swept = await b.diagnostics({ type: NON_OVERRIDABLE }); + expect(swept.status).toBe(200); + expect( + swept.body?.stats?.[NON_OVERRIDABLE]?.count, + 'the sweep read the org partition of a type with no per-org read channel — ' + + 'the phantom rows #6190 stopped minting, resurrected on the read side', + ).toBe(1); + }); + + it('does not sweep org A\'s items for org B on the same boot', async () => { + await b.put(UNCACHED_ARM, 'tenant_bound'); + expect(storedRowsFor(b.rows, UNCACHED_ARM, 'tenant_bound', ORG_A).length).toBe(1); + + b.as(ORG_B); + const swept = await b.diagnostics({ type: UNCACHED_ARM }); + expect(swept.status).toBe(200); + expect( + swept.body?.stats?.[UNCACHED_ARM]?.count, + 'org B was swept over org A\'s items', + ).toBe(0); + }); + + it('does not serve an org-scoped item to a caller that named no org', async () => { + await b.put(CACHED_ARM, 'org_a_only'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'org_a_only', ORG_A).length).toBe(1); + + b.as(undefined); + const swept = await b.diagnostics({ type: CACHED_ARM }); + expect(swept.status).toBe(200); + expect( + swept.body?.stats?.[CACHED_ARM]?.count, + 'an org-less caller was swept over an org-scoped item', + ).toBe(0); + }); + + it('still sweeps env-wide items for an org-scoped caller', async () => { + // The other direction of the same harness: naming the org for org + // callers must not disturb the env-wide read that worked all along. + b.as(undefined); + await b.put(CACHED_ARM, 'env_authored'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'env_authored', null).length).toBe(1); + + b.as(ORG_A); + const swept = await b.diagnostics({ type: CACHED_ARM }); + expect(swept.status).toBe(200); + expect( + swept.body?.stats?.[CACHED_ARM]?.count, + 'an org session lost sight of an env-wide item it could read before', + ).toBe(1); + }); + }); + + describe('the RECORDED GAP — the untyped sweep is still env-wide', () => { + it('an org-scoped item is absent from the whole-registry sweep', async () => { + // ⚠️ This pins a KNOWN GAP, deliberately, so that closing it is a + // decision somebody makes rather than a side effect: one + // `organizationId` cannot express the per-type scope a + // whole-registry sweep needs, and the shape is reported on the card + // with a proposal. If this reddens, the untyped arm has started + // naming an organization — read the card before making it green. + await b.put(CACHED_ARM, 'authored_at_runtime'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'authored_at_runtime', ORG_A).length).toBe(1); + + const swept = await b.diagnostics(); + expect(swept.status).toBe(200); + expect( + swept.body?.scannedTypes, + 'the untyped arm did not sweep the registry; the assertion below would be vacuous', + ).toBeGreaterThan(1); + expect(swept.body?.stats?.[CACHED_ARM]?.count).toBe(0); + }); + + it('and still sees env-wide items — the zero above is scope, not a broken sweep', async () => { + b.as(undefined); + await b.put(CACHED_ARM, 'env_authored'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'env_authored', null).length).toBe(1); + + b.as(ORG_A); + const swept = await b.diagnostics(); + expect(swept.status).toBe(200); + expect(swept.body?.stats?.[CACHED_ARM]?.count).toBe(1); + }); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 1778442c8e..08cdde8c77 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -4565,10 +4565,84 @@ export class RestServer { if (refuseRepeatedQueryParams(req, res, ['severity', 'type', 'package'])) return; const severityParam = (req.query?.severity as string | undefined) ?? 'error'; const severity = severityParam === 'warning' ? 'warning' : 'error'; + const diagnosticsType = (req.query?.type as string | undefined) || undefined; + // [#13753] STATE THE ORG PARTITION — but only on the + // arm where ONE organization is the whole truth. + // + // `getMetaDiagnostics` reads each type through + // `getMetaItems({ type: t, organizationId })`, and + // `getMetaItems` applies NO registry gate of its own: + // whatever organization arrives is used for the type it + // is handed, overridable or not (measured — the only + // `organizationIdForMetaRead` call inside + // `metadata-protocol` is the `page` read in + // `protocol.ts`, nothing on this path). The scope is + // therefore decided HERE, per type, by the caller. + // + // ⇒ The `?type=` arm is exactly one type + // (`targetTypes = [request.type]`), so the predicate + // over that one type IS the request's whole scope and + // the answer is correct by construction. That is the + // arm Studio's per-type directory drill-down uses, and + // it is the arm repaired here. + // + // ⛔ The UNTYPED sweep is deliberately left env-wide, + // and this is a recorded gap rather than an oversight + // (#13753 reports the shape). `targetTypes` is then the + // whole registry — five `allowOrgOverride: true` types + // and every other declared type together — while the + // request carries ONE `organizationId`. Naming the + // tenant there does not merely over-reach: `getMetaItems` + // UNIONs the env-wide rows with the named org's rows, + // so a non-overridable type's org-scoped rows — the + // pre-#6190 phantoms `reportUnhydratableOrgScopedRows` + // warns about, which boot hydration walks past — would + // be read back INTO the governance report as `stats` + // counts and diagnostic entries. A dashboard whose job + // is reporting what is wrong would report rows that do + // not survive a restart. One org id cannot express a + // per-type scope, and inventing one at this call site + // (a fan-out per overridable type, plus a REST-side + // re-aggregation of `total`/`stats`/`scannedTypes`) + // would make this door a second owner of the sweep's + // arithmetic. The decision belongs where the type is + // known — see the card. + // + // ⚠️ NOT a new org-resolution seam: `resolveExecCtx` is + // memoised per request (WeakMap keyed by `req`), the + // same result 40+ handlers here already share. It is + // resolved only on the typed arm so the untyped sweep + // keeps its exact behaviour today, authz-store failure + // modes included — which is why this reads as a + // statement rather than a ternary: the LOCALLY CAUGHT + // continuation-line spelling is the one the sibling + // doors use and the one `execctx-consumer-census` + // reads, and a third layout would be invisible to it. + let diagnosticsOrganizationId: string | undefined; + if (diagnosticsType) { + const diagnosticsCtx = await this.resolveExecCtx(environmentId, req) + .catch(rethrowAuthzStoreUnavailable); + diagnosticsOrganizationId = organizationIdForMetaRead( + // [#10340] FOLDED, not raw — see the PUT door's + // org-scope comment for the measurement. The + // protocol keeps receiving the caller's own + // spelling (it normalises, and refuses an + // unrecognised one with its own 400); only the + // scope decision reads the canonical singular. + canonicalMetaUrlType(diagnosticsType), diagnosticsCtx?.tenantId, + ); + } const result = await (p as any).getMetaDiagnostics({ - type: (req.query?.type as string | undefined) || undefined, + type: diagnosticsType, severity, packageId: (req.query?.package as string | undefined) || undefined, + // SPREAD, never `organizationId: x ?? null` — the + // implementation declares `organizationId?: string` + // (optional plain string, not nullable), and a + // `null` would travel into `getMetaItems` as an + // explicit env-partition statement rather than as + // "unstated". + ...(diagnosticsOrganizationId ? { organizationId: diagnosticsOrganizationId } : {}), }); res.json(result); } catch (error: any) { @@ -5270,6 +5344,57 @@ export class RestServer { }); return; } + // [#13753] ⛔ STILL NO `organizationId`, and that is a + // RECORDED GAP, not an omission nobody looked at. Read + // this before adding the one-line repair that looks + // obviously missing here. + // + // The card prescribed the sibling call-site fix — + // `organizationIdForMetaRead(canonicalMetaUrlType( + // req.params.type), ctx?.tenantId)` — on the premise + // that this door "takes one type". Measured on the + // merged tree, it does not: `req.params.type` is the + // TARGET, and `findReferencesToMeta` spends the + // organization on the SOURCES. It resolves + // `REFERENCE_SITES.byTarget.get(target)`, groups the + // sites by `fromType`, and reads each with + // `getMetaItems({ type: matcher.fromType, + // ...(organizationId ? { organizationId } : {}) })`. So + // one request-level organization is applied to a SET of + // types the target's own registry flag says nothing + // about, and `getMetaItems` applies no gate of its own. + // + // Gating on the target would therefore answer a + // question about the wrong type, in both directions: + // + // • target `allowOrgOverride: true` (`view`, + // `dashboard`, `report`, `translation`, + // `email_template`) ⇒ the org is named for EVERY + // source type, `object` / `flow` / `app` included — + // the unconditional tenant the read predicate exists + // to prevent, unioning pre-#6190 phantom rows back + // into a destructive-action clearance; + // • target `allowOrgOverride: false` (`object`, + // `flow`, `app`, `page`, …) ⇒ nothing is named, so + // an org-scoped `view` that references the object + // being deleted stays invisible and the "Used by" + // panel still renders "Nothing in the metadata graph + // points at this item. Safe to delete." That is the + // card's own false clearance, on the most common + // delete there is. + // + // ⇒ The correct scope is per SOURCE type, and no value + // this call site can pass expresses it. The repair + // belongs where the type being read is known — the + // predicate applied per `matcher.fromType` inside + // `findReferencesToMeta`, or once inside `getMetaItems` + // so read scope cannot drift from write scope for ANY + // caller. Both are `metadata-protocol` changes that the + // card fences off (⛔ "Do not change ... in + // `protocol.ts`"), so this door is reported rather than + // half-repaired: an org-awareness this door cannot + // deliver must not be advertised by a gate that happens + // to read `true` (Prime Directive #10). const result = await (p as any).findReferencesToMeta({ type: req.params.type, name: req.params.name,