diff --git a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java index e79c220c52..727f46d508 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java +++ b/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,8 +213,14 @@ 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"); @@ -221,6 +228,33 @@ public static void sanitizeMapToString(Map env, StringBuilder sb) { } } + /** + * 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. + *

+ * 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 sanitizeJsonTypes(K obj) { return Sanitizer.newInstance().apply(obj); diff --git a/core/src/main/java/org/apache/brooklyn/util/core/json/ShellEnvironmentSerializer.java b/core/src/main/java/org/apache/brooklyn/util/core/json/ShellEnvironmentSerializer.java index 7a2ce38f5d..7b6b4ad90a 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/json/ShellEnvironmentSerializer.java +++ b/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 resolver; @@ -43,7 +47,20 @@ public ShellEnvironmentSerializer(ManagementContext mgmt, Function)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 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("") || result.contains(" 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 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 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 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"; diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/ShellEnvironmentSerializerTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/ShellEnvironmentSerializerTest.java index 321d1e9f8f..69019dea87 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/ShellEnvironmentSerializerTest.java +++ b/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); }