jinghui/hermes-webui
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(media): two-pass MEDIA: scan (pre-pass for wrapped, bare class without backtick exclusion) (#7708 round 2)

Browse Source 
#7708 re-gate (9/22 23:47), both findings:

**Finding 1 (SILENT)**: A bare path that contains a backtick
(MEDIA:/tmp/report`final.png) is truncated at the first backtick,
so the server stops authorizing it. The round-1 fix traded one bug
for another: adding the backtick to the bare-token exclusion set
dropped the inline-code closing backtick (good) but truncated any
bare path with a backtick in the filename (bad — POSIX and Windows
both allow backticks).

**Finding 2 (SILENT)**: TTS now reads filename fragments aloud
(MEDIA:/tmp/report(1).png -> 'a file).png') because the
parenthesis-exclusion was added to a site that should not need it.
Inline code is already stripped at the top of the TTS flattener.

**Fix shape** (maintainer-suggested two-pass scan):

1. Pre-pass: `MEDIA:([^`\s]+)` -> MEDIA:$1 — strips the
   inline-code backticks off the wrapped form so the bare class
   below sees a plain MEDIA:path.
2. Bare class: MEDIA:([^\s\)\]]) (original, backtick
   removed from the exclusion set) — captures the full filename
   even when the path itself contains a backtick.
3. TTS: restore the original MEDIA:[^\s]+ (no paren/bracket
   exclusion either). Inline code is stripped at the top of the
   flattener so the closing backtick of an inline-code wrapper has
   already been eaten by the time the TTS regex scans the text.

Applied to all 7 capture sites:
- api/routes.py:20453 (_MEDIA_TOKEN_RE, findall site 20513)
- api/media_snapshots.py:507 (media_re, findall site 533)
- static/ui.js:7695 (linkifier, replace site)
- static/ui.js:9142 (TTS flattener — separate shape)
- static/messages.js:4848 (chunk finalize, exec)
- static/messages.js:4896 (combined walk, exec loop)
- static/messages.js:4922 (partial tail, exec)

**Tests** (24 total, all green):
- 11 source-shape guards: each JS/PY site must carry the bare-token
  class literal AND the wrapped-form pre-pass literal
- 4 behaviour tests against the live source: wrapped form strips
  closing backtick, bare form captures plain path, bare form with
  backtick in filename captures the full path (the new regression),
  trailing paren is still excluded
- 2 round-1 regression guards documenting the trade-off that drove
  the two-pass compose
- 1 composed two-pass test (pre-pass + bare class on 4 inputs)
- 2 node --check syntax guards
- 1 site-count guard: 5 JS + 2 PY = 7 capture sites, all guarded

**Verification**:
- pytest tests/test_issue7359_media_backtick_strip.py -v -> 24/24 pass
- python3 scripts/ruff_lint.py --diff origin/master -> 'no new
  violations on added/modified lines. OK.'
- node --check on the two modified JS files -> both OK
This commit is contained in:
happy5318 committed 2026-09-23 12:03:33 +08:00
1 parent 2330faa0e4
commit 6455aa9b2b
5 files changed
+249 -101
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

tests/test_issue7359_media_backtick_strip.py
+187 -88
View File
@@ -1,59 +1,64 @@
"""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.
Two-pass scan (#7680 re-gate, 9/22):
The fix adds `` ` `` to the excluded character class on all seven capture
sites:
- `` `MEDIA:path` `` (backtick-wrapped, inline-code form): the wrapping
backticks are stripped by a pre-pass before the bare class scans the
text. This makes the closing backtick disappear as a wrapper instead
of being captured as part of the path.
- ``MEDIA:/path...`` (bare, no surrounding backticks): the capture class
no longer lists the backtick in the exclusion set, so a filename
that legally contains a backtick (``report`final.png``) is captured
in full instead of being truncated at the first backtick.
- ``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.
The pre-fix (round-1) PR added the backtick to the bare exclusion set,
which traded one bug for another: it dropped the closing wrapper on
the inline-code form but truncated any bare path that happened to
contain a backtick in the filename. The two-pass scan keeps the
original permissive bare class and adds an explicit pre-pass for the
wrapped form, so both cases round-trip correctly.
"""
from __future__ import annotations
import re
import subprocess
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.
# Source-shape: each site's expected bare-token capture literal. The
# bare class no longer lists the backtick in the exclusion set; a
# backtick inside the filename (legal on POSIX and Windows) is
# captured in full. The wrapped-form pre-pass below strips the
# inline-code backticks before the bare scan.
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)
r"/MEDIA:([^\s\)\]]+)/g", # linkifier
r"/MEDIA:[^\s]+/g", # TTS flattener (no paren/bracket exclusion)
],
"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
r"/^MEDIA:([^\s\)\]]+)$/", # streaming chunk finalize
r"/MEDIA:([^\s\)\]]+)/g", # streaming combined walk
r"/MEDIA:[^\s\)\]]*$/", # streaming partial tail
],
}
PY_SITES: dict[str, str] = {
"api/routes.py": r"MEDIA:([^\s\)\]\`]+)",
"api/media_snapshots.py": r"MEDIA:([^\s\)\]\`]+)",
"api/routes.py": r"MEDIA:([^\s\)\]]+)",
"api/media_snapshots.py": r"MEDIA:([^\s\)\]]+)",
}
# #7680 re-gate (9/22): every site that scans bare MEDIA: tokens also
# runs the wrapped-form pre-pass (`` `MEDIA:path` `` → ``MEDIA:path``)
# so the closing backtick of an inline-code form is consumed before the
# bare scan.
JS_BACKTICK_PREPASS: dict[str, str] = {
"static/ui.js": r"/`MEDIA:([^`\s]+)`/g",
"static/messages.js": r"/`MEDIA:([^`\s]+)`/g",
}
PY_BACKTICK_PREPASS: dict[str, str] = {
"api/routes.py": r"`MEDIA:([^`\s]+)`",
"api/media_snapshots.py": r"`MEDIA:([^`\s]+)`",
}
@@ -62,7 +67,7 @@ def _read(rel: str) -> str:
# ---------------------------------------------------------------------------
# Source-shape: every site must carry the backtick-safe class
# Source-shape: every site must carry the bare-token class + the pre-pass
# ---------------------------------------------------------------------------
@@ -73,72 +78,114 @@ def _read(rel: str) -> str:
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}"
def test_js_site_carries_bare_token_class(relpath: str, needle: str) -> None:
"""Every JS capture site carries the bare-token class literal."""
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the bare-token capture literal: {needle!r}"
)
@pytest.mark.parametrize(
("relpath", "needle"), list(PY_SITES.items()), ids=lambda v: v
("relpath", "needle"),
[
(relpath, JS_BACKTICK_PREPASS[relpath])
for relpath in JS_BACKTICK_PREPASS
],
)
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_js_site_carries_wrapped_prepass(relpath: str, needle: str) -> None:
"""#7680 re-gate: every JS site that scans bare MEDIA: tokens also
runs the wrapped-form pre-pass so the inline-code closing backtick
is eaten before the bare scan."""
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the wrapped-form pre-pass literal: {needle!r}"
)
@pytest.mark.parametrize(
("relpath", "needle"),
[(relpath, PY_SITES[relpath]) for relpath in PY_SITES],
)
def test_py_site_carries_bare_token_class(relpath: str, needle: str) -> None:
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the bare-token capture literal: {needle!r}"
)
@pytest.mark.parametrize(
("relpath", "needle"),
[(relpath, PY_BACKTICK_PREPASS[relpath]) for relpath in PY_BACKTICK_PREPASS],
)
def test_py_site_carries_wrapped_prepass(relpath: str, needle: str) -> None:
src = _read(relpath)
assert needle in src, (
f"{relpath} is missing the wrapped-form pre-pass 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
# ---------------------------------------------------------------------------
"""Pin: 5 JS sites + 2 Python sites = 7 capture sites, all guarded."""
js_total = sum(len(v) for v in JS_SITES.values())
assert js_total == 5, (
f"expected 5 JS capture sites, got {js_total}; this count is what the "
"review's '7 capture sites' was counting (1 linkifier + 1 TTS + 3 messages.js)"
)
assert len(PY_SITES) == 2
@pytest.fixture(scope="module")
def _media_token_re() -> re.Pattern[str]:
"""Compile the live ``_MEDIA_TOKEN_RE`` from api/routes.py."""
"""Compile the live ``_MEDIA_TOKEN_RE`` from api/routes.py (plus the
pre-pass) so behaviour tests exercise the real source."""
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))
bare = re.compile(m.group(1))
pre = re.compile(r"`MEDIA:([^`\s]+)`")
return _TwoPass(bare, pre)
@pytest.fixture(scope="module")
def _media_snap_re() -> re.Pattern[str]:
"""Compile the live ``media_re`` from api/media_snapshots.py."""
"""Compile the live ``media_re`` from api/media_snapshots.py (plus the
pre-pass) so behaviour tests exercise the real source."""
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))
bare = re.compile(m.group(1))
pre = re.compile(r"`MEDIA:([^`\s]+)`")
return _TwoPass(bare, pre)
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.
class _TwoPass:
"""Compose the wrapped-form pre-pass with the bare-token class so the
behaviour tests below exercise the live source exactly as it runs."""
def __init__(self, bare: re.Pattern[str], prepass: re.Pattern[str]):
self._bare = bare
self._pre = prepass
def findall(self, text: str) -> list[str]:
text = self._pre.sub(lambda m: f"MEDIA:{m.group(1)}", text)
return self._bare.findall(text)
def test_backend_allowlist_strips_backtick(_media_token_re) -> None:
"""``_MEDIA_TOKEN_RE`` (with the pre-pass) must not include the closing
backtick in the captured 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:
def test_backend_allowlist_bare_token_still_captures(_media_token_re) -> 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"
@@ -146,18 +193,27 @@ def test_backend_allowlist_bare_token_still_captures(
assert captured == ["/home/kim/a.zip", "/home/kim/b.tar"], captured
def test_backend_allowlist_rejects_trailing_paren(
_media_token_re: re.Pattern[str],
def test_backend_allowlist_bare_path_with_backtick_in_filename(
_media_token_re,
) -> None:
"""#7680 finding 1: a bare ``MEDIA:/tmp/report`final.png`` (backtick
inside the filename, no surrounding wrapper) must capture the full
path. POSIX and Windows both allow backticks in filenames, so
truncating at the first backtick silently bricks the request."""
captured = _media_token_re.findall("see MEDIA:/tmp/report`final.png now")
assert captured == ["/tmp/report`final.png"], captured
assert not any(c.endswith("`") and c != "/tmp/report`final.png" for c in captured), captured
def test_backend_allowlist_rejects_trailing_paren(_media_token_re) -> 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."""
def test_snapshot_capture_strips_backtick(_media_snap_re) -> None:
"""``media_re`` (with the pre-pass) must not include the closing
backtick in the captured path."""
captured = _media_snap_re.findall(
"assistant: `MEDIA:/home/kim/shot.png` and MEDIA:/home/kim/a.zip"
)
@@ -165,9 +221,18 @@ def test_snapshot_capture_strips_backtick(_media_snap_re: re.Pattern[str]) -> No
assert all(not c.endswith("`") for c in captured)
def test_snapshot_capture_bare_token_still_captures(
_media_snap_re: re.Pattern[str],
def test_snapshot_capture_bare_path_with_backtick_in_filename(
_media_snap_re,
) -> None:
"""#7680 finding 1: same bare-with-backtick case for the snapshot
pipeline. Without the pre-pass + bare class change, the snapshot
store would never see this path because the auth predicate and
the capture predicate must agree on the same set of references."""
captured = _media_snap_re.findall("MEDIA:/home/kim/shot`final.png tail")
assert captured == ["/home/kim/shot`final.png"], captured
def test_snapshot_capture_bare_token_still_captures(_media_snap_re) -> None:
"""Backward-compat: bare ``MEDIA:/path`` still matches."""
captured = _media_snap_re.findall(
"MEDIA:/home/kim/a.zip and MEDIA:/home/kim/b.zip"
@@ -176,20 +241,57 @@ def test_snapshot_capture_bare_token_still_captures(
# ---------------------------------------------------------------------------
# Revert-sensitivity: the pre-fix class really would have captured backtick
# Revert-sensitivity: the pre-fix class really would have truncated
# the bare-with-backtick case, and the round-1 fix would have left the
# wrapped form broken in a different way.
# ---------------------------------------------------------------------------
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_round1_buggy_class_truncated_bare_path_with_backtick() -> None:
"""The buggy (round-1) class greedily truncated at the first backtick
in the path, so ``MEDIA:/tmp/report`final.png`` was captured as
``/tmp/report``. This is the regression we are guarding against."""
buggy = re.compile(r"MEDIA:([^\s\)\]\`]+)")
assert buggy.findall("MEDIA:/tmp/report`final.png") == ["/tmp/report"], (
"round-1 class is no longer buggy; if you are intentionally reverting "
"to it, also remove the corresponding `test_post_fix_class_keeps_"
"bare_path` and update the source-shape guard."
)
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"]
def test_round1_buggy_class_dropped_wrapped_closing_backtick() -> None:
"""The round-1 class was actually correct for the wrapped form: it
dropped the closing backtick that the round-0 class greedily
included. Document the behaviour so a future revert picks the
correct replacement."""
round1 = re.compile(r"MEDIA:([^\s\)\]\`]+)")
assert round1.findall("`MEDIA:/home/kim/a.zip`") == ["/home/kim/a.zip"]
def test_round1_two_pass_compose_keeps_bare_path_intact() -> None:
"""The full #7680 fix is the two-pass compose: pre-pass strips the
wrapped form, then the bare class (no backtick in the exclusion
set) captures the full path. Verify the composition on both
cases so a future refactor that drops either half is caught."""
pre = re.compile(r"`MEDIA:([^`\s]+)`")
bare = re.compile(r"MEDIA:([^\s\)\]]+)")
def findall_composed(text: str) -> list[str]:
text = pre.sub(lambda m: f"MEDIA:{m.group(1)}", text)
return bare.findall(text)
# bare-with-backtick: pre-pass doesn't touch it, bare captures full path
assert findall_composed("MEDIA:/tmp/report`final.png") == [
"/tmp/report`final.png"
]
# wrapped: pre-pass strips the backticks, bare captures the path
assert findall_composed("`MEDIA:/tmp/a.zip`") == ["/tmp/a.zip"]
# mixed
assert findall_composed(
"see `MEDIA:/home/kim/shot.png` and MEDIA:/tmp/r`f.png tail"
) == ["/home/kim/shot.png", "/tmp/r`f.png"]
# plain bare: unchanged
assert findall_composed("MEDIA:/home/kim/a.zip") == ["/home/kim/a.zip"]
# ---------------------------------------------------------------------------
@@ -197,9 +299,6 @@ def test_post_fix_class_drops_backtick() -> None:
# ---------------------------------------------------------------------------
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."""