From 43f08448b772cd5f0273d066c712a186a8d986d3 Mon Sep 17 00:00:00 2001 From: kangkangzi2025 Date: Mon, 6 Jul 2026 22:41:44 +0800 Subject: [PATCH] codex security --- src/rath/backend/persistence/paths.py | 6 +- src/rath/flow/memory_inject.py | 8 +- src/rath/memory/adapters/local.py | 191 +++++++++++++++++- src/rath/memory/persistence/paths.py | 3 +- src/rath/session/persistence/paths.py | 7 +- src/rath/utils/ids.py | 23 +++ .../backends/persistence/test_registry_gc.py | 11 + tests/flow/test_agent_forward_memory.py | 14 +- tests/flow/test_memory_inject.py | 3 +- tests/memory/local_backend/conftest.py | 16 +- .../local_backend/test_local_memory_commit.py | 18 ++ .../test_local_memory_resource.py | 43 +++- tests/memory/persistence/test_persistence.py | 11 + tests/session/persistence/test_loader_gc.py | 12 ++ 14 files changed, 340 insertions(+), 26 deletions(-) create mode 100644 src/rath/utils/ids.py diff --git a/src/rath/backend/persistence/paths.py b/src/rath/backend/persistence/paths.py index c11f4f9..f4a64a2 100644 --- a/src/rath/backend/persistence/paths.py +++ b/src/rath/backend/persistence/paths.py @@ -15,6 +15,7 @@ from uuid import UUID from rath.config.paths import resolve_config_dir +from rath.utils.ids import coerce_uuid_str __all__ = [ "SANDBOXES_DIR_NAME", @@ -48,7 +49,7 @@ def local_root() -> Path: def local_sandbox_dir(sandbox_id: UUID | str) -> Path: """``/`` — the stable working_dir for a Local sandbox.""" - return local_root() / str(sandbox_id) + return local_root() / coerce_uuid_str(sandbox_id, field="sandbox_id") def opensandbox_root() -> Path: @@ -58,7 +59,8 @@ def opensandbox_root() -> Path: def opensandbox_index_path(sandbox_id: UUID | str) -> Path: """``/.json`` — registry entry for a remote sandbox.""" - return opensandbox_root() / f"{sandbox_id}{OPENSANDBOX_INDEX_SUFFIX}" + sid = coerce_uuid_str(sandbox_id, field="sandbox_id") + return opensandbox_root() / f"{sid}{OPENSANDBOX_INDEX_SUFFIX}" def ensure_local_root() -> Path: diff --git a/src/rath/flow/memory_inject.py b/src/rath/flow/memory_inject.py index 291cc19..4717b23 100644 --- a/src/rath/flow/memory_inject.py +++ b/src/rath/flow/memory_inject.py @@ -3,8 +3,7 @@ A policy reads a :class:`~rath.session.session.Session` and a :class:`~rath.memory.abc.MemoryStore`, then returns a tuple of :class:`~rath.session.chunk.ChunkRow` to prepend to the next loop turn -(typically :attr:`ChunkKind.SYSTEM` notes that summarize relevant -recalled memories). +as untrusted user-context notes that summarize relevant recalled memories. The injection step must NEVER raise into the session loop — on store errors or a closed store, return an empty tuple and log a warning so @@ -101,5 +100,6 @@ def _last_user_message(session: Session) -> str | None: def _hit_to_chunk(hit: MemoryHit) -> ChunkRow: snippet = (hit.snippet or "").strip() - body = f"[memory:{hit.uri}] {snippet}" if snippet else f"[memory:{hit.uri}]" - return ChunkRow(kind=ChunkKind.SYSTEM, payload={"content": body}) + prefix = f"[untrusted memory:{hit.uri}]" + body = f"{prefix} {snippet}" if snippet else prefix + return ChunkRow(kind=ChunkKind.USER, payload={"content": body}) diff --git a/src/rath/memory/adapters/local.py b/src/rath/memory/adapters/local.py index c0c5bd6..5257cc4 100644 --- a/src/rath/memory/adapters/local.py +++ b/src/rath/memory/adapters/local.py @@ -13,10 +13,12 @@ from __future__ import annotations import hashlib +import ipaddress import json import logging import math import re +import socket import urllib.error import urllib.parse import urllib.request @@ -76,6 +78,8 @@ _VEC_SUFFIX = ".vec" _META_SUFFIX = ".meta.json" _HIDDEN_SUFFIXES: frozenset[str] = frozenset({_VEC_SUFFIX, _META_SUFFIX}) +_DEFAULT_RESOURCE_MAX_BYTES = 10 * 1024 * 1024 +_SAFE_SEGMENT_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") _CAPABILITIES = MemoryCapabilities( @@ -118,6 +122,14 @@ class _LocalHandle: chat_init_failed: bool = field(default=False) +@dataclass(frozen=True, slots=True) +class _ResourcePolicy: + local_roots: tuple[Path, ...] + allowed_http_hosts: frozenset[str] + allow_private_hosts: bool + max_bytes: int + + @register("local") class LocalMemoryBackend(MemoryBackend): """Filesystem-backed memory backend, default for ``pip install openrath``.""" @@ -306,13 +318,22 @@ def _dispatch_resource( if isinstance(target_path, MemoryExecutionFailure): return target_path + policy = _resource_policy(bound.options) try: - raw_bytes, original_name, source_label = _fetch_resource(op.source) + raw_bytes, original_name, source_label = _fetch_resource( + op.source, + policy=policy, + ) except FileNotFoundError as exc: return MemoryExecutionFailure( kind="not_found", message=f"resource source not found: {exc}", ) + except _ResourceAccessDenied as exc: + return MemoryExecutionFailure( + kind="unauthorized", + message=f"resource source not allowed: {exc}", + ) except _ResourceFetchError as exc: return MemoryExecutionFailure( kind="transport", @@ -362,9 +383,20 @@ def _dispatch_commit( kind="invalid_uri", message="MemoryOpCommit requires non-empty session_id", ) + session_id = _safe_storage_segment(op.session_id, field="session_id") + if isinstance(session_id, MemoryExecutionFailure): + return session_id # Archive messages.json under session//commits//. stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S%f") - commit_root = bound.path / "session" / op.session_id / "commits" / stamp + commit_root = _contained_child( + bound.path, + "session", + session_id, + "commits", + stamp, + ) + if isinstance(commit_root, MemoryExecutionFailure): + return commit_root commit_root.mkdir(parents=True, exist_ok=True) archive_path = commit_root / "messages.json" normalized = [_normalize_message(m) for m in op.messages] @@ -373,7 +405,7 @@ def _dispatch_commit( encoding="utf-8", ) archived_uri = ( - f"{MEMORY_URI_PREFIX}session/{op.session_id}/commits/{stamp}/messages.json" + f"{MEMORY_URI_PREFIX}session/{session_id}/commits/{stamp}/messages.json" ) if not op.wait: @@ -987,11 +1019,63 @@ class _ResourceFetchError(Exception): """Wraps transport errors when fetching a remote resource.""" -def _fetch_resource(source: str) -> tuple[bytes, str, str]: +class _ResourceAccessDenied(Exception): + """Raised when a resource source violates the configured policy.""" + + +class _NoRedirect(urllib.request.HTTPRedirectHandler): + """Reject redirects so each fetched URL is policy-checked directly.""" + + def redirect_request(self, *args: Any, **kwargs: Any) -> None: # noqa: D401 + return None + + +_NO_REDIRECT_OPENER = urllib.request.build_opener(_NoRedirect) + + +def _resource_policy(options: dict[str, Any]) -> _ResourcePolicy: + roots = tuple( + Path(p).expanduser().resolve(strict=False) + for p in _option_strings(options.get("resource_import_roots")) + ) + hosts = frozenset( + h.lower() for h in _option_strings(options.get("resource_allowed_http_hosts")) + ) + raw_max = options.get("resource_max_bytes", _DEFAULT_RESOURCE_MAX_BYTES) + try: + max_bytes = int(raw_max) + except (TypeError, ValueError): + max_bytes = _DEFAULT_RESOURCE_MAX_BYTES + return _ResourcePolicy( + local_roots=roots, + allowed_http_hosts=hosts, + allow_private_hosts=bool(options.get("resource_allow_private_hosts", False)), + max_bytes=max(1, max_bytes), + ) + + +def _option_strings(raw: Any) -> tuple[str, ...]: + if raw is None: + return () + if isinstance(raw, str): + return (raw,) + try: + return tuple(str(x) for x in raw) + except TypeError: + return (str(raw),) + + +def _fetch_resource( + source: str, + *, + policy: _ResourcePolicy, +) -> tuple[bytes, str, str]: """Resolve ``source`` to ``(bytes, original_name, source_label)``. - ``source`` can be a local filesystem path, a ``file://`` URI, or an - ``http(s)://`` URL. Anything else is treated as a local path. + Local paths and HTTP(S) URLs are disabled by default. Callers must opt in + with ``resource_import_roots`` or ``resource_allowed_http_hosts`` in the + store options so untrusted resource ingest cannot read host files or SSRF + internal services. """ parsed = urllib.parse.urlparse(source) scheme = parsed.scheme.lower() @@ -999,11 +1083,21 @@ def _fetch_resource(source: str) -> tuple[bytes, str, str]: if len(scheme) == 1 and scheme.isalpha(): scheme = "" if scheme in ("http", "https"): + _validate_resource_url(parsed, policy=policy) try: - with urllib.request.urlopen(source, timeout=30) as resp: # noqa: S310 - data = resp.read() + with _NO_REDIRECT_OPENER.open(source, timeout=30) as resp: + raw_len = resp.headers.get("Content-Length") + if raw_len is not None and int(raw_len) > policy.max_bytes: + raise _ResourceFetchError( + f"resource exceeds {policy.max_bytes} bytes", + ) + data = resp.read(policy.max_bytes + 1) except urllib.error.URLError as exc: raise _ResourceFetchError(str(exc)) from exc + except ValueError as exc: + raise _ResourceFetchError(str(exc)) from exc + if len(data) > policy.max_bytes: + raise _ResourceFetchError(f"resource exceeds {policy.max_bytes} bytes") name = Path(parsed.path).name or "resource" return data, name, source if scheme == "file": @@ -1012,11 +1106,92 @@ def _fetch_resource(source: str) -> tuple[bytes, str, str]: local = Path(source) else: raise _ResourceFetchError(f"unsupported scheme: {scheme!r}") + local = local.expanduser().resolve(strict=False) + _validate_local_resource_path(local, policy=policy) if not local.is_file(): raise FileNotFoundError(str(local)) + size = local.stat().st_size + if size > policy.max_bytes: + raise _ResourceFetchError(f"resource exceeds {policy.max_bytes} bytes") return local.read_bytes(), local.name, str(local) +def _validate_local_resource_path(local: Path, *, policy: _ResourcePolicy) -> None: + if not policy.local_roots: + raise _ResourceAccessDenied("local paths require resource_import_roots") + for root in policy.local_roots: + try: + local.relative_to(root) + except ValueError: + continue + return + raise _ResourceAccessDenied(f"{local} is outside resource_import_roots") + + +def _validate_resource_url( + parsed: urllib.parse.ParseResult, + *, + policy: _ResourcePolicy, +) -> None: + host = (parsed.hostname or "").lower() + if not host: + raise _ResourceAccessDenied("HTTP(S) resource URL requires a host") + if host not in policy.allowed_http_hosts: + raise _ResourceAccessDenied("HTTP(S) host is not allowed") + try: + infos = socket.getaddrinfo( + host, + parsed.port or (443 if parsed.scheme == "https" else 80), + type=socket.SOCK_STREAM, + ) + except OSError as exc: + raise _ResourceFetchError(str(exc)) from exc + for info in infos: + address = info[4][0] + try: + ip = ipaddress.ip_address(address) + except ValueError as exc: + raise _ResourceAccessDenied(f"cannot validate address {address!r}") from exc + if _is_restricted_address(ip) and not policy.allow_private_hosts: + raise _ResourceAccessDenied("HTTP(S) host resolves to a private address") + + +def _is_restricted_address(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + return ( + ip.is_loopback + or ip.is_private + or ip.is_link_local + or ip.is_multicast + or ip.is_reserved + or ip.is_unspecified + ) + + +def _safe_storage_segment(value: str, *, field: str) -> str | MemoryExecutionFailure: + text = str(value) + if not text or text in (".", "..") or not _SAFE_SEGMENT_RE.fullmatch(text): + return MemoryExecutionFailure( + kind="invalid_uri", + message=f"{field} must be a single safe path segment", + ) + return text + + +def _contained_child( + root: Path, + *parts: str, +) -> Path | MemoryExecutionFailure: + try: + resolved = root.joinpath(*parts).resolve(strict=False) + resolved.relative_to(root.resolve(strict=False)) + except (OSError, ValueError) as exc: + return MemoryExecutionFailure( + kind="invalid_uri", + message=f"path escapes store root: {exc}", + ) + return resolved + + def _walk_tree( dir_path: Path, uri_base: str, diff --git a/src/rath/memory/persistence/paths.py b/src/rath/memory/persistence/paths.py index 3545681..5ae2d2f 100644 --- a/src/rath/memory/persistence/paths.py +++ b/src/rath/memory/persistence/paths.py @@ -15,6 +15,7 @@ from uuid import UUID from rath.config.paths import resolve_config_dir +from rath.utils.ids import coerce_uuid_str __all__ = [ "MEMORY_DIR_NAME", @@ -41,7 +42,7 @@ def local_memory_root() -> Path: def local_store_dir(store_id: UUID | str) -> Path: """``/`` — the per-store directory.""" - return local_memory_root() / str(store_id) + return local_memory_root() / coerce_uuid_str(store_id, field="store_id") def ensure_local_memory_root() -> Path: diff --git a/src/rath/session/persistence/paths.py b/src/rath/session/persistence/paths.py index 1a15eb9..0ce9fa0 100644 --- a/src/rath/session/persistence/paths.py +++ b/src/rath/session/persistence/paths.py @@ -14,6 +14,7 @@ from uuid import UUID from rath.config.paths import resolve_config_dir +from rath.utils.ids import coerce_uuid_str __all__ = [ "SESSIONS_DIR_NAME", @@ -41,7 +42,8 @@ def session_file(session_id: UUID | str) -> Path: Accepts either a :class:`uuid.UUID` or a string; both are normalized via ``str(id)`` so callers don't have to think about it. """ - return sessions_dir() / f"{session_id}{SESSION_FILE_SUFFIX}" + sid = coerce_uuid_str(session_id, field="session_id") + return sessions_dir() / f"{sid}{SESSION_FILE_SUFFIX}" def session_partial_file(session_id: UUID | str) -> Path: @@ -53,7 +55,8 @@ def session_partial_file(session_id: UUID | str) -> Path: writing process crashed mid-session (or that the runtime drain timed out and abandoned the writer). """ - return sessions_dir() / f"{session_id}{SESSION_PARTIAL_SUFFIX}" + sid = coerce_uuid_str(session_id, field="session_id") + return sessions_dir() / f"{sid}{SESSION_PARTIAL_SUFFIX}" def ensure_sessions_dir() -> Path: diff --git a/src/rath/utils/ids.py b/src/rath/utils/ids.py new file mode 100644 index 0000000..48500f7 --- /dev/null +++ b/src/rath/utils/ids.py @@ -0,0 +1,23 @@ +"""Identifier normalization helpers for filesystem-backed persistence.""" + +from __future__ import annotations + +from uuid import UUID + +__all__ = ["coerce_uuid_str"] + + +def coerce_uuid_str(value: UUID | str, *, field: str = "id") -> str: + """Return ``value`` as a canonical UUID string or raise ``ValueError``. + + Persistence identifiers are used as path components. Accepting arbitrary + strings here would make path traversal possible, so string inputs must be + parseable UUIDs before they can reach filesystem helpers. + """ + + if isinstance(value, UUID): + return str(value) + try: + return str(UUID(str(value))) + except ValueError as exc: + raise ValueError(f"{field} must be a UUID") from exc diff --git a/tests/backends/persistence/test_registry_gc.py b/tests/backends/persistence/test_registry_gc.py index 812d1d6..5649099 100644 --- a/tests/backends/persistence/test_registry_gc.py +++ b/tests/backends/persistence/test_registry_gc.py @@ -56,6 +56,17 @@ def test_ensure_local_idempotent(_isolate_openrath_home: Path) -> None: assert first.is_dir() +@pytest.mark.parametrize("bad_id", ["../escape", "/tmp/escape", "not-a-uuid"]) +def test_sandbox_paths_reject_path_like_ids( + _isolate_openrath_home: Path, + bad_id: str, +) -> None: + with pytest.raises(ValueError, match="sandbox_id must be a UUID"): + local_sandbox_dir(bad_id) + with pytest.raises(ValueError, match="sandbox_id must be a UUID"): + opensandbox_index_path(bad_id) + + def test_list_local_enumerates_uuid_dirs(_isolate_openrath_home: Path) -> None: reg = PersistentSandboxRegistry() ids = {reg.alloc_local_id() for _ in range(3)} diff --git a/tests/flow/test_agent_forward_memory.py b/tests/flow/test_agent_forward_memory.py index 5c7585a..fb6dfda 100644 --- a/tests/flow/test_agent_forward_memory.py +++ b/tests/flow/test_agent_forward_memory.py @@ -119,7 +119,7 @@ def test_forward_without_memory_is_unchanged() -> None: assert kinds[-1] == ChunkKind.ASSISTANT -def test_forward_prepends_injected_system_chunks() -> None: +def test_forward_prepends_injected_user_context_chunks() -> None: backend = _FakeBackend( find_hits=( MemoryHit( @@ -146,13 +146,19 @@ def test_forward_prepends_injected_system_chunks() -> None: # Exactly one MemoryOpFind dispatched, no commit. assert any(isinstance(op, MemoryOpFind) for op in backend.ops_seen) assert not any(isinstance(op, MemoryOpCommit) for op in backend.ops_seen) - # The injected snippet should appear as a system chunk in the output. - sys_bodies = "\n".join( + # The injected snippet should appear as untrusted user-context, not SYSTEM. + user_bodies = "\n".join( str(r.payload.get("content", "")) for r in out.chunk_table.rows + if r.kind == ChunkKind.USER + ) + assert "[untrusted memory:" in user_bodies + assert "loves dark mode" in user_bodies + assert all( + "loves dark mode" not in str(r.payload.get("content", "")) + for r in out.chunk_table.rows if r.kind == ChunkKind.SYSTEM ) - assert "loves dark mode" in sys_bodies def test_forward_with_commit_on_forward_dispatches_one_commit() -> None: diff --git a/tests/flow/test_memory_inject.py b/tests/flow/test_memory_inject.py index 8a4b796..41983cc 100644 --- a/tests/flow/test_memory_inject.py +++ b/tests/flow/test_memory_inject.py @@ -101,9 +101,10 @@ def test_default_recall_emits_one_chunk_per_hit() -> None: assert isinstance(chunks, tuple) assert len(chunks) == 2 for chunk in chunks: - assert chunk.kind == ChunkKind.SYSTEM + assert chunk.kind == ChunkKind.USER assert "content" in chunk.payload bodies = "\n".join(c.payload["content"] for c in chunks) + assert "[untrusted memory:" in bodies assert "dark mode preferred" in bodies assert "GMT+8 timezone" in bodies # The dispatch should carry the configured target_uri + top_k diff --git a/tests/memory/local_backend/conftest.py b/tests/memory/local_backend/conftest.py index b9d0dd5..1e29862 100644 --- a/tests/memory/local_backend/conftest.py +++ b/tests/memory/local_backend/conftest.py @@ -7,7 +7,7 @@ import pytest -from rath.memory import MemoryStore +from rath.memory import MemoryStore, MemoryStoreSpec from rath.memory.adapters.local import LocalMemoryBackend @@ -30,8 +30,18 @@ def backend() -> Iterator[LocalMemoryBackend]: @pytest.fixture -def store(backend: LocalMemoryBackend) -> Iterator[MemoryStore]: - s = backend.open() +def store( + backend: LocalMemoryBackend, + tmp_path: Path, +) -> Iterator[MemoryStore]: + spec = MemoryStoreSpec( + options={ + "resource_import_roots": [str(tmp_path)], + "resource_allowed_http_hosts": ["127.0.0.1"], + "resource_allow_private_hosts": True, + }, + ) + s = backend.open(spec) try: yield s finally: diff --git a/tests/memory/local_backend/test_local_memory_commit.py b/tests/memory/local_backend/test_local_memory_commit.py index 6d15840..b9dfc6f 100644 --- a/tests/memory/local_backend/test_local_memory_commit.py +++ b/tests/memory/local_backend/test_local_memory_commit.py @@ -21,6 +21,7 @@ from rath.memory.op_types import MemoryOpCommit, MemoryOpList, MemoryOpRead from rath.memory.results import ( MemoryCommitResult, + MemoryExecutionFailure, MemoryListResult, MemoryReadResult, ) @@ -130,6 +131,23 @@ def test_commit_with_no_chat_client_skips_extraction( assert not extracted_dir.exists() +def test_commit_rejects_path_like_session_id( + backend: LocalMemoryBackend, store: MemoryStore, tmp_path: Path +) -> None: + outside = tmp_path / "outside" + res = backend.dispatch( + store, + MemoryOpCommit( + session_id=f"../{outside.name}", + messages=[{"role": "user", "content": "x"}], + wait=False, + ), + ) + assert isinstance(res, MemoryExecutionFailure) + assert res.kind == "invalid_uri" + assert not outside.exists() + + # ----------------------------------------------------- Extraction (wait=True + live chat) diff --git a/tests/memory/local_backend/test_local_memory_resource.py b/tests/memory/local_backend/test_local_memory_resource.py index dba316f..158d37b 100644 --- a/tests/memory/local_backend/test_local_memory_resource.py +++ b/tests/memory/local_backend/test_local_memory_resource.py @@ -16,7 +16,7 @@ import pytest -from rath.memory import MemoryStore +from rath.memory import MemoryStore, MemoryStoreSpec from rath.memory.adapters.local import LocalMemoryBackend from rath.memory.op_types import MemoryOpResource from rath.memory.results import ( @@ -99,6 +99,20 @@ def test_resource_ingest_missing_local_file_is_not_found( assert res.kind == "not_found" +def test_resource_ingest_rejects_local_file_without_import_root( + backend: LocalMemoryBackend, tmp_path: Path +) -> None: + src = tmp_path / "secret.txt" + src.write_text("secret", encoding="utf-8") + unsafe_store = backend.open() + try: + res = backend.dispatch(unsafe_store, MemoryOpResource(source=str(src))) + finally: + backend.close(unsafe_store) + assert isinstance(res, MemoryExecutionFailure) + assert res.kind == "unauthorized" + + def test_resource_ingest_rejects_unknown_target_scope( backend: LocalMemoryBackend, store: MemoryStore, tmp_path: Path ) -> None: @@ -182,3 +196,30 @@ def test_resource_ingest_http_url_persists_body( assert blob.read_bytes() == _ServeBody.BODY meta_body = (root / "meta.md").read_text(encoding="utf-8") assert "127.0.0.1" in meta_body # original URL in meta + + +def test_resource_ingest_rejects_http_url_without_allowed_host( + backend: LocalMemoryBackend, http_server: str +) -> None: + unsafe_store = backend.open() + try: + res = backend.dispatch(unsafe_store, MemoryOpResource(source=http_server)) + finally: + backend.close(unsafe_store) + assert isinstance(res, MemoryExecutionFailure) + assert res.kind == "unauthorized" + + +def test_resource_ingest_blocks_private_host_without_opt_in( + backend: LocalMemoryBackend, http_server: str +) -> None: + spec = MemoryStoreSpec( + options={"resource_allowed_http_hosts": ["127.0.0.1"]}, + ) + unsafe_store = backend.open(spec) + try: + res = backend.dispatch(unsafe_store, MemoryOpResource(source=http_server)) + finally: + backend.close(unsafe_store) + assert isinstance(res, MemoryExecutionFailure) + assert res.kind == "unauthorized" diff --git a/tests/memory/persistence/test_persistence.py b/tests/memory/persistence/test_persistence.py index e1e6f41..74d5d93 100644 --- a/tests/memory/persistence/test_persistence.py +++ b/tests/memory/persistence/test_persistence.py @@ -13,6 +13,8 @@ from pathlib import Path from uuid import UUID, uuid4 +import pytest + from rath.memory.persistence import ( PersistentMemoryRegistry, local_memory_root, @@ -56,6 +58,15 @@ def test_local_store_dir_accepts_str(_isolate_openrath_home: Path) -> None: assert local_store_dir(str(sid)) == local_store_dir(sid) +@pytest.mark.parametrize("bad_id", ["../escape", "/tmp/escape", "not-a-uuid"]) +def test_local_store_dir_rejects_path_like_ids( + _isolate_openrath_home: Path, + bad_id: str, +) -> None: + with pytest.raises(ValueError, match="store_id must be a UUID"): + local_store_dir(bad_id) + + def test_ensure_local_memory_root_creates_and_is_idempotent( _isolate_openrath_home: Path, ) -> None: diff --git a/tests/session/persistence/test_loader_gc.py b/tests/session/persistence/test_loader_gc.py index d906381..baa55f8 100644 --- a/tests/session/persistence/test_loader_gc.py +++ b/tests/session/persistence/test_loader_gc.py @@ -33,6 +33,7 @@ load_session, prune_sessions, session_file, + session_partial_file, ) from rath.session.session import Session @@ -213,6 +214,17 @@ def test_session_file_path_under_resolved_dir(_isolate_openrath_home: Path) -> N assert str(expected).endswith(f"{sid}.jsonl") +@pytest.mark.parametrize("bad_id", ["../escape", "/tmp/escape", "not-a-uuid"]) +def test_session_paths_reject_path_like_ids( + _isolate_openrath_home: Path, + bad_id: str, +) -> None: + with pytest.raises(ValueError, match="session_id must be a UUID"): + session_file(bad_id) + with pytest.raises(ValueError, match="session_id must be a UUID"): + session_partial_file(bad_id) + + # --------------------------------------------------------------------------- # GC: delete + prune # ---------------------------------------------------------------------------