From 647b43b4d445c172000351ceefbb1c4fdf0d0bcf Mon Sep 17 00:00:00 2001 From: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> Date: Thu, 6 Feb 2025 19:12:32 -0800 Subject: [PATCH] Redacted nested secrets path value (#6710) Redact secrets within complex nested paths. --- ...t_paths-redaction-along-complex-paths.yaml | 32 +++++++++++++++ internal/pkg/diagnostics/diagnostics.go | 11 +++-- internal/pkg/diagnostics/diagnostics_test.go | 41 ++++++++++++++++++- 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 changelog/fragments/1738874791-Fix-secret_paths-redaction-along-complex-paths.yaml diff --git a/changelog/fragments/1738874791-Fix-secret_paths-redaction-along-complex-paths.yaml b/changelog/fragments/1738874791-Fix-secret_paths-redaction-along-complex-paths.yaml new file mode 100644 index 00000000000..9346d6a1e5c --- /dev/null +++ b/changelog/fragments/1738874791-Fix-secret_paths-redaction-along-complex-paths.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: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: Fix secret_paths redaction along complex 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: + +# 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/6710 + +# 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 diff --git a/internal/pkg/diagnostics/diagnostics.go b/internal/pkg/diagnostics/diagnostics.go index 1e7ec30c7d6..f4b0d70ed90 100644 --- a/internal/pkg/diagnostics/diagnostics.go +++ b/internal/pkg/diagnostics/diagnostics.go @@ -398,7 +398,8 @@ func redactKey(k string) bool { strings.Contains(k, "passphrase") || strings.Contains(k, "password") || strings.Contains(k, "token") || - strings.Contains(k, "key") + strings.Contains(k, "key") || + strings.Contains(k, "secret") } func zipLogs(zw *zip.Writer, ts time.Time, topPath string, excludeEvents bool) error { @@ -593,7 +594,7 @@ func RedactSecretPaths(mapStr map[string]any, errOut io.Writer) map[string]any { fmt.Fprintln(errOut, "No output redaction: secret_paths attribute is not a list.") return mapStr } - cfg := ucfg.MustNewFrom(mapStr) + cfg := ucfg.MustNewFrom(mapStr, ucfg.PathSep(".")) for _, v := range arr { key, ok := v.(string) if !ok { @@ -601,11 +602,15 @@ func RedactSecretPaths(mapStr map[string]any, errOut io.Writer) map[string]any { continue } - if ok, _ := cfg.Has(key, -1, ucfg.PathSep(".")); ok { + if ok, err := cfg.Has(key, -1, ucfg.PathSep(".")); err != nil { + fmt.Fprintf(errOut, "Error redacting secret path %q: %v.\n", key, err) + } else if ok { err := cfg.SetString(key, -1, REDACTED, ucfg.PathSep(".")) if err != nil { fmt.Fprintf(errOut, "No output redaction for %q: %v.\n", key, err) } + } else { + fmt.Fprintf(errOut, "Unable to find secret path %q for redaction.\n", key) } } result, err := config.MustNewConfigFrom(cfg).ToMapStr() diff --git a/internal/pkg/diagnostics/diagnostics_test.go b/internal/pkg/diagnostics/diagnostics_test.go index 706ca550f24..87252cbf3e5 100644 --- a/internal/pkg/diagnostics/diagnostics_test.go +++ b/internal/pkg/diagnostics/diagnostics_test.go @@ -248,6 +248,43 @@ secret_paths: - inputs.0.redactKey - inputs.1.missingKey - outputs.default.redactOtherKey +`, + }, { + name: "path in nested list", + input: []byte(`id: test-policy +inputs: + - type: httpjson + data_stream: + namespace: default + streams: + - config_version: "2" + request.transforms: + - set: + target: header.Authorization + value: SSWS this-should-be-redacted + - set: + target: url.params.limit + value: "1000" +secret_paths: + - inputs.0.streams.0.request.transforms.0.set.value +`), + expect: `id: test-policy +inputs: + - data_stream: + namespace: default + streams: + - config_version: "2" + request: + transforms: + - set: + target: header.Authorization + value: + - set: + target: url.params.limit + value: "1000" + type: httpjson +secret_paths: + - inputs.0.streams.0.request.transforms.0.set.value `, }} @@ -255,9 +292,11 @@ secret_paths: 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) + var errOut bytes.Buffer + err := writeRedacted(&errOut, &out, "testPath", file) require.NoError(t, err) + t.Logf("Error output: %s", errOut.String()) assert.Equal(t, tc.expect, out.String()) }) }