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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ pub async fn load_config_as_toml_with_cli_overrides(
Ok(cfg)
}

fn deserialize_config_toml_with_base(
pub(crate) fn deserialize_config_toml_with_base(
root_value: TomlValue,
config_base_dir: &Path,
) -> std::io::Result<ConfigToml> {
Expand Down
70 changes: 61 additions & 9 deletions codex-rs/core/src/config_loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@ mod tests;

use crate::config::CONFIG_TOML_FILE;
use crate::config::ConfigToml;
use crate::config::deserialize_config_toml_with_base;
use crate::config_loader::config_requirements::ConfigRequirementsWithSources;
use crate::config_loader::layer_io::LoadedConfigLayers;
use crate::git_info::resolve_root_git_project_for_trust;
use codex_app_server_protocol::ConfigLayerSource;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::protocol::AskForApproval;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::AbsolutePathBufGuard;
Expand Down Expand Up @@ -64,9 +67,9 @@ const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"];
/// - admin: managed preferences (*)
/// - system `/etc/codex/config.toml`
/// - user `${CODEX_HOME}/config.toml`
/// - cwd `${PWD}/config.toml`
/// - tree parent directories up to root looking for `./.codex/config.toml`
/// - repo `$(git rev-parse --show-toplevel)/.codex/config.toml`
/// - cwd `${PWD}/config.toml` (only when the directory is trusted)
/// - tree parent directories up to root looking for `./.codex/config.toml` (trusted only)
/// - repo `$(git rev-parse --show-toplevel)/.codex/config.toml` (trusted only)
/// - runtime e.g., --config flags, model selector in UI
///
/// (*) Only available on macOS via managed device profiles.
Expand Down Expand Up @@ -114,6 +117,12 @@ pub async fn load_config_layers_state(

let mut layers = Vec::<ConfigLayerEntry>::new();

let cli_overrides_layer = if cli_overrides.is_empty() {
None
} else {
Some(overrides::build_cli_overrides_layer(cli_overrides))
};

// Include an entry for the "system" config folder, loading its config.toml,
// if it exists.
let system_config_toml_file = if cfg!(unix) {
Expand Down Expand Up @@ -158,17 +167,22 @@ pub async fn load_config_layers_state(
for layer in &layers {
merge_toml_values(&mut merged_so_far, &layer.config);
}
if let Some(cli_overrides_layer) = cli_overrides_layer.as_ref() {
merge_toml_values(&mut merged_so_far, cli_overrides_layer);
}

let project_root_markers = project_root_markers_from_config(&merged_so_far)?
.unwrap_or_else(default_project_root_markers);

let project_root = find_project_root(&cwd, &project_root_markers).await?;
let project_layers = load_project_layers(&cwd, &project_root).await?;
layers.extend(project_layers);
if let Some(project_root) =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should send a warning to the user instead of silently ignoring

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will print a warning iff you have CWD / git repo codex config AND the folder is untrusted (will do in a follow up)

trusted_project_root(&merged_so_far, &cwd, &project_root_markers, codex_home).await?
{
let project_layers = load_project_layers(&cwd, &project_root).await?;
layers.extend(project_layers);
}
}

// Add a layer for runtime overrides from the CLI or UI, if any exist.
if !cli_overrides.is_empty() {
let cli_overrides_layer = overrides::build_cli_overrides_layer(cli_overrides);
if let Some(cli_overrides_layer) = cli_overrides_layer {
layers.push(ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
cli_overrides_layer,
Expand Down Expand Up @@ -388,6 +402,44 @@ fn default_project_root_markers() -> Vec<String> {
.collect()
}

async fn trusted_project_root(
merged_config: &TomlValue,
cwd: &AbsolutePathBuf,
project_root_markers: &[String],
config_base_dir: &Path,
) -> io::Result<Option<AbsolutePathBuf>> {
let config_toml = deserialize_config_toml_with_base(merged_config.clone(), config_base_dir)?;

let project_root = find_project_root(cwd, project_root_markers).await?;
let projects = config_toml.projects.unwrap_or_default();

let cwd_key = cwd.as_path().to_string_lossy().to_string();
let project_root_key = project_root.as_path().to_string_lossy().to_string();
let repo_root_key = resolve_root_git_project_for_trust(cwd.as_path())
.map(|root| root.to_string_lossy().to_string());

let trust_level = projects
.get(&cwd_key)
.and_then(|project| project.trust_level)
.or_else(|| {
projects
.get(&project_root_key)
.and_then(|project| project.trust_level)
})
.or_else(|| {
repo_root_key
.as_ref()
.and_then(|root| projects.get(root))
.and_then(|project| project.trust_level)
});

if matches!(trust_level, Some(TrustLevel::Trusted)) {
Ok(Some(project_root))
} else {
Ok(None)
}
}

/// 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.
Expand Down
130 changes: 125 additions & 5 deletions codex-rs/core/src/config_loader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,47 @@ use super::load_config_layers_state;
use crate::config::CONFIG_TOML_FILE;
use crate::config::ConfigBuilder;
use crate::config::ConfigOverrides;
use crate::config::ConfigToml;
use crate::config::ProjectConfig;
use crate::config_loader::ConfigLayerEntry;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::config_requirements::ConfigRequirementsWithSources;
use crate::config_loader::fingerprint::version_for_toml;
use crate::config_loader::load_requirements_toml;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::protocol::AskForApproval;
#[cfg(target_os = "macos")]
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::path::Path;
use tempfile::tempdir;
use toml::Value as TomlValue;

async fn make_config_for_test(
codex_home: &Path,
project_path: &Path,
trust_level: TrustLevel,
project_root_markers: Option<Vec<String>>,
) -> std::io::Result<()> {
tokio::fs::write(
codex_home.join(CONFIG_TOML_FILE),
toml::to_string(&ConfigToml {
projects: Some(HashMap::from([(
project_path.to_string_lossy().to_string(),
ProjectConfig {
trust_level: Some(trust_level),
},
)])),
project_root_markers,
..Default::default()
})
.expect("serialize config"),
)
.await
}

#[tokio::test]
async fn merges_managed_config_layer_on_top() {
let tmp = tempdir().expect("tempdir");
Expand Down Expand Up @@ -365,6 +393,7 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> {

let codex_home = tmp.path().join("home");
tokio::fs::create_dir_all(&codex_home).await?;
make_config_for_test(&codex_home, &project_root, TrustLevel::Trusted, None).await?;
let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;
let layers = load_config_layers_state(
&codex_home,
Expand Down Expand Up @@ -429,6 +458,7 @@ experimental_instructions_file = "child.txt"

let codex_home = tmp.path().join("home");
tokio::fs::create_dir_all(&codex_home).await?;
make_config_for_test(&codex_home, &project_root, TrustLevel::Trusted, None).await?;

let config = ConfigBuilder::default()
.codex_home(codex_home)
Expand Down Expand Up @@ -458,6 +488,7 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s

let codex_home = tmp.path().join("home");
tokio::fs::create_dir_all(&codex_home).await?;
make_config_for_test(&codex_home, &project_root, TrustLevel::Trusted, None).await?;
let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;
let layers = load_config_layers_state(
&codex_home,
Expand Down Expand Up @@ -486,6 +517,95 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s
Ok(())
}

#[tokio::test]
async fn project_layers_skipped_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(
nested.join(".codex").join(CONFIG_TOML_FILE),
"foo = \"child\"\n",
)
.await?;

let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;

let codex_home_untrusted = tmp.path().join("home_untrusted");
tokio::fs::create_dir_all(&codex_home_untrusted).await?;
make_config_for_test(
&codex_home_untrusted,
&project_root,
TrustLevel::Untrusted,
None,
)
.await?;

let layers_untrusted = load_config_layers_state(
&codex_home_untrusted,
Some(cwd.clone()),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
)
.await?;
let project_layers_untrusted = layers_untrusted
.layers_high_to_low()
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.count();
assert_eq!(project_layers_untrusted, 0);
assert_eq!(layers_untrusted.effective_config().get("foo"), None);

let codex_home_unknown = tmp.path().join("home_unknown");
tokio::fs::create_dir_all(&codex_home_unknown).await?;

let layers_unknown = load_config_layers_state(
&codex_home_unknown,
Some(cwd),
&[] as &[(String, TomlValue)],
LoaderOverrides::default(),
)
.await?;
let project_layers_unknown = layers_unknown
.layers_high_to_low()
.into_iter()
.filter(|layer| matches!(layer.name, super::ConfigLayerSource::Project { .. }))
.count();
assert_eq!(project_layers_unknown, 0);
assert_eq!(layers_unknown.effective_config().get("foo"), None);

Ok(())
}

#[tokio::test]
async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> 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).await?;
tokio::fs::write(project_root.join(".git"), "gitdir: here").await?;

let codex_home = tmp.path().join("home");
tokio::fs::create_dir_all(&codex_home).await?;
make_config_for_test(&codex_home, &project_root, TrustLevel::Trusted, None).await?;

let cwd = AbsolutePathBuf::from_absolute_path(&nested)?;
let cli_overrides = vec![(
"experimental_instructions_file".to_string(),
TomlValue::String("relative.md".to_string()),
)];

load_config_layers_state(
&codex_home,
Some(cwd),
&cli_overrides,
LoaderOverrides::default(),
)
.await?;

Ok(())
}

#[tokio::test]
async fn project_root_markers_supports_alternate_markers() -> std::io::Result<()> {
let tmp = tempdir()?;
Expand All @@ -507,11 +627,11 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<()

let codex_home = tmp.path().join("home");
tokio::fs::create_dir_all(&codex_home).await?;
tokio::fs::write(
codex_home.join(CONFIG_TOML_FILE),
r#"
project_root_markers = [".hg"]
"#,
make_config_for_test(
&codex_home,
&project_root,
TrustLevel::Trusted,
Some(vec![".hg".to_string()]),
)
.await?;

Expand Down
20 changes: 20 additions & 0 deletions codex-rs/core/src/skills/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,15 +550,20 @@ fn extract_frontmatter(contents: &str) -> Option<String> {
#[cfg(test)]
mod tests {
use super::*;
use crate::config::CONFIG_TOML_FILE;
use crate::config::ConfigBuilder;
use crate::config::ConfigOverrides;
use crate::config::ConfigToml;
use crate::config::ProjectConfig;
use crate::config_loader::ConfigLayerEntry;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigRequirements;
use crate::config_loader::ConfigRequirementsToml;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::path::Path;
use tempfile::TempDir;
use toml::Value as TomlValue;
Expand All @@ -570,6 +575,21 @@ mod tests {
}

async fn make_config_for_cwd(codex_home: &TempDir, cwd: PathBuf) -> Config {
fs::write(
codex_home.path().join(CONFIG_TOML_FILE),
toml::to_string(&ConfigToml {
projects: Some(HashMap::from([(
cwd.to_string_lossy().to_string(),
ProjectConfig {
trust_level: Some(TrustLevel::Trusted),
},
)])),
..Default::default()
})
.expect("serialize config"),
)
.unwrap();

let harness_overrides = ConfigOverrides {
cwd: Some(cwd),
..Default::default()
Expand Down
Loading