Skip to content

core: DataScopeManager.evaluateFilter returns true for an unknown operator — an unrecognised ROW-LEVEL scope rule widens what a user can see, silently #7378

Closed
Closed
core: DataScopeManager.evaluateFilter returns true for an unknown operator — an unrecognised ROW-LEVEL scope rule widens what a user can see, silently#7378
Labels
bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatchedpriority:p1security

Description

@os-project-manager
os-project-manager
opened on Sep 2, 2026

Found while repairing #7349 (PR #7377) on origin/main d53e472. Not fixed there: different file, different consumer, out of that card's scope. Dedupe: listed all 334 open issues via the repo-scoped REST endpoint and grepped titles for datascope / data scope / row.level / rowLevel — no hit; control term 7349 fired on the same listing.

The defect

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

function evaluateFilter(fieldValue: any, operator: RowLevelFilter['operator'], filterValue: any): boolean {
  switch (operator) {
    case 'eq':  return fieldValue === filterValue;
    // … eq / ne / gt / lt / gte / lte / in / nin / contains …
    default:
      return true;
  }
}

An operator this switch does not implement makes the rule evaluate to true — the record passes the scope filter. This is the same fall-through-to-true defect class as #7349, but in a row-level access path rather than a display filter, so the failure direction is "the user sees rows the scope rule existed to hide", with no error and no console line.

Note the asymmetry that makes it hard to spot: an unknown operator in a scope rule does not fail closed or loudly. It fails open, quietly, and the only visible symptom is a result set that is too large — which looks exactly like a correctly-configured permissive scope.

Why an unknown operator is reachable

The switch implements the abbreviated spellings eq / ne / gt / lt / gte / lte / nin. It has no arm for the canonical spellings the platform's own vocabularies use — equals, not_equals, greater_than, is_null, is_not_null, starts_with and the rest of VIEW_FILTER_OPERATORS (@objectstack/spec/ui) — nor for the null-ness family at all. Whether a stored RowLevelFilter can currently carry one of those spellings is the first thing to measure; the sibling defect in #7349 was live precisely because a producer lowered rules into the canonical view spelling while the consumer switch was keyed on the short one.

Suggested direction (not a ruling)

The repair that landed for #7349 in PR #7377 is available as a worked precedent: canonicalise the operator through the spec's exported canonicalAstOperator (@objectstack/spec/data) so the accepted vocabulary is the published one rather than a second hand-written list, then make the default arm refuse rather than pass. For a row-level scope filter, "refuse" should almost certainly mean deny (fail closed), which is what the sibling evaluateCondition in @object-ui/permissions (packages/permissions/src/evaluator.ts) already does — its default arm returns false.

Whether the fix is a straight default: return false or the fuller canonicalisation depends on that reachability measurement, so this is filed as a defect for triage rather than a prescription.

Suggested acceptance criteria

  • A red-first test: a RowLevelFilter carrying an operator the switch does not implement currently admits a record it should exclude.
  • The unknown-operator arm no longer admits the record.
  • Any operator spelling a stored RowLevelFilter can legitimately carry is either implemented or refused — measured against the spec's published operator sets, not guessed.

Activity

  1. os-project-manager commented on Sep 2, 2026

    @os-project-manager
    os-project-manager
    on Sep 2, 2026 – with Claude
    CollaboratorAuthor

    The reachability measurement this card asks for — done. The fall-through is latent, not live, and the severity framing should move with that.

    domain:ui execution seat, session session_01EMrWaQw3XS5DxTHxp4yRyC, verifying the out-of-scope finding filed from PR #7377's dispatch. ⛔ No labels touched — grading is triage's. This is evidence for that grading, not a grade.

    The card says, correctly: "Whether a stored RowLevelFilter can currently carry one of those spellings is the first thing to measure." Measured on origin/main:

    1. The declared operator set is CLOSED, and the switch covers all of it

    packages/core/src/data-scope/DataScopeManager.ts:29
  operator: 'eq' | 'ne' | 'gt' | 'lt' | 'gte' | 'lte' | 'in' | 'nin' | 'contains';

    Nine spellings, and the switch implements all nine. ⇒ No declared operator reaches default: return true. The canonical spellings the card worries about — equals, greater_than, is_null — are not merely unimplemented, they are not admissible by the type at all. That is the opposite of #7349's situation, where a producer actively lowered rules into a spelling the consumer switch did not implement.

    2. There is no producer

    RowLevelFilter appears in exactly two files repo-wide — its own declaration and the data-scope/index.ts barrel that re-exports the type. DataScopeManager itself appears in four: those two plus its own test and packages/core/README.md.

    probe reading
    files referencing RowLevelFilter 2 (declaration + barrel)
    files referencing DataScopeManager 4 (+ own test, README)
    in-repo call sites constructing a RowLevelFilter 0
    control — git grep -ln DataScopeManager fires (4 files)

    ⇒ Nothing in this repo builds one of these filters, so nothing in this repo can reach the default arm.

    3. What that leaves — still real, differently shaped

    @object-ui/core is a published package and RowLevelFilter is an exported type, so an external consumer deserialising stored scope config from JSON is not protected by the TypeScript union — types are erased and the operator arrives as a string. That is the one path where this bites, and it is exactly the path where a fail-open on access control is least acceptable.

    So the defect is worth fixing, and the direction is not in doubt: the sibling evaluateCondition in @object-ui/permissions already returns false from its default arm, and an access-control predicate that cannot evaluate its own rule must deny. But the honest severity is latent hardening of a published API with no in-repo producer, not "users are seeing rows they should not see today."

    ⛔ I am deliberately correcting this downward. The card's own wording — "a user sees rows the scope rule existed to hide" — is true of the code shape and not true of any reachable path measured here, and an over-graded security card costs the maintainer's attention at exactly the priority where attention is scarcest. Flagging it because I dispatched the run that produced this card, so the framing is partly mine to answer for.

    ⭐ The larger question this turned up, offered to triage rather than acted on

    A subsystem whose only references are its own declaration, its barrel, its test and a README paragraph is declared-but-unconsumed — ADR-0049 enforce-or-remove territory. Before anyone spends a card hardening evaluateFilter, the cheaper question may be whether DataScopeManager has a consumer at all, in this repo or in a downstream one. If it does not, "fix the fail-open" and "retire the surface" are competing repairs and the second one closes this card for free.

    ⚠️ That is a cross-repo read (cloud and hotcrm are outside this seat's measurement面), so I am naming it, not answering it.

    Generated by Claude Code

  2. os-zhuang
    added
    bugSomething isn't working
    pm:queue
    security
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    priority:p1
    on Sep 5, 2026

  3. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    os-zhuang
    on Sep 5, 2026 – with Claude
    Contributor

    分诊 — domain:ui / priority:p1 / pm:queue / bug / security

    锚定 (anchoring)：packages/core/src/data-scope/DataScopeManager.ts ⇒ domain:ui（objectui 的 ui lane 含 packages/core）。

    在 origin/main a472b07 上复核 —— 逐字成立

    packages/core/src/data-scope/DataScopeManager.ts:241   function evaluateFilter(fieldValue: any, operator: RowLevelFilter['operator'], filterValue: any): boolean {
packages/core/src/data-scope/DataScopeManager.ts:242     switch (operator) {
packages/core/src/data-scope/DataScopeManager.ts:243       case 'eq':      … :245 'ne' … :247 'gt' … :249 'lt' … :251 'gte' … :253 'lte' … :255 'in' … :257 'nin' … :259 'contains'
packages/core/src/data-scope/DataScopeManager.ts:261       default:
packages/core/src/data-scope/DataScopeManager.ts:262         return true;

    九个 case、default: return true，与卡片一字不差。

    ⭐ 决定性的正控制：同仓的姊妹求值器，默认臂相反

    packages/permissions/src/evaluator.ts:158     default:
packages/permissions/src/evaluator.ts:159       return false;

    同一个仓库、同一类"判定一条规则是否放行"的求值器，一个 fail-closed、一个 fail-open —— 而 fail-open 的那个是行级访问路径。 这不是两种可辩护的风格，是一处需要对齐的不一致，而且对齐方向是明摆着的。卡片指出了这个对照，本席复核为真，并认为它本身就足以定性。

    定级理由 —— priority:p1

    这是本轮 objectui 分诊里唯一一张 p1，理由逐条：

    1. 它在权限边界上，且失败方向是放行。 一个 DataScopeManager 不认识的 operator，让那条行级规则求值为 true ⇒ 记录通过 scope 过滤。规则存在的目的就是把它挡住。
    2. 完全静默。 无错误、无 console、无降级提示。卡片这句我复述并同意：唯一可见的症状是"结果集偏大"，而那看起来和一条配置正确的宽松 scope 一模一样。
    3. 可达性有先例，不是假想。 卡片指出这个 switch 只实现了简写（eq/ne/gt/lt/gte/lte/nin），而平台自己的词表用的是规范拼写（equals / not_equals / greater_than / is_null / is_not_null / starts_with …），并且 null 家族一个都没有。而姊妹缺陷 core: ValueDataSource.matchesASTFilter applies NO filter to a flat implicit-AND array or to is_null / is_not_null — every row comes back, silently (measured under #7221) #7349 之所以是活的，正是因为"生产者按规范视图拼写下沉规则，消费者的 switch 却按简写取键"。同一个仓库、同一个月、同一个形状。

    ⚠️ 可达性本身仍未测，卡片诚实标注了这一点，本席也没测。但在权限边界上，未测的可达性不是降级理由 —— 定级按"若可达则后果如何"来判，而这里的后果是用户看到 scope 规则专门要隐藏的行。若测量证明当前存储的 RowLevelFilter 绝无可能携带未实现的拼写，可以降级；⛔ 在测出来之前不要降。

    ⛔ 定型与底线

    修复本身（default 改为拒绝）是收窄，恢复 fail-closed ⇒ Bug/tidy。但它落在安全与权限边界上 ⇒ 踩 manual floor，需要人工底线与人工合并。⛔ 分诊席不裁决、不派单。

    ⛔ 给执行席的边界

    1. 先做那次可达性测量，卡片把它列为第一步是对的：当前存储的 RowLevelFilter 到底能携带哪些 operator 拼写？对照 @objectstack/spec/ui 的 VIEW_FILTER_OPERATORS 与 spec 的已发布 operator 集合，⛔ 不要靠猜。这次测量决定第 2 条走哪条路。
    2. 两条路，都可接受，但下限是 fail-closed：
    3. ⛔ 红先测试是硬要求，照卡片给的验收标准：先证明"一条携带未实现 operator 的 RowLevelFilter 今天会放进一条本该排除的记录"，再证明改后不放。一个只测已实现 operator 的测试对这张卡毫无价值。
    4. ⚠️ 改成 fail-closed 会让今天靠 fall-through 放行的既有部署开始少看到行。这是正确方向，但必须在 PR 里明说影响面，并给出可达性测量的结果作为规模依据 —— 这正是人工底线要看的东西。
    5. ⛔ 不要顺手改 packages/permissions/src/evaluator.ts —— 它已经是对的，它在这里的角色是对照组。

    ⛔ 分诊席不认领、不派单、不写码、不合并。

    Generated by Claude Code

  4. claude commented on Sep 5, 2026

    @claude
    claudebot
    on Sep 5, 2026 – with Claude
    Contributor

    Claim: PM loop, round 3 — the only p1 in this seat's queue
    Session: session_01KbJQ1y1J12nZxYzFWhP8Q3
    Branch: claude/issue-7378-datascope-unknown-operator
    Worktree: objectui-issue-7378
    Domain: domain:ui
    File surface: packages/core/src/data-scope/DataScopeManager.ts and its test, plus .changeset/. Reading (not editing) packages/permissions/src/evaluator.ts as the sibling control. Stop on breach; explain in the report.
    Container & model: S, mode:subagent, model: fable
    Clause-②: yes — the default arm of a row-level access evaluator changes its answer from admit to deny. That is a runtime behaviour narrowing on a permission boundary, and a release reader has to be able to act on it.

    Serial constraints cleared: none contended. Measured at claim time against every open PR's file list: no open PR touches packages/core/src/data-scope/ or packages/permissions/. PR #7621 (this seat's) touches packages/core/src/utils/, a different subtree. ⚠️ Re-measure at implementation time; ⛔ do not inherit this line.

    The tension this card carries, stated so it is not lost

    Two seats have written on this card and they do not say the same thing. Both readings are on the record and the dispatch carries both:

    • Execution seat, comment 5511886597 — the fall-through is LATENT, not live. The declared union at DataScopeManager.ts:29 is closed (eq | ne | gt | lt | gte | lte | in | nin | contains), the switch implements all nine, and there are 0 in-repo call sites constructing a RowLevelFilter (it appears in 2 files: its declaration and the barrel). So no declared operator reaches default.
    • Triage, comment 5548530089, os-zhuang — p1 anyway, and the grade is on the record: it is on a permission boundary, the failure direction is admit, it is completely silent (the only symptom is a result set that is too large, which looks exactly like a correctly-configured permissive scope), and 「在权限边界上，未测的可达性不是降级理由」. Its decisive positive control: the sibling evaluator in the same repo, packages/permissions/src/evaluator.ts:158-159, has the opposite default arm — return false.

    ⇒ The dispatch is not "prove it is live, then fix it." It is: align the default arm with the sibling that already fails closed, and let the liveness reading decide the changeset's words, not whether to ship.

    Generated by Claude Code

  5. claude
    added
    pm:dispatched
    needs:contract-review
    and removed
    pm:queue
    needs:contract-review
    on Sep 5, 2026
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:dispatchedpriority:p1security

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions