fix: reject short cache range reads

README.md

@@ -93,14 +93,17 @@ When configuring S3 clients, ensure path-style addressing is enabled. See [docs/
 
 | Header    | Description                                          |
 | --------- | ---------------------------------------------------- |
-| `X-Cache` | Cache status: `HIT`, `MISS`, `BYPASS`, or `DISABLED` |
+| `X-Cache` | Cache decision at response commitment: `HIT`, `MISS`, `BYPASS`, or `DISABLED`; a block-mode `HIT` may include a verified upstream remainder |
 
 ### Cache Behavior
 
 - Objects larger than `size_threshold` are not cached
 - Objects with `Cache-Control: no-store` or `private` are not cached
 - Among cacheable objects (within `size_threshold`), those at or above `block_size` are cached as fixed-size blocks (block caching, on by default), so a range read fetches only the covering blocks; smaller objects are whole-cached
 - Set `block_caching_enabled: false` to cache whole objects instead — a range request then triggers a background fetch of the full object (if within threshold)
+- Cached range reads are checked against the requested byte count. A committed block GET can recover an unwritten suffix only from an upstream response with the same ETag
+- `X-Cache: HIT` identifies a cache-backed response selected at commitment; it does not promise that every byte came from cache or that the transfer completed
+- If a committed block GET has a changed-ETag suffix, or a whole-object Range read is short, TAG keeps the sent status and Content-Length, returns a handler error, and may end with only the cached prefix; client write failures stop without an upstream retry
 - PUT/DELETE operations invalidate the cache entry
 
 See [docs/cache-control.md](docs/cache-control.md) for detailed cache control and revalidation documentation. For tuning and sizing block caching in production, see [docs/deploy.md](docs/deploy.md#block-aligned-caching).
@@ -141,7 +144,7 @@ For production, TAG ships manifests, Compose files, and guides under [`deploy/`]
 ### Request Flow
 
 1. **Cache Check**: TAG first checks if the object exists in its embedded cache
-2. **Cache Hit**: Returns cached object with `X-Cache: HIT` header
+2. **Cache Hit**: Serves using a cache entry with `X-Cache: HIT`; a degraded block stream may append a same-ETag upstream remainder
 3. **Cache Miss**: Forwards request to upstream Tigris, caches response, returns with `X-Cache: MISS`
 
 ### Request Coalescing

cache/cache.go

@@ -471,7 +471,8 @@ func (c *Cache) IsBlockLocal(bucket, key, etag string, blockSize, blockIdx int64
 // Uses ocache's GetRangeStream for efficient partial reads from disk.
 // start and end are inclusive byte positions (HTTP Range semantics).
 // Pass the meta's ETag so the range resolves to the exact cached version.
-// Returns ErrNotFound if the object is not in cache.
+// Returns ErrNotFound if the object is not in cache. A positive short backend read returns an
+// error after writing its available bytes to w.
 func (c *Cache) GetRangeStream(ctx context.Context, bucket, key, etag string, start, end int64, w io.Writer) error {
 	if !c.IsEnabled() {
 		return ErrCacheDisabled
@@ -483,7 +484,8 @@ func (c *Cache) GetRangeStream(ctx context.Context, bucket, key, etag string, st
 // block of a block-mode object to w. blockIdx identifies the block; start and end are
 // offsets WITHIN the block (0 = first byte of the block), not within the object. Pass the
 // meta's ETag so the block resolves to the exact cached version. Returns ErrNotFound if the
-// block is not in cache. See RFC 0001.
+// block is not in cache. A positive short backend read returns an error after writing its
+// available bytes to w. See RFC 0001.
 func (c *Cache) GetBlockRangeStream(ctx context.Context, bucket, key, etag string, blockSize, blockIdx, start, end int64, w io.Writer) error {
 	if !c.IsEnabled() {
 		return ErrCacheDisabled
@@ -505,8 +507,9 @@ func (cw *countingWriter) Write(p []byte) (int, error) {
 }
 
 // getRangeStreamByKey streams an inclusive byte range [start,end] of the blob at cacheKey to
-// w, mapping ocache's not-found to ErrNotFound and handling ocache's read-byte-0 quirk.
-// bucket/key are used for logging only.
+// w, mapping ocache's not-found to ErrNotFound, handling ocache's read-byte-0 quirk, and
+// rejecting successful reads that do not deliver the requested range in full. bucket/key are
+// used for logging only.
 func (c *Cache) getRangeStreamByKey(ctx context.Context, cacheKey, bucket, key string, start, end int64, w io.Writer) error {
 	// Handle ocache quirk: reading byte 0 alone requires reading 2 bytes
 	// and discarding the last byte
@@ -562,6 +565,13 @@ func (c *Cache) getRangeStreamByKey(ctx context.Context, cacheKey, bucket, key s
 		log.Debug().Str("bucket", bucket).Str("key", key).Msg("Cache miss (range)")
 		return ErrNotFound
 	}
+	want := end - start + 1
+	if cw.written < want {
+		return fmt.Errorf("cache range short read: got %d of %d bytes: %w", cw.written, want, io.ErrUnexpectedEOF)
+	}
+	if cw.written > want {
+		return fmt.Errorf("cache range over-read: got %d bytes, want %d", cw.written, want)
+	}
 
 	c.recordServeLocality(cacheKey)
 	log.Debug().

docs/cache-control.md

@@ -6,12 +6,14 @@ TAG supports RFC 7234-compliant cache revalidation. Clients can control caching
 
 | Value         | Meaning                                                                          |
 | ------------- | -------------------------------------------------------------------------------- |
-| `HIT`         | Served from cache (includes revalidation that confirmed the object is unchanged) |
+| `HIT`         | A cache entry was selected to serve the response (including a revalidation that confirmed the object is unchanged) |
 | `MISS`        | Not in cache, fetched from upstream and now cached                               |
 | `REVALIDATED` | Revalidated with upstream, object changed, new content returned                  |
 | `BYPASS`      | Cache bypassed entirely (client requested `no-store`)                            |
 | `DISABLED`    | Caching is disabled server-side (`TAG_CACHE_DISABLED=true`)                      |
 
+In block mode, a committed stream can recover missing bytes from an upstream range only after validating the cached ETag and requested range. `HIT` is committed before the body streams, so it describes the cache-backed request, not each byte's source or whether the transfer completed. See [S3 compatibility](s3-compatibility.md#caching-behavior) for incomplete cache-range behavior.
+
 ## Force Revalidation
 
 Send `Cache-Control: no-cache` or `Cache-Control: max-age=0` to force TAG to check with upstream before serving a cached object. TAG sends a conditional request using the cached ETag. If the object hasn't changed, upstream returns 304 and TAG serves from cache (`X-Cache: HIT`). If changed, TAG streams the new content (`X-Cache: REVALIDATED`).

docs/metrics.md

@@ -28,7 +28,7 @@ Total number of requests processed by TAG.
 | `operation` | S3 operation: `GetObject`, `PutObject`, `DeleteObject`, `HeadObject` |
 | `status`    | Result: `success`, `error`, `auth_error`, `range_not_satisfiable`    |
 | `mode`      | The process's operating mode (`transparent`, `signing`, `tiered`) — one constant value per instance, for fleet-wide splits |
-| `source`    | Where the response was produced: `local` (TAG's own store or knowledge — cache hits, revalidated-304 serves, authoritative misses, auth errors) or `upstream` (proxied) |
+| `source`    | Request handling path, not per-byte provenance: `local` for TAG's cache/local-answer path (including verified block-suffix recovery), or `upstream` for a directly proxied response |
 
 **Example queries:**
 
@@ -39,7 +39,7 @@ rate(tag_requests_total[5m])
 # Error rate
 sum(rate(tag_requests_total{status="error"}[5m])) / sum(rate(tag_requests_total[5m]))
 
-# Share of traffic answered without touching upstream (per mode)
+# Share of requests handled by TAG's local/cache path (per mode; may include upstream recovery)
 sum(rate(tag_requests_total{source="local"}[5m])) by (mode) / sum(rate(tag_requests_total[5m])) by (mode)
 
 # GetObject success rate
@@ -57,7 +57,7 @@ Request duration in seconds.
 | ----------- | ------------ |
 | `operation` | S3 operation |
 | `mode`      | Operating mode (constant per instance) |
-| `source`    | `local` or `upstream` — where the response was produced |
+| `source`    | `local` or `upstream` — request handling path, not every byte's origin |
 
 **Buckets:** 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 1.5, 2, 2.5, 3, 4, 5, 7.5, 10 (the default Prometheus buckets plus extra tail resolution at 1.5, 2, 3, 4 and 7.5 seconds)
 
@@ -80,9 +80,12 @@ rate(tag_request_duration_seconds_sum[5m]) / rate(tag_request_duration_seconds_c
 
 **Type:** Counter
 
-Total number of cache hits — every request served from cache, including
-range-from-cache hits and conditional 304 (Not Modified) responses. It is
-recorded in lockstep with the `X-Cache: HIT` response header.
+Total number of requests whose cache status was committed as `X-Cache: HIT`,
+including range-from-cache hits and conditional 304 (Not Modified) responses. It
+is recorded in lockstep with that header. This is a request-level cache decision,
+not a per-byte source or successful-transfer count: a degraded block response may
+append a verified upstream suffix or later finish with an error, which is recorded
+separately in `tag_requests_total`.
 
 `tag_cache_hits_total` and `tag_cache_misses_total` count only `HIT` and `MISS`.
 `REVALIDATED` responses (object changed on upstream) are neither — they are tracked

docs/rfcs/0001-block-aligned-caching.md

@@ -119,7 +119,7 @@ Rare for a range-read-dominated workload but must be correct — and fast, since
 
 So a block-eligible object is **always** block-mode; a full read never produces a competing whole-body entry.
 
-**Bounded degraded serves.** Because `blocks_complete` is a hint and blocks evict independently of the meta, a committed serve (headers already sent) recovers at most `maxInlineFetchesPerServe` (2) absent blocks via individual aligned upstream fetches. Past that cap, on a populate-budget decline, or on a transient cache-read failure, the committed response is completed **byte-exact from a single uncached upstream remainder stream** — validated against the entry's ETag and the exact remaining byte range — instead of truncating, and a cap-tripped (mass-evicted) entry is invalidated so the next GET re-establishes it with one streaming re-split. Only three things still truncate a committed body: a stale signal (a different object version must never be mixed into a committed response), a client write failure (the client is broken, not the cache — tracked explicitly so it is never "salvaged" with a wasted upstream round trip), and a failed remainder stream.
+**Bounded degraded serves.** Because `blocks_complete` is a hint and blocks evict independently of the meta, a committed serve (headers already sent) recovers at most `maxInlineFetchesPerServe` (2) absent blocks via individual aligned upstream fetches. Past that cap, on a populate-budget decline, or on a transient or short cache-read failure, the committed response is completed **byte-exact from a single uncached upstream remainder stream** — validated against the entry's ETag and the exact remaining byte range — instead of truncating, and a cap-tripped (mass-evicted) entry is invalidated so the next GET re-establishes it with one streaming re-split. Only three things still truncate a committed body: a stale signal (a different object version must never be mixed into a committed response), a client write failure (the client is broken, not the cache — tracked explicitly so it is never "salvaged" with a wasted upstream round trip), and a failed remainder stream. The request-level `X-Cache: HIT` is committed before streaming; it identifies the cache-backed response selection, not every byte's source or successful completion.
 
 A client-forced revalidation (`Cache-Control: no-cache`) of a block-mode entry likewise invalidates it and lets the miss path re-establish the current version.
 

docs/s3-compatibility.md

@@ -71,7 +71,7 @@ TAG adds a caching layer for read operations. The `X-Cache` response header indi
 
 | Value      | Meaning                                                  |
 | ---------- | -------------------------------------------------------- |
-| `HIT`      | Served entirely from cache                               |
+| `HIT`      | A cache entry was selected to serve the request; block streams may also recover a verified upstream remainder |
 | `MISS`     | Fetched from Tigris and cached                           |
 | `BYPASS`   | Caching skipped (e.g., conditional request returned 304) |
 | `DISABLED` | Caching is turned off                                    |
@@ -80,6 +80,7 @@ Key caching behaviors:
 
 - **Request coalescing** — Multiple concurrent GETs for the same object are coalesced into a single upstream fetch using a broadcast/subscriber pattern.
 - **Range requests** — If the full object is cached, range requests are served from cache. On a range cache miss, the range is served directly from Tigris while a background fetch populates the cache with the full object.
+- **Incomplete cache ranges** — TAG checks that a cached range supplies the requested byte count. A committed block GET can recover an unwritten suffix only when the upstream ETag and range match the cached object. `X-Cache: HIT` is committed before the stream; it identifies the cache-backed request, not per-byte provenance or successful completion. If the ETag changes, TAG rejects the suffix without appending it; the already-sent 200 status and Content-Length remain, so the client can receive only the cached prefix while the handler reports an error. A whole-object cached Range response commits after its first byte and cannot be restarted; a later short read returns a handler error, and the 206 body may end before Content-Length. Client write failures are not retried upstream.
 - **Conditional requests** — `If-None-Match` and `If-Modified-Since` are evaluated against cached metadata and can return 304 without hitting Tigris.
 - **Write-through invalidation** — PUT, DELETE, CopyObject, and DeleteObjects invalidate the cache _before_ forwarding to Tigris to prevent stale reads.
 - **Tombstone protection** — A short-lived tombstone is written on DELETE to prevent in-flight background cache writes from resurrecting deleted objects.

metrics/metrics.go

@@ -458,10 +458,11 @@ var (
 )
 
 // RecordRequest records a request with its duration and status.
-// Source label values for RecordRequest: where the response was produced.
+// Source label values identify the request path that owns response accounting,
+// not the origin of every byte in a streamed response.
 const (
-	SourceLocal    = "local"    // answered from TAG's own store or knowledge
-	SourceUpstream = "upstream" // proxied from upstream
+	SourceLocal    = "local"    // handled through TAG's local/cache path, including verified block-suffix recovery
+	SourceUpstream = "upstream" // proxied directly from upstream
 )
 
 // processMode is the operating mode stamped on every request metric. One
@@ -482,6 +483,17 @@ func RecordRequest(operation, status, source string, durationSeconds float64) {
 	RequestDuration.WithLabelValues(operation, processMode, source).Observe(durationSeconds)
 }
 
+// RecordRequestResult maps an operation result to the shared success/error status labels.
+// Callers that own specialized outcomes such as authentication or range validation use
+// RecordRequest with their explicit status instead.
+func RecordRequestResult(operation, source string, err error, durationSeconds float64) {
+	status := "success"
+	if err != nil {
+		status = "error"
+	}
+	RecordRequest(operation, status, source, durationSeconds)
+}
+
 // RecordCacheHit records a cache hit.
 func RecordCacheHit() {
 	CacheHits.Inc()

proxy/block_cache.go

@@ -206,10 +206,11 @@ func (s *Service) serveRangeFromBlockCache(
 	w.WriteHeader(http.StatusPartialContent)
 
 	if _, berr := s.streamBlockRange(ctx, w, bucket, key, accessKey, secretKey, meta, rng.start, rng.end); berr != nil {
+		metrics.RecordRequestResult("GetObject", metrics.SourceLocal, berr, time.Since(startTime).Seconds())
 		return true, berr
 	}
 	metrics.RecordRangeFromCacheHit()
-	metrics.RecordRequest("GetObject", "success", metrics.SourceLocal, time.Since(startTime).Seconds())
+	metrics.RecordRequestResult("GetObject", metrics.SourceLocal, nil, time.Since(startTime).Seconds())
 	return true, nil
 }
 
@@ -371,10 +372,11 @@ func (s *Service) serveAssembledRange(
 		metrics.BytesTransferred.WithLabelValues("out").Add(float64(n))
 	}
 	if werr != nil {
+		metrics.RecordRequestResult("GetObject", metrics.SourceLocal, werr, time.Since(startTime).Seconds())
 		return true, werr
 	}
 	metrics.RecordRangeFromCacheHit()
-	metrics.RecordRequest("GetObject", "success", metrics.SourceLocal, time.Since(startTime).Seconds())
+	metrics.RecordRequestResult("GetObject", metrics.SourceLocal, nil, time.Since(startTime).Seconds())
 	// A parquet reader's trailer probe is a few bytes, so it is served here rather
 	// than by streamBlockRange — this, not the streaming path, is where the footer
 	// signal actually arrives for a reader opening a file.
@@ -872,9 +874,10 @@ func (s *Service) serveFullObjectFromBlockCache(
 	w.WriteHeader(meta.StatusCode)
 
 	if _, berr := s.streamBlockRange(ctx, w, bucket, key, accessKey, secretKey, meta, 0, meta.ContentLength-1); berr != nil {
+		metrics.RecordRequestResult("GetObject", metrics.SourceLocal, berr, time.Since(startTime).Seconds())
 		return true, berr
 	}
-	metrics.RecordRequest("GetObject", "success", metrics.SourceLocal, time.Since(startTime).Seconds())
+	metrics.RecordRequestResult("GetObject", metrics.SourceLocal, nil, time.Since(startTime).Seconds())
 	return true, nil
 }
 
@@ -897,9 +900,9 @@ func (s *Service) remainingMetaTTL(meta *cache.CachedObjectMeta) int {
 // commit: if it is absent (evicted since populate) it is fetched pre-commit, and if that fails
 // the response is untouched — (false, nil) lets the caller fall through to the miss path, and a
 // definitive stale signal invalidates the entry first. After commit, the remaining blocks
-// stream with an inline fetch recovering any absent one; only a fetch failure there truncates
-// (headers are already sent), which is the same terminal behavior the probe path has for a
-// block evicted between probe and stream. Hit/miss metrics count inline-fetched blocks as
+// stream with inline fetch recovering any absent one; a short cache read or failed inline fetch
+// can use the shared same-ETag remainder path. If recovery fails, the caller can only return the
+// post-commit error — it cannot fall through. Hit/miss metrics count inline-fetched blocks as
 // misses, recorded once the serve outcome is known.
 func (s *Service) serveCompleteFromBlocks(
 	ctx context.Context,
@@ -962,9 +965,10 @@ func (s *Service) serveCompleteFromBlocks(
 		}
 	}
 	if berr != nil {
+		metrics.RecordRequestResult("GetObject", metrics.SourceLocal, berr, time.Since(startTime).Seconds())
 		return true, berr
 	}
-	metrics.RecordRequest("GetObject", "success", metrics.SourceLocal, time.Since(startTime).Seconds())
+	metrics.RecordRequestResult("GetObject", metrics.SourceLocal, nil, time.Since(startTime).Seconds())
 	return true, nil
 }