ENG-1427-Custom Query Builder Layout (HTML Templates, MVP0)

[mdroidian]

for google friday

---

[linear]

ENG-1427 Implement Custom Query Builder Layout (HTML Templates, MVP0)

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[devin-ai-integration]

Devin Review found 1 potential issue.

View 6 additional findings in Devin Review.

[devin-ai-integration]

🔴 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);

---

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

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 9 additional findings in Devin Review.

[devin-ai-integration]

🔴 Infinite loop in compileTemplate when result data contains handlebars-like each block syntax

The while loop in compileTemplate will run forever when any result value contains the pattern {{#each results}}{{result.text}}{{/each}}. Since escapeHtml only escapes &, <, >, and " but not { or }, curly-brace template syntax in result data passes through unescaped.

Root cause and reproduction

When substituteResult replaces {{result.text}} with a value like {{#each results}}{{result.text}}{{/each}}, the output after output.replace(fullMatch, replacement) at line 40 is identical to the input — the regex matches the same pattern again, producing the same replacement, ad infinitum.

I confirmed this with a test:

const results = [{ text: '{{#each results}}{{result.text}}{{/each}}', uid: '1' }];
const template = '{{#each results}}{{result.text}}{{/each}}';
compileTemplate({ template, results }); // Never terminates

Result data comes from Roam Research queries and can contain arbitrary user-authored text, so this pattern is plausible in real usage. The browser tab will freeze completely because the while loop at compileTemplate.ts:34 has no iteration limit or cycle detection.

Impact: Complete UI freeze (denial of service) for any query whose results happen to contain handlebars-like each-block syntax.

Suggested change

	while (match) {
	const [fullMatch, body] = match;
	const replacement = results
	.map((result) => substituteResult({ body, result }))
	.join("");
	output = output.replace(fullMatch, replacement);
	match = output.match(EACH_BLOCK);
	}
	const MAX_ITERATIONS = 100;
	let iterations = 0;
	while (match) {
	if (++iterations > MAX_ITERATIONS) break;
	const [fullMatch, body] = match;
	const replacement = results
	.map((result) => substituteResult({ body, result }))
	.join("");
	output = output.replace(fullMatch, replacement);
	match = output.match(EACH_BLOCK);
	}

---

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

[devin-ai-integration]

🔴 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);
	}

---

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

[graphite-app]

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



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