questions: unit-test the manager-only guard on approval cancel (argus nit on #508)
This commit is contained in:
parent
c1f27e3b7b
commit
402ee95d0c
1 changed files with 42 additions and 5 deletions
|
|
@ -184,11 +184,7 @@ pub fn handle_cancel_loose_end(
|
||||||
// Manager-only: only the agent that can submit approvals
|
// Manager-only: only the agent that can submit approvals
|
||||||
// is allowed to withdraw them. Sub-agents would have no
|
// is allowed to withdraw them. Sub-agents would have no
|
||||||
// pending approvals of their own to cancel anyway.
|
// pending approvals of their own to cancel anyway.
|
||||||
if canceller != hive_sh4re::MANAGER_AGENT {
|
check_approval_canceller_is_manager(canceller)?;
|
||||||
return Err(
|
|
||||||
"cancel_loose_end: only the manager can cancel approval rows".to_owned(),
|
|
||||||
);
|
|
||||||
}
|
|
||||||
let approval = coord
|
let approval = coord
|
||||||
.approvals
|
.approvals
|
||||||
.mark_cancelled(id, canceller)
|
.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 +
|
// Real coverage needs a `Coordinator` fixture (broker + sqlite +
|
||||||
// in-memory questions). Skipped for now — the normalisation branches
|
// in-memory questions). Skipped for now — the normalisation branches
|
||||||
// in `handle_ask` are short enough to read line-by-line; once we add
|
// in `handle_ask` are short enough to read line-by-line; once we add
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue