From a0f60dc7f85960409161c191f469f2ec124f8fbb Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Mon, 20 Apr 2026 14:05:21 -0400 Subject: [PATCH] DDMV: fix unresolved Fleet variable in DDM profile behavior (#43556) **Related issue:** Resolves #43047 Follow-up to https://github.com/fleetdm/fleet/pull/43222 # Checklist for submitter - [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 See https://github.com/fleetdm/fleet/issues/42960#issuecomment-4246769629 ## Summary by CodeRabbit * **Bug Fixes** * Improved Apple MDM declaration handling: declarations with unresolved per-device variables are now attempted per host, marked failed when resolution fails, and omitted from device configuration/activation manifests. * Declarations that fail resolution still factor into declaration token computation to keep token behavior consistent. * **Tests** * Updated tests to reflect per-device resolution failures and adjusted validation flow. --- server/datastore/mysql/apple_mdm.go | 5 +++-- server/fleet/apple_mdm.go | 5 +++++ server/service/apple_mdm.go | 24 +++++++++++++++++++--- server/service/integration_mdm_ddm_test.go | 21 +++++++++++-------- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 5d8c15d936..0fe19ef659 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -5703,7 +5703,8 @@ func (ds *Datastore) MDMAppleDDMDeclarationItems(ctx context.Context, hostUUID s SELECT HEX(mad.token) as token, mad.identifier, mad.declaration_uuid, status, operation_type, mad.uploaded_at, - hmad.variables_updated_at + hmad.variables_updated_at, + IF(hmad.variables_updated_at IS NOT NULL AND operation_type = ?, mad.raw_json, NULL) as raw_json FROM host_mdm_apple_declarations hmad JOIN mdm_apple_declarations mad ON mad.declaration_uuid = hmad.declaration_uuid @@ -5711,7 +5712,7 @@ WHERE hmad.host_uuid = ?` var res []fleet.MDMAppleDDMDeclarationItem - if err := sqlx.SelectContext(ctx, ds.reader(ctx), &res, stmt, hostUUID); err != nil { + if err := sqlx.SelectContext(ctx, ds.reader(ctx), &res, stmt, fleet.MDMOperationTypeInstall, hostUUID); err != nil { return nil, ctxerr.Wrap(ctx, err, "get DDM declaration items") } diff --git a/server/fleet/apple_mdm.go b/server/fleet/apple_mdm.go index 9fde6c607f..0632958c6c 100644 --- a/server/fleet/apple_mdm.go +++ b/server/fleet/apple_mdm.go @@ -922,6 +922,11 @@ type MDMAppleDDMDeclarationItem struct { // values depend on the host. It is used to compute the token for the DDM for a specific host, as the // ServerToken field is just for the static token of the DDM. VariablesUpdatedAt *time.Time `db:"variables_updated_at"` + // RawJSON is conditionally loaded only for declarations that use Fleet + // variables (variables_updated_at IS NOT NULL and operation_type = 'install') + // so that handleDeclarationItems can check variable resolution without an + // extra query. + RawJSON *json.RawMessage `db:"raw_json"` } // MDMAppleDDMDeclarationResponse represents a declaration in the datastore. It is used for the DDM diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 7db640770d..e714bc8675 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -1143,9 +1143,9 @@ func (svc *MDMAppleDDMService) replaceDeclarationFleetVariables( } // markDeclarationFailed marks a DDM declaration as failed for a specific host. -func (svc *MDMAppleDDMService) markDeclarationFailed(ctx context.Context, hostUUID string, d *fleet.MDMAppleDeclaration, detail string) error { +func (svc *MDMAppleDDMService) markDeclarationFailed(ctx context.Context, hostUUID string, declarationUUID string, detail string) error { status := fleet.MDMDeliveryFailed - return svc.ds.SetHostMDMAppleDeclarationStatus(ctx, hostUUID, d.DeclarationUUID, &status, detail, nil) + return svc.ds.SetHostMDMAppleDeclarationStatus(ctx, hostUUID, declarationUUID, &status, detail, nil) } func (svc *Service) batchValidateDeclarationLabels(ctx context.Context, labelNames []string, teamID uint) (map[string]fleet.ConfigurationProfileLabel, error) { @@ -6160,6 +6160,24 @@ func (svc *MDMAppleDDMService) handleDeclarationItems(ctx context.Context, hostU } continue } + + // For declarations with fleet variables, check if the variables can + // be resolved for this host. If not, mark as failed and skip the + // declaration from the manifest so the device does not attempt to + // fetch or apply it. NOTE: the declaration is still included in the token + // computation below so that the token matches the SQL-computed + // token from handleTokens. + if d.VariablesUpdatedAt != nil { + if d.RawJSON != nil { + if _, err := svc.replaceDeclarationFleetVariables(ctx, string(*d.RawJSON), hostUUID); err != nil { + if err := svc.markDeclarationFailed(ctx, hostUUID, d.DeclarationUUID, err.Error()); err != nil { + return nil, ctxerr.Wrap(ctx, err, "mark declaration as failed") + } + continue + } + } + } + effectiveToken := fleet.EffectiveDDMToken(d.ServerToken, d.VariablesUpdatedAt) configurations = append(configurations, fleet.MDMAppleDDMManifest{ Identifier: d.Identifier, @@ -6304,7 +6322,7 @@ func (svc *MDMAppleDDMService) handleConfigurationDeclaration(ctx context.Contex expanded, err = svc.replaceDeclarationFleetVariables(ctx, expanded, hostUUID) if err != nil { // Mark this declaration as failed for this host, return empty 200 - if err := svc.markDeclarationFailed(ctx, hostUUID, d, err.Error()); err != nil { + if err := svc.markDeclarationFailed(ctx, hostUUID, d.DeclarationUUID, err.Error()); err != nil { return nil, ctxerr.Wrap(ctx, err, "mark declaration as failed") } return nil, nil diff --git a/server/service/integration_mdm_ddm_test.go b/server/service/integration_mdm_ddm_test.go index 32bd63ab61..3692edf97d 100644 --- a/server/service/integration_mdm_ddm_test.go +++ b/server/service/integration_mdm_ddm_test.go @@ -1758,13 +1758,15 @@ WHERE name = ?` // Get current variables_updated_at for host1's declarations (may have changed since earlier captures) latestVarsUpdatedUUID := getHostDeclVarsUpdatedAt(t, host1.UUID, dbDeclUUID.DeclarationUUID) latestVarsUpdatedSerial := getHostDeclVarsUpdatedAt(t, host1.UUID, dbDeclSerial.DeclarationUUID) - latestVarsUpdatedIdp := getHostDeclVarsUpdatedAt(t, host1.UUID, dbDeclIdpUsername.DeclarationUUID) + // The IDP declaration is excluded from the manifest because its variable + // can't be resolved (no IdP user for this host), but it is still included + // in the DeclarationsToken computation so that the token matches the + // SQL-computed token from the tokens endpoint. declsByToken = map[string]fleet.MDMAppleDeclaration{ fleet.EffectiveDDMToken(dbDeclUUID.Token, latestVarsUpdatedUUID): {Identifier: "com.fleet.var.uuid"}, fleet.EffectiveDDMToken(dbDeclSerial.Token, latestVarsUpdatedSerial): {Identifier: "com.fleet.var.serial"}, dbDeclPlain.Token: {Identifier: "com.fleet.plain"}, - fleet.EffectiveDDMToken(dbDeclIdpUsername.Token, latestVarsUpdatedIdp): {Identifier: "com.fleet.var.idpusername"}, } r, err = mdmDevice1.DeclarativeManagement("declaration-items") @@ -1772,13 +1774,9 @@ WHERE name = ?` itemsResp = parseDeclarationItemsResp(t, r) checkDeclarationItemsResp(t, itemsResp, lastSyncDeclToken, declsByToken) - // Host1 fetches the IdP username declaration — variable resolution fails - // because no IdP user exists for the host. The server returns an empty 200 - // and marks the declaration as failed. - _, err = mdmDevice1.DeclarativeManagement("declaration/configuration/com.fleet.var.idpusername") - require.NoError(t, err) - - // Verify the declaration is marked as failed with the expected detail message + // Verify the IDP declaration is marked as failed after the declaration-items + // fetch (handleDeclarationItems detected unresolvable variables and excluded + // the declaration from the manifest). var hostDecl fleet.MDMAppleHostDeclaration mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { return sqlx.GetContext(ctx, q, &hostDecl, @@ -1790,6 +1788,11 @@ WHERE name = ?` assert.Contains(t, hostDecl.Detail, "There is no IdP username for this host") assert.Contains(t, hostDecl.Detail, "$FLEET_VAR_HOST_END_USER_IDP_USERNAME") + // Host1 fetches the IdP username configuration — variable resolution + // fails again (fallback path). The server returns an empty 200. + _, err = mdmDevice1.DeclarativeManagement("declaration/configuration/com.fleet.var.idpusername") + require.NoError(t, err) + // === Updating variable declaration to non-variable clears variables_updated_at === // Drain host3's pending DDM sync from the IdP batch upload above