diff --git a/engine/src/engine/respond/actuator/node_containment.rs b/engine/src/engine/respond/actuator/node_containment.rs index c52d3ab..c8e664f 100644 --- a/engine/src/engine/respond/actuator/node_containment.rs +++ b/engine/src/engine/respond/actuator/node_containment.rs @@ -10,7 +10,17 @@ //! [`crate::engine::respond::quarantine_workload_link`]'s exact self-reference shape (and //! therefore [`super::render_isolation`]'s renderer) per co-resident LABELLED workload //! ([`crate::engine::respond::co_resident_workloads`]) — an unlabeled pod declines exactly -//! like every other quarantine candidate. +//! like every other quarantine candidate. UNFILTERED by `enforceScope` — the revert seam +//! (`live::NodeContainmentActuator::revert`) needs the full set, since lifting a deny +//! protector never placed under any historical scope is a harmless no-op, but filtering +//! it would orphan an out-of-scope deny protector DID place under a wider, earlier scope. +//! - [`co_resident_denies_in_scope`]/[`ScopedDenies`]: the `enforceScope`-confined subset +//! (ADR-0040 addendum, ADR-0021), and the newtype that is the ONLY door into +//! [`live::NodeContainmentActuator::apply`] — unscoped scope returns the full set +//! (historical meaning), a configured scope retains only the co-resident denies +//! [`super::ActuationScope::in_scope`] itself accepts, so the actuation boundary is +//! scope-confined by TYPE, not caller discipline. [`contain_node_in_scope`] is rebased on +//! this same function, so eligibility and the apply subset share one scope-match source. //! - [`cordon_decision`]/[`revert_decision`]: the deterministic rails (control-plane //! exclusion, one-node cap, the two-worker floor, ownership-gated revert), pure over a //! [`NodeFact`] fleet so they're unit-testable without a live cluster and independent of @@ -37,12 +47,15 @@ //! - [`live`]'s [`NodeContainmentActuator`]/[`NodeContainmentRevert`]: the cluster-facing //! REVERT glue only (lifting an already-standing cut via break-glass/self-revert, //! `crate::engine::node_containment_revert`) — there is no corresponding live APPLY call -//! site; `NodeContainmentActuator::apply` exists and is unit-tested in isolation for a -//! future human-approval-to-apply flow (out of scope here), but nothing in `Engine` -//! invokes it. Thin and untested against a real cluster, like +//! site. Per the ADR-0040 addendum, protector never grows an in-product approve→apply +//! path for node containment at all (the durable act stays out-of-band, a human cordon + +//! runbook); `NodeContainmentActuator::apply` exists and is unit-tested in isolation as +//! the actuator's glue for that out-of-band act, gated on [`ScopedDenies`] by type so it +//! can never be called with an unfiltered set, but nothing in `Engine` invokes it. Thin +//! and untested against a real cluster, like //! [`super::KubeActuator`]/[`super::IsolationActuator`] — [`render_cordon`]/ -//! [`render_uncordon`]/[`evaluate_proposal`]/[`contain_node_in_scope`] are the unit-tested -//! pure half. +//! [`render_uncordon`]/[`evaluate_proposal`]/[`contain_node_in_scope`]/ +//! [`co_resident_denies_in_scope`] are the unit-tested pure half. //! //! `ContainNode` is `is_additive_live() == false` ([`ProposedAction::is_additive_live`]), so //! [`super::decide`]'s generic AutoApply path always routes it to @@ -149,6 +162,70 @@ pub fn co_resident_denies(graph: &SecurityGraph, host: &NodeKey) -> Vec ScopedDenies { + let denies = co_resident_denies(graph, host); + let scoped = if scope.is_unscoped() { + denies + } else { + denies + .into_iter() + .filter(|deny| scope.in_scope(deny)) + .collect() + }; + ScopedDenies(scoped) +} + +/// An `enforceScope`-confined co-resident deny set — constructible ONLY by +/// [`co_resident_denies_in_scope`], so [`live::NodeContainmentActuator::apply`] (the only +/// function that accepts one) can never be handed the unfiltered [`co_resident_denies`] set +/// by a forgetful caller. Safe by construction, mirroring how +/// [`live::NodeContainmentActuator::revert`] is safe by construction on the ownership rail +/// rather than caller discipline. The inner `Vec` is deliberately private with no public +/// constructor besides [`co_resident_denies_in_scope`] — attempting to build one directly +/// from outside this module does not compile: +/// +/// ```compile_fail +/// # use protector::engine::respond::Mitigation; +/// # use protector::engine::respond::actuator::node_containment::ScopedDenies; +/// let denies: Vec = Vec::new(); +/// let _ = ScopedDenies(denies); // E0603: tuple struct constructor is private +/// ``` +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ScopedDenies(Vec); + +impl ScopedDenies { + /// The confined deny set as a slice — the shape [`live::NodeContainmentActuator::apply`] + /// iterates. + pub fn as_slice(&self) -> &[Mitigation] { + &self.0 + } + + /// Whether the confined set is empty. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + /// A per-pass fact for one node in the fleet — the shape [`cordon_decision`]/ /// [`revert_decision`] need, sourced from an observed Kubernetes `Node` /// ([`crate::engine::observe::adapter::node_fact::observe_node_facts`]): name; whether it @@ -281,21 +358,20 @@ pub fn revert_decision(target: &NodeFact) -> Result<(), RailRefusal> { /// construction ([`crate::engine::respond::contain_node_link`]'s own doc). /// /// In scope iff `scope` is unscoped (the historical, no-`enforceScope`-configured -/// behavior), OR at least one co-resident deny target is itself in scope. A host with no -/// co-resident LABELLED pod at all (nothing for [`co_resident_denies`] to return) is never -/// presumed in scope once a scope IS configured — the same "decline rather than widen" -/// discipline [`co_resident_denies`] already applies to an unlabelled pod. +/// behavior), OR at least one co-resident deny target is itself in scope — i.e. the +/// [`co_resident_denies_in_scope`] subset is non-empty. Rebased on that same function +/// (ADR-0040 addendum) so eligibility and the apply-side subset share one scope-match +/// source rather than two implementations that could drift apart; behavior-identical to +/// the prior direct any-match. A host with no co-resident LABELLED pod at all (nothing for +/// [`co_resident_denies`] to return) is never presumed in scope once a scope IS configured +/// — the same "decline rather than widen" discipline [`co_resident_denies`] already applies +/// to an unlabelled pod. pub fn contain_node_in_scope( graph: &SecurityGraph, host: &NodeKey, scope: &ActuationScope, ) -> bool { - if scope.is_unscoped() { - return true; - } - co_resident_denies(graph, host) - .iter() - .any(|deny| scope.in_scope(deny)) + scope.is_unscoped() || !co_resident_denies_in_scope(graph, host, scope).is_empty() } /// What [`evaluate_proposal`] decided for one active `ContainNode` mitigation this pass. diff --git a/engine/src/engine/respond/actuator/node_containment/live.rs b/engine/src/engine/respond/actuator/node_containment/live.rs index 90ec30d..083ed5b 100644 --- a/engine/src/engine/respond/actuator/node_containment/live.rs +++ b/engine/src/engine/respond/actuator/node_containment/live.rs @@ -8,13 +8,16 @@ //! ([`crate::engine::node_containment_revert`]), self-gated on [`super::revert_decision`] //! regardless of caller discipline. **[`Self::apply`] has NO call site in `Engine`** — //! ADR-0040 §5 makes a node cut propose-first by construction (never auto-applied, at any -//! arming rung), so nothing in the engine's per-pass loop ever reaches it; it exists, -//! tested in isolation, for a future human-approval-to-apply flow. +//! arming rung), and the ADR-0040 addendum settles that protector never grows an in-product +//! approve→apply path for node containment at all, so nothing in the engine's per-pass loop +//! ever reaches it. It exists, tested in isolation, as the actuator glue for the human's +//! out-of-band act (cordon + runbook) — gated on [`ScopedDenies`] BY TYPE so it can never +//! be called with an `enforceScope`-unfiltered set. use crate::engine::respond::Mitigation; use crate::engine::respond::actuator::{Actuation, Actuator, IsolationActuator, cut_label}; -use super::{NodeFact, render_cordon, render_uncordon, revert_decision}; +use super::{NodeFact, ScopedDenies, 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 @@ -71,10 +74,13 @@ impl NodeContainmentActuator { } /// Cordon `mitigation`'s target host and default-deny every co-resident mitigation in - /// `co_resident` (built by [`super::co_resident_denies`]). Best-effort: a co-resident - /// deny failure is logged by [`IsolationActuator`] and does not roll back the cordon — a - /// partial containment (node cordoned, some pods still reachable) is safer than none. - pub async fn apply(&self, mitigation: &Mitigation, co_resident: &[Mitigation]) -> Actuation { + /// `co_resident` (built by [`super::co_resident_denies_in_scope`] — the ONLY door into + /// this method takes the `enforceScope`-confined [`ScopedDenies`] set BY TYPE, ADR-0040 + /// addendum: a caller cannot hand this the unfiltered [`super::co_resident_denies`] set + /// even by mistake). Best-effort: a co-resident deny failure is logged by + /// [`IsolationActuator`] and does not roll back the cordon — a partial containment (node + /// cordoned, some pods still reachable) is safer than none. + pub async fn apply(&self, mitigation: &Mitigation, co_resident: &ScopedDenies) -> Actuation { let Some(manifest) = render_cordon(mitigation) else { tracing::warn!(cut = %cut_label(mitigation), "not a ContainNode mitigation; nothing to cordon"); return Actuation::DryRun; @@ -85,7 +91,7 @@ impl NodeContainmentActuator { } tracing::info!(node = %host_name, "cordoned node (ADR-0040 containment)"); let isolation = IsolationActuator::new(self.client.clone()); - for co_resident_mitigation in co_resident { + for co_resident_mitigation in co_resident.as_slice() { isolation.apply(co_resident_mitigation).await; } Actuation::Applied diff --git a/engine/src/engine/respond/actuator/node_containment/tests.rs b/engine/src/engine/respond/actuator/node_containment/tests.rs index 49b4b30..692bf36 100644 --- a/engine/src/engine/respond/actuator/node_containment/tests.rs +++ b/engine/src/engine/respond/actuator/node_containment/tests.rs @@ -317,6 +317,103 @@ fn rail_decisions_take_no_arming_state_and_so_are_exactly_as_meaningful_in_shado assert_eq!(revert_decision(&unowned), Err(RailRefusal::NotOwned)); } +// --- co_resident_denies_in_scope / ScopedDenies (ADR-0040 addendum, ADR-0021) --- + +#[test] +fn co_resident_denies_in_scope_returns_the_full_set_when_unscoped() { + let snap = Snapshot { + pods: vec![ + scheduled_pod("victim", "node-1", json!({"app": "victim"})), + scheduled_pod("neighbor", "node-1", json!({"app": "neighbor"})), + ], + ..Default::default() + }; + let graph = build_graph(&snap, &default_adapters()); + let host = NodeKey("host/node-1".into()); + + let scoped = co_resident_denies_in_scope(&graph, &host, &ActuationScope::unscoped()); + assert_eq!( + scoped.as_slice().len(), + co_resident_denies(&graph, &host).len(), + "unscoped returns the same full set co_resident_denies does" + ); +} + +#[test] +fn co_resident_denies_in_scope_retains_only_the_in_scope_namespace() { + let snap = Snapshot { + pods: vec![ + scheduled_pod("victim", "node-1", json!({"app": "victim"})), + scheduled_pod("neighbor", "node-1", json!({"app": "neighbor"})), + ], + ..Default::default() + }; + let graph = build_graph(&snap, &default_adapters()); + let host = NodeKey("host/node-1".into()); + // Both pods live in the "app" namespace (`scheduled_pod`'s own fixture), so scoping to + // it must retain BOTH — this asserts the subset tracks the scope match, not an + // arbitrary truncation. + let scope = ActuationScope::enforce_namespaces(["app".to_string()]); + + let scoped = co_resident_denies_in_scope(&graph, &host, &scope); + assert_eq!(scoped.as_slice().len(), 2); +} + +#[test] +fn co_resident_denies_in_scope_is_empty_when_no_co_resident_pod_is_in_scope() { + let snap = Snapshot { + pods: vec![scheduled_pod("victim", "node-1", json!({"app": "victim"}))], + ..Default::default() + }; + let graph = build_graph(&snap, &default_adapters()); + let host = NodeKey("host/node-1".into()); + let scope = ActuationScope::enforce_namespaces(["payments".to_string()]); + + let scoped = co_resident_denies_in_scope(&graph, &host, &scope); + assert!( + scoped.is_empty(), + "the only co-resident pod is outside the configured scope" + ); +} + +#[test] +fn co_resident_denies_in_scope_matches_on_the_label_axis() { + let snap = Snapshot { + pods: vec![ + scheduled_pod("victim", "node-1", json!({"app": "victim", "tier": "hot"})), + scheduled_pod("neighbor", "node-1", json!({"app": "neighbor"})), + ], + ..Default::default() + }; + let graph = build_graph(&snap, &default_adapters()); + let host = NodeKey("host/node-1".into()); + // Namespace axis is empty; only the label axis is configured, and only "victim" + // carries it — the subset must track the LABEL match, not the namespace (both pods + // share the same "app" namespace). + let scope = ActuationScope::new( + Default::default(), + vec![("tier".to_string(), "hot".to_string())], + ); + + let scoped = co_resident_denies_in_scope(&graph, &host, &scope); + assert_eq!(scoped.as_slice().len(), 1); + assert_eq!(scoped.as_slice()[0].cut.from.0, "workload/app/Pod/victim"); +} + +// The compile-time half of "constructible only by `co_resident_denies_in_scope`" lives as +// a `compile_fail` doctest on the [`ScopedDenies`] type itself (this `tests` module is +// `#[cfg(test)]`, which rustdoc never compiles, so a doctest here would silently never +// run). This is the runtime witness: the ONLY way this test (or anything outside the +// module) obtains a `ScopedDenies` is through the function — there is no +// `ScopedDenies::new`/`From`/public tuple constructor to call instead. +#[test] +fn scoped_denies_has_no_public_constructor_besides_co_resident_denies_in_scope() { + let graph = crate::engine::graph::SecurityGraph::new(); + let host = NodeKey("host/nonexistent".into()); + let scoped = co_resident_denies_in_scope(&graph, &host, &ActuationScope::unscoped()); + assert!(scoped.is_empty()); +} + // --- contain_node_in_scope: enforceScope confinement (ADR-0021) --- #[test]