From 2c0e8d07795cf7036ed4b8a23b5c830a21138348 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 09:23:02 +0000 Subject: [PATCH 1/8] wip(analytics): object-level read admission at the analytics door Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../plugin-security/src/security-plugin.ts | 123 +++++++++++++- .../src/analytics-service.ts | 127 ++++++++++++-- .../services/service-analytics/src/plugin.ts | 91 ++++++++++ .../service-analytics/src/read-admission.ts | 156 ++++++++++++++++++ .../spec/src/contracts/security-service.ts | 46 ++++++ 5 files changed, 528 insertions(+), 15 deletions(-) create mode 100644 packages/services/service-analytics/src/read-admission.ts diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 81b0e94045..900a79f949 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -1493,6 +1493,13 @@ export class SecurityPlugin implements Plugin { // silently degrading a consumer's feature detection at runtime. const securityService: ISecurityService = { getReadFilter: (object: string, context?: any) => this.getReadFilter(object, context), + // The OBJECT-level half of the same read. `getReadFilter` answers + // "which rows" and answers `undefined` for a caller with NO grant at + // all, so a door holding only the filter cannot tell "unrestricted" + // from "not permitted" — which is how the analytics raw-SQL path served + // a row count for an object whose `/data` door answers 403. Exposed + // here so every door that bypasses the middleware asks BOTH halves. + canReadObject: (object: string, context?: any) => this.canReadObject(object, context), // [#3547] Readable-field projection for a context — the authoritative // column set for a read-derived export (`export ⊆ list`, #3391). // Same field mask as the read middleware (no drift). The REST export @@ -1652,7 +1659,7 @@ export class SecurityPlugin implements Plugin { discardPermissionSetOverlay(overlayDiscardDeps, callerContext, id), }); ctx.registerService('security', registeredSecurityService); - ctx.logger.info('[security] registered "security" service (getReadFilter, getReadableFields, getMetadataReadableFields, canExport, checkAuthoredRowWrite, resolvePermissionSetNames, resolvePermissionSetsForContext, explain, audience-binding suggestions, discardPermissionSetOverlay) — ADR-0021 D-C / ADR-0090 D5/D6/D9 / ADR-0094 / ADR-0106 D7 / #3544 / #3547 / #5493 / #7616'); + ctx.logger.info('[security] registered "security" service (getReadFilter, canReadObject, getReadableFields, getMetadataReadableFields, canExport, checkAuthoredRowWrite, resolvePermissionSetNames, resolvePermissionSetsForContext, explain, audience-binding suggestions, discardPermissionSetOverlay) — ADR-0021 D-C / ADR-0090 D5/D6/D9 / ADR-0094 / ADR-0106 D7 / #3544 / #3547 / #5493 / #7616'); } catch (e) { ctx.logger.warn?.('[security] failed to register "security" service', { error: (e as Error).message, @@ -4645,6 +4652,120 @@ export class SecurityPlugin implements Plugin { return allFields.filter((f) => fieldPerms[f]?.readable !== false || partialRules[f] !== undefined); } + /** + * Whether `context` may READ `object` at all — the OBJECT-level admission, + * exposed for the read doors that bypass the engine middleware. + * + * The middleware answers this before it composes any row filter; a door that + * compiles its own statement (the analytics native-SQL strategy is the one in + * the tree) never reaches the middleware and so never asked. `getReadFilter` + * is not a substitute: it answers "which ROWS", and its `undefined` means "no + * row restriction" — the same answer a caller with NO grant on the object + * gets. So a door holding only the filter reads an ungranted principal as an + * unrestricted one, and answers `200 {"rows":[{"cnt":24}]}` where + * `GET /data/` answers `403 PERMISSION_DENIED` for the same principal + * on the same deployment. + * + * ## The arms, in the middleware's own order + * + * Every one of them is the SAME primitive the middleware calls, not a second + * reading of the same declaration — which is what makes "the two doors reach + * one verdict" a property of the code rather than a promise: + * + * 1. `isSystem` → admit (the middleware's total bypass); + * 2. no permission sets resolved → admit (the middleware guards its whole + * CRUD gate with `if (permissionSets.length > 0)`; reporting a denial the + * data path would not enforce is its own kind of drift); + * 3. `secMeta.unresolved` → DENY (#3545 — `isPrivate` would default to + * `false`, which is exactly what lets a plain `'*'` wildcard reach an + * object ADR-0066 D2 says it must not); + * 4. ADR-0066 D3/⑤ `requiredPermissions` capability AND-gate for the read + * CRUD class, checked BEFORE the grant, for the caller AND (D10) the + * delegator; + * 5. the `allowRead` CRUD grant ({@link PermissionEvaluator.checkObjectPermission} + * on `find`); + * 6. ADR-0090 D10 — the delegator must independently hold the same grant; + * a dangling delegator denies. + * + * `find` is the operation asked for, not `aggregate`, and the two are the same + * question: `OPERATION_PERMISSION_MAP` maps `find`, `findOne`, `count` and + * `aggregate` all onto `allowRead`. Asking `find` keeps the answer readable as + * "may this caller read this object", which is what every consuming door needs. + * + * Fails CLOSED (an access-narrowing answer): a throw anywhere inside denies, + * and callers must treat a throw as a denial too. + * + * ⛔ Object-level ONLY. `true` never means "unrestricted" — the row scope is + * still {@link getReadFilter}'s and it is still mandatory. Nothing here may be + * used to widen. + */ + async canReadObject(object: string, context?: any): Promise { + const objectName = String(object ?? ''); + if (!objectName) return false; + // 1. System operations bypass (mirrors the middleware's isSystem skip). + if (context?.isSystem) return true; + + try { + const permissionSets = await this.resolvePermissionSetsForContext(context); + // 2. No sets resolved (unauthenticated, or a deployment with no sets) → + // no permission-set restriction applies, exactly as the middleware + // treats it. + if (permissionSets.length === 0) return true; + + const { isPrivate, unresolved, requiredPermissions } = + await this.getObjectSecurityMeta(objectName); + // 3. [#3545] Posture unresolvable → deny. + if (unresolved) return false; + + // [ADR-0090 D10] Resolve the delegator ONCE — arms 4 and 6 both need it, + // and a dangling link denies before either runs. + let delegatorSets: PermissionSet[] | null = null; + if (context?.onBehalfOf?.userId) { + const del = await resolveDelegatorContext(this.ql, context); + if (del.kind === 'missing') return false; + if (del.kind === 'resolved') { + delegatorSets = await this.resolvePermissionSetsForContext(del.context); + } + } + + // 4. [ADR-0066 D3/⑤] The capability AND-gate, ahead of the grant, for both + // principals — a caller missing any required capability is denied + // however permissive their grants are. + const required = requiredCapsForOperation(requiredPermissions, 'find'); + if (required.length > 0) { + const held = this.permissionEvaluator.getSystemPermissions(permissionSets); + if (required.some((cap) => !held.has(cap))) return false; + if (delegatorSets && delegatorSets.length > 0) { + const delHeld = this.permissionEvaluator.getSystemPermissions(delegatorSets); + if (required.some((cap) => !delHeld.has(cap))) return false; + } + } + + // 5. The object-level CRUD grant. + if (!this.permissionEvaluator.checkObjectPermission('find', objectName, permissionSets, { isPrivate })) { + return false; + } + + // 6. [ADR-0090 D10] The delegator must independently grant the same read. + if ( + delegatorSets && + delegatorSets.length > 0 && + !this.permissionEvaluator.checkObjectPermission('find', objectName, delegatorSets, { isPrivate }) + ) { + return false; + } + + return true; + } catch (e) { + this.logger.error?.( + `[security] canReadObject could not resolve the object-level read admission for ` + + `'${objectName}' (user ${context?.userId ?? 'unknown'}) — denying (fail-closed)`, + e instanceof Error ? e : new Error(String(e)), + ); + return false; + } + } + /** * [#3544] Whether `context` may EXPORT `object` — the user-level export axis. * diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index a51e7f0fb4..f40f1fdd94 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -33,6 +33,13 @@ import { // docblock for why the edge is acyclic and why it was worth adding. import { matchMissingColumnOfRelation } from '@objectstack/types'; import { CubeRegistry } from './cube-registry.js'; +// The object-level read admission asked at this door, ahead of every strategy +// — the layer the raw-SQL path could not inherit from the engine. See that +// module's header for the request that reached the database without it. +import { + assertObjectsReadable, + type ObjectReadAdmissionProvider, +} from './read-admission.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` @@ -471,6 +478,26 @@ export interface AnalyticsServiceConfig { | null | undefined | Promise; + /** + * The OBJECT-LEVEL read admission — "may this caller read this object AT + * ALL", asked once at the door for the base object and every joined object, + * BEFORE a strategy is selected. + * + * The sibling of {@link AnalyticsServiceConfig.getReadScope} and NOT a + * substitute for it: the scope answers WHICH ROWS and answers `undefined` + * for a caller with no grant at all, so a door holding only the scope cannot + * tell "unrestricted" from "not permitted". The plugin auto-bridges this to + * the `security` service's `canReadObject` (falling back to `explain`, whose + * `allowed` is the same bottom line), so the verdict is the one the engine + * middleware reaches on `GET /data/` — the two doors agree by + * construction rather than by maintenance. + * + * MAY be async. `false` refuses the query with `PERMISSION_DENIED` / 403; a + * THROW also refuses (fail-closed). When the hook is absent entirely no + * object-level gate applies — the deployment has no security service, which + * is the same deployment in which `/data` has no object-level gate either. + */ + admitObjectRead?: ObjectReadAdmissionProvider; /** * ADR-0021 D-C — join allowlist per cube (the dataset's declared `include`). * Joins outside this set are rejected by the strategy. Compiled datasets @@ -715,6 +742,8 @@ export class AnalyticsService implements IAnalyticsService { private readonly baseCtx: DatasetScopedStrategyContext; /** Context-aware read-scope provider (bound to the request's context per call). */ private readonly readScopeProvider?: AnalyticsServiceConfig['getReadScope']; + /** Object-level read-admission provider (bound per call to the request context). */ + private readonly readAdmissionProvider?: ObjectReadAdmissionProvider; /** Compiled datasets by name — feeds the join allowlist (D-C) and queryDataset. */ private readonly datasetRegistry = new Map(); /** Optional object-graph resolver used when compiling datasets. */ @@ -755,6 +784,7 @@ export class AnalyticsService implements IAnalyticsService { } this.readScopeProvider = config.getReadScope; + this.readAdmissionProvider = config.admitObjectRead; this.relationshipResolver = config.relationshipResolver; this.sourceFieldMeta = config.sourceFieldMeta; this.labelResolver = config.labelResolver; @@ -860,6 +890,15 @@ export class AnalyticsService implements IAnalyticsService { // filters. Resolve the channel per request, with the SAME instant as the // query's own fields. const getDatasetScope = this.resolvedDatasetScopeGetter(tokenCtx); + // The OBJECT-LEVEL gate, ahead of everything else on this path — including + // the early return below, which is why it is not folded into the + // read-scope pre-pass: a deployment that wired an admission provider and no + // scope provider must still be gated, and the two questions have different + // answers for the same caller ("no rows visible" vs "not permitted to + // read"). `callCtx` is the ONE thing `query()` and `generateSql()` share, + // so gating it covers the direct `/analytics/query` door, the `/analytics/sql` + // echo door and — through `DatasetExecutor` — every dataset door. + await this.assertReadAdmitted(this.queryObjects(query), context); // #3602 — `context` rides along unconditionally. It is the ENGINE-side belt // (forwarded to `engine.aggregate`, where the middleware chain applies its // own RLS), so it must not be gated on the analytics-side belt being wired: @@ -932,6 +971,72 @@ export class AnalyticsService implements IAnalyticsService { }; } + /** + * Every object this query will READ — the cube's base object plus every + * joined object. + * + * ONE derivation, two consumers: {@link resolveReadScopes} scopes exactly + * this set and {@link assertReadAdmitted} admits exactly this set, so the set + * that is row-scoped and the set that is admitted are provably the same set + * rather than two lists that agree today. It is a SUPERSET of what a strategy + * actually scans (a strategy only joins along declared relationships), which + * is the safe direction: no scanned object is ever left ungated. + * + * An unregistered cube yields the empty set — the query fails its own + * cube-existence gate downstream, and inventing an object name here would + * gate something the request never named. + */ + private queryObjects(query: AnalyticsQuery): Set { + if (!query.cube) return new Set(); + const cube = this.cubeRegistry.get(query.cube); + return cube ? this.cubeObjects(cube) : new Set(); + } + + /** + * {@link queryObjects} for a cube already in hand — the draft-preview branch + * holds the COMPILED dataset rather than a query naming it, and reaching for + * the registry there would make the gate depend on a registration side + * effect. One derivation, two entry points. + */ + private cubeObjects(cube: Cube): Set { + const objects = new Set(); + if (typeof cube.sql === 'string' && cube.sql.trim()) { + objects.add(cube.sql.trim()); + } + const joins = (cube as { joins?: Record }).joins; + if (joins) { + for (const [alias, j] of Object.entries(joins)) { + objects.add(j?.name ?? alias); + } + } + return objects; + } + + /** + * The OBJECT-LEVEL read gate, asked at this door for every object the query + * will read, BEFORE a strategy is selected. + * + * Placement is the whole point. The `NativeSQLStrategy` compiles a statement + * and runs it through the driver's raw `execute()`, which no middleware sits + * in front of, so it could never inherit the admission the ObjectQL path gets + * from `engine.aggregate`. Asking HERE — once, ahead of the chain — makes the + * two strategies give the SAME verdict by construction instead of by each + * carrying its own copy of the check, which is the arrangement that produced + * the divergence in the first place. + * + * A no-op when no provider is wired: that is a deployment with no security + * service, where `/data` has no object-level gate either, so the doors still + * agree. `AnalyticsServicePlugin` reports that state at init. + */ + private async assertReadAdmitted( + objects: Iterable, + context: ExecutionContext | undefined, + ): Promise { + const provider = this.readAdmissionProvider; + if (!provider) return; + await assertObjectsReadable(objects, provider, context, this.logger); + } + /** * Resolve the read scope (tenant + RLS `FilterCondition`) for the base object * AND every joined object of the query's cube, keyed by object name. This is @@ -952,21 +1057,8 @@ export class AnalyticsService implements IAnalyticsService { const map = new Map(); const provider = this.readScopeProvider; if (!provider || !query.cube) return map; - const cube = this.cubeRegistry.get(query.cube); - if (!cube) return map; - - const objects = new Set(); - if (typeof cube.sql === 'string' && cube.sql.trim()) { - objects.add(cube.sql.trim()); - } - const joins = (cube as { joins?: Record }).joins; - if (joins) { - for (const [alias, j] of Object.entries(joins)) { - objects.add(j?.name ?? alias); - } - } - for (const object of objects) { + for (const object of this.queryObjects(query)) { let filter: FilterCondition | null | undefined; try { filter = await provider(object, context); @@ -1120,6 +1212,13 @@ export class AnalyticsService implements IAnalyticsService { this.logger.warn(`[Analytics] draft preview resolver failed for "${dataset.object}" — falling back to live data: ${String((e as Error)?.message ?? e)}`); } if (seedRows) { + // The draft-preview branch evaluates in memory and never reaches + // `query()`, so it is the ONE dataset path that does not inherit + // `callCtx`'s gate. Same gate, same helper, asked here rather than a + // second implementation — drafted seed rows are still this object's + // rows, and a caller who may not read the object may not read its + // pending seed either. + await this.assertReadAdmitted(this.cubeObjects(compiled.cube), context); this.logger.debug(`[Analytics] queryDataset "${dataset.name}" → preview over ${seedRows.length} drafted seed row(s)`); const previewService = { query: async (q: AnalyticsQuery) => evaluateAnalyticsQueryOverRows(q, compiled.cube, seedRows!), diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 8074e13470..4ba7e31b02 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -190,6 +190,21 @@ export interface AnalyticsServicePluginOptions { | null | undefined | Promise; + /** + * The OBJECT-LEVEL read admission — "may this caller read this object at + * all". The sibling of {@link AnalyticsServicePluginOptions.getReadScope} and + * NOT a substitute for it: the scope answers WHICH ROWS and answers + * `undefined` for a caller with no grant at all, so a door holding only the + * scope cannot tell "unrestricted" from "not permitted" — which is how the + * raw-SQL path served a row count for an object whose `/data` door answers + * 403. When omitted, the plugin auto-bridges to a registered `'security'` + * service, preferring `canReadObject(object, context)` and falling back to + * `explain({ object, operation: 'read' }, context).allowed`. + */ + admitObjectRead?: ( + objectName: string, + context?: ExecutionContext, + ) => boolean | Promise; /** * ADR-0021 D-C — join allowlist per cube (the dataset's declared `include`). * Typically wired from the dataset registry's compiled `allowedRelationships`. @@ -492,6 +507,65 @@ export class AnalyticsServicePlugin implements Plugin { autoBridgedReadScope = true; } + // The OBJECT-LEVEL half of the same read, bridged the same way and for the + // same reason the scope is: analytics stays decoupled from security, and + // resolution happens at CALL time so plugin-registration order does not + // decide whether the gate exists. + // + // ## Why there are two spellings and neither of them is "admit" + // + // `canReadObject` is the direct answer and the one this repo's + // `plugin-security` serves. A security service that predates it is still a + // conforming `ISecurityService`, and the fallback for such a service is NOT + // to admit — falling open on absence is exactly the defect this gate + // closes. It is `explain`, which is NOT optional on that contract and whose + // `allowed` is the same bottom line ("would the middleware allow this + // operation?") computed by the same enforcement walk. `explain` is the + // heavier call, which is why it is the fallback and not the primary; it + // never fires against an in-repo stack. + // + // A deployment with NO security service at all gets no gate — and no + // object-level gate on `/data` either, since that gate IS this plugin's + // absent middleware — so the two doors still agree. That state is reported + // at init below. + interface SecurityReadAdmission { + canReadObject?(object: string, context?: ExecutionContext): boolean | Promise; + explain?( + request: { object: string; operation: string }, + callerContext?: ExecutionContext, + ): Promise<{ allowed?: boolean }>; + } + let admitObjectRead = this.options.admitObjectRead; + let autoBridgedReadAdmission = false; + if (!admitObjectRead) { + const trySecurityAdmission = (): SecurityReadAdmission | undefined => { + try { + const svc = ctx.getService('security'); + if (!svc) return undefined; + return typeof svc.canReadObject === 'function' || typeof svc.explain === 'function' + ? svc + : undefined; + } catch { + return undefined; + } + }; + admitObjectRead = async (object, context) => { + const svc = trySecurityAdmission(); + // No security service resolved at call time → no object-level gate on + // this deployment, which is the state reported at init. + if (!svc) return true; + if (typeof svc.canReadObject === 'function') { + return await svc.canReadObject(object, context); + } + const decision = await svc.explain!({ object, operation: 'read' }, context); + // A conforming `explain` always answers `allowed`; a shape that does + // not is a broken provider, and the fail-closed reading is the only + // safe one here. + return decision?.allowed === true; + }; + autoBridgedReadAdmission = true; + } + // ADR-0021 — relationship → target-object resolver. A dataset's `include` // names lookup/master_detail FIELDS on the base object; the joined TABLE is // each field's `reference` target (which can differ from the field name, @@ -715,6 +789,7 @@ export class AnalyticsServicePlugin implements Plugin { executeAggregate, fallbackService, getReadScope, + admitObjectRead, getAllowedRelationships: this.options.getAllowedRelationships, coerceTemporalFilterValue, coerceTemporalFilterColumn, @@ -807,6 +882,22 @@ export class AnalyticsServicePlugin implements Plugin { ); } + if (autoBridgedReadAdmission && securityPresentAtInit) { + ctx.logger.info( + '[Analytics] Auto-bridged admitObjectRead → "security" service (canReadObject, ' + + 'falling back to explain) — every analytics door now asks the object-level ' + + 'read grant the engine middleware asks, ahead of the strategy chain.', + ); + } else if (autoBridgedReadAdmission) { + ctx.logger.warn( + '[Analytics] No admitObjectRead configured and no "security" service registered at init — ' + + 'the bridge resolves per query, but if no security service ever appears, analytics ' + + 'queries will NOT enforce the OBJECT-LEVEL read grant. On a SQL driver that means any ' + + 'authenticated caller can post an inline dataset and read counts and groupings for an ' + + 'object they hold no grant on. Supply admitObjectRead or register a security service.', + ); + } + if (autoBridged) { ctx.logger.info('[Analytics] Auto-bridged executeAggregate → "data" service (IDataEngine)'); } diff --git a/packages/services/service-analytics/src/read-admission.ts b/packages/services/service-analytics/src/read-admission.ts new file mode 100644 index 0000000000..7f78500da4 --- /dev/null +++ b/packages/services/service-analytics/src/read-admission.ts @@ -0,0 +1,156 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The OBJECT-LEVEL read admission this service asks BEFORE it selects a + * strategy — the layer the raw-SQL path had no way to inherit. + * + * ## What was wrong, in one request + * + * `POST /api/v1/analytics/dataset/query` accepts an INLINE dataset definition + * (`body.dataset`) from any authenticated caller and compiles it to a + * statement. On a SQL driver `NativeSQLStrategy` served that statement through + * the driver's raw `execute()`, which is documented as a tenant-isolation + * bypass ("Unlike `find`/`update`/`delete` etc., raw `execute()` does NOT + * inject the `organization_id` predicate", `sql-driver.ts`) and which no + * middleware sits in front of. So the request reached the database having + * passed exactly ONE of the three read layers — the row scope, threaded since + * ADR-0021 D-C through `getReadScope`. A job seeker with NO grant of any kind + * on `ats_employer_member` was answered `200 {"rows":[{"cnt":24}]}` where + * `GET /api/v1/data/ats_employer_member` answered `403 PERMISSION_DENIED`, on + * the same deployment, for the same principal. The memory driver refused the + * identical request, because there the query falls through to the ObjectQL + * engine and the engine applies all three layers in one place. + * + * The exposure is not opt-in and an application cannot decline it: a + * deployment with 0 datasets and 0 dashboards has the identical surface, + * because the reachable slot is the INLINE definition rather than a declared + * one. + * + * ## Why the fix is one gate at the door, not a layer per strategy + * + * Two strategies each enforcing their own copy of three layers is the CAUSE of + * this defect, not its remedy — `driver-memory` is correct today precisely + * because it hands the request to the engine and the engine applies the layers + * once. So the admission question is asked HERE, at the service door, over the + * same object set the read scope is resolved for, before any strategy is + * chosen. Every strategy inherits the verdict by construction, and a strategy + * added tomorrow inherits it without knowing this module exists. + * + * ## Why the row filter could not answer it + * + * `security.getReadFilter` answers "WHICH ROWS", and it answers `undefined` — + * "no row restriction" — for a caller who may not read the object at all. A + * door holding only the filter therefore reads a caller with NO grant as a + * caller with NO restriction. That inversion is the whole defect, which is why + * the admission verdict is a SEPARATE question + * (`ISecurityService.canReadObject`) and why asking it is mandatory rather + * than an optimisation of the filter path. + * + * ## Fail direction + * + * The provider is access-NARROWING, so it fails CLOSED: a provider that throws + * denies the query rather than admitting it. An ABSENT provider is a different + * state — it means no security service answered at all, which is the same + * deployment in which `/data` has no object-level gate either, so the two doors + * still agree. `AnalyticsServicePlugin` logs that state loudly at init, the + * same posture it already takes for a missing `getReadScope`. + */ + +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import type { StandardErrorCode } from '@objectstack/spec/api'; + +/** + * `PERMISSION_DENIED`, pinned against the STANDARD catalog. + * + * Typed as `StandardErrorCode` so a misspelling fails `tsc` rather than + * shipping a code `ApiErrorSchema` rejects. The same code and the same 403 the + * ObjectQL/engine path already answers for this request, so the two strategies + * are indistinguishable on the wire as well as in the verdict. + */ +const PERMISSION_DENIED: StandardErrorCode = 'PERMISSION_DENIED'; + +/** + * The refusal, in the ADR-0112 envelope — `PERMISSION_DENIED` / 403. + * + * `/analytics/dataset/query` classifies a thrown error by reading `code` plus a + * 4xx `status` (#5352), so declaring both is what makes this refusal answer 403 + * instead of falling through to `500 ANALYTICS_QUERY_FAILED`. + * + * ⛔ The message names the OBJECT and nothing else. It must not report which + * layer refused, which permission set the caller holds, or whether the object + * exists with different grants — an admission refusal that explains itself is + * an oracle over exactly the metadata the refusal exists to withhold. The + * server-side log at the producing site carries the detail. + */ +export function readAdmissionDeniedError(objectName: string): Error { + const err = new Error( + `[Analytics] Access denied: reading "${objectName}" is not permitted for this user.`, + ) as Error & { code?: string; status?: number; object?: string }; + err.code = PERMISSION_DENIED; + err.status = 403; + err.object = objectName; + return err; +} + +/** + * The object-level read admission provider the service asks. + * + * MAY be async — the production bridge resolves the verdict from the + * `security` service, which can hit the database. Returns `true` to admit and + * `false` to refuse; a throw is a refusal (fail-closed). + */ +export type ObjectReadAdmissionProvider = ( + objectName: string, + context?: ExecutionContext, +) => boolean | Promise; + +/** Log sink — the subset of `Logger` this module uses. */ +interface AdmissionLogger { + error?(message: string, error?: Error): void; + warn?(message: string): void; +} + +/** + * Refuse the query unless EVERY object it will read is admitted. + * + * `objects` is the base object plus every joined object — the same superset + * `resolveReadScopes` scopes, derived from one shared helper so the set that is + * ADMITTED and the set that is SCOPED are provably the same set. A join the + * caller may not read is refused for the same reason `$expand` is refused on + * `/data`: an expansion may reveal only rows the caller could have read + * directly (#7626). + * + * Order is the object set's iteration order and the first refusal wins; the + * refusal names that object, which the caller already named in their own + * request body. + */ +export async function assertObjectsReadable( + objects: Iterable, + provider: ObjectReadAdmissionProvider, + context: ExecutionContext | undefined, + logger?: AdmissionLogger, +): Promise { + for (const objectName of objects) { + let admitted: boolean; + try { + admitted = await provider(objectName, context); + } catch (e) { + // Fail CLOSED. A resolution failure must deny — admitting on an error is + // the shape this whole module exists to remove. + logger?.error?.( + `[Analytics] read-admission resolution failed for object "${objectName}" — ` + + `denying query (fail-closed)`, + e instanceof Error ? e : new Error(String(e)), + ); + throw readAdmissionDeniedError(objectName); + } + if (!admitted) { + logger?.warn?.( + `[Analytics] object-level read admission denied for "${objectName}" ` + + `(user ${String((context as { userId?: unknown } | undefined)?.userId ?? 'unknown')}) — ` + + `the same verdict GET /data/${objectName} reaches`, + ); + throw readAdmissionDeniedError(objectName); + } + } +} diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index 2572032211..12fb1faac0 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -380,6 +380,52 @@ export interface ISecurityService { */ canExport(object: string, context?: SecurityContext): Promise; + /** + * Whether `context` may READ `object` AT ALL — the OBJECT-level admission the + * engine middleware answers before it ever composes a row filter. + * + * **This is the object-level half of a read, and {@link getReadFilter} is the + * row-level half.** The two are not interchangeable and the name says so on + * purpose: `getReadFilter` answers "which rows", and it answers `undefined` + * — "no row restriction" — for a caller who may not read the object at all. + * A door that asks only for the filter therefore reads a caller with NO grant + * as a caller with NO restriction, which is the exact inversion that let an + * ungranted principal `COUNT(*)` an object through the analytics raw-SQL path + * while `GET /data/` answered 403 for the same principal on the same + * deployment. Any door that bypasses the engine middleware MUST ask both. + * + * The verdict is the middleware's own read gate, arm for arm and in its order: + * the `isSystem` bypass, the "no permission sets resolved" skip, the + * fail-closed refusal on an unresolvable object posture, the ADR-0066 D3 + * `requiredPermissions` capability AND-gate, the `allowRead` CRUD grant, and + * the ADR-0090 D10 delegator intersection for an on-behalf-of caller. It is + * computed from the SAME resolution the enforcement path uses — never + * re-derived from permission sets by the caller — so a door that asks reaches + * the same admission verdict `/data` reaches, by construction. + * + * It answers the object-level question ONLY. A `true` here says nothing about + * which rows the caller may see: the row scope is still + * {@link getReadFilter}'s, and it is still mandatory. Nothing here may be used + * to widen — `true` is "not refused at this layer", never "unrestricted". + * + * **Fails CLOSED.** This is an access-narrowing answer: implementations return + * `false` (and callers must treat a throw as `false`) rather than degrading to + * "allowed". A system context bypasses and returns `true`; so does a caller + * with no resolved permission sets, mirroring the middleware, whose CRUD gate + * is skipped entirely when set resolution comes back empty. + * + * **OPTIONAL, and absence is a defined state — not a bug.** A security service + * that predates this method omits it, and a consumer resolving the service as + * `Partial` (the availability rule at the top of this file) + * feature-detects (`typeof svc.canReadObject === 'function'`). ⛔ The fallback + * for an absent method is NOT "admit": the caller composes the same verdict + * from {@link explain}, which is NOT optional and whose `allowed` is the same + * bottom line ("would the middleware allow this operation?") computed by the + * same enforcement walk. Declaring it optional is what keeps a partial + * implementation legal without making its absence a hole. + */ + canReadObject?(object: string, context?: SecurityContext): Promise; + /** * [ADR-0111 D2] Whether `context` holds the super-user WRITE bypass * (`modifyAllRecords`, "Modify All Data") for `object` — the EXPLICIT bit From 5cb0f73f6746f234214b38b4b485716c19cc3943 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 09:49:20 +0000 Subject: [PATCH 2/8] wip(analytics): tests for the read-admission gate Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../src/can-read-object-admission.test.ts | 238 ++++++++++++++ ...s-inline-dataset-admission.dogfood.test.ts | 234 +++++++++++++ .../fixtures/analytics-admission-fixture.ts | 97 ++++++ .../src/__tests__/read-admission-gate.test.ts | 311 ++++++++++++++++++ packages/verify/src/harness.ts | 26 +- 5 files changed, 904 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/plugin-security/src/can-read-object-admission.test.ts create mode 100644 packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts create mode 100644 packages/qa/dogfood/test/fixtures/analytics-admission-fixture.ts create mode 100644 packages/services/service-analytics/src/__tests__/read-admission-gate.test.ts diff --git a/packages/plugins/plugin-security/src/can-read-object-admission.test.ts b/packages/plugins/plugin-security/src/can-read-object-admission.test.ts new file mode 100644 index 0000000000..288b38725b --- /dev/null +++ b/packages/plugins/plugin-security/src/can-read-object-admission.test.ts @@ -0,0 +1,238 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `ISecurityService.canReadObject` — the OBJECT-level read admission, exposed + * for the read doors that bypass the engine middleware. + * + * ## Why the method exists + * + * `getReadFilter` answers "which ROWS", and it answers `undefined` — "no row + * restriction" — for a caller who may not read the object at all. A door + * holding only the filter therefore reads a caller with NO grant as a caller + * with NO restriction. That inversion is what let the analytics raw-SQL path + * answer `200 {"rows":[{"cnt":24}]}` for an object whose `/data` door answers + * `403 PERMISSION_DENIED`, for the same principal on the same deployment. + * + * ## What these cases pin, and why the first one is the load-bearing one + * + * The value of this method is entirely in NOT DRIFTING from the middleware, so + * the first describe block does not assert an expected boolean per case at + * all: it drives the REAL registered middleware with a `find` for the same + * (object, context) and asserts the method's answer equals whether the + * middleware admitted. A future edit that changes one and not the other fails + * here regardless of which direction it moved. + * + * The remaining blocks pin the arms individually, so a failure says WHICH arm + * moved rather than only that something did. + * + * Harness: `tenant-layer0-verdict-on-operation.test.ts` — a SecurityPlugin over + * a fake ObjectQL, with the registered middleware captured so the same plugin + * instance answers both questions. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { ADMIN_FULL_ACCESS } from '@objectstack/spec/identity'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const ADMIN_SET = defaultPermissionSets.find((s) => s.name === ADMIN_FULL_ACCESS); +if (!ADMIN_SET) throw new Error(`fixture: '${ADMIN_FULL_ACCESS}' is not among the default permission sets`); + +/** + * The reported shape, in miniature: a member who holds a grant on ONE object + * and nothing at all on the other. `employer_member` is the object the probe + * counted 24 rows of while `/data` answered 403. + */ +const SEEKER_SET: PermissionSet = { + name: 'member_default', + label: 'Seeker', + objects: { employer: { allowRead: true } }, +} as unknown as PermissionSet; + +/** Holds the read grant but not the capability the object requires (ADR-0066 D3). */ +const CAPLESS_SET: PermissionSet = { + name: 'member_default', + label: 'Reader without the capability', + objects: { payroll_run: { allowRead: true } }, +} as unknown as PermissionSet; + +/** …and the same grant WITH the capability, so the D3 arm is proven both ways. */ +const CAPABLE_SET: PermissionSet = { + name: 'member_default', + label: 'Reader with the capability', + objects: { payroll_run: { allowRead: true } }, + systemPermissions: ['manage_payroll'], +} as unknown as PermissionSet; + +const schema = (name: string, extra: Record = {}) => ({ + name, + fields: { + organization_id: { type: 'text', label: 'Organization' }, + title: { type: 'text', label: 'Title' }, + }, + ...extra, +}); + +const SCHEMAS: Record> = { + employer: schema('employer'), + employer_member: schema('employer_member'), + payroll_run: schema('payroll_run', { requiredPermissions: ['manage_payroll'] }), +}; + +const SEEKER_CTX = { userId: 'u_seeker', tenantId: 'org-1', positions: [], permissions: [], posture: 'MEMBER' }; + +async function boot(sets: PermissionSet[]) { + const middlewares: Array<(opCtx: any, next: () => Promise) => Promise> = []; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: { + registerMiddleware: (mw: any) => middlewares.push(mw), + getSchema: (name: string) => SCHEMAS[name], + findOne: vi.fn(async () => null), + }, + metadata: { + get: async (_type: string, name: string) => SCHEMAS[name], + list: async () => sets, + }, + }; + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const ctx: Record = { + logger, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as any); + await plugin.start(ctx as any); + if (middlewares.length === 0) throw new Error('SecurityPlugin registered no middleware'); + return { plugin, middleware: middlewares[0] }; +} + +/** Would the ENGINE middleware admit a plain `find` here? */ +async function middlewareAdmits( + middleware: (opCtx: any, next: () => Promise) => Promise, + object: string, + context: Record, +): Promise { + const opCtx: any = { object, operation: 'find', context: { ...context }, options: {}, ast: { where: {} } }; + try { + await middleware(opCtx, async () => {}); + return true; + } catch { + return false; + } +} + +describe('canReadObject agrees with the engine middleware, case for case', () => { + // The acceptance condition of the card this closes, asserted as an + // EQUIVALENCE rather than as two independent expectations: whatever the + // middleware answers for (object, context), this method must answer too. + const CASES: Array<{ label: string; object: string; sets: PermissionSet[]; context: Record }> = [ + { label: 'no grant of any kind on the object', object: 'employer_member', sets: [SEEKER_SET], context: SEEKER_CTX }, + { label: 'an explicit read grant', object: 'employer', sets: [SEEKER_SET], context: SEEKER_CTX }, + { label: 'a superuser wildcard', object: 'employer_member', sets: [ADMIN_SET], context: { ...SEEKER_CTX, posture: 'PLATFORM_ADMIN' } }, + { label: 'a required capability the caller lacks', object: 'payroll_run', sets: [CAPLESS_SET], context: SEEKER_CTX }, + { label: 'a required capability the caller holds', object: 'payroll_run', sets: [CAPABLE_SET], context: SEEKER_CTX }, + // A principal-less context — whichever way the middleware falls (it + // short-circuits before its CRUD gate), this method must fall the same + // way. Asserted as agreement rather than as an expected boolean precisely + // because the fall direction is the middleware's to choose, not this + // method's: pinning a literal here would be a second declaration of it. + { label: 'a principal-less context', object: 'employer_member', sets: [SEEKER_SET], context: { positions: [], permissions: [] } }, + // …and the same question on an object no schema resolves — the #3545 + // fail-closed arm, again pinned as agreement. + { label: 'an object whose posture cannot be resolved', object: 'not_a_registered_object', sets: [SEEKER_SET], context: SEEKER_CTX }, + ]; + + it.each(CASES)('$label — one verdict for both doors', async ({ object, sets, context }) => { + const { plugin, middleware } = await boot(sets); + const viaMiddleware = await middlewareAdmits(middleware, object, context); + const viaService = await (plugin as unknown as { + canReadObject(o: string, c?: unknown): Promise; + }).canReadObject(object, context); + expect(viaService).toBe(viaMiddleware); + }); +}); + +describe('canReadObject — the arms, individually', () => { + it('REFUSES the object the caller holds no grant on (the reported request)', async () => { + const { plugin } = await boot([SEEKER_SET]); + await expect((plugin as any).canReadObject('employer_member', SEEKER_CTX)).resolves.toBe(false); + }); + + it('ADMITS the object the caller does hold a read grant on (the negative control)', async () => { + // The control that separates this fix from "refuse everything on the + // analytics path", which would make the refusal case above green while + // deleting the SQL analytics path. + const { plugin } = await boot([SEEKER_SET]); + await expect((plugin as any).canReadObject('employer', SEEKER_CTX)).resolves.toBe(true); + }); + + it('bypasses for a system context, exactly as the middleware does', async () => { + const { plugin } = await boot([SEEKER_SET]); + await expect( + (plugin as any).canReadObject('employer_member', { isSystem: true }), + ).resolves.toBe(true); + }); + + it('REFUSES a required capability the caller lacks (ADR-0066 D3), ahead of the grant', async () => { + const { plugin } = await boot([CAPLESS_SET]); + await expect((plugin as any).canReadObject('payroll_run', SEEKER_CTX)).resolves.toBe(false); + }); + + it('ADMITS the same object once the caller holds the capability', async () => { + const { plugin } = await boot([CAPABLE_SET]); + await expect((plugin as any).canReadObject('payroll_run', SEEKER_CTX)).resolves.toBe(true); + }); + + it('REFUSES an unresolvable object posture (#3545 fail-closed)', async () => { + // `isPrivate` would default to `false`, which is exactly what lets a plain + // wildcard reach an object ADR-0066 D2 says it must not. + const { plugin } = await boot([ADMIN_SET]); + await expect((plugin as any).canReadObject('not_a_registered_object', SEEKER_CTX)).resolves.toBe(false); + }); + + it('REFUSES an empty object name rather than resolving something', async () => { + const { plugin } = await boot([ADMIN_SET]); + await expect((plugin as any).canReadObject('', SEEKER_CTX)).resolves.toBe(false); + }); + + it('is exposed on the registered "security" service, not only on the class', async () => { + // A method the class declares but the service literal does not expose is + // unreachable across the service-locator seam every cross-package consumer + // uses — which for this method would mean the gate silently never runs. + const registered: Record = {}; + const middlewares: Array = []; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: { + registerMiddleware: (mw: unknown) => middlewares.push(mw), + getSchema: (name: string) => SCHEMAS[name], + findOne: vi.fn(async () => null), + }, + metadata: { get: async (_t: string, n: string) => SCHEMAS[n], list: async () => [SEEKER_SET] }, + }; + const ctx: Record = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: (name: string, svc: unknown) => { + registered[name] = svc; + }, + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as any); + await plugin.start(ctx as any); + + const security = registered['security'] as { canReadObject?: (o: string, c?: unknown) => Promise }; + expect(typeof security?.canReadObject).toBe('function'); + await expect(security.canReadObject!('employer_member', SEEKER_CTX)).resolves.toBe(false); + await expect(security.canReadObject!('employer', SEEKER_CTX)).resolves.toBe(true); + }); +}); diff --git a/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts b/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts new file mode 100644 index 0000000000..0635c9840e --- /dev/null +++ b/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts @@ -0,0 +1,234 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// END-TO-END equivalence gate: `POST /analytics/dataset/query` with an INLINE +// dataset and `GET /data/` must reach the SAME admission verdict, for +// the same principal, ON BOTH DRIVERS. +// +// ## The defect +// +// The analytics route accepts an inline dataset definition (`body.dataset`) +// from any authenticated caller and compiled it straight to SQL. On a SQL +// driver `NativeSQLStrategy` ran that statement through the driver's raw +// `execute()`, which is documented as a tenant-isolation bypass and which no +// middleware sits in front of — so the request reached the database having +// passed exactly ONE of the three read layers (the row scope, threaded since +// ADR-0021 D-C). A job seeker with NO grant on `ats_employer_member` was +// answered `200 {"rows":[{"cnt":24}]}` where `GET /data/ats_employer_member` +// answered `403 PERMISSION_DENIED`. On the memory driver the same request went +// through the ObjectQL engine, which applies all three layers in one place, and +// was refused. Two strategies, two answers about the security boundary, and the +// permissive one was the default driver's. +// +// ## Why this file boots TWICE +// +// The two drivers reach the analytics service through DIFFERENT strategies — +// `NativeSQLStrategy` on `sqlite-wasm`, `ObjectQLStrategy` on `memory` (which +// cannot run raw SQL) — and the defect was precisely that the two disagreed. A +// gate written against one driver cannot see that class of divergence at all, +// which is how it shipped. Every case below therefore runs from one table +// against both boots, and the verdicts are compared to `/data`'s rather than to +// a hard-coded expectation: the assertion is AGREEMENT, so it stays honest if +// the platform's own answer for a persona ever changes. +// +// ## The negative controls, which are the easy thing to lose +// +// An implementation where the native strategy simply refuses makes the +// equivalence green while deleting the SQL analytics path. So the table also +// carries the ADMITTED rows — an administrator's totals and a member's +// RLS-scoped count — and asserts the analytics number equals the `/data` +// number rather than merely that both were 200. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { + admissionFixtureStack, + admissionFixtureSecurity, +} from './fixtures/analytics-admission-fixture.js'; + +const ADMIN_ROWS = 3; +const MEMBER_ROWS = 2; + +/** The smallest dataset query there is — the exact shape the reported probe posted. */ +const inlineCount = (object: string) => ({ + name: `probe_${object}`, + label: 'probe', + object, + dimensions: [], + measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }], +}); + +/** …and the grouped form, which is the expressive half of the oracle. */ +const inlineGrouped = (object: string) => ({ + name: `probe_grouped_${object}`, + label: 'probe grouped', + object, + dimensions: [{ name: 'region', label: 'Region', field: 'region', type: 'string' }], + measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }], +}); + +/** `403` / `200` / `` — the admission verdict, nothing finer. */ +type Verdict = string; + +const DRIVERS = ['sqlite-wasm', 'memory'] as const; + +interface Boot { + stack: VerifyStack; + adminToken: string; + memberToken: string; +} + +const boots = new Map(); + +async function bootFor(driver: (typeof DRIVERS)[number]): Promise { + const stack = await bootStack(admissionFixtureStack as never, { + security: admissionFixtureSecurity(), + databaseDriver: driver, + }); + const adminToken = await stack.signIn(); + const memberToken = await stack.signUp(`admission-${driver}@verify.test`); + + // Author through HTTP as each principal so `created_by` carries the real + // caller — the owner policy on `admission_open` is what makes the member's + // admitted count a scoped number rather than the table total. + for (let i = 0; i < ADMIN_ROWS; i++) { + const r = await stack.apiAs(adminToken, 'POST', '/data/admission_open', { + name: `admin-open-${i}`, + region: i % 2 === 0 ? 'west' : 'east', + }); + expect(r.status).toBeLessThan(300); + const w = await stack.apiAs(adminToken, 'POST', '/data/admission_walled', { + name: `admin-walled-${i}`, + region: 'west', + }); + expect(w.status).toBeLessThan(300); + } + for (let i = 0; i < MEMBER_ROWS; i++) { + const r = await stack.apiAs(memberToken, 'POST', '/data/admission_open', { + name: `member-open-${i}`, + region: 'west', + }); + expect(r.status).toBeLessThan(300); + } + return { stack, adminToken, memberToken }; +} + +/** The `/data` door's verdict, and its count when it admits. */ +async function restProbe( + boot: Boot, + token: string, + object: string, +): Promise<{ verdict: Verdict; count: number | undefined }> { + const res = await boot.stack.apiAs(token, 'GET', `/data/${object}?$top=200&$count=true`); + if (res.status !== 200) return { verdict: String(res.status), count: undefined }; + const body = (await res.json()) as { records?: unknown[]; total?: number; count?: number }; + const count = body.total ?? body.count ?? body.records?.length ?? 0; + return { verdict: '200', count: Number(count) }; +} + +/** The analytics door's verdict for an INLINE dataset, and its count when it admits. */ +async function analyticsProbe( + boot: Boot, + token: string, + dataset: unknown, + selection: { dimensions?: string[]; measures: string[] }, +): Promise<{ verdict: Verdict; count: number | undefined }> { + const res = await boot.stack.apiAs(token, 'POST', '/analytics/dataset/query', { + dataset, + selection, + }); + if (res.status !== 200) return { verdict: String(res.status), count: undefined }; + const body = (await res.json()) as { rows?: Array> }; + const count = (body.rows ?? []).reduce((sum, row) => sum + Number(row.cnt ?? 0), 0); + return { verdict: '200', count }; +} + +describe.each(DRIVERS)( + 'dogfood: inline analytics and /data reach ONE admission verdict [driver=%s]', + (driver) => { + beforeAll(async () => { + boots.set(driver, await bootFor(driver)); + }, 120_000); + + afterAll(async () => { + await boots.get(driver)?.stack.stop(); + boots.delete(driver); + }); + + it('the app declares ZERO datasets — the surface under test is the INLINE slot', async () => { + // The premise the independent reproduction established: a deployment that + // ships no analytics at all has the identical exposure, because + // `body.dataset` is the reachable slot. A named dataset must 404, which is + // what proves the refusals below are not "the dataset was not found". + const boot = boots.get(driver)!; + const named = await boot.stack.apiAs(boot.memberToken, 'POST', '/analytics/dataset/query', { + datasetName: 'admission_walled_metrics', + selection: { measures: ['cnt'] }, + }); + expect(named.status).toBe(404); + }); + + it('a member with NO grant is refused by BOTH doors, with the same status', async () => { + const boot = boots.get(driver)!; + const rest = await restProbe(boot, boot.memberToken, 'admission_walled'); + const analytics = await analyticsProbe( + boot, + boot.memberToken, + inlineCount('admission_walled'), + { measures: ['cnt'] }, + ); + + // The premise: `/data` really does refuse here. Without this the + // equivalence below could hold for the wrong reason. + expect(rest.verdict).toBe('403'); + expect(analytics.verdict).toBe(rest.verdict); + }); + + it('the GROUPED form of the same request is refused too (the expressive oracle)', async () => { + const boot = boots.get(driver)!; + const rest = await restProbe(boot, boot.memberToken, 'admission_walled'); + const analytics = await analyticsProbe( + boot, + boot.memberToken, + inlineGrouped('admission_walled'), + { dimensions: ['region'], measures: ['cnt'] }, + ); + expect(analytics.verdict).toBe(rest.verdict); + }); + + // ── Negative controls ─────────────────────────────────────────────────── + it('a member WITH the grant is admitted by both doors, and gets the SAME number', async () => { + const boot = boots.get(driver)!; + const rest = await restProbe(boot, boot.memberToken, 'admission_open'); + const analytics = await analyticsProbe( + boot, + boot.memberToken, + inlineCount('admission_open'), + { measures: ['cnt'] }, + ); + + expect(rest.verdict).toBe('200'); + expect(analytics.verdict).toBe('200'); + // The owner policy is live, so this is the RLS-scoped number — the + // control that a fix must not break while adding the object-level layer. + expect(rest.count).toBe(MEMBER_ROWS); + expect(analytics.count).toBe(rest.count); + }); + + it('an administrator is admitted on both objects, and gets the SAME numbers', async () => { + const boot = boots.get(driver)!; + for (const [object, expected] of [ + ['admission_open', ADMIN_ROWS + MEMBER_ROWS], + ['admission_walled', ADMIN_ROWS], + ] as const) { + const rest = await restProbe(boot, boot.adminToken, object); + const analytics = await analyticsProbe(boot, boot.adminToken, inlineCount(object), { + measures: ['cnt'], + }); + expect(rest.verdict).toBe('200'); + expect(analytics.verdict).toBe('200'); + expect(rest.count).toBe(expected); + expect(analytics.count).toBe(rest.count); + } + }); + }, +); diff --git a/packages/qa/dogfood/test/fixtures/analytics-admission-fixture.ts b/packages/qa/dogfood/test/fixtures/analytics-admission-fixture.ts new file mode 100644 index 0000000000..9ddac029e5 --- /dev/null +++ b/packages/qa/dogfood/test/fixtures/analytics-admission-fixture.ts @@ -0,0 +1,97 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The reported shape, in miniature — a two-object app where a plain member +// holds a read grant on ONE object and NOTHING at all on the other. +// +// `POST /api/v1/analytics/dataset/query` accepts an INLINE dataset from any +// authenticated caller. On a SQL driver that inline definition was compiled and +// run through the driver's raw `execute()`, which no middleware sits in front +// of, so a caller with no grant received a row count for an object whose +// `GET /api/v1/data/` door answers `403 PERMISSION_DENIED`. +// +// ## Why the app declares NO dataset and NO dashboard +// +// That is the load-bearing property, not an omission. The independent +// reproduction that graded the defect measured a tree with **0 datasets and +// 0 dashboards** and got the identical 200: the reachable slot is the INLINE +// definition, so an application does not have to declare anything and cannot +// decline to. A fixture that declared a dataset would prove a weaker statement +// — that DECLARED analytics is gated — and leave the actual surface untested. +// +// ## Why the member's grant is per-object rather than a wildcard +// +// The gate has to be measured in BOTH directions on one boot. A fixture where +// the member can read nothing would go green under an implementation that +// simply refuses every analytics query on a SQL driver — which passes the +// refusal half while deleting the SQL analytics path. `admission_open` is the +// negative control that such an implementation fails. + +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, RLS, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +/** The object the member MAY read — with an owner RLS policy, so the row scope is live. */ +export const AdmissionOpen = ObjectSchema.create({ + name: 'admission_open', + sharingModel: 'public_read_write', + label: 'Admission Open', + pluralLabel: 'Admission Open', + fields: { + name: Field.text({ label: 'Name', required: true }), + region: Field.text({ label: 'Region' }), + }, +}); + +/** The object the member holds NO grant on at all — the reported `ats_employer_member`. */ +export const AdmissionWalled = ObjectSchema.create({ + name: 'admission_walled', + sharingModel: 'public_read_write', + label: 'Admission Walled', + pluralLabel: 'Admission Walled', + fields: { + name: Field.text({ label: 'Name', required: true }), + region: Field.text({ label: 'Region' }), + }, +}); + +export const admissionFixtureStack = defineStack({ + manifest: { + id: 'com.dogfood.analytics_admission', + namespace: 'admission', + version: '0.0.0', + type: 'app', + name: 'Analytics Admission Fixture', + description: + 'Two objects, one grant, ZERO datasets and ZERO dashboards — the inline-dataset admission surface.', + }, + objects: [AdmissionOpen, AdmissionWalled], +}); + +const FIXTURE_MEMBER_SET = 'admission_fixture_member'; + +/** + * The fallback set a fresh member resolves to: read on `admission_open` only, + * owner-scoped by `created_by`, and no entry whatsoever for `admission_walled`. + * + * The owner policy is what makes the ADMITTED half a real measurement rather + * than a trivially equal pair of totals: the member's count has to be their own + * rows on both routes, which is the reported "the RLS-scoped count survives the + * fix" control. + */ +export const admissionMemberSet: PermissionSet = PermissionSetSchema.parse({ + name: FIXTURE_MEMBER_SET, + label: 'Admission Fixture Member — read on admission_open only', + objects: { + admission_open: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + rowLevelSecurity: [RLS.ownerPolicy('admission_open', 'created_by')], +}); + +/** SecurityPlugin whose fresh-member fallback is the fixture set, over the real defaults. */ +export function admissionFixtureSecurity(): SecurityPlugin { + return new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, admissionMemberSet], + fallbackPermissionSet: admissionMemberSet.name, + }); +} diff --git a/packages/services/service-analytics/src/__tests__/read-admission-gate.test.ts b/packages/services/service-analytics/src/__tests__/read-admission-gate.test.ts new file mode 100644 index 0000000000..36835c73ed --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-admission-gate.test.ts @@ -0,0 +1,311 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The OBJECT-LEVEL read gate at the analytics door — the layer the raw-SQL path + * had no way to inherit. + * + * `POST /analytics/dataset/query` accepts an INLINE dataset from any + * authenticated caller. On a SQL driver `NativeSQLStrategy` compiled it and ran + * it through the driver's raw `execute()`, which no middleware sits in front + * of, so the request reached the database having passed exactly ONE of the + * three read layers — the row scope. A caller with NO grant of any kind on an + * object was answered `200 {"rows":[{"cnt":24}]}` where `GET /data/` + * answered `403 PERMISSION_DENIED` for the same principal on the same + * deployment. The memory driver refused the identical request, because there + * the query falls through to the ObjectQL engine and the engine applies all + * three layers in one place. + * + * ## What these cases are shaped to catch + * + * The gate is asked ONCE at the door, ahead of strategy selection, so the two + * strategies give the SAME verdict by construction rather than by each carrying + * its own copy of the check — two copies being the arrangement that produced + * the divergence. Every refusal case below is therefore run through BOTH + * strategy paths from one table: a fix that only taught `NativeSQLStrategy` to + * refuse would pass half of them, and a gate that sat inside either strategy + * would fail the other half. + * + * The `admitted` cases are the negative controls, and they are the ones a lazy + * fix loses: an implementation where the native strategy simply refuses turns + * every refusal case green while deleting the SQL analytics path. + */ + +import { describe, it, expect } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsService } from '../analytics-service.js'; +import { compileDataset } from '../dataset-compiler.js'; + +/** + * The smallest dataset query there is — the exact shape the reported probe + * posted into `body.dataset`: one object, one count measure, no dimensions. + */ +const memberCount = DatasetSchema.parse({ + name: 'probe_member', + label: 'probe', + object: 'employer_member', + dimensions: [], + measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }], +}); + +/** A dataset that JOINS — the caller may read the base object and not the join. */ +const memberWithEmployer = DatasetSchema.parse({ + name: 'probe_join', + label: 'probe join', + object: 'employer_member', + include: ['employer'], + dimensions: [{ name: 'industry', field: 'employer.industry', type: 'string' }], + measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }], +}); + +const CALLER = { userId: 'u_seeker', tenantId: 'org_a' } as ExecutionContext; + +/** The two capability postures that select the two strategies. */ +const nativeSqlOnly = () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }); +const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }); + +const STRATEGY_PATHS = [ + { label: 'NativeSQLStrategy (SQL driver — the reported path)', capabilities: nativeSqlOnly }, + { label: 'ObjectQLStrategy (memory driver — the engine path)', capabilities: objectqlOnly }, +] as const; + +interface Executions { + sql: Array<{ sql: string; params: unknown[] }>; + aggregate: Array<{ object: string }>; +} + +function makeService( + opts: { + capabilities: () => { nativeSql: boolean; objectqlAggregate: boolean; inMemory: boolean }; + admitObjectRead?: (object: string, context?: ExecutionContext) => boolean | Promise; + getReadScope?: (object: string, context?: ExecutionContext) => Record | undefined; + relationshipResolver?: (base: string, rel: string) => string | undefined; + }, + seen: Executions, +) { + const compiled = compileDataset(memberCount); + const compiledJoin = compileDataset( + memberWithEmployer, + opts.relationshipResolver ?? (() => 'employer'), + ); + return new AnalyticsService({ + cubes: [compiled.cube, compiledJoin.cube], + queryCapabilities: opts.capabilities, + admitObjectRead: opts.admitObjectRead, + getReadScope: opts.getReadScope as never, + executeRawSql: async (_object, sql, params) => { + seen.sql.push({ sql, params }); + return [{ cnt: 24 }]; + }, + executeAggregate: async (object) => { + seen.aggregate.push({ object }); + return [{ cnt: 24 }]; + }, + }); +} + +const emptySeen = (): Executions => ({ sql: [], aggregate: [] }); + +/** Every way the reported request can arrive at this service. */ +const DOORS = [ + { + label: 'queryDataset (the inline body.dataset slot)', + run: (svc: AnalyticsService) => svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER), + }, + { + label: 'query (the direct /analytics/query door)', + run: (svc: AnalyticsService) => svc.query({ cube: 'probe_member', measures: ['cnt'] }, CALLER), + }, + { + label: 'generateSql (the /analytics/sql echo door)', + run: (svc: AnalyticsService) => svc.generateSql({ cube: 'probe_member', measures: ['cnt'] }, CALLER), + }, +] as const; + +describe('analytics — object-level read admission at the door', () => { + describe.each(STRATEGY_PATHS)('$label', ({ capabilities }) => { + it.each(DOORS)( + 'refuses PERMISSION_DENIED / 403 when the caller holds no read grant — $label', + async ({ run }) => { + const seen = emptySeen(); + const svc = makeService({ capabilities, admitObjectRead: () => false }, seen); + + await expect(run(svc)).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + }); + + // The refusal is an ADMISSION verdict: nothing was compiled, nothing + // ran. A gate that refused only after executing would still have + // disclosed the number through timing and through the driver's logs. + expect(seen.sql).toEqual([]); + expect(seen.aggregate).toEqual([]); + }, + ); + + it('names the object it refused, and nothing else about the caller\'s grants', async () => { + const svc = makeService({ capabilities, admitObjectRead: () => false }, emptySeen()); + await expect( + svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER), + ).rejects.toThrow(/employer_member/); + }); + + it('fails CLOSED when the admission provider throws', async () => { + const seen = emptySeen(); + const svc = makeService( + { + capabilities, + admitObjectRead: () => { + throw new Error('permission-set resolution exploded'); + }, + }, + seen, + ); + + await expect( + svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(seen.sql).toEqual([]); + expect(seen.aggregate).toEqual([]); + }); + + it('refuses when the caller may read the BASE object but not a JOINED one', async () => { + const seen = emptySeen(); + const svc = makeService( + { + capabilities, + admitObjectRead: (object) => object === 'employer_member', + }, + seen, + ); + + await expect( + svc.queryDataset(memberWithEmployer, { dimensions: ['industry'], measures: ['cnt'] }, CALLER), + ).rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(seen.sql).toEqual([]); + expect(seen.aggregate).toEqual([]); + }); + + // ── The negative controls ──────────────────────────────────────────────── + // An implementation where the native strategy simply refuses makes every + // case above green while deleting the SQL analytics path. These are what + // separate a fix from that. + it('ADMITS a granted caller and serves the number unchanged', async () => { + const seen = emptySeen(); + const svc = makeService({ capabilities, admitObjectRead: () => true }, seen); + + const result = await svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER); + expect(result.rows).toEqual([{ cnt: 24 }]); + expect(seen.sql.length + seen.aggregate.length).toBe(1); + }); + + it('leaves behaviour unchanged when NO admission provider is wired', async () => { + const seen = emptySeen(); + const svc = makeService({ capabilities }, seen); + + const result = await svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER); + expect(result.rows).toEqual([{ cnt: 24 }]); + expect(seen.sql.length + seen.aggregate.length).toBe(1); + }); + }); + + it('the two strategies reach the SAME verdict for the same caller and object', async () => { + // The acceptance condition, asserted as an EQUIVALENCE rather than as two + // independent per-strategy expectations: whatever one path answers, the + // other must answer too. Written this way so a future divergence fails here + // even if someone updates one of the per-path cases above. + const verdicts = await Promise.all( + STRATEGY_PATHS.map(async ({ capabilities }) => { + const svc = makeService({ capabilities, admitObjectRead: () => false }, emptySeen()); + try { + await svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER); + return 'admitted'; + } catch (e) { + const err = e as { code?: string; status?: number }; + return `${err.status}:${err.code}`; + } + }), + ); + expect(verdicts[0]).toBe('403:PERMISSION_DENIED'); + expect(new Set(verdicts).size).toBe(1); + }); + + it('the admitted path stays identical across both strategies (the number does not move)', async () => { + const results = await Promise.all( + STRATEGY_PATHS.map(async ({ capabilities }) => { + const svc = makeService({ capabilities, admitObjectRead: () => true }, emptySeen()); + const r = await svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER); + return r.rows; + }), + ); + expect(results[0]).toEqual([{ cnt: 24 }]); + expect(results[1]).toEqual(results[0]); + }); +}); + +describe('analytics — the tenant wall reaches the inline dataset on the raw-SQL path', () => { + /** + * The WALLED-posture leg, measured rather than inferred. + * + * `{ organization_id: 'org_a' }` is not a predicate invented here: it is + * exactly what `tenantLayer0FilterOf` projects for the `isolated` wall's + * `{ kind: 'organization' }` verdict, and exactly what plugin-security's own + * `getReadFilter` returns for a member under a walled posture (pinned there + * by `tenant-layer0-verdict-on-operation.test.ts`: + * `getReadFilter('crm_task', MEMBER_CTX)` → `{ organization_id: 'org-1' }`). + * This is the CONSUMER half of that chain: the wall's own predicate has to + * survive into the statement the raw-SQL path compiles, or a cross- + * organization inline dataset counts another tenant's rows. + */ + const walledScope = (_object: string, context?: ExecutionContext) => + context?.tenantId ? { organization_id: context.tenantId } : undefined; + + it('compiles the wall predicate into the statement, bound to the caller organization', async () => { + const seen = emptySeen(); + const svc = makeService( + { capabilities: nativeSqlOnly, admitObjectRead: () => true, getReadScope: walledScope }, + seen, + ); + + await svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER); + + expect(seen.sql).toHaveLength(1); + expect(seen.sql[0].sql).toMatch(/organization_id/); + expect(seen.sql[0].params).toContain('org_a'); + }); + + it('walls EVERY object the statement reads, joined ones included', async () => { + const seen = emptySeen(); + const svc = makeService( + { capabilities: nativeSqlOnly, admitObjectRead: () => true, getReadScope: walledScope }, + seen, + ); + + await svc.queryDataset( + memberWithEmployer, + { dimensions: ['industry'], measures: ['cnt'] }, + CALLER, + ); + + expect(seen.sql).toHaveLength(1); + // Two objects are read (base + join), so the wall appears twice — a single + // occurrence would mean the joined table is unwalled and a cross-org row + // can reach the GROUP BY through it. + const occurrences = seen.sql[0].sql.match(/organization_id/g) ?? []; + expect(occurrences.length).toBeGreaterThanOrEqual(2); + expect(seen.sql[0].params.filter((p) => p === 'org_a').length).toBeGreaterThanOrEqual(2); + }); + + it('the admission gate and the wall are INDEPENDENT — an admitted caller is still walled', async () => { + // The two layers must not collapse into one another: `admitObjectRead` + // answering `true` says the caller may read the OBJECT, never that they may + // read every organization's rows. + const seen = emptySeen(); + const svc = makeService( + { capabilities: nativeSqlOnly, admitObjectRead: () => true, getReadScope: walledScope }, + seen, + ); + await svc.queryDataset(memberCount, { measures: ['cnt'] }, CALLER); + expect(seen.sql[0].params).toContain('org_a'); + }); +}); diff --git a/packages/verify/src/harness.ts b/packages/verify/src/harness.ts index fe454e7f4b..6d667be3d8 100644 --- a/packages/verify/src/harness.ts +++ b/packages/verify/src/harness.ts @@ -301,6 +301,25 @@ export interface BootOptions { * (which is exactly what #4518 turned out to be). */ databaseFile?: string; + /** + * The default datasource's driver. Default `'sqlite-wasm'` — the pure-JS + * in-memory SQLite this harness has always booted, and the driver a real + * `objectstack dev` uses. + * + * `'memory'` boots `@objectstack/driver-memory` instead, which is the OTHER + * half of a two-driver equivalence measurement: the two drivers reach the + * analytics service through DIFFERENT strategies (`NativeSQLStrategy` compiles + * raw SQL on a SQL driver; the memory driver cannot run raw SQL, so the query + * falls through to `ObjectQLStrategy` and the engine's middleware). A gate + * that asserts the two doors reach ONE verdict cannot be written against one + * driver — asking on `sqlite-wasm` alone is exactly how the strategies were + * allowed to disagree about the security boundary. + * + * ⛔ It is NOT a general "run any fixture on memory" switch. The memory driver + * does not implement every SQL behaviour this harness's other gates depend + * on; use it where the DRIVER is the variable under test. + */ + databaseDriver?: 'sqlite-wasm' | 'memory'; /** * Extra plugins to register between the app/service pairs and the * SecurityPlugin — the slot where `objectstack dev` auto-loads optional @@ -391,11 +410,14 @@ export async function bootStack( // §Risk mitigation the ADR promised), not the legacy pre-built DriverPlugin // escape hatch. await kernel.use(new ObjectQLPlugin()); + const databaseDriver = opts.databaseDriver ?? 'sqlite-wasm'; await kernel.use(new DefaultDatasourcePlugin({ - driver: 'sqlite-wasm', + driver: databaseDriver, // `opts.databaseFile` makes the database outlive the kernel, so a second // boot over the same path is a real cold start (see BootOptions.databaseFile). - config: { filename: opts.databaseFile ?? ':memory:' }, + // The memory driver holds no file — it takes no `filename` and a stray one + // would be config the driver silently ignores. + config: databaseDriver === 'memory' ? {} : { filename: opts.databaseFile ?? ':memory:' }, })); // HTTP server (registers the `http-server` IHttpServer service the REST + From f4fa0a6c4ed2498dd5d29360a3367df56388b10c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 09:55:30 +0000 Subject: [PATCH 3/8] wip(analytics): two-driver dogfood acceptance table Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...cs-inline-dataset-admission.dogfood.test.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts b/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts index 0635c9840e..4e8548ed7b 100644 --- a/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts +++ b/packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts @@ -45,7 +45,9 @@ import { admissionFixtureSecurity, } from './fixtures/analytics-admission-fixture.js'; -const ADMIN_ROWS = 3; +const ADMIN_OPEN_ROWS = 3; +/** Deliberately DIFFERENT from the open count, so the two objects' totals cannot be confused. */ +const ADMIN_WALLED_ROWS = 4; const MEMBER_ROWS = 2; /** The smallest dataset query there is — the exact shape the reported probe posted. */ @@ -90,12 +92,14 @@ async function bootFor(driver: (typeof DRIVERS)[number]): Promise { // Author through HTTP as each principal so `created_by` carries the real // caller — the owner policy on `admission_open` is what makes the member's // admitted count a scoped number rather than the table total. - for (let i = 0; i < ADMIN_ROWS; i++) { + for (let i = 0; i < ADMIN_OPEN_ROWS; i++) { const r = await stack.apiAs(adminToken, 'POST', '/data/admission_open', { name: `admin-open-${i}`, region: i % 2 === 0 ? 'west' : 'east', }); expect(r.status).toBeLessThan(300); + } + for (let i = 0; i < ADMIN_WALLED_ROWS; i++) { const w = await stack.apiAs(adminToken, 'POST', '/data/admission_walled', { name: `admin-walled-${i}`, region: 'west', @@ -216,9 +220,15 @@ describe.each(DRIVERS)( it('an administrator is admitted on both objects, and gets the SAME numbers', async () => { const boot = boots.get(driver)!; + // The expected totals are the administrator's OWN rows on each object, + // not the table totals: the platform's ownership floor scopes this + // principal on these public fixture objects. That is `/data`'s answer, so + // it must be the analytics answer too — which is the whole assertion. The + // two objects carry deliberately different counts so a number arriving + // from the wrong table cannot pass. for (const [object, expected] of [ - ['admission_open', ADMIN_ROWS + MEMBER_ROWS], - ['admission_walled', ADMIN_ROWS], + ['admission_open', ADMIN_OPEN_ROWS], + ['admission_walled', ADMIN_WALLED_ROWS], ] as const) { const rest = await restProbe(boot, boot.adminToken, object); const analytics = await analyticsProbe(boot, boot.adminToken, inlineCount(object), { From 927d956b834f1f1ff87f68821787bdddd35fa286 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 10:09:02 +0000 Subject: [PATCH 4/8] fix(analytics): ask the object-level read grant at the analytics door Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...cs-inline-dataset-object-read-admission.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .changeset/analytics-inline-dataset-object-read-admission.md diff --git a/.changeset/analytics-inline-dataset-object-read-admission.md b/.changeset/analytics-inline-dataset-object-read-admission.md new file mode 100644 index 0000000000..d1e21c82a9 --- /dev/null +++ b/.changeset/analytics-inline-dataset-object-read-admission.md @@ -0,0 +1,19 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-security": minor +"@objectstack/service-analytics": minor +"@objectstack/verify": minor +--- + +`POST /analytics/dataset/query` now asks the OBJECT-level read grant before it serves an inline dataset, so the analytics door and `GET /data/` reach one admission verdict on every driver. + +The route accepts an inline dataset definition (`body.dataset`) from any authenticated caller. On a SQL driver the compiled statement ran through the driver's raw `execute()`, which is documented as a tenant-isolation bypass and which no middleware sits in front of — so the request reached the database having passed exactly ONE of the three read layers (the row scope, threaded since ADR-0021 D-C). A caller with **no grant of any kind** on an object received its row count, and with `dimensions` its grouped counts by any column, where the `/data` door answered `403 PERMISSION_DENIED` for the same principal on the same deployment. On the memory driver the identical request fell through to the ObjectQL engine, which applies all three layers in one place, and was refused. The exposure is not opt-in and an application cannot decline it: a deployment shipping 0 datasets and 0 dashboards has the identical surface, because the reachable slot is the inline definition rather than a declared one. + +**This change NARROWS what the analytics doors accept.** Requests that were already refused by `/data` are now refused by analytics too; nothing that was refused becomes admitted. + +- **`ISecurityService.canReadObject(object, context)`** (`@objectstack/spec`, optional) — the object-level half of a read, the sibling of `getReadFilter`'s row-level half. It exists because the two are not interchangeable: `getReadFilter` answers "which rows" and answers `undefined` — "no row restriction" — for a caller who may not read the object at all, so a door holding only the filter reads a caller with NO grant as a caller with NO restriction. Fails CLOSED. Absence is a defined state and its fallback is **not** "admit": a consumer composes the same verdict from `explain`, which is not optional. +- **`@objectstack/plugin-security` implements it** as the middleware's own read gate, arm for arm and in its order — the `isSystem` bypass, the "no permission sets resolved" skip, the #3545 fail-closed refusal on an unresolvable object posture, the ADR-0066 D3 `requiredPermissions` capability AND-gate, the `allowRead` CRUD grant, and the ADR-0090 D10 delegator intersection — from the same primitives the middleware calls, and it is exposed on the registered `security` service. +- **`@objectstack/service-analytics` asks it once at the door**, for the base object and every joined object, **ahead of strategy selection**. Placement is the fix: two strategies each enforcing their own copy of three layers is the CAUSE of the divergence, not its remedy, so both strategies — and any strategy added later — inherit one verdict by construction. `AnalyticsServicePlugin` auto-bridges the new `admitObjectRead` hook to the `security` service (`canReadObject`, falling back to `explain`), the same way it already bridges `getReadScope`, and warns loudly at init when no security service is registered. +- **`@objectstack/verify`** gains `bootStack(app, { databaseDriver: 'sqlite-wasm' | 'memory' })`, because a two-driver equivalence property cannot be measured on one driver — which is how the strategies were allowed to disagree. + +The refusal is `PERMISSION_DENIED` / 403, the same code and status the engine path already answers, and it names only the object the caller themselves named. From 961cdd9393c48997201afcea13c3b155aa5b0665 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 10:26:50 +0000 Subject: [PATCH 5/8] docs(permissions): record canReadObject's elevation read in the system-context census Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- content/docs/permissions/system-context.mdx | 151 ++++++++++---------- scripts/check-system-context-census.mjs | 23 +-- 2 files changed, 88 insertions(+), 86 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index b977ac709c..364db91ad9 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -9,9 +9,9 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a service self-write, a migration. This page is **the authority** for what that flag actually does. It exists -because the flag is not one concept: it is a single boolean read at **106 +because the flag is not one concept: it is a single boolean read at **107 distinct sites across 20 packages**, and knowing three of those behaviours gives -no hint that the other hundred-and-three exist. Every documented app-side bug +no hint that the other hundred-and-four exist. Every documented app-side bug traced to `isSystem` had the same shape — the metadata was complete and correct, and the gap was observable only by querying the resulting rows. @@ -94,90 +94,91 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 1 | **The whole security middleware short-circuits** before any gate runs | plugin-security | Get: every CRUD/FLS/tenant/owner gate below skipped in one branch. Lose: all of rows 2–6 at once — this is the single largest behaviour on the page | `packages/plugins/plugin-security/src/security-plugin.ts#start` | +| 1 | **The whole security middleware short-circuits** before any gate runs | plugin-security | Get: every CRUD/FLS/tenant/owner gate below skipped in one branch. Lose: all of rows 2–7 at once — this is the single largest behaviour on the page | `packages/plugins/plugin-security/src/security-plugin.ts#start` | | 2 | **`owner_id` is not auto-stamped on INSERT** (the step 3.5 anchor guard is inside the block row 1 skips) | plugin-security | Lose: the row lands `owner_id = NULL`, so the default `owner_only_writes` policy hides it **from its own creator**. Get: nothing — this is a gap, not a capability | the step 3.5 guard block and the short-circuit that skips it are both inside `packages/plugins/plugin-security/src/security-plugin.ts#start` | | 3 | Row-level read filter resolves to "no filter" | plugin-security | Get: unscoped reads. Lose: row-level scoping entirely | `packages/plugins/plugin-security/src/security-plugin.ts#getReadFilter` | | 4 | Field-level security returns **all** fields | plugin-security | Get: every column readable. Lose: field masking | `packages/plugins/plugin-security/src/security-plugin.ts#computeReadableFields` | | 5 | Export permission granted unconditionally | plugin-security | Get: `canExport` is `true` | `packages/plugins/plugin-security/src/security-plugin.ts#canExport` | -| 6 | Write bypass = `true`, effective write scope = `org` | plugin-security | Get: widest write scope without holding any capability | `packages/plugins/plugin-security/src/security-plugin.ts#start` | -| 7 | Metadata-plane schema masking exempt (ADR-0106 D4) | metadata-core | Get: unmasked object schema. Note: the exemption is a **caller** property — it short-circuits before the security service is consulted | `packages/metadata-core/src/object-schema-fls.ts#isObjectSchemaMaskExempt` | -| 8 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `packages/plugins/plugin-security/src/security-plugin.ts#explainAccessForCaller` | -| 9 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `packages/core/src/security/anonymous-deny.ts#shouldDenyAnonymous` | -| 10 | Permission-set projection middleware skipped | plugin-security | Lose: projection of permission-set-derived columns | `packages/plugins/plugin-security/src/permission-set-projection.ts#createPermissionSetWriteThrough` | -| 11 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `packages/plugins/plugin-auth/src/auth-plugin.ts#start` | -| 12 | Per-request performance timings disclosed | observability | Get: timing headers a normal caller cannot pull | `packages/observability/src/perf-timing.ts#isPerfDisclosurePrincipal` | -| 13 | Permission-set **overlay discard** skips the tenant-admin assertion | plugin-security | Get: an overlay can be discarded with no authenticated tenant administrator | `packages/plugins/plugin-security/src/permission-set-overlay-discard.ts#assertTenantAdmin` | -| 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `packages/mcp/src/stdio-data-bridge.ts#enforceApiExposure` | -| 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `packages/plugins/plugin-audit/src/read-audit.ts#installReadAuditWriter` | -| 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `packages/plugins/plugin-approvals/src/payload-redaction-middleware.ts#bindSnapshotRedactionMiddleware` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `packages/rest/src/rest-server.ts#enforceAuth` | +| 6 | Object-level read admission granted unconditionally | plugin-security | Get: `canReadObject` is `true`. This is the OBJECT-level half of a read — "may this caller read this object at all" — which the doors that bypass this middleware ask before they compile a statement of their own; `getReadFilter` is its row-level half, and the two are not interchangeable | `packages/plugins/plugin-security/src/security-plugin.ts#canReadObject` | +| 7 | Write bypass = `true`, effective write scope = `org` | plugin-security | Get: widest write scope without holding any capability | `packages/plugins/plugin-security/src/security-plugin.ts#start` | +| 8 | Metadata-plane schema masking exempt (ADR-0106 D4) | metadata-core | Get: unmasked object schema. Note: the exemption is a **caller** property — it short-circuits before the security service is consulted | `packages/metadata-core/src/object-schema-fls.ts#isObjectSchemaMaskExempt` | +| 9 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `packages/plugins/plugin-security/src/security-plugin.ts#explainAccessForCaller` | +| 10 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `packages/core/src/security/anonymous-deny.ts#shouldDenyAnonymous` | +| 11 | Permission-set projection middleware skipped | plugin-security | Lose: projection of permission-set-derived columns | `packages/plugins/plugin-security/src/permission-set-projection.ts#createPermissionSetWriteThrough` | +| 12 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `packages/plugins/plugin-auth/src/auth-plugin.ts#start` | +| 13 | Per-request performance timings disclosed | observability | Get: timing headers a normal caller cannot pull | `packages/observability/src/perf-timing.ts#isPerfDisclosurePrincipal` | +| 14 | Permission-set **overlay discard** skips the tenant-admin assertion | plugin-security | Get: an overlay can be discarded with no authenticated tenant administrator | `packages/plugins/plugin-security/src/permission-set-overlay-discard.ts#assertTenantAdmin` | +| 15 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `packages/mcp/src/stdio-data-bridge.ts#enforceApiExposure` | +| 16 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `packages/plugins/plugin-audit/src/read-audit.ts#installReadAuditWriter` | +| 17 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `packages/plugins/plugin-approvals/src/payload-redaction-middleware.ts#bindSnapshotRedactionMiddleware` | +| 18 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `packages/rest/src/rest-server.ts#enforceAuth` | ### 2. Write pipeline and data integrity | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `packages/objectql/src/engine.ts#update` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `packages/objectql/src/engine.ts#update` | -| 20 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `packages/objectql/src/engine.ts#insert` | -| 21 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `packages/objectql/src/engine.ts#insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | -| 22 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `packages/objectql/src/engine.ts#assertReferencesResolve` | -| 23 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `packages/objectql/src/engine.ts#buildDriverOptions` | -| 24 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `#assertEngineOwnedWriteAllowed` | -| 25 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` | -| 26 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `packages/objectql/src/engine.ts#stripSearchCompanionFromRead` | -| 27 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `packages/objectql/src/engine.ts#dependentCountIsDisclosable` | -| 28 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `packages/objectql/src/engine.ts#recordReferenceCheckElevation` | -| 29 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `packages/plugins/plugin-security/src/security-plugin.ts#start` | +| 19 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `packages/objectql/src/engine.ts#update` | +| 20 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `packages/objectql/src/engine.ts#update` | +| 21 | **`readonly` strip bypassed — INSERT** | objectql | Same, on create — one gate over BOTH create-side passes since the 2026-09-03 ruling moved the static-`readonly` strip in beside the runtime-owned one and deleted the DataProtocol ingress copy. `isSystem` is the **only** exemption on this path: `preserveAudit` is deliberately not read on create, so a non-system historical import is still stripped | `packages/objectql/src/engine.ts#insert` | +| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `packages/objectql/src/engine.ts#insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `packages/objectql/src/engine.ts#assertReferencesResolve` | +| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `packages/objectql/src/engine.ts#buildDriverOptions` | +| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `#assertEngineOwnedWriteAllowed` | +| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` | +| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `packages/objectql/src/engine.ts#stripSearchCompanionFromRead` | +| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `packages/objectql/src/engine.ts#dependentCountIsDisclosable` | +| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `packages/objectql/src/engine.ts#recordReferenceCheckElevation` | +| 30 | **Bulk data event `organizationId` OMITTED** — the batch is published "not asserted" | plugin-security | Get: nothing — the `data.records.*` event still publishes. Lose: the per-organization attribution: this exit is taken before the security middleware composes any tenant wall, so it records no Layer 0 verdict on the operation (`OperationContext.tenantLayer0Verdict`, #15813), and the engine's bulk producer — which reads that recorded verdict and nothing else — omits the key rather than filling it from the caller's `tenantId`; a tenant-scoped consumer then does not deliver the event inside an organization wall (#15225) | `packages/plugins/plugin-security/src/security-plugin.ts#start` | ### 3. Sharing (`plugin-sharing`) -The largest single consumer — **17 of the 106 sites**. +The largest single consumer — **17 of the 107 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| -| 30 | **Sharing-rule REVOCATION is skipped on the record-`afterDelete` hook** — and on that hook only | Lose: nothing permanently — the revoke is **delivered, but deferred on the unbounded shape**. The payload belongs to another subscriber: `packages/plugins/plugin-sharing/src/record-share-cascade.ts` binds on every sharing-capable object and stashes for system writes on its own account (#5103). When the deleted ids are enumerable it revokes inline; when they are not — a predicate delete whose row set the stash could not resolve — it hands the reclaim to a queued background orphan sweep instead, so the share rows outlive the deleted records until that sweep runs, with the boot orphan sweep behind it. No surviving record loses access either way, and a restart re-runs the same sweep. This is one subscriber declining work another owns, not elevation silencing a consequence. ⚠️ **Grant MATERIALISATION no longer asks** — the `afterInsert` / `afterUpdate` skips, and the `before*` stash skip that fed them, were removed by the 2026-08-31 ruling on #13533; a system write materialises exactly as a user write does | `packages/plugins/plugin-sharing/src/rule-hooks.ts#bindRuleHooks` | -| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `packages/plugins/plugin-sharing/src/sharing-service.ts#bypassVerdict` | -| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `packages/plugins/plugin-sharing/src/sharing-service.ts#canManageShares`, `#assertCanManageShares`, `#shouldBypass` | -| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `packages/plugins/plugin-sharing/src/sharing-service.ts#grant` | -| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `packages/plugins/plugin-sharing/src/sharing-service.ts#revoke` (the guard it deletes in front of is in the same function) | -| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `packages/plugins/plugin-sharing/src/sharing-service.ts#listShares` | -| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `packages/plugins/plugin-sharing/src/sharing-plugin.ts#buildSharingMiddleware` | -| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `packages/plugins/plugin-sharing/src/share-link-service.ts#createLink`, `#revokeLink`, `#listLinks` | -| 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `packages/plugins/plugin-sharing/src/sharing-rule-provenance.ts#bindRuleProvenanceStamp` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `packages/plugins/plugin-sharing/src/sharing-rule-service.ts#assertCanManageRules`, `#assertCanDeletePlatformGlobalRule` | +| 31 | **Sharing-rule REVOCATION is skipped on the record-`afterDelete` hook** — and on that hook only | Lose: nothing permanently — the revoke is **delivered, but deferred on the unbounded shape**. The payload belongs to another subscriber: `packages/plugins/plugin-sharing/src/record-share-cascade.ts` binds on every sharing-capable object and stashes for system writes on its own account (#5103). When the deleted ids are enumerable it revokes inline; when they are not — a predicate delete whose row set the stash could not resolve — it hands the reclaim to a queued background orphan sweep instead, so the share rows outlive the deleted records until that sweep runs, with the boot orphan sweep behind it. No surviving record loses access either way, and a restart re-runs the same sweep. This is one subscriber declining work another owns, not elevation silencing a consequence. ⚠️ **Grant MATERIALISATION no longer asks** — the `afterInsert` / `afterUpdate` skips, and the `before*` stash skip that fed them, were removed by the 2026-08-31 ruling on #13533; a system write materialises exactly as a user write does | `packages/plugins/plugin-sharing/src/rule-hooks.ts#bindRuleHooks` | +| 32 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `packages/plugins/plugin-sharing/src/sharing-service.ts#bypassVerdict` | +| 33 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `packages/plugins/plugin-sharing/src/sharing-service.ts#canManageShares`, `#assertCanManageShares`, `#shouldBypass` | +| 34 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `packages/plugins/plugin-sharing/src/sharing-service.ts#grant` | +| 35 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `packages/plugins/plugin-sharing/src/sharing-service.ts#revoke` (the guard it deletes in front of is in the same function) | +| 36 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `packages/plugins/plugin-sharing/src/sharing-service.ts#listShares` | +| 37 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `packages/plugins/plugin-sharing/src/sharing-plugin.ts#buildSharingMiddleware` | +| 38 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `packages/plugins/plugin-sharing/src/share-link-service.ts#createLink`, `#revokeLink`, `#listLinks` | +| 39 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `packages/plugins/plugin-sharing/src/sharing-rule-provenance.ts#bindRuleProvenanceStamp` | +| 40 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `packages/plugins/plugin-sharing/src/sharing-rule-service.ts#assertCanManageRules`, `#assertCanDeletePlatformGlobalRule` | ### 4. Approvals, reports, attachments, comments, knowledge | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `packages/plugins/plugin-approvals/src/lifecycle-hooks.ts#bindApprovalLockHook` | -| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `packages/plugins/plugin-approvals/src/lifecycle-hooks.ts#bindDelegationWriteGuard` | -| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `packages/plugins/plugin-approvals/src/approval-service.ts#isOverrideActor`, `#resolveActor`, `#sendBack`, `#resubmit`, `#reassign`, `#remind`, `#requestInfo`, `#comment` | -| 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `packages/plugins/plugin-reports/src/report-service.ts#saveReport` | -| 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `packages/plugins/plugin-reports/src/report-service.ts#assertExportAllowed`, `#canAccessReport`, `#listReports`, `#listSchedules` | -| 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentAccessHooks`, `#installAttachmentReadVisibility` | -| 46 | Comment access hooks return early (insert + update + delete, and the read AST) | plugin-audit | Lose: comment visibility scoping | `packages/plugins/plugin-audit/src/comment-access-hooks.ts#installCommentAccessHooks`, `#installCommentReadVisibility` | -| 47 | Knowledge search returns hits unfiltered | service-knowledge | Lose: the permission filter over search results | `packages/services/service-knowledge/src/knowledge-service.ts#applyPermissionFilter` | +| 41 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `packages/plugins/plugin-approvals/src/lifecycle-hooks.ts#bindApprovalLockHook` | +| 42 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `packages/plugins/plugin-approvals/src/lifecycle-hooks.ts#bindDelegationWriteGuard` | +| 43 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `packages/plugins/plugin-approvals/src/approval-service.ts#isOverrideActor`, `#resolveActor`, `#sendBack`, `#resubmit`, `#reassign`, `#remind`, `#requestInfo`, `#comment` | +| 44 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `packages/plugins/plugin-reports/src/report-service.ts#saveReport` | +| 45 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `packages/plugins/plugin-reports/src/report-service.ts#assertExportAllowed`, `#canAccessReport`, `#listReports`, `#listSchedules` | +| 46 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `packages/services/service-storage/src/attachment-access-hooks.ts#installAttachmentAccessHooks`, `#installAttachmentReadVisibility` | +| 47 | Comment access hooks return early (insert + update + delete, and the read AST) | plugin-audit | Lose: comment visibility scoping | `packages/plugins/plugin-audit/src/comment-access-hooks.ts#installCommentAccessHooks`, `#installCommentReadVisibility` | +| 48 | Knowledge search returns hits unfiltered | service-knowledge | Lose: the permission filter over search results | `packages/services/service-knowledge/src/knowledge-service.ts#applyPermissionFilter` | ### 5. Actions, metadata plane, provenance, the organization wall | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `packages/runtime/src/action-execution.ts#callData` | -| 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `packages/runtime/src/action-execution.ts#actionPermissionError` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `packages/runtime/src/domains/meta.ts#handleMetadataRequest`, `packages/rest/src/rest-server.ts#registerMetadataEndpointsInner` | -| 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `packages/metadata-core/src/meta-write-capability.ts#metaWriteCapabilityVerdict` | -| 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `packages/runtime/src/domains/actions.ts#handleActionsRequest`, `packages/runtime/src/domains/ai.ts#handleAIRequest`, `packages/runtime/src/domains/automation.ts#handleAutomationRequest`, `packages/runtime/src/domains/meta.ts#handleMetadataRequest`, `packages/runtime/src/domains/security.ts#handleSecurityRequest`, `packages/runtime/src/domains/packages.ts#handlePackagesRequest`, `packages/rest/src/external-datasource-routes.ts#registerExternalDatasourceRoutes`, `packages/rest/src/package-routes.ts#refusePackageRequest` | -| 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `packages/runtime/src/domains/mcp.ts#handleMcpRequest` | -| 54 | Package REST route capability gate bypassed | rest | Get: a marketplace publish over REST (`POST /packages/publish`, the one route the REST registrar mounts since #14503) without `manage_metadata`; the package read cohort (`studio.access` / `setup.access`) is enforced by the dispatcher `/packages` domain's own read gate, where the reads are served | `packages/rest/src/package-routes.ts#refusePackageRequest` | -| 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `packages/runtime/src/domains/packages.ts#requireManageMetadata`, `#requireReadCapability` | -| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `packages/runtime/src/domains/activation-gate.ts#refuseUngrantedActivationWrite`, `#refuseUngrantedActivationAuthoring` | -| 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#refuseUnrelatedScreenRead` | -| 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts#assertTenantAdmin` | -| 59 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `packages/plugins/plugin-email/src/email-template-provenance.ts#bindEmailTemplateProvenanceStamp`, `packages/plugins/plugin-webhooks/src/webhook-provenance.ts#bindWebhookProvenanceStamp` | -| 60 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `packages/services/service-automation/src/runtime-identity.ts#stampSystemInsertOwner`, called from `packages/services/service-automation/src/builtin/crud-nodes.ts#registerCrudNodes` | -| 61 | Inbox caller refusal names `isSystem` as what was carried | service-messaging | Get: nothing — the refusal still fires. The flag only shapes the diagnostic, because privilege is not an authorization subject | `packages/services/service-messaging/src/inbox-caller.ts#resolveInboxRecipient` | -| 62 | **`organization_id` is not auto-stamped on INSERT** — the organization-axis twin of the `owner_id` gap above | organizations | Get: an elevated write may name another organization deliberately, which is what the per-organization seed replay, the orphan-row claim, imports and migrations all rely on. Lose: the authoritative stamp, so an elevated insert that names no organization lands `organization_id = NULL` and the wall hides it. ⛔ This is why a forged `organization_id` is overwritten on the non-elevated path and not here: elevation is the seam the legitimate cross-organization writers use | `packages/plugins/organizations/src/organizations-plugin.ts#start` | +| 49 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `packages/runtime/src/action-execution.ts#callData` | +| 50 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `packages/runtime/src/action-execution.ts#actionPermissionError` | +| 51 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `packages/runtime/src/domains/meta.ts#handleMetadataRequest`, `packages/rest/src/rest-server.ts#registerMetadataEndpointsInner` | +| 52 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 51's doors consult answers yes before any capability is examined | `packages/metadata-core/src/meta-write-capability.ts#metaWriteCapabilityVerdict` | +| 53 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `packages/runtime/src/domains/actions.ts#handleActionsRequest`, `packages/runtime/src/domains/ai.ts#handleAIRequest`, `packages/runtime/src/domains/automation.ts#handleAutomationRequest`, `packages/runtime/src/domains/meta.ts#handleMetadataRequest`, `packages/runtime/src/domains/security.ts#handleSecurityRequest`, `packages/runtime/src/domains/packages.ts#handlePackagesRequest`, `packages/rest/src/external-datasource-routes.ts#registerExternalDatasourceRoutes`, `packages/rest/src/package-routes.ts#refusePackageRequest` | +| 54 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `packages/runtime/src/domains/mcp.ts#handleMcpRequest` | +| 55 | Package REST route capability gate bypassed | rest | Get: a marketplace publish over REST (`POST /packages/publish`, the one route the REST registrar mounts since #14503) without `manage_metadata`; the package read cohort (`studio.access` / `setup.access`) is enforced by the dispatcher `/packages` domain's own read gate, where the reads are served | `packages/rest/src/package-routes.ts#refusePackageRequest` | +| 56 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `packages/runtime/src/domains/packages.ts#requireManageMetadata`, `#requireReadCapability` | +| 57 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `packages/runtime/src/domains/activation-gate.ts#refuseUngrantedActivationWrite`, `#refuseUngrantedActivationAuthoring` | +| 58 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `packages/runtime/src/domains/automation.ts#mayReadRunState`, `#refuseUngrantedFlowWrite`, `#refuseUnrelatedScreenRead` | +| 59 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts#assertTenantAdmin` | +| 60 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `packages/plugins/plugin-email/src/email-template-provenance.ts#bindEmailTemplateProvenanceStamp`, `packages/plugins/plugin-webhooks/src/webhook-provenance.ts#bindWebhookProvenanceStamp` | +| 61 | **Automation flow data nodes re-add the `owner_id` stamp** (the one place row 2's gap is compensated inline) | service-automation | Get: a flow-authored INSERT under system elevation still lands owned, when the run resolved a user. Fill-only — flow-authored values win | `packages/services/service-automation/src/runtime-identity.ts#stampSystemInsertOwner`, called from `packages/services/service-automation/src/builtin/crud-nodes.ts#registerCrudNodes` | +| 62 | Inbox caller refusal names `isSystem` as what was carried | service-messaging | Get: nothing — the refusal still fires. The flag only shapes the diagnostic, because privilege is not an authorization subject | `packages/services/service-messaging/src/inbox-caller.ts#resolveInboxRecipient` | +| 63 | **`organization_id` is not auto-stamped on INSERT** — the organization-axis twin of the `owner_id` gap above | organizations | Get: an elevated write may name another organization deliberately, which is what the per-organization seed replay, the orphan-row claim, imports and migrations all rely on. Lose: the authoritative stamp, so an elevated insert that names no organization lands `organization_id = NULL` and the wall hides it. ⛔ This is why a forged `organization_id` is overwritten on the non-elevated path and not here: elevation is the seam the legitimate cross-organization writers use | `packages/plugins/organizations/src/organizations-plugin.ts#start` | ### 6. Reads that only carry the flag onward @@ -187,10 +188,10 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 63 | `packages/objectql/src/engine.ts#buildSession` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 64 | `packages/objectql/src/engine.ts#isSystem` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | -| 65 | `packages/plugins/plugin-reports/src/report-service.ts#executeReport` | plugin-reports | Threads the flag into the engine call that runs a report | -| 66 | `packages/runtime/src/sandbox/body-runner.ts#executionContextFromHook` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | +| 64 | `packages/objectql/src/engine.ts#buildSession` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 65 | `packages/objectql/src/engine.ts#isSystem` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 66 | `packages/plugins/plugin-reports/src/report-service.ts#executeReport` | plugin-reports | Threads the flag into the engine call that runs a report | +| 67 | `packages/runtime/src/sandbox/body-runner.ts#executionContextFromHook` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | --- @@ -243,7 +244,7 @@ should recognise it instead of re-deriving it. a **bug**, because a sharing rule's declared semantics is a published promise and `isSystem` names the operator, never a consequence that need not happen. Both materialisation skips and the notice that announced them are - gone; row 30 is now the `afterDelete` skip alone, which survives on the + gone; row 31 is now the `afterDelete` skip alone, which survives on the separate ground that another subscriber delivers that payload. ⚠️ The observability half of that reading is worth keeping in mind @@ -252,11 +253,11 @@ should recognise it instead of re-deriving it. see it?". A compensating path that exists but that nobody can be expected to know about is not a compensating path. -3. **Strict write observability is inert under elevation.** Row 21: a caller +3. **Strict write observability is inert under elevation.** Row 22: a caller that asked to be told loudly about dropped fields is told nothing, because nothing was dropped. The two facts are indistinguishable from the outside. -4. **`revoke()` skips its own conflict guard.** Row 34 is correct for the rule +4. **`revoke()` skips its own conflict guard.** Row 35 is correct for the rule evaluator and surprising for anything else: a system caller can delete a rule-materialised grant that the next reconcile silently restores. @@ -277,7 +278,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are independent decisions, and a seed loader plausibly wants the first two but not the third. The concept is nevertheless **staying as one boolean**: -- **Shipped semantics.** `isSystem` is a published contract with 106 read sites +- **Shipped semantics.** `isSystem` is a published contract with 107 read sites in 20 packages. Splitting it is a breaking contract change across all of them. (The ruling was taken when the census read 80 sites in 18 packages; the count has grown, which strengthens rather than weakens the argument.) @@ -334,16 +335,16 @@ still holds equal to the census on every pull request: | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — | | — parsed as a declaration | 22 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | -| — parsed as a property **read** | 112 | ✅ | +| — parsed as a property **read** | 113 | ✅ | | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ | | — the remainder: text inside comments and string literals | 358 | — | | Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ | -| Of those reads: reads of `ExecutionContext.isSystem` | **106** | ✅ | -| — behaviour-bearing (rows 1–62 above) | 102 | ✅ | -| — carry the flag onward only (rows 63–66 above) | 4 | ✅ | +| Of those reads: reads of `ExecutionContext.isSystem` | **107** | ✅ | +| — behaviour-bearing (rows 1–63 above) | 103 | ✅ | +| — carry the flag onward only (rows 64–67 above) | 4 | ✅ | | Packages containing at least one elevation read | **20** | ✅ | | Files containing at least one elevation read | 45 | ✅ | -| — the distinct symbols those reads live in — what this page anchors | 89 | ✅ | +| — the distinct symbols those reads live in — what this page anchors | 90 | ✅ | | — of those files, the ones holding more than one read in one symbol | 9 | ✅ | The six rows marked — are a **dated decomposition, not a live claim**: they were @@ -449,4 +450,4 @@ that introduces it — CI will say so if it is not. - [Authorization Architecture](/docs/permissions/authorization) — the six-gate enforcement chain this flag short-circuits - [Security & Access Control](/docs/protocol/objectql/security) — the `readonly` write strip and its exemptions - [State Machine](/docs/protocol/objectql/state-machine) — `skipStateMachine`, `preserveAudit`, `treatAsHistorical` -- [Sharing Rules](/docs/permissions/sharing-rules) — what row 30 is skipping +- [Sharing Rules](/docs/permissions/sharing-rules) — what row 31 is skipping diff --git a/scripts/check-system-context-census.mjs b/scripts/check-system-context-census.mjs index dece047042..70f41481a5 100644 --- a/scripts/check-system-context-census.mjs +++ b/scripts/check-system-context-census.mjs @@ -431,39 +431,39 @@ export const NON_READ_ANCHORS = [ file: 'packages/objectql/src/engine.ts', symbol: 'buildDriverOptions', collapsesOntoRead: true, - why: 'row 23 -- the early return the tenant-audit read feeds, and where `bypassTenantAudit` is threaded to the driver', + why: 'row 24 -- the early return the tenant-audit read feeds, and where `bypassTenantAudit` is threaded to the driver', rowSeams: ['Tenant-audit warning silenced'], }, { file: 'packages/objectql/src/engine.ts', symbol: 'insert', collapsesOntoRead: true, - why: 'row 21 -- the strict-drop refusal that never fires under elevation, and the strip-before-validation block the validation row cites', + why: 'row 22 -- the strict-drop refusal that never fires under elevation, and the strip-before-validation block the validation row cites', rowSeams: ['Strict-drop refusal never fires'], }, { file: 'packages/objectql/src/readonly-strict-errors.ts', symbol: 'READONLY_CLASS_REASONS', - why: 'row 21 -- the reason set the silent refusal would have used', + why: 'row 22 -- the reason set the silent refusal would have used', rowSeams: ['Strict-drop refusal never fires'], }, { file: 'packages/plugins/plugin-security/src/system-write-guard.ts', symbol: 'assertEngineOwnedWriteAllowed', - why: 'row 24 -- the bypass expressed through a helper rather than a direct read', + why: 'row 25 -- the bypass expressed through a helper rather than a direct read', rowSeams: ['append-only write guard bypassed'], }, { file: 'packages/plugins/plugin-sharing/src/sharing-service.ts', symbol: 'revoke', collapsesOntoRead: true, - why: 'row 34 -- the CONFLICT guard `revoke()` deletes in front of, in the same function', + why: 'row 35 -- the CONFLICT guard `revoke()` deletes in front of, in the same function', rowSeams: ['`revoke()` deletes directly'], }, { file: 'packages/services/service-automation/src/builtin/crud-nodes.ts', symbol: 'registerCrudNodes', - why: 'row 60 -- the call site of the compensating owner stamp', + why: 'row 61 -- the call site of the compensating owner stamp', rowSeams: ['Automation flow data nodes re-add the `owner_id` stamp'], }, { @@ -2573,14 +2573,15 @@ function selfTest() { t( '⭐ ABLATION: one row inserted above row 34 turns the gate RED, naming the falsified page ' + 'references -- this is the exact edit #15687 made under a green gate', - falsifiedRefs.some((p) => p.includes('`Row 34`') && p.includes('is row 35')) && - falsifiedRefs.some((p) => p.includes('`rows 1–62`')), + falsifiedRefs.some((p) => p.includes('`Row 35`') && p.includes('is row 36')) && + falsifiedRefs.some((p) => p.includes('`rows 1–63`')), ablated.problems.join(' | ') ); t( - '⭐ ABLATION: and the `why:` strings for rows 34 and 60 -- the other two references #15687 falsified', - falsifiedWhy.some((p) => p.includes('`row 34`') && p.includes('is row 35')) && - falsifiedWhy.some((p) => p.includes('`row 60`') && p.includes('is row 61')), + '⭐ ABLATION: and the `why:` strings for the two `why:` references #15687 falsified ' + + '(the seams #15687 knew as rows 34 and 60; the page has since grown a row above them)', + falsifiedWhy.some((p) => p.includes('`row 35`') && p.includes('is row 36')) && + falsifiedWhy.some((p) => p.includes('`row 61`') && p.includes('is row 62')), falsifiedWhy.join(' | ') ); t( From 97d0ea087fb254af4b3e113e984fd2efb5f987ef Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 10:47:08 +0000 Subject: [PATCH 6/8] test(analytics): scope the routing case's warn assertion to the degradation it is about Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../src/__tests__/raw-sql-object-routing.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts b/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts index 7b64739edf..ee771ff7bf 100644 --- a/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts +++ b/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts @@ -254,7 +254,13 @@ describe('executeRawSql auto-bridge routes by object (#5033)', () => { { stage: 'won', deal_count: 2 }, { stage: 'lost', deal_count: 1 }, ]); - expect(warn).not.toHaveBeenCalled(); + // Scoped to the degradation this case is about, exactly as its sibling + // above spells it — ⛔ not a blanket "no warning at all". The analytics + // plugin now also reports at init when no `security` service is registered + // to answer the OBJECT-LEVEL read grant, and this fixture deliberately + // registers none; a blanket assertion would read that deliberate report as + // a routing regression. + expect(warn.mock.calls.map(String).join('\n')).not.toMatch(/is unavailable/); }); }); From 08825847fbdbacaf75714a6155eaadb64a283d2a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 14:56:46 +0000 Subject: [PATCH 7/8] fix(service-analytics): require `warn` on the admission sink and deny the two silent bridge corners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract review CHANGES REQUIRED on PR #16860, findings F1, F3 and F9. F1 — `AdmissionLogger` declared `error?` and `warn?`, which is a contract that permits silence (#9754). `warn` is now REQUIRED, so every value of the type carries a destination for a refusal report, and the fail-closed branch reaches for it when `error` is absent instead of dropping the report. F3 — the analytics -> `security` admission bridge collapsed three resolutions into one. A `getService('security')` that THROWS, and a registered service carrying neither `canReadObject` nor `explain`, both returned `undefined` and were then read as "no security service" — admitting the query silently. Those are wired-but-broken providers and `/data`'s middleware does not fall open in either state, so both now DENY and report at `error`. An ABSENT service still admits: that deployment has no object-level gate on `/data` either, so the two doors still agree, which is the property being defended. F9 — `raw-sql-object-routing.test.ts` now asserts that every warning in the fixture IS the deliberate admission-bridge init report, rather than only excluding the routing phrase. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...cs-inline-dataset-object-read-admission.md | 16 +- .../admission-bridge-resolution.test.ts | 206 ++++++++++++++++++ .../__tests__/raw-sql-object-routing.test.ts | 24 +- .../services/service-analytics/src/plugin.ts | 82 +++++-- .../service-analytics/src/read-admission.ts | 51 +++-- 5 files changed, 343 insertions(+), 36 deletions(-) create mode 100644 packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts diff --git a/.changeset/analytics-inline-dataset-object-read-admission.md b/.changeset/analytics-inline-dataset-object-read-admission.md index d1e21c82a9..880074199e 100644 --- a/.changeset/analytics-inline-dataset-object-read-admission.md +++ b/.changeset/analytics-inline-dataset-object-read-admission.md @@ -5,15 +5,27 @@ "@objectstack/verify": minor --- +fix(service-analytics)!: `POST /analytics/dataset/query` asks the OBJECT-level read grant before it serves an inline dataset (#16645) + + + +**BREAKING** in the accept-set sense — an accept-set narrowing on a published +route — landing in the launch window as `minor` on all four packages (the +lockstep convention: during the window the bump level is not the carrier, this +banner and the disposition above are). Nothing that was already admitted +becomes refused **except** the requests `GET /data/` refuses today for +the same principal, which is the defect. Nothing that was refused becomes +admitted. + `POST /analytics/dataset/query` now asks the OBJECT-level read grant before it serves an inline dataset, so the analytics door and `GET /data/` reach one admission verdict on every driver. The route accepts an inline dataset definition (`body.dataset`) from any authenticated caller. On a SQL driver the compiled statement ran through the driver's raw `execute()`, which is documented as a tenant-isolation bypass and which no middleware sits in front of — so the request reached the database having passed exactly ONE of the three read layers (the row scope, threaded since ADR-0021 D-C). A caller with **no grant of any kind** on an object received its row count, and with `dimensions` its grouped counts by any column, where the `/data` door answered `403 PERMISSION_DENIED` for the same principal on the same deployment. On the memory driver the identical request fell through to the ObjectQL engine, which applies all three layers in one place, and was refused. The exposure is not opt-in and an application cannot decline it: a deployment shipping 0 datasets and 0 dashboards has the identical surface, because the reachable slot is the inline definition rather than a declared one. -**This change NARROWS what the analytics doors accept.** Requests that were already refused by `/data` are now refused by analytics too; nothing that was refused becomes admitted. +**This change NARROWS what the analytics doors accept.** Requests that were already refused by `/data` are now refused by analytics too; nothing that was refused becomes admitted. "Fails closed" is a statement about a WIRED provider: a deployment with no `security` service registered keeps its previous analytics behaviour by design, because on that deployment `/data` carries no object-level gate either and the equivalence is what is being defended. - **`ISecurityService.canReadObject(object, context)`** (`@objectstack/spec`, optional) — the object-level half of a read, the sibling of `getReadFilter`'s row-level half. It exists because the two are not interchangeable: `getReadFilter` answers "which rows" and answers `undefined` — "no row restriction" — for a caller who may not read the object at all, so a door holding only the filter reads a caller with NO grant as a caller with NO restriction. Fails CLOSED. Absence is a defined state and its fallback is **not** "admit": a consumer composes the same verdict from `explain`, which is not optional. - **`@objectstack/plugin-security` implements it** as the middleware's own read gate, arm for arm and in its order — the `isSystem` bypass, the "no permission sets resolved" skip, the #3545 fail-closed refusal on an unresolvable object posture, the ADR-0066 D3 `requiredPermissions` capability AND-gate, the `allowRead` CRUD grant, and the ADR-0090 D10 delegator intersection — from the same primitives the middleware calls, and it is exposed on the registered `security` service. -- **`@objectstack/service-analytics` asks it once at the door**, for the base object and every joined object, **ahead of strategy selection**. Placement is the fix: two strategies each enforcing their own copy of three layers is the CAUSE of the divergence, not its remedy, so both strategies — and any strategy added later — inherit one verdict by construction. `AnalyticsServicePlugin` auto-bridges the new `admitObjectRead` hook to the `security` service (`canReadObject`, falling back to `explain`), the same way it already bridges `getReadScope`, and warns loudly at init when no security service is registered. +- **`@objectstack/service-analytics` asks it once at the door**, for the base object and every joined object, **ahead of strategy selection**. Placement is the fix: two strategies each enforcing their own copy of three layers is the CAUSE of the divergence, not its remedy, so both strategies — and any strategy added later — inherit one verdict by construction. `AnalyticsServicePlugin` auto-bridges the new `admitObjectRead` hook to the `security` service (`canReadObject`, falling back to `explain`), the same way it already bridges `getReadScope`, and warns loudly at init when no security service is registered. The bridge tells three resolutions apart: an ABSENT `security` service admits (that deployment has no object-level gate on `/data` either, so the two doors still agree, and this is what keeps a deployment shipping no `plugin-security` working as before); a service that cannot be USED — resolving it throws, or it exposes neither `canReadObject` nor `explain` — DENIES and reports at `error`, because `/data`'s middleware does not fall open in those states. - **`@objectstack/verify`** gains `bootStack(app, { databaseDriver: 'sqlite-wasm' | 'memory' })`, because a two-driver equivalence property cannot be measured on one driver — which is how the strategies were allowed to disagree. The refusal is `PERMISSION_DENIED` / 403, the same code and status the engine path already answers, and it names only the object the caller themselves named. 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 new file mode 100644 index 0000000000..9565faedef --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts @@ -0,0 +1,206 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The analytics → `security` admission bridge, and the three resolutions it + * must tell apart. + * + * The object-level gate at the analytics door is only as good as the answer the + * 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 at + * init; + * - 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 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`. + * + * ⛔ 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. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { AnalyticsServicePlugin } from '../plugin.js'; +import type { AnalyticsService } from '../analytics-service.js'; + +/** The reported probe's shape: one object, one count measure, no dimensions. */ +const probe = DatasetSchema.parse({ + name: 'probe_member', + label: 'probe', + object: 'employer_member', + dimensions: [], + measures: [{ name: 'cnt', label: 'Count', aggregate: 'count' }], +}); + +const CALLER = { userId: 'u_seeker', tenantId: 'org_a' } as ExecutionContext; + +/** The SQL posture — the reported path, where nothing else stands in the way. */ +const nativeSql = () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }); + +/** + * Engine double. Every read it serves is recorded, so a denial can be asserted + * as "the database was never reached" rather than only as a thrown envelope — + * a gate that refuses AFTER running the statement has not refused anything. + */ +function fakeEngine() { + const reads: string[] = []; + return { + reads, + engine: { + execute: async (sql: unknown, options?: { object?: string }) => { + reads.push(`execute:${options?.object ?? String(sql)}`); + return { rows: [{ cnt: 24 }] }; + }, + aggregate: async (object: string) => { + reads.push(`aggregate:${object}`); + return [{ cnt: 24 }]; + }, + getObject: (name: string) => + name === 'employer_member' ? { fields: { id: { type: 'text' } } } : undefined, + resolveEffectiveDatasource: () => undefined, + }, + }; +} + +/** + * Minimal `PluginContext`. `security` is supplied as a THUNK so a fixture can + * make the lookup itself throw — the corner that is otherwise unreachable from + * a plain service map. + */ +function fakePluginContext(opts: { + data: unknown; + security?: () => unknown; +}) { + const registered: Record = {}; + const warn = vi.fn(); + const error = vi.fn(); + return { + registered, + warn, + error, + ctx: { + getService: (name: string) => { + if (name === 'security') return opts.security ? opts.security() : undefined; + if (name === 'data') return opts.data; + return registered[name]; + }, + registerService: (name: string, svc: unknown) => { registered[name] = svc; }, + replaceService: (name: string, svc: unknown) => { registered[name] = svc; }, + logger: { info() {}, warn, error, debug() {} }, + }, + }; +} + +async function bootAnalytics(security?: () => unknown) { + const { engine, reads } = fakeEngine(); + const { ctx, registered, error } = fakePluginContext({ data: engine, security }); + await new AnalyticsServicePlugin({ queryCapabilities: nativeSql }).init(ctx as never); + return { service: registered.analytics as AnalyticsService, reads, error }; +} + +const runProbe = (service: AnalyticsService) => + service.queryDataset(probe as never, { measures: ['cnt'] } as never, CALLER); + +describe('analytics admission bridge — resolving the "security" service', () => { + // ── The two corners that used to admit silently ──────────────────────────── + + it('DENIES when resolving the "security" service THROWS', async () => { + const boom = () => { throw new Error('security service is initialising'); }; + const { service, reads, error } = await bootAnalytics(boom); + + await expect(runProbe(service)).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + }); + // The refusal has to happen BEFORE the statement runs, or it is not a gate. + expect(reads).toEqual([]); + // And it has to be findable. A security refusal nobody can see is + // indistinguishable from a gate that never ran. + expect(error.mock.calls.map((c) => String(c[0])).join('\n')).toMatch( + /read admission could not be resolved .* denying the query \(fail-closed\).*threw/s, + ); + }); + + it('DENIES when the "security" service exposes neither canReadObject nor explain', async () => { + // A registered object that is not the contract it claims to be — + // `explain` is NON-optional on `ISecurityService`, so a conforming + // provider never lands here. + const { service, reads, error } = await bootAnalytics(() => ({ getReadFilter: () => undefined })); + + await expect(runProbe(service)).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + }); + expect(reads).toEqual([]); + expect(error.mock.calls.map((c) => String(c[0])).join('\n')).toMatch( + /read admission could not be resolved .* neither canReadObject\(\) nor explain\(\)/s, + ); + }); + + // ── The negative control: absence is a different state and still ADMITS ──── + + 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`. + const { service, reads } = await bootAnalytics(undefined); + + const result = await runProbe(service); + expect(result.rows).toEqual([{ cnt: 24 }]); + expect(reads).toHaveLength(1); + }); + + // ── The two working spellings, so the deny cases cannot pass by refusing all ─ + + it('asks canReadObject when the service has it, and serves an ADMITTED caller', async () => { + const canReadObject = vi.fn(() => true); + const { service, reads } = await bootAnalytics(() => ({ canReadObject })); + + const result = await runProbe(service); + expect(result.rows).toEqual([{ cnt: 24 }]); + expect(canReadObject).toHaveBeenCalledWith('employer_member', CALLER); + expect(reads).toHaveLength(1); + }); + + it('refuses through canReadObject when that service answers false', async () => { + const { service, reads } = await bootAnalytics(() => ({ canReadObject: () => false })); + + await expect(runProbe(service)).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + }); + expect(reads).toEqual([]); + }); + + it('falls back to explain for a service that predates canReadObject — both verdicts', async () => { + const admitted = await bootAnalytics(() => ({ + explain: async () => ({ allowed: true }), + })); + expect((await runProbe(admitted.service)).rows).toEqual([{ cnt: 24 }]); + + const refused = await bootAnalytics(() => ({ + explain: async () => ({ allowed: false }), + })); + await expect(runProbe(refused.service)).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + }); + expect(refused.reads).toEqual([]); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts b/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts index ee771ff7bf..9cbb9fec81 100644 --- a/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts +++ b/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts @@ -254,13 +254,23 @@ describe('executeRawSql auto-bridge routes by object (#5033)', () => { { stage: 'won', deal_count: 2 }, { stage: 'lost', deal_count: 1 }, ]); - // Scoped to the degradation this case is about, exactly as its sibling - // above spells it — ⛔ not a blanket "no warning at all". The analytics - // plugin now also reports at init when no `security` service is registered - // to answer the OBJECT-LEVEL read grant, and this fixture deliberately - // registers none; a blanket assertion would read that deliberate report as - // a routing regression. - expect(warn.mock.calls.map(String).join('\n')).not.toMatch(/is unavailable/); + // The blanket `expect(warn).not.toHaveBeenCalled()` this replaces could no + // longer hold: the plugin reports at init when no `security` service is + // registered to answer the OBJECT-LEVEL read grant, and this fixture + // deliberately registers none. + // + // ⛔ But the replacement is not "anything except the routing phrase" + // either — that admits every OTHER new warning into a case whose whole job + // is to prove this object's routing did not regress. Every warning that + // fires here must BE the one deliberate report, named; a second one, of any + // wording, fails this case. + const warnings = warn.mock.calls.map((c) => String(c[0])); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch( + /No admitObjectRead configured and no "security" service registered at init/, + ); + // …and it is emphatically not the degradation this case is about. + expect(warnings[0]).not.toMatch(/is unavailable/); }); }); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 4ba7e31b02..b5d20d49e0 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -524,10 +524,31 @@ export class AnalyticsServicePlugin implements Plugin { // heavier call, which is why it is the fallback and not the primary; it // never fires against an in-repo stack. // - // A deployment with NO security service at all gets no gate — and no - // object-level gate on `/data` either, since that gate IS this plugin's - // absent middleware — so the two doors still agree. That state is reported - // at init below. + // ## 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. + // + // 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 at init below. + // 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. + // USABLE — ask it (below). + // + // 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. interface SecurityReadAdmission { canReadObject?(object: string, context?: ExecutionContext): boolean | Promise; explain?( @@ -535,25 +556,58 @@ export class AnalyticsServicePlugin implements Plugin { callerContext?: ExecutionContext, ): Promise<{ allowed?: boolean }>; } + type SecurityAdmissionResolution = + | { kind: 'usable'; svc: SecurityReadAdmission } + | { kind: 'absent' } + | { kind: 'unusable'; why: string }; let admitObjectRead = this.options.admitObjectRead; let autoBridgedReadAdmission = false; if (!admitObjectRead) { - const trySecurityAdmission = (): SecurityReadAdmission | undefined => { + const trySecurityAdmission = (): SecurityAdmissionResolution => { + let svc: SecurityReadAdmission | undefined; try { - const svc = ctx.getService('security'); - if (!svc) return undefined; - return typeof svc.canReadObject === 'function' || typeof svc.explain === 'function' - ? svc - : undefined; - } catch { - return undefined; + svc = ctx.getService('security'); + } catch (e) { + // ⛔ Not `absent`. A throwing resolver is a service that exists and + // failed, and a failed security lookup is a refusal everywhere else + // in this stack. + return { + kind: 'unusable', + why: + `resolving the "security" service threw ` + + `(${String((e as Error)?.message ?? e)})`, + }; + } + if (!svc) return { kind: 'absent' }; + if (typeof svc.canReadObject !== 'function' && typeof svc.explain !== 'function') { + // A registered service that answers neither question cannot admit + // anything. `explain` is NON-optional on `ISecurityService`, so a + // conforming provider never lands here — reaching it means the + // registered object is not the contract it claims to be. + return { + kind: 'unusable', + why: + 'the registered "security" service exposes neither canReadObject() ' + + 'nor explain(), so it cannot answer an object-level read admission', + }; } + return { kind: 'usable', svc }; }; admitObjectRead = async (object, context) => { - const svc = trySecurityAdmission(); + const resolved = trySecurityAdmission(); // No security service resolved at call time → no object-level gate on // this deployment, which is the state reported at init. - if (!svc) return true; + if (resolved.kind === 'absent') return true; + if (resolved.kind === 'unusable') { + ctx.logger.error( + `[Analytics] object-level read admission could not be resolved for "${object}" — ` + + `denying the query (fail-closed): ${resolved.why}. ` + + 'A security service is wired on this deployment, so analytics must not fall open: ' + + 'GET /data/' + object + ' does not.', + ); + return false; + } + const svc = resolved.svc; if (typeof svc.canReadObject === 'function') { return await svc.canReadObject(object, context); } diff --git a/packages/services/service-analytics/src/read-admission.ts b/packages/services/service-analytics/src/read-admission.ts index 7f78500da4..bf385d5a11 100644 --- a/packages/services/service-analytics/src/read-admission.ts +++ b/packages/services/service-analytics/src/read-admission.ts @@ -48,12 +48,21 @@ * * ## Fail direction * - * The provider is access-NARROWING, so it fails CLOSED: a provider that throws - * denies the query rather than admitting it. An ABSENT provider is a different - * state — it means no security service answered at all, which is the same - * deployment in which `/data` has no object-level gate either, so the two doors - * still agree. `AnalyticsServicePlugin` logs that state loudly at init, the - * same posture it already takes for a missing `getReadScope`. + * The provider is access-NARROWING, so it fails CLOSED — but "closed" is a + * claim about a WIRED provider, and it is worth saying which states are which: + * + * - a wired provider that THROWS, or answers `false`, denies (this module); + * - a `security` service that is wired but cannot be used — resolving it + * throws, or it exposes neither `canReadObject` nor `explain` — denies at + * the bridge, because `/data`'s middleware does not fall open in those + * states either (`plugin.ts`); + * - an ABSENT provider is a different state altogether. No security service + * answered at all, which is the same deployment in which `/data` has no + * object-level gate either, so the two doors still AGREE — and agreement is + * the property being defended, not refusal for its own sake. Such a + * deployment keeps its pre-existing analytics behaviour, and + * `AnalyticsServicePlugin` logs that state loudly at init, the same posture + * it already takes for a missing `getReadScope`. */ import type { ExecutionContext } from '@objectstack/spec/kernel'; @@ -104,10 +113,22 @@ export type ObjectReadAdmissionProvider = ( context?: ExecutionContext, ) => boolean | Promise; -/** Log sink — the subset of `Logger` this module uses. */ +/** + * Log sink — the subset of `Logger` this module uses. + * + * `error` is OPTIONAL because hosts legitimately inject reduced sinks, and + * `warn` is REQUIRED because of that: a sink declaring an optional `error` and + * no guaranteed alternative is a contract that PERMITS SILENCE (#9754, + * `check:optional-error-sink`). Every value of this type therefore has a + * destination for a refusal report, and {@link assertObjectsReadable} reaches + * for it when `error` is absent rather than dropping the report. A denial this + * module makes is never allowed to be invisible: it is the one record that a + * request was refused, and a security refusal nobody can see is + * indistinguishable from a gate that never ran. + */ interface AdmissionLogger { error?(message: string, error?: Error): void; - warn?(message: string): void; + warn(message: string): void; } /** @@ -137,15 +158,19 @@ export async function assertObjectsReadable( } catch (e) { // Fail CLOSED. A resolution failure must deny — admitting on an error is // the shape this whole module exists to remove. - logger?.error?.( + const cause = e instanceof Error ? e : new Error(String(e)); + const report = `[Analytics] read-admission resolution failed for object "${objectName}" — ` + - `denying query (fail-closed)`, - e instanceof Error ? e : new Error(String(e)), - ); + `denying query (fail-closed)`; + // `error` is the right level for a gate that could not reach a verdict, + // but it is optional on this sink; `warn` is not, so the report lands + // either way. This is the guarantee the required `warn` above buys. + if (logger?.error) logger.error(report, cause); + else logger?.warn(`${report}: ${cause.message}`); throw readAdmissionDeniedError(objectName); } if (!admitted) { - logger?.warn?.( + logger?.warn( `[Analytics] object-level read admission denied for "${objectName}" ` + `(user ${String((context as { userId?: unknown } | undefined)?.userId ?? 'unknown')}) — ` + `the same verdict GET /data/${objectName} reaches`, From 6d6fbf51eac22f3e16269f03369ae3f9a2a4657a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 15:43:40 +0000 Subject: [PATCH 8/8] docs(permissions): re-derive the system-context census on the merged tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge of #16755 is the case the census gate exists to catch and the merge driver cannot: both sides had independently bumped the SAME declared counts from 106 to 107, so every one of them text-merged cleanly to 107 while the merged tree now holds 108 elevation reads — main's new `#refuseUngrantedRunLifecycleWrite` plus this branch's `canReadObject`. Two correct edits, one wrong sum, and no conflict marker anywhere near it. Re-derived from the merged tree, after the merge was committed and never during MERGE state. Seven declared counts move by one; the gate names each and states there is no mechanical repair, so each was corrected by hand: check-system-context-census: OK — 108 elevation read sites in 20 packages across 45 files, living in 91 symbol(s); the page cites 105 symbol(s) against 105 required, over 129 anchors and 8 file-level citation(s) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- content/docs/permissions/system-context.mdx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 575f82ac81..85f6a9b610 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -9,7 +9,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a service self-write, a migration. This page is **the authority** for what that flag actually does. It exists -because the flag is not one concept: it is a single boolean read at **107 +because the flag is not one concept: it is a single boolean read at **108 distinct sites across 20 packages**, and knowing three of those behaviours gives no hint that the other hundred-and-four exist. Every documented app-side bug traced to `isSystem` had the same shape — the metadata was complete and correct, @@ -132,7 +132,7 @@ that silently does not happen. ### 3. Sharing (`plugin-sharing`) -The largest single consumer — **17 of the 107 sites**. +The largest single consumer — **17 of the 108 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| @@ -278,7 +278,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are independent decisions, and a seed loader plausibly wants the first two but not the third. The concept is nevertheless **staying as one boolean**: -- **Shipped semantics.** `isSystem` is a published contract with 107 read sites +- **Shipped semantics.** `isSystem` is a published contract with 108 read sites in 20 packages. Splitting it is a breaking contract change across all of them. (The ruling was taken when the census read 80 sites in 18 packages; the count has grown, which strengthens rather than weakens the argument.) @@ -335,16 +335,16 @@ still holds equal to the census on every pull request: | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — | | — parsed as a declaration | 22 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | -| — parsed as a property **read** | 113 | ✅ | +| — parsed as a property **read** | 114 | ✅ | | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ | | — the remainder: text inside comments and string literals | 358 | — | | Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ | -| Of those reads: reads of `ExecutionContext.isSystem` | **107** | ✅ | -| — behaviour-bearing (rows 1–63 above) | 103 | ✅ | +| Of those reads: reads of `ExecutionContext.isSystem` | **108** | ✅ | +| — behaviour-bearing (rows 1–63 above) | 104 | ✅ | | — carry the flag onward only (rows 64–67 above) | 4 | ✅ | | Packages containing at least one elevation read | **20** | ✅ | | Files containing at least one elevation read | 45 | ✅ | -| — the distinct symbols those reads live in — what this page anchors | 90 | ✅ | +| — the distinct symbols those reads live in — what this page anchors | 91 | ✅ | | — of those files, the ones holding more than one read in one symbol | 9 | ✅ | The six rows marked — are a **dated decomposition, not a live claim**: they were