From 50305f4dab2c4ba58689204a1463b449c3dcd5e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 02:01:27 +0000 Subject: [PATCH 1/2] fix(runtime): an action or job body refused for a missing capability answers the same coded 403 a hook body does, not a bare 500 One gate, one answer: throwCapabilityDenial no longer sends an action or job origin to throwSandboxFault. The remedy names the body's own owner from the origin kind. The capability check is unchanged. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5opKoaTFmNRbYvKobt --- ...65-action-job-capability-denial-refusal.md | 12 ++ .../import-runner-sandbox-fault-row.test.ts | 198 +++++++++++++----- .../action-capability-refusal.dogfood.test.ts | 85 ++++++++ .../actions-fault-vs-rejection.test.ts | 6 +- ...ty-denial-is-a-refusal.integration.test.ts | 198 ++++++++++++++++++ ...=> capability-denial-is-a-refusal.test.ts} | 147 ++++++------- ...ty-denial-is-a-refusal.integration.test.ts | 5 +- ...nested-hook-refusal-is-a-rejection.test.ts | 21 +- .../runtime/src/sandbox/quickjs-runner.ts | 65 +++--- .../sandboxed-action-crash-is-a-fault.test.ts | 6 +- 10 files changed, 576 insertions(+), 167 deletions(-) create mode 100644 .changeset/26965-action-job-capability-denial-refusal.md create mode 100644 packages/qa/dogfood/test/action-capability-refusal.dogfood.test.ts create mode 100644 packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts rename packages/runtime/src/sandbox/{capability-denial-is-a-fault.test.ts => capability-denial-is-a-refusal.test.ts} (51%) diff --git a/.changeset/26965-action-job-capability-denial-refusal.md b/.changeset/26965-action-job-capability-denial-refusal.md new file mode 100644 index 00000000000..268f98beeb6 --- /dev/null +++ b/.changeset/26965-action-job-capability-denial-refusal.md @@ -0,0 +1,12 @@ +--- +'@objectstack/runtime': patch +--- + +fix(runtime): an action or job body refused for a missing capability answers the same coded 403 a hook body does, not a bare 500 (#26965) + +Clause-②: no +Metadata-protocol: none + +**What was wrong.** Since #26903 a hook body that calls something its `body.capabilities` does not grant is refused `403 INSUFFICIENT_PRIVILEGES`, naming the hook, the capability and the remedy. The same call from an action body (for example `ctx.api.object('note').find(...)` without `api.read`) still answered `POST /api/v1/actions/OBJECT/ACTION` with `500 INTERNAL_ERROR` "Internal server error", so the caller and the author were not told which action or which capability. A job body's denial was the same bare fault. + +**What changes.** One gate, one answer. An action or job body's capability denial is the same refusal: `403 INSUFFICIENT_PRIVILEGES`, an existing standard catalog code, with a sentence such as `capability 'api.read' not granted to action 'note_count' (called ctx.api.object('note').find). Grant it by adding 'api.read' to the action's body.capabilities.` The remedy names the body's own owner (the hook's, the action's or the job's `body.capabilities`). The capability check is exactly as strict as before; nothing the body asked for runs. An action's refusal is logged once at `info`; a job reaches no HTTP caller, so its run records this sentence and its `error` log line stays. diff --git a/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts b/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts index 217e1fcfd82..05b1926986e 100644 --- a/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts +++ b/packages/core/src/utils/import-runner-sandbox-fault-row.test.ts @@ -56,18 +56,18 @@ * names the requested object to the door's `500`, and §3 holds the controls * that keep the heuristic's other answers where they were. * - * [#26903] The capability gates call `throwCapabilityDenial(vm, origin, - * capability, message)`, not `throwSandboxFault` directly. Each of those calls - * is still a site here, its message rendered from argument 3, but with an - * ACTION origin: the helper sends an action or job body's denial to - * `throwSandboxFault` (the fault this file pins) and answers a HOOK body's as - * a coded `403` refusal, which is not a fault and is pinned by - * `runtime/src/sandbox/hook-capability-denial-is-a-refusal.integration.test.ts`. - * The census reads that routing from the helper's body rather than trusting - * this comment: if its first statement stops sending every non-hook origin to - * `throwSandboxFault`, the census is red. The helper's own - * `throwSandboxFault(vm, message)` relays its call sites, like the - * `new SandboxError(message)` inside `throwSandboxFault`. + * [#26903/#26965] The capability gates call `throwCapabilityDenial(vm, origin, + * capability, message)`, not `throwSandboxFault`. Since #26965 that helper + * answers EVERY origin's denial (hook, action, job) as a coded refusal, so a + * capability gate is no longer a fault site: §4 holds each one to the refusal + * the runner hands out. The census reads both halves from the producer rather + * than trusting this comment: + * + * - the helper's body calls no `throwSandboxFault` (it has no fault arm), and + * no fault site's message reads as a capability denial; + * - each gate's message (argument 3) is rendered as before, then run through + * the helper's own sentence template (its `new Error(…)`) with the code and + * status its `Object.assign` declares, once per origin kind. */ import { readFileSync } from 'node:fs'; @@ -105,18 +105,15 @@ const SAMPLES: Readonly> = { fieldName: 'owner', }; -/** - * [#26903] A `throwCapabilityDenial` site is a fault only on the non-hook arm, - * so its message is rendered with an action origin (see the header). - */ -const CAPABILITY_FAULT_SAMPLES: Readonly> = { - ...SAMPLES, - 'origin.kind': 'action', -}; - /** The helper every capability gate throws through, read below from the producer. */ const CAPABILITY_HELPER = 'throwCapabilityDenial'; +/** [#26965] Every origin a body runs under answers a denial the same way; §4 renders each. */ +const ORIGIN_KINDS = ['hook', 'action', 'job'] as const; + +/** A placeholder for `origin.kind` in a gate's rendered message, filled per kind in §4. */ +const ORIGIN_KIND_HOLE = '@@origin.kind@@'; + /** §2: the error a nested body threw, so the construction it lands in is a CRASH. */ const CRASH_SAMPLES: Readonly> = { ...SAMPLES, @@ -130,7 +127,7 @@ const CRASH_SAMPLES: Readonly> = { * runner, or a new non-literal one, fails §1's census. */ const RELAYS: Readonly> = { - message: '`throwSandboxFault` builds the error for each of its call sites, and `throwCapabilityDenial` hands it each of its own (action and job origins); both helpers\' call sites are sites here in their own right', + message: '`throwSandboxFault` builds the error for each of its call sites, which are sites here in their own right', 'withoutSandboxErrorPrefix(String(errStr))': 'the pump loop rebuilds a fault marked across the VM hop: the `throwSandboxFault` messages, the host-side faults thrown inside a host call (sites here), and a nested body\'s crash (§2)', }; @@ -139,7 +136,6 @@ const RELAYS: Readonly> = { const KINDS: ReadonlyArray = [ ['CPU budget', /exceeded CPU budget/, 1], ['wall-clock ceiling', /exceeded wall-clock ceiling/, 1], - ['capability denial', /^capability '/, 5], ['ctx.api unavailable', /^ctx\.api unavailable/, 1], ['install failure', /^failed to install/, 4], ['marshal failure', /^failed to marshal/, 1], @@ -207,7 +203,7 @@ const census = (() => { const crashes: Site[] = []; const relays: string[] = []; const unrenderable: string[] = []; - const capabilityGates: string[] = []; + const capabilityGates: CapabilityGate[] = []; const visit = (node: ts.Node): void => { if (ts.isNewExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'SandboxError') { const args = node.arguments ?? []; @@ -246,15 +242,21 @@ const census = (() => { } } } - // [#26903] A capability gate: a fault on the action/job arm (see the header). + // [#26965] A capability gate: a refusal for every origin, held by §4. + // Its message is rendered per origin kind there; `origin.kind` stays a + // placeholder here. if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === CAPABILITY_HELPER) { const at = `quickjs-runner.ts:${lineOf(node)}`; try { - faults.push({ - label: `${at} ${CAPABILITY_HELPER}(…), action origin`, - message: render(node.arguments[3], CAPABILITY_FAULT_SAMPLES), + capabilityGates.push({ + label: `${at} ${CAPABILITY_HELPER}(…)`, + message: render(node.arguments[3], { ...SAMPLES, 'origin.kind': ORIGIN_KIND_HOLE }), + // The capability is a literal token, or the `required` parameter a + // gate passes on (its sample is the token it is called with). + capability: ts.isIdentifier(node.arguments[2]) && node.arguments[2].text in SAMPLES + ? SAMPLES[node.arguments[2].text] + : render(node.arguments[2], SAMPLES), }); - capabilityGates.push(at); } catch (e) { unrenderable.push(`${at}: ${(e as Error).message}`); } @@ -265,28 +267,85 @@ const census = (() => { return { faults, crashes, relays, unrenderable, capabilityGates }; })(); +/** The value a `const NAME = ` declaration in the producer holds. */ +function constValue(name: string): string | number | undefined { + let found: string | number | undefined; + const visit = (node: ts.Node): void => { + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.name.text === name && node.initializer) { + const init = node.initializer; + if (ts.isStringLiteral(init)) found = init.text; + else if (ts.isNumericLiteral(init)) found = Number(init.text); + } + ts.forEachChild(node, visit); + }; + visit(SOURCE); + return found; +} + /** - * [#26903] Does the capability helper send every non-hook origin to - * `throwSandboxFault` before anything else? Read from its body: the first - * statement must be `if (origin.kind !== 'hook') throwSandboxFault(vm, message)` - * (the parameter names read from its own signature). + * [#26965] The capability helper, read from its body: whether it can still + * reach `throwSandboxFault`, the sentence template its `new Error(…)` builds, + * and the `code` / `status` its `Object.assign` declares. */ -function capabilityHelperFaultsNonHook(): boolean { - let ok = false; +const capabilityHelper = (() => { + let callsFault = false; + let template: ts.Expression | undefined; + let code: unknown; + let status: unknown; + const valueOf = (e: ts.Expression): unknown => { + if (ts.isStringLiteral(e)) return e.text; + if (ts.isNumericLiteral(e)) return Number(e.text); + if (ts.isIdentifier(e)) return constValue(e.text); + return undefined; + }; + const visitBody = (node: ts.Node): void => { + if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'throwSandboxFault') callsFault = true; + if (ts.isNewExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === 'Error') template = node.arguments?.[0]; + if (ts.isPropertyAssignment(node) && ts.isIdentifier(node.name)) { + if (node.name.text === 'code') code = valueOf(node.initializer); + if (node.name.text === 'status') status = valueOf(node.initializer); + } + ts.forEachChild(node, visitBody); + }; + let params: string[] = []; const visit = (node: ts.Node): void => { if (ts.isFunctionDeclaration(node) && node.name?.text === CAPABILITY_HELPER && node.body) { - const [vm, origin, , message] = node.parameters.map((p) => p.name.getText()); - const first = node.body.statements[0]; - ok = first !== undefined - && ts.isIfStatement(first) - && first.elseStatement === undefined - && first.expression.getText() === `${origin}.kind !== 'hook'` - && first.thenStatement.getText().replace(/;$/, '') === `throwSandboxFault(${vm}, ${message})`; + params = node.parameters.map((p) => p.name.getText()); + visitBody(node.body); } ts.forEachChild(node, visit); }; visit(SOURCE); - return ok; + const [, origin, capability, message] = params; + /** The sentence the helper builds for one gate under one origin kind. */ + const sentence = (gate: CapabilityGate, kind: string): string => { + if (template === undefined) throw new Error(`${CAPABILITY_HELPER} builds no \`new Error(…)\``); + const gateText = gate.message.split(ORIGIN_KIND_HOLE).join(kind); + return render(template, { + [`${message}.replace(/\\.$/, '')`]: gateText.replace(/\.$/, ''), + [message]: gateText, + [capability]: gate.capability, + [`${origin}.kind`]: kind, + }); + }; + return { callsFault, code, status, sentence, found: params.length === 4 }; +})(); + +interface CapabilityGate extends Site { + capability: string; +} + +/** + * [#26965] The error the runner hands out for a denied capability: the pump + * loop's relay of a coded refusal — the ` '' threw:` wrapper in + * `.message` for the log, the sentence in `innerMessage`, and the code and + * status the refusal carried across the VM. + */ +function capabilityRefusal(gate: CapabilityGate, kind: string): Error { + const text = capabilityHelper.sentence(gate, kind); + const err = new Error(`${kind} '${SAMPLES['origin.name']}' threw: ${PRODUCER_NAME}: ${text}`); + err.name = PRODUCER_NAME ?? 'unread'; + return Object.assign(err, { innerMessage: text, code: capabilityHelper.code, status: capabilityHelper.status }); } const PRODUCER_NAME = producerName(); @@ -296,6 +355,11 @@ function namingTheObject(): Site[] { return census.faults.filter((s) => s.message.includes(`'${OBJECT}'`)); } +/** [#26965] The capability gates whose rendered message quotes the requested object. */ +function gatesNamingTheObject(): CapabilityGate[] { + return census.capabilityGates.filter((s) => s.message.includes(`'${OBJECT}'`)); +} + /** The producer's own fault: its declared `name`, and no `innerMessage`. */ function ownFault(message: string): Error { const err = new Error(message); @@ -349,21 +413,24 @@ describe('[#22741] §1 — every fault the runner raises itself reads the door\' expect(PRODUCER_NAME, 'the `this.name = …` assignment in SandboxError\'s constructor').toBeTypeOf('string'); expect(census.unrenderable, 'sites this pin cannot render; add a sample or a relay').toEqual([]); expect([...new Set(census.relays)].sort()).toEqual(Object.keys(RELAYS).sort()); - // [#26903] The capability gates throw through the helper, and the helper - // keeps their action/job arm on `throwSandboxFault`, the fault rows below. + // [#26965] The capability gates throw through the helper, and the helper + // has no fault arm: no gate is a fault row here (§4 holds them). expect(census.capabilityGates.length, `${CAPABILITY_HELPER} call sites`).toBeGreaterThanOrEqual(5); - expect(capabilityHelperFaultsNonHook(), `${CAPABILITY_HELPER} sends every non-hook origin to throwSandboxFault first`).toBe(true); + expect(capabilityHelper.found, `${CAPABILITY_HELPER} is declared with (vm, origin, capability, message)`).toBe(true); + expect(capabilityHelper.callsFault, `${CAPABILITY_HELPER} reaches throwSandboxFault`).toBe(false); + expect(census.faults.filter((s) => /^capability '/.test(s.message)), 'a capability denial raised as a fault').toEqual([]); for (const [kind, pattern, floor] of KINDS) { const n = census.faults.filter((s) => pattern.test(s.message)).length; expect(n, `${kind}: sites found`).toBeGreaterThanOrEqual(floor); } // §2's population: the constructions that set `innerMessage` (crash and refusal sites). expect(census.crashes.length).toBeGreaterThanOrEqual(4); - // [#22868] The sites that name the requested object: the `ctx.api.object(…)` - // capability gate and the missing-method arm, each held to `500` below. + // [#22868] The fault site that names the requested object: the + // missing-method arm, held to `500` below. [#26965] The `ctx.api.object(…)` + // capability gate names it too, and §4 holds it to its `403`. const named = namingTheObject(); - expect(named.filter((s) => /^capability '/.test(s.message)).length, 'capability denial').toBeGreaterThanOrEqual(1); expect(named.filter((s) => /not implemented$/.test(s.message)).length, 'missing ctx.api method').toBeGreaterThanOrEqual(1); + expect(gatesNamingTheObject().length, 'the ctx.api.object(…) capability gate').toBeGreaterThanOrEqual(1); }); it.each(census.faults.map((s) => [s.label, s.message] as const))('%s', async (_label, message) => { @@ -426,7 +493,10 @@ describe('[#22741] §3 — controls: what the predicate must NOT change', () => it('CONTROL [#22868] — the same object-naming texts, thrown by anything but the sandbox, keep the heuristic\'s 404', () => { // The door asks the producer's identity, never the words: no second rule. - const named = namingTheObject(); + const named: Site[] = [ + ...namingTheObject(), + ...gatesNamingTheObject().map((g) => ({ label: g.label, message: g.message.split(ORIGIN_KIND_HOLE).join('hook') })), + ]; expect(named.length).toBeGreaterThanOrEqual(2); for (const { label, message } of named) { const err = new Error(message); @@ -461,3 +531,31 @@ describe('[#22741] §3 — controls: what the predicate must NOT change', () => } }); }); + +describe('[#26965] §4 — a capability denial is a coded refusal for every origin, on the door and on the row', () => { + it('the helper declares the standard catalog code at its status', () => { + expect(capabilityHelper.code).toBe('INSUFFICIENT_PRIVILEGES'); + expect(capabilityHelper.status).toBe(403); + }); + + const cases = census.capabilityGates.flatMap((gate) => ORIGIN_KINDS.map((kind) => [`${gate.label}, ${kind} origin`, gate, kind] as const)); + + it.each(cases)('%s', async (_label, gate, kind) => { + const sentence = capabilityHelper.sentence(gate, kind); + // The remedy names the body's own owner, never another origin's. + expect(sentence).toContain(`capability '${gate.capability}' not granted to ${kind} '`); + expect(sentence).toContain(`Grant it by adding '${gate.capability}' to the ${kind}'s body.capabilities.`); + + const err = capabilityRefusal(gate, kind); + expect(isSandboxOwnFault(err)).toBe(false); + // The door answers the refusal, never the fault's 500 nor, for the gate + // that names the requested object, the unknown-object heuristic's 404. + const door = mapDataError(err, OBJECT); + expect(door.status).toBe(403); + expect(door.body).toMatchObject({ error: sentence, code: 'INSUFFICIENT_PRIVILEGES' }); + + // The row stays a row error, reading the refusal's own sentence and code. + const row = await rowFor(() => capabilityRefusal(gate, kind)); + expect(row).toEqual({ row: 1, ok: false, action: 'failed', error: sentence, code: 'INSUFFICIENT_PRIVILEGES' }); + }); +}); diff --git a/packages/qa/dogfood/test/action-capability-refusal.dogfood.test.ts b/packages/qa/dogfood/test/action-capability-refusal.dogfood.test.ts new file mode 100644 index 00000000000..2e3c5603881 --- /dev/null +++ b/packages/qa/dogfood/test/action-capability-refusal.dogfood.test.ts @@ -0,0 +1,85 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#26965] An action body refused for a missing capability, end to end through +// the real stack: QuickJS capability gate → the action body runner → the +// `/actions` door → the HTTP body a client reads. +// +// The gate stays as strict as it is. What this file pins is the ANSWER: an +// action whose body calls `ctx.api` without `api.read` used to get +// `500 INTERNAL_ERROR` ("Internal server error"), while a hook making the same +// call got `403 INSUFFICIENT_PRIVILEGES` naming the hook and the capability +// (#26903). One gate, one answer: the action's denial is the same coded +// refusal now, naming the action and the capability, with the remedy on the +// action's own `body.capabilities`. The same body WITH the capability runs. +// +// The composition pins (every gate, action and job, the log lines) live in +// `packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts`; +// this file is the one place the whole HTTP chain runs for an action. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; + +const READ_SOURCE = "const rows = await ctx.api.object('acr_note').find({}); return { seen: rows.length };"; + +const acrStack = defineStack({ + manifest: { + id: 'com.dogfood.action-capability-refusal', + namespace: 'acr', + version: '0.0.0', + type: 'app', + name: 'Action Capability Refusal Fixture', + description: 'Sandboxed actions calling ctx.api with and without the api.read capability.', + }, + objects: [ + ObjectSchema.create({ name: 'acr_note', label: 'Note', fields: { title: Field.text({ label: 'Title', required: true }) } }), + ], + actions: [ + { + name: 'acr_count_denied', + label: 'Count notes (no capability)', + objectName: 'acr_note', + type: 'script', + body: { language: 'js', source: READ_SOURCE, capabilities: [] }, + }, + { + name: 'acr_count_granted', + label: 'Count notes', + objectName: 'acr_note', + type: 'script', + body: { language: 'js', source: READ_SOURCE, capabilities: ['api.read'] }, + }, + ], +} as any); + +describe('objectstack verify: an action refused for a missing capability (#26965)', () => { + let stack: VerifyStack; + let token: string; + + beforeAll(async () => { + stack = await bootStack(acrStack); + token = await stack.signIn(); + }, 60_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('POST /actions answers 403 INSUFFICIENT_PRIVILEGES naming the action and api.read', async () => { + const r = await stack.apiAs(token, 'POST', '/actions/acr_note/acr_count_denied', {}); + const body = (await r.json()) as any; + expect(r.status, JSON.stringify(body)).toBe(403); + expect(body.error.code).toBe('INSUFFICIENT_PRIVILEGES'); + expect(body.error.message).toContain("capability 'api.read' not granted to action 'acr_count_denied'"); + expect(body.error.message).toContain("adding 'api.read' to the action's body.capabilities"); + expect(JSON.stringify(body)).not.toMatch(/threw:|SandboxError|Internal server error/); + }); + + it('control — the same body WITH api.read runs', async () => { + const r = await stack.apiAs(token, 'POST', '/actions/acr_note/acr_count_granted', {}); + const body = (await r.json()) as any; + expect(r.status, JSON.stringify(body)).toBe(200); + expect(JSON.stringify(body)).toContain('"seen":0'); + }); +}); diff --git a/packages/runtime/src/domains/actions-fault-vs-rejection.test.ts b/packages/runtime/src/domains/actions-fault-vs-rejection.test.ts index 35b77965e27..ca9ec682a6a 100644 --- a/packages/runtime/src/domains/actions-fault-vs-rejection.test.ts +++ b/packages/runtime/src/domains/actions-fault-vs-rejection.test.ts @@ -178,10 +178,10 @@ describe('an unexpected FAULT is a 500', () => { expect(response.body.error.message).toBe('Internal server error'); }); - it("the sandbox's OWN internal errors — a timeout, a capability denial", async () => { + it("the sandbox's OWN internal errors — a timeout, a marshalling failure", async () => { // A `SandboxError` with no `innerMessage` is the sandbox failing, not - // user code rejecting: a timeout, a denied capability, a marshalling - // failure. Precisely the class an operator wants to alert on, and + // user code rejecting: a timeout, a marshalling failure. (A denied + // capability was one until #26965; it is a coded 403 refusal now.) Precisely the class an operator wants to alert on, and // precisely what a 200 made invisible. const response = await invoke(new SandboxError('action timed out after 5000ms')); diff --git a/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts b/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts new file mode 100644 index 00000000000..1560a88d1b1 --- /dev/null +++ b/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#26965] An ACTION or JOB body's capability denial is the same coded refusal + * a hook body's is: one gate, one answer. + * + * ## What was measured broken + * + * After #26903, a hook body calling `ctx.api.object('n').find(…)` without + * `api.read` was refused `403 INSUFFICIENT_PRIVILEGES`, naming the hook, the + * capability and the remedy. The same call from an action body still crossed + * the VM as the sandbox's own fault (#4431), so `POST /actions/OBJECT/ACTION` + * answered `500 INTERNAL_ERROR` `Internal server error`: neither the caller nor + * the author learned which action or which capability. + * + * ## The line this file pins + * + * - The gate is exactly as strict as before: the denied call never runs and + * nothing is written. + * - Action: the real `/actions` door answers `403 INSUFFICIENT_PRIVILEGES`, + * whose sentence names the action, the capability and the remedy on the + * action's own `body.capabilities`. One gate per token. The BodyRunner logs + * it once on the refusal side of its band (`info`, coded, no stack). + * - Job: a job reaches no HTTP caller. Its handler rejects with the same coded + * refusal, the remedy naming the job's `body.capabilities`; that `message` is + * what a job run record stores as its `error` (`DbJobAdapter.finishRun`), and + * the BodyRunner keeps its `error` line for the operator. + * - Control: the same body WITH the capability runs. + * + * Measured in composition: a real `ObjectQL` on a real SQLite driver, the real + * `QuickJSScriptRunner` and body-runner factories, and the real `/actions` + * door (`HttpDispatcher.handleActions`), whose engine double only hands the + * door's action context to the bound handler, as `ObjectQL.executeAction` does. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { mapDataError } from '@objectstack/types'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { actionBodyRunnerFactory, jobBodyRunnerFactory } from './body-runner.js'; +import { QuickJSScriptRunner } from './quickjs-runner.js'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const NOTE = { + name: 'capa_note', + fields: { title: { type: 'text', name: 'title' } }, +}; + +interface Line { level: 'debug' | 'info' | 'warn' | 'error'; msg: string; err?: unknown; meta?: any } + +/** A `Logger`-contract double (`error` is `(message, error, meta)`) that records every call. */ +function recordingLogger() { + const lines: Line[] = []; + return { + lines, + debug: (msg: string, meta?: any) => { lines.push({ level: 'debug', msg, meta }); }, + info: (msg: string, meta?: any) => { lines.push({ level: 'info', msg, meta }); }, + warn: (msg: string, meta?: any) => { lines.push({ level: 'warn', msg, meta }); }, + error: (msg: string, err?: unknown, meta?: any) => { lines.push({ level: 'error', msg, err, meta }); }, + }; +} + +/** One gate per `HookBodyCapability` token: the call that trips it, as a body. */ +const GATES = [ + { capability: 'api.read', source: "await ctx.api.object('capa_note').find({}); return 1;" }, + { capability: 'api.write', source: "await ctx.api.object('capa_note').insert({ title: 'nested' }); return 1;" }, + { capability: 'api.transaction', source: 'await ctx.api.transaction(async () => {}); return 1;' }, + { capability: 'crypto.uuid', source: 'return ctx.crypto.randomUUID();' }, + { capability: 'log', source: "ctx.log.info('stamping'); return 1;" }, +] as const; + +describe('[#26965] an action or job body\'s capability denial is a coded refusal', () => { + let engine: ObjectQL | null = null; + let dir: string | null = null; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = null; + if (dir) { rmSync(dir, { recursive: true, force: true }); dir = null; } + }); + + async function boot(): Promise { + dir = mkdtempSync(join(tmpdir(), 'os-body-cap-26965-')); + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: join(dir, 'data.sqlite') }, useNullAsDefault: true }); + await driver.initObjects([NOTE]); + engine = new ObjectQL(); + engine.registerDriver(driver as any, true); + await engine.init(); + engine.registry.registerObject(NOTE as any); + return engine; + } + + /** The real `/actions` door, dispatching to the action body bound on the real engine. */ + async function throughActionsDoor(e: ObjectQL, source: string, capabilities: string[]) { + const bodyLog = recordingLogger(); + const action = { name: 'capa_probe', objectName: 'capa_note', type: 'script', body: { language: 'js', source, capabilities } }; + const handler = actionBodyRunnerFactory(new QuickJSScriptRunner(), { ql: e, appId: 'capa', logger: bodyLog })(action as any); + expect(handler, 'the action body must bind').toBeTypeOf('function'); + const objectDef = { name: 'capa_note', actions: [action] }; + const ql: any = { + executeAction: vi.fn(async (_o: string, _a: string, ctx: any) => handler!(ctx)), + getSchema: (n: string) => (n === objectDef.name ? objectDef : undefined), + registry: { getObject: (n: string) => (n === objectDef.name ? objectDef : undefined), getItem: () => undefined }, + find: vi.fn(async () => []), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const metadata: any = { + load: vi.fn(async () => null), + listObjects: vi.fn(async () => [objectDef]), + getObject: vi.fn(async () => objectDef), + }; + const kernel: any = { + context: { getService: (n: string) => (n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : null) }, + }; + const res: any = await new HttpDispatcher(kernel).handleActions( + '/capa_note/capa_probe', + 'POST', + {}, + { request: {}, environmentId: 'platform', executionContext: { userId: 'u1', systemPermissions: [] } } as any, + ); + expect(ql.executeAction, 'the door must reach the bound body').toHaveBeenCalledTimes(1); + return { response: res.response, bodyLog }; + } + + function bindJob(e: ObjectQL, source: string, capabilities: string[]) { + const bodyLog = recordingLogger(); + const handler = jobBodyRunnerFactory(new QuickJSScriptRunner(), { ql: e, appId: 'capa', logger: bodyLog })({ + name: 'capa_nightly', + body: { language: 'js', source, capabilities }, + }); + expect(handler, 'the job body must bind').toBeTypeOf('function'); + return { handler: handler!, bodyLog }; + } + + for (const gate of GATES) { + it(`action — '${gate.capability}' denied: POST /actions answers 403 INSUFFICIENT_PRIVILEGES naming action and capability`, async () => { + const e = await boot(); + const { response, bodyLog } = await throughActionsDoor(e, gate.source, []); + + expect(response.status, JSON.stringify(response.body)).toBe(403); + expect(response.body.error.code).toBe('INSUFFICIENT_PRIVILEGES'); + const sentence = String(response.body.error.message); + expect(sentence).toContain(`capability '${gate.capability}' not granted to action 'capa_probe'`); + expect(sentence).toContain(`Grant it by adding '${gate.capability}' to the action's body.capabilities.`); + expect(sentence).not.toMatch(/threw:|SandboxError|Internal server error|hook's/); + + expect((await e.find('capa_note', {})).length).toBe(0); + + const refused = bodyLog.lines.filter((l) => l.msg.includes('sandboxed action refused')); + expect(refused).toHaveLength(1); + expect(refused[0].level).toBe('info'); + expect(refused[0].meta).toMatchObject({ action: 'capa_probe', code: 'INSUFFICIENT_PRIVILEGES', status: 403 }); + expect(bodyLog.lines.filter((l) => l.level === 'error')).toEqual([]); + }, 30_000); + } + + for (const gate of GATES) { + it(`action control — the same body WITH '${gate.capability}' answers 200`, async () => { + const e = await boot(); + const { response, bodyLog } = await throughActionsDoor(e, gate.source, [gate.capability]); + expect(response.status, JSON.stringify(response.body)).toBe(200); + expect(bodyLog.lines.filter((l) => /refused|threw/.test(l.msg))).toEqual([]); + }, 30_000); + } + + for (const gate of GATES) { + it(`job — '${gate.capability}' denied: the run rejects with the coded refusal naming job and capability`, async () => { + const e = await boot(); + const { handler, bodyLog } = bindJob(e, gate.source, []); + + const err: any = await handler().then(() => null, (x) => x); + expect(err, 'the denied call must fail the run').not.toBeNull(); + expect(err.code).toBe('INSUFFICIENT_PRIVILEGES'); + expect(err.status).toBe(403); + // What `DbJobAdapter` stores as the run's `error` is this `message`. + expect(String(err.message)).toContain(`capability '${gate.capability}' not granted to job 'capa_nightly'`); + expect(String(err.message)).toContain(`Grant it by adding '${gate.capability}' to the job's body.capabilities.`); + // Any door that ever answers it reads the same refusal. + const { status, body } = mapDataError(err); + expect(status).toBe(403); + expect(body.code).toBe('INSUFFICIENT_PRIVILEGES'); + + expect((await e.find('capa_note', {})).length).toBe(0); + // The operator keeps the job's error line. + expect(bodyLog.lines.filter((l) => l.level === 'error' && l.msg.includes('sandboxed job threw'))).toHaveLength(1); + }, 30_000); + } + + it('job control — the same body WITH the capability runs', async () => { + const e = await boot(); + const { handler, bodyLog } = bindJob(e, GATES[0].source, ['api.read']); + await expect(handler()).resolves.toBeUndefined(); + expect(bodyLog.lines.filter((l) => l.level === 'error')).toEqual([]); + }, 30_000); +}); diff --git a/packages/runtime/src/sandbox/capability-denial-is-a-fault.test.ts b/packages/runtime/src/sandbox/capability-denial-is-a-refusal.test.ts similarity index 51% rename from packages/runtime/src/sandbox/capability-denial-is-a-fault.test.ts rename to packages/runtime/src/sandbox/capability-denial-is-a-refusal.test.ts index e5d9144d0f5..ba5df45c612 100644 --- a/packages/runtime/src/sandbox/capability-denial-is-a-fault.test.ts +++ b/packages/runtime/src/sandbox/capability-denial-is-a-refusal.test.ts @@ -1,35 +1,39 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#4431] A capability denial is the sandbox FAULTING, not the body rejecting. + * [#26965] An in-VM capability denial is a coded REFUSAL, for every origin. * - * The `action-crash-vs-rejection` contract (#3951) pins the table: + * ## History + * + * [#4431] The `action-crash-vs-rejection` contract (#3951) pins the table: * * | `SandboxError` WITH `innerMessage` — a body's deliberate throw | 400 | - * | `SandboxError` with NO `innerMessage` — timeout, capability denial | 500 | + * | `SandboxError` with NO `innerMessage` — timeout, internal fault | 500 | * - * A capability gate throws `SandboxError` synchronously inside a QuickJS host - * function, which rejects the async IIFE *inside* the VM — so it came back - * through the `__error` side-channel, and the pump loop presumed everything - * arriving there was user code throwing on purpose. It therefore set - * `innerMessage`, the dispatcher's classifier read that as a deliberate - * rejection and answered **400**, and the client got the `SandboxError: ` debug - * prefix that only ever belonged in server logs. + * A capability gate throws synchronously inside a QuickJS host function, which + * rejects the async IIFE *inside* the VM, so it comes back through the + * `__error` side-channel. The pump loop once presumed everything arriving there + * was user code throwing on purpose and served the denial as a deliberate 400 + * carrying the `SandboxError: ` debug prefix. #4431 fixed that by marking the + * denial as the sandbox's own fault: no `innerMessage`, so a bare + * `500 INTERNAL_ERROR`. * - * The contract's own claim — "capability denial has no user-meaningful inner - * message" — held only for denials detected OUTSIDE evaluation (a timeout, - * which takes the separate `budgetError` path). These tests pin the in-VM - * host-call denials that were misclassified: `ctx.api.*`, `ctx.log`, - * `ctx.crypto`, `ctx.api.transaction`. + * [#26903] then ruled that a HOOK body's denial is a refusal: the body's own + * metadata did not grant what it called, and the caller is owed a coded answer + * naming the hook and the capability. [#26965] ruled one gate, one answer: an + * ACTION or JOB body's denial is the same refusal, `403 + * INSUFFICIENT_PRIVILEGES` (a standard catalog code), with the remedy naming + * that body's own `body.capabilities`. The gate is exactly as strict as before. * - * The HTTP half of the contract is asserted by - * `domains/actions-fault-vs-rejection.test.ts`, which already states that a - * `SandboxError` with no `innerMessage` is a 500 — this file is what makes a - * real capability denial actually arrive in that shape. + * ## What this file pins * - * [#26903] These are ACTION-origin denials. A HOOK body's denial is a coded - * refusal (`403 INSUFFICIENT_PRIVILEGES`) since that card, pinned by - * `hook-capability-denial-is-a-refusal.integration.test.ts`. + * The error the runner hands the classifier for each in-VM gate (`ctx.api.*`, + * `ctx.log`, `ctx.crypto`, `ctx.api.transaction`) under an action origin: its + * `innerMessage` is the refusal sentence, without the debug wrapper or the + * `SandboxError: ` prefix, and it carries `code` and `status`. The wire half + * through the real `/actions` door (and the job arm) is + * `action-job-capability-denial-is-a-refusal.integration.test.ts`; the hook + * arm is `hook-capability-denial-is-a-refusal.integration.test.ts`. */ import { describe, it, expect } from 'vitest'; @@ -43,17 +47,33 @@ function ctx(over: Partial = {}): ScriptContext { return { input: {}, ...over }; } -async function faultOf(run: () => Promise): Promise { +async function refusalOf(run: () => Promise): Promise { const e = await run().then(() => null, (err) => err as SandboxError); expect(e, 'expected the sandbox to refuse, but the script resolved').toBeInstanceOf(SandboxError); return e!; } -describe('[#4431] an in-VM capability denial reaches the classifier as a FAULT', () => { +/** The coded refusal every gate answers with, read off the error the runner hands the classifier. */ +function expectCodedRefusal(err: SandboxError, capability: string): void { + // The caller's sentence: which capability, whose, and the remedy on the + // ACTION's own `body.capabilities` (never the hook's). + expect(err.innerMessage).toContain(`capability '${capability}' not granted to action 'rc1_crash_probe'`); + expect(err.innerMessage).toContain(`Grant it by adding '${capability}' to the action's body.capabilities.`); + // Neither the debug wrapper nor the VM's name prefix reaches it. + expect(err.innerMessage).not.toMatch(/threw:|SandboxError/); + // The standard catalog's code at its status: what `mapDataError`'s sandbox + // unwrap arm and the `/actions` door's declared-status branch serve. + expect(err.code).toBe('INSUFFICIENT_PRIVILEGES'); + expect(err.status).toBe(403); + expect(err.fields).toBeUndefined(); + expect(err.name).toBe('SandboxError'); +} + +describe('[#26965] an in-VM capability denial reaches the classifier as a coded REFUSAL', () => { const api = { object: (_n: string) => ({ count: (_f: unknown) => 1 }) }; - it("ctx.api.object(x).count without api.read — the issue's exact repro", async () => { - const err = await faultOf(() => + it("ctx.api.object(x).count without api.read — #4431's exact repro", async () => { + const err = await refusalOf(() => runner.runScript( { language: 'js', @@ -65,71 +85,40 @@ describe('[#4431] an in-VM capability denial reaches the classifier as a FAULT', ), ); - // THE fix: no `innerMessage`. `domains/actions.ts` reads its absence (plus - // the non-`Error` name) as an unexpected fault → `errorFromThrown(err, 500)`. - // With it set, the denial was served as a deliberate 400 and stayed - // invisible to gateway error rates, APM and alerting. - expect(err.innerMessage).toBeUndefined(); - - // The debug prefix must not reach the OPERATOR'S LOG line either. Every - // assertion in this block is about the THROWN error — what the runner hands - // the classifier and what `domains/actions.ts` prints — never about the wire. - // [#18540] Since the door's unexpected-fault terminal answers - // `INTERNAL_ERROR_MESSAGE`, what the CLIENT sees for this denial is - // `Internal server error`, the same sentence `/data` answers; this text is - // the diagnostic half of that split, and it has to stay readable. - expect(err.message).not.toContain('SandboxError:'); - // …while the actionable content survives for the log: which capability, - // whose, and the call that tripped the gate. - expect(err.message).toContain("capability 'api.read' not granted"); - expect(err.message).toContain("action 'rc1_crash_probe'"); - expect(err.message).toContain("ctx.api.object('showcase_task').count"); - - // Nor does it get the ` '' threw:` wrapper — nothing threw; the - // sandbox refused before user code could run the call. - expect(err.message).not.toContain('threw:'); - - // No `code`/`fields` either: both are "structured domain failure" markers - // the classifier reads as a REJECTION, so carrying one would re-break the - // 500 the same way `innerMessage` did. - expect(err.code).toBeUndefined(); - expect(err.fields).toBeUndefined(); - - // The classifier's own predicate, spelled out. - expect(err.name).toBe('SandboxError'); + // [#26965 — FLIPPED] This used to pin `innerMessage` undefined and no + // `code`: the #4431 fault, served as a bare `500 Internal server error`. + expectCodedRefusal(err, 'api.read'); + // The call that tripped the gate is named too. + expect(err.innerMessage).toContain("ctx.api.object('showcase_task').count"); }); it('ctx.log without the log capability', async () => { // Four members since #7661 — `debug` is a real level of this seam, not a // decoration, so a host double that omits it no longer type-checks. const log = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; - const err = await faultOf(() => + const err = await refusalOf(() => runner.runScript( { language: 'js', source: "ctx.log.info('hi'); return 1;", capabilities: [] }, ctx({ log }), actionOpts, ), ); - expect(err.innerMessage).toBeUndefined(); - expect(err.message).not.toContain('SandboxError:'); - expect(err.message).toContain("capability 'log' not granted"); + expectCodedRefusal(err, 'log'); }); it('ctx.crypto.randomUUID without the crypto.uuid capability', async () => { - const err = await faultOf(() => + const err = await refusalOf(() => runner.runScript( { language: 'js', source: 'return ctx.crypto.randomUUID();', capabilities: [] }, ctx(), actionOpts, ), ); - expect(err.innerMessage).toBeUndefined(); - expect(err.message).not.toContain('SandboxError:'); - expect(err.message).toContain("capability 'crypto.uuid' not granted"); + expectCodedRefusal(err, 'crypto.uuid'); }); it('ctx.api.transaction without the api.transaction capability', async () => { - const err = await faultOf(() => + const err = await refusalOf(() => runner.runScript( { language: 'js', @@ -140,9 +129,21 @@ describe('[#4431] an in-VM capability denial reaches the classifier as a FAULT', actionOpts, ), ); - expect(err.innerMessage).toBeUndefined(); - expect(err.message).not.toContain('SandboxError:'); - expect(err.message).toContain("capability 'api.transaction' not granted"); + expectCodedRefusal(err, 'api.transaction'); + }); + + it('a JOB body names the job\'s body.capabilities', async () => { + const err = await refusalOf(() => + runner.runScript( + { language: 'js', source: 'return ctx.crypto.randomUUID();', capabilities: [] }, + ctx(), + { origin: { kind: 'job', name: 'nightly_rollup' } }, + ), + ); + expect(err.innerMessage).toContain("capability 'crypto.uuid' not granted to job 'nightly_rollup'"); + expect(err.innerMessage).toContain("Grant it by adding 'crypto.uuid' to the job's body.capabilities."); + expect(err.code).toBe('INSUFFICIENT_PRIVILEGES'); + expect(err.status).toBe(403); }); it('a denial the body CATCHES and rethrows as its own error stays a rejection', async () => { @@ -150,7 +151,7 @@ describe('[#4431] an in-VM capability denial reaches the classifier as a FAULT', // error rather than a match on the flattened `SandboxError: …` text: once // user code has caught the denial and thrown its own business error, the // outcome IS a deliberate rejection and must keep its 400. - const err = await faultOf(() => + const err = await refusalOf(() => runner.runScript( { language: 'js', @@ -170,7 +171,7 @@ describe('[#4431] an in-VM capability denial reaches the classifier as a FAULT', describe('[#4431] the rejection side of the contract is untouched', () => { it('a deliberate throw still carries innerMessage and the debug wrapper', async () => { - const err = await faultOf(() => + const err = await refusalOf(() => runner.runScript( { language: 'js', source: "throw new Error('线索信息不完整');", capabilities: [] }, ctx(), @@ -197,7 +198,7 @@ describe('[#4431] the rejection side of the contract is untouched', () => { }, }), }; - const err = await faultOf(() => + const err = await refusalOf(() => runner.runScript( { language: 'js', diff --git a/packages/runtime/src/sandbox/hook-capability-denial-is-a-refusal.integration.test.ts b/packages/runtime/src/sandbox/hook-capability-denial-is-a-refusal.integration.test.ts index 8a1f16c59c5..64028346d2f 100644 --- a/packages/runtime/src/sandbox/hook-capability-denial-is-a-refusal.integration.test.ts +++ b/packages/runtime/src/sandbox/hook-capability-denial-is-a-refusal.integration.test.ts @@ -22,8 +22,9 @@ * `log` is recorded (the hook wrapper's `[hook] handler failed` line), naming * the hook and the capability, and the save proceeds as `log` promises. * - Control: the same body WITH the capability runs as it always has. - * - An ACTION body's denial keeps the #4431 fault contract - * (`capability-denial-is-a-fault.test.ts`); this card ruled on hooks only. + * - [#26965] An ACTION or JOB body's denial is the same refusal, its remedy + * naming that body's own `body.capabilities` + * (`action-job-capability-denial-is-a-refusal.integration.test.ts`). * * Measured in composition — a real `ObjectQL`, a real SQLite driver, the real * `QuickJSScriptRunner` and hook wrapper — and read off `mapDataError`, the diff --git a/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts b/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts index 733926fd5c3..ff89d8bccd7 100644 --- a/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts +++ b/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts @@ -30,7 +30,7 @@ * * The marker's own sibling pin already named this risk — "a marker applied too * broadly would turn every failed write into a 500" - * (`capability-denial-is-a-fault.test.ts`) — and measured it with a plain + * (`capability-denial-is-a-refusal.test.ts`, then named `-is-a-fault`) — and measured it with a plain * `ValidationError`, which is not a `SandboxError` and so never tripped the * `instanceof`. A NESTED sandboxed refusal is. * @@ -38,8 +38,9 @@ * * The discriminator is the one `/data` asks (`sandboxBusinessMessage`, #11588): * does the error carry a caller-addressed business sentence that is not a - * script fault? A refusal does and stays a rejection; a capability denial and a - * nested CRASH do not and stay faults. So `/data`'s answer and the action + * script fault? A refusal does and stays a rejection; a nested CRASH does not + * and stays a fault. [#26965] A capability denial is a coded refusal (`403 + * INSUFFICIENT_PRIVILEGES`) since that card, no longer the #4431 fault. So `/data`'s answer and the action * route's answer are the same answer, per route and per status. */ @@ -214,9 +215,10 @@ describe('[#17265] the FAULT side of the #4431 contract is untouched', () => { expect(response.body.error.code).toBe('INTERNAL_ERROR'); }); - it('a capability denial inside the action body is still a fault', async () => { - // The #4431 case itself: the sandbox refused before user code ran, so - // there is no business message to carry and the 500 must stand. + it('[#26965] a capability denial inside the action body is a coded 403 refusal, not the #4431 fault', async () => { + // The #4431 case itself, FLIPPED by #26965: one gate, one answer. The + // action body's denial is the refusal a hook body's is (#26903), + // naming the action, the capability and the remedy. const err = await runner.runScript( { language: 'js', @@ -228,7 +230,10 @@ describe('[#17265] the FAULT side of the #4431 contract is untouched', () => { ).then(() => null, (e: any) => e); expect(err).toBeInstanceOf(SandboxError); - expect(err.innerMessage).toBeUndefined(); - expect((await wireAnswer(err)).status).toBe(500); + expect(err.innerMessage).toContain("capability 'api.read' not granted to action 'submit_contract'"); + const response = await wireAnswer(err); + expect(response.status).toBe(403); + expect(response.body.error.code).toBe('INSUFFICIENT_PRIVILEGES'); + expect(response.body.error.message).toBe(err.innerMessage); }); }); diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index 6534d3728e5..e31f4067d8c 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -475,9 +475,9 @@ export class QuickJSScriptRunner implements ScriptRunner { const budget = budgetError(pumps); if (budget) throw budget; const info = readErrorInfo(vm); - // [#4431] A SANDBOX fault that crossed `__error` — a capability - // denial thrown synchronously inside a host function, an unavailable - // `ctx.api`, a marshalling failure. It is not an outcome the body + // [#4431] A SANDBOX fault that crossed `__error` — an unavailable + // `ctx.api`, a marshalling failure, any other host-side fault thrown + // synchronously inside a host function. It is not an outcome the body // chose to report, so it gets neither the ` '' threw:` // wrapper (nothing threw — the sandbox refused) nor an // `innerMessage` (there is no business message; `SandboxError`'s own @@ -486,9 +486,10 @@ export class QuickJSScriptRunner implements ScriptRunner { // as a CRASH and answers 500 through `errorFromThrown`, instead of // the 400 a denial used to get — and the client sees the capability // text without the `SandboxError: ` debug prefix. - // [#26903] A HOOK body's capability denial does not arrive here: it - // crosses as a coded refusal (`throwCapabilityDenial`) and takes the - // relay below like a body's own coded throw. + // [#26903/#26965] A capability denial does not arrive here, whatever + // the body's origin: it crosses as a coded refusal + // (`throwCapabilityDenial`) and takes the relay below like a body's + // own coded throw. if (info?.sandboxFault) { throw new SandboxError(withoutSandboxErrorPrefix(String(errStr))); } @@ -1688,6 +1689,11 @@ function releaseEngineRefusalHandles(vm: QuickJSContext, bodyPromise: QuickJSHan * denials — `ctx.api.*`, `ctx.log`, `ctx.crypto`, `ctx.api.transaction` — were * misclassified. * + * [#26903/#26965] Capability denials have since left this marker for every + * origin: they cross as a coded `403` refusal ({@link throwCapabilityDenial}). + * The marker still carries the other host-side faults (an unavailable + * `ctx.api`, a marshalling failure). + * * ## Why a marker and not the name * * `__error` is a FLATTENED `: ` string by design (every existing @@ -1801,20 +1807,22 @@ function throwSandboxFault(vm: QuickJSContext, message: string): never { } /** - * [#26903] The code and status a HOOK body's capability denial is answered - * with: the standard catalog's `INSUFFICIENT_PRIVILEGES` ("operation requires - * higher privileges", `@objectstack/spec` `errors.zod.ts`), at its 403. + * [#26903/#26965] The code and status a body's capability denial is answered + * with, for every origin: the standard catalog's `INSUFFICIENT_PRIVILEGES` + * ("operation requires higher privileges", `@objectstack/spec` + * `errors.zod.ts`), at its 403. */ -const HOOK_CAPABILITY_REFUSAL_CODE = 'INSUFFICIENT_PRIVILEGES'; -const HOOK_CAPABILITY_REFUSAL_STATUS = 403; +const CAPABILITY_REFUSAL_CODE = 'INSUFFICIENT_PRIVILEGES'; +const CAPABILITY_REFUSAL_STATUS = 403; /** * [#26903] Throw a capability gate's denial OUT OF a host function. * - * In a HOOK body the denial is a REFUSAL, not the sandbox faulting: the hook's - * metadata did not grant what its body called, and the caller whose save fired - * the hook is owed a coded answer naming the hook and the capability, not the - * sanitised `500 INTERNAL_ERROR` {@link throwSandboxFault} reaches. So the VM + * The denial is a REFUSAL, not the sandbox faulting: the body's own metadata + * did not grant what it called, and the caller (whose save fired the hook, or + * who invoked the action) is owed a coded answer naming the body and the + * capability, not the sanitised `500 INTERNAL_ERROR` {@link throwSandboxFault} + * reaches. So the VM * error carries `code` and `status` the way a body's own * `throw Object.assign(new Error(…), { code, status })` does, and the pump loop * builds the same `SandboxError` (with `innerMessage`) it builds for that @@ -1829,8 +1837,11 @@ const HOOK_CAPABILITY_REFUSAL_STATUS = 403; * `innerMessage` is what it relays as a NESTED body's refusal (which the outer * catch then does not log). Neither is true of this denial. * - * An action or job body's denial keeps the #4431 fault contract - * ({@link SANDBOX_FAULT_PROP}); this card ruled on hooks only. + * [#26965] One gate, one answer: an action or job body's denial is the same + * refusal (it was the #4431 fault, a bare 500, until that card). The remedy + * names the owner from the origin — the hook's, the action's or the job's + * `body.capabilities` — here, once. A job reaches no HTTP caller; its run + * records this sentence, and the BodyRunner keeps its `error` line for it. */ function throwCapabilityDenial( vm: QuickJSContext, @@ -1838,10 +1849,9 @@ function throwCapabilityDenial( capability: HookBodyCapability, message: string, ): never { - if (origin.kind !== 'hook') throwSandboxFault(vm, message); const refusal = Object.assign( - new Error(`${message.replace(/\.$/, '')}. Grant it by adding '${capability}' to the hook's body.capabilities.`), - { name: 'SandboxError', code: HOOK_CAPABILITY_REFUSAL_CODE, status: HOOK_CAPABILITY_REFUSAL_STATUS }, + new Error(`${message.replace(/\.$/, '')}. Grant it by adding '${capability}' to the ${origin.kind}'s body.capabilities.`), + { name: 'SandboxError', code: CAPABILITY_REFUSAL_CODE, status: CAPABILITY_REFUSAL_STATUS }, ); throw hostErrorToVm(vm, refusal); } @@ -2018,13 +2028,12 @@ export class SandboxError extends Error { * message without the ` '' threw:` debug wrapper that lives in * `.message`. Safe to surface to end users (e.g. an action's error toast); * the wrapped `.message` stays for server logs. Undefined for the sandbox's - * own internal errors (capability denials, timeouts, marshalling failures), - * which have no user-meaningful inner message. + * own internal errors (timeouts, marshalling failures), which have no + * user-meaningful inner message. * - * [#26903] Except a HOOK body's capability denial: that is a refusal naming - * the hook and the capability, so it carries this field and its `code`/ - * `status` (see `throwCapabilityDenial`). An action or job body's denial is - * still the fault described above. + * [#26903/#26965] A capability denial is not one of those: it is a refusal + * naming the body and the capability, so it carries this field and its + * `code`/`status` (see `throwCapabilityDenial`), whatever the body's origin. */ readonly innerMessage?: string; /** @@ -2077,8 +2086,8 @@ export interface SandboxErrorInfo { /** [commit 79c46da90] See {@link SandboxError.userMessage}. */ userMessage?: string; /** - * [#4431] The error that crossed `__error` was the SANDBOX's own fault — a - * denied capability, an unavailable `ctx.api`, a marshalling failure — not + * [#4431] The error that crossed `__error` was the SANDBOX's own fault — an + * unavailable `ctx.api`, a marshalling failure — not * user code rejecting. Read by the pump loop, which then leaves * `innerMessage` undefined so the #3951 crash contract's "SandboxError with * no innerMessage → 500" holds for in-VM host-call denials too. diff --git a/packages/runtime/src/sandbox/sandboxed-action-crash-is-a-fault.test.ts b/packages/runtime/src/sandbox/sandboxed-action-crash-is-a-fault.test.ts index 7f5776aec25..a1c9b6e669d 100644 --- a/packages/runtime/src/sandbox/sandboxed-action-crash-is-a-fault.test.ts +++ b/packages/runtime/src/sandbox/sandboxed-action-crash-is-a-fault.test.ts @@ -5,9 +5,9 @@ * * `domains/actions-fault-vs-rejection.test.ts` pins the HTTP half of the * crash-vs-rejection contract against a synthesised `SandboxError`. This file - * is the other half, the split `capability-denial-is-a-fault.test.ts` already - * draws for #4431: it runs a body that genuinely crashes inside QuickJS and - * proves the error ARRIVES in the shape the classifier is pinned against — + * is the other half, the split `capability-denial-is-a-refusal.test.ts` drew + * for #4431 (a capability denial is a coded refusal since #26965): it runs a + * body that genuinely crashes inside QuickJS and proves the error ARRIVES in the shape the classifier is pinned against — * then hands that same object, untouched, to the real door. * * The card this closes was filed on a READING of the source, and read-derived From af5816664b56471a8dedf5150c419901a3c0bef2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 02:18:59 +0000 Subject: [PATCH 2/2] test(runtime): call the job handler with its run context Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5opKoaTFmNRbYvKobt --- ...ion-job-capability-denial-is-a-refusal.integration.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts b/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts index 1560a88d1b1..75ea0c4a418 100644 --- a/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts +++ b/packages/runtime/src/sandbox/action-job-capability-denial-is-a-refusal.integration.test.ts @@ -171,7 +171,7 @@ describe('[#26965] an action or job body\'s capability denial is a coded refusal const e = await boot(); const { handler, bodyLog } = bindJob(e, gate.source, []); - const err: any = await handler().then(() => null, (x) => x); + const err: any = await handler({ jobId: 'run_1' }).then(() => null, (x) => x); expect(err, 'the denied call must fail the run').not.toBeNull(); expect(err.code).toBe('INSUFFICIENT_PRIVILEGES'); expect(err.status).toBe(403); @@ -192,7 +192,7 @@ describe('[#26965] an action or job body\'s capability denial is a coded refusal it('job control — the same body WITH the capability runs', async () => { const e = await boot(); const { handler, bodyLog } = bindJob(e, GATES[0].source, ['api.read']); - await expect(handler()).resolves.toBeUndefined(); + await expect(handler({ jobId: 'run_1' })).resolves.toBeUndefined(); expect(bodyLog.lines.filter((l) => l.level === 'error')).toEqual([]); }, 30_000); });