From 2c5adb7acab9da7d0e8fd4e58def33f22322ebb0 Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 7 Jul 2026 22:46:36 -0700 Subject: [PATCH] fix(web): sanitize the latin1_safe_filename fallback too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: the helper returned `fallback` verbatim when the name sanitized to empty, so a future caller passing an unsafe fallback (non-latin-1, control chars, quote, backslash) could reintroduce the header crash/corruption the helper exists to prevent. Not reachable today — all call sites pass safe ASCII literals — but the helper is a shared safety primitive whose contract is wire-safe output. Run the fallback through the same cleaning, backed by a safe constant if even that is empty, so the return is always wire-safe and never filename="". Adds a test. --- tests/test_web_helpers.py | 12 ++++++++++++ turnstone/core/web_helpers.py | 15 ++++++++++----- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/test_web_helpers.py b/tests/test_web_helpers.py index c430433b..95789bac 100644 --- a/tests/test_web_helpers.py +++ b/tests/test_web_helpers.py @@ -184,3 +184,15 @@ class TestLatin1SafeFilename: # An all-CJK name folds to '???' (truthy) — must NOT hit the fallback. assert latin1_safe_filename("日本語", fallback="preview") == "???" + + def test_fallback_is_also_sanitized(self): + from turnstone.core.web_helpers import latin1_safe_filename + + # The fallback fires only when the name sanitizes to empty, and it is + # cleaned by the SAME rules — a caller can't reintroduce the crash / + # corruption through an unsafe fallback. + assert latin1_safe_filename("", fallback="—\x00.txt") == "?.txt" + self._assert_wire_safe(latin1_safe_filename("", fallback="bad\\\x00name")) + # If even the fallback sanitizes to empty, a safe constant backs it — + # never filename="". + assert latin1_safe_filename("", fallback='"\x00') == "download" diff --git a/turnstone/core/web_helpers.py b/turnstone/core/web_helpers.py index 1db39777..4375a7b6 100644 --- a/turnstone/core/web_helpers.py +++ b/turnstone/core/web_helpers.py @@ -33,12 +33,17 @@ def latin1_safe_filename(name: str, *, fallback: str = "attachment") -> str: C0 / C1 control ranges, and zero-width / bidi format chars) and both quoted-string metacharacters, then fold any surviving non-ASCII codepoint to ``?``. The result is pure printable ASCII with no ``"`` or ``\\`` — - latin-1 clean, control-free, and safely quotable. Falls back to - ``fallback`` when nothing survives, so the header never emits - ``filename=""``. + latin-1 clean, control-free, and safely quotable. ``fallback`` is run + through the same cleaning (so a caller can't reintroduce the crash via an + unsafe fallback), backed by a safe constant if even that is empty — the + return is always wire-safe and never ``filename=""``. """ - kept = "".join(c for c in name if c.isprintable() and c not in '"\\') - return kept.encode("ascii", errors="replace").decode("ascii") or fallback + + def _clean(s: str) -> str: + kept = "".join(c for c in s if c.isprintable() and c not in '"\\') + return kept.encode("ascii", errors="replace").decode("ascii") + + return _clean(name) or _clean(fallback) or "download" def skill_summary_rows(storage: Any) -> list[dict[str, Any]]: