Honor filesystem permissions for app file uploads

codex-rs/core/src/mcp_openai_file.rs

@@ -16,8 +16,10 @@ use crate::session::step_context::StepContext;
 use codex_api::HostedFileUploadContext;
 use codex_api::OPENAI_FILE_UPLOAD_LIMIT_BYTES;
 use codex_api::upload_openai_file;
-use codex_http_client::RouteAwareClientPool;
 use codex_login::CodexAuth;
+use codex_protocol::permissions::FileSystemAccessMode;
+use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
+use codex_sandboxing::policy_transforms::merge_permission_profiles;
 use serde_json::Value as JsonValue;
 use std::collections::HashMap;
 
@@ -51,8 +53,8 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
             continue;
         };
         let Some(uploaded_value) = rewrite_argument_value_for_openai_files(
+            sess,
             step_context,
-            &sess.services.openai_file_upload_client_pool,
             auth.as_ref(),
             field_name,
             optional_fields,
@@ -74,8 +76,8 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
 }
 
 async fn rewrite_argument_value_for_openai_files(
+    sess: &Session,
     step_context: &StepContext,
-    client_pool: &RouteAwareClientPool,
     auth: Option<&CodexAuth>,
     field_name: &str,
     optional_fields: &[String],
@@ -85,8 +87,8 @@ async fn rewrite_argument_value_for_openai_files(
     match value {
         JsonValue::String(file_path) => {
             let rewritten = build_uploaded_argument_value(
+                sess,
                 step_context,
-                client_pool,
                 auth,
                 FileArgumentLocation {
                     field_name,
@@ -106,8 +108,8 @@ async fn rewrite_argument_value_for_openai_files(
                     return Ok(None);
                 };
                 let rewritten = build_uploaded_argument_value(
+                    sess,
                     step_context,
-                    client_pool,
                     auth,
                     FileArgumentLocation {
                         field_name,
@@ -127,8 +129,8 @@ async fn rewrite_argument_value_for_openai_files(
 }
 
 async fn build_uploaded_argument_value(
+    sess: &Session,
     step_context: &StepContext,
-    client_pool: &RouteAwareClientPool,
     auth: Option<&CodexAuth>,
     argument: FileArgumentLocation<'_>,
     optional_fields: &[String],
@@ -158,9 +160,43 @@ async fn build_uploaded_argument_value(
         .cwd()
         .join(file_path)
         .map_err(|error| contextualize_error(error.to_string()))?;
+    let additional_permissions = merge_permission_profiles(
+        sess.granted_session_permissions(&turn_environment.environment_id)
+            .await
+            .as_ref(),
+        sess.granted_turn_permissions(&turn_environment.environment_id)
+            .await
+            .as_ref(),
+    );
+    let file_system_policy = effective_file_system_sandbox_policy(
+        &turn_environment
+            .permission_profile()
+            .file_system_sandbox_policy(),
+        additional_permissions.as_ref(),
+    );
+    let requires_sandbox = !file_system_policy.has_full_disk_read_access()
+        || file_system_policy
+            .entries
+            .iter()
+            .any(|entry| entry.access == FileSystemAccessMode::Deny);
+    let sandbox = requires_sandbox.then(|| {
+        turn_context.file_system_sandbox_context(additional_permissions, turn_environment)
+    });
+    if sandbox.is_some() {
+        let environment_info = turn_environment
+            .environment
+            .info()
+            .await
+            .map_err(|error| contextualize_error(error.to_string()))?;
+        if !environment_info.capabilities.sandboxed_file_streaming {
+            return Err(contextualize_error(
+                "selected executor does not support sandboxed file streaming".to_string(),
+            ));
+        }
+    }
     let fs = turn_environment.environment.get_filesystem();
     let metadata = fs
-        .get_metadata(&path_uri, /*sandbox*/ None)
+        .get_metadata(&path_uri, sandbox.as_ref())
         .await
         .map_err(|error| contextualize_error(error.to_string()))?;
     if !metadata.is_file {
@@ -178,7 +214,7 @@ async fn build_uploaded_argument_value(
         )));
     }
     let contents = fs
-        .read_file_stream(&path_uri, /*sandbox*/ None)
+        .read_file_stream(&path_uri, sandbox.as_ref())
         .await
         .map_err(|error| contextualize_error(error.to_string()))?;
     let file_name = path_uri
@@ -196,7 +232,7 @@ async fn build_uploaded_argument_value(
     let uploaded = upload_openai_file(
         turn_context.config.chatgpt_base_url.trim_end_matches('/'),
         upload_auth.as_ref(),
-        client_pool,
+        &sess.services.openai_file_upload_client_pool,
         file_name,
         metadata.size,
         contents,
@@ -368,8 +404,8 @@ mod tests {
             .environments = step_environments;
 
         let rewritten = build_uploaded_argument_value(
+            &session,
             &step_context,
-            &session.services.openai_file_upload_client_pool,
             Some(&auth),
             FileArgumentLocation {
                 field_name: "file",
@@ -406,8 +442,8 @@ mod tests {
         let step_context = StepContext::for_test(Arc::new(turn_context));
 
         let error = build_uploaded_argument_value(
+            &session,
             &step_context,
-            &session.services.openai_file_upload_client_pool,
             Some(&auth),
             FileArgumentLocation {
                 field_name: "file",
@@ -483,8 +519,8 @@ mod tests {
         turn_context.config = Arc::new(config);
         let step_context = StepContext::for_test(Arc::new(turn_context));
         let rewritten = rewrite_argument_value_for_openai_files(
+            &session,
             &step_context,
-            &session.services.openai_file_upload_client_pool,
             Some(&auth),
             "file",
             &[],
@@ -597,8 +633,8 @@ mod tests {
         turn_context.config = Arc::new(config);
         let step_context = StepContext::for_test(Arc::new(turn_context));
         let rewritten = rewrite_argument_value_for_openai_files(
+            &session,
             &step_context,
-            &session.services.openai_file_upload_client_pool,
             Some(&auth),
             "files",
             &[],

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

@@ -5,9 +5,17 @@ use std::path::Path;
 
 use anyhow::Context;
 use anyhow::Result;
+use codex_core::config::Config;
 use codex_protocol::models::PermissionProfile;
+use codex_protocol::permissions::FileSystemAccessMode;
+use codex_protocol::permissions::FileSystemPath;
+use codex_protocol::permissions::FileSystemSandboxEntry;
+use codex_protocol::permissions::FileSystemSandboxPolicy;
+use codex_protocol::permissions::NetworkSandboxPolicy;
 use codex_protocol::protocol::AskForApproval;
+use codex_utils_absolute_path::AbsolutePathBuf;
 use codex_utils_path_uri::PathUri;
+use core_test_support::PathExt;
 use core_test_support::apps_test_server::AppsTestServer;
 use core_test_support::apps_test_server::CALENDAR_EXTRACT_TEXT_TOOL_NAME;
 use core_test_support::apps_test_server::DIRECT_CALENDAR_EXTRACT_TEXT_TOOL as DOCUMENT_EXTRACT_HOOK_MATCHER;
@@ -27,6 +35,8 @@ use core_test_support::responses::mount_sse_sequence;
 use core_test_support::responses::namespace_child_tool;
 use core_test_support::responses::sse;
 use core_test_support::responses::start_mock_server;
+use core_test_support::skip_if_sandbox;
+use core_test_support::skip_if_target_windows;
 use core_test_support::test_codex::TestCodex;
 use pretty_assertions::assert_eq;
 use serde_json::Value;
@@ -42,6 +52,37 @@ use wiremock::matchers::path;
 
 const STREAMED_FILE_SIZE: usize = 2 * 1024 * 1024;
 
+fn restrict_apps_upload_reads(config: &mut Config, denied_file_name: &str) {
+    let denied_path =
+        AbsolutePathBuf::from_absolute_path(config.cwd.as_path().join(denied_file_name))
+            .expect("denied file path should be absolute");
+    let mut file_system_policy = FileSystemSandboxPolicy::read_only();
+    file_system_policy.entries.push(FileSystemSandboxEntry::new(
+        FileSystemPath::Path { path: denied_path },
+        FileSystemAccessMode::Deny,
+    ));
+    config
+        .permissions
+        .set_permission_profile(PermissionProfile::from_runtime_permissions(
+            &file_system_policy,
+            NetworkSandboxPolicy::Restricted,
+        ))
+        .expect("test config should allow a restricted read policy");
+
+    let user_config_path = config.codex_home.join("config.toml").abs();
+    let user_config = toml::from_str(
+        r#"
+[apps.calendar]
+default_tools_approval_mode = "approve"
+"#,
+    )
+    .expect("Apps approval config should parse");
+    config.config_layer_stack = config
+        .config_layer_stack
+        .with_user_config(&user_config_path, user_config)
+        .expect("Apps approval config should be valid");
+}
+
 fn write_post_tool_use_hook(home: &Path) -> Result<()> {
     let script_path = home.join("post_tool_use_hook.py");
     let log_path = home.join("post_tool_use_hook_log.jsonl");
@@ -135,7 +176,11 @@ async fn mount_file_upload_mocks(server: &MockServer, file_size_bytes: u64) {
         .await;
 }
 
-async fn run_extract_turn(test: &TestCodex, server: &MockServer) -> Result<ResponseMock> {
+async fn run_extract_turn(
+    test: &TestCodex,
+    server: &MockServer,
+    permission_profile: PermissionProfile,
+) -> Result<ResponseMock> {
     let mock = mount_sse_sequence(
         server,
         vec![
@@ -172,7 +217,7 @@ async fn run_extract_turn(test: &TestCodex, server: &MockServer) -> Result<Respo
     test.submit_turn_with_approval_and_permission_profile(
         "Extract the report text with the app tool.",
         AskForApproval::Never,
-        PermissionProfile::Disabled,
+        permission_profile,
     )
     .await?;
 
@@ -197,7 +242,7 @@ async fn codex_apps_file_params_omit_fields_absent_from_tool_schema() -> Result<
             Ok(())
         });
     let test = builder.build_with_auto_env(&server).await?;
-    let mock = run_extract_turn(&test, &server).await?;
+    let mock = run_extract_turn(&test, &server, PermissionProfile::Disabled).await?;
 
     let requests = mock.requests();
     let search_output = requests[1].tool_search_output("extract-search-1");
@@ -269,7 +314,7 @@ async fn codex_apps_file_params_pass_uploaded_file_to_post_tool_use_hook() -> Re
         });
     let test = builder.build(&server).await?;
     tokio::fs::write(test.cwd.path().join("report.txt"), b"hello world").await?;
-    let _responses = run_extract_turn(&test, &server).await?;
+    let _responses = run_extract_turn(&test, &server, PermissionProfile::Disabled).await?;
 
     let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?;
     assert_eq!(hook_inputs.len(), 1);
@@ -281,3 +326,86 @@ async fn codex_apps_file_params_pass_uploaded_file_to_post_tool_use_hook() -> Re
     server.verify().await;
     Ok(())
 }
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn codex_apps_file_params_stream_allowed_file_under_restricted_read_policy() -> Result<()> {
+    skip_if_target_windows!(
+        Ok(()),
+        "Windows restricted-token sandbox cannot enforce deny-read policies"
+    );
+    skip_if_sandbox!(Ok(()));
+
+    let server = start_mock_server().await;
+    let apps_server = AppsTestServer::mount(&server).await?;
+    mount_file_upload_mocks(&server, STREAMED_FILE_SIZE as u64).await;
+
+    let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url)
+        .with_config(|config| restrict_apps_upload_reads(config, "private.txt"))
+        .with_workspace_setup(|cwd, fs| async move {
+            let report_path = PathUri::from_abs_path(&cwd.join("report.txt"));
+            fs.write_file(
+                &report_path,
+                vec![b'x'; STREAMED_FILE_SIZE],
+                /*sandbox*/ None,
+            )
+            .await?;
+            Ok(())
+        });
+    let test = builder.build_with_auto_env(&server).await?;
+    let permission_profile = test.config.permissions.permission_profile().clone();
+
+    run_extract_turn(&test, &server, permission_profile).await?;
+
+    let apps_tool_call =
+        recorded_apps_tool_call_by_name(&server, CALENDAR_EXTRACT_TEXT_TOOL_NAME).await;
+    assert_eq!(
+        apps_tool_call.pointer("/params/arguments/file"),
+        Some(&schema_filtered_uploaded_file(&server))
+    );
+    server.verify().await;
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn codex_apps_file_params_reject_denied_file_before_upload() -> Result<()> {
+    skip_if_target_windows!(
+        Ok(()),
+        "Windows restricted-token sandbox cannot enforce deny-read policies"
+    );
+    skip_if_sandbox!(Ok(()));
+
+    let server = start_mock_server().await;
+    let apps_server = AppsTestServer::mount(&server).await?;
+    let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url)
+        .with_config(|config| restrict_apps_upload_reads(config, "report.txt"))
+        .with_workspace_setup(|cwd, fs| async move {
+            let report_path = PathUri::from_abs_path(&cwd.join("report.txt"));
+            fs.write_file(&report_path, b"private".to_vec(), /*sandbox*/ None)
+                .await?;
+            Ok(())
+        });
+    let test = builder.build_with_auto_env(&server).await?;
+    let permission_profile = test.config.permissions.permission_profile().clone();
+
+    let responses = run_extract_turn(&test, &server, permission_profile).await?;
+    let requests = responses.requests();
+    let output = requests[2]
+        .function_call_output_text("extract-call-1")
+        .context("denied Apps upload should return a tool error")?;
+
+    assert!(
+        output.contains("failed to upload `report.txt`")
+            && (output.contains("Permission denied") || output.contains("Operation not permitted")),
+        "unexpected upload error: {output}"
+    );
+    assert!(
+        server
+            .received_requests()
+            .await
+            .context("mock server should expose received requests")?
+            .iter()
+            .all(|request| request.url.path() != "/files"),
+        "denied files must not start an upload"
+    );
+    Ok(())
+}