Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions apps/roam/src/components/results-view/CustomView.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React, { useMemo } from "react";
import { compileTemplate, sanitizeHtml } from "~/utils/compileTemplate";
import type { Result } from "~/utils/types";

export const DEFAULT_TEMPLATE = `<ul>
{{#each results}}
<li>{{result.text}}</li>
{{/each}}
</ul>`;

type CustomViewProps = {
results: Result[];
template?: string;
};

const CustomView = ({ results, template = DEFAULT_TEMPLATE }: CustomViewProps) => {
const html = useMemo(() => {
const compiled = compileTemplate({ template, results });
return sanitizeHtml({ html: compiled });
}, [results, template]);

return (
<div
className="roamjs-custom-results-view p-4"
dangerouslySetInnerHTML={{ __html: html }}
/>
);
};

export default CustomView;

129 changes: 128 additions & 1 deletion apps/roam/src/components/results-view/ResultsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import getUids from "roamjs-components/dom/getUids";
import Charts from "./Charts";
import Timeline from "./Timeline";
import Kanban from "./Kanban";
import CustomView, { DEFAULT_TEMPLATE } from "./CustomView";
import MenuItemSelect from "roamjs-components/components/MenuItemSelect";
import type { RoamBasicNode } from "roamjs-components/types/native";
import { render as renderToast } from "roamjs-components/components/Toast";
Expand All @@ -62,7 +63,11 @@ const EMBED_FOLD_VALUES = ["default", "open", "closed"];

type EnglishQueryPart = { text: string; clickId?: string };

