From ea2fc2fd409a84e5072bda4dd928eac8ff3b428c Mon Sep 17 00:00:00 2001 From: Kyle Brown Date: Sat, 13 Jun 2026 02:21:25 +0000 Subject: [PATCH] Add fail-closed plugin script resolver --- codex-rs/core/src/lib.rs | 2 + codex-rs/core/src/plugin_script_resolver.rs | 270 ++++++++++++++ .../core/src/plugin_script_resolver_tests.rs | 338 ++++++++++++++++++ 3 files changed, 610 insertions(+) create mode 100644 codex-rs/core/src/plugin_script_resolver.rs create mode 100644 codex-rs/core/src/plugin_script_resolver_tests.rs diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 0641d7226e..1dee74b178 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -69,6 +69,8 @@ pub use mention_syntax::PLUGIN_TEXT_MENTION_SIGIL; pub use mention_syntax::TOOL_MENTION_SIGIL; pub use utils::path_utils; pub mod personality_migration; +#[allow(dead_code)] +mod plugin_script_resolver; pub(crate) mod plugins; #[doc(hidden)] pub(crate) mod prompt_debug; diff --git a/codex-rs/core/src/plugin_script_resolver.rs b/codex-rs/core/src/plugin_script_resolver.rs new file mode 100644 index 0000000000..4a0a91cc56 --- /dev/null +++ b/codex-rs/core/src/plugin_script_resolver.rs @@ -0,0 +1,270 @@ +use std::path::Path; + +use codex_analytics::PluginScriptSkill; +use codex_plugin::FirstPartyPluginRoot; +use codex_utils_absolute_path::AbsolutePathBuf; + +use crate::shell::ShellType; +use crate::skills::SkillLoadOutcome; + +#[derive(Debug)] +pub(crate) struct ResolvedPluginScript { + pub(crate) plugin_id: String, + pub(crate) script_path: String, + pub(crate) skill: Option, +} + +pub(crate) fn resolve_plugin_script( + plugin_roots: &[FirstPartyPluginRoot], + skills_outcome: &SkillLoadOutcome, + command: &str, + cwd: &AbsolutePathBuf, + shell_type: ShellType, +) -> Option { + let script_token = script_token(command, shell_type)?; + let script_path = Path::new(&script_token); + let script_path = if script_path.is_absolute() { + AbsolutePathBuf::try_from(script_path).ok()? + } else { + cwd.join(script_path) + }; + let script_path = script_path.canonicalize().ok()?; + script_path.as_path().is_file().then_some(())?; + + let (root, plugin_root) = plugin_roots + .iter() + .filter_map(|root| { + let plugin_root = root.plugin_root.canonicalize().ok()?; + script_path.strip_prefix(&plugin_root).ok()?; + Some((root, plugin_root)) + }) + .max_by_key(|(_, plugin_root)| plugin_root.components().count())?; + let relative = script_path.strip_prefix(plugin_root).ok()?; + if relative.as_os_str().is_empty() { + return None; + } + Some(ResolvedPluginScript { + plugin_id: root.plugin_id.clone(), + script_path: normalized_relative_path(relative)?, + skill: skill_for_script(skills_outcome, &root.plugin_id, &script_path), + }) +} + +fn skill_for_script( + skills_outcome: &SkillLoadOutcome, + plugin_id: &str, + script_path: &Path, +) -> Option { + skills_outcome + .skills + .iter() + .filter_map(|skill| { + if skill.plugin_id.as_deref() != Some(plugin_id) + || !skills_outcome.is_skill_enabled(skill) + { + return None; + } + let scripts_dir = skill.path_to_skills_md.parent()?.join("scripts"); + let scripts_dir = scripts_dir.canonicalize().ok()?; + script_path.strip_prefix(&scripts_dir).ok()?; + Some((skill, scripts_dir)) + }) + .max_by_key(|(_, scripts_dir)| scripts_dir.components().count()) + .map(|(skill, _)| PluginScriptSkill { + skill_name: skill.name.clone(), + skill_path: skill.path_to_skills_md.clone().into_path_buf(), + }) +} + +fn script_token(command: &str, shell_type: ShellType) -> Option { + let tokens = command_tokens(command, shell_type)?; + let program = tokens.first()?; + let windows_shell = matches!(shell_type, ShellType::PowerShell | ShellType::Cmd); + let basename = if windows_shell { + program.rsplit(['/', '\\']).next()?.to_ascii_lowercase() + } else { + Path::new(program) + .file_name() + .and_then(|name| name.to_str())? + .to_string() + }; + let basename = if windows_shell { + basename.strip_suffix(".exe").unwrap_or(&basename) + } else { + &basename + }; + if !is_safe_script_candidate(program, shell_type) { + return None; + } + let args = &tokens[1..]; + let path_qualified_program = + Path::new(program).is_absolute() || program.contains('/') || program.contains('\\'); + let runner_script = if path_qualified_program { + None + } else { + match basename { + "python" | "python3" => script_after_allowed_options(args, &["-u"]), + "bash" | "zsh" | "sh" => script_after_allowed_options(args, &["-e"]), + "node" => args.first().filter(|arg| !arg.starts_with('-')).cloned(), + "pwsh" | "powershell" => match args { + [option, script, ..] + if matches!(option.to_ascii_lowercase().as_str(), "-file" | "-f") => + { + Some(script.clone()) + } + _ => None, + }, + _ => None, + } + }; + if let Some(runner_script) = runner_script { + return is_safe_script_candidate(&runner_script, shell_type).then_some(runner_script); + } + if matches!( + basename, + "python" | "python3" | "bash" | "zsh" | "sh" | "node" | "pwsh" | "powershell" + ) { + return None; + } + + path_qualified_program.then(|| program.clone()) +} + +fn is_safe_script_candidate(token: &str, shell_type: ShellType) -> bool { + let expands_shell_paths = + matches!(shell_type, ShellType::Bash | ShellType::Sh | ShellType::Zsh); + !(expands_shell_paths && has_shell_path_expansion(token) + || shell_type == ShellType::Zsh && token.starts_with('=')) +} + +fn has_shell_path_expansion(token: &str) -> bool { + token.starts_with('~') || token.contains(['\\', '*', '?', '[', ']', '{', '}']) +} + +fn script_after_allowed_options(args: &[String], allowed_options: &[&str]) -> Option { + let mut args = args.iter(); + loop { + let arg = args.next()?; + if !arg.starts_with('-') { + return Some(arg.clone()); + } + if !allowed_options.contains(&arg.as_str()) { + return None; + } + } +} + +fn command_tokens(command: &str, shell_type: ShellType) -> Option> { + match shell_type { + ShellType::Bash | ShellType::Sh | ShellType::Zsh => { + let tree = codex_shell_command::bash::try_parse_shell(command)?; + let mut commands = + codex_shell_command::bash::try_parse_word_only_commands_sequence(&tree, command)?; + let [tokens] = commands.as_mut_slice() else { + return None; + }; + Some(std::mem::take(tokens)) + } + ShellType::PowerShell => split_powershell_command(command), + ShellType::Cmd => split_cmd_command(command), + } +} + +/// Splits one plain PowerShell-style command without treating backslashes as +/// escapes. Compound commands are rejected because lifecycle events attach to +/// the spawned shell process and cannot represent multiple child scripts. +fn split_powershell_command(command: &str) -> Option> { + if command.contains(['$', '~', '`', '(', ')', '{', '}', ',', '<', '>', '@', '#']) + || matches!(command.trim_start().chars().next(), Some('\'' | '"')) + { + return None; + } + split_windows_command(command) +} + +fn split_windows_command(command: &str) -> Option> { + let mut chars = command.chars().peekable(); + let mut tokens = Vec::new(); + let mut token = String::new(); + let mut quote = None; + let mut saw_token = false; + + while let Some(ch) = chars.next() { + if let Some(active_quote) = quote { + if ch == '`' && active_quote == '"' { + token.push(chars.next()?); + } else if ch == active_quote { + if chars.peek() == Some(&active_quote) { + token.push(active_quote); + chars.next(); + } else { + quote = None; + } + } else { + token.push(ch); + } + continue; + } + + match ch { + '\'' | '"' => { + quote = Some(ch); + saw_token = true; + } + '`' => { + token.push(chars.next()?); + saw_token = true; + } + ' ' | '\t' => { + if saw_token { + tokens.push(std::mem::take(&mut token)); + saw_token = false; + } + } + '&' if tokens.is_empty() && !saw_token => { + if !chars.peek().is_some_and(|next| next.is_whitespace()) { + return None; + } + } + '&' | '|' | ';' | '\r' | '\n' => return None, + _ => { + token.push(ch); + saw_token = true; + } + } + } + + if quote.is_some() { + return None; + } + if saw_token { + tokens.push(token); + } + (!tokens.is_empty()).then_some(tokens) +} + +fn split_cmd_command(command: &str) -> Option> { + if command.chars().any(|ch| { + matches!( + ch, + '\'' | '`' | '^' | '%' | '!' | '&' | '|' | '<' | '>' | '(' | ')' | '\r' | '\n' + ) + }) { + return None; + } + split_windows_command(command) +} + +fn normalized_relative_path(path: &Path) -> Option { + path.components() + .filter_map(|component| match component { + std::path::Component::Normal(value) => Some(value.to_str()), + _ => None, + }) + .collect::>>() + .map(|components| components.join("/")) +} + +#[cfg(test)] +#[path = "plugin_script_resolver_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/plugin_script_resolver_tests.rs b/codex-rs/core/src/plugin_script_resolver_tests.rs new file mode 100644 index 0000000000..e044fcb897 --- /dev/null +++ b/codex-rs/core/src/plugin_script_resolver_tests.rs @@ -0,0 +1,338 @@ +use codex_core_skills::SkillMetadata; +use codex_protocol::protocol::SkillScope; +use pretty_assertions::assert_eq; +use std::fs; +use tempfile::TempDir; + +use super::*; + +fn fixture() -> (TempDir, AbsolutePathBuf, Vec) { + let temp = tempfile::tempdir().expect("tempdir"); + let root = AbsolutePathBuf::try_from(temp.path().join("plugin")).expect("absolute path"); + fs::create_dir_all(root.join("skills/demo/scripts")).expect("create scripts directory"); + fs::write(root.join("skills/demo/SKILL.md"), "# Demo").expect("write skill"); + fs::write(root.join("skills/demo/scripts/run.py"), "print('ok')").expect("write script"); + let roots = vec![FirstPartyPluginRoot { + plugin_id: "openai/demo".to_string(), + plugin_root: root.clone(), + }]; + (temp, root, roots) +} + +fn skill(root: &AbsolutePathBuf, name: &str, path: &str) -> SkillMetadata { + SkillMetadata { + name: name.to_string(), + description: String::new(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: root.join(path), + scope: SkillScope::User, + plugin_id: Some("openai/demo".to_string()), + } +} + +fn skill_outcome(root: &AbsolutePathBuf) -> SkillLoadOutcome { + let mut outcome = SkillLoadOutcome::default(); + outcome.skills = vec![skill(root, "demo", "skills/demo/SKILL.md")]; + outcome +} + +fn resolve( + roots: &[FirstPartyPluginRoot], + skills: &SkillLoadOutcome, + command: &str, + cwd: &AbsolutePathBuf, +) -> Option { + resolve_plugin_script(roots, skills, command, cwd, ShellType::Bash) +} + +#[test] +fn resolves_interpreter_script_to_plugin_relative_path_and_skill() { + let (_temp, root, roots) = fixture(); + let resolved = resolve( + &roots, + &skill_outcome(&root), + "python skills/demo/scripts/run.py --secret argument", + &root, + ) + .expect("plugin script"); + + assert_eq!(resolved.plugin_id, "openai/demo"); + assert_eq!(resolved.script_path, "skills/demo/scripts/run.py"); + assert_eq!(resolved.skill.expect("skill").skill_name, "demo"); + + let absolute = resolve( + &roots, + &SkillLoadOutcome::default(), + root.join("skills/demo/scripts/run.py") + .to_string_lossy() + .replace('\\', "/") + .as_ref(), + &root, + ) + .expect("absolute plugin script"); + assert_eq!(absolute.script_path, "skills/demo/scripts/run.py"); +} + +#[test] +fn resolves_direct_executable_without_a_known_extension() { + let (_temp, root, roots) = fixture(); + fs::create_dir_all(root.join("bin")).expect("create bin"); + fs::write(root.join("bin/run"), "#!/bin/sh\n").expect("write executable"); + + let resolved = + resolve(&roots, &SkillLoadOutcome::default(), "./bin/run", &root).expect("plugin script"); + + assert_eq!(resolved.script_path, "bin/run"); + assert!(resolved.skill.is_none()); +} + +#[test] +fn rejects_non_plugin_and_symlink_escape_paths() { + let (temp, root, roots) = fixture(); + let outside = AbsolutePathBuf::try_from(temp.path().join("outside.py")).expect("absolute path"); + fs::write(&outside, "print('outside')").expect("write outside script"); + + for command in [ + outside.to_string_lossy().into_owned(), + "skills/demo/scripts".to_string(), + ] { + assert!( + resolve(&roots, &SkillLoadOutcome::default(), &command, &root).is_none(), + "unexpected lifecycle attribution for {command}" + ); + } + + #[cfg(unix)] + { + std::os::unix::fs::symlink(&outside, root.join("escape.py")).expect("create symlink"); + assert!( + resolve( + &roots, + &SkillLoadOutcome::default(), + "python escape.py", + &root, + ) + .is_none() + ); + } +} + +#[test] +fn nested_skill_owns_its_deepest_matching_scripts_directory() { + let (_temp, root, roots) = fixture(); + fs::create_dir_all(root.join("skills/demo/scripts/nested/scripts")) + .expect("create nested scripts"); + fs::write(root.join("skills/demo/scripts/nested/SKILL.md"), "# Nested") + .expect("write nested skill"); + fs::write( + root.join("skills/demo/scripts/nested/scripts/run.py"), + "print('ok')", + ) + .expect("write nested script"); + let mut skills = skill_outcome(&root); + skills.skills.push(skill( + &root, + "nested", + "skills/demo/scripts/nested/SKILL.md", + )); + + let resolved = resolve( + &roots, + &skills, + "python skills/demo/scripts/nested/scripts/run.py", + &root, + ) + .expect("nested plugin script"); + assert_eq!(resolved.skill.expect("skill").skill_name, "nested"); +} + +#[test] +fn resolves_node_and_shell_scripts() { + let (_temp, root, roots) = fixture(); + fs::write(root.join("skills/demo/scripts/run.js"), "console.log('ok')") + .expect("write node script"); + fs::write(root.join("skills/demo/scripts/run.sh"), "echo ok").expect("write shell script"); + + for (command, expected) in [ + ( + "node skills/demo/scripts/run.js", + "skills/demo/scripts/run.js", + ), + ( + "sh skills/demo/scripts/run.sh", + "skills/demo/scripts/run.sh", + ), + ( + "python -u skills/demo/scripts/run.py", + "skills/demo/scripts/run.py", + ), + ( + "sh -e skills/demo/scripts/run.sh", + "skills/demo/scripts/run.sh", + ), + ] { + let resolved = + resolve(&roots, &SkillLoadOutcome::default(), command, &root).expect("plugin script"); + assert_eq!(resolved.script_path, expected); + } +} + +#[test] +fn rejects_shell_path_expansion_candidates() { + for shell_type in [ShellType::Bash, ShellType::Sh, ShellType::Zsh] { + for command in [ + "python ~/run.py", + "~/run", + "python scripts/z?.py", + "python scripts/*.py", + "scripts/*.sh", + "python scripts/[rz]un.py", + "python scripts/{run,skip}.py", + r#"python scripts/run\ script.py"#, + r#"python "scripts/run\\script.py""#, + ] { + assert!( + script_token(command, shell_type).is_none(), + "unexpected shell expansion attribution for {shell_type:?}: {command}" + ); + } + } +} + +#[test] +fn rejects_zsh_equals_expansion_candidates() { + for command in ["python =foo", "node =foo", "=foo"] { + assert!( + script_token(command, ShellType::Zsh).is_none(), + "unexpected Zsh EQUALS attribution for {command}" + ); + } + + for shell_type in [ShellType::Bash, ShellType::Sh] { + for command in ["python =foo", "node =foo"] { + assert_eq!( + script_token(command, shell_type), + Some("=foo".to_string()), + "unexpected literal equals rejection for {shell_type:?}: {command}" + ); + } + } +} + +#[test] +fn rejects_compound_commands_and_runner_options() { + let (_temp, root, roots) = fixture(); + for command in [ + "python skills/demo/scripts/run.py && python skills/demo/scripts/run.py", + "python -c skills/demo/scripts/run.py", + "node --loader skills/demo/scripts/loader.js skills/demo/scripts/run.js", + "env -C skills/demo python scripts/run.py", + "timeout 1 python skills/demo/scripts/run.py", + "uv run skills/demo/scripts/run.py", + "cd skills/demo && python scripts/run.py", + r#"C:\tmp\python.exe skills/demo/scripts/run.py"#, + ] { + assert!( + resolve(&roots, &SkillLoadOutcome::default(), command, &root).is_none(), + "unexpected lifecycle attribution for {command}" + ); + } +} + +#[test] +#[cfg(not(windows))] +fn direct_executable_matching_interpreter_name_is_case_sensitive() { + let (_temp, root, roots) = fixture(); + fs::write(root.join("Python"), "#!/bin/sh\n").expect("write case-sensitive executable"); + + let resolved = resolve(&roots, &SkillLoadOutcome::default(), "./Python", &root) + .expect("case-sensitive direct executable"); + assert_eq!(resolved.script_path, "Python"); +} + +#[test] +fn powershell_split_preserves_paths_and_rejects_compounds() { + assert_eq!( + command_tokens( + r#"pwsh.exe -File C:\Users\me\plugin\scripts\run.ps1"#, + ShellType::PowerShell, + ), + Some(vec![ + "pwsh.exe".to_string(), + "-File".to_string(), + r#"C:\Users\me\plugin\scripts\run.ps1"#.to_string(), + ]) + ); + assert_eq!( + command_tokens( + r#"& 'C:\Program Files\plugin\scripts\run.ps1'"#, + ShellType::PowerShell, + ), + Some(vec![ + r#"C:\Program Files\plugin\scripts\run.ps1"#.to_string() + ]) + ); + assert!( + command_tokens( + r#"'C:\Program Files\plugin\scripts\run.ps1'"#, + ShellType::PowerShell + ) + .is_none() + ); + for command in [ + r#"& 'C:\plugin`name\scripts\run.ps1'"#, + r#"& "C:\plugin` name\scripts\run.ps1""#, + r#"& "C:\plugin`u{2e}\scripts\run.ps1""#, + ] { + assert!( + command_tokens(command, ShellType::PowerShell).is_none(), + "unexpected PowerShell backtick attribution for {command}" + ); + } + for command in [ + r#"python "$HOME\run.py""#, + r#"python $HOME\run.py"#, + r#"& "$HOME\run.ps1""#, + r#"& ~\run.ps1"#, + "python (Get-Command outside-script).Source", + "python {outside-script}", + "python @(outside-script)", + "python scripts/run.py > outside.txt", + "python scripts/run.py # outside", + ] { + assert!( + command_tokens(command, ShellType::PowerShell).is_none(), + "unexpected PowerShell expansion attribution for {command}" + ); + } + assert!(command_tokens("python a.py; python b.py", ShellType::PowerShell).is_none()); + assert_eq!( + command_tokens(r#"C:\plugin\scripts\run.cmd"#, ShellType::Cmd), + Some(vec![r#"C:\plugin\scripts\run.cmd"#.to_string()]) + ); + assert_eq!( + command_tokens( + r#""C:\Program Files\plugin\scripts\run.bat""#, + ShellType::Cmd + ), + Some(vec![ + r#"C:\Program Files\plugin\scripts\run.bat"#.to_string() + ]) + ); + assert!(command_tokens("python run.py`&whoami", ShellType::Cmd).is_none()); + assert!(command_tokens(r#"'C:\plugin\scripts\run.cmd'"#, ShellType::Cmd).is_none()); + assert!(script_token(r#"C:\tmp\python.exe run.py"#, ShellType::Cmd).is_none()); +} + +#[test] +#[cfg(unix)] +fn rejects_non_utf8_normalized_paths() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + + let path = OsString::from_vec(b"scripts/run\xff.py".to_vec()); + assert_eq!(normalized_relative_path(Path::new(&path)), None); +}