fix(provider): report boot-security posture without blocking startup#455
Open
Gajesh2007 wants to merge 2 commits into
Open
fix(provider): report boot-security posture without blocking startup#455Gajesh2007 wants to merge 2 commits into
Gajesh2007 wants to merge 2 commits into
Conversation
…oe before serving Gate `darkbloom start` and `darkbloom doctor` on three sudo-free boot-security checks before a provider serves inference, feeding the existing signed attestation (`sip_enabled`, `secure_boot_enabled`) with no wire/protocol change: - macOS >= 26 (Tahoe): the minimum supported OS, injected via ProcessInfo.operatingSystemVersion so it is unit-testable. Below the floor blocks `start` (exit != 0) and FAILs `doctor` with a Software Update guide. - SIP fully enabled: rejects `disabled` and `enabled (Custom Configuration)`. - Secure Boot = Full Security: `system_profiler SPiBridgeDataType` -> `ibridge_secure_boot`, authoritative on Apple Silicon (Tahoe) and Intel T2. Because Tahoe is the floor and SPiBridgeDataType is reliably populated there, the earlier SSV/boot-args proxy fallback is removed as dead code: delete AuthenticatedRootStatus.swift and SecureBootStatusChecker.fallbackProxyStatus; collapse SecureBootStatus to .fullSecurity/.reduced/.permissiveOrDisabled/ .unavailable. `checkAuthenticatedRootEnabled()` remains as a clean standalone SSV-seal reader for the independent `authenticated_root_enabled` attestation field. Undetectable Secure Boot warns rather than locking out; the DARKBLOOM_ALLOW_INSECURE_BOOT escape hatch downgrades a hard failure to a warning for developers. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This PR hardens the SIP, Secure Boot, and ARV detection logic in Trust Boundaries Touched
Threat AnalysisT-015 — Operator disables SIP to bypass security controls ✅ Strengthens mitigation The old T-037 — SIP/SecureBoot state self-reported without independent verification between challenges ✅ Strengthens mitigation The old T-014 — Operator inspects darkbloom process memory ℹ️ Neutral No change to PT_DENY_ATTACH, Hardened Runtime, or memory-wiping. The injectable T-033 / T-036 — Attestation blob replay / trust elevation without completing full chain ✅ Marginally strengthens
New Attack Surface / Gaps Introduced1.
2. The new Secure Boot probe shells out to
3. Untouched files introducing new boot-policy enforcement layer The 11 files listed as not shown in the diff — particularly
Open Findings Resolved
🔐 Threat model: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0db4d26b37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| printError("The coordinator will reject this provider. Re-enable SIP and restart.") | ||
| throw ExitCode.failure | ||
| } | ||
| try enforceBootSecurity() |
There was a problem hiding this comment.
This new boot-security gate only runs through runPreflightChecks, which I traced to the interactive launchDaemon path; darkbloom start --foreground (the existing LaunchAgent login/autostart command) and darkbloom start --local go directly to their serving paths. Since ProviderLoop.verifySecurityPosture still only throws on SIP and the coordinator does not enforce the new macOS floor, an already-installed provider that auto-starts after updating, or any direct --local/--foreground invocation, can bypass the Tahoe/Secure Boot preflight and expose inference. Put enforceBootSecurity in the actual serve paths (before the local endpoint is started) rather than only before installing the daemon.
Useful? React with 👍 / 👎.
Comment on lines
+102
to
+103
| case .unavailable, .unrecognized: | ||
| return .warn |
There was a problem hiding this comment.
When csrutil status is unavailable or unparseable, this preflight now only warns and proceeds, but the launched provider still calls verifySecurityPosture, where checkSIPEnabled() returns false for the same states and throws SecurityError.sipDisabled before the loop can run. In that scenario a user can see the boot-security warning and successful daemon installation, only for the LaunchAgent child to exit immediately; keep the preflight verdict aligned with the hardening path or make the later path honor the warning/override behavior.
Useful? React with 👍 / 👎.
ethenotethan
requested changes
Jun 23, 2026
ethenotethan
left a comment
ethenotethan
left a comment
Contributor
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: REQUEST_CHANGES
Security — 1 finding(s)
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/BootSecurityPolicy.swift:67— Environment variable name hardcoded as override mechanism- Suggestion: Consider making the override environment variable name configurable or document that this is intentionally hardcoded for security
Performance — 4 finding(s) (2 blocking)
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/BootSecurityGuidance.swift:39— String concatenation in loop without pre-allocation- Suggestion: Pre-allocate array capacity or use StringBuilder pattern for better performance
- 🟡 [MEDIUM]
provider-swift/Sources/ProviderCore/Security/BootSecurityPolicy.swift:170-205— Multiple string array appends without pre-allocation in hot path- Suggestion: Pre-allocate lines array with estimated capacity based on number of failing checks
- 🟡 [MEDIUM]
provider-swift/Sources/ProviderCore/Security/SecureBootStatusParser.swift:143-157— String processing in loop without early termination optimization- Suggestion: Add early break after finding first match to avoid processing remaining lines
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/SecurityHardening.swift:198-210— String processing in loop without break after match- Suggestion: Add break statement after finding 'Sealed:' line to avoid processing remaining lines
Type_diligence — 2 finding(s) (1 blocking)
- 🟡 [MEDIUM]
provider-swift/Sources/ProviderCore/Security/SecureBootStatusParser.swift:134— JSONDecoder().decode uses bare Any in error path- Suggestion: Consider using a more specific error type or structured error handling instead of relying on JSONDecoder's generic error behavior
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/SecureBootStatusParser.swift:169-177— JSON parsing relies on string-based CodingKeys- Suggestion: Consider using a more type-safe approach for JSON field mapping, though this is a common pattern for external API parsing
Additive_complexity — 6 finding(s) (3 blocking)
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/BootSecurityGuidance.swift:1-99— Static enum with only static methods could be a simple namespace struct- Suggestion: Consider using a struct with static methods instead of an enum with no cases - it's more conventional for pure utility types
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/BootSecurityPolicy.swift:1-211— Static enum with only static methods could be a simple namespace struct- Suggestion: Consider using a struct with static methods instead of an enum with no cases - it's more conventional for pure utility types
- 🔵 [INFO]
provider-swift/Sources/ProviderCore/Security/SecureBootStatusParser.swift:1-179— Static enum with only static methods could be a simple namespace struct- Suggestion: Consider using a struct with static methods instead of an enum with no cases - it's more conventional for pure utility types
- 🟡 [MEDIUM]
provider-swift/Sources/ProviderCore/Security/SecurityHardening.swift:198-225— Duplicate command execution pattern appears multiple times- Suggestion: Extract the SecurityCommandRunner pattern into a shared helper to avoid duplicating the Process/Pipe setup logic
- 🟡 [MEDIUM]
provider-swift/Sources/ProviderCore/Security/BootSecurityPolicy.swift:130-210— Policy class contains detailed message formatting logic- Suggestion: Move the detailed message construction to BootSecurityGuidance or a dedicated formatter - policy should focus on decisions, not presentation
- 🔴 [CRITICAL]
provider-swift/Tests/ProviderCoreTests/BootSecurityTests.swift:1-495— Extremely large test file with 495 lines testing multiple concerns- Suggestion: Split into separate test files: SecureBootStatusTests, BootSecurityPolicyTests, BootSecurityGuidanceTests - each focused on a single class
13 finding(s) total, 6 blocking. Verdict: REQUEST_CHANGES.
🤖 Automated review by Centaur · DAR-186
| public enum BootSecurityPolicy { | ||
| /// Environment variable that downgrades a hard failure to a loud warning. | ||
| /// Documented developer escape hatch so engineers on non-Full-Security | ||
| /// machines aren't locked out; never for production use. |
Contributor
There was a problem hiding this comment.
🔵 [INFO] 🔒 Environment variable name hardcoded as override mechanism
💡 Suggestion: Consider making the override environment variable name configurable or document that this is intentionally hardcoded for security
📊 Score: 2×2 = 4 · Category: hardcoded-credentials
Comment on lines
+170
to
+205
| let hasFailure = macOSV == .fail || sipV == .fail || secureBootV == .fail | ||
|
|
||
| var lines: [String] = [] | ||
| lines.append( | ||
| hasFailure | ||
| ? "ERROR: macOS boot security is not fully enabled — required to serve inference." | ||
| : "WARNING: macOS boot security could not be fully verified." | ||
| ) | ||
| if macOSV != .pass { | ||
| lines.append(" - macOS version: \(macOSSummary(majorVersion: macOSMajorVersion))") | ||
| } | ||
| if sipV != .pass { | ||
| lines.append(" - System Integrity Protection (SIP): \(sip.summary)") | ||
| } | ||
| if secureBootV != .pass { | ||
| lines.append(" - Secure Boot: \(secureBoot.summary)") | ||
| } | ||
| lines.append("") | ||
| lines.append(BootSecurityGuidance.guide( | ||
| includeMacOS: macOSV != .pass, | ||
| includeSIP: sipV != .pass, | ||
| includeSecureBoot: secureBootV != .pass | ||
| )) | ||
|
|
||
| if hasFailure && allowInsecureOverride { | ||
| lines.append("") | ||
| lines.append("\(overrideEnvVar)=1 is set — continuing despite the failure above.") | ||
| lines.append("This is for development only. DO NOT serve production traffic like this.") | ||
| return PreflightDecision(shouldBlock: false, messageLines: lines, overrodeBlock: true) | ||
| } | ||
|
|
||
| if hasFailure { | ||
| lines.append("") | ||
| lines.append("Refusing to start. Fix the above, or set \(overrideEnvVar)=1 to override (developer use only).") | ||
| return PreflightDecision(shouldBlock: true, messageLines: lines, overrodeBlock: false) | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 [MEDIUM] ⚡ Multiple string array appends without pre-allocation in hot path
💡 Suggestion: Pre-allocate lines array with estimated capacity based on number of failing checks
📊 Score: 2×4 = 8 · Category: unbounded allocations
Comment on lines
+198
to
210
| if let result = try? runner.run("/usr/sbin/diskutil", ["info", "/"]), | ||
| result.terminationStatus == 0 { | ||
| for line in result.stdout.components(separatedBy: "\n") { | ||
| let trimmed = line.trimmingCharacters(in: .whitespaces) | ||
| if trimmed.hasPrefix("Sealed:") { | ||
| return trimmed.contains("Yes") | ||
| } | ||
| guard trimmed.hasPrefix("Sealed:") else { continue } | ||
| // "Sealed: Yes" ⇒ sealed; "No" or an ambiguous "Broken" (macOS 26 | ||
| // mislabels a healthy seal) ⇒ not positively confirmed. | ||
| return trimmed.localizedCaseInsensitiveContains("Yes") | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
🔵 [INFO] ⚡ String processing in loop without break after match
💡 Suggestion: Add break statement after finding 'Sealed:' line to avoid processing remaining lines
📊 Score: 2×3 = 6 · Category: repeated work
Comment on lines
+1
to
+99
| import Foundation | ||
|
|
||
| /// The single source of truth for the user-facing instructions to ENABLE the | ||
| /// boot-security protections the provider requires. Both `darkbloom doctor` and | ||
| /// the `darkbloom start` preflight render these exact strings, so there are no | ||
| /// duplicated literals to drift apart. | ||
| /// | ||
| /// The instructions cover updating to macOS 26 (Tahoe) via Software Update and | ||
| /// turning the protections fully ON — booting into recoveryOS and using | ||
| /// `csrutil enable` / Startup Security Utility's "Full Security" — separately for | ||
| /// Apple Silicon and Intel (Apple T2). | ||
| public enum BootSecurityGuidance { | ||
| public static let macOSTitle = "How to update to macOS 26 (Tahoe) or later:" | ||
| public static let sipTitle = "How to fully ENABLE System Integrity Protection (SIP):" | ||
| public static let secureBootTitle = "How to set Secure Boot to Full Security:" | ||
|
|
||
| /// Combined guide containing only the requested sections, followed by a | ||
| /// single shared verification footer. The single entry point for both the | ||
| /// `start` preflight and `doctor`, which may need to flag any combination of | ||
| /// protections at once — there is exactly one footer, never a duplicate. | ||
| public static func guide(includeMacOS: Bool, includeSIP: Bool, includeSecureBoot: Bool) -> String { | ||
| var lines: [String] = [] | ||
| if includeMacOS { | ||
| lines.append(macOSTitle) | ||
| lines.append(contentsOf: macOSSteps()) | ||
| } | ||
| if includeSIP { | ||
| if !lines.isEmpty { lines.append("") } | ||
| lines.append(sipTitle) | ||
| lines.append(contentsOf: sipSteps()) | ||
| } | ||
| if includeSecureBoot { | ||
| if !lines.isEmpty { lines.append("") } | ||
| lines.append(secureBootTitle) | ||
| lines.append(contentsOf: secureBootSteps()) | ||
| } | ||
| if !lines.isEmpty { lines.append("") } | ||
| lines.append(contentsOf: verifyFooter) | ||
| return join(lines) | ||
| } | ||
|
|
||
| // MARK: - Sections (single source of literals) | ||
|
|
||
| static func macOSSteps() -> [String] { | ||
| [ | ||
| " 1. Open the Apple menu (\u{f8ff}) > System Settings.", | ||
| " 2. Go to General > Software Update.", | ||
| " 3. Install the latest macOS (26 \"Tahoe\" or newer) and restart when prompted.", | ||
| ] | ||
| } | ||
|
|
||
| static func sipSteps() -> [String] { | ||
| [ | ||
| " Apple Silicon:", | ||
| " 1. Shut the Mac down completely.", | ||
| " 2. Press and hold the power button until \"Loading startup options\" appears.", | ||
| " 3. Click Options, then Continue. Pick an admin account and enter its password.", | ||
| " 4. From the menu bar, choose Utilities > Terminal.", | ||
| " 5. Run: csrutil enable", | ||
| " 6. Restart the Mac (Apple menu > Restart).", | ||
| " Intel:", | ||
| " 1. Restart and immediately hold Command (\u{2318})-R until the Apple logo appears.", | ||
| " 2. From the menu bar, choose Utilities > Terminal.", | ||
| " 3. Run: csrutil enable", | ||
| " 4. Restart the Mac.", | ||
| " If 'csrutil status' shows \"enabled (Custom Configuration)\", run 'csrutil clear'", | ||
| " then 'csrutil enable' in recoveryOS to restore full protection.", | ||
| ] | ||
| } | ||
|
|
||
| static func secureBootSteps() -> [String] { | ||
| [ | ||
| " Apple Silicon:", | ||
| " 1. Shut the Mac down completely.", | ||
| " 2. Press and hold the power button until \"Loading startup options\" appears.", | ||
| " 3. Click Options, then Continue. Pick an admin account and enter its password.", | ||
| " 4. From the menu bar, choose Utilities > Startup Security Utility.", | ||
| " 5. Select your system disk, click \"Security Policy\u{2026}\", choose \"Full Security\", confirm.", | ||
| " 6. Restart the Mac.", | ||
| " Intel (Apple T2):", | ||
| " 1. Restart and immediately hold Command (\u{2318})-R to enter Recovery.", | ||
| " 2. From the menu bar, choose Utilities > Startup Security Utility and authenticate.", | ||
| " 3. Under \"Secure Boot\", choose \"Full Security\".", | ||
| " 4. Restart the Mac.", | ||
| ] | ||
| } | ||
|
|
||
| static let verifyFooter: [String] = [ | ||
| " After rebooting, re-run 'darkbloom doctor' to confirm macOS is 26+, SIP is", | ||
| " \"enabled\", and Secure Boot passes. Manual checks: 'sw_vers -productVersion'", | ||
| " (macOS); 'csrutil status' (SIP); 'system_profiler SPiBridgeDataType'", | ||
| " (ibridge_secure_boot) is the authoritative Secure Boot level on Apple Silicon", | ||
| " (Tahoe) and Intel T2.", | ||
| ] | ||
|
|
||
| private static func join(_ lines: [String]) -> String { | ||
| lines.joined(separator: "\n") | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔵 [INFO] 🧩 Static enum with only static methods could be a simple namespace struct
💡 Suggestion: Consider using a struct with static methods instead of an enum with no cases - it's more conventional for pure utility types
📊 Score: 2×3 = 6 · Category: over-abstraction
Comment on lines
+1
to
+211
| import Foundation | ||
|
|
||
| // MARK: - Verdict | ||
|
|
||
| /// The gate verdict for a single boot-security protection. | ||
| public enum BootSecurityVerdict: Sendable, Equatable { | ||
| /// Fully on — acceptable. | ||
| case pass | ||
| /// Could not be determined — surface a warning, but do not block. | ||
| case warn | ||
| /// Confidently not fully on — block startup. | ||
| case fail | ||
| } | ||
|
|
||
| // MARK: - Status summaries (shared wording) | ||
|
|
||
| extension SIPStatus { | ||
| /// One-line, human-readable summary of the SIP state, shared by the doctor | ||
| /// check detail and the start-preflight warning so the wording never drifts. | ||
| public var summary: String { | ||
| switch self { | ||
| case .enabled: | ||
| return "enabled (full protection)" | ||
| case .disabled: | ||
| return "disabled" | ||
| case .enabledWithCustomConfiguration(let disabledProtections): | ||
| let base = "enabled (Custom Configuration) — NOT fully enabled" | ||
| guard !disabledProtections.isEmpty else { return base } | ||
| return "\(base); disabled: \(disabledProtections.joined(separator: ", "))" | ||
| case .unavailable(let reason): | ||
| return "could not determine (\(reason))" | ||
| case .unrecognized(let output): | ||
| let trimmed = output.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return "could not interpret csrutil output (\(trimmed))" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension SecureBootStatus { | ||
| /// One-line, human-readable summary of the Secure Boot state, shared by the | ||
| /// doctor check detail and the start-preflight warning. | ||
| public var summary: String { | ||
| switch self { | ||
| case .fullSecurity: | ||
| return "Full Security" | ||
| case .reduced: | ||
| return "Reduced/Medium Security (system_profiler ibridge_secure_boot) " | ||
| + "— NOT Full Security" | ||
| case .permissiveOrDisabled: | ||
| return "Permissive / No Security (system_profiler ibridge_secure_boot) " | ||
| + "— secure boot not enforced" | ||
| case .unavailable(let reason): | ||
| return "could not determine (\(reason)) — the coordinator still requires " | ||
| + "confirmed Secure Boot, so proceeding locally does not guarantee admission" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Policy | ||
|
|
||
| /// Pure policy that maps detected boot-security states to gate verdicts and to | ||
| /// the combined `start` preflight decision. Free of any process/IO so it is | ||
| /// fully unit-testable. | ||
| public enum BootSecurityPolicy { | ||
| /// Environment variable that downgrades a hard failure to a loud warning. | ||
| /// Documented developer escape hatch so engineers on non-Full-Security | ||
| /// machines aren't locked out; never for production use. | ||
| public static let overrideEnvVar = "DARKBLOOM_ALLOW_INSECURE_BOOT" | ||
|
|
||
| /// The minimum supported macOS major version. macOS 26 (Tahoe) is the floor: | ||
| /// it is what makes `system_profiler SPiBridgeDataType` a reliable, sudo-free | ||
| /// Secure Boot signal, and it is the only OS the provider is validated on. | ||
| public static let minimumMacOSMajorVersion = 26 | ||
|
|
||
| /// macOS version gate: the running major version must be at least | ||
| /// `minimumMacOSMajorVersion`. Always determinable (read from `ProcessInfo`), | ||
| /// so there is no `.warn` case — below the floor is a hard `.fail`. | ||
| public static func macOSVerdict(_ majorVersion: Int) -> BootSecurityVerdict { | ||
| majorVersion >= minimumMacOSMajorVersion ? .pass : .fail | ||
| } | ||
|
|
||
| /// One-line, human-readable summary of the macOS version state, shared by | ||
| /// the doctor check detail and the start-preflight message. | ||
| public static func macOSSummary(majorVersion: Int) -> String { | ||
| if majorVersion >= minimumMacOSMajorVersion { | ||
| return "macOS \(majorVersion) — meets the macOS \(minimumMacOSMajorVersion) (Tahoe) minimum" | ||
| } | ||
| return "macOS \(majorVersion) — below the required macOS \(minimumMacOSMajorVersion) " | ||
| + "(Tahoe); update to continue" | ||
| } | ||
|
|
||
| /// SIP gate: fully enabled passes; disabled or "enabled (Custom | ||
| /// Configuration)" fail (custom config is NOT fully enabled); an | ||
| /// undeterminable result warns rather than blocks (csrutil should always be | ||
| /// present, so this is the pathological case — warn to avoid false lockout). | ||
| public static func sipVerdict(_ status: SIPStatus) -> BootSecurityVerdict { | ||
| switch status { | ||
| case .enabled: | ||
| return .pass | ||
| case .disabled, .enabledWithCustomConfiguration: | ||
| return .fail | ||
| case .unavailable, .unrecognized: | ||
| return .warn | ||
| } | ||
| } | ||
|
|
||
| /// Secure Boot gate: provable Full Security (`ibridge_secure_boot == "Full | ||
| /// Security"`, Apple Silicon or Intel T2) passes; a confidently-reported | ||
| /// downgrade (Reduced/Medium/Permissive/No Security) fails; an undeterminable | ||
| /// posture warns rather than blocks (avoids false-positive lockouts on a | ||
| /// localized `system_profiler` value or an unreadable probe). | ||
| /// | ||
| /// `pass` and `attestsSecureBoot` derive from the SAME `SecureBootStatus`, so | ||
| /// the gate and the attested `secure_boot_enabled` never disagree. | ||
| public static func secureBootVerdict(_ status: SecureBootStatus) -> BootSecurityVerdict { | ||
| switch status { | ||
| case .fullSecurity: | ||
| return .pass | ||
| case .reduced, .permissiveOrDisabled: | ||
| return .fail | ||
| case .unavailable: | ||
| return .warn | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Combined preflight decision | ||
|
|
||
| /// Outcome of evaluating both protections for the `start` preflight: whether | ||
| /// to block, the exact lines to print, and whether a block was overridden. | ||
| public struct PreflightDecision: Sendable, Equatable { | ||
| /// True when `start` must abort (throw a non-zero exit). | ||
| public let shouldBlock: Bool | ||
| /// Ordered lines to print (warnings + the enable guide). Empty when all | ||
| /// protections pass. | ||
| public let messageLines: [String] | ||
| /// True when a confident failure was downgraded to a warning by the | ||
| /// escape-hatch env var. | ||
| public let overrodeBlock: Bool | ||
|
|
||
| public init(shouldBlock: Bool, messageLines: [String], overrodeBlock: Bool) { | ||
| self.shouldBlock = shouldBlock | ||
| self.messageLines = messageLines | ||
| self.overrodeBlock = overrodeBlock | ||
| } | ||
|
|
||
| /// All protections passed: nothing to print, nothing to block. | ||
| public static let ok = PreflightDecision(shouldBlock: false, messageLines: [], overrodeBlock: false) | ||
| } | ||
|
|
||
| /// Evaluate all three protections and produce the preflight decision. | ||
| /// | ||
| /// - Failure (below the macOS floor, or a confident SIP / Secure Boot | ||
| /// downgrade) blocks startup unless `allowInsecureOverride` is set, in | ||
| /// which case it is loudly downgraded to a warning. | ||
| /// - A warning (undeterminable Secure Boot) prints the guide but never | ||
| /// blocks, so an undetectable host is not falsely locked out. | ||
| public static func preflightDecision( | ||
| macOSMajorVersion: Int, | ||
| sip: SIPStatus, | ||
| secureBoot: SecureBootStatus, | ||
| allowInsecureOverride: Bool | ||
| ) -> PreflightDecision { | ||
| let macOSV = macOSVerdict(macOSMajorVersion) | ||
| let sipV = sipVerdict(sip) | ||
| let secureBootV = secureBootVerdict(secureBoot) | ||
| guard macOSV != .pass || sipV != .pass || secureBootV != .pass else { | ||
| return .ok | ||
| } | ||
|
|
||
| let hasFailure = macOSV == .fail || sipV == .fail || secureBootV == .fail | ||
|
|
||
| var lines: [String] = [] | ||
| lines.append( | ||
| hasFailure | ||
| ? "ERROR: macOS boot security is not fully enabled — required to serve inference." | ||
| : "WARNING: macOS boot security could not be fully verified." | ||
| ) | ||
| if macOSV != .pass { | ||
| lines.append(" - macOS version: \(macOSSummary(majorVersion: macOSMajorVersion))") | ||
| } | ||
| if sipV != .pass { | ||
| lines.append(" - System Integrity Protection (SIP): \(sip.summary)") | ||
| } | ||
| if secureBootV != .pass { | ||
| lines.append(" - Secure Boot: \(secureBoot.summary)") | ||
| } | ||
| lines.append("") | ||
| lines.append(BootSecurityGuidance.guide( | ||
| includeMacOS: macOSV != .pass, | ||
| includeSIP: sipV != .pass, | ||
| includeSecureBoot: secureBootV != .pass | ||
| )) | ||
|
|
||
| if hasFailure && allowInsecureOverride { | ||
| lines.append("") | ||
| lines.append("\(overrideEnvVar)=1 is set — continuing despite the failure above.") | ||
| lines.append("This is for development only. DO NOT serve production traffic like this.") | ||
| return PreflightDecision(shouldBlock: false, messageLines: lines, overrodeBlock: true) | ||
| } | ||
|
|
||
| if hasFailure { | ||
| lines.append("") | ||
| lines.append("Refusing to start. Fix the above, or set \(overrideEnvVar)=1 to override (developer use only).") | ||
| return PreflightDecision(shouldBlock: true, messageLines: lines, overrodeBlock: false) | ||
| } | ||
|
|
||
| // Warnings only (state genuinely undeterminable): surface the guide but | ||
| // let startup proceed so we never lock out a correctly configured host. | ||
| return PreflightDecision(shouldBlock: false, messageLines: lines, overrodeBlock: false) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔵 [INFO] 🧩 Static enum with only static methods could be a simple namespace struct
💡 Suggestion: Consider using a struct with static methods instead of an enum with no cases - it's more conventional for pure utility types
📊 Score: 2×3 = 6 · Category: over-abstraction
Comment on lines
+130
to
+210
| public struct PreflightDecision: Sendable, Equatable { | ||
| /// True when `start` must abort (throw a non-zero exit). | ||
| public let shouldBlock: Bool | ||
| /// Ordered lines to print (warnings + the enable guide). Empty when all | ||
| /// protections pass. | ||
| public let messageLines: [String] | ||
| /// True when a confident failure was downgraded to a warning by the | ||
| /// escape-hatch env var. | ||
| public let overrodeBlock: Bool | ||
|
|
||
| public init(shouldBlock: Bool, messageLines: [String], overrodeBlock: Bool) { | ||
| self.shouldBlock = shouldBlock | ||
| self.messageLines = messageLines | ||
| self.overrodeBlock = overrodeBlock | ||
| } | ||
|
|
||
| /// All protections passed: nothing to print, nothing to block. | ||
| public static let ok = PreflightDecision(shouldBlock: false, messageLines: [], overrodeBlock: false) | ||
| } | ||
|
|
||
| /// Evaluate all three protections and produce the preflight decision. | ||
| /// | ||
| /// - Failure (below the macOS floor, or a confident SIP / Secure Boot | ||
| /// downgrade) blocks startup unless `allowInsecureOverride` is set, in | ||
| /// which case it is loudly downgraded to a warning. | ||
| /// - A warning (undeterminable Secure Boot) prints the guide but never | ||
| /// blocks, so an undetectable host is not falsely locked out. | ||
| public static func preflightDecision( | ||
| macOSMajorVersion: Int, | ||
| sip: SIPStatus, | ||
| secureBoot: SecureBootStatus, | ||
| allowInsecureOverride: Bool | ||
| ) -> PreflightDecision { | ||
| let macOSV = macOSVerdict(macOSMajorVersion) | ||
| let sipV = sipVerdict(sip) | ||
| let secureBootV = secureBootVerdict(secureBoot) | ||
| guard macOSV != .pass || sipV != .pass || secureBootV != .pass else { | ||
| return .ok | ||
| } | ||
|
|
||
| let hasFailure = macOSV == .fail || sipV == .fail || secureBootV == .fail | ||
|
|
||
| var lines: [String] = [] | ||
| lines.append( | ||
| hasFailure | ||
| ? "ERROR: macOS boot security is not fully enabled — required to serve inference." | ||
| : "WARNING: macOS boot security could not be fully verified." | ||
| ) | ||
| if macOSV != .pass { | ||
| lines.append(" - macOS version: \(macOSSummary(majorVersion: macOSMajorVersion))") | ||
| } | ||
| if sipV != .pass { | ||
| lines.append(" - System Integrity Protection (SIP): \(sip.summary)") | ||
| } | ||
| if secureBootV != .pass { | ||
| lines.append(" - Secure Boot: \(secureBoot.summary)") | ||
| } | ||
| lines.append("") | ||
| lines.append(BootSecurityGuidance.guide( | ||
| includeMacOS: macOSV != .pass, | ||
| includeSIP: sipV != .pass, | ||
| includeSecureBoot: secureBootV != .pass | ||
| )) | ||
|
|
||
| if hasFailure && allowInsecureOverride { | ||
| lines.append("") | ||
| lines.append("\(overrideEnvVar)=1 is set — continuing despite the failure above.") | ||
| lines.append("This is for development only. DO NOT serve production traffic like this.") | ||
| return PreflightDecision(shouldBlock: false, messageLines: lines, overrodeBlock: true) | ||
| } | ||
|
|
||
| if hasFailure { | ||
| lines.append("") | ||
| lines.append("Refusing to start. Fix the above, or set \(overrideEnvVar)=1 to override (developer use only).") | ||
| return PreflightDecision(shouldBlock: true, messageLines: lines, overrodeBlock: false) | ||
| } | ||
|
|
||
| // Warnings only (state genuinely undeterminable): surface the guide but | ||
| // let startup proceed so we never lock out a correctly configured host. | ||
| return PreflightDecision(shouldBlock: false, messageLines: lines, overrodeBlock: false) | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 [MEDIUM] 🧩 Policy class contains detailed message formatting logic
💡 Suggestion: Move the detailed message construction to BootSecurityGuidance or a dedicated formatter - policy should focus on decisions, not presentation
📊 Score: 3×3 = 9 · Category: misplaced responsibility
Comment on lines
+1
to
+495
| import Foundation | ||
| import Testing | ||
| @testable import ProviderCore | ||
|
|
||
| /// Unit tests for the Secure Boot detection layer, the macOS-version + SIP + | ||
| /// Secure Boot gate policy, and the shared enable guidance. | ||
| /// | ||
| /// Detection runs through an injected `SecurityCommandRunner` fed REAL shapes of | ||
| /// `system_profiler SPiBridgeDataType`, which carries `ibridge_secure_boot` | ||
| /// (Full / Reduced-Medium / Permissive-No Security). On the provider's minimum | ||
| /// OS — macOS 26 (Tahoe) — this array is reliably populated and authoritative on | ||
| /// BOTH Apple Silicon (verified on M4 Max / Mac16,5 / arm64) AND Intel T2. When | ||
| /// it is empty/absent the posture is `.unavailable` (a WARN, never a downgrade). | ||
| /// | ||
| /// Nothing here depends on the host's actual macOS / SIP / Secure Boot state. | ||
| @Suite("boot security gate") | ||
| struct BootSecurityTests { | ||
|
|
||
| // MARK: - Fixtures | ||
|
|
||
| private func ok(_ stdout: String) -> SecurityCommandResult { | ||
| SecurityCommandResult(terminationStatus: 0, stdout: stdout) | ||
| } | ||
|
|
||
| /// A populated `system_profiler -json SPiBridgeDataType` document reporting | ||
| /// the given `ibridge_secure_boot` level. This array is populated on BOTH | ||
| /// Intel T2 AND Apple Silicon (Tahoe), so this minimal shape stands in for | ||
| /// either platform's report. | ||
| private func spiBridge(_ secureBoot: String) -> String { | ||
| """ | ||
| { | ||
| "SPiBridgeDataType" : [ | ||
| { | ||
| "ibridge_sb_sip" : "Enabled", | ||
| "ibridge_sb_ssv" : "Enabled", | ||
| "ibridge_secure_boot" : "\(secureBoot)" | ||
| } | ||
| ] | ||
| } | ||
| """ | ||
| } | ||
|
|
||
| /// An EMPTY `system_profiler -json SPiBridgeDataType` array. This is | ||
| /// anomalous on Tahoe (the minimum supported OS); detection maps it to | ||
| /// `.unavailable` (warn), never a false downgrade. | ||
| private let emptyBridge = #"{ "SPiBridgeDataType" : [ ] }"# | ||
|
|
||
| /// The EXACT real `system_profiler -json SPiBridgeDataType` captured on Apple | ||
| /// Silicon (M4 Max / Mac16,5 / arm64, macOS darwin 25.5.0). The array is | ||
| /// POPULATED, with `ibridge_secure_boot == "Full Security"` — direct proof | ||
| /// that this data type is NOT Intel-T2-only and NOT empty on Apple Silicon. | ||
| /// Detection must parse this as `.fullSecurity`. | ||
| private let realAppleSiliconBridgeJSON = """ | ||
| { | ||
| "SPiBridgeDataType" : [ | ||
| { | ||
| "ibridge_boot_uuid" : "A904AC62-589E-450B-8829-96ADA16DE3DC", | ||
| "ibridge_build" : "mBoot-18000.120.36", | ||
| "ibridge_extra_boot_policies" : " ", | ||
| "ibridge_model_identifier_top" : "Mac16,5", | ||
| "ibridge_sb_boot_args" : "Enabled", | ||
| "ibridge_sb_ctrr" : "Enabled", | ||
| "ibridge_sb_device_mdm" : "Yes", | ||
| "ibridge_sb_manual_mdm" : "No", | ||
| "ibridge_sb_other_kext" : "No", | ||
| "ibridge_sb_sip" : "Enabled", | ||
| "ibridge_sb_ssv" : "Enabled", | ||
| "ibridge_secure_boot" : "Full Security" | ||
| } | ||
| ] | ||
| } | ||
| """ | ||
|
|
||
| /// The EXACT real plain-text `system_profiler SPiBridgeDataType` captured on | ||
| /// the same Apple Silicon machine — the text form the parser must also accept | ||
| /// (when the JSON document is unavailable). | ||
| private let realAppleSiliconBridgeText = """ | ||
| Controller: | ||
| Model Identifier: Mac16,5 | ||
| Firmware Version: mBoot-18000.120.36 | ||
| Boot UUID: A904AC62-589E-450B-8829-96ADA16DE3DC | ||
| Boot Policy: | ||
| Secure Boot: Full Security | ||
| System Integrity Protection: Enabled | ||
| Signed System Volume: Enabled | ||
| Kernel CTRR: Enabled | ||
| Boot Arguments Filtering: Enabled | ||
| Allow All Kernel Extensions: No | ||
| User Approved Privileged MDM Operations: No | ||
| DEP Approved Privileged MDM Operations: Yes | ||
| """ | ||
|
|
||
| /// A runner that serves the `system_profiler SPiBridgeDataType` probe — the | ||
| /// only command the Secure Boot checker runs. Any other probe returns a | ||
| /// non-zero "unexpected" result so a test fails loudly if detection calls a | ||
| /// command it didn't stub. | ||
| private func runner(systemProfiler: SecurityCommandResult) -> SecurityCommandRunner { | ||
| SecurityCommandRunner { path, args in | ||
| switch (path, args) { | ||
| case ("/usr/sbin/system_profiler", ["-json", "SPiBridgeDataType"]): | ||
| return systemProfiler | ||
| default: | ||
| return SecurityCommandResult(terminationStatus: 127, stderr: "unexpected probe: \(path) \(args)") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func occurrences(of needle: String, in haystack: String) -> Int { | ||
| guard !needle.isEmpty else { return 0 } | ||
| var count = 0 | ||
| var idx = haystack.startIndex | ||
| while let found = haystack.range(of: needle, range: idx..<haystack.endIndex) { | ||
| count += 1 | ||
| idx = found.upperBound | ||
| } | ||
| return count | ||
| } | ||
|
|
||
| // MARK: - SPiBridge path (populated SPiBridgeDataType → ibridge_secure_boot) | ||
|
|
||
| @Test("SPiBridge: ibridge_secure_boot maps every level through the checker") | ||
| func spiBridgeLevels() { | ||
| func status(_ level: String) -> SecureBootStatus { | ||
| SecureBootStatusChecker(runner: runner(systemProfiler: ok(spiBridge(level)))).status() | ||
| } | ||
| #expect(status("Full Security") == .fullSecurity) | ||
| #expect(status("Medium Security") == .reduced) | ||
| #expect(status("No Security") == .permissiveOrDisabled) | ||
| // Intel T2 uses "Medium"/"No Security"; Apple Silicon uses | ||
| // "Reduced"/"Permissive Security" — classify maps both vocabularies, so | ||
| // the same SPiBridge path covers either platform's reported level. | ||
| #expect(status("Reduced Security") == .reduced) | ||
| #expect(status("Permissive Security") == .permissiveOrDisabled) | ||
| } | ||
|
|
||
| @Test("Apple Silicon: the REAL populated SPiBridge JSON → .fullSecurity (NOT empty/nil)") | ||
| func appleSiliconFullSecurityViaSPiBridgeJSON() { | ||
| // The exact JSON captured on M4 Max / Mac16,5 / arm64. A populated | ||
| // SPiBridgeDataType on Apple Silicon MUST parse as Full Security. | ||
| let checker = SecureBootStatusChecker( | ||
| runner: runner(systemProfiler: ok(realAppleSiliconBridgeJSON))) | ||
| #expect(checker.status() == .fullSecurity) | ||
| #expect(checker.isFullSecurity()) | ||
| #expect(BootSecurityPolicy.secureBootVerdict(checker.status()) == .pass) | ||
| #expect(checker.status().attestsSecureBoot) | ||
| // The pure parser agrees on both the JSON and the plain-text shapes. | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(realAppleSiliconBridgeJSON) == .fullSecurity) | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(realAppleSiliconBridgeText) == .fullSecurity) | ||
| } | ||
|
|
||
| @Test("Apple Silicon: the REAL plain-text SPiBridge report → .fullSecurity") | ||
| func appleSiliconFullSecurityViaSPiBridgeText() { | ||
| let checker = SecureBootStatusChecker( | ||
| runner: runner(systemProfiler: ok(realAppleSiliconBridgeText))) | ||
| #expect(checker.status() == .fullSecurity) | ||
| #expect(checker.isFullSecurity()) | ||
| #expect(BootSecurityPolicy.secureBootVerdict(checker.status()) == .pass) | ||
| } | ||
|
|
||
| @Test("Intel T2: verdict mapping — Full passes, Medium/No fail") | ||
| func intelT2Verdicts() { | ||
| func verdict(_ level: String) -> BootSecurityVerdict { | ||
| BootSecurityPolicy.secureBootVerdict( | ||
| SecureBootStatusChecker(runner: runner(systemProfiler: ok(spiBridge(level)))).status()) | ||
| } | ||
| #expect(verdict("Full Security") == .pass) | ||
| #expect(verdict("Medium Security") == .fail) | ||
| #expect(verdict("No Security") == .fail) | ||
| } | ||
|
|
||
| @Test("SPiBridge: a localized level is unavailable (warn), never a false downgrade") | ||
| func spiBridgeLocalizedLevelWarns() { | ||
| // system_profiler localizes ibridge_secure_boot on non-English Macs; an | ||
| // unrecognized value must WARN, not be misread as a downgrade. | ||
| let status = SecureBootStatusChecker( | ||
| runner: runner(systemProfiler: ok(spiBridge("Vollständige Sicherheit"))) | ||
| ).status() | ||
| if case .unavailable = status { | ||
| // expected — must NOT be classified as a downgrade | ||
| } else { | ||
| Issue.record("expected .unavailable for a localized boot security level") | ||
| } | ||
| #expect(BootSecurityPolicy.secureBootVerdict(status) == .warn) | ||
| #expect(!status.isConfidentlyNotFullSecurity) | ||
| } | ||
|
|
||
| @Test("empty SPiBridge array → unavailable (WARN only, no lockout)") | ||
| func emptySPiBridgeWarns() { | ||
| // Anomalous on Tahoe, but if the array is empty we warn rather than block. | ||
| let status = SecureBootStatusChecker(runner: runner(systemProfiler: ok(emptyBridge))).status() | ||
| if case .unavailable = status { | ||
| // expected | ||
| } else { | ||
| Issue.record("expected .unavailable for an empty SPiBridgeDataType array") | ||
| } | ||
| #expect(BootSecurityPolicy.secureBootVerdict(status) == .warn) | ||
| #expect(!status.attestsSecureBoot) | ||
| } | ||
|
|
||
| @Test("system_profiler failure → unavailable, never a false pass/fail") | ||
| func commandFailureUnavailable() { | ||
| let status = SecureBootStatusChecker(runner: runner( | ||
| systemProfiler: SecurityCommandResult(terminationStatus: 1, stderr: "system_profiler: boom")) | ||
| ).status() | ||
| if case .unavailable = status { | ||
| // expected | ||
| } else { | ||
| Issue.record("expected .unavailable when system_profiler fails") | ||
| } | ||
| #expect(BootSecurityPolicy.secureBootVerdict(status) == .warn) | ||
| } | ||
|
|
||
| @Test("a throwing runner resolves to unavailable, never a false pass/fail") | ||
| func throwingRunnerUnavailable() { | ||
| struct Boom: Error {} | ||
| let checker = SecureBootStatusChecker(runner: SecurityCommandRunner { _, _ in throw Boom() }) | ||
| if case .unavailable = checker.status() { | ||
| // expected | ||
| } else { | ||
| Issue.record("expected .unavailable when the probe throws") | ||
| } | ||
| #expect(!checker.isFullSecurity()) | ||
| } | ||
|
|
||
| // MARK: - Pure parser (SPiBridgeDataType: Apple Silicon AND Intel T2) | ||
|
|
||
| @Test("parser spiBridgeStatus classifies a populated report and returns nil for an empty array") | ||
| func parserSPiBridgeStatus() { | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(spiBridge("Full Security")) == .fullSecurity) | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(spiBridge("Medium Security")) == .reduced) | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(spiBridge("No Security")) == .permissiveOrDisabled) | ||
| // An empty array → nil → the caller maps it to .unavailable (warn). | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(emptyBridge) == nil) | ||
| #expect(SecureBootStatusParser.spiBridgeStatus("") == nil) | ||
| } | ||
|
|
||
| @Test("parser spiBridgeStatus parses the plain-text 'Secure Boot:' report when JSON is absent") | ||
| func parserSPiBridgeText() { | ||
| let text = """ | ||
| Controller: | ||
|
|
||
| Boot Policy: | ||
| Secure Boot: Full Security | ||
| System Integrity Protection: Enabled | ||
| """ | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(text) == .fullSecurity) | ||
| #expect(SecureBootStatusParser.spiBridgeStatus(" Secure Boot: Reduced Security\n") == .reduced) | ||
| } | ||
|
|
||
| @Test("parser spiBridgeStatus(result): a non-zero termination is not a usable report") | ||
| func parserSPiBridgeCommandFailure() { | ||
| #expect(SecureBootStatusParser.spiBridgeStatus( | ||
| SecurityCommandResult(terminationStatus: 1, stdout: "", stderr: "system_profiler: boom")) == nil) | ||
| } | ||
|
|
||
| @Test("classify is whitespace- and case-insensitive") | ||
| func classifyNormalizes() { | ||
| #expect(SecureBootStatusParser.classify(level: " full security ") == .fullSecurity) | ||
| #expect(SecureBootStatusParser.classify(level: "FULLSECURITY") == .fullSecurity) | ||
| } | ||
|
|
||
| // MARK: - Status semantics | ||
|
|
||
| @Test("attestsSecureBoot is the single source for gate-pass AND the attested bool") | ||
| func attestsSecureBootMapping() { | ||
| #expect(SecureBootStatus.fullSecurity.attestsSecureBoot) | ||
| #expect(!SecureBootStatus.reduced.attestsSecureBoot) | ||
| #expect(!SecureBootStatus.permissiveOrDisabled.attestsSecureBoot) | ||
| #expect(!SecureBootStatus.unavailable(reason: "x").attestsSecureBoot) | ||
| } | ||
|
|
||
| @Test("isConfidentlyNotFullSecurity only fires on real downgrades") | ||
| func confidenceFlag() { | ||
| #expect(SecureBootStatus.reduced.isConfidentlyNotFullSecurity) | ||
| #expect(SecureBootStatus.permissiveOrDisabled.isConfidentlyNotFullSecurity) | ||
| #expect(!SecureBootStatus.fullSecurity.isConfidentlyNotFullSecurity) | ||
| #expect(!SecureBootStatus.unavailable(reason: "x").isConfidentlyNotFullSecurity) | ||
| } | ||
|
|
||
| /// Pins the deliberate gate↔attestation split for an undeterminable posture: | ||
| /// `start` proceeds locally (verdict `.warn`, no false lockout) but the | ||
| /// attested `secure_boot_enabled` is false, so the coordinator still rejects. | ||
| /// The WARNING text must say so, or operators are surprised by a rejection | ||
| /// after a "successful" local start. | ||
| @Test("unavailable warns locally (start proceeds) but does NOT attest Secure Boot (coordinator rejects)") | ||
| func unavailableWarnsLocallyButDoesNotAttestSecureBoot() { | ||
| let status = SecureBootStatus.unavailable(reason: "no ibridge_secure_boot level") | ||
|
|
||
| // Local gate: warn → start proceeds (never a false lockout)… | ||
| #expect(BootSecurityPolicy.secureBootVerdict(status) == .warn) | ||
| // …but the attested boolean is false, so the coordinator stays untrusting. | ||
| #expect(!status.attestsSecureBoot) | ||
|
|
||
| // The same split holds through the attestation-feeding entry point: an | ||
| // unreadable runner (every probe fails → unavailable) attests false. | ||
| let unreadable = SecurityCommandRunner { _, _ in | ||
| SecurityCommandResult(terminationStatus: 1, stderr: "unreadable") | ||
| } | ||
| #expect(!checkSecureBootEnabled(runner: unreadable)) | ||
|
|
||
| // The WARNING text tells the operator that proceeding locally is not | ||
| // admission — the coordinator still requires confirmed Secure Boot. | ||
| let warning = status.summary | ||
| #expect(warning.contains("coordinator")) | ||
| #expect(warning.contains("does not guarantee admission")) | ||
| } | ||
|
|
||
| // MARK: - Verdict mapping | ||
|
|
||
| @Test("macOS verdict: Tahoe+ passes, anything below fails") | ||
| func macOSVerdicts() { | ||
| #expect(BootSecurityPolicy.macOSVerdict(26) == .pass) | ||
| #expect(BootSecurityPolicy.macOSVerdict(27) == .pass) | ||
| #expect(BootSecurityPolicy.macOSVerdict(25) == .fail) | ||
| #expect(BootSecurityPolicy.macOSVerdict(15) == .fail) | ||
| #expect(BootSecurityPolicy.minimumMacOSMajorVersion == 26) | ||
| } | ||
|
|
||
| @Test("macOS summary names the version and the Tahoe floor") | ||
| func macOSSummaries() { | ||
| let below = BootSecurityPolicy.macOSSummary(majorVersion: 25) | ||
| #expect(below.contains("25")) | ||
| #expect(below.contains("26")) | ||
| #expect(below.contains("Tahoe")) | ||
| let ok = BootSecurityPolicy.macOSSummary(majorVersion: 26) | ||
| #expect(ok.contains("26")) | ||
| } | ||
|
|
||
| @Test("SIP verdict: only full passes; custom-config is a failure") | ||
| func sipVerdicts() { | ||
| #expect(BootSecurityPolicy.sipVerdict(.enabled) == .pass) | ||
| #expect(BootSecurityPolicy.sipVerdict(.disabled) == .fail) | ||
| #expect(BootSecurityPolicy.sipVerdict( | ||
| .enabledWithCustomConfiguration(disabledProtections: ["Kext Signing"])) == .fail) | ||
| #expect(BootSecurityPolicy.sipVerdict(.unavailable(reason: "x")) == .warn) | ||
| #expect(BootSecurityPolicy.sipVerdict(.unrecognized(output: "?")) == .warn) | ||
| } | ||
|
|
||
| @Test("Secure Boot verdict: Full Security passes, downgrades fail, unknown warns") | ||
| func secureBootVerdicts() { | ||
| #expect(BootSecurityPolicy.secureBootVerdict(.fullSecurity) == .pass) | ||
| #expect(BootSecurityPolicy.secureBootVerdict(.reduced) == .fail) | ||
| #expect(BootSecurityPolicy.secureBootVerdict(.permissiveOrDisabled) == .fail) | ||
| #expect(BootSecurityPolicy.secureBootVerdict(.unavailable(reason: "x")) == .warn) | ||
| } | ||
|
|
||
| // MARK: - SIP "custom configuration" summary | ||
|
|
||
| @Test("SIP custom configuration is summarized as NOT fully enabled") | ||
| func sipCustomConfigSummary() { | ||
| let summary = SIPStatus.enabledWithCustomConfiguration( | ||
| disabledProtections: ["Kext Signing", "Debugging Restrictions"]).summary | ||
| #expect(summary.contains("NOT fully enabled")) | ||
| #expect(summary.contains("Kext Signing")) | ||
| } | ||
|
|
||
| // MARK: - Preflight decision | ||
|
|
||
| @Test("all protections fully on → no block, nothing to print") | ||
| func preflightAllPass() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 26, sip: .enabled, secureBoot: .fullSecurity, allowInsecureOverride: false) | ||
| #expect(decision == .ok) | ||
| #expect(!decision.shouldBlock) | ||
| #expect(decision.messageLines.isEmpty) | ||
| } | ||
|
|
||
| @Test("macOS below the Tahoe floor blocks and prints the upgrade guide") | ||
| func preflightBelowMacOSFloorBlocks() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 25, sip: .enabled, secureBoot: .fullSecurity, allowInsecureOverride: false) | ||
| #expect(decision.shouldBlock) | ||
| let text = decision.messageLines.joined(separator: "\n") | ||
| #expect(text.contains("Software Update")) | ||
| #expect(text.contains("Tahoe")) | ||
| // SIP and Secure Boot are fine here, so their sections are omitted. | ||
| #expect(!text.contains("csrutil enable")) | ||
| #expect(!text.contains("Startup Security Utility")) | ||
| } | ||
|
|
||
| @Test("escape hatch downgrades a below-floor macOS failure to a loud warning") | ||
| func preflightMacOSOverrideDowngrades() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 25, sip: .enabled, secureBoot: .fullSecurity, allowInsecureOverride: true) | ||
| #expect(!decision.shouldBlock) | ||
| #expect(decision.overrodeBlock) | ||
| let text = decision.messageLines.joined(separator: "\n") | ||
| #expect(text.contains(BootSecurityPolicy.overrideEnvVar)) | ||
| #expect(text.contains("development only")) | ||
| } | ||
|
|
||
| @Test("SIP disabled blocks and prints the enable guide") | ||
| func preflightSIPDisabledBlocks() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 26, sip: .disabled, secureBoot: .fullSecurity, allowInsecureOverride: false) | ||
| #expect(decision.shouldBlock) | ||
| let text = decision.messageLines.joined(separator: "\n") | ||
| #expect(text.contains("csrutil enable")) | ||
| #expect(text.contains("System Integrity Protection")) | ||
| // Secure Boot and macOS are fine here, so those sections are omitted. | ||
| #expect(!text.contains("Startup Security Utility")) | ||
| #expect(!text.contains("Software Update")) | ||
| } | ||
|
|
||
| @Test("SIP custom configuration blocks (treated as not fully enabled)") | ||
| func preflightSIPCustomBlocks() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 26, | ||
| sip: .enabledWithCustomConfiguration(disabledProtections: ["Kext Signing"]), | ||
| secureBoot: .fullSecurity, | ||
| allowInsecureOverride: false) | ||
| #expect(decision.shouldBlock) | ||
| #expect(decision.messageLines.joined(separator: "\n").contains("NOT fully enabled")) | ||
| } | ||
|
|
||
| @Test("Secure Boot downgrade (permissiveOrDisabled) blocks and prints the Full Security guide") | ||
| func preflightSecureBootDowngradeBlocks() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 26, sip: .enabled, secureBoot: .permissiveOrDisabled, allowInsecureOverride: false) | ||
| #expect(decision.shouldBlock) | ||
| let text = decision.messageLines.joined(separator: "\n") | ||
| #expect(text.contains("Startup Security Utility")) | ||
| #expect(text.contains("Full Security")) | ||
| #expect(!text.contains("csrutil enable")) | ||
| } | ||
|
|
||
| @Test("undeterminable Secure Boot warns but does not block") | ||
| func preflightUnavailableWarnsOnly() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 26, sip: .enabled, secureBoot: .unavailable(reason: "no level"), | ||
| allowInsecureOverride: false) | ||
| #expect(!decision.shouldBlock) | ||
| #expect(!decision.overrodeBlock) | ||
| #expect(!decision.messageLines.isEmpty) | ||
| #expect(decision.messageLines.joined(separator: "\n").contains("WARNING")) | ||
| } | ||
|
|
||
| @Test("escape hatch downgrades a hard failure to a loud warning") | ||
| func preflightOverrideDowngradesFailure() { | ||
| let decision = BootSecurityPolicy.preflightDecision( | ||
| macOSMajorVersion: 26, sip: .disabled, secureBoot: .reduced, allowInsecureOverride: true) | ||
| #expect(!decision.shouldBlock) | ||
| #expect(decision.overrodeBlock) | ||
| let text = decision.messageLines.joined(separator: "\n") | ||
| #expect(text.contains(BootSecurityPolicy.overrideEnvVar)) | ||
| #expect(text.contains("development only")) | ||
| } | ||
|
|
||
| // MARK: - Shared guidance content (single combined entry point) | ||
|
|
||
| @Test("combined guide (macOS only) names Software Update and Tahoe") | ||
| func macOSGuideContent() { | ||
| let guide = BootSecurityGuidance.guide(includeMacOS: true, includeSIP: false, includeSecureBoot: false) | ||
| #expect(guide.contains("Software Update")) | ||
| #expect(guide.contains("Tahoe")) | ||
| #expect(!guide.contains("csrutil enable")) | ||
| #expect(!guide.contains("Startup Security Utility")) | ||
| } | ||
|
|
||
| @Test("combined guide (SIP only) contains the actionable csrutil command") | ||
| func sipGuideContent() { | ||
| let guide = BootSecurityGuidance.guide(includeMacOS: false, includeSIP: true, includeSecureBoot: false) | ||
| #expect(guide.contains("csrutil enable")) | ||
| #expect(guide.contains("Apple Silicon")) | ||
| #expect(guide.contains("Intel")) | ||
| #expect(!guide.contains("Startup Security Utility")) | ||
| #expect(!guide.contains("Software Update")) | ||
| } | ||
|
|
||
| @Test("combined guide (Secure Boot only) names Startup Security Utility and Full Security") | ||
| func secureBootGuideContent() { | ||
| let guide = BootSecurityGuidance.guide(includeMacOS: false, includeSIP: false, includeSecureBoot: true) | ||
| #expect(guide.contains("Startup Security Utility")) | ||
| #expect(guide.contains("Full Security")) | ||
| #expect(!guide.contains("csrutil enable")) | ||
| // Boot-args remediation was removed (ibridge_secure_boot already reflects it). | ||
| #expect(!guide.contains("boot-args")) | ||
| } | ||
|
|
||
| @Test("combined guide includes only requested sections with exactly one shared footer") | ||
| func combinedGuideSectioning() { | ||
| let all = BootSecurityGuidance.guide(includeMacOS: true, includeSIP: true, includeSecureBoot: true) | ||
| #expect(all.contains("Software Update")) | ||
| #expect(all.contains("csrutil enable")) | ||
| #expect(all.contains("Startup Security Utility")) | ||
| #expect(all.contains("Full Security")) | ||
| // Dedup: the verification footer must appear exactly once even when all | ||
| // sections are present (no doubled footer). | ||
| #expect(occurrences(of: "re-run 'darkbloom doctor'", in: all) == 1) | ||
|
|
||
| let sbOnly = BootSecurityGuidance.guide(includeMacOS: false, includeSIP: false, includeSecureBoot: true) | ||
| #expect(!sbOnly.contains("csrutil enable")) | ||
| #expect(sbOnly.contains("Startup Security Utility")) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔴 [CRITICAL] 🧩 Extremely large test file with 495 lines testing multiple concerns
💡 Suggestion: Split into separate test files: SecureBootStatusTests, BootSecurityPolicyTests, BootSecurityGuidanceTests - each focused on a single class
📊 Score: 4×4 = 16 · Category: over-configuration
Warn locally when macOS, SIP, or Secure Boot posture is below the target state, but keep provider startup non-blocking while coordinator MDM remains the hard trust boundary. Emit categorical boot-posture telemetry at provider startup, mirror the telemetry allowlists, and surface the same warning posture through doctor/status for fleet rollout audits. Builds on Arthur Bodera's boot-policy work in PR #398. Co-authored-by: Arthur Bodera <abodera@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Gajesh2007
changed the title
ethenotethan
left a comment
ethenotethan
left a comment
Contributor
There was a problem hiding this comment.
Automated Code Review — Layr-Labs/d-inference#
Verdict: COMMENT
Security — ✅ No issues found
Performance — ✅ No issues found
Type_diligence — ✅ No issues found
Additive_complexity — ✅ No issues found
✅ All four passes clean. No issues found.
🤖 Automated review by Centaur · DAR-186
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8c5a26a99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !status.attestsSecureBoot { | ||
| logger.warning("Secure Boot check: \(status.summary)") | ||
| } | ||
| return status.attestsSecureBoot |
There was a problem hiding this comment.
When system_profiler SPiBridgeDataType is empty/unavailable or returns a localized level, SecureBootStatusChecker produces .unavailable, but this line signs that as secure_boot_enabled=false. I checked the coordinator paths: initial attestation validation rejects SecureBootEnabled=false, and challenge handling in coordinator/api/provider.go marks the provider untrusted on the same false value, so Macs that the new rollout messaging says should warn and continue will instead be derouted as if Secure Boot were actually disabled. Consider keeping unavailable as an advisory/tri-state for telemetry, or preserving the previous attestation fallback while MDM remains authoritative.
Useful? React with 👍 / 👎.
|
You've missed my comment but this might be a better way to check the status #420 as it doesn't depend on string parsing (which are localised) and instead continues to use the only public API that gives us the posture. |
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
3 participants
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.
Summary
This PR builds on Arthur Bodera's earlier boot-policy work in #398 and rebases it into a warning-only rollout for provider boot posture.
darkbloom start,doctor, andstatusnow share the same sudo-free checks for macOS version, SIP, and Secure Boot, but provider startup does not block on those checks while coordinator MDM remains the hard trust boundary.Provider startup now emits categorical boot-posture telemetry (
boot_macos_*,boot_sip_*,boot_secure_boot_*) withwarnseverity when any posture is below target. The telemetry allowlists are mirrored across Swift, Go, and TS so fleet readiness can be audited before any future hard gate.Credit: co-authored with Arthur Bodera / @Thinkscape; this supersedes and extends his related PR #398.
Before / After
Behavior
flowchart LR subgraph Before A1[darkbloom start] --> B1{SIP enabled?} B1 -- no --> C1[exit: terse error] B1 -- yes --> D1[serve] L1[start --local / launchd foreground] --> E1[serve without shared boot posture visibility] end subgraph After A2[any serve path] --> S2[BootSecuritySnapshot] S2 --> P2{macOS/SIP/Secure Boot target met?} P2 -- yes --> I2[emit provider starting info] P2 -- no/unknown --> W2[print warning + guidance] W2 --> T2[emit provider starting warn with posture fields] I2 --> D2[serve] T2 --> D2[serve] M2[Coordinator MDM] --> H2[hardware trust enforcement] endCode
flowchart TB subgraph Before P1[Start.runPreflightChecks] --> Q1[checkSIPEnabled only] R1[checkSecureBootEnabled] --> S1[authenticated-root / SSV proxy] L1[runLocalStandalone] --> LS1[StandaloneServer.start] F1[runForeground] --> FS1[ProviderLoop.run] end subgraph After L2[runLocalStandalone] --> E2[enforceBootSecurity warning before bind] F2[runForeground] --> E3[enforceBootSecurity warning before coordinator/local endpoint] P2[Start.runPreflightChecks] --> E4[enforceBootSecurity warning before LaunchAgent install] DOC2[doctor/status] --> SNAP2[BootSecuritySnapshot.live] E2 --> SNAP2 E3 --> SNAP2 E4 --> SNAP2 SNAP2 --> POL2[BootSecurityPolicy warn-only rollout verdicts] POL2 --> GUIDE2[BootSecurityGuidance shared guide] E3 --> TEL2[provider startup telemetry fields] TEL2 --> AL2[Swift/Go/TS allowlists] LA2[LaunchAgent plist] --> ENV2[preserves legacy override env] endProxy removed:
AuthenticatedRootStatus,fallbackProxyStatus,.appleSiliconSealed,ReducedReason.checkAuthenticatedRootEnabled()is kept as a clean standalone SSV-seal reader for the independentauthenticated_root_enabledattestation field (no wire change).Test plan
swift test --filter BootSecurityswift test --filter DoctorChecksTestsswift test --filter StatusCommandTestsswift test --filter TelemetrySymmetryTestsswift test --filter LaunchAgentswift buildgo test ./coordinator/api ./coordinator/protocolgit diff --checknpx eslint src/lib/telemetry-types.tscould not start in this worktree because localeslintdependencies are not installedswift testpreviously aborts in unrelated MLX live test setup because the default metallib is missing:Failed to load the default metallibMade with Cursor