From 38811da1c0b2d87cbdbb06fd56ead3f442eeefb0 Mon Sep 17 00:00:00 2001 From: Jordan Montgomery Date: Fri, 16 May 2025 14:56:27 -0400 Subject: [PATCH] Hold off on policy queries until after setup experience (#29159) For #28205 During setup experience customers often install all or most of the software that would otherwise be installed based on the results of policy queries. If we run policy queries during setup experience we end up trying to install some software twice which, at best, leads to confusing activities listed for the host. With these changes we will not run policy queries on macOS hosts until after the host has exited setup experience, at which point we should be able to avoid duplicate installs # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated automated tests - [x] Manual QA for all new/changed functionality --- ...8205-skip-policies-during-setup-experience | 1 + server/service/osquery.go | 19 ++++ server/service/osquery_test.go | 100 ++++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 changes/28205-skip-policies-during-setup-experience diff --git a/changes/28205-skip-policies-during-setup-experience b/changes/28205-skip-policies-during-setup-experience new file mode 100644 index 0000000000..04b97983b4 --- /dev/null +++ b/changes/28205-skip-policies-during-setup-experience @@ -0,0 +1 @@ +Stopped policy automations from running on macOS hosts until after setup experience finishes so that Fleet doesn't attempt to install software twice diff --git a/server/service/osquery.go b/server/service/osquery.go index 5d077459fe..2a815e64bb 100644 --- a/server/service/osquery.go +++ b/server/service/osquery.go @@ -762,10 +762,29 @@ func (svc *Service) labelQueriesForHost(ctx context.Context, host *fleet.Host) ( return labelQueries, nil } +func (svc *Service) disablePoliciesDuringSetupExperience(ctx context.Context, host *fleet.Host) (bool, error) { + if host.Platform != string(fleet.MacOSPlatform) { + return false, nil + } + inSetupExperience, err := svc.ds.GetHostAwaitingConfiguration(ctx, host.UUID) + if err != nil && !fleet.IsNotFound(err) { + return false, ctxerr.Wrap(ctx, err, "check if host is in setup experience") + } + return inSetupExperience, nil +} + // policyQueriesForHost returns policy queries if it's the time to re-run policies on the given host. // It returns (nil, true, nil) if the interval is so that policies should be executed on the host, but there are no policies // assigned to such host. func (svc *Service) policyQueriesForHost(ctx context.Context, host *fleet.Host) (policyQueries map[string]string, noPoliciesForHost bool, err error) { + disablePolicies, err := svc.disablePoliciesDuringSetupExperience(ctx, host) + if err != nil { + return nil, false, ctxerr.Wrap(ctx, err, "check if host is in setup experience") + } + if disablePolicies { + level.Debug(svc.logger).Log("msg", "skipping policy queries for host in setup experience", "host_id", host.ID) + return nil, false, nil + } policyReportedAt := svc.task.GetHostPolicyReportedAt(ctx, host) if !svc.shouldUpdate(policyReportedAt, svc.config.Osquery.PolicyUpdateInterval, host.ID) && !host.RefetchRequested { return nil, false, nil diff --git a/server/service/osquery_test.go b/server/service/osquery_test.go index 4ec90e355f..5f69ca7f3a 100644 --- a/server/service/osquery_test.go +++ b/server/service/osquery_test.go @@ -1264,6 +1264,10 @@ func TestQueriesAndHostFeatures(t *testing.T) { return map[string]string{}, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } + lq := live_query_mock.New(t) lq.On("QueriesForHost", uint(1)).Return(map[string]string{}, nil) lq.On("QueriesForHost", uint(2)).Return(map[string]string{}, nil) @@ -1364,6 +1368,9 @@ func TestGetDistributedQueriesEmptyQuery(t *testing.T) { ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) { return map[string]string{"empty_policy_query": ""}, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } lq.On("QueriesForHost", uint(0)).Return(map[string]string{"empty_live_query": ""}, nil) @@ -1405,6 +1412,9 @@ func TestLabelQueries(t *testing.T) { EnableSoftwareInventory: true, }}, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) { return map[string]string{}, nil } @@ -1571,6 +1581,9 @@ func TestDetailQueriesWithEmptyStrings(t *testing.T) { ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) { return map[string]string{}, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } ds.HostLiteFunc = func(ctx context.Context, id uint) (*fleet.Host, error) { if id != 1 { return nil, errors.New("not found") @@ -1802,6 +1815,9 @@ func TestDetailQueries(t *testing.T) { } return host, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } // With a new host, we should get the detail queries (and accelerated // queries) @@ -3128,6 +3144,9 @@ func TestPolicyQueries(t *testing.T) { EnableSoftwareInventory: true, }}, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } lq.On("QueriesForHost", uint(0)).Return(map[string]string{}, nil) @@ -3291,6 +3310,81 @@ func TestPolicyQueries(t *testing.T) { noPolicyResults(queries) } +func TestPolicyQueriesDuringSetupExperience(t *testing.T) { + ds := new(mock.Store) + lq := live_query_mock.New(t) + svc, ctx := newTestService(t, ds, nil, lq) + + host := &fleet.Host{ + Platform: "darwin", + } + + ds.LabelQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) { + return map[string]string{}, nil + } + ds.HostLiteFunc = func(ctx context.Context, id uint) (*fleet.Host, error) { + return host, nil + } + ds.UpdateHostFunc = func(ctx context.Context, gotHost *fleet.Host) error { + host = gotHost + return nil + } + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{Features: fleet.Features{ + EnableHostUsers: true, + EnableSoftwareInventory: true, + }}, nil + } + + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostUUID string) (bool, error) { + return true, nil + } + + lq.On("QueriesForHost", uint(0)).Return(map[string]string{}, nil) + + ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) { + return map[string]string{"1": "select 1", "2": "select 42;"}, nil + } + + ctx = hostctx.NewContext(ctx, host) + + queries, discovery, _, err := svc.GetDistributedQueries(ctx) + require.NoError(t, err) + + // Should not return the 2 policy queries because we're in setup experience + require.Equal(t, len(expectedDetailQueriesForPlatform(host.Platform)), len(queries), distQueriesMapKeys(queries)) + verifyDiscovery(t, queries, discovery) + + checkPolicyResults := func(queries map[string]string, shouldHavePolicies bool) { + hasPolicy1, hasPolicy2 := false, false + for name := range queries { + if strings.HasPrefix(name, hostPolicyQueryPrefix) { + if name[len(hostPolicyQueryPrefix):] == "1" { + hasPolicy1 = true + } + if name[len(hostPolicyQueryPrefix):] == "2" { + hasPolicy2 = true + } + } + } + assert.Equal(t, hasPolicy1, shouldHavePolicies) + assert.Equal(t, hasPolicy2, shouldHavePolicies) + } + + // Make it appear the host is out of setup experience and ask again for policies + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostUUID string) (bool, error) { + return false, nil + } + + queries, discovery, _, err = svc.GetDistributedQueries(ctx) + require.NoError(t, err) + // Should now return the 2 additional policy queries because we're out of setup experience + assert.Equal(t, len(expectedDetailQueriesForPlatform(host.Platform))+2, len(queries), distQueriesMapKeys(queries)) + verifyDiscovery(t, queries, discovery) + + checkPolicyResults(queries, true) +} + func TestPolicyWebhooks(t *testing.T) { mockClock := clock.NewMockClock() ds := new(mock.Store) @@ -3320,6 +3414,9 @@ func TestPolicyWebhooks(t *testing.T) { host = gotHost return nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { return &fleet.AppConfig{ Features: fleet.Features{ @@ -3600,6 +3697,9 @@ func TestLiveQueriesFailing(t *testing.T) { ds.PolicyQueriesForHostFunc = func(ctx context.Context, host *fleet.Host) (map[string]string, error) { return map[string]string{}, nil } + ds.GetHostAwaitingConfigurationFunc = func(ctx context.Context, hostuuid string) (bool, error) { + return false, nil + } ctx = hostctx.NewContext(ctx, host)