Fix undeletable Apple DDM declarations when allowed types change (#49810)

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

## Description

`DeleteMDMAppleDeclaration` re-ran the upload-time validator
(`ValidateUserProvided`) on the delete path. That validator enforces
*upload-admission* rules — forbidden declaration types
(`ForbiddenDeclTypes`) and the `AllowAllDeclarations` config flag — so
any declaration that was accepted at upload time became **undeletable**
through the API once the accepted set later shrank. Two ways this
happens in practice:

- A server config flag that had been enabled at upload time is later
disabled (the original customer report, prod 4.86.1).
- A declaration type is added to `ForbiddenDeclTypes` in a later
release, after declarations of that type were already uploaded.

In both cases the UI showed "Couldn't delete. Please try again." and the
API returned `400` with an upload-validation message on a *delete*
request.

Whether a declaration is Fleet-managed (and therefore protected from
deletion through this endpoint) is already determined by the Fleet
reserved-name check that runs immediately above the offending block.
This PR removes the upload-time validation from the delete path and
relies solely on that reserved-name check, so:

- A user can delete any declaration they previously uploaded, regardless
of whether the current validator config would still accept it on upload.
- Fleet-managed declarations (reserved names) remain protected from
deletion.

The `AllowAllDeclarations` flag and `ValidateUserProvided` are unchanged
on the **add/upload** path — admission control still happens where it
belongs.

## Testing

Extended `TestMDMConfigProfileCRUD` (replacing the pre-existing `//
TODO: Add tests for create/delete forbidden declaration types?`) with
two cases:

- A declaration whose type is in `ForbiddenDeclTypes` can be deleted
(regression guard — fails before this change, passes after).
- A declaration with a Fleet-reserved name remains protected from
deletion (guards the reserved-name check that is now the sole
Fleet-managed gate — a boundary that was previously untested).

Manually verified end-to-end in the UI: reproduced the stuck declaration
(upload a forbidden type with `FLEET_MDM_ALLOW_ALL_DECLARATIONS=true`,
restart without the flag), confirmed the pre-fix `400`, then confirmed
deletion succeeds after the fix with the flag off.

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files)
for more information.
- [x] Input data is properly validated, `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

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually


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

* **Bug Fixes**
* Improved Apple MDM declaration deletion to avoid re-running
upload-time validation during the delete flow.
* Declaration deletion checks now rely on Fleet management status and
reserved naming, preserving protection for Fleet-managed declarations.
* Added/adjusted deletion behavior for restricted and Fleet-reserved
declaration types.
* **Tests**
* Added regression coverage for Apple declaration profile deletion via
the configuration profile delete endpoint, including strict-mode
scenarios and cleanup behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

changes/fix-undeletable-ddm-declarations.md

@@ -0,0 +1 @@
+- Fixed a bug where an Apple configuration profile (DDM declaration) could become undeletable if the set of allowed declaration types changed after the profile was added (for example, when a server configuration flag was toggled). Deleting a profile no longer re-runs upload-time validation.

server/service/apple_mdm.go

@@ -1969,6 +1969,14 @@ func (svc *Service) DeleteMDMAppleDeclaration(ctx context.Context, declUUID stri
 
 	// Check if the declaration contains a secret variable. If it does, this means that the declaration
 	// has been provided by the user and can be deleted. We don't need to validate that it is a Fleet declaration.
+	//
+	// Whether a declaration is Fleet-managed (and therefore protected from
+	// deletion through this endpoint) is determined solely by its reserved name.
+	// We deliberately do NOT run the upload-time validator (ValidateUserProvided)
+	// here: any declaration already stored was accepted at upload time, so
+	// re-validating it on delete would trap user-uploaded declarations whenever
+	// the accepted set later shrinks (a config flag is toggled, or a type is
+	// added to ForbiddenDeclTypes). See https://github.com/fleetdm/fleet/issues/47535.
 	hasSecretVariable := len(fleet.ContainsPrefixVars(string(decl.RawJSON), fleet.ServerSecretPrefix)) > 0
 	if !hasSecretVariable {
 		if _, ok := mdm_types.FleetReservedProfileNames()[decl.Name]; ok {
@@ -1977,21 +1985,6 @@ func (svc *Service) DeleteMDMAppleDeclaration(ctx context.Context, declUUID stri
 				InternalErr: fmt.Errorf("deleting profile %s is not allowed because it's managed by Fleet", decl.Name),
 			}
 		}
-
-		// TODO: refine our approach to deleting restricted/forbidden types of declarations so that we
-		// can check that Fleet-managed aren't being deleted; this can be addressed once we add support
-		// for more types of declarations
-		var d fleet.MDMAppleRawDeclaration
-		if err := json.Unmarshal(decl.RawJSON, &d); err != nil {
-			return ctxerr.Wrap(ctx, err, "unmarshalling declaration")
-		}
-
-		// skip declaration validation if the allow all declarations flag is set.
-		if !svc.config.MDM.AllowAllDeclarations {
-			if err := d.ValidateUserProvided(); err != nil {
-				return ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()})
-			}
-		}
 	}
 
 	teamID, teamName, err := svc.resolveProfileTeam(ctx, decl.TeamID)

server/service/integration_mdm_profiles_test.go

@@ -3787,7 +3787,60 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
 			return err
 		})
 	}
-	// TODO: Add tests for create/delete forbidden declaration types?
+	// A declaration whose type would now fail upload validation — e.g. it was
+	// accepted before its type was added to ForbiddenDeclTypes, or before a
+	// config flag was toggled — must still be deletable. Deletion does not
+	// re-run the upload-time validator; doing so would trap declarations the
+	// user already uploaded. Regression test for
+	// https://github.com/fleetdm/fleet/issues/47535.
+	{
+		forbiddenType := "com.apple.configuration.watch.enrollment"
+		require.Contains(t, fleet.ForbiddenDeclTypes, forbiddenType) // guard: type is genuinely forbidden on upload
+
+		// The delete-time validation this fix removed only ran under strict
+		// validation (AllowAllDeclarations == false), so this test only guards
+		// against its reintroduction when the suite runs strict. Assert that
+		// precondition explicitly so the test can't silently become a no-op if
+		// the suite default ever changes.
+		require.False(t, s.fleetCfg.MDM.AllowAllDeclarations)
+
+		declUUID := fleet.MDMAppleDeclarationUUIDPrefix + uuid.NewString()
+		rawJSON := fmt.Sprintf(`{"Type":%q,"Identifier":"com.fleet.forbidden-type","Payload":{}}`, forbiddenType)
+		mysqltest.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
+			_, err := q.ExecContext(ctx,
+				"INSERT INTO mdm_apple_declarations (declaration_uuid, identifier, name, raw_json, scope, uploaded_at, team_id) VALUES (?, ?, ?, ?, ?, NOW(6), 0)",
+				declUUID, "com.fleet.forbidden-type", "forbidden-type-decl", rawJSON, fleet.PayloadScopeSystem)
+			return err
+		})
+
+		var deleteResp deleteMDMConfigProfileResponse
+		s.DoJSON("DELETE", fmt.Sprintf("/api/latest/fleet/configuration_profiles/%s", declUUID), nil, http.StatusOK, &deleteResp)
+	}
+
+	// A declaration that uses a Fleet-reserved name stays protected from deletion
+	// through this endpoint. The reserved-name check is the sole Fleet-managed
+	// gate on the delete path, so make sure removing the upload-time validation
+	// (above) didn't open a hole.
+	{
+		reservedName := servermdm.FleetMacOSUpdatesProfileName
+		declUUID := fleet.MDMAppleDeclarationUUIDPrefix + uuid.NewString()
+		rawJSON := `{"Type":"com.apple.configuration.softwareupdate.enforcement.specific","Identifier":"com.fleet.reserved","Payload":{}}`
+		mysqltest.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
+			_, err := q.ExecContext(ctx,
+				"INSERT INTO mdm_apple_declarations (declaration_uuid, identifier, name, raw_json, scope, uploaded_at, team_id) VALUES (?, ?, ?, ?, ?, NOW(6), 0)",
+				declUUID, "com.fleet.reserved", reservedName, rawJSON, fleet.PayloadScopeSystem)
+			return err
+		})
+
+		var deleteResp deleteMDMConfigProfileResponse
+		s.DoJSON("DELETE", fmt.Sprintf("/api/latest/fleet/configuration_profiles/%s", declUUID), nil, http.StatusBadRequest, &deleteResp)
+
+		// the API refused to delete it, so remove the seeded row directly
+		mysqltest.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
+			_, err := q.ExecContext(ctx, "DELETE FROM mdm_apple_declarations WHERE declaration_uuid = ?", declUUID)
+			return err
+		})
+	}
 
 	// make fleet add a FileVault profile
 	acResp := appConfigResponse{}