diff --git a/.changeset/audit-meta-item-organization-scope.md b/.changeset/audit-meta-item-organization-scope.md new file mode 100644 index 0000000000..13466964b3 --- /dev/null +++ b/.changeset/audit-meta-item-organization-scope.md @@ -0,0 +1,47 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/rest": patch +--- + +fix(metadata-protocol): scope the metadata audit read to the caller's organization (#8747) + +`ObjectStackProtocolImplementation.auditMetaItem` declared +`organizationId?: string | null` and never read it. The comment directly above +its query described the filter it would have built — "include rows for the +specific org AND env-wide (`organization_id IS NULL`) rows" — while the `where` +was exactly `{ type, name }`. The parameter was dead on the caller side too: +`GET /api/v1/meta/:type/:name/audit` never passed one. + +The consequence was a cross-tenant disclosure, measured rather than inferred: +three saves of one view name under two organizations and env-wide, then one +`auditMetaItem({ type, name })` read, returned all three organizations' rows — +and with each row its `actor`, `note`, `lock_state`, `code`, `operation`, +`source` and `request_id`. Nothing compensated lower down. The driver's tenant +wall never engaged, because it is armed only from an execution context this +read did not pass; the security plugin's Layer 0 never engaged, because the +middleware short-circuits on a principal-less call long before the field gate +that would have carried it; and no tenancy posture would have supplied the +scope either. The route carries no capability gate — unlike its `PUT` twin, +which gates on `manage_metadata` — so the reachable cohort was any +authenticated principal of any tenant, on the published `meta.getAudit` SDK +surface. + +The query now builds the described filter: rows for the caller's organization +plus env-wide (`organization_id IS NULL`) rows, and nothing else. The env-wide +limb is load-bearing rather than defensive — the REST `PUT /meta/:type/:name` +door passes no organization, so every row it writes is stamped +`organization_id: null`, and an equality-only filter would have blanked the +audit tab on those deployments instead of scoping it. A read that resolves no +organization is fail-closed onto the env-wide rows, symmetric with what an +org-less write produces, so omitting the parameter is no longer a skeleton key. + +The REST route supplies the organization from the execution context it already +resolves for 40-plus handlers, adding no new organization-resolution plumbing +to `packages/rest`. The same call also stopped passing `environmentId`, which +the request type never declared and the method body never read; environment +scoping is unaffected, since it comes from which protocol instance is resolved +rather than from the request payload. + +Behaviour change worth stating plainly: a caller that previously saw another +tenant's metadata audit rows for a same-named item no longer sees them. Own-org +and env-wide rows are unchanged. diff --git a/packages/metadata-protocol/src/protocol.audit-org-scope.test.ts b/packages/metadata-protocol/src/protocol.audit-org-scope.test.ts new file mode 100644 index 0000000000..053a722e5e --- /dev/null +++ b/packages/metadata-protocol/src/protocol.audit-org-scope.test.ts @@ -0,0 +1,135 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #8747 — `auditMetaItem` declared `organizationId?: string | null`, never read +// it, and carried a comment describing the org filter it would have built: +// +// "Org-scoped lookup: include rows for the specific org AND env-wide +// (organization_id IS NULL) rows so the editor sees both tenant overlays +// and env-level package writes." +// +// The `where` underneath was exactly `{ type, name }`. Measured consequence: one +// read returned three organizations' rows, disclosing another tenant's `actor`, +// `note` and `lock_state` through a GA endpoint that carries no capability gate. +// +// This file pins the QUERY SHAPE, next to the code that builds it. The +// behavioural half — that the shape actually SELECTS the right rows through a +// real SQL driver, in both directions — is pinned by +// `packages/runtime/src/audit-meta-item-org-scope.integration.test.ts`, which +// needs a real driver this package does not depend on. Neither half is +// sufficient alone: a shape assertion cannot tell a correct filter from one +// that hides everything, and a row assertion cannot tell which spelling +// produced it. +// +// The test names below restate the comment's two claims deliberately. That is +// the "comment now describes behaviour that exists" pin the ruling asks for: +// each claim is an assertion, so the comment cannot drift back into +// over-claiming without a red test. + +import { describe, it, expect, vi } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** A `find` that records its options and returns nothing. */ +function makeProtocol() { + const find = vi.fn(async () => []); + const engine = { registry: { getObject: () => undefined }, find }; + return { p: new ObjectStackProtocolImplementation(engine as any), find }; +} + +/** The options the protocol handed to `engine.find` on its first call. */ +const whereFrom = (find: any) => find.mock.calls[0][1].where; + +const ORG = 'org_alpha'; + +describe('#8747 auditMetaItem builds the org scope its comment describes', () => { + it('claim 1 + 2: rows for the specific org AND env-wide (organization_id IS NULL) rows', async () => { + const { p, find } = makeProtocol(); + await p.auditMetaItem({ type: 'views', name: 'shared_grid', organizationId: ORG }); + + const where = whereFrom(find); + // Both limbs, in one `$or`. The env-wide limb is not optional: the REST + // `PUT /meta` door writes rows with `organization_id: null`, so an + // equality-only filter would blank the audit tab on those deployments. + expect(where.$or).toEqual([ + { organization_id: ORG }, + { organization_id: null }, + ]); + }); + + it('does not ALSO constrain organization_id at the top level (which would AND away the env-wide limb)', async () => { + const { p, find } = makeProtocol(); + await p.auditMetaItem({ type: 'views', name: 'shared_grid', organizationId: ORG }); + + // A top-level `organization_id` alongside the `$or` would re-narrow the + // query to the equality and silently undo the env-wide half — the exact + // "looks scoped, hides everything" shape this card warns about. + expect(whereFrom(find)).not.toHaveProperty('organization_id'); + }); + + it('still keys on (type, name), with the plural folded to singular', async () => { + const { p, find } = makeProtocol(); + await p.auditMetaItem({ type: 'views', name: 'shared_grid', organizationId: ORG }); + + const where = whereFrom(find); + expect(where.type).toBe('view'); + expect(where.name).toBe('shared_grid'); + }); + + it('an OMITTED organizationId is fail-closed: env-wide rows only, never unscoped', async () => { + const { p, find } = makeProtocol(); + // This is the exact call the production route made before the fix. + await p.auditMetaItem({ type: 'views', name: 'shared_grid' }); + + const where = whereFrom(find); + expect(where.organization_id).toBe(null); + // The absence of `$or` here is the point: there is no organization to + // widen to, so the read must not widen at all. + expect(where).not.toHaveProperty('$or'); + }); + + it('an explicit null organizationId reads env-wide, identically to omitting it', async () => { + const { p, find } = makeProtocol(); + await p.auditMetaItem({ type: 'views', name: 'shared_grid', organizationId: null }); + + const where = whereFrom(find); + expect(where.organization_id).toBe(null); + expect(where).not.toHaveProperty('$or'); + }); + + it('the parameter is READ — no call shape leaves the query without an organization term', async () => { + // The defect in one assertion: `organizationId` was inert, so every + // spelling produced the same unscoped `where`. Each spelling must now + // constrain `organization_id` one way or the other. + for (const request of [ + { type: 'views', name: 'shared_grid' }, + { type: 'views', name: 'shared_grid', organizationId: null }, + { type: 'views', name: 'shared_grid', organizationId: ORG }, + { type: 'view', name: 'shared_grid', organizationId: ORG, limit: 5 }, + ] as any[]) { + const { p, find } = makeProtocol(); + await p.auditMetaItem(request); + const where = whereFrom(find); + const scoped = where.$or !== undefined || 'organization_id' in where; + expect(scoped, `unscoped where for ${JSON.stringify(request)}`).toBe(true); + } + }); +}); + +describe('#8747 the comment and the code cannot drift apart again', () => { + it('the method that claims an org-scoped lookup is the method that builds one', () => { + const source = readFileSync(new URL('./protocol.ts', import.meta.url), 'utf8'); + const start = source.indexOf('async auditMetaItem('); + expect(start, 'auditMetaItem not found').toBeGreaterThan(-1); + // Slice to the end of the method — the next sibling member declaration. + const rest = source.slice(start); + const end = rest.indexOf('\n async ', 1); + const body = end === -1 ? rest : rest.slice(0, end); + + // The comment makes two claims. Both must be backed by code IN THE SAME + // METHOD. This is what went wrong: the prose survived, the query did + // not, and nothing failed. + expect(body).toContain('organization_id IS NULL'); + expect(body).toContain('$or'); + expect(body).toContain('request.organizationId'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 7921ac807d..f9bf09724c 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -6087,6 +6087,12 @@ export class ObjectStackProtocolImplementation implements * environment has not yet provisioned the table (legacy install * prior to ADR-0010) the call returns `{ events: [] }` instead of * raising, keeping the Studio tab harmless. + * + * `organizationId` SCOPES the read and is enforced in the query below: + * rows for that organization plus env-wide (`organization_id IS NULL`) + * rows, and nothing else. Omitted (or `null`) reads the env-wide rows + * only. It is never a hint — a caller that does not supply it does not + * get another tenant's rows. */ async auditMetaItem(request: { type: string; @@ -6113,13 +6119,58 @@ export class ObjectStackProtocolImplementation implements Math.max(1, request.limit ?? 100), 500, ); + // [#8747] `request.organizationId` is READ here. It was declared and + // never used, while the comment below described the filter it would + // have built — a live cross-tenant disclosure, measured rather than + // argued: three saves of one view name under `org_alpha`, `org_beta` + // and env-wide, then one `auditMetaItem({ type, name })`, returned all + // three orgs' rows (`actor`, `note`, `lock_state` with them). + // + // ⚠️ Nothing below this method compensates, and all three candidates + // were eliminated by measurement, not by reading: + // - the driver's tenant wall never engages — `buildDriverOptions` + // sets `DriverOptions.tenantId` only from `execCtx.tenantId` + // (`objectql/engine.ts`), and this read passes no context; + // - plugin-security's Layer 0 never engages — the middleware takes + // its principal-less `return next()` thousands of lines before the + // `objectFields.has('organization_id')` gate that would have + // carried it; + // - no posture would save it anyway: `computeTenantLayer0Filter` + // yields `null` under `single` and the deny sentinel under + // `isolated` with no tenantId. + // So the scope has to be BUILT here. It is unconditional — it does not + // depend on a posture, a principal, or a layer below choosing to act. + // + // `?? null` is the same normalization the sibling `/published` door + // applies (`request.organizationId ?? null`, mirroring what an org-less + // `publishPackageDrafts` WRITES): a caller that resolves no + // organization reads exactly the env-wide rows an org-less write + // produces. Fail-closed, and symmetric with the write path. + const organizationId = request.organizationId ?? null; try { // Org-scoped lookup: include rows for the specific org AND // env-wide (organization_id IS NULL) rows so the editor // sees both tenant overlays and env-level package writes. + // + // The env-wide limb is LOAD-BEARING, not defensive garnish: the + // REST `PUT /meta/:type/:name` door passes no `organizationId` at + // all, so every row that door writes is stamped + // `organization_id: null` (`recordMetadataAudit` persists + // `entry.organizationId ?? null`). Drop the limb and this read + // returns nothing on a REST-authored deployment — "correctly + // scoped" and "hides everything" are different behaviours and the + // tests pin them apart. const where: Record = { type: singular, name: request.name, + ...(organizationId === null + ? { organization_id: null } + : { + $or: [ + { organization_id: organizationId }, + { organization_id: null }, + ], + }), }; // `order`, NOT `direction`: the QueryAST sort shape is // `SortNodeSchema` = `{ field, order }`, and both drivers normalize diff --git a/packages/rest/src/rest-server-audit-org-scope.test.ts b/packages/rest/src/rest-server-audit-org-scope.test.ts new file mode 100644 index 0000000000..705fe1c953 --- /dev/null +++ b/packages/rest/src/rest-server-audit-org-scope.test.ts @@ -0,0 +1,170 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #8747 — `GET /api/v1/meta/:type/:name/audit` called `auditMetaItem` with +// `type`, `name`, `environmentId` and `limit`, and NO organization. The +// protocol method declared `organizationId` and never read it, so the read was +// unscoped on both ends and returned every tenant's audit rows for a +// `(type, name)`. +// +// This route has no capability gate in its handler — unlike its `PUT` twin, +// which gates on `manage_metadata` — so the reachable cohort was any +// authenticated principal of any tenant, on the published `meta.getAudit` SDK +// surface. That is why the assertions below are about the ARGUMENT rather than +// the status code: a 200 was always the answer; what leaked was the payload. +// +// The organization comes from `resolveExecCtx`, which this file already calls +// in 40+ handlers. Deliberately NOT a new `resolveActiveOrganizationId` — the +// `/published` route's comment in `rest-server.ts` records that inventing org +// plumbing in `packages/rest` under a bug fix would be a smuggled seam, and +// this change reads a field the execution context already carries instead. + +import { describe, it, expect, vi } from 'vitest'; +import { RestServer } from './rest-server.js'; + +const META = '/api/v1/meta'; +const AUDIT = `${META}/:type/:name/audit`; + +function mockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +function mockRes() { + const res: any = { + statusCode: 200, + json: vi.fn(function (this: any, body: any) { this._body = body; return this; }), + send: vi.fn(function (this: any) { return this; }), + setHeader: vi.fn(function (this: any) { return this; }), + status: vi.fn(function (this: any, code: number) { this.statusCode = code; return this; }), + header: vi.fn(function (this: any) { return this; }), + }; + return res; +} + +/** + * @param execCtx what `resolveExecCtx` resolves to for the request under test. + * `undefined` models the branch where it rejects and the handler's `.catch` + * swallows it. + */ +function boot(execCtx: any) { + const auditMetaItem = vi.fn().mockResolvedValue({ events: [] }); + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ + version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' }, + }), + auditMetaItem, + }; + const rest = new RestServer( + mockServer() as any, + protocol as any, + { api: { requireAuth: false } } as any, + ); + (rest as any).resolveExecCtx = async () => execCtx; + rest.registerRoutes(); + + const drive = async (req: Record = {}) => { + const found = (rest as any).getRoutes().find( + (r: any) => r.method === 'GET' && r.path === AUDIT, + ); + if (!found) throw new Error(`route not registered: GET ${AUDIT}`); + const res = mockRes(); + await found.handler( + { + method: 'GET', + path: AUDIT, + params: { type: 'views', name: 'shared_grid' }, + query: {}, + headers: {}, + body: {}, + ...req, + } as any, + res, + ); + return { status: res.statusCode, body: res.json.mock.calls.at(-1)?.[0] }; + }; + + return { auditMetaItem, drive }; +} + +/** The request object the route handed to `auditMetaItem`. */ +const requestFrom = (fn: any) => fn.mock.calls[0][0]; + +describe('#8747 GET /meta/:type/:name/audit scopes the read to the caller organization', () => { + it('threads the execution context tenant as `organizationId`', async () => { + const { auditMetaItem, drive } = boot({ userId: 'u1', tenantId: 'org_alpha' }); + await drive(); + + expect(auditMetaItem).toHaveBeenCalledTimes(1); + expect(requestFrom(auditMetaItem).organizationId).toBe('org_alpha'); + }); + + it('is fail-closed when the caller resolves no organization', async () => { + // A principal with no active organization must read env-wide rows, not + // become a skeleton key. `null` is the env-wide read downstream; an + // ABSENT key would be the pre-fix unscoped call. + const { auditMetaItem, drive } = boot({ userId: 'u1' }); + await drive(); + + const request = requestFrom(auditMetaItem); + expect(request.organizationId).toBe(null); + expect(request).toHaveProperty('organizationId'); + }); + + it('an unresolvable execution context never reaches the read — the anonymous floor refuses first', async () => { + // MEASURED, and it corrects an assumption worth recording: the + // handler's `.catch(() => undefined)` looks like the fail-closed path + // for this case, but it is not reachable through the route. An + // unresolved context is refused by the anonymous floor (`enforceAuth`) + // with a 401 before the handler body runs, so the protocol is never + // called at all. + // + // That floor is the ONLY gate here — this route has no capability gate, + // unlike the `PUT` twin's `manage_metadata` check — which is precisely + // why the organization scope below has to do the tenant separation. + const { auditMetaItem, drive } = boot(undefined); + const answer = await drive(); + + expect(answer.status).toBe(401); + expect(auditMetaItem).not.toHaveBeenCalled(); + }); + + it('never omits the organization — the call shape that leaked is unreachable', async () => { + for (const ctx of [ + { userId: 'u1', tenantId: 'org_alpha' }, + { userId: 'u1', tenantId: undefined }, + { userId: 'u1' }, + ]) { + const { auditMetaItem, drive } = boot(ctx); + await drive(); + const request = requestFrom(auditMetaItem); + expect( + 'organizationId' in request, + `route omitted organizationId for ctx ${JSON.stringify(ctx)}`, + ).toBe(true); + } + }); + + it('does not pass the dead `environmentId` the request type never declared', async () => { + // Swept with the fix: `auditMetaItem` neither declares nor reads it. + // Environment scoping is unaffected — it comes from WHICH protocol + // `resolveProtocol` hands back, not from this payload. + const { auditMetaItem, drive } = boot({ userId: 'u1', tenantId: 'org_alpha' }); + await drive(); + + expect(requestFrom(auditMetaItem)).not.toHaveProperty('environmentId'); + }); + + it('still forwards the (type, name) key and a well-formed limit', async () => { + const { auditMetaItem, drive } = boot({ userId: 'u1', tenantId: 'org_alpha' }); + await drive({ query: { limit: '5' } }); + + const request = requestFrom(auditMetaItem); + expect(request.type).toBe('views'); + expect(request.name).toBe('shared_grid'); + expect(request.limit).toBe(5); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index c38cbae48f..f5136c91cc 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -6418,10 +6418,40 @@ export class RestServer { const limit = req.query?.limit !== undefined ? Number(req.query.limit) : undefined; + // [#8747] SCOPE THE READ. Without an organization this + // route returned every tenant's audit rows for a + // `(type, name)` — measured, not inferred — and it carries + // no capability gate (unlike its `PUT` twin, which gates on + // `manage_metadata`), so the cohort was any authenticated + // principal of any tenant, on the published SDK surface. + // + // The organization comes from `resolveExecCtx`, which this + // file already calls in 40+ handlers including the `PUT` + // twin — `computeExecCtx` assembles `tenantId` from the + // shared `resolveAuthzContext` (an API key's principal + // tenant, else the session's `activeOrganizationId`). + // + // ⚠️ This deliberately does NOT mint the seam the + // `/published` route's comment forbids further down this + // file: no `resolveActiveOrganizationId`, no new org + // plumbing in `packages/rest`. It reads a field the + // execution context already carries. `?? null` keeps the + // fail-closed direction — an unresolved organization reads + // env-wide rows, never everyone's. + // + // `environmentId` is GONE from this payload, and that is a + // deletion of dead weight rather than a behaviour change: + // `auditMetaItem`'s request type never declared it and its + // body never read it. Environment scoping is unaffected + // because it comes from WHICH protocol `resolveProtocol` + // hands back — the same reasoning the `/published` route + // states below — not from the request payload. It is still + // read on the two lines that need it. + const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined); const result = await (p as any).auditMetaItem({ type: req.params.type, name: req.params.name, - ...(environmentId ? { environmentId } : {}), + organizationId: ctx?.tenantId ?? null, ...(limit !== undefined && Number.isFinite(limit) ? { limit } : {}), }); res.json(result); diff --git a/packages/runtime/src/audit-meta-item-org-scope.integration.test.ts b/packages/runtime/src/audit-meta-item-org-scope.integration.test.ts new file mode 100644 index 0000000000..5015e8ee23 --- /dev/null +++ b/packages/runtime/src/audit-meta-item-org-scope.integration.test.ts @@ -0,0 +1,226 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Real-engine regression for #8747 — `protocol.auditMetaItem` returned EVERY +// organization's `sys_metadata_audit` rows for a `(type, name)`, disclosing +// another tenant's `actor` / `note` / `lock_state` through Studio's audit tab +// and the published `meta.getAudit` SDK surface. +// +// ## Why this test uses a real driver rather than a recording double +// +// The defect was an ABSENT predicate, and the fix is a `$or` the SQL layer has +// to execute. A double that records the `where` proves the shape was built; it +// cannot prove the shape SELECTS the right rows, and the two failure modes here +// are opposite and equally plausible: +// +// - too wide → the disclosure is still open (the bug as filed); +// - too narrow → `organization_id = :org` alone hides the env-wide rows, and +// the audit tab goes blank on every deployment that authors through REST. +// +// That second one is not hypothetical. The REST `PUT /meta/:type/:name` door +// passes NO `organizationId`, so every row it writes is stamped +// `organization_id: null`. A fix that kept only the equality limb would look +// correct in a shape assertion and return nothing in production. So the +// env-wide row below is the DISCRIMINATING control, not a courtesy case: it is +// the assertion that separates "correctly scoped" from "hides everything". +// +// The query-shape half is pinned separately, next to the code that builds it, +// in `packages/metadata-protocol/src/protocol.audit-org-scope.test.ts`. +// +// Harness shape copied from `package-uninstall-org-scope.integration.test.ts`, +// the existing precedent for exactly this defect class (a strict +// `organization_id` equality dropping env-wide rows). + +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { + SysMetadataObject, + SysMetadataHistoryObject, + SysMetadataAuditObject, +} from '@objectstack/metadata-core'; + +const ORG_A = 'org_alpha'; +const ORG_B = 'org_beta'; +const ORG_C = 'org_gamma'; +const NAME = 'shared_grid'; + +const ACTOR_A = 'alice@alpha.example'; +const ACTOR_B = 'bob@beta.example'; +const ACTOR_ENV = 'package-installer'; + +let cleanup: Array<() => void> = []; +afterEach(() => { + for (const c of cleanup) c(); + cleanup = []; +}); + +async function boot() { + const dir = mkdtempSync(join(tmpdir(), 'os-8747-')); + cleanup.push(() => rmSync(dir, { recursive: true, force: true })); + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(dir, 'data.sqlite') }, + useNullAsDefault: true, + }); + const objects = [SysMetadataObject, SysMetadataHistoryObject, SysMetadataAuditObject] as any[]; + await driver.initObjects(objects); + const engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + for (const o of objects) engine.registry.registerObject(o, '@objectstack/platform-objects'); + cleanup.push(() => { void engine.destroy(); }); + const protocol = new ObjectStackProtocolImplementation( + engine as any, + undefined, + undefined, + 'package-author', + ); + return { engine, protocol }; +} + +const viewBody = (name: string) => ({ + name, + label: name, + type: 'grid', + object: 'anything', + viewKind: 'list', + data: { provider: 'object', object: 'anything' }, + columns: ['id'], +}); + +/** + * Three saves of ONE view name through the real `saveMetaItem` write path — + * two tenant overlays and one env-wide package write. Rows are seeded by the + * production writer, not hand-inserted, so the stamps under test are the + * stamps production produces. + */ +async function seedThreeOrgs(protocol: any) { + await protocol.saveMetaItem({ + type: 'view', name: NAME, item: viewBody(NAME), + organizationId: ORG_A, actor: ACTOR_A, source: 'studio', + }); + await protocol.saveMetaItem({ + type: 'view', name: NAME, item: viewBody(NAME), + organizationId: ORG_B, actor: ACTOR_B, source: 'studio', + }); + await protocol.saveMetaItem({ + type: 'view', name: NAME, item: viewBody(NAME), + actor: ACTOR_ENV, source: 'package', + }); +} + +const actorsOf = (result: any) => (result.events as any[]).map((e) => e.actor).sort(); + +describe('#8747 auditMetaItem organization scope (real engine + real SqlDriver)', () => { + it('seeds three organizations onto one (type, name) — the precondition the scope is judged against', async () => { + const { engine, protocol } = await boot(); + await seedThreeOrgs(protocol); + + const raw = (await engine.find('sys_metadata_audit', { where: {} })) as any[]; + const stamps = raw + .filter((r) => r.name === NAME) + .map((r) => `${r.actor}:${r.organization_id ?? 'ENV'}`) + .sort(); + + // The write path stamps all three distinctly. If this ever collapses, the + // scope assertions below would pass vacuously, so it is asserted first. + expect(stamps).toEqual([ + `${ACTOR_A}:${ORG_A}`, + `${ACTOR_B}:${ORG_B}`, + `${ACTOR_ENV}:ENV`, + ]); + }); + + it('BOTH DIRECTIONS: an org-scoped read sees its own rows AND env-wide rows, and NOT a third org', async () => { + const { protocol } = await boot(); + await seedThreeOrgs(protocol); + + const result = await (protocol as any).auditMetaItem({ + type: 'view', name: NAME, organizationId: ORG_A, + }); + const actors = actorsOf(result); + + // (1) own-org rows visible — without this the filter is "hides everything". + expect(actors).toContain(ACTOR_A); + // (2) env-wide rows visible — THE discriminating control. Package-level + // and REST-authored writes are env-wide and must stay in the tab. + expect(actors).toContain(ACTOR_ENV); + // (3) the third org is gone — the disclosure this card exists to close. + expect(actors).not.toContain(ACTOR_B); + + expect(actors).toEqual([ACTOR_A, ACTOR_ENV].sort()); + }); + + it('is symmetric — org_beta sees its own rows plus env-wide, never org_alpha', async () => { + const { protocol } = await boot(); + await seedThreeOrgs(protocol); + + const actors = actorsOf(await (protocol as any).auditMetaItem({ + type: 'view', name: NAME, organizationId: ORG_B, + })); + + expect(actors).toEqual([ACTOR_B, ACTOR_ENV].sort()); + expect(actors).not.toContain(ACTOR_A); + }); + + it('an organization with no rows of its own still sees the env-wide rows, and only those', async () => { + const { protocol } = await boot(); + await seedThreeOrgs(protocol); + + // A tenant that has never overlaid this item must still see the package + // install that put it there — and nobody else's overlays. + const actors = actorsOf(await (protocol as any).auditMetaItem({ + type: 'view', name: NAME, organizationId: ORG_C, + })); + + expect(actors).toEqual([ACTOR_ENV]); + }); + + it('an org-less read is fail-closed: env-wide rows only, never every tenant\'s', async () => { + const { protocol } = await boot(); + await seedThreeOrgs(protocol); + + // This is the exact call shape that leaked before the fix — the production + // route omitted `organizationId` entirely. It must no longer be a skeleton + // key. `?? null` folds it onto the env-wide read, symmetric with what an + // org-less write produces. + const actors = actorsOf(await (protocol as any).auditMetaItem({ + type: 'view', name: NAME, + })); + + expect(actors).toEqual([ACTOR_ENV]); + expect(actors).not.toContain(ACTOR_A); + expect(actors).not.toContain(ACTOR_B); + }); + + it('an explicit organizationId: null reads env-wide rows, same as omitting it', async () => { + const { protocol } = await boot(); + await seedThreeOrgs(protocol); + + const actors = actorsOf(await (protocol as any).auditMetaItem({ + type: 'view', name: NAME, organizationId: null, + })); + + expect(actors).toEqual([ACTOR_ENV]); + }); + + it('scoping does not disturb the (type, name) key — a different item is still excluded', async () => { + const { protocol } = await boot(); + await seedThreeOrgs(protocol); + await (protocol as any).saveMetaItem({ + type: 'view', name: 'other_grid', item: viewBody('other_grid'), + organizationId: ORG_A, actor: 'carol@alpha.example', source: 'studio', + }); + + const actors = actorsOf(await (protocol as any).auditMetaItem({ + type: 'view', name: NAME, organizationId: ORG_A, + })); + + expect(actors).not.toContain('carol@alpha.example'); + expect(actors).toEqual([ACTOR_A, ACTOR_ENV].sort()); + }); +});