Skip to content

Add allow same origin to the iFrame widget#16239

Merged
charlesBochet merged 1 commit intomainfrom
r--add-allow-same-origin-to-the-iframe-widget
Dec 2, 2025
Merged

Add allow same origin to the iFrame widget#16239
charlesBochet merged 1 commit intomainfrom
r--add-allow-same-origin-to-the-iframe-widget

Conversation

@bosiraphael
Copy link
Copy Markdown
Contributor

Some iFrames were unusable because of this

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Dec 2, 2025

Greptile Overview

Greptile Summary

Added allow-same-origin to the iframe sandbox attribute to enable same-origin iframe functionality.

  • Enabled same-origin access for iframes by adding allow-same-origin to the sandbox attribute
  • This change fixes unusable iframes that require same-origin access but introduces security considerations
  • The backend validates URLs using @IsUrl() decorator, but combining allow-same-origin with allow-scripts significantly reduces sandbox protections

Confidence Score: 3/5

  • This PR introduces a security consideration that needs verification before merging
  • The change is minimal and solves a real problem, but allow-same-origin combined with allow-scripts creates a security concern. While the backend validates URLs, the effectiveness depends on whether iframe URLs can only come from trusted admin-controlled sources or if they can be influenced by users. The code itself is clean and functional.
  • Verify that IframeWidget.tsx iframe URLs are strictly controlled and cannot be manipulated by untrusted users

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-front/src/modules/page-layout/widgets/iframe/components/IframeWidget.tsx 3/5 Added allow-same-origin to iframe sandbox attribute to enable same-origin iframes, which has security implications

Sequence Diagram

sequenceDiagram
    participant User
    participant IframeWidget
    participant Backend
    participant ExternalSite

    User->>Backend: Create/Update Widget with URL
    Backend->>Backend: Validate URL (@IsUrl decorator)
    Backend-->>User: Widget saved with validated URL
    
    User->>IframeWidget: Render widget on page
    IframeWidget->>IframeWidget: Check isPageLayoutInEditMode
    IframeWidget->>IframeWidget: Validate URL exists
    IframeWidget->>ExternalSite: Load iframe with sandbox="allow-scripts allow-forms allow-popups allow-same-origin"
    Note over IframeWidget,ExternalSite: NEW: allow-same-origin enables<br/>same-origin access (cookies, localStorage, DOM)
    ExternalSite-->>IframeWidget: Render content with same-origin privileges
    IframeWidget->>IframeWidget: Handle onLoad/onError events
    IframeWidget-->>User: Display iframe or error state
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

onLoad={handleIframeLoad}
onError={handleIframeError}
sandbox="allow-scripts allow-forms allow-popups"
sandbox="allow-scripts allow-forms allow-popups allow-same-origin"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: allow-same-origin combined with allow-scripts removes most sandbox protections. The iframe can now access its own origin (cookies, localStorage, DOM), potentially enabling XSS if the URL source is compromised. Verify that iframe URLs come from trusted sources only and cannot be user-controlled.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/page-layout/widgets/iframe/components/IframeWidget.tsx
Line: 106:106

Comment:
**logic:** `allow-same-origin` combined with `allow-scripts` removes most sandbox protections. The iframe can now access its own origin (cookies, localStorage, DOM), potentially enabling XSS if the URL source is compromised. Verify that iframe URLs come from trusted sources only and cannot be user-controlled.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@prastoin prastoin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there, that's pretty risky
I would either opt to pass credentials to the invoked iframe rather than consuming its own cookies 🤔
I honestly don't know what's the most suitable here

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 2, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:18183

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Copy Markdown
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is completely safe for two reasons:

  • users are responsible to what they install / setup in their workspace. If they want to hack themselves, they can. (instead of hacking themselves through an iframe, let's just make a chrome extension :p)
  • none of the browsers allow any scripting / sharing between a host and an iframe having different hosts (scheme + subdomain + domain + protocol have to be exactly equal). So the only way to hack my-twenty.twenty.com is to host an iframe on... my-twenty.twenty.com), which lead to "hack myself" scenario too

@charlesBochet charlesBochet merged commit 269135e into main Dec 2, 2025
69 checks passed
@charlesBochet charlesBochet deleted the r--add-allow-same-origin-to-the-iframe-widget branch December 2, 2025 10:36
@twenty-eng-sync
Copy link
Copy Markdown

Hey @bosiraphael! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

NotYen pushed a commit to NotYen/twenty-ym that referenced this pull request Dec 4, 2025
Some iFrames were unusable because of this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants