diff --git a/ee/desktop/runner/err_filter.go b/ee/desktop/runner/err_filter.go new file mode 100644 index 0000000000..18175837fd --- /dev/null +++ b/ee/desktop/runner/err_filter.go @@ -0,0 +1,20 @@ +package runner + +import ( + "log/slog" + "strings" +) + +type errFilter struct { + matchStrings []string +} + +func (e *errFilter) filter(err error) slog.Level { + for _, matchString := range e.matchStrings { + if strings.Contains(strings.ToLower(err.Error()), strings.ToLower(matchString)) { + return slog.LevelWarn + } + } + + return slog.LevelError +} diff --git a/ee/desktop/runner/err_filter_test.go b/ee/desktop/runner/err_filter_test.go new file mode 100644 index 0000000000..28adc40210 --- /dev/null +++ b/ee/desktop/runner/err_filter_test.go @@ -0,0 +1,121 @@ +package runner + +import ( + "errors" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestErrFilter_filter(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + matchStrings []string + err error + expected slog.Level + }{ + { + name: "error matches first match string", + matchStrings: []string{"signal: killed", "no mapping"}, + err: errors.New("process received signal: killed"), + expected: slog.LevelWarn, + }, + { + name: "error matches second match string", + matchStrings: []string{"signal: killed", "no mapping"}, + err: errors.New("no mapping between account names"), + expected: slog.LevelWarn, + }, + { + name: "error does not match any match string", + matchStrings: []string{"signal: killed", "no mapping"}, + err: errors.New("some other error occurred"), + expected: slog.LevelError, + }, + { + name: "case insensitive matching - lowercase error", + matchStrings: []string{"SIGNAL: KILLED"}, + err: errors.New("process received signal: killed"), + expected: slog.LevelWarn, + }, + { + name: "case insensitive matching - uppercase error", + matchStrings: []string{"signal: killed"}, + err: errors.New("process received SIGNAL: KILLED"), + expected: slog.LevelWarn, + }, + { + name: "substring matching", + matchStrings: []string{"signal: killed"}, + err: errors.New("error: process received signal: killed by system"), + expected: slog.LevelWarn, + }, + { + name: "empty match strings", + matchStrings: []string{}, + err: errors.New("any error"), + expected: slog.LevelError, + }, + { + name: "nil match strings", + matchStrings: nil, + err: errors.New("any error"), + expected: slog.LevelError, + }, + { + name: "multiple match strings - first matches", + matchStrings: []string{"signal: killed", "no mapping", "insufficient resources"}, + err: errors.New("process received signal: killed"), + expected: slog.LevelWarn, + }, + { + name: "multiple match strings - last matches", + matchStrings: []string{"signal: killed", "no mapping", "insufficient resources"}, + err: errors.New("insufficient resources available"), + expected: slog.LevelWarn, + }, + { + name: "error with empty message", + matchStrings: []string{"signal: killed"}, + err: errors.New(""), + expected: slog.LevelError, + }, + { + name: "real world example - no mapping error", + matchStrings: []string{"no mapping between account names and security ids was done"}, + err: errors.New("no mapping between account names and security ids was done"), + expected: slog.LevelWarn, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + filter := &errFilter{ + matchStrings: tt.matchStrings, + } + + result := filter.filter(tt.err) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestErrFilter_filter_nilError(t *testing.T) { + t.Parallel() + + filter := &errFilter{ + matchStrings: []string{"signal: killed"}, + } + + // This should panic or handle nil gracefully - let's test what happens + require.Panics(t, func() { + filter.filter(nil) + }, "filter should panic on nil error") +} diff --git a/ee/desktop/runner/runner.go b/ee/desktop/runner/runner.go index 2842067051..617464050e 100644 --- a/ee/desktop/runner/runner.go +++ b/ee/desktop/runner/runner.go @@ -151,6 +151,8 @@ type DesktopUsersProcessesRunner struct { osVersion string // cachedMenuData is the cached label values of the currently displayed menu data, used for detecting changes cachedMenuData *menuItemCache + // runDesktopProcessErrFilter is used to filter errors from running desktop processes + runDesktopProcessErrFilter errFilter } // processRecord is used to track spawned desktop processes. @@ -188,6 +190,17 @@ func New(k types.Knapsack, messenger runnerserver.Messenger, opts ...desktopUser usersFilesRoot: agent.TempPath("kolide-desktop"), knapsack: k, cachedMenuData: newMenuItemCache(), + // these are common errors that we have no control over, they will be logged as warnings + runDesktopProcessErrFilter: errFilter{ + matchStrings: []string{ + "no mapping between account names and security ids was done", + "signal: killed", + "insufficient system resources exist to complete the requested service", + "no explorer process found", + "a specified logon session does not exist", + "this program is blocked by group policy", + }, + }, } runner.slogger = k.Slogger().With("component", "desktop_runner") @@ -251,18 +264,10 @@ func (r *DesktopUsersProcessesRunner) Execute() error { for { // Check immediately on each iteration, avoiding the initial ticker delay if err := r.runConsoleUserDesktop(); err != nil { - - if errors.Is(err, NoExplorerProcessError{}) { - r.slogger.Log(context.TODO(), slog.LevelDebug, - "no explorer proc, user may not have desktop session", - "err", err, - ) - } else { - r.slogger.Log(context.TODO(), slog.LevelError, - "could not run console user desktop process", - "err", err, - ) - } + r.slogger.Log(context.TODO(), r.runDesktopProcessErrFilter.filter(err), + "could not run console user desktop process", + "err", err, + ) } select {