Conversation
Standalone A2UI v0.9 prototype demonstrating the AUTHORIZE intent from the A2H protocol mapped to A2UI surfaces. - authorize-transfer.json: v0.9 message sequence (createSurface + updateComponents + updateDataModel) for a 00 transfer approval - index.html: Zero-dependency renderer demo with event logging - README.md: Pattern docs, gaps found (no v0.9 Lit renderer, no TTL component, no post-click state transitions) Part of the A2H→A2UI intent mapping exploration.
- escalation-handoff.json: v0.9 message sequence with context preservation, priority indicator, and three connection method buttons - index.html: standalone vanilla JS renderer - README.md: pattern docs, A2H mapping, gaps analysis
- collect-shipping.json: A2UI v0.9 form with TextField data binding, MultipleChoice for delivery speed, sendDataModel on submit - index.html: Standalone renderer with two-way data binding - README.md: Pattern docs, COLLECT mapping, gaps analysis
COLLECT→COLLECT→INFORM→AUTHORIZE sequence on a single surface. Each step swaps content via updateComponents. Data model accumulates across steps. Includes standalone renderer with step navigation and data model state panel.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of prototypes and a design document for integrating A2H (Agent-to-Human) with A2UI, covering five interaction intents and proposing enhancements to the A2UI specification. A critical finding from the security audit is a high-severity Prototype Pollution vulnerability in the renderer logic of the p3-guided-input and p5-wizard prototypes, stemming from unsafe handling of data paths in component JSON, which could allow an attacker to pollute the global Object.prototype. A medium-severity prototype manipulation issue was also found in the component collection logic. From a code quality perspective, key areas for improvement include refactoring state management in the helper library to avoid global state, correcting data binding logic for input components, and fixing implementation bugs in the demo and prototype pages. On a positive note, the renderers consistently use textContent for displaying text, mitigating direct Cross-Site Scripting (XSS) risks.
| function setPath(obj, path, value) { | ||
| const parts = path.split('/').filter(Boolean); | ||
| let cur = obj; | ||
| for (let i = 0; i < parts.length - 1; i++) { | ||
| if (!(parts[i] in cur)) cur[parts[i]] = {}; | ||
| cur = cur[parts[i]]; | ||
| } | ||
| cur[parts[parts.length - 1]] = value; | ||
| } |
There was a problem hiding this comment.
The setPath function is vulnerable to Prototype Pollution. It recursively traverses the obj based on the path provided in the component JSON. If the path contains sensitive keys like __proto__, constructor, or prototype, it can be used to pollute the global Object.prototype. Since the component JSON is intended to come from an AI agent (which is an untrusted source), this allows an attacker to manipulate the renderer's environment, potentially leading to Denial of Service or Cross-Site Scripting.
| function setPath(obj, path, value) { | |
| const parts = path.split('/').filter(Boolean); | |
| let cur = obj; | |
| for (let i = 0; i < parts.length - 1; i++) { | |
| if (!(parts[i] in cur)) cur[parts[i]] = {}; | |
| cur = cur[parts[i]]; | |
| } | |
| cur[parts[parts.length - 1]] = value; | |
| } | |
| function setPath(obj, path, value) { | |
| const parts = path.split('/').filter(Boolean); | |
| let cur = obj; | |
| for (let i = 0; i < parts.length - 1; i++) { | |
| const key = parts[i]; | |
| if (key === '__proto__' || key === 'constructor' || key === 'prototype') return; | |
| if (!Object.prototype.hasOwnProperty.call(cur, key)) cur[key] = {}; | |
| cur = cur[key]; | |
| } | |
| const lastKey = parts[parts.length - 1]; | |
| if (lastKey !== '__proto__' && lastKey !== 'constructor' && lastKey !== 'prototype') { | |
| cur[lastKey] = value; | |
| } | |
| } |
| const parts = path.split('/').filter(Boolean); | ||
| let cur = obj; | ||
| for (let i = 0; i < parts.length - 1; i++) { | ||
| if (!(parts[i] in cur)) cur[parts[i]] = {}; | ||
| cur = cur[parts[i]]; | ||
| } | ||
| cur[parts[parts.length - 1]] = val; | ||
| } | ||
|
|
There was a problem hiding this comment.
The deepSet function is vulnerable to Prototype Pollution. It recursively traverses the obj based on the path provided in the component JSON. If the path contains sensitive keys like __proto__, constructor, or prototype, it can be used to pollute the global Object.prototype. Since the component JSON is intended to come from an AI agent (which is an untrusted source), this allows an attacker to manipulate the renderer's environment, potentially leading to Denial of Service or Cross-Site Scripting.
| const parts = path.split('/').filter(Boolean); | |
| let cur = obj; | |
| for (let i = 0; i < parts.length - 1; i++) { | |
| if (!(parts[i] in cur)) cur[parts[i]] = {}; | |
| cur = cur[parts[i]]; | |
| } | |
| cur[parts[parts.length - 1]] = val; | |
| } | |
| function deepSet(obj, path, val) { | |
| const parts = path.split('/').filter(Boolean); | |
| let cur = obj; | |
| for (let i = 0; i < parts.length - 1; i++) { | |
| const key = parts[i]; | |
| if (key === '__proto__' || key === 'constructor' || key === 'prototype') return; | |
| if (!Object.prototype.hasOwnProperty.call(cur, key)) cur[key] = {}; | |
| cur = cur[key]; | |
| } | |
| const lastKey = parts[parts.length - 1]; | |
| if (lastKey !== '__proto__' && lastKey !== 'constructor' && lastKey !== 'prototype') { | |
| cur[lastKey] = val; | |
| } | |
| } |
| let _counter = 0; | ||
| function uid(prefix) { | ||
| return `${prefix}-${++_counter}`; | ||
| } |
There was a problem hiding this comment.
The uid function relies on a global _counter. This is problematic for a library as it introduces a shared global state. If multiple parts of an application create surfaces, they will all increment the same counter, which can lead to unpredictable component IDs and potential collisions if not managed carefully. The presence of resetIds suggests awareness of this for testing, but it's not a robust solution for production use.
A better approach is to scope the counter to each surface generation. This could be achieved with a factory function or a class that encapsulates the counter state.
function createIdGenerator(prefix) {
let counter = 0;
return (suffix) => `${prefix}-${suffix}-${++counter}`;
}
// Then, inside each `create...Surface` function:
// const uid = createIdGenerator('auth');
// const descId = uid('desc');| if (f.type === 'select') { | ||
| fieldComponents.push({ | ||
| id: fieldId, | ||
| component: 'ChoicePicker', | ||
| options: (f.options || []).map(o => typeof o === 'string' ? { label: o, value: o } : o), | ||
| value: f.value != null ? { path: f.path } : undefined, | ||
| data: { path: f.path }, | ||
| }); | ||
| } else { | ||
| fieldComponents.push({ | ||
| id: fieldId, | ||
| component: 'TextField', | ||
| placeholder: f.label, | ||
| value: f.value != null ? { path: f.path } : undefined, | ||
| data: { path: f.path }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The current implementation for TextField and ChoicePicker components has two issues:
- The
valueproperty is only bound to a data modelpathif an initialf.valueis provided. For two-way data binding to work, thevalueshould always be bound to its path in the data model, regardless of whether an initial value is present. The initial value itself should be set in thedataModelobject passed to the function. - A non-standard
dataproperty is being added with the path. This property is not part of the A2UI v0.9 spec for these components and is not used by the renderers in the prototypes.
To fix this, the value property should always be set to { path: f.path }, and the data property should be removed.
if (f.type === 'select') {
fieldComponents.push({
id: fieldId,
component: 'ChoicePicker',
options: (f.options || []).map(o => typeof o === 'string' ? { label: o, value: o } : o),
value: { path: f.path },
});
} else {
fieldComponents.push({
id: fieldId,
component: 'TextField',
placeholder: f.label,
value: { path: f.path },
});
}| case 'TextField': { | ||
| const el = document.createElement('input'); | ||
| el.className = 'c-TextField'; | ||
| el.placeholder = comp.placeholder || ''; | ||
| const v = resolve(comp.value); | ||
| if (v) el.value = v; | ||
| return el; | ||
| } | ||
| case 'ChoicePicker': { | ||
| const el = document.createElement('select'); | ||
| el.className = 'c-ChoicePicker'; | ||
| const selected = resolve(comp.value); | ||
| for (const opt of (comp.options || [])) { | ||
| const o = document.createElement('option'); | ||
| o.value = opt.value; o.textContent = opt.label; | ||
| if (opt.value === selected) o.selected = true; | ||
| el.appendChild(o); | ||
| } | ||
| return el; | ||
| } |
There was a problem hiding this comment.
The renderers for TextField and ChoicePicker are missing two-way data binding. They set the initial value from the data model but don't update the model when the user interacts with the input. As a result, when the form is submitted, the initial data is sent, not the user's input, which makes the 'Fill & submit' feature of the COLLECT prototype non-functional.
You should add event listeners (input for TextField, change for ChoicePicker) to update the dataModel as the user makes changes. You can see a working example of this in p5-wizard/index.html.
|
|
||
| function renderSurface(messages, container) { | ||
| let surfaceConfig = null; | ||
| let components = {}; |
There was a problem hiding this comment.
The components object is initialized as a plain object and then populated using component IDs provided in the JSON. If a component has an id of __proto__, it will overwrite the prototype of the components object. While the impact is limited in this specific demo, this pattern can lead to unexpected behavior or security issues if the object is used in a way that relies on its prototype chain. Using Object.create(null) ensures that the object has no prototype and is safe from such manipulation.
| let components = {}; | |
| let components = Object.create(null); |
- Text variant 'label' → 'caption' (not in v0.9 spec enum) - Divider renderer: add missing .c-Divider class to <hr> element - CSS: consolidate caption styling, use #id for TTL red color - README: fix cd path to match actual folder name
- Replace invalid Text variant 'label' with 'caption' (5 instances) Same bug as P1 — 'label' is not in the v0.9 Text variant enum - Replace invalid Icon name 'support_agent' with 'person' - Replace invalid Icon name 'priority_high' with 'warning' - Update HTML renderer CSS to match corrected component values - Remove orphaned 'label' variant CSS rule
- Fix variant: 'label' bug on speed-label Text (not in enum) → 'caption' - Replace non-existent MultipleChoice with ChoicePicker component - Add variant: 'mutuallyExclusive', displayStyle: 'chips' - Update data model speed value to string array per ChoicePicker spec - Fix Icon name 'local_shipping' (not in catalog enum) → 'shoppingCart' - Add icon name mapping in renderer (camelCase catalog → snake_case Material Symbols) - Update CSS classes from c-MultipleChoice to c-ChoicePicker - Fix README cd path to match actual directory name - Update README to reference ChoicePicker instead of MultipleChoice
- Remove _comment fields from messages 3,5,7 (violates additionalProperties:false) - Fix renderer delays array to cover all 9 messages with proper progressive timing - Fix renderer labels to persist status text across dataModel+components pairs - Fix README message count (was 4, actually 9 messages in 4 logical steps)
- Remove _comment fields (violate additionalProperties: false)
- MultipleChoice → ChoicePicker with structured {label,value} options
- variant: 'label' → variant: 'caption' (label not in Text enum)
- Data model category: '' → category: [] (ChoicePicker uses DynamicStringList)
- Update HTML renderer for ChoicePicker + array value display
- Document all fixes in README
- Add note about recurring bugs found across prototypes - Update P4 rating 3→4/5 (JSON was fixed during review) - Update P5 rating 3.5→4/5 (bugs fixed, note v0.9.1 improvement) - Frame recurring errors as spec feedback (what devs expect)
RED fixes:
- Remove all _comment fields from P4/P5 v0.9.1 JSON (violate additionalProperties: false)
- P3 v0.9.0: rename event submitShipping → a2h.collect.submit (naming convention)
- Helper lib: fix variant 'label' → 'caption' on Text components (invalid variant)
YELLOW fixes:
- DESIGN.md: update surface ID convention from colons to dashes (matching actual JSON files)
- P3 v0.9.0: fix surfaceId collect-shipping → a2h-collect-shipping-001
- Remove dead allActionComps code from helper library
- P1 v0.9.0: add formatCurrency example to transfer amount
Note: P4 button IDs already use btn-{action} convention — no change needed.
links to twilio repo https://github.com/twilio-labs/Agent2Human
This PR exists as a convenient place to review and discuss the A2H × A2UI prototype exploration. It is not intended to be merged into main.
What's here
DESIGN.md) with findings, comparison metrics, and spec recommendationslib/a2h-a2ui.js)Key finding
A2UI v0.9 covers ~85% of A2H needs today. Four small spec additions —
visiblebinding, Buttonlabelprop,ProgressIndicator,KeyValue— would close most remaining gaps.