Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .changeset/adr-0104-d2-typed-action-handlers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
"@objectstack/spec": minor
"@objectstack/runtime": minor
---

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 ever informed the client
dialog — the server passed `reqBody.params` straight to the handler unvalidated
(REST `handleActions` and the MCP `invokeBusinessAction` path), and handlers
read an untyped bag. D2 makes the declaration enforced and typed.

- **`@objectstack/spec/ui`** now exports `validateActionParams` (+
`ResolvedActionParam`, `ActionParamIssue`, `ACTION_PARAM_BUILTIN_KEYS`): a
pure check that validates a params bag against resolved param declarations,
reusing the D1 `valueSchemaFor` so option membership, `multiple` arrays and
reference-id shape all ride the one value contract. Also exports the typed
authoring surface `ActionHandler` / `ActionHandlerContext` /
`ActionEngineFacade` — annotate a handler with `ActionHandler` instead of
`(ctx: any)`.
- **Dispatch (runtime)**: both the REST and MCP action paths resolve the
action's declared params (field-backed params resolved through the referenced
object field) and validate the request bag **before the handler runs** —
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.

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
the literal `params` array is a deferred DX nicety; the runtime guarantee holds
regardless.
6 changes: 5 additions & 1 deletion packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,11 @@ export class ObjectQL implements IDataEngine {
*
* @param objectName Target object
* @param actionName Unique action name within the object
* @param handler Handler function
* @param handler Handler function. Authoring sites should annotate it with
* `ActionHandler` from `@objectstack/spec/ui` (ADR-0104 D2) rather than an
* inline `(ctx: any)`; the params on `ctx` are validated against the
* action's declared param contract at dispatch before the handler runs.
* The seam itself stays untyped so existing untyped handlers keep working.
* @param packageName Optional package owner (for cleanup)
*/
registerAction(objectName: string, actionName: string, handler: (ctx: any) => Promise<any> | any, packageName?: string): void {
Expand Down
69 changes: 69 additions & 0 deletions packages/qa/dogfood/test/action-params-contract.dogfood.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
//
// ADR-0104 D2 — action param contract enforced at dispatch, over real HTTP.
//
// The showcase `showcase_action_param_gallery` action (on field-zoo) declares
// a required `p_text`, an option-bearing `p_priority` select, and an inline
// 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
// before the handler runs; a conformant bag passes.

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import showcaseStack from '@objectstack/example-showcase';
import { bootStack, type VerifyStack } from '@objectstack/verify';

const ACTION_PATH = '/actions/showcase_field_zoo/showcase_action_param_gallery';

describe('dogfood: action param contract enforced at dispatch (ADR-0104 D2)', () => {
let stack: VerifyStack;
let token: string;

beforeAll(async () => {
stack = await bootStack(showcaseStack);
token = await stack.signIn();
}, 60_000);

afterAll(async () => {
delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED;
await stack?.stop();
});

// A bag that violates the declaration three ways: missing required `p_text`,
// a `p_priority` outside its options, and an undeclared `bogus` key.
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;
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);
});

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('strict: a conformant bag passes (dispatcher built-in keys are allowlisted)', async () => {
process.env.OS_ACTION_PARAMS_STRICT_ENABLED = '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);
} finally {
delete process.env.OS_ACTION_PARAMS_STRICT_ENABLED;
}
});
});
106 changes: 100 additions & 6 deletions packages/runtime/src/http-dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { CoreServiceName } from '@objectstack/spec/system';
import { readServiceSelfInfo } from '@objectstack/spec/api';
import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
import { pluralToSingular, PLURAL_TO_SINGULAR } from '@objectstack/spec/shared';
import { validateActionParams, type ResolvedActionParam } from '@objectstack/spec/ui';
import type { ExecutionContext } from '@objectstack/spec/kernel';
import { setPackageDisabled } from './package-state-store.js';
import { checkApiExposure } from './api-exposure.js';
Expand Down Expand Up @@ -1001,6 +1002,79 @@ export class HttpDispatcher {
return out;
}

/**
* Resolve an action's declared `params[]` to their effective value-shape
* inputs (ADR-0104 D2). A field-backed param inherits type/multiple/
* options/required from the referenced object field; an inline param
* carries them directly (inline overrides win). `obj` is the action's
* parent object schema (holds `.fields`); pass `undefined` for a global
* action with only inline params.
*/
private resolveDeclaredActionParams(action: any, obj: any): ResolvedActionParam[] {
const fields: Record<string, any> = obj?.fields ?? {};
const out: ResolvedActionParam[] = [];
for (const p of (Array.isArray(action?.params) ? action.params : [])) {
const fieldRef: string | undefined = p?.field;
const field = fieldRef ? fields[fieldRef] : undefined;
const name: string | undefined = p?.name ?? fieldRef;
if (!name) continue;
out.push({
name,
type: p?.type ?? field?.type,
multiple: p?.multiple ?? field?.multiple,
required: Boolean(p?.required ?? field?.required ?? false),
options: p?.options ?? field?.options,
});
}
return out;
}

/**
* 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).
*
* Actions that declare no `params` keep today's pass-through — there is
* nothing to validate against, so existing param-less actions are untouched.
*/
private enforceActionParams(
action: any,
obj: any,
bag: Record<string, unknown>,
where: { objectName?: string; actionName?: string },
): string | null {
if (!Array.isArray(action?.params) || action.params.length === 0) return null;
const resolved = this.resolveDeclaredActionParams(action, obj);
const issues = validateActionParams(resolved, bag);
if (issues.length === 0) return null;
const summary = issues.map((i) => i.message).join('; ');
if (HttpDispatcher.actionParamsStrict()) {
return `Invalid action params: ${summary}`;
}
const key = `${where.objectName ?? 'global'}/${where.actionName ?? action?.name ?? 'action'}`;
HttpDispatcher.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)`,
);
return null;
}

/** Strict action-param enforcement opt-in (ADR-0104 D2 warn-first rollout). */
private static actionParamsStrict(): boolean {
return typeof process !== 'undefined' && process.env?.OS_ACTION_PARAMS_STRICT_ENABLED === '1';
}

private static readonly _warnedActionParams = new Set<string>();
private static warnActionParamsOnce(key: string, message: string): void {
if (HttpDispatcher._warnedActionParams.has(key)) return;
HttpDispatcher._warnedActionParams.add(key);
console.warn(message);
}

/**
* Slim engine facade matching the ActionContext.engine shape handlers expect.
*
Expand Down Expand Up @@ -1102,7 +1176,7 @@ export class HttpDispatcher {
: `Action '${name}' not found`,
);
}
const { action, objectName } = resolved;
const { action, objectName, obj } = resolved;

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

// [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).
const paramError = this.enforceActionParams(action, obj, params, { objectName, actionName: name });
if (paramError) throw new Error(paramError);

// Load the subject record under RLS when row-context (engages the same
// permission path as get_record — an unseen record reads as not-found).
let record: Record<string, unknown> = {};
Expand Down Expand Up @@ -1229,19 +1310,19 @@ export class HttpDispatcher {
meta: any,
name: string,
objectName?: string,
): Promise<{ action: any; objectName: string } | null> {
): Promise<{ action: any; objectName: string; obj: any } | null> {
const decls = await this.collectActionDeclarations(meta);
if (objectName) {
const hit = decls.find((d) => d.objectName === objectName && d.action?.name === name);
return hit ? { action: hit.action, objectName } : null;
return hit ? { action: hit.action, objectName, obj: hit.obj } : null;
}
const matches = decls.filter((d) => d.action?.name === name);
if (matches.length === 0) return null;
if (matches.length > 1) {
const where = matches.map((m) => m.objectName).join(', ');
throw new Error(`Action '${name}' exists on multiple objects (${where}); pass objectName to disambiguate`);
}
return { action: matches[0].action, objectName: matches[0].objectName };
return { action: matches[0].action, objectName: matches[0].objectName, obj: matches[0].obj };
}

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

// [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).
const paramError = this.enforceActionParams(actionDef, actionSchema, reqParams, { objectName, actionName });
if (paramError) {
return { handled: true, response: this.error(paramError, 400) };
}

// Load the record (best-effort) so handlers can rely on `ctx.record`.
let record: Record<string, unknown> = {};
if (recordId && objectName !== 'global') {
Expand Down
8 changes: 8 additions & 0 deletions packages/runtime/src/sandbox/script-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ export interface ScriptOrigin {
* actually wired up.
*/
export interface ScriptContext {
/**
* The script input. For an action body this is the action's params bag,
* validated against the action's declared param contract at dispatch before
* the body runs (ADR-0104 D2) — so its shape conforms to the declaration.
* Typed `unknown` because the sandbox is a single generic seam over hooks
* (record) and action bodies (params); the guarantee is enforced upstream,
* not by this type.
*/
input: unknown;
previous?: unknown;
user?: unknown;
Expand Down
7 changes: 7 additions & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -3076,16 +3076,21 @@
],
"./ui": [
"ACTION_LOCATIONS (const)",
"ACTION_PARAM_BUILTIN_KEYS (const)",
"AIChatWindowProps (const)",
"Action (type)",
"ActionAi (type)",
"ActionAiSchema (const)",
"ActionEngineFacade (interface)",
"ActionHandler (type)",
"ActionHandlerContext (interface)",
"ActionInput (type)",
"ActionLocation (type)",
"ActionLocationSchema (const)",
"ActionNavItem (type)",
"ActionNavItemSchema (const)",
"ActionParam (type)",
"ActionParamIssue (interface)",
"ActionParamSchema (const)",
"ActionSchema (const)",
"ActionType (const)",
Expand Down Expand Up @@ -3374,6 +3379,7 @@
"ReportNavItemSchema (const)",
"ReportSchema (const)",
"ReportType (const)",
"ResolvedActionParam (interface)",
"ResponsiveConfig (type)",
"ResponsiveConfigSchema (const)",
"ResponsiveStyles (type)",
Expand Down Expand Up @@ -3487,6 +3493,7 @@
"normalizeFilterOperator (function)",
"pageForm (const)",
"reportForm (const)",
"validateActionParams (function)",
"viewForm (const)"
],
"./contracts": [
Expand Down
Loading