diff --git a/Cargo.lock b/Cargo.lock index f942f9ed..9ed30fa7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1817,6 +1817,7 @@ dependencies = [ "hive-sh4re", "hive-types", "indicatif", + "libc", "serde_json", "tokio", "tracing-subscriber", diff --git a/docs/tools/hivectl.md b/docs/tools/hivectl.md index ffadcd87..c8290160 100644 --- a/docs/tools/hivectl.md +++ b/docs/tools/hivectl.md @@ -211,7 +211,17 @@ new containers and restarts pick the values up immediately. Drop into an interactive Claude session inside an agent container. Replaces the current process with `machinectl shell @h-` running claude from the agent's state dir. Requires root (same as all -`machinectl shell` operations). +`machinectl shell` operations) — hyperhive ships no polkit rule granting +those actions to the operator group, so `choom` refuses up front with a +message naming that requirement rather than letting systemd reject the +exec later. + +It also needs the daemon socket, unlike the other exec-into-a-container +paths: the "is this actually an agent?" pre-flight reads the agents root, +which is owned by the daemon's user and not group-readable, so the check +is a `HostRequest` rather than a local `stat`. A rootless `choom` therefore +tells you it needs root, instead of reporting a permission problem with +the state dir. ```bash hivectl choom iris # fresh blank Claude session in iris's container diff --git a/hive-c0re/src/server.rs b/hive-c0re/src/server.rs index ef74602a..4d0f5295 100644 --- a/hive-c0re/src/server.rs +++ b/hive-c0re/src/server.rs @@ -162,6 +162,10 @@ async fn dispatch(req: &HostRequest, coord: Arc) -> HostResponse { HostResponse::dags(dags) } HostRequest::List => HostResponse::list(lifecycle::list().await?), + // The agents root is ours and not world-traversable, so this + // question is only answerable on this side of the socket — + // see the request's doc comment for why the client asks. + HostRequest::AgentExists { name } => HostResponse::agent_exists(agent_exists(name)?), HostRequest::AgentStatus => handle_agent_status(&coord).await, // The hive domain + per-surface public URLs are injected into // c0re's service env by hive-c0re.nix; surface them so the @@ -380,7 +384,12 @@ fn matrix_http_client() -> Result { } /// True when `name` has a state dir under the agents root, i.e. it's a -/// managed agent rather than a bare (operator/human) matrix account. +/// managed agent rather than a bare (operator/human) account. +/// +/// Used by the provisioning handlers below to tell an agent account from +/// a human one, and exposed over the socket as +/// [`HostRequest::AgentExists`] for clients that can't read the agents +/// root themselves (it's `0700` and owned by the daemon's user). fn agent_exists(name: &hive_types::Ident) -> Result { crate::paths::agent_state_dir(name) .try_exists() diff --git a/hive-host-sock/src/lib.rs b/hive-host-sock/src/lib.rs index 0a70165a..0427c6ee 100644 --- a/hive-host-sock/src/lib.rs +++ b/hive-host-sock/src/lib.rs @@ -136,6 +136,19 @@ pub enum HostRequest { Rebuild { name: Ident }, /// List managed containers. List, + /// Report whether `name` is a managed agent, i.e. whether it has a + /// persistent state dir under the agents root. Answered daemon-side + /// because that root is `0700 hive-core`: a client stat-ing it + /// without root gets EACCES, so the pre-flight "does this agent + /// exist?" guard in front of `hivectl choom` / `agents subvol` used + /// to fail with a permission error instead of an answer. The daemon + /// already runs as the owning user and does the same check for its + /// own provisioning paths. Result: [`HostResponse::agent_exists`]. + /// + /// State dir, not the live container list — a destroyed-but-kept + /// agent is still an agent, and re-provisioning one should drop its + /// credentials into the existing state tree. + AgentExists { name: Ident }, /// List managed agents with their full status + technical state /// (running / needs-login / needs-update / deployed sha / parent / /// pending reminders) — the `hivectl agents list` roster view. @@ -441,6 +454,13 @@ pub struct HostResponse { /// request kind. #[serde(default, skip_serializing_if = "Option::is_none")] pub agent_statuses: Option>, + /// `AgentExists` result — whether the named agent has a state dir + /// under the agents root. `None` for every other request kind, which + /// is why it's an `Option` and not a bare `bool`: a client must + /// be able to tell "the daemon said no" from "the daemon answered a + /// different question". + #[serde(default, skip_serializing_if = "Option::is_none")] + pub agent_exists: Option, /// Ids of the job-queue DAGs this request submitted (rebuild / /// restart / power ops). Clients poll them via /// [`HostRequest::QueueDag`]; `None` for non-submitting requests. @@ -520,6 +540,16 @@ impl HostResponse { } } + /// `AgentExists` result — whether the named agent has a state dir. + #[must_use] + pub fn agent_exists(exists: bool) -> Self { + Self { + ok: true, + agent_exists: Some(exists), + ..Self::default() + } + } + /// A request that submitted job-queue DAGs — carries their ids for /// the client's wait/progress loop. #[must_use] diff --git a/hivectl/Cargo.toml b/hivectl/Cargo.toml index aad05e71..6a080d93 100644 --- a/hivectl/Cargo.toml +++ b/hivectl/Cargo.toml @@ -19,6 +19,7 @@ hive-host-sock.workspace = true hive-sh4re.workspace = true hive-types.workspace = true indicatif.workspace = true +libc.workspace = true serde_json.workspace = true tokio.workspace = true tracing-subscriber.workspace = true diff --git a/hivectl/src/choom.rs b/hivectl/src/choom.rs index 598f35ae..bd429f6d 100644 --- a/hivectl/src/choom.rs +++ b/hivectl/src/choom.rs @@ -4,11 +4,32 @@ #[cfg(unix)] use std::os::unix::process::CommandExt as _; +use std::path::Path; use anyhow::{Result, bail}; use crate::util::agent_exists; +/// Refuse early when the caller isn't root. +/// +/// `machinectl shell` needs root or a polkit grant, and hyperhive ships +/// no polkit rules — so unprivileged callers hit an authentication +/// prompt/refusal from systemd that says nothing about which hivectl +/// verb wanted it. Naming the requirement here keeps the failure honest +/// and points at the one thing that would fix it. +fn require_root() -> Result<()> { + // SAFETY: `geteuid` takes no arguments, reads a process attribute + // the kernel always has, and cannot fail. + let euid = unsafe { libc::geteuid() }; + if euid != 0 { + bail!( + "choom needs root: it runs `machinectl shell`, which requires root \ + (hyperhive ships no polkit rule granting it) - re-run with sudo" + ); + } + Ok(()) +} + /// Drop into an interactive Claude session in the agent container. /// /// Execs `machinectl shell @h-` running claude as the agent @@ -17,13 +38,19 @@ use crate::util::agent_exists; /// live harness session). See `docs/tools/hivectl.md` (Choom) for the /// full rationale. Inherits the caller's PTY; requires root + a running /// container. -pub(crate) fn choom(name: &str, resume_session: Option<&str>) -> Result<()> { - if !agent_exists(name)? { +/// +/// The existence check goes over the daemon socket even though the exec +/// itself doesn't need it: the agents root isn't readable by the +/// operator group, so checking locally reported every rootless invocation +/// as a permission problem with the state dir rather than with the shell. +pub(crate) async fn choom(socket: &Path, name: &str, resume_session: Option<&str>) -> Result<()> { + if !agent_exists(socket, name).await? { bail!( "no such agent: '{name}' (no state dir under {}/)", hive_host_sock::AGENTS_ROOT ); } + require_root()?; let container = hive_host_sock::container_name(name); // Enter as the agent's unix user (== agent name) so claude reads the // right `$HOME/.claude`. diff --git a/hivectl/src/main.rs b/hivectl/src/main.rs index 3f6b33c6..bbea02ff 100644 --- a/hivectl/src/main.rs +++ b/hivectl/src/main.rs @@ -6,9 +6,12 @@ //! restart|…>`, `approvals `, `stop` / `start`) and //! provisioning (`forge` / `matrix` / `github` / `gateway`) all forward to //! the daemon, which owns the broker, the credentials, and the provisioning -//! logic — a running daemon is required for those. A few verbs work off -//! local host state directly instead (`wg` / `peer-config` read the mesh key -//! + TLS CA; `choom` execs into a container), so they don't need the socket. +//! logic — a running daemon is required for those. A couple of verbs work +//! off local host state directly instead (`wg` / `peer-config` read the mesh +//! key + TLS CA), so they don't need the socket. `choom` execs into a +//! container rather than asking the daemon to do anything, but still uses the +//! socket for its "is this an agent?" pre-flight — that answer lives in a +//! directory only the daemon's user can read. //! //! One module per subcommand family (see the `mod` list below); `main` is //! just the clap parse + the top-level dispatch match. @@ -123,7 +126,7 @@ async fn main() -> Result<()> { Cmd::Choom { name, resume_session, - } => choom(&name, resume_session.as_deref()), + } => choom(&socket, &name, resume_session.as_deref()).await, Cmd::MarkdownDocs => { print!("{}", clap_markdown::help_markdown::()); Ok(()) diff --git a/hivectl/src/subvol.rs b/hivectl/src/subvol.rs index c6a8030c..b5371a80 100644 --- a/hivectl/src/subvol.rs +++ b/hivectl/src/subvol.rs @@ -52,7 +52,7 @@ pub(crate) async fn dispatch_subvol(socket: &Path, cmd: SubvolCmd) -> Result<()> } async fn subvol_upgrade(socket: &Path, name: &str, yes: bool) -> Result<()> { - if !agent_exists(name)? { + if !agent_exists(socket, name).await? { bail!("no agent named {name:?} (no state dir under the agents root)"); } if !yes { @@ -151,7 +151,7 @@ async fn subvol_upgrade(socket: &Path, name: &str, yes: bool) -> Result<()> { /// enforces the same rules server-side, so this check is /// belt-and-suspenders (fail fast client-side with a clear message). async fn subvol_snapshot_create(socket: &Path, name: &str, label: String) -> Result<()> { - if !agent_exists(name)? { + if !agent_exists(socket, name).await? { bail!("no agent named {name:?} (no state dir under the agents root)"); } if !label.starts_with("hive-") { diff --git a/hivectl/src/util.rs b/hivectl/src/util.rs index 4b5aa777..9ed1fcd9 100644 --- a/hivectl/src/util.rs +++ b/hivectl/src/util.rs @@ -117,32 +117,34 @@ pub(crate) async fn query_hive_urls(socket: &Path) -> Result Result { - let Ok(name) = hive_types::Ident::parse(name) else { - bail!("invalid agent name {name:?}"); - }; - let root = hive_host_sock::agent_state_dir(&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())), +/// Asks the daemon rather than stat-ing the path. hivectl used to check +/// locally, but the agents root is `0700` and owned by the daemon's +/// user, so an operator without root got EACCES on traversal — which +/// this function then had to report as "needs root", turning every +/// caller's existence guard into a permission error. The daemon owns +/// that directory and answers the same question over the socket, which +/// operators can already reach via the `hive-admin` group, so the guard +/// works sudoless and the remaining root requirements (if any) surface +/// where they actually are. +pub(crate) async fn agent_exists(socket: &Path, name: &str) -> Result { + let resp = crate::client::request( + socket, + hive_host_sock::HostRequest::AgentExists { + name: parse_ident(name)?, + }, + ) + .await?; + if !resp.ok { + bail!( + "check whether {name:?} is an agent: {}", + resp.error.as_deref().unwrap_or("unknown error") + ); } + resp.agent_exists + .context("daemon answered the agent-exists check without a result") }