Skip to content

A disk-full error on the final chunk of a download publishes a truncated object that passes hash verification #45

Description

@matt-edmondson

What's wrong

On a cache miss, ObjectRouteHandler sends the upstream body to the client and, through StreamTee.CopyAsync, to a staging file. It then calls store.PublishAsync(staging, ...), and publishing depends only on whether the digest matches the object id. Three things combine to let a truncated file through:

  1. HashingStream hashes before it writes (Storage/HashingStream.cs, Write / WriteAsync):
    _hash.AppendData(buffer.Span);
    await inner.WriteAsync(buffer, cancellationToken).ConfigureAwait(false);
    If inner.WriteAsync throws, those bytes are already in the digest even though they never reached disk.
  2. StreamTee swallows the sink failure. It catches IOException, sets liveSecondary = null, logs, and keeps streaming to the client.
  3. The download path publishes anyway. The upload path guards with if (response.IsSuccessStatusCode && teed.SinkIsLive). The download path (ObjectRouteHandler.cs, ~lines 330-347) calls PublishAsync with no check on whether the staging write failed.

If the failed write is the last chunk, no further data reaches the hash. The digest equals the oid, so PublishAsync moves the short file into objects/. For any object up to about 80 KB, the first chunk is also the last.

Reproduction (from the review)

HashingStream and StreamTee were copied into a scratch app, writing to a real FileStream on a 64 KB tmpfs with about 20 KB free and sending one 60,000-byte object:

secondary failed: IOException No space left on device
close ok
client got 60000 bytes; digest==oid: True; staging file length on disk: 16384

Why it matters

The first client gets correct bytes. After that, every cache hit serves the 16 KB truncated file with a matching Content-Length, and each git-lfs client fails its own hash check until the object is evicted. The disk-full condition that causes this is also the moment the cache is most likely to be under pressure.

Suggested fix

  • In HashingStream, call _hash.AppendData only after inner.Write / inner.WriteAsync succeeds.
  • Carry the sink-failed state out of StreamTee on the download path (for example StagingHandle.Faulted) and skip PublishAsync when it is set, as the upload path does with SinkIsLive.
  • Optionally, as a further guard, make PublishAsync check that the staged file's length equals the number of bytes hashed.

Acceptance criteria: a test uses a staging stream that throws on its final write and asserts that the object is not published.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

readyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions