Uh oh!
There was an error while loading. Please reload this page.
Stop lowering hook handlers that call ctx.api.sudo() into bodies that cannot run it - #14044
Conversation
ScopedContext.sudo() is real in-process and is not marshalled into the QuickJS sandbox, so a handler that calls it passes a native hook.handler(ctx) test and TypeErrors once the build lowers it into a body -- aborting the triggering write under the default onError:'abort'. - cli: `.sudo(` joins FORBIDDEN_PATTERNS, so lowerCallables keeps the callable bundled (in-process, where sudo() is real) instead of emitting a body that cannot run it. - lint: the two readonly-hook-write hints prescribed ctx.api.sudo() to a population that is entirely L2 bodies; they now name the own-hook stamp and say the elevation is not reachable from a body. - runtime: pin the VM-side ctx.api surface exhaustively, with a host double that DOES carry sudo, so the absence is a recorded fact. - docs: the readonly table claimed the sudo() row lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
The sandbox error is `TypeError: not a function` -- it names neither the member nor the receiver, so production's one diagnostic identifies nothing. That blindness is the argument for refusing at build time, and the test now records it rather than the message it would have been nicer to get. Also make the CLI reverse-test's assertion quote-agnostic: the test file is itself bundled, and esbuild rewrites the literal before String(fn) runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m
📓 Docs Drift Check2 anchor(s) derived from 2 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 26 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 db2fdd66b4dbb0523687675ca59c07f1bb3dd929 && git checkout db2fdd66b4dbb0523687675ca59c07f1bb3dd929
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5e2c04da7db38c4db0b138fad8b9be5b4ef308fc 0b51c6139d822ab268860aa68511f4b5fc334147 && git checkout -B drift-repro 5e2c04da7db38c4db0b138fad8b9be5b4ef308fc && git merge --no-ff 0b51c6139d822ab268860aa68511f4b5fc334147
node scripts/docs-audit/affected-docs.mjs --json 5e2c04da7db38c4db0b138fad8b9be5b4ef308fc |
hotlong
commented
Sep 1, 2026
CI 全绿,交接给本仓分诊席。 head 保持 draft,我不转 ready 也不合。这是下游 这是 #14010 的哪一半半 B(实现面)。 主诉 —— 给钩子一个可声明的提权旋钮(
本 PR 的范围为什么比"沙箱侧"宽派发词只点了沙箱侧,实际多改了 lint 与文档两处。理由写在正文里,这里复述一句:那两处是陷阱的来源 —— 修法遵循本仓已记录的先例(#4391 两处自我更正,值得评审时留意head 上那个 follow-up commit 修的是测试自己的两个错,不是实现:
无重叠与 #14035(#14011 契约散文)不重叠:那张只改 spec 散文加一个测试,本张改 CLI / lint / runtime / 文档。 Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Part of #14010 — this lands the implementation-side half only. The card's primary
ask (a declared elevation knob for hooks) is not addressed here and is left to
this repo's contract review chain; see "What this deliberately does not do".
The defect
ScopedContext.sudo()is real in-process and is not marshalled into the QuickJSsandbox. The VM's
ctx.apicarriesobject()and the transaction surface, andnothing else — now pinned exhaustively in
quickjs-runner.test.ts, with a hostdouble that does carry
sudo, so the pin proves the member stops at the VMboundary rather than being missing upstream.
Every consumer of that fact had it backwards:
content/docs/automation/hook-bodies.mdxreadonly tablectx.api.sudo().object('x').update(...)— "Lands."TypeErrorhook-api-update-readonly-field(severityerror, gates the build)ctx.api.sudo()"TypeErrorhook-api-update-readonly-when-fieldsudo()is"TypeErrorBoth lint rules read L2 (
language:'js') body sources and nothing else(
extractHookBodyWriteSetparseshooks[i].body.source), so a gating rule wasprescribing a dead call to 100% of its own population.
The path that makes this expensive is the ordinary authoring path. An author writes
an inline
handler, tests it the way the docs teach —hook.handler(ctx)callednatively, against the in-process
ScopedContext, wheresudo()exists — and thesuite is green.
objectstack buildthen lowers that same source into an L2body,and production gets
TypeError: ctx.api.sudo is not a function. Under a hook'sdefault
onError: 'abort'that aborts the triggering write, so it surfaces asan unrelated save being refused. Green tests, dead feature.
Measured here, and pinned: QuickJS reports it as
TypeError: not a function—naming neither the member nor the receiver. Production's one diagnostic identifies
nothing, which is the argument for catching this at build time instead.
The fix
packages/cli—.sudo(joinsFORBIDDEN_PATTERNSinextractHookBody, atthe exact boundary where the two runtimes diverge. This is a repair, not only a
refusal:
lowerCallablesalready registers the callable and ships it through the.mjsbundle when extraction throws, so the handler keeps running in-process,where
sudo()is real, and the build declines only the body that cannot run it.The reason is printed;
--strict-body, which demands a body for every callable,makes it a hard failure — correctly, since a body needing elevation cannot be one.
Same family as the
crypto.hashretirement (#4391), whose note sits three linesbelow in the same file: a member advertised ahead of its implementation, where the
build-time inference was the amplifier rather than the safety net. That precedent
also settles what not to do here —
crypto.hashwas removed rather thanimplemented because implementing it widened the sandbox's security surface, and
sudois that case squarely.packages/lint— both hints now name the own-hook stamp and say plainly thatsudo()is not reachable from a body. Findings, severities and exclusions areunchanged; only the advice moved. Two existing assertions asserted
hint).toContain('sudo'), which still passes against the corrected text whilemeaning the opposite — they now pin the direction, not the substring.
content/docs— the readonly table records what actually happens.Verification
Union re-run at
0b51c61, the final commit:typecheckgreen for all three packages on a built closure (the closure matters:before
turbo run build, all three reportedTS2307piles for unbuilt workspacedist/*.d.ts, which say nothing about this diff).check:type-check-debtre-measured 29 ledger entries, 1531 raw errors, none above its recorded number —
the gate that does see test files even where
typecheckexcludes them.Repo gates run locally, all green:
nul-bytes,corpus-claim-drift,doc-anchors,doc-authoring,docs-single-h1,docs-redirects,docs-audit-scope,role-word,changeset-gate-self-tests,objectui-changeset,pm-half-states,cli-test-child-env,cross-package-test-inputs,test-source-alias,skill-identifier-liveness,check-adr-0087-registration,check-changeset-no-major.Ablation — the new guard is proved load-bearing rather than assumed. With the
FORBIDDEN_PATTERNSentry deleted (mutation confirmed on disk: blob6b76a59cto8c1fe103,sudomentions 7 to 4, survivingrx:anchors 0), exactlythe two new refusal tests go red and the other 19 stay green — so the pattern is
what refuses, and it has not widened into the majority case beside it. Restored by
git checkout HEAD -- ABSOLUTE_PATH, verified bygit diff HEADempty and the diskblob back to the HEAD blob. No rebuild leg was needed: the test imports
../src/utils/extract-hook-body.jsdirectly, so vitest reads source and nodist/sits between the mutation and the measurement.
What this deliberately does not do
A hook still has no declared elevation knob — no hook-side
runAsthe wayFlowSchemahas one — so "this column is computed by automation and neverhand-written" stays inexpressible whenever the maintaining write is cross-object.
That is a contract accept-surface change (
HookSchemaisstrictObjectand todayrejects
runAsas unrecognized), and it is not a mechanical port of the flow key:a hook's
ctx.apiisbuildHookApi(opCtx.context), inheritingisSystemfromwhatever triggered it, so
FlowSchema's.default('user')would de-elevateevery hook fired by a system write and, under flow semantics, refuse a run with no
trigger user. The default alone is a real fork with a live regression in one branch.
Left to the review chain rather than guessed at; the options are written up on the
card.
Worth noting for whoever picks that up: the only elevation route that exists today
is the bundled in-process
handler, and that route is on the L3 deprecation path(
--strict-bodynow, "Phase 3" later, perextract-hook-body.ts). So the knob isnot a convenience — it is the replacement for a capability scheduled to disappear.
This PR stays draft. It is filed from a downstream app's PM seat
(
steedos-labs/hotcrm-heimao), which does not hold triage or merge authority here:no
domain:*label is set, and merge is this repo's call.Generated by Claude Code