Add best-effort DDM parsing - #50765
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new DDM activation predicate parser/validator and wires it into Apple activation upload validation so malformed predicates are rejected early, while still storing the predicate verbatim.
Changes:
- Add
server/mdm/apple/ddmpredicatepackage (lexer/parser/AST) plus unit + fuzz tests. - Validate
Payload.PredicateduringMDMAppleRawActivation.ValidateUserProvidedand add coverage for valid/malformed/empty predicates. - Update service tests to use predicates that parse as valid NSPredicate (including Fleet variables embedded as literals).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/apple_mdm_test.go | Adds a test ensuring malformed activation predicates are rejected; updates predicate strings to valid syntax. |
| server/mdm/apple/ddmpredicate/parser.go | Implements the DDM predicate parser and validation logic. |
| server/mdm/apple/ddmpredicate/lexer.go | Adds a lexer and ParseError used by the parser. |
| server/mdm/apple/ddmpredicate/ast.go | Defines AST nodes and canonical String() rendering for round-tripping. |
| server/mdm/apple/ddmpredicate/doc.go | Package-level documentation and accepted syntax notes. |
| server/mdm/apple/ddmpredicate/parser_test.go | Valid/invalid predicate tests, Apple-documented samples, AST tests, and fuzz target. |
| server/mdm/apple/ddmpredicate/testdata/fuzz/FuzzParse/cbf3679c935137e9 | Adds a fuzz corpus seed for parser hardening. |
| server/fleet/apple_mdm.go | Parses/validates activation Payload.Predicate during upload validation. |
| server/fleet/apple_mdm_test.go | Adds predicate validation test cases and helper to set Payload.Predicate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if r.Payload.Predicate != nil { | ||
| if _, err := ddmpredicate.Parse(*r.Payload.Predicate); err != nil { | ||
| invalid.Append("Predicate", err.Error()) | ||
| } | ||
| } |
| func withPredicate(act MDMAppleRawActivation, predicate string) MDMAppleRawActivation { | ||
| act.Payload.Predicate = &predicate | ||
| return act | ||
| } |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #50765 +/- ##
==========================================
+ Coverage 68.40% 68.42% +0.01%
==========================================
Files 3963 3966 +3
Lines 255174 255848 +674
Branches 13420 13420
==========================================
+ Hits 174560 175055 +495
- Misses 65016 65142 +126
- Partials 15598 15651 +53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Related issue: Resolves #
Checklist for submitter
If some of the following don't apply, delete the relevant line.
No changes file, unreleased bug in feature
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See 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
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
For unreleased bug fixes in a release candidate, one of: