Uh oh!
There was an error while loading. Please reload this page.
fix(cli): refuse to lower hook bodies that reference globals the sandbox does not provide (#14301) - #14661
Conversation
…lobals (#14301) `detect-free-identifiers`' ambient allowlist was one generous list documented as "assume the runtime has it". The runtime a lowered body runs in is the QuickJS sandbox, not the Node process that runs `objectstack build` — and the list named `Intl` beside `JSON`. A handler calling `Intl.DateTimeFormat` therefore had no free identifier: it lowered into `body.source`, the #13651 lint rule had nothing to report (it fires only on a refused lowering), every local gate was green because the in-process test runs the raw function in Node, and production threw `ReferenceError: Intl is not defined`. Split the allowlist into `SANDBOX_GLOBALS` (53) and `NODE_ONLY_GLOBALS` (15), with membership MEASURED by a `typeof`/`in globalThis` probe evaluated inside the same `QuickJSScriptRunner` the runtime uses and pinned by a test that reads that probe. A free reference to a host-only name is now reported and refused with a reason that names the identifier and the remedy — a string handler ref or a validation rule, and `ctx.log` for `console` — travelling the existing `free-identifiers` path, so `lowerCallables` still bundles the callable and `os build` still exits 0 with a warning. Not changed: whether `os build` fails on the lowering class (#13838), what the sandbox provides, and anything under `packages/runtime/**`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…ndbox-globals-allowlist
📓 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 — 22 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 bcec761c21b97b1dc8ed9cd61f527e6eb4fa6653 && git checkout bcec761c21b97b1dc8ed9cd61f527e6eb4fa6653
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2aa8456cf2d66ec3825d262686fe4218e57cfd27 f97d3b85c46399f2ccd02ec4c084a6278be20970 && git checkout -B drift-repro 2aa8456cf2d66ec3825d262686fe4218e57cfd27 && git merge --no-ff f97d3b85c46399f2ccd02ec4c084a6278be20970
node scripts/docs-audit/affected-docs.mjs --json 2aa8456cf2d66ec3825d262686fe4218e57cfd27
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14301
detect-free-identifiers' ambient allowlist was ONE generous list, documented as "assume the runtime has it". The runtime a lowered body actually runs in is the QuickJS sandbox, not the Node process that runsobjectstack build— and the list namedIntlbesideJSON, under the comment "Web-ish that the sandbox / Node commonly provide". So the reported handler had no free identifier at all, lowered intobody.source, passed every local gate, and threwReferenceErrorin production.This splits the allowlist in two, measures the membership inside the real sandbox, and turns a free reference to a host-only name into a lowering refusal that names the identifier and a remedy that is actually possible.
The measurement
packages/cli/src/utils/sandbox-globals-probe.test.tsevaluates, for every member of both sets,typeof X !== 'undefined' || 'X' in globalThisinside a realQuickJSScriptRunner— the sameScriptRunnerAppPluginwires for hook and action bodies, on the samerunScriptpath, with the same empty capability set a body with no inferred capabilities gets. The pin fails unless each set is exactly the probe's partition, so a name added from memory reddens it.Two limbs per name, not one:
typeof Xalone reports the single global whose VALUE isundefined—undefineditself — as absent, and would have demanded it be listed host-only.'X' in globalThisasks existence instead of value.Measured present —
SANDBOX_GLOBALS(53):Measured absent —
NODE_ONLY_GLOBALS(15):53 + 15 = 68 = the size of the
GLOBALSset this replaces, and the two sets are disjoint — no name was dropped or invented in the split. Three of the readings are worth stating out loud:Intlis the reported one: ECMA-402, standard in every browser and in Node, absent here.consoleis a HOST object; the sandbox deliberately routes logging through the capability-gatedctx.log(buildBodyLogSurfacein the runtime's body runner) and installs no global. So the refusal forconsolealso namesctx.log— the one entry in a deliberately closed replacement table, because "keep it in a string handler ref" is poor advice for a log line when the platform's own answer is one capability away.argumentsis the one member that is not a global at all. It is an implicit binding of ordinary function scope, and the runner wraps a lowered body in an ARROW ((async (ctx) => { … })(ctx)), which provides none. It measures absent for a different reason than the rest and is refused for the same one: afunction (ctx) { … arguments … }handler works in-process and throws once lowered.Red-first
The card's reproduction, run against byte-for-byte copies of the two base (
HEAD) sources so the reading was about the tree as it was rather than the working tree:No free identifier, extraction succeeded, and the emitted
body.sourceis theIntlcall verbatim. After the change the same handler is REFUSED (kind: 'free-identifiers',freeIdentifiers: ['Intl'],nodeOnlyIdentifiers: ['Intl']), pinned intest/extract-hook-body.test.ts.The named refusal
The module-scope sentence is unchanged and pinned byte-identical (
content/docs/automation/hook-bodies.mdx,os build's warn-and-bundle line and--strict-body's diagnostic all quote it); the host-only branch is additional prose for a case that could not previously arrive here at all.Ablation — the probe pin can fail
One measured-absent global moved into the sandbox set on disk, on the committed tree:
No rebuild leg: the mutated module is CLI source, imported relatively by the pin, so vitest reads it straight from
src— and the red is itself the proof the mutation reached the running code. Restore is trapped (trap … EXIT INT TERM, absolute paths) and proved by bytes, not by the trap firing.Why
free-identifiersand not a new refusal kindforbidden-token's prose fits the sandbox ("the body uses something the sandbox cannot provide") but its severity does not: that class means "writingfetch(IS choosing a bundled closure", andos lintkeeps it awarningso the legitimate path is not punished.Intlis a standard global in every browser and in Node — writing it is not the recognisable "I am reaching for the host" act. It is the ACCIDENTAL class #13651 already defines, so it is anerrora gate can fail on, travelling the path that already exists. No new kind, no change toos build's accept set:lowerCallablesstill catches, still bundles, still exits 0.The one lint edit is the remedy sentence, and it is load-bearing rather than cosmetic: "inline the value(s) into the handler" is impossible for a host global, and an author — or a code-writing model — told to inline
Intllands on a second broken shape. The sentence is chosen from the refusal's ownnodeOnlyIdentifiers, never re-derived in the lint, so the rule cannot disagree with whatos builddid to the same handler. The module-scope sentence is pinned unchanged in the same file.Corpus reading (measured, no edit)
Whether any in-repo hook body would be newly refused: none. Scanned the corpus
checkHookBodyLoweringwalks — all 10objectstack.config.tsroots plus everyhandler:/target:underexamples/andapps/. Two inline function handlers exist (examples/app-crm/src/hooks/opportunity.hook.ts,examples/app-todo/src/objects/task.hook.ts); they useDate,Error,Stringonly — all measured present.git grep -n "\bIntl\b" -- examples appsreturns nothing, and the onlyURLhits inexamples/app-showcase/objectstack.config.tsare in comments.examples/app-showcasealready ships its callables through thefunctions:map by name, which is never lowered.Changeset level
patchon@objectstack/cli. No published accept-set moves — the metadata a valid app may declare is identical,HookBodySchemais untouched, no key is added, removed or re-shaped. What narrows is which handlers the build LOWERS, and for every handler affected the previous outcome was a body that could not run: an affected app gains a warning plus a working bundled closure in place of a productionReferenceError. The deployment shape it "loses" was never one it had in working order, and the measured corpus of affected in-repo sources is zero.Verification
All at
f97d3b85c4(the final commit; the gate union was derived and run on this same tree).pnpm exec turbo run build --concurrency=2 --filter=@objectstack/cli+ the 13 example/plugin filters the i18n gate namespnpm --filter @objectstack/cli run typecheckpnpm --filter @objectstack/cli exec vitest run --maxWorkers=2overdetect-free-identifiers,sandbox-globals-probe,hook-body-lowering,extract-hook-body,lower-callables,hook-body-build-reach.e2e,vitest-tiers-partitionpnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands-> 37 commands, each run with its exit captured before any pipepnpm lint(eslint . --no-inline-config)--ranreconciliation: the 37 derived commands were run 1:1, no additions.pnpm check:nul-byteswas run beyond the derived union (any edit implies it) and is green, plus a direct control-byte scan of the eight changed files (no hits).NOT MEASURED (2), in the gates' own words — neither is a red:
node scripts/check-test-completeness.mjs— exit 3. "Nothing was measured: this gate exited before parsing a single summary line ... ⛔ It is not a red, and there is nothing here to fix. Fix: pass a savedturbo run testlog — or, running the family locally, record this gate as NOT MEASURED."node scripts/pm/check-half-states.mjs— exit 3. "Treat this exit as an unread instrument, never as a quiet board" — a PM-board census over the GitHub API, unrelated to this diff. Its siblingpnpm check:pm-half-states(also in the union) exits 0.Two more first returned a prerequisite verdict rather than a result on a worktree built for the cli graph alone, and are reported green after building the closure they name:
pnpm check:dual-build-cjs-loads("PREREQUISITE NOT MET ... ⛔ This is NOT a pass: nothing was measured") andpnpm check:i18n-coverage("COULD NOT MEASURE — 1 of 13 config(s) failed to lint", a missing@objectstack/connector-mcpdist). The second one matters here beyond bookkeeping: it runsos lintover all 13 in-repo configs, so its green is a direct reading that this change makesos lintreport nothing new on any of them.Deviations from the dispatch
packages/cli/src/utils/extract-hook-body.tsis in the diff. The dispatch's IN list named the detector, its test and the lint rule; the refusal is thrown inextract-hook-body, and "routed through the existing refusal path" cannot be done without composing the reason there. No new refusal kind, no change to whatos buildaccepts, and the pre-existing sentence is pinned byte-identical.detect-free-identifiers.test.ts: it boots a WASM VM and imports@objectstack/runtime, and keeping it separate leaves the pure-AST unit file fast and independent of build state. It classifies asunitunder thetest.projectspartition that landed today (pnpm --filter @objectstack/cli testis a ~24-minute serialized run, and on a shared agent container it holds the verify lock for the whole of it #13504's predicate is SPAWN ∨ KERNEL; this file is neither), andtest/vitest-tiers-partition.test.tspasses on this branch.f97d3b85c4with the build and the two test runs UNDER the lock.check:*gate scripts andpnpm lintare outside the lock's coverage by its own definition, so those ran unlocked by design.🤖 Generated with Claude Code
https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
Generated by Claude Code