fix(sessions): sanitize Unicode in session titles like tags#1067
Open
zied-jlassi wants to merge 1 commit into
Open
fix(sessions): sanitize Unicode in session titles like tags#1067zied-jlassi wants to merge 1 commit into
zied-jlassi wants to merge 1 commit into
Conversation
tag_session/tag_session_via_store run titles through _sanitize_unicode (NFKC + strip format/bidi/zero-width chars) before persisting, but the twin custom-title fields did not: rename_session, rename_session_via_store and fork_session only .strip()ed. A title and a tag are both user-controlled metadata surfaced identically by list_sessions, so a bidi-override or zero-width title was persisted and rendered unsanitized while the same string as a tag was cleaned. The fork case also derives a title from the source transcript (customTitle / aiTitle / first prompt) — content that may not be trusted — and previously copied it through verbatim. Apply _sanitize_unicode(...).strip() at all three sites so the custom-title path matches the tag path (an all-invisible title now raises 'title must be non-empty', consistent with tag_session). NFKC normalization now applies to titles as it already does to tags. Tests: rename sanitization + pure-invisible rejection, explicit fork title sanitization, and derived fork title sanitization (untrusted transcript content). Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
tag_session/tag_session_via_storerun a tag through_sanitize_unicode(NFKC + strip format/bidi/zero-width categories) before persisting it. The twin custom-title fields do not:rename_session,rename_session_via_store, andfork_session's title only.strip().A title and a tag are both user-controlled metadata that
list_sessionssurfaces identically. As a result a title carrying a bidi override (U+202E) or zero-width chars (U+200B, BOM) is persisted and rendered unsanitized, while the exact same string used as a tag is cleaned. Theforkpath additionally derives a title from the source transcript (customTitle/aiTitle/ first prompt) and copied it through verbatim — so untrusted transcript content reached the displayed title.This is a consistency/hardening fix (low severity — title-spoofing in terminal/UI listings), not an exploitable vulnerability.
Change
Apply
_sanitize_unicode(...).strip()at the three custom-title sites so they match the tag path:rename_session,rename_session_via_storefork_session(both the explicit title and the transcript-derived title)Behavior notes (matching
tag_session): a title that is only invisible characters now raisestitle must be non-empty; NFKC normalization now applies to titles as it already does to tags.Tests
Added, mirroring the existing tag sanitization tests:
test_unicode_sanitization+test_sanitization_rejects_pure_invisible(rename)test_fork_title_unicode_sanitized(explicit fork title)test_fork_derived_title_unicode_sanitized(title derived from an unsanitized source transcript)Verified against the repo toolchain (in an isolated container):
uvx ruff check src/ tests/andruff format --check-> cleanuv run mypy src/-> Success, no issues in 24 source filesuv run pytest-> 986 passed, 5 skippedDisclosure
AI-assisted (analysis + implementation), reviewed by me. Commit is DCO signed-off.