fix(engine): arm the node class — propose-only ContainNode, scope-confined, CP fail-closed - #324
Merged
thejefflarson merged 3 commits intoAug 3, 2026
Conversation
…, live rail wiring (ADR-0040) Rung 3 `node` (strictly above `quarantine`) arms ContainNode on the arming ladder; the chart's ClusterRole gains an always-on metadata-only `nodes` read (fleet observation for the rails) and a `nodes` patch write gated on `mode: enforce` + `enforceRung: node`, mirroring the existing posture-derived RBAC pattern. A new metadata-only Node watch (`observe::adapter::node_fact`) feeds the deterministic rails a real NodeFact fleet each pass. Engine::process now evaluates the rails (`node_containment::evaluate_apply`) against that fleet for every active ContainNode mitigation and fires the `applied`/ `rail_refused` metric events, failing closed on a host absent from the fleet (RailRefusal::UnknownNode) rather than fabricating a passing rail. Startup (engine + chart) refuses `enforceRung: node` without `mode: enforce` + a non-empty enforceScope, mirroring the existing empty-scope refusal. The generic decide()/AutoApply path still forbids ContainNode unconditionally (is_additive_live() == false); this ships ContainNode's own apply-side gate instead, armed by the rung and safety-checked by the deterministic rails rather than the generic blast-radius check. The break-glass/self-revert uncordon path is out of scope here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP
…-closed (ADR-0040 §5) Rework of the node-class arming PR after review found ADR-0040 §5 violations: 1. CRITICAL — removed the auto-cordon path entirely. `evaluate_apply`/`ApplyOutcome` (which called `NodeContainmentActuator::apply` once armed+in-scope+rails-clean) are replaced by `evaluate_proposal`/`ProposalOutcome`, which only ever decides whether to SURFACE a proposal. ADR-0040 §5 is explicit that a node cut is propose-first by construction (alive collateral always present) — the deterministic rails are a bound on the human-approval gate, never a replacement for it. There is now no code path in Engine that calls `NodeContainmentActuator::apply` at all; only the REVERT half (`with_node_containment_actuator`, from #323) is wired. The `applied` metric event is gone; `proposed` now also fires (level-triggered) whenever the rails are rail-clean and armed, alongside the pre-existing edge-triggered ledger proposal event. 2. HIGH — fixed the enforceScope bypass. `ActuationScope::in_scope` resolves a namespace per endpoint and returns vacuously true for ContainNode's host self-reference (which has none), so `enforceScope` confined nothing for this class. `contain_node_in_scope` confines it instead through the co-resident labelled workload set (which structurally includes the boundary-broken trigger), reusing `co_resident_denies` + the existing per-mitigation `in_scope` check on each candidate — no scope bypass on the private internals, and a host with no in-scope co-resident pod is never proposed. 3. MEDIUM — control-plane detection now fails closed: recognizes the legacy `node-role.kubernetes.io/master` label and a control-plane-shaped taint (either canonical or legacy key, any effect) as additional, independent control-plane signals, union'd with the canonical label. A node with none of the three signals remains the ordinary cordon-eligible worker default (the correct reading for vanilla/kubeadm clusters, where workers carry no role marker at all). 4. MEDIUM — rewrote the chart/README language describing `enforceRung: node` and the engine's cut-severity ladder doc to state plainly that ContainNode is eligible to PROPOSE, never to auto-apply, at any rung. 5. LOW — documented (rather than implemented) that the two-worker floor counts schedulable nodes, not Ready ones; `Node.status` stays unread per the observation adapter's metadata-only discipline. Rebased onto main (#323's NodeContainmentRevert/`with_node_fact` seam) and composed with it: `Engine::process` now refreshes `node_facts` every pass from the real observation adapter (falling back to test-seeded facts when `Snapshot::nodes` is empty), and the real `NodeContainmentActuator` is wired as the revert actuator in `run_loop.rs`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP
thejefflarson
force-pushed
the
thejefflarson/jef-732-arm-the-node-class-armingrungnode-rung-3-posture-derived
branch
from
August 3, 2026 06:58
f7a318b to
d18c196
Compare
…DR-0040) The engine now runs an unconditional, metadata-only Node watch (observe::adapter::node_fact::observe_node_facts, ADR-0040 §3/§6). The Helm chart's ClusterRole was updated to grant nodes:[get,list,watch] always-on, but the e2e harness deploys from its own inline ClusterRole in scripts/e2e.sh, which was not. Without the grant the Node watch 403-floods the run loop every pass and starves the deterministic proof passes, so the structural web->session-key chain never proves within 300s and e2e fails. Mirror the chart's always-on metadata-only read here. No `patch` grant: e2e never arms enforceRung: node (ContainNode is propose-only at every rung, ADR-0040 §5), so the posture-derived write surface stays absent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP
thejefflarson
deleted the
thejefflarson/jef-732-arm-the-node-class-armingrungnode-rung-3-posture-derived
branch
August 3, 2026 07:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes JEF-732.
Makes protector's
ContainNodenode-containment class eligible to propose, perdocs/adr/0040-node-scoped-containment-mechanism-escalation.md§5–6:node(arming_ladder.rs) — strictly abovequarantine(
edge-cut < quarantine < node), armsContainNodein addition to its narrowerpredecessors. Neither lower rung ever arms it.
enforceRung: nodeunless
mode: enforcewith a non-emptyenforceScope, mirroring the existingenforce-with-empty-scope refusal.
observe/adapter/node_fact.rs) — a metadata-only mappingfrom the watched
Nodefleet (name, control-plane signal,spec.unschedulable, thecordon-ownership annotation) to the rails'
NodeFactshape. No.dataread;statusisnever read either.
charts/protector/templates/clusterrole.yaml) —nodes: [get, list, watch]always-on (metadata-only read, matching every other always-grantedread);
nodes: [patch]ONLY undermode: enforce+enforceRung: node, cluster-scoped.Engine::process) — for every activeContainNodemitigation, evaluates the deterministic rails (
node_containment::evaluate_proposal,wrapping
cordon_decision) against the pass's observedNodeFactfleet and firesproposed/rail_refusedmetric events, failing closed on an unknown host.REWORK (this PR was revised after review)
An initial version of this PR auto-cordoned once a
ContainNodemitigation cleared therails + arming + scope. Review correctly found this violates ADR-0040 §5, which is
explicit: "a real node always has alive collateral, so the existing blast/alive-collateral
gate routes every node cut to human approval even at the armed rung; propose-first is
structural, not a toggle." Fixed, along with two issues the same pass surfaced:
evaluate_apply/ApplyOutcome(whichcalled
NodeContainmentActuator::apply) are replaced byevaluate_proposal/ProposalOutcome, which only ever decides whether to SURFACE a proposal. There is nocode path in
Enginethat callsNodeContainmentActuator::applyat all now — only theREVERT half is wired (
with_node_containment_actuator, from fix(engine): wire ContainNode into break-glass + ledger self-revert (ADR-0040 §5) #323, now pointed at thereal cluster actuator).
decide()'s genericForbiddenpath forContainNode(
is_additive_live() == false) is untouched. New engine-level acceptance test(
contain_node_never_reaches_an_apply_call_even_fully_eligible) proves this with a spyActuatorover a fully-eligible (armed, unscoped, rails-clean) scenario.enforceScopebypass.ActuationScope::in_scoperesolves anamespace per endpoint and returns vacuously
trueforContainNode'shost/<name>self-reference (which has none) —
enforceScopeconfined nothing for this class.contain_node_in_scopeconfines it instead through the co-resident LABELLED workloadset (
co_resident_denies), which structurally includes the boundary-broken trigger(scheduled on the same host by construction). A host with no in-scope co-resident pod
is never proposed.
node_fact.rsnow also recognizesthe legacy
node-role.kubernetes.io/masterlabel and a control-plane-shaped taint(canonical or legacy key, any effect) as independent control-plane signals, unioned with
the canonical label. (A node with none of the three signals stays the ordinary
cordon-eligible worker default — the sound reading for vanilla/kubeadm clusters, where
workers carry no role marker at all; documented inline as the judgment call.)
enforceRung: nodetostate plainly that it makes
ContainNodeeligible to be proposed, never toauto-apply, at any rung.
schedulable nodes, not Ready ones; cross-referencing pod-health-per-node was judged
disproportionate scope for this fix, and
Node.statusstays unread per the observationadapter's metadata-only discipline.
Also rebased onto
main(post-#323, which added theNodeContainmentRevert/with_node_factseam) and composed with it:Engine::processnow refreshes the sharednode_factsmap every pass from the real observation adapter (falling back to anytest-seeded fact when
Snapshot::nodesis empty, so #323's existing tests are untouched),and the real
NodeContainmentActuatoris wired as the revert actuator inrun_loop.rs.Design note, still holds:
ContainNodegets its own PROPOSAL-side gate(
evaluate_proposal, called fromEngine::process) rather than reusing the genericdecide()path, becausedecide()structurally forbids it forever(
is_additive_live() == false). ArmingenforceRung: nodeonly changes whether arails-clean cut is surfaced as an actionable proposal instead of a bare "not auto-enabled"
line — it never causes a cluster write.
Out of scope (a sibling ticket, JEF-733, merged as #323): the break-glass/self-revert
UNCORDON path.
Invariants preserved
mode: auditstill renders zeronodeswrite grant(verified via
helm template) andContainNodeis stillForbiddenthrough the genericpath regardless of rung, and now has no apply path at all.
PROTECTOR_*_ENABLEtoggle — armed viamode+ the existing ladder rung only.engine/src/engine/mod.rsat 969lines — worth watching, not yet over).
../clusterfork changes (that port is a separate ticket).ADR-0040throughout.Test plan
cargo test(workspace): 1212 lib tests + 10main.rsposture tests + 3 guard testbinaries, all green. Covers:
ArmingRung::NodearmsContainNode(and only that rungdoes);
Posture::from_envrefusesenforceRung=nodewithoutmode=enforce;observe_node_facts(metadata-only, legacy label, taint, unrelated-taint,no-signal-default);
evaluate_proposal's full decision space (ineligible/none,fail-closed on unknown node, rail refusal surfaced, proposed with/without a standing
cordon);
contain_node_in_scope(unscoped, in-scope co-resident match, out-of-scopeco-resident, unlabelled-only host); and the engine-level spy-actuator acceptance test
proving no apply call is ever reachable.
main.rs's env-var-mutating posture tests(exposed by the third such test this PR adds) with a shared
Mutex.cargo fmt --check/cargo clippy --all-targets -- -D warnings: clean.helm lint charts/protector: clean.helm templateverified locally for all threepostures (audit / enforce+quarantine / enforce+node) matching the CI assertions added to
.github/workflows/chart.yml, including theenforceRung: nodewithoutmode: enforcerefusal.
/soundcheck:pr-review(manual, per-category) — no Critical/High findings. Ran asingle-pass
/simplify(Agent tool unavailable in this context) on the original diff,which folded a duplicated fleet lookup into the (now-removed) apply path.
🤖 Generated with Claude Code