diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 0bcb2903e2..8e0c475d90 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -2,6 +2,8 @@ //! //! Project-level documentation is primarily stored in files named `AGENTS.md`. //! Additional fallback filenames can be configured via `project_doc_fallback_filenames`. +//! Fallback entries containing path syntax for the executor's OS are ignored +//! before any filesystem probes use them. //! We include the concatenation of all files found along the path from the //! project root to the current working directory as follows: //! @@ -30,6 +32,7 @@ use codex_file_system::FileSystemSandboxContext; use codex_file_system::FindUpErrorPolicy; use codex_file_system::find_nearest_ancestor_with_markers; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use futures::StreamExt; use std::io; @@ -236,7 +239,7 @@ async fn agents_md_paths( vec![dir] }; - let candidate_filenames = candidate_filenames(config); + let candidate_filenames = candidate_filenames(config, cwd); let candidate_filenames = &candidate_filenames; let mut results = futures::stream::iter(search_dirs) .map(|directory| async move { @@ -266,7 +269,7 @@ async fn agents_md_paths( Ok(found) } -fn candidate_filenames(config: &Config) -> Vec<&str> { +fn candidate_filenames<'a>(config: &'a Config, cwd: &PathUri) -> Vec<&'a str> { let mut names: Vec<&str> = Vec::with_capacity(2 + config.project_doc_fallback_filenames.len()); names.push(LOCAL_AGENTS_MD_FILENAME); names.push(DEFAULT_AGENTS_MD_FILENAME); @@ -275,6 +278,16 @@ fn candidate_filenames(config: &Config) -> Vec<&str> { if candidate.is_empty() { continue; } + // Use the executor's path convention, not the host's: resolving a Windows + // network path can send ambient credentials even during metadata probes. + if matches!(candidate, "." | "..") + || candidate.contains(['/', '\0']) + || cwd.infer_path_convention() == Some(PathConvention::Windows) + && candidate.contains(['\\', ':']) + { + tracing::warn!("ignoring project_doc_fallback_filenames entry that is not a filename"); + continue; + } if !names.contains(&candidate) { names.push(candidate); } diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index d2e8f0644a..a3b046b67a 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -51,6 +51,7 @@ use tokio::sync::Semaphore; #[derive(Clone, Copy)] enum InjectedFailure { + MetadataNotFound, Metadata(io::ErrorKind), MetadataBlocked, MetadataBlockedByFilenamePrefix(&'static str), @@ -128,13 +129,16 @@ impl FailingFileSystem { options: GetMetadataOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> io::Result { - let path_abs = path.to_abs_path()?; self.metadata_calls .paths .lock() .expect("metadata paths lock") .push(path.clone()); self.metadata_calls.started.notify_one(); + if matches!(self.failure, InjectedFailure::MetadataNotFound) { + return Err(io::Error::from(io::ErrorKind::NotFound)); + } + let path_abs = path.to_abs_path()?; match self.failure { InjectedFailure::Metadata(kind) if path_abs == self.path => { Err(io::Error::new(kind, "injected metadata failure")) @@ -165,7 +169,8 @@ impl FailingFileSystem { InjectedFailure::MetadataPending if path_abs == self.path => { std::future::pending().await } - InjectedFailure::Metadata(_) + InjectedFailure::MetadataNotFound + | InjectedFailure::Metadata(_) | InjectedFailure::MetadataBlocked | InjectedFailure::MetadataBlockedByFilenamePrefix(_) | InjectedFailure::MetadataPending @@ -1566,6 +1571,72 @@ async fn agents_local_md_preferred() { ); } +#[tokio::test] +async fn fallback_paths_are_rejected_before_filesystem_probes() { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut cfg = make_config_with_project_root_markers( + &tmp, + /*limit*/ 4096, + /*instructions*/ None, + &[], + ) + .await; + let windows_paths = [ + r"..\AGENTS.md", + r"nested\AGENTS.md", + r"\AGENTS.md", + r"C:\AGENTS.md", + "C:AGENTS.md", + r"\\server\share\AGENTS.md", + r"\\?\UNC\server\share\AGENTS.md", + r"\\.\pipe\instructions", + "AGENTS.md:stream", + ]; + cfg.project_doc_fallback_filenames = [ + "", + ".", + "..", + "/AGENTS.md", + "../AGENTS.md", + "nested/AGENTS.md", + "//server/share/AGENTS.md", + "AGENTS\0.md", + ] + .into_iter() + .chain(windows_paths) + .chain(["WORKFLOW.md", "WORKFLOW.md", ".instructions.md"]) + .map(str::to_owned) + .collect(); + + // Backslashes and colons are ordinary filename characters on POSIX executors. + for (cwd, extra_filenames) in [ + ("file:///repo", windows_paths.as_slice()), + ("file:///C:/repo", &[][..]), + ] { + let cwd: PathUri = cwd.parse().expect("cwd URI"); + let metadata_calls = Arc::new(MetadataCallCounts::default()); + let filesystem = FailingFileSystem { + path: tmp.abs(), + failure: InjectedFailure::MetadataNotFound, + metadata_calls: Arc::clone(&metadata_calls), + }; + let paths = super::agents_md_paths(&cfg, &cwd, &filesystem, /*sandbox*/ None) + .await + .expect("discover paths"); + + assert_eq!(paths, Vec::::new()); + assert_eq!( + *metadata_calls.paths.lock().expect("metadata paths lock"), + ["AGENTS.override.md", "AGENTS.md"] + .into_iter() + .chain(extra_filenames.iter().copied()) + .chain(["WORKFLOW.md", ".instructions.md"]) + .map(|name| cwd.join(name).expect("filename")) + .collect::>() + ); + } +} + /// When AGENTS.md is absent but a configured fallback exists, the fallback is used. #[tokio::test] async fn uses_configured_fallback_when_agents_missing() { diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index 8c0ed2b731..5a697a049a 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -374,6 +374,58 @@ async fn configured_fallback_is_used_when_agents_candidate_is_directory() -> Res Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn invalid_fallback_paths_do_not_prevent_loading_valid_filenames() -> Result<()> { + let server = start_mock_server().await; + let response_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + let test = test_codex() + .with_config(|config| { + config.project_doc_fallback_filenames = + [".", "..", "nested/WORKFLOW.md", "WORKFLOW.md"] + .map(str::to_owned) + .to_vec(); + }) + .with_workspace_setup(|cwd, fs| async move { + let nested = executor_path_uri(cwd.join("nested"))?; + fs.create_directory( + &nested, + CreateDirectoryOptions { + recursive: false, + follow_symlinks: true, + }, + /*sandbox*/ None, + ) + .await?; + for (path, contents) in [ + (nested.join("WORKFLOW.md")?, b"nested instructions".to_vec()), + ( + executor_path_uri(cwd.join("WORKFLOW.md"))?, + b"local instructions".to_vec(), + ), + ] { + fs.write_file(&path, contents, Default::default(), /*sandbox*/ None) + .await?; + } + Ok::<(), anyhow::Error>(()) + }) + .build_with_auto_env(&server) + .await?; + test.submit_turn("hello").await?; + + assert_single_instruction_fragment( + &response_mock.single_request(), + &expected_instruction_fragment( + &test.executor_environment().selection().cwd, + "local instructions", + ), + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { let instructions = agents_instructions(