fix(awssm,scaleway): a JSON null field is no value, not the string "null"

CHANGELOG.md

@@ -44,6 +44,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- The `awssm` and `scaleway` providers now treat a JSON `null` in a `ref` field
+  as no value, the same as an absent key, so the provider chain continues.
+  Previously it was rendered as the four-character string `null`, which
+  satisfied a required secret and reached the program as a password or token
+  spelled `n-u-l-l`. The `bw` and `dashlane` providers already behaved this way.
+  An `extract` pointer is unchanged: it names one location and still reports a
+  `null` there, and the two policies now sit next to each other in one place.
 - The `awssm` provider now accepts a trailing slash in `?prefix=` without
   inserting a second slash into the AWS secret name. For example,
   `?prefix=myteam/` resolves to `myteam/secretspec/...`, matching

secretspec/src/json_field.rs

@@ -0,0 +1,80 @@
+//! Rendering one selected JSON value as a secret.
+//!
+//! Three call sites select a single value out of a JSON document: the `awssm`
+//! and `scaleway` providers, which take a flat `field` key, and
+//! `Secrets::extract_stored_value`, which takes a JSON Pointer. Selection
+//! differs on purpose and stays with each caller. Rendering the selected value
+//! is shared, and each caller states its own policy for a JSON null.
+
+use secrecy::SecretString;
+
+/// Renders a selected JSON value as a secret.
+///
+/// A string is taken as-is; anything else is serialized, so a port stays `5432`
+/// and a flag stays `true`. A null renders as `"null"`, which is what an
+/// `extract` pointer wants: it was asked for one specific location, and the
+/// document genuinely holds a JSON null there.
+pub(crate) fn render(value: &serde_json::Value) -> SecretString {
+    match value {
+        serde_json::Value::String(text) => SecretString::new(text.clone().into()),
+        other => SecretString::new(other.to_string().into()),
+    }
+}
+
+/// Renders a value selected by a provider's `field`, treating a null as absent.
+///
+/// A provider answers "is this secret set here?", so a null is no value and the
+/// resolver moves on to the next provider in the chain. Rendering it would
+/// produce the four-character secret `null`, which satisfies a required secret
+/// and reaches the program as a password or token spelled n-u-l-l. The `bw` and
+/// `dashlane` providers already treat a null this way.
+///
+/// This is deliberately not the same policy as [`render`]: an `extract` pointer
+/// names one location and reports what is there, while a provider `field` is a
+/// lookup that can come up empty.
+pub(crate) fn render_field(value: &serde_json::Value) -> Option<SecretString> {
+    match value {
+        serde_json::Value::Null => None,
+        other => Some(render(other)),
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use secrecy::ExposeSecret;
+
+    fn parse(raw: &str) -> serde_json::Value {
+        serde_json::from_str(raw).unwrap()
+    }
+
+    #[test]
+    fn render_takes_a_string_as_is_and_serializes_the_rest() {
+        assert_eq!(render(&parse(r#""s3cret""#)).expose_secret(), "s3cret");
+        assert_eq!(render(&parse("5432")).expose_secret(), "5432");
+        assert_eq!(render(&parse("true")).expose_secret(), "true");
+        assert_eq!(render(&parse(r#"{"a":1}"#)).expose_secret(), r#"{"a":1}"#);
+    }
+
+    #[test]
+    fn render_keeps_a_null_because_an_extract_pointer_reports_what_is_there() {
+        assert_eq!(render(&parse("null")).expose_secret(), "null");
+    }
+
+    #[test]
+    fn render_field_treats_a_null_as_absent() {
+        assert!(render_field(&parse("null")).is_none());
+    }
+
+    #[test]
+    fn render_field_agrees_with_render_on_everything_else() {
+        for raw in [r#""s3cret""#, "5432", "true", r#"{"a":1}"#, r#"["x"]"#] {
+            let value = parse(raw);
+            assert_eq!(
+                render_field(&value).unwrap().expose_secret(),
+                render(&value).expose_secret(),
+                "{raw}"
+            );
+        }
+    }
+}

secretspec/src/lib.rs

@@ -49,6 +49,7 @@ mod composition;
 mod config;
 mod error;
 pub(crate) mod generator;
+pub(crate) mod json_field;
 mod manifest;
 mod plan;
 mod report;

secretspec/src/provider/awssm.rs

@@ -262,12 +262,9 @@ impl AwssmProvider {
                 name, json_key, e
             ))
         })?;
-        match json.get(json_key) {
-            Some(serde_json::Value::String(s)) => Ok(Some(SecretString::new(s.clone().into()))),
-            // Non-string JSON values (numbers, bools) are rendered as-is.
-            Some(other) => Ok(Some(SecretString::new(other.to_string().into()))),
-            None => Ok(None),
-        }
+        // Selection is a flat key here; rendering the selected value is shared
+        // with the scaleway provider and Secrets::extract_stored_value.
+        Ok(json.get(json_key).and_then(crate::json_field::render_field))
     }
 
     /// Retrieves a secret by its full name/ARN, optionally extracting one key
@@ -797,6 +794,32 @@ mod tests {
         );
     }
 
+    #[test]
+    fn extract_json_key_null_is_none_not_the_string_null() {
+        // A null value is no value. Rendering it as "null" would satisfy a
+        // required secret and hand the program a password spelled n-u-l-l.
+        let value = r#"{"password": null}"#;
+        assert!(
+            AwssmProvider::extract_json_key("db", value, "password")
+                .unwrap()
+                .is_none()
+        );
+    }
+
+    #[test]
+    fn extract_json_key_null_matches_a_missing_key() {
+        let with_null = r#"{"password": null}"#;
+        let without = r#"{"username": "admin"}"#;
+        assert_eq!(
+            AwssmProvider::extract_json_key("db", with_null, "password")
+                .unwrap()
+                .is_none(),
+            AwssmProvider::extract_json_key("db", without, "password")
+                .unwrap()
+                .is_none()
+        );
+    }
+
     #[test]
     fn extract_json_key_missing_key_is_none() {
         let value = r#"{"username": "admin"}"#;

secretspec/src/provider/scaleway.rs

@@ -265,11 +265,8 @@ impl ScalewayProvider {
                 "secret '{name}' is not JSON, cannot extract key '{json_key}': {e}"
             ))
         })?;
