feat(server): add auto-detection of compute driver at startup - #1
Closed
sjenning wants to merge 89 commits into
Closed
feat(server): add auto-detection of compute driver at startup#1sjenning wants to merge 89 commits into
sjenning wants to merge 89 commits into
Conversation
) `git describe --tags --long --match "v*"` matches the `vm-dev` tag alongside release tags. When `vm-dev` sits on or past the latest release tag in the commit graph (currently the case on main), git picks it, and the resulting `vm-dev-N-gSHA` string strips the leading `v` down to `m-dev-N-gSHA`. With `commits == 0` that's returned verbatim, producing a binary that reports itself as `m-dev`: $ openshell --version openshell m-dev Confirmed on v0.0.28 (NVIDIA#832) and reproduced on v0.0.29 as well. Restrict the glob to numeric release tags (`v[0-9]*`). All release tags follow `v\d+\.\d+\.\d+`, so this loses no valid version — it only filters out `vm-dev`, `vm-prod`, and any similar non-release tags that share the `v` prefix. Verified locally on this repo's current HEAD: # before $ git describe --tags --long --match 'v*' vm-dev-0-g355d845 # after $ git describe --tags --long --match 'v[0-9]*' v0.0.29-2-g355d845 Closes NVIDIA#832 Signed-off-by: mjamiv <michael.commack@gmail.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
…ns (NVIDIA#862) The proxy's upstream TLS client only trusted Mozilla root CAs (webpki-roots), which prevented TLS termination from working with internal/corporate hosts using private CA certificates. Load system CA certificates from the container's trust store (e.g. /etc/ssl/certs/ca-certificates.crt) in addition to webpki-roots. This allows custom sandbox images to include corporate CAs via update-ca-certificates. Signed-off-by: Matthias Osswald <mat.osswald@sap.com>
Each RFC now lives in its own folder (rfc/NNNN-short-name/) with README.md as the main proposal. This gives each RFC room for diagrams, images, and other supporting files without cluttering the top level. Move 0000-template into the new layout and update rfc/README.md to describe the convention.
…VIDIA#869) * fix(sandbox): block AF_NETLINK in seccomp unconditionally Move AF_NETLINK to the unconditional socket-domain block list alongside AF_PACKET, AF_BLUETOOTH, and AF_VSOCK. Previously it was only blocked in NetworkMode::Block, leaving it accessible in Proxy mode where network namespace isolation already scopes netlink to the sandbox's own veth — making this a defense-in-depth hardening rather than a live exposure. Closes OS-94 * fix(sandbox): scope inference.local interception to port 443 The pre-OPA interception for inference.local matched on hostname alone, allowing any port to bypass OPA policy evaluation — including under deny-all (network_policies: {}). Add a port check so only port 443 takes the interception path; all other ports on inference.local now fall through to OPA and are subject to normal policy evaluation. Closes OS-95 * fix(sandbox): enforce RLIMIT_NPROC to prevent fork bomb DoS Set a hard limit of 512 processes per UID in harden_child_process(), applied before privilege drop so the sandbox user cannot raise it. Prevents unrestricted fork() from exhausting the process table — most relevant for local dev mode where K8s pod cgroup pids.max is absent. Closes OS-96 * chore(sandbox): fix rustfmt formatting for seccomp blocked_domains --------- Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
* docs: fix TOC structure * docs: fix wrong section title * remove Home * fix(fern): properly add the landing page
…es (NVIDIA#868) * docs: refresh user-facing docs for recent sandbox and inference changes - architecture: document system CA loading for upstream TLS, `tls: skip` as the opt-out, gateway state persistence across restarts, and OCSF structured logging surface. - inference: document per-provider header allowlist, Authorization stripping, 120s streaming idle tolerance, and extended-thinking timeout guidance. - manage-sandboxes: add "Execute a Command in a Sandbox" section for `openshell sandbox exec` with flag reference. - security best practices: expand seccomp denylist (unconditional and conditional blocks), document two-phase Landlock probe, High-severity `landlock-unavailable` finding, and inference keep-alive closure. - observability logging: document port in HTTP log URLs, `[reason:...]` denial suffixes, proxy 403/502 JSON error bodies, and Landlock CONFIG:ENABLED/CONFIG:OTHER events. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> * docs(architecture): tighten high-level architecture page Trim implementation detail (CA bundle paths, deprecated TLS keys, SSH handshake secret) from the high-level architecture page, fix accuracy issues surfaced during deep audit, and expand uncommon acronyms on first mention. - Drop unsupported "cost-based routing" claim from Privacy Router row. - Replace "brokers requests across the platform" with auth-boundary description. - Add "inference" to Policy Engine constraint list per AGENTS.md. - Expand Deny rule to include SSRF, blocked control-plane port, and L7 deny paths in addition to deny-by-default. - Switch Allow/Deny labels from hyphen to colon; remove em dashes and a double space. - Expand LLM, SSRF, L7, TLS, CA, PEM, SSH, OCSF, and JSONL on first use. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made-with: Cursor * docs: address audit feedback on refresh PR - observability/logging: rewrite the allowed_ips paragraph after the Denial Reasons table; the previous wording said authors "can use" invalid entries while also stating they were rejected, which was contradictory and conflated load-time validation with the runtime per-CONNECT denial phrases the section documents. - about/architecture: split compound sentences in the new Gateway Lifecycle and Observability sections so each clause stands alone. - inference/about: drop the streaming-tolerance sentence from the prose paragraph since the dedicated Streaming reliability table row already covers it. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made-with: Cursor * docs(architecture): address review feedback on architecture page - Drop the Privacy Router row from the Components table; it is not yet a separately exposed customer-facing component. - Update the page description and intro count to match the remaining three components (gateway, sandbox, policy engine). - Split the policy decision into the three modes that the engine actually implements: Explicit Deny (deny rules and hardening rules, takes precedence), Allow, and Implicit Deny (no rule matched). Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made-with: Cursor * docs(architecture): convert policy decisions to a table Promote the three policy decisions (Explicit Deny, Allow, Implicit Deny) to a top-level table with Decision, When it applies, and Outcome columns instead of a nested bulleted list under list item 5. Top-level tables render reliably across markdown renderers, where nested-in-list tables do not. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made-with: Cursor * docs: repharse a bit --------- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
… paths (NVIDIA#844) * fix(sandbox): strip " (deleted)" suffix from unlinked /proc/<pid>/exe paths When a running binary is unlinked from its filesystem path — the common case is a `docker cp` hot-swap of `/opt/openshell/bin/openshell-sandbox` during the `cluster-deploy-fast` dev upgrade workflow — the Linux kernel appends the literal string ` (deleted)` to the `/proc/<pid>/exe` readlink target. The tainted `PathBuf` then flows into `collect_ancestor_binaries` and on into `BinaryIdentityCache::verify_or_cache`, which tries to `std::fs::metadata` the path. `stat()` fails with `ENOENT` because the literal suffix isn't a real filesystem path, and the CONNECT proxy denies every outbound request with: ancestor integrity check failed for \ /opt/openshell/bin/openshell-sandbox (deleted): \ Failed to stat ...: No such file or directory (os error 2) Reproduced in production 2026-04-15: a cluster-deploy-fast-style hot-swap of the supervisor binary caused every pod whose PID 1 held the now-deleted inode to deny ALL outbound CONNECTs (slack.com, registry.npmjs.org, 169.254.169.254, etc.), breaking Slack REST delivery, npm installs, and IMDS probes simultaneously. Existing pre-hot-swap TCP tunnels (e.g. Slack Socket Mode WSS) kept working because they never re-evaluate the proxy. Strip the suffix in `binary_path()` so downstream callers see the clean, grep-friendly path. This aligns the cache key and log messages with the original on-disk location. Note: stripping the suffix does NOT by itself make the identity cache tolerant of a legitimate binary replacement — `verify_or_cache` will now `stat` and hash whatever currently lives at the stripped path, which is the NEW binary, and surface a clearer `Binary integrity violation` error. Fully unblocking the cluster-deploy-fast hot-swap workflow needs a follow-up that either (a) reads running-binary content from `/proc/<pid>/exe` directly via `File::open` (procfs resolves this to the live in-memory executable even when the original inode has been unlinked), or (b) keys the identity cache by exec dev+inode instead of path. Happy to send that as a separate PR once the approach is decided — filing this narrow fix first because it stands on its own: it fixes a concrete misleading error and unblocks the obvious next step. Added `binary_path_strips_deleted_suffix` test that copies `/bin/sleep` to a temp path, spawns a child from it, unlinks the temp binary, verifies the raw readlink contains the ` (deleted)` suffix, then asserts the public API returns the stripped path. Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(sandbox): narrow the " (deleted)" suffix strip and exercise it end-to-end Address review feedback from @johntmyers on the initial version of this PR: procfs::binary_path - Only strip the kernel's " (deleted)" suffix when stat() on the raw readlink target reports NotFound. A live executable whose basename literally ends with " (deleted)" is now returned unchanged instead of being silently truncated, which matters because identity.rs hashes whatever this function returns as a trust anchor. - Operate on raw bytes via OsStrExt, so filenames that are not valid UTF-8 still get exactly one suffix stripped. The previous strip_suffix-on-&str path skipped non-UTF-8 entirely and fell through to returning the tainted path. - Expand the doc comment to describe both guardrails. procfs tests - binary_path_preserves_live_deleted_basename: copy /bin/sleep to a live file literally named "sleepy (deleted)", spawn it, and assert that the returned path still ends with " (deleted)". - binary_path_strips_suffix_for_non_utf8_filename: exec a binary whose basename contains a 0xFF byte, unlink it, and assert that binary_path returns the stripped non-UTF-8 path. Writes the bytes with OpenOptions + sync_all + explicit drop so the write fd is fully released before exec() to avoid ETXTBSY under concurrent tests. proxy: extract resolve_process_identity helper - Pull the peer-resolution + TOFU verify + ancestor walk + cmdline collection block out of evaluate_opa_tcp into resolve_process_identity. - Introduce IdentityError which carries the deny reason along with whatever partial identity data was resolved before the failure so evaluate_opa_tcp can thread that into ConnectDecision unchanged. - evaluate_opa_tcp now calls the helper and proceeds straight to the OPA evaluate step; the surface visible to OPA and OCSF is unchanged. proxy: end-to-end regression test for the hot-swap contract - resolve_process_identity_surfaces_binary_integrity_violation_on_hot_swap stands up a real TcpListener, copies /bin/bash to a temp path, primes BinaryIdentityCache with that binary, spawns bash with a /dev/tcp one-liner that opens a real connection to the listener, and captures the peer's ephemeral port from accept(). - Simulates `docker cp` correctly: unlink the running binary (which persists via the child's exec mapping) and create a fresh file with different bytes at the same path. Writing in place is rejected by the kernel with ETXTBSY, so the old single-inode approach did not actually model the production failure mode. - Asserts the error returned by resolve_process_identity contains "Binary integrity violation" (from BinaryIdentityCache) and does NOT contain "Failed to stat" or "(deleted)" — the pre-PR-NVIDIA#844 failure mode. The binary field on the error is populated and is free of the tainted suffix. - Skips cleanly if /bin/bash is not installed. Child process is always reaped before the assertion block so a failure does not leak a sleeping process. --------- Signed-off-by: mjamiv <michael.commack@gmail.com>
* feat(policy): add incremental sandbox policy updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(policies): expand incremental update guidance Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(policy): audit incremental updates in gateway logs * docs(policy): quote glob specs in shell examples --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
…#876) The CLI and TUI build an SSH ProxyCommand string by interpolating fields from CreateSshSessionResponse into a shell command. Three fields - sandbox_id, token, and the assembled gateway_url (composed from gateway_scheme, gateway_host, gateway_port, connect_path) - were not passed through shell_escape, even though exe_command and gateway_name were. The resulting string is handed to `ssh -o ProxyCommand=...` which OpenSSH routes through `/bin/sh -c`, so a hostile or compromised gateway could inject shell metacharacters and execute arbitrary commands under the developer's workstation account. - Add build_proxy_command in openshell-core::forward that wraps every interpolated value in shell_escape. Use it at all four ProxyCommand sites: openshell-cli/src/ssh.rs (ssh_session_config) and the three TUI sites in openshell-tui/src/lib.rs (shell connect, sandbox exec, port-forward reconciliation). - Add validate_ssh_session_response in openshell-core::forward, called at each site immediately after response.into_inner(). Enforces a conservative charset per field (sandbox_id, token, gateway_host, gateway_scheme, gateway_port, connect_path, host_key_fingerprint) so malformed responses fail loudly at the gRPC trust boundary before shell escaping is attempted. Belt-and-suspenders with the escape. - Harden render_ssh_config so `gateway` and `name` are shell-escaped even though validate_gateway_name currently gates `gateway`. - Document the charset contract on CreateSshSessionResponse in proto/openshell.proto: servers must uphold these rules and clients reject responses that violate them. - Add regression unit tests covering adversarial values in every field and a build_proxy_command test asserting no shell metacharacter remains outside single-quoted regions. Tracks OS-100.
* fix(sandbox): apply supervisor seccomp prelude Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(sandbox): delay supervisor prelude until bootstrap --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
…#880) * feat(cli): Allow printing only the policy when retrieving sandbox info * feat(cli): Indicate policy source and revision when retrieving sandboxes * cargo fmt
…uation (NVIDIA#878) * fix(sandbox): canonicalize HTTP request-targets before L7 policy evaluation The L7 REST proxy evaluated OPA path rules against the raw request-target and forwarded the raw header block byte-for-byte to the upstream. The upstream normalized `..`, `%2e%2e`, and `//` before dispatching, so a compromised agent inside the sandbox could bypass any path-based allow rule by prefixing the blocked path with an allowed one (e.g. `GET /public/../secret` matches `/public/**` at the proxy but the upstream serves `/secret`). The inference-routing detection had the same normalization gap via `normalize_inference_path`, which only stripped scheme+authority and preserved dot-segments verbatim. - Add `crates/openshell-sandbox/src/l7/path.rs` with `canonicalize_request_target`. Percent-decodes (with uppercase hex re-emission), resolves `.` / `..` segments per RFC 3986 5.2.4, collapses doubled slashes, strips trailing `;params`, rejects fragments / raw non-ASCII / control bytes / encoded slashes (unless explicitly enabled per-endpoint), and returns a `rewritten` flag so callers know when to rewrite the outbound request line. - Wire the canonicalizer into `rest.rs::parse_http_request`. The canonical path is the `target` on `L7Request` that OPA sees, and when the canonical form differs from the raw input the request line is rebuilt in `raw_header` so the upstream dispatches on the exact bytes the policy evaluated. - Canonicalize the forward (plain HTTP proxy) path at `proxy.rs`'s second L7 evaluation site. A non-canonical request-target is rejected with 400 Bad Request and an OCSF `NetworkActivity` Fail event rather than silently passed through. - Replace the old `normalize_inference_path` body with a call to the canonicalizer. On canonicalization error the raw path is returned (so inference detection simply misses and the normal forward path handles and rejects). - Document the invariant in `sandbox-policy.rego`: `input.request.path` is pre-canonicalized so rules must not attempt defensive matching against `..` or `%2e%2e`. - Architecture doc (`architecture/sandbox.md`) lists the new module. - 24 new unit tests cover dot segments, percent-encoded dot segments, double-slash collapse, encoded-slash reject/opt-in, null/control byte rejection, legitimate percent-encoded bytes round-tripping, mixed-case percent normalization, fragment rejection, absolute-form stripping, empty / length-bounded / non-ASCII handling, and regression payloads for the specific bypasses above. Tracks OS-99. * fix(sandbox): close forward-proxy parser-differential and wire allow_encoded_slash Addresses two review findings on the L7 path canonicalization change. 1. `handle_forward_proxy` previously evaluated OPA on the canonical path but wrote the raw path to the upstream via `rewrite_forward_request`, leaving the parser-differential open for plain-HTTP forward-proxy traffic even though it was closed for the L7 tunnel path. `path` is now reassigned to the canonical form inside the L7 block before the outbound write, so the bytes dispatched to the upstream match what OPA evaluated. Covered by a new regression test against `rewrite_forward_request`. 2. `allow_encoded_slash` is now wired through the YAML policy schema, the proto `NetworkEndpoint` message, the Rego endpoint config, and the `L7EndpointConfig` → `RestProvider` canonicalization options. Operators can opt in per-endpoint for upstreams like GitLab that embed `%2F` in namespaced resource paths; the default remains strict. The `RestProvider` gains a constructor `RestProvider::with_options` so `relay_rest` constructs a provider with per-endpoint options while `relay_passthrough_with_credentials` retains strict defaults. Integration tests: - `parse_http_request_canonicalizes_target_and_rewrites_raw_header` drives a non-canonical request through the parser and asserts both `L7Request.target` (what OPA evaluates) and `raw_header` (what the upstream receives) are canonical. - `parse_http_request_canonicalization_preserves_query_string`, `parse_http_request_leaves_canonical_input_byte_for_byte`, `parse_http_request_rejects_traversal_above_root`, `parse_http_request_preserves_http_10_version_on_rewrite`. - `parse_http_request_accepts_encoded_slash_when_endpoint_opts_in` / `_rejects_encoded_slash_by_default` verify the `allow_encoded_slash` gate. - `test_rewrite_forward_request_uses_canonical_path_on_the_wire` asserts the fix to the forward-proxy flow. - `parse_l7_config_allow_encoded_slash_{defaults_false,opt_in}` verify the YAML/Rego wiring.
…gRPC-multiplexed relay (NVIDIA#867)
…IDIA#895) Wrap gRPC and HTTP services with TraceLayer to log method, path, status, and latency for every request. Health/readiness probe endpoints are logged at debug level to reduce noise.
…NVIDIA#903) Move /health, /healthz, and /readyz to a dedicated plaintext HTTP port (default 8081) so Kubernetes probes work without mTLS client certificates. - Add health_bind_address to Config with --health-port CLI arg - Spawn standalone axum::serve for health_router on the health port - Remove health routes from the main multiplexed HTTP router - Update Helm statefulset probes from tcpSocket to httpGet on health port - Fix cluster-healthcheck.sh to open/close TCP without sending data, avoiding InvalidContentType TLS errors in the gateway log
…atches (NVIDIA#907) The 30s read_timeout on the shared kube client was killing the long-lived watch streams during idle periods, causing a reconnect cycle every 30 seconds. Use a separate client with no read_timeout for watch_sandboxes so the streams stay open indefinitely.
…rd (NVIDIA#914) the transfer of very large sandbox images to containerd can timeout depending on the size of the image and the speed of the local host. Made-with: Cursor
Closes NVIDIA#905 Skip proxy baseline read-write enrichment when a path is already explicitly configured as read-only, add regression coverage for the proto and local policy code paths, and document the precedence rule in the sandbox architecture docs. Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Closes NVIDIA#879 Resolve policy hostnames against the sandbox's /etc/hosts before DNS so Kubernetes hostAliases are visible to the proxy while keeping the existing allowed_ips SSRF enforcement semantics intact. Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
NVIDIA#918) Ubuntu Noble's git links against libcurl-gnutls, which does not read SSL_CERT_FILE. Without GIT_SSL_CAINFO, `git clone` over HTTPS fails inside every community sandbox image with "server certificate verification failed". Add GIT_SSL_CAINFO to tls_env_vars pointing at the same combined CA bundle already used by CURL_CA_BUNDLE / REQUESTS_CA_BUNDLE. Fixes NVIDIA#790 Fixes NVIDIA/NemoClaw#2270 Fixes NVIDIA/NemoClaw#1828 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Change sandbox user UID from 1000 to 1000660000 in custom image examples and E2E tests. Using a high UID range (1000000000+) prevents conflicts with host users when running without user namespace remapping, where container UIDs map directly to host UIDs. This resolves fork failures caused by RLIMIT_NPROC enforcement when the host user already has many threads running. Signed-off-by: Derek Carr <decarr@redhat.com>
…NVIDIA#986) Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* docs(release-notes): refresh weekly documentation Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> * docs(release-notes): remove weekly refresh entry Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> --------- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Signed-off-by: John Myers <johntmyers@users.noreply.github.com> Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
The default socket path assumed Linux conventions ($XDG_RUNTIME_DIR or
/run/user/{uid}), which do not exist on macOS. Use the podman machine
socket at $HOME/.local/share/containers/podman/machine/podman.sock
on macOS and show a targeted "podman machine start" hint when missing.
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* chore(tools): sync mise.lock after upgrade * chore(tools): Update mise min_version and version in CI dockerfile
… 4a) (NVIDIA#973) Builds openshell-gateway and openshell-sandbox natively per-arch on the nv-gha-runners shared CPU pool with a GHA-backed sccache, and uploads the resulting binaries as artifacts shaped for Dockerfile.images' BINARY_SOURCE=prebuilt path (added in NVIDIA#945). Dispatch manually 4-5 times after merge to collect cold + warm numbers and compare against the Rust portion of docker-build.yml's ARC baseline. PR 4c will wire these artifacts into the real pipeline once numbers land. Refs OS-128. Signed-off-by: Jonas Toelke <jtoelke@nvidia.com>
* refactor(server): unify policy persistence in objects table Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(server): clean sandbox-owned records on reconcile delete * refactor(server): use protos for stored policy records * refactor(server): move policy persistence into policy_store * fix(server): restore compute runtime merge compatibility * fix(server): validate draft chunk sandbox ownership --------- Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
Signed-off-by: John Myers <johntmyers@users.noreply.github.com> Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
…VIDIA#1032) * fix(net): catch IPv4-mapped blocked ranges in is_always_blocked_net The IPv6 branch only checked whether the network address itself mapped to a blocked IPv4 address. A broader prefix like ::ffff:168.0.0.0/103 has a public network address but spans ::ffff:169.254.0.0, so the old code accepted it at policy load time while is_always_blocked_ip silently rejected every connection at runtime. Add three containment checks for the IPv4-mapped loopback, link-local, and unspecified representatives. The existing network-address check is kept because it handles single-host entries (/128) whose network address is already in a blocked range. Five new tests cover: single-host loopback and link-local mapped addresses, broad prefixes that span each blocked range without starting there, and a public single-host address that must not be blocked. * fix(net): address clippy warnings in is_always_blocked_net Use Ipv4Addr::LOCALHOST instead of Ipv4Addr::new(127, 0, 0, 1) and collapse the nested if let / if into is_some_and.
add support on both side (rust and python) fixes NVIDIA#936 Signed-off-by: Florent Benoit <fbenoit@redhat.com>
* Adding qemu vm driver support with GPU pass-through Signed-off-by: Vincent Caux-Brisebois <vcauxbrisebo@nvidia.com> * Add GPU rootfs variant, harden VFIO binding, and fix networking and supervisor reliability issues discovered during GPU VM bring-up. Signed-off-by: Vincent Caux-Brisebois <vcauxbrisebo@nvidia.com> --------- Signed-off-by: Vincent Caux-Brisebois <vcauxbrisebo@nvidia.com>
Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
* ci(docker): use prebuilt Rust binaries by default Flip Docker image builds to consume staged native Rust artifacts, remove in-Docker Rust build stages, and publish per-arch images with a manifest merge. Add local staging support for prebuilt gateway and sandbox binaries so development image builds continue to work without CI artifacts. Signed-off-by: Jonas Toelke <jtoelke@nvidia.com> * ci(docker): address prebuilt build review feedback * ci(rust): allow existing vfio complexity * ci(rust): pin toolchain to 1.95 --------- Signed-off-by: Jonas Toelke <jtoelke@nvidia.com>
Signed-off-by: Alexander Watson <zredlined@gmail.com>
Signed-off-by: Jonas Toelke <jtoelke@nvidia.com>
…d permissions (NVIDIA#935) * feat(auth): add OIDC/Keycloak authentication with RBAC Add OAuth2/OIDC authentication to the gateway server with role-based access control, CLI login flows, and full deployment plumbing. Server: JWT validation against configurable OIDC issuer (oidc.rs), JWKS key caching with TTL and rotation handling, method classification (unauthenticated/sandbox-secret/dual-auth/bearer), identity extraction with provider-agnostic Identity type, and RBAC enforcement via AuthzPolicy with configurable admin/user roles and auth-only mode. CLI: browser-based Authorization Code + PKCE flow, Client Credentials flow for CI/automation, token storage with refresh, gateway add/login/ logout commands, OIDC bearer token injection over mTLS transport, discovery endpoint for auto-configuration. Security: sandbox-secret scope restriction on UpdateConfig (policy sync only), anti-spoofing header stripping, dual-auth fallthrough from sandbox-secret to Bearer token. Deployment: OIDC config wired through DeployOptions, Docker env vars, Helm values/templates, HelmChart manifest, cluster-entrypoint.sh, and bootstrap scripts. Keycloak dev server script with pre-configured realm (test users, roles, PKCE client, CI client). Tested with Keycloak. The roles claim path and role names are configurable to support other OIDC providers. * feat(auth): add OAuth2 scope-based fine-grained permissions Add opt-in scope enforcement on top of existing OIDC role-based access control. When --oidc-scopes-claim is set, the server extracts scopes from the JWT and checks them per-method against an exhaustive scope map. Scopes: sandbox:read, sandbox:write, provider:read, provider:write, config:read, config:write, inference:read, inference:write, and openshell:all (wildcard). Methods not in the scope map require openshell:all. Scopes layer on top of roles and cannot escalate privilege. Auth-only mode (empty role names) still enforces scopes when enabled. Server: scopes_claim in OidcConfig, scope extraction from JWT (space-delimited and JSON array formats), standard OIDC scope filtering, scope check in AuthzPolicy after role check. CLI: --oidc-scopes on gateway add/start stored in metadata and consumed by gateway login, --oidc-scopes-claim on gateway start forwarded to server, scopes parameter in browser and client credentials OAuth2 flows with openid deduplication. Deployment: oidc_scopes_claim wired through DeployOptions, docker.rs, Helm, bootstrap scripts, and cluster entrypoint. Keycloak: realm config updated with built-in OIDC scopes and 9 OpenShell client scopes as optional on openshell-cli and openshell:all as default on openshell-ci. * fix(auth): address branch review findings Add GetInferenceBundle to sandbox-secret methods so sandbox inference route refresh works under OIDC. Make GetSandboxConfig dual-auth so CLI users can read sandbox settings with Bearer tokens. Preserve OIDC gateway metadata on restart — a bare gateway start without --oidc-* flags no longer erases the stored OIDC registration. Document CI client ID requirement (openshell-ci vs openshell-cli) in the testing guide. Add security note about auth-only mode blast radius for GitHub Actions. * fix(auth): complete review findings for OIDC auth boundary Move OpenShell/GetSandboxConfig from sandbox-secret-only to dual-auth so CLI users can read sandbox settings with Bearer tokens while sandbox supervisors continue using the shared secret. Add sandbox secret interceptor to the inference bundle fetch path so GetInferenceBundle works under OIDC-enabled gateways. Extract shared interceptor constructor to avoid duplication. Add GetSandboxConfig to the config:read scope map so scope enforcement applies consistently when scopes are enabled. Refactor OIDC metadata preservation into apply_oidc_gateway_metadata() with explicit resume semantics — only preserve existing OIDC metadata on real resume paths, not on fresh deployments. Update architecture docs and testing guide to reflect the corrected method classifications and add new test coverage for interceptor injection, scope requirements, metadata preservation, and dual-auth classification. * refactor(auth): use oauth2 crate for CLI OIDC flows Replace hand-written PKCE generation, authorization URL construction, token exchange, client credentials, and token refresh with the oauth2 crate's typed API. Eliminates sha2, hex, and getrandom dependencies from the CLI. The custom urlencoded() helper and manual form POST logic are replaced by BasicClient methods with proper type-state safety. Discovery and the callback server remain custom since the oauth2 crate does not provide OIDC discovery or a localhost redirect listener. * refactor(auth): move server auth modules into auth/ directory Group oidc.rs, authz.rs, identity.rs, and the auth HTTP endpoints under src/auth/ module directory. No behavioral changes. auth/mod.rs — module root, re-exports HTTP router auth/oidc.rs — JWT validation, JWKS caching, method classification auth/authz.rs — role and scope authorization policy auth/identity.rs — provider-agnostic Identity type auth/http.rs — /auth/connect and /auth/oidc-config endpoints * fix(auth): use RequestBody auth type for client credentials flow The oauth2 crate defaults to BasicAuth (HTTP Basic header) but Keycloak and most OIDC providers expect client_secret_post (credentials in the request body). Set AuthType::RequestBody explicitly to match the pre-refactor behavior. Also re-export Identity, IdentityProvider, and JwksCache from the auth module so ServerState's public API remains nameable by external consumers. * fix(auth): forward OPENSHELL_OIDC_SCOPES through cluster bootstrap Pass --oidc-scopes to gateway start so the metadata includes requested scopes after cluster bootstrap. Without this, users had to manually edit metadata.json to set scopes for gateway login. Usage: OPENSHELL_OIDC_SCOPES="openshell:all" mise run cluster * test(auth): add OIDC e2e tests for RBAC, scopes, and client credentials Add 10 end-to-end tests covering OIDC authentication against a live K3s cluster with Keycloak: RBAC (5 tests): admin can create providers, user cannot, user can list sandboxes, unauthenticated requests rejected, health probe works without auth. Scopes (4 tests): sandbox-scoped token can list sandboxes but not providers, openshell:all grants full access, no-scopes token denied. Client credentials (1 test): CI token via client_credentials grant. Tests are opt-in via OPENSHELL_E2E_OIDC=1 and OPENSHELL_E2E_OIDC_SCOPES=1 env vars. They derive the Keycloak URL from gateway metadata to match the server's configured issuer. Run with: OPENSHELL_E2E_OIDC=1 OPENSHELL_E2E_OIDC_SCOPES=1 \ PYTHONPATH=python uv run pytest e2e/python/oidc/ -v * fix(docs): fix markdown lint errors in OIDC architecture docs Add blank lines before lists and fenced code blocks to satisfy markdownlint MD031 and MD032 rules.
When no drivers are explicitly configured, the server now automatically detects the appropriate compute driver by checking the runtime environment: - Kubernetes: detected via KUBERNETES_SERVICE_HOST env var (set inside pods) - Podman: detected by checking if podman binary is available on PATH - Docker: detected by checking if docker binary is available on PATH Priority order: Kubernetes → Podman → Docker. VM is never auto-detected and must be selected explicitly via --drivers vm. The Auto variant is internal-only and does not serialize to config files. The default --drivers value is now empty, triggering auto-detection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add automatic detection of the appropriate compute driver when no drivers are explicitly configured. The server now checks the runtime environment and selects Kubernetes, Podman, or Docker in priority order — eliminating the need for manual
--driversconfiguration in most deployments.Related Issue
None filed.
Changes
Autovariant toComputeDriverKindenum (internal-only, skips serialization)detect_driver()function with priority: Kubernetes → Podman → Dockeris_binary_available()helper usingCommand::new(name).arg("--version").output()default_compute_drivers()to return empty vec, triggering auto-detectionconfigured_compute_driver()to calldetect_driver()when config is emptydefault_value = "kubernetes"from--driversCLI flag--drivers vm)Testing
mise run rust:lintpasses (clippy + format + license headers)mise run rust:checkpasses (cargo check)cargo test -p openshell-core config::(7/7)cargo test -p openshell-server(10/10)Note: One pre-existing flaky integration test (
sandbox_create_keeps_sandbox_with_forwarding) fails due to port 8080 being occupied on this system — unrelated to these changes.Checklist