Watch
0
0
Fork
You've already forked hyperhive
0

Compare commits

...
Author SHA1 Message Date
atlas
8ea1f7daee hive-c0re: assert the scheduling surface permits an unrelated requester
The previous commit deleted `schedule_authorized` and with it three
denial tests, leaving the permit behaviour it introduced unasserted:
nothing in the suite would notice a subtree check creeping back into
the scheduling handlers.

These four pin the inverse of the decision the deleted predicate used
to make. They sit at the handler level because that is the lowest layer
where the decision still exists to be observed — the predicate, its pure
`_in` form and the wrapper are gone, so there is no function left whose
return value *is* the decision, and the layer above (`dispatch`) reaches
these verbs through `require_group("scheduling")`, a different gate that
is still present and not the one under test.

`cancel` and `edit` assert the row afterwards rather than stopping at
`Response::Ok`: a check that silently declines to act while still
answering `Ok` is the regression a response-code assertion misses.
`list_schedules` is the structurally different case — it never rejected,
it filtered per row, so its assertion is that another owner's row is
present at all. `fire` asserts the response only; the fan-out legitimately
finds no live container under test and `fire_now` reports that inside
`Ok(report)`, so the response code is the only honest signal there.

Verified by mutation, not by inspection: reintroducing an ancestry walk
into the four handlers flips all four tests to failing (the three
mutating verbs on the refusal, `list_schedules` on an empty list) and
leaves the two pre-existing ghost-filter tests untouched.

These four cover the complete set of gate points this branch removes.
The roster listing that used to be the fifth is no longer one: `main`
removed `list_containers`, `Request::ListDescendants` and the handler
behind them outright, so there is nothing left there to assert.

Refs #4472
2026-09-21 17:20:44 +02:00
atlas
4121e11d87 hive-c0re: drop the subtree check from the scheduling verbs
The topology predicate `is_descendant_of` gated the four schedule-
managing verbs: a caller could only name a schedule owned by an agent at
or below itself in `topology.json`. Those gates now permit any requester,
so the predicate, its pure `_in` form and the `schedule_authorized`
wrapper built on it are gone rather than left returning a constant. The
other two wrappers went earlier with the verbs they served —
`require_descendant` with the lifecycle MCP verbs in 87970a8c, and
`resolve_agent_state_target` with `get_loose_ends`'s agent parameter.

`require_group(agent, "scheduling", ...)` is untouched and still fires at
dispatch for every one of the five scheduling verbs, so holding the tool
group remains the gate; what goes is the ownership restriction layered on
top of it.

The three schedule-mutating verbs keep their row lookup as a plain
existence check, so a caller naming a schedule that does not exist still
gets `not found` rather than a message from deeper in the cancel path.
`list_schedules` stops filtering per row: it would only have hidden rows
the requester may act on anyway.

Error messages, tool descriptions and docs that described the subtree
relation are reworded — a refusal message naming a topology that no
longer decides anything is worse than none.

The six `is_descendant_of_in` unit tests go with the function they test;
the permit behaviour they leave unasserted is picked up by the next
commit.

Refs #4472
2026-09-21 17:20:44 +02:00
8 changed files with 199 additions and 277 deletions

View file

@ -10,10 +10,9 @@ targets only yourself — use `remind` for an unapproved self-wake. The
other four verbs need no approval: holding the `scheduling` tool group other four verbs need no approval: holding the `scheduling` tool group
is the whole gate. is the whole gate.
Authorization is one rule for all four verbs: you reach schedules you own Authorization is one rule for all four verbs: you reach every schedule in
and any owned by an agent in your topology subtree. `list_schedules` the hive, whoever owns it. `list_schedules` shows the same set, so the
applies it too, so the snapshot only ever shows schedules you could also snapshot only ever shows schedules you could also cancel.
cancel.
### `request_schedule_prompt(targets, body, first_fire_at_unix, interval_seconds?, description?)` ### `request_schedule_prompt(targets, body, first_fire_at_unix, interval_seconds?, description?)`
@ -61,13 +60,11 @@ consumes one-shot schedules and cancels them afterwards.
### `list_schedules()` ### `list_schedules()`
Snapshot the schedules you're authorized to see (active, and cancelled Snapshot every schedule in the hive (active, and cancelled but not yet
but not yet reaped) — same read scope as the rest of this group: your reaped) — same read scope as the rest of this group. Returns id, owner,
own, plus any owned by a sub-agent in your subtree (everything, for body, per-target `last_fired_at` and `last_result`,
the operator). Returns id, owner, body, per-target `last_fired_at` `next_fire_at_unix`, `interval_seconds`. Use to look up an id before
and `last_result`, `next_fire_at_unix`, `interval_seconds`. Use to cancelling, or to audit upcoming wake-ups across the hive.
look up an id before cancelling, or to audit upcoming wake-ups in
your subtree.
## `read_host_journal` capability ## `read_host_journal` capability

View file

@ -10,8 +10,7 @@ would hit. HTTP is the sole transport; there is no stdio mode here.
This is where the core hyperhive tool surface lives: `send`, `recv`, This is where the core hyperhive tool surface lives: `send`, `recv`,
`remind`, `get_loose_ends`, `set_status`, `remind`, `get_loose_ends`, `set_status`,
`get_agent_meta`, lifecycle (`kill`/`start`/`restart`/`update` on the `get_agent_meta`, scheduling, and the approval-request tools. Reach
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 for this crate when you're adding or changing a built-in tool rather
than an `extraMcpServers` add-on — those are separate stdio bridges than an `extraMcpServers` add-on — those are separate stdio bridges
(see `hive-bash-mcp`, `hive-matrix-mcp`) that dial the harness socket (see `hive-bash-mcp`, `hive-matrix-mcp`) that dial the harness socket

View file

@ -639,8 +639,7 @@ impl AgentServer {
keep their next_fire_at unchanged (the manual fire is additive). One-shot \ 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 \ 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\ intent on a one-shot is 'send this now, the scheduled time was wrong'. \n\n\
Authorization mirrors `cancel_schedule`: you can fire your own schedules + any \ Authorization mirrors `cancel_schedule`: you can fire any schedule, whoever owns it."
owned by a sub-agent in your subtree per topology.json."
)] )]
async fn fire_schedule_now(&self, Parameters(args): Parameters<FireScheduleNowArgs>) -> String { async fn fire_schedule_now(&self, Parameters(args): Parameters<FireScheduleNowArgs>) -> String {
let log = format!("{args:?}"); let log = format!("{args:?}");
@ -662,8 +661,7 @@ impl AgentServer {
whole schedule (all recipients flipped). With a non-empty `targets` list, cancels \ whole schedule (all recipients flipped). With a non-empty `targets` list, cancels \
just those recipients; the schedule keeps firing for any remaining active targets \ 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\ and auto-cancels its parent row when every target is cancelled. \n\n\
Authorization: the manager can cancel its own schedules + any schedule whose \ Authorization: you can cancel any schedule, whoever owns it."
owner is one of its sub-agents per topology.json. Other owners are refused."
)] )]
async fn cancel_schedule(&self, Parameters(args): Parameters<CancelScheduleArgs>) -> String { async fn cancel_schedule(&self, Parameters(args): Parameters<CancelScheduleArgs>) -> String {
let log = format!("{args:?}"); let log = format!("{args:?}");
@ -693,8 +691,8 @@ impl AgentServer {
swap a target atomically. Re-adding a previously-removed target starts a fresh \ 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 \ per-target history (drops the tombstone). Draining all targets auto-cancels the \
parent schedule. \n\n\ parent schedule. \n\n\
Authorization mirrors `cancel_schedule` / `fire_schedule_now`: you can edit your \ Authorization mirrors `cancel_schedule` / `fire_schedule_now`: you can edit any \
own schedules + any owned by a sub-agent in your subtree per topology.json. \ schedule, whoever owns it. \
Refuses cancelled schedules (the row's terminal — submit a fresh one)." Refuses cancelled schedules (the row's terminal — submit a fresh one)."
)] )]
async fn edit_schedule(&self, Parameters(args): Parameters<EditScheduleArgs>) -> String { async fn edit_schedule(&self, Parameters(args): Parameters<EditScheduleArgs>) -> String {
@ -721,12 +719,11 @@ impl AgentServer {
} }
#[tool( #[tool(
description = "List the scheduled prompts you're authorized to see — your own, plus \ description = "List every scheduled prompt in the hive, whoever owns it — active and \
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 \ 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. \ 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 \ Use this to look up an id before calling `cancel_schedule`, or to audit what \
your subtree is going to be woken up about next." the hive is going to be woken up about next."
)] )]
async fn list_schedules(&self) -> String { async fn list_schedules(&self) -> String {
run_tool_envelope("list_schedules", String::new(), async move { run_tool_envelope("list_schedules", String::new(), async move {

View file

@ -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. - **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__<server>__<tool>` — 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. - **Extra MCP tools** (some agents only): `mcp__<server>__<tool>` — 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 schedules you own or that a sub-agent in your subtree owns). - **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).
- **Diagnostics**: `get_host_journal` (_requires `read_host_journal` capability_). - **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/<name>` 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. 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/<name>` 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. 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 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/<name>` 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 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/<name>` 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: 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:

View file

@ -16,17 +16,17 @@
//! ## Graph representation //! ## Graph representation
//! //!
//! The on-disk format stays as a flat JSON map `name → parent | null` //! The on-disk format stays as a flat JSON map `name → parent | null`
//! (small, git-diffable). In-memory, heavy algorithms (descendant checks, //! (small, git-diffable). In-memory, cycle detection uses a [`petgraph`]
//! cycle detection) use a [`petgraph`] directed graph where each edge runs //! directed graph where each edge runs
//! **parent → child**. This replaces the ad-hoc bounded walks that existed //! **parent → child**. This replaces the ad-hoc bounded walks that existed
//! before: petgraph's `has_path_connecting` / `is_cyclic_directed` //! before: petgraph's `is_cyclic_directed`
//! are correct for graphs of any depth (no 32-hop ceiling) and well-tested. //! is 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 //! 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). //! calls (the map is small and disk I/O dominates anyway).
use std::collections::BTreeMap; use std::collections::BTreeMap;
use petgraph::algo::{has_path_connecting, is_cyclic_directed}; use petgraph::algo::is_cyclic_directed;
use petgraph::graph::{DiGraph, NodeIndex}; use petgraph::graph::{DiGraph, NodeIndex};
use std::path::PathBuf; use std::path::PathBuf;
@ -141,27 +141,10 @@ 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. /// Build an in-memory petgraph directed graph from the topology map.
/// ///
/// Edges run **parent → child** so that: /// Edges run **parent → child** so that:
/// - `children_of(name)` = outgoing neighbours of `name`'s node /// - `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 /// - cycle detection = `is_cyclic_directed` after a speculative edge insert
/// ///
/// Returns the graph and a `BTreeMap<name → NodeIndex>` for O(log n) /// Returns the graph and a `BTreeMap<name → NodeIndex>` for O(log n)
@ -191,28 +174,6 @@ fn build_graph(
(graph, idx) (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<String, Option<String>>,
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 /// Persist the topology map. Sorted JSON output (`BTreeMap` is sorted by
/// key) keeps git diffs minimal across re-writes. Best-effort — /// key) keeps git diffs minimal across re-writes. Best-effort —
/// returns an `io::Error` so callers can decide whether a failure /// returns an `io::Error` so callers can decide whether a failure
@ -742,75 +703,6 @@ mod tests {
assert!(top_level_agents_in(&topo).is_empty()); 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) // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps)
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------

View file

@ -4,10 +4,10 @@
//! you are `foo`; the manager socket simply serves as `ruth`. There is no //! you are `foo`; the manager socket simply serves as `ruth`. There is no
//! privilege flag — both transports run the same [`serve`] / [`dispatch`] //! privilege flag — both transports run the same [`serve`] / [`dispatch`]
//! code, and authority derives uniformly from the caller's identity: //! code, and authority derives uniformly from the caller's identity:
//! topology (`is_descendant_of`) for subtree-relational verbs, capabilities //! capabilities for the hive-wide queries and tool-group membership for the
//! for hive-wide queries, and tool-group membership for the orchestration //! orchestration verbs. An agent-targeting verb may name any agent, so `ruth`
//! verbs. `ruth` reaches every agent only as a consequence of being the //! reaches every agent exactly the way every other agent does, not via any
//! topology root, not via any hardcoded name match. //! hardcoded name match.
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::sync::Arc; use std::sync::Arc;
@ -87,10 +87,9 @@ pub fn start(agent: &str, socket_path: &Path, coord: Arc<Coordinator>) -> Result
/// Bind + serve the manager socket. This is now **pure transport**: it grants /// 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` /// 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 /// ("ruth"), and ruth's reach comes entirely from the capabilities /
/// (`is_descendant_of` covers every agent) plus the capabilities / tool-groups /// tool-groups it holds, identical to connecting on a per-agent socket — ruth
/// it holds, identical to connecting on a per-agent socket — ruth uses the /// uses the standard per-agent runtime dir + socket, with no dedicated helpers.
/// standard per-agent runtime dir + socket, with no dedicated helpers.
pub fn start_manager(coord: Arc<Coordinator>) -> Result<()> { pub fn start_manager(coord: Arc<Coordinator>) -> Result<()> {
use std::os::unix::fs::PermissionsExt as _; use std::os::unix::fs::PermissionsExt as _;
let dir = crate::paths::agent_runtime_dir(crate::lifecycle::MANAGER_NAME); let dir = crate::paths::agent_runtime_dir(crate::lifecycle::MANAGER_NAME);

View file

@ -1,6 +1,6 @@
//! Scheduled-prompt request handlers (`ListSchedules` / //! Scheduled-prompt request handlers (`ListSchedules` /
//! `RequestSchedulePrompt` / `CancelSchedule` / `EditSchedule` / //! `RequestSchedulePrompt` / `CancelSchedule` / `EditSchedule` /
//! `FireScheduleNow`), their shared ownership check, and the //! `FireScheduleNow`) and the
//! schedule-to-wire mapping reused by the dashboard //! schedule-to-wire mapping reused by the dashboard
//! (`schedule_to_wire_public` / `filter_ghost_schedule_targets`). //! (`schedule_to_wire_public` / `filter_ghost_schedule_targets`).
@ -10,20 +10,15 @@ use hive_core_agent_sock::Response;
use crate::coordinator::Coordinator; use crate::coordinator::Coordinator;
/// `ListSchedules` — snapshot the scheduled prompts `requester` is /// `ListSchedules` — snapshot every scheduled prompt. Ownership no longer
/// authorized to see: its own schedules, ones owned by an agent in its /// narrows the view: `CancelSchedule`/`EditSchedule`/`FireScheduleNow` accept
/// subtree, or (for the operator) everything. Uses the same /// any requester, so filtering the listing would only hide rows the requester
/// `schedule_authorized` rule `CancelSchedule`/`EditSchedule`/ /// can act on anyway.
/// `FireScheduleNow` already enforce, applied here as a per-row filter
/// instead of a hard reject.
pub(super) fn handle_list_schedules(coord: &Arc<Coordinator>, requester: &str) -> Response { pub(super) fn handle_list_schedules(coord: &Arc<Coordinator>, requester: &str) -> Response {
tracing::debug!(%requester, "list schedules");
match coord.scheduled_prompts.list() { match coord.scheduled_prompts.list() {
Ok(schedules) => Response::Schedules { Ok(schedules) => Response::Schedules {
schedules: schedules schedules: schedules.into_iter().map(schedule_to_wire).collect(),
.into_iter()
.filter(|s| schedule_authorized(requester, &s.owner))
.map(schedule_to_wire)
.collect(),
}, },
Err(e) => Response::Err { Err(e) => Response::Err {
message: format!("list scheduled prompts: {e:#}"), message: format!("list scheduled prompts: {e:#}"),
@ -108,39 +103,20 @@ pub(super) fn handle_request_schedule_prompt(
Response::Ok Response::Ok
} }
/// Cancel a schedule (whole or per-target). Manager-surface /// Cancel a schedule (whole or per-target). Any requester may cancel any
/// authorization: a manager can cancel its own schedules + any /// schedule, whoever owns it.
/// 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( pub(super) fn handle_cancel_schedule(
coord: &Arc<Coordinator>, coord: &Arc<Coordinator>,
requester: &str, requester: &str,
schedule_id: i64, schedule_id: i64,
targets: Option<&[String]>, targets: Option<&[String]>,
) -> Response { ) -> Response {
let schedule = match coord.scheduled_prompts.get(schedule_id) { // Existence check only — it buys the caller a clean "not found" instead
Ok(Some(s)) => s, // of whatever the cancel itself would say about an absent row.
Ok(None) => { if let Some(err) = require_schedule(coord, schedule_id) {
return Response::Err { return 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 { let result = match targets {
Some(list) if !list.is_empty() => coord Some(list) if !list.is_empty() => coord
.scheduled_prompts .scheduled_prompts
@ -160,37 +136,18 @@ pub(super) fn handle_cancel_schedule(
} }
} }
/// Authorize + dispatch a `FireScheduleNow` request from the /// Dispatch a `FireScheduleNow` request. Same rule as `CancelSchedule`: any
/// manager surface. Same ownership rules as `CancelSchedule`: /// requester, any schedule. The actual fan-out lives in
/// 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`. /// `scheduled_prompts_worker::fire_now`.
pub(super) async fn handle_fire_schedule_now( pub(super) async fn handle_fire_schedule_now(
coord: &Arc<Coordinator>, coord: &Arc<Coordinator>,
requester: &str, requester: &str,
schedule_id: i64, schedule_id: i64,
) -> Response { ) -> Response {
let schedule = match coord.scheduled_prompts.get(schedule_id) { if let Some(err) = require_schedule(coord, schedule_id) {
Ok(Some(s)) => s, return err;
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 // MCP fire_schedule_now stays no-reset (cadence intact); the
// reset-timer option is a dashboard-dialog affordance. // reset-timer option is a dashboard-dialog affordance.
match crate::scheduled_prompts_worker::fire_now(coord, schedule_id, false).await { match crate::scheduled_prompts_worker::fire_now(coord, schedule_id, false).await {
@ -222,13 +179,11 @@ pub(super) struct EditSchedulePatch {
pub(super) targets_remove: Option<Vec<String>>, pub(super) targets_remove: Option<Vec<String>>,
} }
/// Authorize + dispatch a `EditSchedule` patch. Same ownership /// Dispatch an `EditSchedule` patch. Same rule as `CancelSchedule`: any
/// rules as `CancelSchedule` — the manager can edit /// requester, any schedule. Forwards the partial payload to
/// schedules it owns + any owned by an agent in its subtree.
/// Forwards the partial payload to
/// `ScheduledPrompts::update` which enforces the cancelled-row / /// `ScheduledPrompts::update` which enforces the cancelled-row /
/// zero-interval validation. Returns `Ok` on a clean update; /// zero-interval validation. Returns `Ok` on a clean update;
/// `Err` with the underlying message on any auth / validation /// `Err` with the underlying message on any validation
/// failure so the dashboard can surface it verbatim. /// failure so the dashboard can surface it verbatim.
pub(super) fn handle_edit_schedule( pub(super) fn handle_edit_schedule(
coord: &Arc<Coordinator>, coord: &Arc<Coordinator>,
@ -244,27 +199,10 @@ pub(super) fn handle_edit_schedule(
targets_add, targets_add,
targets_remove, targets_remove,
} = patch; } = patch;
let schedule = match coord.scheduled_prompts.get(schedule_id) { if let Some(err) = require_schedule(coord, schedule_id) {
Ok(Some(s)) => s, return err;
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 { let patch = crate::scheduled_prompts::UpdateSchedule {
body, body,
description, description,
@ -284,23 +222,19 @@ pub(super) fn handle_edit_schedule(
} }
} }
/// Permission check shared by the whole scheduling surface: `requester` /// Shared existence guard for the three schedule-mutating verbs: `None` when
/// can act on a schedule it owns, on one owned by an agent in its /// the row is there, `Some(Err)` to short-circuit when it isn't or the read
/// subtree (delegated to topology — see `crate::topology::is_descendant_of`), /// failed. Not a permission check — any requester may act on any schedule.
/// or, as the operator, on anything. `CancelSchedule`/`EditSchedule`/ fn require_schedule(coord: &Arc<Coordinator>, schedule_id: i64) -> Option<Response> {
/// `FireScheduleNow` apply it as a hard reject; `ListSchedules` applies match coord.scheduled_prompts.get(schedule_id) {
/// it as a per-row filter. Ok(Some(_)) => None,
fn schedule_authorized(requester: &str, owner: &str) -> bool { Ok(None) => Some(Response::Err {
if requester == owner { message: format!("schedule {schedule_id} not found"),
return true; }),
Err(e) => Some(Response::Err {
message: format!("read schedule {schedule_id}: {e:#}"),
}),
} }
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. /// Map a `scheduled_prompts::Schedule` to its public wire shape.
@ -396,32 +330,6 @@ 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] #[test]
fn ghost_filter_drops_dead_agents_keeps_live() { fn ghost_filter_drops_dead_agents_keeps_live() {
let live: std::collections::HashSet<String> = ["iris".to_owned(), "damocles".to_owned()] let live: std::collections::HashSet<String> = ["iris".to_owned(), "damocles".to_owned()]
@ -445,4 +353,136 @@ mod tests {
filter_ghost_schedule_targets(&mut schedules, &live); filter_ghost_schedule_targets(&mut schedules, &live);
assert!(schedules[0].targets.is_empty()); 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<Coordinator>`, so there is no lighter way in;
/// `open` touches nothing outside the db path it is handed.
fn coordinator() -> (tempfile::TempDir, Arc<Coordinator>) {
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<Coordinator>) -> 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:?}"
);
}
} }

View file

@ -157,9 +157,7 @@ pub enum Request {
#[serde(default, skip_serializing_if = "Option::is_none")] #[serde(default, skip_serializing_if = "Option::is_none")]
targets: Option<Vec<String>>, targets: Option<Vec<String>>,
}, },
/// List the schedules the requester is authorized to see: its own, /// List every schedule in the hive, whoever owns it.
/// plus any owned by an agent in its subtree (everything, for the
/// operator).
ListSchedules, ListSchedules,
/// *(privileged)* Fire a scheduled prompt out of band immediately. /// *(privileged)* Fire a scheduled prompt out of band immediately.
FireScheduleNow { id: i64 }, FireScheduleNow { id: i64 },