diff --git a/api/media_snapshots.py b/api/media_snapshots.py index 929c1431..e676655a 100644 --- a/api/media_snapshots.py +++ b/api/media_snapshots.py @@ -504,7 +504,7 @@ def annotate_media_snapshots( resolve_ref = resolve_media_ref if allowed_predicate is None: allowed_predicate = media_capture_allowed - media_re = _re.compile(r"MEDIA:([^\s\)\]]+)") + media_re = _re.compile(r"MEDIA:([^\s\)\]\`]+)") captured = 0 for msg in messages or []: if not isinstance(msg, dict) or msg.get("role") != "assistant": diff --git a/api/routes.py b/api/routes.py index 5a6a319f..5ddcd1ac 100644 --- a/api/routes.py +++ b/api/routes.py @@ -20450,7 +20450,7 @@ def _serve_inline_html_preview(handler, target: Path, cache_control: str, *, csp return True -_MEDIA_TOKEN_RE = re.compile(r"MEDIA:([^\s\)\]]+)") +_MEDIA_TOKEN_RE = re.compile(r"MEDIA:([^\s\)\]\`]+)") def _message_content_text(content) -> str: diff --git a/static/messages.js b/static/messages.js index e7bcb73c..879dff60 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4845,7 +4845,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ function _smdMediaTailFlushEntry(entry){ const chunk=_smdMediaTailEntryChunk(entry); if(!chunk) return; - const m=/^MEDIA:([^\s\)\]]+)$/.exec(String(chunk)); + const m=/^MEDIA:([^\s\)\]\u0060]+)$/.exec(String(chunk)); const emitted=!!(m && entry && entry.parent && _smdAppendMediaNode(entry.parent, m[1])); if(!emitted && entry) _smdMediaWriteText(entry.parent, entry.data, entry.baseAddText, entry.writeText, chunk); } @@ -4893,7 +4893,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // Prose runs go through the owning text writer. MEDIA tokens go through // the single-token DOMParser helper only after a delimiter or // reliable filename suffix proves the ref is complete. - const re=/MEDIA:([^\s\)\]]+)/g; + const re=/MEDIA:([^\s\)\]\u0060]+)/g; let last=0, m; let unmatchedTail=null; while((m=re.exec(combined))){ @@ -4919,7 +4919,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // MEDIA prefix; flush any prose before the partial MEDIA suffix. const rest = combined.slice(last); if(rest){ - const tailMatch = /MEDIA:[^\s\)\]]*$/.exec(rest); + const tailMatch = /MEDIA:[^\s\)\]\u0060]*$/.exec(rest); const prefixTail = tailMatch ? '' : _smdMediaPrefixTail(rest); const tailValue = tailMatch ? tailMatch[0] : prefixTail; if(tailValue && rest.length < _MEDIA_TAIL_MAX){ diff --git a/static/ui.js b/static/ui.js index 37742c5a..c58ed7ed 100644 --- a/static/ui.js +++ b/static/ui.js @@ -7692,7 +7692,7 @@ function renderMd(raw){ // generated images) and replace them with inline or download links. // Stashed so the path/URL is never processed as markdown. const media_stash=[]; - s=s.replace(/MEDIA:([^\s\)\]]+)/g,(_,raw_ref)=>{ + s=s.replace(/MEDIA:([^\s\)\]\`]+)/g,(_,raw_ref)=>{ media_stash.push(raw_ref); return '\x00D'+(media_stash.length-1)+'\x00'; }); @@ -9139,7 +9139,7 @@ function _stripForTTS(text){ // Strip links, keep text text=text.replace(/\[([^\]]+)\]\([^)]+\)/g,'$1'); // Replace MEDIA: paths with a simple label - text=text.replace(/MEDIA:[^\s]+/g,'a file'); + text=text.replace(/MEDIA:[^\s\)\]\`]+/g,'a file'); // Strip emoji and emoticons text=text.replace(/[\u{1F600}-\u{1F64F}\u{1F300}-\u{1F5FF}\u{1F680}-\u{1F6FF}\u{1F1E0}-\u{1F1FF}\u{2600}-\u{26FF}\u{2700}-\u{27BF}\u{FE00}-\u{FE0F}\u{1F900}-\u{1F9FF}\u{1FA00}-\u{1FA6F}\u{1FA70}-\u{1FAFF}\u{200D}]/gu,''); // Strip HTML tags that may leak through markdown diff --git a/tests/test_issue7359_media_backtick_strip.py b/tests/test_issue7359_media_backtick_strip.py new file mode 100644 index 00000000..8fd33fab --- /dev/null +++ b/tests/test_issue7359_media_backtick_strip.py @@ -0,0 +1,214 @@ +"""Regression tests for issue #7359 — MEDIA: token regex must not capture trailing backtick. + +When an assistant emits a MEDIA token wrapped in markdown inline code +(`` `MEDIA:/path/file.zip` ``), every capture regex in the frontend +linkifier, preview flattener, streaming finalizers and backend allowlists +greedily included the closing backtick in the captured path. The +generated URL/allowlist entry ended with a stray backtick, so the file +download link broke and the backend media allowlist rejected the +legitimate path. + +The fix adds `` ` `` to the excluded character class on all seven capture +sites: + + - ``static/ui.js:7695`` linkifier + - ``static/ui.js:9142`` preview flattener + - ``static/messages.js:4848`` streaming chunk finalize + - ``static/messages.js:4896`` streaming combined walk + - ``static/messages.js:4922`` streaming partial tail + - ``api/routes.py:_MEDIA_TOKEN_RE`` backend session allowlist + - ``api/media_snapshots.py:media_re`` settle-time snapshot capture + +These tests verify the source shape on every site (so a site silently +reverting back to the old class fails the suite) and exercise the two +Python capture sites directly. The JS regexes are sibling enough to the +Python ones that a backtick-safe Python regex plus a source-shape +assertion is sufficient: if the JS regexes are not backtick-safe the +end-to-end repro is a known one-line Markdown payload, and a follow-up +node-driver test can be added when the maintainer wants it. +""" +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] + +# Source-shape: each site's expected backtick-safe regex literal. Listed +# alongside the file so a missing/regressed site shows up as a clear diff. +JS_SITES: dict[str, list[str]] = { + # path -> list of expected backtick-safe regex literals + "static/ui.js": [ + r"/MEDIA:([^\s\)\]\`]+)/g", # linkifier + r"/MEDIA:[^\s\)\]\`]+/g", # preview flattener (single, no group) + ], + "static/messages.js": [ + r"/^MEDIA:([^\s\)\]\u0060]+)$/", # streaming chunk finalize + r"/MEDIA:([^\s\)\]\u0060]+)/g", # streaming combined walk + r"/MEDIA:[^\s\)\]\u0060]*$/", # streaming partial tail + ], +} + +PY_SITES: dict[str, str] = { + "api/routes.py": r"MEDIA:([^\s\)\]\`]+)", + "api/media_snapshots.py": r"MEDIA:([^\s\)\]\`]+)", +} + + +def _read(rel: str) -> str: + return (REPO_ROOT / rel).read_text(encoding="utf-8") + + +# --------------------------------------------------------------------------- +# Source-shape: every site must carry the backtick-safe class +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("relpath", "needle"), + [ + (relpath, needle) + for relpath, needles in JS_SITES.items() + for needle in needles + ], + ids=lambda v: v if isinstance(v, str) else "/".join(v), +) +def test_js_site_excludes_backtick(relpath: str, needle: str) -> None: + """Each JS capture site has `` ` `` inside its character class.""" + text = _read(relpath) + assert needle in text, ( + f"{relpath} is missing the backtick-safe capture literal: {needle!r}" + ) + + +@pytest.mark.parametrize( + ("relpath", "needle"), list(PY_SITES.items()), ids=lambda v: v +) +def test_py_site_excludes_backtick(relpath: str, needle: str) -> None: + """Each Python capture site has `` ` `` inside its character class.""" + text = _read(relpath) + assert needle in text, ( + f"{relpath} is missing the backtick-safe capture literal: {needle!r}" + ) + + +def test_all_seven_sites_covered() -> None: + """Guard against silently adding/removing capture sites.""" + expected = 2 + 3 + 2 # ui.js 2 + messages.js 3 + py 2 + actual = sum(len(v) for v in JS_SITES.values()) + len(PY_SITES) + assert actual == expected, f"site count drifted: {actual} != {expected}" + + +# --------------------------------------------------------------------------- +# Python behavioural: compile the actual regexes and exercise them +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def _media_token_re() -> re.Pattern[str]: + """Compile the live ``_MEDIA_TOKEN_RE`` from api/routes.py.""" + routes_text = _read("api/routes.py") + m = re.search( + r'_MEDIA_TOKEN_RE = re\.compile\(r"([^"]+)"\)', routes_text, re.M + ) + assert m, "_MEDIA_TOKEN_RE compile() not found in api/routes.py" + return re.compile(m.group(1)) + + +@pytest.fixture(scope="module") +def _media_snap_re() -> re.Pattern[str]: + """Compile the live ``media_re`` from api/media_snapshots.py.""" + snap_text = _read("api/media_snapshots.py") + m = re.search( + r'media_re = _re\.compile\(r"([^"]+)"\)', snap_text, re.M + ) + assert m, "media_re compile() not found in api/media_snapshots.py" + return re.compile(m.group(1)) + + +def test_backend_allowlist_strips_backtick(_media_token_re: re.Pattern[str]) -> None: + """``_MEDIA_TOKEN_RE`` must not include the closing backtick in the path.""" + # Inline-code wrapper: backtick before MEDIA:, backtick after the path. + captured = _media_token_re.findall("get `MEDIA:/home/kim/skills.zip` now") + assert captured == ["/home/kim/skills.zip"], captured + assert all(not c.endswith("`") for c in captured) + + +def test_backend_allowlist_bare_token_still_captures( + _media_token_re: re.Pattern[str], +) -> None: + """Backward-compat: bare ``MEDIA:/path`` (no backtick) still matches.""" + captured = _media_token_re.findall( + "MEDIA:/home/kim/a.zip and MEDIA:/home/kim/b.tar" + ) + assert captured == ["/home/kim/a.zip", "/home/kim/b.tar"], captured + + +def test_backend_allowlist_rejects_trailing_paren( + _media_token_re: re.Pattern[str], +) -> None: + """Pre-existing behaviour: ``)`` is still excluded.""" + # Trailing ')' is part of markdown link syntax and must NOT be captured. + captured = _media_token_re.findall("see [file](MEDIA:/home/kim/a.zip) here") + # Either nothing (paren excluded) or the path without ')'. + assert all(not c.endswith(")") for c in captured), captured + + +def test_snapshot_capture_strips_backtick(_media_snap_re: re.Pattern[str]) -> None: + """``media_re`` must not include the closing backtick in the path.""" + captured = _media_snap_re.findall( + "assistant: `MEDIA:/home/kim/shot.png` and MEDIA:/home/kim/a.zip" + ) + assert captured == ["/home/kim/shot.png", "/home/kim/a.zip"], captured + assert all(not c.endswith("`") for c in captured) + + +def test_snapshot_capture_bare_token_still_captures( + _media_snap_re: re.Pattern[str], +) -> None: + """Backward-compat: bare ``MEDIA:/path`` still matches.""" + captured = _media_snap_re.findall( + "MEDIA:/home/kim/a.zip and MEDIA:/home/kim/b.zip" + ) + assert captured == ["/home/kim/a.zip", "/home/kim/b.zip"], captured + + +# --------------------------------------------------------------------------- +# Revert-sensitivity: the pre-fix class really would have captured backtick +# --------------------------------------------------------------------------- + + +def test_pre_fix_class_would_have_captured_backtick() -> None: + """The buggy (pre-fix) class greedily included the trailing backtick.""" + buggy = re.compile(r"MEDIA:([^\s\)\]]+)") + assert buggy.findall("`MEDIA:/home/kim/a.zip`") == ["/home/kim/a.zip`"] + + +def test_post_fix_class_drops_backtick() -> None: + """The fixed class strips the closing backtick, as the source does.""" + fixed = re.compile(r"MEDIA:([^\s\)\]\`]+)") + assert fixed.findall("`MEDIA:/home/kim/a.zip`") == ["/home/kim/a.zip"] + + +# --------------------------------------------------------------------------- +# node --check (so a JS syntax error in the modified source fails the suite) +# --------------------------------------------------------------------------- + + +import subprocess # noqa: E402 (kept near the helper that uses it) + + +@pytest.mark.parametrize("relpath", sorted(JS_SITES.keys())) +def test_js_file_parses(relpath: str) -> None: + """``node --check`` on every modified JS file.""" + proc = subprocess.run( + ["node", "--check", str(REPO_ROOT / relpath)], + capture_output=True, + text=True, + timeout=30, + ) + assert proc.returncode == 0, ( + f"{relpath} failed `node --check`:\n{proc.stderr}" + )