Skip to content

Commit 81ff3d1

Browse files
iQQBotiQQBot
authored
[ws-manager-mk2] redact log (#18906)
* update to logrusr v4 * Add DeepCopyStruct to scrubber * use DeepCopyStruct to scrub log * mark wrokspace.stauts.url redact
1 parent bd88436 commit 81ff3d1

File tree

8 files changed

+344
-23
lines changed

8 files changed

+344
-23
lines changed

‎components/common-go/log/log.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,15 @@ type jsonEntry struct {
231231
Msg string `json:"msg,omitempty"`
232232
Time *time.Time `json:"time,omitempty"`
233233
}
234+
235+
// TrustedValueWrap is a simple wrapper that treats the entire value as trusted, which are not processed by the scrubber.
236+
// During JSON marshal, only the Value itself will be processed, without including Wrap.
237+
type TrustedValueWrap struct {
238+
Value any
239+
}
240+
241+
func (TrustedValueWrap) IsTrustedValue() {}
242+
243+
func (t TrustedValueWrap) MarshalJSON() ([]byte, error) {
244+
return json.Marshal(t.Value)
245+
}

‎components/scrubber/scrubber.go‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"reflect"
1111
"regexp"
1212
"strings"
13+
"unsafe"
1314

1415
"github.com/mitchellh/reflectwalk"
1516
)
@@ -86,6 +87,12 @@ type Scrubber interface {
8687
// }
8788
//
8889
Struct(val any) error
90+
91+
// DeepCopyStruct scrubes a struct with a deep copy.
92+
// The difference between `DeepCopyStruct` and `Struct`` is that DeepCopyStruct does not modify the structure directly,
93+
// but creates a deep copy instead.
94+
// Also, val can be a pointer or a structure.
95+
DeepCopyStruct(val any) any
8996
}
9097

9198
// Default is the default scrubber consumers of this package should use
@@ -189,6 +196,145 @@ func (s *scrubberImpl) Struct(val any) error {
189196
return nil
190197
}
191198

