From 15f2c3d357cffb485230f93f0b9dd4ddcbeac5bd Mon Sep 17 00:00:00 2001
From: John Coffey <johnellis@linux.com>
Date: Tue, 1 Sep 2026 20:32:59 -0700
Subject: [PATCH] Read a Markdown file as the document it is

A .md previewed as its own source, which is reading the punctuation
rather than the notes. It now opens rendered, with Rendered | Source in
the dialog footer for anyone who wants what the file actually says.
Markdown only; a .txt has nothing to toggle between.

Rendering is `marked`, sanitised by DOMPurify -- the one the app already
carries for mail. Markdown is not a safe subset of anything: raw HTML
passes through it by design, so a <script> in a file somebody uploaded or
shared into the account is a script tag unless something takes it out.

Images become links rather than pictures. An image in a Markdown file is
either a relative path, which has no base to resolve against here, or a
URL somewhere else, which fetches on open and tells that server the file
was read -- the tracking pixel this app blocks in mail. The link keeps
the alt text and the address, so nothing vanishes silently.

Fixes the PDF preview while here, which never worked: securityHeaders
put X-Frame-Options: DENY on every response including the blob route, so
the iframe showed Chrome's "refused to connect" where the file should
have been -- in Files today and in mail attachments long before that.
The middleware now leaves a header the route has set, and a PDF served
inline says SAMEORIGIN. Nothing else on the server is framable.
---
 package-lock.json                      | 23 ++++++--
 server/src/app.test.ts                 | 19 +++++++
 server/src/app.ts                      | 27 +++++++++-
 web/package.json                       |  1 +
 web/src/lib/__tests__/markdown.test.ts | 75 ++++++++++++++++++++++++++
 web/src/lib/markdown.ts                | 72 +++++++++++++++++++++++++
 web/src/styles/app.css                 | 38 +++++++++++++
 web/src/ui/filepreview.tsx             | 31 ++++++++---
 8 files changed, 273 insertions(+), 13 deletions(-)
 create mode 100644 web/src/lib/__tests__/markdown.test.ts
 create mode 100644 web/src/lib/markdown.ts

diff --git a/package-lock.json b/package-lock.json
index 058d731..f3691f2 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
 {
   "name": "ihasmail",
-  "version": "2.0.0",
+  "version": "0.0.0",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "ihasmail",
-      "version": "2.0.0",
+      "version": "0.0.0",
       "license": "AGPL-3.0-or-later",
       "workspaces": [
         "server",
@@ -2321,6 +2321,18 @@
         "@jridgewell/sourcemap-codec": "^1.5.5"
       }
     },
