From afefd0ba95c56817d78dfc0de817f442cab340a7 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Wed, 4 Nov 2026 00:57:39 +0000 Subject: [PATCH] Enforce current filesystem permissions when reading repository skills (#59278) ## Why Cached repository skill metadata must not authorize reads after access is revoked or a skill file or directory becomes a symlink to a denied location. ## What changed Read repository skill instructions through a current environment accessor for the filesystem that discovered them. Reject reads when no matching accessor is available, and cache filesystem identity instead of retaining unrestricted read access. ## Testing Add regression coverage for revoked permissions, file and directory symlink replacements, and missing or mismatched filesystem accessors. GitOrigin-RevId: 8dad83b7c7893e0d66e2fe1c0c240715db582005 --- codex-rs/core/src/session/turn.rs | 10 +- codex-rs/core/tests/suite/skills.rs | 2 - codex-rs/core/tests/suite/skills_extension.rs | 282 ++++++++++++++++++ codex-rs/ext/skills/src/extension.rs | 9 +- codex-rs/ext/skills/src/host_outcome.rs | 56 ++-- codex-rs/ext/skills/src/host_prompt.rs | 9 +- codex-rs/ext/skills/src/host_snapshot.rs | 9 +- codex-rs/ext/skills/src/loader/host.rs | 6 +- codex-rs/ext/skills/src/loader/host_merge.rs | 11 +- codex-rs/ext/skills/src/provider.rs | 3 +- codex-rs/ext/skills/src/provider/host.rs | 16 +- .../ext/skills/src/provider/host_tests.rs | 1 + .../tests/executor_file_system_authority.rs | 46 ++- .../file-system/src/environment_accessor.rs | 7 + 14 files changed, 422 insertions(+), 45 deletions(-) diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index c32b4472d9..2d3f37e616 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -84,6 +84,7 @@ use codex_extension_api::ExtensionData; use codex_extension_api::TurnInputContext; use codex_extension_api::TurnInputEnvironment; use codex_features::Feature; +use codex_file_system::EnvironmentAccess; use codex_file_system::FindUpErrorPolicy; use codex_file_system::find_nearest_ancestor_with_markers; use codex_hooks::ModelRequestHookEvent::PostModelRequest; @@ -1270,11 +1271,18 @@ async fn build_skills_and_plugins( let injected_host_skill_prompts = turn_context .extension_data .get::(); + let environments = step_context.environments(); + let accessors = environments + .iter() + .map(|(_, access)| access as &dyn EnvironmentAccess) + .collect::>(); let HostSkillPrompts { fragments, injected: injected_host_skills, warnings: host_skill_warnings, - } = skills_snapshot.load_skill_prompts(&mentioned_skills).await; + } = skills_snapshot + .load_skill_prompts(&mentioned_skills, &accessors) + .await; emit_explicit_skill_invocations( sess, turn_context, diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 1c610164b7..4708ae0ec2 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -36,7 +36,6 @@ use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::skip_if_remote; use core_test_support::skip_if_wine_exec; -use core_test_support::test_codex::local_requests; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use pretty_assertions::assert_eq; @@ -148,7 +147,6 @@ async fn user_turn_includes_skill_instructions() -> Result<()> { }, ]) .with_thread_settings(ThreadSettingsOverrides { - environments: Some(local_requests(test.config.cwd.clone())), approval_policy: Some(AskForApproval::Never), sandbox_policy: Some(sandbox_policy), permission_profile, diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index d941aae973..458f4c59d0 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -1,3 +1,7 @@ +use codex_exec_server::FileSystemEnvironmentAccessor; +use codex_exec_server::FileSystemSandboxContext; +use codex_skills_extension::HostSkillsLoadInput; +use core_test_support::test_codex::TestCodexHarness; use std::sync::Arc; use std::sync::Mutex; use std::sync::atomic::AtomicBool; @@ -774,6 +778,284 @@ async fn capability_sections_render_in_order_with_host_repo_and_plugin_skills() Ok(()) } +#[derive(Clone, Copy)] +enum CachedRepositorySkillChange { + ReplaceFileWithSymlink, + ReplaceDirectoryWithSymlink, +} + +/// Cached metadata never authorizes a live read after permissions or a skill's target changes. +#[test_case(CachedRepositorySkillChange::ReplaceFileWithSymlink; "file becomes a symlink")] +#[test_case(CachedRepositorySkillChange::ReplaceDirectoryWithSymlink; "parent becomes a symlink")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn repository_skill_cache_cannot_read_inaccessible_files( + change: CachedRepositorySkillChange, +) -> Result<()> { + skip_if_wine_exec!( + Ok(()), + "executor-backed repo skills require matching host and executor path conventions" + ); + skip_if_target_windows!( + Ok(()), + "restricted reads require the elevated Windows sandbox backend, unavailable in this fixture" + ); + skip_if_no_network!(Ok(())); + + const GATED_BODY: &str = "The gated skill's private instructions."; + let mut extensions = ExtensionRegistryBuilder::::new(); + install(&mut extensions, |config: &Config| SkillsExtensionConfig { + include_instructions: config.include_skill_instructions, + max_context_tokens: config.skill_max_context_tokens, + catalog_deltas_enabled: config.features.enabled(Feature::SkillCatalogDeltas), + omit_descriptions: false, + bundled_skills_enabled: false, + cloud_skill_enabled: false, + shadow_selection_enabled: false, + }); + let builder = test_codex() + .with_extensions(Arc::new(extensions.build())) + .with_config(configure_catalog_test) + .with_workspace_setup(|cwd, file_system| async move { + for (name, body) in [ + ("repo-allowed", "The allowed skill's instructions."), + ("repo-gated", GATED_BODY), + ("repo-revoked", GATED_BODY), + ] { + let directory = + PathUri::from_host_native_path(cwd.join(".agents/skills").join(name))?; + file_system + .create_directory( + &directory, + CreateDirectoryOptions { + recursive: true, + follow_symlinks: true, + }, + /*sandbox*/ None, + ) + .await?; + file_system + .write_file( + &directory.join("SKILL.md")?, + format!( + "---\nname: {name}\ndescription: Repository skill.\n---\n\n{body}\n" + ) + .into_bytes(), + Default::default(), + /*sandbox*/ None, + ) + .await?; + } + Ok(()) + }); + let harness = TestCodexHarness::with_auto_env_builder(builder).await?; + let test = harness.test(); + let responses = responses::mount_sse_sequence( + harness.server(), + vec![ + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]), + sse(vec![ev_response_created("resp-3"), ev_completed("resp-3")]), + ], + ) + .await; + let gated_path = test + .fs() + .canonicalize( + &PathUri::from_abs_path(&test.config.cwd.join(".agents/skills/repo-gated/SKILL.md")), + /*sandbox*/ None, + ) + .await?; + + let selection = test.executor_environment().selection(); + let environment = test + .thread_manager + .environment_manager() + .get_environment(&selection.environment_id) + .expect("configured executor environment"); + let file_system = environment.get_filesystem(); + let executor_cwd = selection.cwd.clone(); + let denied_directory = executor_cwd.join("denied")?; + harness.write_file("denied/SKILL.md", GATED_BODY).await?; + let denied_directory = file_system + .canonicalize(&denied_directory, /*sandbox*/ None) + .await?; + let initial_permissions = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new( + FileSystemPath::Path { + path: denied_directory.clone(), + }, + FileSystemAccessMode::Deny, + ), + ]), + NetworkSandboxPolicy::Restricted, + ); + let revoked_path = file_system + .canonicalize( + &executor_cwd.join(".agents/skills/repo-revoked/SKILL.md")?, + /*sandbox*/ None, + ) + .await?; + let (mut narrowed_policy, network_policy) = initial_permissions.to_runtime_permissions(); + narrowed_policy.entries.push(FileSystemSandboxEntry::new( + FileSystemPath::Path { path: revoked_path }, + FileSystemAccessMode::Deny, + )); + let narrowed_permissions = + PermissionProfile::from_runtime_permissions(&narrowed_policy, network_policy); + test.submit_turn_with_permission_profile("Inspect the repository skills.", initial_permissions) + .await?; + + let open = FileSystemEnvironmentAccessor::unrestricted(&file_system); + let snapshot = test + .thread_manager + .skills_service() + .snapshot_for_config( + &HostSkillsLoadInput::new( + test.config.cwd.clone(), + Vec::new(), + test.config.config_layer_stack.clone(), + ), + Some(Arc::clone(&file_system)), + ) + .await; + let [revoked_skill, gated_skill] = ["repo-revoked", "repo-gated"].map(|name| { + snapshot + .outcome() + .skills + .iter() + .find(|skill| skill.name == name) + .expect("first turn discovered the repository skill") + .clone() + }); + + // Revoking access hides the skill and prevents cached metadata from reopening it. + let restricted = FileSystemEnvironmentAccessor::new( + &file_system, + FileSystemSandboxContext::from_permission_profile( + narrowed_permissions.clone(), + executor_cwd.clone(), + ), + ); + test.submit_turn_with_permission_profile( + "Use $repo-revoked if it is still available.", + narrowed_permissions.clone(), + ) + .await?; + snapshot + .read_skill_text(&revoked_skill, &[&restricted]) + .await + .expect_err("old metadata cannot reopen a now-denied skill"); + assert!( + snapshot + .read_skill_text(&revoked_skill, &[&open]) + .await? + .contains(GATED_BODY) + ); + + // An allowed skill path must not bypass the denial of its new symlink target. + let (link, target) = match change { + CachedRepositorySkillChange::ReplaceFileWithSymlink => { + (gated_path, denied_directory.join("SKILL.md")?) + } + CachedRepositorySkillChange::ReplaceDirectoryWithSymlink => ( + gated_path.parent().expect("skill directory"), + denied_directory, + ), + }; + file_system + .remove( + &link, + RemoveOptions { + recursive: true, + force: false, + follow_symlinks: true, + }, + /*sandbox*/ None, + ) + .await?; + let process = test + .executor_environment() + .environment() + .get_exec_backend() + .start(ExecParams { + process_id: ProcessId::from("replace-cached-skill"), + metadata: None, + argv: vec![ + "/bin/ln".to_string(), + "-s".to_string(), + target.inferred_native_path_string(), + link.inferred_native_path_string(), + ], + cwd: executor_cwd.clone(), + env_policy: None, + shell_snapshot: None, + env: Default::default(), + tty: false, + pipe_stdin: false, + arg0: None, + sandbox: None, + enforce_managed_network: false, + managed_network: None, + network_proxy: None, + }) + .await? + .process; + let mut events = process.subscribe_events(); + loop { + match events.recv().await? { + ExecProcessEvent::Exited { exit_code, .. } => { + assert_eq!(exit_code, 0, "replace the cached skill on its executor"); + break; + } + ExecProcessEvent::Output(_) => {} + event @ (ExecProcessEvent::Closed { .. } | ExecProcessEvent::Failed(_)) => { + panic!("skill symlink process did not exit: {event:?}") + } + } + } + test.submit_turn_with_permission_profile( + "Use $repo-gated if it is still available.", + narrowed_permissions, + ) + .await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 3); + let first_developer_texts = requests[0].message_input_texts("developer"); + let first_catalog = catalog_text(&first_developer_texts, "repo"); + assert!(first_catalog.contains("- repo-revoked:")); + + let second_developer_texts = requests[1].message_input_texts("developer"); + assert!( + second_developer_texts + .iter() + .any(|text| text == first_catalog) + ); + for request in &requests[1..] { + assert!( + request + .message_input_texts("user") + .iter() + .all(|text| !text.contains(GATED_BODY)) + ); + } + + snapshot + .read_skill_text(&gated_skill, &[&restricted]) + .await + .expect_err("old metadata cannot reopen a now-denied skill"); + let restored = snapshot.read_skill_text(&gated_skill, &[&open]).await?; + assert!(restored.contains(GATED_BODY)); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn agent_plugin_skill_prompt_stays_bounded_without_skills_extension() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index f0fdee286d..29ba0065b7 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -541,7 +541,14 @@ impl SkillsExtension { let context = match entry.authority.kind { SkillSourceKind::Cloud => SkillReadContext::Cloud { mcp_resources }, SkillSourceKind::Custom(_) => SkillReadContext::Custom { mcp_resources }, - SkillSourceKind::Host => SkillReadContext::Host { host_snapshot }, + SkillSourceKind::Host => SkillReadContext::Host { + host_snapshot, + accessors: input + .environments + .iter() + .map(|environment| environment.fs) + .collect(), + }, SkillSourceKind::Executor => { let fs = entry .main_prompt diff --git a/codex-rs/ext/skills/src/host_outcome.rs b/codex-rs/ext/skills/src/host_outcome.rs index 96d16214de..4461e0cd94 100644 --- a/codex-rs/ext/skills/src/host_outcome.rs +++ b/codex-rs/ext/skills/src/host_outcome.rs @@ -4,9 +4,13 @@ use std::fmt; use std::io; use std::sync::Arc; -use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::EnvironmentAccess; +use codex_exec_server::EnvironmentAccessExt; +use codex_exec_server::EnvironmentAccessKey; +use codex_exec_server::FileSystemEnvironmentAccessor; use codex_exec_server::LOCAL_FS; use codex_exec_server::ReadFileOptions; +use codex_protocol::protocol::SkillScope; use codex_skills::SkillError; use codex_skills::SkillMetadata; use codex_utils_absolute_path::AbsolutePathBuf; @@ -35,7 +39,7 @@ impl SkillLoadOutcome { skill_root_by_path: HashMap, skill_discovery_path_by_path: HashMap, agent_plugin_skill_paths: HashSet, - file_systems_by_skill_path: HashMap>, + file_systems_by_skill_path: HashMap, ) -> Self { Self { skills, @@ -108,22 +112,38 @@ impl SkillLoadOutcome { pub(crate) fn file_system_for_skill( &self, skill: &SkillMetadata, - ) -> Option> { + ) -> Option<&EnvironmentAccessKey> { self.file_systems_by_skill_path .get(&skill.path_to_skills_md) } - /// Reads one loaded skill through the filesystem that discovered it. - pub(crate) async fn read_skill_text(&self, skill: &SkillMetadata) -> io::Result { - let fs = self - .file_system_for_skill(skill) - .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - fs.read_file_text( - &skill.path_to_skills_md, - ReadFileOptions::default(), - /*sandbox*/ None, - ) - .await + /// Reads through current access to the original filesystem; cached metadata grants no access. + pub(crate) async fn read_skill_text( + &self, + skill: &SkillMetadata, + accessors: &[&dyn EnvironmentAccess], + ) -> io::Result { + let host = FileSystemEnvironmentAccessor::unrestricted(&LOCAL_FS); + let access: &dyn EnvironmentAccess = match skill.scope { + SkillScope::Repo => self + .file_system_for_skill(skill) + .and_then(|source| { + accessors + .iter() + .copied() + .find(|access| source.same_file_system(&access.cache_key())) + }) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::PermissionDenied, + "skill discovery filesystem is unavailable for this callback", + ) + })?, + SkillScope::User | SkillScope::System | SkillScope::Admin => &host, + }; + access + .read_file_text(&skill.path_to_skills_md, ReadFileOptions::default()) + .await } } @@ -159,18 +179,18 @@ impl codex_skills::ExplicitSkillLookup for SkillLoadOutcome { #[derive(Clone, Default)] pub(crate) struct SkillFileSystemsByPath { - values: Arc>>, + values: Arc>, } impl SkillFileSystemsByPath { - pub(crate) fn new(values: HashMap>) -> Self { + pub(crate) fn new(values: HashMap) -> Self { Self { values: Arc::new(values), } } - fn get(&self, path: &PathUri) -> Option> { - self.values.get(path).map(Arc::clone) + fn get(&self, path: &PathUri) -> Option<&EnvironmentAccessKey> { + self.values.get(path) } } diff --git a/codex-rs/ext/skills/src/host_prompt.rs b/codex-rs/ext/skills/src/host_prompt.rs index ea447a2dbf..7d5a62f59d 100644 --- a/codex-rs/ext/skills/src/host_prompt.rs +++ b/codex-rs/ext/skills/src/host_prompt.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; +use codex_exec_server::EnvironmentAccess; use codex_extension_api::ContextualUserFragment; use codex_skills::SkillMetadata; use codex_skills::normalize_skill_path; @@ -66,7 +67,11 @@ impl HostSkillsSnapshot { skip_all, fields(selected_skill_count = selected_skills.len()) )] - pub async fn load_skill_prompts(&self, selected_skills: &[SkillMetadata]) -> HostSkillPrompts { + pub async fn load_skill_prompts( + &self, + selected_skills: &[SkillMetadata], + accessors: &[&dyn EnvironmentAccess], + ) -> HostSkillPrompts { let mut prompts = HostSkillPrompts { fragments: Vec::with_capacity(selected_skills.len()), injected: Vec::with_capacity(selected_skills.len()), @@ -74,7 +79,7 @@ impl HostSkillsSnapshot { }; for skill in selected_skills { - match self.read_skill_text(skill).await { + match self.read_skill_text(skill, accessors).await { Ok(contents) => { let (contents, truncated) = if self.outcome().is_agent_plugin_skill(skill) { truncate_main_prompt_contents(&contents) diff --git a/codex-rs/ext/skills/src/host_snapshot.rs b/codex-rs/ext/skills/src/host_snapshot.rs index 1bdabb033a..efa75ab093 100644 --- a/codex-rs/ext/skills/src/host_snapshot.rs +++ b/codex-rs/ext/skills/src/host_snapshot.rs @@ -2,6 +2,7 @@ use std::io; use std::sync::Arc; use crate::SkillLoadOutcome; +use codex_exec_server::EnvironmentAccess; use codex_skills::SkillMetadata; /// Immutable snapshot of host-owned skills and their source filesystems. @@ -19,7 +20,11 @@ impl HostSkillsSnapshot { self.outcome.as_ref() } - pub async fn read_skill_text(&self, skill: &SkillMetadata) -> io::Result { - self.outcome.read_skill_text(skill).await + pub async fn read_skill_text( + &self, + skill: &SkillMetadata, + accessors: &[&dyn EnvironmentAccess], + ) -> io::Result { + self.outcome.read_skill_text(skill, accessors).await } } diff --git a/codex-rs/ext/skills/src/loader/host.rs b/codex-rs/ext/skills/src/loader/host.rs index 168513dec7..e89c5de1b5 100644 --- a/codex-rs/ext/skills/src/loader/host.rs +++ b/codex-rs/ext/skills/src/loader/host.rs @@ -1,6 +1,8 @@ use std::collections::HashMap; use std::sync::Arc; +use codex_exec_server::EnvironmentAccess; +use codex_exec_server::EnvironmentAccessKey; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::FileSystemEnvironmentAccessor; use codex_exec_server::GetMetadataOptions; @@ -117,7 +119,7 @@ pub(crate) struct HostSkillRootSnapshot { pub(crate) skills: Vec, pub(crate) skill_discovery_path_by_path: Arc>, pub(crate) errors: Vec, - pub(crate) file_system: Arc, + pub(crate) file_system: EnvironmentAccessKey, pub(crate) is_agent_plugin: bool, } @@ -138,7 +140,7 @@ pub(crate) async fn load_host_skill_root(root: HostSkillRoot) -> HostSkillRootSn skills, skill_discovery_path_by_path, errors, - file_system: root.file_system, + file_system: FileSystemEnvironmentAccessor::unrestricted(&root.file_system).cache_key(), is_agent_plugin, } } diff --git a/codex-rs/ext/skills/src/loader/host_merge.rs b/codex-rs/ext/skills/src/loader/host_merge.rs index 9f13644868..56ad696324 100644 --- a/codex-rs/ext/skills/src/loader/host_merge.rs +++ b/codex-rs/ext/skills/src/loader/host_merge.rs @@ -2,7 +2,9 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; -use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::EnvironmentAccess; +use codex_exec_server::EnvironmentAccessKey; +use codex_exec_server::FileSystemEnvironmentAccessor; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_skills::LoadedSkillRoot; @@ -88,7 +90,8 @@ pub(crate) async fn load_and_merge_host_skill_roots_with_request_snapshots( skills: snapshot.skills, skill_discovery_path_by_path: snapshot.skill_discovery_path_by_path, errors: snapshot.errors, - file_system: Arc::clone(&file_system), + file_system: FileSystemEnvironmentAccessor::unrestricted(&file_system) + .cache_key(), is_agent_plugin: snapshot.is_agent_plugin, }, None => { @@ -202,7 +205,7 @@ fn merge_host_skill_root_snapshots(snapshots: Vec) -> Ski let mut skill_root_by_path = HashMap::new(); let mut skill_discovery_path_by_path = HashMap::new(); let mut agent_plugin_skill_paths = HashSet::new(); - let mut file_systems_by_skill_path = HashMap::>::new(); + let mut file_systems_by_skill_path = HashMap::::new(); for snapshot in snapshots { if !snapshot.skills.is_empty() && !skill_roots.contains(&snapshot.root) { @@ -215,7 +218,7 @@ fn merge_host_skill_root_snapshots(snapshots: Vec) -> Ski if let Some(discovery_path) = snapshot.skill_discovery_path_by_path.get(&path) { skill_discovery_path_by_path.insert(path.clone(), discovery_path.clone()); } - file_systems_by_skill_path.insert(path.clone(), Arc::clone(&snapshot.file_system)); + file_systems_by_skill_path.insert(path.clone(), snapshot.file_system.clone()); if snapshot.is_agent_plugin { agent_plugin_skill_paths.insert(path); } diff --git a/codex-rs/ext/skills/src/provider.rs b/codex-rs/ext/skills/src/provider.rs index 260d8476c2..09e6419ec7 100644 --- a/codex-rs/ext/skills/src/provider.rs +++ b/codex-rs/ext/skills/src/provider.rs @@ -54,11 +54,12 @@ pub struct SkillReadRequest<'a> { pub context: SkillReadContext<'a>, } -/// Executor reads require callback access; other providers carry their own source context. +/// Repository and executor reads require callback access; other sources retain provider context. #[derive(Clone)] pub enum SkillReadContext<'a> { Host { host_snapshot: Option>, + accessors: Vec<&'a dyn EnvironmentAccess>, }, Executor { fs: &'a dyn EnvironmentAccess, diff --git a/codex-rs/ext/skills/src/provider/host.rs b/codex-rs/ext/skills/src/provider/host.rs index a5dd192b30..d37a4803be 100644 --- a/codex-rs/ext/skills/src/provider/host.rs +++ b/codex-rs/ext/skills/src/provider/host.rs @@ -55,6 +55,7 @@ impl SkillProvider for HostSkillProvider { Box::pin(async move { let SkillReadContext::Host { host_snapshot: Some(host_snapshot), + accessors, } = request.context else { return Err(SkillProviderError::new( @@ -78,12 +79,15 @@ impl SkillProvider for HostSkillProvider { ))); }; - let contents = host_snapshot.read_skill_text(skill).await.map_err(|err| { - SkillProviderError::new(format!( - "failed to read host skill resource {}: {err}", - request.resource.as_str() - )) - })?; + let contents = host_snapshot + .read_skill_text(skill, &accessors) + .await + .map_err(|err| { + SkillProviderError::new(format!( + "failed to read host skill resource {}: {err}", + request.resource.as_str() + )) + })?; Ok(SkillReadResult { resource: request.resource, diff --git a/codex-rs/ext/skills/src/provider/host_tests.rs b/codex-rs/ext/skills/src/provider/host_tests.rs index 4a6c577097..818dbfc3cc 100644 --- a/codex-rs/ext/skills/src/provider/host_tests.rs +++ b/codex-rs/ext/skills/src/provider/host_tests.rs @@ -110,6 +110,7 @@ async fn host_catalog_resource_spellings_read_the_discovered_skill() package: entry.id.clone(), resource: resource.clone(), context: SkillReadContext::Host { + accessors: Vec::new(), host_snapshot: Some(Arc::clone(&snapshot)), }, }) diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 24c067b023..2265ed1519 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -287,14 +287,16 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { /*bundled_skills_enabled*/ false, /*restriction_product*/ None, ); + let file_system: Arc = Arc::new(SyntheticFileSystem { + alias_root: PathUri::from_abs_path(&alias_root), + canonical_root: PathUri::from_abs_path(&canonical_root), + has_plugin_manifest: false, + }); + let access = FileSystemEnvironmentAccessor::unrestricted(&file_system); let snapshot = service .snapshot_for_config( &HostSkillsLoadInput::new(cwd, Vec::new(), config_layer_stack), - Some(Arc::new(SyntheticFileSystem { - alias_root: PathUri::from_abs_path(&alias_root), - canonical_root: PathUri::from_abs_path(&canonical_root), - has_plugin_manifest: false, - })), + Some(Arc::clone(&file_system)), ) .await; let outcome = snapshot.outcome(); @@ -308,9 +310,41 @@ async fn skill_loading_and_reads_use_the_supplied_executor_file_system() { PathUri::from_abs_path(&canonical_root.join("skill/SKILL.md")) ); assert_eq!( - snapshot.read_skill_text(&skill).await.expect("skill body"), + snapshot + .read_skill_text(&skill, &[&access]) + .await + .expect("skill body"), SKILL_CONTENTS ); + assert_eq!( + snapshot + .read_skill_text(&skill, &[]) + .await + .unwrap_err() + .kind(), + io::ErrorKind::PermissionDenied, + ); + let replacement: Arc = Arc::new(SyntheticFileSystem { + alias_root: PathUri::from_abs_path(&alias_root), + canonical_root: PathUri::from_abs_path(&canonical_root), + has_plugin_manifest: false, + }); + let replacement = FileSystemEnvironmentAccessor::unrestricted(&replacement); + assert_eq!( + snapshot + .read_skill_text(&skill, &[&replacement]) + .await + .unwrap_err() + .kind(), + io::ErrorKind::PermissionDenied, + ); + assert_eq!( + snapshot + .read_skill_text(&skill, &[&replacement, &access]) + .await + .expect("select the original filesystem regardless of accessor order"), + SKILL_CONTENTS, + ); } #[tokio::test] diff --git a/codex-rs/file-system/src/environment_accessor.rs b/codex-rs/file-system/src/environment_accessor.rs index 06f63ede9e..caed0c180c 100644 --- a/codex-rs/file-system/src/environment_accessor.rs +++ b/codex-rs/file-system/src/environment_accessor.rs @@ -130,6 +130,13 @@ pub struct EnvironmentAccessKey { sandbox: Option>, } +impl EnvironmentAccessKey { + /// Compares filesystem identity without comparing captured permissions. + pub fn same_file_system(&self, other: &Self) -> bool { + self.file_system.ptr_eq(&other.file_system) + } +} + impl fmt::Debug for EnvironmentAccessKey { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("EnvironmentAccessKey(..)")