diff --git a/.changeset/refuse-create-at-hook-body-lowering.md b/.changeset/refuse-create-at-hook-body-lowering.md new file mode 100644 index 0000000000..576d8e89c3 --- /dev/null +++ b/.changeset/refuse-create-at-hook-body-lowering.md @@ -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. diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 85b96614e8..845e9bfc40 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -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.) @@ -185,7 +186,7 @@ Four literal write shapes are recognized, and only these: |---|---|---| | `ctx.input. = …` / `ctx.input[''] ⟨op⟩= …` (including `+=`, `??=`, …) | checked | not checked — an action's `ctx.input` is its **params bag**, not a record | | `Object.assign(ctx.input, { : … })` | checked | not checked — same surface | -| `ctx.api.object('').insert\|create\|update({ : … })`, `.updateById(id, { : … })` | checked | checked | +| `ctx.api.object('').insert\|update({ : … })`, `.updateById(id, { : … })` | checked | checked | | `ctx.record. = …` / `ctx.record[''] ⟨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: @@ -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. @@ -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()` — 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`: diff --git a/packages/cli/src/utils/extract-hook-body.ts b/packages/cli/src/utils/extract-hook-body.ts index 3318940380..073ea1c7ea 100644 --- a/packages/cli/src/utils/extract-hook-body.ts +++ b/packages/cli/src/utils/extract-hook-body.ts @@ -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): @@ -213,6 +219,49 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [ + 'before-hook (`ctx.input. = ...`), 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: /(? = [ diff --git a/packages/cli/test/extract-hook-body.test.ts b/packages/cli/test/extract-hook-body.test.ts index 30812071c9..989e0558d1 100644 --- a/packages/cli/test/extract-hook-body.test.ts +++ b/packages/cli/test/extract-hook-body.test.ts @@ -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. */ diff --git a/packages/lint/src/validate-action-body-writes.test.ts b/packages/lint/src/validate-action-body-writes.test.ts index 9c3235307e..1014dbaa50 100644 --- a/packages/lint/src/validate-action-body-writes.test.ts +++ b/packages/lint/src/validate-action-body-writes.test.ts @@ -196,11 +196,10 @@ describe('validateActionBodyWrites — ctx.api writes', () => { expect(finding.message).not.toMatch(/write-path validator skips/); }); - it('checks insert/create/update payloads (argument 0) and updateById at argument 1', () => { + it('checks insert/update payloads (argument 0) and updateById at argument 1', () => { const findings = validateActionBodyWrites( stackWith( "await ctx.api.object('crm_contact').insert({ emial: 'a' }); " + - "await ctx.api.object('crm_contact').create({ email: 'b' }); " + "await ctx.api.object('crm_deal').updateById(ctx.recordId, { stag: 'won' });", ), ); @@ -209,6 +208,25 @@ describe('validateActionBodyWrites — ctx.api writes', () => { expect(findings[1].message).toContain('updateById'); }); + // ⭐ [#16249] The ledger is shared, so the withdrawal lands on THIS surface + // too — and for the same reason: an action `body` runs in the same QuickJS + // sandbox, whose `ctx.api.object()` installs no `create` leaf, and + // `objectstack build` refuses `.create(` at lowering for hook and action + // bodies alike. This case replaces a `.create({ email: 'b' })` line that used + // a VALID field inside the case above: it produced no finding before the + // withdrawal and none after, so it could never have measured the change. + it('does not grade a .create() payload on the action surface either (#16249)', () => { + const findings = validateActionBodyWrites( + stackWith("await ctx.api.object('crm_contact').create({ emial: 'b' });"), + ); + expect(findings).toEqual([]); + // Control, same misspelling through the verb the sandbox has. + const control = validateActionBodyWrites( + stackWith("await ctx.api.object('crm_contact').insert({ emial: 'b' });"), + ); + expect(control).toHaveLength(1); + }); + it('accepts declared fields and system columns', () => { const findings = validateActionBodyWrites( stackWith( diff --git a/packages/lint/src/validate-hook-body-writes.test.ts b/packages/lint/src/validate-hook-body-writes.test.ts index eabc442142..a1129381ac 100644 --- a/packages/lint/src/validate-hook-body-writes.test.ts +++ b/packages/lint/src/validate-hook-body-writes.test.ts @@ -233,11 +233,10 @@ describe('validateHookBodyWrites — ctx.input writes', () => { }); describe('validateHookBodyWrites — ctx.api writes', () => { - it('checks insert/create/update payloads (argument 0) against the named object', () => { + it('checks insert/update payloads (argument 0) against the named object', () => { const findings = validateHookBodyWrites( stackWith( "await ctx.api.object('crm_contact').insert({ emial: 'a' }); " + - "await ctx.api.object('crm_contact').create({ email: 'b' }); " + "await ctx.api.object('crm_contact').update({ id, email: 'c' });", ), ); @@ -246,6 +245,35 @@ describe('validateHookBodyWrites — ctx.api writes', () => { expect(findings[0].hint).toContain("'email'"); }); + // ⭐ [#16249] `create` is WITHDRAWN from `API_WRITE_METHODS` and from the + // ledger's advertised syntax. This case used to sit inside the one above as a + // `.create({ email: 'b' })` line with a VALID field — which produced no + // finding before the withdrawal and produces none after it, so it would have + // gone on passing while measuring nothing. Spelled with a MISSPELLED field, + // it is a real pin: a re-added `create` entry makes this line produce a + // second finding and reddens the length assertion. + // + // Grading the payload was the defect, not the silence: it told an author the + // call was fine and the field was the only question, when the VM installs no + // `create` leaf and the call is `TypeError: not a function` on its first run. + // `objectstack build` now refuses `.create(` at lowering, so this shape does + // not reach a real `body.source` at all — the fixture reaches the extractor + // directly, which is exactly what makes it a pin on THIS module. + it('does not grade a .create() payload — the verb is withdrawn from the ledger (#16249)', () => { + const findings = validateHookBodyWrites( + stackWith("await ctx.api.object('crm_contact').create({ emial: 'b' });"), + ); + expect(findings).toEqual([]); + // The control, sharing the failing query's vocabulary: the SAME misspelling + // through the verb the sandbox does have is still graded, so the zero above + // is a reading about `create` and not about the fixture. + const control = validateHookBodyWrites( + stackWith("await ctx.api.object('crm_contact').insert({ emial: 'b' });"), + ); + expect(control).toHaveLength(1); + expect(control[0].message).toContain("ctx.api.object('crm_contact').insert"); + }); + // [#13858] The message is the whole product of an advisory rule, so the // sentence IS the deliverable. It used to promise a driver-dependent outcome // ("on a SQL driver … a driver-level error; on a schemaless driver … the diff --git a/packages/lint/src/validate-hook-body-writes.ts b/packages/lint/src/validate-hook-body-writes.ts index ed2b0a4fcf..f6b39519c0 100644 --- a/packages/lint/src/validate-hook-body-writes.ts +++ b/packages/lint/src/validate-hook-body-writes.ts @@ -226,13 +226,18 @@ export const HOOK_BODY_WRITE_PATTERNS: readonly HookBodyWritePattern[] = [ }, }, { + // [#16249] `.create({…})` was advertised here and mapped in + // `API_WRITE_METHODS`, and it is WITHDRAWN — the syntax line is what an + // author reads as "this is how you write it", and this spelling cannot run: + // see the note on `API_WRITE_METHODS` below for the sandbox reading and the + // build-time refusal that now backs it. id: 'api-crud-literal', syntax: - "ctx.api.object('').insert({…}) | .create({…}) | .update({…}) | .updateById(id, {…})", + "ctx.api.object('').insert({…}) | .update({…}) | .updateById(id, {…})", example: { // Real ObjectRepository signatures: the record payload is argument 0 for - // insert/create/update and argument 1 for updateById. (`update(data)` — - // NOT `update(id, data)`; the id travels inside the payload/options.) + // insert/update and argument 1 for updateById. (`update(data)` — NOT + // `update(id, data)`; the id travels inside the payload/options.) source: "await ctx.api.object('audit_log').insert({ event: 'won' }); " + "await ctx.api.object('crm_deal').updateById(id, { stage: 'won' });", @@ -284,13 +289,38 @@ const HOOK_APPLICABLE_IDS: ReadonlySet = new Set(HOOK_BODY_WRITE_PATTERN /** * `ctx.api.object(name)` write methods → index of the record-payload argument. - * Mirrors `ObjectRepository` in packages/objectql (the surface hooks actually - * receive): `upsert` exists only on the last-resort engine facade actions may - * fall back to, never on the hook path, so it is deliberately absent. + * Mirrors what a BODY's `ctx.api` actually installs — not the host + * `ObjectRepository` class, which is a superset of it. + * + * Two absences, each measured, so the next reader can tell "excluded" from + * "overlooked": + * + * `upsert` exists only on the last-resort engine facade actions may fall + * back to, never on the hook path. + * `create` [#16249] WITHDRAWN. The host `ObjectRepository` does alias + * `create(data)` to `insert(data)`, but this map grades L2 + * (`language:'js'`) BODIES, which run in QuickJS, and `installCtx` + * (runtime/src/sandbox/quickjs-runner.ts) installs exactly + * `insert / update / delete / updateMany / deleteMany / upsert` as + * the `ctx.api.object()` write leaves. The spec contract agrees and + * is the authority: `IScopedObjectRepository` + * (packages/spec/src/contracts/scoped-context.ts) declares `insert` + * and names `create` as measured-and-excluded. So a body's + * `.create()` is `TypeError: not a function` on its first run — + * grading its payload as a live write told an author the call was + * fine and the field was the only question, and staying silent when + * the field existed read as a clean bill of health for a call that + * cannot run. Since #16249 `objectstack build` refuses `.create(` + * at lowering (`FORBIDDEN_PATTERNS` in + * packages/cli/src/utils/extract-hook-body.ts), so the shape no + * longer reaches a `body.source` at all. + * + * ⛔ Re-adding `create` here without re-adding it to the sandbox AND to the + * contract puts the ledger back in front of both — the defect #16249 names. + * `validate-readonly-hook-writes.test.ts` pins the partition and reddens on it. */ const API_WRITE_METHODS: ReadonlyMap = new Map([ ['insert', 0], - ['create', 0], ['update', 0], ['updateById', 1], ]); diff --git a/packages/lint/src/validate-readonly-hook-writes.test.ts b/packages/lint/src/validate-readonly-hook-writes.test.ts index d3c6f9241e..4216554ef6 100644 --- a/packages/lint/src/validate-readonly-hook-writes.test.ts +++ b/packages/lint/src/validate-readonly-hook-writes.test.ts @@ -340,7 +340,9 @@ describe('validateReadonlyHookWrites - GREEN: what a create is NOT judged on', ( // the runner's test pins its ctx.api surface exhaustively). A body calling // `.create()` therefore throws `TypeError: not a function` on its first run — // a loud failure, not the silent no-op this rule reports — so a finding would - // be false in exactly the way the sudo() hint used to be (#14010). + // be false in exactly the way the sudo() hint used to be (#14010). Since + // #16249 the shape does not arrive at all: the ledger no longer advertises it + // and the build refuses `.create(` at lowering. // The engine's create-side strip does not judge a PLATFORM object at all // (`staticReadonlyInsertSubject`: `managedBy` set, or a `sys_` name — its // own 403 write guard governs it); the UPDATE path applies no such @@ -359,7 +361,7 @@ describe('validateReadonlyHookWrites - GREEN: what a create is NOT judged on', ( expect(control[0].rule).toBe(HOOK_API_UPDATE_READONLY_FIELD); }); - it('never flags create() — the sandbox has no such leaf, so the call throws rather than silently dropping', () => { + it('never flags create() — the shape cannot reach this rule at all since #16249', () => { expect( validateReadonlyHookWrites( crmStack("await ctx.api.object('crm_account').create({ last_activity_date: now });"), @@ -367,25 +369,49 @@ describe('validateReadonlyHookWrites - GREEN: what a create is NOT judged on', ( ).toEqual([]); }); - it('records the create() silence as a reasoned method exclusion — never as "INSERT is exempt"', () => { + // ⭐ [#16249] The exclusion's REASON changed and the exclusion did not, so the + // prose is pinned on the NEW fact ("it can no longer get here") and on the + // one it replaced NOT coming back. The old reason — "it throws, so reporting + // a silent drop would be false" — described a shape that reached this rule + // because the ledger advertised it; that route is closed. The sandbox reading + // stays in the text as the WHY BEHIND the refusal, so the entry still explains + // itself to someone who never reads the CLI. + it('records the create() exclusion on its POST-#16249 reason — never as "INSERT is exempt"', () => { expect(READONLY_HOOK_METHOD_EXCLUSIONS.map((e) => e.method)).toEqual(['create']); const [create] = READONLY_HOOK_METHOD_EXCLUSIONS; + // The new primary reason: the build refuses the shape before it can become + // a body this rule parses. + expect(create.reason).toMatch(/can no longer reach this rule/); + expect(create.reason).toMatch(/refuses `\.create\(` at lowering/); + expect(create.reason).toMatch(/never becomes a body\.source/); + // The sandbox fact behind the refusal is kept, not replaced. expect(create.reason).toMatch(/QuickJS/); expect(create.reason).toMatch(/TypeError: not a function/); expect(create.reason).not.toMatch(/INSERT is exempt|engine exempts INSERT/i); }); - it('partitions the extractor\'s ctx.api write verbs exactly — every verb is a subject or a reasoned exclusion', () => { + // ⭐ [#16249] This case is the ledger-side pin: a re-added `create` entry in + // `HOOK_BODY_WRITE_PATTERNS`' advertised syntax reddens HERE, on the first + // assertion, before anything else in the tree notices. Both directions are + // held — the ledger's verbs, and the fact that `create` is no longer one of + // them while its exclusion record survives. + it('partitions the extractor\'s ctx.api write verbs exactly — every ledger verb is a subject (#16249)', () => { // The extractor's `API_WRITE_METHODS` is module-local, so its verbs are // read off the shared ledger's declared `syntax` line for the shape - // ("ctx.api.object('').insert({…}) | .create({…}) | …") instead - // of being restated here — a fifth verb added there fails this case until - // it is classified. + // ("ctx.api.object('').insert({…}) | .update({…}) | …") instead of + // being restated here — a verb added there fails this case until it is + // classified. const apiPattern = HOOK_BODY_WRITE_PATTERNS.find((p) => p.id === 'api-crud-literal')!; const verbs = [...apiPattern.syntax.matchAll(/\.(\w+)\(/g)].map((m) => m[1]).filter((v) => v !== 'object').sort(); - expect(verbs).toEqual(['create', 'insert', 'update', 'updateById']); - const classified = [...READONLY_HOOK_STRIP_SUBJECT_METHODS, ...READONLY_HOOK_METHOD_EXCLUSIONS.map((e) => e.method)].sort(); - expect(classified).toEqual(verbs); + expect(verbs).toEqual(['insert', 'update', 'updateById']); + // The withdrawal itself, stated as its own assertion so the failure names + // it: re-advertising `.create({…})` in the ledger reddens this line. + expect(verbs).not.toContain('create'); + // Every verb the ledger still carries is a subject of this rule. + expect([...READONLY_HOOK_STRIP_SUBJECT_METHODS].sort()).toEqual(verbs); + // And the one method exclusion is a verb the ledger no longer carries — the + // record of the withdrawal, kept on purpose (see the exclusion's comment). + expect(READONLY_HOOK_METHOD_EXCLUSIONS.map((e) => e.method)).toEqual(['create']); expect(READONLY_HOOK_STRIP_SUBJECT_METHODS.filter((m) => READONLY_HOOK_METHOD_EXCLUSIONS.some((e) => e.method === m))).toEqual([]); }); }); diff --git a/packages/lint/src/validate-readonly-hook-writes.ts b/packages/lint/src/validate-readonly-hook-writes.ts index 91d81976ac..b647c4a1b0 100644 --- a/packages/lint/src/validate-readonly-hook-writes.ts +++ b/packages/lint/src/validate-readonly-hook-writes.ts @@ -58,11 +58,18 @@ // reads L2 bodies, which run in QuickJS, and the VM-side `ctx.api.object()` // installs exactly `insert`/`update`/`delete`/`updateMany`/`deleteMany`/ // `upsert` (`installCtx`, runtime/src/sandbox/quickjs-runner.ts) — so a -// body's `.create()` is `TypeError: not a function` at run time: a LOUD +// body's `.create()` was `TypeError: not a function` at run time: a LOUD // failure on the first run, not the silent no-op this rule exists to // report. Gating it as a silent drop would state something false, the // mirror of the `sudo()` hint defect below. // +// ⚠️ [#16249] Since that card the shape does not arrive here at all: the +// extractor ledger no longer advertises `.create({…})` and `objectstack +// build` refuses `.create(` at lowering, so a handler spelling it is +// bundled and never becomes a `body.source`. The exclusion STAYS — it is +// now the record of a withdrawn verb rather than a live declination — and +// its reason is updated to say so. +// // - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context // with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain // is structurally invisible to the extractor (its `api-crud-literal` @@ -236,21 +243,46 @@ const STRIP_SUBJECT_METHODS: ReadonlySet = new Set(READONLY_HOOK_STRIP_S const CONDITIONAL_SUBJECT_METHODS: ReadonlySet = new Set(['update', 'updateById']); /** - * `ctx.api.object()` write methods the shared extractor recognises - * (`API_WRITE_METHODS` in `validate-hook-body-writes.ts`) that this rule - * deliberately does NOT judge, each with its reason — the same discipline - * {@link READONLY_HOOK_WRITE_EXCLUSIONS} applies to pattern shapes. ⛔ A reason - * here may never be "INSERT is exempt": that sentence is false about the - * engine since the 2026-09-03 ruling. + * `ctx.api.object()` write verbs this rule deliberately does NOT judge, each + * with its reason — the same discipline {@link READONLY_HOOK_WRITE_EXCLUSIONS} + * applies to pattern shapes. ⛔ A reason here may never be "INSERT is exempt": + * that sentence is false about the engine since the 2026-09-03 ruling. + * + * ⚠️ [#16249] The one entry is now a verb the shared extractor no longer + * recognises at all, so this list is NOT a subset of `API_WRITE_METHODS` + * (`validate-hook-body-writes.ts`) any more — it is the record of a verb + * withdrawn from it. That is the point: the entry survives its own cause, and + * deleting it would leave the next author free to re-add `create` to the ledger + * with nothing on this rule's side saying why it was taken out. */ export const READONLY_HOOK_METHOD_EXCLUSIONS: readonly { method: string; reason: string }[] = [ { method: 'create', + // ⚠️ [#16249] The REASON changed, the exclusion did not. Until #16249 the + // reason was "a body's .create() throws, and reporting a silent drop about + // a call that throws would be false" — correct then, because the shape + // reached this rule: the extractor ledger advertised `.create({…})` as + // legal `api-crud-literal` syntax and mapped it in `API_WRITE_METHODS`, so + // this rule had to decline a subject it could actually see. + // + // #16249 closed that at the source: `create` is withdrawn from the ledger, + // and `objectstack build` refuses `.create(` at lowering + // (`FORBIDDEN_PATTERNS`, packages/cli/src/utils/extract-hook-body.ts), so a + // handler spelling it is bundled and never becomes a `body.source`. This + // rule opens on `body.language === 'js'` and parses `body.source`, so the + // shape cannot arrive here at all. "It throws, so do not report it" has + // become "it can no longer get here" — a stronger fact, and a different + // one. ⛔ Do not restore the old sentence: it would describe a route the + // build has closed. reason: - 'this rule reads L2 bodies, which run in QuickJS, and the VM-side ctx.api.object() installs no ' + - '`create` leaf (installCtx in runtime/src/sandbox/quickjs-runner.ts: insert / update / delete / ' + - 'updateMany / deleteMany / upsert) - so a body calling .create() is `TypeError: not a function` on ' + - 'its first run, a LOUD failure, not the silent no-op this rule reports. The host ObjectRepository ' + + 'the shape can no longer reach this rule: `objectstack build` refuses `.create(` at ' + + 'lowering, so a handler spelling it is bundled and never becomes a body.source this rule can parse, ' + + 'and the shared extractor no longer recognises the verb either. What that refusal encodes: this rule ' + + 'reads L2 bodies, which run in QuickJS, and the VM-side ctx.api.object() installs no `create` leaf ' + + '(installCtx in runtime/src/sandbox/quickjs-runner.ts: insert / update / delete / updateMany / ' + + 'deleteMany / upsert), while the spec contract IScopedObjectRepository declares insert and names ' + + 'create as measured-and-excluded - so a body calling .create() was `TypeError: not a function` on ' + + 'its first run, a LOUD failure, never the silent no-op this rule reports. The host ObjectRepository ' + 'does alias create() to insert(), but no body reaches the host repository. A fact about the ' + 'SANDBOX, not about INSERT: the same payload spelled .insert() IS judged', },