diff --git a/.changeset/diagnostics-untyped-sweep-organization-forwarding.md b/.changeset/diagnostics-untyped-sweep-organization-forwarding.md new file mode 100644 index 0000000000..259062b790 --- /dev/null +++ b/.changeset/diagnostics-untyped-sweep-organization-forwarding.md @@ -0,0 +1,9 @@ +--- +"@objectstack/rest": patch +--- + +An organization-scoped caller's own items now appear in the untyped metadata diagnostics sweep. + +`GET /api/v1/meta/diagnostics` has two arms. The `?type=` arm has stated the caller's organization since #13753; the untyped whole-registry sweep passed none, so the Studio governance summary reported clean tiles over a partition it never read — undercounting relative to the per-type drill-down screen you reach by clicking into it. A summary whose whole job is surfacing problems, and which structurally cannot see a class of them while its own drill-down can, issues a false all-clear. The untyped arm now forwards the caller's organization, so items that organization authored on the five `allowOrgOverride: true` types (`view`, `dashboard`, `report`, `translation`, `email_template`) are counted in `stats`, `total` and `scannedItems`. + +The organization is passed RAW, deliberately, and that is the whole of the change — no new parameter, response field, status code or contract surface. There is no single type to fold on for a whole-registry sweep, and folding on any one of them would suppress the organization for every type at once; instead `getMetaDiagnostics` reads each swept type through `getMetaItems`, which applies the `allowOrgOverride` read gate to its own request type, so every type is scoped on its own registry flag. A non-overridable type (`object`, `flow`, `app`, …) is still read environment-wide and no pre-#6190 organization-scoped row is resurrected into the report. An anonymous or organization-less caller reads exactly what it read before, and the `stats` / `total` / `scannedTypes` arithmetic is unchanged in shape. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6aac15a58a..7da996623b 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:5016`, `:6442`, `:6690`, `:7121`, `:7314` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:5048`, `:6474`, `:6722`, `:7153`, `:7346` | | 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:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:422`, `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/rest-server-meta-read-org-scope.test.ts b/packages/rest/src/rest-server-meta-read-org-scope.test.ts index 3b119ae046..679d990de6 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 @@ -568,40 +568,50 @@ describe('#13764 the history seams of this harness honour the org partition', () // 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 +// ⭐ BOTH ARMS STATE THE ORGANIZATION — and the split below is about WHERE the +// fold happens, not about whether one happens. `getMetaDiagnostics` reads each // swept type through `getMetaItems({ type: t, organizationId })`. // // ⚠️ [#14683, recorded by #15034] `getMetaItems` NOW APPLIES THE REGISTRY GATE // ITSELF, after folding the request type. This header used to say it applied // none and that the scope was therefore the caller's to decide per type; that -// sentence is FALSE on today's tree. What survives it is the arm split below, -// which is about how many types ONE `organizationId` is asked to cover: +// sentence is FALSE on today's tree. What that dissolved is the obstacle the +// untyped arm was held shut on: // // • `?type=` ⇒ `targetTypes` is exactly that one type, so // `organizationIdForMetaRead` over it IS the request's whole scope. Correct -// by construction, and repaired here. +// by construction; repaired by #13753 and untouched since. // • no `?type=` ⇒ `targetTypes` is the whole registry, five -// `allowOrgOverride: true` types beside every other declared type. The arm -// names no organization at all, so nothing is folded and nothing is -// unioned. ⚠️ The reason it stays that way is no longer "one org id cannot -// say org-scoped for those five, env-wide for the rest" — since #14683 the -// inner gate folds each `t` separately inside the sweep's own loop, so it -// could. It stays because closing it MOVES BEHAVIOUR and is somebody's -// decision on a card. The gap is pinned below so it cannot widen by -// accident in either direction. +// `allowOrgOverride: true` types beside every other declared type. ⭐ +// [#15622] This arm now forwards the caller's organization **RAW** and lets +// the callee's per-`t` gate narrow it: the org for those five, `undefined` +// for every other type, so no pre-#6190 phantom is unioned back in. ⛔ It +// must NOT be pre-folded at the door — there is no single type to fold on, +// and folding on any one of them would suppress the organization for every +// type at once. +// +// ⚠️ THE GAP THIS SECTION USED TO PIN OPEN IS CLOSED, and the pin was REPLACED +// rather than deleted. `an org-scoped item is absent from the whole-registry +// sweep` carried "if this reddens, read the card before making it green"; +// #15622 is that card, and it ruled the arm forwards. Its inverse now stands in +// the same place, beside the narrowness control #15622 named as missing — an +// overridable type's org-authored row PRESENT and a planted phantom on a +// non-overridable type ABSENT, on ONE request. Read #15622 before touching +// either half: alone, neither can tell a per-type gate from an unconditional +// tenant. // // ── ⛔ WHAT THIS FILE NO LONGER DISCRIMINATES (#15034, MEASURED) ─────────── // // This header used to end: "Swap `organizationIdForMetaRead` for a raw // `ctx?.tenantId` at the call site and that assertion, and only it, turns red." -// MEASURED on the merged tree, that ablation now leaves this file 30/30 GREEN -// — `getMetaItems`' own gate re-folds the raw tenant id, phantom control -// included. Same fate as #14677's ablation B, and for the same reason. +// MEASURED on the merged tree, that ablation now leaves this file GREEN IN FULL +// (30/30 at that revision; the file has grown since) — `getMetaItems`' own gate +// re-folds the raw tenant id, phantom control included. Same fate as #14677's +// ablation B, and for the same reason. // // ⇒ What this file DOES still discriminate is the organization being DROPPED: // remove the `organizationId` the `?type=` arm passes and the six repair cases -// above turn red (measured: 6 failed / 24 passed). Read the two apart before +// above turn red (measured at #15034: 6 failed / 24 passed). Read the two apart before // citing this file as a pin on the door-side predicate — it pins that the arm // still FOLDS, never that the fold happens at the door. @@ -757,27 +767,137 @@ describe('#13753 GET /meta/diagnostics states the org partition on the ?type= ar }); }); - 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. + describe('#15622 the whole-registry sweep states the org partition too', () => { + it('⭐ THE CARD: an org-authored item on an overridable type IS counted', async () => { + // ⚠️ THIS CASE REPLACES the pin `an org-scoped item is absent from + // the whole-registry sweep`, which asserted the OPPOSITE and + // carried "if this reddens, read the card before making it green". + // #15622 IS that card. It ruled the untyped arm forwards the + // caller's organization RAW, because since #14683 the callee folds + // per swept type inside its own loop — so one org id now expresses + // exactly the per-type scope the old pin said it could not. The + // assertion is INVERTED rather than deleted so the next reader sees + // the flip and its reason, and so the arm cannot drift back to + // env-wide unnoticed. + // + // ⭐ Fixture proof first, for the same reason as the `?type=` cases + // above: "the sweep is org-scoped" says nothing if the fixture never + // created an org-scoped row. await b.put(CACHED_ARM, 'authored_at_runtime'); - expect(storedRowsFor(b.rows, CACHED_ARM, 'authored_at_runtime', ORG_A).length).toBe(1); + expect( + storedRowsFor(b.rows, CACHED_ARM, 'authored_at_runtime', ORG_A).length, + 'nothing landed in the org partition', + ).toBe(1); + expect( + storedRowsFor(b.rows, CACHED_ARM, 'authored_at_runtime', null).length, + 'the write also landed env-wide — the partition is not real', + ).toBe(0); const swept = await b.diagnostics(); + expect(swept.thrown, `GET /diagnostics threw: ${swept.thrown?.message}`).toBeUndefined(); 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); + expect( + swept.body?.stats?.[CACHED_ARM]?.count, + 'the governance summary reported a clean tile over a partition it never read, ' + + 'while its own ?type= drill-down could see the item — the card', + ).toBe(1); }); - it('and still sees env-wide items — the zero above is scope, not a broken sweep', async () => { + it('⛔ NARROWNESS CONTROL: a non-overridable type stays env-wide in the SAME sweep', async () => { + // ⭐ THE HALF #15622 NAMED AS MISSING. Without it the change is + // unmeasured: the case above passes just as well for a door that + // hands the callee an UNCONDITIONAL tenant, and that door would + // union a non-overridable type's org-scoped rows — the pre-#6190 + // phantoms `reportUnhydratableOrgScopedRows` warns about, which boot + // hydration walks past — back INTO the governance report as `stats` + // counts. A dashboard whose job is reporting what is wrong would + // report rows that do not survive a restart. This control is what + // proves the CALLEE'S PER-TYPE GATE is doing the work. + // + // `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 + // has to be planted directly rather than written through the door. + 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_sweep_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); + + // ⭐ ONE REQUEST, BOTH TYPES — an org-authored `view` beside the two + // `object` rows, so the two opposite scopes are read on ONE sweep. + // That pairing is the fact neither assertion can state alone. + 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?.stats?.[NON_OVERRIDABLE]?.count, + 'the untyped sweep read the org partition of a type with no per-org read channel — ' + + 'the pre-#6190 phantoms, resurrected inside the governance report. The door passed ' + + 'an unconditional tenant, or the callee stopped gating per type', + ).toBe(1); + expect( + swept.body?.stats?.[CACHED_ARM]?.count, + 'the overridable type lost its org scope on the same request — the gate is not per type', + ).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(); + expect(swept.status).toBe(200); + expect( + swept.body?.stats?.[UNCACHED_ARM]?.count, + 'org B was swept over org A\'s items — forwarding became a cross-tenant read', + ).toBe(0); + }); + + it('an org-LESS caller reads exactly what it read before', async () => { + // ⛔ #15622 moves NO anonymous / organization-less read. This arm + // resolves an exec ctx it did not resolve before, so the case that + // names no org is the one that could regress silently. + 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(); + 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: naming the org must not NARROW what an org + // caller could already see. b.as(undefined); await b.put(CACHED_ARM, 'env_authored'); expect(storedRowsFor(b.rows, CACHED_ARM, 'env_authored', null).length).toBe(1); @@ -785,7 +905,29 @@ describe('#13753 GET /meta/diagnostics states the org partition on the ?type= ar b.as(ORG_A); const swept = await b.diagnostics(); expect(swept.status).toBe(200); - expect(swept.body?.stats?.[CACHED_ARM]?.count).toBe(1); + expect( + swept.body?.stats?.[CACHED_ARM]?.count, + 'an org session lost sight of an env-wide item it could read before', + ).toBe(1); + }); + + it('the response is the SAME wire shape — no new key, and 200 either way', async () => { + // #15622 forwards an EXISTING value to an EXISTING parameter: no new + // parameter, response field or status code. A repair that added a + // scope discriminator to the envelope would satisfy every assertion + // above and still be a contract change. + await b.put(CACHED_ARM, 'authored_at_runtime'); + const swept = await b.diagnostics(); + expect(swept.status).toBe(200); + expect(Object.keys(swept.body ?? {}).sort()).toEqual( + ['entries', 'scannedItems', 'scannedTypes', 'stats', 'total'], + ); + // The `stats` ROW shape too — the arithmetic is unchanged in shape, + // only in what the sweep can now see. + expect(Object.keys(swept.body?.stats?.[CACHED_ARM] ?? {}).sort()).toEqual( + ['count', 'locked', 'packages'], + ); + expect(typeof swept.body?.total).toBe('number'); }); }); }); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 2ad2be6257..1f1ee559ad 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -4749,8 +4749,9 @@ export class RestServer { 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. + // [#13753, #15622] STATE THE ORG PARTITION — on BOTH + // arms. They differ only in whether the fold happens + // HERE or is left entirely to the callee. // // `getMetaDiagnostics` reads each swept type through // `getMetaItems({ type: t, organizationId })`. @@ -4774,7 +4775,7 @@ export class RestServer { // 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. + // it is the arm #13753 repaired. // // ── WHY THE FOLD IS DOUBLED, AND STAYS DOUBLED (#15034) ── // @@ -4785,7 +4786,8 @@ export class RestServer { // `canonicalMetaType` — so `f(t, f(t, o)) === f(t, o)` and the // inner application is the algebraic no-op. MEASURED: replace // this predicate with a raw `diagnosticsCtx?.tenantId` and - // `rest-server-meta-read-org-scope.test.ts` stays 30/30 GREEN; + // `rest-server-meta-read-org-scope.test.ts` stays GREEN IN FULL + // (30/30 at that revision; the file has grown since); // the inner gate re-folds it, phantom control included. // // ⭐ It is KEPT anyway, and the reason is TRUST DOMAIN rather @@ -4801,55 +4803,84 @@ export class RestServer { // bundled implementation. Defence in depth, on a seam the type // system does not cover. // - // ⛔ 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. + // ── [#15622] THE UNTYPED SWEEP FORWARDS THE + // ORGANIZATION TOO, and passes it RAW ─────────────── // - // ⚠️ #14683 MOVED THIS ARGUMENT and the gap outlived it, so - // read the two apart. What used to hold the untyped arm shut - // was that one org id could not express a per-type scope from - // here without a fan-out per overridable type and a REST-side - // re-aggregation of `total`/`stats`/`scannedTypes`. That is no - // longer the obstacle: `getMetaDiagnostics` calls - // `getMetaItems` once per `t` INSIDE its own loop, and the - // inner gate folds each `t` separately, so a single - // `organizationId` handed to the untyped arm would already be - // narrowed per type — phantoms of non-overridable types - // included. ⛔ The gap nevertheless stays OPEN and stays - // PINNED: closing it moves observable behaviour and is a - // decision somebody makes on a card, not a side effect of a - // comment repair (#15034 files it). The pin that guards it is - // `the untyped sweep is still env-wide` in - // `rest-server-meta-read-org-scope.test.ts` — if it reddens, - // read that card before making it green. + // ⛔ This arm used to be a RECORDED GAP, left env-wide + // on this argument: `targetTypes` is then the whole + // registry — five `allowOrgOverride: true` types and + // every other declared type together — while the + // request carries ONE `organizationId`, and one org id + // could not express a per-type scope from here without + // a fan-out per overridable type plus a REST-side + // re-aggregation of `total`/`stats`/`scannedTypes`. + // + // ⚠️ #14683 DISSOLVED THAT OBSTACLE (#15034 recorded + // it, #15622 acted on it). `getMetaDiagnostics` does + // not spend the organization once: it loops `for (const + // t of targetTypes)` calling `getMetaItems({ type: t, + // organizationId, … })`, and the FIRST thing + // `getMetaItems` does with that organization is + // `organizationIdForMetaRead(request.type, …)` on its + // OWN folded type. So one `organizationId` handed to + // this arm is already narrowed PER TYPE by the callee — + // the org for the five overridable types, `undefined` + // for every other, phantoms of non-overridable types + // dropped. That is precisely the scope the paragraph + // above said one id could not say. No fan-out, no + // REST-side re-aggregation, no second owner of the + // sweep's arithmetic: `stats` / `total` / + // `scannedTypes` are untouched by the gate. + // + // ⭐ RULED that the gap CLOSES rather than being + // re-recorded. A governance summary whose whole job is + // surfacing problems, and which structurally cannot see + // a class of them WHILE ITS OWN drill-down can, issues a + // false all-clear — since #13753 repaired the `?type=` + // arm, this summary undercounts relative to the screen + // you reach by clicking into it. An org-scoped caller + // now sees items THEIR OWN organization authored, on the + // five overridable types only, which for a governance + // report is the correct set. + // + // ⛔ RAW, and deliberately NOT pre-folded with + // `organizationIdForMetaRead(...)` the way the `?type=` + // arm folds above. There is no single type to fold on + // here, and folding on any one of them would suppress + // the organization for EVERY type at once. The per-type + // decision belongs to the callee's loop. Identical in + // shape to the `/references` door below, whose + // narrowness control measured the same callee gate; both + // halves are pinned in + // `rest-server-meta-read-org-scope.test.ts`, where ONE + // request shows an overridable type's org-authored row + // present and a planted pre-#6190 phantom on a + // NON-overridable type absent. + // + // ⚠️ ADR-0131 D6/D7 retires the per-organization + // metadata partition in v18 (#15206, C5), so this + // behaviour has ONE MAJOR to live and reverts to + // environment-wide when the partition goes. An existing + // value handed to an existing parameter: no new + // parameter, response field, status code or contract + // surface. ⛔ Nothing is to be built on it. // // ⚠️ 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 + // same result 40+ handlers here already share. It is now + // resolved for BOTH arms — 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( + // This door does not sit behind the shared anonymous + // floor, so it decides an authz-store outage for itself + // rather than laundering it into an org-unscoped 200 — + // and the untyped arm now shares that, deliberately. + const diagnosticsCtx = await this.resolveExecCtx(environmentId, req) + .catch(rethrowAuthzStoreUnavailable); + const diagnosticsOrganizationId: string | undefined = diagnosticsType + ? organizationIdForMetaRead( // [#10340] FOLDED, not raw — see the PUT door's // org-scope comment for the measurement. The // protocol keeps receiving the caller's own @@ -4857,8 +4888,9 @@ export class RestServer { // unrecognised one with its own 400); only the // scope decision reads the canonical singular. canonicalMetaUrlType(diagnosticsType), diagnosticsCtx?.tenantId, - ); - } + ) + // [#15622] The whole-registry arm — raw, per above. + : diagnosticsCtx?.tenantId; const result = await (p as any).getMetaDiagnostics({ type: diagnosticsType, severity,