diff --git a/api/helpers.py b/api/helpers.py index d285cfe46c5..c31522d611a 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -1208,3 +1208,400 @@ def clear_profile_cookie(handler) -> None: cookie[cookie_name]['samesite'] = 'Lax' cookie[cookie_name]['max-age'] = '0' handler.send_header('Set-Cookie', cookie[cookie_name].OutputString()) + +# ── MEDIA: token path matching (shared) ────────────────────────────────────── +# A MEDIA path may legitimately contain spaces: +# MEDIA:/home/u/vault/Meeting Notes/2026-07-29 - SDE Focus Group.md +# A ``[^\s)\]]+`` class stops at the first space, which truncates the path. +# Frontend ui.js/messages.js used to do this (the artifact card rendered the +# wrong basename and the tail leaked into the bubble as prose); the same class +# lives in the /api/media allow-list and the public-share inliner, where a +# truncated capture silently fails to match the real on-disk path and the +# artifact becomes unviewable. +# +# Widening cannot be unbounded: greedy space tolerance would swallow trailing +# prose ("MEDIA:/tmp/a.png looks good") and glue an adjacent tag +# ("MEDIA:/a.png MEDIA:/b.png") into one invalid path. The bare form is +# therefore anchored on a file extension and tempered -- it crosses single +# spaces only while still reaching a ``.ext``, never crosses a newline, and +# carries a ``(?!MEDIA:)`` guard on each continuation token so the next token +# is never absorbed. Extension-less paths still match via the no-space +# fallback, so nothing that resolved before stops resolving. +# +# Keep this the single source of truth for MEDIA path shape on the Python side; +# it mirrors ``_mediaPathSrc()`` in static/ui.js. +_MEDIA_TOKEN_BARE = ( + r"(?!MEDIA:)[^\s)\]]+?(?:[^\S\n](?!MEDIA:)[^\s)\]]+?)*?\.[A-Za-z0-9]+" +) +# Terminal sentence punctuation belongs to the PROSE, not to the ref: +# `see MEDIA:/tmp/a.png.` is a sentence about a file, not a file named +# `a.png.`. `.`/`!`/`?` only close the token when the NEXT character ends the +# token anyway (whitespace, a closing delimiter, or end of input), so a dot that +# is genuinely inside a name still belongs to the ref — `/tmp/a.tar.gz` and +# `/tmp/v1.2/chart.png` keep matching whole. A real HTTP(S) query keeps its `?` +# and `!` because those are followed by query characters, not by a boundary. +# Mirrors ``_mediaPathSrc()`` in static/ui.js; keep the two in lockstep. +# A `.`/`!`/`?` is sentence punctuation only when a REAL delimiter follows it — +# whitespace or a closing delimiter. End-of-input deliberately does NOT count: +# during streaming the text simply stops mid-token, and treating that as a +# sentence end would capture `/tmp/a` out of `MEDIA:/tmp/a.png` on the last +# chunk, so the streamed and settled renderings of one token would disagree. +_MEDIA_TOKEN_SENTENCE_END = r"[.!?](?=[\s)\]}\"'*_,;:])" +_MEDIA_TOKEN_BOUNDARY = ( + r"(?=[\s)\]}\"'*_,;:]|" + _MEDIA_TOKEN_SENTENCE_END + r"|MEDIA:|$)" +) +# Explicit quoted forms. These win before every unquoted alternative so an +# ambiguous path (spaces, a dotted directory before a space, an internal ``)`` +# or ``]``) has one unambiguous spelling that both languages agree on. A quoted +# ref may hold any character except its own quote and a newline — a newline +# always ends a MEDIA token. Mirrors the quoted alternatives in +# ``_mediaPathSrc()`` (static/ui.js); keep the two in lockstep. +_MEDIA_TOKEN_QUOTED = r"\"[^\"\n]+\"|'[^'\n]+'" + + +def unquote_media_ref(ref: str) -> str: + """Strip one matching pair of surrounding quotes from a MEDIA: capture. + + The capture groups in :func:`media_token_pattern` keep the quotes so the + matched span covers the full token (needed to replace it in the source + text). Every consumer that turns a capture into a filesystem path must call + this first, or a quoted ref reaches ``Path()`` with a literal ``"`` in it. + + Mirrors ``_unquoteMediaRef()`` in static/ui.js. + """ + value = str(ref or "").strip() + if len(value) >= 2 and value[0] in ("\"", "'") and value[-1] == value[0]: + return value[1:-1] + return value + + +def is_external_media_url(ref: str) -> bool: + """True when an (already unquoted) MEDIA ref points at a REMOTE http(s) URL. + + Scheme comparison is case-insensitive because URI schemes are + case-insensitive (RFC 3986 §3.1), so ``HTTPS://`` is as external as + ``https://``. + + Deliberately HTTP(S)-only. ``file://`` is NOT reported here: a public share + must actively reject it (absolute and un-scoped, so it can point anywhere on + the host), and callers use this predicate to decide "leave the token alone", + which for ``file://`` would mean leaking the path into the share instead of + replacing it with a placeholder. Same for ``data:`` — the share boundary + handles those on their own terms. + + This answers ONLY "does this token carry an http(s) scheme". It is not a + trust decision: an http(s) URL can still smuggle a local target in its + path, query, or fragment. A caller publishing to an untrusted audience must + additionally consult :func:`external_media_url_hides_local_target`. + + Mirrors ``_isExternalMediaUrl()`` in static/ui.js. + """ + value = unquote_media_ref(ref) + return bool(_re.match(r"(?i)^https?://", value)) + + +# Loopback and RFC 1918 / RFC 4193 private hosts. A share snapshot is rendered +# by an anonymous browser, so a URL naming one of these resolves in the +# VIEWER's network position, not ours — and for a viewer running Hermes +# locally that is the authenticated origin itself. +_PRIVATE_HOST_RE = _re.compile( + r"""(?ix) ^ (?: + localhost + | (?:127|10) \. \d{1,3} \. \d{1,3} \. \d{1,3} + | 192 \. 168 \. \d{1,3} \. \d{1,3} + | 172 \. (?:1[6-9]|2\d|3[01]) \. \d{1,3} \. \d{1,3} + | 169 \. 254 \. \d{1,3} \. \d{1,3} + | 0 \. 0 \. 0 \. 0 + | \[? (?: ::1 | [fF][cCdD][0-9a-fA-F]{2} : .* | [fF][eE][89abAB][0-9a-fA-F] : .* ) \]? + ) $ + """ +) + +# Substrings that mean "this URL leads back to a local or authenticated +# target" once they appear in an http(s) URL's path, query, or fragment. +_LOCAL_TARGET_MARKERS = ( + "media:", # a nested MEDIA: token the share renderer would restore + "file://", # an absolute host path + "/api/media", # our own authenticated media route +) + + +def _decode_url_component_bounded(value: str, *, rounds: int = 3) -> str: + """Percent-decode ``value`` up to ``rounds`` times, stopping when stable. + + Bounded on purpose: a single decode misses ``%254d`` (``%4d`` after one + pass, ``M`` after two), and an unbounded loop is a DoS on crafted input. + Three passes covers realistic nesting with a fixed ceiling. + """ + from urllib.parse import unquote as _unquote + + current = str(value or "") + for _ in range(max(0, rounds)): + nxt = _unquote(current) + if nxt == current: + break + current = nxt + return current + + +def external_media_url_hides_local_target(ref: str) -> bool: + """True when an http(s) MEDIA ref smuggles a LOCAL or AUTHENTICATED target. + + :func:`is_external_media_url` looks only at the scheme, so it says "leave + this token alone" for a URL whose own path/query/fragment names a local + file or our authenticated media route. In a PUBLIC share that is a privacy + hole rather than a cosmetic one: the share renderer restores the preserved + token into an image URL, and shapes such as + + MEDIA:https://cdn.test/i.png?src=MEDIA:/etc/shadow.png + MEDIA:http://127.0.0.1:8080/api/media?path=/home/u/.ssh/id_rsa + + then either round-trip a host path into the published snapshot or issue a + same-origin ``/api/media`` request from the viewer's browser. + + Reported when EITHER holds: + + * the host is loopback/link-local/RFC 1918 — an anonymous viewer resolves + it in their own network position, so it is never a public asset; or + * the normalized path, query, or fragment contains a local-target marker + (a nested ``MEDIA:``, ``file://``, or our ``/api/media`` route). + + The netloc is deliberately excluded from marker matching so an ordinary + public CDN host is never rejected for its name alone. Harmless public query + strings (``?w=800&fmt=webp``) contain no marker and are preserved exactly. + + Callers treat a True result as "reject the WHOLE token", never as "rewrite + part of it" — a partial rewrite is what let the scanner resume inside a + refused token in the first place. + + Mirrors ``_externalMediaUrlHidesLocalTarget()`` in static/ui.js. + """ + from urllib.parse import urlsplit + + value = unquote_media_ref(ref) + if not _re.match(r"(?i)^https?://", value): + return False + try: + parts = urlsplit(value) + except ValueError: + # Unparseable as a URL — fail CLOSED. A share must not preserve a token + # whose shape we cannot reason about. + return True + + host = (parts.hostname or "").strip() + if host and _PRIVATE_HOST_RE.match(host): + return True + # An empty host on an http(s) URL is malformed (`http:///x`); fail closed. + if not host: + return True + + # Only the parts a renderer can turn into a nested target. netloc excluded + # so a public host name is never a marker hit. + probe = "".join((parts.path or "", "?" + parts.query if parts.query else "", + "#" + parts.fragment if parts.fragment else "")) + probe_decoded = _decode_url_component_bounded(probe).lower() + return any(marker in probe_decoded for marker in _LOCAL_TARGET_MARKERS) + + + +# ── MEDIA: token length ceiling (shared lexical contract) ──────────────────── +# The streaming JS parser buffers an unsettled MEDIA candidate in a per-parser +# tail and caps that buffer. The cap is NOT a private implementation detail of +# the streaming path: if streaming treats "buffer full" as "stream ended" and +# finalizes the token, while settled `renderMd()` re-parses the same text with +# no cap and sees ONE complete reference, the two renderings disagree — the +# streamed view splits the ref into a media node plus stray prose. +# +# So the ceiling belongs to the GRAMMAR, in both languages: a MEDIA token may +# not exceed MEDIA_TOKEN_MAX_LENGTH characters after the `MEDIA:` keyword. A +# candidate that reaches the ceiling without a real delimiter is not a token at +# all — it FAILS CLOSED and stays literal text until a genuine delimiter +# arrives. That verdict is reachable identically from a streamed prefix and +# from settled text, which is the property the streamed-vs-settled equality +# tests pin. +# +# UNIT: **UTF-16 CODE UNITS**, in both languages. The choice is deliberate and +# it is the unit that actually bounds the memory this cap exists to bound: the +# unsettled candidate lives in a JavaScript string, JavaScript strings are +# stored as UTF-16, and `String.prototype.length` counts code units. Measuring +# in code points instead would let a 4096-character astral token occupy 8192 +# code units of the buffer the cap is meant to bound. +# +# Python `len()` counts CODE POINTS, so the two languages disagreed for any +# token holding astral characters: a token of 2049 U+1F600 characters measures +# 2049 in Python and 4098 in JavaScript, so Python admitted a token JavaScript +# refused — for every astral token from 2049 through 4096 characters. +# :func:`media_token_length` converts, so both languages return the same +# verdict for ASCII, BMP, and astral input. +# +# Kept in lockstep with `_MEDIA_TAIL_MAX` in static/messages.js and +# `MEDIA_TOKEN_MAX_LENGTH` in static/ui.js. +MEDIA_TOKEN_MAX_LENGTH = 4096 + + +def media_token_length(ref: str) -> int: + """Length of *ref* in UTF-16 code units — the shared contract's unit. + + Mirrors ``String.prototype.length``: a code point above U+FFFF needs a + surrogate pair and counts 2, everything else counts 1. + + A lone surrogate counts 1, which is what JavaScript reports for one too. + Spelled as a sum rather than ``len(ref.encode("utf-16-le")) // 2`` because + that encode raises ``UnicodeEncodeError`` on a lone surrogate, and a lexical + predicate must always return a verdict. Cost is linear in the capture, which + the regex scan that produced the capture already paid. + """ + return sum(2 if ord(ch) > 0xFFFF else 1 for ch in str(ref or "")) + + +def media_token_exceeds_max_length(ref: str) -> bool: + """True when a capture is too long to be a legal MEDIA token. + + Measured on the RAW capture (quotes included), because the streaming buffer + is bounded by the raw characters it holds, not by the unquoted value, and in + UTF-16 code units, because that is the unit that buffer is stored in. + + Mirrors ``_mediaTokenExceedsMaxLength()`` in static/ui.js. + """ + return media_token_length(ref) > MEDIA_TOKEN_MAX_LENGTH + + +def media_token_pattern(extra_exclude: str = "", exclude_urls: bool = False) -> str: + """Return the MEDIA: path-capture pattern (one capture group). + + ``extra_exclude`` adds characters to the excluded set of the unquoted + alternatives (the share inliner also excludes ``>``). ``exclude_urls`` + skips ``MEDIA:http(s)://...`` so external images pass through untouched. + + The alternatives are ordered, and the order is load-bearing: + + 1. **Quoted** — the unambiguous spelling; may hold any character. + 2. **Spaced run whose FINAL space-separated word carries the extension.** + The continuation is greedy up to the last ``.ext`` on the line, so + ``/tmp/v1.2 Reports/chart.png`` resolves whole instead of stopping at + the dotted directory ``/tmp/v1.2``. It is still bounded: each + continuation word must itself be extension-free, which is what stops + ``MEDIA:/tmp/a.png looks good`` from absorbing prose and keeps two + adjacent tags separate. + 3. **No-space fallback** — legacy shape, any extension or none, so + extension-less paths that resolved before keep resolving. + + The returned capture may be quoted; callers must run it through + :func:`unquote_media_ref` before treating it as a path. + """ + # The URL guard is case-insensitive and sits inside an optional quote so a + # QUOTED external URL is skipped too. Spelling it `(?!https?://)` outside the + # capture (the previous shape) let two classes through, both of which the + # share inliner then resolved as LOCAL paths and replaced with the + # missing-media placeholder while the frontend rendered them as remote + # images: `MEDIA:"https://…"` (quote consumed before the guard could see the + # scheme) and `MEDIA:HTTPS://…` (schemes are case-insensitive per RFC 3986). + # + # Callers that need the URL rejected AFTER unquoting should also run + # `is_external_media_url()` on the unquoted capture — the guard here only + # keeps the pattern from matching in the first place. + url_guard = r"(?![\"']?(?i:https?)://)" if exclude_urls else "" + # One path character: no whitespace, and none of the delimiters that close + # a token (plus any caller-specific exclusions). + ch = r"[^\s)\]" + extra_exclude + r"]" + # FIRST character of an unquoted ref. A quote is excluded here — and only + # here — so an UNTERMINATED quoted ref cannot fall through to a bare branch. + # + # Without this, `MEDIA:"/tmp/bad.png` (no closing quote) fails the quoted + # alternative, falls through to the bare grammar, and captures the literal + # `"/tmp/bad.png` INCLUDING the leading quote. The streaming flush then + # activates that leading-quote fragment as a media node, and it reaches + # Path() with a `"` in it. A quoted ref may only activate media via the + # complete same-line quoted alternative above; anything else stays prose + # until a real delimiter, exactly as the settled parse yields. + # + # Interior quotes are still allowed (`ch` is unchanged), so a path that + # merely contains a quote keeps matching as it did. + ch_first = r"[^\s)\]\"'" + extra_exclude + r"]" + # A whole space-separated word with NO dot in it. Requiring the intermediate + # words to be dot-free is the bound: the run can cross `Reports/` and + # `Notes/` but stops dead at the first word carrying a `.ext`, so trailing + # prose after `a.png` is never absorbed. + # + # Spelled as a dot-free character class rather than a lookbehind to stay + # byte-comparable with the JS half, where `(? str: @@ -18852,6 +18855,19 @@ def _session_media_token_allows_path(sid: str, target: Path, allowed_mimes: set[ if "MEDIA:" not in text: continue for ref in _MEDIA_TOKEN_RE.findall(text): + # An over-ceiling capture is not a legal MEDIA token (shared lexical + # contract; see MEDIA_TOKEN_MAX_LENGTH in api/helpers.py). Neither + # renderer will turn it into a media node, so it must not mint an + # allow-list entry either — otherwise a single oversized token could + # authorize a path that nothing on the page can actually request. + if media_token_exceeds_max_length(ref): + continue + # A quoted ref (the unambiguous spelling for spaced paths) is + # captured WITH its quotes so the matched span covers the whole + # token; strip them before touching the filesystem, or the + # allow-list entry would carry a literal quote and never match the + # path the renderer actually requests. + ref = unquote_media_ref(ref) if "://" in ref: continue try: diff --git a/api/share_refs.py b/api/share_refs.py new file mode 100644 index 00000000000..732453d3cee --- /dev/null +++ b/api/share_refs.py @@ -0,0 +1,358 @@ +""" +Hermes Web UI -- ONE fail-closed public-reference decision for share snapshots. + +``api/shares.py`` used to apply its privacy guard only while substituting +canonical ``MEDIA:`` tokens, so every OTHER URL-bearing shape in message +content reached the public share page unexamined. The share page loads +``static/ui.js`` and ``static/share.js`` calls ``renderMd()``, and that +renderer has several sinks that turn message text into a LIVE URL: + +* ``_inlineMediaHtmlForRef()`` -- restores a stashed ``MEDIA:`` token. A local + path becomes ``api/media?path=...``; a loopback http(s) ref is retargeted at + ``document.baseURI``. +* ``_mdImageHtml()`` -- the outer AND inline ``![alt](url)`` passes. A + ``file://`` target is routed through ``_inlineMediaHtmlForRef()`` (so it too + becomes ``api/media?path=...``); an http(s) target becomes a direct + ````. +* ``_markdownHref()`` -- the outer AND inline ``[label](url)`` passes. A + ``file://`` target becomes ``api/media?path=...&inline=1``. +* the autolink pass -- a bare ``http(s)://`` run in prose becomes ````. +* ``_tag()`` -- a raw ```` survives sanitisation, + because ``_isSafeUrl()`` allows an ``api/`` relative src for images. + +Each of those makes an ANONYMOUS viewer's browser issue an authenticated +same-origin request against our own ``/api/media`` route, or round-trips a host +filesystem path into a public snapshot. So the decision cannot live inside the +``MEDIA:`` substitution: it has to cover every token the client will turn into +a URL, and it has to be taken BEFORE publication. + +Two properties this module exists to preserve: + +1. **One pass, whole tokens.** :data:`SHARE_REFERENCE_RE` is a single + alternation and every branch matches a COMPLETE parser-equivalent token. + Callers replace the whole matched span or preserve it byte-for-byte, never + part of it. ``re.sub`` then resumes AFTER the span, so the scanner can never + restart inside a token it already refused -- the bug that made an external + URL's tail rewritable when the old pattern rejected by non-match. +2. **Fail closed.** An unparseable value, an empty host, a value still + percent-decoding at the bound, or a nested absolute/relative URL start in + the path, query, or fragment all mean "placeholder the token". +""" + +from __future__ import annotations + +import re +from urllib.parse import unquote, urlsplit + +from api.helpers import ( + _LOCAL_TARGET_MARKERS, + external_media_url_hides_local_target, + media_token_pattern, +) + +# Number of percent-decode rounds a probe gets before we stop and judge it. +# A single decode misses `%254d` (`%4d` after one pass, `M` after two), and an +# unbounded loop is a DoS on crafted input. +_DECODE_ROUNDS = 3 + +_MEDIA_KEYWORD = "MEDIA:" + + +def decode_probe_bounded(value: str, *, rounds: int = _DECODE_ROUNDS) -> tuple[str, bool]: + """Percent-decode *value* up to *rounds* times; report whether it settled. + + Returns ``(decoded, still_changing)``. ``still_changing`` is ``True`` when + the value was STILL mutating when the bound was reached. + + ``api.helpers._decode_url_component_bounded`` returns only the decoded + string, so a value that never settles is indistinguishable there from one + that decoded cleanly to something harmless -- and the caller then accepts + it. At a publication boundary that is a hole, so this variant hands the + verdict back to the caller instead of hiding it. + """ + current = str(value or "") + for _ in range(max(0, rounds)): + nxt = unquote(current) + if nxt == current: + return current, False + current = nxt + return current, unquote(current) != current + + +# Nested substrings that mean "this value leads back to a local or +# authenticated target" WHATEVER else the value carries. A superset of the +# marker tuple in api/helpers.py, imported rather than restated so the two can +# never drift, plus: +# +# * ``api/media`` -- the slash-less spelling of our own route, which is exactly +# the form ``_isSafeUrl()`` accepts as an image src. The helper tuple only +# carries the rooted ``/api/media`` spelling. +# +# ``http://`` and ``https://`` are deliberately ABSENT. A second absolute URL +# inside the path, query, or fragment of the first is a CANDIDATE to classify, +# not a verdict: an outer public URL carrying a harmless nested public URL (a +# CDN or image proxy passing `?next=https://images.example.test/b.png`) is a +# genuine public asset, and refusing it for owning a scheme token both +# over-blocks and contradicts the byte-for-byte preservation contract above. +# :data:`_NESTED_CANDIDATE_RE` finds those candidates and +# :func:`public_reference_hides_local_target` recurses into each one. +# +# The netloc is deliberately NOT probed, so an ordinary public CDN host is never +# refused for its name alone. +_NESTED_LOCAL_MARKERS: tuple[str, ...] = tuple( + dict.fromkeys(_LOCAL_TARGET_MARKERS + ("api/media",)) +) + +# A nested ABSOLUTE http(s) URL inside a decoded path/query/fragment probe. The +# run is greedy up to the characters that can never appear inside a URL, so an +# ambiguous boundary (`&`, `,`) is swallowed INTO the candidate rather than +# cutting it short: a longer candidate can only carry more evidence, so the +# ambiguity resolves in the fail-closed direction. `file://` is not a candidate +# because it is already an unconditional marker above. +_NESTED_CANDIDATE_RE = re.compile(r"""(?i)https?://[^\s<>"']*""") + +# A nested SCHEME-RELATIVE candidate: `//host/path`. A browser resolves this +# against the containing document's scheme, so `?next=//127.0.0.1/x.png` reaches +# loopback exactly as `http://127.0.0.1/x.png` does. It carries no scheme token, +# so _NESTED_CANDIDATE_RE never matched it and no marker in +# _NESTED_LOCAL_MARKERS appears in it either — the value reached the preserve arm +# and published a live private reference. +# +# The leading `//` must not be preceded by a `:` (that would be the absolute form +# _NESTED_CANDIDATE_RE already owns) and the authority must look like a host: +# at least one character that is not a slash, backslash, or another delimiter. +_NESTED_SCHEME_RELATIVE_RE = re.compile(r"""(?"'/\\?#]+)([^\s<>"']*)""") + +# Characters a browser treats as a path separator even though they are not `/`. +# A target such as `\api\media?path=…` or `\\127.0.0.1\x.png` normalizes to the +# forward-slash spelling before the request goes out, so the classifier must +# normalize first or it compares the wrong string. +_BACKSLASH_RE = re.compile(r"\\") + + +def _browser_normalize(probe: str) -> str: + """Return *probe* as a browser would resolve its separators and dot segments. + + Two normalizations, both of which a real URL parser performs before the + request leaves the page, and neither of which a raw substring check sees: + + * backslashes become forward slashes, so ``\\api\\media?path=`` is compared + as ``/api/media?path=``; + * ``foo/../api/media`` collapses to ``/api/media``, so a dot segment cannot + hide an authenticated route from a prefix match. + + Collapsing happens on the path portion only. A ``..`` inside a query value is + not a path segment, and rewriting it would change the bytes a preserved + public URL is compared against. + """ + normalized = _BACKSLASH_RE.sub("/", probe) + head, sep, tail = normalized.partition("?") + if not sep: + head, sep, tail = normalized.partition("#") + segments: list[str] = [] + for segment in head.split("/"): + if segment == ".": + continue + if segment == "..": + if segments: + segments.pop() + continue + segments.append(segment) + collapsed = "/".join(segments) + return collapsed + sep + tail + + +# How many levels of nesting get classified before the value is refused. Each +# recursion step judges a STRICT substring of its parent's probe, so recursion +# terminates on the value alone; this bound is the second, explicit guarantee +# and it fails CLOSED at the limit rather than accepting an unexamined tail. +_MAX_NESTED_URL_DEPTH = 3 + +_FILE_SCHEME_RE = re.compile(r"(?i)^file://") +_HTTP_SCHEME_RE = re.compile(r"(?i)^https?://") +# Our own media route as a RELATIVE URL. A `?` is required so the words +# "api/media" in ordinary prose are not mistaken for a URL; the dangerous shape +# always carries a query (`api/media?path=...`). +_API_MEDIA_RE = re.compile(r"(?i)^/?api/media\?") + + +def public_reference_hides_local_target(value: str, *, _depth: int = 0) -> bool: + """True when *value* must NOT be published into a public share snapshot. + + Covers every shape :data:`SHARE_REFERENCE_RE` can match, and answers for + the WHOLE value across path, query, and fragment. + + Refused: + + * ``file://`` -- absolute and un-scoped, so it can name anything on the + host. Never embeddable either (``_resolve_against_roots`` rejects it), so + preserving it would only disclose a path. + * a relative ``api/media?…`` / ``/api/media?…`` -- our authenticated route. + * an http(s) URL that :func:`api.helpers.external_media_url_hides_local_target` + already refuses (private host, empty host, unparseable, or a marker in + the decoded path/query/fragment). + * an http(s) URL whose probe is still percent-decoding at the bound. + * an http(s) URL carrying a nested local marker (``file://``, a nested + ``MEDIA:``, or either spelling of our own ``/api/media`` route) anywhere + in its decoded path, query, or fragment. + * an http(s) URL carrying a nested absolute http(s) URL that this same + function refuses -- classified by recursion, so a private or + authenticated descendant is caught however deeply it is wrapped. + * anything else -- a shape we cannot reason about is refused, not accepted. + + Preserved: an ordinary public http(s) URL, including one with a harmless + query string, and including one whose query or fragment carries another + HARMLESS PUBLIC URL (the image-proxy shape). Over-blocking a genuine public + asset is also a failure, and a public host is not a local target. + + *_depth* is the recursion level. It is internal: each nested candidate is a + STRICT substring of its parent's probe, so recursion terminates on the + value alone, and :data:`_MAX_NESTED_URL_DEPTH` is the second, explicit + guarantee. At the bound the answer is True -- an unexamined nested tail is + refused, never accepted. + """ + v = str(value or "").strip() + if not v: + return True + if _FILE_SCHEME_RE.match(v) or _API_MEDIA_RE.match(v): + return True + if not _HTTP_SCHEME_RE.match(v): + # Not a shape this module classifies -- fail closed. + return True + # The host/marker half of the decision, reused verbatim so the server guard + # and its client twin in static/ui.js keep agreeing on those rows. + if external_media_url_hides_local_target(v): + return True + try: + parts = urlsplit(v) + except ValueError: + return True + probe = "".join(( + parts.path or "", + "?" + parts.query if parts.query else "", + "#" + parts.fragment if parts.fragment else "", + )) + decoded, still_changing = decode_probe_bounded(probe) + if still_changing: + return True + # Compare what a BROWSER would request, not the raw bytes. `\api\media?…` + # and `foo/../api/media?…` both resolve to the authenticated route, and a raw + # substring check sees neither. + decoded = _browser_normalize(decoded) + lowered = decoded.lower() + if any(marker in lowered for marker in _NESTED_LOCAL_MARKERS): + return True + if any( + _nested_candidate_hides_local_target(candidate, depth=_depth) + for candidate in _NESTED_CANDIDATE_RE.findall(decoded) + ): + return True + # Scheme-relative descendants: `//host/path` resolves against the containing + # scheme, so it reaches the same host an absolute URL would. Resolve each one + # with this value's scheme and classify it exactly like an absolute + # candidate, so a private or authenticated descendant is refused however it + # is spelled. + scheme = (parts.scheme or "https").lower() + return any( + _nested_candidate_hides_local_target(f"{scheme}:{candidate}", depth=_depth) + for candidate in _scheme_relative_candidates(decoded) + ) + + +def _scheme_relative_candidates(decoded: str) -> list[str]: + """Return every ``//host/...`` run in *decoded*, longest match per position. + + Each returned value keeps its leading ``//`` so a caller can prefix a scheme + and hand the result to a normal absolute-URL classifier. An authority that + parses to nothing is skipped rather than turned into a bogus URL: a bare + ``//`` carries no host and cannot reach one. + """ + out: list[str] = [] + for match in _NESTED_SCHEME_RELATIVE_RE.finditer(decoded): + authority = match.group(1) + if not authority: + continue + out.append("//" + authority + (match.group(2) or "")) + return out + + +def _nested_candidate_hides_local_target(candidate: str, *, depth: int) -> bool: + """Classify one nested absolute http(s) *candidate* found inside a probe. + + Split out so the recursion bound is checked in exactly one place. Refuses + at the bound: a candidate we did not get to examine is not a candidate we + may publish. + """ + if depth >= _MAX_NESTED_URL_DEPTH: + return True + return public_reference_hides_local_target(candidate, _depth=depth + 1) + + +# ── The tokenizer ──────────────────────────────────────────────────────────── +# One alternation over every shape the share renderer turns into a live URL. +# Each branch matches a COMPLETE token, so a caller's replace-or-preserve +# decision always covers the whole span. +# +# Character classes mirror the client regexes so the server and the renderer +# tokenize the same text the same way: +# _URL_RUN -> the autolink class in renderMd() +# _MD_IMAGE_URL -> the `[^)]+` target class of the ![alt](url) passes +# _MD_LINK_URL -> the `[^\s)]+` target class of the [label](url) passes +# `\n` is additionally excluded from the Markdown target classes: a newline +# never appears inside a real target and excluding it keeps one refusal from +# swallowing the following line. +_URL_RUN = r"[^\s<>\"')\]]+" +_MD_IMAGE_URL = r"[^)\n]+" +_MD_LINK_URL = r"[^\s)\n]+" +# Only the schemes whose Markdown target a client sink turns into a local or +# authenticated URL. `data:image/` is validated separately by +# _isSafeDataImageUri() and carries no host path; `workspace://`, `session://`, +# `mailto:`, `tel:` and `message:` produce no fetchable local file URL. +_MD_SCHEMES = r"(?i:https?://|file://)" +# Same, plus the relative spelling of our own media route, so a Markdown +# image/link whose target is `/api/media?…` is refused as ONE whole token +# instead of leaving `![x](` and `)` behind as orphaned prose. +_MD_TARGET_STARTS = r"(?i:https?://|file://|/?api/media\?)" + +SHARE_REFERENCE_PATTERN = ( + # 1. Canonical MEDIA: token. First so `MEDIA:https://…/MEDIA:/etc/x.png` + # stays ONE token instead of being re-entered at the nested keyword. + r"(?P" + media_token_pattern(extra_exclude=">") + r")" + # 2. Markdown image, outer and inline passes. + r"|(?P!\[[^\]]*\]\((?P" + _MD_TARGET_STARTS + _MD_IMAGE_URL + r")\))" + # 3. Markdown link, outer and inline passes. + r"|(?P\[[^\]]+\]\((?P" + _MD_TARGET_STARTS + _MD_LINK_URL + r")\))" + # 4. Bare absolute URL in prose (autolink sink, and path disclosure for + # file://, which no sink linkifies but which still names a host file). + r"|(?P" + _MD_SCHEMES + _URL_RUN + r")" + # 5. Relative reference to our own authenticated media route. Reachable as + # a live image src through _isSafeUrl()/_tag(). + r"|(?P/?(?i:api/media)\?" + _URL_RUN + r")" +) + +SHARE_REFERENCE_RE = re.compile(SHARE_REFERENCE_PATTERN) + + +def media_ref_of_match(match: re.Match) -> str: + """Return the raw (still quoted) ref of a ``media`` branch match. + + The ``media`` branch wraps :func:`api.helpers.media_token_pattern`, whose + own capture group is unnamed and whose index therefore depends on that + helper's internals. Slice the matched TEXT instead: it always begins with + the ``MEDIA:`` keyword and everything after it is the capture. + """ + span = match.group("media") or "" + return span[len(_MEDIA_KEYWORD):] if span.startswith(_MEDIA_KEYWORD) else span + + +def url_of_match(match: re.Match) -> str: + """Return the URL a non-``media`` branch match carries. + + For the Markdown branches that is the target inside the parentheses; for + the bare and relative branches the whole span IS the URL. + """ + for name in ("imgurl", "linkurl"): + value = match.group(name) + if value is not None: + return value + return match.group(0) diff --git a/api/shares.py b/api/shares.py index 9108695e8cc..2ea6fe8b93f 100644 --- a/api/shares.py +++ b/api/shares.py @@ -13,6 +13,7 @@ import io import json import logging +import math import mimetypes import os import re @@ -22,9 +23,21 @@ import time import xml.etree.ElementTree as ET from pathlib import Path +from typing import NamedTuple from api.config import STATE_DIR -from api.helpers import redact_session_data +from api.helpers import ( + redact_session_data, + unquote_media_ref, + is_external_media_url, + media_token_exceeds_max_length, +) +from api.share_refs import ( + SHARE_REFERENCE_RE, + media_ref_of_match, + public_reference_hides_local_target, + url_of_match, +) # _redact_fn_cached is the ALWAYS-ON credential redactor (agent redactor with # force=True + local fallback regex). Unlike redact_session_data it does NOT # consult the user-toggleable api_redact_enabled setting — a public share is a @@ -37,6 +50,17 @@ SHARES_DIR = STATE_DIR / "shares" _SHARE_LOCK = threading.Lock() +# The only roles a public share may publish. Declared once so the WRITE path +# (_sanitize_message) and the READ path (_guarded_public_message) cannot drift: +# a read that accepted a wider set than the write would let a legacy snapshot +# serve system or tool rows that a fresh snapshot can never contain. +_PUBLIC_SHARE_ROLES = frozenset({"user", "assistant"}) + +# Keys a public message row may carry. Anything else a stored snapshot holds +# (provider_details, tool_calls, raw_result, workspace, ...) is dropped rather +# than republished. +_PUBLIC_MESSAGE_KEYS = frozenset({"role", "content", "timestamp"}) + def _ensure_share_dir() -> None: SHARES_DIR.mkdir(parents=True, exist_ok=True) @@ -115,14 +139,23 @@ def _redact_share_paths(text: str, extra_paths) -> str: return text -# Regex matching local MEDIA: references — same pattern that -# _inlineMediaHtmlForRef() in ui.js handles when rendering messages. -# Excludes MEDIA: followed by http/https URLs so external images pass -# through unchanged. file:// references are NOT matched here — they are -# always rejected at the public-share boundary (absolute, un-scoped). -_SHARE_MEDIA_RE = re.compile( - r"MEDIA:(?!https?://)([^\s\)\]>]+)" -) +# Regex matching EVERY URL-bearing token the share renderer can turn into a +# live URL — the canonical `MEDIA:` token plus Markdown images/links, bare +# absolute URLs, and relative `api/media?…` references. Shape and rationale +# live in api/share_refs.py. +# +# NOTE: the `MEDIA:` branch deliberately does NOT pass exclude_urls=True. That +# option is a negative lookahead at the current start position, so an external +# URL is rejected by NOT MATCHING — and `re.sub` then resumes scanning one +# character later, inside the very token it just refused. For +# `MEDIA:https://cdn.test/img/MEDIA:/etc/passwd.png` the outer token is skipped +# and the nested `MEDIA:` matches as a fresh LOCAL token, so a legitimate +# external URL gets its tail rewritten (and local paths get probed from share +# text). Match the complete canonical token here and classify it in +# _replace_ref(): match -> classify -> decide, which cannot restart mid-token. +# Every other branch obeys the same rule, so one refusal always consumes its +# whole token. +_SHARE_MEDIA_RE = SHARE_REFERENCE_RE # Max size (in bytes) for files we'll embed as base64 in a share snapshot. _SHARE_EMBED_MAX_BYTES = 512 * 1024 # 512 KiB @@ -154,6 +187,31 @@ def _redact_share_paths(text: str, extra_paths) -> str: # Static placeholder emitted when a media reference cannot be embedded. _PLACEHOLDER = "[*Local attachment omitted from public share*]" +# ── The three outcomes of ONE reference decision ───────────────────────────── +# Named so the pure classifier and its two callers cannot disagree about what a +# verdict means. `_EMBED` is the only outcome that needs the filesystem, and it +# is reachable ONLY from the create path — the load path treats it as a refusal. +_PRESERVE = "preserve" +_REFUSE = "refuse" +_EMBED = "embed" + + +class _RefVerdict(NamedTuple): + """What to do with one matched reference. + + ``action`` is one of :data:`_PRESERVE`, :data:`_REFUSE`, or :data:`_EMBED`. + ``local_path`` carries the unquoted local ref for :data:`_EMBED` and is + empty otherwise, so an ``_EMBED`` verdict is the only shape that can name a + file at all. + """ + + action: str + local_path: str = "" + + +_PRESERVE_VERDICT = _RefVerdict(_PRESERVE) +_REFUSE_VERDICT = _RefVerdict(_REFUSE) + # Magic byte signatures for allowed image formats — content-based validation # that catches mismatched extensions (e.g. a .png that is actually a script). # SVG is excluded here because it is validated by XML parsing in @@ -236,6 +294,104 @@ def _walk(elem: ET.Element) -> None: return buf.getvalue() +def _classify_share_ref(match: re.Match) -> _RefVerdict: + """Decide ONE matched token, WITHOUT touching the filesystem. + + This is the pure half of the publication decision. It answers with + :data:`_PRESERVE` (publish the span byte-for-byte), :data:`_REFUSE` + (replace the whole span with :data:`_PLACEHOLDER`), or :data:`_EMBED` + (a canonical ``MEDIA:`` token naming a LOCAL path, which only the create + path may resolve and inline). + + Split out of :func:`_embed_share_media` so the LOAD path can reuse the + identical decision. ``load_share()`` must apply the same guard to a stored + snapshot written by an older build, but it must never read a local file to + do it: a public read of an anonymous share is not a licence to touch the + host filesystem, and the concrete path in a legacy snapshot is untrusted + input. Structural separation is stronger than calling the embedder with + empty ``allowed_roots`` — an absolute ref still reaches ``Path.resolve()`` + and ``is_file()`` there, so "no roots" is not "no filesystem access". + """ + # Non-`MEDIA:` branches: a Markdown image/link target, a bare absolute + # URL, or a relative reference to our own media route. Each of these is + # a live-URL sink on the share page (see api/share_refs.py for the sink + # inventory), so ONE fail-closed decision covers the WHOLE matched span + # here, exactly as the `MEDIA:` branch below does. Never embedded: the + # allowed-roots embed path exists for canonical `MEDIA:` tokens, and a + # Markdown target that survives is preserved byte-for-byte instead. + if match.group("media") is None: + if public_reference_hides_local_target(url_of_match(match)): + return _REFUSE_VERDICT + return _PRESERVE_VERDICT + + # Quoted refs are captured with their quotes so the replaced span covers + # the whole token; strip them before any path resolution, or a spaced + # path that the renderer displays would be placeholdered here. + capture = media_ref_of_match(match) + raw = unquote_media_ref(capture) + if not raw: + return _PRESERVE_VERDICT + # An over-ceiling capture is not a legal MEDIA token (shared lexical + # contract; see MEDIA_TOKEN_MAX_LENGTH in api/helpers.py). Neither + # renderer turns it into a media node, so the snapshot must leave the + # span exactly as prose rather than embed or placeholder it — otherwise + # the share and the live view disagree about one token. + if media_token_exceeds_max_length(capture): + return _PRESERVE_VERDICT + # Classification happens HERE, on a token that was actually matched, and + # is the only thing that decides external vs local. The pattern no longer + # rejects external URLs by non-match, so the scanner can never resume + # inside a refused token and rewrite the tail of an external URL that + # itself contains `MEDIA:`. Preserving returns the exact original span, so + # an exempt external token survives byte-for-byte. + # + # But "carries an http(s) scheme" is NOT the same as "safe to publish". + # The share renderer restores a preserved token into an image URL, so a + # URL whose own path/query/fragment names a local file, our + # authenticated /api/media route, or a loopback/private host would + # either round-trip a host path into the anonymous snapshot or make the + # viewer's browser issue a same-origin authenticated request. Decide the + # WHOLE token: preserve it byte-for-byte, or placeholder all of it. A + # partial rewrite is exactly the bug that let the scanner resume inside + # a token it had refused. + if is_external_media_url(raw): + if public_reference_hides_local_target(raw): + return _REFUSE_VERDICT + return _PRESERVE_VERDICT + return _RefVerdict(_EMBED, raw) + + +def guard_public_share_references(text: str) -> str: + """Apply the public-reference decision to *text* with NO filesystem access. + + The guard :func:`_embed_share_media` applies runs when a snapshot is + CREATED. ``load_share()`` used to return a stored snapshot's ``messages`` + untouched, so every share written before that guard existed stayed on the + vulnerable path forever: ``static/share.js`` hands each stored + ``msg.content`` straight to ``renderMd()`` and assigns the result with + ``innerHTML``, and the live sinks there (``_markdownHref()`` turning + ``file://`` into ``api/media?path=…&inline=1``; ``_isSafeUrl()``/``_tag()`` + accepting a relative ``api/`` image or link target) are still reachable. + + So the same decision runs on the way OUT. Every token + :data:`SHARE_REFERENCE_RE` matches is either preserved byte-for-byte or + replaced whole with :data:`_PLACEHOLDER`. A local ``MEDIA:`` ref is + placeholdered rather than embedded: reading it would need the filesystem, + the allowed roots of the originating session are not available at load + time, and the path itself is untrusted input from a stored file. Already + guarded snapshots pass through unchanged, because the decision is + idempotent on its own output. + """ + if not isinstance(text, str) or not text: + return text + + def _decide(match: re.Match) -> str: + verdict = _classify_share_ref(match) + return match.group(0) if verdict.action == _PRESERVE else _PLACEHOLDER + + return _SHARE_MEDIA_RE.sub(_decide, text) + + def _embed_share_media(text: str, *, allowed_roots: tuple[Path, ...] = ()) -> str: """Find local MEDIA: references and replace them with inline tags. @@ -289,11 +445,8 @@ def _resolve_against_roots(raw: str) -> Path | None: return candidate return None - def _replace_ref(m: re.Match) -> str: - raw = (m.group(1) or "").strip() - if not raw: - return m.group(0) - + def _embed_local_ref(raw: str) -> str: + """The effectful half: resolve *raw*, validate it, inline its bytes.""" # --- Resolve and validate against allowed roots ----------------------- p = _resolve_against_roots(raw) if p is None: @@ -339,6 +492,14 @@ def _replace_ref(m: re.Match) -> str: except (OSError, MemoryError): return _PLACEHOLDER + def _replace_ref(m: re.Match) -> str: + verdict = _classify_share_ref(m) + if verdict.action == _PRESERVE: + return m.group(0) + if verdict.action == _REFUSE: + return _PLACEHOLDER + return _embed_local_ref(verdict.local_path) + return _SHARE_MEDIA_RE.sub(_replace_ref, text) @@ -346,7 +507,7 @@ def _sanitize_message(message: dict, *, redact_paths=(), allowed_roots: tuple[Pa if not isinstance(message, dict): return None role = str(message.get("role") or "").strip().lower() - if role not in {"user", "assistant"}: + if role not in _PUBLIC_SHARE_ROLES: return None text = _share_message_text(message) if not text: @@ -372,14 +533,68 @@ def _sanitize_message(message: dict, *, redact_paths=(), allowed_roots: tuple[Pa return sanitized +def _guarded_public_message(message) -> dict | None: + """Rebuild *message* as a public row, or return ``None`` to drop it. + + Stored snapshot JSON is UNTRUSTED. It may predate the public-reference guard, + and it may carry fields an older writer persisted. So this REBUILDS the row + from approved keys rather than spreading the stored dict: a spread + republishes every stored field and every stored role, which let a legacy + snapshot serve ``system`` and ``tool`` rows plus siblings such as + ``provider_details``, ``tool_calls``, ``raw_result``, and workspace paths + through the anonymous ``/api/share/`` response. + ``static/share.js::_shareRenderMessages()`` renders every returned row, so a + republished field is a published field. + + The contract enforced here is the one + ``tests/test_session_public_share.py::test_public_share_payload_is_sanitized_and_read_only`` + already states for freshly built snapshots: user and assistant rows only, no + provider details. A read of an OLD snapshot must satisfy the same contract as + a write of a new one. + + Returned keys are exactly ``role``, ``content``, and an optional numeric + ``timestamp``. Content passes through :func:`guard_public_share_references`, + which performs no filesystem access. + """ + if not isinstance(message, dict): + return None + role = message.get("role") + if role not in _PUBLIC_SHARE_ROLES: + return None + content = message.get("content") + if not isinstance(content, str) or not content.strip(): + return None + public: dict = { + "role": role, + "content": guard_public_share_references(content), + } + # Only a finite number survives. A stored string or NaN would render as a + # bogus time, and callers treat this field as numeric. + ts = message.get("timestamp") + if isinstance(ts, (int, float)) and not isinstance(ts, bool) and math.isfinite(ts): + public["timestamp"] = ts + return public + + def _public_share_payload(payload: dict) -> dict: messages = payload.get("messages") if not isinstance(messages, list): messages = [] + # The stored snapshot is re-guarded on EVERY read, so a share written before + # the public-reference guard existed cannot keep serving a live local + # reference to an anonymous viewer. No filesystem access happens here — see + # guard_public_share_references(). + guarded = [m for m in map(_guarded_public_message, messages) if m is not None] + # Accept only a string title. A structured title would otherwise be + # stringified into its repr, publishing dict syntax and any nested fields. + stored_title = payload.get("title") + title = stored_title.strip() if isinstance(stored_title, str) else "" public = { - "title": str(payload.get("title") or "Untitled"), - "messages": messages, - "message_count": int(payload.get("message_count") or len(messages)), + "title": title or "Untitled", + "messages": guarded, + # RECOMPUTED, never trusted. A stored count survives row drops and would + # tell the viewer that messages are missing rather than filtered. + "message_count": len(guarded), } created_at = payload.get("created_at") updated_at = payload.get("updated_at") diff --git a/static/messages.js b/static/messages.js index cf319acc607..9a87ece06dc 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4493,11 +4493,18 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ }; renderer.add_text=(data,text)=>{ const parent=data&&data.nodes&&data.nodes[data.index]; - if(!parent||_streamFadeSkipNode(parent)){baseAddText(data,text);return;} + if(!parent){baseAddText(data,text);return;} // MEDIA-in-stream: if this chunk carries a MEDIA: token, defer to // the shared interceptor so the token becomes a real media element // instead of plain text. The fade renderer would otherwise wrap every // word in a stream-fade-word span, leaving MEDIA: paths visible. + // + // This check runs BEFORE the generic pre/code skip below. Settled + // renderMd() stashes MEDIA *before* its fence pass, so MEDIA stays active + // inside fenced and inline code; the safe renderer matches that. Skipping + // to baseAddText first made fade the only mode that rendered a MEDIA token + // inside code as literal text. Prose inside a skip node still bypasses fade + // spans, because writeFadeText falls back to _smdAppendPlainText there. const parser=parserFor(data); const hasMediaTail=!!(_SMD_MEDIA_TAIL&&parser&&_SMD_MEDIA_TAIL.has&&_SMD_MEDIA_TAIL.has(parser)); const value=String(text||''); @@ -4506,6 +4513,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ _smdMediaAwareAddText(baseAddText, parent, data, text, _SMD_MEDIA_TAIL, parser, writeFadeText); return; } + if(_streamFadeSkipNode(parent)){baseAddText(data,text);return;} const frag=document.createDocumentFragment(); const wordRe=/(\S+)(\s*)/g; const reduceMotion=_streamFadeReduceMotionEnabled(); @@ -4576,8 +4584,32 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // 3. A MEDIA prefix split across smd flushes (e.g. "MEDIA:" then // "foo.png") is buffered in a per-parser tail buffer and completed // on the next add_text call. - const _MEDIA_TAIL_MAX = 4096; // bytes; defensive cap on per-parser buffer + // MEDIA-in-stream buffering. There is exactly ONE ceiling, and it is + // _smdMediaCandidateMax() below — a candidate is `MEDIA:` plus a capture, so + // the number that bounds it is the shared capture ceiling plus the keyword. + // + // There used to be two: the matched-token branch bounded the candidate at + // _mediaTokenMaxLength() + 6 = 4102, while the no-match / open-quote tail + // branch bounded it at a separate _MEDIA_TAIL_MAX = 4096. A LEGAL quoted + // capture whose opening quote crossed a chunk boundary reaches the second + // branch — the complete quoted grammar has not matched yet — so at candidate + // length 4096 the stream flushed it as literal text and lost quote ownership. + // A later closing quote could not reassemble it, while settled renderMd() + // still accepted the same capture through 4096 characters. const _SMD_MEDIA_PREFIX = 'MEDIA:'; + /** The single inclusive candidate ceiling, in UTF-16 code units. + * + * Derived FROM the shared capture ceiling, never an independent number. The + * ceiling is measured on the CAPTURE (everything after the keyword), so a + * candidate holding a still-legal ref is the keyword plus at most + * _mediaTokenMaxLength() code units. Inclusive: a 4096-code-unit capture is + * legal, and `MEDIA:` plus 4096 is 4102. + * + * Unit: UTF-16 code units, because `.length` is what bounds the buffer this + * cap exists to bound. See _mediaTokenMaxLength() in static/ui.js. */ + function _smdMediaCandidateMax(){ + return _mediaTokenMaxLength() + _SMD_MEDIA_PREFIX.length; + } function _smdMediaPrefixTail(value){ const text=String(value||''); const max=Math.min(_SMD_MEDIA_PREFIX.length,text.length); @@ -4610,21 +4642,182 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ function _smdMediaTailEntryChunk(entry){ return entry && typeof entry==='object' && Object.prototype.hasOwnProperty.call(entry,'chunk') ? entry.chunk : entry; } + /** Park a per-parser "this reference is refused" marker. + * + * An over-limit reference must stay CLOSED, and staying closed has to survive + * a chunk boundary: its continuation arrives in the next add_text call, and + * without a marker that continuation is scanned fresh, so a `MEDIA:` sitting + * inside the refused span activates as a token of its own. Settled + * renderMd() sees one over-limit span and renders no card, so that suffix + * card is a streamed-versus-settled divergence. + * + * `quote` carries the opening quote character when the refused reference is a + * quoted one, because a quoted span ends at its closing quote rather than at a + * delimiter. Spelled as an entry with an empty chunk so it flows through the + * same owner-identity and flush machinery as a buffered candidate: + * _smdMediaTailFlushEntry writes nothing for an empty chunk, and a different + * owner clears it exactly as it clears a real tail. */ + function _smdMediaRefuseLine(tailMap, parser, parent, baseAddText, data, writeText, quote){ + if(!tailMap||!parser||!tailMap.set) return; + tailMap.set(parser, {chunk:'', refused:true, quote:quote||'', parent, baseAddText, data, writeText}); + } + /** True when *entry* is a refused-reference marker rather than a buffered tail. */ + function _smdMediaEntryRefused(entry){ + return !!(entry && typeof entry==='object' && entry.refused); + } + /** True when *ch* can continue an unquoted MEDIA run, per the REAL grammar. + * + * Asked by probing `_mediaTokenRe()` with a two-character subject rather than + * by restating the character class: the class lives in `_mediaPathSrc()` in + * static/ui.js, and a second copy here is exactly the drift the shared grammar + * exists to prevent. A capture of length 2 means the character stayed inside + * the run; length 1 means it closed the token. + * + * Note that whitespace closes the run. The spaced alternative can cross a + * space, but only toward a final word carrying an extension, and an + * already-over-limit reference can never become legal by growing — so the + * refused span ends at the same place the settled parser's refused match ends. */ + function _smdMediaRunChar(ch){ + const m=_mediaTokenRe().exec('MEDIA:a'+ch); + return !!m && m[1].length===2; + } + /** How many leading characters of *text* still belong to a refused reference. + * + * A refused reference does NOT own the whole rest of the line. It ends where + * the grammar ends any token, and an INDEPENDENT reference after that point is + * legal — settled renderMd() resumes scanning right after the refused match + * and renders that card, so the stream must too. + * + * A quoted reference ends after its closing quote, or at a newline when the + * quote never closes. An unquoted one ends at the first character that cannot + * continue its run. */ + function _smdMediaRefusedRunLength(text, quote){ + const value=String(text||''); + if(quote){ + const closing=value.indexOf(quote); + const newline=value.indexOf('\n'); + if(closing>=0 && (newline<0 || closing=2 && raw[raw.length-1]===q; + } + /** True when the text AFTER a matched MEDIA token could still be absorbed + * into that token once more bytes arrive. + * + * Checking only `matchEnd === combined.length` is not enough. The unquoted + * fallback stops at a space, so `MEDIA:/tmp/v1.2 Reports` matches + * `/tmp/v1.2` with 8 characters still to go — the token does NOT reach the + * end of the arrived text, the at-end guard never fires, and a truncated + * card is emitted even though ` Reports/chart.png` is about to land and the + * settled parse yields the whole path. + * + * So: if everything between the match end and the end of the arrived text is + * a run of spaces plus non-delimiter characters on the same line, a + * continuation is still possible and the token must keep buffering. A + * newline, or any token-closing delimiter, ends the MEDIA token for good and + * makes the match final. */ + function _smdMediaTailCouldExtend(trailing){ + const rest=String(trailing||''); + if(!rest) return false; + // A newline or a closing delimiter terminates the token definitively. + // The closing brace is spelled \x7d because the test harnesses extract + // production functions by counting brace depth and do not skip regex + // literals — a literal closing brace here truncates the extraction. + return /^(?:[^\S\n]+[^\s)\]\x7d"'*_,;:]*)+$/.test(rest); + } + /** True when the arrived text opens a quoted MEDIA ref that has not been + * closed yet on the same line. + * + * An unterminated quote must keep buffering as a unit: the quoted + * alternative cannot match, so the grammar falls through to the unquoted + * branch and captures a truncated `"/tmp/My` — a leading-quote fragment that + * the settled parse never produces. Bounded to the current line because a + * newline always ends a MEDIA token. */ + function _smdMediaHasOpenQuote(text){ + return _smdMediaOpenQuoteChar(text) !== ''; + } + /** The opening quote character of an unclosed quoted MEDIA ref, or ''. + * + * Same measurement _smdMediaHasOpenQuote() reports as a boolean, kept in one + * place: the refusal path needs the character itself, because a refused quoted + * span ends at its closing quote rather than at a delimiter. */ + function _smdMediaOpenQuoteChar(text){ + const m=/MEDIA:(["'])([^\n]*)$/.exec(String(text||'')); + if(!m) return ''; + return m[2].indexOf(m[1])===-1 ? m[1] : ''; } function _smdMediaTailFlushEntry(entry){ const chunk=_smdMediaTailEntryChunk(entry); if(!chunk) return; - const m=/^MEDIA:([^\s\)\]]+)$/.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); + // Stream end: the buffered candidate is final, so decide it here. + // + // The candidate is NOT necessarily just a token. `_smdMediaAwareAddText` + // buffers from the MEDIA keyword to end-of-line whenever the same-line + // suffix could still extend the token, so at flush time the candidate can be + // `MEDIA:/tmp/a.png and after` — a complete token PLUS trailing prose. + // + // Matching the whole candidate with the anchored ^...$ matcher fails on that + // shape, and the old fallback wrote the entire string as literal prose: the + // media card vanished and the raw `MEDIA:` keyword was shown to the user, + // while settled `renderMd()` rendered the card and kept ` and after`. + // + // So PARTITION instead: walk every shared-grammar match in the whole + // candidate, emit each normalized capture, and hand every exact prose slice + // (including the trailing suffix) to the owning text writer. If one media + // append fails, preserve that token's raw span and continue so a later valid + // token is still handled. Byte-preserving by construction: the emitted spans + // plus written slices reconstruct the candidate exactly. + const raw=String(chunk); + if(!entry) return; + const re=_mediaTokenRe(); + let last=0, m, matched=false; + while((m=re.exec(raw))){ + matched=true; + if(m.index>last){ + _smdMediaWriteText(entry.parent, entry.data, entry.baseAddText, entry.writeText, raw.slice(last,m.index)); + } + // Same length ceiling as the live path and as settled renderMd(): an + // over-ceiling capture is not a legal token, so preserve its raw span as + // text instead of activating a card at stream end. + const emitted=!_mediaTokenExceedsMaxLength(m[1]) + && !!(entry.parent && _smdAppendMediaNode(entry.parent, _unquoteMediaRef(m[1]))); + if(!emitted){ + _smdMediaWriteText(entry.parent, entry.data, entry.baseAddText, entry.writeText, m[0]); + } + const next=m.index+m[0].length; + last=next; + // Defensive only: the shared grammar cannot match an empty span, but do + // not let a future grammar change turn stream-end flush into an infinite loop. + if(re.lastIndex<=m.index) re.lastIndex=m.index+1; + } + if(!matched){ + _smdMediaWriteText(entry.parent, entry.data, entry.baseAddText, entry.writeText, raw); + return; + } + if(last= value.length){ + writeCurrent(value); + return; + } + writeCurrent(value.slice(0, owned)); + if(tails && parser && tails.delete) tails.delete(parser); + _smdMediaAwareAddText(baseAddText, parent, data, value.slice(owned), tails, parser, writeText); + return; + } + // Different owner: the refused line belonged to another text sink, so drop + // the marker and treat this chunk on its own. + if(tails && parser && tails.delete) tails.delete(parser); + leadEntry = null; + } let lead = _smdMediaTailEntryChunk(leadEntry); if(lead && !_smdMediaTailSameOwner(leadEntry, parent, baseAddText, writeText)){ if(tails && parser && tails.delete) tails.delete(parser); @@ -4657,7 +4878,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // Fast path: no MEDIA tokens in the (possibly combined) string. if(!/MEDIA:/.test(combined)){ const prefixTail=_smdMediaPrefixTail(combined); - if(prefixTail && tails && parser && prefixTail.length < _MEDIA_TAIL_MAX){ + // A bare `MEDIA:` prefix fragment can never exceed the keyword's own 6 + // characters, so this gate is structurally satisfied. Spelled against the + // one shared ceiling anyway, so no second number can reappear here. + if(prefixTail && tails && parser && prefixTail.length <= _smdMediaCandidateMax()){ const stable=combined.slice(0, combined.length-prefixTail.length); if(stable) writeCurrent(stable); _smdMediaTailSet(tails, parser, prefixTail, parent, baseAddText, data, writeText); @@ -4670,7 +4894,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=_mediaTokenRe(); let last=0, m; let unmatchedTail=null; while((m=re.exec(combined))){ @@ -4679,30 +4903,160 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ const slice = combined.slice(last, m.index); writeCurrent(slice); } - if(matchEnd===combined.length && !_smdMediaRefHasReliableBoundary(m[1])){ + // Hold the token when more bytes could still change what it captures. + // Two distinct cases, both of which made streaming disagree with settled: + // 1. The token runs to the end of the arrived text — the next chunk may + // extend it (`MEDIA:/tmp/archive.png` then `.bak`). + // 2. The token ended EARLY at a space with same-line text still + // following — the unquoted fallback stops at the space, so + // `MEDIA:/tmp/v1.2 Reports` matches only `/tmp/v1.2` while the + // settled parse of the finished line yields the whole spaced path. + // A closed quoted ref is self-terminating and finalizes immediately; an + // OPEN quote must keep buffering, or the unquoted fallback captures a + // truncated leading-quote fragment the settled parse never yields. + const trailing = combined.slice(matchEnd); + const openQuote = _smdMediaHasOpenQuote(combined.slice(m.index)); + const mayGrow = openQuote + || (matchEnd===combined.length) + || _smdMediaTailCouldExtend(trailing); + if(mayGrow && !_smdMediaTokenIsSettled(m[1], false)){ const candidate = combined.slice(m.index); - if(candidate.length < _MEDIA_TAIL_MAX){ + // Buffer bound derived FROM the lexical ceiling, not an independent + // number. The ceiling is measured on the CAPTURE (everything after the + // keyword), so a candidate holding a still-legal ref is `MEDIA:` plus at + // most MEDIA_TOKEN_MAX_LENGTH characters. Bounding the candidate at the + // bare ceiling instead would refuse refs the settled parser accepts — + // a 4095-character capture is legal, but `MEDIA:` + 4095 is 4101 bytes. + const candidateMax = _smdMediaCandidateMax(); + if(candidate.length <= candidateMax){ unmatchedTail = candidate; } else { - writeCurrent(candidate); + // Past the ceiling even allowing for the keyword. Two shapes reach + // here and they need opposite treatment — collapsing them is what the + // re-gate caught. + // + // (a) A COMPLETE, legal token followed by a long same-line prose run + // that merely looked extendable (`MEDIA:/tmp/a.png see ...4000`). + // Settled parsing renders its card, so flattening the whole + // candidate to text would silently drop a card. Partition: emit + // the token, write the exact prose slices. + // + // (b) The REFERENCE ITSELF exceeds the ceiling. EOF-style + // partitioning would cut it at the cap, emit the prefix as a media + // node, and leave the rest as prose, while settled renderMd() + // applies the same ceiling and keeps the whole span as text. So + // FAIL CLOSED and write it literally. + // + // Ask the shared grammar which shape this is, and judge the CAPTURE + // against the ceiling — the same measurement api/helpers.py uses. + const probe = _mediaTokenRe(); + const pm = probe.exec(candidate); + if(pm && !_mediaTokenExceedsMaxLength(pm[1])){ + _smdMediaTailFlushEntry({ + chunk:candidate, + parent, + data, + baseAddText, + writeText, + }); + } else if(pm){ + // Case (b): the ref itself is over the ceiling. Two sub-cases, and + // the difference is whether the refused span is FINAL. + // + // If the match ends before the end of the arrived text, a real + // delimiter closed it, the span is final, and scanning resumes after + // it — a later INDEPENDENT token (`...oversized... then + // MEDIA:/tmp/ok.png`) is a legal reference that settled parsing + // renders, so swallowing the rest of the line would drop its card. + // + // If the match runs to the END of the arrived text, the reference was + // cut mid-stream and can only grow, so it can never become legal. + // Partitioning on that truncated span is what let a refused + // reference's SUFFIX activate on its own: an over-cap URL is + // tempered-greedy and swallows a nested `MEDIA:`, so settled parsing + // sees ONE over-cap span and renders no card, while the stream + // flushed the truncated prefix and then matched the nested token in + // the next chunk. FAIL CLOSED and take ownership of the rest of the + // line instead. + const refEnd = m.index + pm.index + pm[0].length; + if(refEnd < combined.length){ + writeCurrent(combined.slice(m.index, refEnd)); + last = refEnd; + re.lastIndex = refEnd; + continue; + } + writeCurrent(combined.slice(m.index)); + _smdMediaRefuseLine(tails, parser, parent, baseAddText, data, writeText, + _smdMediaOpenQuoteChar(combined.slice(m.index))); + return; + } else { + writeCurrent(candidate); + } } last = combined.length; break; } - if(!_smdAppendMediaNode(parent, m[1])) writeCurrent(m[0]); + // Length ceiling applies at the ACTIVATION point too, not only in the + // buffer branch above. A token followed by a delimiter is "settled" and + // never enters the tail buffer, so without this an over-ceiling ref would + // become a media node here while settled renderMd() (which applies the + // same ceiling) keeps it as prose. + if(_mediaTokenExceedsMaxLength(m[1])){ + writeCurrent(m[0]); + } else if(!_smdAppendMediaNode(parent, _unquoteMediaRef(m[1]))) { + writeCurrent(m[0]); + } last = matchEnd; } // 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); if(rest){ - const tailMatch = /MEDIA:[^\s\)\]]*$/.exec(rest); + // Space-tolerant so a half-arrived spaced path (`MEDIA:/tmp/My Files`) + // keeps buffering rather than being flushed into the bubble as prose. + // Bounded to the current line: a newline ends any MEDIA token. + const tailMatch = /MEDIA:[^\n]*$/.exec(rest); const prefixTail = tailMatch ? '' : _smdMediaPrefixTail(rest); const tailValue = tailMatch ? tailMatch[0] : prefixTail; - if(tailValue && rest.length < _MEDIA_TAIL_MAX){ + // Bound the BUFFER by what is actually buffered (tailValue), not by the + // length of the whole remaining text. Gating on `rest.length` meant a long + // prose run ending in a MEDIA prefix blew the cap and discarded the tail: + // the partial `MEDIA:/t` was flushed as prose, the next chunk arrived with + // no buffered tail, and the token never reassembled — so a ref preceded by + // more than the ceiling in characters silently lost its card while settled + // parsing rendered it. The cap exists to stop unbounded tail growth, and + // only tailValue can grow. + // + // The ceiling is _smdMediaCandidateMax() — the SAME inclusive candidate + // ceiling the matched-token branch above uses, and inclusive in the same + // way (`<=`). This branch used a separate, smaller `_MEDIA_TAIL_MAX` of + // 4096, and the two ceilings disagreed for every candidate from 4096 + // through 4102 code units. A legal quoted capture whose opening quote + // crossed a chunk boundary lands HERE — the complete quoted grammar has + // not matched yet — so it was flushed as literal text at 4096, quote + // ownership was lost, and a later closing quote could not reassemble it, + // while settled renderMd() accepted the identical capture. + if(tailValue && tailValue.length <= _smdMediaCandidateMax()){ const tailStart = tailMatch ? tailMatch.index : rest.length-prefixTail.length; if(tailStart>0) writeCurrent(rest.slice(0, tailStart)); unmatchedTail = tailValue; + } else if(tailValue){ + // Genuinely past the ceiling. FAIL THE REFERENCE CLOSED: write its whole + // span as literal text, exactly as settled renderMd() does, and take + // ownership of the rest of the line so the refusal survives the chunk + // boundary. Re-scanning inside a refused reference is what let an + // over-limit ref's SUFFIX activate on its own — an over-cap URL that + // swallows a nested `MEDIA:` rendered a card streamed while settled + // parsing kept the whole span as prose. + // + // Prose before the reference is still written separately, so a long turn + // preceding an oversized ref keeps its exact text. + const tailStart = tailMatch ? tailMatch.index : rest.length-prefixTail.length; + if(tailStart>0) writeCurrent(rest.slice(0, tailStart)); + writeCurrent(rest.slice(tailStart)); + _smdMediaRefuseLine(tails, parser, parent, baseAddText, data, writeText, + _smdMediaOpenQuoteChar(rest.slice(tailStart))); + return; } else { writeCurrent(rest); } diff --git a/static/ui.js b/static/ui.js index 3b51b5a4c4c..4147420a2bf 100644 --- a/static/ui.js +++ b/static/ui.js @@ -2577,6 +2577,228 @@ function _dataImageHtml(ref, altText){ return `${esc(altText||'image')}`; } +// ── MEDIA: path matching (shared with the streaming path in messages.js) ────── +// A MEDIA path may legitimately contain spaces: +// MEDIA:/home/u/vault/Meeting Notes/2026-07-29 - SDE Focus Group.md +// The original `[^\s\)\]]+` class stopped at the first space, so the artifact +// card was built from the truncated path ("Meeting"), rendered the wrong +// basename, and the remainder of the real path leaked into the bubble as raw +// prose beside the card. +// +// Widening cannot be unbounded — greedy space-tolerance would swallow trailing +// prose ("MEDIA:/tmp/a.png looks good") and glue an adjacent tag +// ("MEDIA:/a.png MEDIA:/b.png") into one invalid path, which is the bug class +// the Python gateway already hit (#68773). So the bare form is anchored on a +// file extension and tempered: it crosses single spaces only while still +// reaching a `.ext`, never crosses a newline, and never crosses a following +// MEDIA: keyword. Extension-less paths still match via the fallback branch, so +// nothing that worked before stops working. +// +// Keep this the SINGLE source of truth for MEDIA path shape. messages.js reuses +// it so the streamed and settled renderings of one token stay byte-identical. +// +// Exposed as a FUNCTION, not a const, because the test harnesses in tests/*.py +// white-box-extract production code by `function (` declaration and eval +// it in isolation (see tests/test_data_uri_images.py). A bare top-level const is +// invisible to that extractor, so any renderMd harness would die with +// "_MEDIA_PATH_SRC is not defined". When you add a helper that renderMd calls, +// add it to the eval list in every harness that extracts renderMd. +function _mediaPathSrc(){ + // One path character: no whitespace, and none of the delimiters that close a + // token. + const ch = String.raw`[^\s\)\]]`; + // FIRST character of an unquoted ref. A quote is excluded here — and only + // here — so an UNTERMINATED quoted ref cannot fall through to a bare branch. + // Without it, `MEDIA:"/tmp/bad.png` (no closing quote) fails the quoted + // alternative, falls through to the bare grammar, and captures the literal + // `"/tmp/bad.png` INCLUDING the leading quote, which the streaming flush then + // activates as a media node. Interior quotes stay legal (`ch` is unchanged). + // Kept in lockstep with ch_first in api/helpers.py. + const chFirst = String.raw`[^\s\)\]"']`; + // A whole space-separated word that contains NO dot. Requiring the + // intermediate words to be dot-free is what BOUNDS the spaced form: the run + // can cross `Reports/` or `Notes/`, but stops dead at the first word carrying + // a `.ext`, so trailing prose after `a.png` is never absorbed and two + // adjacent MEDIA tags stay two tags. + // + // Spelled as a dot-free character class rather than a negative lookbehind on + // purpose. A lookbehind assertion here would be a PARSE-TIME brick on engines + // without regex lookbehind support (Safari < 16.4, some embedded WebViews): + // ui.js is a classic deferred script, so the whole file fails to parse and the + // app blanks. tests/test_5552_viewport_anchor_surrogate.py enforces that, and + // greps the raw source — so do not spell the assertion out even in a comment. + const wordNoDot = String.raw`(?!MEDIA:)[^\s\)\]\.]+`; + // The final space-separated word carries the extension. This is what makes a + // dotted DIRECTORY work: `/tmp/v1.2 Reports/chart.png` must not settle on + // `/tmp/v1.2` just because a space follows it. + const finalWithExt = String.raw`(?!MEDIA:)${ch}+?\.[A-Za-z0-9]+`; + // Bare matches must end at whitespace, a closing delimiter, a glued MEDIA: + // keyword, or end of input — never mid-prose. The closing-brace delimiter is + // spelled as the hex escape below because the harnesses' brace counter does + // not skip regex literals or comments, so an unmatched brace anywhere in this + // function truncates the extraction mid-literal. + const boundary = String.raw`(?=[\s\)\]\x7d"'*_,;:]|MEDIA:|$)`; + // Terminal sentence punctuation belongs to the PROSE, not to the ref: + // `see MEDIA:/tmp/a.png.` is a sentence about a file, not a file named + // `a.png.`. `.`/`!`/`?` only close the token when the NEXT character ends the + // token anyway, so a dot genuinely inside a name still belongs to the ref + // (`/tmp/a.tar.gz`, `/tmp/v1.2/chart.png`). Kept in lockstep with + // _MEDIA_TOKEN_BOUNDARY / _MEDIA_TOKEN_SENTENCE_END in api/helpers.py. + // Sentence punctuation only when a REAL delimiter follows. End-of-input does + // NOT count: a streaming chunk can stop mid-token, and treating that as a + // sentence end would capture `/tmp/a` out of `MEDIA:/tmp/a.png` on the final + // chunk, making the streamed and settled renderings disagree. + const sentenceEnd = String.raw`[.!?](?=[\s\)\]\x7d"'*_,;:])`; + const boundaryOrSentence = String.raw`(?=[\s\)\]\x7d"'*_,;:]|${sentenceEnd}|MEDIA:|$)`; + // One path character that is NOT terminal sentence punctuation. TEMPERED + // GREEDY, not lazy: it consumes as much as the old greedy `${ch}+` did, so + // `C:/tmp/live.png` and a URL's `://` plus any nested `MEDIA:` in its query + // stay inside one token. A lazy `${ch}+?` would stop at `C` (because `:` + // closes a token) and at a nested `MEDIA:`. + const chNotSentenceEnd = String.raw`(?:(?!${sentenceEnd})${ch})`; + // Same run, first character only — cannot be a quote (see chFirst). + const chFirstNotSentenceEnd = String.raw`(?:(?!${sentenceEnd})${chFirst})`; + // Ambiguous prose shape: the FIRST word is already a complete dot-bearing + // filename, followed by dot-free prose and then another filename: + // + // MEDIA:/tmp/a.png see README.md + // + // Reject that shape so `/tmp/a.png` wins and the rest remains prose. The + // continuation classes exclude `/`, so `/tmp/v1.2 Reports/chart.png` does not + // match the rejection and dotted-directory support remains intact. A genuinely + // ambiguous spaced filename can use the already-supported quoted form. + const noSlash = String.raw`[^\s\)\]/]`; + const wordNoDotNoSlash = String.raw`(?!MEDIA:)[^\s\)\]\./]+`; + const finalWithExtNoSlash = String.raw`(?!MEDIA:)${noSlash}+?\.[A-Za-z0-9]+`; + const dottedFirstThenDottedProse = String.raw`(?!MEDIA:)${ch}+?\.[A-Za-z0-9]+(?:[^\S\n]${wordNoDotNoSlash})*?[^\S\n]${finalWithExtNoSlash}${boundary}`; + const spaced = String.raw`(?!(?:${dottedFirstThenDottedProse}))(?!MEDIA:)${chFirst}${ch}*?(?:[^\S\n]${wordNoDot})*?[^\S\n]${finalWithExt}${boundary}`; + const nospace = String.raw`(?!MEDIA:)${chFirst}${ch}*?\.[A-Za-z0-9]+${boundaryOrSentence}`; + // A real HTTP(S) URL is one tempered-greedy run, tried BEFORE the bare forms so + // `://host/...` (and a nested `MEDIA:` in its path/query) stays in one token. A + // query string keeps its `?`/`!`; a trailing period stays in the sentence. + const externalUrl = String.raw`(?:[hH][tT][tT][pP][sS]?)://${chNotSentenceEnd}+`; + // Extension-less legacy shape: greedy over path characters, but a trailing + // sentence `.`/`!`/`?` is left to the prose instead of captured. + const extensionless = String.raw`${chFirstNotSentenceEnd}${chNotSentenceEnd}*`; + // Quoted form wins first (can hold any character), then the spaced form, then + // the no-space form, then the fallback for extension-less paths. + // Kept in lockstep with media_token_pattern() in api/helpers.py. + return String.raw`"[^"\n]+"|'[^'\n]+'|${externalUrl}|${spaced}|${nospace}|${extensionless}|${chFirst}${ch}*`; +} + +/** Global matcher for MEDIA: tokens. Fresh instance per call — a shared /g regex + * carries lastIndex between callers and silently skips matches. */ +function _mediaTokenRe(){ + return new RegExp(String.raw`MEDIA:(${_mediaPathSrc()})`, 'g'); +} + +/** Strip surrounding quotes from a captured MEDIA path. */ +function _unquoteMediaRef(ref){ + const value = String(ref || '').trim(); + const quote = value[0]; + return (quote === '"' || quote === "'") && value.endsWith(quote) ? value.slice(1, -1) : value; +} + +/** True when a MEDIA ref carries an http(s) scheme. Scheme only — NOT a trust + * decision. Mirrors is_external_media_url() in api/helpers.py. */ +function _isExternalMediaUrl(ref){ + return /^https?:\/\//i.test(_unquoteMediaRef(ref)); +} + +/** Ceiling on a MEDIA token's captured length — part of the shared lexical + * contract, NOT a private detail of the streaming buffer. If the streaming + * parser treated "buffer full" as "stream ended" and finalized the token while + * settled renderMd() re-parsed the same text uncapped and saw ONE complete + * reference, the two renderings would disagree. A candidate that reaches the + * ceiling without a real delimiter is not a token: it fails closed and stays + * literal text. + * + * Kept in lockstep with MEDIA_TOKEN_MAX_LENGTH in api/helpers.py and + * _MEDIA_TAIL_MAX in static/messages.js. + * + * UNIT: UTF-16 CODE UNITS, in both languages. Deliberate: this cap bounds the + * per-parser streaming buffer, that buffer is a JavaScript string, JavaScript + * strings are stored as UTF-16, and `String.prototype.length` counts code + * units — so code units are the unit that actually bounds the memory. Counting + * code points instead would let a 4096-character astral token occupy 8192 code + * units of the buffer. Python `len()` counts code points, so api/helpers.py + * converts via media_token_length(); before it did, a token of 2049 U+1F600 + * characters measured 2049 in Python and 4098 here, and Python admitted a + * token this side refused. */ +function _mediaTokenMaxLength(){ return 4096; } + +/** True when a capture is too long to be a legal MEDIA token. Measured on the + * RAW capture (quotes included), matching the streaming buffer's own bound, in + * UTF-16 code units — `.length` IS that unit, so no conversion is needed here. + * Mirrors media_token_exceeds_max_length() in api/helpers.py. */ +function _mediaTokenExceedsMaxLength(ref){ + return String(ref == null ? '' : ref).length > _mediaTokenMaxLength(); +} + +/** Markers meaning "this URL leads back to a local or authenticated target". + * Built lazily inside a function rather than held in a module-scope `const`: + * several test harnesses extract these helpers by name and eval them before + * the module body has finished evaluating, and a `const` read in that window + * throws a temporal-dead-zone ReferenceError instead of returning a verdict. + * A security predicate must never fail that way. + * Mirrors _LOCAL_TARGET_MARKERS in api/helpers.py. */ +function _localTargetMarkers(){ + return ['media:', 'file://', '/api/media']; +} + +/** Percent-decode up to `rounds` times, stopping when stable. Bounded: a single + * decode misses `%254d`, and an unbounded loop is a DoS on crafted input. + * Mirrors _decode_url_component_bounded() in api/helpers.py. */ +function _decodeUrlComponentBounded(value, rounds){ + let current = String(value || ''); + const max = typeof rounds === 'number' ? rounds : 3; + for(let i=0; i decoded.indexOf(marker) !== -1); +} + // Markdown image syntax ![alt](url) → HTML. https:// keeps the historical direct // ; file:// and bare data:image/ URIs route through the same helpers the // MEDIA: pipeline uses, so ![x](file:///p.png) renders the artifact card instead @@ -2618,8 +2840,30 @@ function _inlineMediaHtmlForRef(ref, sessionId, altText){ if(/^https?:\/\//i.test(ref)){ let src=ref; if(/^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?/i.test(src)){ + // Retarget a loopback ref at the CURRENT origin. This is deliberate for + // the normal app (a dev-server asset URL should follow the app), but it is + // exactly what makes a hostile ref dangerous on the PUBLIC share page: + // `MEDIA:http://127.0.0.1:8080/api/media?path=/etc/shadow` would become a + // same-origin AUTHENTICATED request issued by the viewer's browser. + // + // So gate only the rewrite, not loopback rendering as a whole: if the ref + // hides a nested MEDIA:/file:///api/media target, render it inert. A plain + // loopback image (`http://127.0.0.1:8787/img/shot.png`) is untouched and + // still follows the origin as before. + // + // api/shares.py placeholders these server-side before a snapshot is ever + // written; this is the second line, so a snapshot written by an older + // build cannot fire one either. + if(typeof _externalMediaUrlHidesLocalTarget==='function' && _externalMediaUrlHidesLocalTarget(ref)){ + return `${esc(String(ref).slice(0,120))}`; + } const base=(typeof document!=='undefined'&&document.baseURI||'').replace(/\/$/,''); src=src.replace(/^https?:\/\/(localhost|127\.0\.0\.1)(:\d+)?/i,base); + }else if(typeof _externalMediaUrlHidesLocalTarget==='function' && _externalMediaUrlHidesLocalTarget(ref)){ + // Non-loopback host, but the URL still smuggles a local/authenticated + // target in its path, query, or fragment (`https://cdn.test/api/media?...`, + // or a nested `MEDIA:`/`file://`). Never a renderable public asset. + return `${esc(String(ref).slice(0,120))}`; } const urlPath=src.split('?')[0]; // SVG URLs → render inline as image (must precede the https:// @@ -7187,8 +7431,14 @@ 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)=>{ - media_stash.push(raw_ref); + s=s.replace(_mediaTokenRe(),(whole,raw_ref)=>{ + // Honor the shared length ceiling here too. The streaming parser refuses an + // over-ceiling candidate (its buffer is bounded), so if settled parsing + // accepted one, a single reference would render as a card in one view and + // as literal text in the other. Leave the whole span untouched: it stays + // prose, exactly as the streamed path writes it. + if(_mediaTokenExceedsMaxLength(raw_ref)) return whole; + media_stash.push(_unquoteMediaRef(raw_ref)); return '\x00D'+(media_stash.length-1)+'\x00'; }); // ── End MEDIA stash ───────────────────────────────────────────────────────── @@ -8538,8 +8788,11 @@ function _stripForTTS(text){ text=text.replace(/^#{1,6}\s+/gm,''); // Strip links, keep text text=text.replace(/\[([^\]]+)\]\([^)]+\)/g,'$1'); - // Replace MEDIA: paths with a simple label - text=text.replace(/MEDIA:[^\s]+/g,'a file'); + // Replace MEDIA: refs with a simple label. Uses the SHARED token grammar so a + // dotted/spaced or quoted path is removed whole — `/MEDIA:[^\s]+/g` stopped at + // the first space, leaving the local-path tail ("Reports/chart.png") to be read + // aloud, and left the closing quote of a quoted ref behind. + text=text.replace(_mediaTokenRe(),'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/js/share_media_guard_driver.js b/tests/js/share_media_guard_driver.js new file mode 100644 index 00000000000..0000b508e85 --- /dev/null +++ b/tests/js/share_media_guard_driver.js @@ -0,0 +1,103 @@ +/** + * Drive the REAL _inlineMediaHtmlForRef() from static/ui.js, using the same + * brace-depth extraction the repo's own test_renderer_js_behaviour.py uses. + * Proves the share-page guard blocks the smuggled-target shapes while leaving + * a legitimate loopback asset rewrite intact. + */ +const fs = require('fs'); +const src = fs.readFileSync(process.argv[2], 'utf8'); + +global.window = {}; +global.document = { createElement: () => ({ innerHTML: '', textContent: '' }), baseURI: 'http://localhost:8787/app/' }; +const esc = s => String(s ?? '').replace(/[&<>"']/g, c => ( + {'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); +const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i; +const _SVG_EXTS=/\.svg$/i; +const _AUDIO_EXTS=/\.(mp3|ogg|wav|m4a|aac|flac|wma|opus|webm)$/i; +const _VIDEO_EXTS=/\.(mp4|webm|mkv|mov|avi|ogv|m4v)$/i; +function _mediaKindForName(n){ + const s=String(n||''); + if(_IMAGE_EXTS.test(s)) return 'image'; + if(_AUDIO_EXTS.test(s)) return 'audio'; + if(_VIDEO_EXTS.test(s)) return 'video'; + return 'file'; +} +function _mediaPlayerHtml(kind,src,name){ return `<${kind} src="${esc(src)}">`; } +function _dataImageHtml(){ return ''; } +function t(k){ return k; } + +function extractFunc(name) { + const re = new RegExp('function\\s+' + name + '\\s*\\('); + const start = src.search(re); + if (start < 0) throw new Error(name + ' not found'); + let i = src.indexOf('{', start); + let depth = 1; i++; + while (depth > 0 && i < src.length) { + if (src[i] === '{') depth++; + else if (src[i] === '}') depth--; + i++; + } + return src.slice(start, i); +} + +eval(extractFunc('_unquoteMediaRef')); +eval(extractFunc('_isExternalMediaUrl')); +eval(extractFunc('_localTargetMarkers')); +eval(extractFunc('_decodeUrlComponentBounded')); +eval(extractFunc('_externalMediaUrlHidesLocalTarget')); +eval(extractFunc('_inlineMediaHtmlForRef')); + +const cases = [ + // [label, ref, mustBeInert] + ['plain loopback image still rewrites to origin', 'http://127.0.0.1:8787/img/shot.png', false], + ['plain localhost image still rewrites to origin', 'http://localhost:8787/img/shot.png', false], + ['public cdn image renders', 'https://cdn.test/a.png', false], + ['public cdn with harmless query renders', 'https://cdn.test/a.png?w=800&fmt=webp', false], + ['loopback /api/media is inert', 'http://127.0.0.1:8787/api/media?path=/etc/shadow', true], + ['loopback nested MEDIA: is inert', 'http://127.0.0.1:8787/i.png?src=MEDIA:/etc/shadow.png', true], + ['public cdn nested MEDIA: is inert', 'https://cdn.test/a.png?src=MEDIA:/etc/shadow.png', true], + ['public cdn /api/media is inert', 'https://cdn.test/api/media?path=/etc/shadow', true], + ['nested MEDIA: in fragment is inert', 'https://cdn.test/a.png#MEDIA:/etc/shadow.png', true], + ['file:// in query is inert', 'https://cdn.test/a.png?src=file:///etc/shadow.png', true], + ['percent-encoded MEDIA: is inert', 'https://cdn.test/a.png?src=%4dEDIA:/etc/shadow.png', true], + ['double-encoded MEDIA: is inert', 'https://cdn.test/a.png?src=%254dEDIA:/etc/shadow.png', true], + // Private HOST alone is deliberately NOT inert client-side: the normal app + // legitimately serves assets from a dev server. The server-side twin in + // api/helpers.py rejects these for published snapshots. + ['RFC1918 plain image renders (server-side rejects for shares)', 'http://192.168.1.5/x.png', false], + ['RFC1918 host reaching /api/media is inert', 'http://192.168.1.5/api/media?path=/etc/shadow', true], + ['10/8 host nested MEDIA: is inert', 'http://10.0.0.7/i.png?src=MEDIA:/etc/shadow.png', true], +]; + +let failures = 0; +for (const [label, ref, mustBeInert] of cases) { + let html; + try { html = _inlineMediaHtmlForRef(ref); } + catch (e) { html = 'THREW: ' + e.message; } + const isInert = /^/.test(html); + const ok = isInert === mustBeInert; + if (!ok) failures++; + console.log(`${ok ? 'PASS' : 'FAIL'} | ${mustBeInert ? 'inert ' : 'render'} | ${label}`); + if (!ok) console.log(` got: ${String(html).slice(0, 200)}`); + // A rendered loopback ref must actually have been retargeted at the origin. + if (ok && !mustBeInert && /127\.0\.0\.1|localhost/.test(ref)) { + const rewritten = html.includes('localhost:8787/app'); + if (!rewritten) { failures++; console.log(`FAIL | loopback rewrite lost for ${ref}: ${html.slice(0,160)}`); } + else console.log(' (rewrite to origin preserved)'); + } + // No inert output may leak the smuggled path as a LIVE attribute. The inert + // body legitimately contains the escaped URL text (which can include + // the literal characters "src="), so assert on real markup instead: no + // element with a src/href attribute, and no unescaped angle brackets. + if (isInert) { + if (/<(img|a|audio|video|iframe|script)\b/i.test(html)) { + failures++; console.log('FAIL | inert output produced a live element'); + } + const body = html.replace(/^/, '').replace(/<\/code>$/, ''); + if (/[<>]/.test(body)) { + failures++; console.log('FAIL | inert output body is not escaped: ' + body.slice(0,120)); + } + } +} +console.log(failures === 0 ? '\nALL OK' : `\n${failures} FAILURE(S)`); +process.exit(failures === 0 ? 0 : 1); diff --git a/tests/test_data_uri_images.py b/tests/test_data_uri_images.py index 7e19509a3dc..77cb40cb0fd 100644 --- a/tests/test_data_uri_images.py +++ b/tests/test_data_uri_images.py @@ -84,6 +84,15 @@ eval(extractFunc('_inlineMediaHtmlForRef')); eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); +// Shared MEDIA length ceiling consulted by renderMd's stash. Extracted, not +// stubbed, so this driver applies the same rule as the streaming path. +eval(extractFunc('_mediaTokenMaxLength')); +eval(extractFunc('_mediaTokenExceedsMaxLength')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_issue1154_fenced_code_leak.py b/tests/test_issue1154_fenced_code_leak.py index 5c24d68a725..8f965ec8106 100644 --- a/tests/test_issue1154_fenced_code_leak.py +++ b/tests/test_issue1154_fenced_code_leak.py @@ -45,6 +45,11 @@ } eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_issue1446_glued_heading_lift.py b/tests/test_issue1446_glued_heading_lift.py index fcd2f924e6d..21df3b1a7d2 100644 --- a/tests/test_issue1446_glued_heading_lift.py +++ b/tests/test_issue1446_glued_heading_lift.py @@ -210,6 +210,11 @@ def test_lift_regex_is_single_line_only(): } eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_issue1618_yaml_json_diff_newline_preserve.py b/tests/test_issue1618_yaml_json_diff_newline_preserve.py index 76bc8d44422..20062c346e7 100644 --- a/tests/test_issue1618_yaml_json_diff_newline_preserve.py +++ b/tests/test_issue1618_yaml_json_diff_newline_preserve.py @@ -147,6 +147,11 @@ def test_pre_stash_still_captures_pre_header_and_optional_div(): } eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_issue2428_table_pipe_protection.py b/tests/test_issue2428_table_pipe_protection.py index 064f689ceb0..110770009a6 100644 --- a/tests/test_issue2428_table_pipe_protection.py +++ b/tests/test_issue2428_table_pipe_protection.py @@ -60,6 +60,11 @@ } eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_issue2768_workspace_links.py b/tests/test_issue2768_workspace_links.py index 0d759d5b43a..662c713b12a 100644 --- a/tests/test_issue2768_workspace_links.py +++ b/tests/test_issue2768_workspace_links.py @@ -44,6 +44,11 @@ def _render(markdown: str) -> str: ) js += "\n" + _extract_function(UI_JS, "_matchBacktickFenceLine") js += "\n" + _extract_function(UI_JS, "_isBacktickFenceClose") + # MEDIA: token matchers renderMd depends on. Must be appended BEFORE + # renderMd so the extracted function can resolve them. + js += "\n" + _extract_function(UI_JS, "_mediaPathSrc") + js += "\n" + _extract_function(UI_JS, "_mediaTokenRe") + js += "\n" + _extract_function(UI_JS, "_unquoteMediaRef") js += "\n" + _extract_function(UI_JS, "renderMd") js += textwrap.dedent( r''' diff --git a/tests/test_issue2881_workspace_chat_links.py b/tests/test_issue2881_workspace_chat_links.py index 6de2b20fdd3..b8654368bed 100644 --- a/tests/test_issue2881_workspace_chat_links.py +++ b/tests/test_issue2881_workspace_chat_links.py @@ -42,6 +42,11 @@ } eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_issue342.py b/tests/test_issue342.py index f555b11857d..85bce86af10 100644 --- a/tests/test_issue342.py +++ b/tests/test_issue342.py @@ -15,6 +15,29 @@ def read_ui_js(): return f.read() +def _extract_js_function(src, header): + """Return a JS function's full source, matched by brace depth. + + Structural tests here used fixed character windows after a `find()`, which + silently drift out of the function whenever anything earlier in it grows. + That produces failures with no behavior change (and can slide PAST the + expression under test, passing for the wrong reason). Brace counting is + exact for this file: ui.js has no unbalanced braces inside string or regex + literals in the functions these tests target. + """ + start = src.index(header) + i = src.index('{', start) + depth = 1 + i += 1 + while depth and i < len(src): + if src[i] == '{': + depth += 1 + elif src[i] == '}': + depth -= 1 + i += 1 + return src[start:i] + + def test_autolink_comment_present(): """The Autolink comment should be present in renderMd() to document the feature.""" content = read_ui_js() @@ -27,11 +50,12 @@ def test_autolink_comment_present(): def test_autolink_regex_in_rendermd(): """The autolink regex pattern (https?://) should appear in renderMd().""" content = read_ui_js() - # Locate the renderMd function body - rendermd_start = content.find('function renderMd(raw){') - assert rendermd_start != -1, "renderMd function not found in ui.js" - # Find the closing brace after renderMd (look for the autolink pattern within it) - rendermd_body = content[rendermd_start:rendermd_start + 15000] + # Extract the WHOLE renderMd body by brace depth rather than slicing a fixed + # character window. renderMd is ~35KB and the autolink pass sits past the + # 15000-char mark, so any edit earlier in the function silently pushed the + # pattern out of the window and failed this test without the behavior + # changing at all. + rendermd_body = _extract_js_function(content, 'function renderMd(raw){') assert 'https?:\\/\\/' in rendermd_body, ( "Autolink regex (https?:\\/\\/) not found inside renderMd() body." ) diff --git a/tests/test_issue347.py b/tests/test_issue347.py index 1b94ec4a5bf..6a87433a92b 100644 --- a/tests/test_issue347.py +++ b/tests/test_issue347.py @@ -56,6 +56,11 @@ def _run_renderers(markdown: str) -> dict: js += "\n" + _extract_function(UI_JS, "_matchBacktickFenceLine") js += "\n" + _extract_function(UI_JS, "_isBacktickFenceClose") js += "\n" + _extract_function(UI_JS, "_renderUserFencedBlocks") + # MEDIA: token matchers renderMd depends on. Must be appended BEFORE + # renderMd so the extracted function can resolve them. + js += "\n" + _extract_function(UI_JS, "_mediaPathSrc") + js += "\n" + _extract_function(UI_JS, "_mediaTokenRe") + js += "\n" + _extract_function(UI_JS, "_unquoteMediaRef") js += "\n" + _extract_function(UI_JS, "renderMd") js += textwrap.dedent( r''' diff --git a/tests/test_media_consumer_consistency.py b/tests/test_media_consumer_consistency.py new file mode 100644 index 00000000000..11e73059ba0 --- /dev/null +++ b/tests/test_media_consumer_consistency.py @@ -0,0 +1,279 @@ +"""MEDIA grammar consistency across every live consumer (PR #6607 re-review). + +Re-review item 2: the shared grammar was not actually shared by all consumers. + +- ``static/ui.js::_stripForTTS()`` used ``/MEDIA:[^\\s]+/g``, so a dotted/spaced + or quoted path was only partly removed and the local-path tail was spoken + aloud. +- ``media_token_pattern(exclude_urls=True)`` applied a case-SENSITIVE + ``https?://`` guard OUTSIDE the capture, so a quoted URL + (``MEDIA:"https://…"``) and an uppercase scheme (``MEDIA:HTTPS://…``) both + matched as local share paths and were replaced with the missing-media + placeholder, while the frontend rendered them as remote images. + +One fixture table drives classification for both languages plus the real +``_embed_share_media()``. +""" +from __future__ import annotations + +import json +import re +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +if str(REPO_ROOT) not in sys.path: + sys.path.insert(0, str(REPO_ROOT)) + +from api.helpers import ( # noqa: E402 + is_external_media_url, + media_token_pattern, + unquote_media_ref, +) + +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") + +# A 1x1 PNG that satisfies the share inliner's magic-byte + MIME checks. +# Written as a commented, chunk-by-chunk bytes literal rather than a single +# bytes.fromhex(...) blob so each PNG chunk is readable in place and the fixture +# needs no decoding step at import time. +_TINY_PNG = ( + b"\x89PNG\r\n\x1a\n" # signature + b"\x00\x00\x00\rIHDR" # IHDR chunk header + b"\x00\x00\x00\x01\x00\x00\x00\x01" # 1x1 + b"\x08\x06\x00\x00\x00" # 8-bit RGBA + b"\x1f\x15\xc4\x89" # IHDR CRC + b"\x00\x00\x00\nIDAT" # IDAT chunk header + b"x\x9cc\x00\x01\x00\x00\x05\x00\x01" # zlib-compressed single pixel + b"\r\n-\xb4" # IDAT CRC + b"\x00\x00\x00\x00IEND\xaeB`\x82" # IEND +) + +# (ref_as_written, is_external) +URL_CASES = [ + ("https://example.test/a.png", True), + ('"https://example.test/a.png"', True), + ("'https://example.test/a.png'", True), + ("HTTPS://example.test/a.png", True), + ("Https://example.test/a.png", True), + ("http://example.test/a.png", True), + ("HTTP://example.test/a.png", True), + ('"HTTPS://example.test/a.png"', True), + ("file:///tmp/a.png", False), + ("data:image/png;base64,AAAA", False), + ("/tmp/local.png", False), + ('"/tmp/My Files/x.png"', False), + ("/tmp/v1.2 Reports/chart.png", False), + ("/tmp/no-ext-file", False), + # A colon inside a local filename is not a scheme. + ("/tmp/notdata:x.png", False), +] + + +@pytest.mark.parametrize("ref,external", URL_CASES) +def test_python_classifies_external_urls_case_insensitively(ref, external): + assert is_external_media_url(ref) is external + + +@pytest.mark.parametrize("ref", [ + c[0] for c in URL_CASES + if c[1] and re.match(r"(?i)^[\"']?https?://", c[0]) +]) +def test_share_matcher_never_captures_an_external_http_url(ref): + """The share pattern must skip HTTP(S) URLs in every spelling. + + A captured URL is resolved as a local path and placeholdered, so this is the + difference between a share showing a remote image and showing "media + unavailable". + + Scoped to HTTP(S) deliberately. ``file://`` and ``data:`` refs ARE matched on + purpose — the share boundary must see them so it can reject ``file://`` as + absolute/un-scoped (api/shares.py:274) rather than let it through unexamined. + That predates this PR and is asserted separately below. + """ + share_re = re.compile(media_token_pattern(extra_exclude=">", exclude_urls=True)) + m = share_re.search(f"MEDIA:{ref}") + assert m is None, ( + f"share matcher captured external URL {ref!r} as " + f"{unquote_media_ref(m.group(1))!r} — it will be placeholdered" + ) + + +def test_share_matcher_still_sees_file_uris_so_they_can_be_rejected(): + """``file://`` must remain visible to the share boundary. + + api/shares.py rejects it explicitly as absolute/un-scoped. If the URL guard + skipped it, the token would pass through a public share unexamined. + """ + share_re = re.compile(media_token_pattern(extra_exclude=">", exclude_urls=True)) + assert share_re.search("MEDIA:file:///tmp/a.png") is not None + + +@pytest.mark.parametrize("ref", [ + c[0] for c in URL_CASES + if c[1] and re.match(r"(?i)^[\"']?https?://", c[0]) +]) +def test_share_inliner_leaves_external_http_urls_untouched(ref, tmp_path): + from api import shares + + text = f"see MEDIA:{ref} ok" + out = shares._embed_share_media(text, allowed_roots=(tmp_path,)) + assert out == text, f"external URL {ref!r} was rewritten: {out!r}" + + +def test_share_inliner_placeholders_a_file_uri(tmp_path): + """Pre-existing security posture, pinned: file:// never resolves in a share.""" + from api import shares + + target = tmp_path / "secret.png" + target.write_bytes(_TINY_PNG) + out = shares._embed_share_media( + f"see MEDIA:file://{target} ok", allowed_roots=(tmp_path,) + ) + assert "data:image/png;base64," not in out + + +@pytest.mark.parametrize("quoted", [True, False]) +def test_share_inliner_still_embeds_local_spaced_paths(tmp_path, quoted): + """The URL guard must not become a blanket skip.""" + from api import shares + + target = tmp_path / "My Files" / "chart.png" + target.parent.mkdir(parents=True) + target.write_bytes(_TINY_PNG) + ref = f'"{target}"' if quoted else str(target) + out = shares._embed_share_media(f"see MEDIA:{ref} ok", allowed_roots=(tmp_path,)) + assert "data:image/png;base64," in out + + +# ── TTS ───────────────────────────────────────────────────────────────────── + + +def _js_strip_for_tts(texts: list[str]) -> list[str]: + """Run the real _stripForTTS MEDIA replacement under node.""" + node = shutil.which("node") + if not node: + pytest.skip("node not available") + + def extract(src: str, name: str) -> str: + start = src.index(f"function {name}(") + depth = 0 + started = False + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + started = True + elif src[i] == "}": + depth -= 1 + if started and depth == 0: + return src[start:i + 1] + raise AssertionError(f"unbalanced braces extracting {name}") + + script = "\n".join([ + extract(UI_JS, "_mediaPathSrc"), + extract(UI_JS, "_mediaTokenRe"), + "const texts = JSON.parse(process.argv[1]);", + "console.log(JSON.stringify(texts.map(t => t.replace(_mediaTokenRe(), 'a file'))));", + ]) + proc = subprocess.run( + [node, "--input-type=module", "-e", script, json.dumps(texts)], + capture_output=True, text=True, timeout=30, check=True, + ) + return json.loads(proc.stdout) + + +TTS_CASES = [ + ("See MEDIA:/tmp/a.png now", "See a file now"), + # Dotted/spaced: the old /MEDIA:[^\s]+/ left "Reports/chart.png" to be spoken. + ("See MEDIA:/tmp/v1.2 Reports/chart.png now", "See a file now"), + # Quoted: the old pattern left the closing quote behind. + ('See MEDIA:"/tmp/My Files/report (final).png" now', "See a file now"), + ("See MEDIA:'/tmp/My Files/single.png' now", "See a file now"), + ("Two MEDIA:/tmp/a.png and MEDIA:/tmp/b.png here", + "Two a file and a file here"), + ("No media here at all", "No media here at all"), +] + + +@pytest.mark.parametrize("text,expected", TTS_CASES) +def test_tts_strips_whole_media_ref(text, expected): + assert _js_strip_for_tts([text]) == [expected] + + +def test_tts_uses_the_shared_grammar_not_a_private_regex(): + """Guard against a private MEDIA regex reappearing in the TTS path.""" + start = UI_JS.index("function _stripForTTS") + # Bound the scan to the function body. + depth = 0 + started = False + end = start + for i in range(start, len(UI_JS)): + if UI_JS[i] == "{": + depth += 1 + started = True + elif UI_JS[i] == "}": + depth -= 1 + if started and depth == 0: + end = i + 1 + break + body = UI_JS[start:end] + assert "_mediaTokenRe()" in body, ( + "_stripForTTS must route MEDIA refs through the shared token grammar" + ) + # Strip line comments before scanning: the explanatory comment legitimately + # quotes the old pattern, and greping raw source would match it. + code = "\n".join( + line.split("//", 1)[0] for line in body.splitlines() + ) + assert "MEDIA:[^\\s]" not in code, ( + "_stripForTTS still carries the whitespace-only MEDIA regex, which " + "leaves the local-path tail of a spaced ref to be spoken aloud" + ) + + +# ── MEDIA inside code fences ──────────────────────────────────────────────── + + +def test_media_in_code_is_active_in_settled_and_streaming(): + """Document and pin the code-fence policy: ACTIVE media, both paths. + + Reviewer question: do safe streaming, fade streaming, and settled + ``renderMd()`` apply the same MEDIA policy inside inline/fenced code? + + They do, and the chosen policy is **active media**: + + - Settled: ``renderMd()`` stashes MEDIA tokens FIRST, before the fence pass + ("must run first, before any other processing"), so a token inside ``` is + already replaced by a stash placeholder by the time fenced code is + HTML-escaped. + - Streaming: the smd parser delivers fenced-code content through + ``add_text``, which is the method the MEDIA interceptor wraps (verified + against the real vendored parser: text inside ``` arrives at ``add_text`` + while the code-block token is open). Both streaming renderers — safe and + fade — wrap the same ``add_text``, so neither can diverge from the other. + + This test pins the ordering that makes settled behave that way, and pins that + the interceptor is installed on ``add_text`` rather than on a text sink that + skips code. Changing either without a deliberate policy decision breaks it. + """ + body_start = UI_JS.index("function renderMd(") + body = UI_JS[body_start:body_start + 40000] + media_stash = body.find("MEDIA: token stash") + fence_stash = body.find("Fence stash:") + assert media_stash != -1 and fence_stash != -1, ( + "renderMd no longer has the expected MEDIA/fence stash passes" + ) + assert media_stash < fence_stash, ( + "MEDIA must be stashed BEFORE fence protection, or settled rendering " + "silently switches to literal-code semantics while streaming keeps " + "rendering cards" + ) + # Streaming: the interceptor wraps add_text, which is what receives fenced + # content — so both stream renderers inherit one policy. + assert "renderer.add_text=" in MESSAGES_JS + assert "_smdMediaAwareAddText(baseAddText" in MESSAGES_JS diff --git a/tests/test_media_grammar_cross_language.py b/tests/test_media_grammar_cross_language.py new file mode 100644 index 00000000000..fe88f92e03a --- /dev/null +++ b/tests/test_media_grammar_cross_language.py @@ -0,0 +1,606 @@ +"""Cross-language MEDIA grammar tests (PR #6607 re-review). + +The MEDIA token grammar is implemented twice — ``media_token_pattern()`` in +api/helpers.py and ``_mediaPathSrc()`` in static/ui.js. A divergence between +them is not cosmetic: the frontend renders and requests one path while the +backend allow-list and the public-share inliner resolve a different string, so +an image the UI just displayed is denied by /api/media and replaced with a +placeholder in a share. + +These tests pin the reviewer-reported cases and assert the two implementations +agree after unquoting, which is the value each side hands to ``Path()``. +""" +from __future__ import annotations + +import re +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +if str(REPO_ROOT) not in sys.path: + sys.path.insert(0, str(REPO_ROOT)) + +from api.helpers import media_token_pattern # noqa: E402 + +SPACED = "/home/samfp/vault/Meeting Notes/2026-07-29 - SDE Focus Group.md" + +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") + + +@pytest.fixture(scope="module") +def media_re(): + return re.compile(media_token_pattern()) + + +# ── Reviewer-reported blockers (#6607 re-review) ───────────────────────────── +# Three deterministic failures at head bfc3c2d3, plus the cross-language +# agreement that the two grammars are one grammar. + + +@pytest.mark.parametrize( + "text,expected", + [ + # 1. Dotted DIRECTORY before a space. The old lazy any-extension run + # settled on `/tmp/v1.2` because the following space satisfied the + # boundary lookahead, so the path never reached `chart.png`. + ("MEDIA:/tmp/v1.2 Reports/chart.png", ["/tmp/v1.2 Reports/chart.png"]), + ("MEDIA:/tmp/v2.5 Data/final.report.png", + ["/tmp/v2.5 Data/final.report.png"]), + # 2. Explicit quoted form: the unambiguous spelling for a path holding + # spaces AND closing delimiters. Python had no quoted alternative at + # all, so this captured `"/tmp/My`. + ('MEDIA:"/tmp/My Files/report (final).png"', + ['"/tmp/My Files/report (final).png"']), + ("MEDIA:'/tmp/My Files/single.png'", + ["'/tmp/My Files/single.png'"]), + ('MEDIA:"/tmp/dir]/od[d).png"', ['"/tmp/dir]/od[d).png"']), + # Unicode and percent-sensitive characters survive. + ("MEDIA:/tmp/café 文字/图.png", ["/tmp/café 文字/图.png"]), + ('MEDIA:"/tmp/pct %20 dir/x.png"', ['"/tmp/pct %20 dir/x.png"']), + # Adjacent tags stay separate; trailing prose is not absorbed. + ("see MEDIA:/tmp/one.png and MEDIA:/tmp/two.png", + ["/tmp/one.png", "/tmp/two.png"]), + ("MEDIA:/tmp/a.png looks good to me", ["/tmp/a.png"]), + # A dotted stem still resolves whole (no known-extension allow-list). + ("MEDIA:/tmp/archive.png.bak", ["/tmp/archive.png.bak"]), + # Non-media extensions must keep working — the grammar is not restricted + # to a renderable-format list. + ("MEDIA:/tmp/My Sheets/book.xlsx", ["/tmp/My Sheets/book.xlsx"]), + ("MEDIA:/tmp/data.json", ["/tmp/data.json"]), + ], +) +def test_reviewer_reported_grammar_cases(media_re, text, expected): + assert media_re.findall(text) == expected + + +def test_unquote_media_ref_strips_one_matching_pair(): + from api.helpers import unquote_media_ref + + assert unquote_media_ref('"/tmp/a b.png"') == "/tmp/a b.png" + assert unquote_media_ref("'/tmp/a b.png'") == "/tmp/a b.png" + # Not a matching pair — leave it alone rather than corrupting the path. + assert unquote_media_ref('"/tmp/a.png') == '"/tmp/a.png' + assert unquote_media_ref("/tmp/it's.png") == "/tmp/it's.png" + assert unquote_media_ref("") == "" + + +def test_quoted_ref_is_admitted_by_the_allow_list(tmp_path, monkeypatch): + """The cross-boundary consequence of the quoted mismatch. + + static/ui.js defines AND unquotes a quoted alternative, so the frontend + requests /api/media?path=/tmp/My Files/x.png. Python had no quoted + alternative and no unquote step, so the allow-list entry was built from + `"/tmp/My` — the path the renderer asks for was never in the allow-list and + /api/media denied a file the UI had just displayed. + """ + from api import routes + + target = tmp_path / "My Files" / "report (final).png" + target.parent.mkdir(parents=True) + target.write_bytes(b"\x89PNG\r\n\x1a\n") + + class _Session: + messages = [ + {"role": "assistant", "content": f'Here you go MEDIA:"{target}" ok'} + ] + + monkeypatch.setattr(routes, "get_session", lambda sid: _Session()) + assert routes._session_media_token_allows_path( + "sess-1", target, {"image/png"} + ) is True + + +def test_quoted_ref_from_user_content_is_still_rejected(tmp_path, monkeypatch): + """Adding the quoted form must not weaken the threat model: user-authored + tokens still cannot mint allow-list entries.""" + from api import routes + + target = tmp_path / "My Files" / "secret.png" + target.parent.mkdir(parents=True) + target.write_bytes(b"\x89PNG\r\n\x1a\n") + + class _Session: + messages = [{"role": "user", "content": f'MEDIA:"{target}"'}] + + monkeypatch.setattr(routes, "get_session", lambda sid: _Session()) + assert routes._session_media_token_allows_path( + "sess-1", target, {"image/png"} + ) is False + + +# A 1x1 PNG that passes the share inliner's magic-byte and MIME checks. +# Written as a commented, chunk-by-chunk bytes literal rather than a single +# bytes.fromhex(...) blob so each PNG chunk is readable in place and the fixture +# needs no decoding step at import time. +_TINY_PNG = ( + b"\x89PNG\r\n\x1a\n" # signature + b"\x00\x00\x00\rIHDR" # IHDR chunk header + b"\x00\x00\x00\x01\x00\x00\x00\x01" # 1x1 + b"\x08\x06\x00\x00\x00" # 8-bit RGBA + b"\x1f\x15\xc4\x89" # IHDR CRC + b"\x00\x00\x00\nIDAT" # IDAT chunk header + b"x\x9cc\x00\x01\x00\x00\x05\x00\x01" # zlib-compressed single pixel + b"\r\n-\xb4" # IDAT CRC + b"\x00\x00\x00\x00IEND\xaeB`\x82" # IEND +) + + +@pytest.mark.parametrize("quoted", [True, False]) +def test_share_inliner_embeds_spaced_path(tmp_path, quoted): + """The share inliner passed the raw capture into Path(), so a spaced ref was + replaced with a placeholder in a PUBLIC share even though the file was + inside an allowed root.""" + from api import shares + + target = tmp_path / "My Files" / ("report final.png" if not quoted + else "report (final).png") + target.parent.mkdir(parents=True) + target.write_bytes(_TINY_PNG) + + ref = f'"{target}"' if quoted else str(target) + out = shares._embed_share_media(f"see MEDIA:{ref} ok", + allowed_roots=(tmp_path,)) + assert "data:image/png;base64," in out + + +def test_share_inliner_still_rejects_path_outside_allowed_roots(tmp_path): + """Quoting must not become a traversal escape hatch.""" + from api import shares + + outside = tmp_path / "outside" / "x.png" + outside.parent.mkdir(parents=True) + outside.write_bytes(_TINY_PNG) + allowed = tmp_path / "allowed" + allowed.mkdir() + + out = shares._embed_share_media(f'see MEDIA:"{outside}" ok', + allowed_roots=(allowed,)) + assert "data:image/png;base64," not in out + + +# ── External URLs carrying a nested MEDIA: keyword ─────────────────────────── +# +# The share matcher used to spell its URL exemption as `exclude_urls=True`, a +# negative lookahead at the match start. An external URL was therefore rejected +# by NOT MATCHING, and `re.sub` resumed scanning one character later — INSIDE the +# token it had just refused. If that URL's own path or query contained the +# literal `MEDIA:`, the nested occurrence matched as a fresh local token, so the +# tail of a legitimate external URL was rewritten (and local paths were probed +# from share text). The fix matches the full canonical token and classifies it in +# `_replace_ref()`. + + +@pytest.mark.parametrize( + "text,label", + [ + ( + "MEDIA:https://cdn.test/i.png?w=800&fmt=webp", + "harmless public query string", + ), + ( + "MEDIA:https://cdn.test/img/photo.png", + "ordinary public asset", + ), + ( + "MEDIA:HTTPS://cdn.test/a/photo.png", + "uppercase scheme (schemes are case-insensitive)", + ), + ( + 'MEDIA:"https://cdn.test/q/spaced name.png"', + "quoted external URL", + ), + ( + "MEDIA:https://cdn.test/i.png?next=/media/other.png", + "public query naming a non-MEDIA path", + ), + ], +) +def test_public_external_url_is_preserved_exactly(text, label): + """An exempt external token must survive byte-for-byte. + + This is the preservation half of the whole-token decision: the scanner must + never resume inside an external token and rewrite its tail, and a harmless + public query string must not be mangled. + """ + from api import shares + + out = shares._embed_share_media(text, allowed_roots=()) + assert out == text, f"{label}: external token was rewritten" + assert shares._PLACEHOLDER not in out, ( + f"{label}: a local-path placeholder was spliced into an external URL" + ) + + +@pytest.mark.parametrize( + "text,label", + [ + ( + "MEDIA:https://cdn.test/img/MEDIA:/etc/passwd.png", + "nested MEDIA: in the URL path", + ), + ( + "MEDIA:https://cdn.test/i.png?src=MEDIA:/etc/shadow.png", + "nested MEDIA: in the URL query", + ), + ( + "MEDIA:HTTPS://cdn.test/a/MEDIA:/etc/passwd.png", + "uppercase scheme with a nested local ref", + ), + ( + 'MEDIA:"https://cdn.test/q/MEDIA:/etc/passwd.png"', + "quoted external URL with a nested local ref", + ), + ( + "MEDIA:https://cdn.test/i.png#MEDIA:/etc/shadow.png", + "nested MEDIA: in the URL fragment", + ), + ( + "MEDIA:https://cdn.test/i.png?src=file:///etc/shadow.png", + "file:// smuggled in the query", + ), + ( + "MEDIA:https://cdn.test/i.png?src=%4dEDIA:/etc/shadow.png", + "percent-encoded MEDIA: (single decode)", + ), + ( + "MEDIA:https://cdn.test/i.png?src=%254dEDIA:/etc/shadow.png", + "double-encoded MEDIA: (bounded multi-decode)", + ), + ( + "MEDIA:http://127.0.0.1:8080/api/media?path=/home/u/.ssh/id_rsa", + "loopback host reaching the authenticated media route", + ), + ( + "MEDIA:http://localhost:8080/api/media?path=/etc/shadow", + "localhost by name", + ), + ( + "MEDIA:http://192.168.1.5/api/media?path=/etc/shadow", + "RFC 1918 private host", + ), + ( + "MEDIA:http://10.0.0.7/x.png", + "RFC 1918 10/8 host", + ), + ( + "MEDIA:http://172.16.0.9/x.png", + "RFC 1918 172.16/12 host", + ), + ( + "MEDIA:https://cdn.test/api/media?path=/etc/shadow", + "public host but our authenticated media route", + ), + ], +) +def test_external_url_hiding_a_local_target_is_placeholdered(text, label): + """The whole token is rejected when an http(s) URL smuggles a local target. + + `is_external_media_url()` only sees the scheme, so these all used to be + preserved byte-for-byte into an anonymous snapshot. The share renderer + restores a preserved token into an image URL, so each of these either + round-trips a host path into the published share or makes the viewer's + browser issue a same-origin `/api/media` request. + + The whole token must become the placeholder — never a partial rewrite, which + is what let the scanner resume inside a refused token. + """ + from api import shares + + out = shares._embed_share_media(text, allowed_roots=()) + assert out == shares._PLACEHOLDER, ( + f"{label}: expected the whole token to be placeholdered, got {out!r}" + ) + # No fragment of the smuggled local target may survive anywhere. + for leaked in ("etc/passwd", "etc/shadow", "id_rsa", "api/media"): + assert leaked not in out, f"{label}: {leaked!r} leaked into the share" + + +def test_hidden_local_target_rejection_does_not_restart_mid_token(): + """Rejecting a token must consume the WHOLE span, not resume inside it. + + If the refusal left the scanner mid-token, the nested `MEDIA:/etc/shadow.png` + would match as a fresh LOCAL token and get its own placeholder — producing + two placeholders and proving the scan restarted inside a refused span. + """ + from api import shares + + text = "before MEDIA:https://cdn.test/i.png?src=MEDIA:/etc/shadow.png after" + out = shares._embed_share_media(text, allowed_roots=()) + assert out.count(shares._PLACEHOLDER) == 1, ( + f"expected exactly one placeholder for one token, got {out!r}" + ) + assert out == f"before {shares._PLACEHOLDER} after" + + +def test_local_path_inside_allowed_root_still_embeds(tmp_path): + """Positive control: dropping the pattern-level guard must not stop embedding.""" + from api import shares + + target = tmp_path / "shot.png" + target.write_bytes(_TINY_PNG) + + out = shares._embed_share_media(f"see MEDIA:{target} ok", allowed_roots=(tmp_path,)) + assert "data:image/png;base64," in out + + +@pytest.mark.parametrize( + "ref,label", + [ + ("file:///etc/passwd.png", "file:// is always rejected"), + ("/etc/passwd.png", "absolute path outside every allowed root"), + ], +) +def test_local_negative_controls_still_placeholdered(ref, label): + """Negative controls: local refs must still never leak into a share.""" + from api import shares + + out = shares._embed_share_media(f"MEDIA:{ref}", allowed_roots=()) + assert shares._PLACEHOLDER in out, f"{label}: expected a placeholder" + assert ref not in out, f"{label}: the local path leaked into the share" + + +# ── One grammar, two languages ─────────────────────────────────────────────── + + +def _js_media_capture_and_remainder(cases: list[str]) -> list[list[str | None]]: + """Return ``[capture, exact remainder]`` per case from the REAL JS grammar.""" + import json + import shutil + import subprocess + + node = shutil.which("node") + if not node: + pytest.skip("node not available") + + def extract(src: str, name: str) -> str: + start = src.index(f"function {name}(") + depth = 0 + started = False + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + started = True + elif src[i] == "}": + depth -= 1 + if started and depth == 0: + return src[start:i + 1] + raise AssertionError(f"unbalanced braces extracting {name}") + + script = "\n".join([ + extract(UI_JS, "_mediaPathSrc"), + extract(UI_JS, "_mediaTokenRe"), + "const cases = JSON.parse(process.argv[1]);", + "const out = cases.map((s) => {", + " const re = _mediaTokenRe();", + " const m = re.exec(s);", + " return m ? [m[1], s.slice(m.index + m[0].length)] : [null, null];", + "});", + "console.log(JSON.stringify(out));", + ]) + proc = subprocess.run( + [node, "--input-type=module", "-e", script, json.dumps(cases)], + capture_output=True, text=True, timeout=30, check=True, + ) + return json.loads(proc.stdout) + + +# Terminal punctuation ownership. `(text, expected capture, exact remainder)` — +# fixed expectations, NOT a mirrored comparison, so the table pins the intended +# token/remainder split rather than merely proving the two sides agree. +# +# Sentence punctuation is only prose when a REAL delimiter follows it. At +# absolute end-of-input the token keeps the character on purpose: a streaming +# chunk can stop mid-token, and splitting there would capture `/tmp/a` out of +# `MEDIA:/tmp/a.png` on the final chunk and desync the streamed and settled +# renderings. Each sentence case below is therefore written with following text. +_PUNCTUATION_OWNERSHIP = [ + ("see MEDIA:/tmp/a.png. Next", "/tmp/a.png", ". Next"), + ("see MEDIA:/tmp/a.png! Next", "/tmp/a.png", "! Next"), + ("see MEDIA:/tmp/a.png? Next", "/tmp/a.png", "? Next"), + # A newline is a delimiter too, so a period at end-of-line is still prose. + ("see MEDIA:/tmp/a.png.\nNext", "/tmp/a.png", ".\nNext"), + # A dot genuinely inside the name is still part of the ref. + ("MEDIA:/tmp/a.tar.gz ok", "/tmp/a.tar.gz", " ok"), + ("MEDIA:/tmp/v1.2/chart.png. Next", "/tmp/v1.2/chart.png", ". Next"), + # Real query punctuation stays INSIDE an external URL. + ("MEDIA:https://x.test/i.png?w=1&h=2", "https://x.test/i.png?w=1&h=2", ""), + ("MEDIA:https://x.test/a.png?q=1. Next", "https://x.test/a.png?q=1", ". Next"), + ("MEDIA:HTTPS://x.test/a.png", "HTTPS://x.test/a.png", ""), + ("MEDIA:https://fal.media/generated", "https://fal.media/generated", ""), + # End-of-input keeps the character: the stream may simply stop here. + ("MEDIA:/tmp/a.png", "/tmp/a.png", ""), + # Pre-existing contract that already worked; must not regress. + ("MEDIA:/tmp/a.png, next", "/tmp/a.png", ", next"), + ("MEDIA:/tmp/noext. Next", "/tmp/noext", ". Next"), + ('MEDIA:"/tmp/a b.png".', '"/tmp/a b.png"', "."), + ("MEDIA:/tmp/v1.2 Reports/chart.png done", "/tmp/v1.2 Reports/chart.png", " done"), + ("MEDIA:/a.png MEDIA:/b.png", "/a.png", " MEDIA:/b.png"), + ("MEDIA:C:/tmp/live.png ", "C:/tmp/live.png", " "), + # Windows-style drive path must survive intact (`:` closes a token, so a lazy + # form would truncate this to `C`). + ("MEDIA:C:/tmp/live.png. Next", "C:/tmp/live.png", ". Next"), +] + + +@pytest.mark.parametrize("text,expected_capture,expected_remainder", _PUNCTUATION_OWNERSHIP) +def test_python_terminal_punctuation_ownership(text, expected_capture, expected_remainder): + """Python: sentence punctuation is prose; URL query punctuation is the ref.""" + import re as _re + + from api.helpers import media_token_pattern + + m = _re.compile(media_token_pattern()).search(text) + assert m is not None, f"no match for {text!r}" + assert m.group(1) == expected_capture + assert text[m.end():] == expected_remainder + + +def test_js_terminal_punctuation_ownership_matches_python(): + """JS must make the SAME token/remainder split as Python, case by case.""" + cases = [text for text, _, _ in _PUNCTUATION_OWNERSHIP] + results = _js_media_capture_and_remainder(cases) + for (text, expected_capture, expected_remainder), (capture, remainder) in zip( + _PUNCTUATION_OWNERSHIP, results, strict=True + ): + assert capture == expected_capture, f"JS capture diverged for {text!r}" + assert remainder == expected_remainder, f"JS remainder diverged for {text!r}" + + +# Ambiguous unquoted spaced refs. If the first word already ends in a filename +# extension, it owns the token and ordinary prose after it must remain prose. +# Dotted-directory support is still unambiguous when the continuation contains +# a path separator (`/tmp/v1.2 Reports/chart.png`). A genuinely ambiguous spaced +# filename can use the already-supported quoted form. +_SPACED_AMBIGUITY_OWNERSHIP = [ + ( + "MEDIA:/tmp/a.png see README.md", + "/tmp/a.png", + " see README.md", + ), + ( + "MEDIA:/tmp/a.png see README.md next", + "/tmp/a.png", + " see README.md next", + ), + ( + "MEDIA:/tmp/v1.2 Reports/chart.png", + "/tmp/v1.2 Reports/chart.png", + "", + ), + ( + "MEDIA:/tmp/My Files/final report.png done", + "/tmp/My Files/final report.png", + " done", + ), +] + + +@pytest.mark.parametrize("text,expected_capture,expected_remainder", _SPACED_AMBIGUITY_OWNERSHIP) +def test_python_spaced_path_does_not_absorb_dotted_prose( + text, expected_capture, expected_remainder +): + """A complete first filename wins over later dot-bearing prose.""" + import re as _re + + from api.helpers import media_token_pattern + + m = _re.compile(media_token_pattern()).search(text) + assert m is not None, f"no match for {text!r}" + assert m.group(1) == expected_capture + assert text[m.end():] == expected_remainder + + +def test_js_spaced_path_does_not_absorb_dotted_prose(): + """JavaScript applies the same fixed capture/remainder contract.""" + cases = [text for text, _, _ in _SPACED_AMBIGUITY_OWNERSHIP] + results = _js_media_capture_and_remainder(cases) + for (text, expected_capture, expected_remainder), (capture, remainder) in zip( + _SPACED_AMBIGUITY_OWNERSHIP, results, strict=True + ): + assert capture == expected_capture, f"JS capture diverged for {text!r}" + assert remainder == expected_remainder, f"JS remainder diverged for {text!r}" + + +def _js_media_captures(cases: list[str]) -> list[str | None]: + """Run the JS grammar over *cases* under node, returning unquoted captures.""" + import json + import shutil + import subprocess + + node = shutil.which("node") + if not node: + pytest.skip("node not available") + + def extract(src: str, name: str) -> str: + start = src.index(f"function {name}(") + depth = 0 + started = False + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + started = True + elif src[i] == "}": + depth -= 1 + if started and depth == 0: + return src[start:i + 1] + raise AssertionError(f"unbalanced braces extracting {name}") + + script = "\n".join([ + extract(UI_JS, "_mediaPathSrc"), + extract(UI_JS, "_unquoteMediaRef"), + "const cases = JSON.parse(process.argv[1]);", + "const out = cases.map((s) => {", + " const re = new RegExp(String.raw`MEDIA:(${_mediaPathSrc()})`);", + " const m = re.exec(s);", + " return m ? _unquoteMediaRef(m[1]) : null;", + "});", + "console.log(JSON.stringify(out));", + ]) + proc = subprocess.run( + [node, "--input-type=module", "-e", script, json.dumps(cases)], + capture_output=True, text=True, timeout=30, check=True, + ) + return json.loads(proc.stdout) + + +def test_python_and_js_grammars_agree(media_re): + """The two implementations must be ONE grammar. + + Compared AFTER unquoting, because that is the value each side actually hands + to Path() / the /api/media request — the layer where a divergence became a + denied image and a placeholdered share. + """ + from api.helpers import unquote_media_ref + + cases = [ + "MEDIA:/tmp/plain.png", + "MEDIA:/tmp/archive.png.bak", + "MEDIA:/tmp/v1.2 Reports/chart.png", + 'MEDIA:"/tmp/My Files/report (final).png"', + "MEDIA:'/tmp/My Files/single.png'", + 'MEDIA:"/tmp/dir]/od[d).png"', + "MEDIA:/tmp/no-ext-file", + "MEDIA:/tmp/a.png trailing prose", + "MEDIA:/tmp/café 文字/图.png", + 'MEDIA:"/tmp/pct %20 dir/x.png"', + "see MEDIA:/tmp/one.png and MEDIA:/tmp/two.png", + "MEDIA:/tmp/v2.5 Data/final.report.png", + f"MEDIA:{SPACED}", + "MEDIA:/tmp/My Files/x.md\nnext line", + "MEDIA:/tmp/Caddyfile", + "(MEDIA:/tmp/a b.png)", + "MEDIA:/tmp/data.json", + "MEDIA:/tmp/My Sheets/book.xlsx", + ] + + js = _js_media_captures(cases) + for text, js_capture in zip(cases, js, strict=True): + m = media_re.search(text) + py_capture = unquote_media_ref(m.group(1)) if m else None + assert py_capture == js_capture, ( + f"grammar divergence for {text!r}: " + f"python={py_capture!r} js={js_capture!r}" + ) diff --git a/tests/test_media_spaced_paths.py b/tests/test_media_spaced_paths.py new file mode 100644 index 00000000000..0fa79fb84ac --- /dev/null +++ b/tests/test_media_spaced_paths.py @@ -0,0 +1,195 @@ +"""Spaced MEDIA: paths must resolve end-to-end (renderer, allow-list, shares). + +A MEDIA path may legitimately contain spaces: + + MEDIA:/home/u/vault/Meeting Notes/2026-07-29 - SDE Focus Group.md + +Every MEDIA parser used to capture the path with a ``[^\\s)\\]]+`` class, which +stops at the first space. Three surfaces were affected by the same class of bug: + + 1. static/ui.js renderMd() -> artifact card built from a truncated + path (wrong basename) and the tail + leaked into the bubble as raw prose + 2. api/routes.py allow-list -> truncated capture never matches the real + on-disk path, so /api/media denies a + legitimate assistant-emitted artifact + 3. api/shares.py inliner -> same truncation, so a public share + silently fails to embed the file + +Widening is bounded on purpose: space tolerance must not swallow trailing prose +or glue an adjacent MEDIA: tag into one invalid path. Those adversarial cases +are asserted here alongside the spaced-path fix so a future widening cannot +regress them. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +if str(REPO_ROOT) not in sys.path: + sys.path.insert(0, str(REPO_ROOT)) + +from api.helpers import media_token_pattern # noqa: E402 + +SPACED = "/home/samfp/vault/Meeting Notes/2026-07-29 - SDE Focus Group.md" + + +# ── The shared Python pattern ──────────────────────────────────────────────── + + +@pytest.fixture(scope="module") +def media_re(): + return re.compile(media_token_pattern()) + + +@pytest.fixture(scope="module") +def share_re(): + return re.compile(media_token_pattern(extra_exclude=">", exclude_urls=True)) + + +def test_spaced_path_captured_whole(media_re): + """The reported bug: capture must not stop at the first space.""" + assert media_re.findall(f"here you go\nMEDIA:{SPACED}\n") == [SPACED] + + +def test_spaced_path_captured_whole_in_share(share_re): + assert share_re.findall(f"MEDIA:{SPACED}") == [SPACED] + + +@pytest.mark.parametrize( + "text,expected", + [ + # Bounded: trailing prose is NOT absorbed into the path. + ("MEDIA:/tmp/a.png looks good to me", ["/tmp/a.png"]), + # Bounded: two tags stay two tags (a greedy widening merges them). + ("MEDIA:/tmp/a.png MEDIA:/tmp/b.png", ["/tmp/a.png", "/tmp/b.png"]), + # Delimiters still terminate the path. + ("(MEDIA:/tmp/a b.png)", ["/tmp/a b.png"]), + # A newline always ends a MEDIA token. + ("MEDIA:/tmp/My Files/x.md\nnext line", ["/tmp/My Files/x.md"]), + # Extension-less paths kept working (no-space fallback branch). + ("MEDIA:/tmp/Caddyfile", ["/tmp/Caddyfile"]), + ], +) +def test_widening_stays_bounded(media_re, text, expected): + assert media_re.findall(text) == expected + + +def test_share_pattern_skips_http_urls(share_re): + """External images pass through the share inliner untouched.""" + assert share_re.findall("MEDIA:https://example.test/a.png") == [] + + +def test_share_pattern_stops_at_angle_bracket(share_re): + """'>' terminates the path so an inlined tag can't be swallowed.""" + assert share_re.findall("MEDIA:/tmp/a.png>trailing") == ["/tmp/a.png"] + + +def test_allow_list_pattern_still_matches_urls(media_re): + """routes.py filters URLs by '://' after matching, so it must capture them.""" + assert media_re.findall("MEDIA:https://example.test/a.png") == [ + "https://example.test/a.png" + ] + + +# ── The /api/media allow-list, end-to-end ─────────────────────────────────── +# This is the behavioral half: the truncated capture made the allow-list DENY a +# real assistant-emitted artifact whose path contains a space. Asserting the +# regex alone would not catch a future change that resolves the path +# differently, so drive the real predicate. + + +def test_allow_list_admits_assistant_spaced_path(tmp_path, monkeypatch): + from api import routes + + target = tmp_path / "Meeting Notes" / "2026-07-29 - SDE Focus Group.md" + target.parent.mkdir(parents=True) + target.write_text("# notes", encoding="utf-8") + + class _Session: + messages = [ + {"role": "assistant", "content": f"Wrote it up.\nMEDIA:{target}\n"} + ] + + monkeypatch.setattr(routes, "get_session", lambda sid: _Session()) + monkeypatch.setitem(routes.MIME_MAP, ".md", "text/markdown") + + assert routes._session_media_token_allows_path( + "sess-1", target, {"text/markdown"} + ) is True + + +def test_allow_list_still_rejects_user_authored_token(tmp_path, monkeypatch): + """User content must not be able to mint allow-list entries (threat model).""" + from api import routes + + target = tmp_path / "My Files" / "secret.md" + target.parent.mkdir(parents=True) + target.write_text("nope", encoding="utf-8") + + class _Session: + messages = [{"role": "user", "content": f"MEDIA:{target}"}] + + monkeypatch.setattr(routes, "get_session", lambda sid: _Session()) + monkeypatch.setitem(routes.MIME_MAP, ".md", "text/markdown") + + assert routes._session_media_token_allows_path( + "sess-1", target, {"text/markdown"} + ) is False + + +def test_allow_list_rejects_unrelated_path(tmp_path, monkeypatch): + """A spaced token must not widen into admitting a *different* file.""" + from api import routes + + mentioned = tmp_path / "Meeting Notes" / "a.md" + mentioned.parent.mkdir(parents=True) + mentioned.write_text("ok", encoding="utf-8") + other = tmp_path / "Meeting Notes" / "b.md" + other.write_text("other", encoding="utf-8") + + class _Session: + messages = [{"role": "assistant", "content": f"MEDIA:{mentioned}"}] + + monkeypatch.setattr(routes, "get_session", lambda sid: _Session()) + monkeypatch.setitem(routes.MIME_MAP, ".md", "text/markdown") + + assert routes._session_media_token_allows_path( + "sess-1", other, {"text/markdown"} + ) is False + + +# ── The browser renderer ──────────────────────────────────────────────────── +# ui.js owns the MEDIA shape for the frontend; messages.js reuses it so the +# streamed and settled renderings of one token stay identical. Assert the shared +# helper exists and that no surface kept a private copy of the old class. + +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") + + +def test_frontend_media_shape_is_shared(): + assert "function _mediaPathSrc" in UI_JS + assert "function _mediaTokenRe" in UI_JS + # messages.js must consume the shared helpers, not re-declare the pattern. + assert "_mediaTokenRe()" in MESSAGES_JS + # The anchored single-token matcher was removed: the stream-end flush now + # partitions the buffered candidate with the shared global matcher, because a + # candidate can be a token PLUS same-line prose. + + +def test_no_surface_kept_the_truncating_class(): + """The old first-space-truncating capture must be gone everywhere.""" + old = r"MEDIA:([^\s\)\]]+)" + for name, src in ( + ("static/ui.js", UI_JS), + ("static/messages.js", MESSAGES_JS), + ("api/routes.py", (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8")), + ("api/shares.py", (REPO_ROOT / "api" / "shares.py").read_text(encoding="utf-8")), + ): + assert old not in src, f"{name} still uses the truncating MEDIA capture" diff --git a/tests/test_media_stream_candidate_ceiling.py b/tests/test_media_stream_candidate_ceiling.py new file mode 100644 index 00000000000..078c0069d12 --- /dev/null +++ b/tests/test_media_stream_candidate_ceiling.py @@ -0,0 +1,428 @@ +"""One inclusive candidate ceiling in the streaming walk (PR #6607 re-review). + +Re-review blocker: ``_smdMediaAwareAddText()`` carried TWO ceilings. + +* The matched-token branch bounded the buffered candidate at + ``_mediaTokenMaxLength() + _SMD_MEDIA_PREFIX.length`` — 4096 + 6 = 4102. +* The no-match / open-quote tail branch bounded it at a separate + ``_MEDIA_TAIL_MAX`` of 4096. + +A LEGAL quoted capture whose opening quote crosses a chunk boundary reaches the +second branch, because the complete quoted grammar has not matched yet. At +candidate length 4096 the stream flushed it as literal text and lost quote +ownership, so a later closing quote could not reassemble it — while settled +``renderMd()`` still accepted the identical capture through 4096 characters. +Streamed and settled output disagreed about one reference. + +Measured at the unmodified head, driving the real ``_smdMediaAwareAddText``: a +4096-code-unit quoted capture lost its card entirely (``[]`` instead of one +media node) at chunk cuts 4100 through 4104, and the whole 4108-character span +was written as prose. + +The fix uses ONE inclusive ceiling in every branch, keeps explicit quote and +owner state across chunks, and fails an actually over-limit reference CLOSED so +its suffix cannot activate on its own. + +Harness discipline follows tests/test_media_stream_settled_equality.py: the +entire production call chain is extracted from the shipped source and only the +leaf sinks are stubbed. No decision function is retyped, so a rename fails loudly +instead of leaving a mirror passing. +""" +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path +from typing import Any + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +if str(REPO_ROOT) not in sys.path: + sys.path.insert(0, str(REPO_ROOT)) + +from api.helpers import ( # noqa: E402 + MEDIA_TOKEN_MAX_LENGTH, + media_token_length, +) + +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") + +CAP = MEDIA_TOKEN_MAX_LENGTH +PREFIX = "MEDIA:" +# The one candidate ceiling: `MEDIA:` plus a legal capture, inclusive. +CANDIDATE_MAX = CAP + len(PREFIX) + +_UI_FUNCS = [ + "_mediaPathSrc", + "_mediaTokenRe", + "_unquoteMediaRef", + "_mediaTokenMaxLength", + "_mediaTokenExceedsMaxLength", +] +_MSG_FUNCS = [ + "_smdMediaPrefixTail", + "_smdMediaTailEntryChunk", + "_smdMediaTailSameOwner", + "_smdMediaTailSet", + "_smdMediaTokenIsSettled", + "_smdMediaTailCouldExtend", + "_smdMediaHasOpenQuote", + "_smdMediaOpenQuoteChar", + "_smdMediaRefuseLine", + "_smdMediaEntryRefused", + "_smdMediaRunChar", + "_smdMediaRefusedRunLength", + "_smdMediaCandidateMax", + "_smdMediaTailFlushEntry", + "_smdMediaTailFlush", + "_smdMediaAwareAddText", # the function under test +] + + +def _extract_js_function(src: str, name: str) -> str: + start = src.index(f"function {name}(") + depth = 0 + started = False + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + started = True + elif src[i] == "}": + depth -= 1 + if started and depth == 0: + return src[start:i + 1] + raise AssertionError(f"unbalanced braces extracting {name}") + + +_HARNESS = r""" +import {readFileSync} from 'node:fs'; +const _SMD_MEDIA_PREFIX = 'MEDIA:'; +const _SMD_MEDIA_TAIL = new Map(); + +let events = []; +// Leaf sinks: the ONLY stubs. +function _smdAppendMediaNode(parent, rawRef){ events.push({kind:'MEDIA', v:rawRef}); return true; } +function _smdMediaWriteText(parent, data, baseAddText, writeText, chunk){ + if (chunk !== '' && chunk != null) events.push({kind:'TEXT', v:String(chunk)}); +} + +// The settled contract, mirroring renderMd()'s MEDIA stash in static/ui.js: an +// over-ceiling capture is not a legal token, so its whole span stays prose. +function settledEvents(text){ + const re = _mediaTokenRe(); + const out = []; + let m, last = 0; + while ((m = re.exec(text))){ + if (_mediaTokenExceedsMaxLength(m[1])) continue; + if (m.index > last) out.push({kind:'TEXT', v:text.slice(last, m.index)}); + out.push({kind:'MEDIA', v:_unquoteMediaRef(m[1])}); + last = m.index + m[0].length; + } + if (last < text.length) out.push({kind:'TEXT', v:text.slice(last)}); + return out; +} + +function streamedEvents(chunks){ + events = []; + _SMD_MEDIA_TAIL.clear(); + const parser = {id:'p1'}, parent = {id:'root'}; + for (const chunk of chunks){ + _smdMediaAwareAddText(null, parent, {}, chunk, _SMD_MEDIA_TAIL, parser, null); + } + _smdMediaTailFlush(parser); + return events.slice(); +} + +const mediaOf = (e) => e.filter(x => x.kind === 'MEDIA').map(x => x.v); +const textOf = (e) => e.filter(x => x.kind === 'TEXT').map(x => x.v).join(''); + +const payload = JSON.parse(readFileSync(process.argv[1], 'utf8')); +const out = []; +for (const row of payload){ + const want = settledEvents(row.text); + const wantMedia = mediaOf(want), wantText = textOf(want); + // `cuts` null means EVERY cut position; otherwise the listed ones. + const cuts = row.cuts + || Array.from({length: row.text.length - 1}, (_, i) => i + 1); + const bad = []; + for (const cut of cuts){ + const got = streamedEvents([row.text.slice(0, cut), row.text.slice(cut)]); + const gm = mediaOf(got), gt = textOf(got); + if (JSON.stringify(gm) !== JSON.stringify(wantMedia) || gt !== wantText){ + bad.push({cut, gotMedia: gm, gotTextLen: gt.length, gotTextTail: gt.slice(-40)}); + } + } + out.push({ + label: row.label, + textLen: row.text.length, + cutsChecked: cuts.length, + wantMedia, wantTextLen: wantText.length, wantText, + badCount: bad.length, bad: bad.slice(0, 5), + }); +} +console.log(JSON.stringify(out)); +""" + + +def _run(rows: list[dict]) -> Any: + node = shutil.which("node") + if not node: + pytest.skip("node not available") + script = "\n".join( + [_extract_js_function(UI_JS, n) for n in _UI_FUNCS] + + [_extract_js_function(MESSAGES_JS, n) for n in _MSG_FUNCS] + + [_HARNESS] + ) + # Payload via a temp file: a 4KB ref in argv overflows execve (Errno 7). + with tempfile.NamedTemporaryFile( + "w", suffix=".json", encoding="utf-8", delete=False + ) as handle: + json.dump(rows, handle) + path = handle.name + try: + proc = subprocess.run( + [node, "--input-type=module", "-e", script, path], + capture_output=True, text=True, timeout=600, check=True, + ) + finally: + Path(path).unlink(missing_ok=True) + return {row["label"]: row for row in json.loads(proc.stdout)} + + +# ── Fixtures: references whose OWN length crosses the ceiling ──────────────── + + +def _quoted_ref(capture_units: int) -> str: + """A quoted capture measuring exactly *capture_units* code units. + + The capture INCLUDES its quotes — that is the span the ceiling measures and + the span the streaming buffer holds. + """ + head, tail = "/tmp/", ".png" + body = capture_units - 2 - len(head) - len(tail) + assert body > 0, capture_units + ref = '"' + head + ("y" * body) + tail + '"' + assert media_token_length(ref) == capture_units + return ref + + +def _bare_ref(capture_units: int) -> str: + head, tail = "/tmp/", ".png" + body = capture_units - len(head) - len(tail) + assert body > 0, capture_units + ref = head + ("y" * body) + tail + assert media_token_length(ref) == capture_units + return ref + + +# 4095 / 4096 / 4097 — the exact rows the re-review asked for. Quoted AND bare, +# because only the quoted shape reaches the branch that carried the wrong ceiling. +_BOUNDARY_ROWS = [ + (f"{shape} capture {CAP + off}", f"see MEDIA:{build(CAP + off)} ok", off <= 0) + for shape, build in (("quoted", _quoted_ref), ("bare", _bare_ref)) + for off in (-1, 0, 1) +] + + +@pytest.fixture(scope="module") +def boundary_result() -> Any: + """EVERY chunk cut, for every boundary row. + + Exhaustive rather than sampled: the head's divergence sat at cuts 4100-4104, + a five-position window inside a 4108-character string, and a sampled sweep + walks straight past it. + """ + return _run([ + {"label": label, "text": text, "cuts": None} + for label, text, _ in _BOUNDARY_ROWS + ]) + + +@pytest.mark.parametrize( + "label,text,legal", _BOUNDARY_ROWS, ids=[r[0] for r in _BOUNDARY_ROWS] +) +def test_streamed_equals_settled_at_every_cut_across_the_ceiling( + label, text, legal, boundary_result +): + """Captures AND exact remaining prose, at every possible chunk boundary.""" + row = boundary_result[label] + assert row["cutsChecked"] > CAP, ( + f"{label}: sweep too small ({row['cutsChecked']} cuts)" + ) + assert row["badCount"] == 0, ( + f"{label}: streamed rendering diverged from settled at " + f"{row['badCount']} of {row['cutsChecked']} cuts; first few: {row['bad']}" + ) + # Non-vacuity: the row must actually be on the side of the boundary it claims. + assert (len(row["wantMedia"]) == 1) is legal, ( + f"{label}: fixture is on the wrong side of the ceiling " + f"(settled media={row['wantMedia'][:1]})" + ) + + +def test_quoted_capture_split_at_its_opening_quote_keeps_its_card(): + """The named blocker: opening quote in one chunk, closing quote in another. + + This is the shape that reached the wrongly-bounded branch. Asserted on the + exact captured token AND the exact remaining prose, at the cuts the head + actually failed on (4100-4104) plus the structural cut right after the + opening quote. + """ + ref = _quoted_ref(CAP) + text = f"see MEDIA:{ref} ok" + open_quote = text.index('"') + rows = [{ + "label": "quoted split at open quote", + "text": text, + "cuts": [open_quote + 1, open_quote + 2, 4100, 4101, 4102, 4103, 4104], + }] + row = _run(rows)["quoted split at open quote"] + + # The whole point: one card, and the prose either side of it, unchanged. + assert row["wantMedia"] == [ref.strip('"')], "settled must render one card" + assert row["wantText"] == "see ok" + assert row["badCount"] == 0, ( + "a legal quoted capture whose opening quote crossed a chunk boundary " + f"diverged from settled output: {row['bad']}" + ) + + +def test_over_limit_reference_fails_closed_without_activating_its_suffix(): + """An over-limit reference must not let its own suffix become a token. + + An over-cap external URL is tempered-greedy and swallows a nested ``MEDIA:``, + so settled parsing sees ONE over-limit span and renders no card. At the head + the stream flushed the truncated prefix and then matched the nested token in + the next chunk, producing a card that settled output does not have. + + The companion row proves the refusal is not over-broad: a genuinely + INDEPENDENT reference after a delimiter is still legal and must still render. + """ + swallowed = "MEDIA:https://h/" + ("y" * 4200) + "MEDIA:/tmp/ok.png" + independent = "MEDIA:/tmp/" + ("y" * 4200) + " and MEDIA:/tmp/ok.png" + cuts = [20, 4090, 4096, 4100, 4102, 4103, 4110] + result = _run([ + {"label": "over-cap url swallows nested token", "text": swallowed, "cuts": cuts}, + {"label": "over-cap ref then independent token", "text": independent, "cuts": cuts}, + ]) + + swallow = result["over-cap url swallows nested token"] + assert swallow["wantMedia"] == [], ( + "settled parsing must render no card for one over-limit span" + ) + assert swallow["badCount"] == 0, ( + f"the refused reference's suffix activated on its own: {swallow['bad']}" + ) + + later = result["over-cap ref then independent token"] + assert later["wantMedia"] == ["/tmp/ok.png"], ( + "an independent later reference is legal and settled renders it" + ) + assert later["badCount"] == 0, ( + f"failing closed swallowed an independent later reference: {later['bad']}" + ) + + +def test_candidate_ceiling_is_inclusive_and_derived_not_a_literal(): + """The ceiling is ONE number, computed from the shared capture ceiling. + + Driven through the real function rather than asserted on source text, so a + literal 4102 reappearing anywhere would still be caught by the boundary + sweeps above. + """ + script = "\n".join( + [_extract_js_function(UI_JS, n) for n in ("_mediaTokenMaxLength",)] + + [ + "const _SMD_MEDIA_PREFIX = 'MEDIA:';", + _extract_js_function(MESSAGES_JS, "_smdMediaCandidateMax"), + "console.log(JSON.stringify({max:_smdMediaCandidateMax()}));", + ] + ) + node = shutil.which("node") + if not node: + pytest.skip("node not available") + proc = subprocess.run( + [node, "--input-type=module", "-e", script], + capture_output=True, text=True, timeout=60, check=True, + ) + assert json.loads(proc.stdout)["max"] == CANDIDATE_MAX == 4102 + + +def test_refused_run_length_stops_where_the_grammar_stops(): + """The refusal owns the reference, not the rest of the turn. + + ``_smdMediaRefusedRunLength`` decides how much of a later chunk still belongs + to a refused reference. It must end at a token-closing character (unquoted) or + after the closing quote (quoted), or a refused reference would swallow an + independent one. Driven through the real function. + """ + script = "\n".join( + [_extract_js_function(UI_JS, n) for n in ("_mediaPathSrc", "_mediaTokenRe")] + + [ + _extract_js_function(MESSAGES_JS, "_smdMediaRunChar"), + _extract_js_function(MESSAGES_JS, "_smdMediaRefusedRunLength"), + r""" +const cases = [ + ['yyy and more', '', 3], // unquoted: stops at the space + ['yyy', '', 3], // runs to the end of the chunk + ['yy)tail', '', 2], // stops at a closing delimiter + ['yy\nnext', '', 2], // a newline ends any token + ['yyy" ok', '"', 4], // quoted: through the closing quote + ['yyy ok" x', '"', 7], // quoted spans a space to reach its quote + ['yyy no close', '"', 12], // never closes: owns the chunk + ['yy\nnext', '"', 2], // unclosed quote still stops at a newline +]; +const out = cases.map(([text, quote, want]) => ({ + text, quote, want, got: _smdMediaRefusedRunLength(text, quote), +})); +console.log(JSON.stringify(out)); +""", + ] + ) + node = shutil.which("node") + if not node: + pytest.skip("node not available") + proc = subprocess.run( + [node, "--input-type=module", "-e", script], + capture_output=True, text=True, timeout=60, check=True, + ) + for row in json.loads(proc.stdout): + assert row["got"] == row["want"], row + + +def test_existing_flush_before_clear_behaviour_is_preserved(): + """The main, anchor, safe, and fade paths still flush before clearing. + + A refused-line marker travels through the same per-parser map as a buffered + tail, so it must not disturb owner handling. Pinned on the wiring, with the + behaviour itself exercised by the sweeps above and by + tests/test_smd_media_in_stream.py. + """ + block = _extract_js_function(MESSAGES_JS, "_smdMediaAwareAddText") + # Foreign-owner tails are flushed through their original writer, not merged. + assert "_smdMediaTailFlushEntry(leadEntry)" in block + assert "_smdMediaTailSameOwner(leadEntry, parent, baseAddText, writeText)" in block + # The refused marker is owner-checked exactly like a real tail. + refuse = _extract_js_function(MESSAGES_JS, "_smdMediaRefuseLine") + for field in ("parent", "baseAddText", "writeText"): + assert field in refuse, f"the refusal marker must carry {field}" + assert "refused:true" in refuse + + +def test_harness_stubs_only_sinks(): + """Meta-test: every function in the real call chain is extracted, not retyped.""" + for name in _MSG_FUNCS: + assert f"function {name}(" in MESSAGES_JS, ( + f"{name} vanished from messages.js — the harness would silently stop " + f"testing production" + ) + for name in _UI_FUNCS + _MSG_FUNCS: + assert f"function {name}(" not in _HARNESS, ( + f"harness reimplements {name} — extract it from production instead" + ) + assert "_smdAppendMediaNode" in _HARNESS and "_smdMediaWriteText" in _HARNESS diff --git a/tests/test_media_stream_settled_equality.py b/tests/test_media_stream_settled_equality.py new file mode 100644 index 00000000000..692fb3da348 --- /dev/null +++ b/tests/test_media_stream_settled_equality.py @@ -0,0 +1,506 @@ +"""Stream-vs-settled MEDIA equality, driven through the REAL production walk. + +Re-review history for this file, both instances of the SAME mistake: + +1. v1 reimplemented stream-end flush with the unanchored matcher, so it reported + equality while production (anchored matcher on the whole candidate) dropped the + media card for `MEDIA:/tmp/a.png and after`. The reviewer caught it. +2. v2 still reimplemented the per-chunk walk. Its tail-buffer gate compared + ``pm[0].length`` where production compared ``rest.length``, so it could not see + that a MEDIA ref preceded by more than ``_MEDIA_TAIL_MAX`` characters of prose + silently lost its card. A self-audit caught it; a 116913-case sweep against + faithful production semantics found 24 real divergences. + +The lesson both times: **a harness that reimplements any decision function can +only validate its own mirror.** So this version extracts and evals the ENTIRE +production call chain — `_smdMediaAwareAddText` itself, plus every helper and +constant it closes over — and stubs ONLY the leaf sinks (`_smdAppendMediaNode`, +`_smdMediaWriteText`) so emissions can be recorded. No decision logic is retyped. + +If a future edit adds a new helper to that call chain, the eval fails loudly with +a ReferenceError rather than silently diverging. +""" +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +if str(REPO_ROOT) not in sys.path: + sys.path.insert(0, str(REPO_ROOT)) + +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") + +# Every function in the real _smdMediaAwareAddText call chain. Extracted, never +# retyped. Order matters only for readability — JS function decls hoist. +_UI_FUNCS = [ + "_mediaPathSrc", + "_mediaTokenRe", + "_unquoteMediaRef", + # Part of the shared lexical contract the streaming path consults when a + # candidate reaches the tail cap. Must be the REAL function, not a stub, or + # the harness would not exercise the ceiling it is meant to pin. + "_mediaTokenMaxLength", + "_mediaTokenExceedsMaxLength", +] +_MSG_FUNCS = [ + "_smdMediaPrefixTail", + "_smdMediaTailEntryChunk", + "_smdMediaTailSameOwner", + "_smdMediaTailSet", + "_smdMediaTokenIsSettled", + "_smdMediaTailCouldExtend", + "_smdMediaHasOpenQuote", + # Refusal state for an over-limit reference: it must stay closed ACROSS a + # chunk boundary, so these are part of the real call chain too. + "_smdMediaOpenQuoteChar", + "_smdMediaRefuseLine", + "_smdMediaEntryRefused", + "_smdMediaRunChar", + "_smdMediaRefusedRunLength", + # The single candidate ceiling, replacing the old second _MEDIA_TAIL_MAX. + "_smdMediaCandidateMax", + "_smdMediaTailFlushEntry", + "_smdMediaTailFlush", + "_smdMediaAwareAddText", # the function under test +] + +CASES = [ + # Intermediate extension mid-token. + "MEDIA:/tmp/archive.png.bak", + "MEDIA:/tmp/img.jpeg.tmp", + "MEDIA:/tmp/plain.png", + # Dotted directory before a space. + "MEDIA:/tmp/v1.2 Reports/chart.png", + "MEDIA:/tmp/deep/v2.5 Data/final.report.png", + # Quoted forms, including one holding a closing delimiter. + 'MEDIA:"/tmp/My Files/report (final).png"', + "MEDIA:'/tmp/My Files/single.png'", + # Final same-line prose (the v1 flush bug). + "prose before MEDIA:/tmp/a.png and after", + "MEDIA:/tmp/a.png and see notes later", + # Adjacent tags, punctuation, malformed quote, extension-less fallback. + "MEDIA:/tmp/one.png MEDIA:/tmp/two.png", + "see MEDIA:/tmp/a.png.", + 'MEDIA:"/tmp/unterminated.png and prose', + "MEDIA:/tmp/no-ext-file", + # Newline handling. + "MEDIA:/tmp/a.png\nsecond line", +] + +# Long-prose cases straddling _MEDIA_TAIL_MAX (4096). The v2 harness could not +# see these because every case above is under 40 chars; production lost the card. +_TAIL_MAX = 4096 +LONG_CASES = [ + "w" * n + " MEDIA:/tmp/a.png trailing words" + for n in (4090, 4094, 4095, 4096, 4097, 4100) +] + [ + "w" * 4094 + ' MEDIA:"/tmp/My Files/a.png" end', + "w" * 4094 + " MEDIA:/tmp/v1.2 Reports/c.png end", +] + +# Reviewer-authored overflow shape: a COMPLETE media token is followed by more +# than the tail limit of same-line prose that itself ends in a filename. Settled +# parsing correctly stops at `/tmp/a.png`; streaming used to buffer the token + +# prose as one possibly-growing candidate and, on overflow, write the ENTIRE +# candidate as plain text — silently dropping the already-complete media card. +LONG_AFTER_CASES = [ + "MEDIA:/tmp/a.png see " + ("w" * (_TAIL_MAX + 32)) + " README.md", +] + +# The REFERENCE ITSELF crosses the ceiling — the case the re-gate called out as +# missing. LONG_AFTER_CASES above covers a SHORT complete ref followed by long +# prose; these cover a single ref whose own length lands exactly on 4095 / 4096 / +# 4097 characters, with no delimiter to settle it early. +# +# This is the boundary that decides whether the cap behaves as a lexical rule or +# as a fake stream end. Under the old code the streaming path ran EOF-style +# partitioning at the cap, emitting the ref's prefix as a media node and the +# remainder as prose, while settled parsing saw one complete reference. Both +# sides now apply the same ceiling: at or below it the ref is a token, above it +# the whole span stays literal text. +# +# Length is measured on the CAPTURE (everything after `MEDIA:`), matching +# media_token_exceeds_max_length() / _mediaTokenExceedsMaxLength(). +def _ref_of_capture_length(n: int) -> str: + """A `MEDIA:` token whose capture is exactly ``n`` characters.""" + suffix = ".png" + prefix = "/tmp/" + filler = n - len(prefix) - len(suffix) + assert filler > 0, n + return "MEDIA:" + prefix + ("y" * filler) + suffix + + +OVERSIZED_REF_CASES = [ + # Exactly at the ceiling — still a legal token on both sides. + _ref_of_capture_length(_TAIL_MAX - 1), + _ref_of_capture_length(_TAIL_MAX), + # One past the ceiling — literal text on both sides. + _ref_of_capture_length(_TAIL_MAX + 1), + # And with trailing prose after the oversized ref, so the "keep scanning" + # path is exercised too. + _ref_of_capture_length(_TAIL_MAX + 1) + " then MEDIA:/tmp/ok.png", +] + + +def _extract_js_function(src: str, name: str) -> str: + start = src.index(f"function {name}(") + depth = 0 + started = False + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + started = True + elif src[i] == "}": + depth -= 1 + if started and depth == 0: + return src[start:i + 1] + raise AssertionError(f"unbalanced braces extracting {name}") + + +# Only SINKS are stubbed. The parser-identity map and constants mirror production +# values, which are asserted against the source below so they cannot drift. +_HARNESS = r""" +const _SMD_MEDIA_PREFIX = 'MEDIA:'; +const _SMD_MEDIA_TAIL = new Map(); + +let events = []; +// --- leaf sinks: the ONLY stubs ------------------------------------------- +function _smdAppendMediaNode(parent, rawRef){ events.push({kind:'MEDIA', v:rawRef}); return true; } +function _smdMediaWriteText(parent, data, baseAddText, writeText, chunk){ + if (chunk !== '' && chunk != null) events.push({kind:'TEXT', v:String(chunk)}); +} + +function settledEvents(text){ + const re = _mediaTokenRe(); + const out = []; + let m, last = 0; + while ((m = re.exec(text))){ + // Mirror renderMd()'s MEDIA stash: an over-ceiling capture is not a legal + // token, so the whole span stays prose. Without this the oracle would model + // a contract the real settled renderer no longer implements, and it would + // demand that streaming split an oversized ref. + if (_mediaTokenExceedsMaxLength(m[1])) continue; + if (m.index > last) out.push({kind:'TEXT', v:text.slice(last, m.index)}); + out.push({kind:'MEDIA', v:_unquoteMediaRef(m[1])}); + last = m.index + m[0].length; + } + if (last < text.length) out.push({kind:'TEXT', v:text.slice(last)}); + return out; +} + +// Drives the REAL _smdMediaAwareAddText, one call per chunk, then the REAL flush. +function streamedEvents(chunks){ + events = []; + _SMD_MEDIA_TAIL.clear(); + const parser = {id: 'p1'}; + const parent = {id: 'root'}; + const baseAddText = null, writeText = null, data = {}; + for (const chunk of chunks){ + _smdMediaAwareAddText(baseAddText, parent, data, chunk, _SMD_MEDIA_TAIL, parser, writeText); + } + _smdMediaTailFlush(parser); + return events.slice(); +} + +const mediaOf = (e) => JSON.stringify(e.filter(x=>x.kind==='MEDIA').map(x=>x.v)); +const textOf = (e) => e.filter(x=>x.kind==='TEXT').map(x=>x.v).join(''); + +const payload = JSON.parse(process.argv[1]); +const failures = []; +let checks = 0; +for (const input of payload.cases){ + const want = settledEvents(input); + const wantMedia = mediaOf(want), wantText = textOf(want); + const splits = []; + for (let i=1;i 80 ? `<${input.length} chars>` : input, + chunkLens: chunks.map(c=>c.length), kind:'MEDIA', + want:wantMedia, got:mediaOf(got)}); + else if (textOf(got) !== wantText) + failures.push({input: input.length > 80 ? `<${input.length} chars>` : input, + chunkLens: chunks.map(c=>c.length), kind:'TEXT', + wantLen: wantText.length, gotLen: textOf(got).length}); + } +} +console.log(JSON.stringify({checks, total: failures.length, failures: failures.slice(0,10)})); +""" + + +def _run(cases: list[str], two_cuts: bool) -> dict: + node = shutil.which("node") + if not node: + pytest.skip("node not available") + script = "\n".join( + [_extract_js_function(UI_JS, n) for n in _UI_FUNCS] + + [_extract_js_function(MESSAGES_JS, n) for n in _MSG_FUNCS] + + [_HARNESS] + ) + proc = subprocess.run( + [node, "--input-type=module", "-e", script, + json.dumps({"cases": cases, "twoCuts": two_cuts})], + capture_output=True, text=True, timeout=300, check=True, + ) + return json.loads(proc.stdout) + + +@pytest.fixture(scope="module") +def short_result(): + return _run(CASES, two_cuts=True) + + +@pytest.fixture(scope="module") +def long_result(): + return _run(LONG_CASES, two_cuts=False) + + +@pytest.fixture(scope="module") +def long_after_result(): + return _run(LONG_AFTER_CASES, two_cuts=False) + + +@pytest.fixture(scope="module") +def oversized_ref_result(): + return _run(OVERSIZED_REF_CASES, two_cuts=False) + + +def test_stream_and_settled_agree_over_every_chunk_cut(short_result): + assert short_result["checks"] > 5000, ( + f"sweep too small: {short_result['checks']} splits" + ) + assert short_result["total"] == 0, ( + "streamed rendering must equal settled rendering (captures AND exact " + f"remainder) for every chunk cut; {short_result['total']} mismatches, " + f"first few: {short_result['failures']}" + ) + + +def test_long_prose_before_a_ref_does_not_lose_the_card(long_result): + """Regression: the tail-buffer cap must bound the BUFFER, not the prose. + + Gating on ``rest.length`` meant a MEDIA ref preceded by more than + ``_MEDIA_TAIL_MAX`` characters of prose had its buffered tail discarded: the + partial ``MEDIA:/t`` was flushed as prose, the next chunk arrived with no + buffered tail, and the token never reassembled — a silently missing media + card on any long agent turn, while settled parsing rendered it fine. + """ + assert long_result["checks"] > 20000, ( + f"long-prose sweep too small: {long_result['checks']}" + ) + assert long_result["total"] == 0, ( + "a MEDIA ref preceded by >_MEDIA_TAIL_MAX chars of prose lost its card; " + f"{long_result['total']} mismatches, first few: {long_result['failures']}" + ) + + +def test_long_dotted_prose_after_a_complete_ref_does_not_lose_card(long_after_result): + """Overflow must partition the candidate, not flatten it to plain text.""" + assert long_after_result["checks"] > _TAIL_MAX, ( + f"overflow sweep too small: {long_after_result['checks']}" + ) + assert long_after_result["total"] == 0, ( + "same-line prose exceeding _MEDIA_TAIL_MAX after a complete ref lost " + f"the card; {long_after_result['total']} mismatches, first few: " + f"{long_after_result['failures']}" + ) + + +def test_oversized_reference_itself_agrees_across_the_ceiling(oversized_ref_result): + """A ref whose OWN length crosses 4095/4096/4097 renders the same both ways. + + The re-gate's blocker: ``_smdMediaTailFlushEntry`` applied final/EOF + partitioning the moment a candidate reached ``_MEDIA_TAIL_MAX``, even though + the stream had not ended. When the reference itself crossed the cap, the + prefix became a media node and the tail became prose, while settled + ``renderMd()`` re-parsed the same text uncapped and saw ONE complete + reference — a single token rendering two different ways. + + The cap is now a shared lexical rule (``MEDIA_TOKEN_MAX_LENGTH`` in + api/helpers.py, ``_mediaTokenMaxLength()`` in static/ui.js) that both the + streaming path and the settled stash apply, so the verdict is identical + regardless of where the chunk boundary falls. + """ + assert oversized_ref_result["checks"] > _TAIL_MAX, ( + f"oversized-ref sweep too small: {oversized_ref_result['checks']}" + ) + assert oversized_ref_result["total"] == 0, ( + "a reference crossing the length ceiling rendered differently streamed " + f"vs settled; {oversized_ref_result['total']} mismatches, first few: " + f"{oversized_ref_result['failures']}" + ) + + +def test_oversized_ref_fixtures_actually_straddle_the_ceiling(): + """Non-vacuity: the fixtures must land on both sides of the boundary. + + Without this, a future edit to ``_ref_of_capture_length`` (or to the ceiling) + could leave every OVERSIZED_REF_CASES row on the legal side, and the sweep + above would stay green while proving nothing about the boundary. + """ + import re as _re + + from api.helpers import ( + MEDIA_TOKEN_MAX_LENGTH, + media_token_exceeds_max_length, + media_token_pattern, + ) + + assert MEDIA_TOKEN_MAX_LENGTH == _TAIL_MAX, ( + "the Python ceiling and the JS tail cap must be the same number" + ) + + rx = _re.compile(media_token_pattern()) + verdicts = [] + for case in OVERSIZED_REF_CASES: + m = rx.search(case) + assert m, f"fixture did not match the grammar at all: {case[:40]}..." + verdicts.append(media_token_exceeds_max_length(m.group(1))) + + assert any(v is False for v in verdicts), ( + "no fixture is at/below the ceiling — the legal side is untested" + ) + assert any(v is True for v in verdicts), ( + "no fixture is above the ceiling — the rejection side is untested" + ) + # And the exact boundary rows must be present: 4095/4096 legal, 4097 not. + assert media_token_exceeds_max_length("x" * (_TAIL_MAX - 1)) is False + assert media_token_exceeds_max_length("x" * _TAIL_MAX) is False + assert media_token_exceeds_max_length("x" * (_TAIL_MAX + 1)) is True + + +def test_one_inclusive_candidate_ceiling_bounds_every_buffering_branch(): + """Pin the exact expressions, so the gate cannot silently regress. + + Two invariants, and the second is the re-review blocker: + + 1. The cap bounds what is actually BUFFERED (``tailValue``), not the whole + remaining text. Gating on ``rest.length`` discards the buffered tail after + long prose, so the token never reassembles. + 2. There is exactly ONE ceiling, ``_smdMediaCandidateMax()``, and every + branch applies it inclusively. The no-match / open-quote branch used a + separate, smaller ``_MEDIA_TAIL_MAX`` of 4096 while the matched-token + branch used 4102, so a legal quoted capture whose opening quote crossed a + chunk boundary was flushed as literal text at 4096 and lost its quote + ownership, while settled ``renderMd()`` accepted the same capture. + + Scoped by brace-accurate function extraction rather than a fixed character + window: a 7000-character slice silently drifts out of the function as soon as + anyone adds a comment, which turns an unrelated edit into a failure here (and, + worse, can slide the window PAST the expression so the assertion passes for + the wrong reason). + """ + block = _extract_js_function(MESSAGES_JS, "_smdMediaAwareAddText") + assert "tailValue.length <= _smdMediaCandidateMax()" in block, ( + "the tail branch must bound tailValue (what is buffered) by the ONE " + "shared candidate ceiling, inclusively" + ) + assert "rest.length < _MEDIA_TAIL_MAX" not in block, ( + "gating on rest.length discards the buffered tail after long prose" + ) + # No second ceiling anywhere in the module. Comments are stripped first: the + # explanatory comments legitimately NAME the removed constant, and scanning + # raw source would match those. + code = "\n".join( + line.split("//", 1)[0] for line in MESSAGES_JS.splitlines() + ) + assert "_MEDIA_TAIL_MAX" not in code, ( + "_MEDIA_TAIL_MAX was a SECOND ceiling that disagreed with " + "_mediaTokenMaxLength() + 'MEDIA:'.length for candidates from 4096 " + "through 4102 code units" + ) + # And the one ceiling is derived from the shared capture ceiling, never a + # literal. + ceiling = _extract_js_function(MESSAGES_JS, "_smdMediaCandidateMax") + assert "_mediaTokenMaxLength()" in ceiling and "_SMD_MEDIA_PREFIX.length" in ceiling, ( + "the candidate ceiling must be derived from the shared capture ceiling" + ) + assert "4096" not in ceiling, "the ceiling must not restate the number" + + +def test_length_ceiling_is_enforced_at_every_activation_point(): + """A ref may become a media node only if its capture is within the ceiling. + + Three places can activate a card, and all three must consult the shared + ceiling or streamed and settled disagree: + * the live match loop in _smdMediaAwareAddText, + * the stream-end partitioner _smdMediaTailFlushEntry, + * the settled MEDIA stash in renderMd() (ui.js). + """ + live = _extract_js_function(MESSAGES_JS, "_smdMediaAwareAddText") + assert "_mediaTokenExceedsMaxLength" in live, ( + "the live path must apply the shared length ceiling before activating " + "a media node" + ) + flush = _extract_js_function(MESSAGES_JS, "_smdMediaTailFlushEntry") + assert "_mediaTokenExceedsMaxLength" in flush, ( + "the stream-end partitioner must apply the shared length ceiling too" + ) + # renderMd() is huge; assert on the stash callback's immediate neighbourhood. + stash = UI_JS.index("const media_stash=[]") + window = UI_JS[stash:stash + 900] + assert "_mediaTokenExceedsMaxLength" in window, ( + "settled renderMd() must apply the same ceiling as the streaming path" + ) + + +def test_harness_stubs_only_sinks_not_decision_logic(): + """Meta-test: keep this file honest. + + Both previous versions of this module shipped a bug because the harness + reimplemented production decision logic. Assert that every function in the + real call chain is EXTRACTED, and that the harness defines only the two leaf + sinks plus the settled reference. + """ + for name in _MSG_FUNCS: + assert f"function {name}(" in MESSAGES_JS, ( + f"{name} vanished from messages.js — the harness would silently stop " + f"testing production" + ) + # The harness must not define its own copy of any extracted function. + for name in _UI_FUNCS + _MSG_FUNCS: + assert f"function {name}(" not in _HARNESS, ( + f"harness reimplements {name} — extract it from production instead" + ) + # Only these stubs are permitted. + assert "_smdAppendMediaNode" in _HARNESS and "_smdMediaWriteText" in _HARNESS + + +def test_completeness_is_not_decided_by_a_trailing_extension(): + idx = MESSAGES_JS.index("function _smdMediaAwareAddText") + block = MESSAGES_JS[idx:idx + 7000] + assert "_smdMediaRefHasReliableBoundary" not in MESSAGES_JS, ( + "the extension-guess heuristic was deleted as dead code; completeness " + "must be a real lexical delimiter or stream end" + ) + assert "_smdMediaTokenIsSettled(m[1], false)" in block + + +def test_flush_partitions_instead_of_matching_the_whole_candidate(): + """The flush must PARTITION the candidate, not handle one anchored match. + + The behavioral contract (later valid token after a malformed one, no-match + passthrough, per-token append-failure fallback) is executed against the real + function in tests/test_smd_media_in_stream.py:: + TestSmdMediaTailFlushPartition. Assert only the wiring here so this cannot + pass on a source string while behavior regresses. + """ + src = _extract_js_function(MESSAGES_JS, "_smdMediaTailFlushEntry") + assert "_mediaTokenRe()" in src + # A partition loop, not a single anchored exec. + assert "while((m=re.exec(raw)))" in src + # Every token span is preserved, including the trailing suffix. + assert "raw.slice(last)" in src diff --git a/tests/test_media_token_length_units.py b/tests/test_media_token_length_units.py new file mode 100644 index 00000000000..54f7914499f --- /dev/null +++ b/tests/test_media_token_length_units.py @@ -0,0 +1,495 @@ +"""The MEDIA length ceiling must use ONE unit in both languages (PR #6607). + +Re-review blocker: ``MEDIA_TOKEN_MAX_LENGTH``/``media_token_exceeds_max_length()`` +in api/helpers.py and ``_mediaTokenMaxLength()``/``_mediaTokenExceedsMaxLength()`` +in static/ui.js both said 4096, and they measured DIFFERENT THINGS. Python +``len()`` counts Unicode code points; JavaScript ``.length`` counts UTF-16 code +units. A token of 2049 U+1F600 characters measured 2049 in Python and 4098 in +JavaScript, so Python admitted a token JavaScript refused — for every astral +token from 2049 through 4096 characters. + +The chosen unit is **UTF-16 code units**, because the cap bounds a streaming +buffer that is a JavaScript string, and code units are what that buffer costs. +api/helpers.py converts via ``media_token_length()``. + +Two things are asserted for every row, and BOTH matter: + +1. **Parity** — Python and JavaScript return the same verdict. +2. **Correctness** — that verdict equals a hardcoded ``expected`` in the table. + +Parity alone is the "mirrored oracle" the reviewer rejects: two implementations +can agree and both be wrong. The hardcoded column is derived from the stated +contract (4096 UTF-16 code units), not from either implementation. + +Fixtures cross the boundary as a RECIPE (code point + repeat count) rather than +as a literal string, so each language builds the subject natively. That keeps a +4KB astral payload and a lone surrogate off the JSON transport, and it means +neither side can be handed a string the other could not represent. +""" +from __future__ import annotations + +import json +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path +from typing import Any + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +if str(REPO_ROOT) not in sys.path: + sys.path.insert(0, str(REPO_ROOT)) + +from api.helpers import ( # noqa: E402 + MEDIA_TOKEN_MAX_LENGTH, + media_token_exceeds_max_length, + media_token_length, +) + +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8") + +CAP = 4096 + +# Code points, one per width class. ASCII and BMP cost 1 UTF-16 code unit; +# anything above U+FFFF needs a surrogate pair and costs 2. +_ASCII = 0x61 # 'a' +_BMP = 0x6587 # '文' — 3 UTF-8 bytes, still ONE UTF-16 code unit +_ASTRAL = 0x1F600 # '😀' — a surrogate pair, TWO UTF-16 code units +_LONE_SURROGATE = 0xD800 + +# (label, code_point, repeat, pad_ascii, expected_exceeds) +# +# The subject is chr(code_point) * repeat + 'a' * pad_ascii. `expected` is the +# contract's own answer — UTF-16 code units above 4096 — written out by hand. +ROWS = [ + # ── ASCII: 1 code unit each ────────────────────────────────────────────── + ("ascii at cap minus one", _ASCII, CAP - 1, 0, False), + ("ascii exactly at cap", _ASCII, CAP, 0, False), + ("ascii at cap plus one", _ASCII, CAP + 1, 0, True), + # ── BMP: 1 code unit each, but 3 UTF-8 BYTES — a byte-based cap would + # reject these, which is why the unit must be named explicitly. + ("bmp at cap minus one", _BMP, CAP - 1, 0, False), + ("bmp exactly at cap", _BMP, CAP, 0, False), + ("bmp at cap plus one", _BMP, CAP + 1, 0, True), + # ── Astral: 2 code units each. This is the class that diverged. + ("astral at cap minus two", _ASTRAL, (CAP // 2) - 1, 0, False), + ("astral exactly at cap", _ASTRAL, CAP // 2, 0, False), + # 2049 astral characters: Python len() said 2049 (legal), JS .length said + # 4098 (illegal). The exact case the reviewer measured. + ("astral 2049 — the reported divergence", _ASTRAL, 2049, 0, True), + # ── Astral landing on the boundary ODD, so a pair straddles it ─────────── + ("astral plus pad exactly at cap", _ASTRAL, (CAP // 2) - 1, 2, False), + ("astral plus pad at cap plus one", _ASTRAL, (CAP // 2) - 1, 3, True), + # ── Lone surrogate: 1 code unit on both sides. A predicate must return a + # verdict for it rather than raise, so it is a row, not a footnote. + ("lone surrogate exactly at cap", _LONE_SURROGATE, CAP, 0, False), + ("lone surrogate at cap plus one", _LONE_SURROGATE, CAP + 1, 0, True), + # ── Degenerate rows ───────────────────────────────────────────────────── + ("empty", _ASCII, 0, 0, False), + ("single ascii", _ASCII, 1, 0, False), +] + + +def _subject(code_point: int, repeat: int, pad_ascii: int) -> str: + """Build a row's subject in Python, from the same recipe JavaScript uses.""" + return chr(code_point) * repeat + "a" * pad_ascii + + +def _extract_js_function(src: str, name: str) -> str: + """Lift a production function out of the shipped source, verbatim. + + Same brace-counting extraction the existing MEDIA harnesses use (see + tests/test_media_stream_settled_equality.py and the note at static/ui.js + ``_localTargetMarkers``). Never a re-implementation: if a helper is renamed + or deleted, this raises instead of quietly testing a copy. + """ + start = src.index(f"function {name}(") + depth = 0 + started = False + for i in range(start, len(src)): + if src[i] == "{": + depth += 1 + started = True + elif src[i] == "}": + depth -= 1 + if started and depth == 0: + return src[start:i + 1] + raise AssertionError(f"unbalanced braces extracting {name}") + + +def _node() -> str: + node = shutil.which("node") + if not node: + pytest.skip("node not available") + return node + + +def _run_node(script: str, payload) -> Any: + """Run *script* under node with *payload* handed over via a temp file. + + Not via ``process.argv``: a consumer row carries a ~4096-character ref, and + nine of them overflow ``execve``'s argument limit with + ``OSError: [Errno 7] Argument list too long``. A file also keeps subject + construction in ONE place (Python), so no fixture logic is duplicated in JS. + """ + with tempfile.NamedTemporaryFile( + "w", suffix=".json", encoding="utf-8", delete=False + ) as handle: + json.dump(payload, handle) + payload_path = handle.name + try: + proc = subprocess.run( + [_node(), "--input-type=module", "-e", script, payload_path], + capture_output=True, text=True, timeout=120, check=True, + ) + finally: + Path(payload_path).unlink(missing_ok=True) + return json.loads(proc.stdout) + + +# ── The matrix ─────────────────────────────────────────────────────────────── + + +@pytest.fixture(scope="module") +def js_verdicts() -> Any: + """Real ``_mediaTokenExceedsMaxLength()`` over every row, under node.""" + script = "\n".join([ + _extract_js_function(UI_JS, "_mediaTokenMaxLength"), + _extract_js_function(UI_JS, "_mediaTokenExceedsMaxLength"), + r""" +import {readFileSync} from 'node:fs'; +const rows = JSON.parse(readFileSync(process.argv[1], 'utf8')); +const out = rows.map((r) => { + // Built HERE from the recipe, so the astral payload and the lone surrogate + // never cross the JSON boundary as literal text. + const subject = String.fromCodePoint(r.codePoint).repeat(r.repeat) + + 'a'.repeat(r.pad); + return { + label: r.label, + utf16Length: subject.length, + exceeds: _mediaTokenExceedsMaxLength(subject), + }; +}); +console.log(JSON.stringify({cap: _mediaTokenMaxLength(), rows: out})); +""", + ]) + payload = [ + {"label": label, "codePoint": cp, "repeat": repeat, "pad": pad} + for label, cp, repeat, pad, _ in ROWS + ] + return _run_node(script, payload) + + +def test_the_two_caps_are_the_same_number(js_verdicts): + assert js_verdicts["cap"] == MEDIA_TOKEN_MAX_LENGTH == CAP + + +@pytest.mark.parametrize( + "label,code_point,repeat,pad,expected", + ROWS, + ids=[row[0] for row in ROWS], +) +def test_python_and_js_agree_and_are_correct( + label, code_point, repeat, pad, expected, js_verdicts +): + """Same verdict in both languages, and that verdict matches the contract.""" + subject = _subject(code_point, repeat, pad) + js = next(r for r in js_verdicts["rows"] if r["label"] == label) + + # The two sides must agree on the MEASUREMENT, not merely on the verdict — + # equal verdicts either side of the boundary would hide a unit mismatch. + assert media_token_length(subject) == js["utf16Length"], ( + f"{label}: length disagreement — python=" + f"{media_token_length(subject)} js={js['utf16Length']}" + ) + assert media_token_exceeds_max_length(subject) is expected, ( + f"{label}: python verdict wrong for {js['utf16Length']} code units" + ) + assert js["exceeds"] is expected, ( + f"{label}: js verdict wrong for {js['utf16Length']} code units" + ) + + +def test_reported_2049_astral_divergence_is_closed(): + """The exact case from the re-review, named so a regression is unmistakable. + + Python ``len()`` returned 2049 and admitted the token; JavaScript + ``.length`` returned 4098 and refused it. Both must now refuse it. + """ + token = "\U0001F600" * 2049 + assert len(token) == 2049, "code-point count, for contrast" + assert media_token_length(token) == 4098, "UTF-16 code units — the contract" + assert media_token_exceeds_max_length(token) is True + + +def test_every_astral_count_from_2049_to_4096_is_refused(): + """The whole divergent band, not just its first row. + + Any astral token from 2049 through 4096 characters exceeds 4096 code units, + so every one of them was accepted by Python and refused by JavaScript. + Sampled rather than exhaustive: the boundaries are pinned above, and this + covers the band's interior at low cost. + """ + for count in (2049, 2050, 3000, 4095, 4096): + token = "\U0001F600" * count + assert media_token_exceeds_max_length(token) is True, count + # And the row immediately below the band is still legal. + assert media_token_exceeds_max_length("\U0001F600" * 2048) is False + + +def test_media_token_length_counts_code_units_not_code_points_or_bytes(): + """Pin the unit itself, against all three plausible readings of "length".""" + assert media_token_length("") == 0 + assert media_token_length(None) == 0 # type: ignore[arg-type] + assert media_token_length("abc") == 3 + # BMP: one code unit, but three UTF-8 bytes. A byte-based cap would say 3. + assert media_token_length("文") == 1 + assert len("文".encode()) == 3, "contrast: bytes are NOT the unit" + # Astral: two code units, one code point, four UTF-8 bytes. + assert media_token_length("\U0001F600") == 2 + assert len("\U0001F600") == 1, "contrast: code points are NOT the unit" + # A lone surrogate is one code unit and must not raise. + assert media_token_length("\ud800") == 1 + assert media_token_length("a\ud800\U0001F600") == 4 + + +# ── Consumer matrices: the same verdict at each place a token can activate ─── +# +# The reviewer asked for matching ASCII / BMP / astral boundary rows in the +# settled renderer, the streaming path, the route allow-list, and the share +# inliner. The rows below use a GRAMMAR-LEGAL ref so each consumer really runs. +# +# Note the physical limit on two of those four: Linux PATH_MAX is 4096 BYTES, so +# no real file can exist at a path of 4096 UTF-16 code units. The share and +# allow-list rows therefore assert the decision the cap drives (an over-cap token +# is inert) rather than a successful embed at the boundary, which is unreachable +# by construction. + + +def _ref_of_utf16_length(units: int, code_point: int = _ASTRAL) -> str: + """A grammar-legal ``/tmp/…​.png`` ref measuring exactly *units* code units.""" + head, tail = "/tmp/", ".png" + body_units = units - len(head) - len(tail) + assert body_units > 0, units + width = 2 if code_point > 0xFFFF else 1 + whole, remainder = divmod(body_units, width) + ref = head + chr(code_point) * whole + "a" * remainder + tail + assert media_token_length(ref) == units, (media_token_length(ref), units) + return ref + + +# (label, code_point) — one row per width class, each swept across the boundary. +_CONSUMER_CLASSES = [("ascii", _ASCII), ("bmp", _BMP), ("astral", _ASTRAL)] +# (offset from the cap, is the ref legal) +_CONSUMER_OFFSETS = [(-1, True), (0, True), (1, False)] + +_CONSUMER_ROWS = [ + (f"{label} cap{offset:+d}", _ref_of_utf16_length(CAP + offset, cp), legal) + for label, cp in _CONSUMER_CLASSES + for offset, legal in _CONSUMER_OFFSETS +] + + +@pytest.fixture(scope="module") +def consumer_verdicts() -> Any: + """Settled renderMd() and the streaming walk, both real, over every row. + + Only leaf sinks are stubbed, matching the harness discipline in + tests/test_media_stream_settled_equality.py: no decision function is retyped. + """ + ui = [ + "_mediaPathSrc", + "_mediaTokenRe", + "_unquoteMediaRef", + "_mediaTokenMaxLength", + "_mediaTokenExceedsMaxLength", + ] + msg = [ + "_smdMediaPrefixTail", + "_smdMediaTailEntryChunk", + "_smdMediaTailSameOwner", + "_smdMediaTailSet", + "_smdMediaTokenIsSettled", + "_smdMediaTailCouldExtend", + "_smdMediaHasOpenQuote", + "_smdMediaOpenQuoteChar", + "_smdMediaRefuseLine", + "_smdMediaEntryRefused", + "_smdMediaRunChar", + "_smdMediaRefusedRunLength", + "_smdMediaCandidateMax", + "_smdMediaTailFlushEntry", + "_smdMediaTailFlush", + "_smdMediaAwareAddText", + ] + script = "\n".join( + [_extract_js_function(UI_JS, n) for n in ui] + + [_extract_js_function(MESSAGES_JS, n) for n in msg] + + [r""" +const _SMD_MEDIA_PREFIX = 'MEDIA:'; +const _SMD_MEDIA_TAIL = new Map(); + +let events = []; +// Leaf sinks only. +function _smdAppendMediaNode(parent, rawRef){ events.push({kind:'MEDIA', v:rawRef}); return true; } +function _smdMediaWriteText(parent, data, baseAddText, writeText, chunk){ + if (chunk !== '' && chunk != null) events.push({kind:'TEXT', v:String(chunk)}); +} + +// The settled MEDIA stash from renderMd(), same shape as static/ui.js: a token +// whose capture is over the ceiling is left as prose. +function settledMedia(text){ + const out = []; + text.replace(_mediaTokenRe(), (whole, raw) => { + if (_mediaTokenExceedsMaxLength(raw)) return whole; + out.push(_unquoteMediaRef(raw)); + return ''; + }); + return out; +} + +function streamedMedia(chunks){ + events = []; + _SMD_MEDIA_TAIL.clear(); + const parser = {id:'p1'}, parent = {id:'root'}; + for (const chunk of chunks) { + _smdMediaAwareAddText(null, parent, {}, chunk, _SMD_MEDIA_TAIL, parser, null); + } + _smdMediaTailFlush(parser); + return events.filter(e => e.kind === 'MEDIA').map(e => e.v); +} + +import {readFileSync} from 'node:fs'; +const rows = JSON.parse(readFileSync(process.argv[1], 'utf8')); +const out = rows.map((r) => { + const text = 'see MEDIA:' + r.ref + ' ok'; + const settled = settledMedia(text); + // Split at a fixed inner offset AND right after the keyword, so a ref that + // crosses the boundary is exercised from more than one cut. + const cuts = [1, 'see MEDIA:'.length, Math.floor(text.length / 2), text.length - 1]; + const streamed = cuts.map((i) => streamedMedia([text.slice(0, i), text.slice(i)])); + return {label: r.label, settled, streamed}; +}); +console.log(JSON.stringify(out)); +"""] + ) + payload = [{"label": label, "ref": ref} for label, ref, _ in _CONSUMER_ROWS] + return {row["label"]: row for row in _run_node(script, payload)} + + +@pytest.mark.parametrize( + "label,ref,legal", _CONSUMER_ROWS, ids=[row[0] for row in _CONSUMER_ROWS] +) +def test_settled_and_streamed_agree_with_python_across_the_boundary( + label, ref, legal, consumer_verdicts +): + """One verdict for one token, at every consumer, in both languages.""" + row = consumer_verdicts[label] + expected_media = [ref] if legal else [] + + assert media_token_exceeds_max_length(ref) is (not legal), ( + f"{label}: python predicate disagrees with the row's contract" + ) + assert row["settled"] == expected_media, ( + f"{label}: settled renderMd() stash diverged" + ) + for i, streamed in enumerate(row["streamed"]): + assert streamed == expected_media, ( + f"{label}: streamed rendering diverged at cut {i}" + ) + + +@pytest.mark.parametrize( + "label,ref,legal", _CONSUMER_ROWS, ids=[row[0] for row in _CONSUMER_ROWS] +) +def test_share_inliner_leaves_an_over_cap_ref_exactly_as_prose( + label, ref, legal, tmp_path +): + """The share snapshot must not touch a span neither renderer will activate. + + An over-cap token is inert everywhere else, so the snapshot has to return it + byte-for-byte rather than embed it or replace it with the missing-media + placeholder — otherwise the share and the live view disagree about one token. + """ + from api import shares + + text = f"see MEDIA:{ref} ok" + out = shares._embed_share_media(text, allowed_roots=(tmp_path,)) + if legal: + # No such file (and none can exist at this length — PATH_MAX is 4096 + # bytes), so a legal ref is resolved and placeholdered. What matters is + # that it was CONSIDERED. + assert out != text, f"{label}: a legal ref was never examined" + else: + assert out == text, ( + f"{label}: an over-cap ref was rewritten in a share snapshot" + ) + + +def test_route_allow_list_ignores_an_over_cap_astral_token(tmp_path, monkeypatch): + """An over-cap token must not mint an /api/media allow-list entry. + + Nothing on the page can request it — neither renderer activates it — so + honouring it would authorize a path no view asks for. Driven through the real + ``_session_media_token_allows_path``. + """ + from api import routes + + target = tmp_path / "real.png" + target.write_bytes(b"\x89PNG\r\n\x1a\n") + over_cap = _ref_of_utf16_length(CAP + 1, _ASTRAL) + + def _session_with(*refs): + class _Session: + messages = [ + { + "role": "assistant", + "content": " ".join(f"MEDIA:{r}" for r in refs), + } + ] + + return _Session() + + # An over-cap token alone authorizes nothing. + monkeypatch.setattr(routes, "get_session", lambda sid: _session_with(over_cap)) + assert routes._session_media_token_allows_path( + "sess-1", target, {"image/png"} + ) is False + + # Non-vacuity: the same call path DOES authorize the real, in-cap path, so + # the False above is the ceiling talking and not a broken fixture. + monkeypatch.setattr( + routes, "get_session", lambda sid: _session_with(over_cap, str(target)) + ) + assert routes._session_media_token_allows_path( + "sess-1", target, {"image/png"} + ) is True + + +# ── Harness integrity ─────────────────────────────────────────────────────── + + +def test_harness_extracts_production_and_reimplements_nothing(): + """Keep this file honest: no local copy of a decision function. + + The reviewer has rejected mirrored oracles here before. Every predicate under + test is lifted from the shipped source by name, so a rename fails loudly + instead of leaving a stale copy passing. + """ + for name in ("_mediaTokenMaxLength", "_mediaTokenExceedsMaxLength"): + assert f"function {name}(" in UI_JS, f"{name} vanished from ui.js" + assert "function _smdMediaAwareAddText(" in MESSAGES_JS + # Python side: the conversion is production code, not a test helper. + assert media_token_length.__module__ == "api.helpers" + # And this file must not carry its own copy of it. Scanned with the token + # split, so this very assertion is not what the scan finds. + source = Path(__file__).read_text(encoding="utf-8") + assert ("def " + "media_token_length") not in source, ( + "the test must not carry its own copy of the length function" + ) + assert ("def " + "media_token_exceeds_max_length") not in source, ( + "the test must not carry its own copy of the ceiling predicate" + ) diff --git a/tests/test_renderer_js_behaviour.py b/tests/test_renderer_js_behaviour.py index 3056a8d61b3..bd914b906e5 100644 --- a/tests/test_renderer_js_behaviour.py +++ b/tests/test_renderer_js_behaviour.py @@ -71,6 +71,15 @@ } eval(extractFunc('_matchBacktickFenceLine')); eval(extractFunc('_isBacktickFenceClose')); +// MEDIA: token matching helpers renderMd depends on. Must be eval'd BEFORE +// renderMd so the extracted function can resolve them. +eval(extractFunc('_mediaPathSrc')); +eval(extractFunc('_mediaTokenRe')); +eval(extractFunc('_unquoteMediaRef')); +// Shared MEDIA length ceiling consulted by renderMd's stash. Extracted, not +// stubbed, so this driver applies the same rule as the streaming path. +eval(extractFunc('_mediaTokenMaxLength')); +eval(extractFunc('_mediaTokenExceedsMaxLength')); eval(extractFunc('renderMd')); let buf = ''; diff --git a/tests/test_share_legacy_snapshot_load_guard.py b/tests/test_share_legacy_snapshot_load_guard.py new file mode 100644 index 00000000000..df0f23e12d3 --- /dev/null +++ b/tests/test_share_legacy_snapshot_load_guard.py @@ -0,0 +1,448 @@ +"""Legacy stored snapshots must pass the public-reference guard on the way OUT. + +``api/shares.py::build_share_snapshot()`` routes NEW message content through +``_sanitize_message()`` and ``_embed_share_media()``, so a share created after +that guard landed carries no live local reference. But ``load_share()`` read a +STORED snapshot's ``messages`` and returned them unchanged, so every share +written by an OLDER build stayed on the vulnerable path forever — the guard can +only protect content it ever saw. + +The sinks are still live on the share page. ``static/share.html`` loads +``static/ui.js``, and ``static/share.js::_shareRenderMessages()`` hands each +stored ``msg.content`` straight to ``renderMd()`` and assigns the result with +``innerHTML``: + +* ``_markdownHref()`` turns a ``file://`` link target into + ``api/media?path=…&inline=1``. +* ``_mdImageHtml()`` routes a ``file://`` image target through + ``_inlineMediaHtmlForRef()``, which emits ``api/media?path=…``. +* ``_isSafeUrl()``/``_tag()`` accept a relative ``api/`` image or link target, + so a raw ```` survives sanitisation. + +Each one makes an ANONYMOUS viewer's browser issue an authenticated +same-origin request against our own ``/api/media`` route. + +Test discipline in this file: + +* the legacy fixture is written DIRECTLY to the share store as JSON, bypassing + ``build_share_snapshot()`` entirely — exactly how an older build left it. +* the REAL ``load_share()`` reads it back. +* the REAL ``renderMd()`` from ``static/ui.js`` runs in node over both the + UNGUARDED stored text (the control) and the loaded text. The contrast is the + test: the control MUST reach a live ``api/media`` sink, and the loaded result + MUST NOT. +* the load path must take NO filesystem access. Asserted by monkeypatching the + ``Path`` methods the embed path would call and failing if any of them fires. +""" +from __future__ import annotations + +import json +import pathlib +import shutil +import subprocess +import textwrap + +import pytest + +from api import shares +from api.shares import _PLACEHOLDER, guard_public_share_references, load_share + +REPO = pathlib.Path(__file__).resolve().parents[1] +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +SHARE_JS = (REPO / "static" / "share.js").read_text(encoding="utf-8") + +NODE = shutil.which("node") + +_SECRET = "/etc/shadow.png" + +# Legacy message content, one row per live sink named in the module docstring. +# Written the way an older build stored it: unguarded, path intact. +# +# `live_attr` says whether the UNGUARDED text renders a LIVE element attribute +# (a real `src`/`href` the viewer's browser fetches) as opposed to merely +# disclosing the host path as prose. Both are leaks and both must be refused, +# but only the first is the authenticated-request sink, so the control asserts +# the two separately instead of blurring them. +_LEGACY_ROWS = [ + # (label, stored content, renders a live attribute unguarded) + ("md link file://", f"here is the file [x](file://{_SECRET}) from earlier", True), + ("md image file://", f"and the shot ![x](file://{_SECRET})", True), + ("raw img api/media", f'raw tag ', True), + ("rooted api/media target", f"rooted target ![x](/api/media?path={_SECRET})", False), + ("loopback api/media", f"loopback ![x](http://127.0.0.1:8080/api/media?path={_SECRET})", True), + ("canonical MEDIA token", f"canonical token MEDIA:{_SECRET}", True), + ("bare file:// prose", f"bare prose file://{_SECRET}", True), +] + +_LEGACY_CONTENTS = [content for _label, content, _live in _LEGACY_ROWS] + +# Genuine public references in the same legacy snapshot. Re-guarding must not +# destroy these: over-blocking a stored public asset is also a failure. +_LEGACY_PUBLIC_CONTENTS = [ + "public image ![ok](https://cdn.test/a.png?w=800&fmt=webp)", + "public link [ok](https://cdn.test/page?x=1&y=2)", + "public proxy ![ok](https://cdn.test/a.png?next=https://images.example.test/b.png)", + "external token MEDIA:https://cdn.test/a.png", +] + + +# ── Real share-page renderer driver ────────────────────────────────────────── +# Extracts the production functions by brace depth, the way the existing share +# tests in this repo already do, and drives the REAL renderMd() in node. + +def _extract_function(src: str, name: str) -> str: + marker = f"function {name}(" + start = src.index(marker) + brace = src.index("{", start) + depth = 1 + pos = brace + 1 + while depth and pos < len(src): + ch = src[pos] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + pos += 1 + assert depth == 0, f"could not extract {name}()" + return src[start:pos] + + +_RENDER_PRELUDE = textwrap.dedent( + r""" + const esc=s=>String(s??'').replace(/[&<>"']/g,c=>({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); + const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i; + const _PDF_EXTS=/\.pdf$/i; + const _SVG_EXTS=/\.svg$/i; + const _HTML_EXTS=/\.html?$/i; + const _CSV_EXTS=/\.csv$/i; + const _EXCALIDRAW_EXTS=/\.excalidraw$/i; + const _AUDIO_EXTS=/\.(mp3|ogg|wav|m4a|aac|flac|wma|opus|webm|oga)$/i; + const _VIDEO_EXTS=/\.(mp4|webm|mkv|mov|avi|ogv|m4v)$/i; + function t(k){ return k; } + function li(){ return ''; } + function _mediaPlayerHtml(kind,src){ return `<${kind} src="${esc(src)}">`; } + function _dataImageHtml(){ return ''; } + function _isSafeDataImageUri(){ return false; } + function _mediaKindForName(n){ + const s=String(n||''); + if(_IMAGE_EXTS.test(s)) return 'image'; + if(_AUDIO_EXTS.test(s)) return 'audio'; + if(_VIDEO_EXTS.test(s)) return 'video'; + return 'file'; + } + // The share page is served from the share origin, which is what makes a + // loopback rewrite dangerous there. + global.window={}; + global.document={baseURI:'https://share.example.test/share/abc123'}; + """ +) + +_RENDER_FUNCS = ( + "_matchBacktickFenceLine", + "_isBacktickFenceClose", + "_mediaPathSrc", + "_mediaTokenRe", + "_unquoteMediaRef", + "_mediaTokenMaxLength", + "_mediaTokenExceedsMaxLength", + "_isExternalMediaUrl", + "_localTargetMarkers", + "_decodeUrlComponentBounded", + "_externalMediaUrlHidesLocalTarget", + "_mdImageHtml", + "_inlineMediaHtmlForRef", + "renderMd", +) + + +def _share_page_html(contents: list[str]) -> list[str]: + """Render each content string exactly as the share page renders it. + + ``share.js`` calls ``renderMd(String(msg.content||''))`` and drops the + result into ``innerHTML``, so rendering the string with the real + ``renderMd()`` is what the anonymous viewer's browser does. + """ + js = _RENDER_PRELUDE + for name in _RENDER_FUNCS: + js += "\n" + _extract_function(UI_JS, name) + js += textwrap.dedent( + r""" + const cases=JSON.parse(process.argv[1]); + process.stdout.write(JSON.stringify(cases.map(c=>renderMd(String(c||''))))); + """ + ) + proc = subprocess.run( + ["node", "-e", js, json.dumps(contents)], + cwd=REPO, + text=True, + capture_output=True, + timeout=60, + check=True, + ) + return json.loads(proc.stdout) + + +def _live_media_sinks(html: str) -> list[str]: + """Every live local/authenticated reference in rendered *html*.""" + lowered = html.lower() + hits = [] + for needle in ( + 'src="api/media', + "src='api/media", + 'href="api/media', + "href='api/media", + 'src="/api/media', + 'href="/api/media', + "api/media?path=", + "file://", + "/etc/shadow", + "127.0.0.1", + ): + if needle in lowered: + hits.append(needle) + return hits + + +def _live_media_attributes(html: str) -> list[str]: + """Only the LIVE element attributes — a rendered ``src``/``href`` sink. + + Stricter than :func:`_live_media_sinks`, which also counts a path merely + disclosed as prose. A live attribute is what actually makes the anonymous + viewer's browser issue the authenticated request. + """ + lowered = html.lower() + return [ + needle + for needle in ( + 'src="api/media', + "src='api/media", + 'href="api/media', + "href='api/media", + 'src="/api/media', + 'href="/api/media', + 'src="http://127.0.0.1', + ) + if needle in lowered + ] + + +# ── The legacy fixture, written DIRECTLY to the share store ────────────────── + +@pytest.fixture() +def legacy_share(tmp_path, monkeypatch): + """Write a pre-guard snapshot straight to the store and return its token. + + ``build_share_snapshot()`` is deliberately NOT called: the whole point is a + snapshot whose content never met the guard. + """ + store = tmp_path / "shares" + store.mkdir() + monkeypatch.setattr(shares, "SHARES_DIR", store) + token = "legacytoken123" + messages = [ + {"role": "assistant", "content": c, "timestamp": 1.0} + for c in _LEGACY_CONTENTS + _LEGACY_PUBLIC_CONTENTS + ] + payload = { + "token": token, + "source_session_id": "sess-legacy", + "title": "Legacy share", + "messages": messages, + "message_count": len(messages), + "created_at": 1.0, + "updated_at": 1.0, + "revoked_at": None, + } + (store / f"{token}.json").write_text( + json.dumps(payload, ensure_ascii=False), encoding="utf-8" + ) + return token + + +def test_legacy_snapshot_is_stored_unguarded(legacy_share, tmp_path): + """Precondition: the fixture really is raw, pre-guard content on disk.""" + raw = json.loads((shares.SHARES_DIR / f"{legacy_share}.json").read_text()) + stored = [m["content"] for m in raw["messages"]] + assert stored == _LEGACY_CONTENTS + _LEGACY_PUBLIC_CONTENTS + assert _PLACEHOLDER not in "".join(stored) + assert "/etc/shadow" in "".join(stored) + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_unguarded_control_reaches_a_live_api_media_sink(): + """The control half of the contrast: without the guard, the sinks FIRE. + + If this ever stops finding a live sink the negative assertion below proves + nothing, so it is asserted explicitly rather than assumed. Rows marked + ``live_attr`` must render a real ``src``/``href`` the browser fetches; the + remaining row must at least disclose the host path. + """ + rendered = _share_page_html(_LEGACY_CONTENTS) + failures = [] + for (label, _content, live_attr), html in zip(_LEGACY_ROWS, rendered): + if live_attr and not _live_media_attributes(html): + failures.append(f"{label}: no live attribute in {html[:200]!r}") + if not _live_media_sinks(html): + failures.append(f"{label}: no local reference at all in {html[:200]!r}") + assert not failures, "control lost its teeth:\n" + "\n".join(failures) + + joined = " ".join(rendered).lower() + assert 'src="api/media?path=' in joined, joined[:400] + assert 'href="api/media?path=' in joined, joined[:400] + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_loaded_legacy_snapshot_reaches_no_live_sink(legacy_share): + """The fix half: load through the REAL load_share(), then render for real.""" + payload = load_share(legacy_share) + assert payload is not None + loaded = [m["content"] for m in payload["messages"]] + attacks = loaded[: len(_LEGACY_ROWS)] + + # The RENDER assertion comes first, deliberately. It is the one that answers + # the question the reviewer asked — does the anonymous viewer's browser get + # a live local reference — so it must be the assertion a regression trips, + # not a text check that happens to fire earlier. + rendered = _share_page_html(attacks) + failures = [ + f"{label}: {hits} in {html[:200]!r}" + for (label, _content, _live), html in zip(_LEGACY_ROWS, rendered) + if (hits := _live_media_sinks(html)) + ] + assert not failures, "loaded legacy snapshot still reaches a live sink:\n" + "\n".join(failures) + assert all(not _live_media_attributes(html) for html in rendered) + + # And the published text itself carries no residue. + for (label, _content, _live), guarded in zip(_LEGACY_ROWS, attacks): + assert _PLACEHOLDER in guarded, (label, guarded) + lowered = guarded.lower() + for leak in ("file://", "api/media", "media:", "/etc/shadow", "127.0.0.1"): + assert leak not in lowered, (label, leak, guarded) + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_loaded_legacy_snapshot_keeps_public_references_renderable(legacy_share): + """The guard must not destroy a stored PUBLIC asset.""" + payload = load_share(legacy_share) + public = [m["content"] for m in payload["messages"]][len(_LEGACY_CONTENTS):] + assert public == _LEGACY_PUBLIC_CONTENTS, "stored public references were altered" + + rendered = _share_page_html(public) + assert 'src="https://cdn.test/a.png?w=800&fmt=webp"' in rendered[0], rendered[0] + assert 'href="https://cdn.test/page?x=1&y=2"' in rendered[1], rendered[1] + assert 'src="https://cdn.test/a.png?next=https://images.example.test/b.png"' in rendered[2], rendered[2] + assert 'src="https://cdn.test/a.png"' in rendered[3], rendered[3] + for html in rendered: + assert _PLACEHOLDER not in html, html + + +def test_load_path_touches_only_the_snapshot_file(legacy_share, monkeypatch): + """The load guard must DECIDE, never resolve or read a REFERENCED file. + + A public read of an anonymous share is not a licence to touch the host + filesystem, and the concrete path inside a legacy snapshot is untrusted + input. Every ``Path`` method the embed path would use is RECORDED here + (delegating to the real one, so pytest keeps working), and the assertion is + that the only path touched during the load is the snapshot JSON itself. + """ + import pathlib as _pathlib + + snapshot = shares.SHARES_DIR / f"{legacy_share}.json" + touched: list[tuple[str, str]] = [] + watched = ("resolve", "is_file", "stat", "read_bytes", "expanduser", "open") + originals = {name: getattr(_pathlib.Path, name) for name in watched} + + def _recorder(name): + original = originals[name] + + def _wrapped(self, *a, **kw): + touched.append((name, str(self))) + return original(self, *a, **kw) + + return _wrapped + + for name in watched: + monkeypatch.setattr(_pathlib.Path, name, _recorder(name), raising=True) + + payload = load_share(legacy_share) + + for name in watched: # stop recording before asserting, or pytest's own + monkeypatch.setattr(_pathlib.Path, name, originals[name], raising=True) + + assert payload is not None + stray = [(name, p) for name, p in touched if p != str(snapshot)] + assert not stray, f"load path touched something other than the snapshot: {stray}" + # The legitimate accesses did happen, so the recorder was actually wired in. + assert touched, "recorder never fired — the assertion above would be vacuous" + assert all( + _PLACEHOLDER in m["content"] + for m in payload["messages"][: len(_LEGACY_CONTENTS)] + ) + + +def test_guard_is_idempotent_on_its_own_output(): + """A snapshot written by the CURRENT build must survive re-guarding. + + ``load_share()`` re-guards on every read, so the decision has to be a + fixed point or an already-published share would degrade over time. + """ + for content in _LEGACY_CONTENTS + _LEGACY_PUBLIC_CONTENTS: + once = guard_public_share_references(content) + assert guard_public_share_references(once) == once, content + + +def test_guard_placeholders_a_local_media_token_instead_of_embedding(tmp_path): + """The load guard has no allowed roots, so a local ref is refused. + + A real image inside a real directory still gets a placeholder from the load + guard: embedding is the CREATE path's job, and the originating session's + allowed roots are not knowable at load time. + """ + png = tmp_path / "shot.png" + png.write_bytes(b"\x89PNG\r\n\x1a\n" + b"\x00" * 64) + out = guard_public_share_references(f"before MEDIA:{png} after") + assert out == f"before {_PLACEHOLDER} after", out + assert "base64," not in out + # The create path with the same root still embeds — the two differ on + # purpose, and this pins that they do. + embedded = shares._embed_share_media( + f"before MEDIA:{png} after", allowed_roots=(tmp_path,) + ) + assert 'src="data:image/png;base64,' in embedded, embedded + + +def test_load_drops_a_message_whose_stored_content_is_not_text(tmp_path, monkeypatch): + """A structured stored ``content`` is not publishable text — drop it. + + ``/api/session/import`` can put a dict where a string belongs, and an older + snapshot may carry one. Publishing ``str(dict)`` would leak a raw payload. + """ + store = tmp_path / "shares" + store.mkdir() + monkeypatch.setattr(shares, "SHARES_DIR", store) + token = "structuredtoken" + (store / f"{token}.json").write_text(json.dumps({ + "token": token, + "title": "Mixed", + "messages": [ + {"role": "user", "content": {"secret": "tool payload"}}, + {"role": "assistant", "content": "plain text survives"}, + "not a dict at all", + ], + "created_at": 1.0, + "updated_at": 1.0, + "revoked_at": None, + }), encoding="utf-8") + + payload = load_share(token) + assert [m["content"] for m in payload["messages"]] == ["plain text survives"] + assert "tool payload" not in json.dumps(payload) + + +def test_revoked_legacy_share_is_still_refused(legacy_share): + """Re-guarding must not accidentally resurrect a revoked snapshot.""" + path = shares.SHARES_DIR / f"{legacy_share}.json" + stored = json.loads(path.read_text()) + stored["revoked_at"] = 123.0 + path.write_text(json.dumps(stored), encoding="utf-8") + assert load_share(legacy_share) is None diff --git a/tests/test_share_media_local_target_guard.py b/tests/test_share_media_local_target_guard.py new file mode 100644 index 00000000000..04710312b1e --- /dev/null +++ b/tests/test_share_media_local_target_guard.py @@ -0,0 +1,161 @@ +"""Share-page MEDIA guard: an http(s) ref must not smuggle a local target. + +Two halves of one privacy decision, pinned in both languages: + +* ``api/helpers.py::external_media_url_hides_local_target`` — server side. + Runs before a public snapshot is written. Strict: also rejects + loopback/RFC 1918 hosts, because an anonymous viewer can never legitimately + fetch one. +* ``static/ui.js::_externalMediaUrlHidesLocalTarget`` — client side, driven + here through node against the REAL ``_inlineMediaHtmlForRef``. Marker-only: + a private HOST alone stays renderable so the normal app keeps following a + dev-server asset URL, which is why the two are deliberately asymmetric. + +The client half matters because ``static/share.html`` loads ``ui.js`` and +``share.js`` calls ``renderMd()``, and the https:// branch of +``_inlineMediaHtmlForRef`` rewrites a loopback host to ``document.baseURI``. +On a share origin that turns ``MEDIA:http://127.0.0.1:8080/api/media?path=...`` +into a same-origin AUTHENTICATED request from the viewer's browser. The server +guard stops new snapshots; this one stops snapshots written by older builds. +""" +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).parent.parent.resolve() +UI_JS_PATH = REPO_ROOT / "static" / "ui.js" +DRIVER = Path(__file__).parent / "js" / "share_media_guard_driver.js" + +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def test_js_share_media_guard_matrix(): + """Drive the real ui.js renderer: attack shapes inert, legit refs intact.""" + result = subprocess.run( + [NODE, str(DRIVER), str(UI_JS_PATH)], + capture_output=True, + text=True, + timeout=60, + ) + assert result.returncode == 0, ( + "JS share-media guard matrix failed:\n" + f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + assert "ALL OK" in result.stdout, result.stdout + + +# ── Python/JS parity on the marker half ────────────────────────────────────── +# Both languages must agree about which refs hide a local target. The private +# host rows are excluded here: that is the documented asymmetry (server-only), +# and it is asserted separately below. +_MARKER_PARITY_CASES = [ + ("https://cdn.test/a.png", False), + ("https://cdn.test/a.png?w=800&fmt=webp", False), + ("https://cdn.test/img/photo.png", False), + ("https://cdn.test/a.png?next=/media/other.png", False), + ("https://cdn.test/img/MEDIA:/etc/passwd.png", True), + ("https://cdn.test/a.png?src=MEDIA:/etc/shadow.png", True), + ("https://cdn.test/a.png#MEDIA:/etc/shadow.png", True), + ("https://cdn.test/a.png?src=file:///etc/shadow.png", True), + ("https://cdn.test/a.png?src=%4dEDIA:/etc/shadow.png", True), + ("https://cdn.test/a.png?src=%254dEDIA:/etc/shadow.png", True), + ("https://cdn.test/api/media?path=/etc/shadow", True), + ("/tmp/local.png", False), + ("file:///etc/shadow.png", False), +] + + +@pytest.mark.parametrize("ref,hides", _MARKER_PARITY_CASES) +def test_python_marker_verdicts(ref, hides): + from api.helpers import external_media_url_hides_local_target + + assert external_media_url_hides_local_target(ref) is hides, ref + + +def test_js_marker_verdicts_match_python(): + """The JS predicate must return the same verdict as Python on every row.""" + from api.helpers import external_media_url_hides_local_target + + cases_js = ",".join( + "[" + repr(ref).replace("'", '"') + "]" for ref, _ in _MARKER_PARITY_CASES + ) + driver = f""" +const fs=require('fs'); +const src=fs.readFileSync(process.argv[2],'utf8'); +function extractFunc(name){{ + const re=new RegExp('function\\\\s+'+name+'\\\\s*\\\\('); + const start=src.search(re); + if(start<0) throw new Error(name+' not found'); + let i=src.indexOf('{{',start); let depth=1; i++; + while(depth>0&&i_externalMediaUrlHidesLocalTarget(c[0])))); +""" + import tempfile, json, os + + with tempfile.NamedTemporaryFile("w", suffix=".js", delete=False) as fh: + fh.write(driver) + path = fh.name + try: + result = subprocess.run( + [NODE, path, str(UI_JS_PATH)], + capture_output=True, text=True, timeout=60, + ) + assert result.returncode == 0, result.stderr + js_verdicts = json.loads(result.stdout.strip()) + finally: + os.unlink(path) + + for (ref, _expected), js in zip(_MARKER_PARITY_CASES, js_verdicts): + py = external_media_url_hides_local_target(ref) + # Python is stricter ONLY on private hosts, none of which appear here, + # so on this set the two must agree exactly. + assert py is js, f"parity break on {ref!r}: python={py} js={js}" + + +@pytest.mark.parametrize( + "ref", + [ + "http://127.0.0.1:8787/img/shot.png", + "http://localhost:8787/img/shot.png", + "http://192.168.1.5/x.png", + "http://10.0.0.7/x.png", + "http://172.16.0.9/x.png", + "http://169.254.1.1/x.png", + "http://[::1]/x.png", + ], +) +def test_private_hosts_are_server_side_only_rejections(ref): + """Documented asymmetry: server rejects a private host, client renders it. + + A published snapshot must never carry a loopback URL (an anonymous viewer + would resolve it in their own network position), but the live app + legitimately serves assets from a dev server, so the client must not break + that. If this ever flips, the two halves have drifted and the scope note in + both files is wrong. + """ + from api.helpers import external_media_url_hides_local_target + + assert external_media_url_hides_local_target(ref) is True, ( + f"{ref}: server side must reject a private host for public shares" + ) + + +def test_malformed_url_fails_closed(): + from api.helpers import external_media_url_hides_local_target + + # No host at all — unparseable as a public asset, so it must not be + # preserved into a snapshot. + assert external_media_url_hides_local_target("http:///nohost.png") is True diff --git a/tests/test_share_public_reference_sinks.py b/tests/test_share_public_reference_sinks.py new file mode 100644 index 00000000000..852440fc24f --- /dev/null +++ b/tests/test_share_public_reference_sinks.py @@ -0,0 +1,448 @@ +""" +Public-share URL sinks: ONE fail-closed decision must cover EVERY URL-bearing +token in message content, not only canonical ``MEDIA:`` tokens. + +``api/shares.py::_embed_share_media()`` used to substitute only the ``MEDIA:`` +pattern, so its whole-reference privacy guard never inspected the other shapes +that reach the public share page. ``static/share.html`` loads ``static/ui.js`` +and ``static/share.js::_shareRenderMessages()`` calls the real ``renderMd()``, +whose sinks turn message text into a LIVE URL: + +* ``_mdImageHtml()`` routes a bare Markdown ``file://`` image through + ``_inlineMediaHtmlForRef()``, which emits ``api/media?path=…``. +* ``_markdownHref()`` converts an ordinary Markdown ``file://`` link into + ``api/media?path=…&inline=1``. +* an http(s) Markdown image is restored as a live ````. +* the autolink pass turns a bare http(s) run into ````. + +Every one of those makes an anonymous viewer's browser issue an authenticated +same-origin request against our own ``/api/media`` route, or round-trips a host +filesystem path into a public snapshot. + +Test discipline in this file: + +* the REAL ``_embed_share_media()`` runs on every input — no mirrored oracle. +* the composed matrix runs the REAL ``renderMd()`` from ``static/ui.js`` in + node over the ACTUAL published snapshot text, then asserts on the final sink + markup. Publication and rendering are chained exactly as production chains + them. +* positive controls assert byte-for-byte preservation of genuine public assets, + because over-blocking is also a failure. +""" +from __future__ import annotations + +import json +import pathlib +import shutil +import subprocess +import textwrap + +import pytest + +from api.shares import _PLACEHOLDER, _embed_share_media + +REPO = pathlib.Path(__file__).resolve().parents[1] +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") + +NODE = shutil.which("node") + + +# ── Real renderMd() driver (same brace-depth extraction the repo already uses) ─ + +def _extract_function(src: str, name: str) -> str: + marker = f"function {name}(" + start = src.index(marker) + brace = src.index("{", start) + depth = 1 + pos = brace + 1 + while depth and pos < len(src): + ch = src[pos] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + pos += 1 + assert depth == 0, f"could not extract {name}()" + return src[start:pos] + + +_RENDER_PRELUDE = textwrap.dedent( + r""" + const esc=s=>String(s??'').replace(/[&<>"']/g,c=>({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); + const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i; + const _PDF_EXTS=/\.pdf$/i; + const _SVG_EXTS=/\.svg$/i; + const _HTML_EXTS=/\.html?$/i; + const _CSV_EXTS=/\.csv$/i; + const _EXCALIDRAW_EXTS=/\.excalidraw$/i; + const _AUDIO_EXTS=/\.(mp3|ogg|wav|m4a|aac|flac|wma|opus|webm|oga)$/i; + const _VIDEO_EXTS=/\.(mp4|webm|mkv|mov|avi|ogv|m4v)$/i; + function t(k){ return k; } + function li(){ return ''; } + function _mediaPlayerHtml(kind,src){ return `<${kind} src="${esc(src)}">`; } + function _dataImageHtml(){ return ''; } + function _isSafeDataImageUri(){ return false; } + function _mediaKindForName(n){ + const s=String(n||''); + if(_IMAGE_EXTS.test(s)) return 'image'; + if(_AUDIO_EXTS.test(s)) return 'audio'; + if(_VIDEO_EXTS.test(s)) return 'video'; + return 'file'; + } + // The share page is served from the share origin, which is what makes a + // loopback rewrite dangerous there. + global.window={}; + global.document={baseURI:'https://share.example.test/share/abc123'}; + """ +) + +_RENDER_FUNCS = ( + "_matchBacktickFenceLine", + "_isBacktickFenceClose", + "_mediaPathSrc", + "_mediaTokenRe", + "_unquoteMediaRef", + "_mediaTokenMaxLength", + "_mediaTokenExceedsMaxLength", + "_isExternalMediaUrl", + "_localTargetMarkers", + "_decodeUrlComponentBounded", + "_externalMediaUrlHidesLocalTarget", + "_mdImageHtml", + "_inlineMediaHtmlForRef", + "renderMd", +) + + +def _render_md_many(inputs: list[str]) -> list[str]: + """Run the REAL renderMd() from static/ui.js over each input, in node.""" + js = _RENDER_PRELUDE + for name in _RENDER_FUNCS: + js += "\n" + _extract_function(UI_JS, name) + js += textwrap.dedent( + r""" + const cases=JSON.parse(process.argv[1]); + process.stdout.write(JSON.stringify(cases.map(c=>renderMd(c)))); + """ + ) + proc = subprocess.run( + ["node", "-e", js, json.dumps(inputs)], + cwd=REPO, + text=True, + capture_output=True, + timeout=60, + check=True, + ) + return json.loads(proc.stdout) + + +# ── The attack matrix ──────────────────────────────────────────────────────── +# Every row is a shape a client sink would turn into a live local/authenticated +# URL. `_embed_share_media` must reduce each to exactly one placeholder, and the +# rendered snapshot must carry no live local reference. +_SECRET = "/etc/shadow.png" + +ATTACK_CASES = [ + # (label, message content) + ("canonical MEDIA token", f"look MEDIA:{_SECRET} here"), + ("bare file:// prose", f"file://{_SECRET}"), + ("outer md image file://", f"![x](file://{_SECRET})"), + ("outer md link file://", f"[x](file://{_SECRET})"), + ("relative api/media image", f"![x](/api/media?path={_SECRET})"), + ("relative api/media no slash", f"![x](api/media?path={_SECRET})"), + ("loopback api/media image", f"![x](http://127.0.0.1:8080/api/media?path={_SECRET})"), + ("loopback api/media bare", f"http://127.0.0.1:8080/api/media?path={_SECRET}"), + ("cdn host reaching api/media", f"![x](https://cdn.test/api/media?path={_SECRET})"), + ("nested MEDIA in query", f"![x](https://cdn.test/a.png?src=MEDIA:{_SECRET})"), + ("nested MEDIA in fragment", f"![x](https://cdn.test/a.png#MEDIA:{_SECRET})"), + ("nested file:// in query", f"![x](https://cdn.test/a.png?src=file://{_SECRET})"), + ("nested loopback http:// start", "![x](https://cdn.test/a.png?u=http://127.0.0.1/x.png)"), + ("nested relative api/media in query", f"![x](https://cdn.test/a.png?next=api/media?path={_SECRET})"), + ("percent-encoded MEDIA", f"![x](https://cdn.test/a.png?src=%4dEDIA:{_SECRET})"), + ("double-encoded MEDIA", f"![x](https://cdn.test/a.png?src=%254dEDIA:{_SECRET})"), + ("residual encoding past the bound", "![x](https://cdn.test/a.png?t=%25252525254dEDIA:/etc/shadow)"), + ("empty host fails closed", "![x](http:///nohost.png)"), + ("list item consumer", f"- ![x](file://{_SECRET})"), + ("nested list consumer", f" - [x](file://{_SECRET})"), + ("blockquote consumer", f"> ![x](file://{_SECRET})"), + ("table cell consumer", f"| col |\n| --- |\n| ![x](file://{_SECRET}) |"), + ("inline md link in list", f"- see [x](file://{_SECRET}) now"), +] + +# Genuine public references. Every one must survive BYTE-FOR-BYTE: over-blocking +# a legitimate public image is a failure too. +PRESERVE_CASES = [ + ("public cdn image", "![ok](https://cdn.test/a.png)"), + ("public cdn image with harmless query", "![ok](https://cdn.test/a.png?w=800&fmt=webp)"), + ("public cdn link", "[ok](https://cdn.test/page?x=1&y=2)"), + ("bare public url in prose", "see https://cdn.test/a.png for more"), + ("external MEDIA token", "MEDIA:https://cdn.test/a.png"), + ("public path that merely mentions media", "![ok](https://cdn.test/media/photo.png)"), + ("harmless percent in query", "![ok](https://cdn.test/a.png?pct=100%25)"), + ("query naming another public asset", "![ok](https://cdn.test/a.png?next=/images/other.png)"), + ("query naming another public URL", "![ok](https://cdn.test/a.png?next=https://images.example.test/b.png)"), +] + + +def _published(text: str) -> str: + """Publish *text* through the REAL sanitizer with NO allowed roots.""" + return _embed_share_media(text, allowed_roots=()) + + +@pytest.mark.parametrize("label,content", ATTACK_CASES, ids=[c[0] for c in ATTACK_CASES]) +def test_attack_shape_is_placeholdered_exactly_once(label, content): + out = _published(content) + assert out.count(_PLACEHOLDER) == 1, ( + f"{label}: expected exactly one placeholder, got {out!r}" + ) + lowered = out.lower() + for leak in ("file://", "api/media", "media:", "/etc/shadow", "127.0.0.1"): + assert leak not in lowered, f"{label}: {leak!r} survived publication in {out!r}" + + +@pytest.mark.parametrize("label,content", PRESERVE_CASES, ids=[c[0] for c in PRESERVE_CASES]) +def test_public_reference_is_preserved_byte_for_byte(label, content): + assert _published(content) == content, f"{label}: public reference was altered" + + +def test_one_refusal_does_not_restart_inside_its_own_token(): + """A refused token must be consumed WHOLE — the previous blocker on this PR. + + ``MEDIA:https://cdn.test/img/MEDIA:/etc/passwd.png`` carries a nested + keyword. If the scanner resumed inside the span it just refused, the nested + ``MEDIA:`` would match as a fresh LOCAL token and produce a second + placeholder plus a rewritten tail. + """ + out = _published("MEDIA:https://cdn.test/img/MEDIA:/etc/passwd.png") + assert out == _PLACEHOLDER, out + assert out.count(_PLACEHOLDER) == 1, out + + +def test_local_image_inside_allowed_roots_still_embeds(tmp_path): + """Positive control on the embed path: a legitimate local image still works.""" + png = tmp_path / "shot.png" + png.write_bytes( + b"\x89PNG\r\n\x1a\n" + b"\x00" * 64 + ) + out = _embed_share_media(f"before MEDIA:{png} after", allowed_roots=(tmp_path,)) + assert 'src="data:image/png;base64,' in out, out + assert _PLACEHOLDER not in out, out + assert str(png) not in out, "the concrete host path must not survive" + + +def test_multiple_tokens_in_one_message_each_get_one_decision(): + text = ( + f"![bad](file://{_SECRET}) and ![ok](https://cdn.test/a.png) " + f"and [bad2](/api/media?path={_SECRET})" + ) + out = _published(text) + assert out.count(_PLACEHOLDER) == 2, out + assert "![ok](https://cdn.test/a.png)" in out, out + assert "file://" not in out and "api/media" not in out, out + + +# ── Composed matrix: publication → share.js → real renderMd() → final sink ─── + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_composed_pipeline_emits_no_live_local_reference(): + """Chain production: publish, then render with the REAL renderMd(). + + ``share.js`` hands ``msg.content`` straight to ``renderMd()``, so rendering + the PUBLISHED text is exactly what the anonymous viewer's browser does. + """ + contents = [content for _label, content in ATTACK_CASES] + published = [_published(c) for c in contents] + rendered = _render_md_many(published) + + failures = [] + for (label, _content), pub, html in zip(ATTACK_CASES, published, rendered): + lowered = html.lower() + for leak in ("api/media", "file://", "media:", "/etc/shadow", "127.0.0.1"): + if leak in lowered: + failures.append(f"{label}: {leak!r} in rendered output: {html[:200]!r}") + # No live element may carry a local or authenticated target. + if 'src="api/media' in lowered or 'href="api/media' in lowered: + failures.append(f"{label}: live api/media attribute: {html[:200]!r}") + if _PLACEHOLDER not in pub: + failures.append(f"{label}: publication left no placeholder: {pub!r}") + assert not failures, "composed pipeline leaks:\n" + "\n".join(failures) + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_composed_pipeline_still_renders_public_images(): + """The same chain must keep rendering a genuine public asset as an image.""" + published = [_published(c) for _l, c in PRESERVE_CASES] + rendered = _render_md_many(published) + by_label = {label: html for (label, _c), html in zip(PRESERVE_CASES, rendered)} + + # The renderer HTML-escapes `&` in an attribute value, so assert on the + # escaped form the browser actually receives. + img = by_label["public cdn image with harmless query"] + assert 'src="https://cdn.test/a.png?w=800&fmt=webp"' in img, img + assert _PLACEHOLDER not in img, img + + link = by_label["public cdn link"] + assert 'href="https://cdn.test/page?x=1&y=2"' in link, link + + media_token = by_label["external MEDIA token"] + assert 'src="https://cdn.test/a.png"' in media_token, media_token + + +# ── Decode bound: a value still changing at the bound must be REFUSED ──────── + +def test_decode_probe_reports_a_value_still_changing_at_the_bound(): + """``api.helpers._decode_url_component_bounded`` cannot express this. + + It returns only the decoded string, so a value still mutating at the bound + is indistinguishable from one that decoded cleanly to something harmless, + and the caller accepts it. The share boundary needs the verdict. + """ + from api.share_refs import decode_probe_bounded + + settled, still_changing = decode_probe_bounded("MEDIA:/x") + assert (settled, still_changing) == ("MEDIA:/x", False) + + _decoded, still_changing = decode_probe_bounded("%25252525254dEDIA:/etc/shadow") + assert still_changing is True + + # A single literal percent is not "still changing". + settled, still_changing = decode_probe_bounded("100%25") + assert (settled, still_changing) == ("100%", False) + + +def test_nested_local_markers_are_a_superset_of_the_helper_markers(): + """The nested marker scan must not drift from the shared marker tuple. + + ``http://`` / ``https://`` are deliberately NOT markers: a nested absolute + URL is a candidate to classify, not a verdict. See + ``test_nested_public_url_is_preserved_and_nested_local_is_refused``. + """ + from api.helpers import _LOCAL_TARGET_MARKERS + from api.share_refs import _NESTED_LOCAL_MARKERS + + assert set(_LOCAL_TARGET_MARKERS) <= set(_NESTED_LOCAL_MARKERS) + assert "api/media" in _NESTED_LOCAL_MARKERS + for scheme in ("http://", "https://"): + assert scheme not in _NESTED_LOCAL_MARKERS, ( + f"{scheme!r} as an unconditional marker refuses every public proxy URL" + ) + + +@pytest.mark.parametrize( + "value,hides", + [ + ("https://cdn.test/a.png", False), + ("https://cdn.test/a.png?w=800&fmt=webp", False), + ("https://cdn.test/media/photo.png", False), + ("file:///etc/shadow", True), + ("FILE:///etc/shadow", True), + ("/api/media?path=/etc/shadow", True), + ("api/media?path=/etc/shadow", True), + ("http:///nohost.png", True), + ("http://127.0.0.1:8080/img.png", True), + # A nested PUBLIC URL is a public asset, not a local target. `evil.test` + # is a public host: this row asserted True before, which locked in an + # over-block that contradicted the module's preservation contract. + ("https://cdn.test/a.png?u=https://evil.test/x", False), + ("https://cdn.test/a.png#api/media?path=/x", True), + ("mailto:someone@example.test", True), + ("", True), + ], +) +def test_public_reference_verdicts(value, hides): + from api.share_refs import public_reference_hides_local_target + + assert public_reference_hides_local_target(value) is hides, value + + +# ── Nested absolute URLs: classify the candidate, never refuse its scheme ───── +# A nested absolute URL inside the outer URL's path, query, or fragment is a +# CANDIDATE. Refusing it for carrying a scheme token destroys every legitimate +# image-proxy / redirect URL, so each candidate is classified by the SAME rules +# instead. The matrix covers all three positions in both directions. + +_NESTED_PRESERVE_CASES = [ + # (label, value) -- outer public URL carrying a HARMLESS PUBLIC nested URL. + ("path public", "https://cdn.test/proxy/https://images.example.test/b.png"), + ("query public", "https://cdn.test/a.png?next=https://images.example.test/b.png"), + ("query public http", "https://cdn.test/a.png?next=http://images.example.test/b.png"), + ("fragment public", "https://cdn.test/a.png#https://images.example.test/b.png"), + ("two public candidates", "https://cdn.test/a.png?a=https://x.test/1.png&b=https://y.test/2.png"), + ("public with harmless query of its own", "https://cdn.test/a.png?u=https://x.test/b.png?w=800"), + ("public host merely named evil", "https://cdn.test/a.png?u=https://evil.test/x"), + ("percent-encoded nested public", "https://cdn.test/a.png?u=https%3A%2F%2Fx.test%2Fb.png"), +] + +_NESTED_REJECT_CASES = [ + # (label, value) -- the nested candidate is local, private, or our route. + ("path file scheme", "https://cdn.test/proxy/file:///etc/passwd.png"), + ("query file scheme", "https://cdn.test/a.png?next=file:///etc/passwd.png"), + ("fragment file scheme", "https://cdn.test/a.png#file:///etc/passwd.png"), + ("path loopback", "https://cdn.test/proxy/http://127.0.0.1/x.png"), + ("query loopback api/media", "https://cdn.test/a.png?next=http://127.0.0.1/api/media?path=/etc/shadow"), + ("fragment loopback", "https://cdn.test/a.png#http://127.0.0.1:8080/x.png"), + ("query rfc1918", "https://cdn.test/a.png?next=http://192.168.1.5/x.png"), + ("query link-local", "https://cdn.test/a.png?next=http://169.254.1.1/x.png"), + ("query rfc4193", "https://cdn.test/a.png?next=http://[fd00::1]/x.png"), + ("query localhost", "https://cdn.test/a.png?next=http://localhost:8787/x.png"), + ("path relative api/media", "https://cdn.test/api/media?path=/etc/shadow"), + ("query relative api/media", "https://cdn.test/a.png?next=api/media?path=/etc/shadow"), + ("fragment nested MEDIA token", "https://cdn.test/a.png#MEDIA:/etc/shadow.png"), + ("query nested MEDIA token", "https://cdn.test/a.png?src=MEDIA:/etc/shadow.png"), + ("nested empty host", "https://cdn.test/a.png?next=http:///nohost.png"), + ("public wrapper around private grandchild", + "https://cdn.test/a.png?u=https://proxy.test/p?next=http://10.0.0.7/x.png"), + ("percent-encoded nested loopback", + "https://cdn.test/a.png?u=http%3A%2F%2F127.0.0.1%2Fx.png"), +] + + +@pytest.mark.parametrize( + "label,value", _NESTED_PRESERVE_CASES, ids=[c[0] for c in _NESTED_PRESERVE_CASES] +) +def test_nested_public_candidate_is_preserved(label, value): + """An outer public URL carrying a harmless public URL is a public asset.""" + from api.share_refs import public_reference_hides_local_target + + assert public_reference_hides_local_target(value) is False, label + # And the whole token must survive publication byte-for-byte. + assert _published(f"![ok]({value})") == f"![ok]({value})", label + + +@pytest.mark.parametrize( + "label,value", _NESTED_REJECT_CASES, ids=[c[0] for c in _NESTED_REJECT_CASES] +) +def test_nested_local_or_private_candidate_is_refused(label, value): + from api.share_refs import public_reference_hides_local_target + + assert public_reference_hides_local_target(value) is True, label + assert _published(f"![x]({value})") == _PLACEHOLDER, label + + +def test_nested_classification_is_bounded_and_fails_closed_at_the_bound(): + """Recursion is bounded explicitly, and the bound REFUSES. + + Each nested candidate is a strict substring of its parent's probe, so the + walk terminates on the value alone. The bound is the second guarantee, and + a candidate we never examined must not be published. + """ + from api.share_refs import _MAX_NESTED_URL_DEPTH, public_reference_hides_local_target + + def chain(levels: int) -> str: + hops = "".join(f"https://h{i}.test/p/" for i in range(1, levels + 1)) + return f"https://h0.test/p/{hops}x.png" + + # Every hop is a public host, so anything within the bound is preserved. + for levels in range(0, _MAX_NESTED_URL_DEPTH + 1): + assert public_reference_hides_local_target(chain(levels)) is False, levels + + # One hop past the bound is refused even though every host is public. + assert public_reference_hides_local_target(chain(_MAX_NESTED_URL_DEPTH + 1)) is True + + # A private host buried past the bound is refused too — the bound can only + # make the answer stricter, never looser. + deep_private = ( + "https://h0.test/p/https://h1.test/p/https://h2.test/p/" + "https://h3.test/p/http://127.0.0.1/x.png" + ) + assert public_reference_hides_local_target(deep_private) is True diff --git a/tests/test_share_read_boundary_projection.py b/tests/test_share_read_boundary_projection.py new file mode 100644 index 00000000000..975682bdedd --- /dev/null +++ b/tests/test_share_read_boundary_projection.py @@ -0,0 +1,314 @@ +"""Public-share read boundary: legacy snapshots and scheme-relative descendants. + +Two blockers from the 3 September re-gate at ``5e90f1fa``. + +1. ``api/shares.py::_guarded_public_message()`` checked only that a row was a + dict with string ``content``, then returned ``{**message, "content": guarded}``. + That republished every stored field and every stored role, so a pre-fix + snapshot could serve ``system`` and ``tool`` rows plus siblings such as + ``provider_details``, ``tool_calls``, ``raw_result``, and workspace paths + through the anonymous ``/api/share/`` response. + ``static/share.js::_shareRenderMessages()`` renders every returned row. + + Measured on the pre-fix code with the fixture below, read through the real + ``load_share()``: 3 forbidden roles published, 4 unapproved sibling fields + published, and the strings ``sk-SHOULD-NOT-LEAK``, ``/etc/shadow``, and + ``/home/samfp/private`` all reached the payload. The stored + ``message_count`` of 99 survived while only 7 rows were returned. + +2. ``api/share_refs.py`` enumerated only literal ``http://`` / ``https://`` + descendants. A scheme-relative descendant such as ``?next=//127.0.0.1/x.png`` + carries no scheme token and matches no marker, so it reached the preserve arm + and published a live private reference. Browser separator and dot-segment + forms had the same gap. + + Measured on the pre-fix code: 8 of 20 probe rows reached the preserve arm + wrongly, including the AWS metadata endpoint ``//169.254.169.254``. + +Every test drives the REAL production functions. The legacy fixture is written +straight to the share store as raw JSON, so nothing in the write path can +sanitize it first — that is the whole point of a read-time guard. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +import api.shares as shares # noqa: E402 +from api.share_refs import public_reference_hides_local_target # noqa: E402 + +APPROVED_KEYS = {"role", "content", "timestamp"} +APPROVED_ROLES = {"user", "assistant"} + + +# ── Blocker 1: the legacy-snapshot read projection ─────────────────────────── + + +def _legacy_snapshot() -> dict: + """A snapshot shaped as an older build could have written it. + + Deliberately carries invalid roles, secret-bearing sibling fields, a + structured title, a stale count, and malformed rows. + """ + return { + "title": {"text": "Structured title", "html": "nope"}, + "message_count": 99, + "created_at": 1_700_000_000.0, + "session_id": "private-session-id", + "workspace": "/home/samfp/private", + "messages": [ + { + "role": "user", + "content": "hello", + "provider_details": {"api_key": "sk-SHOULD-NOT-LEAK"}, + "workspace": "/home/samfp/private", + "raw_result": {"stdout": "internal"}, + }, + { + "role": "assistant", + "content": "hi there", + "tool_calls": [{"name": "terminal", "args": {"cmd": "cat /etc/shadow"}}], + "provider_details": {"model": "internal-only"}, + }, + {"role": "system", "content": "SYSTEM PROMPT SHOULD NOT LEAK"}, + {"role": "tool", "content": "TOOL RESULT SHOULD NOT LEAK"}, + {"role": "debug", "content": "debug row"}, + {"role": "user"}, + {"role": "user", "content": ""}, + {"role": "user", "content": " "}, + "not-a-dict", + None, + {"role": "user", "content": "ok", "timestamp": "not-a-number"}, + {"role": "user", "content": "inf ts", "timestamp": float("inf")}, + ], + } + + +@pytest.fixture() +def loaded_legacy(tmp_path, monkeypatch): + """Write the raw legacy snapshot to an isolated store and load it for real.""" + store = tmp_path / "shares" + store.mkdir() + monkeypatch.setattr(shares, "SHARES_DIR", store) + token = "legacytoken" + (store / f"{token}.json").write_text(json.dumps(_legacy_snapshot()), encoding="utf-8") + out = shares.load_share(token) + assert out is not None, "load_share returned None; the fixture never loaded" + return out + + +def test_legacy_snapshot_publishes_only_approved_roles(loaded_legacy): + roles = [m.get("role") for m in loaded_legacy["messages"]] + bad = sorted({r for r in roles if r not in APPROVED_ROLES}) + assert not bad, ( + f"the anonymous share published forbidden roles {bad}. A stored snapshot " + "is untrusted: the read path must rebuild each row rather than spread it." + ) + assert roles == ["user", "assistant", "user", "user"], roles + + +def test_legacy_snapshot_publishes_no_unapproved_fields(loaded_legacy): + leaked = sorted( + {k for m in loaded_legacy["messages"] for k in m if k not in APPROVED_KEYS} + ) + assert not leaked, ( + f"the anonymous share published unapproved sibling fields {leaked}. " + "`{**message}` republishes whatever an older writer persisted." + ) + + +@pytest.mark.parametrize( + "secret", + [ + "sk-SHOULD-NOT-LEAK", + "SHOULD NOT LEAK", + "/etc/shadow", + "/home/samfp/private", + "private-session-id", + "internal-only", + ], +) +def test_legacy_snapshot_leaks_no_secret_substring(loaded_legacy, secret): + blob = json.dumps(loaded_legacy) + assert secret not in blob, ( + f"{secret!r} reached the anonymous /api/share payload" + ) + + +def test_legacy_snapshot_recomputes_the_message_count(loaded_legacy): + """A stored count must never survive row drops. + + The fixture stores 99 while only 4 rows are publishable. Trusting the stored + value tells the viewer that messages are missing rather than filtered. + """ + assert loaded_legacy["message_count"] == len(loaded_legacy["messages"]) + assert loaded_legacy["message_count"] == 4 + + +def test_legacy_snapshot_accepts_only_a_string_title(loaded_legacy): + """A structured title must not be stringified into its repr.""" + title = loaded_legacy["title"] + assert isinstance(title, str), type(title).__name__ + assert title == "Untitled", ( + f"a non-string stored title must fall back to Untitled, got {title!r}. " + "str() on a dict publishes dict syntax and every nested value." + ) + + +def test_legacy_snapshot_drops_non_finite_and_non_numeric_timestamps(loaded_legacy): + for m in loaded_legacy["messages"]: + if "timestamp" not in m: + continue + ts = m["timestamp"] + assert isinstance(ts, (int, float)) and not isinstance(ts, bool), ts + assert ts == ts and ts not in (float("inf"), float("-inf")), ts + + +def test_read_path_and_write_path_share_one_role_set(): + """The two paths must not drift. + + A read that accepted a wider role set than the write would let a legacy + snapshot serve rows that a fresh snapshot can never contain. + """ + assert shares._PUBLIC_SHARE_ROLES == frozenset({"user", "assistant"}) + src = Path(shares.__file__).read_text(encoding="utf-8") + assert src.count("_PUBLIC_SHARE_ROLES") >= 3, ( + "both _sanitize_message (write) and _guarded_public_message (read) must " + "consult the shared constant, not an inline literal" + ) + assert 'role not in {"user", "assistant"}' not in src, ( + "an inline role literal came back; it can drift from the shared constant" + ) + + +def test_load_path_performs_no_filesystem_reads_of_referenced_files(tmp_path, monkeypatch): + """The read guard classifies; it must never resolve or embed a local file.""" + store = tmp_path / "shares" + store.mkdir() + monkeypatch.setattr(shares, "SHARES_DIR", store) + secret = tmp_path / "secret.png" + secret.write_bytes(b"\x89PNG\r\n\x1a\nSECRETBYTES") + snapshot = { + "title": "legacy", + "messages": [ + {"role": "user", "content": f"![x](file://{secret})"}, + {"role": "user", "content": f"[y](/api/media?path={secret})"}, + ], + } + (store / "fs.json").write_text(json.dumps(snapshot), encoding="utf-8") + + opened: list[str] = [] + real_open = Path.open + + def tracking_open(self, *a, **kw): + opened.append(str(self)) + return real_open(self, *a, **kw) + + monkeypatch.setattr(Path, "open", tracking_open) + out = shares.load_share("fs") + assert out is not None + assert str(secret) not in json.dumps(out), "the referenced path was published" + assert not [p for p in opened if p == str(secret)], ( + f"the load path read the referenced file: {opened}" + ) + + +# ── Blocker 2: scheme-relative and browser-normalized descendants ──────────── + +_MUST_REFUSE = [ + ("scheme-relative loopback in query", "https://cdn.test/a.png?next=//127.0.0.1/x.png"), + ("scheme-relative loopback in fragment", "https://cdn.test/a.png#//127.0.0.1/x.png"), + ("scheme-relative private 10.x", "https://cdn.test/a.png?next=//10.0.0.5/x.png"), + ("scheme-relative private 192.168", "https://cdn.test/a.png?next=//192.168.1.9/x.png"), + ("scheme-relative metadata endpoint", "https://cdn.test/a.png?next=//169.254.169.254/latest/meta-data"), + ("scheme-relative localhost name", "https://cdn.test/a.png?next=//localhost:8080/x.png"), + ("scheme-relative with port", "https://cdn.test/a.png?next=//127.0.0.1:7777/x.png"), + ("encoded scheme-relative loopback", "https://cdn.test/a.png?next=%2F%2F127.0.0.1%2Fx.png"), + ("backslash authenticated route", "https://cdn.test/a.png?next=\\api\\media?path=/etc/shadow"), + ("backslash scheme-relative", "https://cdn.test/a.png?next=\\\\127.0.0.1\\x.png"), + ("dot-segment authenticated route", "https://cdn.test/a.png?next=/foo/../api/media?path=/x"), + ("dot-segment with backslash", "https://cdn.test/a.png?next=\\foo\\..\\api\\media?path=/x"), +] + +_MUST_PRESERVE = [ + ("plain public URL", "https://cdn.test/a.png"), + ("public with harmless query", "https://cdn.test/a.png?v=2"), + ("public wrapping a public absolute", "https://cdn.test/a.png?next=https://images.example.test/b.png"), + ("public wrapping a public scheme-relative", "https://cdn.test/a.png?next=//images.example.test/b.png"), + ("encoded percent literal", "https://cdn.test/a.png?pct=100%25"), + ("query value containing dots", "https://cdn.test/a.png?v=1.2.3"), +] + + +@pytest.mark.parametrize("label,value", _MUST_REFUSE, ids=[c[0] for c in _MUST_REFUSE]) +def test_private_descendant_is_refused(label, value): + assert public_reference_hides_local_target(value) is True, ( + f"{label}: {value!r} reached the preserve arm. A browser resolves this " + "against the containing scheme, so it reaches the private host exactly " + "as an absolute URL would." + ) + + +@pytest.mark.parametrize("label,value", _MUST_PRESERVE, ids=[c[0] for c in _MUST_PRESERVE]) +def test_public_reference_is_preserved(label, value): + """Over-blocking a genuine public asset is also a failure. + + These are the discriminating controls: a classifier that refused every + scheme-relative form would pass every refusal test above and still be wrong. + """ + assert public_reference_hides_local_target(value) is False, ( + f"{label}: {value!r} was refused, but a public host is not a local target" + ) + + +def test_scheme_relative_candidate_inherits_the_containing_scheme(): + """An ``http`` parent must classify its descendant as ``http``, not ``https``.""" + from api.share_refs import _scheme_relative_candidates + + found = _scheme_relative_candidates("/a.png?next=//127.0.0.1/x.png") + assert found == ["//127.0.0.1/x.png"], found + assert public_reference_hides_local_target("http://cdn.test/a.png?next=//127.0.0.1/x.png") is True + + +def test_browser_normalize_collapses_only_the_path(): + """A ``..`` inside a query value is not a path segment. + + Rewriting it would change the bytes a preserved public URL is compared + against, which breaks the byte-for-byte preservation contract. + """ + from api.share_refs import _browser_normalize + + assert _browser_normalize("/foo/../api/media?path=/x") == "/api/media?path=/x" + assert _browser_normalize("\\api\\media?path=/x") == "/api/media?path=/x" + # The query is left alone. + assert _browser_normalize("/a.png?v=1/../2") == "/a.png?v=1/../2" + + +def test_extra_slashes_still_name_a_host(): + """``///host`` is a live host reference, not an authority-less path. + + Verified against a real URL parser: ``new URL('///127.0.0.1/x.png', + 'https://cdn.test/a.png')`` resolves to host ``127.0.0.1``, because extra + leading slashes are collapsed. So a triple slash must be enumerated as a + candidate, exactly like a double slash. + + An earlier version of this test asserted the opposite and was wrong. The + code was right. + """ + from api.share_refs import _scheme_relative_candidates + + # No authority at all: nothing to reach. + assert _scheme_relative_candidates("/a.png?next=//") == [] + # Extra slashes collapse onto a real host, so this IS a candidate. + assert _scheme_relative_candidates("/a.png?next=///x") == ["//x"] + assert public_reference_hides_local_target( + "https://cdn.test/a.png?next=///127.0.0.1/x.png" + ) is True diff --git a/tests/test_smd_media_in_stream.py b/tests/test_smd_media_in_stream.py index a58f8b67560..dd2c1c602e4 100644 --- a/tests/test_smd_media_in_stream.py +++ b/tests/test_smd_media_in_stream.py @@ -60,16 +60,87 @@ def _extract_js_function(src: str, name: str) -> str: return src[start:pos] +def _run_tail_flush_partition_cases() -> dict: + """Execute the real stream-end partitioner with only its leaf sinks stubbed.""" + helpers = "\n".join( + [ + _extract_js_function(UI_JS, "_mediaPathSrc"), + _extract_js_function(UI_JS, "_mediaTokenRe"), + _extract_js_function(UI_JS, "_unquoteMediaRef"), + # Shared lexical ceiling: extracted, never stubbed, so these + # harnesses exercise the same length rule as production. + _extract_js_function(UI_JS, "_mediaTokenMaxLength"), + _extract_js_function(UI_JS, "_mediaTokenExceedsMaxLength"), + _extract_js_function(MESSAGES_JS, "_smdMediaWriteText"), + _extract_js_function(MESSAGES_JS, "_smdMediaTailEntryChunk"), + _extract_js_function(MESSAGES_JS, "_smdMediaCandidateMax"), + _extract_js_function(MESSAGES_JS, "_smdMediaTailFlushEntry"), + ] + ) + script = f""" +{helpers} +function run(raw, failRef=''){{ + const media=[]; + const text=[]; + const parent={{}}; + globalThis._smdAppendMediaNode=(_parent,ref)=>{{ + if(ref===failRef) return false; + media.push(ref); + return true; + }}; + _smdMediaTailFlushEntry({{ + chunk:raw, + parent, + data:{{}}, + baseAddText:null, + writeText:(_parent,_data,value)=>text.push(String(value)), + }}); + return {{media,text:text.join('')}}; +}} +console.log(JSON.stringify({{ + malformedThenValid:run('MEDIA:"/tmp/bad.png and MEDIA:/tmp/good.png'), + noMatch:run('no media here'), + appendFailureThenValid:run('MEDIA:/tmp/bad.png then MEDIA:/tmp/good.png','/tmp/bad.png'), +}})); +""" + completed = subprocess.run( + [NODE, "--input-type=module", "-e", script], + cwd=REPO_ROOT, + text=True, + capture_output=True, + check=True, + timeout=30, + ) + return json.loads(completed.stdout) + + def _run_real_smd_media_cases() -> dict: helpers = "\n".join( [ + # MEDIA: token matchers live in ui.js and are shared with the settled + # renderMd path, so the streaming functions below depend on them. + _extract_js_function(UI_JS, "_mediaPathSrc"), + _extract_js_function(UI_JS, "_mediaTokenRe"), + _extract_js_function(UI_JS, "_unquoteMediaRef"), + # Shared lexical ceiling: extracted, never stubbed, so these + # harnesses exercise the same length rule as production. + _extract_js_function(UI_JS, "_mediaTokenMaxLength"), + _extract_js_function(UI_JS, "_mediaTokenExceedsMaxLength"), _extract_js_function(MESSAGES_JS, "_smdMediaPrefixTail"), _extract_js_function(MESSAGES_JS, "_smdAppendPlainText"), _extract_js_function(MESSAGES_JS, "_smdMediaWriteText"), _extract_js_function(MESSAGES_JS, "_smdMediaTailSet"), _extract_js_function(MESSAGES_JS, "_smdMediaTailEntryChunk"), _extract_js_function(MESSAGES_JS, "_smdMediaTailSameOwner"), - _extract_js_function(MESSAGES_JS, "_smdMediaRefHasReliableBoundary"), + _extract_js_function(MESSAGES_JS, "_smdMediaTokenIsSettled"), + _extract_js_function(MESSAGES_JS, "_smdMediaTailCouldExtend"), + _extract_js_function(MESSAGES_JS, "_smdMediaHasOpenQuote"), + _extract_js_function(MESSAGES_JS, "_smdMediaOpenQuoteChar"), + _extract_js_function(MESSAGES_JS, "_smdMediaRefuseLine"), + _extract_js_function(MESSAGES_JS, "_smdMediaEntryRefused"), + _extract_js_function(MESSAGES_JS, "_smdMediaRunChar"), + _extract_js_function(MESSAGES_JS, "_smdMediaRefusedRunLength"), + _extract_js_function(MESSAGES_JS, "_smdMediaCandidateMax"), _extract_js_function(MESSAGES_JS, "_smdMediaTailFlushEntry"), _extract_js_function(MESSAGES_JS, "_smdMediaTailFlush"), _extract_js_function(MESSAGES_JS, "_smdMediaAwareAddText"), @@ -91,8 +162,7 @@ def _run_real_smd_media_cases() -> dict: "import * as smd from './static/vendor/smd.min.js';\n" "globalThis.window = { smd };\n" "globalThis.requestAnimationFrame = cb => cb();\n" - "const _MEDIA_TAIL_MAX = 4096;\n" - "const _SMD_MEDIA_PREFIX = 'MEDIA:';\n" + "const _SMD_MEDIA_PREFIX = 'MEDIA:';\n" "const _SMD_MEDIA_TAIL = new WeakMap();\n" "const __SMD_PARSER_FALLBACK = {};\n" "const _SMD_SAFE_URL_RE=/^(?:https?:|mailto:|tel:|message:|\\/|#|\\?|\\.|api|session\\/)/i;\n" @@ -190,6 +260,22 @@ def _run_real_smd_media_cases() -> dict: " return { html: root.outerHTML, text: root.textContent, liTexts: collectTagTexts(root, 'li'), fadeWords: collectClassTexts(root, 'stream-fade-word'), postProcessCalls, playbackCalls };\n" "}\n" "function renderModes(chunks){ return { safe: renderChunks(chunks, 'safe'), fade: renderChunks(chunks, 'fade') }; }\n" + "function renderCodePolicyModes(chunks){\n" + " const safe=renderChunks(chunks, 'safe');\n" + " _streamFadeReduceMotion=false;\n" + " const fade=renderChunks(chunks, 'fade');\n" + " _streamFadeReduceMotion=true;\n" + " const fadeReduced=renderChunks(chunks, 'fade');\n" + " _streamFadeReduceMotion=false;\n" + " return { safe, fade, fadeReduced };\n" + "}\n" + "const codePolicy={\n" + " fencedWhole: renderCodePolicyModes(['```\\nMEDIA:/tmp/a.png\\n```\\n']),\n" + " fencedSplit: renderCodePolicyModes(['```\\nMEDIA:/tmp/li', 've.png\\n```\\n']),\n" + " inlineWhole: renderCodePolicyModes(['see `MEDIA:/tmp/a.png` ok\\n']),\n" + " inlineSplit: renderCodePolicyModes(['see `MEDIA:/tmp/li', 've.png` ok\\n']),\n" + " plainControl: renderCodePolicyModes(['look MEDIA:/tmp/a.png ok\\n']),\n" + "};\n" "const marker='MEDIA:';\n" "const prefixSplits={};\n" "for(let i=1;i dict: "const pdf=renderModes(['MEDIA:C:/tmp/report.pdf ']);\n" "const falsePrefix=renderModes(['M', 'aybe plain prose ']);\n" "const crossParent=renderModes(['- ME', '\\n- ow']);\n" - "console.log(JSON.stringify({prefixSplits, refSplit, finalExtensionless, pdf, falsePrefix, crossParent}));\n" + "console.log(JSON.stringify({prefixSplits, refSplit, finalExtensionless, pdf, falsePrefix, crossParent, codePolicy}));\n" ) completed = subprocess.run( [NODE, "--input-type=module", "-e", script], @@ -211,6 +297,140 @@ def _run_real_smd_media_cases() -> dict: return json.loads(completed.stdout) +class TestSmdMediaCodeFencePolicy(unittest.TestCase): + """MEDIA stays active inside fenced/inline code in EVERY streaming mode. + + Settled ``renderMd()`` stashes MEDIA before its fence pass, so a MEDIA token + inside code renders as media. ``_safeSmdRenderer`` matched that, but + ``_streamFadeRenderer.add_text`` returned early through + ``_streamFadeSkipNode(parent)`` (which includes ``pre``/``code``) BEFORE its + MEDIA check, so fade streaming was the only mode that showed the raw path. + + These cases drive the real vendored parser (``static/vendor/smd.min.js``) in + safe, fade, and reduced-motion fade, and compare against each other rather + than against a re-implementation. + """ + + @classmethod + @unittest.skipIf(NODE is None, "node not on PATH") + def setUpClass(cls): + cls.policy = _run_real_smd_media_cases()["codePolicy"] + + def _assert_all_modes_agree(self, case_name): + case = self.policy[case_name] + safe, fade, reduced = case["safe"], case["fade"], case["fadeReduced"] + for mode_name, mode in (("fade", fade), ("fadeReduced", reduced)): + self.assertEqual( + mode["text"], safe["text"], + f"{case_name}: {mode_name} surrounding text must match safe exactly", + ) + self.assertNotIn( + "MEDIA:", mode["text"], + f"{case_name}: {mode_name} must not leave a raw MEDIA: path visible", + ) + return case + + def test_fenced_whole_token_renders_media_in_all_modes(self): + case = self._assert_all_modes_agree("fencedWhole") + for mode_name in ("safe", "fade", "fadeReduced"): + self.assertIn( + "/tmp/a.png", case[mode_name]["html"], + f"fencedWhole: {mode_name} must render the MEDIA token as a node", + ) + + def test_fenced_split_token_renders_media_in_all_modes(self): + case = self._assert_all_modes_agree("fencedSplit") + for mode_name in ("safe", "fade", "fadeReduced"): + self.assertIn( + "/tmp/live.png", case[mode_name]["html"], + f"fencedSplit: {mode_name} must complete the split token across chunks", + ) + + def test_inline_code_whole_token_renders_media_in_all_modes(self): + case = self._assert_all_modes_agree("inlineWhole") + for mode_name in ("safe", "fade", "fadeReduced"): + self.assertIn("/tmp/a.png", case[mode_name]["html"]) + self.assertIn( + "see", case[mode_name]["text"], + "surrounding prose must be conserved exactly", + ) + self.assertIn("ok", case[mode_name]["text"]) + + def test_inline_code_split_token_renders_media_in_all_modes(self): + case = self._assert_all_modes_agree("inlineSplit") + for mode_name in ("safe", "fade", "fadeReduced"): + self.assertIn("/tmp/live.png", case[mode_name]["html"]) + + def test_plain_prose_control_is_unchanged(self): + """The reorder must not alter the ordinary (non-code) MEDIA path.""" + case = self._assert_all_modes_agree("plainControl") + for mode_name in ("safe", "fade", "fadeReduced"): + self.assertIn("/tmp/a.png", case[mode_name]["html"]) + self.assertIn("look", case[mode_name]["text"]) + self.assertIn("ok", case[mode_name]["text"]) + + +class TestSmdMediaTailFlushPartition(unittest.TestCase): + """Stream-end flush must partition the WHOLE buffered candidate. + + The old flush ran the shared matcher once and required the match at offset 0, + then wrote the entire remaining suffix as prose. Any later valid token in the + same buffered candidate was emitted as literal ``MEDIA:`` text, while settled + ``renderMd()`` scans globally and rendered it — so stream and settled token + counts diverged. + """ + + @unittest.skipIf(NODE is None, "node not on PATH") + def setUp(self): + self.cases = _run_tail_flush_partition_cases() + + def test_malformed_open_quote_then_later_valid_token(self): + """An unterminated quoted ref stays PROSE; a later valid token still fires. + + Previously the unterminated `MEDIA:"/tmp/bad.png` fell through to the + bare grammar, which captured the leading-quote fragment `"/tmp/bad.png` + and activated it as a media node — a value that reaches Path() with a + literal `"` in it, and that settled parsing never yields. + + The shared grammar now excludes a quote from the FIRST character of every + unquoted alternative (ch_first / chFirst), so a quoted ref can only + activate media through the complete same-line quoted form. The malformed + span is preserved verbatim as literal text, and scanning continues so an + independent later token is unaffected. + """ + case = self.cases["malformedThenValid"] + self.assertEqual( + case["media"], ["/tmp/good.png"], + "only the later VALID token may become a media node; the " + "unterminated quoted ref must not activate", + ) + self.assertEqual( + case["text"], 'MEDIA:"/tmp/bad.png and ', + "the malformed span is preserved verbatim as literal text, " + "including its MEDIA: keyword and leading quote", + ) + + def test_no_match_writes_candidate_verbatim(self): + case = self.cases["noMatch"] + self.assertEqual(case["media"], []) + self.assertEqual( + case["text"], "no media here", + "a candidate with no token must be preserved byte-for-byte", + ) + + def test_append_failure_preserves_token_and_continues(self): + """A failed media append preserves that token's raw span, then continues.""" + case = self.cases["appendFailureThenValid"] + self.assertEqual( + case["media"], ["/tmp/good.png"], + "the second token must still be appended after the first fails", + ) + self.assertEqual( + case["text"], "MEDIA:/tmp/bad.png then ", + "the failed token keeps its exact raw span; no bytes are dropped", + ) + + class TestSmdMediaInStream(unittest.TestCase): """Verify the streaming smd path produces real for MEDIA tokens.""" @@ -244,13 +464,34 @@ def test_smd_media_aware_wrapper_invokes_shared_renderer(self): # The whole point of the fix: the streaming path uses the SAME renderer # the renderMd pipeline uses. If messages.js constructed the HTML # inline, the live + settled images could diverge. - idx = MESSAGES_JS.index("function _smdMediaAwareAddText") - block = MESSAGES_JS[idx:idx + 6000] + # + # The delegation is INDIRECT and always has been: + # _smdMediaAwareAddText -> _smdAppendMediaNode -> _inlineMediaHtmlForRef + # The previous spelling sliced a fixed 6000-character window after the + # wrapper and searched it for `_inlineMediaHtmlForRef`, which only ever + # passed because `_smdAppendMediaNode` happened to sit inside that + # window. That made the assertion sensitive to unrelated edits (adding + # comments to the wrapper broke it) while never actually pinning the + # chain. Assert each link by brace-accurate function extraction instead, + # so the test fails only if the delegation genuinely breaks. + wrapper = _extract_js_function(MESSAGES_JS, "_smdMediaAwareAddText") self.assertIn( - "_inlineMediaHtmlForRef", block, - "_smdMediaAwareAddText must call _inlineMediaHtmlForRef to keep " + "_smdAppendMediaNode", wrapper, + "_smdMediaAwareAddText must route MEDIA tokens through " + "_smdAppendMediaNode", + ) + appender = _extract_js_function(MESSAGES_JS, "_smdAppendMediaNode") + self.assertIn( + "_inlineMediaHtmlForRef", appender, + "_smdAppendMediaNode must call _inlineMediaHtmlForRef to keep " "streaming and settled MEDIA paths byte-identical", ) + # And the wrapper must not hand-roll its own markup as a side channel. + self.assertNotIn( + "