kvcache-ai / kvcache-ai/Mooncake
[Bug]: File-per-key path mapping can collide and return another object's data
- Dominant language
- C++
- Stars
- 6.6k
- Forks
- 1.2k
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 312
Description
## Bug Report
On current `main` (`f20b7061`), `ResolvePathFromKey()` is not collision-safe for file-per-key storage.
The path uses only the low 8 bits of `std::hash` for its two directory levels, while `SanitizeKey()` maps each of `/\\:*?"<>|` to `_`. For a fixed tenant, the 10,000 four-character keys built from those characters plus `_` all sanitize to the same filename but have only 256 possible directory pairs, so at least two distinct keys must map to the same path.
This is a correctness problem because:
- writes open the destination with `O_TRUNC`, so the later key overwrites the earlier file;
- `StorageBackendAdaptor::BatchLoad()` deserializes the stored `KVEntry` without checking that its key matches the requested key;
- when the colliding keys and values have equal lengths, a read can silently return the other key's value.
The same path helper is used by file-per-key LOCAL_DISK storage and the legacy DISK replica path.
## Expected behavior
Distinct object identities must never resolve to the same on-disk object path in normal operation, and reads must reject records whose stored key does not match the requested key.
## Proposed direction
Use a stable full-key digest for the filename, retain a legacy-path read fallback for existing files, and validate the deserialized key before copying data to the caller.
### Before submitting...
- [x] Searched existing issues and open PRs for path/key collision reports.
Contributor guide
Research direction
Locate ResolvePathFromKey(), SanitizeKey(), and StorageBackendAdaptor::BatchLoad(), then trace their LOCAL_DISK and legacy DISK callers. Reproduce the described colliding keys, inspect existing storage tests if available, and confirm that distinct keys receive distinct paths, legacy files remain readable, and BatchLoad rejects a stored key mismatch.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 62/100