Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 83 additions & 23 deletions codex-rs/tui/src/additional_dirs.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::models::PermissionProfile;
use std::path::PathBuf;

/// Returns a warning describing why `--add-dir` entries will be ignored for the
/// resolved sandbox policy. The caller is responsible for presenting the
/// resolved permission profile. The caller is responsible for presenting the
/// warning to the user (for example, printing to stderr).
pub fn add_dir_warning_message(
additional_dirs: &[PathBuf],
sandbox_policy: &SandboxPolicy,
permission_profile: &PermissionProfile,
cwd: &std::path::Path,
) -> Option<String> {
if additional_dirs.is_empty() {
return None;
}

match sandbox_policy {
SandboxPolicy::WorkspaceWrite { .. }
| SandboxPolicy::DangerFullAccess
| SandboxPolicy::ExternalSandbox { .. } => None,
SandboxPolicy::ReadOnly { .. } => Some(format_warning(additional_dirs)),
if matches!(
permission_profile,
PermissionProfile::Disabled | PermissionProfile::External { .. }
) {
return None;
}

let file_system_policy = permission_profile.file_system_sandbox_policy();
if file_system_policy.has_full_disk_write_access() {
return None;
}

if file_system_policy.can_write_path_with_cwd(cwd, cwd) {
return None;
}

Some(format_warning(additional_dirs))
}

fn format_warning(additional_dirs: &[PathBuf]) -> String {
Expand All @@ -27,57 +39,105 @@ fn format_warning(additional_dirs: &[PathBuf]) -> String {
.collect::<Vec<_>>()
.join(", ");
format!(
"Ignoring --add-dir ({joined_paths}) because the effective sandbox mode is read-only. Switch to workspace-write or danger-full-access to allow additional writable roots."
"Ignoring --add-dir ({joined_paths}) because the effective permissions do not allow additional writable roots. Switch to workspace-write or danger-full-access to allow them."
)
}

#[cfg(test)]
mod tests {
use super::add_dir_warning_message;
use codex_protocol::protocol::NetworkAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::models::ManagedFileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::protocol::NetworkSandboxPolicy;
use pretty_assertions::assert_eq;
use std::path::Path;
use std::path::PathBuf;

#[test]
fn returns_none_for_workspace_write() {
let sandbox = SandboxPolicy::new_workspace_write_policy();
let profile = PermissionProfile::workspace_write();
let dirs = vec![PathBuf::from("/tmp/example")];
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
assert_eq!(
add_dir_warning_message(&dirs, &profile, Path::new("/tmp/project")),
None
);
}

#[test]
fn returns_none_for_danger_full_access() {
let sandbox = SandboxPolicy::DangerFullAccess;
let profile = PermissionProfile::Disabled;
let dirs = vec![PathBuf::from("/tmp/example")];
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
assert_eq!(
add_dir_warning_message(&dirs, &profile, Path::new("/tmp/project")),
None
);
}

#[test]
fn returns_none_for_external_sandbox() {
let sandbox = SandboxPolicy::ExternalSandbox {
network_access: NetworkAccess::Enabled,
let profile = PermissionProfile::External {
network: NetworkSandboxPolicy::Enabled,
};
let dirs = vec![PathBuf::from("/tmp/example")];
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
assert_eq!(
add_dir_warning_message(&dirs, &profile, Path::new("/tmp/project")),
None
);
}

#[test]
fn warns_for_read_only() {
let sandbox = SandboxPolicy::new_read_only_policy();
let profile = PermissionProfile::read_only();
let dirs = vec![PathBuf::from("relative"), PathBuf::from("/abs")];
let message = add_dir_warning_message(&dirs, &sandbox)
let message = add_dir_warning_message(&dirs, &profile, Path::new("/tmp/project"))
.expect("expected warning for read-only sandbox");
assert_eq!(
message,
"Ignoring --add-dir (relative, /abs) because the effective sandbox mode is read-only. Switch to workspace-write or danger-full-access to allow additional writable roots."
"Ignoring --add-dir (relative, /abs) because the effective permissions do not allow additional writable roots. Switch to workspace-write or danger-full-access to allow them."
);
}

#[test]
fn warns_when_profile_can_write_elsewhere_but_not_cwd() {
let profile = PermissionProfile::Managed {
file_system: ManagedFileSystemPermissions::Restricted {
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
access: FileSystemAccessMode::Read,
},
FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: "/tmp/writable".try_into().expect("absolute path"),
},
access: FileSystemAccessMode::Write,
},
],
glob_scan_max_depth: None,
},
network: NetworkSandboxPolicy::Restricted,
};
let dirs = vec![PathBuf::from("/tmp/extra")];

assert_eq!(
add_dir_warning_message(&dirs, &profile, Path::new("/tmp/project")),
Some("Ignoring --add-dir (/tmp/extra) because the effective permissions do not allow additional writable roots. Switch to workspace-write or danger-full-access to allow them.".to_string())
);
}

