mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-21 06:11:31 +08:00
v0.0.43
681
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Drew Newberry
|
09bd8a9c66 | fix(vm): preserve guest TLS hostname (#1416) v0.0.43 | ||
|
Drew Newberry
|
f819f7dcb5 | fix(vm): restore sandboxes after gateway restart (#1407) | ||
|
Drew Newberry
|
403c754842 |
fix(ci): skip helm plugin verification in CI image (#1411)
Signed-off-by: Drew Newberry <anewberry@nvidia.com> |
||
|
Drew Newberry
|
910d3f09f2 |
feat(vm): boot sandboxes from ext4 root disks (#1263)
* feat(vm): boot sandboxes from ext4 root disks * feat(vm): boot sandboxes with writable overlay disk * fix(sandbox): improve sandbox provisioning behavior * fix(vm): repair sandbox image provisioning * fix(vm): clean up rebase fallout * perf(vm): cache empty overlay images * perf(vm): skip repeated sandbox ownership repair * fix(vm): address lifecycle review issues * fix(vm): repair gateway vm startup * wip |
||
|
Taylor MutchandDrew Newberry
|
b61a98dbad |
feat(gateway): add TOML configuration file (RFC 0003) (#1317)
* feat(gateway): add TOML configuration file (RFC 0003)
Introduces an opt-in --config / OPENSHELL_GATEWAY_CONFIG flag that loads a
TOML file with gateway-wide settings and per-driver tables. Source
precedence is CLI > env > file > built-in default, implemented via clap's
ValueSource so existing flags and env vars keep their priority.
Driver crates (kubernetes, docker, podman, vm) now derive Deserialize on
their config structs. SupervisorSideloadMethod gains Deserialize with
kebab-case rename. A per-driver inheritance allowlist on the loader side
overlays [openshell.gateway] shared defaults (default_image,
supervisor_image, image_pull_policy, guest_tls_*, ssh_handshake_skew_secs,
client_tls_secret_name, host_gateway_ip, enable_user_namespaces) onto
each [openshell.drivers.<name>] table before deserialization.
The Helm chart renders a new gateway-config ConfigMap and mounts it at
/etc/openshell/gateway.toml. The migrated OPENSHELL_* env entries are
dropped from the StatefulSet — only the Secret-backed
OPENSHELL_SSH_HANDSHAKE_SECRET remains. database_url stays on --db-url.
Adds examples/gateway/gateway.example.toml and updates architecture/gateway.md
with the source precedence and inheritance rules.
* docs(gateway): drop ssh_handshake_skew_secs and ssh_handshake_secret from examples
Both fields are scheduled for removal. Remove the example values and the
env-only note so the gateway.toml example and the architecture doc stop
recommending settings that will not exist much longer.
* docs(rfc): correct OPENSHELL_CONFIG to OPENSHELL_GATEWAY_CONFIG in RFC 0003
* docs(gateway): add per-driver TOML example configurations
Adds focused single-driver examples next to the comprehensive
gateway.example.toml: kubernetes, docker, podman, and microvm. Each one
demonstrates the realistic settings for that driver plus how shared
[openshell.gateway] defaults inherit into the driver table.
A new unit test (`checked_in_examples_parse`) loads every example through
the config_file loader so schema drift fails CI rather than silently
shipping a broken example.
* refactor(gateway): drop image_pull_policy from shared inheritance
Kubernetes and Podman use mutually-incompatible vocabularies for the same
TOML key:
- Kubernetes: `Always | IfNotPresent | Never` (free-form string passed
verbatim to the K8s API).
- Podman: `always | missing | never | newer` (strict lowercase enum
deserialised into `ImagePullPolicy`).
No value means the same thing in both drivers. Sharing the key at
`[openshell.gateway]` scope and inheriting it into every active driver's
table meant any value safe for one driver was either wrong or silently
dropped for the other (`IfNotPresent` → `ImagePullPolicy::Missing` after
`.unwrap_or_default()`). Operators run one driver per gateway, so the
"shared default" never pays for itself.
Make `image_pull_policy` driver-local:
- Remove the field from `GatewayFileSection` and from
`inheritable_keys()` for both Kubernetes and Podman.
- Drop the file→`RunArgs` merge for the gateway-scope key.
- Stop unconditionally clobbering the driver value with
`config.sandbox_image_pull_policy` in the runtime wiring — only apply
the CLI/env override when it was set (and, for Podman, only when it
parses into the lowercase enum).
- Move the key under `[openshell.drivers.kubernetes]` and
`[openshell.drivers.podman]` in every example, the RFC, the
architecture doc, and the Helm-rendered gateway ConfigMap.
The supervisor pull policy follows the same shape: it is K8s-only and
moves into `[openshell.drivers.kubernetes]` alongside `image_pull_policy`
in the Helm template.
* fix(gateway): address review feedback on TOML configuration
Resolves the P1 and P2 issues raised in PR #1317:
- Helm gateway ConfigMap moves `grpc_endpoint` under
`[openshell.drivers.kubernetes]` so the default install no longer fails
the gateway's `deny_unknown_fields` schema check.
- `kubernetes_config_from_file` and `podman_config_from_file` only let
the gateway-wide CLI/env `grpc_endpoint` overwrite the driver-table
value when it was actually supplied, preserving file-only configs.
- Kubernetes driver default `image_pull_policy` is now empty (was Podman
vocabulary "missing"), so default deployments let the Kubernetes API
apply its own policy instead of being rejected.
- New `disable_tls` gateway field plumbs `.Values.server.disableTls`
through the TOML ConfigMap instead of relying on env vars dropped from
the StatefulSet.
- StatefulSet pod template now carries a `checksum/gateway-config`
annotation so `helm upgrade` rolls pods when the ConfigMap changes.
- Auxiliary listener resolution preserves the full `SocketAddr` from
`health_bind_address` / `metrics_bind_address`, so a loopback-pinned
health port is not silently relocated onto the public bind address.
- `ssh_session_ttl_secs` from the file is now applied to `Config` (it
was previously accepted by the loader but never read).
New regression coverage: cli-level merge tests for the new fields plus
helm-unittest assertions for the ConfigMap shape, checksum annotation,
and `disable_tls` rendering.
* docs(gateway): consolidate gateway TOML examples into docs reference
Replaces the per-driver example files under examples/gateway/ with a
single published reference page at docs/reference/gateway-config.mdx
covering source precedence, layout, the full example, and the four
per-driver examples (Kubernetes, Docker, Podman, microVM). Drew flagged
during PR #1317 review that the examples belong with the user-facing
docs rather than in a sibling examples/ directory.
The cross-references in architecture/gateway.md and RFC 0003 are updated
to point at the new docs page; the round-trip test in config_file.rs is
removed (schema coverage stays on the inline parses_full_example test
and per-field merge tests — doc-snippet drift belongs in a separate
docs-lint, not in a cross-tree Rust unit test).
* refactor(core): move DEFAULT_K8S_NAMESPACE into K8s driver
The constant is Kubernetes-specific (used only by KubernetesComputeConfig's
Default impl) and does not belong in openshell-core. Relocate it to the
driver crate that owns the K8s vocabulary; openshell-core retains only
truly cross-cutting defaults.
* refactor(core): move Podman bridge default into Podman driver
DEFAULT_NETWORK_NAME is Podman vocabulary, consumed only by the Podman
driver. Also drops the unused DEFAULT_IMAGE_PULL_POLICY constant.
* docs(auth): scrub remaining SSH handshake secret references
Sweeps the trailing mentions left after the rebase: the gateway
config-file module doc, the Helm gateway-config ConfigMap header,
the gateway-config.mdx env-only note, and the RPM systemd unit
comment for init-gateway-env.sh.
* docs(gateway): clarify OPENSHELL_GRPC_ENDPOINT applies to all drivers
The previous comment implied the callback endpoint was Kubernetes-only,
but the value is propagated to every compute driver (Kubernetes, Docker,
Podman, VM) and must be reachable from wherever the sandbox runs.
* refactor(gateway): move driver options into config (#1394)
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* fix(e2e): regenerate gateway config via TOML for docker + podman harnesses
The gateway CLI flags moved into TOML config tables in 560550d2 (#1394),
which made every existing e2e/with-{docker,podman}-gateway.sh invocation
fail with "unexpected argument '--sandbox-namespace'" (and a long tail
of similar driver-specific options) before the gateway could even bind.
Replace the obsolete CLI flags with a synthesized
`[openshell.drivers.<driver>]` table written to `${STATE_DIR}/gateway.toml`
and passed via the new `--config` flag. Only the gateway-wide flags
that survived 560550d2 (bind-address, port, drivers, db-url, tls-*,
disable-tls, log-level, health-port) stay on the command line.
Both scripts get a small `toml_string` helper to properly TOML-quote
the values (the previous `%q` printf format produced bash-escape, not
TOML-escape). The Docker harness also corrects two field names that
diverged from the driver schema: `docker_network_name` →
`network_name`, and the supervisor binary/image plumbing now reads
through to `supervisor_bin` / `supervisor_image` in the same table.
The Podman harness drops `--ssh-gateway-port` (deleted in 560550d2 —
gRPC + SSH are multiplexed on the same port now) and substitutes
`network_name` + `gateway_port` for the obsolete `--sandbox-namespace`
(which the Podman driver never had as a typed field).
* fix(core): swap bind-only 0.0.0.0 SSH gateway host for cluster URL host
CLI's resolve_ssh_gateway treated 0.0.0.0 as a loopback "keep as-is"
when the cluster URL was also loopback, so the SSH proxy connected to
0.0.0.0:port. The unspecified address is never a valid connect target
and is not present in any TLS cert SAN, which produced BadCertificate
TLS handshake failures during `openshell sandbox create -- ...` in
docker/podman e2e (e.g. bypass_detection).
Resolution: when the server returns 0.0.0.0 or :: as the gateway host
and both endpoints are loopback, fall back to the cluster URL's host
(which the CLI is already using to reach the gateway, so it must
resolve and match the cert).
* fix(e2e): repair podman harness on macOS
Podman 5.x with the applehv/libkrun provider no longer creates the legacy
~/.local/share/containers/podman/machine/podman.sock symlink, and
`podman system service` is a Linux-only subcommand — the macOS client
delegates the API service to the VM. Both assumptions in the harness
were stale, so the script tried to start a temporary service that podman
rejected with "unknown flag: --time".
- Discover the macOS socket via `podman machine inspect` instead of the
hardcoded path.
- On Darwin, fail fast with a "start podman machine" message rather than
attempting the Linux-only `podman system service` fallback.
- Write socket_path into [openshell.drivers.podman] so the in-process
driver picks up the discovered socket; the driver reads TOML only
after the config refactor (560550d2), so OPENSHELL_PODMAN_SOCKET alone
was no longer enough.
* fix(server): use clone_from for TLS client CA assignment
clippy 1.95.0 rejects assigning the result of `Clone::clone()` to an
existing variable under `-D warnings` (`assigning_clones`). Switch to
`clone_from(&...)` to satisfy the lint and avoid the redundant
allocation.
---------
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Co-authored-by: Drew Newberry <anewberry@nvidia.com>
|
||
|
Mesut Oezdil
|
283defd285 |
fix(sandbox): allow HEAD where GET is permitted in L7 policy (#1382)
* fix(sandbox): allow HEAD where GET is permitted in L7 policy RFC 9110 §9.3.2 defines HEAD as identical to GET except no response body. method_matches in sandbox-policy.rego rejected HEAD even when GET was explicitly allowed, breaking package managers that probe with HEAD before fetching. Signed-off-by: mesutoezdil <mesudozdil@gmail.com> * test(sandbox): fix binary mismatch in HEAD/POST test and add deny-rule regression l7_head_denied_when_only_post_allowed used /bin/sh in the fixture but l7_input() sends /usr/bin/curl, so requests were denied by binary policy before the method rule could be evaluated. A regression allowing HEAD to match POST would have passed undetected. Fixes the fixture binary and adds l7_head_blocked_by_deny_rule_targeting_get to cover the deny_rules path of method_matches(), per review feedback. Signed-off-by: mesutoezdil <mesudozdil@gmail.com> --------- Signed-off-by: mesutoezdil <mesudozdil@gmail.com> |
||
|
Florent BENOIT
|
442b0b6b92 |
feat(exec): add bidirectional streaming for interactive TTY sessions (#1331)
* feat(exec): add bidirectional streaming for interactive TTY sessions The existing ExecSandbox RPC sends stdin upfront in the request body, making interactive programs (bash, top, vim) unusable with --tty. This adds an ExecSandboxInteractive bidirectional streaming RPC that forwards live keystrokes, terminal resize events, and EOF to the sandbox. - Add ExecSandboxInteractive RPC, ExecSandboxInput and ExecSandboxWindowResize messages to the proto definition - Implement server handler with relay bridging, PTY allocation via russh channel.split(), and timeout support - Implement CLI client with raw mode, spawn_blocking stdin reader, SIGWINCH resize forwarding, and process::exit for clean shutdown - Route to interactive path only when --tty is explicitly passed Signed-off-by: Florent Benoit <fbenoit@redhat.com> * fix(exec): address interactive TTY streaming review feedback Server-side (sandbox.rs): - Close SSH channel after EOF so PTY-backed programs (bash, vim, top) terminate on client disconnect instead of leaking the session - Break output loop when tx.send() fails (client gone) to tear down the streaming pipeline promptly - Default zero cols/rows to 80x24 before request_pty, matching the proto "0 = use default" contract and preventing 1x1 terminals for non-CLI clients Client-side (run.rs): - Replace spawn_blocking stdin reader with a detached std::thread so the tokio runtime shutdown does not hang on a thread blocked in stdin.read(). This removes the need for std::process::exit(), allowing save_last_sandbox() in the caller to execute normally - Switch from futures::channel::mpsc with try_send() to tokio::sync::mpsc with blocking_send() for proper backpressure — try_send silently stopped forwarding all input when the channel filled during paste or slow network - Add TaskGuard (abort-on-drop) for the resize task to ensure cleanup on all return paths including early errors --------- Signed-off-by: Florent Benoit <fbenoit@redhat.com> |
||
|
Adam Miller
|
63bdcd1379 |
fix(sandbox): exempt host gateway from SSRF block for rootless Podman (#1279)
* fix(sandbox): exempt host gateway from SSRF block for rootless Podman pasta assigns a link-local IP (169.254.x.x) as the host gateway in rootless Podman containers, which was always blocked by SSRF guards. Read the trusted gateway IP from /etc/hosts at proxy startup (injected by the driver via host.openshell.internal / host.containers.internal aliases) and exempt only that specific IP from the link-local block, while keeping 169.254.169.254 (cloud metadata) hard-blocked regardless. * fix(net): add is_link_local_ip() helper and harden trusted-gateway SSRF Extract a public is_link_local_ip(IpAddr) helper in openshell-core::net covering IPv4 169.254.0.0/16, IPv6 fe80::/10, and IPv4-mapped variants. Refactor the inline IPv6 fe80::/10 bitmask in is_always_blocked_ip() and is_internal_ip() to use the new helper, eliminating two copies of the same pattern. Harden the trusted-gateway SSRF exemption in proxy.rs: both detect_trusted_host_gateway() and resolve_and_check_trusted_gateway() now reject always-blocked non-link-local IPs (loopback, unspecified) with a warning. The exemption was intended only for link-local addresses used by rootless Podman with pasta; a /etc/hosts entry mapping host.openshell.internal to 127.0.0.1 or 0.0.0.0 previously bypassed the loopback/unspecified SSRF invariant. Add 169.254. prefix to the generate_security_notes() host checks in mechanistic_mapper and policy server, closing a gap where link-local literal hosts were not flagged as internal/private in advisory notes. * fix(sandbox): warn on ambiguous /etc/hosts gateway and add real function tests When detect_trusted_host_gateway() finds more than one distinct IP for host.openshell.internal, emit a structured warning so operators can diagnose unexpected /etc/hosts state. The first entry still wins (and any runtime mismatch is caught by resolve_and_check_trusted_gateway), but the ambiguity is now observable. Replace inline guard-logic tests with real calls to resolve_and_check_trusted_gateway() using IP literals, which bypass DNS and give deterministic results. This exercises the actual function wiring rather than mirroring the predicate logic manually. Add parse_hosts_file_for_host tests covering: single entry, same-IP deduplication across lines, multiple distinct IPs (documents first-wins ordering), multi-alias lines, missing alias, and comment handling. * fix(sandbox): narrow trusted-gateway exemption to link-local IPs only Two security fixes addressing PR review feedback: 1. Replace the compound is_always_blocked_ip && !is_link_local_ip guard in detect_trusted_host_gateway() with a plain !is_link_local_ip check. Private RFC 1918 addresses (e.g. Docker bridge 172.17.0.1, K8s node 192.168.x.x) are not always-blocked, so they slipped through the old guard and received the SSRF exemption, bypassing resolve_and_reject_internal(). The exemption is only valid for the rootless Podman/pasta link-local case. 2. Normalize IPv4-mapped IPv6 in is_cloud_metadata_ip() so that ::ffff:169.254.169.254 is correctly identified as cloud metadata. is_link_local_ip() already recognises this form as link-local; without the normalization it would pass the metadata guard and receive the trusted-gateway exemption, violating the 'metadata is never exempted' invariant. Apply the same !is_link_local_ip tightening to resolve_and_check_trusted_gateway() as defense-in-depth, and update/add tests to cover both guards including IPv4-mapped metadata, private IP rejection, and the Docker bridge scenario. |
||
|
Florent BENOIT
|
a1fb9bd954 |
fix(scripts): replace mapfile with bash 3.2-compatible read loop (#1334)
macOS ships bash 3.2 which lacks mapfile (introduced in bash 4.0), causing docker-build-image.sh and stage-prebuilt-binaries.sh to fail on stock macOS developer machines. |
||
|
Adam Miller
|
f672f75e27 |
chore: remove SSH handshake secret residuals and fix agent memory (#1403)
* chore: remove SSH handshake secret residuals and fix agent memory Removes three artifacts left behind by the #1274 removal of OPENSHELL_SSH_HANDSHAKE_SECRET, then corrects a sweep of stale and inaccurate notes in .claude/agent-memory/arch-doc-writer/MEMORY.md that were discovered during the audit. Artifact removal (Refs OS-174): - openshell.spec: stale comment claiming init-gateway-env.sh generates an SSH handshake secret - e2e/with-podman-gateway.sh: dead podman secret rm for openshell-handshake-<id>, which is never created since #1274 Agent memory corrections: - ssh_tunnel.rs no longer exists; replaced by ssh_sessions.rs - Object types list was missing service_endpoint and provider_profile - Pre-exec chain now includes harden_child_process() and uses the linux::prepare()/enforce() two-phase pattern on Linux - CLI SSH function list had nonexistent sandbox_rsync; corrected to actual exported functions - ExecSandbox is in grpc/sandbox.rs (not grpc.rs) and operates over a supervisor relay DuplexStream, not a direct TCP connection - resolve_ssh_gateway() moved to openshell-core/src/forward.rs - SSH transport note rewrote: NSSH1 is an OCSF-only label (not a live protocol preface); actual path is ForwardTcp -> DuplexStream -> RelayStream -> Unix socket; access gated by CreateSshSession token; TLS follows endpoint scheme (https:// = mTLS, http:// = plaintext; Podman driver does not yet inject mTLS client materials) - CLI flag note was self-contradictory; corrected to --gateway-endpoint with resolution priority chain * fix(vm): collapse nested if blocks to satisfy clippy::collapsible_if Three nested if blocks in connect_local_container_engine() were flagged by clippy after #1370. Collapse to single if-let chains using && as suggested. |
||
|
Seth Jennings
|
c8bf387ace |
feat(tui): add OIDC authentication support (#1405)
Move EdgeAuthInterceptor from openshell-cli to openshell-core so the TUI can use it without a circular dependency. The TUI now checks gateway metadata for OIDC auth mode, loads the stored bearer token, and injects it into gRPC requests via the shared interceptor. When switching gateways, the TUI dispatches on auth_mode to build either an mTLS or OIDC-authenticated channel. Expired tokens surface a clear error directing users to re-authenticate with `openshell gateway login`. |
||
|
Taylor Mutch
|
590acded40 |
fix(vm): collapse nested if blocks in container engine connect (#1406)
Clippy's collapsible_if lint fails `mise run rust:lint` on main because
of the nested `if let Ok(..) { if ping().is_ok() { .. } }` pattern in
`connect_local_container_engine`. Introduced by
|
||
|
Seth Jennings
|
c94cddbfb8 |
feat(server): separate HTTPS from mTLS authentication (#1351)
Make --tls-client-ca optional and make client certificates always optional when a CA is configured. This decouples HTTPS encryption from mTLS authentication, allowing mTLS and OIDC bearer tokens to coexist as parallel authentication mechanisms. When --tls-client-ca is provided, client certificates are validated against the CA when presented but never required. Clients may connect with or without a certificate — authentication is handled at the application layer (e.g. OIDC). Two TLS modes are now supported: - HTTPS with optional mTLS (--tls-client-ca provided) - HTTPS-only (--tls-client-ca omitted) The --disable-gateway-auth flag is preserved for backward compatibility but is now a no-op. The allow_unauthenticated field has been removed from TlsConfig. The Helm chart conditionally includes the client-ca volume and env var based on whether clientCaSecretName is configured. |
||
|
Eric Curtin
|
9f8edb5a45 |
docs(installation): add container gateway page with docker run and compose examples (#1321)
Add a dedicated page documenting how to run the OpenShell gateway as a container using docker run, docker-compose, or podman, without the system package manager installer. This is useful for users on immutable OS distributions (Fedora CoreOS, bootc-based images, Silverblue) where the standard install.sh path is not appropriate, and for container-first environments. Covers a quick-start with TLS disabled (localhost-bound), a full mTLS setup using the gateway's generate-certs subcommand, a docker-compose example, and a Podman variant. Closes the gap raised in #1285. Signed-off-by: Eric Curtin <eric.curtin@docker.com> |
||
|
Russell Bryant
|
9a7c0df00b |
fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration (#1329)
* fix(sandbox): remove DNS resolution from mechanistic mapper to prevent data exfiltration The mechanistic mapper resolved denied hostnames via DNS to check whether they mapped to private IPs (for populating allowed_ips in proposals). This leaked the denied hostname through DNS queries even though the HTTP connection was blocked by policy — a data exfiltration vector. Remove resolve_allowed_ips_if_private() entirely. Proposals no longer include allowed_ips. If a user applies a proposed rule and the host resolves to a private IP, the proxy's SSRF defense denies the connection. That denial flows back through the aggregator, and the user can then explicitly configure allowed_ips in their policy. This two-step flow is also more explicit about private IP access, which is desirable given #1245. Closes #1169 Signed-off-by: Russell Bryant <russell.bryant@gmail.com> * fix(sandbox): update SSRF note and document two-step allowed_ips flow Update the mechanistic mapper SSRF security note to reflect that proposals no longer populate allowed_ips — approving a proposal does not bypass SSRF protection. Clarify the two-step approval flow in architecture/security-policy.md. Signed-off-by: Russell Bryant <russell.bryant@gmail.com> --------- Signed-off-by: Russell Bryant <russell.bryant@gmail.com> |
||
|
Russell Bryant
|
44e843ede3 |
feat(vm): fall back to Podman socket when Docker is unavailable (#1370)
* test(e2e): add bypass detection test for sandbox REJECT rules Validates that direct TCP connections bypassing the HTTP CONNECT proxy are rejected immediately (ECONNREFUSED) rather than hanging until a network timeout. This is implementation-agnostic — it tests observable kernel behavior regardless of whether rules are installed via iptables or nftables. * feat(vm): fall back to Podman socket when Docker is unavailable The VM driver resolves sandbox images from a local container engine before falling back to registry pulls. Previously it only checked Docker. Now it tries the Podman socket as a fallback, since Podman exposes a Docker-compatible API via bollard. On Linux, enable the Podman API socket with: systemctl --user start podman.socket Signed-off-by: Russell Bryant <russell.bryant@gmail.com> * chore(vm): fix formatting Signed-off-by: Russell Bryant <russell.bryant@gmail.com> * refactor(vm): rename docker-specific identifiers to engine-neutral names Rename resolve_local_docker_image → resolve_local_container_image, local_docker_image_platform_mismatch → local_image_platform_mismatch, and the local docker variable → engine since the function now supports both Docker and Podman backends. Signed-off-by: Russell Bryant <russell.bryant@gmail.com> --------- Signed-off-by: Russell Bryant <russell.bryant@gmail.com>v0.0.42 |
||
|
Russell Bryant
|
c27dd88aa9 |
fix(vm): enable NFT_LOG kernel module for nftables bypass detection (#1391)
The nftables log statement requires CONFIG_NFT_LOG, which was not included in the VM guest kernel config. Without it, nft rejects rules containing the log keyword with "No such file or directory". |
||
|
Drew Newberry
|
f58a434a95 |
fix(installer): dump gateway logs on startup timeout (#1396)
Signed-off-by: Drew Newberry <anewberry@nvidia.com> |
||
|
Drew Newberry
|
f5b546e41b |
Revert "perf(build): speed up local CLI rebuilds (#1387)" (#1395)
This reverts commit
|
||
|
Russell Bryant
|
0dee90abdb |
fix(vm): make /sandbox chown non-fatal for virtiofs rootless hosts (#1389)
On Linux non-root hosts, virtiofs maps guest root to the host user, so chown inside the VM is denied. This causes the init script to abort under set -e, preventing the sandbox from starting. Make the chown best-effort: if it fails, log a warning and continue. The supervisor's own filesystem preparation handles ownership for the paths that matter, so the sandbox works correctly without it. This is a pragmatic workaround — anything that would break due to the failed chown will still break, just later and with a clearer error rather than a cryptic boot failure. In practice the sandbox functions normally because the supervisor sets up its own working directories. Signed-off-by: Russell Bryant <russell.bryant@gmail.com> |
||
|
Taylor Mutch
|
7a0c444445 |
refactor!(auth): drop SSH handshake secret (#1274)
* refactor!(auth): drop SSH handshake secret in favor of mTLS The OPENSHELL_SSH_HANDSHAKE_SECRET / x-sandbox-secret mechanism was misnamed: it does not authenticate SSH (which flows over the RelayStream gRPC RPC and is gated by mTLS plus supervisor Unix-socket permissions). It only gated a small set of sandbox-to-gateway control-plane RPCs, and production deployments already enforce mTLS on that channel — so the shared secret was redundant. Replace the secret check with an mTLS-presence marker. Sandbox-class methods (ReportPolicyStatus, PushSandboxLogs, GetSandboxProviderEnvironment, SubmitPolicyAnalysis, GetSandboxConfig, GetInferenceBundle) accept callers without a Bearer token; the gRPC mTLS handshake is the trust boundary. Dual-auth methods treat Bearer-present as full-scope CLI access and Bearer-absent as sandbox-restricted scope via validate_sandbox_caller_update. Drops the secret from all drivers (K8s, Podman, VM), the sandbox gRPC interceptor, the Helm chart (values + pre-install hook + StatefulSet env), the RPM bootstrap script, the man pages, and the debug-openshell-cluster skill. Also removes the never-read ssh_handshake_skew_secs flag and config field. BREAKING CHANGE: --ssh-handshake-secret / OPENSHELL_SSH_HANDSHAKE_SECRET and --ssh-handshake-skew-secs / OPENSHELL_SSH_HANDSHAKE_SKEW_SECS are removed from the gateway, sandbox, and all driver binaries. The openshell-ssh-handshake K8s Secret is no longer managed by the chart; operators may delete the orphan. Deployments using --disable-gateway-auth must enforce caller authentication at the fronting proxy, since the gateway no longer validates a per-request secret on sandbox-class methods. Refs OS-174. * docs(auth): scrub residual SSH handshake secret references Sweep across docs, e2e scripts, the Podman driver README/NETWORKING notes, the RPM/Helm/setup guides, the gateway man page, and RFC 0003 to remove instructions and examples that still referenced OPENSHELL_SSH_HANDSHAKE_SECRET / --ssh-handshake-secret / ssh_handshake_skew_secs. The mechanism is gone; nothing should still suggest setting it. Negative-assertion regression tests are kept so the env var cannot silently be re-introduced. * test(drivers): drop SSH handshake secret negative-assertion tests The supporting code, env vars, CLI flags, and config plumbing are gone — these tests asserted absence of strings that no longer have any path to being set. Remove the guards from the Docker, Podman, Kubernetes, and VM driver test modules. |
||
|
Russell Bryant
|
94025d834a |
fix(server): downgrade expected connection teardown errors to debug (#1369)
h2 connection errors during sandbox deletion are expected — the supervisor gRPC stream is torn down when the sandbox is removed. Downgrade these from ERROR/WARN to DEBUG so they don't pollute gateway logs during normal operations. Signed-off-by: Russell Bryant <russell.bryant@gmail.com> |
||
|
John T. Myers
|
668c712b63 | perf(build): speed up local CLI rebuilds (#1387) | ||
|
Eric Curtin
|
6deb1f005a |
refactor(core): eliminate duplicate utilities across crates (#1381)
Consolidate shared constants and helpers into openshell-core to avoid duplicate definitions scattered across crates. Adds OPENSHELL_SANDBOX to the sandbox_env module so all drivers and the sandbox supervisor reference it via a typed constant rather than string literals, matching the existing pattern for SANDBOX_ID, ENDPOINT, and the other env vars.v0.0.41 |
||
|
Mesut Oezdil
|
1c317646c6 |
docs: replace --sync with --upload . in sync-files example (#1366)
The --sync flag was never implemented. The correct flag is --upload with a path argument. Updated examples and workflow snippet to reflect the actual CLI interface. Fixes #1339 Signed-off-by: Mesut Oezdil <versusfinem@gmail.com> |
||
|
Drew Newberry
|
f855c3d8bb |
feat(cli): add sandbox resource flags (#1376)
Signed-off-by: Drew Newberry <anewberry@nvidia.com> |
||
|
Drew Newberry
|
0471c6d2ab | fix(gateway): keep vm driver opt-in (#1375) | ||
|
Seth Jennings
|
52c775701f | feat(helm): support custom CA for OIDC issuer TLS verification (#1373) | ||
|
Russell Bryant
|
bbfcac8a8e |
test(e2e): add bypass detection test for sandbox REJECT rules (#1368)
Validates that direct TCP connections bypassing the HTTP CONNECT proxy are rejected immediately (ECONNREFUSED) rather than hanging until a network timeout. This is implementation-agnostic — it tests observable kernel behavior regardless of whether rules are installed via iptables or nftables. |
||
|
Tinson Lai
|
c99849bcb7 |
fix(cli): cp-style sandbox download and workspace-boundary check (#1353)
* fix(cli): cp-style sandbox download and workspace-boundary check `openshell sandbox download` produced a directory at the host destination instead of writing the source bytes as a regular file, and did not refuse sandbox-side paths that resolved outside `/sandbox` (only `creds.json` was rejected, by filesystem policy rather than by an explicit check). Mirrors the cp-style design from the upload-side fixes (#667 / #694 and #885 / #952): destination resolution now respects trailing slashes and existing-directory destinations, single-file sources land as regular files via a staged extraction and atomic rename, and the function delegates the SSH tar stream to a shared helper. Adds an explicit lexical workspace-boundary check on the sandbox-side source — required on the download side because filesystem policy alone is not a substitute for an explicit check when crossing the trust boundary in the leak direction. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * fix(cli): canonicalise sandbox download path remotely and quote tar member The lexical check in `validate_sandbox_source_path` clears any path whose components stay under `/sandbox` after `.` and `..` are folded out, but it cannot see symlinks. A workspace-relative symlink such as `/sandbox/etc-link -> /etc` lets `openshell sandbox download <name> /sandbox/etc-link/passwd /tmp/passwd` slip through, then `tar -C /sandbox/etc-link passwd` follows the link on the sandbox side and streams `/etc/passwd` back to the host. Add `canonicalize_sandbox_source_path` which runs `realpath -e --` over SSH and rejects the result unless it still equals `/sandbox` or starts with `/sandbox/`. The lexical guard stays as a cheap fail-fast for obviously-bad inputs, and the rest of the download flow now uses the canonical path everywhere a tar invocation or probe might dereference a symlink. While editing the single-file branch, also add the `--` separator before the basename in `tar cf - -C <parent> -- <name>`. Without it, a sandbox file whose basename starts with `-` (e.g. `--checkpoint-action=...`) is parsed by GNU tar as an option rather than a member, which at best fails the wrap and at worst triggers tar option behaviour. The directory branch passes literal `.` and is unaffected. Cover both fixes with pure-function tests: `is_under_sandbox_workspace` gets accept/reject pairs (the same predicate the post-realpath validator uses) and the new `build_single_file_tar_cmd` helper asserts the `--` separator and shell escaping survive any future refactor. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * chore(cli): drop redundant tempfile dev-dependency entry `tempfile` was promoted from `[dev-dependencies]` to `[dependencies]` once the cp-style single-file download path started using `TempDir::new_in` from production code. The original `[dev-dependencies]` line stayed behind by accident — entries under `[dependencies]` are already available to test compilation, so the second declaration is dead weight. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * test(cli): add E2E coverage for sandbox download paths Adds four end-to-end tests for `openshell sandbox download` so the happy path and both Codex regressions have explicit coverage: * `sandbox_download_file_only` — seeds a single file via the new `SandboxGuard::exec` harness helper and pulls it back, exercising `sandbox_sync_down_file` without an upload phase. * `sandbox_download_directory_only` — seeds a small directory tree and pulls it recursively, exercising `sandbox_sync_down_directory`. * `sandbox_download_rejects_symlinks_pointing_outside_workspace` — regression for the high-severity finding. Plants three escape shapes (`/sandbox/etc-link -> /etc`, `/sandbox/passwd-link -> /etc/passwd`, and a path with a symlinked component `/sandbox/etc-link/passwd`) and asserts all three are refused with a workspace-boundary error before any tar streams. Also checks the host destination stays empty so a silent leak would surface. * `sandbox_download_handles_dash_leading_basename` — regression for the medium-severity finding. Seeds a file named `--checkpoint-action=evil` and verifies it downloads with byte-for-byte contents, proving the `--` separator stops tar parsing the basename as an option. The harness gains a single new method, `SandboxGuard::exec`, that wraps `openshell sandbox exec --name <name> --no-tty -- <argv>`. Existing tests preferred the upload path to populate sandbox state, but the regression cases need to plant filesystem state the upload flow cannot produce (symlinks, files whose basenames clap would otherwise reject). Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * docs(cli): note cp-style destination and workspace boundary on download `openshell sandbox download` now refuses any source path that resolves outside `/sandbox` (lexically or through a symlink), and the single-file branch follows `cp`-style placement rules. Document both so users hitting the workspace-boundary error or the "land at exact path vs land inside dir" decision can self-serve. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * test(cli): tolerate miette line-wrapping in symlink-rejection assertions `sandbox_download_rejects_symlinks_pointing_outside_workspace` matched the substring `"outside the sandbox workspace"` against the CLI's error output. The check is correct in spirit but fragile in practice: miette wraps long single-line errors across multiple lines at terminal width and inserts a `│ ` continuation marker, so the substring breaks across the wrap and the test fails even though the canonicalisation step correctly refused the source. Split the match into three short markers — `resolves to`, `outside the`, `sandbox workspace` — each guaranteed to fit on one wrapped line. The combined check is stricter than the original (it now requires evidence that the remote `realpath -e` step actually ran, not just the boundary refusal) without inheriting the wrap fragility. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> |
||
|
Seth Jennings
|
0c8c723096 | fix(images): remove image-specific owner and mode set for gateway binary (#1371) | ||
|
Alexander Watson
|
ea2fddbe2d |
feat(policy): agent-driven policy management — the agent half (#1323)
* feat(policy): plumb chunk_ids and rejection_reason through proposal pipeline Prereq plumbing for the agent revise-and-resubmit loop. Two narrow additive proto changes unblock the upcoming /wait endpoint (#1092), prover validation badge (#1097), and reject --guidance surfaces (#1098). - SubmitPolicyAnalysisResponse: add accepted_chunk_ids so the in-sandbox agent gets handles to watch its proposals. Surfaced through the typed grpc_client wrapper and policy.local's POST /v1/proposals 202 body. Closes #1094. - PolicyChunk + StoredDraftChunk + DraftChunkPayload: add validation_result (gateway prover verdict, populated by #1097) and rejection_reason (operator free-form text). Both plain strings; no enums, no parsing on the read path. Closes #1096. - RejectDraftChunk now persists the existing reason field into the chunk's rejection_reason so it round-trips back to the agent via GetDraftPolicy. UndoDraftChunk clears it on the way back to pending so consumers cannot read a stale guidance string from a prior reject -> re-approve -> undo cycle. Whole surface stays gated behind agent_policy_proposals_enabled. Two focused tests cover the round-trip and the undo-clears guarantee. Signed-off-by: Alexander Watson <zredlined@gmail.com> * feat(sandbox): add /v1/proposals/{id} and /wait long-poll to policy.local The agent feedback channel back from policy.local. Two new routes let the in-sandbox agent learn its proposal's outcome on a single blocking HTTP call — zero LLM tokens during the wait. - GET /v1/proposals/{chunk_id} returns the chunk's current state in one gateway call. - GET /v1/proposals/{chunk_id}/wait?timeout=<s> blocks until the chunk transitions out of pending. Default 60s, clamped [1, 300]. Agent re-issues on timeout to extend. Response carries the chunk's status plus the two feedback fields shipped in the prereq commit: rejection_reason (free-form reviewer text) and validation_result (gateway prover verdict, empty until #1097). On timeout: same shape with timed_out: true so the agent can disambiguate without parsing. Wait handler short-polls GetDraftPolicy every 1s inside the request with a tokio::time::Instant deadline. One gateway connection is opened per request and reused across all polls, so a 60s wait does one TLS handshake instead of sixty. A future commit can swap the loop body for a tokio::sync::broadcast driven by a watcher task — the agent-visible contract (URL, query, response shape) is independent of the polling implementation. All routes stay behind agent_policy_proposals_enabled. Closes #1092. Signed-off-by: Alexander Watson <zredlined@gmail.com> * docs(sandbox): teach policy_advisor skill the wait + redraft loop The agent-facing instructions for the feedback loop. The endpoints exist; this is the doc that makes them usable. policy_advisor.md gains: - API entries for GET /v1/proposals/{chunk_id} and /wait?timeout=<s>, including the field semantics (status, rejection_reason, validation_result, timed_out). - A note on the submit response's accepted_chunk_ids / rejection_reasons split so the agent handles partial acceptance. - Step 6 saves the chunk_ids and addresses any submit-time rejections before waiting. - Step 7 walks the four wait outcomes: approved (retry, with the honest "may still fail" caveat), rejected (read rejection_reason AND validation_result; address whichever has content), still-pending with timed_out (re-call), non-2xx (surface, do not retry). skills.rs gains two assertions on the skill content so a future edit cannot drop the wait endpoint or the rejection_reason directive silently. Closes #1095. Signed-off-by: Alexander Watson <zredlined@gmail.com> * test(policy-advisor): add end-to-end smoke for the agent feedback loop A focused smoke that exercises the new policy.local /wait endpoint on a live gateway + sandbox, separate from the existing no-LLM regression harness (which still drives the OLD retry-with-bash-loop recovery pattern). Two flows: - Flow A — approve-and-retry: agent submits, /wait blocks, host runs `openshell rule approve`, /wait returns status=approved. Confirms the happy path round-trip latency. - Flow B — reject-with-guidance: agent submits, /wait blocks, host runs `openshell rule reject --reason "..."`, /wait returns status=rejected with the exact reviewer text in rejection_reason. Confirms the free-form guidance contract round-trips through the agent feedback channel. No GitHub credentials needed — proposals are synthetic and never trigger outbound traffic. Both flows expect agent_policy_proposals_enabled=true and a running gateway. Adds three cases to sandbox-runner.sh: submit-test-proposal (no GH deps), proposal-status, proposal-wait. The existing put-file and submit-proposal cases used by test.sh are untouched. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(policy-advisor): surface real CLI errors from wait-smoke preflight The preflight piped openshell's stderr to /dev/null and relied on jq to default the missing setting key to "<unset>", but under `set -euo pipefail` a non-zero exit from openshell makes the whole pipeline fail and the command substitution exits the script silently before the intended fail() message can print. Capture stderr explicitly, check the CLI exit code, and surface the real error plus the expected fix (port-forward + gateway add + select) when the CLI cannot reach the gateway. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(policy-advisor): pass --json to settings get in wait-smoke preflight `openshell settings get --global` defaults to a human-readable table; jq cannot parse it and the preflight died with a numeric-literal error. Pass --json so jq gets actual JSON. Also touched up the suggested recovery commands in the preflight error to match the real CLI shape (`gateway add <endpoint> --name <name>` and the env-var override warning). Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(policy): dedup draft chunks only in mechanistic mode; return effective id The smoke harness for the agent feedback loop caught a real bug in the gateway: SubmitPolicyAnalysis's response carried a chunk_id that was never persisted whenever the SQL ON CONFLICT path fired. Two failure modes, both load-bearing: - Agent-authored proposals targeting the same host/port/binary (e.g. the redraft-after-rejection loop) silently folded into one row and any RejectDraftChunk by the new chunk_id failed with "chunk not found." Latent since #1151, surfaced by #1094 returning chunk_ids. - Mechanistic mode had the same class of bug — the dedup fold-in is the intended behavior there, but the response still advertised the newly-generated UUID instead of the existing row's id. Less visible because no current caller reads mechanistic chunk_ids back, but the proto contract was violated either way. Fix in three parts: - put_draft_chunk now takes Option<&str> dedup_key explicitly and returns the effective row id (via RETURNING). None binds NULL to the dedup_key column, which bypasses the partial-index ON CONFLICT path entirely. Caller-decides semantics replace store-side magic. - handle_submit_policy_analysis picks dedup_key per chunk using an allowlist (only "mechanistic" dedups) and pushes the returned effective_id to accepted_chunk_ids. New modes default to no-dedup so a misconfigured caller cannot silently lose proposals. - The two-copy draft_chunk_dedup_key helper consolidated to one observation_dedup_key in policy_store.rs with a doc comment. Tests: - agent_authored_submits_for_same_endpoint_do_not_dedup pins the redraft-loop contract: two intentional submissions with the same host/port/binary get distinct chunk_ids, both findable via GetDraftPolicy, both rejectable by id. - mechanistic_submits_for_same_endpoint_dedup_into_one_chunk locks in the observation-mode dedup AND asserts both submits return the same effective_id — would have caught the deeper bug. Proto: SubmitPolicyAnalysisRequest.analysis_mode doc updated to describe the actual semantics (mechanistic dedups, agent_authored and unknown modes do not). Signed-off-by: Alexander Watson <zredlined@gmail.com> * docs(examples): retarget policy-management demo at the /wait endpoint The narrated demo (examples/agent-driven-policy-management) has been the public face of this feature since #1151. Its agent prompt told Codex to retry the original PUT every few seconds for up to 120 seconds — a polling workaround for the missing /wait endpoint that this branch shipped. Update the demo to exercise /wait so the canonical reading of the feature reflects the actual UX win. - agent-task.md: step 4 is now "call /wait, branch on status" with the three outcomes spelled out (approved → retry once; rejected → read rejection_reason and revise or stop; pending+timed_out → re-issue /wait once, do NOT busy-loop or shorten the timeout). Also makes explicit that the demo submits one rule per proposal so accepted_chunk_ids[0] is the safe single id to wait on. - demo.sh: header docstring rewritten as a six-step loop that mirrors the README. narrate_sandbox_workflow drops its parallel numbering and uses bullets (the runtime narration is the agent's sub-actions, not a separate decomposition of the loop). Approve step header and success message now reference /wait waking the agent, not "policy hot-reload retry." - README.md: top-of-file flow expanded from 5 to 6 steps to include the /wait call and chunk_id capture; "Going further" section now describes both regression scripts and the boundary between them (real-GitHub retry vs. synthetic /wait wire test). Slow-path qualifier corrected from "image pull on first run" to "sandbox cold-start (SSH bring-up plus Codex install)". - wait-smoke.sh header rewritten to make it unambiguous this is a regression, NOT a tutorial, with explicit prereq commands instead of prereq descriptions, and a pointer at demo.sh for the narrated story. No code paths change; this is the readability pass. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(examples): pass --yes on demo.sh's global setting writes Global setting updates require explicit confirmation in non-interactive mode; demo.sh's enable_agent_proposals and the cleanup restore path were missing --yes and hard-failed the preflight. Pre-existing issue that surfaced now that more of the demo runs through this path. No other behavior change. Signed-off-by: Alexander Watson <zredlined@gmail.com> * feat(sandbox): emit OCSF audit events for policy proposal lifecycle The demo's policy decision trace previously showed only the proxy enforcement story (HTTP:PUT DENIED, CONFIG:LOADED, HTTP:PUT ALLOWED). It was silent about who proposed what or who decided what — the audit-trail receipts for the agent feedback loop were missing. policy.local now emits sandbox-side OCSF events at the observation moments, into the same stream as the existing CONFIG:LOADED: - CONFIG:PROPOSED on submit_proposal acceptance. Per accepted chunk: the message names the chunk_id, target endpoint, L7 method/path, and binary so the trace correlates against the inbox card via chunk_id. - CONFIG:APPROVED on /wait observation of approved status. - CONFIG:REJECTED on /wait observation of rejected status. Carries the reviewer's free-form rejection_reason in the message AND as an unmapped field, both sanitized (control chars stripped, capped at 200 chars with an ellipsis marker). The agent still reads the raw text via GET /v1/proposals/{id}; sanitization is audit-side only, per AGENTS.md's no-secrets-in-OCSF rule. The submit path defends the audit_summaries / accepted_chunk_ids index pairing against a future gateway change that compresses past rejected chunks (the proto doesn't promise 1:1 ordering with the request). Today client-side validation makes the lengths always match; if they don't, the pairing falls back to a generic per-id event rather than mis-attribute. The wait handler's emit site fires once per terminal-status observation. Multiple concurrent waiters on the same chunk would each emit one event; acceptable for single-waiter-per-chunk demos and the right place to dedup is the SIEM. demo.sh's trace filter now surfaces the four CONFIG: events alongside HTTP:PUT, so the trace at the end of every run tells the full story from deny to allow via propose -> approve. wait-smoke.sh's prereq notes recommend redirecting kubectl port-forward output so its "Handling connection for 8090" lines don't bleed into demo narration. Three new unit tests on the sandbox-side helpers — summary builder happy path, fallback, and the rejection_reason sanitizer. Signed-off-by: Alexander Watson <zredlined@gmail.com> * feat(policy): /wait awaits local policy reload; demo auto-approves redrafts Three things in one commit, all surfaced by running the demo end-to-end against a real gateway and finding the agent had to draft a broader second proposal. 1. /wait race fix. Previously /wait returned `approved` the moment it observed the gateway's chunk status flip, but the local supervisor reloads policy on its own poll cycle (~10s in practice). The agent's retry would race the reload and hit the still-old policy, getting denied. Codex then drafted a broader rule and re-submitted — sound agent behavior, but not what /wait should provoke. Now /wait captures the local policy version at start, and after observed-approved waits for the supervisor to load a strictly-newer version before returning. Bounded by the caller's deadline; best-effort return if the deadline elapses without the version bumping. Two new unit tests pin the happy path and the deadline-clamped fallback. 2. demo.sh auto-approve loop. Replaces approve_when_pending + wait_for_agent with one approve_pending_until_agent_exits function that keeps watching for pending chunks and approving them until the agent process exits (or the configured timeout). Defense in depth against future redraft scenarios for any reason; today (post-fix #1) the agent should only submit one proposal per task, but we don't want to hang silently if it does submit more. 3. UX. Step headers now carry "[t+1.2s]" relative timestamps so reading the run output makes latency visible (the demo's whole point is the wait is cheap — surface that). A spin_wait helper renders an ASCII spinner during the watch loop so the demo never looks frozen on a TTY. Falls back to plain sleep on non-TTY contexts. Closes the race condition diagnosed from the trace timing where the gateway approved at t+0, sandbox observed at t+0.3s, but the supervisor didn't load v2 until t+9.4s — well after the agent had already retried and been denied. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(sandbox): /wait detects policy reload by content, not the schema version The previous attempt at the /wait-after-approve race fix compared `SandboxPolicy.version` between /wait start and the policy reload — but that field is the *schema* version (constant 1), not a revision counter. Every comparison was `current(1) > baseline(1) == false`, so the wait blocked until the agent's 300s timeout regardless of whether the supervisor had actually reloaded. The demo SSH connection then timed out around the 240s mark. Diagnosed from a live run's OCSF trace: supervisor pulled v2 at +8.5s after approval (CONFIG:LOADED), but the sandbox-side CONFIG:APPROVED that my /wait emits didn't fire until +304s — exactly at the 300s deadline. Fix: compare the whole policy via prost's derived PartialEq. Any field change (network_policies map being the only one that actually mutates today) flips equality. A clone-per-200ms-tick on a few-KB proto is cheap inside the bounded wait window. Tests rewritten to match the new contract: the supervisor-reload fixture now keeps `version: 1` constant and changes `network_policies` contents, mirroring the exact failure mode from the live run. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(examples): redact tokens with python literal-string replace, not sed The sed-based redact_log in demo.sh broke when one of the auth tokens contained a character that conflicted with sed's pattern parser ("unterminated substitute pattern" on the Codex JWT). The whole log tail then blanks on failure, hiding the very failure context we're trying to surface. Switch to a python subprocess that takes the tokens via argv and does literal str.replace. No regex, no delimiter games, no truncation. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(sandbox): scope /wait reload check to the approved rule Reviewer (John Myers) flagged two failure modes in the prior whole-policy fingerprint approach used by policy.local /wait: - False sleep: when the supervisor reloads between two /wait calls (the skill tells the agent to re-issue on timed_out), the new call snapshots the already-updated policy as baseline and burns the full timeout waiting for a change that never comes. - False wakeup: any unrelated reload (other agent's approval, settings change) flips the diff, but the chunk's actual rule may not be loaded yet — the agent retries and hits policy_denied for no real signal. Replace the diff with rule-coverage. New public helper openshell_policy::policy_covers_rule reuses endpoints_overlap (so it matches add_rule's merge semantics, including the fold-into-existing-key case) plus an L7 allow check on method/path (so an existing endpoint that doesn't yet contain the proposed method doesn't signal coverage). Add policy_reloaded: true|false to the /wait response on approve, with a 500ms floor on the reload-wait phase so approvals arriving near the deadline still get a fair shot at reloaded=true. Update the policy_advisor skill to branch on it: reloaded=true → retry; reloaded=false → re-issue /wait once with timeout=30, then surface to user. Don't loop tightly. Tests: - 9 new unit tests in openshell-policy pinning coverage semantics (L4-only, L7 method gap, fold-into-existing-key, empty binaries). - 4 new tokio tests in policy_local mirroring John's exact scenarios. - wait-smoke.sh asserts policy_reloaded=true on Flow A. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(server): make GetDraftPolicy dual-auth so /wait works under OIDC policy.local calls GetDraftPolicy from inside the sandbox supervisor via the sandbox gRPC client, which authenticates with the shared x-sandbox-secret. GetDraftPolicy was listed only in the Bearer-auth scope table (config:read) and was not in SANDBOX_SECRET_METHODS or DUAL_AUTH_METHODS, so OIDC-enabled gateways rejected those calls and the /wait long-poll surfaced gateway_lookup_failed. Local/no-OIDC setups happened to work because the auth check is short-circuited. Add GetDraftPolicy to DUAL_AUTH_METHODS, matching the existing GetSandboxConfig pattern (called by both CLI reviewer surfaces with Bearer and the sandbox supervisor with x-sandbox-secret). Dual-auth short-circuits the scope check for sandbox-secret callers, so the config:read entry in authz.rs continues to gate Bearer-only flows. Mirror the openshell_get_sandbox_config_is_dual_auth assertion for GetDraftPolicy. Note: ssh_handshake_secret is server-wide, not per-sandbox, so a sandbox-secret caller can today name any sandbox in a SubmitPolicyAnalysis request — and now in a GetDraftPolicy request. The exposure is symmetric with the existing SANDBOX_SECRET_METHODS pattern. Filed as a follow-up: per-sandbox secret binding, tracked separately. Signed-off-by: Alexander Watson <zredlined@gmail.com> * fix(ci): address rebased check failures Signed-off-by: Alexander Watson <zredlined@gmail.com> --------- Signed-off-by: Alexander Watson <zredlined@gmail.com> |
||
|
Mesut Oezdil
|
96d909d9a1 |
feat(ci): add helm-unittest mise task and CI step (#1367)
Adds a helm:test mise task that installs the helm-unittest plugin if not present and runs chart unit tests under deploy/helm/openshell. Installs the plugin into Dockerfile.ci so CI runs do not need to download it each time. Closes #1281 Signed-off-by: Mesut Oezdil <versusfinem@gmail.com> |
||
|
alangou
|
5159ebc21d |
fix(server): restrict SQLite database file permissions to 0o600 (#1359)
Signed-off-by: Adrien Langou <alangou@nvidia.com> |
||
|
Piotr Mlocek
|
0797fefa44 | feat(gateway): add local-domain service routing (#1101) v0.0.40 | ||
|
alangou
|
2532687e34 |
fix(secret): Add custom derive Debug for SecretResolver to prevent secret leakage with {:?} (#1322)
Signed-off-by: Adrien Langou <alangou@nvidia.com> |
||
|
Seth Jennings
|
afcd3a9ec4 |
fix(cli): use OS trust store for reqwest TLS verification (#1342)
Switch reqwest from `rustls-tls` (bundled Mozilla webpki roots) to `rustls-tls-native-roots` (OS certificate store) so that the CLI trusts the same CAs as the rest of the system. This fixes OIDC discovery failures against Keycloak instances using certificates signed by internal CAs present in the system trust bundle. |
||
|
Miyoung Choi
|
8322e4fd00 |
docs: style fixes (#1341)
* docs: style fixes * docs: drop observability section overview page and rename a section title * docs: title updates |
||
|
Taylor MutchandDrew Newberry
|
ba77967fc4 |
refactor(docker): split gateway/supervisor Dockerfiles and use native rust builds (#1316)
* wip
* refactor(docker): use native rust builds for split gateway/supervisor images
Drop the in-Docker BUILD_FROM_SOURCE path so both images consume only
prebuilt binaries staged natively via tasks/scripts/stage-prebuilt-binaries.sh.
This mirrors what CI does and reuses the host's cargo target cache and
sccache across rebuilds.
- Dockerfile.gateway: nvcr.io/nvidia/distroless/cc:v4.0.4 base (the 4.0.0
tag does not exist on nvcr.io; the registry uses a v prefix). GNU-linked
binary copied to /usr/local/bin.
- Dockerfile.supervisor: scratch base, static musl binary. Static linkage
lets the image stay scratch while still being executable as a Kubernetes
init container.
- skaffold.yaml: each artifact invokes tasks/scripts/docker-build-image.sh,
which stages the binary natively (cargo / cargo-zigbuild) and then builds
the image. Drops the cross-build.sh dependency from the supervisor build.
- seccomp.rs: add a local SYS_kexec_file_load constant for musl/aarch64.
libc 0.2.185 omits the symbol from its musl/aarch64 bindings, so the
supervisor's seccomp filter previously failed to compile for that target.
- architecture/build.md: describe the native-first pipeline and per-image
runtime choices.
Local validation: gateway image 101MB (was 194MB), supervisor image 21.7MB.
helm:skaffold:run deploys cleanly; the static musl supervisor binary runs
correctly in a non-glibc agent container.
* refactor(docker): tighten binary perms via --chown + 0550
Replace `COPY --chmod=755` with `COPY --chown=<user> --chmod=0550` in
the gateway and supervisor Dockerfiles. The binary is no longer
world-readable or world-executable; ownership is pinned to the runtime
user.
- Gateway uses `--chown=nvs:nvs` + `USER nvs:nvs`, matching the only
non-root user defined in `nvcr.io/nvidia/distroless/cc` (UID 1000) and
the Helm chart's `securityContext.runAsUser: 1000`, which overrides
the Dockerfile USER at runtime.
- Supervisor uses numeric `--chown=65534:65534` because the scratch base
has no `/etc/passwd` for name resolution. The supervisor image is
only consumed by the init-container copy-self path; the destination
pod's runAsUser governs execute access.
Validated by deploying to a local k3d cluster via `helm:skaffold:run`
and confirming the gateway StatefulSet reaches 1/1 Running.
* ci(gpu): repoint GPU probe image lookup at Dockerfile.gateway
The previous awk parsed `FROM <image> AS gateway` from the now-deleted
`Dockerfile.images`. The new `Dockerfile.gateway` uses an ARG with a
default (`ARG GATEWAY_BASE_IMAGE=nvcr.io/nvidia/distroless/cc:v4.0.4`)
and `FROM ${GATEWAY_BASE_IMAGE} AS gateway`, so the old script returns
nothing.
Parse the ARG default value directly so the GPU prerequisites check
keeps using the gateway base image as a `nvidia-smi` probe target.
* ci(gpu): pin GPU probe to nvcr.io/nvidia/base/ubuntu:noble
The previous probe parsed the gateway base image out of the Dockerfile,
relying on the fact that the gateway ran on `nvcr.io/nvidia/base/ubuntu`
and that NVIDIA Container Toolkit CDI injection would populate
`nvidia-smi` and the supporting libs at runtime. The new gateway base
(`nvcr.io/nvidia/distroless/cc`) lacks `ldconfig`, a populated
`/usr/bin`, and the broader filesystem layout CDI injection assumes,
so it cannot serve as a GPU probe.
Pin the probe image explicitly to the NVIDIA-managed Ubuntu base. The
probe is independent of the gateway runtime and survives future base
swaps.
* fix(e2e-gpu): pass GPU probe image via env, drop Dockerfile.images parse
The Rust e2e test `gpu_request_for_each_discovered_device_matches_plain_container`
was still parsing the deleted `Dockerfile.images` to derive its GPU probe
image, panicking with `No such file or directory` after this PR's
Dockerfile split.
Move the probe image to a single source of truth in the workflow
(`OPENSHELL_E2E_GPU_PROBE_IMAGE` env at the job level) and require the
e2e test to read it from there. No silent codebase default — the test
panics with a pointer at the workflow if the env is missing, so it
fails loudly rather than drifting from CI.
The prereq probe step in the workflow now consumes the same env, so
the probe image is declared exactly once.
* fix(docker): keep supervisor binary root-owned for rootless Podman
The Podman driver mounts the supervisor image read-only into the sandbox
container at /opt/openshell/bin and runs that container as UID 0, but
deliberately drops DAC_OVERRIDE for hardening (container.rs:419). With
--chown=65534:65534 --chmod=0550 the binary was r-xr-x--- owned by UID
65534, so the container's UID 0 fell into "other" with no read or exec
access and the supervisor crashed on start (ContainerExited code 1).
Docker and Kubernetes both retain DAC_OVERRIDE, so root could still
exec the file — which is why this regression only surfaced in the
Podman e2e job.
Drop --chown from the supervisor COPY so the binary stays root-owned.
Keep --chmod=0550: the security win was dropping world-execute, not
changing the owner. The chown bought nothing here because the container
is always UID 0 regardless of driver, but it actively broke the only
driver that drops DAC_OVERRIDE.
---------
Co-authored-by: Drew Newberry <anewberry@nvidia.com>
|
||
|
Arnon Rotem-Gal-Oz
|
3b61c9cdcf |
feat(k8s): support nodeSelector and tolerations from platform_config (#1327)
Extract node_selector and tolerations from the sandbox template's platform_config and apply them to the pod spec. This allows callers to schedule sandbox pods on specific node pools and tolerate taints without requiring first-class proto fields. |
||
|
Eric Curtin
|
df5a8b943f |
fix(providers): read opencode config file during credential discovery (#1290)
Supplement env-var discovery with API keys stored in the opencode config file at $XDG_CONFIG_HOME/opencode/opencode.json. Keys under provider.<name>.options.apiKey are surfaced as <NAME>_API_KEY env vars; existing env vars take priority over file-sourced values.v0.0.39 |
||
|
Aaron Erickson 🦞andJohn Myers
|
9ea94b645d |
fix(sandbox): rewrite messaging credential placeholders (#1286)
* fix(sandbox): rewrite credential placeholders in websocket text frames Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): harden websocket credential rewrite Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * feat(sandbox): add websocket l7 inspection and compression Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): harden websocket upgrade validation Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * test(sandbox): cover route-selected websocket upgrades Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): harden websocket negotiation parsing Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * test(sandbox): add websocket conformance relay matrix Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * test(e2e): add websocket conformance lane Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(policy): support websocket incremental rules Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * feat(policy): enable websocket credential rewrite updates Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(cli): make websocket rewrite endpoint-local Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * feat(sandbox): support graphql websocket policy Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(policy): allow private IPs for websocket endpoints * feat(sandbox): rewrite REST credential placeholders Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * refactor(sandbox): generalize credential aliases Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): rewrite encoded form credentials Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(sandbox): close websocket policy and provider alias gaps Signed-off-by: Aaron Erickson <aerickson@nvidia.com> * fix(e2e): route websocket probe through host gateway * fix(e2e): stabilize websocket probe handshake * fix(e2e): exercise websocket probe through proxy * ci: remove websocket conformance workflow --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
Piotr Mlocek
|
5abc36c461 | feat(relay): route forwarding through ForwardTcp (#1029) | ||
|
Taylor MutchandDrew Newberry
|
3f0a0587c8 |
docs(rfc): add gateway configuration file RFC (#951)
* docs(rfc): Add gateway configuration RFC * Update RFC to handle config directories docs(rfc): address conf.d pattern and remove stale scratch branch reference Closes out open question #2 with a conf.d-style directory loader as a planned follow-on to the v1 single-file approach. Documents the three design decisions needed before implementation. Removes reference to scratch/server-config-file branch which does not exist. * docs(rfc): sync gateway config RFC with current repo state Resync RFC 0002 with the gateway code as it stands today: drop already-implemented framing, restructure the schema so each compute driver owns its [openshell.drivers.<name>] table, add the missing OIDC, metrics-listener, docker, and podman sections, and exclude database_url and ssh_handshake_secret from the file schema (env/CLI only). * docs(rfc): renumber gateway configuration RFC to 0003 RFC 0002 was claimed by the agent-driven policy management RFC that landed first. Move the gateway configuration proposal to the next available number. * Update rfc/0003-gateway-configuration/README.md Co-authored-by: Drew Newberry <anewberry@nvidia.com> --------- Co-authored-by: Drew Newberry <anewberry@nvidia.com> |
||
|
Florent BENOIT
|
b33bbd21cb |
fix(vm): correct /sandbox ownership when rootfs is built by non-root host (#1176)
When the VM driver extracts OCI image layers on a non-root host (e.g. macOS UID 501), fs::copy and tar::unpack create files owned by the host user. The sandbox user inside the VM then cannot write to its own home directory. Add a conditional chown in the VM init script that detects the mismatch and fixes ownership before the supervisor starts. The check is skipped when ownership is already correct (e.g. Linux-as-root extraction). Signed-off-by: Florent Benoit <fbenoit@redhat.com> |
||
|
Florent BENOIT
|
764d930680 |
fix(vm): use bash 3.2-safe empty array expansion in supervisor build script (#1311)
macOS ships bash 3.2, which treats empty array expansion ("${arr[@]}")
as an unbound variable error under set -u. Unlike set -e, this error
is not caught by if-constructs — it kills the shell immediately with
no output, causing mise run vm:supervisor to fail silently.
Use the ${arr[@]+"${arr[@]}"} idiom so the script works on both
bash 3.2 (macOS default) and bash 4+.
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
|
||
|
Mesut Oezdil
|
957daa0a0f |
docs(helm): document supervisor.sideloadMethod and sandboxNamespace default (#1309)
Add supervisor.sideloadMethod to the Kubernetes setup chart values table and the compute drivers reference table. The value was added in the ImageVolumeSource sideload PR and controls whether the supervisor binary is delivered via an OCI image volume mount or an init container, with auto-detection based on cluster version when left empty. Update the server.sandboxNamespace description in both pages to reflect that the Helm chart now derives it from the release namespace by default when the value is left empty. |
||
|
Mesut Oezdil
|
b9b8bc3ff2 |
fix(driver-kubernetes): propagate log_level as OPENSHELL_LOG_LEVEL env var (#1310)
The sandbox_to_k8s_spec function was inserting logLevel and environment fields at the Sandbox CR spec level. Neither field exists in the agents.x-k8s.io/v1alpha1 CRD schema, so Kubernetes silently dropped them and the log_level value never reached the sandbox pod. Remove the dead CR-level insertions. Build a merged environment map that includes OPENSHELL_LOG_LEVEL when log_level is set, and pass it to sandbox_template_to_k8s so the value flows through build_env_list into the pod container spec. Add a unit test verifying OPENSHELL_LOG_LEVEL appears in the container env and that logLevel is absent from the CR spec. Fixes #1265 |
||
|
Taylor Mutch
|
59475aabf4 |
ci(kubernetes): add kube gateway e2e tests and gated CI workflow (#1251)
* test(helm): Add kube gateway e2e tests Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * ci(helm): add Branch Helm E2E workflow gated on test:e2e-helm Adds a label-gated GitHub Actions workflow that exercises the Helm chart end-to-end against the Rust e2e suite via `mise run e2e:helm`. Pipeline: - pr_metadata gates on the `test:e2e-helm` label via the pr-gate action. - build-gateway / build-supervisor build and push Docker images using the reusable docker-build.yml workflow. - helm-e2e (bare runner): apt-installs z3 build deps so cargo can compile the openshell-policy crate's z3-sys backend, creates a kind cluster via helm/kind-action, materializes the kind kubeconfig at the path mise's [env] block expects, side-loads the freshly built gateway/supervisor images, applies deploy/kube/manifests/agent-sandbox.yaml so the sandboxes.agents.x-k8s.io CRD and reconciling StatefulSet are in place, and finally runs `mise run e2e:helm`. Also expands the `e2e:helm` task to run the full Rust e2e suite (matching `e2e:podman`) instead of only the smoke test, with OPENSHELL_E2E_KUBE_TEST as an opt-in single-test override for local debugging. Extends the e2e-label-help workflow so applying `test:e2e-helm` posts the next-step hint pointing at this workflow. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * test(helm-e2e): run host-gateway tests on the kube driver Six tests previously skipped on the kubernetes driver — three in host_gateway_alias.rs plus the forward_proxy_l7 + graphql_l7 cases — relied on `host.openshell.internal` reaching either a host process or a sibling docker container. The Helm chart already supports this via `server.hostGatewayIP`, but the e2e wrapper never set it. - with-kube-gateway.sh: auto-detect the host-routable IP (CoreDNS `host.k3d.internal` first to handle Docker Desktop, docker network gateway as a fallback for kind on Linux CI) and pass it to helm install. Also import locally-built images into existing k3d clusters and wait for namespace deletion to complete before exit. - e2e/rust/Cargo.toml + e2e-helm.sh: add an `e2e-host-gateway` feature that gates the three test files; docker and podman runs imply it, helm runs opt in by default (overridable via OPENSHELL_E2E_HELM_FEATURES for remote clusters where the test host is unreachable from pods). - Drop the `skip_if_kube` helper and its callers — feature gating now decides whether the tests compile in. Verified against a local k3d cluster: all six previously-skipped tests pass, smoke regression intact. * refactor(e2e): rename e2e:helm to e2e:kubernetes The runner exercises the gateway on Kubernetes; helm is just the deployment mechanism. Names now describe the target environment. - mise task: `e2e:helm` -> `e2e:kubernetes` - script: e2e/rust/e2e-helm.sh -> e2e/rust/e2e-kubernetes.sh - env var: OPENSHELL_E2E_HELM_FEATURES -> OPENSHELL_E2E_KUBERNETES_FEATURES - workflow: branch-helm-e2e.yml -> branch-kubernetes-e2e.yml (display name "Branch Kubernetes E2E", job "kubernetes-e2e") - PR gate label: test:e2e-helm -> test:e2e-kubernetes (e2e-label-help hint workflow updated to match) PR #1251 needs the new label applied; the old label can be removed. * fix(e2e): prefer IPv4 host-gateway IP from dual-stack docker networks kind dual-stacks its network; the wrapper's awk picked the first non-empty gateway, which was the IPv6 entry (fc00:...). Sandbox pods can't reach the test host's IPv4 listener through that, so the L7 and host-gateway-alias tests failed in CI even though they passed locally against k3d (where host.k3d.internal in CoreDNS short-circuits the docker-network fallback). Restrict the awk filter to IPv4 octets. --------- Signed-off-by: Taylor Mutch <taylormutch@gmail.com> |
||
|
Mrunal Patel
|
6184d24ea1 |
feat(k8s): support ImageVolumeSource for supervisor sideload (#1300)
* feat(k8s): support ImageVolumeSource for supervisor sideload Add a config-driven switch between two supervisor binary delivery methods: image-volume (default) mounts the supervisor OCI image directly as a read-only volume (K8s >= v1.33), while init-container preserves the existing emptyDir + copy-self pattern for older clusters. Configurable via --supervisor-sideload-method CLI arg, OPENSHELL_SUPERVISOR_SIDELOAD_METHOD env var, or Helm supervisor.sideloadMethod value. * feat(helm): auto-detect supervisor sideload method from cluster version Use Helm .Capabilities.KubeVersion to choose the supervisor sideload method automatically: image-volume on K8s >= v1.33, init-container on older clusters. An explicit supervisor.sideloadMethod value overrides auto-detection. * fix(helm): raise auto-detect threshold to K8s v1.35 The ImageVolume feature gate is only enabled by default starting in K8s v1.35, not v1.33 (where it is beta but off by default). Clusters on v1.33-v1.34 can still opt in with an explicit sideloadMethod value. |