fix(cache): treat a cache entry with an overflowing duration as corrupt - #786
Merged
Merged
Conversation
Decoding a cache entry built its duration with `Duration::new` without checking the fields, so seconds near `u64::MAX` plus nanoseconds that carry over panicked and aborted the run. Use wincode's own `Duration` schema, which has the same encoding and rejects the overflow, so a remote entry like this is a corrupt value and a cache miss. Fixes #780 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fspy benchmarklinuxmacoswindows |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Motivation
Fixes #780. A cache entry stores the task's duration as seconds plus nanoseconds, and decoding it called
Duration::newwithout checking them. Seconds nearu64::MAXwith nanoseconds that carry over panic there, and release builds abort on panic, so a single corrupt or malicious remote cache entry killed the wholevp runinstead of being a cache miss.Changes
DurationSchemaand use wincode's built-inDurationschema. The adapter dates from the bincode migration (refactor: migrate from bincode to wincode #334), when wincode 0.5 had noDurationsupport. wincode 0.6's schema has the same encoding (u64seconds, thenu32nanoseconds) and returns a decode error on overflow. The on-disk and remote formats don't change, so the cache schema version stays the same.remote cache value is corruptas its reason.🤖 Generated with Claude Code