Skip to content

fix(objectql): update 剥离作用于调用方提交的值,不再连坐抹掉 beforeUpdate hook 的写入 (#5591) - #6343

Merged
baozhoutao merged 3 commits into
mainfrom
claude/issue-5591-strip-caller-values-only
Aug 7, 2026
Merged

fix(objectql): update 剥离作用于调用方提交的值,不再连坐抹掉 beforeUpdate hook 的写入 (#5591)#6343
baozhoutao merged 3 commits into
mainfrom
claude/issue-5591-strip-caller-values-only

Conversation

@baozhoutao

@baozhoutao baozhoutao commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #5591

问题

静态 readonly 剥离跑在 beforeUpdate 之后,但判据是入口处快照的调用方键名集合。一旦 hook 往某个 readonly 列写了值,「调用方提交过这个键」与「这个键上现在还是调用方的值」就是两件事,而 delete data[name] 删的是后者 —— 于是调用方 payload 里碰巧带了同名键,hook 同一次写入的戳就跟着被删。

前提核验(三条,均在 origin/main 实测,非纸面推断)

前提 结论 证据
P1 suppliedKeys 快照在 hook 前、strip 在 hook 后且 delete 当前值 ✅ 成立 engine.ts:5697(快照)→ :5781(triggerHooks('beforeUpdate'))→ :5928 / :6020(两处 strip);rule-validator.tsdelete result[name]
P2 正文实测(published_at = null)可复现 ✅ 复现 真 ObjectQL + InMemory 驱动 + sys_fetch_previous_update 复刻 + 真 publish hook,落库 {"status":"published","published_at":null,"last_reviewed_at":"2026-08-05T10:00:00.000Z"} —— 与 issue 正文逐字一致,含 last_reviewed_at 活着这一非对称
P3 #4903 对照面(hook 写 key 能落库)仍成立 ✅ 成立,且修法未动它 同一 hook 在调用方不回传该键时落库 published_at = NOW

P2 的那条非对称是本单最有力的证据:同一次写、同一个 hook、两个都是 readonly 的列,调用方回传过的那个死了,没回传的那个活着。存亡只取决于 payload 里有没有同名键 —— 不可能是有意语义。

三路线取舍(按实测,不按偏好)

路线 实测判据 取舍
A. hook 前剥离 探针实测:beforeUpdate hook 当前能看到 ["id","title","published_at"]。hook 前剥离会把 readonly 键从 hook 视野里抹掉 ❌ 否决
B. 快照携带,剥离时比对身份 时序一动不动,只收窄 strip 的判据 采用
C. supplied 快照与 hook 写入分账 需要在引擎里维护第二套 payload 账本;B 已用一个参数达到同一效果 ❌ 过重

否决 A 的实测依据具体到在案代码:plugin-auth 的 ADR-0092 身份写守卫(identity-write-guard.ts guardUpdate)读 ctx.input.data 的键集,并把非白名单键写进它抛出的错误正文和 warn 正文(None of the submitted fields (…) are editable)。hook 前剥离会让这些键在守卫看到之前就消失:纯 readonly 提交时该错误退化成 (—),混合提交时 warn 不再点名 —— 一条 loud 诊断被静默削弱。B 路线让调用方 payload 原封不动地到达 hook,只收窄剥离自己的判定。

修法

入口快照改为携带调用方的(必须是浅拷贝:hook 原地改 opCtx.data,hookContext.input.data 起初就是同一引用),剥离时多一道身份比对:

两处细节写进了注释,因为都是会被后人「顺手简化」掉的:用 Object.is 而非 ===(=== 认为 NaN !== NaN,会把调用方伪造的 NaN 读成「hook 改过」而保留伪造值 —— 唯一一处松散运算符会反转结论的输入);用 own-property 而非 in(字段机器名正则允许 constructor,in 会在任意普通对象上为它返回 true,从而剥掉一个 hook 戳)。

stripReadonlyFields 未从 packages/objectql/src/index.ts 导出,是包内函数,故签名可直接收敛,无对外破坏面。

语义边界(#2948 / #3003 / #3015 一字未弱化)

调用方提交、无 hook 覆写的 readonly 值,照旧被剥 —— 单条与谓词两条路径都有用例钉住。isSystem / preserveAudit 未动。insert 面未动:其自身更窄的 stripRuntimeOwnedFields 带同类缺陷(实测:调用方提交 + hook 覆写 → hook 写入被删、序列补发),已另立 #6339,本 PR 不修,并留了一条 insert 不受影响的回归钉。

反向验证(方向先写死,再跑)

肢 A = 还原剥离判据(删掉值身份比对,回到键集读法)。预测:正文场景用例翻红,published_at 回 null;#2948#4903 全绿。 预测在跑之前写死在 8 条具名用例上。

实测结果 —— 8 红,与预测逐条吻合,预测为绿的 130 条全绿:

× KEEPS a readonly key a hook OVERWROTE, even though the caller supplied it
× KEEPS a hook-REWRITTEN record number the caller DID supply (#5591)
× THE REPORT: a whole-record write-back lands the hook stamp, not null
× the two read-only columns of one write now agree (the asymmetry is gone)
× the BULK path is fixed on the same terms
× a hook-overwritten key is NOT reported as dropped to onFieldsDropped
× strictReadonlyWrites refuses the forge and admits the hook write
× [#5591] a hook OVERWRITING a key the caller supplied now survives the strip

AssertionError: expected null not to be null      ← 正文那一行,原样翻红
Test Files  3 failed (3)
     Tests  8 failed | 130 passed (138)

expected null not to be null 就是 issue 正文报告的那行数据本身。

用例处置(三档分开判,不批量重拼)

engine-readonly-strip-signal.test.ts 里那条 a hook cannot rescue a key the CALLER supplied整条替换,不是改写:它钉的正是本 PR 删掉的那条肢,原注释自己写着「pinned as EXISTING behaviour so the next change to it is deliberate. Nothing here endorses it」—— 本 PR 就是那个 deliberate change,替换后连同该文件头部的机制描述一并更新。其余单测是重拼(new Set([...]) → 调用方 payload 快照对象,值与 data 一致,判定不变)。

测试

新增 packages/objectql/src/engine-readonly-strip-caller-values.test.ts(13 例):正文场景、非对称消失、#2948 两例(含「hook 存在但本次未触发」这一档)、#4903 对照、bulk 两面、onFieldsDropped 两面、strictReadonlyWrites 两面、hook 仍能看见调用方值(A 路线否决理由的钉子)、isSystem、insert 回归。

pnpm --filter @objectstack/objectql test      → Test Files 140 passed, Tests 2298 passed
pnpm --filter @objectstack/objectql typecheck → tsc --noEmit, 干净
node scripts/check-engine-double-contract.mjs → OK — 77 pinned, 133 DEBT, 2 exempt
node scripts/check-durability-degradation-log-level.mjs → ✓ 24 seam(s) all loud
node scripts/check-nul-bytes.mjs → OK (5990 files)
pnpm lint (eslint . --no-inline-config) → 干净
pnpm check:query-options-erasure → ✓ 267 test site(s),在天花板上,未新增

远端 CI:24 个 check 全绿(2 个 skipped 属正常路径过滤),含 Check Changeset、TypeScript Type Check、Test Core ×3、Dogfood Regression Gate ×3。首推曾红一次,原因不是 ESLint 本身而是同 job 的后续步骤 check:query-options-erasure:新用例里复刻 sys_fetch_previous_update 的那次 by-id 查询用了 as any,把 #4918 的测试面计数顶到 267 → 268。该查询并非有意脱离契约(就是一次普通 by-id 读),故按该 ratchet 指定的两条出路里选了「把 options 定型」而非 as unknown as,计数回到 267。

消费半径扫过(该规则只由 engine.ts 消费,故按行为半径跑下游包):
plugin-pinyin-search 14 / plugin-audit 112 / plugin-sharing 359 / platform-objects 275 / metadata-protocol 502 / plugin-security 768,全绿。顺带一提:plugin-pinyin-search__search(readonly + system)正是被 beforeUpdate hook 写的投影列,整记录回传下旧行为会静默删掉 hook 重算的值、让搜索投影变陈旧 —— 本 PR 一并修好。

packages/qa/dogfood/test/authz-conformance.matrix.ts 的 enforcement 描述同步收敛为「caller-supplied VALUES」,避免留下一处 declared ≠ enforced 的陈述。

已知边界(写进代码与 changeset,不粉饰)

快照是浅拷贝,所以 hook 若原地修改调用方提交的对象/数组,身份不变、无法与「hook 没动」区分,该字段仍会被剥。除非每次写都深拷贝 payload,否则任何比较都看不见这一档,这条路径不付这个代价;回落方向是 #5591 前的行为(剥离),即 fail-safe。已单独钉成用例并写明:hook 要写 readonly 列请赋值

必答项

Generated by Claude Code

claude added 2 commits August 7, 2026 14:11
…5591)

The static-`readonly` write strip runs after `beforeUpdate`, but decided what
to delete from a snapshot of the caller's KEY NAMES. Those are different facts
the moment a hook writes to a read-only column: `delete data[name]` took the
hook's value with it whenever the caller's payload happened to carry the same
key.

Measured downstream (hotcrm#788): a REST caller reads a whole
`crm_knowledge_article`, flips `status` to `published`, and PUTs the whole
record back -- `published_at: null` included, because that is what it read. The
publish hook stamped `published_at` on the transition; the strip then deleted
the stamp, and the row committed as `status = "published"` with
`published_at = null`. The same hook's `last_reviewed_at` -- equally read-only,
but not echoed by the caller -- landed in that same write.

The entry snapshot now carries the caller's values (a copy: hooks mutate
`opCtx.data` in place), and a read-only key is stripped only while it still
holds the caller's own value. A key a hook overwrote is a platform write and
survives -- the same verdict #4903 already pins for a read-only key a hook ADDS.

Route chosen by measurement, not preference: stripping BEFORE the hooks would
also work, but a `beforeUpdate` guard that reports on what the caller submitted
reads `ctx.input.data` -- plugin-auth's ADR-0092 identity write guard NAMES the
non-whitelisted keys it finds -- and pre-hook stripping empties that out. The
caller's payload therefore still reaches the hooks unchanged; only the strip's
verdict narrowed.

Not a relaxation of #2948 / #3003 / #3015: a caller-supplied read-only value no
hook overwrote is dropped exactly as before, on both the single-id and predicate
paths, and `isSystem` / `preserveAudit` are untouched. INSERT is unchanged (its
own narrower strip carries the same defect -- filed as #6339, not fixed here).

Reverse-verified: restoring the pre-fix reading turns exactly the 8 predicted
pins red -- including `expected null not to be null` on the reported scenario --
and leaves every #2948 and #4903 case green.

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

vercel Bot commented Aug 7, 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 7, 2026 2:39pm

Request Review

@github-actions github-actions Bot added the size/l label Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/dogfood.

16 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/runtime-services/examples.mdx (via packages/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/permissions/authorization.mdx (via packages/qa/dogfood)
  • content/docs/permissions/delegated-administration.mdx (via packages/qa/dogfood)
  • 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.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 7, 2026
…h `as any`

The #4918 query-options-erasure ratchet counts an erased engine query bag in
test code too (test surface is outside the BLOCKING rule, not outside the
count), and the new suite's `sys_fetch_previous_update` replica pushed it
267 -> 268. The bag is not deliberately off-contract here — it is an ordinary
by-id lookup — so the remedy is the typed one, not `as unknown as`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We
@baozhoutao
baozhoutao marked this pull request as ready for review August 7, 2026 15:00
@baozhoutao
baozhoutao added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit 9c82b89 Aug 7, 2026
28 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-5591-strip-caller-values-only branch August 7, 2026 15:16
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

2 participants