Skip to content

fix(api): evict HTTP cache entries that fail integrity check - #300

Open
LucasLeao18 wants to merge 1 commit into
cli:trunkfrom
LucasLeao18:fix-cache-integrity-check
Open

LucasLeao18 wants to merge 1 commit into
cli:trunkfrom
LucasLeao18:fix-cache-integrity-check

Conversation

@LucasLeao18

Copy link
Copy Markdown

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:

  1. Entries corrupted before the atomic publish existed (e.g. the {{"data":... interleaved writes in gh pr list with flags fails JSON parsing cli#14394) have valid HTTP framing with a corrupt body. read parses the framing fine and serves the bad body, and the caller fails parsing JSON.
  2. Any entry corrupted after publication (crash, disk error, external modification) is served the same way.

This change records a SHA-256 digest of each entry in a .sha256 sidecar 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

  • New TestCacheEvictsEntryWithCorruptBody: corrupts a cached entry on disk (framing intact, body altered — the gh pr list with flags fails JSON parsing cli#14394 shape), asserts refetch + healed_/cache hit afterwards. Fails without the fix.
  • New TestCacheEvictsLegacyEntryWithoutChecksum: drops the sidecar, asserts refresh + heal. Fails without the fix.
  • Updated TestCachePublicationFailureDoesNotFailResponse for the entry+sidecar pair (still asserts no leftovers).
  • go vet clean; full pkg/api suite passes. (The one pkg/config failure, TestStateDir, also fails on a clean checkout on Windows and is unrelated.)

@LucasLeao18
LucasLeao18 requested a review from a team as a code owner September 15, 2026 03:26
@LucasLeao18
LucasLeao18 requested review from williammartin and removed request for a team September 15, 2026 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant