Use MDM SecurityInfo as boot policy source#420
Conversation
|
@Thinkscape is attempting to deploy a commit to the EigenLabs Team on Vercel. A member of the Team first needs to authorize it. |
3020105 to
a0a5ae0
Compare
a0a5ae0 to
32d1a83
Compare
ethenotethan
left a comment
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: COMMENT
Security — ✅ No issues found
Performance — 2 finding(s)
- 🔵 [INFO]
coordinator/api/provider_mdm_reliability_test.go:133— String formatting in test helper could be optimized- Suggestion: Pre-allocate string builder or use strings.Builder for better performance if this helper is called frequently
- 🔵 [INFO]
coordinator/mdm/mdm.go:741-743— fmt.Sprintf in error path allocates unnecessarily- Suggestion: Consider pre-formatting common error messages or using string concatenation for this simple case
Type_diligence — ✅ No issues found
Additive_complexity — 1 finding(s)
- 🔵 [INFO]
coordinator/mdm/mdm.go:251-253— HasFullSecureBoot method adds unnecessary indirection for simple string comparison- Suggestion: Replace method with direct comparison
r.SecureBootLevel == SecureBootLevelFullat call sites, or inline the logic if used only once
- Suggestion: Replace method with direct comparison
3 finding(s) total, 0 blocking. Verdict: COMMENT.
🤖 Automated review by Centaur · DAR-186
ethenotethan
left a comment
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: COMMENT
Security — ✅ No issues found
Performance — 1 finding(s)
- 🔵 [INFO]
coordinator/api/provider_mdm_reliability_test.go:133— String formatting in test helper called repeatedly with same parameters- Suggestion: Pre-compute the plist template or cache formatted strings if this helper is called frequently in test loops
Type_diligence — ✅ No issues found
Additive_complexity — 1 finding(s)
- 🔵 [INFO]
coordinator/mdm/mdm.go:251-253— HasFullSecureBoot method adds unnecessary indirection for simple string comparison- Suggestion: Replace method with direct comparison
r.SecureBootLevel == SecureBootLevelFullat call sites, or inline the constant check
- Suggestion: Replace method with direct comparison
2 finding(s) total, 0 blocking. Verdict: COMMENT.
🤖 Automated review by Centaur · DAR-186
| // securityInfoWebhook builds a MicroMDM acknowledge webhook body carrying a | ||
| // SecurityInfo plist with the given CommandUUID and SIP/SecureBoot posture. | ||
| func securityInfoWebhook(udid, commandUUID string, sipEnabled bool, secureBootFull bool) []byte { | ||
| func securityInfoWebhook(udid, commandUUID string, sipEnabled bool, secureBootLevel string) []byte { |
There was a problem hiding this comment.
🔵 [INFO] ⚡ String formatting in test helper called repeatedly with same parameters
💡 Suggestion: Pre-compute the plist template or cache formatted strings if this helper is called frequently in test loops
📊 Score: 2×1 = 2 · Category: repeated_work
| func (r *SecurityInfoResponse) HasFullSecureBoot() bool { | ||
| return r != nil && r.SecureBootLevel == SecureBootLevelFull | ||
| } |
There was a problem hiding this comment.
🔵 [INFO] 🧩 HasFullSecureBoot method adds unnecessary indirection for simple string comparison
💡 Suggestion: Replace method with direct comparison r.SecureBootLevel == SecureBootLevelFull at call sites, or inline the constant check
📊 Score: 2×3 = 6 · Category: over-abstraction
Summary
Avoid local localized boot-policy parsing in doctor: the detailed local checks no longer render a Secure Boot verdict from the authenticated-root proxy. The coordinator now treats Apple's typed MDM
SecurityInfo.SecureBootLevelas the authoritative boot-policy source, with onlyfullaccepted and documented/tested againstmedium,off, andnot supported.Linked issue
N/A - follow-up to PR #398 boot-policy review
Test plan
cd /Users/thinkscape/d-inference-mdm-boot-policy-20260620-142122/provider-swift && swift build --product darkbloom- passed; productdarkbloombuilt successfully (warnings only).cd /Users/thinkscape/d-inference-mdm-boot-policy-20260620-142122/provider-swift && DARKBLOOM_NO_UPDATE_CHECK=1 .build/debug/darkbloom doctor- ran built binary; exited1because the self-built debug binary is missing the keychain-access-groups entitlement for the Secure Enclave key (OSStatus -34018). Relevant output confirmed no detailedsecure bootcheck is printed;authenticated root,mdm enrollment, andcoordinator trustchecks still print and pass on this machine.cd /Users/thinkscape/d-inference-mdm-boot-policy-20260620-142122/provider-swift && swift test --filter MDMTrustDiagnosis- passed 7 tests inMDMTrustDiagnosisTests.cd /Users/thinkscape/d-inference-mdm-boot-policy-20260620-142122 && go test ./coordinator/mdm- passed.cd /Users/thinkscape/d-inference-mdm-boot-policy-20260620-142122 && go test ./coordinator/api- passed.git diff --check- passed.Components touched
Protocol / interface changes
Notes for reviewers
Verified local alternatives while preparing this change: there is no public
BootPolicyorDeviceManagementSwift module for an unprivileged local doctor check;bputilrequires root; private BootPolicy access is unsuitable for doctor. This keeps local doctor away from localized display strings and leaves boot-policy authority with the typed MDMSecurityInforesponse.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.