fix(api): evict HTTP cache entries that fail integrity check - #300
Open
LucasLeao18 wants to merge 1 commit into
Open
LucasLeao18 wants to merge 1 commit into
LucasLeao18 wants to merge 1 commit into
Conversation
LucasLeao18
requested review from
williammartin
and removed request for
a team
September 15, 2026 03:26
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.
Follows up #252. Fixes the user-facing symptom in cli/cli#14394 for already-corrupted entries.
Description
#263 made cache publication atomic (temp file + rename), which stops new corruption from concurrent writers. But two gaps remain on the read side, and both end in commands failing with
invalid character 'd' after object key:{{"data":...interleaved writes ingh pr listwith flags fails JSON parsing cli#14394) have valid HTTP framing with a corrupt body.readparses the framing fine and serves the bad body, and the caller fails parsing JSON.This change records a SHA-256 digest of each entry in a
.sha256sidecar at publish time (same temp-file + rename pattern, checksum published first) and verifies it on read. An entry that fails verification — including entries written before checksums existed — is evicted and treated as a miss, so the request refetches and the cache self-heals. Failed publications still leave no trace (checksum rolled back with the entry).Read errors that look environmental (missing file, stat/IO failures, expiry) keep the existing behavior; only content verdicts trigger eviction.
Validation
TestCacheEvictsEntryWithCorruptBody: corrupts a cached entry on disk (framing intact, body altered — thegh pr listwith flags fails JSON parsing cli#14394 shape), asserts refetch + healed_/cache hit afterwards. Fails without the fix.TestCacheEvictsLegacyEntryWithoutChecksum: drops the sidecar, asserts refresh + heal. Fails without the fix.TestCachePublicationFailureDoesNotFailResponsefor the entry+sidecar pair (still asserts no leftovers).go vetclean; fullpkg/apisuite passes. (The onepkg/configfailure,TestStateDir, also fails on a clean checkout on Windows and is unrelated.)