fix(core): DataScopeManager denies a row on an operator it does not implement - #7748
Conversation
…mplement `evaluateFilter`'s `default` arm returned `true`, so a stored `RowLevelFilter` carrying an operator outside the nine the switch implements passed every record the rule existed to hide — silently. The arm now returns `false`, the answer `evaluateCondition` in @object-ui/permissions already gives from its own `default` arm. Red-first test exercises the runtime path stored JSON takes (cast past the closed union, deliberately) and pins the fail-closed answer for an operator outside every published vocabulary and for the spec's published spellings that have no arm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract review — VERDICT: PASS
⭐ The finding that reframes the whole area
⇒ The spec's real RLS vocabulary is not The fix itself is right, and its width is larger than the title
Census reproduced exactly on Reverse verification: legitimate, and the better shapeThe shipped test text is held constant and only the source moves. Re-run in the scratch tree: green The liveness framing is honest, and one gap is named rather than assumedIn-repo constructors of a
Census correction confirmed
Semver
Boundary flag → A, with an ordering constraint I am honouringThe reviewer verified all five of the dev's measurements and added three the dev did not list:
⇒ B refused: a design, not a transplant. A stands — the floor triage set. Carriers — and one deliberate deviation from the reviewer's instructionThe reviewer asked for one carrier. I filed two, and the split is deliberate:
Why split: #7751 is independently actionable today whichever way #7750 goes, and it has the same failure direction as this card — admit, silently, on the same boundary. Burying a live hardening gap inside a retire-or-canonicalise decision is how it sits unfixed while the decision stalls. ⛔ Neither of them lives only in this PR body. Both #7379 ( Non-blocking, not gating merge
Coverage limit stated rather than papered over: the reviewer could not read the individual shard log (three routes blocked), so which of the four green shards executed the new file is unmeasured — the execution chain is established ( Generated by Claude Code |
Fixes #7378
DataScopeManager.evaluateFilterimplements nine operator spellings and itsdefaultarm returnedtrue, so a storedRowLevelFiltercarrying any other operator admitted every record the rule existed to hide, with no error and no console line. The arm now returnsfalse, the answer the siblingevaluateConditionin@object-ui/permissionsalready gives from its owndefaultarm. A red-first test reaches the arm the way stored data does, past the closed union, and pins the fail-closed answer.Clause-② — accept-set and behaviour changes in this diff
packages/core/src/data-scope/DataScopeManager.tsevaluateFilter,defaultarm: admit (return true) → deny (return false) for any operator outsideeq/ne/gt/lt/gte/lte/in/nin/contains.applyFilters(unchanged code, changed outcome): a scope holding one unrecognised rule now returns zero rows for that scope, because rules are ANDed; it used to let the remaining rules alone decide.RowLevelFilter['operator']union is unchanged, no export was added or removed, nothing is logged or thrown.Measurements (all taken in this run on
origin/mainat565f2b6aa, the branch base)1. Liveness census — reproduces the execution seat's reading, with one count corrected
RowLevelFilter(git grep -ln)data-scope/index.tsbarrel — reproducedDataScopeManagercontent/docs/guide/architecture-overview.md:311, a directory-tree comment; present at the card's filing commitd53e472(checked via REST at that ref), so it was an undercount then, not a new reference — and it is not a producerRowLevelFiltergit grep -n 'registerScopeWithConfig|setFilters(|applyFilters('hits only the class itself and its own testDataScopeManager.ts:29and the switch at the same ninecaselabelsgit grep -ln evaluateFilter⇒ Nothing in this repository can reach the
defaultarm through a TypeScript caller. The path that can is scope configuration read from stored or hand-written JSON, where the operator arrives as a plain string — the path the test exercises.2. Spec-vocabulary gap census — measured on
@objectstack/spec17.2.0 as resolved frompackages/coreVIEW_FILTER_OPERATORS(@objectstack/spec/ui): 20 members. With an arm: 2 (contains,in). Without an arm: 18 —equals,not_equals,not_contains,icontains,starts_with,ends_with,greater_than,less_than,greater_than_or_equal,less_than_or_equal,not_in,is_empty,is_not_empty,is_null,is_not_null,before,after,between.VALID_AST_OPERATORS(@objectstack/spec/data): 53 members. With an arm: 9 — exactly the nine implemented spellings. Without an arm: 44 (the canonical words; the seven symbolic forms — equals-sign, bang-equals, the angle-bracket not-equals, greater-than, greater-or-equal, less-than, less-or-equal — spelled in words here because GitHub's sanitizer eats angle-bracket shapes even inside backticks;like/ilike; and the whole null-ness / emptiness family).VIEW_FILTER_OPERATOR_ALIASESmaps every implemented abbreviation onto a canonical word the switch has no arm for:eq → equals,ne → not_equals,gt → greater_than,lt → less_than,gte → greater_than_or_equal,lte → less_than_or_equal,nin → not_in.containsis a member of both published sets (true/true); negative —totally_unknown_opis a member of neither (false/false).3. The sibling's
defaultarm, quoted, and whether this matches itpackages/permissions/src/evaluator.ts:158-159reads, verbatim:default: return false;It is silent — no
consoleline, no throw, no diagnostic collector. This PR's arm is the same:return false, silent, with a comment. Its own test pins the behaviour asoperator: 'unknown' as any→toBe(false)(evaluator.test.ts:229-231); the new test here uses the same shape (as unknown as RowLevelFilter) and explains in its header why the cast is the point.evaluator.tsitself is untouched; it is the control group.4. Red-first test — measured, not asserted
Base source (
565f2b6aa) with the final test text, run viapnpm exec vitest run packages/core/src/data-scope/__tests__/DataScopeManager.test.tsfrom the repo root:{ id: 2, status: 'inactive' }— the row the rule existed to exclude — is admitted. All nine new cases fail this way: the two structural cases and the seven published-but-unimplemented spellings (equals,not_equals,greater_than,not_in,starts_with,is_null,is_not_null).Head (
a12751d78), same command:Test Files 1 passed (1) · Tests 28 passed (28).The red run above was a reverse verification from the committed state: source blob checked out from the base sha, on-disk mutation proven by anchored counts (
return true;= 1,Fail closed= 0), restore bygit checkout HEAD -- ABSOLUTE_PATHunder atrap, proven bygit hash-objectequal to the HEAD blob90af27c88…and an emptygit diff HEAD, then re-run green.Every
it.eachcase is chosen so that a correct evaluation of the spelling admits at least one row (numericageforgreater_than): a later change that implements these spellings turns them red rather than passing by coincidence, and the header says the expectation is to be rewritten, never deleted.Boundary flag — a question this PR does NOT resolve, with the measurements a ruler needs
Should
evaluateFiltercanonicalise its operator throughcanonicalAstOperator(@objectstack/spec/data), the way PR #7377 repaired the sibling card objectui#7349, so that the accepted vocabulary is the published one rather than this hand-written nine? The card offers that as a worked precedent, explicitly not a ruling. This PR ships the minimal repair (fail closed) and leaves the question open forneeds:contract-review. What a ruler would need, measured:canonicalAstOperatormaps the nine implemented spellings to the symbolic forms —eqto the equals-sign,neto bang-equals,gt/lt/gte/lteto the greater-than, less-than, greater-or-equal and less-or-equal signs (spelled in words because of the sanitizer), andin/nin/containsto themselves — so canonicalising is not a prefix step, it re-keys the switch onto those symbols (that is whatValueDataSource.matchesComparisonNodekeys on today).totally_unknown_op → totally_unknown_op), so a fail-closeddefaultarm stays load-bearing under either route.between,starts_with/ends_with,not_contains/icontains, and a decision onlike/ilike(the precedent refuses those by name).containsis ASCII case-insensitive (toLowerCaseon both sides); this evaluator'scontainsis case-sensitive. Canonicalising by copying fix(core): teach ValueDataSource's matcher the filter vocabulary the wire already has #7377 would silently changecontainsfor existing rules.RowLevelFilter['operator']union would need widening — a public type change on a published package — or the type would keep lying about what the runtime accepts.Options a ruler might weigh: A keep the minimal fail-closed arm (this PR) and file canonicalisation as its own card; B canonicalise here (rejected for this PR: widens scope on a p1 security card, and the measurements above show it is a design, not a transplant); C retire the surface (needs the cross-repo consumer read). No recommendation beyond "not in this PR" — that is the contract reviewer's call.
Serial constraints — re-measured at implementation time
11 open PRs; every file list paged to completion (PR #5400 = 633 files across 7 pages, sum verified). Zero touch
packages/core/src/data-scope/. The only hits underpackages/permissions/are PR #5400'spackages/permissions/CHANGELOG.mdandpackage.json— release bookkeeping, not source, and not in this PR's file surface. Lit control: PR #7621 showspackages/core/src/utils/date-display.ts, as the claim said.Verification on head
a12751d78(exit codes captured before any pipe; verdict lines quoted)pnpm exec vitest run packages/core/src/data-scope/(repo root, via the verify lock):Test Files 3 passed (3) · Tests 63 passed (63)—VERDICT command-exit 0.pnpm --filter @object-ui/core type-check(afterpnpm --filter '@object-ui/core^...' build): echoedtsc --noEmit && tsc -p tsconfig.test.json—VERDICT command-exit 0.--listFilesproof:tsconfig.test.jsonreads the edited test file (1 hit) and the build project reads the edited source (1 hit); this is not a typecheck that excluded the test.pnpm --filter @object-ui/core lint— exit 0,✖ 515 problems (0 errors, 515 warnings). Not a narrowing: this is the fulleslint .run CI performs for this package, 214 files by--format json. The 7 + 1no-explicit-anywarnings on the two edited files are identical in count to the BASE blobs linted at the same paths (7 + 1), and 0anytokens occur on added lines.node scripts/check-changeset-fixed.mjs·check-changeset-no-major.mjs·check-changeset-overwrite.mjs·check-changeset-presence.mjs— all exit 0; presence gate:2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s).node scripts/check-control-bytes.mjs— exit 0,OK (scanned 6311 tracked text file(s)).pnpm lint/pnpm testshards and the rest of thecheck:*farm.Changeset
.changeset/core-datascope-unknown-operator-denies-7378.md,@object-ui/core: minor. Argued from the accept-set change: the narrowing is observable on stored data — a deployment holding a rule with an unimplemented spelling sees fewer rows after upgrading — sopatchwould tell a release reader nothing they can observe changed, which is false on exactly the boundary where being wrong matters.majoris CI-refused; per AGENTS.md §版本号策略 objectui's own breaking semantics ship asminorwith the break stated in the body, which the body does. The body also carries the liveness reading so a reader can judge whether it bites them today.Not done here, on purpose
packages/core/README.mdis unchanged: it documentsDataScopeManagerwithout describing operators or the fall-through, so there is no sentence to correct.packages/permissions/src/evaluator.tsis unchanged: it is the control group.Session:
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3🤖 Generated with Claude Code
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Generated by Claude Code