Fix malformed chunk cache directory handling - #961
Open
efe-arv wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cac7a48. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
MerkleHashbefore slicingWhy
DiskCache::initialize_statescanned cache directories before fully validating their names. A stray directory such as<cache>/ab/xcaused an out-of-bounds panic while slicing the first two bytes. A matching Base64 name such as<cache>/AA/AApassed that check but decoded to fewer than 32 bytes and panicked while extracting the hash. In debug builds, a valid-looking key under the wrong prefix also triggered an assertion; in release builds it could be admitted under an inconsistent path.Cache initialization already skips other malformed entries, so this keeps that fail-soft behavior consistent without changing valid cache entries.
Verification
test_initialize_ignores_short_key_directory_namereproducedrange end index 2 out of range for slice of length 1test_initialize_ignores_key_directory_under_wrong_prefixreproduced the prefix assertion failurecargo +1.95.0 test -p xet-client --lib: 209 passed, 4 ignoredcargo +1.95.0 test --no-fail-fast --features "strict simulation internal-tools git-xet-for-integration-test": passedcargo +1.95.0 clippy -r -p xet-client --lib -- -D warnings: passedrustup run 1.95.0 rustfmt --edition 2024 --check xet_client/src/chunk_cache/disk.rs: passedgit diff --check: passedScope
One Rust source file; no dependency or public API changes.
Note
Low Risk
Changes only affect scanning of existing cache directories at init; valid entries behave the same and malformed paths are ignored rather than crashing.
Overview
Disk cache startup no longer panics or asserts when the on-disk layout contains stray or invalid key directories. During
initialize_state, encoded key folder names are validated before parsing: names shorter than the two-byte prefix are skipped, and the name’s prefix must match the parent prefix directory (case-insensitive)—replacing a debug-only assertion that could fail in debug builds or admit inconsistent paths in release.Key decoding in
try_parse_keynow checks that Base64-decoded data is at leastMerkleHash-sized before slicing, returning a parse error instead of panicking on truncated payloads (e.g.AA/AA).Three unit tests lock in fail-soft behavior for short directory names, undersized decoded keys, and keys stored under the wrong prefix folder.
Reviewed by Cursor Bugbot for commit 9a5da45. Bugbot is set up for automated code reviews on this repo. Configure here.