windows_mdm: link enrollment row via DevDetail at first management session#46268
Merged
Conversation
…ssion Closes the race after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where mdm_windows_enrollments.host_uuid stayed empty for ~10s while osquery's distributed-read cycle ran directIngestMDMDeviceID Windows. During that gap any server-side lookup keyed on host UUID via MDMWindowsGetEnrolledDeviceWithHostUUID returned NotFound. processIncomingMDMCmds now inspects unlinked enrollments on every management session: it parses any incoming Results for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber, looks up the Windows host by hardware_serial, and updates host_uuid. If still unlinked after processing the incoming message, it appends a Get for that LocURI to the response so the device replies on the next round-trip. The Get is idempotent and reinjected each session until linkage succeeds. The post-link UPN/SCIM/DEP bookkeeping previously inlined in directIngestMDMDeviceIDWindows is extracted into a shared helper (osquery_utils.LinkWindowsHostMDMEnrollment) so both the new SyncML path and the osquery direct-ingest backstop run it exactly once per linkage. New datastore method WindowsHostLiteByHardwareSerial does a Windows-only serial lookup and returns NotFound when two Windows hosts share a serial, so we never mis-link on virtualization-shared SMBIOS values. For Autopilot and Entra-during-OOBE the host record does not exist until fleetd installs later in ESP, so the osquery backstop and the name-based fallback in setup_experience.go remain in place for those flows. Refs #45380
Member
Author
|
@coderabbitai full review |
Member
Author
|
/agentic_review |
Contributor
✅ Actions performedFull review triggered. |
Code Review by Qodo
1.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes a post-enrollment race for Windows (notably BYOD) where mdm_windows_enrollments.host_uuid remains empty until osquery’s next distributed-read cycle, causing server-side lookups by host UUID to miss the enrollment. It makes the SyncML management session proactively link the enrollment to the host by requesting and consuming ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber, and centralizes the “post-link bookkeeping” in a shared helper.
Changes:
- Add a shared
LinkWindowsHostMDMEnrollmenthelper to perform the enrollment→host link plus one-time post-link reconciliation (IDP mapping/SCIM/DEP flag updates). - In Windows MDM SyncML processing, attempt linkage from incoming DevDetail Results and inject an idempotent DevDetail Get when still unlinked.
- Add datastore support for looking up a Windows host by
hardware_serial(with ambiguity protection) plus accompanying tests.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/osquery_utils/queries.go | Extracts shared Windows enrollment↔host linkage + post-link bookkeeping into LinkWindowsHostMDMEnrollment. |
| server/service/microsoft_mdm.go | Adds DevDetail-based linkage attempt during management sessions and reinjects a DevDetail Get until linked. |
| server/service/mdm_test.go | Adds unit coverage for the DevDetail-based linkage behavior (Get injected, link on Results, retry behavior). |
| server/mock/datastore_mock.go | Extends mock datastore with WindowsHostLiteByHardwareSerial support for tests. |
| server/fleet/datastore.go | Adds WindowsHostLiteByHardwareSerial to the datastore interface. |
| server/datastore/mysql/microsoft_mdm.go | Implements Windows-only host lookup by hardware_serial with ambiguity handling. |
| server/datastore/mysql/microsoft_mdm_test.go | Adds MySQL datastore tests for WindowsHostLiteByHardwareSerial. |
| changes/45380-windows-mdm-enrollment-row-linkage | Adds changelog entry describing the race fix and new linkage mechanism. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis pull request implements automatic linkage of unlinked Windows MDM enrollments to Fleet hosts during the first management session using SMBIOS serial data from OMA-DM DevDetail results. Previously, enrollment-to-host linkage relied on osquery distributed-read backfill, which could leave the enrollment unlinked for several seconds. The new mechanism extracts the SMBIOS serial from SyncML DevDetail results, resolves the corresponding Windows host by hardware serial, and establishes the linkage immediately. If the enrollment remains unlinked after processing (no DevDetail result received), the server injects a SyncML Get command for the serial so the device returns it on the next round-trip, enabling linkage in a subsequent session. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 4
🤖 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 `@server/datastore/mysql/microsoft_mdm.go`:
- Line 207: The SELECT for first-session linkage currently uses a replica
connection via ds.reader(ctx) which can return stale NotFound results; change
the read to use the primary connection (e.g., replace ds.reader(ctx) with
ds.writer(ctx) or call ctxdb.RequirePrimary(ctx) before invoking
sqlx.SelectContext) for the sqlx.SelectContext call that queries into &hosts
using stmt and hardwareSerial so this race-sensitive first-session lookup always
hits primary.
In `@server/service/microsoft_mdm.go`:
- Around line 1713-1725: The loop that extracts the SMBIOS serial currently only
inspects the first item (op.Cmd.Items[0]) and must be changed to scan all items
in each Results command: inside the for _, op := range reqMsg.GetOrderedCmds()
block (checking op.Verb == mdm_types.CmdResults and len(op.Cmd.Items) > 0),
iterate over op.Cmd.Items, for each item check item.Source != nil &&
*item.Source == devDetailSMBIOSSerialNumberURI and item.Data != nil, then set
serial = strings.TrimSpace(item.Data.Content) and break out once found; keep the
existing continue guards for non-Results commands unchanged.
- Around line 1739-1747: The call to osquery_utils.LinkWindowsHostMDMEnrollment
can return (false, nil) when another path already linked the DB row, but the
current code only sets enrolledDevice.HostUUID when updated==true; change the
logic in the block around LinkWindowsHostMDMEnrollment (in microsoft_mdm.go) so
that if err==nil you always set enrolledDevice.HostUUID = host.UUID (regardless
of the updated boolean) after the call to LinkWindowsHostMDMEnrollment to ensure
the in-memory enrollment reflects the DB state and prevents requeuing DevDetail
Get or skipping ESP progression.
In `@server/service/osquery_utils/queries.go`:
- Around line 3054-3099: The current flow can leave the enrollment half-linked
if UpdateMDMWindowsEnrollmentsHostUUID returns updated==true but later datastore
actions fail; make the host-UUID update and the follow-up reconciliation
(UpdateMDMInstalledFromDEP, ReplaceHostDeviceMapping,
SetOrUpdateHostSCIMUserMapping/DeleteHostSCIMUserMapping and the
MDMWindowsGetEnrolledDeviceWithDeviceID read) atomic or reliably retryable. Fix
by performing the UpdateMDMWindowsEnrollmentsHostUUID and all subsequent updates
inside a single datastore transaction (or, if transactions aren't available,
implement a compensating rollback or clear the updated flag on failure) so that
either all of UpdateMDMInstalledFromDEP/ReplaceHostDeviceMapping/SCIM mapping
changes commit together with the host UUID or none do; locate this change around
the block that calls UpdateMDMWindowsEnrollmentsHostUUID,
MDMWindowsGetEnrolledDeviceWithDeviceID, UpdateMDMInstalledFromDEP,
ReplaceHostDeviceMapping, SetOrUpdateHostSCIMUserMapping, and
DeleteHostSCIMUserMapping.
🪄 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: 2760b014-2cb9-4110-ae73-830563d94017
📒 Files selected for processing (8)
changes/45380-windows-mdm-enrollment-row-linkageserver/datastore/mysql/microsoft_mdm.goserver/datastore/mysql/microsoft_mdm_test.goserver/fleet/datastore.goserver/mock/datastore_mock.goserver/service/mdm_test.goserver/service/microsoft_mdm.goserver/service/osquery_utils/queries.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #46268 +/- ##
==========================================
- Coverage 66.84% 66.82% -0.02%
==========================================
Files 2761 2773 +12
Lines 220856 223013 +2157
Branches 10879 10879
==========================================
+ Hits 147628 149030 +1402
- Misses 59861 60396 +535
- Partials 13367 13587 +220
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Always refresh enrolledDevice.HostUUID after a successful host lookup + link, even when UpdateMDMWindowsEnrollmentsHostUUID reports updated=false. The "no change" outcome from UpdateMDMWindowsEnrollmentsHostUUID also covers the case where the row was already linked to the same hostUUID by a concurrent path, so the in-memory enrolledDevice must still pick up the linkage to avoid reinjecting a redundant DevDetail Get for the rest of the request. - Scan every Item in every Results command, not just Items[0]. SyncML Results can carry multiple items, so the SMBIOS serial may not be the first one. - Use ds.writer(ctx) for WindowsHostLiteByHardwareSerial. The lookup runs immediately after osquery's enroll write, and replica lag was producing false NotFound results that delayed linkage. - Clarify LinkWindowsHostMDMEnrollment docstring: updated=false also covers "no row matched mdmDeviceID", not only "already linked to this host". - Replace curly quotes with ASCII '' in two comments. Adds two regression tests covering the multi-item Results case and the updated=false in-memory refresh path.
Member
Author
|
@claude review once |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The DevDetail/SMBIOSSerialNumber Get injected by processIncomingMDMCmds
for unlinked enrollments is never inserted into windows_mdm_commands
because it is a protocol-level linkage probe, not a queued command. With
a fresh UUID CmdID each session, the device's Status/Results reply on
the next session left MDMWindowsSaveResponse logging "unmatched Windows
MDM commands" once per session until linkage completed, since BYOD
enrollments in the unlinked window typically have no other pending
commands to suppress the warning.
Use a stable fleet-internal CmdID prefix ("fleet-internal-") for these
server-injected probes. MDMWindowsSaveResponse now filters CmdRefs with
that prefix out before deciding whether to warn, so a properly
functioning linkage probe is silent and real unmatched-command alerts
remain visible. Cross-session correlation is now also possible since the
CmdID is stable.
No DB schema or windows_mdm_commands changes; the probe still lives only
in the SyncML envelope and the message-level audit in
windows_mdm_responses.
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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@server/fleet/datastore.go`:
- Around line 2114-2115: Update the doc comment for
WindowsHostLiteByHardwareSerial to state its unique-match semantics: explain
that it returns a HostLite when exactly one host matches the provided
hardwareSerial, returns NotFound when no host is found, and also returns
NotFound when more than one Windows host shares the same hardwareSerial (i.e.,
duplicate-detection prevents returning an arbitrary match); mention why this
protects against mis-linking on virtualization-shared SMBIOS values so
callers/implementers understand the contract.
🪄 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: 216c9042-ef41-4e48-92ca-269355674316
📒 Files selected for processing (10)
server/datastore/mysql/hosts.goserver/datastore/mysql/microsoft_mdm.goserver/fleet/datastore.goserver/fleet/hosts.goserver/fleet/hosts_test.goserver/fleet/microsoft_mdm.goserver/fleet/microsoft_mdm_test.goserver/service/mdm_test.goserver/service/microsoft_mdm.goserver/service/osquery_utils/queries.go
✅ Files skipped from review due to trivial changes (1)
- server/datastore/mysql/hosts.go
🚧 Files skipped from review as they are similar to previous changes (2)
- server/service/osquery_utils/queries.go
- server/service/microsoft_mdm.go
ksykulev
reviewed
May 29, 2026
| if !updated { | ||
| return false, nil | ||
| } | ||
| device, err := ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, mdmDeviceID) |
Contributor
There was a problem hiding this comment.
Does this need to go to the primary?
Member
Author
There was a problem hiding this comment.
Good catch. I'll explicitly set the hostUUID for the returned device to make sure it doesn't have a stale value. It doesn't seem to be used, but it doesn't hurt to be explicit.
Member
Author
|
@ksykulev ready for re-review |
ksykulev
reviewed
Jun 1, 2026
| @@ -0,0 +1 @@ | |||
| - Fixed a race after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where `mdm_windows_enrollments.host_uuid` stayed empty for several seconds, causing server-side enrollment lookups to miss. The enrollment is now linked to the Fleet host record at the first management session via OMA-DM DevDetail/SMBIOSSerialNumber instead of waiting for osquery's distributed-read backfill. | |||
Contributor
There was a problem hiding this comment.
Lol you can always tell it's generated by Claude when it says "race" vs "race condition".
ksykulev
reviewed
Jun 1, 2026
Co-authored-by: Konstantin Sykulev <konst@sykulev.com>
Member
Author
|
@ksykulev ready for re-review |
ksykulev
approved these changes
Jun 2, 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.
Closes the race after Windows BYOD MDM enrollment (Settings > Access work or school > Connect) where mdm_windows_enrollments.host_uuid stayed empty for ~10s while osquery's distributed-read cycle ran directIngestMDMDeviceID Windows. During that gap any server-side lookup keyed on host UUID via MDMWindowsGetEnrolledDeviceWithHostUUID returned NotFound.
processIncomingMDMCmds now inspects unlinked enrollments on every management session: it parses any incoming Results for ./DevDetail/Ext/Microsoft/SMBIOSSerialNumber, looks up the Windows host by hardware_serial, and updates host_uuid. If still unlinked after processing the incoming message, it appends a Get for that LocURI to the response so the device replies on the next round-trip. The Get is idempotent and reinjected each session until linkage succeeds.
The post-link UPN/SCIM/DEP bookkeeping previously inlined in directIngestMDMDeviceIDWindows is extracted into a shared helper (osquery_utils.LinkWindowsHostMDMEnrollment) so both the new SyncML path and the osquery direct-ingest backstop run it exactly once per linkage.
New datastore method WindowsHostLiteByHardwareSerial does a Windows-only serial lookup and returns NotFound when two Windows hosts share a serial, so we never mis-link on virtualization-shared SMBIOS values.
For Autopilot and Entra-during-OOBE the host record does not exist until fleetd installs later in ESP, so the osquery backstop and the name-based fallback in setup_experience.go remain in place for those flows.
Related issue: Resolves #45380
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Tests