permissions: constrain requirements as profiles (#19736)

codex-rs/config/src/config_requirements.rs

@@ -1,8 +1,8 @@
 use codex_protocol::config_types::ApprovalsReviewer;
 use codex_protocol::config_types::SandboxMode;
 use codex_protocol::config_types::WebSearchMode;
+use codex_protocol::models::PermissionProfile;
 use codex_protocol::protocol::AskForApproval;
-use codex_protocol::protocol::SandboxPolicy;
 use codex_utils_absolute_path::AbsolutePathBuf;
 use serde::Deserialize;
 use serde::Serialize;
@@ -84,7 +84,7 @@ impl<T> std::ops::DerefMut for ConstrainedWithSource<T> {
 pub struct ConfigRequirements {
     pub approval_policy: ConstrainedWithSource<AskForApproval>,
     pub approvals_reviewer: ConstrainedWithSource<ApprovalsReviewer>,
-    pub sandbox_policy: ConstrainedWithSource<SandboxPolicy>,
+    pub permission_profile: ConstrainedWithSource<PermissionProfile>,
     pub web_search_mode: ConstrainedWithSource<WebSearchMode>,
     pub feature_requirements: Option<Sourced<FeatureRequirementsToml>>,
     pub managed_hooks: Option<ConstrainedWithSource<ManagedHooksRequirementsToml>>,
@@ -110,8 +110,8 @@ impl Default for ConfigRequirements {
                 Constrained::allow_any_from_default(),
                 /*source*/ None,
             ),
-            sandbox_policy: ConstrainedWithSource::new(
-                Constrained::allow_any(SandboxPolicy::new_read_only_policy()),
+            permission_profile: ConstrainedWithSource::new(
+                Constrained::allow_any(PermissionProfile::read_only()),
                 /*source*/ None,
             ),
             web_search_mode: ConstrainedWithSource::new(
@@ -967,15 +967,8 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
             ),
         };
 
-        // TODO(gt): `ConfigRequirementsToml` should let the author specify the
-        // default `SandboxPolicy`? Should do this for `AskForApproval` too?
-        //
-        // Currently, we force ReadOnly as the default policy because two of
-        // the other variants (WorkspaceWrite, ExternalSandbox) require
-        // additional parameters. Ultimately, we should expand the config
-        // format to allow specifying those parameters.
-        let default_sandbox_policy = SandboxPolicy::new_read_only_policy();
-        let sandbox_policy = match allowed_sandbox_modes {
+        let default_permission_profile = PermissionProfile::read_only();
+        let permission_profile = match allowed_sandbox_modes {
             Some(Sourced {
                 value: modes,
                 source: requirement_source,
@@ -984,23 +977,15 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
                     return Err(ConstraintError::InvalidValue {
                         field_name: "allowed_sandbox_modes",
                         candidate: format!("{modes:?}"),
-                        allowed: "must include 'read-only' to allow any SandboxPolicy".to_string(),
+                        allowed: "must include 'read-only' to allow any PermissionProfile"
+                            .to_string(),
                         requirement_source,
                     });
                 };
 
                 let requirement_source_for_error = requirement_source.clone();
-                let constrained = Constrained::new(default_sandbox_policy, move |candidate| {
-                    let mode = match candidate {
-                        SandboxPolicy::ReadOnly { .. } => SandboxModeRequirement::ReadOnly,
-                        SandboxPolicy::WorkspaceWrite { .. } => {
-                            SandboxModeRequirement::WorkspaceWrite
-                        }
-                        SandboxPolicy::DangerFullAccess => SandboxModeRequirement::DangerFullAccess,
-                        SandboxPolicy::ExternalSandbox { .. } => {
-                            SandboxModeRequirement::ExternalSandbox
-                        }
-                    };
+                let constrained = Constrained::new(default_permission_profile, move |candidate| {
+                    let mode = sandbox_mode_requirement_for_permission_profile(candidate);
                     if modes.contains(&mode) {
                         Ok(())
                     } else {
@@ -1014,12 +999,10 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
                 })?;
                 ConstrainedWithSource::new(constrained, Some(requirement_source))
             }
-            None => {
-                ConstrainedWithSource::new(
-                    Constrained::allow_any(default_sandbox_policy),
-                    /*source*/ None,
-                )
-            }
+            None => ConstrainedWithSource::new(
+                Constrained::allow_any(default_permission_profile),
+                /*source*/ None,
+            ),
         };
         let exec_policy = match rules {
             Some(Sourced { value, source }) => {
@@ -1145,7 +1128,7 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
         Ok(ConfigRequirements {
             approval_policy,
             approvals_reviewer,
-            sandbox_policy,
+            permission_profile,
             web_search_mode,
             feature_requirements,
             managed_hooks,
@@ -1159,6 +1142,29 @@ impl TryFrom<ConfigRequirementsWithSources> for ConfigRequirements {
     }
 }
 
+pub fn sandbox_mode_requirement_for_permission_profile(
+    permission_profile: &PermissionProfile,
+) -> SandboxModeRequirement {
+    match permission_profile {
+        PermissionProfile::Disabled => SandboxModeRequirement::DangerFullAccess,
+        PermissionProfile::External { .. } => SandboxModeRequirement::ExternalSandbox,
+        PermissionProfile::Managed { .. } => {
+            let file_system_policy = permission_profile.file_system_sandbox_policy();
+            if file_system_policy.has_full_disk_write_access() {
+                SandboxModeRequirement::DangerFullAccess
+            } else if file_system_policy
+                .entries
+                .iter()
+                .any(|entry| entry.access.can_write())
+            {
+                SandboxModeRequirement::WorkspaceWrite
+            } else {
+                SandboxModeRequirement::ReadOnly
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1168,6 +1174,7 @@ mod tests {
     use codex_execpolicy::Evaluation;
     use codex_execpolicy::RuleMatch;
     use codex_protocol::protocol::NetworkAccess;
+    use codex_protocol::protocol::SandboxPolicy;
     use codex_utils_absolute_path::AbsolutePathBuf;
     use codex_utils_absolute_path::AbsolutePathBufGuard;
     use pretty_assertions::assert_eq;
@@ -1183,6 +1190,10 @@ mod tests {
         )?)
     }
 
+    fn profile_from_sandbox_policy(sandbox_policy: &SandboxPolicy) -> PermissionProfile {
+        PermissionProfile::from_legacy_sandbox_policy(sandbox_policy)
+    }
+
     fn with_unknown_source(toml: ConfigRequirementsToml) -> ConfigRequirementsWithSources {
         let ConfigRequirementsToml {
             allowed_approval_policies,
@@ -1724,8 +1735,10 @@ allowed_approvals_reviewers = ["user"]
         );
         assert_eq!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::DangerFullAccess),
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::DangerFullAccess,
+                )),
             Err(ConstraintError::InvalidValue {
                 field_name: "sandbox_mode",
                 candidate: "DangerFullAccess".into(),
@@ -1803,7 +1816,7 @@ allowed_approvals_reviewers = ["user"]
             Some(source_location.clone())
         );
         assert_eq!(
-            requirements.sandbox_policy.source,
+            requirements.permission_profile.source,
             Some(source_location.clone())
         );
         assert_eq!(
@@ -1869,8 +1882,10 @@ allowed_approvals_reviewers = ["user"]
         );
         assert!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::new_read_only_policy())
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::new_read_only_policy()
+                ))
                 .is_ok()
         );
 
@@ -1952,25 +1967,30 @@ allowed_approvals_reviewers = ["user"]
         let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };
         assert!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::new_read_only_policy())
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::new_read_only_policy()
+                ))
                 .is_ok()
         );
+        let workspace_write_policy = SandboxPolicy::WorkspaceWrite {
+            writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
+            network_access: false,
+            exclude_tmpdir_env_var: false,
+            exclude_slash_tmp: false,
+        };
         assert!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::WorkspaceWrite {
-                    writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
-                    network_access: false,
-                    exclude_tmpdir_env_var: false,
-                    exclude_slash_tmp: false,
-                })
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(&workspace_write_policy))
                 .is_ok()
         );
         assert_eq!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::DangerFullAccess),
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::DangerFullAccess,
+                )),
             Err(ConstraintError::InvalidValue {
                 field_name: "sandbox_mode",
                 candidate: "DangerFullAccess".into(),
@@ -1980,10 +2000,12 @@ allowed_approvals_reviewers = ["user"]
         );
         assert_eq!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::ExternalSandbox {
-                    network_access: NetworkAccess::Restricted,
-                }),
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::ExternalSandbox {
+                        network_access: NetworkAccess::Restricted,
+                    }
+                )),
             Err(ConstraintError::InvalidValue {
                 field_name: "sandbox_mode",
                 candidate: "ExternalSandbox".into(),
@@ -2064,21 +2086,24 @@ allowed_approvals_reviewers = ["user"]
 
         let requirements = ConfigRequirements::try_from(requirements_with_sources)?;
         let root = if cfg!(windows) { "C:\\repo" } else { "/repo" };
+        let workspace_write_policy = SandboxPolicy::WorkspaceWrite {
+            writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
+            network_access: false,
+            exclude_tmpdir_env_var: false,
+            exclude_slash_tmp: false,
+        };
         assert!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::WorkspaceWrite {
-                    writable_roots: vec![AbsolutePathBuf::from_absolute_path(root)?],
-                    network_access: false,
-                    exclude_tmpdir_env_var: false,
-                    exclude_slash_tmp: false,
-                })
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(&workspace_write_policy))
                 .is_ok()
         );
         assert_eq!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::DangerFullAccess),
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::DangerFullAccess,
+                )),
             Err(ConstraintError::InvalidValue {
                 field_name: "sandbox_mode",
                 candidate: "DangerFullAccess".into(),
@@ -2108,8 +2133,10 @@ allowed_approvals_reviewers = ["user"]
 
         assert_eq!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::DangerFullAccess),
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::DangerFullAccess,
+                )),
             Err(ConstraintError::InvalidValue {
                 field_name: "sandbox_mode",
                 candidate: "DangerFullAccess".into(),
@@ -2147,8 +2174,10 @@ allowed_approvals_reviewers = ["user"]
 
         assert_eq!(
             requirements
-                .sandbox_policy
-                .can_set(&SandboxPolicy::new_workspace_write_policy()),
+                .permission_profile
+                .can_set(&profile_from_sandbox_policy(
+                    &SandboxPolicy::new_workspace_write_policy(),
+                )),
             Err(ConstraintError::InvalidValue {
                 field_name: "sandbox_mode",
                 candidate: "WorkspaceWrite".into(),