Skip to content

Commit bb09b69

Browse files
committed
fix(native-sidecar): use one universal resolve-beneath walk for host-mount confinement
Replace the Linux openat2(RESOLVE_BENEATH) path and the separate macOS cap-std fallback with a single portable resolve-beneath walker. gVisor/runsc (Rivet Compute) does not support openat2(RESOLVE_BENEATH), so a guest chdir/stat into a host mount returned ENOENT under runsc while passing under runc. The walker descends one component at a time with plain openat(2) (O_NOFOLLOW), keeps its own ancestor-fd stack for .., expands symlinks manually, and rejects absolute components/targets as escapes. - Delete src/macos_fs.rs and drop the cap-std dependency - Move the universal resolver into plugins/host_dir.rs as the confine module - Point filesystem.rs at crate::plugins::host_dir::confine - Document why openat2 is not used across the affected modules and AGENTS.md
1 parent d9179cc commit bb09b69

13 files changed

Lines changed: 635 additions & 750 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 90 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ These are hard rules with no exceptions:
106106
- **Filesystem-side teardown races must fail closed too.** In `crates/sidecar/src/filesystem.rs`, guest filesystem and Python VFS handlers should treat missing VMs or active processes as stale teardown races: log and return a rejection/`Ok(())` instead of `expect(...)`, because dispose can win after a request is queued but before the response is emitted.
107107
- **Mapped host paths in `crates/sidecar/src/filesystem.rs` must resolve symlinks against the mapping root before touching the real host.** For Python/Pyodide cache and other `AGENT_OS_GUEST_PATH_MAPPINGS` accesses, walk existing path components with `symlink_metadata`, reject targets that leave the mapped root, and filter `readdir` results the same way so guest `stat`/`listdir` calls cannot leak host metadata through escaped symlinks.
108108
- **Mapped host-path metadata and symlink ops must not require a pre-existing kernel mirror.** In `crates/sidecar/src/filesystem.rs`, guest writes under `AGENT_OS_GUEST_PATH_MAPPINGS` land on the host first, so `chmod`, `utimes`, `symlink`, and `readlink` need to operate on the mapped host path even when the kernel shadow entry has not been synced yet; only mirror metadata back into the kernel when the guest path already exists there.
109-
- **`host_dir` metadata mutations must use anchored `openat2` handles and reject symlink leaves.** In `crates/sidecar/src/plugins/host_dir.rs`, route `chmod`, `chown`, and `utimes` through `open_beneath(... O_PATH | O_NOFOLLOW ...)` before mutating metadata, and fail closed on symlink leaves so a mounted host directory cannot retarget those ops onto escaped host paths.
109+
- **`host_dir` metadata mutations must use anchored resolve-beneath handles and reject symlink leaves.** In `crates/native-sidecar/src/plugins/host_dir.rs`, route `chmod`, `chown`, and `utimes` through `open_metadata_beneath(...)` (which resolves via the universal `confine::resolve_beneath` walk, `lstat`s the leaf through the anchored parent fd, rejects symlink leaves with `EPERM`, and opens the target `O_RDONLY | O_NOFOLLOW`) before mutating metadata fd-relative (`fchmod`/`fchown`/`futimens`). Do NOT reintroduce `openat2`, an `O_PATH` anchor, `/proc/self/fd` re-opens, or a macOS-specific path — the confinement boundary is a single universal implementation: the `confine` module in `crates/native-sidecar/src/plugins/host_dir.rs` (see that module and `crates/native-sidecar/AGENTS.md` for why `openat2` was removed: gVisor/runsc incompatibility plus single-source maintenance).
110110
- **`VirtualStat` schema changes must cross every filesystem boundary together.** When adding stat metadata for mount plugins such as `host_dir`, update the Rust `VirtualStat` constructors (`kernel`, `device_layer`, plugin adapters), sidecar filesystem JSON serialization, JS bridge conversions, and the V8 `Stats` shim in the same change or the extra metadata gets truncated before guest code can observe it.
111111
- **Stateful JS crypto sync RPCs in `crates/sidecar/src/execution.rs` are per-process state.** Keep `service_javascript_crypto_sync_rpc` threaded with `&mut ActiveProcess` for cipher/Diffie-Hellman session handlers, and for AES-GCM treat `authTagLength` as auth-tag output sizing instead of calling `Crypter::set_tag_len()` during encryption on the current OpenSSL provider.
112112
- **`net.poll` waits in `crates/sidecar/src/execution.rs` must stay explicitly bounded.** `service_javascript_net_sync_rpc(...)` runs on the sidecar's sync-RPC main thread, so clamp guest `wait_ms` values to the 50 ms ceiling in `clamp_javascript_net_poll_wait(...)`; longer waits should return the currently observed socket state after the ceiling expires instead of monopolizing dispose/shutdown or unrelated VM work.

