feat(hooks): Hook.runAs — 'system' | 'user' | 'inherit', default 'inherit' - #14915
Conversation
…herit
WIP: schema + engine derivation. Tests, sweep and generated artifacts follow.
A hook's ctx.api is a ScopedContext over the triggering write's context, so a
column protected for a persona by field-level `editable: false` also refuses the
hook that maintains it — the guard and the legitimate writer were the same door.
`runAs` is the declared knob:
- 'system' elevates the hook's ctx.api data operations (the security
middleware short-circuits on isSystem before the field-level
write check), carrying userId through — elevation is not
anonymity, so updated_by still names the operator;
- 'user' pins them to the triggering user, and REFUSES them when no
trigger user resolves (HOOK_UNSCOPED_DATA_ACCESS, the hook-side
twin of the flow engine's #3760 refusal);
- 'inherit' (default) hands the engine-built api through by reference — the
pre-runAs behaviour, so no existing hook changes.
Scope is ctx.api data operations only: condition evaluation, the readonly strip
on ctx.input, ctx.session and async semantics all keep reading the triggering
operation's context.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
Two layers: wrapDeclarativeHook over a real ScopedContext whose engine records the context every data operation carries, and a real ObjectQL dispatch read at the middleware seam plugin-security reads. Covers the three values, the absent key, the ADR-0112 refusal (with 'the engine was never called'), the restore after return and after a throw, the fire-and-forget detachment, the loud refusal of a non-member value, and the ruling's scope fence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
The surface that had no elevation at all: sudo() is not marshalled into the VM, so the same source passed a native handler test and threw TypeError once the build lowered it into a body. No runtime source change is needed — the sandbox reads ctx.api from the engine context at call time, so the wrapper's swap covers it — which is exactly the composition fact a unit test cannot state. Includes a direct handler-vs-body parity assertion at the same seam. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
…ty stack A computed column protected by field-level editable:false and maintained by a hook. Four legs on one boot: the persona's own PATCH is still refused 403; the persona can still write an ordinary column; a runAs:'system' hook BODY writes the protected column and the value lands, stamping updated_by with the operator; and an undeclared hook is still refused with the card's own error text, which is the zero-migration claim measured rather than asserted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
The gating readonly-hook-writes rule now SKIPS a hook declaring runAs:'system' (the static strip skips a system context, so the write it exists to catch does not happen) and its hints name the knob. The hook-bodies table rows that called the absence a gap now point at it, and the canonical semantics live on the hooks page. The conditional-lock refusal is unchanged and still says why: elevation does not waive readonlyWhen, which is what the pre-existing pin predicted would outlive the sudo half. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
Regenerated exactly the two artifacts check:generated proved stale (the api contract and error-code-ledger pages, both carrying the new HOOK_UNSCOPED_DATA_ACCESS registration) plus the liveness state counts for the hook.runAs row. HookSchema's own keys are not walked by authorable-surface/ — only HookContext is — so no surface or defaults shard moves, and check:authorable-surface is green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
- the refusal message carries ADR-0049 only; tracker ids move to the source header (check:doc-authoring), and the dogfood fixture's authored strings lose theirs too; - the recording engine double routes update/delete through the shared dispatch predicates and is registered in the pinned ledger, so it can never be looser than ObjectQL (check:engine-double-contract); - the sandbox stub driver honours the caller's limit by presence (check:objectql-double-limit); - system-context.mdx anchors re-fixed for the one import line this change adds to engine.ts (15 anchors, pure line rot; check:system-context-census OK). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
…field The new Execution-section select carries a label and helpText, so all four locale bundles gain the row; zh-CN / ja-JP / es-ES are hand-translated in the same pass (merge mode preserves them, and the refreshed source hashes are what makes check:i18n read them as translations rather than drift). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
…catch Two defects the newly-registered dispatch predicate and the test typecheck found in my own fixtures, not in the change under test: three layer-1 calls issued an update with no address (a shape the real engine refuses, and now the double does too — the assert takes BOTH the payload and the options bag, since the address may ride either), and a HookHandler returns void | Promise<void>, so the refusal pins await inside try/catch rather than reaching for .catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
📓 Docs Drift CheckThis PR changes 4 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 129 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 26057561ec9d53c5a750e78dcc3ea0a31a8330c4 && git checkout 26057561ec9d53c5a750e78dcc3ea0a31a8330c4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5bc2f2727aef5d697530c63d64dd5592df445776 423e269cec878485fc3162ca5a0a0abfb561ddb4 && git checkout -B drift-repro 5bc2f2727aef5d697530c63d64dd5592df445776 && git merge --no-ff 423e269cec878485fc3162ca5a0a0abfb561ddb4
node scripts/docs-audit/affected-docs.mjs --json 5bc2f2727aef5d697530c63d64dd5592df445776
|
Contract review: PASS — head
|
|
Director seat (session_01WXyGTWPbbreqXow7Z2pZCk), landing check-in 15:19Z: this PR reads Conflict read with
Ask for the Generated by Claude Code |
…ok-run-as-inherit # Conflicts: # content/docs/permissions/system-context.mdx
Discharges the merge commit's os-regen deferral. - liveness/state-counts.md: regenerated from the merged tree (pnpm --filter @objectstack/spec gen:liveness-counts). - system-context.mdx: the merge's only real conflict was one table row whose prose is byte-identical on both sides and differs only in anchor numbers. Resolved per the driver's own notice (take either side, then re-derive), then corrected to the MEASURED lines: the ledger anchors that row by needle, and on the merged tree the needle sits at 10008 — main's 10007 was stale by exactly the one import line this branch adds to engine.ts. field.zod.ts:1537 is main's and stays. check-system-context-census: OK, 140 anchors resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
|
Re-armed, director seat (session_01WXyGTWPbbreqXow7Z2pZCk), head Delta against the reviewed head Generated by Claude Code |
…ok-run-as-inherit
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0174WZTU6XcFcS7g2kykC53i
|
Generated by Claude Code |
|
Correction to the comment above (same seat, 16:28Z): the card comment carrying the delta measurement is 14010#issuecomment-5528757934 — the id written above was wrong and does not resolve. Generated by Claude Code |
|
Director seat (session_01WXyGTWPbbreqXow7Z2pZCk), re-check on head Same method as comment 5528368781: against the reviewed head Generated by Claude Code |
Fixes #14010
Implements the director-batch ruling of 2026-09-01 (comment 5494343943, maintainer 「同意」) — half A of the card. Half B shipped separately as #14044 and is not redone here.
The ruling's seven items — done / where
HookSchemagainsrunAs: z.enum(['system','user','inherit']).default('inherit')packages/spec/src/data/hook.zod.ts'system'/'user'semantics word-for-wordFlowSchema's, incl. the refusal when no trigger user resolvesHookUnscopedDataAccessError(packages/objectql/src/hook-run-as.ts), whose wording mirrors service-automation's #3760 text rather than importing it'inherit'is the hook-only third value; ⛔ not added to FlowFlowSchemais untouched — the diff names no file underpackages/spec/src/automation/default('inherit')= today's behaviour ⇒ zero migrationctx.apithrough by reference; pinned withtoBe, and pinned again as "an undeclared hook is byte-identical toinherit" through a real dispatchctx.apidata operationshook-wrappers.ts);condition, the readonly strip,ctx.sessionandasyncall still read the triggering context — pinned as the scope fenceneeds:contract-review, changeset minor, implement againstflow.zod.ts:686.changeset/hook-run-as-inherit.mdisminorfor@objectstack/spec; ⛔execution.zod.tswas not used (it isExecutionLogSchema, a run log — the PM's registered correction)Where the api is built, and where
runAsis appliedMeasured on
origin/mainat BASE2b9f5810b, not taken from the card:ObjectQL.buildHookApi(opCtx.context)—packages/objectql/src/engine.ts:4146. Five assembly sites hand its result to every hook: find:9175, findOne:9399, insert:9805, update:10712, delete:12669.buildSandboxApi(packages/runtime/src/sandbox/body-runner.ts) returnsengineCtx.apiwhenever it carriesobject(), andinstallApiMethod(quickjs-runner.ts) resolves it at call time.That second fact is why
runAsis applied inwrapDeclarativeHook(packages/objectql/src/hook-wrappers.ts) — the one place both execution surfaces are wrapped (hook-binder.ts:221) — and why no runtime source change was needed. It also keeps this PR clear of #14826, which is open onquickjs-runner.ts.The derivation itself is
ScopedContext.withRunAs(engine.ts), because only the api holds the full triggeringExecutionContext(ctx.sessionis a projection that drops the transaction handle, among other things).The security short-circuit — the evidence that the symptom is actually fixed
packages/plugins/plugin-security/src/security-plugin.ts:1615short-circuits the whole data-security middleware onopCtx.context?.isSystem. The field-level write check that produced the card's[Security] Field write deniedis step 2.5, at:2402. The short-circuit precedes it, soisSystembypasseseditable: false— and the end-to-end pin below demonstrates that on a real booted app rather than reasoning from it.Behaviour table — both surfaces
runAshandlerbody'inherit'(default)ctx.apiat call time'system'{ ...triggering context, isSystem: true }(ScopedContext.sudo())'user', trigger has a user{ ...triggering context, isSystem: false }'user', no trigger userHOOK_UNSCOPED_DATA_ACCESS/ 403Parity is asserted, not assumed:
hook-run-as.integration.test.tsruns the same hook once as a body and once as a handler and compares the contexts that reach the middleware seam.Scope fence and attribution
runAs: 'system'the write that fired the hook still reaches the middleware withisSystem: falseand the operator'suserId— pinned in both the objectql and runtime suites.conditionevaluation happens before the swap and readsctx.input/ctx.previous; the readonly strip readsopCtx.context; neither is touched.runAs: 'system'write carriesuserIdandtenantIdthrough, so the audit stamps (which gate onsession.userId, never onisSystem) still name the operator. Pinned at the seam and again end to end: the elevated hook's write leavesupdated_by= the member who triggered it.Sweep — the prose that said a hook cannot elevate
packages/lint/src/validate-readonly-hook-writes.ts(header)runAs"runAs: 'system'hookctx.apiwrite to areadonlyfieldrunAs: 'system'(the static strip skips a system context, so the write it exists to catch does not happen).'user'/'inherit'are still flaggedsudo()is not an optionreadonlyhint now offersrunAs: 'system'too; thereadonlyWhenhint keeps refusing elevation, because a system context genuinely does not waive a conditional lockcontent/docs/automation/hook-bodies.mdx(two table rows)runAs); #14010 is where that gap is argued" / "silently dropped" without qualificationcontent/docs/automation/hooks.mdxrunAs" section: the three values, the scope fence, attribution, both surfacespackages/spec/src/contracts/scoped-context.tssudo()exclusion notesudostops being a trapskills/**objectstack-automationandobjectstack-datadescriberunAsfor flows onlyobjectstack-automationis worth a follow-up cardRefusal pins of genuinely invalid shapes are unchanged:
HookSchemastill refuses unknown keys, andrunAs: 'elevated'is refused as a value error located atrunAs(not asunrecognized_keys) with the enum's members in the message.Verification
All readings below are from
c59f87b, the branch head; the union was re-run after the final commit.@objectstack/spec—hook.test.ts,strict-object.test.ts,authoring-key-lint.test.ts@objectstack/objectql— the hook family (hook-run-as, binder, wrappers, provenance, bulk per-row, divergence, input-shape)@objectstack/runtime— the wholesrc/sandbox/directory@objectstack/lint— readonly hook + action writes, hook-body writes, rule input tier@objectstack/dogfood—hook-runas-fls.dogfood.test.tstypecheck— spec, objectql, runtime, lintcheck:test-typecheck, whose checked zone covers the new test file)Every command ran through
scripts/pm/os-verify-lock.sh; each verdict is read from that script's ownVERDICT command-exitline, never from a bare$?, and exits were captured before any pipe.Reverse verification
The fix was committed first, so the restore leg had a real restore point. Direction predicted before running: the pins that assert NEW behaviour go red, the pins that assert the PRE-card behaviour stay green — a mixed direction, which is more informative here than an all-red, because the default's whole claim is that nothing changed for an undeclared hook.
Ablation:
installRunAsApireturns a no-op, i.e. the swap never happens.The 7 red are exactly the elevation, de-elevation, refusal, fire-and-forget and scope-fence pins. The 7 green are
'inherit', the absent key, the byte-identity comparison, "auserhook that touches no data still runs", the restore-after-return/throw pin, and the loud refusal of a non-member value — every one of which is a claim about behaviour this card does not change.The mutation was proved on disk by the blob hash moving and a marker count, not by the editor's exit code; the restore by the blob returning to the HEAD blob plus an empty
git diff HEAD, not bygit checkout's exit code. Absolute paths throughout, with atrapon EXIT INT TERM.No rebuild leg was needed, and here is why rather than as an omission: the ablated suite imports
./hook-wrappers.jsand./engine.js— relative, in-package — so vitest reads the source and nodist/sits between the mutation and the measurement. The runtime and dogfood suites do resolve@objectstack/objectqlthrough packageexports(i.e.dist/), which is why they were not used as the ablation subject — and it is also a reading worth recording: the first sandbox run of this card failed against a staleobjectql/distand said nothing about the change under test.Gate family
Derived on the actual change set with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(no path argument — the script takes the change set from the merge base itself), re-derived after the changeset existed: 79 commands, 67 green. The twelve that were not green break into three groups, all resolved or classified:Real findings in this PR's own new code — all fixed, all re-run green:
check:doc-authoringADR-0049, which a customer can resolvecheck:engine-double-contractupdate/deletedid not route through the shared dispatch predicates. Now they do, and the double is registered in the pinned ledger — which promptly caught three of my own test fixtures issuing an address-less update, a shape the real engine refusescheck:objectql-double-limitfindignored the caller's bound. It now applies it by presencecheck:system-context-censuscontent/docs/permissions/system-context.mdxrotted by the single import line this change adds toengine.ts. Pure line rot, repaired with--fix; the census re-reports OK over 109 read sites, and this change adds no new elevation readcheck:i18nzh-CN/ja-JP/es-ESare hand-translated in the same passNOT MEASURED — a prerequisite this container could not meet at the time, each re-run green after the full workspace build:
check:type-check-debt(re-measured 21 ledger entries, 549 raw tsc errors, none above its recorded number),check:dual-build-cjs-loads,check:dev-prereqs,check:skill-examples(256 prose examples type-check).NOT MEASURED — cannot be measured here at all:
check:engine-split-ratio(refuses on a shallow clone rather than printing a wrong ADR-0076 D7 ratio),check:test-completeness(exit 3, needs a savedturbo run testlog),scripts/pm/check-half-states.mjs(needs repo-scoped egress this container lacks; it hung and was killed by PID after ~100s). None of the three is a finding, and none was read as a pass.CI on this PR is the authority on the rest of the farm.
Notes for the reviewer
needs:contract-review, per the ruling's item 6 — the published accept set widens, so this is not mine to flip ready or land.packages/runtimesource change. The sandbox honoursrunAsbecause it readsctx.apiat call time; that is asserted rather than assumed. This also keeps the diff clear of fix(runtime): carry back only the keys a sandboxed hook body wrote #14826, which is open onquickjs-runner.ts.FlowSchemais untouched (ruling item 3), and half B (Stop lowering hook handlers that callctx.api.sudo()into bodies that cannot run it #14044) is not redone.modifyAllRecordsat field level — is a different card and is not addressed here.skills/**was deliberately not edited: governed surface, another lane. Nothing in it is now false; a hook-siderunAsparagraph inobjectstack-automationwould be a good follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01MCHHMCjeFFjGBm7QQ37CWF
Generated by Claude Code