Skip to content

Improve filtering on commands endpoints (#44426 -> v4.85.0 - #44446

Merged
JordanMontgomery merged 1 commit into
rc-minor-fleet-v4.85.0from
JM-fix-mdm-command-filter-v4.85.0
Apr 30, 2026
Merged

Improve filtering on commands endpoints (#44426 -> v4.85.0#44446
JordanMontgomery merged 1 commit into
rc-minor-fleet-v4.85.0from
JM-fix-mdm-command-filter-v4.85.0

Conversation

@JordanMontgomery

Copy link
Copy Markdown
Member

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

Cherry-pick of #44426

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes
    files
    for more information.

  • 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.

  • Timeouts are implemented and retries are limited to avoid infinite loops

  • If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes

Testing

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.

<!-- 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>
@JordanMontgomery
JordanMontgomery requested a review from a team as a code owner April 29, 2026 20:11

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@qodo-code-review

Copy link
Copy Markdown
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: aggregate-result

Failed stage: Check for failures [❌]

Failed test name: integration-mdm-mysql9.5.0

Failure summary:

The action failed in the status-aggregation step that scans downloaded artifact status files and
exits non-zero if any contain fail.
- The status file ./integration-mdm-mysql9.5.0-status/status
contained fail (line 228), so the script marked the test integration-mdm-mysql9.5.0 as failed (line
229).
- Because failed_tests was non-empty, the step printed ❌ One or more test jobs failed:
integration-mdm-mysql9.5.0 (line 240) and exited with code 1 (line 241), causing the workflow to
fail.

Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

159:  Artifact download completed successfully.
160:  Extracting artifact entry: /home/runner/work/fleet/fleet/service-mysql8.0.44-status/status
161:  Extracting artifact entry: /home/runner/work/fleet/fleet/vuln-mysql9.5.0-status/status
162:  Artifact download completed successfully.
163:  Artifact download completed successfully.
164:  Extracting artifact entry: /home/runner/work/fleet/fleet/service-mysql9.5.0-status/status
165:  Artifact download completed successfully.
166:  Extracting artifact entry: /home/runner/work/fleet/fleet/scripts-status/status
167:  Artifact download completed successfully.
168:  Extracting artifact entry: /home/runner/work/fleet/fleet/vuln-mysql8.0.44-status/status
169:  Artifact download completed successfully.
170:  Extracting artifact entry: /home/runner/work/fleet/fleet/fleetctl-mysql9.5.0-status/status
171:  Artifact download completed successfully.
172:  Total of 18 artifact(s) downloaded
173:  Download artifact has finished successfully
174:  ##[group]Run failed_tests=""
175:  �[36;1mfailed_tests=""�[0m
176:  �[36;1mstatus_count=0�[0m
177:  �[36;1m# Find all status files (they are in directories like 'fleetctl-mysql8.0.44-status/status')�[0m
178:  �[36;1mfor status_file in $(find ./ -type f -name 'status'); do�[0m
179:  �[36;1m  status_count=$((status_count + 1))�[0m
180:  �[36;1m  # Extract test name from parent directory (e.g., 'fleetctl-mysql8.0.44-status')�[0m
181:  �[36;1m  test_dir=$(basename $(dirname "$status_file"))�[0m
182:  �[36;1m  # Remove '-status' suffix to get the test name�[0m
183:  �[36;1m  test_name="${test_dir%-status}"�[0m
184:  �[36;1m  status_content=$(cat "$status_file")�[0m
185:  �[36;1m  echo "Processing: $status_file (Test: $test_name) with status content: $status_content"�[0m
186:  �[36;1m  if grep -q "fail" "$status_file"; then�[0m
187:  �[36;1m    echo "  ❌ Test failed: $test_name"�[0m
188:  �[36;1m    failed_tests="${failed_tests}${test_name}, "�[0m
189:  �[36;1m  else�[0m
190:  �[36;1m    echo "  ✅ Test passed: $test_name"�[0m
191:  �[36;1m  fi�[0m
192:  �[36;1mdone�[0m
193:  �[36;1mif [[ $status_count -eq 0 ]]; then�[0m
194:  �[36;1m  echo "❌ ERROR: No status files found! This indicates a workflow issue."�[0m
195:  �[36;1m  exit 1�[0m
196:  �[36;1mfi�[0m
197:  �[36;1mif [[ -n "$failed_tests" ]]; then�[0m
198:  �[36;1m  echo "❌ One or more test jobs failed: ${failed_tests%, }"�[0m
199:  �[36;1m  exit 1�[0m
...

213:  ✅ Test passed: integration-core-mysql9.5.0
214:  Processing: ./vuln-mysql9.5.0-status/status (Test: vuln-mysql9.5.0) with status content: success
215:  ✅ Test passed: vuln-mysql9.5.0
216:  Processing: ./fleetctl-mysql9.5.0-status/status (Test: fleetctl-mysql9.5.0) with status content: success
217:  ✅ Test passed: fleetctl-mysql9.5.0
218:  Processing: ./main-mysql8.0.44-status/status (Test: main-mysql8.0.44) with status content: success
219:  ✅ Test passed: main-mysql8.0.44
220:  Processing: ./scripts-status/status (Test: scripts) with status content: success
221:  ✅ Test passed: scripts
222:  Processing: ./fleetctl-mysql8.0.44-status/status (Test: fleetctl-mysql8.0.44) with status content: success
223:  ✅ Test passed: fleetctl-mysql8.0.44
224:  Processing: ./mysql-mysql9.5.0-status/status (Test: mysql-mysql9.5.0) with status content: success
225:  ✅ Test passed: mysql-mysql9.5.0
226:  Processing: ./integration-enterprise-mysql8.0.44-status/status (Test: integration-enterprise-mysql8.0.44) with status content: success
227:  ✅ Test passed: integration-enterprise-mysql8.0.44
228:  Processing: ./integration-mdm-mysql9.5.0-status/status (Test: integration-mdm-mysql9.5.0) with status content: fail
229:  ❌ Test failed: integration-mdm-mysql9.5.0
230:  Processing: ./mysql-mysql8.0.44-status/status (Test: mysql-mysql8.0.44) with status content: success
231:  ✅ Test passed: mysql-mysql8.0.44
232:  Processing: ./integration-enterprise-mysql9.5.0-status/status (Test: integration-enterprise-mysql9.5.0) with status content: success
233:  ✅ Test passed: integration-enterprise-mysql9.5.0
234:  Processing: ./service-mysql8.0.44-status/status (Test: service-mysql8.0.44) with status content: success
235:  ✅ Test passed: service-mysql8.0.44
236:  Processing: ./fast-status/status (Test: fast) with status content: success
237:  ✅ Test passed: fast
238:  Processing: ./vuln-mysql8.0.44-status/status (Test: vuln-mysql8.0.44) with status content: success
239:  ✅ Test passed: vuln-mysql8.0.44
240:  ❌ One or more test jobs failed: integration-mdm-mysql9.5.0
241:  ##[error]Process completed with exit code 1.
242:  Post job cleanup.

@codecov

codecov Bot commented Apr 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.76%. Comparing base (9de8af6) to head (d384701).
⚠️ Report is 10 commits behind head on rc-minor-fleet-v4.85.0.

Additional details and impacted files
@@                    Coverage Diff                     @@
##           rc-minor-fleet-v4.85.0   #44446      +/-   ##
==========================================================
+ Coverage                   66.72%   66.76%   +0.04%     
==========================================================
  Files                        2626     2627       +1     
  Lines                      211198   211328     +130     
  Branches                     9428     9428              
==========================================================
+ Hits                       140924   141097     +173     
+ Misses                      57485    57417      -68     
- Partials                    12789    12814      +25     
Flag Coverage Δ
backend 68.54% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JordanMontgomery
JordanMontgomery merged commit 58f3568 into rc-minor-fleet-v4.85.0 Apr 30, 2026
43 of 45 checks passed
@JordanMontgomery
JordanMontgomery deleted the JM-fix-mdm-command-filter-v4.85.0 branch April 30, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants