fix(core): DataScopeManager denies a row on an operator it does not implement

.changeset/core-datascope-unknown-operator-denies-7378.md

@@ -0,0 +1,11 @@
+---
+'@object-ui/core': minor
+---
+
+`DataScopeManager` now **denies** a row when a row-level scope rule carries an operator its evaluator does not implement. It used to **admit** the row.
+
+Behaviour change on a permission boundary, stated plainly. `evaluateFilter` implements nine operator spellings — `eq`, `ne`, `gt`, `lt`, `gte`, `lte`, `in`, `nin`, `contains` — and its `default` arm returned `true`, so a stored `RowLevelFilter` carrying any other spelling passed every record the rule existed to hide, silently: no error, no console line, only a result set that was too large, which looks exactly like a correctly configured permissive scope. The arm now returns `false`, the answer `evaluateCondition` in `@object-ui/permissions` already gives from its own `default` arm. Because `applyFilters` ANDs a scope's rules, one unrecognised rule now denies every row in that scope.
+
+Who this reaches, measured on this release's base rather than assumed. The `RowLevelFilter['operator']` union is closed, so no TypeScript caller can write an unimplemented spelling, and no code in this repository constructs a `RowLevelFilter` outside the evaluator's own test. The path that changes is scope configuration read back from stored or hand-written JSON and handed to `setFilters` / `registerScopeWithConfig`, where the operator arrives as a plain string the type never checked. A deployment holding such a rule with a spelling outside the nine — including the spec's canonical `equals` / `not_equals` / `greater_than` / `starts_with` and the null-ness family `is_null` / `is_not_null`, none of which have an arm — sees fewer rows from that scope after upgrading, never more. Those spellings are not implemented here; they are refused instead of admitted. Whether to canonicalise them through the spec's `canonicalAstOperator` is left open on objectui#7378.
+
+Graded `minor` because a release reader can observe the narrowing on stored data; the declared type is unchanged and the set of spellings the evaluator accepts has not widened.

packages/core/src/data-scope/DataScopeManager.ts

@@ -25,7 +25,11 @@ import type { DataScope, DataContext, DataSource } from '@object-ui/types';
 export interface RowLevelFilter {
   /** Field to filter on */
   field: string;
-  /** Filter operator */
+  /**
+   * 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 */
   value: any;
@@ -236,7 +240,10 @@ export class DataScopeManager implements DataContext {
 }
 
 /**
- * Evaluate a single filter condition against a field value
+ * 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) {
@@ -259,7 +266,15 @@ function evaluateFilter(fieldValue: any, operator: RowLevelFilter['operator'], f
     case 'contains':
       return typeof fieldValue === 'string' && fieldValue.includes(String(filterValue));
     default:
-      return true;
+      // 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;
   }
 }
 

packages/core/src/data-scope/__tests__/DataScopeManager.test.ts

@@ -7,7 +7,7 @@
  */
 
 import { describe, it, expect } from 'vitest';
-import { DataScopeManager } from '../DataScopeManager';
+import { DataScopeManager, type RowLevelFilter } from '../DataScopeManager';
 
 describe('DataScopeManager', () => {
   describe('Scope Registration', () => {
@@ -208,4 +208,88 @@ describe('DataScopeManager', () => {
       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([]);
+    });
+  });
 });