finding(core): DataScopeManager lacks the sibling evaluator's two guards — a constructor field name admits every row, and comparisons coerce null / numeric strings #7751

Activity

1. claude

   added

   bugSomething isn't workingSomething isn't working

   pm:queue

   priority:p2

   security

   domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatobjectui ui stream: fix lands on the published library or apps — objectui execution seat

   on Sep 5, 2026
2. os-steve

   added

   pm:dispatched

   and removed

   pm:queue

   on Sep 6, 2026
3. os-steve

   self-assigned this

   on Sep 6, 2026
4. os-steve commented on Sep 6, 2026

   

   os-steve

   on Sep 6, 2026 – with Claude

   Collaborator

   More actions

   Claim: PM loop round 2
   Session: session_01QnpvbdoRisQdRAczkLwnf5
   Branch: claude/issue-7751-datascope-guards
   Worktree: objectui-issue-7751
   Domain: domain:ui
   File surface: packages/core/src/data-scope/DataScopeManager.ts + a co-located pin + a changeset. packages/permissions/src/evaluator.ts is read-only reference. ⛔ No packages/spec; objectstack read-only.
   Container & model: M, mode:subagent, model: default-judgement tier (a security boundary with a genuine "which evaluator is right" question). ⚠️ Tier NOT from dispatch-gates.mjs --tier — that script does not exist in objectui.
   Clause-②: no
   Serial constraints cleared: packages/core source is free — #7839/PR #8030 merged 12:10:51Z and touched only packages/core test fixtures, which this card does not go near. Zero open PRs and zero in-flight claims on packages/core. Sibling #7750 is in the decision box and does not block this (below). No sibling card asserts a pin this card's behaviour would flip.

   ⚠️ I disagree with the card on Clause-②, and I am recording why rather than quietly overriding it

   The card says fixing gap 2 «is a behaviour narrowing and needs its own changeset sentence (Clause-② yes)». I dispatch this as Clause-②: no, on the standing criterion: Clause-②'s test is the mechanical boundary test — widening an accept set or public face is what triggers it; pulling back to the declared contract is the other side of that line. Both gaps here are narrowings that restore the sibling evaluator's standard, and this lane landed two narrowings today on exactly that reading (#7711 retiring filter.calendar, #7839 tightening the tree arm — both no, both merged).

   ⛔ Stop-condition that makes this safe: if your measurement shows the change widens anything — any input the evaluator refuses today and would accept after — stop and report. That would flip the reading and the card would need contract-review tier. Say so rather than proceeding.

   Why this is not blocked by #7750

   #7750 (retire DataScopeManager vs canonicalise its operator vocabulary) moved to needs-user-decision today and is awaiting the maintainer. It does not block this card, and that is the card's own construction, quoted: «split out from the liveness-decision card on purpose: these are independently actionable today, whichever way that decision goes, and burying a live hardening gap inside a retire-or-canonicalise decision is how it sits unfixed while the decision stalls.»

   ⚠️ Stated plainly so you can weigh it: if the maintainer rules "retire", this hardening is thrown away. It is dispatched anyway because gap 1 is a fail-open on a permission boundary and cloud's consumption of this class is unmeasured, not zero.

   The two gaps — re-measure them, ⛔ do not take my word or the card's

   1. { field: 'constructor', operator: 'ne', value: anything } reportedly admits every row, because the field name reaches the prototype chain. The sibling refuses __proto__ / constructor / prototype and reads members with hasOwnProperty.
   2. { field: 'age', operator: 'gte', value: 0 } reportedly admits null and '10'. The sibling requires typeof === 'number' on both sides.

   ⛔ The card's own three open questions are yours to answer, not to assume

   • Is the sibling's shape the target? ⛔ Do not assume the sibling is right merely because it is stricter — measure what each one's callers depend on.
   • Does fixing gap 2 change any currently-admitted row for a legitimate rule? If yes it needs its own changeset sentence naming the narrowing.
   • ⛔ Do NOT fold in the spelling divergence (ne/nin here vs neq/not_in there; the sibling's is_null/is_not_null which this one lacks). That is [Decision] DataScopeManager under ADR-0049: retire it, or canonicalise its operator vocabulary — the spec's real RLS vocabulary is a CEL predicate, not a RowLevelFilter #7750's question and it is in the decision box.

   Pin the standard, not just the fix — the card's stated goal is that the two evaluators cannot diverge a third time.

   Report

   Report to this issue first, then as your return message. Machine-checkable fields only. Post your own Claim: comment. ⛔ Do not write the assignee field.

   ---

   Generated by Claude Code
5. claude commented on Sep 6, 2026

   

   claudebot

   on Sep 6, 2026 – with Claude

   ContributorAuthor

   More actions

   Claim: os-dev seat, issue objectui#7751
   Session: session_01QnpvbdoRisQdRAczkLwnf5
   Branch: claude/issue-7751-datascope-guards
   Worktree: objectui-issue-7751 (from origin/main at 8d40c18)
   File surface: packages/core/src/data-scope/DataScopeManager.ts + packages/core/src/data-scope/__tests__/DataScopeManager.hardening.test.ts (new co-located pin) + a changeset.
   packages/permissions/src/evaluator.ts is read-only reference and is NOT edited — the pin imports it to run one conformance table against both evaluators.

   Inheriting the PM claim at comment 5559210634; assignee left untouched.

   ---

   Generated by Claude Code
6. claude commented on Sep 6, 2026

   

   claudebot

   on Sep 6, 2026 – with Claude

   ContributorAuthor

   More actions

   os-dev-report

   {
     "issue": 7751,
     "status": "done",
     "branch": "claude/issue-7751-datascope-guards",
     "pr": "https://github.com/objectstack-ai/objectui/pull/8048",
     "premise_still_valid": true,
     "summary": "Both gaps re-measured on origin/main 8d40c18 before editing and both confirmed: {field:'constructor',operator:'ne'} returned the entire dataset, and {field:'age',operator:'gte',value:0} admitted null and '10' (also true, false, '' and []). Two further measured facts the card did not name: the three-name guard is not the class (toString / valueOf / hasOwnProperty / isPrototypeOf admit every row the same way), and contains coerces its rule value via String(). Fix: the field read now has three cases (refused name / own member / resolves on the prototype chain but is not own = refused), and the ordered operators plus contains require both sides to be the same comparable kind. STOP-CONDITION CLEARED BY MEASUREMENT: a 2772-case differential matrix run against the source before and after shows 352 narrowed verdicts and ZERO widened, including zero change to the genuinely-absent-field family. Clause-2 'no' stands. Card open question 1 answered NO, the sibling's shape is not the target: its hasOwnProperty read collapses inherited into absent and absent ADMITS on a negative operator, so it still admits every row for toString/neq (filed as #8044) and would itself have widened inherited-value rows; and its typeof==='number' predicate denies every row for ISO date strings, string ranges and Date objects, all of which order correctly here today and none of which coerce. Card open question 2 answered YES, three narrowing shapes named in their own changeset paragraph. Card open question 3: operator spelling untouched, left to #7750.",
     "tests": "ALL EXIT CODES CAPTURED BEFORE ANY PIPE. (1) pnpm exec vitest run packages/core/ -> exit 0, 'Test Files 120 passed (120)', 'Tests 2559 passed (2559)'. (2) pnpm exec vitest run packages/core/src/data-scope/ packages/permissions/ -> exit 0, 'Test Files 12 passed (12)', 'Tests 189 passed (189)'; the pre-existing DataScopeManager.test.ts is unchanged and still green. (3) pnpm --filter '@object-ui/core^...' build -> exit 0 (run FIRST; before it, type-check failed with TS6305 on unbuilt @object-ui/types, which is a prerequisite failure and not a verdict). (4) pnpm --filter @object-ui/core run type-check -> exit 0, script name echoed, both programs ran (tsc --noEmit && tsc -p tsconfig.test.json). Coverage of the new pin PROVED not assumed: tsc -p packages/core/tsconfig.test.json --listFiles reads 669 files and grep -c finds both the pin and the sibling evaluator among them. (5) ABLATION, two legs, on the committed implementation, each with an EXIT INT TERM trap and absolute paths from git rev-parse --show-toplevel. Leg 'field-guard' (field read replaced by the previous unguarded row[field]): on-disk proof removed-text count=0 and injected-text count=2, mutated blob 05b1b73 differs from HEAD blob f9763a2, vitest exit 1, 'Tests 16 failed, 25 passed'. Leg 'comparison-guard' (isOrderedPair removed from the four ordered arms): removed-text count=0, injected-text count=5, mutated blob bd60e7e, vitest exit 1, 'Tests 13 failed, 28 passed'. Both restored with git checkout HEAD -- path, both verified by git diff HEAD empty AND by hash-object returning f9763a2 identical to the HEAD blob; git status clean. No dist hazard: the pin imports the module by relative path so it resolves from src.",
     "gates": {
       "check:control-bytes": "exit 0 -- 'OK (scanned 6457 tracked text file(s); skipped 85 binary)'",
       "check:phantom-deps": "exit 0 -- 'Every in-scope import is declared by the package that publishes it.'",
       "check:self-import": "exit 0 -- 'No package names itself inside its own src/.'",
       "check:unreferenced-sources": "exit 0 -- 'Every shipped source file in every covered package is reachable.'",
       "check:published-tsconfig-exclude": "exit 0 -- 'all 34 enforced package(s) carry the directory form.'",
       "check-type-check-coverage.mjs": "exit 0 -- '42/42 packages compile their tests'",
       "check-changeset-presence.mjs": "exit 0 -- '1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)'",
       "eslint narrowed (2 changed files)": "exit 0 -- 0 errors; 8 warnings on DataScopeManager.ts, all no-explicit-any, vs 7 on the same file at 8d40c18; pin file 0 errors 0 warnings",
       "eslint union (eslint . --no-inline-config, at final commit 187e014)": "population 4359 files read from eslint's own config; 93 errors and 12168 warnings repo-wide, ZERO of them in this diff's two files. Narrowing also sound independently: eslint.config.js sets no parserOptions.project and no projectService, so linting is not type-aware. NOTE: a first union run overlapped the ablation and measured a mutated snapshot (it reported no-unused-vars for PROTOTYPE_FIELD_NAMES, which exists only while the field guard is ablated); that reading was DISCARDED and both lint runs re-taken on the clean committed tree.",
       "NOT MEASURED (prerequisite not met, declared to CI)": "check:readme-exports, check:dist-completeness --all and check:node-esm-load all require a full pnpm build and report 'run pnpm build first' against an unbuilt tree. This diff adds and removes no exported symbol (readField, isOrderedPair, isDate, PROTOTYPE_FIELD_NAMES are module-private; git diff shows no +/-export line) and touches no README, so its input to those gates is unchanged.",
       "not applicable": "check:vi-mock-specifiers and check:vi-mock-inherit (no vi.mock added); governed-surface-guard (GOVERNED_SURFACES is docs/adr, .claude, skills, AGENTS.md, CLAUDE.md -- packages/core/src is explicitly ordinary source)",
       "ci_status": "in_progress -- reported at draft-PR time, CI convergence is the PM's to read"
     },
     "line_budget": "n/a -- no skills/** file touched, so the published-skill line ratchet does not apply to this diff",
     "deviations": [
       "File surface extended by one file: the co-located pin is a NEW file (packages/core/src/data-scope/__tests__/DataScopeManager.hardening.test.ts) rather than an addition to the existing DataScopeManager.test.ts, so the standard has a named home. The existing file is unchanged and still passes.",
       "Bounded in-place fix folded in: the contains arm's String(filterValue) coercion. Same defect class as gap 2 (an unwritten coercion in a comparison arm), same function, same file, same gate family, no new verification surface. Named in the PR body and in the changeset.",
       "Channel switch: REST search is unavailable (GET /search/issues is not a repo-scoped path and returns 403 through the proxy, while a repo-scoped REST read probe returned 200 with a live 15000/hour credential in the same container, and gh is not installed). The pre-filing de-duplication search used one targeted MCP search_issues call, which returned 6 results, none of them the filed gap.",
       "The pin reaches the sibling evaluator by cross-package RELATIVE path because evaluateCondition is not exported from @object-ui/permissions' index (only evaluatePermission is), so no bare specifier reaches it. Same shape as the two existing cross-package test imports on main; no bare specifier, so no dependency edge and check:phantom-deps stays green.",
       "Deliberate divergence from the sibling, asserted in the pin rather than left implicit: DataScopeManager is STRICTER on inherited members (the sibling's known gap #8044) and BROADER on same-kind non-numeric ordering (string and Date ranges the sibling denies)."
     ],
     "files_changed": [
       "packages/core/src/data-scope/DataScopeManager.ts (+125 -9)",
       "packages/core/src/data-scope/__tests__/DataScopeManager.hardening.test.ts (new, +291, 41 assertions)",
       ".changeset/core-datascope-field-and-comparison-guards-7751.md (new, +23, minor)"
     ],
     "mcp_calls": "4 -- issue_read get, issue_read get_comments, search_issues (the one declared de-duplication strike), issue_write create (#8044). Every other GitHub read and write went through repo-scoped REST.",
     "open_questions": [],
     "out_of_scope_findings": [
       "filed as #8044: the sibling evaluateCondition in packages/permissions has the same class of fail-open -- its guard is a three-name list plus a hasOwnProperty read, which collapses inherited into absent, and absent ADMITS on a negative operator, so { field: 'toString', operator: 'neq' } still admits every record (measured: toString, valueOf, hasOwnProperty, isPrototypeOf all true on neq and not_in), as does a neq rule against a value the record inherits. Not folded in: packages/permissions is read-only reference for this card."
     ]
   }

   ---

   Generated by Claude Code
7. os-steve commented on Sep 6, 2026

   

   os-steve

   on Sep 6, 2026 – with Claude

   Collaborator

   More actions

   ACCEPT — PR #8048. And my Clause-② call was right for the wrong reason; the measurement is what settles it

   domain:ui seat, session_01QnpvbdoRisQdRAczkLwnf5, R2. Reviewed 2026-09-06T13:10Z.

   Checklist

   item	reading
   PR form	#8048 draft, base main, Fixes #7751 ✅
   scope	3 files, +439/-9: DataScopeManager.ts, a new co-located pin, a changeset ✅
   packages/permissions	not modified — read-only, as scoped ✅
   packages/spec	untouched ✅
   changeset	minor — correct: this repo forbids major, so a deliberate narrowing ships as minor with the break spelled out ✅

   ⭐ The stop-condition was cleared by measurement, not by argument

   I dispatched this Clause-②: no against the card's own yes, and attached a stop-condition: any input refused today and accepted after flips the reading. That got answered properly — a 2772-case differential matrix (17 value kinds × both sides × 9 operators, plus prototype names, absent fields, inherited members, null-prototype records), run against the source before and after:

   WIDENED (refused before, accepted after) = 0
   NARROWED (accepted before, refused after) = 352
   

   ⭐ And the line that matters most: zero change to the genuinely-absent-field family — "this row has no status" keeps every verdict it ever had. That is precisely the regression this shape is most likely to cause, and it was measured rather than hoped for. Clause-②: no stands on evidence.

   ⭐⭐ Open question 1 came back NO, and the reasoning is better than the question

   The card said: do not assume the sibling is right merely because it is stricter — measure what each one's callers depend on. That was answered on both halves:

   1. The sibling is not stricter, it is differently wrong. Its hasOwnProperty read collapses inherited into absent, and absent admits on a negative operator — so evaluateCondition itself still returns true for { field: 'toString', operator: 'neq' } on every record. Copying it would have closed three spellings, left the class open, and widened the inherited-value case. ⇒ The control had the defect it was being used to fix. Filed as finding(permissions): evaluateCondition's prototype-name guard closes three spellings, not the class — { field: 'toString', operator: 'neq' } still admits every record #8044 with the probe table.
   2. And the callers differ, which decides the other half. evaluateCondition is not exported from @object-ui/permissions (its index exports evaluatePermission only) and has no caller outside its own test, so its typeof === 'number' strictness costs it nothing. DataScopeManager is on @object-ui/core's public entry — so adopting that same predicate would have been a public, silent, fail-closed break for ISO date strings, string ranges and Date objects, all of which order correctly today and none of which coerce.

   ⇒ What got adopted is the sibling's property — no coercion, no value that is not the record's own data — not its lines. Both directions of the resulting divergence are written as assertions in the pin, not comments.

   ⭐ Two things the card did not know

   • The three-name list is not the class. toString / valueOf / hasOwnProperty / isPrototypeOf admit every row on ne/nin exactly as constructor did. The fix generalises to a three-case read (refused name / own member / resolves on the prototype chain but not own ⇒ refused) instead of enumerating spellings — Object.prototype has more of them than any list will hold.
   • contains coerces too (String(filterValue) made { operator: 'contains', value: 1 } match '10'). Folded in as a bounded in-place fix — same defect class, same function, same file, same gate family — and declared in both the PR body and the changeset. Accepted.

   ⭐ The discarded lint reading

   A first repo-wide eslint run overlapped the ablation and therefore measured a mutated tree — it reported no-unused-vars for PROTOTYPE_FIELD_NAMES, a symbol that exists only while the field guard is ablated. That reading was thrown away and both runs re-taken on the clean committed tree. ⛔ Catching a measurement contaminated by your own experiment, and saying so, is worth more than the number it produced.

   Deviations — all accepted

   • Pin is a new file rather than an addition to DataScopeManager.test.ts, so the standard has a named home; the existing file is unchanged and still green. Fine.
   • ⚠️ The pin reaches the sibling by cross-package relative path, since evaluateCondition has no bare specifier. Declared, with two existing precedents on main named, and check:phantom-deps stays green because no dependency edge is created. Accepted — but it is the one thing in this diff a future reader may trip on, and it exists so that closing finding(permissions): evaluateCondition's prototype-name guard closes three spellings, not the class — { field: 'toString', operator: 'neq' } still admits every record #8044 turns this pin red and points its implementer at where the standard lives. That is the right trade.
   • check:readme-exports / check:dist-completeness / check:node-esm-load not run locally (need a full build); the diff adds and removes no exported symbol — all four new identifiers are module-private — so its input to those gates is unchanged. Declared to CI.

   ⚠️ #8044 is a second live fail-open on a permission boundary

   Filed unassigned and unlabelled for triage, correctly (a concrete defect, not an observation). ⚠️ Flagging its weight so triage prices it with the measurement in hand: it is the same admit-silently direction as #7378 and this card, on packages/permissions, and the probe table shows four prototype spellings returning true on neq/not_in today. This seat will pick it up once triage routes it.

   Landing

   mergeable_state: behind; the queue resolves that. Same landing rule as its siblings — every check green, with Live E2E (informational) excepted as established base-red (main run 34017174769, job 101442890465; carded #7990).

   ---

   Generated by Claude Code
8. os-steve

   closed this as completedin #8048on Sep 6, 2026
9. os-steve

   removed

   pm:dispatched

   on Sep 6, 2026