Skip to content

fix(objectql): 校验规则谓词求值失败时 fail closed,并把合并记录补全为声明形状 (#4649) - #4761

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-4649-validation-fail-closed
Aug 3, 2026
Merged

fix(objectql): 校验规则谓词求值失败时 fail closed,并把合并记录补全为声明形状 (#4649)#4761
os-zhuang merged 2 commits into
mainfrom
claude/issue-4649-validation-fail-closed

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #4649

问题

script / cross_field / conditional 校验规则的 CEL 谓词一旦求值失败,引擎的处理是跳过该规则,只留一条 WARN。规则依然被声明、依然出现在元数据里、依然出现在任何"这个对象受什么保护"的清单里 —— 但它什么也不拦。而且失效范围恰好是触发该形状的那批记录。

校验规则来说这是把保证反过来了:规则存在的意义就是拒绝写入,而它的失败模式是放行写入。HotCRM(objectstack-ai/hotcrm#630)因此让一条线索在不指名幸存记录的情况下被判为重复关闭,而且是在整个测试套件所依赖的内存驱动上发生的 —— 没有任何测试能抓到。

做法(PM 裁定的方案 2 + 方案 1,两半缺一不可)

1. 根修:谓词读到的记录在声明形状上是"全的"

update 时把该对象已声明但缺席的字段物化为 null,与 insert 对齐(#1871 当年只做了 insert);previous 绑定同样物化。这样对着对象声明形状写的谓词永远有值可读,不再取决于驱动回了哪几列 —— "驱动只回写入列"是存储实现细节,作者看不见也不该关心。

物化只覆盖已声明字段:笔误键(record.stauts)必须继续保持不可求值,否则它会被悄悄读成 null 并答"没有违规" —— 那就把方案 1 的兜底抵消掉了。

2. 兜底:物化之后仍求值失败的谓词 fail closed

拒绝写入,错误点名规则名;缺键时同时点名该键并给出修法。severity 仍然管拦不拦 —— warning / info 的规则即使坏了也只记日志,不抛。

3. 连带的正确性修正:conditional 只要声明了 when 就计入"需要 previous"。when 是对着 merged 记录求值的,没有 previous 就等于把一个 PATCH 当成整条记录读 —— 以前这会 fault 然后跳过整条守卫,在 fail-closed 之下则会变成误拒合法的部分更新。让它取到 previous,才是"merged 记录是全的"这句话成立的前提。

行为变更边界(重要,changeset 里写成了显著条目)

  • 以前从没生效过的规则会开始真的执行。 存量部署里可能出现"以前能写、现在 400 VALIDATION_FAILED"的写入。这不是回归,是声明开始被兑现(declared = enforced,与 未知键静默剥离仍是全仓默认:把 #3405 的 strict 收紧从一个 schema 推广到整个可授权面(ADR-0078 完整性闸门) #4001 同轴)。

  • has(...) 包起来的谓词可能开始拒绝。 这一条是本单在实施过程中挖出来的、比 issue 正文更深的一层:has(x) 问的是键是否存在,而一个声明了的列即使值为 NULL 也是"存在"的,所以 has(a) && has(b) && a < b 照样在 null < null 上 fault。也就是说,这类规则在任何会回 NULL 列的驱动上从来就没拦过任何东西 —— 只是 fault 被吞了。正确写法是 != null,不是 has(...):

    - condition: 'has(record.start_date) && has(record.end_date) && record.end_date < record.start_date'
    + condition: 'record.start_date != null && record.end_date != null && record.end_date < record.start_date'

    拒绝消息会把这句话直接讲出来,error.fields[0].constraint{ reason: 'unevaluable', missingKey?, hint?: 'null-comparison' } 供机器消费。has()未声明键的判断仍然正确,那才是它该用的地方。

    ⚠️ 这一条没有破坏 HotCRM 的缓解层的前提:该层用的是相等/空值判断(x == null / x == ""),那类在物化后仍然正确工作(有测试钉住)。真正受影响的是has() 守卫大小比较的形状 —— 而那类规则在真实驱动上本来就是坏的。

  • conditional 规则在 update 时会多一次 findOne

刻意不动:format 的坏 regexjson_schema 编译不了的 schema、字段级 requiredWhen / readonlyWhen / 选项 visibleWhen、以及规则抛异常时的防御性 catch,全部保持原有 fail-open 策略。前三者是本单范围之外的同类问题(见下方 out-of-scope);最后一个是引擎故障而非作者错误,拒绝写入会让整个对象写不进去且作者无从修起。

验收对照

验收项 落点
HotCRM 场景:update 时谓词引用"已声明但驱动未返回"的列 → 规则执行而非跳过 rule-fail-closed.test.tsENFORCES the rule when the prior record omits a declared key entirely
笔误场景:引用未声明键 → 拒绝,错误含规则名与缺失键名 同上 › rejects the write and names the rule AND the missing key
insert 行为回归不变 rule-null-omitted.test.ts(#1871,原样通过)+ behaves identically on insert
has(...) 风格谓词在物化后行为正确、语义被钉住 同上 › #4649 — has() semantics over a materialised field, pinned(4 条)
正文那条 WARN-and-skip 路径不再放行写入 rule-validator.test.ts 里两条原本断言 "fails open" 的用例已翻转为 fail-closed
changeset 在位并写明行为变更边界 .changeset/tender-donkeys-smoke.md

修了我们自己两条从没生效过的示例规则

fail-closed 一上就抓出仓库内两条 has() 守卫大小比较的规则(showcase_project.end_after_startcrm_opportunity.opp_close_date_not_past),它们在任何真实驱动上都没拦过东西 —— dogfood 的 field-zoo-roundtripshowcase-d3-d4-capabilities 两个 suite 立刻变红,正是这个机制在起作用。两条都改成 != null 守卫。

验证

  • packages/objectql 测试:106 files / 1673 passed(新增 rule-fail-closed.test.ts,20 条)
  • @objectstack/dogfood:79 files / 459 passed(修 examples 前是 2 suites failed)
  • 全仓 turbo run test:133 tasks successful
  • 全仓 turbo run typecheck:122 tasks successful
  • 合入 origin/main 后重跑:spec check:generated 8/8 up to date;objectql / rest / dogfood 全绿
  • packages/spec/** 零改动;⛔ content/docs/releases/ 未碰

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny


Generated by Claude Code

claude added 2 commits August 3, 2026 02:54
…make the merged record total (#4649)

A script/cross_field/conditional validation whose CEL predicate could not be
evaluated was logged at WARN and SKIPPED, so the write went through. The rule
stayed declared, listed in the metadata, and enforced nothing — on exactly the
records whose shape triggered the fault. For a validation that inverts the
guarantee: the rule exists to reject a write.

Two halves, neither sufficient alone:

1. The record a predicate reads is TOTAL over the object's declared fields on
   UPDATE as well as insert — `null` when the key is in neither the payload nor
   the prior record — and the `previous` binding is materialised the same way.
   Without this, step 2 would 422 every legitimate predicate on any driver that
   stores only written columns (the shape hotcrm#630 reported). Materialisation
   covers DECLARED fields only, so a typo'd key stays unevaluable and reportable.
2. A predicate that still faults REJECTS the write, naming the rule and the
   offending key. `severity` still governs blocking, so an advisory rule stays
   advisory.

A `conditional` now counts as needing the prior record whenever it declares a
`when`: that predicate is evaluated against the merged record, so without the
prior state it read a PATCH as though it were the whole record.

Fail-closed evaluation immediately found two of our own example rules that had
never enforced anything: `has(x)` is TRUE for a declared column holding NULL, so
`has(a) && has(b) && a < b` faults on `null < null` on any driver that returns
its NULL columns. Both are rewritten with `!= null` guards, and the rejection
message now teaches that distinction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 3:06am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/l labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql.

13 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/objectql)
  • content/docs/data-modeling/formulas.mdx (via packages/objectql)
  • content/docs/deployment/migration-from-objectql.mdx (via @objectstack/objectql)
  • content/docs/deployment/vercel.mdx (via @objectstack/objectql)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/objectql)
  • content/docs/kernel/services.mdx (via @objectstack/objectql)
  • content/docs/permissions/authentication.mdx (via @objectstack/objectql)
  • content/docs/plugins/index.mdx (via @objectstack/objectql)
  • content/docs/plugins/packages.mdx (via @objectstack/objectql)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/objectql)
  • content/docs/protocol/objectql/query-syntax.mdx (via packages/objectql)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/objectql)
  • content/docs/releases/implementation-status.mdx (via @objectstack/objectql)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

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

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Script validation rules are silently skipped when their predicate fails to evaluate — fail-open is the wrong direction for a validation

2 participants