From 52caba768c27de2ebcacf14369cbbcf25531c74a Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Wed, 29 Apr 2026 10:43:39 -0300 Subject: [PATCH] Fix filtering in /api/v1/fleet/labels/:id/hosts endpoint (#44293) - [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/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **Bug Fixes** * Fixed filtering in the /api/v1/fleet/labels/:id/hosts endpoint and tightened validation to reject invalid sort/order keys with HTTP 422 responses. * Enforced ordering restrictions tied to feature flags (issues and device-mapping), rejecting unsupported order_key values. * **Tests** * Added extensive integration tests for order_key validation, deterministic sorting across allowed keys, and cursor pagination. * **Documentation** * Added a changelog entry noting the hosts-in-label filtering fix. --- changes/fix-hosts-in-label-filtering | 1 + server/datastore/mysql/labels.go | 81 ++++++- server/service/integration_core_test.go | 300 ++++++++++++++++++++++++ server/service/transport.go | 41 +++- server/service/transport_test.go | 37 +++ 5 files changed, 451 insertions(+), 9 deletions(-) create mode 100644 changes/fix-hosts-in-label-filtering diff --git a/changes/fix-hosts-in-label-filtering b/changes/fix-hosts-in-label-filtering new file mode 100644 index 0000000000..389ad0d7d2 --- /dev/null +++ b/changes/fix-hosts-in-label-filtering @@ -0,0 +1 @@ +* Fixed filtering in `/api/v1/fleet/labels/:id/hosts` endpoint. diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index ebd63c33d7..11f6f41075 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -14,6 +14,7 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/jmoiron/sqlx" ) @@ -1009,6 +1010,80 @@ func (ds *Datastore) ListLabelsForHost(ctx context.Context, hid uint) ([]*fleet. return labels, nil } +// hostsInLabelAllowedOrderKeys defines the allowed order keys for the hosts in label endpoint. +// SECURITY: This prevents information disclosure via arbitrary column sorting. +// Sensitive columns like 'node_key' and 'orbit_node_key'. +// are intentionally excluded to prevent binary search extraction attacks. +var hostsInLabelAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "h.id", + "osquery_host_id": "h.osquery_host_id", + "created_at": "h.created_at", + "updated_at": "h.updated_at", + "detail_updated_at": "h.detail_updated_at", + "hostname": "h.hostname", + "uuid": "h.uuid", + "platform": "h.platform", + "osquery_version": "h.osquery_version", + "os_version": "h.os_version", + "build": "h.build", + "platform_like": "h.platform_like", + "code_name": "h.code_name", + "uptime": "h.uptime", + "memory": "h.memory", + "cpu_type": "h.cpu_type", + "cpu_subtype": "h.cpu_subtype", + "cpu_brand": "h.cpu_brand", + "cpu_physical_cores": "h.cpu_physical_cores", + "cpu_logical_cores": "h.cpu_logical_cores", + "hardware_vendor": "h.hardware_vendor", + "hardware_model": "h.hardware_model", + "hardware_version": "h.hardware_version", + "hardware_serial": "h.hardware_serial", + "computer_name": "h.computer_name", + "primary_ip_id": "h.primary_ip_id", + "distributed_interval": "h.distributed_interval", + "logger_tls_period": "h.logger_tls_period", + "config_tls_refresh": "h.config_tls_refresh", + "primary_ip": "h.primary_ip", + "primary_mac": "h.primary_mac", + "label_updated_at": "h.label_updated_at", + "last_enrolled_at": "h.last_enrolled_at", + "refetch_requested": "h.refetch_requested", + "refetch_critical_queries_until": "h.refetch_critical_queries_until", + "team_id": "h.team_id", + "policy_updated_at": "h.policy_updated_at", + "public_ip": "h.public_ip", + + "display_name": "hdn.display_name", + + // COALESCE required on the following: + // must match SELECT clause so cursor pagination (WHERE) and ORDER BY are consistent + "gigs_disk_space_available": "COALESCE(hd.gigs_disk_space_available, 0)", + "percent_disk_space_available": "COALESCE(hd.percent_disk_space_available, 0)", + "gigs_total_disk_space": "COALESCE(hd.gigs_total_disk_space, 0)", + "seen_time": "COALESCE(hst.seen_time, h.created_at)", + "software_updated_at": "COALESCE(hu.software_updated_at, h.created_at)", + + "last_restarted_at": "h.last_restarted_at", + "timezone": "h.timezone", + // must match SELECT clause subquery so cursor pagination (WHERE) and + // ORDER BY are consistent — MySQL disallows SELECT aliases in WHERE. + "team_name": "(SELECT name FROM teams t WHERE t.id = h.team_id)", + + // COALESCE required on the following: + // must match SELECT clause so cursor pagination (WHERE) and ORDER BY are consistent + "failing_policies_count": "COALESCE(host_issues.failing_policies_count, 0)", + "critical_vulnerabilities_count": "COALESCE(host_issues.critical_vulnerabilities_count, 0)", + "total_issues_count": "COALESCE(host_issues.total_issues_count, 0)", + "device_mapping": "COALESCE(dm.device_mapping, 'null')", + + "issues": "COALESCE(host_issues.total_issues_count, 0)", + + // + // SECURITY: + // Note: 'h.node_key', 'h.orbit_node_key' intentionally EXCLUDED +} + // ListHostsInLabel returns a list of fleet.Host that are associated // with fleet.Label referenced by Label ID func (ds *Datastore) ListHostsInLabel(ctx context.Context, filter fleet.TeamFilter, lid uint, opt fleet.HostListOptions) ([]*fleet.Host, error) { @@ -1244,10 +1319,10 @@ func (ds *Datastore) applyHostLabelFilters(ctx context.Context, filter fleet.Tea // TODO: should search columns include display_name (requires join to host_display_names)? query, whereParams = hostSearchLike(query, whereParams, opt.MatchQuery, hostSearchColumns...) - if opt.ListOptions.OrderKey == "issues" { - opt.ListOptions.OrderKey = "host_issues.total_issues_count" + query, whereParams, err = appendListOptionsWithCursorToSQLSecure(query, whereParams, &opt.ListOptions, hostsInLabelAllowedOrderKeys) + if err != nil { + return "", nil, ctxerr.Wrap(ctx, err, "apply host list options") } - query, whereParams = appendListOptionsWithCursorToSQL(query, whereParams, &opt.ListOptions) return query, append(joinParams, whereParams...), nil } diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 0f279dc469..46eaa391b8 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -5268,6 +5268,81 @@ func (s *integrationTestSuite) TestLabels() { assert.Equal(t, fleet.WellKnownMDMSimpleMDM, listHostsResp.MDMSolution.Name) assert.Equal(t, "https://simplemdm.com", listHostsResp.MDMSolution.ServerURL) + // invalid order_key returns 422 (sensitive columns must not be sortable to prevent + // information disclosure via binary search extraction). + for _, key := range []string{ + "node_key", "h.node_key", + "orbit_node_key", "h.orbit_node_key", + "invalid_column", "h.invalid_column", + // computer_name is a valid field, but must not contain the table alias + // (previous version of the endpoint allowed setting aliases here). + "h.computer_name", + } { + res := s.Do("GET", fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", lbl2.ID), nil, http.StatusUnprocessableEntity, "order_key", key) + errMsg := extractServerErrorText(res.Body) + assert.Contains(t, errMsg, "invalid order_key") + assert.Contains(t, errMsg, key) + } + + // all allowed order_key values must be accepted by the endpoint and return the + // hosts in lbl2. + allowedOrderKeys := []string{ + "id", "osquery_host_id", "created_at", "updated_at", "detail_updated_at", + "hostname", "uuid", "platform", "osquery_version", "os_version", "build", + "platform_like", "code_name", "uptime", "memory", "cpu_type", "cpu_subtype", + "cpu_brand", "cpu_physical_cores", "cpu_logical_cores", + "hardware_vendor", "hardware_model", "hardware_version", "hardware_serial", + "computer_name", "primary_ip_id", "distributed_interval", "logger_tls_period", + "config_tls_refresh", "primary_ip", "primary_mac", "label_updated_at", + "last_enrolled_at", "refetch_requested", "refetch_critical_queries_until", + "team_id", "policy_updated_at", "public_ip", + "gigs_disk_space_available", "percent_disk_space_available", + "gigs_total_disk_space", "seen_time", "software_updated_at", + "last_restarted_at", "timezone", "team_name", + "failing_policies_count", "critical_vulnerabilities_count", + "total_issues_count", + "issues", // supported as alias for "total_issues_count" + "device_mapping", + "display_name", + } + for _, key := range allowedOrderKeys { + listHostsResp = listHostsResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", lbl2.ID), nil, http.StatusOK, &listHostsResp, "order_key", key, "device_mapping", "true") + assert.Len(t, listHostsResp.Hosts, len(lbl2Hosts), "order_key=%s", key) + } + + // every allowed order_key must also accept the `after` cursor without + // erroring — guards against SELECT-list aliases leaking into the WHERE + // clause (MySQL disallows aliases in WHERE). + for _, key := range allowedOrderKeys { + listHostsResp = listHostsResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", lbl2.ID), nil, http.StatusOK, &listHostsResp, + "order_key", key, "order_direction", "asc", "after", "0", "device_mapping", "true") + } + + // issue-related order_keys are rejected when disable_issues=true. + for _, key := range []string{"issues", "failing_policies_count", "critical_vulnerabilities_count", "total_issues_count"} { + res := s.Do("GET", fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", lbl2.ID), nil, http.StatusBadRequest, + "order_key", key, "disable_issues", "true") + errMsg := extractServerErrorText(res.Body) + assert.Contains(t, errMsg, "Invalid order_key") + assert.Contains(t, errMsg, key) + } + + // device_mapping order_key is rejected when device_mapping is not enabled. + res = s.Do("GET", fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", lbl2.ID), nil, http.StatusBadRequest, + "order_key", "device_mapping") + errMsg = extractServerErrorText(res.Body) + assert.Contains(t, errMsg, "Invalid order_key") + assert.Contains(t, errMsg, "device_mapping") + + // device_mapping=false is also rejected. + res = s.Do("GET", fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", lbl2.ID), nil, http.StatusBadRequest, + "order_key", "device_mapping", "device_mapping", "false") + errMsg = extractServerErrorText(res.Body) + assert.Contains(t, errMsg, "Invalid order_key") + assert.Contains(t, errMsg, "device_mapping") + // delete a label by id var delIDResp fleet.DeleteLabelByIDResponse s.DoJSON("DELETE", fmt.Sprintf("/api/latest/fleet/labels/id/%d", lbl1.ID), nil, http.StatusOK, &delIDResp) @@ -5549,6 +5624,231 @@ func (s *integrationTestSuite) TestLabels() { }) }) }) + + t.Run("Sort by order_key", func(t *testing.T) { + ctx := context.Background() + + // Create three teams so each host has a distinct team_id and team_name. + // Names sort A < B < C and team IDs are assigned in creation order. + teamPrefix := strings.ReplaceAll(t.Name(), "/", "_") + teamA, err := s.ds.NewTeam(ctx, &fleet.Team{Name: teamPrefix + "-team-A"}) + require.NoError(t, err) + teamB, err := s.ds.NewTeam(ctx, &fleet.Team{Name: teamPrefix + "-team-B"}) + require.NoError(t, err) + teamC, err := s.ds.NewTeam(ctx, &fleet.Team{Name: teamPrefix + "-team-C"}) + require.NoError(t, err) + teamIDs := []*uint{&teamA.ID, &teamB.ID, &teamC.ID} + + // Each sortHost[i] is set up with field values that sort in the same direction + // as the index: sortHost[0] is "smallest", sortHost[2] is "largest", for every + // orderable field below. This means ASC order is always [h0, h1, h2] and DESC + // order is always [h2, h1, h0], which keeps the per-field test cases compact. + base := time.Date(2030, 1, 1, 0, 0, 0, 0, time.UTC) + platforms := []string{"darwin", "linux", "windows"} + sortHosts := make([]*fleet.Host, 3) + for i := range 3 { + h, err := s.ds.NewHost(ctx, &fleet.Host{ + OsqueryHostID: ptr.String(fmt.Sprintf("sort-osq-%d", i)), + NodeKey: ptr.String(fmt.Sprintf("sort-nk-%d", i)), + UUID: fmt.Sprintf("aaaaaaaa-0000-0000-0000-00000000000%d", i+1), + Hostname: fmt.Sprintf("sort-host-%d", i), + ComputerName: fmt.Sprintf("sort-comp-%d", i), + HardwareSerial: fmt.Sprintf("sort-ser-%d", i), + Platform: platforms[i], + PlatformLike: fmt.Sprintf("sort-plike-%d", i), + OsqueryVersion: fmt.Sprintf("%d.0.0", i+1), + OSVersion: fmt.Sprintf("OS-%d.0", i+1), + Uptime: time.Duration(i+1) * time.Hour, + Memory: int64(i+1) * 1024, + DistributedInterval: uint(i+1) * 10, + LoggerTLSPeriod: uint(i+1) * 100, + ConfigTLSRefresh: uint(i+1) * 5, + DetailUpdatedAt: base.Add(time.Duration(i+1) * time.Hour), + LabelUpdatedAt: base.Add(time.Duration(i+1) * 2 * time.Hour), + PolicyUpdatedAt: base.Add(time.Duration(i+1) * 3 * time.Hour), + RefetchCriticalQueriesUntil: ptr.Time(base.Add(time.Duration(i+1) * 4 * time.Hour)), + RefetchRequested: i == 2, // only the "largest" host has refetch_requested = true + TeamID: teamIDs[i], + }) + require.NoError(t, err) + sortHosts[i] = h + } + + // Set columns not handled by NewHost via direct UPDATEs. + for i, h := range sortHosts { + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext(ctx, ` + UPDATE hosts SET + created_at = ?, + updated_at = ?, + last_enrolled_at = ?, + last_restarted_at = ?, + primary_ip_id = ?, + primary_ip = ?, + primary_mac = ?, + public_ip = ?, + timezone = ?, + build = ?, + code_name = ?, + cpu_type = ?, + cpu_subtype = ?, + cpu_brand = ?, + cpu_physical_cores = ?, + cpu_logical_cores = ?, + hardware_vendor = ?, + hardware_model = ?, + hardware_version = ? + WHERE id = ?`, + base.Add(time.Duration(i+1)*5*time.Hour), + base.Add(time.Duration(i+1)*6*time.Hour), + base.Add(time.Duration(i+1)*7*time.Hour), + base.Add(time.Duration(i+1)*8*time.Hour), + uint(i+1)*100, //nolint:gosec // ignore G115 + fmt.Sprintf("10.0.0.%d", i+1), + fmt.Sprintf("aa:bb:cc:00:00:0%d", i+1), + fmt.Sprintf("8.0.0.%d", i+1), + fmt.Sprintf("UTC-%d", i), + fmt.Sprintf("sort-build-%d", i), + fmt.Sprintf("sort-code-%d", i), + fmt.Sprintf("sort-ct-%d", i), + fmt.Sprintf("sort-cs-%d", i), + fmt.Sprintf("sort-cb-%d", i), + i+1, + (i+1)*2, + fmt.Sprintf("sort-hv-%d", i), + fmt.Sprintf("sort-hm-%d", i), + fmt.Sprintf("sort-hver-%d", i), + h.ID, + ) + return err + }) + } + + // Populate joined tables (host_disks, host_seen_times, host_updates, + // host_issues, host_emails) with values that also sort by host index. + for i, h := range sortHosts { + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext(ctx, ` + INSERT INTO host_disks (host_id, gigs_disk_space_available, percent_disk_space_available, gigs_total_disk_space) + VALUES (?, ?, ?, ?)`, + h.ID, float64((i+1)*100), float64((i+1)*10), float64((i+1)*1000)) + return err + }) + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext(ctx, + `UPDATE host_seen_times SET seen_time = ? WHERE host_id = ?`, + base.Add(time.Duration(i+1)*9*time.Hour), h.ID) + return err + }) + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext(ctx, + `INSERT INTO host_updates (host_id, software_updated_at) VALUES (?, ?)`, + h.ID, base.Add(time.Duration(i+1)*10*time.Hour)) + return err + }) + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext(ctx, ` + INSERT INTO host_issues (host_id, failing_policies_count, critical_vulnerabilities_count, total_issues_count) + VALUES (?, ?, ?, ?)`, + h.ID, uint(i+1), uint(i+1)*2, uint(i+1)*3) //nolint:gosec // ignore G115 + return err + }) + mysql.ExecAdhocSQL(t, s.ds, func(db sqlx.ExtContext) error { + _, err := db.ExecContext(ctx, + `INSERT INTO host_emails (host_id, email, source) VALUES (?, ?, ?)`, + h.ID, fmt.Sprintf("sort-%d@example.com", i), "src") + return err + }) + } + + // Create a dynamic label and add the three hosts to it. + var createResp fleet.CreateLabelResponse + s.DoJSON("POST", "/api/latest/fleet/labels", + &fleet.LabelPayload{Name: teamPrefix + "-sort", Query: "select 1"}, http.StatusOK, &createResp) + sortLabel := createResp.Label.Label + for _, h := range sortHosts { + require.NoError(t, s.ds.RecordLabelQueryExecutions(ctx, h, + map[uint]*bool{sortLabel.ID: new(true)}, time.Now(), false)) + } + + ascIDs := []uint{sortHosts[0].ID, sortHosts[1].ID, sortHosts[2].ID} + descIDs := []uint{sortHosts[2].ID, sortHosts[1].ID, sortHosts[0].ID} + labelHostsURL := fmt.Sprintf("/api/latest/fleet/labels/%d/hosts", sortLabel.ID) + + // orderKeys lists every field for which sortHosts is set up to produce a + // strict, deterministic ASC ordering of [h0, h1, h2]. Each field gets its + // own subtest verifying both ASC and DESC orderings. + orderKeys := []string{ + "id", "osquery_host_id", "created_at", "updated_at", "detail_updated_at", + "hostname", "uuid", "platform", "osquery_version", "os_version", "build", + "platform_like", "code_name", "uptime", "memory", "cpu_type", "cpu_subtype", + "cpu_brand", "cpu_physical_cores", "cpu_logical_cores", + "hardware_vendor", "hardware_model", "hardware_version", "hardware_serial", + "computer_name", "primary_ip_id", "distributed_interval", "logger_tls_period", + "config_tls_refresh", "primary_ip", "primary_mac", "label_updated_at", + "last_enrolled_at", "refetch_critical_queries_until", "team_id", + "policy_updated_at", "public_ip", + "gigs_disk_space_available", "percent_disk_space_available", + "gigs_total_disk_space", "seen_time", "software_updated_at", + "last_restarted_at", "timezone", "team_name", + "failing_policies_count", "critical_vulnerabilities_count", + "total_issues_count", "issues", + "device_mapping", + "display_name", + } + for _, key := range orderKeys { + t.Run(key, func(t *testing.T) { + params := []string{"order_key", key, "order_direction", "asc"} + if key == "device_mapping" { + params = append(params, "device_mapping", "true") + } + + var resp listHostsResponse + s.DoJSON("GET", labelHostsURL, nil, http.StatusOK, &resp, params...) + require.Len(t, resp.Hosts, 3) + gotAsc := []uint{resp.Hosts[0].ID, resp.Hosts[1].ID, resp.Hosts[2].ID} + assert.Equal(t, ascIDs, gotAsc, "asc order mismatch") + + params[3] = "desc" + resp = listHostsResponse{} + s.DoJSON("GET", labelHostsURL, nil, http.StatusOK, &resp, params...) + require.Len(t, resp.Hosts, 3) + gotDesc := []uint{resp.Hosts[0].ID, resp.Hosts[1].ID, resp.Hosts[2].ID} + assert.Equal(t, descIDs, gotDesc, "desc order mismatch") + }) + } + + // refetch_requested is a bool, so only h2 (true) has a unique value. ASC must + // place h2 last; DESC must place h2 first. The order between h0 and h1 (both + // false) is not deterministic, so we only assert the position of h2. + t.Run("refetch_requested", func(t *testing.T) { + var resp listHostsResponse + s.DoJSON("GET", labelHostsURL, nil, http.StatusOK, &resp, + "order_key", "refetch_requested", "order_direction", "asc") + require.Len(t, resp.Hosts, 3) + assert.Equal(t, sortHosts[2].ID, resp.Hosts[2].ID, "asc: h2 should be last") + + resp = listHostsResponse{} + s.DoJSON("GET", labelHostsURL, nil, http.StatusOK, &resp, + "order_key", "refetch_requested", "order_direction", "desc") + require.Len(t, resp.Hosts, 3) + assert.Equal(t, sortHosts[2].ID, resp.Hosts[0].ID, "desc: h2 should be first") + }) + + // Cursor pagination (`after`) injects the order_key into the WHERE + // clause; SELECT-list aliases like team_name would error there. Verify + // that paging through team_name with a cursor returns the expected + // hosts and does not error. + t.Run("team_name with after cursor", func(t *testing.T) { + var resp listHostsResponse + s.DoJSON("GET", labelHostsURL, nil, http.StatusOK, &resp, + "order_key", "team_name", "order_direction", "asc", + "after", teamA.Name, "per_page", "10") + require.Len(t, resp.Hosts, 2) + assert.Equal(t, sortHosts[1].ID, resp.Hosts[0].ID) + assert.Equal(t, sortHosts[2].ID, resp.Hosts[1].ID) + }) + }) } // Sanity test to make sure fleet/labels//hosts and fleet/hosts return the same thing. diff --git a/server/service/transport.go b/server/service/transport.go index ace8d5b3c8..e2d78765a4 100644 --- a/server/service/transport.go +++ b/server/service/transport.go @@ -330,12 +330,33 @@ func hostListOptionsFromRequest(r *http.Request) (fleet.HostListOptions, error) } hopt.DisableIssues = boolVal } - if hopt.DisableIssues && r.URL.Query().Get("order_key") == "issues" { - return hopt, ctxerr.Wrap( - r.Context(), badRequest( - "Invalid order_key (issues cannot be ordered when they are disabled)", - ), - ) + if hopt.DisableIssues { + switch { + case r.URL.Query().Get("order_key") == "issues": + return hopt, ctxerr.Wrap( + r.Context(), badRequest( + "Invalid order_key (issues cannot be ordered when they are disabled)", + ), + ) + case r.URL.Query().Get("order_key") == "failing_policies_count": + return hopt, ctxerr.Wrap( + r.Context(), badRequest( + "Invalid order_key (failing_policies_count cannot be ordered when they are disabled)", + ), + ) + case r.URL.Query().Get("order_key") == "critical_vulnerabilities_count": + return hopt, ctxerr.Wrap( + r.Context(), badRequest( + "Invalid order_key (critical_vulnerabilities_count cannot be ordered when they are disabled)", + ), + ) + case r.URL.Query().Get("order_key") == "total_issues_count": + return hopt, ctxerr.Wrap( + r.Context(), badRequest( + "Invalid order_key (total_issues_count cannot be ordered when they are disabled)", + ), + ) + } } deviceMapping := r.URL.Query().Get("device_mapping") @@ -347,6 +368,14 @@ func hostListOptionsFromRequest(r *http.Request) (fleet.HostListOptions, error) hopt.DeviceMapping = boolVal } + if !hopt.DeviceMapping && r.URL.Query().Get("order_key") == "device_mapping" { + return hopt, ctxerr.Wrap( + r.Context(), badRequest( + "Invalid order_key (device_mapping cannot be ordered when they are disabled)", + ), + ) + } + mdmID := r.URL.Query().Get("mdm_id") if mdmID != "" { id, err := strconv.ParseUint(mdmID, 10, 32) diff --git a/server/service/transport_test.go b/server/service/transport_test.go index 02b70faf74..5821f37fb4 100644 --- a/server/service/transport_test.go +++ b/server/service/transport_test.go @@ -308,6 +308,43 @@ func TestHostListOptionsFromRequest(t *testing.T) { url: "/foo?disable_failing_policies=true&order_key=issues", errorMessage: "Invalid order_key", }, + "error in failing_policies_count order key when disable_issues is set": { + url: "/foo?disable_issues=true&order_key=failing_policies_count", + errorMessage: "Invalid order_key (failing_policies_count cannot be ordered when they are disabled)", + }, + "error in critical_vulnerabilities_count order key when disable_issues is set": { + url: "/foo?disable_issues=true&order_key=critical_vulnerabilities_count", + errorMessage: "Invalid order_key (critical_vulnerabilities_count cannot be ordered when they are disabled)", + }, + "error in total_issues_count order key when disable_issues is set": { + url: "/foo?disable_issues=true&order_key=total_issues_count", + errorMessage: "Invalid order_key (total_issues_count cannot be ordered when they are disabled)", + }, + "failing_policies_count order key allowed when disable_issues is not set": { + url: "/foo?order_key=failing_policies_count", + hostListOptions: fleet.HostListOptions{ + ListOptions: fleet.ListOptions{ + OrderKey: "failing_policies_count", + }, + }, + }, + "error in device_mapping order key when device_mapping is not enabled": { + url: "/foo?order_key=device_mapping", + errorMessage: "Invalid order_key (device_mapping cannot be ordered when they are disabled)", + }, + "error in device_mapping order key when device_mapping is explicitly false": { + url: "/foo?device_mapping=false&order_key=device_mapping", + errorMessage: "Invalid order_key (device_mapping cannot be ordered when they are disabled)", + }, + "device_mapping order key allowed when device_mapping is enabled": { + url: "/foo?device_mapping=true&order_key=device_mapping", + hostListOptions: fleet.HostListOptions{ + ListOptions: fleet.ListOptions{ + OrderKey: "device_mapping", + }, + DeviceMapping: true, + }, + }, "error in device_mapping": { url: "/foo?device_mapping=foo", errorMessage: "Invalid device_mapping",