Files
codex/codex-rs/core/src/tools/sandboxing.rs
T
Michael Bolin b77fe8fefe Apply argument comment lint across codex-rs (#14652)
## Why

Once the repo-local lint exists, `codex-rs` needs to follow the
checked-in convention and CI needs to keep it from drifting. This commit
applies the fallback `/*param*/` style consistently across existing
positional literal call sites without changing those APIs.

The longer-term preference is still to avoid APIs that require comments
by choosing clearer parameter types and call shapes. This PR is
intentionally the mechanical follow-through for the places where the
existing signatures stay in place.

After rebasing onto newer `main`, the rollout also had to cover newly
introduced `tui_app_server` call sites. That made it clear the first cut
of the CI job was too expensive for the common path: it was spending
almost as much time installing `cargo-dylint` and re-testing the lint
crate as a representative test job spends running product tests. The CI
update keeps the full workspace enforcement but trims that extra
overhead from ordinary `codex-rs` PRs.

## What changed

- keep a dedicated `argument_comment_lint` job in `rust-ci`
- mechanically annotate remaining opaque positional literals across
`codex-rs` with exact `/*param*/` comments, including the rebased
`tui_app_server` call sites that now fall under the lint
- keep the checked-in style aligned with the lint policy by using
`/*param*/` and leaving string and char literals uncommented
- cache `cargo-dylint`, `dylint-link`, and the relevant Cargo
registry/git metadata in the lint job
- split changed-path detection so the lint crate's own `cargo test` step
runs only when `tools/argument-comment-lint/*` or `rust-ci.yml` changes
- continue to run the repo wrapper over the `codex-rs` workspace, so
product-code enforcement is unchanged

Most of the code changes in this commit are intentionally mechanical
comment rewrites or insertions driven by the lint itself.

## Verification

- `./tools/argument-comment-lint/run.sh --workspace`
- `cargo test -p codex-tui-app-server -p codex-tui`
- parsed `.github/workflows/rust-ci.yml` locally with PyYAML

---

* -> #14652
* #14651
2026-03-16 16:48:15 -07:00

368 lines
12 KiB
Rust

//! Shared approvals and sandboxing traits used by tool runtimes.
//!
//! Consolidates the approval flow primitives (`ApprovalDecision`, `ApprovalStore`,
//! `ApprovalCtx`, `Approvable`) together with the sandbox orchestration traits
//! and helpers (`Sandboxable`, `ToolRuntime`, `SandboxAttempt`, etc.).
use crate::codex::Session;
use crate::codex::TurnContext;
use crate::error::CodexErr;
#[cfg(test)]
use crate::protocol::SandboxPolicy;
use crate::sandboxing::CommandSpec;
use crate::sandboxing::SandboxManager;
use crate::sandboxing::SandboxPermissions;
use crate::sandboxing::SandboxTransformError;
use crate::state::SessionServices;
use crate::tools::network_approval::NetworkApprovalSpec;
use codex_network_proxy::NetworkProxy;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::approvals::NetworkApprovalContext;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::ReviewDecision;
use futures::Future;
use futures::future::BoxFuture;
use serde::Serialize;
use std::collections::HashMap;
use std::fmt::Debug;
use std::hash::Hash;
use std::path::Path;
use std::sync::Arc;
#[derive(Clone, Default, Debug)]
pub(crate) struct ApprovalStore {
// Store serialized keys for generic caching across requests.
map: HashMap<String, ReviewDecision>,
}
impl ApprovalStore {
pub fn get<K>(&self, key: &K) -> Option<ReviewDecision>
where
K: Serialize,
{
let s = serde_json::to_string(key).ok()?;
self.map.get(&s).cloned()
}
pub fn put<K>(&mut self, key: K, value: ReviewDecision)
where
K: Serialize,
{
if let Ok(s) = serde_json::to_string(&key) {
self.map.insert(s, value);
}
}
}
/// Takes a vector of approval keys and returns a ReviewDecision.
/// There will be one key in most cases, but apply_patch can modify multiple files at once.
///
/// - If all keys are already approved for session, we skip prompting.
/// - If the user approves for session, we store the decision for each key individually
/// so future requests touching any subset can also skip prompting.
pub(crate) async fn with_cached_approval<K, F, Fut>(
services: &SessionServices,
// Name of the tool, used for metrics collection.
tool_name: &str,
keys: Vec<K>,
fetch: F,
) -> ReviewDecision
where
K: Serialize,
F: FnOnce() -> Fut,
Fut: Future<Output = ReviewDecision>,
{
// To be defensive here, don't bother with checking the cache if keys are empty.
if keys.is_empty() {
return fetch().await;
}
let already_approved = {
let store = services.tool_approvals.lock().await;
keys.iter()
.all(|key| matches!(store.get(key), Some(ReviewDecision::ApprovedForSession)))
};
if already_approved {
return ReviewDecision::ApprovedForSession;
}
let decision = fetch().await;
services.session_telemetry.counter(
"codex.approval.requested",
/*inc*/ 1,
&[
("tool", tool_name),
("approved", decision.to_opaque_string()),
],
);
if matches!(decision, ReviewDecision::ApprovedForSession) {
let mut store = services.tool_approvals.lock().await;
for key in keys {
store.put(key, ReviewDecision::ApprovedForSession);
}
}
decision
}
#[derive(Clone)]
pub(crate) struct ApprovalCtx<'a> {
pub session: &'a Arc<Session>,
pub turn: &'a Arc<TurnContext>,
pub call_id: &'a str,
pub retry_reason: Option<String>,
pub network_approval_context: Option<NetworkApprovalContext>,
}
// Specifies what tool orchestrator should do with a given tool call.
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum ExecApprovalRequirement {
/// No approval required for this tool call.
Skip {
/// The first attempt should skip sandboxing (e.g., when explicitly
/// greenlit by policy).
bypass_sandbox: bool,
/// Proposed execpolicy amendment to skip future approvals for similar commands
/// Only applies if the command fails to run in sandbox and codex prompts the user to run outside the sandbox.
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
},
/// Approval required for this tool call.
NeedsApproval {
reason: Option<String>,
/// Proposed execpolicy amendment to skip future approvals for similar commands
/// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined.
proposed_execpolicy_amendment: Option<ExecPolicyAmendment>,
},
/// Execution forbidden for this tool call.
Forbidden { reason: String },
}
impl ExecApprovalRequirement {
pub fn proposed_execpolicy_amendment(&self) -> Option<&ExecPolicyAmendment> {
match self {
Self::NeedsApproval {
proposed_execpolicy_amendment: Some(prefix),
..
} => Some(prefix),
Self::Skip {
proposed_execpolicy_amendment: Some(prefix),
..
} => Some(prefix),
_ => None,
}
}
}
/// - Never, OnFailure: do not ask
/// - OnRequest: ask unless filesystem access is unrestricted
/// - Granular: ask unless filesystem access is unrestricted, but auto-reject
/// when granular sandbox approval is disabled.
/// - UnlessTrusted: always ask
pub(crate) fn default_exec_approval_requirement(
policy: AskForApproval,
file_system_sandbox_policy: &FileSystemSandboxPolicy,
) -> ExecApprovalRequirement {
let needs_approval = match policy {
AskForApproval::Never | AskForApproval::OnFailure => false,
AskForApproval::OnRequest | AskForApproval::Granular(_) => {
matches!(
file_system_sandbox_policy.kind,
FileSystemSandboxKind::Restricted
)
}
AskForApproval::UnlessTrusted => true,
};
if needs_approval
&& matches!(
policy,
AskForApproval::Granular(granular_config)
if !granular_config.allows_sandbox_approval()
)
{
ExecApprovalRequirement::Forbidden {
reason: "approval policy disallowed sandbox approval prompt".to_string(),
}
} else if needs_approval {
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: None,
}
} else {
ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
}
}
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum SandboxOverride {
NoOverride,
BypassSandboxFirstAttempt,
}
pub(crate) fn sandbox_override_for_first_attempt(
sandbox_permissions: SandboxPermissions,
exec_approval_requirement: &ExecApprovalRequirement,
) -> SandboxOverride {
// ExecPolicy `Allow` can intentionally imply full trust (Skip + bypass_sandbox=true),
// which supersedes `with_additional_permissions` sandboxed execution hints.
if sandbox_permissions.requires_escalated_permissions()
|| matches!(
exec_approval_requirement,
ExecApprovalRequirement::Skip {
bypass_sandbox: true,
..
}
)
{
SandboxOverride::BypassSandboxFirstAttempt
} else {
SandboxOverride::NoOverride
}
}
pub(crate) trait Approvable<Req> {
type ApprovalKey: Hash + Eq + Clone + Debug + Serialize;
// In most cases (shell, unified_exec), a request will have a single approval key.
//
// However, apply_patch needs session "Allow, don't ask again" semantics that
// apply to multiple atomic targets (e.g., apply_patch approves per file path). Returning
// a list of keys lets the runtime treat the request as approved-for-session only if
// *all* keys are already approved, while still caching approvals per-key so future
// requests touching a subset can be auto-approved.
fn approval_keys(&self, req: &Req) -> Vec<Self::ApprovalKey>;
/// Some tools may request to skip the sandbox on the first attempt
/// (e.g., when the request explicitly asks for escalated permissions).
/// Defaults to `NoOverride`.
fn sandbox_mode_for_first_attempt(&self, _req: &Req) -> SandboxOverride {
SandboxOverride::NoOverride
}
fn should_bypass_approval(&self, policy: AskForApproval, already_approved: bool) -> bool {
if already_approved {
// We do not ask one more time
return true;
}
matches!(policy, AskForApproval::Never)
}
/// Return `Some(_)` to specify a custom exec approval requirement, or `None`
/// to fall back to policy-based default.
fn exec_approval_requirement(&self, _req: &Req) -> Option<ExecApprovalRequirement> {
None
}
/// Decide we can request an approval for no-sandbox execution.
fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool {
match policy {
AskForApproval::OnFailure => true,
AskForApproval::UnlessTrusted => true,
AskForApproval::Never => false,
AskForApproval::OnRequest => false,
AskForApproval::Granular(granular_config) => granular_config.sandbox_approval,
}
}
fn start_approval_async<'a>(
&'a mut self,
req: &'a Req,
ctx: ApprovalCtx<'a>,
) -> BoxFuture<'a, ReviewDecision>;
}
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum SandboxablePreference {
Auto,
#[allow(dead_code)] // Will be used by later tools.
Require,
#[allow(dead_code)] // Will be used by later tools.
Forbid,
}
pub(crate) trait Sandboxable {
fn sandbox_preference(&self) -> SandboxablePreference;
fn escalate_on_failure(&self) -> bool {
true
}
}
pub(crate) struct ToolCtx {
pub session: Arc<Session>,
pub turn: Arc<TurnContext>,
pub call_id: String,
pub tool_name: String,
}
#[derive(Debug)]
pub(crate) enum ToolError {
Rejected(String),
Codex(CodexErr),
}
pub(crate) trait ToolRuntime<Req, Out>: Approvable<Req> + Sandboxable {
fn network_approval_spec(&self, _req: &Req, _ctx: &ToolCtx) -> Option<NetworkApprovalSpec> {
None
}
async fn run(
&mut self,
req: &Req,
attempt: &SandboxAttempt<'_>,
ctx: &ToolCtx,
) -> Result<Out, ToolError>;
}
pub(crate) struct SandboxAttempt<'a> {
pub sandbox: crate::exec::SandboxType,
pub policy: &'a crate::protocol::SandboxPolicy,
pub file_system_policy: &'a FileSystemSandboxPolicy,
pub network_policy: NetworkSandboxPolicy,
pub enforce_managed_network: bool,
pub(crate) manager: &'a SandboxManager,
pub(crate) sandbox_cwd: &'a Path,
pub codex_linux_sandbox_exe: Option<&'a std::path::PathBuf>,
pub use_legacy_landlock: bool,
pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel,
pub windows_sandbox_private_desktop: bool,
}
impl<'a> SandboxAttempt<'a> {
pub fn env_for(
&self,
spec: CommandSpec,
network: Option<&NetworkProxy>,
) -> Result<crate::sandboxing::ExecRequest, SandboxTransformError> {
self.manager
.transform(crate::sandboxing::SandboxTransformRequest {
spec,
policy: self.policy,
file_system_policy: self.file_system_policy,
network_policy: self.network_policy,
sandbox: self.sandbox,
enforce_managed_network: self.enforce_managed_network,
network,
sandbox_policy_cwd: self.sandbox_cwd,
#[cfg(target_os = "macos")]
macos_seatbelt_profile_extensions: None,
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe,
use_legacy_landlock: self.use_legacy_landlock,
windows_sandbox_level: self.windows_sandbox_level,
windows_sandbox_private_desktop: self.windows_sandbox_private_desktop,
})
}
}
#[cfg(test)]
#[path = "sandboxing_tests.rs"]
mod tests;