fix(renderer): address review feedback on PR #475

This commit is contained in:
Hermes Agent
2026-04-14 21:13:53 +00:00
parent 5a79dd0dc9
commit b673006b7f
2 changed files with 27 additions and 3 deletions

View File

@@ -465,7 +465,7 @@ function renderMd(raw){
t=t.replace(/`([^`\n]+)`/g,(_,x)=>`<code>${esc(x)}</code>`);
// Stash [label](url) links before autolink so the URL in href= is not re-linked
const _link_stash=[];
t=t.replace(/\[([^\]]+)\]\((https?:\/\/[^\)]+)\)/g,(_,lb,u)=>{_link_stash.push(`<a href="${u}" target="_blank" rel="noopener">${esc(lb)}</a>`);return `\x00L${_link_stash.length-1}\x00`;});
t=t.replace(/\[([^\]]+)\]\((https?:\/\/[^\)]+)\)/g,(_,lb,u)=>{_link_stash.push(`<a href="${u.replace(/"/g,'%22')}" target="_blank" rel="noopener">${esc(lb)}</a>`);return `\x00L${_link_stash.length-1}\x00`;});
t=t.replace(/(https?:\/\/[^\s<>"')\]]+)/g,(url)=>{const trail=url.match(/[.,;:!?)]$/)?url.slice(-1):'';const clean=trail?url.slice(0,-1):url;return `<a href="${clean}" target="_blank" rel="noopener">${esc(clean)}</a>${trail}`;});
t=t.replace(/\x00L(\d+)\x00/g,(_,i)=>_link_stash[+i]);
// Escape any plain text that isn't already wrapped in a tag we produced
@@ -501,10 +501,10 @@ function renderMd(raw){
}
return html+'</ol>';
});
// Stash existing <a> tags before link pass so autolink never re-links already-linked URLs
// Stash existing <a> tags so the autolink pass below does not re-link their href= URLs
const _a_stash=[];
s=s.replace(/(<a\b[^>]*>[\s\S]*?<\/a>)/g,m=>{_a_stash.push(m);return `\x00A${_a_stash.length-1}\x00`;});
s=s.replace(/\[([^\]]+)\]\((https?:\/\/[^\)]+)\)/g,(_,label,url)=>`<a href="${url}" target="_blank" rel="noopener">${esc(label)}</a>`);
s=s.replace(/\[([^\]]+)\]\((https?:\/\/[^\)]+)\)/g,(_,label,url)=>`<a href="${url.replace(/"/g,'%22')}" target="_blank" rel="noopener">${esc(label)}</a>`);
s=s.replace(/\x00A(\d+)\x00/g,(_,i)=>_a_stash[+i]);
// Tables: | col | col | header row followed by | --- | --- | separator then data rows
s=s.replace(/((?:^\|.+\|\n?)+)/gm,block=>{

View File

@@ -236,3 +236,27 @@ def test_link_not_broken_by_prior_autolink():
assert f'href="{url1}"' in result
assert f'href="{url2}"' in result
assert '#461' in result
def test_href_quote_sanitized():
"""A URL containing a double-quote must have it percent-encoded in href to prevent attribute breakout."""
# This would break out of href="..." and inject an event handler without the fix
url = 'https://evil.com" onmouseover="alert(1)'
# The [label](url) regex captures up to the closing ), so we test via the render helper
# by constructing a URL that contains a literal quote character
safe_url = 'https://example.com/path"with"quotes'
result = render_links_only(f'[click]({safe_url})')
# The href must not contain a raw unencoded double-quote
href_start = result.find('href="') + 6
href_end = result.find('"', href_start)
href_val = result[href_start:href_end]
assert '"' not in href_val, (
f"href value must not contain unencoded double-quote. Got href: {href_val}"
)
def test_js_source_sanitizes_quotes_in_href():
"""JS source must apply quote percent-encoding to URLs before placing in href."""
# Both the inlineMd stash and outer link pass must sanitize quotes
assert "%22" in UI_JS, (
"URL placed in href should have double-quotes percent-encoded via .replace to %22"
)