From 227e94de5b279decf0dec83550dd2eaa273aed56 Mon Sep 17 00:00:00 2001 From: Jordan Montgomery Date: Tue, 5 May 2026 10:26:47 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20Chore:=20remove=20deprecated=20a?= =?UTF-8?q?ppendListOptionsWithCursorToSQL=20(#44385)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Related issue:** Resolves #44723 # 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/guides/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), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [ ] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **Bug Fixes** * Strengthened validation of sorting/order parameters across many list and cursor-based endpoints — unsupported sort keys now return explicit errors and prevent unsafe queries. * Labels listing: label-list pagination query name changed; ordering by host_count is rejected when host counts are disabled (validated at request parsing). * **Tests** * Added/expanded tests covering allowed order keys, rejection of unknown keys, and pagination behavior for multiple listing APIs. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Lucas Manuel Rodriguez --- changes/cleanup-list-opts | 1 + server/datastore/mysql/activities.go | 10 +- server/datastore/mysql/activities_test.go | 5 + server/datastore/mysql/carves.go | 22 +++- server/datastore/mysql/carves_test.go | 33 ++++++ server/datastore/mysql/labels.go | 24 ++++- server/datastore/mysql/labels_test.go | 48 +++++++++ server/datastore/mysql/maintained_apps.go | 13 ++- .../datastore/mysql/maintained_apps_test.go | 13 +++ server/datastore/mysql/mdm.go | 17 ++- server/datastore/mysql/mdm_test.go | 9 +- server/datastore/mysql/mysql.go | 18 ---- server/datastore/mysql/mysql_test.go | 49 --------- server/datastore/mysql/packs.go | 19 +++- server/datastore/mysql/packs_test.go | 17 +++ server/datastore/mysql/scheduled_queries.go | 32 +++++- .../datastore/mysql/scheduled_queries_test.go | 36 +++++++ server/datastore/mysql/scripts.go | 24 ++++- server/datastore/mysql/scripts_test.go | 18 ++++ server/datastore/mysql/software.go | 12 ++- server/datastore/mysql/software_test.go | 7 ++ server/datastore/mysql/software_titles.go | 19 +++- .../datastore/mysql/software_titles_test.go | 8 ++ server/datastore/mysql/teams.go | 14 ++- server/datastore/mysql/teams_test.go | 13 +++ server/datastore/mysql/vulnerabilities.go | 19 +++- .../datastore/mysql/vulnerabilities_test.go | 9 +- server/fleet/api_labels.go | 2 +- server/service/endpoint_utils.go | 8 ++ server/service/integration_core_test.go | 100 ++++++++++++++++++ server/service/transport.go | 21 ++++ 31 files changed, 551 insertions(+), 89 deletions(-) create mode 100644 changes/cleanup-list-opts diff --git a/changes/cleanup-list-opts b/changes/cleanup-list-opts new file mode 100644 index 0000000000..9379ff5d47 --- /dev/null +++ b/changes/cleanup-list-opts @@ -0,0 +1 @@ +* Improved validation of order parameters on list endpoints diff --git a/server/datastore/mysql/activities.go b/server/datastore/mysql/activities.go index 0fb3c44409..e34b20e10f 100644 --- a/server/datastore/mysql/activities.go +++ b/server/datastore/mysql/activities.go @@ -12,11 +12,16 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/jmoiron/sqlx" ) var deleteIDsBatchSize = 1000 +// hostUpcomingActivitiesAllowedOrderKeys is empty: the query supplies its own +// ORDER BY and the service layer forces opt.OrderKey to "". +var hostUpcomingActivitiesAllowedOrderKeys = common_mysql.OrderKeyAllowlist{} + // ListHostUpcomingActivities returns the list of activities pending execution // or processing for the specific host. It is the "unified queue" of work to be // done on the host. That queue is "virtual" in the sense that it pulls from a @@ -279,7 +284,10 @@ func (ds *Datastore) ListHostUpcomingActivities(ctx context.Context, hostID uint // the ListOptions supported for this query are limited, only the pagination // OFFSET and LIMIT can be added, so it's fine to have the ORDER BY already // in the query before calling this (enforced at the server layer). - stmt, args := appendListOptionsWithCursorToSQL(listStmt, args, &opt) + stmt, args, err := appendListOptionsWithCursorToSQLSecure(listStmt, args, &opt, hostUpcomingActivitiesAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "list upcoming activities") + } var activities []*fleet.UpcomingActivity if err := sqlx.SelectContext(ctx, ds.reader(ctx), &activities, stmt, args...); err != nil { diff --git a/server/datastore/mysql/activities_test.go b/server/datastore/mysql/activities_test.go index a6f1613754..d1bec082cc 100644 --- a/server/datastore/mysql/activities_test.go +++ b/server/datastore/mysql/activities_test.go @@ -535,6 +535,11 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) { } }) } + + t.Run("rejects_unknown_order_key", func(t *testing.T) { + _, _, err := ds.ListHostUpcomingActivities(ctx, h1.ID, fleet.ListOptions{OrderKey: "h.node_key"}) + require.Error(t, err) + }) } func testCleanupExpiredLiveQueries(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/carves.go b/server/datastore/mysql/carves.go index 4fea620a17..2f512da2d6 100644 --- a/server/datastore/mysql/carves.go +++ b/server/datastore/mysql/carves.go @@ -8,9 +8,26 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/jmoiron/sqlx" ) +var carvesAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "id", + "host_id": "host_id", + "created_at": "created_at", + "name": "name", + "block_count": "block_count", + "block_size": "block_size", + "carve_size": "carve_size", + "carve_id": "carve_id", + "request_id": "request_id", + "session_id": "session_id", + "expired": "expired", + "max_block": "max_block", + "error": "error", +} + func upsertCarveDB(ctx context.Context, writer sqlx.ExecerContext, metadata *fleet.CarveMetadata) (int64, error) { stmt := `INSERT INTO carve_metadata ( host_id, @@ -234,7 +251,10 @@ func (ds *Datastore) ListCarves(ctx context.Context, opt fleet.CarveListOptions) if !opt.Expired { stmt += ` WHERE NOT expired ` } - stmt, params := appendListOptionsToSQL(stmt, &opt.ListOptions) + stmt, params, err := appendListOptionsToSQLSecure(stmt, &opt.ListOptions, carvesAllowedOrderKeys) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list carves") + } carves := []*fleet.CarveMetadata{} if err := sqlx.SelectContext(ctx, ds.reader(ctx), &carves, stmt, params...); err != nil && err != sql.ErrNoRows { return nil, ctxerr.Wrap(ctx, err, "list carves") diff --git a/server/datastore/mysql/carves_test.go b/server/datastore/mysql/carves_test.go index 2b50ee4298..55187682d1 100644 --- a/server/datastore/mysql/carves_test.go +++ b/server/datastore/mysql/carves_test.go @@ -241,6 +241,39 @@ func testCarvesList(t *testing.T, ds *Datastore) { carves, err = ds.ListCarves(context.Background(), fleet.CarveListOptions{Expired: true}) require.NoError(t, err) assert.Len(t, carves, 2) + + for _, key := range []string{ + "id", + "host_id", + "created_at", + "name", + "block_count", + "block_size", + "carve_size", + "carve_id", + "request_id", + "session_id", + "expired", + "max_block", + "error", + } { + t.Run("allowed order_"+key, func(t *testing.T) { + result, err := ds.ListCarves(context.Background(), fleet.CarveListOptions{ + Expired: true, + ListOptions: fleet.ListOptions{OrderKey: key, PerPage: 10}, + }) + require.NoError(t, err) + require.NotEmpty(t, result) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, err := ds.ListCarves(context.Background(), fleet.CarveListOptions{ + Expired: true, + ListOptions: fleet.ListOptions{OrderKey: "h.node_key"}, + }) + require.Error(t, err) + }) } func testCarvesUpdate(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/labels.go b/server/datastore/mysql/labels.go index be74257d89..60ffa6d5fb 100644 --- a/server/datastore/mysql/labels.go +++ b/server/datastore/mysql/labels.go @@ -18,6 +18,25 @@ import ( "github.com/jmoiron/sqlx" ) +var labelsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "l.id", + "created_at": "l.created_at", + "updated_at": "l.updated_at", + "name": "l.name", + "description": "l.description", + "query": "l.query", + "platform": "l.platform", + "label_type": "l.label_type", + "label_membership_type": "l.label_membership_type", + "author_id": "l.author_id", + "criteria": "l.criteria", + "team_id": "l.team_id", + + // dependent on include_host_counts being set on request + // (checked on transport layer). + "host_count": "host_count", +} + func (ds *Datastore) ApplyLabelSpecs(ctx context.Context, specs []*fleet.LabelSpec) (err error) { return ds.ApplyLabelSpecsWithAuthor(ctx, specs, nil) } @@ -814,7 +833,10 @@ func (ds *Datastore) ListLabels(ctx context.Context, filter fleet.TeamFilter, op return nil, err } - query, params = appendListOptionsWithCursorToSQL(query, params, &opt) + query, params, err = appendListOptionsWithCursorToSQLSecure(query, params, &opt, labelsAllowedOrderKeys) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list labels") + } var labels []*fleet.Label if err := sqlx.SelectContext(ctx, ds.reader(ctx), &labels, query, params...); err != nil { diff --git a/server/datastore/mysql/labels_test.go b/server/datastore/mysql/labels_test.go index 0801a30cbb..5287173b3a 100644 --- a/server/datastore/mysql/labels_test.go +++ b/server/datastore/mysql/labels_test.go @@ -107,6 +107,7 @@ func TestLabels(t *testing.T) { {"ApplyLabelSpecsWithManualTeamLabels", testApplyLabelSpecsWithManualTeamLabels}, {"ApplyLabelSpecsErrorsWhenLabelExistsOnAnotherTeam", testApplyLabelSpecsErrorsWhenLabelExistsOnAnotherTeam}, {"ApplyLabelSpecsManualNilHosts", testApplyLabelSpecsManualNilHosts}, + {"ListLabelsOrderKeys", testListLabelsOrderKeys}, {"LabelMembershipHostIDs", testLabelMembershipHostIDs}, } // call TruncateTables first to remove migration-created labels @@ -3608,6 +3609,53 @@ func testApplyLabelSpecsManualNilHosts(t *testing.T, ds *Datastore) { require.Equal(t, h1.ID, hosts[0].ID) } +func testListLabelsOrderKeys(t *testing.T, ds *Datastore) { + ctx := t.Context() + + for _, name := range []string{"alpha", "beta", "gamma"} { + err := ds.ApplyLabelSpecs(ctx, []*fleet.LabelSpec{{Name: name, Query: "select 1"}}) + require.NoError(t, err) + } + + filter := fleet.TeamFilter{User: test.UserAdmin} + for _, key := range []string{ + "id", + "created_at", + "updated_at", + "name", + "description", + "query", + "platform", + "label_type", + "label_membership_type", + "author_id", + "criteria", + "team_id", + "host_count", + } { + t.Run("order_"+key, func(t *testing.T) { + labels, err := ds.ListLabels(ctx, filter, fleet.ListOptions{OrderKey: key, PerPage: 100}, true) + require.NoError(t, err) + require.GreaterOrEqual(t, len(labels), 3) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, err := ds.ListLabels(ctx, filter, fleet.ListOptions{OrderKey: "l.id; SELECT 1"}, false) + require.Error(t, err) + }) + + t.Run("page_pagination_with_allowed_key", func(t *testing.T) { + page0, err := ds.ListLabels(ctx, filter, fleet.ListOptions{OrderKey: "name", PerPage: 2, Page: 0}, false) + require.NoError(t, err) + require.NotEmpty(t, page0) + page1, err := ds.ListLabels(ctx, filter, fleet.ListOptions{OrderKey: "name", PerPage: 2, Page: 1}, false) + require.NoError(t, err) + require.NotEmpty(t, page1) + require.NotEqual(t, page0[0].Name, page1[0].Name) + }) +} + func testLabelMembershipHostIDs(t *testing.T, ds *Datastore) { ctx := t.Context() diff --git a/server/datastore/mysql/maintained_apps.go b/server/datastore/mysql/maintained_apps.go index 6b593b01e5..3aa45a078a 100644 --- a/server/datastore/mysql/maintained_apps.go +++ b/server/datastore/mysql/maintained_apps.go @@ -8,9 +8,17 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/jmoiron/sqlx" ) +var maintainedAppsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "fma.id", + "name": "fma.name", + "platform": "fma.platform", + "slug": "fma.slug", +} + func (ds *Datastore) UpsertMaintainedApp(ctx context.Context, app *fleet.MaintainedApp) (*fleet.MaintainedApp, error) { const upsertStmt = ` INSERT INTO @@ -198,7 +206,10 @@ func (ds *Datastore) ListAvailableFleetMaintainedApps(ctx context.Context, teamI } } - stmtPaged, args := appendListOptionsWithCursorToSQL(stmt, args, &opt) + stmtPaged, args, err := appendListOptionsWithCursorToSQLSecure(stmt, args, &opt, maintainedAppsAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "list fleet maintained apps") + } var avail []fleet.MaintainedApp if err := sqlx.SelectContext(ctx, ds.reader(ctx), &avail, stmtPaged, args...); err != nil { diff --git a/server/datastore/mysql/maintained_apps_test.go b/server/datastore/mysql/maintained_apps_test.go index c68ab4f4ea..e9e898757a 100644 --- a/server/datastore/mysql/maintained_apps_test.go +++ b/server/datastore/mysql/maintained_apps_test.go @@ -452,6 +452,19 @@ func testListAndGetAvailableApps(t *testing.T, ds *Datastore) { require.NoError(t, err) maintained3.TitleID = nil require.Equal(t, maintained3, gotApp) + + for _, key := range []string{"id", "name", "platform", "slug"} { + t.Run("order_"+key, func(t *testing.T) { + result, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{OrderKey: key, PerPage: 10, IncludeMetadata: true}) + require.NoError(t, err) + require.NotEmpty(t, result) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, _, err := ds.ListAvailableFleetMaintainedApps(ctx, &team1.ID, fleet.ListOptions{OrderKey: "h.node_key", IncludeMetadata: true}) + require.Error(t, err) + }) } func testSyncAndRemoveApps(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/mdm.go b/server/datastore/mysql/mdm.go index 1e36923629..f39341c041 100644 --- a/server/datastore/mysql/mdm.go +++ b/server/datastore/mysql/mdm.go @@ -20,6 +20,16 @@ import ( "github.com/jmoiron/sqlx" ) +var mdmConfigProfilesAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "profile_uuid": "profile_uuid", + "team_id": "team_id", + "name": "name", + "platform": "platform", + "identifier": "identifier", + "created_at": "created_at", + "uploaded_at": "uploaded_at", +} + func (ds *Datastore) GetMDMCommandPlatform(ctx context.Context, commandUUID string) (string, error) { stmt := ` SELECT CASE @@ -704,9 +714,12 @@ FROM ( } args := []any{globalOrTeamID, fleetIdentifiers, globalOrTeamID, fleetNames, globalOrTeamID, fleetNames, globalOrTeamID, fleetNames} - stmt, args := appendListOptionsWithCursorToSQL(selectStmt, args, &opt) + stmt, args, err := appendListOptionsWithCursorToSQLSecure(selectStmt, args, &opt, mdmConfigProfilesAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "list MDM config profiles") + } - stmt, args, err := sqlx.In(stmt, args...) + stmt, args, err = sqlx.In(stmt, args...) if err != nil { return nil, nil, ctxerr.Wrap(ctx, err, "sqlx.In ListMDMConfigProfiles") } diff --git a/server/datastore/mysql/mdm_test.go b/server/datastore/mysql/mdm_test.go index 8f03a49b37..7e0cdcad01 100644 --- a/server/datastore/mysql/mdm_test.go +++ b/server/datastore/mysql/mdm_test.go @@ -1043,13 +1043,12 @@ func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) { ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{ - ListOptions: fleet.ListOptions{OrderKey: "command_uuid", PerPage: 1}, + ListOptions: fleet.ListOptions{OrderKey: "command_uuid", PerPage: 1, IncludeMetadata: true}, }, ) require.NoError(t, err) require.Len(t, cmds, 1) afterCursor := cmds[0].CommandUUID - next, _, _, err := ds.ListMDMCommands( ctx, fleet.TeamFilter{User: test.UserAdmin}, @@ -1119,7 +1118,6 @@ func testListMDMAppleCommandsOrderKeys(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, cmds, 1) afterCursor := cmds[0].CommandUUID - next, err := ds.ListMDMAppleCommands( ctx, fleet.TeamFilter{User: test.UserAdmin}, @@ -1857,6 +1855,11 @@ func testListMDMConfigProfiles(t *testing.T, ds *Datastore) { require.Equal(t, c.wantMeta, gotMeta) }) } + + t.Run("rejects_unknown_order_key", func(t *testing.T) { + _, _, err := ds.ListMDMConfigProfiles(ctx, nil, fleet.ListOptions{OrderKey: "h.node_key"}) + require.Error(t, err) + }) } func testBulkSetPendingMDMHostProfilesBatch2(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go index efac0d8236..45f13de5f0 100644 --- a/server/datastore/mysql/mysql.go +++ b/server/datastore/mysql/mysql.go @@ -853,13 +853,6 @@ func sanitizeColumn(col string) string { return common_mysql.SanitizeColumn(col) } -// appendListOptionsToSQL is a facade that calls common_mysql.AppendListOptions. -// -// Deprecated: this method will be removed in favor of appendListOptionsWithCursorToSQL -func appendListOptionsToSQL(sql string, opts *fleet.ListOptions) (string, []any) { - return appendListOptionsWithCursorToSQL(sql, nil, opts) -} - // appendListOptionsToSQLSecure is a facade that calls common_mysql.AppendListOptionsWithParamsSecure. // The allowlist parameter maps user-facing order key names to actual SQL column expressions. // This prevents SQL injection and information disclosure via arbitrary column sorting. @@ -868,17 +861,6 @@ func appendListOptionsToSQLSecure(sql string, opts *fleet.ListOptions, allowlist return appendListOptionsWithCursorToSQLSecure(sql, nil, opts, allowlist) } -// appendListOptionsWithCursorToSQL is a facade that calls common_mysql.AppendListOptionsWithParams. -// NOTE: this method will mutate opts.PerPage if it is 0, setting it to the default value. -// -// Deprecated: this method will be removed in favor of appendListOptionsWithCursorToSQLSecure -func appendListOptionsWithCursorToSQL(sql string, params []any, opts *fleet.ListOptions) (string, []any) { - if opts.PerPage == 0 { - opts.PerPage = fleet.DefaultPerPage - } - return common_mysql.AppendListOptionsWithParams(sql, params, opts) -} - // appendListOptionsWithCursorToSQLSecure is a facade that calls common_mysql.AppendListOptionsWithParamsSecure. // NOTE: this method will mutate opts.PerPage if it is 0, setting it to the default value. // diff --git a/server/datastore/mysql/mysql_test.go b/server/datastore/mysql/mysql_test.go index b9198341f2..411f079599 100644 --- a/server/datastore/mysql/mysql_test.go +++ b/server/datastore/mysql/mysql_test.go @@ -416,55 +416,6 @@ func TestAppendListOptionsToSQLSecure(t *testing.T) { require.Equal(t, "invalid_column", invalidKeyErr.Key) } -func TestAppendListOptionsToSQL(t *testing.T) { - sql := "SELECT * FROM my_table" - opts := fleet.ListOptions{ - OrderKey: "***name***", - } - - actual, _ := appendListOptionsToSQL(sql, &opts) - expected := "SELECT * FROM my_table ORDER BY `name` ASC LIMIT 1000000" - if actual != expected { - t.Error("Expected", expected, "Actual", actual) - } - - sql = "SELECT * FROM my_table" - opts.OrderDirection = fleet.OrderDescending - actual, _ = appendListOptionsToSQL(sql, &opts) - expected = "SELECT * FROM my_table ORDER BY `name` DESC LIMIT 1000000" - if actual != expected { - t.Error("Expected", expected, "Actual", actual) - } - - opts = fleet.ListOptions{ - PerPage: 10, - } - - sql = "SELECT * FROM my_table" - actual, _ = appendListOptionsToSQL(sql, &opts) - expected = "SELECT * FROM my_table LIMIT 10" - if actual != expected { - t.Error("Expected", expected, "Actual", actual) - } - - sql = "SELECT * FROM my_table" - opts.Page = 2 - actual, _ = appendListOptionsToSQL(sql, &opts) - expected = "SELECT * FROM my_table LIMIT 10 OFFSET 20" - if actual != expected { - t.Error("Expected", expected, "Actual", actual) - } - - opts = fleet.ListOptions{} - sql = "SELECT * FROM my_table" - actual, _ = appendListOptionsToSQL(sql, &opts) - expected = "SELECT * FROM my_table LIMIT 1000000" - - if actual != expected { - t.Error("Expected", expected, "Actual", actual) - } -} - func TestWhereFilterHostsByTeams(t *testing.T) { t.Parallel() diff --git a/server/datastore/mysql/packs.go b/server/datastore/mysql/packs.go index cb1cf9142e..9f6f0b825d 100644 --- a/server/datastore/mysql/packs.go +++ b/server/datastore/mysql/packs.go @@ -8,9 +8,21 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/jmoiron/sqlx" ) +var packsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "id", + "name": "name", + "description": "description", + "platform": "platform", + "disabled": "disabled", + "pack_type": "pack_type", + "created_at": "created_at", + "updated_at": "updated_at", +} + func (ds *Datastore) ApplyPackSpecs(ctx context.Context, specs []*fleet.PackSpec) (err error) { err = ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { for _, spec := range specs { @@ -455,8 +467,11 @@ func (ds *Datastore) ListPacks(ctx context.Context, opt fleet.PackListOptions) ( query = `SELECT * FROM packs` } var packs []*fleet.Pack - query, params := appendListOptionsToSQL(query, &opt.ListOptions) - err := sqlx.SelectContext(ctx, ds.reader(ctx), &packs, query, params...) + query, params, err := appendListOptionsToSQLSecure(query, &opt.ListOptions, packsAllowedOrderKeys) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list packs") + } + err = sqlx.SelectContext(ctx, ds.reader(ctx), &packs, query, params...) if err != nil && err != sql.ErrNoRows { return nil, ctxerr.Wrap(ctx, err, "listing packs") } diff --git a/server/datastore/mysql/packs_test.go b/server/datastore/mysql/packs_test.go index 2350e730e7..224aba5eee 100644 --- a/server/datastore/mysql/packs_test.go +++ b/server/datastore/mysql/packs_test.go @@ -136,6 +136,23 @@ func testPacksList(t *testing.T, ds *Datastore) { packs, err = ds.ListPacks(context.Background(), fleet.PackListOptions{IncludeSystemPacks: false}) require.Nil(t, err) assert.Len(t, packs, 2) + + for _, key := range []string{"id", "name", "description", "platform", "disabled", "pack_type", "created_at", "updated_at"} { + t.Run("order_"+key, func(t *testing.T) { + result, err := ds.ListPacks(context.Background(), fleet.PackListOptions{ + ListOptions: fleet.ListOptions{OrderKey: key, PerPage: 10}, + }) + require.NoError(t, err) + require.NotEmpty(t, result) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, err := ds.ListPacks(context.Background(), fleet.PackListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "h.node_key"}, + }) + require.Error(t, err) + }) } func setupPackSpecsTest(t *testing.T, ds fleet.Datastore) []*fleet.PackSpec { diff --git a/server/datastore/mysql/scheduled_queries.go b/server/datastore/mysql/scheduled_queries.go index 3c001b1634..5223a75e64 100644 --- a/server/datastore/mysql/scheduled_queries.go +++ b/server/datastore/mysql/scheduled_queries.go @@ -11,8 +11,35 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" ) +var scheduledQueriesAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "sq.id", + "pack_id": "sq.pack_id", + "name": "sq.name", + "query_name": "sq.query_name", + "description": "sq.description", + "interval": "sq.interval", + "snapshot": "sq.snapshot", + "removed": "sq.removed", + "platform": "sq.platform", + "version": "sq.version", + "shard": "sq.shard", + "denylist": "sq.denylist", + + "query": "q.query", // from queries table + "query_id": "query_id", // from queries table + + // JSON_EXTRACT required on the following: + // must match SELECT clause so cursor pagination (WHERE) and ORDER BY are consistent + "user_time_p50": "JSON_EXTRACT(ag.json_value, '$.user_time_p50')", + "user_time_p95": "JSON_EXTRACT(ag.json_value, '$.user_time_p95')", + "system_time_p50": "JSON_EXTRACT(ag.json_value, '$.system_time_p50')", + "system_time_p95": "JSON_EXTRACT(ag.json_value, '$.system_time_p95')", + "total_executions": "JSON_EXTRACT(ag.json_value, '$.total_executions')", +} + // ListScheduledQueriesInPackWithStats loads a pack's scheduled queries and its aggregated stats. func (ds *Datastore) ListScheduledQueriesInPackWithStats(ctx context.Context, id uint, opts fleet.ListOptions) ([]*fleet.ScheduledQuery, error) { query := ` @@ -42,7 +69,10 @@ func (ds *Datastore) ListScheduledQueriesInPackWithStats(ctx context.Context, id WHERE sq.pack_id = ? ` params := []interface{}{false, fleet.AggregatedStatsTypeScheduledQuery, id} - query, params = appendListOptionsWithCursorToSQL(query, params, &opts) + query, params, err := appendListOptionsWithCursorToSQLSecure(query, params, &opts, scheduledQueriesAllowedOrderKeys) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "listing scheduled queries") + } results := []*fleet.ScheduledQuery{} if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, query, params...); err != nil { diff --git a/server/datastore/mysql/scheduled_queries_test.go b/server/datastore/mysql/scheduled_queries_test.go index d03b2b7d15..58cac42dd3 100644 --- a/server/datastore/mysql/scheduled_queries_test.go +++ b/server/datastore/mysql/scheduled_queries_test.go @@ -127,6 +127,42 @@ func testScheduledQueriesListInPackWithStats(t *testing.T, ds *Datastore) { } } require.True(t, foundAgg) + + for _, key := range []string{ + "id", + "pack_id", + "name", + "query_name", + "description", + "interval", + "snapshot", + "removed", + "platform", + "version", + "shard", + "denylist", + "query", + "query_id", + "user_time_p50", + "user_time_p95", + "system_time_p50", + "system_time_p95", + "total_executions", + } { + t.Run("order_"+key, func(t *testing.T) { + _, err := ds.ListScheduledQueriesInPackWithStats(context.Background(), 1, fleet.ListOptions{ + OrderKey: key, + PerPage: 10, + After: " ", + }) + require.NoError(t, err) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, err := ds.ListScheduledQueriesInPackWithStats(context.Background(), 1, fleet.ListOptions{OrderKey: "h.node_key"}) + require.Error(t, err) + }) } func testScheduledQueriesListInPack(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/scripts.go b/server/datastore/mysql/scripts.go index 4050fa007e..40385e51e2 100644 --- a/server/datastore/mysql/scripts.go +++ b/server/datastore/mysql/scripts.go @@ -15,11 +15,25 @@ import ( constants "github.com/fleetdm/fleet/v4/pkg/scripts" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/google/uuid" "github.com/jmoiron/sqlx" ) +var scriptsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "s.id", + "name": "s.name", + "created_at": "s.created_at", + "updated_at": "s.updated_at", +} + +// hostScriptDetailsAllowedOrderKeys is intentionally minimal: the service layer +// pins OrderKey to "name" before reaching this datastore method. +var hostScriptDetailsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "name": "s.name", +} + func (ds *Datastore) NewHostScriptExecutionRequest(ctx context.Context, request *fleet.HostScriptRequestPayload) (*fleet.HostScriptResult, error) { var res *fleet.HostScriptResult return res, ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { @@ -949,7 +963,10 @@ WHERE } args := []any{globalOrTeamID} - stmt, args := appendListOptionsWithCursorToSQL(selectStmt, args, &opt) + stmt, args, err := appendListOptionsWithCursorToSQLSecure(selectStmt, args, &opt, scriptsAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "list scripts") + } if err := sqlx.SelectContext(ctx, ds.reader(ctx), &scripts, stmt, args...); err != nil { return nil, nil, ctxerr.Wrap(ctx, err, "select scripts") @@ -1115,7 +1132,10 @@ WHERE ) ` } - stmt, args := appendListOptionsWithCursorToSQL(sql, args, &opt) + stmt, args, err := appendListOptionsWithCursorToSQLSecure(sql, args, &opt, hostScriptDetailsAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "get host script details") + } var rows []*row if err := sqlx.SelectContext(ctx, ds.reader(ctx), &rows, stmt, args...); err != nil { diff --git a/server/datastore/mysql/scripts_test.go b/server/datastore/mysql/scripts_test.go index c464532111..08fbd40619 100644 --- a/server/datastore/mysql/scripts_test.go +++ b/server/datastore/mysql/scripts_test.go @@ -560,6 +560,19 @@ func testListScripts(t *testing.T, ds *Datastore) { require.Equal(t, c.wantNames, gotNames) }) } + + for _, key := range []string{"id", "name", "created_at", "updated_at"} { + t.Run("order_"+key, func(t *testing.T) { + result, _, err := ds.ListScripts(ctx, nil, fleet.ListOptions{OrderKey: key, PerPage: 10}) + require.NoError(t, err) + require.NotEmpty(t, result) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, _, err := ds.ListScripts(ctx, nil, fleet.ListOptions{OrderKey: "h.node_key"}) + require.Error(t, err) + }) } func testGetHostScriptDetails(t *testing.T, ds *Datastore) { @@ -788,6 +801,11 @@ func testGetHostScriptDetails(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, pending, 0) }) + + t.Run("rejects_unknown_order_key", func(t *testing.T) { + _, _, err := ds.GetHostScriptDetails(ctx, 42, nil, fleet.ListOptions{OrderKey: "h.node_key"}, "darwin") + require.Error(t, err) + }) } func testBatchSetScripts(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index b6a59b83a4..485aac54ff 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -4513,6 +4513,13 @@ func promoteSoftwareTitleInHouseApp(softwareTitleRecord *hostSoftware) { } } +// hostSoftwareAllowedOrderKeys is minimal: the service layer pins OrderKey to "name". +// "source" is included for test determinism (used as the secondary order key in tests). +var hostSoftwareAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "name": "name", + "source": "source", +} + func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opts fleet.HostSoftwareTitleListOptions) ([]*fleet.HostSoftwareWithInstaller, *fleet.PaginationMetadata, error) { if !opts.VulnerableOnly && (opts.MinimumCVSS > 0 || opts.MaximumCVSS > 0 || opts.KnownExploit) { return nil, nil, fleet.NewInvalidArgumentError( @@ -5970,7 +5977,10 @@ func (ds *Datastore) ListHostSoftware(ctx context.Context, host *fleet.Host, opt } stmt = fmt.Sprintf(stmt, replacements...) stmt = fmt.Sprintf("SELECT * FROM (%s) AS combined_results", stmt) - stmt, _ = appendListOptionsToSQL(stmt, &opts.ListOptions) + stmt, _, err = appendListOptionsToSQLSecure(stmt, &opts.ListOptions, hostSoftwareAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "list host software") + } if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hostSoftwareList, stmt, args...); err != nil { return nil, nil, ctxerr.Wrap(ctx, err, "list host software") diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index 6d73a2392e..f82eddba0f 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -5417,6 +5417,13 @@ func testListHostSoftware(t *testing.T, ds *Datastore) { } } require.False(t, found, "Expected not find software %s in the list", softwareAlreadyInstalled.Name) + + t.Run("rejects_unknown_order_key", func(t *testing.T) { + _, _, err := ds.ListHostSoftware(ctx, host, fleet.HostSoftwareTitleListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "h.node_key"}, + }) + require.Error(t, err) + }) } func testListLinuxHostSoftware(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/software_titles.go b/server/datastore/mysql/software_titles.go index 3753e31fdb..9696d41835 100644 --- a/server/datastore/mysql/software_titles.go +++ b/server/datastore/mysql/software_titles.go @@ -19,6 +19,16 @@ import ( "golang.org/x/sync/errgroup" ) +var softwareTitlesAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "st.id", + "name": "st.name", + "source": "st.source", + "extension_for": "st.extension_for", + "bundle_identifier": "st.bundle_identifier", + "hosts_count": "hosts_count", + "counts_updated_at": "counts_updated_at", +} + func (ds *Datastore) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { var ( teamFilter string // used to filter software titles host counts by team @@ -269,9 +279,12 @@ func (ds *Datastore) ListSoftwareTitles( getTitlesCountStmt := fmt.Sprintf(`SELECT COUNT(DISTINCT s.id) FROM (%s) AS s`, getTitlesStmt) var softwareList []*softwareTitleWithInstallerFields - getTitlesStmt, args = appendListOptionsWithCursorToSQL(getTitlesStmt, args, &opt.ListOptions) - // appendListOptionsWithCursorToSQL doesn't support multicolumn sort, so - // we need to add it here + getTitlesStmt, args, err = appendListOptionsWithCursorToSQLSecure(getTitlesStmt, args, &opt.ListOptions, softwareTitlesAllowedOrderKeys) + if err != nil { + return nil, 0, nil, ctxerr.Wrap(ctx, err, "list software titles") + } + // secondary sort columns must be added separately since the helper above + // only handles a single ORDER BY column. getTitlesStmt = spliceSecondaryOrderBySoftwareTitlesSQL(getTitlesStmt, opt.ListOptions) // Run list and count queries in parallel. diff --git a/server/datastore/mysql/software_titles_test.go b/server/datastore/mysql/software_titles_test.go index 564e17da72..ef041743cb 100644 --- a/server/datastore/mysql/software_titles_test.go +++ b/server/datastore/mysql/software_titles_test.go @@ -557,6 +557,14 @@ func testOrderSoftwareTitles(t *testing.T, ds *Datastore) { require.Len(t, titles, 1) require.Equal(t, "installer1", titles[0].Name) require.Equal(t, "apps", titles[0].Source) + + t.Run("rejects_unknown_order_key", func(t *testing.T) { + _, _, _, err := ds.ListSoftwareTitles(ctx, fleet.SoftwareTitleListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "h.node_key"}, + TeamID: ptr.Uint(0), + }, fleet.TeamFilter{User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}}) + require.Error(t, err) + }) } func listSoftwareTitlesCheckCount(t *testing.T, ds *Datastore, expectedListCount int, expectedFullCount int, opts fleet.SoftwareTitleListOptions) []fleet.SoftwareTitleListResult { diff --git a/server/datastore/mysql/teams.go b/server/datastore/mysql/teams.go index 3f7378df05..404f8ed5e5 100644 --- a/server/datastore/mysql/teams.go +++ b/server/datastore/mysql/teams.go @@ -13,12 +13,21 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/jmoiron/sqlx" ) var teamSearchColumns = []string{"name"} +var teamsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "id": "t.id", + "name": "t.name", + "created_at": "t.created_at", + "user_count": "user_count", + "host_count": "host_count", +} + const teamColumns = `id, created_at, name, filename, description, config` func (ds *Datastore) NewTeam(ctx context.Context, team *fleet.Team) (*fleet.Team, error) { @@ -423,7 +432,10 @@ func (ds *Datastore) ListTeams(ctx context.Context, filter fleet.TeamFilter, opt // We must normalize the name for full Unicode support (Unicode equivalence). matchQuery := norm.NFC.String(opt.MatchQuery) query, params := searchLike(query, nil, matchQuery, teamSearchColumns...) - query, params = appendListOptionsWithCursorToSQL(query, params, &opt) + query, params, err := appendListOptionsWithCursorToSQLSecure(query, params, &opt, teamsAllowedOrderKeys) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list teams") + } teams := []*fleet.Team{} if err := sqlx.SelectContext(ctx, ds.reader(ctx), &teams, query, params...); err != nil { return nil, ctxerr.Wrap(ctx, err, "list teams") diff --git a/server/datastore/mysql/teams_test.go b/server/datastore/mysql/teams_test.go index 7bdb3eb0f4..a35cb7be46 100644 --- a/server/datastore/mysql/teams_test.go +++ b/server/datastore/mysql/teams_test.go @@ -457,6 +457,19 @@ func testTeamsList(t *testing.T, ds *Datastore) { t2.Users = nil require.Equal(t, t1, t2) } + + for _, key := range []string{"id", "name", "created_at", "user_count", "host_count"} { + t.Run("order_"+key, func(t *testing.T) { + result, err := ds.ListTeams(context.Background(), fleet.TeamFilter{User: &user1}, fleet.ListOptions{OrderKey: key, PerPage: 10}) + require.NoError(t, err) + require.NotEmpty(t, result) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, err := ds.ListTeams(context.Background(), fleet.TeamFilter{User: &user1}, fleet.ListOptions{OrderKey: "h.node_key"}) + require.Error(t, err) + }) } func testTeamsSummary(t *testing.T, ds *Datastore) { diff --git a/server/datastore/mysql/vulnerabilities.go b/server/datastore/mysql/vulnerabilities.go index b163ea94e6..41dfa8361a 100644 --- a/server/datastore/mysql/vulnerabilities.go +++ b/server/datastore/mysql/vulnerabilities.go @@ -11,9 +11,23 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/jmoiron/sqlx" ) +var vulnerabilitiesAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "cve": "cve", + "cvss_score": "cvss_score", + "epss_probability": "epss_probability", + "cisa_known_exploit": "cisa_known_exploit", + "cve_published": "cve_published", + "created_at": "created_at", + "host_count": "hosts_count", + "hosts_count": "hosts_count", + "host_count_updated_at": "hosts_count_updated_at", + "hosts_count_updated_at": "hosts_count_updated_at", +} + func (ds *Datastore) Vulnerability(ctx context.Context, cve string, teamID *uint, includeCVEScores bool) (*fleet.VulnerabilityWithMetadata, error) { var vuln fleet.VulnerabilityWithMetadata @@ -303,7 +317,10 @@ func (ds *Datastore) ListVulnerabilities(ctx context.Context, opt fleet.VulnList } opt.ListOptions.IncludeMetadata = !(opt.ListOptions.UsesCursorPagination()) - selectStmt, args = appendListOptionsWithCursorToSQL(selectStmt, args, &opt.ListOptions) + selectStmt, args, err := appendListOptionsWithCursorToSQLSecure(selectStmt, args, &opt.ListOptions, vulnerabilitiesAllowedOrderKeys) + if err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "list vulnerabilities") + } // Execute the query var vulns []fleet.VulnerabilityWithMetadata diff --git a/server/datastore/mysql/vulnerabilities_test.go b/server/datastore/mysql/vulnerabilities_test.go index 4f0d81affa..fa3aa45df0 100644 --- a/server/datastore/mysql/vulnerabilities_test.go +++ b/server/datastore/mysql/vulnerabilities_test.go @@ -509,7 +509,7 @@ func testListVulnerabilitiesSort(t *testing.T, ds *Datastore) { require.Equal(t, "CVE-2020-1237", list[3].CVE.CVE) require.Equal(t, "CVE-2020-1236", list[4].CVE.CVE) - opts.ListOptions.OrderKey = "published" + opts.ListOptions.OrderKey = "cve_published" opts.ListOptions.OrderDirection = fleet.OrderAscending list, _, err = ds.ListVulnerabilities(context.Background(), opts) require.NoError(t, err) @@ -519,6 +519,13 @@ func testListVulnerabilitiesSort(t *testing.T, ds *Datastore) { require.Equal(t, "CVE-2020-1236", list[2].CVE.CVE) require.Equal(t, "CVE-2020-1235", list[3].CVE.CVE) require.Equal(t, "CVE-2020-1237", list[4].CVE.CVE) + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, _, err := ds.ListVulnerabilities(context.Background(), fleet.VulnListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "h.node_key"}, + }) + require.Error(t, err) + }) } func testVulnerabilitiesFilters(t *testing.T, ds *Datastore) { diff --git a/server/fleet/api_labels.go b/server/fleet/api_labels.go index 98421bd9ff..8dcb109da5 100644 --- a/server/fleet/api_labels.go +++ b/server/fleet/api_labels.go @@ -65,7 +65,7 @@ func (r GetLabelResponse) Error() error { return r.Err } //////////////////////////////////////////////////////////////////////////////// type ListLabelsRequest struct { - ListOptions ListOptions `url:"list_options"` + ListOptions ListOptions `url:"label_list_options"` TeamID *string `query:"team_id,optional" renameto:"fleet_id"` // string because it's an int or "global" IncludeHostCounts *bool `query:"include_host_counts,optional"` } diff --git a/server/service/endpoint_utils.go b/server/service/endpoint_utils.go index 3613810097..07c703dd9b 100644 --- a/server/service/endpoint_utils.go +++ b/server/service/endpoint_utils.go @@ -101,6 +101,14 @@ func parseCustomTags(urlTagValue string, r *http.Request, field reflect.Value) ( } field.Set(reflect.ValueOf(opts)) return true, nil + + case "label_list_options": + opts, err := labelListOptionsFromRequest(r) + if err != nil { + return false, err + } + field.Set(reflect.ValueOf(opts)) + return true, nil } return false, nil } diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index e821db422c..0c216e0950 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -2981,6 +2981,22 @@ func (s *integrationTestSuite) TestGetHostSummary() { // 'after' param is not supported for labels s.DoJSON("GET", "/api/latest/fleet/labels", nil, http.StatusBadRequest, &listResp, "order_key", "id", "after", "1") + // ordering by host_count when include_host_counts=false is rejected + res := s.Do("GET", "/api/latest/fleet/labels", nil, http.StatusBadRequest, "order_key", "host_count", "include_host_counts", "false") + require.Contains(t, extractServerErrorText(res.Body), "Invalid order_key (host_count cannot be ordered when they are disabled)") + + // ordering by host_count with include_host_counts=true is allowed + listResp = fleet.ListLabelsResponse{} + s.DoJSON("GET", "/api/latest/fleet/labels", nil, http.StatusOK, &listResp, "order_key", "host_count", "include_host_counts", "true") + + // ordering by host_count without include_host_counts (default true) is allowed + listResp = fleet.ListLabelsResponse{} + s.DoJSON("GET", "/api/latest/fleet/labels", nil, http.StatusOK, &listResp, "order_key", "host_count") + + // include_host_counts=false with a different order_key is allowed + listResp = fleet.ListLabelsResponse{} + s.DoJSON("GET", "/api/latest/fleet/labels", nil, http.StatusOK, &listResp, "order_key", "name", "include_host_counts", "false") + // team filter, no host s.DoJSON("GET", "/api/latest/fleet/host_summary", nil, http.StatusOK, &resp, "team_id", fmt.Sprint(team2.ID)) require.Equal(t, resp.TotalsHostsCount, uint(0)) @@ -4170,6 +4186,90 @@ func (s *integrationTestSuite) TestScheduledQueries() { assert.Equal(t, uint(0), delBatchResp.Deleted) } +func (s *integrationTestSuite) TestScheduledQueriesInPackOrderKey() { + t := s.T() + + // create a pack + var createPackResp createPackResponse + s.DoJSON("POST", "/api/latest/fleet/packs", &createPackRequest{ + PackPayload: fleet.PackPayload{ + Name: new(strings.ReplaceAll(t.Name(), "/", "_")), + }, + }, http.StatusOK, &createPackResp) + pack := createPackResp.Pack.Pack + + // create a query + var createQueryResp fleet.CreateQueryResponse + s.DoJSON("POST", "/api/latest/fleet/queries", &fleet.QueryPayload{ + Name: new(strings.ReplaceAll(t.Name(), "/", "_")), + Query: new("select 1"), + }, http.StatusOK, &createQueryResp) + query := createQueryResp.Query + + // schedule the query in the pack so the listing has at least one row + var createSchedResp fleet.ScheduleQueryResponse + s.DoJSON("POST", "/api/latest/fleet/packs/schedule", &fleet.ScheduleQueryRequest{ + PackID: pack.ID, + QueryID: query.ID, + Interval: 60, + }, http.StatusOK, &createSchedResp) + + // every key in scheduledQueriesAllowedOrderKeys must work end-to-end with cursor pagination. + allowedOrderKeys := []string{ + "id", + "pack_id", + "name", + "query_name", + "description", + "interval", + "snapshot", + "removed", + "platform", + "version", + "shard", + "denylist", + "query", + "query_id", + "user_time_p50", + "user_time_p95", + "system_time_p50", + "system_time_p95", + "total_executions", + } + for _, orderKey := range allowedOrderKeys { + t.Run(orderKey, func(t *testing.T) { + var getInPackResp fleet.GetScheduledQueriesInPackResponse + s.DoJSON( + "GET", fmt.Sprintf("/api/latest/fleet/packs/%d/scheduled", pack.ID), + nil, http.StatusOK, &getInPackResp, + "order_key", orderKey, + "after", "0", + ) + }) + } +} + +func (s *integrationTestSuite) TestScheduledQueriesInPackInvalidOrderKey() { + t := s.T() + + // create a pack so the endpoint has a real id to operate on + var createPackResp createPackResponse + s.DoJSON("POST", "/api/latest/fleet/packs", &createPackRequest{ + PackPayload: fleet.PackPayload{ + Name: new(strings.ReplaceAll(t.Name(), "/", "_")), + }, + }, http.StatusOK, &createPackResp) + pack := createPackResp.Pack.Pack + + var getInPackResp fleet.GetScheduledQueriesInPackResponse + s.DoJSON( + "GET", fmt.Sprintf("/api/latest/fleet/packs/%d/scheduled", pack.ID), + nil, http.StatusUnprocessableEntity, &getInPackResp, + "order_key", "not_a_real_column", + "after", "0", + ) +} + func (s *integrationTestSuite) TestQueriesPaginationAndPlatformFilter() { t := s.T() diff --git a/server/service/transport.go b/server/service/transport.go index e2d78765a4..7c1c5c1672 100644 --- a/server/service/transport.go +++ b/server/service/transport.go @@ -664,6 +664,27 @@ func hostListOptionsFromRequest(r *http.Request) (fleet.HostListOptions, error) return hopt, nil } +func labelListOptionsFromRequest(r *http.Request) (fleet.ListOptions, error) { + opt, err := listOptionsFromRequest(r) + if err != nil { + return fleet.ListOptions{}, err + } + + includeHostCountsStr := r.URL.Query().Get("include_host_counts") + if includeHostCountsStr != "" && opt.OrderKey == "host_count" { + includeHostCounts, parseErr := strconv.ParseBool(includeHostCountsStr) + if parseErr == nil && !includeHostCounts { + return fleet.ListOptions{}, ctxerr.Wrap( + r.Context(), badRequest( + "Invalid order_key (host_count cannot be ordered when they are disabled)", + ), + ) + } + } + + return opt, nil +} + func carveListOptionsFromRequest(r *http.Request) (fleet.CarveListOptions, error) { opt, err := listOptionsFromRequest(r) if err != nil {