Merge pull request #1005 from dealerweb/fix/hide-body-referenced-cid-parts

fix: hide body-embedded cid parts declared as octet-stream from the attachment list

components/email/email-viewer.tsx

@@ -6,6 +6,7 @@ import { emailExportFilename, attachmentDownloadFilename, attachmentsBundleFilen
 import { EML_IMPORT_ACCEPT, expandImportableEmails } from "@/lib/eml-import";
 import { applyNewTabToAnchor, escapeHtml, plainTextToSafeHtml, sanitizeEmailBodyForIframe, sanitizeEmailHtml, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization";
 import { getRenderableHtmlBody } from "@/lib/email-body-selection";
+import { collectReferencedCids, isEmbeddedInBody } from "@/lib/attachment-visibility";
 import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse";
 import { fitEmailBodyWidth } from "@/lib/email-fit-width";
 import { withBasePath } from "@/lib/browser-navigation";
@@ -1583,8 +1584,14 @@ export function EmailViewer({
 
   const effectiveAttachments = useMemo<EffectiveAttachment[]>(() => {
     if (pluginRenderedAttachments.length > 0) {
+      const pluginCids = collectReferencedCids(hideInlineImageAttachments ? pluginRenderedHtml : null);
       return pluginRenderedAttachments
-        .filter(att => !(hideInlineImageAttachments && att.contentId && (att.mimeType || '').startsWith('image/')))
+        // Any cid image counts as embedded here (as before), plus whatever the
+        // decrypted body references by cid (see lib/attachment-visibility.ts).
+        .filter(att => !(hideInlineImageAttachments && (
+          (att.contentId && (att.mimeType || '').startsWith('image/'))
+          || isEmbeddedInBody({ cid: att.contentId, type: att.mimeType, disposition: att.disposition }, pluginCids)
+        )))
         .map((attachment, index) => ({
           id: `smime-${index}-${attachment.filename || attachment.mimeType}`,
           name: attachment.filename,
@@ -1596,6 +1603,12 @@ export function EmailViewer({
     }
 
     const hasCalInvitation = calendarInvitationParsingEnabled && !!email && !!findCalendarAttachment(email);
+    // Parts the rendered body embeds via cid: must not double as chips. The
+    // scan reads the same HTML the body renders from (null when the message
+    // renders as plain text: nothing embedded, nothing hidden), so a part only
+    // recognisable by its reference - octet-stream, no disposition, no name -
+    // is caught as well (see lib/attachment-visibility.ts).
+    const bodyCids = collectReferencedCids(hideInlineImageAttachments && email ? getRenderableHtmlBody(email) : null);
     const jmapAttachments = (email?.attachments ?? [])
       // Hide winmail.dat when we have successfully extracted TNEF content or attachments
       .filter(att => !(tnefHtml || tnefText || tnefAttachments.length > 0) || !isTnefAttachment(att.name, att.type))
@@ -1605,9 +1618,9 @@ export function EmailViewer({
       // Hide calendar MIME parts (text/calendar, application/ics) when the invitation
       // banner is shown - prevents raw ICS files appearing as spurious attachments.
       .filter(att => !hasCalInvitation || !isCalendarMimeType(att.type))
-      // Hide inline cid-referenced images when the user has opted to keep them
-      // out of the attachment list (default on): these are embedded in the body.
-      .filter(att => !(hideInlineImageAttachments && att.cid && att.disposition === 'inline' && (att.type || '').startsWith('image/')))
+      // Hide body-embedded parts when the user has opted to keep them out of
+      // the attachment list (default on).
+      .filter(att => !(hideInlineImageAttachments && isEmbeddedInBody(att, bodyCids)))
       // Hide machine-readable report parts (MDN read-receipts, DSN bounce
       // reports). These are required MIME parts, not real user attachments.
       .filter(att => att.type !== 'message/disposition-notification' && att.type !== 'message/delivery-status')
@@ -1642,11 +1655,11 @@ export function EmailViewer({
 
     return [...jmapAttachments, ...tnefExtracted, ...embeddedExtracted];
     // The memo derives only from `email.attachments` (findCalendarAttachment
-    // scans that array); depending on the whole `email` object would rebuild the
-    // attachment list — and its downstream layout measurement — on every email
-    // field change.
+    // scans that array) and the body parts the cid scan reads; depending on
+    // the whole `email` object would rebuild the attachment list — and its
+    // downstream layout measurement — on every email field change.
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [email?.attachments, pluginRenderedAttachments, tnefHtml, tnefText, tnefAttachments, embeddedEmailAttachments, calendarInvitationParsingEnabled, hideInlineImageAttachments]);
+  }, [email?.attachments, email?.htmlBody, email?.textBody, email?.bodyValues, pluginRenderedAttachments, pluginRenderedHtml, tnefHtml, tnefText, tnefAttachments, embeddedEmailAttachments, calendarInvitationParsingEnabled, hideInlineImageAttachments]);
 
   // Measure attachment chips in the below-header row to determine how many fit
   // on a single line; the rest collapse into a "+N attachments" overflow pill.

components/email/thread-conversation-view.tsx

@@ -5,6 +5,7 @@ import DOMPurify from "dompurify";
 import { Email, ThreadGroup } from "@/lib/jmap/types";
 import { EMAIL_SANITIZE_CONFIG, collapseBlockedImageContainers, plainTextToSafeHtml, restrictDataUriResourcesOnNode, sanitizePlainTextRenderedHtml } from "@/lib/email-sanitization";
 import { getRenderableHtmlBody } from "@/lib/email-body-selection";
+import { collectReferencedCids, isEmbeddedInBody } from "@/lib/attachment-visibility";
 import { collapsePlainTextQuotes, setupQuoteCollapse } from "@/lib/quote-collapse";
 import { fitEmailBodyWidth } from "@/lib/email-fit-width";
 import { transformInlineStyles, transformColorForDarkMode, transformBgColorForDarkMode } from "@/lib/color-transform";
@@ -436,6 +437,16 @@ function EmailCard({
     return { html: "", isHtml: false };
   }, [email, allowExternal, resolvedTheme, emailAlwaysLightMode, cidBlobUrls, t]);
 
+  // Parts the body embeds via cid: stay out of the attachment row while the
+  // user hides inline images - the desktop viewer's rule, shared through
+  // lib/attachment-visibility.ts so the two views cannot drift apart.
+  const visibleAttachments = useMemo(() => {
+    const attachments = email.attachments ?? [];
+    if (!hideInlineImageAttachments) return attachments;
+    const bodyCids = collectReferencedCids(getRenderableHtmlBody(email));
+    return attachments.filter(att => !isEmbeddedInBody(att, bodyCids));
+  }, [email, hideInlineImageAttachments]);
+
   // Render the sanitized HTML body inside a sandboxed iframe so a malicious
   // (or accidentally-bypassed) email cannot inject styles/scripts/forms into
   // the host page. CSP <meta> is defense-in-depth in case the sanitizer ever
@@ -621,11 +632,7 @@ function EmailCard({
           </div>
 
           {/* Attachments */}
-          {(() => {
-            const visibleAttachments = (email.attachments ?? []).filter(
-              att => !(hideInlineImageAttachments && att.cid && att.disposition === 'inline' && (att.type || '').startsWith('image/'))
-            );
-            return visibleAttachments.length > 0 && (
+          {visibleAttachments.length > 0 && (
             <div className="px-4 pb-4">
               <div className="flex flex-wrap gap-2">
                 {visibleAttachments.map((attachment, idx) => {
@@ -657,8 +664,7 @@ function EmailCard({
                 })}
               </div>
             </div>
-            );
-          })()}
+          )}
 
           {/* Action Buttons */}
           <div className="px-4 pb-4 flex gap-2">

lib/__tests__/attachment-visibility.test.ts

@@ -0,0 +1,74 @@
+import { describe, it, expect } from 'vitest';
+import { collectReferencedCids, isEmbeddedInBody } from '@/lib/attachment-visibility';
+
+describe('collectReferencedCids', () => {
+  it('finds cid: references wherever the body uses them', () => {
+    const html = '<img src="cid:logoImage"><td background="cid:bg1">'
+      + '<div style="background-image:url(cid:styled)"></div>'
+      + "<img src='cid:singleQuoted'>";
+    const cids = collectReferencedCids(html);
+    expect(cids.has('logoImage')).toBe(true);
+    expect(cids.has('bg1')).toBe(true);
+    expect(cids.has('styled')).toBe(true);
+    expect(cids.has('singleQuoted')).toBe(true);
+  });
+
+  it('takes references exactly as written - the renderer resolves them the same way', () => {
+    const cids = collectReferencedCids('<img src="cid:part%40host">');
+    expect(cids.has('part%40host')).toBe(true);
+    expect(cids.has('part@host')).toBe(false);
+  });
+
+  it('is empty for plain-text rendering (null) and bodies without references', () => {
+    expect(collectReferencedCids(null).size).toBe(0);
+    expect(collectReferencedCids(undefined).size).toBe(0);
+    expect(collectReferencedCids('').size).toBe(0);
+    expect(collectReferencedCids('<p>no images here</p>').size).toBe(0);
+  });
+});
+
+describe('isEmbeddedInBody', () => {
+  const body = collectReferencedCids('<img src="cid:signaturImage"><img src="cid:logoImage">');
+
+  // The wild shape that started this: octet-stream, no disposition, no name,
+  // rendered in the body via cid:.
+  it('hides sloppy octet-stream parts the body renders', () => {
+    expect(isEmbeddedInBody({ cid: 'signaturImage', type: 'application/octet-stream' }, body)).toBe(true);
+    expect(isEmbeddedInBody({ cid: 'logoImage', type: 'application/octet-stream', disposition: null }, body)).toBe(true);
+  });
+
+  it('hides referenced parts with no type at all', () => {
+    expect(isEmbeddedInBody({ cid: 'logoImage' }, body)).toBe(true);
+  });
+
+  it('tolerates angle brackets around the JMAP cid', () => {
+    expect(isEmbeddedInBody({ cid: '<logoImage>', type: 'application/octet-stream' }, body)).toBe(true);
+  });
+
+  it('keeps the legacy rule: declared inline images hide even without a reference', () => {
+    expect(isEmbeddedInBody({ cid: 'unreferenced', type: 'image/png', disposition: 'inline' }, body)).toBe(true);
+  });
+
+  it('keeps an explicit attachment visible even when the body references it', () => {
+    expect(isEmbeddedInBody({ cid: 'logoImage', type: 'image/png', disposition: 'attachment' }, body)).toBe(false);
+  });
+
+  it('keeps a referenced part with a real non-image type visible - the chip is its only download', () => {
+    expect(isEmbeddedInBody({ cid: 'logoImage', type: 'application/pdf' }, body)).toBe(false);
+  });
+
+  it('never hides unreferenced parts without an inline declaration', () => {
+    expect(isEmbeddedInBody({ cid: 'orphan', type: 'application/octet-stream' }, body)).toBe(false);
+    expect(isEmbeddedInBody({ cid: 'orphan', type: 'image/png' }, body)).toBe(false);
+  });
+
+  it('never hides parts without a cid', () => {
+    expect(isEmbeddedInBody({ type: 'image/png', disposition: 'inline' }, body)).toBe(false);
+    expect(isEmbeddedInBody({ cid: null, type: 'application/octet-stream' }, body)).toBe(false);
+  });
+
+  it('hides nothing when the message renders as plain text', () => {
+    const none = collectReferencedCids(null);
+    expect(isEmbeddedInBody({ cid: 'logoImage', type: 'application/octet-stream' }, none)).toBe(false);
+  });
+});

lib/attachment-visibility.ts

@@ -0,0 +1,70 @@
+/**
+ * Which attachment parts belong in the attachment list, and which are really
+ * pieces of the rendered body.
+ *
+ * A message body embeds images by `cid:` reference. Well-behaved senders mark
+ * those parts `Content-Disposition: inline` with an `image/*` type, and the
+ * "hide inline images" setting keeps them out of the attachment list. Sloppy
+ * senders (Foxmail, various order/invoice systems - #543's pattern) ship the
+ * very same parts as `application/octet-stream` with no disposition and no
+ * name: the body still renders them, but the declaration-based check let them
+ * through and they showed up as nameless "Attachment" chips next to the real
+ * files. The reference scan below closes that gap for the viewers the way the
+ * composer's forward path already handles it.
+ */
+
+/**
+ * The exact pattern the viewers rewrite to blob URLs, so "referenced" here
+ * means "rendered" and a chip is only ever hidden for a part the body shows.
+ */
+const CID_REFERENCE = /\bcid:([^"'\s)]+)/gi;
+
+const EMPTY: ReadonlySet<string> = new Set();
+
+/**
+ * Content-IDs a raw HTML body references via `cid:` URLs (img src, table
+ * background, style url(...) - anything), exactly as written. `null` (the
+ * message renders as plain text) yields an empty set: nothing is embedded, so
+ * nothing must be hidden.
+ */
+export function collectReferencedCids(html: string | null | undefined): ReadonlySet<string> {
+  if (!html || html.indexOf('cid:') === -1) return EMPTY;
+  const cids = new Set<string>();
+  for (const match of html.matchAll(CID_REFERENCE)) {
+    cids.add(match[1]);
+  }
+  return cids;
+}
+
+export interface AttachmentPartLike {
+  cid?: string | null;
+  type?: string | null;
+  disposition?: string | null;
+}
+
+/**
+ * True when the part is embedded in the rendered body and should stay out of
+ * the attachment list while "hide inline images" is on.
+ *
+ * - Declared inline images hide as before, referenced or not.
+ * - An explicit `attachment` disposition always keeps its chip - the sender
+ *   asked for a download, even if the body also references the part.
+ * - Otherwise a part hides only when the body actually references it AND it
+ *   is an image or generically typed (`application/octet-stream`, no type):
+ *   a referenced part with a real non-image type keeps its chip, because that
+ *   chip is its only download.
+ */
+export function isEmbeddedInBody(
+  att: AttachmentPartLike,
+  referencedCids: ReadonlySet<string>,
+): boolean {
+  if (!att.cid) return false;
+  const type = (att.type || '').toLowerCase();
+  const isImage = type.startsWith('image/');
+  if (att.disposition === 'inline' && isImage) return true;
+  if (att.disposition === 'attachment') return false;
+  const genericType = !type || type === 'application/octet-stream';
+  if (!isImage && !genericType) return false;
+  // The Content-ID may still carry its angle brackets; body references never do.
+  return referencedCids.has(att.cid.replace(/^<|>$/g, ''));
+}