#[test]
fn returns_none_when_no_additional_dirs() {
let sandbox = SandboxPolicy::new_read_only_policy();
let profile = PermissionProfile::read_only();
let dirs: Vec<PathBuf> = Vec::new();
assert_eq!(add_dir_warning_message(&dirs, &sandbox), None);
assert_eq!(
add_dir_warning_message(&dirs, &profile, Path::new("/tmp/project")),
None
);
}
}
44 changes: 27 additions & 17 deletions codex-rs/tui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,21 @@ use codex_protocol::approvals::ExecApprovalRequestEvent;
use codex_protocol::config_types::Personality;
#[cfg(target_os = "windows")]
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ModelAvailabilityNux;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ModelUpgrade;
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
use codex_protocol::protocol::AskForApproval;
#[cfg(target_os = "windows")]
use codex_protocol::protocol::FileSystemSandboxKind;
use codex_protocol::protocol::FinalOutput;
use codex_protocol::protocol::GetHistoryEntryResponseEvent;
use codex_protocol::protocol::ListSkillsResponseEvent;
#[cfg(test)]
use codex_protocol::protocol::McpAuthStatus;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RateLimitSnapshot;
use codex_protocol::protocol::SandboxPolicy;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SkillErrorInfo;
use codex_protocol::protocol::TokenUsage;
Expand Down Expand Up @@ -308,7 +310,7 @@ fn default_exec_approval_decisions(
struct AutoReviewMode {
approval_policy: AskForApproval,
approvals_reviewer: ApprovalsReviewer,
sandbox_policy: SandboxPolicy,
permission_profile: PermissionProfile,
}

/// Enabling the Auto-review experiment in the TUI should also switch the
Expand All @@ -319,9 +321,18 @@ fn auto_review_mode() -> AutoReviewMode {
AutoReviewMode {
approval_policy: AskForApproval::OnRequest,
approvals_reviewer: ApprovalsReviewer::AutoReview,
sandbox_policy: SandboxPolicy::new_workspace_write_policy(),
permission_profile: PermissionProfile::workspace_write(),
}
}

#[cfg(target_os = "windows")]
fn managed_filesystem_sandbox_is_restricted(permission_profile: &PermissionProfile) -> bool {
matches!(
permission_profile.file_system_sandbox_policy().kind,
FileSystemSandboxKind::Restricted
)
}

/// Baseline cadence for periodic stream commit animation ticks.
///
/// Smooth-mode streaming drains one line per tick, so this interval controls
Expand Down Expand Up @@ -508,7 +519,7 @@ pub(crate) struct App {
cli_kv_overrides: Vec<(String, TomlValue)>,
harness_overrides: ConfigOverrides,
runtime_approval_policy_override: Option<AskForApproval>,
runtime_sandbox_policy_override: Option<SandboxPolicy>,
runtime_permission_profile_override: Option<PermissionProfile>,

pub(crate) file_search: FileSearchManager,

Expand Down Expand Up @@ -906,7 +917,7 @@ See the Codex keymap documentation for supported actions and examples."
cli_kv_overrides,
harness_overrides,
runtime_approval_policy_override: None,
runtime_sandbox_policy_override: None,
runtime_permission_profile_override: None,
file_search,
enhanced_keys_supported,
keymap: runtime_keymap,
Expand Down Expand Up @@ -947,20 +958,14 @@ See the Codex keymap documentation for supported actions and examples."
.await?;
}

// On startup, if Agent mode (workspace-write) or ReadOnly is active, warn about world-writable dirs on Windows.
// On startup, if a managed filesystem sandbox is active, warn about
// world-writable dirs on Windows.
#[cfg(target_os = "windows")]
{
let startup_sandbox_policy = app
.config
.permissions
.legacy_sandbox_policy(app.config.cwd.as_path());
let startup_permission_profile = app.config.permissions.permission_profile();
let should_check = WindowsSandboxLevel::from_config(&app.config)
!= WindowsSandboxLevel::Disabled
&& matches!(
&startup_sandbox_policy,
codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { .. }
| codex_protocol::protocol::SandboxPolicy::ReadOnly { .. }
)
&& managed_filesystem_sandbox_is_restricted(&startup_permission_profile)
&& !app
.config
.notices
Expand All @@ -971,8 +976,13 @@ See the Codex keymap documentation for supported actions and examples."
let env_map: std::collections::HashMap<String, String> = std::env::vars().collect();
let tx = app.app_event_tx.clone();
let logs_base_dir = app.config.codex_home.clone();
let sandbox_policy = startup_sandbox_policy;
Self::spawn_world_writable_scan(cwd, env_map, logs_base_dir, sandbox_policy, tx);
Self::spawn_world_writable_scan(
cwd,
env_map,
logs_base_dir,
startup_permission_profile,
tx,
);
}
}

Expand Down
Loading
Loading