199+
func (s *scrubberImpl) deepCopyStruct(fieldName string, src reflect.Value, scrubTag string, skipScrub bool) reflect.Value {
200+
if src.Kind() == reflect.Ptr && src.IsNil() {
201+
return reflect.New(src.Type()).Elem()
202+
}
203+
204+
if src.CanInterface() {
205+
value := src.Interface()
206+
if _, ok := value.(TrustedValue); ok {
207+
skipScrub = true
208+
}
209+
}
210+
211+
if src.Kind() == reflect.String && !skipScrub {
212+
dst := reflect.New(src.Type())
213+
var (
214+
setExplicitValue bool
215+
explicitValue string
216+
)
217+
switch scrubTag {
218+
case "ignore":
219+
dst.Elem().SetString(src.String())
220+
if !dst.CanInterface() {
221+
return dst
222+
}
223+
return dst.Elem()
224+
case "hash":
225+
setExplicitValue = true
226+
explicitValue = SanitiseHash(src.String())
227+
case "redact":
228+
setExplicitValue = true
229+
explicitValue = SanitiseRedact(src.String())
230+
}
231+
232+
if setExplicitValue {
233+
dst.Elem().SetString(explicitValue)
234+
} else {
235+
sanitisatiser := s.getSanitisatiser(fieldName)
236+
if sanitisatiser != nil {
237+
dst.Elem().SetString(sanitisatiser(src.String()))
238+
} else {
239+
dst.Elem().SetString(s.Value(src.String()))
240+
}
241+
}
242+
if !dst.CanInterface() {
243+
return dst
244+
}
245+
return dst.Elem()
246+
}
247+
248+
switch src.Kind() {
249+
case reflect.Struct:
250+
dst := reflect.New(src.Type())
251+
t := src.Type()
252+
253+
for i := 0; i < t.NumField(); i++ {
254+
f := t.Field(i)
255+
srcValue := src.Field(i)
256+
dstValue := dst.Elem().Field(i)
257+
258+
if !srcValue.CanInterface() {
259+
dstValue = reflect.NewAt(dstValue.Type(), unsafe.Pointer(dstValue.UnsafeAddr())).Elem()
260+
261+
if !srcValue.CanAddr() {
262+
switch {
263+
case srcValue.CanInt():
264+
dstValue.SetInt(srcValue.Int())
265+
case srcValue.CanUint():
266+
dstValue.SetUint(srcValue.Uint())
267+
case srcValue.CanFloat():
268+
dstValue.SetFloat(srcValue.Float())
269+
case srcValue.CanComplex():
270+
dstValue.SetComplex(srcValue.Complex())
271+
case srcValue.Kind() == reflect.Bool:
272+
dstValue.SetBool(srcValue.Bool())
273+
}
274+
275+
continue
276+
}
277+
278+
srcValue = reflect.NewAt(srcValue.Type(), unsafe.Pointer(srcValue.UnsafeAddr())).Elem()
279+
}
280+
281+
tagValue := f.Tag.Get("scrub")
282+
copied := s.deepCopyStruct(f.Name, srcValue, tagValue, skipScrub)
283+
dstValue.Set(copied)
284+
}
285+
return dst.Elem()
286+
287+
case reflect.Map:
288+
dst := reflect.MakeMap(src.Type())
289+
keys := src.MapKeys()
290+
for i := 0; i < src.Len(); i++ {
291+
mValue := src.MapIndex(keys[i])
292+
dst.SetMapIndex(keys[i], s.deepCopyStruct(keys[i].String(), mValue, "", skipScrub))
293+
}
294+
return dst
295+
296+
case reflect.Slice:
297+
dst := reflect.MakeSlice(src.Type(), src.Len(), src.Cap())
298+
for i := 0; i < src.Len(); i++ {
299+
dst.Index(i).Set(s.deepCopyStruct(fieldName, src.Index(i), "", skipScrub))
300+
}
301+
return dst
302+
303+
case reflect.Array:
304+
if src.Len() == 0 {
305+
return src
306+
}
307+
308+
dst := reflect.New(src.Type()).Elem()
309+
for i := 0; i < src.Len(); i++ {
310+
dst.Index(i).Set(s.deepCopyStruct(fieldName, src.Index(i), "", skipScrub))
311+
}
312+
return dst
313+
314+
case reflect.Interface:
315+
dst := reflect.New(src.Elem().Type())
316+
copied := s.deepCopyStruct(fieldName, src.Elem(), scrubTag, skipScrub)
317+
dst.Elem().Set(copied)
318+
return dst.Elem()
319+
320+
case reflect.Ptr:
321+
dst := reflect.New(src.Elem().Type())
322+
copied := s.deepCopyStruct(fieldName, src.Elem(), scrubTag, skipScrub)
323+
dst.Elem().Set(copied)
324+
return dst
325+
326+
default:
327+
dst := reflect.New(src.Type())
328+
dst.Elem().Set(src)
329+
return dst.Elem()
330+
}
331+
}
332+
333+
// Struct implements Scrubber
334+
func (s *scrubberImpl) DeepCopyStruct(val any) any {
335+
return s.deepCopyStruct("", reflect.ValueOf(val), "", false).Interface()
336+
}
337+
192338
func (s *scrubberImpl) scrubJsonObject(val map[string]interface{}) error {
193339
// fix https://github.com/gitpod-io/security/issues/64
194340
name, _ := val["name"].(string)

‎components/scrubber/scrubber_test.go‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package scrubber
66

77
import (
8+
"encoding/json"
89
"math/rand"
910
"testing"
1011

@@ -68,6 +69,10 @@ type TrustedStructToTest struct {
6869
StructToTest
6970
}
7071

72+
type TestWrap struct {
73+
Test *StructToTest
74+
}
75+
7176
type UnexportedStructToTest struct {
7277
Exported string
7378
unexportedPtr *string
@@ -293,6 +298,171 @@ func TestJSON(t *testing.T) {
293298
}
294299
}
295300

301+
func TestDeepCopyStruct(t *testing.T) {
302+
type Expectation struct {
303+
Error string
304+
Result any
305+
}
306+
tests := []struct {
307+
Name string
308+
Struct any
309+
Expectation Expectation
310+
CmpOpts []cmp.Option
311+
}{
312+
{
313+
Name: "basic happy path",
314+
Struct: &struct {
315+
Username string
316+
Email string
317+
Password string
318+
WorkspaceID string
319+
LeaveMeAlone string
320+
}{Username: "foo", Email: "foo@bar.com", Password: "foobar", WorkspaceID: "gitpodio-gitpod-uesaddev73c", LeaveMeAlone: "foo"},
321+
Expectation: Expectation{
322+
Result: &struct {
323+
Username string
324+
Email string
325+
Password string
326+
WorkspaceID string
327+
LeaveMeAlone string
328+
}{Username: "[redacted:md5:acbd18db4cc2f85cedef654fccc4a4d8]", Email: "[redacted]", Password: "[redacted]", WorkspaceID: "[redacted:md5:a35538939333def8477b5c19ac694b35]", LeaveMeAlone: "foo"},
329+
},
330+
},
331+
{
332+
Name: "stuct without pointer",
333+
Struct: struct {
334+
Username string
335+
Email string
336+
Password string
337+
WorkspaceID string
338+
LeaveMeAlone string
339+
}{Username: "foo", Email: "foo@bar.com", Password: "foobar", WorkspaceID: "gitpodio-gitpod-uesaddev73c", LeaveMeAlone: "foo"},
340+
Expectation: Expectation{
341+
Result: struct {
342+
Username string
343+
Email string
344+
Password string
345+
WorkspaceID string
346+
LeaveMeAlone string
347+
}{Username: "[redacted:md5:acbd18db4cc2f85cedef654fccc4a4d8]", Email: "[redacted]", Password: "[redacted]", WorkspaceID: "[redacted:md5:a35538939333def8477b5c19ac694b35]", LeaveMeAlone: "foo"},
348+
},
349+
},
350+
{
351+
Name: "map field",
352+
Struct: &struct {
353+
WithMap map[string]interface{}
354+
}{
355+
WithMap: map[string]interface{}{
356+
"email": "foo@bar.com",
357+
},
358+
},
359+
Expectation: Expectation{
360+
Result: &struct{ WithMap map[string]any }{WithMap: map[string]any{"email": string("[redacted]")}},
361+
},
362+
},
363+
{
364+
Name: "slices",
365+
Struct: &struct {
366+
Slice []string
367+
}{Slice: []string{"foo", "bar", "foo@bar.com"}},
368+
Expectation: Expectation{
369+
Result: &struct {
370+
Slice []string
371+
}{Slice: []string{"foo", "bar", "[redacted:email]"}},
372+
},
373+
},
374+
{
375+
Name: "struct tags",
376+
Struct: &struct {
377+
Hashed string `scrub:"hash"`
378+
Redacted string `scrub:"redact"`
379+
Email string `scrub:"ignore"`
380+
}{
381+
Hashed: "foo",
382+
Redacted: "foo",
383+
Email: "foo",
384+
},
385+
Expectation: Expectation{
386+
Result: &struct {
387+
Hashed string `scrub:"hash"`
388+
Redacted string `scrub:"redact"`
389+
Email string `scrub:"ignore"`
390+
}{
391+
Hashed: "[redacted:md5:acbd18db4cc2f85cedef654fccc4a4d8]",
392+
Redacted: "[redacted]",
393+
Email: "foo",
394+
},
395+
},
396+
},
397+
{
398+
Name: "trusted struct",
399+
Struct: scrubStructToTest(&StructToTest{
400+
Username: "foo",
401+
Email: "foo@bar.com",
402+
Password: "foobar",
403+
}),
404+
Expectation: Expectation{
405+
Result: &TrustedStructToTest{
406+
StructToTest: StructToTest{
407+
Username: "foo",
408+
Email: "trusted:[redacted:email]",
409+
Password: "trusted:[redacted]",
410+
},
411+
},
412+
},
413+
},
414+
{
415+
Name: "trusted interface",
416+
Struct: scrubStructToTestAsTrustedValue(&StructToTest{
417+
Username: "foo",
418+
Email: "foo@bar.com",
419+
Password: "foobar",
420+
}),
421+
Expectation: Expectation{
422+
Result: &TrustedStructToTest{
423+
StructToTest: StructToTest{
424+
Username: "foo",
425+
Email: "trusted:[redacted:email]",
426+
Password: "trusted:[redacted]",
427+
},
428+
},
429+
},
430+
},
431+
{
432+
Name: "contains unexported pointers",
433+
Struct: UnexportedStructToTest{
434+
Exported: "foo",
435+
unexportedPtr: nil,
436+
},
437+
Expectation: Expectation{
438+
Result: UnexportedStructToTest{
439+
Exported: "foo",
440+
unexportedPtr: nil,
441+
},
442+
},
443+
CmpOpts: []cmp.Option{cmpopts.IgnoreUnexported(UnexportedStructToTest{})},
444+
},
445+
}
446+
447+
for _, test := range tests {
448+
t.Run(test.Name, func(t *testing.T) {
449+
var act Expectation
450+
b, _ := json.Marshal(test.Struct)
451+
452+
act.Result = Default.DeepCopyStruct(test.Struct)
453+
b2, _ := json.Marshal(test.Struct)
454+
455+
if diff := cmp.Diff(b, b2, test.CmpOpts...); diff != "" {
456+
t.Errorf("DeepCopyStruct for origin struct modified (-want +got):\n%s", diff)
457+
}
458+
459+
if diff := cmp.Diff(test.Expectation, act, test.CmpOpts...); diff != "" {
460+
t.Errorf("DeepCopyStruct() mismatch (-want +got):\n%s", diff)
461+
}
462+
})
463+
}
464+
}
465+
296466
func BenchmarkKeyValue(b *testing.B) {
297467
key := HashedFieldNames[rand.Intn(len(HashedFieldNames))]
298468

‎components/ws-manager-api/go/crd/v1/workspace_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func (ps PortSpec) Equal(other PortSpec) bool {
169169
// WorkspaceStatus defines the observed state of Workspace
170170
type WorkspaceStatus struct {
171171
PodStarts int `json:"podStarts"`
172-
URL string `json:"url,omitempty"`
172+
URL string `json:"url,omitempty" scrub:"redact"`
173173
OwnerToken string `json:"ownerToken,omitempty" scrub:"redact"`
174174

175175
// +kubebuilder:default=Unknown

0 commit comments

Comments
 (0)