fix race between concurrent buckle invocations

buckle was writing direct to prelude hash, so two concurrent buckle invocations could race and thus read an incomplete entry

fix it by doing the has before the binary, so its impossible to see a partially written prelude hash

the binary itself is protected by temp file and rename approach from earlier version of this PR which was merged previously

src/main.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Error};
+use anyhow::{anyhow, Context, Error};
 use ini::Ini;
 use once_cell::sync::OnceCell;
 use serde::{Deserialize, Serialize};
@@ -155,52 +155,53 @@ fn get_arch() -> Result<&'static str, Error> {
 
 fn download_http(version: String, output_dir: &Path) -> Result<PathBuf, Error> {
     let releases = get_releases(output_dir)?;
-    let mut buck2_path = output_dir.to_path_buf();
-
+    let mut dir_path = output_dir.to_path_buf();
     let mut release_found = false;
     for release in releases {
         if release.tag_name == version {
-            buck2_path.push(release.target_commitish);
+            dir_path.push(release.target_commitish);
             release_found = true;
         }
     }
     if !release_found {
         return Err(anyhow!("{version} was not available. Please check '{BUCK_RELEASE_URL}' for available releases."));
     }
 
-    // Path to directory that caches buck
-    let dir_path = buck2_path.clone();
-    if dir_path.exists() {
-        // Already downloaded
+    let binary_path: PathBuf = [&dir_path, Path::new("buck2")].iter().collect();
+    if binary_path.exists() {
+        // unpacked binary already present, so do nothing
         return Ok(dir_path);
     }
 
-    buck2_path.push("buck2");
-    if let Some(prefix) = buck2_path.parent() {
-        fs::create_dir_all(prefix)?;
+    // Create the release directory if it doesn't exist
+    fs::create_dir_all(&dir_path).with_context(|| anyhow!("problem creating {:?}", dir_path))?;
+
+    {
+        // Fetch the prelude hash and store it, do this before the binary so we don't see a partial hash
+        // We do this as the complete executable is atomic via tmp_file rename
+        let prelude_path: PathBuf = [&dir_path, Path::new("prelude_hash")].iter().collect();
+        let resp = reqwest::blocking::get(format!("{BASE_URL}/{version}/prelude_hash"))?;
+        let mut prelude_hash = File::create(prelude_path)?;
+        prelude_hash.write_all(&resp.bytes()?)?;
+        prelude_hash.flush()?;
     }
 
     // Fetch the buck2 archive, decode it, make it executable
-    let mut tmp_buck2_bin = NamedTempFile::new_in(dir_path.clone())?;
+    let mut tmp_file = NamedTempFile::new_in(&dir_path)?;
     let arch = get_arch()?;
     eprintln!("buckle: fetching buck2 {version}");
     let resp = reqwest::blocking::get(format!("{BASE_URL}/{version}/buck2-{arch}.zst"))?;
-    zstd::stream::copy_decode(resp, &tmp_buck2_bin).unwrap();
-    tmp_buck2_bin.flush()?;
+    zstd::stream::copy_decode(resp, &tmp_file)?;
+    tmp_file.flush()?;
     #[cfg(unix)]
     {
         let permissions = fs::Permissions::from_mode(0o755);
-        fs::set_permissions(&tmp_buck2_bin, permissions)?;
+        fs::set_permissions(&tmp_file, permissions)
+            .with_context(|| anyhow!("problem setting permissions on {:?}", tmp_file))?;
     }
-    fs::rename(tmp_buck2_bin.path(), &buck2_path)?;
-
-    // Also fetch the prelude hash and store it
-    let mut prelude_path = dir_path.clone();
-    prelude_path.push("prelude_hash");
-    let resp = reqwest::blocking::get(format!("{BASE_URL}/{version}/prelude_hash"))?;
-    let mut prelude_hash = File::create(prelude_path)?;
-    prelude_hash.write_all(&resp.bytes()?)?;
-    prelude_hash.flush()?;
+    // only move to final binary_path once fully written and stable
+    fs::rename(tmp_file.path(), &binary_path)
+        .with_context(|| anyhow!("problem renaming {:?} to {:?}", tmp_file, binary_path))?;
 
     Ok(dir_path)
 }