mirror of
https://github.com/openai/codex.git
synced 2026-11-06 00:30:29 +00:00
Enforce filesystem permissions when loading AGENTS.md (#39653)
## Why Project instructions must respect the selected environment's filesystem read permissions. Tightening those permissions for a later turn must not allow previously cached instructions to reach the model. ## What changed - Apply each environment's filesystem sandbox while discovering and reading `AGENTS.md` files. - Fail thread or turn setup when sandboxing blocks a discovered instruction file, while allowing a restricted project with no instructions to start. - Clear cached instructions before refresh and include the Windows sandbox level in the cache key. - Ignore inaccessible ancestor marker probes so readable instructions in the selected working directory can still load. ## Testing Added coverage for restricted projects, denied instruction files, cache invalidation after permissions tighten, and unreadable ancestor markers. GitOrigin-RevId: 6ea1a27b9c873a6260e4f87d42ae1317a6a4ae4b
This commit is contained in:
1 parent
ab82cddd04
commit
7ece061767
12 files changed
+407
-60
No files matched your search
@@ -22,6 +22,7 @@ use codex_exec_server::ReadDirectoryEntry;
|
||||
use codex_exec_server::RemoveOptions;
|
||||
use codex_extension_api::UserInstructions;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::config_types::WindowsSandboxLevel;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::EnvironmentConfig;
|
||||
use codex_protocol::protocol::EnvironmentConfigState;
|
||||
@@ -314,8 +315,10 @@ async fn load_agents_md(config: &TestConfig) -> Option<LoadedAgentsMd> {
|
||||
&config.config,
|
||||
config.user_instructions.clone(),
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
}
|
||||
|
||||
async fn agents_md_paths(config: &TestConfig) -> std::io::Result<Vec<PathUri>> {
|
||||
@@ -323,6 +326,7 @@ async fn agents_md_paths(config: &TestConfig) -> std::io::Result<Vec<PathUri>> {
|
||||
&config.config,
|
||||
&PathUri::from_abs_path(&config.cwd),
|
||||
LOCAL_FS.as_ref(),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -674,11 +678,16 @@ async fn total_byte_limit_truncates_later_project_docs() {
|
||||
assert_eq!(loaded.text(), "root\n\nabc");
|
||||
}
|
||||
|
||||
/// Unreadable ancestor markers must not hide readable instructions in the selected cwd.
|
||||
#[tokio::test]
|
||||
async fn read_agents_md_propagates_metadata_errors() {
|
||||
async fn read_agents_md_loads_cwd_instructions_when_parent_markers_are_unreadable() {
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let config = make_config(&tmp, /*limit*/ 4096, /*instructions*/ None).await;
|
||||
let marker_path = config.cwd.join(".git");
|
||||
let nested = tmp.path().join("nested");
|
||||
fs::create_dir(&nested).unwrap();
|
||||
fs::write(nested.join("AGENTS.md"), "project doc").unwrap();
|
||||
let mut config = make_config(&tmp, /*limit*/ 4096, /*instructions*/ None).await;
|
||||
config.cwd = nested.abs();
|
||||
let marker_path = tmp.path().join(".git").abs();
|
||||
let fs = FailingFileSystem {
|
||||
path: marker_path,
|
||||
failure: InjectedFailure::Metadata(io::ErrorKind::PermissionDenied),
|
||||
@@ -686,17 +695,19 @@ async fn read_agents_md_propagates_metadata_errors() {
|
||||
};
|
||||
|
||||
let cwd = config.cwd.clone();
|
||||
let err = read_agents_md(
|
||||
let loaded = read_agents_md(
|
||||
&config.config,
|
||||
&fs,
|
||||
"local",
|
||||
&PathUri::from_abs_path(&cwd),
|
||||
config.project_doc_max_bytes,
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("metadata error");
|
||||
.expect("unreadable parent markers should not hide cwd instructions")
|
||||
.expect("cwd instructions");
|
||||
|
||||
assert_eq!(err.kind(), io::ErrorKind::PermissionDenied);
|
||||
assert_eq!(loaded.text(), "project doc");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -717,6 +728,7 @@ async fn read_agents_md_propagates_read_errors() {
|
||||
"local",
|
||||
&PathUri::from_abs_path(&cwd),
|
||||
config.project_doc_max_bytes,
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.expect_err("read error");
|
||||
@@ -742,6 +754,7 @@ async fn read_agents_md_ignores_files_removed_after_discovery() {
|
||||
"local",
|
||||
&PathUri::from_abs_path(&cwd),
|
||||
config.project_doc_max_bytes,
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("removed file is recoverable");
|
||||
@@ -774,7 +787,7 @@ async fn marker_search_does_not_wait_for_a_higher_ancestor() {
|
||||
|
||||
let paths = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(1),
|
||||
super::agents_md_paths(&config.config, &cwd, &fs),
|
||||
super::agents_md_paths(&config.config, &cwd, &fs, /*sandbox*/ None),
|
||||
)
|
||||
.await
|
||||
.expect("nearest marker should complete")
|
||||
@@ -879,7 +892,7 @@ async fn project_root_marker_search_limits_concurrent_probes_and_preserves_order
|
||||
metadata_calls.release.add_permits(max_probe_count);
|
||||
};
|
||||
let (paths, ()) = tokio::join!(
|
||||
super::agents_md_paths(&config.config, &cwd, &fs),
|
||||
super::agents_md_paths(&config.config, &cwd, &fs, /*sandbox*/ None),
|
||||
assertions
|
||||
);
|
||||
let paths = paths.expect("AGENTS.md discovery");
|
||||
@@ -925,9 +938,9 @@ async fn agents_md_search_starts_all_directory_probes() {
|
||||
failure: InjectedFailure::MetadataBlocked,
|
||||
metadata_calls: Arc::clone(&metadata_calls),
|
||||
};
|
||||
|
||||
let search =
|
||||
tokio::spawn(async move { super::agents_md_paths(&config.config, &cwd, &fs).await });
|
||||
let search = tokio::spawn(async move {
|
||||
super::agents_md_paths(&config.config, &cwd, &fs, /*sandbox*/ None).await
|
||||
});
|
||||
tokio::time::timeout(std::time::Duration::from_secs(5), async {
|
||||
loop {
|
||||
let started = metadata_calls.started.notified();
|
||||
@@ -1001,7 +1014,7 @@ async fn empty_project_root_markers_only_probe_cwd_candidates() {
|
||||
};
|
||||
let cwd = PathUri::from_abs_path(&config.cwd);
|
||||
|
||||
let paths = super::agents_md_paths(&config.config, &cwd, &fs)
|
||||
let paths = super::agents_md_paths(&config.config, &cwd, &fs, /*sandbox*/ None)
|
||||
.await
|
||||
.expect("AGENTS.md discovery");
|
||||
|
||||
@@ -1096,9 +1109,15 @@ async fn multiple_environment_docs_use_labeled_layout_and_preserve_source_order(
|
||||
]);
|
||||
let user_instructions = config.user_instructions.clone();
|
||||
|
||||
let loaded = load_project_instructions(&config.config, user_instructions, &environments)
|
||||
.await
|
||||
.expect("instructions expected");
|
||||
let loaded = load_project_instructions(
|
||||
&config.config,
|
||||
user_instructions,
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
.expect("instructions expected");
|
||||
let inner = format!(
|
||||
r#"global instructions
|
||||
|
||||
@@ -1157,9 +1176,15 @@ async fn secondary_only_project_doc_uses_single_contributor_layout() {
|
||||
]);
|
||||
let user_instructions = config.user_instructions.clone();
|
||||
|
||||
let loaded = load_project_instructions(&config.config, user_instructions, &environments)
|
||||
.await
|
||||
.expect("instructions expected");
|
||||
let loaded = load_project_instructions(
|
||||
&config.config,
|
||||
user_instructions,
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
.expect("instructions expected");
|
||||
let inner = format!("global instructions{AGENTS_MD_SEPARATOR}secondary doc");
|
||||
|
||||
assert_eq!(loaded.legacy_text(), inner);
|
||||
@@ -1186,9 +1211,15 @@ async fn primary_only_project_doc_preserves_legacy_layout_with_multiple_bound_en
|
||||
]);
|
||||
let user_instructions = config.user_instructions.clone();
|
||||
|
||||
let loaded = load_project_instructions(&config.config, user_instructions, &environments)
|
||||
.await
|
||||
.expect("instructions expected");
|
||||
let loaded = load_project_instructions(
|
||||
&config.config,
|
||||
user_instructions,
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
.expect("instructions expected");
|
||||
let inner = format!("global instructions{AGENTS_MD_SEPARATOR}primary doc");
|
||||
|
||||
assert_eq!(loaded.legacy_text(), inner);
|
||||
@@ -1216,9 +1247,15 @@ async fn project_doc_byte_limit_is_shared_across_environments() {
|
||||
]);
|
||||
let user_instructions = config.user_instructions.clone();
|
||||
|
||||
let loaded = load_project_instructions(&config.config, user_instructions, &environments)
|
||||
.await
|
||||
.expect("instructions expected");
|
||||
let loaded = load_project_instructions(
|
||||
&config.config,
|
||||
user_instructions,
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
.expect("instructions expected");
|
||||
|
||||
assert_eq!(
|
||||
loaded.text(),
|
||||
@@ -1249,8 +1286,10 @@ async fn full_primary_environment_budget_excludes_later_environment_docs() {
|
||||
&config.config,
|
||||
/*user_instructions*/ None,
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
.expect("instructions expected");
|
||||
let project_bytes = loaded
|
||||
.entries
|
||||
@@ -1280,8 +1319,10 @@ async fn secondary_environment_invalid_utf8_does_not_suppress_other_docs() {
|
||||
&config.config,
|
||||
/*user_instructions*/ None,
|
||||
&environments,
|
||||
WindowsSandboxLevel::Disabled,
|
||||
)
|
||||
.await
|
||||
.expect("project instructions should load")
|
||||
.expect("instructions expected");
|
||||
|
||||
assert!(loaded.text().contains("primary doc"));
|
||||
|
||||
Reference in new issue
Block a user