diff --git a/.changeset/sec-hardening-r18-hook-handler-permanent-delete.md b/.changeset/sec-hardening-r18-hook-handler-permanent-delete.md new file mode 100644 index 00000000000..dc7168ab05e --- /dev/null +++ b/.changeset/sec-hardening-r18-hook-handler-permanent-delete.md @@ -0,0 +1,19 @@ +--- +'@objectstack/objectql': patch +'@objectstack/runtime': patch +--- + +fix(objectql,runtime)!: an app-authored hook handler's permanent delete is refused + +Clause-②: yes (narrowing) — the in-process `handler` of a hook bound from metadata is app-authored code, and its `ctx.api` now refuses `delete(…, { permanent })` with any value but `false`, under every `runAs`, on every api derived from it; a handler that passed the key past `HookDeleteOptions` with a cast and purged the record is refused instead. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing on the `ctx.api` an app-authored hook's in-process `handler` holds (`@objectstack/objectql`). + +App-authored code cannot delete a record permanently: its deletes enter the recycle bin, and the permanent delete stays with a principal that holds `manageRecycleBin` (ADR-0148 D3). A sandboxed hook, action or job body was already held to this. A hook bound through the hook binder with an in-process `handler` is app-authored too, and its `ctx.api.object(name).delete(…)` with `permanent` set to anything but `false` now answers `400 VALIDATION_ERROR`, `param: 'permanent'`, with the same sentence a body gets, before anything is removed. It holds under the triggering caller, under `runAs: 'user'` and under `runAs: 'system'`, by id and in bulk, and on the apis `sudo()`, `withRunAs(...)` and a transaction derive. The handler's ordinary delete (no `permanent`, or `permanent: false`) is unchanged and enters the bin. + +Platform code is unchanged: `engine.delete(…, { permanent: true })`, `engine.createContext(...)`, a hook registered in code with `engine.registerHook`, and `purge` / `empty` keep the permanent delete. + +The refusal now lives in `@objectstack/objectql` (`refuseAppAuthoredPermanentDelete`), and the runtime's sandbox applies that same refusal to every body (`@objectstack/runtime`). It reads the delete's options once, into a copy, and the delete receives that copy. diff --git a/packages/objectql/src/engine-scoped-context.ts b/packages/objectql/src/engine-scoped-context.ts index 9f65bc826ef..91de02fe0e9 100644 --- a/packages/objectql/src/engine-scoped-context.ts +++ b/packages/objectql/src/engine-scoped-context.ts @@ -34,6 +34,7 @@ import { driverSupportsTransactions } from '@objectstack/spec/data'; import type { IScopedContext } from '@objectstack/spec/contracts'; import { appAutomationElevationOf, markAppAutomationElevation } from '@objectstack/core'; import { UnscopedHookApi } from './hook-run-as.js'; +import { refuseAppAuthoredPermanentDelete } from './recycle-bin/app-authored-delete.js'; import type { HookRunAs, HookRunAsRef } from './hook-run-as.js'; import { ObjectRepository } from './engine.js'; import type { ObjectQL, ScopedContext } from './engine.js'; @@ -58,8 +59,13 @@ export interface ScopedContextHost { /** The bodies of `ScopedContext`'s methods: a stateless facet, called by the class's stubs. */ export class ScopedContextFacet { /** The body of `ScopedContext#object`; the method's docblock there is its contract. */ - static object(this: Pick, name: string): ObjectRepository { - return new ObjectRepository(name, this.executionContext, this.engine as any); + static object(this: Pick, name: string): ObjectRepository { + const repo = new ObjectRepository(name, this.executionContext, this.engine as any); + // [ADR-0148 D3, ruling A recorded in 6691358077] An app-authored hook's + // api (and every api derived from it) refuses a permanent delete: the same + // refusal a sandboxed body's api applies, here for the hook's in-process + // `handler`, whatever its `runAs`. + return this.appAuthored ? refuseAppAuthoredPermanentDelete(name, repo, 'hook handler') : repo; } /** The body of `ScopedContext#sudo`; the method's docblock there is its contract. */ diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 455ca65d6ca..d648147871a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -10617,7 +10617,7 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi { /** * [#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}. + * ({@link derive}); read by {@link sudo} and {@link object}. */ private appAuthored = false; @@ -10628,7 +10628,16 @@ export class ScopedContext implements IScopedContext, RunAsDerivableApi { return child; } - /** Get a repository scoped to this context */ + /** + * Get a repository scoped to this context. + * + * [ADR-0148 D3, ruling A recorded in 6691358077] On an app-authored hook's + * api ({@link APP_AUTHORED_HOOK_API}) and every api derived from it, the + * repository's `delete(…, { permanent })` is refused (`400 + * VALIDATION_ERROR`, `param: 'permanent'`): app-authored code cannot purge, + * so its deletes enter the recycle bin, under any `runAs`. Platform code's + * api is not app-authored, and keeps the engine's permanent delete. + */ object(name: string): ObjectRepository { // [#30052 codemod] The body moved to `engine-scoped-context.ts` (see `ScopedContextHost`). return ScopedContextFacet.object.call(scopedContextHost.of(this), name); diff --git a/packages/objectql/src/hook-run-as.ts b/packages/objectql/src/hook-run-as.ts index 74e3f1210d0..c1125b2be1f 100644 --- a/packages/objectql/src/hook-run-as.ts +++ b/packages/objectql/src/hook-run-as.ts @@ -264,6 +264,12 @@ export async function seedHookUserGrants(api: unknown): Promise { * - its `sudo()`, on it and on every api derived from it (`transaction`, * `withRunAs`), marks the elevation it makes. * + * [ADR-0148 D3, ruling A recorded in 6691358077] And every repository it and + * the apis derived from it serve refuses a permanent delete + * (`refuseAppAuthoredPermanentDelete`, `recycle-bin/app-authored-delete.ts`): + * a hook's in-process `handler` is app-authored code, so its deletes enter the + * recycle bin, under the caller and under `runAs: 'system'` alike. + * * A symbol, and ⛔ not re-exported from the package entry, for the reason * {@link SEED_USER_GRANTS} gives. */ diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index a47a7b98ccd..caa3dbaf823 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -633,6 +633,10 @@ export { parseLifecycleDuration } from './lifecycle/duration.js'; // [#30237, ADR-0148 D3] What a host hands `ObjectQLPlugin`'s `recycleBinRetention`. export type { RecycleBinRetentionSettings } from './recycle-bin/retention.js'; export type { RecycleBinRemovalOptions } from './recycle-bin/purge.js'; +// [ADR-0148 D3, ruling A recorded in 6691358077] The one refusal of an +// app-authored permanent delete: the engine applies it to an app-authored +// hook's api, and the runtime's sandbox to every body's. +export { refuseAppAuthoredPermanentDelete } from './recycle-bin/app-authored-delete.js'; export { lifecycleSettingsManifest } from './lifecycle/lifecycle-settings.js'; // [#4551] Read-only referential-integrity audit — the reporting half of the diff --git a/packages/objectql/src/recycle-bin/app-authored-delete.ts b/packages/objectql/src/recycle-bin/app-authored-delete.ts new file mode 100644 index 00000000000..7aa3fa4ef5a --- /dev/null +++ b/packages/objectql/src/recycle-bin/app-authored-delete.ts @@ -0,0 +1,87 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0148 D3, ruling A recorded in 6691358077] App-authored code cannot + * delete a record permanently: its deletes always enter the recycle bin, and + * the permanent delete stays with a principal that holds `manageRecycleBin`. + * + * This module is the ONE refusal, held where every face that hands app code a + * scoped API can reach it: + * + * - a sandboxed body (hook, action, job): `@objectstack/runtime`'s + * `buildSandboxApi` wraps every repository the body's API serves; + * - an app-authored hook's in-process `handler` (a hook bound through the + * hook binder, `APP_AUTHORED_HOOK_API` in `hook-run-as.ts`): the engine's + * own `ScopedContext.object()` wraps every repository of an app-authored + * api, and of every api derived from it (`sudo()`, `withRunAs(...)`, a + * transaction's context), so it holds under the caller and under + * `runAs: 'system'` alike. + * + * The engine's `delete(…, { permanent: true })` is unchanged for every other + * caller (Q3 = A): platform code, a hook registered in code with + * `engine.registerHook`, and `engine.createContext(...)`. + */ + +/** + * The refusal of an app-authored `delete(…, { permanent })`: `400 + * VALIDATION_ERROR`, `param: 'permanent'`. The message names the rule, because + * code that asked for a purge has to learn it got none: dropping the key would + * delete into the bin and answer success, which reads as the purge it asked for. + * + * `face` names what asked (`'hook body'`, `'job body'`, `'hook handler'`, …). + */ +function appAuthoredPermanentDeleteRefusal(objectName: string, face: string): Error { + const err: any = new Error( + `A ${face} cannot delete a record permanently: app-authored code cannot purge; ` + + `its deletes enter the recycle bin. Remove 'permanent' from the delete of '${objectName}'.`, + ); + err.developerMessage = + `'permanent' was passed to ctx.api.object('${objectName}').delete() in a ${face}. App-authored code ` + + `(hook, action and job bodies, and the in-process handler of a hook bound from metadata) cannot erase a ` + + `record past the recycle bin, under any execution context, a job's system envelope and a hook's ` + + `runAs: 'system' included; the permanent delete stays with a principal holding manageRecycleBin ` + + `(ADR-0148 D3).`; + err.code = 'VALIDATION_ERROR'; + err.status = 400; + err.httpStatus = 400; + err.param = 'permanent'; + err.object = objectName; + return err; +} + +/** + * One repository as app-authored code holds it: `delete(options)` whose + * `options` carry a `permanent` key with any value but `false` is refused + * before the underlying verb is called, so nothing is removed and no bin entry + * is skipped. `false` and an absent key are the ordinary delete, and go + * through. Every other member is handed through. + * + * The options are read ONCE, into a shallow copy, and that copy is what the + * verb receives: the key judged is the key forwarded. An in-process caller can + * hand an object whose `permanent` answers differently on a second read (a + * getter, a Proxy) or is inherited rather than own; judging one read and + * forwarding the original would let the verb see another value than the one + * judged. A sandboxed body's options arrive as plain JSON, for which the copy + * is the same value. + */ +export function refuseAppAuthoredPermanentDelete(objectName: string, repo: T, face: string): T { + if (repo === null || typeof repo !== 'object') return repo; + return new Proxy(repo as unknown as Record, { + get(target, prop) { + const value = Reflect.get(target, prop, target); + if (typeof value !== 'function') return value; + if (prop !== 'delete') return value.bind(target); + return async (...args: unknown[]) => { + const [options, ...rest] = args; + if (options === null || (typeof options !== 'object' && typeof options !== 'function')) { + return value.apply(target, args); + } + const judged: Record = { ...(options as Record) }; + if (judged.permanent !== undefined && judged.permanent !== false) { + throw appAuthoredPermanentDeleteRefusal(objectName, face); + } + return value.apply(target, [judged, ...rest]); + }; + }, + }) as unknown as T; +} diff --git a/packages/objectql/src/recycle-bin/purge.test.ts b/packages/objectql/src/recycle-bin/purge.test.ts index 49a014eeef9..1026bc110ed 100644 --- a/packages/objectql/src/recycle-bin/purge.test.ts +++ b/packages/objectql/src/recycle-bin/purge.test.ts @@ -15,6 +15,7 @@ import { describe, it, expect } from 'vitest'; import { RECYCLE_BIN_ENTRY_FIELDS } from '@objectstack/spec/data'; import { ObjectQL } from '../engine.js'; +import { bindHooksToEngine } from '../hook-binder.js'; type Row = Record; @@ -412,3 +413,130 @@ describe('empty(object)', () => { expect(dispatches).toEqual([]); }); }); + +/** + * [ADR-0148 D3, ruling A recorded in 6691358077] A hook bound from metadata + * (through the hook binder) is app-authored code, and so is its in-process + * `handler`: its `ctx.api` refuses a permanent delete as a sandboxed body's + * does, with the same code and sentence, under the caller and under + * `runAs: 'system'`, by id and in bulk, on every api derived from it. Its + * ordinary delete still enters the bin; platform code keeps the permanent + * delete. + */ +describe("an app-authored hook handler's permanent delete (ruling A)", () => { + const RULE_SENTENCE = + 'A hook handler cannot delete a record permanently: app-authored code cannot purge; its deletes enter the recycle bin.'; + + /** + * Bind one hook through the binder, on `account`'s `beforeInsert`, whose + * handler runs `use(ctx.api)` and keeps what it answered or threw; then fire + * it once with an insert by `ORG1_USER`. + */ + async function runHandler( + engine: ObjectQL, + runAs: 'inherit' | 'user' | 'system', + use: (api: any) => Promise, + ): Promise<{ value?: unknown; error?: any }> { + const seen: { value?: unknown; error?: any } = {}; + const bound = bindHooksToEngine(engine, [{ + name: 'purge_on_create', + object: 'account', + events: ['beforeInsert'], + runAs, + handler: async (ctx: any) => { + try { seen.value = await use(ctx.api); } catch (e) { seen.error = e; } + }, + } as any], { packageId: 'app_under_test' }); + expect(bound).toMatchObject({ registered: 1, errors: [] }); + await engine.insert('account', { id: 'acc_trigger', name: 'Trigger', organization_id: 'org_1' }, ORG1_USER); + return seen; + } + + const lonerDispatches = (dispatches: Dispatch[]) => dispatches.filter((d) => d.object === 'loner'); + + for (const runAs of ['inherit', 'user', 'system'] as const) { + for (const [shape, options] of [ + ['by id', { where: { id: 'l1' }, permanent: true }], + ['in bulk', { where: { name: 'one' }, multi: true, permanent: true }], + ] as const) { + it(`runAs '${runAs}': \`delete(…, { permanent: true })\` ${shape} is refused 400 naming the rule; nothing is removed`, async () => { + const { engine, driver, dispatches } = await makeEngine(); + driver.seed('loner', { id: 'l1', name: 'one' }); + + const { error } = await runHandler(engine, runAs, (api) => api.object('loner').delete(options)); + + expect(error).toMatchObject({ code: 'VALIDATION_ERROR', status: 400, param: 'permanent', object: 'loner' }); + expect(String(error?.message).startsWith(RULE_SENTENCE)).toBe(true); + expect(lonerDispatches(dispatches)).toEqual([]); + expect(driver.rowsOf('loner').map((r: Row) => r.id)).toEqual(['l1']); + expect(entriesOf(driver)).toHaveLength(0); + }); + } + } + + for (const [derivation, use] of [ + ['`sudo()`', (api: any) => api.sudo().object('loner').delete({ where: { id: 'l1' }, permanent: true })], + ['`transaction(cb)`', (api: any) => api.transaction((trx: any) => trx.object('loner').delete({ where: { id: 'l1' }, permanent: true }))], + ] as const) { + it(`is refused the same way on the api ${derivation} derives`, async () => { + const { engine, driver, dispatches } = await makeEngine(); + driver.seed('loner', { id: 'l1', name: 'one' }); + + const { error } = await runHandler(engine, 'inherit', use); + + expect(error).toMatchObject({ code: 'VALIDATION_ERROR', status: 400, param: 'permanent', object: 'loner' }); + expect(String(error?.message).startsWith(RULE_SENTENCE)).toBe(true); + expect(lonerDispatches(dispatches)).toEqual([]); + expect(driver.rowsOf('loner').map((r: Row) => r.id)).toEqual(['l1']); + }); + } + + it('judges the options it forwards: a `permanent` that answers differently on a second read is read once', async () => { + const { engine, driver, dispatches } = await makeEngine(); + driver.seed('loner', { id: 'l1', name: 'one' }); + let reads = 0; + const options = { where: { id: 'l1' }, get permanent() { reads += 1; return reads > 1; } }; + + const { error } = await runHandler(engine, 'system', (api) => api.object('loner').delete(options)); + + expect(error).toBeUndefined(); + expect(reads).toBe(1); + expect(lonerDispatches(dispatches).map((d) => d.operation)).toEqual(['delete']); + expect(entriesOf(driver).map((e) => e.record_id)).toEqual(['l1']); + }); + + for (const runAs of ['inherit', 'system'] as const) { + for (const [label, options] of [ + ['no `permanent` key', { where: { id: 'l1' } }], + ['`permanent: false`', { where: { id: 'l1' }, permanent: false }], + ] as const) { + it(`runAs '${runAs}': the handler's delete with ${label} is the ordinary delete, which enters the bin`, async () => { + const { engine, driver, dispatches } = await makeEngine(); + driver.seed('loner', { id: 'l1', name: 'one' }); + + const { error } = await runHandler(engine, runAs, (api) => api.object('loner').delete(options)); + + expect(error).toBeUndefined(); + expect(lonerDispatches(dispatches).map((d) => d.operation)).toEqual(['delete']); + expect(driver.rowsOf('loner')).toEqual([]); + expect(entriesOf(driver).map((e) => e.record_id)).toEqual(['l1']); + }); + } + } + + it("platform code keeps the permanent delete: a hook registered in code, and the engine's own context", async () => { + const { engine, driver, dispatches } = await makeEngine(); + driver.seed('loner', { id: 'l1', name: 'one' }); + driver.seed('loner', { id: 'l2', name: 'two' }); + engine.registerHook('beforeInsert', async (ctx: any) => { + await ctx.api.object('loner').delete({ where: { id: 'l1' }, permanent: true }); + }, { object: 'account' }); + + await engine.insert('account', { id: 'acc_trigger', name: 'Trigger', organization_id: 'org_1' }, ORG1_USER); + await engine.createContext({ isSystem: true }).object('loner').delete({ where: { id: 'l2' }, permanent: true }); + + expect(lonerDispatches(dispatches).map((d) => d.operation)).toEqual(['purge', 'purge']); + expect(driver.rowsOf('loner')).toEqual([]); + expect(entriesOf(driver)).toHaveLength(0); + }); +}); diff --git a/packages/runtime/src/sandbox/body-runner.ts b/packages/runtime/src/sandbox/body-runner.ts index 0a33b238c2c..ed6e5218e5e 100644 --- a/packages/runtime/src/sandbox/body-runner.ts +++ b/packages/runtime/src/sandbox/body-runner.ts @@ -62,6 +62,7 @@ import { resolveRelatedTitleTarget, resolveServedRecordTitle, servedRecordTitle, + refuseAppAuthoredPermanentDelete, type FormulaServingChannel, } from '@objectstack/objectql'; import { deriveThroughSeam, refuseStoredMetadataBodyReads, refuseStoredMetadataBodyWrites } from '../stored-metadata-reader-seam.js'; @@ -1181,58 +1182,6 @@ function buildSandboxApi(engineCtx: any, ql: any, errLabel: string) { /** Marks a scoped API whose permanent deletes are already refused for a body. */ const BODY_PERMANENT_DELETES_REFUSED = Symbol.for('objectstack.runtime.bodyPermanentDeletesRefused'); -/** - * The refusal of a body's `delete(…, { permanent })`: `400 VALIDATION_ERROR`, - * `param: 'permanent'`. The message names the rule, because a job that asked - * for a purge has to learn it got none: dropping the key would delete into the - * bin and answer success, which reads as the purge it asked for. - */ -function bodyPermanentDeleteRefusal(objectName: string, errLabel: string): Error { - const err: any = new Error( - `A ${errLabel} cannot delete a record permanently: app-authored code cannot purge; ` + - `its deletes enter the recycle bin. Remove 'permanent' from the delete of '${objectName}'.`, - ); - err.developerMessage = - `'permanent' was passed to ctx.api.object('${objectName}').delete() in a ${errLabel}. App-authored code ` + - `(hook, action and job bodies) cannot erase a record past the recycle bin, under any execution context, ` + - `a job's system envelope included; the permanent delete stays with a principal holding manageRecycleBin ` + - `(ADR-0148 D3).`; - err.code = 'VALIDATION_ERROR'; - err.status = 400; - err.httpStatus = 400; - err.param = 'permanent'; - err.object = objectName; - return err; -} - -/** - * One repository as a body holds it: `delete(options)` whose `options` carry a - * `permanent` key with any value but `false` is refused before the underlying - * verb is called, so nothing is removed and no bin entry is skipped. `false` - * and an absent key are the ordinary delete, and go through unchanged. Every - * other member is handed through. - */ -function refuseRepositoryPermanentDelete(objectName: string, repo: unknown, errLabel: string): unknown { - if (repo === null || typeof repo !== 'object') return repo; - return new Proxy(repo as Record, { - get(target, prop) { - const value = Reflect.get(target, prop, target); - if (typeof value !== 'function') return value; - if (prop !== 'delete') return value.bind(target); - return async (...args: unknown[]) => { - const options = args[0]; - if (options !== null && typeof options === 'object') { - const permanent = (options as { permanent?: unknown }).permanent; - if (permanent !== undefined && permanent !== false) { - throw bodyPermanentDeleteRefusal(objectName, errLabel); - } - } - return value.apply(target, args); - }; - }, - }); -} - /** * [ADR-0148 D3, ruling A recorded in 6691358077] The scoped API a sandboxed * BODY (a hook, action or job body) holds, with the permanent delete refused: @@ -1240,14 +1189,19 @@ function refuseRepositoryPermanentDelete(objectName: string, repo: unknown, errL * recycle bin. A job body runs under a fresh `{ isSystem: true, tenantId }` * envelope and the system is never refused the engine's `purge` verb, so the * rule is held here, at the one place every body face gets its API, and not by - * the engine, whose `delete(…, { permanent: true })` stays open to every other - * caller. Every context the API derives (`object(name)`, `sudo()`, - * `withRunAs(...)`, a transaction's context) is refused the same way. - * Idempotent. + * the engine's `delete`, whose `delete(…, { permanent: true })` stays open to + * every caller that is not app-authored. Every context the API derives + * (`object(name)`, `sudo()`, `withRunAs(...)`, a transaction's context) is + * refused the same way. Idempotent. + * + * The refusal itself is objectql's `refuseAppAuthoredPermanentDelete`, the + * same one the engine's `ScopedContext.object()` applies to an app-authored + * hook's api, so a hook's in-process `handler` and every body are refused with + * one code and one sentence. */ function refuseBodyPermanentDeletes(api: T, errLabel: string): T { return deriveThroughSeam(api, BODY_PERMANENT_DELETES_REFUSED, (name, repo) => - refuseRepositoryPermanentDelete(name, repo, errLabel), + refuseAppAuthoredPermanentDelete(name, repo, errLabel), ); }