Diagnostics file writes use RedactSecretPaths

changelog/fragments/1728425752-Diagnostics-files-will-redact-secret_paths.yaml

@@ -0,0 +1,34 @@
+# 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: Diagnostics files will redact secret_paths
+
+# 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: |
+  The elastic-agent will use redact secret paths in files written in diagnostics bundles.
+  Secret paths are expected to be specified as a top-level attribute in yaml data being written.
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component:
+
+# 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/elastic/elastic-agent/pull/5745
+
+# 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/diagnostics/diagnostics.go

@@ -327,12 +327,13 @@ func writeRedacted(errOut, resultWriter io.Writer, fullFilePath string, fileResu
 
 	// Should we support json too?
 	if fileResult.ContentType == "application/yaml" {
-		unmarshalled := map[interface{}]interface{}{}
+		unmarshalled := map[string]interface{}{}
 		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 = RedactSecretPaths(unmarshalled, errOut)
 			redacted, err := yaml.Marshal(redactMap(errOut, unmarshalled))
 			if err != nil {
 				// Best effort, output a warning but still include the file
@@ -579,7 +580,6 @@ func saveLogs(name string, logPath string, zw *zip.Writer) error {
 func RedactSecretPaths(mapStr map[string]any, errOut io.Writer) map[string]any {
 	v, ok := mapStr["secret_paths"]
 	if !ok {
-		fmt.Fprintln(errOut, "No output redaction: secret_paths attribute not found.")
 		return mapStr
 	}
 	arr, ok := v.([]interface{})

internal/pkg/diagnostics/diagnostics_test.go

@@ -140,6 +140,103 @@ mapping:
 	require.Empty(t, errOut.String())
 }
 
+func TestRedactWithSecretPaths(t *testing.T) {
+	tests := []struct {
+		name   string
+		input  []byte
+		expect string
+	}{{
+		name: "no secret paths",
+		input: []byte(`id: test-policy
+inputs:
+  - type: test_input
+    redactKey: secretValue
+outputs:
+  default:
+    type: elasticsearch
+    api_key: secretKey
+    redactOtherKey: secretOutputValue
+`),
+		expect: `id: test-policy
+inputs:
+    - redactKey: secretValue
+      type: test_input
+outputs:
+    default:
+        api_key: <REDACTED>
+        redactOtherKey: secretOutputValue
+        type: elasticsearch
+`,
+	}, {
+		name: "secret_paths are redacted",
+		input: []byte(`id: test-policy
+secret_paths:
+  - inputs.0.redactKey
+  - outputs.default.redactOtherKey
+inputs:
+  - type: test_input
+    redactKey: secretValue
+outputs:
+  default:
+    type: elasticsearch
+    api_key: secretKey
+    redactOtherKey: secretOutputValue
+`),
+		expect: `id: test-policy
+inputs:
+    - redactKey: <REDACTED>
+      type: test_input
+outputs:
+    default:
+        api_key: <REDACTED>
+        redactOtherKey: <REDACTED>
+        type: elasticsearch
+secret_paths:
+    - inputs.0.redactKey
+    - outputs.default.redactOtherKey
+`,
+	}, {
+		name: "secret_paths contains extra keys",
+		input: []byte(`id: test-policy
+secret_paths:
+  - inputs.0.redactKey
+  - inputs.1.missingKey
+  - outputs.default.redactOtherKey
+inputs:
+  - type: test_input
+    redactKey: secretValue
+outputs:
+  default:
+    type: elasticsearch
+    api_key: secretKey
+`),
+		expect: `id: test-policy
+inputs:
+    - redactKey: <REDACTED>
+      type: test_input
+outputs:
+    default:
+        api_key: <REDACTED>
+        type: elasticsearch
+secret_paths:
+    - inputs.0.redactKey
+    - inputs.1.missingKey
+    - outputs.default.redactOtherKey
+`,
+	}}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			file := client.DiagnosticFileResult{Content: tc.input, ContentType: "application/yaml"}
+			var out bytes.Buffer
+			err := writeRedacted(io.Discard, &out, "testPath", file)
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.expect, out.String())
+		})
+	}
+}
+
 func TestUnitAndStateMapping(t *testing.T) {
 	// this structure causes problems due to the compound agentruntime.ComponentUnitKey map key
 	exampleState := agentruntime.ComponentState{

testing/fleetservertest/checkin.go

@@ -174,11 +174,11 @@ const (
             "hosts": {{ toJson .FleetHosts }}
           },
           "id": "{{.PolicyID}}",
-          "secret_paths": ["inputs.0.secret_key"],
+          "secret_paths": ["inputs.0.custom_attr"],
           "inputs": [
             {
               "id": "fake-input",
-              "secret_key": "secretValue",
+              "custom_attr": "secretValue",
               "revision": 1,
               "name": "fake-input",
               "type": "fake-input",

testing/fleetservertest/fleetserver_test.go

@@ -258,6 +258,7 @@ func ExampleNewServer_checkin_fleetConnectionParams() {
 	//   "id": "",
 	//   "inputs": [
 	//     {
+	//       "custom_attr": "secretValue",
 	//       "data_stream": {
 	//         "namespace": "default"
 	//       },
@@ -271,7 +272,6 @@ func ExampleNewServer_checkin_fleetConnectionParams() {
 	//       "name": "fake-input",
 	//       "package_policy_id": "",
 	//       "revision": 1,
-	//       "secret_key": "secretValue",
 	//       "streams": [],
 	//       "type": "fake-input",
 	//       "use_output": "default"
@@ -289,7 +289,7 @@ func ExampleNewServer_checkin_fleetConnectionParams() {
 	//   },
 	//   "revision": 2,
 	//   "secret_paths": [
-	//     "inputs.0.secret_key"
+	//     "inputs.0.custom_attr"
 	//   ],
 	//   "secret_references": [],
 	//   "signed": {

testing/integration/diagnostics_test.go

@@ -20,11 +20,14 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
 
 	"github.com/elastic/elastic-agent/pkg/control/v2/client"
 	integrationtest "github.com/elastic/elastic-agent/pkg/testing"
 	"github.com/elastic/elastic-agent/pkg/testing/define"
+	"github.com/elastic/elastic-agent/pkg/testing/tools/check"
 	"github.com/elastic/elastic-agent/pkg/testing/tools/testcontext"
+	"github.com/elastic/elastic-agent/testing/fleetservertest"
 )
 
 const diagnosticsArchiveGlobPattern = "elastic-agent-diagnostics-*.zip"
@@ -200,6 +203,90 @@ func TestIsolatedUnitsDiagnosticsCommand(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestRedactFleetSecretPathsDiagnostics(t *testing.T) {
+	_ = define.Require(t, define.Requirements{
+		Group: Fleet,
+		Local: false,
+		Sudo:  true,
+	})
+
+	ctx, cancel := testcontext.WithTimeout(t, context.Background(), time.Minute*10)
+	defer cancel()
+
+	t.Log("Setup fake fleet-server")
+	apiKey, policy := createBasicFleetPolicyData(t, "http://fleet-server:8220")
+	checkinWithAcker := fleetservertest.NewCheckinActionsWithAcker()
+	fleet := fleetservertest.NewServerWithHandlers(
+		apiKey,
+		"enrollmentToken",
+		policy.AgentID,
+		policy.PolicyID,
+		checkinWithAcker.ActionsGenerator(),
+		checkinWithAcker.Acker(),
+		fleetservertest.WithRequestLog(t.Logf),
+	)
+	defer fleet.Close()
+	policyChangeAction, err := fleetservertest.NewActionPolicyChangeWithFakeComponent("test-policy-change", fleetservertest.TmplPolicy{
+		AgentID:    policy.AgentID,
+		PolicyID:   policy.PolicyID,
+		FleetHosts: []string{fleet.LocalhostURL},
+	})
+	require.NoError(t, err)
+	checkinWithAcker.AddCheckin("token", 0, policyChangeAction)
+
+	t.Log("Enroll agent in fake fleet-server")
+	fixture, err := define.NewFixtureFromLocalBuild(t,
+		define.Version(),
+		integrationtest.WithAllowErrors(),
+		integrationtest.WithLogOutput())
+	require.NoError(t, err, "SetupTest: NewFixtureFromLocalBuild failed")
+	err = fixture.EnsurePrepared(ctx)
+	require.NoError(t, err, "SetupTest: fixture.Prepare failed")
+
+	out, err := fixture.Install(
+		ctx,
+		&integrationtest.InstallOpts{
+			Force:          true,
+			NonInteractive: true,
+			Insecure:       true,
+			Privileged:     false,
+			EnrollOpts: integrationtest.EnrollOpts{
+				URL:             fleet.LocalhostURL,
+				EnrollmentToken: "anythingWillDO",
+			}})
+	require.NoErrorf(t, err, "Error when installing agent, output: %s", out)
+	check.ConnectedToFleet(ctx, t, fixture, 5*time.Minute)
+
+	t.Log("Gather diagnostics.")
+	diagZip, err := fixture.ExecDiagnostics(ctx)
+	require.NoError(t, err, "error when gathering diagnostics")
+	stat, err := os.Stat(diagZip)
+	require.NoErrorf(t, err, "stat file %q failed", diagZip)
+	require.Greaterf(t, stat.Size(), int64(0), "file %s has incorrect size", diagZip)
+
+	t.Log("Check if config files have been redacted.")
+	extractionDir := t.TempDir()
+	extractZipArchive(t, diagZip, extractionDir)
+	path := filepath.Join(extractionDir, "computed-config.yaml")
+	stat, err = os.Stat(path)
+	require.NoErrorf(t, err, "stat file %q failed", path)
+	require.Greaterf(t, stat.Size(), int64(0), "file %s has incorrect size", path)
+	f, err := os.Open(path)
+	require.NoErrorf(t, err, "open file %q failed", path)
+	defer f.Close()
+	var yObj struct {
+		SecretPaths []string `yaml:"secret_paths"`
+		Inputs      []struct {
+			CustomAttr string `yaml:"custom_attr"`
+		} `yaml:"inputs"`
+	}
+	err = yaml.NewDecoder(f).Decode(&yObj)
+	require.NoError(t, err)
+	assert.ElementsMatch(t, []string{"inputs.0.custom_attr"}, yObj.SecretPaths)
+	require.Len(t, yObj.Inputs, 1)
+	assert.Equal(t, "<REDACTED>", yObj.Inputs[0].CustomAttr)
+}
+
 func testDiagnosticsFactory(t *testing.T, compSetup map[string]integrationtest.ComponentState, diagFiles []string, diagCompFiles []string, fix *integrationtest.Fixture, cmd []string) func(ctx context.Context) error {
 	return func(ctx context.Context) error {
 		diagZip, err := fix.ExecDiagnostics(ctx, cmd...)

testing/integration/inspect_test.go

@@ -80,12 +80,12 @@ func TestInspect(t *testing.T) {
 	var yObj struct {
 		SecretPaths []string `yaml:"secret_paths"`
 		Inputs      []struct {
-			SecretKey string `yaml:"secret_key"`
+			CustomAttr string `yaml:"custom_attr"`
 		} `yaml:"inputs"`
 	}
 	err = yaml.Unmarshal(p, &yObj)
 	require.NoError(t, err)
-	assert.ElementsMatch(t, []string{"inputs.0.secret_key"}, yObj.SecretPaths)
+	assert.ElementsMatch(t, []string{"inputs.0.custom_attr"}, yObj.SecretPaths)
 	require.Len(t, yObj.Inputs, 1)
-	assert.Equalf(t, "<REDACTED>", yObj.Inputs[0].SecretKey, "inspect output: %s", p)
+	assert.Equalf(t, "<REDACTED>", yObj.Inputs[0].CustomAttr, "inspect output: %s", p)
 }