Conversation
…l-preview iframes The email template/theme/draft editors render inside VibeCodeLayout, whose mobile and desktop wrappers used `h-full`. The dashboard shell's `<main>` has no explicit height (its flex parent uses `items-start`, so it doesn't stretch), so percentage heights collapsed and the editor rendered with zero height. Anchor the wrappers to viewport-minus-header instead, matching the values already used by `sidebar-layout.tsx` (3.5rem light, 6rem dark for the floating header). This decouples the editor from any ancestor height chain. Also lock down the email preview iframes with `sandbox=""` so untrusted template HTML can't run scripts, navigate the parent, submit forms, etc.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdded ChangesEmail Preview Iframe Sandboxing
Vibe Coding Layout Heights
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/src/components/email-preview.tsx (1)
694-702:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
sandbox=""breaks the WYSIWYG editor functionality.The empty
sandboxattribute blocks all JavaScript execution, butEmailPreviewEditableContentinjects theWYSIWYG_EDITOR_SCRIPT(lines 125-573) which must run for edit mode to work. This script handles editable region parsing, overlays, hover/click events,contentEditable, andpostMessagecommunication with the parent.With
sandbox="", none of this executes—edit mode is completely non-functional.Use
sandbox="allow-scripts"to permit the injected editor script while still blocking navigation, forms, popups, and same-origin access. ThepostMessagecalls using'*'as targetOrigin will continue to work cross-origin.Proposed fix
return ( <iframe - sandbox="" + sandbox="allow-scripts" ref={iframeRef} srcDoc={editableHtml} className="w-full h-full border-0" title="Email Preview (Edit Mode)" /> );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/email-preview.tsx` around lines 694 - 702, The iframe's empty sandbox attribute prevents the injected editor script from running, breaking edit mode; update the iframe in EmailPreviewEditableContent to use sandbox="allow-scripts" (instead of sandbox="") so the WYSIWYG_EDITOR_SCRIPT can execute while still restricting navigation/forms/popups/same-origin access; ensure the change is applied where the iframe with ref={iframeRef} and srcDoc={editableHtml} is returned so postMessage-based communication and contentEditable behaviors work.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/dashboard/src/components/email-preview.tsx`:
- Around line 694-702: The iframe's empty sandbox attribute prevents the
injected editor script from running, breaking edit mode; update the iframe in
EmailPreviewEditableContent to use sandbox="allow-scripts" (instead of
sandbox="") so the WYSIWYG_EDITOR_SCRIPT can execute while still restricting
navigation/forms/popups/same-origin access; ensure the change is applied where
the iframe with ref={iframeRef} and srcDoc={editableHtml} is returned so
postMessage-based communication and contentEditable behaviors work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c829e2f-2a2e-4d1e-940c-82c04d18f638
📒 Files selected for processing (2)
apps/dashboard/src/components/email-preview.tsxapps/dashboard/src/components/vibe-coding/vibe-code-layout.tsx
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Empty sandbox blocked the inline scripts that prevent link clicks and drive the WYSIWYG editor's postMessage flow, breaking the editor. `allow-scripts` (without `allow-same-origin`) lets those scripts run while keeping the iframe in an opaque origin: malicious template HTML still can't read parent cookies/localStorage/DOM, can't make credentialed requests to the dashboard API, and can't navigate or submit forms in the parent. Token exfiltration via XSS in template HTML remains blocked. Residual risk: a script in the iframe can post a fake stack_edit_commit to the parent (the e.source check passes since the script runs in the iframe). That's a cross-admin UI-redress concern, not token theft, and should be addressed separately (e.g. CSP nonce on the injected script).
2 participants
Summary
Two small dashboard fixes bundled together.
1. Email editor renders with zero height
The email template/theme/draft pages render
VibeCodeLayout, whose mobile and desktop root wrappers usedh-full. The dashboard shell's<main>(sidebar-layout.tsx:750) has no explicit height — its flex parent usesitems-start, so<main>shrinks to its content rather than stretching. With no definite height up the chain, everyh-fullalong the way (sidebar-layout's inner div, thedata-full-bleedwrapper,VibeCodeLayout's own root) resolves toauto, and since the editor's content lives inside absolutely-positionedResizablePanels, the wrapper collapses to ~0.Fix: anchor
VibeCodeLayout's root wrappers to viewport-minus-header instead ofh-full. The values match whatsidebar-layout.tsx:738already uses for the sticky sidebar (3.5remlight /6remdark for the floating header card). With a definite height at the top, the existingflex-1chains insideVibeCodeLayoutresolve correctly without any layout/architecture refactor in the surrounding dashboard shell.Trade-off: the editor knows the dashboard header is
3.5rem(6remdark). The same numbers are already hardcoded insidebar-layout.tsx, so this isn't a new coupling.2. Sandbox the email-preview iframes
EmailPreviewContentandEmailPreviewEditableContentrendered user-authored template HTML in iframes with nosandboxat all. WithsrcDoc-rendered iframes treated as same-origin by default, that meant any<script>(oronerror=,javascript:URL, etc.) inside a template could read the dashboard's cookies/localStorage and call the API as the viewing admin.Set
sandbox="allow-scripts"on both iframes:localStorage,sessionStorage, or DOM.allow-same-origin, so credentialed fetches to the dashboard API don't carry the user's session (cookies aren't sent to a third-party opaque origin under defaultSameSite=Lax; cross-origin responses also unreadable due to CORS).allow-top-navigation/allow-forms/allow-popups→ template can't redirect the parent tab, submit forms, or open windows.allow-scriptsis required so the inline scripts we inject (link-click prevention; the WYSIWYG editor that drives thepostMessageflow atemail-preview.tsx:413-435and:625-672) can actually run. Without it, the editor itself was broken and links navigated freely.Note:
allow-scripts allow-same-origintogether would be equivalent to no sandbox at all (the iframe could rewrite its ownsandboxattribute and escape), so we deliberately omitallow-same-origin.Residual risk (not addressed in this PR): a malicious template script can still
postMessagea fakestack_edit_committo the parent — the parent'se.source === iframeWindowcheck passes because the script is running in that iframe. The viewing admin would silently apply attacker-chosen source-code edits on save. That's a cross-admin UI-redress concern, not token exfiltration, and is best fixed with a CSP nonce on the injected script (so user template<script>tags can't run at all). Tracking as a follow-up.Test plan
draftstage.allow-scripts).postMessageand update the source.<script>document.title='pwned'</script>(or<img onerror=...>) into a template, render preview — parent tab title/cookies/etc. should be untouched (script runs in opaque origin, can't reach parent).