fix: use AbsolutePathBufGuard to resolve relative file paths for skill permissions

codex-rs/core/src/skills/loader.rs

@@ -12,11 +12,12 @@ use crate::skills::model::SkillLoadOutcome;
 use crate::skills::model::SkillMetadata;
 use crate::skills::model::SkillPolicy;
 use crate::skills::model::SkillToolDependency;
+use crate::skills::permissions::SkillPermissionProfile;
 use crate::skills::permissions::compile_permission_profile;
 use crate::skills::system::system_cache_root_dir;
 use codex_app_server_protocol::ConfigLayerSource;
-use codex_protocol::models::PermissionProfile;
 use codex_protocol::protocol::SkillScope;
+use codex_utils_absolute_path::AbsolutePathBufGuard;
 use dirs::home_dir;
 use dunce::canonicalize as canonicalize_path;
 use serde::Deserialize;
@@ -54,7 +55,7 @@ struct SkillMetadataFile {
     #[serde(default)]
     policy: Option<Policy>,
     #[serde(default)]
-    permissions: Option<PermissionProfile>,
+    permissions: Option<SkillPermissionProfile>,
 }
 
 #[derive(Default)]
@@ -62,7 +63,7 @@ struct LoadedSkillMetadata {
     interface: Option<SkillInterface>,
     dependencies: Option<SkillDependencies>,
     policy: Option<SkillPolicy>,
-    permission_profile: Option<PermissionProfile>,
+    permission_profile: Option<SkillPermissionProfile>,
     permissions: Option<Permissions>,
 }
 
@@ -573,7 +574,10 @@ fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata {
         }
     };
 
