From 3df2342615e50f4c03a84654c9bba1796874acf5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 01:03:11 +0000 Subject: [PATCH] =?UTF-8?q?feat(runtime)!:=20the=20undeclared-action=20ref?= =?UTF-8?q?usal=20has=20no=20opt-out=20=E2=80=94=20drop=20OS=5FALLOW=5FUND?= =?UTF-8?q?ECLARED=5FACTIONS=20(ADR-0110=20D3=20revised)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D3 as accepted refused an undeclared handler but shipped a migration valve that ran it anyway, slated for removal in 18. Removed before 17 ships. It contradicts the ruling it accompanies: a flag that executes an ungoverned, system-elevated handler IS the fail-open D3 closes, and ADR-0049's trichotomy has no 'enforced unless a flag says otherwise' state. And it had no observed users — a reconciliation sweep across platform packages, every example and every plugin found the only engine.registerAction call sites are app-todo's eight, all declared. What the valve bought is covered without it: the app still boots, declared actions still work, D5's inventory names each offender at startup, and the 404 names the defineAction to add. A regression test pins that the retired variable has no effect, so a stale deployment script fails loudly instead of silently reopening the gate. ADR-0110 carries a dated revision note rather than a rewritten history; D6 now says explicitly that its strongest form is no flag at all. runtime 888, metadata 276, objectql 1163 — each by exit code. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01DTTKgYriDB6RVrkYjxxnG1 --- .changeset/drop-undeclared-actions-valve.md | 26 ++++++++ content/docs/releases/v17.mdx | 18 ++++-- content/docs/ui/actions.mdx | 8 ++- ...tion-identity-and-declaration-admission.md | 59 +++++++++++++------ packages/runtime/src/action-execution.ts | 12 ---- packages/runtime/src/app-plugin.ts | 6 +- packages/runtime/src/domains/actions.ts | 42 ++++++------- ...p-dispatcher.actions-type-dispatch.test.ts | 21 ++++--- 8 files changed, 122 insertions(+), 70 deletions(-) create mode 100644 .changeset/drop-undeclared-actions-valve.md diff --git a/.changeset/drop-undeclared-actions-valve.md b/.changeset/drop-undeclared-actions-valve.md new file mode 100644 index 0000000000..c5e052260d --- /dev/null +++ b/.changeset/drop-undeclared-actions-valve.md @@ -0,0 +1,26 @@ +--- +'@objectstack/runtime': major +--- + +**[ADR-0110 D3, revised] The undeclared-action refusal has no opt-out — +`OS_ALLOW_UNDECLARED_ACTIONS` is removed before 17 ships.** + +D3 as accepted refused an undeclared handler but shipped +`OS_ALLOW_UNDECLARED_ACTIONS=1` as a migration valve that ran it anyway, +"slated for removal in 18". Removed now, for two reasons: + +- **It contradicts the ruling it accompanies.** A flag that executes an + ungoverned, system-elevated handler *is* the fail-open D3 closes. ADR-0049's + trichotomy has no "enforced unless a flag says otherwise" state. +- **It had no observed users.** A reconciliation sweep across the platform + packages, every example and every plugin found the only `engine.registerAction` + call sites are `app-todo`'s eight, all declared. The valve would have shipped + a documented way to reopen the gate for a population nobody has ever seen. + +What it was buying is covered without it: the app still boots, every declared +action still works, D5's boot inventory names each offender at startup, and the +404 names the `defineAction` to add. Migration costs a code change rather than +an env var — the correct price for reopening an authorization gate. + +Setting the retired variable has no effect; a regression test pins that, so a +stale deployment script fails loudly rather than silently re-opening the gate. diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index bd365ae23e..c4a4b088ee 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -188,11 +188,19 @@ Three changes land together: **Migration:** boot logs list every registered-but-undeclared handler under `[action-governance]`, alongside declared script actions bound to no handler. -Declare each one. `OS_ALLOW_UNDECLARED_ACTIONS=1` runs them meanwhile, warning -on every invocation; **it is removed in 18**. Apps whose actions are all -declared — anything with working Console buttons — need no changes, other than -gaining enforcement of the `requiredPermissions` they already declared. Callers -that hard-coded a `target` in an action URL switch to the action's `name`. +Declare each one with `defineAction`, or drop the registration if nothing should +invoke it over HTTP. **There is no opt-out flag** — a switch that ran an +ungoverned handler would be the same fail-open this change closes, and a sweep +of the platform, every package and every example found zero undeclared handlers, +so it would have shipped a way to reopen the gate for a case nobody has +observed. The failure is bounded: the app still boots and every declared action +still works; only an undeclared one returns 404, and it names the `defineAction` +to add. + +Apps whose actions are all declared — anything with working Console buttons — +need no changes, other than gaining enforcement of the `requiredPermissions` +they already declared. Callers that hard-coded a `target` in an action URL +switch to the action's `name`. ### A flow run with no trigger user may not touch data (#3760) diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index c33cb25380..f7d41ca869 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -307,9 +307,11 @@ If server-side logic should never be reachable over HTTP at all, do not register it as an action — export a plain function and call it from your own code. `engine.registerAction` means "publish this on the HTTP and MCP surfaces". -Migrating an app that has undeclared handlers? Boot logs list every one of them -under `[action-governance]`, and `OS_ALLOW_UNDECLARED_ACTIONS=1` runs them -(warning each time) while you add the declarations. That valve is removed in 18. +Migrating an app that has undeclared handlers? Startup lists every one of them +under `[action-governance]`, with the object and key to declare. There is no +flag that runs them meanwhile — one would be the same ungoverned execution this +rule exists to prevent — so declare each one, or drop the registration if +nothing should invoke it over HTTP. ## Expose it to AI (MCP) diff --git a/docs/adr/0110-action-identity-and-declaration-admission.md b/docs/adr/0110-action-identity-and-declaration-admission.md index 26bb9350d5..93447dce32 100644 --- a/docs/adr/0110-action-identity-and-declaration-admission.md +++ b/docs/adr/0110-action-identity-and-declaration-admission.md @@ -52,9 +52,8 @@ on every surface — `target` is never an identity key; **D2** every dispatch surface resolves the declaration *first* and derives handler-key candidates *from* it (the MCP order becomes the shared order); **D3** declaration resolution is a trichotomy — found → gate-then-dispatch, metadata plane -unreachable → 503, genuinely undeclared → **refuse, fail-closed in 17**, -with `OS_ALLOW_UNDECLARED_ACTIONS=1` as a documented migration valve slated -for removal in 18; **D4** "declared but hidden" is the blessed pattern for +unreachable → 503, genuinely undeclared → **refuse, fail-closed in 17, +with no opt-out** (see the D3 revision note); **D4** "declared but hidden" is the blessed pattern for headless actions — "undeclared but executable" has no remaining legitimate use; **D5** a reconciliation lint + boot check: every `registerAction` key must reconcile to a declaration; **D6** security-gate strictness is @@ -217,7 +216,7 @@ documented curl 404s. |---|---| | **Declaration found** | Gate against it, then dispatch (D2). | | **Metadata plane unreachable** | **503.** An availability failure is not an authorization decision; the gate an author declared must not evaporate during an outage. (The ADR-0096 posture — "no context" is a defect, never an authorization — applied to "no declaration source".) | -| **Genuinely undeclared** | **Refuse** (404) with a prescriptive error: `Action 'x' on 'y' has no declaration — add defineAction({ name: 'x', … }) or register it under a declared action's target`. Migration valve: `OS_ALLOW_UNDECLARED_ACTIONS=1` executes it and warns per invocation. | +| **Genuinely undeclared** | **Refuse** (404) with a prescriptive error: `Action 'x' on 'y' has no declaration — add defineAction({ name: 'x', … }) or register it under a declared action's target`. No opt-out — see the revision note below. | **Fail-closed in 17 — no cross-major staging.** An earlier draft staged this over two majors (warn → CI-only → flip). Protocol 17 is in RC *now*, so the @@ -243,14 +242,30 @@ Three things must ship **together, in 17**, or the refusal is undiagnosable: one it is a checklist. Plus a `content/docs/releases/v17.mdx` migration entry, alongside its two -siblings, naming the valve and its removal target. - -**The valve is temporary by construction.** `OS_ALLOW_UNDECLARED_ACTIONS=1` -exists so an upgrade is never blocked at 3am, not so the old behaviour -survives. It warns on every invocation (not just at boot), is documented as -a migration aid rather than a supported mode, and is **slated for removal in -18** — recorded here so it does not silently become permanent, which is the -usual fate of escape hatches with no stated end. +siblings, naming the inventory as the migration path. + +> **Revision (2026-07-30, before 17 shipped) — the migration valve is gone.** +> As accepted, D3 shipped `OS_ALLOW_UNDECLARED_ACTIONS=1`: an opt-out that ran +> the undeclared handler anyway, warning per invocation, "slated for removal in +> 18". It was removed before the release on two grounds. +> +> **It contradicts the ruling it accompanies.** A flag that executes an +> ungoverned, system-elevated handler *is* the fail-open D3 closes. Shipping it +> would have preserved the hole in configurable form, and ADR-0049's trichotomy +> does not have a "enforced unless a flag says otherwise" state. +> +> **It had no observed users.** A reconciliation sweep across the platform +> packages, every example and every plugin found the only `engine.registerAction` +> call sites are `app-todo`'s eight, all declared. The valve would have shipped +> a documented way to reopen the gate for a population nobody has ever seen — +> and escape hatches with no users are the ones that quietly become permanent, +> which the removal-in-18 note was itself an admission of. +> +> What the valve was buying is covered without it: the app still boots, every +> declared action still works, D5's inventory names each offender at startup, +> and the 404 names the `defineAction` to add. The migration costs a code +> change rather than an env var — which, for reopening an authorization gate, +> is the correct price. The deletion inversion closes in 17: deleting a declaration moves the action from "gated" straight to "refused". Removal narrows, immediately. @@ -288,13 +303,21 @@ agent can act on. `OS_ACTION_PARAMS_STRICT_ENABLED` (opt-**in** strict) is an acceptable shape for a *param-contract* ratchet (DX concern, warn-first). It is not an -acceptable shape for an *authorization* gate: D3's refusal stages toward -**strict-by-default with an opt-out escape hatch**, and any future gate -follows the same direction. A flag spelled `OS_ALLOW_*` (opt out of -enforcement) is the sanctioned shape for security; `OS_*_STRICT_ENABLED` +acceptable shape for an *authorization* gate: an enforcement that ships off +for everyone who never read the release notes is not enforcement. A flag +spelled `OS_ALLOW_*` (opt **out** of enforcement) is the sanctioned shape +where a security escape hatch is warranted at all; `OS_*_STRICT_ENABLED` (opt in to enforcement) is reserved for non-security contracts. Existing flags are not renamed by this ADR; new ones conform. +**The strongest form of this rule is no flag at all.** D3 originally carried +an `OS_ALLOW_UNDECLARED_ACTIONS` opt-out — correctly *spelled* under this +ruling, and still wrong, because what it opted out of was the gate itself +rather than a strictness dial around it (see the D3 revision note). So D6 +governs the naming and direction of a security flag *where one is +justified*; it is not a licence to add one. An authorization boundary with a +documented bypass is the boundary its bypass describes. + ## Alternatives considered - **Make the route resolve declarations by `target` too.** Rejected: it @@ -361,8 +384,8 @@ ones); the engine-side aliasing alternative (deferred); any change to how non-breaking for every correctly-declared action. 2. **D5** — reconciliation lint + boot inventory, so orphans are findable before the refusal exists. -3. **D3/D6** — refusal ON by default, `OS_ALLOW_UNDECLARED_ACTIONS` as the - documented valve, plus the `v17.mdx` migration entry. Same PR flips this +3. **D3/D6** — refusal ON by default with no opt-out, plus the `v17.mdx` + migration entry. Same PR flips this ADR to `Accepted — implemented` with evidence (the PRIORITIZATION status-hygiene rule). diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 2c3eaebe8a..549802d186 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -29,18 +29,6 @@ function actionParamsStrict(): boolean { return typeof process !== 'undefined' && process.env?.OS_ACTION_PARAMS_STRICT_ENABLED === '1'; } -/** - * [ADR-0110 D3/D6] Migration valve for executing an action that has no - * declaration. Enforcement is the DEFAULT and this opts OUT of it — the - * direction matters: a security gate whose strictness is opt-in (as - * `OS_ACTION_PARAMS_STRICT_ENABLED` is, acceptably, for a DX contract) ships - * open for everyone who never read the release notes. `OS_ALLOW_*` is the - * sanctioned shape for a security escape hatch; it warns on every invocation - * and is slated for removal in 18. - */ -export function undeclaredActionsAllowed(deps: ActionExecutionDeps): boolean { - return typeof process !== 'undefined' && process.env?.OS_ALLOW_UNDECLARED_ACTIONS === '1'; -} const _warnedActionParams = new Set(); function warnActionParamsOnce(key: string, message: string): void { diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index b12dae7eba..ab0b312358 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -721,9 +721,9 @@ export class AppPlugin implements Plugin { ); if (undeclaredHandlers.length > 0) { ctx.logger.warn( - '[action-governance] registered handlers with NO declaration — these are refused ' + - 'at dispatch (ADR-0110 D3); declare each one with `defineAction`, or set ' + - 'OS_ALLOW_UNDECLARED_ACTIONS=1 to run them during migration (removed in 18)', + '[action-governance] registered handlers with NO declaration — these are REFUSED ' + + 'at dispatch (ADR-0110 D3) and there is no opt-out; declare each one with ' + + '`defineAction`, or drop the registration if nothing should invoke it over HTTP', { appId, count: undeclaredHandlers.length, diff --git a/packages/runtime/src/domains/actions.ts b/packages/runtime/src/domains/actions.ts index ead2ea33bd..78b693c9fc 100644 --- a/packages/runtime/src/domains/actions.ts +++ b/packages/runtime/src/domains/actions.ts @@ -185,27 +185,29 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string // A handler with no declaration is invisible to every governance // surface — ADR-0066 D4 has no `requiredPermissions` to read, ADR-0104 // no param contract, ADR-0109 materialises no `action_` tool — - // yet it executes TRUSTED. Refuse, and say what to add. The valve is - // for an upgrade that cannot stop to declare one at 3am; it warns on - // every invocation and is slated for removal in 18. + // yet it executes TRUSTED. Refuse, and say what to add. + // + // There is no opt-out. An earlier draft shipped + // `OS_ALLOW_UNDECLARED_ACTIONS` as a migration valve slated for removal + // in 18; it was dropped before 17 went out. A flag that runs an + // ungoverned handler IS the fail-open this ruling exists to close, so + // keeping one would have preserved the hole in configurable form — + // and a reconciliation sweep found the platform, every package and + // every example carry zero undeclared handlers, so it would have + // shipped a documented way to reopen the gate for a population nobody + // has ever observed. The boot inventory (D5) names the offenders and + // the 404 below names the fix, which is the whole migration path. if (!actionDef) { - if (!actionExec.undeclaredActionsAllowed(deps)) { - return { - handled: true, - response: deps.error( - `Action '${actionName}' on '${objectName}' has no declaration — ` + - `add \`defineAction({ name: '${actionName}', … })\`, or register the handler under a ` + - `declared action's \`target\`. Undeclared handlers cannot be permission-gated ` + - `(ADR-0110 D3); set OS_ALLOW_UNDECLARED_ACTIONS=1 to run it during migration.`, - 404, - ), - }; - } - console.warn( - `[action-governance] UNDECLARED action '${objectName}/${actionName}' executed under ` + - `OS_ALLOW_UNDECLARED_ACTIONS — it is ungated (no requiredPermissions, no param contract) ` + - `and invisible to the AI surface. Declare it; the valve is removed in 18.`, - ); + return { + handled: true, + response: deps.error( + `Action '${actionName}' on '${objectName}' has no declaration — ` + + `add \`defineAction({ name: '${actionName}', … })\`, or register the handler under a ` + + `declared action's \`target\`. Undeclared handlers cannot be permission-gated ` + + `(ADR-0110 D3); startup logs every one of them under [action-governance].`, + 404, + ), + }; } // ── 1/3: declared → gate against it ── diff --git a/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts b/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts index b0ee5aa9f8..760002a84e 100644 --- a/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts +++ b/packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts @@ -353,8 +353,7 @@ describe('REST /actions — script dispatch is unchanged (#3915 regression guard // [ADR-0110 D3] An UNDECLARED action used to run here, ungated — this test // asserted exactly that. A handler with no declaration has no // `requiredPermissions` to enforce, no param contract, and materialises no - // `action_` tool, yet it executes TRUSTED; it now refuses. The valve - // test below is the only path that still runs it. + // `action_` tool, yet it executes TRUSTED; it now refuses. it('refuses an UNDECLARED action with a prescriptive error instead of running it ungated', async () => { const { dispatcher, executeAction } = makeDispatcher({ objectDef: { name: 'crm_lead', actions: [] } }); @@ -363,24 +362,28 @@ describe('REST /actions — script dispatch is unchanged (#3915 regression guard expect(res.response.status).toBe(404); expect(res.response.body.error.message).toMatch(/has no declaration/i); expect(res.response.body.error.message).toMatch(/defineAction\(\{ name: 'handler_only'/); - expect(res.response.body.error.message).toMatch(/OS_ALLOW_UNDECLARED_ACTIONS=1/); + // The way out is the boot inventory, not a flag. + expect(res.response.body.error.message).toMatch(/\[action-governance\]/); expect(executeAction).not.toHaveBeenCalled(); }); - it('runs an UNDECLARED action when the migration valve is set, warning every time', async () => { + // The refusal has NO opt-out. A draft of ADR-0110 shipped + // `OS_ALLOW_UNDECLARED_ACTIONS` as a migration valve; it was dropped before + // 17 went out, because a flag that runs an ungoverned handler IS the + // fail-open the ruling closes. This pins that no environment variable + // resurrects the old behaviour — the failure a stale deployment script + // would otherwise produce is silent re-opening, not a loud error. + it('refuses regardless of the retired OS_ALLOW_UNDECLARED_ACTIONS flag', async () => { const prev = process.env.OS_ALLOW_UNDECLARED_ACTIONS; process.env.OS_ALLOW_UNDECLARED_ACTIONS = '1'; - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); try { const { dispatcher, executeAction } = makeDispatcher({ objectDef: { name: 'crm_lead', actions: [] } }); const res = await dispatcher.handleActions('/crm_lead/handler_only', 'POST', {}, ctxFor()); - expect(executeAction).toHaveBeenCalledTimes(1); - expect(res.response.body.data.success).toBe(true); - expect(warn).toHaveBeenCalledWith(expect.stringMatching(/UNDECLARED action 'crm_lead\/handler_only'/)); + expect(res.response.status).toBe(404); + expect(executeAction).not.toHaveBeenCalled(); } finally { - warn.mockRestore(); if (prev === undefined) delete process.env.OS_ALLOW_UNDECLARED_ACTIONS; else process.env.OS_ALLOW_UNDECLARED_ACTIONS = prev; }