mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Preserve parent repository discovery through sandbox metadata mounts (#39629)
## Why The Linux sandbox represents missing protected metadata paths such as `.git` with empty read-only directories. Repository and project discovery treated any `.git` directory as a checkout root, so this synthetic directory could hide a real parent repository and its trust configuration. ## What changed - Treat a `.git` directory as repository metadata only when it contains `HEAD`, while continuing to recognize file-based `.git` entries. - Protect missing `.git` paths with the same read-only synthetic mounts as other workspace metadata without disrupting parent-repository discovery. - Canonicalize and read-only bind the synthetic-mount registry into the sandbox, and make protected-path cleanup handle read-only directory trees. ## Testing Added coverage for repository, project-root, trust, synthetic-mount, and Landlock behavior, including nested incomplete `.git` directories and a redirected `TMPDIR`. GitOrigin-RevId: 0724c54d56531143bb28011e4ca414cd3b0212be
This commit is contained in:
@@ -871,6 +871,7 @@ async fn config_read_respects_managed_project_root_markers() -> Result<()> {
|
||||
] {
|
||||
std::fs::create_dir_all(dir)?;
|
||||
}
|
||||
std::fs::write(workspace.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::write(
|
||||
ancestor_config.join("config.toml"),
|
||||
"model_context_window = 32768\n",
|
||||
|
||||
@@ -1640,8 +1640,9 @@ async fn thread_start_with_nested_git_cwd_respects_effective_permissions_for_pro
|
||||
|
||||
let repo_root = TempDir::new()?;
|
||||
std::fs::create_dir(repo_root.path().join(".git"))?;
|
||||
std::fs::write(repo_root.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
let nested = repo_root.path().join("nested/project");
|
||||
std::fs::create_dir_all(&nested)?;
|
||||
std::fs::create_dir_all(nested.join(".git"))?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
|
||||
@@ -33,6 +33,7 @@ impl Fixture {
|
||||
let cwd = project.join("child");
|
||||
std::fs::create_dir_all(&home)?;
|
||||
std::fs::create_dir_all(repo.join(".git"))?;
|
||||
std::fs::write(repo.join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
for (dir, model) in [(&repo, "ancestor"), (&project, "project"), (&cwd, "child")] {
|
||||
std::fs::create_dir_all(dir.join(".codex"))?;
|
||||
std::fs::write(
|
||||
@@ -181,6 +182,35 @@ async fn managed_project_discovery_uses_file_markers_without_changing_precedence
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_project_discovery_ignores_incomplete_nested_git_directory() -> anyhow::Result<()> {
|
||||
let fixture = Fixture::new()?;
|
||||
std::fs::create_dir(fixture.cwd.join(".git"))?;
|
||||
std::fs::write(&fixture.managed, "project_root_markers = [\".git\"]\n")?;
|
||||
|
||||
let (canonical, local) = fixture.load().await?;
|
||||
assert_discovery(
|
||||
&canonical,
|
||||
&local,
|
||||
&[&fixture.repo, &fixture.project, &fixture.cwd],
|
||||
&[".git"],
|
||||
)?;
|
||||
|
||||
std::fs::write(fixture.home.join("config.toml"), fixture.trust("untrusted"))?;
|
||||
let (canonical, local) = fixture.load().await?;
|
||||
assert_discovery(&canonical, &local, &[], &[".git"])?;
|
||||
let trust_key = project_trust_key(fixture.repo.as_path());
|
||||
assert!(
|
||||
canonical
|
||||
.all_layers_low_to_high()
|
||||
.filter(|layer| matches!(layer.name, ConfigLayerSource::Project { .. }))
|
||||
.all(|layer| layer.disabled_reason.as_deref().is_some_and(
|
||||
|reason| reason.starts_with(&format!("{trust_key} is marked as untrusted"))
|
||||
))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_project_discovery_uses_managed_project_trust() -> anyhow::Result<()> {
|
||||
let fixture = Fixture::new()?;
|
||||
|
||||
@@ -1315,13 +1315,22 @@ async fn find_project_root(
|
||||
for marker in project_root_markers {
|
||||
let marker_path = ancestor.join(marker);
|
||||
let marker_path_uri = PathUri::from_abs_path(&marker_path);
|
||||
if fs
|
||||
.get_metadata(&marker_path_uri, /*sandbox*/ None)
|
||||
.await
|
||||
.is_ok()
|
||||
let Ok(metadata) = fs.get_metadata(&marker_path_uri, /*sandbox*/ None).await else {
|
||||
continue;
|
||||
};
|
||||
if marker == ".git"
|
||||
&& metadata.is_directory
|
||||
&& fs
|
||||
.get_metadata(
|
||||
&PathUri::from_abs_path(&marker_path.join("HEAD")),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return Ok(ancestor);
|
||||
continue;
|
||||
}
|
||||
return Ok(ancestor);
|
||||
}
|
||||
}
|
||||
Ok(cwd.clone())
|
||||
@@ -1340,13 +1349,21 @@ async fn find_git_checkout_root(
|
||||
for dir in base.ancestors() {
|
||||
let dot_git = dir.join(".git");
|
||||
let dot_git_uri = PathUri::from_abs_path(&dot_git);
|
||||
if fs
|
||||
.get_metadata(&dot_git_uri, /*sandbox*/ None)
|
||||
.await
|
||||
.is_ok()
|
||||
let Ok(metadata) = fs.get_metadata(&dot_git_uri, /*sandbox*/ None).await else {
|
||||
continue;
|
||||
};
|
||||
if metadata.is_directory
|
||||
&& fs
|
||||
.get_metadata(
|
||||
&PathUri::from_abs_path(&dot_git.join("HEAD")),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.is_err()
|
||||
{
|
||||
return Some(dir);
|
||||
continue;
|
||||
}
|
||||
return Some(dir);
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
@@ -3295,6 +3295,7 @@ async fn codex_home_within_project_tree_is_not_double_loaded() -> std::io::Resul
|
||||
|
||||
tokio::fs::create_dir_all(&nested_dot_codex).await?;
|
||||
tokio::fs::create_dir_all(project_root.join(".git")).await?;
|
||||
tokio::fs::write(project_root.join(".git/HEAD"), "ref: refs/heads/main\n").await?;
|
||||
tokio::fs::write(
|
||||
nested_dot_codex.join(CONFIG_TOML_FILE),
|
||||
r#"foo = "child"
|
||||
|
||||
@@ -8285,6 +8285,7 @@ async fn agent_role_file_without_developer_instructions_is_dropped_with_warning(
|
||||
let repo_root = TempDir::new()?;
|
||||
let nested_cwd = repo_root.path().join("packages").join("app");
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::write(repo_root.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir_all(&nested_cwd)?;
|
||||
|
||||
let workspace_key = repo_root.path().to_string_lossy().replace('\\', "\\\\");
|
||||
@@ -8456,6 +8457,7 @@ async fn discovered_agent_role_file_without_name_is_dropped_with_warning() -> st
|
||||
let repo_root = TempDir::new()?;
|
||||
let nested_cwd = repo_root.path().join("packages").join("app");
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::write(repo_root.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir_all(&nested_cwd)?;
|
||||
|
||||
let workspace_key = repo_root.path().to_string_lossy().replace('\\', "\\\\");
|
||||
@@ -8656,6 +8658,7 @@ async fn discovers_multiple_standalone_agent_role_files() -> std::io::Result<()>
|
||||
let repo_root = TempDir::new()?;
|
||||
let nested_cwd = repo_root.path().join("packages").join("app");
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::write(repo_root.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir_all(&nested_cwd)?;
|
||||
|
||||
let workspace_key = repo_root.path().to_string_lossy().replace('\\', "\\\\");
|
||||
@@ -8787,6 +8790,7 @@ async fn mixed_legacy_and_standalone_agent_role_sources_merge_with_precedence()
|
||||
let repo_root = TempDir::new()?;
|
||||
let nested_cwd = repo_root.path().join("packages").join("app");
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::write(repo_root.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir_all(&nested_cwd)?;
|
||||
|
||||
let workspace_key = repo_root.path().to_string_lossy().replace('\\', "\\\\");
|
||||
@@ -8933,6 +8937,7 @@ async fn higher_precedence_agent_role_can_inherit_description_from_lower_layer()
|
||||
let repo_root = TempDir::new()?;
|
||||
let nested_cwd = repo_root.path().join("packages").join("app");
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::write(repo_root.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir_all(&nested_cwd)?;
|
||||
|
||||
let workspace_key = repo_root.path().to_string_lossy().replace('\\', "\\\\");
|
||||
@@ -10341,6 +10346,41 @@ mcp_oauth_callback_url = "https://example.com/callback"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn untrusted_parent_repo_with_incomplete_child_git_keeps_unless_trusted_approval_policy()
|
||||
-> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let repo = TempDir::new()?;
|
||||
let cwd = repo.path().join("sub");
|
||||
std::fs::create_dir_all(repo.path().join(".git"))?;
|
||||
std::fs::write(repo.path().join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir_all(cwd.join(".git"))?;
|
||||
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml {
|
||||
projects: Some(HashMap::from([(
|
||||
repo.path().to_string_lossy().to_string(),
|
||||
ProjectConfig {
|
||||
trust_level: Some(TrustLevel::Untrusted),
|
||||
},
|
||||
)])),
|
||||
..Default::default()
|
||||
},
|
||||
ConfigOverrides {
|
||||
cwd: Some(cwd),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config.permissions.approval_policy.value(),
|
||||
AskForApproval::UnlessTrusted
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_untrusted_project_gets_unless_trusted_approval_policy() -> anyhow::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -12,6 +12,7 @@ use codex_exec_server::RemoveOptions;
|
||||
use codex_git_utils::GitInfo;
|
||||
use codex_git_utils::GitSha;
|
||||
use codex_git_utils::collect_git_info;
|
||||
use codex_git_utils::get_git_repo_root;
|
||||
use codex_git_utils::get_has_changes_in_repo;
|
||||
use codex_git_utils::git_diff_to_remote;
|
||||
use codex_git_utils::recent_commits;
|
||||
@@ -643,6 +644,7 @@ async fn resolve_root_git_project_for_trust_starts_at_parent_for_file() {
|
||||
let proj = tmp.path().join("proj");
|
||||
let nested = proj.join("nested");
|
||||
std::fs::create_dir_all(proj.join(".git")).unwrap();
|
||||
std::fs::write(proj.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap();
|
||||
std::fs::create_dir_all(&nested).unwrap();
|
||||
let file = nested.join("file.txt");
|
||||
std::fs::write(&file, "contents").unwrap();
|
||||
@@ -659,6 +661,7 @@ async fn resolve_root_git_project_for_trust_ignores_metadata_errors() {
|
||||
let proj = tmp.path().join("proj");
|
||||
let nested = proj.join("nested");
|
||||
std::fs::create_dir_all(proj.join(".git")).unwrap();
|
||||
std::fs::write(proj.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap();
|
||||
std::fs::create_dir_all(&nested).unwrap();
|
||||
let fs = MetadataOverrideFileSystem {
|
||||
path: PathUri::from_abs_path(&nested.join(".git").abs()),
|
||||
@@ -678,6 +681,7 @@ async fn resolve_root_git_project_for_trust_supports_windows_namespace_paths() {
|
||||
let tmp = TempDir::new().expect("tempdir");
|
||||
let repo = tmp.path().join("repo");
|
||||
std::fs::create_dir_all(repo.join(".git")).unwrap();
|
||||
std::fs::write(repo.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap();
|
||||
std::fs::create_dir_all(repo.join("nested")).unwrap();
|
||||
|
||||
let namespace_repo = PathBuf::from(format!(r"\\?\{}", repo.display()));
|
||||
@@ -699,7 +703,11 @@ async fn resolve_root_git_project_for_trust_regular_repo_returns_repo_root() {
|
||||
Some(repo_path.clone())
|
||||
);
|
||||
let nested = repo_path.join("sub/dir");
|
||||
std::fs::create_dir_all(nested.as_path()).unwrap();
|
||||
std::fs::create_dir_all(nested.join(".git")).unwrap();
|
||||
assert_eq!(
|
||||
get_git_repo_root(nested.as_path()),
|
||||
Some(repo_path.as_path().to_path_buf())
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &nested).await,
|
||||
Some(repo_path)
|
||||
|
||||
@@ -1109,6 +1109,11 @@ async fn turn_metadata_state_git_enrichment_cancellation_is_retryable_and_errors
|
||||
|
||||
let invalid_repo = TempDir::new().expect("invalid repo");
|
||||
std::fs::create_dir(invalid_repo.path().join(".git")).expect("invalid git directory");
|
||||
std::fs::write(
|
||||
invalid_repo.path().join(".git/HEAD"),
|
||||
"ref: refs/heads/main\n",
|
||||
)
|
||||
.expect("invalid git HEAD");
|
||||
let invalid_state = Arc::new(TurnMetadataState::new(
|
||||
"session-a".to_string(),
|
||||
"thread-a".to_string(),
|
||||
|
||||
@@ -767,7 +767,7 @@ fn find_ancestor_git_entry(base_dir: &Path) -> Option<(PathBuf, PathBuf)> {
|
||||
|
||||
loop {
|
||||
let dot_git = dir.join(".git");
|
||||
if dot_git.exists() {
|
||||
if dot_git.exists() && (!dot_git.is_dir() || dot_git.join("HEAD").exists()) {
|
||||
return Some((dir, dot_git));
|
||||
}
|
||||
|
||||
|
||||
@@ -19,15 +19,35 @@ pub async fn resolve_root_git_project_for_trust(
|
||||
Ok(metadata) if metadata.is_directory => cwd.clone(),
|
||||
_ => cwd.parent()?,
|
||||
};
|
||||
let repo_root = find_nearest_native_ancestor_with_markers(
|
||||
fs,
|
||||
&base,
|
||||
vec![".git".to_string()],
|
||||
FindUpErrorPolicy::Ignore,
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.ok()??;
|
||||
let mut base = base;
|
||||
let repo_root = loop {
|
||||
let candidate = find_nearest_native_ancestor_with_markers(
|
||||
fs,
|
||||
&base,
|
||||
vec![".git".to_string()],
|
||||
FindUpErrorPolicy::Ignore,
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.ok()??;
|
||||
let dot_git = candidate.join(".git");
|
||||
let metadata = fs
|
||||
.get_metadata(&PathUri::from_abs_path(&dot_git), /*sandbox*/ None)
|
||||
.await
|
||||
.ok()?;
|
||||
if !metadata.is_directory
|
||||
|| fs
|
||||
.get_metadata(
|
||||
&PathUri::from_abs_path(&dot_git.join("HEAD")),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.is_ok()
|
||||
{
|
||||
break candidate;
|
||||
}
|
||||
base = candidate.parent()?;
|
||||
};
|
||||
let dot_git = repo_root.join(".git");
|
||||
let dot_git_uri = PathUri::from_abs_path(&dot_git);
|
||||
let dot_git_metadata = fs.get_metadata(&dot_git_uri, /*sandbox*/ None).await.ok()?;
|
||||
|
||||
@@ -592,7 +592,6 @@ fn create_filesystem_args(
|
||||
append_metadata_path_masks_for_writable_root(
|
||||
&mut read_only_subpaths,
|
||||
root,
|
||||
mount_root,
|
||||
&protected_metadata_names,
|
||||
);
|
||||
if let Some(target) = &symlink_target {
|
||||
@@ -668,48 +667,16 @@ fn append_protected_create_targets_for_writable_root(
|
||||
fn append_metadata_path_masks_for_writable_root(
|
||||
read_only_subpaths: &mut Vec<PathBuf>,
|
||||
root: &Path,
|
||||
mount_root: &Path,
|
||||
protected_metadata_names: &[String],
|
||||
) {
|
||||
for name in protected_metadata_names {
|
||||
let path = root.join(name);
|
||||
if should_leave_missing_git_for_parent_repo_discovery(mount_root, name) {
|
||||
continue;
|
||||
}
|
||||
if !read_only_subpaths.iter().any(|subpath| subpath == &path) {
|
||||
read_only_subpaths.push(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn should_leave_missing_git_for_parent_repo_discovery(mount_root: &Path, name: &str) -> bool {
|
||||
let path = mount_root.join(name);
|
||||
name == ".git"
|
||||
&& matches!(
|
||||
path.symlink_metadata(),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound
|
||||
)
|
||||
&& mount_root
|
||||
.ancestors()
|
||||
.skip(1)
|
||||
.any(ancestor_has_git_metadata)
|
||||
}
|
||||
|
||||
fn ancestor_has_git_metadata(ancestor: &Path) -> bool {
|
||||
let git_path = ancestor.join(".git");
|
||||
let Ok(metadata) = git_path.symlink_metadata() else {
|
||||
return false;
|
||||
};
|
||||
if metadata.is_dir() {
|
||||
return git_path.join("HEAD").symlink_metadata().is_ok();
|
||||
}
|
||||
if metadata.is_file() {
|
||||
return fs::read_to_string(git_path)
|
||||
.is_ok_and(|contents| contents.trim_start().starts_with("gitdir:"));
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
fn expand_unreadable_globs_with_ripgrep(
|
||||
patterns: &[String],
|
||||
cwd: &Path,
|
||||
@@ -1814,7 +1781,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_child_git_under_parent_repo_uses_protected_create_target() {
|
||||
fn missing_child_git_under_parent_repo_is_mounted_read_only() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let repo = temp_dir.path().join("repo");
|
||||
let workspace = repo.join("workspace");
|
||||
@@ -1833,30 +1800,22 @@ mod tests {
|
||||
|
||||
let args = create_filesystem_args(&policy, &workspace, NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH)
|
||||
.expect("filesystem args");
|
||||
assert_empty_directory_mounted_read_only(&args.args, &dot_git);
|
||||
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents"));
|
||||
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex"));
|
||||
let dot_git_str = path_to_string(&dot_git);
|
||||
assert!(
|
||||
!args
|
||||
.args
|
||||
.windows(4)
|
||||
.any(|window| window == ["--perms", "555", "--tmpfs", dot_git_str.as_str()]),
|
||||
"missing child .git should not shadow parent repo discovery",
|
||||
);
|
||||
assert!(
|
||||
!synthetic_mount_target_paths(&args).contains(&dot_git),
|
||||
"missing child .git should not be a transient mount target",
|
||||
);
|
||||
assert_eq!(
|
||||
protected_create_target_paths(&args),
|
||||
vec![dot_git],
|
||||
"missing child .git should fail through protected create cleanup",
|
||||
synthetic_mount_target_paths(&args),
|
||||
vec![workspace.join(".codex"), dot_git, workspace.join(".agents")],
|
||||
);
|
||||
assert!(
|
||||
protected_create_target_paths(&args).is_empty(),
|
||||
"missing child .git should be mount protected before command execution",
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn symlinked_missing_child_git_under_parent_repo_uses_effective_mount_root() {
|
||||
fn symlinked_missing_child_git_under_parent_repo_mounts_effective_path_read_only() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
let repo = temp_dir.path().join("repo");
|
||||
let workspace = repo.join("workspace");
|
||||
@@ -1879,24 +1838,16 @@ mod tests {
|
||||
let args =
|
||||
create_filesystem_args(&policy, &link_workspace, NO_UNREADABLE_GLOB_SCAN_MAX_DEPTH)
|
||||
.expect("filesystem args");
|
||||
assert_empty_directory_mounted_read_only(&args.args, &dot_git);
|
||||
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".agents"));
|
||||
assert_empty_directory_mounted_read_only(&args.args, &workspace.join(".codex"));
|
||||
let dot_git_str = path_to_string(&dot_git);
|
||||
assert!(
|
||||
!args
|
||||
.args
|
||||
.windows(4)
|
||||
.any(|window| window == ["--perms", "555", "--tmpfs", dot_git_str.as_str()]),
|
||||
"symlinked missing child .git should not shadow parent repo discovery",
|
||||
);
|
||||
assert!(
|
||||
!synthetic_mount_target_paths(&args).contains(&dot_git),
|
||||
"symlinked missing child .git should not be a transient mount target",
|
||||
);
|
||||
assert_eq!(
|
||||
protected_create_target_paths(&args),
|
||||
vec![dot_git],
|
||||
"symlinked missing child .git should fail through protected create cleanup",
|
||||
synthetic_mount_target_paths(&args),
|
||||
vec![workspace.join(".codex"), dot_git, workspace.join(".agents")],
|
||||
);
|
||||
assert!(
|
||||
protected_create_target_paths(&args).is_empty(),
|
||||
"symlinked missing child .git should be mount protected before command execution",
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -8,9 +8,12 @@ use std::io::Read;
|
||||
use std::os::fd::AsRawFd;
|
||||
use std::os::fd::FromRawFd;
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicI32;
|
||||
use std::sync::atomic::Ordering;
|
||||
@@ -564,7 +567,7 @@ fn run_or_exec_bwrap(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
|
||||
|
||||
fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::BwrapArgs) -> ! {
|
||||
let crate::bwrap::BwrapArgs {
|
||||
args,
|
||||
mut args,
|
||||
preserved_files,
|
||||
synthetic_mount_targets,
|
||||
protected_create_targets,
|
||||
@@ -573,6 +576,20 @@ fn run_bwrap_in_child_with_synthetic_mount_cleanup(bwrap_args: crate::bwrap::Bwr
|
||||
let synthetic_mount_registrations = register_synthetic_mount_targets(&synthetic_mount_targets);
|
||||
let protected_create_registrations =
|
||||
register_protected_create_targets(&protected_create_targets);
|
||||
let registry_root = synthetic_mount_registry_root()
|
||||
.to_string_lossy()
|
||||
.into_owned();
|
||||
let Some(command_separator) = args.iter().position(|arg| arg == "--") else {
|
||||
panic!("bubblewrap argv is missing command separator '--'");
|
||||
};
|
||||
args.splice(
|
||||
command_separator..command_separator,
|
||||
[
|
||||
"--ro-bind".to_string(),
|
||||
registry_root.clone(),
|
||||
registry_root,
|
||||
],
|
||||
);
|
||||
let exec_start_pipe = create_exec_start_pipe(!protected_create_targets.is_empty());
|
||||
let parent_pid = unsafe { libc::getpid() };
|
||||
let pid = unsafe { libc::fork() };
|
||||
@@ -1136,12 +1153,6 @@ fn cleanup_protected_create_targets(targets: &[ProtectedCreateTargetRegistration
|
||||
|
||||
let mut violation = false;
|
||||
for target in targets.iter().rev() {
|
||||
if synthetic_mount_marker_dir_has_active_process(&target.marker_dir) {
|
||||
if target.target.path().exists() {
|
||||
violation = true;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
violation |= remove_protected_create_target(&target.target);
|
||||
match fs::remove_dir(&target.marker_dir) {
|
||||
Ok(()) => {}
|
||||
@@ -1206,7 +1217,13 @@ fn try_remove_protected_create_target(
|
||||
ProtectedCreateRemoval::Other
|
||||
};
|
||||
let result = if removal == ProtectedCreateRemoval::Directory {
|
||||
fs::remove_dir_all(path)
|
||||
match fs::remove_dir_all(path) {
|
||||
Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => {
|
||||
make_directory_tree_writable(path)?;
|
||||
fs::remove_dir_all(path)
|
||||
}
|
||||
result => result,
|
||||
}
|
||||
} else {
|
||||
fs::remove_file(path)
|
||||
};
|
||||
@@ -1222,6 +1239,28 @@ fn try_remove_protected_create_target(
|
||||
Ok(Some(removal))
|
||||
}
|
||||
|
||||
fn make_directory_tree_writable(path: &Path) -> std::io::Result<()> {
|
||||
let metadata = match fs::symlink_metadata(path) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(()),
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
if !metadata.is_dir() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let directory = OpenOptions::new()
|
||||
.read(true)
|
||||
.custom_flags(libc::O_PATH | libc::O_DIRECTORY | libc::O_NOFOLLOW)
|
||||
.open(path)?;
|
||||
let directory_path = PathBuf::from(format!("/proc/self/fd/{}", directory.as_raw_fd()));
|
||||
fs::set_permissions(&directory_path, fs::Permissions::from_mode(0o700))?;
|
||||
for entry in fs::read_dir(directory_path)? {
|
||||
make_directory_tree_writable(&entry?.path())?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_synthetic_mount_target(target: &crate::bwrap::SyntheticMountTarget) {
|
||||
let path = target.path();
|
||||
let metadata = match fs::symlink_metadata(path) {
|
||||
@@ -1309,10 +1348,23 @@ fn synthetic_mount_marker_dir(path: &Path) -> PathBuf {
|
||||
}
|
||||
|
||||
fn synthetic_mount_registry_root() -> PathBuf {
|
||||
let effective_uid = unsafe { libc::geteuid() };
|
||||
std::env::temp_dir().join(format!(
|
||||
"codex-bwrap-synthetic-mount-targets-{effective_uid}"
|
||||
))
|
||||
static REGISTRY_ROOT: OnceLock<PathBuf> = OnceLock::new();
|
||||
|
||||
REGISTRY_ROOT
|
||||
.get_or_init(|| {
|
||||
let effective_uid = unsafe { libc::geteuid() };
|
||||
let temp_dir = std::env::temp_dir();
|
||||
let temp_dir = temp_dir.canonicalize().unwrap_or_else(|err| {
|
||||
panic!(
|
||||
"failed to resolve synthetic mount registry temp directory {}: {err}",
|
||||
temp_dir.display()
|
||||
)
|
||||
});
|
||||
temp_dir.join(format!(
|
||||
"codex-bwrap-synthetic-mount-targets-{effective_uid}"
|
||||
))
|
||||
})
|
||||
.clone()
|
||||
}
|
||||
|
||||
fn hash_path(path: &Path) -> u64 {
|
||||
|
||||
@@ -14,6 +14,8 @@ use codex_protocol::protocol::NetworkSandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
#[cfg(test)]
|
||||
use pretty_assertions::assert_eq;
|
||||
#[cfg(test)]
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
fn read_only_permission_profile() -> PermissionProfile {
|
||||
PermissionProfile::read_only()
|
||||
@@ -326,31 +328,34 @@ fn synthetic_mount_registry_root_is_unique_to_effective_user() {
|
||||
let effective_uid = unsafe { libc::geteuid() };
|
||||
assert_eq!(
|
||||
synthetic_mount_registry_root(),
|
||||
std::env::temp_dir().join(format!(
|
||||
"codex-bwrap-synthetic-mount-targets-{effective_uid}"
|
||||
))
|
||||
std::env::temp_dir()
|
||||
.canonicalize()
|
||||
.expect("resolve temp directory")
|
||||
.join(format!(
|
||||
"codex-bwrap-synthetic-mount-targets-{effective_uid}"
|
||||
))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_synthetic_mount_targets_waits_for_other_active_registrations() {
|
||||
let temp_dir = tempfile::TempDir::new().expect("tempdir");
|
||||
let empty_file = temp_dir.path().join(".git");
|
||||
std::fs::write(&empty_file, "").expect("write empty file");
|
||||
let target = crate::bwrap::SyntheticMountTarget::missing(&empty_file);
|
||||
let empty_dir = temp_dir.path().join(".git");
|
||||
std::fs::create_dir(&empty_dir).expect("create empty dir");
|
||||
let target = crate::bwrap::SyntheticMountTarget::missing_empty_directory(&empty_dir);
|
||||
|
||||
let registrations = register_synthetic_mount_targets(std::slice::from_ref(&target));
|
||||
let active_marker = registrations[0].marker_dir.join("1");
|
||||
std::fs::write(&active_marker, "").expect("write active marker");
|
||||
|
||||
cleanup_synthetic_mount_targets(®istrations);
|
||||
assert!(empty_file.exists());
|
||||
assert!(empty_dir.exists());
|
||||
|
||||
std::fs::remove_file(active_marker).expect("remove active marker");
|
||||
let registrations = register_synthetic_mount_targets(std::slice::from_ref(&target));
|
||||
cleanup_synthetic_mount_targets(®istrations);
|
||||
|
||||
assert!(!empty_file.exists());
|
||||
assert!(!empty_dir.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -412,7 +417,7 @@ fn cleanup_protected_create_targets_removes_created_path_and_reports_violation()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_protected_create_targets_waits_for_other_active_registrations() {
|
||||
fn cleanup_protected_create_targets_removes_path_despite_active_marker() {
|
||||
let temp_dir = tempfile::TempDir::new().expect("tempdir");
|
||||
let dot_git = temp_dir.path().join(".git");
|
||||
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
|
||||
@@ -424,14 +429,39 @@ fn cleanup_protected_create_targets_waits_for_other_active_registrations() {
|
||||
|
||||
let violation = cleanup_protected_create_targets(®istrations);
|
||||
assert!(violation);
|
||||
assert!(dot_git.exists());
|
||||
assert!(!dot_git.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cleanup_protected_create_targets_removes_read_only_directory_and_reports_violation() {
|
||||
let temp_dir = tempfile::TempDir::new().expect("tempdir");
|
||||
let dot_git = temp_dir.path().join(".git");
|
||||
let outside = temp_dir.path().join("outside");
|
||||
let target = crate::bwrap::ProtectedCreateTarget::missing(&dot_git);
|
||||
|
||||
let registrations = register_protected_create_targets(&[target]);
|
||||
std::fs::create_dir(&outside).expect("create outside directory");
|
||||
std::fs::set_permissions(&outside, std::fs::Permissions::from_mode(0o755))
|
||||
.expect("set outside directory permissions");
|
||||
std::fs::create_dir(&dot_git).expect("create protected path");
|
||||
std::fs::write(dot_git.join("config"), "[core]\n").expect("write protected child");
|
||||
std::os::unix::fs::symlink(&outside, dot_git.join("outside-link"))
|
||||
.expect("link outside directory");
|
||||
std::fs::set_permissions(&dot_git, std::fs::Permissions::from_mode(0o000))
|
||||
.expect("make protected path read-only");
|
||||
|
||||
std::fs::remove_file(active_marker).expect("remove active marker");
|
||||
let registrations = register_protected_create_targets(std::slice::from_ref(&target));
|
||||
let violation = cleanup_protected_create_targets(®istrations);
|
||||
|
||||
assert!(violation);
|
||||
assert!(!dot_git.exists());
|
||||
assert_eq!(
|
||||
std::fs::metadata(&outside)
|
||||
.expect("outside directory remains")
|
||||
.permissions()
|
||||
.mode()
|
||||
& 0o777,
|
||||
0o755
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -121,6 +121,7 @@ async fn run_cmd_result_with_permission_profile(
|
||||
cmd,
|
||||
cwd,
|
||||
permission_profile,
|
||||
create_env_from_core_vars(),
|
||||
timeout_ms,
|
||||
use_legacy_landlock,
|
||||
)
|
||||
@@ -154,6 +155,7 @@ async fn run_cmd_result_with_cwd_and_writable_roots(
|
||||
cmd,
|
||||
cwd,
|
||||
permission_profile,
|
||||
create_env_from_core_vars(),
|
||||
timeout_ms,
|
||||
use_legacy_landlock,
|
||||
)
|
||||
@@ -164,6 +166,7 @@ async fn run_cmd_result_with_permission_profile_for_cwd(
|
||||
cmd: &[&str],
|
||||
cwd: AbsolutePathBuf,
|
||||
permission_profile: PermissionProfile,
|
||||
env: HashMap<String, String>,
|
||||
timeout_ms: u64,
|
||||
use_legacy_landlock: bool,
|
||||
) -> Result<codex_protocol::exec_output::ExecToolCallOutput> {
|
||||
@@ -173,7 +176,7 @@ async fn run_cmd_result_with_permission_profile_for_cwd(
|
||||
cwd,
|
||||
expiration: timeout_ms.into(),
|
||||
capture_policy: ExecCapturePolicy::ShellTool,
|
||||
env: create_env_from_core_vars(),
|
||||
env,
|
||||
network: None,
|
||||
network_environment_id: None,
|
||||
sandbox_permissions: SandboxPermissions::UseDefault,
|
||||
@@ -750,6 +753,12 @@ async fn sandbox_keeps_parent_repo_discovery_while_blocking_child_metadata() {
|
||||
let tmpdir = tempfile::tempdir().expect("tempdir");
|
||||
let repo = tmpdir.path().join("repo");
|
||||
let subdir = repo.join("sub");
|
||||
let real_tmp = tmpdir.path().join("real-tmp");
|
||||
let redirected_tmp = tmpdir.path().join("redirected-tmp");
|
||||
let tmp_alias = tmpdir.path().join("tmp-alias");
|
||||
std::fs::create_dir(&real_tmp).expect("create real temp directory");
|
||||
std::fs::create_dir(&redirected_tmp).expect("create redirected temp directory");
|
||||
std::os::unix::fs::symlink(&real_tmp, &tmp_alias).expect("create temp directory alias");
|
||||
std::fs::create_dir_all(&subdir).expect("create nested workspace");
|
||||
assert!(
|
||||
std::process::Command::new("git")
|
||||
@@ -763,9 +772,22 @@ async fn sandbox_keeps_parent_repo_discovery_while_blocking_child_metadata() {
|
||||
);
|
||||
|
||||
let repo = repo.to_string_lossy();
|
||||
let redirected_tmp = redirected_tmp.to_string_lossy();
|
||||
let script = format!(
|
||||
r#"set -e
|
||||
test "$(git rev-parse --show-toplevel)" = '{repo}'
|
||||
touch "$TMPDIR/writable-sibling"
|
||||
registry="${{TMPDIR:-/tmp}}/codex-bwrap-synthetic-mount-targets-$(id -u)"
|
||||
if touch "$registry/forged-marker" 2>/dev/null; then
|
||||
exit 22
|
||||
fi
|
||||
redirected_registry='{redirected_tmp}'/codex-bwrap-synthetic-mount-targets-$(id -u)
|
||||
for marker_dir in "$registry"/*; do
|
||||
[ -d "$marker_dir" ] || continue
|
||||
mkdir -p "$redirected_registry/${{marker_dir##*/}}"
|
||||
touch "$redirected_registry/${{marker_dir##*/}}/1"
|
||||
done
|
||||
ln -sfn '{redirected_tmp}' "$TMPDIR"
|
||||
git status --short > status.before
|
||||
if grep -E '(^|[[:space:]])\.(git|codex|agents)(/|$)' status.before; then
|
||||
cat status.before
|
||||
@@ -774,13 +796,22 @@ fi
|
||||
"#,
|
||||
);
|
||||
|
||||
let output = run_cmd_result_with_cwd_and_writable_roots(
|
||||
let cwd = AbsolutePathBuf::try_from(subdir.as_path()).expect("cwd should be absolute");
|
||||
let permission_profile = PermissionProfile::workspace_write_with(
|
||||
std::slice::from_ref(&cwd),
|
||||
NetworkSandboxPolicy::Enabled,
|
||||
/*exclude_tmpdir_env_var*/ false,
|
||||
/*exclude_slash_tmp*/ false,
|
||||
);
|
||||
let mut env = create_env_from_core_vars();
|
||||
env.insert("TMPDIR".to_string(), tmp_alias.display().to_string());
|
||||
let output = run_cmd_result_with_permission_profile_for_cwd(
|
||||
&["bash", "-lc", &script],
|
||||
&subdir,
|
||||
std::slice::from_ref(&subdir),
|
||||
cwd,
|
||||
permission_profile,
|
||||
env,
|
||||
LONG_TIMEOUT_MS,
|
||||
/*use_legacy_landlock*/ false,
|
||||
/*network_access*/ true,
|
||||
)
|
||||
.await
|
||||
.expect("sandboxed command should execute");
|
||||
@@ -790,6 +821,7 @@ fi
|
||||
"stdout:\n{}\nstderr:\n{}",
|
||||
output.stdout.text, output.stderr.text
|
||||
);
|
||||
assert!(!subdir.join(".git").exists());
|
||||
|
||||
let git_init_output = expect_denied(
|
||||
run_cmd_result_with_cwd_and_writable_roots(
|
||||
|
||||
@@ -41,6 +41,7 @@ async fn remote_project_trust_guards_thread_start_and_preserves_repository_decis
|
||||
std::fs::create_dir_all(&codex_home)?;
|
||||
std::fs::create_dir_all(&project_cwd)?;
|
||||
std::fs::create_dir(project_root.join(".git"))?;
|
||||
std::fs::write(project_root.join(".git/HEAD"), "ref: refs/heads/main\n")?;
|
||||
std::fs::create_dir(project_cwd.join(".codex"))?;
|
||||
let undecided_config = format!(
|
||||
"[{}]\n",
|
||||
|
||||
Reference in New Issue
Block a user