-        match json.get(json_key) {
-            Some(serde_json::Value::String(s)) => Ok(Some(SecretString::new(s.clone().into()))),
-            Some(other) => Ok(Some(SecretString::new(other.to_string().into()))),
-            None => Ok(None),
-        }
+        // See the AWS provider: flat-key selection, shared rendering.
+        Ok(json.get(json_key).and_then(crate::json_field::render_field))
     }
 
     async fn get_async(
@@ -672,6 +669,17 @@ mod tests {
         assert!(!p.uri().contains("SCW_SECRET_KEY"));
     }
 
+    #[test]
+    fn extract_json_key_null_is_none_not_the_string_null() {
+        // Mirrors the AWS provider: a null value is no value.
+        let v = r#"{"user": "admin", "password": null}"#;
+        assert!(
+            ScalewayProvider::extract_json_key("s", v, "password")
+                .unwrap()
+                .is_none()
+        );
+    }
+
     #[test]
     fn extract_json_key_reads_string_and_renders_scalars() {
         let v = r#"{"user":"admin","port":5432,"tls":true}"#;

secretspec/src/secrets.rs

@@ -1777,13 +1777,12 @@ impl Secrets {
                         ),
                     }
                 })?;
-                let selected = match selected {
-                    serde_json::Value::String(value) => value.clone(),
-                    value => {
-                        serde_json::to_string(value).expect("serializing JSON value cannot fail")
-                    }
-                };
-                Ok(SecretString::new(selected.into()))
+                // Rendering is shared with the awssm and scaleway providers.
+                // A null renders as "null" here: this caller was asked for one
+                // pointer and reports what the document holds, unlike a
+                // provider `field`, where a null means "not set" and the chain
+                // continues. See crate::json_field.
+                Ok(crate::json_field::render(selected))
             }
         }
     }

secretspec/src/tests.rs

@@ -5338,6 +5338,34 @@ FALLBACK = {{ description = "logical default", providers = ["documents"], ref =
     assert_eq!(fs::read_to_string(document_path).unwrap(), original);
 }
 
+#[test]
+fn test_json_extract_renders_a_null_while_a_provider_field_treats_it_as_absent() {
+    use crate::config::{ExtractFormat, SecretExtract};
+
+    // An extract pointer names one location and reports what the document
+    // holds there, so a null renders. test_json_extract_resolves_structured_
+    // values_after_decoding pins this end to end.
+    let extract = SecretExtract {
+        format: ExtractFormat::Json,
+        pointer: "/database/password".to_string(),
+    };
+    let rendered =
+        Secrets::extract_stored_value(&extract, "PASSWORD", r#"{"database":{"password":null}}"#)
+            .unwrap();
+    assert_eq!(rendered.expose_secret(), "null");
+
+    // A provider `field` is a lookup that can come up empty, so the same null
+    // is absent and the provider chain continues.
+    let value: serde_json::Value = serde_json::from_str(r#"{"password":null}"#).unwrap();
+    assert!(crate::json_field::render_field(&value["password"]).is_none());
+
+    // Everything that is not null renders identically on both paths.
+    let port =
+        Secrets::extract_stored_value(&extract, "PASSWORD", r#"{"database":{"password":5432}}"#)
+            .unwrap();
+    assert_eq!(port.expose_secret(), "5432");
+}
+
 #[test]
 fn test_json_extract_errors_do_not_expose_stored_documents() {
     use crate::config::{ExtractFormat, SecretExtract};