diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 48b8ecb107..4f1452c9d2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2929,6 +2929,7 @@ dependencies = [ "tracing-subscriber", "tracing-test", "url", + "uuid", "which 8.0.0", "wiremock", "zip", diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index ea571911de..931497086e 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -53,6 +53,7 @@ tokio = { workspace = true, features = ["fs", "macros", "rt", "time"] } toml = { workspace = true } tracing = { workspace = true } url = { workspace = true } +uuid = { workspace = true, features = ["v4"] } zip = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index ec7a9c365f..cedca7c075 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -16,6 +16,7 @@ pub mod marketplace_upgrade; mod npm_source; mod plugin_bundle_archive; mod plugin_metrics; +mod plugin_metrics_sidecar; mod provider; pub mod remote; pub mod remote_bundle; @@ -78,6 +79,10 @@ pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarket pub use plugin_metrics::PluginMeasurementDefinition; pub use plugin_metrics::PluginMetricsOperation; pub use plugin_metrics::ResolvedPluginMetricsOperation; +pub use plugin_metrics_sidecar::PLUGIN_METRICS_OUTPUT_ENV_VAR; +pub use plugin_metrics_sidecar::PluginMeasurementBatch; +pub use plugin_metrics_sidecar::PluginMetricsSidecar; +pub use plugin_metrics_sidecar::strip_output_env; pub use provider::ExecutorPluginProvider; pub use provider::ExecutorPluginProviderError; pub use provider::ResolvedExecutorPlugin; diff --git a/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs b/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs new file mode 100644 index 0000000000..3b0b4d43a1 --- /dev/null +++ b/codex-rs/core-plugins/src/plugin_metrics_sidecar.rs @@ -0,0 +1,171 @@ +use crate::ResolvedPluginMetricsOperation; +use codex_analytics::PluginMeasurementRow; +use codex_protocol::models::AdditionalPermissionProfile; +use codex_protocol::models::FileSystemPermissions; +use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::collections::HashMap; +use std::io::Read; +use std::io::Seek; +use std::io::SeekFrom; +use tempfile::NamedTempFile; +use uuid::Uuid; + +pub const PLUGIN_METRICS_OUTPUT_ENV_VAR: &str = "CODEX_PLUGIN_METRICS_OUTPUT"; +const MAX_OUTPUT_BYTES: u64 = 64 * 1024; +const MAX_OUTPUT_ROWS: usize = 100; + +#[derive(Debug, PartialEq)] +pub struct PluginMeasurementBatch { + pub plugin_id: String, + pub execution_id: String, + pub operation: String, + pub rows: Vec, +} + +pub struct PluginMetricsSidecar { + output_file: NamedTempFile, + _output_dir: tempfile::TempDir, + absolute_output_dir: AbsolutePathBuf, + output_env_value: String, + resolved: ResolvedPluginMetricsOperation, + execution_id: String, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct OutputEnvelope { + version: u32, + measurements: Vec, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct OutputMeasurement { + name: String, + value: f64, + #[serde(default)] + dimensions: BTreeMap, +} + +impl PluginMetricsSidecar { + pub fn create(resolved: ResolvedPluginMetricsOperation) -> Option { + let sidecar_dir = tempfile::Builder::new() + .prefix("codex-plugin-metrics-") + .tempdir() + .ok()?; + let output_file = tempfile::Builder::new() + .prefix("measurements-") + .suffix(".json") + .tempfile_in(sidecar_dir.path()) + .ok()?; + let absolute_output_dir = AbsolutePathBuf::from_absolute_path(sidecar_dir.path()).ok()?; + let absolute_output_path = AbsolutePathBuf::from_absolute_path(output_file.path()).ok()?; + let output_env_value = absolute_output_path.as_path().to_str()?.to_string(); + Some(Self { + output_file, + _output_dir: sidecar_dir, + absolute_output_dir, + output_env_value, + resolved, + execution_id: Uuid::new_v4().to_string(), + }) + } + + pub fn install_output_env(&self, env: &mut HashMap) { + env.insert( + PLUGIN_METRICS_OUTPUT_ENV_VAR.to_string(), + self.output_env_value.clone(), + ); + } + + #[cfg(test)] + fn absolute_output_path(&self) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(self.output_file.path()).expect("absolute output path") + } + + pub fn additional_permissions(&self) -> AdditionalPermissionProfile { + AdditionalPermissionProfile { + file_system: Some(FileSystemPermissions::from_read_write_roots( + /*read*/ None, + /*write*/ Some(vec![self.absolute_output_dir.clone()]), + )), + ..Default::default() + } + } + + pub fn finish(mut self, exit_code: i32) -> Option { + if exit_code != 0 { + return None; + } + let rows = parse_output(self.output_file.as_file_mut(), &self.resolved)?; + (!rows.is_empty()).then(|| PluginMeasurementBatch { + plugin_id: self.resolved.plugin_id.as_key(), + execution_id: self.execution_id, + operation: self.resolved.operation.operation_name, + rows, + }) + } +} + +pub fn strip_output_env(env: &mut HashMap) { + if cfg!(windows) { + env.retain(|key, _| !key.eq_ignore_ascii_case(PLUGIN_METRICS_OUTPUT_ENV_VAR)); + } else { + env.remove(PLUGIN_METRICS_OUTPUT_ENV_VAR); + } +} + +fn parse_output( + output_file: &mut std::fs::File, + resolved: &ResolvedPluginMetricsOperation, +) -> Option> { + let mut contents = Vec::new(); + output_file.seek(SeekFrom::Start(0)).ok()?; + output_file + .take(MAX_OUTPUT_BYTES + 1) + .read_to_end(&mut contents) + .ok()?; + if contents.len() as u64 > MAX_OUTPUT_BYTES { + return None; + } + let output: OutputEnvelope = serde_json::from_slice(&contents).ok()?; + if output.version != 1 || output.measurements.len() > MAX_OUTPUT_ROWS { + return None; + } + + let mut seen = BTreeSet::new(); + let mut rows = Vec::new(); + for value in output.measurements { + let Ok(measurement) = serde_json::from_value::(value) else { + continue; + }; + let Some(definition) = resolved.operation.measurements.get(&measurement.name) else { + continue; + }; + if !measurement.value.is_finite() + || measurement.dimensions.len() != definition.enum_dimensions.len() + || !definition.enum_dimensions.iter().all(|(name, allowed)| { + measurement + .dimensions + .get(name) + .is_some_and(|value| allowed.contains(value)) + }) + || !seen.insert((measurement.name.clone(), measurement.dimensions.clone())) + { + continue; + } + rows.push(PluginMeasurementRow { + measurement_name: measurement.name, + number_value: measurement.value, + dimensions: measurement.dimensions, + }); + } + Some(rows) +} + +#[cfg(test)] +#[path = "plugin_metrics_sidecar_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/plugin_metrics_sidecar_tests.rs b/codex-rs/core-plugins/src/plugin_metrics_sidecar_tests.rs new file mode 100644 index 0000000000..69b8199aa7 --- /dev/null +++ b/codex-rs/core-plugins/src/plugin_metrics_sidecar_tests.rs @@ -0,0 +1,192 @@ +use super::*; +use crate::PluginMeasurementDefinition; +use crate::PluginMetricsOperation; +use codex_plugin::PluginId; +use codex_protocol::models::LegacyReadWriteRoots; +use pretty_assertions::assert_eq; +use serde_json::json; + +fn create_sidecar() -> PluginMetricsSidecar { + PluginMetricsSidecar::create(resolved_operation()).expect("create sidecar") +} + +#[test] +fn sidecar_is_created_in_system_temp_with_private_permissions() { + let output_dir = + AbsolutePathBuf::from_absolute_path(std::env::temp_dir()).expect("absolute temp directory"); + let sidecar = PluginMetricsSidecar::create(resolved_operation()).expect("create sidecar"); + + assert_eq!( + sidecar.absolute_output_path().parent(), + Some(sidecar.absolute_output_dir.clone()) + ); + assert_eq!(sidecar.absolute_output_dir.parent(), Some(output_dir)); + assert!(sidecar.absolute_output_dir.as_path().is_dir()); + let roots = sidecar + .additional_permissions() + .file_system + .expect("file system permissions") + .legacy_read_write_roots() + .expect("legacy roots"); + assert_eq!( + roots, + LegacyReadWriteRoots { + read: None, + write: Some(vec![sidecar.absolute_output_dir]), + } + ); +} + +fn resolved_operation() -> ResolvedPluginMetricsOperation { + ResolvedPluginMetricsOperation { + plugin_id: PluginId::parse("security@openai-curated").expect("valid plugin id"), + operation: PluginMetricsOperation { + operation_name: "security_scan".to_string(), + measurements: BTreeMap::from([ + ( + "finding_count".to_string(), + PluginMeasurementDefinition { + enum_dimensions: BTreeMap::from([( + "severity".to_string(), + BTreeSet::from(["high".to_string(), "low".to_string()]), + )]), + }, + ), + ( + "files_scanned".to_string(), + PluginMeasurementDefinition { + enum_dimensions: BTreeMap::new(), + }, + ), + ]), + }, + } +} + +#[test] +fn sidecar_keeps_valid_rows_and_first_duplicate_then_cleans_up() { + let sidecar = create_sidecar(); + let path = sidecar.absolute_output_path(); + std::fs::write( + path.as_path(), + json!({ + "version": 1, + "measurements": [ + {"name": "finding_count", "value": 3, "dimensions": {"severity": "high"}}, + {"name": "unknown", "value": 1}, + {"name": "finding_count", "value": 4}, + {"name": "finding_count", "value": 5, "dimensions": {"severity": "critical"}}, + {"name": "finding_count", "value": 6, "dimensions": {"severity": "high", "extra": "x"}}, + {"name": "finding_count", "value": 99, "dimensions": {"severity": "high"}}, + {"name": "files_scanned", "value": 17}, + {"name": "files_scanned", "value": "not-a-number"}, + {"name": "files_scanned", "value": 18, "unknown": true} + ] + }) + .to_string(), + ) + .expect("write output"); + + let batch = sidecar.finish(/*exit_code*/ 0).expect("valid measurements"); + let execution_id = batch.execution_id.clone(); + assert_eq!( + batch, + PluginMeasurementBatch { + plugin_id: "security@openai-curated".to_string(), + execution_id: execution_id.clone(), + operation: "security_scan".to_string(), + rows: vec![ + PluginMeasurementRow { + measurement_name: "finding_count".to_string(), + number_value: 3.0, + dimensions: BTreeMap::from([("severity".to_string(), "high".to_string(),)]), + }, + PluginMeasurementRow { + measurement_name: "files_scanned".to_string(), + number_value: 17.0, + dimensions: BTreeMap::new(), + }, + ], + } + ); + assert_eq!( + Uuid::parse_str(&execution_id) + .expect("execution id UUID") + .get_version(), + Some(uuid::Version::Random) + ); + assert!(!path.exists()); +} + +#[test] +fn malformed_oversized_and_nonzero_outputs_are_ignored_and_cleaned_up() { + for output in [ + r#"{"version":2,"measurements":[]}"#.as_bytes().to_vec(), + r#"{"version":1,"measurements":[],"unknown":true}"#.as_bytes().to_vec(), + json!({ + "version": 1, + "measurements": vec![json!({"name": "files_scanned", "value": 1}); MAX_OUTPUT_ROWS + 1] + }) + .to_string() + .into_bytes(), + vec![b' '; MAX_OUTPUT_BYTES as usize + 1], + ] { + let sidecar = create_sidecar(); + let path = sidecar.absolute_output_path(); + std::fs::write(path.as_path(), output).expect("write output"); + assert_eq!(sidecar.finish(/*exit_code*/ 0), None); + assert!(!path.exists()); + } + + let sidecar = create_sidecar(); + let path = sidecar.absolute_output_path(); + std::fs::write( + path.as_path(), + r#"{"version":1,"measurements":[{"name":"files_scanned","value":1}]}"#, + ) + .expect("write output"); + assert_eq!(sidecar.finish(/*exit_code*/ 1), None); + assert!(!path.exists()); +} + +#[test] +fn reserved_output_env_is_absent_without_sidecar_and_cannot_be_overridden() { + let mut env = HashMap::from([ + ( + PLUGIN_METRICS_OUTPUT_ENV_VAR.to_string(), + "/user/path".to_string(), + ), + ("KEEP".to_string(), "value".to_string()), + ]); + strip_output_env(&mut env); + assert_eq!( + env, + HashMap::from([("KEEP".to_string(), "value".to_string())]) + ); + + let sidecar = create_sidecar(); + let path = sidecar.absolute_output_path(); + sidecar.install_output_env(&mut env); + assert_eq!( + env.get(PLUGIN_METRICS_OUTPUT_ENV_VAR).map(String::as_str), + path.as_path().to_str() + ); + drop(sidecar); + assert!(!path.exists()); +} + +#[cfg(unix)] +#[test] +fn sidecar_reads_the_original_file_after_path_replacement() { + let sidecar = create_sidecar(); + let path = sidecar.absolute_output_path(); + std::fs::remove_file(path.as_path()).expect("remove original output path"); + std::fs::write( + path.as_path(), + r#"{"version":1,"measurements":[{"name":"files_scanned","value":99}]}"#, + ) + .expect("write replacement output"); + + assert_eq!(sidecar.finish(/*exit_code*/ 0), None); + assert!(!path.exists()); +} diff --git a/codex-rs/core/src/plugins/metrics.rs b/codex-rs/core/src/plugins/metrics.rs new file mode 100644 index 0000000000..04d8432179 --- /dev/null +++ b/codex-rs/core/src/plugins/metrics.rs @@ -0,0 +1,32 @@ +use crate::session::session::Session; +use crate::session::turn_context::TurnContext; +use codex_analytics::PluginMeasurementsInput; +use codex_core_plugins::PluginMetricsSidecar; + +/// Finishes a metrics sidecar and publishes any valid rows. +pub(crate) fn finish_and_track_measurements( + metrics_sidecar: Option, + exit_code: i32, + session: &Session, + turn: &TurnContext, + item_id: &str, +) { + let Some(metrics_sidecar) = metrics_sidecar else { + return; + }; + let Some(batch) = metrics_sidecar.finish(exit_code) else { + return; + }; + session + .services + .analytics_events_client + .track_plugin_measurements(PluginMeasurementsInput { + thread_id: session.thread_id().to_string(), + turn_id: turn.sub_id.clone(), + item_id: item_id.to_string(), + plugin_id: batch.plugin_id, + execution_id: batch.execution_id, + operation: batch.operation, + rows: batch.rows, + }); +} diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index fe51b8472f..0a399b4525 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -1,6 +1,7 @@ mod discoverable; mod injection; mod mentions; +pub(crate) mod metrics; mod render; #[cfg(test)] #[path = "skill_snapshot_tests.rs"] diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index aca4785b2d..e1d282c28d 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -4,6 +4,7 @@ use crate::exec_policy::AllowPrefixRules; use crate::shell_snapshot::ShellSnapshotFile; use crate::tools::sandboxing::executor_windows_sandbox_level; use codex_core_plugins::PluginCommandAttribution; +use codex_core_plugins::ResolvedPluginMetricsOperation; use codex_core_plugins::TrustedPluginRoots; use codex_exec_server::ExecutorFileSystem; use codex_file_system::FileSystemSandboxContext; @@ -242,6 +243,16 @@ impl TurnContext { } } + pub(crate) fn plugin_metrics_operation_for_command( + &self, + command: &[String], + cwd: &AbsolutePathBuf, + ) -> Option { + self.extension_data + .get::()? + .resolve_metrics_operation(command, cwd) + } + pub(crate) fn permission_profile(&self) -> PermissionProfile { self.config.permissions.effective_permission_profile() } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 95a8c536c9..7fdd9a42b2 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -24,6 +24,7 @@ use crate::tools::runtimes::shell::ShellRequest; use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::runtimes::shell::ShellRuntimeBackend; use crate::tools::sandboxing::ToolCtx; +use codex_core_plugins::strip_output_env; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::ExecCommandSource; use codex_tools::ToolName; @@ -80,12 +81,15 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result Result process.env(PLUGIN_METRICS_OUTPUT_ENV_VAR, value), + None => process.env_remove(PLUGIN_METRICS_OUTPUT_ENV_VAR), + }; + let output = process.output().expect("run rewritten command"); + + assert!(output.status.success(), "command failed: {output:?}"); + assert_eq!(String::from_utf8_lossy(&output.stdout), expected); + } +} + #[test] fn maybe_wrap_shell_lc_with_snapshot_restores_proxy_env_from_process_env() { let dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index e66b0cdf16..00e0c3e503 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -10,6 +10,7 @@ pub(crate) mod zsh_fork_backend; use crate::exec::ExecCapturePolicy; use crate::guardian::GuardianNetworkAccessTrigger; +use crate::plugins::metrics::finish_and_track_measurements; use crate::sandboxing::ExecOptions; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; @@ -35,10 +36,12 @@ use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::managed_network_for_sandbox_permissions; use crate::tools::sandboxing::sandbox_permissions_preserving_denied_reads; +use codex_core_plugins::PluginMetricsSidecar; use codex_network_proxy::NetworkProxy; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::models::AdditionalPermissionProfile; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_shell_command::powershell::prefix_powershell_script_with_utf8; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -207,8 +210,20 @@ impl ToolRuntime for ShellRuntime { ); let managed_network = managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions); - let env = exec_env_for_sandbox_permissions(&req.env, sandbox_permissions); + let mut env = exec_env_for_sandbox_permissions(&req.env, sandbox_permissions); let explicit_env_overrides = req.explicit_env_overrides.clone(); + let metrics_sidecar = (!req.turn_environment.environment.is_remote() + && ctx.session.services.analytics_events_client.is_enabled()) + .then(|| { + ctx.step_context + .turn + .plugin_metrics_operation_for_command(&req.command, &req.cwd) + }) + .flatten() + .and_then(PluginMetricsSidecar::create); + if let Some(sidecar) = metrics_sidecar.as_ref() { + sidecar.install_output_env(&mut env); + } #[cfg(unix)] let (env, runtime_path_prepends) = { let mut env = env; @@ -246,39 +261,66 @@ impl ToolRuntime for ShellRuntime { command }; - if self.backend == ShellRuntimeBackend::ShellCommandZshFork { - match zsh_fork_backend::maybe_run_shell_command(req, attempt, ctx, &command).await? { - Some(out) => return Ok(out), + let zsh_fork_output = if self.backend == ShellRuntimeBackend::ShellCommandZshFork { + match zsh_fork_backend::maybe_run_shell_command( + req, + attempt, + ctx, + &command, + metrics_sidecar.as_ref(), + ) + .await? + { + Some(out) => Some(out), None => { tracing::warn!( "ZshFork backend specified, but conditions for using it were not met, falling back to normal execution", ); + None } } - } - - let command = - build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone())?; - let mut expiration: crate::exec::ExecExpiration = req.timeout_ms.into(); - expiration = expiration.with_cancellation(req.cancellation_token.clone()); - if let Some(cancellation) = attempt.network_denial_cancellation_token.clone() { - expiration = expiration.with_cancellation(cancellation); - } - let options = ExecOptions { - expiration, - capture_policy: ExecCapturePolicy::ShellTool, + } else { + None }; - let env = attempt - .env_for( - command, - options, - managed_network, - Some(&req.turn_environment.environment_id), - ) - .map_err(ToolError::Codex)?; - let out = execute_env(env, Self::stdout_stream(ctx)) - .await - .map_err(ToolError::Codex)?; + let out = if let Some(out) = zsh_fork_output { + out + } else { + let sidecar_permissions = metrics_sidecar + .as_ref() + .map(PluginMetricsSidecar::additional_permissions); + let additional_permissions = merge_permission_profiles( + req.additional_permissions.as_ref(), + sidecar_permissions.as_ref(), + ); + let command = build_sandbox_command(&command, &req.cwd, &env, additional_permissions)?; + let mut expiration: crate::exec::ExecExpiration = req.timeout_ms.into(); + expiration = expiration.with_cancellation(req.cancellation_token.clone()); + if let Some(cancellation) = attempt.network_denial_cancellation_token.clone() { + expiration = expiration.with_cancellation(cancellation); + } + let options = ExecOptions { + expiration, + capture_policy: ExecCapturePolicy::ShellTool, + }; + let env = attempt + .env_for( + command, + options, + managed_network, + Some(&req.turn_environment.environment_id), + ) + .map_err(ToolError::Codex)?; + execute_env(env, Self::stdout_stream(ctx)) + .await + .map_err(ToolError::Codex)? + }; + finish_and_track_measurements( + metrics_sidecar, + out.exit_code, + &ctx.session, + &ctx.step_context.turn, + &ctx.call_id, + ); Ok(out) } } diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 21047ea521..17dcca68e5 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -19,6 +19,7 @@ use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::managed_network_for_sandbox_permissions; use crate::tools::sandboxing::sandbox_permissions_preserving_denied_reads; use crate::tools::sandboxing::unsandboxed_execution_allowed; +use codex_core_plugins::PluginMetricsSidecar; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::MatchOptions; @@ -41,6 +42,7 @@ use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_sandboxing::record_filesystem_sandbox_violation; use codex_shell_command::bash::parse_shell_lc_plain_commands; use codex_shell_command::bash::parse_shell_lc_single_command_prefix; @@ -103,6 +105,7 @@ pub(super) async fn try_run_zsh_fork( attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + metrics_sidecar: Option<&PluginMetricsSidecar>, ) -> Result, ToolError> { let Some(shell_zsh_path) = ctx.session.services.shell_zsh_path.as_ref() else { tracing::warn!("ZshFork backend specified, but shell_zsh_path is not configured."); @@ -127,9 +130,16 @@ pub(super) async fn try_run_zsh_fork( ..req.clone() }; let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); + if let Some(sidecar) = metrics_sidecar { + sidecar.install_output_env(&mut env); + } prepend_zsh_fork_bin_to_path(&mut env, shell_zsh_path); - let command = - build_sandbox_command(command, &req.cwd, &env, req.additional_permissions.clone())?; + let sidecar_permissions = metrics_sidecar.map(PluginMetricsSidecar::additional_permissions); + let additional_permissions = merge_permission_profiles( + req.additional_permissions.as_ref(), + sidecar_permissions.as_ref(), + ); + let command = build_sandbox_command(command, &req.cwd, &env, additional_permissions)?; let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, diff --git a/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs b/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs index 819658ecca..9d65924a89 100644 --- a/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs +++ b/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs @@ -5,6 +5,7 @@ use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::unified_exec::SpawnLifecycleHandle; +use codex_core_plugins::PluginMetricsSidecar; use codex_protocol::exec_output::ExecToolCallOutput; use codex_tools::ZshForkConfig; @@ -23,8 +24,9 @@ pub(crate) async fn maybe_run_shell_command( attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + metrics_sidecar: Option<&PluginMetricsSidecar>, ) -> Result, ToolError> { - imp::maybe_run_shell_command(req, attempt, ctx, command).await + imp::maybe_run_shell_command(req, attempt, ctx, command, metrics_sidecar).await } /// Prepares unified exec to launch through the zsh-fork backend when the @@ -76,8 +78,9 @@ mod imp { attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + metrics_sidecar: Option<&PluginMetricsSidecar>, ) -> Result, ToolError> { - unix_escalation::try_run_zsh_fork(req, attempt, ctx, command).await + unix_escalation::try_run_zsh_fork(req, attempt, ctx, command, metrics_sidecar).await } pub(super) async fn maybe_prepare_unified_exec( @@ -118,8 +121,9 @@ mod imp { attempt: &SandboxAttempt<'_>, ctx: &ToolCtx, command: &[String], + metrics_sidecar: Option<&PluginMetricsSidecar>, ) -> Result, ToolError> { - let _ = (req, attempt, ctx, command); + let _ = (req, attempt, ctx, command, metrics_sidecar); Ok(None) } diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index af940e3ef0..d9cad14ecf 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -49,6 +49,8 @@ use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; use core_test_support::wait_for_mcp_server; +use core_test_support::zsh_fork::zsh_fork_runtime; +use core_test_support::zsh_fork::zsh_fork_test_builder; use pretty_assertions::assert_eq; use tempfile::TempDir; use test_case::test_case; @@ -340,8 +342,12 @@ fn searched_plugin_tools( ) } +#[test_case(false; "classic shell")] +#[test_case(true; "zsh-fork shell")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn persisted_remote_plugin_command_attribution_flows_through_turn_context() -> Result<()> { +async fn persisted_remote_plugin_command_attribution_flows_through_turn_context( + zsh_fork: bool, +) -> Result<()> { skip_if_target_windows!(Ok(()), "executes a POSIX shell script"); skip_if_no_network!(Ok(())); skip_if_remote!( @@ -352,8 +358,28 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context( let server = start_mock_server().await; let codex_home = Arc::new(TempDir::new()?); let script_path = write_remote_plugin_script_and_config(codex_home.as_ref()); - let script_path = script_path.to_string_lossy(); - let command = shlex::try_join(["/bin/sh", script_path.as_ref()])?; + std::fs::write( + &script_path, + r#"printf '%s' '{"version":1,"measurements":[{"name":"files_scanned","value":7}]}' > "$CODEX_PLUGIN_METRICS_OUTPUT" +"#, + )?; + let plugin_root = script_path + .parent() + .and_then(std::path::Path::parent) + .expect("plugin root"); + std::fs::write( + plugin_root.join("analytics.yaml"), + "version: 1\noperations: {scan: {path: ./scripts/run.sh, measurements: {files_scanned: {}}}}\n", + )?; + let builder = if zsh_fork { + let Some(runtime) = zsh_fork_runtime("zsh-fork plugin measurement test")? else { + return Ok(()); + }; + zsh_fork_test_builder(runtime, AskForApproval::Never) + } else { + test_codex() + }; + let command = shlex::try_join(["/bin/sh", script_path.to_string_lossy().as_ref()])?; let call_id = "remote-plugin-command"; let arguments = serde_json::to_string(&serde_json::json!({ "command": command, @@ -376,16 +402,18 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context( ) .await; - let mut builder = test_codex() + let chatgpt_base_url = server.uri(); + let mut builder = builder .with_home(Arc::clone(&codex_home)) .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_model("gpt-5.2"); + .with_model("gpt-5.2") + .with_config(move |config| config.chatgpt_base_url = chatgpt_base_url); let test_codex = builder.build_with_auto_env(&server).await?; let codex = Arc::clone(&test_codex.codex); let cwd = test_codex.config.cwd.clone(); let session_model = test_codex.session_configured.model.clone(); let (sandbox_policy, permission_profile) = - turn_permission_fields(PermissionProfile::Disabled, cwd.as_path()); + turn_permission_fields(PermissionProfile::read_only(), cwd.as_path()); codex .submit(Op::UserInput { items: vec![codex_protocol::user_input::UserInput::Text { @@ -423,6 +451,11 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context( _ => None, }) .await; + assert_eq!( + end.exit_code, 0, + "sandboxed plugin command failed: {}", + end.aggregated_output + ); wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; for (plugin_id, script_path) in [ @@ -433,6 +466,22 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context( assert_eq!(script_path, Some("scripts/run.sh")); } + let measurement = wait_for_analytics_event(&server, "codex_plugin_measurement_event").await; + assert_eq!( + serde_json::json!({ + "plugin_id": measurement["event_params"]["plugin_id"], + "operation": measurement["event_params"]["operation"], + "measurement_name": measurement["event_params"]["measurement_name"], + "number_value": measurement["event_params"]["number_value"], + }), + serde_json::json!({ + "plugin_id": REMOTE_PLUGIN_CONFIG_NAME, + "operation": "scan", + "measurement_name": "files_scanned", + "number_value": 7.0, + }) + ); + Ok(()) }