feat(sandbox,gateway): route sandbox egress through corporate HTTP proxy#2245
feat(sandbox,gateway): route sandbox egress through corporate HTTP proxy#2245feloy wants to merge 5 commits into
Conversation
PR Review StatusValidation: This is project-valid work for the enterprise restricted-egress gap in #1792. Maintainer triage confirmed the need for corporate proxy chaining after OpenShell policy enforcement, and the PR includes the relevant Podman configuration plus Fern and architecture documentation. Head SHA: Review findings:
Tests should cover the SSRF/DNS binding, CONNECT over-read, invalid-config fail-closed behavior, plain HTTP against a standard forward proxy, combined Podman environment precedence, and HTTPS host-gateway bypass. Runtime proxy behavior also requires Docs: The existing Fern gateway reference is the correct page and no @feloy, please push an updated commit addressing the items above. Gator will re-review the new head before starting the E2E gate. Next state: |
- 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>
ca5c041 to
7a1f8f3
Compare
PR Review StatusValidation: This remains project-valid work for the corporate restricted-egress gap in #1792. The current scope covers the supervisor proxy path and Podman operator configuration, with the relevant Fern and architecture documentation present. Head SHA: The fresh independent code review found these blocking items on the current head:
Also tighten port-qualified The prior control-character concern is not a header-injection blocker because decoded Basic credentials are Base64-encoded before insertion, though rejecting ASCII controls is still sensible input validation. Docs:
Next state: |
PR Review StatusValidation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, and the relevant Fern and architecture documentation is present. Head SHA: The newest commit resolves the prior proxy-selection ownership finding: reserved The fresh independent code review found these remaining blocking items:
Next state: |
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>
9f1b270 to
42291f2
Compare
PR Review StatusValidation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, with the relevant Fern and architecture documentation present. Head SHA: The newest commit resolves the prior child-environment part of the credential finding: reserved The fresh independent code review found these remaining blocking items:
Please add regression coverage for the SSRF/DNS binding, fail-closed startup, HTTP wire form, HTTPS host-gateway bypass, CONNECT over-read, port-aware
Next state: |
…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>
PR Review StatusValidation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, and the relevant Fern and architecture documentation is present. Head SHA: The newest commit resolves the prior credential-storage concern substantially: proxy credentials now use a root-only per-sandbox Podman secret, the reserved auth-file path is stripped from workload children, and generated gateway configuration is mode The fresh independent code review found these remaining blocking items:
Docs: Please add focused regressions for validated-IP binding, fatal invalid config/auth handling, absolute-form HTTP forwarding, CONNECT over-read, HTTPS host-gateway bypass, port-aware
Next state: |
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>
PR Review StatusValidation: This remains project-valid work for the corporate restricted-egress gap in #1792. The supervisor and Podman scope is coherent, and the existing Fern gateway reference is the right documentation location. Head SHA: The latest commit materially improves the fail-closed configuration path: non-empty malformed or unsupported proxy URLs, inline credentials, and unreadable or malformed auth-file contents now fail closed through shared URL validation. The operator-owned environment boundary, workload stripping, and Podman secret staging also remain sound. The fresh independent code review found these remaining blocking items:
Docs/tests: No
Next state: |
… 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.
Summary
HTTPS_PROXY/HTTP_PROXY/NO_PROXY)Related Issue
Part of #1792
Changes
openshell-supervisor-network: newupstream_proxymodule reads standard proxy env vars, implements CONNECT handshake with optional Basic auth, and respectsNO_PROXYbypass rules.dial_upstream()replaces the directTcpStream::connectat the single post-SSRF connect site.openshell-driver-podman:PodmanComputeConfiggainshttps_proxy,http_proxy,no_proxyfields with validation (onlyhttp://schemes accepted). Container creation injects these as environment variables.docs/reference/gateway-config.mdx: documents the new proxy config fields.architecture/sandbox.md: updated with corporate proxy data flow.Testing
Unit tests for
UpstreamProxyConfigparsing,NO_PROXYmatching (wildcards, CIDR, ports, loopback bypass), and CONNECT handshake (success, auth, rejection, timeout)Unit tests for Podman config validation (empty values, unsupported schemes)
Existing proxy and SSRF test suites pass — policy evaluation is unchanged
mise run pre-commitpassesUnit tests added/updated
E2E tests added/updated (if applicable)
Checklist