fix(analytics): make the read-scope auto-bridge order-independent + first real e2e RLS gate (#3618)

.changeset/analytics-read-scope-bridge-order.md

@@ -0,0 +1,28 @@
+---
+"@objectstack/service-analytics": patch
+---
+
+fix(analytics): the read-scope auto-bridge no longer depends on plugin order (#3618)
+
+`getReadScope` was only wired when the `security` service already existed at this
+plugin's `init()`. The closure itself resolved lazily, but the ASSIGNMENT was
+gated on an init-time probe — so a kernel that registers `AnalyticsServicePlugin`
+before the security plugin got **no read-scope provider at all**, and every
+analytics strategy ran unscoped with only a WARN to show for it.
+
+Both sibling bridges (`executeAggregate`, `executeRawSql`) are wired
+unconditionally and resolve at call time, and this one's own comment claimed the
+same. Now it actually does: the probe only decides the log wording.
+
+The CLI (`os serve`) registers security before analytics, so that path was
+already correct. The exposure was for embedders composing their own kernel — and
+for this repo's own `bootStack` harness, which registers analytics first, meaning
+the entire dogfood/verify suite had analytics RLS silently disabled and any RLS
+assertion written there passed vacuously.
+
+Also corrects the WARN text: with no provider, scoping is absent on ALL paths and
+ALL objects, not just "the raw-SQL path" and "joined objects" as it claimed.
+
+Adds `analytics-rls.dogfood.test.ts`: an owner-scoped RLS fixture driven over real
+HTTP as a real non-admin, asserting the rows a member's aggregate actually
+returns. Reverting either this fix or the #3597 strategy fix turns it red.

packages/qa/dogfood/test/analytics-rls.dogfood.test.ts

@@ -0,0 +1,146 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// END-TO-END gate for #3597 — "analytics aggregates are scoped to the caller",
+// proven through the REAL stack: HTTP → REST execution context → SecurityPlugin
+// → the AnalyticsServicePlugin auto-bridges → ObjectQL/NativeSQL strategies.
+//
+// #3597 was a cross-tenant disclosure: ObjectQLStrategy never consumed
+// `getReadScope`, and the `executeAggregate` bridge passes no ExecutionContext,
+// so plugin-security's principal-less fall-open skipped its own RLS injection
+// too. Both belts were off at once and an authenticated non-admin received
+// aggregates computed over every row in the table.
+//
+// Every pre-existing analytics RLS test injects `getReadScope` as a fake into a
+// hand-built AnalyticsService. NONE booted the real plugin, so the
+// `getReadScope → security.getReadFilter` auto-bridge (plugin.ts:291-305) had
+// zero coverage — which is how the gap shipped. This test closes that: it
+// asserts on ROWS RETURNED to a real non-admin over HTTP, not on a filter object.
+//
+// Owner-scoped (`created_by`), not org-scoped, on purpose: a wildcard
+// `organization_id` policy is STRIPPED when the enterprise `org-scoping` service
+// is absent, which it is in this OSS workspace. An owner policy references
+// `current_user.id` and survives single-tenant boots — so this gate actually
+// bites on every CI run instead of silently passing.
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { bootStack, type VerifyStack } from '@objectstack/verify';
+import {
+  rlsFixtureStack,
+  ownerScopedMemberSet,
+  rlsFixtureSecurity,
+} from './fixtures/rls-owner-fixture.js';
+
+const ADMIN_NOTES = 3;
+const MEMBER_NOTES = 2;
+
+/**
+ * Date-bucketed dataset. The granularity is what makes this interesting:
+ * `NativeSQLStrategy.canHandle` DECLINES any query carrying a
+ * `dateGranularity` (native-sql-strategy.ts:30), so this routes to
+ * ObjectQLStrategy — the leaking path — even on a SQL-capable driver.
+ */
+const notesByDay = {
+  name: 'notes_by_day',
+  label: 'Notes by day',
+  object: 'rls_note',
+  dimensions: [
+    { name: 'created', label: 'Created', field: 'created_at', type: 'date', dateGranularity: 'day' },
+  ],
+  measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }],
+};
+
+/** Same object, NO granularity → stays on NativeSQLStrategy. The control. */
+const notesTotal = {
+  name: 'notes_total',
+  label: 'Notes total',
+  object: 'rls_note',
+  dimensions: [],
+  measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }],
+};
+
+describe('dogfood: analytics aggregates are RLS-scoped to the caller (#3597)', () => {
+  let stack: VerifyStack;
+  let adminToken: string;
+  let memberToken: string;
+
+  beforeAll(async () => {
+    stack = await bootStack(rlsFixtureStack as never, {
+      security: rlsFixtureSecurity(ownerScopedMemberSet),
+    });
+
+    // First user is the seeded dev admin; every later sign-up is a plain member
+    // that falls back to the owner-scoped fixture permission set.
+    adminToken = await stack.signIn();
+    memberToken = await stack.signUp('analytics-rls@verify.test');
+
+    // Author through HTTP as each principal so `created_by` is stamped with the
+    // real caller — the whole point of an owner policy.
+    for (let i = 0; i < ADMIN_NOTES; i++) {
+      const r = await stack.apiAs(adminToken, 'POST', '/data/rls_note', {
+        name: `admin-note-${i}`,
+        body: 'owned by admin',
+      });
+      expect(r.status).toBeLessThan(300);
+    }
+    for (let i = 0; i < MEMBER_NOTES; i++) {
+      const r = await stack.apiAs(memberToken, 'POST', '/data/rls_note', {
+        name: `member-note-${i}`,
+        body: 'owned by member',
+      });
+      expect(r.status).toBeLessThan(300);
+    }
+
+    // Premise check — if the owner policy is not actually biting on the plain
+    // read path, an analytics assertion below would pass for the wrong reason.
+    const list = await stack.apiAs(memberToken, 'GET', '/data/rls_note');
+    const listed = (await list.json()) as { records?: Array<{ name: string }> };
+    expect(listed.records ?? []).toHaveLength(MEMBER_NOTES);
+    expect((listed.records ?? []).every((r) => r.name.startsWith('member-note'))).toBe(true);
+  }, 90_000);
+
+  afterAll(async () => {
+    await stack?.stop();
+  });
+
+  async function totalFor(token: string, dataset: unknown, dimensions: string[]): Promise<number> {
+    const res = await stack.apiAs(token, 'POST', '/analytics/dataset/query', {
+      dataset,
+      selection: { dimensions, measures: ['cnt'] },
+    });
+    expect(res.status).toBe(200);
+    const body = (await res.json()) as { rows?: Array<Record<string, number>> };
+    return (body.rows ?? []).reduce((sum, row) => sum + Number(row.cnt ?? 0), 0);
+  }
+
+  it('ObjectQL path (date-bucketed): a member counts ONLY their own rows', async () => {
+    // Before #3601 this returned ADMIN_NOTES + MEMBER_NOTES — the member saw a
+    // count over rows RLS forbids them from reading.
+    const seen = await totalFor(memberToken, notesByDay, ['created']);
+    expect(seen).toBe(MEMBER_NOTES);
+  });
+
+  it('NativeSQL path (no granularity): a member counts ONLY their own rows', async () => {
+    const seen = await totalFor(memberToken, notesTotal, []);
+    expect(seen).toBe(MEMBER_NOTES);
+  });
+
+  it('the two strategies agree for the same principal', async () => {
+    const viaObjectql = await totalFor(memberToken, notesByDay, ['created']);
+    const viaNativeSql = await totalFor(memberToken, notesTotal, []);
+    expect(viaObjectql).toBe(viaNativeSql);
+  });
+
+  it('the rows the member cannot see DO exist — scoping, not an empty table', async () => {
+    // Ground truth straight from the engine under a system context: the table
+    // really holds every note. Without this, "member counts 2" would also pass
+    // on a broken setup that simply never persisted the admin's rows.
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const ql = await stack.kernel.getServiceAsync<any>('objectql');
+    const all = (await ql.find('rls_note', { context: { isSystem: true } })) as unknown[];
+    expect(all).toHaveLength(ADMIN_NOTES + MEMBER_NOTES);
+
+    const memberSees = await totalFor(memberToken, notesByDay, ['created']);
+    expect(memberSees).toBe(MEMBER_NOTES);
+    expect(memberSees).toBeLessThan(all.length);
+  });
+});