+    "node_modules/marked": {
+      "version": "18.0.11",
+      "resolved": "https://registry.npmjs.org/marked/-/marked-18.0.11.tgz",
+      "integrity": "sha512-HnslJfsZkRPBDJRHvVtAaWlZHEpSu7u8LgQuJCELjRKuWR+hpq4A7sLq3p8HaI9ypVoXDXxV34CsQJEe1+J5Aw==",
+      "license": "MIT",
+      "bin": {
+        "marked": "bin/marked.js"
+      },
+      "engines": {
+        "node": ">= 20"
+      }
+    },
     "node_modules/mitt": {
       "version": "3.0.1",
       "resolved": "https://registry.npmjs.org/mitt/-/mitt-3.0.1.tgz",
@@ -3814,7 +3826,8 @@
     },
     "server": {
       "name": "@ihasmail/server",
-      "version": "2.0.0",
+      "version": "2.16.0",
+      "license": "AGPL-3.0-or-later",
       "dependencies": {
         "@hono/node-server": "^1.13.8",
         "hono": "^4.7.4"
@@ -3827,11 +3840,13 @@
     },
     "web": {
       "name": "@ihasmail/web",
-      "version": "2.0.0",
+      "version": "0.0.0",
+      "license": "AGPL-3.0-or-later",
       "dependencies": {
         "@tanstack/react-virtual": "^3.13.2",
         "dompurify": "^3.2.4",
         "lucide-react": "^0.477.0",
+        "marked": "^18.0.11",
         "qrcode-generator": "^2.0.4",
         "react": "^19.0.0",
         "react-dom": "^19.0.0",
diff --git a/server/src/app.test.ts b/server/src/app.test.ts
index 207a629..6699cde 100644
--- a/server/src/app.test.ts
+++ b/server/src/app.test.ts
@@ -88,3 +88,22 @@ test("a Sieve script larger than a compressing hop's threshold survives the prox
     origin.close();
   }
 });
+
+test("only a PDF blob may be framed, and only by us", async () => {
+  /*
+   * The PDF preview is an iframe, and the blanket X-Frame-Options: DENY on
+   * every response blocked it -- the dialog showed Chrome's "refused to
+   * connect" where the file should have been. The middleware now leaves a
+   * header a route has already set, so this pins both halves: the exception
+   * exists, and it did not become the rule.
+   */
+  const app = createApp();
+  const health = await app.request("/api/health");
+  assert.equal(health.headers.get("x-frame-options"), "DENY");
+
+  const { securityHeadersFor } = await import("./app.js");
+  assert.equal(securityHeadersFor("application/pdf", true), "SAMEORIGIN");
+  assert.equal(securityHeadersFor("application/pdf", false), "DENY");
+  assert.equal(securityHeadersFor("image/png", true), "DENY");
+  assert.equal(securityHeadersFor("text/html", true), "DENY");
+});
diff --git a/server/src/app.ts b/server/src/app.ts
index 38bbf00..d9e3781 100644
--- a/server/src/app.ts
+++ b/server/src/app.ts
@@ -82,7 +82,9 @@ const securityHeaders: MiddlewareHandler = async (c, next) => {
   await next();
   const h = c.res.headers;
   h.set("X-Content-Type-Options", "nosniff");
-  h.set("X-Frame-Options", "DENY");
+  /* A route that must be framable says so; everything else is DENY. The blob
+     route is the only one, and only for PDFs -- see the note there. */
+  if (!h.has("X-Frame-Options")) h.set("X-Frame-Options", "DENY");
   h.set("Referrer-Policy", "no-referrer");
   h.set("Permissions-Policy", "camera=(), microphone=(), geolocation=(), payment=(), usb=()");
   h.set("Cross-Origin-Opener-Policy", "same-origin");
@@ -538,7 +540,19 @@ export function createApp(): Hono<Env> {
       );
       headers.set("X-Content-Type-Options", "nosniff");
       // Sandbox everything except the browser's built-in PDF viewer (which needs scripts to render).
-      if (!(safeInline && type === "application/pdf")) {
+      if (securityHeadersFor(type, safeInline) === "SAMEORIGIN") {
+        /*
+         * The one response on the server that may be framed.
+         *
+         * A PDF is shown in an iframe -- it is its own document and the app
+         * cannot lay it out -- and the blanket X-Frame-Options: DENY above
+         * blocked that, so the preview showed Chrome's "refused to connect"
+         * instead of the file. SAMEORIGIN, not a relaxation to any site: the
+         * frame is ours, on our origin, and the app's own CSP already says
+         * frame-src 'self'. Nothing else here is framed, so nothing else asks.
+         */
+        headers.set("X-Frame-Options", "SAMEORIGIN");
+      } else {
         headers.set("Content-Security-Policy", "sandbox; default-src 'none'; style-src 'unsafe-inline'; img-src data:");
       }
       headers.set("Cache-Control", "private, max-age=3600");
@@ -697,6 +711,15 @@ function sanitizeContentType(ct: string): string {
   return lower || "application/octet-stream";
 }
 
+/**
+ * What X-Frame-Options a blob response carries. Exported so the rule is
+ * testable without standing up an upstream: a PDF served inline may be framed
+ * by us and nothing else may be framed at all.
+ */
+export function securityHeadersFor(type: string, safeInline: boolean): "SAMEORIGIN" | "DENY" {
+  return safeInline && type.split(";")[0]!.trim() === "application/pdf" ? "SAMEORIGIN" : "DENY";
+}
+
 function isInlineSafe(type: string): boolean {
   const t = type.split(";")[0]!.trim();
   return (
diff --git a/web/package.json b/web/package.json
index 6245554..b4543e6 100644
--- a/web/package.json
+++ b/web/package.json
@@ -15,6 +15,7 @@
     "@tanstack/react-virtual": "^3.13.2",
     "dompurify": "^3.2.4",
     "lucide-react": "^0.477.0",
+    "marked": "^18.0.11",
     "qrcode-generator": "^2.0.4",
     "react": "^19.0.0",
     "react-dom": "^19.0.0",
diff --git a/web/src/lib/__tests__/markdown.test.ts b/web/src/lib/__tests__/markdown.test.ts
new file mode 100644
index 0000000..d43bb97
--- /dev/null
+++ b/web/src/lib/__tests__/markdown.test.ts
@@ -0,0 +1,75 @@
+import { describe, expect, it } from "vitest";
+import { isMarkdown, renderMarkdown } from "@/lib/markdown";
+
+describe("isMarkdown", () => {
+  it("takes the type when there is one", () => {
+    expect(isMarkdown("text/markdown", "a")).toBe(true);
+    expect(isMarkdown("text/x-markdown; charset=utf-8", "a")).toBe(true);
+    expect(isMarkdown("text/plain", "notes.txt")).toBe(false);
+  });
+
+  it("falls back to the name, which is the usual case for an upload", () => {
+    expect(isMarkdown("application/octet-stream", "README.md")).toBe(true);
+    expect(isMarkdown("application/octet-stream", "NOTES.MARKDOWN")).toBe(true);
+    expect(isMarkdown(null, "changelog.mkd")).toBe(true);
+    expect(isMarkdown(null, "readme.txt")).toBe(false);
+    expect(isMarkdown(null, null)).toBe(false);
+  });
+});
+
+describe("renderMarkdown", () => {
+  it("renders the ordinary things", () => {
+    const html = renderMarkdown("# Title\n\nSome **bold** and `code`.\n\n- one\n- two\n");
+    expect(html).toContain("<h1");
+    expect(html).toContain("<strong>bold</strong>");
+    expect(html).toContain("<code>code</code>");
+    expect(html).toContain("<li>one</li>");
+  });
+
+  it("renders GitHub tables and fenced code", () => {
+    const html = renderMarkdown("| a | b |\n| - | - |\n| 1 | 2 |\n\n```js\nconst x = 1;\n```\n");
+    expect(html).toContain("<table>");
+    expect(html).toContain("<pre>");
+  });
+
+  /*
+   * Markdown passes raw HTML through by design, and the file came from
+   * somewhere else -- an upload, or a share from another account. Every one of
+   * these renders as a script tag without a sanitiser.
+   */
+  it("takes out anything that would execute", () => {
+    const html = renderMarkdown("<script>alert(1)</script>\n\n<img src=x onerror=alert(1)>\n\n<iframe src='https://evil.example'></iframe>\n");
+    expect(html).not.toContain("<script");
+    expect(html).not.toContain("onerror");
+    expect(html).not.toContain("<iframe");
+  });
+
+  it("does not keep a javascript: link", () => {
+    const html = renderMarkdown("[click](javascript:alert(1))");
+    expect(html).not.toContain("javascript:");
+  });
+
+  it("shows an image as a link instead of fetching it", () => {
+    // A remote image in a file is a tracking pixel by another name; this app
+    // blocks those in mail and does not undo that here.
+    const html = renderMarkdown("![a diagram](https://tracker.example/px.png)");
+    expect(html).not.toContain("<img");
+    expect(html).toContain('class="md-img"');
+    expect(html).toContain("a diagram");
+    expect(html).toContain("https://tracker.example/px.png");
+  });
+
+  it("keeps a relative image visible even though it cannot resolve", () => {
+    const html = renderMarkdown("![local](./diagram.png)");
+    expect(html).not.toContain("<img");
+    expect(html).toContain("local");
+    // Nothing to link to, so it is text rather than a dead link.
+    expect(html).not.toContain('href="./diagram.png"');
+  });
+
+  it("sends links out of the app safely", () => {
+    const html = renderMarkdown("[docs](https://docs.ihasmail.org)");
+    expect(html).toContain('rel="noopener noreferrer"');
+    expect(html).toContain('target="_blank"');
+  });
+});
diff --git a/web/src/lib/markdown.ts b/web/src/lib/markdown.ts
new file mode 100644
index 0000000..abec1da
--- /dev/null
+++ b/web/src/lib/markdown.ts
@@ -0,0 +1,72 @@
+import DOMPurify from "dompurify";
+import { marked } from "marked";
+
+/**
+ * Markdown, rendered for the file viewer.
+ *
+ * The source is somebody else's file -- uploaded, or shared into the account
+ * by another user -- so it is treated as hostile. Markdown is not a safe
+ * subset of anything: raw HTML passes straight through it by design, so
+ * `<script>` in a .md is a script tag unless something takes it out. That
+ * something is DOMPurify, which the app already carries for mail.
+ *
+ * Rendered inline rather than in a shadow root the way mail bodies are: this
+ * output is ours, sanitised and styled by `.md-body`, where an email arrives
+ * with a design of its own that has to be quarantined from the app's.
+ */
+
+marked.use({ gfm: true, breaks: false });
+
+export function isMarkdown(type: string | null | undefined, name: string | null | undefined): boolean {
+  const t = (type ?? "").split(";")[0]!.trim().toLowerCase();
+  if (t === "text/markdown" || t === "text/x-markdown") return true;
+  // A .md upload usually arrives as application/octet-stream, so the name is
+  // the only evidence -- the same reason previewKind falls back to it.
+  return /\.(md|markdown|mdown|mkd)$/i.test(name ?? "");
+}
+
+export function renderMarkdown(source: string): string {
+  const html = marked.parse(source, { async: false });
+  const clean = DOMPurify.sanitize(html, {
+    WHOLE_DOCUMENT: false,
+    RETURN_DOM: true,
+    USE_PROFILES: { html: true },
+    FORBID_TAGS: ["script", "iframe", "frame", "frameset", "object", "embed", "applet", "form", "input", "button", "textarea", "select", "meta", "link", "base", "svg", "math", "video", "audio", "source", "track", "canvas", "template", "noscript", "style"],
+    FORBID_ATTR: ["srcdoc", "formaction", "action", "ping", "autofocus", "style"],
+    ALLOW_DATA_ATTR: false,
+    ADD_ATTR: ["target", "rel"],
+  }) as unknown as HTMLElement;
+
+  /*
+   * Pictures become links rather than pictures.
+   *
+   * An image in a Markdown file is either a relative path, which has no base
+   * to resolve against here and would render broken, or a URL somewhere else,
+   * which fetches on open and tells that server the file was read -- the same
+   * tracking pixel this app blocks in mail. Neither is worth rendering. A link
+   * keeps the alt text and the address visible, so nothing vanishes silently
+   * and the reader chooses whether to fetch it.
+   */
+  for (const img of [...clean.querySelectorAll("img")]) {
+    const href = img.getAttribute("src") ?? "";
+    const label = img.getAttribute("alt") || href || "image";
+    const a = clean.ownerDocument.createElement("a");
+    a.className = "md-img";
+    a.textContent = label;
+    if (/^https?:/i.test(href)) {
+      a.setAttribute("href", href);
+      a.setAttribute("target", "_blank");
+      a.setAttribute("rel", "noopener noreferrer");
+      a.setAttribute("title", href);
+    }
+    img.replaceWith(a);
+  }
+
+  // Links leave the app, so they leave it safely.
+  for (const a of clean.querySelectorAll("a[href]")) {
+    a.setAttribute("target", "_blank");
+    a.setAttribute("rel", "noopener noreferrer");
+  }
+
+  return clean.innerHTML;
+}
diff --git a/web/src/styles/app.css b/web/src/styles/app.css
index eb7a961..bf9a589 100644
--- a/web/src/styles/app.css
+++ b/web/src/styles/app.css
@@ -310,6 +310,42 @@ a.menu-item:hover { color: var(--fg); }
 .dialog-body { padding: 8px 20px 16px; overflow: auto; }
 .dialog-foot { display: flex; align-items: center; justify-content: flex-end; gap: 8px; padding: 12px 20px 16px; border-top: 1px solid var(--border); }
 .dialog-foot .left { margin-right: auto; }
+
+/* Two mutually exclusive views of the same thing, sized to sit in a dialog
+   footer beside the ordinary buttons (see ui/filepreview.tsx). */
+.segmented { display: inline-flex; border: 1px solid var(--border); border-radius: var(--radius-sm); overflow: hidden; }
+.segmented button { display: inline-flex; align-items: center; gap: 6px; height: 30px; padding: 0 10px; font-size: .9em; color: var(--fg-muted); background: none; }
+.segmented button + button { border-left: 1px solid var(--border); }
+.segmented button:hover { background: var(--bg-hover); color: var(--fg); }
+.segmented button.active { background: var(--accent-soft); color: var(--accent-soft-fg); font-weight: 600; }
+.segmented button:focus-visible { outline: 2px solid var(--accent); outline-offset: -2px; }
+.segmented.left { margin-right: auto; }
+
+/* Rendered Markdown in the file viewer. Deliberately plain: this is somebody's
+   notes, not a web page, and the point is to read it. */
+.md-body { max-height: 65vh; overflow: auto; padding: 4px 2px; overflow-wrap: anywhere; }
+.md-body > :first-child { margin-top: 0; }
+.md-body > :last-child { margin-bottom: 0; }
+.md-body h1, .md-body h2, .md-body h3, .md-body h4 { margin: 1.2em 0 .5em; line-height: 1.25; font-weight: 650; }
+.md-body h1 { font-size: 1.5em; }
+.md-body h2 { font-size: 1.28em; }
+.md-body h3 { font-size: 1.12em; }
+.md-body h4 { font-size: 1em; }
+.md-body p, .md-body ul, .md-body ol, .md-body blockquote, .md-body table { margin: 0 0 .8em; }
+.md-body ul, .md-body ol { padding-left: 1.6em; }
+.md-body li { margin: .2em 0; }
+.md-body a { color: var(--link); }
+.md-body code { font-family: var(--font-mono); font-size: .9em; background: var(--bg-sunken); border-radius: 4px; padding: .1em .35em; }
+.md-body pre { background: var(--bg-sunken); border: 1px solid var(--border); border-radius: var(--radius-sm); padding: 10px 12px; overflow-x: auto; }
+.md-body pre code { background: none; padding: 0; }
+.md-body blockquote { margin-left: 0; padding-left: 12px; border-left: 3px solid var(--border-strong); color: var(--fg-muted); }
+.md-body hr { border: 0; border-top: 1px solid var(--border); margin: 1.2em 0; }
+.md-body table { border-collapse: collapse; }
+.md-body th, .md-body td { border: 1px solid var(--border); padding: 5px 9px; text-align: left; }
+.md-body th { background: var(--bg-sunken); }
+/* An image is shown as its link, never fetched -- see lib/markdown.ts. */
+.md-body .md-img::before { content: "🖼 "; }
+.md-body .md-img { color: var(--link); text-decoration: underline dotted; text-underline-offset: 2px; }
 /* "This occurrence or the whole series" — one button per answer, stacked, so
    the destructive one is read rather than landed on by muscle memory. */
 .dialog-choices { display: flex; flex-direction: column; gap: 8px; }
@@ -1261,6 +1297,8 @@ select optgroup { background-color: var(--bg-elev); color: var(--fg); }
   .printing-preview .dialog-head, .printing-preview .dialog-foot { display: none !important; }
   .printing-preview .dialog-body { padding: 0 !important; overflow: visible !important; }
   .printing-preview .dialog-body .code { max-height: none !important; overflow: visible !important; border: 0 !important; padding: 0 !important; }
+  .printing-preview .dialog-body .md-body { max-height: none !important; overflow: visible !important; }
+  .printing-preview .md-body pre { overflow: visible !important; white-space: pre-wrap !important; }
   .printing-preview .dialog-body img { max-height: none !important; }
   .print-only { display: block; }
   body { background: #fff; color: #000; }
diff --git a/web/src/ui/filepreview.tsx b/web/src/ui/filepreview.tsx
index 9882751..553d25b 100644
--- a/web/src/ui/filepreview.tsx
+++ b/web/src/ui/filepreview.tsx
@@ -1,8 +1,9 @@
-import { useEffect, useRef, useState, type ReactNode } from "react";
-import { Download, Printer } from "lucide-react";
+import { useEffect, useMemo, useRef, useState, type ReactNode } from "react";
+import { Code2, Download, Eye, Printer } from "lucide-react";
 import { Dialog } from "./dialog";
 import { formatSize } from "@/lib/format";
 import { previewKind, TEXT_PREVIEW_CHARS, TEXT_PREVIEW_MAX } from "@/lib/preview";
+import { isMarkdown, renderMarkdown } from "@/lib/markdown";
 import { t } from "@/lib/i18n";
 
 /**
@@ -31,6 +32,10 @@ export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFil
   const kind = file ? previewKind(file.type, file.name) : null;
   const tooBig = kind === "text" && typeof file?.size === "number" && file.size > TEXT_PREVIEW_MAX;
   const pdfRef = useRef<HTMLIFrameElement>(null);
+  const markdown = Boolean(file) && kind === "text" && isMarkdown(file!.type, file!.name);
+  /* Markdown opens as the document it is meant to be; the source is a click
+     away for anyone who wants to see what it actually says. */
+  const [rendered, setRendered] = useState(true);
 
   /*
    * Print what is on screen, not the mail or the file list behind it.
@@ -79,6 +84,12 @@ export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFil
       size="xl"
       footer={file && (
         <>
+          {markdown && !tooBig && (
+            <div className="segmented left" role="group" aria-label={t("View as")}>
+              <button className={rendered ? "active" : ""} aria-pressed={rendered} onClick={() => setRendered(true)}><Eye size={14} />  {t("Rendered")}</button>
+              <button className={rendered ? "" : "active"} aria-pressed={!rendered} onClick={() => setRendered(false)}><Code2 size={14} />  {t("Source")}</button>
+            </div>
+          )}
           {kind && !tooBig && <button className="btn" onClick={print}><Printer size={16} />  {t("Print")}</button>}
           <a className="btn" href={file.url} download={file.name}><Download size={16} />  {t("Download")}</a>
         </>
@@ -96,7 +107,7 @@ export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFil
             /* `url`, not `inlineUrl`: fetch pays no attention to
                Content-Disposition, so this works for the text types the server
                will not serve inline -- Markdown among them. */
-            <TextPreview url={file.url} />
+            <TextPreview url={file.url} markdown={markdown && rendered} />
           ) : (
             <p className="hint">{t("There is no preview for this kind of file.")}</p>
           )}
@@ -107,7 +118,7 @@ export function FilePreviewDialog({ file, onClose, caption }: { file: PreviewFil
   );
 }
 
-function TextPreview({ url }: { url: string }) {
+function TextPreview({ url, markdown }: { url: string; markdown: boolean }) {
   const [text, setText] = useState<string | null>(null);
   const [truncated, setTruncated] = useState(false);
   useEffect(() => {
@@ -126,12 +137,18 @@ function TextPreview({ url }: { url: string }) {
       live = false;
     };
   }, [url]);
+  /* Rendering is not free on a long file, and the toggle flips back and forth. */
+  const html = useMemo(() => (markdown && text ? renderMarkdown(text) : null), [markdown, text]);
   return (
     <>
       {/* Someone else's file: not ours to translate, and not ours to reflow. */}
-      <pre className="code notranslate" translate="no" style={{ maxHeight: "65vh", whiteSpace: "pre-wrap" }}>
-        {text ?? t("Loading…")}
-      </pre>
+      {html !== null ? (
+        <div className="md-body notranslate" translate="no" dangerouslySetInnerHTML={{ __html: html }} />
+      ) : (
+        <pre className="code notranslate" translate="no" style={{ maxHeight: "65vh", whiteSpace: "pre-wrap" }}>
+          {text ?? t("Loading…")}
+        </pre>
+      )}
       {truncated && <p className="hint">{t("Only the beginning is shown — download the file for the rest.")}</p>}
     </>
   );