Add macOS controls.filevault.prompt_enablement_at to defer FileVault enablement to logout - #48437
Draft
hurricanehrndz wants to merge 19 commits into
Draft
Add macOS controls.filevault.prompt_enablement_at to defer FileVault enablement to logout#48437hurricanehrndz wants to merge 19 commits into
hurricanehrndz wants to merge 19 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #48437 +/- ##
==========================================
- Coverage 67.95% 66.26% -1.69%
==========================================
Files 3677 3677
Lines 233664 233744 +80
Branches 12444 12298 -146
==========================================
- Hits 158785 154890 -3895
- Misses 60636 64891 +4255
+ Partials 14243 13963 -280
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:
|
…, validation Phase 1: introduce macOS-only MDMFileVaultSettings (prompt_enablement_at enum) on app + team MDM config with a login-default resolver, premium gating, and value validation. No behavior change yet; profile template still emits force keys.
Phase 2: wrap the two FileVault2 force keys in a whitespace-controlled
{{ if eq .PromptEnablementAt "login" }} block (login/unset byte-identical to
today; logout drops only those two keys). Resolve the setting team-over-global
at the MDMAppleEnableFileVaultAndEscrow build site.
…g & team APIs Phase 3: round-trip the macOS-only prompt override via GitOps and the general config / team-modify APIs, and re-push the FileVault profile on a prompt-only change while disk encryption stays on (global + all team write paths). A prompt-only change re-pushes without emitting an on/off disk-encryption activity; the dedicated POST /disk_encryption endpoint stays shorthand-only.
…est & UI tolerance Phase 4: tolerate the new macOS-only filevault block in the config/team TS interfaces (not rendered), add an integration test asserting login is byte-stable and logout drops exactly the two FileVault2 force keys in place, document the control in the GitOps reference, and add a changelog fragment.
…e-gitops Emit the macOS-only filevault block in generated GitOps only when prompt_enablement_at is explicitly set, so omitted/default (login) stays out of the YAML and existing output is unchanged. Completes the GitOps round-trip (generate -> apply) for the new setting.
hurricanehrndz
force-pushed
the
filevault-prompt-enablement-at
branch
from
June 29, 2026 18:11
83c0db1 to
3d02d0b
Compare
Contributor
|
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: 📥 CommitsReviewing files that changed from the base of the PR and between 83c0db18e0a2a15b0a42e2db7126f8216fcf8fbc and 3d02d0b. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (22)
WalkthroughAdds a new macOS Fleet Premium configuration control 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
cmd/fleetctl/fleetctl/generate_gitops_test.go (1)
1579-1597: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the global export path too.
These assertions only hit the
no_teambranch. The PR also adds globalfilevaultexport behavior, so a regression in the global controls path could still pass here. Add the same include/omit checks forgenerateControls(nil, "", &mdmConfig)withmdmConfig.FileVaultpopulated from the app config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/fleetctl/fleetctl/generate_gitops_test.go` around lines 1579 - 1597, The current test only validates the no_team branch of generateControls, so it misses regressions in the global filevault export path. Extend the coverage in generate_gitops_test.go by adding the same include/omit assertions for generateControls(nil, "", &mdmConfig) using mdmConfig.FileVault populated from the app config, and verify prompt_enablement_at is exported when set and omitted when unset in that global path.ee/server/service/teams_test.go (1)
848-889: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the prompt value is persisted on successful updates.
These cases prove the re-push/activity behavior, but they never verify that
ModifyTeamactually storesmdm.filevault.prompt_enablement_aton the team. A regression that re-pushes from the request payload but dropsteam.Config.MDM.FileVaultwould still pass here. Capture the saved/returned team and assertPromptEnablementAtin the success cases.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/server/service/teams_test.go` around lines 848 - 889, The prompt-only ModifyTeam test cases verify repush/activity behavior but do not confirm that the team’s FileVault prompt value is actually persisted. In the test setup around ModifyTeam, SaveTeamFunc and the returned team should be captured so the success path can assert team.Config.MDM.FileVault.PromptEnablementAt matches the requested value. Add this assertion for the successful cases to ensure ModifyTeam stores mdm.filevault.prompt_enablement_at on the team, not just in the re-push payload.server/service/appconfig_test.go (1)
219-283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify prompt-only PATCHes stay activity-silent.
ModifyAppConfignow has separate logic to suppressenabled_macos_disk_encryption/disabled_macos_disk_encryptionactivities when onlyfilevault.prompt_enablement_atchanges, but this test only checks the profile push. Count those activity types here too; otherwise that regression path can slip through while the test still passes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/service/appconfig_test.go` around lines 219 - 283, The prompt-only PATCH path in TestModifyAppConfigFileVaultPromptReconcile only verifies the FileVault profile is re-pushed, but it does not assert that ModifyAppConfig stays silent for enabled_macos_disk_encryption and disabled_macos_disk_encryption activities. Extend the test setup to capture activity creation and add assertions for those activity types in both subtests, so a prompt-only change cannot trigger the disk-encryption activity path unnoticed. Use the existing ModifyAppConfig flow and the test helper setup in TestModifyAppConfigFileVaultPromptReconcile to locate where to count these activity events.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/fleetctl/fleetctl/generate_gitops.go`:
- Line 492: The new FileVault seed in generateGitOps is pulling from
cmd.AppConfig.MDM.FileVault, which causes team.ID == 0 to emit the global MDM
value instead of the Unassigned fleet’s actual config. Update the GitOps
generation path in generate_gitops.go so the FileVault field comes from the same
team-scoped MDM source used for other settings in Run, ensuring
fleets/unassigned.yml round-trips team 0 correctly without overriding its prompt
setting.
In `@server/service/appconfig.go`:
- Around line 1729-1734: The FileVault premium check in validateMDM is too broad
and can reject unrelated config updates after a downgrade because it only looks
at the merged value. Update the gate around mdm.FileVault.PromptEnablementAt to
be change-sensitive, like the nearby SSO/setup checks, by comparing the stored
value against the incoming request before appending ErrMissingLicense. Keep the
existing validation for invalid values, but only enforce the premium license
restriction when the PATCH actually modifies filevault.prompt_enablement_at in
appconfig.go’s validateMDM path.
---
Nitpick comments:
In `@cmd/fleetctl/fleetctl/generate_gitops_test.go`:
- Around line 1579-1597: The current test only validates the no_team branch of
generateControls, so it misses regressions in the global filevault export path.
Extend the coverage in generate_gitops_test.go by adding the same include/omit
assertions for generateControls(nil, "", &mdmConfig) using mdmConfig.FileVault
populated from the app config, and verify prompt_enablement_at is exported when
set and omitted when unset in that global path.
In `@ee/server/service/teams_test.go`:
- Around line 848-889: The prompt-only ModifyTeam test cases verify
repush/activity behavior but do not confirm that the team’s FileVault prompt
value is actually persisted. In the test setup around ModifyTeam, SaveTeamFunc
and the returned team should be captured so the success path can assert
team.Config.MDM.FileVault.PromptEnablementAt matches the requested value. Add
this assertion for the successful cases to ensure ModifyTeam stores
mdm.filevault.prompt_enablement_at on the team, not just in the re-push payload.
In `@server/service/appconfig_test.go`:
- Around line 219-283: The prompt-only PATCH path in
TestModifyAppConfigFileVaultPromptReconcile only verifies the FileVault profile
is re-pushed, but it does not assert that ModifyAppConfig stays silent for
enabled_macos_disk_encryption and disabled_macos_disk_encryption activities.
Extend the test setup to capture activity creation and add assertions for those
activity types in both subtests, so a prompt-only change cannot trigger the
disk-encryption activity path unnoticed. Use the existing ModifyAppConfig flow
and the test helper setup in TestModifyAppConfigFileVaultPromptReconcile to
locate where to count these activity events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 14a588a2-e438-49d4-a289-d2e70ba7db30
📥 Commits
Reviewing files that changed from the base of the PR and between b547f1a and 83c0db18e0a2a15b0a42e2db7126f8216fcf8fbc.
⛔ Files ignored due to path filters (1)
docs/Configuration/yaml-files.mdis excluded by!**/*.md
📒 Files selected for processing (22)
changes/filevault-prompt-enablement-atcmd/fleetctl/fleetctl/generate_gitops.gocmd/fleetctl/fleetctl/generate_gitops_test.goee/server/service/mdm.goee/server/service/mdm_external_test.goee/server/service/mdm_profiles.goee/server/service/mdm_profiles_test.goee/server/service/mdm_test.goee/server/service/teams.goee/server/service/teams_test.gofrontend/interfaces/config.tsfrontend/interfaces/team.tspkg/spec/gitops.gopkg/spec/gitops_test.goserver/fleet/app.goserver/fleet/filevault_test.goserver/fleet/teams.goserver/service/appconfig.goserver/service/appconfig_test.goserver/service/client.goserver/service/integration_mdm_lifecycle_test.goserver/service/mdm_test.go
Regenerated via 'go run ./tools/cloner-check/main.go --update' after adding the macOS FileVault settings to AppConfig.MDM and TeamMDM (value types; existing value-copy Clone/Copy remains correct).
Member
|
Thank you, @hurricanehrndz! I'll set this as draft and review per the community PR process: https://fleetdm.com/handbook/engineering#review-a-community-pull-request |
melpike
marked this pull request as draft
June 29, 2026 18:48
Reset disk encryption + delete any existing no-team FileVault profile before enabling, and turn it back off on cleanup, so the shared integration suite doesn't hit a 409 'profile already exists' when this test enables FileVault.
Author
Thank you |
…gitops The FileVault seed in generateGitOps was pulling from cmd.AppConfig.MDM.FileVault, causing team.ID == 0 (Unassigned) to emit the global MDM value instead of the team's actual config. This broke round-trip: generate → apply would override the unassigned fleet's prompt setting with the global value. Use team.Config.MDM.FileVault for team 0, matching how regular teams use their own config. Also adds include/omit test assertions for the global filevault export path.
The premium gate on filevault.prompt_enablement_at only checked the merged config value, rejecting unrelated PATCH requests after a license downgrade when the field was previously set. Compare oldMdm vs mdm before appending ErrMissingLicense, matching the pattern used by adjacent SSO/setup checks. The value-validation for invalid enum values remains unconditional. Also adds activity assertions to the reconcile test (prompt-only change must not emit disk-encryption activities) and a dedicated premium-gate test verifying both the allow and reject paths.
The ModifyTeam prompt-only test verified re-push behavior but did not confirm the team's FileVault prompt value was actually stored. Capture savedTeam from SaveTeamFunc and assert PromptEnablementAt matches the requested value on success paths.
hurricanehrndz
force-pushed
the
filevault-prompt-enablement-at
branch
from
June 29, 2026 19:40
34312cb to
3638dac
Compare
hurricanehrndz
force-pushed
the
filevault-prompt-enablement-at
branch
from
June 29, 2026 21:04
59a8ce1 to
6b85a5c
Compare
Change MDM.FileVault, TeamMDM.FileVault, TeamPayloadMDM.FileVault, and
TeamSpecMDM.FileVault from value type MDMFileVaultSettings to
*MDMFileVaultSettings with json:"filevault,omitempty".
When nil (unset), the field is omitted from JSON entirely, so:
- Golden/expectation tests pass unchanged (no spurious
"filevault":{"prompt_enablement_at":null} in every payload)
- DB round-trip tests pass (no Set:true vs Set:false mismatch)
- Integration tests comparing team/config JSON output pass
Also:
- Nil-guard the resolver and all access sites
- Deep-copy the pointer in AppConfig.Copy() and TeamMDM.Copy()
- Update cloner-check goldens
- Fix lint: ptr.Uint(0)→new(uint), ptr.String→new(...), interface{}→any
hurricanehrndz
force-pushed
the
filevault-prompt-enablement-at
branch
from
June 29, 2026 21:07
6b85a5c to
65cd5ef
Compare
MDMAppleEnableFileVaultAndEscrow used a bare INSERT which 409s when the profile already exists (e.g. prompt-only re-push while disk encryption is already on). Delete the existing profile first so the insert always succeeds — matching the disable+enable pattern used elsewhere. Update mocks and test assertions that now see the additional delete call.
hurricanehrndz
force-pushed
the
filevault-prompt-enablement-at
branch
from
June 30, 2026 01:46
7ddd8ba to
57c936e
Compare
Simplify the team.ID == 0 handling by always assigning team.Config.MDM to mdmConfig (not just FileVault), so all MDM fields come from the team-scoped source. Only agent_options remains gated behind team.ID != 0.
…ablement-at * upstream/main: Website: update report details page (fleetdm#48462) Foreign vitals mapping: Update SCIM integration instructions (fleetdm#48413) Two Product Designers for #g-power-to-pc (fleetdm#48456) Revise user story handling post-design review (fleetdm#48457)
…ablement-at * upstream/main: Zay is DRI of usage statistics (fleetdm#48425)
hurricanehrndz
force-pushed
the
filevault-prompt-enablement-at
branch
from
June 30, 2026 01:53
57c936e to
693bd6f
Compare
47 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: N/A
Summary
Adds a macOS-only
controls.filevault.prompt_enablement_atsetting (login|logout,default
login), layered on the existingenable_disk_encryptionshorthand. By deferringFileVault enablement to logout, Fleet lets an external login-time authorization plugin
(e.g. XCreds, which enables FileVault and writes the Personal Recovery Key to
/var/db/FileVaultPRK.dat) turn FileVault on silently — well before a user would eversee a logout-time prompt — enabling fully silent enablement owned by that plugin.
login(default / unset): byte-identical to today's FileVault profile (force atlogin + Setup Assistant).
logout: drops only the twoFileVault2force keys(
DeferForceAtUserLoginMaxBypassAttempts,ForceEnableInSetupAssistant). Fleet stillescrows the Personal Recovery Key, keeps
dontAllowFDEDisable, and the cert root — sothe external login-time plugin owns turning FileVault on.
enable_disk_encryptionremains the sole on/off (off ⇒ no profile, regardless of thissetting). Exposed via
PATCH /config, team-modify, and GitOps. Premium-gated andvalidated. Windows BitLocker and Linux LUKS are unaffected. The dedicated
POST /disk_encryptionendpoint and the UI stay shorthand-only.Checklist for submitter
changes/.prompt_enablement_ataccepts onlylogin/logout; premium-gated on every write path). No SQL/JS/shell concerns.checked the frontend/CLI — no endpoint paths changed;
login/unset is byte-stable soexisting installs don't churn.
Testing
test proving
logout == loginminus exactly the two force keys; GitOps round-trip;generate-gitopsexport; global + team reconcile re-push on prompt-only change; anintegration test asserting byte-stability and in-place re-push).
plutil -linton both rendered profiles; confirmedlogoutkeepsDefer/Enable=On/escrow/cert/dontAllowFDEDisablewith no force keys.Database migrations
N/A — app/team MDM config is stored as JSON; no schema changes.
New Fleet configuration settings
GitOps checklist:
fleetctl generate-gitops(emitted only when explicitly set).docs/Configuration/yaml-files.md).login); documented as optional.GitOps-mode UI gating is needed (interfaces only tolerate the field on reads).
fleetd/orbit/Fleet Desktop
N/A — server + config profile only; no fleetd/orbit changes.
Summary by CodeRabbit
New Features
mdm.filevault.prompt_enablement_at) with supported values:login(current behavior) orlogout.Bug Fixes