Skip to content

Commit 54b3d1d

Browse files
os-teslaclaude
andauthored
fix(service-analytics): a row-scope refusal carries a declared envelope, so queryDataset stops classifying refusals by their wording (#17336)
* test(service-analytics): derive a guard over refusal wording from the source `queryDataset` classifies a BARE error by its words, and three of the six limbs (`not registered`, `unknown object`, `is not a registered object`) are the phrasings a registry or security refusal reaches for. The guard walks every `throw` in this package's non-test sources with the TypeScript AST, resolves each one's wording through in-package message helpers, and asserts none of the un-enveloped ones can be read as a driver reporting an absent table. Population and verdict are both derived: the sites from the AST, the verdict from `isMissingSourceError` itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * fix(service-analytics): declare an envelope on the row-scope fail-closed refusals `queryDataset` re-throws any error whose producer declared `code` + `status` and classifies everything else by wording — six substrings, three of which (`not registered`, `unknown object`, `is not a registered object`) are what a registry or security refusal reaches for. A hit is not a wrong status code, it is `{rows: [], fields: [], totals: []}`: a fail-closed gate served as a confident empty chart. The row-scope RESOLUTION stage refused bare on both of its sites, and propagated only because its text happened to miss all six. Both now carry `READ_SCOPE_COMPILE_FAILED` / 500 — the code the sibling LOWERING stage has owned since #5367, so no ledger row is added — through one constructor. Every message is byte-unchanged: the fix is the declaration, not a luckier string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 * chore(changeset): row-scope refusals declare READ_SCOPE_COMPILE_FAILED / 500 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e1eee43 commit 54b3d1d

6 files changed

Lines changed: 847 additions & 6 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/service-analytics": patch
3+
---
4+
5+
fix(service-analytics): a fail-closed row-scope refusal can no longer be served as an empty chart (#17130)
6+
7+
`queryDataset` degrades to `{rows: [], fields: [], totals: []}` when a BARE error looks like a driver reporting an absent table — a deliberate leniency (#5033) so a dashboard widget over an unmounted object renders "no data" instead of failing. The test is a substring match over the message, and three of its six limbs — `not registered`, `unknown object`, `is not a registered object` — are exactly the phrasings a registry or security refusal reaches for.
8+
9+
Both sites of the row-scope RESOLUTION stage refused with a bare `throw new Error(…)`: the `security` bridge in `AnalyticsServicePlugin`, and `AnalyticsService.resolveReadScopes`. They propagated only because their wording happened to miss all six — so any reword, or any refusal added to that stage later, could silently turn a fail-closed gate into a `200` with no rows.
10+
11+
Both now declare `READ_SCOPE_COMPILE_FAILED` / `500` — the code the sibling read-scope LOWERING stage has answered with since #5367, so the registered wire vocabulary is unchanged. Two visible consequences for a deployment whose wired `security` service cannot answer a row-level read scope:
12+
13+
- the refusal reaches the caller as a declared `500` instead of relying on its phrasing to escape the degradation path;
14+
- its message is withheld from the response body by declaration (the operator still gets the full text, at `error`, from the producing site) rather than echoed.
15+
16+
Every refusal message is byte-unchanged, and #5033's leniency is untouched: a genuine absent source table still degrades to the empty result with its `warn`, and a deployment with NO security service still runs unscoped exactly as before. A guard derived from the source (`refusal-wording-collision.test.ts`) now walks every `throw` in the package and fails if an un-enveloped refusal can be read as a missing source table.
Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#17130] The row-scope RESOLUTION refusals declare themselves —
5+
* `READ_SCOPE_COMPILE_FAILED` / 500 — so no wording can turn one into an empty
6+
* chart.
7+
*
8+
* ## What was wrong
9+
*
10+
* The row-level read scope is established in two stages. The LOWERING stage
11+
* (`read-scope-sql.ts`) has declared `READ_SCOPE_COMPILE_FAILED` / 500 since
12+
* #5367. The RESOLUTION stage — `plugin.ts`'s `security` bridge, and
13+
* `AnalyticsService.resolveReadScopes` — refused with a bare
14+
* `throw new Error(…)`.
15+
*
16+
* A bare refusal is the one kind `queryDataset`'s catch classifies by WORDING:
17+
* `hasDeclaredErrorEnvelope` re-throws anything a producer classified, and only
18+
* the unclassified reaches `isMissingSourceError` — six substrings, three of
19+
* which (`not registered`, `unknown object`, `is not a registered object`) are
20+
* exactly the phrasings a registry or security refusal reaches for. A hit is
21+
* not a wrong status code; it is `{rows: [], fields: [], totals: []}` served to
22+
* the caller — a fail-closed gate rendered as a confident empty chart, with one
23+
* `warn` and no exception.
24+
*
25+
* PR #17125's refusal propagates today only because its text happens to match
26+
* none of the six. ⛔ A coincidence, not a construction — and the fix is the
27+
* DECLARATION, not a luckier string: every message below is byte-unchanged.
28+
*
29+
* ## The three blocks
30+
*
31+
* `the producers declare it` captures the real refusals — the bridge's and the
32+
* pre-pass's — and pins `code` + `status` on them. These are the rows that go
33+
* red on the ablation, because their wording never matched the sniffer and
34+
* never needed to.
35+
*
36+
* `a colliding refusal propagates` is the property the card asks for, over all
37+
* THREE colliding limbs. Bare, each of these is an empty chart; enveloped, each
38+
* reaches the caller. It uses the package's own constructor rather than a
39+
* synthesised envelope, because the claim under test is about the refusals THIS
40+
* PACKAGE raises.
41+
*
42+
* `#5033's leniency is untouched` is the negative control, and it must stay
43+
* byte-identical: a genuine absent source table still degrades to the empty
44+
* result with its `warn`, and an ABSENT security service still runs unscoped.
45+
* That is the deliberate behaviour the file's own docblock exists to protect,
46+
* and ⛔ nothing here may regress it.
47+
*
48+
* ## Reverse verification — direction predicted BEFORE running
49+
*
50+
* Ordinary direction (red), and SPLIT, because the two blocks fail for
51+
* different reasons:
52+
*
53+
* - Revert both producers to `throw new Error(…)`: every row of
54+
* `the producers declare it` that reads `code`/`status` goes RED on
55+
* `undefined`, and every row of `a colliding refusal propagates` goes RED
56+
* by returning the empty result instead of throwing.
57+
* - The `#5033` block stays GREEN in both states — which is what "the
58+
* leniency is untouched" means as evidence rather than as a claim.
59+
*
60+
* ⛔ Note which rows do NOT move: the real producers' MESSAGES are asserted in
61+
* `the producers declare it` and in `read-scope-bridge-resolution.test.ts`, and
62+
* they stay green under the ablation — the wording did not change, and that
63+
* asymmetry (envelope red, wording green) is the whole finding.
64+
*/
65+
66+
import { describe, it, expect, vi } from 'vitest';
67+
import { DatasetSchema } from '@objectstack/spec/ui';
68+
import type { ExecutionContext } from '@objectstack/spec/kernel';
69+
import { AnalyticsService } from '../analytics-service.js';
70+
import { readScopeUnresolvedError } from '../read-scope-refusal.js';
71+
72+
/** The ADR-0112 fields the REST boundary classifies on. */
73+
interface Refusal extends Error {
74+
code?: unknown;
75+
status?: unknown;
76+
}
77+
78+
const EMPTY = { rows: [], fields: [], totals: [] };
79+
80+
const dataset = DatasetSchema.parse({
81+
name: 'sales',
82+
label: 'Sales',
83+
object: 'opportunity',
84+
dimensions: [{ name: 'stage', field: 'stage', type: 'string' }],
85+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
86+
});
87+
88+
const SELECTION = { dimensions: ['stage'], measures: ['revenue'] };
89+
const CTX = { tenantId: 'org_A', userId: 'u_seeker' } as ExecutionContext;
90+
91+
function logger() {
92+
return { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn(), child: vi.fn() } as any;
93+
}
94+
95+
async function refusalFrom(thunk: () => unknown | Promise<unknown>): Promise<Refusal | undefined> {
96+
try {
97+
await thunk();
98+
return undefined;
99+
} catch (e) {
100+
return e as Refusal;
101+
}
102+
}
103+
104+
/** A service whose EXECUTION throws `thrown` — the way into `queryDataset`'s catch. */
105+
function serviceThatThrows(thrown: unknown, log = logger()) {
106+
return new AnalyticsService({
107+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
108+
executeRawSql: async () => { throw thrown; },
109+
isRegisteredObject: () => true,
110+
logger: log,
111+
});
112+
}
113+
114+
/**
115+
* A service whose ROW-SCOPE PROVIDER throws — the real seam, reached through
116+
* `resolveReadScopes` from inside `queryDataset`'s try. Execution itself is
117+
* healthy, so a result coming back at all means the fail-closed pre-pass was
118+
* bypassed or its refusal was swallowed.
119+
*/
120+
function serviceWhoseScopeProviderThrows(cause: unknown, log = logger()) {
121+
return new AnalyticsService({
122+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
123+
executeRawSql: async () => [{ stage: 'won', revenue: 42 }],
124+
isRegisteredObject: () => true,
125+
getReadScope: () => { throw cause; },
126+
logger: log,
127+
});
128+
}
129+
130+
/**
131+
* The three limbs of `isMissingSourceError` a registry or security refusal
132+
* naturally reaches for — the card's own list, spelled here as the CALLER-side
133+
* inputs of the property rather than as a claim about the predicate (the
134+
* predicate itself is asserted in `refusal-wording-collision.test.ts`, which
135+
* imports the real function).
136+
*
137+
* ⚠️ Each names the dataset's OWN object on purpose. A bare colliding refusal
138+
* lands in one of #5033's TWO arms depending on which relation
139+
* `missingSourceRelation` reads out of it: name the dataset's own object and it
140+
* degrades to the empty chart (the arm this card is about); name anything else
141+
* — including a stray word the extractor mistakes for a table, measured: "…
142+
* unknown object in the resolved scope" yields `in` — and it is re-reported as
143+
* a cross-datasource topology error, loud but describing a JOIN that does not
144+
* exist. Both arms are wrong for a security refusal; the silent one is the one
145+
* under test here, so these fixtures aim at it deliberately rather than by
146+
* luck.
147+
*/
148+
const COLLIDING_WORDINGS = [
149+
'[Analytics] row-level read scope could not be resolved for "opportunity"; the policy names object "opportunity" is not registered with the security service.',
150+
'[Analytics] row-level read scope could not be resolved; the resolved scope names unknown object: opportunity.',
151+
'[Analytics] read-scope resolution failed: "opportunity" is not a registered object on the security service.',
152+
];
153+
154+
describe('[#17130] the row-scope resolution refusals declare an ADR-0112 envelope', () => {
155+
it('the constructor stamps the code the lowering stage already owns', () => {
156+
const err = readScopeUnresolvedError('[Analytics] read-scope resolution failed for "x"; query denied (fail-closed).') as Refusal;
157+
expect(err).toBeInstanceOf(Error);
158+
expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED');
159+
expect(err.status).toBe(500);
160+
// ⛔ The message is the site's, untouched — #17130 fixes the declaration.
161+
expect(err.message).toBe('[Analytics] read-scope resolution failed for "x"; query denied (fail-closed).');
162+
});
163+
164+
it('resolveReadScopes denies with the envelope, and the wording is unchanged', async () => {
165+
const log = logger();
166+
const err = await refusalFrom(() =>
167+
serviceWhoseScopeProviderThrows(new Error('security service exploded'), log)
168+
.queryDataset(dataset, SELECTION, CTX),
169+
);
170+
expect(err, 'a fail-closed row-scope denial was swallowed').toBeInstanceOf(Error);
171+
expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED');
172+
expect(err?.status).toBe(500);
173+
expect(String(err?.message)).toMatch(/read-scope resolution failed for "opportunity"; query denied \(fail-closed\)/);
174+
// Refused ⇒ never degraded, so no "empty result" warn was emitted either.
175+
expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result'));
176+
// …and the operator still gets the cause at `error`, as before.
177+
expect(log.error).toHaveBeenCalledWith(
178+
expect.stringContaining('read-scope resolution failed for object "opportunity"'),
179+
expect.any(Error),
180+
);
181+
});
182+
183+
it('the bridge refusal reaching the pre-pass keeps a declared envelope end to end', async () => {
184+
// The bridge (`plugin.ts`) throws its own enveloped refusal; the pre-pass
185+
// catches it and answers with its own. Both stages declared ⇒ whichever one
186+
// reaches `queryDataset` is re-thrown by declaration, never sniffed.
187+
const bridgeRefusal = readScopeUnresolvedError(
188+
'[Analytics] row-level read scope could not be resolved for "opportunity"; query refused (fail-closed).',
189+
);
190+
const err = await refusalFrom(() =>
191+
serviceWhoseScopeProviderThrows(bridgeRefusal).queryDataset(dataset, SELECTION, CTX),
192+
);
193+
expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED');
194+
expect(err?.status).toBe(500);
195+
});
196+
});
197+
198+
describe('[#17130] a colliding refusal propagates instead of becoming a 200 with an empty chart', () => {
199+
for (const wording of COLLIDING_WORDINGS) {
200+
it(`propagates: ${wording.slice(0, 64)}…`, async () => {
201+
const log = logger();
202+
const err = await refusalFrom(() =>
203+
serviceThatThrows(readScopeUnresolvedError(wording), log).queryDataset(dataset, SELECTION, CTX),
204+
);
205+
expect(err, 'a fail-closed refusal was served as an empty chart').toBeInstanceOf(Error);
206+
expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED');
207+
expect(err?.status).toBe(500);
208+
expect(String(err?.message)).toBe(wording);
209+
expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result'));
210+
});
211+
}
212+
213+
it('…and the same three wordings BARE still degrade — which is what makes the envelope the fix', async () => {
214+
// ⛔ Not an aspiration: this is the measurement that says the property is
215+
// carried by the declaration and not by the phrasing. Strip the envelope
216+
// and every one of the three is an empty chart again. That is also the
217+
// ablation's predicted shape, asserted here so the claim is re-runnable.
218+
for (const wording of COLLIDING_WORDINGS) {
219+
const result = await serviceThatThrows(new Error(wording)).queryDataset(dataset, SELECTION, CTX);
220+
expect(result, `bare wording unexpectedly propagated: ${wording}`).toEqual(EMPTY);
221+
}
222+
});
223+
});
224+
225+
describe('[#17130] #5033’s deliberate leniency is untouched — the negative control', () => {
226+
it('a genuine absent source table still degrades to the empty result, with the warn', async () => {
227+
const log = logger();
228+
const result = await serviceThatThrows(
229+
new Error('SELECT COUNT(*) FROM "opportunity" - no such table: opportunity'),
230+
log,
231+
).queryDataset(dataset, SELECTION, CTX);
232+
expect(result).toEqual(EMPTY);
233+
expect(log.warn).toHaveBeenCalledWith(
234+
expect.stringContaining('backing object "opportunity" is unavailable'),
235+
);
236+
});
237+
238+
it('postgres’s and mysql’s real wordings still degrade too', async () => {
239+
expect(
240+
await serviceThatThrows(
241+
new Error('select "stage" from "opportunity" - relation "opportunity" does not exist'),
242+
).queryDataset(dataset, SELECTION, CTX),
243+
).toEqual(EMPTY);
244+
expect(
245+
await serviceThatThrows(new Error("Table 'app.opportunity' doesn't exist")).queryDataset(
246+
dataset,
247+
SELECTION,
248+
CTX,
249+
),
250+
).toEqual(EMPTY);
251+
});
252+
253+
it('an ABSENT row-scope provider still runs the query unscoped', async () => {
254+
// The state `read-scope-bridge-resolution.test.ts` calls the negative
255+
// control: no security service at all is a real single-tenant deployment,
256+
// reported loudly at init, and refusing there would break it. Re-asserted
257+
// here so this file's own change cannot narrow it.
258+
const svc = new AnalyticsService({
259+
queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
260+
executeRawSql: async () => [{ stage: 'won', revenue: 42 }],
261+
isRegisteredObject: () => true,
262+
logger: logger(),
263+
});
264+
const result = await svc.queryDataset(dataset, SELECTION, CTX);
265+
expect(result.rows).toEqual([{ stage: 'won', revenue: 42 }]);
266+
});
267+
});

0 commit comments

Comments
 (0)