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 f4d53608f37..c2836672f04 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 @@ -8,32 +8,45 @@ * bridge brings back, and the bridge has three outcomes that are easy to * collapse into one: * - * - the `security` service is ABSENT — this deployment has no object-level - * gate anywhere, `GET /data/` included, because that gate IS the - * absent middleware. The two doors agree, which is the equivalence property - * the card asks for, so the query is ADMITTED and the state is reported - * once, by the first query that finds it (`admission-absence-report.test.ts` - * pins when, and at what level); - * - resolving the service THREW — a security service exists on this - * deployment and could not be reached; - * - the service resolved but exposes NEITHER `canReadObject` NOR `explain` — - * it exists and cannot answer. + * - the lookup answers NOTHING (ABSENT) — the context returns no service for + * `security`. Only a context that answers a miss with nothing reaches this + * branch: this file's double does, and no in-repo kernel does. The bridge + * ADMITS there and reports it once, by the first query that finds it + * (`admission-absence-report.test.ts` pins when, and at what level); + * - resolving the service THREW (UNUSABLE), whatever the cause: a security + * service that is wired and could not be reached, or — on the in-repo + * kernels, `ObjectKernel` and `LiteKernel` — a `security` service nothing + * ever registered, because their synchronous `getService` throws on a miss; + * - the service resolved but exposes NEITHER `canReadObject` NOR `explain` + * (UNUSABLE) — it exists and cannot answer. * - * The last two are wired-but-broken providers. `/data`'s middleware does not - * fall open in either state, so admitting here would reopen exactly the - * divergence between the two doors that this gate closes — and would do it - * silently, which is worse than the original defect: the original at least had - * a shape a reader could find in the code. Both DENY, and both say why at - * `error`. + * Both UNUSABLE corners DENY, fail-closed, before anything is read, and both say + * why at `error`. For a wired-but-broken provider, `/data`'s middleware does not + * fall open either, so admitting here would reopen exactly the divergence + * between the two doors that this gate closes — and would do it silently. * - * ⛔ The absent case is not a bug to be tightened away. It is the negative - * control that keeps the two deny cases honest: a bridge that denied on absence - * too would refuse every analytics query on every deployment that ships no - * `plugin-security`, which is a strictly different (and wrong) answer from the - * one `/data` gives on that same deployment. + * ## The declaration: no security service registered ⇒ analytics DENIES + * + * [#22235] A deployment that registers no security service gets its analytics + * read queries REFUSED, fail-closed: the kernel's lookup throws, the bridge + * takes UNUSABLE, the caller gets `PERMISSION_DENIED` / 403 naming the object, + * and the operator gets an `error` line naming the object and the failed + * lookup. That is the declared answer, not an accident of how the lookup fails, + * and it holds even though `/data` carries no object-level gate on such a + * deployment: a loud deny is preferred over a silent admit. A composition that + * wants analytics to answer registers a security service, or its host supplies + * its own `admitObjectRead`. The last block of this file pins it on both + * in-repo kernels. + * + * ⛔ So the ABSENT case below is not a statement about deployments. It pins + * what the bridge does for a context that answers a miss with nothing, and it + * stays the negative control for the two UNUSABLE cases: same double, same + * probe, only the lookup's answer differs, so a bridge that refused everything + * could not pass this file. */ import { describe, it, expect, vi } from 'vitest'; +import { LiteKernel, ObjectKernel, type Plugin, type PluginContext } from '@objectstack/core'; import { DatasetSchema } from '@objectstack/spec/ui'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import { AnalyticsServicePlugin } from '../plugin.js'; @@ -171,13 +184,13 @@ describe('analytics admission bridge — resolving the "security" service', () = ); }); - // ── The negative control: absence is a different state and still ADMITS ──── + // ── The negative control: a lookup that answers NOTHING is a different state ─ - it('ADMITS when NO "security" service is registered at all', async () => { - // ⛔ Not a corner to tighten. On this deployment `/data` has no - // object-level gate either, so the two doors still agree — which is the - // property being defended. Tightening this to a denial would refuse every - // analytics query on every deployment shipping no `plugin-security`. + it('ADMITS when the context answers the "security" lookup with nothing (ABSENT)', async () => { + // The ABSENT branch: this double answers the lookup with nothing, which no + // in-repo kernel does — they throw on a miss (the last block of this file). + // Kept as the negative control for the two deny cases above; whether this + // branch should deny too is a separate question, not settled here. const { service, reads } = await bootAnalytics(undefined); const result = await runProbe(service); @@ -224,3 +237,62 @@ describe('analytics admission bridge — resolving the "security" service', () = expect(refused.reads).toEqual([]); }); }); + +// ── The declaration, on the in-repo kernels: no security service ⇒ DENY ────── + +/** + * Boots a REAL kernel with a `data` provider and this plugin, and NO security + * service, ever: nothing registers one at init or at start. The first plugin + * taps the kernel's shared logger so the bridge's own lines are seen. + */ +async function bootKernelWithoutSecurity(kernel: ObjectKernel | LiteKernel) { + const errors: string[] = []; + const warns: string[] = []; + const { engine, reads } = fakeEngine(); + const tap: Plugin = { + name: 'test.log-tap', + init: async (ctx: PluginContext) => { + vi.spyOn(ctx.logger, 'error').mockImplementation((message: unknown) => { errors.push(String(message)); }); + vi.spyOn(ctx.logger, 'warn').mockImplementation((message: unknown) => { warns.push(String(message)); }); + }, + }; + const data: Plugin = { + name: 'test.data', + init: async (ctx: PluginContext) => { ctx.registerService('data', engine); }, + }; + await kernel.use(tap); + await kernel.use(data); + await kernel.use(new AnalyticsServicePlugin({ queryCapabilities: nativeSql })); + await kernel.bootstrap(); + return { kernel, service: kernel.getService('analytics'), reads, errors, warns }; +} + +describe.each([ + ['ObjectKernel', () => new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false })], + ['LiteKernel', () => new LiteKernel({ logger: { level: 'silent' } })], +] as const)('analytics admission bridge — a real %s that never registers a "security" service', (_name, makeKernel) => { + it('DENIES the dataset query fail-closed, before any read, with an error located on the object and the lookup', async () => { + const { kernel, service, reads, errors, warns } = await bootKernelWithoutSecurity(makeKernel()); + + // The mechanism the declaration rests on: this kernel answers a lookup for + // a never-registered name by THROWING, never with nothing. + expect(() => kernel.getService('security')).toThrow(/'security'/); + + // The caller's refusal: the declared envelope, naming the object. + await expect(runProbe(service)).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + object: 'employer_member', + }); + // Refused BEFORE the statement ran. + expect(reads).toEqual([]); + // The operator's line: exactly one, from the object-level bridge, naming + // the object and the failed `security` lookup as the cause. + const bridgeErrors = errors.filter((l) => l.includes('object-level read admission')); + expect(bridgeErrors).toHaveLength(1); + expect(bridgeErrors[0]).toContain('"employer_member"'); + expect(bridgeErrors[0]).toContain('resolving the "security" service threw'); + // Not the ABSENT branch: that one admits, and WARNs instead. + expect(warns.filter((l) => l.includes('no "security" service registered'))).toEqual([]); + }); +}); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 9525cc9c7b0..ccecf120ec1 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -553,22 +553,28 @@ export class AnalyticsServicePlugin implements Plugin { * 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. + * ABSENT — the context answers `getService('security')` with + * nothing. Only a context that answers a miss that way + * reaches this branch: the package's test doubles do, and no + * in-repo kernel does (see UNUSABLE). Kept as it was: no + * scope, the query runs. + * UNUSABLE — resolving it THREW, or the object it returned carries no + * `getReadFilter`. A throw is UNUSABLE whatever caused it: a + * wired-but-broken provider, or — on the in-repo kernels, + * `ObjectKernel` and `LiteKernel` — a `security` service + * nothing ever registered, because their synchronous + * `getService` throws on a miss. Either way 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. * + * [#22235] So a deployment that registers no security service is REFUSED + * here, fail-closed, by declaration — not by an accident of how the lookup + * fails. Its object-level sibling below refuses the same query first. + * * ⛔ 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 @@ -599,10 +605,13 @@ export class AnalyticsServicePlugin implements Plugin { try { 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. + // ⛔ Not `absent`. A throw is UNUSABLE whatever caused it: a + // registered service that failed, or — on the in-repo kernels — a + // name nothing ever registered, which their synchronous + // `getService` answers by throwing. 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: @@ -637,8 +646,8 @@ export class AnalyticsServicePlugin implements Plugin { 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. + // The context answered the lookup with nothing (ABSENT, above) → no + // row-scope provider. Unchanged. No in-repo kernel answers that way. if (resolved.kind === 'absent') return undefined; if (resolved.kind === 'unusable') { ctx.logger.error( @@ -684,31 +693,42 @@ export class AnalyticsServicePlugin implements Plugin { // // ## The three resolutions, and why only ONE of them admits // - // "No security service" and "the security service could not be used" are - // different states and they get opposite answers. Collapsing them is the - // shape of the defect this whole change removes, one level up. + // "The lookup answered nothing" and "the security service could not be + // used" are different states and they get opposite answers. Collapsing + // them is the shape of the defect this whole change removes, one level up. // - // ABSENT — `getService('security')` returns nothing. There is no - // object-level gate on this deployment at all, including on - // `/data`, because that gate IS this plugin's absent - // middleware. The two doors still agree, which is the - // equivalence property the card asks for, so this ADMITS and - // is reported loudly, once, by the first query that finds it - // (below) — not at init, where the service is usually just - // not registered YET. - // UNUSABLE — a security service exists but cannot answer: resolving it - // THREW, or the object it returned carries neither - // `canReadObject` nor `explain`. This is a wired-but-broken - // provider, and `/data`'s middleware does NOT fall open in - // that state — so admitting here would reopen the exact - // divergence between the two doors that this PR closes, and - // it would do it silently. It DENIES, and says why. + // ABSENT — the context answers `getService('security')` with nothing. + // Only a context that answers a miss that way reaches this + // branch: the package's test doubles do, and no in-repo + // kernel does (see UNUSABLE). It ADMITS, and the first query + // that finds it reports it once (below) — not at init, where + // the service is usually just not registered YET. + // UNUSABLE — resolving it THREW, or the object it returned carries + // neither `canReadObject` nor `explain`. A throw is UNUSABLE + // whatever caused it: a wired-but-broken provider, or — on + // the in-repo kernels, `ObjectKernel` and `LiteKernel` — a + // `security` service nothing ever registered, because their + // synchronous `getService` throws on a miss. For a + // wired-but-broken provider `/data`'s middleware does NOT fall + // open either, so admitting here would reopen the exact + // divergence between the two doors that this gate closes, and + // silently. It DENIES, fail-closed, and says why at `error`, + // naming the object. // USABLE — ask it (below). // + // [#22235] The declaration (the maintainer's ruling): a deployment that + // registers no security service is REFUSED at this door, fail-closed — the + // in-repo kernels throw on the miss, so it lands in UNUSABLE. `/data` + // carries no object-level gate on such a deployment, so the two doors do + // NOT agree there: a loud deny is preferred over a silent admit. A + // composition that wants analytics to answer registers a security service, + // or its host supplies its own `admitObjectRead`. + // `admission-bridge-resolution.test.ts` pins it on both in-repo kernels. + // // The distinction is worth the type: both unusable corners used to be // spelled `return undefined` beside the absent one, and three lines later - // all three read `if (!svc) return true`. A deployment whose security - // service throws on resolution is not a deployment without security. + // all three read `if (!svc) return true`. A lookup that throws is never + // read as a lookup that answered nothing. interface SecurityReadAdmission { canReadObject?(object: string, context?: ExecutionContext): boolean | Promise; explain?( @@ -728,9 +748,11 @@ export class AnalyticsServicePlugin implements Plugin { try { 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. + // ⛔ Not `absent`. A throw is UNUSABLE whatever caused it: a + // registered service that failed, or — on the in-repo kernels — a + // name nothing ever registered, which their synchronous + // `getService` answers by throwing. A failed security lookup is a + // refusal everywhere else in this stack. return { kind: 'unusable', why: @@ -759,20 +781,25 @@ export class AnalyticsServicePlugin implements Plugin { // ships plugin-security: that plugin registers it in its own start(), // after every init(). This bridge resolves per call, so plugin order is // not significant. A WARN at init asserted a verdict the same boot - // contradicts; init reports the absence at `info` (below), and the WARN - // waits for a query that needs the gate and finds no service. + // contradicts; init reports the absence at `info` (below), and the + // report waits for a query that needs the gate and finds no service. On + // the in-repo kernels that query's lookup THROWS, so it is the UNUSABLE + // branch's `error` and a refusal, on every query; only a context that + // answers the miss with nothing reaches the ABSENT branch's WARN. // - // Once per bridge, i.e. per plugin init: the state is the deployment's, - // every later query is admitted the same way, and repeating it per query - // buries it. Deliberately NOT a module binding — a second kernel in the - // same process (a test run, a `verify` harness) must get its own report. + // That WARN is once per bridge, i.e. per plugin init: the state is the + // context's, every later query is admitted the same way, and repeating it + // per query buries it. Deliberately NOT a module binding — a second + // kernel in the same process (a test run, a `verify` harness) must get + // its own report. // The flag records only that the report was made; it caches no verdict: // every query still resolves the service afresh. let admissionAbsenceReported = false; admitObjectRead = async (object, context) => { const resolved = trySecurityAdmission(); - // No security service resolved at call time → no object-level gate on - // this deployment: ADMIT, exactly as before, and say so once. + // The context answered the lookup with nothing (ABSENT, above): ADMIT, + // exactly as before, and say so once. No in-repo kernel reaches this + // line — they throw on a miss, which is UNUSABLE below. if (resolved.kind === 'absent') { if (!admissionAbsenceReported) { admissionAbsenceReported = true; @@ -816,15 +843,17 @@ export class AnalyticsServicePlugin implements Plugin { // the reader is the security service's, and a host that composes its own // reader constructs `AnalyticsService` with it. // - // ABSENT — no security service: no field-level security anywhere on - // this deployment, `/data` included. The provider answers - // `undefined` ("no answer"), which judges no field. - // UNUSABLE — the service exists but cannot answer: resolving it threw, - // or it carries no `getReadableFields` (a REQUIRED member of - // `ISecurityService`, so a conforming provider never lands - // here). The provider THROWS, and the service refuses the - // query fail-closed: a reader that never answered must not be - // read as "every field readable". + // ABSENT — the context answers the lookup with nothing; only a context + // that answers a miss that way reaches this (no in-repo + // kernel does). The provider answers `undefined` ("no + // answer"), which judges no field. + // UNUSABLE — resolving it threw — on the in-repo kernels that includes + // a `security` service nothing ever registered, since they + // throw on a miss — or it carries no `getReadableFields` (a + // REQUIRED member of `ISecurityService`, so a conforming + // provider never lands here). The provider THROWS, and the + // service refuses the query fail-closed: a reader that never + // answered must not be read as "every field readable". // USABLE — ask it. interface SecurityReadableFields { getReadableFields?(object: string, context?: ExecutionContext): Promise; @@ -854,8 +883,10 @@ export class AnalyticsServicePlugin implements Plugin { // queryable — the engine's two query guards refuse it — so the gate asks // both. Bridged the same way, with the same three resolutions: // - // ABSENT — no security service: `undefined`, no answer, as above. - // UNUSABLE — resolving it threw: THROW, the query is refused. + // ABSENT — the lookup answered nothing: `undefined`, no answer, as + // above. + // UNUSABLE — resolving it threw (a never-registered service included, + // on the in-repo kernels): THROW, the query is refused. // USABLE — ask its `getQueryableFields`. // // ⛔ And one more state the read half does not have: a USABLE service that @@ -1474,8 +1505,10 @@ export class AnalyticsServicePlugin implements Plugin { 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. + // heads-up that security had not registered yet at our init. If no + // security service ever registers, the in-repo kernels throw on the + // per-query lookup and the query is REFUSED, fail-closed (UNUSABLE) — + // the declared answer for a deployment with no security service. 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).', @@ -1498,8 +1531,10 @@ export class AnalyticsServicePlugin implements Plugin { // [#22154] A heads-up, not a verdict — the same situation the read-scope // sibling above logs at `info`. The bridge resolves per query, and // plugin-security registers the service in its start(), after this - // init; a query that needs the gate and finds no service WARNs then, - // once (see the bridge). + // init. A query that needs the gate and finds no service is REFUSED at + // `error` on the in-repo kernels (they throw on the miss: UNUSABLE); only + // a context that answers the miss with nothing admits it, with one WARN + // (ABSENT). See the bridge. ctx.logger.info( '[Analytics] admitObjectRead bridged to the "security" service; that service is not ' + 'registered yet at init and will be resolved per query (plugin order is not significant).', diff --git a/scripts/analytics-reconcile/boot.ts b/scripts/analytics-reconcile/boot.ts index 65e9c4b0f56..496911d995d 100644 --- a/scripts/analytics-reconcile/boot.ts +++ b/scripts/analytics-reconcile/boot.ts @@ -2,9 +2,10 @@ // // ADR-0021 Phase 2 — shared kernel-boot + reconciliation driver. // -// Boots an example stack with a real in-memory engine + the analytics service, -// then reconciles every dual-form dashboard widget (legacy `aggregate()` vs -// dataset `queryDataset()`). Read-only: never writes metadata or data. +// Boots an example stack with a real in-memory engine + the analytics service +// (+ an explicit, open `security` service: see `openSecurity`), then reconciles +// every dual-form dashboard widget (legacy `aggregate()` vs dataset +// `queryDataset()`). Read-only: never writes metadata or data. import { ObjectKernel, DriverPlugin, AppPlugin } from '@objectstack/runtime'; import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; @@ -12,11 +13,38 @@ import { ObjectQLPlugin } from '@objectstack/objectql'; import { AnalyticsServicePlugin } from '@objectstack/service-analytics'; import { DatasetSchema } from '@objectstack/spec/ui'; import type { Dataset, Dashboard, Report } from '@objectstack/spec/ui'; -import type { IAnalyticsService, DatasetSelection } from '@objectstack/spec/contracts'; +import type { IAnalyticsService, DatasetSelection, ISecurityService } from '@objectstack/spec/contracts'; import type { FilterCondition } from '@objectstack/spec/data'; import { reconcileDashboard, reconcileReports, type ReconcileExecutors, type WidgetReconcileResult } from './reconcile.js'; import { resolveDateMacros } from './macros.js'; +/** + * The `security` service this reconciliation runs under: registered + * EXPLICITLY, and OPEN on purpose. + * + * [#22235] The analytics read bridges DENY, fail-closed, on a kernel that + * registers no security service: the kernel's lookup throws on the miss, the + * bridge reads that as a security service it cannot use, and every dataset + * query is refused `PERMISSION_DENIED` — which this runner would report as a + * mismatch on every widget. That is the declared behaviour, so a composition + * that wants analytics to answer registers a security service. + * + * This runner compares NUMBERS, not access. Its legacy path + * (`engine.aggregate()`) runs with no caller context on an engine that carries + * no security middleware, so the dataset path is given that same posture, by + * name: every object readable, no row restriction, no field answer. A real + * security plugin would put an access policy on the comparison, which is not + * what a reconciliation measures. + */ +const openSecurity = { + /** Object-level: every object is readable here. */ + canReadObject: async (_object: string): Promise => true, + /** Row-level: `undefined` is "no row restriction on this object". */ + getReadFilter: async (_object: string): Promise => undefined, + /** Field-level: `undefined` is "no answer", which judges no field. */ + getReadableFields: async (_object: string): Promise => undefined, +} satisfies Partial; + interface DataEngineLike { aggregate(object: string, options: { where?: Record; @@ -46,6 +74,14 @@ export async function reconcileApp(opts: ReconcileAppOptions): Promise { await kernel.use(new ObjectQLPlugin()); await kernel.use(new DriverPlugin(new SqliteWasmDriver({ filename: ':memory:' }))); await kernel.use(new AppPlugin(opts.config as ConstructorParameters[0])); + // The open `security` service above, mounted explicitly — without it every + // dataset query is refused (see `openSecurity`). + await kernel.use({ + name: 'analytics-reconcile.security-open', + init: async (ctx: { registerService(name: string, service: unknown): void }) => { + ctx.registerService('security', openSecurity); + }, + }); // Force the ObjectQL aggregate path (no raw SQL) so legacy and dataset paths // run through the identical engine.aggregate() — the cleanest apples-to-apples. await kernel.use(new AnalyticsServicePlugin({