Fix manual-personal enrollment for iOS/iPadOS#48534
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Fixes Apple “personal enrollment” (BYOD) state being lost during subsequent Apple MDM upserts and during macOS osquery “detail-query” ingestion by ensuring is_personal_enrollment is preserved/updated instead of being implicitly forced to false.
Changes:
- macOS osquery ingest: derive
isPersonalEnrollmentfrom the Fleet enrollment profileServerURLquery param (byod=1) instead of hardcodingfalse. - MySQL upsert: ensure
host_mdm.is_personal_enrollmentis updated onON DUPLICATE KEY UPDATE. - Add regression tests covering both the macOS ingest behavior and the MySQL upsert conflict path.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| server/service/osquery_utils/queries.go | Reads byod=1 back from the Fleet ServerURL before stripping query params, preventing macOS ingest from clobbering the personal-enrollment flag. |
| server/service/osquery_utils/queries_test.go | Adds coverage to ensure macOS ingest sets/clears isPersonalEnrollment based on the byod query param (Fleet-only). |
| server/datastore/mysql/apple_mdm.go | Updates host_mdm.is_personal_enrollment on upsert conflicts to keep the flag in sync across re-enrollments. |
| server/datastore/mysql/apple_mdm_test.go | Adds regression test validating is_personal_enrollment flips correctly when a host re-enrolls with different personal/company-owned status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR updates two areas related to Apple MDM personal (BYOD) enrollment. In the MySQL datastore, the host_mdm upsert statement now updates is_personal_enrollment on duplicate key conflicts instead of leaving it unchanged. In osquery ingestion, directIngestMDMMac now derives isPersonalEnrollment from a byod query parameter on the Fleet MDM server URL, rather than hardcoding false, reading it before the URL's query string is cleared. Corresponding datastore and ingestion tests were added to verify the new behavior across insert, update, and non-Fleet scenarios. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48534 +/- ##
==========================================
+ Coverage 67.90% 68.00% +0.09%
==========================================
Files 3678 3678
Lines 233673 233679 +6
Branches 12303 12303
==========================================
+ Hits 158686 158909 +223
+ Misses 60724 60467 -257
- Partials 14263 14303 +40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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.
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves # # Checklist for submitter If some of the following don't apply, delete the relevant line. Unreleased bug, no changes file - [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) - [ ] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results - [ ] Alerted the release DRI if additional load testing is needed <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Personal enrollment status is now preserved and updated correctly when MDM device records change. * macOS MDM ingestion now keeps the BYOD/personal enrollment flag for Fleet devices instead of defaulting it away. * Incoming server URLs continue to have query parameters removed while still retaining the enrollment status used for processing. * **Tests** * Added coverage for personal enrollment updates and macOS ingestion scenarios, including BYOD and non-BYOD cases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Related issue: Resolves #
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Unreleased bug, no changes file
Changes file added for user-visible changes in
changes/,orbit/changes/oree/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
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
Bug Fixes
Tests