mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
changes
This commit is contained in:
@@ -433,6 +433,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: Some(SkillDependencies { tools }),
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: PathBuf::from("skill"),
|
||||
scope: SkillScope::User,
|
||||
|
||||
@@ -482,6 +482,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: PathBuf::from(path),
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
|
||||
@@ -358,6 +358,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: skill_doc_path,
|
||||
scope: codex_protocol::protocol::SkillScope::User,
|
||||
|
||||
@@ -13,6 +13,7 @@ use crate::skills::model::SkillMetadata;
|
||||
use crate::skills::model::SkillPolicy;
|
||||
use crate::skills::model::SkillToolDependency;
|
||||
use crate::skills::permissions::compile_permission_profile;
|
||||
use crate::skills::permissions::normalize_permission_profile;
|
||||
use crate::skills::system::system_cache_root_dir;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
@@ -506,7 +507,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
.as_deref()
|
||||
.map(sanitize_single_line)
|
||||
.filter(|value| !value.is_empty());
|
||||
let (interface, dependencies, policy, permissions) = load_skill_metadata(path);
|
||||
let (interface, dependencies, policy, permission_profile, permissions) = load_skill_metadata(path);
|
||||
|
||||
validate_len(&name, MAX_NAME_LEN, "name")?;
|
||||
validate_len(&description, MAX_DESCRIPTION_LEN, "description")?;
|
||||
@@ -527,6 +528,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
permission_profile,
|
||||
permissions,
|
||||
path_to_skills_md: resolved_path,
|
||||
scope,
|
||||
@@ -539,17 +541,18 @@ fn load_skill_metadata(
|
||||
Option<SkillInterface>,
|
||||
Option<SkillDependencies>,
|
||||
Option<SkillPolicy>,
|
||||
Option<PermissionProfile>,
|
||||
Option<Permissions>,
|
||||
) {
|
||||
// Fail open: optional metadata should not block loading SKILL.md.
|
||||
let Some(skill_dir) = skill_path.parent() else {
|
||||
return (None, None, None, None);
|
||||
return (None, None, None, None, None);
|
||||
};
|
||||
let metadata_path = skill_dir
|
||||
.join(SKILLS_METADATA_DIR)
|
||||
.join(SKILLS_METADATA_FILENAME);
|
||||
if !metadata_path.exists() {
|
||||
return (None, None, None, None);
|
||||
return (None, None, None, None, None);
|
||||
}
|
||||
|
||||
let contents = match fs::read_to_string(&metadata_path) {
|
||||
@@ -560,7 +563,7 @@ fn load_skill_metadata(
|
||||
path = metadata_path.display(),
|
||||
label = SKILLS_METADATA_FILENAME
|
||||
);
|
||||
return (None, None, None, None);
|
||||
return (None, None, None, None, None);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -572,7 +575,7 @@ fn load_skill_metadata(
|
||||
path = metadata_path.display(),
|
||||
label = SKILLS_METADATA_FILENAME
|
||||
);
|
||||
return (None, None, None, None);
|
||||
return (None, None, None, None, None);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -583,11 +586,14 @@ fn load_skill_metadata(
|
||||
permissions,
|
||||
} = parsed;
|
||||
|
||||
let permission_profile = normalize_permission_profile(skill_dir, permissions);
|
||||
|
||||
(
|
||||
resolve_interface(interface, skill_dir),
|
||||
resolve_dependencies(dependencies),
|
||||
resolve_policy(policy),
|
||||
compile_permission_profile(skill_dir, permissions),
|
||||
permission_profile.clone(),
|
||||
compile_permission_profile(skill_dir, permission_profile),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1048,6 +1054,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1196,6 +1203,7 @@ mod tests {
|
||||
],
|
||||
}),
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1252,6 +1260,7 @@ interface:
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(skill_path.as_path()),
|
||||
scope: SkillScope::User,
|
||||
@@ -1356,6 +1365,17 @@ permissions:
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(outcome.skills.len(), 1);
|
||||
assert_eq!(
|
||||
outcome.skills[0].permission_profile,
|
||||
Some(PermissionProfile {
|
||||
network: Some(true),
|
||||
file_system: Some(codex_protocol::models::FileSystemPermissions {
|
||||
read: Some(vec![normalized(skill_dir.join("data").as_path())]),
|
||||
write: Some(vec![normalized(skill_dir.join("output").as_path())]),
|
||||
}),
|
||||
macos: None,
|
||||
})
|
||||
);
|
||||
#[cfg(target_os = "macos")]
|
||||
let macos_seatbelt_profile_extensions =
|
||||
Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default());
|
||||
@@ -1586,6 +1606,7 @@ permissions:
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1627,6 +1648,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1681,6 +1703,7 @@ permissions:
|
||||
}),
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1723,6 +1746,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1768,6 +1792,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&shared_skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1829,6 +1854,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -1866,6 +1892,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&shared_skill_path),
|
||||
scope: SkillScope::Admin,
|
||||
@@ -1907,6 +1934,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&linked_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -1975,6 +2003,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&within_depth_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -2003,6 +2032,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -2035,6 +2065,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -2148,6 +2179,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2184,6 +2216,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2238,6 +2271,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&nested_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2249,6 +2283,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&root_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2289,6 +2324,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2327,6 +2363,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2369,6 +2406,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&repo_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2380,6 +2418,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&user_skill_path),
|
||||
scope: SkillScope::User,
|
||||
@@ -2445,6 +2484,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: first_path,
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2456,6 +2496,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: second_path,
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2528,6 +2569,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
@@ -2587,6 +2629,7 @@ permissions:
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: normalized(&skill_path),
|
||||
scope: SkillScope::System,
|
||||
|
||||
@@ -4,6 +4,7 @@ use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::config::Permissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
@@ -14,6 +15,7 @@ pub struct SkillMetadata {
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
pub policy: Option<SkillPolicy>,
|
||||
pub permission_profile: Option<PermissionProfile>,
|
||||
// This is an experimental field.
|
||||
pub permissions: Option<Permissions>,
|
||||
/// Path to the SKILLS.md file that declares this skill.
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::path::PathBuf;
|
||||
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsAutomationValue;
|
||||
use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::MacOsPermissions;
|
||||
#[cfg(target_os = "macos")]
|
||||
use codex_protocol::models::MacOsPreferencesValue;
|
||||
@@ -25,10 +26,53 @@ use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions;
|
||||
#[cfg(not(target_os = "macos"))]
|
||||
type MacOsSeatbeltProfileExtensions = ();
|
||||
|
||||
pub(crate) fn normalize_permission_profile(
|
||||
skill_dir: &Path,
|
||||
permissions: Option<PermissionProfile>,
|
||||
) -> Option<PermissionProfile> {
|
||||
let PermissionProfile {
|
||||
network,
|
||||
file_system,
|
||||
macos,
|
||||
} = permissions?;
|
||||
let file_system = file_system.unwrap_or_default();
|
||||
let read = normalize_permission_paths(
|
||||
skill_dir,
|
||||
file_system.read.as_deref().unwrap_or_default(),
|
||||
"permissions.file_system.read",
|
||||
)
|
||||
.into_iter()
|
||||
.map(|path| path.as_path().to_path_buf())
|
||||
.collect::<Vec<PathBuf>>();
|
||||
let write = normalize_permission_paths(
|
||||
skill_dir,
|
||||
file_system.write.as_deref().unwrap_or_default(),
|
||||
"permissions.file_system.write",
|
||||
)
|
||||
.into_iter()
|
||||
.map(|path| path.as_path().to_path_buf())
|
||||
.collect::<Vec<PathBuf>>();
|
||||
let file_system = if read.is_empty() && write.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(FileSystemPermissions {
|
||||
read: (!read.is_empty()).then_some(read),
|
||||
write: (!write.is_empty()).then_some(write),
|
||||
})
|
||||
};
|
||||
|
||||
Some(PermissionProfile {
|
||||
network,
|
||||
file_system,
|
||||
macos,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn compile_permission_profile(
|
||||
skill_dir: &Path,
|
||||
permissions: Option<PermissionProfile>,
|
||||
) -> Option<Permissions> {
|
||||
let permissions = normalize_permission_profile(skill_dir, permissions);
|
||||
let PermissionProfile {
|
||||
network,
|
||||
file_system,
|
||||
|
||||
@@ -426,7 +426,7 @@ impl ShellHandler {
|
||||
let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None);
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let (effective_sandbox_policy, exec_approval_requirement) =
|
||||
let (effective_sandbox_policy, exec_approval_requirement, additional_permissions) =
|
||||
if turn.features.enabled(Feature::SkillShellCommandSandbox) {
|
||||
let user_shell = session.user_shell();
|
||||
let matched_skill = Self::detect_skill_shell_command(
|
||||
@@ -435,6 +435,9 @@ impl ShellHandler {
|
||||
&exec_params.command,
|
||||
shell_runtime_backend,
|
||||
);
|
||||
let additional_permissions = matched_skill
|
||||
.as_ref()
|
||||
.and_then(|skill| skill.permission_profile.clone());
|
||||
let effective_sandbox_policy = matched_skill
|
||||
.as_ref()
|
||||
.and_then(|skill| skill.permissions.as_ref())
|
||||
@@ -458,7 +461,11 @@ impl ShellHandler {
|
||||
})
|
||||
.await
|
||||
};
|
||||
(effective_sandbox_policy, exec_approval_requirement)
|
||||
(
|
||||
effective_sandbox_policy,
|
||||
exec_approval_requirement,
|
||||
additional_permissions,
|
||||
)
|
||||
} else {
|
||||
let effective_sandbox_policy = turn.sandbox_policy.get().clone();
|
||||
let exec_approval_requirement = session
|
||||
@@ -472,7 +479,11 @@ impl ShellHandler {
|
||||
prefix_rule,
|
||||
})
|
||||
.await;
|
||||
(effective_sandbox_policy, exec_approval_requirement)
|
||||
(
|
||||
effective_sandbox_policy,
|
||||
exec_approval_requirement,
|
||||
normalized_additional_permissions,
|
||||
)
|
||||
};
|
||||
|
||||
let req = ShellRequest {
|
||||
@@ -483,7 +494,7 @@ impl ShellHandler {
|
||||
explicit_env_overrides,
|
||||
network: exec_params.network.clone(),
|
||||
sandbox_permissions: exec_params.sandbox_permissions,
|
||||
additional_permissions: normalized_additional_permissions,
|
||||
additional_permissions,
|
||||
justification: exec_params.justification.clone(),
|
||||
exec_approval_requirement,
|
||||
effective_sandbox_policy,
|
||||
@@ -557,6 +568,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md,
|
||||
scope: SkillScope::User,
|
||||
|
||||
@@ -248,12 +248,15 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
return Ok(output);
|
||||
}
|
||||
|
||||
let (effective_sandbox_policy, exec_approval_requirement) =
|
||||
let (effective_sandbox_policy, exec_approval_requirement, additional_permissions) =
|
||||
if turn.features.enabled(Feature::SkillShellCommandSandbox) {
|
||||
let matched_skill = Self::detect_skill_exec_command(
|
||||
turn.turn_skills.outcome.as_ref(),
|
||||
&command,
|
||||
);
|
||||
let additional_permissions = matched_skill
|
||||
.as_ref()
|
||||
.and_then(|skill| skill.permission_profile.clone());
|
||||
let effective_sandbox_policy = matched_skill
|
||||
.as_ref()
|
||||
.and_then(|skill| skill.permissions.as_ref())
|
||||
@@ -277,7 +280,11 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
})
|
||||
.await
|
||||
};
|
||||
(effective_sandbox_policy, exec_approval_requirement)
|
||||
(
|
||||
effective_sandbox_policy,
|
||||
exec_approval_requirement,
|
||||
additional_permissions,
|
||||
)
|
||||
} else {
|
||||
let effective_sandbox_policy = turn.sandbox_policy.get().clone();
|
||||
let exec_approval_requirement = session
|
||||
@@ -291,7 +298,11 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
prefix_rule: prefix_rule.clone(),
|
||||
})
|
||||
.await;
|
||||
(effective_sandbox_policy, exec_approval_requirement)
|
||||
(
|
||||
effective_sandbox_policy,
|
||||
exec_approval_requirement,
|
||||
normalized_additional_permissions,
|
||||
)
|
||||
};
|
||||
|
||||
tracing::debug!(
|
||||
@@ -311,7 +322,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
network: context.turn.network.clone(),
|
||||
tty,
|
||||
sandbox_permissions,
|
||||
additional_permissions: normalized_additional_permissions,
|
||||
additional_permissions,
|
||||
justification,
|
||||
effective_sandbox_policy,
|
||||
exec_approval_requirement,
|
||||
@@ -436,6 +447,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md,
|
||||
scope: codex_protocol::protocol::SkillScope::Repo,
|
||||
|
||||
@@ -1494,6 +1494,7 @@ mod tests {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: PathBuf::from("test-skill"),
|
||||
scope: SkillScope::User,
|
||||
|
||||
@@ -190,6 +190,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> SkillMetadata {
|
||||
.collect(),
|
||||
}),
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: skill.path.clone(),
|
||||
scope: skill.scope,
|
||||
|
||||
@@ -930,6 +930,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: repo_skill_path,
|
||||
scope: SkillScope::Repo,
|
||||
@@ -941,6 +942,7 @@ async fn submission_prefers_selected_duplicate_skill_path() {
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
permission_profile: None,
|
||||
permissions: None,
|
||||
path_to_skills_md: user_skill_path.clone(),
|
||||
scope: SkillScope::User,
|
||||
|
||||
Reference in New Issue
Block a user