const QueryUsed = ({ parentUid }: { parentUid: string }) => {
const QueryUsed = ({
parentUid,
}: {
parentUid: string;
}) => {
const { datalogQuery, englishQuery } = useMemo(() => {
const args = parseQuery(parentUid);
const { query: datalogQuery } = getDatalogQuery(args);
Expand Down Expand Up @@ -225,6 +230,11 @@ const SUPPORTED_LAYOUTS = [
{ key: "legend", label: "Show Legend", options: ["No", "Yes"] },
],
},
{
id: "custom",
icon: "code-block",
settings: [],
},
] as const;
const settingsById = Object.fromEntries(
SUPPORTED_LAYOUTS.map((l) => [l.id, l.settings]),
Expand Down Expand Up @@ -452,6 +462,54 @@ const ResultsView: ResultsViewComponent = ({
() => views.filter((view) => view.mode !== "hidden").length,
[views],
);
const customViewPrompt = useMemo(() => {
const selectionKeys = Array.from(
new Set(
columns
.map((c) => c.key.trim())
.filter(Boolean),
),
);
const keyLines = selectionKeys.map((k) => `- ${k}`).join("\n");
const exampleKey = selectionKeys.find((k) => k !== "text") || "text";
const exampleTemplate = `<ul>
{{#each results}}
<li>{{result.${exampleKey}}}</li>
{{/each}}
</ul>`;
const groupByExampleTemplate = `<table>
{{#each results}}
<tr>
<td>{{resultIfChanged.${exampleKey}}}</td>
<td>{{result.text}}</td>
</tr>
{{/each}}
</table>`;

return `Create a Custom HTML Layout for a query result renderer.

Requirements:
- Render with {{#each results}}...{{/each}}
- Use interpolations like {{result.key}}
- You can suppress repeated values with {{resultIfChanged.key}} or {{#ifChanged result.key}}...{{/ifChanged}}
- Do not use JavaScript, window access, or side effects
- Prefer simple, minimal CSS (avoid complex styling)
- Emulate a "group by" row pattern: when looping each result, if "{enter field}" matches the previous row's value, do not render that field again for the current row.

Available result keys:
${keyLines || "- text"}

Default template example:
${DEFAULT_TEMPLATE}

Selection-specific template example:
${exampleTemplate}

Group-by style example (hide repeated values):
${groupByExampleTemplate}

Custom view description:`;
}, [columns]);

return (
<div
Expand Down Expand Up @@ -778,6 +836,66 @@ const ResultsView: ResultsViewComponent = ({
</Label>
);
})}
{layoutMode === "custom" && (
<Label>
<div className="mb-1 flex items-center justify-between gap-2 pr-1">
<span>
Template (HTML + <code>{"{{#each results}}...{{/each}}"}</code>,{" "}
<code>{"{{result.key}}"}</code>,{" "}
<code>{"{{resultIfChanged.key}}"}</code>)
</span>
<Tooltip
content={"Copy Custom View Prompt"}
position={"top"}
openOnTargetFocus={false}
lazy={true}
hoverOpenDelay={250}
autoFocus={false}
>
<Button
minimal
icon={"duplicate"}
onClick={() => {
navigator.clipboard.writeText(customViewPrompt);
renderToast({
id: "custom-view-prompt-copy",
content: "Copied Custom View Prompt",
intent: Intent.PRIMARY,
});
}}
/>
</Tooltip>
</div>
<textarea
className="bp3-input mt-1 w-full font-mono text-sm"
rows={8}
placeholder={DEFAULT_TEMPLATE}
value={
(Array.isArray(layout.template)
? layout.template[0]
: layout.template) ?? ""
}
Comment on lines +873 to +877
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent default template handling: The textarea uses ?? "" to default to empty string, but the actual CustomView rendering (lines 1525-1527) uses || DEFAULT_TEMPLATE. If layout.template is an array with empty string [""], the user sees an empty textarea but the renderer displays DEFAULT_TEMPLATE, creating a confusing mismatch.

Fix: Use consistent logic:

value={
  (Array.isArray(layout.template)
    ? layout.template[0]
    : layout.template) || ""
}

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

onChange={(e) => {
const value = e.target.value;
setLayout({ ...layout, template: value });
if (preventSavingSettings) return;
const resultNode = getSubTree({
key: "results",
parentUid,
});
const layoutNode = getSubTree({
parentUid: resultNode.uid,
key: "layout",
});
setInputSetting({
key: "template",
value,
blockUid: layoutNode.uid,
});
}}
/>
</Label>
)}
</div>
) : isEditColumnSort ? (
<div className="relative p-4">
Expand Down Expand Up @@ -1400,6 +1518,15 @@ const ResultsView: ResultsViewComponent = ({
pageSizeTimeoutRef={pageSizeTimeoutRef}
setPageSize={setPageSize}
/>
) : layoutMode === "custom" ? (
<CustomView
results={allProcessedResults}
template={
(Array.isArray(layout.template)
? layout.template[0]
: layout.template) || DEFAULT_TEMPLATE
}
/>
) : (
<div style={{ padding: "16px 8px" }}>
Layout `{layoutMode}` is not supported
Expand Down
88 changes: 88 additions & 0 deletions apps/roam/src/utils/compileTemplate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import type { Result } from "./types";

const EACH_BLOCK = /\{\{\s*#each\s+results\s*\}\}([\s\S]*?)\{\{\s*\/each\s*\}\}/;
const RESULT_KEY = /\{\{\s*result\.([\w-]+)\s*\}\}/g;
const RESULT_IF_CHANGED = /\{\{\s*resultIfChanged\.([\w-]+)\s*\}\}/g;
const IF_CHANGED_BLOCK =
/\{\{\s*#ifChanged\s+result\.([\w-]+)\s*\}\}([\s\S]*?)\{\{\s*\/ifChanged\s*\}\}/g;

const escapeHtml = (value: unknown): string =>
String(value ?? "")
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;");

const substituteResult = ({
body,
result,
previousResult,
}: {
body: string;
result: Result;
previousResult?: Result;
}): string =>
body
.replace(IF_CHANGED_BLOCK, (_, key: string, sectionBody: string) => {
const currentValue = String(result[key] ?? "");
const previousValue = String(previousResult?.[key] ?? "");
return currentValue === previousValue ? "" : sectionBody;
})
.replace(RESULT_IF_CHANGED, (_, key: string) => {
const currentValue = String(result[key] ?? "");
const previousValue = String(previousResult?.[key] ?? "");
return currentValue === previousValue ? "" : escapeHtml(result[key]);
})
.replace(RESULT_KEY, (_, key: string) => {
return escapeHtml(result[key]);
});

export const compileTemplate = ({
template,
results,
}: {
template: string;
results: Result[];
}): string => {
let output = template;
let match = output.match(EACH_BLOCK);

while (match) {
const [fullMatch, body] = match;
const replacement = results
.map((result, index) =>
substituteResult({ body, result, previousResult: results[index - 1] }),
)
.join("");

output = output.replace(fullMatch, replacement);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Dollar-sign special replacement patterns in result data cause infinite loop or corrupted output

When result data contains $&, $', $`, or $1-$9, the String.prototype.replace call on line 40 interprets these as special replacement patterns, because the second argument is a plain string (not a function).

Root Cause and Impact

In compileTemplate at apps/roam/src/utils/compileTemplate.ts:40, the code does:

output = output.replace(fullMatch, replacement);

where fullMatch is a string and replacement is the concatenated result of substituteResult calls. When replace() receives a string as the replacement, it interprets $& as the matched substring, $' as the portion after the match, $` as the portion before, etc.

If any result value contains $& (which escapeHtml does NOT escape), the replacement re-inserts the fullMatch text (the {{#each}}...{{/each}} block) back into the output. The while loop on line 34 then matches this re-introduced block again, creating an infinite loop that freezes the browser tab.

Verified with:

const fullMatch = '{{#each results}}<li>{{result.text}}</li>{{/each}}';
const replacement = '<li>$&</li>';  // result.text was '$&'
const output = '<ul>{{#each results}}<li>{{result.text}}</li>{{/each}}</ul>';
output.replace(fullMatch, replacement);
// → '<ul><li>{{#each results}}<li>{{result.text}}</li>{{/each}}</li></ul>'
// The #each block is re-introduced → infinite loop

Impact: Any query result containing $& in any field will freeze the browser tab when using the custom view layout. $' and $` will cause corrupted/duplicated output.

Suggested change
output = output.replace(fullMatch, replacement);
output = output.slice(0, match.index!) + replacement + output.slice(match.index! + fullMatch.length);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

match = output.match(EACH_BLOCK);
}

return output;
};

export const sanitizeHtml = ({ html }: { html: string }): string => {
if (typeof document === "undefined") return html;

const container = document.createElement("div");
container.innerHTML = html;

container
.querySelectorAll("script, iframe, object, embed, style[type='text/javascript']")
.forEach((el) => el.remove());

container.querySelectorAll("*").forEach((el) => {
[...el.attributes].forEach((attr) => {
if (
/^on\w+/i.test(attr.name) ||
((attr.name === "href" || attr.name === "src") &&
/^\s*javascript:/i.test(attr.value))
) {
el.removeAttribute(attr.name);
}
Comment on lines +77 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 sanitizeHtml fails to strip javascript: URLs on formaction, action, and other URL-bearing attributes

The sanitizeHtml function only checks href and src attributes for javascript: protocol URLs (compileTemplate.ts:61), but several other HTML attributes also accept URLs and can execute JavaScript. A user-authored custom template containing e.g. <button formaction="javascript:alert(1)">Click</button> or <form action="javascript:alert(1)"> will pass through the sanitizer and execute arbitrary JavaScript when interacted with.

Detailed explanation of the bypass

The attribute check at lines 59-65 is:

(attr.name === "href" || attr.name === "src") &&
  /^\s*javascript:/i.test(attr.value)

This misses at least:

  • formaction on <button> and <input type="submit"> — executes on click
  • action on <form> — executes on submit
  • xlink:href on SVG elements
  • data on <object>
  • srcdoc on <iframe> (though <iframe> is removed, if the list ever changes)
  • poster on <video>

Since the rendered HTML is injected via dangerouslySetInnerHTML in CustomView.tsx:25, any JavaScript that slips through the sanitizer will execute in the extension's context. In Roam Research, queries can be shared, so a malicious template authored by one user could execute code in another user's session.

Impact: XSS in shared query templates via attributes not covered by the sanitizer's allowlist.

Suggested change
if (
/^on\w+/i.test(attr.name) ||
((attr.name === "href" || attr.name === "src") &&
/^\s*javascript:/i.test(attr.value))
) {
el.removeAttribute(attr.name);
}
const URL_ATTRS = new Set(["href", "src", "action", "formaction", "xlink:href", "data", "poster", "srcdoc"]);
if (
/^on\w+/i.test(attr.name) ||
(URL_ATTRS.has(attr.name.toLowerCase()) &&
/^\s*javascript:/i.test(attr.value))
) {
el.removeAttribute(attr.name);
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

});
});

return container.innerHTML;
};