51260 Derive disk encryption status from per-platform flags - #51981
51260 Derive disk encryption status from per-platform flags#51981JordanMontgomery wants to merge 1 commit into
Conversation
GitHub Actions
Inactive
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughmacOS FileVault status derivation now accepts disk-encryption configuration and reported disk state. Enforcement-only configurations use disk state; other configurations use escrowed-key state. Datastore summaries and host filters use the same mode-specific predicates and join 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The changes address macOS enforce-only and escrow-only derivation, summaries, filters, profile handling, and automated coverage. The provided changes do not show implementation of the linked issue's required Windows/Linux setting-based summary and status updates, or platform-gated aggregate summaries. Resolution Implement and test the Windows and Linux platform-setting changes required by Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 9 files. (2 skipped: 2 too large.) Full details: Description checkExplanation The description includes the related issue, relevant checklist confirmations, automated testing, host-isolation testing, and manual QA details. The changes-file checkbox is left unchecked, but the description is otherwise sufficiently complete and relevant.
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Pull request overview
Updates macOS FileVault status derivation to use escrow-key state or reported disk state based on platform settings.
Changes:
- Adds configuration-aware Go and SQL status derivation.
- Updates summaries and host filters.
- Adds unit, datastore, service, and integration coverage.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
server/fleet/app.go |
Adds enforce-only configuration helper. |
server/fleet/hosts.go |
Derives FileVault status from key or disk state. |
server/fleet/hosts_test.go |
Adds derivation matrix tests. |
server/service/hosts.go |
Loads effective host encryption configuration. |
server/service/hosts_test.go |
Tests per-platform host details. |
server/service/apple_mdm_test.go |
Updates datastore mocks. |
server/service/integration_mdm_test.go |
Tests escrow-only and enforce-only flows. |
server/datastore/mysql/apple_mdm.go |
Updates SQL predicates and summaries. |
server/datastore/mysql/apple_mdm_test.go |
Adds SQL behavior matrix coverage. |
server/datastore/mysql/hosts.go |
Updates host-list filters. |
server/datastore/mysql/labels.go |
Updates label host filters. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| verification := d.keyVerification() | ||
| // logging out lets the deferred FileVault enablement run; rotating | ||
| // produces a key Fleet can escrow | ||
| actionRequired := ActionRequiredRotateKey | ||
| if cfg.MacOSEnforceOnly() { |
| func fileVaultVerificationPredicates(enforceOnly bool) (confirmed, notConfirmed, unknown string) { | ||
| if enforceOnly { | ||
| return `hd.encrypted = 1`, `hd.encrypted = 0`, `hd.encrypted IS NULL` | ||
| } | ||
| return `hdek.decryptable = 1`, |
| return []fleet.HostMDMAppleProfile{*c.fvProf}, nil | ||
| } | ||
|
|
||
| hostDetail, err := svc.getHostDetails(test.UserContext(context.Background(), test.UserAdmin), host, fleet.HostDetailOptions{}) |
Related issue: Resolves #51260
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/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
Minimal manual testing but simulated by updating hdek rows in the DB to verify that e.g. in enforce-only mode it no longer blocks on a non-decryptable or nonexistent key
Summary by CodeRabbit
New Features
Bug Fixes