From 879fcee3c3339f8ebd2581c2bda6950988c548fa Mon Sep 17 00:00:00 2001 From: Ramkumar Chinchani <45800463+rchincha@users.noreply.github.com> Date: Sat, 6 Jun 2026 22:18:20 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20enhance=20config=20sanitization=20to=20?= =?UTF-8?q?mask=20sensitive=20keys=20in=20storage=20a=E2=80=A6=20(#4119)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: enhance config sanitization to mask sensitive keys in storage and session drivers Fixes issue #4117 Signed-off-by: Ramkumar Chinchani * fix: annotate test fixture tokens to suppress security linter warnings Signed-off-by: Ramkumar Chinchani * fix: update test fixture credentials to suppress security linter warnings Signed-off-by: Ramkumar Chinchani * fix: simplify slice value sanitization by removing unnecessary index handling Signed-off-by: Ramkumar Chinchani * fix: update RedisDB test to use a guaranteed-invalid endpoint for CI stability Signed-off-by: Ramkumar Chinchani * fix: update sanitization logic to handle empty credentials for event sinks Signed-off-by: Ramkumar Chinchani * fix: format sensitive config map keys for improved readability Signed-off-by: Ramkumar Chinchani --------- Signed-off-by: Ramkumar Chinchani --- pkg/api/config/config.go | 85 ++++++++++++++++++++++++++++++++++- pkg/api/config/config_test.go | 84 ++++++++++++++++++++++++++++++++-- pkg/meta/meta_test.go | 4 +- 3 files changed, 167 insertions(+), 6 deletions(-) diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index 43614748..a0269ed2 100644 --- a/pkg/api/config/config.go +++ b/pkg/api/config/config.go @@ -5,6 +5,7 @@ import ( "maps" "os" "slices" + "strings" "sync" "sync/atomic" "time" @@ -819,6 +820,58 @@ func (c *Config) isTagsRetentionEnabled(tagRetentionPolicy KeepTagsPolicy) bool return false } +func isSensitiveConfigMapKey(key string) bool { + normalized := strings.ToLower(strings.ReplaceAll(key, "_", "")) + + switch normalized { + case "accesskey", "secretkey", "password", "secret", "token", + "clientsecret", "sessionhashkey", "sessionencryptkey", "sentinelpassword": + return true + default: + return false + } +} + +func sanitizeStringMapValues(values map[string]string) { + for key := range values { + if isSensitiveConfigMapKey(key) { + values[key] = "******" + } + } +} + +func sanitizeMapValues(values map[string]any) { + for key, value := range values { + if isSensitiveConfigMapKey(key) { + values[key] = "******" + + continue + } + + switch typedVal := value.(type) { + case map[string]any: + sanitizeMapValues(typedVal) + case map[string]string: + sanitizeStringMapValues(typedVal) + case []any: + sanitizeSliceValues(typedVal) + } + } +} + +func sanitizeSliceValues(values []any) { + for _, value := range values { + switch typedVal := value.(type) { + case map[string]any: + sanitizeMapValues(typedVal) + case map[string]string: + sanitizeStringMapValues(typedVal) + case []any: + sanitizeSliceValues(typedVal) + } + } +} + // Sanitize makes a sanitized copy of the config removing any secrets. func (c *Config) Sanitize() *Config { if c == nil { @@ -834,8 +887,32 @@ func (c *Config) Sanitize() *Config { panic(err) } + if sanitizedConfig.Storage.StorageDriver != nil { + sanitizeMapValues(sanitizedConfig.Storage.StorageDriver) + } + + if sanitizedConfig.Storage.CacheDriver != nil { + sanitizeMapValues(sanitizedConfig.Storage.CacheDriver) + } + + for subPath, subPathCfg := range sanitizedConfig.Storage.SubPaths { + if subPathCfg.StorageDriver != nil { + sanitizeMapValues(subPathCfg.StorageDriver) + } + + if subPathCfg.CacheDriver != nil { + sanitizeMapValues(subPathCfg.CacheDriver) + } + + sanitizedConfig.Storage.SubPaths[subPath] = subPathCfg + } + // Sanitize HTTP config if c.HTTP.Auth != nil { + if sanitizedConfig.HTTP.Auth.SessionDriver != nil { + sanitizeMapValues(sanitizedConfig.HTTP.Auth.SessionDriver) + } + // Sanitize LDAP bind password if c.HTTP.Auth.LDAP != nil && c.HTTP.Auth.LDAP.bindPassword != "" { sanitizedConfig.HTTP.Auth.LDAP = &LDAPConfig{} @@ -880,7 +957,13 @@ func (c *Config) Sanitize() *Config { panic(err) } - sanitizedConfig.Extensions.Events.Sinks[i].Credentials.Password = "******" + if sanitizedConfig.Extensions.Events.Sinks[i].Credentials.Password != "" { + sanitizedConfig.Extensions.Events.Sinks[i].Credentials.Password = "******" + } + + if sanitizedConfig.Extensions.Events.Sinks[i].Credentials.Token != "" { + sanitizedConfig.Extensions.Events.Sinks[i].Credentials.Token = "******" + } } } diff --git a/pkg/api/config/config_test.go b/pkg/api/config/config_test.go index 6467f8ef..0afc8767 100644 --- a/pkg/api/config/config_test.go +++ b/pkg/api/config/config_test.go @@ -165,14 +165,16 @@ func TestConfig(t *testing.T) { Credentials: &eventsconf.Credentials{ Username: "webhook-user", Password: "webhook-password", + Token: "webhook-token", }, }, { Type: eventsconf.NATS, Address: "nats://localhost:4222", - Credentials: &eventsconf.Credentials{ + Credentials: &eventsconf.Credentials{ //nolint:gosec // test fixture Username: "nats-user", Password: "nats-token", + Token: "nats-auth-token", }, }, }, @@ -186,6 +188,8 @@ func TestConfig(t *testing.T) { // Verify event sink credentials passwords are sanitized So(sanitizedConf.Extensions.Events.Sinks[0].Credentials.Password, ShouldEqual, "******") So(sanitizedConf.Extensions.Events.Sinks[1].Credentials.Password, ShouldEqual, "******") + So(sanitizedConf.Extensions.Events.Sinks[0].Credentials.Token, ShouldEqual, "******") + So(sanitizedConf.Extensions.Events.Sinks[1].Credentials.Token, ShouldEqual, "******") // Verify other fields are preserved So(sanitizedConf.Extensions.Events.Sinks[0].Credentials.Username, ShouldEqual, "webhook-user") @@ -196,6 +200,80 @@ func TestConfig(t *testing.T) { // Verify original config is not modified So(conf.Extensions.Events.Sinks[0].Credentials.Password, ShouldEqual, "webhook-password") So(conf.Extensions.Events.Sinks[1].Credentials.Password, ShouldEqual, "nats-token") + So(conf.Extensions.Events.Sinks[0].Credentials.Token, ShouldEqual, "webhook-token") + So(conf.Extensions.Events.Sinks[1].Credentials.Token, ShouldEqual, "nats-auth-token") + }) + + Convey("Test Sanitize() with storage and session driver secrets", func() { + conf := config.New() + So(conf, ShouldNotBeNil) + + conf.Storage.StorageDriver = map[string]any{ + "name": "s3", + "region": "us-east-1", + "accesskey": "storage-access", + "secretkey": "storage-secret", + "nested": map[string]any{ //nolint:gosec // test fixture + "token": "nested-token", + "secretKey": "nested-secret-key", + }, + } + + conf.Storage.CacheDriver = map[string]any{ + "name": "redis", + "password": "cache-password", + } + + conf.Storage.SubPaths = map[string]config.StorageConfig{ + "/team-a": { + StorageDriver: map[string]any{ + "name": "s3", + "access_key": "subpath-access", + "secret_key": "subpath-secret", + }, + CacheDriver: map[string]any{ //nolint:gosec // test fixture + "name": "memory", + "token": "subpath-cache-token", + }, + }, + } + + conf.HTTP.Auth = &config.AuthConfig{ + SessionDriver: map[string]any{ + "name": "redis", + "password": "session-password", + "token": "session-token", + }, + } + + sanitizedConf := conf.Sanitize() + + So(sanitizedConf.Storage.StorageDriver["name"], ShouldEqual, "s3") + So(sanitizedConf.Storage.StorageDriver["accesskey"], ShouldEqual, "******") + So(sanitizedConf.Storage.StorageDriver["secretkey"], ShouldEqual, "******") + + nestedMap, ok := sanitizedConf.Storage.StorageDriver["nested"].(map[string]any) + So(ok, ShouldBeTrue) + So(nestedMap["token"], ShouldEqual, "******") + So(nestedMap["secretKey"], ShouldEqual, "******") + + So(sanitizedConf.Storage.CacheDriver["password"], ShouldEqual, "******") + So(sanitizedConf.Storage.SubPaths["/team-a"].StorageDriver["access_key"], ShouldEqual, "******") + So(sanitizedConf.Storage.SubPaths["/team-a"].StorageDriver["secret_key"], ShouldEqual, "******") + So(sanitizedConf.Storage.SubPaths["/team-a"].CacheDriver["token"], ShouldEqual, "******") + + So(sanitizedConf.HTTP.Auth.SessionDriver["name"], ShouldEqual, "redis") + So(sanitizedConf.HTTP.Auth.SessionDriver["password"], ShouldEqual, "******") + So(sanitizedConf.HTTP.Auth.SessionDriver["token"], ShouldEqual, "******") + + So(conf.Storage.StorageDriver["accesskey"], ShouldEqual, "storage-access") + So(conf.Storage.StorageDriver["secretkey"], ShouldEqual, "storage-secret") + So(conf.Storage.CacheDriver["password"], ShouldEqual, "cache-password") + So(conf.Storage.SubPaths["/team-a"].StorageDriver["access_key"], ShouldEqual, "subpath-access") + So(conf.Storage.SubPaths["/team-a"].StorageDriver["secret_key"], ShouldEqual, "subpath-secret") + So(conf.Storage.SubPaths["/team-a"].CacheDriver["token"], ShouldEqual, "subpath-cache-token") + So(conf.HTTP.Auth.SessionDriver["password"], ShouldEqual, "session-password") + So(conf.HTTP.Auth.SessionDriver["token"], ShouldEqual, "session-token") }) Convey("Test Sanitize() with Event sink credentials including nil credentials", func() { @@ -382,8 +460,8 @@ func TestConfig(t *testing.T) { So(sanitizedConf.HTTP.Auth.LDAP.BindPassword(), ShouldEqual, "") // OpenID empty secret is always sanitized So(sanitizedConf.HTTP.Auth.OpenID.Providers["empty"].ClientSecret, ShouldEqual, "******") - // Event sink empty password is always sanitized - So(sanitizedConf.Extensions.Events.Sinks[0].Credentials.Password, ShouldEqual, "******") + // Event sink empty password remains empty when no credential value is present + So(sanitizedConf.Extensions.Events.Sinks[0].Credentials.Password, ShouldEqual, "") }) Convey("Test Sanitize() with nil config", func() { diff --git a/pkg/meta/meta_test.go b/pkg/meta/meta_test.go index 5153c427..66224411 100644 --- a/pkg/meta/meta_test.go +++ b/pkg/meta/meta_test.go @@ -3191,10 +3191,10 @@ func TestCreateRedisDB(t *testing.T) { }) Convey("Fails on Ping()", func() { - // Redis client will not be responding + // Use a guaranteed-invalid endpoint to avoid free-port races in CI. cacheDriverParams := map[string]any{ "name": "redis", - "url": "redis://127.0.0.1:" + tCommon.GetFreePort(), + "url": "redis://127.0.0.1:0", } conf.Storage.CacheDriver = cacheDriverParams