fix: serialize cache values as JSON instead of pickle

[scttfrdmn]

Problem

diskcache pickles any value that is not a str, int, float, or bytes, and unpickles it on read. That makes the cache directory a code-execution surface: anything able to write into it can hand a poisoned payload to the next reader — CVE-2025-69872 / GHSA-w8v5-vhqr-4h9v.

The CVE is unfixed upstream: 5.6.3 is the newest release and both proposed fixes (grantjenks/python-diskcache#358, #364) were declined. So a version floor is not available to us, and dvc-data is what puts three of its four cache call sites in the pickling path.

Which sites pickle is not obvious. Disk.store dispatches on exact types (type(value) is int), so bool is not covered by the int case:

Call site	Value	Stored as
hashfile/state.py links	(inode, mtime) tuple	pickle
hashfile/db/index.py	True / False	pickle (bool is not int)
index/serialize.py	entry dict	pickle
HashesCache	JSON string	raw

Keys are already pickle-free — dvc-data's keys are strings, which the base class stores raw.

Approach

dvc-data does not need pickle's expressiveness. Every value it caches is a dict, bool, tuple of numbers, or an already-JSON-encoded string, so JSON covers the whole domain with no code-execution primitive on read.

JSONDisk re-encodes only the values the base class would have pickled. str/int/float/bytes keep their raw storage, which is what leaves HashesCache untouched — it writes through the disk layer but reads back with raw SQL (WHERE ... and raw = 1), so re-encoding those would double-encode them.

Migrating existing caches

Entries written by an older dvc-data are recognised by content, not a schema version: a protocol-2+ pickle starts with the PROTO opcode 0x80, which no JSON encoding can begin with. A legacy entry is refused, evicted, and reported as a miss. All four caches live under tmp_dir and are regenerable, so that costs a recomputation, not data.

Reading a legacy entry evicts it, so the two places that iterate a Cache now materialize their keys and read via get() — otherwise .items() races the eviction and raises KeyError. That's a real bug I only found by exercising the Index and links paths rather than the cache in isolation. get_unused_links also compares the stored (inode, mtime) pair as a sequence, since JSON has no tuple type.

On the existing pickle tests

The declined #364 drew the objection that it "changed the existing tests to expect UnpicklingError… documents the regression but does not resolve it." This does not do that. test_pickle_protocol_error still asserts DiskError, because keys are still pickled and that path stays live; the protocol-interop test now asserts a successful read, because values no longer depend on the pickle protocol at all.

Verification

• Full suite green: 241 passed (baseline 219 → +22 new tests).
• Every new test confirmed to fail against unpatched src/ — 15 failed / 29 passed with src/ reverted and the tests kept. A security test that was never watched to fail proves nothing.
• test_poisoned_entry_is_not_executed is an end-to-end exploit: injecting a __reduce__ payload into the cache DB executes it against stock diskcache and returns a miss here.
• ruff check: all checks passed. ruff format --check: clean.
• Large-value (file-backed, >min_file_size) path checked separately: round-trips, payload starts b'{', exactly one value file, no orphan.

Happy to split the migration handling into its own commit, or to gate the JSON disk behind a setting, if you'd prefer a smaller first step.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}