Skip to content

Commit f8d94cb

Browse files
ProximystProximyst
authored
fix: Mask all credentials (#20766)
1 parent c3d0de9 commit f8d94cb

7 files changed

Lines changed: 35 additions & 31 deletions

File tree

‎pkg/loki/config_wrapper_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ memberlist:
630630
assert.Equal(t, "testbucket", actual.Bucket)
631631
assert.Equal(t, "https://example.com", actual.Endpoint)
632632
assert.Equal(t, "abc123", actual.AccessKeyID)
633-
assert.Equal(t, "def789", actual.SecretAccessKey)
633+
assert.Equal(t, flagext.SecretWithValue("def789"), actual.SecretAccessKey)
634634
}
635635

636636
// should remain empty

‎pkg/ruler/base/notifier.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,15 @@ func amConfigFromURL(cfg *ruler_config.AlertManagerConfig, url *url.URL, apiVers
218218
if cfg.Notifier.BasicAuth.IsEnabled() {
219219
amConfig.HTTPClientConfig.BasicAuth = &config_util.BasicAuth{
220220
Username: cfg.Notifier.BasicAuth.Username,
221-
Password: config_util.Secret(cfg.Notifier.BasicAuth.Password),
221+
Password: config_util.Secret(cfg.Notifier.BasicAuth.Password.String()),
222222
}
223223
}
224224

225225
if cfg.Notifier.HeaderAuth.IsEnabled() {
226-
if cfg.Notifier.HeaderAuth.Credentials != "" {
226+
if cfg.Notifier.HeaderAuth.Credentials.String() != "" {
227227
amConfig.HTTPClientConfig.Authorization = &config_util.Authorization{
228228
Type: cfg.Notifier.HeaderAuth.Type,
229-
Credentials: config_util.Secret(cfg.Notifier.HeaderAuth.Credentials),
229+
Credentials: config_util.Secret(cfg.Notifier.HeaderAuth.Credentials.String()),
230230
}
231231
} else if cfg.Notifier.HeaderAuth.CredentialsFile != "" {
232232
amConfig.HTTPClientConfig.Authorization = &config_util.Authorization{

‎pkg/ruler/base/notifier_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414
"github.com/prometheus/prometheus/model/relabel"
1515
"github.com/stretchr/testify/require"
1616

17+
"github.com/grafana/dskit/flagext"
18+
1719
ruler_config "github.com/grafana/loki/v3/pkg/ruler/config"
1820
"github.com/grafana/loki/v3/pkg/util"
1921
)
@@ -210,7 +212,7 @@ func TestBuildNotifierConfig(t *testing.T) {
210212
Notifier: ruler_config.NotifierConfig{
211213
BasicAuth: util.BasicAuth{
212214
Username: "jacob",
213-
Password: "test",
215+
Password: flagext.SecretWithValue("test"),
214216
},
215217
},
216218
},
@@ -245,7 +247,7 @@ func TestBuildNotifierConfig(t *testing.T) {
245247
Notifier: ruler_config.NotifierConfig{
246248
HeaderAuth: util.HeaderAuth{
247249
Type: "Bearer",
248-
Credentials: "jacob",
250+
Credentials: flagext.SecretWithValue("jacob"),
249251
},
250252
},
251253
},

‎pkg/storage/chunk/client/alibaba/oss_object_client.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strconv"
99

1010
"github.com/aliyun/aliyun-oss-go-sdk/oss"
11+
"github.com/grafana/dskit/flagext"
1112
"github.com/grafana/dskit/instrument"
1213
"github.com/pkg/errors"
1314
"github.com/prometheus/client_golang/prometheus"
@@ -35,12 +36,12 @@ type OssObjectClient struct {
3536

3637
// OssConfig is config for the OSS Chunk Client.
3738
type OssConfig struct {
38-
Bucket string `yaml:"bucket"`
39-
Endpoint string `yaml:"endpoint"`
40-
AccessKeyID string `yaml:"access_key_id"`
41-
SecretAccessKey string `yaml:"secret_access_key"`
42-
ConnectionTimeoutSec int64 `yaml:"conn_timeout_sec"`
43-
ReadWriteTimeoutSec int64 `yaml:"read_write_timeout_sec"`
39+
Bucket string `yaml:"bucket"`
40+
Endpoint string `yaml:"endpoint"`
41+
AccessKeyID string `yaml:"access_key_id"`
42+
SecretAccessKey flagext.Secret `yaml:"secret_access_key"`
43+
ConnectionTimeoutSec int64 `yaml:"conn_timeout_sec"`
44+
ReadWriteTimeoutSec int64 `yaml:"read_write_timeout_sec"`
4445
}
4546

4647
// RegisterFlags registers flags.
@@ -53,7 +54,7 @@ func (cfg *OssConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
5354
f.StringVar(&cfg.Bucket, prefix+"oss.bucketname", "", "Name of OSS bucket.")
5455
f.StringVar(&cfg.Endpoint, prefix+"oss.endpoint", "", "oss Endpoint to connect to.")
5556
f.StringVar(&cfg.AccessKeyID, prefix+"oss.access-key-id", "", "alibabacloud Access Key ID")
56-
f.StringVar(&cfg.SecretAccessKey, prefix+"oss.secret-access-key", "", "alibabacloud Secret Access Key")
57+
f.Var(&cfg.SecretAccessKey, prefix+"oss.secret-access-key", "alibabacloud Secret Access Key")
5758
f.Int64Var(&cfg.ConnectionTimeoutSec, prefix+"oss.conn-timeout-sec", 30, "Connection timeout in seconds")
5859
f.Int64Var(&cfg.ReadWriteTimeoutSec, prefix+"oss.read-write-timeout-sec", 60, "Read/Write timeout in seconds")
5960
}
@@ -67,7 +68,7 @@ func (cfg *OssConfig) Validate() error {
6768

6869
// NewOssObjectClient makes a new chunk.Client that writes chunks to OSS.
6970
func NewOssObjectClient(_ context.Context, cfg OssConfig) (client.ObjectClient, error) {
70-
client, err := oss.New(cfg.Endpoint, cfg.AccessKeyID, cfg.SecretAccessKey, oss.Timeout(cfg.ConnectionTimeoutSec, cfg.ReadWriteTimeoutSec))
71+
client, err := oss.New(cfg.Endpoint, cfg.AccessKeyID, cfg.SecretAccessKey.String(), oss.Timeout(cfg.ConnectionTimeoutSec, cfg.ReadWriteTimeoutSec))
7172
if err != nil {
7273
return nil, err
7374
}

‎pkg/storage/chunk/client/azure/blob_storage_client.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ type BlobStorageConfig struct {
8787
Environment string `yaml:"environment"`
8888
StorageAccountName string `yaml:"account_name"`
8989
StorageAccountKey flagext.Secret `yaml:"account_key"`
90-
ConnectionString string `yaml:"connection_string"`
90+
ConnectionString flagext.Secret `yaml:"connection_string"`
9191
ContainerName string `yaml:"container_name"`
9292
ActiveDirectoryEndpoint string `yaml:"active_directory_endpoint"`
9393
EndpointSuffix string `yaml:"endpoint_suffix"`
@@ -123,7 +123,7 @@ func (c *BlobStorageConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagS
123123
f.StringVar(&c.Environment, prefix+"azure.environment", azureGlobal, fmt.Sprintf("Azure Cloud environment. Supported values are: %s.", strings.Join(supportedEnvironments, ", ")))
124124
f.StringVar(&c.StorageAccountName, prefix+"azure.account-name", "", "Azure storage account name.")
125125
f.Var(&c.StorageAccountKey, prefix+"azure.account-key", "Azure storage account key.")
126-
f.StringVar(&c.ConnectionString, prefix+"azure.connection-string", "", "If `connection-string` is set, the values of `account-name` and `endpoint-suffix` values will not be used. Use this method over `account-key` if you need to authenticate via a SAS token. Or if you use the Azurite emulator.")
126+
f.Var(&c.ConnectionString, prefix+"azure.connection-string", "If `connection-string` is set, the values of `account-name` and `endpoint-suffix` values will not be used. Use this method over `account-key` if you need to authenticate via a SAS token. Or if you use the Azurite emulator.")
127127
f.StringVar(&c.ContainerName, prefix+"azure.container-name", constants.Loki, "Name of the storage account blob container used to store chunks. This container must be created before running cortex.")
128128
f.StringVar(&c.EndpointSuffix, prefix+"azure.endpoint-suffix", "", "Azure storage endpoint suffix without schema. The storage account name will be prefixed to this value to create the FQDN.")
129129
f.StringVar(&c.ActiveDirectoryEndpoint, prefix+"azure.active-directory-endpoint", "", "Azure active directory endpoint override. Use when the Azure SDK does not support your environment.")
@@ -407,8 +407,8 @@ func (b *BlobStorage) newPipeline(hedgingCfg hedging.Config, hedging bool) (pipe
407407
})
408408
}
409409

410-
if b.cfg.ConnectionString != "" {
411-
parsed, err := parseConnectionString(b.cfg.ConnectionString)
410+
if b.cfg.ConnectionString.String() != "" {
411+
parsed, err := parseConnectionString(b.cfg.ConnectionString.String())
412412
if err != nil {
413413
return nil, err
414414
}
@@ -626,7 +626,7 @@ func (c *BlobStorageConfig) Validate() error {
626626
}
627627

628628
func (b *BlobStorage) fmtResourceURL() string {
629-
if b.cfg.ConnectionString != "" {
629+
if b.cfg.ConnectionString.String() != "" {
630630
return b.endpointFromConnectionString()
631631
}
632632

@@ -641,7 +641,7 @@ func (b *BlobStorage) fmtResourceURL() string {
641641
}
642642

643643
func (b *BlobStorage) endpointFromConnectionString() string {
644-
parsed, err := parseConnectionString(b.cfg.ConnectionString)
644+
parsed, err := parseConnectionString(b.cfg.ConnectionString.String())
645645
if err != nil || parsed.ServiceURL == "" {
646646
level.Warn(log.Logger).Log("msg", "could not get resource URL from connection string", "err", err)
647647
}

‎pkg/storage/chunk/client/azure/blob_storage_client_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ func Test_EndpointSuffixWithContainer(t *testing.T) {
246246
func Test_ConnectionStringWithContainer(t *testing.T) {
247247
c, err := NewBlobStorage(&BlobStorageConfig{
248248
ContainerName: "foo",
249-
ConnectionString: "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=shorter=;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1;",
249+
ConnectionString: flagext.SecretWithValue("DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=shorter=;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1;"),
250250
}, metrics, hedging.Config{})
251251
require.NoError(t, err)
252252
expect, _ := url.Parse("http://127.0.0.1:10000/devstoreaccount1/foo")
@@ -287,7 +287,7 @@ func Test_EndpointSuffixWithBlob(t *testing.T) {
287287
func Test_ConnectionStringWithBlob(t *testing.T) {
288288
c, err := NewBlobStorage(&BlobStorageConfig{
289289
ContainerName: "foo",
290-
ConnectionString: "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=shorter=;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1;",
290+
ConnectionString: flagext.SecretWithValue("DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=shorter=;BlobEndpoint=http://127.0.0.1:10000/devstoreaccount1;"),
291291
}, metrics, hedging.Config{})
292292
require.NoError(t, err)
293293
expect, _ := url.Parse("http://127.0.0.1:10000/devstoreaccount1/foo/blob")

‎pkg/util/http.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/go-kit/log/level"
1818
"github.com/gogo/protobuf/proto"
1919
"github.com/golang/snappy"
20+
"github.com/grafana/dskit/flagext"
2021
"go.opentelemetry.io/otel/attribute"
2122
"go.opentelemetry.io/otel/trace"
2223
"gopkg.in/yaml.v2"
@@ -43,36 +44,36 @@ func IsRequestBodyTooLarge(err error) bool {
4344

4445
// BasicAuth configures basic authentication for HTTP clients.
4546
type BasicAuth struct {
46-
Username string `yaml:"basic_auth_username"`
47-
Password string `yaml:"basic_auth_password"`
47+
Username string `yaml:"basic_auth_username"`
48+
Password flagext.Secret `yaml:"basic_auth_password"`
4849
}
4950

5051
func (b *BasicAuth) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
5152
f.StringVar(&b.Username, prefix+"basic-auth-username", "", "HTTP Basic authentication username. It overrides the username set in the URL (if any).")
52-
f.StringVar(&b.Password, prefix+"basic-auth-password", "", "HTTP Basic authentication password. It overrides the password set in the URL (if any).")
53+
f.Var(&b.Password, prefix+"basic-auth-password", "HTTP Basic authentication password. It overrides the password set in the URL (if any).")
5354
}
5455

5556
// IsEnabled returns false if basic authentication isn't enabled.
5657
func (b BasicAuth) IsEnabled() bool {
57-
return b.Username != "" || b.Password != ""
58+
return b.Username != "" || b.Password.String() != ""
5859
}
5960

6061
// HeaderAuth condigures header based authorization for HTTP clients.
6162
type HeaderAuth struct {
62-
Type string `yaml:"type,omitempty"`
63-
Credentials string `yaml:"credentials,omitempty"`
64-
CredentialsFile string `yaml:"credentials_file,omitempty"`
63+
Type string `yaml:"type,omitempty"`
64+
Credentials flagext.Secret `yaml:"credentials,omitempty"`
65+
CredentialsFile string `yaml:"credentials_file,omitempty"`
6566
}
6667

6768
func (h *HeaderAuth) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
6869
f.StringVar(&h.Type, prefix+"type", "Bearer", "HTTP Header authorization type (default: Bearer).")
69-
f.StringVar(&h.Credentials, prefix+"credentials", "", "HTTP Header authorization credentials.")
70+
f.Var(&h.Credentials, prefix+"credentials", "HTTP Header authorization credentials.")
7071
f.StringVar(&h.CredentialsFile, prefix+"credentials-file", "", "HTTP Header authorization credentials file.")
7172
}
7273

7374
// IsEnabled returns false if header authorization isn't enabled.
7475
func (h HeaderAuth) IsEnabled() bool {
75-
return h.Credentials != "" || h.CredentialsFile != ""
76+
return h.Credentials.String() != "" || h.CredentialsFile != ""
7677
}
7778

7879
// WriteJSONResponse writes some JSON as a HTTP response.

0 commit comments

Comments
 (0)