chore: force-sync upstream (main) - #16
Merged
Merged
Conversation
Bumps [actions/checkout](https://github.com/actions/checkout) from 7.0.0 to 7.0.1. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@9c091bb...3d3c42e) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
) * 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.
…2236) The branches API endpoint cannot handle ref names with slashes (e.g. pull-request/2223). When the API returns a 404, gh api writes the error JSON to stdout before exiting non-zero, so the || fallback never fires. This causes mirror_sha to contain raw JSON like {"message":"Branch not found",...} and the comment displays `{"messa` as the SHA. Switch to the git/ref/heads/ endpoint which handles slashes natively, and add a regex guard to reset mirror_sha to empty when it does not look like a valid 40-char hex SHA. Signed-off-by: Roland Huß <rhuss@redhat.com>
* docs(agents): refresh CLI and debugging skills Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(agents): keep project skills synchronized Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * docs(agents): cover middleware workflows Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
) * fix(proxy): include OPA deny reason in CONNECT 403 response When a CONNECT request was denied by OPA policy, the 403 response used a generic "not permitted by policy" message for both "endpoint not in policy" and "endpoint matched but binary didn't match." Users had no way to distinguish the two without reading supervisor logs. The OPA policy already computes a detailed deny_reason (e.g., "binary '/usr/bin/node' not allowed in policy 'X'") but the proxy was not including it in the HTTP response. Now the CONNECT deny response includes a "reason" field with the OPA deny reason when available. When the reason is empty, the field is omitted for backward compatibility. Fixes NVIDIA#2355 Signed-off-by: Adel Zaalouk <azaalouk@redhat.com> * docs(observability): document optional reason field in CONNECT 403 response The proxy now includes a reason field in the JSON body of denied CONNECT responses when the policy engine provides a specific denial cause. Update the Proxy Error Responses section to show the field and describe when it is present vs omitted. Signed-off-by: Adel Zaalouk <azaalouk@redhat.com> --------- Signed-off-by: Adel Zaalouk <azaalouk@redhat.com>
…A#2391) The scripts/bin/openshell wrapper hardcoded the binary path to $PROJECT_ROOT/target/debug/openshell. When CARGO_TARGET_DIR is set (e.g. via .bashrc or mise), cargo places the binary elsewhere and the wrapper fails with 'No such file or directory'. Use ${CARGO_TARGET_DIR:-$PROJECT_ROOT/target} so the wrapper finds the binary regardless of where the build artifacts live. Signed-off-by: Jesse Jaggars <jjaggars@jjaggars-kubevirt.rht.csb> Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
…IA#2399) Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
* docs(extensibility): add gateway interceptor guide Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(extensibility): list interceptable routes Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(extensibility): stabilize source links Signed-off-by: Drew Newberry <anewberry@nvidia.com> * docs(extensibility): link canonical interceptor routes Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
… NemoClaw redirect (NVIDIA#2405) The OpenClaw community sandbox was removed from NVIDIA/OpenShell-Community in PR NVIDIA#73 (May 16, 2026). The Docker Compose tutorial and docker-compose.yml comment still referenced the stale --from openclaw command and GHCR image. Replace the broken OpenClaw tab with a redirect to the NemoClaw Quickstart, which is the supported path per docs/about/supported-agents.mdx. Remove the stale pre-pull command for the removed image. Fixes NVIDIA#2404 Signed-off-by: Matias Schimuneck <schimuneck.matias@gmail.com>
Signed-off-by: Evie Howard <evhoward@redhat.com> Signed-off-by: Evan Lezar <elezar@nvidia.com>
… events (NVIDIA#2340) * fix(server): unblock sandbox deletion events Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(server): simplify sandbox settings cleanup Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(server): make delete lock ordering explicit Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(server): bind delete worker to stable sandbox id Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * fix(server): disarm canceled queued deletes Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * test: harden sandbox lifecycle cleanup coverage Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Signed-off-by: Kris Hicks <khicks@nvidia.com>
…IDIA#2224) * feat(tui): add config key editing to create provider form Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * feat(tui): add config key editing to update provider form Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): add Up/Down navigation for config_cursor in provider forms Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): show config values in provider detail view Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): improve provider config form validation and reduce duplication Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): fix config deletion and invisible cursor in provider forms Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * test(tui): add tests for config deletion tombstones and cursor scroll window Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): send only config delta on provider update Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): flush pending config input on provider update submit Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): fix provider config form loading, focus reset, and overflow Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): separate config entry navigation from key input focus Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): move config cursor to newly added entry after flush Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> * fix(tui): always populate provider_entries regardless of providers_v2_enabled Signed-off-by: Artem Lytvyn <alytvyn@redhat.com> --------- Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Remove temporary registry layer data before formatting the merged rootfs, preserve formatter execution failures over missing fallbacks, and validate required ext4 tools in rust-vm CI. Closes NVIDIA#2423 Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
…VIDIA#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 NVIDIA#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>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
…IDIA#2359) * refactor(cli): extract shared helpers into commands/common module Move shared formatting, parsing, display, and settings helpers from run.rs into a new commands/common.rs module. run.rs re-exports all public items so no callers change. This establishes the commands/ directory structure for the incremental run.rs split (NVIDIA#2304). Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * refactor(cli): narrow visibility and fix review issues in shared helpers extraction - Remove orphan doc comment left on sandbox_list after print_yaml_line extraction - Change `pub mod commands` to `pub(crate) mod commands` in lib.rs - Move 6 unnecessarily pub-exported items to private use imports in run.rs - Restore stripped rationale comment in parse_cli_setting_value Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * style(cli): fix rustfmt import line wrapping in run.rs Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(cli): restore parse_secret_material_env_pairs public re-export and narrow includes_policy visibility Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> --------- Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
NVIDIA#2341) * fix(sandbox): skip read-only mounts during recursive chown of /sandbox The sandbox supervisor crashed with EROFS when the recursive chown of /sandbox encountered read-only submounts. This is common in gVisor-based Kubernetes deployments where read-only volume mounts are the only way to enforce per-directory immutability (Landlock is unavailable under gVisor). The fix adds two guards to the ownership walk: 1. Mount-boundary detection via st_dev comparison — paths on a different filesystem than /sandbox are skipped entirely, avoiding the chown call on nested read-only mounts. 2. EROFS tolerance — if chown still returns EROFS (e.g. the root mount itself is read-only), the error is logged at debug level and startup continues. Symlink skipping (already present) is preserved and extracted into the recursive walker for consistency. Manually verified on a kind cluster with a read-only PVC mounted at /sandbox/readonly-data: the pod starts successfully, writable paths are owned by the sandbox user, and the read-only mount retains root ownership. Kubernetes e2e test coverage is a separate follow-up. Closes NVIDIA#2294 Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * refactor(sandbox): consolidate symlink check into chown_recursive Move the per-child symlink guard from the directory iteration loop into the top of chown_recursive, reusing the symlink_metadata call that is already performed there. This centralizes all three skip guards (symlink, mount boundary, EROFS) in one place and eliminates a redundant symlink_metadata call per child entry. Suggested-by: elezar Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(sandbox): recurse into children after EROFS and fix doc comment When chown returns EROFS on a directory, the code previously returned early without recursing into children. This skipped writable submounts nested under a read-only directory on the same device (e.g., a writable emptyDir inside a read-only ConfigMap mount sharing the same st_dev). Now the EROFS case falls through to the directory recursion so that writable children are still chowned. Also fixes the doc comment which said EROFS errors are "logged as warnings" when they are actually logged at debug level. Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(sandbox): allow similar_names clippy lint in chown test Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(sandbox): remove cross-device skip and inject chown for EROFS tests Remove the unconditional st_dev boundary check that prevented chown on writable PVC mounts with a different device ID. Rely solely on EROFS errors to skip read-only paths while continuing traversal into children and siblings. Inject the chown operation into chown_recursive to enable unit testing the EROFS recovery path without requiring root or mount privileges. Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * style(sandbox): apply rustfmt to chown test closures Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(sandbox): stop descending into EROFS subtrees to prevent TOCTOU race After chown returns EROFS, return immediately instead of recursing into the read-only directory's children. Descending into a mount we cannot own creates a TOCTOU window where an attacker could swap a child for a symlink between symlink_metadata and chown (CWE-367/CWE-59). Siblings of the EROFS path are still visited by the caller's loop. Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * fix(sandbox): hard-error on root EROFS, log read_dir failures, eliminate double stat Chown root directly in chown_sandbox_home so EROFS on /sandbox itself is a hard startup error rather than being silently swallowed. Extract chown_children to log read_dir failures instead of silently skipping them. This also eliminates the redundant symlink_metadata syscall on the root path. Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> --------- Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Signed-off-by: Varsha Prasad Narsing <vnarsing@nvidia.com>
…2429) * fix(mise): Properly initialize venv for fresh worktrees Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * fix(mise): use uv sync for protobuf generation Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * docs: remove redundant worktree setup note Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Adrien Langou <alangou@nvidia.com>
…NVIDIA#2436) The ListSandboxesRequest proto gained workspace and all_workspaces fields in the workspace feature but the governance interceptor example was not updated, causing a build failure. Set all_workspaces to true so the interceptor's policy reload covers sandboxes across all workspaces. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
* fix(auth): report gateway authentication status Signed-off-by: Drew Newberry <anewberry@nvidia.com> * fix(auth): reuse gateway info for status probe Signed-off-by: Drew Newberry <anewberry@nvidia.com> --------- Signed-off-by: Drew Newberry <anewberry@nvidia.com>
* 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>
* fix: eliminate parallel Rust test flakes Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> * refactor(supervisor-network): name workload proxy TCP connection Signed-off-by: Evan Lezar <elezar@nvidia.com> * refactor(supervisor-network): name procfs address family Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> --------- Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> Co-authored-by: Evan Lezar <elezar@nvidia.com>
… create (NVIDIA#1989) * feat(cli): add --output json/yaml to sandbox get, status, and sandbox create Add structured output support (JSON and YAML) to sandbox lifecycle commands: sandbox get, status, and sandbox create. Introduce ProgressOutput enum to cleanly separate interactive, plain, and silent display modes during sandbox provisioning. Signed-off-by: Roland Huß <rhuss@redhat.com> * fix(cli): reject structured create output with side effects Signed-off-by: Evan Lezar <elezar@nvidia.com> * fix(cli): suppress upload ssh stdout Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Roland Huß <rhuss@redhat.com> Signed-off-by: Evan Lezar <elezar@nvidia.com> Co-authored-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
…oxy (NVIDIA#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 NVIDIA#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> NVIDIA#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 NVIDIA#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 NVIDIA#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 NVIDIA#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>
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 8.3.2 to 9.0.0. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@11f9893...c771a70) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 9.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Move L7 endpoint semantic checks into the openshell-policy crate so both
profile lint and the runtime validator share one implementation. This
eliminates drift between the two validation paths.
The shared validator covers 9 checks: unknown protocol, rules/access
mutual exclusivity, JSON-RPC family access rejection, json-rpc requires
rules, non-JSON-RPC protocol requires rules or access, MCP requires
rules when allow_all is false, rules-would-deny-all detection,
deny_rules require protocol, and deny_rules require base allow set.
Changes rules/deny_rules fields to Option<Vec<...>> so absent vs empty
is distinguishable at lint time. Adds is_effectively_empty() to
L7AllowProfile for deny-all detection of allow: {} objects. Makes
rules_would_deny_all MCP-aware by checking tool/params.name selectors
before classifying a rule as deny-all. Adds params field to
L7AllowProfile so MCP tool selectors survive proto round-trip.
Signed-off-by: Grace Smith <gsmith@redhat.com>
Signed-off-by: Grace Smith <grasmith@redhat.com>
…stream # Conflicts: # .agents/skills/build-from-issue/SKILL.md # .agents/skills/create-github-pr/SKILL.md # .agents/skills/debug-inference/SKILL.md # .agents/skills/generate-sandbox-policy/SKILL.md # .agents/skills/openshell-cli/SKILL.md # .agents/skills/openshell-cli/cli-reference.md # .agents/skills/review-github-pr/SKILL.md # .agents/skills/sync-agent-infra/SKILL.md
|
Important Review skippedToo many files! This PR contains 124 files, which is 24 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (24)
📒 Files selected for processing (124)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
18 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Force-merge -X theirs + iAiFy overlay (wave4-5 runbook). Pre-sync=5a6db5a1326c149e3956b70274f21a86818c0935. Fleet sync 2026-07-23.