mirror of
https://github.com/openai/codex.git
synced 2026-09-14 11:57:03 +00:00
Merge 4b7d0681bd into sapling-pr-archive-bolinfest
This commit is contained in:
@@ -1940,14 +1940,14 @@ impl Config {
|
||||
let (mut file_system_sandbox_policy, network_sandbox_policy) =
|
||||
compile_permission_profile(permissions, default_permissions)?;
|
||||
let mut sandbox_policy = file_system_sandbox_policy
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd);
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?;
|
||||
if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) {
|
||||
add_additional_file_system_writes(
|
||||
&mut file_system_sandbox_policy,
|
||||
&additional_writable_roots,
|
||||
);
|
||||
sandbox_policy = file_system_sandbox_policy
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd);
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?;
|
||||
}
|
||||
(
|
||||
sandbox_policy,
|
||||
@@ -3150,6 +3150,97 @@ default_permissions = "workspace"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_profiles_reject_writes_outside_workspace_root() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?;
|
||||
let external_write_path = if cfg!(windows) { r"C:\temp" } else { "/tmp" };
|
||||
|
||||
let err = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml {
|
||||
default_permissions: Some("workspace".to_string()),
|
||||
permissions: Some(PermissionsToml {
|
||||
entries: BTreeMap::from([(
|
||||
"workspace".to_string(),
|
||||
PermissionProfileToml {
|
||||
extends: None,
|
||||
filesystem: Some(FilesystemPermissionsToml {
|
||||
entries: BTreeMap::from([(
|
||||
external_write_path.to_string(),
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::Write),
|
||||
)]),
|
||||
}),
|
||||
network: None,
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
)
|
||||
.expect_err("writes outside the workspace root should be rejected");
|
||||
|
||||
assert_eq!(err.kind(), ErrorKind::InvalidInput);
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("filesystem writes outside the workspace root"),
|
||||
"{err}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permissions_profiles_reject_unsupported_network_fields() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let cwd = TempDir::new()?;
|
||||
std::fs::write(cwd.path().join(".git"), "gitdir: nowhere")?;
|
||||
|
||||
let err = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml {
|
||||
default_permissions: Some("workspace".to_string()),
|
||||
permissions: Some(PermissionsToml {
|
||||
entries: BTreeMap::from([(
|
||||
"workspace".to_string(),
|
||||
PermissionProfileToml {
|
||||
extends: None,
|
||||
filesystem: Some(FilesystemPermissionsToml {
|
||||
entries: BTreeMap::from([(
|
||||
":minimal".to_string(),
|
||||
FilesystemPermissionToml::Access(FileSystemAccessMode::Read),
|
||||
)]),
|
||||
}),
|
||||
network: Some(NetworkToml {
|
||||
enabled: Some(true),
|
||||
proxy_url: Some("http://127.0.0.1:43128".to_string()),
|
||||
..Default::default()
|
||||
}),
|
||||
},
|
||||
)]),
|
||||
}),
|
||||
..Default::default()
|
||||
},
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd.path().to_path_buf()),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.path().to_path_buf(),
|
||||
)
|
||||
.expect_err("unsupported profile network fields should be rejected");
|
||||
|
||||
assert_eq!(err.kind(), ErrorKind::InvalidInput);
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("uses unsupported `[permissions.workspace.network]` fields"),
|
||||
"{err}"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tui_theme_deserializes_from_toml() {
|
||||
let cfg = r#"
|
||||
|
||||
@@ -143,6 +143,23 @@ impl NetworkToml {
|
||||
self.apply_to_network_proxy_config(&mut config);
|
||||
config
|
||||
}
|
||||
|
||||
fn profile_has_unsupported_fields(&self) -> bool {
|
||||
self.proxy_url.is_some()
|
||||
|| self.admin_url.is_some()
|
||||
|| self.enable_socks5.is_some()
|
||||
|| self.socks_url.is_some()
|
||||
|| self.enable_socks5_udp.is_some()
|
||||
|| self.allow_upstream_proxy.is_some()
|
||||
|| self.dangerously_allow_non_loopback_proxy.is_some()
|
||||
|| self.dangerously_allow_non_loopback_admin.is_some()
|
||||
|| self.dangerously_allow_all_unix_sockets.is_some()
|
||||
|| self.mode.is_some()
|
||||
|| self.allowed_domains.is_some()
|
||||
|| self.denied_domains.is_some()
|
||||
|| self.allow_unix_sockets.is_some()
|
||||
|| self.allow_local_binding.is_some()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn network_proxy_config_from_network(
|
||||
@@ -197,15 +214,38 @@ pub(crate) fn compile_permission_profile(
|
||||
compile_filesystem_permission(path, permission, &mut entries)?;
|
||||
}
|
||||
|
||||
let network_sandbox_policy =
|
||||
compile_network_sandbox_policy(profile.network.as_ref(), profile_name)?;
|
||||
|
||||
Ok((
|
||||
FileSystemSandboxPolicy::restricted(entries),
|
||||
match profile.network.as_ref().and_then(|network| network.enabled) {
|
||||
Some(true) => NetworkSandboxPolicy::Enabled,
|
||||
_ => NetworkSandboxPolicy::Restricted,
|
||||
},
|
||||
network_sandbox_policy,
|
||||
))
|
||||
}
|
||||
|
||||
fn compile_network_sandbox_policy(
|
||||
network: Option<&NetworkToml>,
|
||||
profile_name: &str,
|
||||
) -> io::Result<NetworkSandboxPolicy> {
|
||||
let Some(network) = network else {
|
||||
return Ok(NetworkSandboxPolicy::Restricted);
|
||||
};
|
||||
|
||||
if network.profile_has_unsupported_fields() {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
format!(
|
||||
"permissions profile `{profile_name}` uses unsupported `[permissions.{profile_name}.network]` fields; only `enabled` is supported for now"
|
||||
),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(match network.enabled {
|
||||
Some(true) => NetworkSandboxPolicy::Enabled,
|
||||
_ => NetworkSandboxPolicy::Restricted,
|
||||
})
|
||||
}
|
||||
|
||||
fn compile_filesystem_permission(
|
||||
path: &str,
|
||||
permission: &FilesystemPermissionToml,
|
||||
|
||||
@@ -7,6 +7,7 @@ use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::ffi::OsStr;
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::str::FromStr;
|
||||
@@ -669,8 +670,8 @@ impl FileSystemSandboxPolicy {
|
||||
&self,
|
||||
network_policy: NetworkSandboxPolicy,
|
||||
cwd: &Path,
|
||||
) -> SandboxPolicy {
|
||||
match self.kind {
|
||||
) -> io::Result<SandboxPolicy> {
|
||||
Ok(match self.kind {
|
||||
FileSystemSandboxKind::ExternalSandbox => SandboxPolicy::ExternalSandbox {
|
||||
network_access: if network_policy.is_enabled() {
|
||||
NetworkAccess::Enabled
|
||||
@@ -768,13 +769,13 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
|
||||
if has_full_disk_write_access {
|
||||
return if network_policy.is_enabled() {
|
||||
return Ok(if network_policy.is_enabled() {
|
||||
SandboxPolicy::DangerFullAccess
|
||||
} else {
|
||||
SandboxPolicy::ExternalSandbox {
|
||||
network_access: NetworkAccess::Restricted,
|
||||
}
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
let read_only_access = if has_full_disk_read_access {
|
||||
@@ -786,11 +787,7 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
};
|
||||
|
||||
if workspace_root_writable
|
||||
|| !writable_roots.is_empty()
|
||||
|| tmpdir_writable
|
||||
|| slash_tmp_writable
|
||||
{
|
||||
if workspace_root_writable {
|
||||
SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: dedup_absolute_paths(writable_roots),
|
||||
read_only_access,
|
||||
@@ -798,6 +795,11 @@ impl FileSystemSandboxPolicy {
|
||||
exclude_tmpdir_env_var: !tmpdir_writable,
|
||||
exclude_slash_tmp: !slash_tmp_writable,
|
||||
}
|
||||
} else if !writable_roots.is_empty() || tmpdir_writable || slash_tmp_writable {
|
||||
return Err(io::Error::new(
|
||||
io::ErrorKind::InvalidInput,
|
||||
"permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly",
|
||||
));
|
||||
} else {
|
||||
SandboxPolicy::ReadOnly {
|
||||
access: read_only_access,
|
||||
@@ -805,7 +807,7 @@ impl FileSystemSandboxPolicy {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3679,6 +3681,36 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_system_policy_rejects_legacy_bridge_for_non_workspace_writes() {
|
||||
let cwd = if cfg!(windows) {
|
||||
Path::new(r"C:\workspace")
|
||||
} else {
|
||||
Path::new("/tmp/workspace")
|
||||
};
|
||||
let external_write_path = if cfg!(windows) {
|
||||
AbsolutePathBuf::from_absolute_path(r"C:\temp").expect("absolute windows temp path")
|
||||
} else {
|
||||
AbsolutePathBuf::from_absolute_path("/tmp").expect("absolute tmp path")
|
||||
};
|
||||
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: external_write_path,
|
||||
},
|
||||
access: FileSystemAccessMode::Write,
|
||||
}]);
|
||||
|
||||
let err = policy
|
||||
.to_legacy_sandbox_policy(NetworkSandboxPolicy::Restricted, cwd)
|
||||
.expect_err("non-workspace writes should be rejected");
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("filesystem writes outside the workspace root"),
|
||||
"{err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn item_started_event_from_web_search_emits_begin_event() {
|
||||
let event = ItemStartedEvent {
|
||||
|
||||
Reference in New Issue
Block a user