mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Sandbox executor skill resource reads (#36121)
## Why Executor-backed skill resources must honor the active filesystem permission profile instead of reading outside its allowed roots. ## What changed - Propagate each environment's filesystem sandbox context into `skills.read` calls. - Apply turn-scoped permission grants when reading executor skill resources. - Fail closed when no matching sandbox context exists or a restricted Windows read cannot be sandboxed. - Preserve the existing resource size limit for both sandboxed and streamed reads. ## Testing - Cover reads within permitted roots and reads that succeed after a permission grant. - Verify denied references do not expose their contents and disabled Windows sandboxing is rejected. GitOrigin-RevId: 5cc679b9385b0621665ff20cf5848863cc6396a4
This commit is contained in:
1 parent
9a46fd33a0
commit
3d805abdf0
8 files changed
+322
-44
No files matched your search
@@ -1,8 +1,10 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::injection::HostSkillsCatalogInWorldState;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::LOCAL_ENVIRONMENT_ID;
|
||||
use codex_exec_server::ResolvedSelectedCapabilityRoot;
|
||||
use codex_extension_api::ConfigContributor;
|
||||
@@ -425,7 +427,12 @@ where
|
||||
session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
self.build_skill_tools(session_store, thread_store, /*executor_query*/ None)
|
||||
self.build_skill_tools(
|
||||
session_store,
|
||||
thread_store,
|
||||
/*executor_query*/ None,
|
||||
/*sandbox_contexts*/ None,
|
||||
)
|
||||
}
|
||||
|
||||
fn tools_for_step(
|
||||
@@ -452,7 +459,12 @@ where
|
||||
mcp_resources: None,
|
||||
executor_capability_discovery: None,
|
||||
});
|
||||
self.build_skill_tools(session_store, thread_store, executor_query)
|
||||
self.build_skill_tools(
|
||||
session_store,
|
||||
thread_store,
|
||||
executor_query,
|
||||
step_store.get::<HashMap<String, FileSystemSandboxContext>>(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -679,6 +691,7 @@ impl<C> SkillsExtension<C> {
|
||||
session_store: &ExtensionData,
|
||||
thread_store: &ExtensionData,
|
||||
executor_query: Option<SkillListQuery>,
|
||||
sandbox_contexts: Option<Arc<HashMap<String, FileSystemSandboxContext>>>,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
let Some(thread_state) = thread_store.get::<SkillsThreadState>() else {
|
||||
return Vec::new();
|
||||
@@ -697,6 +710,7 @@ impl<C> SkillsExtension<C> {
|
||||
thread_state,
|
||||
orchestrator_available,
|
||||
executor_query,
|
||||
sandbox_contexts,
|
||||
Arc::clone(&self.shadow_selection),
|
||||
)
|
||||
}
|
||||
@@ -742,6 +756,7 @@ impl<C> SkillsExtension<C> {
|
||||
package: entry.id.clone(),
|
||||
resource: entry.main_prompt.clone(),
|
||||
resolved_executor_roots: Vec::new(),
|
||||
sandbox: None,
|
||||
host_snapshot,
|
||||
mcp_resources,
|
||||
},
|
||||
|
||||
@@ -8,6 +8,7 @@ mod orchestrator;
|
||||
|
||||
use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_exec_server::ExecutorCapabilityDiscoverySnapshot;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_exec_server::ResolvedSelectedCapabilityRoot;
|
||||
use codex_mcp::McpResourceClient;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
@@ -46,6 +47,7 @@ pub struct SkillReadRequest {
|
||||
pub package: SkillPackageId,
|
||||
pub resource: SkillResourceId,
|
||||
pub resolved_executor_roots: Vec<ResolvedSelectedCapabilityRoot>,
|
||||
pub sandbox: Option<FileSystemSandboxContext>,
|
||||
pub host_snapshot: Option<Arc<HostSkillsSnapshot>>,
|
||||
pub mcp_resources: Option<Arc<McpResourceClient>>,
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::sync::Arc;
|
||||
use codex_core_skills::loader::load_environment_skills_from_discovery;
|
||||
use codex_core_skills::loader::load_environment_skills_from_root;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_skills::EnvironmentSkillMetadata;
|
||||
@@ -147,6 +148,7 @@ impl SkillProvider for ExecutorSkillProvider {
|
||||
file_system.as_ref(),
|
||||
resource_path,
|
||||
request.resource.as_str(),
|
||||
request.sandbox.as_ref(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -265,25 +267,58 @@ async fn read_bounded_text(
|
||||
file_system: &dyn codex_exec_server::ExecutorFileSystem,
|
||||
path: &PathUri,
|
||||
resource: &str,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> Result<String, SkillProviderError> {
|
||||
let read_error = |err| {
|
||||
SkillProviderError::new(format!(
|
||||
"failed to read executor skill resource {resource}: {err}"
|
||||
))
|
||||
};
|
||||
let mut stream = file_system
|
||||
.read_file_stream(path, /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(&read_error)?;
|
||||
let mut contents = Vec::new();
|
||||
while let Some(chunk) = stream.next().await {
|
||||
let chunk = chunk.map_err(&read_error)?;
|
||||
if contents.len().saturating_add(chunk.len()) > MAX_SKILL_RESOURCE_CONTENT_BYTES {
|
||||
let contents = if sandbox.is_some_and(FileSystemSandboxContext::should_run_in_sandbox) {
|
||||
if path.infer_path_convention() == Some(PathConvention::Windows)
|
||||
&& sandbox.is_some_and(|context| {
|
||||
context.windows_sandbox_level
|
||||
== codex_protocol::config_types::WindowsSandboxLevel::Disabled
|
||||
})
|
||||
{
|
||||
return Err(SkillProviderError::new(
|
||||
"executor skill resource requires an unavailable filesystem sandbox",
|
||||
));
|
||||
}
|
||||
let metadata = file_system
|
||||
.get_metadata(path, sandbox)
|
||||
.await
|
||||
.map_err(&read_error)?;
|
||||
if metadata.size > MAX_SKILL_RESOURCE_CONTENT_BYTES as u64 {
|
||||
return Err(SkillProviderError::new(format!(
|
||||
"executor skill resource {resource} exceeds {MAX_SKILL_RESOURCE_CONTENT_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
contents.extend_from_slice(&chunk);
|
||||
file_system
|
||||
.read_file(path, sandbox)
|
||||
.await
|
||||
.map_err(&read_error)?
|
||||
} else {
|
||||
let mut stream = file_system
|
||||
.read_file_stream(path, sandbox)
|
||||
.await
|
||||
.map_err(&read_error)?;
|
||||
let mut contents = Vec::new();
|
||||
while let Some(chunk) = stream.next().await {
|
||||
let chunk = chunk.map_err(&read_error)?;
|
||||
if contents.len().saturating_add(chunk.len()) > MAX_SKILL_RESOURCE_CONTENT_BYTES {
|
||||
return Err(SkillProviderError::new(format!(
|
||||
"executor skill resource {resource} exceeds {MAX_SKILL_RESOURCE_CONTENT_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
contents.extend_from_slice(&chunk);
|
||||
}
|
||||
contents
|
||||
};
|
||||
if contents.len() > MAX_SKILL_RESOURCE_CONTENT_BYTES {
|
||||
return Err(SkillProviderError::new(format!(
|
||||
"executor skill resource {resource} exceeds {MAX_SKILL_RESOURCE_CONTENT_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
String::from_utf8(contents).map_err(|_| {
|
||||
SkillProviderError::new(format!(
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
use std::collections::HashMap;
|
||||
use std::collections::hash_map::DefaultHasher;
|
||||
use std::hash::Hash;
|
||||
use std::hash::Hasher;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_exec_server::FileSystemSandboxContext;
|
||||
use codex_extension_api::FunctionCallError;
|
||||
use codex_extension_api::JsonToolOutput;
|
||||
use codex_extension_api::ResponsesApiTool;
|
||||
@@ -44,6 +46,7 @@ pub(crate) fn skill_tools(
|
||||
thread_state: Arc<SkillsThreadState>,
|
||||
orchestrator_available: bool,
|
||||
executor_query: Option<SkillListQuery>,
|
||||
sandbox_contexts: Option<Arc<HashMap<String, FileSystemSandboxContext>>>,
|
||||
shadow_selection: Arc<ShadowSelectionExperiment>,
|
||||
) -> Vec<Arc<dyn ToolExecutor<ToolCall>>> {
|
||||
let context = SkillToolContext {
|
||||
@@ -52,6 +55,7 @@ pub(crate) fn skill_tools(
|
||||
thread_state,
|
||||
orchestrator_available,
|
||||
executor_query,
|
||||
sandbox_contexts,
|
||||
executor_catalog: Arc::new(OnceCell::new()),
|
||||
shadow_selection,
|
||||
};
|
||||
@@ -70,6 +74,7 @@ struct SkillToolContext {
|
||||
thread_state: Arc<SkillsThreadState>,
|
||||
orchestrator_available: bool,
|
||||
executor_query: Option<SkillListQuery>,
|
||||
sandbox_contexts: Option<Arc<HashMap<String, FileSystemSandboxContext>>>,
|
||||
executor_catalog: Arc<OnceCell<SkillCatalog>>,
|
||||
shadow_selection: Arc<ShadowSelectionExperiment>,
|
||||
}
|
||||
|
||||
@@ -96,6 +96,27 @@ impl ToolExecutor<ToolCall> for ReadTool {
|
||||
.as_ref()
|
||||
.map(|query| query.resolved_executor_roots.clone())
|
||||
.unwrap_or_default();
|
||||
let sandbox = requested_resource
|
||||
.environment_path()
|
||||
.and_then(|(environment_id, _)| {
|
||||
self.context.sandbox_contexts.as_ref().and_then(|contexts| {
|
||||
contexts.get(environment_id).map(|captured| {
|
||||
call.environments
|
||||
.iter()
|
||||
.find(|environment| environment.environment_id == environment_id)
|
||||
.map(|environment| environment.file_system_sandbox_context.clone())
|
||||
.unwrap_or_else(|| captured.clone())
|
||||
})
|
||||
})
|
||||
});
|
||||
if self.context.sandbox_contexts.is_some()
|
||||
&& requested_resource.environment_path().is_some()
|
||||
&& sandbox.is_none()
|
||||
{
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"failed to read skill resource".to_string(),
|
||||
));
|
||||
}
|
||||
let result = self
|
||||
.context
|
||||
.thread_state
|
||||
@@ -106,6 +127,7 @@ impl ToolExecutor<ToolCall> for ReadTool {
|
||||
package,
|
||||
resource: requested_resource.clone(),
|
||||
resolved_executor_roots,
|
||||
sandbox,
|
||||
host_snapshot: None,
|
||||
mcp_resources: self.context.mcp_resources.clone(),
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user