diff --git a/docs/tools/scheduling.md b/docs/tools/scheduling.md index 15291fb9..30641cb5 100644 --- a/docs/tools/scheduling.md +++ b/docs/tools/scheduling.md @@ -10,9 +10,10 @@ targets only yourself — use `remind` for an unapproved self-wake. The other four verbs need no approval: holding the `scheduling` tool group is the whole gate. -Authorization is one rule for all four verbs: you reach every schedule in -the hive, whoever owns it. `list_schedules` shows the same set, so the -snapshot only ever shows schedules you could also cancel. +Authorization is one rule for all four verbs: you reach schedules you own +and any owned by an agent in your topology subtree. `list_schedules` +applies it too, so the snapshot only ever shows schedules you could also +cancel. ### `request_schedule_prompt(targets, body, first_fire_at_unix, interval_seconds?, description?)` @@ -60,11 +61,13 @@ consumes one-shot schedules and cancels them afterwards. ### `list_schedules()` -Snapshot every schedule in the hive (active, and cancelled but not yet -reaped) — same read scope as the rest of this group. Returns id, owner, -body, per-target `last_fired_at` and `last_result`, -`next_fire_at_unix`, `interval_seconds`. Use to look up an id before -cancelling, or to audit upcoming wake-ups across the hive. +Snapshot the schedules you're authorized to see (active, and cancelled +but not yet reaped) — same read scope as the rest of this group: your +own, plus any owned by a sub-agent in your subtree (everything, for +the operator). Returns id, owner, body, per-target `last_fired_at` +and `last_result`, `next_fire_at_unix`, `interval_seconds`. Use to +look up an id before cancelling, or to audit upcoming wake-ups in +your subtree. ## `read_host_journal` capability diff --git a/hive-agent-mcp/README.md b/hive-agent-mcp/README.md index 3f6f086a..e154e58d 100644 --- a/hive-agent-mcp/README.md +++ b/hive-agent-mcp/README.md @@ -10,7 +10,8 @@ would hit. HTTP is the sole transport; there is no stdio mode here. This is where the core hyperhive tool surface lives: `send`, `recv`, `remind`, `get_loose_ends`, `set_status`, -`get_agent_meta`, scheduling, and the approval-request tools. Reach +`get_agent_meta`, lifecycle (`kill`/`start`/`restart`/`update` on the +agent's own subtree), scheduling, and the approval-request tools. Reach for this crate when you're adding or changing a built-in tool rather than an `extraMcpServers` add-on — those are separate stdio bridges (see `hive-bash-mcp`, `hive-matrix-mcp`) that dial the harness socket diff --git a/hive-agent-mcp/src/mcp/mod.rs b/hive-agent-mcp/src/mcp/mod.rs index 455df407..bdc99a4c 100644 --- a/hive-agent-mcp/src/mcp/mod.rs +++ b/hive-agent-mcp/src/mcp/mod.rs @@ -639,7 +639,8 @@ impl AgentServer { keep their next_fire_at unchanged (the manual fire is additive). One-shot \ schedules are CONSUMED by the manual fire (cancelled afterwards): the operator's \ intent on a one-shot is 'send this now, the scheduled time was wrong'. \n\n\ - Authorization mirrors `cancel_schedule`: you can fire any schedule, whoever owns it." + Authorization mirrors `cancel_schedule`: you can fire your own schedules + any \ + owned by a sub-agent in your subtree per topology.json." )] async fn fire_schedule_now(&self, Parameters(args): Parameters) -> String { let log = format!("{args:?}"); @@ -661,7 +662,8 @@ impl AgentServer { whole schedule (all recipients flipped). With a non-empty `targets` list, cancels \ just those recipients; the schedule keeps firing for any remaining active targets \ and auto-cancels its parent row when every target is cancelled. \n\n\ - Authorization: you can cancel any schedule, whoever owns it." + Authorization: the manager can cancel its own schedules + any schedule whose \ + owner is one of its sub-agents per topology.json. Other owners are refused." )] async fn cancel_schedule(&self, Parameters(args): Parameters) -> String { let log = format!("{args:?}"); @@ -691,8 +693,8 @@ impl AgentServer { swap a target atomically. Re-adding a previously-removed target starts a fresh \ per-target history (drops the tombstone). Draining all targets auto-cancels the \ parent schedule. \n\n\ - Authorization mirrors `cancel_schedule` / `fire_schedule_now`: you can edit any \ - schedule, whoever owns it. \ + Authorization mirrors `cancel_schedule` / `fire_schedule_now`: you can edit your \ + own schedules + any owned by a sub-agent in your subtree per topology.json. \ Refuses cancelled schedules (the row's terminal — submit a fresh one)." )] async fn edit_schedule(&self, Parameters(args): Parameters) -> String { @@ -719,11 +721,12 @@ impl AgentServer { } #[tool( - description = "List every scheduled prompt in the hive, whoever owns it — active and \ + description = "List the scheduled prompts you're authorized to see — your own, plus \ + any owned by a sub-agent in your subtree (everything, for the operator) — active and \ cancelled-but-not-yet-reaped. Returns schedule id, owner, body, target set with \ per-target last_fired_at + last_result, next fire time, recurring interval. \ Use this to look up an id before calling `cancel_schedule`, or to audit what \ - the hive is going to be woken up about next." + your subtree is going to be woken up about next." )] async fn list_schedules(&self) -> String { run_tool_envelope("list_schedules", String::new(), async move { diff --git a/hive-agent/prompts/system.md b/hive-agent/prompts/system.md index 13221bcc..53d7d18b 100644 --- a/hive-agent/prompts/system.md +++ b/hive-agent/prompts/system.md @@ -4,14 +4,14 @@ Tools (hyperhive surface). Full signature + behavior for each comes from the too - **Inbox / messaging** (always available): `mcp__hyperhive__recv`, `mcp__hyperhive__ack_until`, `mcp__hyperhive__send`, `mcp__hyperhive__get_loose_ends`, `mcp__hyperhive__cancel_loose_end`, `mcp__hyperhive__mark_todos_done`, `mcp__hyperhive__remind`, `mcp__hyperhive__set_status`, `mcp__hyperhive__get_agent_meta`. One habit worth internalizing beyond the tool descriptions themselves: prefer ending the turn over repeatedly polling `recv` when idle (only turn-boundaries observe in-container todo wakes — bash-task completions, matrix unread, forge activity — and ending the turn is also your checkpoint). For a large todo backlog (`get_loose_ends` caps at 40 rows), clear reviewed ids in bulk with `mark_todos_done` rather than cancelling one at a time — there's no blind range-clear, only ids you've actually looked at. - **Extra MCP tools** (some agents only): `mcp____` — agent-specific (matrix client, scraper, db connector, etc.) declared in your `agent.nix` under `hyperhive.extraMcpServers`. First-class tools, already operator-approved at deploy time. -- **Scheduling** (_requires `scheduling` tool group_): `request_schedule_prompt` (queues an approval), `cancel_schedule`, `fire_schedule_now`, `edit_schedule`, `list_schedules` (these four don't need approval — you can manage any schedule, whoever owns it). +- **Scheduling** (_requires `scheduling` tool group_): `request_schedule_prompt` (queues an approval), `cancel_schedule`, `fire_schedule_now`, `edit_schedule`, `list_schedules` (these four don't need approval — you can manage schedules you own or that a sub-agent in your subtree owns). - **Diagnostics**: `get_host_journal` (_requires `read_host_journal` capability_). Need new packages, env vars, or other NixOS config for yourself? You can't edit your own config directly — open a PR on your `agent-configs/` repo, or contact the operator directly. Config repos live at `/agents/{label}/config/` (read-only inside your container). All changes flow through operator-approved commits. Your config repo is mounted **read-only** at `/agents/{label}/config/` — `agent.nix` plus whatever extra files define you (declared packages, env vars, MCP servers). Read it to see exactly what defines you before asking for a change, so you can point at the precise file and line. -Approval boundary: starting, stopping and rebuilding containers is the operator's — ask them when any agent needs one. _Creating_ a new agent is not something you can do from here — ask the operator, who scaffolds the new agent's config repo and spawns it from the dashboard. _Changing_ any agent's config is not a tool call at all — it's a forge PR on the agent's `agent-configs/` repo, which queues a `MergeConfigPr` approval on open/update. The operator only signs off on changes; you run the day-to-day. +Approval boundary: starting, stopping and rebuilding containers is the operator's — ask them when an agent in your subtree needs one. _Creating_ a new agent is not something you can do from here — ask the operator, who scaffolds the new agent's config repo and spawns it from the dashboard. _Changing_ any agent's config is not a tool call at all — it's a forge PR on the agent's `agent-configs/` repo, which queues a `MergeConfigPr` approval on open/update. The operator only signs off on changes; you run the day-to-day. Messages from sender `system` are hyperhive helper events (JSON body, `event` field discriminates): `approval_resolved`, `container_crash`, `needs_update`. Use these to react to lifecycle changes: diff --git a/hive-c0re/src/agent_config/topology.rs b/hive-c0re/src/agent_config/topology.rs index 8dbff20c..cc74f4cc 100644 --- a/hive-c0re/src/agent_config/topology.rs +++ b/hive-c0re/src/agent_config/topology.rs @@ -16,17 +16,17 @@ //! ## Graph representation //! //! The on-disk format stays as a flat JSON map `name → parent | null` -//! (small, git-diffable). In-memory, cycle detection uses a [`petgraph`] -//! directed graph where each edge runs +//! (small, git-diffable). In-memory, heavy algorithms (descendant checks, +//! cycle detection) use a [`petgraph`] directed graph where each edge runs //! **parent → child**. This replaces the ad-hoc bounded walks that existed -//! before: petgraph's `is_cyclic_directed` -//! is correct for graphs of any depth (no 32-hop ceiling) and well-tested. +//! before: petgraph's `has_path_connecting` / `is_cyclic_directed` +//! are correct for graphs of any depth (no 32-hop ceiling) and well-tested. //! The graph is built on demand from the flat map; it is not cached across //! calls (the map is small and disk I/O dominates anyway). use std::collections::BTreeMap; -use petgraph::algo::is_cyclic_directed; +use petgraph::algo::{has_path_connecting, is_cyclic_directed}; use petgraph::graph::{DiGraph, NodeIndex}; use std::path::PathBuf; @@ -141,10 +141,27 @@ pub fn resolve_recipient_in( } } +/// True when `candidate` is `ancestor` or any descendant of +/// `ancestor` per the current on-disk topology. +/// +/// Delegates to [`is_descendant_of_in`] on the result of [`read`] so +/// the algorithm is the same petgraph BFS used everywhere else. No +/// depth limit — the 32-hop bounded walk this replaced was correct for +/// any plausible hive but carried a latent ceiling; this has none. +/// +/// Used by the cancel-authorization checks in `socket_server` to enforce +/// "managers can cancel anything their subtree owns." +#[must_use] +pub fn is_descendant_of(candidate: &str, ancestor: &str) -> bool { + is_descendant_of_in(&read(), candidate, ancestor) +} + /// Build an in-memory petgraph directed graph from the topology map. /// /// Edges run **parent → child** so that: /// - `children_of(name)` = outgoing neighbours of `name`'s node +/// - `is_descendant_of(candidate, ancestor)` = path exists from `ancestor` +/// to `candidate` via `has_path_connecting` /// - cycle detection = `is_cyclic_directed` after a speculative edge insert /// /// Returns the graph and a `BTreeMap` for O(log n) @@ -174,6 +191,28 @@ fn build_graph( (graph, idx) } +/// Return true when `candidate` is a descendant of `ancestor` in the +/// given topology map. Uses petgraph BFS/DFS (`has_path_connecting`) +/// — no depth limit and no manually bounded walk. Pure; no disk I/O. +/// +/// Same semantics as the disk-reading [`is_descendant_of`]: a node is +/// considered a descendant of itself (`candidate == ancestor` → true). +#[must_use] +pub fn is_descendant_of_in( + topo: &BTreeMap>, + candidate: &str, + ancestor: &str, +) -> bool { + if candidate == ancestor { + return true; + } + let (graph, idx) = build_graph(topo); + let (Some(&anc_ni), Some(&cand_ni)) = (idx.get(ancestor), idx.get(candidate)) else { + return false; + }; + has_path_connecting(&graph, anc_ni, cand_ni, None) +} + /// Persist the topology map. Sorted JSON output (`BTreeMap` is sorted by /// key) keeps git diffs minimal across re-writes. Best-effort — /// returns an `io::Error` so callers can decide whether a failure @@ -703,6 +742,75 @@ mod tests { assert!(top_level_agents_in(&topo).is_empty()); } + // ----------------------------------------------------------------------- + // is_descendant_of_in tests (petgraph-backed; pure / no disk I/O) + // ----------------------------------------------------------------------- + + #[test] + fn is_descendant_of_in_self_is_true() { + let topo = topo_three_level(); + assert!(is_descendant_of_in(&topo, "alice", "alice")); + assert!(is_descendant_of_in( + &topo, + crate::lifecycle::MANAGER_NAME, + crate::lifecycle::MANAGER_NAME + )); + } + + #[test] + fn is_descendant_of_in_direct_child() { + let topo = topo_three_level(); + // alice is a direct child of manager. + assert!(is_descendant_of_in( + &topo, + "alice", + crate::lifecycle::MANAGER_NAME + )); + } + + #[test] + fn is_descendant_of_in_grandchild() { + let topo = topo_three_level(); + // bob is manager → alice → bob; should be reachable from manager. + assert!(is_descendant_of_in( + &topo, + "bob", + crate::lifecycle::MANAGER_NAME + )); + assert!(is_descendant_of_in( + &topo, + "carol", + crate::lifecycle::MANAGER_NAME + )); + } + + #[test] + fn is_descendant_of_in_parent_not_descendant_of_child() { + let topo = topo_three_level(); + // alice is NOT a descendant of bob (alice is bob's grandparent). + assert!(!is_descendant_of_in(&topo, "alice", "bob")); + assert!(!is_descendant_of_in( + &topo, + crate::lifecycle::MANAGER_NAME, + "alice" + )); + } + + #[test] + fn is_descendant_of_in_sibling_is_not_descendant() { + let topo = topo_three_level(); + // bob and carol are siblings under alice; neither descends from the other. + assert!(!is_descendant_of_in(&topo, "bob", "carol")); + assert!(!is_descendant_of_in(&topo, "carol", "bob")); + } + + #[test] + fn is_descendant_of_in_unknown_is_false() { + let topo = topo_three_level(); + assert!(!is_descendant_of_in(&topo, "nobody", "alice")); + assert!(!is_descendant_of_in(&topo, "alice", "nobody")); + } + // ----------------------------------------------------------------------- // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps) // ----------------------------------------------------------------------- diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index 6f564921..2b808f8c 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -4,10 +4,10 @@ //! 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: -//! capabilities for the hive-wide queries and tool-group membership for the -//! orchestration verbs. An agent-targeting verb may name any agent, so `ruth` -//! reaches every agent exactly the way every other agent does, not via any -//! hardcoded name match. +//! 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; @@ -87,9 +87,10 @@ pub fn start(agent: &str, socket_path: &Path, coord: Arc) -> Result /// 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 the capabilities / -/// tool-groups it holds, identical to connecting on a per-agent socket — ruth -/// uses the standard per-agent runtime dir + socket, with no dedicated helpers. +/// ("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 — ruth uses the +/// standard per-agent runtime dir + socket, with no dedicated helpers. pub fn start_manager(coord: Arc) -> Result<()> { use std::os::unix::fs::PermissionsExt as _; let dir = crate::paths::agent_runtime_dir(crate::lifecycle::MANAGER_NAME); diff --git a/hive-c0re/src/socket_server/schedules.rs b/hive-c0re/src/socket_server/schedules.rs index d41d2dfe..c2b07254 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -1,6 +1,6 @@ //! Scheduled-prompt request handlers (`ListSchedules` / //! `RequestSchedulePrompt` / `CancelSchedule` / `EditSchedule` / -//! `FireScheduleNow`) and the +//! `FireScheduleNow`), their shared ownership check, and the //! schedule-to-wire mapping reused by the dashboard //! (`schedule_to_wire_public` / `filter_ghost_schedule_targets`). @@ -10,15 +10,20 @@ use hive_core_agent_sock::Response; use crate::coordinator::Coordinator; -/// `ListSchedules` — snapshot every scheduled prompt. Ownership no longer -/// narrows the view: `CancelSchedule`/`EditSchedule`/`FireScheduleNow` accept -/// any requester, so filtering the listing would only hide rows the requester -/// can act on anyway. +/// `ListSchedules` — snapshot the scheduled prompts `requester` is +/// authorized to see: its own schedules, ones owned by an agent in its +/// subtree, or (for the operator) everything. Uses the same +/// `schedule_authorized` rule `CancelSchedule`/`EditSchedule`/ +/// `FireScheduleNow` already enforce, applied here as a per-row filter +/// instead of a hard reject. pub(super) fn handle_list_schedules(coord: &Arc, requester: &str) -> Response { - tracing::debug!(%requester, "list schedules"); match coord.scheduled_prompts.list() { Ok(schedules) => Response::Schedules { - schedules: schedules.into_iter().map(schedule_to_wire).collect(), + schedules: schedules + .into_iter() + .filter(|s| schedule_authorized(requester, &s.owner)) + .map(schedule_to_wire) + .collect(), }, Err(e) => Response::Err { message: format!("list scheduled prompts: {e:#}"), @@ -103,20 +108,39 @@ pub(super) fn handle_request_schedule_prompt( Response::Ok } -/// Cancel a schedule (whole or per-target). Any requester may cancel any -/// schedule, whoever owns it. +/// Cancel a schedule (whole or per-target). Manager-surface +/// authorization: a manager can cancel its own schedules + any +/// schedule whose owner is one of its sub-agents (topology-walked). +/// The operator surface bypasses this and can cancel anything; +/// agents reaching this path through the manager get the +/// topology-scoped check. pub(super) fn handle_cancel_schedule( coord: &Arc, requester: &str, schedule_id: i64, targets: Option<&[String]>, ) -> Response { - // Existence check only — it buys the caller a clean "not found" instead - // of whatever the cancel itself would say about an absent row. - if let Some(err) = require_schedule(coord, schedule_id) { - return err; + let schedule = match coord.scheduled_prompts.get(schedule_id) { + Ok(Some(s)) => s, + Ok(None) => { + return Response::Err { + message: format!("schedule {schedule_id} not found"), + }; + } + Err(e) => { + return Response::Err { + message: format!("read schedule {schedule_id}: {e:#}"), + }; + } + }; + if !schedule_authorized(requester, &schedule.owner) { + return Response::Err { + message: format!( + "not authorized: {requester} cannot cancel schedule owned by {owner}", + owner = schedule.owner + ), + }; } - tracing::info!(%requester, %schedule_id, "cancel schedule"); let result = match targets { Some(list) if !list.is_empty() => coord .scheduled_prompts @@ -136,18 +160,37 @@ pub(super) fn handle_cancel_schedule( } } -/// Dispatch a `FireScheduleNow` request. Same rule as `CancelSchedule`: any -/// requester, any schedule. The actual fan-out lives in +/// Authorize + dispatch a `FireScheduleNow` request from the +/// manager surface. Same ownership rules as `CancelSchedule`: +/// requester can fire its own schedules + any owned by an agent +/// in its subtree. The actual fan-out lives in /// `scheduled_prompts_worker::fire_now`. pub(super) async fn handle_fire_schedule_now( coord: &Arc, requester: &str, schedule_id: i64, ) -> Response { - if let Some(err) = require_schedule(coord, schedule_id) { - return err; + let schedule = match coord.scheduled_prompts.get(schedule_id) { + Ok(Some(s)) => s, + Ok(None) => { + return Response::Err { + message: format!("schedule {schedule_id} not found"), + }; + } + Err(e) => { + return Response::Err { + message: format!("read schedule {schedule_id}: {e:#}"), + }; + } + }; + if !schedule_authorized(requester, &schedule.owner) { + return Response::Err { + message: format!( + "not authorized: {requester} cannot fire schedule owned by {owner}", + owner = schedule.owner + ), + }; } - tracing::info!(%requester, %schedule_id, "fire schedule now"); // MCP fire_schedule_now stays no-reset (cadence intact); the // reset-timer option is a dashboard-dialog affordance. match crate::scheduled_prompts_worker::fire_now(coord, schedule_id, false).await { @@ -179,11 +222,13 @@ pub(super) struct EditSchedulePatch { pub(super) targets_remove: Option>, } -/// Dispatch an `EditSchedule` patch. Same rule as `CancelSchedule`: any -/// requester, any schedule. Forwards the partial payload to +/// Authorize + dispatch a `EditSchedule` patch. Same ownership +/// rules as `CancelSchedule` — the manager can edit +/// schedules it owns + any owned by an agent in its subtree. +/// Forwards the partial payload to /// `ScheduledPrompts::update` which enforces the cancelled-row / /// zero-interval validation. Returns `Ok` on a clean update; -/// `Err` with the underlying message on any validation +/// `Err` with the underlying message on any auth / validation /// failure so the dashboard can surface it verbatim. pub(super) fn handle_edit_schedule( coord: &Arc, @@ -199,10 +244,27 @@ pub(super) fn handle_edit_schedule( targets_add, targets_remove, } = patch; - if let Some(err) = require_schedule(coord, schedule_id) { - return err; + let schedule = match coord.scheduled_prompts.get(schedule_id) { + Ok(Some(s)) => s, + Ok(None) => { + return Response::Err { + message: format!("schedule {schedule_id} not found"), + }; + } + Err(e) => { + return Response::Err { + message: format!("read schedule {schedule_id}: {e:#}"), + }; + } + }; + if !schedule_authorized(requester, &schedule.owner) { + return Response::Err { + message: format!( + "not authorized: {requester} cannot edit schedule owned by {owner}", + owner = schedule.owner + ), + }; } - tracing::info!(%requester, %schedule_id, "edit schedule"); let patch = crate::scheduled_prompts::UpdateSchedule { body, description, @@ -222,19 +284,23 @@ pub(super) fn handle_edit_schedule( } } -/// Shared existence guard for the three schedule-mutating verbs: `None` when -/// the row is there, `Some(Err)` to short-circuit when it isn't or the read -/// failed. Not a permission check — any requester may act on any schedule. -fn require_schedule(coord: &Arc, schedule_id: i64) -> Option { - match coord.scheduled_prompts.get(schedule_id) { - Ok(Some(_)) => None, - Ok(None) => Some(Response::Err { - message: format!("schedule {schedule_id} not found"), - }), - Err(e) => Some(Response::Err { - message: format!("read schedule {schedule_id}: {e:#}"), - }), +/// Permission check shared by the whole scheduling surface: `requester` +/// can act on a schedule it owns, on one owned by an agent in its +/// subtree (delegated to topology — see `crate::topology::is_descendant_of`), +/// or, as the operator, on anything. `CancelSchedule`/`EditSchedule`/ +/// `FireScheduleNow` apply it as a hard reject; `ListSchedules` applies +/// it as a per-row filter. +fn schedule_authorized(requester: &str, owner: &str) -> bool { + if requester == owner { + return true; } + if requester == hive_sh4re::manager::OPERATOR_RECIPIENT { + return true; + } + // Manager can act on anything owned by an agent in its subtree. For + // the current single-manager topology that covers everything, but + // the check stays correct as the tree grows. + crate::topology::is_descendant_of(owner, requester) } /// Map a `scheduled_prompts::Schedule` to its public wire shape. @@ -330,6 +396,32 @@ mod tests { } } + #[test] + fn schedule_authorized_self_is_true() { + // Disk-free branch — never reaches `is_descendant_of`. + assert!(schedule_authorized("iris", "iris")); + } + + #[test] + fn schedule_authorized_operator_is_true() { + // Disk-free branch — never reaches `is_descendant_of`. + assert!(schedule_authorized( + hive_sh4re::manager::OPERATOR_RECIPIENT, + "iris" + )); + } + + #[test] + fn schedule_authorized_unrelated_requester_is_false() { + // Control: neither self nor operator, and no on-disk topology in + // a test sandbox connects these two names — pins that the + // predicate isn't accidentally stuck at `true`. + assert!(!schedule_authorized( + "definitely-not-the-owner", + "also-not-the-requester" + )); + } + #[test] fn ghost_filter_drops_dead_agents_keeps_live() { let live: std::collections::HashSet = ["iris".to_owned(), "damocles".to_owned()] @@ -353,136 +445,4 @@ mod tests { filter_ghost_schedule_targets(&mut schedules, &live); assert!(schedules[0].targets.is_empty()); } - - // ----------------------------------------------------------------------- - // The scheduling surface permits an unrelated requester - // - // These pin the *decision* the deleted `schedule_authorized` used to make, - // inverted: a requester with no relation to a schedule's owner reaches the - // handler's effect instead of a refusal. They live at the handler level - // because the predicate, its pure `_in` form and all three wrappers are - // gone — the handler is the lowest layer where the decision still exists - // to be observed. - // - // `OWNER` and `STRANGER` are names no hive has, so no `topology.json` on - // any machine running these tests can relate them. Any ancestry check - // reintroduced between the two — disk-reading or pure — therefore answers - // "unrelated", and every assertion below flips from the handler's effect - // to a refusal. - // ----------------------------------------------------------------------- - - /// Owner of the schedule under test. - const OWNER: &str = "schedule-owner-with-no-relations"; - /// Requester acting on it: not the owner, not the operator, and not - /// anybody's ancestor or descendant. - const STRANGER: &str = "requester-with-no-relations"; - - /// A real `Coordinator` over a throwaway sqlite dir. The schedule - /// handlers take `&Arc`, so there is no lighter way in; - /// `open` touches nothing outside the db path it is handed. - fn coordinator() -> (tempfile::TempDir, Arc) { - let dir = tempfile::tempdir().expect("tempdir"); - let coord = Coordinator::open( - &dir.path().join("broker.sqlite"), - crate::coordinator::HiveEnv::default(), - crate::hive_stats::PriceTable::new(), - 1, - ) - .expect("open coordinator"); - (dir, Arc::new(coord)) - } - - /// Insert a schedule owned by [`OWNER`], returning its id. - fn submit_owned_by_other(coord: &Arc) -> i64 { - coord - .scheduled_prompts - .submit(&crate::scheduled_prompts::NewSchedule { - owner: OWNER.to_owned(), - targets: vec!["iris".to_owned()], - body: "ping".to_owned(), - first_fire_at_unix: 100, - interval_seconds: None, - description: None, - source: crate::scheduled_prompts::ScheduleSource::Operator, - }) - .expect("submit") - } - - #[test] - fn cancel_schedule_permits_unrelated_requester() { - let (_dir, coord) = coordinator(); - let id = submit_owned_by_other(&coord); - let resp = handle_cancel_schedule(&coord, STRANGER, id, None); - assert!( - matches!(resp, Response::Ok), - "unrelated requester must be allowed to cancel; got {resp:?}" - ); - // Ok alone could be a refusal that forgot to say so — check the row. - let row = coord - .scheduled_prompts - .get(id) - .expect("read back") - .expect("row present"); - assert!( - row.cancelled_at_unix.is_some(), - "cancel must have taken effect, not just answered Ok" - ); - } - - #[test] - fn edit_schedule_permits_unrelated_requester() { - let (_dir, coord) = coordinator(); - let id = submit_owned_by_other(&coord); - let resp = handle_edit_schedule( - &coord, - STRANGER, - id, - EditSchedulePatch { - body: Some("edited by a stranger".to_owned()), - description: None, - interval_seconds: None, - next_fire_at_unix: None, - targets_add: None, - targets_remove: None, - }, - ); - assert!( - matches!(resp, Response::Ok), - "unrelated requester must be allowed to edit; got {resp:?}" - ); - let row = coord - .scheduled_prompts - .get(id) - .expect("read back") - .expect("row present"); - assert_eq!(row.body, "edited by a stranger"); - } - - #[tokio::test] - async fn fire_schedule_now_permits_unrelated_requester() { - let (_dir, coord) = coordinator(); - let id = submit_owned_by_other(&coord); - let resp = handle_fire_schedule_now(&coord, STRANGER, id).await; - // The fan-out itself is allowed to find no live target (this test has - // no containers) — that is reported inside the report, not as an Err. - assert!( - matches!(resp, Response::Ok), - "unrelated requester must be allowed to fire; got {resp:?}" - ); - } - - #[test] - fn list_schedules_shows_a_schedule_owned_by_someone_else() { - let (_dir, coord) = coordinator(); - let id = submit_owned_by_other(&coord); - let Response::Schedules { schedules } = handle_list_schedules(&coord, STRANGER) else { - panic!("expected a Schedules response"); - }; - // The deleted filter dropped every row the requester was unrelated to, - // which for this requester is all of them. - assert!( - schedules.iter().any(|s| s.id == id && s.owner == OWNER), - "unrelated requester must see another agent's schedule; got {schedules:?}" - ); - } } diff --git a/hive-core-agent-sock/src/lib.rs b/hive-core-agent-sock/src/lib.rs index 8b8c7a47..f7c37212 100644 --- a/hive-core-agent-sock/src/lib.rs +++ b/hive-core-agent-sock/src/lib.rs @@ -157,7 +157,9 @@ pub enum Request { #[serde(default, skip_serializing_if = "Option::is_none")] targets: Option>, }, - /// List every schedule in the hive, whoever owns it. + /// List the schedules the requester is authorized to see: its own, + /// plus any owned by an agent in its subtree (everything, for the + /// operator). ListSchedules, /// *(privileged)* Fire a scheduled prompt out of band immediately. FireScheduleNow { id: i64 },