diff --git a/src/skillsaw/docs/html_renderer.py b/src/skillsaw/docs/html_renderer.py index c2473227..939f2057 100644 --- a/src/skillsaw/docs/html_renderer.py +++ b/src/skillsaw/docs/html_renderer.py @@ -754,7 +754,7 @@ def _get_js() -> str: if (type === 'plugins' && IS_MARKETPLACE) { html += '
'+label+' ('+allPlugins.length+')
'; allPlugins.forEach(function(p) { - html += '
'; + html += '
'; html += '
'+esc(pName(p).charAt(0).toUpperCase())+'
'; html += '
'+esc(pName(p))+'
'; html += '
'+esc(p.description)+'
'; @@ -773,7 +773,7 @@ def _get_js() -> str: if (items.length) { html += '
'+label+' ('+items.length+')
'; 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 += '
'; html += '
'+esc(r.iconChar)+'
'; html += '
'+esc(r.name)+'
'; @@ -801,10 +801,10 @@ def _get_js() -> str: el.innerHTML = filterHtml + '
' + plugins.map(function(p) { var counts = buildCountBadges(p); var ver = p.version ? 'v'+esc(p.version)+'' : ''; - var cat = p.category ? ''+esc(p.category)+'' : ''; + var cat = p.category ? ''+esc(p.category)+'' : ''; var allTags = (p.tags||[]).concat(p.keywords||[]); - var tagsHtml = allTags.length ? '
'+allTags.map(function(t){return ''+esc(t)+'';}).join('')+'
' : ''; - return '
' + + var tagsHtml = allTags.length ? '
'+allTags.map(function(t){return ''+esc(t)+'';}).join('')+'
' : ''; + return '
' + '
'+esc(pName(p))+'
'+ver+'
'+cat+'
' + '
'+(p.description_html || esc(p.description) || 'No description')+'
' + tagsHtml + @@ -821,7 +821,7 @@ def _get_js() -> str: function renderCategoryFilter(cats) { var btns = ''; cats.forEach(function(c) { - btns += ''+esc(c)+''; + btns += ''+esc(c)+''; }); return '
' + btns + '
'; } @@ -1032,7 +1032,7 @@ def _get_js() -> str: if (results.plugins.length && IS_MARKETPLACE) { html += '
Plugins (' + results.plugins.length + ')
'; results.plugins.forEach(function(p) { - html += '
'; + html += '
'; html += '
'+esc(pName(p).charAt(0).toUpperCase())+'
'; html += '
'+hi(pName(p),q)+'
'; html += '
'+hi(p.description,q)+'
'; @@ -1042,7 +1042,7 @@ def _get_js() -> str: if (results.commands.length) { html += '
Commands (' + results.commands.length + ')
'; results.commands.forEach(function(r) { - var onclick = IS_MARKETPLACE ? ' onclick="navigateTo(\\''+escJsAttr(r.plugin)+'\\')"' : ''; + var onclick = IS_MARKETPLACE ? ' onclick="navigateTo(' + escJsAttr(r.plugin) + ')"' : ''; html += '
'; html += '
$
'; html += '
'+hi(r.item.full_name || r.item.name, q)+'
'; @@ -1055,7 +1055,7 @@ def _get_js() -> str: if (results.skills.length) { html += '
Skills (' + results.skills.length + ')
'; 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 += '
'; html += '
S
'; html += '
'+hi(r.item.name,q)+'
'; @@ -1068,7 +1068,7 @@ def _get_js() -> str: if (results.agents.length) { html += '
Agents (' + results.agents.length + ')
'; 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 += '
'; html += '
A
'; html += '
'+hi(r.item.name,q)+'
'; @@ -1081,7 +1081,7 @@ def _get_js() -> str: if (results.hooks.length) { html += '
Hooks (' + results.hooks.length + ')
'; 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 += '
'; html += '
H
'; html += '
'+hi(r.item.event_type,q)+'
'; @@ -1094,7 +1094,7 @@ def _get_js() -> str: if (results.rules.length) { html += '
Rules (' + results.rules.length + ')
'; 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 += '
'; html += '
R
'; html += '
'+hi(r.item.name,q)+'
'; @@ -1182,6 +1182,7 @@ def _get_js() -> str: }); } + // BEGIN_ESCAPERS function esc(str) { if (!str) return ''; var d = document.createElement('div'); @@ -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)); + return escAttr( + literal.replace(/\\u2028/g, '\\\\u2028').replace(/\\u2029/g, '\\\\u2029') + ); } + // END_ESCAPERS init(); })(); diff --git a/tests/codex/test_docs_output_safety.py b/tests/codex/test_docs_output_safety.py index 701d36a6..9ebbd612 100644 --- a/tests/codex/test_docs_output_safety.py +++ b/tests/codex/test_docs_output_safety.py @@ -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: @@ -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("&", "&") + .replace('"', """) + .replace("<", "<") + .replace(">", ">") + ) + 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):