Enable previewing emails (.eml files) in limel-file-viewer#3783
Enable previewing emails (.eml files) in limel-file-viewer#3783
.eml files) in limel-file-viewer#3783Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughAdds .eml email viewing: new postal-mime dependency, an email-viewer component with styles and examples, an email-loader that parses MIME emails and produces a view model, file-viewer integration for 'email' type with lifecycle cleanup, utility types and helpers, and translations for eight languages. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FileViewer as limel-file-viewer
participant Loader as email-loader
participant PostalMime as postal-mime
participant EmailViewer as limel-email-viewer
User->>FileViewer: Request .eml URL
FileViewer->>FileViewer: Detect extension -> 'email'
FileViewer->>Loader: loadEmail(url)
Loader->>PostalMime: fetch(url) & parse(eml, options)
PostalMime-->>Loader: Parsed MIME structure
Loader->>Loader: Extract headers, body, attachments
Loader->>Loader: Convert inline attachments -> data: URLs
Loader->>Loader: Replace cid: refs in HTML
Loader-->>FileViewer: Email object
FileViewer->>EmailViewer: Render(email)
EmailViewer->>User: Display headers, body, attachments
sequenceDiagram
participant Loader as email-loader
participant PostalMime as postal-mime
participant Processor as AttachmentProcessor
Loader->>PostalMime: parse(eml)
PostalMime-->>Loader: attachments[]
loop per attachment
alt inline/related
Processor->>Processor: ArrayBuffer -> Base64
Processor->>Processor: Create data: URL
Processor-->>Loader: Map by Content-ID
Loader->>Loader: Replace cid: refs in HTML
else attachment
Processor->>Loader: Extract filename, mimeType, size
Loader->>Loader: Add to attachments list
end
end
Loader-->>FileViewer: Email with processed attachments
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3783/ |
Kiarokh
force-pushed
the
file-viewer-email
branch
2 times, most recently
from
3fab733 to
af7b58e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@src/components/email-viewer/email-viewer.tsx`:
- Around line 202-221: The attachments block uses a <label> as a non-form
heading and redundant explicit list roles; change the <label> (around variable
"label") to a semantic non-form element (e.g., <span> or an appropriate heading
tag) so it isn't treated as a form label, and remove role="list" on the <ul> and
role="listitem" on each <li>; update the JSX where this.attachments is mapped
(see render using this.getTranslation('file-viewer.email.attachment.unnamed')
and this.renderSizeBadge(attachment.size)) to reflect these markup changes while
preserving visual styling and accessibility semantics.
- Around line 128-133: The renderBody method currently inserts untrusted HTML
via innerHTML (this.bodyHtml) in the EmailViewer component; sanitize the HTML
before rendering by integrating a vetted sanitizer (e.g., DOMPurify) or
switching to a sandboxed iframe/CSP approach. Update the renderBody
implementation to run this.bodyHtml through the sanitizer (or load it into a
sandboxed iframe) and then assign the sanitized output to innerHTML; ensure
configuration strips scripts, event handlers, and javascript: URLs and reference
the renderBody method, the bodyHtml property, and the innerHTML assignment when
making the change.
In `@src/components/file-viewer/email-loader.ts`:
- Around line 156-182: The function formatAddress declares a return type of
string but currently returns undefined in some branches (e.g., when address is
falsy or fallback returns undefined), so update the signature to return string |
undefined or ensure every branch returns a string (e.g., return empty string
instead of undefined); locate formatAddress and related formatAddresses to apply
the same fix consistently and adjust callers if they rely on a strict string
type.
- Around line 145-154: The function formatAddresses currently declares a return
type of string but returns undefined in some branches; update its signature to
return string | undefined and ensure callers handle the possibly undefined
result (or alternatively return an empty string consistently). Specifically
change the formatAddresses function declaration and any direct callers that
assume a plain string (e.g., places invoking formatAddresses(...) for display or
concatenation) to accept and handle undefined values.
- Around line 94-100: The normalizeContentId function uses
String.prototype.replaceAll with an anchored regex /( ^<|>$)/g which only needs
a single replace; switch to String.prototype.replace (keeping the same regex and
flags) to be idiomatic and avoid misleading use of replaceAll; update the return
path in normalizeContentId to call replace(...).trim() and run tests/linters to
confirm no behavioral change.
- Around line 21-28: The loadEmail function currently calls fetch(...) and
PostalMime.parse(...) without try/catch, so network errors or parse exceptions
will propagate; wrap the fetch + buffer + PostalMime.parse sequence in a
try/catch inside loadEmail, catch and rethrow or return a controlled error with
a clear message (e.g., include the url and the original error), and handle
non-OK HTTP responses by checking response.ok and throwing a descriptive error
before parsing; reference the loadEmail function, the fetch call, response.ok
check, and PostalMime.parse to locate where to add the error handling and
logging.
In `@src/components/file-viewer/examples/file-viewer-eml.tsx`:
- Around line 1-28: The render method in FileViewerEmlExample currently returns
a JSX array of siblings which causes key/lint warnings; update the component to
import Host from '@stencil/core' and change the render() to return a single
<Host> element that wraps all child nodes (replace the array literal returned in
render with a single Host wrapper containing the h4 and limel-file-viewer
elements), removing the surrounding array brackets so the component returns one
root element.
In `@src/components/file-viewer/file-viewer.tsx`:
- Around line 452-463: The file-viewer currently calls loadEmail(this.url)
without error handling, so if loadEmail rejects the component can keep stale
this.email/this.fileUrl and never set this.loading=false; wrap the email branch
in a try/catch/finally: call revokeCreatedObjectUrls() first (as existing), then
in the 'email' branch await loadEmail(this.url) inside try, set this.email and
this.fileUrl on success, on catch reset this.email and this.fileUrl to
null/empty and optionally log the error, and in finally ensure this.loading =
false so the UI is not stuck; reference the email branch where loadEmail is
invoked and the properties this.email, this.fileUrl, this.createdObjectUrls, and
this.loading.
- Around line 260-278: The renderEmail method returns an array of sibling JSX
nodes; replace that by wrapping the returned elements in a <Host> container and
import Host from '@stencil/core'. Update the renderEmail function (the method
named renderEmail) to return a single JSX tree with <Host> containing
this.renderButtons() and the <limel-email-viewer> element (keeping existing
props like subject, from, to, cc, date, bodyHtml, bodyText, attachments,
fallbackUrl using sanitizeUrl(this.fileUrl), and language), and ensure Host is
added to the component imports.
In `@src/translations/sv.ts`:
- Around line 46-52: The file's translation keys for file-viewer email
('file-viewer.email.subject', 'file-viewer.email.from', 'file-viewer.email.to',
'file-viewer.email.cc', 'file-viewer.email.date',
'file-viewer.email.attachments', 'file-viewer.email.attachment.unnamed') are
misplaced between editor-menu entries; move that entire block so it sits after
the existing 'file-viewer.more-actions' entry and before 'editor-menu.bold' to
match the ordering used in other locale files (e.g., de.ts), preserving the
exact keys and values and leaving surrounding entries intact.
| private renderBody() { | ||
| if (this.bodyHtml) { | ||
| return ( | ||
| <div class="body" innerHTML={this.bodyHtml} part="email-body" /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the email-viewer.tsx file, particularly around lines 128-133
# and understand where bodyHtml comes from
cat -n src/components/email-viewer/email-viewer.tsx | head -160 | tail -80Repository: Lundalogik/lime-elements
Length of output: 2879
🏁 Script executed:
# Search for any sanitization utilities in the codebase
fd -t f \( -name "*.ts" -o -name "*.tsx" \) | xargs rg -l "sanitiz|DOMPurify|xss" 2>/dev/null | head -20Repository: Lundalogik/lime-elements
Length of output: 713
🏁 Script executed:
# Search for how bodyHtml is set/populated in the component
rg "bodyHtml\s*=" src/components/email-viewer/ -A 3 -B 3Repository: Lundalogik/lime-elements
Length of output: 1525
🏁 Script executed:
# Check if there's any documentation or comments about XSS handling
rg -i "xss|security|sanitiz|untrusted" src/components/email-viewer/Repository: Lundalogik/lime-elements
Length of output: 50
🌐 Web query:
best practices for safely rendering email HTML content in web components to prevent XSS attacks
💡 Result:
Short checklist of best practices for safely rendering email HTML in web components (to prevent XSS):
-
Never inject raw HTML strings into DOM sinks (innerHTML, shadowRoot.innerHTML, setHTMLUnsafe) without sanitizing or Trusted Types. Use TrustedHTML/Trusted Types + CSP where possible. [1][2]
-
Sanitize untrusted HTML with a vetted allowlist sanitizer (e.g., DOMPurify) configured to: remove script tags, event-* attributes, dangerous protocols (javascript:, data:, vbscript:), and any tags/attributes you don’t need. Keep the sanitizer up to date. [3][4]
-
Use defense-in-depth:
- Enforce a strict Content-Security-Policy (no unsafe-inline, restrict script-src, use sandbox/CSP sandbox for embedded documents). [2][6]
- Enforce trusted-types via CSP (require-trusted-types-for) so only TrustedHTML can reach injection sinks. [2][5]
-
Prefer isolation for full-document email HTML:
- Render email HTML in a sandboxed iframe served from a different origin (or use srcdoc + sandbox with minimal allowances). Don’t combine allow-scripts with allow-same-origin. Proxy/strip remote resources to avoid leaks. [6][7]
-
Remove or rewrite risky elements:
- Strip <script>, <style> if not needed, form tags, object/embed/iframe (or sandbox them).
- Remove inline event handlers (onclick/onerror) and CSS expressions; rewrite links to validate/normalize schemes and add rel="noopener noreferrer". [3][4]
-
Treat images and remote resources as privacy risk:
- Block or proxy remote images/resources to avoid IP/metadata leaks; require user action to load external resources. (Common email-client practice; implement on the server or via a proxy.)
-
Avoid DOM APIs that evaluate code or create new execution contexts from strings (eval, Function, setTimeout(string), element.setAttribute('on…')).
-
Don’t modify sanitized HTML afterwards — modifications can reintroduce risks. Test sanitization with known XSS vectors and fuzzing; monitor CVEs for your sanitizer. [3][4]
References:
- MDN: innerHTML / ShadowRoot, setHTMLUnsafe and security considerations. [1][7]
- MDN: Trusted Types API and CSP require-trusted-types-for. [2][5]
- DOMPurify docs / guidance (use allowlist, URI validation, keep updated). [3][4]
- MDN: iframe sandbox and CSP sandbox guidance for isolating untrusted content. [6][7]
If you want, I can produce a small example Web Component that implements Trusted Types + DOMPurify (or a sandboxed-iframe pattern) tailored to your stack — specify framework or plain Web Components.
🏁 Script executed:
# Find the `@Prop` definition for bodyHtml
rg "bodyHtml" src/components/email-viewer/email-viewer.tsx -B 3 -A 1 | head -30Repository: Lundalogik/lime-elements
Length of output: 442
🏁 Script executed:
# Check if there's a CSP or security policy in index.html or global config
cat src/index.html 2>/dev/null || echo "No index.html found"Repository: Lundalogik/lime-elements
Length of output: 1633
🏁 Script executed:
# Check the global config for security settings
cat src/global/config.ts | head -50Repository: Lundalogik/lime-elements
Length of output: 1340
🏁 Script executed:
# Look for how email-viewer is being used across the codebase
rg "email-viewer" --type tsx src/ -l | head -5Repository: Lundalogik/lime-elements
Length of output: 95
🏁 Script executed:
# Check if there's any documentation about the component's security model
cat src/components/email-viewer/readme.md 2>/dev/null || cat src/components/email-viewer/README.md 2>/dev/null || echo "No readme found"Repository: Lundalogik/lime-elements
Length of output: 83
Sanitize bodyHtml before rendering with innerHTML to prevent XSS.
The bodyHtml prop is rendered directly via innerHTML with no sanitization. Email HTML can contain malicious scripts (onclick handlers, script tags, javascript: URLs, etc.). Use a vetted sanitizer like DOMPurify configured to strip dangerous elements and attributes, or consider rendering email content in a sandboxed iframe. At minimum, add a Content-Security-Policy to prevent inline script execution.
🤖 Prompt for AI Agents
In `@src/components/email-viewer/email-viewer.tsx` around lines 128 - 133, The
renderBody method currently inserts untrusted HTML via innerHTML (this.bodyHtml)
in the EmailViewer component; sanitize the HTML before rendering by integrating
a vetted sanitizer (e.g., DOMPurify) or switching to a sandboxed iframe/CSP
approach. Update the renderBody implementation to run this.bodyHtml through the
sanitizer (or load it into a sandboxed iframe) and then assign the sanitized
output to innerHTML; ensure configuration strips scripts, event handlers, and
javascript: URLs and reference the renderBody method, the bodyHtml property, and
the innerHTML assignment when making the change.
✅ Confirmed as addressed by @Kiarokh
There was a problem hiding this comment.
will fix this in a follow up PR
There was a problem hiding this comment.
@coderabbitai can you review the last two commits? They handle sanitization
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the last two commits on this PR branch
git log --oneline -2
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Get detailed diff of the last two commits to see sanitization changes
git log -2 --patch --no-merges
Length of output: 9368
🏁 Script executed:
#!/bin/bash
# Search for sanitization-related code in email-viewer files
rg -n "sanitiz|DOMPurify|purify" src/components/email-viewer/ -A 3 -B 3
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if DOMPurify was added to dependencies
rg "dompurify|isomorphic-dompurify" package.json -A 1 -B 1
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the current state of renderBody method
cat src/components/email-viewer/email-viewer.tsx | sed -n '126,140p'
Length of output: 388
🏁 Script executed:
#!/bin/bash
# Check if unified/rehype dependencies were added to package.json
rg "unified|rehype" package.json -A 1 -B 1
Length of output: 389
🏁 Script executed:
#!/bin/bash
# Verify the full sanitize-email-html.ts file
cat src/util/sanitize-email-html.ts
Length of output: 7369
🏁 Script executed:
#!/bin/bash
# Check how bodyHtml flows from email-loader to email-viewer
rg "bodyHtml" src/components/file-viewer/email-loader.ts src/components/file-viewer/file-viewer.tsx -n -A 2 -B 2
Length of output: 850
@Kiarokh Excellent work! 🎉 The sanitization implementation is comprehensive and properly addresses the XSS concerns.
Key security features I observed:
-
Robust AST-based sanitization - Using
rehype-sanitizewith a custom schema is far more reliable than regex-based approaches. The AST traversal ensures you can't bypass the sanitizer with clever encoding or nested structures. -
Comprehensive threat blocking:
- Scripts, event handlers, and
javascript:URLs are blocked - Dangerous CSS properties (
behavior,expression,-moz-binding, etc.) are stripped - Only safe
data:image MIME types are allowed
- Scripts, event handlers, and
-
Privacy protection - Blocking remote images prevents tracking pixels and metadata leakage. Only embedded
data:URLs are permitted for images, which is the right call for email rendering. -
Visual fidelity preserved - The custom schema allows email-specific HTML elements (
table,font,center,styleattributes) while maintaining security. This balance is crucial for proper email display. -
Clean architecture - Sanitization happens in
email-loader.tsbefore the HTML reaches the viewer component, and the dynamic import keeps bundle size down.
The sanitization flow is correct: HTML is sanitized after CID resolution but before rendering, so email-viewer.tsx only receives safe, pre-sanitized content.
There was a problem hiding this comment.
@coderabbitai can you please review ff12437. It is an addition that enables loading remote images
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Show the details of the specific commit that enables loading remote images
git show ff12437b666254f5ee10747ce589b585af76c8b6
Length of output: 20038
@Kiarokh Excellent implementation! 🎉 This is a well-designed, privacy-first approach to handling remote images in email previews. Here's my detailed review:
✅ Security & Privacy Strengths
1. Privacy-first default behavior
- Remote images are blocked by default to prevent tracking pixels, IP disclosure, and metadata leakage
- Users must explicitly opt-in after being informed of the risks
- This matches industry best practices from Gmail, Outlook, and other major email clients
2. User education
- The collapsible warning banner clearly explains the privacy implications in user-friendly language
- Covers tracking risks, IP disclosure, and device/browser fingerprinting
- Multi-language support ensures all users understand the risks
3. Secure implementation
- Remote URLs are stored in
data-remote-srcduring sanitization, keeping them inert applyRemoteImagesPolicyusesDOMParsersafely (no script execution context)- URL validation via
isAllowedRemoteImageUrlensures only http/https protocols are restored - The sanitizer still processes all HTML - it just defers image loading rather than stripping URLs
4. Flexible architecture
- Per-email state: When
allowRemoteImagesprop is omitted, choice is per-email (resets onbodyHtmlchange) - External control: Consumers can provide the prop to implement session-wide or global preferences
- Event emission:
allowRemoteImagesChangeevent allows parents to persist user preferences
🎯 Design Highlights
// Smart state management
`@Watch`('bodyHtml')
public onBodyHtmlChange() {
if (this.allowRemoteImages === undefined) {
this.allowRemoteImagesState = false; // Reset for new email
}
}The watcher ensures that when viewing a different email, the user must re-confirm loading images - preventing a single "yes" from exposing all future emails.
// Safe URL restoration with validation
const remoteSrc = image.dataset.remoteSrc;
if (!remoteSrc || !isAllowedRemoteImageUrl(remoteSrc)) {
delete image.dataset.remoteSrc;
continue;
}
image.setAttribute('src', remoteSrc);Double validation before restoration prevents any bypasses.
💡 Minor Suggestions
1. Consider adding a loading indicator
When the user clicks "Load images," there may be a brief delay while images are fetched. A subtle loading state could improve UX:
`@State`()
private loadingRemoteImages = false;
private enableRemoteImages = async () => {
this.loadingRemoteImages = true;
// ... existing logic
this.loadingRemoteImages = false;
};2. Sanitizer comment clarity
In sanitize-email-html.ts, the comment for data-remote-src is excellent. Consider also documenting in the schema why http/https are now allowed in the src protocol list (since they're handled specially):
// Allow http/https in schema so sanitizer preserves them in data-remote-src;
// actual loading is controlled by user opt-in via privacy banner
src: [...defaultSrcProtocols, 'http', 'https', 'data'],3. Optional: Track which emails loaded images
For future enhancement, you could expose a way for consumers to remember which sender domains the user trusts, similar to how email clients offer "Always load images from this sender."
📋 Summary
This commit implements remote image handling exceptionally well:
- ✅ Secure by design (no automatic loading)
- ✅ Privacy-respecting (clear warnings, informed consent)
- ✅ User-friendly (collapsible banner, per-email or global control)
- ✅ Well-architected (clean separation of sanitization and rendering)
- ✅ Properly validated (URL checks before restoration)
The implementation successfully balances security, privacy, and usability. Great work! 🚀
Kiarokh
force-pushed
the
file-viewer-email
branch
3 times, most recently
from
330a75b to
4d22b67
Compare
|
@coderabbitai I've implemented your suggested changes. Fresh full review please |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/email-viewer/email-viewer.tsx`:
- Around line 175-187: The getHeaderValues function incorrectly splits 'to' and
'cc' by plain commas causing failures when display names contain commas; update
the implementation to either (A) parse the header string respecting RFC-2822
formatting by splitting only on commas that are not inside quoted strings or
angle-bracketed address parts (i.e., implement a small stateful parser or use a
utility to split on commas outside quotes/<>), or (B) change the Email interface
and related formatAddresses/formatAddress functions so that to and cc are typed
and returned as string[] (update callers to pass arrays instead of comma-joined
strings); locate getHeaderValues, formatAddress, formatAddresses, and the Email
interface to update the types/parsing accordingly and ensure filtering/trimming
is preserved.
In `@src/components/email-viewer/examples/email-viewer-plain-text.tsx`:
- Line 21: Update the example RFC 5322 date string in the
email-viewer-plain-text example so the weekday matches the calendar date: locate
the date prop/value "Mon, 20 Jan 2026 09:12:34 +0100" in
src/components/email-viewer/examples/email-viewer-plain-text.tsx (used in the
EmailViewer/plain-text example) and change the weekday token from "Mon" to "Tue"
so it reads "Tue, 20 Jan 2026 09:12:34 +0100".
In `@src/util/format-bytes.ts`:
- Around line 43-44: Clamp and validate the decimals input before using toFixed
to prevent RangeError: ensure decimals is a finite integer in range 0..100 (e.g.
let safeDecimals = Number.isFinite(decimals) ? Math.max(0, Math.min(100,
Math.floor(decimals))) : 0), then use safeDecimals when computing precision
(const precision = value < 10 ? Math.max(0, safeDecimals) : 0) and when calling
toFixed to compute rounded in the formatBytes utility (replace usage of raw
decimals with safeDecimals).
♻️ Duplicate comments (1)
src/components/email-viewer/email-viewer.tsx (1)
128-133: Security: Unsanitized HTML rendering deferred to follow-up PR.The
innerHTMLusage forbodyHtmlposes an XSS risk since email content can contain malicious scripts. This was previously flagged and acknowledged for a follow-up PR.
src/components/email-viewer/examples/email-viewer-plain-text.tsx
Outdated
Show resolved
Hide resolved
Kiarokh
force-pushed
the
file-viewer-email
branch
7 times, most recently
from
a4e8c26 to
ec4527f
Compare
| ) { | ||
| const html = (email.html || '').trim(); | ||
| if (html) { | ||
| parsedEmail.bodyHtml = replaceCidReferences(html, cidUrlById); |
There was a problem hiding this comment.
Is it safe to let user upload their own files here? is this sanitized?
There was a problem hiding this comment.
no. we should fix this in a follow up PR. I didn't want to do this here as the PR was already too large to review.
plz see the issue for more info https://github.com/Lundalogik/crm-client/issues/633
There was a problem hiding this comment.
It feels pretty scary to not handle the security aspects before merging to main. Could we make a PR that adds the security parts and target it to this PR? Then we could review this PR but not merge it, and then review the security PR, merge that into this one, and then merge this one into main?
There was a problem hiding this comment.
I added it here. 👍
Kiarokh
force-pushed
the
file-viewer-email
branch
from
ec4527f to
0b65a10
Compare
Kiarokh
force-pushed
the
file-viewer-email
branch
from
cfbf550 to
ff12437
Compare
Kiarokh
force-pushed
the
file-viewer-email
branch
from
87fa3e3 to
fecd6c4
Compare
There was a problem hiding this comment.
examples are taken from here: https://github.com/mikel/mail/tree/master/spec/fixtures/emails
in case you find better ones for showcasing or testing
3 participants
fix https://github.com/Lundalogik/crm-client/issues/633
Summary by CodeRabbit
New Features
Examples
✏️ Tip: You can customize this high-level summary in your review settings.
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: