Fix Windows Delete edge cases with labels. (#42632)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #42591

Docs updated here: https://github.com/fleetdm/fleet/pull/42653/changes

# Checklist for submitter

## Testing

- [x] Added/updated automated tests
- [x] Where appropriate, [automated tests simulate multiple hosts and
test for host
isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing)
(updates to one hosts's records do not affect another)

- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## 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.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Victor Lyuboslavsky 2026-03-31 08:59:16 -05:00 committed by GitHub
parent 30212bc20b
commit a6157c13d6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 665 additions and 56 deletions

View file

@ -7,7 +7,9 @@ import (
"encoding/xml"
"errors"
"fmt"
"maps"
"math"
"slices"
"strings"
"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
@ -1183,6 +1185,14 @@ func deleteMDMWindowsConfigProfile(ctx context.Context, tx sqlx.ExtContext, prof
// - Phase 1: Delete rows that were never sent to the device (NULL status + install)
// - Phase 2: For rows that were sent (non-NULL status + install), generate SyncML
// <Delete> commands and enqueue them, then mark the rows for removal.
//
// IMPORTANT: All profileUUIDs must belong to the same team. The LocURI protection
// set is built by querying all active profiles for the affected team(s). If profiles
// from multiple teams were passed, a profile in team A could suppress deletes for
// hosts in team B (over-protection), because the protection set would be the union
// across teams rather than scoped per-team. All current callers (DeleteMDMWindowsConfigProfile,
// DeleteMDMWindowsConfigProfileByTeamAndName, batchSetMDMWindowsProfilesDB) operate
// on a single team.
func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
ctx context.Context, tx sqlx.ExtContext,
profileUUIDs []string, profileContents map[string][]byte,
@ -1263,10 +1273,20 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
// Generate and enqueue <Delete> commands for each profile.
// Track which profiles were successfully enqueued so we only
// update rows that have a corresponding queued command.
// Collect LocURIs from OTHER active profiles in the same team(s) so we
// Collect LocURIs from OTHER active profiles so we
// don't send <Delete> for settings still enforced by a remaining profile.
// This prevents deleting one profile from undoing settings in another.
activeLocURIs := make(map[string]bool)
//
// This is a two-pass approach for performance:
// Pass 1 (team-wide): Build a global protection set from ALL other profiles
// in the team. This is fast and handles the common case.
// Pass 2 (per-host, only if needed): For any LocURIs that were protected in
// pass 1, check if the protecting profile actually applies to each host
// (considering label scope). If it doesn't, send the <Delete> anyway.
activeLocURIs := make(map[string]struct{})
// Map each protected LocURI to the profile UUIDs that protect it,
// so pass 2 can check per-host applicability.
locURIToProtectingProfiles := make(map[string][]string)
if len(profileUUIDs) > 0 {
// Get team IDs for the profiles being deleted. Use the hosts table
// with a single query instead of one per host.
@ -1281,18 +1301,31 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
if err := sqlx.SelectContext(ctx, tx, &teamIDs, teamIDStmt, teamIDArgs...); err != nil {
return ctxerr.Wrap(ctx, err, "selecting team IDs for LocURI protection")
}
if len(teamIDs) > 1 {
// All callers must operate on a single team. If multiple teams are
// found, the LocURI protection set would be the union across teams,
// letting a profile in team A suppress deletes for hosts in team B.
return ctxerr.Errorf(ctx, "cancelWindowsHostInstallsForDeletedMDMProfiles: expected profiles from 1 team, got %d", len(teamIDs))
}
if len(teamIDs) > 0 {
// Query all SyncML from profiles NOT being deleted in the same team(s).
// Query profile UUIDs and SyncML from profiles NOT being deleted.
const activeProfilesStmt = `
SELECT syncml FROM mdm_windows_configuration_profiles
SELECT profile_uuid, syncml FROM mdm_windows_configuration_profiles
WHERE team_id IN (?) AND profile_uuid NOT IN (?)`
apStmt, apArgs, apErr := sqlx.In(activeProfilesStmt, teamIDs, profileUUIDs)
if apErr == nil {
var activeProfiles [][]byte
var activeProfiles []struct {
ProfileUUID string `db:"profile_uuid"`
SyncML []byte `db:"syncml"`
}
if err := sqlx.SelectContext(ctx, tx, &activeProfiles, apStmt, apArgs...); err == nil {
for _, syncML := range activeProfiles {
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(syncML) {
activeLocURIs[uri] = true
for _, ap := range activeProfiles {
// Substitute SCEP variable so LocURIs are compared on
// resolved paths, consistent with the deleted profile side.
resolved := fleet.FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll(ap.SyncML, []byte(ap.ProfileUUID))
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(resolved) {
activeLocURIs[uri] = struct{}{}
locURIToProtectingProfiles[uri] = append(locURIToProtectingProfiles[uri], ap.ProfileUUID)
}
}
}
@ -1301,25 +1334,47 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
}
enqueuedTargets := make(map[string]*removeTarget)
var pass2Params []locURIProtectionParams
for profUUID, target := range targets {
syncML, ok := profileContents[profUUID]
if !ok || len(syncML) == 0 {
// SyncML content not available — cannot generate delete command.
// This is best-effort, so we log and continue.
ds.logger.WarnContext(ctx, "skipping delete command generation: no SyncML content", "profile.uuid", profUUID)
continue
}
deleteCmd, err := fleet.BuildDeleteCommandFromProfileBytes(syncML, target.cmdUUID, profUUID, activeLocURIs)
// Extract all LocURIs from this profile (done once, reused for pass 2).
allURIs := fleet.ExtractLocURIsFromProfileBytes(
fleet.FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll(syncML, []byte(profUUID)),
)
// Partition into safe (not protected) and protected (in activeLocURIs).
var safeURIs, protectedURIs []string
for _, uri := range allURIs {
if _, isProtected := activeLocURIs[uri]; isProtected {
protectedURIs = append(protectedURIs, uri)
} else {
safeURIs = append(safeURIs, uri)
}
}
// Generate <Delete> commands for the safe (unprotected) LocURIs.
deleteCmd, err := fleet.BuildDeleteCommandFromLocURIs(safeURIs, target.cmdUUID)
if err != nil {
ds.logger.WarnContext(ctx, "skipping delete command generation: build error",
ds.logger.ErrorContext(ctx, "skipping delete command generation: build error",
"profile.uuid", profUUID, "err", err)
ctxerr.Handle(ctx, err)
continue
}
if deleteCmd == nil {
// No delete command needed (profile only has Exec commands, or all
// LocURIs are protected by other active profiles).
// Delete the host-profile rows directly since no command is needed.
if deleteCmd != nil {
// Enqueue the primary delete command for unprotected LocURIs.
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, target.hostUUIDs, deleteCmd); err != nil {
return ctxerr.Wrap(ctx, err, "inserting delete commands for hosts")
}
enqueuedTargets[profUUID] = target
} else {
// No primary delete command (all LocURIs protected or profile only
// has Exec commands). Delete the host-profile rows since the config
// profile is being removed and there's no command to track.
delSkipStmt, delSkipArgs, delSkipErr := sqlx.In(
`DELETE FROM host_mdm_windows_profiles WHERE profile_uuid = ? AND host_uuid IN (?)`,
profUUID, target.hostUUIDs)
@ -1329,13 +1384,25 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
if _, err := tx.ExecContext(ctx, delSkipStmt, delSkipArgs...); err != nil {
return ctxerr.Wrap(ctx, err, "cleaning up protected profile rows")
}
continue
}
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, target.hostUUIDs, deleteCmd); err != nil {
return ctxerr.Wrap(ctx, err, "inserting delete commands for hosts")
// Collect protected URIs for pass 2 (label-scoped check).
if len(protectedURIs) > 0 {
pass2Params = append(pass2Params, locURIProtectionParams{
protectedURIs: protectedURIs,
hostUUIDs: target.hostUUIDs,
})
}
}
// Pass 2: For LocURIs that were protected in pass 1, check if the protecting
// profile is label-scoped and doesn't actually apply to some hosts. If so,
// send supplemental <Delete> commands for those specific hosts.
// This only runs when there are protected LocURIs, which is rare.
if len(pass2Params) > 0 {
if err := ds.checkAndEnqueueLabelScopedDeletes(ctx, tx, pass2Params, locURIToProtectingProfiles); err != nil {
return ctxerr.Wrap(ctx, err, "label-scoped LocURI protection check")
}
enqueuedTargets[profUUID] = target
}
// Update host-profile rows only for profiles that had delete commands enqueued.
@ -1365,6 +1432,174 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
return nil
}
// checkAndEnqueueLabelScopedDeletes identifies which protecting profiles are
// label-scoped and, if any, sends supplemental <Delete> commands for hosts
// where the protector doesn't apply.
func (ds *Datastore) checkAndEnqueueLabelScopedDeletes(
ctx context.Context,
tx sqlx.ExtContext,
toCheck []locURIProtectionParams,
locURIToProtectingProfiles map[string][]string,
) error {
// Collect all protecting profile UUIDs.
allProtectingUUIDs := make(map[string]struct{})
for _, uuids := range locURIToProtectingProfiles {
for _, u := range uuids {
allProtectingUUIDs[u] = struct{}{}
}
}
if len(allProtectingUUIDs) == 0 {
return nil
}
// Check which are label-scoped.
lsStmt, lsArgs, lsErr := sqlx.In(
`SELECT DISTINCT windows_profile_uuid FROM mdm_configuration_profile_labels
WHERE windows_profile_uuid IN (?)`, slices.Collect(maps.Keys(allProtectingUUIDs)))
if lsErr != nil {
return ctxerr.Wrap(ctx, lsErr, "building IN for label-scoped profile check")
}
var labelScoped []string
if err := sqlx.SelectContext(ctx, tx, &labelScoped, lsStmt, lsArgs...); err != nil {
return ctxerr.Wrap(ctx, err, "querying label-scoped profiles")
}
labelScopedProfiles := make(map[string]struct{})
for _, u := range labelScoped {
labelScopedProfiles[u] = struct{}{}
}
if len(labelScopedProfiles) == 0 {
return nil
}
return ds.enqueueSupplementalDeletesForLabelScopedProtection(
ctx, tx, toCheck, locURIToProtectingProfiles, labelScopedProfiles)
}
// locURIProtectionParams holds the data needed by enqueueSupplementalDeletesForLabelScopedProtection.
type locURIProtectionParams struct {
// protectedURIs are the LocURIs from this profile that were filtered
// out by the team-wide protection in pass 1 (i.e., another profile in
// the team also targets them). Pass 2 checks per-host if the protector
// actually applies.
protectedURIs []string
hostUUIDs []string
}
// enqueueSupplementalDeletesForLabelScopedProtection handles pass 2 of
// LocURI protection. For each profile being deleted, it checks if any
// protected LocURIs are only protected by label-scoped profiles. If a
// label-scoped protector doesn't actually apply to a host, a supplemental
// <Delete> is enqueued for that host.
//
// Label type handling (include-any, include-all, exclude-any): this function
// does NOT re-implement label matching logic. Instead, it checks
// host_mdm_windows_profiles for an existing install assignment. The reconciler
// already evaluated all label types when it created those rows, so a row with
// operation_type='install' means the profile applies to that host regardless
// of how the label matching was computed.
//
// This is only called when there are protected LocURIs AND at least one
// protecting profile is label-scoped, which is rare.
func (ds *Datastore) enqueueSupplementalDeletesForLabelScopedProtection(
ctx context.Context,
tx sqlx.ExtContext,
profilesToCheck []locURIProtectionParams,
locURIToProtectingProfiles map[string][]string,
labelScopedProfiles map[string]struct{},
) error {
for _, p := range profilesToCheck {
if len(p.protectedURIs) == 0 || len(p.hostUUIDs) == 0 {
continue
}
// Filter to LocURIs where at least one protector is label-scoped.
var labelProtectedURIs []string
for _, uri := range p.protectedURIs {
for _, protector := range locURIToProtectingProfiles[uri] {
if _, isScoped := labelScopedProfiles[protector]; isScoped {
labelProtectedURIs = append(labelProtectedURIs, uri)
break
}
}
}
if len(labelProtectedURIs) == 0 {
continue
}
// Batch: get which label-scoped protecting profiles are installed on which hosts.
type hostProfile struct {
HostUUID string `db:"host_uuid"`
ProfileUUID string `db:"profile_uuid"`
}
var hostProfs []hostProfile
hpStmt, hpArgs, hpErr := sqlx.In(
`SELECT host_uuid, profile_uuid FROM host_mdm_windows_profiles
WHERE host_uuid IN (?) AND profile_uuid IN (?) AND operation_type = 'install'`,
p.hostUUIDs, slices.Collect(maps.Keys(labelScopedProfiles)))
if hpErr != nil {
return ctxerr.Wrap(ctx, hpErr, "building IN for host-profile label check")
}
if err := sqlx.SelectContext(ctx, tx, &hostProfs, hpStmt, hpArgs...); err != nil {
return ctxerr.Wrap(ctx, err, "querying host-profile assignments for label check")
}
hostHasProfile := make(map[string]map[string]struct{})
for _, hp := range hostProfs {
if hostHasProfile[hp.HostUUID] == nil {
hostHasProfile[hp.HostUUID] = make(map[string]struct{})
}
hostHasProfile[hp.HostUUID][hp.ProfileUUID] = struct{}{}
}
// For each host, determine which protected LocURIs are safe to delete.
// Group hosts by their safe-URI set so we can batch the command insertion.
// Key: sorted comma-joined URIs; Value: list of host UUIDs.
hostsByURISet := make(map[string][]string)
for _, hostUUID := range p.hostUUIDs {
var hostSafeURIs []string
for _, uri := range labelProtectedURIs {
protectorApplies := false
for _, protectorUUID := range locURIToProtectingProfiles[uri] {
if _, isScoped := labelScopedProfiles[protectorUUID]; !isScoped {
protectorApplies = true // non-label profile, always applies
break
}
if _, ok := hostHasProfile[hostUUID][protectorUUID]; ok {
protectorApplies = true
break
}
}
if !protectorApplies {
hostSafeURIs = append(hostSafeURIs, uri)
}
}
if len(hostSafeURIs) > 0 {
slices.Sort(hostSafeURIs)
key := strings.Join(hostSafeURIs, ",")
hostsByURISet[key] = append(hostsByURISet[key], hostUUID)
}
}
// One command per unique URI set, shared across all hosts in the group.
for uriKey, hostUUIDs := range hostsByURISet {
uris := strings.Split(uriKey, ",")
cmdUUID := uuid.NewString()
deleteCmd, err := fleet.BuildDeleteCommandFromLocURIs(uris, cmdUUID)
if err != nil {
return ctxerr.Wrap(ctx, err, "building supplemental delete command")
}
if deleteCmd == nil {
continue
}
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, hostUUIDs, deleteCmd); err != nil {
return ctxerr.Wrap(ctx, err, "enqueuing supplemental delete for label-scoped LocURI")
}
}
}
return nil
}
func (ds *Datastore) DeleteMDMWindowsConfigProfileByTeamAndName(ctx context.Context, teamID *uint, profileName string) error {
var globalOrTeamID uint
if teamID != nil {
@ -2637,21 +2872,70 @@ ON DUPLICATE KEY UPDATE
// The delete commands are best-effort and currently not visible to the
// IT admin in the UI or API. They are fire-and-forget MDM commands
// with no corresponding host_mdm_windows_profiles status entry.
// Collect all LocURIs across incoming profiles AND reserved profiles that
// are kept even when not in the incoming list (e.g. "Windows OS Updates").
// This prevents deleting LocURIs that are still enforced by any profile
// that will remain after the batch-set.
allRetainedURIs := make(map[string]bool)
// Two-pass LocURI protection for edited profiles:
// Pass 1 (team-wide): Build protection set from all retained profiles.
// Pass 2 (per-host): For protected LocURIs where the protector is
// label-scoped, check per-host if it actually applies.
//
// Known limitation: pass 2 runs before the INSERT (line ~2967) and
// batchSetLabelAndVariableAssociations (line ~2987), so:
// (a) Brand-new profiles don't have UUIDs yet (generated by MySQL on
// INSERT), so they appear in allRetainedURIs (pass 1 protects their
// LocURIs) but NOT in editLocURIProtectors. Pass 2 can't check their
// label scope.
// (b) Existing profiles whose label associations change in the same batch
// are checked against stale mdm_configuration_profile_labels rows and
// stale host_mdm_windows_profiles install rows.
// In both cases the result is over-protection: the delete is suppressed on
// all hosts even if the protector doesn't apply. The setting stays enforced
// on hosts outside the protector's label scope. Fixing this requires
// restructuring so pass 2 runs after the INSERT and label association.
allRetainedURIs := make(map[string]struct{})
// Track which profile UUID protects which LocURI for pass 2.
editLocURIProtectors := make(map[string][]string) // uri -> []profileUUID
// Build name-to-UUID lookup for incoming profiles.
incomingNameToUUID := make(map[string]string)
for _, ep := range existingProfiles {
incomingNameToUUID[ep.Name] = ep.ProfileUUID
}
for _, p := range incomingProfs {
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(p.SyncML) {
allRetainedURIs[uri] = true
// Normalize SCEP placeholders so LocURIs are compared on resolved
// paths, consistent with the delete path in cancelWindowsHostInstallsForDeletedMDMProfiles.
resolvedSyncML := p.SyncML
if puuid, ok := incomingNameToUUID[p.Name]; ok {
resolvedSyncML = fleet.FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll(p.SyncML, []byte(puuid))
}
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(resolvedSyncML) {
allRetainedURIs[uri] = struct{}{}
if puuid, ok := incomingNameToUUID[p.Name]; ok {
editLocURIProtectors[uri] = append(editLocURIProtectors[uri], puuid)
}
}
}
// Include LocURIs from reserved profiles that are always kept.
for _, ep := range existingProfiles {
if _, isReserved := mdm.FleetReservedProfileNames()[ep.Name]; isReserved {
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(ep.SyncML) {
allRetainedURIs[uri] = true
// Include LocURIs from reserved profiles that are always kept. Reserved
// profiles may not be in existingProfiles (which only loads profiles
// matching incomingNames), so query them separately.
reservedNames := mdm.ListFleetReservedWindowsProfileNames()
if len(reservedNames) > 0 {
rpStmt, rpArgs, rpErr := sqlx.In(
`SELECT profile_uuid, syncml FROM mdm_windows_configuration_profiles WHERE team_id = ? AND name IN (?)`,
profTeamID, reservedNames)
if rpErr != nil {
return false, ctxerr.Wrap(ctx, rpErr, "building IN for reserved profiles query")
}
var reservedProfiles []struct {
ProfileUUID string `db:"profile_uuid"`
SyncML []byte `db:"syncml"`
}
if err := sqlx.SelectContext(ctx, tx, &reservedProfiles, rpStmt, rpArgs...); err != nil {
return false, ctxerr.Wrap(ctx, err, "querying reserved profiles for LocURI protection")
}
for _, rp := range reservedProfiles {
resolved := fleet.FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll(rp.SyncML, []byte(rp.ProfileUUID))
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(resolved) {
allRetainedURIs[uri] = struct{}{}
editLocURIProtectors[uri] = append(editLocURIProtectors[uri], rp.ProfileUUID)
}
}
}
@ -2662,45 +2946,66 @@ ON DUPLICATE KEY UPDATE
continue
}
oldURIs := fleet.ExtractLocURIsFromProfileBytes(existing.SyncML)
newURIs := fleet.ExtractLocURIsFromProfileBytes(incoming.SyncML)
// Normalize SCEP placeholders for consistent LocURI comparison.
resolvedOld := fleet.FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll(existing.SyncML, []byte(existing.ProfileUUID))
resolvedNew := fleet.FleetVarSCEPWindowsCertificateIDRegexp.ReplaceAll(incoming.SyncML, []byte(existing.ProfileUUID))
oldURIs := fleet.ExtractLocURIsFromProfileBytes(resolvedOld)
newURIs := fleet.ExtractLocURIsFromProfileBytes(resolvedNew)
newSet := make(map[string]bool, len(newURIs))
for _, u := range newURIs {
newSet[u] = true
}
// Pass 1: team-wide protection.
var removedURIs []string
var protectedURIs []string
for _, u := range oldURIs {
// Skip if the LocURI is still in the updated version of this profile,
// or if another profile in the batch still targets it.
if !newSet[u] && !allRetainedURIs[u] {
if newSet[u] {
continue // still in updated profile
}
if _, ok := allRetainedURIs[u]; ok {
protectedURIs = append(protectedURIs, u)
} else {
removedURIs = append(removedURIs, u)
}
}
if len(removedURIs) == 0 {
continue
}
cmdUUID := uuid.NewString()
deleteCmd, err := fleet.BuildDeleteCommandFromLocURIs(removedURIs, cmdUUID)
if err != nil || deleteCmd == nil {
continue
}
// Find hosts that have this profile installed (not pending removal).
var hostUUIDs []string
if err := sqlx.SelectContext(ctx, tx, &hostUUIDs,
`SELECT host_uuid FROM host_mdm_windows_profiles WHERE profile_uuid = ? AND operation_type = ? AND status IS NOT NULL`,
existing.ProfileUUID, fleet.MDMOperationTypeInstall); err != nil {
return false, ctxerr.Wrap(ctx, err, "selecting hosts for edited profile LocURI cleanup")
if len(removedURIs) > 0 || len(protectedURIs) > 0 {
if err := sqlx.SelectContext(ctx, tx, &hostUUIDs,
`SELECT host_uuid FROM host_mdm_windows_profiles WHERE profile_uuid = ? AND operation_type = ? AND status IS NOT NULL`,
existing.ProfileUUID, fleet.MDMOperationTypeInstall); err != nil {
return false, ctxerr.Wrap(ctx, err, "selecting hosts for edited profile LocURI cleanup")
}
}
if len(hostUUIDs) > 0 {
ds.logger.InfoContext(ctx, "sending delete commands for LocURIs removed from edited profile",
"profile.name", existing.Name, "profile.uuid", existing.ProfileUUID, "removed_loc_uris", len(removedURIs))
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, hostUUIDs, deleteCmd); err != nil {
return false, ctxerr.Wrap(ctx, err, "inserting delete commands for removed LocURIs")
// Send deletes for unprotected LocURIs (applies to all hosts).
if len(removedURIs) > 0 && len(hostUUIDs) > 0 {
cmdUUID := uuid.NewString()
deleteCmd, err := fleet.BuildDeleteCommandFromLocURIs(removedURIs, cmdUUID)
if err == nil && deleteCmd != nil {
ds.logger.InfoContext(ctx, "sending delete commands for LocURIs removed from edited profile",
"profile.name", existing.Name, "profile.uuid", existing.ProfileUUID, "removed_loc_uris", len(removedURIs))
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, hostUUIDs, deleteCmd); err != nil {
return false, ctxerr.Wrap(ctx, err, "inserting delete commands for removed LocURIs")
}
}
}
// Pass 2: for protected LocURIs where the protector is label-scoped,
// check per-host if the protector actually applies.
if len(protectedURIs) > 0 && len(hostUUIDs) > 0 {
if err := ds.checkAndEnqueueLabelScopedDeletes(
ctx, tx,
[]locURIProtectionParams{{
protectedURIs: protectedURIs,
hostUUIDs: hostUUIDs,
}},
editLocURIProtectors,
); err != nil {
return false, ctxerr.Wrap(ctx, err, "label-scoped LocURI protection check for edited profile")
}
}
}

View file

@ -52,6 +52,8 @@ func TestMDMWindows(t *testing.T) {
{"TestWindowsMDMManagedSCEPCertificates", testWindowsMDMManagedSCEPCertificates},
{"TestGetWindowsMDMCommandsForResending", testGetWindowsMDMCommandsForResending},
{"TestResendWindowsMDMCommand", testResendWindowsMDMCommand},
{"TestDeleteProfileLocURIProtection", testDeleteProfileLocURIProtection},
{"TestEditProfileDeletesRemovedLocURIs", testEditProfileDeletesRemovedLocURIs},
}
for _, c := range cases {
@ -4065,3 +4067,305 @@ func testResendWindowsMDMCommand(t *testing.T, ds *Datastore) {
require.True(t, detail.Valid, "Host profile detail should be cleared on resend")
assert.Empty(t, detail.String, "Host profile detail should be cleared on resend")
}
func testDeleteProfileLocURIProtection(t *testing.T, ds *Datastore) {
ctx := t.Context()
h1 := test.NewHost(t, ds, "host1", "10.0.0.1", uuid.NewString(), uuid.NewString(), time.Now(), test.WithPlatform("windows"))
windowsEnroll(t, ds, h1)
h2 := test.NewHost(t, ds, "host2", "10.0.0.2", uuid.NewString(), uuid.NewString(), time.Now(), test.WithPlatform("windows"))
windowsEnroll(t, ds, h2)
// Profile A: LocURIs X, Y. Profile B: LocURI Y (shared with A).
profA := &fleet.MDMWindowsConfigProfile{Name: "profA", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/X</LocURI></Target><Data>1</Data></Item></Replace><Replace><Item><Target><LocURI>./Device/Y</LocURI></Target><Data>1</Data></Item></Replace>`)}
profB := &fleet.MDMWindowsConfigProfile{Name: "profB", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/Y</LocURI></Target><Data>2</Data></Item></Replace>`)}
// Insert both profiles.
var profAUUID, profBUUID string
err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profA, profB}, nil)
return err
})
require.NoError(t, err)
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &profAUUID, `SELECT profile_uuid FROM mdm_windows_configuration_profiles WHERE name = 'profA'`)
})
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &profBUUID, `SELECT profile_uuid FROM mdm_windows_configuration_profiles WHERE name = 'profB'`)
})
// Simulate both profiles installed on both hosts.
verified := fleet.MDMDeliveryVerified
var hostProfilePayloads []*fleet.MDMWindowsBulkUpsertHostProfilePayload
for _, hUUID := range []string{h1.UUID, h2.UUID} {
for _, pUUID := range []string{profAUUID, profBUUID} {
hostProfilePayloads = append(hostProfilePayloads, &fleet.MDMWindowsBulkUpsertHostProfilePayload{
ProfileUUID: pUUID,
ProfileName: "test",
HostUUID: hUUID,
CommandUUID: uuid.NewString(),
OperationType: fleet.MDMOperationTypeInstall,
Status: &verified,
Checksum: []byte{0},
})
}
}
err = ds.BulkUpsertMDMWindowsHostProfiles(ctx, hostProfilePayloads)
require.NoError(t, err)
t.Run("shared LocURI not deleted when other profile uses it", func(t *testing.T) {
t.Cleanup(func() {
TruncateTables(t, ds, "host_mdm_windows_profiles", "windows_mdm_command_queue", "windows_mdm_commands", "mdm_windows_configuration_profiles", "mdm_configuration_profile_labels", "label_membership")
})
// Delete profile A. LocURI Y is shared with B, so only X should be deleted.
err = ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profB}, nil)
return err
})
require.NoError(t, err)
// Verify the delete command on BOTH hosts.
for _, h := range []*fleet.Host{h1, h2} {
var rawCmd []byte
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &rawCmd,
`SELECT wc.raw_command FROM windows_mdm_commands wc
JOIN host_mdm_windows_profiles hwp ON hwp.command_uuid = wc.command_uuid
WHERE hwp.host_uuid = ? AND hwp.operation_type = 'remove'`, h.UUID)
})
require.NotEmpty(t, rawCmd, "host %s should have a delete command", h.Hostname)
rawStr := string(rawCmd)
assert.Contains(t, rawStr, "./Device/X", "host %s: should delete LocURI X", h.Hostname)
assert.NotContains(t, rawStr, "./Device/Y", "host %s: should NOT delete LocURI Y (protected by profB)", h.Hostname)
}
})
t.Run("label-scoped protector only protects hosts in scope", func(t *testing.T) {
t.Cleanup(func() {
TruncateTables(t, ds, "host_mdm_windows_profiles", "windows_mdm_command_queue", "windows_mdm_commands", "mdm_windows_configuration_profiles", "mdm_configuration_profile_labels", "label_membership")
})
// Profile A: LocURIs X, Y. Profile B: LocURI Y (shared), label-scoped to h1 only.
// When A is deleted:
// - h1: Y is protected (B applies), only X is deleted
// - h2: Y is NOT protected (B doesn't apply), both X and Y are deleted
profA2 := &fleet.MDMWindowsConfigProfile{Name: "ls-profA", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/X</LocURI></Target><Data>1</Data></Item></Replace><Replace><Item><Target><LocURI>./Device/Y</LocURI></Target><Data>1</Data></Item></Replace>`)}
profB2 := &fleet.MDMWindowsConfigProfile{Name: "ls-profB", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/Y</LocURI></Target><Data>2</Data></Item></Replace>`)}
// Insert both profiles.
err = ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profA2, profB2}, nil)
return err
})
require.NoError(t, err)
var profA2UUID, profB2UUID string
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &profA2UUID, `SELECT profile_uuid FROM mdm_windows_configuration_profiles WHERE name = 'ls-profA'`)
})
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &profB2UUID, `SELECT profile_uuid FROM mdm_windows_configuration_profiles WHERE name = 'ls-profB'`)
})
// Create a label and make profB label-scoped to it.
scopeLabel, err := ds.NewLabel(ctx, &fleet.Label{Name: "locuri-scope-label", Query: ""})
require.NoError(t, err)
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx,
`INSERT INTO mdm_configuration_profile_labels (windows_profile_uuid, label_name, label_id) VALUES (?, ?, ?)`,
profB2UUID, scopeLabel.Name, scopeLabel.ID)
return err
})
// Only h1 is a member of the label.
err = ds.AsyncBatchInsertLabelMembership(ctx, [][2]uint{{scopeLabel.ID, h1.ID}})
require.NoError(t, err)
// Simulate both profiles installed on both hosts. For profB, also add
// an install row for h1 (simulating the reconciler assigned it based on
// label membership).
verified := fleet.MDMDeliveryVerified
err = ds.BulkUpsertMDMWindowsHostProfiles(ctx, []*fleet.MDMWindowsBulkUpsertHostProfilePayload{
{ProfileUUID: profA2UUID, ProfileName: "ls-profA", HostUUID: h1.UUID, CommandUUID: uuid.NewString(), OperationType: fleet.MDMOperationTypeInstall, Status: &verified, Checksum: []byte{0}},
{ProfileUUID: profA2UUID, ProfileName: "ls-profA", HostUUID: h2.UUID, CommandUUID: uuid.NewString(), OperationType: fleet.MDMOperationTypeInstall, Status: &verified, Checksum: []byte{0}},
// profB only installed on h1 (label-scoped, reconciler only assigned it to h1).
{ProfileUUID: profB2UUID, ProfileName: "ls-profB", HostUUID: h1.UUID, CommandUUID: uuid.NewString(), OperationType: fleet.MDMOperationTypeInstall, Status: &verified, Checksum: []byte{0}},
})
require.NoError(t, err)
// Delete profA (keep profB).
err = ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profB2}, nil)
return err
})
require.NoError(t, err)
// h1: profB applies (label-scoped, h1 is in the label).
// Y is protected, only X should be deleted.
var h1Cmds [][]byte
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.SelectContext(ctx, q, &h1Cmds,
`SELECT wc.raw_command FROM windows_mdm_commands wc
JOIN windows_mdm_command_queue cq ON cq.command_uuid = wc.command_uuid
JOIN mdm_windows_enrollments mwe ON mwe.id = cq.enrollment_id
WHERE mwe.host_uuid = ?`, h1.UUID)
})
require.NotEmpty(t, h1Cmds, "h1 should have delete commands")
for _, cmd := range h1Cmds {
s := string(cmd)
if strings.Contains(s, "<Delete") {
assert.Contains(t, s, "./Device/X", "h1: should delete X")
assert.NotContains(t, s, "./Device/Y", "h1: should NOT delete Y (protected by label-scoped profB)")
}
}
// h2: profB does NOT apply (h2 not in label).
// Y is NOT protected, both X and Y should be deleted.
var h2Cmds [][]byte
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.SelectContext(ctx, q, &h2Cmds,
`SELECT wc.raw_command FROM windows_mdm_commands wc
JOIN windows_mdm_command_queue cq ON cq.command_uuid = wc.command_uuid
JOIN mdm_windows_enrollments mwe ON mwe.id = cq.enrollment_id
WHERE mwe.host_uuid = ?`, h2.UUID)
})
require.NotEmpty(t, h2Cmds, "h2 should have delete commands")
h2HasX, h2HasY := false, false
for _, cmd := range h2Cmds {
s := string(cmd)
if strings.Contains(s, "<Delete") {
if strings.Contains(s, "./Device/X") {
h2HasX = true
}
if strings.Contains(s, "./Device/Y") {
h2HasY = true
}
}
}
assert.True(t, h2HasX, "h2: should delete X")
assert.True(t, h2HasY, "h2: should delete Y (profB doesn't apply, not in label scope)")
})
}
func testEditProfileDeletesRemovedLocURIs(t *testing.T, ds *Datastore) {
ctx := t.Context()
h1 := test.NewHost(t, ds, "host-edit-1", "10.0.0.3", uuid.NewString(), uuid.NewString(), time.Now(), test.WithPlatform("windows"))
windowsEnroll(t, ds, h1)
t.Run("removed LocURI generates delete", func(t *testing.T) {
t.Cleanup(func() {
TruncateTables(t, ds, "host_mdm_windows_profiles", "windows_mdm_command_queue", "windows_mdm_commands", "mdm_windows_configuration_profiles")
})
// Profile with two LocURIs.
prof := &fleet.MDMWindowsConfigProfile{Name: "edit-test", SyncML: []byte(`<Atomic><Replace><Item><Target><LocURI>./Device/Keep</LocURI></Target><Data>1</Data></Item></Replace><Replace><Item><Target><LocURI>./Device/Remove</LocURI></Target><Data>1</Data></Item></Replace></Atomic>`)}
err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{prof}, nil)
return err
})
require.NoError(t, err)
var profUUID string
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &profUUID, `SELECT profile_uuid FROM mdm_windows_configuration_profiles WHERE name = 'edit-test'`)
})
// Simulate profile installed on the host.
verified := fleet.MDMDeliveryVerified
err = ds.BulkUpsertMDMWindowsHostProfiles(ctx, []*fleet.MDMWindowsBulkUpsertHostProfilePayload{
{ProfileUUID: profUUID, ProfileName: "edit-test", HostUUID: h1.UUID, CommandUUID: uuid.NewString(), OperationType: fleet.MDMOperationTypeInstall, Status: &verified, Checksum: []byte{0}},
})
require.NoError(t, err)
// Edit profile: remove ./Device/Remove.
profEdited := &fleet.MDMWindowsConfigProfile{Name: "edit-test", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/Keep</LocURI></Target><Data>1</Data></Item></Replace>`)}
err = ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profEdited}, nil)
return err
})
require.NoError(t, err)
// A delete command should have been generated for ./Device/Remove.
var deleteCommands [][]byte
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.SelectContext(ctx, q, &deleteCommands,
`SELECT wc.raw_command FROM windows_mdm_commands wc
JOIN windows_mdm_command_queue cq ON cq.command_uuid = wc.command_uuid
JOIN mdm_windows_enrollments mwe ON mwe.id = cq.enrollment_id
WHERE mwe.host_uuid = ?
ORDER BY wc.created_at DESC`, h1.UUID)
})
foundDelete := false
for _, cmd := range deleteCommands {
s := string(cmd)
if strings.Contains(s, "<Delete") && strings.Contains(s, "./Device/Remove") {
foundDelete = true
assert.NotContains(t, s, "./Device/Keep", "should not delete the kept LocURI")
}
}
assert.True(t, foundDelete, "expected a <Delete> command for ./Device/Remove")
})
t.Run("shared LocURI not deleted when editing", func(t *testing.T) {
t.Cleanup(func() {
TruncateTables(t, ds, "host_mdm_windows_profiles", "windows_mdm_command_queue", "windows_mdm_commands", "mdm_windows_configuration_profiles")
})
// Profile A has LocURIs P, Q. Profile B has LocURI Q (shared).
profA := &fleet.MDMWindowsConfigProfile{Name: "edit-shared-A", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/P</LocURI></Target><Data>1</Data></Item></Replace><Replace><Item><Target><LocURI>./Device/Q</LocURI></Target><Data>1</Data></Item></Replace>`)}
profBShared := &fleet.MDMWindowsConfigProfile{Name: "edit-shared-B", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/Q</LocURI></Target><Data>2</Data></Item></Replace>`)}
err := ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profA, profBShared}, nil)
return err
})
require.NoError(t, err)
var profAUUID string
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.GetContext(ctx, q, &profAUUID, `SELECT profile_uuid FROM mdm_windows_configuration_profiles WHERE name = 'edit-shared-A'`)
})
// Simulate profile A installed on the host.
verified := fleet.MDMDeliveryVerified
err = ds.BulkUpsertMDMWindowsHostProfiles(ctx, []*fleet.MDMWindowsBulkUpsertHostProfilePayload{
{ProfileUUID: profAUUID, ProfileName: "edit-shared-A", HostUUID: h1.UUID, CommandUUID: uuid.NewString(), OperationType: fleet.MDMOperationTypeInstall, Status: &verified, Checksum: []byte{0}},
})
require.NoError(t, err)
// Edit A to remove Q (shared with B), keep only P.
profAEdited := &fleet.MDMWindowsConfigProfile{Name: "edit-shared-A", SyncML: []byte(`<Replace><Item><Target><LocURI>./Device/P</LocURI></Target><Data>1</Data></Item></Replace>`)}
err = ds.withTx(ctx, func(tx sqlx.ExtContext) error {
_, err := ds.batchSetMDMWindowsProfilesDB(ctx, tx, nil, []*fleet.MDMWindowsConfigProfile{profAEdited, profBShared}, nil)
return err
})
require.NoError(t, err)
// Check that no delete was generated for Q (protected by B),
// and no delete was generated for P (still in the edited profile).
var deleteCommands [][]byte
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
return sqlx.SelectContext(ctx, q, &deleteCommands,
`SELECT wc.raw_command FROM windows_mdm_commands wc
JOIN windows_mdm_command_queue cq ON cq.command_uuid = wc.command_uuid
JOIN mdm_windows_enrollments mwe ON mwe.id = cq.enrollment_id
WHERE mwe.host_uuid = ?
ORDER BY wc.created_at DESC`, h1.UUID)
})
for _, cmd := range deleteCommands {
s := string(cmd)
if strings.Contains(s, "<Delete") {
assert.NotContains(t, s, "./Device/Q", "should NOT delete Q (protected by edit-shared-B)")
assert.NotContains(t, s, "./Device/P", "should NOT delete P (still in edited profile)")
}
}
})
}