hardening: cache write race — old signed URL can clobber a newer cache blob (generational writes)
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 7
- Forks
- 3
- Avg merge
- 2h 12m
- Merged PRs (30d)
- 30
Description
Split off from #210 (Item 3). Lower priority than the artifact write-once work (#210 Item 1, PR #297) because it is fail-safe.
Problem
Caches share the signing API with artifacts but intentionally overwrite a deterministic key: CacheStorageKey = "cache/<projectID>/<sha256(key)>" (store/caches.go), and UpsertPendingCache keeps the same storage_key so re-uploads replace the blob in place (grpcsrv/cache.go, handler.go routes cache/ keys to Put, not PutCreateOnly).
That opens an "old cache URL clobbers new cache" race: two overlapping uploads for one key share one storage_key; a stale-but-valid signed PUT (15-min TTL) landing after a newer upload overwrites the newer blob, while MarkCacheReady (keyed by row id) may have recorded the newer uploader's sha → blob/metadata divergence. Nothing at the backend gates this.
Why it's lower priority
It is fail-safe: the worst outcome is a corrupted/mismatched cache entry, which a consumer treats as a cache miss and rebuilds. No artifact integrity or deploy correctness is affected (that's #210 Item 1). So this is a robustness/cleanliness fix, not a data-loss fix.
Proposed direction
Make cache writes generational instead of in-place:
- Mint a fresh
storage_keyper cache upload (e.g. append a generation/uuid), and swap the ready-row to point at the new key atomically once the upload is confirmed; delete the superseded blob on a sweep. - OR use a conditional write (
If-Generation-Matchon GCS / a version guard on S3) so a stale PUT can't clobber a newer generation.
Reuse the create-only signing plumbing added in #210 (PutOption / SignedURL.Headers) where it fits.
Acceptance criteria
- A stale signed cache PUT landing after a newer upload cannot overwrite the newer blob (or is detectably rejected).
- Blob bytes and the recorded sha for a cache row never diverge.
- Cache hit/restore semantics unchanged; no new backend round-trip on the hot path.
- Tests covering the overlapping-upload race.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Trace cache key and upload handling in store/caches.go, grpcsrv/cache.go, and handler.go, including the PutOption and SignedURL plumbing from #210. Start by mapping overlapping uploads through signing, blob writes, and MarkCacheReady. Done means stale writes cannot overwrite or diverge from the recorded sha, cache restore behavior is unchanged, and a race test covers the overlap without adding a hot-path backend round trip.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100