diff --git a/changes/44170-list-mdm-commands-perf b/changes/44170-list-mdm-commands-perf
new file mode 100644
index 0000000000..ede914716d
--- /dev/null
+++ b/changes/44170-list-mdm-commands-perf
@@ -0,0 +1 @@
+- Fixed slow load times and timeouts on the list MDM commands API (`GET /api/v1/fleet/commands`) on Fleet deployments with many Windows hosts. The endpoint now caps `per_page` at 1,000 (default 10) and `page` at 100; requests above either limit return HTTP 400. To traverse beyond 100 pages, use cursor pagination via the `after` query parameter.
diff --git a/changes/44422-list-mdm-commands-host-identifier-after b/changes/44422-list-mdm-commands-host-identifier-after
new file mode 100644
index 0000000000..5e0df6d0fc
--- /dev/null
+++ b/changes/44422-list-mdm-commands-host-identifier-after
@@ -0,0 +1 @@
+- Fixed `GET /api/v1/fleet/commands` returning a SQL error when called with `host_identifier` and the `after` cursor parameter, particularly with `order_key=command_uuid` or `order_key=hostname`.
diff --git a/ee/server/service/orbit.go b/ee/server/service/orbit.go
index a30be399e4..f68ba3a53e 100644
--- a/ee/server/service/orbit.go
+++ b/ee/server/service/orbit.go
@@ -108,6 +108,8 @@ func (svc *Service) GetOrbitSetupExperienceStatus(ctx context.Context, orbitNode
User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)},
}
acctCmds, _, _, err := svc.ds.ListMDMCommands(ctx, adminTeamFilter, &fleet.MDMCommandListOptions{
+ // PerPage 1: only acctCmds[0] is read below.
+ ListOptions: fleet.ListOptions{PerPage: 1},
Filters: fleet.MDMCommandFilters{
HostIdentifier: host.UUID,
RequestType: "AccountConfiguration",
diff --git a/server/datastore/mysql/mdm.go b/server/datastore/mysql/mdm.go
index d9431465a0..2cd330514d 100644
--- a/server/datastore/mysql/mdm.go
+++ b/server/datastore/mysql/mdm.go
@@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"log/slog"
+ "slices"
"strings"
"time"
@@ -29,16 +30,6 @@ import (
// failures don't churn nano commands and profile renders hourly. See issue #44111.
const renewalFailedRetryBackoff = 24 * time.Hour
-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
@@ -60,8 +51,17 @@ END AS platform
return p, nil
}
-func getCombinedMDMCommandsQuery(ds *Datastore, hostFilter string) (string, []interface{}) {
- appleStmt := `
+// getMDMCommandsSubqueries returns the Apple and Windows command-list
+// sub-statements separately. The caller is responsible for wrapping each
+// branch with the per-branch pagination (team filter, request_type filter,
+// cursor predicate, ORDER BY, inner LIMIT) before merging them with
+// UNION ALL. Paginating inside each branch keeps work per branch at
+// O(page_size) instead of O(total commands).
+//
+// These subqueries are only used for the all-hosts listing; host-scoped
+// requests go through listMDMCommandsByHostIdentifier instead.
+func getMDMCommandsSubqueries() (appleStmt, windowsStmt string) {
+ appleStmt = `
SELECT
nvq.id as host_uuid,
nvq.command_uuid,
@@ -81,34 +81,77 @@ WHERE
nvq.active = 1
`
- windowsStmt := `
+ // The Windows sub-statement is itself a UNION ALL of two branches: one
+ // driven by windows_mdm_command_queue (any command pending or in
+ // flight), the other driven by windows_mdm_command_results (any
+ // command that produced a result). Branch B's NOT EXISTS clause
+ // excludes (command_uuid, enrollment_id) pairs already covered by
+ // branch A so the union does not double-count a single command/host
+ // pair.
+ windowsStmt = `
SELECT
mwe.host_uuid,
wmc.command_uuid,
COALESCE(NULLIF(wmcr.status_code, ''), '101') as status,
- COALESCE(wmc.updated_at, wmc.created_at) as updated_at,
+ COALESCE(wmcr.updated_at, wmc.updated_at, wmc.created_at) as updated_at,
wmc.target_loc_uri as request_type,
h.hostname,
h.team_id,
NULL as name
FROM windows_mdm_commands wmc
-LEFT JOIN windows_mdm_command_queue wmcq ON wmcq.command_uuid = wmc.command_uuid
-LEFT JOIN windows_mdm_command_results wmcr ON wmc.command_uuid = wmcr.command_uuid
-INNER JOIN mdm_windows_enrollments mwe ON wmcq.enrollment_id = mwe.id OR wmcr.enrollment_id = mwe.id
+INNER JOIN windows_mdm_command_queue wmcq ON wmcq.command_uuid = wmc.command_uuid
+INNER JOIN mdm_windows_enrollments mwe ON wmcq.enrollment_id = mwe.id
INNER JOIN hosts h ON h.uuid = mwe.host_uuid
+LEFT JOIN windows_mdm_command_results wmcr
+ ON wmcr.command_uuid = wmc.command_uuid AND wmcr.enrollment_id = mwe.id
WHERE TRUE
+
+UNION ALL
+
+SELECT
+ mwe.host_uuid,
+ wmc.command_uuid,
+ COALESCE(NULLIF(wmcr.status_code, ''), '101') as status,
+ COALESCE(wmcr.updated_at, wmc.updated_at, wmc.created_at) as updated_at,
+ wmc.target_loc_uri as request_type,
+ h.hostname,
+ h.team_id,
+ NULL as name
+FROM windows_mdm_commands wmc
+INNER JOIN windows_mdm_command_results wmcr ON wmcr.command_uuid = wmc.command_uuid
+INNER JOIN mdm_windows_enrollments mwe ON wmcr.enrollment_id = mwe.id
+INNER JOIN hosts h ON h.uuid = mwe.host_uuid
+WHERE NOT EXISTS (
+ SELECT 1 FROM windows_mdm_command_queue wmcq2
+ WHERE wmcq2.command_uuid = wmc.command_uuid AND wmcq2.enrollment_id = mwe.id
+)
`
- var params []interface{}
- appleStmtWithFilter, params := ds.whereFilterHostsByIdentifier(hostFilter, appleStmt, params)
- windowsStmtWithFilter, params := ds.whereFilterHostsByIdentifier(hostFilter, windowsStmt, params)
+ return appleStmt, windowsStmt
+}
- stmt := fmt.Sprintf(
+// mdmCommandsOrderAllowlist is the closed set of order_key values accepted
+// by GET /api/v1/fleet/commands and GET /api/v1/fleet/mdm/commands.
+var mdmCommandsOrderAllowlist = common_mysql.OrderKeyAllowlist{
+ "host_uuid": "host_uuid",
+ "command_uuid": "command_uuid",
+ "status": "status",
+ "updated_at": "updated_at",
+ "request_type": "request_type",
+ "hostname": "hostname",
+ "name": "name",
+}
+
+// getCombinedMDMCommandsQuery returns the legacy combined statement
+// (Apple UNION ALL Windows) ending in `WHERE `. Used by getMDMCommand for
+// single-command lookups; the list-commands path builds its own form
+// (see getMDMCommandsSubqueries).
+func getCombinedMDMCommandsQuery() string {
+ appleStmt, windowsStmt := getMDMCommandsSubqueries()
+ return fmt.Sprintf(
`SELECT * FROM ((%s) UNION ALL (%s)) as combined_commands WHERE `,
- appleStmtWithFilter, windowsStmtWithFilter,
+ appleStmt, windowsStmt,
)
-
- return stmt, params
}
func (ds *Datastore) ListMDMCommands(
@@ -116,20 +159,78 @@ func (ds *Datastore) ListMDMCommands(
tmFilter fleet.TeamFilter,
listOpts *fleet.MDMCommandListOptions,
) ([]*fleet.MDMCommand, *int64, *fleet.PaginationMetadata, error) {
- if listOpts != nil && listOpts.Filters.HostIdentifier != "" {
+ if listOpts == nil || listOpts.PerPage == 0 {
+ return nil, nil, nil, ctxerr.Wrap(ctx, errors.New("ListMDMCommands requires listOpts.PerPage > 0"))
+ }
+ if listOpts.Filters.HostIdentifier != "" {
// separate codepath for more performant query by host identifier
return ds.listMDMCommandsByHostIdentifier(ctx, tmFilter, listOpts)
}
- jointStmt, params := getCombinedMDMCommandsQuery(ds, listOpts.Filters.HostIdentifier)
- jointStmt += ds.whereFilterHostsByTeams(tmFilter, "combined_commands")
- jointStmt, params = addRequestTypeFilter(jointStmt, &listOpts.Filters, params)
- jointStmt, params, err := appendListOptionsWithCursorToSQLSecure(jointStmt, params, &listOpts.ListOptions, mdmCommandsAllowedOrderKeys)
- if err != nil {
- return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands")
+ if listOpts.OrderKey == "" {
+ listOpts.OrderKey = "updated_at"
+ listOpts.OrderDirection = fleet.OrderDescending
}
+
+ appleStmt, windowsStmt := getMDMCommandsSubqueries()
+
+ // Per-branch pagination: without this, the UNION ALL would materialize every command on
+ // both sides before pagination, which times out at scale (#44170).
+ innerOpts := listOpts.ListOptions
+ // For page-based pagination, inner LIMIT = page*per_page + per_page;
+ // the secure helper adds +1 because IncludeMetadata is true. For
+ // cursor-based pagination (After != ""), the helper ignores Page, so
+ // don't inflate the inner LIMIT (per_page+1 is sufficient). Inner
+ // Page=0 suppresses the inner OFFSET; the outer wrapper handles
+ // offset slicing.
+ if innerOpts.After == "" {
+ innerOpts.PerPage = innerOpts.PerPage*innerOpts.Page + innerOpts.PerPage
+ }
+ innerOpts.Page = 0
+ innerOpts.IncludeMetadata = true
+
+ paginateBranch := func(branch string, params []any) (string, []any, error) {
+ wrapped := fmt.Sprintf("SELECT * FROM (%s) AS branch WHERE ", branch)
+ wrapped += ds.whereFilterHostsByTeams(tmFilter, "branch")
+ wrapped, params = addRequestTypeFilter(wrapped, &listOpts.Filters, params)
+ return appendListOptionsWithCursorToSQLSecure(wrapped, params, &innerOpts, mdmCommandsOrderAllowlist)
+ }
+
+ // Each branch needs its own params slice; sqlx.SelectContext binds
+ // placeholders left-to-right across the merged statement.
+ var appleParams, windowsParams []any
+ var err error
+ if appleStmt, appleParams, err = paginateBranch(appleStmt, appleParams); err != nil {
+ return nil, nil, nil, ctxerr.Wrap(ctx, err, "paginate apple commands branch")
+ }
+ if windowsStmt, windowsParams, err = paginateBranch(windowsStmt, windowsParams); err != nil {
+ return nil, nil, nil, ctxerr.Wrap(ctx, err, "paginate windows commands branch")
+ }
+
+ mergedStmt := fmt.Sprintf(
+ "SELECT * FROM ((%s) UNION ALL (%s)) AS combined_commands",
+ appleStmt, windowsStmt,
+ )
+ mergedParams := append([]any{}, appleParams...)
+ mergedParams = append(mergedParams, windowsParams...)
+
+ // Outer pagination: ORDER BY + LIMIT + OFFSET only. The cursor
+ // predicate is already applied inside each branch, so clear After
+ // here. If the original request used cursor pagination, also clear
+ // Page so the outer query does not apply an OFFSET on top of the
+ // per-branch cursor filtering.
+ outerOpts := listOpts.ListOptions
+ if outerOpts.After != "" {
+ outerOpts.After = ""
+ outerOpts.Page = 0
+ }
+ mergedStmt, mergedParams, err = appendListOptionsWithCursorToSQLSecure(mergedStmt, mergedParams, &outerOpts, mdmCommandsOrderAllowlist)
+ if err != nil {
+ return nil, nil, nil, ctxerr.Wrap(ctx, err, "merge mdm commands pagination")
+ }
+
var results []*fleet.MDMCommand
- if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, jointStmt, params...); err != nil {
+ if err := sqlx.SelectContext(ctx, ds.reader(ctx), &results, mergedStmt, mergedParams...); err != nil {
return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands")
}
@@ -163,9 +264,9 @@ func (ds *Datastore) listMDMCommandsByHostIdentifier(
// First, search for host by identifier (hostname, uuid, or hardware_serial).
//
- // NOTE: We're not using existing methods like ds.whereFilterHostsByIdentifier,
- // ds.HostIDsByIdentifier, ds.HostLiteByIdentifier because those methods are poorly
- // optimized for the indexes we currently have on the hosts table.
+ // NOTE: We're not using existing methods like ds.HostIDsByIdentifier or
+ // ds.HostLiteByIdentifier because those methods are poorly optimized for
+ // the indexes we currently have on the hosts table.
// They filter with disjunctive conditions like `hostname = ? OR uuid = ?` as well as
// `? IN(hostname, uuid)`. These existing queries aren't really suited for either composite
// indexes or indexes on individual columns, and the optimizer ends up with executions that
@@ -252,10 +353,12 @@ SELECT
ELSE 'pending'
END AS command_status,
request_type,
- nc.name
+ nc.name,
+ h.hostname
FROM
nano_enrollment_queue nq
JOIN nano_commands nc ON nq.command_uuid = nc.command_uuid
+ JOIN hosts h ON h.uuid = nq.id
LEFT JOIN nano_command_results ncr ON nq.id = ncr.id
AND nc.command_uuid = ncr.command_uuid
WHERE
@@ -279,11 +382,13 @@ WHERE
'101' AS status,
'pending' AS command_status,
wc.target_loc_uri AS request_type,
- NULL AS name
+ NULL AS name,
+ h.hostname
FROM
windows_mdm_command_queue wq
JOIN mdm_windows_enrollments mwe ON mwe.id = wq.enrollment_id
JOIN windows_mdm_commands wc ON wc.command_uuid = wq.command_uuid
+ JOIN hosts h ON h.uuid = mwe.host_uuid
WHERE
mwe.host_uuid IN (?)
@@ -316,11 +421,13 @@ WHERE
) >= 400 THEN 'failed'
END AS command_status,
wc.target_loc_uri AS request_type,
- NULL AS name
+ NULL AS name,
+ h.hostname
FROM
windows_mdm_command_results wcr
JOIN mdm_windows_enrollments mwe ON mwe.id = wcr.enrollment_id
JOIN windows_mdm_commands wc ON wc.command_uuid = wcr.command_uuid
+ JOIN hosts h ON h.uuid = mwe.host_uuid
WHERE
mwe.host_uuid IN (?)
@@ -346,19 +453,25 @@ WHERE
var listStmt, countStmt string
var params []any
+ // Wrap in `SELECT * FROM (...) u WHERE TRUE` so the cursor and ORDER BY
+ // predicates resolve against the unambiguous `u` projection — the inner
+ // branches join multiple tables that all expose `command_uuid` / `updated_at`.
+ // `WHERE TRUE` is required because the cursor helper picks AND vs WHERE by
+ // substring-matching "where", picks AND from the inner branches, and would
+ // otherwise emit a dangling `AND`. See https://github.com/fleetdm/fleet/issues/44422.
switch {
case len(appleUUIDs) > 0 && len(winUUIDs) > 0:
- listStmt = fmt.Sprintf(`SELECT * FROM ((%s) UNION ALL (%s)) u`,
+ listStmt = fmt.Sprintf(`SELECT * FROM ((%s) UNION ALL (%s)) u WHERE TRUE`,
appleStmt, winStmt)
countStmt = fmt.Sprintf(`SELECT COUNT(1) FROM ((%s) UNION ALL (%s)) u`, appleStmt, winStmt)
params = append(params, appleParams...)
params = append(params, winParams...)
case len(appleUUIDs) > 0:
- listStmt = appleStmt
+ listStmt = `SELECT * FROM (` + appleStmt + `) u WHERE TRUE`
countStmt = `SELECT COUNT(1) FROM (` + appleStmt + `) u`
params = appleParams
case len(winUUIDs) > 0:
- listStmt = winStmt
+ listStmt = `SELECT * FROM (` + winStmt + `) u WHERE TRUE`
countStmt = `SELECT COUNT(1) FROM (` + winStmt + `) u`
params = winParams
}
@@ -378,12 +491,15 @@ WHERE
// if listOpts.OrderDirection == "" {
// listOpts.OrderDirection = fleet.OrderDescending
// }
- if listOpts.PerPage == 0 {
- listOpts.PerPage = 10
- }
- listStmt, params, err = appendListOptionsWithCursorToSQLSecure(listStmt, params, &listOpts.ListOptions, mdmCommandsAllowedOrderKeys)
+ // Snapshot the params before the cursor helper appends to them. countStmt
+ // has no cursor placeholder, so it must run with the pre-cursor args.
+ countParams := slices.Clone(params)
+ // Validate order_key against the closed allowlist before it reaches
+ // ORDER BY (defense against SQL injection / information disclosure
+ // via arbitrary column references).
+ listStmt, params, err = appendListOptionsWithCursorToSQLSecure(listStmt, params, &listOpts.ListOptions, mdmCommandsOrderAllowlist)
if err != nil {
- return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands")
+ return nil, nil, nil, ctxerr.Wrap(ctx, err, "list commands pagination")
}
var results []*fleet.MDMCommand
@@ -394,15 +510,15 @@ WHERE
var total *int64
if len(listOpts.Filters.CommandStatuses) == 1 && listOpts.Filters.CommandStatuses[0] == fleet.MDMCommandStatusFilterPending {
// Only get count if we only filter by pending
- if err := sqlx.GetContext(ctx, ds.reader(ctx), &total, countStmt, params...); err != nil {
+ if err := sqlx.GetContext(ctx, ds.reader(ctx), &total, countStmt, countParams...); err != nil {
return nil, nil, nil, ctxerr.Wrap(ctx, err, "count commands")
}
}
- // Add hostname and team info to the results based on the host UUIDs.
+ // Hostname is now projected in SQL on both branches; only team_id
+ // still needs to be merged from the prefetched host lookup.
for i := range results {
if host, ok := byUUID[results[i].HostUUID]; ok {
- results[i].Hostname = host.Hostname
results[i].TeamID = host.TeamID
}
}
@@ -451,8 +567,7 @@ func addAppleCommandStatusFilter(stmt string, filter *fleet.MDMCommandFilters, p
}
func (ds *Datastore) getMDMCommand(ctx context.Context, q sqlx.QueryerContext, cmdUUID string) (*fleet.MDMCommand, error) {
- stmt, _ := getCombinedMDMCommandsQuery(ds, "")
- stmt += "command_uuid = ?"
+ stmt := getCombinedMDMCommandsQuery() + "command_uuid = ?"
var cmd fleet.MDMCommand
if err := sqlx.GetContext(ctx, q, &cmd, stmt, cmdUUID); err != nil {
diff --git a/server/datastore/mysql/mdm_test.go b/server/datastore/mysql/mdm_test.go
index 35ab1ed255..8f03a49b37 100644
--- a/server/datastore/mysql/mdm_test.go
+++ b/server/datastore/mysql/mdm_test.go
@@ -19,6 +19,7 @@ import (
microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft"
"github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm"
"github.com/fleetdm/fleet/v4/server/mdm/nanomdm/service/certauth"
+ common_mysql "github.com/fleetdm/fleet/v4/server/platform/mysql"
"github.com/fleetdm/fleet/v4/server/ptr"
"github.com/fleetdm/fleet/v4/server/test"
"github.com/google/uuid"
@@ -39,6 +40,8 @@ func TestMDMShared(t *testing.T) {
{"TestListMDMCommandsWithTeamFilter", testListMDMCommandsWithTeamFilter},
{"TestListMDMCommandsOrderKeys", testListMDMCommandsOrderKeys},
{"TestListMDMAppleCommandsOrderKeys", testListMDMAppleCommandsOrderKeys},
+ {"TestListMDMCommandsRequiresPerPage", testListMDMCommandsRequiresPerPage},
+ {"TestListMDMCommandsPagination", testListMDMCommandsPagination},
{"TestBatchSetMDMProfiles", testBatchSetMDMProfiles},
{"TestListMDMConfigProfiles", testListMDMConfigProfiles},
{"TestBulkSetPendingMDMHostProfiles", testBulkSetPendingMDMHostProfiles},
@@ -76,7 +79,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx := context.Background()
// no commands or devices enrolled => no results
- cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{}, &fleet.MDMCommandListOptions{})
+ cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}})
require.NoError(t, err)
require.Empty(t, cmds)
@@ -134,7 +137,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
cmds, _, _, err = ds.ListMDMCommands(
ctx,
fleet.TeamFilter{User: test.UserAdmin},
- &fleet.MDMCommandListOptions{},
+ &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
)
require.NoError(t, err)
require.Empty(t, cmds)
@@ -152,7 +155,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
cmds, total, _, err := ds.ListMDMCommands(
ctx,
fleet.TeamFilter{User: test.UserAdmin},
- &fleet.MDMCommandListOptions{},
+ &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
)
require.NoError(t, err)
require.Len(t, cmds, 1)
@@ -172,7 +175,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
- ListOptions: fleet.ListOptions{OrderKey: "hostname"},
+ ListOptions: fleet.ListOptions{OrderKey: "hostname", PerPage: 100},
})
require.NoError(t, err)
require.Len(t, cmds, 2)
@@ -211,7 +214,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
- ListOptions: fleet.ListOptions{OrderKey: "hostname"},
+ ListOptions: fleet.ListOptions{OrderKey: "hostname", PerPage: 100},
})
require.NoError(t, err)
require.Len(t, cmds, 2)
@@ -255,6 +258,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
Filters: fleet.MDMCommandFilters{
HostIdentifier: "non-existent",
},
@@ -268,6 +272,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
Filters: fleet.MDMCommandFilters{
RequestType: "non-existent",
},
@@ -336,6 +341,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
Filters: fleet.MDMCommandFilters{
HostIdentifier: tc.identifier,
CommandStatuses: commandStatuses,
@@ -383,10 +389,10 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{OrderKey: "hostname", OrderDirection: fleet.OrderAscending, PerPage: 100},
Filters: fleet.MDMCommandFilters{
RequestType: "InstallProfile",
},
- ListOptions: fleet.ListOptions{OrderKey: "hostname", OrderDirection: fleet.OrderAscending},
},
)
require.NoError(t, err)
@@ -399,6 +405,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
Filters: fleet.MDMCommandFilters{
RequestType: "InstallProfile",
HostIdentifier: macH.UUID,
@@ -414,6 +421,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
Filters: fleet.MDMCommandFilters{
HostIdentifier: "123456",
CommandStatuses: []fleet.MDMCommandStatusFilter{fleet.MDMCommandStatusFilterPending},
@@ -455,6 +463,7 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
Filters: fleet.MDMCommandFilters{
HostIdentifier: macH.UUID,
CommandStatuses: []fleet.MDMCommandStatusFilter{fleet.MDMCommandStatusFilterRan, fleet.MDMCommandStatusFilterFailed},
@@ -486,6 +495,213 @@ func testMDMCommands(t *testing.T, ds *Datastore) {
require.NoError(t, err)
require.Equal(t, true, meta.HasNextResults)
require.Equal(t, false, meta.HasPreviousResults)
+
+ // Cursor pagination on the host-scoped path. Regression test for
+ // https://github.com/fleetdm/fleet/issues/44422:
+ // "Error 1052 (23000): Column '' in where clause is ambiguous".
+ // Covers all three Path A SQL shapes: Apple-only, Windows-only, and
+ // multi-platform (host_identifier resolves to both an Apple and a
+ // Windows host via the shared hostname "test-host").
+ t.Run("cursor_pagination_44422", func(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ hostIdentifier string
+ wantPlatforms []string
+ }{
+ {"apple_only", macH.UUID, []string{"darwin"}},
+ {"multi_platform", "test-host", []string{"darwin", "windows"}},
+ {"windows_only", windowsH.UUID, []string{"windows"}},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ filters := fleet.MDMCommandFilters{HostIdentifier: tc.hostIdentifier}
+
+ // Sanity: confirm the dispatch reaches the expected branch(es)
+ // and there is data to paginate over.
+ all, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{PerPage: 100},
+ Filters: filters,
+ },
+ )
+ require.NoError(t, err)
+ require.NotEmpty(t, all)
+ platforms := map[string]struct{}{}
+ for _, c := range all {
+ switch c.HostUUID {
+ case macH.UUID:
+ platforms["darwin"] = struct{}{}
+ case windowsH.UUID:
+ platforms["windows"] = struct{}{}
+ }
+ }
+ for _, p := range tc.wantPlatforms {
+ _, ok := platforms[p]
+ require.True(t, ok, "expected commands from platform %s", p)
+ }
+
+ // Cursor on updated_at — `updated_at` is exposed by multiple
+ // inner-FROM tables on every Path A branch, so this would fail
+ // with "Column 'updated_at' ... is ambiguous" without the wrap.
+ page1, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 1,
+ OrderKey: "updated_at",
+ OrderDirection: fleet.OrderDescending,
+ },
+ Filters: filters,
+ },
+ )
+ require.NoError(t, err)
+ require.Len(t, page1, 1)
+
+ page2, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 1,
+ OrderKey: "updated_at",
+ OrderDirection: fleet.OrderDescending,
+ After: page1[0].UpdatedAt.Format(time.RFC3339Nano),
+ },
+ Filters: filters,
+ },
+ )
+ // require.NoError is the regression guard. The cursor predicate
+ // is strict (`updated_at < ?`, no tiebreaker), so page2 may be
+ // empty when adjacent commands share an updated_at second — that
+ // is expected, not a regression. Only assert distinctness when
+ // a row did come back.
+ require.NoError(t, err)
+ if len(page2) > 0 {
+ require.NotEqual(t, page1[0].CommandUUID, page2[0].CommandUUID)
+ }
+
+ // Cursor on command_uuid — separate ambiguity manifestation;
+ // `command_uuid` is the join column on every inner table.
+ first, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 1,
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ },
+ Filters: filters,
+ },
+ )
+ require.NoError(t, err)
+ require.Len(t, first, 1)
+
+ next, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 1,
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ After: first[0].CommandUUID,
+ },
+ Filters: filters,
+ },
+ )
+ require.NoError(t, err)
+ require.NotEmpty(t, next)
+ require.NotEqual(t, first[0].CommandUUID, next[0].CommandUUID)
+
+ // Cursor on hostname. All hosts in this test share hostname
+ // "test-host", and the helper's cursor predicate is a strict
+ // `hostname > 'test-host'` (no tiebreaker), so the result is
+ // expected to be empty. The bug would surface as a SQL error
+ // before the empty check.
+ afterHostname, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 5,
+ OrderKey: "hostname",
+ OrderDirection: fleet.OrderAscending,
+ After: "test-host",
+ },
+ Filters: filters,
+ },
+ )
+ require.NoError(t, err)
+ require.Empty(t, afterHostname)
+
+ // Pending-only count + cursor
+ if tc.name == "apple_only" {
+ pendingFilters := fleet.MDMCommandFilters{
+ HostIdentifier: tc.hostIdentifier,
+ CommandStatuses: []fleet.MDMCommandStatusFilter{fleet.MDMCommandStatusFilterPending},
+ }
+ // Establish the unfiltered pending baseline so the post-cursor
+ // assertions don't depend on UUID lexicographic ordering.
+ allPending, totalPending, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 100,
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ IncludeMetadata: true,
+ },
+ Filters: pendingFilters,
+ },
+ )
+ require.NoError(t, err)
+ require.NotNil(t, totalPending)
+ require.Equal(t, int64(len(allPending)), *totalPending)
+ require.Greater(t, len(allPending), 1, "test setup expects multiple pending Apple commands")
+
+ // Drive the cursor from a known pending UUID so the post-cursor
+ // expectation is deterministic.
+ cursorUUID := allPending[0].CommandUUID
+ expectedAfter := allPending[1:]
+
+ afterPending, totalAfter, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ PerPage: 100,
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ After: cursorUUID,
+ IncludeMetadata: true,
+ },
+ Filters: pendingFilters,
+ },
+ )
+ require.NoError(t, err)
+ // total comes from countStmt which has no cursor, so it
+ // must report the full pending count regardless of After.
+ require.NotNil(t, totalAfter)
+ require.Equal(t, *totalPending, *totalAfter)
+ require.Len(t, afterPending, len(expectedAfter))
+ gotUUIDs := make([]string, len(afterPending))
+ for i, c := range afterPending {
+ gotUUIDs[i] = c.CommandUUID
+ }
+ expectedUUIDs := make([]string, len(expectedAfter))
+ for i, c := range expectedAfter {
+ expectedUUIDs[i] = c.CommandUUID
+ }
+ require.Equal(t, expectedUUIDs, gotUUIDs)
+ }
+ })
+ }
+ })
+
}
// testListMDMCommandsWithTeamFilter tests listing MDM commands with team filters
@@ -528,7 +744,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
cmds, _, _, err := ds.ListMDMCommands(
ctx,
fleet.TeamFilter{User: teamUser},
- &fleet.MDMCommandListOptions{},
+ &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
)
require.NoError(t, err)
require.Len(t, cmds, 1)
@@ -538,7 +754,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
cmds, _, _, err = ds.ListMDMCommands(
ctx,
fleet.TeamFilter{User: teamUser, TeamID: &team.ID},
- &fleet.MDMCommandListOptions{},
+ &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
)
require.NoError(t, err)
require.Len(t, cmds, 1)
@@ -566,7 +782,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
cmds, _, _, err = ds.ListMDMCommands(
ctx,
fleet.TeamFilter{User: teamUser},
- &fleet.MDMCommandListOptions{},
+ &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
)
require.NoError(t, err)
require.Len(t, cmds, 1)
@@ -577,7 +793,7 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
cmds, _, _, err = ds.ListMDMCommands(
ctx,
fleet.TeamFilter{User: adminUser},
- &fleet.MDMCommandListOptions{},
+ &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}},
)
require.NoError(t, err)
require.Len(t, cmds, 2)
@@ -588,6 +804,178 @@ func testListMDMCommandsWithTeamFilter(t *testing.T, ds *Datastore) {
require.ElementsMatch(t, []string{teamCmdUUID, globalCmdUUID}, got)
}
+func testListMDMCommandsRequiresPerPage(t *testing.T, ds *Datastore) {
+ ctx := t.Context()
+
+ _, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, nil)
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "PerPage")
+
+ _, _, _, err = ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{})
+ require.Error(t, err)
+ require.Contains(t, err.Error(), "PerPage")
+}
+
+// testListMDMCommandsPagination exercises the all-hosts per-branch
+// pagination across both Apple and Windows branches. The path inflates the
+// inner LIMIT to per_page*page+per_page so that page N is correct even if all
+// matching rows came from a single branch, and the outer wrap clears
+// After/Page when cursor pagination is used.
+func testListMDMCommandsPagination(t *testing.T, ds *Datastore) {
+ ctx := t.Context()
+
+ // Enroll one Windows host and one macOS host.
+ winHost := test.NewHost(t, ds, "paginate-win", "1.2.3.4", "paginate-node-win", uuid.NewString(), time.Now(), test.WithPlatform("windows"))
+ winDeviceID := windowsEnroll(t, ds, winHost)
+ winEnrollment, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, winDeviceID)
+ require.NoError(t, err)
+
+ macHost := test.NewHost(t, ds, "paginate-mac", "1.2.3.5", "paginate-node-mac", uuid.NewString(), time.Now())
+ nanoEnroll(t, ds, macHost, false)
+
+ // Insert 3 Apple commands and 3 Windows commands so both branches contribute.
+ commander, _ := createMDMAppleCommanderAndStorage(t, ds)
+ const totalApple = 3
+ const totalWin = 3
+ allUUIDs := make([]string, 0, totalApple+totalWin)
+ for range totalApple {
+ cmdUUID := uuid.NewString()
+ raw := createRawAppleCmd("ProfileList", cmdUUID)
+ require.NoError(t, commander.EnqueueCommand(ctx, []string{macHost.UUID}, raw))
+ allUUIDs = append(allUUIDs, cmdUUID)
+ }
+ winUUIDs := make([]string, 0, totalWin)
+ for range totalWin {
+ cmdUUID := uuid.NewString()
+ require.NoError(t, ds.MDMWindowsInsertCommandForHosts(ctx, []string{winDeviceID}, &fleet.MDMWindowsCommand{
+ CommandUUID: cmdUUID,
+ RawCommand: []byte(""),
+ TargetLocURI: "./test/uri",
+ }))
+ winUUIDs = append(winUUIDs, cmdUUID)
+ allUUIDs = append(allUUIDs, cmdUUID)
+ }
+
+ // Mark one Windows command as responded so the results-backed branch of the
+ // internal Windows UNION ALL is exercised. The dedupe NOT EXISTS clause
+ // must still keep the command from appearing twice across the pagination.
+ respondedWinUUID := winUUIDs[0]
+ _, err = ds.MDMWindowsSaveResponse(ctx, winEnrollment, fleet.EnrichedSyncML{
+ SyncML: &fleet.SyncML{Raw: []byte("")},
+ CmdRefUUIDToStatus: map[string]fleet.SyncMLCmd{
+ respondedWinUUID: {Data: ptr.String("200")},
+ },
+ CmdRefUUIDs: []string{respondedWinUUID},
+ }, []string{})
+ require.NoError(t, err)
+
+ sort.Strings(allUUIDs)
+ totalCount := len(allUUIDs)
+
+ t.Run("page-based across branches", func(t *testing.T) {
+ // PerPage=2 across 6 commands => 3 pages of 2 each.
+ const perPage = 2
+ seen := make(map[string]bool)
+ for page := uint(0); page*perPage < uint(totalCount); page++ { //nolint:gosec
+ cmds, _, meta, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ Page: page,
+ PerPage: perPage,
+ IncludeMetadata: true,
+ },
+ },
+ )
+ require.NoError(t, err)
+ require.LessOrEqual(t, len(cmds), perPage)
+
+ start := int(page * perPage)
+ end := min(start+perPage, totalCount)
+ expected := allUUIDs[start:end]
+ got := make([]string, 0, len(cmds))
+ for _, c := range cmds {
+ require.False(t, seen[c.CommandUUID], "duplicate command UUID across pages: %s", c.CommandUUID)
+ seen[c.CommandUUID] = true
+ got = append(got, c.CommandUUID)
+ }
+ require.Equal(t, expected, got, "page %d", page)
+
+ require.Equal(t, page > 0, meta.HasPreviousResults, "page %d HasPreviousResults", page)
+ require.Equal(t, end < totalCount, meta.HasNextResults, "page %d HasNextResults", page)
+ }
+ require.Len(t, seen, totalCount)
+ })
+
+ t.Run("cursor walks all rows without overlap", func(t *testing.T) {
+ const perPage = 2
+ seen := make(map[string]bool)
+ var after string
+ for {
+ cmds, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ PerPage: perPage,
+ After: after,
+ IncludeMetadata: true,
+ },
+ },
+ )
+ require.NoError(t, err)
+ if len(cmds) == 0 {
+ break
+ }
+ for _, c := range cmds {
+ require.False(t, seen[c.CommandUUID], "duplicate command UUID across cursor pages: %s", c.CommandUUID)
+ seen[c.CommandUUID] = true
+ }
+ after = cmds[len(cmds)-1].CommandUUID
+ if len(cmds) < perPage {
+ break
+ }
+ }
+ require.Len(t, seen, totalCount)
+ })
+
+ t.Run("outer clears Page when After is set", func(t *testing.T) {
+ // Without the outer-wrap clearing of Page, Page=99 with PerPage=2 would
+ // add OFFSET 198 on top of the cursor filter and return zero rows even
+ // though plenty of rows follow allUUIDs[0]. With the clearing, the call
+ // must return the page-sized slice immediately after the cursor.
+ cmds, _, _, err := ds.ListMDMCommands(
+ ctx,
+ fleet.TeamFilter{User: test.UserAdmin},
+ &fleet.MDMCommandListOptions{
+ ListOptions: fleet.ListOptions{
+ OrderKey: "command_uuid",
+ OrderDirection: fleet.OrderAscending,
+ PerPage: 2,
+ Page: 99,
+ After: allUUIDs[0],
+ IncludeMetadata: true,
+ },
+ },
+ )
+ require.NoError(t, err)
+ require.Equal(t, allUUIDs[1:3], extractCommandUUIDs(cmds))
+ })
+}
+
+func extractCommandUUIDs(cmds []*fleet.MDMCommand) []string {
+ out := make([]string, 0, len(cmds))
+ for _, c := range cmds {
+ out = append(out, c.CommandUUID)
+ }
+ return out
+}
+
func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) {
ctx := t.Context()
@@ -627,10 +1015,12 @@ func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
- ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"},
+ ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column", PerPage: 5},
},
)
require.Error(t, err)
+ var invalidKeyErr common_mysql.InvalidOrderKeyError
+ require.ErrorAs(t, err, &invalidKeyErr)
})
// the host-identifier branch uses a separate query; confirm it shares the allowlist
@@ -639,11 +1029,13 @@ func testListMDMCommandsOrderKeys(t *testing.T, ds *Datastore) {
ctx,
fleet.TeamFilter{User: test.UserAdmin},
&fleet.MDMCommandListOptions{
- ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column"},
+ ListOptions: fleet.ListOptions{OrderKey: "not_a_real_column", PerPage: 5},
Filters: fleet.MDMCommandFilters{HostIdentifier: macH.UUID},
},
)
require.Error(t, err)
+ var invalidKeyErr common_mysql.InvalidOrderKeyError
+ require.ErrorAs(t, err, &invalidKeyErr)
})
t.Run("after_pagination_with_allowed_key", func(t *testing.T) {
@@ -9566,7 +9958,7 @@ func testDeleteMDMProfilesCancelsInstalls(t *testing.T, ds *Datastore) {
cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{
User: test.UserAdmin,
IncludeObserver: true,
- }, &fleet.MDMCommandListOptions{Filters: fleet.MDMCommandFilters{HostIdentifier: host1.UUID}})
+ }, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}, Filters: fleet.MDMCommandFilters{HostIdentifier: host1.UUID}})
require.NoError(t, err)
require.Len(t, cmds, 0)
@@ -9832,7 +10224,7 @@ func testEnqueueCommandWithName(t *testing.T, ds *Datastore) {
require.Equal(t, "Test Profile Name", storedName.String)
// Also verify via ListMDMCommands
- cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{})
+ cmds, _, _, err := ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}})
require.NoError(t, err)
require.Len(t, cmds, 1)
require.NotNil(t, cmds[0].Name)
@@ -9853,7 +10245,7 @@ func testEnqueueCommandWithName(t *testing.T, ds *Datastore) {
// Verify name is null in the API
// Verify ListMDMCommands also exposes nil Name for unnamed commands
- cmds, _, _, err = ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{})
+ cmds, _, _, err = ds.ListMDMCommands(ctx, fleet.TeamFilter{User: test.UserAdmin}, &fleet.MDMCommandListOptions{ListOptions: fleet.ListOptions{PerPage: 100}})
require.NoError(t, err)
require.Len(t, cmds, 2)
diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go
index a30cdf061f..efac0d8236 100644
--- a/server/datastore/mysql/mysql.go
+++ b/server/datastore/mysql/mysql.go
@@ -1110,17 +1110,6 @@ func (ds *Datastore) whereOmitIDs(colName string, omit []uint) string {
return fmt.Sprintf("%s NOT IN (%s)", colName, strings.Join(idStrs, ","))
}
-func (ds *Datastore) whereFilterHostsByIdentifier(identifier, stmt string, params []interface{}) (string, []interface{}) {
- if identifier == "" {
- return stmt, params
- }
-
- stmt += " AND ? IN (h.hostname, h.osquery_host_id, h.node_key, h.uuid, h.hardware_serial)"
- params = append(params, identifier)
-
- return stmt, params
-}
-
// registerTLS adds client certificate configuration to the mysql connection.
func registerTLS(conf config.MysqlConfig) error {
tlsCfg := config.TLS{
diff --git a/server/fleet/mdm.go b/server/fleet/mdm.go
index 9f23c370f7..25ea4f2a3e 100644
--- a/server/fleet/mdm.go
+++ b/server/fleet/mdm.go
@@ -423,6 +423,18 @@ type MDMCommandListOptions struct {
Filters MDMCommandFilters
}
+// Pagination bounds for the list-MDM-commands endpoints (GET /api/v1/fleet/commands and GET /api/v1/fleet/mdm/commands).
+const (
+ // DefaultMDMCommandsPerPage is the per_page value used when none is specified on the request.
+ DefaultMDMCommandsPerPage uint = 10
+ // MaxMDMCommandsPerPage caps per_page so a single request can't scan an unbounded number of command rows.
+ MaxMDMCommandsPerPage uint = 1000
+ // MaxMDMCommandsPage caps the offset (page * per_page) so deep
+ // traversal can't cause a timeout issue. Clients that need to walk the full set
+ // should use cursor pagination via the after query parameter.
+ MaxMDMCommandsPage uint = 100
+)
+
type MDMCommandStatusFilter string
const (
diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go
index 654ade7e84..cb61ce6836 100644
--- a/server/service/integration_mdm_test.go
+++ b/server/service/integration_mdm_test.go
@@ -4017,6 +4017,40 @@ func (s *integrationMDMTestSuite) TestListMDMCommands() {
res = s.DoRaw("GET", fmt.Sprintf("/api/latest/fleet/mdm/commands?host_identifier=%s&command_status=ran", h.UUID), nil, http.StatusBadRequest)
errMsg = extractServerErrorText(res.Body)
require.Contains(t, errMsg, `Currently, "command_status" filter is only available for macOS, iOS, and iPadOS hosts.`)
+
+ // per_page above the documented maximum is rejected with a clear message.
+ res = s.DoRaw("GET", "/api/latest/fleet/mdm/commands?per_page=1001", nil, http.StatusBadRequest)
+ errMsg = extractServerErrorText(res.Body)
+ require.Contains(t, errMsg, "Please set a per_page limit of 1000 or less")
+
+ // per_page at the cap is accepted.
+ s.DoRaw("GET", "/api/latest/fleet/mdm/commands?per_page=1000", nil, http.StatusOK)
+
+ // page above the cap is rejected so the inner LIMIT (page*per_page+per_page+1)
+ // stays bounded.
+ res = s.DoRaw("GET", "/api/latest/fleet/mdm/commands?page=101", nil, http.StatusBadRequest)
+ errMsg = extractServerErrorText(res.Body)
+ require.Contains(t, errMsg, "Please set page to 100 or less")
+
+ // page at the cap is accepted.
+ s.DoRaw("GET", "/api/latest/fleet/mdm/commands?page=100", nil, http.StatusOK)
+
+ // order_key not in the allowlist is rejected by the secure list-options
+ // helper (defense against SQL injection via crafted ORDER BY). The
+ // helper's InvalidOrderKeyError implements the validation-error
+ // interface, so the response is a 422. Both the unscoped path and
+ // the host-scoped path enforce the same allowlist.
+ res = s.DoRaw("GET", "/api/latest/fleet/mdm/commands?order_key=team_id", nil, http.StatusUnprocessableEntity)
+ errMsg = extractServerErrorText(res.Body)
+ require.Contains(t, errMsg, "team_id")
+
+ res = s.DoRaw("GET", fmt.Sprintf("/api/latest/fleet/mdm/commands?host_identifier=%s&order_key=team_id", h.UUID), nil, http.StatusUnprocessableEntity)
+ errMsg = extractServerErrorText(res.Body)
+ require.Contains(t, errMsg, "team_id")
+
+ // order_key=hostname is supported on both paths.
+ s.DoRaw("GET", "/api/latest/fleet/mdm/commands?order_key=hostname", nil, http.StatusOK)
+ s.DoRaw("GET", fmt.Sprintf("/api/latest/fleet/mdm/commands?host_identifier=%s&order_key=hostname", h.UUID), nil, http.StatusOK)
}
func (s *integrationMDMTestSuite) TestMDMWindowsCommandResults() {
diff --git a/server/service/mdm.go b/server/service/mdm.go
index a6787abdaf..be1e01201d 100644
--- a/server/service/mdm.go
+++ b/server/service/mdm.go
@@ -1047,6 +1047,18 @@ func (req listMDMCommandsRequest) DecodeBody(ctx context.Context, r io.Reader, u
}
}
+ if req.ListOptions.PerPage > fleet.MaxMDMCommandsPerPage {
+ return &fleet.BadRequestError{
+ Message: fmt.Sprintf("Request could not be processed. Please set a per_page limit of %d or less.", fleet.MaxMDMCommandsPerPage),
+ }
+ }
+
+ if req.ListOptions.Page > fleet.MaxMDMCommandsPage {
+ return &fleet.BadRequestError{
+ Message: fmt.Sprintf("Request could not be processed. Please set page to %d or less, or use cursor pagination via the after parameter for deep traversal.", fleet.MaxMDMCommandsPage),
+ }
+ }
+
return nil
}
@@ -1061,6 +1073,9 @@ func listMDMCommandsEndpoint(ctx context.Context, request interface{}, svc fleet
}
}
+ if req.ListOptions.PerPage == 0 {
+ req.ListOptions.PerPage = fleet.DefaultMDMCommandsPerPage
+ }
req.ListOptions.IncludeMetadata = true
results, total, meta, err := svc.ListMDMCommands(ctx, &fleet.MDMCommandListOptions{
diff --git a/server/service/orbit.go b/server/service/orbit.go
index ee99b25bf8..c7f3549ea5 100644
--- a/server/service/orbit.go
+++ b/server/service/orbit.go
@@ -687,6 +687,8 @@ func (svc *Service) processReleaseDeviceForOldFleetd(ctx context.Context, host *
User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)},
}
acctCmds, _, _, err := svc.ds.ListMDMCommands(ctx, adminTeamFilter, &fleet.MDMCommandListOptions{
+ // PerPage 1: only acctCmds[0] is read below.
+ ListOptions: fleet.ListOptions{PerPage: 1},
Filters: fleet.MDMCommandFilters{
HostIdentifier: host.UUID,
RequestType: "AccountConfiguration",