Skip to content

Fix session-backed HTML media artifact previews #5157

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

Closed
santastabber wants to merge 1 commit into from
Closed

Fix session-backed HTML media artifact previews #5157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 19 additions & 12 deletions api/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15081,13 +15081,13 @@ def _message_content_text(content) -> str:
return str(content or "")


def _session_media_token_allows_image_path(sid: str, target: Path, image_mimes: set[str]) -> bool:
"""Allow exact MEDIA:image paths already present in the requested session."""
def _session_media_token_allows_path(sid: str, target: Path, allowed_mimes: set[str]) -> bool:
"""Allow exact safe MEDIA: paths already present in the requested session."""
sid = str(sid or "").strip()
if not sid:
return False
mime = MIME_MAP.get(target.suffix.lower(), "application/octet-stream")
if mime not in image_mimes:
if mime not in allowed_mimes:
return False
try:
target_resolved = target.resolve()
Expand Down Expand Up @@ -15122,6 +15122,11 @@ def _session_media_token_allows_image_path(sid: str, target: Path, image_mimes:
return False


def _session_media_token_allows_image_path(sid: str, target: Path, image_mimes: set[str]) -> bool:
"""Backward-compatible image-only wrapper for existing callers/tests."""
return _session_media_token_allows_path(sid, target, image_mimes)


def _path_is_within_root(child: Path, root: Path) -> bool:
"""Return True when ``child`` is inside ``root`` without crashing on Windows drives."""
try:
Expand All @@ -15136,7 +15141,7 @@ def _handle_media(handler, parsed):
Security:
- Path must resolve to an allowed root (hermes home, /tmp, common dirs)
- Auth-gated when auth is enabled
- Only image MIME types are served inline; all others force download
- Safe preview MIME types can render inline when requested; SVG always downloads
- SVG always served as attachment (XSS risk)
- No path traversal: resolved path must stay within an allowed root
- Additional roots can be added via MEDIA_ALLOWED_ROOTS env var
Expand Down Expand Up @@ -15210,10 +15215,17 @@ def _handle_media(handler, parsed):
for root in allowed_roots
if root.exists()
)
session_media_allowed = _session_media_token_allows_image_path(
_AUDIO_VIDEO_PDF_TYPES = {
"audio/mpeg", "audio/wav", "audio/x-wav", "audio/mp4", "audio/aac",
"audio/ogg", "audio/opus", "audio/flac",
"video/mp4", "video/quicktime", "video/webm", "video/ogg",
"application/pdf",
}
_SESSION_MEDIA_TOKEN_TYPES = _INLINE_IMAGE_TYPES | _AUDIO_VIDEO_PDF_TYPES | {"text/html"}
session_media_allowed = _session_media_token_allows_path(
qs.get("session_id", [""])[0],
target,
_INLINE_IMAGE_TYPES,
_SESSION_MEDIA_TOKEN_TYPES,
)

# ── #3234: hard-deny Hermes's own state + secret/config files ────────────
Expand Down Expand Up @@ -15399,12 +15411,7 @@ def _workspace_is_safe_carveout(ws):
# Only serve safe media/PDF types inline when explicitly requested. HTML is
# allowed inline only with a CSP sandbox so "open full page" can work without
# granting same-origin access to the WebUI. SVG is always a download (XSS risk).
_INLINE_PREVIEW_TYPES = _INLINE_IMAGE_TYPES | {
"audio/mpeg", "audio/wav", "audio/x-wav", "audio/mp4", "audio/aac",
"audio/ogg", "audio/opus", "audio/flac",
"video/mp4", "video/quicktime", "video/webm", "video/ogg",
"application/pdf",
}
_INLINE_PREVIEW_TYPES = _INLINE_IMAGE_TYPES | _AUDIO_VIDEO_PDF_TYPES
_DOWNLOAD_TYPES = {"image/svg+xml"} # SVG: XSS risk, force download
inline_preview = qs.get("inline", [""])[0] == "1"
html_inline_ok = inline_preview and mime == "text/html"
Expand Down
25 changes: 16 additions & 9 deletions static/ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14599,19 +14599,23 @@ function loadPdfInline(container){
el.setAttribute('data-loaded','1');
const path=el.dataset.path;
const fname=path.split('/').pop()||path;
const mediaSessionId=(typeof S!=='undefined'&&S&&S.session&&S.session.session_id)?String(S.session.session_id):'';
const publicMediaUrl='api/media?path='+encodeURIComponent(path);
const mediaUrl=publicMediaUrl+(mediaSessionId?'&session_id='+encodeURIComponent(mediaSessionId):'');
const loadPdf=(pdfjsLib)=>{
fetch('api/media?path='+encodeURIComponent(path))
fetch(mediaUrl)
.then(r=>{if(!r.ok) throw new Error(r.status); return r.arrayBuffer();})
.then(buf=>{
if(buf.byteLength>PDF_MAX_SIZE){
el.outerHTML=`<div class="pdf-preview-fallback"><a class="msg-media-link" href="api/media?path=${encodeURIComponent(path)}&download=1" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${t('pdf_too_large')}</span></div>`;
const dlUrl=publicMediaUrl+'&download=1';
el.outerHTML=`<div class="pdf-preview-fallback"><a class="msg-media-link" href="${dlUrl}" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${t('pdf_too_large')}</span></div>`;
return;
}
return pdfjsLib.getDocument({data:buf, isEvalSupported:false}).promise;
})
.then(pdf=>{
if(!pdf) return;
const dlUrl='api/media?path='+encodeURIComponent(path)+'&download=1';
const dlUrl=publicMediaUrl+'&download=1';
const total=pdf.numPages;
const pagesLabel=total>1?` · ${total} pages`:'';
const wrap=document.createElement('div');
Expand Down Expand Up @@ -14650,7 +14654,7 @@ function loadPdfInline(container){
renderPage(1);
})
.catch(()=>{
const dlUrl='api/media?path='+encodeURIComponent(path)+'&download=1';
const dlUrl=publicMediaUrl+'&download=1';
el.outerHTML=`<div class="pdf-preview-fallback"><a class="msg-media-link" href="${dlUrl}" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${t('pdf_error')}</span></div>`;
});
};
Expand All @@ -14670,7 +14674,7 @@ function loadPdfInline(container){
window.addEventListener('pdfjs-ready',()=>{ _pdfjsReady=true; loadPdf(window._pdfjsLib); },{once:true});
setTimeout(()=>{
if(!_pdfjsReady){
const dlUrl='api/media?path='+encodeURIComponent(path)+'&download=1';
const dlUrl=publicMediaUrl+'&download=1';
if(el.parentNode){
el.outerHTML=`<div class="pdf-preview-fallback"><a class="msg-media-link" href="${dlUrl}" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${t('pdf_error')}</span></div>`;
}
Expand All @@ -14690,20 +14694,23 @@ function loadHtmlInline(container){
el.setAttribute('data-loaded','1');
const path=el.dataset.path;
const fname=path.split('/').pop()||path;
fetch('api/media?path='+encodeURIComponent(path))
const mediaSessionId=(typeof S!=='undefined'&&S&&S.session&&S.session.session_id)?String(S.session.session_id):'';
const publicMediaUrl='api/media?path='+encodeURIComponent(path);
const mediaUrl=publicMediaUrl+(mediaSessionId?'&session_id='+encodeURIComponent(mediaSessionId):'');
fetch(mediaUrl)
.then(r=>{if(!r.ok) throw new Error(r.status); return r.text();})
.then(html=>{
if(html.length>HTML_MAX_SIZE){
const openUrl='api/media?path='+encodeURIComponent(path)+'&inline=1';
const openUrl=publicMediaUrl+'&inline=1';
el.outerHTML=`<div class="html-preview-fallback"><a class="msg-media-link" href="${openUrl}" target="_blank" rel="noopener">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${t('html_too_large')}</span></div>`;
return;
}
const openUrl='api/media?path='+encodeURIComponent(path)+'&inline=1';
const openUrl=publicMediaUrl+'&inline=1';
const safeHtml=html.replace(/&/g,'&amp;').replace(/"/g,'&quot;').replace(/</g,'&lt;').replace(/>/g,'&gt;');
el.outerHTML=`<div class="html-preview-wrap"><div class="html-preview-header"><span>${t('html_sandbox_label')}</span><a href="${openUrl}" target="_blank" rel="noopener" class="html-open-link">${t('html_open_full')} ↗</a></div><iframe srcdoc="${safeHtml}" sandbox="allow-scripts" class="html-preview-iframe" loading="lazy"></iframe></div>`;
})
.catch(()=>{
const dlUrl='api/media?path='+encodeURIComponent(path)+'&download=1';
const dlUrl=publicMediaUrl+'&download=1';
el.outerHTML=`<div class="html-preview-fallback"><a class="msg-media-link" href="${dlUrl}" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${t('html_error')}</span></div>`;
});
});
Expand Down
104 changes: 102 additions & 2 deletions tests/test_media_inline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,15 @@ def test_svg_forces_download(self):
self.assertIn("image/svg+xml", routes_src,
"SVG MIME type must be handled (forced download) in _handle_media")

def test_non_image_forces_download(self):
"""Non-image files should be forced to download, not served inline."""
def test_inline_preview_mime_whitelist_exists(self):
"""Only the explicit safe preview whitelist should be eligible for inline display."""
routes_src = (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8")
self.assertIn("_INLINE_IMAGE_TYPES", routes_src,
"_INLINE_IMAGE_TYPES whitelist must exist in _handle_media")
self.assertIn("_AUDIO_VIDEO_PDF_TYPES", routes_src,
"shared audio/video/PDF preview MIME whitelist must exist in _handle_media")
self.assertIn('{"text/html"}', routes_src,
"HTML must be added only to the session-token whitelist")

def test_media_allowed_roots_env_var_referenced(self):
"""Handler must reference MEDIA_ALLOWED_ROOTS for configurable roots."""
Expand Down Expand Up @@ -638,6 +642,102 @@ def test_session_media_token_rejects_non_image_path(self):
)
)

def test_session_media_token_allows_exact_html_path_when_mime_is_safe(self):
from api import routes

with tempfile.TemporaryDirectory() as tmpd:
html = pathlib.Path(tmpd) / "report.html"
html.write_text("<h1>Report</h1>", encoding="utf-8")
session = SimpleNamespace(messages=[{"role": "assistant", "content": f"MEDIA:{html}"}])
with mock.patch.object(routes, "get_session", return_value=session):
self.assertTrue(
routes._session_media_token_allows_path(
"s-media", html, {"text/html"}
)
)

def test_session_media_token_rejects_mentioned_html_when_mime_not_allowed(self):
from api import routes

with tempfile.TemporaryDirectory() as tmpd:
html = pathlib.Path(tmpd) / "report.html"
html.write_text("<h1>Report</h1>", encoding="utf-8")
session = SimpleNamespace(messages=[{"role": "assistant", "content": f"MEDIA:{html}"}])
with mock.patch.object(routes, "get_session", return_value=session):
self.assertFalse(
routes._session_media_token_allows_path(
"s-media", html, {"image/png"}
)
)

def test_session_media_token_rejects_user_authored_html_path(self):
from api import routes

with tempfile.TemporaryDirectory() as tmpd:
html = pathlib.Path(tmpd) / "report.html"
html.write_text("<h1>Report</h1>", encoding="utf-8")
session = SimpleNamespace(messages=[{"role": "user", "content": f"MEDIA:{html}"}])
with mock.patch.object(routes, "get_session", return_value=session):
self.assertFalse(
routes._session_media_token_allows_path(
"s-media", html, {"text/html"}
)
)

def test_handle_media_session_authorizes_html_artifact_outside_roots(self):
from api import routes

class _Handler:
def __init__(self):
self.status = None
self.headers = {}
self.body = b""
def send_response(self, code):
self.status = code
def send_header(self, k, v):
self.headers[k.lower()] = v
def end_headers(self):
pass
class _W:
def __init__(self, owner):
self.owner = owner
def write(self, b):
self.owner.body += b
def flush(self):
pass
@property
def wfile(self):
return self._W(self)

with tempfile.TemporaryDirectory() as home, tempfile.TemporaryDirectory() as outside:
hermes_home = pathlib.Path(home) / ".hermes"
hermes_home.mkdir(parents=True)
ws = hermes_home / "workspace"
ws.mkdir()
html = pathlib.Path(outside) / "report.html"
html.write_text("<h1>Report</h1>", encoding="utf-8")
session = SimpleNamespace(messages=[{"role": "assistant", "content": f"MEDIA:{html}"}])
with mock.patch.dict(os.environ, {"HERMES_HOME": str(hermes_home), "MEDIA_ALLOWED_ROOTS": ""}), \
mock.patch.object(routes, "get_last_workspace", lambda: str(ws)), \
mock.patch.object(routes, "get_session", return_value=session), \
mock.patch("api.auth.is_auth_enabled", lambda: False):
handler = _Handler()
routes._handle_media(
handler,
SimpleNamespace(
query=(
f"path={urllib.parse.quote(str(html.resolve()))}"
"&session_id=s-media&inline=1"
),
path="/api/media",
),
)

self.assertEqual(handler.status, 200)
self.assertIn("text/html", handler.headers.get("content-type", ""))
self.assertIn("sandbox", handler.headers.get("content-security-policy", ""))
self.assertIn(b"Report", handler.body)


# ── Integration tests: live server on TEST_PORT ───────────────────────────────
# No collection-time skip guard — conftest.py starts the server via its
Expand Down
24 changes: 22 additions & 2 deletions tests/test_pdf_html_preview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,37 @@ def test_uses_srcdoc_attribute(self):
"""Must use srcdoc (not src) for HTML content to keep it same-origin sandboxed."""
ui = _read_js('ui.js')
idx = ui.find('function loadHtmlInline')
body = ui[idx:idx + 1500]
body = ui[idx:idx + 2200]
assert 'srcdoc=' in body, 'Must use srcdoc attribute for inline HTML rendering'

def test_escapes_html_for_srcdoc(self):
"""HTML content must be escaped before embedding in srcdoc to prevent attribute injection."""
ui = _read_js('ui.js')
idx = ui.find('function loadHtmlInline')
body = ui[idx:idx + 1500]
body = ui[idx:idx + 2200]
# Must escape &, <, >, " to prevent breaking out of srcdoc attribute
assert '&amp;' in body or 'replace' in body, 'Must escape HTML entities for srcdoc'

def test_html_fetch_url_includes_session_id_for_session_media_artifacts(self):
ui = _read_js('ui.js')
idx = ui.find('function loadHtmlInline')
body = ui[idx:idx + 1200]
assert 'const mediaSessionId=' in body
assert "'&session_id='+encodeURIComponent(mediaSessionId)" in body
assert 'fetch(mediaUrl)' in body
assert "const publicMediaUrl='api/media?path='+encodeURIComponent(path);" in body
assert "const openUrl=publicMediaUrl+'&inline=1';" in body

def test_pdf_fetch_url_includes_session_id_for_session_media_artifacts(self):
ui = _read_js('ui.js')
idx = ui.find('function loadPdfInline')
body = ui[idx:idx + 1200]
assert 'const mediaSessionId=' in body
assert "'&session_id='+encodeURIComponent(mediaSessionId)" in body
assert 'fetch(mediaUrl)' in body
assert "const publicMediaUrl='api/media?path='+encodeURIComponent(path);" in body
assert "const dlUrl=publicMediaUrl+'&download=1';" in body


# ── requestAnimationFrame integration ──────────────────────────────────────

Expand Down
Loading