fix(service-analytics): row-scope bridge tells absent from broken security service

.changeset/analytics-row-scope-bridge-three-way.md

@@ -0,0 +1,30 @@
+---
+"@objectstack/service-analytics": minor
+---
+
+fix(service-analytics): the ROW-SCOPE bridge to the `security` service tells the same three resolutions apart as the object-level one — a broken security service refuses the query instead of running it with no row policy (#16918)
+
+`AnalyticsServicePlugin` bridges to the `security` service twice: once for the OBJECT-level read grant (`admitObjectRead` → `canReadObject`, #16645) and once for the ROW-level read scope (`getReadScope` → `getReadFilter`, ADR-0021 D-C). The object-level bridge tells three resolutions apart — ABSENT admits, THROWING and METHOD-LESS deny at `error`. The row-scope bridge collapsed all three into one:
+
+```ts
+const trySecurity = () => {
+  try {
+    const svc = ctx.getService<SecurityReadFilter>('security');
+    return svc && typeof svc.getReadFilter === 'function' ? svc : undefined;
+  } catch { return undefined; }
+};
+getReadScope = (object, context) => trySecurity()?.getReadFilter(object, context);
+```
+
+A throwing resolver and a registered service without `getReadFilter` both produced `undefined` — the same value an absent security service produces, and the value `ISecurityService.getReadFilter` reserves for one meaning only: *"this caller has no row restriction on this object"*. So on a deployment whose security service was wired but broken (a boot-order fault, a mis-registered plugin, a failing dependency, a provider that is not the contract it claims to be) analytics queries ran with **no row-level policy at all**, and nothing said so. One door of the file failed closed on a throwing resolver and its neighbour failed open — and the neighbour is the one carrying row-level policy.
+
+**What changes.** The bridge now resolves the same explicit three-way, at the same reporting level:
+
+- **ABSENT** — no `security` service resolved: **unchanged**. No row-scope provider on this deployment, which is a legitimate configuration (a single-tenant kernel that ships no `plugin-security`, where `/data` carries no row-level policy either) and is already reported loudly at init. ⛔ Deliberately not tightened: refusing here would break every such deployment.
+- **THROWING** resolver, or a registered service with **no `getReadFilter`** — the query is **REFUSED**, and the reason is reported at `error` naming the object and which of the two states it was. The refusal is a throw, which `AnalyticsService.resolveReadScopes` — fail-closed since ADR-0021 D-C — already turns into "deny the whole query rather than emit SQL with that object unscoped". A log over an `undefined` would not have been a refusal.
+
+**This change only NARROWS what analytics serves, and only in a state where the security service is broken.** No deployment with a working `security` service, and no deployment with none, changes behaviour by so much as a byte. Nothing that was refused becomes admitted.
+
+**No published-surface delta.** No new error code (the refusal rides the seam's existing fail-closed error), no exported symbol, no key on `AnalyticsServicePluginOptions` or any payload, and no documented envelope changes shape. Graded `minor` rather than `patch` because it is a behaviour narrowing on a published package's read path, matching how its object-level sibling was graded in the same lockstep window.
+
+⚠️ Deliberately **not** answered here: which tenant wall the platform's is (plugin-security's posture-gated Layer 0, or driver-sql's posture-independent auto-scope) — the escalated maintainer decision of triage condition 5. Refusing to serve is neutral between them: it answers *"should we serve at all"*, never *"what shape is the wall"*.

packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts

@@ -113,6 +113,21 @@ async function bootAnalytics(security?: () => unknown) {
   return { service: registered.analytics as AnalyticsService, reads, error };
 }
 
+/**
+ * A working security service's ROW-SCOPE half, carried by every double below
+ * that is meant to represent one.
+ *
+ * `getReadFilter` is a REQUIRED member of `ISecurityService`, and since #16918
+ * the ROW-SCOPE bridge in the same `plugin.ts` refuses the query when the
+ * registered service does not expose it — the sibling three-way of the one
+ * this file measures. `undefined` is that method's documented answer for "no
+ * row restriction on this object", so a double carrying it stays minimal AND
+ * conforming, and every object-level verdict asserted below is reached exactly
+ * as it was before. The deny-path doubles need none: the object-level gate runs
+ * first and refuses before the row half is ever asked.
+ */
+const rowScopeOpen = { getReadFilter: async () => undefined };
+
 const runProbe = (service: AnalyticsService) =>
   service.queryDataset(probe as never, { measures: ['cnt'] } as never, CALLER);
 
@@ -170,7 +185,7 @@ describe('analytics admission bridge — resolving the "security" service', () =
 
   it('asks canReadObject when the service has it, and serves an ADMITTED caller', async () => {
     const canReadObject = vi.fn(() => true);
-    const { service, reads } = await bootAnalytics(() => ({ canReadObject }));
+    const { service, reads } = await bootAnalytics(() => ({ ...rowScopeOpen, canReadObject }));
 
     const result = await runProbe(service);
     expect(result.rows).toEqual([{ cnt: 24 }]);
@@ -190,6 +205,7 @@ describe('analytics admission bridge — resolving the "security" service', () =
 
   it('falls back to explain for a service that predates canReadObject — both verdicts', async () => {
     const admitted = await bootAnalytics(() => ({
+      ...rowScopeOpen,
       explain: async () => ({ allowed: true }),
     }));
     expect((await runProbe(admitted.service)).rows).toEqual([{ cnt: 24 }]);

packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts

@@ -0,0 +1,229 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The analytics → `security` ROW-SCOPE bridge, and the three resolutions it
+ * must tell apart — the sibling of `admission-bridge-resolution.test.ts`, one
+ * function up in the same file.
+ *
+ * The object-level bridge was made an explicit three-way (#16860); this one had
+ * the identical shape and still collapsed it:
+ *
+ * ```ts
+ * const trySecurity = () => {
+ *   try {
+ *     const svc = ctx.getService<SecurityReadFilter>('security');
+ *     return svc && typeof svc.getReadFilter === 'function' ? svc : undefined;
+ *   } catch { return undefined; }
+ * };
+ * getReadScope = (object, context) => trySecurity()?.getReadFilter(object, context);
+ * ```
+ *
+ * A THROWING resolver and a METHOD-LESS service both produced `undefined` —
+ * the same value an ABSENT security service produces, and the same value
+ * `ISecurityService.getReadFilter` reserves for one meaning only: "this caller
+ * has no row restriction on this object". So a deployment whose security
+ * service was broken ran its analytics queries with NO row-level policy at
+ * all, and the only difference from a correctly unrestricted caller was a state
+ * nothing reported. After #16860 one door of `plugin.ts` failed closed on a
+ * throwing resolver and its neighbour failed open — and the neighbour is the
+ * one carrying row-level policy.
+ *
+ * The two broken corners now REFUSE the query: the bridge throws, and
+ * `AnalyticsService.resolveReadScopes` — fail-closed since ADR-0021 D-C —
+ * denies the whole query rather than emitting SQL with the object unscoped.
+ * Refusing is the outcome the object-level bridge already produces, and it is
+ * neutral between the two candidate tenant walls: it answers "should we serve
+ * at all", never "what shape is the wall".
+ *
+ * ⛔ The ABSENT case is the negative control and must stay UNCHANGED. Refusing
+ * when no security plugin is installed would break every single-tenant
+ * deployment — that is a real configuration, reported loudly at init, and it is
+ * the state in which `/data` has no row-level policy either.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { DatasetSchema } from '@objectstack/spec/ui';
+import type { ExecutionContext } from '@objectstack/spec/kernel';
+import { AnalyticsServicePlugin } from '../plugin.js';
+import type { AnalyticsService } from '../analytics-service.js';
+
+/** The probe: one object, one count measure, no dimensions. */
+const probe = DatasetSchema.parse({
+  name: 'probe_member',
+  label: 'probe',
+  object: 'employer_member',
+  dimensions: [],
+  measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }],
+});
+
+const CALLER = { userId: 'u_seeker', tenantId: 'org_a' } as ExecutionContext;
+
+/** The SQL posture — the reported path, where nothing else stands in the way. */
+const nativeSql = () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false });
+
+/**
+ * Engine double. Every read is recorded WITH the statement, so "refused" is
+ * asserted as "the database was never reached" and "scoped" as "the predicate
+ * was in the statement that ran" — a bridge that refuses after running the
+ * query has refused nothing, and one that serves rows without the predicate is
+ * the defect itself.
+ */
+function fakeEngine() {
+  const reads: string[] = [];
+  return {
+    reads,
+    engine: {
+      execute: async (sql: unknown, options?: { object?: string }) => {
+        reads.push(`execute:${options?.object ?? ''}:${String(sql)}`);
+        return { rows: [{ cnt: 24 }] };
+      },
+      aggregate: async (object: string) => {
+        reads.push(`aggregate:${object}`);
+        return [{ cnt: 24 }];
+      },
+      getObject: (name: string) =>
+        name === 'employer_member'
+          ? { fields: { id: { type: 'text' }, organization_id: { type: 'text' } } }
+          : undefined,
+      resolveEffectiveDatasource: () => undefined,
+    },
+  };
+}
+
+/**
+ * Minimal `PluginContext`. `security` is supplied as a THUNK so a fixture can
+ * make the lookup itself throw — the corner that is otherwise unreachable from
+ * a plain service map.
+ */
+function fakePluginContext(opts: { data: unknown; security?: () => unknown }) {
+  const registered: Record<string, unknown> = {};
+  const warn = vi.fn();
+  const error = vi.fn();
+  return {
+    registered,
+    warn,
+    error,
+    ctx: {
+      getService: (name: string) => {
+        if (name === 'security') return opts.security ? opts.security() : undefined;
+        if (name === 'data') return opts.data;
+        return registered[name];
+      },
+      registerService: (name: string, svc: unknown) => { registered[name] = svc; },
+      replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
+      logger: { info() {}, warn, error, debug() {} },
+    },
+  };
+}
+
+/**
+ * @param admitObjectRead supplied by SOME fixtures on purpose. The object-level
+ *   bridge in the same file resolves the SAME service, so a throwing resolver
+ *   is refused by that gate first and the row-scope bridge under test is never
+ *   reached. Supplying the documented `admitObjectRead` option (a host that
+ *   answers object-level admission itself) leaves the row-scope bridge as the
+ *   only auto-bridge in play, which is what makes this a measurement OF IT.
+ */
+async function bootAnalytics(
+  security?: () => unknown,
+  admitObjectRead?: () => boolean,
+) {
+  const { engine, reads } = fakeEngine();
+  const { ctx, registered, error } = fakePluginContext({ data: engine, security });
+  await new AnalyticsServicePlugin({
+    queryCapabilities: nativeSql,
+    ...(admitObjectRead ? { admitObjectRead } : {}),
+  }).init(ctx as never);
+  return { service: registered.analytics as AnalyticsService, reads, error };
+}
+
+const runProbe = (service: AnalyticsService) =>
+  service.queryDataset(probe as never, { measures: ['cnt'] } as never, CALLER);
+
+const errorText = (error: { mock: { calls: unknown[][] } }) =>
+  error.mock.calls.map((c) => String(c[0])).join('\n');
+
+describe('analytics row-scope bridge — resolving the "security" service', () => {
+  // ── The two corners that used to run the query with no row policy ──────────
+
+  it('REFUSES when resolving the "security" service THROWS', async () => {
+    const boom = () => { throw new Error('security service is initialising'); };
+    const { service, reads, error } = await bootAnalytics(boom, () => true);
+
+    await expect(runProbe(service)).rejects.toThrow(/read-scope resolution failed/i);
+    // The refusal has to happen BEFORE the statement runs, or it is not a
+    // refusal — this is the assertion that fails on `origin/main`, where the
+    // same fixture serves `{cnt: 24}` off an unscoped statement.
+    expect(reads).toEqual([]);
+    // And it has to name why, at error level. A gate that refuses invisibly is
+    // indistinguishable from one that never ran.
+    expect(errorText(error)).toMatch(
+      /row-level read scope could not be resolved .* refusing the query \(fail-closed\).*threw/s,
+    );
+  });
+
+  it('REFUSES when the registered "security" service exposes no getReadFilter', async () => {
+    // ⚠️ No `admitObjectRead` override here, and none is needed: this service
+    // answers the OBJECT-level question (`canReadObject`) and is admitted by
+    // that bridge, so the row-scope bridge is the only one that can refuse.
+    // Both auto-bridges are live — this is the shape reachable end-to-end.
+    const { service, reads, error } = await bootAnalytics(() => ({
+      canReadObject: () => true,
+    }));
+
+    await expect(runProbe(service)).rejects.toThrow(/read-scope resolution failed/i);
+    expect(reads).toEqual([]);
+    expect(errorText(error)).toMatch(
+      /row-level read scope could not be resolved .* exposes no getReadFilter\(\)/s,
+    );
+  });
+
+  // ── The negative control: absence is a different state and is UNCHANGED ────
+
+  it('ADMITS, unscoped, when NO "security" service is registered at all', async () => {
+    // ⛔ Not a corner to tighten. This is a single-tenant deployment that ships
+    // no `plugin-security`: there is no row-level policy anywhere on it,
+    // `/data` included, and the init log says so. Refusing here would break
+    // every such deployment — which is why this arm is what makes the two
+    // above a measurement rather than an over-fix.
+    const { service, reads, error } = await bootAnalytics(undefined);
+
+    const result = await runProbe(service);
+    expect(result.rows).toEqual([{ cnt: 24 }]);
+    expect(reads).toHaveLength(1);
+    expect(errorText(error)).not.toMatch(/row-level read scope/);
+  });
+
+  // ── The working spelling, so the refusals cannot pass by refusing all ──────
+
+  it('asks getReadFilter when the service has it, and SCOPES the statement', async () => {
+    const getReadFilter = vi.fn(async () => ({ organization_id: 'org_a' }));
+    const { service, reads } = await bootAnalytics(() => ({
+      canReadObject: () => true,
+      getReadFilter,
+    }));
+
+    const result = await runProbe(service);
+    expect(result.rows).toEqual([{ cnt: 24 }]);
+    expect(getReadFilter).toHaveBeenCalledWith('employer_member', CALLER);
+    expect(reads).toHaveLength(1);
+    expect(reads[0]).toMatch(/organization_id/);
+  });
+
+  // ── The two doors of this file now agree on a broken provider ──────────────
+
+  it('refuses a throwing resolver with BOTH auto-bridges live (no door falls open)', async () => {
+    // With no `admitObjectRead` override the object-level bridge (#16860)
+    // answers first, with `PERMISSION_DENIED`. Pinned so the file-level
+    // property — a broken security service serves no analytics rows through
+    // EITHER door — cannot regress from the other side.
+    const boom = () => { throw new Error('security service is initialising'); };
+    const { service, reads } = await bootAnalytics(boom);
+
+    await expect(runProbe(service)).rejects.toMatchObject({
+      code: 'PERMISSION_DENIED',
+      status: 403,
+    });
+    expect(reads).toEqual([]);
+  });
+});