Skip to content

Commit bb4f181

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-15769-stale-contract-prose-trio
2 parents a0a8878 + 70f7d6d commit bb4f181

16 files changed

Lines changed: 664 additions & 39 deletions
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`ObjectTitleCompleteness.status` now documents which INPUT each grade describes. No predicate, no grade computation and no accept set moves — the classification is unchanged and correct; what changes is what the declaration tells the reader it means.
6+
7+
`explicit` was documented as "an explicit pointer (`nameField`/`displayNameField`) is set". That is accurate about the predicate, and that is what made it dangerous: on a body served by a `/meta` read exit a pointer is present whether or not the author wrote one, because the exit replays the registry's object-materialization seam (`materializeServedObjectOnto`, which runs `provisionPrimary` in designate-only mode). So `explicit` there means "a pointer is present", never "the author designated this" — and the old wording invited the second reading at every call site.
8+
9+
The corrected declaration says three things the old one left to inference:
10+
11+
- **Provenance decides the grade.** On a served body `explicit` carries no authorship information, and `derived` is unreachable except where that replay withheld the designation. On a body captured before the write seam — an authored definition as written, which is what `os build` / `os lint` hand this predicate through `@objectstack/lint`'s `validateRecordTitle``explicit` really does mean the author wrote a pointer.
12+
- **Authorship is not recoverable afterwards, deliberately.** The write-side inverse `stripProvisionedPrimaryFrom` removes the pointer exactly when it is byte-identical to what the derivation would produce, and says so in its own words: the two are indistinguishable by construction, the same bytes. Neither the served document nor the stored row can answer "did the author designate this?".
13+
- **The consequence for callers.** Never build a check on `explicit` that needs the authored answer without first proving the input is a pre-write body.
14+
15+
`objectTitleCompleteness` itself carries the one-sentence form of the same warning, because a caller hovering the function is on a different path from a caller hovering the grade.
16+
17+
The correct words already existed in this repo — on the WRITE seam, where nobody reading the grade goes. This moves their substance to the grade.
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.
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
fix(spec): `isValueDomainMember` refuses an off-vocabulary domain instead of failing OPEN on `Object.prototype` names
6+
7+
`DOMAIN_MEMBERSHIP` is an object literal, so it inherits `Object.prototype`, and
8+
`isValueDomainMember` indexed it with no own-property guard. Measured against the
9+
built artifact (`dist/shared/index.mjs`) on the repo's Node 22 baseline (v22.22.2),
10+
an off-vocabulary `domain` did one of two wrong things — and one of them was a
11+
membership FALSE POSITIVE out of a predicate whose whole job is to refuse
12+
non-members:
13+
14+
| `domain` | before | after |
15+
|:--|:--|:--|
16+
| `iana_time_zone` (in vocabulary) | `true` for `UTC` | `true` for `UTC` — unmoved |
17+
| `toString` | `'[object Object]'` — a truthy **string** | `false` |
18+
| `valueOf` | a truthy **object** | `false` |
19+
| `constructor` | a truthy **object** | `false` |
20+
| `__proto__` | threw a `TypeError` | `false` |
21+
| `nope`, `''` | threw a `TypeError` | `false` |
22+
23+
**Why it is reachable.** "Unreachable in-repo" is not "unreachable". The parameter
24+
is typed `ValueDomain` and every in-repo call site names a member, but
25+
`isValueDomainMember` is **published** on `@objectstack/spec/shared` (it is in
26+
`packages/spec/api-surface/shared.json`). A plain-JS consumer, or any caller
27+
handing over a domain string read from **metadata** rather than written in source,
28+
reaches it with no type checking at all — and metadata-sourced strings are exactly
29+
where `constructor` and `toString` show up.
30+
31+
**This narrows and widens nothing, measured rather than asserted.** Every accepted
32+
`domain` is an own key of the record, so no value that was accepted before is
33+
refused now; the three real domains answer from their own definitions, unmoved.
34+
The change is one `Object.prototype.hasOwnProperty.call` guard — the same spelling
35+
the `iso_4217_currency` definition in the same module already uses — returning
36+
`false` for a domain that is not an own key. A **null-prototype record** was the
37+
other shape available and was not taken: it converts the truthy answers into
38+
throws rather than into `false`, and it costs the `Readonly<Record<ValueDomain, …>>`
39+
annotation that makes a vocabulary member added without a definition fail to
40+
compile.
41+
42+
**Unknown domain answers `false`; it does not throw.** `false` is the narrowing
43+
reading — it refuses more and accepts nothing new — whereas a thrown refusal would
44+
change published behaviour for callers who today receive a truthy value. This is
45+
the same third branch a sister ruling settled for the same defect family: list
46+
reject / own-member value / prototype-resolvable ⇒ reject.
47+
48+
The pin that existed did not cover this, and the fix is as much about its
49+
POPULATION as about the guard: the totality pin asserted the return `typeof` was
50+
`boolean` but iterated `ValueDomainSchema.options` **only** — exactly the domains
51+
that behave. The new pins put `toString`, `valueOf`, `constructor`,
52+
`hasOwnProperty`, `isPrototypeOf`, `propertyIsEnumerable`, `__proto__` and plainly
53+
absent words into the population, and a third pin holds that population honest by
54+
asserting every one of them is still outside the vocabulary.

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' }> = [

0 commit comments

Comments
 (0)