From b5f67d4f5991813f2f5fff79314f11376213c65e Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 4 Jul 2022 16:07:00 +0100 Subject: [PATCH 1/2] use masktoken pkg for redacting token Signed-off-by: Somtochi Onyekwere --- go.mod | 1 + go.sum | 2 + internal/server/event_handlers.go | 27 +++----- internal/server/event_handlers_test.go | 87 -------------------------- 4 files changed, 11 insertions(+), 106 deletions(-) delete mode 100644 internal/server/event_handlers_test.go diff --git a/go.mod b/go.mod index 1b3b47123..b8420e246 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/containrrr/shoutrrr v0.6.0 github.com/fluxcd/notification-controller/api v0.24.0 github.com/fluxcd/pkg/apis/meta v0.14.2 + github.com/fluxcd/pkg/masktoken v0.0.1 github.com/fluxcd/pkg/runtime v0.16.2 github.com/fluxcd/pkg/ssa v0.16.1 github.com/getsentry/sentry-go v0.13.0 diff --git a/go.sum b/go.sum index 5de7e12ff..0533d7557 100644 --- a/go.sum +++ b/go.sum @@ -207,6 +207,8 @@ github.com/fluxcd/pkg/apis/acl v0.0.3 h1:Lw0ZHdpnO4G7Zy9KjrzwwBmDZQuy4qEjaU/RvA6 github.com/fluxcd/pkg/apis/acl v0.0.3/go.mod h1:XPts6lRJ9C9fIF9xVWofmQwftvhY25n1ps7W9xw0XLU= github.com/fluxcd/pkg/apis/meta v0.14.2 h1:/Hf7I/Vz01vv3m7Qx7DtQvrzAL1oVt0MJcLb/I1Y1HE= github.com/fluxcd/pkg/apis/meta v0.14.2/go.mod h1:ijZ61VG/8T3U17gj0aFL3fdtZL+mulD6V8VrLLUCAgM= +github.com/fluxcd/pkg/masktoken v0.0.1 h1:egWR/ibTzf4L3PxE8TauKO1srD1Ye/aalgQRQuKKRdU= +github.com/fluxcd/pkg/masktoken v0.0.1/go.mod h1:sQmMtX4s5RwdGlByJazzNasWFFgBdmtNcgeZcGBI72Y= github.com/fluxcd/pkg/runtime v0.16.2 h1:CexfMmJK+r12sHTvKWyAax0pcPomjd6VnaHXcxjUrRY= github.com/fluxcd/pkg/runtime v0.16.2/go.mod h1:OHSKsrO+T+Ym8WZRS2oidrnauWRARuE2nfm8ewevm7M= github.com/fluxcd/pkg/ssa v0.16.1 h1:hWXMtDhiAPRPHpHiQ5NzVjqIDhOfyzWmc2zA49Wxw7E= diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go index ddbe8ce51..40ab86d27 100644 --- a/internal/server/event_handlers.go +++ b/internal/server/event_handlers.go @@ -29,13 +29,13 @@ import ( "time" "github.com/fluxcd/pkg/runtime/conditions" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/yaml" + "github.com/fluxcd/pkg/masktoken" "github.com/fluxcd/pkg/runtime/events" "github.com/fluxcd/notification-controller/api/v1beta1" @@ -265,8 +265,13 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) go func(n notifier.Interface, e events.Event) { if err := n.Post(e); err != nil { - err = redactTokenFromError(err, token, s.logger) - + maskedErrStr, maskErr := masktoken.MaskTokenFromString(err.Error(), token) + if maskErr != nil { + err = maskErr + } else { + err = errors.New(maskedErrStr) + } + err := errors.New(maskedErrStr) s.logger.Error(err, "failed to send notification", "reconciler kind", event.InvolvedObject.Kind, "name", event.InvolvedObject.Name, @@ -319,22 +324,6 @@ func (s *EventServer) eventMatchesAlert(ctx context.Context, event *events.Event return false } -func redactTokenFromError(err error, token string, log logr.Logger) error { - if token == "" { - return err - } - - re, compileErr := regexp.Compile(fmt.Sprintf("%s*", regexp.QuoteMeta(token))) - if compileErr != nil { - newErrStr := fmt.Sprintf("error redacting token from error message: %s", compileErr) - return errors.New(newErrStr) - } - - redacted := re.ReplaceAllString(err.Error(), "*****") - - return errors.New(redacted) -} - // TODO: move the metadata filtering function to fluxcd/pkg/runtime/events // cleanupMetadata removes metadata entries which are not used for alerting func cleanupMetadata(event *events.Event) { diff --git a/internal/server/event_handlers_test.go b/internal/server/event_handlers_test.go deleted file mode 100644 index 52291f077..000000000 --- a/internal/server/event_handlers_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package server - -import ( - "errors" - "strings" - "testing" - - "github.com/fluxcd/pkg/runtime/logger" -) - -func TestRedactTokenFromError(t *testing.T) { - tests := []struct { - name string - token string - originalErrStr string - expectedErrStr string - }{ - { - name: "no token", - token: "8h0387hdyehbwwa45", - originalErrStr: "Cannot post to github", - expectedErrStr: "Cannot post to github", - }, - { - name: "empty token", - token: "", - originalErrStr: "Cannot post to github", - expectedErrStr: "Cannot post to github", - }, - { - name: "exact token", - token: "8h0387hdyehbwwa45", - originalErrStr: "Cannot post to github with token 8h0387hdyehbwwa45", - expectedErrStr: "Cannot post to github with token *****", - }, - { - name: "non-exact token", - token: "8h0387hdyehbwwa45", - originalErrStr: `Cannot post to github with token 8h0387hdyehbwwa45\\n`, - expectedErrStr: `Cannot post to github with token *****\\n`, - }, - { - name: "extra text in front token", - token: "8h0387hdyehbwwa45", - originalErrStr: `Cannot post to github with token metoo8h0387hdyehbwwa45\\n`, - expectedErrStr: `Cannot post to github with token metoo*****\\n`, - }, - { - name: "extra text in front token", - token: "8h0387hdyehbwwa45踙", - originalErrStr: `Cannot post to github with token metoo8h0387hdyehbwwa45踙\\n`, - expectedErrStr: `Cannot post to github with token metoo*****\\n`, - }, - { - name: "return error on invalid UTF-8 string", - token: "\x18\xd0\xfa\xab\xb2\x93\xbb;\xc0l\xf4\xdc", - originalErrStr: `Cannot post to github with token \x18\xd0\xfa\xab\xb2\x93\xbb;\xc0l\xf4\xdc\\n`, - expectedErrStr: `error redacting token from error message`, - }, - { - name: "unescaped token", - token: "8h0387hdyehbwwa45\\", - originalErrStr: `Cannot post to github with token metoo8h0387hdyehbwwa45\\\n`, - expectedErrStr: `Cannot post to github with token metoo*****n`, - }, - { - name: "invalid chars", - token: "8h0387hdyehbwwa45(?!\\/)", - originalErrStr: `Cannot post to github`, - expectedErrStr: `Cannot post to github`, - }, - } - - for _, tt := range tests { - log := logger.NewLogger(logger.Options{}) - err := redactTokenFromError(errors.New(tt.originalErrStr), tt.token, log) - if err == nil { - t.Fatalf("error shouldn't be nil") - } - - if !strings.Contains(err.Error(), tt.expectedErrStr) { - t.Errorf("expected error string '%s' but got '%s'", - tt.expectedErrStr, err) - } - } - -} From 9b8586e35d91bf7a373686329a90104670cee7f4 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 4 Jul 2022 16:31:50 +0100 Subject: [PATCH 2/2] remove line overriding err Signed-off-by: Somtochi Onyekwere --- internal/server/event_handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go index 40ab86d27..ecb8cc817 100644 --- a/internal/server/event_handlers.go +++ b/internal/server/event_handlers.go @@ -44,6 +44,7 @@ import ( func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { + r.Context() body, err := io.ReadAll(r.Body) if err != nil { s.logger.Error(err, "reading the request body failed") @@ -271,7 +272,6 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) } else { err = errors.New(maskedErrStr) } - err := errors.New(maskedErrStr) s.logger.Error(err, "failed to send notification", "reconciler kind", event.InvolvedObject.Kind, "name", event.InvolvedObject.Name,