diff --git a/engine/src/engine/break_glass_node_tests.rs b/engine/src/engine/break_glass_node_tests.rs new file mode 100644 index 0000000..84a5c18 --- /dev/null +++ b/engine/src/engine/break_glass_node_tests.rs @@ -0,0 +1,423 @@ +//! Engine-level acceptance tests for ADR-0040 §5's `ContainNode` revert wiring: break-glass +//! (ADR-0036) and the standard ledger self-revert lifecycle (ADR-0017) must uncordon a +//! standing node-containment cut within one pass — never leave it silently in place through +//! the wrong-shaped generic network `actuator`. Split out of `break_glass_tests.rs` purely to +//! keep every file under the 1,000-line cap (repo CLAUDE.md); `use super::*` resolves to the +//! engine module, matching every sibling `*_tests.rs` file. +//! +//! Deliberately does NOT depend on the `node` arming rung (ADR-0040 §6, a separate ticket) or +//! on any node-observation adapter (`respond::actuator::node_containment`'s own doc): +//! `EnabledActions::enable(ProposedAction::ContainNode)` arms the class directly, bypassing +//! `EnabledActions::from_names` (which has no `node` name yet), and each test seeds its own +//! `NodeFact`/actuator double via `Engine::with_node_fact`/`Engine::with_node_containment_actuator` +//! rather than a live fleet watch. The PROPOSAL half is real production machinery — the model +//! naming a boundary-broken workload, the menu escalating it to `ContainNode` +//! (`reason::proof::boundary_break`, `reason::adjudicate::incident::menu`), the ledger +//! tracking it — only the "this was already applied" step is synthesized directly into the +//! action log, standing in for the sibling ticket's future apply-side rail wiring. + +use super::*; +use crate::engine::graph::attack::AttackRef; +use crate::engine::graph::{NodeKey, SecurityGraph}; +use crate::engine::observe::Snapshot; +use crate::engine::reason::adjudicate::incident::{Assessment, IncidentDecision, Menu}; +use crate::engine::respond::actuator::{Actuation, node_containment}; +use serde_json::json; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; + +/// A unique temp flag path for a test, without a temp-file crate — mirrors +/// `break_glass_tests::temp_flag_path`. +fn temp_flag_path(tag: &str) -> std::path::PathBuf { + use std::sync::atomic::AtomicU64; + static NONCE: AtomicU64 = AtomicU64::new(0); + let n = NONCE.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "protector-engine-break-glass-node-{tag}-{}-{n}", + std::process::id() + )) +} + +/// A breach-relevant chain on `web` — the exact `tests::exposed_snapshot(true)` fixture +/// (internet-exposed via `web-lb`, reads `session-key`, runs a critical CVE loaded at +/// runtime) — with `web` additionally scheduled on `node-1` (the `ScheduledOn` placement +/// edge, ADR-0040 §3) alongside a labelled `neighbor` pod, so the co-resident default-deny +/// sweep has a real target. +/// +/// When `tampered`, `web` also carries a live kernel-tamper signal (`Behavior::PtraceAttach`) +/// — `boundary_break` trigger (c), ADR-0040 §3 — so `menu::build_menu` escalates the entry's +/// own line to `ProposedAction::ContainNode` instead of its ordinary pod-scoped quarantine. +/// `tampered = false` is the "healed" snapshot: the CVE/exposure still breach-relevant, but +/// boundary_break no longer holds, so the entry line resolves back to its pod-scoped +/// mechanism — the fixture the self-revert test needs to prove no chain still carries `web` +/// as a boundary-broken target. +fn boundary_broken_snapshot(tampered: bool) -> Snapshot { + use crate::engine::graph::Behavior; + use crate::engine::observe::{Attribution, RuntimeObservation}; + + let mut snap = super::tests::exposed_snapshot(true); + snap.pods[0] + .spec + .as_mut() + .expect("exposed_snapshot's web pod always carries a spec") + .node_name = Some("node-1".to_string()); + snap.pods.push( + serde_json::from_value(json!({ + "apiVersion": "v1", "kind": "Pod", + "metadata": {"name": "neighbor", "namespace": "app", "labels": {"app": "neighbor"}}, + "spec": { + "nodeName": "node-1", + "containers": [{"name": "neighbor", "image": "neighbor:1"}] + } + })) + .unwrap(), + ); + if tampered { + snap.runtime_events.push(RuntimeObservation { + attribution: Attribution::by_namespaced_name("app", "web"), + source: None, + observed_at_ms: None, + node: None, + behavior: Behavior::PtraceAttach, + }); + } + snap +} + +/// A model that always decisively attacks the entry, naming exactly whatever +/// [`Menu::resolve`] resolves for it — for `boundary_broken_snapshot(true)`'s `web` entry +/// that resolution is `ProposedAction::ContainNode` on its host (`menu::escalate`, +/// ADR-0040 §1); for the healed variant it falls back to `web`'s ordinary pod-scoped +/// mechanism. Mirrors `break_glass_tests::AlwaysAttacksTheEntry`, duplicated here (that +/// struct is private to its own file) so this file stays self-contained. +struct NamesWhateverTheMenuResolves; + +#[async_trait::async_trait] +impl reason::adjudicate::Adjudicator for NamesWhateverTheMenuResolves { + async fn judge( + &self, + entry: &NodeKey, + _objectives: &[(NodeKey, AttackRef)], + _graph: &SecurityGraph, + _prompt: &str, + _downstream: &[NodeKey], + menu: &Menu, + ) -> IncidentDecision { + let cut = menu + .resolve(entry) + .expect("the entry is selectable on its own menu"); + IncidentDecision { + assessment: Assessment::Attack, + reason: "names whatever the deterministic menu resolved for the entry".to_string(), + cuts: vec![cut], + } + } +} + +/// A test double for [`node_containment::NodeContainmentRevert`] that mirrors +/// [`node_containment::NodeContainmentActuator::revert`]'s own logic — self-gated on +/// [`node_containment::revert_decision`] — but records counts instead of touching a +/// cluster, so a test can assert the ENGINE actually reached this seam with the right +/// mitigation/target/co-resident set, and that the ownership rail (not a fabricated +/// cluster success) is what decided whether the uncordon happened. +struct RecordingNodeContainmentActuator { + reverted: Arc, + co_resident_lifted: Arc, + refused: Arc, +} + +#[async_trait::async_trait] +impl node_containment::NodeContainmentRevert for RecordingNodeContainmentActuator { + async fn revert( + &self, + _mitigation: &Mitigation, + target: &node_containment::NodeFact, + co_resident: &[Mitigation], + ) -> Actuation { + if node_containment::revert_decision(target).is_err() { + self.refused.fetch_add(1, Ordering::SeqCst); + return Actuation::DryRun; + } + self.reverted.fetch_add(1, Ordering::SeqCst); + self.co_resident_lifted + .fetch_add(co_resident.len(), Ordering::SeqCst); + Actuation::Reverted + } +} + +/// Build a `ContainNode`-armed engine (enabled DIRECTLY via `EnabledActions::enable` — no +/// `node` rung needed) wired to [`NamesWhateverTheMenuResolves`] and a fresh +/// [`RecordingNodeContainmentActuator`], with one seeded [`node_containment::NodeFact`] for +/// `node-1`. Returns the engine plus the double's shared counters. +fn node_containment_engine( + break_glass: break_glass::BreakGlass, + owned_by_protector: bool, +) -> (Engine, Arc, Arc, Arc) { + let reverted = Arc::new(AtomicUsize::new(0)); + let co_resident_lifted = Arc::new(AtomicUsize::new(0)); + let refused = Arc::new(AtomicUsize::new(0)); + let actuator = RecordingNodeContainmentActuator { + reverted: reverted.clone(), + co_resident_lifted: co_resident_lifted.clone(), + refused: refused.clone(), + }; + let engine = Engine::new( + EnabledActions::from_names(["judgement"]).enable(ProposedAction::ContainNode), + ActuationScope::unscoped(), + Box::new(respond::actuator::DryRunActuator), + Box::new(NamesWhateverTheMenuResolves), + ) + .with_break_glass(break_glass) + .with_node_containment_actuator(Box::new(actuator)) + .with_node_fact(node_containment::NodeFact { + name: "node-1".to_string(), + control_plane: false, + schedulable: false, + owned_by_protector, + }); + (engine, reverted, co_resident_lifted, refused) +} + +/// Run one pass over `snap` and pull out the resulting `ContainNode` mitigation the ledger +/// proposed for `web`'s entry — real production proof + menu resolution, not hand-built. +/// Panics if the fixture didn't produce one (a fixture regression, not an assertion about +/// the code under test). +async fn contain_node_mitigation(engine: &mut Engine, snap: &Snapshot) -> Mitigation { + engine.process(snap).await; + engine + .ledger + .active() + .find(|m| m.action == ProposedAction::ContainNode) + .cloned() + .expect("the boundary-broken entry proposes a ContainNode mitigation") +} + +/// (a) Engaging break-glass with a standing `ContainNode` cut uncordons the node AND drops +/// the co-resident deny set within the SAME pass — the core ADR-0040 §5 safety property this +/// ticket verifies. +#[tokio::test] +async fn engaging_break_glass_uncordons_the_node_and_lifts_co_resident_denies_within_one_pass() { + let path = temp_flag_path("reverts"); + let (mut engine, reverted, co_resident_lifted, _refused) = + node_containment_engine(break_glass::BreakGlass::at(&path), true); + let snap = boundary_broken_snapshot(true); + + // The real proof + menu resolution produce the ContainNode mitigation; synthesize it as + // already-applied — standing in for the sibling ticket's future apply-side rail wiring + // (ADR-0040 §6), which this ticket deliberately does not depend on. + let mitigation = contain_node_mitigation(&mut engine, &snap).await; + engine.actions.record(mitigation, Vec::new()); + assert_eq!( + engine.actions.active_count(), + 1, + "the synthesized cut is standing" + ); + + // Pass 1 (break-glass clear, identical facts): still armed and still justified — must + // NOT be touched, so the next pass's revert is attributable to break-glass alone. + engine.process(&snap).await; + assert_eq!(reverted.load(Ordering::SeqCst), 0, "not yet engaged"); + assert_eq!(engine.actions.active_count(), 1); + + // Engage break-glass — presence only, no chart change, no restart. + std::fs::write(&path, "").expect("write the flag file"); + + // Pass 2 (identical facts, break-glass now engaged): uncordon + co-resident lift within + // this ONE pass. + engine.process(&snap).await; + assert_eq!( + reverted.load(Ordering::SeqCst), + 1, + "break-glass uncordons the standing ContainNode cut within one pass" + ); + assert_eq!( + co_resident_lifted.load(Ordering::SeqCst), + 2, + "both labelled pods scheduled on node-1 (web itself and neighbor) have their \ + default-deny lifted in the SAME call — the cordon alone doesn't touch an already-\ + running pod's traffic" + ); + assert_eq!( + engine.actions.active_count(), + 0, + "the applied-action log agrees nothing is standing" + ); + + let reversions = engine.reversions().snapshot(); + let reasons: Vec<&str> = reversions.iter().map(|r| r.reason.as_str()).collect(); + assert!( + reasons.iter().any(|r| r.contains("armed")), + "the reversion log names the disarm, not just \"unjustified\": got {reasons:?}" + ); + + std::fs::remove_file(&path).ok(); +} + +/// (b) The ledger self-reverts the cordon on the standard lifecycle (ADR-0017) once no +/// chain still carries `web` as a boundary-broken target — no break-glass involved. +#[tokio::test] +async fn self_reverts_when_no_chain_still_carries_the_boundary_broken_target() { + let (mut engine, reverted, _co_resident_lifted, _refused) = + node_containment_engine(break_glass::BreakGlass::disabled(), true); + let broken = boundary_broken_snapshot(true); + + let mitigation = contain_node_mitigation(&mut engine, &broken).await; + assert_eq!(mitigation.action, ProposedAction::ContainNode); + engine.actions.record(mitigation, Vec::new()); + assert_eq!(engine.actions.active_count(), 1); + + // Still boundary-broken: the cut stays standing (still armed, still justified). + engine.process(&broken).await; + assert_eq!(reverted.load(Ordering::SeqCst), 0); + assert_eq!(engine.actions.active_count(), 1); + + // The kernel-tamper signal clears — boundary_break(web) no longer holds, so the menu + // resolves web's line back to its ordinary pod-scoped mechanism. No chain still carries + // `web` as a boundary-broken target, so the ContainNode cut's justification is gone. + let healed = boundary_broken_snapshot(false); + engine.process(&healed).await; + assert_eq!( + reverted.load(Ordering::SeqCst), + 1, + "the ledger self-revert lifts the cordon once boundary_break no longer holds" + ); + assert_eq!(engine.actions.active_count(), 0); + + let reversions = engine.reversions().snapshot(); + let reasons: Vec<&str> = reversions.iter().map(|r| r.reason.as_str()).collect(); + assert!( + reasons.iter().any(|r| r.contains("no proven chain")), + "reverted via the ordinary chain-justification path, not break-glass: got {reasons:?}" + ); +} + +/// (c) Revert uncordons ONLY nodes carrying protector's own ownership annotation — a +/// human/autoscaler-cordoned node (no annotation) is left untouched. Falls out of +/// `NodeContainmentActuator::revert`'s own self-gate (`revert_decision`), asserted here +/// end-to-end through the engine's break-glass path. +#[tokio::test] +async fn revert_skips_a_node_lacking_the_ownership_annotation() { + let path = temp_flag_path("not-owned"); + let (mut engine, reverted, co_resident_lifted, refused) = + node_containment_engine(break_glass::BreakGlass::at(&path), false); // NOT protector-owned + let snap = boundary_broken_snapshot(true); + + let mitigation = contain_node_mitigation(&mut engine, &snap).await; + engine.actions.record(mitigation, Vec::new()); + + std::fs::write(&path, "").expect("write the flag file"); + engine.process(&snap).await; + + assert_eq!( + reverted.load(Ordering::SeqCst), + 0, + "a node protector never cordoned (no ownership annotation) is never uncordoned" + ); + assert_eq!( + co_resident_lifted.load(Ordering::SeqCst), + 0, + "no co-resident deny is lifted either — nothing about this node is touched" + ); + assert_eq!( + refused.load(Ordering::SeqCst), + 1, + "the ownership rail explicitly refused, rather than silently doing nothing" + ); + + std::fs::remove_file(&path).ok(); +} + +/// A `ContainNode` revert with no observed [`node_containment::NodeFact`] for the host is +/// SKIPPED, not fabricated as "no data ⇒ pass" — the same discipline +/// `respond::actuator::node_containment`'s own doc already applies to the cordon rails. +#[tokio::test] +async fn revert_skips_when_no_nodefact_is_observed_for_the_host() { + let path = temp_flag_path("no-fact"); + let reverted = Arc::new(AtomicUsize::new(0)); + let co_resident_lifted = Arc::new(AtomicUsize::new(0)); + let refused = Arc::new(AtomicUsize::new(0)); + let actuator = RecordingNodeContainmentActuator { + reverted: reverted.clone(), + co_resident_lifted: co_resident_lifted.clone(), + refused: refused.clone(), + }; + let mut engine = Engine::new( + EnabledActions::from_names(["judgement"]).enable(ProposedAction::ContainNode), + ActuationScope::unscoped(), + Box::new(respond::actuator::DryRunActuator), + Box::new(NamesWhateverTheMenuResolves), + ) + .with_break_glass(break_glass::BreakGlass::at(&path)) + .with_node_containment_actuator(Box::new(actuator)); + // Deliberately no `.with_node_fact(...)` — no observed fleet for this host at all. + + let snap = boundary_broken_snapshot(true); + let mitigation = contain_node_mitigation(&mut engine, &snap).await; + engine.actions.record(mitigation, Vec::new()); + + std::fs::write(&path, "").expect("write the flag file"); + engine.process(&snap).await; + + assert_eq!( + reverted.load(Ordering::SeqCst) + refused.load(Ordering::SeqCst), + 0, + "with no observed NodeFact for this host the actuator is never even called — \ + ownership can't be verified, so the revert is skipped rather than fabricated" + ); + assert_eq!( + engine.actions.active_count(), + 0, + "the applied-action log still drops the entry — the SAME bookkeeping asymmetry the \ + network-cut path already has (a failed/skipped cluster call doesn't re-queue)" + ); + + std::fs::remove_file(&path).ok(); +} + +/// (d) Clearing break-glass restores the node class's posture byte-identically. Unlike the +/// network-cut mirror (`clearing_break_glass_restores_the_configured_posture_byte_identical`, +/// where the standing cut auto-applies again once cleared), `ContainNode` never auto-applies +/// at all (ADR-0040 §5: propose-first by construction) — so the honest analogue is that the +/// ledger's model-chosen `ContainNode` proposal is EXACTLY the cut+mechanism whether +/// break-glass ever engaged or not: disarm narrows ACTUATION only (ADR-0036), so it has +/// nothing to disarm on the propose-only side. +#[tokio::test] +async fn clearing_break_glass_leaves_the_node_classs_posture_byte_identical() { + let snap = boundary_broken_snapshot(true); + + let (mut never_engaged, ..) = + node_containment_engine(break_glass::BreakGlass::disabled(), true); + let baseline = contain_node_mitigation(&mut never_engaged, &snap).await; + assert_eq!(baseline.action, ProposedAction::ContainNode); + + let path = temp_flag_path("posture"); + std::fs::write(&path, "").expect("write the flag file"); // engaged from boot + let (mut engaged_then_cleared, ..) = + node_containment_engine(break_glass::BreakGlass::at(&path), true); + let while_engaged = contain_node_mitigation(&mut engaged_then_cleared, &snap).await; + assert_eq!( + while_engaged.cut, baseline.cut, + "same cut while disarmed from boot" + ); + assert_eq!( + while_engaged.action, baseline.action, + "break-glass narrows ACTUATION only (ADR-0036) — it must not perturb the mechanism \ + the ledger itself resolved" + ); + assert_eq!( + engaged_then_cleared.actions.active_count(), + 0, + "ContainNode never auto-applies regardless of break-glass — propose-first by \ + construction (ADR-0040 §5)" + ); + + std::fs::remove_file(&path).expect("clear the flag file"); + let after_clear = contain_node_mitigation(&mut engaged_then_cleared, &snap).await; + assert_eq!( + after_clear.cut, baseline.cut, + "clearing break-glass restores exactly the same node-class posture — byte-identical \ + to never having engaged it at all" + ); + assert_eq!(after_clear.action, baseline.action); + assert_eq!(engaged_then_cleared.actions.active_count(), 0); +} diff --git a/engine/src/engine/mod.rs b/engine/src/engine/mod.rs index 7c3c079..d5ca93f 100644 --- a/engine/src/engine/mod.rs +++ b/engine/src/engine/mod.rs @@ -283,6 +283,26 @@ pub struct Engine { /// `enforceScope` on demand. Written at the SAME point the live blast gate computes /// each blast, never a second, independently-derived pass over the graph/health. scope_preview: std::sync::Arc, + /// The live cluster-facing revert half of the ADR-0040 node-containment actuator + /// (cordon lift + co-resident-deny lift), reached through the narrow + /// [`respond::actuator::node_containment::NodeContainmentRevert`] seam so the + /// self-revert loop below can hold either the real cluster actuator or a test double — + /// mirroring [`Self::actuator`] above. `None` (the [`Self::new`] default) leaves a + /// standing `ContainNode` mitigation un-reverted rather than driving it through the + /// wrong-shaped generic `actuator` (see [`Self::revert_contain_node`]); wiring a real + /// one in is a follow-up once a node-observation adapter exists + /// ([`respond::actuator::node_containment`]'s own doc). + node_containment_actuator: + Option>, + /// Observed [`respond::actuator::node_containment::NodeFact`]s for the `ContainNode` + /// revert ownership self-gate (ADR-0040 §5), keyed by node name. Empty (the + /// [`Self::new`] default) until seeded via [`Self::with_node_fact`] — a real + /// node-observation adapter refreshing this fleet every pass is a follow-up + /// (`respond::actuator::node_containment`'s own doc). A `ContainNode` mitigation whose + /// host has no entry here is left un-reverted rather than fabricating "no data ⇒ pass" + /// for the ownership check — the same discipline that module already applies to the + /// cordon rails. + node_facts: std::collections::BTreeMap, } impl Engine { @@ -338,6 +358,8 @@ impl Engine { break_glass_was_engaged: false, metrics: EngineMetrics::new(), scope_preview: std::sync::Arc::new(state::ScopePreviewStore::new()), + node_containment_actuator: None, + node_facts: std::collections::BTreeMap::new(), } } @@ -746,7 +768,16 @@ impl Engine { .reconcile(&health, &justified, &effective_active) { tracing::info!(reason = %reversion.reason, "reverting applied mitigation"); - self.actuator.revert(&reversion.mitigation).await; + // ADR-0040 §5: a `ContainNode` reversion is a cordon lift + co-resident-deny + // lift, not the generic network actuator's `AdminNetworkPolicy`/`NetworkPolicy` + // delete — routing it through `self.actuator` would silently no-op (wrong object + // name, wrong kind) and leave the node cordoned. See `revert_contain_node`. + if reversion.mitigation.action == ProposedAction::ContainNode { + self.revert_contain_node(&reversion.mitigation, &graph) + .await; + } else { + self.actuator.revert(&reversion.mitigation).await; + } self.metrics .mitigations .add(1, &[opentelemetry::KeyValue::new("action", "reverted")]); @@ -831,6 +862,11 @@ impl Engine { } } +// The ADR-0040 `ContainNode` revert seam (the builders attaching a live actuator/observed +// `NodeFact`s, and the self-revert loop's `revert_contain_node` call above), extracted to +// keep this orchestrator under the file-size cap (repo CLAUDE.md). +mod node_containment_revert; + // The engine's driver (`run_watch`) and its env-driven builders live in a sibling // module, split out to keep this file under the 1,000-line cap (repo CLAUDE.md). The // public surface (`run_watch`) is re-exported here so external paths @@ -864,6 +900,13 @@ mod judge_freshness_tests; #[cfg(test)] mod break_glass_tests; +// The ADR-0040 `ContainNode` revert-wiring tests (break-glass + the standard ledger +// self-revert must actually uncordon a node, not just drop its co-resident +// NetworkPolicies), split out of `break_glass_tests.rs` to keep every file under the +// 1,000-line cap (CLAUDE.md). +#[cfg(test)] +mod break_glass_node_tests; + // The pre-arm scope-simulation preview's engine-level mutation-free proof (ADR-0021, // ADR-0016), split out of `tests.rs` to keep every file under the 1,000-line cap (CLAUDE.md). #[cfg(test)] diff --git a/engine/src/engine/node_containment_revert.rs b/engine/src/engine/node_containment_revert.rs new file mode 100644 index 0000000..496e1f2 --- /dev/null +++ b/engine/src/engine/node_containment_revert.rs @@ -0,0 +1,83 @@ +//! The ADR-0040 §5 `ContainNode` revert seam: the builders that attach a live +//! [`respond::actuator::node_containment::NodeContainmentRevert`] actuator and observed +//! [`respond::actuator::node_containment::NodeFact`]s, and +//! [`Engine::revert_contain_node`] — the call `Engine::process`'s self-revert loop makes for +//! a standing `ContainNode` reversion instead of the generic network `actuator`. Extracted +//! from the orchestrator purely to keep every file under the 1,000-line cap (repo +//! CLAUDE.md); this is a behavior-neutral code move, not a design change — see +//! `respond::actuator::node_containment`'s own doc for why the revert side is wired now +//! while the apply side (the `node` arming rung, node observation, RBAC) is a follow-up. + +use super::{Engine, Mitigation, graph, respond}; + +impl Engine { + /// Attach the live node-containment revert actuator (ADR-0040 §5): the cluster-facing + /// cordon-lift + co-resident-deny-lift half of `ContainNode`'s closed loop, reached + /// through [`respond::actuator::node_containment::NodeContainmentRevert`] so a test can + /// substitute a double for the real, `kube::Client`-backed + /// [`respond::actuator::node_containment::NodeContainmentActuator`]. Builder-style. + /// Engines that never call this (every existing test, and any embedding that doesn't + /// opt in) leave a standing `ContainNode` mitigation un-reverted — see + /// [`Self::revert_contain_node`] for why that is the safe default rather than a silent + /// no-op through the wrong-shaped generic `actuator`. + pub fn with_node_containment_actuator( + mut self, + actuator: Box, + ) -> Self { + self.node_containment_actuator = Some(actuator); + self + } + + /// Seed one observed [`respond::actuator::node_containment::NodeFact`] for the + /// `ContainNode` revert ownership self-gate (ADR-0040 §5), keyed by its own + /// `name`. Builder-style, chainable per node. A real node-observation adapter + /// refreshing this fleet every pass is a follow-up + /// (`respond::actuator::node_containment`'s own doc); until then the map is exactly + /// what a caller (today, only tests) seeded it with. + pub fn with_node_fact(mut self, fact: respond::actuator::node_containment::NodeFact) -> Self { + self.node_facts.insert(fact.name.clone(), fact); + self + } + + /// Revert a standing `ContainNode` mitigation (ADR-0040 §5): uncordon `mitigation`'s + /// target host and lift its co-resident denies, through + /// [`respond::actuator::node_containment::NodeContainmentRevert`] — never through the + /// generic network `actuator`, whose `revert()` speaks a different object shape entirely + /// (an `AdminNetworkPolicy`/`NetworkPolicy` delete, meaningless for a cordoned `Node`) + /// and would silently leave the node cordoned. + /// + /// Skips (rather than fabricates) when either half of this pass's `ContainNode` + /// readiness is missing: no live actuator attached yet + /// ([`Self::with_node_containment_actuator`], a node-observation follow-up), or no + /// observed [`NodeFact`](respond::actuator::node_containment::NodeFact) for this host + /// ([`Self::with_node_fact`]) — the same "no fabricated no-data pass" discipline + /// [`respond::actuator::node_containment`]'s own doc already applies to the cordon + /// rails. When both are present, the attached actuator's OWN ownership self-gate + /// (`revert_decision`) is still what decides whether the uncordon actually happens — + /// this call only ever REACHES that gate, never bypasses it. + pub(super) async fn revert_contain_node( + &self, + mitigation: &Mitigation, + graph: &graph::SecurityGraph, + ) { + let Some(actuator) = &self.node_containment_actuator else { + tracing::warn!( + cut = %mitigation.cut.from.0, + "no node-containment actuator attached; standing ContainNode cut left in place" + ); + return; + }; + let host_name = mitigation.cut.from.short(); + let Some(target) = self.node_facts.get(host_name) else { + tracing::warn!( + node = %host_name, + "no observed NodeFact for this host; ContainNode revert skipped (ownership \ + cannot be verified)" + ); + return; + }; + let co_resident = + respond::actuator::node_containment::co_resident_denies(graph, &mitigation.cut.from); + actuator.revert(mitigation, target, &co_resident).await; + } +} diff --git a/engine/src/engine/respond/actuator/node_containment.rs b/engine/src/engine/respond/actuator/node_containment.rs index 016776f..b071b65 100644 --- a/engine/src/engine/respond/actuator/node_containment.rs +++ b/engine/src/engine/respond/actuator/node_containment.rs @@ -2,12 +2,19 @@ //! default-deny rendering, and the deterministic rails that gate it. Split out of the //! actuator module root purely to keep every file under the 1,000-line cap (repo CLAUDE.md). //! -//! **This module is unit-tested, wired nowhere live yet.** `ContainNode` is +//! **The apply side is unit-tested, wired nowhere live yet; the revert side IS wired into +//! `Engine::process`'s break-glass/self-revert loop.** `ContainNode` is //! `is_additive_live() == false` ([`ProposedAction::is_additive_live`]), so //! [`super::decide`] already routes every `ContainNode` mitigation to //! [`super::Decision::Forbidden`] regardless of what these rails would say — there is no //! `node` arming rung to escalate past (ADR-0040 §6, a separate ticket), so nothing here can -//! become live-armable through this module alone. What IS delivered: +//! become live-*applied* through this module alone. But ADR-0040 §5 also requires +//! `ContainNode` to join the armed-set revert trigger (ADR-0036) and the standard ledger +//! self-revert (ADR-0017) — that half does not depend on the apply-side rung existing at +//! all, so it is wired now: `Engine::process`'s self-revert loop routes a standing +//! `ContainNode` reversion through [`NodeContainmentRevert`] rather than the generic network +//! `actuator`, whose `revert()` speaks a different object shape entirely and would silently +//! leave the node cordoned. What IS delivered: //! //! - [`render_cordon`]/[`render_uncordon`]: the pure `Node.spec.unschedulable` patch, //! carrying [`CORDON_OWNER_ANNOTATION`] so a revert only ever lifts a cordon protector @@ -22,10 +29,10 @@ //! [`NodeFact`] fleet so they're unit-testable without a live cluster and independent of //! any arming/enabled state — a rail refusal is exactly as meaningful in shadow as it //! would be armed. -//! - [`live`]'s [`NodeContainmentActuator`]: the cluster-facing apply/revert glue a future -//! ticket's break-glass/self-revert verification and rung-3 wiring calls into. Thin and -//! untested against a real cluster, like [`super::KubeActuator`]/[`super::IsolationActuator`] -//! — [`render_cordon`]/[`render_uncordon`] are the unit-tested pure half. +//! - [`live`]'s [`NodeContainmentActuator`]/[`NodeContainmentRevert`]: the cluster-facing +//! apply/revert glue. Thin and untested against a real cluster, like +//! [`super::KubeActuator`]/[`super::IsolationActuator`] — [`render_cordon`]/ +//! [`render_uncordon`] are the unit-tested pure half. //! //! **Node role/schedulability observation is a follow-up, not this ticket.** [`NodeFact`] //! is the fleet-state shape the rails need, but nothing in the engine watches Kubernetes @@ -33,11 +40,13 @@ //! adapter, ADR-0040 §3), which needs no new RBAC. Populating a //! real `NodeFact` fleet needs a `nodes` `get/list/watch` grant this ticket deliberately //! does not add (ADR-0040 §7 ships the actuator split from the chart/RBAC change; the -//! ticket that adds this observation is the natural place to also wire these rails into -//! `Engine::process`'s per-pass loop). Evaluating a rail against a fabricated "no data" +//! ticket that adds this observation is the natural place to also wire the apply side of +//! these rails into `Engine::process`'s per-pass loop, and to keep `Engine`'s attached +//! `NodeFact` fleet fresh every pass). Evaluating a rail against a fabricated "no data" //! fleet would silently default it to PASS — exactly the "weakening the rail" the ADR's -//! build-settled note warns against — so this module is deliberately not wired into the -//! live per-pass loop until real fleet data exists. +//! build-settled note warns against — so the engine's self-revert loop skips (rather than +//! fabricates) a revert for any host with no attached [`NodeContainmentRevert`] or no +//! observed [`NodeFact`], the same discipline this doc already applied to the cordon rails. use crate::engine::graph::{NodeKey, SecurityGraph}; use crate::engine::respond::{ @@ -45,7 +54,7 @@ use crate::engine::respond::{ }; mod live; -pub use live::NodeContainmentActuator; +pub use live::{NodeContainmentActuator, NodeContainmentRevert}; /// The annotation a cordon carries to record that PROTECTOR placed it (ADR-0040 §5). A /// revert only lifts a cordon carrying this — never a human's or the cluster diff --git a/engine/src/engine/respond/actuator/node_containment/live.rs b/engine/src/engine/respond/actuator/node_containment/live.rs index 79aa4f0..883f92b 100644 --- a/engine/src/engine/respond/actuator/node_containment/live.rs +++ b/engine/src/engine/respond/actuator/node_containment/live.rs @@ -15,6 +15,37 @@ use crate::engine::respond::actuator::{Actuation, Actuator, IsolationActuator, c use super::{NodeFact, render_cordon, render_uncordon, revert_decision}; +/// The revert half of [`NodeContainmentActuator`]'s contract, pulled into a trait purely so +/// the break-glass/self-revert loop ([`crate::engine::Engine::process`]) can hold either the +/// live cluster-facing actuator or a test double — mirroring how [`Actuator`] lets +/// [`crate::engine::respond::actuator::KubeActuator`] and a recording double stand in for +/// each other there. `NodeContainmentActuator` itself deliberately does not implement +/// `Actuator` (this module's own doc: one `ContainNode` mitigation maps to MANY cluster +/// objects, a shape `Actuator`'s single-mitigation signature can't express) — this trait +/// keeps that multi-object shape (mitigation + observed [`NodeFact`] + the co-resident set) +/// while still giving the engine a swappable seam onto it. +#[async_trait::async_trait] +pub trait NodeContainmentRevert: Send + Sync { + async fn revert( + &self, + mitigation: &Mitigation, + target: &NodeFact, + co_resident: &[Mitigation], + ) -> Actuation; +} + +#[async_trait::async_trait] +impl NodeContainmentRevert for NodeContainmentActuator { + async fn revert( + &self, + mitigation: &Mitigation, + target: &NodeFact, + co_resident: &[Mitigation], + ) -> Actuation { + NodeContainmentActuator::revert(self, mitigation, target, co_resident).await + } +} + /// A dynamic `Api` for the cluster-scoped core `Node` resource. fn node_api(client: &kube::Client) -> kube::Api { let gvk = kube::core::GroupVersionKind::gvk("", "v1", "Node");