openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-13 21:30:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

changes

Browse Source
This commit is contained in:
celia-oai
2026-02-18 16:33:56 -08:00
parent 13ec48ccb7
commit c33c2a650f
3 changed files with 54 additions and 173 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/skills/command_policy.rs
+40 -155
View File
@@ -1,109 +1,32 @@
use std::collections::HashSet;
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;
use codex_protocol::protocol::SandboxPolicy;
use dirs::home_dir;
use dunce::canonicalize as canonicalize_path;
use crate::bash::parse_shell_lc_plain_commands;
use crate::bash::parse_shell_lc_single_command_prefix;
use crate::config::Permissions;
use crate::path_utils::normalize_for_path_comparison;
use crate::skills::SkillLoadOutcome;
use crate::skills::SkillMetadata;
/// Resolves the sandbox policy extension contributed by the first matching
/// skill for a command invocation.
///
/// Assumptions:
/// 1. `command_cwd` reflects the effective command target location.
/// 2. If `command_cwd` is contained by multiple skill directories, the first
/// enabled skill in `skills_outcome.skills` wins.
/// 3. Command tokens are not used for matching.
///
/// Returns `None` when no enabled skill with permissions matches
/// `command_cwd`.
pub(crate) fn resolve_skill_sandbox_extension_for_command(
skills_outcome: &SkillLoadOutcome,
command: &[String],
command_cwd: &Path,
) -> Option<SandboxPolicy> {
let segments = command_segments_for_matching(command);
let mut best_match: Option<(usize, SandboxPolicy)> = None;
for segment in segments {
for candidate in candidate_paths_for_segment(&segment, command_cwd) {
let Some((depth, _, permissions)) =
match_skill_for_candidate(skills_outcome, candidate.as_path())
else {
continue;
};
let should_replace = match &best_match {
Some((best_depth, _)) => depth > *best_depth,
None => true,
};
if should_replace {
best_match = Some((depth, permissions.sandbox_policy.get().clone()));
}
}
}
best_match.map(|(_, sandbox_policy)| sandbox_policy)
}
fn command_segments_for_matching(command: &[String]) -> Vec<Vec<String>> {
if let Some(commands) = parse_shell_lc_plain_commands(command)
&& !commands.is_empty()
{
return commands;
}
if let Some(command) = parse_shell_lc_single_command_prefix(command) {
return vec![command];
}
vec![command.to_vec()]
}
fn candidate_paths_for_segment(segment: &[String], command_cwd: &Path) -> Vec<PathBuf> {
let mut candidates = Vec::new();
let mut seen = HashSet::new();
if let Some(path) = normalize_candidate_path(command_cwd)
&& seen.insert(path.clone())
{
candidates.push(path);
}
for token in segment {
let Some(path) = candidate_path_from_token(token, command_cwd) else {
continue;
};
if seen.insert(path.clone()) {
candidates.push(path);
}
}
candidates
}
fn candidate_path_from_token(token: &str, command_cwd: &Path) -> Option<PathBuf> {
if token.is_empty() || token.contains("://") || token.starts_with('-') {
return None;
}
let is_path_like = token == "~"
|| token.starts_with("~/")
|| token.starts_with("./")
|| token.starts_with("../")
|| token.contains('/')
|| token.contains('\\')
|| Path::new(token).is_absolute()
|| command_cwd.join(token).exists();
if !is_path_like {
return None;
}
let expanded = expand_home(token);
let path = PathBuf::from(expanded);
let absolute = if path.is_absolute() {
path
} else {
command_cwd.join(path)
};
normalize_candidate_path(&absolute)
let candidate = normalize_candidate_path(command_cwd)?;
let permissions = match_skill_for_candidate(skills_outcome, candidate.as_path())?;
Some(permissions.sandbox_policy.get().clone())
}
fn normalize_candidate_path(path: &Path) -> Option<PathBuf> {
@@ -121,52 +44,32 @@ fn normalize_candidate_path(path: &Path) -> Option<PathBuf> {
fn match_skill_for_candidate<'a>(
skills_outcome: &'a SkillLoadOutcome,
candidate: &Path,
) -> Option<(usize, &'a SkillMetadata, &'a Permissions)> {
let mut best_match: Option<(usize, &SkillMetadata, &Permissions)> = None;
) -> Option<&'a Permissions> {
for skill in &skills_outcome.skills {
// Disabled skills must not contribute sandbox policy extensions.
if skills_outcome.disabled_paths.contains(&skill.path) {
continue;
}
// Skills without a permissions block cannot supply sandbox policy.
let Some(permissions) = skill.permissions.as_ref() else {
continue;
};
// Match against the containing skill directory, not the SKILL.md file.
let Some(skill_dir) = skill.path.parent() else {
continue;
};
// Normalize before comparing so path containment checks are stable.
let Some(skill_dir) = normalize_candidate_path(skill_dir) else {
continue;
};
// The command cwd must be inside the skill directory.
if !candidate.starts_with(&skill_dir) {
continue;
}
let depth = skill_dir.components().count();
let should_replace = match &best_match {
Some((best_depth, _, _)) => depth > *best_depth,
None => true,
};
if should_replace {
best_match = Some((depth, skill, permissions));
}
return Some(permissions);
}
best_match
}
fn expand_home(path: &str) -> String {
if path == "~" {
if let Some(home) = home_dir() {
return home.to_string_lossy().to_string();
}
return path.to_string();
}
if let Some(rest) = path.strip_prefix("~/")
&& let Some(home) = home_dir()
{
return home.join(rest).to_string_lossy().to_string();
}
path.to_string()
None
}
fn normalize_lexically(path: &Path) -> PathBuf {
@@ -237,13 +140,11 @@ mod tests {
}
#[test]
fn resolves_skill_policy_for_executable_inside_skill_directory() {
fn resolves_skill_policy_when_cwd_is_inside_skill_directory() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_dir = tempdir.path().join("skills/demo");
let scripts_dir = skill_dir.join("scripts");
std::fs::create_dir_all(&scripts_dir).expect("create scripts");
let executable = scripts_dir.join("run.sh");
std::fs::write(&executable, "#!/bin/sh\necho ok\n").expect("write script");
let skill_path = skill_dir.join("SKILL.md");
std::fs::write(&skill_path, "skill").expect("write SKILL.md");
@@ -260,19 +161,17 @@ mod tests {
skill_policy.clone(),
)]);
let resolved = resolve_skill_sandbox_extension_for_command(
&outcome,
&[canonical(&executable).to_string_lossy().to_string()],
tempdir.path(),
);
let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &scripts_dir);
assert_eq!(resolved, Some(skill_policy));
}
#[test]
fn resolves_skill_policy_for_shell_wrapped_relative_script_command() {
fn does_not_resolve_policy_when_only_command_path_matches() {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_dir = tempdir.path().join("skills/demo");
let outside_dir = tempdir.path().join("outside");
std::fs::create_dir_all(&outside_dir).expect("create outside");
let scripts_dir = skill_dir.join("scripts");
std::fs::create_dir_all(&scripts_dir).expect("create scripts");
std::fs::write(scripts_dir.join("run.sh"), "#!/bin/sh\necho ok\n").expect("write script");
@@ -292,17 +191,9 @@ mod tests {
skill_policy.clone(),
)]);
let resolved = resolve_skill_sandbox_extension_for_command(
&outcome,
&[
"bash".to_string(),
"-lc".to_string(),
"./scripts/run.sh --flag".to_string(),
],
&skill_dir,
);
let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &outside_dir);
assert_eq!(resolved, Some(skill_policy));
assert_eq!(resolved, None);
}
#[test]
@@ -310,8 +201,7 @@ mod tests {
let tempdir = tempfile::tempdir().expect("tempdir");
let skill_dir = tempdir.path().join("skills/demo");
std::fs::create_dir_all(&skill_dir).expect("create skill dir");
let executable = skill_dir.join("tool.sh");
std::fs::write(&executable, "#!/bin/sh\necho ok\n").expect("write script");
std::fs::write(skill_dir.join("tool.sh"), "#!/bin/sh\necho ok\n").expect("write script");
let skill_path = skill_dir.join("SKILL.md");
std::fs::write(&skill_path, "skill").expect("write SKILL.md");
let skill_path = canonical(&skill_path);
@@ -322,24 +212,23 @@ mod tests {
)]);
outcome.disabled_paths.insert(skill_path);
let resolved = resolve_skill_sandbox_extension_for_command(
&outcome,
&[canonical(&executable).to_string_lossy().to_string()],
tempdir.path(),
);
let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &skill_dir);
assert_eq!(resolved, None);
}
#[test]
fn prefers_most_specific_skill_directory_for_nested_match() {
fn resolves_first_matching_skill_directory_for_nested_match() {
let tempdir = tempfile::tempdir().expect("tempdir");
let parent_skill_dir = tempdir.path().join("skills/parent");
let nested_skill_dir = parent_skill_dir.join("nested");
std::fs::create_dir_all(nested_skill_dir.join("scripts")).expect("create scripts");
let executable = nested_skill_dir.join("scripts/run.sh");
std::fs::write(&executable, "#!/bin/sh\necho ok\n").expect("write script");
std::fs::write(
nested_skill_dir.join("scripts/run.sh"),
"#!/bin/sh\necho ok\n",
)
.expect("write script");
let parent_skill_path = parent_skill_dir.join("SKILL.md");
let nested_skill_path = nested_skill_dir.join("SKILL.md");
@@ -364,16 +253,12 @@ mod tests {
exclude_slash_tmp: false,
};
let outcome = outcome_with_skills(vec![
skill_with_policy(canonical(&parent_skill_path), parent_policy),
skill_with_policy(canonical(&parent_skill_path), parent_policy.clone()),
skill_with_policy(canonical(&nested_skill_path), nested_policy.clone()),
]);
let resolved = resolve_skill_sandbox_extension_for_command(
&outcome,
&[canonical(&executable).to_string_lossy().to_string()],
tempdir.path(),
);
let resolved = resolve_skill_sandbox_extension_for_command(&outcome, &nested_skill_dir);
assert_eq!(resolved, Some(nested_policy));
assert_eq!(resolved, Some(parent_policy));
}
}
codex-rs/core/src/tools/handlers/shell.rs
+8 -11
View File
@@ -304,17 +304,14 @@ impl ShellHandler {
.skills_manager
.skills_for_cwd(&turn.cwd, false)
.await;
let effective_sandbox_policy = resolve_skill_sandbox_extension_for_command(
&skills_outcome,
&exec_params.command,
&exec_params.cwd,
)
.map_or_else(
|| turn.sandbox_policy.clone(),
|skill_sandbox_policy| {
extend_sandbox_policy(&turn.sandbox_policy, &skill_sandbox_policy)
},
);
let effective_sandbox_policy =
resolve_skill_sandbox_extension_for_command(&skills_outcome, &exec_params.cwd)
.map_or_else(
|| turn.sandbox_policy.clone(),
|skill_sandbox_policy| {
extend_sandbox_policy(&turn.sandbox_policy, &skill_sandbox_policy)
},
);
let exec_approval_requirement = session
.services
codex-rs/core/src/unified_exec/process_manager.rs
+6 -7
View File
@@ -615,13 +615,12 @@ impl UnifiedExecProcessManager {
.skills_for_cwd(&context.turn.cwd, false)
.await;
let effective_sandbox_policy =
resolve_skill_sandbox_extension_for_command(&skills_outcome, &request.command, &cwd)
.map_or_else(
|| context.turn.sandbox_policy.clone(),
|skill_sandbox_policy| {
extend_sandbox_policy(&context.turn.sandbox_policy, &skill_sandbox_policy)
},
);
resolve_skill_sandbox_extension_for_command(&skills_outcome, &cwd).map_or_else(
|| context.turn.sandbox_policy.clone(),
|skill_sandbox_policy| {
extend_sandbox_policy(&context.turn.sandbox_policy, &skill_sandbox_policy)
},
);
let exec_approval_requirement = context
.session
.services