Cache bottle attestations #18581
Cache bottle attestations #18581
Conversation
This will help limit the number of times we need to query GitHub for the attestation in CI.
That's where we're making this change! |
I cannot read today 🙈 apologies! |
|
|
||
| sig { params(pathname: Pathname, scrub: T::Boolean).returns(T::Boolean) } | ||
| def stale_attestation?(pathname, scrub) | ||
| scrub || prune?(pathname, ATTESTATION_CLEANUP_DAYS) |
There was a problem hiding this comment.
Might be nice if this could be cleaned up based on the state of the cached bottles? Not a blocker, very much nice-to-have.
Co-authored-by: Carlo Cabrera <[email protected]>
|
|
||
| if cached_attestation.exist? | ||
| begin | ||
| return JSON.parse(cached_attestation.read) |
There was a problem hiding this comment.
Just to make sure I understand: the intended behavior here is to treat a cached attestation as implicitly previously verified, right?
I think we could go two ways with this: either assume cached == verified (which in turn means an attacker who can modify the cache/insert a dummy cache member can bypass verification, although such an attacker is already pretty powerful), or perform verification again on the cached attestation (so skip the download, but do the rest of the gh attestation verify).
The first seems fine to me, but the second is perhaps preferable.
There was a problem hiding this comment.
The first seems fine to me, but the second is perhaps preferable.
Agreed 👍🏻
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
woodruffw
added
in progress
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?This will help limit the number of times we need to query GitHub for the
attestation in CI.