From 876c51d79e61c1c309a47cf42f711a526414c92f Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:31:58 +0000 Subject: [PATCH 1/5] Move ClientSealedValue into sealed.rs Config imported Sealed's type and sealed.rs imported it back. The type now lives beside SealedValue, and config re-exports it for existing callers. --- core/crates/ployz-core/src/config/mod.rs | 1 + core/crates/ployz-core/src/config/service.rs | 15 ++------------- core/crates/ployz-core/src/sealed.rs | 16 +++++++++++++--- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/core/crates/ployz-core/src/config/mod.rs b/core/crates/ployz-core/src/config/mod.rs index 5a09e4cf15..8ded8f5c9d 100644 --- a/core/crates/ployz-core/src/config/mod.rs +++ b/core/crates/ployz-core/src/config/mod.rs @@ -19,6 +19,7 @@ mod sync; mod validation; mod variables; +pub use crate::sealed::ClientSealedValue; pub use branch_plan::*; pub use change_set::*; pub use change_set_types::*; diff --git a/core/crates/ployz-core/src/config/service.rs b/core/crates/ployz-core/src/config/service.rs index 0a352e72ef..dbd2dbfbc8 100644 --- a/core/crates/ployz-core/src/config/service.rs +++ b/core/crates/ployz-core/src/config/service.rs @@ -4,9 +4,8 @@ use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; use ts_rs::TS; -use uuid::Uuid; -use crate::{RestartPolicy, SealedValue, ServiceName, UpdateOrder}; +use crate::{ClientSealedValue, RestartPolicy, ServiceName, UpdateOrder}; /// Authored Cloud settings. Runtime specs are compiled from this configuration; /// they cannot reconstruct repository authority, build policy, or credentials. @@ -325,17 +324,6 @@ pub struct EncryptedSecretValue { pub ciphertext: String, } -/// A Sealed value the browser sealed to one Environment's Sealing Key, bound to -/// the variable it was authored as. Cloud carries it and never opens it. -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TS)] -#[serde(rename_all = "camelCase", deny_unknown_fields)] -pub struct ClientSealedValue { - /// The Environment whose Sealing Key opens it: in any other it is Not set. - #[ts(type = "string")] - pub sealed_for: Uuid, - pub value: SealedValue, -} - /// A secret's value sealed with the Store's own key: the envelope registry /// credentials use. pub type LegacyCiphertext = EncryptedSecretValue; @@ -395,6 +383,7 @@ pub enum ServiceEnvValue { #[cfg(test)] mod tests { use serde_json::{Value, json}; + use uuid::Uuid; use super::*; use crate::config::{SavedVariableIntent, VariableValue}; diff --git a/core/crates/ployz-core/src/sealed.rs b/core/crates/ployz-core/src/sealed.rs index 30fbafb13d..f043d2da95 100644 --- a/core/crates/ployz-core/src/sealed.rs +++ b/core/crates/ployz-core/src/sealed.rs @@ -96,6 +96,17 @@ pub enum SealedScope { Service(#[ts(type = "string")] Uuid), } +/// A Sealed value the browser sealed to one Environment's Sealing Key, bound to +/// the variable it was authored as. Cloud carries it and never opens it. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, TS)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct ClientSealedValue { + /// The Environment whose Sealing Key opens it: in any other it is Not set. + #[ts(type = "string")] + pub sealed_for: Uuid, + pub value: SealedValue, +} + /// HPKE encapsulated key followed by the ciphertext. #[derive(Clone, Eq, PartialEq, Serialize, Deserialize, TS)] #[serde(try_from = "String", into = "String")] @@ -241,8 +252,7 @@ mod sealer { Ciphertext, PLATFORM_SEALED_INFO, PlatformPurpose, SEALED_VALUE_INFO, SealedIdentity, SealedScope, SealedValue, }; - use crate::SealingPublicKey; - use crate::config::ClientSealedValue; + use crate::{ClientSealedValue, SealingPublicKey}; type Kem = X25519HkdfSha256; @@ -501,7 +511,7 @@ mod sealer { assert_eq!( cluster.open( &authored(), - &crate::config::ClientSealedValue { + &crate::ClientSealedValue { sealed_for: authored().environment, value: as_env, } From b222f878c81bb1feb13565ce48d72c3b5f2a4336 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:37:02 +0000 Subject: [PATCH 2/5] Check statement lists and mismatches in one place The sorted-and-distinct list check had six copies across the signed bodies and the Mismatch loop had six more. Both now live in authority, so a bad list reads one problem and every check runs through first_mismatch. --- .../ployz-core/src/authority/canonical.rs | 67 +++++++++++++++++++ .../crates/ployz-core/src/authority/deploy.rs | 37 +++------- core/crates/ployz-core/src/authority/join.rs | 15 ++--- .../ployz-core/src/authority/proposal.rs | 15 ++--- .../ployz-core/src/authority/refusal.rs | 21 ++++++ .../ployz-core/src/authority/restart.rs | 36 +++------- core/crates/ployz-core/src/authority/rule.rs | 45 +++---------- .../ployz-core/src/authority/session.rs | 37 +++------- .../crates/ployz-core/src/authority/volume.rs | 21 +++--- 9 files changed, 144 insertions(+), 150 deletions(-) diff --git a/core/crates/ployz-core/src/authority/canonical.rs b/core/crates/ployz-core/src/authority/canonical.rs index bfcbe6d73e..ee1a4ee923 100644 --- a/core/crates/ployz-core/src/authority/canonical.rs +++ b/core/crates/ployz-core/src/authority/canonical.rs @@ -212,10 +212,77 @@ pub(super) fn write_hex(bytes: &[u8], formatter: &mut fmt::Formatter<'_>) -> fmt .try_for_each(|byte| write!(formatter, "{byte:02x}")) } +/// `deserialize_with` for a list a signed body keeps strictly ascending, so a set of +/// elements has one encoding and one signature input. +/// +/// # Errors +/// Returns a deserialize error when the list is not sorted and distinct. +pub(super) fn sorted_distinct<'de, D, T>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, + T: Deserialize<'de> + Ord, +{ + sorted_by(deserializer, |item: &T| item) +} + +/// [`sorted_distinct`] for elements ordered by one field of theirs, such as a Service's name. +/// +/// # Errors +/// Returns a deserialize error when the keys are not strictly ascending. +pub(super) fn sorted_by<'de, D, T, K>( + deserializer: D, + key: fn(&T) -> &K, +) -> Result, D::Error> +where + D: serde::Deserializer<'de>, + T: Deserialize<'de>, + K: Ord + ?Sized, +{ + let items = Vec::::deserialize(deserializer)?; + if !items.is_sorted_by(|left, right| key(left) < key(right)) { + return Err(serde::de::Error::custom("must be sorted and distinct")); + } + Ok(items) +} + #[cfg(test)] mod tests { use super::*; + #[derive(Debug, Deserialize)] + struct Lists { + #[serde(deserialize_with = "sorted_distinct")] + plain: Vec, + #[serde(deserialize_with = "by_first")] + pairs: Vec<(u8, u8)>, + } + + fn by_first<'de, D: serde::Deserializer<'de>>( + deserializer: D, + ) -> Result, D::Error> { + sorted_by(deserializer, |pair: &(u8, u8)| &pair.0) + } + + #[test] + fn a_list_must_be_sorted_and_distinct() { + let parse = |text: &str| serde_json::from_str::(text); + let lists = parse(r#"{"plain":[1,2],"pairs":[[1,9],[2,0]]}"#).unwrap(); + assert_eq!( + (lists.plain, lists.pairs), + (vec![1, 2], vec![(1, 9), (2, 0)]) + ); + assert!(parse(r#"{"plain":[],"pairs":[]}"#).is_ok()); + for text in [ + r#"{"plain":[2,1],"pairs":[]}"#, + r#"{"plain":[1,1],"pairs":[]}"#, + r#"{"plain":[],"pairs":[[2,0],[1,0]]}"#, + r#"{"plain":[],"pairs":[[1,0],[1,9]]}"#, + ] { + let error = parse(text).unwrap_err().to_string(); + assert!(error.starts_with("must be sorted and distinct"), "{error}"); + } + } + #[test] fn sorts_keys_and_drops_whitespace() { let value = serde_json::json!({"b": 1, "a": [true, "x"], "A": {"z": 0}}); diff --git a/core/crates/ployz-core/src/authority/deploy.rs b/core/crates/ployz-core/src/authority/deploy.rs index 01f20bfa87..0d0a6c9315 100644 --- a/core/crates/ployz-core/src/authority/deploy.rs +++ b/core/crates/ployz-core/src/authority/deploy.rs @@ -19,7 +19,7 @@ use super::{ canonical::{self, CanonicalError, Timestamp, hex_bytes, write_hex}, keys::{Approver, Purpose, Signed, Statement}, proposal::{self, ProposalRef, StepBody, StepKind}, - refusal::AuthorityRefusal, + refusal::{AuthorityRefusal, field, first_mismatch}, restart::SignerKey, session::{ActionFamily, GrantScope}, trusted_key::TrustedKeys, @@ -285,7 +285,7 @@ pub struct DeployApproval { pub operation: OperationId, pub namespace: Namespace, /// Sorted and distinct. - #[serde(deserialize_with = "sorted_distinct")] + #[serde(deserialize_with = "canonical::sorted_distinct")] pub machines: Vec, /// Sorted by name and distinct. #[serde(deserialize_with = "services_by_name")] @@ -466,28 +466,10 @@ impl DeployApproval { } } -fn sorted_distinct<'de, D: serde::Deserializer<'de>>( - deserializer: D, -) -> Result, D::Error> { - let machines = Vec::::deserialize(deserializer)?; - if !machines.is_sorted_by(|left, right| left < right) { - return Err(serde::de::Error::custom( - "Machines must be sorted and distinct", - )); - } - Ok(machines) -} - fn services_by_name<'de, D: serde::Deserializer<'de>>( deserializer: D, ) -> Result, D::Error> { - let services = Vec::::deserialize(deserializer)?; - if !services.is_sorted_by(|left, right| left.name < right.name) { - return Err(serde::de::Error::custom( - "Services must be sorted by name and distinct", - )); - } - Ok(services) + canonical::sorted_by(deserializer, |service: &ApprovedService| &service.name) } /// A Container this Machine is asked to create. @@ -543,17 +525,14 @@ pub fn verify( } None => open_signed(approval, facts)?, }; - let checks = [ - ("a Cluster", body.cluster == cluster), + first_mismatch([ + (field::CLUSTER, body.cluster == cluster), ( - "Machines", + field::MACHINES, body.machines.binary_search(facts.machine).is_ok(), ), - ("an Environment", &body.namespace == facts.namespace), - ]; - if let Some((field, _)) = checks.iter().find(|(_, holds)| !holds) { - return Err(AuthorityRefusal::Mismatch { field }); - } + (field::ENVIRONMENT, &body.namespace == facts.namespace), + ])?; if let Approver::Browser { grant, .. } = &by { grant.body.covers(&GrantScope { cluster, diff --git a/core/crates/ployz-core/src/authority/join.rs b/core/crates/ployz-core/src/authority/join.rs index 47b01e5736..eb7586158a 100644 --- a/core/crates/ployz-core/src/authority/join.rs +++ b/core/crates/ployz-core/src/authority/join.rs @@ -6,7 +6,10 @@ use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use ts_rs::TS; -use super::{AuthorityRefusal, Purpose, Signed, SigningPublicKey, Statement, Timestamp}; +use super::{ + AuthorityRefusal, Purpose, Signed, SigningPublicKey, Statement, Timestamp, + refusal::{field, first_mismatch}, +}; use crate::{DeviceKey, Machine, MachineId, SealingPublicKey, WireGuardPublicKey}; /// The immutable enrollment allocation. Runtime observations confer no authority. @@ -158,8 +161,8 @@ pub fn verify( now: Timestamp, ) -> Result { let body = verify_signature(signed, devices, now)?; - for (field, matches) in [ - ("a Cluster", body.cluster == cluster), + first_mismatch([ + (field::CLUSTER, body.cluster == cluster), ( "approved Devices", body.devices.contains(&body.signer) @@ -177,11 +180,7 @@ pub fn verify( "a sealing recipient", body.sealing_recipient == machine.public_key, ), - ] { - if !matches { - return Err(AuthorityRefusal::Mismatch { field }); - } - } + ])?; Ok(body) } diff --git a/core/crates/ployz-core/src/authority/proposal.rs b/core/crates/ployz-core/src/authority/proposal.rs index 9080eccc48..a1b64b90fd 100644 --- a/core/crates/ployz-core/src/authority/proposal.rs +++ b/core/crates/ployz-core/src/authority/proposal.rs @@ -19,7 +19,7 @@ use ts_rs::TS; use super::{ canonical::{Timestamp, hex_bytes, write_hex}, keys::{Approver, Purpose, Signed, Statement}, - refusal::AuthorityRefusal, + refusal::{AuthorityRefusal, field, first_mismatch}, restart::{Approved, RestartApproval, RestartFacts, SignerKey, WorkflowKind}, session::GrantScope, trusted_key::TrustedKeys, @@ -219,7 +219,7 @@ pub(super) fn open_step( .place() .filter(|place| place.operation == plan.operation); let listed = place.and_then(|place| plan.steps.get(place.index as usize)); - let checks = [ + first_mismatch([ ("the signer", by.is_some() && body.signer() == plan.signer), ("a proposal", place.is_some()), ( @@ -228,16 +228,13 @@ pub(super) fn open_step( listed.kind == body.kind() && listed.digest == StepDigest::of(&step.body) }), ), - ("a Cluster", plan.cluster == body.cluster()), - ("an Environment", &plan.environment == body.namespace()), + (field::CLUSTER, plan.cluster == body.cluster()), + (field::ENVIRONMENT, &plan.environment == body.namespace()), ( - "a window", + field::WINDOW, (plan.issued_at, plan.expires_at) == body.window(), ), - ]; - if let Some((field, _)) = checks.iter().find(|(_, holds)| !holds) { - return Err(AuthorityRefusal::Mismatch { field }); - } + ])?; let by = by.expect("the signer check passed"); Ok((body, by, cluster)) } diff --git a/core/crates/ployz-core/src/authority/refusal.rs b/core/crates/ployz-core/src/authority/refusal.rs index 2c47d46bf9..e7b11c6a03 100644 --- a/core/crates/ployz-core/src/authority/refusal.rs +++ b/core/crates/ployz-core/src/authority/refusal.rs @@ -30,3 +30,24 @@ pub enum AuthorityRefusal { #[error("this GitHub token already minted a Build Grant; a build asks for a new one")] ProofReplayed, } + +/// The first of `checks` that does not hold, as a [`AuthorityRefusal::Mismatch`] naming its field. +/// +/// # Errors +/// Returns [`AuthorityRefusal::Mismatch`] for the first `false` check, in order. +pub(super) fn first_mismatch( + checks: [(&'static str, bool); N], +) -> Result<(), AuthorityRefusal> { + match checks.into_iter().find(|(_, holds)| !holds) { + Some((field, _)) => Err(AuthorityRefusal::Mismatch { field }), + None => Ok(()), + } +} + +/// The names `Mismatch` gives the parts of a request that more than one statement checks. +pub(super) mod field { + pub const CLUSTER: &str = "a Cluster"; + pub const ENVIRONMENT: &str = "an Environment"; + pub const MACHINES: &str = "Machines"; + pub const WINDOW: &str = "a window"; +} diff --git a/core/crates/ployz-core/src/authority/restart.rs b/core/crates/ployz-core/src/authority/restart.rs index e2a6f4efa4..21d355ed78 100644 --- a/core/crates/ployz-core/src/authority/restart.rs +++ b/core/crates/ployz-core/src/authority/restart.rs @@ -5,10 +5,10 @@ use serde::{Deserialize, Serialize}; use ts_rs::TS; use super::{ - canonical::Timestamp, + canonical::{self, Timestamp}, keys::{Approver, Purpose, Signed, SigningPublicKey, Statement}, proposal::ProposalRef, - refusal::AuthorityRefusal, + refusal::{AuthorityRefusal, field, first_mismatch}, session::{ActionFamily, GrantScope}, trusted_key::TrustedKeys, }; @@ -100,7 +100,7 @@ pub struct RestartApproval { pub namespace: Namespace, pub service: ServiceName, /// Sorted and distinct. - #[serde(deserialize_with = "ordered_machines")] + #[serde(deserialize_with = "canonical::sorted_distinct")] pub machines: Vec, pub container: ContainerId, pub issued_at: Timestamp, @@ -124,42 +124,22 @@ impl RestartApproval { cluster: SealingPublicKey, facts: &RestartFacts<'_>, ) -> Result<(), AuthorityRefusal> { - let checks = [ - ("a Cluster", self.cluster == cluster), + first_mismatch([ + (field::CLUSTER, self.cluster == cluster), ( "a workflow", self.kind.permits(facts.step) && self.kind.reaches(facts.kind), ), - ("an Environment", &self.namespace == facts.namespace), + (field::ENVIRONMENT, &self.namespace == facts.namespace), ("a Service", &self.service == facts.service), - ("Machines", self.machines.contains(facts.machine)), + (field::MACHINES, self.machines.contains(facts.machine)), ("a Container", &self.container == facts.container), - ]; - match checks.iter().find(|(_, holds)| !holds) { - Some((field, _)) => Err(AuthorityRefusal::Mismatch { field }), - None => Ok(()), - } + ]) } } pub use super::validity::{CLOCK_SKEW_MILLIS, MAX_LIFETIME_MILLIS}; -pub(super) fn ordered_machines<'de, D: serde::Deserializer<'de>>( - deserializer: D, -) -> Result, D::Error> { - let machines = Vec::::deserialize(deserializer)?; - if machines - .iter() - .zip(machines.iter().skip(1)) - .any(|(left, right)| left >= right) - { - return Err(serde::de::Error::custom( - "Machines must be sorted and distinct", - )); - } - Ok(machines) -} - impl Statement for RestartApproval { const PURPOSE: Purpose = Purpose::Workflow; } diff --git a/core/crates/ployz-core/src/authority/rule.rs b/core/crates/ployz-core/src/authority/rule.rs index fe41e0e59e..ee776733a5 100644 --- a/core/crates/ployz-core/src/authority/rule.rs +++ b/core/crates/ployz-core/src/authority/rule.rs @@ -18,7 +18,7 @@ use super::{ canonical::{self, CanonicalError, Timestamp, hex_bytes, write_hex}, deploy::{Creation, PayloadDigest, content_addressed, drop_nulls}, keys::{Approver, Purpose, Signed, SigningPublicKey, Statement}, - refusal::AuthorityRefusal, + refusal::{AuthorityRefusal, field, first_mismatch}, restart::SignerKey, trusted_key::TrustedKeys, }; @@ -229,7 +229,7 @@ pub struct RuleSource { /// `.github/workflows/ployz-build.yml`. pub workflow: String, /// Sorted and distinct. - #[serde(deserialize_with = "sorted_distinct")] + #[serde(deserialize_with = "canonical::sorted_distinct")] pub events: Vec, pub builder: Builder, pub runner: Runner, @@ -365,7 +365,7 @@ pub struct AutoDeployRule { pub generation: u64, pub namespace: Namespace, /// Sorted and distinct. - #[serde(deserialize_with = "sorted_distinct")] + #[serde(deserialize_with = "canonical::sorted_distinct")] pub machines: Vec, pub source: RuleSource, /// Sorted by name and distinct. @@ -397,7 +397,7 @@ impl AutoDeployRule { pub fn verify_claims(&self, claims: &GithubClaims) -> Result<(), AuthorityRefusal> { let source = &self.source; let git_ref = format!("refs/heads/{}", source.branch); - let checks = [ + first_mismatch([ ( "repository_id", claims.repository_id == source.repository_id, @@ -432,11 +432,7 @@ impl AutoDeployRule { "runner_environment", claims.runner_environment == source.runner.as_str(), ), - ]; - match checks.into_iter().find(|(_, holds)| !holds) { - Some((field, _)) => Err(AuthorityRefusal::Mismatch { field }), - None => Ok(()), - } + ]) } } @@ -591,11 +587,11 @@ impl AutoDeployRule { .map(|service| PolicyDigest::of(&service.policy)); let checks = [ ( - "a Cluster", + field::CLUSTER, self.cluster == facts.cluster && claim.cluster == facts.cluster, ), ("a Machine", self.machines.contains(facts.machine)), - ("an Environment", &self.namespace == facts.namespace), + (field::ENVIRONMENT, &self.namespace == facts.namespace), ("an ended rule", !self.ended), ( "a Service", @@ -614,10 +610,7 @@ impl AutoDeployRule { match claim.authority { CommitAuthority::Head => {} } - match checks.into_iter().find(|(_, holds)| !holds) { - Some((field, _)) => Err(AuthorityRefusal::Mismatch { field }), - None => Ok(()), - } + first_mismatch(checks) } /// Check a new Container under this rule: everything [`Self::verify_step`] checks, @@ -721,30 +714,10 @@ fn decimal<'de, D: Deserializer<'de>>(deserializer: D) -> Result .map_err(|_| serde::de::Error::custom("a GitHub ID is out of range")) } -fn sorted_distinct<'de, D, T>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, - T: Deserialize<'de> + Ord, -{ - let items = Vec::::deserialize(deserializer)?; - if !items.is_sorted_by(|left, right| left < right) { - return Err(serde::de::Error::custom( - "a list must be sorted and distinct", - )); - } - Ok(items) -} - fn services_by_name<'de, D: Deserializer<'de>>( deserializer: D, ) -> Result, D::Error> { - let services = Vec::::deserialize(deserializer)?; - if !services.is_sorted_by(|left, right| left.name < right.name) { - return Err(serde::de::Error::custom( - "Services must be sorted by name and distinct", - )); - } - Ok(services) + canonical::sorted_by(deserializer, |service: &RuleService| &service.name) } #[cfg(test)] diff --git a/core/crates/ployz-core/src/authority/session.rs b/core/crates/ployz-core/src/authority/session.rs index d13ca5d9c7..cc76077289 100644 --- a/core/crates/ployz-core/src/authority/session.rs +++ b/core/crates/ployz-core/src/authority/session.rs @@ -16,6 +16,7 @@ use super::{ AuthorityRefusal, Purpose, SigningPublicKey, Statement, Timestamp, canonical::{self}, keys::signing_input, + refusal::{field, first_mismatch}, trusted_key::{PasskeyAssertion, TrustedKeys}, validity, }; @@ -55,10 +56,10 @@ pub struct GrantBody { #[ts(type = "string")] pub environment_id: Uuid, /// Sorted and distinct. - #[serde(deserialize_with = "sorted_distinct")] + #[serde(deserialize_with = "canonical::sorted_distinct")] pub machines: Vec, /// Sorted and distinct. - #[serde(deserialize_with = "sorted_distinct")] + #[serde(deserialize_with = "canonical::sorted_distinct")] pub families: Vec, pub issued_at: Timestamp, pub expires_at: Timestamp, @@ -69,22 +70,6 @@ impl Statement for GrantBody { const PURPOSE: Purpose = Purpose::Workflow; } -fn sorted_distinct<'de, D, T>(deserializer: D) -> Result, D::Error> -where - D: serde::Deserializer<'de>, - T: Deserialize<'de> + Ord, -{ - let items = Vec::::deserialize(deserializer)?; - if items - .iter() - .zip(items.iter().skip(1)) - .any(|(left, right)| left >= right) - { - return Err(serde::de::Error::custom("must be sorted and distinct")); - } - Ok(items) -} - impl GrantBody { /// The WebAuthn challenge a passkey answers to approve `body`: the SHA-256 of its /// signing input, so an answer made for anything else never reads as a grant. @@ -99,11 +84,11 @@ impl GrantBody { /// Returns [`AuthorityRefusal::Mismatch`] naming the first part of `scope` the grant /// does not cover. pub fn covers(&self, scope: &GrantScope<'_>) -> Result<(), AuthorityRefusal> { - let checks = [ - ("a Cluster", self.cluster == scope.cluster), - ("an Environment", &self.environment == scope.environment), + first_mismatch([ + (field::CLUSTER, self.cluster == scope.cluster), + (field::ENVIRONMENT, &self.environment == scope.environment), ( - "Machines", + field::MACHINES, scope .machines .iter() @@ -111,14 +96,10 @@ impl GrantBody { ), ("an action", self.families.contains(&scope.family)), ( - "a window", + field::WINDOW, scope.issued_at >= self.issued_at && scope.expires_at <= self.expires_at, ), - ]; - match checks.iter().find(|(_, holds)| !holds) { - Some((field, _)) => Err(AuthorityRefusal::Mismatch { field }), - None => Ok(()), - } + ]) } } diff --git a/core/crates/ployz-core/src/authority/volume.rs b/core/crates/ployz-core/src/authority/volume.rs index ba3ca9fa70..274a44f402 100644 --- a/core/crates/ployz-core/src/authority/volume.rs +++ b/core/crates/ployz-core/src/authority/volume.rs @@ -10,12 +10,12 @@ use serde::{Deserialize, Serialize}; use ts_rs::TS; use super::{ - canonical::Timestamp, + canonical::{self, Timestamp}, deploy::ApprovedService, keys::{Approver, Purpose, Signed, SigningPublicKey, Statement}, proposal::StepDigest, - refusal::AuthorityRefusal, - restart::{SignerKey, ordered_machines}, + refusal::{AuthorityRefusal, field, first_mismatch}, + restart::SignerKey, trusted_key::TrustedKeys, }; use crate::{ @@ -56,7 +56,7 @@ pub enum VolumeWorkflow { to: MachineId, service: ApprovedService, /// Sorted and distinct. - #[serde(deserialize_with = "ordered_machines")] + #[serde(deserialize_with = "canonical::sorted_distinct")] mirrors: Vec, }, /// `mirror` keeps a copy of the Writer on `writer`, and every later sync of it, until @@ -85,7 +85,7 @@ pub enum VolumeWorkflow { /// only when `shrink`. Resize { /// Sorted and distinct. - #[serde(deserialize_with = "ordered_machines")] + #[serde(deserialize_with = "canonical::sorted_distinct")] machines: Vec, maximum_bytes: ProvisionedVolumeMaximumBytes, shrink: bool, @@ -304,19 +304,16 @@ pub fn verify( field: "the signer", })?; let cluster = facts.cluster.ok_or(AuthorityRefusal::NoClusterIdentity)?; - let checks = [ - ("a Cluster", body.cluster == cluster), + first_mismatch([ + (field::CLUSTER, body.cluster == cluster), ("a Volume", &body.volume == facts.volume), - ("Machines", body.workflow.distinct()), + (field::MACHINES, body.workflow.distinct()), ("this Server", body.workflow.names(facts.machine)), ( "a Volume step", body.workflow.permits(facts.machine, facts.step), ), - ]; - if let Some((field, _)) = checks.iter().find(|(_, holds)| !holds) { - return Err(AuthorityRefusal::Mismatch { field }); - } + ])?; super::validity::verify(body.issued_at, body.expires_at, facts.now)?; Ok(VolumeApproved { body, From 68c51b8ff847c002ab974089e1c23f146d98e686 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:51:13 +0000 Subject: [PATCH 3/5] Pin the dependency gate's deadline by healthcheck kind Http and unconfigured Containers wait different deadlines in the gate than in a start's wait. The test holds today's verdicts before the gate's logic moves. --- .../ployz/src/deploy/exec_tests/health.rs | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/core/crates/ployz/src/deploy/exec_tests/health.rs b/core/crates/ployz/src/deploy/exec_tests/health.rs index 04bf81cc49..716da7da60 100644 --- a/core/crates/ployz/src/deploy/exec_tests/health.rs +++ b/core/crates/ployz/src/deploy/exec_tests/health.rs @@ -348,6 +348,86 @@ async fn dependency_gate_uses_short_unhealthy_and_healthcheck_starting_deadlines } } +#[tokio::test(start_paused = true)] +async fn dependency_gate_deadline_follows_the_healthcheck_kind() { + let dependency = QualifiedService::parse("app/api").unwrap(); + let http = |timeout_seconds| { + Some(ployz_core::HealthcheckSpec::Http( + ployz_core::HttpHealthcheck { + path: "/".into(), + port: std::num::NonZeroU16::new(80).unwrap(), + timeout_seconds, + }, + )) + }; + for (spec, runtime, seconds, expected) in [ + ( + http(3), + ContainerRuntimeObservation::Restarting, + 3, + "runtime", + ), + (http(3), unhealthy(), 3, "timed_out"), + (None, ContainerRuntimeObservation::Restarting, 5, "runtime"), + (None, running(), 185, "timed_out"), + ( + Some(ployz_core::HealthcheckSpec::Disabled), + running(), + 185, + "timed_out", + ), + ] { + let mut observed = routed(&machine('1'), &container('a'), runtime); + observed + .try_update(|parts| parts.resolved_spec.container.healthcheck = spec) + .unwrap(); + let plan = vec![DeployOperation::WaitHealthy { + machine_id: machine('1'), + dependent: QualifiedService::parse("app/web").unwrap(), + dependency: dependency.clone(), + }]; + let client = Scripted { + observations: Some(vec![observed]), + cancel_on_prepare: None, + ..Scripted::new(Vec::new()) + }; + + let started = tokio::time::Instant::now(); + let outcome = tokio::time::timeout( + std::time::Duration::from_secs(seconds + 1), + execute_with(&plan, &client, &CancellationToken::new()), + ) + .await + .expect("dependency health must finish by its deadline"); + assert_eq!(started.elapsed(), std::time::Duration::from_secs(seconds)); + + let DeployOutcome::Failed { + failed: + FailedOperation::Operation { + error: + ExecutionError::DependencyHealth { + failure: DependencyHealthFailure::Container { failure, .. }, + .. + }, + .. + }, + .. + } = outcome + else { + panic!("unexpected outcome: {outcome:?}"); + }; + assert_eq!( + match failure { + HealthFailure::TimedOut { .. } => "timed_out", + HealthFailure::Runtime { .. } => "runtime", + HealthFailure::Cancelled => "cancelled", + }, + expected + ); + client.assert_done(); + } +} + #[tokio::test] async fn a_failed_verdict_fails_the_start_naming_the_container_without_polling_it() { let machine = machine('1'); From 06bff03f24dfdd74a3a2f228071a6ab4de24dfca Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:51:13 +0000 Subject: [PATCH 4/5] Ask the health policy from the dependency gate The gate carried its own copy of the grace constant, the Http monitor case and the default-deadline fallback. health_wait now owns each once, and the gate's NotConfigured fallback is a named Unconfigured argument. --- .../ployz-core/src/domain/health_wait.rs | 121 ++++++++++++++---- core/crates/ployz/src/deploy/exec/health.rs | 39 ++---- 2 files changed, 105 insertions(+), 55 deletions(-) diff --git a/core/crates/ployz-core/src/domain/health_wait.rs b/core/crates/ployz-core/src/domain/health_wait.rs index 78f5c02cbc..6346cd0c83 100644 --- a/core/crates/ployz-core/src/domain/health_wait.rs +++ b/core/crates/ployz-core/src/domain/health_wait.rs @@ -114,25 +114,21 @@ impl<'spec> ReadyWait<'spec> { } else { &observed.runtime }; - let health_deadline = health_deadline_for(self.healthcheck, observed); - let deadline = health_deadline.unwrap_or(self.monitor); - let monitor_deadline = if matches!( - self.healthcheck.or(observed.effective_healthcheck.as_ref()), - Some(HealthcheckSpec::Http(_)) - ) { - deadline - } else { - self.monitor - }; + let health_deadline = + health_deadline_for(self.healthcheck, observed, Unconfigured::Settled); let poll = classify_health( runtime, details.last_check.as_ref(), elapsed, - monitor_deadline, + monitor_deadline( + self.healthcheck.or(observed.effective_healthcheck.as_ref()), + health_deadline, + self.monitor, + ), health_deadline, HealthExpectation::ContainerReady, ); - (poll, deadline) + (poll, health_deadline.unwrap_or(self.monitor)) } } @@ -234,27 +230,59 @@ pub fn observed_health(runtime: &ContainerRuntimeObservation) -> Option, observed: &ContainerObservation, + unconfigured: Unconfigured, ) -> Option { - match spec { - Some(spec) => spec.deadline(), - None => observed + let default_deadline = || { + let waits = matches!( + &observed.runtime, + ContainerRuntimeObservation::Running { + health: HealthObservation::Starting | HealthObservation::Unrecognized(_), + } + ) || (unconfigured == Unconfigured::UseDeadline + && matches!( + &observed.runtime, + ContainerRuntimeObservation::Running { + health: HealthObservation::NotConfigured, + } + )); + waits.then(|| healthcheck_deadline(None)) + }; + match (spec, unconfigured) { + (Some(spec), Unconfigured::Settled) => spec.deadline(), + (Some(spec), Unconfigured::UseDeadline) => spec.deadline().or_else(default_deadline), + (None, _) => observed .effective_healthcheck .as_ref() .and_then(HealthcheckSpec::deadline) - .or_else(|| { - matches!( - &observed.runtime, - ContainerRuntimeObservation::Running { - health: HealthObservation::Starting | HealthObservation::Unrecognized(_), - } - ) - .then(|| healthcheck_deadline(None)) - }), + .or_else(default_deadline), + } +} + +/// How long a Container that is not running, or not ready, is given before a wait fails +/// it. An Http healthcheck gives its own deadline; any other waits the `monitor` window. +#[must_use] +pub fn monitor_deadline( + healthcheck: Option<&HealthcheckSpec>, + health_deadline: Option, + monitor: Duration, +) -> Duration { + match healthcheck { + Some(HealthcheckSpec::Http(_)) => health_deadline.unwrap_or(monitor), + _ => monitor, } } @@ -607,6 +635,51 @@ mod tests { ); } + #[test] + fn a_container_with_no_check_settles_or_waits_the_default_deadline_as_the_caller_asks() { + let looked = details(running(HealthObservation::NotConfigured), None, None); + assert_eq!( + health_deadline_for(None, &looked.container, Unconfigured::Settled), + None + ); + assert_eq!( + health_deadline_for(None, &looked.container, Unconfigured::UseDeadline), + Some(healthcheck_deadline(None)) + ); + assert_eq!( + health_deadline_for( + Some(&HealthcheckSpec::Disabled), + &looked.container, + Unconfigured::UseDeadline + ), + Some(healthcheck_deadline(None)) + ); + let starting = details(running(HealthObservation::Starting), None, None); + for unconfigured in [Unconfigured::Settled, Unconfigured::UseDeadline] { + assert_eq!( + health_deadline_for(None, &starting.container, unconfigured), + Some(healthcheck_deadline(None)) + ); + } + } + + #[test] + fn only_an_http_check_gives_the_monitor_its_own_deadline() { + let http = http_check(3); + let configured = configured_check(60_000); + let deadline = Some(Duration::from_secs(3)); + assert_eq!( + monitor_deadline(Some(&http), deadline, MONITOR), + Duration::from_secs(3) + ); + assert_eq!(monitor_deadline(Some(&http), None, MONITOR), MONITOR); + assert_eq!( + monitor_deadline(Some(&configured), deadline, MONITOR), + MONITOR + ); + assert_eq!(monitor_deadline(None, deadline, MONITOR), MONITOR); + } + #[test] fn settle_deadline_is_the_longer_of_the_health_deadline_and_the_monitor() { assert_eq!( diff --git a/core/crates/ployz/src/deploy/exec/health.rs b/core/crates/ployz/src/deploy/exec/health.rs index f7c2ac9d71..c8c2021a4b 100644 --- a/core/crates/ployz/src/deploy/exec/health.rs +++ b/core/crates/ployz/src/deploy/exec/health.rs @@ -1,18 +1,14 @@ use ployz_core::{ - ContainerObservation, ContainerRuntimeObservation, CopyRotation, DependencyHealthFailure, - DeploymentLogId, ExecutionError, HEALTH_POLL_INTERVAL, HealthExpectation, HealthObservation, - HealthPoll, HealthcheckSpec, OperationPhase, QualifiedService, classify_health, - health_deadline_for, healthcheck_deadline, observed_health, + ContainerObservation, CopyRotation, DependencyHealthFailure, DeploymentLogId, ExecutionError, + HEALTH_POLL_INTERVAL, HEALTHCHECK_RUNTIME_GRACE, HealthExpectation, HealthPoll, OperationPhase, + QualifiedService, Unconfigured, classify_health, health_deadline_for, monitor_deadline, + observed_health, }; use tokio::time::Instant; use tokio_util::sync::CancellationToken; use super::{MachineOperations, Progress}; -/// A dependency reading unhealthy or restarting can recover, so the dependent's Deploy -/// waits this long before it fails. -const RUNTIME_FAILURE_GRACE: std::time::Duration = std::time::Duration::from_secs(5); - pub(super) async fn wait_healthy( client: &C, index: usize, @@ -71,33 +67,14 @@ pub(super) async fn wait_healthy( let elapsed = started.elapsed(); let mut pending = None; for observed in &containers { - let health_deadline = health_deadline_for( - observed.resolved_spec.container.healthcheck.as_ref(), - observed, - ) - .or_else(|| { - matches!( - &observed.runtime, - ContainerRuntimeObservation::Running { - health: HealthObservation::Starting - | HealthObservation::NotConfigured - | HealthObservation::Unrecognized(_), - } - ) - .then(|| healthcheck_deadline(None)) - }); + let healthcheck = observed.resolved_spec.container.healthcheck.as_ref(); + let health_deadline = + health_deadline_for(healthcheck, observed, Unconfigured::UseDeadline); let deadline = match classify_health( &observed.runtime, None, elapsed, - if matches!( - observed.resolved_spec.container.healthcheck, - Some(HealthcheckSpec::Http(_)) - ) { - health_deadline.unwrap_or(RUNTIME_FAILURE_GRACE) - } else { - RUNTIME_FAILURE_GRACE - }, + monitor_deadline(healthcheck, health_deadline, HEALTHCHECK_RUNTIME_GRACE), health_deadline, HealthExpectation::Healthy, ) { From 7fcd0fc7794a2b770512d212c21eece7fb63b1e2 Mon Sep 17 00:00:00 2001 From: Nick Potts Date: Tue, 10 Nov 2026 12:56:33 +0000 Subject: [PATCH 5/5] Add ployz-decide and the rule for what lives there A decision a model checks lives in one crate as a pure function that the model calls. clippy.toml refuses the clock and filesystem calls, so the purity is checked rather than hoped for. --- core/CODING_STANDARDS.md | 4 ++++ core/Cargo.lock | 9 +++++++++ core/Cargo.toml | 8 ++++++-- core/README.md | 1 + core/crates/ployz-decide/Cargo.toml | 15 +++++++++++++++ core/crates/ployz-decide/clippy.toml | 14 ++++++++++++++ core/crates/ployz-decide/src/lib.rs | 10 ++++++++++ core/crates/ployz-verify/Cargo.toml | 1 + core/crates/ployz-verify/src/lib.rs | 9 +++++++-- 9 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 core/crates/ployz-decide/Cargo.toml create mode 100644 core/crates/ployz-decide/clippy.toml create mode 100644 core/crates/ployz-decide/src/lib.rs diff --git a/core/CODING_STANDARDS.md b/core/CODING_STANDARDS.md index fbbe177894..05d2b75f63 100644 --- a/core/CODING_STANDARDS.md +++ b/core/CODING_STANDARDS.md @@ -66,6 +66,10 @@ Drop a `std::sync` lock before `.await`. Use `tokio::sync::Mutex` when the guard Generics (`T: Trait` / `impl Trait`) until a mixed collection or object-safe trait needs `dyn`. Prefer `&dyn Trait` when the callee does not need ownership. Own the `Box`/`Arc` at that boundary, not inside the module. +## Decisions + +A decision a model checks lives in `ployz-decide` as a pure function; the model calls it, never re-states it. Statement order across processes (the upgrade receipt and marker, the credential temp file and rename, pool ownership, removal marks, volume marks, Cloud's TypeScript walks) stays a model and links to the code by symbol path. + ## Macros Reuse the existing newtype and RPC macros for repeated mechanical shapes. A one-off stays a function or generic. diff --git a/core/Cargo.lock b/core/Cargo.lock index ba40713bf2..262cab8160 100644 --- a/core/Cargo.lock +++ b/core/Cargo.lock @@ -4125,6 +4125,14 @@ dependencies = [ "zeroize", ] +[[package]] +name = "ployz-decide" +version = "0.3.17" +dependencies = [ + "ployz-core", + "serde", +] + [[package]] name = "ployz-sdk" version = "0.3.17" @@ -4197,6 +4205,7 @@ name = "ployz-verify" version = "0.3.17" dependencies = [ "ployz-core", + "ployz-decide", "stateright", ] diff --git a/core/Cargo.toml b/core/Cargo.toml index 2d5fc18115..828cdaae27 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -1,6 +1,6 @@ [workspace] -members = ["crates/ployz-core", "crates/ployz-build", "crates/ployz", "crates/ployz-sdk", "crates/ployz-config-wasm", "crates/ployz-store", "crates/ployzd", "crates/ployz-testkit", "crates/ployz-verify"] -default-members = ["crates/ployz-core", "crates/ployz-build", "crates/ployz", "crates/ployz-sdk", "crates/ployz-store", "crates/ployzd"] +members = ["crates/ployz-core", "crates/ployz-decide", "crates/ployz-build", "crates/ployz", "crates/ployz-sdk", "crates/ployz-config-wasm", "crates/ployz-store", "crates/ployzd", "crates/ployz-testkit", "crates/ployz-verify"] +default-members = ["crates/ployz-core", "crates/ployz-decide", "crates/ployz-build", "crates/ployz", "crates/ployz-sdk", "crates/ployz-store", "crates/ployzd"] resolver = "3" [workspace.package] @@ -14,6 +14,7 @@ license = "Apache-2.0" inotify = "0.11.5" ployz-build = { path = "crates/ployz-build" } ployz-core = { path = "crates/ployz-core" } +ployz-decide = { path = "crates/ployz-decide" } ployz-store = { path = "crates/ployz-store" } ployz-testkit = { path = "crates/ployz-testkit" } ts-rs = { version = "12.0.1", features = ["serde-compat", "serde-json-impl", "no-serde-warnings"] } @@ -36,6 +37,9 @@ debug = "line-tables-only" [profile.dev.package.ployz-verify] opt-level = 3 +[profile.dev.package.ployz-decide] +opt-level = 3 + [profile.dev.package.stateright] opt-level = 3 diff --git a/core/README.md b/core/README.md index a32f3613ee..e3bc1331cd 100644 --- a/core/README.md +++ b/core/README.md @@ -45,6 +45,7 @@ there is no automatic reclamation, cross-store synchronization, or subnet repair Run Cargo and engine script commands from `core/`. - `crates/ployz-core`: domain and wire contracts shared by both binaries +- `crates/ployz-decide`: the decisions a model check calls, pure over plain facts - `crates/ployz`: CLI for Linux, macOS, and Windows through WSL - `crates/ployz-store`: the Config Store, all authored configuration and its history behind one read/write interface, served to Cloud through `@ployz/sdk` - `crates/ployz-build`: BuildKit execution for one captured Build diff --git a/core/crates/ployz-decide/Cargo.toml b/core/crates/ployz-decide/Cargo.toml new file mode 100644 index 0000000000..e819ac4849 --- /dev/null +++ b/core/crates/ployz-decide/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "ployz-decide" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +repository.workspace = true +publish = false +description = "Decision cores the model checks call, pure over plain facts" + +[dependencies] +ployz-core.workspace = true +serde = { version = "1.0.229", features = ["derive"] } + +[lints] +workspace = true diff --git a/core/crates/ployz-decide/clippy.toml b/core/crates/ployz-decide/clippy.toml new file mode 100644 index 0000000000..a274abc5b9 --- /dev/null +++ b/core/crates/ployz-decide/clippy.toml @@ -0,0 +1,14 @@ +disallowed-methods = [ + { path = "std::time::Instant::now", reason = "a decision takes its time as a Duration or SystemTime parameter, so a model can pass any clock" }, + { path = "std::time::SystemTime::now", reason = "a decision takes its time as a Duration or SystemTime parameter, so a model can pass any clock" }, + { path = "std::fs::read", reason = "a decision is pure over plain facts; the caller reads the file and passes what it holds" }, + { path = "std::fs::read_to_string", reason = "a decision is pure over plain facts; the caller reads the file and passes what it holds" }, + { path = "std::fs::read_dir", reason = "a decision is pure over plain facts; the caller lists the directory and passes what it holds" }, + { path = "std::fs::metadata", reason = "a decision is pure over plain facts; the caller asks the filesystem and passes the answer" }, + { path = "std::fs::write", reason = "a decision is pure over plain facts; the caller performs the write" }, + { path = "std::fs::rename", reason = "a decision is pure over plain facts; the caller performs the rename" }, + { path = "std::fs::remove_file", reason = "a decision is pure over plain facts; the caller performs the removal" }, + { path = "std::fs::create_dir_all", reason = "a decision is pure over plain facts; the caller creates the directory" }, + { path = "std::fs::File::open", reason = "a decision is pure over plain facts; the caller opens the file" }, + { path = "std::fs::File::create", reason = "a decision is pure over plain facts; the caller creates the file" }, +] diff --git a/core/crates/ployz-decide/src/lib.rs b/core/crates/ployz-decide/src/lib.rs new file mode 100644 index 0000000000..d38b5773c7 --- /dev/null +++ b/core/crates/ployz-decide/src/lib.rs @@ -0,0 +1,10 @@ +//! The decisions a model check calls instead of re-stating. +//! +//! A decision a model checks is a free function or a small enum machine over plain facts. +//! Time arrives as a `SystemTime` or `Duration` parameter. There is no `tokio`, `std::fs`, +//! `Instant::now`, Docker or SQLite type here, and `clippy.toml` refuses the clock and +//! filesystem calls. Callers map their I/O types to the facts, so the daemon, the CLI and +//! the Store call the same function `ployz-verify` explores. +//! +//! The pure cores `ployz-core` already holds (`issuance_gate`, `plan`, `shrink_floor`, +//! `RestartPolicy`) stay there. diff --git a/core/crates/ployz-verify/Cargo.toml b/core/crates/ployz-verify/Cargo.toml index add41d5547..c536162d4a 100644 --- a/core/crates/ployz-verify/Cargo.toml +++ b/core/crates/ployz-verify/Cargo.toml @@ -9,6 +9,7 @@ description = "Model checks of Ployz protocols that tests cannot reach" [dev-dependencies] ployz-core.workspace = true +ployz-decide.workspace = true stateright = "0.31.0" [lints] diff --git a/core/crates/ployz-verify/src/lib.rs b/core/crates/ployz-verify/src/lib.rs index 1ee6008b0c..e2348ca4d2 100644 --- a/core/crates/ployz-verify/src/lib.rs +++ b/core/crates/ployz-verify/src/lib.rs @@ -1,6 +1,11 @@ //! Model checks of Ployz protocols that tests rarely reach. Each module models one -//! protocol, calls the real code where it can, and its tests explore every -//! interleaving within the bounds the module states. +//! protocol and its tests explore every interleaving within the bounds the module states. +//! +//! A module calls the `ployz-core` code its protocol runs where that code is pure: +//! `issuance_gate`, `issuance_backoff` and `issuance_failure_clock` in `acme_issuance`, +//! `RestartPolicy` in `bridge_notes`, `MachineUpgradeAttempt` in `upgrade_attempt` and +//! `plan` in `volume_runs`. The other modules re-state their protocol's decisions by hand, +//! and a decision that moves into `ployz-decide` is called, never re-stated. #[cfg(test)] mod acme_issuance;