james/desktop err filter

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
+}

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")
+}

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 {