valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: tui default trusted settings should respect workspace write config (#3341)

Browse Source 
## Summary
When using the trusted state during tui startup, we created a new
WorkspaceWrite policy without checking the config.toml for a
`sandbox_workspace_write` field. This would result in us setting the
sandbox_mode as workspace-write, but ignoring the field if the user had
set `sandbox_workspace_write` without also setting `sandbox_mode` in the
config.toml. This PR adds support for respecting
`sandbox_workspace_write` setting in config.toml in the trusted
directory flow, and adds tests to cover this case.

## Testing
- [x] Added unit tests
This commit is contained in:
Dylan
2025-10-16 11:23:38 -07:00
committed by GitHub
parent 0139f6780c
commit 4b01f0f50a
4 changed files with 203 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/Cargo.lock generated
View File

@@ -1422,6 +1422,7 @@ dependencies = [
"textwrap 0.16.2",
"tokio",
"tokio-stream",
"toml",
"tracing",
"tracing-appender",
"tracing-subscriber",

195
codex-rs/core/src/config.rs
View File

@@ -42,6 +42,7 @@ use codex_protocol::config_types::Verbosity;
use codex_rmcp_client::OAuthCredentialsStoreMode;
use dirs::home_dir;
use serde::Deserialize;
use similar::DiffableStr;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::io::ErrorKind;
@@ -100,6 +101,10 @@ pub struct Config {
pub sandbox_policy: SandboxPolicy,
/// True if the user passed in an override or set a value in config.toml
/// for either of approval_policy or sandbox_mode.
pub did_user_set_custom_approval_policy_or_sandbox_mode: bool,
pub shell_environment_policy: ShellEnvironmentPolicy,
/// When `true`, `AgentReasoning` events emitted by the backend will be
@@ -230,6 +235,10 @@ pub struct Config {
/// The active profile name used to derive this `Config` (if any).
pub active_profile: Option<String>,
/// The currently active project config, resolved by checking if cwd:
/// is (1) part of a git repo, (2) a git worktree, or (3) just using the cwd
pub active_project: ProjectConfig,
/// Tracks whether the Windows onboarding screen has been acknowledged.
pub windows_wsl_setup_acknowledged: bool,
@@ -859,6 +868,15 @@ pub struct ProjectConfig {
pub trust_level: Option<String>,
}
impl ProjectConfig {
pub fn is_trusted(&self) -> bool {
match &self.trust_level {
Some(trust_level) => trust_level == "trusted",
None => false,
}
}
}
#[derive(Deserialize, Debug, Clone, Default, PartialEq)]
pub struct ToolsToml {
#[serde(default, alias = "web_search_request")]
@@ -880,9 +898,23 @@ impl From<ToolsToml> for Tools {
impl ConfigToml {
/// Derive the effective sandbox policy from the configuration.
fn derive_sandbox_policy(&self, sandbox_mode_override: Option<SandboxMode>) -> SandboxPolicy {
fn derive_sandbox_policy(
&self,
sandbox_mode_override: Option<SandboxMode>,
resolved_cwd: &Path,
) -> SandboxPolicy {
let resolved_sandbox_mode = sandbox_mode_override
.or(self.sandbox_mode)
.or_else(|| {
// if no sandbox_mode is set, but user has marked directory as trusted, use WorkspaceWrite
self.get_active_project(resolved_cwd).and_then(|p| {
if p.is_trusted() {
Some(SandboxMode::WorkspaceWrite)
} else {
None
}
})
})
.unwrap_or_default();
match resolved_sandbox_mode {
SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(),
@@ -904,30 +936,26 @@ impl ConfigToml {
}
}
pub fn is_cwd_trusted(&self, resolved_cwd: &Path) -> bool {
/// Resolves the cwd to an existing project, or returns None if ConfigToml
/// does not contain a project corresponding to cwd or a git repo for cwd
pub fn get_active_project(&self, resolved_cwd: &Path) -> Option<ProjectConfig> {
let projects = self.projects.clone().unwrap_or_default();
let is_path_trusted = |path: &Path| {
let path_str = path.to_string_lossy().to_string();
projects
.get(&path_str)
.map(|p| p.trust_level.as_deref() == Some("trusted"))
.unwrap_or(false)
};
// Fast path: exact cwd match
if is_path_trusted(resolved_cwd) {
return true;
if let Some(project_config) = projects.get(&resolved_cwd.to_string_lossy().to_string()) {
return Some(project_config.clone());
}
// If cwd lives inside a git worktree, check whether the root git project
// If cwd lives inside a git repo/worktree, check whether the root git project
// (the primary repository working directory) is trusted. This lets
// worktrees inherit trust from the main project.
if let Some(root_project) = resolve_root_git_project_for_trust(resolved_cwd) {
return is_path_trusted(&root_project);
if let Some(repo_root) = resolve_root_git_project_for_trust(resolved_cwd)
&& let Some(project_config_for_root) =
projects.get(&repo_root.to_string_lossy().to_string_lossy().to_string())
{
return Some(project_config_for_root.clone());
}
false
None
}
pub fn get_config_profile(
@@ -986,7 +1014,7 @@ impl Config {
model,
review_model: override_review_model,
cwd,
approval_policy,
approval_policy: approval_policy_override,
sandbox_mode,
model_provider,
config_profile: config_profile_key,
@@ -1026,7 +1054,47 @@ impl Config {
let features = Features::from_config(&cfg, &config_profile, feature_overrides);
let sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode);
let resolved_cwd = {
use std::env;
match cwd {
None => {
tracing::info!("cwd not set, using current dir");
env::current_dir()?
}
Some(p) if p.is_absolute() => p,
Some(p) => {
// Resolve relative path against the current working directory.
tracing::info!("cwd is relative, resolving against current dir");
let mut current = env::current_dir()?;
current.push(p);
current
}
}
};
let active_project = cfg
.get_active_project(&resolved_cwd)
.unwrap_or(ProjectConfig { trust_level: None });
let sandbox_policy = cfg.derive_sandbox_policy(sandbox_mode, &resolved_cwd);
let mut approval_policy = approval_policy_override
.or(config_profile.approval_policy)
.or(cfg.approval_policy)
.unwrap_or_else(|| {
if active_project.is_trusted() {
// If no explicit approval policy is set, but we trust cwd, default to OnRequest
AskForApproval::OnRequest
} else {
AskForApproval::default()
}
});
let did_user_set_custom_approval_policy_or_sandbox_mode = approval_policy_override
.is_some()
|| config_profile.approval_policy.is_some()
|| cfg.approval_policy.is_some()
// TODO(#3034): profile.sandbox_mode is not implemented
|| sandbox_mode.is_some()
|| cfg.sandbox_mode.is_some();
let mut model_providers = built_in_model_providers();
// Merge user-defined providers into the built-in list.
@@ -1050,25 +1118,6 @@ impl Config {
let shell_environment_policy = cfg.shell_environment_policy.into();
let resolved_cwd = {
use std::env;
match cwd {
None => {
tracing::info!("cwd not set, using current dir");
env::current_dir()?
}
Some(p) if p.is_absolute() => p,
Some(p) => {
// Resolve relative path against the current working directory.
tracing::info!("cwd is relative, resolving against current dir");
let mut current = env::current_dir()?;
current.push(p);
current
}
}
};
let history = cfg.history.unwrap_or_default();
let include_plan_tool_flag = features.enabled(Feature::PlanTool);
@@ -1125,11 +1174,6 @@ impl Config {
.or(cfg.review_model)
.unwrap_or_else(default_review_model);
let mut approval_policy = approval_policy
.or(config_profile.approval_policy)
.or(cfg.approval_policy)
.unwrap_or_else(AskForApproval::default);
if features.enabled(Feature::ApproveAll) {
approval_policy = AskForApproval::OnRequest;
}
@@ -1146,6 +1190,7 @@ impl Config {
cwd: resolved_cwd,
approval_policy,
sandbox_policy,
did_user_set_custom_approval_policy_or_sandbox_mode,
shell_environment_policy,
notify: cfg.notify,
user_instructions,
@@ -1200,6 +1245,7 @@ impl Config {
include_view_image_tool: include_view_image_tool_flag,
features,
active_profile: active_profile_name,
active_project,
windows_wsl_setup_acknowledged: cfg.windows_wsl_setup_acknowledged.unwrap_or(false),
disable_paste_burst: cfg.disable_paste_burst.unwrap_or(false),
tui_notifications: cfg
@@ -1395,7 +1441,8 @@ network_access = false # This should be ignored.
let sandbox_mode_override = None;
assert_eq!(
SandboxPolicy::DangerFullAccess,
sandbox_full_access_cfg.derive_sandbox_policy(sandbox_mode_override)
sandbox_full_access_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
let sandbox_read_only = r#"
@@ -1410,7 +1457,8 @@ network_access = true # This should be ignored.
let sandbox_mode_override = None;
assert_eq!(
SandboxPolicy::ReadOnly,
sandbox_read_only_cfg.derive_sandbox_policy(sandbox_mode_override)
sandbox_read_only_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
let sandbox_workspace_write = r#"
@@ -1434,7 +1482,36 @@ exclude_slash_tmp = true
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
sandbox_workspace_write_cfg.derive_sandbox_policy(sandbox_mode_override)
sandbox_workspace_write_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
let sandbox_workspace_write = r#"
sandbox_mode = "workspace-write"
[sandbox_workspace_write]
writable_roots = [
"/my/workspace",
]
exclude_tmpdir_env_var = true
exclude_slash_tmp = true
[projects."/tmp/test"]
trust_level = "trusted"
"#;
let sandbox_workspace_write_cfg = toml::from_str::<ConfigToml>(sandbox_workspace_write)
.expect("TOML deserialization should succeed");
let sandbox_mode_override = None;
assert_eq!(
SandboxPolicy::WorkspaceWrite {
writable_roots: vec![PathBuf::from("/my/workspace")],
network_access: false,
exclude_tmpdir_env_var: true,
exclude_slash_tmp: true,
},
sandbox_workspace_write_cfg
.derive_sandbox_policy(sandbox_mode_override, &PathBuf::from("/tmp/test"))
);
}
@@ -2221,6 +2298,7 @@ model_verbosity = "high"
model_provider: fixture.openai_provider.clone(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
did_user_set_custom_approval_policy_or_sandbox_mode: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
user_instructions: None,
notify: None,
@@ -2250,6 +2328,7 @@ model_verbosity = "high"
include_view_image_tool: true,
features: Features::with_defaults(),
active_profile: Some("o3".to_string()),
active_project: ProjectConfig { trust_level: None },
windows_wsl_setup_acknowledged: false,
disable_paste_burst: false,
tui_notifications: Default::default(),
@@ -2285,6 +2364,7 @@ model_verbosity = "high"
model_provider: fixture.openai_chat_completions_provider.clone(),
approval_policy: AskForApproval::UnlessTrusted,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
did_user_set_custom_approval_policy_or_sandbox_mode: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
user_instructions: None,
notify: None,
@@ -2314,6 +2394,7 @@ model_verbosity = "high"
include_view_image_tool: true,
features: Features::with_defaults(),
active_profile: Some("gpt3".to_string()),
active_project: ProjectConfig { trust_level: None },
windows_wsl_setup_acknowledged: false,
disable_paste_burst: false,
tui_notifications: Default::default(),
@@ -2364,6 +2445,7 @@ model_verbosity = "high"
model_provider: fixture.openai_provider.clone(),
approval_policy: AskForApproval::OnFailure,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
did_user_set_custom_approval_policy_or_sandbox_mode: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
user_instructions: None,
notify: None,
@@ -2393,6 +2475,7 @@ model_verbosity = "high"
include_view_image_tool: true,
features: Features::with_defaults(),
active_profile: Some("zdr".to_string()),
active_project: ProjectConfig { trust_level: None },
windows_wsl_setup_acknowledged: false,
disable_paste_burst: false,
tui_notifications: Default::default(),
@@ -2429,6 +2512,7 @@ model_verbosity = "high"
model_provider: fixture.openai_provider.clone(),
approval_policy: AskForApproval::OnFailure,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
did_user_set_custom_approval_policy_or_sandbox_mode: true,
shell_environment_policy: ShellEnvironmentPolicy::default(),
user_instructions: None,
notify: None,
@@ -2458,6 +2542,7 @@ model_verbosity = "high"
include_view_image_tool: true,
features: Features::with_defaults(),
active_profile: Some("gpt5".to_string()),
active_project: ProjectConfig { trust_level: None },
windows_wsl_setup_acknowledged: false,
disable_paste_burst: false,
tui_notifications: Default::default(),
@@ -2469,6 +2554,24 @@ model_verbosity = "high"
Ok(())
}
#[test]
fn test_did_user_set_custom_approval_policy_or_sandbox_mode_defaults_no() -> anyhow::Result<()>
{
let fixture = create_test_fixture()?;
let config = Config::load_from_base_config_with_overrides(
fixture.cfg.clone(),
ConfigOverrides {
..Default::default()
},
fixture.codex_home(),
)?;
assert!(config.did_user_set_custom_approval_policy_or_sandbox_mode);
Ok(())
}
#[test]
fn test_set_project_trusted_writes_explicit_tables() -> anyhow::Result<()> {
let project_dir = Path::new("/some/path");

1
codex-rs/tui/Cargo.toml
View File

@@ -78,6 +78,7 @@ tokio = { workspace = true, features = [
"signal",
] }
tokio-stream = { workspace = true }
toml = { workspace = true }
tracing = { workspace = true, features = ["log"] }
tracing-appender = { workspace = true }
tracing-subscriber = { workspace = true, features = ["env-filter"] }

151
codex-rs/tui/src/lib.rs
View File

@@ -13,12 +13,8 @@ use codex_core::INTERACTIVE_SESSION_SOURCES;
use codex_core::RolloutRecorder;
use codex_core::config::Config;
use codex_core::config::ConfigOverrides;
use codex_core::config::ConfigToml;
use codex_core::config::find_codex_home;
use codex_core::config::load_config_as_toml_with_cli_overrides;
use codex_core::find_conversation_path_by_id_str;
use codex_core::protocol::AskForApproval;
use codex_core::protocol::SandboxPolicy;
use codex_ollama::DEFAULT_OSS_MODEL;
use codex_protocol::config_types::SandboxMode;
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
@@ -192,52 +188,9 @@ pub async fn run_main(
}
};
let mut config = {
// Load configuration and support CLI overrides.
#[allow(clippy::print_stderr)]
match Config::load_with_cli_overrides(cli_kv_overrides.clone(), overrides).await {
Ok(config) => config,
Err(err) => {
eprintln!("Error loading configuration: {err}");
std::process::exit(1);
}
}
};
// we load config.toml here to determine project state.
#[allow(clippy::print_stderr)]
let config_toml = {
let codex_home = match find_codex_home() {
Ok(codex_home) => codex_home,
Err(err) => {
eprintln!("Error finding codex home: {err}");
std::process::exit(1);
}
};
match load_config_as_toml_with_cli_overrides(&codex_home, cli_kv_overrides).await {
Ok(config_toml) => config_toml,
Err(err) => {
eprintln!("Error loading config.toml: {err}");
std::process::exit(1);
}
}
};
let cli_profile_override = cli.config_profile.clone();
let active_profile = cli_profile_override
.clone()
.or_else(|| config_toml.profile.clone());
let should_show_trust_screen = determine_repo_trust_state(
&mut config,
&config_toml,
approval_policy,
sandbox_mode,
cli_profile_override,
)?;
let config = load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await;
let active_profile = config.active_profile.clone();
let log_dir = codex_core::config::log_dir(&config)?;
std::fs::create_dir_all(&log_dir)?;
// Open (or create) your log file, appending to it.
@@ -303,18 +256,18 @@ pub async fn run_main(
let _ = tracing_subscriber::registry().with(file_layer).try_init();
};
run_ratatui_app(cli, config, active_profile, should_show_trust_screen)
run_ratatui_app(cli, config, overrides, cli_kv_overrides, active_profile)
.await
.map_err(|err| std::io::Error::other(err.to_string()))
}
async fn run_ratatui_app(
cli: Cli,
config: Config,
initial_config: Config,
overrides: ConfigOverrides,
cli_kv_overrides: Vec<(String, toml::Value)>,
active_profile: Option<String>,
should_show_trust_screen: bool,
) -> color_eyre::Result<AppExitInfo> {
let mut config = config;
color_eyre::install()?;
// Forward panic reports through tracing so they appear in the UI status
@@ -337,7 +290,7 @@ async fn run_ratatui_app(
let skip_update_prompt = cli.prompt.as_ref().is_some_and(|prompt| !prompt.is_empty());
if !skip_update_prompt {
match update_prompt::run_update_prompt_if_needed(&mut tui, &config).await? {
match update_prompt::run_update_prompt_if_needed(&mut tui, &initial_config).await? {
UpdatePromptOutcome::Continue => {}
UpdatePromptOutcome::RunUpdate(action) => {
crate::tui::restore()?;
@@ -354,7 +307,7 @@ async fn run_ratatui_app(
// Show update banner in terminal history (instead of stderr) so it is visible
// within the TUI scrollback. Building spans keeps styling consistent.
#[cfg(not(debug_assertions))]
if let Some(latest_version) = updates::get_upgrade_version(&config) {
if let Some(latest_version) = updates::get_upgrade_version(&initial_config) {
use crate::history_cell::padded_emoji;
use crate::history_cell::with_border_with_inner_width;
use ratatui::style::Stylize as _;
@@ -402,27 +355,29 @@ async fn run_ratatui_app(
}
// Initialize high-fidelity session event logging if enabled.
session_log::maybe_init(&config);
session_log::maybe_init(&initial_config);
let auth_manager = AuthManager::shared(config.codex_home.clone(), false);
let login_status = get_login_status(&config);
let auth_manager = AuthManager::shared(initial_config.codex_home.clone(), false);
let login_status = get_login_status(&initial_config);
let should_show_trust_screen = should_show_trust_screen(&initial_config);
let should_show_windows_wsl_screen =
cfg!(target_os = "windows") && !config.windows_wsl_setup_acknowledged;
cfg!(target_os = "windows") && !initial_config.windows_wsl_setup_acknowledged;
let should_show_onboarding = should_show_onboarding(
login_status,
&config,
&initial_config,
should_show_trust_screen,
should_show_windows_wsl_screen,
);
if should_show_onboarding {
let config = if should_show_onboarding {
let onboarding_result = run_onboarding_app(
OnboardingScreenArgs {
show_login_screen: should_show_login_screen(login_status, &initial_config),
show_windows_wsl_screen: should_show_windows_wsl_screen,
show_login_screen: should_show_login_screen(login_status, &config),
show_trust_screen: should_show_trust_screen,
login_status,
auth_manager: auth_manager.clone(),
config: config.clone(),
config: initial_config.clone(),
},
&mut tui,
)
@@ -440,14 +395,20 @@ async fn run_ratatui_app(
update_action: None,
});
}
if should_show_windows_wsl_screen {
config.windows_wsl_setup_acknowledged = true;
// if the user acknowledged windows or made an explicit decision ato trust the directory, reload the config accordingly
if should_show_windows_wsl_screen
|| onboarding_result
.directory_trust_decision
.map(|d| d == TrustDirectorySelection::Trust)
.unwrap_or(false)
{
load_config_or_exit(cli_kv_overrides, overrides).await
} else {
initial_config
}
if let Some(TrustDirectorySelection::Trust) = onboarding_result.directory_trust_decision {
config.approval_policy = AskForApproval::OnRequest;
config.sandbox_policy = SandboxPolicy::new_workspace_write_policy();
}
}
} else {
initial_config
};
// Determine resume behavior: explicit id, then resume last, then picker.
let resume_selection = if let Some(id_str) = cli.resume_session_id.as_deref() {
@@ -588,39 +549,31 @@ fn get_login_status(config: &Config) -> LoginStatus {
}
}
/// Determine if user has configured a sandbox / approval policy,
/// or if the current cwd project is trusted, and updates the config
/// accordingly.
fn determine_repo_trust_state(
config: &mut Config,
config_toml: &ConfigToml,
approval_policy_overide: Option<AskForApproval>,
sandbox_mode_override: Option<SandboxMode>,
config_profile_override: Option<String>,
) -> std::io::Result<bool> {
let config_profile = config_toml.get_config_profile(config_profile_override)?;
async fn load_config_or_exit(
cli_kv_overrides: Vec<(String, toml::Value)>,
overrides: ConfigOverrides,
) -> Config {
#[allow(clippy::print_stderr)]
match Config::load_with_cli_overrides(cli_kv_overrides, overrides).await {
Ok(config) => config,
Err(err) => {
eprintln!("Error loading configuration: {err}");
std::process::exit(1);
}
}
}
if approval_policy_overide.is_some() || sandbox_mode_override.is_some() {
/// Determine if user has configured a sandbox / approval policy,
/// or if the current cwd project is already trusted. If not, we need to
/// show the trust screen.
fn should_show_trust_screen(config: &Config) -> bool {
if config.did_user_set_custom_approval_policy_or_sandbox_mode {
// if the user has overridden either approval policy or sandbox mode,
// skip the trust flow
Ok(false)
} else if config_profile.approval_policy.is_some() {
// if the user has specified settings in a config profile, skip the trust flow
// todo: profile sandbox mode?
Ok(false)
} else if config_toml.approval_policy.is_some() || config_toml.sandbox_mode.is_some() {
// if the user has specified either approval policy or sandbox mode in config.toml
// skip the trust flow
Ok(false)
} else if config_toml.is_cwd_trusted(&config.cwd) {
// if the current cwd project is trusted and no config has been set
// skip the trust flow and set the approval policy and sandbox mode
config.approval_policy = AskForApproval::OnRequest;
config.sandbox_policy = SandboxPolicy::new_workspace_write_policy();
Ok(false)
false
} else {
// if none of the above conditions are met, show the trust screen
Ok(true)
// otherwise, skip iff the active project is trusted
!config.active_project.is_trusted()
}
}