From c905e692288ade2bdfdc83d5acf9df5029328ae2 Mon Sep 17 00:00:00 2001 From: Magnus Jensen Date: Fri, 7 Aug 2026 19:03:07 +0200 Subject: [PATCH] improve validation error messages for custom activations (#50753) **Related issue:** Resolves nothing, just something I caught while doing test plan and PR review. # 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. part of a bigger story. - [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. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] 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 validation messages for custom activations. * Clarified that standard configurations must reference exactly one configuration profile. * Added a documentation link for the single-profile reference requirement. --- server/fleet/apple_mdm.go | 10 +++++----- server/fleet/apple_mdm_test.go | 10 +++++----- server/service/apple_mdm_test.go | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/server/fleet/apple_mdm.go b/server/fleet/apple_mdm.go index 52cc924565..3752f89d38 100644 --- a/server/fleet/apple_mdm.go +++ b/server/fleet/apple_mdm.go @@ -1129,14 +1129,14 @@ func (r *MDMAppleRawActivation) ValidateUserProvided(configurationIdentifier str invalid := &InvalidArgumentError{} if strings.TrimSpace(r.Type) == "" { - invalid.Append("Type", "Activation must include a Type.") + invalid.Append("Type", "The custom activation must include a Type.") } else if !strings.HasPrefix(r.Type, MDMAppleActivationTypePrefix) { invalid.Append("Type", fmt.Sprintf("Only activation declarations (%s) are supported.", MDMAppleActivationTypePrefix)) } switch { case strings.TrimSpace(r.Identifier) == "": - invalid.Append("Identifier", "Activation must include an Identifier.") + invalid.Append("Identifier", "The custom activation must include an Identifier.") case len(r.Identifier) > MDMAppleDeclarationIdentifierMaxLen: invalid.Append("Identifier", fmt.Sprintf( "Identifier must be %d bytes or fewer.", MDMAppleDeclarationIdentifierMaxLen)) @@ -1144,12 +1144,12 @@ func (r *MDMAppleRawActivation) ValidateUserProvided(configurationIdentifier str switch configs := r.Payload.StandardConfigurations; { case len(configs) == 0: - invalid.Append("StandardConfigurations", "Activation must reference the configuration profile it's uploaded with.") + invalid.Append("StandardConfigurations", "The custom activation must reference the identifier of the configuration profile used to upload it.") case len(configs) > 1: - invalid.Append("StandardConfigurations", "Activation can only reference one configuration profile.") + invalid.Append("StandardConfigurations", "The custom activation can only have one referenced configuration profile. Learn more: https://fleetdm.com/learn-more-about/ddm-activations") case configs[0] != configurationIdentifier: invalid.Append("StandardConfigurations", fmt.Sprintf( - "Activation must reference the configuration profile it's uploaded with. Expected %q, got %q.", + "The custom activation must reference the identifier of the configuration profile used to upload it. Expected %q, got %q.", configurationIdentifier, configs[0])) } diff --git a/server/fleet/apple_mdm_test.go b/server/fleet/apple_mdm_test.go index a893011664..ede13abe22 100644 --- a/server/fleet/apple_mdm_test.go +++ b/server/fleet/apple_mdm_test.go @@ -1235,13 +1235,13 @@ func TestMDMAppleRawActivationValidateUserProvided(t *testing.T) { name: "missing type", activation: rawActivation("", "com.fleet.act.passcode", configIdentifier), wantErr: true, - errContains: "Activation must include a Type.", + errContains: "The custom activation must include a Type.", }, { name: "whitespace type", activation: rawActivation(" ", "com.fleet.act.passcode", configIdentifier), wantErr: true, - errContains: "Activation must include a Type.", + errContains: "The custom activation must include a Type.", }, { name: "configuration type is not an activation", @@ -1254,7 +1254,7 @@ func TestMDMAppleRawActivationValidateUserProvided(t *testing.T) { name: "missing identifier", activation: rawActivation("com.apple.activation.simple", " ", configIdentifier), wantErr: true, - errContains: "Activation must include an Identifier.", + errContains: "The custom activation must include an Identifier.", }, { name: "identifier over Apple's 64 octet limit", @@ -1266,14 +1266,14 @@ func TestMDMAppleRawActivationValidateUserProvided(t *testing.T) { name: "no configurations referenced", activation: rawActivation("com.apple.activation.simple", "com.fleet.act.passcode"), wantErr: true, - errContains: "Activation must reference the configuration profile it's uploaded with.", + errContains: "The custom activation must reference the identifier of the configuration profile used to upload it.", }, { name: "more than one configuration referenced", activation: rawActivation("com.apple.activation.simple", "com.fleet.act.passcode", configIdentifier, "com.fleet.cfg.firewall"), wantErr: true, - errContains: "Activation can only reference one configuration profile.", + errContains: "The custom activation can only have one referenced configuration profile.", }, { name: "references a different configuration", diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 37e030f9f1..24b4dfc1c5 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -9993,7 +9993,7 @@ func TestNewMDMAppleDeclarationWithActivation(t *testing.T) { activation := activationBytesForTest("com.fleet.actD1", "com.fleet.configOther") _, err := svc.NewMDMAppleDeclaration(ctx, 0, decl, nil, "name", fleet.LabelsIncludeAll, nil, activation) - require.ErrorContains(t, err, "Activation must reference the configuration profile it's uploaded with") + require.ErrorContains(t, err, "The custom activation must reference the identifier of the configuration profile used to upload it") }) t.Run("malformed activation is rejected", func(t *testing.T) {