refactor: unify workspace root resolution; fix cloud project-config layering + secret-name encoding

crates/hm-core/workspace.rs

@@ -1,28 +1,42 @@
 //! Global utility for managing the workspace.
 
+use std::io;
 use std::path::{Path, PathBuf};
 
 use hm_config::Config;
 use hm_util::path::{AbsPath, AbsPathBuf};
 use thiserror::Error;
 
-/// Failure loading a directory as a [`Workspace`].
+/// Failure resolving or loading a directory as a [`Workspace`].
 #[derive(Debug, Error)]
 pub enum LoadError {
     #[error("{} does not appear to be a valid path", .0.display())]
     InvalidPath(PathBuf),
     #[error("{} is not a harmont workspace. harmont workspaces must have a `.hm/` directory", .0.display())]
     InvalidWorkspace(PathBuf),
+
+    /// The process working directory could not be read while resolving a root.
+    #[error("cannot determine current directory")]
+    CurrentDir(#[source] io::Error),
+
+    /// Walked to the filesystem root without finding a `.hm/` directory.
+    #[error(
+        "no harmont workspace found\n  → run from a directory that contains `.hm/`, or initialize one with `hm init`"
+    )]
+    NotFound,
+
     #[error(transparent)]
     Config(#[from] hm_config::LoadError),
 }
 
 /// Utility for accessing well-known paths inside a workspace.
 ///
