Rust #325: refuse policy maps that are not JSON objects, like PHP #705

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.

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<String>,
+{
+    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<F>(lookup: F, name: &str, legacy: Option<&str>) -> Result<serde_json::Map<String, serde_json::Value>, String>
+where
+    F: Fn(&str) -> Option<String>,
+{
+    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<String, String>) -> 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<String, String>) -> 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::<String>::new(), "{name}={value:?}");
+                assert_eq!(refusals(&[(legacy, value)]), Vec::<String>::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::<String>::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::<String>::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";

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<Self> {
         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()
         })
     }