From 5509243901d8f7296a3dfda433b09c48f9665c57 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 04:07:15 +0000 Subject: [PATCH 1/3] 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 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../refusal-wording-collision.test.ts | 406 ++++++++++++++++++ .../src/analytics-service.ts | 11 +- 2 files changed, 416 insertions(+), 1 deletion(-) create mode 100644 packages/services/service-analytics/src/__tests__/refusal-wording-collision.test.ts diff --git a/packages/services/service-analytics/src/__tests__/refusal-wording-collision.test.ts b/packages/services/service-analytics/src/__tests__/refusal-wording-collision.test.ts new file mode 100644 index 0000000000..3214390d89 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/refusal-wording-collision.test.ts @@ -0,0 +1,406 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17130] No BARE refusal this package raises may be readable as a driver + * saying "the backing table is gone". + * + * ## The fragility this exists to hold down + * + * `queryDataset`'s catch asks two questions in order (`analytics-service.ts`): + * {@link isMissingSourceError} runs only for an error whose producer declared + * NOTHING, and when it says yes the widget is served + * `{rows: [], fields: [], totals: []}` — no exception, no 4xx, no 5xx, one + * `warn`, a confident empty chart. That is #5033's deliberate leniency, and it + * is correct for a driver reporting an absent table. + * + * It is a heuristic over driver PHRASING, 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. So a bare + * refusal this package raises on purpose is one wording away from being served + * to the caller as "no data": a fail-closed gate turned back into a fail-open + * one by substring match. PR #17125's row-scope refusal propagates today + * because its text happens to match none of the six — a coincidence, not a + * construction, and the coincidence is what this file removes. + * + * ⛔ The remedy is NOT to subtract this or any other message from the sniffer + * by hand. #6035 already recorded why that road ends: the missing-COLUMN + * wording literally CONTAINS a well-formed missing-relation wording, so *"no + * tightening of 'does this say a relation is missing' can ever exclude it — + * only asking the more specific question FIRST can. That makes the ORDER the + * fix, not the pattern."* Subtracting a string fixes one string and leaves the + * class standing. + * + * ## Two defences, and which one this file is + * + * - **The envelope (the primary).** A refusal that declares `code` + `status` + * is re-thrown at `hasDeclaredErrorEnvelope` before the sniffer is asked at + * all (#5717 defence B), so its wording cannot classify it and its runtime + * interpolations cannot either. #17130's other half gives the two + * read-scope refusals that envelope. + * - **This guard (the second line).** A refusal that is deliberately bare — + * an internal invariant, the families `dataset-refusal.ts`'s header lists + * as staying bare on purpose — still reaches the sniffer, so its AUTHORED + * wording must not collide with it. That is the property asserted here, and + * it keeps being asserted for refusals written after today. + * + * The split is why an enveloped refusal is deliberately NOT held to the wording + * rule: forcing one to be reworded would buy no safety (nothing reads its + * words) and would push authors toward picking luckier strings — the exact move + * #17130 forbids. + * + * ## Where the population comes from — ⛔ never a hand-written list + * + * A guard whose corpus is typed out rots the same way the sniffer did: it + * describes the refusals someone remembered. So both halves are derived from + * the source, at run time: + * + * - **the sites** — every `throw` statement in every non-test `.ts` file + * under this package's `src/`, found by walking the TypeScript AST + * (`typescript` is already this package's devDependency). Adding a file, or + * a `throw` in one, enlarges the corpus with no edit here. + * - **the words** — every string / template literal reachable from that + * throw's expression, followed THROUGH calls to functions declared in this + * package. That transitive step is what reaches the message of + * `readAdmissionDeniedError(objectName)` or `undefinedComparandError(field, + * path)`, whose throw sites carry no literal of their own. + * - **the verdict** — {@link isMissingSourceError} itself, imported from + * `analytics-service.ts`. ⛔ Not a copy of its six limbs: a copy answers a + * question about the copy and stays green when a seventh limb lands. + * - **enveloped vs bare** — also derived. A throw is ENVELOPED when its + * expression calls an in-package function that assigns both `.code` and + * `.status` (directly, or through another function that does). + * + * ## What it cannot see, stated rather than implied + * + * A template literal's INTERPOLATED value is a runtime string; `${object}` is + * replaced by a placeholder here. So this guard covers authored wording only — + * which is exactly why it is the second line and the envelope is the first. A + * refusal whose text is assembled from an upstream message at run time is made + * safe by declaring `code` + `status`, never by this scan. + * + * ## Positive control (⛔ a probe that can only answer zero is NOT MEASURED) + * + * `the scan can see a colliding refusal` runs the whole pipeline over a + * synthetic source file carrying one bare colliding `throw` and asserts it is + * found — so a green verdict from the real corpus means "looked and found + * none", never "looked at nothing". The census case pins that both classes are + * non-empty, for the same reason. + * + * MEASURED live as well, on the real tree, before this file was finished: a + * bare `throw new Error('[Analytics] object "x" is not registered on this + * datasource.')` inserted into `plugin.ts` turned the collision case RED and + * named that site; removing it turned it green. The PR body carries both runs. + */ + +import { readFileSync, readdirSync } from 'node:fs'; +import { join, relative, resolve, sep } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import ts from 'typescript'; +import { isMissingSourceError } from '../analytics-service.js'; + +/** This package's `src/` — the scan root. Stays inside the package. */ +const SRC_ROOT = resolve(import.meta.dirname, '..'); + +/** A `throw` site, with the wording it can put in front of the sniffer. */ +interface ThrowSite { + /** `src/`-relative path, e.g. `plugin.ts` or `strategies/filter-normalizer.ts`. */ + file: string; + line: number; + /** The throw expression's first line, for a failure a reader can act on. */ + source: string; + /** Does its producer declare `code` + `status`? */ + enveloped: boolean; + /** Authored message texts, interpolations replaced by a placeholder. */ + texts: string[]; +} + +/** What an interpolation becomes — a token no limb of the sniffer can match. */ +const INTERPOLATION = '{value}'; + +function collectSourceFiles(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === '__tests__') continue; + collectSourceFiles(full, out); + } else if ( + entry.name.endsWith('.ts') && + !entry.name.endsWith('.test.ts') && + !entry.name.endsWith('.d.ts') + ) { + out.push(full); + } + } + return out; +} + +type FunctionLike = ts.FunctionDeclaration | ts.FunctionExpression | ts.ArrowFunction | ts.MethodDeclaration; + +function isFunctionLike(node: ts.Node): node is FunctionLike { + return ( + ts.isFunctionDeclaration(node) || + ts.isFunctionExpression(node) || + ts.isArrowFunction(node) || + ts.isMethodDeclaration(node) + ); +} + +/** + * Every function in the package, indexed by the name it is CALLED by. Two files + * may declare the same name (`undefinedComparandError` exists in both + * `read-scope-sql.ts` and `filter-normalizer.ts`); both are kept, and a call + * resolves to all of them. Over-resolution only widens the corpus, which is the + * safe direction for a guard. + */ +function indexFunctions(files: readonly ts.SourceFile[]): Map { + const index = new Map(); + const add = (name: string, fn: FunctionLike) => { + const bucket = index.get(name); + if (bucket) bucket.push(fn); + else index.set(name, [fn]); + }; + for (const sf of files) { + const visit = (node: ts.Node): void => { + if (ts.isFunctionDeclaration(node) && node.name) add(node.name.text, node); + else if (ts.isMethodDeclaration(node) && ts.isIdentifier(node.name)) add(node.name.text, node); + else if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.initializer && + isFunctionLike(node.initializer) + ) { + add(node.name.text, node.initializer); + } + ts.forEachChild(node, visit); + }; + visit(sf); + } + return index; +} + +/** Does this function body assign BOTH `.code` and `.status` on something? */ +function assignsEnvelopeFields(fn: FunctionLike): boolean { + let code = false; + let status = false; + const visit = (node: ts.Node): void => { + if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isPropertyAccessExpression(node.left) + ) { + if (node.left.name.text === 'code') code = true; + if (node.left.name.text === 'status') status = true; + } + ts.forEachChild(node, visit); + }; + if (fn.body) visit(fn.body); + return code && status; +} + +/** + * Which in-package functions produce an ADR-0112 envelope — a fixpoint, so a + * constructor that delegates to another constructor counts as one too. + */ +function resolveEnvelopingNames(index: Map): Set { + const enveloping = new Set(); + for (const [name, fns] of index) { + if (fns.some(assignsEnvelopeFields)) enveloping.add(name); + } + for (let pass = 0; pass < 8; pass += 1) { + const before = enveloping.size; + for (const [name, fns] of index) { + if (enveloping.has(name)) continue; + const delegates = fns.some((fn) => { + let hit = false; + const visit = (node: ts.Node): void => { + if (hit) return; + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && enveloping.has(node.expression.text)) { + hit = true; + return; + } + ts.forEachChild(node, visit); + }; + if (fn.body) visit(fn.body); + return hit; + }); + if (delegates) enveloping.add(name); + } + if (enveloping.size === before) break; + } + return enveloping; +} + +/** The authored text of one literal node, interpolations neutralised. */ +function literalText(node: ts.Node): string | undefined { + if (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) return node.text; + if (ts.isTemplateExpression(node)) { + let text = node.head.text; + for (const span of node.templateSpans) text += INTERPOLATION + span.literal.text; + return text; + } + return undefined; +} + +/** + * Every authored literal reachable from `root`, following calls to functions + * this package declares (bounded by a visited set, so mutual recursion between + * two constructors terminates). + */ +function reachableTexts( + root: ts.Node, + index: Map, + seen: Set = new Set(), +): string[] { + const texts: string[] = []; + const visit = (node: ts.Node): void => { + const text = literalText(node); + if (text !== undefined) { + texts.push(text); + // A template's spans hold expressions that may call a message helper. + if (ts.isTemplateExpression(node)) for (const span of node.templateSpans) visit(span.expression); + return; + } + if ((ts.isCallExpression(node) || ts.isNewExpression(node)) && ts.isIdentifier(node.expression)) { + for (const fn of index.get(node.expression.text) ?? []) { + if (seen.has(fn)) continue; + seen.add(fn); + if (fn.body) texts.push(...reachableTexts(fn.body, index, seen)); + } + } + ts.forEachChild(node, visit); + }; + visit(root); + return texts; +} + +/** Is this throw's producer one of the enveloping constructors? */ +function throwIsEnveloped(expression: ts.Node, enveloping: ReadonlySet): boolean { + let hit = false; + const visit = (node: ts.Node): void => { + if (hit) return; + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && enveloping.has(node.expression.text)) { + hit = true; + return; + } + ts.forEachChild(node, visit); + }; + visit(expression); + return hit; +} + +/** The whole pipeline, over an arbitrary set of parsed sources. */ +function scanThrowSites(sources: readonly ts.SourceFile[]): ThrowSite[] { + const index = indexFunctions(sources); + const enveloping = resolveEnvelopingNames(index); + const sites: ThrowSite[] = []; + for (const sf of sources) { + const visit = (node: ts.Node): void => { + if (ts.isThrowStatement(node) && node.expression) { + const { line } = sf.getLineAndCharacterOfPosition(node.getStart(sf)); + sites.push({ + file: relative(SRC_ROOT, sf.fileName).split(sep).join('/'), + line: line + 1, + source: node.expression.getText(sf).split('\n')[0].trim().slice(0, 120), + enveloped: throwIsEnveloped(node.expression, enveloping), + texts: reachableTexts(node.expression, index), + }); + } + ts.forEachChild(node, visit); + }; + visit(sf); + } + return sites; +} + +function parse(fileName: string, text: string): ts.SourceFile { + return ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true); +} + +const REAL_SOURCES = collectSourceFiles(SRC_ROOT).sort().map((f) => parse(f, readFileSync(f, 'utf8'))); +const REAL_SITES = scanThrowSites(REAL_SOURCES); + +/** Every colliding (site, text) pair in a scan — the failure report itself. */ +function collisions(sites: readonly ThrowSite[]): string[] { + const found: string[] = []; + for (const site of sites) { + if (site.enveloped) continue; + for (const text of site.texts) { + if (isMissingSourceError({ message: text })) { + found.push(`${site.file}:${site.line} — ${JSON.stringify(text)} · ${site.source}`); + } + } + } + return found; +} + +describe('[#17130] the refusals this package raises cannot be mistaken for a missing source table', () => { + it('the corpus is derived from source, and both classes are populated', () => { + // ⛔ The census, not decoration. A scan that silently found nothing — a + // renamed directory, a walk that stopped at the first subfolder — would + // pass the collision case below by finding no refusals at all. These + // floors are deliberately far under today's counts so ordinary authoring + // never touches them; only a broken scan can. + expect(REAL_SOURCES.length).toBeGreaterThan(10); + expect(REAL_SITES.length).toBeGreaterThan(40); + expect(REAL_SITES.filter((s) => s.texts.length > 0).length).toBeGreaterThan(40); + // Both verdicts must be reachable: an all-bare classification would make + // the exemption meaningless, an all-enveloped one would empty the corpus + // the collision case reads. + expect(REAL_SITES.some((s) => s.enveloped)).toBe(true); + expect(REAL_SITES.some((s) => !s.enveloped && s.texts.length > 0)).toBe(true); + // The transitive step earns its keep: these two throw sites carry no + // literal of their own, and their wording lives one call away. + const helperOnly = REAL_SITES.filter((s) => /readAdmissionDeniedError|undefinedComparandError/.test(s.source)); + expect(helperOnly.length).toBeGreaterThan(0); + for (const site of helperOnly) { + expect(site.texts.length, `${site.file}:${site.line} resolved no wording`).toBeGreaterThan(0); + } + }); + + it('the scan can see a colliding refusal — the positive control', () => { + // The same pipeline over a synthetic file. Two throws: one bare and + // colliding (must be reported), one enveloped through a local constructor + // and equally colliding (must NOT be — the envelope answers first). + const control = parse( + join(SRC_ROOT, '__control__.ts'), + [ + 'function envelopedRefusal(message: string): Error {', + ' const err = new Error(message) as Error & { code?: string; status?: number };', + " err.code = 'READ_SCOPE_COMPILE_FAILED';", + ' err.status = 500;', + ' return err;', + '}', + 'export function bare(object: string): never {', + ' throw new Error(`[Analytics] object "${object}" is not registered on this datasource.`);', + '}', + 'export function declared(object: string): never {', + ' throw envelopedRefusal(`[Analytics] object "${object}" is not registered on this datasource.`);', + '}', + ].join('\n'), + ); + const found = collisions(scanThrowSites([control])); + expect(found.length, 'the scan is blind to a colliding bare refusal').toBe(1); + expect(found[0]).toMatch(/__control__\.ts:8/); + expect(found[0]).toMatch(/is not registered/); + }); + + it('the verdict is the real predicate, not a copy of its limbs', () => { + // If this ever goes red, `isMissingSourceError` moved and the import above + // is answering a different question than `queryDataset` asks. + expect(isMissingSourceError({ message: 'no such table: opportunity' })).toBe(true); + expect(isMissingSourceError({ message: 'object "crm_account" is not registered' })).toBe(true); + expect(isMissingSourceError({ message: 'unknown object: crm_account' })).toBe(true); + expect(isMissingSourceError({ message: '"crm_account" is not a registered object' })).toBe(true); + expect(isMissingSourceError({ message: `[Analytics] refused for "${'x'}" (fail-closed).` })).toBe(false); + }); + + it('⛔ no bare refusal in this package matches isMissingSourceError', () => { + // The property. A red here is NOT a licence to subtract the message from + // the sniffer, nor to hunt for a luckier phrasing: give the refusal a + // declared `code` + `status` (see `dataset-refusal.ts` and + // `read-scope-refusal.ts`) so the door classifies it by declaration, or — + // when it is genuinely a driver reporting an absent table — leave it bare + // and let it degrade on purpose. + expect(collisions(REAL_SITES)).toEqual([]); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index f40f1fdd94..239a1f6623 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -237,8 +237,17 @@ function isMissingColumnOfRelation(message: string): boolean { * wording, so no tightening of "does this say a relation is missing" can ever * exclude it — only asking the more specific question FIRST can. That makes the * ORDER the fix, not the pattern. + * + * [#17130] EXPORTED — module-internal still (it is absent from `index.ts`, and + * this package's `exports` map publishes only that entry, so no consumer can + * reach it), but reachable from `refusal-wording-collision.test.ts`. That guard + * asserts no BARE refusal this package raises can be mistaken for a driver + * saying "the table is gone", and it has to ask THIS predicate: a guard + * carrying its own copy of the six limbs answers a question about the copy, and + * would stay green the moment a limb is added here — precisely the drift it + * exists to catch. */ -function isMissingSourceError(err: unknown): boolean { +export function isMissingSourceError(err: unknown): boolean { const raw = String((err as { message?: unknown })?.message ?? err ?? ''); // [#6035] Missing COLUMN is not missing SOURCE — the paragraph above promises // column errors stay hard failures, and this is where that promise is kept. From aa8bc7a3945f6f37e3989f3156d2f314227d9f00 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 04:37:09 +0000 Subject: [PATCH 2/3] fix(service-analytics): declare an envelope on the row-scope fail-closed refusals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../read-scope-resolution-envelope.test.ts | 267 ++++++++++++++++++ .../src/analytics-service.ts | 17 +- .../services/service-analytics/src/plugin.ts | 27 +- .../src/read-scope-refusal.ts | 109 +++++++ 4 files changed, 415 insertions(+), 5 deletions(-) create mode 100644 packages/services/service-analytics/src/__tests__/read-scope-resolution-envelope.test.ts create mode 100644 packages/services/service-analytics/src/read-scope-refusal.ts diff --git a/packages/services/service-analytics/src/__tests__/read-scope-resolution-envelope.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-resolution-envelope.test.ts new file mode 100644 index 0000000000..031e955a64 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-resolution-envelope.test.ts @@ -0,0 +1,267 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17130] The row-scope RESOLUTION refusals declare themselves — + * `READ_SCOPE_COMPILE_FAILED` / 500 — so no wording can turn one into an empty + * chart. + * + * ## What was wrong + * + * The row-level read scope is established in two stages. The LOWERING stage + * (`read-scope-sql.ts`) has declared `READ_SCOPE_COMPILE_FAILED` / 500 since + * #5367. The RESOLUTION stage — `plugin.ts`'s `security` bridge, and + * `AnalyticsService.resolveReadScopes` — refused with a bare + * `throw new Error(…)`. + * + * A bare refusal is the one kind `queryDataset`'s catch classifies by WORDING: + * `hasDeclaredErrorEnvelope` re-throws anything a producer classified, and only + * the unclassified reaches `isMissingSourceError` — six substrings, three of + * which (`not registered`, `unknown object`, `is not a registered object`) are + * exactly the phrasings a registry or security refusal reaches for. A hit is + * not a wrong status code; it is `{rows: [], fields: [], totals: []}` served to + * the caller — a fail-closed gate rendered as a confident empty chart, with one + * `warn` and no exception. + * + * PR #17125's refusal propagates today only because its text happens to match + * none of the six. ⛔ A coincidence, not a construction — and the fix is the + * DECLARATION, not a luckier string: every message below is byte-unchanged. + * + * ## The three blocks + * + * `the producers declare it` captures the real refusals — the bridge's and the + * pre-pass's — and pins `code` + `status` on them. These are the rows that go + * red on the ablation, because their wording never matched the sniffer and + * never needed to. + * + * `a colliding refusal propagates` is the property the card asks for, over all + * THREE colliding limbs. Bare, each of these is an empty chart; enveloped, each + * reaches the caller. It uses the package's own constructor rather than a + * synthesised envelope, because the claim under test is about the refusals THIS + * PACKAGE raises. + * + * `#5033's leniency is untouched` is the negative control, and it must stay + * byte-identical: a genuine absent source table still degrades to the empty + * result with its `warn`, and an ABSENT security service still runs unscoped. + * That is the deliberate behaviour the file's own docblock exists to protect, + * and ⛔ nothing here may regress it. + * + * ## Reverse verification — direction predicted BEFORE running + * + * Ordinary direction (red), and SPLIT, because the two blocks fail for + * different reasons: + * + * - Revert both producers to `throw new Error(…)`: every row of + * `the producers declare it` that reads `code`/`status` goes RED on + * `undefined`, and every row of `a colliding refusal propagates` goes RED + * by returning the empty result instead of throwing. + * - The `#5033` block stays GREEN in both states — which is what "the + * leniency is untouched" means as evidence rather than as a claim. + * + * ⛔ Note which rows do NOT move: the real producers' MESSAGES are asserted in + * `the producers declare it` and in `read-scope-bridge-resolution.test.ts`, and + * they stay green under the ablation — the wording did not change, and that + * asymmetry (envelope red, wording green) is the whole finding. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { readScopeUnresolvedError } from '../read-scope-refusal.js'; + +/** The ADR-0112 fields the REST boundary classifies on. */ +interface Refusal extends Error { + code?: unknown; + status?: unknown; +} + +const EMPTY = { rows: [], fields: [], totals: [] }; + +const dataset = DatasetSchema.parse({ + name: 'sales', + label: 'Sales', + object: 'opportunity', + dimensions: [{ name: 'stage', field: 'stage', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], +}); + +const SELECTION = { dimensions: ['stage'], measures: ['revenue'] }; +const CTX = { tenantId: 'org_A', userId: 'u_seeker' } as ExecutionContext; + +function logger() { + return { info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn(), child: vi.fn() } as any; +} + +async function refusalFrom(thunk: () => unknown | Promise): Promise { + try { + await thunk(); + return undefined; + } catch (e) { + return e as Refusal; + } +} + +/** A service whose EXECUTION throws `thrown` — the way into `queryDataset`'s catch. */ +function serviceThatThrows(thrown: unknown, log = logger()) { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => { throw thrown; }, + isRegisteredObject: () => true, + logger: log, + }); +} + +/** + * A service whose ROW-SCOPE PROVIDER throws — the real seam, reached through + * `resolveReadScopes` from inside `queryDataset`'s try. Execution itself is + * healthy, so a result coming back at all means the fail-closed pre-pass was + * bypassed or its refusal was swallowed. + */ +function serviceWhoseScopeProviderThrows(cause: unknown, log = logger()) { + return new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [{ stage: 'won', revenue: 42 }], + isRegisteredObject: () => true, + getReadScope: () => { throw cause; }, + logger: log, + }); +} + +/** + * The three limbs of `isMissingSourceError` a registry or security refusal + * naturally reaches for — the card's own list, spelled here as the CALLER-side + * inputs of the property rather than as a claim about the predicate (the + * predicate itself is asserted in `refusal-wording-collision.test.ts`, which + * imports the real function). + * + * ⚠️ Each names the dataset's OWN object on purpose. A bare colliding refusal + * lands in one of #5033's TWO arms depending on which relation + * `missingSourceRelation` reads out of it: name the dataset's own object and it + * degrades to the empty chart (the arm this card is about); name anything else + * — including a stray word the extractor mistakes for a table, measured: "… + * unknown object in the resolved scope" yields `in` — and it is re-reported as + * a cross-datasource topology error, loud but describing a JOIN that does not + * exist. Both arms are wrong for a security refusal; the silent one is the one + * under test here, so these fixtures aim at it deliberately rather than by + * luck. + */ +const COLLIDING_WORDINGS = [ + '[Analytics] row-level read scope could not be resolved for "opportunity"; the policy names object "opportunity" is not registered with the security service.', + '[Analytics] row-level read scope could not be resolved; the resolved scope names unknown object: opportunity.', + '[Analytics] read-scope resolution failed: "opportunity" is not a registered object on the security service.', +]; + +describe('[#17130] the row-scope resolution refusals declare an ADR-0112 envelope', () => { + it('the constructor stamps the code the lowering stage already owns', () => { + const err = readScopeUnresolvedError('[Analytics] read-scope resolution failed for "x"; query denied (fail-closed).') as Refusal; + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err.status).toBe(500); + // ⛔ The message is the site's, untouched — #17130 fixes the declaration. + expect(err.message).toBe('[Analytics] read-scope resolution failed for "x"; query denied (fail-closed).'); + }); + + it('resolveReadScopes denies with the envelope, and the wording is unchanged', async () => { + const log = logger(); + const err = await refusalFrom(() => + serviceWhoseScopeProviderThrows(new Error('security service exploded'), log) + .queryDataset(dataset, SELECTION, CTX), + ); + expect(err, 'a fail-closed row-scope denial was swallowed').toBeInstanceOf(Error); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err?.status).toBe(500); + expect(String(err?.message)).toMatch(/read-scope resolution failed for "opportunity"; query denied \(fail-closed\)/); + // Refused ⇒ never degraded, so no "empty result" warn was emitted either. + expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result')); + // …and the operator still gets the cause at `error`, as before. + expect(log.error).toHaveBeenCalledWith( + expect.stringContaining('read-scope resolution failed for object "opportunity"'), + expect.any(Error), + ); + }); + + it('the bridge refusal reaching the pre-pass keeps a declared envelope end to end', async () => { + // The bridge (`plugin.ts`) throws its own enveloped refusal; the pre-pass + // catches it and answers with its own. Both stages declared ⇒ whichever one + // reaches `queryDataset` is re-thrown by declaration, never sniffed. + const bridgeRefusal = readScopeUnresolvedError( + '[Analytics] row-level read scope could not be resolved for "opportunity"; query refused (fail-closed).', + ); + const err = await refusalFrom(() => + serviceWhoseScopeProviderThrows(bridgeRefusal).queryDataset(dataset, SELECTION, CTX), + ); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err?.status).toBe(500); + }); +}); + +describe('[#17130] a colliding refusal propagates instead of becoming a 200 with an empty chart', () => { + for (const wording of COLLIDING_WORDINGS) { + it(`propagates: ${wording.slice(0, 64)}…`, async () => { + const log = logger(); + const err = await refusalFrom(() => + serviceThatThrows(readScopeUnresolvedError(wording), log).queryDataset(dataset, SELECTION, CTX), + ); + expect(err, 'a fail-closed refusal was served as an empty chart').toBeInstanceOf(Error); + expect(err?.code).toBe('READ_SCOPE_COMPILE_FAILED'); + expect(err?.status).toBe(500); + expect(String(err?.message)).toBe(wording); + expect(log.warn).not.toHaveBeenCalledWith(expect.stringContaining('returning an empty result')); + }); + } + + it('…and the same three wordings BARE still degrade — which is what makes the envelope the fix', async () => { + // ⛔ Not an aspiration: this is the measurement that says the property is + // carried by the declaration and not by the phrasing. Strip the envelope + // and every one of the three is an empty chart again. That is also the + // ablation's predicted shape, asserted here so the claim is re-runnable. + for (const wording of COLLIDING_WORDINGS) { + const result = await serviceThatThrows(new Error(wording)).queryDataset(dataset, SELECTION, CTX); + expect(result, `bare wording unexpectedly propagated: ${wording}`).toEqual(EMPTY); + } + }); +}); + +describe('[#17130] #5033’s deliberate leniency is untouched — the negative control', () => { + it('a genuine absent source table still degrades to the empty result, with the warn', async () => { + const log = logger(); + const result = await serviceThatThrows( + new Error('SELECT COUNT(*) FROM "opportunity" - no such table: opportunity'), + log, + ).queryDataset(dataset, SELECTION, CTX); + expect(result).toEqual(EMPTY); + expect(log.warn).toHaveBeenCalledWith( + expect.stringContaining('backing object "opportunity" is unavailable'), + ); + }); + + it('postgres’s and mysql’s real wordings still degrade too', async () => { + expect( + await serviceThatThrows( + new Error('select "stage" from "opportunity" - relation "opportunity" does not exist'), + ).queryDataset(dataset, SELECTION, CTX), + ).toEqual(EMPTY); + expect( + await serviceThatThrows(new Error("Table 'app.opportunity' doesn't exist")).queryDataset( + dataset, + SELECTION, + CTX, + ), + ).toEqual(EMPTY); + }); + + it('an ABSENT row-scope provider still runs the query unscoped', async () => { + // The state `read-scope-bridge-resolution.test.ts` calls the negative + // control: no security service at all is a real single-tenant deployment, + // reported loudly at init, and refusing there would break it. Re-asserted + // here so this file's own change cannot narrow it. + const svc = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }), + executeRawSql: async () => [{ stage: 'won', revenue: 42 }], + isRegisteredObject: () => true, + logger: logger(), + }); + const result = await svc.queryDataset(dataset, SELECTION, CTX); + expect(result.rows).toEqual([{ stage: 'won', revenue: 42 }]); + }); +}); diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 239a1f6623..ce95c6c881 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -40,6 +40,10 @@ import { assertObjectsReadable, type ObjectReadAdmissionProvider, } from './read-admission.js'; +// [#17130] The ROW-scope half's refusal envelope — the sibling of the +// object-level `readAdmissionDeniedError` above, and the reason a fail-closed +// row-scope denial can no longer be re-judged by its wording. +import { readScopeUnresolvedError } from './read-scope-refusal.js'; // [#15768] The measure result-type rule — which aggregates return a value of // the aggregated field's own type, and which are numeric whatever they read. // Owned in its own module so the enumerated verdict per `AggregationFunction` @@ -1058,6 +1062,14 @@ export class AnalyticsService implements IAnalyticsService { * * Fail-closed: if the provider throws for an object, the whole query is * rejected rather than emitting SQL with that object unscoped. + * + * [#17130] And the rejection DECLARES itself. This throw lands inside + * {@link AnalyticsService.queryDataset}'s catch, whose first question is + * `hasDeclaredErrorEnvelope` and whose second is {@link isMissingSourceError} + * — six substrings over driver phrasing, three of which are what a registry + * or security refusal naturally says. Bare, this refusal reached the caller + * as a confident empty chart the day its wording drifted into one of them; + * enveloped, it is re-thrown before the wording is ever read. */ private async resolveReadScopes( query: AnalyticsQuery, @@ -1078,7 +1090,10 @@ export class AnalyticsService implements IAnalyticsService { `rejecting query (fail-closed, ADR-0021 D-C)`, e instanceof Error ? e : new Error(String(e)), ); - throw new Error( + // ⛔ The message is unchanged, deliberately: #17130's fix is the + // DECLARATION, not a luckier string. Rewording to dodge the sniffer + // would leave the next author to rediscover the mine. + throw readScopeUnresolvedError( `[Analytics] read-scope resolution failed for "${object}"; query denied (fail-closed).`, ); } diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 047f8a5795..391f1f1662 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -11,6 +11,7 @@ import type { AnalyticsServiceConfig } from './analytics-service.js'; import type { AnalyticsDriverCapabilities } from './strategies/types.js'; import { pickDisplayField, type DimensionLabelDeps } from './dimension-labels.js'; import { assertReadScopeCannotVacate } from './read-scope-sql.js'; +import { readScopeUnresolvedError } from './read-scope-refusal.js'; /** * The slice of the DECLARED engine contracts this plugin's auto-bridges @@ -507,9 +508,19 @@ export class AnalyticsServicePlugin implements Plugin { * ⛔ 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. + * has since ADR-0021 D-C), so serving nothing is the same outcome the + * object-level bridge produces. + * + * [#17130] It needs no NEW error code — and the clause that used to follow, + * "and no new envelope here", was the finding. An envelope is not a second + * outcome, it is what stops the outcome being decided by wording: + * `queryDataset`'s catch re-throws whatever declares `code` + `status` and + * sends everything else to a six-substring sniff over driver phrasing, + * three limbs of which (`not registered`, `unknown object`, + * `is not a registered object`) are what a security refusal says. This + * refusal propagated only because its text happened to miss all six. + * {@link readScopeUnresolvedError} carries the code the sibling lowering + * stage already owns, so the coincidence is gone without a ledger row. */ type SecurityReadFilterResolution = | { kind: 'usable'; svc: SecurityReadFilter } @@ -572,7 +583,15 @@ export class AnalyticsServicePlugin implements Plugin { '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( + // [#17130] Declared, not bare. `resolveReadScopes` replaces this + // error with its own on the dataset path, but this provider is read + // by four consumers and a bare refusal is the one kind + // `queryDataset`'s catch classifies by WORDING — three of the six + // substrings it matches on (`not registered`, `unknown object`, + // `is not a registered object`) are exactly what a security refusal + // reaches for. ⛔ The message is unchanged: the fix is the + // declaration, never a luckier string. + throw readScopeUnresolvedError( `[Analytics] row-level read scope could not be resolved for "${object}"; ` + 'query refused (fail-closed).', ); diff --git a/packages/services/service-analytics/src/read-scope-refusal.ts b/packages/services/service-analytics/src/read-scope-refusal.ts new file mode 100644 index 0000000000..04b52028ca --- /dev/null +++ b/packages/services/service-analytics/src/read-scope-refusal.ts @@ -0,0 +1,109 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17130] The row-scope fail-closed refusals, in the ADR-0112 envelope. + * + * ## What was wrong + * + * The row-level read scope is established in two stages, and only the second + * one declared itself: + * + * 1. **resolution** — ask the wired provider which rows this caller may see + * (`plugin.ts`'s `security` bridge, then `AnalyticsService.resolveReadScopes`). + * Both stages refuse fail-closed when the provider cannot answer, and both + * refused with a bare `throw new Error(…)`. + * 2. **lowering** — compile the `FilterCondition` that came back into SQL + * (`read-scope-sql.ts`), which has answered `READ_SCOPE_COMPILE_FAILED` / + * 500 through its own module-local constructor since #5367. + * + * A bare refusal is the one kind `queryDataset`'s catch classifies by WORDING: + * `hasDeclaredErrorEnvelope` re-throws anything its producer classified, and + * only what nobody classified reaches {@link isMissingSourceError} — six + * substrings, three of which (`not registered`, `unknown object`, + * `is not a registered object`) are exactly what a registry or security + * refusal reaches for. A hit there is not a wrong status code, it is + * `{rows: [], fields: [], totals: []}` — a fail-closed security gate served to + * the caller as a confident empty chart, with one `warn` and no exception. + * + * The two messages happen to match none of the six today. ⛔ That is a + * coincidence, not a construction, and #17130 exists to remove it rather than + * to keep picking lucky strings. Declaring the envelope answers the + * classification question at the producer, where it is known, so no reword of + * these messages — and no message the resolution stage grows later — can ever + * reach the sniffer again. + * + * ## Why `READ_SCOPE_COMPILE_FAILED` and not a new code + * + * The condition is the one `read-scope-sql.ts`'s ten refusals already carry: + * **the row-level read scope could not be established, so the query is refused + * fail-closed, and neither input is the caller's.** Resolution and lowering are + * two stages of one pipeline with one outcome; giving them two wire spellings + * would be the defect ADR-0112 exists to remove — the same argument + * `dataset-refusal.ts` records for putting member-level refusals on the + * neighbouring gates' `INVALID_FIELD` rather than minting a second code. + * + * The 500 is what #5367's maintainer ruling (2026-08-06) settled for this + * family, and both halves of that ruling apply here verbatim: + * + * - **Attribution.** The inputs are a `security` service the deployment wired + * and a provider contract it failed to honour. A 4xx would tell the caller + * to fix a request that was never the problem, and hide the fault from the + * 5xx alerting that should see it. + * - **Disclosure.** The route withholds the message of any producer that + * declares a server fault, so `read-scope resolution failed for "x"` stops + * at the operator's log instead of telling a tenant that this deployment's + * security service is broken. ⛔ Note the direction: declaring the envelope + * REMOVES disclosure that a bare 500 leaks today. Nothing here makes any + * refusal likelier to degrade — every path this changes moves from + * "classified by its words" to "classified by its declaration". + * + * So no ledger row is added: `READ_SCOPE_COMPILE_FAILED` is already registered + * to `@objectstack/service-analytics` in `error-code-ledger.zod.ts` + * (ADR-0112 D3), which is what keeps this a Clause-② `no`. + * + * ## Why a second constructor rather than importing `readScopeCompileError` + * + * That one is deliberately module-local — *"the only way this module refuses"* + * — and its messages are `[read-scope-sql]`-prefixed lowering diagnostics. This + * file is the resolution stage's counterpart, and it is a MODULE rather than a + * private helper for one reason: the stage spans two files (`plugin.ts` raises + * the bridge's refusal, `analytics-service.ts` the pre-pass's), and two + * spellings of one envelope is how a half-enveloped surface starts — the lesson + * #5352 paid for when seven of `filter-normalizer.ts`'s nine sites stayed bare. + * + * ⛔ One code, two constructors, one condition. A third refusal in this stage + * belongs here too, not in a new spelling of the same thing. + */ + +import type { RegisteredErrorCode } from '@objectstack/spec/api'; + +/** + * `READ_SCOPE_COMPILE_FAILED`, pinned against the ADR-0112 D3 ledger. + * + * Typed as `RegisteredErrorCode` so dropping the ledger row (or misspelling the + * code here) fails `tsc` rather than shipping a code `ApiErrorSchema` rejects — + * the same load-bearing annotation `read-scope-sql.ts` and `dataset-refusal.ts` + * carry. + */ +const READ_SCOPE_COMPILE_FAILED: RegisteredErrorCode = 'READ_SCOPE_COMPILE_FAILED'; + +/** + * A row-scope RESOLUTION failure in the ADR-0112 envelope — + * `READ_SCOPE_COMPILE_FAILED` / 500. + * + * Use it where a wired row-scope provider could not be asked or could not + * answer, and the query is therefore refused fail-closed. ⛔ Not for a caller's + * mistake (that is `dataset-refusal.ts`) and ⛔ not for an ABSENT provider, + * which is a different state entirely: a deployment with no security service is + * one where `/data` has no row-level policy either, it is reported loudly at + * init, and it must keep running unscoped exactly as before. + * + * The message stays whatever the refusing site says — it is for the operator's + * log, which after this change is its only destination. + */ +export function readScopeUnresolvedError(message: string): Error { + const err = new Error(message) as Error & { code?: string; status?: number }; + err.code = READ_SCOPE_COMPILE_FAILED; + err.status = 500; + return err; +} From d16c0474fee18a3179fbced581575cc30a49aed4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 05:11:33 +0000 Subject: [PATCH 3/3] chore(changeset): row-scope refusals declare READ_SCOPE_COMPILE_FAILED / 500 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../analytics-row-scope-refusal-envelope.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .changeset/analytics-row-scope-refusal-envelope.md diff --git a/.changeset/analytics-row-scope-refusal-envelope.md b/.changeset/analytics-row-scope-refusal-envelope.md new file mode 100644 index 0000000000..0a5439bf46 --- /dev/null +++ b/.changeset/analytics-row-scope-refusal-envelope.md @@ -0,0 +1,16 @@ +--- +"@objectstack/service-analytics": patch +--- + +fix(service-analytics): a fail-closed row-scope refusal can no longer be served as an empty chart (#17130) + +`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. + +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. + +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: + +- the refusal reaches the caller as a declared `500` instead of relying on its phrasing to escape the degradation path; +- 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. + +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.