diff --git a/CLAUDE.md b/CLAUDE.md index 0ab594bf..c90dc54e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,6 +1,7 @@ The `cli/` directory is a Cargo workspace. - `crates/hm/` — the `hm` binary (today's CLI body). +- `crates/hm-util/` — shared OS and filesystem utilities. - `crates/hm-plugin-protocol/` — wire types (serde structs only). - `crates/hm-plugin-sdk/` — authoring SDK for plugin writers. - `crates/hm-fixtures/` — test-only WASM plugins; compiled to diff --git a/Cargo.lock b/Cargo.lock index 43ee1aa7..67cd0e65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1542,7 +1542,6 @@ dependencies = [ "comfy-table", "console 0.15.11", "dialoguer", - "dirs", "extism", "flate2", "fs2", @@ -1550,6 +1549,7 @@ dependencies = [ "futures-util", "hex", "hm-plugin-protocol", + "hm-util", "ignore", "indicatif", "nix", @@ -1711,6 +1711,16 @@ dependencies = [ "serde_json", ] +[[package]] +name = "hm-util" +version = "0.0.0-dev" +dependencies = [ + "dirs", + "tempfile", + "tokio", + "windows", +] + [[package]] name = "home" version = "0.5.12" @@ -5010,6 +5020,27 @@ dependencies = [ "wasmtime-internal-math", ] +[[package]] +name = "windows" +version = "0.62.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "527fadee13e0c05939a6a05d5bd6eec6cd2e3dbd648b9f8e447c6518133d8580" +dependencies = [ + "windows-collections", + "windows-core", + "windows-future", + "windows-numerics", +] + +[[package]] +name = "windows-collections" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b2d95af1a8a14a3c7367e1ed4fc9c20e0a26e79551b1454d72583c97cc6610" +dependencies = [ + "windows-core", +] + [[package]] name = "windows-core" version = "0.62.2" @@ -5023,6 +5054,17 @@ dependencies = [ "windows-strings", ] +[[package]] +name = "windows-future" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1d6f90251fe18a279739e78025bd6ddc52a7e22f921070ccdc67dde84c605cb" +dependencies = [ + "windows-core", + "windows-link", + "windows-threading", +] + [[package]] name = "windows-implement" version = "0.60.2" @@ -5051,6 +5093,16 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-numerics" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e2e40844ac143cdb44aead537bbf727de9b044e107a0f1220392177d15b0f26" +dependencies = [ + "windows-core", + "windows-link", +] + [[package]] name = "windows-result" version = "0.4.1" @@ -5138,6 +5190,15 @@ dependencies = [ "windows_x86_64_msvc 0.53.1", ] +[[package]] +name = "windows-threading" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3949bd5b99cafdf1c7ca86b43ca564028dfe27d66958f2470940f73d86d75b37" +dependencies = [ + "windows-link", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" diff --git a/Cargo.toml b/Cargo.toml index f49f751d..5da02aa2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,11 +9,13 @@ members = [ "crates/hm-plugin-output-json", "crates/hm-plugin-cloud", "crates/hm-fixtures", + "crates/hm-util", ] default-members = [ "crates/hm", "crates/hm-plugin-protocol", "crates/hm-plugin-sdk", + "crates/hm-util", ] [workspace.package] @@ -24,6 +26,7 @@ repository = "https://github.com/harmont-dev/harmont-cli" [workspace.dependencies] hm-plugin-protocol = { path = "crates/hm-plugin-protocol", version = "0.0.0-dev" } hm-plugin-sdk = { path = "crates/hm-plugin-sdk", version = "0.0.0-dev" } +hm-util = { path = "crates/hm-util", version = "0.0.0-dev" } serde = { version = "1", features = ["derive"] } serde_json = "1" schemars = { version = "0.8", features = ["preserve_order", "semver", "uuid1", "chrono"] } @@ -54,6 +57,9 @@ cargo_common_metadata = "allow" # schemars 0.8 pulls older indexmap / wit-bindgen transitively; we # can't fix without bumping schemars. multiple_crate_versions = "allow" +# `pub(crate)` inside a non-pub module triggers this, but `unreachable_pub` +# already enforces the same invariant from the rustc side. +redundant_pub_crate = "allow" dbg_macro = "deny" todo = "deny" unimplemented = "deny" diff --git a/crates/hm-util/Cargo.toml b/crates/hm-util/Cargo.toml new file mode 100644 index 00000000..01a9765c --- /dev/null +++ b/crates/hm-util/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "hm-util" +version = "0.0.0-dev" +edition.workspace = true +license.workspace = true +repository.workspace = true +description = "Shared OS and filesystem utilities for Harmont crates." + +[dependencies] +dirs = "6" +tokio = { version = "1", features = ["rt", "fs"] } + +[target.'cfg(windows)'.dependencies.windows] +version = "0.62" +features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", +] + +[dev-dependencies] +tempfile = "3" +tokio = { version = "1", features = ["full", "test-util"] } + +[lints] +workspace = true diff --git a/crates/hm-util/src/dirs.rs b/crates/hm-util/src/dirs.rs new file mode 100644 index 00000000..4838e3f0 --- /dev/null +++ b/crates/hm-util/src/dirs.rs @@ -0,0 +1,62 @@ +//! Harmont-specific directory resolution. +//! +//! Every directory accessor in this module returns a Harmont-namespaced +//! path. Raw platform primitives (`home_dir`, `config_dir`) live in +//! `os::dirs` and are **not** re-exported — callers outside `hm-util` +//! should never need them. + +#![allow(clippy::must_use_candidate)] + +use std::path::PathBuf; + +use crate::os::dirs as platform; + +/// `~/.harmont/` — CLI config home (config.toml, credentials.toml). +pub fn harmont_config_dir() -> Option { + platform::home_dir().map(|h| h.join(".harmont")) +} + +/// `/harmont/` — XDG-aware data root (plugins, state). +pub fn harmont_data_dir() -> Option { + platform::config_dir().map(|c| c.join("harmont")) +} + +/// `/harmont/plugins/` — user-global plugin directory. +pub fn harmont_plugins_dir() -> Option { + harmont_data_dir().map(|d| d.join("plugins")) +} + +/// `/harmont/state/` — per-plugin persistent KV state. +pub fn harmont_plugin_state_dir() -> Option { + harmont_data_dir().map(|d| d.join("state")) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + #[test] + fn harmont_config_dir_under_home() { + let p = harmont_config_dir().unwrap(); + assert!(p.ends_with(".harmont")); + } + + #[test] + fn harmont_data_dir_under_config() { + let p = harmont_data_dir().unwrap(); + assert!(p.ends_with("harmont")); + } + + #[test] + fn harmont_plugins_dir_resolves() { + let p = harmont_plugins_dir().unwrap(); + assert!(p.ends_with("harmont/plugins")); + } + + #[test] + fn harmont_plugin_state_dir_resolves() { + let p = harmont_plugin_state_dir().unwrap(); + assert!(p.ends_with("harmont/state")); + } +} diff --git a/crates/hm-util/src/lib.rs b/crates/hm-util/src/lib.rs new file mode 100644 index 00000000..56ea3f30 --- /dev/null +++ b/crates/hm-util/src/lib.rs @@ -0,0 +1,2 @@ +pub mod os; +pub mod dirs; diff --git a/crates/hm-util/src/os/dirs.rs b/crates/hm-util/src/os/dirs.rs new file mode 100644 index 00000000..2d82f295 --- /dev/null +++ b/crates/hm-util/src/os/dirs.rs @@ -0,0 +1,14 @@ +//! Raw platform directory primitives. +//! +//! This module is `pub(crate)` — external callers must use +//! [`crate::dirs`] which provides Harmont-specific accessors. + +use std::path::PathBuf; + +pub(crate) fn home_dir() -> Option { + dirs::home_dir() +} + +pub(crate) fn config_dir() -> Option { + dirs::config_dir() +} diff --git a/crates/hm-util/src/os/fs.rs b/crates/hm-util/src/os/fs.rs new file mode 100644 index 00000000..fec7a308 --- /dev/null +++ b/crates/hm-util/src/os/fs.rs @@ -0,0 +1,232 @@ +//! Filesystem helpers. +//! +//! The main entry point is [`write_atomic_restricted`]. A synchronous +//! wrapper is available at [`blocking::write_atomic_restricted`] for +//! callers that run inside a tokio runtime but cannot use async +//! (e.g. extism `host_fn` callbacks). +//! +//! Both guarantee that readers observe either the full old contents or +//! the full new contents — never a truncated file — and that Unix +//! file/directory modes are set atomically with creation. + +use std::io; +use std::path::Path; + +/// Write `contents` to `path` atomically with `file_mode`, ensuring the +/// parent directory exists and is set to `dir_mode`. +/// +/// # Errors +/// +/// Returns an error if `path` has no parent or no file-name component, +/// the parent directory cannot be created or chmod'd to `dir_mode`, the +/// tempfile cannot be opened with `file_mode` or written, or the final +/// `rename` over `path` fails. +pub async fn write_atomic_restricted( + path: impl AsRef, + contents: impl AsRef<[u8]>, + file_mode: u32, + dir_mode: u32, +) -> io::Result<()> { + let path = path.as_ref().to_owned(); + let contents = contents.as_ref().to_vec(); + + let parent = path + .parent() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} has no parent directory", path.display()), + ) + })? + .to_owned(); + + create_dir_with_mode(&parent, dir_mode).await?; + + let file_name = path + .file_name() + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("{} has no file name", path.display()), + ) + })? + .to_os_string(); + let mut tmp_name = file_name; + tmp_name.push(format!(".tmp.{}", std::process::id())); + let tmp_path = parent.join(&tmp_name); + + write_file_with_mode(&tmp_path, &contents, file_mode).await?; + + let rename_result = atomic_rename_over(&tmp_path, &path).await; + if rename_result.is_err() { + let _ = tokio::fs::remove_file(&tmp_path).await; + } + rename_result +} + +/// Atomically replace `to` with `from`. +/// +/// On Unix this delegates to [`tokio::fs::rename`] (`rename(2)` — atomic +/// by POSIX guarantee). On Windows this uses `ReplaceFileW` (preserves +/// ACLs and alternate data streams) when the target exists, falling back +/// to `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING` for first-write. +/// +/// # Errors +/// +/// Returns an error if the rename fails (permission denied, cross-device, +/// source missing, etc.). +pub async fn atomic_rename_over( + from: impl AsRef, + to: impl AsRef, +) -> io::Result<()> { + #[cfg(unix)] + { + tokio::fs::rename(from.as_ref(), to.as_ref()).await + } + #[cfg(windows)] + { + fn atomic_rename_over_impl(from: &Path, to: &Path) -> io::Result<()> { + use windows::core::HSTRING; + use windows::Win32::Storage::FileSystem::{ + MoveFileExW, ReplaceFileW, + MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, + REPLACEFILE_IGNORE_MERGE_ERRORS, + }; + + let from_w = HSTRING::from(from.as_os_str()); + let to_w = HSTRING::from(to.as_os_str()); + + if to.exists() { + let result = unsafe { + ReplaceFileW( + &to_w, + &from_w, + windows::core::PCWSTR::null(), + REPLACEFILE_IGNORE_MERGE_ERRORS, + None, + None, + ) + }; + return result.map_err(|e| io::Error::new(io::ErrorKind::Other, e)); + } + + let result = unsafe { + MoveFileExW( + &from_w, + &to_w, + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + }; + result.map_err(|e| io::Error::new(io::ErrorKind::Other, e)) + } + + let from = from.as_ref().to_owned(); + let to = to.as_ref().to_owned(); + tokio::task::spawn_blocking(move || atomic_rename_over_impl(&from, &to)) + .await + .map_err(io::Error::other)? + } +} + +/// Remove a file if it exists; silently return `Ok(())` if it does not. +/// +/// # Errors +/// +/// Returns an error if `remove_file` fails for any reason other than +/// `NotFound`. +pub async fn remove_file_if_exists(path: impl AsRef) -> io::Result<()> { + match tokio::fs::remove_file(path).await { + Ok(()) => Ok(()), + Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e), + } +} + +#[cfg(unix)] +async fn create_dir_with_mode(dir: &Path, mode: u32) -> io::Result<()> { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + match tokio::fs::metadata(dir).await { + Ok(meta) => { + let current = meta.permissions().mode() & 0o777; + if current != mode { + tokio::fs::set_permissions(dir, std::fs::Permissions::from_mode(mode)).await?; + } + } + Err(e) if e.kind() == io::ErrorKind::NotFound => { + let mut builder = tokio::fs::DirBuilder::new(); + builder.recursive(true).mode(mode); + builder.create(dir).await?; + } + Err(e) => return Err(e), + } + } + + #[cfg(windows)] + { + tokio::fs::create_dir_all(dir).await + } + Ok(()) +} + +async fn write_file_with_mode(path: &Path, contents: &[u8], mode: u32) -> io::Result<()> { + #[cfg(unix)] + { + use tokio::io::AsyncWriteExt; + let mut opts = tokio::fs::OpenOptions::new(); + opts.write(true).create(true).truncate(true).mode(mode); + let mut f = opts.open(path).await?; + f.write_all(contents).await?; + f.sync_all().await?; + } + + #[cfg(windows)] + { + tokio::fs::write(path, contents).await + } + + Ok(()) +} + + +/// Synchronous wrappers that shell out to the async API via +/// `tokio::task::block_in_place`. Safe to call from sync contexts +/// that run inside a tokio runtime (e.g. extism `host_fn` callbacks). +pub mod blocking { + use std::io; + use std::path::Path; + + fn block_on>>(f: F) -> io::Result<()> { + tokio::task::block_in_place(|| tokio::runtime::Handle::current().block_on(f)) + } + + /// Blocking counterpart of [`super::write_atomic_restricted`]. + /// + /// See the [module-level documentation](super) for semantics. + /// + /// # Errors + /// + /// Returns an error if `path` has no parent or no file-name component, + /// the parent directory cannot be created or chmod'd to `dir_mode`, the + /// tempfile cannot be opened with `file_mode` or written, or the final + /// `rename` over `path` fails. + pub fn write_atomic_restricted( + path: impl AsRef, + contents: impl AsRef<[u8]>, + file_mode: u32, + dir_mode: u32, + ) -> io::Result<()> { + block_on(super::write_atomic_restricted(path, contents, file_mode, dir_mode)) + } + + /// Blocking counterpart of [`super::remove_file_if_exists`]. + /// + /// # Errors + /// + /// Returns an error if `remove_file` fails for any reason other than + /// `NotFound`. + pub fn remove_if_exists(path: impl AsRef) -> io::Result<()> { + block_on(super::remove_file_if_exists(path)) + } +} diff --git a/crates/hm-util/src/os/mod.rs b/crates/hm-util/src/os/mod.rs new file mode 100644 index 00000000..0bc7829b --- /dev/null +++ b/crates/hm-util/src/os/mod.rs @@ -0,0 +1,2 @@ +pub(crate) mod dirs; +pub mod fs; diff --git a/crates/hm/CLAUDE.md b/crates/hm/CLAUDE.md index e9b1a880..51f28a50 100644 --- a/crates/hm/CLAUDE.md +++ b/crates/hm/CLAUDE.md @@ -18,7 +18,7 @@ archive once into memory (`archive.rs` + `source.rs`), and drives the Docker daemon via the Bollard wrapper (`docker_client.rs`, exposed to step plugins through `docker_host_fns.rs`). -- Owns run-wide cancellation (`cancel.rs`) and shared mutable state +- Owns run-wide cancellation (`tokio_util::sync::CancellationToken`) and shared mutable state (`state.rs`) so step plugins can coordinate without reaching across module boundaries. diff --git a/crates/hm/Cargo.toml b/crates/hm/Cargo.toml index c8e9912e..45ec7da3 100644 --- a/crates/hm/Cargo.toml +++ b/crates/hm/Cargo.toml @@ -46,7 +46,6 @@ tar = "0.4" flate2 = "1" fs2 = "0.4" ignore = "0.4" -dirs = "6" tempfile = "3" anyhow = "1" thiserror = "2" @@ -69,6 +68,7 @@ bollard = "0.18" which = "6" extism = { workspace = true } hm-plugin-protocol = { workspace = true } +hm-util = { workspace = true } schemars = { workspace = true } semver = { workspace = true } once_cell = "1" diff --git a/crates/hm/src/config.rs b/crates/hm/src/config.rs index 36da7439..5a83422b 100644 --- a/crates/hm/src/config.rs +++ b/crates/hm/src/config.rs @@ -12,8 +12,8 @@ const DEFAULT_API_URL: &str = "https://api.harmont.dev"; /// (the `dirs` crate's platform-specific lookup fails — typically only /// happens in restrictive sandboxes with no `HOME` / passwd entry). pub fn user_config_dir() -> Result { - let home = dirs::home_dir().context("could not determine home directory")?; - Ok(home.join(".harmont")) + hm_util::dirs::harmont_config_dir() + .context("could not determine home directory") } /// User preferences stored alongside the config. @@ -81,7 +81,7 @@ impl Config { pub fn save(&self) -> Result<()> { let path = Self::path()?; let serialized = toml::to_string_pretty(self).context("serializing config")?; - crate::fs_util::write_atomic_restricted(&path, serialized.as_bytes(), 0o644, 0o700) + hm_util::os::fs::blocking::write_atomic_restricted(&path, serialized.as_bytes(), 0o644, 0o700) .with_context(|| format!("writing {}", path.display()))?; Ok(()) } diff --git a/crates/hm/src/creds_store.rs b/crates/hm/src/creds_store.rs index d994f729..d6e45d03 100644 --- a/crates/hm/src/creds_store.rs +++ b/crates/hm/src/creds_store.rs @@ -1,7 +1,7 @@ //! File-backed credential store at `~/.harmont/credentials.toml`. //! //! Replaces the OS keyring as the sole backend. The file is written with -//! mode 0o600 (parent dir 0o700) via [`crate::fs_util::write_atomic_restricted`]. +//! mode 0o600 (parent dir 0o700) via [`hm_util::os::fs::blocking::write_atomic_restricted`]. //! Keyed by `(service, account)` to match the host-fn ABI plugins use. use anyhow::{Context, Result}; @@ -32,7 +32,7 @@ fn load() -> CredentialFile { fn save(file: &CredentialFile) -> Result<()> { let p = path()?; let serialized = toml::to_string_pretty(file).context("serializing credentials")?; - crate::fs_util::write_atomic_restricted(&p, serialized.as_bytes(), 0o600, 0o700) + hm_util::os::fs::blocking::write_atomic_restricted(&p, serialized.as_bytes(), 0o600, 0o700) .with_context(|| format!("writing {}", p.display()))?; Ok(()) } @@ -92,8 +92,8 @@ mod tests { } } - #[test] - fn round_trip() { + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn round_trip() { with_home(|| { assert_eq!(get("svc", "acct"), None); set("svc", "acct", "shh"); diff --git a/crates/hm/src/fs_util.rs b/crates/hm/src/fs_util.rs deleted file mode 100644 index 40523afa..00000000 --- a/crates/hm/src/fs_util.rs +++ /dev/null @@ -1,192 +0,0 @@ -//! Small filesystem helpers for atomic, permission-restricted writes. -//! -//! The main entry point is [`write_atomic_restricted`]. It is used by -//! [`crate::creds_store`] (file-backed credential store) and by -//! `Config::save`, both of which write into `~/.harmont/` -//! (see `config::user_config_dir`). - -use anyhow::{Context, Result}; -use std::path::Path; - -/// Write `contents` to `path` atomically with `file_mode`, ensuring the -/// parent directory exists and is set to `dir_mode`. -/// -/// On Unix the target file is created with `OpenOptions::mode(file_mode)` -/// before any bytes are written, closing the TOCTOU window that -/// `fs::write(…)` + `set_permissions(…)` opens. The parent directory is -/// created with `DirBuilder::mode(dir_mode)`; if the directory already -/// exists with a looser mode, it is tightened. -/// -/// On non-Unix platforms the mode arguments are ignored and the function -/// falls back to `std::fs::create_dir_all` + tempfile + rename. -/// -/// Atomicity: contents are written to a sibling tempfile and then -/// `rename`d over `path`, so readers always observe either the full old -/// contents or the full new contents — never a truncated file. -/// -/// # Errors -/// -/// Returns an error if `path` has no parent or no file-name component, -/// the parent directory cannot be created or chmod'd to `dir_mode`, the -/// tempfile cannot be opened with `file_mode` or written, or the final -/// `rename` over `path` fails. The tempfile is cleaned up on rename -/// failure so secret material doesn't linger. -pub fn write_atomic_restricted( - path: &Path, - contents: &[u8], - file_mode: u32, - dir_mode: u32, -) -> Result<()> { - let parent = path - .parent() - .with_context(|| format!("{} has no parent directory", path.display()))?; - - create_dir_with_mode(parent, dir_mode) - .with_context(|| format!("creating {}", parent.display()))?; - - // Tempfile name is unique per process (pid) and per target filename, - // which is sufficient because write_atomic_restricted is never called - // concurrently on the same target from within a single process. - let file_name = path - .file_name() - .with_context(|| format!("{} has no file name", path.display()))? - .to_os_string(); - let mut tmp_name = file_name; - tmp_name.push(format!(".tmp.{}", std::process::id())); - let tmp_path = parent.join(&tmp_name); - - write_file_with_mode(&tmp_path, contents, file_mode) - .with_context(|| format!("writing {}", tmp_path.display()))?; - - let persist_result = std::fs::rename(&tmp_path, path) - .with_context(|| format!("renaming {} -> {}", tmp_path.display(), path.display())); - - if persist_result.is_err() { - // Clean up the orphaned tempfile — it may contain secret material - // and we don't want it sitting at an unexpected path. - let _ = std::fs::remove_file(&tmp_path); - } - persist_result?; - - Ok(()) -} - -/// Remove a file if it exists; silently return `Ok(())` if it does not. -/// -/// # Errors -/// -/// Returns an error if `remove_file` fails for any reason other than -/// `NotFound` (typically permission denied or the path being a -/// non-empty directory). -pub fn remove_if_exists(path: &Path) -> Result<()> { - match std::fs::remove_file(path) { - Ok(()) => Ok(()), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), - Err(e) => Err(e).with_context(|| format!("removing {}", path.display())), - } -} - -#[cfg(unix)] -fn create_dir_with_mode(dir: &Path, mode: u32) -> std::io::Result<()> { - use std::os::unix::fs::{DirBuilderExt, PermissionsExt}; - if dir.exists() { - let current = std::fs::metadata(dir)?.permissions().mode() & 0o777; - if current != mode { - std::fs::set_permissions(dir, std::fs::Permissions::from_mode(mode))?; - } - } else { - std::fs::DirBuilder::new() - .recursive(true) - .mode(mode) - .create(dir)?; - } - Ok(()) -} - -#[cfg(not(unix))] -fn create_dir_with_mode(dir: &Path, _mode: u32) -> std::io::Result<()> { - std::fs::create_dir_all(dir) -} - -#[cfg(unix)] -fn write_file_with_mode(path: &Path, contents: &[u8], mode: u32) -> std::io::Result<()> { - use std::io::Write; - use std::os::unix::fs::OpenOptionsExt; - let mut f = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .mode(mode) - .open(path)?; - f.write_all(contents)?; - f.sync_all()?; - Ok(()) -} - -#[cfg(not(unix))] -fn write_file_with_mode(path: &Path, contents: &[u8], _mode: u32) -> std::io::Result<()> { - std::fs::write(path, contents) -} - -#[cfg(all(test, unix))] -#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] -mod tests { - use super::*; - use std::os::unix::fs::PermissionsExt; - - #[test] - fn writes_file_and_dir_with_requested_modes() { - let tmp = tempfile::tempdir().unwrap(); - let target = tmp.path().join("sub").join("creds"); - write_atomic_restricted(&target, b"hello", 0o600, 0o700).unwrap(); - - assert_eq!(std::fs::read(&target).unwrap(), b"hello"); - let file_mode = std::fs::metadata(&target).unwrap().permissions().mode() & 0o777; - let dir_mode = std::fs::metadata(target.parent().unwrap()) - .unwrap() - .permissions() - .mode() - & 0o777; - assert_eq!( - file_mode, 0o600, - "file mode must be 0o600, got {file_mode:o}" - ); - assert_eq!(dir_mode, 0o700, "dir mode must be 0o700, got {dir_mode:o}"); - } - - #[test] - fn overwrites_existing_file_preserving_mode() { - let tmp = tempfile::tempdir().unwrap(); - let target = tmp.path().join("creds"); - write_atomic_restricted(&target, b"v1", 0o600, 0o700).unwrap(); - write_atomic_restricted(&target, b"v2", 0o600, 0o700).unwrap(); - - assert_eq!(std::fs::read(&target).unwrap(), b"v2"); - let mode = std::fs::metadata(&target).unwrap().permissions().mode() & 0o777; - assert_eq!(mode, 0o600); - } - - #[test] - fn tightens_existing_dir_with_looser_mode() { - let tmp = tempfile::tempdir().unwrap(); - let dir = tmp.path().join("loose"); - std::fs::create_dir(&dir).unwrap(); - std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).unwrap(); - - let target = dir.join("creds"); - write_atomic_restricted(&target, b"x", 0o600, 0o700).unwrap(); - - let dir_mode = std::fs::metadata(&dir).unwrap().permissions().mode() & 0o777; - assert_eq!(dir_mode, 0o700); - } - - #[test] - fn remove_if_exists_is_idempotent() { - let tmp = tempfile::tempdir().unwrap(); - let target = tmp.path().join("nothing"); - remove_if_exists(&target).unwrap(); - std::fs::write(&target, "x").unwrap(); - remove_if_exists(&target).unwrap(); - assert!(!target.exists()); - } -} diff --git a/crates/hm/src/lib.rs b/crates/hm/src/lib.rs index 731a5900..9ae6fcaa 100644 --- a/crates/hm/src/lib.rs +++ b/crates/hm/src/lib.rs @@ -2,6 +2,11 @@ clippy::multiple_crate_versions, reason = "transitive dependency version conflicts in rand/windows-sys/thiserror chains; not fixable without upstream updates" )] +// The `dirs` crate must NOT be added as a direct dependency of this +// crate. All directory resolution goes through `hm_util::dirs`, which +// owns the `dirs` dependency and provides both platform primitives and +// Harmont-specific discovery. Adding `dirs` here would bypass that +// single source of truth. #[allow( clippy::print_stdout, @@ -14,7 +19,6 @@ pub mod config; pub mod context; pub mod creds_store; pub mod error; -pub mod fs_util; pub mod orchestrator; pub mod output; pub mod plugin; diff --git a/crates/hm/src/orchestrator/cancel.rs b/crates/hm/src/orchestrator/cancel.rs deleted file mode 100644 index 7a34ec13..00000000 --- a/crates/hm/src/orchestrator/cancel.rs +++ /dev/null @@ -1,54 +0,0 @@ -//! Cancellation atomic. -//! -//! Ctrl-C handlers and orchestrator failure paths flip the atomic; -//! the `hm_should_cancel` host fn reports its state to plugins; -//! plugins poll between long-running operations and unwind quickly. - -use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; - -#[derive(Debug, Clone, Default)] -pub struct CancellationToken { - inner: Arc, -} - -impl CancellationToken { - #[must_use] - pub fn new() -> Self { - Self::default() - } - - pub fn cancel(&self) { - self.inner.store(true, Ordering::SeqCst); - } - - #[must_use] - pub fn is_cancelled(&self) -> bool { - self.inner.load(Ordering::SeqCst) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn default_is_not_cancelled() { - assert!(!CancellationToken::new().is_cancelled()); - } - - #[test] - fn cancel_persists() { - let t = CancellationToken::new(); - t.cancel(); - assert!(t.is_cancelled()); - } - - #[test] - fn cancel_is_clone_shared() { - let t = CancellationToken::new(); - let u = t.clone(); - t.cancel(); - assert!(u.is_cancelled()); - } -} diff --git a/crates/hm/src/orchestrator/docker_host_fns.rs b/crates/hm/src/orchestrator/docker_host_fns.rs index 1375fc4f..7e7106f4 100644 --- a/crates/hm/src/orchestrator/docker_host_fns.rs +++ b/crates/hm/src/orchestrator/docker_host_fns.rs @@ -160,14 +160,8 @@ pub(crate) async fn exec_impl(args: DockerExecArgs) -> Result { i32::try_from(rc).context("docker exit code out of i32 range") } -async fn wait_cancel(cancel: &crate::orchestrator::cancel::CancellationToken) { - // Poll the atomic every 50ms. Cheap; never wakes a thread early. - loop { - if cancel.is_cancelled() { - return; - } - tokio::time::sleep(std::time::Duration::from_millis(50)).await; - } +async fn wait_cancel(cancel: &tokio_util::sync::CancellationToken) { + cancel.cancelled().await; } pub(crate) async fn commit_impl(args: DockerCommitArgs) -> Result { diff --git a/crates/hm/src/orchestrator/mod.rs b/crates/hm/src/orchestrator/mod.rs index a72d7129..a7e856c6 100644 --- a/crates/hm/src/orchestrator/mod.rs +++ b/crates/hm/src/orchestrator/mod.rs @@ -8,7 +8,6 @@ pub mod archive; pub mod cache; -pub mod cancel; pub mod docker_client; pub mod docker_host_fns; pub mod events; diff --git a/crates/hm/src/orchestrator/scheduler.rs b/crates/hm/src/orchestrator/scheduler.rs index 6a38f4b4..d65d916f 100644 --- a/crates/hm/src/orchestrator/scheduler.rs +++ b/crates/hm/src/orchestrator/scheduler.rs @@ -44,7 +44,7 @@ use crate::plugin::{PluginRegistry, RegistryConfig}; use super::archive::ArchiveStore; use super::cache; -use super::cancel::CancellationToken; +use tokio_util::sync::CancellationToken; use super::events::EventBus; use super::state::{self, OrchestratorState}; diff --git a/crates/hm/src/orchestrator/state.rs b/crates/hm/src/orchestrator/state.rs index ecfaa5a2..e5776a06 100644 --- a/crates/hm/src/orchestrator/state.rs +++ b/crates/hm/src/orchestrator/state.rs @@ -24,7 +24,7 @@ use uuid::Uuid; use crate::orchestrator::docker_client::DockerClient; use super::archive::ArchiveStore; -use super::cancel::CancellationToken; +use tokio_util::sync::CancellationToken; use super::events::EventBus; /// Live state visible to every host fn while an orchestrator run is diff --git a/crates/hm/src/plugin/host_fns.rs b/crates/hm/src/plugin/host_fns.rs index d9b02777..d7c1f7e9 100644 --- a/crates/hm/src/plugin/host_fns.rs +++ b/crates/hm/src/plugin/host_fns.rs @@ -652,7 +652,7 @@ pub fn kv_set_impl(scope: KvScope, key: &str, val: Vec) { // is not a practical concern. fn plugin_state_path() -> Option { - let dir = dirs::config_dir()?.join("harmont").join("state"); + let dir = hm_util::dirs::harmont_plugin_state_dir()?; let plugin = current_plugin_name()?; Some(dir.join(format!("{plugin}.kv"))) } @@ -991,8 +991,8 @@ mod plugin_kv_tests { use super::*; // Both tests mutate the process-wide `XDG_CONFIG_HOME` env var, - // which `dirs::config_dir()` reads. Serialize them so parallel - // test threads don't race on that global. + // which the platform config_dir lookup reads. Serialize them so + // parallel test threads don't race on that global. static ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); #[test] diff --git a/crates/hm/src/plugin/paths.rs b/crates/hm/src/plugin/paths.rs index ebc82b90..b89895c6 100644 --- a/crates/hm/src/plugin/paths.rs +++ b/crates/hm/src/plugin/paths.rs @@ -13,7 +13,7 @@ use std::path::PathBuf; /// `~/.config/harmont/plugins/` (or the platform's XDG equivalent). /// User-global plugins live here. pub fn user_plugins_dir() -> Option { - dirs::config_dir().map(|p| p.join("harmont").join("plugins")) + hm_util::dirs::harmont_plugins_dir() } /// `/.harmont/plugins/`. Project-local plugins live here. diff --git a/crates/hm/src/plugin/signal.rs b/crates/hm/src/plugin/signal.rs index 83c13160..ebe5c1b7 100644 --- a/crates/hm/src/plugin/signal.rs +++ b/crates/hm/src/plugin/signal.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use crate::orchestrator::cancel::CancellationToken; +use tokio_util::sync::CancellationToken; /// Spawn a tokio task that listens for SIGINT (Ctrl-C) and flips /// the token. Returns a handle; aborting the handle is sufficient