Skip to content

Commit a107e81

Browse files
authored
Merge commit from fork
* fix: sanitize html on startup * cleanup
1 parent bcf0ac1 commit a107e81

File tree

5 files changed

+118
-2
lines changed

5 files changed

+118
-2
lines changed

frontend/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@
118118
"cssnano": "^7.1.1",
119119
"date-fns": "^4.1.0",
120120
"dequal": "^2.0.3",
121+
"dompurify": "^3.2.3",
121122
"eslint-plugin-header": "^3.1.1",
122123
"htm": "^3.1.1",
123124
"html-react-parser": "^5.2.6",
@@ -212,6 +213,7 @@
212213
"@swc-jotai/react-refresh": "^0.3.0",
213214
"@testing-library/jest-dom": "^6.8.0",
214215
"@testing-library/react": "^16.3.0",
216+
"@types/dompurify": "^3.2.0",
215217
"@types/katex": "^0.16.7",
216218
"@types/lodash-es": "^4.17.12",
217219
"@types/node": "^24.3.0",

frontend/src/components/editor/cell/useRunCells.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22

33
import { closeCompletion } from "@codemirror/autocomplete";
4+
import { atom, useSetAtom } from "jotai";
45
import useEvent from "react-use-event-hook";
56
import {
67
getNotebook,
@@ -15,6 +16,8 @@ import { useRequestClient } from "@/core/network/requests";
1516
import type { RunRequest } from "@/core/network/types";
1617
import { Logger } from "@/utils/Logger";
1718

19+
export const hasRunAnyCellAtom = atom<boolean>(false);
20+
1821
/**
1922
* Creates a function that runs all cells that have been edited or interrupted.
2023
*/
@@ -50,6 +53,7 @@ export function useRunAllCells() {
5053
export function useRunCells() {
5154
const { prepareForRun } = useCellActions();
5255
const { sendRun } = useRequestClient();
56+
const setHasRunAnyCell = useSetAtom(hasRunAnyCellAtom);
5357

5458
const runCellsMemoized = useEvent(async (cellIds: CellId[]) => {
5559
if (cellIds.length === 0) {
@@ -103,6 +107,9 @@ export async function runCells({
103107
prepareForRun({ cellId });
104108
}
105109

110+
// Set a flag that a user has manually run at least one cell.
111+
setHasRunAnyCell(true);
112+
106113
// Send the run request to the Kernel
107114
await sendRun({ cellIds: cellIds, codes: codes }).catch((error) => {
108115
Logger.error(error);

frontend/src/plugins/core/RenderHTML.tsx

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,15 @@ import parse, {
55
Element,
66
type HTMLReactParserOptions,
77
} from "html-react-parser";
8-
import React, { isValidElement, type JSX, type ReactNode, useId } from "react";
8+
import React, {
9+
isValidElement,
10+
type JSX,
11+
type ReactNode,
12+
useId,
13+
useMemo,
14+
} from "react";
915
import { CopyClipboardIcon } from "@/components/icons/copy-icon";
16+
import { sanitizeHtml, useSanitizeHtml } from "./sanitize";
1017

1118
type ReplacementFn = NonNullable<HTMLReactParserOptions["replace"]>;
1219
type TransformFn = NonNullable<HTMLReactParserOptions["transform"]>;
@@ -133,6 +140,20 @@ const CopyableCode = ({ children }: { children: ReactNode }) => {
133140
};
134141

135142
export const renderHTML = ({ html, additionalReplacements = [] }: Options) => {
143+
return (
144+
<RenderHTML html={html} additionalReplacements={additionalReplacements} />
145+
);
146+
};
147+
148+
const RenderHTML = ({ html, additionalReplacements = [] }: Options) => {
149+
const shouldSanitizeHtml = useSanitizeHtml();
150+
const sanitizedHtml = useMemo(() => {
151+
if (shouldSanitizeHtml) {
152+
return sanitizeHtml(html);
153+
}
154+
return html;
155+
}, [html, shouldSanitizeHtml]);
156+
136157
const renderFunctions: ReplacementFn[] = [
137158
replaceValidTags,
138159
replaceValidIframes,
@@ -146,7 +167,7 @@ export const renderHTML = ({ html, additionalReplacements = [] }: Options) => {
146167
removeWrappingHtmlTags,
147168
];
148169

149-
return parse(html, {
170+
return parse(sanitizedHtml, {
150171
replace: (domNode: DOMNode, index: number) => {
151172
for (const renderFunction of renderFunctions) {
152173
const replacement = renderFunction(domNode, index);
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import DOMPurify, { type Config } from "dompurify";
2+
import { atom, useAtomValue } from "jotai";
3+
import { hasRunAnyCellAtom } from "@/components/editor/cell/useRunCells";
4+
import { getInitialAppMode } from "@/core/mode";
5+
6+
/**
7+
* Whether to sanitize the html.
8+
* When running as an app or with auto_instantiate enabled
9+
* we ignore sanitization because they should be treated as a website.
10+
*/
11+
const sanitizeHtmlAtom = atom<boolean>((get) => {
12+
const hasRunAnyCell = get(hasRunAnyCellAtom);
13+
14+
// If a user has specifically run at least one cell, we don't need to sanitize.
15+
// HTML needs to be rich to allow for interactive widgets and other dynamic content.
16+
if (hasRunAnyCell) {
17+
return false;
18+
}
19+
20+
const isInAppMode = getInitialAppMode() === "read";
21+
// Apps need to run javascript and load external resources.
22+
if (isInAppMode) {
23+
return false;
24+
}
25+
26+
return true;
27+
});
28+
29+
export function useSanitizeHtml() {
30+
return useAtomValue(sanitizeHtmlAtom);
31+
}
32+
33+
// preserve target=_blank https://github.com/cure53/DOMPurify/issues/317#issuecomment-912474068
34+
const TEMPORARY_ATTRIBUTE = "data-temp-href-target";
35+
DOMPurify.addHook("beforeSanitizeAttributes", (node) => {
36+
if (node.tagName === "A") {
37+
if (!node.hasAttribute("target")) {
38+
node.setAttribute("target", "_self");
39+
}
40+
41+
if (node.hasAttribute("target")) {
42+
node.setAttribute(TEMPORARY_ATTRIBUTE, node.getAttribute("target") || "");
43+
}
44+
}
45+
});
46+
47+
DOMPurify.addHook("afterSanitizeAttributes", (node) => {
48+
if (node.tagName === "A" && node.hasAttribute(TEMPORARY_ATTRIBUTE)) {
49+
node.setAttribute("target", node.getAttribute(TEMPORARY_ATTRIBUTE) || "");
50+
node.removeAttribute(TEMPORARY_ATTRIBUTE);
51+
if (node.getAttribute("target") === "_blank") {
52+
node.setAttribute("rel", "noopener");
53+
}
54+
}
55+
});
56+
57+
/**
58+
* This removes script tags, form tags, iframe tags, and other potentially dangerous tags
59+
*/
60+
export function sanitizeHtml(html: string) {
61+
const sanitizationOptions: Config = {
62+
// Default to permit HTML, SVG and MathML, this limits to HTML only
63+
USE_PROFILES: { html: true, svg: true, mathMl: true },
64+
// glue elements like style, script or others to document.body and prevent unintuitive browser behavior in several edge-cases
65+
FORCE_BODY: true,
66+
CUSTOM_ELEMENT_HANDLING: {
67+
tagNameCheck: /^marimo-[A-Za-z][\w-]*$/,
68+
attributeNameCheck: /^[A-Za-z][\w-]*$/,
69+
},
70+
};
71+
return DOMPurify.sanitize(html, sanitizationOptions);
72+
}

pnpm-lock.yaml

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)