mirror of
https://github.com/openai/codex.git
synced 2026-11-07 20:30:33 +00:00
Review terminal input against retained permissions (#41328)
## Why An existing terminal keeps the sandbox, network settings, and grants it had at launch. Later `write_stdin` calls must account for that retained authority, including when permissions change between turns. ## What changed - Capture each terminal's effective launch permissions and compare them with the current environment before accepting non-empty input. - Request approval when the terminal bypassed the sandbox, retains additional grants, strict auto-review is active, or its sandbox or network settings differ from the current policy. - Reject input when current denied-read or environment-owned network restrictions cannot be enforced by the existing terminal, while keeping empty polls and non-TTY interrupts available without review. - Include the effective sandbox mode and user-visible additional grants in `write_stdin` approval actions without exposing host-owned grant paths. ## Testing Added unit and end-to-end coverage for retained cross-turn grants, strict auto-review, sandbox and network policy changes, denied reads, Windows sandbox backends, and private grant redaction. GitOrigin-RevId: e2d71daf9358474c885998786c4928069100711c
This commit is contained in:
@@ -53,6 +53,7 @@ mod process;
|
||||
mod process_manager;
|
||||
mod process_state;
|
||||
mod shell_snapshot;
|
||||
mod stdin_approval;
|
||||
|
||||
pub(crate) fn set_deterministic_process_ids_for_tests(enabled: bool) {
|
||||
process_manager::set_deterministic_process_ids_for_tests(enabled);
|
||||
@@ -64,6 +65,8 @@ pub(crate) use process::NoopSpawnLifecycle;
|
||||
pub(crate) use process::SpawnLifecycle;
|
||||
pub(crate) use process::SpawnLifecycleHandle;
|
||||
pub(crate) use process::UnifiedExecProcess;
|
||||
pub(crate) use stdin_approval::TerminalPermissions;
|
||||
pub(crate) use stdin_approval::TerminalSandboxSource;
|
||||
|
||||
pub(crate) const MIN_YIELD_TIME_MS: u64 = 250;
|
||||
pub(crate) const WINDOWS_INITIAL_EXEC_YIELD_TIME_FLOOR_MS: u64 = 10_000;
|
||||
@@ -185,9 +188,7 @@ struct ProcessEntry {
|
||||
hook_command: String,
|
||||
tty: bool,
|
||||
environment_id: String,
|
||||
// The successful launch bypassed sandboxing required by its ambient policy.
|
||||
// Preserve this across turns so subsequent stdin writes can require approval.
|
||||
escalated: bool,
|
||||
permissions: TerminalPermissions,
|
||||
network_approval: Option<DeferredNetworkApproval>,
|
||||
session: Weak<Session>,
|
||||
last_used: tokio::time::Instant,
|
||||
|
||||
@@ -142,7 +142,14 @@ async fn exec_command_with_tty(
|
||||
hook_command: cmd.to_string(),
|
||||
tty,
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
escalated: false,
|
||||
permissions: TerminalPermissions::for_launch(
|
||||
turn.environments.primary().expect("turn environment"),
|
||||
turn,
|
||||
TerminalSandboxSource::Native,
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
),
|
||||
network_approval: None,
|
||||
session: Arc::downgrade(session),
|
||||
last_used: started_at,
|
||||
@@ -609,7 +616,14 @@ async fn terminating_initial_exec_command_rechecks_initial_response_state() -> a
|
||||
hook_command: "sleep 60".to_string(),
|
||||
tty: true,
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
escalated: false,
|
||||
permissions: TerminalPermissions::for_launch(
|
||||
turn.environments.primary().expect("turn environment"),
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
),
|
||||
network_approval: None,
|
||||
session: Arc::downgrade(&session),
|
||||
last_used: Instant::now(),
|
||||
@@ -685,7 +699,14 @@ async fn terminating_during_stdin_poll_returns_exited_response() -> anyhow::Resu
|
||||
hook_command: "sleep 60".to_string(),
|
||||
tty: true,
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
escalated: false,
|
||||
permissions: TerminalPermissions::for_launch(
|
||||
turn.environments.primary().expect("turn environment"),
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
),
|
||||
network_approval: None,
|
||||
session: Arc::downgrade(&session),
|
||||
last_used,
|
||||
@@ -899,11 +920,32 @@ async fn stdin_approval_preserves_the_reviewed_terminal() -> anyhow::Result<()>
|
||||
let original = {
|
||||
let mut store = manager.process_store.lock().await;
|
||||
let entry = store.processes.get_mut(&process_id).unwrap();
|
||||
entry.escalated = true;
|
||||
entry.permissions = TerminalPermissions::for_launch(
|
||||
turn.environments.primary().expect("turn environment"),
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
SandboxPermissions::RequireEscalated,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
);
|
||||
entry.environment_id = "unselected-executor".to_string();
|
||||
entry.cwd = cwd.clone();
|
||||
Arc::clone(&entry.process)
|
||||
};
|
||||
// A queued write must acquire the terminal lock before reading active strict
|
||||
// mode: code-mode calls can enable it while another interaction is draining.
|
||||
{
|
||||
let interaction = original.interaction_lock().lock_owned().await;
|
||||
let _active_turn = session.active_turn.lock().await;
|
||||
let mut queued = Box::pin(write_stdin(
|
||||
&session, &turn, process_id, "queued\n", /*yield_time_ms*/ 250,
|
||||
));
|
||||
let mut task_context = std::task::Context::from_waker(futures::task::noop_waker_ref());
|
||||
assert!(queued.as_mut().poll(&mut task_context).is_pending());
|
||||
drop(interaction);
|
||||
assert!(queued.as_mut().poll(&mut task_context).is_pending());
|
||||
assert!(original.interaction_lock().try_lock_owned().is_err());
|
||||
}
|
||||
// Empty polling must complete without an approval response.
|
||||
tokio::time::timeout(
|
||||
Duration::from_secs(/*secs*/ 5),
|
||||
@@ -921,6 +963,25 @@ async fn stdin_approval_preserves_the_reviewed_terminal() -> anyhow::Result<()>
|
||||
);
|
||||
Arc::make_mut(&mut Arc::get_mut(&mut turn).unwrap().config).approvals_reviewer =
|
||||
ApprovalsReviewer::User;
|
||||
assert!(matches!(
|
||||
write_stdin(&session, &turn, process_id, input, /*yield_time_ms*/ 250).await,
|
||||
Err(UnifiedExecError::StdinApproval(ToolError::Rejected(reason)))
|
||||
if reason.contains("select it before retrying")
|
||||
));
|
||||
manager
|
||||
.process_store
|
||||
.lock()
|
||||
.await
|
||||
.processes
|
||||
.get_mut(&process_id)
|
||||
.unwrap()
|
||||
.environment_id = turn
|
||||
.environments
|
||||
.primary()
|
||||
.unwrap()
|
||||
.selection
|
||||
.environment_id
|
||||
.clone();
|
||||
for (input, decision) in [
|
||||
("rejected\n", ReviewDecision::denied("test denial")),
|
||||
("accepted\n", ReviewDecision::Approved),
|
||||
|
||||
@@ -37,7 +37,6 @@ use crate::tools::runtimes::is_managed_proxy_env_var;
|
||||
use crate::tools::runtimes::unified_exec::UnifiedExecAttempt;
|
||||
use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest;
|
||||
use crate::tools::runtimes::unified_exec::UnifiedExecRuntime;
|
||||
use crate::tools::sandboxing::ApprovalAction;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
@@ -69,7 +68,6 @@ use codex_core_plugins::PLUGIN_METRICS_OUTPUT_ENV_VAR;
|
||||
use codex_core_plugins::PluginCommandAttribution;
|
||||
use codex_core_plugins::PluginMetricsSidecar;
|
||||
use codex_core_plugins::strip_output_env;
|
||||
use codex_features::Feature;
|
||||
use codex_network_proxy::NetworkPolicyDecider;
|
||||
use codex_network_proxy::NetworkProxy;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicy;
|
||||
@@ -489,7 +487,7 @@ impl UnifiedExecProcessManager {
|
||||
let UnifiedExecAttempt {
|
||||
process,
|
||||
metrics_sidecar,
|
||||
escalated,
|
||||
permissions,
|
||||
} = attempt;
|
||||
let process = Arc::new(process);
|
||||
let network_denial_monitor = deferred_network_approval.as_ref().map(|deferred| {
|
||||
@@ -549,7 +547,7 @@ impl UnifiedExecProcessManager {
|
||||
request.hook_command.clone(),
|
||||
cwd.clone(),
|
||||
request.turn_environment.selection.environment_id.clone(),
|
||||
escalated,
|
||||
permissions,
|
||||
plugin_attribution.clone(),
|
||||
start,
|
||||
request.process_id,
|
||||
@@ -772,33 +770,31 @@ impl UnifiedExecProcessManager {
|
||||
// Different terminal sessions can be polled concurrently, but reads and
|
||||
// writes against one terminal must not overlap because they share a
|
||||
// draining output buffer and process lifecycle.
|
||||
let (locked_process, approval) = {
|
||||
let locked_process = {
|
||||
let store = self.process_store.lock().await;
|
||||
let entry = store
|
||||
.processes
|
||||
.get(&process_id)
|
||||
.ok_or(UnifiedExecError::UnknownProcessId { process_id })?;
|
||||
let approval = (!request.input.is_empty()
|
||||
&& (entry.tty || request.input != INTERRUPT)
|
||||
&& entry.escalated
|
||||
&& context
|
||||
.session
|
||||
.features()
|
||||
.enabled(Feature::WriteStdinApproval))
|
||||
.then(|| ApprovalAction::WriteStdin {
|
||||
id: entry.call_id.clone(),
|
||||
approval_id: context.call_id.clone(),
|
||||
environment_id: entry.environment_id.clone(),
|
||||
process_id,
|
||||
input: request.input.to_string(),
|
||||
cwd: entry.cwd.clone(),
|
||||
tty: entry.tty,
|
||||
});
|
||||
(Arc::clone(&entry.process), approval)
|
||||
Arc::clone(&entry.process)
|
||||
};
|
||||
let _interaction_guard = locked_process.interaction_lock().lock_owned().await;
|
||||
if let Some(approval) = approval {
|
||||
let approval_reason = "Send input to an existing escalated terminal. The cwd is its launch directory; the terminal's current directory and state may have changed.".to_string();
|
||||
// A queued write must observe strict review enabled while it was waiting.
|
||||
let strict_auto_review = context
|
||||
.session
|
||||
.active_turn_context_and_strict_auto_review()
|
||||
.await
|
||||
.is_some_and(|(_, _, strict)| strict);
|
||||
let approval = {
|
||||
let store = self.process_store.lock().await;
|
||||
let entry = store
|
||||
.processes
|
||||
.get(&process_id)
|
||||
.filter(|entry| Arc::ptr_eq(&entry.process, &locked_process))
|
||||
.ok_or(UnifiedExecError::UnknownProcessId { process_id })?;
|
||||
entry.stdin_approval(context, request.input, strict_auto_review)?
|
||||
};
|
||||
if let Some((approval, approval_reason)) = approval {
|
||||
let reviewed = crate::guardian::format_guardian_action_pretty(
|
||||
&approval.clone().into_guardian_request().map_err(|err| {
|
||||
UnifiedExecError::StdinApproval(ToolError::Rejected(err.to_string()))
|
||||
@@ -830,11 +826,6 @@ impl UnifiedExecProcessManager {
|
||||
"terminal input and permission details are too large to review safely; use a smaller input or start a new terminal with fewer grants".to_string(),
|
||||
)));
|
||||
}
|
||||
let strict_auto_review = context
|
||||
.session
|
||||
.active_turn_context_and_strict_auto_review()
|
||||
.await
|
||||
.is_some_and(|(_, _, strict)| strict);
|
||||
let approval_context = ApprovalContext {
|
||||
review_context: GuardianReviewContext::from(&context.step_context),
|
||||
cancellation_token: Some(context.cancellation_token.clone()),
|
||||
@@ -1083,7 +1074,7 @@ impl UnifiedExecProcessManager {
|
||||
hook_command: String,
|
||||
cwd: PathUri,
|
||||
environment_id: String,
|
||||
escalated: bool,
|
||||
permissions: super::TerminalPermissions,
|
||||
plugin_attribution: Option<PluginCommandAttribution>,
|
||||
started_at: Instant,
|
||||
process_id: i32,
|
||||
@@ -1106,7 +1097,7 @@ impl UnifiedExecProcessManager {
|
||||
hook_command,
|
||||
tty,
|
||||
environment_id,
|
||||
escalated,
|
||||
permissions,
|
||||
network_approval,
|
||||
session: Arc::downgrade(&context.session),
|
||||
last_used: started_at,
|
||||
|
||||
@@ -545,6 +545,7 @@ fn pruning_protects_recent_processes_even_if_exited() {
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn pruning_does_not_evict_live_process_while_exited_process_is_finalizing() {
|
||||
let (_, turn) = crate::session::tests::make_session_and_context().await;
|
||||
let exited_process = Arc::new(
|
||||
crate::unified_exec::process_tests::remote_process(
|
||||
codex_exec_server::WriteStatus::Accepted,
|
||||
@@ -590,7 +591,14 @@ async fn pruning_does_not_evict_live_process_while_exited_process_is_finalizing(
|
||||
hook_command: format!("command-{process_id}"),
|
||||
tty: false,
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
escalated: false,
|
||||
permissions: super::super::TerminalPermissions::for_launch(
|
||||
turn.environments.primary().expect("turn environment"),
|
||||
&turn,
|
||||
super::super::TerminalSandboxSource::Native,
|
||||
crate::sandboxing::SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
),
|
||||
network_approval: None,
|
||||
session: std::sync::Weak::new(),
|
||||
last_used: if is_exited {
|
||||
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: core/src/unified_exec/stdin_approval_tests.rs
|
||||
expression: reason
|
||||
---
|
||||
Send input to an existing terminal. This terminal retains additional permissions. It also has an internal plugin metrics write grant. The cwd is its launch directory; the terminal's current directory and state may have changed.
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: core/src/unified_exec/stdin_approval_tests.rs
|
||||
expression: reason
|
||||
---
|
||||
Send input to an existing terminal. This terminal retains sandbox or network settings that differ from the current permissions. The cwd is its launch directory; the terminal's current directory and state may have changed.
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: core/src/unified_exec/stdin_approval_tests.rs
|
||||
expression: reason
|
||||
---
|
||||
Send input to an existing terminal. This terminal was launched outside the sandbox, bypassing any managed network proxy. The cwd is its launch directory; the terminal's current directory and state may have changed.
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: core/src/unified_exec/stdin_approval_tests.rs
|
||||
expression: permissions.approval_reason(expected)?
|
||||
---
|
||||
Send input to an existing terminal. This terminal runs without a filesystem sandbox. The cwd is its launch directory; the terminal's current directory and state may have changed.
|
||||
@@ -0,0 +1,244 @@
|
||||
//! Reviews terminal input against retained launch permissions and current policy.
|
||||
//! Permission snapshots stay host-owned; neither an earlier approval nor a policy
|
||||
//! change alters the sandbox of an already-running process. Native launches keep
|
||||
//! their configured Windows sandbox level; executor launches use executor defaults.
|
||||
|
||||
use super::ProcessEntry;
|
||||
use super::UnifiedExecContext;
|
||||
use super::UnifiedExecError;
|
||||
use crate::config::NetworkProxySpec;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::session::turn_context::TurnEnvironment;
|
||||
use crate::tools::sandboxing::ApprovalAction;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use codex_features::Feature;
|
||||
use codex_file_system::ExecPermissionProfile;
|
||||
use codex_file_system::FileSystemSandboxContext;
|
||||
use codex_network_proxy::EnvironmentNetworkPolicy;
|
||||
use codex_protocol::models::AdditionalPermissionProfile;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::SandboxPermissions;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_sandboxing::policy_transforms::effective_permission_profile;
|
||||
use codex_sandboxing::policy_transforms::merge_permission_profiles;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub(crate) enum TerminalSandboxSource {
|
||||
Native,
|
||||
Executor,
|
||||
}
|
||||
|
||||
pub(crate) struct TerminalPermissions {
|
||||
policy: TerminalPolicy,
|
||||
sandbox_source: TerminalSandboxSource,
|
||||
launch_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
internal_permissions: Option<AdditionalPermissionProfile>,
|
||||
}
|
||||
|
||||
/// Host-owned launch settings. Never serialize these into approval messages.
|
||||
#[derive(PartialEq, Eq)]
|
||||
struct TerminalPolicy {
|
||||
sandbox: FileSystemSandboxContext,
|
||||
environment_network: Option<EnvironmentNetworkPolicy>,
|
||||
controller_network: Option<NetworkProxySpec>,
|
||||
controller_proxy: bool,
|
||||
}
|
||||
|
||||
impl TerminalPolicy {
|
||||
fn capture(
|
||||
environment: &TurnEnvironment,
|
||||
turn: &TurnContext,
|
||||
sandbox_source: TerminalSandboxSource,
|
||||
additional_permissions: Option<AdditionalPermissionProfile>,
|
||||
) -> Self {
|
||||
let mut sandbox = environment.sandbox_context(additional_permissions);
|
||||
if matches!(sandbox_source, TerminalSandboxSource::Native) {
|
||||
// The filesystem helper applies executor defaults, but native process
|
||||
// launches honor Disabled. Preserve it so later enablement is detected.
|
||||
sandbox.windows_sandbox_level = environment.config().windows_sandbox_level;
|
||||
}
|
||||
Self {
|
||||
sandbox,
|
||||
environment_network: environment.config().network_policy.clone(),
|
||||
controller_network: turn.config.permissions.network.clone(),
|
||||
controller_proxy: turn.network.is_some(),
|
||||
}
|
||||
}
|
||||
|
||||
fn file_system_context(&self) -> FileSystemSandboxContext {
|
||||
let mut context = self.sandbox.clone();
|
||||
// Network changes require review, not rejection for denied-read drift.
|
||||
if let ExecPermissionProfile::Managed { network, .. } = &mut context.permissions {
|
||||
*network = NetworkSandboxPolicy::Restricted;
|
||||
}
|
||||
context
|
||||
}
|
||||
}
|
||||
|
||||
impl TerminalPermissions {
|
||||
pub(crate) fn for_launch(
|
||||
environment: &TurnEnvironment,
|
||||
turn: &TurnContext,
|
||||
sandbox_source: TerminalSandboxSource,
|
||||
launch_permissions: SandboxPermissions,
|
||||
additional_permissions: Option<&AdditionalPermissionProfile>,
|
||||
internal_permissions: Option<&AdditionalPermissionProfile>,
|
||||
) -> Self {
|
||||
Self {
|
||||
policy: TerminalPolicy::capture(
|
||||
environment,
|
||||
turn,
|
||||
sandbox_source,
|
||||
merge_permission_profiles(additional_permissions, internal_permissions),
|
||||
),
|
||||
sandbox_source,
|
||||
// A bypass is a property of the successful attempt, not a difference
|
||||
// between settings: a full-access environment can still bypass a proxy.
|
||||
launch_permissions,
|
||||
additional_permissions: additional_permissions.cloned(),
|
||||
internal_permissions: internal_permissions.cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Compares launch policy with current policy including retained grants,
|
||||
/// without reading turn state or touching a process.
|
||||
fn review_requirement(
|
||||
&self,
|
||||
current: &TerminalPolicy,
|
||||
baseline: &PermissionProfile,
|
||||
) -> Result<SandboxPermissions, &'static str> {
|
||||
let bypassed = self.launch_permissions.requires_escalated_permissions();
|
||||
if current.environment_network.is_some()
|
||||
&& (bypassed || self.policy.environment_network != current.environment_network)
|
||||
{
|
||||
return Err(
|
||||
"this terminal cannot enforce the current environment-owned network restrictions; start a new terminal",
|
||||
);
|
||||
}
|
||||
// Approval cannot retrofit denied reads onto a running process. Unless
|
||||
// its sandbox still matches, start a new terminal under the current policy.
|
||||
if baseline
|
||||
.file_system_sandbox_policy()
|
||||
.has_denied_read_restrictions()
|
||||
&& (bypassed || self.policy.file_system_context() != current.file_system_context())
|
||||
{
|
||||
return Err(
|
||||
"this terminal cannot enforce the current denied-read restrictions; start a new terminal",
|
||||
);
|
||||
}
|
||||
// Once the retained settings match, only the baseline permissions can differ.
|
||||
Ok(if bypassed || &self.policy != current {
|
||||
SandboxPermissions::RequireEscalated
|
||||
} else if self.policy.sandbox.permissions
|
||||
== effective_permission_profile(baseline, /*additional_permissions*/ None).into()
|
||||
{
|
||||
SandboxPermissions::UseDefault
|
||||
} else {
|
||||
SandboxPermissions::WithAdditionalPermissions
|
||||
})
|
||||
}
|
||||
|
||||
fn approval_reason(
|
||||
&self,
|
||||
sandbox_permissions: SandboxPermissions,
|
||||
) -> Result<String, serde_json::Error> {
|
||||
let authority = if self.launch_permissions.requires_escalated_permissions() {
|
||||
"This terminal was launched outside the sandbox, bypassing any managed network proxy."
|
||||
} else if self.policy.sandbox.permissions == ExecPermissionProfile::Disabled {
|
||||
"This terminal runs without a filesystem sandbox."
|
||||
} else {
|
||||
match sandbox_permissions {
|
||||
SandboxPermissions::UseDefault => "This terminal uses the current permissions.",
|
||||
SandboxPermissions::WithAdditionalPermissions => {
|
||||
"This terminal retains additional permissions."
|
||||
}
|
||||
SandboxPermissions::RequireEscalated => {
|
||||
"This terminal retains sandbox or network settings that differ from the current permissions."
|
||||
}
|
||||
}
|
||||
};
|
||||
let mut reason = format!("Send input to an existing terminal. {authority}");
|
||||
if self.internal_permissions.is_some() {
|
||||
reason.push_str(" It also has an internal plugin metrics write grant.");
|
||||
}
|
||||
reason.push_str(" The cwd is its launch directory; the terminal's current directory and state may have changed.");
|
||||
if let Some(grants) = &self.additional_permissions {
|
||||
// Stable reason text also reaches clients that strip the experimental
|
||||
// additionalPermissions field. Internal paths never enter this text.
|
||||
reason.push_str(&format!(
|
||||
" Retained grants: {}.",
|
||||
serde_json::to_string(grants)?
|
||||
));
|
||||
}
|
||||
Ok(reason)
|
||||
}
|
||||
}
|
||||
|
||||
impl ProcessEntry {
|
||||
pub(super) fn stdin_approval(
|
||||
&self,
|
||||
context: &UnifiedExecContext,
|
||||
input: &str,
|
||||
strict_auto_review: bool,
|
||||
) -> Result<Option<(ApprovalAction, String)>, UnifiedExecError> {
|
||||
if input.is_empty()
|
||||
|| (!self.tty && input == "\u{3}")
|
||||
|| !context
|
||||
.session
|
||||
.features()
|
||||
.enabled(Feature::WriteStdinApproval)
|
||||
{
|
||||
return Ok(None);
|
||||
}
|
||||
let environment = context
|
||||
.step_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.find(|environment| environment.selection.environment_id == self.environment_id)
|
||||
.ok_or_else(|| {
|
||||
approval_error(
|
||||
"cannot access the terminal's original environment; select it before retrying",
|
||||
)
|
||||
})?;
|
||||
let permissions = &self.permissions;
|
||||
let current = TerminalPolicy::capture(
|
||||
environment,
|
||||
&context.step_context.turn,
|
||||
permissions.sandbox_source,
|
||||
merge_permission_profiles(
|
||||
permissions.additional_permissions.as_ref(),
|
||||
permissions.internal_permissions.as_ref(),
|
||||
),
|
||||
);
|
||||
let sandbox_permissions = permissions
|
||||
.review_requirement(¤t, environment.permission_profile())
|
||||
.map_err(approval_error)?;
|
||||
if sandbox_permissions == SandboxPermissions::UseDefault && !strict_auto_review {
|
||||
return Ok(None);
|
||||
}
|
||||
let reason = permissions
|
||||
.approval_reason(sandbox_permissions)
|
||||
.map_err(approval_error)?;
|
||||
let action = ApprovalAction::WriteStdin {
|
||||
id: self.call_id.clone(),
|
||||
approval_id: context.call_id.clone(),
|
||||
environment_id: self.environment_id.clone(),
|
||||
process_id: self.process_id,
|
||||
input: input.to_string(),
|
||||
cwd: self.cwd.clone(),
|
||||
tty: self.tty,
|
||||
sandbox_permissions,
|
||||
additional_permissions: permissions.additional_permissions.clone(),
|
||||
};
|
||||
Ok(Some((action, reason)))
|
||||
}
|
||||
}
|
||||
|
||||
fn approval_error(reason: impl std::fmt::Display) -> UnifiedExecError {
|
||||
UnifiedExecError::StdinApproval(ToolError::Rejected(reason.to_string()))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "stdin_approval_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,235 @@
|
||||
//! Direct policy comparisons, launch capture, and private approval text.
|
||||
|
||||
use super::*;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use codex_network_proxy::NetworkMode;
|
||||
use codex_network_proxy::NetworkProxyConfig;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::PermissionProfileSnapshot;
|
||||
use codex_protocol::permissions::FileSystemAccessMode;
|
||||
use codex_protocol::permissions::FileSystemPath;
|
||||
use codex_protocol::permissions::FileSystemSandboxEntry;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
|
||||
fn terminal_permissions(profile: &PermissionProfile) -> TerminalPermissions {
|
||||
TerminalPermissions {
|
||||
policy: TerminalPolicy {
|
||||
sandbox: FileSystemSandboxContext::from_permission_profile(
|
||||
effective_permission_profile(profile, /*additional_permissions*/ None),
|
||||
),
|
||||
environment_network: None,
|
||||
controller_network: None,
|
||||
controller_proxy: false,
|
||||
},
|
||||
sandbox_source: TerminalSandboxSource::Native,
|
||||
launch_permissions: SandboxPermissions::UseDefault,
|
||||
additional_permissions: None,
|
||||
internal_permissions: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reduced_permissions_require_review() -> anyhow::Result<()> {
|
||||
let permissions = terminal_permissions(&PermissionProfile::Disabled);
|
||||
let baseline = PermissionProfile::read_only();
|
||||
let current = terminal_permissions(&baseline);
|
||||
let expected = SandboxPermissions::RequireEscalated;
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t.policy, &baseline),
|
||||
Ok(expected)
|
||||
);
|
||||
insta::assert_snapshot!(
|
||||
"reduced_permissions",
|
||||
permissions.approval_reason(expected)?
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn proxy_bypass_requires_review_even_when_permissions_match() -> anyhow::Result<()> {
|
||||
let baseline = PermissionProfile::Disabled;
|
||||
let mut permissions = terminal_permissions(&baseline);
|
||||
permissions.launch_permissions = SandboxPermissions::RequireEscalated;
|
||||
let expected = SandboxPermissions::RequireEscalated;
|
||||
assert_eq!(
|
||||
permissions.review_requirement(&permissions.policy, &baseline),
|
||||
Ok(expected)
|
||||
);
|
||||
insta::assert_snapshot!("proxy_bypass", permissions.approval_reason(expected)?);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn denied_reads_reject_file_system_changes_but_only_review_network_changes() {
|
||||
let mut file_system = FileSystemSandboxPolicy::read_only();
|
||||
file_system.entries.push(FileSystemSandboxEntry::new(
|
||||
FileSystemPath::GlobPattern {
|
||||
pattern: "**/.env".into(),
|
||||
},
|
||||
FileSystemAccessMode::Deny,
|
||||
));
|
||||
let baseline =
|
||||
PermissionProfile::from_runtime_permissions(&file_system, NetworkSandboxPolicy::Restricted);
|
||||
let permissions = terminal_permissions(&PermissionProfile::Disabled);
|
||||
let current = terminal_permissions(&baseline);
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t.policy, &baseline),
|
||||
Err(
|
||||
"this terminal cannot enforce the current denied-read restrictions; start a new terminal"
|
||||
)
|
||||
);
|
||||
|
||||
let permissions = current;
|
||||
let baseline =
|
||||
PermissionProfile::from_runtime_permissions(&file_system, NetworkSandboxPolicy::Enabled);
|
||||
let current = terminal_permissions(&baseline);
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t.policy, &baseline),
|
||||
Ok(SandboxPermissions::RequireEscalated)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn captured_network_changes_require_review_or_a_new_terminal() -> anyhow::Result<()> {
|
||||
let (_session, mut turn) = make_session_and_context().await;
|
||||
let mut environment = turn.environments.primary().expect("environment").clone();
|
||||
environment.config_mut().permission_profile =
|
||||
PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
|
||||
let mut proxy = NetworkProxyConfig {
|
||||
enabled: true,
|
||||
..Default::default()
|
||||
};
|
||||
Arc::make_mut(&mut turn.config).permissions.network =
|
||||
Some(NetworkProxySpec::from_config_and_constraints(
|
||||
proxy.clone(),
|
||||
/*requirements*/ None,
|
||||
&turn.permission_profile(),
|
||||
)?);
|
||||
let permissions = TerminalPermissions::for_launch(
|
||||
&environment,
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
);
|
||||
|
||||
proxy.mode = NetworkMode::Limited;
|
||||
Arc::make_mut(&mut turn.config).permissions.network =
|
||||
Some(NetworkProxySpec::from_config_and_constraints(
|
||||
proxy.clone(),
|
||||
/*requirements*/ None,
|
||||
&turn.permission_profile(),
|
||||
)?);
|
||||
let current = TerminalPolicy::capture(
|
||||
&environment,
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
/*additional_permissions*/ None,
|
||||
);
|
||||
let expected = SandboxPermissions::RequireEscalated;
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t, environment.permission_profile()),
|
||||
Ok(expected)
|
||||
);
|
||||
insta::assert_snapshot!("network_mode", permissions.approval_reason(expected)?);
|
||||
|
||||
environment.config_mut().network_policy = Some(EnvironmentNetworkPolicy::from_config(
|
||||
&proxy, /*managed_allowed_domains_only*/ false,
|
||||
));
|
||||
let current = TerminalPolicy::capture(
|
||||
&environment,
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
/*additional_permissions*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t, environment.permission_profile()),
|
||||
Err(
|
||||
"this terminal cannot enforce the current environment-owned network restrictions; start a new terminal"
|
||||
)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn internal_grants_require_review_without_exposing_paths() -> anyhow::Result<()> {
|
||||
let (_session, turn) = make_session_and_context().await;
|
||||
let mut environment = turn.environments.primary().expect("environment").clone();
|
||||
environment.config_mut().permission_profile =
|
||||
PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
|
||||
let grants = serde_json::from_value(json!({
|
||||
"file_system": {"write": [turn.config.cwd.join("private-metrics")]}
|
||||
}))?;
|
||||
let permissions = TerminalPermissions::for_launch(
|
||||
&environment,
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
Some(&grants),
|
||||
);
|
||||
let current = TerminalPolicy::capture(
|
||||
&environment,
|
||||
&turn,
|
||||
TerminalSandboxSource::Native,
|
||||
Some(grants),
|
||||
);
|
||||
let expected = SandboxPermissions::WithAdditionalPermissions;
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t, environment.permission_profile()),
|
||||
Ok(expected)
|
||||
);
|
||||
assert_eq!(permissions.additional_permissions, None);
|
||||
insta::assert_snapshot!("internal_grant", permissions.approval_reason(expected)?);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(TerminalSandboxSource::Native, SandboxPermissions::RequireEscalated; "native_disabled_sandbox_needs_review_when_enabled")]
|
||||
#[test_case::test_case(TerminalSandboxSource::Executor, SandboxPermissions::UseDefault; "executor_keeps_its_restricted_token_default")]
|
||||
#[tokio::test]
|
||||
async fn enabling_windows_sandbox_respects_the_launch_backend(
|
||||
source: TerminalSandboxSource,
|
||||
expected: SandboxPermissions,
|
||||
) -> anyhow::Result<()> {
|
||||
let (_session, turn) = make_session_and_context().await;
|
||||
let mut environment = turn.environments.primary().expect("environment").clone();
|
||||
environment.selection.cwd = PathUri::parse("file:///C:/workspace")?;
|
||||
environment.config_mut().permission_profile =
|
||||
PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
|
||||
environment.config_mut().windows_sandbox_level = WindowsSandboxLevel::Disabled;
|
||||
let permissions = TerminalPermissions::for_launch(
|
||||
&environment,
|
||||
&turn,
|
||||
source,
|
||||
SandboxPermissions::UseDefault,
|
||||
/*additional_permissions*/ None,
|
||||
/*internal_permissions*/ None,
|
||||
);
|
||||
let current = TerminalPolicy::capture(
|
||||
&environment,
|
||||
&turn,
|
||||
source,
|
||||
/*additional_permissions*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t, environment.permission_profile()),
|
||||
Ok(SandboxPermissions::UseDefault)
|
||||
);
|
||||
environment.config_mut().windows_sandbox_level = WindowsSandboxLevel::RestrictedToken;
|
||||
let current = TerminalPolicy::capture(
|
||||
&environment,
|
||||
&turn,
|
||||
source,
|
||||
/*additional_permissions*/ None,
|
||||
);
|
||||
assert_eq!(
|
||||
permissions.review_requirement(¤t, environment.permission_profile()),
|
||||
Ok(expected)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user