mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
app-server-protocol: remove PermissionProfile from API (#22924)
## Why The app server API should expose permission profile identity, not the lower-level runtime permission model. `PermissionProfile` is the compiled sandbox/network representation that the server uses internally; exposing it through app-server-protocol forces clients to understand details that should remain implementation-level. The API boundary should prefer `ActivePermissionProfile`: a stable profile id, plus future parent-profile metadata, that clients can pass back when they want to select the same active permissions. This also avoids schema generation collisions between the app-server v2 API type space and the core protocol model. Incidentally, while PR makes a number of changes to `command/exec`, note that we are hoping to deprecate this API in favor of `process/spawn`, so we don't need to be too finicky about these changes. ## What Changed - Removed `PermissionProfile` from the app-server-protocol API surface, including generated schema and TypeScript exports. - Changed `CommandExecParams.permissionProfile` to `ActivePermissionProfile`. - Resolve command exec profile ids through `ConfigManager` for the command cwd, matching turn override selection semantics. - Updated downstream TUI tests/helpers to use core permission types directly instead of app-server-protocol `PermissionProfile` shims.
This commit is contained in:
@@ -354,6 +354,7 @@ impl MessageProcessor {
|
||||
arg0_paths.clone(),
|
||||
Arc::clone(&config),
|
||||
outgoing.clone(),
|
||||
config_manager.clone(),
|
||||
);
|
||||
let process_exec_processor = ProcessExecRequestProcessor::new(outgoing.clone());
|
||||
let feedback_processor = FeedbackRequestProcessor::new(
|
||||
|
||||
@@ -356,6 +356,7 @@ use codex_protocol::error::Result as CodexResult;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::items::TurnItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
#[cfg(test)]
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::protocol::AgentStatus;
|
||||
use codex_protocol::protocol::ConversationAudioParams;
|
||||
|
||||
@@ -5,6 +5,7 @@ pub(crate) struct CommandExecRequestProcessor {
|
||||
arg0_paths: Arg0DispatchPaths,
|
||||
config: Arc<Config>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
config_manager: ConfigManager,
|
||||
command_exec_manager: CommandExecManager,
|
||||
}
|
||||
|
||||
@@ -13,11 +14,13 @@ impl CommandExecRequestProcessor {
|
||||
arg0_paths: Arg0DispatchPaths,
|
||||
config: Arc<Config>,
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
config_manager: ConfigManager,
|
||||
) -> Self {
|
||||
Self {
|
||||
arg0_paths,
|
||||
config,
|
||||
outgoing,
|
||||
config_manager,
|
||||
command_exec_manager: CommandExecManager::default(),
|
||||
}
|
||||
}
|
||||
@@ -114,6 +117,13 @@ impl CommandExecRequestProcessor {
|
||||
"`permissionProfile` cannot be combined with `sandboxPolicy`",
|
||||
));
|
||||
}
|
||||
let permission_profile = if let Some(active_permission_profile) = permission_profile {
|
||||
Some(PermissionProfileSelectionParams::new(
|
||||
active_permission_profile.id,
|
||||
))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
if size.is_some() && !tty {
|
||||
return Err(invalid_params("command/exec size requires tty: true"));
|
||||
@@ -159,28 +169,6 @@ impl CommandExecRequestProcessor {
|
||||
},
|
||||
None => None,
|
||||
};
|
||||
let managed_network_requirements_enabled =
|
||||
self.config.managed_network_requirements_enabled();
|
||||
let started_network_proxy = match self.config.permissions.network.as_ref() {
|
||||
Some(spec) => match spec
|
||||
.start_proxy(
|
||||
self.config.permissions.permission_profile(),
|
||||
/*policy_decider*/ None,
|
||||
/*blocked_request_observer*/ None,
|
||||
managed_network_requirements_enabled,
|
||||
NetworkProxyAuditMetadata::default(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(started) => Some(started),
|
||||
Err(err) => {
|
||||
return Err(internal_error(format!(
|
||||
"failed to start managed network proxy: {err}"
|
||||
)));
|
||||
}
|
||||
},
|
||||
None => None,
|
||||
};
|
||||
let windows_sandbox_level = WindowsSandboxLevel::from_config(&self.config);
|
||||
let output_bytes_cap = if disable_output_cap {
|
||||
None
|
||||
@@ -205,47 +193,42 @@ impl CommandExecRequestProcessor {
|
||||
} else {
|
||||
self.config.cwd.clone()
|
||||
};
|
||||
let exec_params = ExecParams {
|
||||
command,
|
||||
cwd: cwd.clone(),
|
||||
expiration,
|
||||
capture_policy,
|
||||
env,
|
||||
network: started_network_proxy
|
||||
.as_ref()
|
||||
.map(codex_core::config::StartedNetworkProxy::proxy),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: self
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
let effective_permission_profile = if let Some(permission_profile) = permission_profile {
|
||||
let permission_profile =
|
||||
codex_protocol::models::PermissionProfile::from(permission_profile);
|
||||
let (mut file_system_sandbox_policy, network_sandbox_policy) =
|
||||
permission_profile.to_runtime_permissions();
|
||||
let configured_file_system_sandbox_policy =
|
||||
self.config.permissions.file_system_sandbox_policy();
|
||||
Self::preserve_configured_deny_read_restrictions(
|
||||
&mut file_system_sandbox_policy,
|
||||
&configured_file_system_sandbox_policy,
|
||||
let (
|
||||
effective_permission_profile,
|
||||
network_proxy_spec,
|
||||
network_proxy_permission_profile,
|
||||
managed_network_requirements_enabled,
|
||||
) = if let Some(permission_profile) = permission_profile {
|
||||
let mut overrides = ConfigOverrides {
|
||||
cwd: Some(cwd.to_path_buf()),
|
||||
..Default::default()
|
||||
};
|
||||
apply_permission_profile_selection_to_config_overrides(
|
||||
&mut overrides,
|
||||
Some(permission_profile),
|
||||
);
|
||||
let effective_permission_profile =
|
||||
codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
permission_profile.enforcement(),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
self.config
|
||||
.permissions
|
||||
.can_set_permission_profile(&effective_permission_profile)
|
||||
let config = self
|
||||
.config_manager
|
||||
.load_for_cwd(
|
||||
/*request_overrides*/ None,
|
||||
overrides,
|
||||
Some(self.config.cwd.to_path_buf()),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| invalid_request(format!("invalid permission profile: {err}")))?;
|
||||
effective_permission_profile
|
||||
if let Some(warning) = config.startup_warnings.iter().find(|warning| {
|
||||
warning.contains("Configured value for `permission_profile` is disallowed")
|
||||
}) {
|
||||
return Err(invalid_request(format!(
|
||||
"invalid permission profile: {warning}"
|
||||
)));
|
||||
}
|
||||
(
|
||||
config.permissions.effective_permission_profile(),
|
||||
config.permissions.network.clone(),
|
||||
config.permissions.permission_profile().clone(),
|
||||
config.managed_network_requirements_enabled(),
|
||||
)
|
||||
} else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) {
|
||||
self.config
|
||||
.permissions
|
||||
@@ -265,9 +248,57 @@ impl CommandExecRequestProcessor {
|
||||
.permissions
|
||||
.can_set_permission_profile(&permission_profile)
|
||||
.map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?;
|
||||
permission_profile
|
||||
(
|
||||
permission_profile,
|
||||
self.config.permissions.network.clone(),
|
||||
self.config.permissions.permission_profile().clone(),
|
||||
self.config.managed_network_requirements_enabled(),
|
||||
)
|
||||
} else {
|
||||
self.config.permissions.effective_permission_profile()
|
||||
(
|
||||
self.config.permissions.effective_permission_profile(),
|
||||
self.config.permissions.network.clone(),
|
||||
self.config.permissions.permission_profile().clone(),
|
||||
self.config.managed_network_requirements_enabled(),
|
||||
)
|
||||
};
|
||||
let started_network_proxy = match network_proxy_spec.as_ref() {
|
||||
Some(spec) => match spec
|
||||
.start_proxy(
|
||||
&network_proxy_permission_profile,
|
||||
/*policy_decider*/ None,
|
||||
/*blocked_request_observer*/ None,
|
||||
managed_network_requirements_enabled,
|
||||
NetworkProxyAuditMetadata::default(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(started) => Some(started),
|
||||
Err(err) => {
|
||||
return Err(internal_error(format!(
|
||||
"failed to start managed network proxy: {err}"
|
||||
)));
|
||||
}
|
||||
},
|
||||
None => None,
|
||||
};
|
||||
let exec_params = ExecParams {
|
||||
command,
|
||||
cwd: cwd.clone(),
|
||||
expiration,
|
||||
capture_policy,
|
||||
env,
|
||||
network: started_network_proxy
|
||||
.as_ref()
|
||||
.map(codex_core::config::StartedNetworkProxy::proxy),
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
windows_sandbox_level,
|
||||
windows_sandbox_private_desktop: self
|
||||
.config
|
||||
.permissions
|
||||
.windows_sandbox_private_desktop,
|
||||
justification: None,
|
||||
arg0: None,
|
||||
};
|
||||
|
||||
let codex_linux_sandbox_exe = self.arg0_paths.codex_linux_sandbox_exe.clone();
|
||||
@@ -304,16 +335,4 @@ impl CommandExecRequestProcessor {
|
||||
})
|
||||
.await
|
||||
}
|
||||
|
||||
fn preserve_configured_deny_read_restrictions(
|
||||
file_system_sandbox_policy: &mut FileSystemSandboxPolicy,
|
||||
configured_file_system_sandbox_policy: &FileSystemSandboxPolicy,
|
||||
) {
|
||||
file_system_sandbox_policy
|
||||
.preserve_deny_read_restrictions_from(configured_file_system_sandbox_policy);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "command_exec_processor_tests.rs"]
|
||||
mod command_exec_processor_tests;
|
||||
|
||||
@@ -1,38 +0,0 @@
|
||||
use super::*;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use codex_utils_absolute_path::test_support::test_path_buf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn command_profile_preserves_configured_deny_read_restrictions() {
|
||||
let readable_entry = FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: test_path_buf("/tmp/project").abs(),
|
||||
},
|
||||
access: FileSystemAccessMode::Read,
|
||||
};
|
||||
let deny_entry = FileSystemSandboxEntry {
|
||||
path: FileSystemPath::GlobPattern {
|
||||
pattern: "/tmp/project/**/*.env".to_string(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
};
|
||||
let mut file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::restricted(vec![readable_entry.clone()]);
|
||||
let mut configured_file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::restricted(vec![deny_entry.clone()]);
|
||||
configured_file_system_sandbox_policy.glob_scan_max_depth = Some(2);
|
||||
|
||||
CommandExecRequestProcessor::preserve_configured_deny_read_restrictions(
|
||||
&mut file_system_sandbox_policy,
|
||||
&configured_file_system_sandbox_policy,
|
||||
);
|
||||
|
||||
let mut expected = FileSystemSandboxPolicy::restricted(vec![readable_entry, deny_entry]);
|
||||
expected.glob_scan_max_depth = Some(2);
|
||||
assert_eq!(file_system_sandbox_policy, expected);
|
||||
}
|
||||
Reference in New Issue
Block a user