Add Windows managed local account server flow (#48721) - #49924
Merged
Conversation
…managed-local-account
Member
Author
|
@coderabbitai full review |
Member
Author
|
/agentic_review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the server-side support for Windows “managed local admin account” creation/escrow via Orbit, plus schema changes to reuse the existing managed-local-account password table for Windows rows.
Changes:
- Advertise a new Orbit capability and send a new Orbit notification during Windows OOBE/ESP to trigger account creation + escrow.
- Add a new Orbit escrow endpoint and service/datastore plumbing to persist a Windows device-generated password (command UUID nullable).
- Extend host managed-account password retrieval to support Windows while keeping rotation macOS-only.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/orbit.go | Adds Windows OOBE notification gating + new Orbit escrow service endpoint for Windows managed local account password. |
| server/service/hosts.go | Populates managed local account status in Windows host details response. |
| server/service/handler.go | Registers new Orbit route for managed local account escrow. |
| server/mock/service/service_mock.go | Adds mock method for the new escrow service call. |
| server/mock/datastore_mock.go | Adds mock method for the new datastore escrow save call. |
| server/fleet/service.go | Extends service interface with EscrowWindowsManagedLocalAccountPassword. |
| server/fleet/orbit.go | Adds OrbitConfigNotifications.CreateWindowsManagedLocalAccount flag. |
| server/fleet/datastore.go | Extends datastore interface with SaveHostManagedLocalAccountFromEscrow. |
| server/fleet/capabilities.go | Adds CapabilityWindowsManagedLocalAccount and advertises it for Windows Orbit clients. |
| server/fleet/api_orbit.go | Adds request/response types for the new Orbit escrow endpoint. |
| server/datastore/mysql/schema.sql | Updates schema dump: command_uuid becomes nullable in host_managed_local_account_passwords. |
| server/datastore/mysql/migrations/tables/20260724210609_RelaxManagedLocalAccountCommandUUID.go | Migration to relax command_uuid to NULL. |
| server/datastore/mysql/migrations/tables/20260724210609_RelaxManagedLocalAccountCommandUUID_test.go | Verifies migration allows NULL command_uuid inserts. |
| server/datastore/mysql/managed_local_account.go | Adds SaveHostManagedLocalAccountFromEscrow upsert behavior for Windows escrow. |
| server/datastore/mysql/managed_local_account_test.go | Adds tests for escrow-save semantics and exclusion from auto-rotation selection. |
| ee/server/service/hosts.go | Allows Windows in GetHostManagedAccountPassword while keeping rotation macOS-only. |
Comments suppressed due to low confidence (1)
server/service/orbit.go:1537
- This new escrow service method introduces multiple validation/gating branches (missing host context, not an enrolled Windows MDM host, clientError short-circuit, empty/too-long password, setting checks, datastore save + activity). There are tests for EscrowLUKSData in orbit_test.go, but none for EscrowWindowsManagedLocalAccountPassword; adding unit tests would help prevent regressions.
func (svc *Service) EscrowWindowsManagedLocalAccountPassword(ctx context.Context, password string, clientError string) error {
// this is not a user-authenticated endpoint
svc.authz.SkipAuthorization(ctx)
host, ok := hostctx.FromContext(ctx)
if !ok {
return newOsqueryError("internal error: missing host from request context")
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughAdds Windows managed local account capability advertisement, Orbit configuration notification, password escrow and error reporting, enrollment-aware persistence, and host status exposure. Windows passwords can be retrieved without rotation metadata, while rotation remains rejected. Datastore schema and queries support nullable passwords, escrow errors, and enrollment identifiers. Tests cover notification gating, escrow validation, error handling, re-enrollment, host details, and auto-rotation exclusion. Possibly related issues
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #49924 +/- ##
==========================================
- Coverage 68.20% 68.14% -0.07%
==========================================
Files 3943 3936 -7
Lines 251273 251426 +153
Branches 13406 13391 -15
==========================================
- Hits 171390 171341 -49
- Misses 64532 64720 +188
- Partials 15351 15365 +14
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:
|
Member
Author
|
@coderabbitai full review |
Member
Author
|
/agentic_review |
Contributor
✅ Action performedFull review finished. |
This was referenced Jul 28, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
server/service/orbit.go:1580
- The escrow endpoint doesn’t enforce Fleet Premium, so a Windows MDM host could escrow a managed local account password even when the server is running with a Free license. The linked requirements for this feature specify premium gating, and it’s also important to avoid storing this sensitive material when the feature isn’t licensed.
// Eligibility is the host's Windows MDM enrollment
if _, err := svc.ds.MDMWindowsGetEnrolledDeviceWithHostUUID(ctx, host.UUID); err != nil {
if fleet.IsNotFound(err) {
return &fleet.BadRequestError{Message: "managed local account escrow is only supported for Windows MDM hosts"}
}
return ctxerr.Wrap(ctx, err, "verify windows mdm enrollment for managed local account escrow")
}
server/service/orbit.go:619
- This sets CreateWindowsManagedLocalAccount even when the host is not in OOBE (AwaitingConfiguration=None). The linked issue’s acceptance criteria specify this notification should only be set during the OOBE/setup phase (AwaitingConfiguration Pending/Active) to avoid asking already-provisioned devices to recreate/reset the account outside enrollment.
// Ask a capable premium fleetd to create and escrow the Windows managed local admin account when the host's fleet has the
// setting enabled. The request stops once the host escrows a password for this enrollment. Re-enrolling deletes the enrollment
// row and with it the flag, so a re-imaged device is asked again.
if mlaCapable && !state.ManagedLocalAccountEscrowed {
if lic, _ := license.FromContext(ctx); lic != nil && lic.IsPremium() {
server/fleet/orbit.go:61
- The comment for CreateWindowsManagedLocalAccount says it’s set for any Windows MDM host “not only during OOBE”, but the linked issue/task description specifies the notification should only be set during OOBE (AwaitingConfiguration Pending/Active). The comment should match the intended gating to avoid misleading fleetd/server implementers.
// CreateWindowsManagedLocalAccount tells fleetd on Windows to create the hidden managed local admin account and escrow its password.
// Set for any Windows MDM host whose fleet has the setting enabled, not only during OOBE, for hosts whose fleetd advertises
// CapabilityWindowsManagedLocalAccount, and until the host has escrowed a password for its current enrollment.
pkg/str/str.go:39
- TruncateRunes will panic if maxRunes is negative (slice bounds). Since this is a general-purpose helper that may get reused, it should defensively handle maxRunes <= 0 to avoid surprising panics from bad inputs.
func TruncateRunes(s string, maxRunes int) string {
if len(s) <= maxRunes {
// Fast path: a string of at most maxRunes bytes cannot exceed maxRunes characters.
return s
}
if utf8.RuneCountInString(s) <= maxRunes {
return s
}
return string([]rune(s)[:maxRunes])
sharon-fdm
previously approved these changes
Jul 31, 2026
sharon-fdm
left a comment
sharon-fdm
left a comment
Collaborator
There was a problem hiding this comment.
Looks solid. Nice work on the idempotent design.
…mla-server # Conflicts: # server/datastore/mysql/schema.sql
…egenerate schema.sql
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (3)
server/service/orbit.go:619
- This sets CreateWindowsManagedLocalAccount even when awaiting_configuration is None. The linked issue (#48721) specifies only sending the notification during OOBE (awaiting_configuration Pending/Active) to avoid asking already-provisioned/fully enrolled devices to create the account outside setup. Consider scoping the notification to Pending/Active (or update the story/docs if the broader behavior is intended).
// Ask a capable premium fleetd to create and escrow the Windows managed local admin account when the host's fleet has the
// setting enabled. The request stops once the host escrows a password for this enrollment. Re-enrolling deletes the enrollment
// row and with it the flag, so a re-imaged device is asked again.
if mlaCapable && !state.ManagedLocalAccountEscrowed {
if lic, _ := license.FromContext(ctx); lic != nil && lic.IsPremium() {
server/service/orbit.go:1623
- EscrowWindowsManagedLocalAccountPassword currently persists the password even if the premium license or managed-local-account setting has been turned off after the device was notified (it only logs a warning). The linked issue (#48721) calls for rejecting escrow when the setting is off and/or the license is not premium, so this behavior needs an explicit decision: either enforce rejection here (and update tests), or update the story/docs to reflect the “always store to avoid orphaning the on-device account” approach.
// The setting or license may have changed between the notification and this escrow. That does not change what we
// store, only whether it is worth flagging, so this check is best-effort.
if appConfig, err := svc.ds.AppConfig(ctx); err != nil {
svc.logger.ErrorContext(ctx, "load app config to check managed local account setting after escrow", "err", err)
} else if enabled, err := svc.windowsManagedLocalAccountEnabled(ctx, host, appConfig); err != nil {
pkg/str/str.go:34
- TruncateRunes can panic when maxRunes <= 0 (negative slice bound in []rune(s)[:maxRunes]). Since this is a general utility, it should defensively handle non-positive limits.
func TruncateRunes(s string, maxRunes int) string {
if len(s) <= maxRunes {
// Fast path: a string of at most maxRunes bytes cannot exceed maxRunes characters.
return s
}
sharon-fdm
previously approved these changes
Aug 3, 2026
…mla-server # Conflicts: # server/service/orbit_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (4)
server/service/orbit.go:1597
- EscrowWindowsManagedLocalAccountPassword currently only checks for a Windows MDM enrollment row, then proceeds to store passwords (and log activity) without enforcing premium license and/or the managed-local-account setting. The linked issue text calls out rejecting escrow when the team setting is off and when the license is not premium. Please confirm the desired server-side enforcement; if enforcement is required, add explicit checks here (and adjust the tests that expect escrow to succeed when the setting is toggled off).
// Eligibility is the host's Windows MDM enrollment
if _, err := svc.ds.MDMWindowsGetEnrolledDeviceWithHostUUID(ctx, host.UUID); err != nil {
if fleet.IsNotFound(err) {
return &fleet.BadRequestError{Message: "managed local account escrow is only supported for Windows MDM hosts"}
}
pkg/str/str.go:39
- TruncateRunes can panic when maxRunes <= 0 (negative slice bound in []rune(s)[:maxRunes]). Since this is an exported helper, it should defensively handle non-positive limits.
func TruncateRunes(s string, maxRunes int) string {
if len(s) <= maxRunes {
// Fast path: a string of at most maxRunes bytes cannot exceed maxRunes characters.
return s
}
if utf8.RuneCountInString(s) <= maxRunes {
return s
}
return string([]rune(s)[:maxRunes])
server/datastore/mysql/managed_local_account.go:121
- GetHostManagedLocalAccountPassword treats a NULL/empty encrypted_password as not-found, but GetHostManagedLocalAccountStatus considers any non-NULL blob as "has_password". Using LENGTH(...) keeps status/password_available consistent even if an empty blob ever gets persisted.
const stmt = `
SELECT
status,
client_error,
encrypted_password IS NOT NULL AS has_password,
pending_encrypted_password IS NOT NULL AS pending_rotation,
auto_rotate_at
server/service/orbit.go:638
- This sets CreateWindowsManagedLocalAccount for any connected Windows MDM host whenever the capability is present and escrow hasn’t happened yet. The linked issue’s acceptance criteria specify sending this only during OOBE (awaiting_configuration Pending/Active). Please confirm the intended scope and either (a) move this under the Pending/Active branch, or (b) update the story/requirements to match the broader behavior (and ensure fleet/orbit.go’s field comment stays accurate).
// Ask a capable premium fleetd to create and escrow the Windows managed local admin account when the host's fleet has the
// setting enabled. The request stops once the host escrows a password for this enrollment. Re-enrolling deletes the enrollment
// row and with it the flag, so a re-imaged device is asked again.
if mlaCapable && !state.ManagedLocalAccountEscrowed {
if lic, _ := license.FromContext(ctx); lic != nil && lic.IsPremium() {
sharon-fdm
approved these changes
Aug 3, 2026
This was referenced Aug 4, 2026
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
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: Resolves #48721
Part 2 of #43488
Checklist for submitter
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.Testing
Added/updated automated tests
QA'd all new/changed functionality manually
Database migrations
COLLATE utf8mb4_unicode_ci).Summary by CodeRabbit