-    let parsed: SkillMetadataFile = match serde_yaml::from_str(&contents) {
+    let parsed: SkillMetadataFile = match {
+        let _guard = AbsolutePathBufGuard::new(skill_dir);
+        serde_yaml::from_str(&contents)
+    } {
         Ok(parsed) => parsed,
         Err(error) => {
             tracing::warn!(
@@ -598,7 +602,7 @@ fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata {
         dependencies: resolve_dependencies(dependencies),
         policy: resolve_policy(policy),
         permission_profile,
-        permissions: compile_permission_profile(skill_dir, permissions),
+        permissions: compile_permission_profile(permissions.as_ref()),
     }
 }
 
@@ -838,10 +842,10 @@ mod tests {
     use crate::config_loader::ConfigLayerStack;
     use crate::config_loader::ConfigRequirements;
     use crate::config_loader::ConfigRequirementsToml;
+    use crate::skills::permissions::SkillFileSystemPermissions;
+    use crate::skills::permissions::SkillPermissionProfile;
     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;
@@ -1373,11 +1377,17 @@ permissions:
         assert_eq!(outcome.skills.len(), 1);
         assert_eq!(
             outcome.skills[0].permission_profile,
-            Some(PermissionProfile {
+            Some(SkillPermissionProfile {
                 network: Some(true),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![PathBuf::from("./data")]),
-                    write: Some(vec![PathBuf::from("./output")]),
+                file_system: Some(SkillFileSystemPermissions {
+                    read: Some(vec![
+                        AbsolutePathBuf::try_from(normalized(skill_dir.join("data").as_path()))
+                            .expect("absolute data path"),
+                    ]),
+                    write: Some(vec![
+                        AbsolutePathBuf::try_from(normalized(skill_dir.join("output").as_path()))
+                            .expect("absolute output path"),
+                    ]),
                 }),
                 macos: None,
             })

codex-rs/core/src/skills/model.rs

@@ -4,7 +4,7 @@ use std::path::PathBuf;
 use std::sync::Arc;
 
 use crate::config::Permissions;
-use codex_protocol::models::PermissionProfile;
+use crate::skills::permissions::SkillPermissionProfile;
 use codex_protocol::protocol::SkillScope;
 
 #[derive(Debug, Clone, PartialEq)]
@@ -15,7 +15,7 @@ pub struct SkillMetadata {
     pub interface: Option<SkillInterface>,
     pub dependencies: Option<SkillDependencies>,
     pub policy: Option<SkillPolicy>,
-    pub permission_profile: Option<PermissionProfile>,
+    pub permission_profile: Option<SkillPermissionProfile>,
     // This is an experimental field.
     pub permissions: Option<Permissions>,
     /// Path to the SKILLS.md file that declares this skill.

codex-rs/core/src/skills/permissions.rs

@@ -1,8 +1,6 @@
-use std::collections::HashSet;
-use std::path::Component;
-use std::path::Path;
 use std::path::PathBuf;
 
+use codex_protocol::models::FileSystemPermissions;
 #[cfg(target_os = "macos")]
 use codex_protocol::models::MacOsAutomationValue;
 use codex_protocol::models::MacOsPermissions;
@@ -11,8 +9,8 @@ use codex_protocol::models::MacOsPreferencesValue;
 use codex_protocol::models::MacOsSeatbeltProfileExtensions;
 use codex_protocol::models::PermissionProfile;
 use codex_utils_absolute_path::AbsolutePathBuf;
-use dirs::home_dir;
 use dunce::canonicalize as canonicalize_path;
+use serde::Deserialize;
 use tracing::warn;
 
 use crate::config::Constrained;
@@ -22,23 +20,86 @@ use crate::protocol::AskForApproval;
 use crate::protocol::ReadOnlyAccess;
 use crate::protocol::SandboxPolicy;
 
+#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)]
+pub struct SkillFileSystemPermissions {
+    pub read: Option<Vec<AbsolutePathBuf>>,
+    pub write: Option<Vec<AbsolutePathBuf>>,
+}
+
+impl SkillFileSystemPermissions {
+    pub(crate) fn is_empty(&self) -> bool {
+        self.read.is_none() && self.write.is_none()
+    }
+}
+
+impl From<&SkillFileSystemPermissions> for FileSystemPermissions {
+    fn from(value: &SkillFileSystemPermissions) -> Self {
+        Self {
+            read: value
+                .read
+                .as_ref()
+                .map(|paths| absolute_paths_to_path_bufs(paths)),
+            write: value
+                .write
+                .as_ref()
+                .map(|paths| absolute_paths_to_path_bufs(paths)),
+        }
+    }
+}
+
+#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize)]
+pub struct SkillPermissionProfile {
+    pub network: Option<bool>,
+    pub file_system: Option<SkillFileSystemPermissions>,
+    pub macos: Option<MacOsPermissions>,
+}
+
+impl SkillPermissionProfile {
+    pub(crate) fn is_empty(&self) -> bool {
+        self.network.is_none()
+            && self
+                .file_system
+                .as_ref()
+                .map(SkillFileSystemPermissions::is_empty)
+                .unwrap_or(true)
+            && self
+                .macos
+                .as_ref()
+                .map(MacOsPermissions::is_empty)
+                .unwrap_or(true)
+    }
+}
+
+impl From<&SkillPermissionProfile> for PermissionProfile {
+    fn from(value: &SkillPermissionProfile) -> Self {
+        Self {
+            network: value.network,
+            file_system: value.file_system.as_ref().map(FileSystemPermissions::from),
+            macos: value.macos.clone(),
+        }
+    }
+}
+
+impl From<SkillPermissionProfile> for PermissionProfile {
+    fn from(value: SkillPermissionProfile) -> Self {
+        Self::from(&value)
+    }
+}
+
 pub(crate) fn compile_permission_profile(
-    skill_dir: &Path,
-    permissions: Option<PermissionProfile>,
+    permissions: Option<&SkillPermissionProfile>,
 ) -> Option<Permissions> {
-    let PermissionProfile {
-        network,
-        file_system,
-        macos,
-    } = permissions?;
-    let file_system = file_system.unwrap_or_default();
-    let fs_read = normalize_permission_paths(
-        skill_dir,
+    let permissions = permissions?;
+    let file_system = permissions
+        .file_system
+        .as_ref()
+        .cloned()
+        .unwrap_or_default();
+    let fs_read = canonicalize_permission_paths(
         file_system.read.as_deref().unwrap_or_default(),
         "permissions.file_system.read",
     );
-    let fs_write = normalize_permission_paths(
-        skill_dir,
+    let fs_write = canonicalize_permission_paths(
         file_system.write.as_deref().unwrap_or_default(),
         "permissions.file_system.write",
     );
@@ -53,7 +114,7 @@ pub(crate) fn compile_permission_profile(
                     readable_roots: fs_read,
                 }
             },
-            network_access: network.unwrap_or_default(),
+            network_access: permissions.network.unwrap_or_default(),
             exclude_tmpdir_env_var: false,
             exclude_slash_tmp: false,
         }
@@ -68,7 +129,7 @@ pub(crate) fn compile_permission_profile(
         // Default sandbox policy
         SandboxPolicy::new_read_only_policy()
     };
-    let macos_permissions = macos.unwrap_or_default();
+    let macos_permissions = permissions.macos.clone().unwrap_or_default();
     let macos_seatbelt_profile_extensions =
         build_macos_seatbelt_profile_extensions(&macos_permissions);
 
@@ -83,68 +144,31 @@ pub(crate) fn compile_permission_profile(
     })
 }
 
-fn normalize_permission_paths(
-    skill_dir: &Path,
-    values: &[PathBuf],
-    field: &str,
-) -> Vec<AbsolutePathBuf> {
-    let mut paths = Vec::new();
-    let mut seen = HashSet::new();
-
-    for value in values {
-        let Some(path) = normalize_permission_path(skill_dir, value, field) else {
-            continue;
-        };
-        if seen.insert(path.clone()) {
-            paths.push(path);
-        }
-    }
-
-    paths
+fn absolute_paths_to_path_bufs(values: &[AbsolutePathBuf]) -> Vec<PathBuf> {
+    values.iter().map(AbsolutePathBuf::to_path_buf).collect()
 }
 
-fn normalize_permission_path(
-    skill_dir: &Path,
-    value: &Path,
-    field: &str,
-) -> Option<AbsolutePathBuf> {
-    let value = value.to_string_lossy();
-    let trimmed = value.trim();
-    if trimmed.is_empty() {
-        warn!("ignoring {field}: value is empty");
-        return None;
-    }
-
-    let expanded = expand_home(trimmed);
-    let absolute = if expanded.is_absolute() {
-        expanded
-    } else {
-        skill_dir.join(expanded)
-    };
-    let normalized = normalize_lexically(&absolute);
-    let canonicalized = canonicalize_path(&normalized).unwrap_or(normalized);
-    match AbsolutePathBuf::from_absolute_path(&canonicalized) {
-        Ok(path) => Some(path),
-        Err(error) => {
-            warn!("ignoring {field}: expected absolute path, got {canonicalized:?}: {error}");
-            None
-        }
-    }
-}
-
-fn expand_home(path: &str) -> PathBuf {
-    if path == "~" {
-        if let Some(home) = home_dir() {
-            return home;
-        }
-        return PathBuf::from(path);
-    }
-    if let Some(rest) = path.strip_prefix("~/")
-        && let Some(home) = home_dir()
-    {
-        return home.join(rest);
-    }
-    PathBuf::from(path)
+fn canonicalize_permission_paths(values: &[AbsolutePathBuf], field: &str) -> Vec<AbsolutePathBuf> {
+    values
+        .iter()
+        .filter_map(|value| {
+            let canonicalized = canonicalize_path(value.as_path()).unwrap_or(value.to_path_buf());
+            match AbsolutePathBuf::from_absolute_path(&canonicalized) {
+                Ok(path) => Some(path),
+                Err(error) => {
+                    warn!(
+                        "ignoring {field}: expected absolute path, got {canonicalized:?}: {error}"
+                    );
+                    None
+                }
+            }
+        })
+        .fold(Vec::new(), |mut paths, path| {
+            if !paths.contains(&path) {
+                paths.push(path);
+            }
+            paths
+        })
 }
 
 #[cfg(target_os = "macos")]
@@ -233,24 +257,10 @@ fn build_macos_seatbelt_profile_extensions(
     None
 }
 
-fn normalize_lexically(path: &Path) -> PathBuf {
-    let mut normalized = PathBuf::new();
-    for component in path.components() {
-        match component {
-            Component::CurDir => {}
-            Component::ParentDir => {
-                normalized.pop();
-            }
-            Component::RootDir | Component::Prefix(_) | Component::Normal(_) => {
-                normalized.push(component.as_os_str());
-            }
-        }
-    }
-    normalized
-}
-
 #[cfg(test)]
 mod tests {
+    use super::SkillFileSystemPermissions;
+    use super::SkillPermissionProfile;
     use super::compile_permission_profile;
     use crate::config::Constrained;
     use crate::config::Permissions;
@@ -258,18 +268,15 @@ mod tests {
     use crate::protocol::AskForApproval;
     use crate::protocol::ReadOnlyAccess;
     use crate::protocol::SandboxPolicy;
-    use codex_protocol::models::FileSystemPermissions;
     #[cfg(target_os = "macos")]
     use codex_protocol::models::MacOsAutomationValue;
     #[cfg(target_os = "macos")]
     use codex_protocol::models::MacOsPermissions;
     #[cfg(target_os = "macos")]
     use codex_protocol::models::MacOsPreferencesValue;
-    use codex_protocol::models::PermissionProfile;
     use codex_utils_absolute_path::AbsolutePathBuf;
     use pretty_assertions::assert_eq;
     use std::fs;
-    use std::path::PathBuf;
 
     #[test]
     fn compile_permission_profile_normalizes_paths() {
@@ -279,21 +286,22 @@ mod tests {
         let read_dir = skill_dir.join("data");
         fs::create_dir_all(&read_dir).expect("read dir");
 
-        let profile = compile_permission_profile(
-            &skill_dir,
-            Some(PermissionProfile {
-                network: Some(true),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![
-                        PathBuf::from("./data"),
-                        PathBuf::from("./data"),
-                        PathBuf::from("scripts/../data"),
-                    ]),
-                    write: Some(vec![PathBuf::from("./output")]),
-                }),
-                ..Default::default()
+        let profile = compile_permission_profile(Some(&SkillPermissionProfile {
+            network: Some(true),
+            file_system: Some(SkillFileSystemPermissions {
+                read: Some(vec![
+                    AbsolutePathBuf::try_from(skill_dir.join("data")).expect("read path"),
+                    AbsolutePathBuf::try_from(skill_dir.join("data")).expect("read path"),
+                    AbsolutePathBuf::try_from(skill_dir.join("scripts/../data"))
+                        .expect("normalized read path"),
+                ]),
+                write: Some(vec![
+                    AbsolutePathBuf::try_from(skill_dir.join("output"))
+                        .expect("absolute output path"),
+                ]),
             }),
-        )
+            ..Default::default()
+        }))
         .expect("profile");
 
         assert_eq!(
@@ -334,11 +342,7 @@ mod tests {
 
     #[test]
     fn compile_permission_profile_without_permissions_has_empty_profile() {
-        let tempdir = tempfile::tempdir().expect("tempdir");
-        let skill_dir = tempdir.path().join("skill");
-        fs::create_dir_all(&skill_dir).expect("skill dir");
-
-        let profile = compile_permission_profile(&skill_dir, None);
+        let profile = compile_permission_profile(None);
 
         assert_eq!(profile, None);
     }
@@ -349,13 +353,10 @@ mod tests {
         let skill_dir = tempdir.path().join("skill");
         fs::create_dir_all(&skill_dir).expect("skill dir");
 
-        let profile = compile_permission_profile(
-            &skill_dir,
-            Some(PermissionProfile {
-                network: Some(true),
-                ..Default::default()
-            }),
-        )
+        let profile = compile_permission_profile(Some(&SkillPermissionProfile {
+            network: Some(true),
+            ..Default::default()
+        }))
         .expect("profile");
 
         assert_eq!(
@@ -384,17 +385,16 @@ mod tests {
         let read_dir = skill_dir.join("data");
         fs::create_dir_all(&read_dir).expect("read dir");
 
-        let profile = compile_permission_profile(
-            &skill_dir,
-            Some(PermissionProfile {
-                network: Some(true),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![PathBuf::from("./data")]),
-                    write: Some(Vec::new()),
-                }),
-                ..Default::default()
+        let profile = compile_permission_profile(Some(&SkillPermissionProfile {
+            network: Some(true),
+            file_system: Some(SkillFileSystemPermissions {
+                read: Some(vec![
+                    AbsolutePathBuf::try_from(skill_dir.join("data")).expect("absolute read path"),
+                ]),
+                write: Some(Vec::new()),
             }),
-        )
+            ..Default::default()
+        }))
         .expect("profile");
 
         assert_eq!(
@@ -433,20 +433,17 @@ mod tests {
         let skill_dir = tempdir.path().join("skill");
         fs::create_dir_all(&skill_dir).expect("skill dir");
 
-        let profile = compile_permission_profile(
-            &skill_dir,
-            Some(PermissionProfile {
-                macos: Some(MacOsPermissions {
-                    preferences: Some(MacOsPreferencesValue::Mode("readwrite".to_string())),
-                    automations: Some(MacOsAutomationValue::BundleIds(vec![
-                        "com.apple.Notes".to_string(),
-                    ])),
-                    accessibility: Some(true),
-                    calendar: Some(true),
-                }),
-                ..Default::default()
+        let profile = compile_permission_profile(Some(&SkillPermissionProfile {
+            macos: Some(MacOsPermissions {
+                preferences: Some(MacOsPreferencesValue::Mode("readwrite".to_string())),
+                automations: Some(MacOsAutomationValue::BundleIds(vec![
+                    "com.apple.Notes".to_string(),
+                ])),
+                accessibility: Some(true),
+                calendar: Some(true),
             }),
-        )
+            ..Default::default()
+        }))
         .expect("profile");
 
         assert_eq!(
@@ -473,8 +470,8 @@ mod tests {
         let skill_dir = tempdir.path().join("skill");
         fs::create_dir_all(&skill_dir).expect("skill dir");
 
-        let profile = compile_permission_profile(&skill_dir, Some(PermissionProfile::default()))
-            .expect("profile");
+        let profile =
+            compile_permission_profile(Some(&SkillPermissionProfile::default())).expect("profile");
 
         assert_eq!(
             profile.macos_seatbelt_profile_extensions,

codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs

@@ -241,7 +241,8 @@ impl CoreShellActionProvider {
             .or_else(|| {
                 skill
                     .permission_profile
-                    .clone()
+                    .as_ref()
+                    .map(PermissionProfile::from)
                     .map(EscalationPermissions::PermissionProfile)
                     .map(EscalationExecution::Permissions)
             })
@@ -486,7 +487,10 @@ impl EscalationPolicy for CoreShellActionProvider {
                     program,
                     argv,
                     workdir,
-                    skill.permission_profile.clone(),
+                    skill
+                        .permission_profile
+                        .as_ref()
+                        .map(PermissionProfile::from),
                     Self::skill_escalation_execution(&skill),
                     decision_source,
                 )

codex-rs/core/tests/suite/skill_approval.rs

@@ -291,6 +291,12 @@ permissions:
     .await?;
 
     let (script_path_str, command) = skill_script_command(&test, "hello-mbolin.sh")?;
+    let skill_dir = PathBuf::from(&script_path_str)
+        .parent()
+        .expect("skill script parent")
+        .parent()
+        .expect("skill root")
+        .to_path_buf();
     let arguments = shell_command_arguments(&command)?;
     let mocks =
         mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command")
@@ -331,8 +337,8 @@ permissions:
         approval.additional_permissions,
         Some(PermissionProfile {
             file_system: Some(FileSystemPermissions {
-                read: Some(vec![PathBuf::from("./data")]),
-                write: Some(vec![PathBuf::from("./output")]),
+                read: Some(vec![skill_dir.join("data")]),
+                write: Some(vec![skill_dir.join("output")]),
             }),
             ..Default::default()
         })