diff --git a/.changeset/analytics-row-scope-bridge-three-way.md b/.changeset/analytics-row-scope-bridge-three-way.md new file mode 100644 index 0000000000..7fcf9b4e83 --- /dev/null +++ b/.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('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"*. diff --git a/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts b/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts index 9565faedef..2c69b73779 100644 --- a/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts +++ b/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 }]); diff --git a/packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-bridge-resolution.test.ts new file mode 100644 index 0000000000..2567bf3c50 --- /dev/null +++ b/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('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 = {}; + 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([]); + }); +}); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index b5d20d49e0..48a0ef4f44 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -482,17 +482,73 @@ export class AnalyticsServicePlugin implements Plugin { | undefined | Promise; } + /** + * The three resolutions of the `security` service, for the ROW-LEVEL half + * of the read — the same three the OBJECT-LEVEL bridge below tells apart, + * and for the same reason. + * + * ABSENT — `getService('security')` returns nothing. No row-scope + * provider on this deployment, which is a legitimate + * configuration (a single-tenant kernel that ships no + * `plugin-security`), reported loudly at init below. Today's + * behaviour is kept EXACTLY: no scope, query runs. + * UNUSABLE — a security service exists but cannot answer: resolving it + * THREW, or the object it returned carries no + * `getReadFilter`. A wired-but-broken provider, and the + * answer here is NOT "no row restriction" — that value means + * one thing only (`ISecurityService.getReadFilter`: "this + * caller has no row restriction on this object"), and + * spending it on a provider that never answered is how a + * query ends up running with no row-level policy at all. + * It REFUSES, and says why. + * USABLE — ask it. + * + * ⛔ The refusal is a THROW, not a louder log over an `undefined`: a log is + * not a refusal. `AnalyticsService.resolveReadScopes` is the fail-closed + * seam that already denies the whole query when this provider throws (it + * has since ADR-0021 D-C), so serving nothing — the same outcome the + * object-level bridge produces — needs no new error code and no new + * envelope here. + */ + type SecurityReadFilterResolution = + | { kind: 'usable'; svc: SecurityReadFilter } + | { kind: 'absent' } + | { kind: 'unusable'; why: string }; let getReadScope = this.options.getReadScope; let autoBridgedReadScope = false; let securityPresentAtInit = false; if (!getReadScope) { - const trySecurity = (): SecurityReadFilter | undefined => { + const trySecurity = (): SecurityReadFilterResolution => { + let svc: SecurityReadFilter | undefined; try { - const svc = ctx.getService('security'); - return svc && typeof svc.getReadFilter === 'function' ? svc : undefined; - } catch { - return undefined; + svc = ctx.getService('security'); + } catch (e) { + // ⛔ Not `absent`. A throwing resolver is a service that exists and + // failed, and a failed security lookup is a refusal everywhere else + // in this stack — including the object-level bridge below, which + // used to be spelled exactly like this one and now denies. + return { + kind: 'unusable', + why: + `resolving the "security" service threw ` + + `(${String((e as Error)?.message ?? e)})`, + }; + } + if (!svc) return { kind: 'absent' }; + if (typeof svc.getReadFilter !== 'function') { + // `getReadFilter` is a REQUIRED member of `ISecurityService`, so a + // conforming provider never lands here — reaching it means the + // registered object is not the contract it claims to be, and a + // provider that cannot answer "which rows" must not be read as + // "every row". + return { + kind: 'unusable', + why: + 'the registered "security" service exposes no getReadFilter(), ' + + 'so it cannot answer a row-level read scope', + }; } + return { kind: 'usable', svc }; }; // ALWAYS wire the bridge — resolution happens at call time, mirroring the // executeAggregate / executeRawSql auto-bridges above. Gating the @@ -502,8 +558,26 @@ export class AnalyticsServicePlugin implements Plugin { // 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); + securityPresentAtInit = trySecurity().kind === 'usable'; + getReadScope = (object, context) => { + const resolved = trySecurity(); + // No security service resolved at call time → no row-scope provider on + // this deployment, the state reported at init. Unchanged. + if (resolved.kind === 'absent') return undefined; + if (resolved.kind === 'unusable') { + ctx.logger.error( + `[Analytics] row-level read scope could not be resolved for "${object}" — ` + + `refusing the query (fail-closed): ${resolved.why}. ` + + 'A security service is wired on this deployment, so analytics must not fall ' + + 'open and serve rows with no row-level policy applied.', + ); + throw new Error( + `[Analytics] row-level read scope could not be resolved for "${object}"; ` + + 'query refused (fail-closed).', + ); + } + return resolved.svc.getReadFilter(object, context); + }; autoBridgedReadScope = true; }