-/// Construct with [`Workspace::load`] on a **specific** project root (the
-/// directory that contains `.hm/`). Callers that need parent-directory
-/// discovery should walk first (e.g. [`hm_util::dirs::find_project_root`])
-/// and only then call [`Workspace::load`].
+/// Two constructors:
+///
+/// - [`Workspace::resolve`] — the one CLI verbs want. Honors a `--dir`-style
+///   override, else walks up from the cwd.
+/// - [`Workspace::load`] — validates one **specific** project root (the
+///   directory that contains `.hm/`), with no discovery.
 #[derive(Debug)]
 pub struct Workspace {
     /// Absolute path to the `.hm` directory.
@@ -37,6 +51,36 @@ pub struct Workspace {
 }
 
 impl Workspace {
+    /// Resolve the workspace a command should operate on.
+    ///
+    /// The single root-resolution rule for every verb that takes a `--dir`:
+    ///
+    /// - **`dir` given** — that directory is the project root, verbatim. No
+    ///   walk-up: the backend runs discovery against cloned repo roots, and
+    ///   walking up out of a clone could bind to an unrelated `.hm/` above it.
+    ///   A relative path is resolved against the cwd, since a workspace root
+    ///   must be absolute.
+    /// - **`dir` absent** — walk up from the cwd, so a verb works from any
+    ///   subdirectory of the project.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`LoadError`] if the cwd cannot be determined, no project root
+    /// is found while walking up, or the resolved root fails [`Self::load`].
+    pub fn resolve(dir: Option<&Path>) -> Result<Self, LoadError> {
+        let root = match dir {
+            Some(d) if d.is_absolute() => d.to_path_buf(),
+            Some(d) => std::env::current_dir()
+                .map_err(LoadError::CurrentDir)?
+                .join(d),
+            None => {
+                let start = std::env::current_dir().map_err(LoadError::CurrentDir)?;
+                hm_util::dirs::find_project_root(&start).ok_or(LoadError::NotFound)?
+            }
+        };
+        Self::load(&root)
+    }
+
     /// Attempt to load the given directory as a workspace, if it appears to be one.
     ///
     /// We label a workspace any directory which has a `.hm` directory within it.

crates/hm-plugin-cloud/Cargo.toml

@@ -19,6 +19,7 @@ hm-exec            = { workspace = true }
 hm-render          = { workspace = true }
 hm-config          = { workspace = true }
 hm-core            = { workspace = true }
+hm-util            = { workspace = true }
 hm-plugin-protocol = { workspace = true }
 serde_json         = { workspace = true }
 chrono             = { workspace = true }

crates/hm-plugin-cloud/src/settings.rs

@@ -15,6 +15,24 @@ use anyhow::{Context, Result};
 use harmont_cloud::HarmontClient;
 use secrecy::{ExposeSecret, SecretString};
 
+/// The layered config for the current directory.
+///
+/// Includes the project `.hm/config.toml` layer whenever the cwd is inside a
+/// harmont project — [`ResolvedCtx::org`]'s own error text tells users to set
+/// `[cloud] org` there, so it has to be read. Outside a project there simply is
+/// no project layer, and the cloud verbs still work from anywhere.
+///
+/// **Read-only callers only.** `hm cloud org switch` deliberately loads the user
+/// layer alone: it saves back to the user file, and merging the project layer in
+/// first would persist project-scoped values into `~/.config/hm/config.toml`.
+fn config() -> Result<hm_config::Config> {
+    let project = std::env::current_dir()
+        .ok()
+        .and_then(|cwd| hm_util::dirs::find_project_root(&cwd))
+        .map(|root| hm_config::Config::project_config_path(&root));
+    hm_config::Config::load(project.as_deref()).context("loading config")
+}
+
 /// Resolve the bearer token, or the shared "not logged in" error.
 ///
 /// Every authenticated path routes through here so the not-logged-in text
@@ -56,7 +74,7 @@ impl ResolvedCtx {
 ///
 /// Returns an error if config can't be loaded or no token is available.
 pub fn client() -> Result<(HarmontClient, ResolvedCtx)> {
-    let cfg = hm_config::Config::load(None).context("loading config")?; // user + env layering
+    let cfg = config()?; // user + project + env layering
     let api = cfg.cloud.api_url.clone();
     let client = HarmontClient::with_base_url(token()?.expose_secret(), &api);
     Ok((
@@ -81,7 +99,7 @@ pub fn client() -> Result<(HarmontClient, ResolvedCtx)> {
 /// Returns an error if config can't be loaded, no token is available, or no
 /// organization is configured.
 pub fn raw_org_ctx() -> Result<(String, SecretString, String)> {
-    let cfg = hm_config::Config::load(None).context("loading config")?;
+    let cfg = config()?;
     let api = cfg.cloud.api_url.clone();
     let token = token()?;
     let org = ResolvedCtx {
@@ -98,7 +116,7 @@ pub fn raw_org_ctx() -> Result<(String, SecretString, String)> {
 ///
 /// Returns an error if config can't be loaded.
 pub fn anon_client() -> Result<(HarmontClient, String)> {
-    let cfg = hm_config::Config::load(None).context("loading config")?;
+    let cfg = config()?;
     let api = cfg.cloud.api_url.clone();
     Ok((HarmontClient::anonymous(&api), api))
 }

crates/hm-plugin-cloud/src/verbs/org.rs

@@ -27,6 +27,9 @@ async fn switch(client: &harmont_cloud::HarmontClient, slug: &str) -> Result<()>
         .iter()
         .find(|o| o.slug == slug)
         .ok_or_else(|| anyhow::anyhow!("no organization with slug '{slug}'"))?;
+    // Load-mutate-save on the *user* config, so this deliberately does NOT use
+    // `settings::config()`: merging the project `.hm/config.toml` layer in first
+    // would persist project-scoped values into ~/.config/hm/config.toml.
     let mut cfg = hm_config::Config::load(None)?;
     cfg.cloud.org = Some(found.slug.clone());
     cfg.save_user().context("saving config")?;

crates/hm-plugin-cloud/src/verbs/secret.rs

@@ -17,11 +17,27 @@ use std::io::Read;
 use std::path::Path;
 
 use anyhow::{Context, Result, bail};
-use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode};
+use percent_encoding::{AsciiSet, NON_ALPHANUMERIC, utf8_percent_encode};
 use secrecy::{ExposeSecret, SecretString};
 
 use crate::cli::SecretCommand;
 
+/// Characters to percent-encode in a single URL path segment.
+///
+/// RFC 3986 leaves the *unreserved* set (`A-Z a-z 0-9 - . _ ~`) safe in a path
+/// segment; everything else is escaped. `NON_ALPHANUMERIC` escapes all four of
+/// `-._~` too, so they are removed here — secret names are conventionally
+/// `[A-Za-z0-9_]`, and encoding `_` as `%5F` requests a different resource than
+/// the one the user named.
+///
+/// What must NOT be removed: `/` and space, which have to survive as `%2F` /
+/// `%20` so a name like `a/b c` cannot escape its scope.
+const SEGMENT: &AsciiSet = &NON_ALPHANUMERIC
+    .remove(b'-')
+    .remove(b'.')
+    .remove(b'_')
+    .remove(b'~');
+
 /// Where the secret value comes from on `set`.
 ///
 /// Exactly one source is allowed; the resolver rejects none and ambiguity.
@@ -47,7 +63,7 @@ fn secrets_path(org: &str, pipeline: Option<&str>) -> String {
 
 /// Append a URL-path-escaped secret name to a collection path.
 fn secret_item_path(collection: &str, name: &str) -> String {
-    format!("{collection}/{}", utf8_percent_encode(name, NON_ALPHANUMERIC))
+    format!("{collection}/{}", utf8_percent_encode(name, SEGMENT))
 }
 
 /// Resolve the secret value from exactly one of: positional `VALUE`,
@@ -318,6 +334,29 @@ mod tests {
         );
     }
 
+    #[test]
+    fn unreserved_name_is_not_encoded() {
+        // RFC 3986 unreserved chars (incl. `-` `_` `.` `~`) must pass through.
+        // Escaping any of them requests a different resource than the user
+        // named. Pinned per-character: encoding these has regressed once
+        // already, via a blanket `NON_ALPHANUMERIC`.
+        let coll = secrets_path("acme", None);
+        for name in [
+            "DEPLOY-TOKEN_v2.0~rc",
+            "DEPLOY_TOKEN",
+            "a-b",
+            "a.b",
+            "a~b",
+            "A-Za-z0-9-._~",
+        ] {
+            assert_eq!(
+                secret_item_path(&coll, name),
+                format!("/api/v0/organizations/acme/secrets/{name}"),
+                "unreserved char was escaped in {name:?}"
+            );
+        }
+    }
+
     #[test]
     fn item_path_escapes_funny_names() {
         let coll = secrets_path("acme", Some("ci"));
@@ -432,11 +471,13 @@ mod tests {
     }
 
     #[test]
-    fn special_chars_are_encoded() {
+    fn reserved_chars_are_encoded() {
+        // The other half of the contract: anything outside the unreserved set
+        // must escape, so a name cannot break out of its path segment.
         let coll = secrets_path("acme", None);
         assert_eq!(
-            secret_item_path(&coll, "DEPLOY-TOKEN_v2.0~rc"),
-            "/api/v0/organizations/acme/secrets/DEPLOY%2DTOKEN%5Fv2%2E0%7Erc"
+            secret_item_path(&coll, "a/b c?d#e%f"),
+            "/api/v0/organizations/acme/secrets/a%2Fb%20c%3Fd%23e%25f"
         );
     }
 

crates/hm/src/cli/mod.rs

@@ -107,7 +107,7 @@ pub async fn dispatch(command: Command, cli: &Cli) -> Result<i32> {
     match command {
         Command::Init(args) => crate::commands::init::handle(args).await.map(|()| 0),
         Command::Run(args) => {
-            let ctx = RunContext::from_cli(cli)?;
+            let ctx = RunContext::from_cli(cli, args.dir.as_deref())?;
             crate::commands::run::handle(args, ctx).await
         }
         Command::Pipelines(args) => crate::cli::pipelines::run(args).await.map(|()| 0),

crates/hm/src/cli/pipelines.rs

@@ -28,10 +28,19 @@ const EMPTY_ENVELOPE: &str = r#"{"schema_version":"1","pipelines":[]}"#;
 /// Returns an error if the engine can't start or the DSL runtime fails to
 /// evaluate the pipelines.
 pub async fn run(args: PipelinesArgs) -> Result<()> {
-    let repo_root = match args.dir {
-        Some(d) => d,
-        None => std::env::current_dir().context("cannot determine current directory")?,
+    use hm_core::WorkspaceLoadError as WsErr;
+    let workspace = match hm_core::Workspace::resolve(args.dir.as_deref()) {
+        Ok(w) => w,
+        // "Not a harmont project" is not an error here — it means this repo
+        // declares no pipelines. A malformed `.hm/config.toml` still fails
+        // loudly rather than masquerading as an empty repo.
+        Err(WsErr::NotFound | WsErr::InvalidPath(_) | WsErr::InvalidWorkspace(_)) => {
+            print!("{EMPTY_ENVELOPE}");
+            return Ok(());
+        }
+        Err(e) => return Err(e.into()),
     };
+    let repo_root = workspace.path().as_path().to_path_buf();
 
     if !detect::has_pipeline_files(&repo_root) {
         print!("{EMPTY_ENVELOPE}");

crates/hm/src/cli/render.rs

@@ -26,10 +26,8 @@ pub struct RenderArgs {
 /// or the slug is unknown / fails to render (the available slugs are written to
 /// stderr by the DSL runtime).
 pub async fn run(args: RenderArgs) -> Result<()> {
-    let repo_root = match args.dir {
-        Some(d) => d,
-        None => std::env::current_dir().context("cannot determine current directory")?,
-    };
+    let workspace = hm_core::Workspace::resolve(args.dir.as_deref())?;
+    let repo_root = workspace.path().as_path().to_path_buf();
 
     detect::check_python(&repo_root).context("detecting pipeline language")?;
     let engine = python_engine().context("initializing DSL engine")?;

crates/hm/src/commands/run/mod.rs

@@ -344,12 +344,12 @@ fn git_remote_repo_name(root: &std::path::Path) -> Option<String> {
 /// the DSL detection / pipeline-render step fails.
 async fn render_pipeline(
     args: &RunArgs,
-    _ctx: &RunContext,
+    ctx: &RunContext,
 ) -> Result<(std::path::PathBuf, String, String)> {
-    let repo_root = match args.dir.clone() {
-        Some(p) => p,
-        None => std::env::current_dir().context("cannot determine current directory")?,
-    };
+    // The root the run executes against is the workspace's, not a second
+    // resolution from `--dir`/cwd — otherwise config could come from one tree
+    // and code from another. `RunContext` already resolved `--dir`.
+    let repo_root = ctx.workspace.path().as_path().to_path_buf();
 
     detect::check_python(&repo_root).map_err(|e| HmError::DslEngine(format!("{e:#}")))?;
     let engine =

crates/hm/src/context.rs

@@ -1,27 +1,9 @@
 use std::io::IsTerminal;
+use std::path::Path;
 
 use crate::cli::Cli;
-use hm_core::Workspace;
+use hm_core::{Workspace, WorkspaceLoadError};
 use hm_render::OutputMode;
-use thiserror::Error;
-
-/// Failure building a [`RunContext`].
-#[derive(Debug, Error)]
-pub enum Error {
-    /// Process current directory could not be resolved.
-    #[error("cannot determine current directory")]
-    CurrentDir(#[source] std::io::Error),
-
-    /// No directory containing `.hm/` was found above the process cwd.
-    #[error(
-        "no harmont workspace found\n  → run from a directory that contains `.hm/`, or initialize one with `hm init`"
-    )]
-    NotFound,
-
-    /// A discovered project root failed [`Workspace::load`].
-    #[error(transparent)]
-    Workspace(#[from] hm_core::WorkspaceLoadError),
-}
 
 /// Runtime context for commands that operate on a harmont project workspace.
 ///
@@ -40,23 +22,20 @@ pub struct RunContext {
 }
 
 impl RunContext {
-    /// Build a [`RunContext`] from parsed CLI args.
+    /// Build a [`RunContext`] from parsed CLI args and the verb's `--dir`.
     ///
-    /// Discovers the project root by walking up from the current directory
-    /// ([`hm_util::dirs::find_project_root`]), then loads a [`Workspace`] for
-    /// that exact path. Walk-up is intentionally separate from
-    /// [`Workspace::load`].
+    /// Root resolution is [`Workspace::resolve`]'s, shared with every other
+    /// `--dir` verb. Passing `dir` through matters: the workspace this loads is
+    /// the one the run executes against, so resolving it from the cwd while the
+    /// run used `--dir` would let config come from one tree and code from
+    /// another.
     ///
     /// # Errors
     ///
-    /// Returns [`Error`] if the current directory cannot be determined, no
-    /// project root is found, or workspace load fails.
-    pub fn from_cli(cli: &Cli) -> Result<Self, Error> {
-        let start_dir = std::env::current_dir().map_err(Error::CurrentDir)?;
-        // Walk-up is separate from Workspace::load (which only validates the
-        // exact path).
-        let root = hm_util::dirs::find_project_root(&start_dir).ok_or(Error::NotFound)?;
-        let workspace = Workspace::load(&root)?;
+    /// Returns [`WorkspaceLoadError`] if the current directory cannot be
+    /// determined, no project root is found, or workspace load fails.
+    pub fn from_cli(cli: &Cli, dir: Option<&Path>) -> Result<Self, WorkspaceLoadError> {
+        let workspace = Workspace::resolve(dir)?;
 
         let output = OutputMode::Human {
             // Single source of truth for the color/TTY rule (still honors --no-color).