Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/refuse-create-at-hook-body-lowering.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

`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.

- The spec contract `IScopedObjectRepository` (`packages/spec/src/contracts/scoped-context.ts`) declares `insert` and names `create` as measured-and-deliberately-excluded.
- 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.
- 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.

What changes:

- **`@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.
- **`@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".

**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.
7 changes: 5 additions & 2 deletions content/docs/automation/hook-bodies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ The CLI builder **rejects** any source that uses:
- `process`, `globalThis`
- `eval`, `new Function`
- references to identifiers from value-only top-level imports
- `.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.

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.)

Expand All @@ -185,7 +186,7 @@ Four literal write shapes are recognized, and only these:
|---|---|---|
| `ctx.input.<field> = …` / `ctx.input['<field>'] ⟨op⟩= …` (including `+=`, `??=`, …) | checked | not checked — an action's `ctx.input` is its **params bag**, not a record |
| `Object.assign(ctx.input, { <field>: … })` | checked | not checked — same surface |
| `ctx.api.object('<literal>').insert\|create\|update({ <field>: … })`, `.updateById(id, { <field>: … })` | checked | checked |
| `ctx.api.object('<literal>').insert\|update({ <field>: … })`, `.updateById(id, { <field>: … })` | checked | checked |
| `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 |

**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:
Expand Down Expand Up @@ -270,7 +271,7 @@ The dropped case is the dangerous one: nothing fails, the step reports success,

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.

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.
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.

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.

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

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.

When inference does not derive what a body needs, declare the tokens yourself by
supplying `body` on the hook or action instead of a `handler`:

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

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
76 changes: 76 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,82 @@ describe('extractHookBody', () => {
const ext = extractHookBody(fn, 'hook contained');
expect(ext.source).toContain('Math.round');
});

// ── `create()` is not a body-reachable member (#16249) ───────────────────
//
// Same family as `sudo()` above, one layer over. The host `ObjectRepository`
// aliases `create(data)` to `insert(data)` and the spec contract
// `IScopedObjectRepository` declares `insert` only, while the VM installs
// `insert / update / delete / updateMany / deleteMany / upsert` and no
// `create` leaf — so a lowered body's `.create()` TypeErrors on its first
// run and, under a hook's default `onError: 'abort'`, aborts the triggering
// write. What made it worse than an omission: the extractor ledger in
// `@objectstack/lint` ADVERTISED `.create({…})` as legal syntax, so the one
// layer that actively told an author how to write it named a spelling that
// cannot run. That entry is withdrawn in the same change; these cases pin the
// build-time half.
it('rejects a handler calling ctx.api.object(x).create() (#16249)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').create({ name: ctx.input.name });
};
expect(() => extractHookBody(fn, 'hook seed')).toThrow(/`create\(\)` is not reachable/);
});

// The reason has to name the spelling the sandbox HAS — a refusal that only
// says "no" leaves the author where the blind `TypeError` left them.
it('names `.insert()` as the remedy, and the leaves the VM installs (#16249)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').create({ name: ctx.input.name });
};
let message = '';
try {
extractHookBody(fn, 'hook seed');
} catch (err) {
message = (err as Error).message;
}
expect(message).toMatch(/`\.insert\(\{ \.\.\. \}\)`/);
expect(message).toMatch(/`insert` \/ `update` \/ `delete` \/ `updateMany` \/ `deleteMany` \/ `upsert`/);
expect(message).toMatch(/no `create` leaf/);
// The carve-out is stated in the refusal itself, so an author who hits it
// over an `Object.create()` false positive is told it is not the subject.
expect(message).toMatch(/`Object\.create\(\)` is unaffected/);
});

it('rejects the aliased receiver too — `const repo = ctx.api.object(x); repo.create()` (#16249)', () => {
// Receiver-loose, decided by `.sudo(` and not re-decided here: under-refusing
// is the failure only production sees.
const fn = async (ctx: any) => {
const repo = ctx.api.object('crm_account');
await repo.create({ name: ctx.input.name });
};
expect(() => extractHookBody(fn, 'hook seed alias')).toThrow(/`create\(\)` is not reachable/);
});

// ⭐ The carve-out, and the ONE thing `.sudo(` needed no equivalent of:
// `Object` is a real sandbox global (pinned in `SANDBOX_GLOBALS`), so
// `Object.create(null)` is working, lowerable code. A bare receiver-loose
// rule would refuse it — turning a correct body into a bundled closure, and a
// hard failure under `--strict-body`. That is a false refusal, not the safe
// direction, so the lookbehind excludes that ONE receiver.
it('does NOT refuse `Object.create(null)` — a real sandbox global (#16249)', () => {
const fn = (ctx: any) => {
const seen = Object.create(null);
seen[ctx.input.email] = true;
ctx.input.dedupe_key = Object.keys(seen).join(',');
};
const ext = extractHookBody(fn, 'hook dedupe');
expect(ext.source).toContain('Object.create(null)');
});

// The reverse leg, as `.sudo(` has: the majority case must still lower.
it('still extracts an ordinary ctx.api insert (#16249)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('audit_log').insert({ event: ctx.input.event });
};
const ext = extractHookBody(fn, 'hook audit');
expect(ext.capabilities).toContain('api.write');
expect(ext.source).toMatch(/object\((['"])audit_log\1\)\.insert/);
});
});

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