From 6455aa9b2b7280c6f342d8f1a34ba24cc59b6a80 Mon Sep 17 00:00:00 2001 From: happy5318 Date: Wed, 23 Sep 2026 12:03:33 +0800 Subject: [PATCH] fix(media): two-pass MEDIA: scan (pre-pass for wrapped, bare class without backtick exclusion) (#7708 round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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 --- api/media_snapshots.py | 10 +- api/routes.py | 16 +- static/messages.js | 29 +- static/ui.js | 20 +- tests/test_issue7359_media_backtick_strip.py | 275 +++++++++++++------ 5 files changed, 249 insertions(+), 101 deletions(-) diff --git a/api/media_snapshots.py b/api/media_snapshots.py index e676655a..e54c34d8 100644 --- a/api/media_snapshots.py +++ b/api/media_snapshots.py @@ -504,7 +504,14 @@ 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\)\]]+)") + # #7680 re-gate (9/22): two-pass scan. First strip backtick + # wrappers (`` `MEDIA:path` `` → ``MEDIA:path``) so the bare + # class below does not consume the closing backtick as part of + # the path. Then the bare class (no backtick in the exclusion + # set) captures the full filename even when the path itself + # contains a backtick (e.g. ``report`final.png``). + backtick_re = _re.compile(r"`MEDIA:([^`\s]+)`") captured = 0 for msg in messages or []: if not isinstance(msg, dict) or msg.get("role") != "assistant": @@ -530,6 +537,7 @@ def annotate_media_snapshots( if not text_parts: continue text = "\n".join(text_parts) + text = backtick_re.sub(lambda m: f"MEDIA:{m.group(1)}", text) refs = media_re.findall(text) if not refs: continue diff --git a/api/routes.py b/api/routes.py index 5ddcd1ac..f6a90ef9 100644 --- a/api/routes.py +++ b/api/routes.py @@ -20450,7 +20450,17 @@ 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\)\]]+)") +# #7680 re-gate (9/22): two-pass scan. +# 1. `` `MEDIA:path` `` (backtick-wrapped, inline-code form) → strip +# the wrapping backticks so the bare-token pass below sees a +# plain ``MEDIA:path`` and the closing backtick is not consumed +# as part of the path. +# 2. ``MEDIA:[^\s\)\]]+`` (bare, no backtick in the exclusion +# class) so a filename that legally contains a backtick +# (``report`final.png``) is captured in full instead of being +# truncated at the first backtick. +_BACKTICK_MEDIA_RE = re.compile(r"`MEDIA:([^`\s]+)`") def _message_content_text(content) -> str: @@ -20510,6 +20520,10 @@ def _session_media_token_allows_path(sid: str, target: Path, allowed_mimes: set[ ) if "MEDIA:" not in text: continue + # #7680 re-gate: strip backtick wrappers first so the bare + # class below captures the full path even when the filename + # itself contains a backtick. + text = _BACKTICK_MEDIA_RE.sub(lambda m: f"MEDIA:{m.group(1)}", text) for ref in _MEDIA_TOKEN_RE.findall(text): if "://" in ref: continue diff --git a/static/messages.js b/static/messages.js index 879dff60..bc431aad 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4845,7 +4845,12 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ function _smdMediaTailFlushEntry(entry){ const chunk=_smdMediaTailEntryChunk(entry); if(!chunk) return; - const m=/^MEDIA:([^\s\)\]\u0060]+)$/.exec(String(chunk)); + // #7680 re-gate (9/22): strip backtick wrappers so the bare-token + // match below sees a plain ``MEDIA:path`` and the bare class + // (no backtick in the exclusion set) captures the full filename + // even when the path itself contains a backtick. + const normalized = String(chunk).replace(/`MEDIA:([^`\s]+)`/g, 'MEDIA:$1'); + const m=/^MEDIA:([^\s\)\]]+)$/.exec(normalized); 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,23 +4898,29 @@ 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\)\]\u0060]+)/g; + // #7680 re-gate (9/22): strip backtick wrappers first so the bare + // class (no backtick in the exclusion set) captures the full + // filename even when the path itself contains a backtick. + // The pre-pass replaces `` `MEDIA:path` `` with ``MEDIA:path`` + // so the wrapped form is consumed before the bare scan. + const normalized = combined.replace(/`MEDIA:([^`\s]+)`/g, 'MEDIA:$1'); + const re=/MEDIA:([^\s\)\]]+)/g; let last=0, m; let unmatchedTail=null; - while((m=re.exec(combined))){ + while((m=re.exec(normalized))){ const matchEnd = m.index + m[0].length; if(m.index>last){ - const slice = combined.slice(last, m.index); + const slice = normalized.slice(last, m.index); writeCurrent(slice); } - if(matchEnd===combined.length && !_smdMediaRefHasReliableBoundary(m[1])){ - const candidate = combined.slice(m.index); + if(matchEnd===normalized.length && !_smdMediaRefHasReliableBoundary(m[1])){ + const candidate = normalized.slice(m.index); if(candidate.length < _MEDIA_TAIL_MAX){ unmatchedTail = candidate; } else { writeCurrent(candidate); } - last = combined.length; + last = normalized.length; break; } if(!_smdAppendMediaNode(parent, m[1])) writeCurrent(m[0]); @@ -4917,9 +4928,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } // Tail buffer — hold trailing bytes that look like an unterminated // MEDIA prefix; flush any prose before the partial MEDIA suffix. - const rest = combined.slice(last); + const rest = normalized.slice(last); if(rest){ - const tailMatch = /MEDIA:[^\s\)\]\u0060]*$/.exec(rest); + const tailMatch = /MEDIA:[^\s\)\]]*$/.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 c58ed7ed..c69e1a4e 100644 --- a/static/ui.js +++ b/static/ui.js @@ -7692,7 +7692,17 @@ 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)=>{ + // #7680 re-gate (9/22): two-pass scan. + // 1. `` `MEDIA:path` `` (backtick-wrapped, inline-code form) → strip + // the wrapping backticks so the bare-token pass below sees a + // plain ``MEDIA:path`` and the closing backtick is not consumed + // as part of the path. + // 2. ``MEDIA:[^\s\)\]]+`` (bare, no backtick in the exclusion + // class) so a filename that legally contains a backtick + // (``report`final.png``) is captured in full instead of being + // truncated at the first backtick. + s=s.replace(/`MEDIA:([^`\s]+)`/g,'MEDIA:$1'); + s=s.replace(/MEDIA:([^\s\)\]]+)/g,(_,raw_ref)=>{ media_stash.push(raw_ref); return '\x00D'+(media_stash.length-1)+'\x00'; }); @@ -9139,7 +9149,13 @@ 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'); + // #7680 re-gate (9/22): TTS does not need the backtick-aware + // terminator. Inline code was already stripped at the top of this + // function (`:9137`), so a bare ``MEDIA:[^\s]+`` is correct and + // round-trips the original filename including a backtick in the + // path — the only thing the TTS ever does with the captured + // substring is throw it away. + 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 index 8fd33fab..09b976ff 100644 --- a/tests/test_issue7359_media_backtick_strip.py +++ b/tests/test_issue7359_media_backtick_strip.py @@ -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."""