diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 1e79865d43..ac473f17a7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2684,6 +2684,7 @@ dependencies = [ "semver", "serde", "serde_json", + "sha2 0.10.9", "tar", "tempfile", "thiserror 2.0.18", @@ -2691,6 +2692,7 @@ dependencies = [ "toml 0.9.11+spec-1.1.0", "tracing", "url", + "walkdir", "wiremock", "zip 2.4.2", ] diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index 7177952c37..fd61795bf0 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -2975,6 +2975,15 @@ fn hook_run_metadata_maps_sources_and_statuses() { }, )) .expect("serialize cloud requirements hook"); + let app_bundled_internal = serde_json::to_value(codex_hook_run_metadata( + &tracking, + HookRunFact { + event_name: HookEventName::Stop, + hook_source: HookSource::AppBundledInternal, + status: HookRunStatus::Completed, + }, + )) + .expect("serialize app-bundled internal hook"); let unknown = serde_json::to_value(codex_hook_run_metadata( &tracking, HookRunFact { @@ -2991,6 +3000,8 @@ fn hook_run_metadata_maps_sources_and_statuses() { assert_eq!(project["status"], "blocked"); assert_eq!(cloud_requirements["hook_source"], "cloud_requirements"); assert_eq!(cloud_requirements["status"], "blocked"); + assert_eq!(app_bundled_internal["hook_source"], "app_bundled_internal"); + assert_eq!(app_bundled_internal["status"], "completed"); assert_eq!(unknown["hook_source"], "unknown"); assert_eq!(unknown["status"], "failed"); } diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index bdc2c996dc..dbb0e2123b 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -1027,6 +1027,7 @@ fn analytics_hook_source(source: HookSource) -> &'static str { HookSource::CloudManagedConfig => "cloud_managed_config", HookSource::LegacyManagedConfigFile => "legacy_managed_config_file", HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm", + HookSource::AppBundledInternal => "app_bundled_internal", HookSource::Unknown => "unknown", } } diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index c75915629c..a6157f5630 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -25,6 +25,7 @@ use std::io::Result as IoResult; use std::sync::Arc; use std::time::Duration; +pub use codex_app_server::AppBundledInternalPlugin; pub use codex_app_server::app_server_control_socket_path; pub use codex_app_server::in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; pub use codex_app_server::in_process::InProcessServerEvent; @@ -365,6 +366,8 @@ pub struct InProcessClientStartArgs { pub opt_out_notification_methods: Vec, /// Queue capacity for command/event channels (clamped to at least 1). pub channel_capacity: usize, + /// Host-owned app-packaged plugin roots allowed to authorize internal hooks. + pub app_bundled_internal_plugins: Vec, } fn configured_thread_config_loader(config: &Config) -> Arc { @@ -417,6 +420,7 @@ impl InProcessClientStartArgs { enable_codex_api_key_env: self.enable_codex_api_key_env, initialize, channel_capacity: self.channel_capacity, + app_bundled_internal_plugins: self.app_bundled_internal_plugins, } } } @@ -1048,6 +1052,7 @@ mod tests { experimental_api: true, opt_out_notification_methods: Vec::new(), channel_capacity, + app_bundled_internal_plugins: Vec::new(), }) .await .expect("in-process app-server client should start"); @@ -2212,6 +2217,7 @@ mod tests { experimental_api: true, opt_out_notification_methods: Vec::new(), channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), } .into_runtime_start_args(); @@ -2253,6 +2259,7 @@ mod tests { experimental_api: true, opt_out_notification_methods: Vec::new(), channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), } .into_runtime_start_args(); diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 16656e3a55..3327e5b348 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2006,6 +2006,7 @@ "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", + "appBundledInternal", "unknown" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 4802a52300..632e55013f 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -10202,6 +10202,7 @@ "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", + "appBundledInternal", "unknown" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 2b9e30bae7..183f6035c7 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6675,6 +6675,7 @@ "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", + "appBundledInternal", "unknown" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json index 8684bf9ae5..eba699b61d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json @@ -169,6 +169,7 @@ "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", + "appBundledInternal", "unknown" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json index 5b2750d780..23d133dd4f 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json @@ -169,6 +169,7 @@ "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", + "appBundledInternal", "unknown" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json index c3288ee0fd..0d7cdfc13d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json @@ -133,6 +133,7 @@ "cloudManagedConfig", "legacyManagedConfigFile", "legacyManagedConfigMdm", + "appBundledInternal", "unknown" ], "type": "string" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts index 5c3cb2eabf..b8c0e18d51 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookSource.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type HookSource = "system" | "user" | "project" | "mdm" | "sessionFlags" | "plugin" | "cloudRequirements" | "cloudManagedConfig" | "legacyManagedConfigFile" | "legacyManagedConfigMdm" | "unknown"; +export type HookSource = "system" | "user" | "project" | "mdm" | "sessionFlags" | "plugin" | "cloudRequirements" | "cloudManagedConfig" | "legacyManagedConfigFile" | "legacyManagedConfigMdm" | "appBundledInternal" | "unknown"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/hook.rs b/codex-rs/app-server-protocol/src/protocol/v2/hook.rs index b8f47276a6..dc57621188 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/hook.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/hook.rs @@ -51,6 +51,7 @@ v2_enum_from_core!( CloudManagedConfig, LegacyManagedConfigFile, LegacyManagedConfigMdm, + AppBundledInternal, Unknown, } ); diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 448a389851..1707fce270 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -82,6 +82,7 @@ use codex_config::LoaderOverrides; use codex_config::ThreadConfigLoader; use codex_core::config::Config; use codex_core::resolve_installation_id; +use codex_core_plugins::AppBundledInternalPlugin; use codex_exec_server::EnvironmentManager; use codex_feedback::CodexFeedback; use codex_login::AuthManager; @@ -146,6 +147,8 @@ pub struct InProcessStartArgs { pub initialize: InitializeParams, /// Capacity used for all runtime queues (clamped to at least 1). pub channel_capacity: usize, + /// Host-owned app-packaged plugin roots allowed to authorize internal hooks. + pub app_bundled_internal_plugins: Vec, } /// Event emitted from the app-server to the in-process client. @@ -438,6 +441,7 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult, } impl Default for AppServerRuntimeOptions { @@ -412,6 +414,7 @@ impl Default for AppServerRuntimeOptions { plugin_startup_tasks: PluginStartupTasks::Start, remote_control_enabled: false, install_shutdown_signal_handler: true, + app_bundled_internal_plugins: Vec::new(), } } } @@ -815,6 +818,7 @@ pub async fn run_main_with_transport_options( rpc_transport: analytics_rpc_transport(&transport), remote_control_handle: Some(remote_control_handle.clone()), plugin_startup_tasks: runtime_options.plugin_startup_tasks, + app_bundled_internal_plugins: runtime_options.app_bundled_internal_plugins, })); let mut thread_created_rx = processor.thread_created_receiver(); let mut running_turn_count_rx = processor.subscribe_running_assistant_turn_count(); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index d89f0b0645..41b6e5b2e8 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -68,6 +68,7 @@ use codex_arg0::Arg0DispatchPaths; use codex_chatgpt::workspace_settings; use codex_core::ThreadManager; use codex_core::config::Config; +use codex_core_plugins::AppBundledInternalPlugin; use codex_exec_server::EnvironmentManager; use codex_feedback::CodexFeedback; use codex_login::AuthManager; @@ -272,6 +273,7 @@ pub(crate) struct MessageProcessorArgs { pub(crate) rpc_transport: AppServerRpcTransport, pub(crate) remote_control_handle: Option, pub(crate) plugin_startup_tasks: crate::PluginStartupTasks, + pub(crate) app_bundled_internal_plugins: Vec, } impl MessageProcessor { @@ -295,6 +297,7 @@ impl MessageProcessor { rpc_transport, remote_control_handle, plugin_startup_tasks, + app_bundled_internal_plugins, } = args; auth_manager.set_external_auth(Arc::new(ExternalAuthRefreshBridge { outgoing: outgoing.clone(), @@ -306,7 +309,7 @@ impl MessageProcessor { let thread_store = codex_core::thread_store_from_config(config.as_ref(), state_db.clone()); let environment_manager_for_requests = Arc::clone(&environment_manager); let thread_manager = Arc::new_cyclic(|thread_manager| { - ThreadManager::new( + ThreadManager::new_with_app_bundled_internal_plugins( config.as_ref(), auth_manager.clone(), session_source, @@ -324,6 +327,7 @@ impl MessageProcessor { outgoing.clone(), thread_state_manager.clone(), )), + app_bundled_internal_plugins, ) }); thread_manager diff --git a/codex-rs/app-server/src/message_processor_tracing_tests.rs b/codex-rs/app-server/src/message_processor_tracing_tests.rs index 771b1fd128..ff991db170 100644 --- a/codex-rs/app-server/src/message_processor_tracing_tests.rs +++ b/codex-rs/app-server/src/message_processor_tracing_tests.rs @@ -267,6 +267,7 @@ async fn build_test_processor( rpc_transport: AppServerRpcTransport::Stdio, remote_control_handle: None, plugin_startup_tasks: crate::PluginStartupTasks::Start, + app_bundled_internal_plugins: Vec::new(), })); (processor, outgoing_rx) } diff --git a/codex-rs/app-server/tests/suite/conversation_summary.rs b/codex-rs/app-server/tests/suite/conversation_summary.rs index 05e3ad1c77..a0de034cb3 100644 --- a/codex-rs/app-server/tests/suite/conversation_summary.rs +++ b/codex-rs/app-server/tests/suite/conversation_summary.rs @@ -171,6 +171,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() -> }), }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; diff --git a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs index d8ecb88996..fb6b38e243 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_resource.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_resource.rs @@ -219,6 +219,7 @@ async fn mcp_resource_read_returns_error_for_unknown_thread() -> Result<()> { capabilities: None, }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; diff --git a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs index 016e4c3df8..9a78f497f4 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_thread_store.rs @@ -94,6 +94,7 @@ async fn thread_start_with_non_local_thread_store_does_not_create_local_persiste capabilities: None, }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index 0a99d30ada..588c963f78 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -397,6 +397,7 @@ async fn thread_turns_list_reads_store_history_without_rollout_path() -> Result< }), }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; @@ -463,6 +464,7 @@ async fn thread_read_loaded_include_turns_reads_store_history_without_rollout_pa }), }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; @@ -549,6 +551,7 @@ async fn thread_list_includes_store_thread_without_rollout_path() -> Result<()> }), }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; diff --git a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs index 84fffd19ba..ec6e5638ee 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_unarchive.rs @@ -268,6 +268,7 @@ async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> { }), }, channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await?; diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 2a5ce171b7..54331bb3a8 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -495,6 +495,14 @@ struct AppServerCommand { #[arg(long = "remote-control", hide = true)] remote_control: bool, + /// Host-owned packaged plugin root for first-class app-bundled internal hooks. + #[arg( + long = "app-bundled-internal-plugin-root", + value_name = "PLUGIN_ID=PATH", + hide = true + )] + app_bundled_internal_plugin_roots: Vec, + /// Controls whether analytics are enabled by default. /// /// Analytics are disabled by default for app-server. Users have to explicitly opt in @@ -1039,6 +1047,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { listen, stdio, remote_control, + app_bundled_internal_plugin_roots, analytics_default_enabled, auth, } = app_server_cli; @@ -1059,6 +1068,7 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { let auth = auth.try_into_settings()?; let runtime_options = codex_app_server::AppServerRuntimeOptions { remote_control_enabled: remote_control, + app_bundled_internal_plugins: app_bundled_internal_plugin_roots, ..Default::default() }; codex_app_server::run_main_with_transport_options( diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index d0096e4455..0fdb8737f5 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -36,6 +36,7 @@ reqwest = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +sha2 = { workspace = true } tar = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } @@ -43,6 +44,7 @@ tokio = { workspace = true, features = ["fs", "macros", "rt", "time"] } toml = { workspace = true } tracing = { workspace = true } url = { workspace = true } +walkdir = { workspace = true } zip = { workspace = true } [dev-dependencies] diff --git a/codex-rs/core-plugins/src/app_bundled_internal.rs b/codex-rs/core-plugins/src/app_bundled_internal.rs new file mode 100644 index 0000000000..218eadbfc6 --- /dev/null +++ b/codex-rs/core-plugins/src/app_bundled_internal.rs @@ -0,0 +1,458 @@ +use std::fs; +use std::io; +use std::io::Write; +use std::str::FromStr; + +use codex_plugin::PluginHookSource; +use codex_plugin::PluginHookSourceKind; +use codex_plugin::PluginId; +use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use serde::Serialize; +use sha2::Digest; +use sha2::Sha256; +use tracing::warn; +use walkdir::WalkDir; + +use crate::loader::load_plugin_hooks; +use crate::manifest::PluginManifestPaths; +use crate::manifest::load_plugin_manifest; + +pub(crate) const APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH: &str = + ".codex-plugin/app-bundled-internal-hooks.json"; +const APP_BUNDLED_INTERNAL_HOOK_RECEIPT_SCHEMA_VERSION: u32 = 1; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AppBundledInternalPlugin { + pub plugin_id: PluginId, + pub plugin_root: AbsolutePathBuf, +} + +impl FromStr for AppBundledInternalPlugin { + type Err = String; + + fn from_str(value: &str) -> Result { + let Some((plugin_id, plugin_root)) = value.split_once('=') else { + return Err( + "expected app-bundled internal plugin as @=" + .to_string(), + ); + }; + let plugin_id = PluginId::parse(plugin_id).map_err(|err| err.to_string())?; + let plugin_root = AbsolutePathBuf::from_absolute_path(plugin_root) + .map_err(|err| format!("invalid packaged plugin root `{plugin_root}`: {err}"))?; + Ok(Self { + plugin_id, + plugin_root, + }) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct AppBundledInternalHookReceipt { + schema_version: u32, + plugin_id: String, + status: AppBundledInternalHookReceiptStatus, + plugin_tree_sha256: String, + hook_sources: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +enum AppBundledInternalHookReceiptStatus { + Verified, + Rejected, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct CanonicalPluginHookSource { + source_relative_path: String, + hooks: codex_config::HookEventsToml, +} + +pub(crate) fn apply_app_bundled_internal_hook_authority( + plugin_id: &PluginId, + installed_plugin_root: &AbsolutePathBuf, + plugin_data_root: &AbsolutePathBuf, + hook_sources: Vec, + hook_load_warnings: Vec, + authorities: &[AppBundledInternalPlugin], +) -> (Vec, Vec) { + if let Some(authority) = authority_for_plugin(plugin_id, authorities) { + return match verify_packaged_hook_declarations( + plugin_id, + installed_plugin_root, + plugin_data_root, + &hook_sources, + &hook_load_warnings, + authority, + ) { + Ok(plugin_tree_sha256) => { + if let Err(err) = write_verified_receipt( + plugin_id, + installed_plugin_root, + &plugin_tree_sha256, + &hook_sources, + ) { + write_rejected_receipt_best_effort(plugin_id, installed_plugin_root); + warn!( + plugin_id = %plugin_id.as_key(), + installed_plugin_root = %installed_plugin_root.display(), + error = %err, + "failed to persist app-bundled internal hook receipt" + ); + return (Vec::new(), Vec::new()); + } + (mark_internal(hook_sources), Vec::new()) + } + Err(err) => { + write_rejected_receipt_best_effort(plugin_id, installed_plugin_root); + warn!( + plugin_id = %plugin_id.as_key(), + installed_plugin_root = %installed_plugin_root.display(), + packaged_plugin_root = %authority.plugin_root.display(), + error = %err, + "app-bundled internal plugin hook verification failed" + ); + (Vec::new(), Vec::new()) + } + }; + } + + match verify_receipt(plugin_id, installed_plugin_root, &hook_sources) { + Ok(ReceiptVerification::Missing) => (hook_sources, hook_load_warnings), + Ok(ReceiptVerification::Valid) => (mark_internal(hook_sources), Vec::new()), + Err(err) => { + warn!( + plugin_id = %plugin_id.as_key(), + installed_plugin_root = %installed_plugin_root.display(), + error = %err, + "app-bundled internal hook receipt verification failed" + ); + (Vec::new(), Vec::new()) + } + } +} + +pub(crate) fn refresh_app_bundled_internal_hook_receipt( + plugin_id: &PluginId, + installed_plugin_root: &AbsolutePathBuf, + plugin_data_root: &AbsolutePathBuf, + authorities: &[AppBundledInternalPlugin], +) -> Result<(), String> { + let Some(authority) = authority_for_plugin(plugin_id, authorities) else { + return Ok(()); + }; + let Some(manifest) = load_plugin_manifest(installed_plugin_root.as_path()) else { + let err = "cannot persist app-bundled internal hook receipt without an installed manifest" + .to_string(); + write_rejected_receipt_best_effort(plugin_id, installed_plugin_root); + return Err(err); + }; + let (hook_sources, hook_load_warnings) = load_plugin_hooks( + installed_plugin_root, + plugin_id, + plugin_data_root, + &manifest.paths, + ); + let result = verify_packaged_hook_declarations( + plugin_id, + installed_plugin_root, + plugin_data_root, + &hook_sources, + &hook_load_warnings, + authority, + ) + .and_then(|plugin_tree_sha256| { + write_verified_receipt( + plugin_id, + installed_plugin_root, + &plugin_tree_sha256, + &hook_sources, + ) + .map_err(|err| err.to_string()) + }); + if let Err(err) = &result { + write_rejected_receipt_best_effort(plugin_id, installed_plugin_root); + warn!( + plugin_id = %plugin_id.as_key(), + installed_plugin_root = %installed_plugin_root.display(), + packaged_plugin_root = %authority.plugin_root.display(), + error = %err, + "failed to persist app-bundled internal hook receipt after plugin install" + ); + } + result +} + +pub(crate) fn should_hide_app_bundled_internal_hook_declarations( + plugin_id: &PluginId, + plugin_root: &AbsolutePathBuf, + plugin_data_root: &AbsolutePathBuf, + manifest_paths: &PluginManifestPaths, + authorities: &[AppBundledInternalPlugin], +) -> bool { + if authority_for_plugin(plugin_id, authorities).is_some() { + return true; + } + + let (hook_sources, hook_load_warnings) = + load_plugin_hooks(plugin_root, plugin_id, plugin_data_root, manifest_paths); + hook_load_warnings.is_empty() + && matches!( + verify_receipt(plugin_id, plugin_root, &hook_sources), + Ok(ReceiptVerification::Valid) + ) +} + +pub(crate) fn remove_app_bundled_internal_hook_receipt( + installed_plugin_root: &AbsolutePathBuf, +) -> io::Result<()> { + let receipt_path = installed_plugin_root.join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH); + match fs::remove_file(receipt_path.as_path()) { + Ok(()) => Ok(()), + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(err), + } +} + +fn authority_for_plugin<'a>( + plugin_id: &PluginId, + authorities: &'a [AppBundledInternalPlugin], +) -> Option<&'a AppBundledInternalPlugin> { + authorities + .iter() + .find(|authority| authority.plugin_id == *plugin_id) +} + +fn verify_packaged_hook_declarations( + plugin_id: &PluginId, + installed_plugin_root: &AbsolutePathBuf, + plugin_data_root: &AbsolutePathBuf, + installed_sources: &[PluginHookSource], + installed_warnings: &[String], + authority: &AppBundledInternalPlugin, +) -> Result { + if !authority.plugin_root.as_path().is_dir() { + return Err("packaged plugin root does not exist or is not a directory".to_string()); + } + let Some(packaged_manifest) = load_plugin_manifest(authority.plugin_root.as_path()) else { + return Err("packaged plugin root is missing a valid plugin manifest".to_string()); + }; + if packaged_manifest.name != plugin_id.plugin_name { + return Err(format!( + "packaged plugin manifest name `{}` does not match `{}`", + packaged_manifest.name, plugin_id.plugin_name + )); + } + if !installed_warnings.is_empty() { + return Err(format!( + "installed declarations failed to load: {}", + installed_warnings.join("; ") + )); + } + + let (packaged_sources, packaged_warnings) = load_plugin_hooks( + &authority.plugin_root, + plugin_id, + plugin_data_root, + &packaged_manifest.paths, + ); + if !packaged_warnings.is_empty() { + return Err(format!( + "packaged declarations failed to load: {}", + packaged_warnings.join("; ") + )); + } + if canonical_hook_sources(installed_sources) != canonical_hook_sources(&packaged_sources) { + return Err(format!( + "installed hook declarations in {} do not match the app-packaged declarations", + installed_plugin_root.display() + )); + } + let installed_tree_sha256 = plugin_tree_sha256(installed_plugin_root)?; + let packaged_tree_sha256 = plugin_tree_sha256(&authority.plugin_root)?; + if installed_tree_sha256 != packaged_tree_sha256 { + return Err(format!( + "installed plugin tree in {} does not match the app-packaged plugin tree", + installed_plugin_root.display() + )); + } + Ok(installed_tree_sha256) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReceiptVerification { + Missing, + Valid, +} + +fn verify_receipt( + plugin_id: &PluginId, + installed_plugin_root: &AbsolutePathBuf, + hook_sources: &[PluginHookSource], +) -> Result { + let receipt_path = installed_plugin_root.join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH); + let contents = match fs::read_to_string(receipt_path.as_path()) { + Ok(contents) => contents, + Err(err) if err.kind() == io::ErrorKind::NotFound => { + return Ok(ReceiptVerification::Missing); + } + Err(err) => return Err(format!("failed to read receipt: {err}")), + }; + let receipt: AppBundledInternalHookReceipt = + serde_json::from_str(&contents).map_err(|err| format!("failed to parse receipt: {err}"))?; + if receipt.schema_version != APP_BUNDLED_INTERNAL_HOOK_RECEIPT_SCHEMA_VERSION { + return Err(format!( + "unsupported receipt schema version {}", + receipt.schema_version + )); + } + if receipt.plugin_id != plugin_id.as_key() { + return Err(format!( + "receipt plugin id `{}` does not match `{}`", + receipt.plugin_id, + plugin_id.as_key() + )); + } + if receipt.status == AppBundledInternalHookReceiptStatus::Rejected { + return Err("app-bundled internal hook receipt records a rejected install".to_string()); + } + let plugin_tree_sha256 = plugin_tree_sha256(installed_plugin_root)?; + if receipt.plugin_tree_sha256 != plugin_tree_sha256 { + return Err("installed plugin tree no longer matches the receipt".to_string()); + } + if receipt.hook_sources != canonical_hook_sources(hook_sources) { + return Err("installed hook declarations no longer match the receipt".to_string()); + } + Ok(ReceiptVerification::Valid) +} + +fn write_verified_receipt( + plugin_id: &PluginId, + installed_plugin_root: &AbsolutePathBuf, + plugin_tree_sha256: &str, + hook_sources: &[PluginHookSource], +) -> io::Result<()> { + write_receipt( + installed_plugin_root, + &AppBundledInternalHookReceipt { + schema_version: APP_BUNDLED_INTERNAL_HOOK_RECEIPT_SCHEMA_VERSION, + plugin_id: plugin_id.as_key(), + status: AppBundledInternalHookReceiptStatus::Verified, + plugin_tree_sha256: plugin_tree_sha256.to_string(), + hook_sources: canonical_hook_sources(hook_sources), + }, + ) +} + +fn write_rejected_receipt_best_effort( + plugin_id: &PluginId, + installed_plugin_root: &AbsolutePathBuf, +) { + let receipt = AppBundledInternalHookReceipt { + schema_version: APP_BUNDLED_INTERNAL_HOOK_RECEIPT_SCHEMA_VERSION, + plugin_id: plugin_id.as_key(), + status: AppBundledInternalHookReceiptStatus::Rejected, + plugin_tree_sha256: String::new(), + hook_sources: Vec::new(), + }; + if let Err(err) = write_receipt(installed_plugin_root, &receipt) { + warn!( + plugin_id = %plugin_id.as_key(), + installed_plugin_root = %installed_plugin_root.display(), + error = %err, + "failed to persist rejected app-bundled internal hook receipt" + ); + } +} + +fn write_receipt( + installed_plugin_root: &AbsolutePathBuf, + receipt: &AppBundledInternalHookReceipt, +) -> io::Result<()> { + let mut contents = serde_json::to_string_pretty(&receipt).map_err(io::Error::other)?; + contents.push('\n'); + let receipt_path = installed_plugin_root.join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH); + let parent = receipt_path.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("receipt path {} has no parent", receipt_path.display()), + ) + })?; + fs::create_dir_all(&parent)?; + let mut temporary = tempfile::NamedTempFile::new_in(&parent)?; + temporary.write_all(contents.as_bytes())?; + temporary + .persist(receipt_path.as_path()) + .map_err(|err| err.error)?; + Ok(()) +} + +fn mark_internal(mut hook_sources: Vec) -> Vec { + for source in &mut hook_sources { + source.kind = PluginHookSourceKind::AppBundledInternal; + } + hook_sources +} + +fn plugin_tree_sha256(plugin_root: &AbsolutePathBuf) -> Result { + let mut files = Vec::new(); + for entry in WalkDir::new(plugin_root.as_path()) { + let entry = entry.map_err(|err| format!("failed to walk plugin tree: {err}"))?; + if !entry.file_type().is_file() { + continue; + } + let relative_path = entry + .path() + .strip_prefix(plugin_root.as_path()) + .map_err(|err| format!("failed to relativize plugin path: {err}"))?; + if relative_path == std::path::Path::new(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH) { + continue; + } + files.push(relative_path.to_path_buf()); + } + files.sort(); + + let mut hasher = Sha256::new(); + for relative_path in files { + let relative_path_string = relative_path + .to_str() + .ok_or_else(|| { + format!( + "app-bundled internal plugin path is not UTF-8: {}", + relative_path.display() + ) + })? + .replace('\\', "/"); + hasher.update((relative_path_string.len() as u64).to_be_bytes()); + hasher.update(relative_path_string.as_bytes()); + let path = plugin_root.join(&relative_path); + let contents = fs::read(path.as_path()) + .map_err(|err| format!("failed to read plugin file `{relative_path_string}`: {err}"))?; + hasher.update((contents.len() as u64).to_be_bytes()); + hasher.update(contents); + } + let digest = hasher.finalize(); + Ok(format!( + "sha256:{}", + digest + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::() + )) +} + +fn canonical_hook_sources(sources: &[PluginHookSource]) -> Vec { + sources + .iter() + .map(|source| CanonicalPluginHookSource { + source_relative_path: source.source_relative_path.clone(), + hooks: source.hooks.clone(), + }) + .collect() +} + +#[cfg(test)] +#[path = "app_bundled_internal_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/app_bundled_internal_tests.rs b/codex-rs/core-plugins/src/app_bundled_internal_tests.rs new file mode 100644 index 0000000000..72c1b2a7fc --- /dev/null +++ b/codex-rs/core-plugins/src/app_bundled_internal_tests.rs @@ -0,0 +1,392 @@ +use std::fs; + +use codex_plugin::PluginHookSource; +use codex_plugin::PluginHookSourceKind; +use codex_plugin::PluginId; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use tempfile::TempDir; + +use super::*; +use crate::loader::load_plugin_hooks; +use crate::manifest::load_plugin_manifest; + +fn computer_use_plugin_id() -> PluginId { + PluginId::parse("computer-use@openai-bundled").expect("computer-use plugin id") +} + +fn plugin_root(temp: &TempDir, name: &str) -> AbsolutePathBuf { + let root = AbsolutePathBuf::from_absolute_path(temp.path().join(name)) + .expect("plugin root should be absolute"); + fs::create_dir_all(root.join(".codex-plugin")).expect("create manifest dir"); + fs::create_dir_all(root.join("hooks")).expect("create hooks dir"); + root +} + +fn write_manifest(root: &AbsolutePathBuf, manifest: &str) { + fs::write(root.join(".codex-plugin/plugin.json"), manifest).expect("write manifest"); +} + +fn write_default_hooks(root: &AbsolutePathBuf, command: &str, command_windows: Option<&str>) { + let command_windows = command_windows + .map(|command| format!(r#","commandWindows":"{command}""#)) + .unwrap_or_default(); + fs::write( + root.join("hooks/hooks.json"), + format!( + r#"{{ + "hooks": {{ + "PreToolUse": [ + {{ + "matcher": "Bash", + "hooks": [ + {{ + "type": "command", + "command": "{command}", + "timeout": 12, + "async": false, + "statusMessage": "checking"{command_windows} + }} + ] + }} + ] + }} +}}"# + ), + ) + .expect("write hooks"); +} + +fn load_sources( + plugin_root: &AbsolutePathBuf, + plugin_id: &PluginId, +) -> (Vec, Vec) { + let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest"); + let plugin_data_root = plugin_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"); + let plugin_data_root = + AbsolutePathBuf::from_absolute_path(plugin_data_root).expect("plugin data root"); + load_plugin_hooks(plugin_root, plugin_id, &plugin_data_root, &manifest.paths) +} + +fn apply_authority( + plugin_id: &PluginId, + installed_root: &AbsolutePathBuf, + packaged_root: &AbsolutePathBuf, + hook_sources: Vec, + hook_load_warnings: Vec, +) -> (Vec, Vec) { + let plugin_data_root = installed_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"); + let plugin_data_root = + AbsolutePathBuf::from_absolute_path(plugin_data_root).expect("plugin data root"); + apply_app_bundled_internal_hook_authority( + plugin_id, + installed_root, + &plugin_data_root, + hook_sources, + hook_load_warnings, + &[AppBundledInternalPlugin { + plugin_id: plugin_id.clone(), + plugin_root: packaged_root.clone(), + }], + ) +} + +#[test] +fn matching_declarations_become_internal_and_persist_a_receipt() { + let temp = TempDir::new().expect("tempdir"); + let installed_root = plugin_root(&temp, "installed"); + let packaged_root = plugin_root(&temp, "packaged"); + write_manifest(&installed_root, r#"{ "name": "computer-use" }"#); + write_manifest(&packaged_root, r#"{ "name": "computer-use" }"#); + write_default_hooks( + &installed_root, + "python3 hooks/check.py", + Some("py hooks/check.py"), + ); + write_default_hooks( + &packaged_root, + "python3 hooks/check.py", + Some("py hooks/check.py"), + ); + let plugin_id = computer_use_plugin_id(); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + + let (verified_sources, warnings) = apply_authority( + &plugin_id, + &installed_root, + &packaged_root, + hook_sources, + hook_load_warnings, + ); + + assert_eq!(warnings, Vec::::new()); + assert_eq!(verified_sources.len(), 1); + assert_eq!( + verified_sources[0].kind, + PluginHookSourceKind::AppBundledInternal + ); + assert_eq!( + verified_sources[0].source_path, + installed_root.join("hooks/hooks.json") + ); + assert!( + installed_root + .join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH) + .is_file() + ); + assert!( + fs::read_to_string(installed_root.join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH)) + .expect("read receipt") + .contains(r#""status": "verified""#) + ); +} + +#[test] +fn mismatched_relative_source_path_fails_closed() { + let temp = TempDir::new().expect("tempdir"); + let installed_root = plugin_root(&temp, "installed"); + let packaged_root = plugin_root(&temp, "packaged"); + write_manifest(&installed_root, r#"{ "name": "computer-use" }"#); + write_manifest( + &packaged_root, + r#"{ "name": "computer-use", "hooks": "./hooks/internal.json" }"#, + ); + write_default_hooks( + &installed_root, + "python3 hooks/check.py", + /*command_windows*/ None, + ); + fs::write( + packaged_root.join("hooks/internal.json"), + fs::read_to_string(installed_root.join("hooks/hooks.json")).expect("read installed hooks"), + ) + .expect("write packaged hooks"); + let plugin_id = computer_use_plugin_id(); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + + let (verified_sources, warnings) = apply_authority( + &plugin_id, + &installed_root, + &packaged_root, + hook_sources, + hook_load_warnings, + ); + + assert_eq!(verified_sources, Vec::::new()); + assert_eq!(warnings, Vec::::new()); + assert!( + fs::read_to_string(installed_root.join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH)) + .expect("read rejected receipt") + .contains(r#""status": "rejected""#) + ); + + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + let plugin_data_root = installed_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"); + let plugin_data_root = + AbsolutePathBuf::from_absolute_path(plugin_data_root).expect("plugin data root"); + let (standalone_sources, standalone_warnings) = apply_app_bundled_internal_hook_authority( + &plugin_id, + &installed_root, + &plugin_data_root, + hook_sources, + hook_load_warnings, + &[], + ); + + assert_eq!(standalone_sources, Vec::::new()); + assert_eq!(standalone_warnings, Vec::::new()); +} + +#[test] +fn exact_app_authority_supports_other_bundled_plugin_ids() { + let temp = TempDir::new().expect("tempdir"); + let installed_root = plugin_root(&temp, "installed"); + let packaged_root = plugin_root(&temp, "packaged"); + write_manifest(&installed_root, r#"{ "name": "demo-plugin" }"#); + write_manifest(&packaged_root, r#"{ "name": "demo-plugin" }"#); + write_default_hooks( + &installed_root, + "echo installed", + /*command_windows*/ None, + ); + write_default_hooks( + &packaged_root, + "echo installed", + /*command_windows*/ None, + ); + let plugin_id = PluginId::parse("demo-plugin@openai-bundled").expect("plugin id"); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + + let (verified_sources, warnings) = apply_authority( + &plugin_id, + &installed_root, + &packaged_root, + hook_sources, + hook_load_warnings, + ); + + assert_eq!(warnings, Vec::::new()); + assert_eq!(verified_sources.len(), 1); + assert_eq!( + verified_sources[0].kind, + PluginHookSourceKind::AppBundledInternal + ); +} + +#[test] +fn receipt_preserves_internal_classification_without_the_app_running() { + let temp = TempDir::new().expect("tempdir"); + let installed_root = plugin_root(&temp, "installed"); + let packaged_root = plugin_root(&temp, "packaged"); + write_manifest(&installed_root, r#"{ "name": "computer-use" }"#); + write_manifest(&packaged_root, r#"{ "name": "computer-use" }"#); + write_default_hooks( + &installed_root, + "python3 hooks/check.py", + /*command_windows*/ None, + ); + write_default_hooks( + &packaged_root, + "python3 hooks/check.py", + /*command_windows*/ None, + ); + let plugin_id = computer_use_plugin_id(); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + + let (verified_sources, warnings) = apply_authority( + &plugin_id, + &installed_root, + &packaged_root, + hook_sources, + hook_load_warnings, + ); + assert_eq!(warnings, Vec::::new()); + assert_eq!( + verified_sources[0].kind, + PluginHookSourceKind::AppBundledInternal + ); + + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + let plugin_data_root = installed_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"); + let plugin_data_root = + AbsolutePathBuf::from_absolute_path(plugin_data_root).expect("plugin data root"); + let (standalone_sources, standalone_warnings) = apply_app_bundled_internal_hook_authority( + &plugin_id, + &installed_root, + &plugin_data_root, + hook_sources, + hook_load_warnings, + &[], + ); + + assert_eq!(standalone_warnings, Vec::::new()); + assert_eq!( + standalone_sources[0].kind, + PluginHookSourceKind::AppBundledInternal + ); +} + +#[test] +fn modified_declarations_fail_closed_when_only_a_receipt_is_available() { + let temp = TempDir::new().expect("tempdir"); + let installed_root = plugin_root(&temp, "installed"); + let packaged_root = plugin_root(&temp, "packaged"); + write_manifest(&installed_root, r#"{ "name": "computer-use" }"#); + write_manifest(&packaged_root, r#"{ "name": "computer-use" }"#); + write_default_hooks(&installed_root, "echo original", None); + write_default_hooks(&packaged_root, "echo original", None); + let plugin_id = computer_use_plugin_id(); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + let _ = apply_authority( + &plugin_id, + &installed_root, + &packaged_root, + hook_sources, + hook_load_warnings, + ); + + write_default_hooks(&installed_root, "echo modified", None); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + let plugin_data_root = installed_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"); + let plugin_data_root = + AbsolutePathBuf::from_absolute_path(plugin_data_root).expect("plugin data root"); + let (standalone_sources, standalone_warnings) = apply_app_bundled_internal_hook_authority( + &plugin_id, + &installed_root, + &plugin_data_root, + hook_sources, + hook_load_warnings, + &[], + ); + + assert_eq!(standalone_sources, Vec::::new()); + assert_eq!(standalone_warnings, Vec::::new()); +} + +#[test] +fn modified_hook_payload_fails_closed_when_only_a_receipt_is_available() { + let temp = TempDir::new().expect("tempdir"); + let installed_root = plugin_root(&temp, "installed"); + let packaged_root = plugin_root(&temp, "packaged"); + write_manifest(&installed_root, r#"{ "name": "computer-use" }"#); + write_manifest(&packaged_root, r#"{ "name": "computer-use" }"#); + write_default_hooks(&installed_root, "python3 hooks/check.py", None); + write_default_hooks(&packaged_root, "python3 hooks/check.py", None); + fs::write(installed_root.join("hooks/check.py"), "print('original')\n") + .expect("write installed payload"); + fs::write(packaged_root.join("hooks/check.py"), "print('original')\n") + .expect("write packaged payload"); + let plugin_id = computer_use_plugin_id(); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + let (verified_sources, warnings) = apply_authority( + &plugin_id, + &installed_root, + &packaged_root, + hook_sources, + hook_load_warnings, + ); + assert_eq!(verified_sources.len(), 1); + assert_eq!(warnings, Vec::::new()); + + fs::write(installed_root.join("hooks/check.py"), "print('modified')\n") + .expect("modify installed payload"); + let (hook_sources, hook_load_warnings) = load_sources(&installed_root, &plugin_id); + let plugin_data_root = installed_root + .as_path() + .parent() + .expect("plugin root parent") + .join("plugin-data"); + let plugin_data_root = + AbsolutePathBuf::from_absolute_path(plugin_data_root).expect("plugin data root"); + let (standalone_sources, standalone_warnings) = apply_app_bundled_internal_hook_authority( + &plugin_id, + &installed_root, + &plugin_data_root, + hook_sources, + hook_load_warnings, + &[], + ); + + assert_eq!(standalone_sources, Vec::::new()); + assert_eq!(standalone_warnings, Vec::::new()); +} diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 98cb73e127..63e9757e95 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -1,3 +1,4 @@ +mod app_bundled_internal; mod discoverable; pub mod installed_marketplaces; pub mod loader; @@ -24,6 +25,7 @@ pub const OPENAI_BUNDLED_MARKETPLACE_NAME: &str = "openai-bundled"; pub type LoadedPlugin = codex_plugin::LoadedPlugin; pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; +pub use app_bundled_internal::AppBundledInternalPlugin; pub use discoverable::ToolSuggestDiscoverablePlugin; pub use discoverable::ToolSuggestPluginDiscoveryInput; pub use loader::PluginHookLoadOutcome; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index d6414be6ff..ce57c316bf 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1,4 +1,6 @@ use crate::OPENAI_CURATED_MARKETPLACE_NAME; +use crate::app_bundled_internal::AppBundledInternalPlugin; +use crate::app_bundled_internal::apply_app_bundled_internal_hook_authority; use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; @@ -25,6 +27,7 @@ use codex_plugin::AppConnectorId; use codex_plugin::LoadedPlugin; use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginHookSource; +use codex_plugin::PluginHookSourceKind; use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_plugin::PluginLoadOutcome; @@ -130,6 +133,7 @@ pub async fn load_plugins_from_layer_stack( store: &PluginStore, restriction_product: Option, prefer_remote_curated_conflicts: bool, + app_bundled_internal_plugins: &[AppBundledInternalPlugin], ) -> PluginLoadOutcome { let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); load_plugins_from_layer_stack_with_scope( @@ -137,6 +141,7 @@ pub async fn load_plugins_from_layer_stack( extra_plugins, store, prefer_remote_curated_conflicts, + app_bundled_internal_plugins, PluginLoadScope::AllCapabilities { restriction_product, skill_config_rules: &skill_config_rules, @@ -150,6 +155,7 @@ async fn load_plugins_from_layer_stack_with_scope( extra_plugins: HashMap, store: &PluginStore, prefer_remote_curated_conflicts: bool, + app_bundled_internal_plugins: &[AppBundledInternalPlugin], scope: PluginLoadScope<'_>, ) -> PluginLoadOutcome { let configured_plugins = merge_configured_plugins_with_remote_installed( @@ -164,7 +170,14 @@ async fn load_plugins_from_layer_stack_with_scope( let mut plugins = Vec::with_capacity(configured_plugins.len()); let mut seen_mcp_server_names = HashMap::::new(); for (configured_name, plugin) in configured_plugins { - let loaded_plugin = load_plugin(configured_name.clone(), &plugin, store, &scope).await; + let loaded_plugin = load_plugin( + configured_name.clone(), + &plugin, + store, + &scope, + app_bundled_internal_plugins, + ) + .await; for name in loaded_plugin.mcp_servers.keys() { if let Some(previous_plugin) = seen_mcp_server_names.insert(name.clone(), configured_name.clone()) @@ -189,12 +202,14 @@ pub async fn load_plugin_hooks_from_layer_stack( extra_plugins: HashMap, store: &PluginStore, prefer_remote_curated_conflicts: bool, + app_bundled_internal_plugins: &[AppBundledInternalPlugin], ) -> PluginHookLoadOutcome { let outcome = load_plugins_from_layer_stack_with_scope( config_layer_stack, extra_plugins, store, prefer_remote_curated_conflicts, + app_bundled_internal_plugins, PluginLoadScope::HooksOnly, ) .await; @@ -607,6 +622,7 @@ async fn load_plugin( plugin: &PluginConfig, store: &PluginStore, scope: &PluginLoadScope<'_>, + app_bundled_internal_plugins: &[AppBundledInternalPlugin], ) -> LoadedPlugin { let plugin_id = PluginId::parse(&config_name); let active_plugin_root = plugin_id @@ -719,6 +735,14 @@ async fn load_plugin( &store.plugin_data_root(&loaded_plugin_id), manifest_paths, ); + let (hook_sources, hook_load_warnings) = apply_app_bundled_internal_hook_authority( + &loaded_plugin_id, + &plugin_root, + &store.plugin_data_root(&loaded_plugin_id), + hook_sources, + hook_load_warnings, + app_bundled_internal_plugins, + ); loaded_plugin.hook_sources = hook_sources; loaded_plugin.hook_load_warnings = hook_load_warnings; loaded_plugin @@ -911,6 +935,7 @@ pub fn load_plugin_hooks( source_path: manifest_path.clone(), source_relative_path: format!("plugin.json#hooks[{index}]"), hooks: hooks_file.hooks.clone(), + kind: PluginHookSourceKind::UserReviewed, }); } } @@ -979,6 +1004,7 @@ fn append_plugin_hook_file( source_path: path.clone(), source_relative_path, hooks: parsed.hooks, + kind: PluginHookSourceKind::UserReviewed, }); } diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 2e89776b49..44458fb450 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -162,6 +162,7 @@ enabled = true &store, Some(Product::Codex), /*prefer_remote_curated_conflicts*/ false, + &[], ) .await; let hooks_only = load_plugins_from_layer_stack_with_scope( @@ -169,6 +170,7 @@ enabled = true HashMap::new(), &store, /*prefer_remote_curated_conflicts*/ false, + &[], PluginLoadScope::HooksOnly, ) .await; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 08f668d130..c262197eb5 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1,6 +1,9 @@ use super::PluginLoadOutcome; use super::startup_remote_sync::start_startup_remote_plugin_sync_once; use crate::OPENAI_CURATED_MARKETPLACE_NAME; +use crate::app_bundled_internal::AppBundledInternalPlugin; +use crate::app_bundled_internal::refresh_app_bundled_internal_hook_receipt; +use crate::app_bundled_internal::should_hide_app_bundled_internal_hook_declarations; use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack; use crate::loader::PluginHookLoadOutcome; use crate::loader::configured_curated_plugin_ids_from_codex_home; @@ -409,6 +412,7 @@ pub struct PluginsManager { remote_sync_lock: Semaphore, restriction_product: Option, analytics_events_client: RwLock>, + app_bundled_internal_plugins: Vec, } #[derive(Clone)] @@ -425,6 +429,18 @@ impl PluginsManager { pub fn new_with_restriction_product( codex_home: PathBuf, restriction_product: Option, + ) -> Self { + Self::new_with_restriction_product_and_app_bundled_internal_plugins( + codex_home, + restriction_product, + Vec::new(), + ) + } + + pub fn new_with_restriction_product_and_app_bundled_internal_plugins( + codex_home: PathBuf, + restriction_product: Option, + app_bundled_internal_plugins: Vec, ) -> Self { // Product restrictions are enforced at marketplace admission time for a given CODEX_HOME: // listing, install, and curated refresh all consult this restriction context before new @@ -449,6 +465,7 @@ impl PluginsManager { remote_sync_lock: Semaphore::new(/*permits*/ 1), restriction_product, analytics_events_client: RwLock::new(None), + app_bundled_internal_plugins, } } @@ -495,6 +512,7 @@ impl PluginsManager { &self.store, self.restriction_product, config.remote_plugin_enabled, + &self.app_bundled_internal_plugins, ) .await; log_plugin_load_errors(&outcome); @@ -541,6 +559,7 @@ impl PluginsManager { &self.store, self.restriction_product, config.remote_plugin_enabled, + &self.app_bundled_internal_plugins, ) .await } @@ -559,6 +578,7 @@ impl PluginsManager { self.remote_installed_plugin_configs(), &self.store, config.remote_plugin_enabled, + &self.app_bundled_internal_plugins, ) .await } @@ -923,6 +943,23 @@ impl PluginsManager { .await .map_err(PluginInstallError::join)??; + if let Err(err) = refresh_app_bundled_internal_hook_receipt( + &result.plugin_id, + &result.installed_path, + &self.store.plugin_data_root(&result.plugin_id), + &self.app_bundled_internal_plugins, + ) { + let store = self.store.clone(); + let plugin_id = result.plugin_id.clone(); + tokio::task::spawn_blocking(move || store.uninstall(&plugin_id)) + .await + .map_err(PluginInstallError::join)??; + return Err(PluginStoreError::Invalid(format!( + "app-bundled internal hook verification failed: {err}" + )) + .into()); + } + set_user_plugin_enabled( &self.codex_home, result.plugin_id.as_key(), @@ -1477,15 +1514,25 @@ impl PluginsManager { ) .await; let plugin_data_root = self.store.plugin_data_root(&plugin_id); - let (hook_sources, _hook_load_warnings) = - load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths); - let hooks = plugin_hook_declarations(&hook_sources) - .into_iter() - .map(|hook| PluginHookSummary { - key: hook.key, - event_name: hook.event_name, - }) - .collect(); + let hooks = if should_hide_app_bundled_internal_hook_declarations( + &plugin_id, + &source_path, + &plugin_data_root, + &manifest.paths, + &self.app_bundled_internal_plugins, + ) { + Vec::new() + } else { + let (hook_sources, _hook_load_warnings) = + load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths); + plugin_hook_declarations(&hook_sources) + .into_iter() + .map(|hook| PluginHookSummary { + key: hook.key, + event_name: hook.event_name, + }) + .collect() + }; let apps = load_plugin_apps(source_path.as_path()).await; let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path()) .await diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index d6c13b1fe0..064cb41672 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -1394,6 +1394,69 @@ async fn install_plugin_updates_config_with_relative_path_and_plugin_key() { assert!(config.contains("enabled = true")); } +#[tokio::test] +async fn install_plugin_rejects_mismatched_app_bundled_internal_hooks() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + let packaged_root = tmp.path().join("packaged-computer-use"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + write_plugin(&repo_root, "computer-use", "computer-use"); + write_plugin(tmp.path(), "packaged-computer-use", "computer-use"); + write_file( + &repo_root.join("computer-use/hooks/hooks.json"), + r#"{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"echo installed"}]}]}}"#, + ); + write_file( + &packaged_root.join("hooks/hooks.json"), + r#"{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"echo packaged"}]}]}}"#, + ); + fs::write( + repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "openai-bundled", + "plugins": [ + { + "name": "computer-use", + "source": { + "source": "local", + "path": "./computer-use" + } + } + ] +}"#, + ) + .unwrap(); + let plugin_id = PluginId::parse("computer-use@openai-bundled").unwrap(); + let manager = PluginsManager::new_with_restriction_product_and_app_bundled_internal_plugins( + tmp.path().to_path_buf(), + Some(Product::Codex), + vec![AppBundledInternalPlugin { + plugin_id, + plugin_root: AbsolutePathBuf::try_from(packaged_root).unwrap(), + }], + ); + + let err = manager + .install_plugin(PluginInstallRequest { + plugin_name: "computer-use".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + repo_root.join(".agents/plugins/marketplace.json"), + ) + .unwrap(), + }) + .await + .unwrap_err(); + + assert!(err.is_invalid_request()); + assert!( + !tmp.path() + .join("plugins/cache/openai-bundled/computer-use") + .exists() + ); + assert!(!tmp.path().join(CONFIG_TOML_FILE).exists()); +} + #[tokio::test] async fn install_openai_curated_plugin_uses_short_sha_cache_version() { let tmp = tempfile::tempdir().unwrap(); @@ -2213,6 +2276,76 @@ enabled = false ); } +#[tokio::test] +async fn app_bundled_internal_hooks_are_hidden_from_plugin_detail() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + let plugin_root = repo_root.join("computer-use"); + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"computer-use"}"#, + ); + write_file( + &plugin_root.join("hooks/hooks.json"), + r#"{ + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "echo internal" + } + ] + } + ] + } +}"#, + ); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + let config = load_config(tmp.path(), &repo_root).await; + let plugin_root = AbsolutePathBuf::try_from(plugin_root).unwrap(); + let manager = PluginsManager::new_with_restriction_product_and_app_bundled_internal_plugins( + tmp.path().to_path_buf(), + Some(Product::Codex), + vec![AppBundledInternalPlugin { + plugin_id: PluginId::parse("computer-use@openai-bundled").unwrap(), + plugin_root: plugin_root.clone(), + }], + ); + + let detail = manager + .read_plugin_detail_for_marketplace_plugin( + &config, + "openai-bundled", + ConfiguredMarketplacePlugin { + id: "computer-use@openai-bundled".to_string(), + name: "computer-use".to_string(), + local_version: None, + installed_version: None, + source: MarketplacePluginSource::Local { path: plugin_root }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: None, + keywords: Vec::new(), + installed: false, + enabled: false, + }, + ) + .await + .unwrap(); + + assert_eq!(detail.hooks, Vec::::new()); +} + #[tokio::test] async fn list_marketplaces_installed_git_source_reads_metadata_from_cache_without_cloning() { let tmp = tempfile::tempdir().unwrap(); @@ -3853,6 +3986,7 @@ async fn load_plugins_ignores_project_config_files() { &PluginStore::new(codex_home.path().to_path_buf()), Some(Product::Codex), /*prefer_remote_curated_conflicts*/ false, + &[], ) .await; diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 19c3b0116f..92015f9644 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -1,3 +1,4 @@ +use crate::app_bundled_internal::remove_app_bundled_internal_hook_receipt; use crate::manifest::PluginManifest; use crate::manifest::load_plugin_manifest; use codex_plugin::PluginId; @@ -135,6 +136,12 @@ impl PluginStore { self.plugin_base_root(&plugin_id).as_path(), &plugin_version, )?; + remove_app_bundled_internal_hook_receipt(&installed_path).map_err(|err| { + PluginStoreError::io( + "failed to remove source-supplied app-bundled internal hook receipt", + err, + ) + })?; Ok(PluginInstallResult { plugin_id, diff --git a/codex-rs/core-plugins/src/store_tests.rs b/codex-rs/core-plugins/src/store_tests.rs index 200055fe6c..33af5f7abe 100644 --- a/codex-rs/core-plugins/src/store_tests.rs +++ b/codex-rs/core-plugins/src/store_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::app_bundled_internal::APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH; use codex_plugin::PluginId; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -71,6 +72,34 @@ fn install_copies_plugin_into_default_marketplace() { assert!(installed_path.join("skills/SKILL.md").is_file()); } +#[test] +fn install_removes_source_supplied_internal_hook_receipt() { + let tmp = tempdir().unwrap(); + write_plugin(tmp.path(), "sample-plugin", "sample-plugin"); + fs::write( + tmp.path() + .join("sample-plugin") + .join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH), + r#"{"schema_version":1}"#, + ) + .unwrap(); + let plugin_id = PluginId::new("sample-plugin".to_string(), "debug".to_string()).unwrap(); + + let result = PluginStore::new(tmp.path().to_path_buf()) + .install( + AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap(), + plugin_id, + ) + .unwrap(); + + assert!( + !result + .installed_path + .join(APP_BUNDLED_INTERNAL_HOOK_RECEIPT_PATH) + .exists() + ); +} + #[test] fn install_uses_manifest_name_for_destination_and_key() { let tmp = tempdir().unwrap(); diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index 6e2d4fe33c..384a890f65 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -36,6 +36,7 @@ use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use codex_thread_store::ReadThreadParams; use serde_json::Value; +use tracing::warn; use crate::context::ContextualUserFragment; use crate::context::HookAdditionalContext; @@ -609,6 +610,9 @@ async fn emit_hook_started_events( preview_runs: Vec, ) { for run in preview_runs { + if run.source == HookSource::AppBundledInternal { + continue; + } sess.send_event( turn_context, EventMsg::HookStarted(HookStartedEvent { @@ -628,6 +632,17 @@ pub(crate) async fn emit_hook_completed_events( for completed in completed_events { emit_hook_completed_metrics(turn_context, &completed); track_hook_completed_analytics(sess, turn_context, &completed); + if completed.run.source == HookSource::AppBundledInternal { + if completed.run.status != HookRunStatus::Completed { + warn!( + hook_name = hook_event_name_label(completed.run.event_name), + status = hook_run_status_label(completed.run.status), + source = %completed.run.source_path.display(), + "app-bundled internal plugin hook did not complete successfully" + ); + } + continue; + } sess.send_event(turn_context, EventMsg::HookCompleted(completed)) .await; } @@ -684,7 +699,19 @@ fn hook_run_analytics_payload( } fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); 3] { - let hook_name = match run.event_name { + let hook_name = hook_event_name_label(run.event_name); + let hook_source = hook_source_label(run.source); + let status = hook_run_status_label(run.status); + + [ + ("hook_name", hook_name), + ("source", hook_source), + ("status", status), + ] +} + +fn hook_event_name_label(event_name: HookEventName) -> &'static str { + match event_name { HookEventName::PreToolUse => "PreToolUse", HookEventName::PermissionRequest => "PermissionRequest", HookEventName::PostToolUse => "PostToolUse", @@ -695,8 +722,11 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); HookEventName::SubagentStart => "SubagentStart", HookEventName::SubagentStop => "SubagentStop", HookEventName::Stop => "Stop", - }; - let hook_source = match run.source { + } +} + +fn hook_source_label(source: HookSource) -> &'static str { + match source { HookSource::System => "system", HookSource::User => "user", HookSource::Project => "project", @@ -707,21 +737,19 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); HookSource::CloudManagedConfig => "cloud_managed_config", HookSource::LegacyManagedConfigFile => "legacy_managed_config_file", HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm", + HookSource::AppBundledInternal => "app_bundled_internal", HookSource::Unknown => "unknown", - }; - let status = match run.status { + } +} + +fn hook_run_status_label(status: HookRunStatus) -> &'static str { + match status { HookRunStatus::Running => "running", HookRunStatus::Completed => "completed", HookRunStatus::Failed => "failed", HookRunStatus::Blocked => "blocked", HookRunStatus::Stopped => "stopped", - }; - - [ - ("hook_name", hook_name), - ("source", hook_source), - ("status", status), - ] + } } fn hook_permission_mode(turn_context: &TurnContext) -> String { @@ -775,9 +803,12 @@ mod tests { use pretty_assertions::assert_eq; use super::additional_context_messages; + use super::emit_hook_completed_events; + use super::emit_hook_started_events; use super::hook_run_analytics_payload; use super::hook_run_metric_tags; use crate::session::tests::make_session_and_context; + use crate::session::tests::make_session_and_context_with_rx; use codex_protocol::protocol::HookCompletedEvent; use codex_protocol::protocol::HookRunSummary; use codex_utils_absolute_path::test_support::PathBufExt; @@ -852,6 +883,49 @@ mod tests { assert_eq!(hook.status, HookRunStatus::Failed); } + #[tokio::test] + async fn app_bundled_internal_hook_notifications_are_suppressed_when_completed() { + let (session, turn_context, rx) = make_session_and_context_with_rx().await; + + emit_hook_started_events( + &session, + &turn_context, + vec![sample_hook_run( + HookRunStatus::Running, + HookSource::AppBundledInternal, + )], + ) + .await; + emit_hook_completed_events( + &session, + &turn_context, + vec![HookCompletedEvent { + turn_id: None, + run: sample_hook_run(HookRunStatus::Completed, HookSource::AppBundledInternal), + }], + ) + .await; + + assert!(rx.try_recv().is_err()); + } + + #[tokio::test] + async fn app_bundled_internal_hook_failures_do_not_emit_client_events() { + let (session, turn_context, rx) = make_session_and_context_with_rx().await; + + emit_hook_completed_events( + &session, + &turn_context, + vec![HookCompletedEvent { + turn_id: None, + run: sample_hook_run(HookRunStatus::Blocked, HookSource::AppBundledInternal), + }], + ) + .await; + + assert!(rx.try_recv().is_err()); + } + #[test] fn hook_run_metric_tags_match_analytics_shape() { let run = sample_hook_run(HookRunStatus::Blocked, HookSource::Project); @@ -890,6 +964,17 @@ mod tests { ("status", "completed"), ] ); + + let internal = sample_hook_run(HookRunStatus::Completed, HookSource::AppBundledInternal); + + assert_eq!( + hook_run_metric_tags(&internal), + [ + ("hook_name", "Stop"), + ("source", "app_bundled_internal"), + ("status", "completed"), + ] + ); } fn sample_hook_run(status: HookRunStatus, source: HookSource) -> HookRunSummary { diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index d130a35815..e7273dc484 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -19,6 +19,7 @@ use crate::tasks::interrupted_turn_history_marker; use codex_analytics::AnalyticsEventsClient; use codex_app_server_protocol::ThreadHistoryBuilder; use codex_app_server_protocol::TurnStatus; +use codex_core_plugins::AppBundledInternalPlugin; use codex_core_plugins::PluginsManager; use codex_exec_server::EnvironmentManager; use codex_extension_api::ExtensionRegistry; @@ -262,14 +263,46 @@ impl ThreadManager { state_db: Option, installation_id: String, attestation_provider: Option>, + ) -> Self { + Self::new_with_app_bundled_internal_plugins( + config, + auth_manager, + session_source, + environment_manager, + extensions, + analytics_events_client, + thread_store, + state_db, + installation_id, + attestation_provider, + Vec::new(), + ) + } + + #[allow(clippy::too_many_arguments)] + pub fn new_with_app_bundled_internal_plugins( + config: &Config, + auth_manager: Arc, + session_source: SessionSource, + environment_manager: Arc, + extensions: Arc>, + analytics_events_client: Option, + thread_store: Arc, + state_db: Option, + installation_id: String, + attestation_provider: Option>, + app_bundled_internal_plugins: Vec, ) -> Self { let codex_home = config.codex_home.clone(); let restriction_product = session_source.restriction_product(); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); - let plugins_manager = Arc::new(PluginsManager::new_with_restriction_product( - codex_home.to_path_buf(), - restriction_product, - )); + let plugins_manager = Arc::new( + PluginsManager::new_with_restriction_product_and_app_bundled_internal_plugins( + codex_home.to_path_buf(), + restriction_product, + app_bundled_internal_plugins, + ), + ); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( codex_home, diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 5813508e7a..97527a4105 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -9,6 +9,7 @@ use codex_features::Feature; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::built_in_model_providers; use codex_plugin::PluginHookSource; +use codex_plugin::PluginHookSourceKind; use codex_plugin::PluginId; use codex_protocol::items::parse_hook_prompt_fragment; use codex_protocol::models::ContentItem; @@ -2926,6 +2927,7 @@ print(json.dumps({{ hooks: serde_json::from_str::(plugin_hooks_json) .context("parse plugin hooks")? .hooks, + kind: PluginHookSourceKind::UserReviewed, }]; let mut builder = test_codex() diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 797e168dc5..f2eb778510 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -551,6 +551,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result experimental_api: true, opt_out_notification_methods: Vec::new(), channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }; run_exec_session(ExecRunArgs { in_process_start_args, diff --git a/codex-rs/hooks/src/declarations.rs b/codex-rs/hooks/src/declarations.rs index 468fbc947a..98f017b418 100644 --- a/codex-rs/hooks/src/declarations.rs +++ b/codex-rs/hooks/src/declarations.rs @@ -41,6 +41,7 @@ mod tests { use codex_config::HookEventsToml; use codex_config::HookHandlerConfig; use codex_config::MatcherGroup; + use codex_plugin::PluginHookSourceKind; use codex_plugin::PluginId; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::test_path_buf; @@ -78,6 +79,7 @@ mod tests { }], ..Default::default() }, + kind: PluginHookSourceKind::UserReviewed, }]); assert_eq!( diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 92d17fbabd..37860abdc0 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -17,6 +17,7 @@ use codex_config::RequirementSource; use codex_config::TomlValue; use codex_config::version_for_toml; use codex_plugin::PluginHookSource; +use codex_plugin::PluginHookSourceKind; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; @@ -41,6 +42,7 @@ struct HookHandlerSource<'a> { key_source: String, source: HookSource, is_managed: bool, + include_in_hook_list: bool, bypass_hook_trust: bool, hook_states: &'a HashMap, env: HashMap, @@ -103,6 +105,7 @@ pub(crate) fn discover_handlers( key_source: policy_path.display().to_string(), source: hook_source, is_managed, + include_in_hook_list: true, bypass_hook_trust: false, hook_states: &hook_states, env: HashMap::new(), @@ -137,6 +140,7 @@ pub(crate) fn discover_handlers( key_source: source_path.display().to_string(), source: hook_source, is_managed, + include_in_hook_list: true, bypass_hook_trust: policy.bypass_hook_trust, hook_states: &hook_states, env: HashMap::new(), @@ -189,6 +193,7 @@ fn append_managed_requirement_handlers( key_source: source_path.display().to_string(), source: hook_source_for_requirement_source(managed_hooks.source.as_ref()), is_managed: true, + include_in_hook_list: true, bypass_hook_trust: false, hook_states, env: HashMap::new(), @@ -216,7 +221,14 @@ fn append_plugin_hook_sources( source_path, source_relative_path, hooks, + kind, } = source; + let (hook_source, is_managed, include_in_hook_list) = match kind { + PluginHookSourceKind::UserReviewed => (HookSource::Plugin, false, true), + PluginHookSourceKind::AppBundledInternal => { + (HookSource::AppBundledInternal, true, false) + } + }; let mut env = HashMap::new(); let plugin_root_value = plugin_root.display().to_string(); let plugin_data_root_value = plugin_data_root.display().to_string(); @@ -238,8 +250,9 @@ fn append_plugin_hook_sources( plugin_id.as_str(), source_relative_path.as_str(), ), - source: HookSource::Plugin, - is_managed: false, + source: hook_source, + is_managed, + include_in_hook_list, bypass_hook_trust: policy.bypass_hook_trust, hook_states, env, @@ -495,28 +508,34 @@ fn append_matcher_groups( // TODO(abhinav): replace this positional suffix with a durable hook id. let key = crate::hook_key(&source.key_source, event_name, group_index, handler_index); - let state = source.hook_states.get(&key); + let state = if source.source == HookSource::AppBundledInternal { + None + } else { + source.hook_states.get(&key) + }; let enabled = hook_enabled(source.is_managed, state); let trusted_hash = hook_trusted_hash(source.is_managed, state); let trust_status = hook_trust_status(source.is_managed, ¤t_hash, trusted_hash); - hook_entries.push(HookListEntry { - key, - event_name, - handler_type: HookHandlerType::Command, - matcher: matcher.map(ToOwned::to_owned), - command: Some(command.clone()), - timeout_sec, - status_message: status_message.clone(), - source_path: source.path.clone(), - source: source.source, - plugin_id: source.plugin_id.clone(), - display_order: *display_order, - enabled, - is_managed: source.is_managed, - current_hash, - trust_status, - }); + if source.include_in_hook_list { + hook_entries.push(HookListEntry { + key, + event_name, + handler_type: HookHandlerType::Command, + matcher: matcher.map(ToOwned::to_owned), + command: Some(command.clone()), + timeout_sec, + status_message: status_message.clone(), + source_path: source.path.clone(), + source: source.source, + plugin_id: source.plugin_id.clone(), + display_order: *display_order, + enabled, + is_managed: source.is_managed, + current_hash, + trust_status, + }); + } if enabled && (source.bypass_hook_trust || matches!( @@ -682,6 +701,7 @@ mod tests { key_source: path.display().to_string(), source: hook_source(), is_managed: true, + include_in_hook_list: true, bypass_hook_trust: false, hook_states, env: std::collections::HashMap::new(), @@ -699,6 +719,7 @@ mod tests { key_source: path.display().to_string(), source: HookSource::User, is_managed: false, + include_in_hook_list: true, bypass_hook_trust, hook_states, env: std::collections::HashMap::new(), diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 4b7697b34f..c3d7e831f6 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -18,6 +18,7 @@ use codex_config::RequirementSource; use codex_config::Sourced; use codex_config::TomlValue; use codex_plugin::PluginHookSource; +use codex_plugin::PluginHookSourceKind; use codex_plugin::PluginId; use codex_protocol::ThreadId; use codex_protocol::protocol::HookOutputEntry; @@ -900,6 +901,7 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() { source_path, source_relative_path: "hooks/hooks.json".to_string(), hooks: pre_tool_use_hook_events("python3 /tmp/plugin-hook.py"), + kind: PluginHookSourceKind::UserReviewed, }]; let (requirements, requirements_toml) = requirements_with_managed_hooks_only( /*allow_managed_hooks_only*/ true, /*managed_hooks*/ None, @@ -923,6 +925,97 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() { assert!(engine.warnings().is_empty()); } +#[test] +fn app_bundled_internal_plugin_hooks_are_managed_and_hidden_from_listing() { + let temp = tempdir().expect("create temp dir"); + let plugin_root = + AbsolutePathBuf::try_from(temp.path().join("computer-use")).expect("plugin root"); + let plugin_data_root = + AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root"); + let source_path = plugin_root.join("hooks/hooks.json"); + let plugin_id = PluginId::parse("computer-use@openai-bundled").expect("plugin id"); + let disabled_key = "computer-use@openai-bundled:hooks/hooks.json:pre_tool_use:0:0"; + let plugin_hook_sources = vec![PluginHookSource { + plugin_id, + plugin_root, + plugin_data_root, + source_path, + source_relative_path: "hooks/hooks.json".to_string(), + hooks: pre_tool_use_hook_events("python3 /tmp/internal-plugin-hook.py"), + kind: PluginHookSourceKind::AppBundledInternal, + }]; + let (requirements, requirements_toml) = requirements_with_managed_hooks_only( + /*allow_managed_hooks_only*/ true, /*managed_hooks*/ None, + ); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: AbsolutePathBuf::try_from(temp.path().join("config.toml")) + .expect("absolute config path"), + profile: None, + }, + serde_json::from_value(serde_json::json!({ + "hooks": { + "state": { + (disabled_key): { + "enabled": false, + "trusted_hash": "sha256:not-the-internal-hook", + }, + }, + }, + })) + .expect("config TOML should deserialize"), + )], + requirements, + requirements_toml, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*bypass_hook_trust*/ false, + Some(&config_layer_stack), + plugin_hook_sources.clone(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().is_empty()); + assert_eq!(engine.handlers.len(), 1); + assert_eq!(engine.handlers[0].source, HookSource::AppBundledInternal); + let preview = engine.preview_pre_tool_use(&PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + subagent: None, + cwd: cwd(), + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + matcher_aliases: Vec::new(), + tool_use_id: "tool-1".to_string(), + tool_input: serde_json::json!({ "command": "echo hello" }), + }); + assert_eq!(preview.len(), 1); + assert_eq!(preview[0].source, HookSource::AppBundledInternal); + + let listed = crate::list_hooks(crate::HooksConfig { + legacy_notify_argv: None, + feature_enabled: true, + bypass_hook_trust: false, + config_layer_stack: Some(config_layer_stack), + plugin_hook_sources, + plugin_hook_load_warnings: Vec::new(), + shell_program: None, + shell_args: Vec::new(), + }); + assert_eq!(listed.hooks, Vec::new()); + assert_eq!(listed.warnings, Vec::::new()); +} + #[test] fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { let temp = tempdir().expect("create temp dir"); @@ -1177,6 +1270,7 @@ print(json.dumps({ }], ..Default::default() }, + kind: PluginHookSourceKind::UserReviewed, }]; let config_layer_stack = trusted_plugin_hook_stack( AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"), @@ -1296,6 +1390,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() { }], ..Default::default() }, + kind: PluginHookSourceKind::UserReviewed, }]; let config_layer_stack = trusted_plugin_hook_stack( AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"), diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 92a2ace21a..2fd4da5868 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -37,6 +37,14 @@ pub struct PluginHookSource { pub source_path: AbsolutePathBuf, pub source_relative_path: String, pub hooks: HookEventsToml, + pub kind: PluginHookSourceKind, +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] +pub enum PluginHookSourceKind { + #[default] + UserReviewed, + AppBundledInternal, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 91ef624fa7..8a14515093 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1400,6 +1400,7 @@ pub enum HookSource { CloudManagedConfig, LegacyManagedConfigFile, LegacyManagedConfigMdm, + AppBundledInternal, #[default] Unknown, } diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index bd6e25f7ca..71f6e9cbe8 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -801,6 +801,7 @@ fn config_source_label(source: HookSource) -> &'static str { HookSource::CloudManagedConfig => "Cloud-managed config", HookSource::LegacyManagedConfigFile => "Admin config", HookSource::LegacyManagedConfigMdm => "Admin config", + HookSource::AppBundledInternal => "App-bundled internal", HookSource::Unknown => "Unknown source", } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index b53ec0f83d..18225a8c78 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -625,6 +625,7 @@ where experimental_api: true, opt_out_notification_methods: Vec::new(), channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await .wrap_err("failed to start embedded app server")?; diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 0d8a1fa773..dbe877a4d7 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -1055,6 +1055,7 @@ mod tests { experimental_api: true, opt_out_notification_methods: Vec::new(), channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + app_bundled_internal_plugins: Vec::new(), }) .await .unwrap();