fix(media): two-pass MEDIA: scan (pre-pass for wrapped, bare class without backtick exclusion) (#7708 round 2)
#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:
1 parent
2330faa0e4
commit
6455aa9b2b
5 files changed
+249
-101
No files matched your search
@@ -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
|
||||
|
||||
+15
-1
@@ -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
|
||||
|
||||
+20
-9
@@ -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){
|
||||
|
||||
+18
-2
@@ -7692,7 +7692,17 @@ function renderMd(raw){
|
||||
// generated images) and replace them with inline <img> 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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in new issue
Block a user