fix(plugin-chatbot): widgets pass only whitelisted DOM props to the host element (#4431)

.changeset/chatbot-dom-prop-whitelist-4431.md

@@ -0,0 +1,15 @@
+---
+'@object-ui/core': minor
+'@object-ui/plugin-chatbot': patch
+'@object-ui/fields': patch
+---
+
+`chatbot` and `chatbot-enhanced` now pass only whitelisted DOM props to their host element (objectui#4431)
+
+Both registrations destructured `schema` and `className` and forwarded everything else. `SchemaRenderer` hands a registered component the authored node's own keys, the contents of its `props` container, the ARIA it resolved and the host's trailing props — so all of it became attributes on the chat root `div`, because React passes unknown lowercase attributes through in silence and stringifies object values. Measured through the real SDUI path with a data-source adapter attached: **14 non-DOM attributes on each widget**, including `datasource="[object Object]"` (the injected adapter, which only appears on a deployment that really loads data) and a camelCase `arialabel` sitting next to the resolved `aria-label`, so the element carried each ARIA value twice under two spellings — one of them meaningless to assistive technology.
+
+Both are now consume-or-whitelist: configuration is read off `schema` as before, the evaluated `disabled` verdict is consumed by name, and only `toDomProps`' output reaches the element. The resolved `aria-label` / `aria-describedby`, `role`, `id`, `tabIndex` and the `data-*` family still arrive — dropping them would have been an accessibility regression dressed as a leak fix, so the pin asserts the delivered set exactly, not just the absent one. `chatbot-floating` is untouched: its content mounts through a portal and its root never spread.
+
+`@object-ui/core` gains the shared executor this migration needs (`utils/dom-props.ts`): `toDomProps` for the SDUI widget contract, plus `pickDomProps` — the mechanism — for a package whose own contract declares a different key set. That is the objectui#4409 dependency direction: plugin packages declare `@object-ui/core` and must not grow a dependency on `@object-ui/fields` to reach a whitelist.
+
+`@object-ui/fields` keeps its own key list and its compile-time bindings, and now executes them through core's mechanism. Its behaviour is unchanged and its exported `DomProps<P>` is the same structural type. The two lists differ for measured reasons and no longer can drift silently: `name` and `disabled` are legal only on form controls, which is what every field widget renders and what `FieldWidgetComponentProps` declares, while `role` is resolved by `SchemaRenderer` for every SDUI node and is not part of the field contract. A new assertion binds every shared key in both directions, with `role` named as the single deliberate exception.

packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx

@@ -508,34 +508,6 @@ interface LedgerEntry {
 }
 
 const LEAK_LEDGER: Readonly<Record<string, LedgerEntry>> = {
-  /* ── plugin-chatbot: the whole node, minus the two keys it destructures ──── */
-  //
-  // Both registrations are `({ schema, className, ...props }) => … {...props}`,
-  // so everything except those two becomes an attribute — including the injected
-  // `datasource` adapter, which only a host that really loads data supplies.
-  // `chatbot-floating` is NOT here: it is clean.
-  'plugin-chatbot:chatbot': {
-    attributes: [
-      'ariadescribedby', 'arialabel', 'bind', 'colorvariant', 'datasource',
-      'events', 'name', 'props', 'reference_to', 'zzcanary', 'zzcanarycamel',
-      'zzcanarynum', 'zzcanaryobj', 'zzcanaryprop',
-    ],
-    reason:
-      'renderer.tsx destructures only `schema` and `className` before spreading ' +
-      'the rest onto the chatbot root; every other injected and authored key ' +
-      'becomes a DOM attribute.',
-    issue: 'objectui#4431',
-  },
-  'plugin-chatbot:chatbot-enhanced': {
-    attributes: [
-      'ariadescribedby', 'arialabel', 'bind', 'colorvariant', 'datasource',
-      'events', 'name', 'props', 'reference_to', 'zzcanary', 'zzcanarycamel',
-      'zzcanarynum', 'zzcanaryobj', 'zzcanaryprop',
-    ],
-    reason: 'the same spread shape as `plugin-chatbot:chatbot`, same 14 attributes.',
-    issue: 'objectui#4431',
-  },
-
   /* ── plugin-dashboard: the OPEN TAIL a deny-list cannot close ───────────── */
   //
   // Read these two rows against what is NOT in them. Every one of the seven keys

packages/core/src/index.ts

@@ -14,6 +14,12 @@ export * from './registry/PluginScopeImpl.js';
 export * from './registry/WidgetRegistry.js';
 export * from './validation/index.js';
 export * from './builder/schema-builder.js';
+// The DOM pass-through whitelist of the SDUI widget prop contract
+// (objectui#4425 phase 2): a registered widget's host element receives only
+// what `toDomProps` passes — everything else is consumed or dropped.
+// `@object-ui/fields` executes the SAME mechanism against its own declared key
+// list (`pickDomProps`), so there is one judge, not two.
+export * from './utils/dom-props.js';
 export * from './utils/filter-converter.js';
 export * from './utils/managedBy.js';
 export * from './utils/extract-records.js';

packages/core/src/utils/dom-props.ts

@@ -0,0 +1,171 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * The RUNTIME EXECUTOR of the SDUI widget prop contract's DOM pass-through
+ * (objectui#4425 phase 2, promoting objectui#3291's whitelist from
+ * `packages/fields` to every registered SDUI widget).
+ *
+ * ## The defect this closes
+ *
+ * `SchemaRenderer` hands a registered component the authored node's own keys,
+ * the contents of its `props` container, the ARIA it resolved, and the host's
+ * trailing props — then a widget ending in a bare `{...props}` spread puts all
+ * of it on a DOM element. React passes unknown lowercase attributes through in
+ * complete silence and stringifies object values, so the failure is invisible:
+ *
+ * ```
+ * <div id="canary-node" name="canary_node" bind="data.revenue"
+ *      events="[object Object]" arialabel="Canary label"
+ *      datasource="[object Object]" props="[object Object]" …>
+ * ```
+ *
+ * That is one real measurement, from `plugin-chatbot:chatbot` on the sweep gate
+ * (`packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx`).
+ *
+ * ## Why a whitelist, and not a list of keys to drop
+ *
+ * Measured, not assumed. objectui#4357/PR #4428 answered the same defect in
+ * `plugin-dashboard` with a deny-list: it closed every one of the seven keys it
+ * enumerated, and the sweep still found the OPEN TAIL leaking on the same two
+ * components — `zzcanary`, `reference_to`, an authored `props: { colorVariant }`
+ * — because a deny-list can only name what already exists. The set of keys an
+ * author may put on a node is unbounded; the set a widget may put on an element
+ * is declared. A deny-list bounded by enumeration cannot be finished; a
+ * whitelist bounded by declaration can.
+ *
+ * ## The set, and why each key is in it
+ *
+ * Every key here is either injected by `SchemaRenderer` for the whole SDUI
+ * contract, or a GLOBAL HTML attribute legal on whatever element a widget
+ * renders — this helper cannot know that element, so nothing element-specific
+ * belongs here:
+ *
+ *  - `id` — global; `SchemaRenderer` carries the node's id.
+ *  - `className` — global (`class`); the styling channel AGENTS.md #3 requires
+ *    every widget to expose, and the carrier of ADR-0065's scope class.
+ *  - `role` — the third member of `@objectstack/spec`'s `AriaPropsSchema`,
+ *    resolved and injected by `SchemaRenderer.resolveAriaProps` alongside
+ *    `aria-label` / `aria-describedby`. Withholding it would make an authored
+ *    `role` type-check, read as supported, and silently never arrive — the
+ *    DECLARED-BUT-NOT-DELIVERED failure this repo treats as first class.
+ *  - `tabIndex`, `autoFocus` — global HTML attributes.
+ *  - `onClick`, `onBlur`, `onFocus` — React synthetic handlers, which never
+ *    become attributes at all.
+ *
+ * Plus the two OPEN families, matched by prefix so no per-attribute list is
+ * needed: `aria-*` (the resolved ARIA above — note the camelCase `ariaLabel` /
+ * `ariaDescribedBy` the author wrote do NOT match, which is the point: they are
+ * meaningless to assistive technology and the resolver already emitted the
+ * hyphenated forms) and `data-*` (open in HTML too, and how `data-obj-id` /
+ * `data-obj-type` / the debug attributes reach the element).
+ *
+ * ## Deliberately NOT here
+ *
+ * `name` and `disabled` are legal only on FORM CONTROLS, not on the container a
+ * typical SDUI widget renders — `name` on a `<div>` is exactly one of the 14
+ * leaked attributes this closes. `@object-ui/fields` declares both, because
+ * every widget there renders a control and its contract
+ * (`FieldWidgetComponentProps`) says so; that package keeps its own declared key
+ * list and calls {@link pickDomProps} with it. One MECHANISM, two DECLARATIONS,
+ * each bound to the contract it executes — not two judges.
+ *
+ * A semantic prop is CONSUMED, never forwarded: a widget that can be disabled
+ * reads `disabled` and applies it (see `plugin-chatbot`'s registrations), which
+ * is what keeps `disabledOn` working without putting `disabled="true"` on a
+ * `<div>`.
+ *
+ * Deliberate DOM pass-through beyond this set stays available the objectui#4435
+ * way — DECLARE it (`interface Props extends HTMLAttributes<HTMLDivElement>`)
+ * and forward it by name. Do not reopen the spread (AGENTS.md #0.1: fix the
+ * contract, never widen the consumer).
+ */
+export const SDUI_DOM_PASS_THROUGH_KEYS = [
+  'id',
+  'className',
+  'role',
+  'tabIndex',
+  'autoFocus',
+  'onClick',
+  'onBlur',
+  'onFocus',
+] as const;
+
+/** A key {@link toDomProps} forwards by name (the open families are prefixes). */
+export type SduiDomPassThroughKey = (typeof SDUI_DOM_PASS_THROUGH_KEYS)[number];
+
+/**
+ * `aria-*` and `data-*` are open families — open in HTML, and open in the
+ * widget contract — so they are matched by prefix rather than enumerated.
+ *
+ * Note what this does NOT match: `dataSource`. The injected data-source ADAPTER
+ * is not a `data-` attribute, and a prefix test that accepted it would put
+ * `datasource="[object Object]"` on the element of every data-bound widget —
+ * the one leak a schema-only measurement cannot see, because it only appears
+ * when a host really supplies an adapter (objectui#4428).
+ */
+export function isOpenDomAttributeFamily(key: string): boolean {
+  return key.startsWith('data-') || key.startsWith('aria-');
+}
+
+/** The subset of `P` a whitelist of `K` forwards, with each key's declared type. */
+export type DomProps<P, K extends string = SduiDomPassThroughKey> = Pick<
+  P,
+  Extract<keyof P, K>
+> & {
+  [Key in Extract<keyof P, `data-${string}` | `aria-${string}`>]: P[Key];
+};
+
+/**
+ * The MECHANISM: keep the keys a caller's contract declares as DOM-safe plus
+ * the two open families, drop everything else.
+ *
+ * Exported for the one caller whose contract declares a different set —
+ * `@object-ui/fields`, whose widgets render form controls and whose
+ * `FieldWidgetComponentProps` therefore declares `name` and `disabled` as DOM
+ * pass-through (objectui#3291). Everything else should call {@link toDomProps},
+ * which is this function with the SDUI contract's own set already applied.
+ */
+export function pickDomProps<P extends object, K extends string>(
+  props: P,
+  keys: readonly K[],
+): DomProps<P, K> {
+  const allowed: ReadonlySet<string> = new Set<string>(keys);
+  const domProps: Record<string, unknown> = {};
+  for (const key of Object.keys(props)) {
+    if (allowed.has(key) || isOpenDomAttributeFamily(key)) {
+      domProps[key] = (props as Record<string, unknown>)[key];
+    }
+  }
+  return domProps as DomProps<P, K>;
+}
+
+/**
+ * Keep only what may legitimately become a DOM attribute on a registered SDUI
+ * widget's host element, and drop everything else — the injected `schema`, the
+ * data-source adapter, the authored node's metadata, the `props` container, and
+ * any extra key an author put on the node.
+ *
+ * Replaces the bare `{...props}` spread in a widget registration:
+ *
+ * ```tsx
+ * // before — forwards whatever `SchemaRenderer` handed it
+ * ({ schema, className, ...props }) => <Chatbot className={className} {...props} />
+ *
+ * // after
+ * ({ schema, className, ...props }) => (
+ *   <Chatbot {...toDomProps(props)} className={className} />
+ * )
+ * ```
+ *
+ * Semantic props a widget INTERPRETS (`disabled`, and everything it reads off
+ * `schema`) are read as before; this function only governs what gets spread.
+ */
+export function toDomProps<P extends object>(props: P): DomProps<P> {
+  return pickDomProps(props, SDUI_DOM_PASS_THROUGH_KEYS);
+}

packages/fields/src/widgets/toDomProps.ts

@@ -6,6 +6,11 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+import {
+  pickDomProps,
+  type SduiDomPassThroughKey,
+  type DomProps as CoreDomProps,
+} from '@object-ui/core';
 import type { FieldWidgetComponentProps, FieldWidgetDomProps } from './types';
 
 /**
@@ -62,6 +67,34 @@ import type { FieldWidgetComponentProps, FieldWidgetDomProps } from './types';
  * open spread. If a field node should be able to author one, DECLARE it on
  * `FieldWidgetComponentProps` first and add it here — do not reopen the spread
  * (AGENTS.md #0.1: fix the contract, never widen the consumer).
+ *
+ * ## Where the mechanism lives now, and why the key list stayed here
+ *
+ * objectui#4425 phase 2 promoted this whitelist to the SDUI widget contract
+ * generally, so the MECHANISM — filter by a declared key list plus the `aria-*`
+ * / `data-*` open families — moved to `@object-ui/core`
+ * (`utils/dom-props.ts`, `pickDomProps`), where every plugin package can reach
+ * it without depending on `@object-ui/fields` (the objectui#4409
+ * dependency-direction method: `plugin-chatbot` declares `@object-ui/core` and
+ * does not declare this package).
+ *
+ * The KEY LIST did not move, because it is not the same list. Measured on
+ * objectui#4431:
+ *
+ *  - `name` and `disabled` are here and NOT in the SDUI set: both are legal
+ *    only on form controls, which is what every widget in this package renders
+ *    and what `FieldWidgetComponentProps` declares. `name` on the `<div>` an
+ *    SDUI container widget renders is a leak — one of the 14 attributes #4431
+ *    closed — and two widgets here already hand-strip it when they spread onto
+ *    something that is not a control (`ObjectRefField`'s trigger,
+ *    `FileField`'s dropzone).
+ *  - `role` is in the SDUI set and NOT here: `SchemaRenderer` resolves it for
+ *    every node from `@objectstack/spec`'s `AriaPropsSchema`, while this
+ *    contract deliberately does not declare it (see "Deliberately NOT
+ *    forwarded" above).
+ *
+ * So: one mechanism, two declarations, each bound by the assertions below to
+ * the contract it executes. Not two judges.
  */
 const DOM_PASS_THROUGH_KEYS = [
   /* ── The contract's own "DOM pass-through" block, verbatim ─────────────── */
@@ -126,22 +159,24 @@ type EveryDeclaredDomKeyIsForwarded =
 const _everyDeclaredDomKeyIsForwarded: EveryDeclaredDomKeyIsForwarded = true;
 void _everyDeclaredDomKeyIsForwarded;
 
-const DOM_PASS_THROUGH: ReadonlySet<string> = new Set<string>(DOM_PASS_THROUGH_KEYS);
-
 /**
- * `aria-*` is declared on the contract as React's `AriaAttributes`; `data-*`
- * is declared as an open template-literal family (open in HTML too, and the
- * only open family the contract has). Both are matched by prefix so the helper
- * needs no per-attribute list.
+ * Direction 3, added with the core lift (objectui#4431): every key of the SDUI
+ * contract's own whitelist that this contract also declares must be forwarded
+ * here too, so the two lists cannot silently drift apart. `role` is the ONE
+ * measured exception — the SDUI contract declares it, this one does not — and
+ * naming it in the `Exclude` is what makes the divergence a deliberate,
+ * reviewable act rather than an omission.
+ *
+ * Catches: a key added to the shared SDUI set that field widgets quietly stop
+ * delivering.
  */
-function isOpenDomFamily(key: string): boolean {
-  return key.startsWith('data-') || key.startsWith('aria-');
-}
+type SharedSduiKey = Exclude<SduiDomPassThroughKey, 'role'>;
+type EverySharedSduiKeyIsForwarded = SharedSduiKey extends DomPassThroughKey ? true : never;
+const _everySharedSduiKeyIsForwarded: EverySharedSduiKeyIsForwarded = true;
+void _everySharedSduiKeyIsForwarded;
 
 /** The subset of `P` this helper forwards, with each key's declared type. */
-export type DomProps<P> = Pick<P, Extract<keyof P, DomPassThroughKey>> & {
-  [K in Extract<keyof P, `data-${string}` | `aria-${string}`>]: P[K];
-};
+export type DomProps<P> = CoreDomProps<P, DomPassThroughKey>;
 
 /**
  * Keep only what may legitimately become a DOM attribute, and drop everything
@@ -165,11 +200,5 @@ export type DomProps<P> = Pick<P, Extract<keyof P, DomPassThroughKey>> & {
  * governs what gets spread.
  */
 export function toDomProps<P extends object>(props: P): DomProps<P> {
-  const domProps: Record<string, unknown> = {};
-  for (const key of Object.keys(props)) {
-    if (DOM_PASS_THROUGH.has(key) || isOpenDomFamily(key)) {
-      domProps[key] = (props as Record<string, unknown>)[key];
-    }
-  }
-  return domProps as DomProps<P>;
+  return pickDomProps(props, DOM_PASS_THROUGH_KEYS);
 }