Fix Windows Delete edge cases with labels.

[getvictor]

Related issue: Resolves #42591

Checklist for submitter

Testing

• Added/updated automated tests

• Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)

• QA'd all new/changed functionality manually

Summary by CodeRabbit

• Bug Fixes
  • Enhanced Windows MDM profile deletion and cleanup to properly handle shared configuration settings across multiple profiles, preventing unintended removal of settings required by other profiles.
  • Improved reliability of profile management when multiple profiles use overlapping configuration settings.

[getvictor]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[Copilot]

Pull request overview

This PR addresses Windows MDM profile deletion/edit edge cases where LocURI <Delete> commands were being over-protected due to team-wide LocURI retention logic, especially in the presence of label-scoped profiles (issue #42591).

Changes:

• Adds a two-pass LocURI “protection” approach (team-wide first, then per-host refinement for label-scoped protectors).
• Introduces helper logic to enqueue supplemental <Delete> commands for hosts where label-scoped protectors don’t apply.
• Adds datastore tests for LocURI protection and for delete generation when LocURIs are removed during profile edits.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
server/datastore/mysql/microsoft_mdm.go	Implements two-pass LocURI protection and per-host label-scoped supplemental delete handling.
server/datastore/mysql/microsoft_mdm_test.go	Adds tests around shared-LocURI protection and edit-time LocURI deletion behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Walkthrough

This change updates Windows MDM profile deletion and editing logic in the MySQL datastore to implement a two-pass LocURI protection strategy accounting for label scoping. The first pass collects protected LocURIs from other active profiles into a protection map with reverse indexing. The second pass identifies which protecting profiles are label-scoped, checks per-host install assignments in the database, and enqueues supplemental delete commands only for hosts where the label-scoped protector does not apply. Two new test cases validate LocURI lifecycle behavior across profile deletion and editing scenarios.

Possibly related PRs

• PR 42206: Implements related two-phase profile removal flow for generating SyncML delete commands from stored profile bytes and reconciling host-profile state changes.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The pull request description is significantly incomplete compared to the required template. Multiple critical sections from the checklist are missing or left unchecked.	Complete all applicable checklist items including: changes file verification, input validation/security checks, testing details, database migration checks, and any fleetd/orbit/Fleet Desktop compatibility verification. Provide explicit confirmation for each applicable item.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'Fix Windows Delete edge cases with labels' directly relates to the main objective of fixing LocURI protection to account for label scoping in Windows MDM profile deletion/editing.
Linked Issues check	✅ Passed	Code changes implement the two-pass label-scoped deletion strategy outlined in issue #42591: pass 1 builds team-wide protection sets, pass 2 applies per-host label-scoped checks before enqueuing supplemental deletes.
Out of Scope Changes check	✅ Passed	All changes focus on Windows MDM profile deletion/edit logic and comprehensive test coverage; no unrelated modifications detected outside the scope of issue #42591.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch windows-delete-2

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

server/datastore/mysql/microsoft_mdm.go (2)

2762-2770: ⚠️ Potential issue | 🟠 Major

Reserved profiles are missing from the edit-time protection set.

Line 2764 only loads existingProfiles for incomingNames, so the loop at Lines 2880-2885 never sees reserved Windows profiles that are implicitly kept but omitted from the request. Any LocURI owned only by one of those reserved profiles is absent from allRetainedURIs, and an edit can emit a <Delete> for a setting Fleet still intends to enforce. Load kept reserved profiles explicitly before building the retained/protector maps.

Also applies to: 2880-2885

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/datastore/mysql/microsoft_mdm.go` around lines 2762 - 2770, The code
only loads existingProfiles for incomingNames, so reserved Windows profiles that
are implicitly kept are never included and their LocURIs may be missing from
allRetainedURIs; modify the logic in the block that builds existingProfiles (use
the same loadExistingProfiles stmt or an additional query) to also explicitly
load reserved/kept profiles for profTeamID (e.g., those marked reserved or with
the known reserved names) and merge them into existingProfiles before building
the retained/protector maps (used later to populate allRetainedURIs and in the
loop around Lines ~2880-2885); ensure you update any variables used in that flow
(incomingNames, existingProfiles, allRetainedURIs) so reserved profiles are
treated as retained.

---

1283-1318: ⚠️ Potential issue | 🟠 Major

Scope the protection set per current team, not across the whole union.

At Line 1285, teamIDs is built as a union of every affected host's current team, and Lines 1296-1317 collapse all protectors into one activeLocURIs set. That set is then reused for every host in targets, so an unscoped profile from team A can suppress deletes for a host still in no-team or team B. Pass 2 won't fix that because it only rechecks label-scoped protectors. Key the protection set by current team, or partition target.hostUUIDs by team before classifying safeURIs.

As per coding guidelines, queries that drive host-specific behavior should be precisely scoped to avoid unintended results.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/datastore/mysql/microsoft_mdm.go` around lines 1283 - 1318, The code
currently builds a single unioned activeLocURIs/locURIToProtectingProfiles from
teamIDs and applies it to every host in targets, letting unscoped profiles on
team A block deletes for hosts in other teams; fix by partitioning targets by
their current team (e.g., group target.hostUUIDs or targets by host.TeamID) and
for each team run the mdm_windows_configuration_profiles query (the
activeProfilesStmt flow that produces activeLocURIs and
locURIToProtectingProfiles via ExtractLocURIsFromProfileBytes and
FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll) to produce per-team maps;
then use the per-team activeLocURIs when classifying safeURIs for hosts in that
team (instead of the single shared activeLocURIs/teamIDs variables).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 2871-2877: The LocURI extraction in the incoming profile loop uses
fleet.ExtractLocURIsFromProfileBytes on p.SyncML without normalizing SCEP
placeholders, so replace occurrences of the
$FLEET_VAR_SCEP_WINDOWS_CERTIFICATE_ID placeholder with the profile UUID (p.UUID
or use incomingNameToUUID[p.Name]) the same way the delete path does before
calling fleet.ExtractLocURIsFromProfileBytes; apply the same normalization
wherever LocURIs are extracted for retained/new/old sets (the blocks populating
allRetainedURIs, oldURIs, and newURIs) and then populate allRetainedURIs and
editLocURIProtectors as before so resolved on-device LocURIs, not raw
placeholders, are used for protection checks.

---

Outside diff comments:
In `@server/datastore/mysql/microsoft_mdm.go`:
- Around line 2762-2770: The code only loads existingProfiles for incomingNames,
so reserved Windows profiles that are implicitly kept are never included and
their LocURIs may be missing from allRetainedURIs; modify the logic in the block
that builds existingProfiles (use the same loadExistingProfiles stmt or an
additional query) to also explicitly load reserved/kept profiles for profTeamID
(e.g., those marked reserved or with the known reserved names) and merge them
into existingProfiles before building the retained/protector maps (used later to
populate allRetainedURIs and in the loop around Lines ~2880-2885); ensure you
update any variables used in that flow (incomingNames, existingProfiles,
allRetainedURIs) so reserved profiles are treated as retained.
- Around line 1283-1318: The code currently builds a single unioned
activeLocURIs/locURIToProtectingProfiles from teamIDs and applies it to every
host in targets, letting unscoped profiles on team A block deletes for hosts in
other teams; fix by partitioning targets by their current team (e.g., group
target.hostUUIDs or targets by host.TeamID) and for each team run the
mdm_windows_configuration_profiles query (the activeProfilesStmt flow that
produces activeLocURIs and locURIToProtectingProfiles via
ExtractLocURIsFromProfileBytes and
FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll) to produce per-team maps;
then use the per-team activeLocURIs when classifying safeURIs for hosts in that
team (instead of the single shared activeLocURIs/teamIDs variables).

🪄 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: 0051a101-2216-40a3-ba14-5e457321de48

📥 Commits

Reviewing files that changed from the base of the PR and between 819aabd and d5b4a3e.

📒 Files selected for processing (2)

• server/datastore/mysql/microsoft_mdm.go
• server/datastore/mysql/microsoft_mdm_test.go

[codecov]

Codecov Report

❌ Patch coverage is 76.92308% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.69%. Comparing base (819aabd) to head (5a5edac).

Files with missing lines	Patch %	Lines
server/datastore/mysql/microsoft_mdm.go	76.92%	23 Missing and 19 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##           windows-delete   #42632      +/-   ##
==================================================
+ Coverage           66.67%   66.69%   +0.01%     
==================================================
  Files                2533     2533              
  Lines              203254   203397     +143     
  Branches             9019     9019              
==================================================
+ Hits               135527   135647     +120     
- Misses              55455    55467      +12     
- Partials            12272    12283      +11     

Flag	Coverage Δ
backend	68.52% <76.92%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.