fix(metadata-protocol): scope the metadata audit read to the caller's organization (#8747)

.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.

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');
+ });
+});

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<string, unknown> = {
  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