diff --git a/.pylintrc b/.pylintrc index 4e7b350..8b58011 100644 --- a/.pylintrc +++ b/.pylintrc @@ -341,7 +341,7 @@ indent-string=' ' max-line-length=100 # Maximum number of lines in a module. -max-module-lines=4200 +max-module-lines=4300 # Allow the body of a class to be on the same line as the declaration if body # contains single statement. diff --git a/CHANGELOG.md b/CHANGELOG.md index f5dcaf2..d0bb44a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,30 @@ diskcache-community forked from DiskCache at upstream commit ebfa37c (3 March 20 ## Unreleased +### Security + +- `SafeDisk.with_types` refuses the classes of scipy, torch, bitstruct and pyahocorasick, by class, by module name and through `DISKCACHE_SAFE_MODULES`, as it refuses numpy: a crafted scipy sparse matrix, pyahocorasick automaton or bitstruct compiled format crashes the reader, and torch builds some objects by compiling code. Code that allowed them gets `ValueError` from `with_types` (PR #42). + ### Fixes +- `Cache.get` works again in subclasses that override `_con` to keep their rows in another database file, as subclasses written for 5.6.3 do. Since 5.8.2 its fast path took its cursor from a connection to `cache.db`, so every read raised `OperationalError: no such table: Cache` (PR #42). +- `SafeDisk` stores the classes in its allowlist, such as `int` and `str`, so `memoize(typed=True)` works with it: those keys hold the classes of the arguments, which every `SafeDisk` reads but plain `SafeDisk` and `with_types()` with no arguments refused to write. `NoneType` and `bool` stay refused, because the reader refuses them, and a `defaultdict(bool)`, which was written but could not be read back, is now refused too (PR #42). +- `SafeDisk.with_numpy()` stores `datetime64` and `timedelta64` arrays, which raised `ValueError` from `pickle.PickleBuffer`. Their scalars still raise `TypeError` (PR #42). +- Every `SafeDisk` checks a key or value outside the standard-library types against the names its pickle looks up, as the disks from `with_types` already did. A plain `SafeDisk` therefore stores a dict subclass that pickles as a `dict`, such as DRF's `ReturnDict`, and reads back a `dict`, and an object that cannot be pickled raises `TypeError` on every `SafeDisk` (PR #42). +- Refusal messages say what to do: a refused function no longer points to `with_types()`, which allows only classes; numpy objects point to `with_numpy()`; an array of objects mentions pandas DataFrames and what to store instead; and a class from a package `with_types` refuses says so (PR #42). - `incr` and `decr` on `Cache` and `FanoutCache` type-check with `default=None`, which makes a missing key raise `KeyError`: the stubs typed `default` as `int`, so type checkers rejected the call. `tox -e mypy` now also checks `tests/typecheck_stubs.py`, a file of calls that must type-check against the stubs (PR #41). +- The `modules` argument of `SafeDisk.with_types` type-checks with a single string or any iterable of strings, as the code accepts (PR #42). + +### Docs + +- The CVE page and the migration guide say up front that `JSONDisk` does not protect reads, because it reads each row as its `mode` column says, and point to `SafeJSONDisk`. The migration guide explains that Dash's `DiskcacheManager` takes only an upstream cache and keeps its signing secret there, so its folder should be its own (PR #42). +- The tutorial no longer says that `memoize(typed=True)` needs the default `Disk` (PR #42). + +### Known issues + +- A cache whose directory or `cache.db` the process cannot write opens read-only, as since 5.7.0, where 5.6.3 failed to open it. A cache owned by another account is then read, and a library's fallback for an unwritable cache is skipped. A later release will warn, refuse or offer an option that requires a writable cache. +- The write check of `SafeDisk` scans the pickle in Python, which takes about ten times as long as pickling a very large value. Reads are not affected. +- Objects that no allowlist can hold, such as those whose pickle calls a function, need the default `Disk`. Signed entries, planned for 5.9, are meant to cover them. ## 5.8.2 (2026-11-02) diff --git a/SECURITY.md b/SECURITY.md index aa292dc..bdfa195 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -20,7 +20,7 @@ Where this fork stands: - 5.8.0 is imported as `diskcache_community`, next to the upstream `diskcache` package. `SafeDisk.with_types` allows your own classes and the classes defined in named modules while everything else stays refused; numpy classes cannot be allowed, because numpy unpickling is not memory-safe. `configure()` and `DISKCACHE_DISK=safe` select `SafeDisk` for caches created without a disk, including those that libraries create through `diskcache_community`. `Disk.with_options(on_error='miss')` deletes entries that cannot be read and treats them as missing. The steps are in [Fix CVE-2025-69872 in diskcache](https://diskcache-community.readthedocs.io/en/latest/fix-cve-2025-69872.html). - `JSONDisk` writes no pickles but still reads one from a crafted row, as it does in 5.6.3. `SafeJSONDisk` reads and writes the same files and never reads a pickle; `SafeDisk` does not decode what `JSONDisk` wrote. - `SafeDisk.with_numpy()` returns a `SafeDisk` that also stores numpy arrays, in the `.npy` format rather than as pickles. numpy pickles stay refused, because unpickling an array is not memory-safe. -- The default stays as it is. Existing caches hold objects outside any allowlist, such as instances of your own classes, Django responses and the keys of `memoize(typed=True)`, so `SafeDisk` remains an explicit choice. +- The default stays as it is. Existing caches hold objects outside any allowlist, such as instances of your own classes and Django responses, so `SafeDisk` remains an explicit choice. ## Dependencies diff --git a/diskcache_community/arrays.py b/diskcache_community/arrays.py index 8cf13ed..a1348de 100644 --- a/diskcache_community/arrays.py +++ b/diskcache_community/arrays.py @@ -93,7 +93,9 @@ def _pack_array(numpy, array): The buffer is the array's own memory, so pickle copies it once, into the row, as a bytearray. An array that is not contiguous or not writable is copied first, since pickle stores a read-only buffer as - bytes, which the reader refuses. + bytes, which the reader refuses. The buffer is a flat view of bytes, + since numpy exports no buffer for ``datetime64`` and ``timedelta64`` + data; the dtype is in the header. """ # pylint: disable=no-member @@ -114,11 +116,16 @@ def _pack_array(numpy, array): header = buffer.getvalue() _read_header(numpy, header, array.nbytes) except (ValueError, pickle.UnpicklingError) as error: - raise TypeError( - 'SafeDisk cannot store an array of dtype %s' % (array.dtype,) - ) from error - - return header, pickle.PickleBuffer(array) + message = 'SafeDisk cannot store an array of dtype %s' % (array.dtype,) + if array.dtype.hasobject: + message += ( + '; a pandas DataFrame holds its labels and text columns in' + ' such arrays, so store its records or columns instead' + ) + raise TypeError(message) from error + + data = array.reshape(-1, order='A').view(numpy.uint8) + return header, pickle.PickleBuffer(data) def _unpack_array(header, data): diff --git a/diskcache_community/core.py b/diskcache_community/core.py index 8647070..cdda6fd 100644 --- a/diskcache_community/core.py +++ b/diskcache_community/core.py @@ -557,9 +557,11 @@ def fetch(self, mode, filename, value, read): 'uuid': frozenset({'UUID'}), } -# Python types that SafeDisk accepts for writing: exactly the types that the -# names above read back. Subclasses are rejected because pickle records the -# subclass, which the reader would refuse. +# Python types that SafeDisk accepts for writing without pickling them first: +# exactly the types that the names above read back. Anything else is pickled +# and accepted only if the reader resolves every name the pickle looks up, so +# a subclass is refused, because pickle records the subclass, unless it +# pickles as one of these types. SAFE_TYPES = frozenset( { @@ -593,15 +595,35 @@ def fetch(self, mode, filename, value, read): } ) +# Classes that SafeDisk accepts as objects, such as the classes that +# memoize(typed=True) puts into keys and the factory of a defaultdict: the +# classes in SAFE_TYPES that pickle as a name the reader resolves. NoneType +# pickles as a call to builtins.type, and bool as builtins.bool, which the +# reader refuses. + +_SAFE_CLASSES = SAFE_TYPES - {type(None), bool} + _SAFE_MAPPINGS = (dict, co.Counter, co.OrderedDict, co.defaultdict) _SAFE_SEQUENCES = (list, tuple, set, frozenset, co.deque) _SAFE_TZ_TYPES = (type(None), dt.timezone) -# Packages whose classes SafeDisk.with_types refuses to allow. numpy restores -# the internal flags of a dtype from its pickle without checking them, so a -# crafted pickle can make an array read raw bytes as object pointers. +# Packages whose classes SafeDisk.with_types refuses to allow, because a +# crafted pickle of one can corrupt memory or run code. numpy restores the +# internal flags of a dtype from its pickle without checking them, so a +# crafted pickle can make an array read raw bytes as object pointers. scipy's +# sparse matrices and pyahocorasick's automatons keep index data that their +# compiled code trusts, bitstruct's compiled formats crash on state they did +# not build, and torch builds some objects by compiling or running code. + +_UNSAFE_MODULES = ('numpy', 'ahocorasick', 'bitstruct', 'scipy', 'torch') + + +def _unsafe_reason(package): + """Return why with_types refuses classes of the unsafe `package`.""" + if package == 'numpy': + return '; use with_numpy() to store arrays' + return ': a crafted pickle of a %s object is not safe to read' % package -_UNSAFE_MODULES = ('numpy',) _PICKLE_STRING_OPS = frozenset( {'BINUNICODE', 'BINUNICODE8', 'SHORT_BINUNICODE', 'UNICODE'} @@ -838,10 +860,12 @@ class SafeDisk(Disk): ``FanoutCache(directory, disk=SafeDisk)``, ``Deque(disk=SafeDisk)``, ``Index(disk=SafeDisk)`` or the ``DISK`` option of the Django backend. - Objects outside the allowlist, such as instances of your own classes, - Django model instances and responses, and the ``type`` objects that - ``memoize(typed=True)`` puts into keys, need either the default - :class:`Disk` or a disk from :meth:`with_types` that allows them. + Objects outside the allowlist, such as instances of your own classes and + Django model instances and responses, need either the default + :class:`Disk` or a disk from :meth:`with_types` that allows them. The + allowed classes themselves can be stored too, except ``NoneType`` and + ``bool``, so ``memoize(typed=True)`` works unless an argument is + ``None``, ``True`` or ``False``. """ @@ -868,7 +892,9 @@ def with_types(cls, *types, modules=()): do nothing beyond storing their fields, such as dataclasses, named tuples, enums and pydantic models. Reading looks classes up only in modules that are already imported, so import them before reading. - numpy classes are refused; use :meth:`with_numpy` to store arrays. + Classes of numpy, scipy, torch, bitstruct and pyahocorasick are + refused, because a crafted pickle of one can corrupt memory or run + code; use :meth:`with_numpy` to store arrays. >>> import collections >>> Point = collections.namedtuple('Point', 'x y') @@ -882,17 +908,18 @@ def with_types(cls, *types, modules=()): :return: SafeDisk subclass to pass as ``disk=`` :raises TypeError: if an entry of `types` is not a class :raises ValueError: if an entry of `modules` is not a module name, or - a type or module is numpy (arrays belong to :meth:`with_numpy`) + a type or module belongs to one of those packages """ for entry in types: if not isinstance(entry, type): raise TypeError('with_types takes classes, not %r' % (entry,)) - if _in_modules(entry.__module__, _UNSAFE_MODULES): - raise ValueError( - 'with_types refuses the class %r; use with_numpy() to' - ' store arrays' % (entry,) - ) + for unsafe in _UNSAFE_MODULES: + if _in_modules(entry.__module__, (unsafe,)): + raise ValueError( + 'with_types refuses the class %r%s' + % (entry, _unsafe_reason(unsafe)) + ) if isinstance(modules, str): modules = (modules,) @@ -903,15 +930,14 @@ def with_types(cls, *types, modules=()): part.isidentifier() for part in prefix.split('.') ): raise ValueError('%r is not a module name' % (entry,)) - if any( - _in_modules(prefix, (unsafe,)) - or _in_modules(unsafe, (prefix,)) - for unsafe in _UNSAFE_MODULES - ): - raise ValueError( - 'with_types refuses the module %r; use with_numpy() to' - ' store arrays' % (entry,) - ) + for unsafe in _UNSAFE_MODULES: + if _in_modules(prefix, (unsafe,)) or _in_modules( + unsafe, (prefix,) + ): + raise ValueError( + 'with_types refuses the module %r%s' + % (entry, _unsafe_reason(unsafe)) + ) prefixes.append(prefix) return _derive_disk( @@ -933,7 +959,7 @@ def with_numpy(cls): value except in keys, and checks each header before it reads an array back. (Plain :class:`SafeDisk` refuses numpy pickles, because unpickling an array is not memory-safe.) numpy scalars keep their - type. + type, and arrays of ``datetime64`` and ``timedelta64`` are stored too. Object arrays, ``ndarray`` subclasses and other numpy scalars raise :exc:`TypeError` when written. It combines with :meth:`with_types` and :meth:`with_options` in any order. @@ -966,22 +992,29 @@ def _unpickler(self, file): return arrays.unpickler(self, file) return self._plain_unpickler(file) - def _extended(self): - return bool(self.safe_types or self.safe_modules) - def _check_object(self, obj, what): - """Raise TypeError unless the reader would accept `obj` back.""" + """Raise TypeError unless the reader would accept `obj` back. + + Outside SAFE_TYPES, an object is accepted when its pickle looks up + only names the reader resolves, such as a dict subclass that pickles + as a plain dict, which is then what the reader builds. + + """ try: self._check(obj, what) except TypeError: - if not self._extended(): - raise + pass else: return - self._check_pickle( - pickle.dumps(obj, protocol=self.pickle_protocol), obj, what - ) + try: + data = pickle.dumps(obj, protocol=self.pickle_protocol) + except (pickle.PicklingError, AttributeError) as error: + raise TypeError( + 'SafeDisk cannot store %s: %s' % (what, error) + ) from error + + self._check_pickle(data, obj, what) def _check_pickle(self, data, obj, what): """Raise TypeError unless the reader would accept the pickle `data`.""" @@ -1002,17 +1035,38 @@ def _check_pickle(self, data, obj, what): except pickle.UnpicklingError: raise TypeError( 'SafeDisk cannot store %s of type %s.%s: it pickles' - ' %s.%s, which the allowlist refuses; allow it with' - ' SafeDisk.with_types()' + ' %s.%s, which the allowlist refuses%s' % ( what, type(obj).__module__, type(obj).__qualname__, module, name, + self._refusal_hint(module, name), ) ) from None + def _refusal_hint(self, module, name): + """Return how to store what pickles the refused `module`.`name`.""" + if _in_modules(module, ('numpy',)): + if self.safe_numpy: + return '; with_numpy() stores only arrays and numpy scalars' + return '; store arrays with SafeDisk.with_numpy()' + + for unsafe in _UNSAFE_MODULES: + if _in_modules(module, (unsafe,)): + return '; with_types() refuses %s classes' % unsafe + + obj = sys.modules.get(module) + for part in name.split('.'): + obj = getattr(obj, part, None) + + if not isinstance(obj, type): + return '; it is not a class, and with_types() allows only classes' + if module in ('builtins', '__builtin__'): + return '' + return '; allow it with SafeDisk.with_types()' + def _check(self, obj, what): """Raise TypeError unless every object in `obj` is in SAFE_TYPES.""" seen = set() @@ -1023,6 +1077,8 @@ def _check(self, obj, what): type_item = type(item) if type_item not in SAFE_TYPES: + if isinstance(item, type) and item in _SAFE_CLASSES: + continue raise TypeError( 'SafeDisk cannot store %s of type %s.%s' % (what, type_item.__module__, type_item.__qualname__) @@ -1034,7 +1090,7 @@ def _check(self, obj, what): seen.add(id(item)) if type_item is co.defaultdict: factory = item.default_factory - if factory is not None and factory not in SAFE_TYPES: + if factory is not None and factory not in _SAFE_CLASSES: raise TypeError( 'SafeDisk cannot store %s with defaultdict' ' factory %r' % (what, factory) @@ -1996,8 +2052,12 @@ def _cur(self): cur = getattr(self._local, 'cur', None) if cur is None: - self._connect() - cur = self._local.cur + # Through _con, so a subclass that overrides it, to open another + # database file say, gets a cursor on that connection. + con = self._con + cur = getattr(self._local, 'cur', None) + if cur is None or cur.connection is not con: + cur = self._local.cur = con.cursor() return cur diff --git a/diskcache_community/core.pyi b/diskcache_community/core.pyi index 9fa0a99..a607ba3 100644 --- a/diskcache_community/core.pyi +++ b/diskcache_community/core.pyi @@ -8,6 +8,7 @@ from typing import ( Final, Generator, Generic, + Iterable, Literal, NoReturn, overload, @@ -192,7 +193,7 @@ class SafeDisk(Disk): safe_numpy: bool @classmethod def with_types( - cls, *types: type, modules: tuple[str, ...] = ... + cls, *types: type, modules: str | Iterable[str] = ... ) -> type[Self]: ... @classmethod def with_numpy(cls) -> type[Self]: ... diff --git a/docs/api.rst b/docs/api.rst index 7ca394a..5fe4afc 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -154,8 +154,9 @@ Read the :ref:`SafeDisk tutorial ` for details. .. data:: diskcache_community.core._UNSAFE_MODULES Packages whose classes :meth:`SafeDisk.with_types - ` refuses to allow, because - unpickling them is not memory-safe. + ` refuses to allow, because a + crafted pickle of one can corrupt memory or run code: numpy, scipy, torch, + bitstruct and pyahocorasick. SafeJSONDisk ------------ diff --git a/docs/fix-cve-2025-69872.rst b/docs/fix-cve-2025-69872.rst index 6d1cfce..749f150 100644 --- a/docs/fix-cve-2025-69872.rst +++ b/docs/fix-cve-2025-69872.rst @@ -14,6 +14,15 @@ Install diskcache-community and read your caches with :class:`SafeDisk cache = Cache('/var/tmp/mycache', disk=SafeDisk) +.. important:: + + :class:`JSONDisk ` does not protect reads. It + writes JSON, but it reads each row as the row's ``mode`` column says, so a + crafted row holding a pickle is unpickled as with the default :class:`Disk + `. Use :class:`SafeJSONDisk + `, which reads the same files and never + reads a pickle. + The rest of this page explains what the vulnerability is, who is exposed, the steps for caches you create and for caches libraries create for you, and what a restricted disk has to cover if you write your own. @@ -93,7 +102,8 @@ Steps that imports the upstream ``diskcache`` package keeps reading pickles with :func:`pickle.load` until it moves to the fork. A library that passes its own disk keeps it. :ref:`tutorial-default-disk` in the tutorial has the - details. + details. Dash's ``DiskcacheManager`` takes only an upstream cache; see + :ref:`the note on Dash `. 4. **Allow your own classes.** If the cache holds dataclasses, named tuples, enums or pydantic models, allow them with :meth:`SafeDisk.with_types @@ -180,9 +190,10 @@ covers all of the following, and :class:`SafeDisk step is a class defined in a trusted module, and it refuses dunder attributes. -* **No numpy.** Array pickles can corrupt memory. +* **No numpy, scipy, torch, bitstruct or pyahocorasick.** Crafted pickles of + their objects can corrupt memory or run code. :meth:`SafeDisk.with_types ` - refuses numpy classes even when asked for them; use :meth:`with_numpy + refuses their classes even when asked for them; use :meth:`with_numpy ` to store arrays instead. * **No imports while reading.** :class:`SafeDisk diff --git a/docs/migrating.rst b/docs/migrating.rst index 81e0e2f..05e12f8 100644 --- a/docs/migrating.rst +++ b/docs/migrating.rst @@ -18,6 +18,15 @@ the memoize key format. A directory written by 5.6.3 opens with the fork, and a directory written by the fork opens with 5.6.3. The fork needs Python 3.8 or later and has no runtime dependencies. +.. important:: + + :class:`JSONDisk ` does not protect reads. It + writes JSON, but it reads each row as the row's ``mode`` column says, so a + crafted row holding a pickle is unpickled as with the default :class:`Disk + `. Use :class:`SafeJSONDisk + `, which reads the same files and never + reads a pickle. + Steps ----- @@ -85,6 +94,16 @@ installed: except ImportError: import diskcache +.. _migrating-dash: + +Dash is one such dependency: its ``DiskcacheManager`` checks that it is given +an upstream ``diskcache.Cache``, so a diskcache-community cache cannot be +handed to it. Dash keeps the secret that signs its background callbacks in +that cache under a fixed key, and the cache holds pickles, so do not share the +folder with anything else. ``DiskcacheManager()`` without a cache creates a +private folder of its own, and setting ``DASH_SECRET_KEY`` keeps the secret +out of the cache. + Scanners keep reporting CVE-2025-69872 while the upstream package is installed. Run ``pip show diskcache`` to see whether it is, and, with pipdeptree installed, ``pipdeptree -r -p diskcache`` to see which dependency diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 6d8bfc6..ea7b698 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -974,14 +974,19 @@ with the default :class:`Disk ` and the reverse, as long as the values are within the allowlist. Some uses need the default :class:`Disk ` because their objects are outside it: instances of your own classes, Django model instances and ``HttpResponse`` -objects, datetimes whose ``tzinfo`` is a ``zoneinfo.ZoneInfo`` rather than a -``datetime.timezone``, and :meth:`memoize ` -with ``typed=True``, which puts ``type`` objects into the cache key. Every -process that writes to a shared directory with the default :class:`Disk -` must be trusted, and a reader using -:class:`SafeDisk ` raises +objects, and datetimes whose ``tzinfo`` is a ``zoneinfo.ZoneInfo`` rather than +a ``datetime.timezone``. Every process that writes to a shared directory with +the default :class:`Disk ` must be trusted, and a +reader using :class:`SafeDisk ` raises :exc:`pickle.UnpicklingError` on those rows. +:meth:`memoize ` with ``typed=True`` puts +the class of each argument into the cache key. :class:`SafeDisk +` stores and reads the classes in its allowlist, +such as ``int`` and ``str``, except ``NoneType`` and ``bool``: a call with +``None``, ``True`` or ``False`` as an argument raises :exc:`TypeError` before +the function runs. + Instead of falling back to the default :class:`Disk `, :meth:`SafeDisk.with_types ` returns a :class:`SafeDisk @@ -1014,7 +1019,10 @@ pydantic models, because allowing a class lets a crafted pickle create instances of it with any field values. numpy classes are refused: numpy restores a dtype's internal flags from the pickle without checking them, so a crafted array pickle can corrupt memory. Use :meth:`SafeDisk.with_numpy -` to store arrays instead. A write +` to store arrays instead. Classes of +scipy, torch, bitstruct and pyahocorasick are refused too: crafted sparse +matrices, automatons and compiled formats crash the reader, and torch builds +some objects by compiling code. A write whose value pickles a class outside the allowlist raises :exc:`TypeError` that names the class. Calling ``with_types`` on the returned class adds to its allowlist, and a cache that uses it can be pickled as usual. diff --git a/tests/test_core.py b/tests/test_core.py index 77b9b70..1b42e7d 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -2319,6 +2319,50 @@ def test_get_after_close_uses_new_cursor(cache): assert cache._local.cur is not cur +class OtherDatabaseCache(dc.Cache): + """Cache written against 5.6.3 that keeps its rows in other.db.""" + + @property + def _con(self): + con = getattr(self._local, 'con', None) + if con is None: + con = self._local.con = sqlite3.connect( + op.join(self._directory, 'other.db'), + timeout=self._timeout, + isolation_level=None, + ) + return con + + +def test_get_uses_cursor_of_overridden_con(): + directory = tempfile.mkdtemp() + try: + with OtherDatabaseCache(directory) as cache: + cache.set('a', 1) + assert cache.get('a') == 1 + assert cache._local.cur.connection is cache._con + + other = [] + + def get_in_thread(): + other.append(cache.get('a')) + cache.close() + + thread = threading.Thread(target=get_in_thread) + thread.start() + thread.join() + assert other == [1] + + with OtherDatabaseCache(directory) as cache: + assert cache.get('a') == 1 + assert cache.get('b') is None + + assert op.exists(op.join(directory, 'other.db')) + assert not op.exists(op.join(directory, 'cache.db')) + finally: + shutil.rmtree(directory, ignore_errors=True) + + def test_get_reads_only_value_columns_without_metadata(cache): cache.set('a', 1, tag='t') statements = [] diff --git a/tests/test_safedisk.py b/tests/test_safedisk.py index ac12df1..88d011d 100644 --- a/tests/test_safedisk.py +++ b/tests/test_safedisk.py @@ -18,7 +18,12 @@ import pytest import diskcache_community as dc -from diskcache_community.core import MODE_BINARY, MODE_PICKLE, SafeUnpickler +from diskcache_community.core import ( + MODE_BINARY, + MODE_PICKLE, + SAFE_TYPES, + SafeUnpickler, +) pytestmark = pytest.mark.filterwarnings('ignore', category=dc.EmptyDirWarning) @@ -104,6 +109,10 @@ def utcoffset(self, _): decimal.Decimal('-1e100'), fractions.Fraction(1, 3), uuid.UUID('12345678-1234-5678-1234-567812345678'), + co.defaultdict(int, {'a': 1}), + # The classes themselves, as memoize(typed=True) puts them into keys. + *sorted(SAFE_TYPES - {type(None), bool}, key=repr), + ('f', 1, 'a', int, str), ] HASHABLE_VALUES = [ @@ -121,14 +130,18 @@ def utcoffset(self, _): (1, (2, (Custom(),))), co.deque([Custom()]), co.defaultdict(Custom), + co.defaultdict(bool), dt.datetime(2020, 1, 2, tzinfo=CustomTz()), dt.time(1, tzinfo=CustomTz()), dt.datetime(2020, 1, 2, tzinfo=zoneinfo.ZoneInfo('UTC')), type, - int, + type(None), + bool, + (1, type(None)), + Custom, len, Poison, - object(), + lambda: None, Exception('boom'), ] @@ -330,21 +343,84 @@ def test_key_rejected(cache): assert len(cache) == 0 -def test_typed_memoize_rejected(cache): - @cache.memoize(typed=True) - def identity(value): - return value +@pytest.mark.parametrize('disk', [dc.SafeDisk, dc.SafeDisk.with_types()]) +def test_typed_memoize(directory, disk): + with dc.Cache(directory, disk=disk) as cache: - with pytest.raises(TypeError, match='SafeDisk cannot store key'): - identity(1) + @cache.memoize(typed=True) + def identity(value): + return value + + assert identity(1) == 1 + assert identity(1.0) == 1.0 + assert identity('a') == 'a' + assert identity(1) == 1 + assert len(cache) == 3 + + with pytest.raises(TypeError, match='SafeDisk cannot store key'): + identity(None) + + with pytest.raises(TypeError, match='SafeDisk cannot store key'): + identity(True) + + assert len(cache) == 3 + + with dc.Cache(directory, disk=dc.SafeDisk) as cache: + assert sorted(key[-1].__name__ for key in cache) == [ + 'float', + 'int', + 'str', + ] - @cache.memoize() - def plain(value): - return value - assert plain(1) == 1 - assert plain(1) == 1 - assert len(cache) == 1 +class PlainDict(dict): + """A dict subclass that pickles as a dict, as DRF's ReturnDict does.""" + + def __reduce__(self): + return dict, (dict(self),) + + +@pytest.mark.parametrize( + 'disk', + [dc.SafeDisk, dc.SafeDisk.with_types(), dc.SafeDisk.with_types(Custom)], +) +def test_objects_that_pickle_as_allowed_names_are_stored(directory, disk): + with dc.Cache(directory, disk=disk) as cache: + cache['dict'] = PlainDict(a=1) + cache[PlainDict(b=2).get('b')] = [PlainDict(c=3)] + cache['object'] = object() + + assert cache['dict'] == {'a': 1} + assert type(cache['dict']) is dict + assert type(cache[2][0]) is dict + assert type(cache['object']) is object + + +def test_refusal_messages(cache): + def refusal(value): + with pytest.raises(TypeError, match='SafeDisk cannot store') as info: + cache['key'] = value + return str(info.value) + + assert refusal(Custom()).endswith( + 'it pickles %s.Custom, which the allowlist refuses; allow it with' + ' SafeDisk.with_types()' % __name__ + ) + assert refusal(_execute).endswith( + 'it pickles %s._execute, which the allowlist refuses; it is not a' + ' class, and with_types() allows only classes' % __name__ + ) + assert refusal((1, type(None))).endswith( + 'it pickles builtins.type, which the allowlist refuses' + ) + assert refusal([bool]).endswith( + 'it pickles builtins.bool, which the allowlist refuses' + ) + assert refusal(co.defaultdict(bool)).endswith( + 'it pickles builtins.bool, which the allowlist refuses' + ) + assert refusal(lambda: None).startswith('SafeDisk cannot store value: ') + assert len(cache) == 0 def test_read_values_untouched(cache): diff --git a/tests/test_safedisk_numpy.py b/tests/test_safedisk_numpy.py index 5084d4e..e4ba060 100644 --- a/tests/test_safedisk_numpy.py +++ b/tests/test_safedisk_numpy.py @@ -187,6 +187,13 @@ def raw(header, body=b''): np.zeros((0, 3)), np.arange(4.0).reshape(2, 2).T, np.arange(3, dtype='>i4'), + # numpy exports no buffer for these, so the data goes as bytes. + np.array(['2026-01-01T00:00', 'NaT'], dtype='datetime64[ns]'), + np.arange(6).astype('datetime64[D]').reshape(2, 3).T, + np.array(np.datetime64('2026-01-01')), + np.arange(3).astype('>m8[s]'), + np.array([(1, 2)], dtype=[('a', 'i4'), ('t', 'M8[us]')]), + np.zeros(0, dtype='m8[ns]'), ], ids=lambda array: '%s-%s' % (array.dtype, array.shape), ) @@ -910,6 +917,34 @@ def test_safedisk_still_refuses_numpy(directory): assert len(cache) == 0 +@pytest.mark.parametrize( + 'disk', [dc.SafeDisk, dc.SafeDisk.with_types(co.UserDict)] +) +def test_numpy_refusals_point_to_with_numpy(directory, disk): + with dc.Cache(directory, disk=disk) as cache: + for value in (np.arange(2.0), np.dtype('f8'), [np.float64(1.5)]): + with pytest.raises(TypeError) as info: + cache['key'] = value + assert str(info.value).endswith( + 'store arrays with SafeDisk.with_numpy()' + ) + + +def test_with_numpy_refusal_of_other_numpy_objects(directory): + with dc.Cache(directory, disk=ArrayDisk) as cache: + with pytest.raises(TypeError) as info: + cache['key'] = {'dtype': np.dtype('f8')} + assert str(info.value).endswith( + 'with_numpy() stores only arrays and numpy scalars' + ) + + +def test_object_array_refusal_mentions_dataframes(directory): + with dc.Cache(directory, disk=ArrayDisk) as cache: + with pytest.raises(TypeError, match='DataFrame .* records'): + cache['key'] = np.array(['a', None], dtype=object) + + def test_safejsondisk_refuses_with_numpy(): with pytest.raises(TypeError, match='SafeJSONDisk'): dc.SafeJSONDisk.with_numpy() diff --git a/tests/test_safedisk_types.py b/tests/test_safedisk_types.py index c65248a..bab62e5 100644 --- a/tests/test_safedisk_types.py +++ b/tests/test_safedisk_types.py @@ -11,6 +11,7 @@ import sys import tempfile import tracemalloc +import types import pytest @@ -443,6 +444,44 @@ def test_numpy_is_refused(directory): cache['value'] = value +@pytest.mark.parametrize( + 'package, module', + [ + ('ahocorasick', 'ahocorasick'), + ('bitstruct', 'bitstruct.c'), + ('scipy', 'scipy.sparse._csr'), + ('torch', 'torch.fx.graph_module'), + ], +) +def test_unsafe_packages_are_refused(package, module): + entry = type('Thing', (), {'__module__': module}) + + with pytest.raises(ValueError, match='refuses the class .*not safe'): + dc.SafeDisk.with_types(entry) + + for prefix in (package, module, package + '.'): + with pytest.raises(ValueError, match='refuses the module .*not safe'): + dc.SafeDisk.with_types(modules=prefix) + + # A package whose name only starts with the same letters is allowed. + dc.SafeDisk.with_types(modules=package + 'x') + + +def test_refusal_names_an_unsafe_package(directory, monkeypatch): + module = types.ModuleType('bitstruct') + module.Format = type('Format', (), {'__module__': 'bitstruct'}) + monkeypatch.setitem(sys.modules, 'bitstruct', module) + + with dc.Cache(directory, disk=dc.SafeDisk) as cache: + with pytest.raises(TypeError) as info: + cache['value'] = [module.Format()] + + assert str(info.value).endswith( + 'it pickles bitstruct.Format, which the allowlist refuses;' + ' with_types() refuses bitstruct classes' + ) + + def test_modules_are_never_imported_by_a_read(): name = 'tests.safedisk_models.side_effect' data = stack_global(name, 'Thing') diff --git a/tests/typecheck_stubs.py b/tests/typecheck_stubs.py index 106b8c4..79d2204 100644 --- a/tests/typecheck_stubs.py +++ b/tests/typecheck_stubs.py @@ -20,3 +20,11 @@ def fanout_incr_without_default(cache: dc.FanoutCache, key: str) -> int: def fanout_decr_without_default(cache: dc.FanoutCache, key: str) -> int: return cache.decr(key, 1, default=None) + + +def with_types_one_module() -> type[dc.SafeDisk]: + return dc.SafeDisk.with_types(modules='myapp.models') + + +def with_types_module_list() -> type[dc.SafeDisk]: + return dc.SafeDisk.with_types(int, modules=['myapp.models', 'myapp.api'])