Skip to content

Commit 18f58f0

Browse files
authored
br: redact secret strings when logging arguments (#57593) (#57601)
close #57585
1 parent 26d1843 commit 18f58f0

File tree

2 files changed

+49
-8
lines changed

2 files changed

+49
-8
lines changed

br/pkg/task/common.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,16 +738,20 @@ func ReadBackupMeta(
738738
// flagToZapField checks whether this flag can be logged,
739739
// if need to log, return its zap field. Or return a field with hidden value.
740740
func flagToZapField(f *pflag.Flag) zap.Field {
741-
if f.Name == flagStorage {
741+
switch f.Name {
742+
case flagStorage, FlagStreamFullBackupStorage:
742743
hiddenQuery, err := url.Parse(f.Value.String())
743744
if err != nil {
744745
return zap.String(f.Name, "<invalid URI>")
745746
}
746747
// hide all query here.
747748
hiddenQuery.RawQuery = ""
748749
return zap.Stringer(f.Name, hiddenQuery)
750+
case flagCipherKey:
751+
return zap.String(f.Name, "<redacted>")
752+
default:
753+
return zap.Stringer(f.Name, f.Value)
749754
}
750-
return zap.Stringer(f.Name, f.Value)
751755
}
752756

753757
// LogArguments prints origin command arguments.

br/pkg/task/common_test.go

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,50 @@ func (f fakeValue) Type() string {
3232
}
3333

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

4481
func TestTiDBConfigUnchanged(t *testing.T) {

0 commit comments

Comments
 (0)