Skip to content

Commit

Permalink
br: redact secret strings when logging arguments (#57593) (#57605)
Browse files Browse the repository at this point in the history
close #57585
  • Loading branch information
ti-chi-bot authored Nov 22, 2024
1 parent c257fbe commit de49db5
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 8 deletions.
11 changes: 9 additions & 2 deletions br/pkg/task/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -895,16 +895,23 @@ func ReadBackupMeta(
// flagToZapField checks whether this flag can be logged,
// if need to log, return its zap field. Or return a field with hidden value.
func flagToZapField(f *pflag.Flag) zap.Field {
if f.Name == flagStorage {
switch f.Name {
case flagStorage, FlagStreamFullBackupStorage:
hiddenQuery, err := url.Parse(f.Value.String())
if err != nil {
return zap.String(f.Name, "<invalid URI>")
}
// hide all query here.
hiddenQuery.RawQuery = ""
return zap.Stringer(f.Name, hiddenQuery)
case flagFullBackupCipherKey, flagLogBackupCipherKey, "azblob.encryption-key":
return zap.String(f.Name, "<redacted>")
case flagMasterKeyConfig:
// TODO: we don't really need to hide the entirety of --master-key, consider parsing the URL here.
return zap.String(f.Name, "<redacted>")
default:
return zap.Stringer(f.Name, f.Value)
}
return zap.Stringer(f.Name, f.Value)
}

// LogArguments prints origin command arguments.
Expand Down
68 changes: 62 additions & 6 deletions br/pkg/task/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,69 @@ func (f fakeValue) Type() string {
}

func TestUrlNoQuery(t *testing.T) {
flag := &pflag.Flag{
Name: flagStorage,
Value: fakeValue("s3://some/what?secret=a123456789&key=987654321"),
testCases := []struct {
inputName string
expectedName string
inputValue string
expectedValue string
}{
{
inputName: flagSendCreds,
expectedName: "send-credentials-to-tikv",
inputValue: "true",
expectedValue: "true",
},
{
inputName: flagStorage,
expectedName: "storage",
inputValue: "s3://some/what?secret=a123456789&key=987654321",
expectedValue: "s3://some/what",
},
{
inputName: FlagStreamFullBackupStorage,
expectedName: "full-backup-storage",
inputValue: "s3://bucket/prefix/?access-key=1&secret-key=2",
expectedValue: "s3://bucket/prefix/",
},
{
inputName: flagFullBackupCipherKey,
expectedName: "crypter.key",
inputValue: "537570657253656372657456616C7565",
expectedValue: "<redacted>",
},
{
inputName: flagLogBackupCipherKey,
expectedName: "log.crypter.key",
inputValue: "537570657253656372657456616C7565",
expectedValue: "<redacted>",
},
{
inputName: "azblob.encryption-key",
expectedName: "azblob.encryption-key",
inputValue: "SUPERSECRET_AZURE_ENCRYPTION_KEY",
expectedValue: "<redacted>",
},
{
inputName: flagMasterKeyConfig,
expectedName: "master-key",
inputValue: "local:///path/abcd,aws-kms:///abcd?AWS_ACCESS_KEY_ID=SECRET1&AWS_SECRET_ACCESS_KEY=SECRET2&REGION=us-east-1,azure-kms:///abcd/v1?AZURE_TENANT_ID=tenant-id&AZURE_CLIENT_ID=client-id&AZURE_CLIENT_SECRET=client-secret&AZURE_VAULT_NAME=vault-name",
expectedValue: "<redacted>",
// expectedValue: "local:///path/abcd,aws-kms:///abcd,azure-kms:///abcd/v1"
},
}

for _, tc := range testCases {
flag := pflag.Flag{
Name: tc.inputName,
Value: fakeValue(tc.inputValue),
}
field := flagToZapField(&flag)
require.Equal(t, tc.expectedName, field.Key, `test-case [%s="%s"]`, tc.expectedName, tc.expectedValue)
if stringer, ok := field.Interface.(fmt.Stringer); ok {
field.String = stringer.String()
}
require.Equal(t, tc.expectedValue, field.String, `test-case [%s="%s"]`, tc.expectedName, tc.expectedValue)
}
field := flagToZapField(flag)
require.Equal(t, flagStorage, field.Key)
require.Equal(t, "s3://some/what", field.Interface.(fmt.Stringer).String())
}

func TestTiDBConfigUnchanged(t *testing.T) {
Expand Down

0 comments on commit de49db5

Please sign in to comment.