mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
exec-server: canonicalize bound filesystem paths (#25149)
## Summary - add executor filesystem canonicalization as a bound-path operation - route remote canonicalization through the exec-server filesystem RPC surface - keep path normalization attached to the filesystem that owns the path ## Stack - 2/5 in the skills path authority stack extracted from https://github.com/openai/codex/pull/25098 - follows merged https://github.com/openai/codex/pull/25121 ## Validation - `cd /Users/starr/code/codex-worktrees/pr-25098-restack-review-pr1b/codex-rs && just fmt` - Not run: tests/checks (not requested) - GitHub CI pending on rewritten head
This commit is contained in:
committed by
GitHub
Unverified
parent
f1609d9fb6
commit
53ac02356e
@@ -8,12 +8,33 @@ use codex_file_system::FileSystemSandboxContext;
|
||||
use codex_file_system::ReadDirectoryEntry;
|
||||
use codex_file_system::RemoveOptions;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use tempfile::tempdir;
|
||||
|
||||
struct TestFileSystem;
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for TestFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
_sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
path.canonicalize()
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -41,19 +41,28 @@ use crate::protocol::ExecExitedNotification;
|
||||
use crate::protocol::ExecOutputDeltaNotification;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::ExecResponse;
|
||||
use crate::protocol::FS_CANONICALIZE_METHOD;
|
||||
use crate::protocol::FS_COPY_METHOD;
|
||||
use crate::protocol::FS_CREATE_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_GET_METADATA_METHOD;
|
||||
use crate::protocol::FS_JOIN_METHOD;
|
||||
use crate::protocol::FS_PARENT_METHOD;
|
||||
use crate::protocol::FS_READ_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_READ_FILE_METHOD;
|
||||
use crate::protocol::FS_REMOVE_METHOD;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCopyResponse;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsCreateDirectoryResponse;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsGetMetadataResponse;
|
||||
use crate::protocol::FsJoinParams;
|
||||
use crate::protocol::FsJoinResponse;
|
||||
use crate::protocol::FsParentParams;
|
||||
use crate::protocol::FsParentResponse;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadDirectoryResponse;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
@@ -414,6 +423,24 @@ impl ExecServerClient {
|
||||
self.call(FS_GET_METADATA_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn fs_canonicalize(
|
||||
&self,
|
||||
params: FsCanonicalizeParams,
|
||||
) -> Result<FsCanonicalizeResponse, ExecServerError> {
|
||||
self.call(FS_CANONICALIZE_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn fs_join(&self, params: FsJoinParams) -> Result<FsJoinResponse, ExecServerError> {
|
||||
self.call(FS_JOIN_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn fs_parent(
|
||||
&self,
|
||||
params: FsParentParams,
|
||||
) -> Result<FsParentResponse, ExecServerError> {
|
||||
self.call(FS_PARENT_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn fs_read_directory(
|
||||
&self,
|
||||
params: FsReadDirectoryParams,
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::fmt;
|
||||
use std::hash::Hash;
|
||||
use std::hash::Hasher;
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -20,22 +21,27 @@ pub struct EnvironmentPathRef {
|
||||
}
|
||||
|
||||
impl EnvironmentPathRef {
|
||||
/// Creates a path ref bound to the filesystem that owns `path`.
|
||||
pub fn new(file_system: Arc<dyn ExecutorFileSystem>, path: AbsolutePathBuf) -> Self {
|
||||
Self { file_system, path }
|
||||
}
|
||||
|
||||
/// Creates a path ref bound to the shared unsandboxed local filesystem.
|
||||
pub fn local(path: AbsolutePathBuf) -> Self {
|
||||
Self::new(Arc::clone(&LOCAL_FS), path)
|
||||
}
|
||||
|
||||
/// Returns the absolute path held by this ref.
|
||||
pub fn path(&self) -> &AbsolutePathBuf {
|
||||
&self.path
|
||||
}
|
||||
|
||||
/// Returns the filesystem that owns this path.
|
||||
pub fn file_system(&self) -> Arc<dyn ExecutorFileSystem> {
|
||||
Arc::clone(&self.file_system)
|
||||
}
|
||||
|
||||
/// Reads this path as UTF-8 text through its bound filesystem.
|
||||
pub async fn read_to_string(
|
||||
&self,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
@@ -43,6 +49,7 @@ impl EnvironmentPathRef {
|
||||
self.file_system.read_file_text(&self.path, sandbox).await
|
||||
}
|
||||
|
||||
/// Reads metadata for this path through its bound filesystem.
|
||||
pub async fn metadata(
|
||||
&self,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
@@ -50,6 +57,7 @@ impl EnvironmentPathRef {
|
||||
self.file_system.get_metadata(&self.path, sandbox).await
|
||||
}
|
||||
|
||||
/// Reads directory entries for this path through its bound filesystem.
|
||||
pub async fn read_directory(
|
||||
&self,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
@@ -57,9 +65,37 @@ impl EnvironmentPathRef {
|
||||
self.file_system.read_directory(&self.path, sandbox).await
|
||||
}
|
||||
|
||||
/// Returns a ref with the same filesystem and a replacement path.
|
||||
pub fn with_path(&self, path: AbsolutePathBuf) -> Self {
|
||||
Self::new(Arc::clone(&self.file_system), path)
|
||||
}
|
||||
|
||||
/// Lexically joins `path` onto this path through its bound filesystem.
|
||||
pub async fn join<P: AsRef<Path>>(&self, path: P) -> io::Result<Self> {
|
||||
self.file_system
|
||||
.join(&self.path, path.as_ref())
|
||||
.await
|
||||
.map(|path| self.with_path(path))
|
||||
}
|
||||
|
||||
/// Returns the parent of this path through its bound filesystem.
|
||||
pub async fn parent(&self) -> io::Result<Option<Self>> {
|
||||
self.file_system
|
||||
.parent(&self.path)
|
||||
.await
|
||||
.map(|path| path.map(|path| self.with_path(path)))
|
||||
}
|
||||
|
||||
/// Canonicalizes this path through its bound filesystem.
|
||||
pub async fn canonicalize(
|
||||
&self,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> io::Result<Self> {
|
||||
self.file_system
|
||||
.canonicalize(&self.path, sandbox)
|
||||
.await
|
||||
.map(|path| self.with_path(path))
|
||||
}
|
||||
}
|
||||
|
||||
impl PartialEq for EnvironmentPathRef {
|
||||
@@ -96,10 +132,16 @@ mod tests {
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use crate::LOCAL_FS;
|
||||
|
||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
||||
enum RecordedMethod {
|
||||
Canonicalize,
|
||||
Join,
|
||||
Parent,
|
||||
ReadFileText,
|
||||
Metadata,
|
||||
ReadDirectory,
|
||||
@@ -140,8 +182,47 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn local_path_ref(path: AbsolutePathBuf) -> EnvironmentPathRef {
|
||||
EnvironmentPathRef::new(Arc::clone(&LOCAL_FS), path)
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for RecordingFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
_sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> io::Result<AbsolutePathBuf> {
|
||||
self.push_call(RecordedCall {
|
||||
method: RecordedMethod::Canonicalize,
|
||||
path: path.clone(),
|
||||
sandbox: None,
|
||||
});
|
||||
Ok(path.parent().unwrap())
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> io::Result<AbsolutePathBuf> {
|
||||
self.push_call(RecordedCall {
|
||||
method: RecordedMethod::Join,
|
||||
path: base_path.clone(),
|
||||
sandbox: None,
|
||||
});
|
||||
AbsolutePathBuf::from_absolute_path_checked(base_path.as_path().join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> io::Result<Option<AbsolutePathBuf>> {
|
||||
self.push_call(RecordedCall {
|
||||
method: RecordedMethod::Parent,
|
||||
path: path.clone(),
|
||||
sandbox: None,
|
||||
});
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
@@ -318,4 +399,129 @@ mod tests {
|
||||
let set = HashSet::from([left, same, different_path, different_fs]);
|
||||
assert_eq!(set.len(), 3);
|
||||
}
|
||||
#[tokio::test]
|
||||
async fn canonicalize_keeps_bound_file_system_identity() {
|
||||
let path = std::env::temp_dir().join("skills/demo").abs();
|
||||
let file_system = Arc::new(RecordingFileSystem::default());
|
||||
let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone());
|
||||
|
||||
let canonicalized = path_ref
|
||||
.canonicalize(/*sandbox*/ None)
|
||||
.await
|
||||
.expect("canonicalize");
|
||||
|
||||
assert_eq!(canonicalized.path(), &path.parent().unwrap());
|
||||
assert_eq!(
|
||||
canonicalized,
|
||||
EnvironmentPathRef::new(file_system.clone(), path.parent().unwrap())
|
||||
);
|
||||
assert_eq!(
|
||||
file_system.recorded_calls(),
|
||||
vec![RecordedCall {
|
||||
method: RecordedMethod::Canonicalize,
|
||||
path,
|
||||
sandbox: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn join_keeps_bound_file_system_identity() {
|
||||
let path = std::env::temp_dir().join("skills").abs();
|
||||
let file_system = Arc::new(RecordingFileSystem::default());
|
||||
let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone());
|
||||
|
||||
assert_eq!(
|
||||
path_ref.join(Path::new("demo")).await.ok(),
|
||||
Some(EnvironmentPathRef::new(
|
||||
file_system.clone(),
|
||||
std::env::temp_dir().join("skills/demo").abs(),
|
||||
))
|
||||
);
|
||||
assert_eq!(
|
||||
file_system.recorded_calls(),
|
||||
vec![RecordedCall {
|
||||
method: RecordedMethod::Join,
|
||||
path,
|
||||
sandbox: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn join_matches_absolute_path_buf_for_tilde_paths() {
|
||||
let path_ref = local_path_ref(std::env::temp_dir().join("skills").abs());
|
||||
|
||||
assert_eq!(
|
||||
path_ref
|
||||
.join(Path::new("~"))
|
||||
.await
|
||||
.ok()
|
||||
.map(|path_ref| path_ref.path().clone()),
|
||||
Some(path_ref.path().join(Path::new("~")))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn join_matches_absolute_path_buf_for_parent_dirs() {
|
||||
let path_ref = local_path_ref(std::env::temp_dir().join("skills").abs());
|
||||
|
||||
assert_eq!(
|
||||
path_ref
|
||||
.join(Path::new("../outside"))
|
||||
.await
|
||||
.expect("join")
|
||||
.path()
|
||||
.clone(),
|
||||
path_ref.path().join(Path::new("../outside"))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn parent_keeps_bound_file_system_identity() {
|
||||
let path = std::env::temp_dir().join("skills/demo").abs();
|
||||
let file_system = Arc::new(RecordingFileSystem::default());
|
||||
let path_ref = EnvironmentPathRef::new(file_system.clone(), path.clone());
|
||||
|
||||
assert_eq!(
|
||||
path_ref.parent().await.expect("parent"),
|
||||
Some(EnvironmentPathRef::new(
|
||||
file_system.clone(),
|
||||
std::env::temp_dir().join("skills").abs(),
|
||||
))
|
||||
);
|
||||
assert_eq!(
|
||||
file_system.recorded_calls(),
|
||||
vec![RecordedCall {
|
||||
method: RecordedMethod::Parent,
|
||||
path,
|
||||
sandbox: None,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn join_matches_absolute_path_buf_for_windows_prefixed_and_rooted_paths() {
|
||||
let path_ref = local_path_ref(std::env::temp_dir().join("skills").abs());
|
||||
|
||||
assert_eq!(
|
||||
path_ref
|
||||
.join(Path::new(r"C:temp"))
|
||||
.await
|
||||
.expect("join")
|
||||
.path()
|
||||
.clone(),
|
||||
path_ref.path().join(Path::new(r"C:temp"))
|
||||
);
|
||||
assert_eq!(
|
||||
path_ref
|
||||
.join(Path::new(r"\temp"))
|
||||
.await
|
||||
.expect("join")
|
||||
.path()
|
||||
.clone(),
|
||||
path_ref.path().join(Path::new(r"\temp"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ use crate::CreateDirectoryOptions;
|
||||
use crate::ExecutorFileSystem;
|
||||
use crate::RemoveOptions;
|
||||
use crate::local_file_system::DirectFileSystem;
|
||||
use crate::protocol::FS_CANONICALIZE_METHOD;
|
||||
use crate::protocol::FS_COPY_METHOD;
|
||||
use crate::protocol::FS_CREATE_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_GET_METADATA_METHOD;
|
||||
@@ -17,6 +18,8 @@ use crate::protocol::FS_READ_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_READ_FILE_METHOD;
|
||||
use crate::protocol::FS_REMOVE_METHOD;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCopyResponse;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
@@ -49,6 +52,8 @@ pub(crate) enum FsHelperRequest {
|
||||
CreateDirectory(FsCreateDirectoryParams),
|
||||
#[serde(rename = "fs/getMetadata")]
|
||||
GetMetadata(FsGetMetadataParams),
|
||||
#[serde(rename = "fs/canonicalize")]
|
||||
Canonicalize(FsCanonicalizeParams),
|
||||
#[serde(rename = "fs/readDirectory")]
|
||||
ReadDirectory(FsReadDirectoryParams),
|
||||
#[serde(rename = "fs/remove")]
|
||||
@@ -75,6 +80,8 @@ pub(crate) enum FsHelperPayload {
|
||||
CreateDirectory(FsCreateDirectoryResponse),
|
||||
#[serde(rename = "fs/getMetadata")]
|
||||
GetMetadata(FsGetMetadataResponse),
|
||||
#[serde(rename = "fs/canonicalize")]
|
||||
Canonicalize(FsCanonicalizeResponse),
|
||||
#[serde(rename = "fs/readDirectory")]
|
||||
ReadDirectory(FsReadDirectoryResponse),
|
||||
#[serde(rename = "fs/remove")]
|
||||
@@ -90,6 +97,7 @@ impl FsHelperPayload {
|
||||
Self::WriteFile(_) => FS_WRITE_FILE_METHOD,
|
||||
Self::CreateDirectory(_) => FS_CREATE_DIRECTORY_METHOD,
|
||||
Self::GetMetadata(_) => FS_GET_METADATA_METHOD,
|
||||
Self::Canonicalize(_) => FS_CANONICALIZE_METHOD,
|
||||
Self::ReadDirectory(_) => FS_READ_DIRECTORY_METHOD,
|
||||
Self::Remove(_) => FS_REMOVE_METHOD,
|
||||
Self::Copy(_) => FS_COPY_METHOD,
|
||||
@@ -132,6 +140,16 @@ impl FsHelperPayload {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn expect_canonicalize(self) -> Result<FsCanonicalizeResponse, JSONRPCErrorError> {
|
||||
match self {
|
||||
Self::Canonicalize(response) => Ok(response),
|
||||
other => Err(unexpected_response(
|
||||
FS_CANONICALIZE_METHOD,
|
||||
other.operation(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn expect_read_directory(
|
||||
self,
|
||||
) -> Result<FsReadDirectoryResponse, JSONRPCErrorError> {
|
||||
@@ -219,6 +237,15 @@ pub(crate) async fn run_direct_request(
|
||||
modified_at_ms: metadata.modified_at_ms,
|
||||
}))
|
||||
}
|
||||
FsHelperRequest::Canonicalize(params) => {
|
||||
let path = file_system
|
||||
.canonicalize(¶ms.path, /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsHelperPayload::Canonicalize(FsCanonicalizeResponse {
|
||||
path,
|
||||
}))
|
||||
}
|
||||
FsHelperRequest::ReadDirectory(params) => {
|
||||
let entries = file_system
|
||||
.read_directory(¶ms.path, /*sandbox*/ None)
|
||||
|
||||
@@ -64,12 +64,18 @@ pub use protocol::ExecOutputDeltaNotification;
|
||||
pub use protocol::ExecOutputStream;
|
||||
pub use protocol::ExecParams;
|
||||
pub use protocol::ExecResponse;
|
||||
pub use protocol::FsCanonicalizeParams;
|
||||
pub use protocol::FsCanonicalizeResponse;
|
||||
pub use protocol::FsCopyParams;
|
||||
pub use protocol::FsCopyResponse;
|
||||
pub use protocol::FsCreateDirectoryParams;
|
||||
pub use protocol::FsCreateDirectoryResponse;
|
||||
pub use protocol::FsGetMetadataParams;
|
||||
pub use protocol::FsGetMetadataResponse;
|
||||
pub use protocol::FsJoinParams;
|
||||
pub use protocol::FsJoinResponse;
|
||||
pub use protocol::FsParentParams;
|
||||
pub use protocol::FsParentResponse;
|
||||
pub use protocol::FsReadDirectoryEntry;
|
||||
pub use protocol::FsReadDirectoryParams;
|
||||
pub use protocol::FsReadDirectoryResponse;
|
||||
|
||||
@@ -79,6 +79,27 @@ impl LocalFileSystem {
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for LocalFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
let (file_system, sandbox) = self.file_system_for(sandbox)?;
|
||||
file_system.canonicalize(path, sandbox).await
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
self.unsandboxed.join(base_path, path).await
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
self.unsandboxed.parent(path).await
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
@@ -152,6 +173,27 @@ impl ExecutorFileSystem for LocalFileSystem {
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for UnsandboxedFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
reject_platform_sandbox_context(sandbox)?;
|
||||
self.file_system.canonicalize(path, /*sandbox*/ None).await
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
self.file_system.join(base_path, path).await
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
self.file_system.parent(path).await
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
@@ -238,6 +280,27 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for DirectFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
reject_sandbox_context(sandbox)?;
|
||||
AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?)
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -23,6 +23,9 @@ pub const FS_READ_FILE_METHOD: &str = "fs/readFile";
|
||||
pub const FS_WRITE_FILE_METHOD: &str = "fs/writeFile";
|
||||
pub const FS_CREATE_DIRECTORY_METHOD: &str = "fs/createDirectory";
|
||||
pub const FS_GET_METADATA_METHOD: &str = "fs/getMetadata";
|
||||
pub const FS_CANONICALIZE_METHOD: &str = "fs/canonicalize";
|
||||
pub const FS_JOIN_METHOD: &str = "fs/join";
|
||||
pub const FS_PARENT_METHOD: &str = "fs/parent";
|
||||
pub const FS_READ_DIRECTORY_METHOD: &str = "fs/readDirectory";
|
||||
pub const FS_REMOVE_METHOD: &str = "fs/remove";
|
||||
pub const FS_COPY_METHOD: &str = "fs/copy";
|
||||
@@ -211,6 +214,44 @@ pub struct FsGetMetadataResponse {
|
||||
pub modified_at_ms: i64,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsCanonicalizeParams {
|
||||
pub path: AbsolutePathBuf,
|
||||
pub sandbox: Option<FileSystemSandboxContext>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsCanonicalizeResponse {
|
||||
pub path: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsJoinParams {
|
||||
pub base_path: AbsolutePathBuf,
|
||||
pub path: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsJoinResponse {
|
||||
pub path: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsParentParams {
|
||||
pub path: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsParentResponse {
|
||||
pub path: Option<AbsolutePathBuf>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct FsReadDirectoryParams {
|
||||
|
||||
@@ -2,6 +2,7 @@ use async_trait::async_trait;
|
||||
use base64::Engine as _;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
use tokio::io;
|
||||
use tracing::trace;
|
||||
|
||||
@@ -15,9 +16,12 @@ use crate::FileSystemSandboxContext;
|
||||
use crate::ReadDirectoryEntry;
|
||||
use crate::RemoveOptions;
|
||||
use crate::client::LazyRemoteExecServerClient;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsJoinParams;
|
||||
use crate::protocol::FsParentParams;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
use crate::protocol::FsRemoveParams;
|
||||
@@ -26,7 +30,6 @@ use crate::protocol::FsWriteFileParams;
|
||||
const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
|
||||
const NOT_FOUND_ERROR_CODE: i64 = -32004;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct RemoteFileSystem {
|
||||
client: LazyRemoteExecServerClient,
|
||||
}
|
||||
@@ -40,6 +43,50 @@ impl RemoteFileSystem {
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for RemoteFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
trace!("remote fs canonicalize");
|
||||
let client = self.client.get().await.map_err(map_remote_error)?;
|
||||
let response = client
|
||||
.fs_canonicalize(FsCanonicalizeParams {
|
||||
path: path.clone(),
|
||||
sandbox: remote_sandbox_context(sandbox),
|
||||
})
|
||||
.await
|
||||
.map_err(map_remote_error)?;
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
trace!("remote fs join");
|
||||
let client = self.client.get().await.map_err(map_remote_error)?;
|
||||
let response = client
|
||||
.fs_join(FsJoinParams {
|
||||
base_path: base_path.clone(),
|
||||
path: path.to_path_buf(),
|
||||
})
|
||||
.await
|
||||
.map_err(map_remote_error)?;
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
trace!("remote fs parent");
|
||||
let client = self.client.get().await.map_err(map_remote_error)?;
|
||||
let response = client
|
||||
.fs_parent(FsParentParams { path: path.clone() })
|
||||
.await
|
||||
.map_err(map_remote_error)?;
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -3,6 +3,7 @@ use base64::Engine as _;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
use tokio::io;
|
||||
|
||||
use crate::CopyOptions;
|
||||
@@ -17,6 +18,7 @@ use crate::RemoveOptions;
|
||||
use crate::fs_helper::FsHelperPayload;
|
||||
use crate::fs_helper::FsHelperRequest;
|
||||
use crate::fs_sandbox::FileSystemSandboxRunner;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
@@ -51,6 +53,38 @@ impl SandboxedFileSystem {
|
||||
|
||||
#[async_trait]
|
||||
impl ExecutorFileSystem for SandboxedFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
let sandbox = require_platform_sandbox(sandbox)?;
|
||||
let response = self
|
||||
.run_sandboxed(
|
||||
sandbox,
|
||||
FsHelperRequest::Canonicalize(FsCanonicalizeParams {
|
||||
path: path.clone(),
|
||||
sandbox: None,
|
||||
}),
|
||||
)
|
||||
.await?
|
||||
.expect_canonicalize()
|
||||
.map_err(map_sandbox_error)?;
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -11,12 +11,18 @@ use crate::ExecutorFileSystem;
|
||||
use crate::RemoveOptions;
|
||||
use crate::local_file_system::LocalFileSystem;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCopyResponse;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsCreateDirectoryResponse;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsGetMetadataResponse;
|
||||
use crate::protocol::FsJoinParams;
|
||||
use crate::protocol::FsJoinResponse;
|
||||
use crate::protocol::FsParentParams;
|
||||
use crate::protocol::FsParentResponse;
|
||||
use crate::protocol::FsReadDirectoryEntry;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadDirectoryResponse;
|
||||
@@ -106,6 +112,42 @@ impl FileSystemHandler {
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn canonicalize(
|
||||
&self,
|
||||
params: FsCanonicalizeParams,
|
||||
) -> Result<FsCanonicalizeResponse, JSONRPCErrorError> {
|
||||
let path = self
|
||||
.file_system
|
||||
.canonicalize(¶ms.path, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsCanonicalizeResponse { path })
|
||||
}
|
||||
|
||||
pub(crate) async fn join(
|
||||
&self,
|
||||
params: FsJoinParams,
|
||||
) -> Result<FsJoinResponse, JSONRPCErrorError> {
|
||||
let path = self
|
||||
.file_system
|
||||
.join(¶ms.base_path, ¶ms.path)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsJoinResponse { path })
|
||||
}
|
||||
|
||||
pub(crate) async fn parent(
|
||||
&self,
|
||||
params: FsParentParams,
|
||||
) -> Result<FsParentResponse, JSONRPCErrorError> {
|
||||
let path = self
|
||||
.file_system
|
||||
.parent(¶ms.path)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsParentResponse { path })
|
||||
}
|
||||
|
||||
pub(crate) async fn read_directory(
|
||||
&self,
|
||||
params: FsReadDirectoryParams,
|
||||
|
||||
@@ -16,12 +16,18 @@ use crate::client::http_client::PendingReqwestHttpBodyStream;
|
||||
use crate::client::http_client::ReqwestHttpRequestRunner;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::ExecResponse;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCopyResponse;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsCreateDirectoryResponse;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsGetMetadataResponse;
|
||||
use crate::protocol::FsJoinParams;
|
||||
use crate::protocol::FsJoinResponse;
|
||||
use crate::protocol::FsParentParams;
|
||||
use crate::protocol::FsParentResponse;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadDirectoryResponse;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
@@ -240,6 +246,30 @@ impl ExecServerHandler {
|
||||
self.file_system.get_metadata(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_canonicalize(
|
||||
&self,
|
||||
params: FsCanonicalizeParams,
|
||||
) -> Result<FsCanonicalizeResponse, JSONRPCErrorError> {
|
||||
self.require_initialized_for("filesystem")?;
|
||||
self.file_system.canonicalize(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_join(
|
||||
&self,
|
||||
params: FsJoinParams,
|
||||
) -> Result<FsJoinResponse, JSONRPCErrorError> {
|
||||
self.require_initialized_for("filesystem")?;
|
||||
self.file_system.join(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_parent(
|
||||
&self,
|
||||
params: FsParentParams,
|
||||
) -> Result<FsParentResponse, JSONRPCErrorError> {
|
||||
self.require_initialized_for("filesystem")?;
|
||||
self.file_system.parent(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_read_directory(
|
||||
&self,
|
||||
params: FsReadDirectoryParams,
|
||||
|
||||
@@ -5,16 +5,22 @@ use crate::protocol::EXEC_READ_METHOD;
|
||||
use crate::protocol::EXEC_TERMINATE_METHOD;
|
||||
use crate::protocol::EXEC_WRITE_METHOD;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::FS_CANONICALIZE_METHOD;
|
||||
use crate::protocol::FS_COPY_METHOD;
|
||||
use crate::protocol::FS_CREATE_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_GET_METADATA_METHOD;
|
||||
use crate::protocol::FS_JOIN_METHOD;
|
||||
use crate::protocol::FS_PARENT_METHOD;
|
||||
use crate::protocol::FS_READ_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_READ_FILE_METHOD;
|
||||
use crate::protocol::FS_REMOVE_METHOD;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsJoinParams;
|
||||
use crate::protocol::FsParentParams;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
use crate::protocol::FsRemoveParams;
|
||||
@@ -96,6 +102,24 @@ pub(crate) fn build_router() -> RpcRouter<ExecServerHandler> {
|
||||
handler.fs_get_metadata(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_CANONICALIZE_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsCanonicalizeParams| async move {
|
||||
handler.fs_canonicalize(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_JOIN_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsJoinParams| async move {
|
||||
handler.fs_join(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_PARENT_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsParentParams| async move {
|
||||
handler.fs_parent(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_READ_DIRECTORY_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsReadDirectoryParams| async move {
|
||||
|
||||
@@ -365,6 +365,49 @@ async fn file_system_methods_cover_surface_area(use_remote: bool) -> Result<()>
|
||||
.await
|
||||
.with_context(|| format!("mode={use_remote}"))?;
|
||||
|
||||
let source_link = tmp.path().join("source-link");
|
||||
symlink(&source_dir, &source_link)?;
|
||||
let joined_nested = file_system
|
||||
.join(
|
||||
&absolute_path(source_link.clone()),
|
||||
Path::new("nested/note.txt"),
|
||||
)
|
||||
.await
|
||||
.with_context(|| format!("mode={use_remote}"))?;
|
||||
assert_eq!(
|
||||
joined_nested,
|
||||
absolute_path(source_link.join("nested").join("note.txt"))
|
||||
);
|
||||
let joined_parent = file_system
|
||||
.parent(&joined_nested)
|
||||
.await
|
||||
.with_context(|| format!("mode={use_remote}"))?;
|
||||
assert_eq!(
|
||||
joined_parent,
|
||||
Some(absolute_path(source_link.join("nested")))
|
||||
);
|
||||
let joined_parent_traversal = file_system
|
||||
.join(&absolute_path(source_dir.clone()), Path::new("../outside"))
|
||||
.await
|
||||
.with_context(|| format!("mode={use_remote}"))?;
|
||||
assert_eq!(
|
||||
joined_parent_traversal,
|
||||
absolute_path(source_dir.join("../outside"))
|
||||
);
|
||||
let canonical_nested = file_system
|
||||
.canonicalize(
|
||||
&absolute_path(source_link.join("nested").join("note.txt")),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.with_context(|| format!("mode={use_remote}"))?;
|
||||
assert_eq!(
|
||||
canonical_nested,
|
||||
absolute_path(std::fs::canonicalize(
|
||||
source_dir.join("nested").join("note.txt")
|
||||
)?)
|
||||
);
|
||||
|
||||
let nested_file_contents = file_system
|
||||
.read_file(&absolute_path(nested_file.clone()), /*sandbox*/ None)
|
||||
.await
|
||||
@@ -530,6 +573,32 @@ async fn file_system_sandboxed_read_allows_readable_root(use_remote: bool) -> Re
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false ; "local")]
|
||||
#[test_case(true ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn file_system_sandboxed_canonicalize_allows_readable_root(use_remote: bool) -> Result<()> {
|
||||
let context = create_file_system_context(use_remote).await?;
|
||||
let file_system = context.file_system;
|
||||
|
||||
let tmp = TempDir::new()?;
|
||||
let allowed_dir = tmp.path().join("allowed");
|
||||
let file_path = allowed_dir.join("note.txt");
|
||||
std::fs::create_dir_all(&allowed_dir)?;
|
||||
std::fs::write(&file_path, "sandboxed hello")?;
|
||||
let sandbox = read_only_sandbox(allowed_dir);
|
||||
|
||||
let canonical_path = file_system
|
||||
.canonicalize(&absolute_path(file_path.clone()), Some(&sandbox))
|
||||
.await
|
||||
.with_context(|| format!("mode={use_remote}"))?;
|
||||
assert_eq!(
|
||||
canonical_path,
|
||||
absolute_path(std::fs::canonicalize(file_path)?)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false ; "local")]
|
||||
#[test_case(true ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
|
||||
@@ -133,6 +133,23 @@ pub type FileSystemResult<T> = io::Result<T>;
|
||||
/// a remote environment.
|
||||
#[async_trait]
|
||||
pub trait ExecutorFileSystem: Send + Sync {
|
||||
/// Resolves a path within this filesystem.
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf>;
|
||||
|
||||
/// Lexically joins a path onto an existing bound path.
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf>;
|
||||
|
||||
/// Returns the parent directory of a bound path.
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>>;
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
Reference in New Issue
Block a user