From e93b6d73e586fa4515e4add5b2cb8c347bd2ef04 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 23 Jan 2023 17:56:19 +0200 Subject: [PATCH] Sanitize log entries While most of these log entries come from either github or our own database, it's still a good idea to sanitize them. --- apiserver/controllers/controllers.go | 3 ++- cmd/garm-cli/cmd/log.go | 3 ++- runner/pool/pool.go | 13 +++++++------ runner/runner.go | 4 ++-- util/util.go | 4 ++++ 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/apiserver/controllers/controllers.go b/apiserver/controllers/controllers.go index be27d92b..1c2905c0 100644 --- a/apiserver/controllers/controllers.go +++ b/apiserver/controllers/controllers.go @@ -25,6 +25,7 @@ import ( gErrors "garm/errors" runnerParams "garm/params" "garm/runner" + "garm/util" wsWriter "garm/websocket" "github.com/gorilla/websocket" @@ -113,7 +114,7 @@ func (a *APIController) CatchAll(w http.ResponseWriter, r *http.Request) { case runnerParams.WorkflowJobEvent: a.handleWorkflowJobEvent(w, r) default: - log.Printf("ignoring unknown event %s", event) + log.Printf("ignoring unknown event %s", util.SanitizeLogEntry(string(event))) return } } diff --git a/cmd/garm-cli/cmd/log.go b/cmd/garm-cli/cmd/log.go index 9ddb3087..44b96428 100644 --- a/cmd/garm-cli/cmd/log.go +++ b/cmd/garm-cli/cmd/log.go @@ -11,6 +11,7 @@ import ( "time" apiParams "garm/apiserver/params" + "garm/util" "github.com/gorilla/websocket" "github.com/spf13/cobra" @@ -61,7 +62,7 @@ var logCmd = &cobra.Command{ log.Printf("read: %q", err) return } - log.Print(string(message)) + log.Print(util.SanitizeLogEntry(string(message))) } }() diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 2e0bbbdd..9670e1d2 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -30,6 +30,7 @@ import ( "garm/params" "garm/runner/common" providerCommon "garm/runner/providers/common" + "garm/util" "github.com/google/go-github/v48/github" "github.com/google/uuid" @@ -365,7 +366,7 @@ func (r *basePoolManager) acquireNewInstance(job params.WorkflowJob) error { } return errors.Wrap(err, "fetching suitable pool") } - log.Printf("adding new runner with requested tags %s in pool %s", strings.Join(job.WorkflowJob.Labels, ", "), pool.ID) + log.Printf("adding new runner with requested tags %s in pool %s", util.SanitizeLogEntry(strings.Join(job.WorkflowJob.Labels, ", ")), util.SanitizeLogEntry(pool.ID)) if !pool.Enabled { log.Printf("selected pool (%s) is disabled", pool.ID) @@ -658,7 +659,7 @@ func (r *basePoolManager) getRunnerDetailsFromJob(job params.WorkflowJob) (param runnerDetails, err := r.store.GetInstanceByName(context.Background(), runnerInfo.Name) if err != nil { - log.Printf("could not find runner details for %s", runnerInfo.Name) + log.Printf("could not find runner details for %s", util.SanitizeLogEntry(runnerInfo.Name)) return params.RunnerInfo{}, errors.Wrap(err, "fetching runner details") } @@ -701,15 +702,15 @@ func (r *basePoolManager) HandleWorkflowJob(job params.WorkflowJob) error { if errors.Is(err, runnerErrors.ErrNotFound) { return nil } - log.Printf("failed to update runner %s status", runnerInfo.Name) + log.Printf("failed to update runner %s status", util.SanitizeLogEntry(runnerInfo.Name)) return errors.Wrap(err, "updating runner") } - log.Printf("marking instance %s as pending_delete", runnerInfo.Name) + log.Printf("marking instance %s as pending_delete", util.SanitizeLogEntry(runnerInfo.Name)) if err := r.setInstanceStatus(runnerInfo.Name, providerCommon.InstancePendingDelete, nil); err != nil { if errors.Is(err, runnerErrors.ErrNotFound) { return nil } - log.Printf("failed to update runner %s status", runnerInfo.Name) + log.Printf("failed to update runner %s status", util.SanitizeLogEntry(runnerInfo.Name)) return errors.Wrap(err, "updating runner") } case "in_progress": @@ -733,7 +734,7 @@ func (r *basePoolManager) HandleWorkflowJob(job params.WorkflowJob) error { if errors.Is(err, runnerErrors.ErrNotFound) { return nil } - log.Printf("failed to update runner %s status", runnerInfo.Name) + log.Printf("failed to update runner %s status", util.SanitizeLogEntry(runnerInfo.Name)) return errors.Wrap(err, "updating runner") } } diff --git a/runner/runner.go b/runner/runner.go index 8218656d..098bdec9 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -599,10 +599,10 @@ func (r *Runner) DispatchWorkflowJob(hookTargetType, signature string, jobData [ switch HookTargetType(hookTargetType) { case RepoHook: - log.Printf("got hook for repo %s/%s", job.Repository.Owner.Login, job.Repository.Name) + log.Printf("got hook for repo %s/%s", util.SanitizeLogEntry(job.Repository.Owner.Login), util.SanitizeLogEntry(job.Repository.Name)) poolManager, err = r.findRepoPoolManager(job.Repository.Owner.Login, job.Repository.Name) case OrganizationHook: - log.Printf("got hook for org %s", job.Organization.Login) + log.Printf("got hook for org %s", util.SanitizeLogEntry(job.Organization.Login)) poolManager, err = r.findOrgPoolManager(job.Organization.Login) case EnterpriseHook: poolManager, err = r.findEnterprisePoolManager(job.Enterprise.Slug) diff --git a/util/util.go b/util/util.go index 4834be32..e7129929 100644 --- a/util/util.go +++ b/util/util.go @@ -327,3 +327,7 @@ func NewLoggingMiddleware(writer io.Writer) func(http.Handler) http.Handler { return gorillaHandlers.CombinedLoggingHandler(writer, next) } } + +func SanitizeLogEntry(entry string) string { + return strings.Replace(strings.Replace(entry, "\n", "", -1), "\r", "", -1) +}