fix Echo to respect secure types

SteveL-MSFT · SteveL-MSFT · commit 6da122af55d7 · 2025-09-21T22:58:43.000-07:00
diff --git a/dsc/examples/secure_parameters.dsc.yaml b/dsc/examples/secure_parameters.dsc.yaml
@@ -4,12 +4,17 @@ parameters:
     type: secureString
   myObject:
     type: secureObject
+  showSecrets:
+    type: bool
+    defaultValue: false
 resources:
   - name: Echo 1
     type: Microsoft.DSC.Debug/Echo
     properties:
       output: "[parameters('myString')]"
+      showSecrets: "[parameters('showSecrets')]"
   - name: Echo 2
     type: Microsoft.DSC.Debug/Echo
     properties:
       output: "[parameters('myObject').myProperty]"
+      showSecrets: "[parameters('showSecrets')]"
diff --git a/dsc/examples/secure_parameters_shown.parameters.yaml b/dsc/examples/secure_parameters_shown.parameters.yaml
@@ -0,0 +1,5 @@
+parameters:
+  myString: mySecret
+  myObject:
+    myProperty: mySecretProperty
+  showSecrets: true
diff --git a/dsc_lib/src/configure/mod.rs b/dsc_lib/src/configure/mod.rs
@@ -3,7 +3,6 @@
 
 use crate::configure::config_doc::{ExecutionKind, Metadata, Resource};
 use crate::configure::context::{Context, ProcessMode};
-use crate::configure::parameters::is_secure_value;
 use crate::configure::{config_doc::RestartRequired, parameters::Input};
 use crate::discovery::discovery_trait::DiscoveryFilter;
 use crate::dscerror::DscError;
@@ -215,19 +214,7 @@ fn add_metadata(dsc_resource: &DscResource, mut properties: Option<Map<String, V
 
     match properties {
         Some(properties) => {
-            let mut unsecure_properties = Map::new();
-            for (key, value) in properties {
-                if is_secure_value(&value) {
-                    if value.get("secureString").is_some() {
-                        unsecure_properties.insert(key.clone(), "<secureString>".into());
-                    } else if value.get("secureObject").is_some() {
-                        unsecure_properties.insert(key.clone(), "<secureObject>".into());
-                    }
-                } else {
-                    unsecure_properties.insert(key, value);
-                }
-            }
-            Ok(serde_json::to_string(&unsecure_properties)?)
+            Ok(serde_json::to_string(&properties)?)
         },
         _ => {
             Ok(String::new())
diff --git a/dsc_lib/src/configure/parameters.rs b/dsc_lib/src/configure/parameters.rs
@@ -19,7 +19,7 @@ pub struct SecureString {
 
 impl Display for SecureString {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "<secureString>")
+        write!(f, "<secureValue>")
     }
 }
 
@@ -31,7 +31,7 @@ pub struct SecureObject {
 
 impl Display for SecureObject {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "<secureObject>")
+        write!(f, "<secureValue>")
     }
 }
 
diff --git a/dsc_lib/src/dscresources/command_resource.rs b/dsc_lib/src/dscresources/command_resource.rs
@@ -9,7 +9,7 @@ use serde_json::{Map, Value};
 use std::{collections::HashMap, env, process::Stdio};
 use crate::configure::{config_doc::ExecutionKind, config_result::{ResourceGetResult, ResourceTestResult}};
 use crate::dscerror::DscError;
-use super::{dscresource::get_diff, invoke_result::{ExportResult, GetResult, ResolveResult, SetResult, TestResult, ValidateResult, ResourceGetResponse, ResourceSetResponse, ResourceTestResponse, get_in_desired_state}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}};
+use super::{dscresource::{get_diff, redact}, invoke_result::{ExportResult, GetResult, ResolveResult, SetResult, TestResult, ValidateResult, ResourceGetResponse, ResourceSetResponse, ResourceTestResponse, get_in_desired_state}, resource_manifest::{ArgKind, InputKind, Kind, ResourceManifest, ReturnKind, SchemaKind}};
 use tracing::{error, warn, info, debug, trace};
 use tokio::{io::{AsyncBufReadExt, AsyncWriteExt, BufReader}, process::Command};
 
@@ -286,7 +286,7 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re
         return Ok(TestResult::Group(group_test_response));
     }
 
