mirror of
https://github.com/openai/codex.git
synced 2026-10-24 18:18:41 +00:00
Migrate ExecutorFileSystem read_file to PathUri
This commit is contained in:
@@ -882,6 +882,7 @@ mod tests {
|
||||
std::env::current_exe().expect("current exe").as_path(),
|
||||
)
|
||||
.expect("absolute current exe");
|
||||
let path = codex_utils_path_uri::PathUri::from_abs_path(&path).expect("path URI");
|
||||
let sandbox = crate::FileSystemSandboxContext::from_permission_profile(
|
||||
codex_protocol::models::PermissionProfile::from_runtime_permissions(
|
||||
&codex_protocol::permissions::FileSystemSandboxPolicy::restricted(Vec::new()),
|
||||
|
||||
@@ -189,8 +189,10 @@ pub(crate) async fn run_direct_request(
|
||||
let file_system = DirectFileSystem;
|
||||
match request {
|
||||
FsHelperRequest::ReadFile(params) => {
|
||||
let path =
|
||||
codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?;
|
||||
let data = file_system
|
||||
.read_file(¶ms.path, /*sandbox*/ None)
|
||||
.read_file(&path, /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsHelperPayload::ReadFile(FsReadFileResponse {
|
||||
|
||||
@@ -91,7 +91,7 @@ impl ExecutorFileSystem for LocalFileSystem {
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<Vec<u8>> {
|
||||
let (file_system, sandbox) = self.file_system_for(sandbox)?;
|
||||
@@ -173,7 +173,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<Vec<u8>> {
|
||||
reject_platform_sandbox_context(sandbox)?;
|
||||
@@ -271,10 +271,11 @@ impl ExecutorFileSystem for DirectFileSystem {
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<Vec<u8>> {
|
||||
reject_sandbox_context(sandbox)?;
|
||||
let path = path.to_abs_path()?;
|
||||
let metadata = tokio::fs::metadata(path.as_path()).await?;
|
||||
if metadata.len() > MAX_READ_FILE_BYTES {
|
||||
return Err(io::Error::new(
|
||||
@@ -599,3 +600,7 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, any(unix, windows)))]
|
||||
#[path = "local_file_system_path_uri_tests.rs"]
|
||||
mod path_uri_tests;
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::io;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[tokio::test]
|
||||
async fn direct_file_system_rejects_non_native_uri_as_invalid_input() {
|
||||
let error = DirectFileSystem
|
||||
.read_file(&non_native_uri(), /*sandbox*/ None)
|
||||
.await
|
||||
.expect_err("non-native URI should be rejected");
|
||||
|
||||
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
|
||||
}
|
||||
|
||||
fn non_native_uri() -> PathUri {
|
||||
#[cfg(unix)]
|
||||
let uri = "file://server/share/file.txt";
|
||||
#[cfg(windows)]
|
||||
let uri = "file:///usr/local/file.txt";
|
||||
|
||||
match PathUri::parse(uri) {
|
||||
Ok(uri) => uri,
|
||||
Err(err) => panic!("valid non-native URI should parse: {err}"),
|
||||
}
|
||||
}
|
||||
@@ -61,14 +61,15 @@ impl ExecutorFileSystem for RemoteFileSystem {
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<Vec<u8>> {
|
||||
trace!("remote fs read_file");
|
||||
let path = path.to_abs_path()?;
|
||||
let client = self.client.get().await.map_err(map_remote_error)?;
|
||||
let response = client
|
||||
.fs_read_file(FsReadFileParams {
|
||||
path: path.clone(),
|
||||
path,
|
||||
sandbox: remote_sandbox_context(sandbox),
|
||||
})
|
||||
.await
|
||||
@@ -322,3 +323,7 @@ mod tests {
|
||||
AbsolutePathBuf::from_absolute_path(&path).expect("absolute path")
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, any(unix, windows)))]
|
||||
#[path = "remote_file_system_path_uri_tests.rs"]
|
||||
mod path_uri_tests;
|
||||
|
||||
@@ -0,0 +1,225 @@
|
||||
#![allow(clippy::expect_used)]
|
||||
|
||||
#[cfg(windows)]
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
#[cfg(windows)]
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
#[cfg(windows)]
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
#[cfg(windows)]
|
||||
use futures::SinkExt;
|
||||
#[cfg(windows)]
|
||||
use futures::StreamExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::io;
|
||||
#[cfg(windows)]
|
||||
use tokio::net::TcpListener;
|
||||
#[cfg(windows)]
|
||||
use tokio::net::TcpStream;
|
||||
#[cfg(windows)]
|
||||
use tokio::sync::oneshot;
|
||||
#[cfg(windows)]
|
||||
use tokio::time::Duration;
|
||||
#[cfg(windows)]
|
||||
use tokio::time::timeout;
|
||||
#[cfg(windows)]
|
||||
use tokio_tungstenite::WebSocketStream;
|
||||
#[cfg(windows)]
|
||||
use tokio_tungstenite::accept_async;
|
||||
#[cfg(windows)]
|
||||
use tokio_tungstenite::tungstenite::Message;
|
||||
|
||||
use super::*;
|
||||
use crate::client_api::ExecServerTransportParams;
|
||||
#[cfg(windows)]
|
||||
use crate::protocol::FS_READ_FILE_METHOD;
|
||||
#[cfg(windows)]
|
||||
use crate::protocol::FsReadFileParams;
|
||||
#[cfg(windows)]
|
||||
use crate::protocol::FsReadFileResponse;
|
||||
#[cfg(windows)]
|
||||
use crate::protocol::INITIALIZE_METHOD;
|
||||
#[cfg(windows)]
|
||||
use crate::protocol::INITIALIZED_METHOD;
|
||||
#[cfg(windows)]
|
||||
use crate::protocol::InitializeResponse;
|
||||
|
||||
#[tokio::test]
|
||||
async fn non_native_uri_is_rejected_before_connecting() {
|
||||
let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new(
|
||||
ExecServerTransportParams::websocket_url("not a websocket URL".to_string()),
|
||||
));
|
||||
|
||||
let error = file_system
|
||||
.read_file(&non_native_uri(), /*sandbox*/ None)
|
||||
.await
|
||||
.expect_err("non-native URI should be rejected");
|
||||
|
||||
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn remote_file_system_sends_explicit_windows_native_paths() {
|
||||
let (websocket_url, captured_paths, server) = record_read_file_paths(2).await;
|
||||
let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new(
|
||||
ExecServerTransportParams::websocket_url(websocket_url),
|
||||
));
|
||||
let paths = vec![
|
||||
(
|
||||
PathUri::parse("file:///C:/Users/Alice/src/main.rs").expect("valid drive URI"),
|
||||
absolute_windows_path(r"C:\Users\Alice\src\main.rs"),
|
||||
),
|
||||
(
|
||||
PathUri::parse("file://server/share/src/main.rs").expect("valid UNC URI"),
|
||||
absolute_windows_path(r"\\server\share\src\main.rs"),
|
||||
),
|
||||
];
|
||||
let expected_paths = paths
|
||||
.iter()
|
||||
.map(|(_, expected_path)| expected_path.clone())
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
for (path, _) in paths {
|
||||
assert_eq!(
|
||||
file_system
|
||||
.read_file(&path, /*sandbox*/ None)
|
||||
.await
|
||||
.expect("remote read should succeed"),
|
||||
Vec::<u8>::new()
|
||||
);
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
captured_paths.await.expect("captured paths"),
|
||||
expected_paths
|
||||
);
|
||||
server.await.expect("recording server should succeed");
|
||||
}
|
||||
|
||||
fn non_native_uri() -> PathUri {
|
||||
#[cfg(unix)]
|
||||
let uri = "file://server/share/file.txt";
|
||||
#[cfg(windows)]
|
||||
let uri = "file:///usr/local/file.txt";
|
||||
|
||||
PathUri::parse(uri).expect("valid non-native URI")
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn absolute_windows_path(path: &str) -> AbsolutePathBuf {
|
||||
AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute Windows path")
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
async fn record_read_file_paths(
|
||||
expected_requests: usize,
|
||||
) -> (
|
||||
String,
|
||||
oneshot::Receiver<Vec<AbsolutePathBuf>>,
|
||||
tokio::task::JoinHandle<()>,
|
||||
) {
|
||||
let listener = TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("listener should bind");
|
||||
let websocket_url = format!("ws://{}", listener.local_addr().expect("listener address"));
|
||||
let (captured_paths_tx, captured_paths_rx) = oneshot::channel();
|
||||
let server = tokio::spawn(async move {
|
||||
let (stream, _) = listener.accept().await.expect("listener should accept");
|
||||
let mut websocket = accept_async(stream)
|
||||
.await
|
||||
.expect("websocket handshake should succeed");
|
||||
complete_websocket_initialize(&mut websocket).await;
|
||||
|
||||
let mut captured_paths = Vec::with_capacity(expected_requests);
|
||||
for _ in 0..expected_requests {
|
||||
let request = match read_jsonrpc_websocket(&mut websocket).await {
|
||||
JSONRPCMessage::Request(request) if request.method == FS_READ_FILE_METHOD => {
|
||||
request
|
||||
}
|
||||
other => panic!("expected fs/readFile request, got {other:?}"),
|
||||
};
|
||||
let params: FsReadFileParams =
|
||||
serde_json::from_value(request.params.expect("fs/readFile params should exist"))
|
||||
.expect("fs/readFile params should deserialize");
|
||||
captured_paths.push(params.path);
|
||||
write_jsonrpc_websocket(
|
||||
&mut websocket,
|
||||
JSONRPCMessage::Response(JSONRPCResponse {
|
||||
id: request.id,
|
||||
result: serde_json::to_value(FsReadFileResponse {
|
||||
data_base64: String::new(),
|
||||
})
|
||||
.expect("fs/readFile response should serialize"),
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
captured_paths_tx
|
||||
.send(captured_paths)
|
||||
.expect("captured paths receiver should stay open");
|
||||
});
|
||||
|
||||
(websocket_url, captured_paths_rx, server)
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
async fn complete_websocket_initialize(websocket: &mut WebSocketStream<TcpStream>) {
|
||||
let request = match read_jsonrpc_websocket(websocket).await {
|
||||
JSONRPCMessage::Request(request) if request.method == INITIALIZE_METHOD => request,
|
||||
other => panic!("expected initialize request, got {other:?}"),
|
||||
};
|
||||
write_jsonrpc_websocket(
|
||||
websocket,
|
||||
JSONRPCMessage::Response(JSONRPCResponse {
|
||||
id: request.id,
|
||||
result: serde_json::to_value(InitializeResponse {
|
||||
session_id: "session-1".to_string(),
|
||||
})
|
||||
.expect("initialize response should serialize"),
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
|
||||
match read_jsonrpc_websocket(websocket).await {
|
||||
JSONRPCMessage::Notification(notification) if notification.method == INITIALIZED_METHOD => {
|
||||
}
|
||||
other => panic!("expected initialized notification, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
async fn read_jsonrpc_websocket(websocket: &mut WebSocketStream<TcpStream>) -> JSONRPCMessage {
|
||||
loop {
|
||||
match timeout(Duration::from_secs(1), websocket.next())
|
||||
.await
|
||||
.expect("json-rpc websocket read should not time out")
|
||||
.expect("websocket should stay open")
|
||||
.expect("websocket frame should read")
|
||||
{
|
||||
Message::Text(text) => {
|
||||
return serde_json::from_str(text.as_ref())
|
||||
.expect("json-rpc text frame should parse");
|
||||
}
|
||||
Message::Binary(bytes) => {
|
||||
return serde_json::from_slice(bytes.as_ref())
|
||||
.expect("json-rpc binary frame should parse");
|
||||
}
|
||||
Message::Ping(_) | Message::Pong(_) => {}
|
||||
other => panic!("expected json-rpc websocket frame, got {other:?}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
async fn write_jsonrpc_websocket(
|
||||
websocket: &mut WebSocketStream<TcpStream>,
|
||||
message: JSONRPCMessage,
|
||||
) {
|
||||
let encoded = serde_json::to_string(&message).expect("json-rpc should serialize");
|
||||
websocket
|
||||
.send(Message::Text(encoded.into()))
|
||||
.await
|
||||
.expect("json-rpc websocket frame should write");
|
||||
}
|
||||
@@ -75,7 +75,7 @@ impl ExecutorFileSystem for SandboxedFileSystem {
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<Vec<u8>> {
|
||||
let sandbox = require_platform_sandbox(sandbox)?;
|
||||
@@ -83,7 +83,7 @@ impl ExecutorFileSystem for SandboxedFileSystem {
|
||||
.run_sandboxed(
|
||||
sandbox,
|
||||
FsHelperRequest::ReadFile(FsReadFileParams {
|
||||
path: path.clone(),
|
||||
path: path.to_abs_path()?,
|
||||
sandbox: None,
|
||||
}),
|
||||
)
|
||||
@@ -260,3 +260,7 @@ fn map_sandbox_error(error: JSONRPCErrorError) -> io::Error {
|
||||
_ => io::Error::other(error.message),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(all(test, any(unix, windows)))]
|
||||
#[path = "sandboxed_file_system_path_uri_tests.rs"]
|
||||
mod path_uri_tests;
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::permissions::FileSystemSandboxPolicy;
|
||||
use codex_protocol::permissions::NetworkSandboxPolicy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::io;
|
||||
|
||||
use super::*;
|
||||
|
||||
#[tokio::test]
|
||||
async fn sandboxed_file_system_rejects_non_native_uri_as_invalid_input() {
|
||||
let runtime_paths = ExecServerRuntimePaths::new(
|
||||
std::env::current_exe().expect("current exe"),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
)
|
||||
.expect("runtime paths");
|
||||
let file_system = SandboxedFileSystem::new(runtime_paths);
|
||||
let sandbox = FileSystemSandboxContext::from_permission_profile(
|
||||
PermissionProfile::from_runtime_permissions(
|
||||
&FileSystemSandboxPolicy::restricted(Vec::new()),
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
),
|
||||
);
|
||||
|
||||
let error = file_system
|
||||
.read_file(&non_native_uri(), Some(&sandbox))
|
||||
.await
|
||||
.expect_err("non-native URI should be rejected");
|
||||
|
||||
assert_eq!(error.kind(), io::ErrorKind::InvalidInput);
|
||||
}
|
||||
|
||||
fn non_native_uri() -> PathUri {
|
||||
#[cfg(unix)]
|
||||
let uri = "file://server/share/file.txt";
|
||||
#[cfg(windows)]
|
||||
let uri = "file:///usr/local/file.txt";
|
||||
|
||||
match PathUri::parse(uri) {
|
||||
Ok(uri) => uri,
|
||||
Err(err) => panic!("valid non-native URI should parse: {err}"),
|
||||
}
|
||||
}
|
||||
@@ -53,9 +53,10 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsReadFileParams,
|
||||
) -> Result<FsReadFileResponse, JSONRPCErrorError> {
|
||||
let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?;
|
||||
let bytes = self
|
||||
.file_system
|
||||
.read_file(¶ms.path, params.sandbox.as_ref())
|
||||
.read_file(&path, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsReadFileResponse {
|
||||
|
||||
Reference in New Issue
Block a user