From 66939494f2befda00bcacf8e70173bc0b8bb771d Mon Sep 17 00:00:00 2001
From: Jahziel Villasana-Espinoza <jahziel@fleetdm.com>
Date: Fri, 8 Dec 2023 10:20:31 -0500
Subject: [PATCH] fix: validate agent platform options (#15482)

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/` or
`orbit/changes/`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
---
 changes/14778-agent-option                    |  1 +
 server/fleet/agent_options.go                 |  5 ++
 server/fleet/agent_options_test.go            |  6 +++
 server/service/integration_enterprise_test.go | 54 ++++++++++++-------
 4 files changed, 46 insertions(+), 20 deletions(-)
 create mode 100644 changes/14778-agent-option

diff --git a/changes/14778-agent-option b/changes/14778-agent-option
new file mode 100644
index 0000000000..d4de9ec944
--- /dev/null
+++ b/changes/14778-agent-option
@@ -0,0 +1 @@
+- Fixes a validation bug that allowed the agent options `overrides.platform` field to be set to `null`.
\ No newline at end of file
diff --git a/server/fleet/agent_options.go b/server/fleet/agent_options.go
index cf085209cf..89bfff2a16 100644
--- a/server/fleet/agent_options.go
+++ b/server/fleet/agent_options.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 )
@@ -59,6 +60,10 @@ func ValidateJSONAgentOptions(ctx context.Context, ds Datastore, rawJSON json.Ra
 
 	for platform, platformOpts := range opts.Overrides.Platforms {
 		if len(platformOpts) > 0 {
+			if string(platformOpts) == "null" {
+				return errors.New("platforms cannot be null. To remove platform overrides omit overrides from agent options.")
+			}
+
 			if err := validateJSONAgentOptionsSet(platformOpts); err != nil {
 				return fmt.Errorf("%s platform config: %w", platform, err)
 			}
diff --git a/server/fleet/agent_options_test.go b/server/fleet/agent_options_test.go
index 522e956705..ddff3b1dd4 100644
--- a/server/fleet/agent_options_test.go
+++ b/server/fleet/agent_options_test.go
@@ -27,6 +27,12 @@ func TestValidateAgentOptions(t *testing.T) {
 			}
 		}}`, `unknown field "foo"`},
 
+		{"overrides.platform is null", `{"overrides": {
+			"platforms": {
+				"darwin": null
+			}
+		}}`, `platforms cannot be null. To remove platform overrides omit overrides from agent options.`},
+
 		{"extra top-level bytes", `{}true`, `extra bytes`},
 		{"extra config bytes", `{"config":{}true}`, `invalid character 't' after object`},
 		{"extra overrides bytes", `{"overrides":{}true}`, `invalid character 't' after object`},
diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go
index 963fb954b8..5d15d97125 100644
--- a/server/service/integration_enterprise_test.go
+++ b/server/service/integration_enterprise_test.go
@@ -979,6 +979,16 @@ func (s *integrationEnterpriseTestSuite) TestTeamEndpoints() {
 		"x": "y"
 	}`), http.StatusBadRequest, &tmResp)
 
+	// modify team agent options with invalid platform options
+	tmResp.Team = nil
+	s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(
+		`{"overrides": {
+			"platforms": {
+				"linux": null
+			}
+		}}`,
+	), http.StatusBadRequest, &tmResp)
+
 	// modify team agent options with invalid options, but force-apply them
 	tmResp.Team = nil
 	s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/teams/%d/agent_options", tm1ID), json.RawMessage(`{
@@ -6073,15 +6083,17 @@ func (s *integrationEnterpriseTestSuite) TestAllSoftwareTitles() {
 		// valid title
 		resp = getSoftwareTitleResponse{}
 		s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", fooTitle.ID), getSoftwareTitleRequest{}, http.StatusOK, &resp)
-		softwareTitlesMatch([]fleet.SoftwareTitle{{
-			Name:          "foo",
-			Source:        "homebrew",
-			VersionsCount: 2,
-			HostsCount:    2,
-			Versions: []fleet.SoftwareVersion{
-				{Version: "0.0.1", Vulnerabilities: nil, HostsCount: ptr.Uint(2)},
-				{Version: "0.0.3", Vulnerabilities: nil, HostsCount: ptr.Uint(1)},
-			}},
+		softwareTitlesMatch([]fleet.SoftwareTitle{
+			{
+				Name:          "foo",
+				Source:        "homebrew",
+				VersionsCount: 2,
+				HostsCount:    2,
+				Versions: []fleet.SoftwareVersion{
+					{Version: "0.0.1", Vulnerabilities: nil, HostsCount: ptr.Uint(2)},
+					{Version: "0.0.3", Vulnerabilities: nil, HostsCount: ptr.Uint(1)},
+				},
+			},
 		}, []fleet.SoftwareTitle{*resp.SoftwareTitle})
 
 		// find the ID of "bar"
@@ -6100,18 +6112,20 @@ func (s *integrationEnterpriseTestSuite) TestAllSoftwareTitles() {
 		// valid title with vulnerabilities
 		resp = getSoftwareTitleResponse{}
 		s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", barTitle.ID), getSoftwareTitleRequest{}, http.StatusOK, &resp)
-		softwareTitlesMatch([]fleet.SoftwareTitle{{
-			Name:          "bar",
-			Source:        "apps",
-			VersionsCount: 1,
-			HostsCount:    1,
-			Versions: []fleet.SoftwareVersion{
-				{
-					Version:         "0.0.4",
-					Vulnerabilities: &fleet.SliceString{"cve-123-123-132"},
-					HostsCount:      ptr.Uint(1),
+		softwareTitlesMatch([]fleet.SoftwareTitle{
+			{
+				Name:          "bar",
+				Source:        "apps",
+				VersionsCount: 1,
+				HostsCount:    1,
+				Versions: []fleet.SoftwareVersion{
+					{
+						Version:         "0.0.4",
+						Vulnerabilities: &fleet.SliceString{"cve-123-123-132"},
+						HostsCount:      ptr.Uint(1),
+					},
 				},
-			}},
+			},
 		}, []fleet.SoftwareTitle{*resp.SoftwareTitle})
 	})
 }