codex: pass cwd through sandboxed fs ops

This commit is contained in:
starr-openai
2026-04-06 16:40:53 -07:00
parent 3aa46997c0
commit fc86503a4e
30 changed files with 1169 additions and 56 deletions
@@ -663,6 +663,17 @@
"FsCopyParams": {
"description": "Copy a file or directory tree on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"destinationPath": {
"allOf": [
{
@@ -693,6 +704,17 @@
"FsCreateDirectoryParams": {
"description": "Create a directory on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -717,6 +739,17 @@
"FsGetMetadataParams": {
"description": "Request metadata for an absolute path.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -734,6 +767,17 @@
"FsReadDirectoryParams": {
"description": "List direct child names for a directory.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -751,6 +795,17 @@
"FsReadFileParams": {
"description": "Read a file from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -768,6 +823,17 @@
"FsRemoveParams": {
"description": "Remove a file or directory tree from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"force": {
"description": "Whether missing paths should be ignored. Defaults to `true`.",
"type": [
@@ -829,6 +895,17 @@
"FsWriteFileParams": {
"description": "Write a file on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"dataBase64": {
"description": "File contents encoded as base64.",
"type": "string"
@@ -7502,6 +7502,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Copy a file or directory tree on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"destinationPath": {
"allOf": [
{
@@ -7540,6 +7551,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Create a directory on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -7572,6 +7594,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Request metadata for an absolute path.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -7646,6 +7679,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "List direct child names for a directory.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -7683,6 +7727,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Read a file from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -7717,6 +7772,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Remove a file or directory tree from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"force": {
"description": "Whether missing paths should be ignored. Defaults to `true`.",
"type": [
@@ -7820,6 +7886,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Write a file on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"dataBase64": {
"description": "File contents encoded as base64.",
"type": "string"
@@ -4170,6 +4170,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Copy a file or directory tree on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"destinationPath": {
"allOf": [
{
@@ -4208,6 +4219,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Create a directory on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -4240,6 +4262,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Request metadata for an absolute path.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -4314,6 +4347,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "List direct child names for a directory.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -4351,6 +4395,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Read a file from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -4385,6 +4440,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Remove a file or directory tree from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"force": {
"description": "Whether missing paths should be ignored. Defaults to `true`.",
"type": [
@@ -4488,6 +4554,17 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Write a file on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"dataBase64": {
"description": "File contents encoded as base64.",
"type": "string"
@@ -8,6 +8,17 @@
},
"description": "Copy a file or directory tree on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"destinationPath": {
"allOf": [
{
@@ -8,6 +8,17 @@
},
"description": "Create a directory on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -8,6 +8,17 @@
},
"description": "Request metadata for an absolute path.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -8,6 +8,17 @@
},
"description": "List direct child names for a directory.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -8,6 +8,17 @@
},
"description": "Read a file from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"path": {
"allOf": [
{
@@ -8,6 +8,17 @@
},
"description": "Remove a file or directory tree from the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"force": {
"description": "Whether missing paths should be ignored. Defaults to `true`.",
"type": [
@@ -8,6 +8,17 @@
},
"description": "Write a file on the host filesystem.",
"properties": {
"cwd": {
"anyOf": [
{
"$ref": "#/definitions/AbsolutePathBuf"
},
{
"type": "null"
}
],
"description": "Optional cwd to resolve legacy sandbox workspace roots against."
},
"dataBase64": {
"description": "File contents encoded as base64.",
"type": "string"
@@ -2,20 +2,29 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* Copy a file or directory tree on the host filesystem.
*/
export type FsCopyParams = {
export type FsCopyParams = {
/**
* Absolute source path.
*/
sourcePath: AbsolutePathBuf,
sourcePath: AbsolutePathBuf,
/**
* Absolute destination path.
*/
destinationPath: AbsolutePathBuf,
destinationPath: AbsolutePathBuf,
/**
* Required for directory copies; ignored for file copies.
*/
recursive?: boolean, };
recursive?: boolean,
/**
* Optional sandbox policy for this filesystem copy.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -2,16 +2,25 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* Create a directory on the host filesystem.
*/
export type FsCreateDirectoryParams = {
export type FsCreateDirectoryParams = {
/**
* Absolute directory path to create.
*/
path: AbsolutePathBuf,
path: AbsolutePathBuf,
/**
* Whether parent directories should also be created. Defaults to `true`.
*/
recursive?: boolean | null, };
recursive?: boolean | null,
/**
* Optional sandbox policy for this filesystem mutation.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -2,12 +2,21 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* Request metadata for an absolute path.
*/
export type FsGetMetadataParams = {
export type FsGetMetadataParams = {
/**
* Absolute path to inspect.
*/
path: AbsolutePathBuf, };
path: AbsolutePathBuf,
/**
* Optional sandbox policy for this filesystem metadata lookup.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -2,12 +2,21 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* List direct child names for a directory.
*/
export type FsReadDirectoryParams = {
export type FsReadDirectoryParams = {
/**
* Absolute directory path to read.
*/
path: AbsolutePathBuf, };
path: AbsolutePathBuf,
/**
* Optional sandbox policy for this directory read.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -2,12 +2,21 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* Read a file from the host filesystem.
*/
export type FsReadFileParams = {
export type FsReadFileParams = {
/**
* Absolute path to read.
*/
path: AbsolutePathBuf, };
path: AbsolutePathBuf,
/**
* Optional sandbox policy for this filesystem read.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -2,20 +2,29 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* Remove a file or directory tree from the host filesystem.
*/
export type FsRemoveParams = {
export type FsRemoveParams = {
/**
* Absolute path to remove.
*/
path: AbsolutePathBuf,
path: AbsolutePathBuf,
/**
* Whether directory removal should recurse. Defaults to `true`.
*/
recursive?: boolean | null,
recursive?: boolean | null,
/**
* Whether missing paths should be ignored. Defaults to `true`.
*/
force?: boolean | null, };
force?: boolean | null,
/**
* Optional sandbox policy for this filesystem mutation.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -2,16 +2,25 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { SandboxPolicy } from "./SandboxPolicy";
/**
* Write a file on the host filesystem.
*/
export type FsWriteFileParams = {
export type FsWriteFileParams = {
/**
* Absolute path to write.
*/
path: AbsolutePathBuf,
path: AbsolutePathBuf,
/**
* File contents encoded as base64.
*/
dataBase64: string, };
dataBase64: string,
/**
* Optional sandbox policy for this filesystem write.
*/
sandboxPolicy?: SandboxPolicy | null,
/**
* Optional cwd to resolve legacy sandbox workspace roots against.
*/
cwd?: AbsolutePathBuf | null, };
@@ -1610,14 +1610,18 @@ mod tests {
request_id: RequestId::Integer(9),
params: v2::FsGetMetadataParams {
path: absolute_path("tmp/example"),
sandbox_policy: None,
cwd: None,
},
};
assert_eq!(
json!({
"method": "fs/getMetadata",
"id": 9,
"params": {
"path": absolute_path_string("tmp/example")
"params": {
"path": absolute_path_string("tmp/example"),
"sandboxPolicy": null,
"cwd": null
}
}),
serde_json::to_value(&request)?,
@@ -2170,6 +2170,12 @@ pub struct FeedbackUploadResponse {
pub struct FsReadFileParams {
/// Absolute path to read.
pub path: AbsolutePathBuf,
/// Optional sandbox policy for this filesystem read.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// Base64-encoded file contents returned by `fs/readFile`.
@@ -2190,6 +2196,12 @@ pub struct FsWriteFileParams {
pub path: AbsolutePathBuf,
/// File contents encoded as base64.
pub data_base64: String,
/// Optional sandbox policy for this filesystem write.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// Successful response for `fs/writeFile`.
@@ -2208,6 +2220,12 @@ pub struct FsCreateDirectoryParams {
/// Whether parent directories should also be created. Defaults to `true`.
#[ts(optional = nullable)]
pub recursive: Option<bool>,
/// Optional sandbox policy for this filesystem mutation.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// Successful response for `fs/createDirectory`.
@@ -2223,6 +2241,12 @@ pub struct FsCreateDirectoryResponse {}
pub struct FsGetMetadataParams {
/// Absolute path to inspect.
pub path: AbsolutePathBuf,
/// Optional sandbox policy for this filesystem metadata lookup.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// Metadata returned by `fs/getMetadata`.
@@ -2249,6 +2273,12 @@ pub struct FsGetMetadataResponse {
pub struct FsReadDirectoryParams {
/// Absolute directory path to read.
pub path: AbsolutePathBuf,
/// Optional sandbox policy for this directory read.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// A directory entry returned by `fs/readDirectory`.
@@ -2286,6 +2316,12 @@ pub struct FsRemoveParams {
/// Whether missing paths should be ignored. Defaults to `true`.
#[ts(optional = nullable)]
pub force: Option<bool>,
/// Optional sandbox policy for this filesystem mutation.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// Successful response for `fs/remove`.
@@ -2306,6 +2342,12 @@ pub struct FsCopyParams {
/// Required for directory copies; ignored for file copies.
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
pub recursive: bool,
/// Optional sandbox policy for this filesystem copy.
#[ts(optional = nullable)]
pub sandbox_policy: Option<SandboxPolicy>,
/// Optional cwd to resolve legacy sandbox workspace roots against.
#[ts(optional = nullable)]
pub cwd: Option<AbsolutePathBuf>,
}
/// Successful response for `fs/copy`.
@@ -6528,6 +6570,8 @@ mod tests {
fn fs_read_file_params_round_trip() {
let params = FsReadFileParams {
path: absolute_path("tmp/example.txt"),
sandbox_policy: None,
cwd: None,
};
let value = serde_json::to_value(&params).expect("serialize fs/readFile params");
@@ -6535,6 +6579,8 @@ mod tests {
value,
json!({
"path": absolute_path_string("tmp/example.txt"),
"sandboxPolicy": null,
"cwd": null,
})
);
@@ -6548,6 +6594,8 @@ mod tests {
let params = FsCreateDirectoryParams {
path: absolute_path("tmp/example"),
recursive: None,
sandbox_policy: None,
cwd: None,
};
let value = serde_json::to_value(&params).expect("serialize fs/createDirectory params");
@@ -6556,6 +6604,8 @@ mod tests {
json!({
"path": absolute_path_string("tmp/example"),
"recursive": null,
"sandboxPolicy": null,
"cwd": null,
})
);
@@ -6569,6 +6619,8 @@ mod tests {
let params = FsWriteFileParams {
path: absolute_path("tmp/example.bin"),
data_base64: "AAE=".to_string(),
sandbox_policy: None,
cwd: None,
};
let value = serde_json::to_value(&params).expect("serialize fs/writeFile params");
@@ -6577,6 +6629,8 @@ mod tests {
json!({
"path": absolute_path_string("tmp/example.bin"),
"dataBase64": "AAE=",
"sandboxPolicy": null,
"cwd": null,
})
);
@@ -6591,6 +6645,8 @@ mod tests {
source_path: absolute_path("tmp/source"),
destination_path: absolute_path("tmp/destination"),
recursive: true,
sandbox_policy: None,
cwd: None,
};
let value = serde_json::to_value(&params).expect("serialize fs/copy params");
@@ -6600,6 +6656,8 @@ mod tests {
"sourcePath": absolute_path_string("tmp/source"),
"destinationPath": absolute_path_string("tmp/destination"),
"recursive": true,
"sandboxPolicy": null,
"cwd": null,
})
);
+34 -7
View File
@@ -22,6 +22,7 @@ use codex_exec_server::CopyOptions;
use codex_exec_server::CreateDirectoryOptions;
use codex_exec_server::Environment;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemOperationOptions;
use codex_exec_server::RemoveOptions;
use std::io;
use std::sync::Arc;
@@ -46,7 +47,10 @@ impl FsApi {
) -> Result<FsReadFileResponse, JSONRPCErrorError> {
let bytes = self
.file_system
.read_file(&params.path)
.read_file_with_options(
&params.path,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsReadFileResponse {
@@ -64,7 +68,11 @@ impl FsApi {
))
})?;
self.file_system
.write_file(&params.path, bytes)
.write_file_with_options(
&params.path,
bytes,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsWriteFileResponse {})
@@ -75,11 +83,12 @@ impl FsApi {
params: FsCreateDirectoryParams,
) -> Result<FsCreateDirectoryResponse, JSONRPCErrorError> {
self.file_system
.create_directory(
.create_directory_with_options(
&params.path,
CreateDirectoryOptions {
recursive: params.recursive.unwrap_or(true),
},
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
@@ -92,7 +101,10 @@ impl FsApi {
) -> Result<FsGetMetadataResponse, JSONRPCErrorError> {
let metadata = self
.file_system
.get_metadata(&params.path)
.get_metadata_with_options(
&params.path,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsGetMetadataResponse {
@@ -109,7 +121,10 @@ impl FsApi {
) -> Result<FsReadDirectoryResponse, JSONRPCErrorError> {
let entries = self
.file_system
.read_directory(&params.path)
.read_directory_with_options(
&params.path,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsReadDirectoryResponse {
@@ -129,12 +144,13 @@ impl FsApi {
params: FsRemoveParams,
) -> Result<FsRemoveResponse, JSONRPCErrorError> {
self.file_system
.remove(
.remove_with_options(
&params.path,
RemoveOptions {
recursive: params.recursive.unwrap_or(true),
force: params.force.unwrap_or(true),
},
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
@@ -146,12 +162,13 @@ impl FsApi {
params: FsCopyParams,
) -> Result<FsCopyResponse, JSONRPCErrorError> {
self.file_system
.copy(
.copy_with_options(
&params.source_path,
&params.destination_path,
CopyOptions {
recursive: params.recursive,
},
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
@@ -159,6 +176,16 @@ impl FsApi {
}
}
fn fs_operation_options(
sandbox_policy: Option<codex_app_server_protocol::SandboxPolicy>,
cwd: Option<codex_utils_absolute_path::AbsolutePathBuf>,
) -> FileSystemOperationOptions {
FileSystemOperationOptions {
sandbox_policy: sandbox_policy.map(|policy| policy.to_core()),
cwd,
}
}
pub(crate) fn invalid_request(message: impl Into<String>) -> JSONRPCErrorError {
JSONRPCErrorError {
code: INVALID_REQUEST_ERROR_CODE,
+110
View File
@@ -13,7 +13,9 @@ use codex_app_server_protocol::FsUnwatchParams;
use codex_app_server_protocol::FsWatchResponse;
use codex_app_server_protocol::FsWriteFileParams;
use codex_app_server_protocol::JSONRPCNotification;
use codex_app_server_protocol::ReadOnlyAccess;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use serde_json::json;
@@ -61,6 +63,16 @@ fn absolute_path(path: PathBuf) -> AbsolutePathBuf {
AbsolutePathBuf::try_from(path).expect("path should be absolute")
}
fn read_only_sandbox_policy(readable_root: PathBuf) -> SandboxPolicy {
SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![absolute_path(readable_root)],
},
network_access: false,
}
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn fs_get_metadata_returns_only_used_fields() -> Result<()> {
let codex_home = TempDir::new()?;
@@ -71,6 +83,8 @@ async fn fs_get_metadata_returns_only_used_fields() -> Result<()> {
let request_id = mcp
.send_fs_get_metadata_request(codex_app_server_protocol::FsGetMetadataParams {
path: absolute_path(file_path.clone()),
sandbox_policy: None,
cwd: None,
})
.await?;
let response = timeout(
@@ -129,6 +143,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
.send_fs_create_directory_request(codex_app_server_protocol::FsCreateDirectoryParams {
path: absolute_path(nested_dir.clone()),
recursive: None,
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -141,6 +157,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
.send_fs_write_file_request(FsWriteFileParams {
path: absolute_path(nested_file.clone()),
data_base64: STANDARD.encode("hello from app-server"),
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -153,6 +171,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
.send_fs_write_file_request(FsWriteFileParams {
path: absolute_path(source_file.clone()),
data_base64: STANDARD.encode("hello from source root"),
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -164,6 +184,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
let read_request_id = mcp
.send_fs_read_file_request(codex_app_server_protocol::FsReadFileParams {
path: absolute_path(nested_file.clone()),
sandbox_policy: None,
cwd: None,
})
.await?;
let read_response: FsReadFileResponse = to_response(
@@ -185,6 +207,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
source_path: absolute_path(nested_file.clone()),
destination_path: absolute_path(copy_file_path.clone()),
recursive: false,
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -202,6 +226,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
source_path: absolute_path(source_dir.clone()),
destination_path: absolute_path(copied_dir.clone()),
recursive: true,
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -217,6 +243,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
let read_directory_request_id = mcp
.send_fs_read_directory_request(codex_app_server_protocol::FsReadDirectoryParams {
path: absolute_path(source_dir.clone()),
sandbox_policy: None,
cwd: None,
})
.await?;
let readdir_response = timeout(
@@ -249,6 +277,8 @@ async fn fs_methods_cover_current_fs_utils_surface() -> Result<()> {
path: absolute_path(copied_dir.clone()),
recursive: None,
force: None,
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -275,6 +305,8 @@ async fn fs_write_file_accepts_base64_bytes() -> Result<()> {
.send_fs_write_file_request(FsWriteFileParams {
path: absolute_path(file_path.clone()),
data_base64: STANDARD.encode(bytes),
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -287,6 +319,8 @@ async fn fs_write_file_accepts_base64_bytes() -> Result<()> {
let read_request_id = mcp
.send_fs_read_file_request(codex_app_server_protocol::FsReadFileParams {
path: absolute_path(file_path),
sandbox_policy: None,
cwd: None,
})
.await?;
let read_response: FsReadFileResponse = to_response(
@@ -316,6 +350,8 @@ async fn fs_write_file_rejects_invalid_base64() -> Result<()> {
.send_fs_write_file_request(FsWriteFileParams {
path: absolute_path(file_path),
data_base64: "%%%".to_string(),
sandbox_policy: None,
cwd: None,
})
.await?;
let error = timeout(
@@ -335,6 +371,70 @@ async fn fs_write_file_rejects_invalid_base64() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn fs_read_file_respects_sandbox_policy() -> Result<()> {
let codex_home = TempDir::new()?;
let allowed_dir = codex_home.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 mut mcp = initialized_mcp(&codex_home).await?;
let request_id = mcp
.send_fs_read_file_request(codex_app_server_protocol::FsReadFileParams {
path: absolute_path(file_path),
sandbox_policy: Some(read_only_sandbox_policy(allowed_dir)),
cwd: None,
})
.await?;
let response: FsReadFileResponse = to_response(
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
)
.await??,
)?;
assert_eq!(
response,
FsReadFileResponse {
data_base64: STANDARD.encode("sandboxed hello"),
}
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn fs_write_file_rejects_path_outside_sandbox_policy() -> Result<()> {
let codex_home = TempDir::new()?;
let allowed_dir = codex_home.path().join("allowed");
let blocked_path = codex_home.path().join("blocked.txt");
std::fs::create_dir_all(&allowed_dir)?;
let mut mcp = initialized_mcp(&codex_home).await?;
let request_id = mcp
.send_fs_write_file_request(FsWriteFileParams {
path: absolute_path(blocked_path.clone()),
data_base64: STANDARD.encode("nope"),
sandbox_policy: Some(read_only_sandbox_policy(allowed_dir)),
cwd: None,
})
.await?;
expect_error_message(
&mut mcp,
request_id,
format!(
"fs/write is not permitted by sandbox policy for path {}",
blocked_path.display()
)
.as_str(),
)
.await?;
assert!(!blocked_path.exists());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn fs_methods_reject_relative_paths() -> Result<()> {
let codex_home = TempDir::new()?;
@@ -471,6 +571,8 @@ async fn fs_copy_rejects_directory_without_recursive() -> Result<()> {
source_path: absolute_path(source_dir),
destination_path: absolute_path(codex_home.path().join("dest")),
recursive: false,
sandbox_policy: None,
cwd: None,
})
.await?;
let error = timeout(
@@ -498,6 +600,8 @@ async fn fs_copy_rejects_copying_directory_into_descendant() -> Result<()> {
source_path: absolute_path(source_dir.clone()),
destination_path: absolute_path(source_dir.join("nested").join("copy")),
recursive: true,
sandbox_policy: None,
cwd: None,
})
.await?;
let error = timeout(
@@ -529,6 +633,8 @@ async fn fs_copy_preserves_symlinks_in_recursive_copy() -> Result<()> {
source_path: absolute_path(source_dir),
destination_path: absolute_path(copied_dir.clone()),
recursive: true,
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -569,6 +675,8 @@ async fn fs_copy_ignores_unknown_special_files_in_recursive_copy() -> Result<()>
source_path: absolute_path(source_dir),
destination_path: absolute_path(copied_dir.clone()),
recursive: true,
sandbox_policy: None,
cwd: None,
})
.await?;
timeout(
@@ -606,6 +714,8 @@ async fn fs_copy_rejects_standalone_fifo_source() -> Result<()> {
source_path: absolute_path(fifo_path),
destination_path: absolute_path(codex_home.path().join("copied")),
recursive: false,
sandbox_policy: None,
cwd: None,
})
.await?;
expect_error_message(
+7 -2
View File
@@ -48,21 +48,26 @@ pub(crate) struct EnvironmentApplyPatchFileSystem {
}
impl EnvironmentApplyPatchFileSystem {
pub(crate) fn for_verification(file_system: Arc<dyn ExecutorFileSystem>) -> Self {
pub(crate) fn for_verification(file_system: Arc<dyn ExecutorFileSystem>, cwd: PathBuf) -> Self {
Self {
file_system,
operation_options: FileSystemOperationOptions::default(),
operation_options: FileSystemOperationOptions {
cwd: AbsolutePathBuf::from_absolute_path(cwd).ok(),
..FileSystemOperationOptions::default()
},
}
}
pub(crate) fn for_apply(
file_system: Arc<dyn ExecutorFileSystem>,
cwd: PathBuf,
sandbox_policy: SandboxPolicy,
) -> Self {
Self {
file_system,
operation_options: FileSystemOperationOptions {
sandbox_policy: Some(sandbox_policy),
cwd: AbsolutePathBuf::from_absolute_path(cwd).ok(),
},
}
}
+32 -14
View File
@@ -34,13 +34,13 @@ fn convert_apply_patch_maps_add_variant() {
#[derive(Default)]
struct RecordingExecutorFileSystem {
raw_reads: Mutex<Vec<PathBuf>>,
option_reads: Mutex<Vec<Option<SandboxPolicy>>>,
option_reads: Mutex<Vec<FileSystemOperationOptions>>,
raw_writes: Mutex<Vec<PathBuf>>,
option_writes: Mutex<Vec<Option<SandboxPolicy>>>,
option_writes: Mutex<Vec<FileSystemOperationOptions>>,
raw_creates: Mutex<Vec<PathBuf>>,
option_creates: Mutex<Vec<Option<SandboxPolicy>>>,
option_creates: Mutex<Vec<FileSystemOperationOptions>>,
raw_removes: Mutex<Vec<PathBuf>>,
option_removes: Mutex<Vec<Option<SandboxPolicy>>>,
option_removes: Mutex<Vec<FileSystemOperationOptions>>,
}
#[async_trait]
@@ -61,7 +61,7 @@ impl ExecutorFileSystem for RecordingExecutorFileSystem {
self.option_reads
.lock()
.expect("option_reads lock")
.push(options.sandbox_policy.clone());
.push(options.clone());
self.read_file(path).await
}
@@ -82,7 +82,7 @@ impl ExecutorFileSystem for RecordingExecutorFileSystem {
self.option_writes
.lock()
.expect("option_writes lock")
.push(options.sandbox_policy.clone());
.push(options.clone());
self.write_file(path, contents).await
}
@@ -107,7 +107,7 @@ impl ExecutorFileSystem for RecordingExecutorFileSystem {
self.option_creates
.lock()
.expect("option_creates lock")
.push(fs_options.sandbox_policy.clone());
.push(fs_options.clone());
self.create_directory(path, options).await
}
@@ -136,7 +136,7 @@ impl ExecutorFileSystem for RecordingExecutorFileSystem {
self.option_removes
.lock()
.expect("option_removes lock")
.push(fs_options.sandbox_policy.clone());
.push(fs_options.clone());
self.remove(path, options).await
}
@@ -153,7 +153,9 @@ impl ExecutorFileSystem for RecordingExecutorFileSystem {
#[tokio::test]
async fn verification_filesystem_uses_default_operation_options() {
let file_system = Arc::new(RecordingExecutorFileSystem::default());
let adapter = EnvironmentApplyPatchFileSystem::for_verification(file_system.clone());
let cwd = PathBuf::from("/tmp/apply-patch-verification");
let adapter =
EnvironmentApplyPatchFileSystem::for_verification(file_system.clone(), cwd.clone());
let content = adapter
.read_text(Path::new("/tmp/apply-patch-verification.txt"))
@@ -167,7 +169,10 @@ async fn verification_filesystem_uses_default_operation_options() {
.lock()
.expect("option_reads lock")
.as_slice(),
[None]
[FileSystemOperationOptions {
sandbox_policy: None,
cwd: Some(AbsolutePathBuf::from_absolute_path(&cwd).expect("absolute cwd")),
}]
);
assert_eq!(
file_system
@@ -184,9 +189,13 @@ async fn apply_filesystem_uses_sandbox_options() {
let file_system = Arc::new(RecordingExecutorFileSystem::default());
let sandbox_policy = SandboxPolicy::new_workspace_write_policy();
let path = Path::new("/tmp/apply-patch-sandboxed/new.txt");
let cwd = PathBuf::from("/tmp/apply-patch-sandboxed");
let action = ApplyPatchAction::new_add_for_test(path, "hello".to_string());
let adapter =
EnvironmentApplyPatchFileSystem::for_apply(file_system.clone(), sandbox_policy.clone());
let adapter = EnvironmentApplyPatchFileSystem::for_apply(
file_system.clone(),
cwd.clone(),
sandbox_policy.clone(),
);
codex_apply_patch::apply_action_with_fs(&action, &adapter)
.await
@@ -198,7 +207,10 @@ async fn apply_filesystem_uses_sandbox_options() {
.lock()
.expect("option_creates lock")
.as_slice(),
[Some(sandbox_policy.clone())]
[FileSystemOperationOptions {
sandbox_policy: Some(sandbox_policy.clone()),
cwd: Some(AbsolutePathBuf::from_absolute_path(&cwd).expect("absolute cwd")),
}]
);
assert_eq!(
file_system
@@ -206,6 +218,12 @@ async fn apply_filesystem_uses_sandbox_options() {
.lock()
.expect("option_writes lock")
.as_slice(),
[Some(sandbox_policy)]
[FileSystemOperationOptions {
sandbox_policy: Some(sandbox_policy),
cwd: Some(
AbsolutePathBuf::from_absolute_path(PathBuf::from("/tmp/apply-patch-sandboxed"))
.expect("absolute cwd")
),
}]
);
}
@@ -361,7 +361,10 @@ async fn parse_apply_patch_verified(
command: &[String],
cwd: &Path,
) -> codex_apply_patch::MaybeApplyPatchVerified {
let fs = EnvironmentApplyPatchFileSystem::for_verification(turn.environment.get_filesystem());
let fs = EnvironmentApplyPatchFileSystem::for_verification(
turn.environment.get_filesystem(),
turn.cwd.to_path_buf(),
);
codex_apply_patch::maybe_parse_apply_patch_verified_with_fs(command, cwd, &fs).await
}
@@ -212,6 +212,7 @@ impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {
) -> Result<ExecToolCallOutput, ToolError> {
let fs = EnvironmentApplyPatchFileSystem::for_apply(
ctx.turn.environment.get_filesystem(),
req.action.cwd.clone(),
req.sandbox_policy.clone(),
);
Self::run_with_environment_fs(req, fs).await
+68
View File
@@ -1,4 +1,5 @@
use async_trait::async_trait;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use tokio::io;
@@ -18,6 +19,12 @@ pub struct CopyOptions {
pub recursive: bool,
}
#[derive(Clone, Debug, Default, Eq, PartialEq)]
pub struct FileSystemOperationOptions {
pub sandbox_policy: Option<SandboxPolicy>,
pub cwd: Option<AbsolutePathBuf>,
}
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct FileMetadata {
pub is_directory: bool,
@@ -39,27 +46,88 @@ pub type FileSystemResult<T> = io::Result<T>;
pub trait ExecutorFileSystem: Send + Sync {
async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult<Vec<u8>>;
async fn read_file_with_options(
&self,
path: &AbsolutePathBuf,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<Vec<u8>> {
self.read_file(path).await
}
async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()>;
async fn write_file_with_options(
&self,
path: &AbsolutePathBuf,
contents: Vec<u8>,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
self.write_file(path, contents).await
}
async fn create_directory(
&self,
path: &AbsolutePathBuf,
options: CreateDirectoryOptions,
) -> FileSystemResult<()>;
async fn create_directory_with_options(
&self,
path: &AbsolutePathBuf,
create_directory_options: CreateDirectoryOptions,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
self.create_directory(path, create_directory_options).await
}
async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata>;
async fn get_metadata_with_options(
&self,
path: &AbsolutePathBuf,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<FileMetadata> {
self.get_metadata(path).await
}
async fn read_directory(
&self,
path: &AbsolutePathBuf,
) -> FileSystemResult<Vec<ReadDirectoryEntry>>;
async fn read_directory_with_options(
&self,
path: &AbsolutePathBuf,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
self.read_directory(path).await
}
async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()>;
async fn remove_with_options(
&self,
path: &AbsolutePathBuf,
remove_options: RemoveOptions,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
self.remove(path, remove_options).await
}
async fn copy(
&self,
source_path: &AbsolutePathBuf,
destination_path: &AbsolutePathBuf,
options: CopyOptions,
) -> FileSystemResult<()>;
async fn copy_with_options(
&self,
source_path: &AbsolutePathBuf,
destination_path: &AbsolutePathBuf,
copy_options: CopyOptions,
_options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
self.copy(source_path, destination_path, copy_options).await
}
}
@@ -1,4 +1,5 @@
use async_trait::async_trait;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use std::path::Component;
use std::path::Path;
@@ -11,6 +12,7 @@ use crate::CopyOptions;
use crate::CreateDirectoryOptions;
use crate::ExecutorFileSystem;
use crate::FileMetadata;
use crate::FileSystemOperationOptions;
use crate::FileSystemResult;
use crate::ReadDirectoryEntry;
use crate::RemoveOptions;
@@ -33,10 +35,29 @@ impl ExecutorFileSystem for LocalFileSystem {
tokio::fs::read(path.as_path()).await
}
async fn read_file_with_options(
&self,
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<Vec<u8>> {
enforce_read_access(path, options)?;
self.read_file(path).await
}
async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()> {
tokio::fs::write(path.as_path(), contents).await
}
async fn write_file_with_options(
&self,
path: &AbsolutePathBuf,
contents: Vec<u8>,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
enforce_write_access(path, options)?;
self.write_file(path, contents).await
}
async fn create_directory(
&self,
path: &AbsolutePathBuf,
@@ -50,6 +71,16 @@ impl ExecutorFileSystem for LocalFileSystem {
Ok(())
}
async fn create_directory_with_options(
&self,
path: &AbsolutePathBuf,
create_directory_options: CreateDirectoryOptions,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
enforce_write_access(path, options)?;
self.create_directory(path, create_directory_options).await
}
async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata> {
let metadata = tokio::fs::metadata(path.as_path()).await?;
Ok(FileMetadata {
@@ -60,6 +91,15 @@ impl ExecutorFileSystem for LocalFileSystem {
})
}
async fn get_metadata_with_options(
&self,
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<FileMetadata> {
enforce_read_access(path, options)?;
self.get_metadata(path).await
}
async fn read_directory(
&self,
path: &AbsolutePathBuf,
@@ -77,6 +117,15 @@ impl ExecutorFileSystem for LocalFileSystem {
Ok(entries)
}
async fn read_directory_with_options(
&self,
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
enforce_read_access(path, options)?;
self.read_directory(path).await
}
async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()> {
match tokio::fs::symlink_metadata(path.as_path()).await {
Ok(metadata) => {
@@ -97,6 +146,16 @@ impl ExecutorFileSystem for LocalFileSystem {
}
}
async fn remove_with_options(
&self,
path: &AbsolutePathBuf,
remove_options: RemoveOptions,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
enforce_write_access(path, options)?;
self.remove(path, remove_options).await
}
async fn copy(
&self,
source_path: &AbsolutePathBuf,
@@ -147,6 +206,70 @@ impl ExecutorFileSystem for LocalFileSystem {
.await
.map_err(|err| io::Error::other(format!("filesystem task failed: {err}")))?
}
async fn copy_with_options(
&self,
source_path: &AbsolutePathBuf,
destination_path: &AbsolutePathBuf,
copy_options: CopyOptions,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
enforce_read_access(source_path, options)?;
enforce_write_access(destination_path, options)?;
self.copy(source_path, destination_path, copy_options).await
}
}
fn enforce_read_access(
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
enforce_access(
path,
options,
FileSystemSandboxPolicy::can_read_path_with_cwd,
"read",
)
}
fn enforce_write_access(
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
enforce_access(
path,
options,
FileSystemSandboxPolicy::can_write_path_with_cwd,
"write",
)
}
fn enforce_access(
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
is_allowed: fn(&FileSystemSandboxPolicy, &Path, &Path) -> bool,
access_kind: &str,
) -> FileSystemResult<()> {
let Some(sandbox_policy) = &options.sandbox_policy else {
return Ok(());
};
let cwd = match &options.cwd {
Some(cwd) => cwd.clone().into_path_buf(),
None => std::env::current_dir()
.map_err(|err| io::Error::other(format!("failed to read current dir: {err}")))?,
};
let file_system_policy = FileSystemSandboxPolicy::from(sandbox_policy);
if is_allowed(&file_system_policy, path.as_path(), cwd.as_path()) {
Ok(())
} else {
Err(io::Error::new(
io::ErrorKind::InvalidInput,
format!(
"fs/{access_kind} is not permitted by sandbox policy for path {}",
path.as_path().display()
),
))
}
}
fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> {
+196 -3
View File
@@ -18,6 +18,7 @@ use crate::ExecServerClient;
use crate::ExecServerError;
use crate::ExecutorFileSystem;
use crate::FileMetadata;
use crate::FileSystemOperationOptions;
use crate::FileSystemResult;
use crate::ReadDirectoryEntry;
use crate::RemoveOptions;
@@ -42,7 +43,37 @@ impl ExecutorFileSystem for RemoteFileSystem {
trace!("remote fs read_file");
let response = self
.client
.fs_read_file(FsReadFileParams { path: path.clone() })
.fs_read_file(FsReadFileParams {
path: path.clone(),
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
STANDARD.decode(response.data_base64).map_err(|err| {
io::Error::new(
io::ErrorKind::InvalidData,
format!("remote fs/readFile returned invalid base64 dataBase64: {err}"),
)
})
}
async fn read_file_with_options(
&self,
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<Vec<u8>> {
trace!("remote fs read_file_with_options");
let response = self
.client
.fs_read_file(FsReadFileParams {
path: path.clone(),
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
STANDARD.decode(response.data_base64).map_err(|err| {
@@ -59,6 +90,30 @@ impl ExecutorFileSystem for RemoteFileSystem {
.fs_write_file(FsWriteFileParams {
path: path.clone(),
data_base64: STANDARD.encode(contents),
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
Ok(())
}
async fn write_file_with_options(
&self,
path: &AbsolutePathBuf,
contents: Vec<u8>,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
trace!("remote fs write_file_with_options");
self.client
.fs_write_file(FsWriteFileParams {
path: path.clone(),
data_base64: STANDARD.encode(contents),
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
@@ -75,6 +130,30 @@ impl ExecutorFileSystem for RemoteFileSystem {
.fs_create_directory(FsCreateDirectoryParams {
path: path.clone(),
recursive: Some(options.recursive),
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
Ok(())
}
async fn create_directory_with_options(
&self,
path: &AbsolutePathBuf,
create_directory_options: CreateDirectoryOptions,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
trace!("remote fs create_directory_with_options");
self.client
.fs_create_directory(FsCreateDirectoryParams {
path: path.clone(),
recursive: Some(create_directory_options.recursive),
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
@@ -85,7 +164,37 @@ impl ExecutorFileSystem for RemoteFileSystem {
trace!("remote fs get_metadata");
let response = self
.client
.fs_get_metadata(FsGetMetadataParams { path: path.clone() })
.fs_get_metadata(FsGetMetadataParams {
path: path.clone(),
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
Ok(FileMetadata {
is_directory: response.is_directory,
is_file: response.is_file,
created_at_ms: response.created_at_ms,
modified_at_ms: response.modified_at_ms,
})
}
async fn get_metadata_with_options(
&self,
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<FileMetadata> {
trace!("remote fs get_metadata_with_options");
let response = self
.client
.fs_get_metadata(FsGetMetadataParams {
path: path.clone(),
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
Ok(FileMetadata {
@@ -103,7 +212,40 @@ impl ExecutorFileSystem for RemoteFileSystem {
trace!("remote fs read_directory");
let response = self
.client
.fs_read_directory(FsReadDirectoryParams { path: path.clone() })
.fs_read_directory(FsReadDirectoryParams {
path: path.clone(),
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
Ok(response
.entries
.into_iter()
.map(|entry| ReadDirectoryEntry {
file_name: entry.file_name,
is_directory: entry.is_directory,
is_file: entry.is_file,
})
.collect())
}
async fn read_directory_with_options(
&self,
path: &AbsolutePathBuf,
options: &FileSystemOperationOptions,
) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
trace!("remote fs read_directory_with_options");
let response = self
.client
.fs_read_directory(FsReadDirectoryParams {
path: path.clone(),
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
Ok(response
@@ -124,6 +266,31 @@ impl ExecutorFileSystem for RemoteFileSystem {
path: path.clone(),
recursive: Some(options.recursive),
force: Some(options.force),
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
Ok(())
}
async fn remove_with_options(
&self,
path: &AbsolutePathBuf,
remove_options: RemoveOptions,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
trace!("remote fs remove_with_options");
self.client
.fs_remove(FsRemoveParams {
path: path.clone(),
recursive: Some(remove_options.recursive),
force: Some(remove_options.force),
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
@@ -142,6 +309,32 @@ impl ExecutorFileSystem for RemoteFileSystem {
source_path: source_path.clone(),
destination_path: destination_path.clone(),
recursive: options.recursive,
sandbox_policy: None,
cwd: None,
})
.await
.map_err(map_remote_error)?;
Ok(())
}
async fn copy_with_options(
&self,
source_path: &AbsolutePathBuf,
destination_path: &AbsolutePathBuf,
copy_options: CopyOptions,
options: &FileSystemOperationOptions,
) -> FileSystemResult<()> {
trace!("remote fs copy_with_options");
self.client
.fs_copy(FsCopyParams {
source_path: source_path.clone(),
destination_path: destination_path.clone(),
recursive: copy_options.recursive,
sandbox_policy: options
.sandbox_policy
.clone()
.map(codex_app_server_protocol::SandboxPolicy::from),
cwd: options.cwd.clone(),
})
.await
.map_err(map_remote_error)?;
@@ -22,6 +22,7 @@ use codex_app_server_protocol::JSONRPCErrorError;
use crate::CopyOptions;
use crate::CreateDirectoryOptions;
use crate::ExecutorFileSystem;
use crate::FileSystemOperationOptions;
use crate::RemoveOptions;
use crate::local_file_system::LocalFileSystem;
use crate::rpc::internal_error;
@@ -39,7 +40,10 @@ impl FileSystemHandler {
) -> Result<FsReadFileResponse, JSONRPCErrorError> {
let bytes = self
.file_system
.read_file(&params.path)
.read_file_with_options(
&params.path,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsReadFileResponse {
@@ -57,7 +61,11 @@ impl FileSystemHandler {
))
})?;
self.file_system
.write_file(&params.path, bytes)
.write_file_with_options(
&params.path,
bytes,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsWriteFileResponse {})
@@ -68,11 +76,12 @@ impl FileSystemHandler {
params: FsCreateDirectoryParams,
) -> Result<FsCreateDirectoryResponse, JSONRPCErrorError> {
self.file_system
.create_directory(
.create_directory_with_options(
&params.path,
CreateDirectoryOptions {
recursive: params.recursive.unwrap_or(true),
},
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
@@ -85,7 +94,10 @@ impl FileSystemHandler {
) -> Result<FsGetMetadataResponse, JSONRPCErrorError> {
let metadata = self
.file_system
.get_metadata(&params.path)
.get_metadata_with_options(
&params.path,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsGetMetadataResponse {
@@ -102,7 +114,10 @@ impl FileSystemHandler {
) -> Result<FsReadDirectoryResponse, JSONRPCErrorError> {
let entries = self
.file_system
.read_directory(&params.path)
.read_directory_with_options(
&params.path,
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
Ok(FsReadDirectoryResponse {
@@ -122,12 +137,13 @@ impl FileSystemHandler {
params: FsRemoveParams,
) -> Result<FsRemoveResponse, JSONRPCErrorError> {
self.file_system
.remove(
.remove_with_options(
&params.path,
RemoveOptions {
recursive: params.recursive.unwrap_or(true),
force: params.force.unwrap_or(true),
},
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
@@ -139,12 +155,13 @@ impl FileSystemHandler {
params: FsCopyParams,
) -> Result<FsCopyResponse, JSONRPCErrorError> {
self.file_system
.copy(
.copy_with_options(
&params.source_path,
&params.destination_path,
CopyOptions {
recursive: params.recursive,
},
&fs_operation_options(params.sandbox_policy, params.cwd),
)
.await
.map_err(map_fs_error)?;
@@ -152,6 +169,16 @@ impl FileSystemHandler {
}
}
fn fs_operation_options(
sandbox_policy: Option<codex_app_server_protocol::SandboxPolicy>,
cwd: Option<codex_utils_absolute_path::AbsolutePathBuf>,
) -> FileSystemOperationOptions {
FileSystemOperationOptions {
sandbox_policy: sandbox_policy.map(|policy| policy.to_core()),
cwd,
}
}
fn map_fs_error(err: io::Error) -> JSONRPCErrorError {
if err.kind() == io::ErrorKind::InvalidInput {
invalid_request(err.to_string())
+105
View File
@@ -12,8 +12,11 @@ use codex_exec_server::CopyOptions;
use codex_exec_server::CreateDirectoryOptions;
use codex_exec_server::Environment;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemOperationOptions;
use codex_exec_server::ReadDirectoryEntry;
use codex_exec_server::RemoveOptions;
use codex_protocol::protocol::ReadOnlyAccess;
use codex_protocol::protocol::SandboxPolicy;
use codex_utils_absolute_path::AbsolutePathBuf;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
@@ -56,6 +59,48 @@ fn absolute_path(path: std::path::PathBuf) -> AbsolutePathBuf {
}
}
fn read_only_options(readable_root: std::path::PathBuf) -> FileSystemOperationOptions {
FileSystemOperationOptions {
sandbox_policy: Some(SandboxPolicy::ReadOnly {
access: ReadOnlyAccess::Restricted {
include_platform_defaults: false,
readable_roots: vec![absolute_path(readable_root)],
},
network_access: false,
}),
cwd: None,
}
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn file_system_write_with_sandbox_policy_uses_supplied_cwd(use_remote: bool) -> Result<()> {
let context = create_file_system_context(use_remote).await?;
let file_system = context.file_system;
let tmp = TempDir::new()?;
let workspace_root = absolute_path(tmp.path().to_path_buf());
let file_path = tmp.path().join("workspace-write.txt");
let options = FileSystemOperationOptions {
sandbox_policy: Some(SandboxPolicy::new_workspace_write_policy()),
cwd: Some(workspace_root),
};
file_system
.write_file_with_options(
&absolute_path(file_path.clone()),
b"allowed".to_vec(),
&options,
)
.await
.with_context(|| format!("mode={use_remote}"))?;
assert_eq!(std::fs::read_to_string(file_path)?, "allowed");
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -212,6 +257,66 @@ async fn file_system_copy_rejects_directory_without_recursive(use_remote: bool)
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn file_system_read_with_sandbox_policy_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 contents = file_system
.read_file_with_options(&absolute_path(file_path), &read_only_options(allowed_dir))
.await
.with_context(|| format!("mode={use_remote}"))?;
assert_eq!(contents, b"sandboxed hello");
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn file_system_write_with_sandbox_policy_rejects_unwritable_path(
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 blocked_path = tmp.path().join("blocked.txt");
std::fs::create_dir_all(&allowed_dir)?;
let error = match file_system
.write_file_with_options(
&absolute_path(blocked_path.clone()),
b"nope".to_vec(),
&read_only_options(allowed_dir),
)
.await
{
Ok(()) => anyhow::bail!("write should be blocked"),
Err(error) => error,
};
assert_eq!(error.kind(), std::io::ErrorKind::InvalidInput);
assert_eq!(
error.to_string(),
format!(
"fs/write is not permitted by sandbox policy for path {}",
blocked_path.display()
)
);
assert!(!blocked_path.exists());
Ok(())
}
#[test_case(false ; "local")]
#[test_case(true ; "remote")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]