From 983008cba3ef230b0e865a12b28a5cc1997c94f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= <mail@mikolajswiatek.com>
Date: Fri, 21 Feb 2025 17:08:28 +0100
Subject: [PATCH] Fix warning on emitting diagnostics for otel hybrid mode

If an Otel hybrid mode configuration doesn't exist, we emit a plain text
message into the config file generated by diagnostics. When we try to
redact secrets from it, we fail to unmarshal it into a map and log a
warning about it.

Instead, only run the redaction after confirming the value is actually
a map.
---
 internal/pkg/diagnostics/diagnostics.go      | 20 ++++++++++++--------
 internal/pkg/diagnostics/diagnostics_test.go | 13 +++++++++++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/internal/pkg/diagnostics/diagnostics.go b/internal/pkg/diagnostics/diagnostics.go
index f4b0d70ed90..0c568302a9f 100644
--- a/internal/pkg/diagnostics/diagnostics.go
+++ b/internal/pkg/diagnostics/diagnostics.go
@@ -327,19 +327,23 @@ func writeRedacted(errOut, resultWriter io.Writer, fullFilePath string, fileResu
 
 	// Should we support json too?
 	if fileResult.ContentType == "application/yaml" {
-		unmarshalled := map[string]interface{}{}
+		var unmarshalled any
 		err := yaml.Unmarshal(fileResult.Content, &unmarshalled)
 		if err != nil {
 			// Best effort, output a warning but still include the file
 			fmt.Fprintf(errOut, "[WARNING] Could not redact %s due to unmarshalling error: %s\n", fullFilePath, err)
 		} else {
-			unmarshalled = Redact(unmarshalled, errOut)
-			redacted, err := yaml.Marshal(unmarshalled)
-			if err != nil {
-				// Best effort, output a warning but still include the file
-				fmt.Fprintf(errOut, "[WARNING] Could not redact %s due to marshalling error: %s\n", fullFilePath, err)
-			} else {
-				out = &redacted
+			switch t := unmarshalled.(type) { // could be a plain string, we only redact if this is a proper map
+			case map[string]any:
+				t = Redact(t, errOut)
+				redacted, err := yaml.Marshal(t)
+				if err != nil {
+					// Best effort, output a warning but still include the file
+					fmt.Fprintf(errOut, "[WARNING] Could not redact %s due to marshalling error: %s\n", fullFilePath, err)
+				} else {
+					out = &redacted
+				}
+			default:
 			}
 		}
 	}
diff --git a/internal/pkg/diagnostics/diagnostics_test.go b/internal/pkg/diagnostics/diagnostics_test.go
index 87252cbf3e5..c00fa728090 100644
--- a/internal/pkg/diagnostics/diagnostics_test.go
+++ b/internal/pkg/diagnostics/diagnostics_test.go
@@ -117,6 +117,19 @@ i4EFZLWrFRsAAAARYWxleGtAZ3JlbWluLm5lc3QBAg==
 	require.NotContains(t, outWriter.String(), privKey)
 }
 
+func TestRedactPlainString(t *testing.T) {
+	errOut := strings.Builder{}
+	outWriter := strings.Builder{}
+	inputString := "Just a string"
+	res := client.DiagnosticFileResult{Content: []byte(inputString), ContentType: "application/yaml"}
+
+	err := writeRedacted(&errOut, &outWriter, "test/path", res)
+	require.NoError(t, err)
+
+	require.Empty(t, errOut.String())
+	require.Equal(t, outWriter.String(), inputString)
+}
+
 func TestRedactComplexKeys(t *testing.T) {
 	// taken directly from the yaml spec: https://yaml.org/spec/1.1/#c-mapping-key
 	// This test mostly serves to document that part of the YAML library doesn't work properly