Skip to content

Commit f611684

Browse files
committed
feat: enforce declared action-param contract at dispatch — ADR-0104 phase 2 (D2)
An action's declared params[] (type/required/multiple/options/reference) was a complete value contract that only informed the client dialog — the server passed reqBody.params straight to the handler unvalidated (REST handleActions + MCP invokeBusinessAction), and handlers read an untyped bag. - @objectstack/spec/ui: validateActionParams (+ ResolvedActionParam, ActionParamIssue, ACTION_PARAM_BUILTIN_KEYS) — pure check reusing the D1 valueSchemaFor so option membership / multiple arrays / reference-id shape ride the one value contract; plus typed authoring surface ActionHandler / ActionHandlerContext / ActionEngineFacade. - runtime: both REST and MCP action paths resolve declared params (field-backed resolved through the referenced object field) and validate the request bag before the handler runs — required/shape/unknown-key; recordId/objectName allowlisted. Warn-first unless OS_ACTION_PARAMS_STRICT_ENABLED=1 (then 400). Actions with no declared params are untouched. - ScriptContext.input doc + registerAction JSDoc state the validated contract. Tests: spec 6847 (incl. 9 action-params units), runtime 587, objectql 1043, new action-params dogfood 3 (warn-first passes / strict 400 / conformant passes) — all green. API-surface snapshot updated (+7 additive exports). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHpGw3GBA9aFpfwVArRWfd
1 parent e43626a commit f611684

9 files changed

Lines changed: 464 additions & 7 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/runtime": minor
4+
---
5+
6+
feat: enforce declared action-param contract at dispatch — ADR-0104 phase 2 (D2)
7+
8+
An action's declared `params[]` (`type` / `required` / `multiple` / `options` /
9+
`reference`) was a complete value contract that only ever informed the client
10+
dialog — the server passed `reqBody.params` straight to the handler unvalidated
11+
(REST `handleActions` and the MCP `invokeBusinessAction` path), and handlers
12+
read an untyped bag. D2 makes the declaration enforced and typed.
13+
14+
- **`@objectstack/spec/ui`** now exports `validateActionParams` (+
15+
`ResolvedActionParam`, `ActionParamIssue`, `ACTION_PARAM_BUILTIN_KEYS`): a
16+
pure check that validates a params bag against resolved param declarations,
17+
reusing the D1 `valueSchemaFor` so option membership, `multiple` arrays and
18+
reference-id shape all ride the one value contract. Also exports the typed
19+
authoring surface `ActionHandler` / `ActionHandlerContext` /
20+
`ActionEngineFacade` — annotate a handler with `ActionHandler` instead of
21+
`(ctx: any)`.
22+
- **Dispatch (runtime)**: both the REST and MCP action paths resolve the
23+
action's declared params (field-backed params resolved through the referenced
24+
object field) and validate the request bag **before the handler runs**
25+
required presence, per-type value shape, and unknown keys (the dispatcher's
26+
own `recordId` / `objectName` are allowlisted).
27+
28+
**Warn-first rollout (ADR-0104 R3).** A violation is **logged and passes** by
29+
default — params that were silently wrong before keep working while the drift
30+
becomes visible. Set `OS_ACTION_PARAMS_STRICT_ENABLED=1` to reject with a
31+
`400 VALIDATION` (REST) / an error (MCP). Actions that declare no `params` are
32+
untouched (nothing to validate against). The flip to strict-by-default rides a
33+
later minor once telemetry is quiet.
34+
35+
Not included: file/image params becoming `sys_file` references — that depends
36+
on file-as-reference (ADR-0104 D3). Per-name static typing of `ctx.params` from
37+
the literal `params` array is a deferred DX nicety; the runtime guarantee holds
38+
regardless.

