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 <img>. 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.
This commit is contained in:
1 parent
26305c2ebb
commit
5758d4a827
3 files changed
+54
-13
No files matched your search
+13
-3
@@ -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)
|
||||
|
||||
+11
-10
@@ -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('<code>'+esc(c)+'</code>');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=[];
|
||||
|
||||
@@ -702,3 +702,33 @@ class TestHeadingLevelsH1ThroughH6:
|
||||
assert "<h4><strong>bold</strong> in h4</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 "<img" in out
|
||||
|
||||
def test_file_url_inside_fenced_code_stays_literal(self, driver_path):
|
||||
out = _render(driver_path, "```\nfile:///tmp/shot.png\n```")
|
||||
# Inside a code fence it must remain literal text, NOT become an <img>.
|
||||
assert "file:///tmp/shot.png" in out
|
||||
assert "<img" not in out
|
||||
assert "api/media?path=" not in out
|
||||
|
||||
def test_file_url_inside_inline_code_stays_literal(self, driver_path):
|
||||
out = _render(driver_path, "run `open file:///tmp/shot.png` now")
|
||||
assert "file:///tmp/shot.png" in out
|
||||
assert "<img" not in out
|
||||
assert "api/media?path=" not in out
|
||||
|
||||
def test_markdown_anchor_file_link_uses_link_path_not_media(self, driver_path):
|
||||
out = _render(driver_path, "[the file](file:///tmp/shot.png)")
|
||||
# Labeled anchors keep the normal link path (routed to /api/media as a link,
|
||||
# not auto-loaded as an <img>).
|
||||
assert "<img" not in out
|
||||
Reference in new issue
Block a user