Skip to content

Commit ed6579b

Browse files
fix(cli,lint): refuse a hook/action body calling .create() at lowering, and withdraw the verb from the write-pattern ledger (#16900)
* wip(cli,lint): refuse .create( at lowering; withdraw create from the write ledger Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * chore(changeset): patch changeset for the .create( refusal and ledger withdrawal Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * fix(lint,cli): keep the tracker id out of the runtime reason string; pin the refusal text Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent a8be5f7 commit ed6579b

9 files changed

Lines changed: 315 additions & 36 deletions
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/cli": patch
3+
"@objectstack/lint": patch
4+
---
5+
6+
`objectstack build` now refuses to lower a hook/action body that calls `.create(`, and the shared write-pattern ledger stops advertising the verb. Three layers used to disagree about `ctx.api.object('x').create({ … })`, and the loudest one was wrong.
7+
8+
- The spec contract `IScopedObjectRepository` (`packages/spec/src/contracts/scoped-context.ts`) declares `insert` and names `create` as measured-and-deliberately-excluded.
9+
- The QuickJS sandbox installs exactly `insert / update / delete / updateMany / deleteMany / upsert` as the `ctx.api.object()` write leaves — no `create`. An L2 body calling `.create()` therefore threw `TypeError: not a function` on its **first run**, and under a hook's default `onError: 'abort'` that throw aborted the triggering write, with a message naming no member.
10+
- The extractor ledger nonetheless advertised `.create({…})` as legal `api-crud-literal` syntax and mapped it in `API_WRITE_METHODS`, so `hook-body-write-unknown-field` graded the payload as a live write and stayed silent when the field existed — a clean bill of health for a call that cannot run. Build time said nothing at all.
11+
12+
What changes:
13+
14+
- **`@objectstack/cli`**`.create(` joins `FORBIDDEN_PATTERNS` in the hook/action body extractor, beside `.sudo(` and for the same reason (a member real on the in-process `ScopedContext` / `ObjectRepository` and absent from the VM). The refusal names `.insert({ ... })` as the spelling the sandbox actually has. Behaviour is the `forbidden-token` fallback every other entry has: the callable is still registered and still shipped through the back-compat `.mjs` bundle, so a handler keeps running in-process where the host `create()` alias exists — `objectstack build` merely declines to *also* emit it as a body that cannot run. Under `--strict-body` it is a hard failure, correctly. The rule is receiver-loose like `.sudo(` (`const repo = ctx.api.object('x'); repo.create(…)` is refused too) with one carve-out: `Object.create()` is a real sandbox global and is **not** affected.
15+
- **`@objectstack/lint`**`create` is withdrawn from `HOOK_BODY_WRITE_PATTERNS`' advertised `api-crud-literal` syntax and from `API_WRITE_METHODS`, on the hook and action surfaces alike. `hook-body-write-unknown-field` / `action-body-write-unknown-field` no longer grade a `.create()` payload; `hook-api-update-readonly-field` keeps its existing `create` exclusion, whose *reason* is updated — it is no longer "the call throws, so a silently-dropped finding would be false" but "the shape can no longer reach this rule at all".
16+
17+
**Migration.** If a hook or action body calls `ctx.api.object('x').create({ … })`, spell it `ctx.api.object('x').insert({ … })` — the same host method, the one the sandbox installs and the only insert verb the contract declares. The host-side `ObjectRepository.create()` alias is untouched and stays reachable from in-process handlers and actions.

content/docs/automation/hook-bodies.mdx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ The CLI builder **rejects** any source that uses:
166166
- `process`, `globalThis`
167167
- `eval`, `new Function`
168168
- references to identifiers from value-only top-level imports
169+
- `.sudo(` and `.create(` — members that are real on the **in-process** `ScopedContext` / `ObjectRepository` and absent from the VM's `ctx.api`, so lowering them would ship a body that `TypeError`s on its first run. Write `.insert({ ... })` instead of `.create({ ... })` — it is the only insert verb the `IScopedObjectRepository` contract declares — and reach for [`runAs: 'system'`](/docs/automation/hooks#elevation--runas) instead of `.sudo()`. `Object.create()` is a real sandbox global and is **not** affected.
169170

170171
Need outbound HTTP? Define a **Connector recipe** as metadata and call it via `ctx.connector(...)`. (Connector spec is tracked separately and ships after L1+L2 stabilises.)
171172

@@ -185,7 +186,7 @@ Four literal write shapes are recognized, and only these:
185186
|---|---|---|
186187
| `ctx.input.<field> = …` / `ctx.input['<field>'] ⟨op⟩= …` (including `+=`, `??=`, …) | checked | not checked — an action's `ctx.input` is its **params bag**, not a record |
187188
| `Object.assign(ctx.input, { <field>: … })` | checked | not checked — same surface |
188-
| `ctx.api.object('<literal>').insert\|create\|update({ <field>: … })`, `.updateById(id, { <field>: … })` | checked | checked |
189+
| `ctx.api.object('<literal>').insert\|update({ <field>: … })`, `.updateById(id, { <field>: … })` | checked | checked |
189190
| `ctx.record.<field> = …` / `ctx.record['<field>'] ⟨op⟩= …` | n/a — a hook context has no `ctx.record` (the expression throws) | checked: warns as **discarded**, declared field or not |
190191

191192
**A missing warning is not a clean bill of health.** The rule bails *silently* on everything it cannot resolve statically, deliberately preferring a missed finding to a false one — a false positive kills an advisory lint, while a miss just leaves the gap open a little longer:
@@ -270,7 +271,7 @@ The dropped case is the dangerous one: nothing fails, the step reports success,
270271

271272
Which hooks these rules can *see* depends on the command, because every rule in this family opens on `body.language === 'js'`. A hook authored as an inline `handler` function carries no `body`, so it is judged only where the command has first lowered the handler to a metadata body: `objectstack build` always has (it lowers before it parses — see [How the build lowers a handler](#build-pipeline)), and since [#16095](https://github.com/objectstack-ai/objectstack/issues/16095) `objectstack lint` judges that same lowered view, so an author who runs only the pre-flight is told the same thing the build would refuse. Since [#16544](https://github.com/objectstack-ai/objectstack/issues/16544) `objectstack validate` lowers before it parses as well, so all three commands judge the same view of a handler-authored hook — a stack `objectstack validate` passes is one `objectstack build` does not refuse on this family. A handler the build cannot lower (a forbidden token, a module-scope identifier) has no body on any command and is reported by the lowering rules instead, never guessed at here.
272273

273-
Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `.create()` is skipped too, for a reason about the **sandbox** rather than the engine: the VM-side `ctx.api.object()` installs `insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, so a body calling `.create()` throws `TypeError: not a function` on its first run — a loud failure, not a silent drop — and the same payload spelled `.insert()` is what the rule judges. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), and since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) it reports a non-`runAs: 'system'` `create_record` node's static-`readonly` write at the same **error**, again with no conditional finding on a create.
274+
Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `.create()` is skipped too, and since [#16249](https://github.com/objectstack-ai/objectstack/issues/16249) it cannot even arrive: `objectstack build` refuses `.create(` at lowering, so a handler spelling it is bundled and never becomes a `body` these rules parse, and the write-shape ledger no longer advertises the verb. The reason behind that refusal is about the **sandbox** rather than the engine: the VM-side `ctx.api.object()` installs `insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, and the `IScopedObjectRepository` contract declares `insert` only — so a body calling `.create()` threw `TypeError: not a function` on its first run, aborting the triggering write under the default `onError: 'abort'`. A loud failure, never a silent drop; the same payload spelled `.insert()` is what the rules judge. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), and since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) it reports a non-`runAs: 'system'` `create_record` node's static-`readonly` write at the same **error**, again with no conditional finding on a create.
274275

275276
The table above is about a **hook** body. An **action** body is the one surface where the answer changes, so read this before you move a body from one to the other: an action body runs **elevated** — its `ctx.api` is built over the caller's envelope with `isSystem` set, which is the same trusted posture that lets an action bypass row and field permissions — and the static strip applies only to non-system callers. So `ctx.api.object('x').update({ someReadonlyField })` **lands** in an action, and there is no finding for it. Elevation does not waive the *conditional* lock, though, so that half does carry across: `action-api-update-readonly-when-field` — a **warning** — on an action body's literal `ctx.api` update to a `readonlyWhen` field ([#13770](https://github.com/objectstack-ai/objectstack/issues/13770)). Net effect when you move a body: a `readonly` write changes behaviour, a `readonlyWhen` write does not.
276277

@@ -404,6 +405,8 @@ The extractor scans each body for known patterns and adds the matching capabilit
404405
| `ctx.log.info / warn / error / debug` | `log` |
405406
| `*.title(<argument>)` — the related-record form only; bare `ctx.title()` performs no read | `api.read` |
406407

408+
The matcher is deliberately over-inclusive — it names spellings the VM does not install (`patch`, `remove`, `get`, `list`, `create`) because an over-inferred token costs nothing the sandbox ever checks, while an under-inferred one surfaces as a sandbox refusal far from its cause. `create` is listed only for that reason: a body spelling it is refused at lowering (see [What the sandbox forbids](#what-the-sandbox-forbids)) and never reaches inference at all.
409+
407410
When inference does not derive what a body needs, declare the tokens yourself by
408411
supplying `body` on the hook or action instead of a `handler`:
409412

packages/cli/src/utils/extract-hook-body.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,14 @@
1414
* For v1 we apply a deliberately simple **regex allow-list** over the
1515
* extracted body — full TypeScript AST analysis is deferred to v2. Anything
1616
* the regex rejects (top-level `import`, `require(` / esbuild's `__require(`,
17-
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
18-
* extraction **throw**.
17+
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`,
18+
* `.create(`) makes extraction **throw**.
19+
*
20+
* The last two are one family: a member that is REAL on the host
21+
* `ScopedContext`/`ObjectRepository` and absent from the VM's `ctx.api`, so the
22+
* same handler source passes an in-process test and TypeErrors the moment the
23+
* build lowers it into a body. `.create(` carries one wrinkle `.sudo(` does not
24+
* — see its entry in `FORBIDDEN_PATTERNS`.
1925
*
2026
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
2127
* are not the same one. This header used to claim only the second (#10678):
@@ -213,6 +219,49 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
213219
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
214220
+ 'where `sudo()` exists',
215221
},
222+
// [#16249] Same family as `.sudo(` above, one layer over: the host
223+
// `ObjectRepository` aliases `create(data)` to `insert(data)`, the spec
224+
// contract `IScopedObjectRepository` declares `insert` and NOT `create`
225+
// (packages/spec/src/contracts/scoped-context.ts — `create` is listed there
226+
// as measured and deliberately excluded), and the VM installs exactly
227+
// `insert / update / delete / updateMany / deleteMany / upsert` as the
228+
// `ctx.api.object()` write leaves (`installCtx`,
229+
// runtime/src/sandbox/quickjs-runner.ts). So a lowered body's `.create()` is
230+
// `TypeError: not a function` on its FIRST run, and under a hook's default
231+
// `onError: 'abort'` that aborts the triggering write with a message naming
232+
// no member — the blind message #14010 measured for `sudo()`.
233+
//
234+
// What made this worse than an omission: the extractor ledger
235+
// (`HOOK_BODY_WRITE_PATTERNS`, packages/lint) ADVERTISED `.create({…})` as
236+
// legal `api-crud-literal` syntax and graded its payload as a live write, so
237+
// the one layer that actively told an author how to write it named a spelling
238+
// that cannot run. That entry is withdrawn in the same change; refusing here
239+
// is what makes build time say what the contract already said.
240+
//
241+
// ⛔ The alternative — installing a `create` leaf in `installCtx` — is
242+
// rejected on purpose: it would have the SANDBOX ratify a verb the CONTRACT
243+
// never declared, which is the wrong direction under contract-first.
244+
//
245+
// Receiver-loose like `.sudo(` (a local alias `const repo =
246+
// ctx.api.object('x'); repo.create(…)` must not slip through), with ONE
247+
// carve-out that `.sudo(` needs no equivalent of: `Object` is a real sandbox
248+
// global (pinned in `SANDBOX_GLOBALS`), so `Object.create(null)` is working,
249+
// lowerable code. Refusing it would turn a correct body into a bundled
250+
// closure — and a hard failure under `--strict-body` — which is a false
251+
// refusal, not the safe direction. The lookbehind excludes that ONE receiver
252+
// and nothing else: `myObject.create(` still matches, because `\b` requires a
253+
// word boundary before `Object`.
254+
{
255+
rx: /(?<!\bObject\s*)\.\s*create\s*\(/,
256+
reason:
257+
'`create()` is not reachable from a sandboxed body — the VM\'s `ctx.api.object()` installs '
258+
+ '`insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, so '
259+
+ 'the call is a TypeError at run time (and under a hook\'s default `onError: \'abort\'` that aborts '
260+
+ 'the triggering write). Spell the same payload `.insert({ ... })`, which is the member the sandbox '
261+
+ 'actually has and the only insert verb the spec contract declares; `Object.create()` is unaffected. '
262+
+ 'Alternatively leave this handler bundled so it runs in-process, where the host repository\'s '
263+
+ '`create()` alias exists',
264+
},
216265
];
217266

218267
const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [

packages/cli/test/extract-hook-body.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,82 @@ describe('extractHookBody', () => {
333333
const ext = extractHookBody(fn, 'hook contained');
334334
expect(ext.source).toContain('Math.round');
335335
});
336+
337+
// ── `create()` is not a body-reachable member (#16249) ───────────────────
338+
//
339+
// Same family as `sudo()` above, one layer over. The host `ObjectRepository`
340+
// aliases `create(data)` to `insert(data)` and the spec contract
341+
// `IScopedObjectRepository` declares `insert` only, while the VM installs
342+
// `insert / update / delete / updateMany / deleteMany / upsert` and no
343+
// `create` leaf — so a lowered body's `.create()` TypeErrors on its first
344+
// run and, under a hook's default `onError: 'abort'`, aborts the triggering
345+
// write. What made it worse than an omission: the extractor ledger in
346+
// `@objectstack/lint` ADVERTISED `.create({…})` as legal syntax, so the one
347+
// layer that actively told an author how to write it named a spelling that
348+
// cannot run. That entry is withdrawn in the same change; these cases pin the
349+
// build-time half.
350+
it('rejects a handler calling ctx.api.object(x).create() (#16249)', () => {
351+
const fn = async (ctx: any) => {
352+
await ctx.api.object('crm_account').create({ name: ctx.input.name });
353+
};
354+
expect(() => extractHookBody(fn, 'hook seed')).toThrow(/`create\(\)` is not reachable/);
355+
});
356+
357+
// The reason has to name the spelling the sandbox HAS — a refusal that only
358+
// says "no" leaves the author where the blind `TypeError` left them.
359+
it('names `.insert()` as the remedy, and the leaves the VM installs (#16249)', () => {
360+
const fn = async (ctx: any) => {
361+
await ctx.api.object('crm_account').create({ name: ctx.input.name });
362+
};
363+
let message = '';
364+
try {
365+
extractHookBody(fn, 'hook seed');
366+
} catch (err) {
367+
message = (err as Error).message;
368+
}
369+
expect(message).toMatch(/`\.insert\(\{ \.\.\. \}\)`/);
370+
expect(message).toMatch(/`insert` \/ `update` \/ `delete` \/ `updateMany` \/ `deleteMany` \/ `upsert`/);
371+
expect(message).toMatch(/no `create` leaf/);
372+
// The carve-out is stated in the refusal itself, so an author who hits it
373+
// over an `Object.create()` false positive is told it is not the subject.
374+
expect(message).toMatch(/`Object\.create\(\)` is unaffected/);
375+
});
376+
377+
it('rejects the aliased receiver too — `const repo = ctx.api.object(x); repo.create()` (#16249)', () => {
378+
// Receiver-loose, decided by `.sudo(` and not re-decided here: under-refusing
379+
// is the failure only production sees.
380+
const fn = async (ctx: any) => {
381+
const repo = ctx.api.object('crm_account');
382+
await repo.create({ name: ctx.input.name });
383+
};
384+
expect(() => extractHookBody(fn, 'hook seed alias')).toThrow(/`create\(\)` is not reachable/);
385+
});
386+
387+
// ⭐ The carve-out, and the ONE thing `.sudo(` needed no equivalent of:
388+
// `Object` is a real sandbox global (pinned in `SANDBOX_GLOBALS`), so
389+
// `Object.create(null)` is working, lowerable code. A bare receiver-loose
390+
// rule would refuse it — turning a correct body into a bundled closure, and a
391+
// hard failure under `--strict-body`. That is a false refusal, not the safe
392+
// direction, so the lookbehind excludes that ONE receiver.
393+
it('does NOT refuse `Object.create(null)` — a real sandbox global (#16249)', () => {
394+
const fn = (ctx: any) => {
395+
const seen = Object.create(null);
396+
seen[ctx.input.email] = true;
397+
ctx.input.dedupe_key = Object.keys(seen).join(',');
398+
};
399+
const ext = extractHookBody(fn, 'hook dedupe');
400+
expect(ext.source).toContain('Object.create(null)');
401+
});
402+
403+
// The reverse leg, as `.sudo(` has: the majority case must still lower.
404+
it('still extracts an ordinary ctx.api insert (#16249)', () => {
405+
const fn = async (ctx: any) => {
406+
await ctx.api.object('audit_log').insert({ event: ctx.input.event });
407+
};
408+
const ext = extractHookBody(fn, 'hook audit');
409+
expect(ext.capabilities).toContain('api.write');
410+
expect(ext.source).toMatch(/object\((['"])audit_log\1\)\.insert/);
411+
});
336412
});
337413

338414
/** Module-scope helper used by the #1876 free-identifier test above. */

0 commit comments

Comments
 (0)