-
Notifications
You must be signed in to change notification settings - Fork 26
Promote seatbelt from experimental to stable #500
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -228,6 +228,9 @@ struct RawConfig { | |
| network: Option<RawNetwork>, | ||
| ui: Option<RawUi>, | ||
| experimental: Option<RawExperimental>, | ||
| /// Top-level seatbelt config (preferred over experimental.seatbelt). | ||
| #[serde(alias = "macos_sandbox")] | ||
| seatbelt: Option<RawSeatbelt>, | ||
| } | ||
|
|
||
| // State-aware request shape. `phase` is required (no `#[serde(default)]` on | ||
|
|
@@ -618,13 +621,16 @@ fn present_backend_sections(raw: &RawConfig) -> Vec<&'static str> { | |
| if experimental.wslc.is_some() { | ||
| push(ContainmentBackend::Wslc); | ||
| } | ||
| if experimental.seatbelt.is_some() { | ||
| if experimental.seatbelt.is_some() && raw.seatbelt.is_none() { | ||
| push(ContainmentBackend::Seatbelt); | ||
| } | ||
| if experimental.isolation_session.is_some() { | ||
| push(ContainmentBackend::IsolationSession); | ||
| } | ||
| } | ||
| if raw.seatbelt.is_some() { | ||
| push(ContainmentBackend::Seatbelt); | ||
| } | ||
| sections | ||
| } | ||
|
|
||
|
|
@@ -1211,6 +1217,26 @@ fn convert_raw_config_inner( | |
| ExperimentalConfig::default() | ||
| }; | ||
|
|
||
| // Top-level `seatbelt` takes precedence over `experimental.seatbelt`. | ||
| // This supports the two-phase migration: new configs use the top-level key, | ||
| // old configs still work via the experimental path. | ||
| let experimental = if let Some(raw_sb) = raw.seatbelt { | ||
|
Comment on lines
+1220
to
+1223
Collaborator
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. issue: my thinking is that we should error if the user provides both. The migration would then be either one will work but you can only specify one e.g either top level or experimental but not together. I think they should be mutually exclusive from the config creators perspective while the parser allows for both. Does that make sense? |
||
| let sb = SeatbeltConfig { | ||
| profile_override: raw_sb.profile_override, | ||
| gui_access: raw_sb.gui_access.unwrap_or(false), | ||
| launch_method: raw_sb.launch_method.unwrap_or_default(), | ||
| nested_pty: raw_sb.nested_pty.unwrap_or(true), | ||
| keychain_access: raw_sb.keychain_access.unwrap_or(false), | ||
| extra_mach_lookups: raw_sb.extra_mach_lookups.unwrap_or_default(), | ||
| }; | ||
| ExperimentalConfig { | ||
| seatbelt: Some(sb), | ||
| ..experimental | ||
| } | ||
| } else { | ||
| experimental | ||
| }; | ||
|
|
||
| // UI section | ||
| if let Some(raw_ui) = raw.ui { | ||
| let clipboard = match raw_ui.clipboard.as_deref() { | ||
|
|
@@ -1281,6 +1307,7 @@ fn convert_raw_state_aware( | |
| // one-shot RawExperimental; it is preserved separately on | ||
| // ParsedStateAwareRequest as raw JSON. | ||
| experimental: None, | ||
| seatbelt: None, | ||
| }; | ||
|
|
||
| let require_process = phase == Phase::Exec; | ||
|
|
@@ -3312,6 +3339,36 @@ mod tests { | |
| assert!(cfg.keychain_access); | ||
| } | ||
|
|
||
| #[test] | ||
| fn top_level_seatbelt_config_accepted() { | ||
| let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "seatbelt", "seatbelt": {"nestedPty": false, "keychainAccess": true}}"#; | ||
| let encoded = base64_encode(json.as_bytes()); | ||
| let mut logger = test_logger(); | ||
|
|
||
| let req = load_request(&encoded, &mut logger, true).unwrap(); | ||
| let cfg = req | ||
| .experimental | ||
| .seatbelt | ||
| .expect("top-level seatbelt should populate experimental.seatbelt internally"); | ||
| assert!(!cfg.nested_pty); | ||
| assert!(cfg.keychain_access); | ||
| } | ||
|
|
||
| #[test] | ||
| fn top_level_seatbelt_takes_precedence_over_experimental() { | ||
| let json = r#"{"process": {"commandLine": "echo hi"}, "containment": "seatbelt", "seatbelt": {"nestedPty": false}, "experimental": {"seatbelt": {"nestedPty": true}}}"#; | ||
| let encoded = base64_encode(json.as_bytes()); | ||
| let mut logger = test_logger(); | ||
|
|
||
| let req = load_request(&encoded, &mut logger, true).unwrap(); | ||
| let cfg = req | ||
| .experimental | ||
| .seatbelt | ||
| .expect("seatbelt config should be set"); | ||
| // Top-level wins over experimental.seatbelt | ||
| assert!(!cfg.nested_pty); | ||
| } | ||
|
|
||
| // Legacy wire-name aliases. The parser accepts the pre-0.6 wire vocabulary | ||
| // (`appcontainer`, `macos_sandbox`, and the `appContainer` / | ||
| // `experimental.macos_sandbox` sub-block keys) so that configs declaring | ||
|
|
@@ -3450,7 +3507,7 @@ mod tests { | |
| assert_multi_backend_rejected( | ||
| "processcontainer", | ||
| r#""experimental": {"seatbelt": {"guiAccess": true}}"#, | ||
| "experimental.seatbelt", | ||
| "seatbelt", | ||
| ); | ||
| } | ||
|
|
||
|
|
||
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.
This PR seems to go against what is laid out in docs\versioning.md