Skip to content

Commit 3d9feab

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 3d9feab

13 files changed

Lines changed: 1004 additions & 771 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 ops must resolve beneath the mount root, reject symlink leaves, and NOT require read on the target (non-root sidecar).** In `crates/native-sidecar/src/plugins/host_dir.rs`: `chown`/`utimes` resolve the parent via `split_parent`, `reject_symlink_leaf` (an `fstatat` `lstat` through the anchored parent fd → `EPERM` on a symlink), then mutate with `fchownat`/`utimensat(.., AT_SYMLINK_NOFOLLOW)` against `(parent_fd, leaf_name)` — no leaf open, so no read requirement, and `AT_SYMLINK_NOFOLLOW` closes the check→mutate swap race. `stat`/`exists` use `confine::resolve_parent_beneath` + `fstatat`. `chmod` is the deliberate exception: `fchmodat` has no `AT_SYMLINK_NOFOLLOW`, so it keeps `open_metadata_beneath` (leaf `O_RDONLY | O_NOFOLLOW`) + `fchmod` for TOCTOU safety, accepting the read requirement. Do NOT reintroduce `openat2`, an `O_PATH` *metadata/leaf* anchor, `/proc/self/fd` re-opens, or a macOS-specific path — the confinement boundary is the single universal `confine` module (see it and `crates/native-sidecar/CLAUDE.md` for why `openat2` was removed and why the non-root read constraint drives these choices). An `O_PATH` *directory traversal* anchor for search-only intermediate dirs (`confine::open_dir_anchor`, Linux-only, `EACCES` fallback) is allowed and is not a metadata anchor.
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: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,11 @@ 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+
- The native sidecar must run UNPRIVILEGED (non-root uid) AND under gVisor/runsc (Rivet Compute), so host-mount confinement cannot assume either root DAC bypass or Linux-only syscalls. Two consequences bind the `confine` boundary: (1) `openat2(RESOLVE_BENEATH)` is unsupported under gVisor, so the resolve-beneath walk must use plain `openat(2)` (see the `confine` module); (2) because the sidecar uid is not root, DAC permission checks are real and are NOT masked away — so confinement must not require MORE access than POSIX does. Concretely, metadata ops must not require READ permission on the target file (POSIX only requires search on the parent). Rules:
35+
- `stat`/`exists` use `confine::resolve_parent_beneath` + `fstatat`/`fstat` against `(parent_fd, leaf_name)` — never `open_beneath(O_RDONLY)` on the leaf, which would falsely report a statable-but-unreadable file (mode `0600` owned by another uid, or `0000`) as missing/`EACCES` under a non-root sidecar.
36+
- `chown`/`utimes` use `split_parent` + `fchownat`/`utimensat(.., AT_SYMLINK_NOFOLLOW/NoFollowSymlink)` after `reject_symlink_leaf`. `AT_SYMLINK_NOFOLLOW` on the mutation both keeps the leaf read-free AND closes the check→mutate symlink-swap TOCTOU (a leaf swapped to a symlink after the reject-check is operated on in place, staying confined, never followed to an escaped host path).
37+
- `chmod` is the ONE deliberate exception: `fchmodat` has no `AT_SYMLINK_NOFOLLOW`, so a leaf-free `chmod` cannot close that swap race. `chmod` therefore keeps the `open_metadata_beneath` leaf open (`O_RDONLY | O_NOFOLLOW`) + `fchmod`, accepting the non-root READ requirement in exchange for TOCTOU safety. Do NOT "fix" it by switching to `fchmodat(parent, name)` — that reintroduces a symlink-swap host-escape.
38+
- A SEARCH-ONLY directory in the path (mode `0711`/`0111`: execute without read for the sidecar uid) must still be traversable, both mid-walk (intermediate dirs, via `confine::open_dir_anchor`) AND as the immediate *parent* opened for `*at` ops (`split_parent` → `open_dir_anchor_beneath` → `confine::resolve_dir_anchor_beneath`, NOT `open_directory_beneath`). Each tries `O_RDONLY` and on Linux falls back to `O_PATH` on `EACCES`. This `O_PATH` is a directory *traversal anchor only* (used solely as the `dirfd` for `statat`/`openat`/`readlinkat`/`fstatat`/`fchownat`/`utimensat`/…, never `read`/`readdir`/`fchmod`), so it does NOT violate the metadata-anchor prohibition below. Keep `readdir` on `open_directory_beneath` (`O_RDONLY`) so LISTING a search-only dir still fails `EACCES` as POSIX requires. The fallback is additive and cannot regress gVisor (it only runs after `O_RDONLY` already failed); full coverage of this case under gVisor still assumes gVisor honors `O_PATH` dir fds as `*at` anchors, which should be confirmed — but if it does not, only the already-failing search-only case is affected, nothing else.
39+
- 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` / `::resolve_parent_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/leaf* anchor, `/proc/self/fd/<fd>` re-opens, a macOS/`cap-std` fallback, or any per-platform split of this boundary. (An `O_PATH` *directory traversal* anchor for search-only intermediate dirs — `confine::open_dir_anchor`, Linux-only `EACCES` fallback — is explicitly allowed; it is a `dirfd` for `*at` calls only, never a metadata/leaf anchor.) `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.
40+
- 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` / `resolve_parent_beneath` (from `crate::plugins::host_dir`), then perform the actual syscall fd-relative against the returned `OwnedFd` (`fstat`/`fchmod`, fd `read`/`write`) or `*at` against the anchored parent dir + leaf name (`fstatat`/`fchownat`/`utimensat`). Metadata ops that do not need the file contents (`stat`/`exists`/`chown`/`utimes`) MUST use the parent-fd + `*at` form, never a leaf `O_RDONLY` open, so they do not require read permission the op does not need under a non-root sidecar (`chmod` is the exception — see the metadata-ops rule above). `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`.
3541
- 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: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ 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 the `confine` module of `src/plugins/host_dir.rs`. Plain
55+
# `openat(2)` is portable across Linux, macOS, and gVisor; `openat2` is
56+
# deliberately NOT used (see that module for why).
57+
rustix = { version = "1", features = ["fs"] }
5258
scrypt = "0.11"
5359
serde = { version = "1.0", features = ["derive"] }
5460
serde_json = "1.0"
@@ -62,11 +68,6 @@ ureq = { version = "2.10", features = ["json"] }
6268
url = "2"
6369
vfs = { workspace = true }
6470

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-
7071
[dev-dependencies]
7172
serde_bare = "0.5"
7273
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)