mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
changes
This commit is contained in:
@@ -326,6 +326,11 @@ impl Codex {
|
||||
let exec_policy = ExecPolicyManager::load(&config.config_layer_stack)
|
||||
.await
|
||||
.map_err(|err| CodexErr::Fatal(format!("failed to load rules: {err}")))?;
|
||||
exec_policy.add_skill_script_allow_rules(
|
||||
loaded_skills
|
||||
.skills_with_enabled()
|
||||
.filter_map(|(skill, enabled)| enabled.then_some(skill)),
|
||||
);
|
||||
|
||||
let config = Arc::new(config);
|
||||
let _ = models_manager
|
||||
|
||||
@@ -9,6 +9,8 @@ use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigLayerStackOrdering;
|
||||
use crate::is_dangerous_command::command_might_be_dangerous;
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
use crate::skills::SkillMetadata;
|
||||
use crate::skills::permissions::exec_policy_prefixes_for_skill_scripts;
|
||||
use codex_execpolicy::AmendError;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Error as ExecPolicyRuleError;
|
||||
@@ -158,6 +160,38 @@ impl ExecPolicyManager {
|
||||
self.policy.load_full()
|
||||
}
|
||||
|
||||
pub(crate) fn add_skill_script_allow_rules<'a, I>(&self, skills: I)
|
||||
where
|
||||
I: IntoIterator<Item = &'a SkillMetadata>,
|
||||
{
|
||||
let mut prefixes = Vec::new();
|
||||
for skill in skills {
|
||||
let Some(skill_dir) = skill.path.parent() else {
|
||||
continue;
|
||||
};
|
||||
prefixes.extend(exec_policy_prefixes_for_skill_scripts(
|
||||
skill_dir,
|
||||
skill.permissions.as_ref(),
|
||||
));
|
||||
}
|
||||
if prefixes.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
prefixes.sort();
|
||||
prefixes.dedup();
|
||||
let mut updated_policy = self.current().as_ref().clone();
|
||||
for prefix in prefixes {
|
||||
if let Err(error) = updated_policy.add_prefix_rule(&prefix, Decision::Allow) {
|
||||
tracing::warn!(
|
||||
?prefix,
|
||||
"failed to add skill script execpolicy rule: {error}"
|
||||
);
|
||||
}
|
||||
}
|
||||
self.policy.store(Arc::new(updated_policy));
|
||||
}
|
||||
|
||||
pub(crate) async fn create_exec_approval_requirement_for_command(
|
||||
&self,
|
||||
req: ExecApprovalRequest<'_>,
|
||||
@@ -688,9 +722,13 @@ mod tests {
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use crate::skills::SkillMetadata;
|
||||
use crate::skills::permissions::SkillManifestPermissions;
|
||||
use crate::skills::permissions::compile_permission_profile;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
@@ -1388,6 +1426,45 @@ prefix_rule(
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_skill_script_allow_rules_includes_script_files_for_permissioned_skills() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
let skill_dir = temp_dir.path().join("skill");
|
||||
let scripts_dir = skill_dir.join("scripts");
|
||||
fs::create_dir_all(&scripts_dir).expect("create scripts dir");
|
||||
let script_path = scripts_dir.join("run.sh");
|
||||
fs::write(&script_path, "#!/bin/sh\necho hi\n").expect("write script");
|
||||
let permission_profile =
|
||||
compile_permission_profile(&skill_dir, Some(SkillManifestPermissions::default()))
|
||||
.expect("permission profile");
|
||||
let skill = SkillMetadata {
|
||||
name: "test-skill".to_string(),
|
||||
description: "skill".to_string(),
|
||||
short_description: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permissions: Some(permission_profile),
|
||||
path: skill_dir.join("SKILL.md"),
|
||||
scope: SkillScope::User,
|
||||
};
|
||||
let manager = ExecPolicyManager::default();
|
||||
|
||||
manager.add_skill_script_allow_rules([&skill]);
|
||||
let command = [script_path.to_string_lossy().into_owned()];
|
||||
let evaluation = manager.current().check(&command, &|_| Decision::Forbidden);
|
||||
|
||||
assert_eq!(evaluation.decision, Decision::Allow);
|
||||
assert_eq!(
|
||||
evaluation.matched_rules,
|
||||
vec![RuleMatch::PrefixRuleMatch {
|
||||
matched_prefix: vec![script_path.to_string_lossy().into_owned()],
|
||||
decision: Decision::Allow,
|
||||
justification: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn proposed_execpolicy_amendment_is_present_for_single_command_without_policy_match() {
|
||||
let command = vec!["cargo".to_string(), "build".to_string()];
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use std::collections::HashSet;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
@@ -118,6 +119,70 @@ pub(crate) fn compile_permission_profile(
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn exec_policy_prefixes_for_skill_scripts(
|
||||
skill_dir: &Path,
|
||||
permission_profile: Option<&Permissions>,
|
||||
) -> Vec<Vec<String>> {
|
||||
let Some(permission_profile) = permission_profile else {
|
||||
return Vec::new();
|
||||
};
|
||||
match permission_profile.sandbox_policy.get() {
|
||||
SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => {}
|
||||
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {
|
||||
return Vec::new();
|
||||
}
|
||||
}
|
||||
|
||||
let scripts_dir = skill_dir.join("scripts");
|
||||
let mut pending_dirs = vec![scripts_dir];
|
||||
let mut script_files = Vec::new();
|
||||
while let Some(dir) = pending_dirs.pop() {
|
||||
let read_dir = match std::fs::read_dir(&dir) {
|
||||
Ok(read_dir) => read_dir,
|
||||
Err(error) if error.kind() == ErrorKind::NotFound => continue,
|
||||
Err(error) => {
|
||||
warn!("failed to read scripts dir {}: {error}", dir.display());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
for entry in read_dir {
|
||||
let entry = match entry {
|
||||
Ok(entry) => entry,
|
||||
Err(error) => {
|
||||
warn!("failed to read scripts entry in {}: {error}", dir.display());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let path = entry.path();
|
||||
let file_type = match entry.file_type() {
|
||||
Ok(file_type) => file_type,
|
||||
Err(error) => {
|
||||
warn!(
|
||||
"failed to read scripts entry type for {}: {error}",
|
||||
path.display()
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
if file_type.is_dir() {
|
||||
pending_dirs.push(path);
|
||||
continue;
|
||||
}
|
||||
if file_type.is_file() {
|
||||
script_files.push(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
script_files.sort();
|
||||
script_files.dedup();
|
||||
script_files
|
||||
.into_iter()
|
||||
.map(|path| vec![path.to_string_lossy().into_owned()])
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn normalize_permission_paths(
|
||||
skill_dir: &Path,
|
||||
values: &[String],
|
||||
@@ -289,6 +354,7 @@ mod tests {
|
||||
use super::SkillManifestMacOsPermissions;
|
||||
use super::SkillManifestPermissions;
|
||||
use super::compile_permission_profile;
|
||||
use super::exec_policy_prefixes_for_skill_scripts;
|
||||
use crate::config::Constrained;
|
||||
use crate::config::Permissions;
|
||||
use crate::config::types::ShellEnvironmentPolicy;
|
||||
@@ -370,6 +436,49 @@ mod tests {
|
||||
assert_eq!(profile, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_policy_prefixes_for_skill_scripts_returns_empty_without_permission_profile() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
fs::create_dir_all(skill_dir.join("scripts")).expect("scripts dir");
|
||||
fs::write(
|
||||
skill_dir.join("scripts").join("run.sh"),
|
||||
"#!/bin/sh\necho hi\n",
|
||||
)
|
||||
.expect("script file");
|
||||
|
||||
let prefixes = exec_policy_prefixes_for_skill_scripts(&skill_dir, None);
|
||||
|
||||
assert_eq!(prefixes, Vec::<Vec<String>>::new());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_policy_prefixes_for_skill_scripts_has_one_rule_per_script_file() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
let skill_dir = tempdir.path().join("skill");
|
||||
let scripts_dir = skill_dir.join("scripts");
|
||||
fs::create_dir_all(scripts_dir.join("nested")).expect("scripts dirs");
|
||||
let root_script = scripts_dir.join("run.sh");
|
||||
let nested_script = scripts_dir.join("nested").join("build.py");
|
||||
fs::write(&root_script, "#!/bin/sh\necho hi\n").expect("root script");
|
||||
fs::write(&nested_script, "#!/usr/bin/env python3\nprint('ok')\n").expect("nested script");
|
||||
|
||||
let permission_profile =
|
||||
compile_permission_profile(&skill_dir, Some(SkillManifestPermissions::default()))
|
||||
.expect("permission profile");
|
||||
|
||||
let prefixes =
|
||||
exec_policy_prefixes_for_skill_scripts(&skill_dir, Some(&permission_profile));
|
||||
|
||||
assert_eq!(
|
||||
prefixes,
|
||||
vec![
|
||||
vec![nested_script.to_string_lossy().into_owned()],
|
||||
vec![root_script.to_string_lossy().into_owned()],
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn compile_permission_profile_with_network_only_uses_read_only_policy() {
|
||||
let tempdir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user