Skip to content

Commit 65fd2ad

Browse files
authored
br: redact secret strings when logging arguments (pingcap#57593) (pingcap#57604)
close pingcap#57585
1 parent 7959487 commit 65fd2ad

File tree

2 files changed

+55
-8
lines changed

2 files changed

+55
-8
lines changed

br/pkg/task/common.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -732,16 +732,20 @@ func ReadBackupMeta(
732732
// flagToZapField checks whether this flag can be logged,
733733
// if need to log, return its zap field. Or return a field with hidden value.
734734
func flagToZapField(f *pflag.Flag) zap.Field {
735-
if f.Name == flagStorage {
735+
switch f.Name {
736+
case flagStorage, FlagStreamFullBackupStorage:
736737
hiddenQuery, err := url.Parse(f.Value.String())
737738
if err != nil {
738739
return zap.String(f.Name, "<invalid URI>")
739740
}
740741
// hide all query here.
741742
hiddenQuery.RawQuery = ""
742743
return zap.Stringer(f.Name, hiddenQuery)
744+
case flagCipherKey, "azblob.encryption-key":
745+
return zap.String(f.Name, "<redacted>")
746+
default:
747+
return zap.Stringer(f.Name, f.Value)
743748
}
744-
return zap.Stringer(f.Name, f.Value)
745749
}
746750

747751
// LogArguments prints origin command arguments.

br/pkg/task/common_test.go

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,56 @@ func (f fakeValue) Type() string {
3333
}
3434

3535
func TestUrlNoQuery(t *testing.T) {
36-
flag := &pflag.Flag{
37-
Name: flagStorage,
38-
Value: fakeValue("s3://some/what?secret=a123456789&key=987654321"),
36+
testCases := []struct {
37+
inputName string
38+
expectedName string
39+
inputValue string
40+
expectedValue string
41+
}{
42+
{
43+
inputName: flagSendCreds,
44+
expectedName: "send-credentials-to-tikv",
45+
inputValue: "true",
46+
expectedValue: "true",
47+
},
48+
{
49+
inputName: flagStorage,
50+
expectedName: "storage",
51+
inputValue: "s3://some/what?secret=a123456789&key=987654321",
52+
expectedValue: "s3://some/what",
53+
},
54+
{
55+
inputName: FlagStreamFullBackupStorage,
56+
expectedName: "full-backup-storage",
57+
inputValue: "s3://bucket/prefix/?access-key=1&secret-key=2",
58+
expectedValue: "s3://bucket/prefix/",
59+
},
60+
{
61+
inputName: flagCipherKey,
62+
expectedName: "crypter.key",
63+
inputValue: "537570657253656372657456616C7565",
64+
expectedValue: "<redacted>",
65+
},
66+
{
67+
inputName: "azblob.encryption-key",
68+
expectedName: "azblob.encryption-key",
69+
inputValue: "SUPERSECRET_AZURE_ENCRYPTION_KEY",
70+
expectedValue: "<redacted>",
71+
},
72+
}
73+
74+
for _, tc := range testCases {
75+
flag := pflag.Flag{
76+
Name: tc.inputName,
77+
Value: fakeValue(tc.inputValue),
78+
}
79+
field := flagToZapField(&flag)
80+
require.Equal(t, tc.expectedName, field.Key, `test-case [%s="%s"]`, tc.expectedName, tc.expectedValue)
81+
if stringer, ok := field.Interface.(fmt.Stringer); ok {
82+
field.String = stringer.String()
83+
}
84+
require.Equal(t, tc.expectedValue, field.String, `test-case [%s="%s"]`, tc.expectedName, tc.expectedValue)
3985
}
40-
field := flagToZapField(flag)
41-
require.Equal(t, flagStorage, field.Key)
42-
require.Equal(t, "s3://some/what", field.Interface.(fmt.Stringer).String())
4386
}
4487

4588
func TestTiDBConfigUnchanged(t *testing.T) {

0 commit comments

Comments
 (0)