fix: reject failed or incomplete downloads

[sdwoodard]

Summary

This PR fixes a download validation failure in which an HTTP error response,
interrupted transfer, or unverifiable cached file could be accepted as a valid artifact.

Downloads now require a successful response and completed transfer before validation and
atomic publication. Cached files are reused only with matching trustworthy metadata, and
callers either receive the accepted path or stop. The change is covered by 58 deterministic
tests with no public network dependency.

Root cause

The file download path wrote directly to the final target, did not reject unsuccessful
responses before writing, and returned the same implicit value after success and caught
request failures.

Verification checked size or MD5 only when the server supplied it, then returned success
when both were absent. Resume also appended responses without requiring a valid 206
response and consistent Content-Range.

Changes

• Require a final 200 OK response and finite (10, 30) second connect and read inactivity
  timeouts for the affected requests.
• Treat HEAD as optional metadata discovery. Failed, unsupported, or metadata free probes
  require a fresh GET and are not cached as proof that metadata is absent.
• Stream into an owned file beside the target, then flush, close, validate, and publish once
  with os.replace().
• Validate every trustworthy size or digest from the GET response. A normally completed
  200 without validators is treated as transport complete, not cryptographically verified.
• Reuse cached files only when available comparison metadata matches, and publish cache
  copies through the same staging process.
• Remove Range and resume behavior so every retry begins from byte zero.
• Propagate the exact accepted path through installer, ICU, winetricks, recommended AppImage,
  and self update callers so follow on work stops after failure.
• Keep compatibility MD5 use in nonsecurity mode, convert algorithm unavailability to a
  typed failure, and remove credentials, query strings, and fragments from diagnostics.
• Replace the live ip.me test with mocks and a local ThreadingHTTPServer.

No dependency or CI configuration changes are included.

Safety properties

After this change:

• unsuccessful response bodies and interrupted transfers cannot be published as valid
  artifacts;
• existing targets remain unchanged after transport, validation, or publication failure;
• cached files are not accepted solely because they exist or because a metadata probe
  failed; and
• callers do not continue to copy, extract, execute, install, replace, or restart after a
  failed download.

Testing

Tested commit: c3ed0a752f492d9c6cec2c59af33b553404265ff
Base: 43fa3e508dce3f11a15339dd9e65a4973fc57ffc

• python -m unittest -v tests.test_network — 58 of 58 passed.
• Python 3.12 with podman --network=none — all 58 network tests and Ruff passed.
• PyInstaller x86_64 build and built binary --version and --help smoke tests passed
  without network access.
• Localhost tests reject 404, 500, truncated, and unsolicited 206 responses while
  preserving existing targets. They also cover unsupported HEAD, metadata free GET,
  restart from byte zero, sanitized diagnostics, and the recommended AppImage handoff.
• Manual winetricks testing produced a valid executable installation and replaced a
  deliberately corrupted cache. Manual AppImage testing preserved the target and symlink
  on failure, then published and linked the accepted executable after success.
• Diff, ancestry, remote safety, clean tree, and unchanged HEAD checks passed.

The formal bash ./quality-gate.sh --full run exits 1 because of four known upstream
baseline unittest failures on the host and 21 existing mypy findings. The same environment
comparison found no unittest or mypy regression unique to this branch; pristine upstream
has 24 mypy findings. I have not represented the overall gate as passing.

Compatibility and limitations

• Interrupted downloads restart from zero.
• Endpoints without trustworthy HEAD validators require a fresh download instead of cache
  reuse.
• A fresh 200 without size or digest metadata is accepted only after normal stream
  completion, which establishes transport completion rather than cryptographic integrity.
• The 30 second read timeout measures socket inactivity, not total transfer duration.

[ctrlaltf24]

PR scope too large - rationale ambiguous.

What was the actual issue?

Why the sweeping changes?

[sdwoodard]

Thanks for the feedback.

This came from a static code scan which found that failed or interrupted downloads could be treated as valid when the server did not provide size or checksum metadata. That could allow an error response, partial transfer, or unverifiable cached file to continue into installation or execution. A failed replacement could also damage an existing valid file.

The change reaches several callers because they all rely on the same download behavior and could continue after a failure. I addressed the shared failure contract rather than adding separate checks at each call site, and removed the unsafe resume path instead of adding more protocol complexity. Most of the added lines are tests covering those affected paths.

I agree that the rationale should have been clearer and the review surface is large. I am happy to simplify or separate parts of the change if that would make it easier to review.

[ctrlaltf24]

I'm mostly concerned with the things that appear to be changing paths and whatnot. If the changes could be contained to logos_reuse_download that would be ideal.

Sorry for coming on a little strong, but the testing required to validate this change is much higher than the slight benefit of having it. I agree for robustness this can be a good change - just trying to limit the fallout/impact.

[sdwoodard]

Ok, that's reasonable. I can rework this so the changes are contained within logos_reuse_download.

There are two calls that would remain outside that protection. One skips logos_reuse_download when the recommended AppImage already exists. The other executes a cached updater binary before logos_reuse_download is called. Would you prefer those remain as they are or should those be updated?

They're uncommon cases, but an interrupted or corrupted cached file could potentially be used before any validation runs.

[ctrlaltf24]

I don't see the changes you're speaking about in the diff.

How about this: anything outside of logos_reuse_download have a comment rationalizing the change and try to keep the changes outside that scope to a minimum. Sound reasonable?