From e6118b4cc5c5995aba5c4780efbcf940b7d4ea51 Mon Sep 17 00:00:00 2001 From: Magnus Jensen Date: Wed, 29 Jul 2026 14:58:08 +0200 Subject: [PATCH] extra error message checks and correct escaping in error message (#50136) **Related issue:** Resolves #40074 unreleased bug image # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [ ] 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. - [ ] Timeouts are implemented and retries are limited to avoid infinite loops - [ ] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit * **Bug Fixes** * Improved configuration profile validation for unescaped special characters in Apple payloads. * Error messages now consistently indicate when characters like `&` and `<` must be XML-escaped. * Updated error examples to show properly escaped guidance. * Expanded test coverage to verify the standardized XML-escaping error for additional failing scenarios. --- server/fleet/apple_mdm_test.go | 22 +++++++++++++++++++ server/mdm/apple/mobileconfig/mobileconfig.go | 8 +++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/server/fleet/apple_mdm_test.go b/server/fleet/apple_mdm_test.go index 6469d97f67..679f3e9f56 100644 --- a/server/fleet/apple_mdm_test.go +++ b/server/fleet/apple_mdm_test.go @@ -30,6 +30,7 @@ func TestMDMAppleConfigProfile(t *testing.T) { testName string mobileconfig mobileconfig.Mobileconfig shouldFail bool + errString *string }{ { testName: "TestParseConfigProfileOK", @@ -91,6 +92,24 @@ func TestMDMAppleConfigProfile(t *testing.T) { }(), shouldFail: true, }, + { + testName: "TestParseConfigProfileUnescapedCharsInPayload", + mobileconfig: MobileconfigForTest("ValidName", "ValidIdentifier", uuid.NewString(), `Unescaped & < > ' "`), + shouldFail: true, + errString: new("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again."), + }, + { + testName: "TestParseConfigProfileUnescapedCharsInIdentifier", + mobileconfig: MobileconfigForTest("ValidName", "ValidValid`), + shouldFail: true, + errString: new("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again."), + }, + { + testName: "TestParseConfigProfileUnescapedCharsInName", + mobileconfig: MobileconfigForTest("ValidValid`), + shouldFail: true, + errString: new("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again."), + }, } for _, c := range cases { @@ -98,6 +117,9 @@ func TestMDMAppleConfigProfile(t *testing.T) { parsed, err := NewMDMAppleConfigProfile(c.mobileconfig, nil) if c.shouldFail { require.Error(t, err) + if c.errString != nil { + require.ErrorContains(t, err, *c.errString) + } } else { require.NoError(t, err) require.Equal(t, "ValidName", parsed.Name) diff --git a/server/mdm/apple/mobileconfig/mobileconfig.go b/server/mdm/apple/mobileconfig/mobileconfig.go index 2a48797f8f..53a7c2e9b2 100644 --- a/server/mdm/apple/mobileconfig/mobileconfig.go +++ b/server/mdm/apple/mobileconfig/mobileconfig.go @@ -113,8 +113,8 @@ func (mc Mobileconfig) ParseConfigProfile() (*Parsed, error) { } var p Parsed if _, err := plist.Unmarshal(mcBytes, &p); err != nil { - if strings.Contains(err.Error(), "illegal base64 data") { - return nil, errors.New("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again.") + if strings.Contains(err.Error(), "illegal base64 data") || strings.Contains(err.Error(), "invalid character entity") || strings.Contains(err.Error(), "expected attribute name in element") { + return nil, errors.New("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again.") } return nil, err } @@ -168,8 +168,8 @@ func (mc Mobileconfig) payloadSummary() ([]payloadSummary, error) { } _, err := plist.Unmarshal(mcBytes, &tlo) if err != nil { - if strings.Contains(err.Error(), "illegal base64 data") { - return nil, errors.New("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again.") + if strings.Contains(err.Error(), "illegal base64 data") || strings.Contains(err.Error(), "invalid character entity") || strings.Contains(err.Error(), "expected attribute name in element") { + return nil, errors.New("The configuration profile contains special characters (&, <, >, ', \") that must be XML-escaped. Please escape them (e.g. & → &, < → <) and try again.") } return nil, err }