mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Avoid persisting project trust for projectless directories (#46328)
## Why Starting a thread in a directory without a project could persist trust and preapprove project configuration added later. ## What changed Track whether configuration discovery found no project-root marker, Git checkout, or project-local `.codex` directory. Skip implicit project trust in `thread/start` for these directories. Preserve existing trust decisions and permission checks. ## Testing Add regression coverage for starting a thread with full access, then adding project configuration and verifying that a read-only thread does not load it or persist trust. Add loader tests for projectless classification, project markers, saved trust, managed configuration, and skipped discovery. GitOrigin-RevId: da490c649d272494f65d4e22da1ebf89f9085477
This commit is contained in:
@@ -57,6 +57,13 @@ stdio servers. The existing `mcp_2026_07_28` flag still governs eligible other
|
||||
servers, regardless of whether their names or URLs resemble hosted Apps.
|
||||
App-server does not persist this selection.
|
||||
|
||||
# Project trust
|
||||
|
||||
`thread/start` does not persist project trust for a directory where configuration
|
||||
discovery finds no project-root marker, Git checkout, or project-local `.codex`
|
||||
directory. Starting a task there does not preapprove project configuration added
|
||||
later. Existing trust decisions and permission checks for projects are unchanged.
|
||||
|
||||
# Thread removal
|
||||
|
||||
`thread/archive` and `thread/delete` reject attempts to remove a live internal
|
||||
|
||||
@@ -1360,6 +1360,7 @@ impl ThreadRequestProcessor {
|
||||
|
||||
if requested_cwd.is_some()
|
||||
&& config.active_project.trust_level.is_none()
|
||||
&& !config.config_layer_stack.is_projectless()
|
||||
&& effective_permissions_trust_project
|
||||
{
|
||||
let trust_target = resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &config.cwd)
|
||||
|
||||
@@ -1693,6 +1693,50 @@ async fn thread_start_with_nested_git_cwd_respects_effective_permissions_for_pro
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_projectless_does_not_preapprove_later_project_config() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?;
|
||||
let workspace = TempDir::new()?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config_before = std::fs::read_to_string(&config_path)?;
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
.build_initialized()
|
||||
.await?;
|
||||
|
||||
let response = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
cwd: Some(workspace.path().display().to_string()),
|
||||
sandbox: Some(SandboxMode::DangerFullAccess),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(
|
||||
response.sandbox,
|
||||
codex_app_server_protocol::SandboxPolicy::DangerFullAccess
|
||||
);
|
||||
assert_eq!(std::fs::read_to_string(&config_path)?, config_before);
|
||||
|
||||
let project_config_dir = workspace.path().join(".codex");
|
||||
std::fs::create_dir(&project_config_dir)?;
|
||||
std::fs::write(
|
||||
project_config_dir.join("config.toml"),
|
||||
"model_reasoning_effort = \"high\"\n",
|
||||
)?;
|
||||
let response = mcp
|
||||
.start_thread(ThreadStartParams {
|
||||
cwd: Some(workspace.path().display().to_string()),
|
||||
sandbox: Some(SandboxMode::ReadOnly),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
assert_eq!(response.reasoning_effort, None);
|
||||
assert_eq!(std::fs::read_to_string(&config_path)?, config_before);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_start_with_read_only_sandbox_does_not_persist_project_trust() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -1701,6 +1745,7 @@ async fn thread_start_with_read_only_sandbox_does_not_persist_project_trust() ->
|
||||
create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?;
|
||||
|
||||
let workspace = TempDir::new()?;
|
||||
std::fs::create_dir(workspace.path().join(".codex"))?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
@@ -1728,6 +1773,7 @@ async fn thread_start_preserves_untrusted_project_trust() -> Result<()> {
|
||||
create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?;
|
||||
|
||||
let workspace = TempDir::new()?;
|
||||
std::fs::create_dir(workspace.path().join(".codex"))?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let workspace_key = workspace.path().display().to_string();
|
||||
let mut config_toml =
|
||||
|
||||
@@ -6,6 +6,10 @@ mod managed_requirements;
|
||||
mod project_discovery;
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "projectless_directory_tests.rs"]
|
||||
mod projectless_directory_tests;
|
||||
#[cfg(windows)]
|
||||
mod windows;
|
||||
|
||||
@@ -325,6 +329,7 @@ pub async fn load_config_layers_state(
|
||||
);
|
||||
}
|
||||
|
||||
let mut is_projectless = false;
|
||||
if !ignore_project_config && let Some(cwd) = cwd {
|
||||
let mut merged_so_far = TomlValue::Table(toml::map::Map::new());
|
||||
for layer in &layers {
|
||||
@@ -396,6 +401,9 @@ pub async fn load_config_layers_state(
|
||||
strict_config,
|
||||
)
|
||||
.await?;
|
||||
is_projectless = !project_trust_context.has_project_root_marker
|
||||
&& project_trust_context.checkout_root.is_none()
|
||||
&& project_layers.layers.is_empty();
|
||||
layers.extend(project_layers.layers);
|
||||
startup_warnings.extend(project_layers.startup_warnings);
|
||||
}
|
||||
@@ -474,12 +482,13 @@ pub async fn load_config_layers_state(
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
let config_layer_stack = ConfigLayerStack::new(
|
||||
let mut config_layer_stack = ConfigLayerStack::new(
|
||||
layers,
|
||||
config_requirements_toml.clone().try_into()?,
|
||||
config_requirements_toml.into_toml(),
|
||||
)?
|
||||
.with_user_and_project_exec_policy_rules_ignored(ignore_user_and_project_exec_policy_rules);
|
||||
config_layer_stack.is_projectless = is_projectless;
|
||||
startup_warnings.extend(ignored_config_warning(
|
||||
&config_layer_stack,
|
||||
&requirements_layers,
|
||||
@@ -995,6 +1004,7 @@ fn apply_credential_broker_requirements(
|
||||
|
||||
struct ProjectTrustContext {
|
||||
project_root: AbsolutePathBuf,
|
||||
has_project_root_marker: bool,
|
||||
project_root_key: String,
|
||||
project_root_lookup_keys: Vec<String>,
|
||||
checkout_root: Option<AbsolutePathBuf>,
|
||||
@@ -1247,7 +1257,9 @@ async fn project_trust_context(
|
||||
.map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?
|
||||
};
|
||||
|
||||
let project_root = find_project_root(fs, cwd, project_root_markers).await?;
|
||||
let discovered_root = discover_project_root(fs, cwd, project_root_markers).await?;
|
||||
let has_project_root_marker = discovered_root.is_some();
|
||||
let project_root = discovered_root.unwrap_or_else(|| cwd.clone());
|
||||
let projects = project_trust_config.projects.unwrap_or_default();
|
||||
|
||||
let project_root_lookup_keys = normalized_project_trust_keys(project_root.as_path());
|
||||
@@ -1312,6 +1324,7 @@ async fn project_trust_context(
|
||||
|
||||
Ok(ProjectTrustContext {
|
||||
project_root,
|
||||
has_project_root_marker,
|
||||
project_root_key,
|
||||
project_root_lookup_keys,
|
||||
checkout_root,
|
||||
@@ -1463,8 +1476,18 @@ pub async fn find_project_root(
|
||||
cwd: &AbsolutePathBuf,
|
||||
project_root_markers: &[String],
|
||||
) -> io::Result<AbsolutePathBuf> {
|
||||
Ok(discover_project_root(fs, cwd, project_root_markers)
|
||||
.await?
|
||||
.unwrap_or_else(|| cwd.clone()))
|
||||
}
|
||||
|
||||
async fn discover_project_root(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
cwd: &AbsolutePathBuf,
|
||||
project_root_markers: &[String],
|
||||
) -> io::Result<Option<AbsolutePathBuf>> {
|
||||
if project_root_markers.is_empty() {
|
||||
return Ok(cwd.clone());
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
for ancestor in cwd.ancestors() {
|
||||
@@ -1490,10 +1513,10 @@ pub async fn find_project_root(
|
||||
{
|
||||
continue;
|
||||
}
|
||||
return Ok(ancestor);
|
||||
return Ok(Some(ancestor));
|
||||
}
|
||||
}
|
||||
Ok(cwd.clone())
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
async fn find_git_checkout_root(
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
//! Projectless classification requires completed discovery without project inputs.
|
||||
|
||||
use crate::ConfigLayerStack;
|
||||
use crate::LoaderOverrides;
|
||||
use crate::NoopThreadConfigLoader;
|
||||
use crate::loader::find_project_root;
|
||||
use crate::loader::load_config_layers_state;
|
||||
use crate::loader::project_trust_key;
|
||||
use crate::loader::tests::TestFileSystem;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
struct Fixture {
|
||||
_temp: TempDir,
|
||||
home: AbsolutePathBuf,
|
||||
cwd: AbsolutePathBuf,
|
||||
overrides: LoaderOverrides,
|
||||
}
|
||||
|
||||
impl Fixture {
|
||||
fn new() -> anyhow::Result<Self> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let root = AbsolutePathBuf::from_absolute_path(temp.path().canonicalize()?)?;
|
||||
let home = root.join("home");
|
||||
let cwd = root.join("workspace");
|
||||
std::fs::create_dir_all(&home)?;
|
||||
std::fs::create_dir_all(&cwd)?;
|
||||
Ok(Self {
|
||||
_temp: temp,
|
||||
home,
|
||||
cwd,
|
||||
overrides: LoaderOverrides::without_managed_config_for_tests(),
|
||||
})
|
||||
}
|
||||
|
||||
async fn load(&self) -> anyhow::Result<ConfigLayerStack> {
|
||||
Ok(load_config_layers_state(
|
||||
&TestFileSystem,
|
||||
self.home.as_path(),
|
||||
Some(self.cwd.clone()),
|
||||
&[],
|
||||
self.overrides.clone(),
|
||||
&NoopThreadConfigLoader,
|
||||
)
|
||||
.await?)
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn project_root_lookup_preserves_cwd_fallback() -> anyhow::Result<()> {
|
||||
let fixture = Fixture::new()?;
|
||||
let markers = vec![".company-root".to_string()];
|
||||
for markers in [&[][..], markers.as_slice()] {
|
||||
assert_eq!(
|
||||
find_project_root(&TestFileSystem, &fixture.cwd, markers).await?,
|
||||
fixture.cwd,
|
||||
);
|
||||
}
|
||||
|
||||
std::fs::create_dir(fixture.cwd.join(".company-root"))?;
|
||||
let nested = fixture.cwd.join("nested");
|
||||
std::fs::create_dir(&nested)?;
|
||||
assert_eq!(
|
||||
find_project_root(&TestFileSystem, &nested, &markers).await?,
|
||||
fixture.cwd,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unmarked_directory_is_projectless_even_with_saved_trust() -> anyhow::Result<()> {
|
||||
let fixture = Fixture::new()?;
|
||||
assert!(fixture.load().await?.is_projectless());
|
||||
let key = TomlValue::String(project_trust_key(fixture.cwd.as_path()));
|
||||
for level in ["trusted", "untrusted"] {
|
||||
std::fs::write(
|
||||
fixture.home.join("config.toml"),
|
||||
format!("[projects.{key}]\ntrust_level = \"{level}\"\n"),
|
||||
)?;
|
||||
assert!(fixture.load().await?.is_projectless(), "{level}");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skipped_discovery_does_not_claim_projectless() -> anyhow::Result<()> {
|
||||
let fixture = Fixture::new()?;
|
||||
for (cwd, ignore_project_config) in [(None, false), (Some(fixture.cwd.clone()), true)] {
|
||||
let layers = load_config_layers_state(
|
||||
&TestFileSystem,
|
||||
fixture.home.as_path(),
|
||||
cwd,
|
||||
&[],
|
||||
LoaderOverrides {
|
||||
ignore_project_config,
|
||||
..fixture.overrides.clone()
|
||||
},
|
||||
&NoopThreadConfigLoader,
|
||||
)
|
||||
.await?;
|
||||
assert!(!layers.is_projectless());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn project_markers_and_local_layers_prevent_projectless_classification() -> anyhow::Result<()>
|
||||
{
|
||||
for (marker, config, child) in [
|
||||
(".git", "", ""),
|
||||
(".git", "", "nested"),
|
||||
(".git", "project_root_markers = []", "nested"),
|
||||
(".codex", "", ""),
|
||||
(".codex", "project_root_markers = ['.codex']", "nested"),
|
||||
(
|
||||
".company-root",
|
||||
"project_root_markers = ['.company-root']",
|
||||
"",
|
||||
),
|
||||
(
|
||||
".company-root",
|
||||
"project_root_markers = ['.company-root']",
|
||||
"nested",
|
||||
),
|
||||
] {
|
||||
let mut fixture = Fixture::new()?;
|
||||
std::fs::create_dir(fixture.cwd.join(marker))?;
|
||||
if marker == ".git" {
|
||||
std::fs::write(fixture.cwd.join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
}
|
||||
std::fs::write(fixture.home.join("config.toml"), config)?;
|
||||
fixture.cwd = fixture.cwd.join(child);
|
||||
std::fs::create_dir_all(&fixture.cwd)?;
|
||||
assert!(
|
||||
!fixture.load().await?.is_projectless(),
|
||||
"marker={marker}, config={config}, child={child}"
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn user_codex_home_is_not_a_project_layer() -> anyhow::Result<()> {
|
||||
let mut fixture = Fixture::new()?;
|
||||
fixture.home = fixture.cwd.join(".codex");
|
||||
std::fs::create_dir(&fixture.home)?;
|
||||
std::fs::write(fixture.home.join("config.toml"), "model = 'user-model'\n")?;
|
||||
assert!(fixture.load().await?.is_projectless());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_root_markers_control_projectless_classification() -> anyhow::Result<()> {
|
||||
let mut fixture = Fixture::new()?;
|
||||
std::fs::create_dir(fixture.cwd.join(".company-root"))?;
|
||||
fixture.cwd = fixture.cwd.join("nested");
|
||||
std::fs::create_dir(&fixture.cwd)?;
|
||||
std::fs::write(
|
||||
fixture.home.join("config.toml"),
|
||||
"project_root_markers = []",
|
||||
)?;
|
||||
let managed = fixture.home.join("managed_config.toml");
|
||||
fixture.overrides = LoaderOverrides::with_managed_config_path_for_tests(managed.to_path_buf());
|
||||
for (markers, expected) in [("['.company-root']", false), ("[]", true)] {
|
||||
std::fs::write(&managed, format!("project_root_markers = {markers}\n"))?;
|
||||
assert_eq!(
|
||||
fixture.load().await?.is_projectless(),
|
||||
expected,
|
||||
"{markers}"
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -270,6 +270,7 @@ pub struct ConfigLayerStack {
|
||||
/// `None` means the loader did not check for stack-level warnings, while
|
||||
/// `Some(vec![])` means it checked and found nothing to report.
|
||||
startup_warnings: Option<Vec<String>>,
|
||||
pub(crate) is_projectless: bool,
|
||||
}
|
||||
|
||||
impl ConfigLayerStack {
|
||||
@@ -290,6 +291,7 @@ impl ConfigLayerStack {
|
||||
requirements_toml,
|
||||
ignore_user_and_project_exec_policy_rules: false,
|
||||
startup_warnings: None,
|
||||
is_projectless: false,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -314,6 +316,12 @@ impl ConfigLayerStack {
|
||||
self.startup_warnings.as_deref()
|
||||
}
|
||||
|
||||
/// Whether discovery found no project markers or project-local configuration.
|
||||
/// Returns false when project discovery was skipped.
|
||||
pub fn is_projectless(&self) -> bool {
|
||||
self.is_projectless
|
||||
}
|
||||
|
||||
/// Returns the active raw user config layer, if any.
|
||||
///
|
||||
/// This does not merge other config layers or apply any requirements. When
|
||||
@@ -420,6 +428,7 @@ impl ConfigLayerStack {
|
||||
ignore_user_and_project_exec_policy_rules: self
|
||||
.ignore_user_and_project_exec_policy_rules,
|
||||
startup_warnings: self.startup_warnings.clone(),
|
||||
is_projectless: self.is_projectless,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -455,6 +464,7 @@ impl ConfigLayerStack {
|
||||
ignore_user_and_project_exec_policy_rules: self
|
||||
.ignore_user_and_project_exec_policy_rules,
|
||||
startup_warnings: self.startup_warnings.clone(),
|
||||
is_projectless: self.is_projectless,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user