From 7dd52207de766ca59ef72821c07e31098e5f39f3 Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 24 Sep 2026 11:38:29 +0200 Subject: [PATCH] dashboard: fail purge-tombstone closed when the container list is unreadable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit post_purge_tombstone's live-container guard used lifecycle::list().await .unwrap_or_default(), so a failed list() (hive-priv socket unreachable, restarting, ...) read as 'no live containers' and let the purge proceed — deleting agent_state_dir/applied_dir for an agent whose container may still be running. Factor the decision into check_not_live(), a pure helper that refuses on both 'still listed' and 'list unreadable', with unit tests covering both plus the allowed case. Closes #4671 --- hive-c0re/src/dashboard/tombstones.rs | 78 +++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/hive-c0re/src/dashboard/tombstones.rs b/hive-c0re/src/dashboard/tombstones.rs index b8c6c605..eebf7c49 100644 --- a/hive-c0re/src/dashboard/tombstones.rs +++ b/hive-c0re/src/dashboard/tombstones.rs @@ -115,6 +115,31 @@ pub(crate) async fn emit_tombstones_snapshot(coord: &Arc) { }); } +/// Decide whether `post_purge_tombstone` may proceed, given the outcome of +/// `lifecycle::list()`. `Ok` only when the list was read and names no live +/// container for `name`; an unreadable list proves nothing about whether a +/// container is live, so it refuses too rather than reading absence into it. +fn check_not_live( + name: &hive_types::Ident, + listed: anyhow::Result>, +) -> Result<(), String> { + match listed { + Ok(live) + if live.iter().any(|c| { + c == &format!("{}{name}", lifecycle::AGENT_PREFIX) || c.as_str() == name.as_str() + }) => + { + Err(format!( + "refusing to purge {name}: container still exists — use DESTR0Y first" + )) + } + Ok(_) => Ok(()), + Err(e) => Err(format!( + "refusing to purge {name}: cannot confirm no live container exists: {e:#}" + )), + } +} + /// Wipe a tombstoned agent's /// retained state dir + applied config dir entirely. #[utoipa::path( @@ -148,15 +173,10 @@ pub(super) async fn post_purge_tombstone( }; // Sanity: refuse to purge if a live container still exists with this // name. The dashboard already filters tombstones to non-live names, - // but the operator could send a stale POST. - let live = lifecycle::list().await.unwrap_or_default(); - if live - .iter() - .any(|c| c == &format!("{}{name}", lifecycle::AGENT_PREFIX) || c.as_str() == name.as_str()) - { - return error_response(&format!( - "refusing to purge {name}: container still exists — use DESTR0Y first" - )); + // but the operator could send a stale POST. An unreadable list is not + // proof of absence — see `check_not_live`. + if let Err(reason) = check_not_live(&name, lifecycle::list().await) { + return error_response(&reason); } let mut errors = Vec::new(); for dir in [ @@ -184,3 +204,43 @@ pub(super) async fn post_purge_tombstone( error_response(&format!("purge {name} partial: {}", errors.join(", "))) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn ident(s: &str) -> hive_types::Ident { + hive_types::Ident::parse(s).unwrap() + } + + /// A live container by that name in the list refuses the purge. + #[test] + fn live_container_in_list_refuses() { + let name = ident("doomed"); + let err = check_not_live(&name, Ok(vec!["h-doomed".to_owned()])) + .expect_err("a listed container must refuse the purge"); + assert!(err.contains("still exists"), "{err}"); + } + + /// No matching name in a readable list allows the purge. + #[test] + fn absent_from_list_allows() { + let name = ident("doomed"); + check_not_live(&name, Ok(vec!["h-other".to_owned()])) + .expect("an absent container must allow the purge"); + // Control: the same name present in the list still refuses, so the + // pass above comes from the absence, not from the helper accepting + // anything. + assert!(check_not_live(&name, Ok(vec!["h-doomed".to_owned()])).is_err()); + } + + /// An unreadable list proves nothing, so it must refuse — this is the + /// case that regresses to a silent delete under `unwrap_or_default()`. + #[test] + fn unreadable_list_refuses() { + let name = ident("doomed"); + let err = check_not_live(&name, Err(anyhow::anyhow!("connect to hive-priv socket"))) + .expect_err("an unreadable list must not read as an absent container"); + assert!(err.contains("cannot confirm"), "{err}"); + } +}