diff --git a/.changeset/hook-run-as-inherit.md b/.changeset/hook-run-as-inherit.md new file mode 100644 index 0000000000..0d12ef3523 --- /dev/null +++ b/.changeset/hook-run-as-inherit.md @@ -0,0 +1,57 @@ +--- +"@objectstack/spec": minor +"@objectstack/objectql": minor +"@objectstack/lint": patch +--- + +feat(hooks): `runAs` on a hook — `'system' | 'user' | 'inherit'`, default `'inherit'` + +A hook's `ctx.api` runs with the context of the write that fired it, so a column +an app wants **computed and never hand-written** could not be expressed: author +`editable: false` for the persona and the direct `PATCH` is refused — and so is +the hook that maintains the column, by the same field-level check. The guard and +the legitimate writer were the same door. The only elevation a hook had was the +in-process `ctx.api.sudo()`, which is not marshalled into the sandbox (a +`TypeError` once a build lowers the handler into a body) and which rides the L3 +bundle path that is being retired. + +`HookSchema` now accepts `runAs`: + +| value | the hook's `ctx.api` data operations run as | +| --- | --- | +| `'inherit'` (default) | the context of the triggering write — exactly the behaviour every hook has today | +| `'system'` | elevated: a full-access, RLS-bypassing system principal | +| `'user'` | the triggering user; a hook whose trigger resolved no user has its data operations **refused** (`HOOK_UNSCOPED_DATA_ACCESS`) rather than run unscoped | + +`'system'` and `'user'` mean here exactly what they mean on `flow.runAs` — same +word, same semantics. `'inherit'` is the hook-only third value, because only a +hook has a context to inherit; a flow establishes its identity from nothing, +which is why its default is `'user'` and this one's is `'inherit'`. Nothing on +`FlowSchema` changes. + +**Purely additive: no migration, no behaviour change for any existing hook.** +The default reproduces today's behaviour by handing the engine-built `ctx.api` +through unchanged, and an absent key parses to it. + +Scope, deliberately narrow: `ctx.api` data operations only. `condition` +evaluation, the `readonly` strip applied to the hook's own `ctx.input` payload, +`ctx.session` and `async` semantics all keep reading the triggering operation's +context, and declaring `runAs: 'system'` does not elevate the write that fired +the hook. + +Elevation is authorization, not anonymity: a `runAs: 'system'` write still +carries the triggering user, so `created_by` / `updated_by` and the audit row +still name the operator. + +Honoured on both execution surfaces — the in-process `handler` and the +sandboxed `body`. + +Authoring notes: + +- `sudo`, `elevate`, `elevated` and `isSystem` are refused with a prescription + naming `runAs`, and `run_as` is answered as a rename. +- `@objectstack/lint`'s gating `hook-api-update-readonly-field` rule now skips a + hook that declares `runAs: 'system'` — the static `readonly` strip skips a + system context, so the write it exists to catch does not happen — and its + hints name the knob. The `readonlyWhen` warning is unchanged: a system context + does not waive a conditional lock. diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 483a6bc0a6..4bf5090d7e 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -259,19 +259,39 @@ There is an asymmetry here that costs data if you learn it the hard way, so lear | How the body writes it | What happens | |:---|:---| | `ctx.input. = …` 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({ })` | **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({ })` | **`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').update({ })` | **Silently dropped** — unless the hook declares `runAs: 'system'`. `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, and the call still returns success. Declaring [`runAs: 'system'`](/docs/automation/hooks#elevation--runas) gives that `ctx.api` a system context, which the strip skips, so the write lands. | +| `ctx.api.sudo().object('x').update({ })` | **`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. The knob to reach for is [`runAs: 'system'`](/docs/automation/hooks#elevation--runas) on the hook itself, which is declarative and works on **both** surfaces. | | `ctx.api.object('x').insert({ })` | **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*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: `sudo()` a body cannot reach (see the row above), and — unlike the static strip — the conditional lock is **not** waived by a system context either, so there is no elevated caller for which a caller-supplied value survives. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object. +- `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: unlike the static strip, the conditional lock is **not** waived by a system context, so neither `runAs: 'system'` nor the `sudo()` a body cannot reach makes a caller-supplied value survive. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object. 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). The table above is about a **hook** body. An **action** body is the one surface where the answer changes, so read this before you move a body from one to the other: an action body runs **elevated** — its `ctx.api` is built over the caller's envelope with `isSystem` set, which is the same trusted posture that lets an action bypass row and field permissions — and the static strip applies only to non-system callers. So `ctx.api.object('x').update({ someReadonlyField })` **lands** in an action, and there is no finding for it. Elevation does not waive the *conditional* lock, though, so that half does carry across: `action-api-update-readonly-when-field` — a **warning** — on an action body's literal `ctx.api` update to a `readonlyWhen` field ([#13770](https://github.com/objectstack-ai/objectstack/issues/13770)). Net effect when you move a body: a `readonly` write changes behaviour, a `readonlyWhen` write does not. +### Elevating a body — `runAs` + +The knob that makes "guarded **and** maintained" expressible is a hook-level declaration, not a body API: `runAs: 'system'` gives the hook's `ctx.api` a system context, which is what lets a column nobody may hand-write still be maintained by the automation that owns it. It is honoured identically for an in-process `handler` and a sandboxed `body` — which is the whole reason it exists rather than `ctx.api.sudo()`, a member only the in-process surface has. + +```ts +{ + name: 'stamp_account_grade', + object: 'rating', + events: ['afterInsert'], + runAs: 'system', // this hook maintains a column nobody may hand-write + body: { + language: 'js', + source: `await ctx.api.object('account').update({ id: ctx.input.account_id, current_grade: ctx.input.grade });`, + capabilities: ['api.read', 'api.write'], + }, +} +``` + +It scopes `ctx.api` and nothing else — the `condition` gate, `ctx.session`, the strip applied to the hook's own `ctx.input`, and `async` behaviour all still read the triggering operation's context. Full semantics, including the `'user'` value and what it refuses, are on [Hooks → Elevation](/docs/automation/hooks#elevation--runas). + ### Errors from `ctx.api` A rejected `ctx.api` call gives your body the host error's `name` and `message`, plus two structured properties when the host supplied them: diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index e9c76f88ff..7cc7f7ef99 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -141,6 +141,39 @@ Because `record` now means the record's *state*, `record.done == true` alone is on **every** update of an already-done row. If you wrote a condition under the old payload semantics expecting "the write that changed it", add the `previous` half. +## Elevation — `runAs` + +A hook's `ctx.api` runs with the context of the write that **fired** it. That is usually what you want, and it is the one thing that makes a column both *guarded* and *maintained* impossible to express: author `editable: false` (or `readonly: true`) so nobody hand-writes the column, and the hook that computes it is refused by the very same check. The guard and the legitimate writer are the same door. + +`runAs` is the declaration that separates them. It is the same key a [flow](/docs/automation/flows) declares, and `'system'` and `'user'` mean there exactly what they mean here, plus one value only a hook can have — a flow establishes its identity from nothing, so it has no context to inherit: + +| `runAs` | The hook's `ctx.api` runs as | +|:---|:---| +| `'inherit'` *(default)* | the context of the triggering write — the behaviour every hook had before this key existed. | +| `'system'` | elevated: a full-access, RLS-bypassing system principal. Row and field permissions do not apply, and the static `readonly` strip is skipped. | +| `'user'` | the triggering **user**, so the hook can never exceed that user's grants. A hook fired by a write that carried no user has no identity to scope to, so its `ctx.api` data operations are **refused** (`HOOK_UNSCOPED_DATA_ACCESS`) rather than run unscoped. | + +```ts +{ + name: 'stamp_account_grade', + object: 'rating', + events: ['afterInsert'], + runAs: 'system', // this hook maintains a column nobody may hand-write + body: { + language: 'js', + source: `await ctx.api.object('account').update({ id: ctx.input.account_id, current_grade: ctx.input.grade });`, + capabilities: ['api.read', 'api.write'], + }, +} +``` + +Four things worth knowing before you reach for it: + +- **It scopes `ctx.api`, and nothing else.** The `condition` gate, `ctx.session`, the `readonly` strip applied to the hook's own `ctx.input` payload, and `async` behaviour all still read the triggering operation's context. Declaring `runAs: 'system'` does **not** elevate the write that fired the hook. +- **Elevation is authorization, not anonymity.** The triggering user rides along, so an elevated write still stamps `created_by` / `updated_by` with the operator and still appears under their name in the audit log. You do not trade the audit trail for the write. +- **Both surfaces honour it** — an in-process `handler` and a sandboxed `body` alike. This is the reason `runAs` exists rather than `ctx.api.sudo()`: `sudo()` is real only in-process, so the same source passed a unit test and threw in production. +- **`'user'` is a narrowing, and it fails closed.** It cannot resolve to a grant: a hook that declares it and finds no trigger user refuses its data operations instead of running them with no principal at all. + ## Before Hook Mutate the incoming record before it is saved. The engine exposes the pending diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 3e311b82dc..7a644eed97 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -109,18 +109,18 @@ that silently does not happen. | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor | |:--|:---|:---|:---|:---| -| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11289` | -| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11472` | -| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10024` | +| 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11290` | +| 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11473` | +| 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10025` | | 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1746` | -| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10072`, `readonly-strict-errors.ts:66` | -| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5891` | -| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3735`, `:3745`, `:3772` | +| 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10073`, `readonly-strict-errors.ts:66` | +| 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5892` | +| 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3736`, `:3746`, `:3773` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `system-write-guard.ts:96`, `:120` | | 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `identity-write-guard.ts:99` | -| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6589` | -| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12084` | -| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12013` | +| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `objectql/src/engine.ts:6590` | +| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `objectql/src/engine.ts:12085` | +| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `objectql/src/engine.ts:12014` | ### 3. Sharing (`plugin-sharing`) @@ -179,8 +179,8 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| -| 62 | `objectql/src/engine.ts:3542` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14433` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 62 | `objectql/src/engine.ts:3543` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | +| 63 | `objectql/src/engine.ts:14463` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -195,7 +195,7 @@ assuming `isSystem` covers it is a documented source of bugs. |:---|:---|:---| | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:702` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | -| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10007`–`10024` | +| "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10008`–`10025` | | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1537` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index 867fef07ac..b5fb06793a 100644 --- a/content/docs/references/api/contract.mdx +++ b/content/docs/references/api/contract.mdx @@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +292 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | +| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +293 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | | **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112) | | **message** | `string` | ✅ | Readable error message | | **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. | @@ -187,6 +187,7 @@ const result = ApiErrorSchema.parse(data); * `FORBIDDEN` * `FORM_NOT_FOUND` * `FORM_RESOLVE_FAILED` +* `HOOK_UNSCOPED_DATA_ACCESS` * `IMPORT_JOB_CREATE_FAILED` * `IMPORT_ROW_FAILED` * `INTERNAL` diff --git a/content/docs/references/api/error-code-ledger.mdx b/content/docs/references/api/error-code-ledger.mdx index 44312c13df..590e7ec045 100644 --- a/content/docs/references/api/error-code-ledger.mdx +++ b/content/docs/references/api/error-code-ledger.mdx @@ -303,6 +303,7 @@ const result = ErrorCode.parse(data); * `FORBIDDEN` * `FORM_NOT_FOUND` * `FORM_RESOLVE_FAILED` +* `HOOK_UNSCOPED_DATA_ACCESS` * `IMPORT_JOB_CREATE_FAILED` * `IMPORT_ROW_FAILED` * `INTERNAL` diff --git a/packages/lint/src/validate-readonly-hook-writes.test.ts b/packages/lint/src/validate-readonly-hook-writes.test.ts index 3c3414cde7..b654c1b6b1 100644 --- a/packages/lint/src/validate-readonly-hook-writes.test.ts +++ b/packages/lint/src/validate-readonly-hook-writes.test.ts @@ -337,6 +337,34 @@ describe('validateReadonlyHookWrites - GREEN: nothing statically knowable is gue }); describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the same verdict', () => { + // [#14010] A hook that DECLARES elevation is not the shape this rule judges: + // `runAs: 'system'` gives its `ctx.api` a system context, and the static + // strip skips a system context, so the write lands. Gating a build over + // working code is the mirror of the defect the hint fix above closed. + it("skips a hook that declares runAs: 'system' — its write is not stripped", () => { + const stack = crmStack("await ctx.api.object('crm_account').update({ last_activity_date: now });"); + const elevated = { + ...stack, + hooks: (stack.hooks as any[]).map((h) => ({ ...h, runAs: 'system' })), + }; + expect(validateReadonlyHookWrites(elevated)).toHaveLength(0); + // …and the control, so the skip is attributable to the declaration alone: + // the SAME body without it is still the gating error. + expect(validateReadonlyHookWrites(stack)).toHaveLength(1); + }); + + it.each(['user', 'inherit'])( + "still flags a hook that declares runAs: '%s' — neither reaches the strip elevated", + (runAs) => { + const stack = crmStack("await ctx.api.object('crm_account').update({ last_activity_date: now });"); + const declared = { + ...stack, + hooks: (stack.hooks as any[]).map((h) => ({ ...h, runAs })), + }; + expect(validateReadonlyHookWrites(declared)).toHaveLength(1); + }, + ); + // readonlyWhen strips per record STATE, so the write is conditional, not // certain - warning, exactly as the flow sibling grades it. it('grades a readonlyWhen field as an advisory warning, not an error', () => { @@ -368,6 +396,11 @@ describe('validateReadonlyHookWrites - readonlyWhen is a SECOND shape, not the s expect(finding.hint).toContain('beforeUpdate hook'); expect(finding.hint).toContain('does land, even on a locked record'); expect(finding.hint).not.toMatch(/strips even a beforeUpdate-derived value/); + // [#14010] The conditional lock is the one place elevation genuinely does + // NOT help, so the hint must keep saying so now that the platform HAS a + // declared elevation knob — an author who just learned `runAs: 'system'` + // from the static-readonly hint would otherwise reach for it here. + expect(finding.hint).toContain("runAs: 'system'"); expect(finding.hint).not.toMatch(/own-hook stamp is NOT a workaround/); // NOT elevation, for two independent reasons, both stated. `sudo()` is diff --git a/packages/lint/src/validate-readonly-hook-writes.ts b/packages/lint/src/validate-readonly-hook-writes.ts index e08ea46cd4..33c871e708 100644 --- a/packages/lint/src/validate-readonly-hook-writes.ts +++ b/packages/lint/src/validate-readonly-hook-writes.ts @@ -65,10 +65,19 @@ // 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. +// [#14010] A hook now HAS a declared elevation knob: `runAs: 'system'`, +// which the engine applies to `ctx.api` on BOTH surfaces (the in-process +// handler and the sandboxed body). That changes this rule twice over: +// +// - a hook declaring `runAs: 'system'` is SKIPPED entirely, because the +// static strip skips a system context and the write genuinely lands. +// Gating a build over a write that works would be the mirror of the +// defect above - a rule punishing the very shape it should teach; +// - the hint now names `runAs: 'system'` beside the own-hook stamp, and +// still says why `sudo()` is not the answer from a body. +// +// 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 @@ -100,18 +109,24 @@ // local and visible, which is the flow sibling's epistemic position // (`flow-update-readonly-field`, `error`), not the unknown-field rule's. // -// The honest caveat, measured rather than glossed: a hook has NO declared run -// identity. A flow declares `runAs`, which is what lets its rule call the strip -// a certainty; a hook inherits its context from whoever triggered the write, so -// this write is dropped whenever the triggering operation is non-system - the -// default, and the only path a user-reachable object can rely on - and lands on -// the system-triggered path. The residual case (a hook whose `ctx.api` write -// only ever runs under a system-triggered operation) is not a stable invariant: -// nothing declares or enforces it, and the first user-context write silently -// voids the stamp. Its remedy is the same `.sudo()` this rule points at, which -// makes the elevation explicit instead of accidental - so the flagged code is -// worth changing under BOTH readings, which is what makes gating defensible -// here where it would not be for an existence check. +// The caveat this rule was written under, and how #14010 closed it: a hook used +// to have NO declared run identity. A flow declares `runAs`, which is what lets +// its rule call the strip a certainty; a hook inherited its context from whoever +// triggered the write, so the write was dropped whenever the triggering +// operation was non-system - the default, and the only path a user-reachable +// object can rely on - and landed on the system-triggered path. The residual +// case (a hook whose `ctx.api` write only ever runs under a system-triggered +// operation) was not a stable invariant: nothing declared or enforced it, and +// the first user-context write silently voided the stamp. +// +// Since #14010 a hook declares `runAs` too, so that residual case became a +// DECLARATION - which is why this rule now skips a `runAs: 'system'` hook +// outright (the guard beside `extractHookBodyWriteSet` below) instead of +// grading it. What stays flagged is the undeclared write, whose outcome still +// depends on who triggered it; the remedy is to declare the elevation, which +// makes it explicit instead of accidental - so the flagged code is worth +// changing under BOTH readings, which is what makes gating defensible here +// where it would not be for an existence check. // // Measured field data before choosing to gate: 0 findings across every example // app in this repo (`examples/app-crm`, `app-showcase`, `app-todo`) - the @@ -239,6 +254,16 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind const source = body.source; if (typeof source !== 'string' || source.trim() === '') return; + // [#14010] A hook that DECLARES elevation is not writing through a + // caller-shaped payload at all: `runAs: 'system'` gives its `ctx.api` a + // system context, and the static-`readonly` strip skips a system context + // entirely (`stripReadonlyFields` — the same exemption an action body has + // always had). So the write this rule exists to catch — the one that is + // silently dropped — does not happen, and reporting it would fail a build + // over working code. Only `'system'` is exempt: `'user'` and `'inherit'` + // both reach the strip as an ordinary caller payload. + if (hook.runAs === 'system') return; + const extracted = extractHookBodyWriteSet(source); // A body that did not parse yields whatever error recovery left readable, // and this rule GATES - so a mis-extraction here would break a build over a @@ -304,9 +329,11 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind hint: `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}'.`, + `recommended shape. To keep writing it CROSS-OBJECT from here, declare runAs: 'system' on ` + + `this hook: the strip skips a system context, so the write lands, and the triggering user ` + + `is still stamped on the record. 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); @@ -326,8 +353,9 @@ export function validateReadonlyHookWrites(stack: AnyRec): ReadonlyHookWriteFind `Either confirm this call only targets records whose readonlyWhen predicate is FALSE, or ` + `derive '${w.field}' in a beforeUpdate hook on '${objectName}' - a hook-derived value is not ` + `caller-supplied and does land, even on a locked record. Elevation is not a workaround here: ` + - `ctx.api.sudo() is not marshalled into the sandbox (calling it from a body is a TypeError at ` + - `run time), and a system context does not waive the conditional lock in any case. Otherwise ` + + `a system context does not waive the conditional lock (unlike the static readonly strip), so ` + + `neither runAs: 'system' nor ctx.api.sudo() helps - and sudo() is not marshalled into the ` + + `sandbox in any case (calling it from a body is a TypeError at run time). Otherwise ` + `drop '${w.field}' from this payload. This warning never blocks a build.`, }); } diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 116d840815..bf75d36620 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -203,6 +203,7 @@ import { divergingHookPayloadKeys, MultiUpdateHookKeyDivergenceError, } from './multi-update-hook-key-divergence.js'; +import { UnscopedHookApi, type HookRunAs, type HookRunAsRef, type RunAsDerivableApi } from './hook-run-as.js'; import type { HookWriteRecording } from './hook-write-provenance.js'; import { resolveMasterDetailRelation } from './master-detail.js'; // [#6457] The master-detail header a `parent`-scoped predicate reads is made @@ -14098,7 +14099,7 @@ export class ObjectRepository implements IScopedObjectRepository { * begin/commit/rollback trio and the identity getters stay off it, so this * class is deliberately wider than what it implements. */ -export class ScopedContext implements IScopedContext { +export class ScopedContext implements IScopedContext, RunAsDerivableApi { constructor( private executionContext: ExecutionContext, private engine: IDataEngine @@ -14117,6 +14118,35 @@ export class ScopedContext implements IScopedContext { ); } + /** + * [#14010] The api a hook that declared `runAs` is handed — derived from the + * engine-built one (`buildHookApi(opCtx.context)`) at dispatch, by + * `wrapDeclarativeHook`, for the duration of the handler call. `'inherit'` + * never reaches here (the wrapper hands the original through by reference). + * + * - `'system'` → {@link sudo}: `{ ...triggering context, isSystem: true }`. + * The same envelope the in-process `ctx.api.sudo()` idiom produced, so a + * hook that used to elevate by hand elevates identically by declaration + * — and now on the sandboxed surface too. `userId` / `tenantId` / + * `transaction` ride along: elevation is authorization, not anonymity + * (#5494), and the hook's writes stay inside the triggering transaction. + * - `'user'` → `{ ...triggering context, isSystem: false }` when the + * trigger resolved a user — the pin de-elevates a hook fired by a + * `runAs:'system'` flow node or an `isSystem` service write that still + * named its operator. With NO `userId` there is nothing to scope to, so + * the api is an {@link UnscopedHookApi}: every data door refuses + * (`HOOK_UNSCOPED_DATA_ACCESS`, the #3760 posture — never a silent + * fall-open, never a silent re-badge as system). + */ + withRunAs(runAs: Exclude, ref: HookRunAsRef): IScopedContext { + if (runAs === 'system') return this.sudo(); + if (!this.executionContext.userId) return new UnscopedHookApi(ref); + return new ScopedContext( + { ...this.executionContext, isSystem: false }, + this.engine + ); + } + /** * Execute a callback within a database transaction. * diff --git a/packages/objectql/src/hook-run-as.test.ts b/packages/objectql/src/hook-run-as.test.ts new file mode 100644 index 0000000000..18094af7cc --- /dev/null +++ b/packages/objectql/src/hook-run-as.test.ts @@ -0,0 +1,425 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14010] `Hook.runAs` — the declared execution identity of a hook's `ctx.api` + * data operations. Ruling 2026-09-01 (director batch #22, maintainer 「同意」): + * `'system' | 'user' | 'inherit'`, default `'inherit'`. + * + * ## What the card measured, and what these pins therefore have to say + * + * An app wanted a column COMPUTED and never hand-written. Authoring + * `editable: false` for the persona refuses the direct `PATCH` — and refuses + * the cross-object hook that maintains the column, because the hook's + * `ctx.api` is a `ScopedContext` over the TRIGGERING write's context. The + * guard and the legitimate writer were the same door. So the pins below are + * about ONE question: **what execution context does a hook's `ctx.api` present + * to the middleware chain?** They read it where `plugin-security` reads it — + * `opCtx.context` at the engine middleware seam — rather than at the driver, + * because that is the seam whose `isSystem` short-circuit precedes the + * field-level write check (`security-plugin.ts`, step 2.5). + * + * ## Two layers, deliberately + * + * 1. `wrapDeclarativeHook` over a REAL `ScopedContext` whose engine is a + * recorder — the surgical layer, where "the engine was never called" is + * assertable, which is the whole point of the `'user'` refusal. + * 2. A real `ObjectQL` + stub driver dispatch — the assembly layer, which is + * what proves `buildHookApi` produces an api the wrapper can derive from + * at all. Without it every pin in layer 1 could pass while every real hook + * that declared `runAs` threw. + * + * The scope fence (ruling item 5) is measured, not asserted in prose: the + * TRIGGERING operation's own context is read back at the same seam and shown + * unchanged under `runAs: 'system'`. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL, ScopedContext } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; +import { wrapDeclarativeHook } from './hook-wrappers.js'; +import { + HookUnscopedDataAccessError, + HOOK_UNSCOPED_DATA_ACCESS_CODE, + HOOK_UNSCOPED_DATA_ACCESS_STATUS, + hookRunAs, +} from './hook-run-as.js'; +import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js'; +import { assertEngineUpdateDispatch } from './engine-update-dispatch.js'; +import type { Hook, HookContext } from '@objectstack/spec/data'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; + +const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; + +/** The triggering operator: a real user, no elevation — the card's persona. */ +const OPERATOR: ExecutionContext = { + userId: 'usr_operator', + tenantId: 'org_1', + positions: ['member'], + permissions: ['member_default'], + isSystem: false, +} as ExecutionContext; + +/** A trigger that resolved NO user: an `isSystem` service write, a system flow node. */ +const USERLESS: ExecutionContext = { isSystem: true, positions: [], permissions: [] } as ExecutionContext; + +// --------------------------------------------------------------------------- +// Layer 1 — the wrapper over a real ScopedContext whose engine records. +// --------------------------------------------------------------------------- + +/** Records the `context` every data operation carries — the security seam's input. */ +function recordingEngine() { + const seen: Array<{ method: string; context: unknown }> = []; + const engine: any = { + seen, + async insert(_o: string, _d: unknown, options: any) { seen.push({ method: 'insert', context: options?.context }); return { id: 'r1' }; }, + // The shared dispatch predicates, so this double can never be LOOSER than + // ObjectQL itself about which write shapes it accepts + // (`check:engine-double-contract`). + async update(_o: string, _d: unknown, options: any) { + // Both arguments: the address may ride the PAYLOAD (`update({ id, … })`, + // the shape a hook writes) or `options.where` — passing only the bag + // refuses a call the real engine accepts. + assertEngineUpdateDispatch(_d as any, options); + seen.push({ method: 'update', context: options?.context }); + return { id: 'r1' }; + }, + async find(_o: string, query: any) { seen.push({ method: 'find', context: query?.context }); return []; }, + async delete(_o: string, options: any) { + assertEngineDeleteDispatch(options); + seen.push({ method: 'delete', context: options?.context }); + return true; + }, + }; + return engine; +} + +function makeCtx(execCtx: ExecutionContext, engine: any, overrides: Partial = {}): HookContext { + return { + object: 'account', + event: 'afterInsert', + input: { data: { name: 'acme' } }, + session: { userId: execCtx.userId, organizationId: execCtx.tenantId }, + api: new ScopedContext(execCtx, engine), + ql: undefined, + ...overrides, + } as unknown as HookContext; +} + +function hookOf(runAs: unknown, extra: Partial = {}): Hook { + return { + name: 'stamp_grade', + object: 'account', + events: ['afterInsert'], + priority: 100, + ...(runAs === undefined ? {} : { runAs }), + ...extra, + } as unknown as Hook; +} + +describe('#14010 Hook.runAs — the identity a hook\'s ctx.api presents', () => { + it("'inherit' hands the engine-built api through BY REFERENCE — the default is today's behaviour", async () => { + const engine = recordingEngine(); + const ctx = makeCtx(OPERATOR, engine); + const original = ctx.api; + let seenInside: unknown; + + const wrapped = wrapDeclarativeHook(hookOf('inherit'), async (c) => { + seenInside = c.api; + await (c.api as any).object('grade').insert({ v: 1 }); + }, { logger: silentLogger }); + await wrapped(ctx); + + // Reference equality, not deep equality: "byte-identical to today" is a + // claim about the object the handler is handed, and a structurally equal + // copy would satisfy a deep comparison while being a new derivation. + expect(seenInside).toBe(original); + expect(engine.seen).toEqual([{ method: 'insert', context: OPERATOR }]); + }); + + it('an ABSENT runAs behaves exactly as `inherit` — an existing hook is unaffected', async () => { + const engine = recordingEngine(); + const ctx = makeCtx(OPERATOR, engine); + const original = ctx.api; + let seenInside: unknown; + + const wrapped = wrapDeclarativeHook(hookOf(undefined), async (c) => { seenInside = c.api; }, { logger: silentLogger }); + await wrapped(ctx); + + expect(hookRunAs({ name: 'stamp_grade' })).toBe('inherit'); + expect(seenInside).toBe(original); + }); + + it("'system' ELEVATES the hook's data operations — and carries the operator through (#5494)", async () => { + const engine = recordingEngine(); + const ctx = makeCtx(OPERATOR, engine); + + const wrapped = wrapDeclarativeHook(hookOf('system'), async (c) => { + await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' }); + }, { logger: silentLogger }); + await wrapped(ctx); + + const context = engine.seen[0]!.context as ExecutionContext; + // The flag the security middleware short-circuits on, BEFORE its + // field-level write check — which is what makes `editable: false` and a + // hook-maintained column able to coexist at all. + expect(context.isSystem).toBe(true); + // Elevation is authorization, not anonymity: the operator rides along, so + // the audit stamps (which gate on `session.userId`, never on `isSystem`) + // still name them. `updated_by` does not become a system principal. + expect(context.userId).toBe('usr_operator'); + expect(context.tenantId).toBe('org_1'); + }); + + it("'user' DE-ELEVATES — a hook fired by an elevated write is pinned to the trigger's user", async () => { + const engine = recordingEngine(); + // The realistic shape: an elevated service write that still names its + // operator (the #3783 approvals mirror's `{ isSystem: true, userId }`). + const elevatedButAttributed = { ...OPERATOR, isSystem: true } as ExecutionContext; + const ctx = makeCtx(elevatedButAttributed, engine); + + const wrapped = wrapDeclarativeHook(hookOf('user'), async (c) => { + await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' }); + }, { logger: silentLogger }); + await wrapped(ctx); + + const context = engine.seen[0]!.context as ExecutionContext; + expect(context.isSystem).toBe(false); + expect(context.userId).toBe('usr_operator'); + }); + + describe("'user' with NO trigger user — the refusal, not a fall-open (#3760's posture)", () => { + it('refuses the data operation with the ADR-0112 envelope, and the engine is never called', async () => { + const engine = recordingEngine(); + const ctx = makeCtx(USERLESS, engine); + let thrown: any; + + const wrapped = wrapDeclarativeHook(hookOf('user', { onError: 'abort' }), async (c) => { + await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' }); + }, { logger: silentLogger }); + try { await wrapped(ctx); } catch (e) { thrown = e; } + + // Envelope first: code AND status, per the standard clause. A bare + // "it threw" would pass for a TypeError from a missing member, which is + // exactly the failure shape this card exists to end. + expect(thrown).toBeInstanceOf(HookUnscopedDataAccessError); + expect(thrown.code).toBe(HOOK_UNSCOPED_DATA_ACCESS_CODE); + expect(thrown.status).toBe(HOOK_UNSCOPED_DATA_ACCESS_STATUS); + expect(thrown.hook).toBe('stamp_grade'); + // The remedy the author can act on, and the reason. + expect(thrown.message).toContain("runAs: 'system'"); + expect(thrown.message).toContain('UNSCOPED'); + // The decisive half: refusing means the operation did NOT run unscoped. + expect(engine.seen).toEqual([]); + }); + + it('refuses `transaction()` too — both doors, not just `object()`', async () => { + const engine = recordingEngine(); + const ctx = makeCtx(USERLESS, engine); + let thrown: any; + + const wrapped = wrapDeclarativeHook(hookOf('user'), async (c) => { + await (c.api as any).transaction(async () => 'never'); + }, { logger: silentLogger }); + try { await wrapped(ctx); } catch (e) { thrown = e; } + + expect(thrown?.code).toBe(HOOK_UNSCOPED_DATA_ACCESS_CODE); + expect(engine.seen).toEqual([]); + }); + + it('a `user` hook that touches NO data still runs — the refusal is at the data door', async () => { + const engine = recordingEngine(); + const ctx = makeCtx(USERLESS, engine); + let ran = false; + + const wrapped = wrapDeclarativeHook(hookOf('user'), async () => { ran = true; }, { logger: silentLogger }); + await expect(wrapped(ctx)).resolves.toBeUndefined(); + expect(ran).toBe(true); + }); + }); + + describe('the swap is scoped to the handler call', () => { + it('restores ctx.api after the handler returns, and after it throws', async () => { + const engine = recordingEngine(); + const ctx = makeCtx(OPERATOR, engine); + const original = ctx.api; + + await wrapDeclarativeHook(hookOf('system'), async () => {}, { logger: silentLogger })(ctx); + expect(ctx.api).toBe(original); + + const boom = wrapDeclarativeHook(hookOf('system'), async () => { throw new Error('boom'); }, { logger: silentLogger }); + await expect(boom(ctx)).rejects.toThrow('boom'); + expect(ctx.api).toBe(original); + }); + + it('a fire-and-forget after* hook keeps its derived api past the synchronous restore', async () => { + // The restore runs in a `finally` that fires before an async handler's + // first `await` resumes, so a shared ctx would silently un-elevate the + // very writes an author declared `runAs` for. + const engine = recordingEngine(); + const ctx = makeCtx(OPERATOR, engine); + const original = ctx.api; + let resolveGate: () => void = () => {}; + const gate = new Promise((r) => { resolveGate = r; }); + let done: () => void = () => {}; + const finished = new Promise((r) => { done = r; }); + + const wrapped = wrapDeclarativeHook(hookOf('system', { async: true }), async (c) => { + await gate; + await (c.api as any).object('account').update({ id: 'acct_1', current_grade: 'A' }); + done(); + }, { logger: silentLogger }); + + await wrapped(ctx); + // The engine's own api is already back on the shared context… + expect(ctx.api).toBe(original); + resolveGate(); + await finished; + + // …and the detached handler still wrote elevated. + expect(engine.seen).toHaveLength(1); + expect((engine.seen[0]!.context as ExecutionContext).isSystem).toBe(true); + }); + }); + + it('a runAs value outside the enum is refused LOUDLY at bind time, never silently ignored', () => { + // Declared ≠ enforced is the defect this key exists to end, so a hook that + // reached the engine without going through `HookSchema` cannot declare an + // identity the engine then drops on the floor. + expect(() => wrapDeclarativeHook(hookOf('elevated'), async () => {}, { logger: silentLogger })) + .toThrow(/not one of 'system' \| 'user' \| 'inherit'/); + }); +}); + +// --------------------------------------------------------------------------- +// Layer 2 — a real ObjectQL dispatch, read at the middleware seam. +// --------------------------------------------------------------------------- + +const FIELDS = { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + current_grade: { name: 'current_grade', label: 'Grade', type: 'text' as const }, +}; +const accountObject = { name: 'runas_account', label: 'Account', fields: FIELDS }; +const ratingObject = { name: 'runas_rating', label: 'Rating', fields: FIELDS }; + +function stubDriver() { + const store = new Map>(); + const storeFor = (o: string) => { if (!store.has(o)) store.set(o, new Map()); return store.get(o)!; }; + let nextId = 0; + const d: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, async syncSchema() {}, + async find(o: string) { return Array.from(storeFor(o).values()); }, + async findOne(o: string, ast: any) { + for (const r of storeFor(o).values()) if (!ast?.where?.id || r.id === ast.where.id) return r; + return null; + }, + async create(o: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; storeFor(o).set(id, row); return row; + }, + async update(o: string, id: string, data: Record) { + const s = storeFor(o); const cur = s.get(id) ?? { id }; + const u = { ...cur, ...data, id }; s.set(id, u); return u; + }, + async upsert(o: string, data: any) { return data.id ? this.update(o, data.id, data) : this.create(o, data); }, + async delete(o: string, id: string) { return storeFor(o).delete(id); }, + async count(o: string) { return storeFor(o).size; }, + async bulkCreate(o: string, rows: any[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async updateMany() { return 0; }, async deleteMany() { return 0; }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return d; +} + +/** + * A real dispatch: inserting a `runas_rating` fires a hook that writes the + * computed column on `runas_account` through `ctx.api`. Every context that + * reaches the middleware chain is recorded, per object — which is where + * `plugin-security` reads the flag its short-circuit keys on. + */ +async function bootReal(runAs: string | undefined) { + const engine = new ObjectQL(); + engine.registerDriver(stubDriver(), true); + await engine.init(); + engine.registry.registerObject(accountObject as any); + engine.registry.registerObject(ratingObject as any); + + const contexts: Record = { runas_account: [], runas_rating: [] }; + engine.registerMiddleware(async (opCtx: any, next: any) => { + (contexts[opCtx.object] ??= []).push(opCtx.context); + return next(); + }); + + const hook = { + name: 'stamp_account_grade', + object: 'runas_rating', + events: ['afterInsert'], + priority: 100, + ...(runAs === undefined ? {} : { runAs }), + handler: async (ctx: any) => { + await ctx.api.object('runas_account').update({ id: 'acct_1', current_grade: 'A' }); + }, + } as unknown as Hook; + bindHooksToEngine(engine, [hook], { packageId: 'app:test', logger: silentLogger }); + + // The row the hook maintains has to exist: the engine's write-not-found gate + // refuses an update to a missing id, and that refusal would read exactly like + // a `runAs` failure. Seeded elevated, then the recording is cleared so every + // context read below belongs to the dispatch under test. + await engine.insert('runas_account', { id: 'acct_1', name: 'Acme' }, { context: { isSystem: true } as any }); + contexts.runas_account.length = 0; + contexts.runas_rating.length = 0; + return { engine, contexts }; +} + +describe('#14010 Hook.runAs — through a real ObjectQL dispatch', () => { + it("'system' reaches the middleware seam elevated, while the TRIGGERING write does not (scope fence)", async () => { + const { engine, contexts } = await bootReal('system'); + + await engine.insert('runas_rating', { name: 'r1' }, { context: OPERATOR }); + + // The hook's own write — elevated, and attributed. + const hookWrite = contexts.runas_account[0]; + expect(hookWrite?.isSystem).toBe(true); + expect(hookWrite?.userId).toBe('usr_operator'); + + // Ruling item 5: the fence. The write that FIRED the hook keeps its own + // context — `runAs` elevates the hook's api, never the triggering + // operation, so the readonly strip, the field-level check and the RLS + // filter on that operation are exactly what they were. + const triggering = contexts.runas_rating[0]; + expect(triggering?.isSystem).toBe(false); + expect(triggering?.userId).toBe('usr_operator'); + }); + + it("'inherit' presents the triggering context unchanged — the pre-runAs behaviour, measured", async () => { + const { engine, contexts } = await bootReal('inherit'); + await engine.insert('runas_rating', { name: 'r1' }, { context: OPERATOR }); + expect(contexts.runas_account[0]?.isSystem).toBe(false); + expect(contexts.runas_account[0]?.userId).toBe('usr_operator'); + }); + + it('an UNDECLARED hook is byte-identical to `inherit` — the zero-migration claim', async () => { + const declared = await bootReal('inherit'); + await declared.engine.insert('runas_rating', { name: 'r1' }, { context: OPERATOR }); + const absent = await bootReal(undefined); + await absent.engine.insert('runas_rating', { name: 'r1' }, { context: OPERATOR }); + expect(absent.contexts.runas_account[0]).toEqual(declared.contexts.runas_account[0]); + }); + + it("'user' with a user-less trigger refuses the hook's write, and the trigger keeps its own context", async () => { + const { engine, contexts } = await bootReal('user'); + + await expect(engine.insert('runas_rating', { name: 'r1' }, { context: USERLESS })) + .rejects.toMatchObject({ code: HOOK_UNSCOPED_DATA_ACCESS_CODE, status: HOOK_UNSCOPED_DATA_ACCESS_STATUS }); + + // Nothing ran unscoped: no account write reached the chain at all. + expect(contexts.runas_account).toEqual([]); + expect(contexts.runas_rating[0]?.isSystem).toBe(true); + }); +}); diff --git a/packages/objectql/src/hook-run-as.ts b/packages/objectql/src/hook-run-as.ts new file mode 100644 index 0000000000..45aed9b014 --- /dev/null +++ b/packages/objectql/src/hook-run-as.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14010] `Hook.runAs` — the declared execution identity of a hook's `ctx.api` + * data operations (ruling 2026-09-01: `'system' | 'user' | 'inherit'`, default + * `'inherit'`). + * + * ## What it repairs + * + * A hook's `ctx.api` is a `ScopedContext` over the context of the write that + * fired it (`ObjectQL.buildHookApi(opCtx.context)`), so the hook runs as the + * operator. A column an app wants COMPUTED and never hand-written is protected + * by authoring `editable: false` for the persona — and that same field-level + * check then refuses the hook that maintains the column: the guard and the + * legitimate writer were the same door. The only elevation a hook had was the + * in-process `ctx.api.sudo()`, which the sandbox does not carry (a `TypeError` + * once the build lowers the handler into a body — closed as a trap by #14044) + * and which rides the L3 bundle path that is being retired. `runAs` is the + * declared knob, honoured on BOTH surfaces at the one place both are wrapped + * (`hook-wrappers.ts` `wrapDeclarativeHook`). + * + * ## The three values, and why the default is not FlowSchema's + * + * - `'system'` — elevate: the hook's `ctx.api` carries `isSystem: true` over + * the triggering context. The security middleware short-circuits on that + * flag before every row-level and field-level gate, so the card's symptom is + * fixed exactly there. Elevation is not anonymity (#5494): `userId` rides + * along, so `updated_by` and the audit row still name the operator. + * - `'user'` — pin: `isSystem: false` over the triggering context. A hook + * whose trigger resolved NO user has nothing to scope to, so its data + * operations are REFUSED ({@link HookUnscopedDataAccessError}) rather than + * run unscoped — the hook-side twin of service-automation's #3760 refusal, + * same reasoning, same remedy sentence, its own registered code. + * - `'inherit'` — the hook-only third value and the default: the engine-built + * api is handed through UNTOUCHED (same object reference), which is the + * pre-`runAs` behaviour. A flow establishes its run identity from nothing + * and so has no such value; only a hook has a context to inherit, which is + * why the defaults differ while `'system'` / `'user'` mean the same thing + * in both places. + * + * ## Scope fence (ruling item 5) + * + * `ctx.api` data operations ONLY. The `condition` gate, the `readonly` strip + * on the hook's own `ctx.input` payload, `ctx.session`, and the `async` + * semantics all keep reading the TRIGGERING operation's context — this module + * never touches `opCtx.context`, only the api object the handler is handed. + */ + +import type { + EngineTransactionInfo, + EngineTransactionOptions, + IScopedContext, + IScopedObjectRepository, +} from '@objectstack/spec/contracts'; + +/** The declared values, in the schema's order. */ +export const HOOK_RUN_AS_VALUES = ['system', 'user', 'inherit'] as const; +export type HookRunAs = (typeof HOOK_RUN_AS_VALUES)[number]; + +/** ADR-0112 code for the `'user'`-without-a-user refusal (ledger: `@objectstack/objectql`). */ +export const HOOK_UNSCOPED_DATA_ACCESS_CODE = 'HOOK_UNSCOPED_DATA_ACCESS' as const; +/** + * `403`: the operation is refused for lack of an identity to authorize it as, + * which is an authorization answer, not a malformed request (`400`) — the + * payload and the predicate are both fine; there is no principal to scope + * them to. + */ +export const HOOK_UNSCOPED_DATA_ACCESS_STATUS = 403 as const; + +/** Where a refusal happened — carried on the error so the message can name it. */ +export interface HookRunAsRef { + /** The hook whose `runAs` produced the refusal. */ + hook: string; + /** The object the TRIGGERING operation targeted (not the object the hook tried to reach). */ + object?: string; + /** The lifecycle event that fired the hook. */ + event?: string; +} + +/** + * Thrown from a `runAs: 'user'` hook's `ctx.api` when its trigger resolved no + * user (#14010; wording mirrors #3760's `UnscopedRunDataAccessError` rather + * than importing it — the flow engine is not a dependency of the query + * engine). + * + * The refusal is the point: `'user'` is an access-NARROWING declaration, and + * ADR-0049's standing rule is that failing to resolve a narrowing declaration + * must never resolve to a grant. Deliberately NOT `{ isSystem: true }` either + * (see #3760): the middleware's `isSystem` short-circuit precedes gates a + * principal-less context still has to clear, so re-badging the hook as system + * would WIDEN it. + * + * Thrown at the DATA DOOR (`object()` / `transaction()`), not at dispatch: a + * `'user'` hook that never touches data still runs, exactly as a `runAs:'user'` + * flow still runs its non-data nodes. + */ +export class HookUnscopedDataAccessError extends Error { + override readonly name = 'HookUnscopedDataAccessError'; + readonly code = HOOK_UNSCOPED_DATA_ACCESS_CODE; + readonly status = HOOK_UNSCOPED_DATA_ACCESS_STATUS; + readonly hook: string; + readonly object?: string; + readonly event?: string; + + // The message carries ADR-0049 — customer-resolvable — and NOT the tracker ids + // (#3760, #14010): those live in this file's header, where a reader who can + // resolve them is already looking (`check:doc-authoring`). + constructor(ref: HookRunAsRef) { + const where = [ + `hook '${ref.hook}'`, + ref.object ? `object '${ref.object}'` : undefined, + ref.event ? `event '${ref.event}'` : undefined, + ] + .filter(Boolean) + .join(', '); + super( + `[runAs] refusing a data operation (${where}): this hook's runAs is 'user' but no trigger user ` + + `could be resolved, so the operation would execute UNSCOPED (elevated, RLS-bypassing) rather ` + + `than restricted to a user. Declare \`runAs: 'system'\` on the hook to make the elevation ` + + `explicit and intended, or arrange for the trigger to supply a user (a write made with a system ` + + `context carries none). Branch on \`code === '${HOOK_UNSCOPED_DATA_ACCESS_CODE}'\` (ADR-0112) ` + + `to detect this. (ADR-0049)`, + ); + this.hook = ref.hook; + if (ref.object !== undefined) this.object = ref.object; + if (ref.event !== undefined) this.event = ref.event; + } +} + +/** + * The `ctx.api` a `runAs: 'user'` hook is handed when its trigger resolved no + * user: every data door refuses with {@link HookUnscopedDataAccessError}. + * + * It implements the same contract the engine's `ScopedContext` does + * (`IScopedContext`: `object`, `transaction`), so a body's `ctx.api.object(…)` + * reaches the refusal through the ordinary sandbox plumbing rather than a + * `TypeError` about a missing member. + */ +export class UnscopedHookApi implements IScopedContext { + constructor(private readonly ref: HookRunAsRef) {} + + object(_name: string): IScopedObjectRepository { + throw new HookUnscopedDataAccessError(this.ref); + } + + transaction( + _callback: (trxCtx: IScopedContext, info: EngineTransactionInfo) => Promise, + _opts?: EngineTransactionOptions, + ): Promise { + return Promise.reject(new HookUnscopedDataAccessError(this.ref)); + } +} + +/** + * What an api must offer for a hook's `runAs` to be applied to it. The + * engine's `ScopedContext` implements it (`engine.ts` `withRunAs`); it is + * deliberately NOT on `IScopedContext`, the hook-author contract — a hook + * never calls this, the wrapper does. + */ +export interface RunAsDerivableApi extends IScopedContext { + withRunAs(runAs: Exclude, ref: HookRunAsRef): IScopedContext; +} + +/** + * Read a hook's declared `runAs`. An absent key is the schema default + * (`'inherit'`); any other non-member is refused LOUDLY here rather than + * tolerated, so a hook bound from a source that skipped `HookSchema` cannot + * declare an identity the engine then silently ignores (declared ≠ enforced + * is the defect this key exists to end). Thrown from `wrapDeclarativeHook`, + * which the binder calls per hook — under `strict` binding the boot fails, + * otherwise the hook is skipped with the reason logged. + */ +export function hookRunAs(meta: { name?: unknown; runAs?: unknown }): HookRunAs { + const raw = meta.runAs; + if (raw === undefined) return 'inherit'; + if (typeof raw === 'string' && (HOOK_RUN_AS_VALUES as readonly string[]).includes(raw)) { + return raw as HookRunAs; + } + throw new Error( + `[hook] hook '${String(meta.name ?? '')}' declares runAs: ${JSON.stringify(raw)}, which is ` + + `not one of ${HOOK_RUN_AS_VALUES.map((v) => `'${v}'`).join(' | ')}. HookSchema refuses this at ` + + `authoring time; the engine refuses it here so the declaration is never silently ignored.`, + ); +} + +/** + * Derive the api a hook runs with from the engine-built one. + * + * `'inherit'` returns the SAME object (pinned by reference equality — that is + * the "byte-identical to today" guarantee). Anything else requires an api that + * can derive identities; an api that cannot is a loud error, never a silent + * fall-through to the un-elevated context. + */ +export function deriveHookApi(api: unknown, runAs: HookRunAs, ref: HookRunAsRef): unknown { + if (runAs === 'inherit') return api; + const derivable = api as Partial | undefined; + if (!derivable || typeof derivable.withRunAs !== 'function') { + throw new Error( + `[hook] hook '${ref.hook}' declares runAs: '${runAs}' but its ctx.api cannot derive an execution ` + + `identity (no withRunAs). Only the engine-built ScopedContext can; a hook context assembled ` + + `by hand must supply one or leave runAs at 'inherit'.`, + ); + } + return derivable.withRunAs(runAs, ref); +} diff --git a/packages/objectql/src/hook-wrappers.ts b/packages/objectql/src/hook-wrappers.ts index a6239076f2..431c1eef99 100644 --- a/packages/objectql/src/hook-wrappers.ts +++ b/packages/objectql/src/hook-wrappers.ts @@ -22,6 +22,7 @@ import { ExpressionEngine } from '@objectstack/formula'; import { noopHookMetricsRecorder, type HookMetricsRecorder, type HookMetricOutcome } from './hook-metrics.js'; import { materializeDeclaredFields } from './declared-fields.js'; import { describeCelFault, type CelFault } from './cel-fault.js'; +import { hookRunAs, deriveHookApi, type HookRunAs } from './hook-run-as.js'; /** * The logger the hook layer writes its diagnostics to — the `Logger` CONTRACT @@ -359,6 +360,10 @@ export function wrapDeclarativeHook( // `async` is only meaningful for after* events; ignore on before* (we must // wait for the handler to potentially mutate ctx.input). const fireAndForget = Boolean(meta.async) && isAfterEvent; + // [#14010] Read ONCE at wrap time, and refused here for a non-member so the + // binder skips (or, under `strict`, fails) the hook instead of running it + // under an identity it never declared. `undefined` is the schema default. + const runAs: HookRunAs = hookRunAs(meta as { name?: unknown; runAs?: unknown }); const runWithTimeout = async (ctx: HookContext): Promise => { if (!timeoutMs) { @@ -444,6 +449,13 @@ export function wrapDeclarativeHook( } const restore = installFlatInput(ctx); + // [#14010] `runAs` is applied to `ctx.api` ONLY, and only for the handler + // call — the same install/restore shape as the flat input above, so the + // engine's own context object is never touched (the condition gate above + // already ran against it; the readonly strip and the session keep reading + // it). Under `'inherit'` this is a no-op that leaves the engine-built api + // in place by reference. + const restoreApi = installRunAsApi(ctx, runAs, meta.name); const startedAt = Date.now(); const recordOutcome = (err?: any) => { @@ -464,7 +476,15 @@ export function wrapDeclarativeHook( try { metrics.recordSkip(labelFor(ctx), 'fire_and_forget'); } catch { /* noop */ } // For fire-and-forget we can't keep ctx.input swapped while the // engine moves on — copy what we need, restore, and run async. - void runWithErrorPolicy(ctx) + // + // [#14010] The SAME race would un-elevate a `runAs` hook: the `finally` + // below restores `ctx.api` synchronously, before an async handler's + // first `await` returns. So a non-`'inherit'` hook runs against a + // detached view that keeps the derived api past that restore. Under + // `'inherit'` the context object is handed through unchanged, exactly + // as before this key existed. + const detached = runAs === 'inherit' ? ctx : { ...ctx }; + void runWithErrorPolicy(detached) .then(() => recordOutcome()) .catch((err) => { recordOutcome(err); @@ -485,11 +505,36 @@ export function wrapDeclarativeHook( throw err; } } finally { + restoreApi(); restore(); } }; } +/** + * [#14010] Swap `ctx.api` for the api the hook's declared `runAs` derives from + * it, returning the restore. `'inherit'` installs nothing and restores nothing + * — the engine-built api stays in place BY REFERENCE, which is the "byte- + * identical to today" guarantee the ruling's default rests on and the pin in + * `hook-run-as.test.ts` asserts with `toBe`. + * + * The derivation itself lives on the api (`ScopedContext.withRunAs`) because + * only the api holds the full triggering `ExecutionContext` — `ctx.session` is + * a projection of it and drops the transaction handle, among other things. + */ +function installRunAsApi(ctx: HookContext, runAs: HookRunAs, hookName: string): () => void { + if (runAs === 'inherit') return () => {}; + const original = ctx.api; + ctx.api = deriveHookApi(original, runAs, { + hook: hookName, + object: ctx.object, + event: ctx.event, + }) as HookContext['api']; + return () => { + ctx.api = original; + }; +} + /** * Swap `ctx.input` in place for a Proxy that exposes a flat record view * over the engine's `{ data, options, id? }` wrapper. Returns a function diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 9a6dc785f3..672312296e 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -125,6 +125,20 @@ export { MULTI_UPDATE_HOOK_KEY_DIVERGENCE_STATUS, divergingHookPayloadKeys, } from './multi-update-hook-key-divergence.js'; +// [#14010] `Hook.runAs` — the declared execution identity of a hook's `ctx.api` +// data operations. The refusal a `runAs: 'user'` hook raises when its trigger +// resolved no user (ADR-0112 code + status), the api that raises it, and the +// reader the wrapper applies per hook. +export { + HookUnscopedDataAccessError, + HOOK_UNSCOPED_DATA_ACCESS_CODE, + HOOK_UNSCOPED_DATA_ACCESS_STATUS, + HOOK_RUN_AS_VALUES, + UnscopedHookApi, + hookRunAs, + deriveHookApi, +} from './hook-run-as.js'; +export type { HookRunAs, HookRunAsRef, RunAsDerivableApi } from './hook-run-as.js'; // Boot guard: thrown by `ObjectQL.init()` when a registered driver's connect() // fails (framework#3741). Hosts that boot the engine themselves can catch it to // render their own "database unreachable" message. diff --git a/packages/platform-objects/src/apps/translations/en.metadata-forms.generated.ts b/packages/platform-objects/src/apps/translations/en.metadata-forms.generated.ts index 5e0e20908b..ffb5cb1f08 100644 --- a/packages/platform-objects/src/apps/translations/en.metadata-forms.generated.ts +++ b/packages/platform-objects/src/apps/translations/en.metadata-forms.generated.ts @@ -551,6 +551,10 @@ export const enMetadataForms: NonNullable = { label: "Timeout", helpText: "Abort the hook after N milliseconds" }, + runAs: { + label: "Run As", + helpText: "Identity for ctx.api data operations: inherit the triggering write (default), system (elevated), or user (the triggering user)" + }, condition: { label: "Condition", helpText: "Optional formula — skip the hook when this evaluates to false" diff --git a/packages/platform-objects/src/apps/translations/es-ES.metadata-forms.generated.ts b/packages/platform-objects/src/apps/translations/es-ES.metadata-forms.generated.ts index 9d7a327d81..4c37198c4d 100644 --- a/packages/platform-objects/src/apps/translations/es-ES.metadata-forms.generated.ts +++ b/packages/platform-objects/src/apps/translations/es-ES.metadata-forms.generated.ts @@ -551,6 +551,10 @@ export const esESMetadataForms: NonNullable = label: "Timeout", helpText: "Abort the hook after N milliseconds" }, + runAs: { + label: "Ejecutar como", + helpText: "Identidad de las operaciones de datos de ctx.api: inherit hereda el contexto de la escritura que lo disparó (predeterminado), system eleva, user lo fija al usuario que lo disparó" + }, condition: { label: "Condición", helpText: "Fórmula opcional — omite el hook cuando evalúa a false" diff --git a/packages/platform-objects/src/apps/translations/ja-JP.metadata-forms.generated.ts b/packages/platform-objects/src/apps/translations/ja-JP.metadata-forms.generated.ts index 022dd25c29..630f3918f9 100644 --- a/packages/platform-objects/src/apps/translations/ja-JP.metadata-forms.generated.ts +++ b/packages/platform-objects/src/apps/translations/ja-JP.metadata-forms.generated.ts @@ -551,6 +551,10 @@ export const jaJPMetadataForms: NonNullable = label: "Timeout", helpText: "Abort the hook after N milliseconds" }, + runAs: { + label: "実行主体", + helpText: "ctx.api のデータ操作の主体：inherit は起点となった書き込みのコンテキストを継承（既定）、system は昇格、user は起点ユーザーに固定" + }, condition: { label: "条件", helpText: "任意の数式 — false 評価時はフックをスキップ" diff --git a/packages/platform-objects/src/apps/translations/zh-CN.metadata-forms.generated.ts b/packages/platform-objects/src/apps/translations/zh-CN.metadata-forms.generated.ts index 266c81315c..99d5f73b9f 100644 --- a/packages/platform-objects/src/apps/translations/zh-CN.metadata-forms.generated.ts +++ b/packages/platform-objects/src/apps/translations/zh-CN.metadata-forms.generated.ts @@ -551,6 +551,10 @@ export const zhCNMetadataForms: NonNullable = label: "Timeout", helpText: "Abort the hook after N milliseconds" }, + runAs: { + label: "运行身份", + helpText: "ctx.api 数据操作的身份：inherit 继承触发写入的上下文（默认）、system 提权、user 钉到触发用户" + }, condition: { label: "条件", helpText: "可选公式——求值为 false 时跳过该钩子" diff --git a/packages/qa/dogfood/test/fixtures/hook-runas-fixture.ts b/packages/qa/dogfood/test/fixtures/hook-runas-fixture.ts new file mode 100644 index 0000000000..47bb7c1041 --- /dev/null +++ b/packages/qa/dogfood/test/fixtures/hook-runas-fixture.ts @@ -0,0 +1,147 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#14010] The card's symptom, as a bootable app: a column that must be +// COMPUTED and never hand-written, maintained by a cross-object hook. +// +// The requirement the downstream app wrote down is "通过界面、批量导入、数据接口 +// 任何途径都不能人工写入" — no channel may write this column by hand. The two +// halves could not both be had: +// +// • author `editable: false` for the persona → the direct PATCH is refused, +// • …and the hook that maintains the column → refused by the SAME check. +// +// The guard and the legitimate writer were the same door, because a hook's +// `ctx.api` is a ScopedContext over the TRIGGERING write's context. This +// fixture keeps both halves in one app so a single boot can show the guard +// still shut and the declared writer now through it. +// +// Deliberately TWO trigger objects rather than two hooks on one, because each +// hook aborts its own triggering write: `hookrunas_rating` fires the hook that +// declares `runAs: 'system'`, and `hookrunas_legacy_rating` fires an +// identical-but-undeclared one, which is the pre-#14010 behaviour and must stay +// exactly that (the ruling's zero-migration claim, measured rather than +// asserted). +// +// The declared hook ships as an L2 BODY on purpose: that is the shape the +// downstream app ships, and the surface that had no elevation at all before +// this card (`sudo()` is not marshalled into the sandbox — #14044). The +// undeclared control is an in-process handler, so the file also shows the two +// execution surfaces side by side. + +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +/** The object carrying the computed column. */ +export const HookRunAsAccount = ObjectSchema.create({ + name: 'hookrunas_account', + // [ADR-0090 D1] grandfather stamp: the gate under test here is FIELD-level + // security, so row-level sharing is deliberately wide open — an RLS refusal + // would be indistinguishable from the FLS one this fixture is about. + sharingModel: 'public_read_write', + label: 'Hook RunAs Account', + pluralLabel: 'Hook RunAs Accounts', + fields: { + name: Field.text({ label: 'Name', required: true }), + /** The computed column: system-maintained, never hand-written. */ + current_grade: Field.text({ label: 'Current grade' }), + /** An ordinary column the same persona MAY edit — the positive control. */ + note: Field.text({ label: 'Note' }), + }, +}); + +/** Inserting one of these fires the hook that DECLARES `runAs: 'system'`. */ +export const HookRunAsRating = ObjectSchema.create({ + name: 'hookrunas_rating', + sharingModel: 'public_read_write', + label: 'Hook RunAs Rating', + pluralLabel: 'Hook RunAs Ratings', + fields: { + account_id: Field.text({ label: 'Account', required: true }), + grade: Field.text({ label: 'Grade', required: true }), + }, +}); + +/** Inserting one of these fires the UNDECLARED hook — today's behaviour. */ +export const HookRunAsLegacyRating = ObjectSchema.create({ + name: 'hookrunas_legacy_rating', + sharingModel: 'public_read_write', + label: 'Hook RunAs Legacy Rating', + pluralLabel: 'Hook RunAs Legacy Ratings', + fields: { + account_id: Field.text({ label: 'Account', required: true }), + grade: Field.text({ label: 'Grade', required: true }), + }, +}); + +const STAMP_SOURCE = ` + await ctx.api.object('hookrunas_account').update({ + id: ctx.input.account_id, + current_grade: ctx.input.grade, + }); +`; + +export const hookRunAsFixtureStack = defineStack({ + manifest: { + id: 'com.dogfood.hookrunas_fixture', + namespace: 'hookrunas', + version: '0.0.0', + type: 'app', + name: 'Hook runAs Fixture', + description: + 'A computed column protected by field-level editable:false and maintained by a hook.', + }, + objects: [HookRunAsAccount, HookRunAsRating, HookRunAsLegacyRating], + hooks: [ + { + name: 'stamp_grade_declared', + label: 'Stamp the computed grade (declared system)', + object: 'hookrunas_rating', + events: ['afterInsert'], + runAs: 'system', + body: { language: 'js', source: STAMP_SOURCE, capabilities: ['api.read', 'api.write'] }, + }, + { + name: 'stamp_grade_undeclared', + label: 'Stamp the computed grade (no runAs — the pre-runAs behaviour)', + object: 'hookrunas_legacy_rating', + events: ['afterInsert'], + handler: async (ctx: any) => { + await ctx.api.object('hookrunas_account').update({ + id: ctx.input.account_id, + current_grade: ctx.input.grade, + }); + }, + }, + ], +} as any); + +const FIXTURE_MEMBER_SET = 'hookrunas_fixture_member'; + +/** + * The persona: full CRUD on all three objects, and ONE field denied for + * editing — `hookrunas_account.current_grade`, object-qualified (a bare + * `current_grade` key enforces nothing; that spelling trap is what the + * permission-zoo audit found). `readable: true` keeps the column visible, which + * is what the app wants: shown everywhere, writable nowhere by hand. + */ +export const hookRunAsMemberSet: PermissionSet = PermissionSetSchema.parse({ + name: FIXTURE_MEMBER_SET, + label: 'Hook RunAs Fixture Member — the computed column is read-only', + objects: { + hookrunas_account: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + hookrunas_rating: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + hookrunas_legacy_rating: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + fields: { + 'hookrunas_account.current_grade': { readable: true, editable: false }, + }, +}); + +export function hookRunAsFixtureSecurity(): SecurityPlugin { + return new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, hookRunAsMemberSet], + fallbackPermissionSet: hookRunAsMemberSet.name, + }); +} diff --git a/packages/qa/dogfood/test/hook-runas-fls.dogfood.test.ts b/packages/qa/dogfood/test/hook-runas-fls.dogfood.test.ts new file mode 100644 index 0000000000..f83e13cee0 --- /dev/null +++ b/packages/qa/dogfood/test/hook-runas-fls.dogfood.test.ts @@ -0,0 +1,146 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#14010] The card's symptom, end to end, through the real HTTP + security +// stack: a computed column can now be BOTH protected from every hand-written +// channel AND maintained by the hook that owns it. +// +// ## What was measured broken (17.1.0, by the reporting app) +// +// | | result | +// | author `editable: false` for the persona | direct PATCH refused, 403 | +// | …and the hook that maintains the column | dies: "[Security] Field write +// | | denied: not permitted to edit" | +// +// The hook ran as the operator, so the field permission that blocked the +// operator blocked the hook — the guard and the legitimate writer were the same +// door. Under the hook's default `onError: 'abort'` the failure surfaced as the +// TRIGGERING save being refused, which is how it presented downstream: an +// approval that would not save. +// +// ## Why this file boots a real app rather than asserting at the seam +// +// `packages/objectql`'s pins measure the ExecutionContext a hook's `ctx.api` +// presents, and `packages/runtime`'s measure the same for a sandboxed body. +// Both stop one layer short of the claim the ruling is actually about, which is +// about `plugin-security`'s composed middleware: that its `isSystem` +// short-circuit precedes the field-level write check (step 2.5), so elevation +// reaches past a denial the same permission set still enforces on the persona. +// "Who serves this path" is a question about the provisioned runtime, so this +// drives the real one: real REST routes, real SecurityPlugin, real QuickJS body. +// +// ## The four legs, and why the last one is mandatory +// +// 1. the guard is REAL — the persona's own PATCH of the column is refused; +// 2. the persona is not simply locked out — an ordinary column still saves +// (without this, leg 1 passes for a broken app); +// 3. the DECLARED hook (`runAs: 'system'`, an L2 body) writes the protected +// column, and the value lands; +// 4. an UNDECLARED hook is still refused, exactly as before this card. That is +// the zero-migration claim, and it is what stops leg 3 from being read as +// "field-level security stopped working". +// +// Attribution is asserted alongside leg 3: elevation is authorization, not +// anonymity (#5494), so the elevated write still stamps the OPERATOR. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { hookRunAsFixtureStack, hookRunAsFixtureSecurity } from './fixtures/hook-runas-fixture.js'; + +const SYS = { isSystem: true } as const; +const MEMBER = 'hook-runas@verify.test'; + +const recordOf = (b: any) => b?.record ?? b?.data ?? b; + +describe('#14010 a hook-maintained column can be protected by editable:false', () => { + let stack: VerifyStack; + let ql: any; + let adminTok: string; + let memberTok: string; + let memberId: string; + let accountId: string; + + beforeAll(async () => { + stack = await bootStack(hookRunAsFixtureStack, { security: hookRunAsFixtureSecurity() }); + adminTok = await stack.signIn(); + // The first user is the seeded dev admin, so this fresh sign-up is a plain + // member who falls back to the fixture permission set. + memberTok = await stack.signUp(MEMBER); + ql = await stack.kernel.getServiceAsync('objectql'); + + const account = await ql.insert( + 'hookrunas_account', + { name: 'Acme', current_grade: null, note: 'seed' }, + { context: SYS }, + ); + accountId = account.id; + memberId = (await ql.findOne('sys_user', { where: { email: MEMBER }, context: SYS }))?.id; + expect(memberId, 'member provisioned').toBeTruthy(); + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + /** The stored row, read with a system context so no read mask is in play. */ + const storedAccount = async () => + (await ql.findOne('hookrunas_account', { where: { id: accountId }, context: SYS })) as any; + + it('leg 1 — the persona CANNOT hand-write the computed column (the guard is real)', async () => { + const res = await stack.apiAs(memberTok, 'PATCH', `/data/hookrunas_account/${accountId}`, { + current_grade: 'FORGED', + }); + expect(res.status).toBe(403); + const body: any = await res.json(); + expect(body?.error?.code ?? body?.code).toBe('PERMISSION_DENIED'); + expect((await storedAccount()).current_grade ?? null).toBeNull(); + }, 60_000); + + it('leg 2 — the same persona CAN write an ordinary column (not simply locked out)', async () => { + const res = await stack.apiAs(memberTok, 'PATCH', `/data/hookrunas_account/${accountId}`, { + note: 'member wrote this', + }); + expect(res.status).toBeLessThan(300); + expect((await storedAccount()).note).toBe('member wrote this'); + }, 60_000); + + it("leg 3 — a `runAs: 'system'` hook body writes the protected column, and it lands", async () => { + const res = await stack.apiAs(memberTok, 'POST', '/data/hookrunas_rating', { + account_id: accountId, + grade: 'A', + }); + expect( + res.status, + `the member's rating insert must succeed: ${res.status} ${await res.clone().text()}`, + ).toBeLessThan(300); + expect(recordOf(await res.json())).toBeTruthy(); + + const account = await storedAccount(); + // The whole point of the card: the column the persona may not touch is + // maintained by the automation that owns it. + expect(account.current_grade).toBe('A'); + // #5494 — elevation is not anonymity. The elevated write is still the + // member's: `updated_by` names the operator, not a system principal. + expect(account.updated_by).toBe(memberId); + }, 60_000); + + it('leg 4 — an UNDECLARED hook is still refused, exactly as before this card', async () => { + const before = (await storedAccount()).current_grade; + + const res = await stack.apiAs(memberTok, 'POST', '/data/hookrunas_legacy_rating', { + account_id: accountId, + grade: 'Z', + }); + + // The pre-#14010 behaviour, unchanged: the hook's write is refused by the + // same field-level check, and under the default `onError: 'abort'` that + // refusal surfaces on the TRIGGERING save — which is exactly how the + // downstream app experienced it. + expect(res.status).toBeGreaterThanOrEqual(400); + const text = await res.text(); + expect(text).toMatch(/not permitted to edit|PERMISSION_DENIED/); + expect(text).toContain('current_grade'); + + // …and nothing moved. + expect((await storedAccount()).current_grade).toBe(before); + }, 60_000); +}); diff --git a/packages/runtime/src/sandbox/hook-run-as.integration.test.ts b/packages/runtime/src/sandbox/hook-run-as.integration.test.ts new file mode 100644 index 0000000000..4c87155475 --- /dev/null +++ b/packages/runtime/src/sandbox/hook-run-as.integration.test.ts @@ -0,0 +1,244 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14010] `Hook.runAs` on the SANDBOXED surface — the half that had no + * elevation at all. + * + * ## Why this file exists at all, and why it is an integration test + * + * Before this card a hook could only elevate through the in-process + * `ctx.api.sudo()`, which is a member of the host `ScopedContext` and is NOT + * marshalled into the VM. So the same handler source PASSED a native + * `hook.handler(ctx)` unit test and threw `TypeError` once `objectstack build` + * lowered it into a `body` — green tests, dead feature, and under the default + * `onError: 'abort'` a dead feature that aborts the triggering write. #14044 + * stopped the build from lowering such a handler; this card supplies what an + * author should write instead, and the ruling requires it to work on BOTH + * surfaces. A unit test of `wrapDeclarativeHook` cannot say whether it does: + * the answer depends on the sandbox reading `ctx.api` from the engine context + * at CALL time (`buildSandboxApi` in `body-runner.ts`, + * `installApiMethod` in `quickjs-runner.ts`) rather than closing over it at + * install time. That is a composition fact, so it is measured in composition: + * a real `ObjectQL`, a real `QuickJSScriptRunner`, real hook bodies. + * + * The reading is taken at the engine middleware seam — `opCtx.context` — which + * is where `plugin-security` reads the `isSystem` its short-circuit keys on, + * ahead of the field-level write check that refused the card's hook. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL, bindHooksToEngine, HOOK_UNSCOPED_DATA_ACCESS_CODE } from '@objectstack/objectql'; +import { hookBodyRunnerFactory } from './body-runner.js'; +import { QuickJSScriptRunner } from './quickjs-runner.js'; + +const account = { + name: 'runas_account', + label: 'Account', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + current_grade: { name: 'current_grade', type: 'text' as const }, + }, +}; +const rating = { + name: 'runas_rating', + label: 'Rating', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + account_id: { name: 'account_id', type: 'text' as const }, + grade: { name: 'grade', type: 'text' as const }, + }, +}; + +/** The triggering operator — a real user, unelevated. */ +const OPERATOR = { + userId: 'usr_operator', + tenantId: 'org_1', + positions: ['member'], + permissions: ['member_default'], + isSystem: false, +} as any; + +/** A trigger that resolved no user at all. */ +const USERLESS = { isSystem: true, positions: [], permissions: [] } as any; + +function makeStubDriver() { + const stores = new Map>>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + let nextId = 0; + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const exp = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (exp ?? null)) return false; + } + return true; + }; + const driver: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + // Honour the caller's bound BY PRESENCE, after the filter — a double that + // ignores `limit` answers a paged read with the whole table + // (`check:objectql-double-limit`). + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; }, + async create(o: string, data: Record) { + nextId += 1; const id = (data.id as string) ?? `r_${nextId}`; const row = { ...data, id }; storeFor(o).set(id, row); return row; + }, + async update(o: string, id: string, data: Record) { + const s = storeFor(o); const cur = s.get(id); if (!cur) throw new Error(`nf ${o}/${id}`); + const up = { ...cur, ...data, id }; s.set(id, up); return up; + }, + async upsert(o: string, data: Record) { const id = data.id as string | undefined; return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data); }, + async delete(o: string, id: string) { return storeFor(o).delete(id); }, + async count(o: string, ast: any) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Record[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, async commit() {}, async rollback() {}, + }; + return { driver, stores }; +} + +/** The body every case runs — the card's shape: stamp a computed column cross-object. */ +const STAMP_SOURCE = ` + await ctx.api.object('runas_account').update({ id: ctx.input.account_id, current_grade: ctx.input.grade }); +`; + +describe('#14010 Hook.runAs — the sandboxed (L2 body) surface', () => { + let engine: ObjectQL; + let contexts: Record; + + beforeEach(async () => { + engine = new ObjectQL(); + const { driver } = makeStubDriver(); + engine.registerDriver(driver, true); + await engine.init(); + for (const o of [account, rating]) engine.registry.registerObject(o as any); + engine.setDefaultBodyRunner( + hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'test' }), + ); + contexts = { runas_account: [], runas_rating: [] }; + engine.registerMiddleware(async (opCtx: any, next: any) => { + (contexts[opCtx.object] ??= []).push(opCtx.context); + return next(); + }); + }); + + async function bind(runAs: string | undefined) { + bindHooksToEngine( + engine, + [ + { + name: 'stamp_account_grade', + object: 'runas_rating', + events: ['afterInsert'], + ...(runAs === undefined ? {} : { runAs }), + body: { language: 'js', source: STAMP_SOURCE, capabilities: ['api.read', 'api.write'] }, + } as any, + ], + { packageId: 'test' }, + ); + await engine.insert('runas_account', { id: 'acct_1', name: 'Acme' }, { context: USERLESS }); + contexts.runas_account.length = 0; + contexts.runas_rating.length = 0; + } + + it("'system' elevates a BODY's ctx.api — the surface that had no elevation at all", async () => { + await bind('system'); + + await engine.insert( + 'runas_rating', + { id: 'rat_1', account_id: 'acct_1', grade: 'A' }, + { context: OPERATOR }, + ); + + const hookWrite = contexts.runas_account[0]; + expect(hookWrite?.isSystem).toBe(true); + // Attribution survives elevation (#5494): the operator is still on the + // context the audit stamps read, so `updated_by` names them. + expect(hookWrite?.userId).toBe('usr_operator'); + expect(hookWrite?.tenantId).toBe('org_1'); + + // …and the write really landed, rather than merely being attempted. + const row = (await engine.findOne('runas_account', { where: { id: 'acct_1' } }, { context: USERLESS })) as any; + expect(row.current_grade).toBe('A'); + + // The scope fence (ruling item 5): the triggering write is untouched. + expect(contexts.runas_rating[0]?.isSystem).toBe(false); + }, 30000); + + it("'inherit' is the sandbox's pre-runAs behaviour, and an ABSENT key is the same thing", async () => { + await bind('inherit'); + await engine.insert('runas_rating', { id: 'rat_1', account_id: 'acct_1', grade: 'B' }, { context: OPERATOR }); + const inherited = contexts.runas_account[0]; + expect(inherited?.isSystem).toBe(false); + expect(inherited?.userId).toBe('usr_operator'); + }, 30000); + + it("'user' with a user-less trigger REFUSES the body's write, with the ADR-0112 envelope", async () => { + await bind('user'); + + // The refusal reaches the body as a real error through the ordinary sandbox + // plumbing (the refusing api implements the same contract), so it surfaces + // on the triggering write under the default `onError: 'abort'` — not as a + // `TypeError` about a missing member, which is the shape #14044 closed. + let thrown: any; + await engine + .insert('runas_rating', { id: 'rat_1', account_id: 'acct_1', grade: 'C' }, { context: USERLESS }) + .catch((e) => { thrown = e; }); + + expect(thrown, 'the user-less write must be refused, not run unscoped').toBeDefined(); + expect(String(thrown?.message)).toContain(HOOK_UNSCOPED_DATA_ACCESS_CODE); + // Nothing ran unscoped. + expect(contexts.runas_account).toEqual([]); + const row = (await engine.findOne('runas_account', { where: { id: 'acct_1' } }, { context: USERLESS })) as any; + expect(row.current_grade ?? null).toBeNull(); + }, 30000); + + it('the two surfaces agree: an in-process handler and a body see the SAME context per runAs', async () => { + // Parity is the point of the ruling — one declaration, one meaning, + // whichever way the hook happens to be executed. Measured rather than + // asserted: the same hook, once as a body (bound above) and once as an + // in-process handler, read at the same seam. + await bind('system'); + await engine.insert('runas_rating', { id: 'rat_1', account_id: 'acct_1', grade: 'A' }, { context: OPERATOR }); + const fromBody = contexts.runas_account[0]; + + const handlerEngine = new ObjectQL(); + const { driver } = makeStubDriver(); + handlerEngine.registerDriver(driver, true); + await handlerEngine.init(); + for (const o of [account, rating]) handlerEngine.registry.registerObject(o as any); + const handlerContexts: any[] = []; + handlerEngine.registerMiddleware(async (opCtx: any, next: any) => { + if (opCtx.object === 'runas_account') handlerContexts.push(opCtx.context); + return next(); + }); + bindHooksToEngine( + handlerEngine, + [{ + name: 'stamp_account_grade', + object: 'runas_rating', + events: ['afterInsert'], + runAs: 'system', + handler: async (ctx: any) => { + await ctx.api.object('runas_account').update({ id: ctx.input.account_id, current_grade: ctx.input.grade }); + }, + } as any], + { packageId: 'test' }, + ); + await handlerEngine.insert('runas_account', { id: 'acct_1', name: 'Acme' }, { context: USERLESS }); + handlerContexts.length = 0; + await handlerEngine.insert('runas_rating', { id: 'rat_1', account_id: 'acct_1', grade: 'A' }, { context: OPERATOR }); + + expect(handlerContexts[0]).toEqual(fromBody); + }, 30000); +}); diff --git a/packages/spec/liveness/hook.json b/packages/spec/liveness/hook.json index 7920f1b70f..43b503bc5e 100644 --- a/packages/spec/liveness/hook.json +++ b/packages/spec/liveness/hook.json @@ -71,6 +71,14 @@ "producer": "packages/objectql/src/hook-binder.ts#bindHooksToEngine (the same `wrapDeclarativeHook` call as retryPolicy)", "note": "'log' suppresses+continues; 'abort' rethrows. 2026-08-28: RE-ANCHORED (#13003) — accurate producer line migrated; the path-only evidence pointer is now an anchor (see `retryPolicy`). Re-closed by hand against 8cb96ec41." }, + "runAs": { + "status": "live", + "verifiedAt": "2026-09-03", + "evidenceScope": "in-repo", + "evidence": "packages/objectql/src/hook-wrappers.ts#wrapDeclarativeHook (`hookRunAs(meta)` read once at wrap time, then `installRunAsApi(ctx, runAs, …)` swaps `ctx.api` for the handler call — 'system' elevates, 'user' pins to the triggering user or refuses data operations, 'inherit' leaves the engine-built api untouched); packages/objectql/src/engine.ts#withRunAs (the ScopedContext derivation both surfaces reach)", + "producer": "packages/objectql/src/hook-binder.ts#bindHooksToEngine (the same `wrapDeclarativeHook(hook, resolved, …)` call as retryPolicy — the binder hands the AUTHORED hook to the wrapper)", + "note": "#14010 (ruling 2026-09-01): 'system' | 'user' | 'inherit', default 'inherit' = the pre-runAs behaviour. Scope is ctx.api data operations only (condition, readonly strip, session, async untouched). Honoured on both the in-process handler and the sandboxed body because the sandbox reads ctx.api from the engine context at call time (runtime/src/sandbox/body-runner.ts buildSandboxApi). Pinned in packages/objectql/src/hook-run-as.test.ts and packages/runtime/src/sandbox/hook-run-as.integration.test.ts." + }, "label": { "status": "dead", "evidence": "pure docs, zero runtime readers (runtime uses only name)", diff --git a/packages/spec/liveness/state-counts.md b/packages/spec/liveness/state-counts.md index b33daf0eea..e8758582ed 100644 --- a/packages/spec/liveness/state-counts.md +++ b/packages/spec/liveness/state-counts.md @@ -31,7 +31,7 @@ for both corollaries. | `field` | 89 | 0 | 0 | 1 | 2 | 92 | | `flow` | 34 | 0 | 0 | 6 | 0 | 40 | | `action` | 41 | 0 | 0 | 3 | 2 | 46 | -| `hook` | 18 | 0 | 0 | 2 | 0 | 20 | +| `hook` | 19 | 0 | 0 | 2 | 0 | 21 | | `permission` | 36 | 0 | 0 | 6 | 0 | 42 | | `position` | 12 | 0 | 0 | 0 | 0 | 12 | | `agent` | 21 | 4 | 0 | 1 | 0 | 26 | @@ -62,4 +62,4 @@ for both corollaries. | `metadata_endpoints` | 6 | 0 | 0 | 2 | 0 | 8 | | `batch_endpoints` | 5 | 0 | 0 | 2 | 0 | 7 | | `route_generation` | 0 | 0 | 0 | 4 | 0 | 4 | -| **total** | **843** | **5** | **1** | **84** | **10** | **943** | +| **total** | **844** | **5** | **1** | **84** | **10** | **944** | diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 2954a3f116..886736d354 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -535,6 +535,13 @@ export const ERROR_CODE_LEDGER = { // been written (`TransactionUnsupportedError`, `transaction-errors.ts`; // ADR-0119 D1/D4 fail-closed posture). Same #8087-gate family. 'ERR_TRANSACTION_UNSUPPORTED', + // [#14010] a hook declared `runAs: 'user'` and its trigger resolved NO user + // (an `isSystem` plugin/service write, a system-elevated flow node), so its + // `ctx.api` data operation has no identity to scope to and is REFUSED + // rather than run unscoped — the hook-side twin of service-automation's + // `AUTOMATION_UNSCOPED_RUN_DATA_ACCESS` (#3760), registered under the + // package that throws it. `HookUnscopedDataAccessError`, `hook-run-as.ts`. + 'HOOK_UNSCOPED_DATA_ACCESS', // [#14099] a `multi: true` update whose per-row `beforeUpdate` dispatches // assigned DIFFERENT sets of payload keys — a transition stamp // (`completed_at` on the move into `done`) is the measured shape. One `SET` diff --git a/packages/spec/src/contracts/scoped-context.ts b/packages/spec/src/contracts/scoped-context.ts index 8814cb6f86..5c1878edc0 100644 --- a/packages/spec/src/contracts/scoped-context.ts +++ b/packages/spec/src/contracts/scoped-context.ts @@ -71,7 +71,11 @@ * permissions" as part of the first-hook vocabulary, which no document * teaches and #5945's ruling did not authorize. The engine's own privileged * writers reach it as the engine, not as a hook. Declaring it is a - * maintainer call, not a measurement. + * maintainer call, not a measurement. Since #14010 the DECLARED way for a + * hook to run its data operations elevated is `Hook.runAs: 'system'` + * (`data/hook.zod.ts`), which the engine applies to this very `api` on + * both the in-process and the sandboxed surface — so an author never + * needs `sudo()` here, and the sandbox's lack of it stops being a trap. * - The discrete `beginTransaction` / `commitTransaction` / `rollbackTransaction` * trio, which exists for the sandbox RUNNER — it drives a body's * `ctx.api.transaction(fn)` across host event-loop turns where the ambient diff --git a/packages/spec/src/data/hook.form.ts b/packages/spec/src/data/hook.form.ts index 5a1c1aae89..2ce9faa927 100644 --- a/packages/spec/src/data/hook.form.ts +++ b/packages/spec/src/data/hook.form.ts @@ -69,6 +69,11 @@ export const hookForm = defineForm({ { label: 'Log', value: 'log' }, ] }, { field: 'timeout', type: 'number', colSpan: 1, helpText: 'Abort the hook after N milliseconds' }, + { field: 'runAs', type: 'select', colSpan: 1, helpText: 'Identity for ctx.api data operations: inherit the triggering write (default), system (elevated), or user (the triggering user)', options: [ + { label: 'Inherit (triggering write)', value: 'inherit' }, + { label: 'System (elevated)', value: 'system' }, + { label: 'User (triggering user)', value: 'user' }, + ] }, { field: 'condition', type: 'code', language: 'javascript', colSpan: 2, helpText: 'Optional formula — skip the hook when this evaluates to false' }, { field: 'retryPolicy', diff --git a/packages/spec/src/data/hook.test.ts b/packages/spec/src/data/hook.test.ts index 405ad3d70d..3705ad8df8 100644 --- a/packages/spec/src/data/hook.test.ts +++ b/packages/spec/src/data/hook.test.ts @@ -335,6 +335,85 @@ describe('HookSchema', () => { }); }); + // --------------------------------------------------------------------------- + // [#14010] `runAs` — the declared execution identity of a hook's ctx.api. + // + // Ruling 2026-09-01: `'system' | 'user' | 'inherit'`, default `'inherit'`. + // The `'system'` / `'user'` value semantics are FlowSchema's word for word; + // `'inherit'` is the hook-only third value (only a hook has a context to + // inherit) and is what makes the key purely additive. + // + // Both halves of the reachability judgement are asserted, because they are + // different facts: that the KEY is a real authoring surface (a fixture + // carrying it parses fully green, not merely "no unrecognized_keys"), and + // that the VALUE set is closed (a non-member is a VALUE error located at + // `runAs`, never a top-level unrecognized key). + // --------------------------------------------------------------------------- + describe('runAs (#14010)', () => { + const base = { + name: 'stamp_grade', + object: 'account', + events: ['afterInsert'] as const, + handler: 'stampGrade', + }; + + it('defaults to inherit — the pre-runAs behaviour, so no existing hook changes', () => { + const hook = HookSchema.parse({ ...base }); + expect(hook.runAs).toBe('inherit'); + }); + + it.each(['system', 'user', 'inherit'] as const)('accepts %s, full parse green', (runAs) => { + const result = HookSchema.safeParse({ ...base, runAs }); + expect(result.success, JSON.stringify(result.error?.issues)).toBe(true); + expect(result.data!.runAs).toBe(runAs); + }); + + it('refuses a value outside the enum as a VALUE error at runAs, and prescribes the members', () => { + const result = HookSchema.safeParse({ ...base, runAs: 'elevated' }); + expect(result.success).toBe(false); + const issue = result.error!.issues[0]!; + // Located at the key, not reported as an unknown key at the top level: + // the author wrote a real key with an unreal value, and the two failures + // send them to different places. + expect(issue.code).not.toBe('unrecognized_keys'); + expect(issue.path).toEqual(['runAs']); + const message = JSON.stringify(result.error!.issues); + for (const member of ['system', 'user', 'inherit']) { + expect(message).toContain(member); + } + }); + + it('still refuses unknown keys — widening the accept set by one key widened it by ONE key', () => { + const result = HookSchema.safeParse({ ...base, runAsUser: 'someone' }); + expect(result.success).toBe(false); + expect(result.error!.issues.some((i) => i.code === 'unrecognized_keys')).toBe(true); + }); + + describe('the elevation near-misses are answered with runAs, not with a bare refusal', () => { + // Every one of these is a spelling an author reaches for when they want a + // hook to write past a permission. Before #14010 they were refused with + // nothing to do next; `ctx.api.sudo()` — the shape the platform's own + // lint used to prescribe — is a TypeError in the sandbox (#14044). + it.each(['sudo', 'elevate', 'elevated', 'isSystem'])( + '`%s` is refused with a prescription naming runAs', + (key) => { + const result = HookSchema.safeParse({ ...base, [key]: true }); + expect(result.success).toBe(false); + const message = result.error!.issues.map((i) => i.message).join('\n'); + expect(message).toContain('runAs'); + expect(message).toContain(key); + }, + ); + + it('`run_as` is a RENAME — the spelling changes, the value carries over', () => { + const result = HookSchema.safeParse({ ...base, run_as: 'system' }); + expect(result.success).toBe(false); + const message = result.error!.issues.map((i) => i.message).join('\n'); + expect(message).toContain('runAs'); + }); + }); + }); + describe('Complete Hook Examples', () => { it('should accept validation hook', () => { const hook: Hook = { diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index c4d3ff2bbf..f8153955ed 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -163,6 +163,8 @@ export const HookSchema = lazySchema(() => strictObject( timeoutms: 'timeout', errorpolicy: 'onError', onfailure: 'onError', + // [#14010] `run_as` / `run-as` / `RunAs` all probe-fold to this one entry. + runas: 'runAs', }, guidance: { enabled: @@ -171,6 +173,24 @@ export const HookSchema = lazySchema(() => strictObject( active: '`active` is not a hook key — a hook has no on/off switch. Gate it with `condition`, ' + 'or remove the hook.', + // [#14010] The elevation near-misses. A hook's ONLY declared elevation knob + // is `runAs`; `ctx.api.sudo()` is real on the in-process ScopedContext and + // a TypeError in the sandbox, so every spelling that reaches for it is + // pointed at the key that works on both surfaces. + sudo: + "`sudo` is not a hook key. Declare `runAs: 'system'` to run the hook's `ctx.api` " + + 'data operations elevated (bypassing RLS and field-level write checks); ' + + "`runAs: 'user'` pins them to the triggering user; the default `'inherit'` keeps " + + 'the context of the write that fired the hook.', + elevate: + "`elevate` is not a hook key. Declare `runAs: 'system'` to run the hook's `ctx.api` " + + "data operations elevated, `runAs: 'user'` to pin them to the triggering user.", + elevated: + "`elevated` is not a hook key. Declare `runAs: 'system'` to run the hook's `ctx.api` " + + "data operations elevated, `runAs: 'user'` to pin them to the triggering user.", + isSystem: + "`isSystem` is not a hook key — it is an ExecutionContext flag, not a declaration. " + + "Declare `runAs: 'system'` to run the hook's `ctx.api` data operations elevated.", }, history: 'Until this shape was closed, these were dropped silently — the hook still registered and ran.', }, @@ -313,6 +333,51 @@ export const HookSchema = lazySchema(() => strictObject( */ onError: z.enum(['abort', 'log']).default('abort').describe('Error handling strategy'), + /** + * Execution identity for the hook's `ctx.api` data operations (#14010; + * ruling 2026-09-01). The value semantics of `'system'` / `'user'` are + * FlowSchema's, word for word (`automation/flow.zod.ts` `runAs`): `system` + * elevates (a full-access, RLS-bypassing system principal — the security + * middleware short-circuits before every field-level and row-level gate, so + * a column the triggering persona may not edit is writable through the + * hook), `user` pins the operations to the triggering user, and a `user` + * hook whose trigger resolved NO user has nothing to scope to, so its + * `ctx.api` data operations are REFUSED (`HOOK_UNSCOPED_DATA_ACCESS`, the + * hook-side twin of the flow engine's #3760 refusal) rather than run + * unscoped. + * + * `'inherit'` is the hook-only third value and the default: the hook's + * `ctx.api` carries the context of the write that fired it — exactly the + * pre-`runAs` behaviour, so an existing hook changes nothing. A flow has no + * context to inherit, which is why FlowSchema has no such value and defaults + * to `'user'`; the default differs because the situations differ, not the + * word. ⛔ Do not add `'inherit'` to FlowSchema. + * + * Scope, first cut: `ctx.api` data operations ONLY. `condition` evaluation, + * the `readonly` strip on the hook's own `ctx.input` payload, `ctx.session` + * and the `async` semantics are untouched by this key — the triggering + * operation keeps its own context. Elevation is authorization, not + * anonymity: a `'system'` hook's writes still stamp `updated_by` with the + * triggering user (the engine's audit stamps read `session.userId`, never + * `isSystem`). + * + * Honoured on BOTH execution surfaces — the in-process `handler` and the + * sandboxed `body` — at the one place both are wrapped + * (`packages/objectql/src/hook-wrappers.ts` `wrapDeclarativeHook`). + */ + runAs: z + .enum(['system', 'user', 'inherit']) + .default('inherit') + .describe( + "Execution identity for the hook's ctx.api data operations: system = elevated (bypasses RLS), " + + 'user = the triggering user (RLS-respecting), inherit = the context of the write that fired the hook ' + + '(the pre-runAs behaviour; the default). ' + + 'A hook with no trigger user has no identity to scope to, so under user its ctx.api data operations are REFUSED — ' + + 'declare system to make the elevation explicit. This covers any hook fired by a write that carried no user ' + + '(an isSystem plugin/service write; a system-elevated flow node). ' + + 'Scope: ctx.api only — condition evaluation, the readonly strip on ctx.input, ctx.session and async are unchanged.', + ), + // ADR-0010 — runtime protection envelope (internal — set by the loader). // MISSING until the registered-type invariant test was written: `hook` closed // strict in the #4001 data step without declaring it, so the `_packageId` / diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 9772bf3184..7cdb9bc534 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1676,6 +1676,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/objectql/src/hook-run-as.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/objectql/src/hook-run-as.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/objectql/src/layered-overlay-integration.test.ts", "verb": "delete",