Skip to content

Add best-effort DDM parsing - #50765

Draft
JordanMontgomery wants to merge 1 commit into
mainfrom
JM-caddm-parser
Draft

Add best-effort DDM parsing#50765
JordanMontgomery wants to merge 1 commit into
mainfrom
JM-caddm-parser

Conversation

@JordanMontgomery

@JordanMontgomery JordanMontgomery commented Aug 7, 2026

Copy link
Copy Markdown
Member

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/ or ee/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

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/ddmpredicate package (lexer/parser/AST) plus unit + fuzz tests.
  • Validate Payload.Predicate during MDMAppleRawActivation.ValidateUserProvided and 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.

Comment thread server/fleet/apple_mdm.go
Comment on lines +1159 to +1163
if r.Payload.Predicate != nil {
if _, err := ddmpredicate.Parse(*r.Payload.Predicate); err != nil {
invalid.Append("Predicate", err.Error())
}
}
Comment on lines +1344 to +1347
func withPredicate(act MDMAppleRawActivation, predicate string) MDMAppleRawActivation {
act.Payload.Predicate = &predicate
return act
}
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.70030% with 184 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.42%. Comparing base (5a1365d) to head (ede6927).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/mdm/apple/ddmpredicate/parser.go 74.45% 56 Missing and 49 partials ⚠️
server/mdm/apple/ddmpredicate/lexer.go 74.43% 42 Missing and 3 partials ⚠️
server/mdm/apple/ddmpredicate/ast.go 59.52% 33 Missing and 1 partial ⚠️
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     
Flag Coverage Δ
backend 69.62% <72.70%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants