From 3a28411b3824dc786405653c056b639b0264d5a9 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sat, 8 Aug 2026 18:59:49 -0700 Subject: [PATCH] fix(core): validate program-cache constructor arguments Two argument checks in the program caches accept values that make the cache silently useless. max_size_bytes -------------- Both backends guard with a bare comparison: if max_size_bytes is not None and max_size_bytes <= 0: raise ValueError("max_size_bytes must be positive or None ...") bool is an int subclass, so `True` passes as a one-byte cap. Every write is then evicted immediately by the size enforcer: cache = InMemoryProgramCache(max_size_bytes=True) cache[b"k"] = b"ab" len(cache) -> 0 while its twin `False` is rejected by the same line. A float cap is accepted too, and a str cap raises `TypeError: '<=' not supported between instances of 'str' and 'int'` rather than the ValueError the constructor documents. Require a positive int, using the same `isinstance(x, bool) or not isinstance(x, int)` shape already used by `Host.__new__` and `checkpoint._check_pid`. The message keeps the word "positive" that the existing tests match on. path ---- `FileStreamProgramCache.__init__` does: self._root = Path(path) if path is not None else _default_cache_dir() `Path("")` is `Path(".")`, so an empty path roots the cache in the current working directory and `__init__` then creates `entries/` and `tmp/` there -- directories a later `clear()` will rmdir. `path=os.environ.get("VAR", "")` is how a caller lands on it. The asymmetry is in the same file: `_default_cache_dir()` deliberately treats an empty `XDG_CACHE_HOME` / `LOCALAPPDATA` as unset. Reject the empty string rather than defaulting silently, and point at `None` for the default. An explicit `"."` still works. --- .../core/utils/_program_cache/_file_stream.py | 16 +++++- .../core/utils/_program_cache/_in_memory.py | 10 +++- cuda_core/docs/source/release/1.2.0-notes.rst | 8 +++ cuda_core/tests/test_program_cache.py | 54 +++++++++++++++++++ 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/cuda_core/cuda/core/utils/_program_cache/_file_stream.py b/cuda_core/cuda/core/utils/_program_cache/_file_stream.py index eb71abf5446..bdac50a334c 100644 --- a/cuda_core/cuda/core/utils/_program_cache/_file_stream.py +++ b/cuda_core/cuda/core/utils/_program_cache/_file_stream.py @@ -391,8 +391,20 @@ def __init__( *, max_size_bytes: int | None = None, ) -> None: - if max_size_bytes is not None and max_size_bytes <= 0: - raise ValueError("max_size_bytes must be positive or None (0 would evict every write)") + if max_size_bytes is not None and ( + # bool is an int subclass, so True would sail through as a 1-byte + # cap that discards every write while its twin False is rejected. + isinstance(max_size_bytes, bool) or not isinstance(max_size_bytes, int) or max_size_bytes <= 0 + ): + raise ValueError( + f"max_size_bytes must be a positive int or None (0 would evict every write), got {max_size_bytes!r}" + ) + if path is not None and os.fspath(path) == "": + # Path("") is Path("."), so an empty string would quietly root the + # cache in the current working directory and create entries/ and + # tmp/ there. Callers reach this via os.environ.get("VAR", ""); say + # so rather than scribbling in whatever directory they ran from. + raise ValueError("path must be a non-empty directory, or None to use the default user cache directory") self._root = Path(path) if path is not None else _default_cache_dir() self._entries = self._root / _ENTRIES_SUBDIR self._tmp = self._root / _TMP_SUBDIR diff --git a/cuda_core/cuda/core/utils/_program_cache/_in_memory.py b/cuda_core/cuda/core/utils/_program_cache/_in_memory.py index 6a25e2afbc6..0cb9b9b3768 100644 --- a/cuda_core/cuda/core/utils/_program_cache/_in_memory.py +++ b/cuda_core/cuda/core/utils/_program_cache/_in_memory.py @@ -54,8 +54,14 @@ def __init__( *, max_size_bytes: int | None = None, ) -> None: - if max_size_bytes is not None and max_size_bytes <= 0: - raise ValueError("max_size_bytes must be positive or None (0 would evict every write)") + if max_size_bytes is not None and ( + # bool is an int subclass, so True would sail through as a 1-byte + # cap that discards every write while its twin False is rejected. + isinstance(max_size_bytes, bool) or not isinstance(max_size_bytes, int) or max_size_bytes <= 0 + ): + raise ValueError( + f"max_size_bytes must be a positive int or None (0 would evict every write), got {max_size_bytes!r}" + ) self._max_size_bytes = max_size_bytes # Key insertion order encodes LRU order: oldest first, newest last. # Each value is ``(payload_bytes, payload_size)``; caching the size diff --git a/cuda_core/docs/source/release/1.2.0-notes.rst b/cuda_core/docs/source/release/1.2.0-notes.rst index 120d2c2a253..6a4d97d9341 100644 --- a/cuda_core/docs/source/release/1.2.0-notes.rst +++ b/cuda_core/docs/source/release/1.2.0-notes.rst @@ -73,6 +73,14 @@ Fixes and enhancements Windows, both ``ctypes.CFUNCTYPE`` and ``ctypes.WINFUNCTYPE`` are accepted. (`#2439 `__) +- The program caches now reject a ``max_size_bytes`` that is not a positive + ``int``. ``max_size_bytes=True`` was accepted as a one-byte cap that silently + discarded every write, even though ``False`` was rejected, and a ``str`` cap + raised ``TypeError`` from the comparison rather than the documented + ``ValueError``. :class:`~cuda.core.utils.FileStreamProgramCache` also rejects + an empty ``path``: ``Path("")`` is ``Path(".")``, so it used to root the cache + in the current working directory. + Deprecation Notices ------------------- diff --git a/cuda_core/tests/test_program_cache.py b/cuda_core/tests/test_program_cache.py index a8d3fc85f7e..3c0af4712bb 100644 --- a/cuda_core/tests/test_program_cache.py +++ b/cuda_core/tests/test_program_cache.py @@ -1512,6 +1512,51 @@ def test_filestream_cache_rejects_non_positive_size_cap(tmp_path, bad): FileStreamProgramCache(tmp_path / "fc", max_size_bytes=bad) +# Values a bare `<= 0` test lets through. `True` is the sharp one: bool is an +# int subclass, so it becomes a 1-byte cap that silently discards every write, +# while its twin `False` is rejected. +NON_INT_SIZE_CAPS = [ + pytest.param(True, id="bool-true"), + pytest.param(1.5, id="float"), + pytest.param("100", id="str"), +] + + +@pytest.mark.agent_authored(model="claude-opus-5") +@pytest.mark.parametrize("bad", NON_INT_SIZE_CAPS) +def test_filestream_cache_rejects_non_int_size_cap(tmp_path, bad): + from cuda.core.utils import FileStreamProgramCache + + with pytest.raises(ValueError, match="positive"): + FileStreamProgramCache(tmp_path / "fc", max_size_bytes=bad) + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_filestream_cache_rejects_an_empty_path(tmp_path, monkeypatch): + """`Path("")` is `Path(".")`, so an empty path would root the cache in the + current working directory and create entries/ and tmp/ there. Callers reach + this through `os.environ.get("VAR", "")`.""" + from cuda.core.utils import FileStreamProgramCache + + monkeypatch.chdir(tmp_path) + with pytest.raises(ValueError, match="non-empty directory"): + FileStreamProgramCache("") + + assert sorted(p.name for p in tmp_path.iterdir()) == [] + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_filestream_cache_still_accepts_an_explicit_dot(tmp_path, monkeypatch): + """Only the empty string is rejected; an explicit relative path is fine.""" + from cuda.core.utils import FileStreamProgramCache + + monkeypatch.chdir(tmp_path) + with FileStreamProgramCache(".") as cache: + cache[b"k"] = b"hello" + assert cache[b"k"] == b"hello" + assert (tmp_path / "entries").is_dir() + + def test_default_cache_dir_lives_under_user_cache_root(monkeypatch, tmp_path): """The cache root is platform-specific: @@ -2442,6 +2487,15 @@ def test_inmemory_cache_rejects_non_positive_size_cap(bad): InMemoryProgramCache(max_size_bytes=bad) +@pytest.mark.agent_authored(model="claude-opus-5") +@pytest.mark.parametrize("bad", NON_INT_SIZE_CAPS) +def test_inmemory_cache_rejects_non_int_size_cap(bad): + from cuda.core.utils import InMemoryProgramCache + + with pytest.raises(ValueError, match="positive"): + InMemoryProgramCache(max_size_bytes=bad) + + def test_inmemory_cache_size_cap_evicts_oldest(): from cuda.core.utils import InMemoryProgramCache