diff --git a/changes/fix-mdm-commands-filtering b/changes/fix-mdm-commands-filtering new file mode 100644 index 00000000000..ec0d2d9ff10 --- /dev/null +++ b/changes/fix-mdm-commands-filtering @@ -0,0 +1 @@ +* Improved validation for invalid `order_key` values in `/api/v1/fleet/commands`, `/api/v1/fleet/mdm/commands` and `/api/v1/fleet/mdm/apple/commands` endpoints. diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 2e3747a33b4..991e02d0fb2 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -1117,6 +1117,16 @@ WHERE return results, nil } +var mdmAppleCommandsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "command_uuid": "nvq.command_uuid", + "request_type": "nvq.request_type", + "status": "COALESCE(NULLIF(nvq.status, ''), 'Pending')", + "updated_at": "COALESCE(nvq.result_updated_at, nvq.created_at)", + "hostname": "h.hostname", + "device_id": "ne.device_id", + "name": "nvq.name", +} + func (ds *Datastore) ListMDMAppleCommands( ctx context.Context, tmFilter fleet.TeamFilter, @@ -1148,7 +1158,10 @@ WHERE nvq.active = 1 AND %s `, ds.whereFilterHostsByTeams(tmFilter, "h")) - stmt, params := appendListOptionsWithCursorToSQL(stmt, nil, &listOpts.ListOptions) + stmt, params, err := appendListOptionsWithCursorToSQLSecure(stmt, nil, &listOpts.ListOptions, mdmAppleCommandsAllowedOrderKeys) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list commands") + } var results []*fleet.MDMAppleCommand if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, stmt, params...); err != nil { diff --git a/server/datastore/mysql/mdm.go b/server/datastore/mysql/mdm.go index 6f4987a3ca4..c5188506757 100644 --- a/server/datastore/mysql/mdm.go +++ b/server/datastore/mysql/mdm.go @@ -14,10 +14,21 @@ import ( "github.com/fleetdm/fleet/v4/server/mdm" "github.com/fleetdm/fleet/v4/server/mdm/apple/mobileconfig" microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft" + common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql" "github.com/google/go-cmp/cmp" "github.com/jmoiron/sqlx" ) +var mdmCommandsAllowedOrderKeys = common_mysql.OrderKeyAllowlist{ + "command_uuid": "command_uuid", + "request_type": "request_type", + "status": "status", + "updated_at": "updated_at", + "hostname": "hostname", + "host_uuid": "host_uuid", + "name": "name", +} + func (ds *Datastore) GetMDMCommandPlatform(ctx context.Context, commandUUID string) (string, error) { stmt := ` SELECT CASE @@ -103,7 +114,10 @@ func (ds *Datastore) ListMDMCommands( jointStmt, params := getCombinedMDMCommandsQuery(ds, listOpts.Filters.HostIdentifier) jointStmt += ds.whereFilterHostsByTeams(tmFilter, "combined_commands") jointStmt, params = addRequestTypeFilter(jointStmt, &listOpts.Filters, params) - jointStmt, params = appendListOptionsWithCursorToSQL(jointStmt, params, &listOpts.ListOptions) + jointStmt, params, err := appendListOptionsWithCursorToSQLSecure(jointStmt, params, &listOpts.ListOptions, mdmCommandsAllowedOrderKeys) + if err != nil { + return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands") + } var results []*fleet.MDMCommand if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, jointStmt, params...); err != nil { return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands") @@ -357,7 +371,10 @@ WHERE if listOpts.PerPage == 0 { listOpts.PerPage = 10 } - listStmt, params = appendListOptionsWithCursorToSQL(listStmt, params, &listOpts.ListOptions) + listStmt, params, err = appendListOptionsWithCursorToSQLSecure(listStmt, params, &listOpts.ListOptions, mdmCommandsAllowedOrderKeys) + if err != nil { + return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands") + } var results []*fleet.MDMCommand if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, listStmt, params...); err != nil { diff --git a/server/datastore/mysql/mdm_test.go b/server/datastore/mysql/mdm_test.go index 8bc2845f59b..6d36fd5a170 100644 --- a/server/datastore/mysql/mdm_test.go +++ b/server/datastore/mysql/mdm_test.go @@ -37,6 +37,8 @@ func TestMDMShared(t *testing.T) { }{ {"TestMDMCommands", testMDMCommands}, {"TestListMDMCommandsWithTeamFilter", testListMDMCommandsWithTeamFilter}, + {"TestListMDMCommandsOrderKeys", testListMDMCommandsOrderKeys}, + {"TestListMDMAppleCommandsOrderKeys", testListMDMAppleCommandsOrderKeys}, {"TestBatchSetMDMProfiles", testBatchSetMDMProfiles}, {"TestListMDMConfigProfiles", testListMDMConfigProfiles}, {"TestBulkSetPendingMDMHostProfiles", testBulkSetPendingMDMHostProfiles}, @@ -584,6 +586,159 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) { require.ElementsMatch(t, []string{teamCmdUUID, globalCmdUUID}, got) } +func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) { + ctx := t.Context() + + macH, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "ord-host", + OsqueryHostID: ptr.String("ord-osq"), + NodeKey: ptr.String("ord-nk"), + UUID: uuid.NewString(), + Platform: "darwin", + HardwareSerial: "ORDABC", + }) + require.NoError(t, err) + nanoEnroll(t, ds, macH, false) + + commander, _ := createMDMAppleCommanderAndStorage(t, ds) + for range 3 { + err = commander.EnqueueCommand(ctx, []string{macH.UUID}, createRawAppleCmd("ProfileList", uuid.NewString())) + require.NoError(t, err) + } + + for _, key := range []string{"command_uuid", "request_type", "status", "updated_at", "hostname", "host_uuid", "name"} { + t.Run("order_"+key, func(t *testing.T) { + cmds, _, _, err := ds.ListMDMCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: key, PerPage: 5}, + }, + ) + require.NoError(t, err) + require.Len(t, cmds, 3) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, _, _, err := ds.ListMDMCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"}, + }, + ) + require.Error(t, err) + }) + + // the host-identifier branch uses a separate query; confirm it shares the allowlist + t.Run("rejects_unknown_key_host_identifier", func(t *testing.T) { + _, _, _, err := ds.ListMDMCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"}, + Filters: fleet.MDMCommandFilters{HostIdentifier: macH.UUID}, + }, + ) + require.Error(t, err) + }) + + t.Run("after_pagination_with_allowed_key", func(t *testing.T) { + cmds, _, _, err := ds.ListMDMCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "command_uuid", PerPage: 1}, + }, + ) + require.NoError(t, err) + require.Len(t, cmds, 1) + afterCursor := cmds[0].CommandUUID + + next, _, _, err := ds.ListMDMCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "command_uuid", PerPage: 1, After: afterCursor}, + }, + ) + require.NoError(t, err) + require.Len(t, next, 1) + require.NotEqual(t, afterCursor, next[0].CommandUUID) + }) +} + +func testListMDMAppleCommandsOrderKeys(t *testing.T, ds *Datastore) { + ctx := t.Context() + + macH, err := ds.NewHost(ctx, &fleet.Host{ + Hostname: "ord-apple-host", + OsqueryHostID: ptr.String("ord-apple-osq"), + NodeKey: ptr.String("ord-apple-nk"), + UUID: uuid.NewString(), + Platform: "darwin", + HardwareSerial: "ORDA1", + }) + require.NoError(t, err) + nanoEnroll(t, ds, macH, false) + + commander, _ := createMDMAppleCommanderAndStorage(t, ds) + for range 2 { + err = commander.EnqueueCommand(ctx, []string{macH.UUID}, createRawAppleCmd("ProfileList", uuid.NewString())) + require.NoError(t, err) + } + + for _, key := range []string{"command_uuid", "request_type", "status", "updated_at", "hostname", "device_id"} { + t.Run("order_"+key, func(t *testing.T) { + cmds, err := ds.ListMDMAppleCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: key, PerPage: 5}, + }, + ) + require.NoError(t, err) + require.Len(t, cmds, 2) + }) + } + + t.Run("rejects_unknown_key", func(t *testing.T) { + _, err := ds.ListMDMAppleCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"}, + }, + ) + require.Error(t, err) + }) + + t.Run("after_pagination_with_allowed_key", func(t *testing.T) { + cmds, err := ds.ListMDMAppleCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "command_uuid", PerPage: 1}, + }, + ) + require.NoError(t, err) + require.Len(t, cmds, 1) + afterCursor := cmds[0].CommandUUID + + next, err := ds.ListMDMAppleCommands( + ctx, + fleet.TeamFilter{User: test.UserAdmin}, + &fleet.MDMCommandListOptions{ + ListOptions: fleet.ListOptions{OrderKey: "command_uuid", PerPage: 1, After: afterCursor}, + }, + ) + require.NoError(t, err) + require.Len(t, next, 1) + require.NotEqual(t, afterCursor, next[0].CommandUUID) + }) +} + func testBatchSetMDMProfiles(t *testing.T, ds *Datastore) { ctx := context.Background()