Improve filtering on commands endpoints (#44426)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> Provides better errors on invalid/unexpected sort keys passed to `/api/v1/fleet/commands`, `/api/v1/fleet/mdm/commands` and `/api/v1/fleet/mdm/apple/commands` endpoints # 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] Timeouts are implemented and retries are limited to avoid infinite loops - [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) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved validation for invalid `order_key` values on MDM command endpoints (`/api/v1/fleet/commands`, `/api/v1/fleet/mdm/commands`, and `/api/v1/fleet/mdm/apple/commands`), ensuring only approved sorting parameters are accepted. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot Autofix powered by AI
parent
b67ab72349
commit
62b60fef24
@@ -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.
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -37,6 +37,8 @@ func TestMDMShared(t *testing.T) {
|
||||
}{
|
||||
{"TestMDMCommands", testMDMCommands},
|
||||
{"TestListMDMCommandsWithTeamFilter", testListMDMCommandsWithTeamFilter},
|
||||
{"TestListMDMCommandsOrderKeys", testListMDMCommandsOrderKeys},
|
||||
{"TestListMDMAppleCommandsOrderKeys", testListMDMAppleCommandsOrderKeys},
|
||||
{"TestBatchSetMDMProfiles", testBatchSetMDMProfiles},
|
||||
{"TestListMDMConfigProfiles", testListMDMConfigProfiles},
|
||||
{"TestBulkSetPendingMDMHostProfiles", testBulkSetPendingMDMHostProfiles},
|
||||
@@ -586,6 +588,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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user