Skip to content

fix(winbindex): verify the SHA-1 before sending, never serve wrong bytes - #75

Merged
Wenzel merged 1 commit into
masterfrom
fix/winbindex-verify-before-send
Sep 23, 2026
Merged

Wenzel merged 1 commit into
masterfrom
fix/winbindex-verify-before-send

Conversation

@Wenzel

@Wenzel Wenzel commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Problem

GET /blob/:hash?filename=<pe> streamed the symbol-server body to the client while hashing it, and compared the SHA-1 only once the stream ended. By then every byte, and the upstream Content-Length, had already gone out, so res.destroy() could not turn the download into a failure. The client received a complete file with the wrong content and HTTP 200.

Reproduced on a fresh quickstart stack with the public corpus:

notepad.exe  (win11-21h2-22000.194, expected 80d9c8bb…)  http=200 size=348160  got bf24c04a…
regedit.exe  (win11-21h2-22000.194, expected b1d804a1…)  http=200 size=397312  got 207a861f…

The API logged SHA-1 mismatch … destroying response, but curl and the browser saved the wrong binary with no error.

Fix

Buffer, verify, then send:

  • The symbol-server body is read into memory, capped at WINBINDEX_MAX_PE_BYTES (256 MiB, upstream cancelled past it).
  • The SHA-1 is checked before any header or byte is written. Only a matching file is sent, with an exact Content-Length. That also removes the content-encoding special case.
  • Every failure (non-2xx, fetch or stream error, timeout, oversized body, SHA-1 mismatch) returns not_available with the response untouched, so the route falls back to MinIO.
  • failed_after_send, waitForDrain and the backpressure loop are removed, since no outcome can fail after sending any more.

Trade-off: each in-flight download holds one PE file in memory (a few MB to a few tens of MB) instead of streaming it. The reason is documented in docs/reference/winbindex-source.md.

Verification

  • Unit tests updated to the new contract: a mismatch or mid-transfer stream error writes nothing, Content-Length is the verified size, and there's a new size-cap test that also checks the upstream is cancelled. tests/winbindex.test.ts: 37/37 pass. Full suite: 89/89 tests pass. tests/git-log/index.test.ts still fails to compile (ts-jest top-level await), same as on master.
  • npm run format-check and npm run build pass.
  • End to end, with this branch built as an image and run in the quickstart stack:
notepad.exe   http=502  (mismatch logged, fell back to MinIO, no wrong bytes sent)
regedit.exe   http=502  (same)
explorer.exe  http=200  size=5028992   SHA-1 OK
ntoskrnl.exe  http=200  size=11747664  SHA-1 OK

Not in this PR

On the quickstart stack the MinIO fallback answers 502 Storage Error (NoSuchBucket), because the public corpus ships no blobs. A 404 Blob not available would be a clearer answer. That's pre-existing MinIO-path behavior, left for a separate change.

🤖 Generated with Claude Code

The symbol-server body was streamed to the client while its SHA-1 was
computed, and compared only at the end. By then every byte (and the
Content-Length) had been sent, so destroying the response could not
fail the download: the client received a complete file with the wrong
content and HTTP 200. Win11 21H2 notepad.exe and regedit.exe hit this
on the public quickstart stack.

Buffer the file (capped at WINBINDEX_MAX_PE_BYTES, 256 MiB), verify it,
and only then send it with an exact Content-Length. Every failure
(non-2xx, stream error, timeout, oversized body, SHA-1 mismatch) now
leaves the response untouched and falls back to MinIO, so the
failed_after_send outcome and the drain/backpressure code are gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Wenzel
Wenzel merged commit 8631e27 into master Sep 23, 2026
2 checks passed
@Wenzel
Wenzel deleted the fix/winbindex-verify-before-send branch September 23, 2026 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant