diff --git a/.changeset/24662-hook-condition-coded-refusal.md b/.changeset/24662-hook-condition-coded-refusal.md new file mode 100644 index 00000000000..ad481dc3d74 --- /dev/null +++ b/.changeset/24662-hook-condition-coded-refusal.md @@ -0,0 +1,12 @@ +--- +'@objectstack/types': patch +--- + +fix(types): a write an authored hook condition refused because it could not be evaluated answers a coded `400 VALIDATION_ERROR`, not the unclassified `500 INTERNAL_ERROR` + +Clause-②: no (the write is refused before and after, with nothing stored; only the refusal's status and code move, onto the published `StandardErrorCode` member `VALIDATION_ERROR`; no accept set moves and no code is added) + +- **What changed on the wire.** A `HookConditionError` whose condition compiled and then faulted on the record it judged (`reason: 'unevaluable'`) is now a refusal at every data door: `400`, code `VALIDATION_ERROR`, and the hook's own message, which names the hook and the CEL fault and never the condition source or a record value. Before, it matched nothing and was answered `500 INTERNAL_ERROR` — on `@objectstack/rest`'s doors with a `[REST] Unhandled error` line, and on the runtime dispatcher's `/data` door (through `@objectstack/hono`'s catch-all) with the hook's message at status 500. A `/batch` row for the same throw now reads `VALIDATION_ERROR` with `httpStatus: 400` and the hook's message, where it read `INTERNAL_ERROR` with a withheld message. +- **Why `400` and `VALIDATION_ERROR`.** The status is the one a validation rule that cannot be evaluated answers, and the two share one message builder. The code is the catalog member `400` names, because a hook's gate addresses no field and the per-field `VALIDATION_FAILED` envelope does not fit it. +- **The log.** The refusal is handed to the caller, so `@objectstack/rest` no longer prints `[REST] Unhandled error` for it. Its log verdict reads the declaration from `resolveThrownHttpError`, as it does for any declared client refusal. +- **Unchanged.** A condition that never compiled (`reason: 'uncompilable'`) keeps its sanitised `500` and its log line. That fault does not depend on the request, and its message quotes the condition source. A hook that refuses on purpose answers exactly as before. diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index 2f74269af70..61b538a22c1 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -115,7 +115,10 @@ condition" carry *opposite* risks depending on the hook: swallowed into a `befor guard it silently lets the write through; swallowed into an audit hook it silently drops the record. They are now distinct outcomes, and the second **fails the write**. Hooks that have been getting by on that skip will start failing — that is how you -find out they were never enforcing anything. +find out they were never enforcing anything. Over the API the refused write answers +`400` with the code `VALIDATION_ERROR`, and its message names the hook and the fault, +never the condition's source — the same status a validation rule that cannot be +evaluated answers. **The condition reads the record, not the payload (#4770).** It used to evaluate against `ctx.input.data` — only the fields the current write happened to carry — so diff --git a/docs/qa/platform-checklist/areas/records-forms.json b/docs/qa/platform-checklist/areas/records-forms.json index d80083ef225..05d6e3e8932 100644 --- a/docs/qa/platform-checklist/areas/records-forms.json +++ b/docs/qa/platform-checklist/areas/records-forms.json @@ -3337,7 +3337,7 @@ "title": "Object lifecycle hooks fire on the write path with the right timing, gate, async and error semantics — driven over /api/v1/data/*, oracled by the record effect and the log line", "since": "v15", "status": "active", - "revision": 6, + "revision": 7, "priority": "P1", "surface": "mixed", "personas": [ @@ -3442,8 +3442,8 @@ { "clause": "a condition that cannot be evaluated FAILS CLOSED (#4775): the write is ABORTED, not silently skipped — and it is NOT softened by onError:'log' nor fire-and-forgotten, because the condition gate runs OUTSIDE the async/retry/onError wrappers", "oracle": "api", - "verify": "scratch hook whose condition COMPILES but cannot be evaluated at runtime (e.g. int(record.name) > 0, written to a row whose name is not numeric); a write to that object is refused and the row does not land (hook-wrappers.ts: conditionFn throws outside runWithErrorPolicy). The 18.0 wire envelope is a generic 500 {code: 'INTERNAL_ERROR'}, with the class only in the server log ('[REST] Unhandled error: HookConditionError …') — #24619 row (b), open, tracks the coded refusal it should be. Until that lands score the clause on the refusal, the absent row and the log line, and record the 500 against #24619 rather than as a new finding. An UNCOMPILABLE condition never reaches this gate: it is refused at authoring (PUT /api/v1/meta/hook → 422 INVALID_METADATA, expression-invalid), so it cannot stand in for the runtime fail-closed probe", - "evidence": "the refusal (status + body) + the HookConditionError server-log line + a post-attempt count showing no row" + "verify": "scratch hook whose condition COMPILES but cannot be evaluated at runtime (e.g. int(record.name) > 0, written to a row whose name is not numeric); a write to that object is refused and the row does not land (hook-wrappers.ts: conditionFn throws outside runWithErrorPolicy). The wire envelope is a coded refusal: 400 {code: 'VALIDATION_ERROR'}, whose error text names the hook and the CEL fault (\"Hook 'NAME' could not evaluate its condition (FAULT) — operation aborted.\") and never the condition source, and no '[REST] Unhandled error' line is logged for it (#24662 retired the generic 500 INTERNAL_ERROR that #24619 row (b) recorded). A 500 INTERNAL_ERROR here is a FAIL. An UNCOMPILABLE condition never reaches this gate: it is refused at authoring (PUT /api/v1/meta/hook → 422 INVALID_METADATA, expression-invalid), so it cannot stand in for the runtime fail-closed probe", + "evidence": "the refusal (status 400 + code VALIDATION_ERROR + the hook named in its error text) + a post-attempt count showing no row" }, { "clause": "two hooks on the SAME object+event run lowest-priority-first: scratch hooks at priority 10 and 90 log in that order on one write (hook-binder passes priority to engine.registerHook; the engine orders by it, lower first)", @@ -3516,6 +3516,12 @@ "date": "2026-10-19", "change": "steps[0], negative[2], fixtures.requires[3], acceptance[8] (verify, evidence) and automated.ref corrected from wave anchor #24579 round 2 part 1 (6272032736) and QA runs #24598 / #24616 (ba4afcc2), re-read at objectstack f89d29e382. (1) The hook list route is GET /api/v1/meta/hook; /meta/types/hook answers 404. (2) acceptance[8]'s refusal answers 500 INTERNAL_ERROR with the class in the server log only; #24619 row (b) (open) tracks the coded refusal, so the clause states the shipped envelope and scores against #24619. (3) The ref named source files (hook-wrappers.ts, hook-binder.ts) no resolver can run; it now names four test files, each measured at base under os-verify-lock (83 passed), mapped to the clause whose mechanism it pins. (4) The run-proven scratch-hook recipe is recorded in requires[3]. Part 1's note on a `$select` in the steps does not hold: no step of this item spells `$select` (0 hits in records-forms.json)", "ref": "#24671" + }, + { + "revision": 7, + "date": "2026-10-19", + "change": "acceptance[8] (verify, evidence) follow the shipped envelope. An unevaluable hook condition's refusal is now a coded 400 VALIDATION_ERROR whose error text names the hook and the CEL fault, and no '[REST] Unhandled error' line is logged for it (#24662, split from #24619 row (b)). The 500 INTERNAL_ERROR that revision 6 recorded is retired and is now a FAIL; the server-log evidence goes with it. The clause is unchanged: the write is still refused and the row still absent", + "ref": "#24662" } ] }, diff --git a/packages/qa/dogfood/test/hook-after-condition-rollback.dogfood.test.ts b/packages/qa/dogfood/test/hook-after-condition-rollback.dogfood.test.ts index 948edfc8482..8cefca9c244 100644 --- a/packages/qa/dogfood/test/hook-after-condition-rollback.dogfood.test.ts +++ b/packages/qa/dogfood/test/hook-after-condition-rollback.dogfood.test.ts @@ -13,9 +13,10 @@ // its handler throws; an `abort` hook with the same unbindable condition is // rolled back as #22782 leaves it. One more: an `async: true` hook whose // condition evaluates now starts inside the door's unit and writes after it -// commits — its late write must still land (#22855). The refusal is REST's -// sanitised `500 INTERNAL_ERROR` on both sides of the fix; what this file pins -// is what is STORED. +// commits — its late write must still land (#22855). What this file pins is +// what is STORED, and the refusal's status and code: since #24662 an +// unevaluable condition is REST's coded `400 VALIDATION_ERROR` (it was the +// unclassified `500 INTERNAL_ERROR` on both sides of #22856's fix). import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { bootStack, type VerifyStack } from '@objectstack/verify'; @@ -72,13 +73,13 @@ describe('an after* hook whose condition cannot be evaluated rolls the write bac it('onError: \'log\' — REST createData is refused and the row is absent', async () => { const name = uniq('c'); - expect(await refusal(await create(LOGGED, name))).toEqual({ status: 500, code: 'INTERNAL_ERROR' }); + expect(await refusal(await create(LOGGED, name))).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); expect(await stored(LOGGED, name)).toBe(0); }); it('async: true — the condition is evaluated before the hook detaches: refused, the row absent', async () => { const name = uniq('c'); - expect(await refusal(await create(DETACHED, name))).toEqual({ status: 500, code: 'INTERNAL_ERROR' }); + expect(await refusal(await create(DETACHED, name))).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); expect(await stored(DETACHED, name)).toBe(0); }); @@ -90,7 +91,7 @@ describe('an after* hook whose condition cannot be evaluated rolls the write bac it('CONTROL an abort hook with the same condition: refused, the row absent — as #22782 leaves it', async () => { const name = uniq('c'); - expect(await refusal(await create(ABORTING, name))).toEqual({ status: 500, code: 'INTERNAL_ERROR' }); + expect(await refusal(await create(ABORTING, name))).toEqual({ status: 400, code: 'VALIDATION_ERROR' }); expect(await stored(ABORTING, name)).toBe(0); }); diff --git a/packages/rest/src/error-response-structured-arm-door-parity.test.ts b/packages/rest/src/error-response-structured-arm-door-parity.test.ts index d20da596a6d..e18d20b847e 100644 --- a/packages/rest/src/error-response-structured-arm-door-parity.test.ts +++ b/packages/rest/src/error-response-structured-arm-door-parity.test.ts @@ -75,7 +75,7 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { DuplicateRecordError } from '@objectstack/objectql'; +import { DuplicateRecordError, HookConditionError } from '@objectstack/objectql'; import { ConcurrentUpdateError } from '@objectstack/metadata-protocol'; import { mapDataError, @@ -436,6 +436,32 @@ const CASES: readonly Case[] = [ }, }, }, + { + // [#24662] An arm keyed on the CLASS (`name`) and its declared + // `reason`, so the code-literal census below cannot see it; listed + // here so the three doors are held together for it anyway. Its + // producer declares no `status` either. + covers: ['HookConditionError'], + what: "objectql's HookConditionError, reason 'unevaluable' — the condition compiled and faulted on this record", + error: () => new HookConditionError( + "Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int) — operation aborted.", + { + hook: 'qa_failclosed', object: 'inquiry', event: 'beforeInsert', + condition: "record.name.startsWith('qa-fc') && int(record.name) > 0", + reason: 'unevaluable', fault: 'runtime: int() type error: cannot convert to int', + }, + ), + object: 'inquiry', + restored: [], + expect: { + status: 400, + body: { + error: "Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int) — operation aborted.", + code: 'VALIDATION_ERROR', + object: 'inquiry', + }, + }, + }, { // The third control: a 5xx ARM. Its producer declares no `status` // either, so both doors reach it today; §4 pins that it can never diff --git a/packages/rest/src/rest-hook-condition-unevaluable-refusal.test.ts b/packages/rest/src/rest-hook-condition-unevaluable-refusal.test.ts new file mode 100644 index 00000000000..ed86d3732a7 --- /dev/null +++ b/packages/rest/src/rest-hook-condition-unevaluable-refusal.test.ts @@ -0,0 +1,215 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24662] A write an authored hook `condition` refused because the condition + * could not be evaluated on the record answers a coded `400 VALIDATION_ERROR`, + * not the unclassified `500 INTERNAL_ERROR`. + * + * Driven through a REAL engine (`ObjectQL` + sqlite `SqlDriver`), a real + * `ObjectStackProtocolImplementation`, the hook bound by the engine's own + * `bindHooksToEngine`, and `RestServer`'s registered handlers — the harness + * `rest-data-create-address-unknown-key.test.ts` boots. The hook is the + * card's: `beforeInsert`, condition + * `record.name.startsWith('qa-fc') && int(record.name) > 0`, which faults on + * `qa-fc-abc` (`int()` cannot convert it). + * + * Measured on `origin/main` `eadea5720` before the arm: the single-record door + * answered `500 {"error":"Internal server error","code":"INTERNAL_ERROR"}` and + * logged `[REST] Unhandled error: HookConditionError: Hook 'qa_failclosed' + * could not evaluate its condition (runtime: int() type error: cannot convert + * to int) — operation aborted.`; `createMany` answered the same `500` and the + * same line. The row was absent both times, which is the half that must not + * move. + * + * The controls: a hook that refuses ON PURPOSE (a declared `403 + * PERMISSION_DENIED`) answers byte for byte what it answered before, and a + * condition that never COMPILED keeps its sanitised `500` and its log line — + * the second is also the positive control proving this file's log spy sees a + * `[REST] Unhandled error` line when there is one. + */ + +import { describe, it, expect, vi, afterEach, beforeAll, afterAll } from 'vitest'; +import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +// This file observes the REST fault log, so it declares the level it asserts +// against ('info' is the shipped default) instead of the suite's quiet one. +beforeAll(() => { vi.stubEnv('OS_REST_LOG', 'info'); }); +afterAll(() => { vi.unstubAllEnvs(); }); + +const PACKAGE_ID = 'objectstack-test'; +const INQUIRY = { + name: 'inquiry', label: 'Inquiry', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +/** The card's hook: its condition compiles, and faults on a `qa-fc` name that is not an integer. */ +const CARD_CONDITION = "record.name.startsWith('qa-fc') && int(record.name) > 0"; +const UNEVALUABLE = { + name: 'qa_failclosed', object: 'inquiry', events: ['beforeInsert'], + condition: CARD_CONDITION, handler: async () => {}, +}; +/** + * The `after*` path: a condition reading `previous` on an insert (unbound + * there), on an `onError: 'log'` hook — the shape + * `hook-after-condition-rollback.dogfood.test.ts` pins through a booted stack. + * The throw reaches the door from inside the write door's unit (#22782 / + * #22856) rather than ahead of the insert, and must answer the same refusal. + */ +const AFTER_UNEVALUABLE = { + name: 'qa_after_watch', object: 'inquiry', events: ['afterInsert'], onError: 'log', + condition: 'previous.name != record.name', handler: async () => {}, +}; +/** CONTROL: a hook that refuses on purpose, declaring its ADR-0112 envelope. */ +const REFUSING = { + name: 'qa_refuse', object: 'inquiry', events: ['beforeInsert'], + handler: async () => { + throw Object.assign(new Error('Inquiries are closed.'), { status: 403, code: 'PERMISSION_DENIED' }); + }, +}; +/** CONTROL: a condition that never compiled — `reason: 'uncompilable'`, not this card's. */ +const UNCOMPILABLE = { + name: 'qa_broken', object: 'inquiry', events: ['beforeInsert'], + condition: 'record.name ==', handler: async () => {}, +}; + +const UNEVALUABLE_MESSAGE = + "Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int)" + + ' — operation aborted.'; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = (code: number) => { res.statusCode = code; return res; }; + res.json = (body: any) => { res.body = body; return res; }; + res.header = () => res; + res.setHeader = () => {}; + res.send = () => res; + res.end = () => res; + res.write = () => true; + return res; +} + +async function boot(hooks: any[]) { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), true); + await engine.init(); + engine.registry.registerObject(INQUIRY as any, PACKAGE_ID); + await engine.syncSchemas(); + const bound = bindHooksToEngine(engine, hooks as any, { packageId: PACKAGE_ID }); + expect(bound.registered).toBe(hooks.length); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const noop = () => {}; + const server = { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; + const rest: any = new RestServer(server as any, protocol as any, { api: { requireAuth: false } } as any); + rest.resolveExecCtx = async () => ({ userId: 'test-user', isSystem: false }); + rest.registerRoutes(); + const route = (path: string) => { + const found = rest.getRoutes().find((r: any) => r.method === 'POST' && r.path === `/api/v1/data/:object${path}`); + if (!found) throw new Error(`POST /api/v1/data/:object${path} is not registered`); + return found; + }; + const unhandled = vi.spyOn(console, 'error').mockImplementation(() => {}); + const drive = async (path: string, body: unknown) => { + const res = makeRes(); + await route(path).handler({ + method: 'POST', path: `/api/v1/data/inquiry${path}`, params: { object: 'inquiry' }, + headers: { 'content-type': 'application/json' }, query: {}, body, + }, res); + return { status: res.statusCode as number, body: res.body as Record }; + }; + return { + create: (body: unknown) => drive('', body), + createMany: (body: unknown) => drive('/createMany', body), + rows: async () => ((await engine.find('inquiry', {})) as unknown[]).length, + /** Every `[REST] Unhandled error` line this boot printed. */ + unhandledLines: () => unhandled.mock.calls.filter((c) => c[0] === '[REST] Unhandled error:'), + }; +} + +describe('[#24662] an unevaluable hook condition is a coded refusal on the REST data doors', () => { + it("the card's request: 400 VALIDATION_ERROR naming the hook, the row absent, no Unhandled line", async () => { + const stack = await boot([UNEVALUABLE]); + + const res = await stack.create({ id: 'r1', name: 'qa-fc-abc' }); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: UNEVALUABLE_MESSAGE, code: 'VALIDATION_ERROR', object: 'inquiry' }); + // What the sentence names: the hook and the CEL fault. Never the + // condition source, never the value that faulted. + expect(String(res.body.error)).not.toContain(CARD_CONDITION); + expect(String(res.body.error)).not.toContain('qa-fc-abc'); + // Fail-closed holds: nothing was written. + expect(await stack.rows()).toBe(0); + // A refusal the caller was told about is not an unhandled fault. + expect(stack.unhandledLines()).toEqual([]); + }); + + it('the bulk door (createMany, through handleRouteError) answers the same refusal', async () => { + const stack = await boot([UNEVALUABLE]); + + const res = await stack.createMany([{ id: 'r1', name: 'qa-fc-abc' }]); + + expect(res.status).toBe(400); + expect(res.body).toEqual({ error: UNEVALUABLE_MESSAGE, code: 'VALIDATION_ERROR', object: 'inquiry' }); + expect(await stack.rows()).toBe(0); + expect(stack.unhandledLines()).toEqual([]); + }); + + it('an afterInsert condition that reads previous on an insert: the same coded refusal, the row rolled back', async () => { + const stack = await boot([AFTER_UNEVALUABLE]); + + const res = await stack.create({ id: 'r1', name: 'x' }); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('VALIDATION_ERROR'); + expect(String(res.body.error)).toContain("Hook 'qa_after_watch' could not evaluate its condition"); + expect(String(res.body.error)).not.toContain('previous.name != record.name'); + expect(await stack.rows()).toBe(0); + expect(stack.unhandledLines()).toEqual([]); + }); + + it('CONTROL: a hook that refuses on purpose answers exactly as before', async () => { + const stack = await boot([REFUSING]); + + const res = await stack.create({ id: 'r1', name: 'qa-fc-abc' }); + + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: 'Inquiries are closed.', code: 'PERMISSION_DENIED', object: 'inquiry' }); + expect(await stack.rows()).toBe(0); + expect(stack.unhandledLines()).toEqual([]); + }); + + it('CONTROL: a condition that never compiled keeps its sanitised 500 and its log line', async () => { + const stack = await boot([UNCOMPILABLE]); + + const res = await stack.create({ id: 'r1', name: 'x' }); + + expect(res.status).toBe(500); + expect(res.body).toEqual({ error: 'Internal server error', code: 'INTERNAL_ERROR' }); + expect(await stack.rows()).toBe(0); + // The positive control for the spy the two cases above read as empty. + const lines = stack.unhandledLines(); + expect(lines).toHaveLength(1); + expect((lines[0][1] as Error).name).toBe('HookConditionError'); + expect((lines[0][1] as { reason?: unknown }).reason).toBe('uncompilable'); + }); +}); diff --git a/packages/runtime/src/domains/data-hook-condition-refusal.test.ts b/packages/runtime/src/domains/data-hook-condition-refusal.test.ts new file mode 100644 index 00000000000..cd3933ab67c --- /dev/null +++ b/packages/runtime/src/domains/data-hook-condition-refusal.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24662] The runtime dispatcher's `/data` door answers a write an + * unevaluable hook condition refused the way `RestServer` does: `400 + * VALIDATION_ERROR`, with the row absent. + * + * The dispatcher's `/data` domain does not catch the engine's throw, so the + * `HookConditionError` leaves `dispatch()` and the HOST's exit renders it — + * `@objectstack/hono`'s `${prefix}/*` catch-all, the only `/data` door on a + * host that mounts just that catch-all, answers through + * `declaredEnvelopeForThrow` (`@objectstack/plugin-hono-server`, imported + * here, ⛔ not restated), and `dispatcher-plugin`'s `errorResponseBase` through + * `resolveThrownHttpError`. Both read the one recogniser in + * `@objectstack/types` (`hook-condition-refusal.ts`). + * + * Measured on `origin/main` `eadea5720` before it: the throw left `dispatch()`, + * `declaredEnvelopeForThrow` declined it, and the catch-all's fallback arm + * answered `500 INTERNAL_ERROR` carrying the hook's message; the resolver said + * `500 INTERNAL_ERROR`. The row was absent. + * + * The rig is `data-write-request-gate.test.ts`'s: a real store (better-sqlite3 + * `:memory:`, the real `ObjectQL`, a real `ObjectStackProtocolImplementation`) + * and `HttpDispatcher.dispatch()` handed what the catch-all hands it. The hook + * is bound by the engine's own `bindHooksToEngine`. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { declaredEnvelopeForThrow } from '@objectstack/plugin-hono-server'; +import { resolveThrownHttpError } from '@objectstack/types'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const PREFIX = '/api/v1'; +const PACKAGE_ID = 'objectstack-test'; +const CALLER = { userId: 'u_member', isSystem: false }; +const INQUIRY = { + name: 'inquiry', label: 'Inquiry', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +const UNEVALUABLE = { + name: 'qa_failclosed', object: 'inquiry', events: ['beforeInsert'], + condition: "record.name.startsWith('qa-fc') && int(record.name) > 0", handler: async () => {}, +}; +const REFUSING = { + name: 'qa_refuse', object: 'inquiry', events: ['beforeInsert'], + handler: async () => { + throw Object.assign(new Error('Inquiries are closed.'), { status: 403, code: 'PERMISSION_DENIED' }); + }, +}; +const UNCOMPILABLE = { + name: 'qa_broken', object: 'inquiry', events: ['beforeInsert'], + condition: 'record.name ==', handler: async () => {}, +}; + +const UNEVALUABLE_MESSAGE = + "Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int)" + + ' — operation aborted.'; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot(hooks: any[]) { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), true); + await engine.init(); + engine.registry.registerObject(INQUIRY as any, PACKAGE_ID); + await engine.syncSchemas(); + expect(bindHooksToEngine(engine, hooks as any, { packageId: PACKAGE_ID }).registered).toBe(hooks.length); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + // `callData`'s exposure gate reads the kernel's `metadata` service. + const metadata = { getObject: async (name: string) => engine.registry.getObject(name) }; + const services: Record = { protocol, metadata }; + const get = (n: string) => services[n] ?? null; + const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) }; + const dispatcher = new HttpDispatcher(kernel); + (dispatcher as any).timedResolveExecutionContext = async () => ({ ...CALLER }); + + /** `POST /data/inquiry` as the catch-all hands it: the returned answer, or what left `dispatch()`. */ + const create = async (body: Record): Promise<{ returned?: any; thrown?: unknown }> => { + const raw = new Request(`http://localhost${PREFIX}/data/inquiry`, { + method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify(body), + }); + try { + return { returned: await dispatcher.dispatch('POST', '/data/inquiry', body, {}, { request: raw } as any, PREFIX) }; + } catch (thrown) { + return { thrown }; + } + }; + const rows = async () => ((await engine.find('inquiry', {})) as unknown[]).length; + return { create, rows }; +} + +describe('[#24662] the dispatcher /data door — an unevaluable hook condition is a coded refusal', () => { + it("the card's request: both host exits answer 400 VALIDATION_ERROR, and the row is absent", async () => { + const stack = await boot([UNEVALUABLE]); + + const { thrown } = await stack.create({ id: 'r1', name: 'qa-fc-abc' }); + + expect((thrown as Error | undefined)?.name).toBe('HookConditionError'); + // `@objectstack/hono`'s catch-all: the declared limb now serves it. + expect(declaredEnvelopeForThrow(thrown)).toEqual({ + status: 400, + body: { success: false, error: { code: 'VALIDATION_ERROR', message: UNEVALUABLE_MESSAGE } }, + }); + // `dispatcher-plugin`'s `errorResponseBase` reads this resolution. + expect(resolveThrownHttpError(thrown)).toMatchObject({ status: 400, code: 'VALIDATION_ERROR' }); + expect(await stack.rows()).toBe(0); + }); + + it('CONTROL: a hook that refuses on purpose answers exactly as before', async () => { + const stack = await boot([REFUSING]); + + const { returned, thrown } = await stack.create({ id: 'r1', name: 'qa-fc-abc' }); + + expect(thrown).toBeUndefined(); + expect(returned.response.status).toBe(403); + expect(returned.response.body).toEqual({ + success: false, + error: { code: 'PERMISSION_DENIED', message: 'Inquiries are closed.', httpStatus: 403, details: { object: 'inquiry' } }, + }); + expect(await stack.rows()).toBe(0); + }); + + it('CONTROL: a condition that never compiled still reaches the hosts\' undeclared 500', async () => { + const stack = await boot([UNCOMPILABLE]); + + const { thrown } = await stack.create({ id: 'r1', name: 'x' }); + + expect((thrown as { reason?: unknown } | undefined)?.reason).toBe('uncompilable'); + expect(declaredEnvelopeForThrow(thrown)).toBeUndefined(); + expect(resolveThrownHttpError(thrown)).toMatchObject({ status: 500, code: 'INTERNAL_ERROR' }); + expect(await stack.rows()).toBe(0); + }); +}); diff --git a/packages/types/src/data-error-classification.ts b/packages/types/src/data-error-classification.ts index 2456ae803e5..be3acedb101 100644 --- a/packages/types/src/data-error-classification.ts +++ b/packages/types/src/data-error-classification.ts @@ -35,6 +35,7 @@ import { declaredRefusalMessage, } from './thrown-http-error.js'; import { isNativeErrorName } from './native-error-name.js'; +import { unevaluableHookConditionRefusal } from './hook-condition-refusal.js'; /** * How many characters of a domain error's OWN message reach the client. @@ -1191,6 +1192,37 @@ function structuredCodeAnswer( }, }; } + // [#24662] An authored hook `condition` that compiled and then faulted on + // the record it judged → 400 `VALIDATION_ERROR`. The write is refused + // fail-closed (#4775) and nothing is stored; until this arm the refusal + // matched nothing here and left through `UNCLASSIFIED_FAULT` as + // `500 INTERNAL_ERROR` with a `[REST] Unhandled error` line. The status, + // the code, the `unevaluable`-only scope and why neither is new are + // stated once, in `hook-condition-refusal.ts`; the runtime doors read the + // same recogniser through `resolveThrownHttpError`. + // + // Beside the arm above because that is its precedent: the validation + // predicate's unevaluable refusal (#4649), whose sentence comes out of the + // same `cel-fault.ts`. So the message is the producer's, relayed as that + // arm relays its own: it names the hook, the CEL fault in one line and, + // when known, the key the condition read — never the condition source + // (only the `uncompilable` message quotes that, and this arm declines it) + // and never a record value. `object` is the route's, as above. + // + // The log verdict needs no list entry: `isDeclaredClientRefusal` + // (`@objectstack/rest`) reads the declaration off `resolveThrownHttpError`, + // which reports this shape's status and code, so the refusal is quiet. + const hookCondition = unevaluableHookConditionRefusal(error); + if (hookCondition !== undefined) { + return { + status: hookCondition.status, + body: { + error: armSentence(error) ?? 'A hook condition could not be evaluated for this record', + code: hookCondition.code, + ...(object ? { object } : {}), + }, + }; + } // Capability gates (#2707 feeds / #2727 files): plugin-audit's engine // hooks reject sys_comment / sys_attachment inserts fail-closed when the // TARGET object's capability flag disallows them. 403 like diff --git a/packages/types/src/hook-condition-refusal.test.ts b/packages/types/src/hook-condition-refusal.test.ts new file mode 100644 index 00000000000..cc3e78a958a --- /dev/null +++ b/packages/types/src/hook-condition-refusal.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24662] The one recogniser for an unevaluable hook condition's refusal, and + * the two rules that read it: `resolveThrownHttpError` (the runtime doors, the + * Hono doors and the batch rows) and `mapDataError` (`@objectstack/rest`'s + * doors). Both must answer `400 VALIDATION_ERROR` for the same throw, and both + * must leave the `uncompilable` reason on the sanitised `500` it had. + * + * The error is built in the shape `@objectstack/objectql`'s `HookConditionError` + * has — its declared `name` and fields. This package cannot import that class; + * the real class drives the same answer end to end in `@objectstack/rest`'s + * `rest-hook-condition-unevaluable-refusal.test.ts` and in + * `@objectstack/runtime`'s `domains/data-hook-condition-refusal.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { unevaluableHookConditionRefusal } from './hook-condition-refusal.js'; +import { resolveThrownHttpError } from './thrown-http-error.js'; +import { mapDataError } from './data-error-classification.js'; + +const UNEVALUABLE_MESSAGE = + "Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int)" + + ' — operation aborted.'; +const UNCOMPILABLE_MESSAGE = + "Hook 'qa_broken' declares a condition that does not compile (parse: Unexpected token: EOF) — operation aborted." + + ' The condition can never be evaluated, so the hook can neither run nor be skipped honestly;' + + ' fix the expression: record.name =='; + +function hookConditionError(reason: 'unevaluable' | 'uncompilable'): Error { + const unevaluable = reason === 'unevaluable'; + const err = new Error(unevaluable ? UNEVALUABLE_MESSAGE : UNCOMPILABLE_MESSAGE); + return Object.assign(err, { + name: 'HookConditionError', + hook: unevaluable ? 'qa_failclosed' : 'qa_broken', + object: 'inquiry', + event: 'beforeInsert', + condition: unevaluable ? "record.name.startsWith('qa-fc') && int(record.name) > 0" : 'record.name ==', + reason, + fault: unevaluable ? 'runtime: int() type error: cannot convert to int' : 'parse: Unexpected token: EOF', + }); +} + +describe('[#24662] unevaluableHookConditionRefusal', () => { + it('answers 400 and the catalog member 400 names for the unevaluable reason', () => { + expect(unevaluableHookConditionRefusal(hookConditionError('unevaluable'))) + .toEqual({ status: 400, code: 'VALIDATION_ERROR' }); + }); + + it('declines the uncompilable reason, another class, a bare reason and a non-object', () => { + expect(unevaluableHookConditionRefusal(hookConditionError('uncompilable'))).toBeUndefined(); + expect(unevaluableHookConditionRefusal(Object.assign(new Error('x'), { reason: 'unevaluable' }))).toBeUndefined(); + expect(unevaluableHookConditionRefusal(Object.assign(new Error('x'), { name: 'HookConditionError' }))).toBeUndefined(); + expect(unevaluableHookConditionRefusal(undefined)).toBeUndefined(); + expect(unevaluableHookConditionRefusal('HookConditionError')).toBeUndefined(); + }); +}); + +describe('[#24662] the two rules that read it agree', () => { + it('resolveThrownHttpError: a declaration by shape — status AND code, the message verbatim', () => { + const thrown = resolveThrownHttpError(hookConditionError('unevaluable')); + expect(thrown).toMatchObject({ + status: 400, + declaredStatus: 400, + code: 'VALIDATION_ERROR', + declaredCode: 'VALIDATION_ERROR', + message: UNEVALUABLE_MESSAGE, + }); + }); + + it('mapDataError: the same status and code, the producer sentence, the route object', () => { + expect(mapDataError(hookConditionError('unevaluable'), 'inquiry')).toEqual({ + status: 400, + body: { error: UNEVALUABLE_MESSAGE, code: 'VALIDATION_ERROR', object: 'inquiry' }, + }); + }); + + it('CONTROL: the uncompilable reason keeps the undeclared 500 on both rules', () => { + const thrown = resolveThrownHttpError(hookConditionError('uncompilable')); + expect(thrown.status).toBe(500); + expect(thrown.declaredStatus).toBeUndefined(); + expect(thrown.code).toBe('INTERNAL_ERROR'); + expect(mapDataError(hookConditionError('uncompilable'), 'inquiry')).toEqual({ + status: 500, + body: { error: 'Internal server error', code: 'INTERNAL_ERROR' }, + }); + }); +}); diff --git a/packages/types/src/hook-condition-refusal.ts b/packages/types/src/hook-condition-refusal.ts new file mode 100644 index 00000000000..6099c687414 --- /dev/null +++ b/packages/types/src/hook-condition-refusal.ts @@ -0,0 +1,103 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24662] Recognising, at an HTTP boundary, a write that an authored hook + * `condition` refused because the condition could not be evaluated on the + * record it was judging. + * + * `HookConditionError` (`@objectstack/objectql`, `hook-wrappers.ts`) is the + * fail-closed answer ADR-0058's 2026-08 addendum gives an unevaluable write + * gate (#4775): the operation is aborted and nothing is written. It carries no + * `status` and no `code`, deliberately — its docblock rules that a fact on it + * reaches the wire through the ADR-0112 vocabulary as a decision, never as a + * property that happens to be named `code`. So, exactly as for the + * record-validation shape (`validation-failure.ts`), deciding what status it + * means is the job of whichever boundary serves it. No boundary decided, so + * it fell to the unclassified terminal on every door: `500 INTERNAL_ERROR`, + * read by a client as "the server fell over", and logged as + * `[REST] Unhandled error`. + * + * ## The answer: `400`, and the catalog member `400` names + * + * The nearest landed precedent is the condition's own sibling in that + * addendum's table: an object validation predicate that cannot be evaluated + * (#4649). Measured on the `/data` write door, it answers `400` (code + * `VALIDATION_FAILED`, a per-field envelope naming the rule with + * `constraint.reason: 'unevaluable'`), and its message is built by the same + * `cel-fault.ts` sentence this class's message is built by. So the status is + * the sibling's: the refusal turned on THIS record's values, a different + * record can pass the same condition, and retrying the same request cannot. + * + * The code is NOT the sibling's `VALIDATION_FAILED`, whose contract is the + * per-field `fields[]` envelope a form maps onto its inputs; a hook's gate + * addresses no field. It is `standardErrorCodeForHttpStatus(400)` — + * `VALIDATION_ERROR`, the `StandardErrorCode` member ADR-0112's status map + * names for a `400` whose producer declared no more specific code, and the + * code a batch row and the nested-envelope doors already derive for an + * un-coded `400` refusal. No code is minted and no ledger row is added. + * + * Why no new code: a consumer branches on it exactly as it branches on a + * generic `400` — do not retry the same request, show the message. The + * finer fact ("the author's expression faulted, not your input") already has + * a discriminator on the sibling (`constraint.reason`) and no consumer was + * found branching on it, so it gives no branch a new top-level code would + * serve. + * + * ## Why `unevaluable` only + * + * The class has two `reason`s. `uncompilable` — the condition never compiled — + * does not turn on the request at all: every write to the object fails until + * an author fixes the metadata, the gate already logs it at `error` when it is + * bound, and its message quotes the condition SOURCE, which no sibling + * refusal puts on the wire. It keeps the sanitised `500` it has today. Only a + * condition that compiled and then faulted on the record it was handed is a + * refusal of THIS request. + * + * ## Recognised by declared name and field, never by prose + * + * `@objectstack/types` cannot import `@objectstack/objectql` (the arrow points + * the other way), so the class is recognised the way its siblings are: by its + * declared `name` and by its declared `reason` field, both read off the + * producer. ⛔ Never by its message text. + * + * ## Read by two rules, so the doors answer one throw alike + * + * - `resolveThrownHttpError` (`thrown-http-error.ts`) — the rule the runtime + * dispatcher's exits, both Hono doors and the batch rows read. The shape is + * a declaration there, as the validation shape is: status `400` and the + * code that status names. + * - `structuredCodeAnswer` (`data-error-classification.ts`) — the arms + * `@objectstack/rest`'s single-record and bulk doors ask first. + * + * Both take the status and the code from {@link unevaluableHookConditionRefusal}, + * so neither spells them. + */ + +import { standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; +import type { StandardErrorCode } from '@objectstack/spec/api'; + +/** The HTTP status an unevaluable hook condition's refusal is served with. */ +export const UNEVALUABLE_HOOK_CONDITION_STATUS = 400; + +/** What a boundary answers for a write an unevaluable hook condition refused. */ +export interface UnevaluableHookConditionRefusal { + status: typeof UNEVALUABLE_HOOK_CONDITION_STATUS; + /** The `StandardErrorCode` member the status names (`VALIDATION_ERROR`). */ + code: StandardErrorCode; +} + +/** + * The boundary's answer for `err` when it is a `HookConditionError` whose + * condition compiled and then faulted on the record it judged, or `undefined` + * for everything else, an `uncompilable` condition included. Callers use the + * `undefined` result as the predicate and the object as the answer, so the + * two cannot disagree. + */ +export function unevaluableHookConditionRefusal(err: unknown): UnevaluableHookConditionRefusal | undefined { + const e = err as { name?: unknown; reason?: unknown } | null | undefined; + if (e?.name !== 'HookConditionError' || e?.reason !== 'unevaluable') return undefined; + return { + status: UNEVALUABLE_HOOK_CONDITION_STATUS, + code: standardErrorCodeForHttpStatus(UNEVALUABLE_HOOK_CONDITION_STATUS), + }; +} diff --git a/packages/types/src/thrown-http-error.ts b/packages/types/src/thrown-http-error.ts index ad4842f44bc..8742ad4f1ac 100644 --- a/packages/types/src/thrown-http-error.ts +++ b/packages/types/src/thrown-http-error.ts @@ -93,6 +93,7 @@ import { ErrorCode, standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; import { declaresServerFault, looksLikeInternalErrorLeak } from './error-leak.js'; import { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js'; +import { unevaluableHookConditionRefusal } from './hook-condition-refusal.js'; /** The HTTP answer a thrown error declares. See {@link resolveThrownHttpError}. */ export interface ThrownHttpError { @@ -100,7 +101,8 @@ export interface ThrownHttpError { status: number; /** * The status the THROW ITSELF declared — `.status`, `.statusCode`, or the - * 400 a validation-shaped throw declares by shape — and **absent** when it + * 400 a validation-shaped throw or an unevaluable hook condition (#24662) + * declares by shape — and **absent** when it * declared none, i.e. when {@link ThrownHttpError.status} above is the * caller's `fallbackStatus`. * @@ -190,10 +192,10 @@ export interface ThrownHttpError { * * | Question | Answer | * |---|---| - * | status | `.status` → `.statusCode` → 400 if it is a validation failure → `fallbackStatus` | + * | status | `.status` → `.statusCode` → 400 if it is a validation failure → 400 if it is an unevaluable hook condition → `fallbackStatus` | * | declaredStatus | the same chain WITHOUT the fallback — absent when the throw declared none | * | code | `VALIDATION_FAILED` if it is one → a REGISTERED `.code` → derived from the status | - * | declaredCode | `VALIDATION_FAILED` if it is one → any non-empty string `.code` → absent | + * | declaredCode | `VALIDATION_FAILED` if it is one → any non-empty string `.code` → the derived code if it is an unevaluable hook condition → absent | * | message | `.message` when it is a string → `String(error)` | * | userMessage | a non-empty string `.userMessage` → absent (see {@link declaredUserMessage}) | * @@ -212,10 +214,21 @@ export function resolveThrownHttpError(error: unknown, fallbackStatus = 500): Th // producer did say "this is a client's input problem" — which is the fact // `declaredStatus` reports. Only the `fallbackStatus` limb below is the // caller's own invention, and it is the only one left out. + // [#24662] …and so is an unevaluable hook condition's: `HookConditionError` + // carries no status or code by its own contract, and the shape says "the + // author's condition faulted on this record, and the write was refused" — + // a 400 refusal, the status its validation-predicate sibling is served + // with. See `hook-condition-refusal.ts` for the code choice and the scope. + // It is a declaration of BOTH halves, as the validation shape's is, so the + // Hono doors' declared-envelope gate (a declared status AND a registered + // code) serves it rather than the bare 500. + const hookCondition = validation ? undefined : unevaluableHookConditionRefusal(e); + const declaredStatus = typeof e?.status === 'number' ? e.status : typeof e?.statusCode === 'number' ? e.statusCode : validation ? VALIDATION_FAILED_STATUS + : hookCondition ? hookCondition.status : undefined; const status = declaredStatus ?? fallbackStatus; @@ -228,7 +241,7 @@ export function resolveThrownHttpError(error: unknown, fallbackStatus = 500): Th const code: ErrorCode = validation ? validation.code : (registered ?? standardErrorCodeForHttpStatus(status)); - const declaredCode = validation ? validation.code : spelled; + const declaredCode = validation ? validation.code : (spelled ?? hookCondition?.code); const issues = Array.isArray(e?.issues) ? e.issues : undefined; const details: Record = {