From b56f8308c5ae7cb2de7d67c7b7825f8ef41744d8 Mon Sep 17 00:00:00 2001 From: "John T. Myers" <9696606+johntmyers@users.noreply.github.com> Date: Thu, 2 Apr 2026 12:54:59 -0700 Subject: [PATCH 1/9] fix(security): update OSS dependencies to remediate 3 high-severity CVEs (#737) - tar 0.4.44 -> 0.4.45 (CVE-2026-33055: PAX size header skip) - aws-lc-rs 1.16.1 -> 1.16.2 / aws-lc-sys 0.38.0 -> 0.39.1 (BDSA-2026-5232: name constraints bypass in certificate validation) - Pygments 2.19.2 -> 2.20.0 (BDSA-2026-5113 / CVE-2026-4539: catastrophic regex backtracking) --- Cargo.lock | 26 +++++++++++++------------- uv.lock | 6 +++--- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8f827bc88..7d20c9bd8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -125,7 +125,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -136,7 +136,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -232,9 +232,9 @@ dependencies = [ [[package]] name = "aws-lc-rs" -version = "1.16.1" +version = "1.16.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "94bffc006df10ac2a68c83692d734a465f8ee6c5b384d8545a636f81d858f4bf" +checksum = "a054912289d18629dc78375ba2c3726a3afe3ff71b4edba9dedfca0e3446d1fc" dependencies = [ "aws-lc-sys", "untrusted 0.7.1", @@ -243,9 +243,9 @@ dependencies = [ [[package]] name = "aws-lc-sys" -version = "0.38.0" +version = "0.39.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4321e568ed89bb5a7d291a7f37997c2c0df89809d7b6d12062c81ddb54aa782e" +checksum = "83a25cf98105baa966497416dbd42565ce3a8cf8dbfd59803ec9ad46f3126399" dependencies = [ "cc", "cmake", @@ -1283,7 +1283,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -2691,7 +2691,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.59.0", ] [[package]] @@ -4044,7 +4044,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4519,7 +4519,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -4902,9 +4902,9 @@ dependencies = [ [[package]] name = "tar" -version = "0.4.44" +version = "0.4.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d863878d212c87a19c1a610eb53bb01fe12951c0501cf5a0d65f724914a667a" +checksum = "22692a6476a21fa75fdfc11d452fda482af402c008cdbaf3476414e122040973" dependencies = [ "filetime", "libc", @@ -4930,7 +4930,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] diff --git a/uv.lock b/uv.lock index 687a035ae..38a03ce29 100644 --- a/uv.lock +++ b/uv.lock @@ -637,11 +637,11 @@ wheels = [ [[package]] name = "pygments" -version = "2.19.2" +version = "2.20.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/b0/77/a5b8c569bf593b0140bde72ea885a803b82086995367bf2037de0159d924/pygments-2.19.2.tar.gz", hash = "sha256:636cb2477cec7f8952536970bc533bc43743542f70392ae026374600add5b887", size = 4968631, upload-time = "2025-06-21T13:39:12.283Z" } +sdist = { url = "https://files.pythonhosted.org/packages/c3/b2/bc9c9196916376152d655522fdcebac55e66de6603a76a02bca1b6414f6c/pygments-2.20.0.tar.gz", hash = "sha256:6757cd03768053ff99f3039c1a36d6c0aa0b263438fcab17520b30a303a82b5f", size = 4955991, upload-time = "2026-03-29T13:29:33.898Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/c7/21/705964c7812476f378728bdf590ca4b771ec72385c533964653c68e86bdc/pygments-2.19.2-py3-none-any.whl", hash = "sha256:86540386c03d588bb81d44bc3928634ff26449851e99741617ecb9037ee5ec0b", size = 1225217, upload-time = "2025-06-21T13:39:07.939Z" }, + { url = "https://files.pythonhosted.org/packages/f4/7e/a72dd26f3b0f4f2bf1dd8923c85f7ceb43172af56d63c7383eb62b332364/pygments-2.20.0-py3-none-any.whl", hash = "sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176", size = 1231151, upload-time = "2026-03-29T13:29:30.038Z" }, ] [[package]] From 8887d7c66ac6589617e70776e2c7e2104a9a81e6 Mon Sep 17 00:00:00 2001 From: "John T. Myers" <9696606+johntmyers@users.noreply.github.com> Date: Thu, 2 Apr 2026 14:30:51 -0700 Subject: [PATCH 2/9] fix(sandbox): harden seccomp filter to block dangerous syscalls (#740) --- architecture/sandbox.md | 46 ++++- architecture/security-policy.md | 29 ++- .../src/sandbox/linux/seccomp.rs | 182 ++++++++++++++++++ 3 files changed, 248 insertions(+), 9 deletions(-) diff --git a/architecture/sandbox.md b/architecture/sandbox.md index c870708dd..c5e212f85 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -24,7 +24,7 @@ All paths are relative to `crates/openshell-sandbox/src/`. | `sandbox/mod.rs` | Platform abstraction -- dispatches to Linux or no-op | | `sandbox/linux/mod.rs` | Linux composition: Landlock then seccomp | | `sandbox/linux/landlock.rs` | Filesystem isolation via Landlock LSM (ABI V1) | -| `sandbox/linux/seccomp.rs` | Syscall filtering via BPF on `SYS_socket` | +| `sandbox/linux/seccomp.rs` | Syscall filtering via BPF: socket domain blocks, dangerous syscall blocks, conditional flag blocks | | `bypass_monitor.rs` | Background `/dev/kmsg` reader for iptables bypass detection events | | `sandbox/linux/netns.rs` | Network namespace creation, veth pair setup, bypass detection iptables rules, cleanup on drop | | `l7/mod.rs` | L7 types (`L7Protocol`, `TlsMode`, `EnforcementMode`, `L7EndpointConfig`), config parsing, validation, access preset expansion, deprecated `tls` value handling | @@ -451,13 +451,7 @@ Kernel-level error behavior (e.g., Landlock ABI unavailable) depends on `Landloc **File:** `crates/openshell-sandbox/src/sandbox/linux/seccomp.rs` -Seccomp blocks socket creation for specific address families. The filter targets a single syscall (`SYS_socket`) and inspects argument 0 (the domain). - -**Always blocked** (regardless of network mode): -- `AF_NETLINK`, `AF_PACKET`, `AF_BLUETOOTH`, `AF_VSOCK` - -**Additionally blocked in `Block` mode** (no proxy): -- `AF_INET`, `AF_INET6` +Seccomp provides three layers of syscall restriction: socket domain blocks, unconditional syscall blocks, and conditional syscall blocks. The filter uses a default-allow policy (`SeccompAction::Allow`) with targeted rules that return `Errno(EPERM)`. **Skipped entirely** in `Allow` mode. @@ -465,8 +459,44 @@ Setup: 1. `prctl(PR_SET_NO_NEW_PRIVS, 1)` -- required before seccomp 2. `seccompiler::apply_filter()` with default action `Allow` and per-rule action `Errno(EPERM)` +#### Socket domain blocks + +| Domain | Always blocked | Additionally blocked in Block mode | +|--------|:-:|:-:| +| `AF_PACKET` | Yes | | +| `AF_BLUETOOTH` | Yes | | +| `AF_VSOCK` | Yes | | +| `AF_INET` | | Yes | +| `AF_INET6` | | Yes | +| `AF_NETLINK` | | Yes | + In `Proxy` mode, `AF_INET`/`AF_INET6` are allowed because the sandboxed process needs to connect to the proxy over the veth pair. The network namespace ensures it can only reach the proxy's IP (`10.200.0.1`). +#### Unconditional syscall blocks + +These syscalls are blocked entirely (EPERM for any invocation): + +| Syscall | Reason | +|---------|--------| +| `memfd_create` | Fileless binary execution bypasses Landlock filesystem restrictions | +| `ptrace` | Cross-process memory inspection and code injection | +| `bpf` | Kernel BPF program loading | +| `process_vm_readv` | Cross-process memory read | +| `io_uring_setup` | Async I/O subsystem with extensive CVE history | +| `mount` | Filesystem mount could subvert Landlock or overlay writable paths | + +#### Conditional syscall blocks + +These syscalls are only blocked when specific flag patterns are present: + +| Syscall | Condition | Reason | +|---------|-----------|--------| +| `execveat` | `AT_EMPTY_PATH` flag set (arg4) | Fileless execution from an anonymous fd | +| `unshare` | `CLONE_NEWUSER` flag set (arg0) | User namespace creation enables privilege escalation | +| `seccomp` | operation == `SECCOMP_SET_MODE_FILTER` (arg0) | Prevents sandboxed code from replacing the active filter | + +Conditional blocks use `MaskedEq` for flag checks (bit-test) and `Eq` for exact-value matches. This allows normal use of these syscalls while blocking the dangerous flag combinations. + ### Network namespace isolation **File:** `crates/openshell-sandbox/src/sandbox/linux/netns.rs` diff --git a/architecture/security-policy.md b/architecture/security-policy.md index 555ba67a5..01eb96f94 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -850,6 +850,10 @@ The response includes an `X-OpenShell-Policy` header and `Connection: close`. Se ## Seccomp Filter Details +The seccomp filter uses a default-allow policy (`SeccompAction::Allow`) with targeted rules that return `EPERM`. It provides three layers of protection: socket domain blocks, unconditional syscall blocks, and conditional syscall blocks. See `crates/openshell-sandbox/src/sandbox/linux/seccomp.rs`. + +### Blocked socket domains + Regardless of network mode, certain socket domains are always blocked: | Domain | Constant | Reason | @@ -861,7 +865,30 @@ Regardless of network mode, certain socket domains are always blocked: In proxy mode (which is always active), `AF_INET` (2) and `AF_INET6` (10) are allowed so the sandbox process can reach the proxy. -The seccomp filter uses a default-allow policy (`SeccompAction::Allow`) with specific `socket()` syscall rules that return `EPERM` when the first argument (domain) matches a blocked value. See `crates/openshell-sandbox/src/sandbox/linux/seccomp.rs`. +### Blocked syscalls + +These syscalls are blocked unconditionally (EPERM for any invocation): + +| Syscall | NR (x86-64) | Reason | +|---------|-------------|--------| +| `memfd_create` | 319 | Fileless binary execution bypasses Landlock filesystem restrictions | +| `ptrace` | 101 | Cross-process memory inspection and code injection | +| `bpf` | 321 | Kernel BPF program loading | +| `process_vm_readv` | 310 | Cross-process memory read | +| `io_uring_setup` | 425 | Async I/O subsystem with extensive CVE history | +| `mount` | 165 | Filesystem mount could subvert Landlock or overlay writable paths | + +### Conditionally blocked syscalls + +These syscalls are blocked only when specific flag patterns are present in their arguments: + +| Syscall | NR (x86-64) | Condition | Reason | +|---------|-------------|-----------|--------| +| `execveat` | 322 | `AT_EMPTY_PATH` (0x1000) set in flags (arg4) | Fileless execution from an anonymous fd | +| `unshare` | 272 | `CLONE_NEWUSER` (0x10000000) set in flags (arg0) | User namespace creation enables privilege escalation | +| `seccomp` | 317 | operation == `SECCOMP_SET_MODE_FILTER` (1) in arg0 | Prevents sandboxed code from replacing the active filter | + +Flag checks use `MaskedEq` (`(arg & mask) == mask`) to detect the flag bit regardless of other bits. The `seccomp` syscall check uses `Eq` for exact value comparison on the operation argument. --- diff --git a/crates/openshell-sandbox/src/sandbox/linux/seccomp.rs b/crates/openshell-sandbox/src/sandbox/linux/seccomp.rs index 6c9d8307b..e23447498 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/seccomp.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/seccomp.rs @@ -2,6 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 //! Seccomp syscall filtering. +//! +//! The filter uses a default-allow policy with targeted blocks: +//! +//! 1. **Socket domain blocks** -- prevent raw/kernel sockets that bypass the proxy +//! 2. **Unconditional syscall blocks** -- block syscalls that enable sandbox escape +//! (fileless exec, ptrace, BPF, cross-process memory access, io_uring, mount) +//! 3. **Conditional syscall blocks** -- block dangerous flag combinations on otherwise +//! needed syscalls (execveat+AT_EMPTY_PATH, unshare+CLONE_NEWUSER, +//! seccomp+SET_MODE_FILTER) use crate::policy::{NetworkMode, SandboxPolicy}; use miette::{IntoDiagnostic, Result}; @@ -13,6 +22,9 @@ use std::collections::BTreeMap; use std::convert::TryInto; use tracing::debug; +/// Value of `SECCOMP_SET_MODE_FILTER` (linux/seccomp.h). +const SECCOMP_SET_MODE_FILTER: u64 = 1; + pub fn apply(policy: &SandboxPolicy) -> Result<()> { if matches!(policy.network.mode, NetworkMode::Allow) { return Ok(()); @@ -37,6 +49,7 @@ pub fn apply(policy: &SandboxPolicy) -> Result<()> { fn build_filter(allow_inet: bool) -> Result { let mut rules: BTreeMap> = BTreeMap::new(); + // --- Socket domain blocks --- let mut blocked_domains = vec![libc::AF_PACKET, libc::AF_BLUETOOTH, libc::AF_VSOCK]; if !allow_inet { blocked_domains.push(libc::AF_INET); @@ -49,6 +62,51 @@ fn build_filter(allow_inet: bool) -> Result { add_socket_domain_rule(&mut rules, domain)?; } + // --- Unconditional syscall blocks --- + // These syscalls are blocked entirely (empty rule vec = unconditional EPERM). + + // Fileless binary execution via memfd bypasses Landlock filesystem restrictions. + rules.entry(libc::SYS_memfd_create).or_default(); + // Cross-process memory inspection and code injection. + rules.entry(libc::SYS_ptrace).or_default(); + // Kernel BPF program loading. + rules.entry(libc::SYS_bpf).or_default(); + // Cross-process memory read. + rules.entry(libc::SYS_process_vm_readv).or_default(); + // Async I/O subsystem with extensive CVE history. + rules.entry(libc::SYS_io_uring_setup).or_default(); + // Filesystem mount could subvert Landlock or overlay writable paths. + rules.entry(libc::SYS_mount).or_default(); + + // --- Conditional syscall blocks --- + + // execveat with AT_EMPTY_PATH enables fileless execution from an anonymous fd. + add_masked_arg_rule( + &mut rules, + libc::SYS_execveat, + 4, // flags argument + libc::AT_EMPTY_PATH as u64, + )?; + + // unshare with CLONE_NEWUSER allows creating user namespaces to escalate privileges. + add_masked_arg_rule( + &mut rules, + libc::SYS_unshare, + 0, // flags argument + libc::CLONE_NEWUSER as u64, + )?; + + // seccomp(SECCOMP_SET_MODE_FILTER) would let sandboxed code replace the active filter. + let condition = SeccompCondition::new( + 0, // operation argument + SeccompCmpArgLen::Dword, + SeccompCmpOp::Eq, + SECCOMP_SET_MODE_FILTER, + ) + .into_diagnostic()?; + let rule = SeccompRule::new(vec![condition]).into_diagnostic()?; + rules.entry(libc::SYS_seccomp).or_default().push(rule); + let arch = std::env::consts::ARCH .try_into() .map_err(|_| miette::miette!("Unsupported architecture for seccomp"))?; @@ -74,3 +132,127 @@ fn add_socket_domain_rule(rules: &mut BTreeMap>, domain: i rules.entry(libc::SYS_socket).or_default().push(rule); Ok(()) } + +/// Block a syscall when a specific bit pattern is set in an argument. +/// +/// Uses `MaskedEq` to check `(arg & flag_bit) == flag_bit`, which triggers +/// EPERM when the flag is present regardless of other bits in the argument. +fn add_masked_arg_rule( + rules: &mut BTreeMap>, + syscall: i64, + arg_index: u8, + flag_bit: u64, +) -> Result<()> { + let condition = SeccompCondition::new( + arg_index, + SeccompCmpArgLen::Dword, + SeccompCmpOp::MaskedEq(flag_bit), + flag_bit, + ) + .into_diagnostic()?; + let rule = SeccompRule::new(vec![condition]).into_diagnostic()?; + rules.entry(syscall).or_default().push(rule); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn build_filter_proxy_mode_compiles() { + let filter = build_filter(true); + assert!(filter.is_ok(), "build_filter(true) should succeed"); + } + + #[test] + fn build_filter_block_mode_compiles() { + let filter = build_filter(false); + assert!(filter.is_ok(), "build_filter(false) should succeed"); + } + + #[test] + fn add_masked_arg_rule_creates_entry() { + let mut rules: BTreeMap> = BTreeMap::new(); + let result = add_masked_arg_rule(&mut rules, libc::SYS_execveat, 4, 0x1000); + assert!(result.is_ok()); + assert!( + rules.contains_key(&libc::SYS_execveat), + "should have an entry for SYS_execveat" + ); + assert_eq!( + rules[&libc::SYS_execveat].len(), + 1, + "should have exactly one rule" + ); + } + + #[test] + fn unconditional_blocks_present_in_filter() { + let mut rules: BTreeMap> = BTreeMap::new(); + + // Simulate what build_filter does for unconditional blocks + rules.entry(libc::SYS_memfd_create).or_default(); + rules.entry(libc::SYS_ptrace).or_default(); + rules.entry(libc::SYS_bpf).or_default(); + rules.entry(libc::SYS_process_vm_readv).or_default(); + rules.entry(libc::SYS_io_uring_setup).or_default(); + rules.entry(libc::SYS_mount).or_default(); + + // Unconditional blocks have an empty Vec (no conditions = always match) + for syscall in [ + libc::SYS_memfd_create, + libc::SYS_ptrace, + libc::SYS_bpf, + libc::SYS_process_vm_readv, + libc::SYS_io_uring_setup, + libc::SYS_mount, + ] { + assert!( + rules.contains_key(&syscall), + "syscall {syscall} should be in the rules map" + ); + assert!( + rules[&syscall].is_empty(), + "syscall {syscall} should have empty rules (unconditional block)" + ); + } + } + + #[test] + fn conditional_blocks_have_rules() { + // Build a real filter and verify the conditional syscalls have rule entries + // (non-empty Vec means conditional match) + let mut rules: BTreeMap> = BTreeMap::new(); + + add_masked_arg_rule( + &mut rules, + libc::SYS_execveat, + 4, + libc::AT_EMPTY_PATH as u64, + ) + .unwrap(); + add_masked_arg_rule(&mut rules, libc::SYS_unshare, 0, libc::CLONE_NEWUSER as u64).unwrap(); + + let condition = SeccompCondition::new( + 0, + SeccompCmpArgLen::Dword, + SeccompCmpOp::Eq, + SECCOMP_SET_MODE_FILTER, + ) + .unwrap(); + let rule = SeccompRule::new(vec![condition]).unwrap(); + rules.entry(libc::SYS_seccomp).or_default().push(rule); + + for syscall in [libc::SYS_execveat, libc::SYS_unshare, libc::SYS_seccomp] { + assert!( + rules.contains_key(&syscall), + "syscall {syscall} should be in the rules map" + ); + assert!( + !rules[&syscall].is_empty(), + "syscall {syscall} should have conditional rules" + ); + } + } +} From 77e55ea989d144b8761875a6c566d9289dac460b Mon Sep 17 00:00:00 2001 From: "John T. Myers" <9696606+johntmyers@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:06:32 -0700 Subject: [PATCH 3/9] test(e2e): replace flaky Python live policy update tests with Rust (#742) Remove test_live_policy_update_and_logs and test_live_policy_update_from_empty_network_policies from the Python e2e suite. Both used a manual 90s poll loop against GetSandboxPolicyStatus that flaked in CI with 'Policy v2 was not loaded within 90s'. Add e2e/rust/tests/live_policy_update.rs with two replacement tests that exercise the same policy lifecycle (version bumping, hash idempotency, policy list history) through the CLI using the built-in --wait flag for reliable synchronization. --- e2e/python/test_sandbox_policy.py | 260 +--------------- e2e/rust/tests/live_policy_update.rs | 423 +++++++++++++++++++++++++++ 2 files changed, 424 insertions(+), 259 deletions(-) create mode 100644 e2e/rust/tests/live_policy_update.rs diff --git a/e2e/python/test_sandbox_policy.py b/e2e/python/test_sandbox_policy.py index 625fe8da0..092f99784 100644 --- a/e2e/python/test_sandbox_policy.py +++ b/e2e/python/test_sandbox_policy.py @@ -314,9 +314,7 @@ def log_message(self, *args): {"connect_status": connect_resp.strip(), "http_status": 0} ) - request = ( - f"{method} {path} HTTP/1.1\r\nHost: {target_host}\r\nConnection: close\r\n\r\n" - ) + request = f"{method} {path} HTTP/1.1\r\nHost: {target_host}\r\nConnection: close\r\n\r\n" conn.sendall(request.encode()) data = b"" @@ -1348,262 +1346,6 @@ def test_l7_rule_without_query_matcher_allows_any_query_params( assert "connect-server-ok" in resp["body"] -# ============================================================================= -# Live policy update + log streaming tests -# -# LPU-1: Create sandbox, verify initial policy is v1 -# LPU-2: Set the same policy again -> unchanged (no new version) -# LPU-3: Push a different policy -> new version loaded, verify connectivity -# LPU-4: Push v2 again -> unchanged -# LPU-5: Fetch logs (one-shot + streaming) and verify both sources appear -# ============================================================================= - - -def test_live_policy_update_and_logs( - sandbox: Callable[..., Sandbox], - sandbox_client: SandboxClient, -) -> None: - """End-to-end: live policy update lifecycle with log verification.""" - from openshell._proto import openshell_pb2, sandbox_pb2 - - # --- Setup: two distinct policies --- - # Policy A: python can reach api.anthropic.com - policy_a = _base_policy( - network_policies={ - "anthropic": sandbox_pb2.NetworkPolicyRule( - name="anthropic", - endpoints=[ - sandbox_pb2.NetworkEndpoint(host="api.anthropic.com", port=443), - ], - binaries=[sandbox_pb2.NetworkBinary(path="/**")], - ), - }, - ) - # Policy B: python can reach api.anthropic.com AND example.com - policy_b = _base_policy( - network_policies={ - "anthropic": sandbox_pb2.NetworkPolicyRule( - name="anthropic", - endpoints=[ - sandbox_pb2.NetworkEndpoint(host="api.anthropic.com", port=443), - ], - binaries=[sandbox_pb2.NetworkBinary(path="/**")], - ), - "example": sandbox_pb2.NetworkPolicyRule( - name="example", - endpoints=[ - sandbox_pb2.NetworkEndpoint(host="example.com", port=443), - ], - binaries=[sandbox_pb2.NetworkBinary(path="/**")], - ), - }, - ) - - spec = datamodel_pb2.SandboxSpec(policy=policy_a) - stub = sandbox_client._stub - - with sandbox(spec=spec, delete_on_exit=True) as sb: - sandbox_name = sb.sandbox.name - - # --- LPU-1: Initial policy should be version 1 --- - status_resp = stub.GetSandboxPolicyStatus( - openshell_pb2.GetSandboxPolicyStatusRequest(name=sandbox_name, version=0) - ) - assert status_resp.revision.version >= 1, "Initial policy should be at least v1" - initial_version = status_resp.revision.version - initial_hash = status_resp.revision.policy_hash - - # --- LPU-2: Set the same policy -> no new version --- - update_resp = stub.UpdateConfig( - openshell_pb2.UpdateConfigRequest( - name=sandbox_name, - policy=policy_a, - ) - ) - assert update_resp.version == initial_version, ( - f"Same policy should return existing version {initial_version}, " - f"got {update_resp.version}" - ) - assert update_resp.policy_hash == initial_hash - - # --- LPU-3: Push policy B -> new version --- - update_resp = stub.UpdateConfig( - openshell_pb2.UpdateConfigRequest( - name=sandbox_name, - policy=policy_b, - ) - ) - new_version = update_resp.version - assert new_version > initial_version, ( - f"Different policy should create new version > {initial_version}, " - f"got {new_version}" - ) - assert update_resp.policy_hash != initial_hash - - # Wait for the sandbox to load the new policy (poll loop is 30s default). - import time - - deadline = time.time() + 90 - loaded = False - while time.time() < deadline: - status_resp = stub.GetSandboxPolicyStatus( - openshell_pb2.GetSandboxPolicyStatusRequest( - name=sandbox_name, version=new_version - ) - ) - status = status_resp.revision.status - if status == openshell_pb2.POLICY_STATUS_LOADED: - loaded = True - break - if status == openshell_pb2.POLICY_STATUS_FAILED: - pytest.fail( - f"Policy v{new_version} failed to load: " - f"{status_resp.revision.load_error}" - ) - time.sleep(2) - assert loaded, f"Policy v{new_version} was not loaded within 90s" - - # Verify the new policy works: example.com should now be allowed - result = sb.exec_python(_proxy_connect(), args=("example.com", 443)) - assert result.exit_code == 0, result.stderr - assert "200" in result.stdout, ( - f"example.com should be allowed after policy update, got: {result.stdout}" - ) - - # --- LPU-4: Push policy B again -> unchanged --- - update_resp = stub.UpdateConfig( - openshell_pb2.UpdateConfigRequest( - name=sandbox_name, - policy=policy_b, - ) - ) - assert update_resp.version == new_version, ( - f"Same policy B should return existing version {new_version}, " - f"got {update_resp.version}" - ) - - # --- LPU-5: Verify policy history --- - list_resp = stub.ListSandboxPolicies( - openshell_pb2.ListSandboxPoliciesRequest(name=sandbox_name, limit=10) - ) - versions = [r.version for r in list_resp.revisions] - assert new_version in versions - assert initial_version in versions - - # Only one version should be Loaded - loaded_count = sum( - 1 - for r in list_resp.revisions - if r.status == openshell_pb2.POLICY_STATUS_LOADED - ) - assert loaded_count == 1, ( - f"Expected exactly 1 loaded version, got {loaded_count}: " - f"{[(r.version, r.status) for r in list_resp.revisions]}" - ) - - # --- LPU-6: Fetch logs (one-shot) and verify both sources --- - # Resolve sandbox ID for log RPCs - get_resp = stub.GetSandbox(openshell_pb2.GetSandboxRequest(name=sandbox_name)) - sandbox_id = get_resp.sandbox.id - - logs_resp = stub.GetSandboxLogs( - openshell_pb2.GetSandboxLogsRequest(sandbox_id=sandbox_id, lines=500) - ) - assert logs_resp.buffer_total > 0, "Expected some logs in the buffer" - - sources = {log.source or "gateway" for log in logs_resp.logs} - assert "gateway" in sources, ( - f"Expected gateway logs in response, got sources: {sources}" - ) - # Sandbox logs may take a moment to arrive via the push stream. - # If they're present, verify the source tag. - if "sandbox" in sources: - sandbox_logs = [l for l in logs_resp.logs if l.source == "sandbox"] - assert len(sandbox_logs) > 0 - # Verify structured fields are present on at least one sandbox log - has_fields = any(len(l.fields) > 0 for l in sandbox_logs) - # Not all sandbox logs have fields (e.g., "Starting sandbox" doesn't), - # so we just check at least one does if there are CONNECT logs - connect_logs = [l for l in sandbox_logs if "CONNECT" in l.message] - if connect_logs: - assert has_fields, "CONNECT logs should have structured fields" - - -def test_live_policy_update_from_empty_network_policies( - sandbox: Callable[..., Sandbox], - sandbox_client: SandboxClient, -) -> None: - """End-to-end: add the first network rule to a running sandbox.""" - from openshell._proto import openshell_pb2, sandbox_pb2 - - initial_policy = _base_policy() - updated_policy = _base_policy( - network_policies={ - "example": sandbox_pb2.NetworkPolicyRule( - name="example", - endpoints=[ - sandbox_pb2.NetworkEndpoint(host="example.com", port=443), - ], - binaries=[sandbox_pb2.NetworkBinary(path="/**")], - ), - }, - ) - - spec = datamodel_pb2.SandboxSpec(policy=initial_policy) - stub = sandbox_client._stub - - with sandbox(spec=spec, delete_on_exit=True) as sb: - sandbox_name = sb.sandbox.name - - denied = sb.exec_python(_proxy_connect(), args=("example.com", 443)) - assert denied.exit_code == 0, denied.stderr - assert "403" in denied.stdout, denied.stdout - - initial_status = stub.GetSandboxPolicyStatus( - openshell_pb2.GetSandboxPolicyStatusRequest(name=sandbox_name, version=0) - ) - initial_version = initial_status.revision.version - - update_resp = stub.UpdateConfig( - openshell_pb2.UpdateConfigRequest( - name=sandbox_name, - policy=updated_policy, - ) - ) - new_version = update_resp.version - assert new_version > initial_version, ( - f"Adding the first network rule should create a new version > {initial_version}, " - f"got {new_version}" - ) - - import time - - deadline = time.time() + 90 - loaded = False - while time.time() < deadline: - status_resp = stub.GetSandboxPolicyStatus( - openshell_pb2.GetSandboxPolicyStatusRequest( - name=sandbox_name, version=new_version - ) - ) - status = status_resp.revision.status - if status == openshell_pb2.POLICY_STATUS_LOADED: - loaded = True - break - if status == openshell_pb2.POLICY_STATUS_FAILED: - pytest.fail( - f"Policy v{new_version} failed to load: " - f"{status_resp.revision.load_error}" - ) - time.sleep(2) - - assert loaded, f"Policy v{new_version} was not loaded within 90s" - - allowed = sb.exec_python(_proxy_connect(), args=("example.com", 443)) - assert allowed.exit_code == 0, allowed.stderr - assert "200" in allowed.stdout, allowed.stdout - - # ============================================================================= # Forward proxy tests (plain HTTP, non-CONNECT) # ============================================================================= diff --git a/e2e/rust/tests/live_policy_update.rs b/e2e/rust/tests/live_policy_update.rs new file mode 100644 index 000000000..c60b29548 --- /dev/null +++ b/e2e/rust/tests/live_policy_update.rs @@ -0,0 +1,423 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! E2E tests for live policy updates on a running sandbox. +//! +//! Covers the full round-trip: +//! - Create sandbox with policy A +//! - Verify initial policy version via `policy get` +//! - Push same policy A again -> no version bump (idempotent) +//! - Push different policy B -> new version, `--wait` for sandbox to load it +//! - Verify policy history via `policy list` +//! +//! These tests replace the Python e2e tests `test_live_policy_update_and_logs` +//! and `test_live_policy_update_from_empty_network_policies`, which were flaky +//! due to hard-coded 90s poll timeouts. The Rust tests use the CLI's built-in +//! `--wait` flag for reliable synchronization. +//! +//! Note: the removed Python tests also covered `GetSandboxLogs` RPC and +//! verified actual proxy connectivity after policy update. Those are tracked +//! as follow-up coverage gaps -- the proxy enforcement path is covered by the +//! existing L4/L7/SSRF Python e2e tests, and log fetching needs a dedicated +//! test. + +#![cfg(feature = "e2e")] + +use std::fmt::Write as _; +use std::io::Write; +use std::process::Stdio; + +use openshell_e2e::harness::binary::openshell_cmd; +use openshell_e2e::harness::output::{extract_field, strip_ansi}; +use openshell_e2e::harness::sandbox::SandboxGuard; +use tempfile::NamedTempFile; + +// --------------------------------------------------------------------------- +// Policy YAML builders +// --------------------------------------------------------------------------- + +/// Build a policy YAML that allows any binary to reach the given hosts on +/// port 443. +/// +/// NOTE: The indentation in the format string is load-bearing YAML structure. +fn write_policy(hosts: &[&str]) -> Result { + let mut file = NamedTempFile::new().map_err(|e| format!("create temp policy file: {e}"))?; + + let mut network_rules = String::new(); + for (i, host) in hosts.iter().enumerate() { + let _ = write!( + network_rules, + r#" rule_{i}: + name: rule_{i} + endpoints: + - host: {host} + port: 443 + binaries: + - path: "/**" +"# + ); + } + + let policy = format!( + r"version: 1 + +filesystem_policy: + include_workdir: true + read_only: + - /usr + - /lib + - /proc + - /dev/urandom + - /app + - /etc + - /var/log + read_write: + - /sandbox + - /tmp + - /dev/null + +landlock: + compatibility: best_effort + +process: + run_as_user: sandbox + run_as_group: sandbox + +network_policies: +{network_rules}" + ); + + file.write_all(policy.as_bytes()) + .map_err(|e| format!("write temp policy file: {e}"))?; + file.flush() + .map_err(|e| format!("flush temp policy file: {e}"))?; + Ok(file) +} + +/// Build a minimal policy YAML with no network rules. +fn write_empty_network_policy() -> Result { + let mut file = NamedTempFile::new().map_err(|e| format!("create temp policy file: {e}"))?; + + let policy = r"version: 1 + +filesystem_policy: + include_workdir: true + read_only: + - /usr + - /lib + - /proc + - /dev/urandom + - /app + - /etc + - /var/log + read_write: + - /sandbox + - /tmp + - /dev/null + +landlock: + compatibility: best_effort + +process: + run_as_user: sandbox + run_as_group: sandbox +"; + + file.write_all(policy.as_bytes()) + .map_err(|e| format!("write temp policy file: {e}"))?; + file.flush() + .map_err(|e| format!("flush temp policy file: {e}"))?; + Ok(file) +} + +// --------------------------------------------------------------------------- +// CLI helpers +// --------------------------------------------------------------------------- + +struct CliResult { + success: bool, + output: String, + exit_code: Option, +} + +/// Run an `openshell` CLI command and return the result. +async fn run_cli(args: &[&str]) -> CliResult { + let mut cmd = openshell_cmd(); + cmd.args(args).stdout(Stdio::piped()).stderr(Stdio::piped()); + + let output = cmd.output().await.expect("spawn openshell command"); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let combined = strip_ansi(&format!("{stdout}{stderr}")); + + CliResult { + success: output.status.success(), + output: combined, + exit_code: output.status.code(), + } +} + +/// Extract the policy version number from `policy get` output. +/// +/// Uses the shared `extract_field` helper to find `Version: ` or +/// `Revision: ` in CLI tabular output. +fn extract_version(output: &str) -> Option { + extract_field(output, "Version") + .or_else(|| extract_field(output, "Revision")) + .and_then(|v| v.parse::().ok()) +} + +/// Extract the policy hash from `policy get` output. +fn extract_hash(output: &str) -> Option { + extract_field(output, "Hash") + .or_else(|| extract_field(output, "Policy hash")) +} + +/// Check that a version number appears in `policy list` output as a +/// distinct field value (not just a substring of some other number). +/// +/// Looks for the version number preceded by whitespace or at the start +/// of a line, to avoid matching "2" inside "12" or timestamps. +fn list_output_contains_version(output: &str, version: u32) -> bool { + let v = version.to_string(); + output.lines().any(|line| { + line.split_whitespace() + .any(|word| word == v || word.starts_with(&format!("{v} "))) + }) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +/// Test the full live policy update lifecycle: +/// +/// 1. Create sandbox with `--keep` +/// 2. Set policy A, verify initial version >= 1 +/// 3. Push same policy A -> version unchanged (idempotent) +/// 4. Push policy B (adds example.com) with `--wait` -> new version +/// 5. Push policy B again -> idempotent +/// 6. Verify policy list shows both versions +#[tokio::test] +#[allow(clippy::too_many_lines)] +async fn live_policy_update_round_trip() { + // --- Write two distinct policy files --- + let policy_a = write_policy(&["api.anthropic.com"]).expect("write policy A"); + let policy_b = + write_policy(&["api.anthropic.com", "example.com"]).expect("write policy B"); + + let policy_a_path = policy_a + .path() + .to_str() + .expect("policy A path should be utf-8") + .to_string(); + let policy_b_path = policy_b + .path() + .to_str() + .expect("policy B path should be utf-8") + .to_string(); + + // --- Create a long-running sandbox --- + let mut guard = SandboxGuard::create_keep( + &["sh", "-c", "echo Ready && sleep infinity"], + "Ready", + ) + .await + .expect("create keep sandbox"); + + // --- Set initial policy A --- + let r = run_cli(&[ + "policy", "set", &guard.name, "--policy", &policy_a_path, "--wait", "--timeout", "120", + ]) + .await; + assert!( + r.success, + "policy set A should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + // --- Verify initial policy version --- + let r = run_cli(&["policy", "get", &guard.name]).await; + assert!( + r.success, + "policy get should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + let initial_version = extract_version(&r.output) + .unwrap_or_else(|| panic!("could not parse version from policy get output:\n{}", r.output)); + assert!( + initial_version >= 1, + "initial policy version should be >= 1, got {initial_version}" + ); + + let initial_hash = extract_hash(&r.output); + + // --- Push same policy A again -> should be idempotent --- + let r = run_cli(&[ + "policy", "set", &guard.name, "--policy", &policy_a_path, "--wait", "--timeout", "120", + ]) + .await; + assert!( + r.success, + "policy set A (repeat) should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + let r = run_cli(&["policy", "get", &guard.name]).await; + assert!(r.success, "policy get after repeat should succeed:\n{}", r.output); + + let repeat_version = extract_version(&r.output) + .unwrap_or_else(|| panic!("could not parse version after repeat:\n{}", r.output)); + assert_eq!( + repeat_version, initial_version, + "same policy should not bump version: expected {initial_version}, got {repeat_version}" + ); + + if let (Some(ih), Some(rh)) = (&initial_hash, &extract_hash(&r.output)) { + assert_eq!(ih, rh, "same policy should produce same hash"); + } + + // --- Push policy B -> should create new version --- + let r = run_cli(&[ + "policy", "set", &guard.name, "--policy", &policy_b_path, "--wait", "--timeout", "120", + ]) + .await; + assert!( + r.success, + "policy set B should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + let r = run_cli(&["policy", "get", &guard.name]).await; + assert!(r.success, "policy get after B should succeed:\n{}", r.output); + + let new_version = extract_version(&r.output) + .unwrap_or_else(|| panic!("could not parse version after B:\n{}", r.output)); + assert!( + new_version > initial_version, + "different policy should bump version: expected > {initial_version}, got {new_version}" + ); + + if let (Some(ih), Some(nh)) = (&initial_hash, &extract_hash(&r.output)) { + assert_ne!(ih, nh, "different policy should produce different hash"); + } + + // --- Push policy B again -> idempotent --- + let r = run_cli(&[ + "policy", "set", &guard.name, "--policy", &policy_b_path, "--wait", "--timeout", "120", + ]) + .await; + assert!( + r.success, + "policy set B (repeat) should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + let r = run_cli(&["policy", "get", &guard.name]).await; + assert!(r.success, "policy get after B repeat should succeed:\n{}", r.output); + + let repeat_b_version = extract_version(&r.output) + .unwrap_or_else(|| panic!("could not parse version after B repeat:\n{}", r.output)); + assert_eq!( + repeat_b_version, new_version, + "same policy B should not bump version: expected {new_version}, got {repeat_b_version}" + ); + + // --- Verify policy list shows revision history --- + let r = run_cli(&["policy", "list", &guard.name]).await; + assert!( + r.success, + "policy list should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + // Both versions should appear in the list output. + assert!( + list_output_contains_version(&r.output, new_version), + "policy list should contain version {new_version}:\n{}", + r.output + ); + assert!( + list_output_contains_version(&r.output, initial_version), + "policy list should contain initial version {initial_version}:\n{}", + r.output + ); + + guard.cleanup().await; +} + +/// Test live policy update from an initially empty network policy: +/// +/// 1. Create sandbox with `--keep` +/// 2. Set policy with no network rules +/// 3. Push policy with a network rule using `--wait` +/// 4. Verify the version bumped +#[tokio::test] +async fn live_policy_update_from_empty_network_policies() { + let empty_policy = write_empty_network_policy().expect("write empty network policy"); + let full_policy = write_policy(&["example.com"]).expect("write full policy"); + + let empty_path = empty_policy + .path() + .to_str() + .expect("empty policy path should be utf-8") + .to_string(); + let full_path = full_policy + .path() + .to_str() + .expect("full policy path should be utf-8") + .to_string(); + + // Create sandbox with empty network policy. + let mut guard = SandboxGuard::create_keep( + &["sh", "-c", "echo Ready && sleep infinity"], + "Ready", + ) + .await + .expect("create keep sandbox"); + + // Set initial empty policy. + let r = run_cli(&[ + "policy", "set", &guard.name, "--policy", &empty_path, "--wait", "--timeout", "120", + ]) + .await; + assert!( + r.success, + "policy set (empty) should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + let r = run_cli(&["policy", "get", &guard.name]).await; + assert!(r.success, "policy get (empty) should succeed:\n{}", r.output); + + let initial_version = extract_version(&r.output) + .unwrap_or_else(|| panic!("could not parse version from empty policy:\n{}", r.output)); + + // Push policy with network rules. + let r = run_cli(&[ + "policy", "set", &guard.name, "--policy", &full_path, "--wait", "--timeout", "120", + ]) + .await; + assert!( + r.success, + "policy set (full) should succeed (exit {:?}):\n{}", + r.exit_code, r.output + ); + + let r = run_cli(&["policy", "get", &guard.name]).await; + assert!(r.success, "policy get (full) should succeed:\n{}", r.output); + + let new_version = extract_version(&r.output).unwrap_or_else(|| { + panic!( + "could not parse version after adding network rules:\n{}", + r.output + ) + }); + assert!( + new_version > initial_version, + "adding network rules should create new version > {initial_version}, got {new_version}" + ); + + guard.cleanup().await; +} From eea495e6b9002dc611cf73daa893fb13a1a24dce Mon Sep 17 00:00:00 2001 From: "John T. Myers" <9696606+johntmyers@users.noreply.github.com> Date: Thu, 2 Apr 2026 20:32:59 -0700 Subject: [PATCH 4/9] fix: remediate 9 security findings from external audit (OS-15 through OS-23) (#744) * fix(install): restrict tar extraction to expected binary member Prevents CWE-22 path traversal by extracting only the expected APP_NAME member instead of the full archive contents. Adds --no-same-owner and --no-same-permissions for defense-in-depth. OS-20 * fix(deploy): quote registry credentials in YAML heredocs Wraps username/password values with a yaml_quote helper to prevent YAML injection from special characters in registry credentials (CWE-94). Applied to all three heredoc blocks that emit registries.yaml auth. OS-23 * fix(server): redact session token in SSH tunnel rate-limit log Logs only the last 4 characters of bearer tokens to prevent credential exposure in log aggregation systems (CWE-532). OS-18 * fix(server): escape gateway_display in auth connect page Applies html_escape() to the Host/X-Forwarded-Host header value before rendering it into the HTML template, preventing HTML injection (CWE-79). OS-17 * fix(server): prevent XSS via code param with validation and proper JS escaping Adds server-side validation rejecting confirmation codes that do not match the CLI-generated format, replaces manual JS string escaping with serde_json serialization (handling U+2028/U+2029 line terminators), and adds a Content-Security-Policy header with nonce-based script-src. OS-16 * fix(sandbox): add byte cap and idle timeout to streaming inference relay Prevents resource exhaustion from upstream inference endpoints that stream indefinitely or hold connections open. Adds a 32 MiB total body limit and 30-second per-chunk idle timeout (CWE-400). OS-21 * fix(policy): narrow port field from u32 to u16 to reject invalid values Prevents meaningless port values >65535 from being accepted in policy YAML definitions. The proto field remains uint32 (protobuf has no u16) with validation at the conversion boundary. OS-22 * fix(deps): migrate from archived serde_yaml to serde_yml Replaces serde_yaml 0.9 (archived, RUSTSEC-2024-0320) with serde_yml 0.0.12, a maintained API-compatible fork. All import sites updated across openshell-policy, openshell-sandbox, and openshell-router. OS-19 * fix(server): re-validate sandbox-submitted security_notes and cap hit_count The gateway now re-runs security heuristics on proposed policy chunks instead of trusting sandbox-provided security_notes, validates host wildcards, caps hit_count at 100, and clamps confidence to [0,1]. The TUI approve-all path is updated to use ApproveAllDraftChunks RPC which respects the security_notes filtering gate (CWE-284, confused deputy). OS-15 * chore: apply cargo fmt and update Cargo.lock for serde_yml --------- Co-authored-by: John Myers --- Cargo.lock | 45 +++++-- Cargo.toml | 2 +- crates/openshell-policy/Cargo.toml | 2 +- crates/openshell-policy/src/lib.rs | 39 ++++-- crates/openshell-router/Cargo.toml | 2 +- crates/openshell-router/src/config.rs | 2 +- crates/openshell-sandbox/Cargo.toml | 2 +- crates/openshell-sandbox/src/opa.rs | 2 +- crates/openshell-sandbox/src/proxy.rs | 33 ++++- crates/openshell-server/src/auth.rs | 139 +++++++++++++++++----- crates/openshell-server/src/grpc.rs | 64 ++++++++-- crates/openshell-server/src/ssh_tunnel.rs | 11 +- crates/openshell-tui/src/lib.rs | 75 ++++++------ deploy/docker/cluster-entrypoint.sh | 21 +++- install.sh | 2 +- 15 files changed, 326 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d20c9bd8..852d97a0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -125,7 +125,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -136,7 +136,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -1283,7 +1283,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -2497,6 +2497,16 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "libyml" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3302702afa434ffa30847a83305f0a69d6abd74293b6554c18ec85c7ef30c980" +dependencies = [ + "anyhow", + "version_check", +] + [[package]] name = "linux-raw-sys" version = "0.4.15" @@ -2691,7 +2701,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2902,7 +2912,7 @@ dependencies = [ "miette", "openshell-core", "serde", - "serde_yaml", + "serde_yml", ] [[package]] @@ -2922,7 +2932,7 @@ dependencies = [ "reqwest", "serde", "serde_json", - "serde_yaml", + "serde_yml", "tempfile", "thiserror 2.0.18", "tokio", @@ -2958,7 +2968,7 @@ dependencies = [ "rustls-pemfile", "seccompiler", "serde_json", - "serde_yaml", + "serde_yml", "sha2 0.10.9", "temp-env", "tempfile", @@ -4044,7 +4054,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4348,6 +4358,21 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serde_yml" +version = "0.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59e2dd588bf1597a252c3b920e0143eb99b0f76e4e082f4c92ce34fbc9e71ddd" +dependencies = [ + "indexmap 2.13.0", + "itoa", + "libyml", + "memchr", + "ryu", + "serde", + "version_check", +] + [[package]] name = "serdect" version = "0.4.2" @@ -4519,7 +4544,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -4930,7 +4955,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4fecf1940..08b699d47 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,7 +64,7 @@ nix = { version = "0.29", features = ["signal", "process", "user", "fs", "term"] # Serialization serde = { version = "1", features = ["derive"] } serde_json = "1" -serde_yaml = "0.9" +serde_yml = "0.0.12" # HTTP client reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } diff --git a/crates/openshell-policy/Cargo.toml b/crates/openshell-policy/Cargo.toml index 311bb4e86..f26136c6b 100644 --- a/crates/openshell-policy/Cargo.toml +++ b/crates/openshell-policy/Cargo.toml @@ -13,7 +13,7 @@ repository.workspace = true [dependencies] openshell-core = { path = "../openshell-core" } serde = { workspace = true } -serde_yaml = { workspace = true } +serde_yml = { workspace = true } miette = { workspace = true } [lints] diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index 7adb4dfda..9cf543bdf 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -82,11 +82,12 @@ struct NetworkEndpointDef { #[serde(default, skip_serializing_if = "String::is_empty")] host: String, /// Single port (backwards compat). Mutually exclusive with `ports`. + /// Uses `u16` to reject invalid values >65535 at parse time. #[serde(default, skip_serializing_if = "is_zero")] - port: u32, + port: u16, /// Multiple ports. When non-empty, this endpoint covers all listed ports. #[serde(default, skip_serializing_if = "Vec::is_empty")] - ports: Vec, + ports: Vec, #[serde(default, skip_serializing_if = "String::is_empty")] protocol: String, #[serde(default, skip_serializing_if = "String::is_empty")] @@ -101,7 +102,7 @@ struct NetworkEndpointDef { allowed_ips: Vec, } -fn is_zero(v: &u32) -> bool { +fn is_zero(v: &u16) -> bool { *v == 0 } @@ -169,10 +170,10 @@ fn to_proto(raw: PolicyFile) -> SandboxPolicy { .map(|e| { // Normalize port/ports: ports takes precedence, else // single port is promoted to ports array. - let normalized_ports = if !e.ports.is_empty() { - e.ports + let normalized_ports: Vec = if !e.ports.is_empty() { + e.ports.into_iter().map(u32::from).collect() } else if e.port > 0 { - vec![e.port] + vec![u32::from(e.port)] } else { vec![] }; @@ -285,10 +286,12 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { .map(|e| { // Use compact form: if ports has exactly 1 element, // emit port (scalar). If >1, emit ports (array). + // Proto uses u32; YAML uses u16. Clamp at boundary. + let clamp = |v: u32| -> u16 { v.min(65535) as u16 }; let (port, ports) = if e.ports.len() > 1 { - (0, e.ports.clone()) + (0, e.ports.iter().map(|&p| clamp(p)).collect()) } else { - (e.ports.first().copied().unwrap_or(e.port), vec![]) + (clamp(e.ports.first().copied().unwrap_or(e.port)), vec![]) }; NetworkEndpointDef { host: e.host.clone(), @@ -358,7 +361,7 @@ fn from_proto(policy: &SandboxPolicy) -> PolicyFile { /// Parse a sandbox policy from a YAML string. pub fn parse_sandbox_policy(yaml: &str) -> Result { - let raw: PolicyFile = serde_yaml::from_str(yaml) + let raw: PolicyFile = serde_yml::from_str(yaml) .into_diagnostic() .wrap_err("failed to parse sandbox policy YAML")?; Ok(to_proto(raw)) @@ -371,7 +374,7 @@ pub fn parse_sandbox_policy(yaml: &str) -> Result { /// and is round-trippable through `parse_sandbox_policy`. pub fn serialize_sandbox_policy(policy: &SandboxPolicy) -> Result { let yaml_repr = from_proto(policy); - serde_yaml::to_string(&yaml_repr) + serde_yml::to_string(&yaml_repr) .into_diagnostic() .wrap_err("failed to serialize policy to YAML") } @@ -1207,4 +1210,20 @@ network_policies: proto2.network_policies["test"].endpoints[0].host ); } + + #[test] + fn rejects_port_above_65535() { + let yaml = r#" +version: 1 +network_policies: + test: + endpoints: + - host: example.com + port: 70000 +"#; + assert!( + parse_sandbox_policy(yaml).is_err(), + "port >65535 should fail to parse" + ); + } } diff --git a/crates/openshell-router/Cargo.toml b/crates/openshell-router/Cargo.toml index dc8e9c924..e4c3d5ea7 100644 --- a/crates/openshell-router/Cargo.toml +++ b/crates/openshell-router/Cargo.toml @@ -19,7 +19,7 @@ serde_json = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tokio = { workspace = true } -serde_yaml = { workspace = true } +serde_yml = { workspace = true } uuid = { workspace = true } [dev-dependencies] diff --git a/crates/openshell-router/src/config.rs b/crates/openshell-router/src/config.rs index 52c22da9f..b531e091d 100644 --- a/crates/openshell-router/src/config.rs +++ b/crates/openshell-router/src/config.rs @@ -75,7 +75,7 @@ impl RouterConfig { path.display() )) })?; - let config: Self = serde_yaml::from_str(&content).map_err(|e| { + let config: Self = serde_yml::from_str(&content).map_err(|e| { RouterError::Internal(format!( "failed to parse router config {}: {e}", path.display() diff --git a/crates/openshell-sandbox/Cargo.toml b/crates/openshell-sandbox/Cargo.toml index 68e696e95..e8e7e2c97 100644 --- a/crates/openshell-sandbox/Cargo.toml +++ b/crates/openshell-sandbox/Cargo.toml @@ -60,7 +60,7 @@ ipnet = "2" # Serialization serde_json = { workspace = true } -serde_yaml = { workspace = true } +serde_yml = { workspace = true } # Logging tracing = { workspace = true } diff --git a/crates/openshell-sandbox/src/opa.rs b/crates/openshell-sandbox/src/opa.rs index f1df12ff4..f1c0ad293 100644 --- a/crates/openshell-sandbox/src/opa.rs +++ b/crates/openshell-sandbox/src/opa.rs @@ -511,7 +511,7 @@ fn parse_process_policy(val: ®orus::Value) -> ProcessPolicy { /// Preprocess YAML policy data: parse, normalize, validate, expand access presets, return JSON. fn preprocess_yaml_data(yaml_str: &str) -> Result { - let mut data: serde_json::Value = serde_yaml::from_str(yaml_str) + let mut data: serde_json::Value = serde_yml::from_str(yaml_str) .map_err(|e| miette::miette!("failed to parse YAML data: {e}"))?; // Normalize port → ports for all endpoints so Rego always sees "ports" array. diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index a7df76e2f..9e87450d4 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -23,6 +23,12 @@ use tracing::{debug, info, warn}; const MAX_HEADER_BYTES: usize = 8192; const INFERENCE_LOCAL_HOST: &str = "inference.local"; +/// Maximum total bytes for a streaming inference response body (32 MiB). +const MAX_STREAMING_BODY: usize = 32 * 1024 * 1024; + +/// Idle timeout per chunk when relaying streaming inference responses. +const CHUNK_IDLE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + /// Result of a proxy CONNECT policy decision. struct ConnectDecision { action: NetworkAction, @@ -1045,18 +1051,35 @@ async fn route_inference_request( let header_bytes = format_http_response_header(resp.status, &resp_headers); write_all(tls_client, &header_bytes).await?; - // Stream body chunks as they arrive from the upstream. + // Stream body chunks with byte cap and idle timeout. + let mut total_bytes: usize = 0; loop { - match resp.next_chunk().await { - Ok(Some(chunk)) => { + match tokio::time::timeout(CHUNK_IDLE_TIMEOUT, resp.next_chunk()).await { + Ok(Ok(Some(chunk))) => { + total_bytes += chunk.len(); + if total_bytes > MAX_STREAMING_BODY { + warn!( + total_bytes = total_bytes, + limit = MAX_STREAMING_BODY, + "streaming response exceeded byte limit, truncating" + ); + break; + } let encoded = format_chunk(&chunk); write_all(tls_client, &encoded).await?; } - Ok(None) => break, - Err(e) => { + Ok(Ok(None)) => break, + Ok(Err(e)) => { warn!(error = %e, "error reading upstream response chunk"); break; } + Err(_) => { + warn!( + idle_timeout_secs = CHUNK_IDLE_TIMEOUT.as_secs(), + "streaming response chunk idle timeout, closing" + ); + break; + } } } diff --git a/crates/openshell-server/src/auth.rs b/crates/openshell-server/src/auth.rs index 5a3229ffa..b896d062c 100644 --- a/crates/openshell-server/src/auth.rs +++ b/crates/openshell-server/src/auth.rs @@ -22,11 +22,28 @@ use axum::{ response::{Html, IntoResponse}, routing::get, }; +use http::header; use serde::Deserialize; use std::sync::Arc; use crate::ServerState; +/// Validate that a confirmation code matches the CLI-generated format. +/// +/// Codes are 3 alphanumeric characters, a dash, then 4 alphanumeric characters +/// (e.g., "AB7-X9KM"). The CLI generates these from the charset `[A-Z2-9]`. +fn is_valid_code(code: &str) -> bool { + let bytes = code.as_bytes(); + bytes.len() == 8 + && bytes[3] == b'-' + && bytes[..3] + .iter() + .all(|b| b.is_ascii_uppercase() || b.is_ascii_digit()) + && bytes[4..] + .iter() + .all(|b| b.is_ascii_uppercase() || b.is_ascii_digit()) +} + #[derive(Deserialize)] struct ConnectParams { callback_port: u16, @@ -54,6 +71,15 @@ async fn auth_connect( Query(params): Query, headers: HeaderMap, ) -> impl IntoResponse { + // Reject codes that don't match the CLI-generated format to prevent + // reflected XSS via crafted URLs. + if !is_valid_code(¶ms.code) { + return Html( + "

Invalid confirmation code format.

".to_string(), + ) + .into_response(); + } + let cf_token = headers .get("cookie") .and_then(|v| v.to_str().ok()) @@ -68,14 +94,34 @@ async fn auth_connect( .and_then(|v| v.to_str().ok()) .map_or_else(|| state.config.bind_address.to_string(), String::from); + let safe_gateway = html_escape(&gateway_display); + match cf_token { - Some(token) => Html(render_connect_page( - &gateway_display, - params.callback_port, - &token, - ¶ms.code, - )), - None => Html(render_waiting_page(params.callback_port, ¶ms.code)), + Some(token) => { + let nonce = uuid::Uuid::new_v4().to_string(); + let csp = format!( + "default-src 'none'; script-src 'nonce-{nonce}'; style-src 'unsafe-inline'; connect-src http://127.0.0.1:*" + ); + ( + [(header::CONTENT_SECURITY_POLICY, csp)], + Html(render_connect_page( + &safe_gateway, + params.callback_port, + &token, + ¶ms.code, + &nonce, + )), + ) + .into_response() + } + None => { + let csp = "default-src 'none'; style-src 'unsafe-inline'".to_string(); + ( + [(header::CONTENT_SECURITY_POLICY, csp)], + Html(render_waiting_page(params.callback_port, ¶ms.code)), + ) + .into_response() + } } } @@ -104,22 +150,27 @@ fn render_connect_page( callback_port: u16, cf_token: &str, code: &str, + nonce: &str, ) -> String { - // Escape the token for safe embedding in a JS string literal. - let escaped_token = cf_token - .replace('\\', "\\\\") - .replace('\'', "\\'") - .replace('"', "\\\"") - .replace('<', "\\x3c") - .replace('>', "\\x3e"); + // Use JSON serialization for JS-safe string embedding — handles all + // edge cases including \n, \r, U+2028, U+2029 that break JS string + // literals. serde_json::to_string produces a quoted JSON string + // (e.g., "value") which is a valid JS string literal. + // + // We additionally escape < and > to \u003c / \u003e because while + // they're valid in JSON, they're dangerous inside an HTML before the JS parser runs). + let json_token = serde_json::to_string(cf_token) + .unwrap_or_else(|_| "\"\"".to_string()) + .replace('<', "\\u003c") + .replace('>', "\\u003e"); + let json_code = serde_json::to_string(code) + .unwrap_or_else(|_| "\"\"".to_string()) + .replace('<', "\\u003c") + .replace('>', "\\u003e"); - // Escape the code the same way (it's alphanumeric + dash, but be safe). - let escaped_code = code - .replace('\\', "\\\\") - .replace('\'', "\\'") - .replace('"', "\\\"") - .replace('<', "\\x3c") - .replace('>', "\\x3e"); + // HTML-safe version of the code for display in the page body. + let html_code = html_escape(code); let version = openshell_core::VERSION; @@ -250,7 +301,7 @@ fn render_connect_page(
Connect to Gateway
Confirmation Code
-
{escaped_code}
+
{html_code}
Verify this matches the code shown in your terminal
@@ -271,9 +322,9 @@ fn render_connect_page(
- ", "ABC-1234"); - // < and > should be escaped + let html = render_connect_page( + "gw", + 1234, + "token", + "ABC-1234", + "nonce", + ); + // < and > should be escaped via JSON encoding (\u003c) assert!(!html.contains("