packages/services/service-analytics/src/plugin.ts

@@ -290,6 +290,7 @@ export class AnalyticsServicePlugin implements Plugin {
     }
     let getReadScope = this.options.getReadScope;
     let autoBridgedReadScope = false;
+    let securityPresentAtInit = false;
     if (!getReadScope) {
       const trySecurity = (): SecurityReadFilter | undefined => {
         try {
@@ -299,10 +300,17 @@ export class AnalyticsServicePlugin implements Plugin {
           return undefined;
         }
       };
-      if (trySecurity()) {
-        getReadScope = (object, context) => trySecurity()?.getReadFilter(object, context);
-        autoBridgedReadScope = true;
-      }
+      // ALWAYS wire the bridge — resolution happens at call time, mirroring the
+      // executeAggregate / executeRawSql auto-bridges above. Gating the
+      // ASSIGNMENT on an init-time probe (as this did) made analytics RLS
+      // silently plugin-ORDER-DEPENDENT: a kernel that registers this plugin
+      // before the security plugin got NO read-scope provider at all, so every
+      // strategy ran unscoped and only a WARN marked it. The repo's own
+      // `bootStack` harness registers in exactly that order, which is why no
+      // dogfood test could ever observe analytics RLS.
+      securityPresentAtInit = !!trySecurity();
+      getReadScope = (object, context) => trySecurity()?.getReadFilter(object, context);
+      autoBridgedReadScope = true;
     }
 
     // ADR-0021 — relationship → target-object resolver. A dataset's `include`
@@ -457,12 +465,20 @@ export class AnalyticsServicePlugin implements Plugin {
       draftRowsResolver,
     };
 
-    if (autoBridgedReadScope) {
+    if (autoBridgedReadScope && securityPresentAtInit) {
       ctx.logger.info('[Analytics] Auto-bridged getReadScope → "security" service (getReadFilter)');
+    } else if (autoBridgedReadScope) {
+      // The bridge IS wired and will resolve at call time — this is only a
+      // heads-up that security had not registered yet at our init. It becomes a
+      // real problem only if no security service ever appears.
+      ctx.logger.info(
+        '[Analytics] getReadScope bridged to the "security" service; that service is not ' +
+        'registered yet at init and will be resolved per query (plugin order is not significant).',
+      );
     } else if (!getReadScope) {
       ctx.logger.warn(
         '[Analytics] No getReadScope configured and no "security" service with getReadFilter found — ' +
-        'the raw-SQL analytics path will NOT enforce tenant/RLS scoping on joined objects (ADR-0021 D-C). ' +
+        'analytics queries will NOT enforce tenant/RLS scoping (ADR-0021 D-C). ' +
         'Supply getReadScope or register a security service in multi-tenant deployments.',
       );
     }