From 5758d4a8272130a24fb3ecb34db0221d29c2503a Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Sun, 31 May 2026 03:17:10 +0000 Subject: [PATCH] fix(security/#3234 + #3219): case-fold deny checks + make bare file:// rewrite code-fence-aware (Codex review #5) 1. api/routes.py: case-fold /api/media deny filename + dir containment checks (os.path.normcase + casefold) so STATE.DB / Sessions/ cannot bypass the state/secret deny on case-insensitive filesystems (macOS/Windows). 2. static/ui.js: move the bare file:// media-stash pass to run AFTER fenced-block and inline-code stashing, so a file:// inside a code block / backtick span stays literal text instead of becoming an auto-loaded . The MEDIA: stash keeps its first-position precedent. Adds behavioral renderer tests (real renderMd via node) for fenced + inline code file:// staying literal, bare file:// becoming media, and anchors keeping the link path. Closes the last Codex review items for #3234. --- api/routes.py | 16 ++++++++++++--- static/ui.js | 21 ++++++++++---------- tests/test_renderer_js_behaviour.py | 30 +++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/api/routes.py b/api/routes.py index 75870e7c..79bcc19f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -8177,8 +8177,18 @@ def _handle_media(handler, parsed): ) if not _in_active_workspace: + # Case-insensitive containment + basename compare so STATE.DB / Sessions/ + # cannot bypass the deny on macOS/Windows filesystems (Codex review #3234). + def _within_ci(child, root): + try: + c = os.path.normcase(str(Path(child).resolve())) + r = os.path.normcase(str(Path(root).resolve())) + return os.path.commonpath([c, r]) == r + except (ValueError, OSError): + return False + _deny_names_ci = {n.casefold() for n in _DENY_FILENAMES} _under_hermes_root = any( - _path_is_within_root(target, _root) for _root in _hermes_roots + _within_ci(target, _root) for _root in _hermes_roots ) # State-subdir deny: any DENY_SUBDIR directly under a Hermes root, and # the STATE_DIR itself (sensitive in its entirety). @@ -8189,8 +8199,8 @@ def _handle_media(handler, parsed): for _sub in _DENY_SUBDIRS: _deny_dirs.append((_root / _sub).resolve()) _denied = ( - (_under_hermes_root and target.name in _DENY_FILENAMES) - or any(_path_is_within_root(target, d) for d in _deny_dirs) + (_under_hermes_root and target.name.casefold() in _deny_names_ci) + or any(_within_ci(target, d) for d in _deny_dirs) ) if _denied: return bad(handler, "Path not in allowed location", 403) diff --git a/static/ui.js b/static/ui.js index 94aeabe5..2fc753ec 100644 --- a/static/ui.js +++ b/static/ui.js @@ -3014,16 +3014,6 @@ function renderMd(raw){ media_stash.push(raw_ref); return '\x00D'+(media_stash.length-1)+'\x00'; }); - // Some gateway/tool surfaces still emit file:// links for local artifacts - // instead of MEDIA: tokens. Browser clients cannot open the server's - // filesystem directly, so treat bare file:// URLs as media artifacts and - // route them through /api/media. This intentionally runs only for bare URLs - // (line-start or whitespace-delimited), not inside markdown links, so normal - // [label](file://...) anchors continue to use the link path below. - s=s.replace(/(^|\s)(file:\/\/[^\s<>"')\]]+)/g,(_,lead,raw_ref)=>{ - media_stash.push(raw_ref); - return lead+'\x00D'+(media_stash.length-1)+'\x00'; - }); // ── End MEDIA stash ───────────────────────────────────────────────────────── // Pre-pass: decode HTML entities first so markdown processing works correctly. // This prevents double-escaping when LLM outputs entities like < > & @@ -3101,6 +3091,17 @@ function renderMd(raw){ return lead+'\x00P'+(_preBlock_stash.length-1)+'\x00'; }); s=s.replace(/`([^`\n]+)`/g,(_,c)=>{fence_stash.push(''+esc(c)+'');return '\x00F'+(fence_stash.length-1)+'\x00';}); + // Bare file:// artifact links → media. Some gateway/tool surfaces emit bare + // file:// links for local artifacts instead of MEDIA: tokens; browser clients + // cannot open the server filesystem directly, so route them through /api/media. + // This runs AFTER fenced-block (\x00P) and inline-code (\x00F) stashing so a + // file:// inside a code block / backtick span stays literal text (#3219/#3234). + // Only bare URLs (line-start or whitespace-delimited) are matched, so normal + // [label](file://...) markdown anchors keep the link path below. + s=s.replace(/(^|\s)(file:\/\/[^\s<>"')\]]+)/g,(_,lead,raw_ref)=>{ + media_stash.push(raw_ref); + return lead+'\x00D'+(media_stash.length-1)+'\x00'; + }); // Math stash: protect $$..$$ and $..$ from markdown processing // Runs AFTER fence_stash so backtick code spans protect their dollar-sign contents const math_stash=[]; diff --git a/tests/test_renderer_js_behaviour.py b/tests/test_renderer_js_behaviour.py index 72a33485..5c92eb7b 100644 --- a/tests/test_renderer_js_behaviour.py +++ b/tests/test_renderer_js_behaviour.py @@ -702,3 +702,33 @@ class TestHeadingLevelsH1ThroughH6: assert "

bold in h4

" in out, ( f"inline markdown inside h4 must still render: {out!r}" ) + + +class TestBareFileUrlMediaRendering: + """#3219/#3234: bare file:// artifact links render as media, but file:// + inside fenced/inline code stays literal (the new pass runs AFTER code-stash).""" + + def test_bare_file_url_becomes_media(self, driver_path): + out = _render(driver_path, "Here is the screenshot file:///tmp/shot.png done") + # Routed through /api/media as an inline image, not left as a raw path. + assert "api/media?path=" in out + assert "msg-media-img" in out or ". + assert "file:///tmp/shot.png" in out + assert "). + assert "