Add secret_paths attributes to policies sent to agents (elastic#3908)

Add a new attribute "secret_paths" to the policy data sent to agents.
This attribute is a list of keys where the fleet-server has repalced a
reference with a secret value. The agent is expected to redact the
values of these keys when outputting policy data.

Makefile

@@ -112,6 +112,7 @@ generate: ## - Generate schema models
 	env GOBIN=${GOBIN} go install github.com/deepmap/oapi-codegen/v2/cmd/[email protected]
 	@printf "${CMD_COLOR_ON} Running go generate\n${CMD_COLOR_OFF}"
 	env PATH="${GOBIN}:${PATH}" go generate ./...
+	@$(MAKE) check-headers
 
 .PHONY: check-ci
 check-ci: ## - Run all checks of the ci without linting, the linter is run through github action to have comments in the pull-request.

changelog/fragments/1726509909-Add-secret-paths-list-to-policies-sent-to-agents.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: feature
+
+# Change summary; a 80ish characters long description of the change.
+summary: Add secret paths list to policies sent to agents
+
+# 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: |
+  Add a secret_paths attribute as part of the policy response data. This attribute is a list of keys where secret substitution has occured.
+  The agent should redact the values of these keys when outputting them.
+
+# 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/fleet-server/pull/3908
+
+# 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:

internal/pkg/api/handleCheckin.go

@@ -16,6 +16,7 @@ import (
 	"math/rand"
 	"net/http"
 	"reflect"
+	"slices"
 	"sync"
 	"time"
 
@@ -818,11 +819,13 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a
 
 	data := model.ClonePolicyData(pp.Policy.Data)
 	for policyName, policyOutput := range data.Outputs {
-		err := policy.ProcessOutputSecret(ctx, policyOutput, bulker)
+		// NOTE: Not sure if output secret keys collected here include new entries, but they are collected for completeness
+		ks, err := policy.ProcessOutputSecret(ctx, policyOutput, bulker)
 		if err != nil {
 			return nil, fmt.Errorf("failed to process output secrets %q: %w",
 				policyName, err)
 		}
+		pp.SecretKeys = append(pp.SecretKeys, ks...)
 	}
 	// Iterate through the policy outputs and prepare them
 	for _, policyOutput := range pp.Outputs {
@@ -845,6 +848,10 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a
 	if err != nil {
 		return nil, err
 	}
+	// remove duplicates from secretkeys
+	slices.Sort(pp.SecretKeys)
+	keys := slices.Compact(pp.SecretKeys)
+	d.SecretPaths = &keys
 	ad := Action_Data{}
 	err = ad.FromActionPolicyChange(ActionPolicyChange{d})
 	if err != nil {

internal/pkg/policy/parsed_policy.go

@@ -10,10 +10,11 @@ import (
 	"errors"
 	"fmt"
 
+	"go.elastic.co/apm/v2"
+
 	"github.com/elastic/fleet-server/v7/internal/pkg/bulk"
 	"github.com/elastic/fleet-server/v7/internal/pkg/model"
 	"github.com/elastic/fleet-server/v7/internal/pkg/smap"
-	"go.elastic.co/apm/v2"
 )
 
 const (
@@ -44,16 +45,18 @@ type ParsedPolicyDefaults struct {
 }
 
 type ParsedPolicy struct {
-	Policy  model.Policy
-	Roles   RoleMapT
-	Outputs map[string]Output
-	Default ParsedPolicyDefaults
-	Inputs  []map[string]interface{}
-	Links   apm.SpanLink
+	Policy     model.Policy
+	Roles      RoleMapT
+	Outputs    map[string]Output
+	Default    ParsedPolicyDefaults
+	Inputs     []map[string]interface{}
+	SecretKeys []string
+	Links      apm.SpanLink
 }
 
 func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*ParsedPolicy, error) {
 	var err error
+	secretKeys := make([]string, 0)
 	// Interpret the output permissions if available
 	var roles map[string]RoleT
 	if roles, err = parsePerms(p.Data.OutputPermissions); err != nil {
@@ -64,20 +67,24 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa
 	if err != nil {
 		return nil, err
 	}
-	for _, policyOutput := range p.Data.Outputs {
-		err := ProcessOutputSecret(ctx, policyOutput, bulker)
+	for name, policyOutput := range p.Data.Outputs {
+		ks, err := ProcessOutputSecret(ctx, policyOutput, bulker)
 		if err != nil {
 			return nil, err
 		}
+		for _, key := range ks {
+			secretKeys = append(secretKeys, "outputs."+name+"."+key)
+		}
 	}
 	defaultName, err := findDefaultOutputName(p.Data.Outputs)
 	if err != nil {
 		return nil, err
 	}
-	policyInputs, err := getPolicyInputsWithSecrets(ctx, p.Data, bulker)
+	policyInputs, keys, err := getPolicyInputsWithSecrets(ctx, p.Data, bulker)
 	if err != nil {
 		return nil, err
 	}
+	secretKeys = append(secretKeys, keys...)
 
 	// We are cool and the gang
 	pp := &ParsedPolicy{
@@ -87,7 +94,8 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa
 		Default: ParsedPolicyDefaults{
 			Name: defaultName,
 		},
-		Inputs: policyInputs,
+		Inputs:     policyInputs,
+		SecretKeys: secretKeys,
 	}
 	if trace := apm.TransactionFromContext(ctx); trace != nil {
 		// Pass current transaction link (should be a monitor transaction) to caller (likely a client request).

internal/pkg/policy/secret.go

@@ -7,6 +7,7 @@ package policy
 import (
 	"context"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"github.com/elastic/fleet-server/v7/internal/pkg/bulk"
@@ -39,57 +40,99 @@ func getSecretValues(ctx context.Context, secretRefs []model.SecretReferencesIte
 
 // read inputs and secret_references from agent policy
 // replace values of secret refs in inputs and input streams properties
-func getPolicyInputsWithSecrets(ctx context.Context, data *model.PolicyData, bulker bulk.Bulk) ([]map[string]interface{}, error) {
+func getPolicyInputsWithSecrets(ctx context.Context, data *model.PolicyData, bulker bulk.Bulk) ([]map[string]interface{}, []string, error) {
 	if len(data.Inputs) == 0 {
-		return nil, nil
+		return nil, nil, nil
 	}
 
 	if len(data.SecretReferences) == 0 {
-		return data.Inputs, nil
+		return data.Inputs, nil, nil
 	}
 
 	secretValues, err := getSecretValues(ctx, data.SecretReferences, bulker)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	result := make([]map[string]interface{}, 0)
-	for _, input := range data.Inputs {
-		newInput := make(map[string]interface{})
-		for k, v := range input {
-			newInput[k] = replaceAnyRef(v, secretValues)
+	keys := make([]string, 0)
+	for i, input := range data.Inputs {
+		newInput, ks := replaceMapRef(input, secretValues)
+		for _, key := range ks {
+			keys = append(keys, "inputs."+strconv.Itoa(i)+"."+key)
 		}
 		result = append(result, newInput)
 	}
 	data.SecretReferences = nil
-	return result, nil
+	return result, keys, nil
 }
 
-// replaceAnyRef is a generic approach to replacing any secret references in the passed item.
-// It will go through any slices or maps and replace any secret references.
-//
-// go's generic parameters are not a good fit for rewriting this method as the typeswitch will not work.
-func replaceAnyRef(ref any, secrets map[string]string) any {
+// replaceMapRef replaces all nested secret values in the passed input and returns the resulting input along with a list of keys where inputs have been replaced.
+func replaceMapRef(input map[string]any, secrets map[string]string) (map[string]any, []string) {
+	keys := make([]string, 0)
+	result := make(map[string]any, len(input))
 	var r any
-	switch val := ref.(type) {
-	case string:
-		r = replaceStringRef(val, secrets)
-	case map[string]any:
-		obj := make(map[string]any)
-		for k, v := range val {
-			obj[k] = replaceAnyRef(v, secrets)
+
+	for k, v := range input {
+		switch value := v.(type) {
+		case string:
+			ref, replaced := replaceStringRef(value, secrets)
+			if replaced {
+				keys = append(keys, k)
+			}
+			r = ref
+		case map[string]any:
+			ref, ks := replaceMapRef(value, secrets)
+			for _, key := range ks {
+				keys = append(keys, k+"."+key)
+			}
+			r = ref
+		case []any:
+			ref, ks := replaceSliceRef(value, secrets)
+			for _, key := range ks {
+				keys = append(keys, k+"."+key)
+			}
+			r = ref
+		default:
+			r = v
 		}
-		r = obj
-	case []any:
-		arr := make([]any, len(val))
-		for i, v := range val {
-			arr[i] = replaceAnyRef(v, secrets)
+		result[k] = r
+	}
+	return result, keys
+}
+
+// replaceSliceRef replaces all nested secrets within the passed slice and returns the resulting slice along with a list of keys that indicate where values have been replaced.
+func replaceSliceRef(arr []any, secrets map[string]string) ([]any, []string) {
+	keys := make([]string, 0)
+	result := make([]any, len(arr))
+	var r any
+
+	for i, v := range arr {
+		switch value := v.(type) {
+		case string:
+			ref, replaced := replaceStringRef(value, secrets)
+			if replaced {
+				keys = append(keys, strconv.Itoa(i))
+			}
+			r = ref
+		case map[string]any:
+			ref, ks := replaceMapRef(value, secrets)
+			for _, key := range ks {
+				keys = append(keys, strconv.Itoa(i)+"."+key)
+			}
+			r = ref
+		case []any:
+			ref, ks := replaceSliceRef(value, secrets)
+			for _, key := range ks {
+				keys = append(keys, strconv.Itoa(i)+"."+key)
+			}
+			r = ref
+		default:
+			r = v
 		}
-		r = arr
-	default:
-		r = val
+		result[i] = r
 	}
-	return r
+	return result, keys
 }
 
 type OutputSecret struct {
@@ -145,14 +188,15 @@ func setSecretPath(output smap.Map, secretValue string, secretPaths []string) er
 }
 
 // Read secret from output and mutate output with secret value
-func ProcessOutputSecret(ctx context.Context, output smap.Map, bulker bulk.Bulk) error {
+func ProcessOutputSecret(ctx context.Context, output smap.Map, bulker bulk.Bulk) ([]string, error) {
 	secrets := output.GetMap(FieldOutputSecrets)
 
 	delete(output, FieldOutputSecrets)
 	secretReferences := make([]model.SecretReferencesItems, 0)
 	outputSecrets, err := getSecretIDAndPath(secrets)
+	keys := make([]string, 0, len(outputSecrets))
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	for _, secret := range outputSecrets {
@@ -161,33 +205,45 @@ func ProcessOutputSecret(ctx context.Context, output smap.Map, bulker bulk.Bulk)
 		})
 	}
 	if len(secretReferences) == 0 {
-		return nil
+		return nil, nil
 	}
 	secretValues, err := getSecretValues(ctx, secretReferences, bulker)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	for _, secret := range outputSecrets {
+		var key string
+		for _, p := range secret.Path {
+			if key == "" {
+				key = p
+				continue
+			}
+			key = key + "." + p
+		}
+		keys = append(keys, key)
 		err = setSecretPath(output, secretValues[secret.ID], secret.Path)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
-	return nil
+	return keys, nil
 }
 
 // replaceStringRef replaces values matching a secret ref regex, e.g. $co.elastic.secret{<secret ref>} -> <secret value>
 // and does this for multiple matches
-func replaceStringRef(ref string, secretValues map[string]string) string {
+// returns the resulting string value, and if any replacements were made
+func replaceStringRef(ref string, secretValues map[string]string) (string, bool) {
+	hasReplaced := false
 	matches := secretRegex.FindStringSubmatch(ref)
 	for len(matches) > 1 {
 		secretRef := matches[1]
 		if val, ok := secretValues[secretRef]; ok {
+			hasReplaced = true
 			ref = strings.Replace(ref, matches[0], val, 1)
 			matches = secretRegex.FindStringSubmatch(ref)
 			continue
 		}
 		break
 	}
-	return ref
+	return ref, hasReplaced
 }