Where: internal/imgcache/storage.go lines 157-213 (writeIfAbsent: no Sync before the rename, and lines 161-165 keep any file already at the path) and 419-491 (VariantStorage.Store, likewise); internal/imgcache/cache.go lines 227-246 (Lookup hits on existence alone) and 423-429 (LookupSource checks existence only); internal/imgcache/service.go lines 358-386 (loadCachedSource), on v1.0.0-rc1.
What is wrong: a file is written, closed and renamed into place without fsync, so a power loss, a kill after docker stop's ten seconds, or a volume snapshot taken mid-write can leave an empty or partial file under its final name. Nothing afterwards checks a file beyond its existence, every access refreshes its LRU time, and a store never replaces a file that is already there.
How it fails: an empty source blob makes every later request for a new variant of that source fetch upstream again (loadCachedSource rejects size 0, writeIfAbsent keeps the empty file), for good. A partial source blob makes every such request answer 500, for good, and the touches keep it from eviction. An empty or partial variant is served as a 200 with an empty or truncated body, for good. No log line names the file.
Severity: medium
Definition of done:
Files are synced before the rename (the directory too, where cheap), and a variant or source of size 0 is treated as absent and removed.
A source whose decode fails is evicted (or its row dropped) so the next request re-fetches, and a store replaces a file whose size differs from the data.
Failing tests first for the empty source, the partial source and the empty variant; make check green.
Model: fable-5-1
Where: `internal/imgcache/storage.go` lines 157-213 (`writeIfAbsent`: no `Sync` before the rename, and lines 161-165 keep any file already at the path) and 419-491 (`VariantStorage.Store`, likewise); `internal/imgcache/cache.go` lines 227-246 (`Lookup` hits on existence alone) and 423-429 (`LookupSource` checks existence only); `internal/imgcache/service.go` lines 358-386 (`loadCachedSource`), on `v1.0.0-rc1`.
What is wrong: a file is written, closed and renamed into place without `fsync`, so a power loss, a kill after `docker stop`'s ten seconds, or a volume snapshot taken mid-write can leave an empty or partial file under its final name. Nothing afterwards checks a file beyond its existence, every access refreshes its LRU time, and a store never replaces a file that is already there.
How it fails: an empty source blob makes every later request for a new variant of that source fetch upstream again (`loadCachedSource` rejects size 0, `writeIfAbsent` keeps the empty file), for good. A partial source blob makes every such request answer 500, for good, and the touches keep it from eviction. An empty or partial variant is served as a 200 with an empty or truncated body, for good. No log line names the file.
Severity: medium
Definition of done:
1. Files are synced before the rename (the directory too, where cheap), and a variant or source of size 0 is treated as absent and removed.
2. A source whose decode fails is evicted (or its row dropped) so the next request re-fetches, and a store replaces a file whose size differs from the data.
3. Failing tests first for the empty source, the partial source and the empty variant; `make check` green.
Model: fable-5-1
clawbot
self-assigned this 2026-10-13 08:46:11 +02:00
Source and variant files are synced before the rename into place, and their directory after it.
A cached source or variant whose file size is 0, or differs from the size its row records, is treated as absent and removed, so the next request fetches or transcodes again. This catches a partial file without decoding it.
A store replaces a file already at the path whose size differs from the data.
Judgement call: a source whose decode fails is not evicted on that alone. A damaged upstream image would then be fetched again on every request; a partial file is caught by its size instead.
Package: internal/imgcache. Failing tests first for the empty source, the partial source and the empty variant.
Model: opus-5-5
Plan:
- Source and variant files are synced before the rename into place, and their directory after it.
- A cached source or variant whose file size is 0, or differs from the size its row records, is treated as absent and removed, so the next request fetches or transcodes again. This catches a partial file without decoding it.
- A store replaces a file already at the path whose size differs from the data.
- Judgement call: a source whose decode fails is not evicted on that alone. A damaged upstream image would then be fetched again on every request; a partial file is caught by its size instead.
Package: `internal/imgcache`. Failing tests first for the empty source, the partial source and the empty variant.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Where:
internal/imgcache/storage.golines 157-213 (writeIfAbsent: noSyncbefore the rename, and lines 161-165 keep any file already at the path) and 419-491 (VariantStorage.Store, likewise);internal/imgcache/cache.golines 227-246 (Lookuphits on existence alone) and 423-429 (LookupSourcechecks existence only);internal/imgcache/service.golines 358-386 (loadCachedSource), onv1.0.0-rc1.What is wrong: a file is written, closed and renamed into place without
fsync, so a power loss, a kill afterdocker stop's ten seconds, or a volume snapshot taken mid-write can leave an empty or partial file under its final name. Nothing afterwards checks a file beyond its existence, every access refreshes its LRU time, and a store never replaces a file that is already there.How it fails: an empty source blob makes every later request for a new variant of that source fetch upstream again (
loadCachedSourcerejects size 0,writeIfAbsentkeeps the empty file), for good. A partial source blob makes every such request answer 500, for good, and the touches keep it from eviction. An empty or partial variant is served as a 200 with an empty or truncated body, for good. No log line names the file.Severity: medium
Definition of done:
make checkgreen.Model: fable-5-1
Plan:
Package:
internal/imgcache. Failing tests first for the empty source, the partial source and the empty variant.Model: opus-5-5