Add Windows managed local admin account support to fleetd

[getvictor]

Related issue: Resolves #48723

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.

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

Testing

• Added/updated automated tests
• QA'd all new/changed functionality manually

fleetd/orbit/Fleet Desktop

• Verified compatibility with the latest released version of Fleet (see Must rule)
• If the change applies to only one platform, confirmed that runtime.GOOS is used as needed to isolate changes
• Verified that fleetd runs on macOS, Linux and Windows (did not do macOS, but should be the same as Linux)
• Verified auto-update works from the released version of component to the new version (see tools/tuf/test)

Summary by CodeRabbit

• New Features
  • Added support for Fleet-managed local administrator accounts on Windows.
  • When enabled, creates or updates a hidden _fleetadmin account, securely generates a password, and escrows it to Fleet.
  • Reports provisioning errors and supports safe retries without blocking other configuration updates.

[Copilot]

🟡 Not ready to approve

The new password generator has a confirmed crypto/rand error-handling issue and there are unresolved contract/requirement mismatches (length and symbol-class guarantees) that should be reconciled before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (3)

server/fleet/managed_local_account.go:29

• Windows provisioning requirements in the linked issue call for guaranteeing upper/lower/digit/symbol classes. This generator only includes digits/upper/lower (no symbol class), so hosts with policies requiring non-alphanumeric characters will always fail provisioning. Consider adding an allowed symbol set and including it (at least for the Windows variant) so retries can eventually succeed under stricter policies.

const (
	managedAccountDigits    = "23456789"
	managedAccountUppercase = "ABCDEFGHJKLMNPQRSTUVWXYZ"
	managedAccountLowercase = "abcdefghijkmnpqrstuvwxyz"
)

server/fleet/managed_local_account.go:87

• crypto/rand.Read can return an error; ignoring it here risks generating deterministic/low-entropy output if the entropy source fails. The surrounding comment is also inaccurate (rand.Read does not panic on failure). Handle the error explicitly (panic is OK here since the generator can't safely continue without randomness).

		// crypto/rand.Read never returns an error; it crashes the program if the system entropy source
		// fails, so there is no failure mode for a caller to handle.
		_, _ = rand.Read(b[:])

server/fleet/managed_local_account.go:17

• The managed-local-account password generator currently produces 6×4 = 24 characters (plus separators). The linked issue/contract text indicates Windows should use a 32-character password; also server/service/orbit.go documents "fleetd generates 32-character passwords". Please reconcile this (either adjust the generator and its callers/tests, or update the contract/comments if 24 is the intended length).

This issue also appears on line 25 of the same file.

const (
	// managedAccountPasswordGroupCount is the number of character groups in a managed account password.
	managedAccountPasswordGroupCount = 6
	// managedAccountPasswordGroupLen is the number of characters per group.
	managedAccountPasswordGroupLen = 4

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[sharon-fdm]

@getvictor , the password generation was refactored from the macOS-specific code into a shared package, so macOS is also affected by this change (the new function fixes modulo bias and adds character class guarantees). Two macOS callers were updated: password rotation and post-DEP enrollment.

Two questions:

1. Was macOS managed admin account creation and password rotation tested with this change? If not, can we add it to the test plan?
2. How was the Windows side tested? I did not find detailed test steps in the PR or the ticket.

[sharon-fdm]

LGTM!

[getvictor]

@getvictor , the password generation was refactored from the macOS-specific code into a shared package, so macOS is also affected by this change (the new function fixes modulo bias and adds character class guarantees). Two macOS callers were updated: password rotation and post-DEP enrollment.

Two questions:

1. Was macOS managed admin account creation and password rotation tested with this change? If not, can we add it to the test plan?
2. How was the Windows side tested? I did not find detailed test steps in the PR or the ticket.

@sharon-fdm Yes, I'll add macOS admin account creation and password rotation to the test plan.

We have the test plan in the parent story. I tested the flow between fleetd and server with OOBE and already-enrolled hosts. Once I finish the frontend portion, I will redo the full testplan again.

The merge-base changed after approval.

[Copilot]

🟡 Not ready to approve

It includes at least one correctness issue in retry throttling logic and a mismatch with the stated Windows password requirements that can prevent provisioning on some hosts.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (4)

server/fleet/managed_local_account.go:44

• With the added symbol class, GenerateManagedLocalAccountPassword(true) should include it in the enabled classes so the generated password is guaranteed to contain a non-alphanumeric character (as required by the Windows managed account story).

// includeLowercase controls whether lowercase letters appear.
//
// The password is guaranteed to contain at least one character from each enabled class, so the category count never
// depends on chance. That guarantee is applied by discarding and redrawing a password that misses a class, rather than
// by seeding one character per class and shuffling: seeding skews the result towards balanced class counts, while
// redrawing stays exactly uniform over the passwords that satisfy the guarantee. Roughly one draw in 40 is discarded.
func GenerateManagedLocalAccountPassword(includeLowercase bool) string {
	classes := []string{managedAccountDigits, managedAccountUppercase}
	if includeLowercase {
		classes = append(classes, managedAccountLowercase)
	}

server/fleet/managed_local_account_test.go:48

• The Windows test variant should assert the generated password contains at least one symbol when the symbol class is enabled, matching the managed local account requirements.

				if tt.includeLowercase {
					require.True(t, strings.ContainsAny(password, managedAccountLowercase), "no lowercase in %q", password)
				}

orbit/pkg/managedaccount/managedaccount.go:79

• The retry throttle check uses <= r.retryFrequency, which can unintentionally drop retries when retryFrequency is 0 (a common test/default), because time.Since(lastFailure) can be exactly 0. Using a strict < avoids throttling when the frequency is zero and still enforces the window for positive durations.

	// The server re-sends the notification on every config fetch, so without this a host that cannot
	// provision would redo the syscalls and re-post its error every 30 seconds, indefinitely.
	if !r.lastFailure.IsZero() && time.Since(r.lastFailure) <= r.retryFrequency {
		log.Debug().Msg("managed local account: last attempt failed too recently, skipping")
		r.mu.Unlock()
		return nil

server/fleet/managed_local_account_test.go:15

• If the Windows variant is updated to include a symbol class, the unit test alphabet for the Windows case should include that symbol set so the membership assertions remain accurate.

This issue also appears on line 46 of the same file.

	macOSAlphabet := managedAccountDigits + managedAccountUppercase
	windowsAlphabet := macOSAlphabet + managedAccountLowercase

• Files reviewed: 14/14 changed files
• Comments generated: 1
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.