From fe8269b2556541a41510e0647a485abb0ef7e9fa Mon Sep 17 00:00:00 2001 From: Durable Workflow Date: Fri, 23 Oct 2026 21:18:31 +0000 Subject: [PATCH] Refuse policy maps that are not JSON objects, like PHP #705 The six admission, durable-limit and external-payload policy maps are read like PHP's EnvAuditor::jsonObject: unset, or raw text that decodes to a JSON object. Malformed JSON, null, lists and scalars now stop startup instead of silently becoming an empty policy. A set DW_ name wins over its legacy name even when invalid, and the refusal names the variable without its value. The external payload quota reads its overrides with the same decoder. --- .../2026-10-23-policy-map-json.md | 33 +++++ rust/src/config_audit.rs | 131 ++++++++++++++++-- rust/src/runtime/external_config.rs | 14 +- 3 files changed, 163 insertions(+), 15 deletions(-) create mode 100644 docs/rust-server-port-checkpoints/2026-10-23-policy-map-json.md diff --git a/docs/rust-server-port-checkpoints/2026-10-23-policy-map-json.md b/docs/rust-server-port-checkpoints/2026-10-23-policy-map-json.md new file mode 100644 index 000000000..80db765d9 --- /dev/null +++ b/docs/rust-server-port-checkpoints/2026-10-23-policy-map-json.md @@ -0,0 +1,33 @@ +# October 23, policy maps must be JSON objects + +- **Found by.** PHP Server #705 (merged to main at `f67f2816e`, not yet in a + published image). Published PHP turned malformed admission-policy JSON + into an empty policy, so a configured cap could disappear without notice. + Native had the same defect. + - `DW_TASK_QUEUE_ADMISSION_OVERRIDES` and + `DW_NAMESPACE_ADMISSION_OVERRIDES` accepted malformed JSON. + - All five maps native refuses accepted `null`, `[]`, booleans and + numbers. + - The external-payload namespace overrides, which native honors, became + an empty policy when malformed. +- **Change.** The six maps PHP now reads with `EnvAuditor::jsonObject` are + read by the same rule in `config_audit::json_object`: + - the value is either unset or raw text that decodes to a JSON object; + - Laravel's `env()` conversions do not apply, so `null`, `(null)`, + `empty`, quoted text and an empty value are refused; + - a set `DW_*` name wins over its legacy name even when the `DW_*` value + is invalid; + - the startup refusal names the variable it read, never the value. + + Native still refuses a non-empty object in the five maps it has no policy + for. It still honors `DW_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES`, and the + external payload quota now reads that value with the same decoder. +- **Reference.** This follows unreleased PHP source, not the published + 2.11.1 image. For any valid configuration the two behave the same. The + only values refused are ones published PHP silently discarded and the + next PHP image refuses at boot. +- **Test.** `policy_maps_must_be_json_objects_like_php_705` covers all six + maps under both names, 17 invalid values, padded whitespace around `{}`, + DW-over-legacy precedence in both directions, and that the refusal never + repeats the value. The shipped Compose and Helm values (`{}` for the + durable maps) still start. diff --git a/rust/src/config_audit.rs b/rust/src/config_audit.rs index 3eb4572f0..82cf036c0 100644 --- a/rust/src/config_audit.rs +++ b/rust/src/config_audit.rs @@ -238,6 +238,44 @@ where ) } +/// The raw value PHP's `EnvAuditor::jsonObject` reads: the `DW_*` name +/// whenever it is set, even to an invalid value, else the legacy name. +/// Laravel's `env()` conversions do not apply. +fn selected<'a, F>(lookup: &F, name: &'a str, legacy: Option<&'a str>) -> Option<(String, &'a str)> +where + F: Fn(&str) -> Option, +{ + lookup(name) + .map(|raw| (raw, name)) + .or_else(|| legacy.and_then(|legacy| lookup(legacy).map(|raw| (raw, legacy)))) +} + +/// How a refusal names the variable it read. +fn read_from(name: &str, source: &str) -> String { + if source == name { name.to_owned() } else { format!("{name} (read from {source})") } +} + +/// `EnvAuditor::jsonObject` (Server #705): a policy map is unset, or raw text +/// that decodes to a JSON object. Malformed JSON, `null`, booleans, numbers, +/// strings and lists are refused instead of becoming an empty policy, so a +/// mistyped cap cannot silently disappear. The refusal never repeats the value. +pub fn json_object(lookup: F, name: &str, legacy: Option<&str>) -> Result, String> +where + F: Fn(&str) -> Option, +{ + let Some((raw, source)) = selected(&lookup, name, legacy) else { + return Ok(serde_json::Map::new()); + }; + match serde_json::from_str(&raw) { + Ok(serde_json::Value::Object(policy)) => Ok(policy), + _ => Err(format!("{} must contain a valid JSON object", read_from(name, source))), + } +} + +/// Policy maps native honors. Each must still be unset or a JSON object. +const HONORED_JSON_OBJECTS: &[(&str, &str)] = + &[("DW_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES", "WORKFLOW_SERVER_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES")]; + /// What a refused setting must be for native to start. #[derive(Clone, Copy, Debug)] pub enum Rule { @@ -256,6 +294,9 @@ pub enum Rule { /// `json_decode` gives nothing. Strict settings also refuse invalid JSON, /// which PHP either rejects or cannot have meant. EmptyJson { strict: bool }, + /// A policy map read by `EnvAuditor::jsonObject`: unset or a JSON object + /// (see [`json_object`]), and empty, since native has no such policy. + EmptyObject, /// The `(int)` cast equals PHP's default. Int(i64), /// `filter_var(..., FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? default` @@ -287,6 +328,9 @@ impl Rule { Ok(decoded) => !php_truthy(&decoded), Err(_) => !strict, }, + Self::EmptyObject => { + matches!(serde_json::from_str(text), Ok(serde_json::Value::Object(policy)) if policy.is_empty()) + } Self::Int(expected) => value.int() == expected, Self::FilterBool(default) => value.filter_bool().unwrap_or(default) == default, Self::CastBool(default) => value.cast_bool() == default, @@ -308,7 +352,7 @@ impl Rule { Self::AnyCase(value) => format!("unset or `{value}`"), Self::Empty => "unset or empty".into(), Self::NoPositiveInt => "unset, empty or not a positive number".into(), - Self::EmptyJson { .. } => "unset or `{}`".into(), + Self::EmptyJson { .. } | Self::EmptyObject => "unset or `{}`".into(), Self::Int(value) => format!("unset or `{value}`"), Self::FilterBool(value) | Self::CastBool(value) => format!("unset or {value}"), Self::DatabaseConnection => "unset, empty or the DB_CONNECTION name".into(), @@ -380,6 +424,17 @@ pub fn audit(environment: &BTreeMap) -> Report { let Disposition::Refused(rule, reason) = setting.disposition else { continue; }; + if let Rule::EmptyObject = rule { + match json_object(lookup, setting.name, setting.legacy) { + Err(message) => report.refusals.push(message), + Ok(policy) if !policy.is_empty() => { + let source = selected(&lookup, setting.name, setting.legacy).map_or(setting.name, |(_, source)| source); + report.refusals.push(format!("{} must be {}: {reason}", read_from(setting.name, source), rule.expectation())); + } + Ok(_) => {} + } + continue; + } let read = |name: &'static str| lookup(name).map(|raw| (PhpValue::from_env(&raw), name)); let primary = read(setting.name).filter(|(value, _)| *value != PhpValue::Null); let Some((value, source)) = primary.or_else(|| setting.legacy.and_then(read)) else { @@ -395,6 +450,11 @@ pub fn audit(environment: &BTreeMap) -> Report { if let Err(message) = activity_task_lease_seconds(lookup) { report.refusals.push(message.into()); } + for (name, legacy) in HONORED_JSON_OBJECTS { + if let Err(message) = json_object(lookup, name, Some(legacy)) { + report.refusals.push(message); + } + } let unknown: Vec<&str> = environment .keys() .map(String::as_str) @@ -458,7 +518,7 @@ const STORAGE_CONNECTION: &str = "native keeps workflow tables on its one databa const LEGACY_SCHEDULE_TIMEZONE: &str = "native does not convert pre-UTC schedule timestamps"; use Disposition::{Honored, Ignored, Refused}; -use Rule::{AnyCase, CastBool, DatabaseConnection, Empty, EmptyJson, Exact, FilterBool, Int, NoPositiveInt, NotAnyCase, NotIn}; +use Rule::{AnyCase, CastBool, DatabaseConnection, Empty, EmptyJson, EmptyObject, Exact, FilterBool, Int, NoPositiveInt, NotAnyCase, NotIn}; /// Settings PHP reads that its contract does not list. PHP's audit warns about /// them as unknown names; native also refuses values it cannot honor. @@ -530,10 +590,10 @@ pub const SETTINGS: &[Setting] = &[ Setting { name: "DW_METRICS_PROMETHEUS_WORKFLOW_SERIES_LIMIT", legacy: Some("WORKFLOW_SERVER_METRICS_PROMETHEUS_WORKFLOW_SERIES_LIMIT"), disposition: Refused(Int(100), METRICS) }, Setting { name: "DW_METRICS_WORKFLOW_TASK_FAILURE_TYPE_LIMIT", legacy: Some("WORKFLOW_SERVER_METRICS_WORKFLOW_TASK_FAILURE_TYPE_LIMIT"), disposition: Refused(Int(20), METRICS) }, Setting { name: "DW_MODE", legacy: Some("WORKFLOW_SERVER_MODE"), disposition: Refused(Exact(&["service"]), TOPOLOGY) }, - Setting { name: "DW_NAMESPACE_ADMISSION_OVERRIDES", legacy: Some("WORKFLOW_SERVER_NAMESPACE_ADMISSION_OVERRIDES"), disposition: Refused(EmptyJson { strict: false }, NAMESPACE_ADMISSION) }, - Setting { name: "DW_NAMESPACE_DURABLE_HARD_LIMITS", legacy: Some("WORKFLOW_SERVER_NAMESPACE_DURABLE_HARD_LIMITS"), disposition: Refused(EmptyJson { strict: true }, DURABLE_LIMITS) }, - Setting { name: "DW_NAMESPACE_DURABLE_LIMITS", legacy: Some("WORKFLOW_SERVER_NAMESPACE_DURABLE_LIMITS"), disposition: Refused(EmptyJson { strict: true }, DURABLE_LIMITS) }, - Setting { name: "DW_NAMESPACE_DURABLE_OVERRIDES", legacy: Some("WORKFLOW_SERVER_NAMESPACE_DURABLE_OVERRIDES"), disposition: Refused(EmptyJson { strict: true }, DURABLE_LIMITS) }, + Setting { name: "DW_NAMESPACE_ADMISSION_OVERRIDES", legacy: Some("WORKFLOW_SERVER_NAMESPACE_ADMISSION_OVERRIDES"), disposition: Refused(EmptyObject, NAMESPACE_ADMISSION) }, + Setting { name: "DW_NAMESPACE_DURABLE_HARD_LIMITS", legacy: Some("WORKFLOW_SERVER_NAMESPACE_DURABLE_HARD_LIMITS"), disposition: Refused(EmptyObject, DURABLE_LIMITS) }, + Setting { name: "DW_NAMESPACE_DURABLE_LIMITS", legacy: Some("WORKFLOW_SERVER_NAMESPACE_DURABLE_LIMITS"), disposition: Refused(EmptyObject, DURABLE_LIMITS) }, + Setting { name: "DW_NAMESPACE_DURABLE_OVERRIDES", legacy: Some("WORKFLOW_SERVER_NAMESPACE_DURABLE_OVERRIDES"), disposition: Refused(EmptyObject, DURABLE_LIMITS) }, Setting { name: "DW_NAMESPACE_HARD_MAX_CONCURRENT_REQUESTS", legacy: Some("WORKFLOW_SERVER_NAMESPACE_HARD_MAX_CONCURRENT_REQUESTS"), disposition: Refused(Empty, NAMESPACE_ADMISSION) }, Setting { name: "DW_NAMESPACE_HARD_MAX_REQUESTS_PER_MINUTE", legacy: Some("WORKFLOW_SERVER_NAMESPACE_HARD_MAX_REQUESTS_PER_MINUTE"), disposition: Refused(Empty, NAMESPACE_ADMISSION) }, Setting { name: "DW_NAMESPACE_MAX_CONCURRENT_REQUESTS", legacy: Some("WORKFLOW_SERVER_NAMESPACE_MAX_CONCURRENT_REQUESTS"), disposition: Refused(Empty, NAMESPACE_ADMISSION) }, @@ -573,7 +633,7 @@ pub const SETTINGS: &[Setting] = &[ Setting { name: "DW_STORAGE_ADMISSION_MAX_AGE_SECONDS", legacy: None, disposition: Ignored }, Setting { name: "DW_STORAGE_ADMISSION_SOURCE", legacy: None, disposition: Refused(Empty, STORAGE_ADMISSION) }, Setting { name: "DW_TASK_DISPATCH_MODE", legacy: Some("WORKFLOW_V2_TASK_DISPATCH_MODE"), disposition: Refused(Exact(&["poll"]), POLL_DISPATCH) }, - Setting { name: "DW_TASK_QUEUE_ADMISSION_OVERRIDES", legacy: Some("WORKFLOW_SERVER_TASK_QUEUE_ADMISSION_OVERRIDES"), disposition: Refused(EmptyJson { strict: false }, TASK_ADMISSION) }, + Setting { name: "DW_TASK_QUEUE_ADMISSION_OVERRIDES", legacy: Some("WORKFLOW_SERVER_TASK_QUEUE_ADMISSION_OVERRIDES"), disposition: Refused(EmptyObject, TASK_ADMISSION) }, Setting { name: "DW_TRUST_FORWARDED_ATTRIBUTION_HEADERS", legacy: Some("WORKFLOW_SERVER_TRUST_FORWARDED_ATTRIBUTION_HEADERS"), disposition: Refused(FilterBool(false), FORWARDED) }, Setting { name: "DW_UPDATE_VALIDATION_LEASE_TIMEOUT", legacy: Some("WORKFLOW_SERVER_UPDATE_VALIDATION_LEASE_TIMEOUT"), disposition: Ignored }, Setting { name: "DW_UPDATE_VALIDATION_TIMEOUT", legacy: Some("WORKFLOW_SERVER_UPDATE_VALIDATION_TIMEOUT"), disposition: Ignored }, @@ -875,8 +935,10 @@ mod tests { &[("DW_WORKFLOW_TASK_MAX_ACTIVE_LEASES_PER_QUEUE", "0")], &[("DW_WORKFLOW_TASK_MAX_ACTIVE_LEASES_PER_QUEUE", "true")], &[("DW_WORKFLOW_TASK_MAX_ACTIVE_LEASES_PER_QUEUE", "none")], - &[("DW_TASK_QUEUE_ADMISSION_OVERRIDES", "not json")], - &[("DW_TASK_QUEUE_ADMISSION_OVERRIDES", "[]")], + &[("DW_TASK_QUEUE_ADMISSION_OVERRIDES", "{}")], + &[("DW_TASK_QUEUE_ADMISSION_OVERRIDES", " \n{ } ")], + &[("WORKFLOW_SERVER_NAMESPACE_ADMISSION_OVERRIDES", "{}")], + &[("DW_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES", "{\"billing\":{\"max_bytes\":5}}")], &[("DW_V2_FLEET_VALIDATION_MODE", "warn")], &[("DW_V2_PIN_TO_RECORDED_FINGERPRINT", "no")], &[("DW_SERIALIZER", "")], @@ -926,6 +988,57 @@ mod tests { } } + #[test] + fn policy_maps_must_be_json_objects_like_php_705() { + // The six maps PHP reads with `EnvAuditor::jsonObject`. + let maps = [ + ("DW_TASK_QUEUE_ADMISSION_OVERRIDES", "WORKFLOW_SERVER_TASK_QUEUE_ADMISSION_OVERRIDES"), + ("DW_NAMESPACE_ADMISSION_OVERRIDES", "WORKFLOW_SERVER_NAMESPACE_ADMISSION_OVERRIDES"), + ("DW_NAMESPACE_DURABLE_LIMITS", "WORKFLOW_SERVER_NAMESPACE_DURABLE_LIMITS"), + ("DW_NAMESPACE_DURABLE_HARD_LIMITS", "WORKFLOW_SERVER_NAMESPACE_DURABLE_HARD_LIMITS"), + ("DW_NAMESPACE_DURABLE_OVERRIDES", "WORKFLOW_SERVER_NAMESPACE_DURABLE_OVERRIDES"), + ("DW_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES", "WORKFLOW_SERVER_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES"), + ]; + let invalid = [ + "", " ", "not json", "{", "{\"cap\":1", "null", "(null)", "empty", "true", "false", "0", "1", "[]", + "[{\"cap\":1}]", "\"{}\"", "'{}'", "\"text\"", + ]; + for (name, legacy) in maps { + for value in ["{}", " \n\t{}\r\n"] { + assert_eq!(refusals(&[(name, value)]), Vec::::new(), "{name}={value:?}"); + assert_eq!(refusals(&[(legacy, value)]), Vec::::new(), "{legacy}={value:?}"); + } + for value in invalid { + assert_eq!(refusals(&[(name, value)]), [format!("{name} must contain a valid JSON object")], "{name}={value:?}"); + assert_eq!( + refusals(&[(legacy, value)]), + [format!("{name} (read from {legacy}) must contain a valid JSON object")], + "{legacy}={value:?}" + ); + // A set DW name wins even when it is invalid and the legacy + // value is valid. + assert_eq!(refusals(&[(name, value), (legacy, "{}")]).len(), 1, "{name}={value:?}"); + } + // A valid DW name hides an invalid legacy value. + assert_eq!(refusals(&[(name, "{}"), (legacy, "not json")]), Vec::::new(), "{name}"); + } + // Native honors the external payload overrides; it refuses the other + // five unless they are empty, as before. + let policy = "{\"billing\":{\"max_bytes\":5}}"; + for (name, legacy) in &maps[..5] { + assert_eq!(refusals(&[(name, policy)]).len(), 1, "{name}"); + assert!(refusals(&[(legacy, policy)])[0].starts_with(&format!("{name} (read from {legacy}) must be unset or `{{}}`"))); + } + assert_eq!(refusals(&[(maps[5].0, policy)]), Vec::::new()); + assert_eq!( + json_object(|_| Some(policy.to_owned()), maps[5].0, Some(maps[5].1)).unwrap()["billing"]["max_bytes"], + 5 + ); + // The refusal never repeats the value. + let secret = "{\"secret-tenant-value\":"; + assert!(refusals(&[(maps[5].0, secret)]).iter().all(|refusal| !refusal.contains("secret-tenant-value"))); + } + #[test] fn refusals_never_repeat_the_value() { let secret = "principal-secret-value"; diff --git a/rust/src/runtime/external_config.rs b/rust/src/runtime/external_config.rs index e4992b199..070638fc7 100644 --- a/rust/src/runtime/external_config.rs +++ b/rust/src/runtime/external_config.rs @@ -122,11 +122,13 @@ fn member<'a>(value: &'a Value, key: &str) -> Option<&'a Value> { impl ExternalPayloadConfig { pub(super) fn from_env() -> Result { let completion = process_value("COMPLETION_MAX_BYTES", Value::Null)?; - let overrides = process_value("NAMESPACE_OVERRIDES", Value::String("{}".into()))?; - let overrides = overrides - .as_str() - .and_then(|value| serde_json::from_str(value).ok()) - .unwrap_or(Value::Null); + // Startup refuses a value that is not a JSON object (Server #705). + let overrides = crate::config_audit::json_object( + |name| env::var_os(name).map(|value| value.to_string_lossy().into_owned()), + "DW_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES", + Some("WORKFLOW_SERVER_EXTERNAL_PAYLOAD_NAMESPACE_OVERRIDES"), + ) + .map_err(|_| configuration_error())?; Ok(Self { completion_max_bytes: (!completion.is_null()) .then(|| integer(&completion).max(1) as u64), @@ -150,7 +152,7 @@ impl ExternalPayloadConfig { "HARD_MAX_OBJECTS_PER_NAMESPACE", Value::Null, )?, - namespace_overrides: overrides, + namespace_overrides: Value::Object(overrides), ..Self::default() }) }