feat(lint): gate a hook body's ctx.api write to a readonly field - #13771
Conversation
) A hook's `ctx.api` is a ScopedContext over the TRIGGERING operation's execution context, so `ctx.api.object('x').update({ readonlyField })` reaches the engine as an ordinary non-system caller and the update path strips the key. The call returns success and the column stays null - a failure only an end-to-end read-back detects. This completes the hook side of the flow-side gate that shipped as `flow-update-readonly-field`. The rule keys on the write CHANNEL, not on the field: a beforeInsert / beforeUpdate body stamping `ctx.input.<field> = ...` writes a server value that survives the strip (#5591) and is never flagged - `readonly` plus a before-hook is a correct and widely used pairing. Also skipped, each for a stated reason: `ctx.api.sudo()` chains (elevated, the intended channel), insert/create (INSERT is engine-exempt, #3043/#3413), dynamic object names, non-literal payloads, objects or fields this stack does not declare, and `id` in an update payload (the row address, #8141). - `hook-api-update-readonly-field` - error - `hook-api-update-readonly-when-field` - warning (per-record state) Wired through REFERENCE_INTEGRITY_RULES so it runs on `os validate`, `os lint` and `os compile` at once. `buildReadonlyIndex` is now shared from the flow rule rather than copied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
`check:doc-authoring` gates on this: a finding's `message`/`hint` reaches authors, operators and generated surfaces, none of whom can resolve `#NNNN`. The four ids (#2948, #3042, #5591, #9107) move to adjacent `//` comments, where the reader who CAN resolve them already is. Maintainer ruling 2026-08-12: 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a8d7da766886b19c42565013c252be130e8765f5 && git checkout a8d7da766886b19c42565013c252be130e8765f5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1 915f9cc5cfb9ef72425ff06a92b0e45bd07c2495 && git checkout -B drift-repro 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1 && git merge --no-ff 915f9cc5cfb9ef72425ff06a92b0e45bd07c2495
node scripts/docs-audit/affected-docs.mjs --json 8ab4ace4dd9b17b6e75b90b1cc77193685a748d1
|
… this rule falsifies Three statements went stale the moment a hook-body write check began to gate, and none of them was reachable by the docs drift check: `packages/lint/src/ index.ts` yields no anchor, and a page that states a rule by its INPUTS shares no token with a diff that changed the EMITTER. - hooks.mdx listed "readonly targets" as a flow-only `os validate` check and said a hook body's write set is checked "only as an advisory warning". - hook-bodies.mdx called the whole write side "advisory and literal-only". Existence stays advisory for its own reason (the answer can depend on a package the build cannot see); writability gates because both halves of that judgement are declared in the stack being checked. The pages now say which is which. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
Fixes #13653
A hook body writing a
readonlyfield throughctx.apiis a silent no-op.ctx.apiis aScopedContextover the triggering operation's execution context (buildHookApi), soctx.api.object('x').update({ readonlyField })reaches the engine as an ordinary non-system caller; the update path runsstripReadonlyFieldsunderif (!opCtx.context?.isSystem)and deletes the key. The call returns success and the column stays null for the life of the app.This is the hook-side completion of the flow-side gate
flow-update-readonly-field.The asymmetry the rule is built around
readonly+ abeforeInsert/beforeUpdatestamp is a correct and widely used pairing and is never flagged. The strip drops a key only while it is stillObject.is-equal to what the caller supplied, so actx.input.<field> = ...stamp writes a server value that survives. The judgement is therefore keyed on the write channel, not on the field:ctx.input.<field> = .../Object.assign(ctx.input, ...)ctx.api.object('lit').update()/.updateById()ctx.api.sudo().object(...)ctx.api.object(...).insert()/.create()Both directions are pinned by tests, and the GREEN half is proven load-bearing by ablation (below), not merely asserted.
New rule ids
hook-api-update-readonly-field— errorhook-api-update-readonly-when-field— warning (per-record state)Wired through
REFERENCE_INTEGRITY_RULES, so it runs onos validate,os lintandos compileat once rather than being hand-wired per command.buildReadonlyIndexis now shared from the flow rule instead of copied.Zone 2 — the PM's assumptions, measured
A2.1 — CONFIRMED. The extractor already separates the two write kinds: an
api-crud-literalwrite carriesobject+method;ctx.inputwrites carry neither. Measured with the real extractor, not read off the source.A2.2 — FALSIFIED, and it shrinks the card. The flow half is not something to build from
validate-flow-node-writes.ts: #3425 was closed as delivered, not rejected, andvalidate-readonly-flow-writes.tsalready ships the complete flow-side gate (errorfor staticreadonly,warningforreadonlyWhen, skippingrunAs:'system'andcreate_record). So this card is purely the hook-side completion, and the right move was to mirror that rule rather than invent a posture.A2.3 — severity resolved as
error, and here is the reasoning rather than a coin-flip. The two candidate precedents disagree because they answer different questions:flow-update-readonly-field(error) is this same judgement one surface over.hook-body-write-unknown-field(warning) asks "does this field exist?", which can be wrong for reasons outside the stack — another package may declare the field.The readonly judgement is answered entirely from two facts this stack declares: the field's
readonly, and the body's literalctx.apiupdate. That is the flow rule's epistemic position, not the unknown-field rule's — so it gates.The honest caveat, stated rather than glossed: a hook has no declared run identity. A flow declares
runAs, which is what lets its rule call the strip a certainty. A hook inherits its context from whoever triggered the write, so the write is dropped whenever the trigger is non-system (the default, and the only path a user-reachable object can rely on) and lands on a system-triggered one. The residual case is not a stable invariant — nothing declares or enforces it, and the first user-context write silently voids the stamp. Its remedy is the same.sudo()the rule points at, so the flagged code is worth changing under both readings. That is what makes gating defensible here where it would not be for an existence check.A2.4 —
readonlyWhencovered at the sibling's boundary, not widened. The flow rule already grades itwarning; this rule mirrors that exactly. Its hint differs on a measured point:readonlyWhenstrips abeforeUpdate-derived value too, so the own-hook stamp is not a workaround there and the hint does not offer it.A2.5 — two-way fixtures built in this repo. The reference-app shape is reproduced locally (
crm_account.last_activity_dateas the outage field,name_normalizedas the correctreadonly+ before-hook pairing).A false-positive vector the order did not know about
ScopedContext.sudo()returns a context withisSystem: true, which the strip skips entirely — the hook-side analogue ofrunAs:'system'. It is already structurally invisible to the extractor (the matcher requires a literalctx.apireceiver;ctx.api.sudo()is a CallExpression), so elevated writes cannot be flagged even by accident. A test pins this, so a future extractor change cannot start gating the one channel the platform recommends.Also excluded, each for a stated reason: dynamic object names, non-literal payloads, objects/fields this stack does not declare, objects declaring no fields at all, an unparseable body (a gating rule must not fire off a partially recovered tree), and
idin anupdatepayload — that key is the row address, and the engine deliberately does not report it.Verification
Built at
915f9cc5c.Ablation — the GREEN half is load-bearing. Committed first, then mutated the rule into the naive blanket form (accept every write shape, resolve an object-less write against the hook's own target). Mutation proven on disk before measuring: injected marker
0 -> 2, removed text1 -> 0, blob hash changed. Result: 7 failures, and they are exactly the GREEN cases — all fourctx.inputstamp shapes, both INSERT cases, and the exclusion-ledger drive. The RED cases stayed green. Restore leg verified by state, not exit code: worktree blob hash4f582d01equals the HEAD blob,git diff HEADempty, marker count back to 0. (Source-resolved via the relative import, so there is no dist leg to go stale.)Field data before choosing to gate. Ran the real extractor over every
source:literal inexamples/— 187 body sources, positive controls live (4ctx.inputwrites seen).ctx.apiupdate writes in hook bodies: 0. End-to-end,turbo run buildover the whole workspace is green (70/70 tasks), which includesos buildon all three example apps under the new gating rule.Tests.
@objectstack/lintfull suite 2387 passed / 86 files; the new suite is 27 cases.Gates. Family re-derived on the final commit
915f9cc5cwithnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(53 commands / 49 families, unchanged by the docs commit) and all 53 re-run on that head: 52 GREEN, 1 NOT MEASURED. The one isscripts/check-test-completeness.mjs, exit 3, which the script itself defines as PREREQUISITE NOT MET — it grades a savedturbo run testlog that only CI tees, and its own output says "⛔ It is not a red". Every exit code was captured before any pipe.pnpm lint(repo-wide eslint,.with--no-inline-config) is GREEN with no narrowing.pnpm --filter @objectstack/lint typecheckis green — with the honest qualifier that--listFilesshows it coversvalidate-readonly-hook-writes.tsbut notvalidate-readonly-hook-writes.test.ts(this package hides its tests from tsc, pre-existing ledgered debt), so that green says nothing about the test file's types; vitest exercises it.dispatch-gatesalso warned the tree is 7 commits behindorigin/mainwith 10 gate-defining files changed in that range — CI runs the authoritative family on the merge.A stale sentence in the card, stated plainly
The card lists its prior art under "Prior art, none of it open" and concludes "None of them gave the author a signal". That is stale for the flow half, and correcting it changes what a reader should understand this PR to have added.
#3425 was closed as completed, not rejected. Its closing comment records the delivery:
validateReadonlyFlowWritesshipped in #3465 (4340f1319), wired intoos validate/os build, gating arunAs:'user'update_recordwrite to a static-readonlyfield as an error, gradingreadonlyWhenas a warning, and exemptingcreate_recordandrunAs:'system'— the same carve-outs this rule mirrors.packages/lint/src/validate-readonly-flow-writes.tsis onorigin/maintoday and its header says so.So this PR is not the first author-time signal of its kind. It is the hook-side sibling of an existing flow-side rule, and the design work was to mirror that rule's boundaries rather than to invent a posture. The card's underlying complaint is still exactly right for the surface it was filed about: the hook side had no signal, which is what this closes.
Docs drift check — the reading
The drift check listed
content/docs/api/error-handling-server.mdxvia the anchorupdateById, a string literal in this diff'sSTRIP_SUBJECT_METHODS. Read, no change needed, for two independent reasons: itsupdateByIdexample is an L1handlerhook, and this rule only reads L2 (body.language === 'js') sources, so that code is outside its reach entirely; and the page's other mention states the repository signature — "the record id travels insidedataforupdate" — which is the very fact this rule's address-key exclusion encodes, so the page corroborates the rule rather than being falsified by it.The check declares two blind spots, and both turned out to matter here — hand-checking them found real staleness the tool could not:
packages/lint/src/index.tsyielded no anchor, so pages naming the lint rule roster were not covered by that run.content/docs/automation/hooks.mdxwas one: it listed "readonly targets" as a flow-onlyos validatecheck and said a hook body's write set is checked "only as an advisory warning". Both went false with this PR; both are corrected here.content/docs/automation/hook-bodies.mdxis exactly that shape, so it was re-read by hand: two further over-broad claims ("write side, advisory and literal-only"; "Because the checking is advisory and literal-only") are now scoped, because existence stays advisory for its own reason while writability gates.That drift run also reports its checkout carried uncommitted changes, so its named commit does not fully identify what it read — its row is quoted above as a pointer, not treated as authoritative over the tree.
Docs
content/docs/automation/hook-bodies.mdxcarried the claim that a gating readonly error is something "hooks have no counterpart for" — true before this PR, false after, so it is corrected rather than left to rot, and a new "Writing areadonlyfield" section documents the four-channel table above.content/docs/automation/hooks.mdxand two further "advisory" claims are corrected for the reasons in the drift-reading section.Out of scope, filed not folded
#13770 — the action body surface has the identical hole, with a live population of five
ctx.apiupdate writes in the shipped showcase app (none currently on areadonlyfield). Not folded in here: the ruling narrowed this card to the hook side, and the action surface is a separate rule file with its own ledger and tests.Option 2 of the card (making the strip observable) was not crossed into — it lands in
packages/objectql. Nopackages/spec/src/**change was needed, and no governed-surface file was touched.Generated by Claude Code
Generated by Claude Code