diff --git a/hive-c0re/src/questions.rs b/hive-c0re/src/questions.rs index b634d1f8..339d377c 100644 --- a/hive-c0re/src/questions.rs +++ b/hive-c0re/src/questions.rs @@ -184,11 +184,7 @@ pub fn handle_cancel_loose_end( // Manager-only: only the agent that can submit approvals // is allowed to withdraw them. Sub-agents would have no // pending approvals of their own to cancel anyway. - if canceller != hive_sh4re::MANAGER_AGENT { - return Err( - "cancel_loose_end: only the manager can cancel approval rows".to_owned(), - ); - } + check_approval_canceller_is_manager(canceller)?; let approval = coord .approvals .mark_cancelled(id, canceller) @@ -212,6 +208,47 @@ pub fn handle_cancel_loose_end( } } +/// Manager-only guard on the `Approval` cancel arm. Pulled out so +/// the auth check has its own focused unit test (argus nit on #508) +/// — testing the full `handle_cancel_loose_end` flow would need a +/// `Coordinator` fixture (broker + sqlite + in-memory questions), +/// which we don't have today. The check is a single string compare, +/// so a function-level test gives the same coverage with no harness. +fn check_approval_canceller_is_manager(canceller: &str) -> Result<(), String> { + if canceller != hive_sh4re::MANAGER_AGENT { + return Err("cancel_loose_end: only the manager can cancel approval rows".to_owned()); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn approval_cancel_rejects_sub_agent_callers() { + // Argus nit on #508: sub-agents must not be able to cancel + // approval rows even if they invent an id. The guard is + // server-side so client cooperation is irrelevant. + let err = check_approval_canceller_is_manager("bitburner").unwrap_err(); + assert!(err.contains("only the manager"), "{err}"); + // Bonus: empty / operator strings also rejected (only the + // exact MANAGER_AGENT constant passes). + assert!(check_approval_canceller_is_manager("").is_err()); + assert!( + check_approval_canceller_is_manager(hive_sh4re::OPERATOR_RECIPIENT) + .is_err(), + "operator surface uses the dashboard cancel path, not this dispatcher", + ); + } + + #[test] + fn approval_cancel_allows_manager() { + check_approval_canceller_is_manager(hive_sh4re::MANAGER_AGENT) + .expect("MANAGER_AGENT must pass the guard"); + } +} + // Real coverage needs a `Coordinator` fixture (broker + sqlite + // in-memory questions). Skipped for now — the normalisation branches // in `handle_ask` are short enough to read line-by-line; once we add