fix(security)!: an app-authored hook's inherited elevation, its sudo() and an action it reaches by execute() write no identity assignment

.changeset/25653-app-hook-inherited-elevation-identity-write-refusal.md

@@ -0,0 +1,23 @@
+---
+'@objectstack/objectql': patch
+'@objectstack/plugin-security': patch
+---
+
+fix(security): an app-authored hook that inherits a system context, its `ctx.api.sudo()`, and an action it reaches through `execute()` are refused writes to position holdings, permission-set grants, invitations and memberships
+
+Clause-②: no (narrowing: an app-authored hook's inherited or sudo() elevation, and an action reached through ObjectRepository.execute(), are refused the four identity objects at run time, as a runAs: 'system' hook already is)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any metadata schema is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. What changes is the run-time answer to an app-authored hook's elevated write to four platform objects. -->
+
+**BREAKING** (a served-set narrowing) for an app-authored hook that writes `sys_user_position`, `sys_user_permission_set`, `sys_invitation` or `sys_member` while elevated. A census of the hooks in this repository and in HotCRM found none that does.
+
+Since the previous release, a hook that declares `runAs: 'system'` and an action body are refused these writes. Three other ways app-authored code ran elevated were not:
+
+- a hook that declares no `runAs` and fires on a write made under a system context, which it inherits;
+- a hook's in-process `ctx.api.sudo()`;
+- an action reached through `ctx.api.object(name).execute(action)`.
+
+Each is now refused the same way: `403 PERMISSION_DENIED`, nothing written, and under the default `onError: 'abort'` the triggering write aborts. The message names the hook and the fix: set its `runAs` to `'user'` and leave `ctx.api.sudo()` out, or use the organization endpoints for invitations and memberships.
+
+A hook is app-authored when it is bound from metadata through the hook binder (the same hooks the `skipAutomations` opt-out skips). A hook a plugin registers in code with `engine.registerHook`, and platform code's own `sudo()`, keep writing these objects. Reads of the four objects, and writes to every other object, are unchanged.

content/docs/permissions/system-context.mdx

@@ -97,7 +97,7 @@ that silently does not happen.
 | # | Behaviour when `isSystem` | Package | What you get / what you lose | Anchor |
 |:--|:---|:---|:---|:---|
 | 1 | **The whole security middleware short-circuits** before any gate runs | plugin-security | Get: every CRUD/FLS/tenant/owner gate below skipped in one branch. Lose: every gate under it in this lane at once, down to and including the write-bypass row that widens the effective write scope to `org` (row 7) — this is the single largest behaviour on the page | `packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts#registerAuthzMiddleware` |
-| 1b | **App-authored automation's system context is REFUSED governed writes to the identity-assignment objects** — a flow run's, a `runAs: 'system'` hook's and an action body's; the one check ahead of the middleware's short-circuit | plugin-security | Get: nothing — an insert, update, delete, transfer, restore or purge of `sys_user_position`, `sys_user_permission_set`, `sys_invitation` or `sys_member` from a context carrying `isSystem` and either a `flowRunId` (a `runAs: 'system'` flow's data nodes) or the app-automation elevation mark (`@objectstack/core`; set only by a hook's `runAs: 'system'` derivation and by the action body's execution-context builder), at any depth of their writes, is refused `PERMISSION_DENIED` before the short-circuit skips the ADR-0090 D12 gate and the identity write guard. Lose: nothing a platform writer held — a system context with neither (the seeders, better-auth and its reconciliation, invitation placement, any `sudo()`, which never sets the mark) passes on to the short-circuit unchanged, reads are not judged, and a `runAs: 'user'` flow or hook carries no `isSystem`, so the D12 gate judges it as the user it acts for | `packages/plugins/plugin-security/src/flow-system-identity-write-refusal.ts#elevationSource` |
+| 1b | **App-authored automation's system context is REFUSED governed writes to the identity-assignment objects** — a flow run's, an app-authored hook's (`runAs: 'system'`, a system context it inherits, its `sudo()`) and an action body's; the one check ahead of the middleware's short-circuit | plugin-security | Get: nothing — an insert, update, delete, transfer, restore or purge of `sys_user_position`, `sys_user_permission_set`, `sys_invitation` or `sys_member` from a context carrying `isSystem` and either a `flowRunId` (a `runAs: 'system'` flow's data nodes) or the app-automation elevation mark (`@objectstack/core`; set only by a hook's `runAs: 'system'` derivation, by the action body's execution-context builder and `ObjectRepository.execute`, and on the api of a hook bound through the hook binder, whose inherited system context and `sudo()` it marks), at any depth of their writes, is refused `PERMISSION_DENIED` before the short-circuit skips the ADR-0090 D12 gate and the identity write guard. Lose: nothing a platform writer held — a system context with neither (the seeders, better-auth and its reconciliation, invitation placement, platform code's `sudo()`, which never sets the mark, and a hook registered in code) passes on to the short-circuit unchanged, reads are not judged, and a `runAs: 'user'` flow or hook carries no `isSystem`, so the D12 gate judges it as the user it acts for | `packages/plugins/plugin-security/src/flow-system-identity-write-refusal.ts#elevationSource`, `packages/objectql/src/engine.ts#inheritsElevation` |
 | 2 | **`owner_id` is not auto-stamped on INSERT** (the step 3.5 anchor guard is inside the block row 1 skips) | plugin-security | Lose: the row lands `owner_id = NULL`, so the default `owner_only_writes` policy hides it **from its own creator**. Get: nothing — this is a gap, not a capability | the step 3.5 guard block and the short-circuit that skips it are both inside `packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts#registerAuthzMiddleware` |
 | 3 | Row-level read filter resolves to "no filter" | plugin-security | Get: unscoped reads. Lose: row-level scoping entirely | `packages/plugins/plugin-security/src/security-plugin-read-filter.ts#getReadFilter` |
 | 4 | Field-level security returns **all** fields | plugin-security | Get: every column readable (`getReadableFields`) and writable (`getWritableFields`). Lose: field masking | `packages/plugins/plugin-security/src/security-plugin-field-readers.ts#resolveProjectionFieldMask` |

packages/objectql/src/engine.ts

@@ -153,9 +153,12 @@ import {
   resolveUserAuthzGrants,
   // [#25653] The app-automation elevation mark: set by a hook's
   // `runAs: 'system'` derivation, carried across `createContext`'s parse —
-  // see `ScopedContext.withRunAs`.
+  // see `ScopedContext.withRunAs` — and by an app-authored hook's inherited
+  // elevation and `sudo()`, and `ObjectRepository.execute` — see
+  // `ScopedContext[APP_AUTHORED_HOOK_API]`.
   markAppAutomationElevation,
   carryAppAutomationElevation,
+  appAutomationElevationOf,
 } from '@objectstack/core';
 import { WriteEpoch, isWriteEpochOperation } from './write-epoch.js';
 import { bridgeAuthzInvalidation } from './authz-invalidation-bridge.js';
@@ -312,7 +315,7 @@ import {
   divergingHookPayloadKeys,
   MultiUpdateHookKeyDivergenceError,
 } from './multi-update-hook-key-divergence.js';
-import { UnscopedHookApi, SEED_USER_GRANTS, type HookRunAs, type HookRunAsRef, type RunAsDerivableApi } from './hook-run-as.js';
+import { UnscopedHookApi, SEED_USER_GRANTS, APP_AUTHORED_HOOK_API, 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';
 // [#19911] The settlement's question "does judging the FK's own lock need the
@@ -22957,7 +22960,14 @@ export class ObjectRepository implements IScopedObjectRepository {
    */
   async execute(actionName: string, params?: any): Promise<any> {
     if (this.engine.executeAction) {
-      const executionContext: ExecutionContext = { ...this.context, isSystem: true };
+      // [#25653] With the `'action'` mark, as `buildActionExecutionContext`
+      // (runtime) gives the REST and MCP doors' action bodies: the action this
+      // reaches is app-authored automation running elevated, so its writes to
+      // the four identity-assignment objects are refused (plugin-security).
+      const executionContext: ExecutionContext = markAppAutomationElevation(
+        { ...this.context, isSystem: true },
+        'action',
+      );
       return this.engine.executeAction(this.objectName, actionName, {
         ...params,
         userId: this.context.userId,
@@ -22990,19 +23000,62 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
     private engine: IDataEngine
   ) {}
 
+  /**
+   * [#25653] Set on the api an app-authored hook is handed
+   * ({@link APP_AUTHORED_HOOK_API}) and kept by every api derived from it
+   * ({@link derive}); read by {@link sudo}.
+   */
+  private appAuthored = false;
+
+  /** A child over `executionContext` that keeps this api's provenance. */
+  private derive(executionContext: ExecutionContext): ScopedContext {
+    const child = new ScopedContext(executionContext, this.engine);
+    child.appAuthored = this.appAuthored;
+    return child;
+  }
+
   /** Get a repository scoped to this context */
   object(name: string): ObjectRepository {
     return new ObjectRepository(name, this.executionContext, this.engine as any);
   }
 
-  /** Create an elevated (system) context */
+  /**
+   * Create an elevated (system) context.
+   *
+   * [#25653] Platform code's `sudo()` carries no app-automation mark, so its
+   * writes to the four identity-assignment objects land. An app-authored
+   * hook's in-process `ctx.api.sudo()` is the hook's elevation: it carries
+   * the `'hook'` mark (unless the context already carries one).
+   */
   sudo(): ScopedContext {
-    return new ScopedContext(
-      { ...this.executionContext, isSystem: true },
-      this.engine
+    const elevated = { ...this.executionContext, isSystem: true };
+    return this.derive(
+      this.appAuthored && !appAutomationElevationOf(elevated)
+        ? markAppAutomationElevation(elevated, 'hook')
+        : elevated,
     );
   }
 
+  /**
+   * [#25653] The api an app-authored hook runs with — see
+   * {@link APP_AUTHORED_HOOK_API} (`hook-run-as.ts`), where the line between
+   * app-authored and platform hooks is drawn. Derived by `wrapDeclarativeHook`
+   * for a hook the binder bound, before its `runAs` is applied.
+   *
+   * An inherited system context with no `flowRunId` and no mark gets the
+   * `'hook'` mark: the hook runs with that elevation, so its writes to the four
+   * identity-assignment objects are refused as a `runAs: 'system'` hook's are.
+   * A run's context (`flowRunId`) is already judged as the run's; a marked one
+   * keeps its mark; a user's context is not elevated and needs none.
+   */
+  [APP_AUTHORED_HOOK_API](): ScopedContext {
+    const ctx = this.executionContext;
+    const inheritsElevation = ctx.isSystem && !ctx.flowRunId && !appAutomationElevationOf(ctx);
+    const api = new ScopedContext(inheritsElevation ? markAppAutomationElevation(ctx, 'hook') : ctx, this.engine);
+    api.appAuthored = true;
+    return api;
+  }
+
   /**
    * [#14010] The api a hook that declared `runAs` is handed — derived from the
    * engine-built one (`buildHookApi(opCtx.context)`) at dispatch, by
@@ -23031,16 +23084,12 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
    */
   withRunAs(runAs: Exclude<HookRunAs, 'inherit'>, ref: HookRunAsRef): IScopedContext {
     if (runAs === 'system') {
-      return new ScopedContext(
+      return this.derive(
         markAppAutomationElevation({ ...this.executionContext, isSystem: true }, 'hook'),
-        this.engine
       );
     }
     if (!this.executionContext.userId) return new UnscopedHookApi(ref);
-    return new ScopedContext(
-      { ...this.executionContext, isSystem: false },
-      this.engine
-    );
+    return this.derive({ ...this.executionContext, isSystem: false });
   }
 
   /**
@@ -23108,15 +23157,12 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
       ctx.userId,
       ctx.tenantId ? { tenantId: ctx.tenantId } : {},
     );
-    return new ScopedContext(
-      {
-        ...ctx,
-        ...(needsGrants ? { positions: grants.positions, permissions: grants.permissions } : {}),
-        ...(needsPeers ? { org_user_ids: grants.org_user_ids } : {}),
-        ...(needsEmail && grants.email ? { email: grants.email } : {}),
-      },
-      this.engine
-    );
+    return this.derive({
+      ...ctx,
+      ...(needsGrants ? { positions: grants.positions, permissions: grants.permissions } : {}),
+      ...(needsPeers ? { org_user_ids: grants.org_user_ids } : {}),
+      ...(needsEmail && grants.email ? { email: grants.email } : {}),
+    });
   }
 
   /**
@@ -23201,10 +23247,7 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
       // boundaries the store does not. It is identity-equal to the store's
       // handle, so `transactionCoversDriverFor` still attributes it to the
       // OUTER owner and the #5351 same-origin gate judges it unchanged.
-      const joinedCtx = new ScopedContext(
-        { ...this.executionContext, transaction: ambient.transaction },
-        this.engine
-      );
+      const joinedCtx = this.derive({ ...this.executionContext, transaction: ambient.transaction });
       // JOINED, not owned: the outer caller decides commit vs rollback (#5696).
       return callback(joinedCtx, { owned: false });
     }
@@ -23230,10 +23273,7 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
     }
 
     const trx = await driver.beginTransaction();
-    const trxCtx = new ScopedContext(
-      { ...this.executionContext, transaction: trx },
-      this.engine
-    );
+    const trxCtx = this.derive({ ...this.executionContext, transaction: trx });
     // Publish this transaction into the engine's ambient store so internal
     // queries during writes reuse its connection (ADR-0034) — and so a nested
     // `transaction()` on either surface can JOIN it. The store entry also
@@ -23384,20 +23424,14 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi {
       // store's handle — so `buildDriverOptions` binds every op on it to the
       // outer connection and `transactionCoversDriverFor` still attributes the
       // handle to the OUTER owner (#5351 unchanged).
-      const ctx = new ScopedContext(
-        { ...this.executionContext, transaction: ambient.transaction },
-        this.engine
-      );
+      const ctx = this.derive({ ...this.executionContext, transaction: ambient.transaction });
       this.joinedHandles.add(ambient.transaction);
       return { ctx, handle: ambient.transaction, owned: false };
     }
     const driver = this.txDriver();
     if (!driver) return null;
     const trx = await driver.beginTransaction();
-    const ctx = new ScopedContext(
-      { ...this.executionContext, transaction: trx },
-      this.engine
-    );
+    const ctx = this.derive({ ...this.executionContext, transaction: trx });
     return { ctx, handle: trx, owned: true };
   }
 

packages/objectql/src/hook-binder.ts

@@ -312,7 +312,11 @@ export function bindHooksToEngine(
         continue;
       }
 
-      const wrapped = wrapDeclarativeHook(hook, resolved, { logger, metrics: opts.metrics });
+      // [#25653] `appAuthored`: every hook this binder binds is app-authored —
+      // the same set the `skipAutomations` opt-out keys on through `meta`
+      // below — so its inherited elevation and its `sudo()` carry the
+      // app-automation mark (`APP_AUTHORED_HOOK_API`, `hook-run-as.ts`).
+      const wrapped = wrapDeclarativeHook(hook, resolved, { logger, metrics: opts.metrics, appAuthored: true });
       const events = Array.isArray(hook.events) ? hook.events : [];
 
       for (const event of events) {