fix: authorize session html media artifacts
This commit is contained in:
santastabber
nesquena-hermes
parent
d9c4bc6d76
commit
178db13512
4 changed files with 159 additions and 25 deletions
|
|
@ -15111,13 +15111,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()
|
||||
|
|
@ -15152,6 +15152,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:
|
||||
|
|
@ -15166,7 +15171,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
|
||||
|
|
@ -15240,10 +15245,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 ────────────
|
||||
|
|
@ -15429,12 +15441,7 @@ def _handle_media(handler, parsed):
|
|||
# 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"
|
||||
|
|
|
|||
25
static/ui.js
25
static/ui.js
|
|
@ -14808,19 +14808,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');
|
||||
|
|
@ -14859,7 +14863,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>`;
|
||||
});
|
||||
};
|
||||
|
|
@ -14879,7 +14883,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>`;
|
||||
}
|
||||
|
|
@ -14899,20 +14903,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,'&').replace(/"/g,'"').replace(/</g,'<').replace(/>/g,'>');
|
||||
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>`;
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -292,11 +292,15 @@ class TestMediaEndpointUnit(unittest.TestCase):
|
|||
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."""
|
||||
|
|
@ -638,6 +642,102 @@ class TestMediaEndpointUnit(unittest.TestCase):
|
|||
)
|
||||
)
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -180,17 +180,37 @@ class TestLoadHtmlInlineFunction:
|
|||
"""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 '&' 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 ──────────────────────────────────────
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue