add pr comment fixes

Author: pthurlow

Cargo.lock

@@ -39,10 +39,21 @@ sqlformat = "0.5.0"
 sysinfo = { version = "0.38.4", default-features = false, features = ["system"] }
 self_update = { version = "0.42", default-features = false, features = ["rustls"] }
 lzma-rs = "0.3"
+tempfile = "3"
 
 [dev-dependencies]
 mockito = "1"
-tempfile = "3"
+
+[build-dependencies]
+toml = "0.8"
+
+# Project distribution config read by build.rs and exposed as compile-time
+# env vars (HOTDATA_HOMEBREW_FORMULA, ...) so source files don't hardcode
+# values that also live in the README / dist-workspace.toml.
+[package.metadata.hotdata]
+# Fully-qualified Homebrew install target. Matches the `brew install` line
+# in README.md; the trailing segment must match `formula` in dist-workspace.toml.
+homebrew_formula = "hotdata-dev/tap/cli"
 
 [package.metadata.release]
 pre-release-hook = ["git-cliff", "-o", "CHANGELOG.md", "--tag", "v{{version}}" ]

Cargo.toml

@@ -0,0 +1,30 @@
+// Reads `[package.metadata.hotdata]` from Cargo.toml and re-exports the
+// values as compile-time environment variables, so source files can read
+// distribution config (e.g. the Homebrew formula) via `env!()` without
+// duplicating strings that also live in README.md / dist-workspace.toml.
+
+use std::path::PathBuf;
+
+fn main() {
+    let manifest_dir = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap());
+    let manifest_path = manifest_dir.join("Cargo.toml");
+    println!("cargo:rerun-if-changed={}", manifest_path.display());
+
+    let raw = std::fs::read_to_string(&manifest_path)
+        .unwrap_or_else(|e| panic!("could not read {}: {e}", manifest_path.display()));
+    let parsed: toml::Value = toml::from_str(&raw)
+        .unwrap_or_else(|e| panic!("could not parse {}: {e}", manifest_path.display()));
+
+    let meta = parsed
+        .get("package")
+        .and_then(|p| p.get("metadata"))
+        .and_then(|m| m.get("hotdata"))
+        .unwrap_or_else(|| panic!("missing [package.metadata.hotdata] in Cargo.toml"));
+
+    let homebrew_formula = meta
+        .get("homebrew_formula")
+        .and_then(|v| v.as_str())
+        .unwrap_or_else(|| panic!("missing package.metadata.hotdata.homebrew_formula"));
+
+    println!("cargo:rustc-env=HOTDATA_HOMEBREW_FORMULA={homebrew_formula}");
+}

build.rs

@@ -9,6 +9,10 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH};
 const REPO_OWNER: &str = "hotdata-dev";
 const REPO_NAME: &str = "hotdata-cli";
 const CURRENT_VERSION: &str = env!("CARGO_PKG_VERSION");
+/// Fully-qualified Homebrew formula (e.g. `hotdata-dev/tap/cli`). Pulled from
+/// `[package.metadata.hotdata]` in Cargo.toml via build.rs so the README,
+/// dist-workspace.toml, and this binary all agree on the same target.
+const HOMEBREW_FORMULA: &str = env!("HOTDATA_HOMEBREW_FORMULA");
 const CHECK_INTERVAL_SECS: u64 = 86_400;
 const NETWORK_TIMEOUT_SECS: u64 = 5;
 
@@ -134,8 +138,8 @@ pub fn maybe_print_update_notice() {
         return;
     };
     let how = match detect_install_method() {
-        InstallMethod::Homebrew => "Run: brew upgrade hotdata",
-        InstallMethod::Other => "Run: hotdata update",
+        InstallMethod::Homebrew => format!("Run: brew upgrade {HOMEBREW_FORMULA}"),
+        InstallMethod::Other => "Run: hotdata update".to_string(),
     };
     eprintln!(
         "{}",
@@ -151,7 +155,7 @@ pub fn run_update() {
 
     if detect_install_method() == InstallMethod::Homebrew {
         println!("hotdata was installed via Homebrew. Update with:");
-        println!("  {}", "brew upgrade hotdata".cyan());
+        println!("  {}", format!("brew upgrade {HOMEBREW_FORMULA}").cyan());
         return;
     }
 
@@ -222,23 +226,23 @@ fn perform_update(version: &Version) -> Result<(), String> {
     lzma_rs::xz_decompress(&mut std::io::Cursor::new(&xz_bytes[..]), &mut tar_bytes)
         .map_err(|e| format!("xz decompress: {e}"))?;
 
-    let tmp_dir = std::env::temp_dir().join(format!("hotdata-update-{}", std::process::id()));
-    if tmp_dir.exists() {
-        let _ = fs::remove_dir_all(&tmp_dir);
-    }
-    fs::create_dir_all(&tmp_dir).map_err(|e| format!("creating temp dir: {e}"))?;
+    // `tempfile::TempDir` creates a randomly-named directory with 0700
+    // permissions and removes it on drop. The random suffix prevents a
+    // local attacker on a shared system from pre-planting a symlink at a
+    // predictable path and redirecting the extraction to a directory
+    // they control.
+    let tmp_dir = tempfile::TempDir::new().map_err(|e| format!("creating temp dir: {e}"))?;
 
     let mut archive = tar::Archive::new(std::io::Cursor::new(&tar_bytes[..]));
     archive
-        .unpack(&tmp_dir)
+        .unpack(tmp_dir.path())
         .map_err(|e| format!("extract tar: {e}"))?;
 
     // cargo-dist lays out the tarball as `<asset-stem>/hotdata` (the binary
     // sits at the top of a single directory matching the asset name without
     // its extension).
-    let new_binary = tmp_dir.join(&asset_stem).join("hotdata");
+    let new_binary = tmp_dir.path().join(&asset_stem).join("hotdata");
     if !new_binary.exists() {
-        let _ = fs::remove_dir_all(&tmp_dir);
         return Err(format!(
             "binary not found in archive at {}",
             new_binary.display()
@@ -253,13 +257,10 @@ fn perform_update(version: &Version) -> Result<(), String> {
     let backup = current_exe.with_extension("old");
     let _ = fs::remove_file(&backup);
 
-    let result = self_update::Move::from_source(&new_binary)
+    self_update::Move::from_source(&new_binary)
         .replace_using_temp(&backup)
         .to_dest(&current_exe)
-        .map_err(|e| format!("replacing binary: {e}"));
-
-    let _ = fs::remove_dir_all(&tmp_dir);
-    result
+        .map_err(|e| format!("replacing binary: {e}"))
 }
 
 #[cfg(test)]