diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 24722f640b..871d3a2cc4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3280,6 +3280,7 @@ dependencies = [ "codex-context-fragments", "codex-exec-server-protocol", "codex-file-system", + "codex-guardian-context", "codex-history", "codex-mcp", "codex-protocol", diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs index 7b917868ab..58841fb8f5 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs @@ -1207,9 +1207,16 @@ async fn guardian_v2_routes_scoped_tool_approvals( assert!(history_texts.contains(&">>> TRANSCRIPT START\n")); assert!(history_texts.iter().any(|text| text.contains(USER_CONTEXT))); assert!(history_texts.iter().any(|text| text.contains("guardian-0"))); + assert!(history_texts.windows(2).any(|texts| { + texts + == [ + ">>> TRANSCRIPT END\n\n", + "\n>>> PARENT TURN PERMISSION CONTEXT START\n", + ] + })); assert_eq!( history_texts.last().copied(), - Some(">>> TRANSCRIPT END\n\n") + Some(">>> PARENT TURN PERMISSION CONTEXT END\n") ); assert!(history.iter().all(|item| item["type"] == "input_text")); diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index c8765943a5..588664d113 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -7,10 +7,12 @@ mod coverage; mod decision; mod feedback; mod input_budget; +mod permissions; mod prompt; pub(crate) use input_budget::PendingReviewContext; pub(crate) use input_budget::check_pending as check_pending_guardian_input; pub(crate) use input_budget::finalize as finalize_guardian_input; +pub(crate) use permissions::for_tool as tool_permission_context; mod request_budget; pub(crate) use request_budget::ExhaustedReviewBudget; pub(crate) use request_budget::check_prompt as check_guardian_prompt_budget; diff --git a/codex-rs/core/src/guardian/permissions.rs b/codex-rs/core/src/guardian/permissions.rs new file mode 100644 index 0000000000..3580d73eb9 --- /dev/null +++ b/codex-rs/core/src/guardian/permissions.rs @@ -0,0 +1,109 @@ +//! Resolves Guardian permission evidence from the host's captured environment context. +//! Sync review and async scoring use the same rules and environment identity. + +use super::GuardianReviewContext; +use crate::context::GuardianPermissionContext; +use crate::session::turn_context::TurnEnvironment; +use crate::tools::context::ToolInvocation; +use crate::tools::context::ToolPayload; +use crate::tools::handlers::RequestPermissionsEnvironmentArgs; + +pub(crate) async fn for_tool( + invocation: &ToolInvocation, +) -> anyhow::Result { + let context = GuardianReviewContext::from(&invocation.step_context); + if !invocation.tool_name.is_default_namespace() { + return for_environment(&context, /*environment_id*/ None); + } + let environment_id = match (invocation.tool_name.name.as_str(), &invocation.payload) { + ("apply_patch", ToolPayload::Custom { input }) => { + codex_apply_patch::parse_patch(input)?.environment_id + } + ("write_stdin", ToolPayload::Function { arguments }) => { + let arguments: serde_json::Value = serde_json::from_str(arguments)?; + let process_id = arguments["session_id"] + .as_i64() + .and_then(|id| i32::try_from(id).ok()) + .ok_or_else(|| anyhow::anyhow!("missing terminal session id"))?; + Some( + invocation + .session + .services + .unified_exec_manager + .environment_id_for_process(process_id) + .await + .ok_or_else(|| anyhow::anyhow!("terminal session is unavailable"))?, + ) + } + ("request_permissions", ToolPayload::Function { arguments }) => { + serde_json::from_str::(arguments)?.environment_id + } + ("exec_command" | "view_image", ToolPayload::Function { arguments }) => { + let arguments: serde_json::Value = serde_json::from_str(arguments)?; + serde_json::from_value::>(arguments["environment_id"].clone())? + } + _ => return for_environment(&context, /*environment_id*/ None), + }; + for_environment(&context, environment_id.as_deref()) +} + +pub(super) fn for_environment( + context: &GuardianReviewContext, + environment_id: Option<&str>, +) -> anyhow::Result { + let turn = context.turn(); + let environment = match environment_id { + Some(id) => Some( + context + .environments() + .turn_environments() + .find(|environment| environment.selection.environment_id == id) + .ok_or_else(|| anyhow::anyhow!("approval environment {id} is unavailable"))?, + ), + None => context.environments().primary(), + }; + let environment_id = + environment.map(|environment| environment.selection.environment_id.clone()); + let native_cwd = environment + .filter(|environment| !environment.environment.is_remote()) + .and_then(|environment| environment.cwd().to_abs_path().ok()); + let permission_profile = environment + .map(TurnEnvironment::permission_profile_with_workspace_roots) + .unwrap_or_else(|| turn.permission_profile_for_environments(context.environments())); + let file_system_policy = permission_profile.file_system_sandbox_policy(); + // Remote restrictions must not be interpreted using the filesystem running Guardian. + // Older executors may not report their temp folders. If a rule explicitly denies those + // folders, decline automatic approval rather than guess. Default rules do not deny them. + if let Some(environment) = environment + && native_cwd.is_none() + { + let sandbox = environment.sandbox_context(/*additional_permissions*/ None); + let paths = sandbox.policy_context(); + let mut denied_globs = file_system_policy + .get_unreadable_globs_with_context(&paths) + .map_err(anyhow::Error::msg)?; + denied_globs.sort(); + denied_globs.dedup(); + return Ok(GuardianPermissionContext { + environment_id, + denied_paths: file_system_policy + .get_unreadable_roots_with_context(&paths) + .map_err(anyhow::Error::msg)? + .into_iter() + .map(|path| path.inferred_native_path_string()) + .collect(), + denied_globs, + }); + } + #[allow(deprecated)] + let cwd = native_cwd.unwrap_or_else(|| turn.cwd.clone()); + Ok(GuardianPermissionContext { + environment_id, + denied_paths: file_system_policy + .get_unreadable_roots_with_cwd(&cwd) + .into_iter() + .map(|root| root.to_string_lossy().into_owned()) + .collect(), + denied_globs: file_system_policy.get_unreadable_globs_with_cwd(&cwd), + }) +} diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index a2d11eba87..9ccfa8baae 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -27,7 +27,6 @@ use crate::context::NodeReplReviewEvidenceMode; use crate::context::is_guardian_context_message; use crate::context::node_repl_review_evidence_mode; use crate::session::session::Session; -use crate::session::turn_context::TurnEnvironment; use codex_utils_output_truncation::TruncationPolicy; use codex_utils_output_truncation::approx_bytes_for_tokens; use codex_utils_output_truncation::truncate_text; @@ -153,7 +152,9 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( }), }; let permissions = parent_context - .map(|context| parent_turn_permissions(context, &request)) + .map(|context| { + super::permissions::for_environment(context, request.target_environment_id()) + }) .transpose()?; let node_repl_snapshot = if node_repl_transcripts_enabled { session @@ -219,65 +220,6 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( }) } -fn parent_turn_permissions( - context: &GuardianReviewContext, - request: &GuardianApprovalRequest, -) -> anyhow::Result { - let turn = context.turn(); - let environment = match request.target_environment_id() { - Some(id) => Some( - context - .environments() - .turn_environments() - .find(|environment| environment.selection.environment_id == id) - .ok_or_else(|| anyhow::anyhow!("approval environment {id} is unavailable"))?, - ), - None => context.environments().primary(), - }; - let native_cwd = environment - .filter(|environment| !environment.environment.is_remote()) - .and_then(|environment| environment.cwd().to_abs_path().ok()); - let permission_profile = environment - .map(TurnEnvironment::permission_profile_with_workspace_roots) - .unwrap_or_else(|| turn.permission_profile_for_environments(context.environments())); - let file_system_policy = permission_profile.file_system_sandbox_policy(); - // Remote restrictions must not be interpreted using the filesystem running Guardian. - // Older executors may not report their temp folders. If a rule explicitly denies those - // folders, decline automatic approval rather than guess. Default rules do not deny them. - if let Some(environment) = environment - && native_cwd.is_none() - { - let sandbox = environment.sandbox_context(/*additional_permissions*/ None); - let paths = sandbox.policy_context(); - let mut denied_globs = file_system_policy - .get_unreadable_globs_with_context(&paths) - .map_err(anyhow::Error::msg)?; - denied_globs.sort(); - denied_globs.dedup(); - return Ok(GuardianPermissionContext { - environment_id: request.target_environment_id().map(str::to_owned), - denied_paths: file_system_policy - .get_unreadable_roots_with_context(&paths) - .map_err(anyhow::Error::msg)? - .into_iter() - .map(|path| path.inferred_native_path_string()) - .collect(), - denied_globs, - }); - } - #[allow(deprecated)] - let cwd = native_cwd.unwrap_or_else(|| turn.cwd.clone()); - Ok(GuardianPermissionContext { - environment_id: request.target_environment_id().map(str::to_owned), - denied_paths: file_system_policy - .get_unreadable_roots_with_cwd(&cwd) - .into_iter() - .map(|root| root.to_string_lossy().into_owned()) - .collect(), - denied_globs: file_system_policy.get_unreadable_globs_with_cwd(&cwd), - }) -} - /// Exercises the sync profile through the host's existing transcript tests. #[cfg(test)] pub(crate) fn render_guardian_transcript_entries( diff --git a/codex-rs/core/src/guardian/review_request.rs b/codex-rs/core/src/guardian/review_request.rs index d7750cfd1a..56e9bebd43 100644 --- a/codex-rs/core/src/guardian/review_request.rs +++ b/codex-rs/core/src/guardian/review_request.rs @@ -23,6 +23,15 @@ impl ReviewHost for super::super::runtime::ReviewRuntime { type Prepared = PreparedApproval; type Evidence = ApprovalEvidence; + fn permissions(&self) -> Option { + let request = self.request.request.as_ref().ok()?; + crate::guardian::permissions::for_environment( + &self.context, + request.target_environment_id(), + ) + .ok() + } + async fn servicing_turn( &self, ) -> Option<(String, Arc)> { diff --git a/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_followup_review_request_layout.snap b/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_followup_review_request_layout.snap index 30b8e74b6e..8786cbd443 100644 --- a/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_followup_review_request_layout.snap +++ b/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_followup_review_request_layout.snap @@ -49,7 +49,7 @@ Scenario: Guardian follow-up review request layout [13] >>> PARENT TURN PERMISSION CONTEXT START - [14] For this action on environment "local", the active permission profile has no explicit denied-read paths/globs. + [14] The active permission profile for environment "local" has no explicit denied-read paths/globs. [15] >>> PARENT TURN PERMISSION CONTEXT END @@ -116,7 +116,7 @@ Scenario: Guardian follow-up review request layout [12] >>> PARENT TURN PERMISSION CONTEXT START - [13] For this action on environment "local", the active permission profile has no explicit denied-read paths/globs. + [13] The active permission profile for environment "local" has no explicit denied-read paths/globs. [14] >>> PARENT TURN PERMISSION CONTEXT END diff --git a/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_review_request_layout.snap b/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_review_request_layout.snap index 957a4ac541..d2f9e8d2d7 100644 --- a/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_review_request_layout.snap +++ b/codex-rs/core/src/guardian/snapshots/codex_core__guardian__tests__guardian_review_request_layout.snap @@ -52,7 +52,7 @@ Scenario: Guardian review request layout [14] >>> PARENT TURN PERMISSION CONTEXT START - [15] For this action on environment "local", the active permission profile has no explicit denied-read paths/globs. + [15] The active permission profile for environment "local" has no explicit denied-read paths/globs. [16] >>> PARENT TURN PERMISSION CONTEXT END diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index b72aa60309..6b8a676187 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -625,6 +625,13 @@ async fn approval_permissions_use_the_owning_environment() -> anyhow::Result<()> port: 443, trigger: None, }; + assert_eq!( + super::permissions::for_environment(&context, /*environment_id*/ None)?, + super::permissions::for_environment( + &context, + Some(codex_exec_server::LOCAL_ENVIRONMENT_ID), + )?, + ); let requests = [ network_request("secondary"), network_request("windows"), @@ -674,7 +681,7 @@ async fn approval_permissions_use_the_owning_environment() -> anyhow::Result<()> .await?; let text = guardian_prompt_text(&prompt.context.into_user_inputs()?); assert!(text.contains(&format!( - "For this action on environment {environment_id:?}" + "The active permission profile for environment {environment_id:?}" ))); if is_windows { assert!( diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 38cc41c748..2d0b6177e1 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -67,6 +67,7 @@ pub use mcp_resource::ListMcpResourcesHandler; pub use mcp_resource::ReadMcpResourceHandler; pub use new_context_window::NewContextWindowHandler; pub use plan::PlanHandler; +pub(crate) use request_permissions::RequestPermissionsEnvironmentArgs; pub use request_permissions::RequestPermissionsHandler; pub use request_plugin_install::RequestPluginInstallHandler; pub use request_user_input::RequestUserInputHandler; diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index ececd52c6d..a4edc0ce9a 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -22,9 +22,9 @@ use serde::Deserialize; pub struct RequestPermissionsHandler; #[derive(Deserialize)] -struct RequestPermissionsEnvironmentArgs { +pub(crate) struct RequestPermissionsEnvironmentArgs { #[serde(default, rename = "environment_id", alias = "environmentId")] - environment_id: Option, + pub(crate) environment_id: Option, } impl ToolExecutor for RequestPermissionsHandler { diff --git a/codex-rs/core/src/tools/lifecycle.rs b/codex-rs/core/src/tools/lifecycle.rs index 82ba4a6c6b..e26d83da6b 100644 --- a/codex-rs/core/src/tools/lifecycle.rs +++ b/codex-rs/core/src/tools/lifecycle.rs @@ -12,6 +12,7 @@ use codex_protocol::mcp::CallToolResult; use codex_tools::ToolName; use codex_utils_path_uri::PathUri; +use crate::guardian::tool_permission_context; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::tools::context::ToolCallSource; @@ -49,6 +50,7 @@ pub(crate) async fn notify_tool_start( originating_item_id: originating_item_id.as_ref(), tool_name: &invocation.tool_name, mcp_tool, + permissions: Box::pin(async { tool_permission_context(invocation).await.ok() }), payload: &invocation.payload, conversation_history: Arc::clone(&conversation_history), source: extension_tool_call_source(invocation.source.clone()), diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 6f15f174b4..d8433551e5 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -1816,6 +1816,16 @@ impl UnifiedExecProcessManager { } } + /// Resolves stdin's target from the host-owned terminal, not model-supplied metadata. + pub(crate) async fn environment_id_for_process(&self, process_id: i32) -> Option { + self.process_store + .lock() + .await + .processes + .get(&process_id) + .map(|entry| entry.environment_id.clone()) + } + pub(crate) async fn list_processes(&self) -> Vec { let store = self.process_store.lock().await; let mut entries = store diff --git a/codex-rs/core/tests/suite/guardian_environments_tests.rs b/codex-rs/core/tests/suite/guardian_environments_tests.rs index 280ebeb6a0..5f720bddda 100644 --- a/codex-rs/core/tests/suite/guardian_environments_tests.rs +++ b/codex-rs/core/tests/suite/guardian_environments_tests.rs @@ -1,8 +1,16 @@ //! Reviews bind each action to its captured target policy while reusing the reviewer context. use super::*; +use codex_history::RolloutItem; +use core_test_support::streaming_sse::StreamingSseChunk; +use core_test_support::streaming_sse::start_streaming_sse_server; use pretty_assertions::assert_eq; use test_case::test_case; +use wiremock::Mock; +use wiremock::ResponseTemplate; +use wiremock::matchers::body_partial_json; +use wiremock::matchers::method; +use wiremock::matchers::path; #[test_case("exec_command")] #[test_case("apply_patch")] @@ -21,6 +29,17 @@ async fn guardian_reviews_target_environment_and_reuses_prefix(tool: &str) -> Re let server = start_mock_server().await; let test = test_codex() + .with_pre_build_hook(|home| { + std::fs::write( + home.join("config.toml"), + "[features.guardianv2]\nenabled = true\npersist_scores = true\n\n[features.guardianv2.review_scope]\ncomputer_use_only = false\n", + ) + .expect("configure asynchronous review"); + }) + .with_model_info_override("guardian-environments-parent", |model| { + model.guardian = None; + model.auto_review_model_override = Some("gpt-5.5".to_owned()); + }) .with_config(|config| { config.project_doc_max_bytes = 0; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); @@ -37,10 +56,6 @@ async fn guardian_reviews_target_environment_and_reuses_prefix(tool: &str) -> Re .features .disable(Feature::DeferredExecutor) .expect("disable deferred executor"); - config - .features - .disable(Feature::GuardianV2) - .expect("disable Guardian prewarm"); }) .build_with_auto_env(&server) .await?; @@ -122,7 +137,61 @@ async fn guardian_reviews_target_environment_and_reuses_prefix(tool: &str) -> Re ev_assistant_message("done", "done"), ev_completed("done"), ])); - let responses = mount_sse_sequence(&server, events).await; + // Hold the second action until the first score is published. Its own classifier + // response stays pending, so only the score for the other computer is available. + let (advance, next_action) = tokio::sync::oneshot::channel(); + let mut next_action = Some(next_action); + let (parent, _) = start_streaming_sse_server( + events + .iter() + .step_by(2) + .enumerate() + .map(|(index, body)| { + vec![StreamingSseChunk { + gate: if index == 1 { next_action.take() } else { None }, + body: body.clone(), + }] + }) + .collect(), + ) + .await; + let (publish, score_ready) = tokio::sync::oneshot::channel(); + let (_hold_score, pending_score) = tokio::sync::oneshot::channel(); + let (classifier, _) = start_streaming_sse_server(vec![ + vec![StreamingSseChunk { + gate: Some(score_ready), + body: sse(vec![ + ev_assistant_message("score-0", "low"), + ev_completed("score-0"), + ]), + }], + vec![StreamingSseChunk { + gate: Some(pending_score), + body: sse(vec![ + ev_assistant_message("score-1", "low"), + ev_completed("score-1"), + ]), + }], + ]) + .await; + for (model, destination) in [ + ("guardian-environments-parent", parent.uri()), + ("gpt-5.6-luna", classifier.uri()), + ] { + Mock::given(method("POST")) + .and(path("/v1/responses")) + .and(body_partial_json(json!({"model": model}))) + .respond_with( + ResponseTemplate::new(/*s*/ 307) + .insert_header("location", format!("{destination}/v1/responses")), + ) + .with_priority(/*p*/ 1) + .mount(&server) + .await; + } + let responses = + mount_sse_sequence(&server, events.into_iter().skip(1).step_by(2).collect()).await; + test.codex.ensure_rollout_materialized().await; test.codex .start_or_steer_turn( TurnInputRequest::user_input(vec![UserInput::Text { @@ -138,6 +207,23 @@ async fn guardian_reviews_target_environment_and_reuses_prefix(tool: &str) -> Re }), ) .await?; + tokio::time::timeout(std::time::Duration::from_secs(30), async { + parent.wait_for_request_count(/*count*/ 2).await; + publish.send(()).expect("publish the first computer score"); + loop { + let history = test.codex.load_history(/*include_archived*/ false).await?; + if history.items.into_iter().any(|item| { + matches!(item, RolloutItem::SecurityRiskScore(score) if score.call_id.as_deref() == Some("action-0")) + }) { + return Ok::<_, anyhow::Error>(()); + } + tokio::task::yield_now().await; + } + }) + .await??; + advance + .send(()) + .expect("start action on the primary computer"); wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) }) @@ -181,6 +267,14 @@ async fn guardian_reviews_target_environment_and_reuses_prefix(tool: &str) -> Re reviews[1].input().starts_with(&reviews[0].input()), "earlier Guardian input must remain unchanged" ); + let classifier_requests = classifier.requests().await; + let request: Value = serde_json::from_slice(&classifier_requests[0])?; + let input = request["input"].to_string(); + assert!(input.contains(secondary_id)); + assert!(input.contains("secondary-private")); + test.codex.shutdown_and_wait().await?; + parent.shutdown().await; + classifier.shutdown().await; shutdown.send(()).expect("stop secondary executor"); executor.await?; Ok(()) diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index 536cdfa443..09d89f4e29 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -1697,7 +1697,7 @@ async fn guardian_session_is_reused_for_consecutive_tool_reviews_without_prewarm let permission_section = [ "\n>>> PARENT TURN PERMISSION CONTEXT START\n".to_string(), format!( - "For this action on environment \"local\", the active permission profile denies reading these paths/globs. These are policy restrictions; do not approve escalation whose purpose is to read them.\n- path `{}`\n- glob `{}`\n", + "The active permission profile for environment \"local\" denies reading these paths/globs. These are policy restrictions; do not approve escalation whose purpose is to read them.\n- path `{}`\n- glob `{}`\n", fs::canonicalize(&secret_file)?.display(), test.config.cwd.join("guardian-*.key").display(), ), diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 491c72631e..3a536f64f4 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -1222,8 +1222,8 @@ async fn guardian_checkpoint_migration_request_history() -> Result<()> { "$1\"\"", ), ( - r#"(For this action on environment )"(?:local|remote)","#, - "$1\"\",", + r#"(The active permission profile for environment )"(?:local|remote)""#, + "$1\"\"", ), (r#"(?m)^(\s*"cwd": )"[^"]*""#, "$1\"\""), ( diff --git a/codex-rs/core/tests/suite/scenarios_guardian_extra_policy.rs b/codex-rs/core/tests/suite/scenarios_guardian_extra_policy.rs index 7a77d86cc3..4ab85ec00f 100644 --- a/codex-rs/core/tests/suite/scenarios_guardian_extra_policy.rs +++ b/codex-rs/core/tests/suite/scenarios_guardian_extra_policy.rs @@ -125,8 +125,8 @@ guardian_extra_policy = "Draft reminders without sending them." "\"environment_id\": \"\"", ) .replace( - &format!("For this action on environment {environment_id:?},"), - "For this action on environment \"\",", + &format!("The active permission profile for environment {environment_id:?}"), + "The active permission profile for environment \"\"", ); for (pattern, replacement) in [ (r#"(?m)^(\s*"cwd": )"[^"]*""#, "$1\"\""), diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_checkpoint_migration.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_checkpoint_migration.snap index a4fc72be05..46344b70a1 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_checkpoint_migration.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_checkpoint_migration.snap @@ -152,7 +152,7 @@ Settings: relative to window 2 [23] >>> PARENT TURN PERMISSION CONTEXT START - [24] For this action on environment "", the active permission profile has no explicit denied-read paths/globs. + [24] The active permission profile for environment "" has no explicit denied-read paths/globs. [25] >>> PARENT TURN PERMISSION CONTEXT END @@ -305,7 +305,7 @@ Settings: same as window 3 [22] >>> PARENT TURN PERMISSION CONTEXT START - [23] For this action on environment "", the active permission profile has no explicit denied-read paths/globs. + [23] The active permission profile for environment "" has no explicit denied-read paths/globs. [24] >>> PARENT TURN PERMISSION CONTEXT END diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_extra_policy__extra_policy_reaches_guardian_review.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_extra_policy__extra_policy_reaches_guardian_review.snap index de7dd0832e..dcb88267cf 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_extra_policy__extra_policy_reaches_guardian_review.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_extra_policy__extra_policy_reaches_guardian_review.snap @@ -154,7 +154,7 @@ Settings: relative to window 1 [12] >>> PARENT TURN PERMISSION CONTEXT START - [13] For this action on environment "", the active permission profile has no explicit denied-read paths/globs. + [13] The active permission profile for environment "" has no explicit denied-read paths/globs. [14] >>> PARENT TURN PERMISSION CONTEXT END diff --git a/codex-rs/ext/extension-api/Cargo.toml b/codex-rs/ext/extension-api/Cargo.toml index e98063a8d2..4b487226eb 100644 --- a/codex-rs/ext/extension-api/Cargo.toml +++ b/codex-rs/ext/extension-api/Cargo.toml @@ -16,6 +16,7 @@ workspace = true [dependencies] codex-api = { workspace = true } futures = { workspace = true } +codex-guardian-context = { workspace = true } codex-history = { workspace = true } codex-config = { workspace = true } codex-context-fragments = { workspace = true } diff --git a/codex-rs/ext/extension-api/src/contributors/approval_review.rs b/codex-rs/ext/extension-api/src/contributors/approval_review.rs index ebff9c7a56..2f813c0849 100644 --- a/codex-rs/ext/extension-api/src/contributors/approval_review.rs +++ b/codex-rs/ext/extension-api/src/contributors/approval_review.rs @@ -33,6 +33,8 @@ pub struct ApprovalDecisionInput<'a> { pub approval_id: &'a str, /// Host tool invocation being approved, absent for approvals without a tool call. pub tool_call_id: Option<&'a str>, + /// Host-resolved permission evidence; unavailable evidence cannot authorize cached approval. + pub permissions: Option<&'a codex_guardian_context::PermissionContext>, pub action: &'a serde_json::Value, pub thread_id: ThreadId, pub thread_store: &'a ExtensionData, diff --git a/codex-rs/ext/extension-api/src/contributors/tool_lifecycle.rs b/codex-rs/ext/extension-api/src/contributors/tool_lifecycle.rs index 464389cf66..3b31eaaf56 100644 --- a/codex-rs/ext/extension-api/src/contributors/tool_lifecycle.rs +++ b/codex-rs/ext/extension-api/src/contributors/tool_lifecycle.rs @@ -135,6 +135,8 @@ pub struct ToolStartInput<'a> { pub tool_name: &'a ToolName, /// Read-only metadata and provenance from the exact MCP call that will execute. pub mcp_tool: Option<&'a McpToolContext>, + /// Resolves evidence from the issuing step only when awaited; `None` prevents cached approval. + pub permissions: crate::ExtensionFuture<'a, Option>, /// Finalized tool arguments, including any pre-tool-use hook rewrites. /// /// Payloads can contain sensitive plaintext and must not be logged. diff --git a/codex-rs/ext/extension-api/tests/registry.rs b/codex-rs/ext/extension-api/tests/registry.rs index c27718b9a5..daf919269a 100644 --- a/codex-rs/ext/extension-api/tests/registry.rs +++ b/codex-rs/ext/extension-api/tests/registry.rs @@ -204,6 +204,7 @@ async fn build_round_trips_every_contributor_category() { assert_eq!(registry.turn_item_contributors().len(), 1); let thread_store = ExtensionData::new("thread"); let input = codex_extension_api::ApprovalDecisionInput { + permissions: Some(&Default::default()), approval_id: "approval-1", tool_call_id: None, action: &serde_json::Value::Null, diff --git a/codex-rs/ext/guardian-reviewer/src/review.rs b/codex-rs/ext/guardian-reviewer/src/review.rs index 86c4f0ab8c..9fade3c15d 100644 --- a/codex-rs/ext/guardian-reviewer/src/review.rs +++ b/codex-rs/ext/guardian-reviewer/src/review.rs @@ -29,6 +29,8 @@ pub trait ReviewHost: Send + Sync { type Prepared: Send + Sync; /// Evidence captured for one completed assessment, never reused by another attempt. type Evidence: Send; + /// Returns the target's rules, or no evidence when they cannot be resolved. + fn permissions(&self) -> Option; /// Captures the turn currently servicing reviews, which may differ from a yielded cell's origin. fn servicing_turn(&self) -> impl Future)>> + Send; /// Returns the owning turn and optional target item after validating the action. diff --git a/codex-rs/ext/guardian-reviewer/src/routing.rs b/codex-rs/ext/guardian-reviewer/src/routing.rs index 87a872f211..b2cc9cfeff 100644 --- a/codex-rs/ext/guardian-reviewer/src/routing.rs +++ b/codex-rs/ext/guardian-reviewer/src/routing.rs @@ -87,7 +87,9 @@ impl ReviewRequest<'_, H> { "automatic approval review could not prepare the action", )); }; + let permissions = self.host.permissions(); let input = ApprovalDecisionInput { + permissions: permissions.as_ref(), approval_id: self.approval_id, tool_call_id: self.tool_call_id, action, diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs b/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs index 82e215ee9f..0562213841 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/approval.rs @@ -188,7 +188,11 @@ async fn cached_evidence( record_fast_decision(metrics, "approved", "initial_cua_call"); return Ok(()); } - let current = ScoreAuthorization::current(thread).await; + let Some(permissions) = input.permissions else { + record_fast_decision(metrics, "deferred", "permission_resolution_error"); + return Err(GuardianReviewReason::AuthorizationChanged); + }; + let current = ScoreAuthorization::current(thread, permissions).await; // Classification may publish or fail while authorization is collected. let cached = progress.inspect(input.tool_call_id); if cached.oversized { diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/authorization.rs b/codex-rs/ext/guardian-v2/src/async_scorer/authorization.rs index 8bee74b5b3..b4b67f06fa 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/authorization.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/authorization.rs @@ -5,6 +5,7 @@ use codex_core::GuardianAuthorizationVersion; #[derive(Clone, Debug, PartialEq)] pub(super) struct ScoreAuthorization { + pub(super) permissions: codex_guardian_context::PermissionContext, pub(super) settings: codex_protocol::protocol::ThreadSettingsSnapshot, pub(super) environments: Vec, pub(super) local: GuardianAuthorizationVersion, @@ -13,12 +14,16 @@ pub(super) struct ScoreAuthorization { } impl ScoreAuthorization { - pub(super) async fn current(thread: &CodexThread) -> Self { + pub(super) async fn current( + thread: &CodexThread, + permissions: &codex_guardian_context::PermissionContext, + ) -> Self { let root = thread .guardian_root_snapshot() .await .map(|snapshot| snapshot.authorization_version); Self { + permissions: permissions.clone(), settings: thread.thread_settings_snapshot().await, environments: thread .config_snapshot() diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/budget_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/budget_tests.rs index 3774542a50..945a4604b9 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/budget_tests.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/budget_tests.rs @@ -196,7 +196,8 @@ async fn assert_catalog_budget(evidence: BudgetEvidence) -> Result<()> { }, ); let progress = thread_store.get::().unwrap(); - let authorization = ScoreAuthorization::current(&fixture.test.codex).await; + let authorization = + ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await; seed_cached_score(&progress, thread_store, /*index*/ 0, authorization); assert_eq!( cached_approval( @@ -216,6 +217,7 @@ async fn assert_catalog_budget(evidence: BudgetEvidence) -> Result<()> { }; fixture.registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &fixture.session_store, thread_store, turn_store: &turn_store, diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/classification.rs b/codex-rs/ext/guardian-v2/src/async_scorer/classification.rs index 80aed65e5a..13399b50b0 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/classification.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/classification.rs @@ -190,6 +190,7 @@ impl Classification { root_conversation: root_conversation.as_deref().unwrap_or_default(), trusted_user_answers: &trusted_user_inputs, planned_action: Some(&action_section), + permissions: Some(&score_authorization.permissions), previous_reviews: Some(&reviews), trusted_tool: trusted_tool_context.as_ref(), trusted_skill_paths: &trusted_skill_paths, @@ -302,7 +303,9 @@ impl Classification { ), sampled_at: Some(sampled_at.into()), }; - if score_authorization != ScoreAuthorization::current(&thread).await { + if score_authorization + != ScoreAuthorization::current(&thread, &score_authorization.permissions).await + { return Ok(ClassificationOutcome::Superseded); } let accepted = diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs index 2a1403cc32..b7a294d060 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/extension_tests.rs @@ -287,6 +287,7 @@ async fn installed_extension_uses_http_after_warm_socket_auth_expires() -> Resul } registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &session_store, thread_store, turn_store: &turn_store, @@ -557,6 +558,7 @@ async fn sandboxed_shell_classification_respects_review_scope() -> Result<()> { ] { fixture.registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { panic!("unexpected permission resolution") }), session_store: &fixture.session_store, thread_store, turn_store: &turn_store, @@ -645,8 +647,7 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> { .get::() .expect("Guardian v2 should track score progress per thread"); // The seeded low score belongs to the model selected above. - let authorization = - super::super::authorization::ScoreAuthorization::current(&fixture.test.codex).await; + let authorization = ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await; seed_cached_score(&progress, thread_store, /*index*/ 1, authorization); let cached = progress.inspect(/*call_id*/ None); let turn_store = ExtensionData::new("turn-1"); @@ -656,6 +657,7 @@ async fn computer_use_only_scores_cannot_approve_other_actions() -> Result<()> { }; fixture.registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &fixture.session_store, thread_store, turn_store: &turn_store, @@ -972,6 +974,7 @@ async fn sample_configured_conversation_history_with_source( registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &session_store, thread_store, turn_store: &turn_store, @@ -1091,6 +1094,7 @@ impl GuardianFailureFixture { }; self.registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &self.session_store, thread_store, turn_store: &turn_store, @@ -1560,7 +1564,7 @@ max_recent_non_user_entries = 8 &score_progress, thread_store, first_unscored, - ScoreAuthorization::current(&test.codex).await, + ScoreAuthorization::current(&test.codex, &Default::default()).await, ); assert_eq!( cached_approval( @@ -2101,6 +2105,35 @@ async fn contributor_samples_tool_calls_with_the_existing_luna_pool() -> Result< Some(ReviewDecision::Approved) ); + let progress = thread_store.get::().unwrap(); + for permissions in [ + codex_guardian_context::PermissionContext { + environment_id: Some("other-computer".to_owned()), + ..Default::default() + }, + codex_guardian_context::PermissionContext { + denied_paths: vec!["/private".to_owned()], + ..Default::default() + }, + ] { + seed_cached_score( + &progress, + thread_store, + /*index*/ 1, + ScoreAuthorization::current(&test.codex, &permissions).await, + ); + assert_eq!( + cached_approval( + ®istry, + thread_store, + "review action", + /*metrics*/ None + ) + .await, + None + ); + } + let disabled_thread_store = ExtensionData::new("disabled-thread"); set_cached_score( &disabled_thread_store, @@ -2258,7 +2291,7 @@ async fn contributor_skips_required_models_in_standard_scope() -> Result<()> { thread_store.insert(model_info); // A late prewarm preview must leave the active model's review requirements intact. let _ = codex_core::guardian_review::prepare_review_prewarm(&test.codex).await?; - let authorization = ScoreAuthorization::current(&test.codex).await; + let authorization = ScoreAuthorization::current(&test.codex, &Default::default()).await; let progress = thread_store .get::() .expect("Guardian v2 should track score progress per thread"); @@ -2296,6 +2329,7 @@ async fn contributor_skips_required_models_in_standard_scope() -> Result<()> { }; registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { panic!("unexpected permission resolution") }), session_store: &session_store, thread_store, turn_store: &turn_store, @@ -2451,7 +2485,8 @@ async fn incompatible_compaction_blocks_cached_score_and_initial_cua_allowance() fixture.test.codex.guardian_authorization_version().await, authorization ); - let score_authorization = ScoreAuthorization::current(&fixture.test.codex).await; + let score_authorization = + ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await; let progress = thread_store .get::() .expect("score progress"); @@ -2553,6 +2588,7 @@ async fn contributor_counts_failed_thread_lookups_toward_score_lag() -> Result<( }; registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &session_store, thread_store, turn_store: &turn_store, @@ -3057,6 +3093,7 @@ async fn assert_parent_compaction_reuse(parent_context_for_review: bool) -> Resu registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &session_store, thread_store, turn_store: &turn_store, @@ -3134,6 +3171,7 @@ async fn assert_parent_compaction_reuse(parent_context_for_review: bool) -> Resu }; registry.tool_lifecycle_contributors()[0] .on_tool_start(ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &session_store, thread_store, turn_store: &turn_store, @@ -3229,6 +3267,7 @@ async fn cached_approval_discounts_only_its_own_unscored_wrapper() -> Result<()> }, }; progress.observe(&ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: &fixture.session_store, thread_store: store, turn_store: &fixture.session_store, @@ -3271,14 +3310,14 @@ async fn cached_approval_discounts_only_its_own_unscored_wrapper() -> Result<()> &progress, store, wrapper, - ScoreAuthorization::current(&fixture.test.codex).await, + ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await, ); assert_eq!(approve("third").await, None); seed_cached_score( &progress, store, wrapper + 3, - ScoreAuthorization::current(&fixture.test.codex).await, + ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await, ); let output = start("output-only", &origin, ToolCallSource::Direct); let other = ResponseItemId::from_server("other-wrapper".to_owned()); @@ -3293,7 +3332,7 @@ async fn cached_approval_discounts_only_its_own_unscored_wrapper() -> Result<()> &progress, store, output, - ScoreAuthorization::current(&fixture.test.codex).await, + ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await, ); assert_eq!( approve("other-second").await, @@ -3339,6 +3378,7 @@ async fn cached_approval( } }; let input = codex_extension_api::ApprovalDecisionInput { + permissions: Some(&Default::default()), approval_id: "cache-probe", tool_call_id: action.get("id").and_then(serde_json::Value::as_str), action: &action, @@ -3391,6 +3431,7 @@ fn seed_cached_score( fn observe_unscored_call(progress: &GuardianV2ScoreProgress, store: &ExtensionData) -> usize { progress.observe(&ToolStartInput { + permissions: Box::pin(async { Some(Default::default()) }), session_store: store, thread_store: store, turn_store: store, @@ -3415,7 +3456,7 @@ async fn cached_score_publication_rejects_delayed_results_without_changing_cover let fixture = GuardianFailureFixture::new().await?; let store = fixture.test.codex.thread_extension_data(); let progress = store.get::().unwrap(); - let authorization = ScoreAuthorization::current(&fixture.test.codex).await; + let authorization = ScoreAuthorization::current(&fixture.test.codex, &Default::default()).await; seed_cached_score(&progress, store, /*index*/ 1, authorization.clone()); let score = cached_score(store).unwrap(); observe_unscored_call(&progress, store); diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/observation.rs b/codex-rs/ext/guardian-v2/src/async_scorer/observation.rs index 90563df016..f1990c3534 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/observation.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/observation.rs @@ -310,7 +310,17 @@ impl GuardianV2Extension { None }; - let score_authorization = ScoreAuthorization::current(&thread).await; + let Some(permissions) = input.permissions.await else { + score_progress.fail_closed(sampled_at); + record_classification( + metrics.as_deref(), + classification_started_at.elapsed(), + "failure", + Some("permission_resolution_error"), + ); + return; + }; + let score_authorization = ScoreAuthorization::current(&thread, &permissions).await; let classification = Classification { classification_started_at, sampler, diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs b/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs index 5b1678b0e0..1650993e64 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/transcript.rs @@ -45,6 +45,7 @@ pub(crate) struct ContextInput<'a> { pub(crate) root_conversation: &'a [GuardianRootMessage], pub(crate) trusted_user_answers: &'a [String], pub(crate) planned_action: Option<&'a PlannedAction>, + pub(crate) permissions: Option<&'a codex_guardian_context::PermissionContext>, pub(crate) previous_reviews: Option<&'a PreviousReviews>, pub(crate) trusted_tool: Option<&'a TrustedTool>, pub(crate) trusted_skill_paths: &'a [String], @@ -89,6 +90,7 @@ impl TranscriptConfig { root_conversation, trusted_user_answers, planned_action, + permissions, previous_reviews, trusted_tool, trusted_skill_paths, @@ -123,7 +125,7 @@ impl TranscriptConfig { root_conversation, trusted_user_answers, planned_action, - permissions: None, + permissions, previous_reviews, trusted_tool, trusted_skill_paths, diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs b/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs index ea586bc52a..97a18e6905 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/transcript_tests.rs @@ -101,6 +101,7 @@ fn transcript_keeps_conversation_and_configured_sources() { let transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -128,6 +129,7 @@ fn transcript_keeps_conversation_and_configured_sources() { let answers = ["assistant: Publish?\nuser: No.\n".to_string()]; let context = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &root, @@ -172,6 +174,7 @@ fn transcript_keeps_conversation_and_configured_sources() { let transcript = output_and_reasoning .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -200,6 +203,7 @@ fn transcript_keeps_conversation_and_configured_sources() { let transcript = calls_only .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -237,6 +241,7 @@ fn transcript_truncates_oversized_assistant_entries_without_splitting_characters let mut rendered = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -303,6 +308,7 @@ fn transcript_preserves_user_restrictions_before_final_assistant_messages() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -343,6 +349,7 @@ fn transcript_preserves_recent_tool_evidence_when_protected_messages_fill_entry_ ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -409,6 +416,7 @@ fn transcript_reserves_five_recent_tool_entries_from_protected_messages() { let transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -465,6 +473,7 @@ fn rejected_commentary_does_not_evict_retained_message_evidence() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -506,6 +515,7 @@ fn transcript_evicts_protected_messages_in_cacheable_chunks() { })); config .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -577,6 +587,7 @@ fn transcript_preserves_latest_final_when_reserved_tools_fill_entry_window() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -650,6 +661,7 @@ fn transcript_does_not_protect_legacy_inter_agent_instructions() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -697,6 +709,7 @@ fn transcript_reserves_separate_budget_for_recent_tool_evidence() { let transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -759,6 +772,7 @@ fn transcript_reserves_separate_budget_for_recent_tool_evidence() { }); let next_transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -823,6 +837,7 @@ fn transcript_preserves_newest_manual_approval_when_message_budget_overflows() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -883,6 +898,7 @@ fn rejected_message_does_not_evict_retained_tool_entries() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -928,6 +944,7 @@ fn transcript_evicts_non_user_entries_in_cacheable_chunks() { ); config .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -1001,6 +1018,7 @@ fn transcript_truncates_tool_results_using_standard_budget() { let transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -1062,6 +1080,7 @@ fn configured_reasoning_counts_against_message_budget() { ..TranscriptConfig::default() } .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -1151,6 +1170,7 @@ fn transcript_omits_media_payloads_and_keeps_readable_content() { let transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], @@ -1220,6 +1240,7 @@ fn transcript_omits_encrypted_messages_arguments_and_tool_outputs() { let transcript = TranscriptConfig::default() .build_context(ContextInput { + permissions: None, target: ContextTarget::Async, history: &TestConversationHistory(&items), root_conversation: &[], diff --git a/codex-rs/guardian-context/src/permissions.rs b/codex-rs/guardian-context/src/permissions.rs index fa10b25fe3..1013ee5978 100644 --- a/codex-rs/guardian-context/src/permissions.rs +++ b/codex-rs/guardian-context/src/permissions.rs @@ -1,4 +1,4 @@ -//! Sync permission evidence formatting. The host resolves the reviewed environment +//! Shared permission evidence formatting. The host resolves the reviewed environment //! and its filesystem policy; this section neither resolves nor relaxes restrictions. use crate::ContextSection; @@ -50,9 +50,9 @@ impl ContextualUserFragment for PermissionContext { ) .collect::>(); let scope = match &self.environment_id { - Some(environment_id) => format!( - "For this action on environment {environment_id:?}, the active permission profile" - ), + Some(environment_id) => { + format!("The active permission profile for environment {environment_id:?}") + } None => "The parent turn's active permission profile".to_string(), }; if entries.is_empty() { @@ -66,11 +66,14 @@ impl ContextualUserFragment for PermissionContext { } } +// Keep async permission evidence below one thousand estimated tokens. +const MAX_ASYNC_PERMISSION_BYTES: usize = 3_000; + pub(crate) struct PermissionContextSection; impl SectionContributor for PermissionContextSection { fn scope(&self) -> SectionScope { - SectionScope::SyncOnly + SectionScope::Shared } fn contribute(&self, input: &SectionInput<'_>) -> Result, SectionError> { @@ -83,9 +86,16 @@ impl SectionContributor for PermissionContextSection { { return Ok(None); } + let body = permissions.body(); + // Async scoring falls back to synchronous review instead of dropping restrictions. + if input.target == crate::ContextTarget::Async && body.len() > MAX_ASYNC_PERMISSION_BYTES { + return Err(SectionError::EvidenceLimitExceeded { + section: "permissions", + }); + } let (start, end) = permissions.markers(); Ok(Some(ContextSection::PermissionContext { - items: vec![start.into(), permissions.body(), end.into()], + items: vec![start.into(), body, end.into()], })) } } diff --git a/codex-rs/guardian-context/src/registry_tests.rs b/codex-rs/guardian-context/src/registry_tests.rs index 2df72a6ea2..10f3094f50 100644 --- a/codex-rs/guardian-context/src/registry_tests.rs +++ b/codex-rs/guardian-context/src/registry_tests.rs @@ -242,22 +242,21 @@ fn reused_registry_preserves_section_identity_and_source_roles() { internal_chat_message_metadata_passthrough: None, }]; for target in [ContextTarget::Sync, ContextTarget::Async] { - let context = super::default_registry() - .collect(&SectionInput { - target, - history: &history, - transcript: &transcript, - root_conversation: &root, - trusted_user_answers: &answers, - planned_action: Some(&action), - permissions: Some(&permissions), - previous_reviews: Some(&reviews), - trusted_tool: Some(&tool), - trusted_skill_paths: &["debug-secret/SKILL.md".into()], - images: None, - node_repl: Some(&repl), - }) - .unwrap(); + let input = SectionInput { + target, + history: &history, + transcript: &transcript, + root_conversation: &root, + trusted_user_answers: &answers, + planned_action: Some(&action), + permissions: Some(&permissions), + previous_reviews: Some(&reviews), + trusted_tool: Some(&tool), + trusted_skill_paths: &["debug-secret/SKILL.md".into()], + images: None, + node_repl: Some(&repl), + }; + let context = super::default_registry().collect(&input).unwrap(); assert!(!format!("{context:?}").contains("debug-secret")); let mut expected = vec![ContextSection::RootConversation { items: vec![ @@ -288,12 +287,12 @@ fn reused_registry_preserves_section_identity_and_source_roles() { text_elements: Vec::new(), }], })); - expected.push(ContextSection::PermissionContext { items: vec![ + } + expected.push(ContextSection::PermissionContext { items: vec![ "\n>>> PARENT TURN PERMISSION CONTEXT START\n".into(), "The parent turn's active permission profile denies reading these paths/globs. These are policy restrictions; do not approve escalation whose purpose is to read them.\n- path `/private`\n- glob `**/*.key`\n".into(), ">>> PARENT TURN PERMISSION CONTEXT END\n".into(), ] }); - } if target == ContextTarget::Async { expected.insert( 0, @@ -306,6 +305,21 @@ fn reused_registry_preserves_section_identity_and_source_roles() { } expected.push(ContextSection::PlannedAction(action.clone())); assert_eq!(context, expected); + if target == ContextTarget::Async { + let oversized = super::PermissionContext { + denied_paths: vec!["x".repeat(/*n*/ 3_001)], + ..Default::default() + }; + assert_eq!( + super::default_registry().collect(&SectionInput { + permissions: Some(&oversized), + ..input + }), + Err(SectionError::EvidenceLimitExceeded { + section: "permissions" + }) + ); + } assert_eq!( super::default_registry() .collect(&SectionInput {