diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index fdaeaf51a1..48b8ecb107 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2917,6 +2917,7 @@ dependencies = [ "semver", "serde", "serde_json", + "serde_with", "serde_yaml", "sha2 0.10.9", "tar", diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 56cfd12a92..ea571911de 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -43,6 +43,7 @@ regex = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +serde_with = { workspace = true } serde_yaml = { workspace = true } sha2 = { workspace = true } tar = { workspace = true } diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 9681bb27a9..ec7a9c365f 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -15,6 +15,7 @@ pub mod marketplace_remove; pub mod marketplace_upgrade; mod npm_source; mod plugin_bundle_archive; +mod plugin_metrics; mod provider; pub mod remote; pub mod remote_bundle; @@ -74,6 +75,9 @@ pub use manager::RecommendedPluginCandidatesInput; pub use marketplace_policy::allowed_configured_marketplace_names; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome; +pub use plugin_metrics::PluginMeasurementDefinition; +pub use plugin_metrics::PluginMetricsOperation; +pub use plugin_metrics::ResolvedPluginMetricsOperation; pub use provider::ExecutorPluginProvider; pub use provider::ExecutorPluginProviderError; pub use provider::ResolvedExecutorPlugin; diff --git a/codex-rs/core-plugins/src/plugin_metrics.rs b/codex-rs/core-plugins/src/plugin_metrics.rs new file mode 100644 index 0000000000..b7df811968 --- /dev/null +++ b/codex-rs/core-plugins/src/plugin_metrics.rs @@ -0,0 +1,158 @@ +use crate::script_attribution::normalized_relative_script_path; +use codex_plugin::PluginId; +use codex_protocol::items::is_safe_plugin_relative_path; +use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use std::collections::BTreeMap; +use std::collections::BTreeSet; +use std::fs::File; +use std::io::Read; + +const ANALYTICS_MANIFEST_FILE: &str = "analytics.yaml"; +const MAX_ANALYTICS_MANIFEST_BYTES: u64 = 64 * 1024; +const MAX_IDENTIFIER_LEN: usize = 64; +const MAX_DIMENSIONS_PER_MEASUREMENT: usize = 8; + +/// The manifest declaration for one numeric measurement. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct PluginMeasurementDefinition { + pub enum_dimensions: BTreeMap>, +} + +/// Custom metrics allowed for one trusted plugin script operation. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct PluginMetricsOperation { + pub operation_name: String, + pub measurements: BTreeMap, +} + +/// A metrics operation bound to identity from a fresh trusted command lookup. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ResolvedPluginMetricsOperation { + pub plugin_id: PluginId, + pub operation: PluginMetricsOperation, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct AnalyticsManifest { + version: u32, + #[serde(with = "serde_with::rust::maps_duplicate_key_is_error")] + operations: BTreeMap, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct OperationDeclaration { + path: String, + #[serde(with = "serde_with::rust::maps_duplicate_key_is_error")] + measurements: BTreeMap, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct MeasurementDeclaration { + #[serde(default, with = "serde_with::rust::maps_duplicate_key_is_error")] + dimensions: BTreeMap>, +} + +pub(crate) fn load_plugin_metrics_operations( + plugin_root: &AbsolutePathBuf, +) -> Option> { + let manifest_path = plugin_root.join(ANALYTICS_MANIFEST_FILE); + let canonical_manifest_path = manifest_path.canonicalize().ok()?; + if canonical_manifest_path != manifest_path || !manifest_path.as_path().is_file() { + return None; + } + + let mut contents = Vec::new(); + File::open(manifest_path.as_path()) + .ok()? + .take(MAX_ANALYTICS_MANIFEST_BYTES + 1) + .read_to_end(&mut contents) + .ok()?; + if contents.len() as u64 > MAX_ANALYTICS_MANIFEST_BYTES { + return None; + } + let manifest: AnalyticsManifest = serde_yaml::from_slice(&contents).ok()?; + validate_manifest(manifest, plugin_root) +} + +fn validate_manifest( + manifest: AnalyticsManifest, + plugin_root: &AbsolutePathBuf, +) -> Option> { + if manifest.version != 1 || manifest.operations.is_empty() { + return None; + } + + let mut operations_by_path = BTreeMap::new(); + for (operation_name, operation) in manifest.operations { + if !valid_identifier(&operation_name) || operation.measurements.is_empty() { + return None; + } + let normalized_path = validated_operation_path(plugin_root, &operation.path)?; + let mut measurements = BTreeMap::new(); + for (measurement_name, measurement) in operation.measurements { + if !valid_identifier(&measurement_name) + || measurement.dimensions.len() > MAX_DIMENSIONS_PER_MEASUREMENT + { + return None; + } + let mut enum_dimensions = BTreeMap::new(); + for (dimension_name, values) in measurement.dimensions { + if !valid_identifier(&dimension_name) || values.is_empty() { + return None; + } + let value_count = values.len(); + let values = values.into_iter().collect::>(); + if values.len() != value_count + || values.iter().any(|value| !valid_identifier(value)) + { + return None; + } + enum_dimensions.insert(dimension_name, values); + } + measurements.insert( + measurement_name, + PluginMeasurementDefinition { enum_dimensions }, + ); + } + if operations_by_path + .insert( + normalized_path, + PluginMetricsOperation { + operation_name, + measurements, + }, + ) + .is_some() + { + return None; + } + } + Some(operations_by_path) +} + +fn validated_operation_path(plugin_root: &AbsolutePathBuf, path: &str) -> Option { + let normalized_path = path.strip_prefix("./").unwrap_or(path); + if !is_safe_plugin_relative_path(normalized_path) { + return None; + } + let script = plugin_root.join(normalized_path).canonicalize().ok()?; + if !script.as_path().is_file() { + return None; + } + let canonical_relative_path = script.as_path().strip_prefix(plugin_root.as_path()).ok()?; + (normalized_relative_script_path(canonical_relative_path)?.as_str() == normalized_path) + .then(|| normalized_path.to_string()) +} + +fn valid_identifier(value: &str) -> bool { + let mut chars = value.chars(); + matches!(chars.next(), Some('a'..='z')) + && value.len() <= MAX_IDENTIFIER_LEN + && chars.all(|character| { + character.is_ascii_lowercase() || character.is_ascii_digit() || character == '_' + }) +} diff --git a/codex-rs/core-plugins/src/script_attribution.rs b/codex-rs/core-plugins/src/script_attribution.rs index 1b8024272d..509af95db1 100644 --- a/codex-rs/core-plugins/src/script_attribution.rs +++ b/codex-rs/core-plugins/src/script_attribution.rs @@ -6,6 +6,9 @@ use crate::loader::curated_plugin_cache_version; use crate::marketplace::MarketplacePluginSource; use crate::marketplace::find_marketplace_plugin; use crate::marketplace_policy::primary_runtime_marketplace_root; +use crate::plugin_metrics::PluginMetricsOperation; +use crate::plugin_metrics::ResolvedPluginMetricsOperation; +use crate::plugin_metrics::load_plugin_metrics_operations; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::startup_sync::curated_plugins_api_marketplace_path; use crate::startup_sync::curated_plugins_repo_path; @@ -21,6 +24,7 @@ use codex_shell_command::bash::parse_shell_lc_plain_commands; use codex_shell_command::parse_command::is_pathish; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; +use std::collections::BTreeMap; use std::collections::HashSet; use std::path::Component; use std::path::Path; @@ -29,6 +33,7 @@ use std::path::Path; struct TrustedPluginRoot { plugin_id: PluginId, root: AbsolutePathBuf, + metrics_operations_by_path: BTreeMap, } /// Trusted plugin command attribution safe to carry into command analytics. @@ -79,9 +84,12 @@ impl TrustedPluginRoots { return None; } let root = expected_root.canonicalize().ok()?; - root.as_path() - .is_dir() - .then_some(TrustedPluginRoot { plugin_id, root }) + root.as_path().is_dir().then(|| TrustedPluginRoot { + plugin_id, + metrics_operations_by_path: load_plugin_metrics_operations(&root) + .unwrap_or_default(), + root, + }) }) .filter(|root| seen.insert((root.plugin_id.as_key(), root.root.clone()))) .collect(); @@ -193,6 +201,31 @@ impl TrustedPluginRoots { matches.next().is_none().then_some(attribution) } + /// Resolves one exact command to one trusted manifest-declared operation. + pub fn resolve_metrics_operation( + &self, + command: &[String], + cwd: &AbsolutePathBuf, + ) -> Option { + let attribution = self.resolve_attribution(command, cwd)?; + let mut matches = self.roots.iter().filter_map(|root| { + (root.plugin_id == attribution.plugin_id) + .then(|| { + root.metrics_operations_by_path + .get(&attribution.normalized_relative_path) + }) + .flatten() + }); + let operation = matches.next()?.clone(); + matches + .next() + .is_none() + .then_some(ResolvedPluginMetricsOperation { + plugin_id: attribution.plugin_id, + operation, + }) + } + /// Resolves a trusted script on the selected executor filesystem. /// /// Remote commands can use a path convention that the app-server host cannot @@ -306,7 +339,7 @@ pub fn command_script_arguments(command: &[String]) -> Option> { /// Converts a path already proven to be below a trusted plugin root into the /// only path shape that may leave the resolver: non-empty, relative, and /// slash-separated with no traversal or platform-specific prefixes. -fn normalized_relative_script_path(relative_path: &Path) -> Option { +pub(crate) fn normalized_relative_script_path(relative_path: &Path) -> Option { let normalized = relative_path .components() .map(|component| { diff --git a/codex-rs/core-plugins/src/script_attribution_tests.rs b/codex-rs/core-plugins/src/script_attribution_tests.rs index 8616af8f50..36f33d3acc 100644 --- a/codex-rs/core-plugins/src/script_attribution_tests.rs +++ b/codex-rs/core-plugins/src/script_attribution_tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::LoadedPlugin; +use crate::PluginMeasurementDefinition; use crate::loader::curated_plugin_cache_version; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::startup_sync::curated_plugins_repo_path; @@ -13,6 +14,8 @@ use codex_plugin::PluginLoadOutcome; use codex_utils_path_uri::PathUri; use codex_utils_plugins::SkillDiscoveryMode; use pretty_assertions::assert_eq; +use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::HashSet; use std::fs; @@ -148,6 +151,7 @@ fn resolves_primary_runtime_scripts_from_the_installed_plugin_cache() { let roots = TrustedPluginRoots { roots: vec![TrustedPluginRoot { plugin_id: plugin_id.clone(), + metrics_operations_by_path: BTreeMap::new(), root: plugin_root.canonicalize().expect("canonical plugin root"), }], }; @@ -240,6 +244,24 @@ fn recognizes_windows_executor_plugin_cache_root() { assert!(executor_plugin_root_matches(&script, &attribution)); } +fn assert_invalid_metrics_manifest(codex_home: &Path, root: &AbsolutePathBuf, manifest: &str) { + fs::write(root.join("analytics.yaml"), manifest).expect("write analytics manifest"); + let roots = roots_for( + codex_home, + vec![loaded_plugin( + "sample@openai-curated", + root.as_path(), + ENABLED, + )], + ); + roots + .resolve_attribution(&command(&["scripts/run.py"]), root) + .expect("Part 1 attribution remains enabled"); + assert_eq!( + roots.resolve_metrics_operation(&command(&["scripts/run.py"]), root), + None + ); +} fn assert_untrusted(codex_home: &Path, config_name: &str, root: &Path) { assert!( roots_for(codex_home, vec![loaded_plugin(config_name, root, ENABLED)]) @@ -299,15 +321,18 @@ fn trusted_roots_require_verified_curated_or_remote_cache() { vec![ TrustedPluginRoot { plugin_id: PluginId::parse("sample@openai-curated").expect("plugin id"), + metrics_operations_by_path: BTreeMap::new(), root: root.canonicalize().expect("canonical root"), }, TrustedPluginRoot { plugin_id: PluginId::parse("api-sample@openai-api-curated").expect("plugin id"), + metrics_operations_by_path: BTreeMap::new(), root: api_root.canonicalize().expect("canonical root"), }, TrustedPluginRoot { plugin_id: PluginId::parse("remote-sample@openai-curated-remote") .expect("plugin id"), + metrics_operations_by_path: BTreeMap::new(), root: remote_root.canonicalize().expect("canonical root"), }, ] @@ -353,6 +378,184 @@ fn trusted_roots_require_verified_curated_or_remote_cache() { ); } +#[test] +fn resolves_manifest_operation_for_exact_attributed_script() { + let (temp, root, _) = script_fixture(); + fs::write( + root.join("analytics.yaml"), + r#"version: 1 +operations: + security_scan: + path: ./scripts/run.py + measurements: + repository_files: {} + findings: + dimensions: + severity: [critical, high, medium, low] +"#, + ) + .expect("write analytics manifest"); + let roots = roots_for( + temp.path(), + vec![loaded_plugin( + "sample@openai-curated", + root.as_path(), + ENABLED, + )], + ); + assert_eq!( + roots.resolve_metrics_operation(&command(&["scripts/run.py"]), &root), + Some(ResolvedPluginMetricsOperation { + plugin_id: PluginId::parse("sample@openai-curated").expect("plugin id"), + operation: PluginMetricsOperation { + operation_name: "security_scan".to_string(), + measurements: BTreeMap::from([ + ( + "findings".to_string(), + PluginMeasurementDefinition { + enum_dimensions: BTreeMap::from([( + "severity".to_string(), + BTreeSet::from([ + "critical".to_string(), + "high".to_string(), + "low".to_string(), + "medium".to_string(), + ]), + )]), + }, + ), + ( + "repository_files".to_string(), + PluginMeasurementDefinition { + enum_dimensions: BTreeMap::new(), + }, + ), + ]), + }, + }) + ); + + let undeclared_script = root.join("scripts/undeclared.py"); + fs::write(undeclared_script.as_path(), "print('ok')\n").expect("write undeclared script"); + roots + .resolve_attribution(&command(&["scripts/undeclared.py"]), &root) + .expect("trusted attribution"); + assert_eq!( + roots.resolve_metrics_operation(&command(&["scripts/undeclared.py"]), &root), + None + ); +} + +#[test] +fn allows_measurement_names_reused_across_operations() { + let (temp, root, _) = script_fixture(); + let other_script = root.join("scripts/other.py"); + fs::write(other_script.as_path(), "#!/usr/bin/env python3\n").expect("write script"); + fs::write( + root.join("analytics.yaml"), + r#"version: 1 +operations: + first: + path: ./scripts/run.py + measurements: + count: {} + second: + path: ./scripts/other.py + measurements: + count: {} +"#, + ) + .expect("write analytics manifest"); + let roots = roots_for( + temp.path(), + vec![loaded_plugin( + "sample@openai-curated", + root.as_path(), + ENABLED, + )], + ); + + for (script, operation_name) in [("scripts/run.py", "first"), ("scripts/other.py", "second")] { + let resolved = roots + .resolve_metrics_operation(&command(&[script]), &root) + .expect("resolved metrics operation"); + assert_eq!(resolved.operation.operation_name, operation_name); + assert!(resolved.operation.measurements.contains_key("count")); + } +} + +#[test] +fn invalid_manifest_disables_metrics_without_disabling_attribution() { + let (temp, root, _) = script_fixture(); + let invalid_manifests = [ + r#"version: 2 +operations: {scan: {path: scripts/run.py, measurements: {count: {}}}} +"#, + r#"version: 1 +unknown: true +operations: {scan: {path: scripts/run.py, measurements: {count: {}}}} +"#, + r#"version: 1 +operations: + scan: {path: scripts/run.py, measurements: {count: {}}} + scan: {path: scripts/run.py, measurements: {count: {}}} +"#, + r#"version: 1 +operations: + scan: + path: scripts/run.py + measurements: + count: {} + count: {} +"#, + r#"version: 1 +operations: + scan: + path: ../outside.py + measurements: + count: {} +"#, + r#"version: 1 +operations: {scan: {path: scripts/run.py, measurements: {count: {dimensions: {status: ["needs review"]}}}}} +"#, + r#"version: 1 +operations: {BadName: {path: scripts/run.py, measurements: {count: {}}}} +"#, + r#"version: 1 +operations: {scan: {path: scripts/run.py, measurements: {count: {}}}, scan_again: {path: ./scripts/run.py, measurements: {count: {}}}} +"#, + ]; + + for manifest in invalid_manifests { + assert_invalid_metrics_manifest(temp.path(), &root, manifest); + } + + let oversized_manifest = format!( + "version: 1\noperations: {{scan: {{path: scripts/run.py, measurements: {{count: {{}}}}}}}}\n#{}", + "x".repeat(64 * 1024) + ); + assert_invalid_metrics_manifest(temp.path(), &root, &oversized_manifest); + + #[cfg(unix)] + { + let outside = temp.path().join("outside.py"); + fs::write(&outside, "print('outside')\n").expect("write outside script"); + std::os::unix::fs::symlink(&outside, root.join("scripts/escape.py")) + .expect("symlink script"); + assert_invalid_metrics_manifest( + temp.path(), + &root, + r#"version: 1 +operations: + scan: + path: scripts/escape.py + measurements: + count: {} +"#, + ); + } +} + #[test] fn resolves_local_attribution_for_safe_interpreters_and_wrappers() { let (temp, root, script) = script_fixture(); @@ -442,10 +645,12 @@ fn rejects_ambiguous_commands_overlaps_and_symlink_escapes() { roots: vec![ TrustedPluginRoot { plugin_id: PluginId::parse("sample@openai-curated").expect("plugin id"), + metrics_operations_by_path: BTreeMap::new(), root: root.canonicalize().expect("canonical root"), }, TrustedPluginRoot { plugin_id: PluginId::parse("nested@openai-curated").expect("plugin id"), + metrics_operations_by_path: BTreeMap::new(), root: root.join("scripts").canonicalize().expect("nested root"), }, ],