packages/objectql/src/engine.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,11 @@ export class ObjectQL implements IDataEngine {
672672
*
673673
* @param objectName Target object
674674
* @param actionName Unique action name within the object
675-
* @param handler Handler function
675+
* @param handler Handler function. Authoring sites should annotate it with
676+
* `ActionHandler` from `@objectstack/spec/ui` (ADR-0104 D2) rather than an
677+
* inline `(ctx: any)`; the params on `ctx` are validated against the
678+
* action's declared param contract at dispatch before the handler runs.
679+
* The seam itself stays untyped so existing untyped handlers keep working.
676680
* @param packageName Optional package owner (for cleanup)
677681
*/
678682
registerAction(objectName: string, actionName: string, handler: (ctx: any) => Promise<any> | any, packageName?: string): void {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// ADR-0104 D2 — action param contract enforced at dispatch, over real HTTP.
4+
//
5+
// The showcase `showcase_action_param_gallery` action (on field-zoo) declares
6+
// a required `p_text`, an option-bearing `p_priority` select, and an inline
7+
// lookup `p_account`. Its body echoes the received param keys. We drive the
8+
// real `/actions/:object/:action` route to prove the declared contract is
9+
// enforced BEFORE the body runs:
10+
// - warn-first (default): a malformed bag still passes (legacy callers keep
11+
// working; the drift is logged, not fatal).
12+
// - strict (OS_ACTION_PARAMS_STRICT_ENABLED=1): the same bag is rejected 400
13+
// before the handler runs; a conformant bag passes.
14+
15+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
16+
import showcaseStack from '@objectstack/example-showcase';
17+
import { bootStack, type VerifyStack } from '@objectstack/verify';
18+
19+
const ACTION_PATH = '/actions/showcase_field_zoo/showcase_action_param_gallery';
20+
21+
describe('dogfood: action param contract enforced at dispatch (ADR-0104 D2)', () => {
22+
let stack: VerifyStack;
23+
let token: string;
24+
25+
beforeAll(async () => {
26+
stack = await bootStack(showcaseStack);
27+
token = await stack.signIn();
28+
}, 60_000);
29+
30+
afterAll(async () => {
31+
delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED;
32+
await stack?.stop();
33+
});
34+
35+
// A bag that violates the declaration three ways: missing required `p_text`,
36+
// a `p_priority` outside its options, and an undeclared `bogus` key.
37+
const badBag = { p_priority: 'NOT_AN_OPTION', bogus: 123 };
38+
const goodBag = { p_text: 'Hello', p_priority: 'high' };
39+
40+
it('warn-first (default): a malformed param bag still passes and the body runs', async () => {
41+
delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED;
42+
const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: badBag });
43+
expect(res.status, `expected pass-through, got ${res.status}: ${await res.clone().text()}`).toBeLessThan(300);
44+
});
45+
46+
it('strict: the same malformed bag is rejected 400 before the handler runs', async () => {
47+
process.env.OS_ACTION_PARAMS_STRICT_ENABLED = '1';
48+
try {
49+
const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: badBag });
50+
expect(res.status).toBe(400);
51+
const text = await res.text();
52+
expect(text).toMatch(/p_text/); // required
53+
expect(text).toMatch(/p_priority/); // bad option
54+
expect(text).toMatch(/bogus/); // unknown key
55+
} finally {
56+
delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED;
57+
}
58+
});
59+
60+
it('strict: a conformant bag passes (dispatcher built-in keys are allowlisted)', async () => {
61+
process.env.OS_ACTION_PARAMS_STRICT_ENABLED = '1';
62+
try {
63+
const res = await stack.apiAs(token, 'POST', ACTION_PATH, { params: goodBag });
64+
expect(res.status, `expected pass, got ${res.status}: ${await res.clone().text()}`).toBeLessThan(300);
65+
} finally {
66+
delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED;
67+
}
68+
});
69+
});

packages/runtime/src/http-dispatcher.ts

Lines changed: 100 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { CoreServiceName } from '@objectstack/spec/system';
1010
import { readServiceSelfInfo } from '@objectstack/spec/api';
1111
import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
1212
import { pluralToSingular, PLURAL_TO_SINGULAR } from '@objectstack/spec/shared';
13+
import { validateActionParams, type ResolvedActionParam } from '@objectstack/spec/ui';
1314
import type { ExecutionContext } from '@objectstack/spec/kernel';
1415
import { setPackageDisabled } from './package-state-store.js';
1516
import { checkApiExposure } from './api-exposure.js';
@@ -1001,6 +1002,79 @@ export class HttpDispatcher {
10011002
return out;
10021003
}
10031004

