Skip to content

Commit 5d12b16

Browse files
Trumpclaude
andauthored
fix(service-analytics): row-scope bridge tells absent from broken security service (#17125)
* fix(service-analytics): row-scope bridge tells absent from broken security service The plugin bridges to the `security` service twice. The object-level bridge resolves an explicit three-way — ABSENT admits, THROWING and METHOD-LESS deny at `error`. The row-scope bridge collapsed all three into `undefined`, which is the value `ISecurityService.getReadFilter` reserves for "this caller has no row restriction", so a wired-but-broken security service made analytics queries run with no row-level policy at all, indistinguishable from a deployment that ships no security plugin. Resolve the same three-way here. ABSENT keeps today's behaviour byte for byte (a legitimate single-tenant configuration, reported at init). THROWING and METHOD-LESS report at `error` and throw, which the fail-closed `AnalyticsService.resolveReadScopes` seam already turns into a refusal of the whole query rather than SQL with the object unscoped — the same outcome the object-level bridge produces, and neutral between the two candidate tenant walls. No new error code, no exported symbol, no payload key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * test(service-analytics): admission-bridge doubles carry the row-scope half Two doubles in `admission-bridge-resolution.test.ts` model a WORKING security service with `canReadObject` (or the `explain` fallback) and nothing else. With the row-scope bridge now refusing a registered service that exposes no `getReadFilter`, those doubles describe a state the platform refuses, and the two object-level ADMIT assertions could no longer be reached. `getReadFilter` is a required member of `ISecurityService`, and `undefined` is its documented "no row restriction on this object", so the doubles gain exactly that and stay minimal and conforming. No assertion, no verdict and no object-level behaviour changes; the deny-path doubles are untouched because the object-level gate refuses before the row half is asked. Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 44c917a commit 5d12b16

4 files changed

Lines changed: 357 additions & 8 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@objectstack/service-analytics": minor
3+
---
4+
5+
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)
6+
7+
`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:
8+
9+
```ts
10+
const trySecurity = () => {
11+
try {
12+
const svc = ctx.getService<SecurityReadFilter>('security');
13+
return svc && typeof svc.getReadFilter === 'function' ? svc : undefined;
14+
} catch { return undefined; }
15+
};
16+
getReadScope = (object, context) => trySecurity()?.getReadFilter(object, context);
17+
```
18+
19+
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.
20+
21+
**What changes.** The bridge now resolves the same explicit three-way, at the same reporting level:
22+
23+
- **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.
24+
- **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.
25+
26+
**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.
27+
28+
**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.
29+
30+
⚠️ 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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ async function bootAnalytics(security?: () => unknown) {
113113
return { service: registered.analytics as AnalyticsService, reads, error };
114114
}
115115

116+
/**
117+
* A working security service's ROW-SCOPE half, carried by every double below
118+
* that is meant to represent one.
119+
*
120+
* `getReadFilter` is a REQUIRED member of `ISecurityService`, and since #16918
121+
* the ROW-SCOPE bridge in the same `plugin.ts` refuses the query when the
122+
* registered service does not expose it — the sibling three-way of the one
123+
* this file measures. `undefined` is that method's documented answer for "no
124+
* row restriction on this object", so a double carrying it stays minimal AND
125+
* conforming, and every object-level verdict asserted below is reached exactly
126+
* as it was before. The deny-path doubles need none: the object-level gate runs
127+
* first and refuses before the row half is ever asked.
128+
*/
129+
const rowScopeOpen = { getReadFilter: async () => undefined };
130+
116131
const runProbe = (service: AnalyticsService) =>
117132
service.queryDataset(probe as never, { measures: ['cnt'] } as never, CALLER);
118133

@@ -170,7 +185,7 @@ describe('analytics admission bridge — resolving the "security" service', () =
170185

171186
it('asks canReadObject when the service has it, and serves an ADMITTED caller', async () => {
172187
const canReadObject = vi.fn(() => true);
173-
const { service, reads } = await bootAnalytics(() => ({ canReadObject }));
188+
const { service, reads } = await bootAnalytics(() => ({ ...rowScopeOpen, canReadObject }));
174189

175190
const result = await runProbe(service);
176191
expect(result.rows).toEqual([{ cnt: 24 }]);
@@ -190,6 +205,7 @@ describe('analytics admission bridge — resolving the "security" service', () =
190205

191206
it('falls back to explain for a service that predates canReadObject — both verdicts', async () => {
192207
const admitted = await bootAnalytics(() => ({
208+
...rowScopeOpen,
193209
explain: async () => ({ allowed: true }),
194210
}));
195211
expect((await runProbe(admitted.service)).rows).toEqual([{ cnt: 24 }]);
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The analytics → `security` ROW-SCOPE bridge, and the three resolutions it
5+
* must tell apart — the sibling of `admission-bridge-resolution.test.ts`, one
6+
* function up in the same file.
7+
*
8+
* The object-level bridge was made an explicit three-way (#16860); this one had
9+
* the identical shape and still collapsed it:
10+
*
11+
* ```ts
12+
* const trySecurity = () => {
13+
* try {
14+
* const svc = ctx.getService<SecurityReadFilter>('security');
15+
* return svc && typeof svc.getReadFilter === 'function' ? svc : undefined;
16+
* } catch { return undefined; }
17+
* };
18+
* getReadScope = (object, context) => trySecurity()?.getReadFilter(object, context);
19+
* ```
20+
*
21+
* A THROWING resolver and a METHOD-LESS service both produced `undefined` —
22+
* the same value an ABSENT security service produces, and the same value
23+
* `ISecurityService.getReadFilter` reserves for one meaning only: "this caller
24+
* has no row restriction on this object". So a deployment whose security
25+
* service was broken ran its analytics queries with NO row-level policy at
26+
* all, and the only difference from a correctly unrestricted caller was a state
27+
* nothing reported. After #16860 one door of `plugin.ts` failed closed on a
28+
* throwing resolver and its neighbour failed open — and the neighbour is the
29+
* one carrying row-level policy.
30+
*
31+
* The two broken corners now REFUSE the query: the bridge throws, and
32+
* `AnalyticsService.resolveReadScopes` — fail-closed since ADR-0021 D-C —
33+
* denies the whole query rather than emitting SQL with the object unscoped.
34+
* Refusing is the outcome the object-level bridge already produces, and it is
35+
* neutral between the two candidate tenant walls: it answers "should we serve
36+
* at all", never "what shape is the wall".
37+
*
38+
* ⛔ The ABSENT case is the negative control and must stay UNCHANGED. Refusing
39+
* when no security plugin is installed would break every single-tenant
40+
* deployment — that is a real configuration, reported loudly at init, and it is
41+
* the state in which `/data` has no row-level policy either.
42+
*/
43+
44+
import { describe, it, expect, vi } from 'vitest';
45+
import { DatasetSchema } from '@objectstack/spec/ui';
46+
import type { ExecutionContext } from '@objectstack/spec/kernel';
47+
import { AnalyticsServicePlugin } from '../plugin.js';
48+
import type { AnalyticsService } from '../analytics-service.js';
49+
50+
/** The probe: one object, one count measure, no dimensions. */
51+
const probe = DatasetSchema.parse({
52+
name: 'probe_member',
53+
label: 'probe',
54+
object: 'employer_member',
55+
dimensions: [],
56+
measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }],
57+
});
58+
59+
const CALLER = { userId: 'u_seeker', tenantId: 'org_a' } as ExecutionContext;
60+
61+
/** The SQL posture — the reported path, where nothing else stands in the way. */
62+
const nativeSql = () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false });
63+
64+
/**
65+
* Engine double. Every read is recorded WITH the statement, so "refused" is
66+
* asserted as "the database was never reached" and "scoped" as "the predicate
67+
* was in the statement that ran" — a bridge that refuses after running the
68+
* query has refused nothing, and one that serves rows without the predicate is
69+
* the defect itself.
70+
*/
71+
function fakeEngine() {
72+
const reads: string[] = [];
73+
return {
74+
reads,
75+
engine: {
76+
execute: async (sql: unknown, options?: { object?: string }) => {
77+
reads.push(`execute:${options?.object ?? ''}:${String(sql)}`);
78+
return { rows: [{ cnt: 24 }] };
79+
},
80+
aggregate: async (object: string) => {
81+
reads.push(`aggregate:${object}`);
82+
return [{ cnt: 24 }];
83+
},
84+
getObject: (name: string) =>
85+
name === 'employer_member'
86+
? { fields: { id: { type: 'text' }, organization_id: { type: 'text' } } }
87+
: undefined,
88+
resolveEffectiveDatasource: () => undefined,
89+
},
90+
};
91+
}
92+
93+
/**
94+
* Minimal `PluginContext`. `security` is supplied as a THUNK so a fixture can
95+
* make the lookup itself throw — the corner that is otherwise unreachable from
96+
* a plain service map.
97+
*/
98+
function fakePluginContext(opts: { data: unknown; security?: () => unknown }) {
99+
const registered: Record<string, unknown> = {};
100+
const warn = vi.fn();
101+
const error = vi.fn();
102+
return {
103+
registered,
104+
warn,
105+
error,
106+
ctx: {
107+
getService: (name: string) => {
108+
if (name === 'security') return opts.security ? opts.security() : undefined;
109+
if (name === 'data') return opts.data;
110+
return registered[name];
111+
},
112+
registerService: (name: string, svc: unknown) => { registered[name] = svc; },
113+
replaceService: (name: string, svc: unknown) => { registered[name] = svc; },
114+
logger: { info() {}, warn, error, debug() {} },
115+
},
116+
};
117+
}
118+
119+
/**
120+
* @param admitObjectRead supplied by SOME fixtures on purpose. The object-level
121+
* bridge in the same file resolves the SAME service, so a throwing resolver
122+
* is refused by that gate first and the row-scope bridge under test is never
123+
* reached. Supplying the documented `admitObjectRead` option (a host that
124+
* answers object-level admission itself) leaves the row-scope bridge as the
125+
* only auto-bridge in play, which is what makes this a measurement OF IT.
126+
*/
127+
async function bootAnalytics(
128+
security?: () => unknown,
129+
admitObjectRead?: () => boolean,
130+
) {
131+
const { engine, reads } = fakeEngine();
132+
const { ctx, registered, error } = fakePluginContext({ data: engine, security });
133+
await new AnalyticsServicePlugin({
134+
queryCapabilities: nativeSql,
135+
...(admitObjectRead ? { admitObjectRead } : {}),
136+
}).init(ctx as never);
137+
return { service: registered.analytics as AnalyticsService, reads, error };
138+
}
139+
140+
const runProbe = (service: AnalyticsService) =>
141+
service.queryDataset(probe as never, { measures: ['cnt'] } as never, CALLER);
142+
143+
const errorText = (error: { mock: { calls: unknown[][] } }) =>
144+
error.mock.calls.map((c) => String(c[0])).join('\n');
145+
146+
describe('analytics row-scope bridge — resolving the "security" service', () => {
147+
// ── The two corners that used to run the query with no row policy ──────────
148+
149+
it('REFUSES when resolving the "security" service THROWS', async () => {
150+
const boom = () => { throw new Error('security service is initialising'); };
151+
const { service, reads, error } = await bootAnalytics(boom, () => true);
152+
153+
await expect(runProbe(service)).rejects.toThrow(/read-scope resolution failed/i);
154+
// The refusal has to happen BEFORE the statement runs, or it is not a
155+
// refusal — this is the assertion that fails on `origin/main`, where the
156+
// same fixture serves `{cnt: 24}` off an unscoped statement.
157+
expect(reads).toEqual([]);
158+
// And it has to name why, at error level. A gate that refuses invisibly is
159+
// indistinguishable from one that never ran.
160+
expect(errorText(error)).toMatch(
161+
/row-level read scope could not be resolved .* refusing the query \(fail-closed\).*threw/s,
162+
);
163+
});
164+
165+
it('REFUSES when the registered "security" service exposes no getReadFilter', async () => {
166+
// ⚠️ No `admitObjectRead` override here, and none is needed: this service
167+
// answers the OBJECT-level question (`canReadObject`) and is admitted by
168+
// that bridge, so the row-scope bridge is the only one that can refuse.
169+
// Both auto-bridges are live — this is the shape reachable end-to-end.
170+
const { service, reads, error } = await bootAnalytics(() => ({
171+
canReadObject: () => true,
172+
}));
173+
174+
await expect(runProbe(service)).rejects.toThrow(/read-scope resolution failed/i);
175+
expect(reads).toEqual([]);
176+
expect(errorText(error)).toMatch(
177+
/row-level read scope could not be resolved .* exposes no getReadFilter\(\)/s,
178+
);
179+
});
180+
181+
// ── The negative control: absence is a different state and is UNCHANGED ────
182+
183+
it('ADMITS, unscoped, when NO "security" service is registered at all', async () => {
184+
// ⛔ Not a corner to tighten. This is a single-tenant deployment that ships
185+
// no `plugin-security`: there is no row-level policy anywhere on it,
186+
// `/data` included, and the init log says so. Refusing here would break
187+
// every such deployment — which is why this arm is what makes the two
188+
// above a measurement rather than an over-fix.
189+
const { service, reads, error } = await bootAnalytics(undefined);
190+
191+
const result = await runProbe(service);
192+
expect(result.rows).toEqual([{ cnt: 24 }]);
193+
expect(reads).toHaveLength(1);
194+
expect(errorText(error)).not.toMatch(/row-level read scope/);
195+
});
196+
197+
// ── The working spelling, so the refusals cannot pass by refusing all ──────
198+
199+
it('asks getReadFilter when the service has it, and SCOPES the statement', async () => {
200+
const getReadFilter = vi.fn(async () => ({ organization_id: 'org_a' }));
201+
const { service, reads } = await bootAnalytics(() => ({
202+
canReadObject: () => true,
203+
getReadFilter,
204+
}));
205+
206+
const result = await runProbe(service);
207+
expect(result.rows).toEqual([{ cnt: 24 }]);
208+
expect(getReadFilter).toHaveBeenCalledWith('employer_member', CALLER);
209+
expect(reads).toHaveLength(1);
210+
expect(reads[0]).toMatch(/organization_id/);
211+
});
212+
213+
// ── The two doors of this file now agree on a broken provider ──────────────
214+
215+
it('refuses a throwing resolver with BOTH auto-bridges live (no door falls open)', async () => {
216+
// With no `admitObjectRead` override the object-level bridge (#16860)
217+
// answers first, with `PERMISSION_DENIED`. Pinned so the file-level
218+
// property — a broken security service serves no analytics rows through
219+
// EITHER door — cannot regress from the other side.
220+
const boom = () => { throw new Error('security service is initialising'); };
221+
const { service, reads } = await bootAnalytics(boom);
222+
223+
await expect(runProbe(service)).rejects.toMatchObject({
224+
code: 'PERMISSION_DENIED',
225+
status: 403,
226+
});
227+
expect(reads).toEqual([]);
228+
});
229+
});

0 commit comments

Comments
 (0)