diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index ba946c67b653..65374a0d9830 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -280,21 +280,16 @@ fn project_config_warning(config: &Config) -> Option ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ true, ) { - if !matches!(layer.name, ConfigLayerSource::Project { .. }) - || layer.disabled_reason.is_none() - { + let ConfigLayerSource::Project { dot_codex_folder } = &layer.name else { continue; - } - if let ConfigLayerSource::Project { dot_codex_folder } = &layer.name { - disabled_folders.push(( - dot_codex_folder.as_path().display().to_string(), - layer - .disabled_reason - .as_ref() - .map(ToString::to_string) - .unwrap_or_else(|| "config.toml is disabled.".to_string()), - )); - } + }; + let Some(disabled_reason) = &layer.disabled_reason else { + continue; + }; + disabled_folders.push(( + dot_codex_folder.as_path().display().to_string(), + disabled_reason.clone(), + )); } if disabled_folders.is_empty() { @@ -302,8 +297,8 @@ fn project_config_warning(config: &Config) -> Option } let mut message = concat!( - "Project config.toml files are disabled in the following folders. ", - "Settings in those files are ignored, but skills and exec policies still load.\n", + "Project-local config, hooks, and exec policies are disabled in the following folders ", + "until the project is trusted, but skills still load.\n", ) .to_string(); for (index, (folder, reason)) in disabled_folders.iter().enumerate() { diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 508c77f7ffb8..9f212de14f10 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -21,6 +21,7 @@ use codex_app_server_protocol::ThreadStatus; use codex_app_server_protocol::ThreadStatusChangedNotification; use codex_config::types::AuthCredentialsStoreMode; use codex_core::config::set_project_trust_level; +use codex_core::config_loader::project_trust_key; use codex_exec_server::LOCAL_FS; use codex_git_utils::resolve_root_git_project_for_trust; use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; @@ -722,7 +723,8 @@ model_reasoning_effort = "high" let trusted_root = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &workspace_abs) .await .unwrap_or(workspace_abs); - assert!(config_toml.contains(&persisted_trust_path(trusted_root.as_path()))); + let trusted_root_key = project_trust_key(trusted_root.as_path()); + assert!(config_toml.contains(&trusted_root_key)); assert!(config_toml.contains("trust_level = \"trusted\"")); Ok(()) @@ -761,8 +763,10 @@ async fn thread_start_with_nested_git_cwd_trusts_repo_root() -> Result<()> { let trusted_root = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested_abs) .await .expect("git root should resolve"); - assert!(config_toml.contains(&persisted_trust_path(trusted_root.as_path()))); - assert!(!config_toml.contains(&persisted_trust_path(&nested))); + let trusted_root_key = project_trust_key(trusted_root.as_path()); + let nested_key = project_trust_key(&nested); + assert!(config_toml.contains(&trusted_root_key)); + assert!(!config_toml.contains(&nested_key)); Ok(()) } @@ -856,21 +860,6 @@ fn create_config_toml_without_approval_policy( ) } -fn persisted_trust_path(project_path: &Path) -> String { - let project_path = - std::fs::canonicalize(project_path).unwrap_or_else(|_| project_path.to_path_buf()); - let project_path = project_path.display().to_string(); - - if let Some(project_path) = project_path.strip_prefix(r"\\?\UNC\") { - return format!(r"\\{project_path}"); - } - - project_path - .strip_prefix(r"\\?\") - .unwrap_or(&project_path) - .to_string() -} - fn create_config_toml_with_optional_approval_policy( codex_home: &Path, server_uri: &str, diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 83fe6c88e749..6556ca73e363 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -684,25 +684,21 @@ impl ConfigToml { resolved_cwd: &Path, repo_root: Option<&Path>, ) -> Option { - let projects = self.projects.clone().unwrap_or_default(); + let projects = self.projects.as_ref()?; - let resolved_cwd_key = project_trust_key(resolved_cwd); - let resolved_cwd_raw_key = resolved_cwd.to_string_lossy().to_string(); - if let Some(project_config) = projects - .get(&resolved_cwd_key) - .or_else(|| projects.get(&resolved_cwd_raw_key)) - { - return Some(project_config.clone()); + for normalized_cwd in normalized_project_lookup_keys(resolved_cwd) { + if let Some(project_config) = project_config_for_lookup_key(projects, &normalized_cwd) { + return Some(project_config); + } } if let Some(repo_root) = repo_root { - let repo_root_key = project_trust_key(repo_root); - let repo_root_raw_key = repo_root.to_string_lossy().to_string(); - if let Some(project_config_for_root) = projects - .get(&repo_root_key) - .or_else(|| projects.get(&repo_root_raw_key)) - { - return Some(project_config_for_root.clone()); + for normalized_repo_root in normalized_project_lookup_keys(repo_root) { + if let Some(project_config_for_root) = + project_config_for_lookup_key(projects, &normalized_repo_root) + { + return Some(project_config_for_root); + } } } @@ -734,11 +730,45 @@ impl ConfigToml { /// Canonicalize the path and convert it to a string to be used as a key in the /// projects trust map. On Windows, strips UNC, when possible, to try to ensure /// that different paths that point to the same location have the same key. -fn project_trust_key(project_path: &Path) -> String { - normalize_for_path_comparison(project_path) - .unwrap_or_else(|_| project_path.to_path_buf()) - .to_string_lossy() - .to_string() +fn normalized_project_lookup_keys(path: &Path) -> Vec { + let normalized_path = normalize_project_lookup_key(path.to_string_lossy().to_string()); + let normalized_canonical_path = normalize_project_lookup_key( + normalize_for_path_comparison(path) + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .to_string(), + ); + if normalized_path == normalized_canonical_path { + vec![normalized_canonical_path] + } else { + vec![normalized_canonical_path, normalized_path] + } +} + +fn normalize_project_lookup_key(key: String) -> String { + if cfg!(windows) { + key.to_ascii_lowercase() + } else { + key + } +} + +fn project_config_for_lookup_key( + projects: &HashMap, + lookup_key: &str, +) -> Option { + if let Some(project_config) = projects.get(lookup_key) { + return Some(project_config.clone()); + } + + let mut normalized_matches: Vec<_> = projects + .iter() + .filter(|(key, _)| normalize_project_lookup_key((*key).clone()) == lookup_key) + .collect(); + normalized_matches.sort_by(|(left, _), (right, _)| left.cmp(right)); + normalized_matches + .first() + .map(|(_, project_config)| (**project_config).clone()) } pub fn validate_reserved_model_provider_ids( diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 6e721b1238c7..bc65aac61c7c 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -4,6 +4,7 @@ use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config::edit::apply_blocking; use crate::config_loader::RequirementSource; +use crate::config_loader::project_trust_key; use crate::plugins::PluginsManager; use assert_matches::assert_matches; use codex_config::CONFIG_TOML_FILE; @@ -5449,7 +5450,9 @@ model = "foo""#; // Since we created the [projects] table as part of migration, it is kept implicit. // Expect explicit per-project tables, preserving prior entries and appending the new one. - let expected = r#"toplevel = "baz" + let new_project_key = project_trust_key(new_project); + let expected = format!( + r#"toplevel = "baz" model = "foo" [projects."/Users/mbolin/code/codex4"] @@ -5459,14 +5462,42 @@ foo = "bar" [projects."/Users/mbolin/code/codex3"] trust_level = "trusted" -[projects."/Users/mbolin/code/codex2"] +[projects."{new_project_key}"] trust_level = "trusted" -"#; +"# + ); assert_eq!(contents, expected); Ok(()) } +#[cfg(unix)] +#[tokio::test] +async fn active_project_does_not_match_configured_alias_for_canonical_cwd() -> anyhow::Result<()> { + let tmp = tempdir()?; + let project_root = tmp.path().join("project"); + let alias_root = tmp.path().join("project_alias"); + std::fs::create_dir_all(&project_root)?; + std::os::unix::fs::symlink(&project_root, &alias_root)?; + + let config = ConfigToml { + projects: Some(HashMap::from([( + alias_root.to_string_lossy().to_string(), + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + ..Default::default() + }; + + assert_eq!( + config.get_active_project(&project_root, /*repo_root*/ None), + None + ); + + Ok(()) +} + #[test] fn test_set_default_oss_provider() -> std::io::Result<()> { let temp_dir = TempDir::new()?; diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 9f9d83866de9..9eaeb7149cb1 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -561,7 +561,9 @@ async fn load_requirements_from_legacy_scheme( struct ProjectTrustContext { project_root: AbsolutePathBuf, project_root_key: String, + project_root_lookup_keys: Vec, repo_root_key: Option, + repo_root_lookup_keys: Option>, projects_trust: std::collections::HashMap, user_config_file: AbsolutePathBuf, } @@ -584,28 +586,39 @@ impl ProjectTrustDecision { impl ProjectTrustContext { fn decision_for_dir(&self, dir: &AbsolutePathBuf) -> ProjectTrustDecision { - let dir_key = project_trust_key(dir.as_path()); - if let Some(trust_level) = self.projects_trust.get(&dir_key).copied() { - return ProjectTrustDecision { - trust_level: Some(trust_level), - trust_key: dir_key, - }; + for dir_key in normalized_project_trust_keys(dir.as_path()) { + if let Some((trust_key, trust_level)) = + project_trust_for_lookup_key(&self.projects_trust, &dir_key) + { + return ProjectTrustDecision { + trust_level: Some(trust_level), + trust_key, + }; + } } - if let Some(trust_level) = self.projects_trust.get(&self.project_root_key).copied() { - return ProjectTrustDecision { - trust_level: Some(trust_level), - trust_key: self.project_root_key.clone(), - }; + for project_root_key in &self.project_root_lookup_keys { + if let Some((trust_key, trust_level)) = + project_trust_for_lookup_key(&self.projects_trust, project_root_key) + { + return ProjectTrustDecision { + trust_level: Some(trust_level), + trust_key, + }; + } } - if let Some(repo_root_key) = self.repo_root_key.as_ref() - && let Some(trust_level) = self.projects_trust.get(repo_root_key).copied() - { - return ProjectTrustDecision { - trust_level: Some(trust_level), - trust_key: repo_root_key.clone(), - }; + if let Some(repo_root_lookup_keys) = self.repo_root_lookup_keys.as_ref() { + for repo_root_key in repo_root_lookup_keys { + if let Some((trust_key, trust_level)) = + project_trust_for_lookup_key(&self.projects_trust, repo_root_key) + { + return ProjectTrustDecision { + trust_level: Some(trust_level), + trust_key, + }; + } + } } ProjectTrustDecision { @@ -617,37 +630,35 @@ impl ProjectTrustContext { } } - fn disabled_reason_for_dir(&self, dir: &AbsolutePathBuf) -> Option { - let decision = self.decision_for_dir(dir); + fn disabled_reason_for_decision(&self, decision: &ProjectTrustDecision) -> Option { if decision.is_trusted() { return None; } + let gated_features = "project-local config, hooks, and exec policies"; let trust_key = decision.trust_key.as_str(); let user_config_file = self.user_config_file.as_path().display(); match decision.trust_level { Some(TrustLevel::Untrusted) => Some(format!( - "{trust_key} is marked as untrusted in {user_config_file}. To load config.toml, mark it trusted." + "{trust_key} is marked as untrusted in {user_config_file}. To load {gated_features}, mark it trusted." )), _ => Some(format!( - "To load config.toml, add {trust_key} as a trusted project in {user_config_file}." + "To load {gated_features}, add {trust_key} as a trusted project in {user_config_file}." )), } } } fn project_layer_entry( - trust_context: &ProjectTrustContext, dot_codex_folder: &AbsolutePathBuf, - layer_dir: &AbsolutePathBuf, config: TomlValue, - config_toml_exists: bool, + disabled_reason: Option, ) -> ConfigLayerEntry { let source = ConfigLayerSource::Project { dot_codex_folder: dot_codex_folder.clone(), }; - if config_toml_exists && let Some(reason) = trust_context.disabled_reason_for_dir(layer_dir) { + if let Some(reason) = disabled_reason { ConfigLayerEntry::new_disabled(source, config, reason) } else { ConfigLayerEntry::new(source, config) @@ -673,25 +684,30 @@ async fn project_trust_context( let project_root = find_project_root(fs, cwd, project_root_markers).await?; let projects = project_trust_config.projects.unwrap_or_default(); - let project_root_key = project_trust_key(project_root.as_path()); + let project_root_lookup_keys = normalized_project_trust_keys(project_root.as_path()); + let project_root_key = project_root_lookup_keys + .first() + .cloned() + .unwrap_or_else(|| project_trust_key(project_root.as_path())); let repo_root = resolve_root_git_project_for_trust(fs, cwd).await; - let repo_root_key = repo_root + let repo_root_lookup_keys = repo_root .as_ref() - .map(|root| project_trust_key(root.as_path())); + .map(|root| normalized_project_trust_keys(root.as_path())); + let repo_root_key = repo_root_lookup_keys + .as_ref() + .and_then(|keys| keys.first().cloned()); let projects_trust = projects .into_iter() - .filter_map(|(key, project)| { - project - .trust_level - .map(|trust_level| (project_trust_key(Path::new(&key)), trust_level)) - }) + .filter_map(|(key, project)| project.trust_level.map(|trust_level| (key, trust_level))) .collect(); Ok(ProjectTrustContext { project_root, project_root_key, + project_root_lookup_keys, repo_root_key, + repo_root_lookup_keys, projects_trust, user_config_file: user_config_file.clone(), }) @@ -700,13 +716,52 @@ async fn project_trust_context( /// Canonicalize the path and convert it to a string to be used as a key in the /// projects trust map. On Windows, strips UNC, when possible, to try to ensure /// that different paths that point to the same location have the same key. -pub fn project_trust_key(project_path: &Path) -> String { - normalize_path(project_path) - .unwrap_or_else(|_| project_path.to_path_buf()) - .to_string_lossy() - .to_string() +pub fn project_trust_key(path: &Path) -> String { + normalized_project_trust_keys(path) + .into_iter() + .next() + .unwrap_or_else(|| normalize_project_trust_lookup_key(path.to_string_lossy().to_string())) } +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) + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .to_string(), + ); + if normalized_path == normalized_canonical_path { + vec![normalized_canonical_path] + } else { + vec![normalized_canonical_path, normalized_path] + } +} + +fn normalize_project_trust_lookup_key(key: String) -> String { + if cfg!(windows) { + key.to_ascii_lowercase() + } else { + key + } +} +fn project_trust_for_lookup_key( + projects_trust: &std::collections::HashMap, + lookup_key: &str, +) -> Option<(String, TrustLevel)> { + if let Some(trust_level) = projects_trust.get(lookup_key).copied() { + return Some((lookup_key.to_string(), trust_level)); + } + + let mut normalized_matches: Vec<_> = projects_trust + .iter() + .filter(|(key, _)| normalize_project_trust_lookup_key((*key).clone()) == lookup_key) + .collect(); + normalized_matches.sort_by(|(left, _), (right, _)| left.cmp(right)); + normalized_matches + .first() + .map(|(key, trust_level)| ((**key).clone(), **trust_level)) +} /// Takes a `toml::Value` parsed from a config.toml file and walks through it, /// resolving any `AbsolutePathBuf` fields against `base_dir`, returning a new /// `toml::Value` with the same shape but with paths resolved. @@ -834,6 +889,7 @@ async fn load_project_layers( } let decision = trust_context.decision_for_dir(&dir); + let disabled_reason = trust_context.disabled_reason_for_decision(&decision); let dot_codex_normalized = normalize_path(dot_codex_abs.as_path()).unwrap_or_else(|_| dot_codex_abs.to_path_buf()); if dot_codex_abs == codex_home_abs || dot_codex_normalized == codex_home_normalized { @@ -855,24 +911,16 @@ async fn load_project_layers( )); } layers.push(project_layer_entry( - trust_context, &dot_codex_abs, - &dir, TomlValue::Table(toml::map::Map::new()), - /*config_toml_exists*/ true, + disabled_reason.clone(), )); continue; } }; let config = resolve_relative_paths_in_config_toml(config, dot_codex_abs.as_path())?; - let entry = project_layer_entry( - trust_context, - &dot_codex_abs, - &dir, - config, - /*config_toml_exists*/ true, - ); + let entry = project_layer_entry(&dot_codex_abs, config, disabled_reason.clone()); layers.push(entry); } Err(err) => { @@ -881,11 +929,9 @@ async fn load_project_layers( // for this project layer, as this may still have subfolders // that are significant in the overall ConfigLayerStack. layers.push(project_layer_entry( - trust_context, &dot_codex_abs, - &dir, TomlValue::Table(toml::map::Map::new()), - /*config_toml_exists*/ false, + disabled_reason, )); } else { let config_file_display = config_file.as_path().display(); @@ -900,7 +946,6 @@ async fn load_project_layers( Ok(layers) } - /// The legacy mechanism for specifying admin-enforced configuration is to read /// from a file like `/etc/codex/managed_config.toml` that has the same /// structure as `config.toml` where fields like `approval_policy` can specify diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index eb533c4ca909..a8f4eda8ec04 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -1343,6 +1343,66 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< Ok(()) } +#[cfg(unix)] +#[tokio::test] +async fn project_trust_does_not_match_configured_alias_for_canonical_cwd() -> std::io::Result<()> { + let tmp = tempdir()?; + let project_root = tmp.path().join("project"); + let alias_root = tmp.path().join("project_alias"); + tokio::fs::create_dir_all(project_root.join(".codex")).await?; + tokio::fs::write(project_root.join(".git"), "gitdir: here").await?; + tokio::fs::write( + project_root.join(".codex").join(CONFIG_TOML_FILE), + "foo = \"project\"\n", + ) + .await?; + std::os::unix::fs::symlink(&project_root, &alias_root)?; + + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + tokio::fs::write( + codex_home.join(CONFIG_TOML_FILE), + toml::to_string(&ConfigToml { + projects: Some(HashMap::from([( + alias_root.to_string_lossy().to_string(), + ProjectConfig { + trust_level: Some(TrustLevel::Trusted), + }, + )])), + ..Default::default() + }) + .expect("serialize config"), + ) + .await?; + + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(AbsolutePathBuf::from_absolute_path(&project_root)?), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + ) + .await?; + + let project_layers: Vec<_> = layers + .get_layers( + super::ConfigLayerStackOrdering::HighestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. })) + .collect(); + assert_eq!(project_layers.len(), 1); + assert!( + project_layers[0].disabled_reason.is_some(), + "configured aliases must not collapse into the canonical project key" + ); + assert_eq!(layers.effective_config().get("foo"), None); + + Ok(()) +} + #[tokio::test] async fn cli_override_can_update_project_local_mcp_server_when_project_is_trusted() -> std::io::Result<()> { @@ -1507,6 +1567,71 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io:: Ok(()) } +#[tokio::test] +async fn project_layer_without_config_toml_is_disabled_when_untrusted_or_unknown() +-> std::io::Result<()> { + let tmp = tempdir()?; + let project_root = tmp.path().join("project"); + let nested = project_root.join("child"); + tokio::fs::create_dir_all(nested.join(".codex")).await?; + tokio::fs::write(project_root.join(".git"), "gitdir: here").await?; + + let cwd = AbsolutePathBuf::from_absolute_path(&nested)?; + let cases = [ + ("untrusted", Some(TrustLevel::Untrusted), true), + ("unknown", None, true), + ("trusted", Some(TrustLevel::Trusted), false), + ]; + + for (name, trust_level, expect_disabled) in cases { + let codex_home = tmp.path().join(format!("home_no_config_{name}")); + tokio::fs::create_dir_all(&codex_home).await?; + if let Some(trust_level) = trust_level { + make_config_for_test( + &codex_home, + &project_root, + trust_level, + /*project_root_markers*/ None, + ) + .await?; + } + + let layers = load_config_layers_state( + LOCAL_FS.as_ref(), + &codex_home, + Some(cwd.clone()), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + ) + .await?; + let project_layers: Vec<_> = layers + .get_layers( + super::ConfigLayerStackOrdering::HighestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. })) + .collect(); + assert_eq!( + project_layers.len(), + 1, + "expected one project layer for {name}" + ); + assert_eq!( + project_layers[0].disabled_reason.is_some(), + expect_disabled, + "unexpected disabled state for {name}", + ); + assert_eq!( + project_layers[0].config, + TomlValue::Table(toml::map::Map::new()) + ); + } + + Ok(()) +} + #[tokio::test] async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io::Result<()> { let tmp = tempdir()?; diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 798bacfbf348..8f0f076f0886 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -495,6 +495,8 @@ async fn load_exec_policy_with_warning( } pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result { + // Disabled project layers already represent the trust decision, so hooks + // and exec-policy loading can reuse the normal trusted-layer view. // Iterate the layers in increasing order of precedence, adding the *.rules // from each layer, so that higher-precedence layers can override // rules defined in lower-precedence ones. diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index f17869205d17..64deca6bc67f 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -10,6 +10,9 @@ use crate::config_loader::RequirementSource; use crate::config_loader::Sourced; use codex_app_server_protocol::ConfigLayerSource; use codex_config::RequirementsExecPolicy; +use codex_config::config_toml::ConfigToml; +use codex_config::config_toml::ProjectConfig; +use codex_protocol::config_types::TrustLevel; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; @@ -67,6 +70,33 @@ fn starlark_string(value: &str) -> String { value.replace('\\', "\\\\").replace('"', "\\\"") } +async fn write_project_trust_config( + codex_home: &Path, + trusted_projects: &[(&Path, TrustLevel)], +) -> std::io::Result<()> { + tokio::fs::write( + codex_home.join(codex_config::CONFIG_TOML_FILE), + toml::to_string(&ConfigToml { + projects: Some( + trusted_projects + .iter() + .map(|(project, trust_level)| { + ( + project.to_string_lossy().to_string(), + ProjectConfig { + trust_level: Some(*trust_level), + }, + ) + }) + .collect::>(), + ), + ..Default::default() + }) + .expect("serialize config"), + ) + .await +} + fn read_only_file_system_sandbox_policy() -> FileSystemSandboxPolicy { FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Special { @@ -1756,3 +1786,159 @@ async fn assert_exec_approval_requirement_for_command( assert_eq!(requirement, expected_requirement); } + +#[tokio::test] +async fn exec_policies_only_load_from_trusted_project_layers() -> std::io::Result<()> { + let temp = tempfile::tempdir()?; + let codex_home = temp.path().join("home_execpolicy_nested"); + let project_root = temp.path().join("project_execpolicy_nested"); + let nested = project_root.join("nested"); + let root_rules = project_root.join(".codex").join(RULES_DIR_NAME); + let nested_rules = nested.join(".codex").join(RULES_DIR_NAME); + + fs::create_dir_all(&codex_home)?; + fs::create_dir_all(&nested_rules)?; + fs::write(project_root.join(".git"), "gitdir: here")?; + fs::create_dir_all(&root_rules)?; + fs::write( + root_rules.join("deny-rm.rules"), + r#"prefix_rule(pattern=["rm"], decision="forbidden")"#, + )?; + fs::write( + nested_rules.join("deny-mv.rules"), + r#"prefix_rule(pattern=["mv"], decision="forbidden")"#, + )?; + write_project_trust_config(&codex_home, &[(&nested, TrustLevel::Trusted)]).await?; + + let config = ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(nested)) + .build() + .await?; + + let policy = load_exec_policy(&config.config_layer_stack) + .await + .map_err(std::io::Error::other)?; + assert_eq!( + policy + .check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow) + .decision, + Decision::Allow, + ); + assert_eq!( + policy + .check_multiple([vec!["mv".to_string()]].iter(), &|_| Decision::Allow) + .decision, + Decision::Forbidden, + ); + + Ok(()) +} + +#[tokio::test] +async fn exec_policies_require_project_trust_without_config_toml() -> std::io::Result<()> { + let temp = tempfile::tempdir()?; + let project_root = temp.path().join("project_execpolicy"); + let nested = project_root.join("nested"); + let rules_dir = project_root.join(".codex").join(RULES_DIR_NAME); + fs::create_dir_all(&nested)?; + fs::write(project_root.join(".git"), "gitdir: here")?; + fs::create_dir_all(&rules_dir)?; + fs::write( + rules_dir.join("deny-rm.rules"), + r#"prefix_rule(pattern=["rm"], decision="forbidden")"#, + )?; + + let cases = [ + ( + "unknown", + Vec::<(&Path, TrustLevel)>::new(), + Decision::Allow, + ), + ( + "untrusted", + vec![(&project_root as &Path, TrustLevel::Untrusted)], + Decision::Allow, + ), + ( + "trusted", + vec![(&project_root as &Path, TrustLevel::Trusted)], + Decision::Forbidden, + ), + ]; + + for (name, trust_entries, expected_decision) in cases { + let codex_home = temp.path().join(format!("home_execpolicy_{name}")); + fs::create_dir_all(&codex_home)?; + write_project_trust_config(&codex_home, &trust_entries).await?; + + let config = ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(nested.clone())) + .build() + .await?; + + let policy = load_exec_policy(&config.config_layer_stack) + .await + .map_err(std::io::Error::other)?; + assert_eq!( + policy + .check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow) + .decision, + expected_decision, + "unexpected execpolicy decision for {name}", + ); + } + + Ok(()) +} + +#[tokio::test] +async fn exec_policy_warnings_ignore_untrusted_project_rules_without_config_toml() +-> std::io::Result<()> { + let temp = tempfile::tempdir()?; + let project_root = temp.path().join("project_execpolicy_warning"); + let nested = project_root.join("nested"); + let rules_dir = project_root.join(".codex").join(RULES_DIR_NAME); + fs::create_dir_all(&nested)?; + fs::write(project_root.join(".git"), "gitdir: here")?; + fs::create_dir_all(&rules_dir)?; + fs::write(rules_dir.join("broken.rules"), "prefix_rule(")?; + + let cases = [ + ("unknown", Vec::<(&Path, TrustLevel)>::new(), false), + ( + "untrusted", + vec![(&project_root as &Path, TrustLevel::Untrusted)], + false, + ), + ( + "trusted", + vec![(&project_root as &Path, TrustLevel::Trusted)], + true, + ), + ]; + + for (name, trust_entries, expect_warning) in cases { + let codex_home = temp.path().join(format!("home_execpolicy_warning_{name}")); + fs::create_dir_all(&codex_home)?; + write_project_trust_config(&codex_home, &trust_entries).await?; + + let config = ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(nested.clone())) + .build() + .await?; + + let warning = check_execpolicy_for_warnings(&config.config_layer_stack) + .await + .map_err(std::io::Error::other)?; + assert_eq!( + matches!(warning, Some(ExecPolicyError::ParsePolicy { .. })), + expect_warning, + "unexpected execpolicy warning state for {name}", + ); + } + + Ok(()) +} diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index abe96c5490af..60183e986cd8 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8,6 +8,7 @@ use crate::config_loader::NetworkDomainPermissionToml; use crate::config_loader::NetworkDomainPermissionsToml; use crate::config_loader::RequirementSource; use crate::config_loader::Sourced; +use crate::config_loader::project_trust_key; use crate::exec::ExecCapturePolicy; use crate::function_tool::FunctionCallError; use crate::shell::default_user_shell; @@ -21,6 +22,7 @@ use codex_models_manager::bundled_models_response; use codex_models_manager::model_info; use codex_protocol::AgentPath; use codex_protocol::ThreadId; +use codex_protocol::config_types::TrustLevel; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; @@ -53,6 +55,8 @@ use crate::tools::registry::ToolHandler; use crate::tools::router::ToolCallSource; use crate::turn_diff_tracker::TurnDiffTracker; use codex_app_server_protocol::AppInfo; +use codex_config::config_toml::ConfigToml; +use codex_config::config_toml::ProjectConfig; use codex_execpolicy::Decision; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; @@ -303,6 +307,75 @@ fn user_input_texts(items: &[ResponseItem]) -> Vec<&str> { .collect() } +fn write_project_hooks(dot_codex: &Path) -> std::io::Result<()> { + std::fs::create_dir_all(dot_codex)?; + std::fs::write( + dot_codex.join("hooks.json"), + r#"{ + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "echo hello from hook" + } + ] + } + ] + } +}"#, + ) +} + +async fn write_project_trust_config( + codex_home: &Path, + trusted_projects: &[(&Path, TrustLevel)], +) -> std::io::Result<()> { + tokio::fs::write( + codex_home.join(codex_config::CONFIG_TOML_FILE), + toml::to_string(&ConfigToml { + projects: Some( + trusted_projects + .iter() + .map(|(project, trust_level)| { + ( + project_trust_key(project), + ProjectConfig { + trust_level: Some(*trust_level), + }, + ) + }) + .collect::>(), + ), + ..Default::default() + }) + .expect("serialize config"), + ) + .await +} + +async fn preview_session_start_hooks( + config: &crate::config::Config, +) -> std::io::Result> { + let hooks = Hooks::new(HooksConfig { + feature_enabled: true, + config_layer_stack: Some(config.config_layer_stack.clone()), + ..HooksConfig::default() + }); + + Ok( + hooks.preview_session_start(&codex_hooks::SessionStartRequest { + session_id: ThreadId::new(), + cwd: config.cwd.clone(), + transcript_path: None, + model: "gpt-5".to_string(), + permission_mode: "default".to_string(), + source: codex_hooks::SessionStartSource::Startup, + }), + ) +} + fn test_tool_runtime(session: Arc, turn_context: Arc) -> ToolCallRuntime { let router = Arc::new(ToolRouter::from_config( &turn_context.tools_config, @@ -6044,3 +6117,85 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() pretty_assertions::assert_eq!(output, expected); } + +#[tokio::test] +async fn session_start_hooks_only_load_from_trusted_project_layers() -> std::io::Result<()> { + let temp = tempfile::tempdir()?; + let codex_home = temp.path().join("home"); + let project_root = temp.path().join("project"); + let nested = project_root.join("nested"); + let root_dot_codex = project_root.join(".codex"); + let nested_dot_codex = nested.join(".codex"); + + std::fs::create_dir_all(&codex_home)?; + std::fs::create_dir_all(&nested_dot_codex)?; + std::fs::write(project_root.join(".git"), "gitdir: here")?; + write_project_hooks(&root_dot_codex)?; + write_project_hooks(&nested_dot_codex)?; + write_project_trust_config(&codex_home, &[(&nested, TrustLevel::Trusted)]).await?; + + let config = ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(nested)) + .build() + .await?; + + let preview = preview_session_start_hooks(&config).await?; + let expected_source_path = codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path( + nested_dot_codex.join("hooks.json"), + )?; + assert_eq!( + preview + .iter() + .map(|run| &run.source_path) + .collect::>(), + vec![&expected_source_path], + ); + + Ok(()) +} + +#[tokio::test] +async fn session_start_hooks_require_project_trust_without_config_toml() -> std::io::Result<()> { + let temp = tempfile::tempdir()?; + let project_root = temp.path().join("project"); + let nested = project_root.join("nested"); + let dot_codex = project_root.join(".codex"); + std::fs::create_dir_all(&nested)?; + std::fs::write(project_root.join(".git"), "gitdir: here")?; + write_project_hooks(&dot_codex)?; + + let cases = [ + ("unknown", Vec::<(&Path, TrustLevel)>::new(), 0_usize), + ( + "untrusted", + vec![(&project_root as &Path, TrustLevel::Untrusted)], + 0_usize, + ), + ( + "trusted", + vec![(&project_root as &Path, TrustLevel::Trusted)], + 1_usize, + ), + ]; + + for (name, trust_entries, expected_hooks) in cases { + let codex_home = temp.path().join(format!("home_{name}")); + std::fs::create_dir_all(&codex_home)?; + write_project_trust_config(&codex_home, &trust_entries).await?; + + let config = ConfigBuilder::default() + .codex_home(codex_home) + .fallback_cwd(Some(nested.clone())) + .build() + .await?; + + assert_eq!( + preview_session_start_hooks(&config).await?.len(), + expected_hooks, + "unexpected hook count for {name}", + ); + } + + Ok(()) +} diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7254272138f8..0f801eb5f474 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -479,16 +479,12 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) let ConfigLayerSource::Project { dot_codex_folder } = &layer.name else { continue; }; - if layer.disabled_reason.is_none() { + let Some(disabled_reason) = &layer.disabled_reason else { continue; - } + }; disabled_folders.push(( dot_codex_folder.as_path().display().to_string(), - layer - .disabled_reason - .as_ref() - .map(ToString::to_string) - .unwrap_or_else(|| "config.toml is disabled.".to_string()), + disabled_reason.clone(), )); } @@ -497,8 +493,8 @@ fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) } let mut message = concat!( - "Project config.toml files are disabled in the following folders. ", - "Settings in those files are ignored, but skills and exec policies still load.\n", + "Project-local config, hooks, and exec policies are disabled in the following folders ", + "until the project is trusted, but skills still load.\n", ) .to_string(); for (index, (folder, reason)) in disabled_folders.iter().enumerate() { diff --git a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap index 3c8c248c36ee..0ffd8d35ca83 100644 --- a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap +++ b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap @@ -5,7 +5,9 @@ expression: terminal.backend() > You are in /workspace/project Do you trust the contents of this directory? Working with untrusted - contents comes with higher risk of prompt injection. + contents comes with higher risk of prompt injection. Trusting the + directory allows project-local config, hooks, and exec policies to + load. › 1. Yes, continue 2. No, quit diff --git a/codex-rs/tui/src/onboarding/trust_directory.rs b/codex-rs/tui/src/onboarding/trust_directory.rs index e9f2cd58bdc4..f04e9f9af97d 100644 --- a/codex-rs/tui/src/onboarding/trust_directory.rs +++ b/codex-rs/tui/src/onboarding/trust_directory.rs @@ -53,10 +53,15 @@ impl WidgetRef for &TrustDirectoryWidget { column.push( Paragraph::new( - "Do you trust the contents of this directory? Working with untrusted contents comes with higher risk of prompt injection.".to_string(), + "Do you trust the contents of this directory? Working with untrusted \ + contents comes with higher risk of prompt injection. Trusting the \ + directory allows project-local config, hooks, and exec policies to load." + .to_string(), ) - .wrap(Wrap { trim: true }) - .inset(Insets::tlbr(/*top*/ 0, /*left*/ 2, /*bottom*/ 0, /*right*/ 0)), + .wrap(Wrap { trim: true }) + .inset(Insets::tlbr( + /*top*/ 0, /*left*/ 2, /*bottom*/ 0, /*right*/ 0, + )), ); column.push("");