diff --git a/.changeset/references-door-organization-forwarding.md b/.changeset/references-door-organization-forwarding.md new file mode 100644 index 0000000000..a642db13da --- /dev/null +++ b/.changeset/references-door-organization-forwarding.md @@ -0,0 +1,9 @@ +--- +"@objectstack/rest": patch +--- + +The admin "Used by" panel no longer clears a delete when the caller's own organization is using the item. + +`GET /api/v1/meta/:type/:name/references` backs that panel, whose empty case reads "Nothing in the metadata graph points at this item. Safe to delete." — advice given to an operator about to delete something. The door supplied no organization, so the reference sweep read the environment partition only: an organization-scoped `view` (or `dashboard`, `report`, `translation`, `email_template`) pointing straight at the object being deleted was invisible, and the panel issued a false clearance. It now passes the caller's organization, and those references are returned. + +The organization is passed RAW, deliberately, and that is the whole of the change — no new parameter, response field or contract surface. `req.params.type` is the reference TARGET, while the sweep spends the organization on the SOURCES it reads per type; `getMetaItems` applies the `allowOrgOverride` read gate to its own request type, so each source is scoped on its own registry flag. A non-overridable source (`object`, `flow`, `app`, …) is still read environment-wide and no pre-#6190 organization-scoped row is resurrected into a delete clearance. An anonymous or organization-less caller reads exactly what it read before, and no status code or response shape moves. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 77d53a0121..6aac15a58a 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`, `:6430`, `:6678`, `:7109`, `:7302` | +| 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` | | 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/execctx-consumer-census.test.ts b/packages/rest/src/execctx-consumer-census.test.ts index 58e2a8f189..96e7c3e078 100644 --- a/packages/rest/src/execctx-consumer-census.test.ts +++ b/packages/rest/src/execctx-consumer-census.test.ts @@ -309,7 +309,23 @@ describe('[#13160] §1 the production supplier fulfils with `undefined` rather t // --------------------------------------------------------------------------- describe('[#13160] §2 the consumer surface, counted from the tree', () => { - it('76 invocation sites, 97 mentions — the thread\'s two control numbers hold', () => { + it('77 invocation sites, 98 mentions — the thread\'s two control numbers hold', () => { + // [#13753, the `/references` half] 76 → 77 sites / 97 → 98 mentions. + // `GET /meta/:type/:name/references` resolved NO identity, so the + // reference sweep behind the admin "Used by" panel read the env + // partition only and rendered "Nothing in the metadata graph points at + // this item. Safe to delete." over an organization it never read. It + // joins as a LOCALLY CAUGHT site in the continuation-line spelling, for + // the same reason as its siblings: this door does not sit behind the + // shared anonymous floor either. + // + // ⚠️ Here the two numbers moved by the SAME amount (+1 and +1), which + // is the third pattern this block has recorded and is not a mistake: + // the door's new comment states the memoised resolution in prose + // WITHOUT naming the symbol, so the call is the only new mention. A + // reader checking the +1/+2 shape of the entries below should read this + // as the mention count tracking mentions, not as a lost site. + // // [#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 @@ -361,11 +377,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(76); - expect(SOURCE.split('resolveExecCtx').length - 1).toBe(97); + expect(SITES.length).toBe(77); + expect(SOURCE.split('resolveExecCtx').length - 1).toBe(98); }); - it('the split is 23 locally caught / 53 bare — NOT 16 / 53, which does not add to 76', () => { + it('the split is 24 locally caught / 53 bare — NOT 16 / 53, which does not add to 77', () => { // 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. @@ -375,12 +391,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(23); + expect(CAUGHT.length).toBe(24); 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 23 caught ones is', () => { + it('⭐ every one of the 53 bare sites is guarded on the VERY NEXT LINE, and none of the 24 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 cc71c4a222..3b119ae046 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 @@ -341,6 +341,9 @@ function boot() { /** [#13753] The cross-type spec-validation sweep. */ diagnostics: (query: Record = {}) => drive('GET', `${META}/diagnostics`, { query }), + /** [#13753] The "Used by" sweep an operator reads before a delete. */ + references: (type: string, name: string) => + drive('GET', `${META}/:type/:name/references`, { params: { type, name } }), 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. */ @@ -786,3 +789,212 @@ describe('#13753 GET /meta/diagnostics states the org partition on the ?type= ar }); }); }); + +// ── [#13753] `GET /meta/:type/:name/references` ─────────────────────────── +// +// `findReferencesToMeta` backs the admin "Used by" panel, whose empty case +// reads — verbatim, objectui `metadata-admin/i18n.ts` — "Nothing in the +// metadata graph points at this item. Safe to delete.", shown to an operator +// about to delete something. The door named no organization, so the sweep read +// the env partition only: an org-scoped `view` pointing at the object being +// deleted was invisible and the panel issued a FALSE CLEARANCE. That is the +// ADR-0110 D3 harm this route's own 501 refusal (#9326) was added to prevent, +// answered by the door after the protocol had refused to answer it. +// +// ⭐ WHY THE DOOR PASSES THE TENANT **RAW** — and why the two cases below are a +// PAIR rather than a case and a decoration. `req.params.type` is the TARGET; +// the organization is spent on the SOURCES (`getMetaItems({ type: +// matcher.fromType, … })` per `matcher`). Pre-gating on the target the way the +// sibling `/meta` doors do would answer a question about the wrong type, and +// on a non-overridable target (`object`, `flow`, `app` — the most common +// delete there is) it would suppress the organization altogether and leave the +// false clearance exactly where it was. Raw is nevertheless not an +// unconditional tenant: since #14683 `getMetaItems` applies +// `organizationIdForMetaRead` to its OWN `request.type`, so the per-SOURCE +// decision is the callee's. +// +// ⇒ The first case pins that an OVERRIDABLE source is now found; the second +// that a NON-OVERRIDABLE source is still read env-wide, phantom row and all. +// One request, two source types, opposite scopes — which is the fact that +// makes "raw" correct and that no assertion on either case alone can state. + +/** An `object`-typed SOURCE: a lookup field naming `target`. */ +function objectReferencing(name: string, target: string): Record { + return { + // [ADR-0090 D1] `sharingModel` is required at the write door; without + // it this fixture fails on the WRITE and never reaches the read. + name, + label: MARKER, + sharingModel: 'private', + fields: { task_ref: { type: 'lookup', label: 'Task', reference: target } }, + }; +} + +/** The item an operator is about to delete — what `bodyFor('view', …)` binds to. */ +const TARGET_OBJECT = 'task'; + +describe('#13753 GET /meta/:type/:name/references states the org partition', () => { + let b: ReturnType; + beforeEach(() => { b = boot(); }); + + interface RefRow { type: string; name: string; label?: string; path: string; kind: string } + const rowsOf = (body: any): RefRow[] => (body?.references ?? []) as RefRow[]; + const namesOf = (body: any, type: string) => rowsOf(body).filter((r) => r.type === type).map((r) => r.name); + + it('⭐ THE CARD: an org-scoped `view` that references the object is FOUND', async () => { + // `view` is `allowOrgOverride: true`, so this PUT lands in the org + // partition — the fixture proof below is what makes the read + // assertion a statement about scope rather than about the store. + const written = await b.put(CACHED_ARM, 'task_list'); + expect(written.status, 'the view was never written').toBe(200); + expect( + storedRowsFor(b.rows, CACHED_ARM, 'task_list', ORG_A).length, + 'nothing landed in the org partition', + ).toBe(1); + expect( + storedRowsFor(b.rows, CACHED_ARM, 'task_list', null).length, + 'the write also landed env-wide — the partition is not real', + ).toBe(0); + + const used = await b.references(NON_OVERRIDABLE, TARGET_OBJECT); + expect(used.thrown, `the door threw: ${used.thrown?.message}`).toBeUndefined(); + expect(used.status).toBe(200); + expect( + namesOf(used.body, CACHED_ARM), + 'the sweep read a partition the caller does not live in, and the "Used by" panel ' + + 'rendered "Safe to delete." over an org-scoped view that points straight at this object', + ).toContain('task_list'); + }); + + it('⛔ NARROWNESS CONTROL: a non-overridable SOURCE stays env-wide — no phantom row is resurrected', async () => { + // The other half of the pair. `object` is `allowOrgOverride: false`, so + // its runtime writes land ENV-WIDE even under an active org + // (`organizationIdForMetaWrite`, #6190) — which is why the phantom has + // to be planted directly. Rows like it exist in deployments that ran + // before that ruling; boot hydration walks past them, so they are dead, + // and a door that named the org for EVERY source type would read them + // back into a destructive-action clearance — worse than an omission, + // because a resurrected row reads as evidence. + const written = await b.put(NON_OVERRIDABLE, 'env_orders'); + expect(written.status, 'the control never wrote').toBe(200); + // Rewrite the stored document so this object actually REFERENCES the + // target; the write door validates, so the shape is a real one. + const envRow = storedRowsFor(b.rows, NON_OVERRIDABLE, 'env_orders', null); + expect(envRow.length, 'a non-overridable write went org-scoped; the control controls nothing').toBe(1); + envRow[0].metadata = JSON.stringify(objectReferencing('env_orders', TARGET_OBJECT)); + + b.rows.set( + keyOf({ type: NON_OVERRIDABLE, name: 'phantom_orders', organization_id: ORG_A, state: 'active' }), + { + id: 'phantom_ref_1', + type: NON_OVERRIDABLE, + name: 'phantom_orders', + organization_id: ORG_A, + package_id: null, + state: 'active', + metadata: JSON.stringify(objectReferencing('phantom_orders', TARGET_OBJECT)), + }, + ); + expect( + storedRowsFor(b.rows, NON_OVERRIDABLE, 'phantom_orders', ORG_A).length, + 'the phantom was not planted; the control proves nothing', + ).toBe(1); + + // ⭐ Same request, both source types — one org-scoped `view` beside the + // two `object` rows, so the two scopes are read on ONE sweep. + await b.put(CACHED_ARM, 'task_list'); + const used = await b.references(NON_OVERRIDABLE, TARGET_OBJECT); + expect(used.status).toBe(200); + + expect( + namesOf(used.body, NON_OVERRIDABLE), + 'the env-wide `object` source was not swept at all — the exclusion below would be vacuous', + ).toContain('env_orders'); + expect( + namesOf(used.body, NON_OVERRIDABLE), + 'the door named the organization for a type with no per-org read channel — the pre-#6190 ' + + 'phantoms, resurrected on the read side inside a delete clearance', + ).not.toContain('phantom_orders'); + expect( + namesOf(used.body, CACHED_ARM), + 'the overridable source lost its org scope on the same request — the gate is not per type', + ).toContain('task_list'); + }); + + describe('⛔ controls — the scope is STATED, and nothing else moves', () => { + it('does not serve org A\'s source to org B on the same boot', async () => { + await b.put(CACHED_ARM, 'task_list'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'task_list', ORG_A).length).toBe(1); + + b.as(ORG_B); + const used = await b.references(NON_OVERRIDABLE, TARGET_OBJECT); + expect(used.status).toBe(200); + expect(namesOf(used.body, CACHED_ARM), 'org B was served org A\'s view').not.toContain('task_list'); + }); + + it('an org-LESS caller reads exactly what it read before', async () => { + await b.put(CACHED_ARM, 'task_list'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'task_list', ORG_A).length).toBe(1); + + b.as(undefined); + const used = await b.references(NON_OVERRIDABLE, TARGET_OBJECT); + expect(used.status).toBe(200); + expect( + namesOf(used.body, CACHED_ARM), + 'an anonymous / org-less read moved — this door must not change for a caller that names no org', + ).not.toContain('task_list'); + }); + + it('and still serves ENV-WIDE sources to an org-scoped caller', async () => { + // The other direction: naming the org must not narrow the answer + // an org caller could already see. + b.as(undefined); + await b.put(CACHED_ARM, 'env_task_list'); + expect(storedRowsFor(b.rows, CACHED_ARM, 'env_task_list', null).length).toBe(1); + + b.as(ORG_A); + const used = await b.references(NON_OVERRIDABLE, TARGET_OBJECT); + expect(used.status).toBe(200); + expect( + namesOf(used.body, CACHED_ARM), + 'an org session lost sight of an env-wide reference it could see before', + ).toContain('env_task_list'); + }); + + it('the response is the SAME wire shape — one `references` key, no new field', async () => { + await b.put(CACHED_ARM, 'task_list'); + const used = await b.references(NON_OVERRIDABLE, TARGET_OBJECT); + expect(used.status).toBe(200); + expect(Object.keys(used.body ?? {})).toEqual(['references']); + // The ROW shape too: a repair that added a scope discriminator per + // row would satisfy every assertion above. + expect(rowsOf(used.body).find((r) => r.name === 'task_list')).toEqual({ + type: CACHED_ARM, name: 'task_list', label: MARKER, path: 'object', kind: 'view object', + }); + }); + + it('the #9327 unanswerable-target refusal keeps its code and status', async () => { + // Asserted as `code` + `status` (ADR-0112) rather than as "it + // threw": this route's refusals are the one thing on it an operator + // reads as "the question was never asked", so a scope repair that + // moved either would be moving the destructive-action clearance. + // + // ⚠️ The code is read through BOTH refusal dialects on purpose. + // Measured on this boot, the two 501s this route can answer do not + // agree: the missing-method branch hand-builds the ADR-0112 NESTED + // `{ error: { code, message } }`, while the protocol-raised + // unanswerable-target refusal reaches the wire as the FLAT + // `{ error: 'Internal server error', code }` — the prescriptive + // "ask the owning object instead" message scrubbed. That is a + // finding of its own, filed as #15685; it is NOT this card's + // subject, and reading both keeps this pin measuring the thing it + // is about. + const refused = await b.references('field', 'account.owner'); + const body = refused.body as any; + const observed = refused.thrown + ? { status: refused.thrown.status, code: refused.thrown.code } + : { status: refused.status, code: body?.error?.code ?? body?.code }; + expect(observed).toEqual({ status: 501, code: 'NOT_IMPLEMENTED' }); + }); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 3c66d49dbb..2ad2be6257 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -5571,60 +5571,72 @@ 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. + // ── [#13753] STATE THE ORG PARTITION — and pass it + // RAW, which is the whole of the decision ─────────── // - // 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 + // The admin "Used by" panel renders its empty case as + // "Nothing in the metadata graph points at this item. + // Safe to delete." (objectui `metadata-admin/i18n.ts`), + // shown to an operator about to delete something. With + // no organization stated, the sweep read the env + // partition only: an org-scoped `view` referencing the + // item was invisible and the panel issued a false + // clearance — the ADR-0110 D3 harm this route's own 501 + // refusal (#9326) exists to prevent, delivered by the + // door after the protocol had refused to deliver it. + // + // ⛔ NOT pre-gated with `organizationIdForMetaRead( + // canonicalMetaUrlType(req.params.type), ...)`, the way + // the sibling `/meta` doors gate. Here `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. + // sites by `fromType` and reads each through + // `getMetaItems({ type: matcher.fromType, ... })`. The + // target's own registry flag therefore says nothing + // about the types actually read, and gating on it would + // suppress the organization for exactly the `object` / + // `flow` / `app` deletes this card is about — the card's + // own false clearance, left standing by a change that + // looks like its repair. // - // Gating on the target would therefore answer a - // question about the wrong type, in both directions: + // ⭐ And RAW is not the unconditional tenant that + // predicate exists to prevent, because since #14683 + // `getMetaItems` applies it ITSELF, to its OWN + // `request.type`, after the fold. The per-SOURCE-type + // decision is already the callee's: an overridable + // source (`view`, `dashboard`, `report`, `translation`, + // `email_template`) honours the organization, every + // other source drops it and stays env-wide, so no + // pre-#6190 phantom row is resurrected into a + // destructive-action clearance. `request.organizationId` + // has exactly ONE use inside `findReferencesToMeta` — + // that `getMetaItems` spread — so passing it raw carries + // no other consequence. Both halves are pinned in + // `rest-server-meta-read-org-scope.test.ts`, the second + // as the narrowness control. // - // • 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. + // ⚠️ ADR-0131 D6/D7 retires the per-organization + // metadata partition in v18 (#15206, C5), so this is a + // repair inside a mechanism being removed: an existing + // value handed to an existing parameter, no new contract + // surface. ⛔ Nothing is to be built on it. // - // ⇒ 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). + // The same memoised resolution the sibling read doors + // share, in the same locally-caught spelling: 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. + const referencesCtx = await this.resolveExecCtx(environmentId, req) + .catch(rethrowAuthzStoreUnavailable); const result = await (p as any).findReferencesToMeta({ type: req.params.type, name: req.params.name, + // SPREAD, never `organizationId: x ?? null` — the + // implementation declares `organizationId?: string` + // (optional plain string, not nullable), and it + // forwards on truthiness. + ...(referencesCtx?.tenantId ? { organizationId: referencesCtx.tenantId } : {}), ...(environmentId ? { environmentId } : {}), }); res.json(result);