suppressing of secrets in complex JSON serializable types

core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java

@@ -31,6 +31,7 @@
 import java.util.stream.Collectors;
 
 import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
@@ -212,15 +213,48 @@ public static void sanitizeMapToString(Map<?, ?> env, StringBuilder sb) {
             for (Map.Entry<?, ?> kv : env.entrySet()) {
                 String stringValue = kv.getValue() != null ? kv.getValue().toString() : "";
                 if (!stringValue.isEmpty()) {
-                    stringValue = Sanitizer.suppressIfSecret(kv.getKey(), stringValue);
-                    stringValue = sanitizeMultilineString(stringValue);
+                    if (Sanitizer.IS_SECRET_PREDICATE.apply(kv.getKey())) {
+                        // key name is a secret token: suppress the entire value
+                        stringValue = suppress(stringValue);
+                    } else {
+                        // key is not a secret name, but the value might be JSON with nested secret fields
+                        stringValue = suppressNestedSecretsInJsonString(stringValue);
+                        stringValue = sanitizeMultilineString(stringValue);
+                    }
                     stringValue = BashStringEscapes.wrapBash(stringValue);
                 }
                 sb.append(kv.getKey()).append("=").append(stringValue).append("\n");
             }
         }
     }
 
+    /**
+     * If the string is a JSON object or array, parses it and suppresses any nested secret fields
+     * (fields whose key names match {@link #IS_SECRET_PREDICATE}).
+     * If it is not a JSON object/array (or is malformed JSON), returns the original string unchanged.
+     * <p>
+     * This prevents nested passwords from leaking when a complex value is serialized to a JSON string
+     * and stored as an environment variable whose top-level key name does not itself contain a secret token.
+     */
+    static String suppressNestedSecretsInJsonString(String stringValue) {
+        if (stringValue == null || stringValue.isEmpty()) return stringValue;
+        char first = stringValue.charAt(0);
+        if (first != '{' && first != '[') {
+            // fast path: not a JSON object/array, skip parsing
+            return stringValue;
+        }
+        try {
+            Object parsed = new Gson().fromJson(stringValue, Object.class);
+            if (parsed instanceof Map || parsed instanceof Iterable) {
+                Object suppressed = suppressNestedSecretsJson(parsed, false);
+                return new GsonBuilder().disableHtmlEscaping().create().toJson(suppressed);
+            }
+        } catch (Exception e) {
+            // not valid JSON or unexpected structure; return original
+        }
+        return stringValue;
+    }
+
     /** applies to strings, sets, lists, maps */
     public static <K> K sanitizeJsonTypes(K obj) {
         return Sanitizer.newInstance().apply(obj);

core/src/main/java/org/apache/brooklyn/util/core/json/ShellEnvironmentSerializer.java

@@ -17,6 +17,7 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
 import com.google.common.collect.Maps;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
@@ -25,11 +26,14 @@
 import org.apache.commons.lang3.StringUtils;
 
 import javax.annotation.Nullable;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.function.Function;
 
 public class ShellEnvironmentSerializer {
+    private static final YAMLMapper YAML_MAPPER = new YAMLMapper();
+
     private ObjectMapper mapper;
     private final Function<Object, Object> resolver;
 
@@ -43,7 +47,20 @@ public ShellEnvironmentSerializer(ManagementContext mgmt, Function<Object,Object
 
     public String serialize(Object value) {
         if (value == null) return null;
-        if (value instanceof String) return (String)value;
+        if (value instanceof String) {
+            String str = (String) value;
+            // If the string is a YAML/JSON-serialized complex type (List or Map), normalize to JSON.
+            // This handles effector parameters submitted as YAML strings from the UI.
+            try {
+                Object parsed = YAML_MAPPER.readValue(str, Object.class);
+                if (parsed instanceof Map || parsed instanceof List) {
+                    return mapper.writeValueAsString(parsed);
+                }
+            } catch (Exception e) {
+                // not parseable as a YAML/JSON complex type; return as-is
+            }
+            return str;
+        }
         try {
             if (value instanceof DeferredSupplier) {
                 if (resolver!=null) {

core/src/test/java/org/apache/brooklyn/core/config/SanitizerTest.java

@@ -27,6 +27,8 @@
 import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.Strings;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 import org.testng.annotations.Test;
 
 public class SanitizerTest {
@@ -70,6 +72,87 @@ public void testSanitizeWithNull() throws Exception {
         assertEquals(Sanitizer.newInstance().apply((Map<?,?>)null), null);
     }
 
+    @Test
+    public void testSanitizeMapToString_nestedPasswordInJsonArray() {
+        // MY_DATA key doesn't contain a secret token, but its JSON value contains a "password" field
+        Map<String, Object> env = ImmutableMap.of(
+            "MY_DATA", "[{\"name\":\"element1\",\"password\":\"s3cr3t\",\"a_list\":[\"x\"]}]"
+        );
+        StringBuilder sb = new StringBuilder();
+        Sanitizer.sanitizeMapToString(env, sb);
+        String result = sb.toString();
+
+        assertFalse(result.contains("s3cr3t"), "Nested password should be suppressed, got: " + result);
+        assertTrue(result.contains("element1"), "Non-secret field should remain visible, got: " + result);
+        assertTrue(result.contains("suppressed"), "Should show suppressed marker, got: " + result);
+        assertFalse(result.contains("\\u003c"), "Suppressed marker should not be HTML-escaped, got: " + result);
+        assertTrue(result.contains("<suppressed>") || result.contains("<suppressed "), "Suppressed marker should use literal angle brackets, got: " + result);
+    }
+
+    @Test
+    public void testSanitizeMapToString_nestedPasswordInJsonObject() {
+        Map<String, Object> env = ImmutableMap.of(
+            "MY_DATA", "{\"name\":\"element1\",\"password\":\"s3cr3t\"}"
+        );
+        StringBuilder sb = new StringBuilder();
+        Sanitizer.sanitizeMapToString(env, sb);
+        String result = sb.toString();
+
+        assertFalse(result.contains("s3cr3t"), "Nested password in JSON object should be suppressed, got: " + result);
+        assertTrue(result.contains("element1"), "Non-secret field should remain visible, got: " + result);
+    }
+
+    @Test
+    public void testSanitizeMapToString_secretKeyStillSuppressesEntireValue() {
+        // When the key itself is a secret, the entire value is suppressed (existing behaviour)
+        Map<String, Object> env = ImmutableMap.of(
+            "SECRET_FIELD", "[{\"name\":\"toto\",\"password\":\"s3cr3t\"}]"
+        );
+        StringBuilder sb = new StringBuilder();
+        Sanitizer.sanitizeMapToString(env, sb);
+        String result = sb.toString();
+
+        assertFalse(result.contains("s3cr3t"), "Password should be suppressed, got: " + result);
+        assertFalse(result.contains("toto"), "Entire value should be suppressed for secret key, got: " + result);
+    }
+
+    @Test
+    public void testSanitizeMapToString_plainStringUnchanged() {
+        Map<String, Object> env = ImmutableMap.of("MY_VAR", "hello world");
+        StringBuilder sb = new StringBuilder();
+        Sanitizer.sanitizeMapToString(env, sb);
+        assertTrue(sb.toString().contains("hello world"), "Plain string should be unchanged");
+    }
+
+    @Test
+    public void testSanitizeMapToString_nonJsonStringStartingWithBrace() {
+        // A string that starts with { but is not valid JSON should be returned unchanged
+        Map<String, Object> env = ImmutableMap.of("MY_VAR", "{not valid json");
+        StringBuilder sb = new StringBuilder();
+        Sanitizer.sanitizeMapToString(env, sb);
+        assertTrue(sb.toString().contains("{not valid json"), "Invalid JSON should pass through unchanged");
+    }
+
+    @Test
+    public void testSuppressNestedSecretsInJsonString_jsonArray() {
+        String input = "[{\"name\":\"e1\",\"password\":\"secret123\"}]";
+        String result = Sanitizer.suppressNestedSecretsInJsonString(input);
+        assertFalse(result.contains("secret123"), "Password should be suppressed");
+        assertTrue(result.contains("e1"), "Non-secret field should remain");
+    }
+
+    @Test
+    public void testSuppressNestedSecretsInJsonString_nonJsonPassthrough() {
+        String input = "just a plain string";
+        assertEquals(Sanitizer.suppressNestedSecretsInJsonString(input), input);
+    }
+
+    @Test
+    public void testSuppressNestedSecretsInJsonString_malformedJsonPassthrough() {
+        String input = "[malformed";
+        assertEquals(Sanitizer.suppressNestedSecretsInJsonString(input), input);
+    }
+
     @Test
     public void testSanitizeMultiline() throws Exception {
         String hashPassword2 = "6CB75F652A9B52798EB6CF2201057C73";

software/base/src/test/java/org/apache/brooklyn/entity/software/base/ShellEnvironmentSerializerTest.java

@@ -16,6 +16,8 @@
 package org.apache.brooklyn.entity.software.base;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 
 import java.util.Date;
 
@@ -70,6 +72,24 @@ public void testSerialize() {
         assertSerialize(getClass(), getClass().getName());
     }
 
+    @Test
+    public void testSerializeYamlStringNormalizedToJson() {
+        // YAML list → JSON array
+        assertEquals(ser.serialize("- item1\n- item2\n"), "[\"item1\",\"item2\"]");
+
+        // YAML list of maps → JSON array of objects
+        String yamlList = "- name: element1\n  password: s3cr3t\n";
+        String jsonResult = ser.serialize(yamlList);
+        assertTrue(jsonResult.startsWith("["), "Expected JSON array, got: " + jsonResult);
+        assertFalse(jsonResult.contains("- name:"), "Should not contain YAML list syntax: " + jsonResult);
+
+        // Plain string → unchanged
+        assertEquals(ser.serialize("plain string value"), "plain string value");
+
+        // JSON string → unchanged (already JSON, re-serialization produces same output)
+        assertEquals(ser.serialize("[\"a\",\"b\"]"), "[\"a\",\"b\"]");
+    }
+
     private void assertSerialize(Object actual, String expected) {
         assertEquals(ser.serialize(actual), expected);
     }