diff --git a/packages/twenty-front-component-renderer/src/__stories__/html-tag/grouping/div/div-crossing-attributes.front-component.tsx b/packages/twenty-front-component-renderer/src/__stories__/html-tag/grouping/div/div-crossing-attributes.front-component.tsx new file mode 100644 index 0000000000..068c93e845 --- /dev/null +++ b/packages/twenty-front-component-renderer/src/__stories__/html-tag/grouping/div/div-crossing-attributes.front-component.tsx @@ -0,0 +1,30 @@ +import { defineFrontComponent } from 'twenty-sdk/define'; + +import { FrontComponentCard } from '@/__stories__/shared/front-components/front-component-card'; + +const DivCrossingAttributesFrontComponent = () => ( + +
+ content +
+ + danger + +
+); + +export default defineFrontComponent({ + universalIdentifier: 'fc-div-cross-00000000-0000-0000-0000-000000000021', + name: 'div-crossing-attributes-front-component', + description: + 'Front component proving arbitrary aria-*/data-*/draggable attributes cross to the host DOM while dangerous URLs stay filtered', + component: DivCrossingAttributesFrontComponent, +}); diff --git a/packages/twenty-front-component-renderer/src/__stories__/html-tag/grouping/div/div-crossing-attributes.stories.tsx b/packages/twenty-front-component-renderer/src/__stories__/html-tag/grouping/div/div-crossing-attributes.stories.tsx new file mode 100644 index 0000000000..ed7131d8c8 --- /dev/null +++ b/packages/twenty-front-component-renderer/src/__stories__/html-tag/grouping/div/div-crossing-attributes.stories.tsx @@ -0,0 +1,49 @@ +import { type Meta } from '@storybook/react-vite'; +import { expect, waitFor, within } from 'storybook/test'; + +import { FrontComponentRenderer } from '@/host/components/FrontComponentRenderer'; +import { + FRONT_COMPONENT_STORY_DEFAULT_ARGS, + resetFrontComponentStoryMocks, +} from '@/__stories__/shared/test-utils/createFrontComponentStoryMeta'; +import { expectFrontComponentMounted } from '@/__stories__/shared/test-utils/matchers/expectFrontComponentMounted'; +import { expectAttributesReflected } from '@/__stories__/shared/test-utils/matchers/expectPropertyReflected'; +import { runFrontComponentStory } from '@/__stories__/shared/test-utils/runFrontComponentStory'; + +const meta: Meta = { + title: 'FrontComponent/HtmlTag/Grouping/Div/CrossingAttributes', + component: FrontComponentRenderer, + parameters: { layout: 'centered' }, + args: FRONT_COMPONENT_STORY_DEFAULT_ARGS, + beforeEach: resetFrontComponentStoryMocks, +}; + +export default meta; + +export const CrossingAttributes = runFrontComponentStory({ + frontComponentBundleName: 'div-crossing-attributes', + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await expectFrontComponentMounted(canvas); + + await expectAttributesReflected({ + canvas, + attributes: { + role: 'option', + 'aria-selected': 'true', + 'aria-activedescendant': 'item-2', + 'data-state': 'open', + 'data-count': '3', + draggable: 'true', + }, + }); + + await waitFor(() => { + const dangerLink = canvas.queryByTestId('danger-link'); + + expect(dangerLink).not.toBeNull(); + expect(dangerLink?.getAttribute('href')).toBeNull(); + }); + }, +}); diff --git a/packages/twenty-front-component-renderer/src/constants/HtmlCommonProperties.ts b/packages/twenty-front-component-renderer/src/constants/HtmlCommonProperties.ts index fa50bd218f..6ae2fef8e0 100644 --- a/packages/twenty-front-component-renderer/src/constants/HtmlCommonProperties.ts +++ b/packages/twenty-front-component-renderer/src/constants/HtmlCommonProperties.ts @@ -10,4 +10,5 @@ export const HTML_COMMON_PROPERTIES: Record = { 'aria-label': { type: 'string', optional: true }, 'aria-hidden': { type: 'boolean', optional: true }, 'data-testid': { type: 'string', optional: true }, + draggable: { type: 'string', optional: true }, }; diff --git a/packages/twenty-front-component-renderer/src/host/utils/__tests__/filterProps.test.ts b/packages/twenty-front-component-renderer/src/host/utils/__tests__/filterProps.test.ts index 8b2306f3fa..149c7a3586 100644 --- a/packages/twenty-front-component-renderer/src/host/utils/__tests__/filterProps.test.ts +++ b/packages/twenty-front-component-renderer/src/host/utils/__tests__/filterProps.test.ts @@ -60,4 +60,33 @@ describe('filterProps', () => { expect(result.href).toBe('https://twenty.com'); }); + + it('should forward arbitrary aria-* and data-* attributes', () => { + const result = filter( + { + 'aria-selected': 'true', + 'aria-activedescendant': 'item-2', + 'data-state': 'open', + 'data-count': '3', + }, + 'div', + ); + + expect(result['aria-selected']).toBe('true'); + expect(result['aria-activedescendant']).toBe('item-2'); + expect(result['data-state']).toBe('open'); + expect(result['data-count']).toBe('3'); + }); + + it('should forward the draggable attribute', () => { + expect(filter({ draggable: 'true' }, 'div').draggable).toBe('true'); + expect(filter({ draggable: true }, 'div').draggable).toBe(true); + }); + + it('should still drop a non-function on* handler smuggled as a data-adjacent prop', () => { + const result = filter({ onClick: 'alert(1)', 'data-state': 'open' }, 'div'); + + expect('onClick' in result).toBe(false); + expect(result['data-state']).toBe('open'); + }); }); diff --git a/packages/twenty-front-component-renderer/src/remote/generated/remote-elements.ts b/packages/twenty-front-component-renderer/src/remote/generated/remote-elements.ts index 51f0f92fe4..c42855ef23 100644 --- a/packages/twenty-front-component-renderer/src/remote/generated/remote-elements.ts +++ b/packages/twenty-front-component-renderer/src/remote/generated/remote-elements.ts @@ -20,6 +20,7 @@ export type HtmlCommonProperties = { 'aria-label'?: string; 'aria-hidden'?: boolean; 'data-testid'?: string; + draggable?: string; }; export type HtmlCommonEvents = { click(event: RemoteEvent): void; @@ -121,6 +122,7 @@ const HTML_COMMON_PROPERTIES_CONFIG = { 'aria-label': { type: String }, 'aria-hidden': { type: Boolean }, 'data-testid': { type: String }, + draggable: { type: String }, }; export const HtmlDivElement = createRemoteElement< HtmlCommonProperties, diff --git a/packages/twenty-front-component-renderer/src/remote/utils/__tests__/isAriaOrDataAttribute.test.ts b/packages/twenty-front-component-renderer/src/remote/utils/__tests__/isAriaOrDataAttribute.test.ts new file mode 100644 index 0000000000..d1107da01e --- /dev/null +++ b/packages/twenty-front-component-renderer/src/remote/utils/__tests__/isAriaOrDataAttribute.test.ts @@ -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); + }); +}); diff --git a/packages/twenty-front-component-renderer/src/remote/utils/isAriaOrDataAttribute.ts b/packages/twenty-front-component-renderer/src/remote/utils/isAriaOrDataAttribute.ts new file mode 100644 index 0000000000..aabf4f6abf --- /dev/null +++ b/packages/twenty-front-component-renderer/src/remote/utils/isAriaOrDataAttribute.ts @@ -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), + ); +}; diff --git a/packages/twenty-front-component-renderer/src/remote/utils/patchRemoteElementAttributes.ts b/packages/twenty-front-component-renderer/src/remote/utils/patchRemoteElementAttributes.ts new file mode 100644 index 0000000000..49adae0240 --- /dev/null +++ b/packages/twenty-front-component-renderer/src/remote/utils/patchRemoteElementAttributes.ts @@ -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 = { + className: 'className', + class: 'className', + + htmlFor: 'htmlFor', + for: 'htmlFor', + + tabIndex: 'tabIndex', + tabindex: 'tabIndex', + + srcDoc: 'srcDoc', + srcdoc: 'srcDoc', +}; + +type RemoteElementWithAttributeUpdater = Element & + Record & { + 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( + ( + 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); + } + }; + } +}; diff --git a/packages/twenty-front-component-renderer/src/remote/utils/patchRemoteElementSetAttribute.ts b/packages/twenty-front-component-renderer/src/remote/utils/patchRemoteElementSetAttribute.ts deleted file mode 100644 index 43c9c57601..0000000000 --- a/packages/twenty-front-component-renderer/src/remote/utils/patchRemoteElementSetAttribute.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { ALLOWED_HTML_ELEMENTS } from '@/constants/AllowedHtmlElements'; - -const ATTRIBUTE_TO_PROPERTY_MAP: Record = { - 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, - name: string, - value: string, - ) { - const propertyName = ATTRIBUTE_TO_PROPERTY_MAP[name]; - - if (propertyName) { - this[propertyName] = value; - - return; - } - - originalSetAttribute.call(this, name, value); - }; - } -}; diff --git a/packages/twenty-front-component-renderer/src/remote/worker/remote-worker.ts b/packages/twenty-front-component-renderer/src/remote/worker/remote-worker.ts index 00d4cc4759..2e02fc71ce 100644 --- a/packages/twenty-front-component-renderer/src/remote/worker/remote-worker.ts +++ b/packages/twenty-front-component-renderer/src/remote/worker/remote-worker.ts @@ -14,7 +14,7 @@ import { isDefined } from 'twenty-shared/utils'; import { installStyleBridge } from '@/polyfills/installStyleBridge'; import { installStylePropertyOnRemoteElements } from '@/remote/utils/installStylePropertyOnRemoteElements'; -import { patchRemoteElementSetAttribute } from '@/remote/utils/patchRemoteElementSetAttribute'; +import { patchRemoteElementAttributes } from '@/remote/utils/patchRemoteElementAttributes'; import { installErrorEventBridge } from './utils/installErrorEventBridge'; import { type FrontComponentExecutionContext } from 'twenty-sdk/front-component'; import { frontComponentHostCommunicationApi } from '@/constants/frontComponentHostCommunicationApi'; @@ -31,7 +31,7 @@ import { import { setWorkerEnv } from './utils/setWorkerEnv'; installStylePropertyOnRemoteElements(); -patchRemoteElementSetAttribute(); +patchRemoteElementAttributes(); installErrorEventBridge(); exposeGlobals({