Redact common secrets in elastic-agent inspect output (#6224) (#6240)

* Redact common  secrets in elastic-agent inspect output

* add changelog entry

* test: update tests that check `inspect` output

(cherry picked from commit f3aea40)

Co-authored-by: Andrzej Stencel <[email protected]>

changelog/fragments/1733397457-redact-common-secrets-in-inspect-output.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: enhancement
+
+# Change summary; a 80ish characters long description of the change.
+summary: Redact common secrets like API keys and passwords in the output from `inspect` command
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-agent
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+#pr: https://github.com/owner/repo/1234
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

internal/pkg/agent/cmd/inspect.go

@@ -236,7 +236,7 @@ func inspectConfig(ctx context.Context, cfgPath string, opts inspectConfigOpts,
 }
 
 func printMapStringConfig(mapStr map[string]interface{}, streams *cli.IOStreams) error {
-	data, err := yaml.Marshal(diagnostics.RedactSecretPaths(mapStr, streams.Err))
+	data, err := yaml.Marshal(diagnostics.Redact(mapStr, streams.Err))
 	if err != nil {
 		return errors.New(err, "could not marshal to YAML")
 	}

internal/pkg/diagnostics/diagnostics.go

@@ -333,8 +333,8 @@ func writeRedacted(errOut, resultWriter io.Writer, fullFilePath string, fileResu
 			// 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 = RedactSecretPaths(unmarshalled, errOut)
-			redacted, err := yaml.Marshal(redactMap(errOut, unmarshalled))
+			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)
@@ -575,6 +575,11 @@ func saveLogs(name string, logPath string, zw *zip.Writer) error {
 	return nil
 }
 
+// Redact redacts sensitive values from the passed mapStr.
+func Redact(mapStr map[string]any, errOut io.Writer) map[string]any {
+	return redactMap(errOut, RedactSecretPaths(mapStr, errOut))
+}
+
 // RedactSecretPaths will check the passed mapStr input for a secret_paths attribute.
 // If found it will replace the value for every key in the paths list with <REDACTED> and return the resulting map.
 // Any issues or errors will be written to the errOut writer.

testing/integration/endpoint_security_test.go

@@ -1063,8 +1063,8 @@ func TestInstallDefendWithMTLSandEncCertKey(t *testing.T) {
 				require.NoErrorf(t, err, "error running inspect cmd")
 
 				assert.Equal(t, proxyCLI.URL, got.Fleet.ProxyURL)
-				assert.Equal(t, mtlsCLI.clientCertPath, got.Fleet.Ssl.Certificate)
-				assert.Equal(t, mtlsCLI.clientCertKeyPath, got.Fleet.Ssl.Key)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Certificate)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Key)
 				assert.Empty(t, got.Fleet.Ssl.KeyPassphrasePath, "policy should have removed key_passphrase_path as key isn't passphrase protected anymore")
 			},
 		},
@@ -1083,9 +1083,9 @@ func TestInstallDefendWithMTLSandEncCertKey(t *testing.T) {
 				require.NoErrorf(t, err, "error running inspect cmd")
 
 				assert.Equal(t, proxyCLI.URL, got.Fleet.ProxyURL)
-				assert.Equal(t, mtlsCLI.clientCertPath, got.Fleet.Ssl.Certificate)
-				assert.Equal(t, mtlsCLI.clientCertKeyEncPath, got.Fleet.Ssl.Key)
-				assert.Equal(t, mtlsCLI.clientCertKeyPassPath, got.Fleet.Ssl.KeyPassphrasePath)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Certificate)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Key)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.KeyPassphrasePath)
 			},
 		},
 		{
@@ -1155,8 +1155,8 @@ func TestInstallDefendWithMTLSandEncCertKey(t *testing.T) {
 					return
 				}
 				assert.Equal(t, proxyPolicymTLS.URL, got.Fleet.ProxyURL)
-				assert.Equal(t, mtlsPolicy.clientCertPath, got.Fleet.Ssl.Certificate)
-				assert.Equal(t, mtlsPolicy.clientCertKeyPath, got.Fleet.Ssl.Key)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Certificate)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Key)
 				assert.Empty(t, got.Fleet.Ssl.KeyPassphrasePath, "policy should have removed key_passphrase_path as key isn't passphrase protected anymore")
 			},
 		},
@@ -1193,8 +1193,8 @@ func TestInstallDefendWithMTLSandEncCertKey(t *testing.T) {
 				}
 
 				assert.Equal(t, proxyPolicymTLS.URL, got.Fleet.ProxyURL)
-				assert.Equal(t, mtlsPolicy.clientCertPath, got.Fleet.Ssl.Certificate)
-				assert.Equal(t, mtlsPolicy.clientCertKeyPath, got.Fleet.Ssl.Key)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Certificate)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Key)
 				assert.Empty(t, got.Fleet.Ssl.KeyPassphrasePath, "policy should have removed key_passphrase_path as key isn't passphrase protected anymore")
 			},
 		},
@@ -1225,8 +1225,8 @@ func TestInstallDefendWithMTLSandEncCertKey(t *testing.T) {
 				}
 
 				assert.Equal(t, proxyPolicymTLS.URL, got.Fleet.ProxyURL)
-				assert.Equal(t, mtlsPolicy.clientCertPath, got.Fleet.Ssl.Certificate)
-				assert.Equal(t, mtlsPolicy.clientCertKeyPath, got.Fleet.Ssl.Key)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Certificate)
+				assert.Equal(t, "<REDACTED>", got.Fleet.Ssl.Key)
 				assert.Empty(t, got.Fleet.Ssl.KeyPassphrasePath, "key_passphrase_path was never set")
 			},
 		},

testing/integration/inspect_test.go

@@ -78,6 +78,12 @@ func TestInspect(t *testing.T) {
 	require.NoErrorf(t, err, "Error when running inspect, output: %s", p)
 	// Unmarshal into minimal object just to check if a secret has been redacted.
 	var yObj struct {
+		Agent struct {
+			Protection struct {
+				SigningKey         string `yaml:"signing_key"`
+				UninstallTokenHash string `yaml:"uninstall_token_hash"`
+			} `yaml:"protection"`
+		} `yaml:"agent"`
 		SecretPaths []string `yaml:"secret_paths"`
 		Inputs      []struct {
 			CustomAttr string `yaml:"custom_attr"`
@@ -88,4 +94,6 @@ func TestInspect(t *testing.T) {
 	assert.ElementsMatch(t, []string{"inputs.0.custom_attr"}, yObj.SecretPaths)
 	require.Len(t, yObj.Inputs, 1)
 	assert.Equalf(t, "<REDACTED>", yObj.Inputs[0].CustomAttr, "inspect output: %s", p)
+	assert.Equalf(t, "<REDACTED>", yObj.Agent.Protection.SigningKey, "`signing_key` is not redacted but it should be, because it contains `key`. inspect output: %s", p)
+	assert.Equalf(t, "<REDACTED>", yObj.Agent.Protection.UninstallTokenHash, "`uninstall_token_hash` is not redacted but it should be, because it contains `token`. inspect output: %s", p)
 }