diff --git a/crates/transcription/src/model_manager.rs b/crates/transcription/src/model_manager.rs index aa21d09..0c339c5 100644 --- a/crates/transcription/src/model_manager.rs +++ b/crates/transcription/src/model_manager.rs @@ -168,6 +168,12 @@ async fn download_file( // full file on top of our partial bytes (which would produce a // corrupt result), restart cleanly. This mirrors the kon-llm // ResumeUnsupported branch — item #8 of the brief. + // + // For the non-resume path, we still have to validate the status: + // reqwest does not error on 4xx/5xx by default, so without this + // check a 404 or 500 would be streamed into `.part` and renamed + // over the destination as if the download succeeded + // (2026-04-22 review MAJOR). let actually_resuming = if resuming { match response.status().as_u16() { 206 => true, @@ -183,6 +189,13 @@ async fn download_file( } } } else { + if !response.status().is_success() { + return Err(KonError::DownloadFailed(format!( + "download returned HTTP {} for {}", + response.status(), + file.filename + ))); + } false }; @@ -466,6 +479,63 @@ mod tests { assert!(!part.exists(), ".part → dest rename must run after restart"); } + /// Always returns HTTP 500 with a short error body. Used to verify + /// the non-resume download path validates status codes rather than + /// writing error bodies into `.part` and renaming them over the + /// destination. + async fn spawn_500_server() -> std::net::SocketAddr { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut buf = vec![0u8; 2048]; + let _ = socket.read(&mut buf).await.unwrap(); + let body = b"internal error"; + let response = format!( + "HTTP/1.1 500 Internal Server Error\r\n\ + Content-Length: {}\r\n\r\n", + body.len() + ); + socket.write_all(response.as_bytes()).await.unwrap(); + socket.write_all(body).await.unwrap(); + }); + + addr + } + + #[tokio::test] + async fn download_file_rejects_5xx_on_non_resume_path() { + // Regression for the 2026-04-22 review: reqwest does not + // auto-error on 4xx/5xx, and the non-resume branch previously + // streamed any status' body into `.part` and renamed it over + // the destination. + let addr = spawn_500_server().await; + + let dir = tempdir().unwrap(); + let dest = dir.path().join("fixture.bin"); + let part = dest.with_extension("bin.part"); + + let file = ModelFile { + filename: leak(dest.file_name().unwrap().to_string_lossy().into_owned()), + url: leak(format!("http://{addr}/fixture.bin")), + size: kon_core::types::Megabytes(0), + sha256: None, + }; + let id = ModelId::new("test-fixture"); + + let err = download_file(&file, &dest, &id, &|_| ()) + .await + .expect_err("5xx must fail"); + let msg = err.to_string(); + assert!( + msg.contains("HTTP 500"), + "error should name the HTTP status, got: {msg}" + ); + assert!(!dest.exists(), "5xx must not leave a destination file"); + assert!(!part.exists(), "5xx must not leave a .part file"); + } + #[tokio::test] async fn download_file_fails_on_sha_mismatch_and_cleans_part_file() { let body = b"speech-to-text fixture body".to_vec();