extra error message checks and correct escaping in error message (#50136)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #40074 unreleased bug <img width="539" height="141" alt="image" src="https://github.com/user-attachments/assets/1ac8e2c2-236d-4567-a200-0eb35cce46e7" /> # 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -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(), `<string>Unescaped & < > ' "</string>`),
|
||||
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", "Valid<Identifier", uuid.NewString(), `<string>Valid</string>`),
|
||||
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("Valid<Name", "ValidIdentifier", uuid.NewString(), `<string>Valid</string>`),
|
||||
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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user