Skip to content

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

Description

@claude

Carved out of the contract review of PR #7748 (card objectui#7378) by the domain:ui PM seat. Pre-existing — PR #7748 neither introduced nor widened these; it aligned the default arm and the reviewer measured these two while comparing the pair.

Split out from the liveness-decision card objectui#7750 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.

The two gaps, measured by probe against the head source

packages/core/src/data-scope/DataScopeManager.ts vs the sibling packages/permissions/src/evaluator.ts, which is the repo's other evaluator of the same kind and is the one the #7378 triage used as its decisive control.

1. ⭐ No field-name guard — a prototype key admits everything

{ field: 'constructor', operator: 'ne', value: <anything> }   ⇒ admits [1, 2, 3]

The sibling refuses __proto__ / constructor / prototype and reads members with hasOwnProperty. DataScopeManager does neither, so a scope rule naming a prototype key evaluates against the prototype chain rather than the record — and on a ne comparison that means every row passes.

⚠️ Same failure direction as #7378admit, silently — on the same permission boundary. That is why it is filed rather than noted.

2. Unguarded comparisons coerce

{ field: 'age', operator: 'gte', value: 0 }   ⇒ admits null  and  admits '10'

The sibling requires typeof === 'number' on both sides before comparing. This evaluator does not, so null and numeric strings fall through the comparison arms.

What this card is

Bring the two evaluators to one hardening standard, and pin the standard so they cannot diverge a third time.

⚠️ Open questions the implementer must answer rather than assume:

  • Is the sibling's shape the target, or is there a reason DataScopeManager should coerce? ⛔ Do not assume the sibling is right merely because it is stricter — measure what each one's callers depend on.
  • Does fixing Add automated testing infrastructure and CI/CD workflows #2 change any currently-admitted row for a legitimate rule? That is a behaviour narrowing and needs its own changeset sentence (Clause-② yes).
  • The two evaluators also differ in spelling: ne/nin here vs neq/not_in there, and the sibling implements is_null/is_not_null which this one does not. ⛔ Do not fold that in — spelling is the liveness-decision card's question, not this one's.

Refs: objectui#7378 · PR #7748 · objectui#7750 (the liveness decision) · packages/permissions/src/evaluator.ts:158-159.


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:queuepriority:p2security

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions