From 53b4e752efee52d961c05ab4cca231679e4b6872 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 22 Jun 2026 13:34:29 +0200 Subject: [PATCH] refactor(#1865): replace the privileged flag with topology + capability gating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per operator direction (no privileged mode; everything is perms / capabilities), remove the socket-derived `privileged: bool` from the unified dispatch and gate every verb on the caller's identity instead: - serve/dispatch/dispatch_shared/dispatch_orchestration + all lifecycle handlers drop the `privileged` param. - lifecycle (start/kill/restart/update/init_config/apply_commit) + get_logs gate on `topology::is_descendant_of` (a parent owns its whole subtree; the root covers every agent as a consequence, no positional privilege). The restart infra-branch stays InfraAdmin-gated (orthogonal). - agent-state queries (loose-ends / reminder count + rollup): own subtree is free, other agents + the hive-wide `"*"` sweep require QueryAgentState. require_new_child + resolve_agent_state_target widened direct-child -> subtree. - hive-wide orchestration verbs gate on the grantable tool-group via tool_groups::groups_for: schedules -> `scheduling`, meta-inputs + cancel-approval -> `approvals`. update_meta_inputs now attributes the approval to the caller, not a hardcoded MANAGER_AGENT. - #1834 cancel-guard unwind: handle_cancel_loose_end drops `privileged` (agent path is never privileged); question/reminder cancels are ownership-only, approval cancel checks the `approvals` tool-group. The manager socket stays as pure transport (serves agent=ruth, no authority of its own); collapsing it into ruth's per-agent socket is the #1825 follow-up. No is_root here — root-identity primitives are #1825's. --- hive-c0re/src/questions.rs | 84 +++--- hive-c0re/src/socket_server.rs | 510 +++++++++++++++------------------ 2 files changed, 282 insertions(+), 312 deletions(-) diff --git a/hive-c0re/src/questions.rs b/hive-c0re/src/questions.rs index 4e23f8eb..fb287613 100644 --- a/hive-c0re/src/questions.rs +++ b/hive-c0re/src/questions.rs @@ -141,26 +141,27 @@ pub fn handle_answer( Ok(()) } -/// Handle `CancelLooseEnd` from either surface. Dispatches by kind to -/// the per-kind cancel, each of which does its own auth check -/// (canceller == owner / asker, or `operator`, or `privileged`). -/// `privileged` is `true` when the request arrived on the manager -/// socket — privilege derives from the socket (the trust boundary), -/// not from matching a hardcoded agent name. On question cancel, fires -/// the `QuestionAnswered` event back to the asker so the harness -/// loop can react (mirrors the operator-cancel dashboard path). +/// Handle `CancelLooseEnd` from a per-agent socket. Dispatches by kind, each +/// with its own auth check: question / reminder cancels are ownership-only +/// (an agent cancels its own), and approval cancels require the `approvals` +/// tool-group (the grantable capability) — no positional / hardcoded +/// privilege. (The operator's cancel-anything path is a separate handler.) +/// On question cancel, fires the `QuestionAnswered` event back to the asker +/// so the harness loop can react (mirrors the operator-cancel dashboard path). pub fn handle_cancel_loose_end( coord: &Arc, canceller: &str, - privileged: bool, kind: hive_sh4re::CancelLooseEndKind, id: i64, ) -> Result<(), String> { match kind { hive_sh4re::CancelLooseEndKind::Question => { + // Agent-socket path: never privileged — an agent may only cancel + // its own question (ownership). The operator's cancel-anything + // path goes through a separate handler with `privileged = true`. let (question, asker, target) = coord .questions - .cancel(id, canceller, privileged) + .cancel(id, canceller, false) .map_err(|e| format!("{e:#}"))?; let sentinel = format!("[cancelled by {canceller}]"); tracing::info!(%id, %canceller, %asker, "question cancelled"); @@ -184,19 +185,21 @@ pub fn handle_cancel_loose_end( Ok(()) } hive_sh4re::CancelLooseEndKind::Reminder => { + // Agent-socket path: ownership-only (cancel your own reminder). let owner = coord .broker - .cancel_reminder_as(id, canceller, privileged) + .cancel_reminder_as(id, canceller, false) .map_err(|e| format!("{e:#}"))?; tracing::info!(%id, %canceller, %owner, "reminder cancelled"); coord.emit_reminders_snapshot(); Ok(()) } hive_sh4re::CancelLooseEndKind::Approval => { - // Privileged-only: only a caller on the manager socket (which - // is the sole approval submitter) may withdraw approvals. - // Sub-agents have no pending approvals of their own anyway. - check_can_cancel_approval(privileged)?; + // Withdrawing an approval is a hive-wide orchestration action, + // gated on the grantable `approvals` tool-group (held by the + // orchestrator that submits approvals) — not on a positional / + // hardcoded privilege. + check_can_cancel_approval(canceller)?; let approval = coord .approvals .mark_cancelled(id, canceller) @@ -220,21 +223,26 @@ pub fn handle_cancel_loose_end( } } -/// Privileged-only guard on the `Approval` cancel arm. Pulled out so -/// the auth check has its own focused unit test — testing the full -/// `handle_cancel_loose_end` flow would need a `Coordinator` fixture -/// (broker + sqlite + in-memory questions), which we don't have -/// today. Privilege is a property of the socket the request arrived -/// on (the manager socket), threaded in as `privileged` — not a match -/// against a hardcoded agent name. -fn check_can_cancel_approval(privileged: bool) -> Result<(), String> { - if !privileged { - return Err( - "cancel_loose_end: only a privileged (manager-socket) caller can cancel approval rows" +/// Capability guard on the `Approval` cancel arm: the caller must hold the +/// `approvals` tool-group (the grantable capability for approval-submitting +/// orchestrators), checked server-side via `tool_groups::groups_for`. Pulled +/// out so the auth check has its own focused unit test — exercising the full +/// `handle_cancel_loose_end` flow would need a `Coordinator` fixture (broker + +/// sqlite + in-memory questions) we don't have. Keys on a grantable capability, +/// not a positional / hardcoded privilege. +fn check_can_cancel_approval(canceller: &str) -> Result<(), String> { + const APPROVALS_GROUP: &str = "approvals"; + if crate::tool_groups::groups_for(canceller) + .iter() + .any(|g| g == APPROVALS_GROUP) + { + Ok(()) + } else { + Err( + "cancel_loose_end: cancelling approval rows requires the `approvals` tool group" .to_owned(), - ); + ) } - Ok(()) } #[cfg(test)] @@ -242,18 +250,14 @@ mod tests { use super::*; #[test] - fn approval_cancel_rejects_unprivileged_callers() { - // A non-privileged caller (any regular agent socket) must not be - // able to cancel approval rows even if it invents an id. The guard - // is server-side so client cooperation is irrelevant — and it keys - // on the socket-derived `privileged` flag, not on any agent name. - let err = check_can_cancel_approval(false).unwrap_err(); - assert!(err.contains("only a privileged"), "{err}"); - } - - #[test] - fn approval_cancel_allows_privileged() { - check_can_cancel_approval(true).expect("a privileged caller must pass the guard"); + fn approval_cancel_rejects_callers_without_the_approvals_group() { + // A caller that doesn't hold the `approvals` tool-group must not be + // able to cancel approval rows even if it invents an id. The guard is + // server-side so client cooperation is irrelevant — and it keys on a + // grantable capability (the tool-group), not on any agent name. + // `groups_for` of a name with no tool_groups.json entry is empty. + let err = check_can_cancel_approval("nobody-with-no-groups").unwrap_err(); + assert!(err.contains("approvals` tool group"), "{err}"); } } diff --git a/hive-c0re/src/socket_server.rs b/hive-c0re/src/socket_server.rs index 3e0ff05c..54384096 100644 --- a/hive-c0re/src/socket_server.rs +++ b/hive-c0re/src/socket_server.rs @@ -1,11 +1,13 @@ //! Unix-socket request server, shared by the per-agent sockets and the -//! manager socket. The socket file's existence on disk authenticates the -//! caller: connecting to `<.../agents/foo/mcp.sock>` means you are `foo` -//! (non-privileged); connecting to the manager socket grants privileged -//! authority. Both transports run the same [`serve`] / [`dispatch`] code, -//! parameterised by a `privileged: bool` carried from the listener — the -//! privilege gate (and the topology guards on lifecycle verbs) keys off -//! that flag, not off matching the `MANAGER_AGENT` name. +//! (pure-transport) manager socket. The socket file's existence on disk +//! authenticates the caller: connecting to `<.../agents/foo/mcp.sock>` means +//! you are `foo`; the manager socket simply serves as `ruth`. There is no +//! privilege flag — both transports run the same [`serve`] / [`dispatch`] +//! code, and authority derives uniformly from the caller's identity: +//! topology (`is_descendant_of`) for subtree-relational verbs, capabilities +//! for hive-wide queries, and tool-group membership for the orchestration +//! verbs. `ruth` reaches every agent only as a consequence of being the +//! topology root, not via any hardcoded name match. use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -54,8 +56,7 @@ pub fn start(agent: &str, socket_path: &Path, coord: Arc) -> Result let agent = agent.clone(); let coord = coord.clone(); tokio::spawn(async move { - // Per-agent socket: never privileged. - if let Err(e) = serve(stream, agent, false, coord).await { + if let Err(e) = serve(stream, agent, coord).await { tracing::warn!(error = ?e, "agent connection failed"); } }); @@ -70,11 +71,12 @@ pub fn start(agent: &str, socket_path: &Path, coord: Arc) -> Result Ok(AgentSocket { path, handle }) } -/// Bind + serve the manager socket. The manager socket is the privileged -/// trust boundary: every connection that lands here runs `dispatch` with -/// `privileged = true`. `MANAGER_AGENT` is passed as the actor name for -/// attribution/routing (notifications, ownership), not for authorisation — -/// authority comes from the socket itself. +/// Bind + serve the manager socket. This is now **pure transport**: it grants +/// no authority of its own — it just serves requests as `agent = MANAGER_AGENT` +/// ("ruth"), and ruth's reach comes entirely from being the topology root +/// (`is_descendant_of` covers every agent) plus the capabilities / tool-groups +/// it holds, identical to connecting on a per-agent socket. (The redundant +/// path — ruth using the standard per-agent socket — is collapsed in #1825.) pub fn start_manager(coord: Arc) -> Result<()> { use std::os::unix::fs::PermissionsExt as _; let dir = Coordinator::manager_dir(); @@ -98,8 +100,8 @@ pub fn start_manager(coord: Arc) -> Result<()> { Ok((stream, _)) => { let coord = coord.clone(); tokio::spawn(async move { - // Manager socket: privileged. - if let Err(e) = serve(stream, MANAGER_AGENT.to_owned(), true, coord).await { + // Pure transport: serve as `ruth`, no privilege grant. + if let Err(e) = serve(stream, MANAGER_AGENT.to_owned(), coord).await { tracing::warn!(error = ?e, "manager connection failed"); } }); @@ -114,12 +116,7 @@ pub fn start_manager(coord: Arc) -> Result<()> { Ok(()) } -async fn serve( - stream: UnixStream, - agent: String, - privileged: bool, - coord: Arc, -) -> Result<()> { +async fn serve(stream: UnixStream, agent: String, coord: Arc) -> Result<()> { let (read, mut write) = stream.into_split(); let mut reader = BufReader::new(read); let mut line = String::new(); @@ -130,7 +127,7 @@ async fn serve( return Ok(()); } let resp = match serde_json::from_str::(line.trim()) { - Ok(req) => dispatch(&req, &agent, privileged, &coord).await, + Ok(req) => dispatch(&req, &agent, &coord).await, Err(e) => AgentResponse::Err { message: format!("parse error: {e}"), }, @@ -174,12 +171,11 @@ pub(crate) fn recv_timeout(wait_seconds: Option) -> std::time::Duration { /// semantics (e.g. `GetLooseEnds` / `CountPendingReminders` / `ReminderRollup` /// where the manager can target other agents) or for manager-only variants. /// -/// The unified `dispatch` calls this first (for both the privileged and -/// non-privileged paths); the remaining arms are handled there. +/// The unified `dispatch` calls this first; the remaining arms (which gate +/// on topology / capabilities / tool-groups) are handled there. pub(crate) async fn dispatch_shared( req: &hive_sh4re::Request, agent: &str, - privileged: bool, coord: &Arc, ) -> Option { Some(match req { @@ -234,11 +230,10 @@ pub(crate) async fn dispatch_shared( handle_get_agent_meta(coord, agent, name.as_deref()).await } hive_sh4re::Request::CancelLooseEnd { kind, id } => { - crate::questions::handle_cancel_loose_end(coord, agent, privileged, *kind, *id) - .map_or_else( - |message| hive_sh4re::Response::Err { message }, - |()| hive_sh4re::Response::Ok, - ) + crate::questions::handle_cancel_loose_end(coord, agent, *kind, *id).map_or_else( + |message| hive_sh4re::Response::Err { message }, + |()| hive_sh4re::Response::Ok, + ) } hive_sh4re::Request::CreateRepo { repo } => handle_create_repo(agent, repo).await, hive_sh4re::Request::AckTurn => handle_ack_turn(coord, agent), @@ -489,30 +484,28 @@ fn handle_requeue_inflight(coord: &Arc, agent: &str) -> hive_sh4re: } } -/// Unified dispatch for both the per-agent sockets (`privileged = false`) -/// and the manager socket (`privileged = true`). Shared variants go through -/// [`dispatch_shared`]; lifecycle/config + agent-state-query verbs apply the -/// topology/capability guards on the non-privileged path and skip them when -/// privileged; the schedule / meta-input / log verbs are privileged-only. -async fn dispatch( - req: &AgentRequest, - agent: &str, - privileged: bool, - coord: &Arc, -) -> AgentResponse { - if let Some(resp) = dispatch_shared(req, agent, privileged, coord).await { +/// Unified dispatch for every socket connection — per-agent sockets and the +/// (now pure-transport) manager socket alike. There is no privilege bit; +/// authority derives uniformly from the caller's identity: subtree-relational +/// verbs (lifecycle/config/logs) require the caller to be an ancestor of the +/// target (`is_descendant_of`, so the root covers all); hive-wide agent-state +/// queries require the `QueryAgentState` capability; hive-wide orchestration +/// verbs (schedules / meta-inputs) require the matching tool-group (the +/// grantable capability). +async fn dispatch(req: &AgentRequest, agent: &str, coord: &Arc) -> AgentResponse { + if let Some(resp) = dispatch_shared(req, agent, coord).await { return resp; } match req { - // Lifecycle + config: topology-gated when `!privileged`, ungated - // (any agent) when privileged. - AgentRequest::Start { name } => handle_start(coord, agent, name, privileged).await, - AgentRequest::Restart { name } => handle_restart(coord, agent, name, privileged).await, - AgentRequest::Kill { name } => handle_kill(coord, agent, name, privileged).await, - AgentRequest::Update { name } => handle_update(coord, agent, name, privileged), + // Lifecycle + config: caller must be an ancestor of the target + // (a parent owns its whole subtree; the root covers every agent). + AgentRequest::Start { name } => handle_start(coord, agent, name).await, + AgentRequest::Restart { name } => handle_restart(coord, agent, name).await, + AgentRequest::Kill { name } => handle_kill(coord, agent, name).await, + AgentRequest::Update { name } => handle_update(coord, agent, name), AgentRequest::ListDescendants => handle_list_descendants(agent).await, AgentRequest::RequestInitConfig { name, description } => { - handle_request_init_config(coord, agent, name, description.clone(), privileged) + handle_request_init_config(coord, agent, name, description.clone()) } AgentRequest::RequestApplyCommit { agent: target_agent, @@ -525,54 +518,58 @@ async fn dispatch( target_agent, commit_ref, description.as_deref(), - privileged, ) .await } - // Agent-state queries: own/child/cap-gated when `!privileged`; - // any-agent + hive-wide (`"*"`) when privileged. + // Agent-state queries: own subtree is free; other agents + the + // hive-wide `"*"` sweep require `QueryAgentState`. AgentRequest::GetLooseEnds { agent: target } => { - handle_get_loose_ends(coord, agent, target.as_deref(), privileged) + handle_get_loose_ends(coord, agent, target.as_deref()) } AgentRequest::CountPendingReminders { agent: target } => { - handle_count_pending_reminders(coord, agent, target.as_deref(), privileged) + handle_count_pending_reminders(coord, agent, target.as_deref()) } AgentRequest::ReminderRollup { since_secs, agent: target, - } => handle_reminder_rollup(coord, agent, target.as_deref(), *since_secs, privileged), - // Everything else is privileged-only (scheduling / meta-inputs / - // logs) or unknown — gated + handled in `dispatch_privileged_only`. - _ => dispatch_privileged_only(req, agent, privileged, coord).await, + } => handle_reminder_rollup(coord, agent, target.as_deref(), *since_secs), + // Orchestration / diagnostics verbs — gated per-verb on tool-group + // membership or topology (see `dispatch_orchestration`). + _ => dispatch_orchestration(req, agent, coord).await, } } -/// Handle the privileged-only verbs (scheduling, meta-input updates, -/// container logs). Reached as the fallback arm of [`dispatch`]: rejects the -/// whole group up front when `!privileged` (these never appear on a per-agent -/// socket), then matches the individual verbs. Any other variant is a -/// host-admin-only / unknown request that's invalid on either socket. -async fn dispatch_privileged_only( +/// Handle the hive-wide orchestration verbs (scheduling, meta-input updates) +/// plus container-log reads. No blanket socket gate: each verb gates on the +/// grantable capability that authorises it — the matching tool-group +/// (`scheduling` / `approvals`), or `is_descendant_of` for `get_logs` +/// (a parent reads its subtree's logs). Any other variant is a host-admin / +/// unknown request invalid on either socket. +async fn dispatch_orchestration( req: &AgentRequest, agent: &str, - privileged: bool, coord: &Arc, ) -> AgentResponse { - if !privileged { - return AgentResponse::Err { - message: "request not available on the agent socket (privileged / manager-only)" - .to_owned(), - }; - } match req { AgentRequest::RequestUpdateMetaInputs { inputs, description, - } => handle_request_update_meta_inputs(coord, inputs, description.as_deref()), + } => { + if let Some(err) = require_group(agent, "approvals", "request update_meta_inputs") { + return err; + } + handle_request_update_meta_inputs(coord, agent, inputs, description.as_deref()) + } AgentRequest::RequestSchedulePrompt(payload) => { + if let Some(err) = require_group(agent, "scheduling", "schedule a prompt") { + return err; + } handle_request_schedule_prompt(coord, agent, payload) } AgentRequest::CancelSchedule { id, targets } => { + if let Some(err) = require_group(agent, "scheduling", "cancel a schedule") { + return err; + } handle_cancel_schedule(coord, agent, *id, targets.as_deref()) } AgentRequest::EditSchedule { @@ -583,25 +580,45 @@ async fn dispatch_privileged_only( next_fire_at_unix, targets_add, targets_remove, - } => handle_edit_schedule( - coord, - agent, - *id, - EditSchedulePatch { - body: body.clone(), - description: description.clone(), - interval_seconds: *interval_seconds, - next_fire_at_unix: *next_fire_at_unix, - targets_add: targets_add.clone(), - targets_remove: targets_remove.clone(), - }, - ), - AgentRequest::ListSchedules => handle_list_schedules(coord), - AgentRequest::FireScheduleNow { id } => handle_fire_schedule_now(coord, agent, *id).await, + } => { + if let Some(err) = require_group(agent, "scheduling", "edit a schedule") { + return err; + } + handle_edit_schedule( + coord, + agent, + *id, + EditSchedulePatch { + body: body.clone(), + description: description.clone(), + interval_seconds: *interval_seconds, + next_fire_at_unix: *next_fire_at_unix, + targets_add: targets_add.clone(), + targets_remove: targets_remove.clone(), + }, + ) + } + AgentRequest::ListSchedules => { + if let Some(err) = require_group(agent, "scheduling", "list schedules") { + return err; + } + handle_list_schedules(coord) + } + AgentRequest::FireScheduleNow { id } => { + if let Some(err) = require_group(agent, "scheduling", "fire a schedule") { + return err; + } + handle_fire_schedule_now(coord, agent, *id).await + } AgentRequest::GetLogs { agent: target, lines, - } => handle_get_logs(target, *lines).await, + } => { + if let Some(err) = require_descendant(agent, target, "read logs of") { + return err; + } + handle_get_logs(target, *lines).await + } // Host-admin-only / unknown variants: never valid on either socket. _ => AgentResponse::Err { message: "request not handled on this socket".to_owned(), @@ -609,27 +626,44 @@ async fn dispatch_privileged_only( } } -/// Topology guard for the agent-socket lifecycle/config tools: the -/// caller must be the direct parent of `target`. Returns `Some(Err)` -/// to short-circuit the dispatch arm when it isn't, `None` when the -/// call is authorised. `action` is the verb phrase for the message -/// (e.g. `"start"`, `"request_apply_commit for"`). -fn require_child(agent: &str, target: &str, action: &str) -> Option { - if crate::topology::children_of(agent) - .iter() - .any(|c| c == target) - { +/// Topology guard for the subtree-relational lifecycle/config/log tools: the +/// `target` must be the caller itself or one of its topology descendants — a +/// parent owns its whole subtree, and the root (`ruth`) covers every agent as +/// a consequence, with no positional/hardcoded privilege. Returns `Some(Err)` +/// to short-circuit the dispatch arm when it isn't, `None` when authorised. +/// `action` is the verb phrase for the message (e.g. `"start"`). +fn require_descendant(agent: &str, target: &str, action: &str) -> Option { + if crate::topology::is_descendant_of(target, agent) { None } else { Some(AgentResponse::Err { message: format!( "agent `{agent}` cannot {action} `{target}`: \ - not a direct child in the topology tree" + not in its subtree (topology)" ), }) } } +/// Capability guard for the hive-wide orchestration verbs: the caller must +/// hold the given tool-group. The tool-group (c0re-owned `tool_groups.json`, +/// read server-side via [`crate::tool_groups::groups_for`]) is the grantable +/// capability — granting it to an orchestrator (e.g. the root) authorises +/// these verbs without any positional/hardcoded privilege. `action` is the +/// verb phrase for the message. +fn require_group(agent: &str, group: &str, action: &str) -> Option { + if crate::tool_groups::groups_for(agent) + .iter() + .any(|g| g == group) + { + None + } else { + Some(AgentResponse::Err { + message: format!("agent `{agent}` cannot {action}: requires the `{group}` tool group"), + }) + } +} + /// Topology guard for `request_init_config` / `request_apply_commit`, /// which may legitimately target a child that does not exist *yet* /// (spawning a brand-new sub-agent). The caller may act on a @@ -639,57 +673,54 @@ fn require_child(agent: &str, target: &str, action: &str) -> Option Option { if let Some(reason) = crate::dashboard::validate_agent_name(target) { return Some(AgentResponse::Err { message: format!("agent `{agent}` cannot {action} `{target}`: {reason}"), }); } - match crate::topology::read().get(target) { - // brand-new name — requester becomes the parent on approval. - None => None, - // already our direct child — re-init / config update path. - Some(Some(p)) if p == agent => None, - // owned by someone else, or a root agent — refuse. - Some(_) => Some(AgentResponse::Err { + // brand-new name (absent from topology) — requester becomes the parent on + // approval; allowed for any caller. + if !crate::topology::read().contains_key(target) { + return None; + } + // existing agent — allowed only if it's in the caller's subtree + // (re-init / config update of an agent the caller owns; the root owns + // every existing agent). Refuses an agent outside the caller's subtree + // so one agent can't hijack another's config. + if crate::topology::is_descendant_of(target, agent) { + None + } else { + Some(AgentResponse::Err { message: format!( "agent `{agent}` cannot {action} `{target}`: it already exists \ - under a different parent in the topology tree" + outside its subtree in the topology tree" ), - }), + }) } } -/// `GetLooseEnds` — read the target's loose ends. The privileged (manager) -/// socket may target any named agent, defaults to itself, and may pass -/// `"*"` for a hive-wide sweep (gated on `query_agent_state`). The -/// non-privileged path resolves through `resolve_agent_state_target` -/// (own / direct-child free; other agents need `query_agent_state`; `"*"` -/// is rejected). +/// `GetLooseEnds` — read the target's loose ends. `None` / own / a subtree +/// descendant resolve freely (a parent sees its subtree, the root sees all); +/// any other named agent needs `QueryAgentState`; `"*"` is a hive-wide sweep +/// gated on `QueryAgentState`. fn handle_get_loose_ends( coord: &Arc, agent: &str, target: Option<&str>, - privileged: bool, ) -> AgentResponse { - let result = if privileged { - match target { - Some("*") => { - if !crate::capabilities::has_cap(agent, hive_sh4re::Capability::QueryAgentState) { - return AgentResponse::Err { - message: "query_agent_state capability required for hive-wide loose ends" - .to_owned(), - }; - } - crate::loose_ends::hive_wide(coord) - } - Some(name) => crate::loose_ends::for_agent(coord, name), - None => crate::loose_ends::for_agent(coord, agent), + let result = if target == Some("*") { + if !crate::capabilities::has_cap(agent, hive_sh4re::Capability::QueryAgentState) { + return AgentResponse::Err { + message: "query_agent_state capability required for hive-wide loose ends" + .to_owned(), + }; } + crate::loose_ends::hive_wide(coord) } else { match resolve_agent_state_target(agent, target) { Ok(name) => crate::loose_ends::for_agent(coord, name), @@ -704,31 +735,14 @@ fn handle_get_loose_ends( } } -/// Resolve the target agent for `CountPendingReminders` / `ReminderRollup`. -/// Privileged (manager) callers may name any agent (or default to -/// themselves); non-privileged callers go through the topology/capability -/// gate in `resolve_agent_state_target`. -fn resolve_reminder_target<'a>( - caller: &'a str, - target: Option<&'a str>, - privileged: bool, -) -> Result<&'a str, String> { - if privileged { - Ok(target.unwrap_or(caller)) - } else { - resolve_agent_state_target(caller, target) - } -} - -/// `CountPendingReminders` — resolve the target then count its pending -/// reminders. +/// `CountPendingReminders` — resolve the target (own / subtree free, else +/// `QueryAgentState`) then count its pending reminders. fn handle_count_pending_reminders( coord: &Arc, agent: &str, target: Option<&str>, - privileged: bool, ) -> AgentResponse { - match resolve_reminder_target(agent, target, privileged) { + match resolve_agent_state_target(agent, target) { Ok(name) => match coord.broker.count_pending_reminders_for(name) { Ok(count) => AgentResponse::PendingRemindersCount { count }, Err(e) => AgentResponse::Err { @@ -739,16 +753,16 @@ fn handle_count_pending_reminders( } } -/// `ReminderRollup` — resolve the target then roll up its reminders -/// fired in the last `since_secs`. +/// `ReminderRollup` — resolve the target (own / subtree free, else +/// `QueryAgentState`) then roll up its reminders fired in the last +/// `since_secs`. fn handle_reminder_rollup( coord: &Arc, agent: &str, target: Option<&str>, since_secs: u64, - privileged: bool, ) -> AgentResponse { - match resolve_reminder_target(agent, target, privileged) { + match resolve_agent_state_target(agent, target) { Ok(name) => match coord.broker.reminder_rollup_for(name, since_secs) { Ok(stats) => AgentResponse::ReminderRollup(stats), Err(e) => AgentResponse::Err { @@ -759,19 +773,13 @@ fn handle_reminder_rollup( } } -/// `Start` — start a container, kicking its next turn. Non-privileged -/// callers may only start a direct child; the privileged (manager) socket -/// may start any agent. -async fn handle_start( - coord: &Arc, - agent: &str, - name: &str, - privileged: bool, -) -> AgentResponse { - if !privileged && let Some(err) = require_child(agent, name, "start") { +/// `Start` — start a container, kicking its next turn. The caller must be an +/// ancestor of `name` in the topology (the root covers every agent). +async fn handle_start(coord: &Arc, agent: &str, name: &str) -> AgentResponse { + if let Some(err) = require_descendant(agent, name, "start") { return err; } - tracing::info!(%agent, %name, %privileged, "start container"); + tracing::info!(%agent, %name, "start container"); match crate::lifecycle::start(name).await { Ok(()) => { coord.kick_agent(name, "container started"); @@ -783,17 +791,11 @@ async fn handle_start( } } -/// `Restart` — enqueue a restart for a container. Non-privileged callers -/// may only restart a direct child; the privileged (manager) socket may -/// restart any agent. The infra-container branch is orthogonal: it is gated -/// on the `infra_admin` capability (applies to privileged + non-privileged -/// callers alike) and audited, so it stays ahead of the topology guard. -async fn handle_restart( - coord: &Arc, - agent: &str, - name: &str, - privileged: bool, -) -> AgentResponse { +/// `Restart` — enqueue a restart for a container. The caller must be an +/// ancestor of `name` in the topology. The infra-container branch is +/// orthogonal: it is gated on the `infra_admin` capability and audited, so it +/// stays ahead of the topology guard. +async fn handle_restart(coord: &Arc, agent: &str, name: &str) -> AgentResponse { // Infra-container restart: an agent holding the `infra_admin` // capability can restart a hive infrastructure container (hive-ci / // hive-gateway / hive-forge / hive-matrix) by passing its name to the @@ -803,10 +805,10 @@ async fn handle_restart( if let Ok(container) = name.parse::() { return handle_restart_infra(coord, agent, container).await; } - if !privileged && let Some(err) = require_child(agent, name, "restart") { + if let Some(err) = require_descendant(agent, name, "restart") { return err; } - tracing::info!(%agent, %name, %privileged, "enqueue restart"); + tracing::info!(%agent, %name, "enqueue restart"); coord.rebuild_queue.enqueue( crate::rebuild_queue::QueueKind::Restart, name.to_owned(), @@ -869,19 +871,13 @@ async fn handle_restart_infra( } } -/// `Kill` — kill a container, unregister it, notify the manager. -/// Non-privileged callers may only kill a direct child; the privileged -/// (manager) socket may kill any agent. -async fn handle_kill( - coord: &Arc, - agent: &str, - name: &str, - privileged: bool, -) -> AgentResponse { - if !privileged && let Some(err) = require_child(agent, name, "kill") { +/// `Kill` — kill a container, unregister it, notify the manager. The caller +/// must be an ancestor of `name` in the topology. +async fn handle_kill(coord: &Arc, agent: &str, name: &str) -> AgentResponse { + if let Some(err) = require_descendant(agent, name, "kill") { return err; } - tracing::info!(%agent, %name, %privileged, "kill container"); + tracing::info!(%agent, %name, "kill container"); let result: anyhow::Result<()> = async { crate::lifecycle::kill(name).await?; coord.unregister_agent(name); @@ -901,19 +897,13 @@ async fn handle_kill( } } -/// `Update` — enqueue a rebuild for a container. Non-privileged callers may -/// only rebuild a direct child; the privileged (manager) socket may rebuild -/// any agent. -fn handle_update( - coord: &Arc, - agent: &str, - name: &str, - privileged: bool, -) -> AgentResponse { - if !privileged && let Some(err) = require_child(agent, name, "rebuild") { +/// `Update` — enqueue a rebuild for a container. The caller must be an +/// ancestor of `name` in the topology. +fn handle_update(coord: &Arc, agent: &str, name: &str) -> AgentResponse { + if let Some(err) = require_descendant(agent, name, "rebuild") { return err; } - tracing::info!(%agent, %name, %privileged, "enqueue rebuild"); + tracing::info!(%agent, %name, "enqueue rebuild"); coord.rebuild_queue.enqueue( crate::rebuild_queue::QueueKind::Rebuild, name.to_owned(), @@ -963,24 +953,22 @@ async fn handle_list_descendants(agent: &str) -> AgentResponse { AgentResponse::Containers { containers } } -/// `RequestInitConfig` — queue an `InitConfig` approval for an agent. -/// Non-privileged callers may only init a (brand-new or existing) direct -/// child and are recorded as its parent; the privileged (manager) socket -/// may init any agent and records no explicit parent edge (the new agent -/// lands at `topology::reconcile`'s default position on first spawn). +/// `RequestInitConfig` — queue an `InitConfig` approval for an agent. The +/// `name` must be brand-new (absent from the topology) or already in the +/// caller's subtree; the requester is recorded as the new agent's parent (the +/// root requesting a new agent → a top-level agent, matching reconcile's +/// default). fn handle_request_init_config( coord: &Arc, agent: &str, name: &str, description: Option, - privileged: bool, ) -> AgentResponse { - if !privileged && let Some(err) = require_new_child(agent, name, "request_init_config for") { + if let Some(err) = require_new_child(agent, name, "request_init_config for") { return err; } - tracing::info!(%agent, %name, %privileged, "request_init_config"); - let parent = if privileged { None } else { Some(agent) }; - match submit_init_config(coord, name, parent, description) { + tracing::info!(%agent, %name, "request_init_config"); + match submit_init_config(coord, name, Some(agent), description) { Ok(_id) => AgentResponse::Ok, Err(e) => AgentResponse::Err { message: format!("{e:#}"), @@ -988,23 +976,19 @@ fn handle_request_init_config( } } -/// `RequestApplyCommit` — queue an apply-commit approval for an agent. -/// Non-privileged callers may only target a direct child; the privileged -/// (manager) socket may target any agent. +/// `RequestApplyCommit` — queue an apply-commit approval for an agent. The +/// target must be in the caller's subtree (the root covers every agent). async fn handle_request_apply_commit( coord: &Arc, agent: &str, target_agent: &str, commit_ref: &str, description: Option<&str>, - privileged: bool, ) -> AgentResponse { - if !privileged - && let Some(err) = require_new_child(agent, target_agent, "request_apply_commit for") - { + if let Some(err) = require_new_child(agent, target_agent, "request_apply_commit for") { return err; } - tracing::info!(%agent, %target_agent, %commit_ref, %privileged, "request_apply_commit"); + tracing::info!(%agent, %target_agent, %commit_ref, "request_apply_commit"); match submit_apply_commit(coord, target_agent, commit_ref, description).await { Ok((id, sha)) => { tracing::info!(%id, %target_agent, %sha, "apply_commit approval queued"); @@ -1352,16 +1336,13 @@ fn auto_reminder_path(agent: &str) -> String { format!("/agents/{agent}/state/reminders/auto-{ts_ns}.md") } -/// Resolve the target agent name for `GetLooseEnds`, `CountPendingReminders`, -/// and `ReminderRollup` on the agent socket. Rules: +/// Resolve the target agent name for a *named* `GetLooseEnds` / +/// `CountPendingReminders` / `ReminderRollup` query. Rules: /// -/// - `None` → caller's own threads (always allowed). -/// - `Some(caller)` → same as `None`. -/// - `Some("")` where child is a direct descendant of caller per -/// `topology.json` → allowed without any extra capability. -/// - `Some("")` where other is not a child → requires the -/// `query_agent_state` capability; returns an error otherwise. -/// - `Some("*")` → always rejected (hive-wide scans are manager-only). +/// - `None` (or `Some(caller)`) → the caller's own threads (always allowed). +/// - `Some(descendant)` in the caller's subtree (the root's subtree is the whole hive) → allowed, no extra capability. +/// - `Some(other)` outside the subtree → requires the `query_agent_state` capability; error otherwise. +/// - `Some("*")` → rejected here; the hive-wide sweep is handled by `handle_get_loose_ends` under the same `query_agent_state` gate. fn resolve_agent_state_target<'a>( caller: &'a str, target: Option<&'a str>, @@ -1369,26 +1350,21 @@ fn resolve_agent_state_target<'a>( match target { None => Ok(caller), Some("*") => Err( - "hive-wide query (agent=\"*\") is not available on the agent socket; \ - use the manager socket for swarm-wide scans" + "hive-wide query (agent=\"*\") is only valid for loose-ends; \ + not available for this query" .to_owned(), ), Some(name) => { - if name == caller { - return Ok(caller); - } - // Direct children are visible to their parent without extra capability. - if crate::topology::children_of(caller) - .iter() - .any(|c| c == name) - { + // Own subtree (the root covers all) is visible without extra + // capability; `is_descendant_of` returns true for `name == caller`. + if crate::topology::is_descendant_of(name, caller) { return Ok(name); } if crate::capabilities::has_cap(caller, hive_sh4re::Capability::QueryAgentState) { Ok(name) } else { Err(format!( - "agent `{caller}` cannot query `{name}`: not a direct child and \ + "agent `{caller}` cannot query `{name}`: not in its subtree and \ `query_agent_state` capability is not granted" )) } @@ -1421,11 +1397,11 @@ fn resolve_due_at(timing: &hive_sh4re::ReminderTiming) -> anyhow::Result { } // --------------------------------------------------------------------------- -// Privileged-only handlers + submit/schedule helpers (manager socket). -// Reached only via the `require_privileged!()`-gated arms in `dispatch`, or -// re-used by the agent-socket lifecycle handlers (`submit_init_config` / -// `submit_apply_commit`) and the dashboard (`schedule_to_wire_public` / -// `filter_ghost_schedule_targets`). +// Orchestration handlers + submit/schedule helpers. +// The schedule / meta-input handlers are reached via the tool-group-gated arms +// in `dispatch_orchestration`; `submit_init_config` / `submit_apply_commit` +// are re-used by the lifecycle handlers; `schedule_to_wire_public` / +// `filter_ghost_schedule_targets` are re-used by the dashboard. // --------------------------------------------------------------------------- /// `RequestUpdateMetaInputs` — queue an `UpdateMetaInputs` approval @@ -1433,6 +1409,7 @@ fn resolve_due_at(timing: &hive_sh4re::ReminderTiming) -> anyhow::Result { /// is involved; the field is the payload the approval handler decodes). fn handle_request_update_meta_inputs( coord: &Arc, + requester: &str, inputs: &[String], description: Option<&str>, ) -> AgentResponse { @@ -1441,12 +1418,12 @@ fn handle_request_update_meta_inputs( } else { inputs.join(", ") }; - tracing::info!(%label, "manager: request_update_meta_inputs"); + tracing::info!(%requester, %label, "request_update_meta_inputs"); let commit_ref = serde_json::to_string(inputs).unwrap_or_default(); let id = match coord .approvals .submit_kind( - MANAGER_AGENT, + requester, hive_sh4re::ApprovalKind::UpdateMetaInputs, &commit_ref, description, @@ -1463,7 +1440,7 @@ fn handle_request_update_meta_inputs( tracing::info!(%id, %label, "update_meta_inputs approval queued"); coord.emit_approval_added( id, - MANAGER_AGENT, + requester, "update_meta_inputs", None, None, @@ -1797,10 +1774,10 @@ pub(crate) fn validate_commit_ref(commit_ref: &str) -> Result<()> { /// field (unused for `InitConfig` otherwise — same pattern /// `UpdateMetaInputs` uses to carry its inputs JSON) and consumed in /// `run_approval_init_config` to write the `child -> parent` topology -/// edge. The agent socket passes the requesting agent. `None` (the -/// privileged manager socket) writes no explicit edge — the new agent -/// lands at `topology::reconcile`'s default position when it first -/// spawns, so no caller has to name a specific root agent here. +/// edge. Callers pass the requesting agent, so the requester becomes the +/// new agent's parent (the root requesting a new agent → a top-level agent, +/// matching `topology::reconcile`'s default). `None` writes no explicit +/// edge (reconcile-default placement) — retained for that fallback. pub(crate) fn submit_init_config( coord: &Arc, name: &str, @@ -2172,24 +2149,13 @@ mod tests { } #[test] - fn reminder_target_privileged_resolves_any_or_self() { - // Privileged (manager) callers may name any agent, or default to - // themselves — no topology/capability gate on this path. - assert_eq!( - resolve_reminder_target("ruth", Some("iris"), true), - Ok("iris") - ); - assert_eq!(resolve_reminder_target("ruth", None, true), Ok("ruth")); - } - - #[test] - fn reminder_target_non_privileged_self_is_free() { - // The non-privileged path defers to `resolve_agent_state_target`; - // the self / default case needs no topology state. - assert_eq!(resolve_reminder_target("iris", None, false), Ok("iris")); - assert_eq!( - resolve_reminder_target("iris", Some("iris"), false), - Ok("iris") - ); + fn resolve_agent_state_target_self_and_default_are_free() { + // No topology/capability state needed for these: `None` and the + // caller's own name resolve to the caller (`is_descendant_of` short- + // circuits to true when candidate == ancestor); `"*"` is rejected + // (the hive-wide sweep is handled by the loose-ends caller instead). + assert_eq!(resolve_agent_state_target("iris", None), Ok("iris")); + assert_eq!(resolve_agent_state_target("iris", Some("iris")), Ok("iris")); + assert!(resolve_agent_state_target("iris", Some("*")).is_err()); } }