fix(#1953): gate approval cancel on submitter ownership + document submitter_of errors

This commit is contained in:
damocles 2026-06-23 21:22:08 +02:00 committed by mara
commit ebaf192476
2 changed files with 27 additions and 6 deletions

View file

@ -136,6 +136,11 @@ impl Approvals {
/// The agent that submitted approval `id` (the authenticated socket
/// caller at submit time). `None` for legacy rows predating the
/// `submitter` column — callers route those to the root agent.
///
/// # Errors
///
/// Returns an error if the sqlite prepare/query fails. A missing row
/// or a `NULL` submitter is not an error — both yield `Ok(None)`.
pub fn submitter_of(&self, id: i64) -> Result<Option<String>> {
let conn = self.conn.lock().unwrap();
let submitter: Option<String> = conn

View file

@ -144,8 +144,10 @@ pub fn handle_answer(
/// 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.)
/// tool-group (the grantable capability) AND ownership — the canceller must
/// be the approval's submitter — so no positional / hardcoded privilege and
/// no cross-agent cancellation. (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(
@ -195,11 +197,25 @@ pub fn handle_cancel_loose_end(
Ok(())
}
hive_sh4re::CancelLooseEndKind::Approval => {
// 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.
// Withdrawing an approval needs the grantable `approvals`
// tool-group (held by any approval-submitting orchestrator)
// AND ownership: only the agent that submitted the approval
// may withdraw it. Without the ownership check, any
// approvals-group agent could cancel any other's approval by
// id. A NULL submitter (legacy row predating the column) is
// owned by the root agent.
check_can_cancel_approval(canceller)?;
let submitter = coord
.approvals
.submitter_of(id)
.map_err(|e| format!("{e:#}"))?
.unwrap_or_else(|| hive_sh4re::MANAGER_AGENT.to_owned());
if submitter != canceller {
return Err(format!(
"cancel_loose_end: approval {id} was submitted by {submitter}, \
not {canceller}; only the submitting agent can withdraw it"
));
}
let approval = coord
.approvals
.mark_cancelled(id, canceller)