Widen front component crossing attributes to aria-*, data-* and draggable (#22614)
Only a closed allow-list of props crossed the front-component worker→host boundary (`id, className, style, title, tabIndex, role, aria-label, aria-hidden, data-testid`), so arbitrary `aria-*`/`data-*` attributes and `draggable` never reached the host DOM. That breaks headless UI libraries (Radix, cmdk, react-aria) that drive styling/state through those attributes. This widens the crossing set to all `aria-*`, all `data-*`, and `draggable`: - `draggable` becomes an enumerated remote property (it's a DOM IDL property React may set as a property, bypassing `setAttribute`, so it can't ride the prefix path). - Arbitrary `aria-*`/`data-*` are forwarded in the worker by patching `setAttribute`/`removeAttribute` through remote-dom's attribute channel, only for names not already synced as observed attributes. Security: only inert `aria-*`/`data-*`/`draggable` cross, and they still route through the host `filterProps` guards (non-function `on*` dropped, `javascript:` URLs denied) — nothing bypasses them. The enumerated `aria-label`/`aria-hidden`/`data-testid` keep their existing path. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22614?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+31
@@ -0,0 +1,31 @@
|
||||
import { isAriaOrDataAttribute } from '../isAriaOrDataAttribute';
|
||||
|
||||
describe('isAriaOrDataAttribute', () => {
|
||||
it('should accept arbitrary aria-* attributes', () => {
|
||||
expect(isAriaOrDataAttribute('aria-selected')).toBe(true);
|
||||
expect(isAriaOrDataAttribute('aria-activedescendant')).toBe(true);
|
||||
});
|
||||
|
||||
it('should accept arbitrary data-* attributes', () => {
|
||||
expect(isAriaOrDataAttribute('data-state')).toBe(true);
|
||||
expect(isAriaOrDataAttribute('data-radix-collection-item')).toBe(true);
|
||||
});
|
||||
|
||||
it('should be case-insensitive on the prefix', () => {
|
||||
expect(isAriaOrDataAttribute('DATA-State')).toBe(true);
|
||||
expect(isAriaOrDataAttribute('Aria-Label')).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject non aria/data attributes', () => {
|
||||
expect(isAriaOrDataAttribute('draggable')).toBe(false);
|
||||
expect(isAriaOrDataAttribute('class')).toBe(false);
|
||||
expect(isAriaOrDataAttribute('href')).toBe(false);
|
||||
expect(isAriaOrDataAttribute('onclick')).toBe(false);
|
||||
expect(isAriaOrDataAttribute('id')).toBe(false);
|
||||
});
|
||||
|
||||
it('should not match names that merely contain the prefix', () => {
|
||||
expect(isAriaOrDataAttribute('metadata-id')).toBe(false);
|
||||
expect(isAriaOrDataAttribute('x-aria-label')).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,9 @@
|
||||
const ARIA_OR_DATA_ATTRIBUTE_PREFIXES = ['aria-', 'data-'];
|
||||
|
||||
export const isAriaOrDataAttribute = (attributeName: string): boolean => {
|
||||
const lowercasedAttributeName = attributeName.toLowerCase();
|
||||
|
||||
return ARIA_OR_DATA_ATTRIBUTE_PREFIXES.some((attributePrefix) =>
|
||||
lowercasedAttributeName.startsWith(attributePrefix),
|
||||
);
|
||||
};
|
||||
+94
@@ -0,0 +1,94 @@
|
||||
import { ALLOWED_HTML_ELEMENTS } from '@/constants/AllowedHtmlElements';
|
||||
import { isAriaOrDataAttribute } from '@/remote/utils/isAriaOrDataAttribute';
|
||||
|
||||
const ATTRIBUTE_NAME_TO_ELEMENT_PROPERTY_NAME: Record<string, string> = {
|
||||
className: 'className',
|
||||
class: 'className',
|
||||
|
||||
htmlFor: 'htmlFor',
|
||||
for: 'htmlFor',
|
||||
|
||||
tabIndex: 'tabIndex',
|
||||
tabindex: 'tabIndex',
|
||||
|
||||
srcDoc: 'srcDoc',
|
||||
srcdoc: 'srcDoc',
|
||||
};
|
||||
|
||||
type RemoteElementWithAttributeUpdater = Element &
|
||||
Record<string, unknown> & {
|
||||
updateRemoteAttribute: (attributeName: string, value?: string) => void;
|
||||
};
|
||||
|
||||
export const patchRemoteElementAttributes = (): void => {
|
||||
for (const allowedHtmlElement of ALLOWED_HTML_ELEMENTS) {
|
||||
const elementConstructor = customElements.get(allowedHtmlElement.tag);
|
||||
|
||||
if (!elementConstructor) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const attributeNamesAlreadySyncedByRemoteDom = new Set<string>(
|
||||
(
|
||||
elementConstructor as CustomElementConstructor & {
|
||||
observedAttributes?: string[];
|
||||
}
|
||||
).observedAttributes ?? [],
|
||||
);
|
||||
|
||||
const shouldForwardAttributeAcrossBoundary = (
|
||||
attributeName: string,
|
||||
): boolean =>
|
||||
isAriaOrDataAttribute(attributeName) &&
|
||||
!attributeNamesAlreadySyncedByRemoteDom.has(attributeName);
|
||||
|
||||
const originalSetAttribute = elementConstructor.prototype.setAttribute as (
|
||||
attributeName: string,
|
||||
attributeValue: string,
|
||||
) => void;
|
||||
|
||||
elementConstructor.prototype.setAttribute = function (
|
||||
this: RemoteElementWithAttributeUpdater,
|
||||
attributeName: string,
|
||||
attributeValue: string,
|
||||
) {
|
||||
const mappedElementPropertyName =
|
||||
ATTRIBUTE_NAME_TO_ELEMENT_PROPERTY_NAME[attributeName];
|
||||
|
||||
if (mappedElementPropertyName) {
|
||||
this[mappedElementPropertyName] = attributeValue;
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
originalSetAttribute.call(this, attributeName, attributeValue);
|
||||
|
||||
if (shouldForwardAttributeAcrossBoundary(attributeName)) {
|
||||
this.updateRemoteAttribute(attributeName, attributeValue);
|
||||
}
|
||||
};
|
||||
|
||||
const originalRemoveAttribute = elementConstructor.prototype
|
||||
.removeAttribute as (attributeName: string) => void;
|
||||
|
||||
elementConstructor.prototype.removeAttribute = function (
|
||||
this: RemoteElementWithAttributeUpdater,
|
||||
attributeName: string,
|
||||
) {
|
||||
const mappedElementPropertyName =
|
||||
ATTRIBUTE_NAME_TO_ELEMENT_PROPERTY_NAME[attributeName];
|
||||
|
||||
if (mappedElementPropertyName) {
|
||||
this[mappedElementPropertyName] = undefined;
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
originalRemoveAttribute.call(this, attributeName);
|
||||
|
||||
if (shouldForwardAttributeAcrossBoundary(attributeName)) {
|
||||
this.updateRemoteAttribute(attributeName);
|
||||
}
|
||||
};
|
||||
}
|
||||
};
|
||||
-46
@@ -1,46 +0,0 @@
|
||||
import { ALLOWED_HTML_ELEMENTS } from '@/constants/AllowedHtmlElements';
|
||||
|
||||
const ATTRIBUTE_TO_PROPERTY_MAP: Record<string, string> = {
|
||||
className: 'className',
|
||||
class: 'className',
|
||||
|
||||
htmlFor: 'htmlFor',
|
||||
for: 'htmlFor',
|
||||
|
||||
tabIndex: 'tabIndex',
|
||||
tabindex: 'tabIndex',
|
||||
|
||||
srcDoc: 'srcDoc',
|
||||
srcdoc: 'srcDoc',
|
||||
};
|
||||
|
||||
export const patchRemoteElementSetAttribute = (): void => {
|
||||
for (const elementConfig of ALLOWED_HTML_ELEMENTS) {
|
||||
const elementConstructor = customElements.get(elementConfig.tag);
|
||||
|
||||
if (!elementConstructor) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const originalSetAttribute = elementConstructor.prototype.setAttribute as (
|
||||
name: string,
|
||||
value: string,
|
||||
) => void;
|
||||
|
||||
elementConstructor.prototype.setAttribute = function (
|
||||
this: Element & Record<string, unknown>,
|
||||
name: string,
|
||||
value: string,
|
||||
) {
|
||||
const propertyName = ATTRIBUTE_TO_PROPERTY_MAP[name];
|
||||
|
||||
if (propertyName) {
|
||||
this[propertyName] = value;
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
originalSetAttribute.call(this, name, value);
|
||||
};
|
||||
}
|
||||
};
|
||||
Reference in New Issue
Block a user