Skip to content

Commit a8b74a4

Browse files
NathanFlurryclaude
andauthored
feat(python): support file delete + rename in the Pyodide VFS bridge (#138)
Wire os.remove/os.rmdir/os.rename/os.replace through to the kernel VFS: - runner: implement the rename/unlink/rmdir node_ops (were ENOSYS stubs) + add fsUnlink/fsRmdir/fsRename to both RPC bridges - execution: add Unlink/Rmdir/Rename to PythonVfsRpcMethod (+ wire destination field) and route them to the filesystem dispatcher - filesystem: dispatch to kernel.remove_file/remove_dir/rename AND mirror into the host-side shadow (remove/rename_guest_shadow_path) so a later shadow->kernel sync can't resurrect a just-deleted entry Test: python_runtime_supports_file_delete_and_rename verifies the ops in-isolate and cross-checks the host kernel VFS. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 27b48c5 commit a8b74a4

9 files changed

Lines changed: 1545 additions & 61 deletions

File tree

crates/execution/assets/runners/python-runner.mjs

Lines changed: 324 additions & 29 deletions
Large diffs are not rendered by default.

crates/execution/src/python.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ pub enum PythonVfsRpcMethod {
5050
Stat,
5151
ReadDir,
5252
Mkdir,
53+
Unlink,
54+
Rmdir,
55+
Rename,
5356
HttpRequest,
5457
DnsLookup,
5558
SubprocessRun,
@@ -63,6 +66,9 @@ impl PythonVfsRpcMethod {
6366
"fsStat" => Some(Self::Stat),
6467
"fsReaddir" => Some(Self::ReadDir),
6568
"fsMkdir" => Some(Self::Mkdir),
69+
"fsUnlink" => Some(Self::Unlink),
70+
"fsRmdir" => Some(Self::Rmdir),
71+
"fsRename" => Some(Self::Rename),
6672
"httpRequest" => Some(Self::HttpRequest),
6773
"dnsLookup" => Some(Self::DnsLookup),
6874
"subprocessRun" => Some(Self::SubprocessRun),
@@ -76,6 +82,8 @@ pub struct PythonVfsRpcRequest {
7682
pub id: u64,
7783
pub method: PythonVfsRpcMethod,
7884
pub path: String,
85+
/// Second path for `Rename` (the destination); `None` for other methods.
86+
pub destination: Option<String>,
7987
pub content_base64: Option<String>,
8088
pub recursive: bool,
8189
pub url: Option<String>,
@@ -137,6 +145,8 @@ struct PythonVfsBridgeRequestWire {
137145
#[serde(default)]
138146
path: String,
139147
#[serde(default)]
148+
destination: Option<String>,
149+
#[serde(default)]
140150
content_base64: Option<String>,
141151
#[serde(default)]
142152
recursive: bool,
@@ -1176,6 +1186,7 @@ fn parse_python_bridge_sync_rpc_request(
11761186
id: request.id,
11771187
method,
11781188
path: wire.path,
1189+
destination: wire.destination,
11791190
content_base64: wire.content_base64,
11801191
recursive: wire.recursive,
11811192
url: wire.url,

crates/sidecar/src/execution.rs

Lines changed: 331 additions & 18 deletions
Large diffs are not rendered by default.

crates/sidecar/src/filesystem.rs

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,38 @@ where
786786
.mkdir(&path, request.recursive)
787787
.map(|()| PythonVfsRpcResponsePayload::Empty)
788788
.map_err(kernel_error),
789+
// Mirror the delete/rename into the host-side shadow too, the
790+
// same way the wire `GuestFilesystemOperation` handlers do —
791+
// otherwise a later shadow→kernel sync would resurrect the
792+
// entry the guest just removed.
793+
PythonVfsRpcMethod::Unlink => {
794+
match vm.kernel.remove_file(&path).map_err(kernel_error) {
795+
Ok(()) => remove_guest_shadow_path(vm, &path)
796+
.map(|()| PythonVfsRpcResponsePayload::Empty),
797+
Err(error) => Err(error),
798+
}
799+
}
800+
PythonVfsRpcMethod::Rmdir => {
801+
match vm.kernel.remove_dir(&path).map_err(kernel_error) {
802+
Ok(()) => remove_guest_shadow_path(vm, &path)
803+
.map(|()| PythonVfsRpcResponsePayload::Empty),
804+
Err(error) => Err(error),
805+
}
806+
}
807+
PythonVfsRpcMethod::Rename => {
808+
let destination = request.destination.as_deref().ok_or_else(|| {
809+
SidecarError::InvalidState(format!(
810+
"python VFS fsRename for {} requires destination",
811+
path
812+
))
813+
})?;
814+
let destination = normalize_python_vfs_rpc_path(destination)?;
815+
match vm.kernel.rename(&path, &destination).map_err(kernel_error) {
816+
Ok(()) => rename_guest_shadow_path(vm, &path, &destination)
817+
.map(|()| PythonVfsRpcResponsePayload::Empty),
818+
Err(error) => Err(error),
819+
}
820+
}
789821
PythonVfsRpcMethod::HttpRequest
790822
| PythonVfsRpcMethod::DnsLookup
791823
| PythonVfsRpcMethod::SubprocessRun => {
@@ -860,16 +892,13 @@ pub(crate) fn normalize_python_vfs_rpc_path(path: &str) -> Result<String, Sideca
860892
)));
861893
}
862894

895+
// Root is `/`: Python may address the whole guest VFS. Textual `..` segments
896+
// are resolved by `normalize_path`, and the kernel enforces fs permissions
897+
// plus mount-confinement (openat2 RESOLVE_BENEATH refuses escaping symlinks)
898+
// on every op — so confinement is the kernel's job, not a prefix check here.
863899
let normalized = normalize_path(path);
864-
if normalized == PYTHON_VFS_RPC_GUEST_ROOT
865-
|| normalized.starts_with(&format!("{PYTHON_VFS_RPC_GUEST_ROOT}/"))
866-
{
867-
Ok(normalized)
868-
} else {
869-
Err(SidecarError::InvalidState(format!(
870-
"python VFS RPC path {normalized} escapes guest workspace root {PYTHON_VFS_RPC_GUEST_ROOT}"
871-
)))
872-
}
900+
debug_assert_eq!(PYTHON_VFS_RPC_GUEST_ROOT, "/");
901+
Ok(normalized)
873902
}
874903

875904
/// Kernel-VFS-backed reader for the module resolver. The resolution algorithm

crates/sidecar/src/state.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ pub(crate) const EXECUTION_DRIVER_NAME: &str = "secure-exec-sidecar-execution";
5252
pub(crate) const JAVASCRIPT_COMMAND: &str = "node";
5353
pub(crate) const PYTHON_COMMAND: &str = "python";
5454
pub(crate) const WASM_COMMAND: &str = "wasm";
55-
pub(crate) const PYTHON_VFS_RPC_GUEST_ROOT: &str = "/workspace";
55+
// The Python runtime addresses the whole guest VFS (the kernel enforces fs
56+
// permissions and mount-confinement on every op, identical to what the JS/WASM
57+
// runtimes and `vm.readFile()` see), so the VFS-RPC root is `/`, not a single
58+
// workspace dir.
59+
pub(crate) const PYTHON_VFS_RPC_GUEST_ROOT: &str = "/";
5660
pub(crate) const EXECUTION_SANDBOX_ROOT_ENV: &str = "AGENTOS_SANDBOX_ROOT";
5761
pub(crate) const WASM_STDIO_SYNC_RPC_ENV: &str = "AGENTOS_WASI_STDIO_SYNC_RPC";
5862
#[cfg(test)]

crates/sidecar/src/vm.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ const SHADOW_ROOT_BOOTSTRAP_DIRS: &[(&str, u32)] = &[
104104

105105
pub(crate) const DEFAULT_GUEST_PATH_ENV: &str =
106106
"/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin";
107+
#[cfg(test)]
107108
const KERNEL_COMMAND_STUB: &[u8] = b"#!/bin/sh\n# kernel command stub\n";
108109
pub(crate) const MAX_VM_LAYERS: usize = 256;
109110

@@ -222,6 +223,10 @@ where
222223
let mut execution_commands = vec![
223224
String::from(JAVASCRIPT_COMMAND),
224225
String::from(PYTHON_COMMAND),
226+
// `python3` resolves to the same Pyodide runtime; register it so the
227+
// guest shell can find `/bin/python3` on PATH (the command resolver
228+
// already rewrites the alias to `python`).
229+
String::from("python3"),
225230
String::from(WASM_COMMAND),
226231
];
227232
execution_commands.extend(command_guest_paths.keys().cloned());
@@ -231,7 +236,6 @@ where
231236
execution_commands,
232237
))
233238
.map_err(kernel_error)?;
234-
prune_kernel_command_stub(&mut kernel, "/bin/python")?;
235239
if let Some(root) = kernel.root_filesystem_mut() {
236240
root.finish_bootstrap();
237241
}
@@ -2043,6 +2047,9 @@ pub(crate) fn normalize_dns_hostname(hostname: &str) -> Result<String, SidecarEr
20432047
Ok(normalized)
20442048
}
20452049

2050+
// Retained for the native-root command-stub test; `python` is now a real
2051+
// command so production no longer prunes `/bin/python`.
2052+
#[cfg(test)]
20462053
fn prune_kernel_command_stub(
20472054
kernel: &mut KernelVm<secure_exec_kernel::mount_table::MountTable>,
20482055
path: &str,

0 commit comments

Comments
 (0)