Commit 9f79bfb
authored
fix(dashboard): collapsed email editor height; sandbox email-preview iframes (#1406)
## 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 used `h-full`. The dashboard shell's
`<main>` (`sidebar-layout.tsx:750`) has no explicit height — its flex
parent uses `items-start`, so `<main>` shrinks to its content rather
than stretching. With no definite height up the chain, every `h-full`
along the way (sidebar-layout's inner div, the `data-full-bleed`
wrapper, `VibeCodeLayout`'s own root) resolves to `auto`, and since the
editor's content lives inside absolutely-positioned `ResizablePanel`s,
the wrapper collapses to ~0.
**Fix:** anchor `VibeCodeLayout`'s root wrappers to
viewport-minus-header instead of `h-full`. The values match what
`sidebar-layout.tsx:738` already uses for the sticky sidebar (`3.5rem`
light / `6rem` dark for the floating header card). With a definite
height at the top, the existing `flex-1` chains inside `VibeCodeLayout`
resolve correctly without any layout/architecture refactor in the
surrounding dashboard shell.
```diff
-<div className="flex flex-col h-full w-full overflow-hidden md:hidden">
+<div className="flex flex-col h-[calc(100dvh-3.5rem)] w-full overflow-hidden md:hidden">
-<div className="hidden md:flex flex-col h-full w-full overflow-hidden">
+<div className="hidden md:flex flex-col h-[calc(100vh-3.5rem)] dark:h-[calc(100vh-6rem)] w-full overflow-hidden">
```
Trade-off: the editor knows the dashboard header is `3.5rem` (`6rem`
dark). The same numbers are already hardcoded in `sidebar-layout.tsx`,
so this isn't a new coupling.
### 2. Sandbox the email-preview iframes
`EmailPreviewContent` and `EmailPreviewEditableContent` rendered
user-authored template HTML in iframes with no `sandbox` at all. With
`srcDoc`-rendered iframes treated as same-origin by default, that meant
any `<script>` (or `onerror=`, `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:
- Iframe is forced into a unique opaque origin → no access to parent
cookies, `localStorage`, `sessionStorage`, or DOM.
- No `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 default `SameSite=Lax`; cross-origin responses also
unreadable due to CORS).
- No `allow-top-navigation` / `allow-forms` / `allow-popups` → template
can't redirect the parent tab, submit forms, or open windows.
- `allow-scripts` is required so the inline scripts we inject
(link-click prevention; the WYSIWYG editor that drives the `postMessage`
flow at `email-preview.tsx:413-435` and `:625-672`) can actually run.
Without it, the editor itself was broken and links navigated freely.
Note: `allow-scripts allow-same-origin` together would be equivalent to
no sandbox at all (the iframe could rewrite its own `sandbox` attribute
and escape), so we deliberately omit `allow-same-origin`.
**Residual risk (not addressed in this PR):** a malicious template
script can still `postMessage` a fake `stack_edit_commit` to the parent
— the parent's `e.source === iframeWindow` check 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
- [ ] Open an email template editor — verify the preview, code panel,
and chat panel are all visible at full height (light + dark mode).
- [ ] Same for an email theme editor and an email draft editor in the
`draft` stage.
- [ ] Resize the window vertically — editor should fill the viewport
below the header without overflowing past the bottom.
- [ ] Click a link inside the rendered preview — should not navigate
(link-click prevention script works under `allow-scripts`).
- [ ] In edit mode, hover an editable text region, click to edit, type a
change, hit ✓ — change should round-trip through `postMessage` and
update the source.
- [ ] Sanity check: paste `<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).1 parent 0ab2654 commit 9f79bfb
2 files changed
Lines changed: 9 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
| 109 | + | |
109 | 110 | | |
110 | 111 | | |
111 | 112 | | |
| |||
692 | 693 | | |
693 | 694 | | |
694 | 695 | | |
| 696 | + | |
695 | 697 | | |
696 | 698 | | |
697 | 699 | | |
| |||
Lines changed: 7 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
132 | 132 | | |
133 | 133 | | |
134 | 134 | | |
135 | | - | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
136 | 139 | | |
137 | 140 | | |
138 | 141 | | |
| |||
354 | 357 | | |
355 | 358 | | |
356 | 359 | | |
357 | | - | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
358 | 363 | | |
359 | 364 | | |
360 | 365 | | |
| |||
0 commit comments