Skip to content

HookContext 契约表把 before* 的 input.options 记成 DriverOptions —— 实测那里仍是调用方的 engine options(含 where),两个 break-glass 守卫正读它 #5997

Description

@baozhoutao

观察类发现(finding),来自 #5941(break-glass delete 守卫)的实测。今天没有用户会撞上:代码行为正确且被两个守卫依赖,问题在于同一段散文对它的描述不精确,而这段散文就是下一个作者能读到的唯一规格。

两条陈述

packages/spec/src/data/hook.zod.ts 的 HookContextSchema.input 契约表(#5273 / PR #5668 落地,#5964 刚把枚举注释对齐到它):

   * - delete (bulk, multi:true) — before: { id: undefined, options: DriverOptions }
   * - update (bulk, multi:true) — before: { id: undefined, data: Record, options: DriverOptions }
   ...
   * The row-scoping predicate is NOT reachable from `input` at all. …
   * scope the batch through `options.where` at the CALLER, or work per row on
   * the `after*` events below.

实测(对 origin/main = ffd51fd7a)

packages/objectql/src/engine.ts:

行 事实
5516 → 5517 await this.triggerHooks('beforeUpdate', …) 之后才 hookContext.input.options = this.buildDriverOptions(…)
6137 → 6152 await this.triggerHooks('beforeDelete', …) 之后才做同一件事

也就是说 before* 期间 input.options 仍是调用方那只 engine options 包(EngineUpdateOptions / EngineDeleteOptions),where 与 multi 都在;它变成 DriverOptions 是在钩子返回之后、驱动调用之前。真engine + better-sqlite3 上探针实测到的 beforeDelete 载荷:

by-id      : { inputId: 'u1', inputOptionsWhere: { id: 'u1' }, previous: [...] }
multi      : { inputId: undefined, inputOptionsWhere: { id: { $in: ['u2','u3'] } }, inputOptionsMulti: true }

所以契约表里两处 before 行的 options: DriverOptions 与实测不符,而「NOT reachable from input at all」+「scope … at the CALLER」这组措辞,读起来像是「钩子看不到谓词」——钉子测试并没有钉这一句:hook-input-shape-contract.test.ts 断言的是 'ast' in input === false(以及读路径的阳性对照),没有任何一条断言 input.options 上没有 where。

为什么值得记

这不是措辞洁癖:packages/plugins/plugin-auth 的两个 break-glass 守卫(#5892 的 ban 半边、#5941 的 delete 半边)正是靠 before* 期间的 input.options.where 解析谓词/multi 写的目标行集 —— 没有它,批量写这条能一次扫掉全部管理员的路径就是盲区。按现在的散文,下一个安全钩子作者会得到「谓词拿不到,放弃」的结论,或者反过来读到守卫的代码后认为它违反契约。

两句都可以同时为真,只是要把区别写明:钩子拿不到的是 composed ast(生效谓词,filters 中间件可能往上叠 RLS / sharing 的收窄);拿得到的是调用方原始 options.where。因为中间件只会收窄不会放宽,把调用方谓词当作行集是上界近似 —— 对 fail-closed 的守卫恰好是安全方向。

建议

契约表两处 before 行改成实测形状(调用方 engine options,DriverOptions 是 after* / 驱动调用起才成立),并在那段说明里补一句区分 composed ast 与调用方 options.where;顺手给 hook-input-shape-contract.test.ts 加一条正向断言(before* 的 input.options.where 就是调用方传入的谓词),这样这条被两个安全守卫依赖的性质从散文变成钉子。

参考

Activity

  1. claude commented on Aug 6, 2026

    @claude
    Contributor

    发现分诊:晋级 —— 摘 finding,改 pm:queue + domain:spec

    判级 — 晋级入队,且建议在 spec 车道里排靠前。立单人自评「今天没有用户会撞上」成立(代码行为正确),但观察类的判据不是「代码对不对」,而是「散文错了会不会咬人」——这里会:packages/plugins/plugin-auth 的两个 break-glass 守卫(#5892 的 ban 半边、#5941 的 delete 半边)正是靠 before* 期间的 input.options.where 解析批量写的目标行集,而契约表现在写着 options: DriverOptions + 「The row-scoping predicate is NOT reachable from input at all」。下一个安全钩子作者按这段散文只会得出两个结论之一:「谓词拿不到,放弃」(于是批量写扫掉全部管理员那条路成为盲区),或「守卫违反契约」(于是去删守卫)。两个结论都指向拆掉一道安全防线,这就是它不该留在观察档的原因。

    域 — 落点 packages/spec/src/data/hook.zod.ts(契约表)+ hook-input-shape-contract.test.ts(补正向钉子)⇒ domain:spec(「shared contract surfaces have one owner」)。

    过时前提检查(origin/main 44106d9;正文引用 ffd51fd7a,已前移):

    同文件并发提示(给 spec 座位) — hook.zod.ts 上本轮共有两单:本单与 #6001(positions JSDoc 仍举 services.sharing.canEdit(…) 为正例,同轮入队)。⚠️ 两单改的是同一文件的不同段落,建议同批或明确串行;另 #5899 已 MERGED(2381c19),座位表该行仍记其为在飞,文件锁实际已释放。

    范围提示 — 正文的两句区分是本单的核心产出,建议原样进 PR 措辞:钩子拿不到的是 composed ast(filters 中间件可能叠 RLS / sharing 的收窄);拿得到的是调用方原始 options.where;因中间件只收窄不放宽,把调用方谓词当行集是上界近似,对 fail-closed 守卫恰是安全方向。⛔ 不要为了「让散文变简单」把守卫依赖的这条性质写没了。

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. hotlong commented on Aug 7, 2026

    @hotlong
    Contributor

    Release-board audit (maintainer-directed re-audit of non-board domain:spec items, 2026-08-07): adding target:v17 — criterion ②: the published HookContext contract table says before* input.options is DriverOptions and "predicate NOT reachable", while the runtime passes the caller's engine options including where — the very shape two break-glass guards rely on. Declared ≠ enforced on a public contract. Triage seat may veto.


    Generated by Claude Code

  3. hotlong commented on Aug 7, 2026

    @hotlong
    Contributor

    Triage: domain:spec → domain:spec-surface (seat #6298; existing-stock migration pass, maintainer-instructed 2026-08-07). Direction decided before routing, since it is what picks the seat: the runtime shape here (caller engine options carrying where) is what two break-glass guards depend on, so the contract table is the wrong artifact, not the runtime — a doc-side correction with the acceptance surface untouched. ⚠️ If anyone later rules that the runtime should be narrowed to match the table instead, that is an acceptance change and the card returns to domain:spec.


    Generated by Claude Code

  4. hotlong commented on Aug 7, 2026

    @hotlong
    Contributor

    pm:blocked applied + Blocked-by: #6001 body line (spec-surface seat #6298, session session_01JTSZAjgtL3oR6YcpNDhW3T). Reason: #6001 is in flight (claimed by the spec lane's wave, pm:dispatched) on the same file this card's fix lands in — packages/spec/src/data/hook.zod.ts. Same-file claims serialize strictly across rounds, no exception.

    Recorded for the dispatch that picks this up after unlock (deferring is not shelving):

    1. Triage's stale-premise pass already re-anchored the engine line numbers: engine.ts:5529→5530 (beforeUpdate) and :6150→:6165 (beforeDelete) at 44106d9; re-verify at dispatch time.
    2. The core sentence pair to preserve (from triage, do not simplify away): hooks cannot see the composed ast (middleware may narrow it with RLS/sharing), hooks CAN see the caller's raw options.where; middleware only narrows, so treating the caller predicate as the row set is an upper-bound approximation — the safe direction for fail-closed guards.
    3. The positive pin goes in hook-input-shape-contract.test.ts (NOT hook.test.ts, which the in-flight sweep sweep(spec-surface): 5-item description-surface truth sweep — one claim, one PR, per-item checklist review (#6085 #6081 #6137 #6065 #5676) #6243 touches at :422).
    4. This card is target:v17; it outranks non-board cards the moment it unlocks.

    Generated by Claude Code

  5. self-assigned this
    on Aug 7, 2026
  6. hotlong commented on Aug 7, 2026

    @hotlong
    Contributor

    Claim: PM loop round 3 (domain:spec-surface seat #6298)
    Session: session_01JTSZAjgtL3oR6YcpNDhW3T
    Branch: claude/issue-5997-hook-input-options-contract
    Worktree: objectstack-issue-5997
    Domain: domain:spec-surface
    File surface: packages/spec/src/data/hook.zod.ts (the HookContextSchema.input contract table + its surrounding prose — ⛔ NOT the positions JSDoc, which is #6001's stanza), packages/spec/src/data/hook-input-shape-contract.test.ts (new positive pin), .changeset/*.md (stop on breach; explain in the report)
    Serial constraints cleared: Blocked-by: #6001 released — #6001 closed completed 2026-08-07T15:32:10Z. ⚠️ Recorded honestly: its fix is not yet on origin/main (git grep -c "services.sharing.canEdit" origin/main -- packages/spec/src/data/hook.zod.ts still returns 1), so #6001's PR is presumably still in the queue. Same file, different stanza (contract table vs positions JSDoc); the dispatch carries an explicit in-flight-overlap clause requiring a git merge origin/main and a read of #6001's diff before the final push. ⛔ NOT pinned in hook.test.ts — the in-flight sweep #6243 touches that file at :422.
    Container weight: S (text surface + one test), mode:subagent shared container.

    Correction to this card's earlier pm:blocked note: the Blocked-by: #6001 line has been removed from the body and the label dropped, paired with this comment.


    Generated by Claude Code

  7. hotlong commented on Aug 7, 2026

    @hotlong
    Contributor

    os-dev report for #5997 (PM handover: posting here so the incoming PM can sweep for the marker above).

    {
      "issue": 5997,
      "status": "done",
      "branch": "claude/issue-5997-hook-input-options-contract",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/6426",
      "premise_still_valid": true,
      "summary": "Corrected the HookContextSchema.input contract table in packages/spec/src/data/hook.zod.ts: the two `before` rows typed `input.options` as DriverOptions, but the engine hands `before*` the CALLER's own engine options bag (EngineUpdateOptions / EngineDeleteOptions) with `where` and `multi` present, and merges the driver-facing keys onto it only after the handlers return. Preserved both statements explicitly with the distinction spelled out: the composed `ast` (effective predicate, onto which filters middleware may layer RLS/sharing narrowing) is unreachable, while the caller's raw `options.where` is reachable and is an UPPER-BOUND approximation of the row set because middleware only narrows -- the safe direction for a fail-closed guard. Added a positive pin in hook-input-shape-contract.test.ts covering all four write shapes (update/delete x by-id/multi), keeping the existing `'ast' in input === false` assertions verbatim. Two refinements the measurement forced on the issue's own framing: (a) buildDriverOptions is an ADDITIVE MERGE (`{ ...base }` then only `if (=== undefined)` assignments; returns `base` itself when there is nothing to add), so `where`/`multi` are never stripped -- the issue's 'becomes DriverOptions' reads as replacement and is not; (b) there are THREE consumers of this slot, not two -- besides plugin-auth's #5892 ban half and #5941 delete half, objectql's own isPredicateBulkWrite in hook-wrappers.ts reads options.multi off it. The phase rule holds on every path (find, insert, both writes), so the table marks `options` phase-dependent rather than fixing two rows and leaving their neighbours saying the opposite. Changeset: patch (@objectstack/spec + @objectstack/objectql). No runtime change; accepted set unmoved.",
      "tests": "Reverse verification (direction predicted BEFORE running: RED -- matched). Counterfactual = rebuild the before* slot into a STRIPPED DriverOptions, one line on each write path: `input: { id, options: (() => { const { where, multi, ...rest } = (opCtx.options ?? {}) as any; return this.buildDriverOptions(object, opCtx.context, rest); })() }`. Result: 'Test Files 1 failed (1) / Tests 4 failed | 11 passed (15)' -- the 4 NEW cases all red, all 11 pre-existing cases GREEN, including section 2's `expect(seen[0]!.options).toBeDefined()` (a stripped bag is still defined). That is the measured proof the pin is not decorative: the suite as it stood could not tell the two shapes apart. Whole-package run under the same counterfactual also reddened hook-condition-bulk-previous.test.ts and hook-condition-fail-loud.test.ts -- consumer 3 (isPredicateBulkWrite) going blind, which does not assert the property directly and so is no substitute. Counterfactual reverted; PR diff contains no engine.ts change. LOCAL: `pnpm lint` EXIT=0; `pnpm --filter @objectstack/spec typecheck` + `--filter @objectstack/objectql typecheck` EXIT=0; tests re-run after `git merge origin/main`: 'packages/spec test: Test Files 338 passed (338) / Tests 8649 passed (8649)' and 'packages/objectql test: Test Files 140 passed (140) / Tests 2325 passed (2325)'. GATES: enumerated from .github/workflows/lint.yml and run one by one, 43 check:* all green. One was GENUINELY RED first: check:query-options-erasure ('test surface grew 263 -> 264') because my `engine.count('task', {} as any)` added an erasure site -- fixed the way the gate directs (type the options, do not raise the ratchet) by dropping the `as any`; back to '263 site(s) ... at the ceiling ... none new'. check:i18n and check:i18n-coverage first reported an ENVIRONMENT PREREQUISITE (unbuilt CLI; connector-mcp without dist -- 'COULD NOT MEASURE'), not a defect; both EXIT=0 after `pnpm exec turbo run build`, read via `echo \"EXIT=$?\"` since both gates warn that piping swallows their status. CI ON PR #6426: all 25 check runs converged, zero failures -- ESLint success (this repo's family gates run inside it), TypeScript Type Check success, Test Core 1/3+2/3+3/3 all success (the known #6380 format.exit-code flake did NOT fire), Check Changeset success, Build Core / Dogfood Regression Gate / Temporal Conformance / Spec property liveness all success.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Explicit answers to the four handover questions

    1. Premise verification — still valid, all three legs. Line numbers in the issue and in triage had both drifted again; re-derived against origin/main at b70e534cc:

    Fact Location
    Both before rows still say options: DriverOptions packages/spec/src/data/hook.zod.ts:338 / :341
    beforeUpdate context built from caller options packages/objectql/src/engine.ts:5792 — input: { id, data: opCtx.data, options: opCtx.options }
    beforeUpdate fires, only then buildDriverOptions :5800 then :5801
    beforeDelete context, same :6350 — input: { id, options: opCtx.options }
    beforeDelete fires, only then buildDriverOptions :6425 then :6440
    Guards still read the slot packages/plugins/plugin-auth/src/last-admin-guard.ts:190, :792, :886, :957

    Yes, input.options still carries where during before*. Probed on the real engine — and reference identity input.options === callerOptions is true in all four shapes, i.e. the handler gets the caller's very object:

    update by-id : input.id = 'r_1',     input.options = { where: { id: 'r_1' } }
    update multi : input.id = undefined, input.options = { multi: true, where: { status: 'todo' } }
    delete by-id : input.id = 'r_1',     input.options = { where: { id: 'r_1' } }
    delete multi : input.id = undefined, input.options = { multi: true, where: { id: { $in: ['r_2','r_3'] } } }
    

    2. JSDoc or .describe()? — JSDoc. The PM's zero-regen assumption HELD. The contract table sits in the /** ... */ block above input: z.record(...).describe('Mutable input parameters'), so it is not a .describe() input. Measured, not assumed: after pnpm --filter @objectstack/spec gen:schema, git status --porcelain was empty — zero regeneration under packages/spec/json-schema/** and content/docs/references/**, and zero drift in authorable-surface.base.json.

    3. #6001 — had NOT landed; no clobber possible. At the moment I ran git merge origin/main (no rebase, no force-push), git grep -c "services.sharing.canEdit" origin/main -- packages/spec/src/data/hook.zod.ts still returned 1. I did not touch the positions JSDoc stanza; this PR's diff on that file touches 0 canEdit lines. hook.test.ts (PR #6243's file) is untouched — the pin went into hook-input-shape-contract.test.ts as directed.

    4. Would the pin go red on a stripped DriverOptions? — YES, measured, and the old suite would not have. 4 failed | 11 passed (15): exactly the four new cases red, every pre-existing case green. Each case asserts reference identity toBe(callerOptions), options.where deep-equal to the caller's predicate, and the existing 'ast' in input === false restated in place so the "composed unreachable / raw reachable" pair can only be read together.

    Housekeeping

    No blockers. PR #6426 is a draft and nothing was merged; the assignee was not touched. skip-changeset does not apply — this PR ships a patch changeset for @objectstack/spec + @objectstack/objectql and CI's Check Changeset is green, so no label write was made (note for the record: issue_read get_labels cannot resolve a PR number, as the playbook says; the bots' own labels from the green Auto Label run were left untouched). Worktree objectstack-issue-5997 removed after the PR went up. No out-of-scope findings were filed: nothing outside the card's scope turned up that a user hits today.


    Generated by Claude Code

  8. hotlong commented on Aug 7, 2026

    @hotlong
    Contributor

    Review: ACCEPT — PR #6426. Flipped ready, auto-merge armed.

    Reviewed by the domain:spec-surface seat (#6298, session session_01JTSZAjgtL3oR6YcpNDhW3T) against the diff.

    Every criterion this card was dispatched under is met, and two are exceeded.

    The load-bearing requirement — the pin must be able to go red

    My dispatch said a pin that stays green when input.options is stripped to a DriverOptions is not a pin of this property. Measured counterfactual in the PR: rebuilding the before* slot into a stripped bag turns exactly the 4 new cases red and leaves all 11 pre-existing cases green — including §2's expect(seen[0].options).toBeDefined(), which a stripped bag still satisfies. That gap is the justification for the pin, and it is stated as such rather than implied.

    The strongest of the three per-case assertions is toBe(callerOptions) — reference identity, not shape equality. A substitute that happened to copy where across would still fail it. That is a better assertion than the card asked for.

    Both sentences survive, and can no longer be read apart

    The core deliverable. The JSDoc now carries composed-ast-unreachable and caller's-raw-options.where-reachable together, with the reason they are simultaneously true (middleware only narrows ⇒ the caller's predicate is an upper-bound approximation ⇒ safe for a fail-closed guard, wrong for anything needing the effective set exactly). The old sentence "A bulk write therefore hands hooks no queryable predicate" — the one actually being misread — is rewritten rather than deleted. And §1's 'ast' in input === false is restated inside each new case so neither half can be read alone. Pin is in hook-input-shape-contract.test.ts, hook.test.ts untouched (#6243 safe).

    Three things measured beyond the brief

    1. The issue's own framing was corrected, in the stronger direction. HookContext 契约表把 before* 的 input.options 记成 DriverOptions —— 实测那里仍是调用方的 engine options(含 where),两个 break-glass 守卫正读它 #5997 says the slot "becomes DriverOptions after the hooks return", which reads as replacement. Measured: buildDriverOptions is an additive merge — spreads the base, only assigns keys that are undefined, never deletes; with no transaction/tenant/timezone it returns base unchanged. So the caller's where/multi are never stripped and the widening is one-way. Independently corroborated at hook-wrappers.ts:543. The prose is written to the measurement, not to my card's wording.
    2. There are three consumers, not two. Besides plugin-auth's ban (plugin-auth: break-glass 守卫 —— SCIM/ban 不得停用最后一个管理员(ADR-0024 D5.2,cloud#621 转入) #5892) and delete (最后一个管理员的「删除」路径无守卫:目标不持本地密码时,SCIM/admin remove 可删掉环境最后一个管理员 #5941) halves, objectql's own isPredicateBulkWrite reads options.multi off the same slot. Recorded in both the prose and the test header — future narrowing now has a complete list to re-read.
    3. The fix was widened to the whole table, correctly argued: the phase pattern holds on find and insert too, so fixing two rows while their neighbours kept saying the opposite would have left the next author tripping over the same thing. A new PHASE paragraph states the rule once.

    Discipline notes

    check:query-options-erasure went genuinely red (263→264) because a new engine.count('task', {} as any) added an erasure site. It was fixed the way the gate directs — by typing the options, not by raising the ratchet — dropping the as any since count(object, query?) already infers the empty query. Back to 263, at the ceiling, none new. The test even carries a comment explaining why no as any there. That is the right resolution of a ratchet gate and worth naming.

    Generated artifacts: zero regen, git status --porcelain empty after gen:schema — the contract table is a JSDoc block, not a .describe(). Third independent confirmation of that lane fact this shift.

    One correction to my own claim comment: I listed the pin's path as packages/spec/src/data/hook-input-shape-contract.test.ts. It actually lives in packages/objectql/ — where the sibling 'ast' in input === false assertions already are, which is the right home. The dev put it in the correct place; the wrong path was mine. Not a scope breach.

    Landing

    Flipped ready, auto-merge armed. Changeset patch for @objectstack/spec + @objectstack/objectql, correct for a prose-and-pin change that moves no accepted set.


    Generated by Claude Code

  9. added a commit that references this issue on Aug 8, 2026
    5d022a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions