clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-06-01 19:02:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

Surface skill permission profiles in zsh-fork exec approvals (#12753)

Browse Source 
## Summary

- Preserve each skill’s raw permissions block as a permission_profile on
SkillMetadata during skill loading.
- Keep compiling that same metadata into the existing runtime
Permissions object, so current enforcement
    behavior stays intact.
- When zsh-fork intercepts execution of a script that belongs to a
skill, include the skill’s
    permission_profile in the exec approval request.
- This lets approval UIs show the extra filesystem access the skill
declared when prompting for approval.
This commit is contained in:
Celia Chen
2026-02-25 01:23:10 -08:00
committed by GitHub
parent c4ec6be4ab
commit 6a3233da64
10 changed files with 137 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
codex-rs/core/src/mcp/skill_dependencies.rs
View File

@@ -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,

1
codex-rs/core/src/skills/injection.rs
View File

@@ -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,

1
codex-rs/core/src/skills/invocation_utils.rs
View File

@@ -336,6 +336,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,

90
codex-rs/core/src/skills/loader.rs
View File

@@ -57,6 +57,15 @@ struct SkillMetadataFile {
permissions: Option<PermissionProfile>,
}
#[derive(Default)]
struct LoadedSkillMetadata {
interface: Option<SkillInterface>,
dependencies: Option<SkillDependencies>,
policy: Option<SkillPolicy>,
permission_profile: Option<PermissionProfile>,
permissions: Option<Permissions>,
}
#[derive(Debug, Default, Deserialize)]
struct Interface {
display_name: Option<String>,
@@ -506,7 +515,13 @@ 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 LoadedSkillMetadata {
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,29 +542,23 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
interface,
dependencies,
policy,
permission_profile,
permissions,
path_to_skills_md: resolved_path,
scope,
})
}
fn load_skill_metadata(
skill_path: &Path,
) -> (
Option<SkillInterface>,
Option<SkillDependencies>,
Option<SkillPolicy>,
Option<Permissions>,
) {
fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata {
// Fail open: optional metadata should not block loading SKILL.md.
let Some(skill_dir) = skill_path.parent() else {
return (None, None, None, None);
return LoadedSkillMetadata::default();
};
let metadata_path = skill_dir
.join(SKILLS_METADATA_DIR)
.join(SKILLS_METADATA_FILENAME);
if !metadata_path.exists() {
return (None, None, None, None);
return LoadedSkillMetadata::default();
}
let contents = match fs::read_to_string(&metadata_path) {
@@ -560,7 +569,7 @@ fn load_skill_metadata(
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return (None, None, None, None);
return LoadedSkillMetadata::default();
}
};
@@ -572,7 +581,7 @@ fn load_skill_metadata(
path = metadata_path.display(),
label = SKILLS_METADATA_FILENAME
);
return (None, None, None, None);
return LoadedSkillMetadata::default();
}
};
@@ -582,13 +591,15 @@ fn load_skill_metadata(
policy,
permissions,
} = parsed;
let permission_profile = permissions.clone().filter(|profile| !profile.is_empty());
(
resolve_interface(interface, skill_dir),
resolve_dependencies(dependencies),
resolve_policy(policy),
compile_permission_profile(skill_dir, permissions),
)
LoadedSkillMetadata {
interface: resolve_interface(interface, skill_dir),
dependencies: resolve_dependencies(dependencies),
policy: resolve_policy(policy),
permission_profile,
permissions: compile_permission_profile(skill_dir, permissions),
}
}
fn resolve_interface(interface: Option<Interface>, skill_dir: &Path) -> Option<SkillInterface> {
@@ -829,6 +840,8 @@ mod tests {
use crate::config_loader::ConfigRequirementsToml;
use codex_config::CONFIG_TOML_FILE;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::models::FileSystemPermissions;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
@@ -1048,6 +1061,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 +1210,7 @@ mod tests {
],
}),
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
@@ -1252,6 +1267,7 @@ interface:
}),
dependencies: None,
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: normalized(skill_path.as_path()),
scope: SkillScope::User,
@@ -1341,7 +1357,6 @@ permissions:
file_system:
read:
- "./data"
- "./data"
write:
- "./output"
"#,
@@ -1356,6 +1371,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(FileSystemPermissions {
read: Some(vec![PathBuf::from("./data")]),
write: Some(vec![PathBuf::from("./output")]),
}),
macos: None,
})
);
#[cfg(target_os = "macos")]
let macos_seatbelt_profile_extensions =
Some(crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions::default());
@@ -1444,6 +1470,7 @@ permissions: {}
windows_sandbox_mode: None,
macos_seatbelt_profile_extensions: None,
});
assert_eq!(outcome.skills[0].permission_profile, None);
assert_eq!(outcome.skills[0].permissions, expected);
}
@@ -1586,6 +1613,7 @@ permissions:
}),
dependencies: None,
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
@@ -1627,6 +1655,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 +1710,7 @@ permissions:
}),
dependencies: None,
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::User,
@@ -1723,6 +1753,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 +1799,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 +1861,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 +1899,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 +1941,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 +2010,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 +2039,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 +2072,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 +2186,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 +2223,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 +2278,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 +2290,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 +2331,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 +2370,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 +2413,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 +2425,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 +2491,7 @@ permissions:
interface: None,
dependencies: None,
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: first_path,
scope: SkillScope::Repo,
@@ -2456,6 +2503,7 @@ permissions:
interface: None,
dependencies: None,
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: second_path,
scope: SkillScope::Repo,
@@ -2528,6 +2576,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 +2636,7 @@ permissions:
interface: None,
dependencies: None,
policy: None,
permission_profile: None,
permissions: None,
path_to_skills_md: normalized(&skill_path),
scope: SkillScope::System,

2
codex-rs/core/src/skills/model.rs
View File

@@ -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.

26
codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs
View File

@@ -17,6 +17,7 @@ use codex_execpolicy::Decision;
use codex_execpolicy::Policy;
use codex_execpolicy::RuleMatch;
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::NetworkPolicyRuleAction;
use codex_protocol::protocol::RejectConfig;
@@ -179,6 +180,7 @@ impl CoreShellActionProvider {
argv: &[String],
workdir: &AbsolutePathBuf,
stopwatch: &Stopwatch,
additional_permissions: Option<PermissionProfile>,
) -> anyhow::Result<ReviewDecision> {
let command = join_program_and_argv(program, argv);
let workdir = workdir.to_path_buf();
@@ -198,7 +200,7 @@ impl CoreShellActionProvider {
None,
None,
None,
None,
additional_permissions,
)
.await
})
@@ -238,6 +240,7 @@ impl CoreShellActionProvider {
program: &AbsolutePathBuf,
argv: &[String],
workdir: &AbsolutePathBuf,
additional_permissions: Option<PermissionProfile>,
) -> anyhow::Result<EscalateAction> {
let action = match decision {
Decision::Forbidden => EscalateAction::Deny {
@@ -253,7 +256,16 @@ impl CoreShellActionProvider {
reason: Some("Execution forbidden by policy".to_string()),
}
} else {
match self.prompt(program, argv, workdir, &self.stopwatch).await? {
match self
.prompt(
program,
argv,
workdir,
&self.stopwatch,
additional_permissions,
)
.await?
{
ReviewDecision::Approved
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::ApprovedForSession => {
@@ -326,7 +338,14 @@ impl EscalationPolicy for CoreShellActionProvider {
// skill matches.
let needs_escalation = true;
return self
.process_decision(Decision::Prompt, needs_escalation, program, argv, workdir)
.process_decision(
Decision::Prompt,
needs_escalation,
program,
argv,
workdir,
skill.permission_profile.clone(),
)
.await;
}
@@ -366,6 +385,7 @@ impl EscalationPolicy for CoreShellActionProvider {
program,
argv,
workdir,
None,
)
.await
}