mirror of
https://github.com/openai/codex.git
synced 2026-10-27 18:48:37 +00:00
fix: restore deterministic project trust lookups
Re-enable hook discovery on Windows so the trust-gating tests cover the real behavior instead of skipping it. Avoid alias-expanded project maps that can let a configured symlink alias satisfy the canonical project lookup; keep exact/case-normalized matching deterministic instead. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -658,16 +658,11 @@ impl ConfigToml {
|
||||
/// Resolves the cwd to an existing project, or returns None if ConfigToml
|
||||
/// does not contain a project corresponding to cwd or a git repo for cwd
|
||||
pub fn get_active_project(&self, resolved_cwd: &Path) -> Option<ProjectConfig> {
|
||||
let mut projects = HashMap::new();
|
||||
for (key, project_config) in self.projects.clone().unwrap_or_default() {
|
||||
for normalized_key in normalized_project_lookup_keys(Path::new(&key)) {
|
||||
projects.insert(normalized_key, project_config.clone());
|
||||
}
|
||||
}
|
||||
let projects = self.projects.as_ref()?;
|
||||
|
||||
for normalized_cwd in normalized_project_lookup_keys(resolved_cwd) {
|
||||
if let Some(project_config) = projects.get(&normalized_cwd) {
|
||||
return Some(project_config.clone());
|
||||
if let Some(project_config) = project_config_for_lookup_key(projects, &normalized_cwd) {
|
||||
return Some(project_config);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -676,8 +671,10 @@ impl ConfigToml {
|
||||
// worktrees inherit trust from the main project.
|
||||
if let Some(repo_root) = resolve_root_git_project_for_trust(resolved_cwd) {
|
||||
for normalized_repo_root in normalized_project_lookup_keys(&repo_root) {
|
||||
if let Some(project_config_for_root) = projects.get(&normalized_repo_root) {
|
||||
return Some(project_config_for_root.clone());
|
||||
if let Some(project_config_for_root) =
|
||||
project_config_for_lookup_key(projects, &normalized_repo_root)
|
||||
{
|
||||
return Some(project_config_for_root);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -733,6 +730,24 @@ fn normalize_project_lookup_key(key: String) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
fn project_config_for_lookup_key(
|
||||
projects: &HashMap<String, ProjectConfig>,
|
||||
lookup_key: &str,
|
||||
) -> Option<ProjectConfig> {
|
||||
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(
|
||||
model_providers: &HashMap<String, ModelProviderInfo>,
|
||||
) -> Result<(), String> {
|
||||
|
||||
@@ -5622,7 +5622,6 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request()
|
||||
pretty_assertions::assert_eq!(output, expected);
|
||||
}
|
||||
|
||||
#[cfg_attr(windows, ignore)]
|
||||
#[tokio::test]
|
||||
async fn session_start_hooks_only_load_from_trusted_project_layers() -> std::io::Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
@@ -5657,7 +5656,6 @@ async fn session_start_hooks_only_load_from_trusted_project_layers() -> std::io:
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg_attr(windows, ignore)]
|
||||
#[tokio::test]
|
||||
async fn session_start_hooks_require_project_trust_without_config_toml() -> std::io::Result<()> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
|
||||
@@ -5136,6 +5136,30 @@ trust_level = "trusted"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
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), None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_set_default_oss_provider() -> std::io::Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
|
||||
@@ -564,29 +564,35 @@ impl ProjectTrustDecision {
|
||||
impl ProjectTrustContext {
|
||||
fn decision_for_dir(&self, dir: &AbsolutePathBuf) -> ProjectTrustDecision {
|
||||
for dir_key in normalized_project_trust_keys(dir.as_path()) {
|
||||
if let Some(trust_level) = self.projects_trust.get(&dir_key).copied() {
|
||||
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: dir_key,
|
||||
trust_key,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
for project_root_key in &self.project_root_lookup_keys {
|
||||
if let Some(trust_level) = self.projects_trust.get(project_root_key).copied() {
|
||||
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: project_root_key.clone(),
|
||||
trust_key,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
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_level) = self.projects_trust.get(repo_root_key).copied() {
|
||||
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: repo_root_key.clone(),
|
||||
trust_key,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -669,16 +675,7 @@ async fn project_trust_context(
|
||||
|
||||
let projects_trust = projects
|
||||
.into_iter()
|
||||
.flat_map(|(key, project)| {
|
||||
project
|
||||
.trust_level
|
||||
.into_iter()
|
||||
.flat_map(move |trust_level| {
|
||||
normalized_project_trust_key_strs(&key)
|
||||
.into_iter()
|
||||
.map(move |normalized_key| (normalized_key, trust_level))
|
||||
})
|
||||
})
|
||||
.filter_map(|(key, project)| project.trust_level.map(|trust_level| (key, trust_level)))
|
||||
.collect();
|
||||
|
||||
Ok(ProjectTrustContext {
|
||||
@@ -714,17 +711,6 @@ fn normalized_project_trust_keys(path: &Path) -> Vec<String> {
|
||||
}
|
||||
}
|
||||
|
||||
fn normalized_project_trust_key_strs(path: &str) -> Vec<String> {
|
||||
let path = Path::new(path);
|
||||
if path.is_absolute() {
|
||||
normalized_project_trust_keys(path)
|
||||
} else {
|
||||
vec![normalize_project_trust_lookup_key(
|
||||
path.to_string_lossy().to_string(),
|
||||
)]
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_project_trust_lookup_key(key: String) -> String {
|
||||
if cfg!(windows) {
|
||||
key.to_ascii_lowercase()
|
||||
@@ -733,10 +719,28 @@ fn normalize_project_trust_lookup_key(key: String) -> String {
|
||||
}
|
||||
}
|
||||
/// Convert the path to the canonical trust key we store and compare against.
|
||||
/// On Windows this also normalizes case so alias paths still match.
|
||||
/// On Windows this also normalizes case so lookups remain case-insensitive.
|
||||
pub fn project_trust_key(project_path: &Path) -> String {
|
||||
normalized_project_trust_key(project_path)
|
||||
}
|
||||
|
||||
fn project_trust_for_lookup_key(
|
||||
projects_trust: &std::collections::HashMap<String, TrustLevel>,
|
||||
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.
|
||||
|
||||
@@ -1202,6 +1202,65 @@ 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(
|
||||
&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<()> {
|
||||
|
||||
@@ -80,17 +80,6 @@ impl ClaudeHooksEngine {
|
||||
};
|
||||
}
|
||||
|
||||
if cfg!(windows) {
|
||||
return Self {
|
||||
handlers: Vec::new(),
|
||||
warnings: vec![
|
||||
"Disabled `codex_hooks` for this session because `hooks.json` lifecycle hooks are not supported on Windows yet."
|
||||
.to_string(),
|
||||
],
|
||||
shell,
|
||||
};
|
||||
}
|
||||
|
||||
let _ = schema_loader::generated_hook_schemas();
|
||||
let discovered = discovery::discover_handlers(config_layer_stack);
|
||||
Self {
|
||||
|
||||
Reference in New Issue
Block a user