diff --git a/.changeset/7750-retire-datascopemanager.md b/.changeset/7750-retire-datascopemanager.md new file mode 100644 index 0000000000..bb5fbab561 --- /dev/null +++ b/.changeset/7750-retire-datascopemanager.md @@ -0,0 +1,53 @@ +--- +'@object-ui/core': minor +--- + +**BREAKING:** `@object-ui/core` no longer exports `DataScopeManager` or the +row-level filter vocabulary that came with it (objectui#7750). This narrows the +package's published surface. It is declared `minor`, not `major`, because this +fixed release group follows the `@objectstack` major (AGENTS.md §9); the +breaking change is stated here instead. + +Removed, with no replacement and no deprecated alias: + +- `DataScopeManager`, the class +- `defaultDataScopeManager`, its shared instance +- `RowLevelFilter`, the type of one row-level rule +- `DataScopeConfig`, the type `registerScopeWithConfig` took; nothing else in + the package read it + +Why they went: `DataScopeManager` carried a third hand-written row-level +security evaluator, beside `evaluateCondition` in `@object-ui/permissions` and +the platform's own. The platform keeps one row-level security language: the CEL +predicate a `RowLevelSecurityPolicySchema` policy declares in its `using` +clause (`@objectstack/spec`), which the server lowers to an ObjectQL filter and +which fails closed when it does not lower. Aligning this class's operator +vocabulary with the spec's instead was considered and refused: on a permission +boundary it would have turned refused operators into evaluated ones, three of +them with silently different meanings. On this change's base, no code in this +repository constructed a `DataScopeManager` or a `RowLevelFilter` outside the +class's own tests and its documentation examples, and the downstream readings +recorded on objectui#7750 found no consumer in `objectstack`, `hotcrm` or +`cloud`. The maintainer ruled to retire it; the ruling is on objectui#7750. + +Not affected: the rest of `data-scope/` stays exported, `ViewDataProvider` and +the element data-source helpers (`composeElementDataSource`, +`resolveSavedView` and their types) included. The `DataScope` and `DataContext` +interfaces in `@object-ui/types` are unchanged. + +Other entries in this same release describe work on `DataScopeManager`: the +`@object-ui/core` entries for objectui#7378 (an unknown operator denies the +row) and objectui#7751 (the own-member field read and the same-kind ordered +comparison). That work shipped in a class this entry removes. The +`@object-ui/permissions` entry for objectui#8044 says `evaluateCondition` now +gives the same answer as `DataScopeManager` for a prototype-named field; the +guard it describes stays in `@object-ui/permissions`, and after this release it +is the only one of the two evaluators left. + +**Migration:** there is no replacement in `@object-ui/core`. Declare row-level +security on the server, as a `RowLevelSecurityPolicySchema` policy whose +`using` clause is a CEL predicate; the platform lowers it to an ObjectQL filter +and enforces it fail-closed. If you used `DataScopeManager` only as a registry +of named scopes, copy `packages/core/src/data-scope/DataScopeManager.ts` from a +release tag that still ships it, for example `@object-ui/core@17.5.0`, into +your own code. diff --git a/content/docs/guide/architecture-overview.md b/content/docs/guide/architecture-overview.md index 9f87874b09..ae1a9fa7d5 100644 --- a/content/docs/guide/architecture-overview.md +++ b/content/docs/guide/architecture-overview.md @@ -308,7 +308,7 @@ packages/ │ ├── actions/ # ActionRunner, TransactionManager │ ├── validation/ # Schema validation engine │ ├── adapters/ # Data source adapters (API, Value) -│ ├── data-scope/ # DataScopeManager +│ ├── data-scope/ # ViewDataProvider, element data sources │ ├── query/ # Query AST (filtering/sorting) │ ├── theme/ # ThemeEngine │ └── builder/ # Schema builder utilities diff --git a/packages/core/README.md b/packages/core/README.md index 245641e967..ef17a97325 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -7,7 +7,7 @@ Core logic, types, and validation for Object UI. Zero React dependencies. - 🎯 **Type Definitions** - Re-exported runtime types; the component schema vocabulary itself is `@object-ui/types` - 🔍 **Component Registry** - Framework-agnostic component registration system -- 📊 **Data Scope** - Data scope management and expression evaluation +- 🧮 **Expressions** - `${...}` expression evaluation against a context - ✅ **Validation** - Zod-based schema validation - 🚀 **Zero React** - Can run in Node.js or any JavaScript environment @@ -60,19 +60,13 @@ here is renderable from schema anywhere in the app. `register()`'s second argument is the component itself; registration metadata is its optional third argument, and `getMeta()` — not `get()` — reads that metadata back. -### Data Scope +### Expressions -`DataScopeManager` owns the named scopes a component tree reads from, and -`evaluateExpression` evaluates a `${...}` expression against a context. They -are separate exports: a scope holds data, it does not evaluate. +`evaluateExpression` evaluates a `${...}` expression against a context. ```typescript -import { DataScopeManager, evaluateExpression } from '@object-ui/core' +import { evaluateExpression } from '@object-ui/core' -const manager = new DataScopeManager() -manager.registerScope('user', { data: { name: 'John', role: 'admin' } }) - -const userName = manager.getScope('user')?.data.name // 'John' const isAdmin = evaluateExpression('${user.role === "admin"}', { user: { name: 'John', role: 'admin' }, }) // true diff --git a/packages/core/src/data-scope/DataScopeManager.ts b/packages/core/src/data-scope/DataScopeManager.ts deleted file mode 100644 index c3019ee922..0000000000 --- a/packages/core/src/data-scope/DataScopeManager.ts +++ /dev/null @@ -1,401 +0,0 @@ -/** - * ObjectUI - * Copyright (c) 2024-present ObjectStack Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -/** - * @object-ui/core - DataScope Manager - * - * Runtime implementation of the DataContext interface for managing - * named data scopes. Provides row-level data access control and - * reactive data state management within the UI component tree. - * - * @module data-scope - * @packageDocumentation - */ - -import type { DataScope, DataContext, DataSource } from '@object-ui/types'; - -/** - * Row-level filter for restricting data access within a scope - */ -export interface RowLevelFilter { - /** - * Field to filter on. Read as an OWN member of the record and nothing else: - * a name that resolves on the prototype chain instead is refused, and the - * rule denies the row. See `readField`. - */ - field: string; - /** - * Filter operator. The set is closed: a rule whose operator is outside it - * (possible for a rule read back from stored JSON, which the type does not - * guard) evaluates to `false` and denies the row. - */ - operator: 'eq' | 'ne' | 'gt' | 'lt' | 'gte' | 'lte' | 'in' | 'nin' | 'contains'; - /** - * Filter value. The ordered operators (`gt` / `gte` / `lt` / `lte`) compare - * it against the field value WITHOUT coercion — both sides must be the same - * comparable kind or the rule denies the row. See `isOrderedPair`. - */ - value: any; -} - -/** - * Configuration for creating a data scope - */ -export interface DataScopeConfig { - /** Data source instance */ - dataSource?: DataSource; - /** Initial data */ - data?: any; - /** Row-level filters to apply */ - filters?: RowLevelFilter[]; - /** Whether this scope is read-only */ - readOnly?: boolean; -} - -/** - * DataScopeManager — Runtime implementation of DataContext. - * - * Manages named data scopes for the component tree, providing: - * - Scope registration and lookup - * - Row-level security via filters - * - Data state management (data, loading, error) - * - * @example - * ```ts - * const manager = new DataScopeManager(); - * manager.registerScope('contacts', { - * dataSource: myDataSource, - * data: [], - * }); - * const scope = manager.getScope('contacts'); - * ``` - */ -export class DataScopeManager implements DataContext { - scopes: Record = {}; - private filters: Record = {}; - private readOnlyScopes: Set = new Set(); - private listeners: Map void>> = new Map(); - - /** - * Register a data scope - */ - registerScope(name: string, scope: DataScope): void { - this.scopes[name] = scope; - this.notifyListeners(name, scope); - } - - /** - * Register a data scope with configuration - */ - registerScopeWithConfig(name: string, config: DataScopeConfig): void { - const scope: DataScope = { - dataSource: config.dataSource, - data: config.data, - loading: false, - error: null, - }; - - if (config.filters) { - this.filters[name] = config.filters; - } - - if (config.readOnly) { - this.readOnlyScopes.add(name); - } - - this.scopes[name] = scope; - this.notifyListeners(name, scope); - } - - /** - * Get a data scope by name - */ - getScope(name: string): DataScope | undefined { - return this.scopes[name]; - } - - /** - * Remove a data scope - */ - removeScope(name: string): void { - delete this.scopes[name]; - delete this.filters[name]; - this.readOnlyScopes.delete(name); - this.listeners.delete(name); - } - - /** - * Check if a scope is read-only - */ - isReadOnly(name: string): boolean { - return this.readOnlyScopes.has(name); - } - - /** - * Get row-level filters for a scope - */ - getFilters(name: string): RowLevelFilter[] { - return this.filters[name] || []; - } - - /** - * Set row-level filters for a scope - */ - setFilters(name: string, filters: RowLevelFilter[]): void { - this.filters[name] = filters; - } - - /** - * Apply row-level filters to a dataset - */ - applyFilters(name: string, data: any[]): any[] { - const scopeFilters = this.filters[name]; - if (!scopeFilters || scopeFilters.length === 0) { - return data; - } - - return data.filter(row => { - return scopeFilters.every(filter => { - const read = readField(row, filter.field); - // Fail closed, on the #7378 principle: a rule this evaluator cannot - // answer FROM THE RECORD must not admit the row it exists to hide. - if (!read.readable) return false; - return evaluateFilter(read.value, filter.operator, filter.value); - }); - }); - } - - /** - * Update data in a scope - */ - updateScopeData(name: string, data: any): void { - const scope = this.scopes[name]; - if (!scope) return; - - if (this.readOnlyScopes.has(name)) { - throw new Error(`Cannot update read-only scope: ${name}`); - } - - scope.data = data; - this.notifyListeners(name, scope); - } - - /** - * Update loading state for a scope - */ - updateScopeLoading(name: string, loading: boolean): void { - const scope = this.scopes[name]; - if (!scope) return; - - scope.loading = loading; - this.notifyListeners(name, scope); - } - - /** - * Update error state for a scope - */ - updateScopeError(name: string, error: Error | string | null): void { - const scope = this.scopes[name]; - if (!scope) return; - - scope.error = error; - this.notifyListeners(name, scope); - } - - /** - * Subscribe to scope changes - */ - onScopeChange(name: string, listener: (scope: DataScope) => void): () => void { - if (!this.listeners.has(name)) { - this.listeners.set(name, []); - } - this.listeners.get(name)!.push(listener); - - return () => { - const arr = this.listeners.get(name); - if (arr) { - const idx = arr.indexOf(listener); - if (idx >= 0) arr.splice(idx, 1); - } - }; - } - - /** - * Get all registered scope names - */ - getScopeNames(): string[] { - return Object.keys(this.scopes); - } - - /** - * Clear all scopes - */ - clear(): void { - this.scopes = {}; - this.filters = {}; - this.readOnlyScopes.clear(); - this.listeners.clear(); - } - - private notifyListeners(name: string, scope: DataScope): void { - const arr = this.listeners.get(name); - if (arr) { - arr.forEach(listener => listener(scope)); - } - } -} - -/** - * Field names that are never record data, whatever the record looks like. - * - * `prototype` earns its place separately from the other two: it is NOT present - * on a plain object's chain (`'prototype' in {}` is `false`), so the own-member - * rule below would classify it as an ordinary absent field. Naming it here - * refuses it outright, the way `evaluateCondition` in `@object-ui/permissions` - * refuses all three. - */ -const PROTOTYPE_FIELD_NAMES: ReadonlySet = new Set([ - '__proto__', - 'constructor', - 'prototype', -]); - -/** - * The outcome of reading a rule's field off a record. - * - * `readable: false` is not "the value was falsy" — it is "this evaluator - * refuses to answer this rule from this record", which `applyFilters` turns - * into a denial. - */ -type FieldRead = { readable: true; value: unknown } | { readable: false }; - -/** - * Read a rule's field as an OWN member of the record. - * - * Measured on the pre-fix source (objectui#7751): a rule naming a prototype - * member evaluated against the prototype chain rather than the record, and on - * a negative operator that admitted EVERY row — - * `{ field: 'constructor', operator: 'ne', value: 'x' }` returned the whole - * dataset, silently. A fail-open on a row-level permission boundary. - * - * Three cases, and the third is why this is not simply `hasOwnProperty`: - * - * 1. A name in `PROTOTYPE_FIELD_NAMES` — refused outright. - * 2. An own member — its value, which is the only value ever read. - * 3. Not an own member. Here the record is asked whether the name resolves - * on its prototype chain at all: - * - it does (`toString`, `valueOf`, an `Object.create` parent's field) - * → REFUSED. The value exists but is not this record's data. - * - it does not → the field is genuinely absent, and `undefined` is - * returned exactly as before, so the ordinary "this row has no - * `status`" rules keep the verdicts they have always had. - * - * Case 3 is where this went further than the sibling. Reading with - * `hasOwnProperty` alone collapses "inherited" into "absent", and on a - * negative operator absent ADMITS: `{ field: 'toString', operator: 'ne' }` - * admitted every row through `evaluateCondition` in `@object-ui/permissions` - * — measured — because `toString` was not one of the three names its list - * refused. objectui#8044 ported this same three-case read there, so the two - * evaluators now agree. Distinguishing inherited from absent closes the whole - * class rather than three spellings of it, and it is what keeps this change a - * NARROWING: collapsing inherited into absent would have flipped - * inherited-value rows from denied to admitted on `ne` / `nin`. - * - * A `null` / `undefined` row still throws from the `hasOwnProperty` call, as - * the direct `row[field]` access it replaces did. - */ -function readField(row: any, field: string): FieldRead { - if (PROTOTYPE_FIELD_NAMES.has(field)) return { readable: false }; - if (Object.prototype.hasOwnProperty.call(row, field)) return { readable: true, value: row[field] }; - if (field in Object(row)) return { readable: false }; - return { readable: true, value: undefined }; -} - -/** - * Realm-safe `Date` test — `instanceof` answers `false` for a `Date` from - * another realm (an iframe, a worker, a VM context), and a row-level rule - * silently denying every row there would be the same class of bug this file - * keeps paying for. - */ -function isDate(value: unknown): boolean { - return Object.prototype.toString.call(value) === '[object Date]'; -} - -/** - * May these two values be compared with `<` / `>` without JavaScript coercing - * one of them? - * - * Measured on the pre-fix source (objectui#7751): `{ field: 'age', - * operator: 'gte', value: 0 }` admitted `null`, `'10'`, `true`, `false`, `''` - * and `[]` — every one of them by coercion to a number the rule's author never - * wrote. Requiring the two sides to be the same comparable kind refuses all of - * them. - * - * Same KIND, not "both numbers". `evaluateCondition` in - * `@object-ui/permissions` requires `typeof === 'number'` on both sides, and - * copying that line here would deny every row for `{ field: 'created', - * operator: 'gte', value: '2023-01-01' }` — ISO date strings, plain string - * ranges and `Date` objects all order correctly on this evaluator today - * (measured), and none of those comparisons coerces anything. The hazard is - * cross-kind comparison, so cross-kind is what this refuses; the sibling's - * extra strictness is not part of the property and it costs real rules. - */ -function isOrderedPair(a: unknown, b: unknown): boolean { - if (typeof a === 'number' && typeof b === 'number') return true; - if (typeof a === 'string' && typeof b === 'string') return true; - return isDate(a) && isDate(b); -} - -/** - * Evaluate a single filter condition against a field value. - * - * An operator the switch does not implement evaluates to `false` (fail - * closed); see the `default` arm. - */ -function evaluateFilter(fieldValue: any, operator: RowLevelFilter['operator'], filterValue: any): boolean { - switch (operator) { - case 'eq': - return fieldValue === filterValue; - case 'ne': - return fieldValue !== filterValue; - case 'gt': - return isOrderedPair(fieldValue, filterValue) && fieldValue > filterValue; - case 'lt': - return isOrderedPair(fieldValue, filterValue) && fieldValue < filterValue; - case 'gte': - return isOrderedPair(fieldValue, filterValue) && fieldValue >= filterValue; - case 'lte': - return isOrderedPair(fieldValue, filterValue) && fieldValue <= filterValue; - case 'in': - return Array.isArray(filterValue) && filterValue.includes(fieldValue); - case 'nin': - return Array.isArray(filterValue) && !filterValue.includes(fieldValue); - case 'contains': - // The rule's value is required to BE a string rather than be turned into - // one: `String(filterValue)` made `{ operator: 'contains', value: 1 }` - // match the record `'10'`, which is the same unwritten coercion the - // ordered arms above just stopped doing. `evaluateCondition` in - // `@object-ui/permissions` already required both sides to be strings. - return typeof fieldValue === 'string' && typeof filterValue === 'string' && fieldValue.includes(filterValue); - default: - // Fail closed. A row-level rule this evaluator cannot answer must not - // admit the row it exists to hide: the same answer `evaluateCondition` - // in @object-ui/permissions gives from its own `default` arm, and the - // opposite of the admit-all this arm used to return. The declared union - // above keeps TypeScript callers off this arm; a rule read back from - // stored JSON arrives as a plain string and is not protected by it. - // Silent, like the sibling: the caller sees a narrower result set, not - // a thrown error (objectui#7378). - return false; - } -} - -/** - * Default DataScopeManager instance - */ -export const defaultDataScopeManager = new DataScopeManager(); diff --git a/packages/core/src/data-scope/__tests__/DataScopeManager.hardening.test.ts b/packages/core/src/data-scope/__tests__/DataScopeManager.hardening.test.ts deleted file mode 100644 index 2bbe099fd6..0000000000 --- a/packages/core/src/data-scope/__tests__/DataScopeManager.hardening.test.ts +++ /dev/null @@ -1,291 +0,0 @@ -/** - * ObjectUI - * Copyright (c) 2024-present ObjectStack Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -/** - * The hardening standard for row-level rule evaluation, and the pin that keeps - * this repository's two evaluators of that kind from diverging a third time. - * - * The two divergences already paid for: - * - objectui#7378 — this file's `default` arm returned `true`, so an operator - * spelling it did not implement admitted every row. - * - objectui#7751 — this file read the field off the record unguarded, so a - * prototype member name admitted every row, and compared without a type - * guard, so `null` and `'10'` satisfied a numeric rule. - * - * Both were found by comparing against `evaluateCondition` in - * `@object-ui/permissions`, and both were the same failure direction: ADMIT, - * silently, on a permission boundary. So the standard is written here as a - * shared table rather than as prose, and the last describe block runs it - * against BOTH evaluators. - * - * ⛔ What this file deliberately takes no position on: operator SPELLING. - * `ne`/`nin` here, `neq`/`not_in` there, and the sibling's `is_null` / - * `is_not_null` which this evaluator does not implement. That divergence is - * objectui#7750's question and it is in the decision box. The conformance - * table below therefore addresses each evaluator in its own spelling through - * an explicit map, rather than asserting either vocabulary is the right one. - */ - -import { describe, it, expect } from 'vitest'; -import { DataScopeManager, type RowLevelFilter } from '../DataScopeManager'; -// Cross-package relative import, on purpose: `evaluateCondition` is NOT part of -// `@object-ui/permissions`' public entry (its `index.ts` exports -// `evaluatePermission` only), so there is no bare specifier that reaches it, -// and a conformance table that cannot see the other evaluator cannot pin -// anything. Same shape as the two existing cross-package test imports on main -// (`activityItemType-6730.test.ts`, `degradeSetTwin-5880.test.ts`). -import { evaluateCondition } from '../../../../permissions/src/evaluator'; -import type { PermissionCondition } from '@object-ui/types'; - -/** - * A rule as it arrives from STORED JSON — the path that matters, because - * `RowLevelFilter['operator']` is a closed union that protects TypeScript call - * sites and nothing else, and `field` is a bare `string` that no type narrows. - */ -const storedRule = (field: string, operator: string, value: unknown): RowLevelFilter => - ({ field, operator, value }) as unknown as RowLevelFilter; - -/** Does this rule admit this row? One row in, one verdict out. */ -function admits(field: string, operator: string, value: unknown, row: unknown): boolean { - const manager = new DataScopeManager(); - manager.registerScope('t', { data: [] }); - manager.setFilters('t', [storedRule(field, operator, value)]); - return manager.applyFilters('t', [row]).length === 1; -} - -describe('DataScopeManager hardening — field reads (objectui#7751 gap 1)', () => { - const row = { id: 1, tenant: 'acme' }; - - it('refuses the rule the card was filed for: a `constructor` name no longer admits every row', () => { - // Measured on the pre-fix source: this returned the ENTIRE dataset. The - // name reached `Object.prototype.constructor`, `Function !== 'x'` is true, - // and every row passed the rule that existed to hide it. - expect(admits('constructor', 'ne', 'x', row)).toBe(false); - expect(admits('constructor', 'ne', 'x', 1)).toBe(false); - }); - - // The class, not three spellings of it. A name list enumerates; the prototype - // chain has more members than any list holds, and each one that is not on the - // list reads as an absent field, which ADMITS on a negative operator. That is - // the reason this evaluator distinguishes "inherited" from "absent" instead - // of copying the sibling's read — and objectui#8044 has since ported the same - // three-case read back to the sibling, so the two now agree. - it.each([ - '__proto__', - 'constructor', - 'prototype', - 'toString', - 'valueOf', - 'hasOwnProperty', - 'isPrototypeOf', - 'propertyIsEnumerable', - ])('denies rather than admits for the prototype member name %s', (field) => { - expect(admits(field, 'ne', 'x', row)).toBe(false); - expect(admits(field, 'nin', ['x'], row)).toBe(false); - expect(admits(field, 'eq', 'x', row)).toBe(false); - expect(admits(field, 'in', ['x'], row)).toBe(false); - expect(admits(field, 'contains', 'x', row)).toBe(false); - expect(admits(field, 'gte', 0, row)).toBe(false); - }); - - it('reads a value the record inherits as NOT the record\'s data', () => { - const inherited = () => Object.assign(Object.create({ tenant: 'acme' }), { id: 1 }); - // The record's tenant IS `acme` — through its prototype. Admitting it under - // "tenant is not acme" is the fail-open; denying under "tenant is acme" is - // the narrowing that buys it. Both verdicts are the refusal, not a compare. - expect(admits('tenant', 'eq', 'acme', inherited())).toBe(false); - expect(admits('tenant', 'ne', 'other', inherited())).toBe(false); - }); - - // The regression THIS fix is most likely to cause, pinned against itself. - // "Field not set on this row" is ordinary, legitimate data — not an attack — - // and its verdicts must be exactly what they have always been. Measured: of - // 2772 differential cases, the genuinely-absent family changed zero verdicts. - it('leaves a genuinely absent field with the verdicts it has always had', () => { - const noSuchName = { id: 1 }; - expect(admits('status', 'ne', 'archived', noSuchName)).toBe(true); - expect(admits('status', 'nin', ['archived'], noSuchName)).toBe(true); - expect(admits('status', 'eq', 'archived', noSuchName)).toBe(false); - expect(admits('status', 'in', ['archived'], noSuchName)).toBe(false); - expect(admits('status', 'contains', 'arch', noSuchName)).toBe(false); - }); - - it('reads own members off a null-prototype record unchanged', () => { - const bare = Object.create(null); - bare.tenant = 'acme'; - expect(admits('tenant', 'eq', 'acme', bare)).toBe(true); - expect(admits('tenant', 'ne', 'acme', bare)).toBe(false); - }); -}); - -describe('DataScopeManager hardening — comparisons (objectui#7751 gap 2)', () => { - it('refuses the two coercions the card was filed for', () => { - // Measured on the pre-fix source: both admitted. `null >= 0` is `0 >= 0`; - // `'10' >= 0` is `10 >= 0`. Neither coercion is anything the rule's author - // wrote, and both widen a rule meant to select numbers. - expect(admits('age', 'gte', 0, { age: null })).toBe(false); - expect(admits('age', 'gte', 0, { age: '10' })).toBe(false); - }); - - it.each([ - ['null', null], - ['a boolean', true], - ['a false boolean', false], - ['an empty string', ''], - ['a numeric string', '10'], - ['an empty array', []], - ['a single-element array', [10]], - ])('denies a numeric rule for %s on the record side', (_label, fieldValue) => { - expect(admits('age', 'gte', 0, { age: fieldValue })).toBe(false); - expect(admits('age', 'lte', 0, { age: fieldValue })).toBe(false); - }); - - it('denies a numeric record value against a non-numeric rule value', () => { - expect(admits('age', 'gte', '10', { age: 30 })).toBe(false); - expect(admits('age', 'gte', null, { age: 30 })).toBe(false); - expect(admits('age', 'gte', true, { age: 30 })).toBe(false); - }); - - // The reason this evaluator does NOT copy the sibling's `typeof === 'number'` - // on both sides. These three orderings work on this evaluator today, none of - // them coerces anything, and the sibling's predicate denies every row for all - // three (measured). Same KIND is the property; "both numbers" is a different, - // stricter rule that costs real rules and buys no safety. - it('keeps ordered comparisons that do not coerce: ISO date strings', () => { - expect(admits('created', 'gte', '2023-01-01', { created: '2024-06-01' })).toBe(true); - expect(admits('created', 'gte', '2023-01-01', { created: '2022-01-01' })).toBe(false); - }); - - it('keeps ordered comparisons that do not coerce: string ranges', () => { - expect(admits('name', 'gte', 'b', { name: 'zoe' })).toBe(true); - expect(admits('name', 'gte', 'b', { name: 'alice' })).toBe(false); - }); - - it('keeps ordered comparisons that do not coerce: Date objects', () => { - expect(admits('at', 'gte', new Date('2023-01-01'), { at: new Date('2024-01-01') })).toBe(true); - expect(admits('at', 'gte', new Date('2023-01-01'), { at: new Date('2020-01-01') })).toBe(false); - }); - - it('keeps numbers comparing as numbers', () => { - expect(admits('age', 'gte', 18, { age: 30 })).toBe(true); - expect(admits('age', 'gte', 18, { age: 17 })).toBe(false); - expect(admits('age', 'gt', 0, { age: Infinity })).toBe(true); - expect(admits('age', 'gte', 0, { age: NaN })).toBe(false); - }); - - it('requires the `contains` rule value to BE a string rather than become one', () => { - // `String(filterValue)` made a numeric rule value match a numeric string - // record value — the same unwritten coercion as the ordered arms, and the - // sibling already refused it. - expect(admits('code', 'contains', 1, { code: '10' })).toBe(false); - expect(admits('code', 'contains', '1', { code: '10' })).toBe(true); - }); - - it('leaves the non-ordering operators alone — they never coerced', () => { - // `===`, `!==` and `includes` (SameValueZero) do not coerce, so no guard is - // added to them and no verdict moves. Pinned so a later "consistency" pass - // does not narrow them on the strength of the arms above. - expect(admits('age', 'eq', 0, { age: null })).toBe(false); - expect(admits('age', 'ne', 0, { age: null })).toBe(true); - expect(admits('age', 'in', [10], { age: '10' })).toBe(false); - expect(admits('age', 'in', ['10'], { age: '10' })).toBe(true); - }); -}); - -/** - * The anti-divergence pin. - * - * One table, both evaluators, each addressed in its own operator spelling. A - * case listed here is a HARDENING case: a rule that names something which is - * not the record's data, or compares two values of different kinds. Every one - * of them must DENY in both evaluators, and a future change that reopens one - * of them in either evaluator turns this red — which is the whole point, since - * both previous divergences were found by hand and only after they shipped. - */ -describe('cross-evaluator conformance — both evaluators refuse the same rules', () => { - const record = { id: 1, tenant: 'acme', age: 30 }; - - /** Spelling map. Its existence is objectui#7750's question, not this pin's. */ - const NEGATIVE_EQ = { core: 'ne', sibling: 'neq' } as const; - - const sibling = (field: string, operator: string, value: unknown, row: Record) => - evaluateCondition({ field, operator, value } as unknown as PermissionCondition, row); - - it.each([ - ['a `__proto__` field name', '__proto__', NEGATIVE_EQ, 'x'], - ['a `constructor` field name', 'constructor', NEGATIVE_EQ, 'x'], - ['a `prototype` field name', 'prototype', NEGATIVE_EQ, 'x'], - ])('%s is refused by both', (_label, field, ops, value) => { - expect(admits(field, ops.core, value, record)).toBe(false); - expect(sibling(field, ops.sibling, value, record)).toBe(false); - }); - - it.each([ - ['null against a number', 'age', null], - ['a numeric string against a number', 'age', '10'], - ['a boolean against a number', 'age', true], - ['an array against a number', 'age', []], - ])('a coercing ordered comparison — %s — is refused by both', (_label, field, fieldValue) => { - const row = { ...record, [field]: fieldValue }; - expect(admits(field, 'gte', 0, row)).toBe(false); - expect(sibling(field, 'gte', 0, row)).toBe(false); - }); - - it('a `contains` rule with a non-string value is refused by both', () => { - const row = { ...record, code: '10' }; - expect(admits('code', 'contains', 1, row)).toBe(false); - expect(sibling('code', 'contains', 1, row)).toBe(false); - }); - - it('an unimplemented operator spelling denies rather than admits in both', () => { - expect(admits('tenant', 'no_such_operator', 'acme', record)).toBe(false); - expect(sibling('tenant', 'no_such_operator', 'acme', record)).toBe(false); - }); - - /** - * CLOSED at objectui#8044 — these two blocks were written as `KNOWN GAP` - * assertions expecting the sibling to ADMIT, precisely so that fixing it - * would turn this file red and lead the implementer here. It did. The - * sibling now reads its field in the same three cases `readField` above - * does, so the rows below have flipped to `false` and joined the conformance - * table: both evaluators refuse a prototype member name, and both refuse a - * value the record inherits rather than owns. - * - * ⛔ A red here is never fixed by loosening either evaluator to match the - * other. The whole file exists because the two diverged twice. - */ - it.each([ - ['toString'], - ['valueOf'], - ['hasOwnProperty'], - ])('a prototype member name outside the refusal list — %s — is refused by both', (field) => { - expect(admits(field, NEGATIVE_EQ.core, 'x', record)).toBe(false); - expect(sibling(field, NEGATIVE_EQ.sibling, 'x', record)).toBe(false); - }); - - it('an inherited value under a negative rule is refused by both', () => { - const inherited = Object.assign(Object.create({ tenant: 'acme' }), { id: 1 }) as Record; - expect(admits('tenant', NEGATIVE_EQ.core, 'acme', inherited)).toBe(false); - expect(sibling('tenant', NEGATIVE_EQ.sibling, 'acme', inherited)).toBe(false); - }); - - /** - * The reverse direction, also asserted: where `DataScopeManager` is - * deliberately BROADER than the sibling. The sibling requires - * `typeof === 'number'` on both sides of every ordered comparison, so it - * denies ISO date strings, string ranges and `Date` objects — none of which - * coerce. Copying its predicate here would have denied every row for those - * rules. Pinned so the difference reads as a decision with a reason rather - * than as the next divergence. - */ - it('DELIBERATE DIVERGENCE — same-kind non-numeric ordering works here, not in the sibling', () => { - const row = { created: '2024-06-01' }; - expect(admits('created', 'gte', '2023-01-01', row)).toBe(true); - expect(sibling('created', 'gte', '2023-01-01', row)).toBe(false); - }); -}); diff --git a/packages/core/src/data-scope/__tests__/DataScopeManager.test.ts b/packages/core/src/data-scope/__tests__/DataScopeManager.test.ts deleted file mode 100644 index a1286073d6..0000000000 --- a/packages/core/src/data-scope/__tests__/DataScopeManager.test.ts +++ /dev/null @@ -1,295 +0,0 @@ -/** - * ObjectUI - * Copyright (c) 2024-present ObjectStack Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { describe, it, expect } from 'vitest'; -import { DataScopeManager, type RowLevelFilter } from '../DataScopeManager'; - -describe('DataScopeManager', () => { - describe('Scope Registration', () => { - it('should register and retrieve a scope', () => { - const manager = new DataScopeManager(); - manager.registerScope('contacts', { data: [{ name: 'Alice' }] }); - const scope = manager.getScope('contacts'); - expect(scope).toBeDefined(); - expect(scope?.data).toEqual([{ name: 'Alice' }]); - }); - - it('should return undefined for unregistered scope', () => { - const manager = new DataScopeManager(); - expect(manager.getScope('unknown')).toBeUndefined(); - }); - - it('should remove a scope', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - expect(manager.getScope('test')).toBeDefined(); - manager.removeScope('test'); - expect(manager.getScope('test')).toBeUndefined(); - }); - - it('should list scope names', () => { - const manager = new DataScopeManager(); - manager.registerScope('a', { data: [] }); - manager.registerScope('b', { data: [] }); - expect(manager.getScopeNames()).toEqual(['a', 'b']); - }); - - it('should clear all scopes', () => { - const manager = new DataScopeManager(); - manager.registerScope('a', { data: [] }); - manager.registerScope('b', { data: [] }); - manager.clear(); - expect(manager.getScopeNames()).toEqual([]); - }); - }); - - describe('Scope Configuration', () => { - it('should register scope with config', () => { - const manager = new DataScopeManager(); - manager.registerScopeWithConfig('test', { - data: [1, 2, 3], - readOnly: true, - filters: [{ field: 'status', operator: 'eq', value: 'active' }], - }); - - expect(manager.getScope('test')?.data).toEqual([1, 2, 3]); - expect(manager.isReadOnly('test')).toBe(true); - expect(manager.getFilters('test')).toHaveLength(1); - }); - - it('should throw when updating read-only scope', () => { - const manager = new DataScopeManager(); - manager.registerScopeWithConfig('readonly', { readOnly: true }); - expect(() => manager.updateScopeData('readonly', [1])).toThrow('Cannot update read-only scope'); - }); - }); - - describe('Row-Level Filtering', () => { - it('should apply eq filter', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [{ field: 'status', operator: 'eq', value: 'active' }]); - - const result = manager.applyFilters('test', [ - { id: 1, status: 'active' }, - { id: 2, status: 'inactive' }, - { id: 3, status: 'active' }, - ]); - - expect(result).toHaveLength(2); - expect(result[0].id).toBe(1); - expect(result[1].id).toBe(3); - }); - - it('should apply gt filter', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [{ field: 'age', operator: 'gt', value: 18 }]); - - const result = manager.applyFilters('test', [ - { name: 'A', age: 15 }, - { name: 'B', age: 25 }, - { name: 'C', age: 18 }, - ]); - - expect(result).toHaveLength(1); - expect(result[0].name).toBe('B'); - }); - - it('should apply in filter', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [{ field: 'role', operator: 'in', value: ['admin', 'editor'] }]); - - const result = manager.applyFilters('test', [ - { name: 'A', role: 'admin' }, - { name: 'B', role: 'viewer' }, - { name: 'C', role: 'editor' }, - ]); - - expect(result).toHaveLength(2); - }); - - it('should apply contains filter', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [{ field: 'name', operator: 'contains', value: 'ob' }]); - - const result = manager.applyFilters('test', [ - { name: 'Bob' }, - { name: 'Alice' }, - { name: 'Robert' }, - ]); - - expect(result).toHaveLength(2); - }); - - it('should apply multiple filters (AND logic)', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [ - { field: 'status', operator: 'eq', value: 'active' }, - { field: 'age', operator: 'gte', value: 18 }, - ]); - - const result = manager.applyFilters('test', [ - { status: 'active', age: 25 }, - { status: 'inactive', age: 25 }, - { status: 'active', age: 15 }, - ]); - - expect(result).toHaveLength(1); - expect(result[0].age).toBe(25); - }); - - it('should return all data when no filters set', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - const data = [{ a: 1 }, { a: 2 }]; - expect(manager.applyFilters('test', data)).toEqual(data); - }); - }); - - describe('Scope Updates', () => { - it('should update scope data', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.updateScopeData('test', [1, 2, 3]); - expect(manager.getScope('test')?.data).toEqual([1, 2, 3]); - }); - - it('should update loading state', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [], loading: false }); - manager.updateScopeLoading('test', true); - expect(manager.getScope('test')?.loading).toBe(true); - }); - - it('should update error state', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.updateScopeError('test', new Error('fail')); - expect(manager.getScope('test')?.error).toBeDefined(); - }); - }); - - describe('Change Listeners', () => { - it('should notify listeners on scope registration', () => { - const manager = new DataScopeManager(); - let notified = false; - manager.onScopeChange('test', () => { notified = true; }); - manager.registerScope('test', { data: [] }); - expect(notified).toBe(true); - }); - - it('should notify listeners on data update', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - let newData: any; - manager.onScopeChange('test', (scope) => { newData = scope.data; }); - manager.updateScopeData('test', [1, 2]); - expect(newData).toEqual([1, 2]); - }); - - it('should allow unsubscribing', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - let count = 0; - const unsub = manager.onScopeChange('test', () => { count++; }); - manager.updateScopeData('test', [1]); - expect(count).toBe(1); - unsub(); - manager.updateScopeData('test', [2]); - expect(count).toBe(1); - }); - }); - - describe('Unknown operator fails closed (objectui#7378)', () => { - // Why the cast is here, and why it must stay: `RowLevelFilter['operator']` - // is a closed nine-member union, so TypeScript refuses `'equals'` or - // `'is_null'` at a call site. That protects TypeScript callers and nothing - // else. A scope rule read back from stored JSON reaches `applyFilters` as a - // plain string, and the switch keys on that string at runtime, exactly the - // way it is spelled below. The cast reproduces the path stored data takes; - // it is the point of these tests, not a shortcut to be "cleaned up". A - // test that only spells the nine declared operators cannot reach the - // `default` arm at all. - const storedRule = (field: string, operator: string, value: unknown): RowLevelFilter => - ({ field, operator, value }) as unknown as RowLevelFilter; - - it('does not admit a record it cannot evaluate (operator outside every published vocabulary)', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [storedRule('status', 'not_an_operator', 'active')]); - - const result = manager.applyFilters('test', [ - { id: 1, status: 'active' }, - { id: 2, status: 'inactive' }, - ]); - - // Fail closed: a rule the evaluator cannot answer denies every row in - // the scope, the same answer `evaluateCondition` in @object-ui/permissions - // gives from its own `default` arm. Before the fix this returned both - // rows, `{ id: 2 }` included, with no error and no console line. - expect(result).toEqual([]); - }); - - it('does not let an unrecognised rule widen a scope another rule narrows (AND semantics)', () => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [ - { field: 'status', operator: 'eq', value: 'active' }, - storedRule('tenant', 'not_an_operator', 'acme'), - ]); - - const result = manager.applyFilters('test', [ - { id: 1, status: 'active', tenant: 'acme' }, - { id: 2, status: 'active', tenant: 'other' }, - { id: 3, status: 'inactive', tenant: 'acme' }, - ]); - - // Before the fix the unknown `tenant` rule evaluated to `true`, so the - // `status` rule alone decided and `{ id: 2, tenant: 'other' }` passed — - // the row the second rule existed to hide. Now nothing passes. - expect(result).toEqual([]); - }); - - // The spec's published vocabularies (`VIEW_FILTER_OPERATORS` from - // `@objectstack/spec/ui`, `VALID_AST_OPERATORS` from `@objectstack/spec/data`) - // carry spellings this switch has no arm for: the canonical forms of the - // implemented abbreviations, and the whole null-ness family. Measured on - // @objectstack/spec 17.2.0: 18 of the 20 view operators and 44 of the 53 - // AST operators have no arm. Until they are either implemented or refused - // by name, they MUST take the fail-closed arm rather than the admit-all one. - // A change that implements these spellings rewrites the expectations below - // to the evaluated result; it never deletes the cases. Every case is - // chosen so that a CORRECT evaluation of the spelling admits at least one - // of the two rows: an implementation that lands without rewriting the - // expectation turns red here instead of passing by coincidence. - it.each([ - ['equals', 'status', 'active'], - ['not_equals', 'status', 'active'], - ['greater_than', 'age', 20], - ['not_in', 'status', ['inactive']], - ['starts_with', 'status', 'act'], - ['is_null', 'status', null], - ['is_not_null', 'status', null], - ])('denies rather than admits for the published-but-unimplemented spelling %s', (operator, field, value) => { - const manager = new DataScopeManager(); - manager.registerScope('test', { data: [] }); - manager.setFilters('test', [storedRule(field, operator, value)]); - - const result = manager.applyFilters('test', [ - { id: 1, status: 'active', age: 30 }, - { id: 2, status: null, age: 10 }, - ]); - - expect(result).toEqual([]); - }); - }); -}); diff --git a/packages/core/src/data-scope/index.ts b/packages/core/src/data-scope/index.ts index e510da9597..577ff32d37 100644 --- a/packages/core/src/data-scope/index.ts +++ b/packages/core/src/data-scope/index.ts @@ -1,20 +1,19 @@ /** * @object-ui/core - DataScope Module * - * Runtime data scope management for row-level security and - * reactive data state within the UI component tree. + * Resolution of the data a view or a page element reads: the spec's + * `ViewData` union (`ViewDataProvider`) and the per-element + * `ElementDataSource` binding. + * + * Row-level security is not evaluated here. The platform declares it once, as + * the CEL predicate of a `@objectstack/spec` row-level security policy, and + * enforces it on the server; objectui#7750 retired the client-side row-level + * filter evaluator this module used to export. * * @module data-scope * @packageDocumentation */ -export { - DataScopeManager, - defaultDataScopeManager, - type RowLevelFilter, - type DataScopeConfig, -} from './DataScopeManager.js'; - export { ViewDataProvider, type ViewDataConfig, diff --git a/packages/permissions/src/__tests__/evaluator.prototype-guard-8044.test.ts b/packages/permissions/src/__tests__/evaluator.prototype-guard-8044.test.ts index 6bbac8a9c0..8b2225b3cd 100644 --- a/packages/permissions/src/__tests__/evaluator.prototype-guard-8044.test.ts +++ b/packages/permissions/src/__tests__/evaluator.prototype-guard-8044.test.ts @@ -21,9 +21,10 @@ * * ⛔ The fix is NOT a longer name list. A list enumerates spellings, and * `Object.prototype` has more of them than any list will hold; the defect is - * the shape of the guard. The shape that closes the class is `readField` in - * `packages/core/src/data-scope/DataScopeManager.ts` (objectui#7751), and this - * change is a port of it back to the evaluator that was #7751's reference. + * the shape of the guard. The shape that closes the class is the three-case + * read objectui#7751 landed on a sibling row-level evaluator in + * `@object-ui/core` (retired since, at objectui#7750), and this change is a + * port of it back to the evaluator that was #7751's reference. * * ## What this file measures, and why the last describe block is the important one * @@ -38,8 +39,8 @@ * the verdicts that moved in each direction. * * ⭐ `widened === 0` is the acceptance criterion, matching the bar objectui#7751 - * set on `DataScopeManager` over its own 2772-case matrix (352 narrowed, zero - * widened, zero change in the genuinely-absent family). + * set on that sibling evaluator over its own 2772-case matrix (352 narrowed, + * zero widened, zero change in the genuinely-absent family). */ import { describe, it, expect } from 'vitest'; diff --git a/packages/permissions/src/evaluator.ts b/packages/permissions/src/evaluator.ts index df32575e89..9f859dfe0c 100644 --- a/packages/permissions/src/evaluator.ts +++ b/packages/permissions/src/evaluator.ts @@ -125,8 +125,7 @@ function resolveRoles(userRoles: string[], roleDefinitions: RoleDefinition[]): s * the own-member rule in `readField` would refuse them anyway. `prototype` * earns its place separately: it is NOT present on a plain object's chain * (`'prototype' in {}` is `false`), so without this list it would classify as - * an ordinary absent field. The same three names, for the same reasons, as - * `PROTOTYPE_FIELD_NAMES` in `DataScopeManager` (`@object-ui/core`). + * an ordinary absent field. */ const PROTOTYPE_FIELD_NAMES: ReadonlySet = new Set([ '__proto__', @@ -176,10 +175,11 @@ type FieldRead = { readable: true; value: unknown } | { readable: false }; * matrix in `__tests__/evaluator.prototype-guard-8044.test.ts` measures: the * genuinely-absent family changed zero verdicts. * - * This is a port of the shape objectui#7751 landed in `readField` in - * `packages/core/src/data-scope/DataScopeManager.ts`, which was written - * against this evaluator as its reference — and then went further than it, - * because this evaluator had the defect it was being used as the standard for. + * This is a port of the shape objectui#7751 landed on a sibling row-level + * evaluator in `@object-ui/core`, which was written against this evaluator as + * its reference — and then went further than it, because this evaluator had + * the defect it was being used as the standard for. objectui#7750 retired that + * sibling; the guard stands here on its own. */ function readField(record: Record, field: string): FieldRead { if (PROTOTYPE_FIELD_NAMES.has(field)) return { readable: false }; diff --git a/scripts/check-doc-example-types.mjs b/scripts/check-doc-example-types.mjs index e5bcd1a611..96918f9175 100644 --- a/scripts/check-doc-example-types.mjs +++ b/scripts/check-doc-example-types.mjs @@ -616,12 +616,6 @@ export const UNGATED_EXAMPLES = { reason: 'usage fragment: references `contextDataSource`, which the example never declares, so what depends on it is judged unbound', }, - 'packages/core/src/data-scope/DataScopeManager.ts DataScopeManager #1': { - card: null, - codes: [2304], - reason: - 'usage fragment: references `myDataSource`, which the example never declares', - }, 'packages/core/src/data-scope/ViewDataProvider.ts ViewDataProvider #1': { card: null, codes: [2304],