diff --git a/static/messages.js b/static/messages.js index 85a16c54..cb6ce5cf 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4219,7 +4219,15 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // Allowed URL schemes for anchors and images rendered from agent-streamed markdown. // Raw file:// anchors are rewritten to /api/media before the user can click them. const _SMD_SAFE_URL_RE=/^(?:https?:|mailto:|tel:|message:|\/|#|\?|\.|api|session\/)/i; + // ui.js owns the image-only data URI policy. It loads before this script; + // fail closed if that contract is unavailable rather than inventing a second + // allowlist that can drift from settled rendering. const _SMD_SAFE_IMG_URL_RE=/^(?:https?:|mailto:|tel:|\/|#|\?|\.)/i; + function _smdImgSrcAllowed(v){ + const s=String(v||''); + if(/^data:/i.test(s)) return typeof _isSafeDataImageUri==='function'&&_isSafeDataImageUri(s); + return _SMD_SAFE_IMG_URL_RE.test(s); + } function _smdLinkHref(raw){ const href=String(raw||''); if(/^session:\/\//i.test(href)){ @@ -4262,7 +4270,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const _im=root.querySelectorAll('img[src]'); for(let i=0;i<_im.length;i++){ const n=_im[i],v=n.getAttribute('src')||''; - if(!_SMD_SAFE_IMG_URL_RE.test(v)){n.removeAttribute('src');n.setAttribute('data-blocked-scheme','1');} + if(!_smdImgSrcAllowed(v)){n.removeAttribute('src');n.setAttribute('data-blocked-scheme','1');} } } @@ -4390,7 +4398,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ renderer.set_attr=(data,attr,value)=>{ const isHref=window.smd&&attr===window.smd.HREF; const isSrc=window.smd&&attr===window.smd.SRC; - const safeUrl=isSrc?_SMD_SAFE_IMG_URL_RE:_SMD_SAFE_URL_RE; + const allowed=isSrc?_smdImgSrcAllowed(value):_SMD_SAFE_URL_RE.test(String(value||'')); if(isHref&&/^(file|workspace|session):\/\//i.test(String(value||''))){ baseSetAttr(data,attr,_smdLinkHref(value)); if(/^session:\/\//i.test(String(value||''))){ @@ -4399,7 +4407,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } return; } - if((isHref||isSrc)&&!safeUrl.test(String(value||''))){ + if((isHref||isSrc)&&!allowed){ const node=data&&data.nodes&&data.nodes[data.index]; if(node&&node.setAttribute) node.setAttribute('data-blocked-scheme','1'); return; @@ -4642,7 +4650,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ renderer.set_attr=(data,attr,value)=>{ const isHref=window.smd&&attr===window.smd.HREF; const isSrc=window.smd&&attr===window.smd.SRC; - const safeUrl=isSrc?_SMD_SAFE_IMG_URL_RE:_SMD_SAFE_URL_RE; + const allowed=isSrc?_smdImgSrcAllowed(value):_SMD_SAFE_URL_RE.test(String(value||'')); if(isHref&&/^(file|workspace|session):\/\//i.test(String(value||''))){ baseSetAttr(data,attr,_smdLinkHref(value)); if(/^session:\/\//i.test(String(value||''))){ @@ -4651,7 +4659,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } return; } - if((isHref||isSrc)&&!safeUrl.test(String(value||''))){ + if((isHref||isSrc)&&!allowed){ const node=data&&data.nodes&&data.nodes[data.index]; if(node&&node.setAttribute) node.setAttribute('data-blocked-scheme','1'); return; diff --git a/static/ui.js b/static/ui.js index ab1ae0df..00952e4a 100644 --- a/static/ui.js +++ b/static/ui.js @@ -2553,8 +2553,53 @@ function _mediaPlayerHtml(kind, src, name, extra=''){ // streamed prose loses its image when the answer settles (#MEDIA-in-stream). // `sessionId` is forwarded into /api/media so the same allow-list check applies // to streamed references too; falls back to whatever the current session is. -function _inlineMediaHtmlForRef(ref, sessionId){ +// data:image/* URIs the renderer may embed directly as . Only raster +// formats plus base64 SVG (scripts do not execute inside ), only safe payload +// chars, and bounded size — everything else (data:text/html etc.) must +// keep rendering as inert text so a model-emitted data: URI can never become an +// executable document. +const _DATA_IMAGE_RE=/^data:image\/(?:png|jpe?g|gif|webp|avif)(?:;base64)?,[a-z0-9+/=%._~:@!$&'()*+,;-]*$/i; +const _DATA_IMAGE_SVG_RE=/^data:image\/svg\+xml;base64,[a-z0-9+/=]+$/i; +const _DATA_IMAGE_MAX_LEN=2*1024*1024; + +// The streaming renderer calls this ui-owned predicate too. Keep the dangerous +// SVG form base64-only: URL-encoded XML is a document-shaped payload, not a +// normal inline image transport. +function _isSafeDataImageUri(ref){ + const value=String(ref||''); + return value.length<=_DATA_IMAGE_MAX_LEN + && (_DATA_IMAGE_RE.test(value)||_DATA_IMAGE_SVG_RE.test(value)); +} + +function _dataImageHtml(ref, altText){ + if(!_isSafeDataImageUri(ref)) return null; + return `${esc(altText||'image')}`; +} + +// Markdown image syntax ![alt](url) → HTML. https:// keeps the historical direct +// ; file:// and bare data:image/ URIs route through the same helpers the +// MEDIA: pipeline uses, so ![x](file:///p.png) renders the artifact card instead +// of the broken "!" anchor it used to produce, and ![x](data:image/...) stops +// dumping raw base64 text into the chat. +function _mdImageHtml(alt, url){ + if(/^data:/i.test(url)){ + const img=_dataImageHtml(url, alt); + if(img) return img; + return esc(`![${alt}](${String(url).slice(0,64)}…)`); + } + if(/^file:\/\//i.test(url)) return _inlineMediaHtmlForRef(url,undefined,alt); + return `${esc(alt)}`; +} + +function _inlineMediaHtmlForRef(ref, sessionId, altText){ if(ref==null) return ''; + // data:image/* → inline ; any other data: scheme renders as inert + // truncated text (never routed to api/media, never embedded). + if(/^data:/i.test(ref)){ + const img=_dataImageHtml(ref,altText===undefined?'image':altText); + if(img) return img; + return `${esc(String(ref).slice(0,64))}…`; + } // Keep this logic self-contained: some tests extract renderMd() alone and // execute it in node, without the top-level helper functions from ui.js. // Tests look for `new URL(ref)` / `u.pathname` / `api/media?path=` patterns, @@ -2599,13 +2644,13 @@ function _inlineMediaHtmlForRef(ref, sessionId){ const localKind=_mediaKindForName(ref); // localArtifactCard(...) if(localKind==='image'){ - const safeName=esc(ref.split('/').pop()||'image'); + const safeName=esc(altText===undefined?(ref.split('/').pop()||'image'):altText); const tt=(typeof t==='function')?t:(key=>({media_download:'Download'}[key]||key)); const dlLabel=esc(tt('media_download')); const dlSvg=''; return `${safeName}${dlSvg}`; } - if(_SVG_EXTS.test(ref)) return `${esc(typeof t==='function'?t('media_svg_label'):'svg')}`; + if(_SVG_EXTS.test(ref)) return `${esc(altText===undefined?(typeof t==='function'?t('media_svg_label'):'svg'):altText)}`; if(localKind==='audio'||localKind==='video'){ return _mediaPlayerHtml(localKind,apiUrl+'&inline=1',ref.split('/').pop()||ref); } @@ -7268,7 +7313,7 @@ function renderMd(raw){ // backticks stays protected as a \x00C token and is never rendered as . // Must run before _code_stash restore and before _link_stash so the image // is not consumed by the [label](url) link regex. - t=t.replace(/!\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,(_,alt,url)=>`${esc(alt)}`); + t=t.replace(/!\[([^\]]*)\]\(((?:https?:\/\/|file:\/\/|data:image\/)[^\)]+)\)/g,(_,alt,url)=>(typeof _mdImageHtml==='function')?_mdImageHtml(alt,url):`${esc(alt)}`); // Stash rendered tags so autolink never matches URLs inside src= const _img_stash=[]; t=t.replace(/(]*>)/g,m=>{_img_stash.push(m);return `\x00G${_img_stash.length-1}\x00`;}); @@ -7410,7 +7455,7 @@ function renderMd(raw){ // #487: Outer image pass — handles ![alt](url) in plain paragraphs (outside tables/lists). // Runs AFTER the table pass (images in table cells are handled by inlineMd() above). // Runs BEFORE the outer [label](url) link pass so the image is not consumed as a plain link. - s=s.replace(/!\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,(_,alt,url)=>`${esc(alt)}`); + s=s.replace(/!\[([^\]]*)\]\(((?:https?:\/\/|file:\/\/|data:image\/)[^\)]+)\)/g,(_,alt,url)=>(typeof _mdImageHtml==='function')?_mdImageHtml(alt,url):`${esc(alt)}`); // Outer link pass for labeled links in plain paragraphs (outside table cells). // Runs AFTER the table pass so table cells are processed by inlineMd() only. // Stash existing tags first to avoid re-linking already-linked URLs. @@ -7500,7 +7545,11 @@ function renderMd(raw){ const raw=_safeAttrValue(v); const compact=raw.replace(/[\u0000-\u001f\u007f\s]+/g,'').toLowerCase(); if(!compact) return false; - if(/^(javascript|data|vbscript):/i.test(compact)) return false; + // data:image/* is permitted for only, validated by the shared strict + // predicate. Every other + // data: scheme stays blocked for both anchors and images. + if(/^data:/i.test(compact)) return !!(img && typeof _isSafeDataImageUri==='function' && _isSafeDataImageUri(raw)); + if(/^(javascript|vbscript):/i.test(compact)) return false; if(/^https?:\/\//i.test(raw)) return true; if(/^(mailto:|tel:|message:)/i.test(raw)) return true; if(img && /^api\//i.test(raw)) return true; diff --git a/tests/test_data_uri_images.py b/tests/test_data_uri_images.py new file mode 100644 index 00000000..7e19509a --- /dev/null +++ b/tests/test_data_uri_images.py @@ -0,0 +1,183 @@ +"""data:image URI rendering in chat markdown (base64 images must render, not dump text). + +Covers the renderer fixes for model-emitted base64 images: + - ![alt](data:image/...;base64,...) -> inline (was: raw base64 text) + - -> inline (was: swallowed entirely) + - MEDIA:data:image/... -> inline (was: broken api/media link) + - ![alt](file:///x.png) -> media (was: "!" anchor bug) +Safety invariants: + - data:text/html and every non-image data: scheme stays inert text / blocked + - oversized data:image URIs (> _DATA_IMAGE_MAX_LEN) degrade to truncated text + - event-handler attributes on raw are stripped +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + +PNG_B64 = ( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQ" + "GAhKmMIQAAAABJRU5ErkJggg==" +) +PNG_URI = f"data:image/png;base64,{PNG_B64}" + +_DRIVER_SRC = r""" +const fs = require('fs'); +const src = fs.readFileSync(process.argv[2], 'utf8'); +global.window = {}; +global.document = { createElement: () => ({ innerHTML: '', textContent: '' }), baseURI: 'http://127.0.0.1:8787/' }; +const esc = s => String(s ?? '').replace(/[&<>"']/g, c => ( + {'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); +const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i; +const _SVG_EXTS=/\.svg$/i; +const _AUDIO_EXTS=/\.(mp3|ogg|wav|m4a|aac|flac|wma|opus|webm)$/i; +const _VIDEO_EXTS=/\.(mp4|webm|mkv|mov|avi|ogv|m4v)$/i; +const _PDF_EXTS=/\.pdf$/i; +const _HTML_EXTS=/\.html?$/i; +const _CSV_EXTS=/\.(csv|tsv)$/i; +const _EXCALIDRAW_EXTS=/\.excalidraw$/i; +const _mediaKindForName=(name='')=>{ + const clean=String(name||'').split('?')[0].toLowerCase(); + if(_AUDIO_EXTS.test(clean)) return 'audio'; + if(_VIDEO_EXTS.test(clean)) return 'video'; + if(_IMAGE_EXTS.test(clean)) return 'image'; + return ''; +}; +const _mediaPlayerHtml=(k,s,n)=>`<${k} src="${esc(s)}">`; +const t = k => k; +const S = {}; + +// The data-image consts and predicate are production code. Extract/eval them +// in source order so the renderer functions run against the REAL policy, not +// a test stand-in. +for (const name of ['_DATA_IMAGE_RE', '_DATA_IMAGE_SVG_RE', '_DATA_IMAGE_MAX_LEN']) { + const m = src.match(new RegExp('const ' + name + '=([^\\n]*);')); + if (!m) throw new Error(name + ' const not found in ui.js'); + globalThis[name] = eval('(' + m[1] + ')'); +} + +function extractFunc(name) { + const re = new RegExp('function\\s+' + name + '\\s*\\('); + const start = src.search(re); + if (start < 0) throw new Error(name + ' not found'); + let i = src.indexOf('{', start); + let depth = 1; i++; + while (depth > 0 && i < src.length) { + if (src[i] === '{') depth++; + else if (src[i] === '}') depth--; + i++; + } + return src.slice(start, i); +} +eval(extractFunc('_isSafeDataImageUri')); +eval(extractFunc('_dataImageHtml')); +eval(extractFunc('_mdImageHtml')); +eval(extractFunc('_inlineMediaHtmlForRef')); +eval(extractFunc('_matchBacktickFenceLine')); +eval(extractFunc('_isBacktickFenceClose')); +eval(extractFunc('renderMd')); + +let buf = ''; +process.stdin.on('data', c => { buf += c; }); +process.stdin.on('end', () => { process.stdout.write(renderMd(buf)); }); +""" + + +@pytest.fixture(scope="module") +def driver_path(tmp_path_factory): + path = tmp_path_factory.mktemp("data_uri_renderer") / "driver.js" + path.write_text(_DRIVER_SRC, encoding="utf-8") + return str(path) + + +def _render(driver_path: str, markdown: str) -> str: + result = subprocess.run( + [NODE, driver_path, str(REPO_ROOT / "static" / "ui.js")], + input=markdown, + capture_output=True, + text=True, + timeout=30, + ) + if result.returncode != 0: + raise RuntimeError(result.stderr) + return result.stdout + + +def test_markdown_data_image_renders_img(driver_path): + html = _render(driver_path, f"![chart]({PNG_URI})") + assert f'src="{PNG_URI}"' in html + assert 'class="msg-media-img"' in html + assert "![chart]" not in html, "base64 must not appear as raw markdown text" + + +def test_raw_img_tag_with_data_image_survives(driver_path): + html = _render(driver_path, f'') + assert f'src="{PNG_URI}"' in html, "raw must not be swallowed" + + +def test_media_token_with_data_image_renders_img(driver_path): + html = _render(driver_path, f"MEDIA:{PNG_URI}") + assert f'src="{PNG_URI}"' in html + assert "api/media?path=data" not in html, "data: URI must never route to api/media" + + +def test_markdown_file_image_renders_media_img_not_anchor_bug(driver_path): + html = _render(driver_path, "![chart](file:///tmp/chart.png)") + assert "api/media?path=%2Ftmp%2Fchart.png" in html + assert "" rendering bug must stay fixed' + + +def test_https_image_unchanged(driver_path): + html = _render(driver_path, "![capy](https://example.com/capy.png)") + assert "') + assert 'src="data:text/html' not in html + assert "') + assert "onerror" not in html + assert f'src="{PNG_URI}"' in html diff --git a/tests/test_issue6209_data_image_renderer.py b/tests/test_issue6209_data_image_renderer.py new file mode 100644 index 00000000..3651c63f --- /dev/null +++ b/tests/test_issue6209_data_image_renderer.py @@ -0,0 +1,139 @@ +"""Regression coverage for PR #6209's markdown-image safety contract. + +The settled renderer in ``ui.js`` and the streaming renderer in ``messages.js`` +must resolve data-image policy through the same ui-owned predicate. These tests +execute the real small renderer bodies under Node rather than comparing regex +source text, and pin the observable image markup/safety outcomes. +""" +from __future__ import annotations + +import json +import pathlib +import shutil +import subprocess + +import pytest + +REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent +NODE = shutil.which("node") + + +def _node_contract_cases() -> dict[str, object]: + script = r""" +const fs = require('fs'); +const ui = fs.readFileSync('static/ui.js', 'utf8'); +const messages = fs.readFileSync('static/messages.js', 'utf8'); +function fn(source, name) { + const marker = 'function ' + name + '('; + const start = source.indexOf(marker); + if (start < 0) throw new Error('missing ' + name); + const brace = source.indexOf('{', start); + let depth = 1, pos = brace + 1; + while (depth && pos < source.length) { + if (source[pos] === '{') depth++; + else if (source[pos] === '}') depth--; + pos++; + } + return source.slice(start, pos); +} +function constLine(source, name) { + const start = source.indexOf('const ' + name + '='); + if (start < 0) throw new Error('missing ' + name); + const end = source.indexOf('\n', start); + return source.slice(start, end < 0 ? source.length : end); +} +function esc(value) { + return String(value ?? '').replace(/[&<>"']/g, char => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[char])); +} +const _IMAGE_EXTS = /\.(?:png|jpe?g|gif|webp|bmp|ico|avif)$/i; +const _SVG_EXTS = /\.svg$/i; +const _PDF_EXTS = /\.pdf$/i; +const _HTML_EXTS = /\.html?$/i; +const _CSV_EXTS = /\.csv$/i; +const _EXCALIDRAW_EXTS = /\.excalidraw$/i; +function _mediaKindForName(ref) { return /\.(?:png|jpe?g|gif|webp|bmp|ico|avif)$/i.test(ref) ? 'image' : ''; } +function _mediaPlayerHtml() { return ''; } +const S = {session: {session_id: 's1'}}; +const t = undefined; +eval(constLine(ui, '_DATA_IMAGE_RE').replace(/^const /, 'var ')); +eval(constLine(ui, '_DATA_IMAGE_SVG_RE').replace(/^const /, 'var ')); +eval(constLine(ui, '_DATA_IMAGE_MAX_LEN').replace(/^const /, 'var ')); +eval(fn(ui, '_isSafeDataImageUri')); +eval(fn(ui, '_dataImageHtml')); +eval(fn(ui, '_inlineMediaHtmlForRef')); +eval(fn(ui, '_mdImageHtml')); +eval(constLine(messages, '_SMD_SAFE_URL_RE').replace(/^const /, 'var ')); +eval(constLine(messages, '_SMD_SAFE_IMG_URL_RE').replace(/^const /, 'var ')); +eval(fn(messages, '_smdImgSrcAllowed')); +function _smdLinkHref(value) { return String(value || ''); } +function _streamFadeBindCleanup() {} +globalThis.window = { + smd: { + HREF: 'href', SRC: 'src', + default_renderer: () => ({ + add_text() {}, + set_attr(data, attr, value) { data.baseCalls.push({attr, value}); }, + }), + }, +}; +eval(fn(messages, '_streamFadeRenderer')); +eval(fn(messages, '_safeSmdRenderer')); +function invoke(factory, value) { + const node = { + attributes: {}, + setAttribute(name, attrValue) { this.attributes[name] = String(attrValue); }, + classList: { add() {} }, + }; + const data = { nodes: [node], index: 0, baseCalls: [] }; + factory({}).set_attr(data, window.smd.SRC, value); + return {baseCalls: data.baseCalls, blocked: node.attributes['data-blocked-scheme'] === '1'}; +} +const encodedSvg = 'data:image/svg+xml,%3Csvg%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2Fsvg%22%3E%3C%2Fsvg%3E'; +const html = 'data:text/html;base64,PGgxPkJsb2NrZWQ8L2gxPg=='; +const raster = 'data:image/png;base64,iVBORw0KGgo='; +const fileHtml = _mdImageHtml('annotated chart', 'file:///tmp/chart.png'); +console.log(JSON.stringify({ + fileHtml, + settledEncodedSvg: _dataImageHtml(encodedSvg, 'svg') !== null, + settledRaster: _dataImageHtml(raster, 'chart') !== null, + fade: { raster: invoke(_streamFadeRenderer, raster), encodedSvg: invoke(_streamFadeRenderer, encodedSvg), html: invoke(_streamFadeRenderer, html) }, + safe: { raster: invoke(_safeSmdRenderer, raster), encodedSvg: invoke(_safeSmdRenderer, encodedSvg), html: invoke(_safeSmdRenderer, html) }, +})); +""" + completed = subprocess.run( + [NODE, "-e", script], + cwd=REPO_ROOT, + capture_output=True, + check=False, + text=True, + timeout=30, + ) + assert completed.returncode == 0, completed.stderr + return json.loads(completed.stdout) + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_file_markdown_image_preserves_author_alt_text() -> None: + cases = _node_contract_cases() + assert 'alt="annotated chart"' in cases["fileHtml"] + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_url_encoded_svg_and_html_are_blocked_in_actual_stream_hooks() -> None: + cases = _node_contract_cases() + assert cases["settledEncodedSvg"] is False + for mode in ("fade", "safe"): + for blocked in ("encodedSvg", "html"): + outcome = cases[mode][blocked] + assert outcome["baseCalls"] == [] + assert outcome["blocked"] is True + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_base64_raster_reaches_base_set_attr_in_actual_stream_hooks() -> None: + cases = _node_contract_cases() + assert cases["settledRaster"] is True + for mode in ("fade", "safe"): + outcome = cases[mode]["raster"] + assert outcome["baseCalls"] == [{"attr": "src", "value": "data:image/png;base64,iVBORw0KGgo="}] + assert outcome["blocked"] is False diff --git a/tests/test_streaming_markdown.py b/tests/test_streaming_markdown.py index e5449dc6..28ef38f5 100644 --- a/tests/test_streaming_markdown.py +++ b/tests/test_streaming_markdown.py @@ -727,8 +727,8 @@ class TestSmdUrlSchemeSanitization: assert "_SMD_SAFE_URL_RE" in safefn, ( "_safeSmdRenderer set_attr must use _SMD_SAFE_URL_RE for href safety" ) - assert "_SMD_SAFE_IMG_URL_RE" in safefn, ( - "_safeSmdRenderer set_attr must use _SMD_SAFE_IMG_URL_RE for src safety" + assert "_smdImgSrcAllowed" in safefn, ( + "_safeSmdRenderer set_attr must delegate src safety to the shared data-image policy" ) assert "data-blocked-scheme" in safefn, ( "_safeSmdRenderer set_attr must set data-blocked-scheme on unsafe URLs"