Repository navigation
cache: reject path separators in LocalCacheBackend keys - #2
benjaminkincses wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Hardens LocalCacheBackend._path to reject any key containing /, , or the literals . and .. by raising ValueError before any path is built. A remote-supplied key now fails fast instead of producing a cache miss or an escaped file. Sharding on the first two characters still lets a crafted key open a path outside the cache directory.
Worth a look
- LocalCacheBackend.get now raises for slash-containing keys instead of returning a miss —
reagraph/cache.py:96· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Sharded path can still escape via two-dot shard prefix —
reagraph/cache.py:98· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 88 functions depend on the 38 functions this change touches.
Health — grade A; 6 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
main()— 1 callers, 10 callees (medium)main()— 1 callers, 7 callees (medium)test_tiered_backend_warms_local_on_remote_hit()— 0 callers, 9 callees (medium)test_cross_machine_round_trip()— 0 callers, 6 callees (medium)test_persist_remote_false_serves_remote_without_warming_local()— 0 callers, 6 callees (medium)test_tiered_remote_hit_populates_local()— 0 callers, 6 callees (medium)
Verification — 88 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 88 function(s) in the blast radius were not formally verified this run
LocalCacheBackend._pathbuilds a filesystem path from the cache key with no validation: shard on the first two characters, the rest becomes the filename. Keys are sha256 hex digests when Cache makes them, butTieredBackend.keys()yields keys supplied by a remote cache server, andCache.clear()feeds backend keys back into delete and get. A key like../../escape_markerresolves outside the cache directory, so a malicious or buggy shared-cache server can make a tiered client read or unlink .json files outside its cache dir.Fix: reject keys containing a path separator or equal to
.or..in_path, the one place every local backend operation funnels through.Verified locally:
_path("../../escape_marker")now raises ValueError, and a normal 64-hex key still resolves inside the cache dir.