From 49067ad83ebf615787dd658d0e43da5f528abbf4 Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 26 Jun 2026 22:00:13 +0200 Subject: [PATCH] fix(#2018): surface permission error in hivectl agent lookup instead of misleading 'no such agent' --- hive-c0re/src/bin/hivectl.rs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/hive-c0re/src/bin/hivectl.rs b/hive-c0re/src/bin/hivectl.rs index 4379f1bf..90ea17f9 100644 --- a/hive-c0re/src/bin/hivectl.rs +++ b/hive-c0re/src/bin/hivectl.rs @@ -933,8 +933,26 @@ fn human_bytes(n: u64) -> String { /// state dir (not the live container list) so kept-state tombstones /// still resolve as agents — re-provisioning a destroyed-but-kept agent /// should still drop its token in the existing state tree. -fn is_agent(name: &str) -> bool { - Coordinator::agent_state_root(name).exists() +/// +/// Uses `try_exists()` rather than `Path::exists()` so a permission +/// error reaching the agents root is surfaced, not collapsed into +/// `false`. The agents root is `0700 hive-core`, so running hivectl +/// without root yields EACCES on traversal — `Path::exists()` would +/// silently report `false`, which callers turn into a misleading "no +/// such agent" (or, for the create-user paths, a silent misclassify of +/// a real agent as a non-agent account). Mapping EACCES to an explicit +/// "needs root" error fixes that first-run footgun (#2018). +fn agent_exists(name: &str) -> Result { + let root = Coordinator::agent_state_root(name); + match root.try_exists() { + Ok(found) => Ok(found), + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => bail!( + "cannot read the agents root {} (permission denied) - this command needs root; \ + re-run with sudo", + root.parent().unwrap_or(&root).display() + ), + Err(e) => Err(e).with_context(|| format!("check agent state dir {}", root.display())), + } } /// Drop into an interactive Claude session in the agent container. @@ -973,7 +991,7 @@ fn is_agent(name: &str) -> bool { /// `machinectl shell` inherits the caller's PTY, so the session is /// fully interactive. Requires root and a running container. fn choom(name: &str, fresh: bool) -> Result<()> { - if !is_agent(name) { + if !agent_exists(name)? { bail!("no such agent: '{name}' (no state dir under /var/lib/hyperhive/agents/)"); } let container = hive_c0re::lifecycle::container_name(name); @@ -1023,7 +1041,7 @@ async fn forge_create_user(name: &str, password: Option<&str>, password_stdin: b ); } let user_password = resolve_password(password, password_stdin)?; - if is_agent(name) { + if agent_exists(name)? { if user_password.is_some() { bail!( "forge create-user: --password / --password-stdin is for non-agent (operator) accounts only; '{name}' is an agent which authenticates via API token" @@ -1095,7 +1113,7 @@ async fn matrix_create_user( .build() .context("build reqwest client")?; let user_password = resolve_password(password, password_stdin)?; - if is_agent(name) { + if agent_exists(name)? { if user_password.is_some() { // The boot-sweep / approval-time agent provisioning path // doesn't accept a password — agents auth by access_token, @@ -1421,7 +1439,7 @@ fn single_agent_scope(name: &str) -> hive_sh4re::LifecycleScope { /// regardless of the migration outcome so a failed migration never leaves /// the agent down; the migration error (if any) is surfaced afterwards. async fn subvol_upgrade(socket: &Path, name: &str, yes: bool) -> Result<()> { - if !is_agent(name) { + if !agent_exists(name)? { bail!("no agent named {name:?} (no state dir under the agents root)"); } if !yes {