-    let expected_value: Value = serde_json::from_str(expected)?;
+    let mut expected_value: Value = serde_json::from_str(expected)?;
     match test.returns {
         Some(ReturnKind::State) => {
             let actual_value: Value = match serde_json::from_str(&stdout){
@@ -297,6 +297,7 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re
             };
             let in_desired_state = get_desired_state(&actual_value)?;
             let diff_properties = get_diff(&expected_value, &actual_value);
+            expected_value = redact(&expected_value);
             Ok(TestResult::Resource(ResourceTestResponse {
                 desired_state: expected_value,
                 actual_state: actual_value,
@@ -315,6 +316,7 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re
                 return Err(DscError::Command(resource.resource_type.clone(), exit_code, t!("dscresources.commandResource.testNoDiff").to_string()));
             };
             let diff_properties: Vec<String> = serde_json::from_str(diff_properties)?;
+            expected_value = redact(&expected_value);
             let in_desired_state = get_desired_state(&actual_value)?;
             Ok(TestResult::Resource(ResourceTestResponse {
                 desired_state: expected_value,
@@ -339,6 +341,7 @@ pub fn invoke_test(resource: &ResourceManifest, cwd: &str, expected: &str) -> Re
                 }
             };
             let diff_properties = get_diff( &expected_value, &actual_state);
+            expected_value = redact(&expected_value);
             Ok(TestResult::Resource(ResourceTestResponse {
                 desired_state: expected_value,
                 actual_state,
diff --git a/dsc_lib/src/dscresources/dscresource.rs b/dsc_lib/src/dscresources/dscresource.rs
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use crate::{configure::{config_doc::{Configuration, ExecutionKind, Resource}, Configurator, context::ProcessMode}, dscresources::resource_manifest::Kind};
+use crate::{configure::{Configurator, config_doc::{Configuration, ExecutionKind, Resource}, context::ProcessMode, parameters::is_secure_value}, dscresources::resource_manifest::Kind};
 use crate::dscresources::invoke_result::{ResourceGetResponse, ResourceSetResponse};
 use dscerror::DscError;
 use jsonschema::Validator;
@@ -311,7 +311,7 @@ impl Invoke for DscResource {
             let TestResult::Resource(ref resource_result) = result.results[0].result else {
                 return Err(DscError::Operation(t!("dscresources.dscresource.invokeReturnedWrongResult", operation = "test", resource = self.type_name).to_string()));
             };
-            let desired_state = resource_result.desired_state
+            let mut desired_state = resource_result.desired_state
                 .as_object().ok_or(DscError::Operation(t!("dscresources.dscresource.propertyIncorrectType", property = "desiredState", property_type = "object").to_string()))?
                 .get("resources").ok_or(DscError::Operation(t!("dscresources.dscresource.propertyNotFound", property = "resources").to_string()))?
                 .as_array().ok_or(DscError::Operation(t!("dscresources.dscresource.propertyIncorrectType", property = "resources", property_type = "array").to_string()))?[0]
@@ -324,6 +324,7 @@ impl Invoke for DscResource {
                 .as_object().ok_or(DscError::Operation(t!("dscresources.dscresource.propertyIncorrectType", property = "result", property_type = "object").to_string()))?
                 .get("properties").ok_or(DscError::Operation(t!("dscresources.dscresource.propertyNotFound", property = "properties").to_string()))?.clone();
             let diff_properties = get_diff(&desired_state, &actual_state);
+            desired_state = redact(&desired_state);
             let test_result = TestResult::Resource(ResourceTestResponse {
                 desired_state,
                 actual_state,
@@ -346,7 +347,7 @@ impl Invoke for DscResource {
                 let resource_manifest = import_manifest(manifest.clone())?;
                 if resource_manifest.test.is_none() {
                     let get_result = self.get(expected)?;
-                    let desired_state = serde_json::from_str(expected)?;
+                    let mut desired_state = serde_json::from_str(expected)?;
                     let actual_state = match get_result {
                         GetResult::Group(results) => {
                             let mut result_array: Vec<Value> = Vec::new();
@@ -360,6 +361,7 @@ impl Invoke for DscResource {
                         }
                     };
                     let diff_properties = get_diff( &desired_state, &actual_state);
+                    desired_state = redact(&desired_state);
                     let test_result = TestResult::Resource(ResourceTestResponse {
                         desired_state,
                         actual_state,
@@ -491,6 +493,37 @@ pub fn get_well_known_properties() -> HashMap<String, Value> {
     ])
 }
 
+/// Checks if the JSON value is sensitive and should be redacted
+///
+/// # Arguments
+///
+/// * `value` - The JSON value to check
+///
+/// # Returns
+///
+/// Original value if not sensitive, otherwise a redacted value
+pub fn redact(value: &Value) -> Value {
+    trace!("Redacting value: {value}");
+    if is_secure_value(value) {
+        return Value::String("<secureValue>".to_string());
+    }
+
+    if let Some(map) = value.as_object() {
+        let mut new_map = Map::new();
+        for (key, val) in map {
+            new_map.insert(key.clone(), redact(val));
+        }
+        return Value::Object(new_map);
+    }
+
+    if let Some(array) = value.as_array() {
+        let new_array: Vec<Value> = array.iter().map(|val| redact(val)).collect();
+        return Value::Array(new_array);
+    }
+
+    value.clone()
+}
+
 #[must_use]
 /// Performs a comparison of two JSON Values if the expected is a strict subset of the actual
 ///
@@ -524,6 +557,11 @@ pub fn get_diff(expected: &Value, actual: &Value) -> Vec<String> {
         }
 
         for (key, value) in &*map {
+            if is_secure_value(&value) {
+                // skip secure values as they are not comparable
+                continue;
+            }
+
             if value.is_object() {
                 let sub_diff = get_diff(value, &actual[key]);
                 if !sub_diff.is_empty() {
diff --git a/dsc_lib/src/parser/expressions.rs b/dsc_lib/src/parser/expressions.rs
@@ -146,13 +146,21 @@ impl Expression {
                             if !object.contains_key(member) {
                                 return Err(DscError::Parser(t!("parser.expression.memberNameNotFound", member = member).to_string()));
                             }
-                            value = convert_to_secure(&object[member]);
+                            if is_secure {
+                                value = convert_to_secure(&object[member]);
+                            } else {
+                                value = object[member].clone();
+                            }
                         } else {
                             return Err(DscError::Parser(t!("parser.expression.accessOnNonObject").to_string()));
                         }
                     },
                     Accessor::Index(index_value) => {
-                        index = convert_to_secure(index_value);
+                        if is_secure {
+                            index = convert_to_secure(index_value);
+                        } else {
+                            index = index_value.clone();
+                        }
                     },
                     Accessor::IndexExpression(expression) => {
                         index = expression.invoke(function_dispatcher, context)?;
@@ -169,7 +177,11 @@ impl Expression {
                         if index >= array.len() {
                             return Err(DscError::Parser(t!("parser.expression.indexOutOfBounds").to_string()));
                         }
-                        value = convert_to_secure(&array[index]);
+                        if is_secure {
+                            value = convert_to_secure(&array[index]);
+                        } else {
+                            value = array[index].clone();
+                        }
                     } else {
                         return Err(DscError::Parser(t!("parser.expression.indexOnNonArray").to_string()));
                     }
@@ -185,10 +197,6 @@ impl Expression {
 }
 
 fn convert_to_secure(value: &Value) -> Value {
-    if !is_secure_value(value) {
-        return value.clone();
-    }
-
     if let Some(string) = value.as_str() {
         let secure_string = crate::configure::parameters::SecureString {
             secure_string: string.to_string(),
diff --git a/dscecho/src/echo.rs b/dscecho/src/echo.rs
@@ -14,30 +14,35 @@ pub enum Output {
     Bool(bool),
     #[serde(rename = "number")]
     Number(i64),
-    #[serde(rename = "object")]
-    Object(Value),
     #[serde(rename = "secureObject")]
     SecureObject(SecureObject),
     #[serde(rename = "secureString")]
     SecureString(SecureString),
     #[serde(rename = "string")]
     String(String),
+    // Object has to be last so it doesn't get matched first
+    #[serde(rename = "object")]
+    Object(Value),
 }
 
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
 #[serde(deny_unknown_fields)]
 pub struct SecureString {
+    #[serde(rename = "secureString")]
     pub secure_string: String,
 }
 
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
 #[serde(deny_unknown_fields)]
 pub struct SecureObject {
+    #[serde(rename = "secureObject")]
     pub secure_object: Value,
 }
 
 #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
 #[serde(deny_unknown_fields)]
 pub struct Echo {
     pub output: Output,
+    #[serde(rename = "showSecrets", skip_serializing_if = "Option::is_none")]
+    pub show_secrets: Option<bool>,
 }
diff --git a/dscecho/src/main.rs b/dscecho/src/main.rs
@@ -23,14 +23,13 @@ fn main() {
                     std::process::exit(1);
                 }
             };
-            match echo.output {
-                Output::SecureObject(_) => {
-                    echo.output = Output::String("<secureObject>".to_string());
-                },
-                Output::SecureString(_) => {
-                    echo.output = Output::String("<secureString>".to_string());
-                },
-                _ => {}
+            if echo.show_secrets != Some(true) {
+                match &echo.output {
+                    Output::SecureString(_) | Output::SecureObject(_) => {
+                        echo.output = Output::String("<secureValue>".to_string());
+                    },
+                    _ => {}
+                }
             }
             let json = serde_json::to_string(&echo).unwrap();
             println!("{json}");

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ pub struct SecureString {`
`19`	`19`
`20`	`20`	`impl Display for SecureString {`
`21`	`21`	`fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {`
`22`		`- write!(f, "<secureString>")`
	`22`	`+ write!(f, "<secureValue>")`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`
`@@ -31,7 +31,7 @@ pub struct SecureObject {`
`31`	`31`
`32`	`32`	`impl Display for SecureObject {`
`33`	`33`	`fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {`
`34`		`- write!(f, "<secureObject>")`
	`34`	`+ write!(f, "<secureValue>")`
`35`	`35`	`}`
`36`	`36`	`}`
`37`	`37`