mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-15 05:09:44 +08:00
pull-request/2832
135
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 alangou
|
0d708d6d51 |
fix(policy): gate uninspected credentialed endpoints (#2493)
* fix(policy): gate uninspected credentialed endpoints Signed-off-by: Adrien Langou <alangou@nvidia.com> * refactor(cli): extract allowed-ip option parsing Signed-off-by: Adrien Langou <alangou@nvidia.com> * fix(policy): gate endpointless credential bindings Signed-off-by: Adrien Langou <alangou@nvidia.com> --------- Signed-off-by: Adrien Langou <alangou@nvidia.com> |
||
|
Mrunal Patel
|
8d67250a5d |
fix(providers): keep refresh credential handles stable (#2780)
* fix(providers): keep refresh credential handles stable Signed-off-by: Mrunal Patel <mrunalp@gmail.com> * fix(providers): protect refresh-owned credentials Signed-off-by: Mrunal Patel <mrunalp@gmail.com> --------- Signed-off-by: Mrunal Patel <mrunalp@gmail.com> |
||
|
Simon Scatton
|
88cf35efe4 |
fix(bazel): enable driver extraction in core (#2769)
Signed-off-by: Simon Scatton <sscatton@nvidia.com> |
||
|
Giuseppe Scrivano
|
d51a653f9c |
feat(driver-podman): add userns config (#2562)
* refactor(driver): extract shared supervisor binary helpers Move supervisor binary extraction, caching, and validation helpers from the Docker driver into openshell-core::driver_utils so both Docker and Podman drivers can reuse them. Moved helpers: extract_first_tar_entry, write_cache_binary_atomic, supervisor_cache_path, temp_extract_container_name, and validate_linux_elf_binary. The shared extract_first_tar_entry gains entry-type and empty-payload checks that the Docker-local version lacked. supervisor_cache_path takes a driver_subdir parameter so each driver caches under its own namespace (docker-supervisor vs podman-supervisor). Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> * feat(driver-podman): add userns config Add a `userns` option to the Podman compute driver that maps to Podman's user namespace modes. The mode string is split on the first colon into the API's `nsmode` and `value` fields so parameterized values like `auto:size=65536` and `keep-id:uid=1000,gid=1000` are forwarded correctly. When the mode is `auto`, the container spec also sets `idmappings.AutoUserNs = true` as required by the API. An allowlist validates the mode at startup: `auto` and `keep-id` accept optional parameters; `host`, `private`, and `nomap` reject them; everything else is an error. Podman image volumes use overlay mounts internally and the kernel does not support idmapped mounts on overlay (`mount_setattr` returns EINVAL). When userns is configured (any mode except `host`), the driver extracts the supervisor binary from the image to a host-side cache and bind-mounts it instead of using an image volume. Configurable via TOML `userns = "auto"`, CLI `--userns`, or environment variable `OPENSHELL_PODMAN_USERNS`. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --------- Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> |
||
|
Piotr Mlocek
|
44bf0df485 |
feat(middleware): inspect WebSocket text messages (#2477)
* feat(middleware): inspect websocket text messages Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): address websocket review feedback Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): bound websocket message assembly Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): harden websocket upgrade lifecycle Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(middleware): unify in-process and remote transports Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * feat(middleware): support regex websocket redaction Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): bound persistent streaming sessions Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): accept websocket sequence gaps Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(middleware): refine websocket introspection contract Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): clarify websocket preflight lifecycle Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): clarify websocket coverage semantics Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): type websocket frame failures Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): return 503 when middleware admission is exhausted Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): align streaming API contract Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): clarify WebSocket event result scope Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(rfc): simplify middleware revision history Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * feat(examples): add WebSocket content guard support Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): unify binding payload limits Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(middleware): align payload limit terminology Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): address websocket review feedback Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): address websocket review findings Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * test(network): allow Linux handler setup in preflight regression Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): harden websocket relay finalization Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(network): inspect compressed websocket messages Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * test(network): stabilize compressed websocket regressions Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(go-sdk): regenerate middleware protobuf binding Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): clarify websocket skip lifecycle Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(middleware): address WebSocket review feedback Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> |
||
|
Derek Carr
|
59479f492a |
feat(k8s): add namespace-per-workspace support (RFC 0011 Phase 3) (#2656)
* feat(k8s): add namespace-per-workspace support (RFC 0011 Phase 3) Implement three workspace namespace modes for the Kubernetes compute driver: shared (default, preserves current single-namespace behavior), managed (auto-creates/deletes namespaces per workspace), and operator (pre-provisioned namespaces with dynamic discovery via label selector or drop-in allowlist file). Key changes: - WorkspaceMode enum and namespace resolution in driver config - Managed namespace lifecycle with ServiceAccount and OpenShift SCC annotation propagation - Cluster-wide sandbox CR watchers for managed/operator modes - NamespaceValidator (Exact/Prefix/Allowlist) for SA token auth - Workspace-aware credential secret storage - Helm ClusterRole for multi-namespace RBAC - Gateway config, architecture, and reference docs Signed-off-by: Derek Carr <decarr@redhat.com> * test(k8s): add e2e tests for workspace namespace modes Add end-to-end tests for managed and operator workspace modes introduced in RFC 0011 Phase 3. The managed mode tests verify namespace creation with correct labels, ServiceAccount provisioning, sandbox CR placement, and namespace survival with remaining sandboxes. The operator mode tests verify rejection of unlabeled and nonexistent namespaces. The positive operator path (sandbox in labeled namespace) is known to fail due to an RBAC gap and will be addressed separately. Also fixes Helm 4 compatibility: move SPDX license headers inside conditional guards in 8 chart templates to prevent empty comment-only documents, and fix a trailing whitespace trimmer in clusterrole.yaml that concatenated the license header with apiVersion. Adds cleanup sweep in with-kube-gateway.sh to remove managed and operator namespaces before Helm uninstall, and mise tasks for running each mode independently. Signed-off-by: Derek Carr <decarr@redhat.com> * feat(k8s): add operator namespace label watcher Spawn a background kube::runtime::watcher in the K8s driver that watches namespaces matching the configured label selector and populates the OperatorNamespaceAllowlist at runtime. The driver owns the allowlist and exposes its Arc so the server can share the same set with the SA token authenticator. create_sandbox now gates pod creation on the allowlist in operator mode — workspaces whose namespace is not yet labeled are rejected at resource render time rather than silently proceeding. Workspace lifecycle itself is unaffected; only sandbox (resource) creation is gated. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): harden operator mode and address review findings Close the fail-open gap in operator mode when only operator_namespace_file is configured: the allowlist is now created unconditionally in operator mode (fail-closed from startup). Implement the namespace file watcher using the notify crate, following the TLS hot-reload pattern (parent-directory watch, 1s debounce, ConfigMap symlink-swap safe). The file format is a JSON array of namespace name strings. Additional fixes from the 10-reviewer audit: - Change allowlist rejection from InvalidArgument to FailedPrecondition so callers know the request may succeed later once the namespace is provisioned. - NamespaceValidator::Allowlist now holds the OperatorNamespaceAllowlist newtype instead of a raw Arc<RwLock<BTreeSet>>, eliminating silent denial on RwLock poison. - Verify LABEL_MANAGED_BY and LABEL_GATEWAY_ID ownership before deleting a managed namespace. - Replace fixed 5s sleep in operator e2e test with a 30s poll loop. - Add Helm validation for workspaceMode values. - Fix Helm README type column and description for operator fields. - Add insert/remove methods to OperatorNamespaceAllowlist; label watcher now uses them instead of reaching through shared(). - Reject configs with both operator_namespace_label and operator_namespace_file set. Signed-off-by: Derek Carr <decarr@redhat.com> * feat(k8s): add workspace-level compute driver RPCs and harden RBAC Decouple namespace lifecycle from sandbox lifecycle by adding EnsureWorkspace/DeleteWorkspace RPCs to the ComputeDriver service. Namespace creation now happens before credential storage and namespace deletion happens on workspace delete, fixing credential storage in managed workspace mode. - Add EnsureWorkspace and DeleteWorkspace proto RPCs with implementations across all compute drivers (K8s managed delegates to ensure_namespace/delete_namespace_if_empty; others no-op) - Wire ensure_workspace into provider create/update/refresh paths so the namespace exists before the credential driver writes secrets - Wire delete_workspace into workspace deletion for cleanup - Remove delete_namespace_if_empty from sandbox deletion path - Scope ClusterRole secrets access to non-shared workspace modes - Add TODO for TLS cert hot-reload in sandbox gRPC client - Harden e2e tests with control-plane sandbox resolution assertions - Fix docker image save --platform flag for OCI index manifests Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): address re-review findings and add test coverage - Use server-side apply for TLS secret sync (fixes second sandbox creation failure when TLS is enabled) - Scope gateway-ID label selector unconditionally across all workspace modes (fixes operator reads/watches/deletes seeing foreign sandboxes) - Validate operator allowlist in EnsureWorkspace and DeleteWorkspace RPCs (prevents credential writes to namespaces outside the allowlist) - Extend ClusterRole secrets patch+delete to all non-shared modes with credential driver enabled (fixes operator credential storage RBAC) - Validate namespace ownership on 409 conflict in ensure_namespace (prevents adopting unowned namespaces in managed mode) - Replace delete_namespace_if_empty with unconditional delete_namespace letting Kubernetes cascade cleanup (fixes stuck terminating CRs) - Strengthen NetworkPolicy TODO to cover both managed and operator modes - Extract selector and ownership logic into testable free functions - Add unit tests for gateway-ID selectors and namespace ownership - Add Helm ClusterRole RBAC tests for operator credential driver Signed-off-by: Derek Carr <decarr@redhat.com> * ci(k8s): add workspace managed and operator mode e2e to CI Wire the existing e2e:kubernetes:workspace-managed and e2e:kubernetes:workspace-operator mise tasks into the branch-e2e workflow so they run alongside the other core Kubernetes e2e suites. Both are gated by run_core_e2e and included in the Core E2E result gate. Signed-off-by: Derek Carr <decarr@redhat.com> * test(k8s): add e2e tests for workspace namespace modes Add 7 new e2e tests covering workspace namespace lifecycle, TLS secret copying, ownership conflict detection, DNS-1123 validation, operator namespace preservation, and dynamic label watcher behavior. Fix async sandbox deletion race condition in existing tests by polling sandbox list instead of asserting immediately after delete. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): grant secrets/patch unconditionally and backfill gateway-id labels Address two review findings: 1. RBAC: server-side apply (PATCH) is used for TLS secret sync in multi-namespace modes, but the ClusterRole only granted patch when the kubernetes-secrets credential driver was enabled. Grant patch unconditionally for non-shared modes since TLS sync always needs it; keep delete gated on the credential driver. 2. Upgrade safety: the new gateway-id label selector would orphan legacy Sandbox CRs that predate its introduction. Add a startup backfill in shared mode that patches any managed Sandbox CR missing the gateway-id label before the driver begins serving requests. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): address workspace namespace review findings Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): address follow-up review findings Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): preserve workspace lookup after rebase Signed-off-by: Derek Carr <decarr@redhat.com> * fix(helm): allow managed secret creation Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): stop pods in workspace namespace Signed-off-by: Derek Carr <decarr@redhat.com> * test(k8s): scope pod deletion check to v1alpha1 Signed-off-by: Derek Carr <decarr@redhat.com> * fix(k8s): address workspace namespace review findings Signed-off-by: Derek Carr <decarr@redhat.com> --------- Signed-off-by: Derek Carr <decarr@redhat.com> |
||
|
Piotr Mlocek
|
bdabb54cb3 |
fix(security): authenticate extension services (#2638)
* fix(security): authenticate extension services Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * feat(extension-core): verify gateway JWTs Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(extension-core): keep inbound verification external This should become an extension SDK package. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(security): harden the extension authentication contract Follow-up hardening on the alpha extension authentication mechanism. Claim contract: - Extension tokens carry an explicit `typ` of `openshell-ext+jwt`. They share a signing key with sandbox-to-gateway admission tokens and were otherwise separated by audience alone, so a verifier that neglects to check `aud` could accept a gateway credential. The header is a second, independent discriminator. - Publish OIDC-shaped discovery at `/.well-known/openid-configuration` so a service configured with only the gateway URL can learn the exact expected issuer and the JWKS location. It is shaped, not compliant: `issuer` is the gateway identity, not the serving URL. Audience agreement: - `MiddlewareManifest` and `InterceptorManifest` gain `expected_audience`. The audience is otherwise configured independently on each side of the boundary, where a mismatch surfaces only as an opaque authentication failure on every call. OpenShell now compares the two and fails at startup. An empty field keeps the check off for existing services. Compatibility: - Add `allow_insecure_transport` per registration. Enabling gateway JWT signing previously made any plaintext endpoint a hard startup failure, including the endpoint form used in our own documentation. The opt-out attaches no credential, is refused by the gateway if a supervisor asks for one, and warns at every startup. - Make the transport requirement kind-aware. A middleware endpoint must be reachable from every sandbox supervisor, so only interceptors may use a gateway-local Unix socket. Credential lifecycle: - Replace the process-global slot map with a supervisor-owned `ExtensionCredentialStore` shared explicitly across the gateway connections the supervisor opens, removing test-order coupling. - Rotate only when a credential is missing or has passed four fifths of its lifetime. Configuration polling ran every ten seconds against fifteen-minute credentials, so each poll re-ran gateway effective-policy resolution and re-minted the gateway token. - Bound credential minting per sandbox, since each request resolves the caller's effective policy. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs: record alpha extension authentication in RFC appendices Restore the RFC 0009 and 0010 bodies to their accepted text and move every extension-authentication update into appendices instead. An RFC records a decision at a point in time; superseding detail belongs alongside it rather than rewritten into it. RFC 0009's appendix carries the shared contract: claims, authorization, key distribution, the `allow_insecure_transport` replacement for the body's `allow_insecure`, and residual risks. RFC 0010's records only what differs for interceptors and links to it. The existing protocol-extensions appendix, which parked the phase 2 transport question, now points forward to what was built. Also document the audience handshake, the discovery endpoint, the `typ` requirement, and `jti` replay guidance in the extensibility and gateway configuration pages, and correct the middleware transport guidance: middleware endpoints must be reachable from sandbox supervisors, so Unix sockets are not an option there. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(extension-core): abstract extension server trust Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(core): update middleware manifest example Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(extension-auth): preserve unsigned gateway compatibility Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(extension-auth): reject cross-domain token replay Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> |
||
|
Jesse Jaggars
|
c4b500a7de |
feat(helm): cert-manager external issuer + OpenShift passthrough Route (#2468)
* fix(core): trust public root CAs alongside the sandbox mTLS CA The supervisor gRPC client only trusted the CA configured via OPENSHELL_TLS_CA, since tonic ClientTlsConfig starts with an empty root store unless with_native_roots()/with_webpki_roots() is also enabled. Deployments where the gateway server certificate is issued by a public CA (e.g. cert-manager against an ACME issuer) caused every supervisor connection to fail the TLS handshake with "UnknownCA", since the sandbox mTLS CA and the server cert issuer were no longer the same. Enable both native and webpki roots in addition to the configured CA. tonic root store is a union of all configured sources, so this does not weaken verification for existing self-signed deployments. webpki-roots (compiled in) is enabled alongside native-roots since the supervisor binary may run in minimal sandbox images without a populated system CA bundle. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * feat(helm): support external cert-manager issuers and OpenShift Route passthrough Add certManager.serverIssuerRef/clientIssuerRef so the gateway and mTLS client certificates can be issued by a real Issuer/ClusterIssuer (e.g. ACME) instead of only the chart built-in self-signed CA. Add openshiftRoute template for exposing the gateway via a TLS passthrough Route so the gateway keeps terminating its own TLS/mTLS. The server Certificate excludes internal-only SANs (cluster-local, localhost, loopback) when an external issuer is configured, since ACME issuers reject those per CA/Browser Forum baseline requirements. A template-time fail guard catches the misconfiguration at helm install time rather than asynchronously at cert-manager issuance time. Includes Helm unittest coverage for both issuerRef overrides and Route rendering, plus a CI values overlay for lint coverage. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * docs: document cert-manager external issuer and OpenShift Route Update managing-certificates.mdx with the serverIssuerRef workflow and install-time validation behavior. Add a production section to the OpenShift guide covering passthrough Route with a real certificate. Regenerate Helm README for new certManager and openshiftRoute values. Sync debug-openshell-cluster skill with new troubleshooting steps for ACME issuance failures and supervisor UnknownCA from mismatched CAs. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * fix(helm,core): address PR review feedback on cert-manager external issuer Addresses all five blocking review items from #2468: 1. Remove .with_native_roots() from supervisor gRPC client -- the supervisor runs inside the user-selected sandbox image, so the image CA bundle is not operator-controlled. Keep .with_webpki_roots() (compiled-in, not user-controlled) alongside the configured CA. 2. Fail at render time when serverIssuerRef.name is set but clientCaFromServerTlsSecret is still true. Add negative Helm test. 3. Remove clientIssuerRef -- changing only clientIssuerRef breaks both directions because trust bundles are not modeled separately. Change serverIssuerRef.kind default from ClusterIssuer to Issuer. 4. Add server.oidc.issuer and server.oidc.audience to the documented OpenShift production Helm command. Add Access Control prerequisite. 5. Fail at render time when openshiftRoute.enabled and disableTls are both true. Add negative Helm test. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * fix(drivers): strip GATEWAY_TLS_SERVER_NAME from Docker and Podman env Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * fix(helm,drivers): guard default clientCaSecretName and add env-strip tests Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> * feat(tls): SNI-based dual certificate for internal and external server TLS Split the gateway server certificate into two: an internal cert issued by the chart's own CA (for supervisor connections via cluster-local SANs) and an external cert issued by an operator-configured Issuer such as ACME/Let's Encrypt (for CLI and Route access via public SANs). The gateway uses SNI-based certificate selection: connections whose SNI hostname matches external_server_names receive the external cert; all others (including those with no SNI) receive the internal cert. Security improvement: remove .with_webpki_roots() from the supervisor gRPC client so supervisors trust only the chart CA, closing a MITM vector via publicly-trusted certificates in user-supplied container images. Key changes: - Add DualCertResolver with SNI-based cert selection and full test coverage - Add external_cert_path, external_key_path, external_server_names to TlsConfig - Validate partial external cert config (error on cert-without-key or vice versa) - Validate empty external_server_names when external cert is configured - Split cert-manager templates into internal + external Certificate resources - Add Helm guards for misconfigured external issuer (empty serverDnsNames, internal-only SANs with external issuer, conflicting clientCaFromServerTlsSecret) - Update gateway-config.mdx, managing-certificates.mdx, openshift.mdx docs - Update debug-openshell-cluster skill for dual-cert troubleshooting Signed-off-by: Pi Agent <agent@openshell.local> * fix(drivers): strip GATEWAY_TLS_SERVER_NAME in VM driver and correct comments Add the same GATEWAY_TLS_SERVER_NAME environment stripping to the VM compute driver that Docker, Podman, and Kubernetes drivers already perform. Without this, a sandbox user on the VM driver could override the TLS server name the supervisor verifies. Fix stale comments in Docker and Podman drivers that referenced 'with WebPKI roots trusted' — WebPKI roots are explicitly not trusted after the tls-webpki-roots removal. Use tls-ring instead of bare channel for tonic in openshell-core so the TLS API (ClientTlsConfig, Endpoint::tls_config) is available without pulling in any root certificate store. Signed-off-by: Pi Agent <agent@openshell.local> * fix(tls,helm): wildcard SNI matching and Route host validation Add RFC 6125 single-level wildcard matching to DualCertResolver so external_server_names entries like *.example.com correctly match SNI hostnames like gw.example.com. Previously only exact matches worked, silently falling back to the internal cert for wildcard configurations. Add a Helm fail guard in route.yaml that rejects openshiftRoute.host values not listed in certManager.serverDnsNames when an external issuer is configured — catches cert/route hostname mismatches at install time instead of at TLS connect time. Quote the host field in route.yaml for robustness. Signed-off-by: Pi Agent <agent@openshell.local> * fix(helm): address blocking review items — client-CA guard, wildcard Route, serverIssuerRef gate 1. Remove the obsolete guard rejecting serverIssuerRef + clientCaFromServerTlsSecret=true. The internal server certificate is always signed by the chart CA (the same CA that signs the client cert), so clientCaFromServerTlsSecret=true is correct — its filtered ca.crt is exactly the right trust anchor. The old workaround (mounting openshell-ca-tls directly) unnecessarily exposed the CA private key to the gateway container. Remove the client-CA overrides from docs, CI overlay, and production examples. 2. Route host validation now supports wildcard certificates per RFC 6125: single-level wildcards like *.example.com match gateway.example.com but not deep.sub.example.com. Require an explicit openshiftRoute.host when an external issuer is configured — without one, OpenShift generates a hostname absent from serverDnsNames. 3. Reject serverIssuerRef.name when certManager.enabled is false — the external certificate, its Secret mount, and the gateway TLS config all require cert-manager to be enabled. Validated on ROSA (dev.dyee.p3) with branch-built images: - Fresh install with letsencrypt-prod ClusterIssuer - SNI dual-cert: external hostname served Let's Encrypt cert - Supervisor mTLS via internal cert path: ConnectSupervisor accepted - Client CA volume: filtered ca.crt from internal server secret (no key) - CLI connected via Route + OIDC Helm tests: 81 pass across 7 suites. Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> --------- Signed-off-by: Jesse Jaggars <jjaggars@redhat.com> Signed-off-by: Pi Agent <agent@openshell.local> |
||
|
Seth Jennings
|
0f8fad23c4 |
feat(sandbox): add stop and start operations (#2653)
* feat(sandbox): add suspend and resume operations Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(server): preserve lifecycle work after cancellation Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(server): reconcile ambiguous lifecycle outcomes Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(server): complete suspended session cleanup Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(vm): preserve suspension state on resume failure Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(server): retry retained lifecycle transitions Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(server): clean sessions after suspend reconciliation Signed-off-by: Seth Jennings <sjenning@redhat.com> * test(sandbox): cover deleting suspended sandbox Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(kubernetes): preserve progressing sandbox suspension Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(kubernetes): bound suspend status polling Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(kubernetes): detect legacy sandbox suspension Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(tui): render suspended sandbox phases Signed-off-by: Seth Jennings <sjenning@redhat.com> * refactor(sandbox): rename suspend and resume lifecycle Signed-off-by: Seth Jennings <sjenning@redhat.com> * perf(server): clean stopped sessions on transition Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(kubernetes): fail fast on rejected stop Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(compute): fence stale restart lifecycle events Signed-off-by: Seth Jennings <sjenning@redhat.com> --------- Signed-off-by: Seth Jennings <sjenning@redhat.com> |
||
|
|
2f96c53b8c |
feat(gateway,cli): windows compilation support (#2496)
* chore(windows): gate Unix-only workspace code for MSVC Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * feat(windows): stub unsupported compute drivers Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * ci(windows): add MSVC mise build lane Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * docs(windows): document MSVC build-only design Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * docs(agent): add Windows MSVC build skill Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * feat(windows): add Windows build support Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor(windows): consolidate Windows-specific dependencies and improve build logic Signed-off-by: Akber Raza <akberr@nvidia.com> * feat(windows): add libclang path resolution and update cargo commands with bundled Z3 features Signed-off-by: Akber Raza <akberr@nvidia.com> * chore(tooling): lock Windows tool artifacts Signed-off-by: Giedrius Burachas <gburachas@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * feat(windows): enhance libclang path resolution to support architecture-specific subdirectories Signed-off-by: Akber Raza <akberr@nvidia.com> * Fix Windows dependency gating after sync merge Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(z3): update Z3 header path requirements in Windows build documentation and scripts Signed-off-by: Akber Raza <akberr@nvidia.com> * docs(windows): relocate Windows MSVC build design to architecture/ Why: windows-msvc-build-design.mdx is a design document ("design decisions for the native Windows MSVC build lane"), but it lived in the published, user-facing docs/reference/ tree. Per AGENTS.md (Documentation) and architecture/README.md ("rfc/ vs architecture/"), design content belongs in architecture/ (or rfc/), not in published reference. It also shared Fern sidebar "position: 6" with the MXC compute-driver design page, colliding in the Reference nav ordering. What: - Move docs/reference/windows-msvc-build-design.mdx -> architecture/windows-msvc-build.md. - Strip the Fern publish frontmatter and add a plain H1, matching the other architecture docs. - Register it in the architecture doc index in architecture/README.md. - Repoint the inbound references (build-openshell-mxc-windows skill + reference, implement-openshell-mxc-driver skill) to the new path. With both design pages moved out of docs/reference/, the duplicate position-6 sidebar collision is resolved. Signed-off-by: Akber Raza <akberr@nvidia.com> * remove openshell-supervisor-network from unsupported driver package test exclusion list Signed-off-by: Akber Raza <akberr@nvidia.com> # Conflicts: # tasks/scripts/windows-msvc.ps1 * fix(interceptors): gate unix-only imports so the crate builds on Windows openshell-gateway-interceptors failed to compile on Windows (E0432: no UnixStream in tokio::net), breaking any Windows build of openshell-server (which depends on it unconditionally). The connect_unix_endpoint fn was already #[cfg(unix)]-gated, but the imports it uses (UnixStream, TokioIo, Uri, service_fn) were left ungated. Gate those four imports with #[cfg(unix)] too. No behavior change on unix; Windows now compiles (no errors, no unused-import warnings). Signed-off-by: Akber Raza <akberr@nvidia.com> * feat(windows): add native ARM64 test support Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(mise): skip Skaffold on Windows Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): harden ARM64 toolchain discovery Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): scope ARM64 toolchain preflight Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): restore compatibility after GitHub sync Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): avoid rate-limited Z3 source lookup Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(mise): skip Helm checks on Windows Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): support repository pre-commit checks Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): stabilize native MSVC validation Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): harden shared Z3 source cache Signed-off-by: Shailendra Singh <shailendras@nvidia.com> * fix(windows): avoid leaking MSVC flags into clang-cl Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): complete ARM64 migration audit Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): restore ARM64 Ninja discovery Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor(windows): separate platform crate roots Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): restore proto include cfg gating Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor: address lint errors * fix(windows): add preflight check for proxy auth file path * docs(windows): update GitHub checkout guidance Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): restore CI after dependency updates Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(mise): repair Windows sccache lock entry Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(windows): reconcile validation after rebase Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor(server): exclude unsupported drivers on Windows Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(server): isolate platform driver config Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(windows): repair unsupported driver contract test Signed-off-by: Akber Raza <akberr@nvidia.com> * fix(sandbox): remove stale dependencies Signed-off-by: Akber Raza <akberr@nvidia.com> * ci(windows): pin x64 workflow actions Signed-off-by: Akber Raza <akberr@nvidia.com> * ci(windows): align x64 Rust toolchain Signed-off-by: Akber Raza <akberr@nvidia.com> * ci(windows): align ARM64 workflow setup Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor(windows): exclude unsupported runtime crates Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor(windows): exclude unsupported crates at workspace boundary Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * refactor(server): gate builtin driver config by platform Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * fix(sandbox): restore crate documentation Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * ci(windows): make build workflow manual Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * ci(windows): temporarily enable pull request builds Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * ci(windows): cache Rust dependencies Signed-off-by: Akber Raza <akberr@nvidia.com> * refactor(windows): remove unnecessary platform changes Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * ci(windows): make build workflow manual Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * fix(ci): synchronize mise lockfile Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * fix(ci): normalize mise provenance metadata Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * refactor(python): isolate Windows atomic replace retry Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> * fix(python): type Windows permission test errors Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> --------- Signed-off-by: Shailendra Singh <shailendras@nvidia.com> Signed-off-by: Akber Raza <akberr@nvidia.com> Signed-off-by: Giedrius Burachas <gburachas@nvidia.com> Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> Signed-off-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> Co-authored-by: Shailendra Singh <shailendras@nvidia.com> Co-authored-by: Giedrius Burachas <gburachas@nvidia.com> Co-authored-by: Jamie King <jamiek@nvidia.com> Co-authored-by: Piotr Mlocek <pmlocek@nvidia.com> Co-authored-by: Piotr Mlocek <1116309+pimlock@users.noreply.github.com> |
||
|
Shiju
|
c825b1f8ef |
perf(supervisor-middleware): remove body clones from local dispatch (#2679)
Signed-off-by: Shiju <shiju@nvidia.com> Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> |
||
|
John T. MyersandJohn Myers
|
0120535efc |
feat(proxy): bind static credentials to provider endpoints (#2510)
* feat(proxy): bind static credentials to provider endpoints Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * test(e2e): verify static credential endpoint isolation Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * docs(provider): explain static credential endpoint binding Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * fix(e2e): use valid endpoint isolation fixtures Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * docs(provider): explain static credential endpoint binding Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * fix(credentials): preserve binding identity across rotations Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(proxy): enforce bindings across request lifecycle Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(proxy): close credential relay gaps Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(credentials): clarify binding failure behavior Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): hash selected provider profile scope Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(proxy): resolve credentials after request admission Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(credentials): clarify binding failure diagnostics Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(proxy): align single-route credential denials Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): harden endpoint-bound rotation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): enforce identity and authority binding Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): snapshot provider environment atomically Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(e2e): include authority port in query proxy requests Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): close credential revocation gaps Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(proxy): explain authority mismatch diagnostics Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): enforce binding lifecycle invariants Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(provider): reject credential config collisions Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(network): capture credential scope atomically Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(network): distinguish origin and absolute targets Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(provider): isolate endpointless profile credentials Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(network): normalize IPv6 request authorities Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(credentials): clarify endpointless profile isolation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(policy): bind endpointless provider credentials Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): use current GCP placeholder revision Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(providers): explain policy credential bindings Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(credentials): cover endpointless fail-closed invariant Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(policy): expect ambiguity rejection at creation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(server): authenticate rebased policy requests Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * refactor(proxy): share credential mismatch finding builder Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(credentials): cover malformed binding metadata Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(credentials): verify multi-key endpoint isolation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(e2e): cover same-host credential path denial Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(credentials): document serialized refresh contract Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * refactor(proxy): consolidate L7 log formatting Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * perf(credentials): precompile endpoint binding patterns Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * perf(credentials): share identity epoch revisions Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(proxy): require explicit request default ports Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(policy): validate SigV4 credential sources Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(credentials): preserve endpoint bindings for credential handles Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(go-sdk): expose network credential bindings Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --------- Signed-off-by: John Myers <johntmyers@users.noreply.github.com> Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> Co-authored-by: John Myers <johntmyers@users.noreply.github.com> |
||
|
Matthew Grossman
|
4cb77a900e |
fix(e2e): separate Podman Machine loopback listeners (#2622)
* fix(e2e): separate Podman Machine loopback listeners Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(e2e): remove shallow harness checks Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * refactor(e2e): trim Podman listener workaround Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(e2e): bypass proxies for Podman health probe Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> |
||
|
Simon Scatton
|
d85339d621 |
build(bazel): add credential driver targets (#2649)
* build(bazel): add credential driver targets Signed-off-by: Simon Scatton <sscatton@nvidia.com> * fix(bazel): sync default CA root features Signed-off-by: Simon Scatton <sscatton@nvidia.com> --------- Signed-off-by: Simon Scatton <sscatton@nvidia.com> |
||
|
|
5548405fcb |
feat(credentials): add provider credential storage drivers (#2437)
* feat(credentials): add provider credential storage drivers Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(credentials): harden credential update handling Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(credentials): harden credential driver security, correctness, and performance Address review findings from the credential storage drivers PR: - Route additional_credentials through the driver on refresh to prevent silent data loss for multi-credential providers (e.g. AWS STS) - Clean up stored credential handles on CAS failure during refresh to prevent orphaned secrets in external backends - Enforce namespace validation in the Kubernetes Secrets driver to prevent cross-namespace credential access when allow_reference_namespace is not enabled - Cache Vault Kubernetes auth tokens with 80% TTL to avoid re-authenticating on every credential operation - Parallelize resolve_credentials in all three drivers using try_join_all for faster sandbox startup - Add existingSecret support for the KEK Secret to fix helm template/GitOps workflows where lookup returns empty and regenerates the key - Document RBAC blast radius for the Kubernetes Secrets credential driver and recommend a dedicated namespace Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(credentials): add optimistic concurrency, fix thundering herd, parallelize operations Use resourceVersion optimistic concurrency with retry loop for K8s Secret ownership checks to prevent TOCTOU races. Switch Vault token cache from RwLock to Mutex with double-check pattern to prevent thundering herd on cache miss. Parallelize credential store and delete operations across independent keys using try_join_all. Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(credentials): handle partial failures, add delete retry, consolidate cleanup Replace try_join_all with join_all in credential store/delete operations to handle partial failures — successfully-stored handles are cleaned up when another key fails. Add retry loop with conflict detection to db-credstore delete_credential, matching the K8s driver pattern. Consolidate 4 manual cleanup_pre_stored_provider_credentials call sites into a single error handler using an async block. Remove inconsistent .trim() from db-credstore validate_handle_owner. Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(credentials): fix retry loop guard and remove unprotected validation Remove attempt-count guard from 409/Aborted match arms in retry loops so the post-loop Status::aborted error is reachable after exhausting retries. Previously, last-attempt conflicts fell through to the catch-all error arm, producing misleading Status::unavailable errors. Remove duplicate validation calls that ran after prepare_provider_credential_update but outside the cleanup-protected async block, which would leak pre-stored handles on failure. Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(credentials): add workspace/provider UUID to credential backend paths Include workspace and provider ID in credential backend object paths to ensure cross-workspace uniqueness and prevent credential collision (GATOR-1806c9be-01). - Updated credential driver proto to include workspace and provider_id fields - Modified Vault driver to include workspace/provider_id in managed_secret_path - Modified Kubernetes Secrets driver to include workspace/provider_id in credential_owner_id and managed_secret_name - Updated all credential runtime calls to pass workspace/provider_id - Updated tests to use the new signatures This prevents two workspaces sharing the same external credential store from colliding on provider names, which was a critical security issue (CWE-639). * fix(credentials): preserve provider-level expiration for handle-backed credentials Compute effective expiration from both provider and driver values using the earliest non-zero timestamp and skip expired values before insertion (GATOR-1806c9be-02). - Modified resolve_provider_handles to check provider credential_expires_at_ms - Skip expired credentials during resolution instead of returning them - Use effective expiration (min of provider and driver) in resolution results - Fix inference.rs to preserve earliest expiration when merging This ensures handle-backed credentials respect the same expiration semantics as inline credentials. * fix(credentials): stage refresh changes under new handles before validation Stage credential replacements under new immutable handles instead of reusing existing handles to prevent overwriting committed values before validation/CAS (GATOR-1806c9be-03). - Stage credentials with empty existing_handles map to force new handle creation - Validate and CAS before the new values are committed to backend storage - Delete old handles only after successful CAS - On CAS failure, delete only the newly staged handles - This prevents CWE-362/CWE-367 race conditions where failed refreshes could still modify or delete the active credential The fix ensures that a rejected refresh cannot modify the backend object still referenced by the committed provider record. * fix(credentials): add timeouts to credential driver RPCs Apply configured timeouts to both startup capability negotiation and runtime RPCs to prevent indefinite hangs (GATOR-1806c9be-05). - Add DEFAULT_CREDENTIAL_DRIVER_RPC_TIMEOUT_SECS constant (30s) - Apply timeout to GetCapabilities during startup connection - Apply timeout to all runtime RPCs (store, delete, resolve) - Use tokio::time::timeout to bound the entire GetCapabilities operation during startup, not just the socket connection - Return contextual deadline errors on timeout This prevents a faulty or overloaded driver from hanging gateway operations indefinitely. * fix(credentials): fix test to use consistent workspace/provider identity The Kubernetes auth Vault resolve test was constructing a managed path with test-workspace/test-provider-id but sending default/prov-123 in the request, causing validation to reject the request (GATOR-18e32351-01). - Update test to use test-workspace and test-provider-id in the request to match the logical_path construction - This ensures the test exercises the intended code path and validates Kubernetes auth resolution properly The test now passes and correctly validates identity enforcement. * fix(credentials): use unique staging ID for refresh to avoid overwrites Stage refresh replacements under genuinely distinct immutable handles using a unique staging ID to prevent overwriting committed values (GATOR-1806c9be-03). - Generate a unique staging ID using UUID for each refresh operation - Use this staging ID when storing credentials instead of the real provider ID - Pass the same staging ID during cleanup on failure to delete only staged objects - This ensures deterministic paths (Vault) and object names (K8s) don't collide with the committed provider's credentials The fix prevents failed refreshes from silently replacing active credentials or breaking providers by deleting still-referenced backend objects. * fix(credentials): wrap credential driver RPCs in local timeouts Add local tokio::time::timeout wrappers around credential driver RPCs to bound non-compliant or stalled UDS peers (GATOR-1806c9be-05). - Wrap StoreCredential, DeleteCredential, and ResolveCredentials in local timeouts - Return contextual deadline_exceeded errors when timeouts occur - Keep existing gRPC timeout metadata for compliant implementations - GetCapabilities during startup was already wrapped in previous commit This ensures a faulty local driver cannot hang gateway operations indefinitely, even if it accepts the connection but never responds to the RPC. * fix(credentials): preserve ownership for staged refreshes Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(credentials): bound startup capability probe Signed-off-by: Seth Jennings <sjenning@redhat.com> * test(provider): authenticate credential handler requests Signed-off-by: Seth Jennings <sjenning@redhat.com> * fix(ci): grant actions read to credential driver e2e Signed-off-by: Seth Jennings <sjenning@redhat.com> --------- Signed-off-by: Taylor Mutch <taylormutch@gmail.com> Signed-off-by: Varsha Prasad <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Signed-off-by: Seth Jennings <sjenning@redhat.com> Co-authored-by: Taylor Mutch <taylormutch@gmail.com> Co-authored-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> |
||
|
Jim Meyer
|
8c7dd148a9 |
perf(net): set TCP_NODELAY on latency-sensitive TCP hops (#2220)
* perf(net): set TCP_NODELAY on all tunnel and proxy TCP hops The sandbox tunnel added ~44 ms of latency to every small request/response because no socket in the path disabled Nagle's algorithm, so sub-MSS writes waited on delayed ACKs at each hop. Set TCP_NODELAY on every latency-sensitive TCP socket: - gateway: accepted connections on the public listener (gRPC relay frames and WS tunnel writes) - CLI: edge tunnel local accept + underlying WebSocket TCP stream, insecure TLS connector (tonic's default connector already does this), and service-forward accepted sockets - supervisor: direct-tcpip connect into the sandbox netns, TCP relay target dials, egress proxy accepted connections, and all upstream CONNECT/HTTP dials (via a new connect_upstream helper) Setting TCP_NODELAY on connect is best-effort: a failure only costs latency, so we log and continue rather than fail the connection. The sandbox SSH transport rides a unix domain socket and gRPC client channels use tonic defaults (nodelay on), so no change is needed there. Fixes #2219 Signed-off-by: Jim Meyer <jim@meyer4hire.com> * refactor(net): house TCP_NODELAY helper in a shared net module Address review feedback on the TCP_NODELAY change: - Move the shared set-nodelay helper out of supervisor_session into a new crate-private `net` module in openshell-supervisor-process, so ssh and supervisor_session no longer reach across modules through a pub(crate) item. - Make the best-effort comments at each call site terse and consistent. No behavior change; the benchmark ladder reproduces the same numbers. Signed-off-by: Jim Meyer <jim@meyer4hire.com> * refactor(net): consolidate TCP_NODELAY helpers into openshell_core::net Move the best-effort TCP_NODELAY helpers into the shared openshell_core::net module so every crate dials and configures sockets the same way: - Add set_tcp_nodelay_best_effort (accepted/existing streams) and connect_tcp_nodelay_best_effort (dial + set) with unit tests. - Migrate all call sites in openshell-cli, openshell-server, and the supervisor crates to the shared helpers. - Remove the crate-private net module from openshell-supervisor-process. - Document socket guidance in AGENTS.md (Network Sockets). Signed-off-by: Jim Meyer <jim@meyer4hire.com> * perf(net): set TCP_NODELAY on exec bridge and metadata server The gateway-side single-use SSH-over-relay loopback bridge and the sandbox IMDS metadata server were missed latency-sensitive TCP hops. Set TCP_NODELAY on the accepted client connection and both russh client dials of the exec bridge — interactive keystrokes and line-buffered PTY output are the most tinygram-heavy traffic in the system — and on the metadata server's accepted connections. Also log unrecognized MaybeTlsStream variants in the edge tunnel so a future TLS-backend change surfaces a silent TCP_NODELAY miss instead of skipping it quietly. Signed-off-by: Jim Meyer <jim@meyer4hire.com> * perf(net): set TCP_NODELAY on openshell-sdk socket paths The openshell-sdk crate landed on main with its own copies of the CLI's hand-rolled sockets, which the CLI and TUI are meant to consume. Give them the same treatment as the CLI equivalents: - edge_tunnel: the accepted local tunnel connection and the WebSocket's underlying TCP socket (plain and rustls variants). - transport: the dial in InsecureTlsConnector, tonic's custom-connector path. Only these hand-rolled sockets need it. Tonic's own connector defaults tcp_nodelay to true and applies it itself, so plain Endpoint::connect callers were already covered. Signed-off-by: Jim Meyer <jim@meyer4hire.com> --------- Signed-off-by: Jim Meyer <jim@meyer4hire.com> |
||
|
Matthew Grossman
|
537805568d |
feat(sandbox): honor OCI image working directories (#2530)
* feat(sandbox): honor Docker OCI working directories Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): honor effective workspace access Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(sandbox): cover enforced workspace denial Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * docs(docker): explain effective workdir checks Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): validate effective workspace writes Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): reserve supervisor control roots Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * refactor(sandbox): centralize control paths Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): reserve OCI runtime mount roots Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> |
||
|
Polite_realismandAdam Miller
|
0e9a44cfa9 |
feat(build): add system CA root mode (#2324)
* feat(build): add system CA root mode Allow distro builds to use native trust stores for supervisor upstream TLS while keeping bundled Mozilla roots as the default. Avoid bundled root crates in system-ca-roots builds by using native-root TLS features and z3 0.20. Signed-off-by: Adam Miller <admiller@redhat.com> * fix(build): keep CA root feature in telemetry-off verification The telemetry-off task uses --no-default-features which now disables bundled-ca-roots in addition to telemetry, triggering the compile_error guard. Re-enable bundled-ca-roots explicitly so the task verifies only telemetry compilation. Signed-off-by: Scott Burdine <sburdine@nvidia.com> Signed-off-by: politerealism <burdcat17@gmail.com> * fix(sdk): disable oauth2 default features to prevent webpki-roots leak The bare `oauth2 = "5"` dependency re-enabled default features (rustls-tls → reqwest/rustls-tls → webpki-roots), defeating the system-ca-roots feature gate. Mirror the CLI fix: disable defaults and enable only the `reqwest` feature. Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com> * refactor(build): simplify CA root selection to single feature toggle Replace mutually exclusive bundled-ca-roots / system-ca-roots features with a single bundled-ca-roots toggle. Disabling it implies system roots via rustls-native-certs, which is now a regular (non-optional) dependency. This fixes cargo --all-features and simplifies the distro build interface from --no-default-features --features system-ca-roots to just --no-default-features. Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com> * feat(build): add system-ca-roots convenience alias and fix verify task Add a system-ca-roots feature alias on openshell-sandbox that includes all other defaults (telemetry) except bundled-ca-roots, so distro builds can use --no-default-features --features system-ca-roots without manually re-adding unrelated defaults. Update the verify CI task to use the alias and scope checks to the sandbox package. Fix task description to use "build mode" terminology instead of implying a Cargo feature. Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com> * ci: fix system CA roots step name to use build mode terminology Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com> * refactor(sandbox): reorder features to place system-ca-roots alias near default Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com> * fix(proxy): unwrap Result from build_upstream_client_config in tests The function signature changed to return Result but the test call sites were not updated, causing type mismatch compilation errors in CI. Signed-off-by: Quinn Burdine <sburdine@redhat.com> Signed-off-by: politerealism <burdcat17@gmail.com> --------- Signed-off-by: Adam Miller <admiller@redhat.com> Signed-off-by: Scott Burdine <sburdine@nvidia.com> Signed-off-by: politerealism <burdcat17@gmail.com> Signed-off-by: Quinn Burdine <sburdine@redhat.com> Co-authored-by: Adam Miller <admiller@redhat.com> |
||
|
Simon Scatton
|
b9818619b8 |
test: disable tests flaky under parallel stress (#2611)
Repeated Bazel test targets with --runs_per_test at 10, 20, 50, 100, and 200 runs. The tests failed intermittently when multiple instances ran concurrently. This indicates timing, shared tracing state, socket readiness, or parallel-safety issues that need focused follow-up before re-enabling the tests. A normal non-repeated bazel test //... run passes with these tests ignored. Disabled tests: - sandbox_forward_foreground_fails_when_ssh_exits_before_listener_opens - sandbox_forward_background_terminates_owned_child_when_listener_never_opens - podman_socket_probe_accepts_successful_ping_response - podman_socket_probe_rejects_docker_ping_response - docker_socket_probe_accepts_successful_ping_response - docker_socket_probe_rejects_podman_ping_response - docker_socket_detection_returns_the_responsive_candidate - podman_socket_detection_returns_the_responsive_candidate - driver_watch_events_are_roots_and_store_operations_have_parents - reconcile_sweeps_are_roots_and_operations_have_parents - gateway_listeners_bind_ipv6_wildcard_and_ipv4_callback_on_same_port - watch_producer_releases_request_span_when_client_disconnects - expected_conflicts_leave_the_span_unmarked - store_spans_record_what_they_touched_as_attributes - store_operations_export_spans_with_parents - refresh_worker_ticks_are_roots_and_store_operations_have_parents Signed-off-by: Simon Scatton <sscatton@nvidia.com> |
||
|
Simon Scatton
|
0a3ec7a112 | feat(bazel): add rustfmt checks to tests (#2599) | ||
|
Simon Scatton
|
1959ea19be |
build(bazel): establish RFC 0012 Rust reference graph (#2414)
Establish the Phase 1 reference implementation proposed by RFC 0012 while retaining the existing Cargo and Mise workflows during evaluation. - pin Bazel 9.1.1 and configure Bzlmod, rules_rs, LLVM, protobuf, and Rust 1.95 toolchains - import third-party crates from Cargo metadata and propagate the workspace version into Bazel targets - add library, binary, proc-macro, unit-test, and integration-test targets across the supported Rust workspace crates and drivers - generate protobuf Rust sources and descriptor sets under Bazel while preserving Cargo-compatible generated-code imports - annotate aws-lc-sys and zstd-sys native dependencies, build Z3 4.15.2 from source, and generate z3-sys bindings - make CLI and procfs test fixtures available as explicit Bazel inputs without relying on fixed host binary paths - define optimized release targets for Linux x86_64 and aarch64 CLI, sandbox, and gateway binaries, plus macOS aarch64 artifacts - add Bazel, buildifier, and lcov to the Nix development environment RFC: 0012 (rfc12 branch) Refs: #2491 Signed-off-by: Simon Scatton <sscatton@nvidia.com> |
||
|
John T. MyersandJohn Myers
|
905b554c7c |
refactor(network): consolidate proxy egress pipeline (#2373)
* refactor(network): introduce shared egress pipeline Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): cover shared proxy egress paths Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * refactor(network): make destination authorization explicit Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * refactor(network): pin proxy relay policy context Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): lock relay generation contracts Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): establish phase zero compatibility baseline Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(policy): detect ambiguous network endpoints Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * feat(sandbox): fail closed on invalid policy updates Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * refactor(network): invalidate relays on policy changes Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(policy): document validation failure posture Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): cover validation and middleware egress Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(config): move policy failure mode to gateway toml Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): name proxy contracts by behavior Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(network): align overlap validation with endpoint selection Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): expect hard loopback denial Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): match declared endpoint denial Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(policy): preserve path-specific endpoint overrides Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(network): respect hard-blocked host gateways Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(network): reconcile proxy refactor with main Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(network): preserve CONNECT policy generation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(policy): cover runtime endpoint glob semantics Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(server): reject ambiguous policies before persistence Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * docs(policy): explain ambiguity preflight behavior Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(policy): compare body limits within protocol Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(proxy): avoid global tracing capture race Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * chore(server): format rebased provider tests Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * test(server): authenticate rebased policy requests Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(sandbox): retain runtime on middleware outage Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(server): preflight provider composition activation Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> * fix(sandbox): distinguish runtime failure transitions Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --------- 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> |
||
|
Russell Bryant
|
0cecb54249 |
fix(cli): bracket IPv6 bind literals in SSH forwards (#2552)
ForwardSpec accepts IPv6 bind addresses, but ssh_forward_arg() emitted them unbracketed (e.g. ::1:8080:127.0.0.1:8080), which OpenSSH rejects as a bad local forwarding specification. access_url() likewise produced invalid URLs like http://::1:8080/. Extract a shared bracket_ipv6_host() helper and use it in ssh_forward_arg(), access_url() (via format_gateway_url), and format_gateway_url() so IPv6 literals are bracketed consistently. Fixes #2279 Signed-off-by: Russell Bryant <rbryant@redhat.com> |
||
|
Matthew Grossman
|
bc14018cad |
feat(sandbox): use policy-first OCI image identity (#2509)
* feat(sandbox): use policy-first OCI image identity Closes #2331 Preserve per-field policy omission, derive Docker and Podman fallbacks from the inspected immutable image, and resolve the final numeric identity before starting agent children. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): preserve declared process identities Keep explicit policy values and OCI-declared names intact, defer passwd lookup until a primary GID is required, and refresh stale policy examples. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(supervisor): reuse resolved OCI identity Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(supervisor): allow Linux pre-exec arguments Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(kubernetes): protect resolved sandbox identity Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): prepare workspace for OCI identity Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * refactor(sandbox): own only workspace root Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): harden partial identity drops Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(sandbox): scope OCI image e2e to Docker Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(sandbox): narrow OCI identity fallback scope Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * test(podman): cover OCI identity launch Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(podman): exercise OCI fallback in E2E Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> |
||
|
Evan Lezar
|
f00ad23a26 |
fix(podman): tolerate shutdown transport closes (#2498)
Signed-off-by: Evan Lezar <elezar@nvidia.com> |
||
|
Andrew White
|
76a5397e6f |
fix: assorted arithmetic and indexing robustness fixes (#2451)
* fix(core): avoid i64 underflow on expired GCP token When a cached GCP access token was already expired (expires_at_ms < now), the remaining lifetime calculation used plain subtraction: (expires_at_ms - now) / 1000. This underflows in debug builds and wraps in release builds. Use saturating_sub so an expired token simply yields a non-positive remaining lifetime and is skipped. Add a regression test. Signed-off-by: Andrew White <andrewh@cdw.com> * fix(server): avoid i64 underflow in lease age calculation The lease steal path computed age as now_ms() - record.updated_at_ms. If the stored updated_at timestamp is in the future (e.g. clock skew), the subtraction underflows. Use saturating_sub to treat a future timestamp as age 0, keeping the lease considered fresh. Signed-off-by: Andrew White <andrewh@cdw.com> * fix(server): avoid i64 overflow on large credential max lifetime max_lifetime_seconds is only validated as >= 0. The code multiplied the raw i64 value by 1000 to compute max_expires, which overflows for values above i64::MAX / 1000. The capped i32 value used for the STS request was already computed but not reused. Compute max_lifetime_ms from the capped value with saturating_mul and use it for both expires_at fallback and max_expires. Signed-off-by: Andrew White <andrewh@cdw.com> * fix(server): avoid hardcoded indexing into base_url_config_keys resolve_vertex_ai_route assumed every inference provider profile has at least two base URL config keys by indexing [0] and [1]. Only the Vertex profile satisfies that today; other profiles would panic here. Iterate base_url_config_keys with find_map instead, and add a test with an empty key list to ensure no panic. Signed-off-by: Andrew White <andrewh@cdw.com> * fix(server): clamp lease age at zero for future timestamps Address review feedback: add regression coverage for future-timestamp lease clock skew. Extract lease_is_expired() and clamp the age at zero — i64::saturating_sub saturates at i64::MIN, not zero, so a future updated_at_ms must be clamped explicitly to be treated as age zero and remain unstealable with a positive TTL. Signed-off-by: Andrew White <andrewh@cdw.com> * fix(server): fall back past blank preferred Vertex base URL Address review feedback: find_map stopped at the first present key even when its value was blank, and the outer filter discarded it without considering lower-priority aliases. Filter blank values inside the closure so a blank preferred key falls back to a valid alias, matching the documented priority order. Add a regression test with a blank preferred key and a valid fallback. Signed-off-by: Andrew White <andrewh@cdw.com> * fix(server): saturate AWS STS expiry cap on saturated clock Address review feedback: now_ms + max_lifetime_ms could still overflow when current_time_ms() saturates near i64::MAX. Compute max_expires = now_ms.saturating_add(max_lifetime_ms) once and use it for both the conversion fallback and the expiry cap. Add a boundary unit test covering a saturated clock. Signed-off-by: Andrew White <andrewh@cdw.com> --------- Signed-off-by: Andrew White <andrewh@cdw.com> |
||
|
Philippe Martin
|
77e5c32217 |
feat(sandbox,gateway): route sandbox egress through corporate HTTP proxy (#2245)
* feat(sandbox,gateway): route sandbox egress through corporate HTTP proxy - Chain sandbox egress through a corporate HTTP proxy so outbound traffic from within the sandbox respects the host proxy settings - Forward sandbox proxy environment variables to the generated Podman config so the proxy is applied consistently to Podman-managed workloads Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): make corporate proxy routing operator-owned The operator-configured corporate egress proxy was injected under the conventional HTTPS_PROXY/HTTP_PROXY/NO_PROXY names as defaults beneath sandbox spec/template environment, so a sandbox creator could redirect egress at an arbitrary proxy or disable proxying with NO_PROXY=*. Route the boundary through reserved, supervisor-only variables (OPENSHELL_UPSTREAM_HTTPS_PROXY/HTTP_PROXY/NO_PROXY) written in the Podman driver's required-variable tier. Any sandbox-supplied value under a reserved name is stripped before the operator value is applied, so the supervisor never observes a reserved proxy variable the operator did not set. The supervisor now reads only the reserved names and ignores the conventional proxy variables the sandbox controls. Add the reserved proxy variables to the supervisor-only child-environment denylist so the corporate proxy URL and any embedded credentials are not inherited by the sandbox workload, which reaches egress through the local policy proxy and never needs them. Signed-off-by: Philippe Martin <phmartin@redhat.com> * feat(sandbox,podman): deliver corporate proxy credentials via secret file Proxy credentials were embedded inline in the proxy URL, so they were stored in gateway.toml and exposed in container metadata via 'podman inspect'. Reject inline 'user:pass@' credentials in https_proxy/http_proxy at startup (parsed with the url crate rather than hand-splitting), and add a proxy_auth_file option pointing at a 'user:pass' file. The driver stages that file as a per-sandbox root-only Podman secret, mounts it at a fixed path, and exports only the path in the reserved OPENSHELL_UPSTREAM_PROXY_AUTH_FILE variable, so the credential never appears in config, environment, or container metadata. Reading the file fails closed on a missing, empty, or control-character-bearing value. The supervisor reads the credential from the mounted file and builds the Proxy-Authorization: Basic header, rejecting control characters, and no longer derives credentials from URL userinfo. The auth-file path is added to the child-environment strip list, and the generated gateway.toml is written owner-only (mode 0600). Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): fail closed on invalid upstream proxy configuration The reserved OPENSHELL_UPSTREAM_* variables are an operator-owned egress boundary, but the supervisor treated present-but-invalid values as unset: an unsupported or malformed proxy URL was ignored with a warning, an unreadable auth file proceeded without credentials, and a malformed credential silently became unauthenticated. Any of these could quietly downgrade the corporate proxy boundary to direct dialing or unauthenticated proxy access. Make every configured-but-invalid proxy or auth setting fatal to supervisor proxy startup, emit an OCSF ConfigStateChange failure event before refusing, and share URL validation semantics between the Podman driver and the supervisor through a single validator in openshell-core (parse_upstream_proxy_url), so a value accepted at sandbox-create time can never be rejected in-container or vice versa. Inline user:pass@ URL credentials are now fatal in the supervisor too (previously warn-and-strip), matching the driver. Unset or empty variables still mean no proxy; only present-but-invalid values fail. Error paths never include credential content. Addresses the fail-closed review item on #2245. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): finish the fail-closed upstream proxy credential contract The upstream proxy URL already had a single shared validator, but the credential did not: the Podman driver rejected only CR/LF/NUL while the supervisor rejected every control character, so a credential accepted at sandbox-create time (e.g. one containing a tab) could still be rejected in-container. Present-but-whitespace reserved OPENSHELL_UPSTREAM_* values were also silently treated as unset, quietly downgrading the operator's egress boundary to direct dialing. Add parse_upstream_proxy_credential to openshell-core as the single source of truth for the documented user:pass credential form (non-empty user, no control characters, trimmed) and use it in both the Podman driver's secret staging and the supervisor's Proxy-Authorization header construction. Error variants carry no payload so credential content can never leak into messages. Make a present-but-empty reserved variable fatal to supervisor proxy startup instead of meaning "unset"; only fully unset variables disable the proxy. The driver correspondingly rejects an empty no_proxy at config time so it can never inject a value the supervisor refuses. Addresses the remaining fail-closed credential/config review item on Signed-off-by: Philippe Martin <phmartin@redhat.com> #2245. * fix(sandbox,podman): close remaining fail-open upstream proxy config paths Two configuration paths could still silently run without the proxy boundary the operator believed was in effect. A no_proxy bypass list configured without any https_proxy/http_proxy was accepted by both the driver and the supervisor and simply meant "dial everything directly". Reject it on both sides, exactly like the existing proxy_auth_file-without-proxy rule: an operator who wrote a bypass list assumed proxying was active, so accepting it hides a fail-open state. The gateway.sh dev script guarded proxy settings with [[ -n "${VAR:-}" ]], which conflates unset with explicitly-empty and dropped the latter before the gateway's validation could see it. Use ${VAR+x} instead so a set-but-empty variable is written into gateway.toml and rejected at startup by validate_proxy_config rather than silently discarded. Addresses the remaining fail-open configuration review item on #2245. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): reject upstream proxy URLs with path, query, or fragment parse_upstream_proxy_url accepted URLs like http://proxy.corp.com:8080/some/path and silently discarded everything after host:port, a lenience inherited from the original supervisor parser. A forward proxy is addressed by host:port only, so extra components indicate a misconfiguration (for example a pasted endpoint URL) and silently truncating them violates the present-but-invalid-is-fatal contract enforced everywhere else in this configuration surface. Reject a path, query, or fragment in the shared validator with a new UnexpectedComponent error. A bare trailing slash remains accepted because the url crate normalizes an absent http path to "/", making the two indistinguishable. Both the Podman driver (gateway startup) and the supervisor (sandbox startup) inherit the rule through the shared parser, keeping their semantics identical by construction. Addresses the proxy URL component review item on #2245. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): remove plain-HTTP upstream proxy support The http_proxy path tunneled plain-HTTP requests through the corporate proxy with CONNECT to port 80 and then sent origin-form requests down the tunnel. Conventional enterprise forward proxies expect plain HTTP as absolute-form requests sent directly over the proxy connection, and commonly refuse CONNECT to port 80, so the setting looked supported but failed against typical deployments. Tunneling also blinds the proxy to the one protocol it could inspect. Narrow the feature to TLS (CONNECT) egress only, which is the conventional and already-correct case: plain-HTTP requests now always dial the destination directly, and only client CONNECT tunnels chain through the corporate proxy. Remove the http_proxy config field, the --sandbox-http-proxy / OPENSHELL_SANDBOX_HTTP_PROXY driver surface, the reserved OPENSHELL_UPSTREAM_HTTP_PROXY variable, and the UpstreamScheme plumbing. The feature never shipped, so this is a clean removal; a stray http_proxy key in gateway.toml still fails loudly through the config's deny_unknown_fields. Removing the plain-HTTP proxy branch also removes its host-gateway special case; the architecture doc now documents the real host-gateway behavior (add driver-injected host aliases to the reserved NO_PROXY list) instead of an invariant the HTTPS path never implemented. Plain-HTTP forwarding through a corporate proxy can return later as absolute-form forwarding behind its own design review. Addresses the plain-HTTP forwarding review item on #2245. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): escape generated TOML and require explicit proxy URL form Escape backslashes, quotes, and control characters when gateway.sh writes proxy values into gateway.toml, so a hostile or unusual environment value cannot corrupt the config or inject extra keys. Restrict the upstream proxy URL grammar to the documented http://host:port form: a scheme-less value is no longer normalized to http:// and a missing port is no longer silently defaulted to 80. Docs, README, and CLI help now state the explicit-form requirement consistently. Also fix a test-only call of handle_tcp_connection that was missing the upstream_proxy argument added in an earlier commit. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): preserve tunneled bytes read with the CONNECT response The CONNECT handshake reads the corporate proxy's response in chunks, so the read that completes the header block can also contain the first tunneled payload bytes. Those bytes were discarded, silently corrupting the start of the tunnel for server-speaks-first destinations or proxies that coalesce writes. connect_via now returns a PrefixedStream that replays any bytes received past the response terminator before reading from the socket again; writes pass through unchanged. Direct dials wrap the stream with an empty prefix so downstream relay and TLS paths keep a single stream type, and tls_connect_upstream is generalized to any AsyncRead + AsyncWrite stream. Adds regression coverage for a combined response/payload read and for prefix replay ordering. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): gate cleartext proxy Basic auth behind an explicit opt-in Proxy-Authorization: Basic is base64 over the plain-TCP connection to the http:// corporate proxy, so anyone on the network path between the sandbox host and the proxy can recover the credential. Sending it is now an explicit operator decision instead of an implicit side effect of configuring proxy_auth_file. Add a proxy_auth_allow_insecure driver setting (CLI --sandbox-proxy-auth-allow-insecure, env OPENSHELL_SANDBOX_PROXY_AUTH_ALLOW_INSECURE), delivered to the supervisor as the reserved OPENSHELL_UPSTREAM_PROXY_AUTH_ALLOW_INSECURE variable. Fail-closed pairing on both sides: an auth file without the acknowledgement is rejected at gateway startup and at supervisor startup, as is the acknowledgement without an auth file or any value other than 'true'. gateway.sh writes the key only as a TOML boolean; a non-boolean value is emitted as a quoted string so the gateway rejects it at startup instead of risking injection. Documents the exposure prominently in the gateway config reference, the driver README, and the sandbox architecture doc. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): honor port qualifiers and resolved addresses in NO_PROXY Two divergences from the documented NO_PROXY contract: - A port-qualified entry (internal.corp:8443) was silently stripped to its hostname and bypassed the proxy for every port, excluding traffic the operator never listed. Entries now keep the optional :port qualifier (also on IP and CIDR entries) and only apply to that destination port; a trailing qualifier that is not a valid port stays part of the pattern instead of widening the entry. - IP and CIDR entries only matched IP-literal hosts, so a bypass like 10.0.0.0/8 never applied to hostnames resolving into that range. NO_PROXY evaluation now sees the validated resolved addresses: an IP/CIDR entry matching through resolution authorizes a direct dial of only the addresses it contains, so a bypass scoped to an internal range cannot widen into a direct dial of addresses outside it. Hostname-level matches (loopback, wildcard, domain entries, IP-literal hosts) keep authorizing all validated addresses. proxy_for is replaced by decision(host, port, resolved) returning either the proxy endpoint or the permitted direct-dial subset, and dial_upstream restricts the direct connect to that subset. Adds regression coverage for port-scoped bypasses on domain, IP, and CIDR entries, invalid port qualifiers, resolved-address matching, and split-resolution subset dialing. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): bind proxied CONNECT tunnels to validated addresses The CONNECT request sent to the corporate proxy carried the destination hostname, so the proxy resolved the name itself and the addresses that had passed SSRF and allowed_ips validation were discarded. Split-horizon DNS or rebinding at the proxy could then reach internal or otherwise unapproved destinations through a tunnel the supervisor logged as validated, and IP-range policy could not be enforced at all on proxied dials. CONNECT now targets a validated resolved address by default: the proxy performs no DNS resolution and the tunnel stays bound to the answer the supervisor checked. The hostname still travels inside the tunnel (TLS SNI, application Host), so destination servers behave normally. In split-horizon networks, operators point the gateway host at the corporate resolver so internal names validate to their internal addresses. For proxies whose ACLs filter on hostnames and reject IP CONNECT targets, a new proxy_connect_by_hostname opt-in (CLI --sandbox-proxy-connect-by-hostname, env OPENSHELL_SANDBOX_PROXY_CONNECT_BY_HOSTNAME, reserved OPENSHELL_UPSTREAM_PROXY_CONNECT_BY_HOSTNAME) restores hostname CONNECT, documented as re-opening proxy-side resolution and making the proxy's ACLs the effective egress control. Fail-closed pairing on both sides: the opt-in without a proxy, or any value other than 'true', is fatal. Adds regression coverage for the IP CONNECT request line (including IPv6 bracketing and hostname non-leakage), the hostname opt-in, and the config pairing rules in driver and supervisor. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): reject empty port after bracketed IPv6 proxy host http://[fd00::1]: passed the explicit-port check because the bracketed branch only tested for a colon after the bracket, then fell back to port 80 — violating the fail-closed http://host:port contract and potentially sending configured Basic credentials to an unintended service. Require a non-empty suffix after ]:, matching the unbracketed branch, and cover the case in the shared parser tests. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): fall back across validated addresses in proxied CONNECT The direct path hands TcpStream::connect the whole validated address list and it falls back across them, but the validated-IP CONNECT path attempted only the first address, so a dual-stack destination could fail through the corporate proxy even when a later validated address was reachable. connect_via_validated tries each validated address in order under one aggregate CONNECT_HANDSHAKE_TIMEOUT budget, returning the first success; when every attempt fails the error names the attempt count and carries the last failure. An empty address list is rejected up front. Adds regressions for first-fails/second-succeeds fallback (asserting both CONNECT request lines), the aggregate all-addresses failure message, and the empty-list rejection. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): strip new reserved proxy vars and complete docs/tests Follow-ups from review: - Add OPENSHELL_UPSTREAM_PROXY_AUTH_ALLOW_INSECURE and OPENSHELL_UPSTREAM_PROXY_CONNECT_BY_HOSTNAME to the supervisor-only strip list so workload child processes never inherit them, matching the documented contract for the other reserved proxy variables, and cover both in the supervisor-only variable test. - Document all five OPENSHELL_SANDBOX_* proxy variables in the mise run gateway help text, marked Podman-only and stating the auth-file/acknowledgement pairing, and complete the gateway-key list in the Podman README. - Add NO_PROXY composition coverage for bracketed IPv6 entries with port qualifiers, bare IPv6 entries, and IPv6 CIDR matching against an IPv6-literal host and against a hostname's resolved addresses, including the port-qualified CIDR form. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): cap each proxied CONNECT attempt within the shared budget A proxy that accepted the first CONNECT request but never responded consumed the entire aggregate handshake timeout, so later validated addresses were never tried and the hang defeated the multi-address fallback. Each attempt is now time-boxed to its fair share of the time remaining before the shared deadline (remaining / attempts_left): a hanging attempt is cut off with enough budget left for every remaining address, while time a fast failure does not use rolls over to later attempts and the total never exceeds CONNECT_HANDSHAKE_TIMEOUT. A timed-out attempt is recorded like any other failure, and the aggregate error distinguishes all-attempted from budget-exhausted runs. Adds a first-hangs/second-succeeds regression driven through a test-visible budget parameter so it runs in about a second instead of a real 30s window. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): deliver corporate proxy config on the supervisor argv The proxy settings were injected as reserved OPENSHELL_UPSTREAM_* container environment variables. The driver only wrote names the operator configured, but container runtimes layer the spec environment over ENV values baked into the sandbox image, so an image could supply NO_PROXY=*, enable hostname CONNECT, or point an unconfigured deployment at an attacker-controlled proxy whenever the operator left a field unset. The settings now travel as supervisor command-line arguments (--upstream-proxy, --upstream-no-proxy, --upstream-proxy-auth-file, --upstream-proxy-auth-allow-insecure, --upstream-proxy-connect-by-hostname) built by the driver from operator config. The driver sets the container entrypoint and command explicitly, so neither sandbox spec/template environment nor image ENV can influence argv, and an omitted flag genuinely means unconfigured — in every supervisor topology, since the supervisor no longer consults its environment for these settings at all. Credentials stay on the root-only secret mount; only the mount path appears on argv. The reserved environment names, their strip-list entries, and the env-based validation surface are removed. UpstreamProxyConfig::from_args replaces from_env, reusing the same shared fail-closed validation and pairing rules keyed by the CLI flag names. Driver tests now assert the argv contract, including that sandbox-supplied environment cannot add, remove, or redirect proxy flags; supervisor tests cover from_args mapping and its pairing rules. Signed-off-by: Philippe Martin <phmartin@redhat.com> * docs(sandbox): align proxy comments with the argv transport The argv migration left comments describing the configuration as reserved environment variables ("reserved value", "present-but-empty variable", "reserved upstream proxy variables"). Rephrase them as driver-supplied arguments and operator settings so the documented trust boundary matches the implementation. Comment-only change. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox): parse bracketed IPv6 authorities in client CONNECT targets parse_target split the CONNECT authority at the first colon, so an IPv6-literal target like [2001:db8::1]:443 always failed port parsing and IPv6-literal clients could never reach policy evaluation; a regression test even locked in that failure. Parse the RFC 3986 bracketed form and return the host bracket-free, matching what DNS resolution, SSRF validation, NO_PROXY matching, and the upstream CONNECT builder expect. Unclosed brackets, a missing or empty port after the bracket, and non-numeric ports are rejected; unbracketed behavior is unchanged. Replaces the failure-locking test with success coverage for bracketed targets and adds malformed-bracket rejection cases. Signed-off-by: Philippe Martin <phmartin@redhat.com> * test(podman): cover proxy-auth secret cleanup across lifecycle failures The per-sandbox proxy-auth credential secret is staged before the container is created and removed on cleanup, but no test proved the cleanup paths actually issue the secret removal. Add Podman-stub tests that drive create_sandbox to a container-create failure and to a start failure, and delete_sandbox for an out-of-band deletion, asserting each path issues the DELETE for the per-sandbox proxy-auth secret so a credential can never outlive the sandbox that owned it. Signed-off-by: Philippe Martin <phmartin@redhat.com> * docs: list corporate proxy keys in the Podman compute-driver overview The Fern Podman driver section enumerated its gateway.toml keys but omitted the corporate egress proxy settings. Add https_proxy, no_proxy, proxy_auth_file, proxy_auth_allow_insecure, and proxy_connect_by_hostname with a pointer to the gateway configuration reference for the full contract. No navigation change: the reference folder already includes the gateway configuration page. Signed-off-by: Philippe Martin <phmartin@redhat.com> * test(sandbox): cover the SSRF-to-TLS composition across the proxy tunnel Existing tests exercised validated-IP CONNECT and the upstream-TLS helper independently, but not the full boundary. Add an end-to-end regression that stands up a fake corporate proxy tunneling to a fake TLS server and drives the real path: connect_via_validated CONNECTs to the validated address, the proxy splices the tunnel, and tls_connect_upstream verifies the upstream certificate against the original hostname carried in SNI. It asserts the CONNECT authority is the validated IP and never the hostname, that verification succeeds for the matching hostname, and that a mismatched hostname is rejected — proving a rebinding or split-horizon substitution behind the proxy cannot pass certificate verification. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox,podman): bound proxy-auth reads, reject port 0, fix stale comment Three review findings: - CWE-400: the proxy-auth credential file was read with an unbounded read_to_string on both the driver (sandbox-create) and supervisor (startup) paths, so a huge file or a special file such as /dev/zero could exhaust memory. Add a shared bounded reader in openshell-core that rejects non-regular files, caps the size at 4 KiB, and reads at most that many bytes; the driver runs it via spawn_blocking. Covers oversized, special-file, and missing-path cases on both sides. - Reject an upstream proxy URL with port 0: it passed the explicit-port check and startup validation but is not a connectable TCP port, so every proxied dial would fail later. Add a typed ZeroPort error with shared-validator and Podman-config tests. - Reword a driver-config comment that still described a 'reserved variable' to match the argv transport. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(sandbox): open proxy-auth file non-blocking to reject FIFOs promptly read_upstream_proxy_credential_file opened the path with a blocking File::open before the regular-file check, so a configured FIFO with no writer would block open() indefinitely — hanging sandbox creation on the driver and supervisor startup. Open with O_NONBLOCK on Unix so the open returns immediately, then reject the non-regular file as before; O_NONBLOCK has no effect on the later read of a regular file. Adds a mkfifo regression asserting the reader returns promptly with a non-regular-file error instead of hanging. Signed-off-by: Philippe Martin <phmartin@redhat.com> * test(podman): cover corporate proxy egress across driver and supervisor The existing corporate-proxy tests construct config structs or call CONNECT helpers directly, so none of them detect a break in the wiring between layers: gateway TOML deserialization, the Podman argv and secret-mount semantics, supervisor CLI parsing, or policy denial before proxy contact. Add a Podman e2e that drives the whole chain against a fake authenticated forward proxy and asserts that an approved TLS request traverses it with a validated-IP CONNECT, a policy-denied destination is refused with 403 without ever reaching the proxy, credentials arrive through the mounted per-sandbox secret, and deleting the sandbox removes that secret. SupportContainer is a new harness fixture. Unlike ContainerHttpServer it probes readiness with a TCP connect rather than an HTTP GET, so it can host a forward proxy and TLS servers, and it exposes container logs and network IP for assertions. Signed-off-by: Philippe Martin <phmartin@redhat.com> * test(podman): restart the gateway on the proxy-config panic path The panic cleanup for the temporary corporate proxy configuration restored the gateway TOML but left the gateway process running with the temporary configuration still loaded, which could poison later test binaries in the same run. Nothing restarted it: the only ManagedGateway is the short-lived one inside restart_gateway, and its Drop only calls start, which does not reload config for an already-running gateway. Restore and synchronously stop/start the gateway in Drop, and set restored only after the normal restore and restart both succeed so a failed restart no longer suppresses the fallback. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(supervisor-network): derive the loopback proxy bypass from resolved IPs The automatic bypass treated the host string "localhost" as proof that the destination was loopback and returned every resolved address. A sandbox controls its own /etc/hosts and resolve_socket_addrs consults it before DNS, so a workload could map localhost to any policy-allowed address and dial it directly, escaping the operator proxy and the inspection and audit boundary it exists to provide. Check the resolved addresses instead: the name bypasses only when the resolution is non-empty and every address is loopback. A mixed answer is not partially honored, and an IP literal is still authoritative for itself. A spoofed localhost falls through to the entries below, so an explicit operator NO_PROXY entry is still honored. This matches the trust model detect_trusted_host_gateway already applies to the same hosts file, which validates the mapped address rather than the alias. Signed-off-by: Philippe Martin <phmartin@redhat.com> * fix(podman): clean up proxy-auth secret when container already deleted The delete_sandbox early-return path cleaned up the token secret but skipped the proxy-auth secret, leaking it on disk. Also update tests for recent API changes (Optional socket_path, workspace field, list-based container lookup). Signed-off-by: Philippe Martin <philippe@openshell.dev> Signed-off-by: Philippe Martin <phmartin@redhat.com> --------- Signed-off-by: Philippe Martin <phmartin@redhat.com> Signed-off-by: Philippe Martin <philippe@openshell.dev> |
||
|
Evan Lezar
|
21da343c9f |
refactor(supervisor): pass agent proposal state explicitly (#2421)
* test(supervisor-network): default L7 eval context in tests Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(supervisor): pass agent proposal state explicitly Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com> |
||
|
alangou
|
1d4ac708f1 |
fix(policy): keep internal allowed IP proposals pending (#2416)
Signed-off-by: Adrien Langou <alangou@nvidia.com> |
||
|
Roland Huss
|
cd9a0bf214 |
fix(driver-k8s): add label selector to sandbox watch stream and list (#2223)
* fix(driver-k8s): harden list path and tighten label selectors - Add warning log to list_sandboxes() filter_map so skipped objects are visible in logs instead of silently dropped. - Add LABEL_SANDBOX_ID existence requirement to both list and watch label selectors so managed-but-non-sandbox resources (e.g. warm pool pods without sandbox identity) never enter either code path. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(driver-k8s): add watch cancellation and selector tests Address review feedback on PR #2223: - Extract shared label selector into openshell_sandbox_label_selector() helper in openshell-core so both list and watch paths use a single tested definition. - Add tx.closed() branch to the watch_sandboxes tokio::select! loop so the producer task exits when the receiver is dropped, preventing leaked Kubernetes watch connections. - Add unit tests for the label selector string and for the cancellation behavior. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(driver-k8s): remove tautological and non-functional tests Remove three tests that don't provide meaningful regression coverage: - openshell_sandbox_label_selector_contains_managed_by_and_sandbox_id: mirrors the implementation with hardcoded strings; any constant change requires updating the test string in lockstep. - label_selector_used_by_list_and_watch_matches_shared_helper: weaker duplicate of the above, checking substrings instead of equality. - watch_producer_exits_when_receiver_is_dropped: tests tokio channel semantics (tx.closed() fires when rx is dropped), not watch_sandboxes(). No K8s API mock infrastructure exists to test the actual watch path. Signed-off-by: Roland Huß <rhuss@redhat.com> * style(driver-k8s): fix rustfmt formatting Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(driver-k8s): use explicit unit pattern for clippy 1.95 Replace `_ = tx.closed()` with `() = tx.closed()` to satisfy the `ignored_unit_patterns` clippy lint introduced in Rust 1.95. Assisted-By: 🤖 Claude Code Signed-off-by: Roland Huß <rhuss@redhat.com> --------- Signed-off-by: Roland Huß <rhuss@redhat.com> |
||
|
Mesut Oezdil
|
744a65d52e |
fix(driver-podman): avoid panic when HOME is unset on macOS (#2327)
* fix(driver-podman): resolve Podman socket via auto-detection Align Podman socket selection with the existing Docker model: explicit config wins, otherwise probe openshell-core for a responsive socket. This also fixes the original HOME-unset panic, since resolution no longer hardcodes a per-OS default path. - add detect_podman_socket() in openshell-core, mirroring detect_docker_socket - PodmanComputeConfig.socket_path is now Option<PathBuf>, no default - remove default_socket_path() (podman driver) and podman_socket_path() (vm driver), both replaced by the shared detector - update server env override and CLI for the new Option type - add tests: responsive-candidate detection in openshell-core, and config-error (not panic) when no socket is configured or reachable * fix(driver-podman): address review feedback on socket resolution Extract socket resolution into resolve_socket_path, taking the detector as a parameter so tests do not depend on real env vars or the host's actual Podman state. Replace the flaky env-mutating test with three deterministic cases: explicit wins, detected is used when absent, and neither source errors. Fix the socket_path doc comment to describe it from a config user's point of view, matching DockerComputeConfig's docstring. Drop a comment that only made sense next to the Docker driver code. Update the Podman README and gateway docs: they described a fixed per-OS default path that no longer exists, replace with the actual probe-then-fail behavior. * docs(driver-podman): simplify socket default description Previous wording was self-contradictory (says auto-detect on unset, then lists the same var as a probed candidate) and omitted the Linux /run/user/uid/podman/podman.sock candidate. |
||
|
Derek Carr
|
5952a5a23f |
feat(workspace): add workspace resource model with scoping, membershi… (#2243)
* feat(workspace): implement workspace model (Phase 1 of RFC 0011) Implements workspace and membership model providing hard isolation boundaries for multi-player OpenShell deployments. Workspace CRUD with Kubernetes-style Terminating phase for graceful deletion. All resources scoped by workspace via ObjectMeta. Membership RPCs for workspace access control. Persistence migration shifts name uniqueness to (object_type, workspace, name). Provider profiles support platform and workspace scoping. Service routing uses workspace-prefixed DNS labels. Inference routes renamed and workspace-scoped with DeleteInferenceRoute RPC. Python SDK with WorkspaceClient, two-method list pattern (workspace-scoped and for_all_workspaces), and workspace parameter on all methods. CLI workspace flags, TUI workspace cycling. K8s driver filters unmanaged CRs and uses delete preconditions. Podman driver uses immutable container IDs. Label serialization fixed across all put_if call sites. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(cli): delegate sandbox upload command to existing upload function The standalone `sandbox upload` command reimplemented upload logic inline with two bugs: it used `Path::exists()` which follows symlinks (rejecting dangling symlinks), and it ran git-aware filtering on symlink sources. The `run::sandbox_upload()` function already handles both cases correctly via `sandbox_upload_plan()`. Replace the inline logic with a call to the existing function. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(e2e): shorten sandbox names and fix test compatibility Shorten the sandbox name in initial_sparse_policy_is_acknowledged_as_loaded from 'e2e-2159-sparse-enrich' (22 chars) to 'e2e-sparse-enrich' (17 chars) to comply with MAX_ROUTABLE_NAME_LEN (19 chars). Also capture stderr in create_keep_with_args so future sandbox creation failures include the actual CLI error instead of reporting empty output. Signed-off-by: Derek Carr <decarr@redhat.com> * test(workspace): add test coverage for workspace CRUD and persistence isolation Add unit tests for workspace create happy path, get round-trip, get not-found, get empty-name rejection, already-exists error, and resolve_workspace not-found. Add persistence test proving cross-workspace name uniqueness (same name in different workspaces produces separate records). Add workspace name max-length boundary tests. Fix e2e harness to include stderr in name-parse-failure error path. Align Python e2e test_workspace_crud with try/finally pattern. Document provider profile catalog workspace scoping gap in RFC 0011. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(examples): update examples for workspace model compatibility Shorten sandbox names in demo scripts to fit the 19-character MAX_ROUTABLE_NAME_LEN limit: policy-demo prefix to pd-, multi-agent notepad derives a short SANDBOX_TAG from the run ID, governance interceptor uses gs-PID-RANDOM. Update vscode-remote-sandbox.md SSH host aliases from openshell-{name} to openshell-{name}.{workspace} format. Signed-off-by: Derek Carr <decarr@redhat.com> * feat(sdk): add workspace-scoped client and workspace CRUD Add WorkspaceScopedClient modeled after kube::Api::namespaced — captures workspace once and injects it into every sandbox request. Add workspace CRUD methods (create, get, list, delete) and list_sandboxes_all_workspaces on OpenShellClient. Extend SandboxRef with workspace field and add WorkspaceRef type. Include mock tests for all new operations. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(lint): resolve clippy warnings in workspace test assertions Signed-off-by: Derek Carr <decarr@redhat.com> * fix(docs): convert indented code blocks to fenced in RFC 0011 Signed-off-by: Derek Carr <decarr@redhat.com> * fix(lint): resolve clippy warnings and apply cargo fmt across workspace Auto-format with cargo fmt and fix clippy warnings exposed by the reformat: unnecessary qualifications, map_unwrap_or, identical match arms, unused variable prefix, dead code annotations, and let-unit-value in e2e harness. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(workspace): address workspace scoping issues from review - Add workspace field to settings JSON output (CLI) - Skip Podman containers missing workspace label instead of defaulting to empty string, matching K8s driver behavior - Add resource_version to list_by_scope SELECT in both SQLite and Postgres backends, with regression test - Gate PolicyLocalContext proposal/lookup routes on workspace readiness, returning 503 when workspace is not yet discovered - Block sandbox and provider creation in TUI all-workspaces mode - Clear workspace vectors in TUI reset_sandbox_state Signed-off-by: Derek Carr <decarr@redhat.com> * fix(workspace): make provider profile catalog workspace-aware Thread workspace through snapshot_catalog so the EffectiveProviderProfileCatalog enforces workspace boundaries on both read and write paths. UserProviderProfileSource now loads platform-scoped profiles (workspace "") plus the target workspace's profiles, preventing cross-workspace duplicate profile ID collisions that previously caused global catalog failures. Update RFC 0011 to reflect catalog scoping is implemented in Phase 1 rather than deferred to future work. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(persistence): include workspace column in atomic policy revision INSERT put_policy_revision_atomic omitted the workspace column from the INSERT into the objects table in both SQLite and Postgres backends, causing atomically-written policy revisions to lose their workspace association. Add workspace field to AtomicPolicyRevisionWrite and thread it through both backend INSERT statements, matching the non-atomic put_policy_revision path which already included it. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(proxy): skip ancestor walk when socket owner is the entrypoint collect_ancestor_identities walked the entire process tree above the entrypoint when the connecting process was the entrypoint itself, SHA256-hashing every ancestor binary (IDE, shell, container runtime). On dev machines with large binaries in the ancestor chain this exceeded the 30-second test timeout. When start_pid == stop_pid there are no intermediate ancestors to verify, so return an empty list immediately. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(workspace): make provider profile catalog scope-aware Allow the same profile ID at platform and workspace scopes by introducing layered catalog entries where workspace profiles shadow platform profiles. Add source and scope fields to the ProviderProfile proto and CLI output. Migrate List/Get handlers to the catalog, fixing divergence with runtime profile resolution. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(e2e): align podman e2e labels with centralized driver constants The podman driver moved its container labels to the centralized openshell.ai/ prefix, but the e2e test harness and cleanup script still referenced the old openshell.sandbox-* keys, causing the local_driver_token_restart test to fail on container lookup. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(e2e): align python profile isolation test with scope-aware catalog Platform profiles are now visible in workspace listings as fallbacks per the layered catalog design. Update the assertion to match. Signed-off-by: Derek Carr <decarr@redhat.com> * fix(workspace): honor profile_workspace in runtime profile resolution Runtime profile lookups now consult provider.profile_workspace via get_type_profile_for_scope. Providers created with --global-profile (profile_workspace="") resolve to the platform profile even when a workspace profile shadows the same ID. All 6 runtime call sites updated; type-only call sites remain scope-agnostic. Signed-off-by: Derek Carr <decarr@redhat.com> --------- Signed-off-by: Derek Carr <decarr@redhat.com> |
||
|
Piotr Mlocek
|
d556748771 |
feat(supervisor-middleware): add network egress middleware (#2027)
Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> |
||
|
krishicks
|
cf4deccd37 |
fix(gateway): probe Docker socket during driver auto-detection (#2303)
Previously, Docker was auto-detected when the CLI was installed or a candidate Unix socket existed. Neither check verified that the Docker API was responsive. A similar check was done when auto-detecting Podman in the past, but was replaced in |
||
|
Max Dubrinsky
|
e6f319c76d |
feat(sdk): add openshell-sdk crate (#1862)
* feat(sdk): add openshell-sdk crate Additive extraction of the shared async gRPC client core (transport, TLS, OIDC single-flight refresh, edge tunnel, high-level sandbox surface, raw escape hatch) as a new workspace crate. No existing consumers yet; CLI/TUI migration follows in a separate PR. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * refactor(sdk,core): reuse shared JWT exp decoder for refresh deadlines Extract the signature-unverified JWT exp decode out of openshell-core/grpc_client.rs into openshell_core::jwt::parse_exp_secs, and have the openshell-sdk refresh path reuse it to derive a proactive refresh deadline from a bearer JWT when the caller does not advertise expires_at. Addresses review feedback to reuse pre-existing logic rather than reimplement JWT expiry handling per client. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(sdk): harden refresh single-flight and redact tokens in Debug Addresses review feedback on the openshell-sdk refresh path. - Make the single-flight cleanup cancellation-safe. The in-flight slot is now cleared by the shared refresh computation itself (epoch-guarded) rather than the leader's post-await code. Previously, if the leader future was dropped (e.g. an FFI caller cancelling its promise) after a follower drove the refresh to completion, the completed future was stranded in the slot and later refresh_now() calls re-joined it, pinning the client to a stale or already-rejected token. Adds a regression test that cancels the leader and asserts the next refresh starts a fresh attempt. - Redact bearer secrets from Debug. RefreshedToken and the oidc RefreshTokenInput/RefreshTokenOutput now use manual Debug impls that omit the access/refresh token fields via finish_non_exhaustive, matching the house style (e.g. SecretResolver, SandboxJwtIssuer). Prevents a stray {:?} or a containing struct's derived Debug from writing tokens to logs. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(sdk): fail refresh when the new token can't be encoded as metadata store_bearer now returns an error instead of silently keeping the previous bearer value. The TokenSource commits the refreshed token to its state before the client writes it into the interceptor slot, so a silent drop left the interceptor on the old (expiring) token with no path back to a refresh. Surfacing the error fails the call loudly instead. Adds a unit test covering a token that can't be encoded as gRPC metadata. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(sdk): refresh OIDC tokens on raw routes and harden rotation Raw gRPC access never triggered OIDC refresh: a client that only used raw_grpc/raw_inference kept sending the initial bearer until it expired, with no proactive or reactive refresh. Add raw_grpc_fresh and raw_inference_fresh accessors that refresh before returning the client, plus force_refresh for reactive recovery after an Unauthenticated raw RPC. Guard the single-flight refresh commit against a concurrent replace(). The in-flight attempt now records the generation it started from and skips its write when an external replace() has advanced it, so timer or callback driven rotation is no longer clobbered by a slower refresh. Remove TokenSource::snapshot(): it returned an empty string under write contention and had no consumer on the CLI/TUI path. Tests read committed state directly instead. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * docs(sdk): rewrite crate README for consumers Recast the openshell-sdk README as a usable crate README rather than an RFC excerpt. Drop the Responsibilities/Non-responsibilities/Consumers scope-boundary sections and the mTLS migration rationale, folding the useful facts (explicit token, no disk/name resolution, Refresh trait, SdkError mapping) into the intro, a new Auth and refresh section, and Public surface. Remove the dead relative RFC link and status-label prose so the doc renders cleanly wherever it is published. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(sdk): address review feedback on OIDC refresh and transport Apply Drew's review notes on PR #1862: - Drop unused `rustls-pemfile` dependency and move `tokio-stream` to dev-dependencies (only used by tests). - Guard OIDC `expires_at` against u64 overflow with `saturating_add`. - Fix stale `#[non_exhaustive]` rationale in `AuthConfig` (the struct `Oidc` variant it described as future already ships). - Stop double-wrapping refresh errors: store the bare refresh-error text so the single `SdkError::auth` wrap happens once at await. - Strip stale CLI porting breadcrumbs from `build_channel` docs, keeping the branch table. - Treat proactive token refresh as best-effort: a transient failure falls through to the request instead of failing an RPC whose current token is still valid, with a regression test. - Collapse `exec`'s inline auth retry into the shared `unary` helper. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> * fix(sdk): preserve transient/terminal distinction in refresh errors The refresh single-flight collapsed both `RefreshError::Transient` and `RefreshError::Terminal` into a stringified `SdkError::Auth`, so consumers (CLI, TUI, future language bindings) had no machine-readable way to tell a retryable IdP blip from a dead session that needs re-authentication. Carry the `RefreshError` through the shared outcome (kept `Clone` for `Shared`) instead of its rendered text, and map it at the await site to a new `retryable` flag on `SdkError::Auth`. Add `SdkError::auth_retryable` and a `SdkError::retryable()` accessor; transient refresh failures report `true`, every other error `false`. Add classification tests. Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> --------- Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com> |
||
|
Drew Newberry
|
83003e80fc |
feat(interceptors): initial gateway interceptor implementation and reference example (#2005)
* feat(gateway): add descriptor-driven interceptors Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(gateway): add service-reflected interceptors Signed-off-by: Drew Newberry <anewberry@nvidia.com> * wip * fix(gateway): harden interceptor evaluation Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(interceptors): label metrics and harden governance smoke Signed-off-by: Drew Newberry <anewberry@nvidia.com> * remove on_error: ignore * feat(gateway-interceptors): emit log annotations Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(examples): govern provider profiles in interceptor Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway): preserve update config annotations Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(providers): support interceptor profile catalogs Signed-off-by: Drew Newberry <anewberry@nvidia.com> * wip * feat(governance-interceptor): sign provider profiles Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(providers): use configured profile sources for refresh updates Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(gateway-interceptors): add phase-specific evaluation payloads Signed-off-by: Drew Newberry <anewberry@nvidia.com> * feat(providers): compose provider profile sources Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway-interceptors): preserve committed responses Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway-interceptors): reject ambiguous protobuf oneofs Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway-interceptors): validate patch candidates per binding Signed-off-by: Drew Newberry <anewberry@nvidia.com> * refactor(gateway-interceptors): use reflected protobuf codec Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(governance-example): canonicalize signed protobuf hashes Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway): commit policy provenance atomically Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway): close signed governance bypasses Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway): isolate interceptor secrets and authority Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway): snapshot provider profiles per request Signed-off-by: Drew Newberry <anewberry@nvidia.com> * chore(gateway): resolve server clippy warnings Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(server): satisfy provider source clippy lint Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(server): initialize policy test annotations Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(gateway-interceptors): require explicit route allowlist Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(proto): clarify update annotation semantics Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com> |
||
|
Tony Luo
|
40194f935e |
fix(network): fail closed when credential placeholders cannot be rewritten (#2162)
* fix(network): fail closed when credential placeholders cannot be rewritten When the credential rewriter degrades internally, the proxy forwarded the literal `openshell:resolve:env:<NAME>` placeholder (or its provider alias marker) to the upstream instead of the resolved secret, leaking the reserved token on the wire and causing upstream auth failures (#2161). Two fail-open paths are closed: - secrets: `rewrite_http_header_block` returned the header block verbatim when no `SecretResolver` was available, so the fail-closed marker scan (which ran only on the resolved path) never saw the placeholder. It now scans the header region for reserved markers even with no resolver and returns `UnresolvedPlaceholderError` when one is present. Marker-free traffic still passes through unchanged. - proxy: when TLS was detected on a CONNECT but `tls_state` was `None` (ephemeral CA generation or CA file write failed at startup), the handler fell back to a raw `copy_bidirectional` tunnel, bypassing credential rewrite. Inside the proxy handler `tls_state` is `None` only on CA-init failure (`mode != Proxy` never starts the handler, and `tls: skip` is handled earlier), so it now refuses the connection with a 503 and a High-severity denial event instead of tunneling. The two startup CA-failure logs are raised from Medium to High. Tests: resolver=None with a placeholder in the request line, a header value, and the provider-alias form now fail closed; marker-free passthrough is unchanged; the relay integration test asserts the request is rejected before any byte reaches upstream; and the 503 fail-closed response contract is locked. Signed-off-by: Tony Luo <xialuo@nvidia.com> * fix(network): refuse CONNECT before 200 when TLS termination is unavailable The fail-closed refusal for a terminating CONNECT with no TLS termination state (ephemeral CA init failed) was written after the 200 Connection Established response. Because a CONNECT client only sends its TLS ClientHello after reading the 200, the peek-based TLS detection is inherently post-200, so the 503 landed inside the established tunnel and surfaced to the client as a TLS protocol error rather than a readable status. An 'allowed CONNECT' event was also logged first. Move the decision to a pre-200 gate: query_tls_mode resolves purely from the policy decision + host/port (no peeked bytes), so the route's TLS treatment is known before the tunnel is acknowledged. When TLS state is absent and the route is not tls: skip, write the 503 as the first bytes on the socket, emit the High-severity Denied event, and close. tls: skip routes tunnel raw exactly as before, and no allowed-CONNECT event is emitted on the refusal path. The now-unreachable post-200 branch is kept as defense in depth but no longer writes an in-tunnel 503; it fails closed by dropping the connection instead. Add connection-level regression tests over a real loopback socket: the gate refuses with HTTP/1.1 503 as the first bytes for a terminating route, and writes nothing when TLS termination is present or the route is tls: skip. Signed-off-by: Tony Luo <xialuo@nvidia.com> * fix(network): order the CONNECT TLS-unavailable refusal after SSRF Addresses the gator re-check on #2162. Ordering: the pre-200 fail-closed refusal ran before SSRF/allowed_ips validation, so during CA-init failure an internal-address CONNECT got a 503 tls_termination_unavailable instead of the normal 403 ssrf_denied, weakening operator visibility in degraded state. The SSRF branches now return validated addresses; the refusal runs after that validation (an internal address has already been denied with 403) but still before the upstream connect and before 200 Connection Established. effective_tls_skip is still resolved up front since the refusal consumes it. Tests: add connection-level regressions through the real handle_tcp_connection, driving a CONNECT from a child /bin/bash copy so the /proc process-identity binding resolves it against a permissive policy (the hot-swap test's identity pattern). They assert the first bytes are HTTP/1.1 503 for a terminating route with no TLS state, a 403 (not 503) for an internal address, and no refusal for a tls: skip route. These are gated to Linux at runtime (evaluate_opa_tcp needs /proc); a companion test verifies the OPA policy shape (glob allow, tls mode) on every platform so the precondition is locked where /proc is unavailable. rest.rs: tighten the fail-closed relay test to assert the forwarded buffer is_empty() rather than merely lacking the placeholder/secret. Signed-off-by: Tony Luo <xialuo@nvidia.com> * test(network): keep the CONNECT handler test client fork-free The handler regression tests forked cat to read the proxy reply, so the client socket fd was inherited by a second process with a different binary. The identity resolver correctly denies that as ambiguous shared-socket ownership (the same invariant resolve_process_identity_denies_fork_exec_shared_socket_ambiguity pins), so the tests exercised the deny path instead of the allow path — and on busy CI runners the deny-path /proc fallback scan exceeded the test budget and looked like a hang. The client script now uses only bash builtins (exec, printf, read -d '') so exactly one process owns the socket, and the child is left to exit on EOF instead of being killed mid-read. Signed-off-by: Tony Luo <xialuo@nvidia.com> * test(network): drive the CONNECT handler tests with an in-process client The child-process client (even fork-free) made the handler tests environment-sensitive: on CI runners with a busy or restricted /proc, resolving the child's socket ownership degraded into the whole-/proc fallback scan and a deny, which surfaced as a hang. The client is now an in-process TcpStream and the test policy allows current_exe(), so identity resolution binds the socket to the test process itself in the descendant scan — the same in-process pattern the passing resolve_process_identity tests rely on. The tls: skip test additionally asserts that the handler emitted no DenialEvent at any stage, so it can no longer pass vacuously on a policy or identity deny. Refusal budgets widened to 30s as a belt for slow runners; the refusals themselves return in milliseconds. Signed-off-by: Tony Luo <xialuo@nvidia.com> * test(core): pin the percent-encoded marker no-resolver fail-closed path The no-resolver scan already catches the percent-encoded canonical marker through its decoded pass; this regression pins it: a request line carrying openshell%3Aresolve%3Aenv%3AKEY with no resolver must fail closed with UnresolvedPlaceholderError { location: header }. Signed-off-by: Tony Luo <xialuo@nvidia.com> --------- Signed-off-by: Tony Luo <xialuo@nvidia.com> |
||
|
mjamiv
|
614c8c164d |
feat(kubernetes): support PVC subPath driver config (#2034)
* feat(kubernetes): support PVC subPath driver config Signed-off-by: mjamiv <michael.commack@gmail.com> * test(kubernetes): cover writable PVC driver config Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(kubernetes): address PVC subPath review feedback Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(kubernetes): address PVC config review follow-up Signed-off-by: mjamiv <michael.commack@gmail.com> * fix(kubernetes): address PVC review follow-ups --------- Signed-off-by: mjamiv <michael.commack@gmail.com> |
||
|
Taylor MutchandSeth Jennings
|
8eacb4779f |
feat(kubernetes): add sidecar supervisor topology (#2076)
* feat(kubernetes): add sidecar supervisor topology Add the Kubernetes sidecar supervisor topology, its Helm/Skaffold configuration, topology documentation, and sidecar e2e matrix coverage. Skip root-only sandbox identity rewriting when process enforcement is network-only so the low-permission sidecar process container can start successfully. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid similar process id names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid similar process id names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(sandbox): avoid similar proxy id names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * docs(kubernetes): clarify sidecar topology limits Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): keep sidecar process leaf capless Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): refresh sidecar provider env snapshots Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * test(supervisor): align hot-swap identity regression Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): stage sidecar mtls files before proxy chown Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): simplify sidecar supervisor topology Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * chore(helm): reuse sidecar skaffold values Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid similar iptables helper names Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(e2e): harden kube gateway wrapper setup Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(supervisor): avoid nft batch rollback on OCP Run nftables setup as individual commands so optional conntrack and log expressions can fail without rolling back required table, chain, and reject rules. Signed-off-by: Seth Jennings <sjenning@redhat.com> Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): preserve process identity in sidecar topology Render sidecar pods with a shared process namespace, keep binary-aware network policy enabled, and move Kubernetes sidecar settings under the nested sidecar config table. Also apply unprivileged Landlock/seccomp setup in NetworkOnly supervisor mode so sidecar topology keeps sandbox child hardening without privileged process setup. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * refactor(kubernetes): replace sidecar snapshots with control socket Coordinate sidecar policy and provider bootstrap over a local Unix socket so the process leaf no longer reads policy/provider snapshot files. Report entrypoint startup through the control channel and keep gateway credentials confined to the network sidecar. Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * feat(kubernetes): support relaxed sidecar network identity Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(sandbox): satisfy sidecar clippy lint Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * refactor(kubernetes): standardize topology naming Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(sandbox): satisfy linux clippy timeout import Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): support kata sidecar on ipv4 pods Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): satisfy linux clippy for sidecar fallback Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * chore(kubernetes): remove stale supervisor topology references Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): enable sidecar binary policy inspection Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): harden sidecar control boundary Signed-off-by: Taylor Mutch <taylormutch@gmail.com> * fix(kubernetes): couple sidecar supervisor lifecycles Signed-off-by: Taylor Mutch <taylormutch@gmail.com> --------- Signed-off-by: Taylor Mutch <taylormutch@gmail.com> Signed-off-by: Seth Jennings <sjenning@redhat.com> Co-authored-by: Seth Jennings <sjenning@redhat.com> |
||
|
Florent BENOIT
|
10702133a3 |
fix(core): pin supervisor image tag to gateway version for all drivers (#2070)
* fix(core): pin supervisor image tag to gateway version for all drivers The Podman and Kubernetes drivers defaulted the supervisor image to `:latest` via DEFAULT_SUPERVISOR_IMAGE, while the Docker driver already resolved a version-pinned tag. Extract the tag resolution logic into openshell-core so all three drivers use the same OPENSHELL_IMAGE_TAG > IMAGE_TAG > CARGO_PKG_VERSION priority chain. Closes #2068 Signed-off-by: Florent Benoit <fbenoit@redhat.com> * refactor(core): simplify supervisor image tag resolver to slice-based API Remove the Docker driver's wrapper functions and call openshell_core::config::default_supervisor_image() directly. Simplify resolve_supervisor_image_tag to accept &[&str] instead of three separate parameters. Signed-off-by: Florent Benoit <fbenoit@nvidia.com> Signed-off-by: Florent Benoit <fbenoit@redhat.com> --------- Signed-off-by: Florent Benoit <fbenoit@redhat.com> Signed-off-by: Florent Benoit <fbenoit@nvidia.com> |
||
|
Kyle ZhengandJohn Myers
|
ff9af8e320 |
fix(sandbox): acknowledge initial policy revision; expose SDK labels/selectors (#2170)
* fix(sandbox): acknowledge initial policy revision The supervisor loaded and enforced a sandbox-scoped policy but never told the gateway which revision it loaded. The policy poll loop seeded itself with the initial revision's hash on its first poll, so `policy_changed` was never true for that revision and `ReportPolicyStatus(LOADED)` — which only ran in the hot-reload branch — was never called. The revision stayed `Pending` and `current_policy_version` stayed 0 even though the sandbox was `Ready` and the policy was effective. This was most visible with sparse policies that get baseline-enriched into a new revision during startup. After the OPA engine is constructed, report the exact sandbox revision the supervisor loaded as LOADED, and seed the poll loop from that revision so it is not re-reported. Report FAILED with the original construction error if engine construction or conversion fails. Only sandbox-sourced revisions (version > 0) whose canonical content matches the loaded policy are acknowledged; global and local-file policies are untouched. Delivery uses the shared bounded retry, is non-fatal on transient failure, and a pending initial acknowledgement is delivered before any newer revision so policy history is never reordered. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * feat(python): expose sandbox labels and selectors The gateway protobuf and CLI already support request-level sandbox labels (`CreateSandboxRequest.name`/`labels`) and selector-based listing (`ListSandboxesRequest.label_selector`), but the public Python SDK dropped them, so Python-created sandboxes could not be found via `openshell sandbox list --selector ...`. Add optional, source-compatible `name`/`labels` to `SandboxClient.create`, `create_session`, and the high-level `Sandbox`, and `label_selector` to `list`/`list_ids`. `SandboxRef` now carries the gateway labels as an immutable mapping (default empty, so `SandboxRef(id, name, status)` still works). Caller-provided label mappings are copied. Attaching the high-level `Sandbox` to an existing sandbox rejects `name`/`labels` since creation metadata cannot change on attach. Template labels remain a separate concept. No protobuf changes are required. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * fix(python): keep SandboxRef hashable and copy high-level labels Excluding the new immutable `labels` field from SandboxRef equality/hash (`compare=False`) preserves the original (id, name, status) identity and keeps the frozen dataclass hashable — a MappingProxyType field would otherwise make `hash(SandboxRef(...))` raise. Also defensively copy caller-provided labels in the high-level `Sandbox` so later caller mutation cannot change what is sent. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * fix(sandbox): bound initial-policy-ack retries The poll loop retried a pending initial acknowledgement before processing any newer revision, but retried unconditionally forever. A permanently undeliverable ack (e.g. the revision was superseded before it could be reported) would then stall all later policy hot-reloads and provider-env refreshes. Cap the retries; after the bound, give up and resume normal polling so the loop cannot livelock on a stuck acknowledgement. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * test(sandbox): add sparse-policy revision-2 acknowledgement e2e Regression for #2159: create a sandbox with the network-only policy-advisor fixture, which the supervisor enriches with baseline filesystem paths during startup (creating revision 2, superseding revision 1). Assert the effective policy reaches revision 2 and no revision remains Pending once the supervisor acknowledges the load. Adds SandboxGuard::create_keep_with_args to create a kept sandbox with an initial --policy. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * fix(ci): correct sandbox checks Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * test(cli): serialize mTLS environment access Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * fix(sandbox): address policy review feedback Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * fix(sandbox): preserve exact policy acknowledgements Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> * fix(sandbox): preserve local policy overrides Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> --------- Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com> Co-authored-by: John Myers <9696606+johntmyers@users.noreply.github.com> |
||
|
Seth Jennings
|
6461677c32 |
feat(policy): accept numeric UIDs for sandbox process identity (#1973)
* feat(policy): accept numeric UIDs in sandbox process identity validation Allow run_as_user and run_as_group to be either the literal 'sandbox' or a numeric UID/GID within [1000, 2_000_000_000]. This removes the hard dependency on a baked-in 'sandbox' user in container images, enabling compute drivers to inject resolved UIDs at sandbox creation. Phase 1 of #1959. Signed-off-by: Seth Jennings <sjenning@redhat.com> * feat(supervisor): accept numeric UIDs for process identity dropping Allow run_as_user and run_as_group to be numeric UIDs/GIDs, removing the hard dependency on a baked-in 'sandbox' user in container images. Changes: - validate_sandbox_user(): accepts numeric UIDs without passwd lookup (logs OCSF event); keeps passwd check for "sandbox" name; rejects non-numeric non-sandbox strings that fail passwd lookup - prepare_filesystem(): passes numeric UIDs/GIDs directly to chown() instead of requiring a passwd entry - drop_privileges(): resolves numeric UIDs/GIDs directly via UID::from_raw / Gid::from_raw; skips initgroups when target uid matches current euid; uses guard conditions before setgid/setuid calls - session_user_and_home(): falls back to ("{uid}", "/sandbox") for numeric UIDs, avoiding a passwd lookup that will fail Re-exports MIN_SANDBOX_UID and MAX_SANDBOX_UID from openshell-policy so callers have consistent range constants. Phase 2 of #1959. Signed-off-by: Seth Jennings <sjenning@redhat.com> * feat(driver-kubernetes): resolve sandbox UID/GID from config or OpenShift SCC annotations Phase 3 of the numeric-UID plan: allow operators to specify explicit sandbox_uid/sandbox_gid in Kubernetes driver config, auto-detect from OpenShift SCC namespace annotations, and propagate resolved values to supervisor container env vars and PVC init container securityContext. Changes: - Add sandbox_uid/sandbox_gid fields to KubernetesComputeConfig - Add SANDBOX_UID/SANDBOX_GID env var constants to openshell-core - Implement resolve_sandbox_identity() to fetch namespace annotations and auto-detect OpenShift SCC UID ranges (sa.scc.uid-range) - Pass resolved UID/GID through SandboxPodParams to pod spec builder - Inject SANDBOX_UID/SANDBOX_GID env vars into supervisor container - Update PVC init container securityContext with resolved UID/GID instead of hard-coded root - Add comprehensive unit tests for resolution logic and annotation parsing (resolve_sandbox_uid, resolve_sandbox_gid, OpenShift SCC annotation parsing) Signed-off-by: Seth Jennings <sjenning@redhat.com> * feat(driver-vm): add configurable sandbox UID/GID and update docs/examples Phase 4 of the numeric-UID plan: replace hardcoded SANDBOX_UID (10001) in VM rootfs preparation with configurable sandbox_uid/sandbox_gid fields. Changes: - Add sandbox_uid/sandbox_gid to VmDriverConfig with serde derives - Pass resolved UID/GID through prepare_sandbox_rootfs_from_image_root to ensure_sandbox_guest_user which writes /etc/passwd/group/gshadow - Update BYOC Dockerfile: remove groupadd/useradd, document runtime UID injection and the ability to skip baked-in sandbox user - Update gateway-config.mdx: document sandbox_uid/sandbox_gid for both Kubernetes (with OpenShift SCC autodetection) and VM drivers - Update sandbox-compute-drivers.mdx: add Sandbox User Identity section explaining numeric UID support across all compute drivers - Update rootfs tests to use non-default UIDs, verify config passthrough Signed-off-by: Seth Jennings <sjenning@redhat.com> * code review changes * fix(supervisor): harden tests for restricted CI container environments Guard tests against CI-specific constraints: root without CAP_SETPCAP, UIDs with no /etc/passwd entry, and restricted /proc access. Signed-off-by: Seth Jennings <sjennings@nvidia.com> Signed-off-by: Seth Jennings <sjenning@redhat.com> --------- Signed-off-by: Seth Jennings <sjenning@redhat.com> Signed-off-by: Seth Jennings <sjennings@nvidia.com> |
||
|
Florian Bergmann
|
43bb030266 |
feat(docker,podman): add SELinux label support for bind mounts (#2092)
* feat(docker,podman): add SELinux label support for bind mounts The Docker Engine structured Mount API does not support SELinux relabelling (:z / :Z). Move user-supplied bind mounts from the structured `mounts` field to the legacy string-format `binds` field, which does support these options. Add a shared `SelinuxLabel` enum (shared/private) to openshell-core so both Docker and Podman drivers accept an optional `selinux_label` field on bind mount configs. For Docker, labels are appended to the bind string; for Podman, they are pushed to the mount options vec. Signed-off-by: Florian Bergmann <fbergman@redhat.com> * fix(docker): reject missing bind source paths on legacy binds Moving user bind mounts from the structured Mount API to the legacy Binds field changed Docker's behavior for missing source directories: the legacy path silently creates them as empty root-owned dirs instead of erroring. Add an explicit Path::exists() check to preserve the fail-fast behavior operators expect. Signed-off-by: Florian Bergmann <fbergman@redhat.com> --------- Signed-off-by: Florian Bergmann <fbergman@redhat.com> |
||
|
Evan Lezar
|
450685c755 |
fix(drivers): reject whitespace in mount fields (#2086)
Signed-off-by: Evan Lezar <elezar@nvidia.com> |
||
|
Evan Lezar
|
0a25fdf525 |
refactor(core): remove unused extra bind addresses (#2059)
Signed-off-by: Evan Lezar <elezar@nvidia.com> |
||
|
John T. MyersandJohn Myers
|
f27ff1507c |
fix(providers): reserve credential placeholder revisions (#2049)
* fix(providers): reserve credential placeholder revisions Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * fix(providers): share placeholder namespace parser Signed-off-by: John Myers <johntmyers@users.noreply.github.com> * test(providers): cover non-revision env key Signed-off-by: John Myers <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> |
||
|
st-grandEvan Lezar
|
75a317ea45 |
feat(server): support out-of-tree compute drivers via --compute-driver-socket (#1703)
* feat(server): support remote compute driver endpoints Add named remote compute driver endpoint support to the gateway. Remote drivers are selected by a non-reserved compute driver name and either a CLI/env socket endpoint or [openshell.drivers.<name>].socket_path. The VM driver now enters ComputeRuntime through the same acquired remote endpoint path, while Docker, Podman, and Kubernetes retain their in-process drivers. Require --drivers/OPENSHELL_DRIVERS when pairing an ad-hoc socket endpoint so the socket does not imply a magic driver name, and keep reserved in-tree names unavailable for unmanaged socket endpoints. Co-authored-by: Evan Lezar <elezar@nvidia.com> Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> * test(server): cover remote compute driver UDS lifecycle Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> Co-authored-by: Evan Lezar <elezar@nvidia.com> |
||
|
Simon Scatton
|
c7879a0a1d |
test(e2e): stop using custom e2e binary builds (#2000)
Remove the production-crate dev-settings feature so e2e tests build the same gateway, CLI, and supervisor binaries that we ship. Move settings coverage from dummy test-only keys to the production ocsf_json_enabled setting. Signed-off-by: Simon Scatton <sscatton@nvidia.com> |
||
|
Evan Lezar
|
2c545893ed |
feat(cli): add GPU count requests (#1812)
* feat(gpu)!: add resource requirements BREAKING CHANGE: SandboxSpec.gpu and DriverSandboxSpec.gpu were replaced with resource_requirements.gpu, changing protobuf field 9 from a bool to a message for both public and driver APIs. Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(gpu): pass requirements through sandbox create Pass the coupled GPU requirement object through the CLI sandbox_create boundary instead of splitting presence and count into separate arguments. Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(gpu): pass requirements to timeout message Pass ResourceRequirements into the provisioning timeout message helper so GPU hints are derived from the same nested request object used to create the sandbox. Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(gpu): pass driver requirements through helpers Thread Option<GpuResourceRequirements> through driver validation and rendering helpers instead of splitting GPU presence and count into separate arguments. Signed-off-by: Evan Lezar <elezar@nvidia.com> * fix(gpu): validate exact device requests Require exact driver GPU device lists to be tied to a GPU request, allow a single exact device to use the default countless request, and require explicit matching counts for multi-device lists. Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com> |
||
|
st-grandClaude Opus 4.7
|
4ee27d9955 |
feat(sandbox,providers): add aws-bedrock as a recognized inference provider (#1704)
* feat(sandbox): allow AWS Bedrock InvokeModel paths through the L7 router
Adds two patterns to `default_patterns()` so the supervisor's L7
inference router recognizes the Bedrock InvokeModel URL shape and
forwards matched requests to the registered upstream:
- `POST /model/{modelId}/invoke` → aws_bedrock_invoke
- `POST /model/{modelId}/invoke-with-response-stream` → aws_bedrock_invoke_stream
The `{modelId}` segment is wildcarded by extending `detect_inference_pattern`
to handle one middle `/*/` segment in addition to the existing trailing
`/*`. The wildcard is constrained to a single non-empty path segment to
avoid path-traversal liabilities — `/model//invoke` and `/model/a/b/invoke`
both no-match.
Without this, sandboxes running Claude Code in its native Bedrock mode
(`CLAUDE_CODE_USE_BEDROCK=1`, `ANTHROPIC_BEDROCK_BASE_URL`, AWS-style
auth) hit the supervisor with `403 connection not allowed by policy`
because their URL doesn't match `/v1/*` shapes. The fix unblocks
operators wanting to register direct AWS Bedrock, an in-cluster
Bedrock-compatible bridge, or a Bedrock-emulating LiteLLM as
`--type aws-bedrock` providers.
Tests cover: positive matches for invoke + invoke-with-response-stream,
query-string handling, GET rejection, empty-segment rejection,
multi-segment rejection, and unknown-action rejection.
Companion changes (provider discovery spec + YAML profile) follow in
the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* feat(providers): add aws-bedrock provider profile + discovery spec
Adds `aws-bedrock` to the built-in provider catalog so operators can
run `openshell provider create --type aws-bedrock --credential ...`
and have the gateway treat it as a first-class inference provider
alongside `anthropic`, `openai`, etc.
- `providers/aws-bedrock.yaml`: YAML profile declaring four credentials
(AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_SESSION_TOKEN, AWS_REGION).
Default endpoint is `bedrock-runtime.us-east-1.amazonaws.com:443`;
operators in other regions or running against a Bedrock-compatible
proxy override via the operator-supplied `BEDROCK_BASE_URL` config-key
(mirrors `ANTHROPIC_BASE_URL` for the `anthropic` provider).
- `crates/openshell-providers/src/providers/aws_bedrock.rs`: the
`ProviderDiscoverySpec` so `openshell provider create --auto-providers`
picks up AWS_* env vars from local credentials.
- `crates/openshell-providers/src/providers/mod.rs`: register the module.
- `crates/openshell-providers/src/lib.rs`: register the SPEC in the
default registry alongside the other providers.
- `crates/openshell-providers/src/profiles.rs`: include the new YAML in
`BUILT_IN_PROFILE_YAMLS`.
What this PR explicitly does NOT add (intentionally separated for
review-size reasons; will follow up):
- A SigV4 signer in `openshell-router`. The current change simply
declares the protocol; a follow-up PR adds outbound SigV4 signing
using the `aws-sigv4` crate and a new `auth_style: sigv4` validator
branch in profiles.rs. Operators who don't need SigV4 (e.g. an
in-cluster bridge that ignores it and authenticates separately to
the upstream) can use this PR today.
- Body translation between Bedrock InvokeModel shape and other
inference shapes. The router treats Bedrock requests as opaque
pass-through; if the operator's upstream is real AWS Bedrock it
speaks Bedrock natively, if it's a translating bridge the bridge
does any conversion server-side.
- `BEDROCK_BASE_URL` placeholder substitution in the YAML loader.
Today the YAML's `host` is a literal default; operators override
with the config-key the same way `ANTHROPIC_BASE_URL` works.
Tested: `cargo test -p openshell-providers` (35 tests green) and
`cargo test -p openshell-sandbox --lib l7::inference` (40 tests green
including the seven new aws_bedrock cases from the previous commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* revert(providers): drop legacy aws-bedrock SPEC, rely on v2 YAML profile
Addresses johntmyers's review on NVIDIA/OpenShell#1704: net-new
providers should land via the v2 YAML profile only and should NOT
require changes to the legacy `ProviderDiscoverySpec` registry.
- Delete `crates/openshell-providers/src/providers/aws_bedrock.rs`
(the legacy SPEC + `test_discovers_env_credential!` invocation).
- Drop `pub mod aws_bedrock;` from `crates/openshell-providers/src/providers/mod.rs`.
- Drop `registry.register(providers::aws_bedrock::SPEC)` from
`crates/openshell-providers/src/lib.rs`.
Kept:
- `providers/aws-bedrock.yaml` and the `include_str!` in
`BUILT_IN_PROFILE_YAMLS` (`profiles.rs`) — the v2 path.
`discover_from_profile()` (`crates/openshell-providers/src/discovery.rs`)
picks up AWS_* env vars via `discovery.credentials` in the YAML.
- L7 router patterns in `crates/openshell-sandbox/src/l7/inference.rs`
— orthogonal to the provider registry.
The discovery test in the deleted file goes with it; v2 doesn't have
an established per-provider env-var-pickup unit test pattern, and
other YAML-only registrations (none today, but this is the new
direction) won't carry one either.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(providers): include aws_session_token in discovery + update profile assertion
Two fixes from johntmyers's gator-agent re-check on NVIDIA/OpenShell#1704:
1. `providers/aws-bedrock.yaml`: add `aws_session_token` to
`discovery.credentials`. The credential is declared in the profile
but was missing from the discovery scan list, so Providers v2
`--from-existing` would silently drop temporary AWS credentials
(STS / IRSA scenarios).
2. `crates/openshell-server/src/grpc/provider.rs`: update the static
`list_provider_profiles_returns_built_in_profile_categories`
assertion to include `aws-bedrock` at alphabetical position 0.
Adding `providers/aws-bedrock.yaml` to BUILT_IN_PROFILE_YAMLS made
the prior `["claude-code", "github", "nvidia"]` expectation stale.
Remaining blockers from the same review (deferred to follow-up
commits): `inference::profile_for` registration for aws-bedrock,
user-facing provider + inference-routing docs, and an
`upsert_cluster_inference_route` integration test.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* feat(inference): register aws-bedrock profile (bridge-fronted) + docs
Addresses johntmyers's blocking review feedback on PR #1704:
"aws-bedrock still is not wired into the managed inference.local route
registry. profile_for only registers openai, anthropic, and nvidia, so
inference set --provider <aws-bedrock-provider> will reject this
provider before the new sandbox L7 patterns can be used."
Approach: register aws-bedrock as a *bridge-fronted* upstream — the
router does not inject any auth header on outbound requests; the
configured BEDROCK_BASE_URL is expected to point at a translating
bridge / Bedrock-compatible proxy that handles auth in its own pod.
This is the shape the L7 patterns commit (8b30211a) and the YAML
profile (6b51e1a6) were designed for. SigV4 signing for direct AWS
Bedrock is a separate follow-up; see PR thread.
Changes:
- core::inference::AuthHeader: add `None` variant for upstreams that
authenticate themselves.
- core::inference: add AWS_BEDROCK_PROFILE static + register in
profile_for. Default base URL is bedrock-runtime.us-east-1, override
via BEDROCK_BASE_URL config-key (mirrors ANTHROPIC_BASE_URL pattern).
Empty credential_key_names + auth: None means no router-side
credential lookup at route time.
- router::backend: handle AuthHeader::None as a no-op (skip auth
injection).
- server::inference::resolve_provider_route: gate find_provider_api_key
on auth != None. aws-bedrock providers with empty credentials now
resolve cleanly. Updated the unsupported-type error message to
include aws-bedrock in the supported list.
- server::inference tests: add positive
upsert_cluster_route_succeeds_for_aws_bedrock_without_api_key test
covering the new code path end-to-end (provider with empty creds +
BEDROCK_BASE_URL config → upsert succeeds → resolved route has
empty api_key + provider_type aws-bedrock + bridge URL).
- core::inference tests: profile_for_known_types covers aws-bedrock,
case-insensitive lookup, plus three new aws-bedrock-specific tests
(auth: None, no credential keys, bedrock-specific protocols).
- docs/sandboxes/inference-routing.mdx: header forwarding row
mentions aws-bedrock has no passthrough headers; new tabs in
Supported API Patterns (InvokeModel + InvokeModelWithResponseStream)
and Create a Provider (with the bridge-fronted shape note + SigV4
deferral).
- docs/sandboxes/manage-providers.mdx: new row in Supported Provider
Types table; new row in Supported Inference Providers table.
Verification (in dev container):
- cargo check -p openshell-core -p openshell-router -p openshell-server: clean
- cargo test -p openshell-core --lib inference: 14/14 pass (incl. 3 new)
- cargo test -p openshell-server --lib inference::tests::upsert: 6/6 pass
(incl. new aws-bedrock test)
- cargo fmt --check: clean
- cargo clippy --all-targets -D warnings: clean
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(aws-bedrock): bridge-only YAML, doc actual cmd shape, neg test
Addresses four findings from gator-agent's #1704 re-check on 4ab587f1:
- **Item 5** (YAML collects unused AWS creds): mark all four AWS
credentials `required: false` and clear `discovery.credentials`.
Bridge-fronted routing intentionally does not consume AWS
credentials, so `--from-existing` no longer scans for them. The
credentials remain in the schema (not deleted) so the SigV4
follow-up can flip them back without a schema migration. Added a
multi-line description that names the bridge-fronted shape and the
SigV4 deferral so readers don't have to cross-reference the PR
thread.
- **Item 3** (docs show command that the CLI rejects): rewrite the
Create-a-Provider example for AWS Bedrock to use the actual
required shape — placeholder `--credential AWS_ACCESS_KEY_ID=
unused-bridge-fronted-shape` plus the `--config BEDROCK_BASE_URL`.
The placeholder satisfies the gRPC handler's
`provider.credentials.is_empty()` rejection without expanding
server-side validation; the router ignores it on the outbound path
because `auth: AuthHeader::None` skips header injection. Operators
see a clearly-labeled placeholder in `provider get` output.
- **Item 1** (validator probe): document `--no-verify` as required
for `openshell inference set --provider <aws-bedrock>` since the
default validation probe doesn't recognize the
`aws_bedrock_invoke` / `aws_bedrock_invoke_stream` protocols. Doc
now shows the full `provider create` + `inference set --no-verify`
flow with rationale for both decisions inline.
- **Item 6** (docs polish): `inference-routing.mdx` summary row now
lists AWS Bedrock alongside NVIDIA, Anthropic, Vertex AI, and
OpenAI-compatible providers, with the bridge-fronted caveat
inline.
Test additions in `crates/openshell-server/src/inference.rs`:
- Renamed the existing aws-bedrock test from
`..._without_api_key` to `..._with_bridge_url` and updated it to
use a placeholder credential (mirroring the doc-recommended
pattern operators will copy-paste). The `auth: None` path still
produces an empty `api_key` on the resolved route — the test now
documents that the credential is *stored* but not *used*.
- Added `upsert_cluster_route_rejects_aws_bedrock_without_bedrock_base_url`:
the negative half of johntmyers' "successfully used by
upsert_cluster_inference_route or intentionally rejected with a
clear documented error" ask. With
`default_base_url: ""` and no `BEDROCK_BASE_URL` config, route
resolution returns `InvalidArgument` naming the missing base_url
rather than silently forwarding prompts to AWS Bedrock with no
usable auth.
Verification (in dev container):
- cargo test -p openshell-core --lib inference: 18/18 (incl. 3 new)
- cargo test -p openshell-server --lib inference::tests::upsert: 8/8
(incl. 2 new aws-bedrock cases — positive + negative)
- cargo fmt --check: clean
- cargo clippy --all-targets -D warnings: clean
Item 2 (router-side enforcement of operator-configured Bedrock model
path, replacing the current verbatim path forwarding + body-only
model rewrite) is the remaining blocker and is genuinely separable —
it touches the L7 router with streaming-aware test coverage.
Deferring to its own commit so the security-critical change gets the
review attention it deserves.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(router): enforce operator-configured Bedrock model in request path
Closes the security-blocking item from gator-agent's #1704 re-check
on 4ab587f1: "Bedrock carries the model id in /model/{modelId}/invoke,
but the router currently forwards the caller's original path and only
rewrites JSON body model. That lets sandbox code choose a different
upstream model than the operator-configured route model, and may also
mutate native Bedrock request bodies incorrectly."
Two changes in `prepare_backend_request`:
1. **Path rewrite for Bedrock routes.** Before computing the upstream
URL, parse the inbound path's `/model/<id>/invoke[-with-response-stream]`
shape and substitute the operator-configured `route.model` for the
caller-supplied model segment. Sandbox code that hardcodes a
different model still works (we don't reject on mismatch), but the
operator's configured model is what reaches the upstream / bridge.
If the inbound path is somehow not a recognized Bedrock shape on a
Bedrock route (the L7 pattern detector upstream of the router
should never produce this combination), reject with
RouterError::Internal naming the offending path rather than
forwarding verbatim.
2. **Skip body-model injection for Bedrock routes.** The existing body
rewriter unconditionally inserts `route.model` into the JSON body
for non-Vertex routes. AWS Bedrock InvokeModel encodes the model
in the URL path; the body is the raw provider-specific payload
(Anthropic Messages for Claude, Mistral payload for Mistral, etc.)
and must not be mutated. The branch ordering is now:
needs_vertex_anthropic_version → strip body model + inject
anthropic_version; route_is_bedrock → leave body alone; else →
inject route.model (existing default).
New helpers, all in `crates/openshell-router/src/backend.rs`:
- `route_is_bedrock(route)` — true when route.protocols contains
aws_bedrock_invoke or aws_bedrock_invoke_stream.
- `parse_bedrock_invocation_path(path)` — returns
Some((model_id, "/invoke" | "/invoke-with-response-stream")) for
paths matching the recognized Bedrock shapes. Strips query strings.
Rejects empty model ids and multi-segment ids (defense-in-depth
matching the L7 pattern detector's existing guards).
- `rewrite_bedrock_path(route, path)` — returns the path with the
caller's model segment replaced by route.model.
Test coverage in the same file (9 new tests):
- parse_bedrock_invocation_path: positive cases for both invoke
variants, query-string stripping; negative cases for empty model id,
multi-segment id, unknown action, wrong prefix, missing slash.
- route_is_bedrock: matches both protocol variants singly and
combined; rejects openai_chat_completions.
- rewrite_bedrock_path: substitutes operator model on both invoke
variants; returns None for non-Bedrock paths.
- bedrock_route_rewrites_model_in_path_and_preserves_body
(wiremock end-to-end): caller sends /model/some-other-model/invoke
with a body containing model: "caller-supplied-model-name". Mock
asserts the upstream receives /model/<operator-model>/invoke and the
body's model field is the caller's value (NOT route.model) — proves
both the path rewrite and the body preservation.
- bedrock_route_streaming_rewrites_model_in_path: same contract for
invoke-with-response-stream.
- bedrock_route_rejects_non_bedrock_path: defense-in-depth coverage of
the Internal-error path when a Bedrock route receives a path that
doesn't match Bedrock shape.
Verification (in dev container):
- cargo test -p openshell-router --lib: 53/53 (incl. 9 new)
- cargo fmt --check: clean
- cargo clippy -p openshell-core -p openshell-router -p openshell-server
--all-targets -- -D warnings: clean
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(sandbox/l7): declare framing for Bedrock patterns
Closes the buffered-vs-streaming framing warning from gator-agent's
re-check on 4ab587f1: "Bedrock InvokeModel should be buffered while
InvokeModelWithResponseStream is streaming. Please add framing/coverage
so /model/{id}/invoke cannot be corrupted by the streaming proxy's
truncation/error-frame behavior."
The InferenceApiPattern struct gained a `framing: ResponseFraming`
field upstream after the original Bedrock-patterns commit (#22b78cff)
landed; the cherry-pick onto current upstream/main left the two
Bedrock entries without the new field. Fixed here:
- aws_bedrock_invoke (POST /model/{id}/invoke):
framing = ResponseFraming::Buffered
InvokeModel returns one JSON object the caller decodes whole. Sending
it through the streaming proxy would risk a mid-body size-cap
truncation or idle-timeout failure appending an SSE error event onto
bytes the caller decodes as one JSON body — the same corruption mode
that drove the existing embeddings + model-discovery to Buffered.
- aws_bedrock_invoke_stream (POST /model/{id}/invoke-with-response-stream):
framing = ResponseFraming::Streaming
InvokeModelWithResponseStream returns an AWS event-stream of binary
chunks; the caller wants chunks incrementally, so the streaming proxy
path is correct.
Two new tests in `crates/openshell-sandbox/src/l7/inference.rs` pin
down the contract:
- aws_bedrock_invoke_is_buffered — detect_inference_pattern returns a
Buffered pattern for /model/<id>/invoke, with explanatory message
naming the corruption mode being prevented.
- aws_bedrock_invoke_stream_is_streaming — same shape, asserting
Streaming for /model/<id>/invoke-with-response-stream.
Verification (in dev container):
- cargo check -p openshell-sandbox: clean (was failing on missing
`framing` field before this commit)
- cargo test -p openshell-sandbox --lib l7::inference::tests::aws_bedrock:
7/7 (incl. 2 new framing tests)
- cargo fmt --check: clean
- cargo clippy --all-targets -- -D warnings: clean
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(inference): drop aws_bedrock_invoke_stream until protocol-aware errors land
Per PR #1704 review (johntmyers): defer Bedrock streaming to a follow-up
that also wires protocol-aware error framing for the AWS event-stream
shape. Until then, surfacing `/model/{id}/invoke-with-response-stream`
risks shipping responses the sandbox cannot interpret on failure.
- AWS_BEDROCK_PROTOCOLS no longer advertises `aws_bedrock_invoke_stream`.
- The L7 inference pattern table drops the streaming entry; only
`aws_bedrock_invoke` (buffered) is recognized.
- Test `aws_bedrock_invoke_stream_pattern_is_deferred` asserts no
pattern claims that protocol so the gap is visible.
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(inference): validate AWS Bedrock model_id at route + rewrite, preserve query
Per PR #1704 review (johntmyers, gator agent): the operator-configured
Bedrock model_id flows verbatim into the upstream URL path; the previous
plumbing left no enforcement that the value was a single benign path
segment, opening a path-injection vector.
Defense in depth, both layers:
* `openshell-server::inference`: new `validate_aws_bedrock_model_id`
rejects empty, leading/trailing whitespace, `/`, `\\`, `?`, `#`, `%`,
`..`, control/whitespace characters. Wired into `resolve_provider_route`
ahead of base_url resolution so the route store cannot persist a
malformed model_id. Mirrors `validate_vertex_model_id` exactly.
* `openshell-router::backend`: `rewrite_bedrock_path` now refuses to
construct the upstream URL unless `route.model` passes
`is_valid_bedrock_model_id`, so even a stale or hand-edited route
cannot reach the wire. The parser also drops the
`/invoke-with-response-stream` arm to match the protocol catalog.
* `parse_bedrock_invocation_path` returns the `?`-prefixed query tail
as a third element; `rewrite_bedrock_path` re-attaches it so any
caller-supplied query string is preserved through the model rewrite.
Tests: 5 unit tests for the validator, 1 integration test that
exercises every unsafe-model_id reject path through
`upsert_cluster_inference_route`, plus a router-side rewrite-rejects
test covering 11 unsafe `route.model` values. All 72 server inference
tests + router tests pass.
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* fix(aws-bedrock): make built-in profile non-egress-granting; tighten docs
Per PR #1704 review (johntmyers): a single profile that is "usable as
is" — not a profile that auto-grants direct AWS Bedrock egress to any
sandbox that selects it. The bridge IS the egress point and is
operator-managed; the profile must not implicitly punch a hole through
the cluster's network policy on its behalf.
* `providers/aws-bedrock.yaml`: clear `endpoints` and `binaries` to
empty arrays. Rewrite the description to spell out that the profile
is intentionally non-egress-granting, that operators are responsible
for declaring their bridge's egress endpoint and binary attribution,
and that the SigV4 follow-up will repopulate these fields once
router-side signing exists.
* `docs/sandboxes/inference-routing.mdx`:
- Drop the `InvokeModelWithResponseStream` row from the supported
patterns table (matches the protocol-catalog + L7-pattern drop).
- Update the `--no-verify` paragraph to reference only
`aws_bedrock_invoke`.
- Generalise the placeholder-credential rationale: any
standalone-router profile registering `AuthHeader::None` will hit
the same non-empty-credentials structural requirement.
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* style(router): satisfy cargo fmt for Bedrock query-tail extraction
CI's rustfmt rejected the one-line `let (path_only, query_tail) = path.find('?').map_or(...)`
shape from commit c51160e4 and required the chained-method layout
instead. Functional behaviour is unchanged; tests still pass.
Signed-off-by: st-gr <38470677+st-gr@users.noreply.github.com>
* style(router): drop unnecessary `crate::` prefix on RouterError matcher
`RouterError` is already imported at the top of the file, so the
two test-side `matches!(result, Err(crate::RouterError::UpstreamProtocol(_)))`
uses trip clippy's `unused_qualifications` under `-D warnings`. Drop
the prefix on both sites; functional behaviour is unchanged.
These warnings predate this PR (originated in
|