1005+
/**
1006+
* Resolve an action's declared `params[]` to their effective value-shape
1007+
* inputs (ADR-0104 D2). A field-backed param inherits type/multiple/
1008+
* options/required from the referenced object field; an inline param
1009+
* carries them directly (inline overrides win). `obj` is the action's
1010+
* parent object schema (holds `.fields`); pass `undefined` for a global
1011+
* action with only inline params.
1012+
*/
1013+
private resolveDeclaredActionParams(action: any, obj: any): ResolvedActionParam[] {
1014+
const fields: Record<string, any> = obj?.fields ?? {};
1015+
const out: ResolvedActionParam[] = [];
1016+
for (const p of (Array.isArray(action?.params) ? action.params : [])) {
1017+
const fieldRef: string | undefined = p?.field;
1018+
const field = fieldRef ? fields[fieldRef] : undefined;
1019+
const name: string | undefined = p?.name ?? fieldRef;
1020+
if (!name) continue;
1021+
out.push({
1022+
name,
1023+
type: p?.type ?? field?.type,
1024+
multiple: p?.multiple ?? field?.multiple,
1025+
required: Boolean(p?.required ?? field?.required ?? false),
1026+
options: p?.options ?? field?.options,
1027+
});
1028+
}
1029+
return out;
1030+
}
1031+
1032+
/**
1033+
* Enforce an action's declared param contract against the request bag
1034+
* BEFORE the handler runs (ADR-0104 D2). Returns a `400`-worthy error
1035+
* message when the contract is violated AND strict mode is on
1036+
* (`OS_ACTION_PARAMS_STRICT_ENABLED=1`); otherwise returns `null`, logging
1037+
* a one-time warning per (object/action) so the drift is visible without
1038+
* breaking callers whose params were silently wrong before (warn-first, R3).
1039+
*
1040+
* Actions that declare no `params` keep today's pass-through — there is
1041+
* nothing to validate against, so existing param-less actions are untouched.
1042+
*/
1043+
private enforceActionParams(
1044+
action: any,
1045+
obj: any,
1046+
bag: Record<string, unknown>,
1047+
where: { objectName?: string; actionName?: string },
1048+
): string | null {
1049+
if (!Array.isArray(action?.params) || action.params.length === 0) return null;
1050+
const resolved = this.resolveDeclaredActionParams(action, obj);
1051+
const issues = validateActionParams(resolved, bag);
1052+
if (issues.length === 0) return null;
1053+
const summary = issues.map((i) => i.message).join('; ');
1054+
if (HttpDispatcher.actionParamsStrict()) {
1055+
return `Invalid action params: ${summary}`;
1056+
}
1057+
const key = `${where.objectName ?? 'global'}/${where.actionName ?? action?.name ?? 'action'}`;
1058+
HttpDispatcher.warnActionParamsOnce(
1059+
key,
1060+
`[action-params] ${key}: ${summary} — accepted for now (ADR-0104 D2 warn-first; ` +
1061+
`set OS_ACTION_PARAMS_STRICT_ENABLED=1 to reject with 400)`,
1062+
);
1063+
return null;
1064+
}
1065+
1066+
/** Strict action-param enforcement opt-in (ADR-0104 D2 warn-first rollout). */
1067+
private static actionParamsStrict(): boolean {
1068+
return typeof process !== 'undefined' && process.env?.OS_ACTION_PARAMS_STRICT_ENABLED === '1';
1069+
}
1070+
1071+
private static readonly _warnedActionParams = new Set<string>();
1072+
private static warnActionParamsOnce(key: string, message: string): void {
1073+
if (HttpDispatcher._warnedActionParams.has(key)) return;
1074+
HttpDispatcher._warnedActionParams.add(key);
1075+
console.warn(message);
1076+
}
1077+
10041078
/**
10051079
* Slim engine facade matching the ActionContext.engine shape handlers expect.
10061080
*
@@ -1102,7 +1176,7 @@ export class HttpDispatcher {
11021176
: `Action '${name}' not found`,
11031177
);
11041178
}
1105-
const { action, objectName } = resolved;
1179+
const { action, objectName, obj } = resolved;
11061180

11071181
// Fail-closed on system-object actions (mirrors the object-tool guard).
11081182
if (isSystemObjectName(objectName)) {
@@ -1125,6 +1199,13 @@ export class HttpDispatcher {
11251199
const gateError = this.actionPermissionError(action, ec, objectName);
11261200
if (gateError) throw new Error(gateError);
11271201

1202+
// [ADR-0104 D2] Declared param contract — same enforcement as the REST
1203+
// route. AI/MCP is the caller most likely to send a plausible-but-wrong
1204+
// bag, so the check belongs here too. Warn-first unless
1205+
// OS_ACTION_PARAMS_STRICT_ENABLED=1 (then throws → surfaced as an error).
1206+
const paramError = this.enforceActionParams(action, obj, params, { objectName, actionName: name });
1207+
if (paramError) throw new Error(paramError);
1208+
11281209
// Load the subject record under RLS when row-context (engages the same
11291210
// permission path as get_record — an unseen record reads as not-found).
11301211
let record: Record<string, unknown> = {};
@@ -1229,19 +1310,19 @@ export class HttpDispatcher {
12291310
meta: any,
12301311
name: string,
12311312
objectName?: string,
1232-
): Promise<{ action: any; objectName: string } | null> {
1313+
): Promise<{ action: any; objectName: string; obj: any } | null> {
12331314
const decls = await this.collectActionDeclarations(meta);
12341315
if (objectName) {
12351316
const hit = decls.find((d) => d.objectName === objectName && d.action?.name === name);
1236-
return hit ? { action: hit.action, objectName } : null;
1317+
return hit ? { action: hit.action, objectName, obj: hit.obj } : null;
12371318
}
12381319
const matches = decls.filter((d) => d.action?.name === name);
12391320
if (matches.length === 0) return null;
12401321
if (matches.length > 1) {
12411322
const where = matches.map((m) => m.objectName).join(', ');
12421323
throw new Error(`Action '${name}' exists on multiple objects (${where}); pass objectName to disambiguate`);
12431324
}
1244-
return { action: matches[0].action, objectName: matches[0].objectName };
1325+
return { action: matches[0].action, objectName: matches[0].objectName, obj: matches[0].obj };
12451326
}
12461327

12471328
/**
@@ -3855,11 +3936,16 @@ export class HttpDispatcher {
38553936
// server-closed (and the inverse footgun is removed). System/engine
38563937
// self-invocation (isSystem) bypasses; an unauthenticated caller holds
38573938
// no capabilities and is therefore denied for a gated action.
3939+
// Resolve the object schema + this action's declaration once — both the
3940+
// permission gate (ADR-0066 D4) and the param contract (ADR-0104 D2)
3941+
// read it.
3942+
let actionSchema: any;
3943+
let actionDef: any;
38583944
try {
3859-
const actionSchema: any =
3945+
actionSchema =
38603946
(typeof ql.getSchema === 'function' ? ql.getSchema(objectName) : undefined) ??
38613947
ql.registry?.getObject?.(objectName);
3862-
const actionDef: any = Array.isArray(actionSchema?.actions)
3948+
actionDef = Array.isArray(actionSchema?.actions)
38633949
? actionSchema.actions.find((a: any) => a?.name === actionName)
38643950
: undefined;
38653951
const gateError = this.actionPermissionError(actionDef, _context?.executionContext, objectName);
@@ -3881,6 +3967,14 @@ export class HttpDispatcher {
38813967
const recordId = recordIdFromPath ?? reqBody.recordId;
38823968
const reqParams = (reqBody.params && typeof reqBody.params === 'object') ? reqBody.params : {};
38833969

3970+
// [ADR-0104 D2] Enforce the declared param contract before the handler
3971+
// runs — required/option/multiple/reference-id shape + unknown keys.
3972+
// Warn-first unless OS_ACTION_PARAMS_STRICT_ENABLED=1 (then a 400).
3973+
const paramError = this.enforceActionParams(actionDef, actionSchema, reqParams, { objectName, actionName });
3974+
if (paramError) {
3975+
return { handled: true, response: this.error(paramError, 400) };
3976+
}
3977+
38843978
// Load the record (best-effort) so handlers can rely on `ctx.record`.
38853979
let record: Record<string, unknown> = {};
38863980
if (recordId && objectName !== 'global') {

packages/runtime/src/sandbox/script-runner.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ export interface ScriptOrigin {
5757
* actually wired up.
5858
*/
5959
export interface ScriptContext {
60+
/**
61+
* The script input. For an action body this is the action's params bag,
62+
* validated against the action's declared param contract at dispatch before
63+
* the body runs (ADR-0104 D2) — so its shape conforms to the declaration.
64+
* Typed `unknown` because the sandbox is a single generic seam over hooks
65+
* (record) and action bodies (params); the guarantee is enforced upstream,
66+
* not by this type.
67+
*/
6068
input: unknown;
6169
previous?: unknown;
6270
user?: unknown;

packages/spec/api-surface.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3076,16 +3076,21 @@
30763076
],
30773077
"./ui": [
30783078
"ACTION_LOCATIONS (const)",
3079+
"ACTION_PARAM_BUILTIN_KEYS (const)",
30793080
"AIChatWindowProps (const)",
30803081
"Action (type)",
30813082
"ActionAi (type)",
30823083
"ActionAiSchema (const)",
3084+
"ActionEngineFacade (interface)",
3085+
"ActionHandler (type)",
3086+
"ActionHandlerContext (interface)",
30833087
"ActionInput (type)",
30843088
"ActionLocation (type)",
30853089
"ActionLocationSchema (const)",
30863090
"ActionNavItem (type)",
30873091
"ActionNavItemSchema (const)",
30883092
"ActionParam (type)",
3093+
"ActionParamIssue (interface)",
30893094
"ActionParamSchema (const)",
30903095
"ActionSchema (const)",
30913096
"ActionType (const)",
@@ -3374,6 +3379,7 @@
33743379
"ReportNavItemSchema (const)",
33753380
"ReportSchema (const)",
33763381
"ReportType (const)",
3382+
"ResolvedActionParam (interface)",
33773383
"ResponsiveConfig (type)",
33783384
"ResponsiveConfigSchema (const)",
33793385
"ResponsiveStyles (type)",
@@ -3487,6 +3493,7 @@
34873493
"normalizeFilterOperator (function)",
34883494
"pageForm (const)",
34893495
"reportForm (const)",
3496+
"validateActionParams (function)",
34903497
"viewForm (const)"
34913498
],
34923499
"./contracts": [

0 commit comments

Comments
 (0)