Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions pkg/util/scrubber/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,39 @@ func AddDefaultReplacers(scrubber *Scrubber) {
)
appKeyYaml.LastUpdated = parseVersion("7.44.0") // https://github.com/DataDog/datadog-agent/pull/15707

// additional_endpoints has two schemas in the codebase:
//
// Format A: map[string][]string keyed by endpoint URL
// - Top-level `additional_endpoints`, `process_config.additional_endpoints`, etc.
// - Sub-values are endpoint URLs
// - Each URL maps to an array of API keys
//
// Format B: []map[string]interface{} a list of structured endpoints will well-defined fields
// - Fields include `api_key` which should be scrubbed, and `host`, `port`, etc. which shouldn't
// - Used by `logs_config.additional_endpoints`
//
// For A we scrub all leaf nodes aggressively. For B we re-enter the scrubber on the subtree so
// the existing key-based replacers (apiKeyYaml, passwordReplacer, ...) handle the sensitive
// fields (just api_key right now, maybe more in the future) while preserving host / port / etc.
//
// (We have to manually recurse in the case of Format B because otherwise matching the top-level
// additional-endpoints key would cause the YAML parser to skip all subnodes).
additionalEndpointsYaml := Replacer{
YAMLKeyRegex: regexp.MustCompile(`^additional_endpoints$`),
ProcessValue: func(data any) any {
switch data.(type) {
case map[string]interface{}, map[interface{}]interface{}:
return scrubAllLeafValues(data)
case []interface{}:
wrapped := data
scrubber.ScrubDataObj(&wrapped)
return wrapped
}
return data
},
LastUpdated: parseVersion("7.78.5"),
}

// HTTP header-style API keys with "key" suffix
httpHeaderKeyReplacer := matchYAMLKeyPrefixSuffix(
`x-`,
Expand Down Expand Up @@ -317,6 +350,7 @@ func AddDefaultReplacers(scrubber *Scrubber) {

scrubber.AddReplacer(SingleLine, apiKeyYaml)
scrubber.AddReplacer(SingleLine, appKeyYaml)
scrubber.AddReplacer(SingleLine, additionalEndpointsYaml)

scrubber.AddReplacer(MultiLine, snmpMultilineReplacer)
scrubber.AddReplacer(MultiLine, certReplacer)
Expand Down Expand Up @@ -484,6 +518,34 @@ func ScrubDataObj(data *interface{}) {
DefaultScrubber.ScrubDataObj(data)
}

// scrubAllLeafValues recursively walks data and replaces every leaf value with
// defaultReplacement while preserving the surrounding map and slice structure.
// Nil values are preserved so YAML round-trips don't materialize "********" in
// place of an absent value.
func scrubAllLeafValues(data interface{}) interface{} {
switch v := data.(type) {
case map[string]interface{}:
for k, vv := range v {
v[k] = scrubAllLeafValues(vv)
}
return v
case map[interface{}]interface{}:
for k, vv := range v {
v[k] = scrubAllLeafValues(vv)
}
return v
case []interface{}:
for i, vv := range v {
v[i] = scrubAllLeafValues(vv)
}
return v
case nil:
return nil
default:
return defaultReplacement
Comment on lines +544 to +545
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve ENC placeholders in additional_endpoints values

ScrubDataObj normally preserves ENC[...] strings, but scrubAllLeafValues now replaces every non-nil leaf under additional_endpoints with "********". For configs that use secret-backend placeholders inside URL-keyed additional_endpoints maps, this commit will scrub the placeholder itself instead of preserving it, which is a behavior regression compared with other sensitive keys and can remove useful diagnostics in flare output.

Useful? React with 👍 / 👎.

}
}

// HideKeyExceptLastChars replaces all characters in the key with "*", except
// for the last N characters, where N scales logarithmically with key length:
// - ≤ 4 chars: fully replaced with defaultReplacement
Expand Down
71 changes: 71 additions & 0 deletions pkg/util/scrubber/yaml_scrubber_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,77 @@ func TestConfigScrubbedYaml(t *testing.T) {
assert.Equal(t, trimmedOutput, trimmedCleaned)
}

func TestAdditionalEndpointsScrub(t *testing.T) {
t.Run("URL-keyed map: opaque API keys are scrubbed", func(t *testing.T) {
input := `additional_endpoints:
"https://mydomain.datadoghq.com":
- apikey2
- apikey3
"https://mydomain.datadoghq.eu":
- apikey4
`
expected := `additional_endpoints:
"https://mydomain.datadoghq.com":
- "********"
- "********"
"https://mydomain.datadoghq.eu":
- "********"
`
scrubbed, err := ScrubYamlString(input)
require.NoError(t, err)
require.YAMLEq(t, expected, scrubbed)
})

t.Run("URL-keyed map: URL keys themselves are preserved", func(t *testing.T) {
input := `additional_endpoints:
"https://mydomain.datadoghq.com":
- secret
`
scrubbed, err := ScrubYamlString(input)
require.NoError(t, err)
assert.Contains(t, scrubbed, "https://mydomain.datadoghq.com")
})

t.Run("URL-keyed map: siblings are untouched", func(t *testing.T) {
input := `site: datadoghq.com
additional_endpoints:
"https://mydomain.datadoghq.com":
- apikey2
`
scrubbed, err := ScrubYamlString(input)
require.NoError(t, err)
assert.Contains(t, scrubbed, "site: datadoghq.com")
})

t.Run("list-of-endpoint-structs: only api_key is scrubbed", func(t *testing.T) {
// This is the format used by logs_config.additional_endpoints and other
// bindEnvAndSetLogsConfigKeys-prefixed bindings.
input := `logs_config:
additional_endpoints:
- api_key: aaaaaaaaaaaaaaaaaaaaaaaaaaaabbbb
host: agent-http-intake.logs.datadoghq.com
port: 443
use_ssl: true
path_prefix: /v1
- api_key: cccccccccccccccccccccccccccccccc
host: backup-intake.logs.datadoghq.eu
port: 10516
use_ssl: false
`
scrubbed, err := ScrubYamlString(input)
require.NoError(t, err)
// API keys should be scrubbed
assert.NotContains(t, scrubbed, "aaaaaaaaaaaaaaaaaaaaaaaaaaaabbbb")
assert.NotContains(t, scrubbed, "cccccccccccccccccccccccccccccccc")
// Non-sensitive fields must be preserved
assert.Contains(t, scrubbed, "agent-http-intake.logs.datadoghq.com")
assert.Contains(t, scrubbed, "backup-intake.logs.datadoghq.eu")
assert.Contains(t, scrubbed, "443")
assert.Contains(t, scrubbed, "10516")
assert.Contains(t, scrubbed, "/v1")
})
}

func TestEmptyYaml(t *testing.T) {
cleaned, err := ScrubYaml(nil)
require.NoError(t, err)
Expand Down
Loading