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 { ); 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("bold"); + expect(html).toContain("code"); + expect(html).toContain("
  • one
  • "); + }); + + 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(""); + expect(html).toContain("
    ");
    +  });
    +
    +  /*
    +   * 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("\n\n\n\n\n");
    +    expect(html).not.toContain(" {
    +    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(" {
    +    const html = renderMarkdown("![local](./diagram.png)");
    +    expect(html).not.toContain(" {
    +    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
    + * `