lint: warn when an action body writes a readonlyWhen field through ctx.api - #13844
Conversation
…tion-body-readonly-write
…ugh ctx.api
Adds validateReadonlyActionWrites, the action-surface member of the readonly
write family, wired through REFERENCE_INTEGRITY_RULES.
An action body's ctx.api is createContext({ ...callerEnvelope, isSystem: true }),
so the engine's static readonly strip - which runs only under
!opCtx.context?.isSystem - is skipped and a readonly:true write LANDS there. The
conditional strip takes no isSystem exemption, so a readonlyWhen field written
through ctx.api is still dropped on records whose predicate is TRUE. Only that
second shape is reported, as a warning.
ctx.record is excluded from the match set: an action's ctx.record is a dead
snapshot the runtime never writes back, so no strip is ever consulted on it and
a readonly verdict there would be false on every occurrence.
Reuses buildReadonlyIndex from the flow rule and collectActionBodies from the
action rule rather than growing a second walk.
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 c297243b1f4008648e8f51346278786746729e9d && git checkout c297243b1f4008648e8f51346278786746729e9d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f532630d0246e93b36f869579eb5fa4184d51141 337814c259646d285d2b4d24fe1b2a67fa4c3f10 && git checkout -B drift-repro f532630d0246e93b36f869579eb5fa4184d51141 && git merge --no-ff 337814c259646d285d2b4d24fe1b2a67fa4c3f10
node scripts/docs-audit/affected-docs.mjs --json f532630d0246e93b36f869579eb5fa4184d51141
|
…enumeration The 'Writing a readonly field' section's table is hook-scoped and its closing sentence enumerated the surfaces carrying the gate (hook, flow). Landing action-api-update-readonly-when-field would have left that enumeration one short, and left the hook-scoped table readable as covering actions on a page titled 'Hook & Action Bodies'. States the measured difference: an action body runs elevated, so the static strip does not apply and a readonly write LANDS there, while the conditional lock is not waived by elevation and does carry across. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
Part of #13770 — deliberately not
Fixes. The card asked for two rule ids on the action surface; only one of them is true on this tree, and the other needs a decision this PR does not take. Details under "The premise, re-measured" below. #13770 remains open.Adds
validateReadonlyActionWrites, the action-surface member of the readonly write family, wired throughREFERENCE_INTEGRITY_RULESso it runs onos validate,os lintandos compileat once.The premise, re-measured — and half of it does not hold
The card's mechanism is: "An action body reaches the engine through the same
ScopedContext, soctx.api.object('x').update({ someReadonlyField })in an action is dropped by the same non-systemstripReadonlyFieldspass". That is true of a hook body. It is not true of an action body, and the difference is the run identity, not a reading.An action body's
ctx.apiisbuildActionApi=ql.createContext(buildActionExecutionContext(ec)), andbuildActionExecutionContextis three lines:Both production dispatch paths build it that way — REST
/actionsinpackages/runtime/src/domains/actions.ts(whose own comment reads "TRUSTED — system-elevated, RLS/FLS-bypassing by design") and MCPrun_actioninpackages/runtime/src/action-execution.ts. The engine's static strip runs underif (!opCtx.context?.isSystem); the conditional one runs before that guard and takes noisSystemexemption at all.Measured, not read — a real
ObjectQLengine over a memory driver, driven with exactly the context an action body gets ({ userId, tenantId, isSystem: true }), against a field declaredreadonly: trueand a field declaredreadonlyWhen: record.status == 'paid'on a record in the locked state:The repo already pins both halves independently, and both suites pass on this branch:
packages/objectql/src/engine-readonly-strict-writes.test.ts— "strict invents no rejection where the strip does not run — isSystem still writes readonly columns"packages/objectql/src/engine-readonly-when-derived-writes.test.ts— "LOCK 2 — isSystem does NOT exempt a caller-supplied value"So
action-api-update-readonly-fieldaterrorwould gate a build over code that works, on a message ("the write never lands") that is false. That is the claim #8141 removed from the engine's own log, and it is not re-manufactured here. Only the conditional half ships:action-api-update-readonly-when-field— warning. A literalctx.api.object('…').update()/.updateById()in an action body writing a field the named object declaresreadonlyWhen. Elevation is not the remedy and the hint does not offer one: an action body is already system-elevated and the lock still applies. What it names instead is what measures true — confirm the call only targets records whose predicate is FALSE, or derive the field in abeforeUpdatehook on the target object (a hook-written value is not caller-supplied and does land, even on a locked record).The residual is recorded in the rule header rather than guessed at: the third
executeActioncaller, ObjectQL'sObjectRepository.execute(), supplies neitherapinorexecutionContext, so the sandbox falls back to a context-less repo facade and the static strip does run on that path. A gate whose truth depends on which of three dispatchers invoked the action is not a statically decidable fact; the honest fix there is to give that path the identity the other two have. That is the decision left on #13770.The
ctx.recordconfirmation the card and triage requiredStill true on this tree, and it is what fixes the rule's match surface.
buildActionSandboxContext(packages/runtime/src/sandbox/body-runner.ts) bindsrecord: unwrapProxyToPlain(actionCtx?.record)— a materialised plain copy, commented in place as "A snapshot by construction, and read-only by contract (#4345): nothing downstream writes it back" — andboundActionHandlerreturnsresult.valuewith no write-back step.applyMutationsToInputis called on the hook path only; the action path's counterpart iswarnDiscardedRecordWrites, which merely reports.So a
ctx.recordwrite never reaches the engine, no readonly strip is ever consulted on it, and a readonly verdict there would be a false positive on every occurrence.record-property-assignis therefore an entry inREADONLY_ACTION_WRITE_EXCLUSIONSwith that reason attached, andaction-record-write-discardedkeeps ownership of the shape.One refinement on the dispatch's expectation: the rule does not consult
ctxRecordEscapes. That flag answers "could this record write be live?", which gates the discarded-write finding; the readonly question is about a live write being silently stripped. The exclusion here is unconditional, so the escape verdict never enters it. The escaping shape (ctx.record.f = x; ctx.api.object('o').update(ctx.record)) stays silent for a different reason — the payload is an identifier, not a literal, so no field name is statically knowable — and that is pinned as its own test.The
examples/population, re-measured with its positive controlRe-ran the card's method on this branch (the real
extractHookBodyWriteSetover everysource:string literal underexamples/, string-concatenation chains folded):Identical to the card's measurement at
787d75740. None of the three non-address targets carries either flag today:showcase_task.doneandshowcase_task.progressare plain fields, andshowcase_invoice.statusis aField.selectwith noreadonlyand noreadonlyWhen(the object'sreadonlyWhendeclarations sit ontax_rateand the line-item columns, which no action body writes). So the rule reports zero findings acrossexamples/— this is prevention, not a live outage, exactly as the card graded it.The fixture that proves the rule fires
A green run over today's tree proves nothing on its own, so the suite carries a fixture that would fire, modelled on the shipped showcase invoice's state lock and reproduced synthetically rather than imported from
examples/**(maintainer ruling, 2026-08-13):which yields exactly one
action-api-update-readonly-when-fieldwarning atactions[0].body.source, and the same stack withinvoice_numberin the payload yields none — that pair is the falsification above, pinned as a test rather than left as prose. The suite also carries the walk's positive control (a body declared underobjects[].actionsis reached, and a merged action is reported once, not twice), the wholectx.recordfamily,ctx.input,insert/create,sudo, dynamic and unknown targets, theidaddress key in both directions, and a ledger-partition test that fails if a fifth shared write pattern ever lands unclassified.Reuse rather than new machinery
Per the card's scope note, nothing new was built.
buildReadonlyIndexcomes from the flow rule;collectActionBodiesandActionBodySiteare now exported fromvalidate-action-body-writes.tsand shared, so both registration sites, the by-value de-duplication of a merged action, thetype: 'script'default and the authored-location path stay a single implementation. The hook rule's behaviour is untouched.Verification
All on
337814c25, this PR's final commit.pnpm --filter '@objectstack/lint^...' buildpnpm --filter @objectstack/lint typecheckpnpm --filter @objectstack/lint testpnpm --filter @objectstack/lint buildcheck-dts-emitted: 4/4 declaration file(s) presentnpx eslint . --no-inline-config(repo-wide sweep)packages/objectqlreadonly-strip suitesscripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack)The sweep gate was run in full rather than narrowed. The family was re-derived three times — after merging
origin/main(the first derivation warned it was reading stale workflow files), after the code commit, and again after the docs commit below, which pulled in 20 further gates the code-only diff never touched (check:doc-anchors,check:docs-single-h1,check:corpus-claim-drift, the@objectstack/specdocs family, and the rest). All were run on the final commit.Two readings that are NOT MEASURED and are reported as such rather than as green:
node scripts/check-test-completeness.mjsexits 3 withPREREQUISITE NOT METlocally — it grades a savedturbo run testlog that only CI produces. Its own text says to record it as NOT MEASURED.packages/lint/tsconfig.jsonexcludes**/*.test.ts, so the package'stypechecksays nothing about the new test file. Checked separately with a temporary project including tests: zero diagnostics referencevalidate-readonly-action-writes.test.ts(the errors that run reports are pre-existing debt in six other test files, untouched here, which is why the exclusion exists).Docs drift, addressed
The Docs Drift Check on this PR raised two things. Both are answered here rather than deferred.
1.
content/docs/api/error-handling-server.mdx— named, and not falsified. The anchor is the string literalupdateById, which this PR introduces in the new rule'sSTRIP_SUBJECT_METHODS; the existing rule file's diff is purely additive (two symbols exported, JSDoc), with no behaviour change. Read in full, the page makes no claim this PR can falsify: it contains noreadonly,readonlyWhen,stripReadonly,isSystem,sudoor elevation statement anywhere (its onlyreadonlyhits are TypeScriptprivate readonlyclass fields in a circuit-breaker sample). Its twoupdateByIdmentions are a hook example writing a plain, non-locked field, and theScopedContextsignature note — "the record id travels insidedataforupdate" — which is exactly the fact the new rule'sPAYLOAD_ADDRESSED_METHODSencodes. Page and code agree; no edit.2.
packages/lint/src/index.tsyielded no anchor, so the drift run did not cover it — checked by hand, and it did find something. Those eleven lines export a new rule id, and one page enumerates this family across surfaces:content/docs/automation/hook-bodies.mdx, whose "Writing areadonlyfield" section closes by naming the surfaces that carry the gate. Landingaction-api-update-readonly-when-fieldwould have left that enumeration one short — the same defect class as the four pages fixed in #13720 — and, worse on a page titled "Hook & Action Bodies", would have left its hook-scoped table readable as covering actions, which is exactly the reading the measurement above refutes.So this PR adds two sentences there, stating the measured difference: an action body runs elevated, so the static strip does not apply and a
readonlywrite lands, while the conditional lock is not waived by elevation and does carry across. Nothing else on that page is touched.Two neighbouring pages were checked and deliberately left alone.
content/docs/automation/hooks.mdxsays writability is "now gated on both", meaning hook and flow — still true, since the action rule warns rather than gates, and that passage is a hooks-versus-flows comparison rather than a surface enumeration.content/docs/ui/actions.mdxdocuments onlyaction-record-write-discarded, scoped toctx.record, and its closing example (mutate the snapshot, then hand it to an api update) is precisely the escaping shape the new rule leaves silent — consistent, not falsified. No page enumeratesREFERENCE_INTEGRITY_RULESmembers to consumers; the only mention is in a dated audit record underdocs/audits/, which describes the mechanism, not a member list.One thing the drift check surfaced belongs to #13832 rather than here: the same section's
hook-api-update-readonly-when-fieldbullet tells authors thatreadonlyWhenstrips abeforeUpdate-derived value and thatsudo()is the workaround. Both are measured false, both are the prose twin of the hint defect filed there, and neither is created by this PR — so they are recorded on that card for a single fix, not patched here.Out of scope, filed separately
#13832 records a defect the measurement above turned up in the two shipped siblings: the
readonlyWhenhints onhook-api-update-readonly-when-fieldandflow-update-readonly-when-fieldboth recommend elevation (ctx.api.sudo(),runAs: 'system'), which the conditional strip does not honour, and the hook one additionally asserts abeforeUpdate-derived value is stripped — the behaviour #9107 removed. Message text only; not folded in here, because #13770's scope fence forbids touching the hook rule.Authored by Claude Code, session
session_01Pk26oZ12t5N1hwGW1m1MgC— recorded in the prose because a body edit rewrites the footer link.Generated by Claude Code
Generated by Claude Code