Skip to content

fix: escape JavaScript line terminators in generated docs #489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stbenjam merged 5 commits into from
Aug 20, 2026
+62 −23
Merged

fix: escape JavaScript line terminators in generated docs #489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aed6228
fix: escape line terminators in generated docs
stbenjam Aug 7, 2026
3677556
fix: serialize inline JavaScript strings safely
stbenjam Aug 7, 2026
23b7c1f
test: stabilize generated script helper extraction
stbenjam Aug 7, 2026
03ea660
refactor: collapse inline JavaScript escapers
stbenjam Aug 7, 2026
a3024ec
Merge branch 'main' into agent/escape-js-attr-line-terminators
stbenjam Aug 20, 2026
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
40 changes: 22 additions & 18 deletions src/skillsaw/docs/html_renderer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ def _get_js() -> str:
if (type === 'plugins' && IS_MARKETPLACE) {
html += '<div class="search-results-heading">'+label+' ('+allPlugins.length+')</div>';
allPlugins.forEach(function(p) {
html += '<div class="search-result-item" onclick="navigateTo(\\''+escJsAttr(p.name)+'\\')">';
html += '<div class="search-result-item" onclick="navigateTo(' + escJsAttr(p.name) + ')">';
html += '<div class="search-result-icon plugin">'+esc(pName(p).charAt(0).toUpperCase())+'</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+esc(pName(p))+'</div>';
html += '<div class="search-result-subtitle">'+esc(p.description)+'</div></div></div>';
Expand All @@ -773,7 +773,7 @@ def _get_js() -> str:
if (items.length) {
html += '<div class="search-results-heading">'+label+' ('+items.length+')</div>';
items.forEach(function(r) {
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : '';
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : '';
html += '<div class="search-result-item"'+onclick+'>';
html += '<div class="search-result-icon '+esc(r.icon)+'">'+esc(r.iconChar)+'</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+esc(r.name)+'</div>';
Expand Down Expand Up @@ -801,10 +801,10 @@ def _get_js() -> str:
el.innerHTML = filterHtml + '<div class="plugins-grid" id="plugins-grid">' + plugins.map(function(p) {
var counts = buildCountBadges(p);
var ver = p.version ? '<span class="plugin-version">v'+esc(p.version)+'</span>' : '';
var cat = p.category ? '<span class="plugin-category" onclick="event.stopPropagation();navigateTo(\\'category='+escJsAttr(p.category)+'\\')">'+esc(p.category)+'</span>' : '';
var cat = p.category ? '<span class="plugin-category" onclick="event.stopPropagation();navigateTo(' + escJsAttr('category=' + p.category) + ')">'+esc(p.category)+'</span>' : '';
var allTags = (p.tags||[]).concat(p.keywords||[]);
var tagsHtml = allTags.length ? '<div class="plugin-tags">'+allTags.map(function(t){return '<span class="plugin-tag" onclick="event.stopPropagation();navigateTo(\\'q='+escJsAttr(t)+'\\')">'+esc(t)+'</span>';}).join('')+'</div>' : '';
return '<div class="plugin-card" data-category="'+(escAttr(p.category)||'')+'" onclick="navigateTo(\\''+escJsAttr(p.name)+'\\')">' +
var tagsHtml = allTags.length ? '<div class="plugin-tags">'+allTags.map(function(t){return '<span class="plugin-tag" onclick="event.stopPropagation();navigateTo(' + escJsAttr('q=' + t) + ')">'+esc(t)+'</span>';}).join('')+'</div>' : '';
return '<div class="plugin-card" data-category="'+(escAttr(p.category)||'')+'" onclick="navigateTo(' + escJsAttr(p.name) + ')">' +
'<div class="plugin-header"><div><div class="plugin-name">'+esc(pName(p))+'</div>'+ver+'</div>'+cat+'</div>' +
'<div class="plugin-description">'+(p.description_html || esc(p.description) || '<em>No description</em>')+'</div>' +
tagsHtml +
Expand All @@ -821,7 +821,7 @@ def _get_js() -> str:
function renderCategoryFilter(cats) {
var btns = '<button class="category-btn'+(activeCategory?'':' active')+'" onclick="navigateTo(\\'\\')">All</button>';
cats.forEach(function(c) {
btns += '<a href="#category='+encodeURIComponent(c)+'" class="category-btn'+(activeCategory===c?' active':'')+'" onclick="event.preventDefault();navigateTo(\\'category='+escJsAttr(c)+'\\')">'+esc(c)+'</a>';
btns += '<a href="#category='+encodeURIComponent(c)+'" class="category-btn'+(activeCategory===c?' active':'')+'" onclick="event.preventDefault();navigateTo(' + escJsAttr('category=' + c) + ')">'+esc(c)+'</a>';
});
return '<div class="category-filter" id="category-filter">' + btns + '</div>';
}
Expand Down Expand Up @@ -1032,7 +1032,7 @@ def _get_js() -> str:
if (results.plugins.length && IS_MARKETPLACE) {
html += '<div class="search-results-heading">Plugins (' + results.plugins.length + ')</div>';
results.plugins.forEach(function(p) {
html += '<div class="search-result-item" onclick="navigateTo(\\''+escJsAttr(p.name)+'\\')">';
html += '<div class="search-result-item" onclick="navigateTo(' + escJsAttr(p.name) + ')">';
html += '<div class="search-result-icon plugin">'+esc(pName(p).charAt(0).toUpperCase())+'</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+hi(pName(p),q)+'</div>';
html += '<div class="search-result-subtitle">'+hi(p.description,q)+'</div></div></div>';
Expand All @@ -1042,7 +1042,7 @@ def _get_js() -> str:
if (results.commands.length) {
html += '<div class="search-results-heading">Commands (' + results.commands.length + ')</div>';
results.commands.forEach(function(r) {
var onclick = IS_MARKETPLACE ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : '';
var onclick = IS_MARKETPLACE ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : '';
html += '<div class="search-result-item"'+onclick+'>';
html += '<div class="search-result-icon cmd">$</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+hi(r.item.full_name || r.item.name, q)+'</div>';
Expand All @@ -1055,7 +1055,7 @@ def _get_js() -> str:
if (results.skills.length) {
html += '<div class="search-results-heading">Skills (' + results.skills.length + ')</div>';
results.skills.forEach(function(r) {
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : '';
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : '';
html += '<div class="search-result-item"'+onclick+'>';
html += '<div class="search-result-icon skill">S</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+hi(r.item.name,q)+'</div>';
Expand All @@ -1068,7 +1068,7 @@ def _get_js() -> str:
if (results.agents.length) {
html += '<div class="search-results-heading">Agents (' + results.agents.length + ')</div>';
results.agents.forEach(function(r) {
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : '';
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : '';
html += '<div class="search-result-item"'+onclick+'>';
html += '<div class="search-result-icon agent">A</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+hi(r.item.name,q)+'</div>';
Expand All @@ -1081,7 +1081,7 @@ def _get_js() -> str:
if (results.hooks.length) {
html += '<div class="search-results-heading">Hooks (' + results.hooks.length + ')</div>';
results.hooks.forEach(function(r) {
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : '';
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : '';
html += '<div class="search-result-item"'+onclick+'>';
html += '<div class="search-result-icon hook">H</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+hi(r.item.event_type,q)+'</div>';
Expand All @@ -1094,7 +1094,7 @@ def _get_js() -> str:
if (results.rules.length) {
html += '<div class="search-results-heading">Rules (' + results.rules.length + ')</div>';
results.rules.forEach(function(r) {
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : '';
var onclick = IS_MARKETPLACE && r.plugin ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : '';
html += '<div class="search-result-item"'+onclick+'>';
html += '<div class="search-result-icon rule">R</div>';
html += '<div class="search-result-content"><div class="search-result-title">'+hi(r.item.name,q)+'</div>';
Expand Down Expand Up @@ -1182,6 +1182,7 @@ def _get_js() -> str:
});
}

// BEGIN_ESCAPERS
function esc(str) {
if (!str) return '';
var d = document.createElement('div');
Expand All @@ -1202,13 +1203,16 @@ def _get_js() -> str:
}

function escJsAttr(str) {
// A JS string literal nested inside an HTML attribute — two contexts, so
// two escapes in that order. innerHTML decodes the entities before the
// handler compiles, so the JS escapes must survive that decode: \' stays
// \', while " arrives as a plain quote which cannot close a
// single-quoted JS string.
return escAttr(String(str).replace(/\\\\/g, '\\\\\\\\').replace(/'/g, "\\\\'"));
// JSON.stringify is the JavaScript string-literal serializer. It handles
// quotes, backslashes, and ordinary line terminators; escape the two
// legacy Unicode line separators that it leaves literal. Then HTML-escape
// the complete literal for the double-quoted attribute.
var literal = !str ? '""' : JSON.stringify(String(str));

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve falsy values in the serializer

When an inline-handler value is numeric 0 or boolean false, this truthiness check serializes it as "" instead of preserving String(str) as the previous helper did. The docs model deliberately supports falsy manifest-derived names such as PluginDoc(name=0), so the generated plugin card now calls navigateTo("") and returns home rather than opening the plugin named 0; serialize all non-null values directly instead of using a truthiness test.

Useful? React with 👍 / 👎.

return escAttr(
literal.replace(/\\u2028/g, '\\\\u2028').replace(/\\u2029/g, '\\\\u2029')
);
}
// END_ESCAPERS

init();
})();
Expand Down
45 changes: 40 additions & 5 deletions tests/codex/test_docs_output_safety.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,16 +475,18 @@ def test_entity_encoded_schemes_are_neutralized_in_both_renderers(self, tmp_path
assert "javascript:alert" not in content

def test_emitted_page_script_survives_backslash_escaping(self, tmp_path):
"""The JS template is a non-raw Python string — backslash halving
once shipped an unparseable script and a blank page. Pin the
emitted (post-halving) escJsAttr line, and parse every script
block with node when it is available."""
"""Pin the emitted escJsAttr escapes and parse every script block
with node when it is available."""
import shutil
import subprocess

from skillsaw.docs.html_renderer import _get_js

assert ".replace(/\\\\/g, '\\\\\\\\').replace(/'/g, \"\\\\'\")" in _get_js()
js = _get_js()
assert "function escJsAttr(str)" in js
assert "JSON.stringify(String(str))" in js
assert ".replace(/\\u2028/g, '\\\\u2028')" in js
assert ".replace(/\\u2029/g, '\\\\u2029')" in js

node = shutil.which("node")
if node is None:
Expand All @@ -506,6 +508,39 @@ def test_emitted_page_script_survives_backslash_escaping(self, tmp_path):
proc = subprocess.run([node, "--check", str(js)], capture_output=True, text=True)
assert proc.returncode == 0, proc.stderr

def test_escjsattr_serializes_javascript_line_terminators(self, tmp_path):
import shutil
import subprocess

from skillsaw.docs.html_renderer import _get_js

node = shutil.which("node")
if node is None:
pytest.skip("node not available")

js = _get_js()
start_marker = " // BEGIN_ESCAPERS"
end_marker = " // END_ESCAPERS"
assert start_marker in js, "missing JavaScript escaper start marker"
assert end_marker in js, "missing JavaScript escaper end marker"
start = js.index(start_marker)
end = js.index(end_marker, start) + len(end_marker)
helpers = js[start:end]
value = "first\r\nsecond\u2028third\u2029fourth\\'quote"
expected_js = json.dumps(value, ensure_ascii=False)
expected_js = expected_js.replace("\u2028", "\\u2028").replace("\u2029", "\\u2029")
expected_attr = (
expected_js.replace("&", "&amp;")
.replace('"', "&quot;")
.replace("<", "&lt;")
.replace(">", "&gt;")
)
script = f"{helpers}\nprocess.stdout.write(JSON.stringify(escJsAttr({json.dumps(value)})));"
proc = subprocess.run([node, "-e", script], capture_output=True, text=True)

assert proc.returncode == 0, proc.stderr
assert json.loads(proc.stdout) == expected_attr


class TestSafeUrlEntityDecoding:
def test_safe_url_decodes_entities_before_scheme_validation(self):
Expand Down