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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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
47 changes: 47 additions & 0 deletions .changeset/hook-body-sudo-is-not-reachable.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/cli": patch
"@objectstack/lint": patch
---

fix(cli,lint): stop lowering hook handlers that call `ctx.api.sudo()` into bodies that cannot run it (#14010)

`ScopedContext.sudo()` is real in-process and is **not** marshalled into the
QuickJS sandbox: the VM's `ctx.api` carries `object()` and the transaction
surface, and nothing else. Every consumer of that fact had it backwards.

The failure this closes is the expensive shape, not a cosmetic one. An author
writes an inline `handler`, tests it the way the docs teach — calling
`hook.handler(ctx)` natively, against the in-process `ScopedContext`, where
`sudo()` exists — and the suite is green. `objectstack build` then lowers that
same source into an L2 `body`, and in production the call is
`TypeError: ctx.api.sudo is not a function`. Under a hook's default
`onError: 'abort'` the TypeError aborts the **triggering write**, so the
symptom surfaces as an unrelated save being refused. Green tests, dead feature.

- **`@objectstack/cli`** — `.sudo(` joins `FORBIDDEN_PATTERNS` in
`extractHookBody`, so the build declines to emit such a handler as
`body.source`. This is a repair, not just a refusal: `lowerCallables` already
registers the callable and ships it through the `.mjs` bundle when extraction
throws, so the handler keeps running **in-process, where `sudo()` is real**.
The build prints the reason; `--strict-body`, which demands a body for every
callable, turns it into a hard failure — correctly, since a body needing
elevation genuinely cannot be one. Same family as the `crypto.hash`
retirement (#4391): a member advertised ahead of its implementation, where
build-time inference was the amplifier rather than the safety net.
- **`@objectstack/lint`** — `hook-api-update-readonly-field` (severity
`error`, gating) and its `readonlyWhen` sibling both *prescribed*
`ctx.api.sudo()` as the remedy. That rule reads L2 body sources and nothing
else, so the prescribed shape was a TypeError for **100%** of its population:
a gating rule pointing at a dead feature. Both hints now name the own-hook
stamp and say plainly that `sudo()` is not reachable from a body. The rule's
findings, severities and exclusions are unchanged — only the advice.

Docs: the `readonly` table in `automation/hook-bodies.mdx` claimed the
`sudo()` row **Lands**; it now records what actually happens.

Not addressed here, and the reason this is only half the card: a hook still has
**no declared elevation knob** — there is no hook-side `runAs` the way
`FlowSchema` has one — so "this column is computed by automation and never
hand-written" remains inexpressible whenever the maintaining write is
cross-object. That is a contract-surface decision (see #14010), left to the
review chain rather than guessed at here.
6 changes: 3 additions & 3 deletions content/docs/automation/hook-bodies.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,21 +247,21 @@ A body may write *other* objects — e.g. `await ctx.api.object('parent').update

### Writing a `readonly` field

There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** — but only through two channels, and a nested `ctx.api` write is **not** one of them.
There is an asymmetry here that costs data if you learn it the hard way, so learn it here. A field declared `readonly: true` can still be **maintained by automation** from a body — but through exactly **one** channel (the own-hook stamp, plus INSERT), and a nested `ctx.api` write is **not** one of them.

`readonly` governs the *caller* surface. On UPDATE the engine strips read-only keys from the payload, but only the ones the **caller supplied** and only when the value is still the caller's. So:

| How the body writes it | What happens |
|:---|:---|
| `ctx.input.<field> = …` in `beforeInsert`/`beforeUpdate` | **Lands.** The stamp is a *server* value, not a caller-supplied one, so the strip leaves it alone. This is the recommended shape. |
| `ctx.api.object('x').update({ <field> })` | **Silently dropped.** `ctx.api` is scoped to the *triggering* operation's context, so on any non-system trigger the payload is an ordinary caller payload and the key is stripped. The call still returns success. |
| `ctx.api.sudo().object('x').update({ <field> })` | **Lands.** `sudo()` elevates to a system context, which the strip skips — the hook-side analogue of a flow's `runAs: 'system'`. Use it deliberately: it also bypasses the acting user's row and field permissions for that write. |
| `ctx.api.sudo().object('x').update({ <field> })` | **`TypeError` — not available here.** `sudo()` is a member of the *in-process* `ScopedContext`; the VM's `ctx.api` carries `object()` and `transaction()` and nothing else, so a **body** cannot reach it. Worse than unavailable: the same source *works* when the handler runs in-process, so it passes a native `hook.handler(ctx)` test and throws only once the build lowers it into a body — aborting the triggering write under the default `onError: 'abort'`. `objectstack build` now refuses to lower such a handler and keeps it bundled instead. A hook has **no** declared elevation knob (no hook-side `runAs`); [#14010](https://github.com/objectstack-ai/objectstack/issues/14010) is where that gap is argued. |
| `ctx.api.object('x').insert({ <field> })` | **Lands.** INSERT is exempt — a create may legitimately seed read-only columns. |

The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:

- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — `sudo()` is.
- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. Note that `readonlyWhen` also strips a `beforeUpdate`-derived value, so the own-hook stamp is **not** a workaround for it — and neither is `sudo()`, which a body cannot reach (see the row above). On this shape, confirm the write only targets records whose predicate is `false`, or drop the field from the payload.

Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name, an object this stack does not declare, and `insert`/`create` are all skipped, so the rule has no opinion on them. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425).

Expand Down
31 changes: 30 additions & 1 deletion packages/cli/src/utils/extract-hook-body.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@
* 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`) makes
* `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`, `.sudo(`) makes
* extraction **throw**.
*
* ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes
Expand DownExpand Up@@ -85,6 +85,35 @@ const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [
{ rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' },
{ rx: /\beval\s*\(/, reason: '`eval()` is not allowed in hook/action bodies' },
{ rx: /\bnew\s+Function\s*\(/, reason: '`new Function()` is not allowed in hook/action bodies' },
// [#14010] `sudo()` exists on the HOST `ScopedContext` and is NOT marshalled
// into the VM, so lowering a handler that calls it turns working in-process
// code into a `TypeError` that only production sees. Refusing here is what
// makes the two runtimes agree: the callable is still registered in
// `functions` and still shipped through the `.mjs` bundle by `lowerCallables`,
// so the handler keeps running in-process where `sudo()` is real — the build
// just declines to ALSO emit it as a body that cannot run.
//
// Same family as the `crypto.hash` retirement three lines into
// CAPABILITY_PATTERNS below (#4391): a member advertised ahead of its
// implementation, where the build-time inference was the amplifier rather
// than the safety net. The difference is the remedy — `crypto.hash` had no
// working channel to fall back to, this one does.
//
// Receiver-loose, like the `.object(...)` / `.title(...)` capability patterns:
// a local alias (`const api = ctx.api; api.sudo()`) must not slip through,
// and over-refusal is the SAFE direction here (the handler is bundled and
// works; it is only `--strict-body`, which demands a body for every callable,
// that turns this into a hard failure — correctly, since a body needing
// elevation genuinely cannot be one).
{
rx: /\.\s*sudo\s*\(/,
reason:
'`sudo()` is not reachable from a sandboxed body — the VM\'s `ctx.api` carries only `object()` '
+ 'and `transaction()`, so the call is a TypeError at run time (and under a hook\'s default '
+ '`onError: \'abort\'` that aborts the triggering write). Stamp the value from the record\'s own '
+ 'before-hook (`ctx.input.<field> = ...`), or leave this handler bundled so it runs in-process '
+ 'where `sudo()` exists',
},
];

const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | 'crypto.uuid' | 'log' }> = [
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/extract-hook-body.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -193,6 +193,46 @@ describe('extractHookBody', () => {
expect(() => extractHookBody(fn, 'hook free')).toThrow(/not in scope at runtime|moduleScopeHelper/);
});

// ── `sudo()` is not a body-reachable member (#14010) ────────────────────
//
// `ScopedContext.sudo()` is REAL in-process and absent from the VM's
// `ctx.api`, so the same handler source passes a native `hook.handler(ctx)`
// test and TypeErrors once the build lowers it into a body. Refusing the
// extraction is what keeps the two runtimes honest: `lowerCallables` catches
// this throw and ships the callable through the .mjs bundle, so the handler
// keeps working in-process — only the unrunnable body is declined.
it('rejects a handler calling ctx.api.sudo() (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.sudo().object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
expect(() => extractHookBody(fn, 'hook elevate')).toThrow(/`sudo\(\)` is not reachable/);
});

it('rejects the aliased receiver too — `const api = ctx.api; api.sudo()` (#14010)', () => {
// Receiver-loose on purpose: under-refusing here is the failure that only
// production sees, which is the whole defect.
const fn = async (ctx: any) => {
const api = ctx.api;
await api.sudo().object('crm_account').update({ id: ctx.input.id, x: 1 });
};
expect(() => extractHookBody(fn, 'hook elevate alias')).toThrow(/`sudo\(\)` is not reachable/);
});

// The reverse leg: without the pattern this body extracts CLEANLY and the
// build emits a `body.source` that TypeErrors in the sandbox. Asserting the
// ordinary shape still passes is what proves the pattern did not widen into
// the majority case it sits beside.
it('still extracts an ordinary non-elevated ctx.api write (#14010)', () => {
const fn = async (ctx: any) => {
await ctx.api.object('crm_account').update({ id: ctx.input.id, current_grade: 'A' });
};
const ext = extractHookBody(fn, 'hook plain');
expect(ext.capabilities).toContain('api.write');
// Quote-agnostic: this file is itself bundled, and esbuild rewrites the
// literal's quotes before `String(fn)` ever runs.
expect(ext.source).toMatch(/object\((['"])crm_account\1\)/);
});

it('extracts a self-contained handler that only uses params + globals (#1876)', () => {
const fn = (ctx: any) => {
ctx.record.id = Math.round(Number(ctx.record.raw));
Expand Down
19 changes: 14 additions & 5 deletions packages/lint/src/validate-readonly-hook-writes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,11 +74,16 @@ describe('validateReadonlyHookWrites - RED: a ctx.api write to a readonly field'
expect(findings[0].path).toBe('hooks[0].body.source');
expect(findings[0].message).toContain("'last_activity_date'");
expect(findings[0].message).toContain('crm_account');
// The remedy must name BOTH legitimate channels, not only sudo - telling an
// author to elevate is a security-relevant instruction, and the own-hook
// stamp is the shape that needs no elevation at all.
// [#14010] The remedy is the own-hook stamp, and the hint must say so.
expect(findings[0].hint).toContain('ctx.input.last_activity_date');
expect(findings[0].hint).toContain('sudo');
// ...and it must NOT prescribe sudo. This rule reads L2 body sources, which
// run in QuickJS, whose ctx.api has no `sudo` - so the hint used to point
// 100% of its population at a TypeError (aborting the triggering write,
// under the default onError:'abort'). Substring-matching 'sudo' is not
// enough to tell "offers it" from "warns against it": the corrected hint
// still contains the word. Pin the DIRECTION.
expect(findings[0].hint).toMatch(/sudo\(\) is NOT an option|not marshalled into the sandbox/);
expect(findings[0].hint).not.toMatch(/make the elevation explicit|write it through ctx\.api\.sudo/);
});

it('flags updateById, whose payload is argument 1', () => {
Expand DownExpand Up@@ -344,7 +349,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s
expect(findings[0].severity).toBe('warning');
// The own-hook stamp is NOT the remedy here, and the hint must not offer it.
expect(findings[0].hint).not.toContain('ctx.input.credit_hold');
expect(findings[0].hint).toContain('sudo');
// [#14010] Nor is sudo, for the sandbox-reachability reason above - so this
// hint offers NEITHER, and says which record states the write is safe on.
expect(findings[0].hint).toContain('not marshalled into the sandbox');
expect(findings[0].hint).not.toMatch(/write it through ctx\.api\.sudo/);
expect(findings[0].hint).toContain('readonlyWhen predicate is FALSE');
});

it('reports a field carrying BOTH flags as the certain (static readonly) finding', () => {
Expand Down
38 changes: 27 additions & 11 deletions packages/lint/src/validate-readonly-hook-writes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -46,14 +46,30 @@
// flagged. Exactly the reason the flow sibling skips `create_record`.
//
// - Only a NON-ELEVATED `ctx.api`. `ScopedContext.sudo()` returns a context
// with `isSystem: true`, which the strip skips entirely - the hook-side
// analogue of a flow's `runAs:'system'`, and the intended channel for
// "users cannot edit this, but automation maintains it". A `.sudo()` chain
// with `isSystem: true`, which the strip skips entirely. A `.sudo()` chain
// is structurally invisible to the extractor (its `api-crud-literal`
// matcher requires a literal `ctx.api` receiver, and `ctx.api.sudo()` is a
// CallExpression), so elevated writes cannot be flagged even by accident.
// Measured, not assumed - `validate-readonly-hook-writes.test.ts` pins it.
//
// ⚠️ [#14010] What that exclusion must NOT become is a recommendation, and
// until this edit both hints below made it one. This rule reads L2
// (`language:'js'`) BODIES - `extractHookBodyWriteSet` parses
// `hooks[i].body.source` and nothing else - and a body runs in QuickJS,
// whose VM-side `ctx.api` carries `object()` and the transaction leaves and
// NO `sudo` (`installCtx` in runtime/src/sandbox/quickjs-runner.ts; pinned
// exhaustively in `quickjs-runner.test.ts`). `sudo()` is real only on the
// HOST `ScopedContext` handed to an in-process `handler`. So the prescribed
// remedy was a `TypeError` for 100% of this rule's population - and under a
// hook's default `onError: 'abort'` that aborts the triggering write, which
// is a gating rule pointing at a dead feature. The exclusion stands (an
// elevated write is genuinely not stripped); the ADVICE does not.
//
// A hook still has no DECLARED elevation knob - there is no hook-side
// `runAs` - so the honest hint is the own-hook stamp, and #14010 is where
// the missing knob is argued. Issue ids stay in this comment, out of the
// message an author reads and cannot act on.
//
// - Only a LITERAL object name and a LITERAL payload key. A dynamic object
// (`ctx.api.object(name)`) or a non-literal payload yields no extraction at
// all, so nothing is guessed.
Expand DownExpand Up@@ -286,11 +302,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`on every non-system trigger the engine strips readonly keys from that UPDATE payload - ` +
`the write never lands, while the call still returns success.`,
hint:
`If automation is meant to maintain '${w.field}', either stamp it on the record's OWN hook ` +
`(ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape), or make the elevation explicit with ctx.api.sudo().object('${objectName}') ` +
`- deliberately, since sudo bypasses the acting user's row and field permissions for that write. ` +
`Otherwise drop readonly:true from '${w.field}'.`,
`If automation is meant to maintain '${w.field}', stamp it on the record's OWN hook - ` +
`ctx.input.${w.field} = ... in beforeInsert/beforeUpdate survives the strip, and is the ` +
`recommended shape. Note that ctx.api.sudo() is NOT an option from a body: sudo() lives on ` +
`the in-process ScopedContext and is not marshalled into the sandbox, so calling it here is a ` +
`TypeError at run time. Otherwise drop readonly:true from '${w.field}'.`,
});
} else if (meta.readonlyWhen) {
reported.add(dedupeKey);
Expand All@@ -307,9 +323,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind
`write may silently not land depending on the record's state.`,
hint:
`readonlyWhen strips even a beforeUpdate-derived value, so an own-hook stamp is NOT a ` +
`workaround here. If automation must maintain '${w.field}' regardless of record state, write it ` +
`through ctx.api.sudo(). Otherwise confirm this call only targets records whose readonlyWhen ` +
`predicate is FALSE.`,
`workaround here - and neither is ctx.api.sudo(), which is not marshalled into the sandbox ` +
`(calling it from a body is a TypeError at run time). Confirm this call only targets records ` +
`whose readonlyWhen predicate is FALSE, or drop '${w.field}' from this payload.`,
});
}
}
Expand Down
Loading
Loading