permissions: carry request paths as URIs

This commit is contained in:
Adam Perry
2026-07-10 03:34:27 +00:00
parent 21e1986b68
commit f6801b5a15
20 changed files with 883 additions and 499 deletions
@@ -7,17 +7,19 @@ use codex_protocol::approvals::NetworkPolicyRuleAction as CoreNetworkPolicyRuleA
use codex_protocol::models::ActivePermissionProfile as CoreActivePermissionProfile;
use codex_protocol::models::AdditionalPermissionProfile as CoreAdditionalPermissionProfile;
use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions;
use codex_protocol::models::ManagedFileSystemPermissions as CoreManagedFileSystemPermissions;
use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions;
use codex_protocol::models::PermissionProfile as CorePermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode as CoreFileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath as CoreFileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry as CoreFileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSpecialPath as CoreFileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy as CoreNetworkSandboxPolicy;
use codex_protocol::protocol::NetworkAccess as CoreNetworkAccess;
use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathConvention;
use codex_utils_path_uri::PathUri;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
@@ -68,9 +70,8 @@ pub struct AdditionalFileSystemPermissions {
pub entries: Option<Vec<FileSystemSandboxEntry>>,
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl From<CoreFileSystemPermissions<AbsolutePathBuf>> for AdditionalFileSystemPermissions {
fn from(value: CoreFileSystemPermissions<AbsolutePathBuf>) -> Self {
impl From<CoreFileSystemPermissions<PathUri>> for AdditionalFileSystemPermissions {
fn from(value: CoreFileSystemPermissions<PathUri>) -> Self {
if let Some((read, write)) = value.legacy_read_write_roots() {
let mut entries = Vec::with_capacity(
read.as_ref().map_or(0, Vec::len) + write.as_ref().map_or(0, Vec::len),
@@ -78,7 +79,7 @@ impl From<CoreFileSystemPermissions<AbsolutePathBuf>> for AdditionalFileSystemPe
if let Some(paths) = read.as_ref() {
entries.extend(paths.iter().map(|path| FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: LegacyAppPathString::from_abs_path(path),
path: path.clone().into(),
},
access: FileSystemAccessMode::Read,
}));
@@ -86,7 +87,7 @@ impl From<CoreFileSystemPermissions<AbsolutePathBuf>> for AdditionalFileSystemPe
if let Some(paths) = write.as_ref() {
entries.extend(paths.iter().map(|path| FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: LegacyAppPathString::from_abs_path(path),
path: path.clone().into(),
},
access: FileSystemAccessMode::Write,
}));
@@ -95,13 +96,15 @@ impl From<CoreFileSystemPermissions<AbsolutePathBuf>> for AdditionalFileSystemPe
read: read.map(|paths| {
paths
.iter()
.map(LegacyAppPathString::from_abs_path)
.cloned()
.map(LegacyAppPathString::from)
.collect()
}),
write: write.map(|paths| {
paths
.iter()
.map(LegacyAppPathString::from_abs_path)
.cloned()
.map(LegacyAppPathString::from)
.collect()
}),
glob_scan_max_depth: None,
@@ -124,8 +127,7 @@ impl From<CoreFileSystemPermissions<AbsolutePathBuf>> for AdditionalFileSystemPe
}
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl TryFrom<AdditionalFileSystemPermissions> for CoreFileSystemPermissions<AbsolutePathBuf> {
impl TryFrom<AdditionalFileSystemPermissions> for CoreFileSystemPermissions<PathUri> {
type Error = io::Error;
fn try_from(value: AdditionalFileSystemPermissions) -> Result<Self, Self::Error> {
@@ -133,7 +135,7 @@ impl TryFrom<AdditionalFileSystemPermissions> for CoreFileSystemPermissions<Abso
Self {
entries: entries
.into_iter()
.map(CoreFileSystemSandboxEntry::<AbsolutePathBuf>::try_from)
.map(CoreFileSystemSandboxEntry::<PathUri>::try_from)
.collect::<io::Result<_>>()?,
glob_scan_max_depth: None,
}
@@ -144,9 +146,8 @@ impl TryFrom<AdditionalFileSystemPermissions> for CoreFileSystemPermissions<Abso
paths
.into_iter()
.map(|path| {
path.to_path_uri(PathConvention::native())
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?
.to_abs_path()
PathUri::try_from(path)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))
})
.collect::<io::Result<Vec<_>>>()
})
@@ -157,9 +158,8 @@ impl TryFrom<AdditionalFileSystemPermissions> for CoreFileSystemPermissions<Abso
paths
.into_iter()
.map(|path| {
path.to_path_uri(PathConvention::native())
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?
.to_abs_path()
PathUri::try_from(path)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))
})
.collect::<io::Result<Vec<_>>>()
})
@@ -194,7 +194,7 @@ impl From<AdditionalNetworkPermissions> for CoreNetworkPermissions {
}
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[serde(deny_unknown_fields)]
#[ts(export_to = "v2/")]
@@ -203,9 +203,8 @@ pub struct RequestPermissionProfile {
pub file_system: Option<AdditionalFileSystemPermissions>,
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl From<CoreRequestPermissionProfile> for RequestPermissionProfile {
fn from(value: CoreRequestPermissionProfile) -> Self {
impl From<CoreRequestPermissionProfile<PathUri>> for RequestPermissionProfile {
fn from(value: CoreRequestPermissionProfile<PathUri>) -> Self {
Self {
network: value.network.map(AdditionalNetworkPermissions::from),
file_system: value.file_system.map(AdditionalFileSystemPermissions::from),
@@ -213,7 +212,7 @@ impl From<CoreRequestPermissionProfile> for RequestPermissionProfile {
}
}
impl TryFrom<RequestPermissionProfile> for CoreRequestPermissionProfile {
impl TryFrom<RequestPermissionProfile> for CoreRequestPermissionProfile<PathUri> {
type Error = io::Error;
fn try_from(value: RequestPermissionProfile) -> Result<Self, Self::Error> {
@@ -221,7 +220,7 @@ impl TryFrom<RequestPermissionProfile> for CoreRequestPermissionProfile {
network: value.network.map(CoreNetworkPermissions::from),
file_system: value
.file_system
.map(CoreFileSystemPermissions::<AbsolutePathBuf>::try_from)
.map(CoreFileSystemPermissions::<PathUri>::try_from)
.transpose()?,
})
}
@@ -307,13 +306,10 @@ pub enum FileSystemPath {
Special { value: FileSystemSpecialPath },
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl From<CoreFileSystemPath<AbsolutePathBuf>> for FileSystemPath {
fn from(value: CoreFileSystemPath<AbsolutePathBuf>) -> Self {
impl From<CoreFileSystemPath<PathUri>> for FileSystemPath {
fn from(value: CoreFileSystemPath<PathUri>) -> Self {
match value {
CoreFileSystemPath::Path { path } => Self::Path {
path: LegacyAppPathString::from_abs_path(&path),
},
CoreFileSystemPath::Path { path } => Self::Path { path: path.into() },
CoreFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern },
CoreFileSystemPath::Special { value } => Self::Special {
value: value.into(),
@@ -322,17 +318,14 @@ impl From<CoreFileSystemPath<AbsolutePathBuf>> for FileSystemPath {
}
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl TryFrom<FileSystemPath> for CoreFileSystemPath<AbsolutePathBuf> {
impl TryFrom<FileSystemPath> for CoreFileSystemPath<PathUri> {
type Error = io::Error;
fn try_from(value: FileSystemPath) -> Result<Self, Self::Error> {
Ok(match value {
FileSystemPath::Path { path } => Self::Path {
path: path
.to_path_uri(PathConvention::native())
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?
.to_abs_path()?,
path: PathUri::try_from(path)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?,
},
FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern },
FileSystemPath::Special { value } => Self::Special {
@@ -350,9 +343,8 @@ pub struct FileSystemSandboxEntry {
pub access: FileSystemAccessMode,
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl From<CoreFileSystemSandboxEntry<AbsolutePathBuf>> for FileSystemSandboxEntry {
fn from(value: CoreFileSystemSandboxEntry<AbsolutePathBuf>) -> Self {
impl From<CoreFileSystemSandboxEntry<PathUri>> for FileSystemSandboxEntry {
fn from(value: CoreFileSystemSandboxEntry<PathUri>) -> Self {
Self {
path: value.path.into(),
access: value.access.into(),
@@ -360,7 +352,7 @@ impl From<CoreFileSystemSandboxEntry<AbsolutePathBuf>> for FileSystemSandboxEntr
}
}
impl TryFrom<FileSystemSandboxEntry> for CoreFileSystemSandboxEntry<AbsolutePathBuf> {
impl TryFrom<FileSystemSandboxEntry> for CoreFileSystemSandboxEntry<PathUri> {
type Error = io::Error;
fn try_from(value: FileSystemSandboxEntry) -> Result<Self, Self::Error> {
@@ -383,7 +375,7 @@ pub struct PermissionProfileListParams {
pub limit: Option<u32>,
/// Optional working directory to resolve project config layers.
#[ts(optional = nullable)]
pub cwd: Option<String>,
pub cwd: Option<LegacyAppPathString>,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
@@ -461,9 +453,8 @@ pub struct AdditionalPermissionProfile {
pub file_system: Option<AdditionalFileSystemPermissions>,
}
// TODO(anp): Remove this conversion once core permission paths use PathUri.
impl From<CoreAdditionalPermissionProfile> for AdditionalPermissionProfile {
fn from(value: CoreAdditionalPermissionProfile) -> Self {
impl From<CoreAdditionalPermissionProfile<PathUri>> for AdditionalPermissionProfile {
fn from(value: CoreAdditionalPermissionProfile<PathUri>) -> Self {
Self {
network: value.network.map(AdditionalNetworkPermissions::from),
file_system: value.file_system.map(AdditionalFileSystemPermissions::from),
@@ -471,7 +462,7 @@ impl From<CoreAdditionalPermissionProfile> for AdditionalPermissionProfile {
}
}
impl TryFrom<AdditionalPermissionProfile> for CoreAdditionalPermissionProfile {
impl TryFrom<AdditionalPermissionProfile> for CoreAdditionalPermissionProfile<PathUri> {
type Error = io::Error;
fn try_from(value: AdditionalPermissionProfile) -> Result<Self, Self::Error> {
@@ -479,7 +470,7 @@ impl TryFrom<AdditionalPermissionProfile> for CoreAdditionalPermissionProfile {
network: value.network.map(CoreNetworkPermissions::from),
file_system: value
.file_system
.map(CoreFileSystemPermissions::<AbsolutePathBuf>::try_from)
.map(CoreFileSystemPermissions::<PathUri>::try_from)
.transpose()?,
})
}
@@ -497,7 +488,7 @@ pub struct GrantedPermissionProfile {
pub file_system: Option<AdditionalFileSystemPermissions>,
}
impl TryFrom<GrantedPermissionProfile> for CoreAdditionalPermissionProfile {
impl TryFrom<GrantedPermissionProfile> for CoreAdditionalPermissionProfile<PathUri> {
type Error = io::Error;
fn try_from(value: GrantedPermissionProfile) -> Result<Self, Self::Error> {
@@ -505,12 +496,18 @@ impl TryFrom<GrantedPermissionProfile> for CoreAdditionalPermissionProfile {
network: value.network.map(CoreNetworkPermissions::from),
file_system: value
.file_system
.map(CoreFileSystemPermissions::<AbsolutePathBuf>::try_from)
.map(CoreFileSystemPermissions::<PathUri>::try_from)
.transpose()?,
})
}
}
impl GrantedPermissionProfile {
pub fn is_empty(&self) -> bool {
self.network.is_none() && self.file_system.is_none()
}
}
#[derive(Serialize, Deserialize, Debug, Default, Clone, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
@@ -542,7 +539,7 @@ pub enum SandboxPolicy {
#[ts(rename_all = "camelCase")]
WorkspaceWrite {
#[serde(default)]
writable_roots: Vec<AbsolutePathBuf>,
writable_roots: Vec<LegacyAppPathString>,
#[serde(default)]
network_access: bool,
#[serde(default)]
@@ -571,7 +568,7 @@ enum SandboxPolicyDeserialize {
#[serde(rename_all = "camelCase")]
WorkspaceWrite {
#[serde(default)]
writable_roots: Vec<AbsolutePathBuf>,
writable_roots: Vec<LegacyAppPathString>,
#[serde(default)]
read_only_access: Option<LegacyReadOnlyAccess>,
#[serde(default)]
@@ -635,8 +632,103 @@ impl<'de> Deserialize<'de> for SandboxPolicy {
}
impl SandboxPolicy {
pub fn to_core(&self) -> codex_protocol::protocol::SandboxPolicy {
match self {
pub fn try_to_path_uri_permission_profile(&self) -> io::Result<CorePermissionProfile<PathUri>> {
let network = match self {
SandboxPolicy::DangerFullAccess => CoreNetworkSandboxPolicy::Enabled,
SandboxPolicy::ReadOnly { network_access }
| SandboxPolicy::WorkspaceWrite { network_access, .. } => {
if *network_access {
CoreNetworkSandboxPolicy::Enabled
} else {
CoreNetworkSandboxPolicy::Restricted
}
}
SandboxPolicy::ExternalSandbox { network_access } => match network_access {
NetworkAccess::Restricted => CoreNetworkSandboxPolicy::Restricted,
NetworkAccess::Enabled => CoreNetworkSandboxPolicy::Enabled,
},
};
Ok(match self {
SandboxPolicy::DangerFullAccess => CorePermissionProfile::Disabled,
SandboxPolicy::ExternalSandbox { .. } => CorePermissionProfile::External { network },
SandboxPolicy::ReadOnly { .. } => CorePermissionProfile::Managed {
file_system: CoreManagedFileSystemPermissions::Restricted {
entries: vec![CoreFileSystemSandboxEntry {
path: CoreFileSystemPath::Special {
value: CoreFileSystemSpecialPath::Root,
},
access: CoreFileSystemAccessMode::Read,
}],
glob_scan_max_depth: None,
},
network,
},
SandboxPolicy::WorkspaceWrite {
writable_roots,
exclude_tmpdir_env_var,
exclude_slash_tmp,
..
} => {
let mut entries = vec![
CoreFileSystemSandboxEntry {
path: CoreFileSystemPath::Special {
value: CoreFileSystemSpecialPath::Root,
},
access: CoreFileSystemAccessMode::Read,
},
CoreFileSystemSandboxEntry {
path: CoreFileSystemPath::Special {
value: CoreFileSystemSpecialPath::ProjectRoots { subpath: None },
},
access: CoreFileSystemAccessMode::Write,
},
];
if !exclude_slash_tmp {
entries.push(CoreFileSystemSandboxEntry {
path: CoreFileSystemPath::Special {
value: CoreFileSystemSpecialPath::SlashTmp,
},
access: CoreFileSystemAccessMode::Write,
});
}
if !exclude_tmpdir_env_var {
entries.push(CoreFileSystemSandboxEntry {
path: CoreFileSystemPath::Special {
value: CoreFileSystemSpecialPath::Tmpdir,
},
access: CoreFileSystemAccessMode::Write,
});
}
entries.extend(
writable_roots
.iter()
.cloned()
.map(|root| {
Ok(CoreFileSystemSandboxEntry {
path: CoreFileSystemPath::Path {
path: PathUri::try_from(root).map_err(|err| {
io::Error::new(io::ErrorKind::InvalidInput, err)
})?,
},
access: CoreFileSystemAccessMode::Write,
})
})
.collect::<io::Result<Vec<_>>>()?,
);
CorePermissionProfile::Managed {
file_system: CoreManagedFileSystemPermissions::Restricted {
entries,
glob_scan_max_depth: None,
},
network,
}
}
})
}
pub fn try_to_core(&self) -> io::Result<codex_protocol::protocol::SandboxPolicy> {
Ok(match self {
SandboxPolicy::DangerFullAccess => {
codex_protocol::protocol::SandboxPolicy::DangerFullAccess
}
@@ -659,12 +751,26 @@ impl SandboxPolicy {
exclude_tmpdir_env_var,
exclude_slash_tmp,
} => codex_protocol::protocol::SandboxPolicy::WorkspaceWrite {
writable_roots: writable_roots.clone(),
writable_roots: writable_roots
.iter()
.cloned()
.map(|root| {
if let Ok(root) = root.to_host_abs_path() {
return Ok(root);
}
let root_uri = PathUri::try_from(root)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?;
Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!("sandbox writable root path URI {root_uri} is not local"),
))
})
.collect::<io::Result<_>>()?,
network_access: *network_access,
exclude_tmpdir_env_var: *exclude_tmpdir_env_var,
exclude_slash_tmp: *exclude_slash_tmp,
},
}
})
}
}
@@ -691,7 +797,10 @@ impl From<codex_protocol::protocol::SandboxPolicy> for SandboxPolicy {
exclude_tmpdir_env_var,
exclude_slash_tmp,
} => SandboxPolicy::WorkspaceWrite {
writable_roots,
writable_roots: writable_roots
.iter()
.map(LegacyAppPathString::from_abs_path)
.collect(),
network_access,
exclude_tmpdir_env_var,
exclude_slash_tmp,
@@ -765,7 +874,7 @@ pub struct PermissionsRequestApprovalParams {
/// Unix timestamp (in milliseconds) when this approval request started.
#[ts(type = "number")]
pub started_at_ms: i64,
pub cwd: AbsolutePathBuf,
pub cwd: LegacyAppPathString,
pub reason: Option<String>,
pub permissions: RequestPermissionProfile,
}
@@ -109,10 +109,11 @@ use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequest
use codex_protocol::request_permissions::RequestPermissionsResponse as CoreRequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputAnswer as CoreRequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputResponse as CoreRequestUserInputResponse;
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
use codex_sandboxing::policy_transforms::intersect_permission_profiles_for_uri;
use codex_shell_command::parse_command::shlex_join;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathUri;
use std::collections::HashMap;
use std::sync::Arc;
use std::time::SystemTime;
@@ -539,7 +540,10 @@ pub(crate) async fn apply_bespoke_event_handling(
item_id: item_id.clone(),
started_at_ms: event.started_at_ms,
reason: event.reason.clone(),
grant_root: event.grant_root.clone(),
grant_root: event
.grant_root
.as_deref()
.map(LegacyAppPathString::from_path),
};
let (pending_request_id, rx) = outgoing
.send_request(ServerRequestPayload::FileChangeRequestApproval(params))
@@ -790,7 +794,7 @@ pub(crate) async fn apply_bespoke_event_handling(
let requested_permissions = request.permissions.clone();
let request_cwd = match request.cwd.clone() {
Some(cwd) => cwd,
None => conversation.config_snapshot().await.cwd().clone(),
None => PathUri::from_abs_path(conversation.config_snapshot().await.cwd()),
};
let params = PermissionsRequestApprovalParams {
thread_id: conversation_id.to_string(),
@@ -798,7 +802,7 @@ pub(crate) async fn apply_bespoke_event_handling(
item_id: request.call_id.clone(),
environment_id: request.environment_id.clone(),
started_at_ms: request.started_at_ms,
cwd: request_cwd.clone(),
cwd: request_cwd.clone().into(),
reason: request.reason,
permissions: request.permissions.into(),
};
@@ -1783,14 +1787,12 @@ async fn on_request_permissions_response(
let response = match request_permissions_response_from_client_result(
requested_permissions,
response,
request_cwd.as_path(),
&request_cwd,
) {
Ok(Some(response)) => response,
Ok(None) => return,
// TODO(anp): Remove this native-path localization error path once core permission paths
// remain PathUri after crossing the app-server boundary.
Err(err) => {
let message = format!("failed to localize granted filesystem paths: {err}");
let message = format!("failed to parse granted filesystem paths: {err}");
handle_error_notification(
conversation_id,
&turn_id,
@@ -1826,8 +1828,8 @@ struct PendingRequestPermissionsResponse {
call_id: String,
conversation_id: ThreadId,
turn_id: String,
requested_permissions: CoreRequestPermissionProfile,
request_cwd: AbsolutePathBuf,
requested_permissions: CoreRequestPermissionProfile<PathUri>,
request_cwd: PathUri,
pending_request_id: RequestId,
outgoing: ThreadScopedOutgoingMessageSender,
receiver: oneshot::Receiver<ClientRequestResult>,
@@ -1835,10 +1837,10 @@ struct PendingRequestPermissionsResponse {
}
fn request_permissions_response_from_client_result(
requested_permissions: CoreRequestPermissionProfile,
requested_permissions: CoreRequestPermissionProfile<PathUri>,
response: std::result::Result<ClientRequestResult, oneshot::error::RecvError>,
cwd: &std::path::Path,
) -> std::io::Result<Option<CoreRequestPermissionsResponse>> {
cwd: &PathUri,
) -> std::io::Result<Option<CoreRequestPermissionsResponse<PathUri>>> {
let value = match response {
Ok(Ok(value)) => value,
Ok(Err(err)) if is_turn_transition_server_request_error(&err) => return Ok(None),
@@ -1883,11 +1885,17 @@ fn request_permissions_response_from_client_result(
strict_auto_review: false,
}));
}
let granted_permissions: CoreAdditionalPermissionProfile = response.permissions.try_into()?;
let granted_permissions: CoreAdditionalPermissionProfile<PathUri> =
response.permissions.try_into()?;
let permissions = if granted_permissions.is_empty() {
CoreRequestPermissionProfile::default()
} else {
intersect_permission_profiles(requested_permissions.into(), granted_permissions, cwd).into()
intersect_permission_profiles_for_uri(
requested_permissions.into(),
granted_permissions,
cwd,
)
.into()
};
Ok(Some(CoreRequestPermissionsResponse {
permissions,
@@ -2148,6 +2156,14 @@ mod tests {
Arc::new(Mutex::new(ThreadState::default()))
}
fn current_dir_uri() -> PathUri {
let cwd = AbsolutePathBuf::from_absolute_path(
std::env::current_dir().expect("current dir should be available"),
)
.expect("current dir should be absolute");
PathUri::from_abs_path(&cwd)
}
const TEST_TURN_COMPLETED_AT: i64 = 1_716_000_456;
const TEST_TURN_DURATION_MS: i64 = 1_234;
@@ -2350,7 +2366,7 @@ mod tests {
let action = codex_protocol::protocol::GuardianAssessmentAction::Command {
source: codex_protocol::protocol::GuardianCommandSource::Shell,
command: "rm -rf /tmp/example.sqlite".to_string(),
cwd: test_path_buf("/tmp").abs(),
cwd: test_path_buf("/tmp").abs().into(),
};
let notification = guardian_auto_approval_review_notification(
&conversation_id,
@@ -2396,7 +2412,7 @@ mod tests {
let action = codex_protocol::protocol::GuardianAssessmentAction::Command {
source: codex_protocol::protocol::GuardianCommandSource::Shell,
command: "rm -rf /tmp/example.sqlite".to_string(),
cwd: test_path_buf("/tmp").abs(),
cwd: test_path_buf("/tmp").abs().into(),
};
let notification = guardian_auto_approval_review_notification(
&conversation_id,
@@ -2882,7 +2898,7 @@ mod tests {
let response = request_permissions_response_from_client_result(
CoreRequestPermissionProfile::default(),
Ok(Err(error)),
std::env::current_dir().expect("current dir").as_path(),
&current_dir_uri(),
)
.expect("paths should localize");
@@ -2906,16 +2922,18 @@ mod tests {
} else {
"/tmp/ignored"
};
let absolute_path = |path: &str| {
AbsolutePathBuf::try_from(std::path::PathBuf::from(path)).expect("absolute path")
let path_uri = |path: &str| {
PathUri::from_abs_path(
&AbsolutePathBuf::try_from(std::path::PathBuf::from(path)).expect("absolute path"),
)
};
let requested_permissions = CoreRequestPermissionProfile {
network: Some(CoreNetworkPermissions {
enabled: Some(true),
}),
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(input_path)]),
Some(vec![absolute_path(output_path)]),
Some(vec![path_uri(input_path)]),
Some(vec![path_uri(output_path)]),
)),
};
let cases = vec![
@@ -2945,7 +2963,7 @@ mod tests {
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![absolute_path(output_path)]),
Some(vec![path_uri(output_path)]),
)),
..CoreRequestPermissionProfile::default()
},
@@ -2962,22 +2980,22 @@ mod tests {
}),
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(input_path)]),
Some(vec![absolute_path(output_path)]),
Some(vec![path_uri(input_path)]),
Some(vec![path_uri(output_path)]),
)),
..CoreRequestPermissionProfile::default()
},
),
];
let cwd = std::env::current_dir().expect("current dir");
let cwd = current_dir_uri();
for (granted_permissions, expected_permissions) in cases {
let response = request_permissions_response_from_client_result(
requested_permissions.clone(),
Ok(Ok(serde_json::json!({
"permissions": granted_permissions,
}))),
cwd.as_path(),
&cwd,
)
.expect("paths should localize")
.expect("response should be accepted");
@@ -3001,7 +3019,7 @@ mod tests {
"scope": "session",
"permissions": {},
}))),
std::env::current_dir().expect("current dir").as_path(),
&current_dir_uri(),
)
.expect("paths should localize")
.expect("response should be accepted");
@@ -3029,7 +3047,7 @@ mod tests {
},
},
}))),
std::env::current_dir().expect("current dir").as_path(),
&current_dir_uri(),
)
.expect("paths should localize")
.expect("response should be accepted");
@@ -3061,7 +3079,7 @@ mod tests {
},
},
}))),
std::env::current_dir().expect("current dir").as_path(),
&current_dir_uri(),
)
.expect("paths should localize")
.expect("response should be accepted");
@@ -3075,18 +3093,21 @@ mod tests {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute cwd");
let child = cwd.join("child");
let requested_permissions = CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let cwd_uri = PathUri::from_abs_path(&cwd);
let child_uri = PathUri::from_abs_path(&child);
let requested_permissions: CoreRequestPermissionProfile<PathUri> =
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let response = request_permissions_response_from_client_result(
requested_permissions,
@@ -3097,7 +3118,7 @@ mod tests {
},
},
}))),
cwd.as_path(),
&cwd_uri,
)
.expect("paths should localize")
.expect("response should be accepted");
@@ -3107,7 +3128,7 @@ mod tests {
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![child]),
Some(vec![child_uri]),
)),
..Default::default()
}
@@ -3122,18 +3143,20 @@ mod tests {
let later_cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("later-cwd"))
.expect("absolute later cwd");
let later_child = later_cwd.join("child");
let requested_permissions = CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let request_cwd_uri = PathUri::from_abs_path(&request_cwd);
let requested_permissions: CoreRequestPermissionProfile<PathUri> =
CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
let response = request_permissions_response_from_client_result(
requested_permissions,
@@ -3144,7 +3167,7 @@ mod tests {
},
},
}))),
request_cwd.as_path(),
&request_cwd_uri,
)
.expect("paths should localize")
.expect("response should be accepted");
@@ -3160,10 +3183,12 @@ mod tests {
let temp_dir = TempDir::new().expect("temp dir");
let cwd = AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute cwd");
let child = cwd.join("child");
let cwd_uri = PathUri::from_abs_path(&cwd);
let child_uri = PathUri::from_abs_path(&child);
let requested_permissions = CoreRequestPermissionProfile {
file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
/*read*/ None,
Some(vec![child]),
Some(vec![child_uri]),
)),
..Default::default()
};
@@ -3186,7 +3211,7 @@ mod tests {
},
},
}))),
cwd.as_path(),
&cwd_uri,
)
.expect("paths should localize")
.expect("response should be accepted");
+70 -33
View File
@@ -1,12 +1,10 @@
use std::path::Path;
use codex_analytics::GuardianReviewedAction;
use codex_protocol::approvals::GuardianAssessmentAction;
use codex_protocol::approvals::GuardianCommandSource;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use serde::Serialize;
use serde_json::Value;
@@ -18,17 +16,17 @@ pub(crate) enum GuardianApprovalRequest {
Shell {
id: String,
command: Vec<String>,
cwd: AbsolutePathBuf,
cwd: PathUri,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
justification: Option<String>,
},
ExecCommand {
id: String,
command: Vec<String>,
cwd: AbsolutePathBuf,
cwd: PathUri,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
justification: Option<String>,
tty: bool,
},
@@ -38,13 +36,13 @@ pub(crate) enum GuardianApprovalRequest {
source: GuardianCommandSource,
program: String,
argv: Vec<String>,
cwd: AbsolutePathBuf,
additional_permissions: Option<AdditionalPermissionProfile>,
cwd: PathUri,
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
},
ApplyPatch {
id: String,
cwd: AbsolutePathBuf,
files: Vec<AbsolutePathBuf>,
cwd: PathUri,
files: Vec<PathUri>,
patch: String,
},
NetworkAccess {
@@ -73,7 +71,7 @@ pub(crate) enum GuardianApprovalRequest {
id: String,
turn_id: String,
reason: Option<String>,
permissions: RequestPermissionProfile,
permissions: RequestPermissionProfile<PathUri>,
},
}
@@ -83,10 +81,14 @@ pub(crate) struct GuardianNetworkAccessTrigger {
pub(crate) call_id: String,
pub(crate) tool_name: String,
pub(crate) command: Vec<String>,
pub(crate) cwd: AbsolutePathBuf,
#[serde(serialize_with = "serialize_path_uri_as_native_path")]
pub(crate) cwd: PathUri,
pub(crate) sandbox_permissions: crate::sandboxing::SandboxPermissions,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) additional_permissions: Option<AdditionalPermissionProfile>,
#[serde(
skip_serializing_if = "Option::is_none",
serialize_with = "serialize_optional_additional_permissions_as_native_paths"
)]
pub(crate) additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) justification: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -107,10 +109,10 @@ pub(crate) struct GuardianMcpAnnotations {
struct CommandApprovalAction<'a> {
tool: &'a str,
command: &'a [String],
cwd: &'a Path,
cwd: String,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
#[serde(skip_serializing_if = "Option::is_none")]
additional_permissions: Option<&'a AdditionalPermissionProfile>,
additional_permissions: Option<AdditionalPermissionProfile<String>>,
#[serde(skip_serializing_if = "Option::is_none")]
justification: Option<&'a String>,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -123,9 +125,9 @@ struct ExecveApprovalAction<'a> {
tool: &'a str,
program: &'a str,
argv: &'a [String],
cwd: &'a Path,
cwd: String,
#[serde(skip_serializing_if = "Option::is_none")]
additional_permissions: Option<&'a AdditionalPermissionProfile>,
additional_permissions: Option<AdditionalPermissionProfile<String>>,
}
#[derive(Serialize)]
@@ -169,28 +171,50 @@ struct RequestPermissionsApprovalAction<'a> {
turn_id: &'a str,
#[serde(skip_serializing_if = "Option::is_none")]
reason: Option<&'a String>,
permissions: &'a RequestPermissionProfile,
permissions: RequestPermissionProfile<String>,
}
fn serialize_guardian_action(value: impl Serialize) -> serde_json::Result<Value> {
serde_json::to_value(value)
}
fn serialize_path_uri_as_native_path<S>(path: &PathUri, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(&path.inferred_native_path_string())
}
fn serialize_optional_additional_permissions_as_native_paths<S>(
permissions: &Option<AdditionalPermissionProfile<PathUri>>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
permissions
.clone()
.map(|permissions| permissions.map_paths(|path| path.inferred_native_path_string()))
.serialize(serializer)
}
fn serialize_command_guardian_action(
tool: &'static str,
command: &[String],
cwd: &Path,
cwd: &PathUri,
sandbox_permissions: crate::sandboxing::SandboxPermissions,
additional_permissions: Option<&AdditionalPermissionProfile>,
additional_permissions: Option<&AdditionalPermissionProfile<PathUri>>,
justification: Option<&String>,
tty: Option<bool>,
) -> serde_json::Result<Value> {
serialize_guardian_action(CommandApprovalAction {
tool,
command,
cwd,
cwd: cwd.inferred_native_path_string(),
sandbox_permissions,
additional_permissions,
additional_permissions: additional_permissions
.cloned()
.map(|permissions| permissions.map_paths(|path| path.inferred_native_path_string())),
justification,
tty,
})
@@ -199,7 +223,7 @@ fn serialize_command_guardian_action(
fn command_assessment_action(
source: GuardianCommandSource,
command: &[String],
cwd: &AbsolutePathBuf,
cwd: &PathUri,
) -> GuardianAssessmentAction {
GuardianAssessmentAction::Command {
source,
@@ -308,8 +332,10 @@ pub(crate) fn guardian_approval_request_to_json(
tool: guardian_command_source_tool_name(*source),
program,
argv,
cwd,
additional_permissions: additional_permissions.as_ref(),
cwd: cwd.inferred_native_path_string(),
additional_permissions: additional_permissions.clone().map(|permissions| {
permissions.map_paths(|path| path.inferred_native_path_string())
}),
}),
GuardianApprovalRequest::ApplyPatch {
id: _,
@@ -318,8 +344,11 @@ pub(crate) fn guardian_approval_request_to_json(
patch,
} => Ok(serde_json::json!({
"tool": "apply_patch",
"cwd": cwd,
"files": files,
"cwd": cwd.inferred_native_path_string(),
"files": files
.iter()
.map(PathUri::inferred_native_path_string)
.collect::<Vec<_>>(),
"patch": patch,
})),
GuardianApprovalRequest::NetworkAccess {
@@ -372,7 +401,9 @@ pub(crate) fn guardian_approval_request_to_json(
tool: "request_permissions",
turn_id,
reason: reason.as_ref(),
permissions,
permissions: permissions
.clone()
.map_paths(|path| path.inferred_native_path_string()),
}),
}
}
@@ -455,7 +486,9 @@ pub(crate) fn guardian_reviewed_action(
..
} => GuardianReviewedAction::Shell {
sandbox_permissions: *sandbox_permissions,
additional_permissions: additional_permissions.clone(),
additional_permissions: additional_permissions.clone().map(|permissions| {
permissions.map_paths(|path| path.inferred_native_path_string())
}),
},
GuardianApprovalRequest::ExecCommand {
sandbox_permissions,
@@ -464,7 +497,9 @@ pub(crate) fn guardian_reviewed_action(
..
} => GuardianReviewedAction::UnifiedExec {
sandbox_permissions: *sandbox_permissions,
additional_permissions: additional_permissions.clone(),
additional_permissions: additional_permissions.clone().map(|permissions| {
permissions.map_paths(|path| path.inferred_native_path_string())
}),
tty: *tty,
},
#[cfg(unix)]
@@ -476,7 +511,9 @@ pub(crate) fn guardian_reviewed_action(
} => GuardianReviewedAction::Execve {
source: *source,
program: program.clone(),
additional_permissions: additional_permissions.clone(),
additional_permissions: additional_permissions.clone().map(|permissions| {
permissions.map_paths(|path| path.inferred_native_path_string())
}),
},
GuardianApprovalRequest::ApplyPatch { .. } => GuardianReviewedAction::ApplyPatch {},
GuardianApprovalRequest::NetworkAccess { protocol, port, .. } => {
+26 -54
View File
@@ -3,7 +3,6 @@ use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt::Debug;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::AtomicU64;
@@ -142,7 +141,7 @@ use codex_rollout::state_db;
use codex_rollout_trace::AgentResultTracePayload;
use codex_rollout_trace::ThreadStartedTraceMetadata;
use codex_rollout_trace::ThreadTraceContext;
use codex_sandboxing::policy_transforms::intersect_permission_profiles;
use codex_sandboxing::policy_transforms::intersect_permission_profiles_for_uri;
use codex_shell_command::parse_command::parse_command;
use codex_terminal_detection::user_agent;
use codex_thread_store::CreateThreadParams;
@@ -2151,11 +2150,11 @@ impl Session {
approval_id: Option<String>,
environment_id: Option<String>,
command: Vec<String>,
cwd: AbsolutePathBuf,
cwd: PathUri,
reason: Option<String>,
network_approval_context: Option<NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
additional_permissions: Option<AdditionalPermissionProfile>,
additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
available_decisions: Option<Vec<ReviewDecision>>,
) -> ReviewDecision {
// command-level approvals use `call_id`.
@@ -2267,10 +2266,10 @@ impl Session {
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
args: RequestPermissionsArgs<PathUri>,
environment: TurnEnvironmentSelection,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
) -> Option<RequestPermissionsResponse<PathUri>> {
match turn_context.as_ref().approval_policy.value() {
AskForApproval::Never => {
return Some(RequestPermissionsResponse {
@@ -2294,18 +2293,6 @@ impl Session {
}
let requested_permissions = args.permissions;
// TODO(anp): Migrate request_permissions to support paths from foreign environments.
let Ok(native_environment_cwd) = environment.cwd.to_abs_path() else {
warn!(
cwd = %environment.cwd,
"request_permissions requires a cwd native to the Codex host"
);
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
if crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) {
let originating_turn_state = {
@@ -2373,7 +2360,7 @@ impl Session {
let response = Self::normalize_request_permissions_response(
requested_permissions,
response,
native_environment_cwd.as_path(),
&environment.cwd,
);
self.record_granted_request_permissions_for_turn(
&response,
@@ -2413,7 +2400,7 @@ impl Session {
started_at_ms: now_unix_timestamp_ms(),
reason: args.reason,
permissions: requested_permissions,
cwd: Some(native_environment_cwd),
cwd: Some(environment.cwd),
});
self.send_event(turn_context.as_ref(), event).await;
tokio::select! {
@@ -2434,10 +2421,10 @@ impl Session {
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
args: RequestPermissionsArgs<PathUri>,
cwd: PathUri,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
) -> Option<RequestPermissionsResponse<PathUri>> {
let turn_environment = match args.environment_id.as_deref() {
Some(environment_id) => turn_context
.environments
@@ -2454,7 +2441,7 @@ impl Session {
});
};
let mut environment = turn_environment.selection();
environment.cwd = PathUri::from_abs_path(&cwd);
environment.cwd = cwd;
self.request_permissions_for_environment(
turn_context,
call_id,
@@ -2541,7 +2528,7 @@ impl Session {
pub async fn notify_request_permissions_response(
&self,
call_id: &str,
response: RequestPermissionsResponse,
response: RequestPermissionsResponse<PathUri>,
) {
let (entry, originating_turn_state) = {
let mut active = self.active_turn.lock().await;
@@ -2557,26 +2544,11 @@ impl Session {
};
match entry {
Some(entry) => {
// TODO(anp): Migrate request_permissions to support paths from foreign environments.
let response = match entry.environment.cwd.to_abs_path() {
Ok(native_environment_cwd) => Self::normalize_request_permissions_response(
entry.requested_permissions,
response,
native_environment_cwd.as_path(),
),
Err(err) => {
warn!(
cwd = %entry.environment.cwd,
%err,
"request_permissions requires a cwd native to the Codex host"
);
RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
}
}
};
let response = Self::normalize_request_permissions_response(
entry.requested_permissions,
response,
&entry.environment.cwd,
);
self.record_granted_request_permissions_for_turn(
&response,
&entry.environment.environment_id,
@@ -2592,10 +2564,10 @@ impl Session {
}
fn normalize_request_permissions_response(
requested_permissions: RequestPermissionProfile,
response: RequestPermissionsResponse,
cwd: &Path,
) -> RequestPermissionsResponse {
requested_permissions: RequestPermissionProfile<PathUri>,
response: RequestPermissionsResponse<PathUri>,
cwd: &PathUri,
) -> RequestPermissionsResponse<PathUri> {
if response.strict_auto_review && matches!(response.scope, PermissionGrantScope::Session) {
return RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
@@ -2609,7 +2581,7 @@ impl Session {
}
RequestPermissionsResponse {
permissions: intersect_permission_profiles(
permissions: intersect_permission_profiles_for_uri(
requested_permissions.into(),
response.permissions.into(),
cwd,
@@ -2622,7 +2594,7 @@ impl Session {
async fn record_granted_request_permissions_for_turn(
&self,
response: &RequestPermissionsResponse,
response: &RequestPermissionsResponse<PathUri>,
environment_id: &str,
originating_turn_state: Option<&Arc<Mutex<crate::state::TurnState>>>,
) {
@@ -2633,7 +2605,7 @@ impl Session {
PermissionGrantScope::Turn => {
if let Some(turn_state) = originating_turn_state {
let mut ts = turn_state.lock().await;
let permissions: AdditionalPermissionProfile =
let permissions: AdditionalPermissionProfile<PathUri> =
response.permissions.clone().into();
ts.record_granted_permissions(environment_id, permissions);
if response.strict_auto_review {
@@ -2658,7 +2630,7 @@ impl Session {
pub(crate) async fn granted_turn_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
) -> Option<AdditionalPermissionProfile<PathUri>> {
let active = self.active_turn.lock().await;
let active = active.as_ref()?;
let ts = active.turn_state.lock().await;
@@ -2681,7 +2653,7 @@ impl Session {
pub(crate) async fn granted_session_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
) -> Option<AdditionalPermissionProfile<PathUri>> {
let state = self.state.lock().await;
state.granted_permissions(environment_id)
}
+6 -5
View File
@@ -15,6 +15,7 @@ use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::ElicitationResponse;
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use codex_utils_path_uri::PathUri;
use rmcp::model::RequestId;
use tokio::sync::oneshot;
@@ -92,7 +93,7 @@ pub(crate) struct TurnState {
pending_dynamic_tools: HashMap<String, oneshot::Sender<DynamicToolResponse>>,
pub(crate) pending_input: TurnInputQueue,
mailbox_delivery_phase: MailboxDeliveryPhase,
granted_permissions_by_environment_id: HashMap<String, AdditionalPermissionProfile>,
granted_permissions_by_environment_id: HashMap<String, AdditionalPermissionProfile<PathUri>>,
strict_auto_review_enabled: bool,
pub(crate) tool_calls: u64,
pub(crate) has_memory_citation: bool,
@@ -100,8 +101,8 @@ pub(crate) struct TurnState {
}
pub(crate) struct PendingRequestPermissions {
pub(crate) tx_response: oneshot::Sender<RequestPermissionsResponse>,
pub(crate) requested_permissions: RequestPermissionProfile,
pub(crate) tx_response: oneshot::Sender<RequestPermissionsResponse<PathUri>>,
pub(crate) requested_permissions: RequestPermissionProfile<PathUri>,
pub(crate) environment: TurnEnvironmentSelection,
}
@@ -209,7 +210,7 @@ impl TurnState {
pub(crate) fn record_granted_permissions(
&mut self,
environment_id: &str,
permissions: AdditionalPermissionProfile,
permissions: AdditionalPermissionProfile<PathUri>,
) {
let granted_permissions = merge_permission_profiles(
self.granted_permissions_by_environment_id
@@ -225,7 +226,7 @@ impl TurnState {
pub(crate) fn granted_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
) -> Option<AdditionalPermissionProfile<PathUri>> {
self.granted_permissions_by_environment_id
.get(environment_id)
.cloned()
@@ -1,5 +1,6 @@
use codex_protocol::request_permissions::RequestPermissionsArgs;
use codex_sandboxing::policy_transforms::normalize_additional_permissions;
use codex_sandboxing::policy_transforms::normalize_additional_permissions_for_uri;
use codex_sandboxing::policy_transforms::resolve_additional_permission_paths;
use crate::function_tool::FunctionCallError;
use crate::tools::context::FunctionToolOutput;
@@ -7,7 +8,6 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_tool_environment;
use crate::tools::handlers::shell_spec::create_request_permissions_tool;
use crate::tools::handlers::shell_spec::request_permissions_tool_description;
@@ -15,16 +15,9 @@ use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use serde::Deserialize;
pub struct RequestPermissionsHandler;
#[derive(Deserialize)]
struct RequestPermissionsEnvironmentArgs {
#[serde(default, rename = "environment_id", alias = "environmentId")]
environment_id: Option<String>,
}
impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
fn tool_name(&self) -> ToolName {
ToolName::plain("request_permissions")
@@ -63,34 +56,29 @@ impl RequestPermissionsHandler {
}
};
let environment_args: RequestPermissionsEnvironmentArgs = parse_arguments(&arguments)?;
let Some(turn_environment) = resolve_tool_environment(
&step_context.environments,
environment_args.environment_id.as_deref(),
)?
let args: RequestPermissionsArgs<String> = parse_arguments(&arguments)?;
let Some(turn_environment) =
resolve_tool_environment(&step_context.environments, args.environment_id.as_deref())?
else {
return Err(FunctionCallError::RespondToModel(
"request_permissions requires a primary environment".to_string(),
));
};
// TODO(anp): Migrate request_permissions parsing and permission profiles to PathUri so
// environment-native foreign paths do not require host conversion.
let native_cwd = turn_environment.cwd().to_abs_path().map_err(|err| {
FunctionCallError::RespondToModel(format!(
"request_permissions cwd `{}` is not native to the Codex host: {err}",
turn_environment.cwd()
))
})?;
let mut args: RequestPermissionsArgs =
parse_arguments_with_base_path(&arguments, &native_cwd)?;
args.permissions = normalize_additional_permissions(args.permissions.into())
.map(codex_protocol::request_permissions::RequestPermissionProfile::from)
.map_err(FunctionCallError::RespondToModel)?;
if args.permissions.is_empty() {
let permissions =
resolve_additional_permission_paths(args.permissions.into(), turn_environment.cwd())
.and_then(normalize_additional_permissions_for_uri)
.map(codex_protocol::request_permissions::RequestPermissionProfile::from)
.map_err(FunctionCallError::RespondToModel)?;
if permissions.is_empty() {
return Err(FunctionCallError::RespondToModel(
"request_permissions requires at least one permission".to_string(),
));
}
let args = RequestPermissionsArgs {
environment_id: args.environment_id,
reason: args.reason,
permissions,
};
let response = session
.request_permissions_for_environment(
@@ -107,6 +95,7 @@ impl RequestPermissionsHandler {
)
})?;
let response = response.map_paths(|path| path.inferred_native_path_string());
let content = serde_json::to_string(&response).map_err(|err| {
FunctionCallError::Fatal(format!(
"failed to serialize request_permissions response: {err}"
@@ -20,6 +20,7 @@ use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
@@ -259,7 +260,7 @@ async fn wait_for_exec_approval_or_completion(
async fn expect_request_permissions_event(
test: &TestCodex,
expected_call_id: &str,
) -> RequestPermissionProfile {
) -> RequestPermissionProfile<PathUri> {
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
@@ -297,11 +298,15 @@ fn requested_directory_write_permissions(path: &Path) -> RequestPermissionProfil
}
}
fn normalized_directory_write_permissions(path: &Path) -> Result<RequestPermissionProfile> {
fn normalized_directory_write_permissions(
path: &Path,
) -> Result<RequestPermissionProfile<PathUri>> {
Ok(RequestPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![]),
Some(vec![AbsolutePathBuf::try_from(path.canonicalize()?)?]),
Some(vec![PathUri::from_abs_path(&AbsolutePathBuf::try_from(
path.canonicalize()?,
)?)]),
)),
..RequestPermissionProfile::default()
})
@@ -372,7 +377,7 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res
let approval = expect_exec_approval(&test, command).await;
assert_eq!(
approval.additional_permissions,
Some(requested_permissions.clone())
Some(requested_permissions.clone().into())
);
test.codex
.submit(Op::ExecApproval {
@@ -594,7 +599,7 @@ async fn relative_additional_permissions_resolve_against_tool_workdir(
let approval = expect_exec_approval(&test, command).await;
assert_eq!(
approval.additional_permissions,
Some(expected_permissions.clone())
Some(expected_permissions.clone().into())
);
test.codex
.submit(Op::ExecApproval {
@@ -875,9 +880,9 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() ->
let normalized_requested_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![]),
Some(vec![AbsolutePathBuf::try_from(
Some(vec![PathUri::from_abs_path(&AbsolutePathBuf::try_from(
outside_dir.path().canonicalize()?,
)?]),
)?)]),
)),
..RequestPermissionProfile::default()
};
@@ -1011,7 +1016,7 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul
let approval = expect_exec_approval(&test, &command).await;
assert_eq!(
approval.additional_permissions,
Some(normalized_requested_permissions)
Some(normalized_requested_permissions.into())
);
test.codex
.submit(Op::ExecApproval {
@@ -1085,9 +1090,9 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls() -> Resul
let normalized_requested_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![]),
Some(vec![AbsolutePathBuf::try_from(
Some(vec![PathUri::from_abs_path(&AbsolutePathBuf::try_from(
outside_dir.path().canonicalize()?,
)?]),
)?)]),
)),
..Default::default()
};
@@ -1569,8 +1574,12 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![]),
Some(vec![
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
PathUri::from_abs_path(&AbsolutePathBuf::try_from(
first_dir.path().canonicalize()?,
)?),
PathUri::from_abs_path(&AbsolutePathBuf::try_from(
second_dir.path().canonicalize()?,
)?),
]),
)),
..RequestPermissionProfile::default()
@@ -1581,8 +1590,12 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![]),
Some(vec![
AbsolutePathBuf::try_from(first_dir.path().canonicalize()?)?,
AbsolutePathBuf::try_from(second_dir.path().canonicalize()?)?,
PathUri::from_abs_path(&AbsolutePathBuf::try_from(
first_dir.path().canonicalize()?,
)?),
PathUri::from_abs_path(&AbsolutePathBuf::try_from(
second_dir.path().canonicalize()?,
)?),
]),
)),
..Default::default()
@@ -1655,7 +1668,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
assert!(approval_reads.as_ref().is_none_or(Vec::is_empty));
let mut approval_writes = approval_writes.unwrap_or_default();
approval_writes.sort_by_key(|path| path.display().to_string());
approval_writes.sort_by_key(ToString::to_string);
let (_, expected_writes) = merged_permissions
.file_system
@@ -1663,7 +1676,7 @@ async fn partial_request_permissions_grants_do_not_preapprove_new_permissions()
.legacy_read_write_roots()
.expect("expected legacy-compatible permissions");
let mut expected_writes = expected_writes.unwrap_or_default();
expected_writes.sort_by_key(|path| path.display().to_string());
expected_writes.sort_by_key(ToString::to_string);
assert_eq!(approval_writes, expected_writes);
test.codex
+152 -12
View File
@@ -1,11 +1,17 @@
use crate::mcp::RequestId;
use crate::models::AdditionalPermissionProfile;
use crate::models::FileSystemPermissions;
use crate::models::PermissionProfile;
use crate::parse_command::ParsedCommand;
use crate::permissions::FileSystemPath;
use crate::permissions::FileSystemSandboxEntry;
use crate::protocol::FileChange;
use crate::protocol::ReviewDecision;
use crate::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::request_permissions::deserialize_request_permission_profile_from_native_paths;
use crate::request_permissions::serialize_request_permission_profile_as_native_paths;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathUri;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
@@ -138,17 +144,33 @@ pub enum GuardianAssessmentAction {
Command {
source: GuardianCommandSource,
command: String,
cwd: AbsolutePathBuf,
#[serde(
serialize_with = "serialize_path_uri_as_native_path",
deserialize_with = "deserialize_path_uri_from_native_path"
)]
cwd: PathUri,
},
Execve {
source: GuardianCommandSource,
program: String,
argv: Vec<String>,
cwd: AbsolutePathBuf,
#[serde(
serialize_with = "serialize_path_uri_as_native_path",
deserialize_with = "deserialize_path_uri_from_native_path"
)]
cwd: PathUri,
},
ApplyPatch {
cwd: AbsolutePathBuf,
files: Vec<AbsolutePathBuf>,
#[serde(
serialize_with = "serialize_path_uri_as_native_path",
deserialize_with = "deserialize_path_uri_from_native_path"
)]
cwd: PathUri,
#[serde(
serialize_with = "serialize_path_uris_as_native_paths",
deserialize_with = "deserialize_path_uris_from_native_paths"
)]
files: Vec<PathUri>,
},
NetworkAccess {
target: String,
@@ -165,10 +187,119 @@ pub enum GuardianAssessmentAction {
},
RequestPermissions {
reason: Option<String>,
permissions: RequestPermissionProfile,
#[serde(
serialize_with = "serialize_request_permission_profile_as_native_paths",
deserialize_with = "deserialize_request_permission_profile_from_native_paths"
)]
permissions: RequestPermissionProfile<PathUri>,
},
}
fn serialize_path_uri_as_native_path<S>(path: &PathUri, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
serializer.serialize_str(&path.inferred_native_path_string())
}
fn deserialize_path_uri_from_native_path<'de, D>(deserializer: D) -> Result<PathUri, D::Error>
where
D: serde::Deserializer<'de>,
{
let path = LegacyAppPathString::deserialize(deserializer)?;
PathUri::try_from(path).map_err(serde::de::Error::custom)
}
fn serialize_path_uris_as_native_paths<S>(
paths: &[PathUri],
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
paths
.iter()
.map(PathUri::inferred_native_path_string)
.collect::<Vec<_>>()
.serialize(serializer)
}
fn deserialize_path_uris_from_native_paths<'de, D>(
deserializer: D,
) -> Result<Vec<PathUri>, D::Error>
where
D: serde::Deserializer<'de>,
{
Vec::<LegacyAppPathString>::deserialize(deserializer)?
.into_iter()
.map(|path| PathUri::try_from(path).map_err(serde::de::Error::custom))
.collect()
}
fn serialize_optional_additional_permissions_as_native_paths<S>(
permissions: &Option<AdditionalPermissionProfile<PathUri>>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
permissions
.clone()
.map(|permissions| permissions.map_paths(|path| path.inferred_native_path_string()))
.serialize(serializer)
}
fn deserialize_optional_additional_permissions_from_native_paths<'de, D>(
deserializer: D,
) -> Result<Option<AdditionalPermissionProfile<PathUri>>, D::Error>
where
D: serde::Deserializer<'de>,
{
Option::<AdditionalPermissionProfile<LegacyAppPathString>>::deserialize(deserializer)?
.map(|permissions| {
try_map_additional_permission_paths(permissions, |path| {
PathUri::try_from(path).map_err(serde::de::Error::custom)
})
})
.transpose()
}
fn try_map_additional_permission_paths<InputPath, OutputPath, Error>(
permissions: AdditionalPermissionProfile<InputPath>,
mut map: impl FnMut(InputPath) -> Result<OutputPath, Error>,
) -> Result<AdditionalPermissionProfile<OutputPath>, Error> {
let file_system = permissions
.file_system
.map(|file_system| {
let entries = file_system
.entries
.into_iter()
.map(|entry| {
let path = match entry.path {
FileSystemPath::Path { path } => FileSystemPath::Path { path: map(path)? },
FileSystemPath::GlobPattern { pattern } => {
FileSystemPath::GlobPattern { pattern }
}
FileSystemPath::Special { value } => FileSystemPath::Special { value },
};
Ok(FileSystemSandboxEntry {
path,
access: entry.access,
})
})
.collect::<Result<Vec<_>, Error>>()?;
Ok(FileSystemPermissions {
entries,
glob_scan_max_depth: file_system.glob_scan_max_depth,
})
})
.transpose()?;
Ok(AdditionalPermissionProfile {
network: permissions.network,
file_system,
})
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
pub struct NetworkPolicyAmendment {
pub host: String,
@@ -244,7 +375,11 @@ pub struct ExecApprovalRequestEvent {
/// The command to be executed.
pub command: Vec<String>,
/// The command's working directory.
pub cwd: AbsolutePathBuf,
#[serde(
serialize_with = "serialize_path_uri_as_native_path",
deserialize_with = "deserialize_path_uri_from_native_path"
)]
pub cwd: PathUri,
/// Optional human-readable reason for the approval (e.g. retry without sandbox).
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
@@ -261,9 +396,14 @@ pub struct ExecApprovalRequestEvent {
#[ts(optional)]
pub proposed_network_policy_amendments: Option<Vec<NetworkPolicyAmendment>>,
/// Optional additional filesystem permissions requested for this command.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[serde(
default,
skip_serializing_if = "Option::is_none",
serialize_with = "serialize_optional_additional_permissions_as_native_paths",
deserialize_with = "deserialize_optional_additional_permissions_from_native_paths"
)]
#[ts(optional)]
pub additional_permissions: Option<AdditionalPermissionProfile>,
pub additional_permissions: Option<AdditionalPermissionProfile<PathUri>>,
/// Ordered list of decisions the client may present for this prompt.
///
/// When absent, clients should derive the legacy default set from the
@@ -299,7 +439,7 @@ impl ExecApprovalRequestEvent {
network_approval_context: Option<&NetworkApprovalContext>,
proposed_execpolicy_amendment: Option<&ExecPolicyAmendment>,
proposed_network_policy_amendments: Option<&[NetworkPolicyAmendment]>,
additional_permissions: Option<&AdditionalPermissionProfile>,
additional_permissions: Option<&AdditionalPermissionProfile<PathUri>>,
) -> Vec<ReviewDecision> {
if network_approval_context.is_some() {
let mut decisions = vec![ReviewDecision::Approved, ReviewDecision::ApprovedForSession];
@@ -432,7 +572,7 @@ mod tests {
GuardianAssessmentAction::Command {
source: GuardianCommandSource::Shell,
command: "rm -rf /tmp/guardian".to_string(),
cwd: test_path_buf("/tmp").abs(),
cwd: test_path_buf("/tmp").abs().into(),
}
);
}
@@ -465,7 +605,7 @@ mod tests {
"-f".to_string(),
"/tmp/file.sqlite".to_string(),
],
cwd: test_path_buf("/tmp").abs(),
cwd: test_path_buf("/tmp").abs().into(),
}
);
}
+109 -3
View File
@@ -1,7 +1,11 @@
use crate::models::AdditionalPermissionProfile;
use crate::models::FileSystemPermissions;
use crate::models::NetworkPermissions;
use crate::permissions::FileSystemPath;
use crate::permissions::FileSystemSandboxEntry;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathUri;
use schemars::JsonSchema;
use serde::Deserialize;
use serde::Serialize;
@@ -139,8 +143,110 @@ pub struct RequestPermissionsEvent {
pub started_at_ms: i64,
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
pub permissions: RequestPermissionProfile,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[serde(
serialize_with = "serialize_request_permission_profile_as_native_paths",
deserialize_with = "deserialize_request_permission_profile_from_native_paths"
)]
pub permissions: RequestPermissionProfile<PathUri>,
#[serde(
default,
skip_serializing_if = "Option::is_none",
serialize_with = "serialize_optional_path_uri_as_native_path",
deserialize_with = "deserialize_optional_path_uri_from_native_path"
)]
#[ts(optional)]
pub cwd: Option<AbsolutePathBuf>,
pub cwd: Option<PathUri>,
}
pub(crate) fn serialize_request_permission_profile_as_native_paths<S>(
permissions: &RequestPermissionProfile<PathUri>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
permissions
.clone()
.map_paths(|path| path.inferred_native_path_string())
.serialize(serializer)
}
pub(crate) fn deserialize_request_permission_profile_from_native_paths<'de, D>(
deserializer: D,
) -> Result<RequestPermissionProfile<PathUri>, D::Error>
where
D: serde::Deserializer<'de>,
{
let permissions = RequestPermissionProfile::<LegacyAppPathString>::deserialize(deserializer)?;
try_map_request_permission_profile_paths(permissions, path_uri_from_legacy_app_path)
.map_err(serde::de::Error::custom)
}
fn serialize_optional_path_uri_as_native_path<S>(
path: &Option<PathUri>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
path.as_ref()
.map(PathUri::inferred_native_path_string)
.serialize(serializer)
}
fn deserialize_optional_path_uri_from_native_path<'de, D>(
deserializer: D,
) -> Result<Option<PathUri>, D::Error>
where
D: serde::Deserializer<'de>,
{
Option::<LegacyAppPathString>::deserialize(deserializer)?
.map(path_uri_from_legacy_app_path)
.transpose()
.map_err(serde::de::Error::custom)
}
fn path_uri_from_legacy_app_path(
path: LegacyAppPathString,
) -> Result<PathUri, codex_utils_path_uri::LegacyAppPathStringError> {
if let Ok(path_uri) = PathUri::parse(path.as_str()) {
return Ok(path_uri);
}
path.try_into()
}
fn try_map_request_permission_profile_paths<InputPath, OutputPath, Error>(
permissions: RequestPermissionProfile<InputPath>,
mut map: impl FnMut(InputPath) -> Result<OutputPath, Error>,
) -> Result<RequestPermissionProfile<OutputPath>, Error> {
let file_system = permissions
.file_system
.map(|file_system| {
let entries = file_system
.entries
.into_iter()
.map(|entry| {
let path = match entry.path {
FileSystemPath::Path { path } => FileSystemPath::Path { path: map(path)? },
FileSystemPath::GlobPattern { pattern } => {
FileSystemPath::GlobPattern { pattern }
}
FileSystemPath::Special { value } => FileSystemPath::Special { value },
};
Ok(FileSystemSandboxEntry {
path,
access: entry.access,
})
})
.collect::<Result<Vec<_>, Error>>()?;
Ok(FileSystemPermissions {
entries,
glob_scan_max_depth: file_system.glob_scan_max_depth,
})
})
.transpose()?;
Ok(RequestPermissionProfile {
network: permissions.network,
file_system,
})
}
+30 -76
View File
@@ -3,16 +3,13 @@ use std::collections::VecDeque;
use super::App;
use crate::app_command::AppCommand;
use crate::app_server_approval_conversions::granted_permission_profile_from_request;
use crate::app_server_session::AppServerSession;
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::McpServerElicitationRequestResponse;
use codex_app_server_protocol::PermissionsRequestApprovalResponse;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_app_server_protocol::ServerRequest;
use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile;
impl App {
pub(super) async fn reject_app_server_request(
@@ -104,18 +101,6 @@ impl PendingAppServerRequests {
None
}
ServerRequest::PermissionsRequestApproval { request_id, params } => {
// TODO(anp): Remove this duplicate validation once core permission paths remain
// PathUri after crossing the app-server boundary. Native permission paths do not
// yet have an ingress validation step, so validate them here before recording the
// request as pending. Discovering an invalid path later in a UI delivery path
// would leave the app-server RPC waiting without a clean rejection path.
if let Err(err) = CoreRequestPermissionProfile::try_from(params.permissions.clone())
{
return Some(UnsupportedAppServerRequest {
request_id: request_id.clone(),
message: format!("failed to localize requested filesystem paths: {err}"),
});
}
self.permissions_approvals
.insert(params.item_id.clone(), request_id.clone());
None
@@ -223,14 +208,7 @@ impl PendingAppServerRequests {
.map(|request_id| {
Ok::<AppServerRequestResolution, String>(AppServerRequestResolution {
request_id,
result: serde_json::to_value(PermissionsRequestApprovalResponse {
permissions: granted_permission_profile_from_request(
response.permissions.clone(),
),
scope: response.scope.into(),
strict_auto_review: response.strict_auto_review.then_some(true),
})
.map_err(|err| {
result: serde_json::to_value(response).map_err(|err| {
format!("failed to serialize permissions approval response: {err}")
})?,
})
@@ -417,7 +395,6 @@ struct McpRequestKey {
mod tests {
use super::PendingAppServerRequests;
use super::ResolvedAppServerRequest;
use super::UnsupportedAppServerRequest;
use crate::app_command::AppCommand as Op;
use codex_app_server_protocol::AdditionalFileSystemPermissions;
use codex_app_server_protocol::AdditionalNetworkPermissions;
@@ -438,9 +415,6 @@ mod tests {
use codex_app_server_protocol::ToolRequestUserInputAnswer;
use codex_app_server_protocol::ToolRequestUserInputParams;
use codex_app_server_protocol::ToolRequestUserInputResponse;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use serde_json::json;
@@ -488,9 +462,8 @@ mod tests {
}
#[test]
fn rejects_permissions_with_paths_that_cannot_be_localized() {
fn preserves_permissions_with_relative_target_paths() {
let mut pending = PendingAppServerRequests::default();
let request_id = AppServerRequestId::Integer(7);
let permissions = codex_app_server_protocol::RequestPermissionProfile {
network: None,
file_system: Some(AdditionalFileSystemPermissions {
@@ -503,36 +476,28 @@ mod tests {
entries: None,
}),
};
let localization_error =
RequestPermissionProfile::try_from(permissions.clone()).expect_err("relative path");
let cwd = AbsolutePathBuf::try_from(PathBuf::from(if cfg!(windows) {
r"C:\tmp"
} else {
"/tmp"
}))
.expect("path must be absolute");
let request = ServerRequest::PermissionsRequestApproval {
request_id: AppServerRequestId::Integer(7),
params: PermissionsRequestApprovalParams {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "perm-1".to_string(),
environment_id: None,
started_at_ms: 0,
cwd: cwd.into(),
reason: None,
permissions,
},
};
assert_eq!(
pending.note_server_request(&ServerRequest::PermissionsRequestApproval {
request_id: request_id.clone(),
params: PermissionsRequestApprovalParams {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "perm-1".to_string(),
environment_id: None,
started_at_ms: 0,
cwd,
reason: None,
permissions,
},
}),
Some(UnsupportedAppServerRequest {
request_id,
message: format!(
"failed to localize requested filesystem paths: {localization_error}"
),
})
);
assert_eq!(pending.note_server_request(&request), None);
assert!(pending.contains_server_request(&request));
}
#[test]
@@ -561,7 +526,7 @@ mod tests {
item_id: "perm-1".to_string(),
environment_id: None,
started_at_ms: 0,
cwd: absolute_path(if cfg!(windows) { r"C:\tmp" } else { "/tmp" }),
cwd: absolute_path(if cfg!(windows) { r"C:\tmp" } else { "/tmp" }).into(),
reason: None,
permissions: serde_json::from_value(json!({
"network": { "enabled": null }
@@ -588,18 +553,20 @@ mod tests {
let permissions = pending
.take_resolution(&Op::RequestPermissionsResponse {
id: "perm-1".to_string(),
response: codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
response: PermissionsRequestApprovalResponse {
permissions: codex_app_server_protocol::GrantedPermissionProfile {
network: Some(AdditionalNetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path(read_path)]),
Some(vec![absolute_path(write_path)]),
)),
file_system: Some(AdditionalFileSystemPermissions {
read: Some(vec![absolute_path(read_path).into()]),
write: Some(vec![absolute_path(write_path).into()]),
glob_scan_max_depth: None,
entries: None,
}),
},
scope: codex_protocol::request_permissions::PermissionGrantScope::Session,
strict_auto_review: false,
scope: PermissionGrantScope::Session,
strict_auto_review: None,
},
})
.expect("permissions response should serialize")
@@ -617,20 +584,7 @@ mod tests {
read: Some(vec![absolute_path(read_path).into()]),
write: Some(vec![absolute_path(write_path).into()]),
glob_scan_max_depth: None,
entries: Some(vec![
codex_app_server_protocol::FileSystemSandboxEntry {
path: codex_app_server_protocol::FileSystemPath::Path {
path: absolute_path(read_path).into(),
},
access: codex_app_server_protocol::FileSystemAccessMode::Read,
},
codex_app_server_protocol::FileSystemSandboxEntry {
path: codex_app_server_protocol::FileSystemPath::Path {
path: absolute_path(write_path).into(),
},
access: codex_app_server_protocol::FileSystemAccessMode::Write,
},
]),
entries: None,
}),
},
scope: PermissionGrantScope::Session,
+62 -46
View File
@@ -26,6 +26,7 @@ use crate::history_cell::UserHistoryCell;
use crate::history_cell::new_session_info;
use crate::multi_agents::AgentPickerThreadEntry;
use crate::multi_agents::SubAgentActivityDisplay;
use crate::session_state::LocalWorkspaceState;
use assert_matches::assert_matches;
use crate::app_command::AppCommand as Op;
@@ -91,11 +92,8 @@ use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::config_types::Settings;
use codex_protocol::models::ActivePermissionProfile;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::MAX_THREAD_GOAL_OBJECTIVE_CHARS;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::user_input::TextElement;
use codex_utils_absolute_path::AbsolutePathBuf;
use crossterm::event::KeyModifiers;
@@ -2241,7 +2239,7 @@ async fn inactive_thread_approval_bubbles_into_active_view() -> Result<()> {
/*capacity*/ 1,
ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
permission_profile: PermissionProfile::workspace_write().into(),
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
},
@@ -2400,7 +2398,7 @@ async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()>
/*capacity*/ 4,
ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
permission_profile: PermissionProfile::workspace_write().into(),
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
},
@@ -2643,7 +2641,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions(
item_id: "call-approval".to_string(),
environment_id: Some("remote".to_string()),
started_at_ms: 0,
cwd: test_absolute_path("/tmp"),
cwd: test_absolute_path("/tmp").into(),
reason: Some("Need access to .git".to_string()),
permissions: codex_app_server_protocol::RequestPermissionProfile {
network: Some(AdditionalNetworkPermissions {
@@ -2674,14 +2672,16 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions(
assert_eq!(environment_id.as_deref(), Some("remote"));
assert_eq!(
permissions,
RequestPermissionProfile {
network: Some(NetworkPermissions {
codex_app_server_protocol::RequestPermissionProfile {
network: Some(AdditionalNetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![test_absolute_path("/tmp/read-only")]),
Some(vec![test_absolute_path("/tmp/write")]),
)),
file_system: Some(AdditionalFileSystemPermissions {
read: Some(vec![test_absolute_path("/tmp/read-only").into()]),
write: Some(vec![test_absolute_path("/tmp/write").into()]),
glob_scan_max_depth: None,
entries: None,
}),
}
);
}
@@ -2784,7 +2784,7 @@ async fn inactive_thread_approval_badge_clears_after_turn_completion_notificatio
/*capacity*/ 4,
ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
permission_profile: PermissionProfile::workspace_write().into(),
rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")),
..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent"))
},
@@ -2839,8 +2839,8 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re
let shared_root = test_path_buf("/tmp/shared").abs();
let primary_session = ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
runtime_workspace_roots: vec![primary_cwd.clone(), shared_root.clone()],
permission_profile: PermissionProfile::workspace_write().into(),
runtime_workspace_roots: vec![primary_cwd.clone().into(), shared_root.clone().into()],
..test_thread_session(main_thread_id, primary_cwd.to_path_buf())
};
@@ -2886,8 +2886,10 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re
updated_at: 2,
recency_at: Some(2),
status: codex_app_server_protocol::ThreadStatus::Idle,
path: Some(rollout_path.clone()),
cwd: test_path_buf("/tmp/agent").abs(),
path: Some(codex_app_server_protocol::LegacyAppPathString::from_path(
&rollout_path,
)),
cwd: test_path_buf("/tmp/agent").abs().into(),
cli_version: "0.0.0".to_string(),
source: codex_app_server_protocol::SessionSource::Unknown,
thread_source: None,
@@ -2916,10 +2918,10 @@ async fn inactive_thread_started_notification_initializes_replay_session() -> Re
assert_eq!(session.model, "gpt-agent");
assert_eq!(session.model_provider_id, "agent-provider");
assert_eq!(session.approval_policy, primary_session.approval_policy);
assert_eq!(session.cwd.as_path(), test_path_buf("/tmp/agent").as_path());
assert_eq!(session.cwd, test_path_buf("/tmp/agent").abs().into());
assert_eq!(
session.runtime_workspace_roots,
vec![test_path_buf("/tmp/agent").abs(), shared_root]
vec![test_path_buf("/tmp/agent").abs().into(), shared_root.into()]
);
assert_eq!(session.rollout_path, Some(rollout_path));
assert_eq!(
@@ -2947,8 +2949,8 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_
let primary_cwd = test_path_buf("/tmp/main").abs();
let primary_session = ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
runtime_workspace_roots: vec![primary_cwd.clone()],
permission_profile: PermissionProfile::workspace_write().into(),
runtime_workspace_roots: vec![primary_cwd.clone().into()],
..test_thread_session(main_thread_id, primary_cwd.to_path_buf())
};
@@ -2982,7 +2984,7 @@ async fn inactive_thread_started_notification_preserves_primary_model_when_path_
recency_at: Some(2),
status: codex_app_server_protocol::ThreadStatus::Idle,
path: None,
cwd: test_path_buf("/tmp/agent").abs(),
cwd: test_path_buf("/tmp/agent").abs().into(),
cli_version: "0.0.0".to_string(),
source: codex_app_server_protocol::SessionSource::Unknown,
thread_source: None,
@@ -3023,8 +3025,8 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
let primary_cwd = test_path_buf("/tmp/main").abs();
let primary_session = ThreadSessionState {
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::workspace_write(),
runtime_workspace_roots: vec![primary_cwd.clone()],
permission_profile: PermissionProfile::workspace_write().into(),
runtime_workspace_roots: vec![primary_cwd.clone().into()],
..test_thread_session(main_thread_id, primary_cwd.to_path_buf())
};
app.primary_session_configured = Some(primary_session);
@@ -3044,7 +3046,7 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
recency_at: Some(2),
status: codex_app_server_protocol::ThreadStatus::Idle,
path: None,
cwd: test_path_buf("/tmp/read").abs(),
cwd: test_path_buf("/tmp/read").abs().into(),
cli_version: "0.0.0".to_string(),
source: codex_app_server_protocol::SessionSource::Unknown,
thread_source: None,
@@ -3060,10 +3062,10 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
.await;
assert_eq!(session.thread_id, read_thread_id);
assert_eq!(session.cwd.as_path(), test_path_buf("/tmp/read").as_path());
assert_eq!(session.cwd, test_path_buf("/tmp/read").abs().into());
assert_eq!(
session.runtime_workspace_roots,
vec![test_path_buf("/tmp/read").abs()]
vec![test_path_buf("/tmp/read").abs().into()]
);
let expected_permission_profile = app
.chat_widget
@@ -3072,7 +3074,8 @@ async fn thread_read_session_state_does_not_reuse_primary_permission_profile() {
.permission_profile()
.clone();
assert_eq!(
session.permission_profile, expected_permission_profile,
session.permission_profile,
expected_permission_profile.into(),
"thread/read does not return fresh server permissions; the fallback profile must use the \
active widget permissions rather than reusing the cached primary session profile"
);
@@ -3930,9 +3933,9 @@ async fn render_clear_ui_header_after_long_transcript_for_snapshot() -> String {
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: test_path_buf("/tmp/project").abs(),
cwd: test_path_buf("/tmp/project").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: Some(ReasoningEffortConfig::High),
@@ -3940,6 +3943,7 @@ async fn render_clear_ui_header_after_long_transcript_for_snapshot() -> String {
personality: None,
message_history: None,
network_proxy: None,
local_workspace: None,
rollout_path: Some(PathBuf::new()),
};
Arc::new(new_session_info(
@@ -4395,6 +4399,7 @@ async fn replace_goal_confirmation_snapshot() {
}
fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState {
let cwd = cwd.abs();
ThreadSessionState {
thread_id,
forked_from_id: None,
@@ -4405,9 +4410,9 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: cwd.abs(),
cwd: cwd.clone().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
@@ -4415,6 +4420,10 @@ fn test_thread_session(thread_id: ThreadId, cwd: PathBuf) -> ThreadSessionState
personality: None,
message_history: None,
network_proxy: None,
local_workspace: Some(LocalWorkspaceState {
cwd,
runtime_workspace_roots: Vec::new(),
}),
rollout_path: Some(PathBuf::new()),
}
}
@@ -5019,9 +5028,9 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() {
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: test_path_buf("/home/user/project").abs(),
cwd: test_path_buf("/home/user/project").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
@@ -5029,6 +5038,7 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() {
personality: None,
message_history: None,
network_proxy: None,
local_workspace: None,
rollout_path: Some(PathBuf::new()),
};
Arc::new(new_session_info(
@@ -5085,9 +5095,9 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() {
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: test_path_buf("/home/user/project").abs(),
cwd: test_path_buf("/home/user/project").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
@@ -5095,6 +5105,7 @@ async fn backtrack_selection_with_duplicate_history_targets_unique_turn() {
personality: None,
message_history: None,
network_proxy: None,
local_workspace: None,
rollout_path: Some(PathBuf::new()),
});
@@ -5235,9 +5246,9 @@ async fn backtrack_resubmit_preserves_data_image_urls_in_user_turn() {
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: test_path_buf("/home/user/project").abs(),
cwd: test_path_buf("/home/user/project").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
@@ -5245,6 +5256,7 @@ async fn backtrack_resubmit_preserves_data_image_urls_in_user_turn() {
personality: None,
message_history: None,
network_proxy: None,
local_workspace: None,
rollout_path: Some(PathBuf::new()),
});
@@ -5478,7 +5490,7 @@ async fn refreshed_snapshot_session_persists_resumed_turns() {
}],
)];
let resumed_session = ThreadSessionState {
cwd: test_path_buf("/tmp/refreshed").abs(),
cwd: test_path_buf("/tmp/refreshed").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
..initial_session.clone()
@@ -5662,7 +5674,7 @@ async fn thread_rollback_response_discards_queued_active_thread_events() {
recency_at: Some(0),
status: codex_app_server_protocol::ThreadStatus::Idle,
path: None,
cwd: test_path_buf("/tmp/project").abs(),
cwd: test_path_buf("/tmp/project").abs().into(),
cli_version: "0.0.0".to_string(),
source: SessionSource::Cli,
thread_source: None,
@@ -5699,9 +5711,9 @@ async fn new_session_requests_shutdown_for_previous_conversation() {
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: test_path_buf("/home/user/project").abs(),
cwd: test_path_buf("/home/user/project").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
@@ -5709,6 +5721,7 @@ async fn new_session_requests_shutdown_for_previous_conversation() {
personality: None,
message_history: None,
network_proxy: None,
local_workspace: None,
rollout_path: Some(PathBuf::new()),
};
@@ -6035,7 +6048,7 @@ async fn inactive_thread_settings_notification_updates_cached_collaboration_mode
let notification = ThreadSettingsUpdatedNotification {
thread_id: inactive_thread_id.to_string(),
thread_settings: ThreadSettings {
cwd: test_absolute_path("/tmp/thread-settings"),
cwd: test_absolute_path("/tmp/thread-settings").into(),
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: codex_app_server_protocol::ApprovalsReviewer::AutoReview,
sandbox_policy: codex_app_server_protocol::SandboxPolicy::ReadOnly {
@@ -6113,9 +6126,9 @@ async fn clear_only_ui_reset_preserves_chat_session_state() {
service_tier: None,
approval_policy: AskForApproval::Never,
approvals_reviewer: ApprovalsReviewer::User,
permission_profile: PermissionProfile::read_only(),
permission_profile: PermissionProfile::read_only().into(),
active_permission_profile: None,
cwd: test_path_buf("/tmp/project").abs(),
cwd: test_path_buf("/tmp/project").abs().into(),
runtime_workspace_roots: Vec::new(),
instruction_source_paths: Vec::new(),
reasoning_effort: None,
@@ -6123,6 +6136,7 @@ async fn clear_only_ui_reset_preserves_chat_session_state() {
personality: None,
message_history: None,
network_proxy: None,
local_workspace: None,
rollout_path: Some(PathBuf::new()),
});
app.chat_widget
@@ -6162,7 +6176,9 @@ async fn clear_only_ui_reset_preserves_chat_session_state() {
async fn clear_only_ui_reset_allows_active_skill_warning_to_render_again() {
let mut app = make_test_app().await;
let error = SkillErrorInfo {
path: test_path_buf("/tmp/project/.codex/skills/abc/SKILL.md"),
path: codex_utils_path_uri::LegacyAppPathString::from_path(&test_path_buf(
"/tmp/project/.codex/skills/abc/SKILL.md",
)),
message: "invalid description".to_string(),
};
+47 -25
View File
@@ -5,7 +5,9 @@
//! when the visible thread changes.
use super::*;
use crate::app_server_session::local_rollout_path;
use crate::session_resume::read_session_model;
use codex_utils_path_uri::PathUri;
#[derive(Clone, Copy)]
pub(super) enum ThreadRollbackOrigin {
@@ -196,6 +198,17 @@ impl App {
}
pub(super) async fn thread_cwd(&self, thread_id: ThreadId) -> Option<AbsolutePathBuf> {
let channel = self.thread_event_channels.get(&thread_id)?;
let store = channel.store.lock().await;
store
.session
.as_ref()?
.local_workspace
.as_ref()
.map(|workspace| workspace.cwd.clone())
}
async fn thread_target_cwd(&self, thread_id: ThreadId) -> Option<PathUri> {
let channel = self.thread_event_channels.get(&thread_id)?;
let store = channel.store.lock().await;
store.session.as_ref().map(|session| session.cwd.clone())
@@ -315,26 +328,16 @@ impl App {
}
}
}
ServerRequest::PermissionsRequestApproval { params, .. } => {
// TODO(anp): Remove this native-path localization error path once core permission
// paths remain PathUri after crossing the app-server boundary.
let permissions = params.permissions.clone().try_into().map_err(|err| {
std::io::Error::new(
std::io::ErrorKind::InvalidInput,
format!("failed to localize requested filesystem paths: {err}"),
)
})?;
Some(ThreadInteractiveRequest::Approval(
ApprovalRequest::Permissions {
thread_id,
thread_label,
call_id: params.item_id.clone(),
environment_id: params.environment_id.clone(),
reason: params.reason.clone(),
permissions,
},
))
}
ServerRequest::PermissionsRequestApproval { params, .. } => Some(
ThreadInteractiveRequest::Approval(ApprovalRequest::Permissions {
thread_id,
thread_label,
call_id: params.item_id.clone(),
environment_id: params.environment_id.clone(),
reason: params.reason.clone(),
permissions: params.permissions.clone(),
}),
),
_ => None,
})
}
@@ -639,6 +642,10 @@ impl App {
}
}
if should_start_turn {
let target_cwd = self.thread_target_cwd(thread_id).await.unwrap_or_else(|| {
PathUri::from_host_native_path(cwd)
.unwrap_or_else(|_| PathUri::from_abs_path(&self.config.cwd))
});
let config = self.chat_widget.config_ref();
let approvals_reviewer =
approvals_reviewer.unwrap_or(config.approvals_reviewer);
@@ -653,7 +660,8 @@ impl App {
.turn_start(
thread_id,
items.to_vec(),
cwd.clone(),
target_cwd,
config.cwd.as_path(),
*approval_policy,
approvals_reviewer,
permissions_override,
@@ -680,7 +688,12 @@ impl App {
self.handle_skills_list_result(
app_server
.skills_list(codex_app_server_protocol::SkillsListParams {
cwds: cwds.clone(),
cwds: cwds
.iter()
.map(|cwd| {
codex_utils_path_uri::LegacyAppPathString::from_path(cwd)
})
.collect(),
force_reload: *force_reload,
})
.await,
@@ -992,9 +1005,18 @@ impl App {
session.thread_id = thread_id;
session.thread_name = notification.thread.name.clone();
session.model_provider_id = notification.thread.model_provider.clone();
session
.set_cwd_retargeting_implicit_runtime_workspace_root(notification.thread.cwd.clone());
let rollout_path = notification.thread.path.clone();
if let Some(cwd) = notification.thread.cwd.to_inferred_path_uri() {
session.set_cwd_retargeting_implicit_runtime_workspace_root(cwd);
} else {
tracing::warn!(
cwd = %notification.thread.cwd,
"ignoring non-absolute cwd in thread/started notification"
);
}
let rollout_path = local_rollout_path(
notification.thread.path.as_ref(),
self.app_server_target.thread_params_mode(),
);
if let Some(model) =
read_session_model(self.state_db.as_deref(), thread_id, rollout_path.as_deref()).await
{
+3 -3
View File
@@ -4,6 +4,7 @@ use codex_app_server_protocol::AskForApproval;
use codex_app_server_protocol::CommandExecutionApprovalDecision;
use codex_app_server_protocol::FileChangeApprovalDecision;
use codex_app_server_protocol::McpServerElicitationAction;
use codex_app_server_protocol::PermissionsRequestApprovalResponse;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_app_server_protocol::ReviewTarget;
use codex_app_server_protocol::ToolRequestUserInputResponse;
@@ -17,7 +18,6 @@ use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::ActivePermissionProfile;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use serde::Serialize;
use serde_json::Value;
@@ -81,7 +81,7 @@ pub(crate) enum AppCommand {
},
RequestPermissionsResponse {
id: String,
response: RequestPermissionsResponse,
response: PermissionsRequestApprovalResponse,
},
ReloadUserConfig,
ListSkills {
@@ -230,7 +230,7 @@ impl AppCommand {
pub(crate) fn request_permissions_response(
id: String,
response: RequestPermissionsResponse,
response: PermissionsRequestApprovalResponse,
) -> Self {
Self::RequestPermissionsResponse { id, response }
}
+2 -2
View File
@@ -9,11 +9,11 @@ use crate::app_command::AppCommand;
use codex_app_server_protocol::CommandExecutionApprovalDecision;
use codex_app_server_protocol::FileChangeApprovalDecision;
use codex_app_server_protocol::McpServerElicitationAction;
use codex_app_server_protocol::PermissionsRequestApprovalResponse;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_app_server_protocol::ReviewTarget;
use codex_app_server_protocol::ToolRequestUserInputResponse;
use codex_protocol::ThreadId;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use tokio::sync::mpsc::UnboundedSender;
use crate::app_event::AppEvent;
@@ -93,7 +93,7 @@ impl AppEventSender {
&self,
thread_id: ThreadId,
id: String,
response: RequestPermissionsResponse,
response: PermissionsRequestApprovalResponse,
) {
self.send(AppEvent::SubmitThreadOp {
thread_id,
@@ -6,22 +6,19 @@
//! submission.
use crate::diff_model::FileChange;
use codex_app_server_protocol::AdditionalNetworkPermissions;
use codex_app_server_protocol::FileUpdateChange;
use codex_app_server_protocol::GrantedPermissionProfile;
use codex_app_server_protocol::PatchChangeKind;
use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile;
use codex_app_server_protocol::RequestPermissionProfile;
use std::collections::HashMap;
use std::path::PathBuf;
pub(crate) fn granted_permission_profile_from_request(
value: CoreRequestPermissionProfile,
value: RequestPermissionProfile,
) -> GrantedPermissionProfile {
GrantedPermissionProfile {
network: value.network.map(|network| AdditionalNetworkPermissions {
enabled: network.enabled,
}),
file_system: value.file_system.map(Into::into),
network: value.network,
file_system: value.file_system,
}
}
@@ -41,7 +38,7 @@ pub(crate) fn file_update_changes_to_display(
},
PatchChangeKind::Update { move_path } => FileChange::Update {
unified_diff: change.diff,
move_path,
move_path: move_path.map(PathBuf::from),
},
};
(path, file_change)
@@ -64,7 +61,6 @@ mod tests {
use codex_app_server_protocol::GrantedPermissionProfile;
use codex_app_server_protocol::PatchChangeKind;
use codex_app_server_protocol::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
@@ -104,9 +100,6 @@ mod tests {
entries: None,
}),
};
let request = CoreRequestPermissionProfile::try_from(request)
.expect("API paths should convert to native paths");
assert_eq!(
granted_permission_profile_from_request(request),
GrantedPermissionProfile {
@@ -117,20 +110,7 @@ mod tests {
read: Some(vec![absolute_path("/tmp/read-only").into()]),
write: Some(vec![absolute_path("/tmp/write").into()]),
glob_scan_max_depth: None,
entries: Some(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: absolute_path("/tmp/read-only").into(),
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: absolute_path("/tmp/write").into(),
},
access: FileSystemAccessMode::Write,
},
]),
entries: None,
}),
}
);
@@ -152,9 +132,6 @@ mod tests {
}]),
}),
};
let request = CoreRequestPermissionProfile::try_from(request)
.expect("API paths should convert to native paths");
assert_eq!(
granted_permission_profile_from_request(request),
GrantedPermissionProfile {
+19 -3
View File
@@ -12,10 +12,11 @@ use crate::diff_model::FileChange;
use codex_app_server_protocol::AdditionalPermissionProfile;
use codex_app_server_protocol::CommandExecutionApprovalDecision;
use codex_app_server_protocol::ExecPolicyAmendment;
use codex_app_server_protocol::LegacyAppPathString;
use codex_app_server_protocol::NetworkApprovalContext;
use codex_app_server_protocol::NetworkPolicyAmendment;
use codex_app_server_protocol::NetworkPolicyRuleAction;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_app_server_protocol::RequestPermissionProfile;
use serde::Deserialize;
use serde::Serialize;
@@ -29,7 +30,7 @@ pub(crate) struct ExecApprovalRequestEvent {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) environment_id: Option<String>,
pub(crate) command: Vec<String>,
pub(crate) cwd: AbsolutePathBuf,
pub(crate) cwd: LegacyAppPathString,
pub(crate) reason: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
@@ -117,5 +118,20 @@ pub(crate) struct ApplyPatchApprovalRequestEvent {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) reason: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) grant_root: Option<PathBuf>,
pub(crate) grant_root: Option<LegacyAppPathString>,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
pub(crate) struct RequestPermissionsEvent {
pub(crate) call_id: String,
#[serde(default)]
pub(crate) turn_id: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) environment_id: Option<String>,
pub(crate) started_at_ms: i64,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) reason: Option<String>,
pub(crate) permissions: RequestPermissionProfile,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub(crate) cwd: Option<LegacyAppPathString>,
}
@@ -49,13 +49,13 @@ use codex_app_server_protocol::McpServerElicitationAction;
use codex_app_server_protocol::NetworkApprovalContext;
use codex_app_server_protocol::NetworkApprovalProtocol;
use codex_app_server_protocol::NetworkPolicyRuleAction;
use codex_app_server_protocol::PermissionGrantScope;
use codex_app_server_protocol::PermissionsRequestApprovalResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::RequestPermissionProfile;
use codex_features::Features;
use codex_protocol::ThreadId;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::LegacyAppPathString;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyEventKind;
@@ -399,7 +399,11 @@ impl ApprovalOverlay {
let granted_permissions = match decision {
PermissionsDecision::GrantForTurn
| PermissionsDecision::GrantForTurnWithStrictAutoReview
| PermissionsDecision::GrantForSession => permissions.clone(),
| PermissionsDecision::GrantForSession => {
crate::app_server_approval_conversions::granted_permission_profile_from_request(
permissions.clone(),
)
}
PermissionsDecision::Deny => Default::default(),
};
let scope = if matches!(decision, PermissionsDecision::GrantForSession) {
@@ -429,10 +433,10 @@ impl ApprovalOverlay {
self.app_event_tx.request_permissions_response(
thread_id,
call_id.to_string(),
codex_protocol::request_permissions::RequestPermissionsResponse {
PermissionsRequestApprovalResponse {
permissions: granted_permissions,
scope,
strict_auto_review,
strict_auto_review: strict_auto_review.then_some(true),
},
);
}
@@ -1029,9 +1033,12 @@ fn special_path_label(value: &FileSystemSpecialPath) -> String {
}
}
fn path_label(base: &str, subpath: &Option<LegacyAppPathString>) -> String {
fn path_label(
base: &str,
subpath: &Option<codex_app_server_protocol::LegacyAppPathString>,
) -> String {
match subpath {
Some(subpath) => format!("{base}/{subpath}"),
Some(subpath) => format!("{base}/{}", subpath.render_for_ui()),
None => base.to_string(),
}
}
@@ -1250,13 +1257,19 @@ mod tests {
environment_id: None,
reason: Some("need workspace access".to_string()),
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
file_system: Some(FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path("/tmp/readme.txt")]),
Some(vec![absolute_path("/tmp/out.txt")]),
)),
network: Some(
NetworkPermissions {
enabled: Some(true),
}
.into(),
),
file_system: Some(
FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path("/tmp/readme.txt").into()]),
Some(vec![absolute_path("/tmp/out.txt").into()]),
)
.into(),
),
},
}
}
@@ -1787,8 +1800,8 @@ mod tests {
network: None,
file_system: Some(
FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path("/tmp/readme.txt")]),
Some(vec![absolute_path("/tmp/out.txt")]),
Some(vec![absolute_path("/tmp/readme.txt").into()]),
Some(vec![absolute_path("/tmp/out.txt").into()]),
)
.into(),
),
@@ -1872,9 +1885,11 @@ mod tests {
entries: Some(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::ProjectRoots {
subpath: Some(LegacyAppPathString::from_path(std::path::Path::new(
".git",
))),
subpath: Some(
codex_app_server_protocol::LegacyAppPathString::from_path(
std::path::Path::new(".git"),
),
),
},
},
access: FileSystemAccessMode::Read,
@@ -1941,7 +1956,7 @@ mod tests {
{
assert!(response.permissions.is_empty());
assert_eq!(response.scope, PermissionGrantScope::Turn);
assert!(!response.strict_auto_review);
assert_eq!(response.strict_auto_review, None);
saw_op = true;
break;
}
@@ -1968,7 +1983,7 @@ mod tests {
} = ev
{
assert_eq!(response.scope, PermissionGrantScope::Turn);
assert!(response.strict_auto_review);
assert_eq!(response.strict_auto_review, Some(true));
saw_op = true;
break;
}
@@ -2001,8 +2016,8 @@ mod tests {
}),
file_system: Some(
FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path("/tmp/readme.txt")]),
Some(vec![absolute_path("/tmp/out.txt")]),
Some(vec![absolute_path("/tmp/readme.txt").into()]),
Some(vec![absolute_path("/tmp/out.txt").into()]),
)
.into(),
),
@@ -2058,8 +2073,8 @@ mod tests {
}),
file_system: Some(
FileSystemPermissions::from_read_write_roots(
Some(vec![absolute_path("/tmp/readme.txt")]),
Some(vec![absolute_path("/tmp/out.txt")]),
Some(vec![absolute_path("/tmp/readme.txt").into()]),
Some(vec![absolute_path("/tmp/out.txt").into()]),
)
.into(),
),
+12 -11
View File
@@ -47,6 +47,7 @@ use crate::app_event::HistoryLookupResponse;
use crate::app_server_approval_conversions::file_update_changes_to_display;
use crate::approval_events::ApplyPatchApprovalRequestEvent;
use crate::approval_events::ExecApprovalRequestEvent;
use crate::approval_events::RequestPermissionsEvent;
use crate::bottom_pane::StatusLineItem;
use crate::bottom_pane::StatusLineSetupView;
use crate::bottom_pane::StatusSurfacePreviewData;
@@ -62,6 +63,7 @@ use crate::mention_codec::encode_history_mentions;
use crate::model_catalog::ModelCatalog;
use crate::multi_agents;
use crate::multi_agents::AgentMetadata;
use crate::session_state::LocalWorkspaceState;
use crate::session_state::SessionNetworkProxyRuntime;
use crate::session_state::ThreadSessionState;
use crate::status::RateLimitWindowDisplay;
@@ -153,7 +155,6 @@ use codex_protocol::items::AgentMessageItem;
use codex_protocol::models::MessagePhase;
use codex_protocol::plan_tool::PlanItemArg as UpdatePlanItemArg;
use codex_protocol::plan_tool::StepStatus as UpdatePlanItemStatus;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use codex_protocol::user_input::ByteRange;
use codex_protocol::user_input::TextElement;
use codex_terminal_detection::Multiplexer;
@@ -162,6 +163,7 @@ use codex_terminal_detection::TerminalName;
use codex_terminal_detection::terminal_info;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_cli::resume_hint;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::mention_syntax::PLUGIN_TEXT_MENTION_SIGIL;
use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL;
@@ -583,7 +585,7 @@ pub(crate) struct ChatWidget {
pending_collab_spawn_requests: HashMap<String, multi_agents::SpawnRequestSummary>,
suppressed_exec_calls: HashSet<String>,
skills_all: Vec<ProtocolSkillMetadata>,
skills_initial_state: Option<HashMap<AbsolutePathBuf, bool>>,
skills_initial_state: Option<HashMap<LegacyAppPathString, bool>>,
last_unified_wait: Option<UnifiedExecWaitState>,
unified_exec_wait_streak: Option<UnifiedExecWaitStreak>,
turn_lifecycle: TurnLifecycleState,
@@ -689,6 +691,8 @@ pub(crate) struct ChatWidget {
current_rollout_path: Option<PathBuf>,
// Current working directory (if known)
current_cwd: Option<PathBuf>,
// Host-local workspace state for embedded sessions. Remote target paths never enter Config.
local_workspace: Option<LocalWorkspaceState>,
// App-server-backed command runner for status-line workspace metadata lookups.
workspace_command_runner: Option<WorkspaceCommandRunner>,
// Instruction source files loaded for the current session, supplied by app-server.
@@ -855,12 +859,9 @@ fn exec_approval_request_from_params(
params: CommandExecutionRequestApprovalParams,
fallback_cwd: &AbsolutePathBuf,
) -> ExecApprovalRequestEvent {
// TODO(anp): Keep this as PathUri once `tui::approval_events::ExecApprovalRequestEvent` and
// approval rendering support foreign paths.
let cwd = params
.cwd
.and_then(|cwd| cwd.to_inferred_abs_path())
.unwrap_or_else(|| fallback_cwd.clone());
.unwrap_or_else(|| LegacyAppPathString::from_abs_path(fallback_cwd));
ExecApprovalRequestEvent {
call_id: params.item_id,
command: params
@@ -895,16 +896,16 @@ fn patch_approval_request_from_params(
fn request_permissions_from_params(
params: codex_app_server_protocol::PermissionsRequestApprovalParams,
) -> std::io::Result<RequestPermissionsEvent> {
Ok(RequestPermissionsEvent {
) -> RequestPermissionsEvent {
RequestPermissionsEvent {
turn_id: params.turn_id,
call_id: params.item_id,
environment_id: params.environment_id,
started_at_ms: params.started_at_ms,
reason: params.reason,
permissions: params.permissions.try_into()?,
permissions: params.permissions,
cwd: Some(params.cwd),
})
}
}
fn token_usage_info_from_app_server(token_usage: ThreadTokenUsage) -> TokenUsageInfo {
@@ -1291,7 +1292,7 @@ impl ChatWidget {
UserInput::Skill { name, path } => Some(MentionBinding {
sigil: TOOL_MENTION_SIGIL,
mention: name.clone(),
path: path.to_string_lossy().into_owned(),
path: path.as_str().to_string(),
}),
UserInput::Mention { name, path } => {
let plugin_id = path.strip_prefix("plugin://");
+2 -2
View File
@@ -5,11 +5,11 @@ use std::collections::VecDeque;
use crate::app::app_server_requests::ResolvedAppServerRequest;
use crate::approval_events::ApplyPatchApprovalRequestEvent;
use crate::approval_events::ExecApprovalRequestEvent;
use crate::approval_events::RequestPermissionsEvent;
use codex_app_server_protocol::McpServerElicitationRequestParams;
use codex_app_server_protocol::RequestId as AppServerRequestId;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ToolRequestUserInputParams;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use super::ChatWidget;
@@ -163,7 +163,7 @@ mod tests {
turn_id: "turn".to_string(),
environment_id: None,
command: vec!["true".to_string()],
cwd: AbsolutePathBuf::current_dir().expect("current dir"),
cwd: AbsolutePathBuf::current_dir().expect("current dir").into(),
reason: None,
network_approval_context: None,
proposed_execpolicy_amendment: None,
@@ -30,16 +30,7 @@ impl ChatWidget {
self.on_elicitation_request(request_id, params);
}
ServerRequest::PermissionsRequestApproval { params, .. } => {
// TODO(anp): Remove this native-path localization error path once core permission
// paths remain PathUri after crossing the app-server boundary.
match request_permissions_from_params(params) {
Ok(event) => self.on_request_permissions(event),
Err(err) => {
self.add_error_message(format!(
"failed to localize requested filesystem paths: {err}"
));
}
}
self.on_request_permissions(request_permissions_from_params(params));
}
ServerRequest::ToolRequestUserInput { params, .. } => {
self.on_request_user_input(params);