crates/native-sidecar/CLAUDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,6 @@ Migration status: **resource limits** (typed `*ExecutionLimits` on the execution
3131
- `plugins/host_dir.rs` metadata writes must keep the old symlink-leaf safety contract for plain ops (`chmod`/`chown`/`utimes` still reject symlink leaves), while the richer timestamp path should only mutate symlink metadata when the caller explicitly requests nofollow semantics (`lutimes` / `utimes_spec(..., false)`).
3232
- Mounted filesystem shutdown now happens explicitly during `src/vm.rs` disposal/reconfigure, not just in `MountTable::drop`. Bridge-backed mounts can therefore emit `SidecarRequestPayload::JsBridgeCall` during teardown, and host-visible flush failures should surface as the structured event `filesystem.mount.shutdown_failed` with the mount metadata plus the original error code/message.
3333
- Guest runtime env setup in `src/execution.rs` must add writable `host_dir` / `module_access` mount roots to `AGENTOS_EXTRA_FS_WRITE_PATHS`, not just the VM shadow cwd. Without those extra write roots, guest `fs.*` bridge calls misclassify writable mounts as read-only (`EROFS`) and cross-mount rename tests never reach the intended `EXDEV` path.
34-
- Python mapped host-path access in `src/filesystem.rs` must stay on the anchored-fd path: open the mapped root once, resolve descendants with `openat2(RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS)`, and perform the actual syscall through `/proc/self/fd/<fd>` or an anchored parent dir. Do not reintroduce resolve-then-use `PathBuf` opens, and when filtering `read_dir` results from a proc-fd directory, rebuild child host paths from the resolved directory host path plus `file_name()` instead of reusing `DirEntry::path()`, which points back into procfs.
34+
- Host-mount confinement uses ONE universal resolve-beneath walk, the `confine` module in `src/plugins/host_dir.rs` (`crate::plugins::host_dir::confine::resolve_beneath` / `::read_dir`), for every platform. Do NOT split it back out into a separate `src/fs.rs` module, and do NOT reintroduce `openat2(RESOLVE_BENEATH)`, an `O_PATH` metadata anchor, `/proc/self/fd/<fd>` re-opens, a macOS/`cap-std` fallback, or any per-platform split of this boundary. `openat2` was deleted because gVisor/runsc (Rivet Compute) does not support `openat2(RESOLVE_BENEATH)` — a guest `chdir`/`stat` into a host mount returned `ENOENT` under runsc while passing under runc — and because maintaining a second macOS implementation doubled the escape-bug surface. The walk descends one component at a time with plain `openat(2)` (`O_NOFOLLOW`), keeps its own ancestor-fd stack for `..` (never asking the kernel to resolve `..`), expands symlinks manually, and rejects absolute components/targets as escapes.
35+
- Python mapped host-path access in `src/filesystem.rs` and all `host_dir` ops must stay on the anchored-fd path: resolve descendants via `confine::resolve_beneath` (from `crate::plugins::host_dir`), then perform the actual syscall fd-relative against the returned `OwnedFd` (`fstat`/`fchmod`/`fchown`/`futimens`, fd `read`/`write`, or `*at` against an anchored parent dir). `confine::Resolved::real_path` is diagnostic/logical only — never re-open it as an authority handle. Do not reintroduce resolve-then-use `PathBuf` opens, and enumerate directories with `confine::read_dir(dir_fd)` (fd-based, classifying entries via the directory fd) instead of a path-based `fs::read_dir`.
3536
- In `tests/builtin_conformance.rs`, isolated extra tests that open a host listener should finish sidecar/session/VM setup before starting any accept deadline. Those tests run in subprocesses that can queue behind the shared sidecar-runtime lock, so a server timeout that starts before VM creation will flake under the normal parallel `cargo test` harness.

crates/native-sidecar/Cargo.toml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ rustls-native-certs = "0.8"
4949
rustls-pemfile = "2.2"
5050
tokio-rustls = { version = "0.26", default-features = false, features = ["aws_lc_rs", "tls12"] }
5151
rusqlite = { version = "0.32", features = ["bundled"] }
52+
# `rustix` provides the safe (owned-fd, no `unsafe`) `openat(2)`/`statat`/
53+
# `readlinkat`/`Dir` primitives used by the universal resolve-beneath
54+
# implementation in `src/fs.rs`. Plain `openat(2)` is portable across Linux,
55+
# macOS, and gVisor; `openat2` is deliberately NOT used (see `src/fs.rs`).
56+
rustix = { version = "1", features = ["fs"] }
5257
scrypt = "0.11"
5358
serde = { version = "1.0", features = ["derive"] }
5459
serde_json = "1.0"
@@ -62,11 +67,6 @@ ureq = { version = "2.10", features = ["json"] }
6267
url = "2"
6368
vfs = { workspace = true }
6469

65-
[target.'cfg(target_os = "macos")'.dependencies]
66-
# macOS has no openat2/RESOLVE_BENEATH; cap-std provides the audited
67-
# resolve-beneath path resolution used in place of openat2 on this platform.
68-
cap-std = "3"
69-
7070
[dev-dependencies]
7171
serde_bare = "0.5"
7272
tar = "0.4"

crates/native-sidecar/src/execution.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11099,7 +11099,7 @@ fn runtime_guest_path_mappings(vm: &VmState) -> Vec<RuntimeGuestPathMapping> {
1109911099
/// inline against this reader — concurrently with the service loop — so a large
1110011100
/// cold-start module graph never serializes behind / starves an in-flight ACP
1110111101
/// `session/new` bootstrap on the single service-loop thread. The reader reads
11102-
/// the same mounted tree the guest sees (anchored `openat2`, escaping-symlink
11102+
/// the same mounted tree the guest sees (anchored resolve-beneath, escaping-symlink
1110311103
/// refusal), never the host-direct path translator. Returns `None` when the VM
1110411104
/// has no usable read-only mount, so resolution falls back to the service-loop
1110511105
/// kernel reader.

0 commit comments

Comments
 (0)