clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-18 18:22:39 +00:00
Code Issues Packages Projects Releases Wiki Activity

codex: harden dynamic hook naming

Browse Source
This commit is contained in:
Andrei Eternal
2026-04-30 22:13:56 -07:00
parent c98b0d1925
commit 060e2fdcab
4 changed files with 125 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
codex-rs/core/src/tools/handlers/dynamic.rs
View File

@@ -33,7 +33,7 @@ impl ToolHandler for DynamicToolHandler {
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
Some(PreToolUsePayload {
tool_name: HookToolName::for_function_tool(&invocation.tool_name),
tool_name: HookToolName::for_dynamic_tool(&invocation.tool_name),
tool_input: dynamic_tool_input(invocation).ok()?,
})
}
@@ -44,7 +44,7 @@ impl ToolHandler for DynamicToolHandler {
result: &Self::Output,
) -> Option<PostToolUsePayload> {
Some(PostToolUsePayload {
tool_name: HookToolName::for_function_tool(&invocation.tool_name),
tool_name: HookToolName::for_dynamic_tool(&invocation.tool_name),
tool_use_id: invocation.call_id.clone(),
tool_input: dynamic_tool_input(invocation).ok()?,
tool_response: result

10
codex-rs/core/src/tools/handlers/mcp.rs
View File

@@ -29,7 +29,7 @@ impl ToolHandler for McpHandler {
};
Some(PreToolUsePayload {
tool_name: HookToolName::for_function_tool(&invocation.tool_name),
tool_name: HookToolName::for_mcp_tool(&invocation.tool_name),
tool_input: mcp_hook_tool_input(raw_arguments),
})
}
@@ -46,7 +46,7 @@ impl ToolHandler for McpHandler {
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::for_function_tool(&invocation.tool_name),
tool_name: HookToolName::for_mcp_tool(&invocation.tool_name),
tool_use_id: invocation.call_id.clone(),
tool_input: result.tool_input.clone(),
tool_response,
@@ -78,7 +78,7 @@ impl ToolHandler for McpHandler {
let (server, tool, raw_arguments) = payload;
let arguments_str = raw_arguments;
let hook_tool_name = HookToolName::for_function_tool(&model_tool_name);
let hook_tool_name = HookToolName::for_mcp_tool(&model_tool_name);
let started = Instant::now();
let result = handle_mcp_tool_call(
@@ -148,7 +148,7 @@ mod tests {
payload,
}),
Some(PreToolUsePayload {
tool_name: HookToolName::for_function_tool(&codex_tools::ToolName::namespaced(
tool_name: HookToolName::for_mcp_tool(&codex_tools::ToolName::namespaced(
"mcp__memory__",
"create_entities",
)),
@@ -202,7 +202,7 @@ mod tests {
assert_eq!(
McpHandler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_name: HookToolName::for_function_tool(&codex_tools::ToolName::namespaced(
tool_name: HookToolName::for_mcp_tool(&codex_tools::ToolName::namespaced(
"mcp__filesystem__",
"read_file",
)),

111
codex-rs/core/src/tools/hook_names.rs
View File

@@ -27,21 +27,34 @@ impl HookToolName {
}
}
/// Builds the canonical hook-facing identity for function-style tools that
/// do not need Claude-compatibility aliases.
/// Builds the canonical hook-facing identity for dynamic tools.
///
/// MCP tool names already use a stable fully qualified `mcp__...__tool`
/// form, so we preserve them verbatim. Other namespaced tools use the
/// `dynamic__namespace__tool` form so hooks can wildcard an entire dynamic
/// namespace without colliding with plain tool names.
pub(crate) fn for_function_tool(tool_name: &ToolName) -> Self {
/// Plain dynamic tools keep their original name. Namespaced dynamic tools
/// use `dynamic__namespace__tool` so hooks can target the dynamic-tool
/// surface without colliding with plain Codex tools or MCP names.
///
/// Each segment is escaped independently to keep the flattened form
/// unambiguous even when namespaces or tool names contain separator-like
/// substrings or punctuation.
pub(crate) fn for_dynamic_tool(tool_name: &ToolName) -> Self {
match tool_name.namespace.as_deref() {
Some(namespace) if namespace.starts_with("mcp__") => Self::new(tool_name.display()),
Some(namespace) => Self::new(format!("dynamic__{namespace}__{}", tool_name.name)),
Some(namespace) => Self::new(format!(
"dynamic__{}__{}",
encode_dynamic_segment(namespace),
encode_dynamic_segment(&tool_name.name),
)),
None => Self::new(tool_name.name.clone()),
}
}
/// Builds the canonical hook-facing identity for MCP tools.
///
/// MCP tool names already use the stable fully qualified
/// `mcp__server__tool` form, so we preserve them verbatim.
pub(crate) fn for_mcp_tool(tool_name: &ToolName) -> Self {
Self::new(tool_name.display())
}
/// Returns the hook identity for file edits performed through `apply_patch`.
///
/// The serialized name remains `apply_patch` so logs and policies can key
@@ -71,6 +84,38 @@ impl HookToolName {
}
}
fn encode_dynamic_segment(segment: &str) -> String {
let bytes = segment.as_bytes();
let mut encoded = String::with_capacity(segment.len());
for (index, byte) in bytes.iter().copied().enumerate() {
if should_preserve_dynamic_byte(bytes, index, byte) {
encoded.push(char::from(byte));
} else {
encoded.push('Z');
encoded.push(char::from(HEX_DIGITS[usize::from(byte >> 4)]));
encoded.push(char::from(HEX_DIGITS[usize::from(byte & 0x0F)]));
}
}
encoded
}
fn should_preserve_dynamic_byte(bytes: &[u8], index: usize, byte: u8) -> bool {
match byte {
b'a'..=b'z' | b'A'..=b'Y' | b'0'..=b'9' => true,
b'_' => {
index > 0
&& index + 1 < bytes.len()
&& bytes[index - 1].is_ascii_alphanumeric()
&& bytes[index + 1].is_ascii_alphanumeric()
}
_ => false,
}
}
const HEX_DIGITS: &[u8; 16] = b"0123456789ABCDEF";
#[cfg(test)]
mod tests {
use super::HookToolName;
@@ -78,32 +123,56 @@ mod tests {
use pretty_assertions::assert_eq;
#[test]
fn for_function_tool_keeps_plain_tool_names() {
fn for_dynamic_tool_keeps_plain_tool_names() {
assert_eq!(
HookToolName::for_function_tool(&ToolName::plain("tool_search")),
HookToolName::for_dynamic_tool(&ToolName::plain("tool_search")),
HookToolName::new("tool_search"),
);
}
#[test]
fn for_function_tool_keeps_mcp_names_stable() {
fn for_mcp_tool_keeps_mcp_names_stable() {
assert_eq!(
HookToolName::for_function_tool(&ToolName::namespaced(
"mcp__memory__",
"create_entities",
)),
HookToolName::for_mcp_tool(&ToolName::namespaced("mcp__memory__", "create_entities",)),
HookToolName::new("mcp__memory__create_entities"),
);
}
#[test]
fn for_function_tool_prefixes_dynamic_namespaces() {
fn for_dynamic_tool_prefixes_dynamic_namespaces() {
assert_eq!(
HookToolName::for_function_tool(&ToolName::namespaced(
"codex_app",
"automation_update",
)),
HookToolName::for_dynamic_tool(
&ToolName::namespaced("codex_app", "automation_update",)
),
HookToolName::new("dynamic__codex_app__automation_update"),
);
}
#[test]
fn for_dynamic_tool_does_not_spoof_mcp_namespaces() {
assert_eq!(
HookToolName::for_dynamic_tool(
&ToolName::namespaced("mcp__filesystem__", "read_file",)
),
HookToolName::new("dynamic__mcpZ5FZ5FfilesystemZ5FZ5F__read_file"),
);
}
#[test]
fn for_dynamic_tool_escapes_ambiguous_delimiters() {
let first = HookToolName::for_dynamic_tool(&ToolName::namespaced("foo__bar", "baz"));
let second = HookToolName::for_dynamic_tool(&ToolName::namespaced("foo", "bar__baz"));
assert_eq!(first, HookToolName::new("dynamic__fooZ5FZ5Fbar__baz"));
assert_eq!(second, HookToolName::new("dynamic__foo__barZ5FZ5Fbaz"));
assert_ne!(first, second);
}
#[test]
fn for_dynamic_tool_escapes_punctuation_and_edge_underscores() {
assert_eq!(
HookToolName::for_dynamic_tool(&ToolName::namespaced("_google-drive", "update.file_",)),
HookToolName::new("dynamic__Z5FgoogleZ2Ddrive__updateZ2EfileZ5F"),
);
}
}

33
codex-rs/hooks/src/events/common.rs
View File

@@ -109,7 +109,7 @@ pub(crate) fn matcher_pattern_for_event(
}
pub(crate) fn validate_matcher_pattern(matcher: &str) -> Result<(), regex::Error> {
if is_match_all_matcher(matcher) || is_exact_matcher(matcher) {
if is_match_all_matcher(matcher) || is_literal_tool_name_matcher(matcher) {
return Ok(());
}
regex::Regex::new(matcher).map(|_| ())
@@ -119,7 +119,7 @@ pub(crate) fn matches_matcher(matcher: Option<&str>, input: Option<&str>) -> boo
match matcher {
None => true,
Some(matcher) if is_match_all_matcher(matcher) => true,
Some(matcher) if is_exact_matcher(matcher) => input
Some(matcher) if is_literal_tool_name_matcher(matcher) => input
.map(|input| matcher.split('|').any(|candidate| candidate == input))
.unwrap_or(false),
Some(matcher) => input
@@ -147,10 +147,19 @@ fn is_match_all_matcher(matcher: &str) -> bool {
matcher.is_empty() || matcher == "*"
}
fn is_exact_matcher(matcher: &str) -> bool {
fn is_literal_tool_name_matcher(matcher: &str) -> bool {
matcher
.chars()
.all(|ch| ch.is_ascii_alphanumeric() || ch == '_' || ch == '|')
.split('|')
.all(|candidate| !candidate.is_empty() && !contains_regex_syntax(candidate))
}
fn contains_regex_syntax(candidate: &str) -> bool {
candidate.chars().any(|ch| {
matches!(
ch,
'\\' | '^' | '$' | '*' | '(' | ')' | '[' | ']' | '{' | '}'
)
})
}
#[cfg(test)]
@@ -194,6 +203,18 @@ mod tests {
fn literal_matcher_uses_exact_matching() {
assert!(matches_matcher(Some("Bash"), Some("Bash")));
assert!(!matches_matcher(Some("Bash"), Some("BashOutput")));
assert!(matches_matcher(
Some("tool-search.v2"),
Some("tool-search.v2")
));
assert!(!matches_matcher(
Some("tool-search.v2"),
Some("tool-searchXv2")
));
assert!(matches_matcher(
Some("lookup+ticket"),
Some("lookup+ticket")
));
assert!(matches_matcher(
Some("dynamic__codex_app__automation_update"),
Some("dynamic__codex_app__automation_update")
@@ -210,6 +231,8 @@ mod tests {
Some("mcp__memory"),
Some("mcp__memory__create_entities")
));
assert_eq!(validate_matcher_pattern("tool-search.v2"), Ok(()));
assert_eq!(validate_matcher_pattern("lookup+ticket"), Ok(()));
assert_eq!(validate_matcher_pattern("mcp__memory"), Ok(()));
}