mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: merge skill permission profiles into the turn sandbox for zsh-fork execs (#13496)
## Summary This changes the Unix shell escalation path for skill-matched executables to apply a skill's `PermissionProfile` as additive permissions on top of the existing turn/request sandbox policy. Previously, skill-matched executables compiled the skill permission profile into a standalone sandbox policy and executed against that replacement policy. Now they go through the same `additional_permissions` merge path used elsewhere in shell sandbox preparation. ## What Changed - Changed `skill_escalation_execution()` to return `EscalationPermissions::PermissionProfile(...)` for non-empty skill permission profiles. - Kept empty or missing skill permission profiles on the `TurnDefault` path. - Added tests covering the new additive skill-permission behavior. - Added inline comments in `prepare_escalated_exec()` clarifying the difference between additive permission merging and fully specified replacement sandbox policies. - Removed the now-unused skill permission compiler module after switching this path away from standalone compiled skill sandbox policies. ## Testing - Ran `just fmt` in `codex-rs` - Ran `cargo test -p codex-core` `cargo test -p codex-core` still hits an unrelated existing failure: `shell_snapshot::tests::snapshot_shell_does_not_inherit_stdin` ## Follow-up This change intentionally does not merge skill-specific macOS seatbelt profile extensions through the `additional_permissions` path yet. Filesystem and network permissions now follow the additive merge path, but seatbelt extension permissions still need separate handling in a follow-up PR.
This commit is contained in:
committed by
GitHub
Unverified
parent
9fcbbeb5ae
commit
a63624a61a
@@ -4,7 +4,6 @@ pub(crate) mod invocation_utils;
|
||||
pub mod loader;
|
||||
pub mod manager;
|
||||
pub mod model;
|
||||
pub mod permissions;
|
||||
pub mod remote;
|
||||
pub mod render;
|
||||
pub mod system;
|
||||
|
||||
@@ -1,454 +0,0 @@
|
||||
#[cfg(any(unix, test))]
|
||||
use std::collections::HashSet;
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsAutomationValue;
|
||||
#[cfg(any(unix, test))]
|
||||
use codex_protocol::models::MacOsPermissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsPreferencesValue;
|
||||
#[cfg(any(unix, test))]
|
||||
use codex_protocol::models::MacOsSeatbeltProfileExtensions;
|
||||
#[cfg(any(unix, test))]
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
#[cfg(any(unix, test))]
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
#[cfg(any(unix, test))]
|
||||
use dunce::canonicalize as canonicalize_path;
|
||||
#[cfg(any(unix, test))]
|
||||
use tracing::warn;
|
||||
|
||||
#[cfg(any(unix, test))]
|
||||
use crate::config::Constrained;
|
||||
#[cfg(any(unix, test))]
|
||||
use crate::config::Permissions;
|
||||
#[cfg(any(unix, test))]
|
||||
use crate::config::types::ShellEnvironmentPolicy;
|
||||
#[cfg(any(unix, test))]
|
||||
use crate::protocol::AskForApproval;
|
||||
#[cfg(any(unix, test))]
|
||||
use crate::protocol::ReadOnlyAccess;
|
||||
#[cfg(any(unix, test))]
|
||||
use crate::protocol::SandboxPolicy;
|
||||
|
||||
/// Compiles a skill `PermissionProfile` for the Unix shell escalation path.
|
||||
///
|
||||
/// Normal Windows builds do not currently call this helper, so it is only
|
||||
/// compiled on Unix and in tests.
|
||||
#[cfg(any(unix, test))]
|
||||
pub(crate) fn compile_permission_profile(
|
||||
permissions: Option<PermissionProfile>,
|
||||
) -> Option<Permissions> {
|
||||
let PermissionProfile {
|
||||
network,
|
||||
file_system,
|
||||
macos,
|
||||
} = permissions?;
|
||||
let network_access = network.and_then(|value| value.enabled).unwrap_or_default();
|
||||
let file_system = file_system.unwrap_or_default();
|
||||
let fs_read = normalize_permission_paths(
|
||||
file_system.read.as_deref().unwrap_or_default(),
|
||||
"permissions.file_system.read",
|
||||
);
|
||||
let fs_write = normalize_permission_paths(
|
||||
file_system.write.as_deref().unwrap_or_default(),
|
||||
"permissions.file_system.write",
|
||||
);
|
||||
let sandbox_policy = if !fs_write.is_empty() {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: fs_write,
|
||||
read_only_access: if fs_read.is_empty() {
|
||||
ReadOnlyAccess::FullAccess
|
||||
} else {
|
||||
ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: true,
|
||||
readable_roots: fs_read,
|
||||
}
|
||||
},
|
||||
network_access,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
}
|
||||
} else if !fs_read.is_empty() {
|
||||
SandboxPolicy::ReadOnly {
|
||||
access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: true,
|
||||
readable_roots: fs_read,
|
||||
},
|
||||
network_access,
|
||||
}
|
||||
} else if network_access {
|
||||
SandboxPolicy::ReadOnly {
|
||||
access: ReadOnlyAccess::FullAccess,
|
||||
network_access: true,
|
||||
}
|
||||
} else {
|
||||
// Default sandbox policy
|
||||
SandboxPolicy::new_read_only_policy()
|
||||
};
|
||||
let macos_permissions = macos.unwrap_or_default();
|
||||
let macos_seatbelt_profile_extensions =
|
||||
build_macos_seatbelt_profile_extensions(&macos_permissions);
|
||||
|
||||
Some(Permissions {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::Never),
|
||||
sandbox_policy: Constrained::allow_any(sandbox_policy),
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
macos_seatbelt_profile_extensions,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(any(unix, test))]
|
||||
fn normalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec<AbsolutePathBuf> {
|
||||
let mut paths = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
|
||||
for value in values {
|
||||
let Some(path) = normalize_permission_path(value, field) else {
|
||||
continue;
|
||||
};
|
||||
if seen.insert(path.clone()) {
|
||||
paths.push(path);
|
||||
}
|
||||
}
|
||||
|
||||
paths
|
||||
}
|
||||
|
||||
#[cfg(any(unix, test))]
|
||||
fn normalize_permission_path(value: &AbsolutePathBuf, field: &str) -> Option<AbsolutePathBuf> {
|
||||
let canonicalized = canonicalize_path(value.as_path()).unwrap_or_else(|_| value.to_path_buf());
|
||||
match AbsolutePathBuf::from_absolute_path(&canonicalized) {
|
||||
Ok(path) => Some(path),
|
||||
Err(error) => {
|
||||
warn!("ignoring {field}: expected absolute path, got {canonicalized:?}: {error}");
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
fn build_macos_seatbelt_profile_extensions(
|
||||
permissions: &MacOsPermissions,
|
||||
) -> Option<MacOsSeatbeltProfileExtensions> {
|
||||
let defaults = MacOsSeatbeltProfileExtensions::default();
|
||||
|
||||
let extensions = MacOsSeatbeltProfileExtensions {
|
||||
macos_preferences: resolve_macos_preferences_permission(
|
||||
permissions.preferences.as_ref(),
|
||||
defaults.macos_preferences,
|
||||
),
|
||||
macos_automation: resolve_macos_automation_permission(
|
||||
permissions.automations.as_ref(),
|
||||
defaults.macos_automation,
|
||||
),
|
||||
macos_accessibility: permissions
|
||||
.accessibility
|
||||
.unwrap_or(defaults.macos_accessibility),
|
||||
macos_calendar: permissions.calendar.unwrap_or(defaults.macos_calendar),
|
||||
};
|
||||
Some(extensions)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
fn resolve_macos_preferences_permission(
|
||||
value: Option<&MacOsPreferencesValue>,
|
||||
default: crate::seatbelt_permissions::MacOsPreferencesPermission,
|
||||
) -> crate::seatbelt_permissions::MacOsPreferencesPermission {
|
||||
use crate::seatbelt_permissions::MacOsPreferencesPermission;
|
||||
|
||||
match value {
|
||||
Some(MacOsPreferencesValue::Bool(true)) => MacOsPreferencesPermission::ReadOnly,
|
||||
Some(MacOsPreferencesValue::Bool(false)) => MacOsPreferencesPermission::None,
|
||||
Some(MacOsPreferencesValue::Mode(mode)) => {
|
||||
let mode = mode.trim();
|
||||
if mode.eq_ignore_ascii_case("readonly") || mode.eq_ignore_ascii_case("read-only") {
|
||||
MacOsPreferencesPermission::ReadOnly
|
||||
} else if mode.eq_ignore_ascii_case("readwrite")
|
||||
|| mode.eq_ignore_ascii_case("read-write")
|
||||
{
|
||||
MacOsPreferencesPermission::ReadWrite
|
||||
} else {
|
||||
warn!(
|
||||
"ignoring permissions.macos.preferences: expected true/false, readonly, or readwrite"
|
||||
);
|
||||
default
|
||||
}
|
||||
}
|
||||
None => default,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
fn resolve_macos_automation_permission(
|
||||
value: Option<&MacOsAutomationValue>,
|
||||
default: crate::seatbelt_permissions::MacOsAutomationPermission,
|
||||
) -> crate::seatbelt_permissions::MacOsAutomationPermission {
|
||||
use crate::seatbelt_permissions::MacOsAutomationPermission;
|
||||
|
||||
match value {
|
||||
Some(MacOsAutomationValue::Bool(true)) => MacOsAutomationPermission::All,
|
||||
Some(MacOsAutomationValue::Bool(false)) => MacOsAutomationPermission::None,
|
||||
Some(MacOsAutomationValue::BundleIds(bundle_ids)) => {
|
||||
let bundle_ids = bundle_ids
|
||||
.iter()
|
||||
.map(|bundle_id| bundle_id.trim())
|
||||
.filter(|bundle_id| !bundle_id.is_empty())
|
||||
.map(ToOwned::to_owned)
|
||||
.collect::<Vec<String>>();
|
||||
if bundle_ids.is_empty() {
|
||||
MacOsAutomationPermission::None
|
||||
} else {
|
||||
MacOsAutomationPermission::BundleIds(bundle_ids)
|
||||
}
|
||||
}
|
||||
None => default,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(not(target_os = "macos"), any(unix, test)))]
|
||||
fn build_macos_seatbelt_profile_extensions(
|
||||
_: &MacOsPermissions,
|
||||
) -> Option<MacOsSeatbeltProfileExtensions> {
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::compile_permission_profile;
|
||||
use crate::config::Constrained;
|
||||
use crate::config::Permissions;
|
||||
use crate::config::types::ShellEnvironmentPolicy;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::ReadOnlyAccess;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsAutomationValue;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsPermissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsPreferencesValue;
|
||||
use codex_protocol::models::NetworkPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
fn absolute_path(path: &Path) -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::try_from(path).expect("absolute path")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compile_permission_profile_normalizes_paths() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
fs::create_dir_all(skill_dir.join("scripts")).expect("skill dir");
|
||||
let read_dir = skill_dir.join("data");
|
||||
fs::create_dir_all(&read_dir).expect("read dir");
|
||||
|
||||
let profile = compile_permission_profile(Some(PermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![
|
||||
absolute_path(&skill_dir.join("data")),
|
||||
absolute_path(&skill_dir.join("data")),
|
||||
absolute_path(&skill_dir.join("scripts/../data")),
|
||||
]),
|
||||
write: Some(vec![absolute_path(&skill_dir.join("output"))]),
|
||||
}),
|
||||
..Default::default()
|
||||
}))
|
||||
.expect("profile");
|
||||
|
||||
assert_eq!(
|
||||
profile,
|
||||
Permissions {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::Never),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![
|
||||
AbsolutePathBuf::try_from(skill_dir.join("output"))
|
||||
.expect("absolute output path")
|
||||
],
|
||||
read_only_access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: true,
|
||||
readable_roots: vec![
|
||||
AbsolutePathBuf::try_from(
|
||||
dunce::canonicalize(&read_dir).unwrap_or(read_dir)
|
||||
)
|
||||
.expect("absolute read path")
|
||||
],
|
||||
},
|
||||
network_access: true,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
}),
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: Some(
|
||||
crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(),
|
||||
),
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compile_permission_profile_without_permissions_has_empty_profile() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
fs::create_dir_all(&skill_dir).expect("skill dir");
|
||||
|
||||
let profile = compile_permission_profile(None);
|
||||
|
||||
assert_eq!(profile, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compile_permission_profile_with_network_only_uses_read_only_policy() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
fs::create_dir_all(&skill_dir).expect("skill dir");
|
||||
|
||||
let profile = compile_permission_profile(Some(PermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
}))
|
||||
.expect("profile");
|
||||
|
||||
assert_eq!(
|
||||
profile,
|
||||
Permissions {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::Never),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly {
|
||||
access: ReadOnlyAccess::FullAccess,
|
||||
network_access: true,
|
||||
}),
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: Some(
|
||||
crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(),
|
||||
),
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compile_permission_profile_with_network_and_read_only_paths_uses_read_only_policy() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
let read_dir = skill_dir.join("data");
|
||||
fs::create_dir_all(&read_dir).expect("read dir");
|
||||
|
||||
let profile = compile_permission_profile(Some(PermissionProfile {
|
||||
network: Some(NetworkPermissions {
|
||||
enabled: Some(true),
|
||||
}),
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: Some(vec![absolute_path(&skill_dir.join("data"))]),
|
||||
write: Some(Vec::new()),
|
||||
}),
|
||||
..Default::default()
|
||||
}))
|
||||
.expect("profile");
|
||||
|
||||
assert_eq!(
|
||||
profile,
|
||||
Permissions {
|
||||
approval_policy: Constrained::allow_any(AskForApproval::Never),
|
||||
sandbox_policy: Constrained::allow_any(SandboxPolicy::ReadOnly {
|
||||
access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: true,
|
||||
readable_roots: vec![
|
||||
AbsolutePathBuf::try_from(
|
||||
dunce::canonicalize(&read_dir).unwrap_or(read_dir)
|
||||
)
|
||||
.expect("absolute read path")
|
||||
],
|
||||
},
|
||||
network_access: true,
|
||||
}),
|
||||
network: None,
|
||||
allow_login_shell: true,
|
||||
shell_environment_policy: ShellEnvironmentPolicy::default(),
|
||||
windows_sandbox_mode: None,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: Some(
|
||||
crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default(),
|
||||
),
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn compile_permission_profile_builds_macos_permission_file() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
fs::create_dir_all(&skill_dir).expect("skill dir");
|
||||
|
||||
let profile = compile_permission_profile(Some(PermissionProfile {
|
||||
macos: Some(MacOsPermissions {
|
||||
preferences: Some(MacOsPreferencesValue::Mode("readwrite".to_string())),
|
||||
automations: Some(MacOsAutomationValue::BundleIds(vec![
|
||||
"com.apple.Notes".to_string(),
|
||||
])),
|
||||
accessibility: Some(true),
|
||||
calendar: Some(true),
|
||||
}),
|
||||
..Default::default()
|
||||
}))
|
||||
.expect("profile");
|
||||
|
||||
assert_eq!(
|
||||
profile.macos_seatbelt_profile_extensions,
|
||||
Some(
|
||||
crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions {
|
||||
macos_preferences:
|
||||
crate::seatbelt_permissions::MacOsPreferencesPermission::ReadWrite,
|
||||
macos_automation:
|
||||
crate::seatbelt_permissions::MacOsAutomationPermission::BundleIds(vec![
|
||||
"com.apple.Notes".to_string()
|
||||
],),
|
||||
macos_accessibility: true,
|
||||
macos_calendar: true,
|
||||
}
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
#[test]
|
||||
fn compile_permission_profile_uses_macos_defaults_when_values_missing() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
fs::create_dir_all(&skill_dir).expect("skill dir");
|
||||
|
||||
let profile =
|
||||
compile_permission_profile(Some(PermissionProfile::default())).expect("profile");
|
||||
|
||||
assert_eq!(
|
||||
profile.macos_seatbelt_profile_extensions,
|
||||
Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default())
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -10,7 +10,6 @@ use crate::sandboxing::ExecRequest;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
use crate::shell::ShellType;
|
||||
use crate::skills::SkillMetadata;
|
||||
use crate::skills::permissions::compile_permission_profile;
|
||||
use crate::tools::runtimes::ExecveSessionApproval;
|
||||
use crate::tools::runtimes::build_command_spec;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
@@ -332,18 +331,14 @@ impl CoreShellActionProvider {
|
||||
}
|
||||
|
||||
fn skill_escalation_execution(skill: &SkillMetadata) -> EscalationExecution {
|
||||
compile_permission_profile(skill.permission_profile.clone())
|
||||
.map(|permissions| {
|
||||
EscalationExecution::Permissions(EscalationPermissions::Permissions(
|
||||
EscalatedPermissions {
|
||||
sandbox_policy: permissions.sandbox_policy.get().clone(),
|
||||
macos_seatbelt_profile_extensions: permissions
|
||||
.macos_seatbelt_profile_extensions
|
||||
.clone(),
|
||||
},
|
||||
))
|
||||
})
|
||||
.unwrap_or(EscalationExecution::TurnDefault)
|
||||
let permission_profile = skill.permission_profile.clone().unwrap_or_default();
|
||||
if permission_profile.is_empty() {
|
||||
EscalationExecution::TurnDefault
|
||||
} else {
|
||||
EscalationExecution::Permissions(EscalationPermissions::PermissionProfile(
|
||||
permission_profile,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
async fn prompt(
|
||||
@@ -741,11 +736,22 @@ struct CoreShellCommandExecutor {
|
||||
justification: Option<String>,
|
||||
arg0: Option<String>,
|
||||
sandbox_policy_cwd: PathBuf,
|
||||
#[cfg_attr(not(target_os = "macos"), allow(dead_code))]
|
||||
macos_seatbelt_profile_extensions: Option<MacOsSeatbeltProfileExtensions>,
|
||||
codex_linux_sandbox_exe: Option<PathBuf>,
|
||||
use_linux_sandbox_bwrap: bool,
|
||||
}
|
||||
|
||||
struct PrepareSandboxedExecParams<'a> {
|
||||
command: Vec<String>,
|
||||
workdir: &'a AbsolutePathBuf,
|
||||
env: HashMap<String, String>,
|
||||
sandbox_policy: &'a SandboxPolicy,
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: Option<&'a MacOsSeatbeltProfileExtensions>,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
async fn run(
|
||||
@@ -816,33 +822,46 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
env,
|
||||
arg0: Some(first_arg.clone()),
|
||||
},
|
||||
EscalationExecution::TurnDefault => self.prepare_sandboxed_exec(
|
||||
command,
|
||||
workdir,
|
||||
env,
|
||||
&self.sandbox_policy,
|
||||
None,
|
||||
self.macos_seatbelt_profile_extensions.as_ref(),
|
||||
)?,
|
||||
EscalationExecution::Permissions(EscalationPermissions::PermissionProfile(
|
||||
permission_profile,
|
||||
)) => self.prepare_sandboxed_exec(
|
||||
command,
|
||||
workdir,
|
||||
env,
|
||||
&self.sandbox_policy,
|
||||
Some(permission_profile),
|
||||
None,
|
||||
)?,
|
||||
EscalationExecution::Permissions(EscalationPermissions::Permissions(permissions)) => {
|
||||
self.prepare_sandboxed_exec(
|
||||
EscalationExecution::TurnDefault => {
|
||||
self.prepare_sandboxed_exec(PrepareSandboxedExecParams {
|
||||
command,
|
||||
workdir,
|
||||
env,
|
||||
&permissions.sandbox_policy,
|
||||
None,
|
||||
permissions.macos_seatbelt_profile_extensions.as_ref(),
|
||||
)?
|
||||
sandbox_policy: &self.sandbox_policy,
|
||||
additional_permissions: None,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: self
|
||||
.macos_seatbelt_profile_extensions
|
||||
.as_ref(),
|
||||
})?
|
||||
}
|
||||
EscalationExecution::Permissions(EscalationPermissions::PermissionProfile(
|
||||
permission_profile,
|
||||
)) => {
|
||||
// Merge additive permissions into the existing turn/request sandbox policy.
|
||||
self.prepare_sandboxed_exec(PrepareSandboxedExecParams {
|
||||
command,
|
||||
workdir,
|
||||
env,
|
||||
sandbox_policy: &self.sandbox_policy,
|
||||
additional_permissions: Some(permission_profile),
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: None,
|
||||
})?
|
||||
}
|
||||
EscalationExecution::Permissions(EscalationPermissions::Permissions(permissions)) => {
|
||||
// Use a fully specified sandbox policy instead of merging into the turn policy.
|
||||
self.prepare_sandboxed_exec(PrepareSandboxedExecParams {
|
||||
command,
|
||||
workdir,
|
||||
env,
|
||||
sandbox_policy: &permissions.sandbox_policy,
|
||||
additional_permissions: None,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions: permissions
|
||||
.macos_seatbelt_profile_extensions
|
||||
.as_ref(),
|
||||
})?
|
||||
}
|
||||
};
|
||||
|
||||
@@ -853,18 +872,17 @@ impl ShellCommandExecutor for CoreShellCommandExecutor {
|
||||
impl CoreShellCommandExecutor {
|
||||
fn prepare_sandboxed_exec(
|
||||
&self,
|
||||
command: Vec<String>,
|
||||
workdir: &AbsolutePathBuf,
|
||||
env: HashMap<String, String>,
|
||||
sandbox_policy: &SandboxPolicy,
|
||||
additional_permissions: Option<PermissionProfile>,
|
||||
#[cfg(target_os = "macos")] macos_seatbelt_profile_extensions: Option<
|
||||
&MacOsSeatbeltProfileExtensions,
|
||||
>,
|
||||
#[cfg(not(target_os = "macos"))] _macos_seatbelt_profile_extensions: Option<
|
||||
&MacOsSeatbeltProfileExtensions,
|
||||
>,
|
||||
params: PrepareSandboxedExecParams<'_>,
|
||||
) -> anyhow::Result<PreparedExec> {
|
||||
let PrepareSandboxedExecParams {
|
||||
command,
|
||||
workdir,
|
||||
env,
|
||||
sandbox_policy,
|
||||
additional_permissions,
|
||||
#[cfg(target_os = "macos")]
|
||||
macos_seatbelt_profile_extensions,
|
||||
} = params;
|
||||
let (program, args) = command
|
||||
.split_first()
|
||||
.ok_or_else(|| anyhow::anyhow!("prepared command must not be empty"))?;
|
||||
|
||||
@@ -20,6 +20,7 @@ use crate::protocol::SandboxPolicy;
|
||||
use crate::sandboxing::SandboxPermissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE;
|
||||
use crate::skills::SkillMetadata;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
@@ -30,6 +31,7 @@ use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::MacOsPreferencesPermission;
|
||||
use codex_protocol::models::MacOsSeatbeltProfileExtensions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_shell_escalation::EscalationExecution;
|
||||
use codex_shell_escalation::EscalationPermissions;
|
||||
use codex_shell_escalation::ExecResult;
|
||||
@@ -59,6 +61,20 @@ fn starlark_string(value: &str) -> String {
|
||||
value.replace('\\', "\\\\").replace('"', "\\\"")
|
||||
}
|
||||
|
||||
fn test_skill_metadata(permission_profile: Option<PermissionProfile>) -> SkillMetadata {
|
||||
SkillMetadata {
|
||||
name: "skill".to_string(),
|
||||
description: "description".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile,
|
||||
path_to_skills_md: PathBuf::from("/tmp/skill/SKILL.md"),
|
||||
scope: SkillScope::User,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_shell_script_preserves_login_flag() {
|
||||
assert_eq!(
|
||||
@@ -246,6 +262,42 @@ fn shell_request_escalation_execution_is_explicit() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skill_escalation_execution_uses_additional_permissions() {
|
||||
let requested_permissions = PermissionProfile {
|
||||
file_system: Some(FileSystemPermissions {
|
||||
read: None,
|
||||
write: Some(vec![
|
||||
AbsolutePathBuf::from_absolute_path("/tmp/output").unwrap(),
|
||||
]),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
CoreShellActionProvider::skill_escalation_execution(&test_skill_metadata(Some(
|
||||
requested_permissions.clone(),
|
||||
))),
|
||||
EscalationExecution::Permissions(EscalationPermissions::PermissionProfile(
|
||||
requested_permissions,
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skill_escalation_execution_ignores_empty_permissions() {
|
||||
assert_eq!(
|
||||
CoreShellActionProvider::skill_escalation_execution(&test_skill_metadata(Some(
|
||||
PermissionProfile::default(),
|
||||
))),
|
||||
EscalationExecution::TurnDefault,
|
||||
);
|
||||
assert_eq!(
|
||||
CoreShellActionProvider::skill_escalation_execution(&test_skill_metadata(None)),
|
||||
EscalationExecution::TurnDefault,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluate_intercepted_exec_policy_uses_wrapper_command_when_shell_wrapper_parsing_disabled() {
|
||||
let policy_src = r#"prefix_rule(pattern = ["npm", "publish"], decision = "prompt")"#;
|
||||
|
||||
Reference in New Issue
Block a user