From 048f39dc23c5e9d9e87ccf2207118a55dcb550a9 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 20 Feb 2026 17:11:55 -0800 Subject: [PATCH] fix(permissions): harden deny_read no-sandbox retries --- codex-rs/core/src/tools/orchestrator.rs | 8 ++--- codex-rs/linux-sandbox/src/bwrap.rs | 40 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 2457dae8d6..e5010ddd36 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -215,11 +215,9 @@ impl ToolOrchestrator { }))); } // Managed filesystem read denies are non-overridable. If a command - // was denied by the sandbox and this was not a managed network - // policy denial, do not offer a no-sandbox retry path. - if turn_ctx.sandbox_policy.has_denied_read_paths() - && network_approval_context.is_none() - { + // was denied by the sandbox, do not offer a no-sandbox retry path + // because the retry would bypass deny_read enforcement. + if turn_ctx.sandbox_policy.has_denied_read_paths() { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, network_policy_decision, diff --git a/codex-rs/linux-sandbox/src/bwrap.rs b/codex-rs/linux-sandbox/src/bwrap.rs index 07c166d7a4..437fc896c3 100644 --- a/codex-rs/linux-sandbox/src/bwrap.rs +++ b/codex-rs/linux-sandbox/src/bwrap.rs @@ -309,7 +309,14 @@ fn apply_deny_read_overlays( .map(|root| root.root.as_path().to_path_buf()) .collect(); + let mut applied_paths: Vec = Vec::new(); for denied_path in deny_read_paths { + if applied_paths + .iter() + .any(|applied| denied_path.starts_with(applied)) + { + continue; + } if !denied_path.exists() { continue; } @@ -333,10 +340,12 @@ fn apply_deny_read_overlays( if is_dir { args.push("--tmpfs".to_string()); args.push(path_to_string(&denied_path)); + applied_paths.push(denied_path); } else { args.push("--ro-bind".to_string()); args.push("/dev/null".to_string()); args.push(path_to_string(&denied_path)); + applied_paths.push(denied_path); } } } @@ -627,4 +636,35 @@ mod tests { assert!(deny_overlay_index > readable_mount_index); } + + #[test] + fn deny_read_parent_directory_skips_redundant_child_overlay() { + let temp_dir = TempDir::new().expect("temp dir"); + let denied_dir = temp_dir.path().join("private"); + let denied_file = denied_dir.join("secret.txt"); + std::fs::create_dir(&denied_dir).expect("create denied dir"); + std::fs::write(&denied_file, "secret").expect("create denied file"); + + let policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + deny_read_paths: vec![ + AbsolutePathBuf::try_from(denied_dir.as_path()).expect("absolute dir"), + AbsolutePathBuf::try_from(denied_file.as_path()).expect("absolute file"), + ], + }; + + let args = create_filesystem_args(&policy, temp_dir.path()).expect("filesystem args"); + let denied_dir_str = path_to_string(&denied_dir); + let denied_file_str = path_to_string(&denied_file); + + assert!( + args.windows(2) + .any(|window| window == ["--tmpfs", denied_dir_str.as_str()]) + ); + assert!( + !args + .windows(3) + .any(|window| window == ["--ro-bind", "/dev/null", denied_file_str.as_str()]) + ); + } }