load app-bundled internal hooks

This commit is contained in:
Abhinav Vedmala
2026-06-15 05:35:51 +00:00
parent 9b2efe0c3a
commit 907bd32322
19 changed files with 780 additions and 78 deletions
+1
View File
@@ -2715,6 +2715,7 @@ dependencies = [
"codex-app-server-protocol",
"codex-config",
"codex-core-skills",
"codex-desktop-installation",
"codex-exec-server",
"codex-git-utils",
"codex-hooks",
+5 -4
View File
@@ -1101,13 +1101,13 @@ pub(crate) fn codex_hook_run_metadata(
thread_id: Some(tracking.thread_id.clone()),
turn_id: Some(tracking.turn_id.clone()),
model_slug: Some(tracking.model_slug.clone()),
hook_name: Some(analytics_hook_event_name(hook.event_name).to_owned()),
hook_source: Some(analytics_hook_source(hook.hook_source)),
hook_name: Some(hook_event_name(hook.event_name).to_owned()),
hook_source: Some(hook_source_name(hook.hook_source)),
status: Some(analytics_hook_status(hook.status)),
}
}
fn analytics_hook_event_name(event_name: HookEventName) -> &'static str {
pub fn hook_event_name(event_name: HookEventName) -> &'static str {
match event_name {
HookEventName::PreToolUse => "PreToolUse",
HookEventName::PermissionRequest => "PermissionRequest",
@@ -1122,7 +1122,7 @@ fn analytics_hook_event_name(event_name: HookEventName) -> &'static str {
}
}
fn analytics_hook_source(source: HookSource) -> &'static str {
pub fn hook_source_name(source: HookSource) -> &'static str {
match source {
HookSource::System => "system",
HookSource::User => "user",
@@ -1130,6 +1130,7 @@ fn analytics_hook_source(source: HookSource) -> &'static str {
HookSource::Mdm => "mdm",
HookSource::SessionFlags => "session_flags",
HookSource::Plugin => "plugin",
HookSource::AppBundledInternal => "app_bundled_internal",
HookSource::CloudRequirements => "cloud_requirements",
HookSource::CloudManagedConfig => "cloud_managed_config",
HookSource::LegacyManagedConfigFile => "legacy_managed_config_file",
+2
View File
@@ -21,6 +21,8 @@ pub use events::GuardianReviewSessionKind;
pub use events::GuardianReviewTerminalStatus;
pub use events::GuardianReviewTrackContext;
pub use events::GuardianReviewedAction;
pub use events::hook_event_name;
pub use events::hook_source_name;
pub use facts::AcceptedLineFingerprint;
pub use facts::AnalyticsJsonRpcError;
pub use facts::AppInvocation;
@@ -53,6 +53,9 @@ v2_enum_from_core!(
LegacyManagedConfigMdm,
Unknown,
}
aliases {
AppBundledInternal => Plugin,
}
);
v2_enum_from_core!(
@@ -24,6 +24,10 @@ macro_rules! v2_enum_from_core {
pub enum $Name:ident from $Src:path {
$( $(#[$variant_meta:meta])* $Variant:ident ),+ $(,)?
}
// Map private core-only variants onto existing public wire variants.
$(aliases {
$( $SourceVariant:ident => $TargetVariant:ident ),+ $(,)?
})?
) => {
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)]
$(#[$enum_meta])*
@@ -41,7 +45,10 @@ macro_rules! v2_enum_from_core {
impl From<$Src> for $Name {
fn from(value: $Src) -> Self {
match value { $( <$Src>::$Variant => $Name::$Variant ),+ }
match value {
$( <$Src>::$Variant => $Name::$Variant ),+,
$( $( <$Src>::$SourceVariant => $Name::$TargetVariant ),+ )?
}
}
}
};
+1
View File
@@ -18,6 +18,7 @@ codex-analytics = { workspace = true }
codex-app-server-protocol = { workspace = true }
codex-config = { workspace = true }
codex-core-skills = { workspace = true }
codex-desktop-installation = { workspace = true }
codex-exec-server = { workspace = true }
codex-git-utils = { workspace = true }
codex-hooks = { workspace = true }
@@ -0,0 +1,146 @@
use std::path::PathBuf;
use std::sync::OnceLock;
use codex_desktop_installation::DesktopResources;
use codex_desktop_installation::locate_current_or_installed_resources;
use codex_plugin::PluginHookSource;
use codex_plugin::PluginId;
use codex_protocol::protocol::HookSource;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde::Deserialize;
use tracing::warn;
use crate::OPENAI_BUNDLED_MARKETPLACE_NAME;
use crate::loader::load_plugin_hooks;
use crate::manifest::load_plugin_manifest;
const BUNDLED_MARKETPLACE_PATH: &str = "plugins/openai-bundled/.agents/plugins/marketplace.json";
// Standalone CLI discovery may invoke LaunchServices or PowerShell, so do it once per process.
static DESKTOP_RESOURCES: OnceLock<Result<DesktopResources, String>> = OnceLock::new();
pub(crate) fn is_app_bundled_plugin(plugin_id: &PluginId) -> bool {
plugin_id.marketplace_name == OPENAI_BUNDLED_MARKETPLACE_NAME
}
pub(crate) async fn load_app_bundled_internal_hooks(
plugin_id: &PluginId,
plugin_data_root: &AbsolutePathBuf,
) -> Vec<PluginHookSource> {
let worker_plugin_id = plugin_id.clone();
let plugin_data_root = plugin_data_root.clone();
let worker = tokio::task::spawn_blocking(move || {
let resources = DESKTOP_RESOURCES
.get_or_init(|| {
locate_current_or_installed_resources().map_err(|error| error.to_string())
})
.as_ref()
.map_err(Clone::clone)?;
load_app_bundled_internal_hooks_from_resources(
resources,
&worker_plugin_id,
&plugin_data_root,
)
})
.await;
let result = match worker {
Ok(result) => result,
Err(error) => Err(format!("Desktop discovery worker failed: {error}")),
};
match result {
Ok(sources) => sources,
Err(error) => {
warn!(
diagnostic_code = "app_bundled_internal_hook_load_failed",
plugin_id = %plugin_id.as_key(),
error,
"app-bundled internal hooks failed closed"
);
Vec::new()
}
}
}
/// Loads this plugin's hooks only from the located Desktop resources root.
pub(crate) fn load_app_bundled_internal_hooks_from_resources(
resources: &DesktopResources,
plugin_id: &PluginId,
plugin_data_root: &AbsolutePathBuf,
) -> Result<Vec<PluginHookSource>, String> {
if !is_app_bundled_plugin(plugin_id) {
return Err("plugin is not from the app-bundled marketplace".to_string());
}
let marketplace_path = resources
.contained_file(BUNDLED_MARKETPLACE_PATH)
.map_err(|error| format!("invalid bundled marketplace: {error}"))?;
let contents = std::fs::read_to_string(marketplace_path.as_path())
.map_err(|error| format!("failed to read bundled marketplace: {error}"))?;
let marketplace: BundledMarketplace = serde_json::from_str(&contents)
.map_err(|error| format!("failed to parse bundled marketplace: {error}"))?;
if marketplace.name != OPENAI_BUNDLED_MARKETPLACE_NAME {
return Err("bundled marketplace has the wrong name".to_string());
}
let mut matching_plugins = marketplace
.plugins
.into_iter()
.filter(|plugin| plugin.name == plugin_id.plugin_name);
let plugin = matching_plugins
.next()
.ok_or_else(|| "plugin is missing from the bundled marketplace".to_string())?;
if matching_plugins.next().is_some() {
return Err("bundled marketplace contains duplicate plugin entries".to_string());
}
let expected_source = format!("./plugins/{}", plugin_id.plugin_name);
if plugin.source.source != "local" || plugin.source.path != expected_source {
return Err("bundled plugin has an unexpected source path".to_string());
}
let plugin_relative = PathBuf::from("plugins")
.join(OPENAI_BUNDLED_MARKETPLACE_NAME)
.join("plugins")
.join(&plugin_id.plugin_name);
let plugin_root = resources
.contained_directory(plugin_relative)
.map_err(|error| format!("invalid bundled plugin root: {error}"))?;
let manifest = load_plugin_manifest(plugin_root.as_path())
.ok_or_else(|| "bundled plugin manifest is missing or invalid".to_string())?;
if manifest.name != plugin_id.plugin_name {
return Err("bundled plugin manifest has the wrong name".to_string());
}
let (mut sources, warnings) =
load_plugin_hooks(&plugin_root, plugin_id, plugin_data_root, &manifest.paths);
if !warnings.is_empty() {
return Err(format!(
"failed to load bundled plugin hooks: {}",
warnings.join("; ")
));
}
for source in &mut sources {
source.source = HookSource::AppBundledInternal;
}
Ok(sources)
}
#[derive(Deserialize)]
struct BundledMarketplace {
name: String,
plugins: Vec<BundledMarketplacePlugin>,
}
#[derive(Deserialize)]
struct BundledMarketplacePlugin {
name: String,
source: BundledMarketplacePluginSource,
}
#[derive(Deserialize)]
struct BundledMarketplacePluginSource {
source: String,
path: String,
}
#[cfg(test)]
#[path = "app_bundled_internal_tests.rs"]
mod tests;
@@ -0,0 +1,103 @@
use std::fs;
use std::path::Path;
use std::path::PathBuf;
use codex_desktop_installation::DesktopResources;
use codex_plugin::PluginId;
use codex_utils_absolute_path::AbsolutePathBuf;
use serde_json::json;
use tempfile::TempDir;
use super::*;
struct Fixture {
_temp: TempDir,
marketplace_path: PathBuf,
resources: DesktopResources,
plugin_data_root: AbsolutePathBuf,
}
fn fixture() -> Fixture {
let temp = tempfile::tempdir().expect("temp dir");
let resources_root = temp.path().join("resources");
let marketplace_path = resources_root.join(BUNDLED_MARKETPLACE_PATH);
write(
&marketplace_path,
&json!({
"name": "openai-bundled",
"plugins": [{
"name": "computer-use",
"source": {"source": "local", "path": "./plugins/computer-use"}
}]
})
.to_string(),
);
let plugin_data_root = temp.path().join("data/computer-use");
fs::create_dir_all(&plugin_data_root).expect("plugin data root");
Fixture {
resources: DesktopResources::from_trusted_path(resources_root).expect("Desktop resources"),
plugin_data_root: AbsolutePathBuf::try_from(plugin_data_root)
.expect("absolute plugin data root"),
marketplace_path,
_temp: temp,
}
}
fn write(path: &Path, contents: &str) {
fs::create_dir_all(path.parent().expect("file parent")).expect("create parent");
fs::write(path, contents).expect("write fixture");
}
fn replace_marketplace(fixture: &Fixture, marketplace: serde_json::Value) {
write(&fixture.marketplace_path, &marketplace.to_string());
}
fn load(fixture: &Fixture) -> Result<Vec<PluginHookSource>, String> {
let plugin_id = PluginId::parse("computer-use@openai-bundled").expect("plugin id");
load_app_bundled_internal_hooks_from_resources(
&fixture.resources,
&plugin_id,
&fixture.plugin_data_root,
)
}
#[test]
fn marketplace_identity_and_plugin_source_must_match() {
let cases = [
(
"wrong marketplace",
json!({"name": "spoofed", "plugins": []}),
),
(
"wrong source",
json!({
"name": "openai-bundled",
"plugins": [{
"name": "computer-use",
"source": {"source": "local", "path": "./plugins/other"}
}]
}),
),
];
for (label, marketplace) in cases {
let fixture = fixture();
replace_marketplace(&fixture, marketplace);
load(&fixture).expect_err(label);
}
}
#[test]
fn non_bundled_marketplace_cannot_request_internal_hook_loading() {
let fixture = fixture();
let plugin_id = PluginId::parse("computer-use@spoofed").expect("plugin id");
load_app_bundled_internal_hooks_from_resources(
&fixture.resources,
&plugin_id,
&fixture.plugin_data_root,
)
.expect_err("wrong marketplace must be rejected");
}
+1
View File
@@ -1,3 +1,4 @@
mod app_bundled_internal;
mod discoverable;
pub mod installed_marketplaces;
pub mod loader;
+20 -6
View File
@@ -1,4 +1,6 @@
use crate::OPENAI_CURATED_MARKETPLACE_NAME;
use crate::app_bundled_internal::is_app_bundled_plugin;
use crate::app_bundled_internal::load_app_bundled_internal_hooks;
use crate::manifest::PluginManifestHooks;
use crate::manifest::PluginManifestPaths;
use crate::manifest::load_plugin_manifest;
@@ -34,6 +36,7 @@ use codex_plugin::PluginIdError;
use codex_plugin::PluginLoadOutcome;
use codex_plugin::PluginTelemetryMetadata;
use codex_plugin::app_connector_ids_from_declarations;
use codex_protocol::protocol::HookSource;
use codex_protocol::protocol::Product;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -706,12 +709,21 @@ async fn load_plugin(
}
PluginLoadScope::HooksOnly => {}
}
let (hook_sources, hook_load_warnings) = load_plugin_hooks(
&plugin_root,
&loaded_plugin_id,
&store.plugin_data_root(&loaded_plugin_id),
manifest_paths,
);
let plugin_data_root = store.plugin_data_root(&loaded_plugin_id);
// Hooks load from Desktop resources; runtime-variant plugin capabilities remain cache-backed.
let (hook_sources, hook_load_warnings) = if is_app_bundled_plugin(&loaded_plugin_id) {
(
load_app_bundled_internal_hooks(&loaded_plugin_id, &plugin_data_root).await,
Vec::new(),
)
} else {
load_plugin_hooks(
&plugin_root,
&loaded_plugin_id,
&plugin_data_root,
manifest_paths,
)
};
loaded_plugin.hook_sources = hook_sources;
loaded_plugin.hook_load_warnings = hook_load_warnings;
loaded_plugin
@@ -914,6 +926,7 @@ pub fn load_plugin_hooks(
source_path: manifest_path.clone(),
source_relative_path: format!("plugin.json#hooks[{index}]"),
hooks: hooks_file.hooks.clone(),
source: HookSource::Plugin,
});
}
}
@@ -982,6 +995,7 @@ fn append_plugin_hook_file(
source_path: path.clone(),
source_relative_path,
hooks: parsed.hooks,
source: HookSource::Plugin,
});
}
+15 -10
View File
@@ -1,5 +1,6 @@
use super::PluginLoadOutcome;
use crate::OPENAI_CURATED_MARKETPLACE_NAME;
use crate::app_bundled_internal::is_app_bundled_plugin;
use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack;
use crate::loader::PluginHookLoadOutcome;
use crate::loader::configured_curated_plugin_ids_from_codex_home;
@@ -1309,16 +1310,20 @@ 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 is_app_bundled_plugin(&plugin_id) {
Vec::new()
} else {
let plugin_data_root = self.store.plugin_data_root(&plugin_id);
let (sources, _warnings) =
load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths);
plugin_hook_declarations(&sources)
.into_iter()
.map(|hook| PluginHookSummary {
key: hook.key,
event_name: hook.event_name,
})
.collect()
};
let app_declarations = load_plugin_apps(source_path.as_path()).await;
let apps = app_connector_ids_from_declarations(&app_declarations);
let mut seen_app_connector_ids = HashSet::new();
+65 -28
View File
@@ -5,6 +5,8 @@ use std::time::Duration;
use codex_analytics::CompactionTrigger;
use codex_analytics::HookRunFact;
use codex_analytics::build_track_events_context;
use codex_analytics::hook_event_name;
use codex_analytics::hook_source_name;
use codex_hooks::PermissionRequestDecision;
use codex_hooks::PermissionRequestOutcome;
use codex_hooks::PermissionRequestRequest;
@@ -27,7 +29,6 @@ use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::HookCompletedEvent;
use codex_protocol::protocol::HookEventName;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookRunSummary;
use codex_protocol::protocol::HookSource;
@@ -613,12 +614,19 @@ fn additional_context_messages(additional_contexts: Vec<String>) -> Vec<Response
.collect()
}
/// Emits lifecycle-start events for user-visible hooks.
///
/// App-bundled internal hooks still run, but their lifecycle is intentionally omitted from the
/// public event stream because they are a Desktop implementation detail.
async fn emit_hook_started_events(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
preview_runs: Vec<HookRunSummary>,
) {
for run in preview_runs {
if run.source == HookSource::AppBundledInternal {
continue;
}
sess.send_event(
turn_context,
EventMsg::HookStarted(HookStartedEvent {
@@ -630,6 +638,11 @@ async fn emit_hook_started_events(
}
}
/// Records metrics and analytics for every completed hook, while emitting lifecycle-completion
/// events only for user-visible hooks.
///
/// App-bundled internal hook notifications are intentionally suppressed because they are a
/// Desktop implementation detail.
pub(crate) async fn emit_hook_completed_events(
sess: &Arc<Session>,
turn_context: &Arc<TurnContext>,
@@ -638,6 +651,9 @@ 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 {
continue;
}
sess.send_event(turn_context, EventMsg::HookCompleted(completed))
.await;
}
@@ -694,31 +710,6 @@ fn hook_run_analytics_payload(
}
fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str); 3] {
let hook_name = match run.event_name {
HookEventName::PreToolUse => "PreToolUse",
HookEventName::PermissionRequest => "PermissionRequest",
HookEventName::PostToolUse => "PostToolUse",
HookEventName::PreCompact => "PreCompact",
HookEventName::PostCompact => "PostCompact",
HookEventName::SessionStart => "SessionStart",
HookEventName::UserPromptSubmit => "UserPromptSubmit",
HookEventName::SubagentStart => "SubagentStart",
HookEventName::SubagentStop => "SubagentStop",
HookEventName::Stop => "Stop",
};
let hook_source = match run.source {
HookSource::System => "system",
HookSource::User => "user",
HookSource::Project => "project",
HookSource::Mdm => "mdm",
HookSource::SessionFlags => "session_flags",
HookSource::Plugin => "plugin",
HookSource::CloudRequirements => "cloud_requirements",
HookSource::CloudManagedConfig => "cloud_managed_config",
HookSource::LegacyManagedConfigFile => "legacy_managed_config_file",
HookSource::LegacyManagedConfigMdm => "legacy_managed_config_mdm",
HookSource::Unknown => "unknown",
};
let status = match run.status {
HookRunStatus::Running => "running",
HookRunStatus::Completed => "completed",
@@ -728,8 +719,8 @@ fn hook_run_metric_tags(run: &HookRunSummary) -> [(&'static str, &'static str);
};
[
("hook_name", hook_name),
("source", hook_source),
("hook_name", hook_event_name(run.event_name)),
("source", hook_source_name(run.source)),
("status", status),
]
}
@@ -785,9 +776,13 @@ 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::EventMsg;
use codex_protocol::protocol::HookCompletedEvent;
use codex_protocol::protocol::HookRunSummary;
use codex_utils_absolute_path::test_support::PathBufExt;
@@ -862,6 +857,38 @@ mod tests {
assert_eq!(hook.status, HookRunStatus::Failed);
}
#[tokio::test]
async fn user_visible_hook_notifications_are_emitted() {
let (session, turn_context, rx_event) = make_session_and_context_with_rx().await;
while rx_event.try_recv().is_ok() {}
let regular = sample_hook_run(HookRunStatus::Completed, HookSource::Plugin);
emit_hook_started_events(&session, &turn_context, vec![regular.clone()]).await;
emit_hook_completed_events(
&session,
&turn_context,
vec![HookCompletedEvent {
turn_id: Some(turn_context.sub_id.clone()),
run: regular,
}],
)
.await;
assert!(matches!(
rx_event.try_recv().expect("regular hook started event"),
codex_protocol::protocol::Event {
msg: EventMsg::HookStarted(_),
..
}
));
assert!(matches!(
rx_event.try_recv().expect("regular hook completed event"),
codex_protocol::protocol::Event {
msg: EventMsg::HookCompleted(_),
..
}
));
}
#[test]
fn hook_run_metric_tags_match_analytics_shape() {
let run = sample_hook_run(HookRunStatus::Blocked, HookSource::Project);
@@ -900,6 +927,16 @@ 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 {
+231
View File
@@ -1,3 +1,4 @@
use std::ffi::OsString;
use std::fs;
use std::path::Path;
@@ -17,6 +18,7 @@ use codex_protocol::models::ResponseItem;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::HookSource;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
@@ -46,6 +48,7 @@ use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serial_test::serial;
use std::sync::Arc;
use std::time::Duration;
use tempfile::TempDir;
@@ -58,6 +61,33 @@ const SECOND_CONTINUATION_PROMPT: &str = "Now tighten it to just: meow.";
const BLOCKED_PROMPT_CONTEXT: &str = "Remember the blocked lighthouse note.";
const PERMISSION_REQUEST_HOOK_MATCHER: &str = "^Bash$";
const PERMISSION_REQUEST_ALLOW_REASON: &str = "should not be used for allow";
const DESKTOP_RESOURCES_PATH_ENV_VAR: &str = "CODEX_DESKTOP_RESOURCES_PATH";
struct EnvVarGuard {
key: &'static str,
original: Option<OsString>,
}
impl EnvVarGuard {
fn set(key: &'static str, value: &std::ffi::OsStr) -> Self {
let original = std::env::var_os(key);
unsafe {
std::env::set_var(key, value);
}
Self { key, original }
}
}
impl Drop for EnvVarGuard {
fn drop(&mut self) {
unsafe {
match &self.original {
Some(value) => std::env::set_var(self.key, value),
None => std::env::remove_var(self.key),
}
}
}
}
fn restrictive_workspace_write_profile() -> PermissionProfile {
PermissionProfile::workspace_write_with(
@@ -2814,6 +2844,206 @@ text(output.output);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial(desktop_resources)]
async fn app_bundled_internal_pre_tool_use_runs_with_hooks_disabled_without_lifecycle_events()
-> Result<()> {
skip_if_no_network!(Ok(()));
let resources = TempDir::new()?;
let resources_root = resources.path();
let marketplace_root = resources_root.join("plugins/openai-bundled");
let plugin_root = marketplace_root.join("plugins/computer-use");
let hooks_dir = plugin_root.join("hooks");
let hook_log_path = resources_root.join("internal_hook_log.jsonl");
fs::create_dir_all(marketplace_root.join(".agents/plugins"))
.context("create bundled marketplace metadata directory")?;
fs::create_dir_all(plugin_root.join(".codex-plugin"))
.context("create bundled plugin manifest directory")?;
fs::create_dir_all(&hooks_dir).context("create bundled plugin hooks directory")?;
fs::write(
marketplace_root.join(".agents/plugins/marketplace.json"),
serde_json::json!({
"name": "openai-bundled",
"plugins": [{
"name": "computer-use",
"source": {"source": "local", "path": "./plugins/computer-use"}
}]
})
.to_string(),
)
.context("write bundled marketplace metadata")?;
fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"computer-use"}"#,
)
.context("write bundled plugin manifest")?;
fs::write(
hooks_dir.join("pre_tool_use_hook.py"),
format!(
r#"import json
from pathlib import Path
import sys
payload = json.load(sys.stdin)
with Path(r"{hook_log_path}").open("a", encoding="utf-8") as handle:
handle.write(json.dumps(payload) + "\n")
print(json.dumps({{
"hookSpecificOutput": {{
"hookEventName": "PreToolUse",
"permissionDecision": "deny",
"permissionDecisionReason": "blocked by app-bundled internal hook"
}}
}}))
"#,
hook_log_path = hook_log_path.display(),
),
)
.context("write bundled pre tool use hook script")?;
fs::write(
hooks_dir.join("hooks.json"),
r#"{
"hooks": {
"PreToolUse": [{
"matcher": "^Bash$",
"hooks": [{
"type": "command",
"command": "python3 ${PLUGIN_ROOT}/hooks/pre_tool_use_hook.py"
}]
}]
}
}"#,
)
.context("write bundled hooks config")?;
let _desktop_resources =
EnvVarGuard::set(DESKTOP_RESOURCES_PATH_ENV_VAR, resources_root.as_os_str());
let server = start_mock_server().await;
let call_id = "app-bundled-internal-pretooluse-shell-command";
let marker = resources_root.join("blocked-command-marker");
let command = format!("printf should-not-run > {}", marker.display());
let args = serde_json::json!({ "command": command });
let responses = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "internal hook blocked it"),
ev_completed("resp-2"),
]),
],
)
.await;
let home = Arc::new(TempDir::new()?);
let cached_plugin_root = home
.path()
.join("plugins/cache/openai-bundled/computer-use/local");
fs::create_dir_all(cached_plugin_root.join(".codex-plugin"))
.context("create cached plugin manifest directory")?;
fs::write(
cached_plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"computer-use"}"#,
)
.context("write cached plugin manifest")?;
fs::write(
home.path().join("config.toml"),
r#"[plugins."computer-use@openai-bundled"]
enabled = true
"#,
)
.context("write app-bundled plugin config")?;
let mut builder = test_codex()
.with_home(Arc::clone(&home))
.with_config(|config| {
config
.features
.enable(Feature::Plugins)
.expect("test config should allow feature update");
config
.features
.disable(Feature::CodexHooks)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "run the shell command blocked by an internal hook".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
approval_policy: Some(AskForApproval::Never),
sandbox_policy: Some(codex_protocol::protocol::SandboxPolicy::DangerFullAccess),
..Default::default()
},
})
.await?;
let mut turn_id = None;
let mut saw_hook_started = false;
let mut saw_hook_completed = false;
timeout(Duration::from_secs(10), async {
loop {
let event = test
.codex
.next_event()
.await
.expect("event stream should remain open");
match event.msg {
EventMsg::HookStarted(_) => saw_hook_started = true,
EventMsg::HookCompleted(_) => saw_hook_completed = true,
EventMsg::TurnStarted(event) => turn_id = Some(event.turn_id),
EventMsg::TurnComplete(event)
if turn_id.as_deref() == Some(event.turn_id.as_str()) =>
{
break;
}
_ => {}
}
}
})
.await
.context("wait for app-bundled internal hook turn")?;
assert!(!saw_hook_started, "internal hook start should stay hidden");
assert!(
!saw_hook_completed,
"internal hook completion should stay hidden"
);
assert!(!marker.exists(), "internal hook should block execution");
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("shell command output string");
assert!(
output.contains("Command blocked by PreToolUse hook: blocked by app-bundled internal hook"),
"blocked tool output should surface the internal hook reason",
);
let hook_inputs = read_hook_inputs_from_log(&hook_log_path)?;
assert_eq!(hook_inputs.len(), 1);
assert_eq!(hook_inputs[0]["hook_event_name"], "PreToolUse");
assert_eq!(hook_inputs[0]["tool_name"], "Bash");
assert_eq!(hook_inputs[0]["tool_use_id"], call_id);
assert_eq!(hook_inputs[0]["tool_input"]["command"], command);
Ok(())
}
#[tokio::test]
async fn plugin_pre_tool_use_blocks_shell_command_before_execution() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -2916,6 +3146,7 @@ print(json.dumps({{
hooks: serde_json::from_str::<codex_config::HooksFile>(plugin_hooks_json)
.context("parse plugin hooks")?
.hooks,
source: HookSource::Plugin,
}];
let mut builder = test_codex()
+4 -2
View File
@@ -52,7 +52,7 @@ mod tests {
fn lists_declared_plugin_handlers_with_persisted_hook_keys() {
let plugin_root = test_path_buf("/tmp/plugin").abs();
let source_path = plugin_root.join("hooks/hooks.json");
let declarations = plugin_hook_declarations(&[PluginHookSource {
let source = PluginHookSource {
plugin_id: PluginId::parse("demo@test").expect("plugin id"),
plugin_root: plugin_root.clone(),
plugin_data_root: plugin_root.join("data"),
@@ -78,7 +78,9 @@ mod tests {
}],
..Default::default()
},
}]);
source: codex_protocol::protocol::HookSource::Plugin,
};
let declarations = plugin_hook_declarations(std::slice::from_ref(&source));
assert_eq!(
declarations,
+40 -24
View File
@@ -56,7 +56,9 @@ struct HookDiscoveryPolicy {
impl HookDiscoveryPolicy {
fn allows(self, source: &HookHandlerSource<'_>) -> bool {
!self.allow_managed_hooks_only || source.is_managed
!self.allow_managed_hooks_only
|| source.is_managed
|| source.source == HookSource::AppBundledInternal
}
}
@@ -223,7 +225,14 @@ fn append_plugin_hook_sources(
source_path,
source_relative_path,
hooks,
source,
} = source;
if !matches!(source, HookSource::Plugin | HookSource::AppBundledInternal) {
warnings.push(format!(
"skipping plugin hook source with invalid source {source:?}"
));
continue;
}
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();
@@ -245,7 +254,7 @@ fn append_plugin_hook_sources(
plugin_id.as_str(),
source_relative_path.as_str(),
),
source: HookSource::Plugin,
source,
is_managed: false,
bypass_hook_trust: policy.bypass_hook_trust,
hook_states,
@@ -502,30 +511,37 @@ 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 enabled = hook_enabled(source.is_managed, state);
let is_forced = source.source == HookSource::AppBundledInternal;
let state = (!is_forced).then(|| source.hook_states.get(&key)).flatten();
let enabled = is_forced || 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, &current_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,
});
let trust_status = if is_forced {
HookTrustStatus::Trusted
} else {
hook_trust_status(source.is_managed, &current_hash, trusted_hash)
};
if !is_forced {
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
&& (is_forced
|| source.bypass_hook_trust
|| matches!(
trust_status,
HookTrustStatus::Managed | HookTrustStatus::Trusted
+7 -3
View File
@@ -109,11 +109,15 @@ impl ClaudeHooksEngine {
enabled: bool,
bypass_hook_trust: bool,
config_layer_stack: Option<&ConfigLayerStack>,
plugin_hook_sources: Vec<PluginHookSource>,
plugin_hook_load_warnings: Vec<String>,
mut plugin_hook_sources: Vec<PluginHookSource>,
mut plugin_hook_load_warnings: Vec<String>,
shell: CommandShell,
) -> Self {
if !enabled {
plugin_hook_sources.retain(|source| source.source == HookSource::AppBundledInternal);
plugin_hook_load_warnings.clear();
}
if !enabled && plugin_hook_sources.is_empty() {
return Self {
handlers: Vec::new(),
warnings: Vec::new(),
@@ -124,7 +128,7 @@ impl ClaudeHooksEngine {
let _ = schema_loader::generated_hook_schemas();
let discovered = discovery::discover_handlers(
config_layer_stack,
config_layer_stack.filter(|_| enabled),
plugin_hook_sources,
plugin_hook_load_warnings,
bypass_hook_trust,
+125
View File
@@ -20,11 +20,13 @@ use codex_config::TomlValue;
use codex_plugin::PluginHookSource;
use codex_plugin::PluginId;
use codex_protocol::ThreadId;
use codex_protocol::protocol::HookEventName;
use codex_protocol::protocol::HookOutputEntry;
use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookSource;
use codex_protocol::protocol::HookTrustStatus;
use codex_utils_absolute_path::test_support::PathBufExt;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
@@ -900,6 +902,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"),
source: HookSource::Plugin,
}];
let (requirements, requirements_toml) = requirements_with_managed_hooks_only(
/*allow_managed_hooks_only*/ true, /*managed_hooks*/ None,
@@ -923,6 +926,126 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() {
assert!(engine.warnings().is_empty());
}
fn build_engine(
enabled: bool,
config_layer_stack: Option<&ConfigLayerStack>,
plugin_hook_sources: Vec<PluginHookSource>,
) -> ClaudeHooksEngine {
ClaudeHooksEngine::new(
enabled,
/*bypass_hook_trust*/ false,
config_layer_stack,
plugin_hook_sources,
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
},
)
}
fn plugin_hook_source(root: &Path, source: HookSource) -> PluginHookSource {
let plugin_root = AbsolutePathBuf::try_from(root.join("demo-plugin")).expect("plugin root");
let plugin_data_root =
AbsolutePathBuf::try_from(root.join("plugin-data")).expect("plugin data root");
let source_path = plugin_root.join("hooks/hooks.json");
PluginHookSource {
plugin_id: PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"),
plugin_root,
plugin_data_root,
source_path,
source_relative_path: "hooks/hooks.json".to_string(),
hooks: pre_tool_use_hook_events(r#""${PLUGIN_ROOT}/internal-hook""#),
source,
}
}
#[test]
fn plugin_hook_source_rejects_non_plugin_provenance() {
let temp = tempdir().expect("create temp dir");
let invalid_engine = build_engine(
/*enabled*/ true,
/*config_layer_stack*/ None,
vec![plugin_hook_source(temp.path(), HookSource::User)],
);
assert!(invalid_engine.handlers.is_empty());
assert_eq!(
invalid_engine.warnings(),
&["skipping plugin hook source with invalid source User".to_string()]
);
}
#[test]
fn app_bundled_internal_hooks_ignore_user_controls_and_are_hidden() {
let temp = tempdir().expect("create temp dir");
let plugin_hook_sources = vec![plugin_hook_source(
temp.path(),
HookSource::AppBundledInternal,
)];
let key_source = crate::declarations::plugin_hook_key_source(
"demo-plugin@test-marketplace",
"hooks/hooks.json",
);
let hook_key = crate::hook_key(
&key_source,
HookEventName::PreToolUse,
/*group_index*/ 0,
/*handler_index*/ 0,
);
let state = [(
hook_key,
serde_json::json!({ "enabled": false, "trusted_hash": "stale" }),
)]
.into_iter()
.collect::<serde_json::Map<_, _>>();
let config = serde_json::from_value(serde_json::json!({
"hooks": { "state": state },
}))
.expect("config TOML should deserialize");
let disabled_stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::User {
file: temp.path().join("config.toml").abs(),
profile: None,
},
config,
)],
ConfigRequirements::default(),
ConfigRequirementsToml::default(),
)
.expect("config stack");
let disabled_state_engine = build_engine(
/*enabled*/ true,
Some(&disabled_stack),
plugin_hook_sources.clone(),
);
assert_eq!(disabled_state_engine.handlers.len(), 1);
let (requirements, requirements_toml) = requirements_with_managed_hooks_only(
/*allow_managed_hooks_only*/ true, /*managed_hooks*/ None,
);
let managed_only_stack =
ConfigLayerStack::new(Vec::new(), requirements, requirements_toml).expect("config stack");
let managed_only_engine = build_engine(
/*enabled*/ true,
Some(&managed_only_stack),
plugin_hook_sources.clone(),
);
assert_eq!(managed_only_engine.handlers.len(), 1);
let listed = crate::list_hooks(crate::HooksConfig {
legacy_notify_argv: None,
feature_enabled: false,
bypass_hook_trust: false,
config_layer_stack: None,
plugin_hook_sources,
plugin_hook_load_warnings: Vec::new(),
shell_program: None,
shell_args: Vec::new(),
});
assert!(listed.hooks.is_empty());
}
#[test]
fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() {
let temp = tempdir().expect("create temp dir");
@@ -1324,6 +1447,7 @@ print(json.dumps({
}],
..Default::default()
},
source: HookSource::Plugin,
}];
let config_layer_stack = trusted_plugin_hook_stack(
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"),
@@ -1443,6 +1567,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() {
}],
..Default::default()
},
source: HookSource::Plugin,
}];
let config_layer_stack = trusted_plugin_hook_stack(
AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"),
+2
View File
@@ -11,6 +11,7 @@ mod plugin_id;
mod provider;
use codex_config::HookEventsToml;
use codex_protocol::protocol::HookSource;
use codex_utils_absolute_path::AbsolutePathBuf;
pub use load_outcome::EffectiveSkillRoots;
pub use load_outcome::LoadedPlugin;
@@ -66,6 +67,7 @@ pub struct PluginHookSource {
pub source_path: AbsolutePathBuf,
pub source_relative_path: String,
pub hooks: HookEventsToml,
pub source: HookSource,
}
#[derive(Debug, Clone, PartialEq, Eq)]
+1
View File
@@ -1429,6 +1429,7 @@ pub enum HookSource {
Mdm,
SessionFlags,
Plugin,
AppBundledInternal,
CloudRequirements,
CloudManagedConfig,
LegacyManagedConfigFile,