iOS/iPadOS managed config: Configuration dict injection (#43966) - #44934
Merged
Conversation
…o []byte Replaces the stub from the datastore-methods branch (#43964) with a real plist validator. Lives in server/fleet/vpp.go alongside the rest of VPP because validation is part of the InstallApplication command flow. - ValidateAppleAppConfiguration parses the payload via howett.net/plist into a map[string]any (which naturally rejects non-dict roots), then walks string-typed leaves checking Fleet variable tokens against the app-config allow-list. - FleetVarsSupportedInAppleAppConfig: host-scoped subset of the variables permitted in Apple configuration profiles. Excludes credential / SCEP / NDES variables that don't fit the InstallApplication shape. - Configuration field type: json.RawMessage -> []byte across VPPAppTeam, VPPAppStoreApp, AppStoreAppUpdatePayload. The Apple payload is plist XML, not JSON, so the field is just opaque bytes. Existing Android callers pass and receive []byte transparently (json.RawMessage is []byte-underlying, assignable in both directions). - Updated comments on the three Configuration fields to reflect Apple support. - No size cap in the validator — caps belong at the API layer. - Empty input is allowed: callers (e.g. the gitops change-detector in vpp.go) decide whether to store or clear. Issue: #43963
Replaces the ad-hoc walker over the parsed plist tree with a raw-bytes scan using server/variables.Find — the same helper validateConfigProfileFleetVariables in apple_mdm.go uses for profile validation. Drops the local fleetVarTokenRegexp, the per-call allowed-set map, the walkAppleAppConfigStrings recursive walker, and the higher-order-function callback style. The plist.Unmarshal step stays as the structural check (must be valid plist with a <dict> root). Token allow-list check is now a flat loop with slices.Contains.
…figuration Mirrors the Android datastore methods for the new vpp_app_configurations and in_house_app_configurations tables (subtask 01). Wires VPP methods into the existing add/update/delete code paths in vpp.go alongside the Android branches. In-house methods land here but aren't wired yet — fleet.InHouseAppPayload doesn't carry a Configuration field until subtask 02. ValidateAppleAppConfiguration is a temporary stub (TODO #43963) replaced by the real plist validator in subtask 02. Issue: #43964
Migration #44435 dropped team_id from in_house_app_configurations (the parent in_house_apps row already pins team and platform). Updates the five in-house datastore methods, the four interface signatures, and the regenerated mocks to match.
- Test helpers use real datastore APIs (insertInHouseApp, InsertVPPAppWithTeam, DeleteInHouseApp) instead of raw SQL where a public function exists. - Replace fleetdm.com sample with example.com in test plist. - HasChanged methods early-return false when incoming config is empty — null/empty incoming is a no-op; clearing requires explicit Delete*Configuration. Destructive-on-omit semantics for gitops will live in #43969 (TODO added).
- HasVPPAppConfigurationChanged / HasInHouseAppConfigurationChanged: empty incoming against existing config now returns true (= delete intent), matching Android's semantic. Removing a config from gitops YAML clears it. - SetTeamVPPApps batch upsert path: when iOS/iPadOS Configuration is empty, DELETE the existing config row instead of skipping the write. Implements the destructive-on-omit behavior at the wiring layer. - Get*AppConfiguration return type changed from *[]byte to []byte (nil = not found, returned alongside notFound error). Drops a non-idiomatic pointer. - Updated stale comment on VPPAppTeam.Configuration / VPPAppStoreApp.Configuration to reflect that Apple platforms now use the field (plist payload). - Added documentation on Datastore interface that in-house config methods do not enforce team-scoped auth — callers must validate. - Added cross-team isolation assertions to the VPP CRUDFlow test.
…byte Now that VPPAppTeam.Configuration is []byte (subtask 02), align the Android datastore surface to match. Drops the now-redundant pointer wrapping on Get* returns at the same time. Method signatures: - GetAndroidAppConfiguration -> ([]byte, error) (was *json.RawMessage) - GetAndroidAppConfigurationByAppTeamID -> ([]byte, error) - BulkGetAndroidAppConfigurations -> (map[string][]byte, error) - HasAndroidAppConfigurationChanged -> takes []byte - updateAndroidAppConfigurationTx -> takes []byte Callers updated: - vpp.go app-store fetch (drops *config deref) - worker/software_worker.go (configByAppID maps + helper signature) - Tests in android_test.go, vpp_test.go, software_worker_test.go Mocks regenerated. Also fixes a side-effect in TestAndroid/AddDeleteAndroidAppWithConfiguration: the test previously sent JSON-shaped Configuration to an iOS app (vestigial "ios shouldn't have configuration" wording). The real iOS validator from subtask 02 now rejects non-plist input, so the test seeds a valid plist fragment instead.
Adds Configuration on InHouseAppPayload and the SoftwareInstaller upload/update payloads, and threads it through MatchOrCreateSoftwareInstaller, insertInHouseApp, and SaveInHouseAppUpdates inside their existing transactions. GetInHouseAppMetadataByTeamAndTitleID now hydrates the Configuration field.
Implements subtask 04: AddAppStoreApp, UpdateAppStoreApp, and
BatchAssociateVPPApps now persist the configuration field for iOS and
iPadOS VPP apps (previously silently dropped). Each iOS/iPadOS branch
JSON-decodes the wire-format string, calls fleet.ValidateAppleAppConfiguration,
and forwards the decoded plist bytes to the datastore.
The configuration wipe at the top of the Apple-platform branch is scoped
to macOS (which doesn't support managed configuration on App Store apps).
Activity emission keeps the JSON-encoded form so ActivityEditedAppStoreApp
/ ActivityAddedAppStoreApp's Configuration json.RawMessage field continues
to marshal as valid JSON for both Android (inline JSON object) and Apple
(JSON-quoted plist string).
decodeAppleAppConfiguration helper translates the wire format
("configuration": "<plist XML>") into raw bytes for validator and
datastore consumption.
New integration test TestVPPAppleManagedAppConfiguration covers add with
valid plist, update with allowed Fleet variable, omit-field no-op,
malformed XML rejection, and disallowed-variable rejection.
Premium gate and authorization unchanged (existing entry-point checks
already cover iOS/iPadOS — same fleet.VPPApp authz + EE-only service
implementation).
Issue: #43965
- Add iOS update path validator coverage (malformed plist + disallowed Fleet variable) - Add macOS invalid-plist cases on add, update, and batch paths to lock down the silent-drop ordering (validation must run after macOS configuration is cleared, not before)
Cover platform-specific Configuration encoding (iOS/iPadOS as JSON string, Android as base64), field omission for nil/empty Configuration, and JSON null handling. Also tightens the early-exit in both UnmarshalJSON sites to treat the literal `null` token as nil instead of decoding it through the platform-specific path.
Default []byte JSON encoding produced base64 for Android responses, which is an artifact of Go's defaults rather than a deliberate choice. Override MarshalJSON to wrap the field as json.RawMessage and update UnmarshalJSON to mirror it, so requests and responses both use a JSON object for Android (symmetric with iOS / iPadOS using a JSON string of plist).
The custom UnmarshalJSON was localizing wire-format knowledge to the request type, but with the symmetric Android wire format the service can take the configuration as raw JSON directly and unwrap iOS plist strings inline. The activity emission also goes back to using the wire form directly without a re-encode round trip.
VPPAppStoreApp.Configuration is now json.RawMessage (the form returned in API responses) and the type-level MarshalJSON / UnmarshalJSON methods are gone. UpdateAppStoreApp (EE) and the GET software title endpoint each wrap iOS / iPadOS plist bytes as a JSON string after fetching from the datastore, before assigning to the response. Android passes through as raw JSON.
- Configuration is a multipart form field (raw XML bytes for iOS / iPadOS). - Upload validates with ValidateAppleAppConfiguration; for non-.ipa extensions the configuration is silently dropped. - Update treats nil as "leave unchanged", non-nil empty as "clear", non-nil non-empty as "validate + set". - Response sites (UploadSoftwareInstaller, UpdateSoftwareInstaller, GET software title) wrap the iOS configuration as a JSON string so it appears as the same wire form clients send on input. - Integration test covers upload + update + clear + validation errors.
GitOps apply now reads `configuration.path` from disk for iOS / iPadOS app store apps (validated with `ValidateAppleAppConfiguration`, sent on the wire as a JSON-encoded string of XML to match the API shape), and `fleetctl generate-gitops` emits stored configurations under `lib/<team>/software/<slug>-<platform>-config.xml` alongside the existing Android `*-config.json` artifacts.
… gitops Adds a `configuration.path` field to `software.packages` entries. When present, gitops apply reads the file (validated with `ValidateAppleAppConfiguration`), threads it through the batch upload pipeline, and `BatchSetInHouseAppsInstallers` upserts or clears the in-house app configuration declaratively. Configuration is silently dropped for non-.ipa packages. `fleetctl generate-gitops` emits the stored configuration as `<slug>-<platform>-config.xml` alongside the in-house app's other artifacts.
Contributor
|
Caution Review failedPull request was closed or merged during review WalkthroughThis PR refactors Apple MDM Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cdcme
marked this pull request as ready for review
May 9, 2026 00:20
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Member
Author
|
@claude review once |
jkatz01
reviewed
May 11, 2026
…urationsTx variants
jkatz01
reviewed
May 11, 2026
jkatz01
approved these changes
May 11, 2026
jkatz01
previously approved these changes
May 12, 2026
jkatz01
approved these changes
May 12, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Moves Apple InstallApplication MDM command generation out of inline SQL and into Go so iOS/iPadOS managed app configuration (per app/team/platform) can be injected per-host.
Changes:
- Added
BuildInstallApplicationCommandhelper to build per-hostInstallApplicationplist bytes (with platform-specificConfigurationbehavior). - Refactored VPP and in-house install enqueue paths to SELECT pending installs, bulk-fetch app configurations, build commands in Go, and batch-INSERT into
nano_commands. - Added unit tests covering configuration injection/drop/normalization cases.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/mdm/apple/install_application.go | New command builder + helper to strip outer plist wrapper from stored configuration bytes. |
| server/mdm/apple/install_application_test.go | New tests validating generated plists and configuration injection semantics. |
| server/datastore/mysql/vpp.go | Refactors VPP install enqueue to build commands in Go and bulk-fetch configuration keyed by (platform, adam_id, team_id). |
| server/datastore/mysql/in_house_apps.go | Adds bulk-get helpers supporting both reader and transaction contexts. |
| server/datastore/mysql/activities.go | Refactors in-house install enqueue to build per-app commands in Go and bulk-fetch configurations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+95
| fmt.Fprintf(&b, " <key>iTunesStoreID</key>\n <integer>%s</integer>\n", params.ITunesStoreID) | ||
| case params.ManifestURL != "": | ||
| fmt.Fprintf(&b, " <key>ManifestURL</key>\n <string>%s</string>\n", params.ManifestURL) |
Comment on lines
+99
to
+102
| <key>CommandUUID</key> | ||
| <string>`) | ||
| b.WriteString(params.CommandUUID) | ||
| b.WriteString(`</string> |
Comment on lines
+2938
to
2961
| // one batch INSERT into nano_commands. Per-host, per-app build is required | ||
| // so the Configuration dict (which varies by team / adam_id) can be | ||
| // inlined. | ||
| insValues := make([]string, 0, len(pending)) | ||
| insArgs := make([]any, 0, len(pending)*4) | ||
| for _, p := range pending { | ||
| var cfg []byte | ||
| if cfgs, ok := configsByPlatformAdamID[p.Platform]; ok { | ||
| cfg = cfgs[p.AdamID] | ||
| } | ||
| cmdBytes := apple_mdm.BuildInstallApplicationCommand(apple_mdm.InstallApplicationParams{ | ||
| CommandUUID: p.ExecutionID, | ||
| HostPlatform: hostData.Platform, | ||
| ITunesStoreID: p.AdamID, | ||
| Configuration: cfg, | ||
| }) | ||
| insValues = append(insValues, "(?, 'InstallApplication', ?, ?)") | ||
| insArgs = append(insArgs, p.ExecutionID, string(cmdBytes), mdm.CommandSubtypeNone) | ||
| } | ||
| insCmdStmt := `INSERT INTO nano_commands (command_uuid, request_type, command, subtype) VALUES ` + | ||
| strings.Join(insValues, ", ") | ||
| if _, err := tx.ExecContext(ctx, insCmdStmt, insArgs...); err != nil { | ||
| return ctxerr.Wrap(ctx, err, "insert nano commands") | ||
| } |
| require.Equal(t, 1, strings.Count(out, "<?xml"), "only one XML declaration allowed") | ||
| require.Equal(t, 1, strings.Count(out, "<plist"), "only one <plist> element allowed") | ||
|
|
||
| cmd := parsed["Command"].(map[string]any) |
| "howett.net/plist" | ||
| ) | ||
|
|
||
| func TestBuildInstallApplicationCommand_VPP(t *testing.T) { |
cdcme
added a commit
that referenced
this pull request
May 12, 2026
Part of #38790. Stacked on top of #44934. Closes #43967. Adds `SubstituteFleetVarsInAppConfig` in `server/mdm/apple`, called from `nanoEnqueueVPPInstall` and `activateNextInHouseAppInstallActivity` right before the `InstallApplication` plist is built. Reuses `profiles.ReplaceFleetVariableInXML` (XML-escapes substituted values) and `profiles.ReplaceHostEndUserIDPVariables` for IDP fields. Supports the full `FleetVarsSupportedInAppleAppConfig` allow-list: `HOST_UUID`, `HOST_HARDWARE_SERIAL`, `HOST_PLATFORM`, `HOST_END_USER_EMAIL_IDP`, `HOST_END_USER_IDP_USERNAME` / `_LOCAL_PART` / `_GROUPS` / `_DEPARTMENT` / `_FULLNAME`. Returns `ErrUnresolvableAppConfigVar` when the host can't supply a referenced variable (e.g. end-user IDP not enrolled) so the caller can fail the install rather than send an empty value to the device. Pulls `hardware_serial` onto the host SELECT so `HOST_HARDWARE_SERIAL` is available without a second round-trip. --------- Co-authored-by: jkatz01 <yehonatankatz@gmail.com>
This was referenced May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #38790. Stacked on top of #44933.
Closes #43966.
The
InstallApplicationMDM command for VPP and in-house (.ipa) apps was assembled inline in SQL viaCONCAT, which can't carry per-(adam_id, team_id, platform) configuration. Move the assembly into Go: newBuildInstallApplicationCommandhelper inserver/mdm/apple, plus refactorednanoEnqueueVPPInstallandactivateNextInHouseAppInstallActivityto SELECT pending tuples, bulk-fetch configurations, build per-host plist bytes, and batch-INSERT intonano_commands.For iOS / iPadOS, the
Configurationdict is inlined when stored configuration is present. macOS VPP installs always drop the field regardless of input. Empty / absent configuration omits the<key>Configuration>entry — Apple treats this as "clear any managed config for this app on next apply."$FLEET_VAR_*tokens in the configuration are passed through unsubstituted in this PR; per-host substitution comes in #43967.Summary by CodeRabbit
New Features
Tests