|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * [#17265] A sandboxed hook's BUSINESS REFUSAL reached through a script |
| 5 | + * action's `ctx.api` write is a rejection, not the sandbox faulting. |
| 6 | + * |
| 7 | + * ## What was measured broken |
| 8 | + * |
| 9 | + * The card reports `POST /api/v1/actions/clm_contract/submit_contract` |
| 10 | + * answering `500 INTERNAL_ERROR` for a `beforeUpdate` hook that refused a state |
| 11 | + * transition with a written, user-facing sentence — the same refusal `/data` |
| 12 | + * has answered `400` with the sentence verbatim since #11588. |
| 13 | + * |
| 14 | + * `domains/actions.ts`'s classifier is NOT the producer: it reads the shape it |
| 15 | + * is handed correctly (`actions-fault-vs-rejection.test.ts` pins both sides of |
| 16 | + * that line and neither moves). The refusal arrives at it already stripped of |
| 17 | + * every mark that says "a body reported this on purpose", one VM hop earlier: |
| 18 | + * |
| 19 | + * 1. the sandboxed `beforeUpdate` hook refuses — its own runner wraps that as |
| 20 | + * `SandboxError("hook 'g' threw: <biz>", "<biz>")`, `innerMessage` SET; |
| 21 | + * 2. it travels out of `engine.update()` into the ACTION body's host call, so |
| 22 | + * `hostErrorToVm` marshals it INTO the action's VM — and marked every |
| 23 | + * `SandboxError` reaching it as {@link SANDBOX_FAULT_PROP}, the sandbox's |
| 24 | + * OWN fault (#4431), on an `instanceof` test; |
| 25 | + * 3. escaping the action body uncaught, the pump loop reads that marker and |
| 26 | + * throws a bare `SandboxError` — no `innerMessage`, no `code`, no |
| 27 | + * `status`, no `fields`; |
| 28 | + * 4. which is, by the #3951 contract, exactly a CRASH ⇒ `errorFromThrown(err, |
| 29 | + * 500)` ⇒ `500 INTERNAL_ERROR`. |
| 30 | + * |
| 31 | + * The marker's own sibling pin already named this risk — "a marker applied too |
| 32 | + * broadly would turn every failed write into a 500" |
| 33 | + * (`capability-denial-is-a-fault.test.ts`) — and measured it with a plain |
| 34 | + * `ValidationError`, which is not a `SandboxError` and so never tripped the |
| 35 | + * `instanceof`. A NESTED sandboxed refusal is. |
| 36 | + * |
| 37 | + * ## The line this file pins |
| 38 | + * |
| 39 | + * The discriminator is the one `/data` asks (`sandboxBusinessMessage`, #11588): |
| 40 | + * does the error carry a caller-addressed business sentence that is not a |
| 41 | + * script fault? A refusal does and stays a rejection; a capability denial and a |
| 42 | + * nested CRASH do not and stay faults. So `/data`'s answer and the action |
| 43 | + * route's answer are the same answer, per route and per status. |
| 44 | + */ |
| 45 | + |
| 46 | +import { describe, it, expect, vi } from 'vitest'; |
| 47 | + |
| 48 | +import { HttpDispatcher } from '../http-dispatcher.js'; |
| 49 | +import { QuickJSScriptRunner, SandboxError } from './quickjs-runner.js'; |
| 50 | +import type { ScriptContext, ScriptRunOptions } from './script-runner.js'; |
| 51 | + |
| 52 | +/** The sentence the consuming app's guard addressed to its end user. */ |
| 53 | +const REFUSAL = 'A contract cannot be submitted without a version file'; |
| 54 | + |
| 55 | +const runner = new QuickJSScriptRunner({ hookTimeoutMs: 10_000, actionTimeoutMs: 10_000 }); |
| 56 | +const actionOpts: ScriptRunOptions = { origin: { kind: 'action', name: 'submit_contract' } }; |
| 57 | + |
| 58 | +/** |
| 59 | + * The shape a SANDBOXED `beforeUpdate` hook's deliberate refusal really has |
| 60 | + * when `engine.update()` hands it back to the action body's host call — built |
| 61 | + * by `quickjs-runner`'s own pump loop one level down, so `innerMessage` is set. |
| 62 | + */ |
| 63 | +function nestedHookRefusal(): SandboxError { |
| 64 | + return new SandboxError(`hook 'guard_contract_submit' threw: ${REFUSAL}`, REFUSAL); |
| 65 | +} |
| 66 | + |
| 67 | +/** A `ctx.api.object(x).update(...)` host seam whose write the hook refuses. */ |
| 68 | +function refusingApi(thrown: () => unknown) { |
| 69 | + return { |
| 70 | + object: (_n: string) => ({ |
| 71 | + update: async () => { throw thrown(); }, |
| 72 | + }), |
| 73 | + }; |
| 74 | +} |
| 75 | + |
| 76 | +function ctx(over: Partial<ScriptContext> = {}): ScriptContext { |
| 77 | + return { input: {}, ...over }; |
| 78 | +} |
| 79 | + |
| 80 | +/** Run a script action whose `ctx.api` write throws `thrown`, and return the escape. */ |
| 81 | +async function escapeOf(thrown: () => unknown): Promise<any> { |
| 82 | + const err = await runner.runScript( |
| 83 | + { |
| 84 | + language: 'js', |
| 85 | + source: "return await ctx.api.object('clm_contract').update('c_1', { status: 'submitted' });", |
| 86 | + capabilities: ['api.write'], |
| 87 | + }, |
| 88 | + ctx({ api: refusingApi(thrown) }), |
| 89 | + actionOpts, |
| 90 | + ).then(() => null, (e) => e); |
| 91 | + expect(err, 'expected the action to reject, but the script resolved').toBeInstanceOf(SandboxError); |
| 92 | + return err; |
| 93 | +} |
| 94 | + |
| 95 | +// ── the wire half ──────────────────────────────────────────────────────────── |
| 96 | + |
| 97 | +const scriptAction = { |
| 98 | + name: 'submit_contract', |
| 99 | + objectName: 'clm_contract', |
| 100 | + type: 'script', |
| 101 | + body: { language: 'js', source: 'return 1;', capabilities: ['api.write'] }, |
| 102 | +}; |
| 103 | + |
| 104 | +/** The same dispatcher harness `actions-fault-vs-rejection.test.ts` uses. */ |
| 105 | +function makeDispatcher(thrown: unknown) { |
| 106 | + const objectDef = { name: 'clm_contract', actions: [scriptAction] }; |
| 107 | + const ql: any = { |
| 108 | + executeAction: vi.fn(async () => { throw thrown; }), |
| 109 | + getSchema: (n: string) => (n === objectDef.name ? objectDef : undefined), |
| 110 | + registry: { getObject: (n: string) => (n === objectDef.name ? objectDef : undefined), getItem: () => undefined }, |
| 111 | + find: vi.fn(async () => [{ id: 'c_1', status: 'draft' }]), |
| 112 | + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), |
| 113 | + }; |
| 114 | + const metadata: any = { |
| 115 | + load: vi.fn(async () => null), |
| 116 | + listObjects: vi.fn(async () => [objectDef]), |
| 117 | + getObject: vi.fn(async () => objectDef), |
| 118 | + }; |
| 119 | + const kernel: any = { |
| 120 | + context: { |
| 121 | + getService: (n: string) => |
| 122 | + n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : null, |
| 123 | + }, |
| 124 | + }; |
| 125 | + return new HttpDispatcher(kernel); |
| 126 | +} |
| 127 | + |
| 128 | +async function wireAnswer(thrown: unknown) { |
| 129 | + const res: any = await makeDispatcher(thrown).handleActions( |
| 130 | + '/clm_contract/submit_contract/c_1', |
| 131 | + 'POST', |
| 132 | + {}, |
| 133 | + { request: {}, environmentId: 'platform', executionContext: { userId: 'u1', systemPermissions: [] } } as any, |
| 134 | + ); |
| 135 | + return res.response; |
| 136 | +} |
| 137 | + |
| 138 | +describe('[#17265] a nested sandboxed hook refusal keeps its business message', () => { |
| 139 | + it("a beforeUpdate refusal reached through ctx.api.update is NOT the sandbox's own fault", async () => { |
| 140 | + const err = await escapeOf(nestedHookRefusal); |
| 141 | + |
| 142 | + // THE defect: the marker was applied on `instanceof SandboxError`, so |
| 143 | + // the nested refusal came back as a bare fault — no business message at |
| 144 | + // all, which is exactly what the #3951 contract reads as a CRASH. |
| 145 | + expect(err.innerMessage, 'the hook refusal must survive as a business message').toBeDefined(); |
| 146 | + expect(err.innerMessage).toContain(REFUSAL); |
| 147 | + // …and the action's own debug wrapper still identifies who threw, in |
| 148 | + // the log-only `.message`, which keeps the WHOLE chain. |
| 149 | + expect(err.message).toContain("action 'submit_contract' threw:"); |
| 150 | + expect(err.message).toContain("hook 'guard_contract_submit' threw:"); |
| 151 | + |
| 152 | + // Message-NEUTRAL: this repair moves the status, not the sentence. The |
| 153 | + // client-facing text is byte-identical to what the 500 already carried |
| 154 | + // — the VM's `SandboxError: ` name prefix is a debug artefact and has |
| 155 | + // never reached the wire. |
| 156 | + expect(err.innerMessage).toBe(`hook 'guard_contract_submit' threw: ${REFUSAL}`); |
| 157 | + expect(err.innerMessage).not.toContain('SandboxError:'); |
| 158 | + }); |
| 159 | + |
| 160 | + it('the wire answer matches /data: 400 VALIDATION_ERROR with the sentence', async () => { |
| 161 | + // /data answers this refusal `400` with `error.innerMessage` verbatim |
| 162 | + // (`error-response.ts`'s sandbox unwrap door, `declared ?? 400`). The |
| 163 | + // action route must not answer a second thing for one refusal. |
| 164 | + const response = await wireAnswer(await escapeOf(nestedHookRefusal)); |
| 165 | + |
| 166 | + expect(response.status).toBe(400); |
| 167 | + expect(response.body.error.code).toBe('VALIDATION_ERROR'); |
| 168 | + expect(response.body.error.message).toContain(REFUSAL); |
| 169 | + }); |
| 170 | + |
| 171 | + it("a nested refusal's DECLARED status and code survive the hop", async () => { |
| 172 | + // The fault branch dropped the whole `__errorInfo` payload, not just |
| 173 | + // `innerMessage`, so a hook declaring `{ status: 409, code: |
| 174 | + // 'RECORD_LOCKED' }` lost both and was flattened to 500. `/data` |
| 175 | + // answers `declared ?? 400` for this producer (#9967); the action door |
| 176 | + // honours a declared status at its own first arm (#7867), so once the |
| 177 | + // classification is right the two agree without a second rule. |
| 178 | + const locked = () => { |
| 179 | + const e: any = new SandboxError( |
| 180 | + `hook 'guard_contract_submit' threw: ${REFUSAL}`, |
| 181 | + REFUSAL, |
| 182 | + { code: 'RECORD_LOCKED', status: 409 }, |
| 183 | + ); |
| 184 | + return e; |
| 185 | + }; |
| 186 | + const response = await wireAnswer(await escapeOf(locked)); |
| 187 | + |
| 188 | + expect(response.status).toBe(409); |
| 189 | + expect(response.body.error.code).toBe('RECORD_LOCKED'); |
| 190 | + expect(response.body.error.message).toContain(REFUSAL); |
| 191 | + }); |
| 192 | +}); |
| 193 | + |
| 194 | +describe('[#17265] the FAULT side of the #4431 contract is untouched', () => { |
| 195 | + it("a nested CRASH is still a fault — sandboxBusinessMessage declines it", async () => { |
| 196 | + // A hook that blew up arrives in the same shape with a native error |
| 197 | + // name inside `innerMessage`. `/data` answers the sanitised 500 for it |
| 198 | + // (#7543), so the action route must too: the business-message read is |
| 199 | + // what separates them, never the error's class. |
| 200 | + const crash = () => |
| 201 | + new SandboxError( |
| 202 | + "hook 'guard_contract_submit' threw: TypeError: cannot read properties of undefined", |
| 203 | + 'TypeError: cannot read properties of undefined', |
| 204 | + ); |
| 205 | + const response = await wireAnswer(await escapeOf(crash)); |
| 206 | + |
| 207 | + expect(response.status).toBe(500); |
| 208 | + expect(response.body.error.code).toBe('INTERNAL_ERROR'); |
| 209 | + }); |
| 210 | + |
| 211 | + it('a capability denial inside the action body is still a fault', async () => { |
| 212 | + // The #4431 case itself: the sandbox refused before user code ran, so |
| 213 | + // there is no business message to carry and the 500 must stand. |
| 214 | + const err = await runner.runScript( |
| 215 | + { |
| 216 | + language: 'js', |
| 217 | + source: "return ctx.api.object('clm_contract').count({});", |
| 218 | + capabilities: [], |
| 219 | + }, |
| 220 | + ctx({ api: { object: (_n: string) => ({ count: (_f: unknown) => 1 }) } }), |
| 221 | + actionOpts, |
| 222 | + ).then(() => null, (e: any) => e); |
| 223 | + |
| 224 | + expect(err).toBeInstanceOf(SandboxError); |
| 225 | + expect(err.innerMessage).toBeUndefined(); |
| 226 | + expect((await wireAnswer(err)).status).toBe(500); |
| 227 | + }); |
| 228 | +}); |
0 commit comments