-
Notifications
You must be signed in to change notification settings - Fork 83
feat(seatbelt): add system power access #1066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c14791b
ddbaacd
654b66a
1a5bc5d
fc97809
9e042bc
9f0d93e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,7 @@ pub fn build_profile_with_proxy( | |
| write_network_rules(&mut out, request, proxy_address); | ||
| write_nested_pty_rules(&mut out, request); | ||
| write_keychain_rules(&mut out, request)?; | ||
| write_system_power_rules(&mut out, request); | ||
| write_extra_seatbelt_rules(&mut out, request); | ||
| write_ui_rules(&mut out, request); | ||
|
|
||
|
|
@@ -590,6 +591,26 @@ fn write_keychain_rules(out: &mut String, request: &ExecutionRequest) -> Result< | |
| Ok(()) | ||
| } | ||
|
|
||
| /// Emit the narrow Seatbelt capability needed for system power notifications | ||
| /// and assertions. Off by default because opening `RootDomainUserClient` also | ||
| /// exposes host power-management operations beyond notification registration. | ||
| fn write_system_power_rules(out: &mut String, request: &ExecutionRequest) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most methods have the out parameter as the last parameter. Can the out variable be moved to the end? Also, why have an out param instead of a return value? |
||
| let enabled = request | ||
| .seatbelt | ||
| .as_ref() | ||
| .is_some_and(|c| c.system_power_access); | ||
| if !enabled { | ||
| return; | ||
| } | ||
|
|
||
| out.push_str(";; --- systemPowerAccess: sleep/wake notifications and assertions ---\n"); | ||
| out.push_str("(allow mach-lookup\n"); | ||
| out.push_str(" (global-name \"com.apple.PowerManagement.control\")\n"); | ||
| out.push_str(" (global-name \"com.apple.iokit.powerdxpc\"))\n"); | ||
| out.push_str("(allow iokit-open\n"); | ||
| out.push_str(" (iokit-user-client-class \"RootDomainUserClient\"))\n"); | ||
| } | ||
|
|
||
| /// Emit caller-provided `extraMachLookups` rules: additional Mach service | ||
| /// global-names the inner process may resolve. No-op when the list is empty. | ||
| fn write_extra_seatbelt_rules(out: &mut String, request: &ExecutionRequest) { | ||
|
|
@@ -1748,6 +1769,31 @@ mod tests { | |
| assert!(!p.contains("/private/var/db/mds")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn system_power_access_default_off_omits_power_services() { | ||
| let r = req(); | ||
| let p = build_profile(&r).unwrap(); | ||
| assert!(!p.contains("systemPowerAccess")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can these strings ever change? If they do will this test catch that change? |
||
| assert!(!p.contains("com.apple.PowerManagement.control")); | ||
| assert!(!p.contains("com.apple.iokit.powerdxpc")); | ||
| assert!(!p.contains("RootDomainUserClient")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn system_power_access_true_allows_power_services() { | ||
| let mut r = req(); | ||
| r.seatbelt = Some(SeatbeltConfig { | ||
| system_power_access: true, | ||
| ..Default::default() | ||
| }); | ||
| let p = build_profile(&r).unwrap(); | ||
| assert!(p.contains("systemPowerAccess")); | ||
| assert!(p.contains("(global-name \"com.apple.PowerManagement.control\")")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kinda odd that the names here have a quite around them but the previous test has no quotes. Is this delibrate? |
||
| assert!(p.contains("(global-name \"com.apple.iokit.powerdxpc\")")); | ||
| assert!(p.contains("(iokit-user-client-class \"RootDomainUserClient\")")); | ||
| assert!(!p.contains("(allow iokit-open)\n")); | ||
| } | ||
|
|
||
| // Keychain rules expand `~/Library/Keychains` from $HOME at build | ||
| // time, so the tests that exercise `keychain_access: true` are gated | ||
| // to macOS (the only OS where this code path is actually used and | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,32 @@ use wxc_common::models::{ | |
| ContainerPolicy, ExecutionRequest, NetworkAction, NetworkEnforcementMode, NetworkPolicy, | ||
| }; | ||
|
|
||
| const SYSTEM_POWER_ACCESS_VERSION_ERROR: &str = | ||
| "seatbelt.systemPowerAccess requires schema version 0.9 or later"; | ||
|
|
||
| fn system_power_access_support(version: &str) -> Option<bool> { | ||
| semver::Version::parse(version) | ||
| .ok() | ||
| .map(|version| version.major > 0 || version.minor >= 9) | ||
| } | ||
|
|
||
| /// Reject system power access when the request predates its 0.9 contract. | ||
| /// | ||
| /// This validation runs at execution time so requests changed after parsing, | ||
| /// including through the Rust SDK setters, cannot bypass the version boundary. | ||
| pub fn validate_system_power_access(request: &ExecutionRequest) -> Result<(), String> { | ||
| let enabled = request | ||
| .seatbelt | ||
| .as_ref() | ||
| .is_some_and(|seatbelt| seatbelt.system_power_access); | ||
|
|
||
| if enabled && !system_power_access_support(&request.schema_version).unwrap_or(false) { | ||
| return Err(SYSTEM_POWER_ACCESS_VERSION_ERROR.to_string()); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| /// Effective GUI posture: `seatbelt.guiAccess` only means anything when the UI | ||
| /// policy leaves UI enabled, since every GUI grant is emitted alongside the | ||
| /// WindowServer allows. Single source of truth so the profile builder and the | ||
|
|
@@ -233,6 +259,28 @@ mod tests { | |
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn system_power_access_support_starts_at_v09() { | ||
| assert_eq!(system_power_access_support("0.8.0-alpha"), Some(false)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why only go down to 0.8.0? |
||
| assert_eq!(system_power_access_support("0.9.0-alpha"), Some(true)); | ||
| assert_eq!(system_power_access_support("invalid"), None); | ||
| } | ||
|
|
||
| #[test] | ||
| fn system_power_access_validation_rejects_pre_v09_requests() { | ||
| let request = ExecutionRequest { | ||
| schema_version: "0.8.0-alpha".to_string(), | ||
| seatbelt: Some(SeatbeltConfig { | ||
| system_power_access: true, | ||
| ..Default::default() | ||
| }), | ||
| ..Default::default() | ||
| }; | ||
|
|
||
| let error = validate_system_power_access(&request).unwrap_err(); | ||
| assert!(error.contains("schema version 0.9"), "got: {error}"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn rejects_proxy_with_default_allow() { | ||
| // Outbound is already unrestricted under 'allow' β a proxy adds no | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -124,6 +124,7 @@ impl SandboxBackend for SeatbeltScriptRunner { | |
|
|
||
| // Seatbelt's own invariants β the only home for them, so a caller that | ||
| // builds an ExecutionRequest directly gets the same rules. | ||
| crate::seatbelt_policy::validate_system_power_access(request).map_err(error_response)?; | ||
| crate::seatbelt_policy::validate_seatbelt_network_policy(&request.policy) | ||
| .map_err(error_response)?; | ||
| crate::seatbelt_policy::validate_seatbelt_ui_policy(request).map_err(error_response)?; | ||
|
|
@@ -855,6 +856,23 @@ mod tests { | |
| assert!(response.error_message.contains("cannot be enforced")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn rejects_system_power_access_before_v09() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this is the same test as |
||
| let mut request = base_request(); | ||
| request.schema_version = "0.8.0-alpha".to_string(); | ||
| request | ||
| .seatbelt | ||
| .as_mut() | ||
| .expect("Seatbelt config") | ||
| .system_power_access = true; | ||
|
|
||
| let error = SeatbeltScriptRunner::new().validate(&request).unwrap_err(); | ||
| assert!( | ||
| error.error_message.contains("schema version 0.9"), | ||
| "{error:?}" | ||
| ); | ||
| } | ||
|
|
||
| /// The parser is not a door at all for these rules: `validate` is the only | ||
| /// place they live, and `mxc_engine` will happily take an `ExecutionRequest` | ||
| /// built by hand. These assert `validate` rejects them without any help from | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -348,6 +348,9 @@ pub struct Seatbelt { | |
| /// Whether macOS Keychain access is allowed. | ||
| #[serde(default)] | ||
| pub keychain_access: OptionalField<bool>, | ||
| /// Whether system sleep/wake notifications and power assertions are allowed. | ||
| #[serde(default)] | ||
| pub system_power_access: OptionalField<bool>, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment states |
||
| /// Additional Mach service global names the process may resolve. | ||
| #[serde(default)] | ||
| pub extra_mach_lookups: OptionalField<Vec<String>>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ fn accepts_complete_seatbelt_object() { | |
| "launchMethod": "exec", | ||
| "nestedPty": false, | ||
| "keychainAccess": false, | ||
| "systemPowerAccess": false, | ||
| "extraMachLookups": ["com.apple.securityd", "com.apple.coreservices.launchservicesd"] | ||
| }, | ||
| "process": {"commandLine": "echo"} | ||
|
|
@@ -165,6 +166,23 @@ fn rejects_non_boolean_keychain_access_values() { | |
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn rejects_non_boolean_system_power_access_values() { | ||
| for system_power_access in ["0", "1", "\"string\"", "[]", "{}"] { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think Copilot went a bit too in-depth with its unit testing. |
||
| let json = format!( | ||
| r#"{{ | ||
| "version": "0.9.0-alpha", | ||
| "seatbelt": {{ | ||
| "systemPowerAccess": {system_power_access} | ||
| }}, | ||
| "process": {{"commandLine": "echo"}} | ||
| }}"# | ||
| ); | ||
|
|
||
| assert_invalid(&json); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn rejects_unknown_seatbelt_field() { | ||
| let json = r#"{ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is a
narrowcapability?