refactor(#1865): replace the privileged flag with topology + capability gating
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.
This commit is contained in:
parent
674505fbe7
commit
53b4e752ef
2 changed files with 282 additions and 312 deletions
|
|
@ -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<Coordinator>,
|
||||
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}");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue