From f108c72f252b6833f8e29343ee032369cadc2b72 Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 26 Jul 2026 16:27:38 +0200 Subject: [PATCH] hivectl: say which of the three socket failures actually happened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hivectl open forge` on a host where the daemon is fine and the socket is fine printed "could not reach the hive-c0re daemon for URLs — is hive-c0re running?". It was running. The operator was not in `hive-admin` in that shell, and the connect got EACCES. The message was a guess, not a diagnosis, because `query_hive_urls` returned `Option` and threw the cause away with `.ok()`. Three different failures — not in the group, no socket at all, nobody listening — all arrived as the same sentence, and only one of the three is fixed by looking at the daemon. Classify the connect error in `client::request`, which every daemon-assisted verb goes through, and keep the io error as the anyhow cause so the output reads fix-first. EACCES names `hive-admin`, `services.hyperhive.adminUsers`, and — the part that actually bites — the re-login, since secondary group membership is only applied at login, so a shell opened before the grant still cannot connect. ENOENT and ECONNREFUSED point at the units instead. Then stop discarding it: `query_hive_urls` returns `Result>`, `open` and `require_hive_domain` propagate, and `daemon_request` drops its own "connect to daemon socket" context, which only buried the actionable line under a vaguer one. `wg init`'s domain lookup stays best-effort by an explicit `.ok().flatten()` rather than by accident. Same footgun `agent_exists` was already fixed for: a permission error collapsed into a value that reads as a different, wrong story. --- hivectl/src/client.rs | 107 ++++++++++++++++++++++++++++++++++++++++-- hivectl/src/open.rs | 14 +++--- hivectl/src/util.rs | 27 +++++++---- hivectl/src/wg.rs | 19 +++++--- 4 files changed, 139 insertions(+), 28 deletions(-) diff --git a/hivectl/src/client.rs b/hivectl/src/client.rs index 51b79cef..344b2ade 100644 --- a/hivectl/src/client.rs +++ b/hivectl/src/client.rs @@ -1,14 +1,60 @@ +//! Host admin socket client: one request/response round trip over +//! `/run/hyperhive/host.sock`. +//! +//! Connect failures are classified into an actionable message before they +//! reach the operator (see [`connect_hint`]) — the three ways this fails +//! (not in `hive-admin`, no socket, nobody listening) need three different +//! fixes, and the raw `Permission denied (os error 13)` names none of them. + +use std::io::ErrorKind; use std::path::Path; -use anyhow::{Context, Result, bail}; +use anyhow::{Result, bail}; use hive_host_sock::{HostRequest, HostResponse}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::UnixStream; +/// Turn a socket-connect `io::ErrorKind` into a message that names the fix. +/// +/// The interesting one is `PermissionDenied`: the socket is `0660 +/// root:hive-admin` behind a `0751` runtime dir, so a non-member gets EACCES +/// on connect — and so does a *member* whose shell predates the group being +/// granted, because secondary group membership is only applied at login. That +/// second case is the one that reads as "the daemon is down" and isn't. +fn connect_hint(kind: ErrorKind, socket: &Path) -> String { + let path = socket.display(); + match kind { + ErrorKind::PermissionDenied => format!( + "permission denied opening the host admin socket {path} — reaching it requires \ + membership in the `hive-admin` group. Add your user to \ + `services.hyperhive.adminUsers`, then log out and back in: secondary group \ + membership is applied at login, so a shell opened before the change still \ + cannot connect. `id -nG` shows what the running shell actually has; \ + `newgrp hive-admin` picks the group up without a full re-login" + ), + ErrorKind::NotFound => format!( + "no socket at {path} — hive-c0re is not running, or it binds somewhere else \ + (`--socket`). Check `systemctl status hive-c0re.socket hive-c0re.service`" + ), + ErrorKind::ConnectionRefused => format!( + "nothing is listening on {path} — the socket exists but the daemon behind it \ + is down. Check `systemctl status hive-c0re.service` and \ + `journalctl -u hive-c0re`" + ), + _ => format!("could not connect to the hive-c0re host admin socket {path}"), + } +} + pub async fn request(socket: &Path, req: HostRequest) -> Result { - let stream = UnixStream::connect(socket) - .await - .with_context(|| format!("connect to {}", socket.display()))?; + let stream = match UnixStream::connect(socket).await { + Ok(stream) => stream, + // Keep the io error as the cause and put the actionable line on top, + // so `Error: ` / `Caused by: ` reads fix-first. + Err(e) => { + let hint = connect_hint(e.kind(), socket); + return Err(anyhow::Error::new(e).context(hint)); + } + }; let (read, mut write) = stream.into_split(); let mut payload = serde_json::to_string(&req)?; @@ -25,3 +71,56 @@ pub async fn request(socket: &Path, req: HostRequest) -> Result { let resp: HostResponse = serde_json::from_str(line.trim())?; Ok(resp) } + +#[cfg(test)] +mod tests { + use super::{ErrorKind, Path, connect_hint}; + + fn hint(kind: ErrorKind) -> String { + connect_hint(kind, Path::new("/run/hyperhive/host.sock")) + } + + /// EACCES is an operator-side fix, and the non-obvious half is the + /// re-login — so both the group and the login requirement must appear, + /// and it must not send the operator off checking a daemon that is fine. + #[test] + fn permission_denied_names_the_group_and_the_relogin() { + let h = hint(ErrorKind::PermissionDenied); + assert!(h.contains("hive-admin"), "{h}"); + assert!(h.contains("adminUsers"), "{h}"); + assert!(h.contains("log out"), "{h}"); + assert!( + !h.contains("not running"), + "EACCES must not blame the daemon: {h}" + ); + } + + /// The inverse: a missing socket is a daemon-side problem, so it must not + /// send the operator chasing group membership. + #[test] + fn missing_socket_blames_the_daemon_not_the_operator() { + let h = hint(ErrorKind::NotFound); + assert!(h.contains("not running"), "{h}"); + assert!(!h.contains("hive-admin"), "{h}"); + } + + #[test] + fn refused_socket_points_at_the_service_not_the_group() { + let h = hint(ErrorKind::ConnectionRefused); + assert!(h.contains("nothing is listening"), "{h}"); + assert!(!h.contains("hive-admin"), "{h}"); + } + + #[test] + fn every_hint_names_the_socket_path() { + for kind in [ + ErrorKind::PermissionDenied, + ErrorKind::NotFound, + ErrorKind::ConnectionRefused, + ErrorKind::BrokenPipe, + ] { + let h = hint(kind); + assert!(h.contains("/run/hyperhive/host.sock"), "{kind:?}: {h}"); + } + } +} diff --git a/hivectl/src/open.rs b/hivectl/src/open.rs index e951804c..e00499ee 100644 --- a/hivectl/src/open.rs +++ b/hivectl/src/open.rs @@ -5,7 +5,7 @@ use std::path::Path; use anyhow::{Context as _, Result}; -use crate::cli::{DEFAULT_HOST_SOCKET, OpenTarget}; +use crate::cli::OpenTarget; use crate::util::query_hive_urls; /// `open ` — resolve the surface URL from the daemon, @@ -13,12 +13,12 @@ use crate::util::query_hive_urls; /// core (headless / SSH hosts where no browser opener exists); the open /// is convenience on top, so a missing/failed `xdg-open` is not an error. pub(crate) async fn open_url(socket: &Path, target: OpenTarget) -> Result<()> { - let urls = query_hive_urls(socket).await.with_context(|| { - format!( - "could not reach the hive-c0re daemon for URLs — is hive-c0re running? \ - (the socket is at {DEFAULT_HOST_SOCKET})" - ) - })?; + // The connect error is already actionable (`client::request` classifies + // it), so propagate it rather than restating a guess about the cause. + let urls = query_hive_urls(socket) + .await + .context("could not read this hive's URLs from the daemon")? + .context("the daemon reported no URLs — `services.hyperhive.domain` is unset")?; let (url, hint) = match target { OpenTarget::Home => ( urls.home, diff --git a/hivectl/src/util.rs b/hivectl/src/util.rs index ceec313a..4b5aa777 100644 --- a/hivectl/src/util.rs +++ b/hivectl/src/util.rs @@ -24,9 +24,10 @@ pub(crate) async fn daemon_request( req: hive_host_sock::HostRequest, label: &str, ) -> Result<()> { - let resp = crate::client::request(socket, req) - .await - .with_context(|| format!("connect to daemon socket {}", socket.display()))?; + // No extra context on the request: `client::request` already names the + // socket and classifies the failure, and wrapping it here would put a + // vaguer line on top of the actionable one. + let resp = crate::client::request(socket, req).await?; if !resp.ok { bail!( "{label}: {}", @@ -100,13 +101,19 @@ pub(crate) fn render_lifecycle(resp: &hive_host_sock::HostResponse, verb: &str) Ok(()) } -/// Best-effort query for this hive's domain + browser-facing web URLs -/// (`HostRequest::Urls`). `None` when the daemon is unreachable. -pub(crate) async fn query_hive_urls(socket: &Path) -> Option { - crate::client::request(socket, hive_host_sock::HostRequest::Urls) - .await - .ok() - .and_then(|r| r.urls) +/// Query this hive's domain + browser-facing web URLs (`HostRequest::Urls`). +/// +/// `Ok(None)` = the daemon answered and has nothing to report; `Err` = it was +/// never reached, and the error carries the actionable connect hint (see +/// [`crate::client::request`]). Keep the two apart: collapsing the error into +/// `None` here is what made a permission problem on the socket read as "is +/// hive-c0re running?", sending the operator to fix the wrong thing. +pub(crate) async fn query_hive_urls(socket: &Path) -> Result> { + Ok( + crate::client::request(socket, hive_host_sock::HostRequest::Urls) + .await? + .urls, + ) } /// True when `name` matches an existing hyperhive agent — i.e. it has a diff --git a/hivectl/src/wg.rs b/hivectl/src/wg.rs index d5c5083e..f4cba2d7 100644 --- a/hivectl/src/wg.rs +++ b/hivectl/src/wg.rs @@ -24,19 +24,24 @@ const HIVE_TLS_CA_PATH: &str = "/var/lib/hive-tls/ca.pem"; /// Best-effort query for this hive's domain from the running daemon /// (`HostRequest::Urls`, which reads `HYPERHIVE_HIVE_DOMAIN` from c0re's /// service env). `None` when the daemon is unreachable or the domain is -/// unset — callers decide whether that's fatal. +/// unset — for the callers that treat both as "skip the optional block". +/// Use [`require_hive_domain`] where the distinction matters: it keeps the +/// connect error (and its hint) instead of flattening it away. async fn query_hive_domain(socket: &Path) -> Option { - query_hive_urls(socket).await.and_then(|u| u.domain) + query_hive_urls(socket).await.ok().flatten()?.domain } /// Require this hive's domain from the daemon for snippet generation. /// Errors with a clear hint when it can't be resolved, so `peer-config` -/// never silently emits a wrong key. +/// never silently emits a wrong key. An unreachable daemon propagates its +/// own (classified) connect error; only a *reachable* daemon with no domain +/// gets the config hint. pub(crate) async fn require_hive_domain(socket: &Path) -> Result { - query_hive_domain(socket).await.context( - "could not determine this hive's domain from the daemon — is hive-c0re running \ - and `services.hyperhive.domain` set?", - ) + query_hive_urls(socket) + .await + .context("could not determine this hive's domain from the daemon")? + .and_then(|u| u.domain) + .context("the daemon reported no domain — set `services.hyperhive.domain`") } /// `wg init` — generate (if absent) the hive's WireGuard key, print its