diff --git a/.changeset/automation-resume-envelope-closed-set.md b/.changeset/automation-resume-envelope-closed-set.md new file mode 100644 index 0000000000..78ee027f21 --- /dev/null +++ b/.changeset/automation-resume-envelope-closed-set.md @@ -0,0 +1,34 @@ +--- +'@objectstack/runtime': minor +--- + +**BREAKING** — `POST /api/v1/automation/:name/runs/:runId/resume` refuses a request +body carrying an unknown top-level key. The resume body's outer envelope is now a +closed set: exactly `inputs`, `variables`, `output`, `branchLabel`. + +Until now the route read the keys it knows and silently ignored the rest, so a body +like `{"nodeId":"ask","values":{...}}` — no key of which the route reads — answered +HTTP 200 `success:true` with the screen submission treated as empty: the run +completed and the submitted value never reached the flow. A caller that guessed +`values` for the key the route spells `inputs` got silence instead of a correction. + +What changes on the wire: + +- **A body with any unknown top-level key ⇒ `400` with `error.code: + 'VALIDATION_FAILED'`.** The message names the offending key(s) and the accepted + set; `error.details.fields[]` carries one `unknown_field` entry per offending key. + The request never reaches the flow engine, the suspension is untouched, and the + same request with a corrected body is expected to succeed — this refusal sits on + the retryable side beside `INVALID_SIGNAL` and `INVALID_SCREEN_INPUT`, and is + deliberately not `FLOW_FAILED` (which the console treats as terminal, because it + means the engine consumed the suspension and the run actually ran). +- **Unchanged:** a body made only of accepted keys behaves exactly as before, + including an empty body (a legal empty submission for a screen whose declared + fields are all optional). The signal is still assembled field-by-field — never a + body spread — so the service-authority marker stays unforgeable. + +Any client already sending only the documented keys is unaffected. A client sending +extra keys alongside a correct `inputs` now gets the located 400 above instead of +having the extras silently dropped. + + diff --git a/packages/runtime/src/domains/automation-resume-envelope.test.ts b/packages/runtime/src/domains/automation-resume-envelope.test.ts new file mode 100644 index 0000000000..01bdfec554 --- /dev/null +++ b/packages/runtime/src/domains/automation-resume-envelope.test.ts @@ -0,0 +1,211 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #8796 — the resume body's OUTER envelope is a closed set. + * + * `POST /automation/:name/runs/:runId/resume` assembles its engine signal + * field-by-field from the body — deliberately (#3801: never spread the body, or + * a caller could forge the service-authority marker). What was missing was the + * other half: nothing told the caller that the keys it sent were not among the + * ones read. Measured on GA: `{"nodeId":"ask","values":{…}}` — no key of which + * the route reads — answered HTTP 200 `success:true` with the screen submission + * treated as EMPTY; the run completed and the submitted value never reached the + * flow. Maintainer ruling 2026-08-15 (Option A, on #8796): an unknown top-level + * key is refused, located, naming the offending key(s) and the accepted set — + * exactly `inputs` / `variables` / `output` / `branchLabel`. + * + * The refusal is thrown as the same duck-typed validation failure the toggle + * arm's closed set throws; both HTTP error exits map it to `400 + * VALIDATION_FAILED` + `details.fields[]` (#3918, + * `dispatcher-validation-error.test.ts` pins that mapping end-to-end for both + * exits). It is deliberately NOT `FLOW_FAILED`: the console treats 400 + * `FLOW_FAILED` as terminal — the engine consumed the suspension and ran + * (#8684, objectui PR #4899) — while this refusal never reaches the engine and + * the suspension stays live, so the caller can retry with a corrected body. It + * sits with `INVALID_SIGNAL` / `INVALID_SCREEN_INPUT` on the retryable side. + * + * Both halves of the closed-set policy are pinned here (Route & surface + * ownership rule 5): the refusal pin includes THE SERVICE WAS NEVER CALLED, and + * the preservation pin asserts the arguments the service actually received for + * a body made only of accepted keys. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import { validationFailureDetails } from '../validation-failure.js'; + +function makeDispatcher() { + const spies = { + resume: vi.fn(async () => ({ success: true, output: {}, durationMs: 7 })), + }; + const services: Record = { automation: spies }; + const resolve = (name: string) => services[name]; + const kernel: any = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + return { dispatcher: new HttpDispatcher(kernel), spies }; +} + +const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; + +const RESUME = '/flow_a/runs/run_1/resume'; + +async function expectRefusal(run: Promise, label: string) { + let thrown: unknown; + try { + await run; + } catch (e) { + thrown = e; + } + expect(thrown, `${label} was accepted instead of refused`).toBeDefined(); + const details = validationFailureDetails(thrown); + expect(details?.code).toBe('VALIDATION_FAILED'); + expect(details?.fields.length).toBeGreaterThan(0); + return thrown as Error; +} + +describe('#8796 — the resume body refuses an unknown top-level key', () => { + it('refuses the measured GA body — both keys named, engine never consulted', async () => { + const { dispatcher, spies } = makeDispatcher(); + const thrown = await expectRefusal( + dispatcher.handleAutomation( + RESUME, 'POST', + { nodeId: 'ask', values: { resolution: 'submitted via the wrong key' } }, + CTX, + ), + '{"nodeId":"ask","values":{…}}', + ); + // Located: every offending key is named, each as its own field entry. + expect(validationFailureDetails(thrown)?.fields).toMatchObject([ + { field: 'nodeId', code: 'unknown_field' }, + { field: 'values', code: 'unknown_field' }, + ]); + // The message names the offending keys AND the accepted set, so a + // guessed `values` is corrected at authoring time. + expect(thrown.message).toMatch(/`nodeId`/); + expect(thrown.message).toMatch(/`values`/); + expect(thrown.message).toMatch(/`inputs`/); + expect(thrown.message).toMatch(/`variables`/); + expect(thrown.message).toMatch(/`output`/); + expect(thrown.message).toMatch(/`branchLabel`/); + // The refusal pin's load-bearing half: the request never reached the + // engine, so the suspension was not consumed and a corrected retry is + // legitimate. + expect(spies.resume).not.toHaveBeenCalled(); + }); + + it('refuses a single guessed key and stays off FLOW_FAILED — the retryable side', async () => { + const { dispatcher, spies } = makeDispatcher(); + const thrown = await expectRefusal( + dispatcher.handleAutomation(RESUME, 'POST', { values: { x: 1 } }, CTX), + '{"values":{…}}', + ); + // ⚠️ #8684 hazard pin: the console treats 400 FLOW_FAILED as terminal + // (the wizard closes). This refusal leaves the suspension intact, so + // it must never wear that code. + expect((thrown as any).code).toBe('VALIDATION_FAILED'); + expect((thrown as any).code).not.toBe('FLOW_FAILED'); + expect(validationFailureDetails(thrown)?.fields).toMatchObject([{ field: 'values' }]); + expect(spies.resume).not.toHaveBeenCalled(); + }); + + it('refuses a half-wrong body — a valid `inputs` does not buy an unknown sibling through', async () => { + // Option B (refuse only when NOTHING is recognized) was explicitly + // declined: the half-wrong body must not be silently half-dropped. + const { dispatcher, spies } = makeDispatcher(); + const thrown = await expectRefusal( + dispatcher.handleAutomation( + RESUME, 'POST', + { inputs: { resolution: 'real value' }, values: { resolution: 'stray' } }, + CTX, + ), + '{"inputs":{…},"values":{…}}', + ); + expect(validationFailureDetails(thrown)?.fields).toMatchObject([{ field: 'values' }]); + // Ordering (#3899): the envelope refusal precedes every engine verdict + // — permission, signal, screen-input — because the engine is never + // consulted on an illegal body. + expect(spies.resume).not.toHaveBeenCalled(); + }); + + it('refuses a non-empty array body — indices are not accepted keys', async () => { + const { dispatcher, spies } = makeDispatcher(); + await expectRefusal( + dispatcher.handleAutomation(RESUME, 'POST', [{ inputs: {} }], CTX), + '[{"inputs":{}}]', + ); + expect(spies.resume).not.toHaveBeenCalled(); + }); + + it('escapes dispatch() as the recognized validation-failure shape', async () => { + // dispatch()'s catch rethrows non-permission errors on purpose — the + // HTTP envelope is built by the error exits #3918 pinned + // (`dispatcher-validation-error.test.ts`, both exits: 400, + // `error.code: 'VALIDATION_FAILED'`, `details.fields[]`). This pins + // that the refusal arrives at those exits as the shape they recognise, + // with the service still never called. + const { dispatcher, spies } = makeDispatcher(); + (dispatcher as any).timedResolveExecutionContext = async () => ({ userId: 'user_1' }); + let thrown: unknown; + try { + await dispatcher.dispatch( + 'POST', '/automation/flow_a/runs/run_1/resume', + { nodeId: 'ask', values: {} }, {}, {} as any, + ); + } catch (e) { + thrown = e; + } + expect(thrown, 'the refusal must reach the HTTP error exits').toBeDefined(); + expect(validationFailureDetails(thrown)?.code).toBe('VALIDATION_FAILED'); + expect(spies.resume).not.toHaveBeenCalled(); + }); +}); + +describe('#8796 — a body made only of accepted keys is unaffected', () => { + it('forwards all four accepted keys exactly as before', async () => { + const { dispatcher, spies } = makeDispatcher(); + const result = await dispatcher.handleAutomation( + RESUME, 'POST', + { + inputs: { new_assignee: 'ada' }, + output: { comment: 'ok' }, + branchLabel: 'approve', + }, + CTX, + ); + expect(result.response?.status).toBe(200); + // The preservation pin: the arguments the service actually received — + // #3801's field-by-field assembly, byte-for-byte what it always sent. + expect(spies.resume).toHaveBeenCalledWith('run_1', { + variables: { new_assignee: 'ada' }, + output: { comment: 'ok' }, + branchLabel: 'approve', + }); + }); + + it('keeps the `variables` alias working', async () => { + const { dispatcher, spies } = makeDispatcher(); + const result = await dispatcher.handleAutomation( + RESUME, 'POST', { variables: { note: 'hi' } }, CTX, + ); + expect(result.response?.status).toBe(200); + expect(spies.resume).toHaveBeenCalledWith('run_1', { variables: { note: 'hi' } }); + }); + + it.each([ + ['empty object', {}], + ['undefined body', undefined], + ['null body', null], + ])('still accepts %s as an empty submission', async (_label, body) => { + // An empty submission is a legal one (a screen whose declared fields + // are all optional) — the closed set refuses unknown KEYS, it does not + // demand keys exist. + const { dispatcher, spies } = makeDispatcher(); + const result = await dispatcher.handleAutomation(RESUME, 'POST', body, CTX); + expect(result.response?.status).toBe(200); + expect(spies.resume).toHaveBeenCalledWith('run_1', {}); + }); +}); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 7418231a39..e3f0de1bef 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -822,8 +822,10 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // POST /:name/runs/:runId/resume → resume a paused run (screen-flow // runtime / ADR-0019). Body `{ inputs }` = a screen node's collected // values, applied as bare flow variables; `output`/`branchLabel` also - // forwarded for approval-style resumes. Returns the next paused - // `{ screen }` (multi-screen) or the completed result. + // forwarded for approval-style resumes. The outer envelope is a CLOSED + // set — exactly the four keys below — and an unknown top-level key is + // refused (#8796). Returns the next paused `{ screen }` (multi-screen) + // or the completed result. // // The signal is built key-by-key from the JSON body on purpose (#3801): // the engine gates a suspension whose node declares @@ -862,6 +864,53 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str if (parts[1] === 'runs' && parts[2] && parts[3] === 'resume' && m === 'POST') { if (typeof automationService.resume === 'function') { const b = (body && typeof body === 'object') ? body : {}; + // [#8796] The outer envelope is a CLOSED SET (maintainer ruling + // 2026-08-15, Option A): an unknown top-level key is refused, + // located, naming the offending key(s) AND the accepted set — + // the closed-parameter-set policy (Route & surface ownership + // rule 5) applied to a request body, and the declared=enforced + // treatment #4477 gave the INNER bag, one level up. Until now + // the assembly below read the keys it knows and silently + // dropped the rest, so `{"nodeId":"ask","values":{…}}` — no + // key of which this route reads — was answered 200 + // `success:true` with the submission treated as EMPTY: the run + // completed and the submitted value never reached the flow. A + // caller that guesses `values` instead of `inputs` now gets a + // correction at authoring time instead of silence. + // + // The refusal WRAPS #3801's field-by-field signal assembly, it + // never replaces it with a body spread — the service-authority + // marker stays unforgeable exactly as before. + // + // Deliberately BEFORE the `resume()` call: nothing reaches the + // engine until the body is legal (#3899, the same ordering the + // toggle arm above enforces), so this refusal composes AHEAD + // of every engine verdict — a body that is both malformed and + // unauthorized answers the envelope 400 and the suspension is + // never consulted, let alone consumed. + // + // Thrown as the duck-typed validation failure both dispatcher + // error exits map to 400 `VALIDATION_FAILED` + `fields[]` + // (#3918) — the same wire shape the toggle arm's closed set + // answers. NOT `FLOW_FAILED` (#8684, a few lines down): the + // console treats 400 `FLOW_FAILED` as terminal (the engine + // consumed the suspension and ran — objectui PR #4899), while + // this refusal leaves the suspension intact and the caller can + // retry with a corrected body. It sits with `INVALID_SIGNAL` / + // `INVALID_SCREEN_INPUT` on the retryable side. + const RESUME_BODY_KEYS = ['inputs', 'variables', 'output', 'branchLabel']; + const unknownKeys = Object.keys(b).filter((k) => !RESUME_BODY_KEYS.includes(k)); + if (unknownKeys.length > 0) { + const accepted = RESUME_BODY_KEYS.map((k) => `\`${k}\``).join(', '); + throw validationFailure( + `Unknown key${unknownKeys.length > 1 ? 's' : ''} ${unknownKeys.map((k) => `\`${k}\``).join(', ')} — the resume body accepts ${accepted}`, + unknownKeys.map((k) => ({ + field: k, + code: 'unknown_field', + message: `not a resume body key — the resume body accepts ${accepted}`, + })), + ); + } const inputs = (b.inputs ?? b.variables); const signal: any = {}; if (inputs && typeof inputs === 'object') signal.variables = inputs;