Merge remote-tracking branch 'upstream/8.18' into mergify/bp/8.18/pr-9662

Author: fr4nc1sc0-r4m0n

.package-version

@@ -1,8 +1,8 @@
 {
-  "version": "8.18.6-SNAPSHOT",
-  "build_id": "8.18.6-a4a77cad",
-  "manifest_url": "https://snapshots.elastic.co/8.18.6-a4a77cad/manifest-8.18.6-SNAPSHOT.json",
-  "summary_url": "https://snapshots.elastic.co/8.18.6-a4a77cad/summary-8.18.6-SNAPSHOT.html",
-  "core_version": "8.18.6",
-  "stack_build_id": "8.18.6-a4a77cad-SNAPSHOT"
+  "version": "8.18.7-SNAPSHOT",
+  "build_id": "8.18.7-4f6bc48f",
+  "manifest_url": "https://snapshots.elastic.co/8.18.7-4f6bc48f/manifest-8.18.7-SNAPSHOT.json",
+  "summary_url": "https://snapshots.elastic.co/8.18.7-4f6bc48f/summary-8.18.7-SNAPSHOT.html",
+  "core_version": "8.18.7",
+  "stack_build_id": "8.18.7-4f6bc48f-SNAPSHOT"
 }

changelog/fragments/1756122869-update-secret-redaction-in-diagnostics.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: security
+
+# Change summary; a 80ish characters long description of the change.
+summary: redact secrets from pre-config, computed-config, components-expected, and components-actual files in diagnostics archive
+
+# 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/elastic/elastic-agent/pull/9560
+
+# 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/application/coordinator/coordinator.go

@@ -322,6 +322,9 @@ type Coordinator struct {
 	// run a ticker that checks to see if we have a new PID.
 	componentPIDTicker         *time.Ticker
 	componentPidRequiresUpdate *atomic.Bool
+
+	// Abstraction for diagnostics AddSecretMarkers function for testability
+	secretMarkerFunc func(*logger.Logger, *config.Config) error
 }
 
 // The channels Coordinator reads to receive updates from the various managers.
@@ -442,7 +445,8 @@ func New(
 		componentPIDTicker:         time.NewTicker(time.Second * 30),
 		componentPidRequiresUpdate: &atomic.Bool{},
 
-		fleetAcker: fleetAcker,
+		fleetAcker:       fleetAcker,
+		secretMarkerFunc: diagnostics.AddSecretMarkers,
 	}
 	// Setup communication channels for any non-nil components. This pattern
 	// lets us transparently accept nil managers / simulated events during
@@ -1296,6 +1300,10 @@ func (c *Coordinator) generateAST(cfg *config.Config) (err error) {
 		return err
 	}
 
+	if err = c.secretMarkerFunc(c.logger, cfg); err != nil {
+		c.logger.Errorf("failed to add secret markers: %v", err)
+	}
+
 	// perform and verify ast translation
 	m, err := cfg.ToMapStr()
 	if err != nil {

internal/pkg/agent/application/coordinator/coordinator_unit_test.go

@@ -37,10 +37,16 @@ import (
 	"github.com/elastic/elastic-agent/pkg/component"
 	"github.com/elastic/elastic-agent/pkg/component/runtime"
 	agentclient "github.com/elastic/elastic-agent/pkg/control/v2/client"
+	"github.com/elastic/elastic-agent/pkg/core/logger"
 	"github.com/elastic/elastic-agent/pkg/core/logger/loggertest"
 	"github.com/elastic/elastic-agent/pkg/utils/broadcaster"
 )
 
+var testSecretMarkerFunc = func(*logger.Logger, *config.Config) error {
+	// no-op secret marker function for testing
+	return nil
+}
+
 func TestVarsManagerError(t *testing.T) {
 	// Set a one-second timeout -- nothing here should block, but if it
 	// does let's report a failure instead of timing out the test runner.
@@ -470,6 +476,7 @@ func TestCoordinatorReportsInvalidPolicy(t *testing.T) {
 		vars:               emptyVars(t),
 		ast:                emptyAST(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   testSecretMarkerFunc,
 	}
 
 	// Send an invalid config update and confirm that Coordinator reports
@@ -585,6 +592,7 @@ func TestCoordinatorReportsComponentModelError(t *testing.T) {
 		vars:               emptyVars(t),
 		ast:                emptyAST(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   testSecretMarkerFunc,
 	}
 
 	// This configuration produces a valid AST but its EQL condition is
@@ -653,7 +661,7 @@ func TestCoordinatorPolicyChangeUpdatesMonitorReloader(t *testing.T) {
 	// does let's report a failure instead of timing out the test runner.
 	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
 	defer cancel()
-	logger := logp.NewLogger("testing")
+	log := logp.NewLogger("testing")
 
 	configChan := make(chan ConfigChange, 1)
 
@@ -668,10 +676,16 @@ func TestCoordinatorPolicyChangeUpdatesMonitorReloader(t *testing.T) {
 	newServerFn := func(*monitoringCfg.MonitoringConfig) (reload.ServerController, error) {
 		return monitoringServer, nil
 	}
-	monitoringReloader := reload.NewServerReloader(newServerFn, logger, monitoringCfg.DefaultConfig())
+	monitoringReloader := reload.NewServerReloader(newServerFn, log, monitoringCfg.DefaultConfig())
+
+	secretMarkerCalled := false
+	mockSecretMarkerFunc := func(*logger.Logger, *config.Config) error {
+		secretMarkerCalled = true
+		return nil
+	}
 
 	coord := &Coordinator{
-		logger:           logger,
+		logger:           log,
 		agentInfo:        &info.AgentInfo{},
 		stateBroadcaster: broadcaster.New(State{}, 0, 0),
 		managerChans: managerChans{
@@ -680,6 +694,7 @@ func TestCoordinatorPolicyChangeUpdatesMonitorReloader(t *testing.T) {
 		runtimeMgr:         runtimeManager,
 		vars:               emptyVars(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   mockSecretMarkerFunc,
 	}
 	coord.RegisterMonitoringServer(monitoringReloader)
 
@@ -700,6 +715,8 @@ inputs:
 	coord.runLoopIteration(ctx)
 	assert.True(t, cfgChange.acked, "Coordinator should ACK a successful policy change")
 
+	assert.True(t, secretMarkerCalled, "secret marker should be called")
+
 	// server is started by default
 	assert.True(t, monitoringServer.startTriggered)
 	assert.True(t, monitoringServer.isRunning)
@@ -819,6 +836,7 @@ func TestCoordinatorPolicyChangeUpdatesRuntimeAndOTelManager(t *testing.T) {
 		otelMgr:            otelManager,
 		vars:               emptyVars(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   testSecretMarkerFunc,
 	}
 
 	// Create a policy with one input and one output (no otel configuration)
@@ -950,6 +968,7 @@ func TestCoordinatorReportsRuntimeManagerUpdateFailure(t *testing.T) {
 		runtimeMgr:         runtimeManager,
 		vars:               emptyVars(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   testSecretMarkerFunc,
 	}
 
 	// Send an empty policy which should forward an empty component model to
@@ -1011,6 +1030,7 @@ func TestCoordinatorReportsOTelManagerUpdateFailure(t *testing.T) {
 		otelMgr:            otelManager,
 		vars:               emptyVars(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   testSecretMarkerFunc,
 	}
 
 	// Send an empty policy which should forward an empty component model to
@@ -1074,6 +1094,7 @@ func TestCoordinatorAppliesVarsToPolicy(t *testing.T) {
 		runtimeMgr:         runtimeManager,
 		vars:               emptyVars(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
+		secretMarkerFunc:   testSecretMarkerFunc,
 	}
 
 	// Create a policy with one input and one output

internal/pkg/agent/cmd/inspect.go

@@ -145,6 +145,11 @@ func inspectConfig(ctx context.Context, cfgPath string, opts inspectConfigOpts,
 		if err != nil {
 			return fmt.Errorf("error loading agent config: %w", err)
 		}
+		// Ensure secret markers are injected based on secret_paths before redaction.
+		if err := diagnostics.AddSecretMarkers(l, fullCfg); err != nil {
+			fmt.Fprintf(streams.Err, "failed to add secret markers: %v\n", err)
+		}
+
 		err = printConfig(fullCfg, streams)
 		if err != nil {
 			return fmt.Errorf("error printing config: %w", err)
@@ -228,6 +233,16 @@ func inspectConfig(ctx context.Context, cfgPath string, opts inspectConfigOpts,
 
 	}
 
+	// Ensure secret markers are injected based on secret_paths before redaction.
+	rawCfg := config.MustNewConfigFrom(cfg)
+	if err := diagnostics.AddSecretMarkers(l, rawCfg); err != nil {
+		fmt.Fprintf(streams.Err, "failed to add secret markers: %v\n", err)
+	}
+	cfg, err = rawCfg.ToMapStr()
+	if err != nil {
+		return fmt.Errorf("failed to convert config with secret markers: %w", err)
+	}
+
 	return printMapStringConfig(cfg, streams)
 }