From df3c3921f8e15e1268b2485f939cf690b5496e1c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 15:44:57 +0000 Subject: [PATCH] feat(runtime)!: action params are enforced by default, and the opt-in is gone (#3438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0104 D2 landed warn-first behind OS_ACTION_PARAMS_STRICT_ENABLED. The 2026-07-30 addendum decided that window closes in 17.0 rather than 18.0, so this executes it: - actionParamsStrict() becomes laxActionParams(): enforcement is the default and OS_ALLOW_LAX_ACTION_PARAMS=1 is the only knob — spelled OS_ALLOW_* per PD #9 and ADR-0110 D6, which this change also removes the sole cited exception from. The opt-in name never reached `latest` (RC-only), so it is deleted outright rather than deprecated for a release. - The escape-hatch path still warns once per action, so opting out tolerates the drift instead of hiding it again. - Dogfood duals are inverted: the DEFAULT path is now what the gate proves, and the hatch — the branch nobody sets — is what gets a test so it cannot rot unnoticed. Also corrects three docs the flip falsifies, plus two the earlier #3681 media work had already left self-contradictory in the v17 notes (the section said hard rejection arrives only via OS_DATA_VALUE_SHAPE_STRICT_ENABLED two paragraphs before saying media enforces per verified deployment; the upgrade checklist prescribed the env var as the way to get there, which would opt in every value class including the ones with no migration behind them). Verified: runtime 951, spec 7147, dogfood 425 (+3 skipped) all green; the D2 contract dogfood passes with the inverted duals; all ten spec artifact gates pass. Swept examples and packages for callers posting param bags — the todo MCP e2e invokes only param-less actions, showcase tests call handlers directly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_016zgA8CQMJeJbFEjnbib1Uv --- .changeset/adr-0104-d2-strict-by-default.md | 53 +++++++++++++ .../adr-0104-d2-typed-action-handlers.md | 13 ++-- content/docs/releases/v17.mdx | 75 ++++++++++++++++--- ...0104-field-runtime-value-shape-contract.md | 5 +- ...tion-identity-and-declaration-admission.md | 8 +- .../action-params-contract.dogfood.test.ts | 51 +++++++------ packages/runtime/src/action-execution.ts | 49 ++++++++---- packages/runtime/src/domains/actions.ts | 2 +- packages/spec/src/ui/action-params.zod.ts | 6 +- 9 files changed, 203 insertions(+), 59 deletions(-) create mode 100644 .changeset/adr-0104-d2-strict-by-default.md diff --git a/.changeset/adr-0104-d2-strict-by-default.md b/.changeset/adr-0104-d2-strict-by-default.md new file mode 100644 index 0000000000..72628b0c43 --- /dev/null +++ b/.changeset/adr-0104-d2-strict-by-default.md @@ -0,0 +1,53 @@ +--- +"@objectstack/runtime": major +--- + +feat(runtime)!: action params are enforced by default, and the opt-in is gone (#3438, ADR-0104 D2) + +A request bag that violates an action's declared `params[]` — a missing +`required` param, a value outside its `options`, a scalar where `multiple` +declares an array, a non-id where a `reference` declares one, or a key the +action never declared — is now **rejected before the handler runs**: +`400 VALIDATION_FAILED` on REST, a thrown error on MCP. It used to be logged +and passed through. + +```diff +- OS_ACTION_PARAMS_STRICT_ENABLED=1 # removed — enforcement is the default ++ OS_ALLOW_LAX_ACTION_PARAMS=1 # escape hatch: warn and pass, as before +``` + +**What breaks.** Only calls that were *already* wrong. The declaration was a +complete contract that informed nothing but the client dialog, so a bag the +server accepted could still have been silently ignored by the handler — which +is exactly how a correctly-intended `reference: 'sys_user'` degraded into a +paste-a-UUID box (#3405) with a success envelope on top. Those calls now fail +loudly instead of quietly. Actions declaring no `params` are untouched, and the +dispatcher's own `recordId` / `objectName` are allowlisted +(`ACTION_PARAM_BUILTIN_KEYS`), so the keys dispatch itself merges in were never +candidates for the unknown-key error. + +**Fixing a rejection** takes one edit at the call site: the message names the +offending param and the declared list. If an integration you cannot reach in +time is affected, set `OS_ALLOW_LAX_ACTION_PARAMS=1` to restore the old +pass-through — the violation still logs once per action, so the drift stays +visible rather than becoming invisible again. + +**Why 17.0 rather than a warn window in 17 and the flip in 18.** R3 asked for +warn-then-error, and ADR-0104's 2026-07-30 addendum declined it on the merits +rather than postponing. What a violation strands is a **caller**, not data: the +rejection reaches a developer or an agent who can fix it in one edit, no stored +row is made unwritable, and the escape hatch makes it reversible in a restart. +Deferring that by a major would have charged every deployment a second upgrade +ceremony — 16→17 is already a substantial, tested migration — to postpone a +break that costs one edited call. v17 already carries harsher zero-window +flips (`allowExport` unset now means denied; an undeclared action handler 404s +with no opt-out at all), so holding the milder change to a stricter standard +would have been inconsistent rather than cautious. + +For AI and MCP callers specifically — the population D2 was built for — a 400 +is corrective feedback consumed in-loop, while a server-side warning is +feedback nobody ever reads. + +D1's value-shape half went the opposite way for the opposite reason: it rejects +on the basis of **stored data**, which an author cannot edit their way out of, +so it stays gated per deployment on that deployment's own migration evidence. diff --git a/.changeset/adr-0104-d2-typed-action-handlers.md b/.changeset/adr-0104-d2-typed-action-handlers.md index 36e450de6e..a54cecdc6d 100644 --- a/.changeset/adr-0104-d2-typed-action-handlers.md +++ b/.changeset/adr-0104-d2-typed-action-handlers.md @@ -25,12 +25,13 @@ read an untyped bag. D2 makes the declaration enforced and typed. required presence, per-type value shape, and unknown keys (the dispatcher's own `recordId` / `objectName` are allowlisted). -**Warn-first rollout (ADR-0104 R3).** A violation is **logged and passes** by -default — params that were silently wrong before keep working while the drift -becomes visible. Set `OS_ACTION_PARAMS_STRICT_ENABLED=1` to reject with a -`400 VALIDATION` (REST) / an error (MCP). Actions that declare no `params` are -untouched (nothing to validate against). The flip to strict-by-default rides a -later minor once telemetry is quiet. +**Enforced by default.** A violation is rejected with a `400 VALIDATION_FAILED` +(REST) / an error (MCP) before the handler runs. Actions that declare no +`params` are untouched — there is nothing to validate against. This landed +warn-first behind an `OS_ACTION_PARAMS_STRICT_ENABLED` opt-in during the 17.0 +RC line; that variable does **not** exist in 17.0 — see the companion entry +"action params are enforced by default, and the opt-in is gone" for the +reasoning and the `OS_ALLOW_LAX_ACTION_PARAMS` escape hatch. Not included: file/image params becoming `sys_file` references — that depends on file-as-reference (ADR-0104 D3). Per-name static typing of `ctx.params` from diff --git a/content/docs/releases/v17.mdx b/content/docs/releases/v17.mdx index 0ab0823350..7105371264 100644 --- a/content/docs/releases/v17.mdx +++ b/content/docs/releases/v17.mdx @@ -216,6 +216,47 @@ 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`. +### An action's declared params are enforced at dispatch (ADR-0104 D2, #3438) + +An action's `params[]` (`required`, `options`, `multiple`, `reference`) was a +complete contract that informed **only the client dialog**. The server passed +`reqBody.params` to the handler unvalidated on both the REST and MCP paths, so +a wrong bag could return a success envelope while the handler quietly ignored +it — the #3405 shape, one layer out. It is now checked before the handler runs; +a violation is `400 VALIDATION_FAILED` (REST) or a thrown error (MCP). + +| Violation | Example | +|---|---| +| missing `required` param | declared `p_text` absent from the bag | +| value outside `options` | `p_priority: 'NOT_AN_OPTION'` | +| `multiple` shape | a bare string where an array is declared | +| `reference` shape | a non-id where `reference: 'sys_user'` is declared | +| undeclared key | `bogus: 123` | + +```diff +- OS_ACTION_PARAMS_STRICT_ENABLED=1 # removed — enforcement is the default ++ OS_ALLOW_LAX_ACTION_PARAMS=1 # escape hatch: warn and pass, as before +``` + +**Only already-wrong calls break**, and each rejection names the offending param +and the declared list, so the fix is one edit at the call site. Actions +declaring no `params` are untouched — there is nothing to validate against — +and the dispatcher's own `recordId` / `objectName` are allowlisted, so keys +dispatch merges in are never unknown-key errors. If an integration you cannot +reach in time is affected, `OS_ALLOW_LAX_ACTION_PARAMS=1` restores the old +pass-through in one restart; the violation still logs once per action, so the +drift stays visible rather than becoming invisible again. + +The RC line carried this warn-first behind an `OS_ACTION_PARAMS_STRICT_ENABLED` +opt-in. **That variable does not exist in 17.0.** The window was closed on +purpose rather than deferred to 18.0: what a violation strands is a *caller*, +not data — no stored row becomes unwritable, the party who sees the error is +the party who can fix it, and the hatch makes it reversible. Deferring would +have charged every deployment a second upgrade ceremony to postpone a break +that costs one edited call. The value-shape half of the same ADR went the other +way for the opposite reason: it rejects on the basis of data already at rest, +so it stays gated per deployment (see *Files become platform records* below). + ### A flow run with no trigger user may not touch data (#3760) An effective `runAs: 'user'` run that resolved **no trigger user** used to @@ -893,10 +934,10 @@ For media fields specifically: toward an explicit `url` field, so "managed file" and "external link" stop being the same declaration). -**Value-shape checking is warn-first.** A not-yet-backfilled row still writes and -the author gets a warning naming the field. Hard rejection arrives only when a -deployment opts into `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` — which it should do -*after* running the backfill and confirming reconciliation: +**Value-shape checking follows your own migration, not the version number.** A +not-yet-backfilled row still writes, and the author gets a warning naming the +field. Media fields start *rejecting* malformed values only once **this +deployment** has run the migration and passed its self-check: ```bash os migrate files-to-references # dry run: reports, writes nothing @@ -906,9 +947,18 @@ os migrate files-to-references --apply # converts, verifies, records the flag The run backfills legacy file-field values (inline metadata blobs, own-resolver URLs, `data:` URIs) into owned `sys_file` references and reconciles the ownership ledger against what records actually hold. The **deployment-level flag it -records** — never the platform version — is what may later authorise -irreversible behaviour, and media value shapes enforce only once *this* -deployment has verified its own migration. +records** — never the platform version — is what authorises both strict media +value shapes and, later, irreversible file collection. Upgrading changes +neither; running the migration does. + +Two knobs sit either side of that flag. `OS_ALLOW_LAX_MEDIA_VALUES=1` returns a +verified deployment to warnings, for an operator who hits an unforeseen +rejection and needs writes flowing while they diagnose. `OS_DATA_VALUE_SHAPE_STRICT_ENABLED=1` +goes the other way and opts *every* value class into strict immediately — +including the reference (`lookup`/`user`/…) and structured-JSON +(`location`/`address`/…) types, whose own per-deployment gate is still being +built (#3438). Those stay warn-only by default until it lands, because unlike +media they have no migration standing behind them yet. ### Approvals: dynamic approver routing (#3447) @@ -1195,9 +1245,16 @@ objectui commits on top of the pin 16.1.0 shipped. - **Auth:** plan for the better-auth 1.7 account-identity backfill — check the boot log for federated accounts whose IdP is no longer registered, and stamp or remove those rows. +- **Actions:** check any programmatic caller that posts `params` — a bag the + server used to accept silently now 400s if it misses a `required` param, + breaks `options`/`multiple`/`reference`, or carries an undeclared key. The + error names the param and the declared list. `OS_ALLOW_LAX_ACTION_PARAMS=1` + buys time for an integration you cannot reach today. - **Files:** run `os migrate files-to-references` (dry run first, then - `--apply`), reconcile, and only then set - `OS_DATA_VALUE_SHAPE_STRICT_ENABLED`. + `--apply`) and reconcile — passing its self-check is what turns strict media + value shapes on for this deployment. Do **not** reach for + `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` to get there: it opts every value class + in at once, including ones with no migration behind them. - **Datasources:** verify every declared datasource connects in every environment — a bound datasource that cannot connect now fails the boot instead of failing every later query. diff --git a/docs/adr/0104-field-runtime-value-shape-contract.md b/docs/adr/0104-field-runtime-value-shape-contract.md index b733aaeb76..91985bb9c4 100644 --- a/docs/adr/0104-field-runtime-value-shape-contract.md +++ b/docs/adr/0104-field-runtime-value-shape-contract.md @@ -414,7 +414,10 @@ rejects a targetless inline lookup param) — a build failure the AI must fix, n a runtime warning it never sees. The runtime **warn-first** posture (D1's `OS_DATA_VALUE_SHAPE_STRICT_ENABLED`, D2's `OS_ACTION_PARAMS_STRICT_ENABLED`) exists only to protect **already-deployed data** from stranding — it is not the -authoring gate. +authoring gate. *(D2's variable no longer exists: the 2026-07-30 addendum flips +that half strict-by-default in 17.0, leaving only the `OS_ALLOW_LAX_ACTION_PARAMS` +opt-out. The sentence's own logic is why — action params strand no deployed +data, so the posture it justifies never applied to them.)* Therefore the target end-state is **two enforcement points, each with one job**: diff --git a/docs/adr/0110-action-identity-and-declaration-admission.md b/docs/adr/0110-action-identity-and-declaration-admission.md index 93447dce32..d45406adf3 100644 --- a/docs/adr/0110-action-identity-and-declaration-admission.md +++ b/docs/adr/0110-action-identity-and-declaration-admission.md @@ -301,7 +301,7 @@ agent can act on. ### D6 — Security-gate strictness is opt-out, never opt-in -`OS_ACTION_PARAMS_STRICT_ENABLED` (opt-**in** strict) is an acceptable +`OS_ACTION_PARAMS_STRICT_ENABLED` (opt-**in** strict) was an acceptable shape for a *param-contract* ratchet (DX concern, warn-first). It is not an acceptable shape for an *authorization* gate: an enforcement that ships off for everyone who never read the release notes is not enforcement. A flag @@ -310,6 +310,12 @@ 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. +*(Update, 2026-07-30: the one flag this section cited as tolerable no longer +exists. ADR-0104's addendum flipped the param contract strict-by-default in +17.0 and replaced the opt-in with the opt-out `OS_ALLOW_LAX_ACTION_PARAMS` +(#3438), which the RC-only lifetime of the old name made free. The rule is +unchanged; it simply has no remaining exception to point at.)* + **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 diff --git a/packages/qa/dogfood/test/action-params-contract.dogfood.test.ts b/packages/qa/dogfood/test/action-params-contract.dogfood.test.ts index 55b164dd9e..20fe20d58a 100644 --- a/packages/qa/dogfood/test/action-params-contract.dogfood.test.ts +++ b/packages/qa/dogfood/test/action-params-contract.dogfood.test.ts @@ -7,10 +7,17 @@ // lookup `p_account`. Its body echoes the received param keys. We drive the // real `/actions/:object/:action` route to prove the declared contract is // enforced BEFORE the body runs: -// - warn-first (default): a malformed bag still passes (legacy callers keep -// working; the drift is logged, not fatal). -// - strict (OS_ACTION_PARAMS_STRICT_ENABLED=1): the same bag is rejected 400 +// - default (strict since 17.0, #3438): a malformed bag is rejected 400 // before the handler runs; a conformant bag passes. +// - escape hatch (OS_ALLOW_LAX_ACTION_PARAMS=1): the same malformed bag is +// accepted again, for the operator who must keep an integration +// dispatching while they fix its caller. +// +// The duals are this way round on purpose. Under warn-first the strict path was +// the one nobody reached without setting a variable; now the DEFAULT path is +// what every caller hits, so it is what the gate must prove — and the hatch, +// which is the branch nobody sets, is exactly the kind of thing that rots +// unnoticed unless a test drives it. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import showcaseStack from '@objectstack/example-showcase'; @@ -28,7 +35,7 @@ describe('dogfood: action param contract enforced at dispatch (ADR-0104 D2)', () }, 60_000); afterAll(async () => { - delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED; + delete process.env.OS_ALLOW_LAX_ACTION_PARAMS; await stack?.stop(); }); @@ -37,33 +44,29 @@ describe('dogfood: action param contract enforced at dispatch (ADR-0104 D2)', () const badBag = { p_priority: 'NOT_AN_OPTION', bogus: 123 }; const goodBag = { p_text: 'Hello', p_priority: 'high' }; - it('warn-first (default): a malformed param bag still passes and the body runs', async () => { - delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED; + it('default: a malformed param bag is rejected 400 before the handler runs', async () => { + delete process.env.OS_ALLOW_LAX_ACTION_PARAMS; const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: badBag }); - expect(res.status, `expected pass-through, got ${res.status}: ${await res.clone().text()}`).toBeLessThan(300); + expect(res.status).toBe(400); + const text = await res.text(); + expect(text).toMatch(/p_text/); // required + expect(text).toMatch(/p_priority/); // bad option + expect(text).toMatch(/bogus/); // unknown key }); - it('strict: the same malformed bag is rejected 400 before the handler runs', async () => { - process.env.OS_ACTION_PARAMS_STRICT_ENABLED = '1'; - try { - const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: badBag }); - expect(res.status).toBe(400); - const text = await res.text(); - expect(text).toMatch(/p_text/); // required - expect(text).toMatch(/p_priority/); // bad option - expect(text).toMatch(/bogus/); // unknown key - } finally { - delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED; - } + it('default: a conformant bag passes (dispatcher built-in keys are allowlisted)', async () => { + delete process.env.OS_ALLOW_LAX_ACTION_PARAMS; + const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: goodBag }); + expect(res.status, `expected pass, got ${res.status}: ${await res.clone().text()}`).toBeLessThan(300); }); - it('strict: a conformant bag passes (dispatcher built-in keys are allowlisted)', async () => { - process.env.OS_ACTION_PARAMS_STRICT_ENABLED = '1'; + it('escape hatch: OS_ALLOW_LAX_ACTION_PARAMS=1 accepts the malformed bag again', async () => { + process.env.OS_ALLOW_LAX_ACTION_PARAMS = '1'; try { - const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: goodBag }); - expect(res.status, `expected pass, got ${res.status}: ${await res.clone().text()}`).toBeLessThan(300); + const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: badBag }); + expect(res.status, `expected pass-through, got ${res.status}: ${await res.clone().text()}`).toBeLessThan(300); } finally { - delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED; + delete process.env.OS_ALLOW_LAX_ACTION_PARAMS; } }); }); diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 0b614f0c11..c4260f5909 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -44,9 +44,20 @@ export function isSystemObjectName(name: string): boolean { return /^sys_/i.test(name); } -/** Strict action-param enforcement opt-in (ADR-0104 D2 warn-first rollout). */ -function actionParamsStrict(): boolean { - return typeof process !== 'undefined' && process.env?.OS_ACTION_PARAMS_STRICT_ENABLED === '1'; +/** + * Escape hatch: accept param bags that violate the declared contract, the way + * the pre-17 dispatcher did (ADR-0104 D2, 2026-07-30 addendum). + * + * Enforcement is the default. This exists for the operator whose integration + * hits an unforeseen rejection and needs it dispatching again before they can + * reach the caller's code — the violation still logs, so setting this makes the + * drift tolerated, not invisible. + * + * Spelled `OS_ALLOW_*` per Prime Directive #9 and ADR-0110 D6: an opt-OUT of a + * check that ships on, never an opt-IN to a check that ships off. + */ +function laxActionParams(): boolean { + return typeof process !== 'undefined' && process.env?.OS_ALLOW_LAX_ACTION_PARAMS === '1'; } @@ -561,14 +572,21 @@ export function resolveDeclaredActionParams(deps: ActionExecutionDeps, action: a /** * Enforce an action's declared param contract against the request bag - * BEFORE the handler runs (ADR-0104 D2). Returns a `400`-worthy error - * message when the contract is violated AND strict mode is on - * (`OS_ACTION_PARAMS_STRICT_ENABLED=1`); otherwise returns `null`, logging - * a one-time warning per (object/action) so the drift is visible without - * breaking callers whose params were silently wrong before (warn-first, R3). + * BEFORE the handler runs (ADR-0104 D2). Returns a `400`-worthy error message + * when the contract is violated, `null` when the bag conforms. + * + * **Strict by default since 17.0** (#3438). R3 asked for a warn-then-error + * window; the ADR's 2026-07-30 addendum declined it on the merits rather than + * postponing the flip by a major. What a violation strands here is a CALLER, + * not data: the rejection is a 400 naming the offending param and the declared + * list, delivered to the developer or agent who can fix it in one edit, and + * undoable with `OS_ALLOW_LAX_ACTION_PARAMS`. Deferring that to 18.0 would have + * charged every deployment a second upgrade ceremony to defer a break that + * costs one edited call. (D1's half went the opposite way for the opposite + * reason — it strands stored rows, which nobody can edit their way out of.) * - * Actions that declare no `params` keep today's pass-through — there is - * nothing to validate against, so existing param-less actions are untouched. + * Actions that declare no `params` keep the pass-through — there is nothing to + * validate against, so existing param-less actions are untouched. */ export function enforceActionParams(deps: ActionExecutionDeps, action: any, @@ -581,14 +599,14 @@ export function enforceActionParams(deps: ActionExecutionDeps, const issues = validateActionParams(resolved, bag); if (issues.length === 0) return null; const summary = issues.map((i) => i.message).join('; '); - if (actionParamsStrict()) { + if (!laxActionParams()) { return `Invalid action params: ${summary}`; } const key = `${where.objectName ?? 'global'}/${where.actionName ?? action?.name ?? 'action'}`; warnActionParamsOnce( key, - `[action-params] ${key}: ${summary} — accepted for now (ADR-0104 D2 warn-first; ` + - `set OS_ACTION_PARAMS_STRICT_ENABLED=1 to reject with 400)`, + `[action-params] ${key}: ${summary} — accepted because ` + + `OS_ALLOW_LAX_ACTION_PARAMS=1 (ADR-0104 D2; unset it to reject with 400)`, ); return null; } @@ -773,8 +791,9 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps, // [ADR-0104 D2] Declared param contract — same enforcement as the REST // route. AI/MCP is the caller most likely to send a plausible-but-wrong - // bag, so the check belongs here too. Warn-first unless - // OS_ACTION_PARAMS_STRICT_ENABLED=1 (then throws → surfaced as an error). + // bag, and a rejection is corrective feedback the agent consumes in-loop, + // which a server-side warning never was. Strict by default (#3438); + // OS_ALLOW_LAX_ACTION_PARAMS=1 restores the pass-through. const paramError = enforceActionParams(deps, action, obj, params, { objectName, actionName: name }); if (paramError) throw new Error(paramError); diff --git a/packages/runtime/src/domains/actions.ts b/packages/runtime/src/domains/actions.ts index 2a1410b7f3..1b72630dfe 100644 --- a/packages/runtime/src/domains/actions.ts +++ b/packages/runtime/src/domains/actions.ts @@ -246,7 +246,7 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string // [ADR-0104 D2] Enforce the declared param contract before the handler // runs — required/option/multiple/reference-id shape + unknown keys. - // Warn-first unless OS_ACTION_PARAMS_STRICT_ENABLED=1 (then a 400). + // Strict by default (#3438); OS_ALLOW_LAX_ACTION_PARAMS=1 warns instead. const paramError = actionExec.enforceActionParams(deps, actionDef, actionSchema, reqParams, { objectName, actionName }); if (paramError) { return { handled: true, response: deps.error(paramError, 400) }; diff --git a/packages/spec/src/ui/action-params.zod.ts b/packages/spec/src/ui/action-params.zod.ts index da17cad3a2..0ad52c079e 100644 --- a/packages/spec/src/ui/action-params.zod.ts +++ b/packages/spec/src/ui/action-params.zod.ts @@ -68,8 +68,10 @@ function isPresent(v: unknown): boolean { /** * Validate a params bag against an action's resolved param declarations. - * Returns the list of issues (empty = conformant). Does NOT throw — the caller - * decides warn-vs-reject (ADR-0104 D2 warn-first rollout, R3). + * Returns the list of issues (empty = conformant). Does NOT throw — this is a + * pure check, and the disposition belongs to the caller: the dispatcher rejects + * with a 400 by default (ADR-0104 D2, strict since 17.0), while an authoring or + * preview surface may want to render the same issues without failing. * * Enforced: `required` presence, per-type value shape (via the D1 * `valueSchemaFor`, so option membership / `multiple` arrays / reference-id