From 6303746ace8a844289f6bb6b33cb760bd1146a5b Mon Sep 17 00:00:00 2001 From: D41910 <158550716+D41910@users.noreply.github.com> Date: Wed, 7 Oct 2026 17:03:27 +0800 Subject: [PATCH] fix(filesystem): bound the literal storage-key branch by the segment cap _storage_key_segment defines _MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH and both the base32 and digest branches honor it, but the literal branch returned any literal-safe value verbatim with no upper bound. A 308-character session id therefore reached the filesystem unchanged and failed there with ENAMETOOLONG once the caller appended its extension, turning a valid id into a failed write. Apply the same bound to the literal branch so longer literal-safe values fall through to the encoded forms like any other over-long value. No data migration is affected: names in the newly-encoded range were already unwritable. --- .../core/agent_framework/_filesystem.py | 10 +- .../core/tests/core/test_filesystem.py | 95 ++++++++++++++++++- 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/python/packages/core/agent_framework/_filesystem.py b/python/packages/core/agent_framework/_filesystem.py index 512ca17ff7f..7811e8cf746 100644 --- a/python/packages/core/agent_framework/_filesystem.py +++ b/python/packages/core/agent_framework/_filesystem.py @@ -102,7 +102,13 @@ def _storage_key_segment(value: str, *, encoded_prefix: str) -> str: # pyright: The derivation has three branches: - * Literal-safe values are returned verbatim, so on-disk layouts stay readable. + * Literal-safe values no longer than :data:`_MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH` + are returned verbatim, so on-disk layouts stay readable. The length bound is + part of the literal branch because a verbatim segment is otherwise unbounded: + most filesystems cap a single name at 255 bytes and a caller appends its own + extension, so a long literal-safe value would reach the filesystem unchanged + and fail there with ``ENAMETOOLONG`` instead of being encoded. Longer values + fall through to the branches below. * Everything else is encoded under ``encoded_prefix`` using lowercase base32. This branch is **injective**: base32 is exactly reversible, and its alphabet (``a``-``z`` and ``2``-``7``) is case-stable, so the mapping stays one-to-one @@ -126,7 +132,7 @@ def _storage_key_segment(value: str, *, encoded_prefix: str) -> str: # pyright: Returns: A single path segment containing no path separators. """ - if _is_literal_storage_key_segment_safe(value): + if _is_literal_storage_key_segment_safe(value) and len(value) <= _MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH: return value encoded_value = b32encode(value.encode("utf-8")).decode("ascii").rstrip("=").lower() encoded_segment = f"{encoded_prefix}{encoded_value}" diff --git a/python/packages/core/tests/core/test_filesystem.py b/python/packages/core/tests/core/test_filesystem.py index 764f8307aac..00ded30f1c2 100644 --- a/python/packages/core/tests/core/test_filesystem.py +++ b/python/packages/core/tests/core/test_filesystem.py @@ -17,6 +17,8 @@ from __future__ import annotations +from pathlib import Path + import pytest from agent_framework._filesystem import ( # pyright: ignore[reportPrivateUsage] @@ -90,7 +92,10 @@ "session-1", "console", "cont", - "x" * 200, + # The literal branch is bounded by the segment cap, so the longest + # literal-safe example sits exactly at the cap. Over-cap lengths are + # covered by test_long_literal_safe_values_are_length_bounded. + "x" * _MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH, ) @@ -227,3 +232,91 @@ def test_derivation_is_deterministic(value: str) -> None: assert _storage_key_segment(value, encoded_prefix="~scope-") == _storage_key_segment( value, encoded_prefix="~scope-" ) + + +# region Length bound on the literal branch +# +# The literal branch used to return any literal-safe value verbatim, with no +# upper bound, so a long -- but otherwise perfectly safe -- identifier became a +# path segment longer than any filesystem accepts. Callers append their own +# extension (``_sessions.FileHistoryProvider`` appends ``.jsonl``), so the +# failure surfaced at the call site as ``OSError: [Errno 63] File name too long``. + + +@pytest.mark.parametrize("length", [181, 182, 200, 240, 249, 250, 255, 300, 308, 400, 1000]) +def test_long_literal_safe_values_are_length_bounded(length: int) -> None: + """A literal-safe value over the cap is encoded rather than returned verbatim. + + Only the length disqualifies these values; their charset is still + literal-safe. They must land in the encoded namespace and respect the cap + like any other over-long value, so the derived name fits a filesystem slot. + """ + value = "a" * length + assert _is_literal_storage_key_segment_safe(value), "charset is still literal-safe; only the length changed" + + segment = _storage_key_segment(value, encoded_prefix="~session-") + + assert segment != value + assert segment.startswith("~session-") + assert len(segment) <= _MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH + + +@pytest.mark.parametrize("length", [1, 2, 175, 178, 179, 180]) +def test_literal_safe_values_within_the_cap_stay_verbatim(length: int) -> None: + """The bound must not move: at or under the cap the value is still literal. + + Deployments rely on readable on-disk layouts, so the fix may only affect + values that the previous branch would have made unusable on disk. + """ + value = "a" * length + assert _storage_key_segment(value, encoded_prefix="~session-") == value + + +@pytest.mark.parametrize("encoded_prefix", ["~scope-", "~session-", "~todo-", "~memory-", "~access-"]) +def test_no_derived_segment_exceeds_the_cap(encoded_prefix: str) -> None: + """Every branch honors the cap, so every derived name fits a filesystem slot. + + The cap is this module's own invariant; a segment above it is a name the + filesystem rejects at the call site rather than a usable storage location. + """ + probe_values = ( + *COLLIDING_IDENTIFIERS, + *UNSAFE_IDENTIFIERS, + *SAFE_IDENTIFIERS, + "a" * 1000, + "session-" + "a" * 300, + ) + for value in probe_values: + segment = _storage_key_segment(value, encoded_prefix=encoded_prefix) + assert len(segment) <= _MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH, (value[:32], encoded_prefix, len(segment)) + + +def test_length_bounded_literals_stay_injective() -> None: + """Moving long literals into the encoded namespace must preserve injectivity. + + The segment is an isolation boundary, so two distinct over-long identifiers + may not be folded together by the change -- not even after case folding. + """ + values = ["a" * length for length in range(_MAX_ENCODED_STORAGE_KEY_SEGMENT_LENGTH + 1, 400)] + segments = [_storage_key_segment(value, encoded_prefix="~session-") for value in values] + + assert len(set(segments)) == len(values) + assert len({segment.lower() for segment in segments}) == len(values) + + +def test_long_literal_safe_value_is_creatable_on_a_real_filesystem(tmp_path: Path) -> None: + """Regression guard: the derived name must survive a real ``open()``. + + A 308-character literal-safe session id previously produced a 308-character + file stem, which ``open()`` rejects with ``ENAMETOOLONG`` (errno 63) on + macOS, Linux and Windows alike -- turning a valid session id into a failed + write. Writing ``stem + extension`` is exactly what + ``_sessions.FileHistoryProvider`` does. + """ + value = "session-" + "a" * 300 + segment = _storage_key_segment(value, encoded_prefix="~session-") + + target = tmp_path / f"{segment}.jsonl" + target.write_text('{"role":"user"}\n', encoding="utf-8") + + assert target.read_text(encoding="utf-8") == '{"role":"user"}\n'