From 84e7d4a1fefd0c595c29cdb9af6c065fc7277d98 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 11 Sep 2026 06:33:13 +0000 Subject: [PATCH] Check folder trust after resolving the startup destination (#44746) ## Why Resume and fork selection can change the working directory. Folder consent needs to cover the selected destination, and existing tasks in untrusted folders may retain settings and history loaded while trusted. ## What changed - Move startup consent after destination selection and reload local configuration when trust is saved. - Check both selected and saved folders for local daemon resumes, and reread the task after consent to catch working directory changes. - Resolve local trust using project markers, Git roots, and normalized paths, honoring explicit working directory decisions and canonical path precedence. - Offer restricted-folder consent without changing saved trust, with distinct wording for existing tasks that may retain configuration or hooks. Preserve remote checks scoped to explicit `--cd`. ## Testing Extend trust lookup coverage for parent/child decisions, repositories without project configuration, path aliases, and custom project markers. Update restricted-folder snapshots and add an existing-task snapshot. GitOrigin-RevId: caf8c70f1e953c4f7b4ddc89dd29bb1fd8ed62f1 --- codex-rs/config/src/loader/mod.rs | 7 +- codex-rs/tui/src/app/agents_overview_tests.rs | 25 ++ codex-rs/tui/src/app/tests.rs | 20 ++ codex-rs/tui/src/config_update.rs | 146 +++++++++--- codex-rs/tui/src/config_update_tests.rs | 132 ++++++++++- codex-rs/tui/src/lib.rs | 213 +++++++++++------- .../tui/src/onboarding/directory_trust.rs | 153 +++++++++++++ .../tui/src/onboarding/onboarding_screen.rs | 29 ++- ...ctory__tests__existing_untrusted_task.snap | 16 ++ ...ory__tests__renders_restricted_folder.snap | 10 +- .../tui/src/onboarding/trust_directory.rs | 47 ++-- codex-rs/tui/tests/suite/reconnect.rs | 5 + 12 files changed, 664 insertions(+), 139 deletions(-) create mode 100644 codex-rs/tui/src/onboarding/directory_trust.rs create mode 100644 codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__existing_untrusted_task.snap diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index cf2ea1e603..6f6a7bef03 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -1394,7 +1394,8 @@ pub fn project_trust_key(path: &Path) -> String { .unwrap_or_else(|| normalize_project_trust_lookup_key(path.to_string_lossy().to_string())) } -fn normalized_project_trust_keys(path: &Path) -> Vec { +/// Returns canonical and original path spellings in trust-lookup precedence order. +pub fn normalized_project_trust_keys(path: &Path) -> Vec { let normalized_path = normalize_project_trust_lookup_key(path.to_string_lossy().to_string()); let normalized_canonical_path = normalize_project_trust_lookup_key( normalize_path(path) @@ -1494,7 +1495,9 @@ fn copy_shape_from_original(original: &TomlValue, resolved: &TomlValue) -> TomlV } } -async fn find_project_root( +/// Finds the nearest ancestor with a configured project marker, or returns `cwd`. +/// Callers must use markers from configuration loaded before project layers. +pub async fn find_project_root( fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, project_root_markers: &[String], diff --git a/codex-rs/tui/src/app/agents_overview_tests.rs b/codex-rs/tui/src/app/agents_overview_tests.rs index b660725f4c..89c3d970dc 100644 --- a/codex-rs/tui/src/app/agents_overview_tests.rs +++ b/codex-rs/tui/src/app/agents_overview_tests.rs @@ -711,6 +711,7 @@ async fn shared_overview_keeps_rows_and_replays_changes_over_stale_reads() -> Re #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shared_overview_seeds_once_and_retains_locally_resumed_history() -> Result<()> { let mut app = make_test_app().await; + trust_fixture_folders(&mut app); let mut ids = Vec::new(); for day in 1..=22 { let source = match day { @@ -856,6 +857,12 @@ async fn shared_overview_seeds_once_and_retains_locally_resumed_history() -> Res app.config.codex_home.join("config.toml"), "[tui]\nresume_cwd = \"session\"\n", )?; + crate::legacy_core::config::set_project_trust_level( + app.config.codex_home.as_path(), + &test_path_buf("/"), + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(std::io::Error::other)?; let mut tui = crate::tui::test_support::make_test_tui()?; Box::pin(app.select_agents_overview_thread(&mut tui, &mut app_server, ids[2])).await?; assert_eq!(app.primary_thread_id, Some(ids[2])); @@ -1674,6 +1681,7 @@ async fn root_switch_preserves_vim_line_yank() -> Result<()> { async fn root_switch_loads_local_preferences_from_disk() -> Result<()> { // Keep the large setup and root-switch futures off the test thread's stack. let mut app = Box::pin(make_test_app()).await; + trust_fixture_folders(&mut app); let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker( app.chat_widget.config_ref(), )) @@ -1713,6 +1721,7 @@ async fn root_switch_loads_local_preferences_from_disk() -> Result<()> { #[tokio::test] async fn root_switch_preserves_idle_root_with_running_subagent() -> Result<()> { let mut app = make_test_app().await; + trust_fixture_folders(&mut app); let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()).await?; let previous = app_server.start_thread(&app.config).await?; @@ -1784,6 +1793,7 @@ async fn root_switch_preserves_idle_root_with_running_subagent() -> Result<()> { #[tokio::test] async fn overview_selection_applies_user_permissions_only_to_unloaded_threads() -> Result<()> { let mut app = make_test_app().await; + trust_fixture_folders(&mut app); std::fs::write( app.config.codex_home.join("config.toml"), "[tui]\nresume_cwd = \"session\"\n", @@ -1930,6 +1940,7 @@ async fn overview_cold_resume_honors_working_directory_selection() -> Result<()> ("session", false, false), ] { let mut app = make_test_app().await; + trust_fixture_folders(&mut app); let chosen = app.config.codex_home.join("chosen"); let overridden = app.config.codex_home.join("overridden"); std::fs::create_dir(&chosen)?; @@ -2240,6 +2251,7 @@ async fn resume_failure_keeps_command_center_available() { #[tokio::test] async fn resume_picker_round_trip_preserves_each_threads_input() -> Result<()> { let mut app = make_test_app().await; + trust_fixture_folders(&mut app); std::fs::write( app.config.codex_home.join("config.toml"), "[tui]\nresume_cwd = \"current\"\n", @@ -2338,6 +2350,7 @@ async fn resume_picker_round_trip_preserves_each_threads_input() -> Result<()> { #[tokio::test] async fn command_center_handles_resume_failure_and_success() -> Result<()> { let mut app = make_test_app().await; + trust_fixture_folders(&mut app); std::fs::write( app.config.codex_home.join("config.toml"), "[tui]\nresume_cwd = \"current\"\n", @@ -2409,6 +2422,7 @@ async fn command_center_handles_resume_failure_and_success() -> Result<()> { #[tokio::test] async fn command_center_attach_conflict_preserves_selection_and_draft() -> Result<()> { let mut app = Box::pin(make_test_app()).await; + trust_fixture_folders(&mut app); std::fs::write( app.config.codex_home.join("config.toml"), "[tui]\nresume_cwd = \"current\"\n", @@ -2628,3 +2642,14 @@ async fn command_center_action_failures_remain_visible() -> Result<()> { } #[path = "agents_overview_actions_tests.rs"] mod actions; + +fn trust_fixture_folders(app: &mut App) { + let projects = serde_json::json!({ + test_path_buf("/").display().to_string(): {"trust_level": "trusted"}, + app.config.cwd.display().to_string(): {"trust_level": "trusted"}, + }); + app.cli_kv_overrides.push(( + "projects".into(), + toml::Value::try_from(projects).expect("trust fixture"), + )); +} diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 3397652530..b640db1dbc 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -7715,6 +7715,20 @@ async fn in_app_resume_uses_configured_or_explicit_cwd() -> Result<()> { codex_home.join("config.toml"), format!("[tui]\nresume_cwd = \"{configured_mode}\"\n"), )?; + for cwd in [ + &launch_cwd, + &active_cwd, + &session_cwd, + &explicit_cwd, + &runtime_cwd, + ] { + crate::legacy_core::config::set_project_trust_level( + &codex_home, + cwd, + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(std::io::Error::other)?; + } let config = ConfigBuilder::default() .codex_home(codex_home.clone()) .loader_overrides(LoaderOverrides::without_managed_config_for_tests()) @@ -7843,6 +7857,12 @@ async fn remembered_current_cwd_stays_at_launch_across_in_app_resumes() -> Resul std::fs::create_dir_all(&active_cwd)?; std::fs::create_dir_all(&first_session_cwd)?; std::fs::create_dir_all(&second_session_cwd)?; + crate::legacy_core::config::set_project_trust_level( + &codex_home, + &launch_cwd, + codex_protocol::config_types::TrustLevel::Trusted, + ) + .map_err(std::io::Error::other)?; let config = ConfigBuilder::default() .codex_home(codex_home.clone()) .loader_overrides(LoaderOverrides::without_managed_config_for_tests()) diff --git a/codex-rs/tui/src/config_update.rs b/codex-rs/tui/src/config_update.rs index 259c043f4f..96b16a4564 100644 --- a/codex-rs/tui/src/config_update.rs +++ b/codex-rs/tui/src/config_update.rs @@ -18,13 +18,19 @@ use codex_app_server_protocol::MergeStrategy; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SkillsConfigWriteParams; use codex_app_server_protocol::SkillsConfigWriteResponse; +use codex_config::default_project_root_markers; +use codex_config::loader::find_project_root; +use codex_config::loader::normalized_project_trust_keys; use codex_config::loader::project_trust_key; use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_exec_server::LOCAL_FS; use codex_features::FEATURES; +use codex_git_utils::resolve_root_git_project_for_trust; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::config_types::TrustLevel; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathConvention; use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; use serde_json::Value as JsonValue; @@ -33,8 +39,15 @@ use std::path::Path; use std::path::PathBuf; use uuid::Uuid; +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum ProjectTrustHost { + Local, + Remote, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct RemoteProjectTrust { + pub trust_level: Option, pub cwd: PathBuf, pub trust_target: PathBuf, } @@ -202,6 +215,7 @@ pub(crate) async fn read_effective_config( pub(crate) async fn read_remote_project_trust( request_handle: AppServerRequestHandle, cwd: &Path, + host: ProjectTrustHost, ) -> Result> { let cwd_string = cwd.to_string_lossy().into_owned(); let cwd = match LegacyAppPathString::from_string(cwd_string.clone()).to_inferred_path_uri() { @@ -253,44 +267,115 @@ pub(crate) async fn read_remote_project_trust( let disabled_reason = disabled_project .and_then(|layer| layer.get("disabledReason")) .and_then(JsonValue::as_str); - let trust_target = disabled_reason - .and_then(|reason| reason.split_once(", add ")) - .and_then(|(_, reason)| reason.rsplit_once(" as a trusted project in ")) - .map(|(trust_target, _)| trust_target) + let projects = response["config"]["projects"].as_object(); + let windows_path = cwd_uri.infer_path_convention() == Some(PathConvention::Windows); + let cwd_key = if windows_path { + cwd.to_ascii_lowercase() + } else { + cwd.clone() + }; + let project_entry = |key: &str| { + let projects = projects?; + projects + .get_key_value(key) + .filter(|(_, project)| project["trust_level"].as_str().is_some()) + .or_else(|| { + projects + .iter() + .filter(|(path, project)| { + windows_path + && path.eq_ignore_ascii_case(key) + && project["trust_level"].as_str().is_some() + }) + .min_by_key(|(path, _)| *path) + }) + }; + let local_roots = if host == ProjectTrustHost::Local && project_layers.is_empty() { + let cwd = AbsolutePathBuf::from_absolute_path(&cwd)?; + let markers = serde_json::from_value::>>( + response["config"]["project_root_markers"].clone(), + )? + .unwrap_or_else(default_project_root_markers); + let project_root = find_project_root(LOCAL_FS.as_ref(), &cwd, &markers).await?; + let git_root = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &cwd).await; + Some(( + normalized_project_trust_keys(project_root.as_path()), + git_root.map(|root| normalized_project_trust_keys(root.as_path())), + )) + } else { + None + }; + let cwd_keys = if host == ProjectTrustHost::Local { + normalized_project_trust_keys(Path::new(&cwd)) + } else { + vec![cwd_key] + }; + let saved_trust_target = |keys: &[String]| { + keys.iter() + .find_map(|key| project_entry(key).map(|(path, _)| path.as_str())) + }; + let cwd_trust_target = saved_trust_target(&cwd_keys); + let trust_target = cwd_trust_target .or_else(|| { - disabled_project - .and_then(|layer| layer["name"]["dotCodexFolder"].as_str()) - .and_then(|path| { - path.strip_suffix("/.codex") - .or_else(|| path.strip_suffix("\\.codex")) + disabled_reason + .and_then(|reason| reason.split_once(", add ")) + .and_then(|(_, reason)| reason.rsplit_once(" as a trusted project in ")) + .map(|(trust_target, _)| trust_target) + .or_else(|| { + disabled_project + .and_then(|layer| layer["name"]["dotCodexFolder"].as_str()) + .and_then(|path| { + path.strip_suffix("/.codex") + .or_else(|| path.strip_suffix("\\.codex")) + }) }) }) + .or_else(|| { + let (project_root, git_root) = local_roots.as_ref()?; + saved_trust_target(project_root) + .or_else(|| git_root.as_deref().and_then(saved_trust_target)) + .or_else(|| { + git_root + .as_ref() + .and_then(|keys| keys.first()) + .map(String::as_str) + }) + .or_else(|| project_root.first().map(String::as_str)) + }) .unwrap_or(&cwd); - let projects = response["config"]["projects"].as_object(); - let has_trust_decision = projects - .and_then(|projects| projects.get(trust_target)) + let trust_level = project_entry(trust_target) + .map(|(_, project)| project) .and_then(|project| project.get("trust_level")) .and_then(JsonValue::as_str) - .is_some_and(|level| matches!(level, "trusted" | "untrusted")); - let explicitly_untrusted = disabled_reason.is_some_and(|reason| { - projects.into_iter().flatten().any(|(path, project)| { - project.get("trust_level").and_then(JsonValue::as_str) == Some("untrusted") - && reason - .strip_prefix(path) - .is_some_and(|suffix| suffix.starts_with(" is marked as untrusted")) - }) - }); - if has_trust_decision - || explicitly_untrusted - || (disabled_project.is_none() - && project_layers - .iter() - .any(|layer| layer.get("disabledReason").is_none())) + .and_then(|level| match level { + "trusted" => Some(TrustLevel::Trusted), + "untrusted" => Some(TrustLevel::Untrusted), + _ => None, + }); + let explicitly_untrusted = cwd_trust_target.is_none() + && disabled_reason.is_some_and(|reason| { + projects.into_iter().flatten().any(|(path, project)| { + project.get("trust_level").and_then(JsonValue::as_str) == Some("untrusted") + && reason + .strip_prefix(path) + .is_some_and(|suffix| suffix.starts_with(" is marked as untrusted")) + }) + }); + if !explicitly_untrusted + && (trust_level == Some(TrustLevel::Trusted) + || (trust_level.is_none() + && disabled_project.is_none() + && project_layers + .iter() + .any(|layer| layer.get("disabledReason").is_none()))) { return Ok(None); } - if project_layers.is_empty() + if host == ProjectTrustHost::Remote + && trust_level.is_none() + && !explicitly_untrusted + && project_layers.is_empty() && projects.into_iter().flatten().any(|(path, project)| { project.get("trust_level").and_then(JsonValue::as_str) == Some("untrusted") && LegacyAppPathString::from_string(path.clone()) @@ -304,6 +389,11 @@ pub(crate) async fn read_remote_project_trust( } Ok(Some(RemoteProjectTrust { + trust_level: if explicitly_untrusted { + Some(TrustLevel::Untrusted) + } else { + trust_level + }, cwd: PathBuf::from(&cwd), trust_target: PathBuf::from(trust_target), })) diff --git a/codex-rs/tui/src/config_update_tests.rs b/codex-rs/tui/src/config_update_tests.rs index e7f2aada8a..90bbdcd77d 100644 --- a/codex-rs/tui/src/config_update_tests.rs +++ b/codex-rs/tui/src/config_update_tests.rs @@ -64,12 +64,16 @@ async fn remote_project_trust_guards_thread_start_and_preserves_repository_decis .await?; let app_server = AppServerClient::InProcess(crate::tests::start_test_embedded_app_server(config).await?); + let read_trust = async |cwd: &Path, host| { + read_remote_project_trust(app_server.request_handle(), cwd, host).await + }; let relative_cwd = pathdiff::diff_paths(&project_cwd, std::env::current_dir()?) .ok_or_else(|| color_eyre::eyre::eyre!("failed to calculate relative project path"))?; assert_eq!( - read_remote_project_trust(app_server.request_handle(), &relative_cwd).await?, + read_trust(&relative_cwd, ProjectTrustHost::Remote).await?, Some(RemoteProjectTrust { + trust_level: None, cwd: project_cwd.clone(), trust_target: PathBuf::from(project_trust_key(&project_root)), }) @@ -98,7 +102,7 @@ async fn remote_project_trust_guards_thread_start_and_preserves_repository_decis }) .await?; assert_eq!( - read_remote_project_trust(app_server.request_handle(), &project_cwd).await?, + read_trust(&project_cwd, ProjectTrustHost::Remote).await?, None ); @@ -106,8 +110,12 @@ async fn remote_project_trust_guards_thread_start_and_preserves_repository_decis untrusted_project.value = serde_json::json!("untrusted"); write_config_batch(app_server.request_handle(), vec![untrusted_project]).await?; assert_eq!( - read_remote_project_trust(app_server.request_handle(), &project_cwd).await?, - None + read_trust(&project_cwd, ProjectTrustHost::Remote).await?, + Some(RemoteProjectTrust { + trust_level: Some(TrustLevel::Untrusted), + cwd: project_cwd.clone(), + trust_target: project_cwd.clone(), + }) ); let response: JsonValue = app_server @@ -143,14 +151,128 @@ async fn remote_project_trust_guards_thread_start_and_preserves_repository_decis " ); + std::fs::create_dir(project_root.join(".codex"))?; + std::fs::write( + project_root.join(".codex/config.toml"), + "model_reasoning_effort = \"low\"\n", + )?; + for (parent, child) in [("untrusted", "trusted"), ("trusted", "untrusted")] { + write_config_batch( + app_server.request_handle(), + vec![replace_config_value( + "projects", + serde_json::json!({ + project_trust_key(&project_root): {"trust_level": parent}, + project_trust_key(&project_cwd): {"trust_level": child} + }), + )], + ) + .await?; + let consent = read_trust(&project_cwd, ProjectTrustHost::Local).await?; + assert_eq!( + consent.map(|project| project.trust_level), + (child == "untrusted").then_some(Some(TrustLevel::Untrusted)) + ); + } + std::fs::remove_dir_all(project_root.join(".codex"))?; + let untrusted_projects = replace_config_value( + "projects", + serde_json::json!({ + project_trust_key(&project_root): {"trust_level": "untrusted"} + }), + ); + write_config_batch( + app_server.request_handle(), + vec![untrusted_projects.clone()], + ) + .await?; + std::fs::remove_file(project_cwd.join(".codex/config.toml"))?; std::fs::remove_dir(project_cwd.join(".codex"))?; let canonical_project_cwd = PathBuf::from(project_trust_key(&project_root)).join("nested"); - let error = read_remote_project_trust(app_server.request_handle(), &canonical_project_cwd) + let error = read_trust(&canonical_project_cwd, ProjectTrustHost::Remote) .await .expect_err("an untrusted repository must not be overridden by its subdirectory"); assert!(error.to_string().contains("explicitly untrusted project")); + assert_eq!( + read_trust(&canonical_project_cwd, ProjectTrustHost::Local).await?, + Some(RemoteProjectTrust { + trust_level: Some(TrustLevel::Untrusted), + cwd: canonical_project_cwd.clone(), + trust_target: PathBuf::from(project_trust_key(&project_root)), + }) + ); + write_trusted_project(app_server.request_handle(), &project_root).await?; + assert_eq!( + read_trust(&canonical_project_cwd, ProjectTrustHost::Local).await?, + None + ); + + let key = project_trust_key(&project_root); + #[cfg(windows)] + let (legacy_key, lookup_cwd) = (key.to_ascii_uppercase(), canonical_project_cwd.clone()); + #[cfg(unix)] + let (legacy_key, lookup_cwd) = { + let alias = temp_dir.path().join("project-alias"); + std::os::unix::fs::symlink(&project_root, &alias)?; + (alias.to_string_lossy().into_owned(), alias.join("nested")) + }; + for (trust_level, canonical_key) in [ + ("trusted", true), + ("untrusted", true), + ("trusted", false), + ("untrusted", false), + ] { + // Canonical decisions win over legacy symlink or case aliases. + let mut projects = serde_json::json!({&legacy_key: {"trust_level": trust_level}}); + projects[&key] = serde_json::json!({}); + if canonical_key { + projects[&legacy_key]["trust_level"] = serde_json::json!(if trust_level == "trusted" { + "untrusted" + } else { + "trusted" + }); + projects[&key] = serde_json::json!({"trust_level": trust_level}); + } + write_config_batch( + app_server.request_handle(), + vec![replace_config_value("projects", projects)], + ) + .await?; + let consent = read_trust(&lookup_cwd, ProjectTrustHost::Local).await?; + assert_eq!( + consent.map(|project| project.trust_level), + (trust_level == "untrusted").then_some(Some(TrustLevel::Untrusted)), + ); + } + std::fs::remove_dir_all(project_root.join(".git"))?; + write_config_batch(app_server.request_handle(), vec![untrusted_projects]).await?; + for marked_root in [false, true] { + if marked_root { + std::fs::write(project_root.join(".project-root"), "")?; + write_config_batch( + app_server.request_handle(), + vec![replace_config_value( + "project_root_markers", + serde_json::json!([".project-root"]), + )], + ) + .await?; + } + assert_eq!( + read_trust(&canonical_project_cwd, ProjectTrustHost::Local).await?, + Some(RemoteProjectTrust { + trust_level: marked_root.then_some(TrustLevel::Untrusted), + cwd: canonical_project_cwd.clone(), + trust_target: if marked_root { + PathBuf::from(project_trust_key(&project_root)) + } else { + canonical_project_cwd.clone() + }, + }) + ); + } app_server.shutdown().await?; Ok(()) } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index bc0de7bda4..0784998138 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1159,36 +1159,9 @@ async fn run_ratatui_app( } } } - let remote_project_trust = - if uses_remote_workspace && let Some(remote_cwd) = remote_cwd_override.as_deref() { - match startup_draft - .run_until( - &mut tui, - config_update::read_remote_project_trust( - app_server_session.request_handle(), - remote_cwd, - ), - ) - .await - { - Ok(Ok(remote_project_trust)) => remote_project_trust, - Ok(Err(err)) => { - shutdown_startup_session(Some(app_server_session), &mut terminal_restore_guard) - .await; - return Err(err); - } - Err(err) => { - shutdown_startup_session(Some(app_server_session), &mut terminal_restore_guard) - .await; - return Err(err.into()); - } - } - } else { - None - }; let mut app_server = Some(app_server_session); - let should_show_trust_screen_flag = remote_project_trust.is_some() - || (!uses_remote_workspace && should_show_trust_screen(&initial_config)); + // Folder consent runs after the picker resolves the actual destination. + let should_show_trust_screen_flag = false; #[cfg(target_os = "windows")] let mut trust_decision_was_made = false; let startup_model_provider = initial_config.model_provider_id.clone(); @@ -1242,7 +1215,7 @@ async fn run_ratatui_app( show_login_screen, bedrock_setup_enabled, show_trust_screen: should_show_trust_screen_flag, - remote_project_trust, + remote_project_trust: None, login_status, app_server_request_handle: app_server .as_ref() @@ -1662,6 +1635,137 @@ async fn run_ratatui_app( }; startup_draft.apply_config(&config); + if config.model_provider_id != startup_model_provider { + startup_account = None; + if matches!(&app_server_target, AppServerTarget::Embedded) { + // App-server providers are fixed at startup, so onboarding cannot + // reuse a server initialized before it persisted another provider. + shutdown_app_server_if_present(app_server.take()).await; + } + } + let mut app_server = match app_server { + Some(app_server) => app_server, + None => match startup_draft + .run_until( + &mut tui, + start_app_server( + &mut app_server_target, + arg0_paths.clone(), + config.clone(), + cli_kv_overrides.clone(), + loader_overrides.clone(), + strict_config, + cloud_config_bundle.clone(), + feedback.clone(), + log_db.clone(), + &mut state_db, + environment_manager.clone(), + ), + ) + .await + { + Ok(Ok(app_server)) => { + // A picker can replace the server; account reads belong to their original session. + startup_account = None; + AppServerSession::new(app_server, app_server_target.thread_params_mode()) + .with_local_codex_home(&config.codex_home) + .with_remote_cwd_override(remote_cwd_override.clone()) + } + Ok(Err(err)) => { + terminal_restore_guard.restore_silently(); + session_log::log_session_end(); + return Err(err); + } + Err(err) => { + terminal_restore_guard.restore_silently(); + session_log::log_session_end(); + return Err(err.into()); + } + }, + }; + + // Remote startup keeps its existing explicit --cd trust check. Resolving other + // remote folders requires authoritative project-root information from the server. + if !uses_remote_workspace || remote_cwd_override.is_some() { + let resumed_thread = if matches!(app_server_target, AppServerTarget::LocalDaemon { .. }) + && let resume_picker::SessionSelection::Resume(target) = &session_selection + { + Some( + startup_draft + .run_until( + &mut tui, + app_server.thread_read(target.thread_id, /*include_turns*/ false), + ) + .await??, + ) + } else { + None + }; + let trust_cwd = remote_cwd_override + .as_deref() + .unwrap_or(config.cwd.as_path()); + let consent = onboarding::onboarding_screen::check_directory_trust( + &mut tui, + &app_server, + &config, + &app_server_target, + trust_cwd, + resumed_thread.as_ref(), + Some(&mut startup_draft), + ) + .await?; + startup_account = None; + if consent.directory_trust_persisted && !uses_remote_workspace { + let previous_provider = config.model_provider_id.clone(); + config = load_config_or_exit_with_fallback_cwd( + cli_kv_overrides.clone(), + overrides.clone(), + loader_overrides.clone(), + cloud_config_bundle.clone(), + strict_config, + Some(config.cwd.to_path_buf()), + managed_worktree.as_ref(), + ) + .await; + if config.model_provider_id != previous_provider + && matches!(app_server_target, AppServerTarget::Embedded) + { + app_server.shutdown().await?; + let client = start_app_server( + &mut app_server_target, + arg0_paths.clone(), + config.clone(), + cli_kv_overrides.clone(), + loader_overrides.clone(), + strict_config, + cloud_config_bundle.clone(), + feedback.clone(), + log_db.clone(), + &mut state_db, + environment_manager.clone(), + ) + .await?; + app_server = AppServerSession::new(client, app_server_target.thread_params_mode()) + .with_local_codex_home(&config.codex_home); + } + #[cfg(target_os = "windows")] + { + trust_decision_was_made = true; + } + } else if consent.should_exit { + shutdown_startup_session(Some(app_server), &mut terminal_restore_guard).await; + return Ok(AppExitInfo { + token_usage: crate::token_usage::TokenUsage::default(), + thread_id: None, + resume_hint: None, + disconnect_info: None, + update_action: None, + exit_reason: ExitReason::UserRequested, + }); + } + } + startup_draft.apply_config(&config); + let local_settings = crate::local_settings::LocalSettings::from(&config); // Configure syntax highlighting theme from the final config — onboarding // and resume/fork can both reload config with a different tui_theme, so @@ -1705,55 +1809,6 @@ async fn run_ratatui_app( let use_alt_screen = determine_alt_screen_mode(no_alt_screen, local_settings.tui.alternate_screen); tui.set_alt_screen_enabled(use_alt_screen); - if config.model_provider_id != startup_model_provider { - startup_account = None; - if matches!(&app_server_target, AppServerTarget::Embedded) { - // App-server providers are fixed at startup, so onboarding cannot - // reuse a server initialized before it persisted another provider. - shutdown_app_server_if_present(app_server.take()).await; - } - } - let mut app_server = match app_server { - Some(app_server) => app_server, - None => match startup_draft - .run_until( - &mut tui, - start_app_server( - &mut app_server_target, - arg0_paths, - config.clone(), - cli_kv_overrides.clone(), - loader_overrides.clone(), - strict_config, - cloud_config_bundle.clone(), - feedback.clone(), - log_db.clone(), - &mut state_db, - environment_manager.clone(), - ), - ) - .await - { - Ok(Ok(app_server)) => { - // A picker can replace the server; account reads belong to their original session. - startup_account = None; - AppServerSession::new(app_server, app_server_target.thread_params_mode()) - .with_local_codex_home(&config.codex_home) - .with_remote_cwd_override(remote_cwd_override.clone()) - } - Ok(Err(err)) => { - terminal_restore_guard.restore_silently(); - session_log::log_session_end(); - return Err(err); - } - Err(err) => { - terminal_restore_guard.restore_silently(); - session_log::log_session_end(); - return Err(err.into()); - } - }, - }; - // Persistent app-server resumes may attach to an already-running thread, // where resume config overrides are ignored. let is_persistent_resume = !matches!(&app_server_target, AppServerTarget::Embedded) diff --git a/codex-rs/tui/src/onboarding/directory_trust.rs b/codex-rs/tui/src/onboarding/directory_trust.rs new file mode 100644 index 0000000000..01e00295cf --- /dev/null +++ b/codex-rs/tui/src/onboarding/directory_trust.rs @@ -0,0 +1,153 @@ +//! Folder consent for startup and task navigation. +//! Local daemon resumes recheck folders changed during consent. Remote checks +//! retain their existing explicit-cwd scope until authoritative roots are available. + +use super::OnboardingResult; +use super::OnboardingScreen; +use super::Step; +use super::run_onboarding_screen; +use crate::AppServerTarget; +use crate::app_server_session::AppServerSession; +use crate::config_update::ProjectTrustHost; +use crate::config_update::RemoteProjectTrust; +use crate::config_update::read_remote_project_trust; +use crate::legacy_core::config::Config; +use crate::onboarding::trust_directory::TrustDirectorySelection; +use crate::onboarding::trust_directory::TrustDirectoryWidget; +use crate::startup_draft::StartupDraftPump; +use crate::tui::Tui; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::Thread; +use codex_app_server_protocol::ThreadReadParams; +use codex_app_server_protocol::ThreadReadResponse; +use codex_exec_server::LOCAL_FS; +use codex_git_utils::resolve_root_git_project_for_trust; +use codex_protocol::config_types::TrustLevel; +use color_eyre::eyre::Result; +use std::collections::VecDeque; +use std::path::Path; +use uuid::Uuid; + +pub(crate) async fn check_directory_trust( + tui: &mut Tui, + app_server: &AppServerSession, + config: &Config, + target: &AppServerTarget, + cwd: &Path, + resumed_thread: Option<&Thread>, + mut startup_draft: Option<&mut StartupDraftPump>, +) -> Result { + let connected = !matches!(target, AppServerTarget::Embedded); + let mut consent = OnboardingResult::default(); + // Another client can load the saved task while consent is pending. Check both folders. + let saved_cwd = resumed_thread + .filter(|thread| { + matches!(target, AppServerTarget::LocalDaemon { .. }) && thread.cwd.as_path() != cwd + }) + .map(|thread| thread.cwd.as_path()); + let mut pending_cwds: VecDeque<_> = std::iter::once(cwd) + .chain(saved_cwd) + .map(Path::to_path_buf) + .collect(); + let mut checked_cwds = Vec::new(); + while let Some(cwd) = pending_cwds.pop_front() { + if checked_cwds.contains(&cwd) { + continue; + } + checked_cwds.push(cwd.clone()); + let cwd = cwd.as_path(); + let lookup = async { + if connected { + let host = if matches!(target, AppServerTarget::LocalDaemon { .. }) { + ProjectTrustHost::Local + } else { + ProjectTrustHost::Remote + }; + read_remote_project_trust(app_server.request_handle(), cwd, host).await + } else if config.active_project.trust_level == Some(TrustLevel::Trusted) { + Ok(None) + } else { + Ok(Some(RemoteProjectTrust { + cwd: cwd.to_path_buf(), + trust_target: resolve_root_git_project_for_trust( + LOCAL_FS.as_ref(), + &config.cwd, + ) + .await + .map(Into::into) + .unwrap_or_else(|| cwd.to_path_buf()), + trust_level: config.active_project.trust_level, + })) + } + }; + let project = if let Some(draft) = startup_draft.as_deref_mut() { + draft.run_until(tui, lookup).await?? + } else { + lookup.await? + }; + + let Some(project) = project else { + continue; + }; + // Remote connections retain the existing behavior for saved untrusted folders. + if target.uses_remote_workspace() && project.trust_level == Some(TrustLevel::Untrusted) { + continue; + } + let remote_trust_key = + connected.then(|| project.trust_target.to_string_lossy().into_owned()); + if let Some(draft) = startup_draft.as_deref_mut() { + draft.flush_pending_events(tui).await?; + } + let screen = OnboardingScreen { + request_frame: tui.frame_requester(), + steps: vec![Step::TrustDirectory(TrustDirectoryWidget { + restricted: project.trust_level == Some(TrustLevel::Untrusted), + existing_task: connected && resumed_thread.is_some(), + cwd: project.cwd, + trust_target: project.trust_target, + show_windows_create_sandbox_hint: false, + should_quit: false, + selection: None, + highlighted: TrustDirectorySelection::Trust, + error: None, + })], + remote_trust_key, + is_done: false, + should_exit: false, + }; + let result = run_onboarding_screen( + screen, + Some(app_server.request_handle()), + /*app_server*/ None, + tui, + ) + .await?; + tui.terminal.clear()?; + tui.frame_requester().schedule_frame(); + if result.should_exit { + return Ok(result); + } + consent.directory_trust_persisted |= result.directory_trust_persisted; + if matches!(target, AppServerTarget::LocalDaemon { .. }) + && let Some(thread) = resumed_thread + { + // Another client may have reopened this task in a different folder during consent. + let request_handle = app_server.request_handle(); + let recheck = request_handle.request_typed(ClientRequest::ThreadRead { + request_id: RequestId::String(format!("tui-trust-recheck-{}", Uuid::new_v4())), + params: ThreadReadParams { + thread_id: thread.id.clone(), + include_turns: false, + }, + }); + let latest: ThreadReadResponse = if let Some(draft) = startup_draft.as_deref_mut() { + draft.run_until(tui, recheck).await?? + } else { + recheck.await? + }; + pending_cwds.push_back(latest.thread.cwd.to_path_buf()); + } + } + Ok(consent) +} diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index 846d56554e..f2bcfc5eee 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -3,7 +3,7 @@ //! The onboarding flow is a small state machine over visible steps //! (welcome/auth/trust). This module decides which step receives key/paste //! events and enforces flow-level safety rules that cut across individual step -//! widgets. +//! widgets. Folder-entry consent reuses this same event loop for startup and in-app navigation. //! //! In particular, onboarding quit handling has a text-entry guard for API-key //! input: the printable `q` quit key is treated as text input while the user is @@ -57,6 +57,10 @@ use std::sync::Arc; use std::sync::RwLock; use uuid::Uuid; +#[path = "directory_trust.rs"] +mod directory_trust; +pub(crate) use directory_trust::check_directory_trust; + #[allow(clippy::large_enum_variant)] enum Step { Welcome(WelcomeWidget), @@ -98,6 +102,7 @@ pub(crate) struct OnboardingScreenArgs { pub config: Config, } +#[derive(Default)] pub(crate) struct OnboardingResult { pub directory_trust_persisted: bool, pub should_exit: bool, @@ -166,7 +171,9 @@ impl OnboardingScreen { let highlighted = TrustDirectorySelection::Trust; if show_trust_screen { let (cwd, trust_target) = match remote_project_trust { - Some(RemoteProjectTrust { cwd, trust_target }) => (cwd, trust_target), + Some(RemoteProjectTrust { + cwd, trust_target, .. + }) => (cwd, trust_target), None => { let trust_target = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &config.cwd) @@ -178,6 +185,7 @@ impl OnboardingScreen { }; steps.push(Step::TrustDirectory(TrustDirectoryWidget { restricted: false, + existing_task: false, cwd, trust_target, show_windows_create_sandbox_hint, @@ -504,13 +512,21 @@ impl WidgetRef for Step { pub(crate) async fn run_onboarding_app( args: OnboardingScreenArgs, + app_server: Option<&mut AppServerSession>, + tui: &mut Tui, +) -> Result { + let request_handle = args.app_server_request_handle.clone(); + let screen = OnboardingScreen::new(tui, args).await; + run_onboarding_screen(screen, request_handle, app_server, tui).await +} + +async fn run_onboarding_screen( + mut onboarding_screen: OnboardingScreen, + app_server_request_handle: Option, mut app_server: Option<&mut AppServerSession>, tui: &mut Tui, ) -> Result { use tokio_stream::StreamExt; - - let app_server_request_handle = args.app_server_request_handle.clone(); - let mut onboarding_screen = OnboardingScreen::new(tui, args).await; let mut directory_trust_persisted = false; // One-time guard to fully clear the screen after ChatGPT login success message is shown let mut did_full_clear_after_success = false; @@ -786,6 +802,7 @@ mod tests { request_frame: FrameRequester::test_dummy(), steps: vec![Step::TrustDirectory(TrustDirectoryWidget { restricted: false, + existing_task: false, cwd: PathBuf::from("/workspace/project"), trust_target: PathBuf::from("/workspace/project"), show_windows_create_sandbox_hint: false, @@ -851,6 +868,7 @@ mod tests { request_frame: FrameRequester::test_dummy(), steps: vec![Step::TrustDirectory(TrustDirectoryWidget { restricted: true, + existing_task: false, cwd: PathBuf::from("/workspace/project"), trust_target: PathBuf::from("/workspace/project"), show_windows_create_sandbox_hint: false, @@ -880,6 +898,7 @@ mod tests { request_frame: FrameRequester::test_dummy(), steps: vec![Step::TrustDirectory(TrustDirectoryWidget { restricted: false, + existing_task: false, cwd: PathBuf::from("/workspace/project"), trust_target: PathBuf::from("/workspace/project"), show_windows_create_sandbox_hint: false, diff --git a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__existing_untrusted_task.snap b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__existing_untrusted_task.snap new file mode 100644 index 0000000000..1ed5811be0 --- /dev/null +++ b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__existing_untrusted_task.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/onboarding/trust_directory.rs +assertion_line: 355 +expression: terminal.backend() +--- +> You are in /workspace/project + + This existing task may retain settings and history, including + project configuration or hooks loaded while it was trusted. To use + restricted settings, start a new task. The folder's trust setting + will not change. + +› 1. Open existing task + 2. Quit + + Press enter to continue; esc to quit diff --git a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_restricted_folder.snap b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_restricted_folder.snap index 23b7e16b77..c8750af1fb 100644 --- a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_restricted_folder.snap +++ b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_restricted_folder.snap @@ -1,14 +1,14 @@ --- source: tui/src/onboarding/trust_directory.rs -assertion_line: 335 +assertion_line: 357 expression: terminal.backend() --- > You are in /workspace/project - This folder is marked untrusted. Project-local config, hooks, and - exec policies stay disabled. Skills still load, and tool execution - follows your permission settings. Opening it will not change its - trust setting. + Config, hooks, and exec policies from untrusted folders stay + disabled. Trusted project folders can still contribute settings. + Skills still load, and tools follow your permission settings. + Opening will not change saved trust. › 1. Open restricted 2. Quit diff --git a/codex-rs/tui/src/onboarding/trust_directory.rs b/codex-rs/tui/src/onboarding/trust_directory.rs index 7a55e6985f..28d865ff52 100644 --- a/codex-rs/tui/src/onboarding/trust_directory.rs +++ b/codex-rs/tui/src/onboarding/trust_directory.rs @@ -25,6 +25,7 @@ use crate::selection_list::selection_option_row; use super::onboarding_screen::StepState; pub(crate) struct TrustDirectoryWidget { pub restricted: bool, + pub existing_task: bool, pub cwd: PathBuf, pub trust_target: PathBuf, pub show_windows_create_sandbox_hint: bool, @@ -69,10 +70,14 @@ impl WidgetRef for &TrustDirectoryWidget { } column.push( - Paragraph::new(if self.restricted { - "This folder is marked untrusted. Project-local config, hooks, and exec \ - policies stay disabled. Skills still load, and tool execution follows \ - your permission settings. Opening it will not change its trust setting." + Paragraph::new(if self.restricted && self.existing_task { + "This existing task may retain settings \ + and history, including project configuration or hooks loaded while it was trusted. \ + To use restricted settings, start a new task. The folder's trust setting will not change." + } else if self.restricted { + "Config, hooks, and exec policies from untrusted folders stay disabled. \ + Trusted project folders can still contribute settings. Skills still load, \ + and tools follow your permission settings. Opening will not change saved trust." } else { "Trust this folder? Codex can read, edit, and run files here, subject to \ your permission settings. Folder settings can run code automatically, \ @@ -88,7 +93,9 @@ impl WidgetRef for &TrustDirectoryWidget { let options: Vec<(&str, TrustDirectorySelection)> = vec![ ( - if self.restricted { + if self.restricted && self.existing_task { + "Open existing task" + } else if self.restricted { "Open restricted" } else { "Trust and continue" @@ -210,6 +217,7 @@ mod tests { fn widget(error: Option) -> TrustDirectoryWidget { TrustDirectoryWidget { restricted: false, + existing_task: false, cwd: PathBuf::from("/workspace/project"), trust_target: PathBuf::from("/workspace/project"), show_windows_create_sandbox_hint: false, @@ -224,6 +232,7 @@ mod tests { fn release_event_does_not_change_selection() { let mut widget = TrustDirectoryWidget { restricted: false, + existing_task: false, cwd: PathBuf::from("."), trust_target: PathBuf::from("."), show_windows_create_sandbox_hint: false, @@ -285,6 +294,7 @@ mod tests { fn renders_snapshot_for_remote_git_subdirectory() { let widget = TrustDirectoryWidget { restricted: false, + existing_task: false, cwd: PathBuf::from("/srv/remote/project/nested"), trust_target: PathBuf::from("/srv/remote/project"), ..widget(/*error*/ None) @@ -309,16 +319,23 @@ mod tests { #[test] fn renders_restricted_folder() { - let widget = TrustDirectoryWidget { - restricted: true, - ..widget(/*error*/ None) - }; - let mut terminal = - Terminal::new(VT100Backend::new(/*width*/ 70, /*height*/ 18)).expect("terminal"); - terminal - .draw(|f| (&widget).render_ref(f.area(), f.buffer_mut())) - .expect("draw"); - insta::assert_snapshot!(terminal.backend()); + for existing_task in [false, true] { + let widget = TrustDirectoryWidget { + restricted: true, + existing_task, + ..widget(/*error*/ None) + }; + let mut terminal = + Terminal::new(VT100Backend::new(/*width*/ 70, /*height*/ 18)).expect("terminal"); + terminal + .draw(|f| (&widget).render_ref(f.area(), f.buffer_mut())) + .expect("draw"); + if existing_task { + insta::assert_snapshot!("existing_untrusted_task", terminal.backend()); + } else { + insta::assert_snapshot!(terminal.backend()); + } + } } #[test] diff --git a/codex-rs/tui/tests/suite/reconnect.rs b/codex-rs/tui/tests/suite/reconnect.rs index fb630a83a4..3b530d463c 100644 --- a/codex-rs/tui/tests/suite/reconnect.rs +++ b/codex-rs/tui/tests/suite/reconnect.rs @@ -81,6 +81,11 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re json!({"account": {"type": "apiKey"}, "requiresOpenaiAuth": false}) } "model/list" => json!({"data": [], "nextCursor": null}), + "config/read" => { + json!({"config": {"model": "gpt-5.6-terra", "model_provider": "openai", "projects": { + server_cwd.to_string_lossy(): {"trust_level": "trusted"} + }}, "origins": {}, "layers": []}) + } "configRequirements/read" => json!({"requirements": null}), "thread/start" | "thread/resume" => { json!({"thread": thread, "model": "gpt-5.6-terra", "modelProvider": "openai",